From fd4b139dec7a2f9ab671774260ff2d9b4fc3bc9f Mon Sep 17 00:00:00 2001 From: Michael Bridgen Date: Mon, 28 Sep 2020 11:06:18 +0100 Subject: [PATCH 01/10] Add setters strategy to ImageUpdateAutomation type This is the means for telling the controller to use kyaml setters, per the design in https://github.com/fluxcd/toolkit/discussions/107#discussioncomment-82746 --- api/v1alpha1/imageupdateautomation_types.go | 14 +++++++++++ api/v1alpha1/zz_generated.deepcopy.go | 25 +++++++++++++++++++ ...lkit.fluxcd.io_imageupdateautomations.yaml | 12 +++++++++ 3 files changed, 51 insertions(+) diff --git a/api/v1alpha1/imageupdateautomation_types.go b/api/v1alpha1/imageupdateautomation_types.go index a131fb7d..efeea820 100644 --- a/api/v1alpha1/imageupdateautomation_types.go +++ b/api/v1alpha1/imageupdateautomation_types.go @@ -62,6 +62,20 @@ type UpdateStrategy struct { // given policy's image, to the policy's latest image reference. // +optional ImagePolicyRef *corev1.LocalObjectReference `json:"imagePolicyRef,omitempty"` + // Setters if present means update workloads using setters, via + // fields marked in the files themselves. + // +optional + Setters *SettersStrategy `json:"setters,omitempty"` +} + +// SettersStrategy specifies how to use kyaml setters to update the +// git repository. +type SettersStrategy struct { + // Paths gives all paths that should be subject to updates using + // setters. If missing, the path `.` (the root of the git + // repository) is assumed. + // +optional + Paths []string `json:"paths,omitempty"` } // CommitSpec specifies how to commit changes to the git repository diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index c4782046..dd13b5d2 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -158,6 +158,26 @@ func (in *ImageUpdateAutomationStatus) DeepCopy() *ImageUpdateAutomationStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SettersStrategy) DeepCopyInto(out *SettersStrategy) { + *out = *in + if in.Paths != nil { + in, out := &in.Paths, &out.Paths + *out = make([]string, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SettersStrategy. +func (in *SettersStrategy) DeepCopy() *SettersStrategy { + if in == nil { + return nil + } + out := new(SettersStrategy) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *UpdateStrategy) DeepCopyInto(out *UpdateStrategy) { *out = *in @@ -166,6 +186,11 @@ func (in *UpdateStrategy) DeepCopyInto(out *UpdateStrategy) { *out = new(corev1.LocalObjectReference) **out = **in } + if in.Setters != nil { + in, out := &in.Setters, &out.Setters + *out = new(SettersStrategy) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new UpdateStrategy. diff --git a/config/crd/bases/image.toolkit.fluxcd.io_imageupdateautomations.yaml b/config/crd/bases/image.toolkit.fluxcd.io_imageupdateautomations.yaml index ddfaa3db..fda0b94e 100644 --- a/config/crd/bases/image.toolkit.fluxcd.io_imageupdateautomations.yaml +++ b/config/crd/bases/image.toolkit.fluxcd.io_imageupdateautomations.yaml @@ -99,6 +99,18 @@ spec: TODO: Add other useful fields. apiVersion, kind, uid?' type: string type: object + setters: + description: Setters if present means update workloads using setters, + via fields marked in the files themselves. + properties: + paths: + description: Paths gives all paths that should be subject to + updates using setters. If missing, the path `.` (the root + of the git repository) is assumed. + items: + type: string + type: array + type: object type: object required: - checkout From e176c22fa993f1288670c76f3c99de2e20232559 Mon Sep 17 00:00:00 2001 From: Michael Bridgen Date: Mon, 28 Sep 2020 12:35:58 +0100 Subject: [PATCH 02/10] Add stub method and test for update by setters This rearranges the test file a little, so that the different strategies can share some setup. --- .../imageupdateautomation_controller.go | 18 ++ controllers/update_test.go | 203 ++++++++++-------- 2 files changed, 128 insertions(+), 93 deletions(-) diff --git a/controllers/imageupdateautomation_controller.go b/controllers/imageupdateautomation_controller.go index 829b82a5..2ea0bc30 100644 --- a/controllers/imageupdateautomation_controller.go +++ b/controllers/imageupdateautomation_controller.go @@ -143,6 +143,18 @@ func (r *ImageUpdateAutomationReconciler) Reconcile(req ctrl.Request) (ctrl.Resu } return ctrl.Result{}, err } + case updateStrat.Setters != nil: + // For setters we first want to compile a list of _all_ the + // policies (maybe in the future this could be filtered by the + // automation object). + var policies imagev1alpha1_reflect.ImagePolicyList + if err := r.List(ctx, &policies, &client.ListOptions{Namespace: req.NamespacedName.Namespace}); err != nil { + return ctrl.Result{}, err + } + + if err := updateAccordingToSetters(ctx, tmp, policies.Items); err != nil { + return ctrl.Result{}, err + } default: log.Info("no update strategy given in the spec") // no sense rescheduling until this resource changes @@ -392,3 +404,9 @@ func updateAccordingToImagePolicy(ctx context.Context, path string, policy *imag } return update.UpdateImageEverywhere(path, path, latestRef, latestRef) } + +// updateAccordingToSetters updates files under the root by treating +// the given image policies as kyaml setters. +func updateAccordingToSetters(ctx context.Context, root string, policies []imagev1alpha1_reflect.ImagePolicy) error { + return nil +} diff --git a/controllers/update_test.go b/controllers/update_test.go index 59f39369..9697af87 100644 --- a/controllers/update_test.go +++ b/controllers/update_test.go @@ -114,11 +114,9 @@ var _ = Describe("ImageUpdateAutomation", func() { Context("with ImagePolicy", func() { var ( - localRepo *git.Repository - updateKey types.NamespacedName - policy *imagev1alpha1_reflect.ImagePolicy - updateByImagePolicy *imagev1alpha1.ImageUpdateAutomation - commitMessage string + localRepo *git.Repository + policy *imagev1alpha1_reflect.ImagePolicy + policyKey types.NamespacedName ) const latestImage = "helloworld:1.0.1" @@ -135,7 +133,7 @@ var _ = Describe("ImageUpdateAutomation", func() { }) Expect(err).ToNot(HaveOccurred()) - policyKey := types.NamespacedName{ + policyKey = types.NamespacedName{ Name: "policy-" + randStringRunes(5), Namespace: namespace.Name, } @@ -163,111 +161,130 @@ var _ = Describe("ImageUpdateAutomation", func() { Expect(k8sClient.Create(context.Background(), policy)).To(Succeed()) Expect(k8sClient.Status().Update(context.Background(), policy)).To(Succeed()) - commitMessage = "Commit a difference " + randStringRunes(5) - updateKey = types.NamespacedName{ - Namespace: gitRepoKey.Namespace, - Name: "update-" + randStringRunes(5), - } - updateByImagePolicy = &imagev1alpha1.ImageUpdateAutomation{ - ObjectMeta: metav1.ObjectMeta{ - Name: updateKey.Name, - Namespace: updateKey.Namespace, - }, - Spec: imagev1alpha1.ImageUpdateAutomationSpec{ - Checkout: imagev1alpha1.GitCheckoutSpec{ - GitRepositoryRef: corev1.LocalObjectReference{ - Name: gitRepoKey.Name, - }, - Branch: defaultBranch, - }, - Update: imagev1alpha1.UpdateStrategy{ - ImagePolicyRef: &corev1.LocalObjectReference{ - Name: policyKey.Name, - }, - }, - Commit: imagev1alpha1.CommitSpec{ - MessageTemplate: commitMessage, - }, - }, - } - Expect(k8sClient.Create(context.Background(), updateByImagePolicy)).To(Succeed()) - head, _ := localRepo.Head() - headHash := head.Hash().String() - working, err := localRepo.Worktree() - Expect(err).ToNot(HaveOccurred()) - Eventually(func() bool { - if working.Pull(&git.PullOptions{ - ReferenceName: plumbing.NewBranchReferenceName(defaultBranch), - }); err != nil { - return false - } - h, _ := localRepo.Head() - return headHash != h.Hash().String() - }, timeout, time.Second).Should(BeTrue()) }) AfterEach(func() { - Expect(k8sClient.Delete(context.Background(), updateByImagePolicy)).To(Succeed()) Expect(k8sClient.Delete(context.Background(), policy)).To(Succeed()) }) - 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)) + Context("with ImagePolicyRef strategy", func() { - tmp, err := ioutil.TempDir("", "gotest-imageauto") - Expect(err).ToNot(HaveOccurred()) - defer os.RemoveAll(tmp) + var ( + updateKey types.NamespacedName + updateByImagePolicy *imagev1alpha1.ImageUpdateAutomation + commitMessage string + ) - _, err = git.PlainClone(tmp, false, &git.CloneOptions{ - URL: repoURL, - ReferenceName: plumbing.NewBranchReferenceName(defaultBranch), + BeforeEach(func() { + commitMessage = "Commit a difference " + randStringRunes(5) + updateKey = types.NamespacedName{ + Namespace: gitRepoKey.Namespace, + Name: "update-" + randStringRunes(5), + } + updateByImagePolicy = &imagev1alpha1.ImageUpdateAutomation{ + ObjectMeta: metav1.ObjectMeta{ + Name: updateKey.Name, + Namespace: updateKey.Namespace, + }, + Spec: imagev1alpha1.ImageUpdateAutomationSpec{ + Checkout: imagev1alpha1.GitCheckoutSpec{ + GitRepositoryRef: corev1.LocalObjectReference{ + Name: gitRepoKey.Name, + }, + Branch: defaultBranch, + }, + Update: imagev1alpha1.UpdateStrategy{ + ImagePolicyRef: &corev1.LocalObjectReference{ + Name: policyKey.Name, + }, + }, + Commit: imagev1alpha1.CommitSpec{ + MessageTemplate: commitMessage, + }, + }, + } + Expect(k8sClient.Create(context.Background(), updateByImagePolicy)).To(Succeed()) + head, _ := localRepo.Head() + headHash := head.Hash().String() + working, err := localRepo.Worktree() + Expect(err).ToNot(HaveOccurred()) + Eventually(func() bool { + if working.Pull(&git.PullOptions{ + ReferenceName: plumbing.NewBranchReferenceName(defaultBranch), + }); err != nil { + return false + } + h, _ := localRepo.Head() + return headHash != h.Hash().String() + }, timeout, time.Second).Should(BeTrue()) }) - Expect(err).ToNot(HaveOccurred()) - test.ExpectMatchingDirectories(tmp, "testdata/appconfig-expected") - }) - It("makes a commit when the policy changes", func() { - // make sure the first commit happened - head, _ := localRepo.Head() - commit, err := localRepo.CommitObject(head.Hash()) - Expect(err).ToNot(HaveOccurred()) - Expect(commit.Message).To(Equal(commitMessage)) + AfterEach(func() { + Expect(k8sClient.Delete(context.Background(), updateByImagePolicy)).To(Succeed()) + }) - headHash := head.Hash().String() + 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)) - // change the status and - // make sure there's a commit for that. - policy.Status.LatestImage = evenLatestImage - Expect(k8sClient.Status().Update(context.Background(), policy)).To(Succeed()) + tmp, err := ioutil.TempDir("", "gotest-imageauto") + Expect(err).ToNot(HaveOccurred()) + defer os.RemoveAll(tmp) - working, err := localRepo.Worktree() - Expect(err).ToNot(HaveOccurred()) - Eventually(func() bool { - if working.Pull(&git.PullOptions{ + _, err = git.PlainClone(tmp, false, &git.CloneOptions{ + URL: repoURL, ReferenceName: plumbing.NewBranchReferenceName(defaultBranch), - }); err != nil { - return false - } - h, _ := localRepo.Head() - return headHash != h.Hash().String() - }, timeout, time.Second).Should(BeTrue()) - - tmp, err := ioutil.TempDir("", "gotest-imageauto") - Expect(err).ToNot(HaveOccurred()) - defer os.RemoveAll(tmp) + }) + Expect(err).ToNot(HaveOccurred()) + test.ExpectMatchingDirectories(tmp, "testdata/appconfig-expected") + }) - _, err = git.PlainClone(tmp, false, &git.CloneOptions{ - URL: repoURL, - ReferenceName: plumbing.NewBranchReferenceName(defaultBranch), + It("makes a commit when the policy changes", func() { + // make sure the first commit happened + head, _ := localRepo.Head() + commit, err := localRepo.CommitObject(head.Hash()) + Expect(err).ToNot(HaveOccurred()) + Expect(commit.Message).To(Equal(commitMessage)) + + headHash := head.Hash().String() + + // change the status and + // make sure there's a commit for that. + policy.Status.LatestImage = evenLatestImage + Expect(k8sClient.Status().Update(context.Background(), policy)).To(Succeed()) + + working, err := localRepo.Worktree() + Expect(err).ToNot(HaveOccurred()) + Eventually(func() bool { + if working.Pull(&git.PullOptions{ + ReferenceName: plumbing.NewBranchReferenceName(defaultBranch), + }); err != nil { + return false + } + h, _ := localRepo.Head() + return headHash != h.Hash().String() + }, timeout, time.Second).Should(BeTrue()) + + tmp, err := ioutil.TempDir("", "gotest-imageauto") + Expect(err).ToNot(HaveOccurred()) + defer os.RemoveAll(tmp) + + _, err = git.PlainClone(tmp, false, &git.CloneOptions{ + URL: repoURL, + ReferenceName: plumbing.NewBranchReferenceName(defaultBranch), + }) + Expect(err).ToNot(HaveOccurred()) + test.ExpectMatchingDirectories(tmp, "testdata/appconfig-expected2") }) - Expect(err).ToNot(HaveOccurred()) - test.ExpectMatchingDirectories(tmp, "testdata/appconfig-expected2") }) }) + + Context("with Setters", func() { + + }) }) // Initialise a git server with a repo including the files in dir. From b9d345dbc4e5256128489b19bc90f95209e09dc2 Mon Sep 17 00:00:00 2001 From: Michael Bridgen Date: Mon, 28 Sep 2020 12:52:57 +0100 Subject: [PATCH 03/10] Test for update via setters (this test fails, since the implementation is still a stub) --- controllers/update_test.go | 79 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 75 insertions(+), 4 deletions(-) diff --git a/controllers/update_test.go b/controllers/update_test.go index 9697af87..5b6888af 100644 --- a/controllers/update_test.go +++ b/controllers/update_test.go @@ -66,6 +66,7 @@ var _ = Describe("ImageUpdateAutomation", func() { namespace *corev1.Namespace gitServer *testserver.GitServer gitRepoKey types.NamespacedName + commitMessage string ) // Start the git server @@ -123,6 +124,8 @@ var _ = Describe("ImageUpdateAutomation", func() { const evenLatestImage = "helloworld:1.2.0" BeforeEach(func() { + commitMessage = "Commit a difference " + randStringRunes(5) + Expect(initGitRepo(gitServer, "testdata/appconfig", repositoryPath)).To(Succeed()) var err error @@ -172,11 +175,9 @@ var _ = Describe("ImageUpdateAutomation", func() { var ( updateKey types.NamespacedName updateByImagePolicy *imagev1alpha1.ImageUpdateAutomation - commitMessage string ) BeforeEach(func() { - commitMessage = "Commit a difference " + randStringRunes(5) updateKey = types.NamespacedName{ Namespace: gitRepoKey.Namespace, Name: "update-" + randStringRunes(5), @@ -280,11 +281,81 @@ var _ = Describe("ImageUpdateAutomation", func() { test.ExpectMatchingDirectories(tmp, "testdata/appconfig-expected2") }) }) - }) - Context("with Setters", func() { + Context("with Setters", func() { + + var ( + updateKey types.NamespacedName + updateByImagePolicy *imagev1alpha1.ImageUpdateAutomation + ) + + BeforeEach(func() { + updateKey = types.NamespacedName{ + Namespace: gitRepoKey.Namespace, + Name: "update-" + randStringRunes(5), + } + updateByImagePolicy = &imagev1alpha1.ImageUpdateAutomation{ + ObjectMeta: metav1.ObjectMeta{ + Name: updateKey.Name, + Namespace: updateKey.Namespace, + }, + Spec: imagev1alpha1.ImageUpdateAutomationSpec{ + Checkout: imagev1alpha1.GitCheckoutSpec{ + GitRepositoryRef: corev1.LocalObjectReference{ + Name: gitRepoKey.Name, + }, + Branch: defaultBranch, + }, + Update: imagev1alpha1.UpdateStrategy{ + Setters: &imagev1alpha1.SettersStrategy{}, + }, + Commit: imagev1alpha1.CommitSpec{ + MessageTemplate: commitMessage, + }, + }, + } + Expect(k8sClient.Create(context.Background(), updateByImagePolicy)).To(Succeed()) + head, _ := localRepo.Head() + headHash := head.Hash().String() + working, err := localRepo.Worktree() + Expect(err).ToNot(HaveOccurred()) + Eventually(func() bool { + if working.Pull(&git.PullOptions{ + ReferenceName: plumbing.NewBranchReferenceName(defaultBranch), + }); err != nil { + return false + } + h, _ := localRepo.Head() + return headHash != h.Hash().String() + }, timeout, time.Second).Should(BeTrue()) + }) + + AfterEach(func() { + Expect(k8sClient.Delete(context.Background(), updateByImagePolicy)).To(Succeed()) + }) + + 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)) + + tmp, err := ioutil.TempDir("", "gotest-imageauto") + Expect(err).ToNot(HaveOccurred()) + defer os.RemoveAll(tmp) + + _, err = git.PlainClone(tmp, false, &git.CloneOptions{ + URL: repoURL, + ReferenceName: plumbing.NewBranchReferenceName(defaultBranch), + }) + Expect(err).ToNot(HaveOccurred()) + test.ExpectMatchingDirectories(tmp, "testdata/appconfig-setters-expected") + }) + }) }) + }) // Initialise a git server with a repo including the files in dir. From 963ee35c232906f9324a674c58de027ca2e51fa9 Mon Sep 17 00:00:00 2001 From: Michael Bridgen Date: Tue, 29 Sep 2020 12:19:12 +0100 Subject: [PATCH 04/10] Preserve line-end comments when replacing image Previously: replace the YNode (yaml.Node) with a new one which is just the replacement string. Now: change the value of the YNode, and set it back in place. --- controllers/testdata/appconfig-expected/deploy.yaml | 2 +- controllers/testdata/appconfig-expected2/deploy.yaml | 2 +- controllers/testdata/appconfig/deploy.yaml | 2 +- .../replace/commented-expected/deployment.yaml | 11 +++++++++++ pkg/update/testdata/replace/commented/deployment.yaml | 11 +++++++++++ pkg/update/update.go | 5 +++-- pkg/update/update_test.go | 8 ++++++++ 7 files changed, 36 insertions(+), 5 deletions(-) create mode 100644 pkg/update/testdata/replace/commented-expected/deployment.yaml create mode 100644 pkg/update/testdata/replace/commented/deployment.yaml diff --git a/controllers/testdata/appconfig-expected/deploy.yaml b/controllers/testdata/appconfig-expected/deploy.yaml index bd4cef0f..924875cf 100644 --- a/controllers/testdata/appconfig-expected/deploy.yaml +++ b/controllers/testdata/appconfig-expected/deploy.yaml @@ -7,4 +7,4 @@ spec: spec: containers: - name: hello - image: helloworld:1.0.1 + image: helloworld:1.0.1 # SETTER_SITE diff --git a/controllers/testdata/appconfig-expected2/deploy.yaml b/controllers/testdata/appconfig-expected2/deploy.yaml index ad86e2e4..136c05a4 100644 --- a/controllers/testdata/appconfig-expected2/deploy.yaml +++ b/controllers/testdata/appconfig-expected2/deploy.yaml @@ -7,4 +7,4 @@ spec: spec: containers: - name: hello - image: helloworld:1.2.0 + image: helloworld:1.2.0 # SETTER_SITE diff --git a/controllers/testdata/appconfig/deploy.yaml b/controllers/testdata/appconfig/deploy.yaml index 4363ab4a..1ca5a035 100644 --- a/controllers/testdata/appconfig/deploy.yaml +++ b/controllers/testdata/appconfig/deploy.yaml @@ -7,4 +7,4 @@ spec: spec: containers: - name: hello - image: helloworld:1.0.0 + image: helloworld:1.0.0 # SETTER_SITE diff --git a/pkg/update/testdata/replace/commented-expected/deployment.yaml b/pkg/update/testdata/replace/commented-expected/deployment.yaml new file mode 100644 index 00000000..d0f0fcdd --- /dev/null +++ b/pkg/update/testdata/replace/commented-expected/deployment.yaml @@ -0,0 +1,11 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: foo + namespace: bar +spec: + template: + spec: + containers: + - name: c + image: used:v1.1.0 # the comment must stay! diff --git a/pkg/update/testdata/replace/commented/deployment.yaml b/pkg/update/testdata/replace/commented/deployment.yaml new file mode 100644 index 00000000..6e503f6a --- /dev/null +++ b/pkg/update/testdata/replace/commented/deployment.yaml @@ -0,0 +1,11 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: foo + namespace: bar +spec: + template: + spec: + containers: + - name: c + image: used:v1.0.0 # the comment must stay! diff --git a/pkg/update/update.go b/pkg/update/update.go index 67404456..49cd4017 100644 --- a/pkg/update/update.go +++ b/pkg/update/update.go @@ -36,7 +36,6 @@ func makeUpdateImagesFilter(originalRepo, replacement string) kio.Filter { } canonName := originalRef.Context().String() - replacementNode := yaml.NewScalarRNode(replacement) replaceContainerImage := func(container *yaml.RNode) error { if imageField := container.Field("image"); imageField != nil { @@ -45,7 +44,9 @@ func makeUpdateImagesFilter(originalRepo, replacement string) kio.Filter { return err } if ref.Context().String() == canonName { - imageField.Value.SetYNode(replacementNode.YNode()) + ynode := imageField.Value.YNode() + ynode.Value = replacement + imageField.Value.SetYNode(ynode) } } return nil diff --git a/pkg/update/update_test.go b/pkg/update/update_test.go index 36d48753..0557192e 100644 --- a/pkg/update/update_test.go +++ b/pkg/update/update_test.go @@ -32,4 +32,12 @@ var _ = Describe("Update image everywhere", func() { Expect(UpdateImageEverywhere("testdata/replace/original", tmp, "used", "used:v1.1.0")).To(Succeed()) test.ExpectMatchingDirectories("testdata/replace/expected", tmp) }) + + It("keeps comments intact", func() { + tmp, err := ioutil.TempDir("", "gotest") + Expect(err).ToNot(HaveOccurred()) + defer os.RemoveAll(tmp) + Expect(UpdateImageEverywhere("testdata/replace/commented", tmp, "used", "used:v1.1.0")).To(Succeed()) + test.ExpectMatchingDirectories("testdata/replace/commented-expected", tmp) + }) }) From ec8795dcba2cf98874dfb8819cf16329512025d4 Mon Sep 17 00:00:00 2001 From: Michael Bridgen Date: Tue, 29 Sep 2020 19:02:16 +0100 Subject: [PATCH 05/10] Get the test to a point where it could pass In particular, insert the setter reference into the deployment file. --- controllers/update_test.go | 114 ++++++++++++++++++++++--------------- 1 file changed, 69 insertions(+), 45 deletions(-) diff --git a/controllers/update_test.go b/controllers/update_test.go index 5b6888af..8e7e0594 100644 --- a/controllers/update_test.go +++ b/controllers/update_test.go @@ -17,7 +17,9 @@ limitations under the License. package controllers import ( + "bytes" "context" + "fmt" "io/ioutil" "math/rand" "os" @@ -205,19 +207,7 @@ var _ = Describe("ImageUpdateAutomation", func() { }, } Expect(k8sClient.Create(context.Background(), updateByImagePolicy)).To(Succeed()) - head, _ := localRepo.Head() - headHash := head.Hash().String() - working, err := localRepo.Worktree() - Expect(err).ToNot(HaveOccurred()) - Eventually(func() bool { - if working.Pull(&git.PullOptions{ - ReferenceName: plumbing.NewBranchReferenceName(defaultBranch), - }); err != nil { - return false - } - h, _ := localRepo.Head() - return headHash != h.Hash().String() - }, timeout, time.Second).Should(BeTrue()) + waitForNewHead(localRepo) }) AfterEach(func() { @@ -250,24 +240,13 @@ var _ = Describe("ImageUpdateAutomation", func() { Expect(err).ToNot(HaveOccurred()) Expect(commit.Message).To(Equal(commitMessage)) - headHash := head.Hash().String() - // change the status and // make sure there's a commit for that. policy.Status.LatestImage = evenLatestImage Expect(k8sClient.Status().Update(context.Background(), policy)).To(Succeed()) - working, err := localRepo.Worktree() Expect(err).ToNot(HaveOccurred()) - Eventually(func() bool { - if working.Pull(&git.PullOptions{ - ReferenceName: plumbing.NewBranchReferenceName(defaultBranch), - }); err != nil { - return false - } - h, _ := localRepo.Head() - return headHash != h.Hash().String() - }, timeout, time.Second).Should(BeTrue()) + waitForNewHead(localRepo) tmp, err := ioutil.TempDir("", "gotest-imageauto") Expect(err).ToNot(HaveOccurred()) @@ -285,16 +264,54 @@ var _ = Describe("ImageUpdateAutomation", func() { Context("with Setters", func() { var ( - updateKey types.NamespacedName - updateByImagePolicy *imagev1alpha1.ImageUpdateAutomation + updateKey types.NamespacedName + updateBySetters *imagev1alpha1.ImageUpdateAutomation ) BeforeEach(func() { + // Insert a setter reference into the deployment file, + // before creating the automation object itself. + tmp, err := ioutil.TempDir("", "gotest-imageauto-setters") + Expect(err).ToNot(HaveOccurred()) + defer os.RemoveAll(tmp) + repo, err := git.PlainClone(tmp, false, &git.CloneOptions{ + URL: repoURL, + ReferenceName: plumbing.NewBranchReferenceName(defaultBranch), + }) + Expect(err).ToNot(HaveOccurred()) + // NB this requires knowledge of what's in the git + // repo, so a little brittle + deployment := filepath.Join(tmp, "deploy.yaml") + filebytes, err := ioutil.ReadFile(deployment) + Expect(err).NotTo(HaveOccurred()) + newfilebytes := bytes.ReplaceAll(filebytes, []byte("SETTER_SITE"), []byte(setterName(policyKey))) + Expect(ioutil.WriteFile(deployment, newfilebytes, os.FileMode(0666))).To(Succeed()) + worktree, err := repo.Worktree() + Expect(err).ToNot(HaveOccurred()) + _, err = worktree.Add("deploy.yaml") + Expect(err).ToNot(HaveOccurred()) + _, err = worktree.Commit("Install setter marker", &git.CommitOptions{ + Author: &object.Signature{ + Name: "Testbot", + Email: "test@example.com", + When: time.Now(), + }, + }) + Expect(err).ToNot(HaveOccurred()) + Expect(repo.Push(&git.PushOptions{RemoteName: "origin"})).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) + + // now create the automation object, and let it (one + // hopes!) make a commit itself. updateKey = types.NamespacedName{ Namespace: gitRepoKey.Namespace, Name: "update-" + randStringRunes(5), } - updateByImagePolicy = &imagev1alpha1.ImageUpdateAutomation{ + updateBySetters = &imagev1alpha1.ImageUpdateAutomation{ ObjectMeta: metav1.ObjectMeta{ Name: updateKey.Name, Namespace: updateKey.Namespace, @@ -314,24 +331,13 @@ var _ = Describe("ImageUpdateAutomation", func() { }, }, } - Expect(k8sClient.Create(context.Background(), updateByImagePolicy)).To(Succeed()) - head, _ := localRepo.Head() - headHash := head.Hash().String() - working, err := localRepo.Worktree() - Expect(err).ToNot(HaveOccurred()) - Eventually(func() bool { - if working.Pull(&git.PullOptions{ - ReferenceName: plumbing.NewBranchReferenceName(defaultBranch), - }); err != nil { - return false - } - h, _ := localRepo.Head() - return headHash != h.Hash().String() - }, timeout, time.Second).Should(BeTrue()) + Expect(k8sClient.Create(context.Background(), updateBySetters)).To(Succeed()) + // wait for a new commit to be made by the controller + waitForNewHead(localRepo) }) AfterEach(func() { - Expect(k8sClient.Delete(context.Background(), updateByImagePolicy)).To(Succeed()) + Expect(k8sClient.Delete(context.Background(), updateBySetters)).To(Succeed()) }) It("updates to the most recent image", func() { @@ -353,11 +359,29 @@ var _ = Describe("ImageUpdateAutomation", func() { test.ExpectMatchingDirectories(tmp, "testdata/appconfig-setters-expected") }) }) - }) - }) +func setterName(name types.NamespacedName) string { + return fmt.Sprintf("#/image/%s/%s", name.Namespace, name.Name) +} + +func waitForNewHead(repo *git.Repository) { + head, _ := repo.Head() + headHash := head.Hash().String() + working, err := repo.Worktree() + Expect(err).ToNot(HaveOccurred()) + Eventually(func() bool { + if working.Pull(&git.PullOptions{ + ReferenceName: plumbing.NewBranchReferenceName(defaultBranch), + }); err != nil { + return false + } + h, _ := repo.Head() + return headHash != h.Hash().String() + }, timeout, time.Second).Should(BeTrue()) +} + // Initialise a git server with a repo including the files in dir. func initGitRepo(gitServer *testserver.GitServer, fixture, repositoryPath string) error { fs := memfs.New() From 7e58a5bf47603bcf9134966fff3ee2539fd0fb63 Mon Sep 17 00:00:00 2001 From: Michael Bridgen Date: Tue, 6 Oct 2020 11:54:18 +0100 Subject: [PATCH 06/10] Implement and test func for update using setters This adds another means of updating files to the package pkg/update/, in setters.go (and gives the existing file a better name). In passing, I changed the test util for comparing before/after updates, in pkg/files/, to give a little more context when comparing file contents; and, since the comparison between actual and expected is not symmetrical, I corrected the order of the args in the tests. --- go.mod | 1 + go.sum | 10 ++ pkg/test/files.go | 7 +- pkg/update/{update.go => imagepolicy.go} | 0 pkg/update/setters.go | 113 ++++++++++++++++++ .../testdata/setters/expected/marked.yaml | 14 +++ .../testdata/setters/expected/otherns.yaml | 11 ++ .../testdata/setters/expected/unmarked.yaml | 11 ++ .../testdata/setters/original/marked.yaml | 14 +++ .../testdata/setters/original/otherns.yaml | 11 ++ .../testdata/setters/original/unmarked.yaml | 11 ++ pkg/update/update_test.go | 33 ++++- 12 files changed, 231 insertions(+), 5 deletions(-) rename pkg/update/{update.go => imagepolicy.go} (100%) create mode 100644 pkg/update/setters.go create mode 100644 pkg/update/testdata/setters/expected/marked.yaml create mode 100644 pkg/update/testdata/setters/expected/otherns.yaml create mode 100644 pkg/update/testdata/setters/expected/unmarked.yaml create mode 100644 pkg/update/testdata/setters/original/marked.yaml create mode 100644 pkg/update/testdata/setters/original/otherns.yaml create mode 100644 pkg/update/testdata/setters/original/unmarked.yaml diff --git a/go.mod b/go.mod index 5ebb1f60..cc6a3887 100644 --- a/go.mod +++ b/go.mod @@ -9,6 +9,7 @@ require ( github.com/go-git/go-billy/v5 v5.0.0 github.com/go-git/go-git/v5 v5.1.0 github.com/go-logr/logr v0.1.0 + github.com/go-openapi/spec v0.19.5 github.com/google/go-containerregistry v0.1.1 github.com/onsi/ginkgo v1.12.1 github.com/onsi/gomega v1.10.1 diff --git a/go.sum b/go.sum index 1288ecf9..f379d722 100644 --- a/go.sum +++ b/go.sum @@ -345,9 +345,11 @@ github.com/go-openapi/analysis v0.0.0-20180825180245-b006789cd277/go.mod h1:k70t github.com/go-openapi/analysis v0.17.0/go.mod h1:IowGgpVeD0vNm45So8nr+IcQ3pxVtpRoBWb8PVZO0ik= github.com/go-openapi/analysis v0.18.0/go.mod h1:IowGgpVeD0vNm45So8nr+IcQ3pxVtpRoBWb8PVZO0ik= github.com/go-openapi/analysis v0.19.2/go.mod h1:3P1osvZa9jKjb8ed2TPng3f0i/UY9snX6gxi44djMjk= +github.com/go-openapi/analysis v0.19.5 h1:8b2ZgKfKIUTVQpTb77MoRDIMEIwvDVw40o3aOXdfYzI= github.com/go-openapi/analysis v0.19.5/go.mod h1:hkEAkxagaIvIP7VTn8ygJNkd4kAYON2rCu0v0ObL0AU= github.com/go-openapi/errors v0.17.0/go.mod h1:LcZQpmvG4wyF5j4IhA73wkLFQg+QJXOQHVjmcZxhka0= github.com/go-openapi/errors v0.18.0/go.mod h1:LcZQpmvG4wyF5j4IhA73wkLFQg+QJXOQHVjmcZxhka0= +github.com/go-openapi/errors v0.19.2 h1:a2kIyV3w+OS3S97zxUndRVD46+FhGOUBDFY7nmu4CsY= github.com/go-openapi/errors v0.19.2/go.mod h1:qX0BLWsyaKfvhluLejVpVNwNRdXZhEbTA4kxxpKBC94= github.com/go-openapi/jsonpointer v0.0.0-20160704185906-46af16f9f7b1/go.mod h1:+35s3my2LFTysnkMfxsJBAMHj/DoqoB9knIWoYG/Vk0= github.com/go-openapi/jsonpointer v0.17.0/go.mod h1:cOnomiV+CVVwFLk0A/MExoFMjwdsUdVpsRhURCKh+3M= @@ -365,9 +367,11 @@ github.com/go-openapi/loads v0.17.0/go.mod h1:72tmFy5wsWx89uEVddd0RjRWPZm92WRLhf github.com/go-openapi/loads v0.18.0/go.mod h1:72tmFy5wsWx89uEVddd0RjRWPZm92WRLhf7AC+0+OOU= github.com/go-openapi/loads v0.19.0/go.mod h1:72tmFy5wsWx89uEVddd0RjRWPZm92WRLhf7AC+0+OOU= github.com/go-openapi/loads v0.19.2/go.mod h1:QAskZPMX5V0C2gvfkGZzJlINuP7Hx/4+ix5jWFxsNPs= +github.com/go-openapi/loads v0.19.4 h1:5I4CCSqoWzT+82bBkNIvmLc0UOsoKKQ4Fz+3VxOB7SY= github.com/go-openapi/loads v0.19.4/go.mod h1:zZVHonKd8DXyxyw4yfnVjPzBjIQcLt0CCsn0N0ZrQsk= github.com/go-openapi/runtime v0.0.0-20180920151709-4f900dc2ade9/go.mod h1:6v9a6LTXWQCdL8k1AO3cvqx5OtZY/Y9wKTgaoP6YRfA= github.com/go-openapi/runtime v0.19.0/go.mod h1:OwNfisksmmaZse4+gpV3Ne9AyMOlP1lt4sK4FXt0O64= +github.com/go-openapi/runtime v0.19.4 h1:csnOgcgAiuGoM/Po7PEpKDoNulCcF3FGbSnbHfxgjMI= github.com/go-openapi/runtime v0.19.4/go.mod h1:X277bwSUBxVlCYR3r7xgZZGKVvBd/29gLDlFGtJ8NL4= github.com/go-openapi/spec v0.0.0-20160808142527-6aced65f8501/go.mod h1:J8+jY1nAiCcj+friV/PDoE1/3eeccG9LYBs0tYvLOWc= github.com/go-openapi/spec v0.17.0/go.mod h1:XkF/MOi14NmjsfZ8VtAKf8pIlbZzyoTvZsdfssdxcBI= @@ -380,6 +384,7 @@ github.com/go-openapi/strfmt v0.17.0/go.mod h1:P82hnJI0CXkErkXi8IKjPbNBM6lV6+5pL github.com/go-openapi/strfmt v0.18.0/go.mod h1:P82hnJI0CXkErkXi8IKjPbNBM6lV6+5pLP5l494TcyU= github.com/go-openapi/strfmt v0.19.0/go.mod h1:+uW+93UVvGGq2qGaZxdDeJqSAqBqBdl+ZPMF/cC8nDY= github.com/go-openapi/strfmt v0.19.3/go.mod h1:0yX7dbo8mKIvc3XSKp7MNfxw4JytCfCD6+bY1AVL9LU= +github.com/go-openapi/strfmt v0.19.5 h1:0utjKrw+BAh8s57XE9Xz8DUBsVvPmRUB6styvl9wWIM= github.com/go-openapi/strfmt v0.19.5/go.mod h1:eftuHTlB/dI8Uq8JJOyRlieZf+WkkxUuk0dgdHXr2Qk= github.com/go-openapi/swag v0.0.0-20160704191624-1d0bd113de87/go.mod h1:DXUve3Dpr1UfpPtxFw+EFuQ41HhCWZfha5jSVRG7C7I= github.com/go-openapi/swag v0.17.0/go.mod h1:AByQ+nYG6gQg71GINrmuDXCPWdL640yX49/kXLo40Tg= @@ -390,11 +395,13 @@ github.com/go-openapi/swag v0.19.5/go.mod h1:POnQmlKehdgb5mhVOsnJFsivZCEZ/vjK9gh github.com/go-openapi/validate v0.18.0/go.mod h1:Uh4HdOzKt19xGIGm1qHf/ofbX1YQ4Y+MYsct2VUrAJ4= github.com/go-openapi/validate v0.19.2/go.mod h1:1tRCw7m3jtI8eNWEEliiAqUIcBztB2KDnRCRMUi7GTA= github.com/go-openapi/validate v0.19.5/go.mod h1:8DJv2CVJQ6kGNpFW6eV9N3JviE1C85nY1c2z52x1Gk4= +github.com/go-openapi/validate v0.19.8 h1:YFzsdWIDfVuLvIOF+ZmKjVg1MbPJ1QgY9PihMwei1ys= github.com/go-openapi/validate v0.19.8/go.mod h1:8DJv2CVJQ6kGNpFW6eV9N3JviE1C85nY1c2z52x1Gk4= github.com/go-sql-driver/mysql v1.4.0/go.mod h1:zAC/RDZ24gD3HViQzih4MyKcchzm+sOG5ZlKdlhCg5w= github.com/go-sql-driver/mysql v1.4.1/go.mod h1:zAC/RDZ24gD3HViQzih4MyKcchzm+sOG5ZlKdlhCg5w= github.com/go-sql-driver/mysql v1.5.0 h1:ozyZYNQW3x3HtqT1jira07DN2PArx2v7/mN66gGcHOs= github.com/go-sql-driver/mysql v1.5.0/go.mod h1:DCzpHaOWr8IXmIStZouvnhqoel9Qv2LBy8hT2VhHyBg= +github.com/go-stack/stack v1.8.0 h1:5SgMzNM5HxrEjV0ww2lTmX6E2Izsfxas4+YHWRs3Lsk= github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY= github.com/go-toolsmith/astcast v1.0.0/go.mod h1:mt2OdQTeAQcY4DQgPSArJjHCcOwlX+Wl/kwN+LbLGQ4= github.com/go-toolsmith/astcopy v1.0.0/go.mod h1:vrgyG+5Bxrnz4MZWPF+pI4R8h3qKRjjyvV/DSez4WVQ= @@ -727,6 +734,7 @@ github.com/mitchellh/gox v0.4.0/go.mod h1:Sd9lOJ0+aimLBi73mGofS1ycjY8lL3uZM3JPS4 github.com/mitchellh/iochan v1.0.0/go.mod h1:JwYml1nuB7xOzsp52dPpHFffvOCDupsG0QubkSMEySY= github.com/mitchellh/mapstructure v0.0.0-20160808181253-ca63d7c062ee/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y= github.com/mitchellh/mapstructure v1.1.2/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y= +github.com/mitchellh/mapstructure v1.3.1 h1:cCBH2gTD2K0OtLlv/Y5H01VQCqmlDxz30kS5Y5bqfLA= github.com/mitchellh/mapstructure v1.3.1/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo= github.com/mitchellh/osext v0.0.0-20151018003038-5e2d6d41470f h1:2+myh5ml7lgEU/51gbeLHfKGNfgEQQIWrlbdaOsidbQ= github.com/mitchellh/osext v0.0.0-20151018003038-5e2d6d41470f/go.mod h1:OkQIRizQZAeMln+1tSwduZz7+Af5oFlKirV/MSYes2A= @@ -932,6 +940,7 @@ github.com/tdakkota/asciicheck v0.0.0-20200416190851-d7f85be797a2/go.mod h1:yHp0 github.com/tdakkota/asciicheck v0.0.0-20200416200610-e657995f937b/go.mod h1:yHp0ai0Z9gUljN3o0xMhYJnH/IcvkdTBOX2fmJ93JEM= github.com/tetafro/godot v0.3.7/go.mod h1:/7NLHhv08H1+8DNj0MElpAACw1ajsCuf3TKNQxA5S+0= github.com/tetafro/godot v0.4.2/go.mod h1:/7NLHhv08H1+8DNj0MElpAACw1ajsCuf3TKNQxA5S+0= +github.com/tidwall/pretty v1.0.0 h1:HsD+QiTn7sK6flMKIvNmpqz1qrpP3Ps6jOKIKMooyg4= github.com/tidwall/pretty v1.0.0/go.mod h1:XNkn88O1ChpSDQmQeStsy+sBenx6DDtFZJxhVysOjyk= github.com/timakin/bodyclose v0.0.0-20190930140734-f7f2e9bca95e/go.mod h1:Qimiffbc6q9tBWlVV6x0P9sat/ao1xEkREYPPj9hphk= github.com/timakin/bodyclose v0.0.0-20200424151742-cb6215831a94/go.mod h1:Qimiffbc6q9tBWlVV6x0P9sat/ao1xEkREYPPj9hphk= @@ -992,6 +1001,7 @@ go.etcd.io/bbolt v1.3.3/go.mod h1:IbVyRI1SCnLcuJnV2u8VeU0CEYM7e686BmAb1XKL+uU= go.etcd.io/etcd v0.0.0-20191023171146-3cf2f69b5738/go.mod h1:dnLIgRNXwCJa5e+c6mIZCrds/GIG4ncV9HhK5PX7jPg= go.mongodb.org/mongo-driver v1.0.3/go.mod h1:u7ryQJ+DOzQmeO7zB6MHyr8jkEQvC8vH7qLUO4lqsUM= go.mongodb.org/mongo-driver v1.1.1/go.mod h1:u7ryQJ+DOzQmeO7zB6MHyr8jkEQvC8vH7qLUO4lqsUM= +go.mongodb.org/mongo-driver v1.1.2 h1:jxcFYjlkl8xaERsgLo+RNquI0epW6zuy/ZRQs6jnrFA= go.mongodb.org/mongo-driver v1.1.2/go.mod h1:u7ryQJ+DOzQmeO7zB6MHyr8jkEQvC8vH7qLUO4lqsUM= go.opencensus.io v0.15.0/go.mod h1:UffZAU+4sDEINUGP/B7UfBBkq4fqLu9zXAX7ke6CHW0= go.opencensus.io v0.21.0/go.mod h1:mSImk1erAIZhrmZN+AvHh14ztQfjbGwt4TtuofqLduU= diff --git a/pkg/test/files.go b/pkg/test/files.go index 4a5dbf08..f2a9e3ff 100644 --- a/pkg/test/files.go +++ b/pkg/test/files.go @@ -29,6 +29,8 @@ import ( // fails at the right times too. func ExpectMatchingDirectories(actualRoot, expectedRoot string) { Expect(actualRoot).To(BeADirectory()) + // every file and directory in the expected result should have a + // corresponding file or directory in the actual result filepath.Walk(expectedRoot, func(path string, info os.FileInfo, err error) error { if err != nil { return nil @@ -48,10 +50,13 @@ func ExpectMatchingDirectories(actualRoot, expectedRoot string) { } Expect(actualPath).To(BeARegularFile()) actualBytes, err := ioutil.ReadFile(actualPath) + Expect(err).ToNot(HaveOccurred()) expectedBytes, err := ioutil.ReadFile(path) - Expect(string(actualBytes)).To(Equal(string(expectedBytes))) + Expect(err).ToNot(HaveOccurred()) + Expect(string(actualBytes)).To(Equal(string(expectedBytes)), "file %q same as %q", actualPath, path) return nil }) + // every file and directory in the actual result should be expected filepath.Walk(actualRoot, func(path string, info os.FileInfo, err error) error { p := path[len(actualRoot):] // ignore emacs backups diff --git a/pkg/update/update.go b/pkg/update/imagepolicy.go similarity index 100% rename from pkg/update/update.go rename to pkg/update/imagepolicy.go diff --git a/pkg/update/setters.go b/pkg/update/setters.go new file mode 100644 index 00000000..be79630c --- /dev/null +++ b/pkg/update/setters.go @@ -0,0 +1,113 @@ +package update + +import ( + "fmt" + "sync" + + "github.com/go-openapi/spec" + "sigs.k8s.io/kustomize/kyaml/fieldmeta" + "sigs.k8s.io/kustomize/kyaml/kio" + "sigs.k8s.io/kustomize/kyaml/openapi" + "sigs.k8s.io/kustomize/kyaml/setters2" + + imagev1alpha1_reflect "github.com/fluxcd/image-reflector-controller/api/v1alpha1" +) + +const ( + // SetterShortHand is a shorthand that can be used to mark + // setters; instead of + // # { "$ref": "#/definitions/ + SetterShortHand = "$imagepolicy" +) + +func init() { + fieldmeta.SetShortHandRef(SetterShortHand) +} + +var ( + // used to serialise access to the global schema, which needs to + // be reset for each run + schemaMu = &sync.Mutex{} +) + +func resetSchema() { + openapi.ResetOpenAPI() + openapi.SuppressBuiltInSchemaUse() +} + +func UpdateWithSetters(inpath, outpath string, policies []imagev1alpha1_reflect.ImagePolicy) error { + // the OpenAPI schema is a package variable in kyaml/openapi. In + // lieu of being able to isolate invocations (per + // https://github.com/kubernetes-sigs/kustomize/issues/3058), I + // serialise access to it and reset it each time. + + // construct definitions + + // the format of the definitions expected is given here: + // https://github.com/kubernetes-sigs/kustomize/blob/master/kyaml/setters2/doc.go + // + // { + // "definitions": { + // "io.k8s.cli.setters.replicas": { + // "x-k8s-cli": { + // "setter": { + // "name": "replicas", + // "value": "4" + // } + // } + // } + // } + // } + // + // (there are consts in kyaml/fieldmeta with the + // prefixes). + // + // `fieldmeta.SetShortHandRef("$imagepolicy")` makes it possible + // to just use (e.g.,) + // + // image: foo:v1 # {"$imagepolicy": "automation-ns:foo"} + // + // to mark the fields at which to make replacements. A colon is + // used to separate namespace and name in the key, because a slash + // would be interpreted as part of the $ref path. + + defs := map[string]spec.Schema{} + for _, policy := range policies { + if policy.Status.LatestImage == "" { + continue + } + setterKey := fmt.Sprintf("%s:%s", policy.GetNamespace(), policy.GetName()) + schema := spec.StringProperty() + schema.Extensions = map[string]interface{}{} + schema.Extensions.Add(setters2.K8sCliExtensionKey, map[string]interface{}{ + "setter": map[string]string{ + "name": setterKey, + "value": policy.Status.LatestImage, + }, + }) + defs[fieldmeta.SetterDefinitionPrefix+setterKey] = *schema + } + + // get ready with the reader and writer + reader := &kio.LocalPackageReader{ + PackagePath: inpath, + IncludeSubpackages: true, + } + writer := &kio.LocalPackageWriter{ + PackagePath: outpath, + } + + pipeline := kio.Pipeline{ + Inputs: []kio.Reader{reader}, + Outputs: []kio.Writer{writer}, + Filters: []kio.Filter{kio.FilterAll(&setters2.Set{SetAll: true})}, + } + + // go! + schemaMu.Lock() + resetSchema() + openapi.AddDefinitions(defs) + err := pipeline.Execute() + schemaMu.Unlock() + return err +} diff --git a/pkg/update/testdata/setters/expected/marked.yaml b/pkg/update/testdata/setters/expected/marked.yaml new file mode 100644 index 00000000..4efafc52 --- /dev/null +++ b/pkg/update/testdata/setters/expected/marked.yaml @@ -0,0 +1,14 @@ +apiVersion: batch/v1beta1 +kind: CronJob +metadata: + name: foo + namespace: bar +spec: + schedule: "*/1 * * * *" + jobTemplate: + spec: + template: + spec: + containers: + - name: c + image: updated:v1.0.1 # {"$imagepolicy": "automation-ns:policy"} diff --git a/pkg/update/testdata/setters/expected/otherns.yaml b/pkg/update/testdata/setters/expected/otherns.yaml new file mode 100644 index 00000000..6fe2da05 --- /dev/null +++ b/pkg/update/testdata/setters/expected/otherns.yaml @@ -0,0 +1,11 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: foo + namespace: bar +spec: + template: + spec: + containers: + - name: c + image: user:v1.0.0 # {"$imagepolicy": "other-namespace:policy"} diff --git a/pkg/update/testdata/setters/expected/unmarked.yaml b/pkg/update/testdata/setters/expected/unmarked.yaml new file mode 100644 index 00000000..a2863ba8 --- /dev/null +++ b/pkg/update/testdata/setters/expected/unmarked.yaml @@ -0,0 +1,11 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: foo + namespace: bar +spec: + template: + spec: + containers: + - name: c + image: image:v1.0.0 # uses the same image. but should be left alone diff --git a/pkg/update/testdata/setters/original/marked.yaml b/pkg/update/testdata/setters/original/marked.yaml new file mode 100644 index 00000000..4032f9c9 --- /dev/null +++ b/pkg/update/testdata/setters/original/marked.yaml @@ -0,0 +1,14 @@ +apiVersion: batch/v1beta1 +kind: CronJob +metadata: + name: foo + namespace: bar +spec: + schedule: "*/1 * * * *" + jobTemplate: + spec: + template: + spec: + containers: + - name: c + image: image:v1.0.0 # {"$imagepolicy": "automation-ns:policy"} diff --git a/pkg/update/testdata/setters/original/otherns.yaml b/pkg/update/testdata/setters/original/otherns.yaml new file mode 100644 index 00000000..6fe2da05 --- /dev/null +++ b/pkg/update/testdata/setters/original/otherns.yaml @@ -0,0 +1,11 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: foo + namespace: bar +spec: + template: + spec: + containers: + - name: c + image: user:v1.0.0 # {"$imagepolicy": "other-namespace:policy"} diff --git a/pkg/update/testdata/setters/original/unmarked.yaml b/pkg/update/testdata/setters/original/unmarked.yaml new file mode 100644 index 00000000..a2863ba8 --- /dev/null +++ b/pkg/update/testdata/setters/original/unmarked.yaml @@ -0,0 +1,11 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: foo + namespace: bar +spec: + template: + spec: + containers: + - name: c + image: image:v1.0.0 # uses the same image. but should be left alone diff --git a/pkg/update/update_test.go b/pkg/update/update_test.go index 0557192e..8cd3c097 100644 --- a/pkg/update/update_test.go +++ b/pkg/update/update_test.go @@ -7,8 +7,10 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/fluxcd/image-automation-controller/pkg/test" + imagev1alpha1_reflect "github.com/fluxcd/image-reflector-controller/api/v1alpha1" ) func TestUpdate(t *testing.T) { @@ -16,13 +18,13 @@ func TestUpdate(t *testing.T) { RunSpecs(t, "Update suite") } -var _ = Describe("Update image everywhere", func() { +var _ = Describe("Update image everywhere from policy", func() { It("leaves a different image alone", func() { tmp, err := ioutil.TempDir("", "gotest") Expect(err).ToNot(HaveOccurred()) defer os.RemoveAll(tmp) Expect(UpdateImageEverywhere("testdata/leave/original", tmp, "notused", "notused:v1.0.1")).To(Succeed()) - test.ExpectMatchingDirectories("testdata/leave/expected", tmp) + test.ExpectMatchingDirectories(tmp, "testdata/leave/expected") }) It("replaces the given image", func() { @@ -30,7 +32,7 @@ var _ = Describe("Update image everywhere", func() { Expect(err).ToNot(HaveOccurred()) defer os.RemoveAll(tmp) Expect(UpdateImageEverywhere("testdata/replace/original", tmp, "used", "used:v1.1.0")).To(Succeed()) - test.ExpectMatchingDirectories("testdata/replace/expected", tmp) + test.ExpectMatchingDirectories(tmp, "testdata/replace/expected") }) It("keeps comments intact", func() { @@ -38,6 +40,29 @@ var _ = Describe("Update image everywhere", func() { Expect(err).ToNot(HaveOccurred()) defer os.RemoveAll(tmp) Expect(UpdateImageEverywhere("testdata/replace/commented", tmp, "used", "used:v1.1.0")).To(Succeed()) - test.ExpectMatchingDirectories("testdata/replace/commented-expected", tmp) + test.ExpectMatchingDirectories(tmp, "testdata/replace/commented-expected") + }) +}) + +var _ = Describe("Update image via kyaml setters2", func() { + 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 := []imagev1alpha1_reflect.ImagePolicy{ + imagev1alpha1_reflect.ImagePolicy{ + ObjectMeta: metav1.ObjectMeta{ // name matches marker used in testdata/setters/{original,expected} + Namespace: "automation-ns", + Name: "policy", + }, + Status: imagev1alpha1_reflect.ImagePolicyStatus{ + LatestImage: "updated:v1.0.1", + }, + }, + } + + Expect(UpdateWithSetters("testdata/setters/original", tmp, policies)).To(Succeed()) + test.ExpectMatchingDirectories(tmp, "testdata/setters/expected") }) }) From 4006b6d5c44e17e53da4cb6aafeda55588c2dd58 Mon Sep 17 00:00:00 2001 From: Michael Bridgen Date: Tue, 6 Oct 2020 13:04:28 +0100 Subject: [PATCH 07/10] Use UpdateWithSetters in controller (and update the test to suit) --- .../imageupdateautomation_controller.go | 8 ++--- .../appconfig-setters-expected/deploy.yaml | 10 ++++++ controllers/update_test.go | 33 +++++++++++++------ go.mod | 1 + go.sum | 8 +++++ 5 files changed, 46 insertions(+), 14 deletions(-) create mode 100644 controllers/testdata/appconfig-setters-expected/deploy.yaml diff --git a/controllers/imageupdateautomation_controller.go b/controllers/imageupdateautomation_controller.go index 2ea0bc30..03bf370a 100644 --- a/controllers/imageupdateautomation_controller.go +++ b/controllers/imageupdateautomation_controller.go @@ -145,8 +145,8 @@ func (r *ImageUpdateAutomationReconciler) Reconcile(req ctrl.Request) (ctrl.Resu } case updateStrat.Setters != nil: // For setters we first want to compile a list of _all_ the - // policies (maybe in the future this could be filtered by the - // automation object). + // policies in the same namespace (maybe in the future this + // could be filtered by the automation object). var policies imagev1alpha1_reflect.ImagePolicyList if err := r.List(ctx, &policies, &client.ListOptions{Namespace: req.NamespacedName.Namespace}); err != nil { return ctrl.Result{}, err @@ -407,6 +407,6 @@ func updateAccordingToImagePolicy(ctx context.Context, path string, policy *imag // updateAccordingToSetters updates files under the root by treating // the given image policies as kyaml setters. -func updateAccordingToSetters(ctx context.Context, root string, policies []imagev1alpha1_reflect.ImagePolicy) error { - return nil +func updateAccordingToSetters(ctx context.Context, path string, policies []imagev1alpha1_reflect.ImagePolicy) error { + return update.UpdateWithSetters(path, path, policies) } diff --git a/controllers/testdata/appconfig-setters-expected/deploy.yaml b/controllers/testdata/appconfig-setters-expected/deploy.yaml new file mode 100644 index 00000000..924875cf --- /dev/null +++ b/controllers/testdata/appconfig-setters-expected/deploy.yaml @@ -0,0 +1,10 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: test +spec: + template: + spec: + containers: + - name: hello + image: helloworld:1.0.1 # SETTER_SITE diff --git a/controllers/update_test.go b/controllers/update_test.go index 8e7e0594..cbe531b8 100644 --- a/controllers/update_test.go +++ b/controllers/update_test.go @@ -35,12 +35,14 @@ import ( "github.com/go-git/go-git/v5/storage/memory" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + "github.com/otiai10/copy" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" imagev1alpha1 "github.com/fluxcd/image-automation-controller/api/v1alpha1" "github.com/fluxcd/image-automation-controller/pkg/test" + "github.com/fluxcd/image-automation-controller/pkg/update" imagev1alpha1_reflect "github.com/fluxcd/image-reflector-controller/api/v1alpha1" sourcev1alpha1 "github.com/fluxcd/source-controller/api/v1alpha1" ) @@ -279,13 +281,8 @@ var _ = Describe("ImageUpdateAutomation", func() { ReferenceName: plumbing.NewBranchReferenceName(defaultBranch), }) Expect(err).ToNot(HaveOccurred()) - // NB this requires knowledge of what's in the git - // repo, so a little brittle - deployment := filepath.Join(tmp, "deploy.yaml") - filebytes, err := ioutil.ReadFile(deployment) - Expect(err).NotTo(HaveOccurred()) - newfilebytes := bytes.ReplaceAll(filebytes, []byte("SETTER_SITE"), []byte(setterName(policyKey))) - Expect(ioutil.WriteFile(deployment, newfilebytes, os.FileMode(0666))).To(Succeed()) + + replaceMarker(tmp, policyKey) worktree, err := repo.Worktree() Expect(err).ToNot(HaveOccurred()) _, err = worktree.Add("deploy.yaml") @@ -351,19 +348,35 @@ var _ = Describe("ImageUpdateAutomation", func() { Expect(err).ToNot(HaveOccurred()) defer os.RemoveAll(tmp) + expected, err := ioutil.TempDir("", "gotest-imageauto-expected") + Expect(err).ToNot(HaveOccurred()) + defer os.RemoveAll(expected) + copy.Copy("testdata/appconfig-setters-expected", expected) + replaceMarker(expected, policyKey) + _, err = git.PlainClone(tmp, false, &git.CloneOptions{ URL: repoURL, ReferenceName: plumbing.NewBranchReferenceName(defaultBranch), }) Expect(err).ToNot(HaveOccurred()) - test.ExpectMatchingDirectories(tmp, "testdata/appconfig-setters-expected") + test.ExpectMatchingDirectories(tmp, expected) }) }) }) }) -func setterName(name types.NamespacedName) string { - return fmt.Sprintf("#/image/%s/%s", name.Namespace, name.Name) +func replaceMarker(path string, policyKey types.NamespacedName) { + // NB this requires knowledge of what's in the git + // repo, so a little brittle + deployment := filepath.Join(path, "deploy.yaml") + filebytes, err := ioutil.ReadFile(deployment) + Expect(err).NotTo(HaveOccurred()) + newfilebytes := bytes.ReplaceAll(filebytes, []byte("SETTER_SITE"), []byte(setterRef(policyKey))) + Expect(ioutil.WriteFile(deployment, newfilebytes, os.FileMode(0666))).To(Succeed()) +} + +func setterRef(name types.NamespacedName) string { + return fmt.Sprintf(`{"%s": "%s:%s"}`, update.SetterShortHand, name.Namespace, name.Name) } func waitForNewHead(repo *git.Repository) { diff --git a/go.mod b/go.mod index cc6a3887..0bcf9f06 100644 --- a/go.mod +++ b/go.mod @@ -13,6 +13,7 @@ require ( github.com/google/go-containerregistry v0.1.1 github.com/onsi/ginkgo v1.12.1 github.com/onsi/gomega v1.10.1 + github.com/otiai10/copy v1.2.0 k8s.io/api v0.18.6 k8s.io/apimachinery v0.18.6 k8s.io/client-go v0.18.6 diff --git a/go.sum b/go.sum index f379d722..26a849ef 100644 --- a/go.sum +++ b/go.sum @@ -800,6 +800,14 @@ github.com/opencontainers/runc v0.1.1 h1:GlxAyO6x8rfZYN9Tt0Kti5a/cP41iuiO2yYT0IJ github.com/opencontainers/runc v0.1.1/go.mod h1:qT5XzbpPznkRYVz/mWwUaVBUv2rmF59PVA73FjuZG0U= github.com/opencontainers/runtime-spec v0.1.2-0.20190507144316-5b71a03e2700/go.mod h1:jwyrGlmzljRJv/Fgzds9SsS/C5hL+LL3ko9hs6T5lQ0= github.com/opencontainers/runtime-tools v0.0.0-20181011054405-1d69bd0f9c39/go.mod h1:r3f7wjNzSs2extwzU3Y+6pKfobzPh+kKFJ3ofN+3nfs= +github.com/otiai10/copy v1.2.0 h1:HvG945u96iNadPoG2/Ja2+AUJeW5YuFQMixq9yirC+k= +github.com/otiai10/copy v1.2.0/go.mod h1:rrF5dJ5F0t/EWSYODDu4j9/vEeYHMkc8jt0zJChqQWw= +github.com/otiai10/curr v0.0.0-20150429015615-9b4961190c95/go.mod h1:9qAhocn7zKJG+0mI8eUu6xqkFDYS2kb2saOteoSB3cE= +github.com/otiai10/curr v1.0.0 h1:TJIWdbX0B+kpNagQrjgq8bCMrbhiuX73M2XwgtDMoOI= +github.com/otiai10/curr v1.0.0/go.mod h1:LskTG5wDwr8Rs+nNQ+1LlxRjAtTZZjtJW4rMXl6j4vs= +github.com/otiai10/mint v1.3.0/go.mod h1:F5AjcsTsWUqX+Na9fpHb52P8pcRX2CI6A3ctIT91xUo= +github.com/otiai10/mint v1.3.1 h1:BCmzIS3n71sGfHB5NMNDB3lHYPz8fWSkCAErHed//qc= +github.com/otiai10/mint v1.3.1/go.mod h1:/yxELlJQ0ufhjUwhshSj+wFjZ78CnZ48/1wtmBH1OTc= github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc= github.com/paulmach/orb v0.1.3/go.mod h1:VFlX/8C+IQ1p6FTRRKzKoOPJnvEtA5G0Veuqwbu//Vk= github.com/pborman/uuid v1.2.0/go.mod h1:X/NO0urCmaxf9VXbdlT7C2Yzkj2IKimNn4k+gtPdI/k= From 9e486739e90bb6c777fa30f33e1a6d14abb3bf76 Mon Sep 17 00:00:00 2001 From: Michael Bridgen Date: Fri, 9 Oct 2020 16:57:42 +0100 Subject: [PATCH 08/10] Adapt instructions in README to use setters --- README.md | 88 ++++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 68 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index 7d3501f6..fb930f6a 100644 --- a/README.md +++ b/README.md @@ -32,31 +32,34 @@ the GitRepository kind, and doesn't need the source-controller itself. If you're not already using the [GitOps toolkit][gotk], you can just install the custom resource definition for GitRepository: - kubectl apply -f https://raw.githubusercontent.com/fluxcd/source-controller/master/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml + kubectl apply -f https://raw.githubusercontent.com/fluxcd/source-controller/v0.0.18/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml **To install the image reflector controller** This controller relies on the image reflector controller. A working configuration for the latter can be applied straight from the GitHub -repository (NB `-k`): +repository: - kubectl apply -k github.com/fluxcd/image-reflector-controller/config/default + kustomize build github.com/fluxcd/image-reflector-controller//config/default/?ref=main | kubectl apply -f- ### Installing the automation controller You can apply a working configuration directly from GitHub: - kubectl apply -k github.com/fluxcd/image-automation-controller/config/default + kustomize build github.com/fluxcd/image-automation-controller//config/default?ref=main | kubectl apply -f- or, in a clone of this repository, make docker-build deploy +You will need to do the latter if you're trying the controller on a +branch other than `main`. + ## How to use it Here is a quick example of configuring an automation. I'm going to use -[cuttlefacts-app][cuttlefacts-app-repo] because it's minimal and -thereby, easy to follow. +[cuttlefacts-app][cuttlefacts-app-repo] because it's minimal and easy +to follow. ### Image policy @@ -92,7 +95,7 @@ kind: ImagePolicy metadata: name: app-policy spec: - imageRepository: + imageRepositoryRef: name: app-image policy: semver: @@ -111,7 +114,7 @@ NAME LATESTIMAGE app-policy cuttlefacts/cuttlefacts-app:1.0.0 ``` -### Git repository and automation +### Creating the git repository object You need a writable git repository, so fork [`cuttlefacts-app`][cuttlefacts-app-repo] to your own account, and @@ -176,9 +179,9 @@ EOF $ $EDITOR repo.yaml ``` -Create the repository; be aware that unless you're running the full -GitOps toolkit suite, there will be no controller acting on it (and -doesn't need to be, for the purpose of this run-through). +Create the repository object; be aware that unless you're running the +full GitOps toolkit suite, there will be no controller acting on it +(and doesn't need to be, for the purpose of this run-through). ```bash $ kubectl apply -f repo.yaml @@ -188,9 +191,50 @@ NAME URL READY STATU cuttlefacts-repo ssh://git@github.com/squaremo/cuttlefacts-app 9s ``` +### Adding a marker to the YAML to update + +To tell the controller what to update, you add some markers to the +files to be updated. Each marker says which field to update, and which +image policy to use for the new value. + +In this case, it's the image in the deployment that needs to be +updated, with the latest image from the image policy made +earlier. Edit the file either locally or through GitHub, and add a +marker to the file `deploy/deployment.yaml` at the line with the image +field, `image: cuttlefacts/cuttlefacts-app`. The surrounding lines +look like this: + +``` + containers: + - name: server + image: cuttlefacts/cuttlefacts-app + imagePullPolicy: IfNotPresent +``` + +With the marker, they look like this: + +``` + containers: + - name: server + image: cuttlefacts/cuttlefacts-app # {"$imagepolicy": "default:app-policy"} + imagePullPolicy: IfNotPresent +``` + +The marker is a comment at the end of the `image:` line, with a JSON +value (so remember the double quotes), naming the image policy object +to use for the value. A `:` character separates the namespace from the +name of the `ImagePolicy` object. (The namespace is default because it +wasn't specified in the manifest (`policy.yaml`) or when it was +applied.) + +Commit that change, and push it if you made the commit locally. + +### Creating the automation object + Now we have an image policy, which calculates the most recent image, -and a git repository to update -- the last ingredient is to tie them -together with an `ImageUpdateAutomation` resource: +and a git repository to update, and we've marked the field to update, +in a file. The last ingredient is to tie these together with an +`ImageUpdateAutomation` resource: ``` $ cat > update.yaml < Date: Tue, 13 Oct 2020 10:51:37 +0100 Subject: [PATCH 09/10] Rewrite directory comparison test helper It's difficult to test the files comparison if it contains all its own assertions, since you can only test successes. This commit rewrites the helper to use a func without assertions, that can be more roundly tested. --- pkg/test/files.go | 139 +++++++++++++++--- pkg/test/files_test.go | 23 ++- .../testdata/diff/a/different/content.yaml | 2 + pkg/test/testdata/diff/a/dirfile | 1 + pkg/test/testdata/diff/a/only/here.yaml | 1 + pkg/test/testdata/diff/a/onlyhere.yaml | 1 + .../testdata/diff/b/different/content.yaml | 2 + pkg/test/testdata/diff/b/dirfile/.ignore | 1 + 8 files changed, 149 insertions(+), 21 deletions(-) create mode 100644 pkg/test/testdata/diff/a/different/content.yaml create mode 100644 pkg/test/testdata/diff/a/dirfile create mode 100644 pkg/test/testdata/diff/a/only/here.yaml create mode 100644 pkg/test/testdata/diff/a/onlyhere.yaml create mode 100644 pkg/test/testdata/diff/b/different/content.yaml create mode 100644 pkg/test/testdata/diff/b/dirfile/.ignore diff --git a/pkg/test/files.go b/pkg/test/files.go index f2a9e3ff..afa8969a 100644 --- a/pkg/test/files.go +++ b/pkg/test/files.go @@ -29,44 +29,143 @@ import ( // fails at the right times too. func ExpectMatchingDirectories(actualRoot, expectedRoot string) { Expect(actualRoot).To(BeADirectory()) - // every file and directory in the expected result should have a - // corresponding file or directory in the actual result - filepath.Walk(expectedRoot, func(path string, info os.FileInfo, err error) error { + 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) + // these are enumerated, so that the output is the actual difference + for _, diff := range different { + diff.FailedExpectation() + } +} + +type Diff interface { + Path() string + FailedExpectation() +} + +type contentdiff struct { + path, actual, expected string +} + +func (d contentdiff) Path() string { + return d.path +} + +// Run an expectation that will fail, giving an appropriate error +func (d contentdiff) FailedExpectation() { + Expect(d.actual).To(Equal(d.expected)) +} + +type dirfile struct { + abspath, path string + expectedRegularFile bool +} + +func (d dirfile) Path() string { + return d.path +} + +func (d dirfile) FailedExpectation() { + if d.expectedRegularFile { + Expect(d.path).To(BeARegularFile()) + } else { + Expect(d.path).To(BeADirectory()) + } +} + +// DiffDirectories walks the two given directories, recursively, and +// reports relative paths for any files that are: +// +// (in actual but not expected, in expected but not actual, in both but different) +// +// It ignores dot directories (e.g., `.git/`) and Emacs backups (e.g., +// `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) { + filepath.Walk(expected, func(expectedPath string, expectedInfo os.FileInfo, err error) error { if err != nil { - return nil + panic(err) } // ignore emacs backups - if strings.HasSuffix(path, "~") { + if strings.HasSuffix(expectedPath, "~") { return nil } - relPath := path[len(expectedRoot):] - actualPath := filepath.Join(actualRoot, relPath) - if info.IsDir() { - if strings.HasPrefix(filepath.Base(path), ".") { + relPath := expectedPath[len(expected):] + actualPath := filepath.Join(actual, relPath) + // ignore dotfiles + if strings.HasPrefix(filepath.Base(expectedPath), ".") { + if expectedInfo.IsDir() { return filepath.SkipDir } - Expect(actualPath).To(BeADirectory()) return nil } - Expect(actualPath).To(BeARegularFile()) + + actualInfo, err := os.Stat(actualPath) + switch { + case err == nil: + break + case os.IsNotExist(err): + expectedonly = append(expectedonly, relPath) + 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()}) + return nil + } + + // both regular files + actualBytes, err := ioutil.ReadFile(actualPath) - Expect(err).ToNot(HaveOccurred()) - expectedBytes, err := ioutil.ReadFile(path) - Expect(err).ToNot(HaveOccurred()) - Expect(string(actualBytes)).To(Equal(string(expectedBytes)), "file %q same as %q", actualPath, path) + if err != nil { + panic(err) + } + expectedBytes, err := ioutil.ReadFile(expectedPath) + if err != nil { + panic(err) + } + if string(actualBytes) != string(expectedBytes) { + different = append(different, contentdiff{path: relPath, actual: string(actualBytes), expected: string(expectedBytes)}) + } return nil }) + // every file and directory in the actual result should be expected - filepath.Walk(actualRoot, func(path string, info os.FileInfo, err error) error { - p := path[len(actualRoot):] + filepath.Walk(actual, func(actualPath string, actualInfo os.FileInfo, err error) error { + if err != nil { + panic(err) + } + relPath := actualPath[len(actual):] // ignore emacs backups - if strings.HasSuffix(p, "~") { + if strings.HasSuffix(actualPath, "~") { return nil } - if info.IsDir() && strings.HasPrefix(filepath.Base(p), ".") { + // skip dotdirs + if actualInfo.IsDir() && strings.HasPrefix(filepath.Base(actualPath), ".") { return filepath.SkipDir } - Expect(filepath.Join(expectedRoot, p)).To(BeAnExistingFile()) + // 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): + actualonly = append(actualonly, relPath) + default: + panic(err) + } return nil }) + return } diff --git a/pkg/test/files_test.go b/pkg/test/files_test.go index 1301d021..02643ab6 100644 --- a/pkg/test/files_test.go +++ b/pkg/test/files_test.go @@ -28,7 +28,7 @@ func TestFiles(t *testing.T) { RunSpecs(t, "Files comparison helper") } -var _ = Describe("Test helper", func() { +var _ = Describe("when no differences", func() { It("matches when given the same directory", func() { ExpectMatchingDirectories("testdata/base", "testdata/base") }) @@ -36,3 +36,24 @@ var _ = Describe("Test helper", 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"})) + }) +}) diff --git a/pkg/test/testdata/diff/a/different/content.yaml b/pkg/test/testdata/diff/a/different/content.yaml new file mode 100644 index 00000000..c381fd53 --- /dev/null +++ b/pkg/test/testdata/diff/a/different/content.yaml @@ -0,0 +1,2 @@ +# This file has different content here than in the corresponding file +# in b/. diff --git a/pkg/test/testdata/diff/a/dirfile b/pkg/test/testdata/diff/a/dirfile new file mode 100644 index 00000000..890a1e9e --- /dev/null +++ b/pkg/test/testdata/diff/a/dirfile @@ -0,0 +1 @@ +# This is a file here, and a directory in the compared root diff --git a/pkg/test/testdata/diff/a/only/here.yaml b/pkg/test/testdata/diff/a/only/here.yaml new file mode 100644 index 00000000..d61eecef --- /dev/null +++ b/pkg/test/testdata/diff/a/only/here.yaml @@ -0,0 +1 @@ +# this file exists for the purpose of diffing directories diff --git a/pkg/test/testdata/diff/a/onlyhere.yaml b/pkg/test/testdata/diff/a/onlyhere.yaml new file mode 100644 index 00000000..d61eecef --- /dev/null +++ b/pkg/test/testdata/diff/a/onlyhere.yaml @@ -0,0 +1 @@ +# this file exists for the purpose of diffing directories diff --git a/pkg/test/testdata/diff/b/different/content.yaml b/pkg/test/testdata/diff/b/different/content.yaml new file mode 100644 index 00000000..86d7f589 --- /dev/null +++ b/pkg/test/testdata/diff/b/different/content.yaml @@ -0,0 +1,2 @@ +# This file has different content here to the corresponding file in +# a/. diff --git a/pkg/test/testdata/diff/b/dirfile/.ignore b/pkg/test/testdata/diff/b/dirfile/.ignore new file mode 100644 index 00000000..511466c6 --- /dev/null +++ b/pkg/test/testdata/diff/b/dirfile/.ignore @@ -0,0 +1 @@ +Just here to preserve the directory. From 76da2079e669bbb8fda6dcef1256aa1e018dd990 Mon Sep 17 00:00:00 2001 From: Michael Bridgen Date: Tue, 13 Oct 2020 20:54:51 +0100 Subject: [PATCH 10/10] Supply :tag and :name setters for each image It will be useful, for kustomizations e.g., to be able to set just the tag or just the name (repository). This commit adds setters for those to the schema -- they have the name of the image setter plus a suffix of `:tag` or `:name`. For example: newName: ubuntu # {"$imagepolicy": "ns:policy:name"} newTag: 18.10 # {"$imagepolicy": "ns:policy:tag"} --- pkg/update/setters.go | 47 +++++++++++++++---- .../setters/expected/kustomization.yaml | 9 ++++ .../setters/original/kustomization.yaml | 9 ++++ 3 files changed, 55 insertions(+), 10 deletions(-) create mode 100644 pkg/update/testdata/setters/expected/kustomization.yaml create mode 100644 pkg/update/testdata/setters/original/kustomization.yaml diff --git a/pkg/update/setters.go b/pkg/update/setters.go index be79630c..fe152427 100644 --- a/pkg/update/setters.go +++ b/pkg/update/setters.go @@ -5,6 +5,7 @@ import ( "sync" "github.com/go-openapi/spec" + "github.com/google/go-containerregistry/pkg/name" "sigs.k8s.io/kustomize/kyaml/fieldmeta" "sigs.k8s.io/kustomize/kyaml/kio" "sigs.k8s.io/kustomize/kyaml/openapi" @@ -76,16 +77,30 @@ func UpdateWithSetters(inpath, outpath string, policies []imagev1alpha1_reflect. if policy.Status.LatestImage == "" { continue } - setterKey := fmt.Sprintf("%s:%s", policy.GetNamespace(), policy.GetName()) - schema := spec.StringProperty() - schema.Extensions = map[string]interface{}{} - schema.Extensions.Add(setters2.K8sCliExtensionKey, map[string]interface{}{ - "setter": map[string]string{ - "name": setterKey, - "value": policy.Status.LatestImage, - }, - }) - defs[fieldmeta.SetterDefinitionPrefix+setterKey] = *schema + // Using strict validation would mean any image that omits the + // registry would be rejected, so that can't be used + // here. Using _weak_ validation means that defaults will be + // filled in. Usually this would mean the tag would end up + // being `latest` if empty in the input; but I'm assuming here + // that the policy won't have a tagless ref. + image := policy.Status.LatestImage + ref, err := name.ParseReference(image, name.WeakValidation) + if err != nil { + return fmt.Errorf("encountered invalid image ref %q: %w", policy.Status.LatestImage, err) + } + tag := ref.Identifier() + // annoyingly, neither the library imported above, nor an + // alternative, I found will yield the original image name; + // this is an easy way to get it + name := image[:len(tag)+1] + + imageSetter := fmt.Sprintf("%s:%s", policy.GetNamespace(), policy.GetName()) + defs[fieldmeta.SetterDefinitionPrefix+imageSetter] = setterSchema(imageSetter, policy.Status.LatestImage) + tagSetter := imageSetter + ":tag" + defs[fieldmeta.SetterDefinitionPrefix+tagSetter] = setterSchema(tagSetter, tag) + // Context().Name() gives the image repository _as supplied_ + nameSetter := imageSetter + ":name" + defs[fieldmeta.SetterDefinitionPrefix+nameSetter] = setterSchema(nameSetter, name) } // get ready with the reader and writer @@ -111,3 +126,15 @@ func UpdateWithSetters(inpath, outpath string, policies []imagev1alpha1_reflect. schemaMu.Unlock() return err } + +func setterSchema(name, value string) spec.Schema { + schema := spec.StringProperty() + schema.Extensions = map[string]interface{}{} + schema.Extensions.Add(setters2.K8sCliExtensionKey, map[string]interface{}{ + "setter": map[string]string{ + "name": name, + "value": value, + }, + }) + return *schema +} diff --git a/pkg/update/testdata/setters/expected/kustomization.yaml b/pkg/update/testdata/setters/expected/kustomization.yaml new file mode 100644 index 00000000..ab72d3cc --- /dev/null +++ b/pkg/update/testdata/setters/expected/kustomization.yaml @@ -0,0 +1,9 @@ +# This is not intended to be a working kustomization +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- unimportant.yaml +images: +- name: container + newName: updated # {"$imagepolicy": "automation-ns:policy:name"} + newTag: v1.0.1 # {"$imagepolicy": "automation-ns:policy:tag"} diff --git a/pkg/update/testdata/setters/original/kustomization.yaml b/pkg/update/testdata/setters/original/kustomization.yaml new file mode 100644 index 00000000..4245dd2f --- /dev/null +++ b/pkg/update/testdata/setters/original/kustomization.yaml @@ -0,0 +1,9 @@ +# This is not intended to be a working kustomization +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- unimportant.yaml +images: +- name: container + newName: replaced # {"$imagepolicy": "automation-ns:policy:name"} + newTag: v1 # {"$imagepolicy": "automation-ns:policy:tag"}