Skip to content

Commit 9176026

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 d35a65c commit 9176026

File tree

13 files changed

+215
-263
lines changed

13 files changed

+215
-263
lines changed

api/v1beta2/git.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,9 @@ 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.
69+
// Note: The `.Changed.ImageResult` template field has been removed. Use `.Changed.FileChanges`,
70+
// `.Changed.Objects`, or `.Changed.Changes` instead.
6871
// +optional
6972
MessageTemplate string `json:"messageTemplate,omitempty"`
7073

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,9 @@ spec:
426426
description: |-
427427
MessageTemplate provides a template for the commit message,
428428
into which will be interpolated the details of the change made.
429+
Note: The `Updated` template field has been removed. Use `Changed` instead.
430+
Note: The `.Changed.ImageResult` template field has been removed. Use `.Changed.FileChanges`,
431+
`.Changed.Objects`, or `.Changed.Changes` instead.
429432
type: string
430433
messageTemplateValues:
431434
additionalProperties:

docs/api/v1beta2/image-automation.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,10 @@ 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.
72+
Note: The <code>.Changed.ImageResult</code> template field has been removed. Use <code>.Changed.FileChanges</code>,
73+
<code>.Changed.Objects</code>, or <code>.Changed.Changes</code> instead.</p>
7174
</td>
7275
</tr>
7376
<tr>

docs/spec/v1beta2/imageupdateautomations.md

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -380,6 +380,13 @@ 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+
**Removal Note:** The `.Changed.ImageResult` field has been removed from the API.
384+
Use the following alternatives:
385+
- `.Changed.ImageResult.Files` → `.Changed.FileChanges`
386+
- `.Changed.ImageResult.Images` → `.Changed.Changes` (iterate to get image values)
387+
- `.Changed.ImageResult.Objects` → `.Changed.Objects`
388+
Templates using `.Changed.ImageResult` will result in an error and the ImageUpdateAutomation will be marked as Stalled.
389+
383390
The message template also has access to the data related to the changes made by
384391
the automation. The template is a [Go text template][go-text-template]. The data
385392
available to the template have the following structure (not reproduced
@@ -392,14 +399,14 @@ type TemplateData struct {
392399
AutomationObject struct {
393400
Name, Namespace string
394401
}
395-
Changed update.ResultV2
402+
Changed update.Result
396403
Values map[string]string
397404
}
398405
399-
// ResultV2 contains the file changes made during the update. It contains
406+
// Result contains the file changes made during the update. It contains
400407
// details about the exact changes made to the files and the objects in them. It
401408
// has a nested structure file->objects->changes.
402-
type ResultV2 struct {
409+
type Result struct {
403410
FileChanges map[string]ObjectChanges
404411
}
405412
@@ -426,10 +433,10 @@ over the changed objects and changes:
426433

427434
```go
428435
// Changes returns all the changes that were made in at least one update.
429-
func (r ResultV2) Changes() []Change
436+
func (r Result) Changes() []Change
430437
431438
// Objects returns ObjectChanges, regardless of which file they appear in.
432-
func (r ResultV2) Objects() ObjectChanges
439+
func (r Result) Objects() ObjectChanges
433440
```
434441

435442
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",

0 commit comments

Comments
 (0)