Skip to content

Commit f1a92cb

Browse files
committed
Consolidate Result and ResultV2 types for GA
Remove old Result type and rename ResultV2 to Result, following Stefan's review feedback to eliminate complex nested structures. This creates a single, flat Result type containing only FileChanges for improved API simplicity. Breaking changes: - .Changed.ImageResult.Files/.Images/.Objects no longer available - Users must migrate to .Changed.FileChanges, .Changed.Objects, and .Changed.Changes Enhanced error handling provides specific guidance for removed template fields, setting Stalled condition with clear migration instructions. Updated documentation includes removal notes and migration examples. Signed-off-by: cappyzawa <[email protected]>
1 parent 7975a00 commit f1a92cb

File tree

13 files changed

+203
-263
lines changed

13 files changed

+203
-263
lines changed

api/v1beta2/git.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ type CommitSpec struct {
6565
SigningKey *SigningKey `json:"signingKey,omitempty"`
6666
// MessageTemplate provides a template for the commit message,
6767
// into which will be interpolated the details of the change made.
68+
// Note: The `Updated` template field has been removed. Use `Changed` instead.
6869
// +optional
6970
MessageTemplate string `json:"messageTemplate,omitempty"`
7071

config/crd/bases/image.toolkit.fluxcd.io_imageupdateautomations.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,7 @@ spec:
440440
description: |-
441441
MessageTemplate provides a template for the commit message,
442442
into which will be interpolated the details of the change made.
443+
Note: The `Updated` template field has been removed. Use `Changed` instead.
443444
type: string
444445
messageTemplateValues:
445446
additionalProperties:

docs/api/v1beta2/image-automation.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,8 @@ string
6767
<td>
6868
<em>(Optional)</em>
6969
<p>MessageTemplate provides a template for the commit message,
70-
into which will be interpolated the details of the change made.</p>
70+
into which will be interpolated the details of the change made.
71+
Note: The <code>Updated</code> template field has been removed. Use <code>Changed</code> instead.</p>
7172
</td>
7273
</tr>
7374
<tr>

docs/spec/v1beta2/imageupdateautomations.md

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -380,6 +380,7 @@ including partial updates to just the image name or the tag, not just full image
380380
with name and tag update. Templates using `Updated` will result in an error and
381381
the ImageUpdateAutomation will be marked as Stalled.
382382

383+
383384
The message template also has access to the data related to the changes made by
384385
the automation. The template is a [Go text template][go-text-template]. The data
385386
available to the template have the following structure (not reproduced
@@ -392,14 +393,14 @@ type TemplateData struct {
392393
AutomationObject struct {
393394
Name, Namespace string
394395
}
395-
Changed update.ResultV2
396+
Changed update.Result
396397
Values map[string]string
397398
}
398399
399-
// ResultV2 contains the file changes made during the update. It contains
400+
// Result contains the file changes made during the update. It contains
400401
// details about the exact changes made to the files and the objects in them. It
401402
// has a nested structure file->objects->changes.
402-
type ResultV2 struct {
403+
type Result struct {
403404
FileChanges map[string]ObjectChanges
404405
}
405406
@@ -426,10 +427,10 @@ over the changed objects and changes:
426427

427428
```go
428429
// Changes returns all the changes that were made in at least one update.
429-
func (r ResultV2) Changes() []Change
430+
func (r Result) Changes() []Change
430431
431432
// Objects returns ObjectChanges, regardless of which file they appear in.
432-
func (r ResultV2) Objects() ObjectChanges
433+
func (r Result) Objects() ObjectChanges
433434
```
434435

435436
Example of using the methods in a template:

internal/controller/imageupdateautomation_controller_test.go

Lines changed: 92 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -71,12 +71,12 @@ const (
7171
Automation: {{ .AutomationObject }}
7272
7373
Files:
74-
{{ range $filename, $_ := .Changed.ImageResult.Files -}}
74+
{{ range $filename, $_ := .Changed.FileChanges -}}
7575
- {{ $filename }}
7676
{{ end -}}
7777
7878
Objects:
79-
{{ range $resource, $_ := .Changed.ImageResult.Objects -}}
79+
{{ range $resource, $_ := .Changed.Objects -}}
8080
{{ if eq $resource.Kind "Deployment" -}}
8181
- {{ $resource.Kind | lower }} {{ $resource.Name | lower }}
8282
{{ else -}}
@@ -85,8 +85,10 @@ Objects:
8585
{{ end -}}
8686
8787
Images:
88-
{{ range .Changed.ImageResult.Images -}}
89-
- {{.}} ({{.Policy.Name}})
88+
{{ range .Changed.Changes -}}
89+
{{ if .Setter -}}
90+
- {{.NewValue}} ({{.Setter}})
91+
{{ end -}}
9092
{{ end -}}
9193
`
9294
testCommitMessageFmt = `Commit summary
@@ -98,7 +100,7 @@ Files:
98100
Objects:
99101
- deployment test
100102
Images:
101-
- helloworld:v1.0.0 (%s)
103+
- helloworld:v1.0.0 (%s:%s)
102104
`
103105
)
104106

@@ -727,7 +729,7 @@ func TestImageUpdateAutomationReconciler_commitMessage(t *testing.T) {
727729
name: "template with update Result",
728730
template: testCommitTemplate,
729731
wantCommitMsg: func(policyName, policyNS string) string {
730-
return fmt.Sprintf(testCommitMessageFmt, policyNS, policyName)
732+
return fmt.Sprintf(testCommitMessageFmt, policyNS, policyNS, policyName)
731733
},
732734
},
733735
{
@@ -841,11 +843,8 @@ Automation: %s/update-test
841843
}
842844
}
843845

844-
// TestImageUpdateAutomationReconciler_removedTemplateField tests removed .Updated template field usage.
846+
// TestImageUpdateAutomationReconciler_removedTemplateField tests removed template field usage (.Updated and .Changed.ImageResult).
845847
func TestImageUpdateAutomationReconciler_removedTemplateField(t *testing.T) {
846-
g := NewWithT(t)
847-
ctx := context.TODO()
848-
849848
policySpec := imagev1_reflect.ImagePolicySpec{
850849
ImageRepositoryRef: meta.NamespacedObjectReference{
851850
Name: "not-expected-to-exist",
@@ -859,7 +858,14 @@ func TestImageUpdateAutomationReconciler_removedTemplateField(t *testing.T) {
859858
fixture := "testdata/appconfig"
860859
latest := "helloworld:v1.0.0"
861860

862-
removedTemplate := `Commit summary
861+
testCases := []struct {
862+
name string
863+
template string
864+
expectedErrMsg string
865+
}{
866+
{
867+
name: ".Updated field",
868+
template: `Commit summary
863869
864870
Automation: {{ .AutomationObject }}
865871
@@ -881,55 +887,88 @@ Images:
881887
{{ range .Updated.Images -}}
882888
- {{.}} ({{.Policy.Name}})
883889
{{ end -}}
884-
`
890+
`,
891+
expectedErrMsg: "template uses removed '.Updated' field",
892+
},
893+
{
894+
name: ".Changed.ImageResult field",
895+
template: `Commit summary
885896
886-
namespace, err := testEnv.CreateNamespace(ctx, "image-auto-test")
887-
g.Expect(err).ToNot(HaveOccurred())
888-
defer func() { g.Expect(testEnv.Delete(ctx, namespace)).To(Succeed()) }()
897+
Automation: {{ .AutomationObject }}
889898
890-
testWithRepoAndImagePolicy(
891-
ctx, g, testEnv, namespace.Name, fixture, policySpec, latest,
892-
func(g *WithT, s repoAndPolicyArgs, repoURL string, localRepo *extgogit.Repository) {
893-
policyKey := types.NamespacedName{
894-
Name: s.imagePolicyName,
895-
Namespace: s.namespace,
896-
}
897-
_ = testutil.CommitInRepo(ctx, g, repoURL, s.branch, originRemote, "Install setter marker", func(tmp string) {
898-
g.Expect(testutil.ReplaceMarker(filepath.Join(tmp, "deploy.yaml"), policyKey)).To(Succeed())
899-
})
899+
Files:
900+
{{ range $filename, $_ := .Changed.ImageResult.Files -}}
901+
- {{ $filename }}
902+
{{ end -}}
900903
901-
preChangeCommitId := testutil.CommitIdFromBranch(localRepo, s.branch)
902-
waitForNewHead(g, localRepo, s.branch, preChangeCommitId)
903-
preChangeCommitId = testutil.CommitIdFromBranch(localRepo, s.branch)
904+
Objects:
905+
{{ range $resource, $_ := .Changed.ImageResult.Objects -}}
906+
- {{ $resource.Kind }} {{ $resource.Name }}
907+
{{ end -}}
904908
905-
updateStrategy := &imagev1.UpdateStrategy{
906-
Strategy: imagev1.UpdateStrategySetters,
907-
}
908-
err := createImageUpdateAutomation(ctx, testEnv, "update-test", s.namespace, s.gitRepoName, s.gitRepoNamespace, s.branch, s.branch, "", removedTemplate, "", updateStrategy)
909+
Images:
910+
{{ range .Changed.ImageResult.Images -}}
911+
- {{.}} ({{.Policy.Name}})
912+
{{ end -}}
913+
`,
914+
expectedErrMsg: "template uses removed '.Changed.ImageResult' field",
915+
},
916+
}
917+
918+
for _, tc := range testCases {
919+
t.Run(tc.name, func(t *testing.T) {
920+
g := NewWithT(t)
921+
ctx := context.TODO()
922+
923+
namespace, err := testEnv.CreateNamespace(ctx, "image-auto-test")
909924
g.Expect(err).ToNot(HaveOccurred())
910-
defer func() {
911-
g.Expect(deleteImageUpdateAutomation(ctx, testEnv, "update-test", s.namespace)).To(Succeed())
912-
}()
925+
defer func() { g.Expect(testEnv.Delete(ctx, namespace)).To(Succeed()) }()
913926

914-
imageUpdateKey := types.NamespacedName{
915-
Namespace: s.namespace,
916-
Name: "update-test",
917-
}
927+
testWithRepoAndImagePolicy(
928+
ctx, g, testEnv, namespace.Name, fixture, policySpec, latest,
929+
func(g *WithT, s repoAndPolicyArgs, repoURL string, localRepo *extgogit.Repository) {
930+
policyKey := types.NamespacedName{
931+
Name: s.imagePolicyName,
932+
Namespace: s.namespace,
933+
}
934+
_ = testutil.CommitInRepo(ctx, g, repoURL, s.branch, originRemote, "Install setter marker", func(tmp string) {
935+
g.Expect(testutil.ReplaceMarker(filepath.Join(tmp, "deploy.yaml"), policyKey)).To(Succeed())
936+
})
918937

919-
g.Eventually(func() bool {
920-
var imageUpdate imagev1.ImageUpdateAutomation
921-
_ = testEnv.Get(context.TODO(), imageUpdateKey, &imageUpdate)
922-
stalledCondition := apimeta.FindStatusCondition(imageUpdate.Status.Conditions, meta.StalledCondition)
923-
return stalledCondition != nil &&
924-
stalledCondition.Status == metav1.ConditionTrue &&
925-
stalledCondition.Reason == imagev1.RemovedTemplateFieldReason &&
926-
strings.Contains(stalledCondition.Message, "template uses removed '.Updated' field")
927-
}, timeout).Should(BeTrue())
938+
preChangeCommitId := testutil.CommitIdFromBranch(localRepo, s.branch)
939+
waitForNewHead(g, localRepo, s.branch, preChangeCommitId)
940+
preChangeCommitId = testutil.CommitIdFromBranch(localRepo, s.branch)
928941

929-
head, _ := localRepo.Head()
930-
g.Expect(head.Hash().String()).To(Equal(preChangeCommitId))
931-
},
932-
)
942+
updateStrategy := &imagev1.UpdateStrategy{
943+
Strategy: imagev1.UpdateStrategySetters,
944+
}
945+
err := createImageUpdateAutomation(ctx, testEnv, "update-test", s.namespace, s.gitRepoName, s.gitRepoNamespace, s.branch, s.branch, "", tc.template, "", updateStrategy)
946+
g.Expect(err).ToNot(HaveOccurred())
947+
defer func() {
948+
g.Expect(deleteImageUpdateAutomation(ctx, testEnv, "update-test", s.namespace)).To(Succeed())
949+
}()
950+
951+
imageUpdateKey := types.NamespacedName{
952+
Namespace: s.namespace,
953+
Name: "update-test",
954+
}
955+
956+
g.Eventually(func() bool {
957+
var imageUpdate imagev1.ImageUpdateAutomation
958+
_ = testEnv.Get(context.TODO(), imageUpdateKey, &imageUpdate)
959+
stalledCondition := apimeta.FindStatusCondition(imageUpdate.Status.Conditions, meta.StalledCondition)
960+
return stalledCondition != nil &&
961+
stalledCondition.Status == metav1.ConditionTrue &&
962+
stalledCondition.Reason == imagev1.RemovedTemplateFieldReason &&
963+
strings.Contains(stalledCondition.Message, tc.expectedErrMsg)
964+
}, timeout).Should(BeTrue())
965+
966+
head, _ := localRepo.Head()
967+
g.Expect(head.Hash().String()).To(Equal(preChangeCommitId))
968+
},
969+
)
970+
})
971+
}
933972
}
934973

935974
func TestImageUpdateAutomationReconciler_crossNamespaceRef(t *testing.T) {
@@ -966,7 +1005,7 @@ func TestImageUpdateAutomationReconciler_crossNamespaceRef(t *testing.T) {
9661005
testWithCustomRepoAndImagePolicy(
9671006
ctx, g, testEnv, fixture, policySpec, latest, args,
9681007
func(g *WithT, s repoAndPolicyArgs, repoURL string, localRepo *extgogit.Repository) {
969-
commitMessage := fmt.Sprintf(testCommitMessageFmt, s.namespace, s.imagePolicyName)
1008+
commitMessage := fmt.Sprintf(testCommitMessageFmt, s.namespace, s.namespace, s.imagePolicyName)
9701009

9711010
// Update the setter marker in the repo.
9721011
policyKey := types.NamespacedName{

internal/policy/applier.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,8 @@ var (
4242

4343
// ApplyPolicies applies the given set of policies on the source present in the
4444
// workDir based on the provided ImageUpdateAutomation configuration.
45-
func ApplyPolicies(ctx context.Context, workDir string, obj *imagev1.ImageUpdateAutomation, policies []imagev1_reflect.ImagePolicy) (update.ResultV2, error) {
46-
var result update.ResultV2
45+
func ApplyPolicies(ctx context.Context, workDir string, obj *imagev1.ImageUpdateAutomation, policies []imagev1_reflect.ImagePolicy) (update.Result, error) {
46+
var result update.Result
4747
if obj.Spec.Update == nil {
4848
return result, ErrNoUpdateStrategy
4949
}
@@ -62,5 +62,5 @@ func ApplyPolicies(ctx context.Context, workDir string, obj *imagev1.ImageUpdate
6262
}
6363

6464
tracelog := log.FromContext(ctx).V(logger.TraceLevel)
65-
return update.UpdateV2WithSetters(tracelog, manifestPath, manifestPath, policies)
65+
return update.UpdateWithSetters(tracelog, manifestPath, manifestPath, policies)
6666
}

internal/policy/applier_test.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ import (
3131
imagev1 "github.com/fluxcd/image-automation-controller/api/v1beta2"
3232
"github.com/fluxcd/image-automation-controller/internal/testutil"
3333
"github.com/fluxcd/image-automation-controller/pkg/test"
34-
"github.com/fluxcd/image-automation-controller/pkg/update"
3534
)
3635

3736
func testdataPath(path string) string {
@@ -48,7 +47,6 @@ func Test_applyPolicies(t *testing.T) {
4847
inputPath string
4948
expectedPath string
5049
wantErr bool
51-
wantResult update.Result
5250
}{
5351
{
5452
name: "valid update strategy and one policy",

internal/source/source.go

Lines changed: 44 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,16 +48,36 @@ import (
4848
// ErrInvalidSourceConfiguration is an error for invalid source configuration.
4949
var ErrInvalidSourceConfiguration = errors.New("invalid source configuration")
5050

51-
// ErrRemovedTemplateField is an error for removed template field usage.
52-
var ErrRemovedTemplateField = errors.New("template uses removed '.Updated' field. Please use '.Changed' instead. See: https://fluxcd.io/flux/components/image/imageupdateautomations/#message-template")
51+
// RemovedTemplateFieldError represents an error when a removed template field is used.
52+
type RemovedTemplateFieldError struct {
53+
Field string
54+
}
55+
56+
func (e *RemovedTemplateFieldError) Error() string {
57+
switch e.Field {
58+
case ".Updated":
59+
return "template uses removed '.Updated' field. Please use '.Changed' instead. See: https://fluxcd.io/flux/components/image/imageupdateautomations/#message-template"
60+
case ".Changed.ImageResult":
61+
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"
62+
default:
63+
return fmt.Sprintf("template uses removed '%s' field. See: https://fluxcd.io/flux/components/image/imageupdateautomations/#message-template", e.Field)
64+
}
65+
}
66+
67+
func (e *RemovedTemplateFieldError) Is(target error) bool {
68+
return errors.Is(target, ErrRemovedTemplateField)
69+
}
70+
71+
// ErrRemovedTemplateField is a sentinel error for removed template field usage.
72+
var ErrRemovedTemplateField = &RemovedTemplateFieldError{}
5373

5474
const defaultMessageTemplate = `Update from image update automation`
5575

5676
// TemplateData is the type of the value given to the commit message
5777
// template.
5878
type TemplateData struct {
5979
AutomationObject types.NamespacedName
60-
Changed update.ResultV2
80+
Changed update.Result
6181
Values map[string]string
6282
}
6383

@@ -270,7 +290,7 @@ func WithPushConfigOptions(opts map[string]string) PushConfig {
270290

271291
// CommitAndPush performs a commit in the source and pushes it to the remote
272292
// repository.
273-
func (sm SourceManager) CommitAndPush(ctx context.Context, obj *imagev1.ImageUpdateAutomation, policyResult update.ResultV2, pushOptions ...PushConfig) (*PushResult, error) {
293+
func (sm SourceManager) CommitAndPush(ctx context.Context, obj *imagev1.ImageUpdateAutomation, policyResult update.Result, pushOptions ...PushConfig) (*PushResult, error) {
274294
tracelog := log.FromContext(ctx).V(logger.TraceLevel)
275295

276296
// Make sure there were file changes that need to be committed.
@@ -357,14 +377,32 @@ func templateMsg(messageTemplate string, templateValues *TemplateData) (string,
357377

358378
b := &strings.Builder{}
359379
if err := t.Execute(b, *templateValues); err != nil {
360-
if strings.Contains(err.Error(), "can't evaluate field Updated in type source.TemplateData") {
361-
return "", ErrRemovedTemplateField
380+
if removedFieldErr := checkRemovedTemplateField(err); removedFieldErr != nil {
381+
return "", removedFieldErr
362382
}
363383
return "", fmt.Errorf("failed to run template from spec: %w", err)
364384
}
365385
return b.String(), nil
366386
}
367387

388+
// checkRemovedTemplateField checks if the template error is due to removed fields
389+
func checkRemovedTemplateField(err error) error {
390+
removedFieldChecks := []struct {
391+
fieldName string
392+
errorPattern string
393+
}{
394+
{".Updated", "can't evaluate field Updated in type source.TemplateData"},
395+
{".Changed.ImageResult", "can't evaluate field ImageResult in type update.Result"},
396+
}
397+
398+
for _, check := range removedFieldChecks {
399+
if strings.Contains(err.Error(), check.errorPattern) {
400+
return &RemovedTemplateFieldError{Field: check.fieldName}
401+
}
402+
}
403+
return nil
404+
}
405+
368406
// PushResultOption allows configuring the options of PushResult.
369407
type PushResultOption func(*PushResult)
370408

0 commit comments

Comments
 (0)