diff --git a/controllers/git_test.go b/controllers/git_test.go index a163030f..90bc3402 100644 --- a/controllers/git_test.go +++ b/controllers/git_test.go @@ -10,6 +10,9 @@ import ( "github.com/go-logr/logr" libgit2 "github.com/libgit2/git2go/v33" + "k8s.io/apimachinery/pkg/types" + + . "github.com/onsi/gomega" "github.com/fluxcd/pkg/gittestserver" "github.com/fluxcd/source-controller/pkg/git/libgit2/managed" @@ -129,12 +132,12 @@ func TestPushRejected(t *testing.T) { } // this is currently defined in update_test.go, but handy right here .. - if err = initGitRepo(gitServer, "testdata/appconfig", "master", "/appconfig.git"); err != nil { + if err = initGitRepo(gitServer, "testdata/appconfig", "test", "/appconfig.git"); err != nil { t.Fatal(err) } repoURL := gitServer.HTTPAddressWithCredentials() + "/appconfig.git" - repo, err := clone(repoURL, "master") + repo, err := clone(repoURL, "test") if err != nil { t.Fatal(err) } @@ -148,7 +151,7 @@ func TestPushRejected(t *testing.T) { repo.Remotes.SetUrl("origin", transportOptsURL) // This is here to guard against push in general being broken - err = push(context.TODO(), repo.Workdir(), "master", repoAccess{}) + err = push(context.TODO(), repo.Workdir(), "test", repoAccess{}) if err != nil { t.Fatal(err) } @@ -165,3 +168,93 @@ func TestPushRejected(t *testing.T) { t.Error("push to a forbidden branch is expected to fail, but succeeded") } } + +func Test_switchToBranch(t *testing.T) { + g := NewWithT(t) + gitServer, err := gittestserver.NewTempGitServer() + g.Expect(err).ToNot(HaveOccurred()) + gitServer.AutoCreate() + g.Expect(gitServer.StartHTTP()).To(Succeed()) + + branch := "test" + g.Expect(initGitRepo(gitServer, "testdata/appconfig", branch, "/appconfig.git")).To(Succeed()) + + repoURL := gitServer.HTTPAddressWithCredentials() + "/appconfig.git" + repo, err := clone(repoURL, branch) + g.Expect(err).ToNot(HaveOccurred()) + defer repo.Free() + + head, err := repo.Head() + g.Expect(err).ToNot(HaveOccurred()) + defer head.Free() + target := head.Target() + + // register transport options and update remote to transport url + transportOptsURL := "http://" + randStringRunes(5) + managed.AddTransportOptions(transportOptsURL, managed.TransportOptions{ + TargetURL: repoURL, + }) + defer managed.RemoveTransportOptions(transportOptsURL) + repo.Remotes.SetUrl("origin", transportOptsURL) + + // calling switchToBranch with a branch that doesn't exist on origin + // should result in the branch being created and switched to. + branch = "not-on-origin" + switchToBranch(repo, context.TODO(), branch, repoAccess{}) + + head, err = repo.Head() + g.Expect(err).ToNot(HaveOccurred()) + name, err := head.Branch().Name() + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(name).To(Equal(branch)) + + cc, err := repo.LookupCommit(head.Target()) + g.Expect(err).ToNot(HaveOccurred()) + defer cc.Free() + g.Expect(cc.Id().String()).To(Equal(target.String())) + + // create a branch with the HEAD commit and push it to origin + branch = "exists-on-origin" + _, err = repo.CreateBranch(branch, cc, false) + g.Expect(err).ToNot(HaveOccurred()) + origin, err := repo.Remotes.Lookup("origin") + g.Expect(err).ToNot(HaveOccurred()) + defer origin.Free() + + g.Expect(origin.Push( + []string{fmt.Sprintf("refs/heads/%s:refs/heads/%s", branch, branch)}, &libgit2.PushOptions{}, + )).To(Succeed()) + + // push a new commit to the switch branch. this is done to test whether we properly + // sync our local branch with the remote branch, before switching. + policyKey := types.NamespacedName{ + Name: "policy", + Namespace: "ns", + } + commitID := commitInRepo(g, repoURL, branch, "Install setter marker", func(tmp string) { + g.Expect(replaceMarker(tmp, policyKey)).To(Succeed()) + }) + + // calling switchToBranch with a branch that exists should make sure to fetch latest + // for that branch from origin, and then switch to it. + switchToBranch(repo, context.TODO(), branch, repoAccess{}) + head, err = repo.Head() + g.Expect(err).ToNot(HaveOccurred()) + name, err = head.Branch().Name() + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(name).To(Equal(branch)) + g.Expect(head.Target().String()).To(Equal(commitID.String())) + + // push a commit after switching to the switch branch, to check if the local + // branch is synced with origin. + replaceMarker(repo.Workdir(), policyKey) + sig := &libgit2.Signature{ + Name: "Testbot", + Email: "test@example.com", + When: time.Now(), + } + _, err = commitWorkDir(repo, branch, "update policy", sig) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(push(context.TODO(), repo.Workdir(), branch, repoAccess{})).To(Succeed()) +} diff --git a/controllers/imageupdateautomation_controller.go b/controllers/imageupdateautomation_controller.go index 4aa99128..3b940d7c 100644 --- a/controllers/imageupdateautomation_controller.go +++ b/controllers/imageupdateautomation_controller.go @@ -749,7 +749,6 @@ func switchToBranch(repo *libgit2.Repository, ctx context.Context, branch string callbacks = managed.RemoteCallbacks() } - branchRef := fmt.Sprintf("origin/%s", branch) // Force the fetching of the remote branch. err = origin.Fetch([]string{branch}, &libgit2.FetchOptions{ RemoteCallbacks: callbacks, @@ -758,7 +757,7 @@ func switchToBranch(repo *libgit2.Repository, ctx context.Context, branch string return fmt.Errorf("cannot fetch remote branch: %w", err) } - remoteBranch, err := repo.LookupBranch(branchRef, libgit2.BranchRemote) + remoteBranch, err := repo.References.Lookup(fmt.Sprintf("refs/remotes/origin/%s", branch)) if err != nil && !libgit2.IsErrorCode(err, libgit2.ErrorCodeNotFound) { return err } @@ -785,15 +784,28 @@ func switchToBranch(repo *libgit2.Repository, ctx context.Context, branch string } defer commit.Free() - localBranch, err := repo.LookupBranch(branch, libgit2.BranchLocal) + localBranch, err := repo.References.Lookup(fmt.Sprintf("refs/heads/%s", branch)) 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) - } - if localBranch == nil { - return fmt.Errorf("cannot create local branch '%s': %w", branch, err) + // We could've done something like: + // lb, err := repo.CreateBranch() + // if lb != nil { + // defer lb.Free() + // localBranch = lb.Reference + // } + // But for some reasone, calling `lb.Free()` under any circumstances, causes a really + // nasty crash. Since not calling `lb.Free()` could result in memory leaks, we avoid + // assigning a variable like `lb` and get the branch reference manually. + _, err := repo.CreateBranch(branch, commit, false) + if err != nil { + return fmt.Errorf("cannot create local branch '%s': %w", branch, err) + } + localBranch, err = repo.References.Lookup(fmt.Sprintf("refs/heads/%s", branch)) + if err != nil { + return fmt.Errorf("cannot lookup branch '%s': %w", branch, err) + } } defer localBranch.Free() @@ -811,6 +823,11 @@ func switchToBranch(repo *libgit2.Repository, ctx context.Context, branch string return fmt.Errorf("cannot checkout tree for branch '%s': %w", branch, err) } + _, err = localBranch.SetTarget(commit.Id(), "") + if err != nil { + return fmt.Errorf("cannot update branch '%s' to be at target commit: %w", branch, err) + } + return repo.SetHead("refs/heads/" + branch) } diff --git a/controllers/update_test.go b/controllers/update_test.go index bd1da1ce..315489c2 100644 --- a/controllers/update_test.go +++ b/controllers/update_test.go @@ -904,7 +904,7 @@ func configureManagedTransportOptions(repo *libgit2.Repository, repoURL string) }, nil } -func commitInRepo(g *WithT, repoURL, branch, msg string, changeFiles func(path string)) { +func commitInRepo(g *WithT, repoURL, branch, msg string, changeFiles func(path string)) *libgit2.Oid { repo, err := clone(repoURL, branch) g.Expect(err).ToNot(HaveOccurred()) defer repo.Free() @@ -916,21 +916,18 @@ func commitInRepo(g *WithT, repoURL, branch, msg string, changeFiles func(path s Email: "test@example.com", When: time.Now(), } - _, err = commitWorkDir(repo, branch, msg, sig) + id, err := commitWorkDir(repo, branch, msg, sig) g.Expect(err).ToNot(HaveOccurred()) cleanup, err := configureManagedTransportOptions(repo, repoURL) - if err != nil { - panic(err) - } + g.Expect(err).ToNot(HaveOccurred()) defer cleanup() origin, err := repo.Remotes.Lookup(originRemote) - if err != nil { - panic(fmt.Errorf("cannot find origin: %v", err)) - } + g.Expect(err).ToNot(HaveOccurred()) defer origin.Free() g.Expect(origin.Push([]string{branchRefName(branch)}, &libgit2.PushOptions{})).To(Succeed()) + return id } // Initialise a git server with a repo including the files in dir.