diff --git a/controllers/git_test.go b/controllers/git_test.go index b578d804..8d8c5b95 100644 --- a/controllers/git_test.go +++ b/controllers/git_test.go @@ -134,6 +134,9 @@ func TestPushRejected(t *testing.T) { repoURL := gitServer.HTTPAddressWithCredentials() + "/appconfig.git" repo, err := clone(repoURL, "origin", "main") + if err != nil { + t.Fatal(err) + } // This is here to guard against push in general being broken err = push(context.TODO(), repo.Workdir(), "main", repoAccess{ diff --git a/controllers/imageupdateautomation_controller.go b/controllers/imageupdateautomation_controller.go index 16f76a93..583d5325 100644 --- a/controllers/imageupdateautomation_controller.go +++ b/controllers/imageupdateautomation_controller.go @@ -160,7 +160,7 @@ func (r *ImageUpdateAutomationReconciler) Reconcile(ctx context.Context, req ctr // only GitRepository objects are supported for now if kind := auto.Spec.SourceRef.Kind; kind != sourcev1.GitRepositoryKind { - return failWithError(fmt.Errorf("source kind %q not supported", kind)) + return failWithError(fmt.Errorf("source kind '%s' not supported", kind)) } gitSpec := auto.Spec.GitSpec @@ -292,10 +292,7 @@ func (r *ImageUpdateAutomationReconciler) Reconcile(ctx context.Context, req ctr // Use the git operations timeout for the repo. fetchCtx, cancel := context.WithTimeout(ctx, origin.Spec.Timeout.Duration) defer cancel() - if err := fetch(fetchCtx, tmp, pushBranch, access); err != nil && err != errRemoteBranchMissing { - return failWithError(err) - } - if err = switchBranch(repo, pushBranch); err != nil { + if err := switchToBranch(repo, fetchCtx, pushBranch, access); err != nil && err != errRemoteBranchMissing { return failWithError(err) } } @@ -556,25 +553,6 @@ func cloneInto(ctx context.Context, access repoAccess, ref *sourcev1.GitReposito return libgit2.OpenRepository(path) } -// switchBranch switches the repo from the current branch to the -// branch given. If the branch does not exist, it is created using the -// head as the starting point. -func switchBranch(repo *libgit2.Repository, pushBranch string) error { - if err := repo.SetHead(fmt.Sprintf("refs/heads/%s", pushBranch)); err != nil { - head, err := headCommit(repo) - if err != nil { - return err - } - defer head.Free() - - branch, err := repo.CreateBranch(pushBranch, head, false) - defer branch.Free() - return err - } - - return nil -} - func headCommit(repo *libgit2.Repository) (*libgit2.Commit, error) { head, err := repo.Head() if err != nil { @@ -748,35 +726,65 @@ func (r *ImageUpdateAutomationReconciler) getSigningEntity(ctx context.Context, var errRemoteBranchMissing = errors.New("remote branch missing") -// fetch gets the remote branch given and updates the local branch -// head of the same name, so it can be switched to. If the fetch -// completes, it returns nil; if the remote branch is missing, it -// returns errRemoteBranchMissing (this is to work in sympathy with -// `switchBranch`, which will create the branch if it doesn't -// exist). For any other problem it will return the error. -func fetch(ctx context.Context, path string, branch string, access repoAccess) error { - refspec := fmt.Sprintf("refs/heads/%s:refs/heads/%s", branch, branch) - repo, err := libgit2.OpenRepository(path) - if err != nil { +// switchToBranch switches to a branch after fetching latest from upstream. +// If the branch does not exist, it is created using the head as the starting point. +func switchToBranch(repo *libgit2.Repository, ctx context.Context, branch string, access repoAccess) error { + checkoutOpts := &libgit2.CheckoutOpts{ + // the remote branch should take precedence if it exists at this point in time. + Strategy: libgit2.CheckoutForce, + } + + branchRef := fmt.Sprintf("origin/%s", branch) + remoteBranch, err := repo.LookupBranch(branchRef, libgit2.BranchRemote) + if err != nil && !libgit2.IsErrorCode(err, libgit2.ErrorCodeNotFound) { return err } - defer repo.Free() - origin, err := repo.Remotes.Lookup(originRemote) + if remoteBranch != nil { + defer remoteBranch.Free() + } + err = nil + + var commit *libgit2.Commit + // tries to get tip commit from remote branch, if it exists. + // otherwise gets the commit that local head is pointing to. + if remoteBranch != nil { + commit, err = repo.LookupCommit(remoteBranch.Target()) + } else { + head, err := repo.Head() + if err != nil { + return fmt.Errorf("cannot get repo head: %w", err) + } + commit, err = repo.LookupCommit(head.Target()) + } if err != nil { - return err + return fmt.Errorf("cannot find the head commit: %w", err) } - defer origin.Free() - err = origin.Fetch( - []string{refspec}, - &libgit2.FetchOptions{ - RemoteCallbacks: access.remoteCallbacks(ctx), - ProxyOptions: libgit2.ProxyOptions{Type: libgit2.ProxyTypeAuto}, - }, "", - ) - if err != nil && libgit2.IsErrorCode(err, libgit2.ErrorCodeNotFound) { - return errRemoteBranchMissing + defer commit.Free() + + localBranch, err := repo.LookupBranch(branch, libgit2.BranchLocal) + if err != nil && !libgit2.IsErrorCode(err, libgit2.ErrorCodeNotFound) { + return fmt.Errorf("cannot lookup branch '%s': %w", branch, err) + } + if localBranch == nil { + localBranch, err = repo.CreateBranch(branch, commit, false) } - return err + if localBranch == nil { + return fmt.Errorf("cannot create local branch '%s': %w", branch, err) + } + defer localBranch.Free() + + tree, err := repo.LookupTree(commit.TreeId()) + if err != nil { + return fmt.Errorf("cannot lookup tree for branch '%s': %w", branch, err) + } + defer tree.Free() + + err = repo.CheckoutTree(tree, checkoutOpts) + if err != nil { + return fmt.Errorf("cannot checkout tree for branch '%s': %w", branch, err) + } + + return repo.SetHead("refs/heads/" + branch) } // push pushes the branch given to the origin using the git library diff --git a/controllers/update_test.go b/controllers/update_test.go index c50668bc..5248df71 100644 --- a/controllers/update_test.go +++ b/controllers/update_test.go @@ -990,6 +990,10 @@ Images: }) It("creates and pushes the push branch", func() { + initialHead, err := headFromBranch(localRepo, branch) + Expect(err).ToNot(HaveOccurred()) + defer initialHead.Free() + // pull the head commit we just pushed, so it's not // considered a new commit when checking for a commit // made by automation. @@ -1003,6 +1007,13 @@ Images: Expect(err).ToNot(HaveOccurred()) defer commit.Free() Expect(commit.Message()).To(Equal(commitMessage)) + + // previous commits should still exist in the tree + // checks for regression in which the new branch had only the new commit. + oldCommit, err := localRepo.LookupCommit(initialHead.Id()) + Expect(err).ToNot(HaveOccurred()) + Expect(oldCommit).ToNot(BeNil()) + defer commit.Free() }) It("pushes another commit to the existing push branch", func() { @@ -1731,6 +1742,10 @@ func rebase(repo *git2go.Repository, sourceBranch, targetBranch string) (*git2go // Check operation index is correct rebaseOperationIndex, err = rebase.CurrentOperationIndex() + if err != nil { + return nil, err + } + if int(rebaseOperationIndex) != op { return nil, errors.New("Bad operation index") }