From e862a1b9a5d5aeb30debf6abec656ce13ac7d411 Mon Sep 17 00:00:00 2001 From: Sanskar Jaiswal Date: Wed, 1 Jun 2022 19:51:49 +0530 Subject: [PATCH] refactor controller and git tests to use managed transport Signed-off-by: Sanskar Jaiswal --- controllers/git_test.go | 23 +-- .../imageupdateautomation_controller.go | 1 + controllers/suite_test.go | 4 + controllers/update_test.go | 136 +++++++++++++----- 4 files changed, 116 insertions(+), 48 deletions(-) diff --git a/controllers/git_test.go b/controllers/git_test.go index 8d8c5b95..a163030f 100644 --- a/controllers/git_test.go +++ b/controllers/git_test.go @@ -12,6 +12,7 @@ import ( libgit2 "github.com/libgit2/git2go/v33" "github.com/fluxcd/pkg/gittestserver" + "github.com/fluxcd/source-controller/pkg/git/libgit2/managed" ) func populateRepoFromFixture(repo *libgit2.Repository, fixture string) error { @@ -128,21 +129,26 @@ func TestPushRejected(t *testing.T) { } // this is currently defined in update_test.go, but handy right here .. - if err = initGitRepo(gitServer, "testdata/appconfig", "main", "/appconfig.git"); err != nil { + if err = initGitRepo(gitServer, "testdata/appconfig", "master", "/appconfig.git"); err != nil { t.Fatal(err) } repoURL := gitServer.HTTPAddressWithCredentials() + "/appconfig.git" - repo, err := clone(repoURL, "origin", "main") + repo, err := clone(repoURL, "master") if err != nil { t.Fatal(err) } + defer repo.Free() - // This is here to guard against push in general being broken - err = push(context.TODO(), repo.Workdir(), "main", repoAccess{ - url: repoURL, - auth: nil, + transportOptsURL := "http://" + randStringRunes(5) + managed.AddTransportOptions(transportOptsURL, managed.TransportOptions{ + TargetURL: repoURL, }) + defer managed.RemoveTransportOptions(transportOptsURL) + repo.Remotes.SetUrl("origin", transportOptsURL) + + // This is here to guard against push in general being broken + err = push(context.TODO(), repo.Workdir(), "master", repoAccess{}) if err != nil { t.Fatal(err) } @@ -154,10 +160,7 @@ func TestPushRejected(t *testing.T) { // This is supposed to fail, because the hook rejects the branch // pushed to. - err = push(context.TODO(), repo.Workdir(), branch, repoAccess{ - url: repoURL, - auth: nil, - }) + err = push(context.TODO(), repo.Workdir(), branch, repoAccess{}) if err == nil { t.Error("push to a forbidden branch is expected to fail, but succeeded") } diff --git a/controllers/imageupdateautomation_controller.go b/controllers/imageupdateautomation_controller.go index 801be227..4aa99128 100644 --- a/controllers/imageupdateautomation_controller.go +++ b/controllers/imageupdateautomation_controller.go @@ -510,6 +510,7 @@ type repoAccess struct { func (r *ImageUpdateAutomationReconciler) getRepoAccess(ctx context.Context, repository *sourcev1.GitRepository) (repoAccess, error) { var access repoAccess access.url = repository.Spec.URL + access.auth = &git.AuthOptions{} if repository.Spec.SecretRef != nil { name := types.NamespacedName{ diff --git a/controllers/suite_test.go b/controllers/suite_test.go index 35e66600..7d6a2817 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -24,6 +24,7 @@ import ( "testing" "time" + "github.com/go-logr/logr" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/kubernetes/scheme" ctrl "sigs.k8s.io/controller-runtime" @@ -31,6 +32,7 @@ import ( imagev1_reflect "github.com/fluxcd/image-reflector-controller/api/v1beta1" "github.com/fluxcd/pkg/runtime/testenv" sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" + "github.com/fluxcd/source-controller/pkg/git/libgit2/managed" imagev1 "github.com/fluxcd/image-automation-controller/api/v1beta1" // +kubebuilder:scaffold:imports @@ -61,6 +63,8 @@ func TestMain(m *testing.M) { filepath.Join("testdata", "crds"), )) + managed.InitManagedTransport(logr.Discard()) + controllerName := "image-automation-controller" if err := (&ImageUpdateAutomationReconciler{ Client: testEnv, diff --git a/controllers/update_test.go b/controllers/update_test.go index ab6ddf0a..bd1da1ce 100644 --- a/controllers/update_test.go +++ b/controllers/update_test.go @@ -54,6 +54,7 @@ import ( "github.com/fluxcd/pkg/gittestserver" "github.com/fluxcd/pkg/ssh" sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" + "github.com/fluxcd/source-controller/pkg/git/libgit2/managed" imagev1 "github.com/fluxcd/image-automation-controller/api/v1beta1" "github.com/fluxcd/image-automation-controller/pkg/test" @@ -405,10 +406,9 @@ func TestImageAutomationReconciler_signedCommit(t *testing.T) { } func TestImageAutomationReconciler_e2e(t *testing.T) { - gitImpls := []string{sourcev1.GoGitImplementation, sourcev1.LibGit2Implementation} protos := []string{"http", "ssh"} - testFunc := func(t *testing.T, proto string, impl string) { + testFunc := func(t *testing.T, proto string) { g := NewWithT(t) const latestImage = "helloworld:1.0.1" @@ -462,10 +462,10 @@ func TestImageAutomationReconciler_e2e(t *testing.T) { // in a secret. err = createSSHIdentitySecret(testEnv, gitSecretName, namespace, repoURL) g.Expect(err).ToNot(HaveOccurred()) - err = createGitRepository(testEnv, gitRepoName, namespace, impl, repoURL, gitSecretName) + err = createGitRepository(testEnv, gitRepoName, namespace, repoURL, gitSecretName) g.Expect(err).ToNot(HaveOccurred()) } else { - err = createGitRepository(testEnv, gitRepoName, namespace, impl, repoURL, "") + err = createGitRepository(testEnv, gitRepoName, namespace, repoURL, "") g.Expect(err).ToNot(HaveOccurred()) } @@ -480,8 +480,12 @@ func TestImageAutomationReconciler_e2e(t *testing.T) { t.Run("PushSpec", func(t *testing.T) { // Clone the repo locally. - localRepo, err := clone(cloneLocalRepoURL, "origin", branch) + localRepo, err := clone(cloneLocalRepoURL, branch) g.Expect(err).ToNot(HaveOccurred(), "failed to clone git repo") + defer localRepo.Free() + origin, err := localRepo.Remotes.Lookup("origin") + g.Expect(err).ToNot(HaveOccurred(), "failed to clone git repo") + defer origin.Free() // NB not testing the image reflector controller; this // will make a "fully formed" ImagePolicy object. @@ -613,8 +617,9 @@ func TestImageAutomationReconciler_e2e(t *testing.T) { // test helper. When switching branches, the localRepo seems to get // stuck in one particular branch. As a workaround, create a // separate localRepo. - localRepo, err := clone(cloneLocalRepoURL, "origin", branch) + localRepo, err := clone(cloneLocalRepoURL, branch) g.Expect(err).ToNot(HaveOccurred(), "failed to clone git repo") + defer localRepo.Free() g.Expect(checkoutBranch(localRepo, branch)).ToNot(HaveOccurred()) err = createImagePolicyWithLatestImage(testEnv, imagePolicyName, namespace, "not-expected-to-exist", "1.x", latestImage) @@ -736,13 +741,10 @@ func TestImageAutomationReconciler_e2e(t *testing.T) { }) } - // Run the protocol based e2e tests against the git implementations. - for _, gitImpl := range gitImpls { - for _, proto := range protos { - t.Run(fmt.Sprintf("%s_%s", gitImpl, proto), func(t *testing.T) { - testFunc(t, proto, gitImpl) - }) - } + for _, proto := range protos { + t.Run(fmt.Sprintf("%s", proto), func(t *testing.T) { + testFunc(t, proto) + }) } } @@ -861,8 +863,9 @@ func compareRepoWithExpected(g *WithT, repoURL, branch, fixture string, changeFi copy.Copy(fixture, expected) changeFixture(expected) - repo, err := clone(repoURL, "origin", branch) + repo, err := clone(repoURL, branch) g.Expect(err).ToNot(HaveOccurred()) + defer repo.Free() // NOTE: The workdir contains a trailing /. Clean it to not confuse the // DiffDirectories(). actual := filepath.Clean(repo.Workdir()) @@ -872,10 +875,39 @@ func compareRepoWithExpected(g *WithT, repoURL, branch, fixture string, changeFi test.ExpectMatchingDirectories(g, actual, expected) } +// configureManagedTransportOptions registers the transport options for this repository +// and sets the remote url of origin to the transport options url. If repoURL is empty +// it tries to figure it out by looking at the remote url of origin. It returns a function +// which removes the transport options for this repo and sets the remote url of the origin +// back to the actual url. Callers are expected to call this function in a deferred manner. +func configureManagedTransportOptions(repo *libgit2.Repository, repoURL string) (func(), error) { + if repoURL == "" { + origin, err := repo.Remotes.Lookup(originRemote) + if err != nil { + return nil, err + } + defer origin.Free() + repoURL = origin.Url() + } + transportOptsURL := "http://" + randStringRunes(5) + managed.AddTransportOptions(transportOptsURL, managed.TransportOptions{ + TargetURL: repoURL, + }) + + err := repo.Remotes.SetUrl(originRemote, transportOptsURL) + if err != nil { + return nil, fmt.Errorf("could not set remote origin url: %v", err) + } + return func() { + managed.RemoveTransportOptions(transportOptsURL) + repo.Remotes.SetUrl(originRemote, repoURL) + }, nil +} + func commitInRepo(g *WithT, repoURL, branch, msg string, changeFiles func(path string)) { - originRemote := "origin" - repo, err := clone(repoURL, originRemote, branch) + repo, err := clone(repoURL, branch) g.Expect(err).ToNot(HaveOccurred()) + defer repo.Free() changeFiles(repo.Workdir()) @@ -887,6 +919,11 @@ func commitInRepo(g *WithT, repoURL, branch, msg string, changeFiles func(path s _, err = commitWorkDir(repo, branch, msg, sig) g.Expect(err).ToNot(HaveOccurred()) + cleanup, err := configureManagedTransportOptions(repo, repoURL) + if err != nil { + panic(err) + } + defer cleanup() origin, err := repo.Remotes.Lookup(originRemote) if err != nil { panic(fmt.Errorf("cannot find origin: %v", err)) @@ -941,7 +978,7 @@ func initGitRepoPlain(fixture, repositoryPath string) (*git2go.Repository, error } func headFromBranch(repo *git2go.Repository, branchName string) (*git2go.Commit, error) { - branch, err := repo.LookupBranch(branchName, git2go.BranchAll) + branch, err := repo.LookupBranch(branchName, git2go.BranchLocal) if err != nil { return nil, err } @@ -1123,40 +1160,53 @@ func mockSignature(time time.Time) *git2go.Signature { } } -func clone(repoURL, remoteName, branchName string) (*git2go.Repository, error) { +func clone(repoURL, branchName string) (*git2go.Repository, error) { dir, err := os.MkdirTemp("", "iac-clone-*") if err != nil { return nil, err } + transportOptsURL := "http://" + randStringRunes(5) + managed.AddTransportOptions(transportOptsURL, managed.TransportOptions{ + TargetURL: repoURL, + }) + defer managed.RemoveTransportOptions(transportOptsURL) + opts := &git2go.CloneOptions{ Bare: false, CheckoutBranch: branchName, CheckoutOptions: git2go.CheckoutOptions{ Strategy: git2go.CheckoutForce, }, - FetchOptions: git2go.FetchOptions{ - RemoteCallbacks: git2go.RemoteCallbacks{ - CertificateCheckCallback: func(cert *git2go.Certificate, valid bool, hostname string) error { - return nil - }, - }, - }, } + repo, err := git2go.Clone(transportOptsURL, dir, opts) - return git2go.Clone(repoURL, dir, opts) + // set the origin remote url to the actual repo url, since + // the origin remote will have transportOptsURl as the it's url + // because that's the url used to clone the repo. + err = repo.Remotes.SetUrl("origin", repoURL) + if err != nil { + return nil, err + } + return repo, nil } func waitForNewHead(g *WithT, repo *git2go.Repository, branch, preChangeHash string) { var commitToResetTo *git2go.Commit + cleanup, err := configureManagedTransportOptions(repo, "") + if err != nil { + panic(err) + } + defer cleanup() + + origin, err := repo.Remotes.Lookup("origin") + if err != nil { + panic("origin not set") + } + defer origin.Free() + // Now try to fetch new commits from that remote branch g.Eventually(func() bool { - origin, err := repo.Remotes.Lookup("origin") - if err != nil { - panic("origin not set") - } - defer origin.Free() - if err := origin.Fetch( []string{branchRefName(branch)}, &libgit2.FetchOptions{}, "", @@ -1205,6 +1255,12 @@ func commitIdFromBranch(repo *git2go.Repository, branchName string) string { } func getRemoteHead(repo *git2go.Repository, branchName string) (*git2go.Oid, error) { + cleanup, err := configureManagedTransportOptions(repo, "") + if err != nil { + return nil, err + } + defer cleanup() + remote, err := repo.Remotes.Lookup("origin") if err != nil { return nil, err @@ -1385,11 +1441,17 @@ func testWithCustomRepoAndImagePolicy( // Clone the repo. repoURL := gitServer.HTTPAddressWithCredentials() + repositoryPath - localRepo, err := clone(repoURL, "origin", args.branch) + localRepo, err := clone(repoURL, args.branch) g.Expect(err).ToNot(HaveOccurred(), "failed to clone git repo") + defer localRepo.Free() + + origin, err := localRepo.Remotes.Lookup("origin") + g.Expect(err).ToNot(HaveOccurred(), "failed to look up remote origin") + defer origin.Free() + localRepo.Remotes.SetUrl("origin", repoURL) // Create GitRepository resource for the above repo. - err = createGitRepository(kClient, args.gitRepoName, args.gitRepoNamespace, "", repoURL, "") + err = createGitRepository(kClient, args.gitRepoName, args.gitRepoNamespace, repoURL, "") g.Expect(err).ToNot(HaveOccurred(), "failed to create GitRepository resource") // Create ImagePolicy with populated latest image in the status. @@ -1440,7 +1502,7 @@ func createNamespace(kClient client.Client, name string) (cleanup, error) { return cleanup, nil } -func createGitRepository(kClient client.Client, name, namespace, impl, repoURL, secretRef string) error { +func createGitRepository(kClient client.Client, name, namespace, repoURL, secretRef string) error { gitRepo := &sourcev1.GitRepository{ Spec: sourcev1.GitRepositorySpec{ URL: repoURL, @@ -1452,9 +1514,7 @@ func createGitRepository(kClient client.Client, name, namespace, impl, repoURL, if secretRef != "" { gitRepo.Spec.SecretRef = &meta.LocalObjectReference{Name: secretRef} } - if impl != "" { - gitRepo.Spec.GitImplementation = impl - } + gitRepo.Spec.GitImplementation = sourcev1.LibGit2Implementation return kClient.Create(context.Background(), gitRepo) }