From 31b304ddf891481e01368e6bc50fddaf76ae4ffc Mon Sep 17 00:00:00 2001 From: Sunny Date: Thu, 23 Sep 2021 02:10:47 +0530 Subject: [PATCH 01/11] Add testenv TestMain and move ginkgo to legacy Add new testenv based TestMain in suite_test.go and move the ginkgo test setup to legacy_suite_test.go. This helps to run both the ginkgo tests and testenv based tests. Signed-off-by: Sunny --- controllers/legacy_suite_test.go | 111 +++++++++++++++++++++++++++++++ controllers/suite_test.go | 101 +++++++++------------------- 2 files changed, 143 insertions(+), 69 deletions(-) create mode 100644 controllers/legacy_suite_test.go diff --git a/controllers/legacy_suite_test.go b/controllers/legacy_suite_test.go new file mode 100644 index 00000000..42fc5161 --- /dev/null +++ b/controllers/legacy_suite_test.go @@ -0,0 +1,111 @@ +/* +Copyright 2020 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controllers + +import ( + "path/filepath" + "testing" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/rest" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/envtest" + "sigs.k8s.io/controller-runtime/pkg/envtest/printer" + "sigs.k8s.io/controller-runtime/pkg/log/zap" + + imagev1_reflect "github.com/fluxcd/image-reflector-controller/api/v1beta1" + sourcev1 "github.com/fluxcd/source-controller/api/v1beta1" + + imagev1 "github.com/fluxcd/image-automation-controller/api/v1beta1" + // +kubebuilder:scaffold:imports +) + +// These tests use Ginkgo (BDD-style Go testing framework). Refer to +// http://onsi.github.io/ginkgo/ to learn more about Ginkgo. + +var cfg *rest.Config +var k8sClient client.Client +var k8sManager ctrl.Manager +var imageAutoReconciler *ImageUpdateAutomationReconciler +var ginkgoTestEnv *envtest.Environment + +func TestAPIs(t *testing.T) { + RegisterFailHandler(Fail) + + RunSpecsWithDefaultAndCustomReporters(t, + "Controller Suite", + []Reporter{printer.NewlineReporter{}}) +} + +var _ = BeforeSuite(func(done Done) { + ctrl.SetLogger( + zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true)), + ) + + By("bootstrapping test environment") + ginkgoTestEnv = &envtest.Environment{ + CRDDirectoryPaths: []string{ + filepath.Join("..", "config", "crd", "bases"), + filepath.Join("testdata", "crds"), + }, + } + + var err error + cfg, err = ginkgoTestEnv.Start() + Expect(err).ToNot(HaveOccurred()) + Expect(cfg).ToNot(BeNil()) + + Expect(sourcev1.AddToScheme(scheme.Scheme)).To(Succeed()) + Expect(imagev1_reflect.AddToScheme(scheme.Scheme)).To(Succeed()) + + Expect(imagev1.AddToScheme(scheme.Scheme)).To(Succeed()) + // +kubebuilder:scaffold:scheme + + k8sManager, err = ctrl.NewManager(cfg, ctrl.Options{ + Scheme: scheme.Scheme, + }) + Expect(err).ToNot(HaveOccurred()) + + imageAutoReconciler = &ImageUpdateAutomationReconciler{ + Client: k8sManager.GetClient(), + Scheme: scheme.Scheme, + } + Expect(imageAutoReconciler.SetupWithManager(k8sManager, ImageUpdateAutomationReconcilerOptions{})).To(Succeed()) + + go func() { + defer GinkgoRecover() + err = k8sManager.Start(ctrl.SetupSignalHandler()) + Expect(err).ToNot(HaveOccurred()) + }() + + // Specifically an uncached client. Use .Get if you + // want to see what the reconcilers see. + k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}) + Expect(err).ToNot(HaveOccurred()) + Expect(k8sClient).ToNot(BeNil()) + + close(done) +}, 60) + +var _ = AfterSuite(func() { + By("tearing down the test environment") + err := ginkgoTestEnv.Stop() + Expect(err).ToNot(HaveOccurred()) +}) diff --git a/controllers/suite_test.go b/controllers/suite_test.go index c323772c..0413c2b8 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -17,95 +17,58 @@ limitations under the License. package controllers import ( + "fmt" + "os" "path/filepath" "testing" - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" + "github.com/fluxcd/pkg/runtime/testenv" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/kubernetes/scheme" - "k8s.io/client-go/rest" ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/envtest" - "sigs.k8s.io/controller-runtime/pkg/envtest/printer" - "sigs.k8s.io/controller-runtime/pkg/log/zap" imagev1_reflect "github.com/fluxcd/image-reflector-controller/api/v1beta1" sourcev1 "github.com/fluxcd/source-controller/api/v1beta1" imagev1 "github.com/fluxcd/image-automation-controller/api/v1beta1" - // +kubebuilder:scaffold:imports ) -// These tests use Ginkgo (BDD-style Go testing framework). Refer to -// http://onsi.github.io/ginkgo/ to learn more about Ginkgo. - -var cfg *rest.Config -var k8sClient client.Client -var k8sManager ctrl.Manager -var imageAutoReconciler *ImageUpdateAutomationReconciler -var testEnv *envtest.Environment - -func TestAPIs(t *testing.T) { - RegisterFailHandler(Fail) - - RunSpecsWithDefaultAndCustomReporters(t, - "Controller Suite", - []Reporter{printer.NewlineReporter{}}) -} - -var _ = BeforeSuite(func(done Done) { - ctrl.SetLogger( - zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true)), - ) - - By("bootstrapping test environment") - testEnv = &envtest.Environment{ - CRDDirectoryPaths: []string{ - filepath.Join("..", "config", "crd", "bases"), - filepath.Join("testdata", "crds"), - }, - } - - var err error - cfg, err = testEnv.Start() - Expect(err).ToNot(HaveOccurred()) - Expect(cfg).ToNot(BeNil()) - - Expect(sourcev1.AddToScheme(scheme.Scheme)).To(Succeed()) - Expect(imagev1_reflect.AddToScheme(scheme.Scheme)).To(Succeed()) +var ( + testEnv *testenv.Environment + ctx = ctrl.SetupSignalHandler() +) - Expect(imagev1.AddToScheme(scheme.Scheme)).To(Succeed()) - // +kubebuilder:scaffold:scheme +func TestMain(m *testing.M) { + utilruntime.Must(imagev1_reflect.AddToScheme(scheme.Scheme)) + utilruntime.Must(sourcev1.AddToScheme(scheme.Scheme)) + utilruntime.Must(imagev1.AddToScheme(scheme.Scheme)) - k8sManager, err = ctrl.NewManager(cfg, ctrl.Options{ - Scheme: scheme.Scheme, - }) - Expect(err).ToNot(HaveOccurred()) + testEnv = testenv.New(testenv.WithCRDPath( + filepath.Join("..", "config", "crd", "bases"), + filepath.Join("testdata", "crds"), + )) - imageAutoReconciler = &ImageUpdateAutomationReconciler{ - Client: k8sManager.GetClient(), + if err := (&ImageUpdateAutomationReconciler{ + Client: testEnv, Scheme: scheme.Scheme, + }).SetupWithManager(testEnv, ImageUpdateAutomationReconcilerOptions{}); err != nil { + panic(fmt.Sprintf("Failed to start ImageUpdateAutomationReconciler: %v", err)) } - Expect(imageAutoReconciler.SetupWithManager(k8sManager, ImageUpdateAutomationReconcilerOptions{})).To(Succeed()) go func() { - defer GinkgoRecover() - err = k8sManager.Start(ctrl.SetupSignalHandler()) - Expect(err).ToNot(HaveOccurred()) + fmt.Println("Starting the test environment") + if err := testEnv.Start(ctx); err != nil { + panic(fmt.Sprintf("Failed to start the test environment manager: %v", err)) + } }() + <-testEnv.Manager.Elected() - // Specifically an uncached client. Use .Get if you - // want to see what the reconcilers see. - k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}) - Expect(err).ToNot(HaveOccurred()) - Expect(k8sClient).ToNot(BeNil()) + code := m.Run() - close(done) -}, 60) + fmt.Println("Stopping the test environment") + if err := testEnv.Stop(); err != nil { + panic(fmt.Sprintf("Failed to stop the test environment: %v", err)) + } -var _ = AfterSuite(func() { - By("tearing down the test environment") - err := testEnv.Stop() - Expect(err).ToNot(HaveOccurred()) -}) + os.Exit(code) +} From 3b131bacc50c22f0cd91ca4f5f89594d8ecb0bd8 Mon Sep 17 00:00:00 2001 From: Sunny Date: Sat, 25 Sep 2021 03:22:34 +0530 Subject: [PATCH 02/11] controllers/update_test.go: Update to use testenv Signed-off-by: Sunny --- controllers/suite_test.go | 5 + .../appconfig-setters-expected/deploy.yaml | 2 +- controllers/update_test.go | 1249 +++++++---------- 3 files changed, 542 insertions(+), 714 deletions(-) diff --git a/controllers/suite_test.go b/controllers/suite_test.go index 0413c2b8..09aab236 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -21,6 +21,7 @@ import ( "os" "path/filepath" "testing" + "time" "github.com/fluxcd/pkg/runtime/testenv" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -33,6 +34,10 @@ import ( imagev1 "github.com/fluxcd/image-automation-controller/api/v1beta1" ) +const ( + contextTimeout = time.Second * 10 +) + var ( testEnv *testenv.Environment ctx = ctrl.SetupSignalHandler() diff --git a/controllers/testdata/appconfig-setters-expected/deploy.yaml b/controllers/testdata/appconfig-setters-expected/deploy.yaml index e8394aa4..8edf19fa 100644 --- a/controllers/testdata/appconfig-setters-expected/deploy.yaml +++ b/controllers/testdata/appconfig-setters-expected/deploy.yaml @@ -7,4 +7,4 @@ spec: spec: containers: - name: hello - image: helloworld:1.0.1 # SETTER_SITE + image: helloworld:v1.3.1 # SETTER_SITE diff --git a/controllers/update_test.go b/controllers/update_test.go index d4c9f107..18ee099d 100644 --- a/controllers/update_test.go +++ b/controllers/update_test.go @@ -27,6 +27,7 @@ import ( "path" "path/filepath" "strings" + "testing" "time" "github.com/ProtonMail/go-crypto/openpgp" @@ -38,13 +39,13 @@ import ( "github.com/go-git/go-git/v5/plumbing/object" "github.com/go-git/go-git/v5/storage/memory" "github.com/go-logr/logr" - . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "github.com/otiai10/copy" corev1 "k8s.io/api/core/v1" apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/kubernetes/scheme" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -73,58 +74,13 @@ func randStringRunes(n int) string { return string(b) } -var _ = Describe("ImageUpdateAutomation", func() { - var ( - branch string - repositoryPath string - namespace *corev1.Namespace - username, password string - gitServer *gittestserver.GitServer - ) - - // Start the git server - BeforeEach(func() { - branch = randStringRunes(8) - repositoryPath = "/config-" + randStringRunes(5) + ".git" - - namespace = &corev1.Namespace{} - namespace.Name = "image-auto-test-" + randStringRunes(5) - Expect(k8sClient.Create(context.Background(), namespace)).To(Succeed()) - - var err error - gitServer, err = gittestserver.NewTempGitServer() - Expect(err).NotTo(HaveOccurred()) - username = randStringRunes(5) - password = randStringRunes(5) - // using authentication makes using the server more fiddly in - // general, but is required for testing SSH. - gitServer.Auth(username, password) - gitServer.AutoCreate() - Expect(gitServer.StartHTTP()).To(Succeed()) - gitServer.KeyDir(filepath.Join(gitServer.Root(), "keys")) - Expect(gitServer.ListenSSH()).To(Succeed()) - }) - - AfterEach(func() { - gitServer.StopHTTP() - os.RemoveAll(gitServer.Root()) - }) - - It("Initialises git OK", func() { - Expect(initGitRepo(gitServer, "testdata/appconfig", branch, repositoryPath)).To(Succeed()) - }) - - Context("commit spec", func() { - - var ( - localRepo *git.Repository - commitMessage string - ) +func TestImageUpdateAutomation_commit_spec(t *testing.T) { + g := NewWithT(t) - const ( - authorName = "Flux B Ot" - authorEmail = "fluxbot@example.com" - commitTemplate = `Commit summary + const ( + authorName = "Flux B Ot" + authorEmail = "fluxbot@example.com" + commitTemplate = `Commit summary Automation: {{ .AutomationObject }} @@ -143,7 +99,7 @@ Images: - {{.}} ({{.Policy.Name}}) {{ end -}} ` - commitMessageFmt = `Commit summary + commitMessageFmt = `Commit summary Automation: %s/update-test @@ -154,34 +110,104 @@ Objects: Images: - helloworld:v1.0.0 (%s) ` - ) + ) + + tests := []struct { + name string + testdataPath string + updateStrategy *imagev1.UpdateStrategy + sign bool + }{ + { + name: "with non-path update setter", + testdataPath: "testdata/appconfig", + updateStrategy: &imagev1.UpdateStrategy{ + Strategy: imagev1.UpdateStrategySetters, + }, + }, + { + name: "with path update setter", + testdataPath: "testdata/pathconfig", + updateStrategy: &imagev1.UpdateStrategy{ + Strategy: imagev1.UpdateStrategySetters, + Path: "./yes", + }, + }, + { + name: "with signed commit", + testdataPath: "testdata/appconfig", + updateStrategy: &imagev1.UpdateStrategy{ + Strategy: imagev1.UpdateStrategySetters, + }, + sign: true, + }, + } + + // Create test git server. + gitServer, err := gittestserver.NewTempGitServer() + g.Expect(err).NotTo(HaveOccurred()) + defer os.RemoveAll(gitServer.Root()) + + username := randStringRunes(5) + password := randStringRunes(5) + // using authentication makes using the server more fiddly in + // general, but is required for testing SSH. + gitServer.Auth(username, password) + gitServer.AutoCreate() + + g.Expect(gitServer.StartHTTP()).To(Succeed()) + defer gitServer.StopHTTP() + + gitServer.KeyDir(filepath.Join(gitServer.Root(), "keys")) + g.Expect(gitServer.ListenSSH()).To(Succeed()) + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + namespace := &corev1.Namespace{} + namespace.Name = "image-auto-test-" + randStringRunes(5) + + ctx, cancel := context.WithTimeout(context.Background(), contextTimeout) + defer cancel() + + // Create a test namespace. + g.Expect(testEnv.Create(ctx, namespace)).To(Succeed()) + defer func() { + g.Expect(testEnv.Delete(ctx, namespace)).To(Succeed()) + }() - BeforeEach(func() { - Expect(initGitRepo(gitServer, "testdata/appconfig", branch, repositoryPath)).To(Succeed()) + branch := randStringRunes(8) + repositoryPath := "/config-" + randStringRunes(6) + ".git" + + // Initialize a git repo. + g.Expect(initGitRepo(gitServer, tt.testdataPath, branch, repositoryPath)).To(Succeed()) + + // Clone the repo. repoURL := gitServer.HTTPAddressWithCredentials() + repositoryPath - var err error - localRepo, err = git.Clone(memory.NewStorage(), memfs.New(), &git.CloneOptions{ + localRepo, err := git.Clone(memory.NewStorage(), memfs.New(), &git.CloneOptions{ URL: repoURL, RemoteName: "origin", ReferenceName: plumbing.NewBranchReferenceName(branch), }) - Expect(err).ToNot(HaveOccurred()) + g.Expect(err).ToNot(HaveOccurred()) + // Create GitRepository resource for the above repo. gitRepoKey := types.NamespacedName{ Name: "image-auto-" + randStringRunes(5), Namespace: namespace.Name, } gitRepo := &sourcev1.GitRepository{ - ObjectMeta: metav1.ObjectMeta{ - Name: gitRepoKey.Name, - Namespace: namespace.Name, - }, Spec: sourcev1.GitRepositorySpec{ URL: repoURL, Interval: metav1.Duration{Duration: time.Minute}, }, } - Expect(k8sClient.Create(context.Background(), gitRepo)).To(Succeed()) + gitRepo.Name = gitRepoKey.Name + gitRepo.Namespace = namespace.Name + g.Expect(testEnv.Create(ctx, gitRepo)).To(Succeed()) + + // Create an image policy. policyKey := types.NamespacedName{ Name: "policy-" + randStringRunes(5), Namespace: namespace.Name, @@ -189,10 +215,6 @@ Images: // NB not testing the image reflector controller; this // will make a "fully formed" ImagePolicy object. policy := &imagev1_reflect.ImagePolicy{ - ObjectMeta: metav1.ObjectMeta{ - Name: policyKey.Name, - Namespace: policyKey.Namespace, - }, Spec: imagev1_reflect.ImagePolicySpec{ ImageRepositoryRef: meta.NamespacedObjectReference{ Name: "not-expected-to-exist", @@ -204,35 +226,71 @@ Images: }, }, } - Expect(k8sClient.Create(context.Background(), policy)).To(Succeed()) - policy.Status.LatestImage = "helloworld:v1.0.0" - Expect(k8sClient.Status().Update(context.Background(), policy)).To(Succeed()) + policy.Name = policyKey.Name + policy.Namespace = policyKey.Namespace + g.Expect(testEnv.Create(ctx, policy)).To(Succeed()) + g.Expect(testEnv.Status().Update(ctx, policy)).To(Succeed()) // Format the expected message given the generated values - commitMessage = fmt.Sprintf(commitMessageFmt, namespace.Name, policyKey.Name) + commitMessage := fmt.Sprintf(commitMessageFmt, namespace.Name, policyKey.Name) // Insert a setter reference into the deployment file, // before creating the automation object itself. - commitInRepo(repoURL, branch, "Install setter marker", func(tmp string) { - Expect(replaceMarker(tmp, policyKey)).To(Succeed()) - }) + if tt.updateStrategy.Path == "" { + commitInRepo(g, repoURL, branch, "Install setter marker", func(tmp string) { + g.Expect(replaceMarker(tmp, policyKey)).To(Succeed()) + }) + } else { + commitInRepo(g, repoURL, branch, "Install setter marker", func(tmp string) { + g.Expect(replaceMarker(path.Join(tmp, "yes"), policyKey)).To(Succeed()) + }) + commitInRepo(g, repoURL, branch, "Install setter marker", func(tmp string) { + g.Expect(replaceMarker(path.Join(tmp, "no"), policyKey)).To(Succeed()) + }) + } - // pull the head commit we just pushed, so it's not + // Pull the head commit we just pushed, so it's not // considered a new commit when checking for a commit // made by automation. - waitForNewHead(localRepo, branch) + waitForNewHead(g, localRepo, branch) - // now create the automation object, and let it (one + var pgpEntity *openpgp.Entity + var sec *corev1.Secret + if tt.sign { + var err error + // generate keypair for signing + pgpEntity, err = openpgp.NewEntity("", "", "", nil) + g.Expect(err).ToNot(HaveOccurred()) + + // configure OpenPGP armor encoder + b := bytes.NewBuffer(nil) + w, err := armor.Encode(b, openpgp.PrivateKeyType, nil) + g.Expect(err).ToNot(HaveOccurred()) + + // serialize private key + err = pgpEntity.SerializePrivate(w, nil) + g.Expect(err).ToNot(HaveOccurred()) + err = w.Close() + g.Expect(err).ToNot(HaveOccurred()) + + // create the secret containing signing key + sec = &corev1.Secret{ + Data: map[string][]byte{ + "git.asc": b.Bytes(), + }, + } + sec.Name = "signing-key-secret-" + randStringRunes(5) + sec.Namespace = namespace.Name + g.Expect(testEnv.Create(ctx, sec)).To(Succeed()) + } + + // Now create the automation object, and let it (one // hopes!) make a commit itself. updateKey := types.NamespacedName{ Namespace: namespace.Name, Name: "update-test", } updateBySetters := &imagev1.ImageUpdateAutomation{ - ObjectMeta: metav1.ObjectMeta{ - Name: updateKey.Name, - Namespace: updateKey.Namespace, - }, Spec: imagev1.ImageUpdateAutomationSpec{ Interval: metav1.Duration{Duration: 2 * time.Hour}, // this is to ensure any subsequent run should be outside the scope of the testing SourceRef: imagev1.SourceReference{ @@ -253,73 +311,203 @@ Images: }, }, }, - Update: &imagev1.UpdateStrategy{ - Strategy: imagev1.UpdateStrategySetters, - }, + Update: tt.updateStrategy, }, } - Expect(k8sClient.Create(context.Background(), updateBySetters)).To(Succeed()) - // wait for a new commit to be made by the controller - waitForNewHead(localRepo, branch) - }) + updateBySetters.Name = updateKey.Name + updateBySetters.Namespace = updateKey.Namespace - AfterEach(func() { - Expect(k8sClient.Delete(context.Background(), namespace)).To(Succeed()) - }) + // Add commit signing info. + if tt.sign { + updateBySetters.Spec.GitSpec.Commit.SigningKey = &imagev1.SigningKey{ + SecretRef: meta.LocalObjectReference{Name: sec.Name}, + } + } - It("formats the commit message as in the template", func() { - head, _ := localRepo.Head() - commit, err := localRepo.CommitObject(head.Hash()) - Expect(err).ToNot(HaveOccurred()) - Expect(commit.Message).To(Equal(commitMessage)) - }) + // Create the image update automation object. + g.Expect(testEnv.Create(ctx, updateBySetters)).To(Succeed()) + // Wait for a new commit to be made by the controller. + waitForNewHead(g, localRepo, branch) - It("has the commit author as given", func() { head, _ := localRepo.Head() commit, err := localRepo.CommitObject(head.Hash()) - Expect(err).ToNot(HaveOccurred()) - Expect(commit.Author).NotTo(BeNil()) - Expect(commit.Author.Name).To(Equal(authorName)) - Expect(commit.Author.Email).To(Equal(authorEmail)) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(commit.Author).NotTo(BeNil()) + g.Expect(commit.Author.Name).To(Equal(authorName)) + g.Expect(commit.Author.Email).To(Equal(authorEmail)) + + if tt.updateStrategy.Path == "" { + g.Expect(commit.Message).To(Equal(commitMessage)) + } else { + g.Expect(commit.Message).To(Not(ContainSubstring("update-no"))) + g.Expect(commit.Message).To(ContainSubstring("update-yes")) + } + + if tt.sign { + // configure OpenPGP armor encoder + b := bytes.NewBuffer(nil) + w, err := armor.Encode(b, openpgp.PublicKeyType, nil) + g.Expect(err).ToNot(HaveOccurred()) + + // serialize public key + err = pgpEntity.Serialize(w) + g.Expect(err).ToNot(HaveOccurred()) + err = w.Close() + g.Expect(err).ToNot(HaveOccurred()) + + // verify commit + ent, err := commit.Verify(b.String()) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(ent.PrimaryKey.Fingerprint).To(Equal(pgpEntity.PrimaryKey.Fingerprint)) + } }) - }) + } +} - Context("update path", func() { +func TestImageUpdateAutomation_e2e(t *testing.T) { + tests := []struct { + name string + proto string + impl string + }{ + { + name: "go-git with HTTP", + proto: "http", + impl: sourcev1.GoGitImplementation, + }, + { + name: "go-git with SSH", + proto: "ssh", + impl: sourcev1.GoGitImplementation, + }, + { + name: "libgit2 with HTTP", + proto: "http", + impl: sourcev1.LibGit2Implementation, + }, + { + name: "libgit2 with SSH", + proto: "ssh", + impl: sourcev1.LibGit2Implementation, + }, + } - var localRepo *git.Repository - const commitTemplate = `Commit summary + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) -{{ range $resource, $_ := .Updated.Objects -}} -- {{ $resource.Name }} -{{ end -}} -` + const latestImage = "helloworld:1.0.1" - BeforeEach(func() { - Expect(initGitRepo(gitServer, "testdata/pathconfig", branch, repositoryPath)).To(Succeed()) - repoURL := gitServer.HTTPAddressWithCredentials() + repositoryPath - var err error - localRepo, err = git.Clone(memory.NewStorage(), memfs.New(), &git.CloneOptions{ - URL: repoURL, + ctx, cancel := context.WithTimeout(context.Background(), contextTimeout) + defer cancel() + + // Create a test namespace. + namespace := &corev1.Namespace{} + namespace.Name = "image-auto-test-" + randStringRunes(5) + g.Expect(testEnv.Create(ctx, namespace)).To(Succeed()) + defer func() { + g.Expect(testEnv.Delete(ctx, namespace)).To(Succeed()) + }() + + branch := randStringRunes(8) + repositoryPath := "/config-" + randStringRunes(6) + ".git" + + // Create git server. + gitServer, err := gittestserver.NewTempGitServer() + g.Expect(err).NotTo(HaveOccurred()) + defer os.RemoveAll(gitServer.Root()) + + username := randStringRunes(5) + password := randStringRunes(5) + // Using authentication makes using the server more fiddly in + // general, but is required for testing SSH. + gitServer.Auth(username, password) + gitServer.AutoCreate() + + g.Expect(gitServer.StartHTTP()).To(Succeed()) + defer gitServer.StopHTTP() + + gitServer.KeyDir(filepath.Join(gitServer.Root(), "keys")) + g.Expect(gitServer.ListenSSH()).To(Succeed()) + + var repoURL string + cloneLocalRepoURL := gitServer.HTTPAddressWithCredentials() + repositoryPath + + if tt.proto == "http" { + repoURL = cloneLocalRepoURL // NB not testing auth for git over HTTP + } else if tt.proto == "ssh" { + sshURL := gitServer.SSHAddress() + // this is expected to use 127.0.0.1, but host key + // checking usually wants a hostname, so use + // "localhost". + sshURL = strings.Replace(sshURL, "127.0.0.1", "localhost", 1) + repoURL = sshURL + repositoryPath + + // NOTE: Check how this is done in source-controller. + go func() { + gitServer.StartSSH() + }() + defer func() { + g.Expect(gitServer.StopSSH()).To(Succeed()) + }() + } else { + t.Fatal("proto not set to http or ssh") + } + + commitMessage := "Commit a difference " + randStringRunes(5) + + // Initialize a git repo. + g.Expect(initGitRepo(gitServer, "testdata/appconfig", branch, repositoryPath)).To(Succeed()) + + localRepo, err := git.Clone(memory.NewStorage(), memfs.New(), &git.CloneOptions{ + URL: cloneLocalRepoURL, RemoteName: "origin", ReferenceName: plumbing.NewBranchReferenceName(branch), }) - Expect(err).ToNot(HaveOccurred()) + g.Expect(err).ToNot(HaveOccurred()) + // Create git repo resource for the above repo. gitRepoKey := types.NamespacedName{ Name: "image-auto-" + randStringRunes(5), Namespace: namespace.Name, } gitRepo := &sourcev1.GitRepository{ - ObjectMeta: metav1.ObjectMeta{ - Name: gitRepoKey.Name, - Namespace: namespace.Name, - }, Spec: sourcev1.GitRepositorySpec{ - URL: repoURL, - Interval: metav1.Duration{Duration: time.Minute}, + URL: repoURL, + Interval: metav1.Duration{Duration: time.Minute}, + GitImplementation: tt.impl, }, } - Expect(k8sClient.Create(context.Background(), gitRepo)).To(Succeed()) + gitRepo.Name = gitRepoKey.Name + gitRepo.Namespace = namespace.Name + + // If using SSH, we need to provide an identity (private + // key) and known_hosts file in a secret. + if tt.proto == "ssh" { + url, err := url.Parse(repoURL) + g.Expect(err).ToNot(HaveOccurred()) + knownhosts, err := ssh.ScanHostKey(url.Host, 5*time.Second) + g.Expect(err).ToNot(HaveOccurred()) + keygen := ssh.NewRSAGenerator(2048) + pair, err := keygen.Generate() + g.Expect(err).ToNot(HaveOccurred()) + + sec := &corev1.Secret{ + StringData: map[string]string{ + "known_hosts": string(knownhosts), + "identity": string(pair.PrivateKey), + "identity.pub": string(pair.PublicKey), + }, + } + sec.Name = "git-secret-" + randStringRunes(5) + sec.Namespace = namespace.Name + g.Expect(testEnv.Create(ctx, sec)).To(Succeed()) + gitRepo.Spec.SecretRef = &meta.LocalObjectReference{Name: sec.Name} + } + + g.Expect(testEnv.Create(ctx, gitRepo)).To(Succeed()) + + // Create an image policy. policyKey := types.NamespacedName{ Name: "policy-" + randStringRunes(5), Namespace: namespace.Name, @@ -327,10 +515,6 @@ Images: // NB not testing the image reflector controller; this // will make a "fully formed" ImagePolicy object. policy := &imagev1_reflect.ImagePolicy{ - ObjectMeta: metav1.ObjectMeta{ - Name: policyKey.Name, - Namespace: policyKey.Namespace, - }, Spec: imagev1_reflect.ImagePolicySpec{ ImageRepositoryRef: meta.NamespacedObjectReference{ Name: "not-expected-to-exist", @@ -341,42 +525,40 @@ Images: }, }, }, + Status: imagev1_reflect.ImagePolicyStatus{ + LatestImage: latestImage, + }, } - Expect(k8sClient.Create(context.Background(), policy)).To(Succeed()) - policy.Status.LatestImage = "helloworld:v1.0.0" - Expect(k8sClient.Status().Update(context.Background(), policy)).To(Succeed()) - - // Insert a setter reference into the deployment file, - // before creating the automation object itself. - commitInRepo(repoURL, branch, "Install setter marker", func(tmp string) { - Expect(replaceMarker(path.Join(tmp, "yes"), policyKey)).To(Succeed()) - }) - commitInRepo(repoURL, branch, "Install setter marker", func(tmp string) { - Expect(replaceMarker(path.Join(tmp, "no"), policyKey)).To(Succeed()) + policy.Name = policyKey.Name + policy.Namespace = policyKey.Namespace + g.Expect(testEnv.Create(ctx, policy)).To(Succeed()) + g.Expect(testEnv.Status().Update(ctx, policy)).To(Succeed()) + defer func() { + g.Expect(testEnv.Delete(ctx, policy)).To(Succeed()) + }() + + // --- update with PushSpec + + commitInRepo(g, cloneLocalRepoURL, branch, "Install setter marker", func(tmp string) { + g.Expect(replaceMarker(tmp, policyKey)).To(Succeed()) }) - // pull the head commit we just pushed, so it's not + // Pull the head commit we just pushed, so it's not // considered a new commit when checking for a commit // made by automation. - waitForNewHead(localRepo, branch) + waitForNewHead(g, localRepo, branch) - // now create the automation object, and let it (one + pushBranch := "pr-" + randStringRunes(5) + + // Now create the automation object, and let it (one // hopes!) make a commit itself. updateKey := types.NamespacedName{ Namespace: namespace.Name, - Name: "update-test", + Name: "update-" + randStringRunes(5), } updateBySetters := &imagev1.ImageUpdateAutomation{ - ObjectMeta: metav1.ObjectMeta{ - Name: updateKey.Name, - Namespace: updateKey.Namespace, - }, Spec: imagev1.ImageUpdateAutomationSpec{ Interval: metav1.Duration{Duration: 2 * time.Hour}, // this is to ensure any subsequent run should be outside the scope of the testing - Update: &imagev1.UpdateStrategy{ - Strategy: imagev1.UpdateStrategySetters, - Path: "./yes", - }, SourceRef: imagev1.SourceReference{ Kind: "GitRepository", Name: gitRepoKey.Name, @@ -388,144 +570,101 @@ Images: }, }, Commit: imagev1.CommitSpec{ + MessageTemplate: commitMessage, Author: imagev1.CommitUser{ Email: "fluxbot@example.com", }, - MessageTemplate: commitTemplate, }, + Push: &imagev1.PushSpec{ + Branch: pushBranch, + }, + }, + Update: &imagev1.UpdateStrategy{ + Strategy: imagev1.UpdateStrategySetters, }, }, } - Expect(k8sClient.Create(context.Background(), updateBySetters)).To(Succeed()) - // wait for a new commit to be made by the controller - waitForNewHead(localRepo, branch) - }) + updateBySetters.Name = updateKey.Name + updateBySetters.Namespace = updateKey.Namespace + g.Expect(testEnv.Create(ctx, updateBySetters)).To(Succeed()) - AfterEach(func() { - Expect(k8sClient.Delete(context.Background(), namespace)).To(Succeed()) - }) + // -- Creates and pushes the push branch. - It("updates only the deployment in the specified path", func() { - head, _ := localRepo.Head() + // Wait for a new commit to be made by the controller. + waitForNewHead(g, localRepo, pushBranch) + head, err := localRepo.Reference(plumbing.NewRemoteReferenceName(originRemote, pushBranch), true) + g.Expect(err).NotTo(HaveOccurred()) commit, err := localRepo.CommitObject(head.Hash()) - Expect(err).ToNot(HaveOccurred()) - Expect(commit.Message).To(Not(ContainSubstring("update-no"))) - Expect(commit.Message).To(ContainSubstring("update-yes")) - }) - }) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(commit.Message).To(Equal(commitMessage)) - Context("commit signing", func() { + // -- Pushes another commit to the existing push branch. - var ( - localRepo *git.Repository - pgpEntity *openpgp.Entity - ) + head, err = localRepo.Reference(plumbing.NewRemoteReferenceName(originRemote, pushBranch), true) + headHash := head.String() + g.Expect(err).NotTo(HaveOccurred()) - BeforeEach(func() { - Expect(initGitRepo(gitServer, "testdata/appconfig", branch, repositoryPath)).To(Succeed()) - repoURL := gitServer.HTTPAddressWithCredentials() + repositoryPath - var err error - localRepo, err = git.Clone(memory.NewStorage(), memfs.New(), &git.CloneOptions{ - URL: repoURL, - RemoteName: "origin", - ReferenceName: plumbing.NewBranchReferenceName(branch), - }) - Expect(err).ToNot(HaveOccurred()) + // Update the policy and expect another commit in the push branch. + policy.Status.LatestImage = "helloworld:v1.3.0" + g.Expect(testEnv.Status().Update(ctx, policy)).To(Succeed()) + waitForNewHead(g, localRepo, pushBranch) + head, err = localRepo.Reference(plumbing.NewRemoteReferenceName(originRemote, pushBranch), true) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(head.String()).NotTo(Equal(headHash)) - gitRepoKey := types.NamespacedName{ - Name: "image-auto-" + randStringRunes(5), - Namespace: namespace.Name, - } - gitRepo := &sourcev1.GitRepository{ - ObjectMeta: metav1.ObjectMeta{ - Name: gitRepoKey.Name, - Namespace: namespace.Name, - }, - Spec: sourcev1.GitRepositorySpec{ - URL: repoURL, - Interval: metav1.Duration{Duration: time.Minute}, - }, - } - Expect(k8sClient.Create(context.Background(), gitRepo)).To(Succeed()) - policyKey := types.NamespacedName{ - Name: "policy-" + randStringRunes(5), - Namespace: namespace.Name, - } - // NB not testing the image reflector controller; this - // will make a "fully formed" ImagePolicy object. - policy := &imagev1_reflect.ImagePolicy{ - ObjectMeta: metav1.ObjectMeta{ - Name: policyKey.Name, - Namespace: policyKey.Namespace, - }, - Spec: imagev1_reflect.ImagePolicySpec{ - ImageRepositoryRef: meta.NamespacedObjectReference{ - Name: "not-expected-to-exist", - }, - Policy: imagev1_reflect.ImagePolicyChoice{ - SemVer: &imagev1_reflect.SemVerPolicy{ - Range: "1.x", - }, - }, - }, - } - Expect(k8sClient.Create(context.Background(), policy)).To(Succeed()) - policy.Status.LatestImage = "helloworld:v1.0.0" - Expect(k8sClient.Status().Update(context.Background(), policy)).To(Succeed()) + // -- Still pushes to the push branch after it's merged. + + head, err = localRepo.Reference(plumbing.NewRemoteReferenceName(originRemote, pushBranch), true) + headHash = head.String() + g.Expect(err).NotTo(HaveOccurred()) + + // Merge the push branch into checkout branch, and push the merge commit + // upstream. + // waitForNewHead() leaves the repo at the head of the branch given, i.e., the + // push branch), so we have to check out the "main" branch first. + g.Expect(checkoutBranch(localRepo, branch)).To(Succeed()) + mergeBranchIntoHead(g, localRepo, pushBranch) + + // Update the policy and expect another commit in the push branch + policy.Status.LatestImage = "helloworld:v1.3.1" + g.Expect(testEnv.Status().Update(ctx, policy)).To(Succeed()) + waitForNewHead(g, localRepo, pushBranch) + head, err = localRepo.Reference(plumbing.NewRemoteReferenceName(originRemote, pushBranch), true) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(head.String()).NotTo(Equal(headHash)) + + // Cleanup the image update automation used above. + g.Expect(testEnv.Delete(ctx, updateBySetters)).To(Succeed()) + + // --- with update strategy setters // Insert a setter reference into the deployment file, // before creating the automation object itself. - commitInRepo(repoURL, branch, "Install setter marker", func(tmp string) { - Expect(replaceMarker(tmp, policyKey)).To(Succeed()) + commitInRepo(g, cloneLocalRepoURL, branch, "Install setter marker", func(tmp string) { + g.Expect(replaceMarker(tmp, policyKey)).To(Succeed()) }) - // pull the head commit we just pushed, so it's not + // Pull the head commit we just pushed, so it's not // considered a new commit when checking for a commit // made by automation. - waitForNewHead(localRepo, branch) - - // generate keypair for signing - pgpEntity, err = openpgp.NewEntity("", "", "", nil) - Expect(err).ToNot(HaveOccurred()) - - // configure OpenPGP armor encoder - b := bytes.NewBuffer(nil) - w, err := armor.Encode(b, openpgp.PrivateKeyType, nil) - Expect(err).ToNot(HaveOccurred()) - - // serialize private key - err = pgpEntity.SerializePrivate(w, nil) - Expect(err).ToNot(HaveOccurred()) - err = w.Close() - Expect(err).ToNot(HaveOccurred()) - - // create the secret containing signing key - sec := &corev1.Secret{ - Data: map[string][]byte{ - "git.asc": b.Bytes(), - }, - } - sec.Name = "signing-key-secret-" + randStringRunes(5) - sec.Namespace = namespace.Name - Expect(k8sClient.Create(context.Background(), sec)).To(Succeed()) + waitForNewHead(g, localRepo, branch) - // now create the automation object, and let it (one + // Now create the automation object, and let it (one // hopes!) make a commit itself. - updateKey := types.NamespacedName{ - Namespace: namespace.Name, - Name: "update-test", + updateKey = types.NamespacedName{ + Namespace: gitRepoKey.Namespace, + Name: "update-" + randStringRunes(5), } - updateBySetters := &imagev1.ImageUpdateAutomation{ - ObjectMeta: metav1.ObjectMeta{ - Name: updateKey.Name, - Namespace: updateKey.Namespace, - }, + updateBySetters = &imagev1.ImageUpdateAutomation{ Spec: imagev1.ImageUpdateAutomationSpec{ + Interval: metav1.Duration{Duration: 2 * time.Hour}, // this is to ensure any subsequent run should be outside the scope of the testing SourceRef: imagev1.SourceReference{ Kind: "GitRepository", Name: gitRepoKey.Name, }, - Interval: metav1.Duration{Duration: 2 * time.Hour}, // this is to ensure any subsequent run should be outside the scope of the testing + Update: &imagev1.UpdateStrategy{ + Strategy: imagev1.UpdateStrategySetters, + }, GitSpec: &imagev1.GitSpec{ Checkout: &imagev1.GitCheckoutSpec{ Reference: sourcev1.GitRepositoryRef{ @@ -533,465 +672,149 @@ Images: }, }, Commit: imagev1.CommitSpec{ - SigningKey: &imagev1.SigningKey{ - SecretRef: meta.LocalObjectReference{Name: sec.Name}, + Author: imagev1.CommitUser{ + Email: "fluxbot@example.com", }, + MessageTemplate: commitMessage, }, }, - Update: &imagev1.UpdateStrategy{ - Strategy: imagev1.UpdateStrategySetters, - }, }, } - - Expect(k8sClient.Create(context.Background(), updateBySetters)).To(Succeed()) + updateBySetters.Name = updateKey.Name + updateBySetters.Namespace = updateKey.Namespace + g.Expect(testEnv.Create(context.Background(), updateBySetters)).To(Succeed()) // wait for a new commit to be made by the controller - waitForNewHead(localRepo, branch) - }) - - AfterEach(func() { - Expect(k8sClient.Delete(context.Background(), namespace)).To(Succeed()) - }) - - It("signs the commit with the generated GPG key", func() { - head, _ := localRepo.Head() - commit, err := localRepo.CommitObject(head.Hash()) - Expect(err).ToNot(HaveOccurred()) - - // configure OpenPGP armor encoder - b := bytes.NewBuffer(nil) - w, err := armor.Encode(b, openpgp.PublicKeyType, nil) - Expect(err).ToNot(HaveOccurred()) - - // serialize public key - err = pgpEntity.Serialize(w) - Expect(err).ToNot(HaveOccurred()) - err = w.Close() - Expect(err).ToNot(HaveOccurred()) - - // verify commit - ent, err := commit.Verify(b.String()) - Expect(err).ToNot(HaveOccurred()) - Expect(ent.PrimaryKey.Fingerprint).To(Equal(pgpEntity.PrimaryKey.Fingerprint)) - }) - }) + waitForNewHead(g, localRepo, branch) - endToEnd := func(impl, proto string) func() { - return func() { - var ( - // for cloning locally - cloneLocalRepoURL string - // for the controller - repoURL string - localRepo *git.Repository - policy *imagev1_reflect.ImagePolicy - policyKey types.NamespacedName - gitRepoKey types.NamespacedName - commitMessage string - ) + // -- Update to the most recent image. - const latestImage = "helloworld:1.0.1" + head, _ = localRepo.Head() + commit, err = localRepo.CommitObject(head.Hash()) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(commit.Message).To(Equal(commitMessage)) - BeforeEach(func() { - cloneLocalRepoURL = gitServer.HTTPAddressWithCredentials() + repositoryPath - if proto == "http" { - repoURL = cloneLocalRepoURL // NB not testing auth for git over HTTP - } else if proto == "ssh" { - sshURL := gitServer.SSHAddress() - // this is expected to use 127.0.0.1, but host key - // checking usually wants a hostname, so use - // "localhost". - sshURL = strings.Replace(sshURL, "127.0.0.1", "localhost", 1) - repoURL = sshURL + repositoryPath - go func() { - defer GinkgoRecover() - gitServer.StartSSH() - }() - } else { - Fail("proto not set to http or ssh") - } + var newObj imagev1.ImageUpdateAutomation + g.Expect(testEnv.Get(ctx, updateKey, &newObj)).To(Succeed()) + g.Expect(newObj.Status.LastPushCommit).To(Equal(head.Hash().String())) + g.Expect(newObj.Status.LastPushTime).ToNot(BeNil()) - commitMessage = "Commit a difference " + randStringRunes(5) - - Expect(initGitRepo(gitServer, "testdata/appconfig", branch, repositoryPath)).To(Succeed()) - - var err error - localRepo, err = git.Clone(memory.NewStorage(), memfs.New(), &git.CloneOptions{ - URL: cloneLocalRepoURL, - RemoteName: "origin", - ReferenceName: plumbing.NewBranchReferenceName(branch), - }) - Expect(err).ToNot(HaveOccurred()) - - gitRepoKey = types.NamespacedName{ - Name: "image-auto-" + randStringRunes(5), - Namespace: namespace.Name, - } + compareRepoWithExpected(g, cloneLocalRepoURL, branch, "testdata/appconfig-setters-expected", func(tmp string) { + g.Expect(replaceMarker(tmp, policyKey)).To(Succeed()) + }) - gitRepo := &sourcev1.GitRepository{ - ObjectMeta: metav1.ObjectMeta{ - Name: gitRepoKey.Name, - Namespace: namespace.Name, - }, - Spec: sourcev1.GitRepositorySpec{ - URL: repoURL, - Interval: metav1.Duration{Duration: time.Minute}, - GitImplementation: impl, - }, - } + // -- Suspend the update object - reconciliation should not run. - // If using SSH, we need to provide an identity (private - // key) and known_hosts file in a secret. - if proto == "ssh" { - url, err := url.Parse(repoURL) - Expect(err).ToNot(HaveOccurred()) - knownhosts, err := ssh.ScanHostKey(url.Host, 5*time.Second) - Expect(err).ToNot(HaveOccurred()) - keygen := ssh.NewRSAGenerator(2048) - pair, err := keygen.Generate() - Expect(err).ToNot(HaveOccurred()) - - sec := &corev1.Secret{ - StringData: map[string]string{ - "known_hosts": string(knownhosts), - "identity": string(pair.PrivateKey), - "identity.pub": string(pair.PublicKey), - }, - } - sec.Name = "git-secret-" + randStringRunes(5) - sec.Namespace = namespace.Name - Expect(k8sClient.Create(context.Background(), sec)).To(Succeed()) - gitRepo.Spec.SecretRef = &meta.LocalObjectReference{Name: sec.Name} - } + var updatePatch imagev1.ImageUpdateAutomation + g.Expect(testEnv.Get(context.TODO(), updateKey, &updatePatch)).To(Succeed()) + updatePatch.Spec.Suspend = true + g.Expect(testEnv.Patch(context.Background(), &updatePatch, client.Merge)).To(Succeed()) - Expect(k8sClient.Create(context.Background(), gitRepo)).To(Succeed()) + // Create a new image automation reconciler and run it explicitly. + imageAutoReconciler := &ImageUpdateAutomationReconciler{ + Client: testEnv, + Scheme: scheme.Scheme, + } - policyKey = types.NamespacedName{ - Name: "policy-" + randStringRunes(5), - Namespace: namespace.Name, + // Wait for the suspension to reach the cache + var newUpdate imagev1.ImageUpdateAutomation + g.Eventually(func() bool { + if err := imageAutoReconciler.Get(context.Background(), updateKey, &newUpdate); err != nil { + return false } - // NB not testing the image reflector controller; this - // will make a "fully formed" ImagePolicy object. - policy = &imagev1_reflect.ImagePolicy{ - ObjectMeta: metav1.ObjectMeta{ - Name: policyKey.Name, - Namespace: policyKey.Namespace, - }, - Spec: imagev1_reflect.ImagePolicySpec{ - ImageRepositoryRef: meta.NamespacedObjectReference{ - Name: "not-expected-to-exist", - }, - Policy: imagev1_reflect.ImagePolicyChoice{ - SemVer: &imagev1_reflect.SemVerPolicy{ - Range: "1.x", - }, - }, - }, - } - Expect(k8sClient.Create(context.Background(), policy)).To(Succeed()) - policy.Status.LatestImage = latestImage - Expect(k8sClient.Status().Update(context.Background(), policy)).To(Succeed()) - + return newUpdate.Spec.Suspend + }, timeout, time.Second).Should(BeTrue()) + // Run the reconciliation explicitly, and make sure it + // doesn't do anything + result, err := imageAutoReconciler.Reconcile(logr.NewContext(context.TODO(), ctrl.Log), ctrl.Request{ + NamespacedName: updateKey, }) + g.Expect(err).To(BeNil()) + // This ought to fail if suspend is not working, since the item would be requeued; + // but if not, additional checks lie below. + g.Expect(result).To(Equal(ctrl.Result{})) - AfterEach(func() { - Expect(k8sClient.Delete(context.Background(), namespace)).To(Succeed()) - Expect(k8sClient.Delete(context.Background(), policy)).To(Succeed()) - Expect(gitServer.StopSSH()).To(Succeed()) - }) - - Context("with PushSpec", func() { - - var ( - update *imagev1.ImageUpdateAutomation - pushBranch string - ) - - BeforeEach(func() { - commitInRepo(cloneLocalRepoURL, branch, "Install setter marker", func(tmp string) { - Expect(replaceMarker(tmp, policyKey)).To(Succeed()) - }) - waitForNewHead(localRepo, branch) - - pushBranch = "pr-" + randStringRunes(5) - - update = &imagev1.ImageUpdateAutomation{ - Spec: imagev1.ImageUpdateAutomationSpec{ - SourceRef: imagev1.SourceReference{ - Kind: "GitRepository", - Name: gitRepoKey.Name, - }, - Update: &imagev1.UpdateStrategy{ - Strategy: imagev1.UpdateStrategySetters, - }, - Interval: metav1.Duration{Duration: 2 * time.Hour}, - GitSpec: &imagev1.GitSpec{ - Checkout: &imagev1.GitCheckoutSpec{ - Reference: sourcev1.GitRepositoryRef{ - Branch: branch, - }, - }, - Commit: imagev1.CommitSpec{ - Author: imagev1.CommitUser{ - Email: "fluxbot@example.com", - }, - MessageTemplate: commitMessage, - }, - Push: &imagev1.PushSpec{ - Branch: pushBranch, - }, - }, - }, - } - update.Name = "update-" + randStringRunes(5) - update.Namespace = namespace.Name - - Expect(k8sClient.Create(context.Background(), update)).To(Succeed()) - }) + var checkUpdate imagev1.ImageUpdateAutomation + g.Expect(testEnv.Get(context.Background(), updateKey, &checkUpdate)).To(Succeed()) + g.Expect(checkUpdate.Status.ObservedGeneration).NotTo(Equal(checkUpdate.ObjectMeta.Generation)) - It("creates and pushes the push branch", func() { - waitForNewHead(localRepo, pushBranch) - head, err := localRepo.Reference(plumbing.NewRemoteReferenceName(originRemote, pushBranch), true) - Expect(err).NotTo(HaveOccurred()) - commit, err := localRepo.CommitObject(head.Hash()) - Expect(err).ToNot(HaveOccurred()) - Expect(commit.Message).To(Equal(commitMessage)) - }) + // -- Reconciles when reconcile request annotation is added. - It("pushes another commit to the existing push branch", func() { - // observe the first commit - waitForNewHead(localRepo, pushBranch) - head, err := localRepo.Reference(plumbing.NewRemoteReferenceName(originRemote, pushBranch), true) - headHash := head.String() - Expect(err).NotTo(HaveOccurred()) - - // update the policy and expect another commit in the push branch - policy.Status.LatestImage = "helloworld:v1.3.0" - Expect(k8sClient.Status().Update(context.TODO(), policy)).To(Succeed()) - waitForNewHead(localRepo, pushBranch) - head, err = localRepo.Reference(plumbing.NewRemoteReferenceName(originRemote, pushBranch), true) - Expect(err).NotTo(HaveOccurred()) - Expect(head.String()).NotTo(Equal(headHash)) - }) + // The automation has run, and is not expected to run + // again for 2 hours. Make a commit to the git repo + // which needs to be undone by automation, then add + // the annotation and make sure it runs again. - It("still pushes to the push branch after it's merged", func() { - // observe the first commit - waitForNewHead(localRepo, pushBranch) - head, err := localRepo.Reference(plumbing.NewRemoteReferenceName(originRemote, pushBranch), true) - headHash := head.String() - Expect(err).NotTo(HaveOccurred()) - - // merge the push branch into checkout branch, and push the merge commit - // upstream. - // waitForNewHead() leaves the repo at the head of the branch given, i.e., the - // push branch), so we have to check out the "main" branch first. - Expect(checkoutBranch(localRepo, branch)).To(Succeed()) - mergeBranchIntoHead(localRepo, pushBranch) - - // update the policy and expect another commit in the push branch - policy.Status.LatestImage = "helloworld:v1.3.0" - Expect(k8sClient.Status().Update(context.TODO(), policy)).To(Succeed()) - waitForNewHead(localRepo, pushBranch) - head, err = localRepo.Reference(plumbing.NewRemoteReferenceName(originRemote, pushBranch), true) - Expect(err).NotTo(HaveOccurred()) - Expect(head.String()).NotTo(Equal(headHash)) - }) - - AfterEach(func() { - Expect(k8sClient.Delete(context.Background(), update)).To(Succeed()) - }) + // TODO: Implement adding request annotation. + // Refer: https://github.com/fluxcd/image-automation-controller/pull/82/commits/4fde199362b42fa37068f2e6c6885cfea474a3d1#diff-1168fadffa18bd096582ae7f8b6db744fd896bd5600ee1d1ac6ac4474af251b9L292-L334 - }) + g.Expect(testEnv.Get(context.Background(), updateKey, updateBySetters)).To(Succeed()) + g.Expect(updateBySetters.Status.LastAutomationRunTime).ToNot(BeNil()) + }) + } +} - Context("with Setters", func() { - - var ( - updateKey types.NamespacedName - updateBySetters *imagev1.ImageUpdateAutomation - ) - - BeforeEach(func() { - // Insert a setter reference into the deployment file, - // before creating the automation object itself. - commitInRepo(cloneLocalRepoURL, branch, "Install setter marker", func(tmp string) { - Expect(replaceMarker(tmp, policyKey)).To(Succeed()) - }) - - // pull the head commit we just pushed, so it's not - // considered a new commit when checking for a commit - // made by automation. - waitForNewHead(localRepo, branch) - - // now create the automation object, and let it (one - // hopes!) make a commit itself. - updateKey = types.NamespacedName{ - Namespace: gitRepoKey.Namespace, - Name: "update-" + randStringRunes(5), - } - updateBySetters = &imagev1.ImageUpdateAutomation{ - ObjectMeta: metav1.ObjectMeta{ - Name: updateKey.Name, - Namespace: updateKey.Namespace, - }, - Spec: imagev1.ImageUpdateAutomationSpec{ - Interval: metav1.Duration{Duration: 2 * time.Hour}, // this is to ensure any subsequent run should be outside the scope of the testing - SourceRef: imagev1.SourceReference{ - Kind: "GitRepository", - Name: gitRepoKey.Name, - }, - Update: &imagev1.UpdateStrategy{ - Strategy: imagev1.UpdateStrategySetters, - }, - GitSpec: &imagev1.GitSpec{ - Checkout: &imagev1.GitCheckoutSpec{ - Reference: sourcev1.GitRepositoryRef{ - Branch: branch, - }, - }, - Commit: imagev1.CommitSpec{ - Author: imagev1.CommitUser{ - Email: "fluxbot@example.com", - }, - MessageTemplate: commitMessage, - }, - }, - }, - } - Expect(k8sClient.Create(context.Background(), updateBySetters)).To(Succeed()) - // wait for a new commit to be made by the controller - waitForNewHead(localRepo, branch) - }) +func TestImageUpdateAutomation_defaulting(t *testing.T) { + g := NewWithT(t) - AfterEach(func() { - Expect(k8sClient.Delete(context.Background(), updateBySetters)).To(Succeed()) - }) + branch := randStringRunes(8) + namespace := &corev1.Namespace{} + namespace.Name = "image-auto-test-" + randStringRunes(5) - It("updates to the most recent image", func() { - // having passed the BeforeEach, we should see a commit - head, _ := localRepo.Head() - commit, err := localRepo.CommitObject(head.Hash()) - Expect(err).ToNot(HaveOccurred()) - Expect(commit.Message).To(Equal(commitMessage)) - - var newObj imagev1.ImageUpdateAutomation - Expect(k8sClient.Get(context.Background(), updateKey, &newObj)).To(Succeed()) - Expect(newObj.Status.LastPushCommit).To(Equal(head.Hash().String())) - Expect(newObj.Status.LastPushTime).ToNot(BeNil()) - - compareRepoWithExpected(cloneLocalRepoURL, branch, "testdata/appconfig-setters-expected", func(tmp string) { - Expect(replaceMarker(tmp, policyKey)).To(Succeed()) - }) - }) + ctx, cancel := context.WithTimeout(context.Background(), contextTimeout) + defer cancel() - It("stops updating when suspended", func() { - // suspend it, and check that reconciliation does not run - var updatePatch imagev1.ImageUpdateAutomation - Expect(k8sClient.Get(context.TODO(), updateKey, &updatePatch)).To(Succeed()) - updatePatch.Spec.Suspend = true - Expect(k8sClient.Patch(context.Background(), &updatePatch, client.Merge)).To(Succeed()) - // wait for the suspension to reach the cache - var newUpdate imagev1.ImageUpdateAutomation - Eventually(func() bool { - if err := imageAutoReconciler.Get(context.Background(), updateKey, &newUpdate); err != nil { - return false - } - return newUpdate.Spec.Suspend - }, timeout, time.Second).Should(BeTrue()) - // run the reconciliation explicitly, and make sure it - // doesn't do anything - result, err := imageAutoReconciler.Reconcile(logr.NewContext(context.TODO(), ctrl.Log), ctrl.Request{ - NamespacedName: updateKey, - }) - Expect(err).To(BeNil()) - // this ought to fail if suspend is not working, since the item would be requeued; - // but if not, additional checks lie below. - Expect(result).To(Equal(ctrl.Result{})) - - var checkUpdate imagev1.ImageUpdateAutomation - Expect(k8sClient.Get(context.Background(), updateKey, &checkUpdate)).To(Succeed()) - Expect(checkUpdate.Status.ObservedGeneration).NotTo(Equal(checkUpdate.ObjectMeta.Generation)) - }) + // Create a test namespace. + g.Expect(testEnv.Create(ctx, namespace)).To(Succeed()) + defer func() { + g.Expect(testEnv.Delete(ctx, namespace)).To(Succeed()) + }() - It("runs when the reconcile request annotation is added", func() { - // the automation has run, and is not expected to run - // again for 2 hours. Make a commit to the git repo - // which needs to be undone by automation, then add - // the annotation and make sure it runs again. - Expect(k8sClient.Get(context.Background(), updateKey, updateBySetters)).To(Succeed()) - Expect(updateBySetters.Status.LastAutomationRunTime).ToNot(BeNil()) - }) - }) - } + // Create an instance of ImageUpdateAutomation. + key := types.NamespacedName{ + Name: "update-" + randStringRunes(5), + Namespace: namespace.Name, } - - Context("Using go-git", func() { - Context("with HTTP", func() { - Describe("runs end to end", endToEnd(sourcev1.GoGitImplementation, "http")) - }) - Context("with SSH", func() { - Describe("runs end to end", endToEnd(sourcev1.GoGitImplementation, "ssh")) - }) - }) - - Context("Using libgit2", func() { - Context("with HTTP", func() { - Describe("runs end to end", endToEnd(sourcev1.LibGit2Implementation, "http")) - }) - Context("with SSH", func() { - Describe("runs end to end", endToEnd(sourcev1.LibGit2Implementation, "ssh")) - }) - }) - - Context("defaulting", func() { - var key types.NamespacedName - var auto *imagev1.ImageUpdateAutomation - - BeforeEach(func() { - key = types.NamespacedName{ - Namespace: namespace.Name, - Name: "update-" + randStringRunes(5), - } - auto = &imagev1.ImageUpdateAutomation{ - ObjectMeta: metav1.ObjectMeta{ - Name: key.Name, - Namespace: key.Namespace, - }, - Spec: imagev1.ImageUpdateAutomationSpec{ - SourceRef: imagev1.SourceReference{ - Kind: "GitRepository", - Name: "garbage", + auto := &imagev1.ImageUpdateAutomation{ + ObjectMeta: metav1.ObjectMeta{ + Name: key.Name, + Namespace: key.Namespace, + }, + Spec: imagev1.ImageUpdateAutomationSpec{ + SourceRef: imagev1.SourceReference{ + Kind: "GitRepository", + Name: "garbage", + }, + Interval: metav1.Duration{Duration: 2 * time.Hour}, // this is to ensure any subsequent run should be outside the scope of the testing + GitSpec: &imagev1.GitSpec{ + Checkout: &imagev1.GitCheckoutSpec{ + Reference: sourcev1.GitRepositoryRef{ + Branch: branch, }, - Interval: metav1.Duration{Duration: 2 * time.Hour}, // this is to ensure any subsequent run should be outside the scope of the testing - GitSpec: &imagev1.GitSpec{ - Checkout: &imagev1.GitCheckoutSpec{ - Reference: sourcev1.GitRepositoryRef{ - Branch: branch, - }, - }, - // leave Update field out - Commit: imagev1.CommitSpec{ - Author: imagev1.CommitUser{ - Email: "fluxbot@example.com", - }, - MessageTemplate: "nothing", - }, + }, + // leave Update field out + Commit: imagev1.CommitSpec{ + Author: imagev1.CommitUser{ + Email: "fluxbot@example.com", }, + MessageTemplate: "nothing", }, - } - Expect(k8sClient.Create(context.Background(), auto)).To(Succeed()) - }) - - AfterEach(func() { - Expect(k8sClient.Delete(context.Background(), auto)).To(Succeed()) - }) - - It("defaults .spec.update to {strategy: Setters}", func() { - var fetchedAuto imagev1.ImageUpdateAutomation - Expect(k8sClient.Get(context.Background(), key, &fetchedAuto)).To(Succeed()) - Expect(fetchedAuto.Spec.Update).To(Equal(&imagev1.UpdateStrategy{Strategy: imagev1.UpdateStrategySetters})) - }) - }) -}) + }, + }, + } + g.Expect(testEnv.Create(ctx, auto)).To(Succeed()) + defer func() { + g.Expect(testEnv.Delete(ctx, auto)).To(Succeed()) + }() + + // Should default .spec.update to {strategy: Setters}. + var fetchedAuto imagev1.ImageUpdateAutomation + g.Eventually(func() bool { + err := testEnv.Get(ctx, key, &fetchedAuto) + return err == nil + }, timeout, time.Second).Should(BeTrue()) + g.Expect(fetchedAuto.Spec.Update). + To(Equal(&imagev1.UpdateStrategy{Strategy: imagev1.UpdateStrategySetters})) +} func expectCommittedAndPushed(conditions []metav1.Condition) { rc := apimeta.FindStatusCondition(conditions, meta.ReadyCondition) @@ -1021,9 +844,9 @@ func setterRef(name types.NamespacedName) string { // the remote ref locally (or if there's no ref locally, until it has // fetched the remote branch). It resets the working tree head to the // remote branch ref. -func waitForNewHead(repo *git.Repository, branch string) { +func waitForNewHead(g *WithT, repo *git.Repository, branch string) { working, err := repo.Worktree() - Expect(err).ToNot(HaveOccurred()) + g.Expect(err).ToNot(HaveOccurred()) // Try to find the remote branch in the repo locally; this will // fail if we're on a branch that didn't exist when we cloned the @@ -1032,14 +855,14 @@ func waitForNewHead(repo *git.Repository, branch string) { remoteBranch := plumbing.NewRemoteReferenceName(originRemote, branch) remoteHead, err := repo.Reference(remoteBranch, false) if err != plumbing.ErrReferenceNotFound { - Expect(err).ToNot(HaveOccurred()) + g.Expect(err).ToNot(HaveOccurred()) } if err == nil { remoteHeadHash = remoteHead.Hash().String() } // otherwise, any reference fetched will do. // Now try to fetch new commits from that remote branch - Eventually(func() bool { + g.Eventually(func() bool { if err := repo.Fetch(&git.FetchOptions{ RefSpecs: []config.RefSpec{ config.RefSpec("refs/heads/" + branch + ":refs/remotes/origin/" + branch), @@ -1057,46 +880,46 @@ func waitForNewHead(repo *git.Repository, branch string) { // New commits in the remote branch -- reset the working tree head // to that. Note this does not create a local branch tracking the // remote, so it is a detached head. - Expect(working.Reset(&git.ResetOptions{ + g.Expect(working.Reset(&git.ResetOptions{ Commit: remoteHead.Hash(), Mode: git.HardReset, })).To(Succeed()) } -func compareRepoWithExpected(repoURL, branch, fixture string, changeFixture func(tmp string)) { +func compareRepoWithExpected(g *WithT, repoURL, branch, fixture string, changeFixture func(tmp string)) { expected, err := ioutil.TempDir("", "gotest-imageauto-expected") - Expect(err).ToNot(HaveOccurred()) + g.Expect(err).ToNot(HaveOccurred()) defer os.RemoveAll(expected) copy.Copy(fixture, expected) changeFixture(expected) tmp, err := ioutil.TempDir("", "gotest-imageauto") - Expect(err).ToNot(HaveOccurred()) + g.Expect(err).ToNot(HaveOccurred()) defer os.RemoveAll(tmp) _, err = git.PlainClone(tmp, false, &git.CloneOptions{ URL: repoURL, ReferenceName: plumbing.NewBranchReferenceName(branch), }) - Expect(err).ToNot(HaveOccurred()) - test.ExpectMatchingDirectories(tmp, expected) + g.Expect(err).ToNot(HaveOccurred()) + test.ExpectMatchingDirectories(g, tmp, expected) } -func commitInRepo(repoURL, branch, msg string, changeFiles func(path string)) { +func commitInRepo(g *WithT, repoURL, branch, msg string, changeFiles func(path string)) { tmp, err := ioutil.TempDir("", "gotest-imageauto") - Expect(err).ToNot(HaveOccurred()) + g.Expect(err).ToNot(HaveOccurred()) defer os.RemoveAll(tmp) repo, err := git.PlainClone(tmp, false, &git.CloneOptions{ URL: repoURL, ReferenceName: plumbing.NewBranchReferenceName(branch), }) - Expect(err).ToNot(HaveOccurred()) + g.Expect(err).ToNot(HaveOccurred()) changeFiles(tmp) worktree, err := repo.Worktree() - Expect(err).ToNot(HaveOccurred()) + g.Expect(err).ToNot(HaveOccurred()) _, err = worktree.Add(".") - Expect(err).ToNot(HaveOccurred()) + g.Expect(err).ToNot(HaveOccurred()) _, err = worktree.Commit(msg, &git.CommitOptions{ Author: &object.Signature{ Name: "Testbot", @@ -1104,8 +927,8 @@ func commitInRepo(repoURL, branch, msg string, changeFiles func(path string)) { When: time.Now(), }, }) - Expect(err).ToNot(HaveOccurred()) - Expect(repo.Push(&git.PushOptions{RemoteName: "origin"})).To(Succeed()) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(repo.Push(&git.PushOptions{RemoteName: "origin"})).To(Succeed()) } // Initialise a git server with a repo including the files in dir. @@ -1174,16 +997,16 @@ func checkoutBranch(repo *git.Repository, branch string) error { // This merges the push branch into HEAD, and pushes upstream. This is // to simulate e.g., a PR being merged. -func mergeBranchIntoHead(repo *git.Repository, pushBranch string) { +func mergeBranchIntoHead(g *WithT, repo *git.Repository, pushBranch string) { // hash of head headRef, err := repo.Head() - Expect(err).NotTo(HaveOccurred()) + g.Expect(err).NotTo(HaveOccurred()) pushBranchRef, err := repo.Reference(plumbing.NewRemoteReferenceName(originRemote, pushBranch), false) - Expect(err).NotTo(HaveOccurred()) + g.Expect(err).NotTo(HaveOccurred()) // You need the worktree to be able to create a commit worktree, err := repo.Worktree() - Expect(err).NotTo(HaveOccurred()) + g.Expect(err).NotTo(HaveOccurred()) _, err = worktree.Commit(fmt.Sprintf("Merge %s", pushBranch), &git.CommitOptions{ Author: &object.Signature{ Name: "Testbot", @@ -1192,11 +1015,11 @@ func mergeBranchIntoHead(repo *git.Repository, pushBranch string) { }, Parents: []plumbing.Hash{headRef.Hash(), pushBranchRef.Hash()}, }) - Expect(err).NotTo(HaveOccurred()) + g.Expect(err).NotTo(HaveOccurred()) // push upstream err = repo.Push(&git.PushOptions{ RemoteName: originRemote, }) - Expect(err).NotTo(HaveOccurred()) + g.Expect(err).NotTo(HaveOccurred()) } From 3a03fad8251f13299411e88c6049a8c235cc18cc Mon Sep 17 00:00:00 2001 From: Sunny Date: Sat, 25 Sep 2021 03:27:26 +0530 Subject: [PATCH 03/11] update pkg/test and pkg/update with testenv Signed-off-by: Sunny --- pkg/test/files.go | 24 ++--- pkg/test/files_test.go | 76 +++++++++------- pkg/update/filereader_test.go | 55 +++++------ pkg/update/result_test.go | 104 +++++++++++---------- pkg/update/update_test.go | 167 +++++++++++++--------------------- 5 files changed, 198 insertions(+), 228 deletions(-) diff --git a/pkg/test/files.go b/pkg/test/files.go index e6117588..e58e11d9 100644 --- a/pkg/test/files.go +++ b/pkg/test/files.go @@ -27,21 +27,21 @@ import ( // TODO rewrite this as just doing the diff, so I can test that it // fails at the right times too. -func ExpectMatchingDirectories(actualRoot, expectedRoot string) { - Expect(actualRoot).To(BeADirectory()) - Expect(expectedRoot).To(BeADirectory()) +func ExpectMatchingDirectories(g *WithT, actualRoot, expectedRoot string) { + g.Expect(actualRoot).To(BeADirectory()) + g.Expect(expectedRoot).To(BeADirectory()) actualonly, expectedonly, different := DiffDirectories(actualRoot, expectedRoot) - Expect(actualonly).To(BeEmpty(), "Expect no files in %s but not in %s", actualRoot, expectedRoot) - Expect(expectedonly).To(BeEmpty(), "Expect no files in %s but not in %s", expectedRoot, actualRoot) + g.Expect(actualonly).To(BeEmpty(), "Expect no files in %s but not in %s", actualRoot, expectedRoot) + g.Expect(expectedonly).To(BeEmpty(), "Expect no files in %s but not in %s", expectedRoot, actualRoot) // these are enumerated, so that the output is the actual difference for _, diff := range different { - diff.FailedExpectation() + diff.FailedExpectation(g) } } type Diff interface { Path() string - FailedExpectation() + FailedExpectation(g *WithT) } type contentdiff struct { @@ -53,8 +53,8 @@ func (d contentdiff) Path() string { } // Run an expectation that will fail, giving an appropriate error -func (d contentdiff) FailedExpectation() { - Expect(d.actual).To(Equal(d.expected)) +func (d contentdiff) FailedExpectation(g *WithT) { + g.Expect(d.actual).To(Equal(d.expected)) } type dirfile struct { @@ -66,11 +66,11 @@ func (d dirfile) Path() string { return d.path } -func (d dirfile) FailedExpectation() { +func (d dirfile) FailedExpectation(g *WithT) { if d.expectedRegularFile { - Expect(d.path).To(BeARegularFile()) + g.Expect(d.path).To(BeARegularFile()) } else { - Expect(d.path).To(BeADirectory()) + g.Expect(d.path).To(BeADirectory()) } } diff --git a/pkg/test/files_test.go b/pkg/test/files_test.go index 1e270bb4..46ce50b2 100644 --- a/pkg/test/files_test.go +++ b/pkg/test/files_test.go @@ -19,41 +19,51 @@ package test import ( "testing" - . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" ) -func TestFiles(t *testing.T) { - RegisterFailHandler(Fail) - RunSpecs(t, "Files comparison helper") +func TestExpectMatchingDirectories(t *testing.T) { + tests := []struct { + name string + actualRoot string + expectedRoot string + }{ + { + name: "same directory", + actualRoot: "testdata/base", + expectedRoot: "testdata/base", + }, + { + name: "different equivalent directories", + actualRoot: "testdata/base", + expectedRoot: "testdata/equiv", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + ExpectMatchingDirectories(g, tt.actualRoot, tt.expectedRoot) + }) + } } -var _ = Describe("when no differences", func() { - It("matches when given the same directory", func() { - ExpectMatchingDirectories("testdata/base", "testdata/base") - }) - It("matches when given equivalent directories", func() { - ExpectMatchingDirectories("testdata/base", "testdata/equiv") - }) -}) - -var _ = Describe("with differences", func() { - It("finds files in expected from a/ but not in actual b/", func() { - aonly, _, _ := DiffDirectories("testdata/diff/a", "testdata/diff/b") - Expect(aonly).To(Equal([]string{"/only", "/only/here.yaml", "/onlyhere.yaml"})) - }) - - It("finds files in actual a/ that weren't expected from b/", func() { - bonly, _, _ := DiffDirectories("testdata/diff/a", "testdata/diff/b") // change in order - Expect(bonly).To(Equal([]string{"/only", "/only/here.yaml", "/onlyhere.yaml"})) - }) - - It("finds files that are different in a and b", func() { - _, _, diffs := DiffDirectories("testdata/diff/a", "testdata/diff/b") - var diffpaths []string - for _, d := range diffs { - diffpaths = append(diffpaths, d.Path()) - } - Expect(diffpaths).To(Equal([]string{"/different/content.yaml", "/dirfile"})) - }) -}) +func TestDiffDirectories(t *testing.T) { + g := NewWithT(t) + + // Finds files in expected from a/ but not in actual b/. + aonly, _, _ := DiffDirectories("testdata/diff/a", "testdata/diff/b") + g.Expect(aonly).To(Equal([]string{"/only", "/only/here.yaml", "/onlyhere.yaml"})) + + // Finds files in actual a/ that weren't expected from b/. + bonly, _, _ := DiffDirectories("testdata/diff/a", "testdata/diff/b") // change in order + g.Expect(bonly).To(Equal([]string{"/only", "/only/here.yaml", "/onlyhere.yaml"})) + + // Finds files that are different in a and b. + _, _, diffs := DiffDirectories("testdata/diff/a", "testdata/diff/b") + var diffpaths []string + for _, d := range diffs { + diffpaths = append(diffpaths, d.Path()) + } + g.Expect(diffpaths).To(Equal([]string{"/different/content.yaml", "/dirfile"})) +} diff --git a/pkg/update/filereader_test.go b/pkg/update/filereader_test.go index 38e57a0b..935968eb 100644 --- a/pkg/update/filereader_test.go +++ b/pkg/update/filereader_test.go @@ -17,34 +17,35 @@ limitations under the License. package update import ( - . "github.com/onsi/ginkgo" + "testing" + . "github.com/onsi/gomega" "sigs.k8s.io/kustomize/kyaml/kio/kioutil" ) -var _ = Describe("load YAMLs with ScreeningLocalReader", func() { - It("loads only the YAMLs containing the token", func() { - r := ScreeningLocalReader{ - Path: "testdata/setters/original", - Token: "$imagepolicy", - } - nodes, err := r.Read() - Expect(err).ToNot(HaveOccurred()) - // the test fixture has three files that contain the marker: - // - otherns.yaml - // - marked.yaml - // - kustomization.yaml - Expect(len(nodes)).To(Equal(3)) - filesSeen := map[string]struct{}{} - for i := range nodes { - path, _, err := kioutil.GetFileAnnotations(nodes[i]) - Expect(err).ToNot(HaveOccurred()) - filesSeen[path] = struct{}{} - } - Expect(filesSeen).To(Equal(map[string]struct{}{ - "marked.yaml": struct{}{}, - "kustomization.yaml": struct{}{}, - "otherns.yaml": struct{}{}, - })) - }) -}) +func TestScreeningLocalReader(t *testing.T) { + g := NewWithT(t) + r := ScreeningLocalReader{ + Path: "testdata/setters/original", + Token: "$imagepolicy", + } + nodes, err := r.Read() + g.Expect(err).ToNot(HaveOccurred()) + // the test fixture has three files that contain the marker: + // - otherns.yaml + // - marked.yaml + // - kustomization.yaml + g.Expect(len(nodes)).To(Equal(3)) + filesSeen := map[string]struct{}{} + for i := range nodes { + path, _, err := kioutil.GetFileAnnotations(nodes[i]) + g.Expect(err).ToNot(HaveOccurred()) + filesSeen[path] = struct{}{} + } + g.Expect(filesSeen).To(Equal(map[string]struct{}{ + "marked.yaml": {}, + "kustomization.yaml": {}, + "otherns.yaml": {}, + })) + +} diff --git a/pkg/update/result_test.go b/pkg/update/result_test.go index e96205d9..0292470d 100644 --- a/pkg/update/result_test.go +++ b/pkg/update/result_test.go @@ -1,8 +1,9 @@ package update import ( + "testing" + "github.com/google/go-containerregistry/pkg/name" - . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/kustomize/kyaml/yaml" @@ -18,79 +19,76 @@ func mustRef(ref string) imageRef { return imageRef{r, types.NamespacedName{}} } -var _ = Describe("image ref", func() { - It("gives each component of an image ref", func() { +func TestMustRef(t *testing.T) { + g := NewWithT(t) + + t.Run("gives each component of an image ref", func(t *testing.T) { ref := mustRef("helloworld:v1.0.1") - Expect(ref.String()).To(Equal("helloworld:v1.0.1")) - Expect(ref.Identifier()).To(Equal("v1.0.1")) - Expect(ref.Repository()).To(Equal("library/helloworld")) - Expect(ref.Registry()).To(Equal("index.docker.io")) - Expect(ref.Name()).To(Equal("index.docker.io/library/helloworld:v1.0.1")) + g.Expect(ref.String()).To(Equal("helloworld:v1.0.1")) + g.Expect(ref.Identifier()).To(Equal("v1.0.1")) + g.Expect(ref.Repository()).To(Equal("library/helloworld")) + g.Expect(ref.Registry()).To(Equal("index.docker.io")) + g.Expect(ref.Name()).To(Equal("index.docker.io/library/helloworld:v1.0.1")) }) - It("deals with hostnames and digests", func() { + t.Run("deals with hostnames and digests", func(t *testing.T) { image := "localhost:5000/org/helloworld@sha256:6745aaad46d795c9836632e1fb62f24b7e7f4c843144da8e47a5465c411a14be" ref := mustRef(image) - Expect(ref.String()).To(Equal(image)) - Expect(ref.Identifier()).To(Equal("sha256:6745aaad46d795c9836632e1fb62f24b7e7f4c843144da8e47a5465c411a14be")) - Expect(ref.Repository()).To(Equal("org/helloworld")) - Expect(ref.Registry()).To(Equal("localhost:5000")) - Expect(ref.Name()).To(Equal(image)) + g.Expect(ref.String()).To(Equal(image)) + g.Expect(ref.Identifier()).To(Equal("sha256:6745aaad46d795c9836632e1fb62f24b7e7f4c843144da8e47a5465c411a14be")) + g.Expect(ref.Repository()).To(Equal("org/helloworld")) + g.Expect(ref.Registry()).To(Equal("localhost:5000")) + g.Expect(ref.Name()).To(Equal(image)) }) -}) +} -var _ = Describe("update results", func() { +func TestUpdateResults(t *testing.T) { + g := NewWithT(t) var result Result objectNames := []ObjectIdentifier{ - ObjectIdentifier{yaml.ResourceIdentifier{ + {yaml.ResourceIdentifier{ NameMeta: yaml.NameMeta{Namespace: "ns", Name: "foo"}, }}, - ObjectIdentifier{yaml.ResourceIdentifier{ + {yaml.ResourceIdentifier{ NameMeta: yaml.NameMeta{Namespace: "ns", Name: "bar"}, }}, } - BeforeEach(func() { - result = Result{ - Files: map[string]FileResult{ - "foo.yaml": { - Objects: map[ObjectIdentifier][]ImageRef{ - objectNames[0]: { - mustRef("image:v1.0"), - mustRef("other:v2.0"), - }, + result = Result{ + Files: map[string]FileResult{ + "foo.yaml": { + Objects: map[ObjectIdentifier][]ImageRef{ + objectNames[0]: { + mustRef("image:v1.0"), + mustRef("other:v2.0"), }, }, - "bar.yaml": { - Objects: map[ObjectIdentifier][]ImageRef{ - objectNames[1]: { - mustRef("image:v1.0"), - mustRef("other:v2.0"), - }, + }, + "bar.yaml": { + Objects: map[ObjectIdentifier][]ImageRef{ + objectNames[1]: { + mustRef("image:v1.0"), + mustRef("other:v2.0"), }, }, }, - } - }) + }, + } - It("deduplicates images", func() { - Expect(result.Images()).To(Equal([]ImageRef{ + g.Expect(result.Images()).To(Equal([]ImageRef{ + mustRef("image:v1.0"), + mustRef("other:v2.0"), + })) + + g.Expect(result.Objects()).To(Equal(map[ObjectIdentifier][]ImageRef{ + objectNames[0]: { mustRef("image:v1.0"), mustRef("other:v2.0"), - })) - }) - - It("collects images by object", func() { - Expect(result.Objects()).To(Equal(map[ObjectIdentifier][]ImageRef{ - objectNames[0]: { - mustRef("image:v1.0"), - mustRef("other:v2.0"), - }, - objectNames[1]: { - mustRef("image:v1.0"), - mustRef("other:v2.0"), - }, - })) - }) -}) + }, + objectNames[1]: { + mustRef("image:v1.0"), + mustRef("other:v2.0"), + }, + })) +} diff --git a/pkg/update/update_test.go b/pkg/update/update_test.go index 03efe317..486d47c3 100644 --- a/pkg/update/update_test.go +++ b/pkg/update/update_test.go @@ -23,7 +23,6 @@ import ( "github.com/go-logr/logr" "github.com/google/go-containerregistry/pkg/name" - . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -33,116 +32,78 @@ import ( imagev1_reflect "github.com/fluxcd/image-reflector-controller/api/v1beta1" ) -func TestUpdate(t *testing.T) { - RegisterFailHandler(Fail) - RunSpecs(t, "Update suite") -} - -var _ = Describe("Update image via kyaml setters2", func() { +func TestUpdateWithSetters(t *testing.T) { + g := NewWithT(t) - var ( - policies = []imagev1_reflect.ImagePolicy{ - { - ObjectMeta: metav1.ObjectMeta{ // name matches marker used in testdata/setters/{original,expected} - Namespace: "automation-ns", - Name: "policy", - }, - Status: imagev1_reflect.ImagePolicyStatus{ - LatestImage: "index.repo.fake/updated:v1.0.1", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ // name matches marker used in testdata/setters/{original,expected} - Namespace: "automation-ns", - Name: "unchanged", - }, - Status: imagev1_reflect.ImagePolicyStatus{ - LatestImage: "image:v1.0.0", - }, - }, - } - ) - - It("updates the image marked with the image policy (setter) ref", func() { - tmp, err := ioutil.TempDir("", "gotest") - Expect(err).ToNot(HaveOccurred()) - defer os.RemoveAll(tmp) - - policies := []imagev1_reflect.ImagePolicy{ - { - ObjectMeta: metav1.ObjectMeta{ // name matches marker used in testdata/setters/{original,expected} - Namespace: "automation-ns", - Name: "policy", - }, - Status: imagev1_reflect.ImagePolicyStatus{ - LatestImage: "index.repo.fake/updated:v1.0.1", - }, + policies := []imagev1_reflect.ImagePolicy{ + { + ObjectMeta: metav1.ObjectMeta{ // name matches marker used in testdata/setters/{original,expected} + Namespace: "automation-ns", + Name: "policy", }, - { - ObjectMeta: metav1.ObjectMeta{ // name matches marker used in testdata/setters/{original,expected} - Namespace: "automation-ns", - Name: "unchanged", - }, - Status: imagev1_reflect.ImagePolicyStatus{ - LatestImage: "image:v1.0.0", - }, + Status: imagev1_reflect.ImagePolicyStatus{ + LatestImage: "index.repo.fake/updated:v1.0.1", }, - } - - _, err = UpdateWithSetters(logr.Discard(), "testdata/setters/original", tmp, policies) - Expect(err).ToNot(HaveOccurred()) - test.ExpectMatchingDirectories(tmp, "testdata/setters/expected") - }) - - It("gives the result of the updates", func() { - tmp, err := ioutil.TempDir("", "gotest") - Expect(err).ToNot(HaveOccurred()) - defer os.RemoveAll(tmp) - - result, err := UpdateWithSetters(logr.Discard(), "testdata/setters/original", tmp, policies) - Expect(err).ToNot(HaveOccurred()) - - kustomizeResourceID := ObjectIdentifier{yaml.ResourceIdentifier{ - TypeMeta: yaml.TypeMeta{ - APIVersion: "kustomize.config.k8s.io/v1beta1", - Kind: "Kustomization", - }, - }} - markedResourceID := ObjectIdentifier{yaml.ResourceIdentifier{ - TypeMeta: yaml.TypeMeta{ - APIVersion: "batch/v1beta1", - Kind: "CronJob", + }, + { + ObjectMeta: metav1.ObjectMeta{ // name matches marker used in testdata/setters/{original,expected} + Namespace: "automation-ns", + Name: "unchanged", }, - NameMeta: yaml.NameMeta{ - Namespace: "bar", - Name: "foo", + Status: imagev1_reflect.ImagePolicyStatus{ + LatestImage: "image:v1.0.0", }, - }} - r, _ := name.ParseReference("index.repo.fake/updated:v1.0.1") - expectedImageRef := imageRef{r, types.NamespacedName{ - Name: "policy", - Namespace: "automation-ns", - }} - - expectedResult := Result{ - Files: map[string]FileResult{ - "kustomization.yaml": { - Objects: map[ObjectIdentifier][]ImageRef{ - kustomizeResourceID: { - expectedImageRef, - }, + }, + } + + tmp, err := ioutil.TempDir("", "gotest") + g.Expect(err).ToNot(HaveOccurred()) + defer os.RemoveAll(tmp) + + result, err := UpdateWithSetters(logr.Discard(), "testdata/setters/original", tmp, policies) + g.Expect(err).ToNot(HaveOccurred()) + test.ExpectMatchingDirectories(g, tmp, "testdata/setters/expected") + + kustomizeResourceID := ObjectIdentifier{yaml.ResourceIdentifier{ + TypeMeta: yaml.TypeMeta{ + APIVersion: "kustomize.config.k8s.io/v1beta1", + Kind: "Kustomization", + }, + }} + markedResourceID := ObjectIdentifier{yaml.ResourceIdentifier{ + TypeMeta: yaml.TypeMeta{ + APIVersion: "batch/v1beta1", + Kind: "CronJob", + }, + NameMeta: yaml.NameMeta{ + Namespace: "bar", + Name: "foo", + }, + }} + r, _ := name.ParseReference("index.repo.fake/updated:v1.0.1") + expectedImageRef := imageRef{r, types.NamespacedName{ + Name: "policy", + Namespace: "automation-ns", + }} + + expectedResult := Result{ + Files: map[string]FileResult{ + "kustomization.yaml": { + Objects: map[ObjectIdentifier][]ImageRef{ + kustomizeResourceID: { + expectedImageRef, }, }, - "marked.yaml": { - Objects: map[ObjectIdentifier][]ImageRef{ - markedResourceID: { - expectedImageRef, - }, + }, + "marked.yaml": { + Objects: map[ObjectIdentifier][]ImageRef{ + markedResourceID: { + expectedImageRef, }, }, }, - } + }, + } - Expect(result).To(Equal(expectedResult)) - }) -}) + g.Expect(result).To(Equal(expectedResult)) +} From 11dfea5d945635b6d9d2a967edfe79670599fac5 Mon Sep 17 00:00:00 2001 From: Sunny Date: Sat, 25 Sep 2021 03:28:10 +0530 Subject: [PATCH 04/11] Remove legacy_suite_test.go All the tests use testenv. Remove legacy envtest suite_test. Signed-off-by: Sunny --- controllers/legacy_suite_test.go | 111 ------------------------------- 1 file changed, 111 deletions(-) delete mode 100644 controllers/legacy_suite_test.go diff --git a/controllers/legacy_suite_test.go b/controllers/legacy_suite_test.go deleted file mode 100644 index 42fc5161..00000000 --- a/controllers/legacy_suite_test.go +++ /dev/null @@ -1,111 +0,0 @@ -/* -Copyright 2020 The Flux authors - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package controllers - -import ( - "path/filepath" - "testing" - - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - "k8s.io/client-go/kubernetes/scheme" - "k8s.io/client-go/rest" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/envtest" - "sigs.k8s.io/controller-runtime/pkg/envtest/printer" - "sigs.k8s.io/controller-runtime/pkg/log/zap" - - imagev1_reflect "github.com/fluxcd/image-reflector-controller/api/v1beta1" - sourcev1 "github.com/fluxcd/source-controller/api/v1beta1" - - imagev1 "github.com/fluxcd/image-automation-controller/api/v1beta1" - // +kubebuilder:scaffold:imports -) - -// These tests use Ginkgo (BDD-style Go testing framework). Refer to -// http://onsi.github.io/ginkgo/ to learn more about Ginkgo. - -var cfg *rest.Config -var k8sClient client.Client -var k8sManager ctrl.Manager -var imageAutoReconciler *ImageUpdateAutomationReconciler -var ginkgoTestEnv *envtest.Environment - -func TestAPIs(t *testing.T) { - RegisterFailHandler(Fail) - - RunSpecsWithDefaultAndCustomReporters(t, - "Controller Suite", - []Reporter{printer.NewlineReporter{}}) -} - -var _ = BeforeSuite(func(done Done) { - ctrl.SetLogger( - zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true)), - ) - - By("bootstrapping test environment") - ginkgoTestEnv = &envtest.Environment{ - CRDDirectoryPaths: []string{ - filepath.Join("..", "config", "crd", "bases"), - filepath.Join("testdata", "crds"), - }, - } - - var err error - cfg, err = ginkgoTestEnv.Start() - Expect(err).ToNot(HaveOccurred()) - Expect(cfg).ToNot(BeNil()) - - Expect(sourcev1.AddToScheme(scheme.Scheme)).To(Succeed()) - Expect(imagev1_reflect.AddToScheme(scheme.Scheme)).To(Succeed()) - - Expect(imagev1.AddToScheme(scheme.Scheme)).To(Succeed()) - // +kubebuilder:scaffold:scheme - - k8sManager, err = ctrl.NewManager(cfg, ctrl.Options{ - Scheme: scheme.Scheme, - }) - Expect(err).ToNot(HaveOccurred()) - - imageAutoReconciler = &ImageUpdateAutomationReconciler{ - Client: k8sManager.GetClient(), - Scheme: scheme.Scheme, - } - Expect(imageAutoReconciler.SetupWithManager(k8sManager, ImageUpdateAutomationReconcilerOptions{})).To(Succeed()) - - go func() { - defer GinkgoRecover() - err = k8sManager.Start(ctrl.SetupSignalHandler()) - Expect(err).ToNot(HaveOccurred()) - }() - - // Specifically an uncached client. Use .Get if you - // want to see what the reconcilers see. - k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}) - Expect(err).ToNot(HaveOccurred()) - Expect(k8sClient).ToNot(BeNil()) - - close(done) -}, 60) - -var _ = AfterSuite(func() { - By("tearing down the test environment") - err := ginkgoTestEnv.Stop() - Expect(err).ToNot(HaveOccurred()) -}) From f415d86eb966e6459c0074b25297fe44de7c06f8 Mon Sep 17 00:00:00 2001 From: Sunny Date: Fri, 1 Oct 2021 21:57:29 +0530 Subject: [PATCH 05/11] Update tests to be compatible with CR > v0.10.0 In controller-runtime v0.10.0, the client is updated to clean any stale data in the target object when performing any operation. This results in test failure for the code that constructs an object with both spec and status, and creates the object and updates status it with the same object. The fix is to set the status separately on the object before updating it. Refer: https://github.com/kubernetes-sigs/controller-runtime/pull/1640 Signed-off-by: Sunny --- controllers/update_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/controllers/update_test.go b/controllers/update_test.go index 18ee099d..d2b3f153 100644 --- a/controllers/update_test.go +++ b/controllers/update_test.go @@ -229,6 +229,7 @@ Images: policy.Name = policyKey.Name policy.Namespace = policyKey.Namespace g.Expect(testEnv.Create(ctx, policy)).To(Succeed()) + policy.Status.LatestImage = "helloworld:v1.0.0" g.Expect(testEnv.Status().Update(ctx, policy)).To(Succeed()) // Format the expected message given the generated values @@ -525,13 +526,11 @@ func TestImageUpdateAutomation_e2e(t *testing.T) { }, }, }, - Status: imagev1_reflect.ImagePolicyStatus{ - LatestImage: latestImage, - }, } policy.Name = policyKey.Name policy.Namespace = policyKey.Namespace g.Expect(testEnv.Create(ctx, policy)).To(Succeed()) + policy.Status.LatestImage = latestImage g.Expect(testEnv.Status().Update(ctx, policy)).To(Succeed()) defer func() { g.Expect(testEnv.Delete(ctx, policy)).To(Succeed()) From 8a36a79d9f8da4454a090ac57fb38d1ba4b74218 Mon Sep 17 00:00:00 2001 From: Michael Bridgen Date: Mon, 18 Oct 2021 14:35:45 +0100 Subject: [PATCH 06/11] Correct directory diffing test and algorithm Two steps: 1. TestDiffDirectories did not check if the expected only return value was correct; the intention was there to do so (judging by the comment "change in order"), but my eye for detail failed me. 2. Reversing the directory comparison in the test revealed bugs in the comparison code -- in general, it should skip any directory that is not a directory in the comparator. To make this easier, the code now keeps track of the expected files it saw. That means the check for whether an actual file has an expected counterpart only has two cases, yes or no (rather that trying to account for whether it's a directory and so on). If a directory was skipped while scanning the expected files, it won't be in the actual files anyway. Signed-off-by: Michael Bridgen --- pkg/test/files.go | 33 +++++++++++++++++++-------------- pkg/test/files_test.go | 12 ++++++------ 2 files changed, 25 insertions(+), 20 deletions(-) diff --git a/pkg/test/files.go b/pkg/test/files.go index e58e11d9..fc133782 100644 --- a/pkg/test/files.go +++ b/pkg/test/files.go @@ -83,16 +83,21 @@ func (d dirfile) FailedExpectation(g *WithT) { // `foo.yaml~`). It panics if it encounters any error apart from a // file not found. func DiffDirectories(actual, expected string) (actualonly []string, expectedonly []string, different []Diff) { + seen := make(map[string]struct{}) + filepath.Walk(expected, func(expectedPath string, expectedInfo os.FileInfo, err error) error { if err != nil { panic(err) } + + relPath := expectedPath[len(expected):] + seen[relPath] = struct{}{} + // ignore emacs backups if strings.HasSuffix(expectedPath, "~") { return nil } - relPath := expectedPath[len(expected):] - actualPath := filepath.Join(actual, relPath) + // ignore dotfiles if strings.HasPrefix(filepath.Base(expectedPath), ".") { if expectedInfo.IsDir() { @@ -101,24 +106,30 @@ func DiffDirectories(actual, expected string) (actualonly []string, expectedonly return nil } + actualPath := filepath.Join(actual, relPath) actualInfo, err := os.Stat(actualPath) switch { case err == nil: break case os.IsNotExist(err): expectedonly = append(expectedonly, relPath) + if expectedInfo.IsDir() { + return filepath.SkipDir + } return nil default: panic(err) } // file exists in both places - switch { case actualInfo.IsDir() && expectedInfo.IsDir(): return nil // i.e., keep recursing case actualInfo.IsDir() || expectedInfo.IsDir(): different = append(different, dirfile{path: relPath, abspath: actualPath, expectedRegularFile: actualInfo.IsDir()}) + if expectedInfo.IsDir() { + return filepath.SkipDir + } return nil } @@ -152,18 +163,12 @@ func DiffDirectories(actual, expected string) (actualonly []string, expectedonly if actualInfo.IsDir() && strings.HasPrefix(filepath.Base(actualPath), ".") { return filepath.SkipDir } - // since I've already compared any file that exists in - // expected or both, I'm only concerned with files that appear - // in actual but not in expected. - expectedPath := filepath.Join(expected, relPath) - _, err = os.Stat(expectedPath) - switch { - case err == nil: - break - case os.IsNotExist(err): + + if _, ok := seen[relPath]; !ok { actualonly = append(actualonly, relPath) - default: - panic(err) + if actualInfo.IsDir() { + return filepath.SkipDir + } } return nil }) diff --git a/pkg/test/files_test.go b/pkg/test/files_test.go index 46ce50b2..b5e5f37d 100644 --- a/pkg/test/files_test.go +++ b/pkg/test/files_test.go @@ -51,13 +51,13 @@ func TestExpectMatchingDirectories(t *testing.T) { func TestDiffDirectories(t *testing.T) { g := NewWithT(t) - // Finds files in expected from a/ but not in actual b/. - aonly, _, _ := DiffDirectories("testdata/diff/a", "testdata/diff/b") - g.Expect(aonly).To(Equal([]string{"/only", "/only/here.yaml", "/onlyhere.yaml"})) - // Finds files in actual a/ that weren't expected from b/. - bonly, _, _ := DiffDirectories("testdata/diff/a", "testdata/diff/b") // change in order - g.Expect(bonly).To(Equal([]string{"/only", "/only/here.yaml", "/onlyhere.yaml"})) + actualonly, _, _ := DiffDirectories("testdata/diff/a", "testdata/diff/b") + g.Expect(actualonly).To(Equal([]string{"/only", "/onlyhere.yaml"})) + + // Finds files in expected from a/ but not in actual b/. + _, expectedonly, _ := DiffDirectories("testdata/diff/b", "testdata/diff/a") // NB change in order + g.Expect(expectedonly).To(Equal([]string{"/only", "/onlyhere.yaml"})) // Finds files that are different in a and b. _, _, diffs := DiffDirectories("testdata/diff/a", "testdata/diff/b") From b8c9f43b314fa0f03f68fef5eed082e5d734fefb Mon Sep 17 00:00:00 2001 From: Sunny Date: Wed, 20 Oct 2021 21:34:03 +0530 Subject: [PATCH 07/11] Restructure tests in update_test.go Restructures the tests in update_test.go to separate the individual checks into separate tests with helpers for common operations. Signed-off-by: Sunny --- controllers/update_test.go | 1173 ++++++++++++++++++++---------------- 1 file changed, 643 insertions(+), 530 deletions(-) diff --git a/controllers/update_test.go b/controllers/update_test.go index d2b3f153..4618b224 100644 --- a/controllers/update_test.go +++ b/controllers/update_test.go @@ -60,8 +60,6 @@ import ( "github.com/fluxcd/image-automation-controller/pkg/update" ) -const timeout = 10 * time.Second - // Copied from // https://github.com/fluxcd/source-controller/blob/master/controllers/suite_test.go var letterRunes = []rune("abcdefghijklmnopqrstuvwxyz1234567890") @@ -74,13 +72,11 @@ func randStringRunes(n int) string { return string(b) } -func TestImageUpdateAutomation_commit_spec(t *testing.T) { - g := NewWithT(t) - - const ( - authorName = "Flux B Ot" - authorEmail = "fluxbot@example.com" - commitTemplate = `Commit summary +const ( + timeout = 10 * time.Second + testAuthorName = "Flux B Ot" + testAuthorEmail = "fluxbot@example.com" + testCommitTemplate = `Commit summary Automation: {{ .AutomationObject }} @@ -99,7 +95,7 @@ Images: - {{.}} ({{.Policy.Name}}) {{ end -}} ` - commitMessageFmt = `Commit summary + testCommitMessageFmt = `Commit summary Automation: %s/update-test @@ -110,259 +106,243 @@ Objects: Images: - helloworld:v1.0.0 (%s) ` - ) +) - tests := []struct { - name string - testdataPath string - updateStrategy *imagev1.UpdateStrategy - sign bool - }{ - { - name: "with non-path update setter", - testdataPath: "testdata/appconfig", - updateStrategy: &imagev1.UpdateStrategy{ - Strategy: imagev1.UpdateStrategySetters, - }, - }, - { - name: "with path update setter", - testdataPath: "testdata/pathconfig", - updateStrategy: &imagev1.UpdateStrategy{ - Strategy: imagev1.UpdateStrategySetters, - Path: "./yes", - }, - }, - { - name: "with signed commit", - testdataPath: "testdata/appconfig", - updateStrategy: &imagev1.UpdateStrategy{ - Strategy: imagev1.UpdateStrategySetters, - }, - sign: true, - }, - } +func TestImageUpdateAutomation_commit_message(t *testing.T) { + g := NewWithT(t) + + namespace := "image-auto-test-" + randStringRunes(5) + branch := randStringRunes(8) + repositoryPath := "/config-" + randStringRunes(6) + ".git" + gitRepoName := "image-auto-" + randStringRunes(5) + imagePolicyName := "policy-" + randStringRunes(5) // Create test git server. - gitServer, err := gittestserver.NewTempGitServer() - g.Expect(err).NotTo(HaveOccurred()) + gitServer, err := setupGitTestServer() + g.Expect(err).ToNot(HaveOccurred(), "failed to create test git server") defer os.RemoveAll(gitServer.Root()) + defer gitServer.StopHTTP() - username := randStringRunes(5) - password := randStringRunes(5) - // using authentication makes using the server more fiddly in - // general, but is required for testing SSH. - gitServer.Auth(username, password) - gitServer.AutoCreate() + // Create test namespace. + nsCleanup, err := createNamespace(namespace) + g.Expect(err).ToNot(HaveOccurred(), "failed to create test namespace") + defer func() { + g.Expect(nsCleanup()).To(Succeed()) + }() - g.Expect(gitServer.StartHTTP()).To(Succeed()) - defer gitServer.StopHTTP() + // Create a git repo. + g.Expect(initGitRepo(gitServer, "testdata/appconfig", branch, repositoryPath)).To(Succeed()) - gitServer.KeyDir(filepath.Join(gitServer.Root(), "keys")) - g.Expect(gitServer.ListenSSH()).To(Succeed()) + // Clone the repo. + repoURL := gitServer.HTTPAddressWithCredentials() + repositoryPath + localRepo, err := cloneRepo(repoURL, branch) + g.Expect(err).ToNot(HaveOccurred(), "failed to clone git repo") - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - g := NewWithT(t) + // Create GitRepository resource for the above repo. + err = createGitRepository(gitRepoName, namespace, "", repoURL, "") + g.Expect(err).ToNot(HaveOccurred(), "failed to create GitRepository resource") - namespace := &corev1.Namespace{} - namespace.Name = "image-auto-test-" + randStringRunes(5) + // Create ImagePolicy with populated latest image in the status. + err = createImagePolicyWithLatestImage(imagePolicyName, namespace, "not-expected-to-exist", "1.x", "helloworld:v1.0.0") + g.Expect(err).ToNot(HaveOccurred(), "failed to create ImagePolicy resource") - ctx, cancel := context.WithTimeout(context.Background(), contextTimeout) - defer cancel() + t.Run("update with commit template", func(t *testing.T) { + commitMessage := fmt.Sprintf(testCommitMessageFmt, namespace, imagePolicyName) - // Create a test namespace. - g.Expect(testEnv.Create(ctx, namespace)).To(Succeed()) - defer func() { - g.Expect(testEnv.Delete(ctx, namespace)).To(Succeed()) - }() + // Update the setter marker in the repo. + policyKey := types.NamespacedName{ + Name: imagePolicyName, + Namespace: namespace, + } + commitInRepo(g, repoURL, branch, "Install setter marker", func(tmp string) { + g.Expect(replaceMarker(tmp, policyKey)).To(Succeed()) + }) - branch := randStringRunes(8) - repositoryPath := "/config-" + randStringRunes(6) + ".git" + // Pull the head commit that was just pushed, so it's not considered a new + // commit when checking for a commit made by automation. + waitForNewHead(g, localRepo, branch) - // Initialize a git repo. - g.Expect(initGitRepo(gitServer, tt.testdataPath, branch, repositoryPath)).To(Succeed()) - - // Clone the repo. - repoURL := gitServer.HTTPAddressWithCredentials() + repositoryPath - localRepo, err := git.Clone(memory.NewStorage(), memfs.New(), &git.CloneOptions{ - URL: repoURL, - RemoteName: "origin", - ReferenceName: plumbing.NewBranchReferenceName(branch), - }) - g.Expect(err).ToNot(HaveOccurred()) + // Create the automation object and let it make a commit itself. + updateStrategy := &imagev1.UpdateStrategy{ + Strategy: imagev1.UpdateStrategySetters, + } + err = createImageUpdateAutomation("update-test", namespace, gitRepoName, branch, "", testCommitTemplate, "", updateStrategy) + g.Expect(err).ToNot(HaveOccurred()) - // Create GitRepository resource for the above repo. - gitRepoKey := types.NamespacedName{ - Name: "image-auto-" + randStringRunes(5), - Namespace: namespace.Name, - } - gitRepo := &sourcev1.GitRepository{ - Spec: sourcev1.GitRepositorySpec{ - URL: repoURL, - Interval: metav1.Duration{Duration: time.Minute}, - }, - } - gitRepo.Name = gitRepoKey.Name - gitRepo.Namespace = namespace.Name - g.Expect(testEnv.Create(ctx, gitRepo)).To(Succeed()) + // Wait for a new commit to be made by the controller. + waitForNewHead(g, localRepo, branch) - // Create an image policy. - policyKey := types.NamespacedName{ - Name: "policy-" + randStringRunes(5), - Namespace: namespace.Name, - } - // NB not testing the image reflector controller; this - // will make a "fully formed" ImagePolicy object. - policy := &imagev1_reflect.ImagePolicy{ - Spec: imagev1_reflect.ImagePolicySpec{ - ImageRepositoryRef: meta.NamespacedObjectReference{ - Name: "not-expected-to-exist", - }, - Policy: imagev1_reflect.ImagePolicyChoice{ - SemVer: &imagev1_reflect.SemVerPolicy{ - Range: "1.x", - }, - }, - }, - } - policy.Name = policyKey.Name - policy.Namespace = policyKey.Namespace - g.Expect(testEnv.Create(ctx, policy)).To(Succeed()) - policy.Status.LatestImage = "helloworld:v1.0.0" - g.Expect(testEnv.Status().Update(ctx, policy)).To(Succeed()) - - // Format the expected message given the generated values - commitMessage := fmt.Sprintf(commitMessageFmt, namespace.Name, policyKey.Name) - - // Insert a setter reference into the deployment file, - // before creating the automation object itself. - if tt.updateStrategy.Path == "" { - commitInRepo(g, repoURL, branch, "Install setter marker", func(tmp string) { - g.Expect(replaceMarker(tmp, policyKey)).To(Succeed()) - }) - } else { - commitInRepo(g, repoURL, branch, "Install setter marker", func(tmp string) { - g.Expect(replaceMarker(path.Join(tmp, "yes"), policyKey)).To(Succeed()) - }) - commitInRepo(g, repoURL, branch, "Install setter marker", func(tmp string) { - g.Expect(replaceMarker(path.Join(tmp, "no"), policyKey)).To(Succeed()) - }) - } + head, _ := localRepo.Head() + commit, err := localRepo.CommitObject(head.Hash()) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(commit.Author).NotTo(BeNil()) + g.Expect(commit.Author.Name).To(Equal(testAuthorName)) + g.Expect(commit.Author.Email).To(Equal(testAuthorEmail)) + g.Expect(commit.Message).To(Equal(commitMessage)) + }) +} - // Pull the head commit we just pushed, so it's not - // considered a new commit when checking for a commit - // made by automation. - waitForNewHead(g, localRepo, branch) - - var pgpEntity *openpgp.Entity - var sec *corev1.Secret - if tt.sign { - var err error - // generate keypair for signing - pgpEntity, err = openpgp.NewEntity("", "", "", nil) - g.Expect(err).ToNot(HaveOccurred()) +func TestImageUpdateAutomation_update_path(t *testing.T) { + g := NewWithT(t) - // configure OpenPGP armor encoder - b := bytes.NewBuffer(nil) - w, err := armor.Encode(b, openpgp.PrivateKeyType, nil) - g.Expect(err).ToNot(HaveOccurred()) + namespace := "image-auto-test-" + randStringRunes(5) + branch := randStringRunes(8) + repositoryPath := "/config-" + randStringRunes(6) + ".git" + gitRepoName := "image-auto-" + randStringRunes(5) + imagePolicyName := "policy-" + randStringRunes(5) - // serialize private key - err = pgpEntity.SerializePrivate(w, nil) - g.Expect(err).ToNot(HaveOccurred()) - err = w.Close() - g.Expect(err).ToNot(HaveOccurred()) + // Create test git server. + gitServer, err := setupGitTestServer() + g.Expect(err).ToNot(HaveOccurred(), "failed to create test git server") + defer os.RemoveAll(gitServer.Root()) + defer gitServer.StopHTTP() - // create the secret containing signing key - sec = &corev1.Secret{ - Data: map[string][]byte{ - "git.asc": b.Bytes(), - }, - } - sec.Name = "signing-key-secret-" + randStringRunes(5) - sec.Namespace = namespace.Name - g.Expect(testEnv.Create(ctx, sec)).To(Succeed()) - } + // Create test namespace. + nsCleanup, err := createNamespace(namespace) + g.Expect(err).ToNot(HaveOccurred(), "failed to create test namespace") + defer func() { + g.Expect(nsCleanup()).To(Succeed()) + }() - // Now create the automation object, and let it (one - // hopes!) make a commit itself. - updateKey := types.NamespacedName{ - Namespace: namespace.Name, - Name: "update-test", - } - updateBySetters := &imagev1.ImageUpdateAutomation{ - Spec: imagev1.ImageUpdateAutomationSpec{ - Interval: metav1.Duration{Duration: 2 * time.Hour}, // this is to ensure any subsequent run should be outside the scope of the testing - SourceRef: imagev1.SourceReference{ - Kind: "GitRepository", - Name: gitRepoKey.Name, - }, - GitSpec: &imagev1.GitSpec{ - Checkout: &imagev1.GitCheckoutSpec{ - Reference: sourcev1.GitRepositoryRef{ - Branch: branch, - }, - }, - Commit: imagev1.CommitSpec{ - MessageTemplate: commitTemplate, - Author: imagev1.CommitUser{ - Name: authorName, - Email: authorEmail, - }, - }, - }, - Update: tt.updateStrategy, - }, - } - updateBySetters.Name = updateKey.Name - updateBySetters.Namespace = updateKey.Namespace + // Create a git repo. + g.Expect(initGitRepo(gitServer, "testdata/pathconfig", branch, repositoryPath)).To(Succeed()) - // Add commit signing info. - if tt.sign { - updateBySetters.Spec.GitSpec.Commit.SigningKey = &imagev1.SigningKey{ - SecretRef: meta.LocalObjectReference{Name: sec.Name}, - } - } + // Clone the repo. + repoURL := gitServer.HTTPAddressWithCredentials() + repositoryPath + localRepo, err := cloneRepo(repoURL, branch) + g.Expect(err).ToNot(HaveOccurred(), "failed to clone git repo") - // Create the image update automation object. - g.Expect(testEnv.Create(ctx, updateBySetters)).To(Succeed()) - // Wait for a new commit to be made by the controller. - waitForNewHead(g, localRepo, branch) + // Create GitRepository resource for the above repo. + err = createGitRepository(gitRepoName, namespace, "", repoURL, "") + g.Expect(err).ToNot(HaveOccurred(), "failed to create GitRepository resource") - head, _ := localRepo.Head() - commit, err := localRepo.CommitObject(head.Hash()) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(commit.Author).NotTo(BeNil()) - g.Expect(commit.Author.Name).To(Equal(authorName)) - g.Expect(commit.Author.Email).To(Equal(authorEmail)) + // Create ImagePolicy with populated latest image in the status. + err = createImagePolicyWithLatestImage(imagePolicyName, namespace, "not-expected-to-exist", "1.x", "helloworld:v1.0.0") + g.Expect(err).ToNot(HaveOccurred(), "failed to create ImagePolicy resource") - if tt.updateStrategy.Path == "" { - g.Expect(commit.Message).To(Equal(commitMessage)) - } else { - g.Expect(commit.Message).To(Not(ContainSubstring("update-no"))) - g.Expect(commit.Message).To(ContainSubstring("update-yes")) - } + t.Run("update only under the update path", func(t *testing.T) { + // Update the setter marker in the repo. + policyKey := types.NamespacedName{ + Name: imagePolicyName, + Namespace: namespace, + } + commitInRepo(g, repoURL, branch, "Install setter marker", func(tmp string) { + g.Expect(replaceMarker(path.Join(tmp, "yes"), policyKey)).To(Succeed()) + }) + commitInRepo(g, repoURL, branch, "Install setter marker", func(tmp string) { + g.Expect(replaceMarker(path.Join(tmp, "no"), policyKey)).To(Succeed()) + }) - if tt.sign { - // configure OpenPGP armor encoder - b := bytes.NewBuffer(nil) - w, err := armor.Encode(b, openpgp.PublicKeyType, nil) - g.Expect(err).ToNot(HaveOccurred()) + // Pull the head commit that was just pushed, so it's not considered a new + // commit when checking for a commit made by automation. + waitForNewHead(g, localRepo, branch) - // serialize public key - err = pgpEntity.Serialize(w) - g.Expect(err).ToNot(HaveOccurred()) - err = w.Close() - g.Expect(err).ToNot(HaveOccurred()) + // Create the automation object and let it make a commit itself. + updateStrategy := &imagev1.UpdateStrategy{ + Strategy: imagev1.UpdateStrategySetters, + Path: "./yes", + } + err = createImageUpdateAutomation("update-test", namespace, gitRepoName, branch, "", testCommitTemplate, "", updateStrategy) + g.Expect(err).ToNot(HaveOccurred()) - // verify commit - ent, err := commit.Verify(b.String()) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(ent.PrimaryKey.Fingerprint).To(Equal(pgpEntity.PrimaryKey.Fingerprint)) - } + // Wait for a new commit to be made by the controller. + waitForNewHead(g, localRepo, branch) + + head, _ := localRepo.Head() + commit, err := localRepo.CommitObject(head.Hash()) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(commit.Message).ToNot(ContainSubstring("update-no")) + g.Expect(commit.Message).To(ContainSubstring("update-yes")) + }) +} + +func TestImageUpdateAutomation_signed_commit(t *testing.T) { + g := NewWithT(t) + + namespace := "image-auto-test-" + randStringRunes(5) + branch := randStringRunes(8) + repositoryPath := "/config-" + randStringRunes(6) + ".git" + gitRepoName := "image-auto-" + randStringRunes(5) + imagePolicyName := "policy-" + randStringRunes(5) + signingKeySecretName := "signing-key-secret-" + randStringRunes(5) + + // Create test git server. + gitServer, err := setupGitTestServer() + g.Expect(err).ToNot(HaveOccurred(), "failed to create test git server") + defer os.RemoveAll(gitServer.Root()) + defer gitServer.StopHTTP() + + // Create test namespace. + nsCleanup, err := createNamespace(namespace) + g.Expect(err).ToNot(HaveOccurred(), "failed to create test namespace") + defer func() { + g.Expect(nsCleanup()).To(Succeed()) + }() + + // Create a git repo. + g.Expect(initGitRepo(gitServer, "testdata/appconfig", branch, repositoryPath)).To(Succeed()) + + // Clone the repo. + repoURL := gitServer.HTTPAddressWithCredentials() + repositoryPath + localRepo, err := cloneRepo(repoURL, branch) + g.Expect(err).ToNot(HaveOccurred(), "failed to clone git repo") + + // Create GitRepository resource for the above repo. + err = createGitRepository(gitRepoName, namespace, "", repoURL, "") + g.Expect(err).ToNot(HaveOccurred(), "failed to create GitRepository resource") + + // Create ImagePolicy with populated latest image in the status. + err = createImagePolicyWithLatestImage(imagePolicyName, namespace, "not-expected-to-exist", "1.x", "helloworld:v1.0.0") + g.Expect(err).ToNot(HaveOccurred(), "failed to create ImagePolicy resource") + + t.Run("update with signed commit", func(t *testing.T) { + // Update the setter marker in the repo. + policyKey := types.NamespacedName{ + Name: imagePolicyName, + Namespace: namespace, + } + commitInRepo(g, repoURL, branch, "Install setter marker", func(tmp string) { + g.Expect(replaceMarker(tmp, policyKey)).To(Succeed()) }) - } + + // Pull the head commit that was just pushed, so it's not considered a new + // commit when checking for a commit made by automation. + waitForNewHead(g, localRepo, branch) + + pgpEntity, err := createSigningKeyPair(signingKeySecretName, namespace) + g.Expect(err).ToNot(HaveOccurred(), "failed to create signing key pair") + + // Create the automation object and let it make a commit itself. + updateStrategy := &imagev1.UpdateStrategy{ + Strategy: imagev1.UpdateStrategySetters, + } + err = createImageUpdateAutomation("update-test", namespace, gitRepoName, branch, "", testCommitTemplate, signingKeySecretName, updateStrategy) + g.Expect(err).ToNot(HaveOccurred()) + + // Wait for a new commit to be made by the controller. + waitForNewHead(g, localRepo, branch) + + head, err := localRepo.Head() + g.Expect(err).ToNot(HaveOccurred()) + commit, err := localRepo.CommitObject(head.Hash()) + g.Expect(err).ToNot(HaveOccurred()) + + // Configure OpenPGP armor encoder. + b := bytes.NewBuffer(nil) + w, err := armor.Encode(b, openpgp.PrivateKeyType, nil) + g.Expect(err).ToNot(HaveOccurred()) + + // Serialize public key. + err = pgpEntity.Serialize(w) + g.Expect(err).ToNot(HaveOccurred()) + err = w.Close() + g.Expect(err).ToNot(HaveOccurred()) + + // Verify commit. + ent, err := commit.Verify(b.String()) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(ent.PrimaryKey.Fingerprint).To(Equal(pgpEntity.PrimaryKey.Fingerprint)) + }) } func TestImageUpdateAutomation_e2e(t *testing.T) { @@ -399,51 +379,35 @@ func TestImageUpdateAutomation_e2e(t *testing.T) { const latestImage = "helloworld:1.0.1" - ctx, cancel := context.WithTimeout(context.Background(), contextTimeout) - defer cancel() + namespace := "image-auto-test-" + randStringRunes(5) + branch := randStringRunes(8) + repositoryPath := "/config-" + randStringRunes(6) + ".git" + gitRepoName := "image-auto-" + randStringRunes(5) + gitSecretName := "git-secret-" + randStringRunes(5) + imagePolicyName := "policy-" + randStringRunes(5) + updateStrategy := &imagev1.UpdateStrategy{ + Strategy: imagev1.UpdateStrategySetters, + } // Create a test namespace. - namespace := &corev1.Namespace{} - namespace.Name = "image-auto-test-" + randStringRunes(5) - g.Expect(testEnv.Create(ctx, namespace)).To(Succeed()) + nsCleanup, err := createNamespace(namespace) + g.Expect(err).ToNot(HaveOccurred(), "failed to create test namespace") defer func() { - g.Expect(testEnv.Delete(ctx, namespace)).To(Succeed()) + g.Expect(nsCleanup()).To(Succeed()) }() - branch := randStringRunes(8) - repositoryPath := "/config-" + randStringRunes(6) + ".git" - // Create git server. - gitServer, err := gittestserver.NewTempGitServer() - g.Expect(err).NotTo(HaveOccurred()) + gitServer, err := setupGitTestServer() + g.Expect(err).ToNot(HaveOccurred(), "failed to create test git server") defer os.RemoveAll(gitServer.Root()) - - username := randStringRunes(5) - password := randStringRunes(5) - // Using authentication makes using the server more fiddly in - // general, but is required for testing SSH. - gitServer.Auth(username, password) - gitServer.AutoCreate() - - g.Expect(gitServer.StartHTTP()).To(Succeed()) defer gitServer.StopHTTP() - gitServer.KeyDir(filepath.Join(gitServer.Root(), "keys")) - g.Expect(gitServer.ListenSSH()).To(Succeed()) - - var repoURL string cloneLocalRepoURL := gitServer.HTTPAddressWithCredentials() + repositoryPath + repoURL, err := getRepoURL(gitServer, repositoryPath, tt.proto) + g.Expect(err).ToNot(HaveOccurred()) - if tt.proto == "http" { - repoURL = cloneLocalRepoURL // NB not testing auth for git over HTTP - } else if tt.proto == "ssh" { - sshURL := gitServer.SSHAddress() - // this is expected to use 127.0.0.1, but host key - // checking usually wants a hostname, so use - // "localhost". - sshURL = strings.Replace(sshURL, "127.0.0.1", "localhost", 1) - repoURL = sshURL + repositoryPath - + // Start the ssh server if needed. + if tt.proto == "ssh" { // NOTE: Check how this is done in source-controller. go func() { gitServer.StartSSH() @@ -451,8 +415,6 @@ func TestImageUpdateAutomation_e2e(t *testing.T) { defer func() { g.Expect(gitServer.StopSSH()).To(Succeed()) }() - } else { - t.Fatal("proto not set to http or ssh") } commitMessage := "Commit a difference " + randStringRunes(5) @@ -460,294 +422,218 @@ func TestImageUpdateAutomation_e2e(t *testing.T) { // Initialize a git repo. g.Expect(initGitRepo(gitServer, "testdata/appconfig", branch, repositoryPath)).To(Succeed()) - localRepo, err := git.Clone(memory.NewStorage(), memfs.New(), &git.CloneOptions{ - URL: cloneLocalRepoURL, - RemoteName: "origin", - ReferenceName: plumbing.NewBranchReferenceName(branch), - }) - g.Expect(err).ToNot(HaveOccurred()) - - // Create git repo resource for the above repo. - gitRepoKey := types.NamespacedName{ - Name: "image-auto-" + randStringRunes(5), - Namespace: namespace.Name, - } - gitRepo := &sourcev1.GitRepository{ - Spec: sourcev1.GitRepositorySpec{ - URL: repoURL, - Interval: metav1.Duration{Duration: time.Minute}, - GitImplementation: tt.impl, - }, - } - gitRepo.Name = gitRepoKey.Name - gitRepo.Namespace = namespace.Name + // Clone the repo locally. + localRepo, err := cloneRepo(cloneLocalRepoURL, branch) + g.Expect(err).ToNot(HaveOccurred(), "failed to clone git repo") - // If using SSH, we need to provide an identity (private - // key) and known_hosts file in a secret. + // Create GitRepository resource for the above repo. if tt.proto == "ssh" { - url, err := url.Parse(repoURL) + // SSH requires an identity (private key) and known_hosts file + // in a secret. + err = createSSHIdentitySecret(gitSecretName, namespace, repoURL) g.Expect(err).ToNot(HaveOccurred()) - knownhosts, err := ssh.ScanHostKey(url.Host, 5*time.Second) + err = createGitRepository(gitRepoName, namespace, tt.impl, repoURL, gitSecretName) g.Expect(err).ToNot(HaveOccurred()) - keygen := ssh.NewRSAGenerator(2048) - pair, err := keygen.Generate() + } else { + err = createGitRepository(gitRepoName, namespace, tt.impl, repoURL, "") g.Expect(err).ToNot(HaveOccurred()) - - sec := &corev1.Secret{ - StringData: map[string]string{ - "known_hosts": string(knownhosts), - "identity": string(pair.PrivateKey), - "identity.pub": string(pair.PublicKey), - }, - } - sec.Name = "git-secret-" + randStringRunes(5) - sec.Namespace = namespace.Name - g.Expect(testEnv.Create(ctx, sec)).To(Succeed()) - gitRepo.Spec.SecretRef = &meta.LocalObjectReference{Name: sec.Name} } - g.Expect(testEnv.Create(ctx, gitRepo)).To(Succeed()) - // Create an image policy. policyKey := types.NamespacedName{ - Name: "policy-" + randStringRunes(5), - Namespace: namespace.Name, + Name: imagePolicyName, + Namespace: namespace, } // NB not testing the image reflector controller; this // will make a "fully formed" ImagePolicy object. - policy := &imagev1_reflect.ImagePolicy{ - Spec: imagev1_reflect.ImagePolicySpec{ - ImageRepositoryRef: meta.NamespacedObjectReference{ - Name: "not-expected-to-exist", - }, - Policy: imagev1_reflect.ImagePolicyChoice{ - SemVer: &imagev1_reflect.SemVerPolicy{ - Range: "1.x", - }, - }, - }, - } - policy.Name = policyKey.Name - policy.Namespace = policyKey.Namespace - g.Expect(testEnv.Create(ctx, policy)).To(Succeed()) - policy.Status.LatestImage = latestImage - g.Expect(testEnv.Status().Update(ctx, policy)).To(Succeed()) - defer func() { - g.Expect(testEnv.Delete(ctx, policy)).To(Succeed()) - }() - - // --- update with PushSpec - - commitInRepo(g, cloneLocalRepoURL, branch, "Install setter marker", func(tmp string) { - g.Expect(replaceMarker(tmp, policyKey)).To(Succeed()) - }) - - // Pull the head commit we just pushed, so it's not - // considered a new commit when checking for a commit - // made by automation. - waitForNewHead(g, localRepo, branch) - - pushBranch := "pr-" + randStringRunes(5) + err = createImagePolicyWithLatestImage(imagePolicyName, namespace, "not-expected-to-exist", "1.x", latestImage) + g.Expect(err).ToNot(HaveOccurred(), "failed to create ImagePolicy resource") + + // Create ImageUpdateAutomation resource for each of the test cases + // and cleanup at the end. + + t.Run("PushSpec", func(t *testing.T) { + imageUpdateAutomationName := "update-" + randStringRunes(5) + pushBranch := "pr-" + randStringRunes(5) + + t.Run("update with PushSpec", func(t *testing.T) { + commitInRepo(g, cloneLocalRepoURL, branch, "Install setter marker", func(tmp string) { + g.Expect(replaceMarker(tmp, policyKey)).To(Succeed()) + }) + // Pull the head commit we just pushed, so it's not + // considered a new commit when checking for a commit + // made by automation. + waitForNewHead(g, localRepo, branch) + + // Now create the automation object, and let it (one + // hopes!) make a commit itself. + err = createImageUpdateAutomation(imageUpdateAutomationName, namespace, gitRepoName, branch, pushBranch, commitMessage, "", updateStrategy) + g.Expect(err).ToNot(HaveOccurred()) + + // Wait for a new commit to be made by the controller. + waitForNewHead(g, localRepo, pushBranch) + + head, err := localRepo.Reference(plumbing.NewRemoteReferenceName(originRemote, pushBranch), true) + g.Expect(err).NotTo(HaveOccurred()) + commit, err := localRepo.CommitObject(head.Hash()) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(commit.Message).To(Equal(commitMessage)) + }) - // Now create the automation object, and let it (one - // hopes!) make a commit itself. - updateKey := types.NamespacedName{ - Namespace: namespace.Name, - Name: "update-" + randStringRunes(5), - } - updateBySetters := &imagev1.ImageUpdateAutomation{ - Spec: imagev1.ImageUpdateAutomationSpec{ - Interval: metav1.Duration{Duration: 2 * time.Hour}, // this is to ensure any subsequent run should be outside the scope of the testing - SourceRef: imagev1.SourceReference{ - Kind: "GitRepository", - Name: gitRepoKey.Name, - }, - GitSpec: &imagev1.GitSpec{ - Checkout: &imagev1.GitCheckoutSpec{ - Reference: sourcev1.GitRepositoryRef{ - Branch: branch, - }, - }, - Commit: imagev1.CommitSpec{ - MessageTemplate: commitMessage, - Author: imagev1.CommitUser{ - Email: "fluxbot@example.com", - }, - }, - Push: &imagev1.PushSpec{ - Branch: pushBranch, - }, - }, - Update: &imagev1.UpdateStrategy{ - Strategy: imagev1.UpdateStrategySetters, - }, - }, - } - updateBySetters.Name = updateKey.Name - updateBySetters.Namespace = updateKey.Namespace - g.Expect(testEnv.Create(ctx, updateBySetters)).To(Succeed()) + t.Run("push branch gets updated", func(t *testing.T) { + // Get the head hash before update. + head, err := localRepo.Reference(plumbing.NewRemoteReferenceName(originRemote, pushBranch), true) + headHash := head.String() + g.Expect(err).NotTo(HaveOccurred()) + + // Update the policy and expect another commit in the push + // branch. + err = updateImagePolicyWithLatestImage(imagePolicyName, namespace, "helloworld:v1.3.0") + g.Expect(err).ToNot(HaveOccurred()) + waitForNewHead(g, localRepo, pushBranch) + head, err = localRepo.Reference(plumbing.NewRemoteReferenceName(originRemote, pushBranch), true) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(head.String()).NotTo(Equal(headHash)) + }) - // -- Creates and pushes the push branch. + t.Run("still pushes to the push branch after it's merged", func(t *testing.T) { + // Get the head hash before. + head, err := localRepo.Reference(plumbing.NewRemoteReferenceName(originRemote, pushBranch), true) + headHash := head.String() + g.Expect(err).NotTo(HaveOccurred()) + + // Merge the push branch into checkout branch, and push the merge commit + // upstream. + // waitForNewHead() leaves the repo at the head of the branch given, i.e., the + // push branch), so we have to check out the "main" branch first. + g.Expect(checkoutBranch(localRepo, branch)).To(Succeed()) + mergeBranchIntoHead(g, localRepo, pushBranch) + + // Update the policy and expect another commit in the push + // branch. + err = updateImagePolicyWithLatestImage(imagePolicyName, namespace, "helloworld:v1.3.1") + g.Expect(err).ToNot(HaveOccurred()) + + waitForNewHead(g, localRepo, pushBranch) + + head, err = localRepo.Reference(plumbing.NewRemoteReferenceName(originRemote, pushBranch), true) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(head.String()).NotTo(Equal(headHash)) + }) - // Wait for a new commit to be made by the controller. - waitForNewHead(g, localRepo, pushBranch) - head, err := localRepo.Reference(plumbing.NewRemoteReferenceName(originRemote, pushBranch), true) - g.Expect(err).NotTo(HaveOccurred()) - commit, err := localRepo.CommitObject(head.Hash()) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(commit.Message).To(Equal(commitMessage)) - - // -- Pushes another commit to the existing push branch. - - head, err = localRepo.Reference(plumbing.NewRemoteReferenceName(originRemote, pushBranch), true) - headHash := head.String() - g.Expect(err).NotTo(HaveOccurred()) - - // Update the policy and expect another commit in the push branch. - policy.Status.LatestImage = "helloworld:v1.3.0" - g.Expect(testEnv.Status().Update(ctx, policy)).To(Succeed()) - waitForNewHead(g, localRepo, pushBranch) - head, err = localRepo.Reference(plumbing.NewRemoteReferenceName(originRemote, pushBranch), true) - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(head.String()).NotTo(Equal(headHash)) - - // -- Still pushes to the push branch after it's merged. - - head, err = localRepo.Reference(plumbing.NewRemoteReferenceName(originRemote, pushBranch), true) - headHash = head.String() - g.Expect(err).NotTo(HaveOccurred()) - - // Merge the push branch into checkout branch, and push the merge commit - // upstream. - // waitForNewHead() leaves the repo at the head of the branch given, i.e., the - // push branch), so we have to check out the "main" branch first. - g.Expect(checkoutBranch(localRepo, branch)).To(Succeed()) - mergeBranchIntoHead(g, localRepo, pushBranch) - - // Update the policy and expect another commit in the push branch - policy.Status.LatestImage = "helloworld:v1.3.1" - g.Expect(testEnv.Status().Update(ctx, policy)).To(Succeed()) - waitForNewHead(g, localRepo, pushBranch) - head, err = localRepo.Reference(plumbing.NewRemoteReferenceName(originRemote, pushBranch), true) - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(head.String()).NotTo(Equal(headHash)) - - // Cleanup the image update automation used above. - g.Expect(testEnv.Delete(ctx, updateBySetters)).To(Succeed()) - - // --- with update strategy setters - - // Insert a setter reference into the deployment file, - // before creating the automation object itself. - commitInRepo(g, cloneLocalRepoURL, branch, "Install setter marker", func(tmp string) { - g.Expect(replaceMarker(tmp, policyKey)).To(Succeed()) + // Cleanup the image update automation used above. + g.Expect(deleteImageUpdateAutomation(imageUpdateAutomationName, namespace)).To(Succeed()) }) - // Pull the head commit we just pushed, so it's not - // considered a new commit when checking for a commit - // made by automation. - waitForNewHead(g, localRepo, branch) - - // Now create the automation object, and let it (one - // hopes!) make a commit itself. - updateKey = types.NamespacedName{ - Namespace: gitRepoKey.Namespace, - Name: "update-" + randStringRunes(5), - } - updateBySetters = &imagev1.ImageUpdateAutomation{ - Spec: imagev1.ImageUpdateAutomationSpec{ - Interval: metav1.Duration{Duration: 2 * time.Hour}, // this is to ensure any subsequent run should be outside the scope of the testing - SourceRef: imagev1.SourceReference{ - Kind: "GitRepository", - Name: gitRepoKey.Name, - }, - Update: &imagev1.UpdateStrategy{ - Strategy: imagev1.UpdateStrategySetters, - }, - GitSpec: &imagev1.GitSpec{ - Checkout: &imagev1.GitCheckoutSpec{ - Reference: sourcev1.GitRepositoryRef{ - Branch: branch, - }, - }, - Commit: imagev1.CommitSpec{ - Author: imagev1.CommitUser{ - Email: "fluxbot@example.com", - }, - MessageTemplate: commitMessage, - }, - }, - }, - } - updateBySetters.Name = updateKey.Name - updateBySetters.Namespace = updateKey.Namespace - g.Expect(testEnv.Create(context.Background(), updateBySetters)).To(Succeed()) - // wait for a new commit to be made by the controller - waitForNewHead(g, localRepo, branch) - - // -- Update to the most recent image. - - head, _ = localRepo.Head() - commit, err = localRepo.CommitObject(head.Hash()) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(commit.Message).To(Equal(commitMessage)) + t.Run("with update strategy setters", func(t *testing.T) { + // Insert a setter reference into the deployment file, + // before creating the automation object itself. + commitInRepo(g, cloneLocalRepoURL, branch, "Install setter marker", func(tmp string) { + g.Expect(replaceMarker(tmp, policyKey)).To(Succeed()) + }) - var newObj imagev1.ImageUpdateAutomation - g.Expect(testEnv.Get(ctx, updateKey, &newObj)).To(Succeed()) - g.Expect(newObj.Status.LastPushCommit).To(Equal(head.Hash().String())) - g.Expect(newObj.Status.LastPushTime).ToNot(BeNil()) + // Pull the head commit we just pushed, so it's not + // considered a new commit when checking for a commit + // made by automation. + waitForNewHead(g, localRepo, branch) - compareRepoWithExpected(g, cloneLocalRepoURL, branch, "testdata/appconfig-setters-expected", func(tmp string) { - g.Expect(replaceMarker(tmp, policyKey)).To(Succeed()) - }) + // Now create the automation object, and let it (one + // hopes!) make a commit itself. + updateKey := types.NamespacedName{ + Namespace: namespace, + Name: "update-" + randStringRunes(5), + } + err = createImageUpdateAutomation(updateKey.Name, namespace, gitRepoName, branch, "", commitMessage, "", updateStrategy) + g.Expect(err).ToNot(HaveOccurred()) + defer func() { + g.Expect(deleteImageUpdateAutomation(updateKey.Name, namespace)).To(Succeed()) + }() - // -- Suspend the update object - reconciliation should not run. + // Wait for a new commit to be made by the controller. + waitForNewHead(g, localRepo, branch) - var updatePatch imagev1.ImageUpdateAutomation - g.Expect(testEnv.Get(context.TODO(), updateKey, &updatePatch)).To(Succeed()) - updatePatch.Spec.Suspend = true - g.Expect(testEnv.Patch(context.Background(), &updatePatch, client.Merge)).To(Succeed()) + // Check if the repo head matches with the ImageUpdateAutomation + // last push commit status. + head, err := localRepo.Head() + g.Expect(err).ToNot(HaveOccurred()) - // Create a new image automation reconciler and run it explicitly. - imageAutoReconciler := &ImageUpdateAutomationReconciler{ - Client: testEnv, - Scheme: scheme.Scheme, - } + var newObj imagev1.ImageUpdateAutomation + g.Expect(testEnv.Get(context.Background(), updateKey, &newObj)).To(Succeed()) + g.Expect(newObj.Status.LastPushCommit).To(Equal(head.Hash().String())) + g.Expect(newObj.Status.LastPushTime).ToNot(BeNil()) - // Wait for the suspension to reach the cache - var newUpdate imagev1.ImageUpdateAutomation - g.Eventually(func() bool { - if err := imageAutoReconciler.Get(context.Background(), updateKey, &newUpdate); err != nil { - return false - } - return newUpdate.Spec.Suspend - }, timeout, time.Second).Should(BeTrue()) - // Run the reconciliation explicitly, and make sure it - // doesn't do anything - result, err := imageAutoReconciler.Reconcile(logr.NewContext(context.TODO(), ctrl.Log), ctrl.Request{ - NamespacedName: updateKey, + compareRepoWithExpected(g, cloneLocalRepoURL, branch, "testdata/appconfig-setters-expected", func(tmp string) { + g.Expect(replaceMarker(tmp, policyKey)).To(Succeed()) + }) }) - g.Expect(err).To(BeNil()) - // This ought to fail if suspend is not working, since the item would be requeued; - // but if not, additional checks lie below. - g.Expect(result).To(Equal(ctrl.Result{})) - var checkUpdate imagev1.ImageUpdateAutomation - g.Expect(testEnv.Get(context.Background(), updateKey, &checkUpdate)).To(Succeed()) - g.Expect(checkUpdate.Status.ObservedGeneration).NotTo(Equal(checkUpdate.ObjectMeta.Generation)) + t.Run("no reconciliation when object is suspended", func(t *testing.T) { + // Create the automation object. + updateKey := types.NamespacedName{ + Namespace: namespace, + Name: "update-" + randStringRunes(5), + } + err = createImageUpdateAutomation(updateKey.Name, namespace, gitRepoName, branch, "", commitMessage, "", updateStrategy) + g.Expect(err).ToNot(HaveOccurred()) + defer func() { + g.Expect(deleteImageUpdateAutomation(updateKey.Name, namespace)).To(Succeed()) + }() - // -- Reconciles when reconcile request annotation is added. + // Wait for the object to be available in the cache before + // attempting update. + g.Eventually(func() bool { + obj := &imagev1.ImageUpdateAutomation{} + if err := testEnv.Get(context.Background(), updateKey, obj); err != nil { + return false + } + return true + }, timeout, time.Second).Should(BeTrue()) + + // Suspend the automation object. + var updatePatch imagev1.ImageUpdateAutomation + g.Expect(testEnv.Get(context.TODO(), updateKey, &updatePatch)).To(Succeed()) + updatePatch.Spec.Suspend = true + g.Expect(testEnv.Patch(context.Background(), &updatePatch, client.Merge)).To(Succeed()) + + // Create a new image automation reconciler and run it + // explicitly. + imageAutoReconciler := &ImageUpdateAutomationReconciler{ + Client: testEnv, + Scheme: scheme.Scheme, + } - // The automation has run, and is not expected to run - // again for 2 hours. Make a commit to the git repo - // which needs to be undone by automation, then add - // the annotation and make sure it runs again. + // Wait for the suspension to reach the cache + var newUpdate imagev1.ImageUpdateAutomation + g.Eventually(func() bool { + if err := imageAutoReconciler.Get(context.Background(), updateKey, &newUpdate); err != nil { + return false + } + return newUpdate.Spec.Suspend + }, timeout, time.Second).Should(BeTrue()) + // Run the reconciliation explicitly, and make sure it + // doesn't do anything + result, err := imageAutoReconciler.Reconcile(logr.NewContext(context.TODO(), ctrl.Log), ctrl.Request{ + NamespacedName: updateKey, + }) + g.Expect(err).To(BeNil()) + // This ought to fail if suspend is not working, since the item would be requeued; + // but if not, additional checks lie below. + g.Expect(result).To(Equal(ctrl.Result{})) + + var checkUpdate imagev1.ImageUpdateAutomation + g.Expect(testEnv.Get(context.Background(), updateKey, &checkUpdate)).To(Succeed()) + g.Expect(checkUpdate.Status.ObservedGeneration).NotTo(Equal(checkUpdate.ObjectMeta.Generation)) + }) - // TODO: Implement adding request annotation. - // Refer: https://github.com/fluxcd/image-automation-controller/pull/82/commits/4fde199362b42fa37068f2e6c6885cfea474a3d1#diff-1168fadffa18bd096582ae7f8b6db744fd896bd5600ee1d1ac6ac4474af251b9L292-L334 + t.Run("reconciles with reconcile request annotation", func(t *testing.T) { + // The automation has run, and is not expected to run + // again for 2 hours. Make a commit to the git repo + // which needs to be undone by automation, then add + // the annotation and make sure it runs again. - g.Expect(testEnv.Get(context.Background(), updateKey, updateBySetters)).To(Succeed()) - g.Expect(updateBySetters.Status.LastAutomationRunTime).ToNot(BeNil()) + // TODO: Implement adding request annotation. + // Refer: https://github.com/fluxcd/image-automation-controller/pull/82/commits/4fde199362b42fa37068f2e6c6885cfea474a3d1#diff-1168fadffa18bd096582ae7f8b6db744fd896bd5600ee1d1ac6ac4474af251b9L292-L334 + }) }) } } @@ -1022,3 +908,230 @@ func mergeBranchIntoHead(g *WithT, repo *git.Repository, pushBranch string) { }) g.Expect(err).NotTo(HaveOccurred()) } + +// setupGitTestServer creates and returns a git test server. The caller must +// ensure it's stopped and cleaned up. +func setupGitTestServer() (*gittestserver.GitServer, error) { + gitServer, err := gittestserver.NewTempGitServer() + if err != nil { + return nil, err + } + username := randStringRunes(5) + password := randStringRunes(5) + // Using authentication makes using the server more fiddly in + // general, but is required for testing SSH. + gitServer.Auth(username, password) + gitServer.AutoCreate() + if err := gitServer.StartHTTP(); err != nil { + return nil, err + } + gitServer.KeyDir(filepath.Join(gitServer.Root(), "keys")) + if err := gitServer.ListenSSH(); err != nil { + return nil, err + } + return gitServer, nil +} + +// cleanup is used to return closures for cleaning up. +type cleanup func() error + +// createNamespace creates a namespace and returns a closure for deleting the +// namespace. +func createNamespace(name string) (cleanup, error) { + namespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + } + if err := testEnv.Create(context.Background(), namespace); err != nil { + return nil, err + } + cleanup := func() error { + return testEnv.Delete(context.Background(), namespace) + } + return cleanup, nil +} + +func cloneRepo(repoURL, branch string) (*git.Repository, error) { + return git.Clone(memory.NewStorage(), memfs.New(), &git.CloneOptions{ + URL: repoURL, + RemoteName: "origin", + ReferenceName: plumbing.NewBranchReferenceName(branch), + }) +} + +func createGitRepository(name, namespace, impl, repoURL, secretRef string) error { + gitRepo := &sourcev1.GitRepository{ + Spec: sourcev1.GitRepositorySpec{ + URL: repoURL, + Interval: metav1.Duration{Duration: time.Minute}, + }, + } + gitRepo.Name = name + gitRepo.Namespace = namespace + if secretRef != "" { + gitRepo.Spec.SecretRef = &meta.LocalObjectReference{Name: secretRef} + } + if impl != "" { + gitRepo.Spec.GitImplementation = impl + } + return testEnv.Create(context.Background(), gitRepo) +} + +func createImagePolicyWithLatestImage(name, namespace, repoRef, semverRange, latest string) error { + policy := &imagev1_reflect.ImagePolicy{ + Spec: imagev1_reflect.ImagePolicySpec{ + ImageRepositoryRef: meta.NamespacedObjectReference{ + Name: repoRef, + }, + Policy: imagev1_reflect.ImagePolicyChoice{ + SemVer: &imagev1_reflect.SemVerPolicy{ + Range: semverRange, + }, + }, + }, + } + policy.Name = name + policy.Namespace = namespace + err := testEnv.Create(context.Background(), policy) + if err != nil { + return err + } + policy.Status.LatestImage = latest + return testEnv.Status().Update(context.Background(), policy) +} + +func updateImagePolicyWithLatestImage(name, namespace, latest string) error { + policy := &imagev1_reflect.ImagePolicy{} + key := types.NamespacedName{ + Name: name, + Namespace: namespace, + } + if err := testEnv.Get(context.Background(), key, policy); err != nil { + return err + } + policy.Status.LatestImage = latest + return testEnv.Status().Update(context.Background(), policy) +} + +func createImageUpdateAutomation( + name, namespace, gitRepo, checkoutBranch, pushBranch, commitTemplate, signingKeyRef string, + updateStrategy *imagev1.UpdateStrategy) error { + updateAutomation := &imagev1.ImageUpdateAutomation{ + Spec: imagev1.ImageUpdateAutomationSpec{ + Interval: metav1.Duration{Duration: 2 * time.Hour}, // This is to ensure any subsequent run should be outside the scope of the testing. + SourceRef: imagev1.SourceReference{ + Kind: "GitRepository", + Name: gitRepo, + }, + GitSpec: &imagev1.GitSpec{ + Checkout: &imagev1.GitCheckoutSpec{ + Reference: sourcev1.GitRepositoryRef{ + Branch: checkoutBranch, + }, + }, + Commit: imagev1.CommitSpec{ + MessageTemplate: commitTemplate, + Author: imagev1.CommitUser{ + Name: testAuthorName, + Email: testAuthorEmail, + }, + }, + }, + Update: updateStrategy, + }, + } + updateAutomation.Name = name + updateAutomation.Namespace = namespace + if pushBranch != "" { + updateAutomation.Spec.GitSpec.Push = &imagev1.PushSpec{ + Branch: pushBranch, + } + } + if signingKeyRef != "" { + updateAutomation.Spec.GitSpec.Commit.SigningKey = &imagev1.SigningKey{ + SecretRef: meta.LocalObjectReference{Name: signingKeyRef}, + } + } + return testEnv.Create(context.Background(), updateAutomation) +} + +func deleteImageUpdateAutomation(name, namespace string) error { + update := &imagev1.ImageUpdateAutomation{} + update.Name = name + update.Namespace = namespace + return testEnv.Delete(context.Background(), update) +} + +func createSigningKeyPair(name, namespace string) (*openpgp.Entity, error) { + pgpEntity, err := openpgp.NewEntity("", "", "", nil) + if err != nil { + return nil, err + } + // Configure OpenPGP armor encoder. + b := bytes.NewBuffer(nil) + w, err := armor.Encode(b, openpgp.PrivateKeyType, nil) + if err != nil { + return nil, err + } + // Serialize private key. + if err := pgpEntity.SerializePrivate(w, nil); err != nil { + return nil, err + } + if err = w.Close(); err != nil { + return nil, err + } + // Create the secret containing signing key. + sec := &corev1.Secret{ + Data: map[string][]byte{ + "git.asc": b.Bytes(), + }, + } + sec.Name = name + sec.Namespace = namespace + if err := testEnv.Create(ctx, sec); err != nil { + return nil, err + } + return pgpEntity, nil +} + +func createSSHIdentitySecret(name, namespace, repoURL string) error { + url, err := url.Parse(repoURL) + if err != nil { + return err + } + knownhosts, err := ssh.ScanHostKey(url.Host, 5*time.Second) + if err != nil { + return err + } + keygen := ssh.NewRSAGenerator(2048) + pair, err := keygen.Generate() + if err != nil { + return err + } + sec := &corev1.Secret{ + StringData: map[string]string{ + "known_hosts": string(knownhosts), + "identity": string(pair.PrivateKey), + "identity.pub": string(pair.PublicKey), + }, + } + sec.Name = name + sec.Namespace = namespace + return testEnv.Create(ctx, sec) +} + +func getRepoURL(gitServer *gittestserver.GitServer, repoPath, proto string) (string, error) { + if proto == "http" { + return gitServer.HTTPAddressWithCredentials() + repoPath, nil + } else if proto == "ssh" { + return getSSHRepoURL(gitServer.SSHAddress(), repoPath), nil + } + return "", fmt.Errorf("proto not set to http or ssh") +} + +func getSSHRepoURL(sshAddress, repoPath string) string { + // This is expected to use 127.0.0.1, but host key + // checking usually wants a hostname, so use + // "localhost". + sshURL := strings.Replace(sshAddress, "127.0.0.1", "localhost", 1) + return sshURL + repoPath +} From 5ae1d28c0ab7ebc27ca9530ce870dd71db7aa770 Mon Sep 17 00:00:00 2001 From: Sunny Date: Wed, 3 Nov 2021 00:59:17 +0530 Subject: [PATCH 08/11] Add testWithRepoAndImagePolicy() in update_test.go testWithRepoAndImagePolicy() contains common code to create a git server, git repository and ImagePolicy for the test setup. Also updates some test structure slightly. Signed-off-by: Sunny --- controllers/update_test.go | 472 ++++++++++++++++++------------------- 1 file changed, 223 insertions(+), 249 deletions(-) diff --git a/controllers/update_test.go b/controllers/update_test.go index 4618b224..19c21cb7 100644 --- a/controllers/update_test.go +++ b/controllers/update_test.go @@ -109,272 +109,182 @@ Images: ) func TestImageUpdateAutomation_commit_message(t *testing.T) { - g := NewWithT(t) - - namespace := "image-auto-test-" + randStringRunes(5) - branch := randStringRunes(8) - repositoryPath := "/config-" + randStringRunes(6) + ".git" - gitRepoName := "image-auto-" + randStringRunes(5) - imagePolicyName := "policy-" + randStringRunes(5) - - // Create test git server. - gitServer, err := setupGitTestServer() - g.Expect(err).ToNot(HaveOccurred(), "failed to create test git server") - defer os.RemoveAll(gitServer.Root()) - defer gitServer.StopHTTP() - - // Create test namespace. - nsCleanup, err := createNamespace(namespace) - g.Expect(err).ToNot(HaveOccurred(), "failed to create test namespace") - defer func() { - g.Expect(nsCleanup()).To(Succeed()) - }() + policySpec := imagev1_reflect.ImagePolicySpec{ + ImageRepositoryRef: meta.NamespacedObjectReference{ + Name: "not-expected-to-exist", + }, + Policy: imagev1_reflect.ImagePolicyChoice{ + SemVer: &imagev1_reflect.SemVerPolicy{ + Range: "1.x", + }, + }, + } + fixture := "testdata/appconfig" + latest := "helloworld:v1.0.0" - // Create a git repo. - g.Expect(initGitRepo(gitServer, "testdata/appconfig", branch, repositoryPath)).To(Succeed()) + testWithRepoAndImagePolicy( + NewWithT(t), fixture, policySpec, latest, + func(g *WithT, namespace, repoURL, gitRepoName, branch, imagePolicyName string, localRepo *git.Repository) { + commitMessage := fmt.Sprintf(testCommitMessageFmt, namespace, imagePolicyName) - // Clone the repo. - repoURL := gitServer.HTTPAddressWithCredentials() + repositoryPath - localRepo, err := cloneRepo(repoURL, branch) - g.Expect(err).ToNot(HaveOccurred(), "failed to clone git repo") + // Update the setter marker in the repo. + policyKey := types.NamespacedName{ + Name: imagePolicyName, + Namespace: namespace, + } + commitInRepo(g, repoURL, branch, "Install setter marker", func(tmp string) { + g.Expect(replaceMarker(tmp, policyKey)).To(Succeed()) + }) - // Create GitRepository resource for the above repo. - err = createGitRepository(gitRepoName, namespace, "", repoURL, "") - g.Expect(err).ToNot(HaveOccurred(), "failed to create GitRepository resource") + // Pull the head commit that was just pushed, so it's not considered a new + // commit when checking for a commit made by automation. + waitForNewHead(g, localRepo, branch) - // Create ImagePolicy with populated latest image in the status. - err = createImagePolicyWithLatestImage(imagePolicyName, namespace, "not-expected-to-exist", "1.x", "helloworld:v1.0.0") - g.Expect(err).ToNot(HaveOccurred(), "failed to create ImagePolicy resource") + // Create the automation object and let it make a commit itself. + updateStrategy := &imagev1.UpdateStrategy{ + Strategy: imagev1.UpdateStrategySetters, + } + err := createImageUpdateAutomation("update-test", namespace, gitRepoName, branch, "", testCommitTemplate, "", updateStrategy) + g.Expect(err).ToNot(HaveOccurred()) - t.Run("update with commit template", func(t *testing.T) { - commitMessage := fmt.Sprintf(testCommitMessageFmt, namespace, imagePolicyName) + // Wait for a new commit to be made by the controller. + waitForNewHead(g, localRepo, branch) - // Update the setter marker in the repo. - policyKey := types.NamespacedName{ - Name: imagePolicyName, - Namespace: namespace, - } - commitInRepo(g, repoURL, branch, "Install setter marker", func(tmp string) { - g.Expect(replaceMarker(tmp, policyKey)).To(Succeed()) + head, _ := localRepo.Head() + commit, err := localRepo.CommitObject(head.Hash()) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(commit.Author).NotTo(BeNil()) + g.Expect(commit.Author.Name).To(Equal(testAuthorName)) + g.Expect(commit.Author.Email).To(Equal(testAuthorEmail)) + g.Expect(commit.Message).To(Equal(commitMessage)) }) - - // Pull the head commit that was just pushed, so it's not considered a new - // commit when checking for a commit made by automation. - waitForNewHead(g, localRepo, branch) - - // Create the automation object and let it make a commit itself. - updateStrategy := &imagev1.UpdateStrategy{ - Strategy: imagev1.UpdateStrategySetters, - } - err = createImageUpdateAutomation("update-test", namespace, gitRepoName, branch, "", testCommitTemplate, "", updateStrategy) - g.Expect(err).ToNot(HaveOccurred()) - - // Wait for a new commit to be made by the controller. - waitForNewHead(g, localRepo, branch) - - head, _ := localRepo.Head() - commit, err := localRepo.CommitObject(head.Hash()) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(commit.Author).NotTo(BeNil()) - g.Expect(commit.Author.Name).To(Equal(testAuthorName)) - g.Expect(commit.Author.Email).To(Equal(testAuthorEmail)) - g.Expect(commit.Message).To(Equal(commitMessage)) - }) } func TestImageUpdateAutomation_update_path(t *testing.T) { - g := NewWithT(t) - - namespace := "image-auto-test-" + randStringRunes(5) - branch := randStringRunes(8) - repositoryPath := "/config-" + randStringRunes(6) + ".git" - gitRepoName := "image-auto-" + randStringRunes(5) - imagePolicyName := "policy-" + randStringRunes(5) - - // Create test git server. - gitServer, err := setupGitTestServer() - g.Expect(err).ToNot(HaveOccurred(), "failed to create test git server") - defer os.RemoveAll(gitServer.Root()) - defer gitServer.StopHTTP() - - // Create test namespace. - nsCleanup, err := createNamespace(namespace) - g.Expect(err).ToNot(HaveOccurred(), "failed to create test namespace") - defer func() { - g.Expect(nsCleanup()).To(Succeed()) - }() + policySpec := imagev1_reflect.ImagePolicySpec{ + ImageRepositoryRef: meta.NamespacedObjectReference{ + Name: "not-expected-to-exist", + }, + Policy: imagev1_reflect.ImagePolicyChoice{ + SemVer: &imagev1_reflect.SemVerPolicy{ + Range: "1.x", + }, + }, + } + fixture := "testdata/pathconfig" + latest := "helloworld:v1.0.0" - // Create a git repo. - g.Expect(initGitRepo(gitServer, "testdata/pathconfig", branch, repositoryPath)).To(Succeed()) + testWithRepoAndImagePolicy( + NewWithT(t), fixture, policySpec, latest, + func(g *WithT, namespace, repoURL, gitRepoName, branch, imagePolicyName string, localRepo *git.Repository) { + // Update the setter marker in the repo. + policyKey := types.NamespacedName{ + Name: imagePolicyName, + Namespace: namespace, + } + commitInRepo(g, repoURL, branch, "Install setter marker", func(tmp string) { + g.Expect(replaceMarker(path.Join(tmp, "yes"), policyKey)).To(Succeed()) + }) + commitInRepo(g, repoURL, branch, "Install setter marker", func(tmp string) { + g.Expect(replaceMarker(path.Join(tmp, "no"), policyKey)).To(Succeed()) + }) - // Clone the repo. - repoURL := gitServer.HTTPAddressWithCredentials() + repositoryPath - localRepo, err := cloneRepo(repoURL, branch) - g.Expect(err).ToNot(HaveOccurred(), "failed to clone git repo") + // Pull the head commit that was just pushed, so it's not considered a new + // commit when checking for a commit made by automation. + waitForNewHead(g, localRepo, branch) - // Create GitRepository resource for the above repo. - err = createGitRepository(gitRepoName, namespace, "", repoURL, "") - g.Expect(err).ToNot(HaveOccurred(), "failed to create GitRepository resource") + // Create the automation object and let it make a commit itself. + updateStrategy := &imagev1.UpdateStrategy{ + Strategy: imagev1.UpdateStrategySetters, + Path: "./yes", + } + err := createImageUpdateAutomation("update-test", namespace, gitRepoName, branch, "", testCommitTemplate, "", updateStrategy) + g.Expect(err).ToNot(HaveOccurred()) - // Create ImagePolicy with populated latest image in the status. - err = createImagePolicyWithLatestImage(imagePolicyName, namespace, "not-expected-to-exist", "1.x", "helloworld:v1.0.0") - g.Expect(err).ToNot(HaveOccurred(), "failed to create ImagePolicy resource") + // Wait for a new commit to be made by the controller. + waitForNewHead(g, localRepo, branch) - t.Run("update only under the update path", func(t *testing.T) { - // Update the setter marker in the repo. - policyKey := types.NamespacedName{ - Name: imagePolicyName, - Namespace: namespace, - } - commitInRepo(g, repoURL, branch, "Install setter marker", func(tmp string) { - g.Expect(replaceMarker(path.Join(tmp, "yes"), policyKey)).To(Succeed()) - }) - commitInRepo(g, repoURL, branch, "Install setter marker", func(tmp string) { - g.Expect(replaceMarker(path.Join(tmp, "no"), policyKey)).To(Succeed()) + head, _ := localRepo.Head() + commit, err := localRepo.CommitObject(head.Hash()) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(commit.Message).ToNot(ContainSubstring("update-no")) + g.Expect(commit.Message).To(ContainSubstring("update-yes")) }) - - // Pull the head commit that was just pushed, so it's not considered a new - // commit when checking for a commit made by automation. - waitForNewHead(g, localRepo, branch) - - // Create the automation object and let it make a commit itself. - updateStrategy := &imagev1.UpdateStrategy{ - Strategy: imagev1.UpdateStrategySetters, - Path: "./yes", - } - err = createImageUpdateAutomation("update-test", namespace, gitRepoName, branch, "", testCommitTemplate, "", updateStrategy) - g.Expect(err).ToNot(HaveOccurred()) - - // Wait for a new commit to be made by the controller. - waitForNewHead(g, localRepo, branch) - - head, _ := localRepo.Head() - commit, err := localRepo.CommitObject(head.Hash()) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(commit.Message).ToNot(ContainSubstring("update-no")) - g.Expect(commit.Message).To(ContainSubstring("update-yes")) - }) } func TestImageUpdateAutomation_signed_commit(t *testing.T) { - g := NewWithT(t) + policySpec := imagev1_reflect.ImagePolicySpec{ + ImageRepositoryRef: meta.NamespacedObjectReference{ + Name: "not-expected-to-exist", + }, + Policy: imagev1_reflect.ImagePolicyChoice{ + SemVer: &imagev1_reflect.SemVerPolicy{ + Range: "1.x", + }, + }, + } + fixture := "testdata/appconfig" + latest := "helloworld:v1.0.0" + + testWithRepoAndImagePolicy( + NewWithT(t), fixture, policySpec, latest, + func(g *WithT, namespace, repoURL, gitRepoName, branch, imagePolicyName string, localRepo *git.Repository) { + signingKeySecretName := "signing-key-secret-" + randStringRunes(5) + // Update the setter marker in the repo. + policyKey := types.NamespacedName{ + Name: imagePolicyName, + Namespace: namespace, + } + commitInRepo(g, repoURL, branch, "Install setter marker", func(tmp string) { + g.Expect(replaceMarker(tmp, policyKey)).To(Succeed()) + }) - namespace := "image-auto-test-" + randStringRunes(5) - branch := randStringRunes(8) - repositoryPath := "/config-" + randStringRunes(6) + ".git" - gitRepoName := "image-auto-" + randStringRunes(5) - imagePolicyName := "policy-" + randStringRunes(5) - signingKeySecretName := "signing-key-secret-" + randStringRunes(5) + // Pull the head commit that was just pushed, so it's not considered a new + // commit when checking for a commit made by automation. + waitForNewHead(g, localRepo, branch) - // Create test git server. - gitServer, err := setupGitTestServer() - g.Expect(err).ToNot(HaveOccurred(), "failed to create test git server") - defer os.RemoveAll(gitServer.Root()) - defer gitServer.StopHTTP() + pgpEntity, err := createSigningKeyPair(signingKeySecretName, namespace) + g.Expect(err).ToNot(HaveOccurred(), "failed to create signing key pair") - // Create test namespace. - nsCleanup, err := createNamespace(namespace) - g.Expect(err).ToNot(HaveOccurred(), "failed to create test namespace") - defer func() { - g.Expect(nsCleanup()).To(Succeed()) - }() + // Create the automation object and let it make a commit itself. + updateStrategy := &imagev1.UpdateStrategy{ + Strategy: imagev1.UpdateStrategySetters, + } + err = createImageUpdateAutomation("update-test", namespace, gitRepoName, branch, "", testCommitTemplate, signingKeySecretName, updateStrategy) + g.Expect(err).ToNot(HaveOccurred()) - // Create a git repo. - g.Expect(initGitRepo(gitServer, "testdata/appconfig", branch, repositoryPath)).To(Succeed()) + // Wait for a new commit to be made by the controller. + waitForNewHead(g, localRepo, branch) - // Clone the repo. - repoURL := gitServer.HTTPAddressWithCredentials() + repositoryPath - localRepo, err := cloneRepo(repoURL, branch) - g.Expect(err).ToNot(HaveOccurred(), "failed to clone git repo") + head, err := localRepo.Head() + g.Expect(err).ToNot(HaveOccurred()) + commit, err := localRepo.CommitObject(head.Hash()) + g.Expect(err).ToNot(HaveOccurred()) - // Create GitRepository resource for the above repo. - err = createGitRepository(gitRepoName, namespace, "", repoURL, "") - g.Expect(err).ToNot(HaveOccurred(), "failed to create GitRepository resource") + // Configure OpenPGP armor encoder. + b := bytes.NewBuffer(nil) + w, err := armor.Encode(b, openpgp.PrivateKeyType, nil) + g.Expect(err).ToNot(HaveOccurred()) - // Create ImagePolicy with populated latest image in the status. - err = createImagePolicyWithLatestImage(imagePolicyName, namespace, "not-expected-to-exist", "1.x", "helloworld:v1.0.0") - g.Expect(err).ToNot(HaveOccurred(), "failed to create ImagePolicy resource") + // Serialize public key. + err = pgpEntity.Serialize(w) + g.Expect(err).ToNot(HaveOccurred()) + err = w.Close() + g.Expect(err).ToNot(HaveOccurred()) - t.Run("update with signed commit", func(t *testing.T) { - // Update the setter marker in the repo. - policyKey := types.NamespacedName{ - Name: imagePolicyName, - Namespace: namespace, - } - commitInRepo(g, repoURL, branch, "Install setter marker", func(tmp string) { - g.Expect(replaceMarker(tmp, policyKey)).To(Succeed()) + // Verify commit. + ent, err := commit.Verify(b.String()) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(ent.PrimaryKey.Fingerprint).To(Equal(pgpEntity.PrimaryKey.Fingerprint)) }) - - // Pull the head commit that was just pushed, so it's not considered a new - // commit when checking for a commit made by automation. - waitForNewHead(g, localRepo, branch) - - pgpEntity, err := createSigningKeyPair(signingKeySecretName, namespace) - g.Expect(err).ToNot(HaveOccurred(), "failed to create signing key pair") - - // Create the automation object and let it make a commit itself. - updateStrategy := &imagev1.UpdateStrategy{ - Strategy: imagev1.UpdateStrategySetters, - } - err = createImageUpdateAutomation("update-test", namespace, gitRepoName, branch, "", testCommitTemplate, signingKeySecretName, updateStrategy) - g.Expect(err).ToNot(HaveOccurred()) - - // Wait for a new commit to be made by the controller. - waitForNewHead(g, localRepo, branch) - - head, err := localRepo.Head() - g.Expect(err).ToNot(HaveOccurred()) - commit, err := localRepo.CommitObject(head.Hash()) - g.Expect(err).ToNot(HaveOccurred()) - - // Configure OpenPGP armor encoder. - b := bytes.NewBuffer(nil) - w, err := armor.Encode(b, openpgp.PrivateKeyType, nil) - g.Expect(err).ToNot(HaveOccurred()) - - // Serialize public key. - err = pgpEntity.Serialize(w) - g.Expect(err).ToNot(HaveOccurred()) - err = w.Close() - g.Expect(err).ToNot(HaveOccurred()) - - // Verify commit. - ent, err := commit.Verify(b.String()) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(ent.PrimaryKey.Fingerprint).To(Equal(pgpEntity.PrimaryKey.Fingerprint)) - }) } func TestImageUpdateAutomation_e2e(t *testing.T) { - tests := []struct { - name string - proto string - impl string - }{ - { - name: "go-git with HTTP", - proto: "http", - impl: sourcev1.GoGitImplementation, - }, - { - name: "go-git with SSH", - proto: "ssh", - impl: sourcev1.GoGitImplementation, - }, - { - name: "libgit2 with HTTP", - proto: "http", - impl: sourcev1.LibGit2Implementation, - }, - { - name: "libgit2 with SSH", - proto: "ssh", - impl: sourcev1.LibGit2Implementation, - }, - } + gitImpls := []string{sourcev1.GoGitImplementation, sourcev1.LibGit2Implementation} + protos := []string{"http", "ssh"} - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { + testFunc := func(proto string, impl string) func(t *testing.T) { + return func(t *testing.T) { g := NewWithT(t) const latestImage = "helloworld:1.0.1" @@ -403,11 +313,11 @@ func TestImageUpdateAutomation_e2e(t *testing.T) { defer gitServer.StopHTTP() cloneLocalRepoURL := gitServer.HTTPAddressWithCredentials() + repositoryPath - repoURL, err := getRepoURL(gitServer, repositoryPath, tt.proto) + repoURL, err := getRepoURL(gitServer, repositoryPath, proto) g.Expect(err).ToNot(HaveOccurred()) // Start the ssh server if needed. - if tt.proto == "ssh" { + if proto == "ssh" { // NOTE: Check how this is done in source-controller. go func() { gitServer.StartSSH() @@ -427,15 +337,15 @@ func TestImageUpdateAutomation_e2e(t *testing.T) { g.Expect(err).ToNot(HaveOccurred(), "failed to clone git repo") // Create GitRepository resource for the above repo. - if tt.proto == "ssh" { + if proto == "ssh" { // SSH requires an identity (private key) and known_hosts file // in a secret. err = createSSHIdentitySecret(gitSecretName, namespace, repoURL) g.Expect(err).ToNot(HaveOccurred()) - err = createGitRepository(gitRepoName, namespace, tt.impl, repoURL, gitSecretName) + err = createGitRepository(gitRepoName, namespace, impl, repoURL, gitSecretName) g.Expect(err).ToNot(HaveOccurred()) } else { - err = createGitRepository(gitRepoName, namespace, tt.impl, repoURL, "") + err = createGitRepository(gitRepoName, namespace, impl, repoURL, "") g.Expect(err).ToNot(HaveOccurred()) } @@ -634,7 +544,14 @@ func TestImageUpdateAutomation_e2e(t *testing.T) { // TODO: Implement adding request annotation. // Refer: https://github.com/fluxcd/image-automation-controller/pull/82/commits/4fde199362b42fa37068f2e6c6885cfea474a3d1#diff-1168fadffa18bd096582ae7f8b6db744fd896bd5600ee1d1ac6ac4474af251b9L292-L334 }) - }) + } + } + + // Run the protocol based e2e tests against the git implementations. + for _, gitImpl := range gitImpls { + for _, p := range protos { + t.Run(gitImpl+"_"+p, testFunc(p, gitImpl)) + } } } @@ -909,6 +826,58 @@ func mergeBranchIntoHead(g *WithT, repo *git.Repository, pushBranch string) { g.Expect(err).NotTo(HaveOccurred()) } +// testWithRepoAndImagePolicyTestFunc is the test closure function type passed +// to testWithRepoAndImagePolicy. +type testWithRepoAndImagePolicyTestFunc func(g *WithT, namespace, repoURL, gitRepoName, branch, imagePolicyName string, localRepo *git.Repository) + +// testWithRepoAndImagePolicy sets up a git server, a repository in the git +// server, a GitRepository object for the created git repo, and an ImagePolicy +// with the given policy spec. It calls testFunc to run the test in the created +// environment. +func testWithRepoAndImagePolicy( + g *WithT, + fixture string, + policySpec imagev1_reflect.ImagePolicySpec, + latest string, + testFunc testWithRepoAndImagePolicyTestFunc) { + namespace := "image-auto-test-" + randStringRunes(5) + branch := randStringRunes(8) + repositoryPath := "/config-" + randStringRunes(6) + ".git" + gitRepoName := "image-auto-" + randStringRunes(5) + imagePolicyName := "policy-" + randStringRunes(5) + + // Create test git server. + gitServer, err := setupGitTestServer() + g.Expect(err).ToNot(HaveOccurred(), "failed to create test git server") + defer os.RemoveAll(gitServer.Root()) + defer gitServer.StopHTTP() + + // Create test namespace. + nsCleanup, err := createNamespace(namespace) + g.Expect(err).ToNot(HaveOccurred(), "failed to create test namespace") + defer func() { + g.Expect(nsCleanup()).To(Succeed()) + }() + + // Create a git repo. + g.Expect(initGitRepo(gitServer, fixture, branch, repositoryPath)).To(Succeed()) + + // Clone the repo. + repoURL := gitServer.HTTPAddressWithCredentials() + repositoryPath + localRepo, err := cloneRepo(repoURL, branch) + g.Expect(err).ToNot(HaveOccurred(), "failed to clone git repo") + + // Create GitRepository resource for the above repo. + err = createGitRepository(gitRepoName, namespace, "", repoURL, "") + g.Expect(err).ToNot(HaveOccurred(), "failed to create GitRepository resource") + + // Create ImagePolicy with populated latest image in the status. + err = createImagePolicyWithLatestImageForSpec(imagePolicyName, namespace, policySpec, latest) + g.Expect(err).ToNot(HaveOccurred(), "failed to create ImagePolicy resource") + + testFunc(g, namespace, repoURL, gitRepoName, branch, imagePolicyName, localRepo) +} + // setupGitTestServer creates and returns a git test server. The caller must // ensure it's stopped and cleaned up. func setupGitTestServer() (*gittestserver.GitServer, error) { @@ -977,18 +946,23 @@ func createGitRepository(name, namespace, impl, repoURL, secretRef string) error } func createImagePolicyWithLatestImage(name, namespace, repoRef, semverRange, latest string) error { - policy := &imagev1_reflect.ImagePolicy{ - Spec: imagev1_reflect.ImagePolicySpec{ - ImageRepositoryRef: meta.NamespacedObjectReference{ - Name: repoRef, - }, - Policy: imagev1_reflect.ImagePolicyChoice{ - SemVer: &imagev1_reflect.SemVerPolicy{ - Range: semverRange, - }, + policySpec := imagev1_reflect.ImagePolicySpec{ + ImageRepositoryRef: meta.NamespacedObjectReference{ + Name: repoRef, + }, + Policy: imagev1_reflect.ImagePolicyChoice{ + SemVer: &imagev1_reflect.SemVerPolicy{ + Range: semverRange, }, }, } + return createImagePolicyWithLatestImageForSpec(name, namespace, policySpec, latest) +} + +func createImagePolicyWithLatestImageForSpec(name, namespace string, policySpec imagev1_reflect.ImagePolicySpec, latest string) error { + policy := &imagev1_reflect.ImagePolicy{ + Spec: policySpec, + } policy.Name = name policy.Namespace = namespace err := testEnv.Create(context.Background(), policy) From f4b89bdc60c4c107369b91874d3e59f7019a4aff Mon Sep 17 00:00:00 2001 From: Michael Bridgen Date: Tue, 9 Nov 2021 15:13:05 +0000 Subject: [PATCH 09/11] Reduce higher-order function in test template This is just a cosmetic thing -- there's no need for the higher-order func used to create mini test suites, given the protocol and git implementation. This made the Gingko version clearer, arguably, but it can be reduced away here for a bit less nesting. Signed-off-by: Michael Bridgen --- controllers/update_test.go | 456 ++++++++++++++++++------------------- 1 file changed, 228 insertions(+), 228 deletions(-) diff --git a/controllers/update_test.go b/controllers/update_test.go index 19c21cb7..b730e1af 100644 --- a/controllers/update_test.go +++ b/controllers/update_test.go @@ -283,165 +283,92 @@ func TestImageUpdateAutomation_e2e(t *testing.T) { gitImpls := []string{sourcev1.GoGitImplementation, sourcev1.LibGit2Implementation} protos := []string{"http", "ssh"} - testFunc := func(proto string, impl string) func(t *testing.T) { - return func(t *testing.T) { - g := NewWithT(t) - - const latestImage = "helloworld:1.0.1" - - namespace := "image-auto-test-" + randStringRunes(5) - branch := randStringRunes(8) - repositoryPath := "/config-" + randStringRunes(6) + ".git" - gitRepoName := "image-auto-" + randStringRunes(5) - gitSecretName := "git-secret-" + randStringRunes(5) - imagePolicyName := "policy-" + randStringRunes(5) - updateStrategy := &imagev1.UpdateStrategy{ - Strategy: imagev1.UpdateStrategySetters, - } + testFunc := func(t *testing.T, proto string, impl string) { + g := NewWithT(t) + + const latestImage = "helloworld:1.0.1" + + namespace := "image-auto-test-" + randStringRunes(5) + branch := randStringRunes(8) + repositoryPath := "/config-" + randStringRunes(6) + ".git" + gitRepoName := "image-auto-" + randStringRunes(5) + gitSecretName := "git-secret-" + randStringRunes(5) + imagePolicyName := "policy-" + randStringRunes(5) + updateStrategy := &imagev1.UpdateStrategy{ + Strategy: imagev1.UpdateStrategySetters, + } + + // Create a test namespace. + nsCleanup, err := createNamespace(namespace) + g.Expect(err).ToNot(HaveOccurred(), "failed to create test namespace") + defer func() { + g.Expect(nsCleanup()).To(Succeed()) + }() + + // Create git server. + gitServer, err := setupGitTestServer() + g.Expect(err).ToNot(HaveOccurred(), "failed to create test git server") + defer os.RemoveAll(gitServer.Root()) + defer gitServer.StopHTTP() + + cloneLocalRepoURL := gitServer.HTTPAddressWithCredentials() + repositoryPath + repoURL, err := getRepoURL(gitServer, repositoryPath, proto) + g.Expect(err).ToNot(HaveOccurred()) - // Create a test namespace. - nsCleanup, err := createNamespace(namespace) - g.Expect(err).ToNot(HaveOccurred(), "failed to create test namespace") + // Start the ssh server if needed. + if proto == "ssh" { + // NOTE: Check how this is done in source-controller. + go func() { + gitServer.StartSSH() + }() defer func() { - g.Expect(nsCleanup()).To(Succeed()) + g.Expect(gitServer.StopSSH()).To(Succeed()) }() + } - // Create git server. - gitServer, err := setupGitTestServer() - g.Expect(err).ToNot(HaveOccurred(), "failed to create test git server") - defer os.RemoveAll(gitServer.Root()) - defer gitServer.StopHTTP() - - cloneLocalRepoURL := gitServer.HTTPAddressWithCredentials() + repositoryPath - repoURL, err := getRepoURL(gitServer, repositoryPath, proto) - g.Expect(err).ToNot(HaveOccurred()) - - // Start the ssh server if needed. - if proto == "ssh" { - // NOTE: Check how this is done in source-controller. - go func() { - gitServer.StartSSH() - }() - defer func() { - g.Expect(gitServer.StopSSH()).To(Succeed()) - }() - } - - commitMessage := "Commit a difference " + randStringRunes(5) - - // Initialize a git repo. - g.Expect(initGitRepo(gitServer, "testdata/appconfig", branch, repositoryPath)).To(Succeed()) + commitMessage := "Commit a difference " + randStringRunes(5) - // Clone the repo locally. - localRepo, err := cloneRepo(cloneLocalRepoURL, branch) - g.Expect(err).ToNot(HaveOccurred(), "failed to clone git repo") + // Initialize a git repo. + g.Expect(initGitRepo(gitServer, "testdata/appconfig", branch, repositoryPath)).To(Succeed()) - // Create GitRepository resource for the above repo. - if proto == "ssh" { - // SSH requires an identity (private key) and known_hosts file - // in a secret. - err = createSSHIdentitySecret(gitSecretName, namespace, repoURL) - g.Expect(err).ToNot(HaveOccurred()) - err = createGitRepository(gitRepoName, namespace, impl, repoURL, gitSecretName) - g.Expect(err).ToNot(HaveOccurred()) - } else { - err = createGitRepository(gitRepoName, namespace, impl, repoURL, "") - g.Expect(err).ToNot(HaveOccurred()) - } + // Clone the repo locally. + localRepo, err := cloneRepo(cloneLocalRepoURL, branch) + g.Expect(err).ToNot(HaveOccurred(), "failed to clone git repo") - // Create an image policy. - policyKey := types.NamespacedName{ - Name: imagePolicyName, - Namespace: namespace, - } - // NB not testing the image reflector controller; this - // will make a "fully formed" ImagePolicy object. - err = createImagePolicyWithLatestImage(imagePolicyName, namespace, "not-expected-to-exist", "1.x", latestImage) - g.Expect(err).ToNot(HaveOccurred(), "failed to create ImagePolicy resource") - - // Create ImageUpdateAutomation resource for each of the test cases - // and cleanup at the end. - - t.Run("PushSpec", func(t *testing.T) { - imageUpdateAutomationName := "update-" + randStringRunes(5) - pushBranch := "pr-" + randStringRunes(5) - - t.Run("update with PushSpec", func(t *testing.T) { - commitInRepo(g, cloneLocalRepoURL, branch, "Install setter marker", func(tmp string) { - g.Expect(replaceMarker(tmp, policyKey)).To(Succeed()) - }) - // Pull the head commit we just pushed, so it's not - // considered a new commit when checking for a commit - // made by automation. - waitForNewHead(g, localRepo, branch) - - // Now create the automation object, and let it (one - // hopes!) make a commit itself. - err = createImageUpdateAutomation(imageUpdateAutomationName, namespace, gitRepoName, branch, pushBranch, commitMessage, "", updateStrategy) - g.Expect(err).ToNot(HaveOccurred()) - - // Wait for a new commit to be made by the controller. - waitForNewHead(g, localRepo, pushBranch) - - head, err := localRepo.Reference(plumbing.NewRemoteReferenceName(originRemote, pushBranch), true) - g.Expect(err).NotTo(HaveOccurred()) - commit, err := localRepo.CommitObject(head.Hash()) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(commit.Message).To(Equal(commitMessage)) - }) + // Create GitRepository resource for the above repo. + if proto == "ssh" { + // SSH requires an identity (private key) and known_hosts file + // in a secret. + err = createSSHIdentitySecret(gitSecretName, namespace, repoURL) + g.Expect(err).ToNot(HaveOccurred()) + err = createGitRepository(gitRepoName, namespace, impl, repoURL, gitSecretName) + g.Expect(err).ToNot(HaveOccurred()) + } else { + err = createGitRepository(gitRepoName, namespace, impl, repoURL, "") + g.Expect(err).ToNot(HaveOccurred()) + } - t.Run("push branch gets updated", func(t *testing.T) { - // Get the head hash before update. - head, err := localRepo.Reference(plumbing.NewRemoteReferenceName(originRemote, pushBranch), true) - headHash := head.String() - g.Expect(err).NotTo(HaveOccurred()) - - // Update the policy and expect another commit in the push - // branch. - err = updateImagePolicyWithLatestImage(imagePolicyName, namespace, "helloworld:v1.3.0") - g.Expect(err).ToNot(HaveOccurred()) - waitForNewHead(g, localRepo, pushBranch) - head, err = localRepo.Reference(plumbing.NewRemoteReferenceName(originRemote, pushBranch), true) - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(head.String()).NotTo(Equal(headHash)) - }) + // Create an image policy. + policyKey := types.NamespacedName{ + Name: imagePolicyName, + Namespace: namespace, + } + // NB not testing the image reflector controller; this + // will make a "fully formed" ImagePolicy object. + err = createImagePolicyWithLatestImage(imagePolicyName, namespace, "not-expected-to-exist", "1.x", latestImage) + g.Expect(err).ToNot(HaveOccurred(), "failed to create ImagePolicy resource") - t.Run("still pushes to the push branch after it's merged", func(t *testing.T) { - // Get the head hash before. - head, err := localRepo.Reference(plumbing.NewRemoteReferenceName(originRemote, pushBranch), true) - headHash := head.String() - g.Expect(err).NotTo(HaveOccurred()) - - // Merge the push branch into checkout branch, and push the merge commit - // upstream. - // waitForNewHead() leaves the repo at the head of the branch given, i.e., the - // push branch), so we have to check out the "main" branch first. - g.Expect(checkoutBranch(localRepo, branch)).To(Succeed()) - mergeBranchIntoHead(g, localRepo, pushBranch) - - // Update the policy and expect another commit in the push - // branch. - err = updateImagePolicyWithLatestImage(imagePolicyName, namespace, "helloworld:v1.3.1") - g.Expect(err).ToNot(HaveOccurred()) - - waitForNewHead(g, localRepo, pushBranch) - - head, err = localRepo.Reference(plumbing.NewRemoteReferenceName(originRemote, pushBranch), true) - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(head.String()).NotTo(Equal(headHash)) - }) + // Create ImageUpdateAutomation resource for each of the test cases + // and cleanup at the end. - // Cleanup the image update automation used above. - g.Expect(deleteImageUpdateAutomation(imageUpdateAutomationName, namespace)).To(Succeed()) - }) + t.Run("PushSpec", func(t *testing.T) { + imageUpdateAutomationName := "update-" + randStringRunes(5) + pushBranch := "pr-" + randStringRunes(5) - t.Run("with update strategy setters", func(t *testing.T) { - // Insert a setter reference into the deployment file, - // before creating the automation object itself. + t.Run("update with PushSpec", func(t *testing.T) { commitInRepo(g, cloneLocalRepoURL, branch, "Install setter marker", func(tmp string) { g.Expect(replaceMarker(tmp, policyKey)).To(Succeed()) }) - // Pull the head commit we just pushed, so it's not // considered a new commit when checking for a commit // made by automation. @@ -449,108 +376,181 @@ func TestImageUpdateAutomation_e2e(t *testing.T) { // Now create the automation object, and let it (one // hopes!) make a commit itself. - updateKey := types.NamespacedName{ - Namespace: namespace, - Name: "update-" + randStringRunes(5), - } - err = createImageUpdateAutomation(updateKey.Name, namespace, gitRepoName, branch, "", commitMessage, "", updateStrategy) + err = createImageUpdateAutomation(imageUpdateAutomationName, namespace, gitRepoName, branch, pushBranch, commitMessage, "", updateStrategy) g.Expect(err).ToNot(HaveOccurred()) - defer func() { - g.Expect(deleteImageUpdateAutomation(updateKey.Name, namespace)).To(Succeed()) - }() // Wait for a new commit to be made by the controller. - waitForNewHead(g, localRepo, branch) + waitForNewHead(g, localRepo, pushBranch) - // Check if the repo head matches with the ImageUpdateAutomation - // last push commit status. - head, err := localRepo.Head() + head, err := localRepo.Reference(plumbing.NewRemoteReferenceName(originRemote, pushBranch), true) + g.Expect(err).NotTo(HaveOccurred()) + commit, err := localRepo.CommitObject(head.Hash()) g.Expect(err).ToNot(HaveOccurred()) + g.Expect(commit.Message).To(Equal(commitMessage)) + }) - var newObj imagev1.ImageUpdateAutomation - g.Expect(testEnv.Get(context.Background(), updateKey, &newObj)).To(Succeed()) - g.Expect(newObj.Status.LastPushCommit).To(Equal(head.Hash().String())) - g.Expect(newObj.Status.LastPushTime).ToNot(BeNil()) + t.Run("push branch gets updated", func(t *testing.T) { + // Get the head hash before update. + head, err := localRepo.Reference(plumbing.NewRemoteReferenceName(originRemote, pushBranch), true) + headHash := head.String() + g.Expect(err).NotTo(HaveOccurred()) - compareRepoWithExpected(g, cloneLocalRepoURL, branch, "testdata/appconfig-setters-expected", func(tmp string) { - g.Expect(replaceMarker(tmp, policyKey)).To(Succeed()) - }) + // Update the policy and expect another commit in the push + // branch. + err = updateImagePolicyWithLatestImage(imagePolicyName, namespace, "helloworld:v1.3.0") + g.Expect(err).ToNot(HaveOccurred()) + waitForNewHead(g, localRepo, pushBranch) + head, err = localRepo.Reference(plumbing.NewRemoteReferenceName(originRemote, pushBranch), true) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(head.String()).NotTo(Equal(headHash)) }) - t.Run("no reconciliation when object is suspended", func(t *testing.T) { - // Create the automation object. - updateKey := types.NamespacedName{ - Namespace: namespace, - Name: "update-" + randStringRunes(5), - } - err = createImageUpdateAutomation(updateKey.Name, namespace, gitRepoName, branch, "", commitMessage, "", updateStrategy) + t.Run("still pushes to the push branch after it's merged", func(t *testing.T) { + // Get the head hash before. + head, err := localRepo.Reference(plumbing.NewRemoteReferenceName(originRemote, pushBranch), true) + headHash := head.String() + g.Expect(err).NotTo(HaveOccurred()) + + // Merge the push branch into checkout branch, and push the merge commit + // upstream. + // waitForNewHead() leaves the repo at the head of the branch given, i.e., the + // push branch), so we have to check out the "main" branch first. + g.Expect(checkoutBranch(localRepo, branch)).To(Succeed()) + mergeBranchIntoHead(g, localRepo, pushBranch) + + // Update the policy and expect another commit in the push + // branch. + err = updateImagePolicyWithLatestImage(imagePolicyName, namespace, "helloworld:v1.3.1") g.Expect(err).ToNot(HaveOccurred()) - defer func() { - g.Expect(deleteImageUpdateAutomation(updateKey.Name, namespace)).To(Succeed()) - }() - - // Wait for the object to be available in the cache before - // attempting update. - g.Eventually(func() bool { - obj := &imagev1.ImageUpdateAutomation{} - if err := testEnv.Get(context.Background(), updateKey, obj); err != nil { - return false - } - return true - }, timeout, time.Second).Should(BeTrue()) - - // Suspend the automation object. - var updatePatch imagev1.ImageUpdateAutomation - g.Expect(testEnv.Get(context.TODO(), updateKey, &updatePatch)).To(Succeed()) - updatePatch.Spec.Suspend = true - g.Expect(testEnv.Patch(context.Background(), &updatePatch, client.Merge)).To(Succeed()) - - // Create a new image automation reconciler and run it - // explicitly. - imageAutoReconciler := &ImageUpdateAutomationReconciler{ - Client: testEnv, - Scheme: scheme.Scheme, - } - // Wait for the suspension to reach the cache - var newUpdate imagev1.ImageUpdateAutomation - g.Eventually(func() bool { - if err := imageAutoReconciler.Get(context.Background(), updateKey, &newUpdate); err != nil { - return false - } - return newUpdate.Spec.Suspend - }, timeout, time.Second).Should(BeTrue()) - // Run the reconciliation explicitly, and make sure it - // doesn't do anything - result, err := imageAutoReconciler.Reconcile(logr.NewContext(context.TODO(), ctrl.Log), ctrl.Request{ - NamespacedName: updateKey, - }) - g.Expect(err).To(BeNil()) - // This ought to fail if suspend is not working, since the item would be requeued; - // but if not, additional checks lie below. - g.Expect(result).To(Equal(ctrl.Result{})) - - var checkUpdate imagev1.ImageUpdateAutomation - g.Expect(testEnv.Get(context.Background(), updateKey, &checkUpdate)).To(Succeed()) - g.Expect(checkUpdate.Status.ObservedGeneration).NotTo(Equal(checkUpdate.ObjectMeta.Generation)) + waitForNewHead(g, localRepo, pushBranch) + + head, err = localRepo.Reference(plumbing.NewRemoteReferenceName(originRemote, pushBranch), true) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(head.String()).NotTo(Equal(headHash)) }) - t.Run("reconciles with reconcile request annotation", func(t *testing.T) { - // The automation has run, and is not expected to run - // again for 2 hours. Make a commit to the git repo - // which needs to be undone by automation, then add - // the annotation and make sure it runs again. + // Cleanup the image update automation used above. + g.Expect(deleteImageUpdateAutomation(imageUpdateAutomationName, namespace)).To(Succeed()) + }) - // TODO: Implement adding request annotation. - // Refer: https://github.com/fluxcd/image-automation-controller/pull/82/commits/4fde199362b42fa37068f2e6c6885cfea474a3d1#diff-1168fadffa18bd096582ae7f8b6db744fd896bd5600ee1d1ac6ac4474af251b9L292-L334 + t.Run("with update strategy setters", func(t *testing.T) { + // Insert a setter reference into the deployment file, + // before creating the automation object itself. + commitInRepo(g, cloneLocalRepoURL, branch, "Install setter marker", func(tmp string) { + g.Expect(replaceMarker(tmp, policyKey)).To(Succeed()) }) - } + + // Pull the head commit we just pushed, so it's not + // considered a new commit when checking for a commit + // made by automation. + waitForNewHead(g, localRepo, branch) + + // Now create the automation object, and let it (one + // hopes!) make a commit itself. + updateKey := types.NamespacedName{ + Namespace: namespace, + Name: "update-" + randStringRunes(5), + } + err = createImageUpdateAutomation(updateKey.Name, namespace, gitRepoName, branch, "", commitMessage, "", updateStrategy) + g.Expect(err).ToNot(HaveOccurred()) + defer func() { + g.Expect(deleteImageUpdateAutomation(updateKey.Name, namespace)).To(Succeed()) + }() + + // Wait for a new commit to be made by the controller. + waitForNewHead(g, localRepo, branch) + + // Check if the repo head matches with the ImageUpdateAutomation + // last push commit status. + head, err := localRepo.Head() + g.Expect(err).ToNot(HaveOccurred()) + + var newObj imagev1.ImageUpdateAutomation + g.Expect(testEnv.Get(context.Background(), updateKey, &newObj)).To(Succeed()) + g.Expect(newObj.Status.LastPushCommit).To(Equal(head.Hash().String())) + g.Expect(newObj.Status.LastPushTime).ToNot(BeNil()) + + compareRepoWithExpected(g, cloneLocalRepoURL, branch, "testdata/appconfig-setters-expected", func(tmp string) { + g.Expect(replaceMarker(tmp, policyKey)).To(Succeed()) + }) + }) + + t.Run("no reconciliation when object is suspended", func(t *testing.T) { + // Create the automation object. + updateKey := types.NamespacedName{ + Namespace: namespace, + Name: "update-" + randStringRunes(5), + } + err = createImageUpdateAutomation(updateKey.Name, namespace, gitRepoName, branch, "", commitMessage, "", updateStrategy) + g.Expect(err).ToNot(HaveOccurred()) + defer func() { + g.Expect(deleteImageUpdateAutomation(updateKey.Name, namespace)).To(Succeed()) + }() + + // Wait for the object to be available in the cache before + // attempting update. + g.Eventually(func() bool { + obj := &imagev1.ImageUpdateAutomation{} + if err := testEnv.Get(context.Background(), updateKey, obj); err != nil { + return false + } + return true + }, timeout, time.Second).Should(BeTrue()) + + // Suspend the automation object. + var updatePatch imagev1.ImageUpdateAutomation + g.Expect(testEnv.Get(context.TODO(), updateKey, &updatePatch)).To(Succeed()) + updatePatch.Spec.Suspend = true + g.Expect(testEnv.Patch(context.Background(), &updatePatch, client.Merge)).To(Succeed()) + + // Create a new image automation reconciler and run it + // explicitly. + imageAutoReconciler := &ImageUpdateAutomationReconciler{ + Client: testEnv, + Scheme: scheme.Scheme, + } + + // Wait for the suspension to reach the cache + var newUpdate imagev1.ImageUpdateAutomation + g.Eventually(func() bool { + if err := imageAutoReconciler.Get(context.Background(), updateKey, &newUpdate); err != nil { + return false + } + return newUpdate.Spec.Suspend + }, timeout, time.Second).Should(BeTrue()) + // Run the reconciliation explicitly, and make sure it + // doesn't do anything + result, err := imageAutoReconciler.Reconcile(logr.NewContext(context.TODO(), ctrl.Log), ctrl.Request{ + NamespacedName: updateKey, + }) + g.Expect(err).To(BeNil()) + // This ought to fail if suspend is not working, since the item would be requeued; + // but if not, additional checks lie below. + g.Expect(result).To(Equal(ctrl.Result{})) + + var checkUpdate imagev1.ImageUpdateAutomation + g.Expect(testEnv.Get(context.Background(), updateKey, &checkUpdate)).To(Succeed()) + g.Expect(checkUpdate.Status.ObservedGeneration).NotTo(Equal(checkUpdate.ObjectMeta.Generation)) + }) + + t.Run("reconciles with reconcile request annotation", func(t *testing.T) { + // The automation has run, and is not expected to run + // again for 2 hours. Make a commit to the git repo + // which needs to be undone by automation, then add + // the annotation and make sure it runs again. + + // TODO: Implement adding request annotation. + // Refer: https://github.com/fluxcd/image-automation-controller/pull/82/commits/4fde199362b42fa37068f2e6c6885cfea474a3d1#diff-1168fadffa18bd096582ae7f8b6db744fd896bd5600ee1d1ac6ac4474af251b9L292-L334 + }) } // Run the protocol based e2e tests against the git implementations. for _, gitImpl := range gitImpls { - for _, p := range protos { - t.Run(gitImpl+"_"+p, testFunc(p, gitImpl)) + for _, proto := range protos { + t.Run(gitImpl+"_"+proto, func(t *testing.T) { + testFunc(t, proto, gitImpl) + }) } } } From dd81020ef9380937f98d248d3e3d74ee16370339 Mon Sep 17 00:00:00 2001 From: Michael Bridgen Date: Tue, 9 Nov 2021 17:11:38 +0000 Subject: [PATCH 10/11] Collect test args into struct This tidies the random string testWithRepoAndImagePolicy() arguments into a struct, which reduces clutter (and the chance of getting them in the wrong order) in invocations. Signed-off-by: Michael Bridgen --- controllers/update_test.go | 81 ++++++++++++++++++++++---------------- 1 file changed, 46 insertions(+), 35 deletions(-) diff --git a/controllers/update_test.go b/controllers/update_test.go index b730e1af..086feaef 100644 --- a/controllers/update_test.go +++ b/controllers/update_test.go @@ -124,31 +124,31 @@ func TestImageUpdateAutomation_commit_message(t *testing.T) { testWithRepoAndImagePolicy( NewWithT(t), fixture, policySpec, latest, - func(g *WithT, namespace, repoURL, gitRepoName, branch, imagePolicyName string, localRepo *git.Repository) { - commitMessage := fmt.Sprintf(testCommitMessageFmt, namespace, imagePolicyName) + func(g *WithT, s repoAndPolicyArgs, repoURL string, localRepo *git.Repository) { + commitMessage := fmt.Sprintf(testCommitMessageFmt, s.namespace, s.imagePolicyName) // Update the setter marker in the repo. policyKey := types.NamespacedName{ - Name: imagePolicyName, - Namespace: namespace, + Name: s.imagePolicyName, + Namespace: s.namespace, } - commitInRepo(g, repoURL, branch, "Install setter marker", func(tmp string) { + commitInRepo(g, repoURL, s.branch, "Install setter marker", func(tmp string) { g.Expect(replaceMarker(tmp, policyKey)).To(Succeed()) }) // Pull the head commit that was just pushed, so it's not considered a new // commit when checking for a commit made by automation. - waitForNewHead(g, localRepo, branch) + waitForNewHead(g, localRepo, s.branch) // Create the automation object and let it make a commit itself. updateStrategy := &imagev1.UpdateStrategy{ Strategy: imagev1.UpdateStrategySetters, } - err := createImageUpdateAutomation("update-test", namespace, gitRepoName, branch, "", testCommitTemplate, "", updateStrategy) + err := createImageUpdateAutomation("update-test", s.namespace, s.gitRepoName, s.branch, "", testCommitTemplate, "", updateStrategy) g.Expect(err).ToNot(HaveOccurred()) // Wait for a new commit to be made by the controller. - waitForNewHead(g, localRepo, branch) + waitForNewHead(g, localRepo, s.branch) head, _ := localRepo.Head() commit, err := localRepo.CommitObject(head.Hash()) @@ -176,33 +176,33 @@ func TestImageUpdateAutomation_update_path(t *testing.T) { testWithRepoAndImagePolicy( NewWithT(t), fixture, policySpec, latest, - func(g *WithT, namespace, repoURL, gitRepoName, branch, imagePolicyName string, localRepo *git.Repository) { + func(g *WithT, s repoAndPolicyArgs, repoURL string, localRepo *git.Repository) { // Update the setter marker in the repo. policyKey := types.NamespacedName{ - Name: imagePolicyName, - Namespace: namespace, + Name: s.imagePolicyName, + Namespace: s.namespace, } - commitInRepo(g, repoURL, branch, "Install setter marker", func(tmp string) { + commitInRepo(g, repoURL, s.branch, "Install setter marker", func(tmp string) { g.Expect(replaceMarker(path.Join(tmp, "yes"), policyKey)).To(Succeed()) }) - commitInRepo(g, repoURL, branch, "Install setter marker", func(tmp string) { + commitInRepo(g, repoURL, s.branch, "Install setter marker", func(tmp string) { g.Expect(replaceMarker(path.Join(tmp, "no"), policyKey)).To(Succeed()) }) // Pull the head commit that was just pushed, so it's not considered a new // commit when checking for a commit made by automation. - waitForNewHead(g, localRepo, branch) + waitForNewHead(g, localRepo, s.branch) // Create the automation object and let it make a commit itself. updateStrategy := &imagev1.UpdateStrategy{ Strategy: imagev1.UpdateStrategySetters, Path: "./yes", } - err := createImageUpdateAutomation("update-test", namespace, gitRepoName, branch, "", testCommitTemplate, "", updateStrategy) + err := createImageUpdateAutomation("update-test", s.namespace, s.gitRepoName, s.branch, "", testCommitTemplate, "", updateStrategy) g.Expect(err).ToNot(HaveOccurred()) // Wait for a new commit to be made by the controller. - waitForNewHead(g, localRepo, branch) + waitForNewHead(g, localRepo, s.branch) head, _ := localRepo.Head() commit, err := localRepo.CommitObject(head.Hash()) @@ -228,33 +228,33 @@ func TestImageUpdateAutomation_signed_commit(t *testing.T) { testWithRepoAndImagePolicy( NewWithT(t), fixture, policySpec, latest, - func(g *WithT, namespace, repoURL, gitRepoName, branch, imagePolicyName string, localRepo *git.Repository) { + func(g *WithT, s repoAndPolicyArgs, repoURL string, localRepo *git.Repository) { signingKeySecretName := "signing-key-secret-" + randStringRunes(5) // Update the setter marker in the repo. policyKey := types.NamespacedName{ - Name: imagePolicyName, - Namespace: namespace, + Name: s.imagePolicyName, + Namespace: s.namespace, } - commitInRepo(g, repoURL, branch, "Install setter marker", func(tmp string) { + commitInRepo(g, repoURL, s.branch, "Install setter marker", func(tmp string) { g.Expect(replaceMarker(tmp, policyKey)).To(Succeed()) }) // Pull the head commit that was just pushed, so it's not considered a new // commit when checking for a commit made by automation. - waitForNewHead(g, localRepo, branch) + waitForNewHead(g, localRepo, s.branch) - pgpEntity, err := createSigningKeyPair(signingKeySecretName, namespace) + pgpEntity, err := createSigningKeyPair(signingKeySecretName, s.namespace) g.Expect(err).ToNot(HaveOccurred(), "failed to create signing key pair") // Create the automation object and let it make a commit itself. updateStrategy := &imagev1.UpdateStrategy{ Strategy: imagev1.UpdateStrategySetters, } - err = createImageUpdateAutomation("update-test", namespace, gitRepoName, branch, "", testCommitTemplate, signingKeySecretName, updateStrategy) + err = createImageUpdateAutomation("update-test", s.namespace, s.gitRepoName, s.branch, "", testCommitTemplate, signingKeySecretName, updateStrategy) g.Expect(err).ToNot(HaveOccurred()) // Wait for a new commit to be made by the controller. - waitForNewHead(g, localRepo, branch) + waitForNewHead(g, localRepo, s.branch) head, err := localRepo.Head() g.Expect(err).ToNot(HaveOccurred()) @@ -826,9 +826,22 @@ func mergeBranchIntoHead(g *WithT, repo *git.Repository, pushBranch string) { g.Expect(err).NotTo(HaveOccurred()) } +type repoAndPolicyArgs struct { + namespace, gitRepoName, branch, imagePolicyName string +} + +func newRepoAndPolicyArgs() repoAndPolicyArgs { + return repoAndPolicyArgs{ + namespace: "image-auto-test-" + randStringRunes(5), + gitRepoName: "image-auto-test-" + randStringRunes(5), + branch: randStringRunes(8), + imagePolicyName: "policy-" + randStringRunes(5), + } +} + // testWithRepoAndImagePolicyTestFunc is the test closure function type passed // to testWithRepoAndImagePolicy. -type testWithRepoAndImagePolicyTestFunc func(g *WithT, namespace, repoURL, gitRepoName, branch, imagePolicyName string, localRepo *git.Repository) +type testWithRepoAndImagePolicyTestFunc func(g *WithT, s repoAndPolicyArgs, repoURL string, localRepo *git.Repository) // testWithRepoAndImagePolicy sets up a git server, a repository in the git // server, a GitRepository object for the created git repo, and an ImagePolicy @@ -840,11 +853,9 @@ func testWithRepoAndImagePolicy( policySpec imagev1_reflect.ImagePolicySpec, latest string, testFunc testWithRepoAndImagePolicyTestFunc) { - namespace := "image-auto-test-" + randStringRunes(5) - branch := randStringRunes(8) repositoryPath := "/config-" + randStringRunes(6) + ".git" - gitRepoName := "image-auto-" + randStringRunes(5) - imagePolicyName := "policy-" + randStringRunes(5) + + s := newRepoAndPolicyArgs() // Create test git server. gitServer, err := setupGitTestServer() @@ -853,29 +864,29 @@ func testWithRepoAndImagePolicy( defer gitServer.StopHTTP() // Create test namespace. - nsCleanup, err := createNamespace(namespace) + nsCleanup, err := createNamespace(s.namespace) g.Expect(err).ToNot(HaveOccurred(), "failed to create test namespace") defer func() { g.Expect(nsCleanup()).To(Succeed()) }() // Create a git repo. - g.Expect(initGitRepo(gitServer, fixture, branch, repositoryPath)).To(Succeed()) + g.Expect(initGitRepo(gitServer, fixture, s.branch, repositoryPath)).To(Succeed()) // Clone the repo. repoURL := gitServer.HTTPAddressWithCredentials() + repositoryPath - localRepo, err := cloneRepo(repoURL, branch) + localRepo, err := cloneRepo(repoURL, s.branch) g.Expect(err).ToNot(HaveOccurred(), "failed to clone git repo") // Create GitRepository resource for the above repo. - err = createGitRepository(gitRepoName, namespace, "", repoURL, "") + err = createGitRepository(s.gitRepoName, s.namespace, "", repoURL, "") g.Expect(err).ToNot(HaveOccurred(), "failed to create GitRepository resource") // Create ImagePolicy with populated latest image in the status. - err = createImagePolicyWithLatestImageForSpec(imagePolicyName, namespace, policySpec, latest) + err = createImagePolicyWithLatestImageForSpec(s.imagePolicyName, s.namespace, policySpec, latest) g.Expect(err).ToNot(HaveOccurred(), "failed to create ImagePolicy resource") - testFunc(g, namespace, repoURL, gitRepoName, branch, imagePolicyName, localRepo) + testFunc(g, s, repoURL, localRepo) } // setupGitTestServer creates and returns a git test server. The caller must From 8b96c1521c96246ff4239f6987d40d3691a74970 Mon Sep 17 00:00:00 2001 From: Sunny Date: Thu, 11 Nov 2021 20:37:38 +0530 Subject: [PATCH 11/11] update_test: Create ImagePolicy per subtest In TestImageUpdateAutomation_e2e, move the ImagePolicy to be created per subtest and not shared in the common test environment. This makes the tests more independent of each other. Signed-off-by: Sunny --- .../appconfig-setters-expected/deploy.yaml | 2 +- controllers/update_test.go | 40 +++++++++++++++---- pkg/update/result_test.go | 2 +- 3 files changed, 35 insertions(+), 9 deletions(-) diff --git a/controllers/testdata/appconfig-setters-expected/deploy.yaml b/controllers/testdata/appconfig-setters-expected/deploy.yaml index 8edf19fa..e8394aa4 100644 --- a/controllers/testdata/appconfig-setters-expected/deploy.yaml +++ b/controllers/testdata/appconfig-setters-expected/deploy.yaml @@ -7,4 +7,4 @@ spec: spec: containers: - name: hello - image: helloworld:v1.3.1 # SETTER_SITE + image: helloworld:1.0.1 # SETTER_SITE diff --git a/controllers/update_test.go b/controllers/update_test.go index 086feaef..30e8b131 100644 --- a/controllers/update_test.go +++ b/controllers/update_test.go @@ -353,15 +353,20 @@ func TestImageUpdateAutomation_e2e(t *testing.T) { Name: imagePolicyName, Namespace: namespace, } - // NB not testing the image reflector controller; this - // will make a "fully formed" ImagePolicy object. - err = createImagePolicyWithLatestImage(imagePolicyName, namespace, "not-expected-to-exist", "1.x", latestImage) - g.Expect(err).ToNot(HaveOccurred(), "failed to create ImagePolicy resource") - // Create ImageUpdateAutomation resource for each of the test cases - // and cleanup at the end. + // Create ImagePolicy and ImageUpdateAutomation resource for each of the + // test cases and cleanup at the end. t.Run("PushSpec", func(t *testing.T) { + // NB not testing the image reflector controller; this + // will make a "fully formed" ImagePolicy object. + err = createImagePolicyWithLatestImage(imagePolicyName, namespace, "not-expected-to-exist", "1.x", latestImage) + g.Expect(err).ToNot(HaveOccurred(), "failed to create ImagePolicy resource") + + defer func() { + g.Expect(deleteImagePolicy(imagePolicyName, namespace)).ToNot(HaveOccurred()) + }() + imageUpdateAutomationName := "update-" + randStringRunes(5) pushBranch := "pr-" + randStringRunes(5) @@ -435,6 +440,13 @@ func TestImageUpdateAutomation_e2e(t *testing.T) { }) t.Run("with update strategy setters", func(t *testing.T) { + err = createImagePolicyWithLatestImage(imagePolicyName, namespace, "not-expected-to-exist", "1.x", latestImage) + g.Expect(err).ToNot(HaveOccurred(), "failed to create ImagePolicy resource") + + defer func() { + g.Expect(deleteImagePolicy(imagePolicyName, namespace)).ToNot(HaveOccurred()) + }() + // Insert a setter reference into the deployment file, // before creating the automation object itself. commitInRepo(g, cloneLocalRepoURL, branch, "Install setter marker", func(tmp string) { @@ -477,6 +489,13 @@ func TestImageUpdateAutomation_e2e(t *testing.T) { }) t.Run("no reconciliation when object is suspended", func(t *testing.T) { + err = createImagePolicyWithLatestImage(imagePolicyName, namespace, "not-expected-to-exist", "1.x", latestImage) + g.Expect(err).ToNot(HaveOccurred(), "failed to create ImagePolicy resource") + + defer func() { + g.Expect(deleteImagePolicy(imagePolicyName, namespace)).ToNot(HaveOccurred()) + }() + // Create the automation object. updateKey := types.NamespacedName{ Namespace: namespace, @@ -548,7 +567,7 @@ func TestImageUpdateAutomation_e2e(t *testing.T) { // Run the protocol based e2e tests against the git implementations. for _, gitImpl := range gitImpls { for _, proto := range protos { - t.Run(gitImpl+"_"+proto, func(t *testing.T) { + t.Run(fmt.Sprintf("%s_%s", gitImpl, proto), func(t *testing.T) { testFunc(t, proto, gitImpl) }) } @@ -1046,6 +1065,13 @@ func deleteImageUpdateAutomation(name, namespace string) error { return testEnv.Delete(context.Background(), update) } +func deleteImagePolicy(name, namespace string) error { + imagePolicy := &imagev1_reflect.ImagePolicy{} + imagePolicy.Name = name + imagePolicy.Namespace = namespace + return testEnv.Delete(context.Background(), imagePolicy) +} + func createSigningKeyPair(name, namespace string) (*openpgp.Entity, error) { pgpEntity, err := openpgp.NewEntity("", "", "", nil) if err != nil { diff --git a/pkg/update/result_test.go b/pkg/update/result_test.go index 0292470d..1d70643f 100644 --- a/pkg/update/result_test.go +++ b/pkg/update/result_test.go @@ -19,7 +19,7 @@ func mustRef(ref string) imageRef { return imageRef{r, types.NamespacedName{}} } -func TestMustRef(t *testing.T) { +func TestImageRef(t *testing.T) { g := NewWithT(t) t.Run("gives each component of an image ref", func(t *testing.T) {