diff --git a/api/v1beta2/condition_types.go b/api/v1beta2/condition_types.go index 910b706f..c4eb81e4 100644 --- a/api/v1beta2/condition_types.go +++ b/api/v1beta2/condition_types.go @@ -36,4 +36,7 @@ const ( // InvalidPolicySelectorReason represents an invalid policy selector. InvalidPolicySelectorReason string = "InvalidPolicySelector" + + // RemovedTemplateFieldReason represents usage of removed template field. + RemovedTemplateFieldReason string = "RemovedTemplateField" ) diff --git a/api/v1beta2/git.go b/api/v1beta2/git.go index 7fc647a6..59e8bcc0 100644 --- a/api/v1beta2/git.go +++ b/api/v1beta2/git.go @@ -65,6 +65,7 @@ type CommitSpec struct { SigningKey *SigningKey `json:"signingKey,omitempty"` // MessageTemplate provides a template for the commit message, // into which will be interpolated the details of the change made. + // Note: The `Updated` template field has been removed. Use `Changed` instead. // +optional MessageTemplate string `json:"messageTemplate,omitempty"` diff --git a/config/crd/bases/image.toolkit.fluxcd.io_imageupdateautomations.yaml b/config/crd/bases/image.toolkit.fluxcd.io_imageupdateautomations.yaml index 166b539d..a7fca845 100644 --- a/config/crd/bases/image.toolkit.fluxcd.io_imageupdateautomations.yaml +++ b/config/crd/bases/image.toolkit.fluxcd.io_imageupdateautomations.yaml @@ -440,6 +440,7 @@ spec: description: |- MessageTemplate provides a template for the commit message, into which will be interpolated the details of the change made. + Note: The `Updated` template field has been removed. Use `Changed` instead. type: string messageTemplateValues: additionalProperties: diff --git a/docs/api/v1beta2/image-automation.md b/docs/api/v1beta2/image-automation.md index ab12d64b..ba8501ec 100644 --- a/docs/api/v1beta2/image-automation.md +++ b/docs/api/v1beta2/image-automation.md @@ -67,7 +67,8 @@ string (Optional)

MessageTemplate provides a template for the commit message, -into which will be interpolated the details of the change made.

+into which will be interpolated the details of the change made. +Note: The Updated template field has been removed. Use Changed instead.

diff --git a/docs/spec/v1beta2/imageupdateautomations.md b/docs/spec/v1beta2/imageupdateautomations.md index 648ea232..81d7d102 100644 --- a/docs/spec/v1beta2/imageupdateautomations.md +++ b/docs/spec/v1beta2/imageupdateautomations.md @@ -374,12 +374,12 @@ spec: Automated image update by Flux ``` -**Deprecation Note:** The `Updated` template data available in v1beta1 API is -deprecated. `Changed` template data is recommended for template data, as it -accommodates for all the updates, including partial updates to just the image -name or the tag, not just full image with name and tag update. The old templates -will continue to work in v1beta2 API as `Updated` has not been removed yet. In -the next API version, `Updated` may be removed. +**Removal Note:** The `Updated` template data has been removed from the API. +Use `Changed` template data instead, as it accommodates for all the updates, +including partial updates to just the image name or the tag, not just full image +with name and tag update. Templates using `Updated` will result in an error and +the ImageUpdateAutomation will be marked as Stalled. + The message template also has access to the data related to the changes made by the automation. The template is a [Go text template][go-text-template]. The data @@ -393,14 +393,14 @@ type TemplateData struct { AutomationObject struct { Name, Namespace string } - Changed update.ResultV2 + Changed update.Result Values map[string]string } -// ResultV2 contains the file changes made during the update. It contains +// Result contains the file changes made during the update. It contains // details about the exact changes made to the files and the objects in them. It // has a nested structure file->objects->changes. -type ResultV2 struct { +type Result struct { FileChanges map[string]ObjectChanges } @@ -427,10 +427,10 @@ over the changed objects and changes: ```go // Changes returns all the changes that were made in at least one update. -func (r ResultV2) Changes() []Change +func (r Result) Changes() []Change // Objects returns ObjectChanges, regardless of which file they appear in. -func (r ResultV2) Objects() ObjectChanges +func (r Result) Objects() ObjectChanges ``` Example of using the methods in a template: diff --git a/internal/controller/imageupdateautomation_controller.go b/internal/controller/imageupdateautomation_controller.go index d3bbc0a5..624eac40 100644 --- a/internal/controller/imageupdateautomation_controller.go +++ b/internal/controller/imageupdateautomation_controller.go @@ -481,6 +481,14 @@ func (r *ImageUpdateAutomationReconciler) reconcile(ctx context.Context, sp *pat pushResult, err = sm.CommitAndPush(ctx, obj, policyResult, pushCfg...) if err != nil { + // Check if error is due to removed template field usage. + // Set Stalled condition and return nil error to prevent requeue, allowing user to fix template. + if errors.Is(err, source.ErrRemovedTemplateField) { + conditions.MarkStalled(obj, imagev1.RemovedTemplateFieldReason, "%s", err) + result, retErr = ctrl.Result{}, nil + return + } + e := fmt.Errorf("failed to update source: %w", err) conditions.MarkFalse(obj, meta.ReadyCondition, imagev1.GitOperationFailedReason, "%s", e) result, retErr = ctrl.Result{}, e diff --git a/internal/controller/imageupdateautomation_controller_test.go b/internal/controller/imageupdateautomation_controller_test.go index f5705908..f48607eb 100644 --- a/internal/controller/imageupdateautomation_controller_test.go +++ b/internal/controller/imageupdateautomation_controller_test.go @@ -71,12 +71,12 @@ const ( Automation: {{ .AutomationObject }} Files: -{{ range $filename, $_ := .Updated.Files -}} +{{ range $filename, $_ := .Changed.FileChanges -}} - {{ $filename }} {{ end -}} Objects: -{{ range $resource, $_ := .Updated.Objects -}} +{{ range $resource, $_ := .Changed.Objects -}} {{ if eq $resource.Kind "Deployment" -}} - {{ $resource.Kind | lower }} {{ $resource.Name | lower }} {{ else -}} @@ -85,8 +85,10 @@ Objects: {{ end -}} Images: -{{ range .Updated.Images -}} -- {{.}} ({{.Policy.Name}}) +{{ range .Changed.Changes -}} +{{ if .Setter -}} +- {{.NewValue}} ({{.Setter}}) +{{ end -}} {{ end -}} ` testCommitMessageFmt = `Commit summary @@ -98,7 +100,7 @@ Files: Objects: - deployment test Images: -- helloworld:v1.0.0 (%s) +- helloworld:v1.0.0 (%s:%s) ` ) @@ -727,7 +729,7 @@ func TestImageUpdateAutomationReconciler_commitMessage(t *testing.T) { name: "template with update Result", template: testCommitTemplate, wantCommitMsg: func(policyName, policyNS string) string { - return fmt.Sprintf(testCommitMessageFmt, policyNS, policyName) + return fmt.Sprintf(testCommitMessageFmt, policyNS, policyNS, policyName) }, }, { @@ -841,6 +843,134 @@ Automation: %s/update-test } } +// TestImageUpdateAutomationReconciler_removedTemplateField tests removed template field usage (.Updated and .Changed.ImageResult). +func TestImageUpdateAutomationReconciler_removedTemplateField(t *testing.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" + + testCases := []struct { + name string + template string + expectedErrMsg string + }{ + { + name: ".Updated field", + template: `Commit summary + +Automation: {{ .AutomationObject }} + +Files: +{{ range $filename, $_ := .Updated.Files -}} +- {{ $filename }} +{{ end -}} + +Objects: +{{ range $resource, $_ := .Updated.Objects -}} +{{ if eq $resource.Kind "Deployment" -}} +- {{ $resource.Kind | lower }} {{ $resource.Name | lower }} +{{ else -}} +- {{ $resource.Kind }} {{ $resource.Name }} +{{ end -}} +{{ end -}} + +Images: +{{ range .Updated.Images -}} +- {{.}} ({{.Policy.Name}}) +{{ end -}} +`, + expectedErrMsg: "template uses removed '.Updated' field", + }, + { + name: ".Changed.ImageResult field", + template: `Commit summary + +Automation: {{ .AutomationObject }} + +Files: +{{ range $filename, $_ := .Changed.ImageResult.Files -}} +- {{ $filename }} +{{ end -}} + +Objects: +{{ range $resource, $_ := .Changed.ImageResult.Objects -}} +- {{ $resource.Kind }} {{ $resource.Name }} +{{ end -}} + +Images: +{{ range .Changed.ImageResult.Images -}} +- {{.}} ({{.Policy.Name}}) +{{ end -}} +`, + expectedErrMsg: "template uses removed '.Changed.ImageResult' field", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + ctx := context.TODO() + + namespace, err := testEnv.CreateNamespace(ctx, "image-auto-test") + g.Expect(err).ToNot(HaveOccurred()) + defer func() { g.Expect(testEnv.Delete(ctx, namespace)).To(Succeed()) }() + + testWithRepoAndImagePolicy( + ctx, g, testEnv, namespace.Name, fixture, policySpec, latest, + func(g *WithT, s repoAndPolicyArgs, repoURL string, localRepo *extgogit.Repository) { + policyKey := types.NamespacedName{ + Name: s.imagePolicyName, + Namespace: s.namespace, + } + _ = testutil.CommitInRepo(ctx, g, repoURL, s.branch, originRemote, "Install setter marker", func(tmp string) { + g.Expect(testutil.ReplaceMarker(filepath.Join(tmp, "deploy.yaml"), policyKey)).To(Succeed()) + }) + + preChangeCommitId := testutil.CommitIdFromBranch(localRepo, s.branch) + waitForNewHead(g, localRepo, s.branch, preChangeCommitId) + preChangeCommitId = testutil.CommitIdFromBranch(localRepo, s.branch) + + updateStrategy := &imagev1.UpdateStrategy{ + Strategy: imagev1.UpdateStrategySetters, + } + err := createImageUpdateAutomation(ctx, testEnv, "update-test", s.namespace, s.gitRepoName, s.gitRepoNamespace, s.branch, s.branch, "", tc.template, "", updateStrategy) + g.Expect(err).ToNot(HaveOccurred()) + defer func() { + g.Expect(deleteImageUpdateAutomation(ctx, testEnv, "update-test", s.namespace)).To(Succeed()) + }() + + imageUpdateKey := types.NamespacedName{ + Namespace: s.namespace, + Name: "update-test", + } + + g.Eventually(func() bool { + var imageUpdate imagev1.ImageUpdateAutomation + _ = testEnv.Get(context.TODO(), imageUpdateKey, &imageUpdate) + stalledCondition := apimeta.FindStatusCondition(imageUpdate.Status.Conditions, meta.StalledCondition) + return stalledCondition != nil && + stalledCondition.Status == metav1.ConditionTrue && + stalledCondition.Reason == imagev1.RemovedTemplateFieldReason && + strings.Contains(stalledCondition.Message, tc.expectedErrMsg) + }, timeout).Should(BeTrue()) + + head, _ := localRepo.Head() + g.Expect(head.Hash().String()).To(Equal(preChangeCommitId)) + }, + ) + }) + } +} + func TestImageUpdateAutomationReconciler_crossNamespaceRef(t *testing.T) { g := NewWithT(t) policySpec := imagev1_reflect.ImagePolicySpec{ @@ -875,7 +1005,7 @@ func TestImageUpdateAutomationReconciler_crossNamespaceRef(t *testing.T) { testWithCustomRepoAndImagePolicy( ctx, g, testEnv, fixture, policySpec, latest, args, func(g *WithT, s repoAndPolicyArgs, repoURL string, localRepo *extgogit.Repository) { - commitMessage := fmt.Sprintf(testCommitMessageFmt, s.namespace, s.imagePolicyName) + commitMessage := fmt.Sprintf(testCommitMessageFmt, s.namespace, s.namespace, s.imagePolicyName) // Update the setter marker in the repo. policyKey := types.NamespacedName{ diff --git a/internal/policy/applier.go b/internal/policy/applier.go index 2722746e..93ed6194 100644 --- a/internal/policy/applier.go +++ b/internal/policy/applier.go @@ -42,8 +42,8 @@ var ( // ApplyPolicies applies the given set of policies on the source present in the // workDir based on the provided ImageUpdateAutomation configuration. -func ApplyPolicies(ctx context.Context, workDir string, obj *imagev1.ImageUpdateAutomation, policies []imagev1_reflect.ImagePolicy) (update.ResultV2, error) { - var result update.ResultV2 +func ApplyPolicies(ctx context.Context, workDir string, obj *imagev1.ImageUpdateAutomation, policies []imagev1_reflect.ImagePolicy) (update.Result, error) { + var result update.Result if obj.Spec.Update == nil { return result, ErrNoUpdateStrategy } @@ -62,5 +62,5 @@ func ApplyPolicies(ctx context.Context, workDir string, obj *imagev1.ImageUpdate } tracelog := log.FromContext(ctx).V(logger.TraceLevel) - return update.UpdateV2WithSetters(tracelog, manifestPath, manifestPath, policies) + return update.UpdateWithSetters(tracelog, manifestPath, manifestPath, policies) } diff --git a/internal/policy/applier_test.go b/internal/policy/applier_test.go index 7fc1bf59..f2745e2b 100644 --- a/internal/policy/applier_test.go +++ b/internal/policy/applier_test.go @@ -31,7 +31,6 @@ import ( imagev1 "github.com/fluxcd/image-automation-controller/api/v1beta2" "github.com/fluxcd/image-automation-controller/internal/testutil" "github.com/fluxcd/image-automation-controller/pkg/test" - "github.com/fluxcd/image-automation-controller/pkg/update" ) func testdataPath(path string) string { @@ -48,7 +47,6 @@ func Test_applyPolicies(t *testing.T) { inputPath string expectedPath string wantErr bool - wantResult update.Result }{ { name: "valid update strategy and one policy", diff --git a/internal/source/source.go b/internal/source/source.go index 51403c87..fc883144 100644 --- a/internal/source/source.go +++ b/internal/source/source.go @@ -48,14 +48,36 @@ import ( // ErrInvalidSourceConfiguration is an error for invalid source configuration. var ErrInvalidSourceConfiguration = errors.New("invalid source configuration") +// RemovedTemplateFieldError represents an error when a removed template field is used. +type RemovedTemplateFieldError struct { + Field string +} + +func (e *RemovedTemplateFieldError) Error() string { + switch e.Field { + case ".Updated": + return "template uses removed '.Updated' field. Please use '.Changed' instead. See: https://fluxcd.io/flux/components/image/imageupdateautomations/#message-template" + case ".Changed.ImageResult": + return "template uses removed '.Changed.ImageResult' field. Please use '.Changed.FileChanges' or '.Changed.Objects' instead. See: https://fluxcd.io/flux/components/image/imageupdateautomations/#message-template" + default: + return fmt.Sprintf("template uses removed '%s' field. See: https://fluxcd.io/flux/components/image/imageupdateautomations/#message-template", e.Field) + } +} + +func (e *RemovedTemplateFieldError) Is(target error) bool { + return errors.Is(target, ErrRemovedTemplateField) +} + +// ErrRemovedTemplateField is a sentinel error for removed template field usage. +var ErrRemovedTemplateField = &RemovedTemplateFieldError{} + const defaultMessageTemplate = `Update from image update automation` // TemplateData is the type of the value given to the commit message // template. type TemplateData struct { AutomationObject types.NamespacedName - Updated update.Result - Changed update.ResultV2 + Changed update.Result Values map[string]string } @@ -268,7 +290,7 @@ func WithPushConfigOptions(opts map[string]string) PushConfig { // CommitAndPush performs a commit in the source and pushes it to the remote // repository. -func (sm SourceManager) CommitAndPush(ctx context.Context, obj *imagev1.ImageUpdateAutomation, policyResult update.ResultV2, pushOptions ...PushConfig) (*PushResult, error) { +func (sm SourceManager) CommitAndPush(ctx context.Context, obj *imagev1.ImageUpdateAutomation, policyResult update.Result, pushOptions ...PushConfig) (*PushResult, error) { tracelog := log.FromContext(ctx).V(logger.TraceLevel) // Make sure there were file changes that need to be committed. @@ -279,7 +301,6 @@ func (sm SourceManager) CommitAndPush(ctx context.Context, obj *imagev1.ImageUpd // Perform a Git commit. templateValues := &TemplateData{ AutomationObject: sm.automationObjKey, - Updated: policyResult.ImageResult, Changed: policyResult, Values: obj.Spec.GitSpec.Commit.MessageTemplateValues, } @@ -356,11 +377,32 @@ func templateMsg(messageTemplate string, templateValues *TemplateData) (string, b := &strings.Builder{} if err := t.Execute(b, *templateValues); err != nil { + if removedFieldErr := checkRemovedTemplateField(err); removedFieldErr != nil { + return "", removedFieldErr + } return "", fmt.Errorf("failed to run template from spec: %w", err) } return b.String(), nil } +// checkRemovedTemplateField checks if the template error is due to removed fields +func checkRemovedTemplateField(err error) error { + removedFieldChecks := []struct { + fieldName string + errorPattern string + }{ + {".Updated", "can't evaluate field Updated in type source.TemplateData"}, + {".Changed.ImageResult", "can't evaluate field ImageResult in type update.Result"}, + } + + for _, check := range removedFieldChecks { + if strings.Contains(err.Error(), check.errorPattern) { + return &RemovedTemplateFieldError{Field: check.fieldName} + } + } + return nil +} + // PushResultOption allows configuring the options of PushResult. type PushResultOption func(*PushResult) diff --git a/internal/source/source_test.go b/internal/source/source_test.go index 0a66e43a..95ad65fc 100644 --- a/internal/source/source_test.go +++ b/internal/source/source_test.go @@ -55,8 +55,8 @@ import ( ) const ( - originRemote = "origin" - testCommitTemplate = `Commit summary + originRemote = "origin" + testCommitTemplateRemoved = `Commit summary Automation: {{ .AutomationObject }} @@ -101,6 +101,25 @@ Automation: {{ .AutomationObject }} Cluster: {{ index .Values "cluster" }} Testing: {{ .Values.testing }} +` + testCommitTemplateImageResult = `Commit summary + +Automation: {{ .AutomationObject }} + +Files: +{{ range $filename, $_ := .Changed.ImageResult.Files -}} +- {{ $filename }} +{{ end -}} + +Objects: +{{ range $resource, $_ := .Changed.ImageResult.Objects -}} +- {{ $resource.Kind }} {{ $resource.Name }} +{{ end -}} + +Images: +{{ range .Changed.ImageResult.Images -}} +- {{.}} ({{.Policy.Name}}) +{{ end -}} ` ) @@ -469,31 +488,36 @@ func test_sourceManager_CommitAndPush(t *testing.T, proto string) { checkRefSpecBranch string }{ { - name: "push to cloned branch with custom template", + name: "push to cloned branch with removed template field", gitSpec: &imagev1.GitSpec{ Push: &imagev1.PushSpec{ Branch: "main", }, Commit: imagev1.CommitSpec{ - MessageTemplate: testCommitTemplate, + MessageTemplate: testCommitTemplateRemoved, }, }, gitRepoReference: &sourcev1.GitRepositoryRef{ Branch: "main", }, latestImage: "helloworld:1.0.1", - wantErr: false, - wantCommitMsg: `Commit summary - -Automation: test-ns/test-update - -Files: -- deploy.yaml -Objects: -- deployment test -Images: -- helloworld:1.0.1 (policy1) -`, + wantErr: true, + }, + { + name: "push to cloned branch with removed ImageResult field", + gitSpec: &imagev1.GitSpec{ + Push: &imagev1.PushSpec{ + Branch: "main", + }, + Commit: imagev1.CommitSpec{ + MessageTemplate: testCommitTemplateImageResult, + }, + }, + gitRepoReference: &sourcev1.GitRepositoryRef{ + Branch: "main", + }, + latestImage: "helloworld:1.0.1", + wantErr: true, }, { name: "commit with update ResultV2 template", @@ -771,6 +795,11 @@ Testing: value pushResult, err := sm.CommitAndPush(ctx, updateAuto, result) g.Expect(err != nil).To(Equal(tt.wantErr)) + if tt.wantErr { + g.Expect(pushResult).To(BeNil()) + g.Expect(err).To(MatchError(ErrRemovedTemplateField)) + return + } if tt.noChange { g.Expect(pushResult).To(BeNil()) return diff --git a/pkg/update/result.go b/pkg/update/result.go index 3dfafd1b..a930c679 100644 --- a/pkg/update/result.go +++ b/pkg/update/result.go @@ -55,54 +55,10 @@ type ObjectIdentifier struct { yaml.ResourceIdentifier } -// Result reports the outcome of an automated update. It has a nested -// structure file->objects->images. Different projections (e.g., all -// the images, regardless of object) are available via methods. +// Result contains the file changes made during the update. It contains +// details about the exact changes made to the files and the objects in them. +// It has a nested structure file->objects->changes. type Result struct { - Files map[string]FileResult -} - -// FileResult gives the updates in a particular file. -type FileResult struct { - Objects map[ObjectIdentifier][]ImageRef -} - -// Images returns all the images that were involved in at least one -// update. -func (r Result) Images() []ImageRef { - seen := make(map[ImageRef]struct{}) - var result []ImageRef - for _, file := range r.Files { - for _, images := range file.Objects { - for _, ref := range images { - if _, ok := seen[ref]; !ok { - seen[ref] = struct{}{} - result = append(result, ref) - } - } - } - } - return result -} - -// Objects returns a map of all the objects against the images updated -// within, regardless of which file they appear in. -func (r Result) Objects() map[ObjectIdentifier][]ImageRef { - result := make(map[ObjectIdentifier][]ImageRef) - for _, file := range r.Files { - for res, refs := range file.Objects { - result[res] = append(result[res], refs...) - } - } - return result -} - -// ResultV2 contains Result of update and also the file changes made during the -// update. This extends the Result to include details about the exact changes -// made to the files and the objects in them. It has a nested structure -// file->objects->changes. -type ResultV2 struct { - ImageResult Result FileChanges map[string]ObjectChanges } @@ -117,9 +73,9 @@ type Change struct { Setter string } -// AddChange adds changes to Resultv2 for a given file, object and changes +// AddChange adds changes to Result for a given file, object and changes // associated with it. -func (r *ResultV2) AddChange(file string, objectID ObjectIdentifier, changes ...Change) { +func (r *Result) AddChange(file string, objectID ObjectIdentifier, changes ...Change) { if r.FileChanges == nil { r.FileChanges = map[string]ObjectChanges{} } @@ -133,7 +89,7 @@ func (r *ResultV2) AddChange(file string, objectID ObjectIdentifier, changes ... } // Changes returns all the changes that were made in at least one update. -func (r ResultV2) Changes() []Change { +func (r Result) Changes() []Change { seen := make(map[Change]struct{}) var result []Change for _, objChanges := range r.FileChanges { @@ -150,7 +106,7 @@ func (r ResultV2) Changes() []Change { } // Objects returns ObjectChanges, regardless of which file they appear in. -func (r ResultV2) Objects() ObjectChanges { +func (r Result) Objects() ObjectChanges { result := make(ObjectChanges) for _, objChanges := range r.FileChanges { for obj, change := range objChanges { diff --git a/pkg/update/result_test.go b/pkg/update/result_test.go index 42283eef..47b4a85e 100644 --- a/pkg/update/result_test.go +++ b/pkg/update/result_test.go @@ -42,7 +42,7 @@ func TestMustRef(t *testing.T) { }) } -func TestUpdateResults(t *testing.T) { +func TestResult(t *testing.T) { g := NewWithT(t) var result Result @@ -55,57 +55,6 @@ func TestUpdateResults(t *testing.T) { }}, } - 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"), - }, - }, - }, - }, - } - - 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"), - }, - objectNames[1]: { - mustRef("image:v1.0"), - mustRef("other:v2.0"), - }, - })) -} - -func TestResultV2(t *testing.T) { - g := NewWithT(t) - - var result ResultV2 - objectNames := []ObjectIdentifier{ - {yaml.ResourceIdentifier{ - NameMeta: yaml.NameMeta{Namespace: "ns", Name: "foo"}, - }}, - {yaml.ResourceIdentifier{ - NameMeta: yaml.NameMeta{Namespace: "ns", Name: "bar"}, - }}, - } - result.AddChange("foo.yaml", objectNames[0], Change{ OldValue: "aaa", NewValue: "bbb", @@ -117,7 +66,7 @@ func TestResultV2(t *testing.T) { Setter: "foo-ns:policy", }) - result = ResultV2{ + result = Result{ FileChanges: map[string]ObjectChanges{ "foo.yaml": { objectNames[0]: []Change{ diff --git a/pkg/update/setters.go b/pkg/update/setters.go index c5566fde..4038f904 100644 --- a/pkg/update/setters.go +++ b/pkg/update/setters.go @@ -49,17 +49,9 @@ func init() { // UpdateWithSetters takes all YAML files from `inpath`, updates any // that contain an "in scope" image policy marker, and writes files it -// updated (and only those files) back to `outpath`. -func UpdateWithSetters(tracelog logr.Logger, inpath, outpath string, policies []imagev1_reflect.ImagePolicy) (Result, error) { - result, err := UpdateV2WithSetters(tracelog, inpath, outpath, policies) - return result.ImageResult, err -} - -// UpdateV2WithSetters takes all YAML files from `inpath`, updates any -// that contain an "in scope" image policy marker, and writes files it // updated (and only those files) back to `outpath`. It also returns the result -// of the changes it made as ResultV2. -func UpdateV2WithSetters(tracelog logr.Logger, inpath, outpath string, policies []imagev1_reflect.ImagePolicy) (ResultV2, error) { +// of the changes it made as Result. +func UpdateWithSetters(tracelog logr.Logger, inpath, outpath string, policies []imagev1_reflect.ImagePolicy) (Result, 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 @@ -99,11 +91,7 @@ func UpdateV2WithSetters(tracelog logr.Logger, inpath, outpath string, policies // collect setter defs and setters by going through all the image // policies available. - result := Result{ - Files: make(map[string]FileResult), - } - - var resultV2 ResultV2 + var result Result // Compilng the result needs the file, the image ref used, and the // object. Each setter will supply its own name to its callback, @@ -112,7 +100,7 @@ func UpdateV2WithSetters(tracelog logr.Logger, inpath, outpath string, policies // iterates. imageRefs := make(map[string]imageRef) setAllCallback := func(file, setterName string, node *yaml.RNode, old, new string) { - ref, ok := imageRefs[setterName] + _, ok := imageRefs[setterName] if !ok { return } @@ -130,23 +118,7 @@ func UpdateV2WithSetters(tracelog logr.Logger, inpath, outpath string, policies Setter: setterName, } // Append the change for the file and identifier. - resultV2.AddChange(file, oid, ch) - - fileres, ok := result.Files[file] - if !ok { - fileres = FileResult{ - Objects: make(map[ObjectIdentifier][]ImageRef), - } - result.Files[file] = fileres - } - objres := fileres.Objects[oid] - for _, n := range objres { - if n == ref { - return - } - } - objres = append(objres, ref) - fileres.Objects[oid] = objres + result.AddChange(file, oid, ch) } defs := map[string]spec.Schema{} @@ -163,7 +135,7 @@ func UpdateV2WithSetters(tracelog logr.Logger, inpath, outpath string, policies image := policy.Status.LatestRef.String() r, err := name.ParseReference(image, name.WeakValidation) if err != nil { - return ResultV2{}, fmt.Errorf("encountered invalid image ref %q: %w", image, err) + return Result{}, fmt.Errorf("encountered invalid image ref %q: %w", image, err) } ref := imageRef{ Reference: r, @@ -221,12 +193,10 @@ func UpdateV2WithSetters(tracelog logr.Logger, inpath, outpath string, policies // go! err := pipeline.Execute() if err != nil { - return ResultV2{}, err + return Result{}, err } - // Combine the results. - resultV2.ImageResult = result - return resultV2, nil + return result, nil } // setAll returns a kio.Filter using the supplied SetAllCallback diff --git a/pkg/update/update_test.go b/pkg/update/update_test.go index 9e6df4ac..f8e2e9de 100644 --- a/pkg/update/update_test.go +++ b/pkg/update/update_test.go @@ -20,10 +20,8 @@ import ( "testing" "github.com/go-logr/logr" - "github.com/google/go-containerregistry/pkg/name" . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/kustomize/kyaml/yaml" "github.com/fluxcd/image-automation-controller/internal/testutil" @@ -64,6 +62,7 @@ func TestUpdateWithSetters(t *testing.T) { }, } + // Test Result. tmp := t.TempDir() result, err := UpdateWithSetters(logr.Discard(), "testdata/setters/original", tmp, policies) g.Expect(err).ToNot(HaveOccurred()) @@ -86,55 +85,7 @@ func TestUpdateWithSetters(t *testing.T) { }, }} - r, _ := name.ParseReference("index.repo.fake/updated:v1.0.1") - expectedImageRef := imageRef{r, types.NamespacedName{ - Name: "policy", - Namespace: "automation-ns", - }} - - r, _ = name.ParseReference("image:v1.0.0@sha256:6745aaad46d795c9836632e1fb62f24b7e7f4c843144da8e47a5465c411a14be") - expectedImageRefDigest := imageRef{r, types.NamespacedName{ - Name: "policy-with-digest", - Namespace: "automation-ns", - }} - expectedResult := Result{ - Files: map[string]FileResult{ - "kustomization.yml": { - Objects: map[ObjectIdentifier][]ImageRef{ - kustomizeResourceID: { - expectedImageRef, - expectedImageRefDigest, - }, - }, - }, - "Kustomization": { - Objects: map[ObjectIdentifier][]ImageRef{ - kustomizeResourceID: { - expectedImageRef, - }, - }, - }, - "marked.yaml": { - Objects: map[ObjectIdentifier][]ImageRef{ - markedResourceID: { - expectedImageRef, - }, - }, - }, - }, - } - - g.Expect(result).To(Equal(expectedResult)) - - // Test ResultV2. - tmp2 := t.TempDir() - resultV2, err := UpdateV2WithSetters(logr.Discard(), "testdata/setters/original", tmp2, policies) - g.Expect(err).ToNot(HaveOccurred()) - test.ExpectMatchingDirectories(g, tmp2, "testdata/setters/expected") - - expectedResultV2 := ResultV2{ - ImageResult: expectedResult, FileChanges: map[string]ObjectChanges{ "kustomization.yml": { kustomizeResourceID: []Change{ @@ -186,5 +137,5 @@ func TestUpdateWithSetters(t *testing.T) { }, } - g.Expect(resultV2).To(Equal(expectedResultV2)) + g.Expect(result).To(Equal(expectedResult)) }