fixup! Add deprecation handling for .Updated template field

Address PR review feedback

- Remove regex pattern matching in favor of t.Execute() error catching
- Rename DeprecatedTemplateFieldReason to RemovedTemplateFieldReason
- Update all comments from 'deprecated' to 'removed'
- Remove TODO comments and improve error handling comments
- Update test names and variables to reflect 'removed' terminology

Signed-off-by: cappyzawa <[email protected]>

Author: cappyzawa

api/v1beta2/condition_types.go

@@ -37,6 +37,6 @@ const (
 	// InvalidPolicySelectorReason represents an invalid policy selector.
 	InvalidPolicySelectorReason string = "InvalidPolicySelector"
 
-	// DeprecatedTemplateFieldReason represents usage of deprecated template field.
-	DeprecatedTemplateFieldReason string = "DeprecatedTemplateField"
+	// RemovedTemplateFieldReason represents usage of removed template field.
+	RemovedTemplateFieldReason string = "RemovedTemplateField"
 )

internal/controller/imageupdateautomation_controller.go

@@ -481,11 +481,10 @@ 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 deprecated template field usage.
+		// 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.
-		// TODO: Remove this block after .Updated template field is completely removed from the API.
-		if errors.Is(err, source.ErrDeprecatedTemplateField) {
-			conditions.MarkStalled(obj, imagev1.DeprecatedTemplateFieldReason, "%s", err)
+		if errors.Is(err, source.ErrRemovedTemplateField) {
+			conditions.MarkStalled(obj, imagev1.RemovedTemplateFieldReason, "%s", err)
 			result, retErr = ctrl.Result{}, nil
 			return
 		}

internal/controller/imageupdateautomation_controller_test.go

@@ -841,9 +841,8 @@ Automation: %s/update-test
 	}
 }
 
-// TestImageUpdateAutomationReconciler_deprecatedTemplateField tests deprecated .Updated template field usage.
-// TODO: Remove this test function after .Updated template field is completely removed from the API.
-func TestImageUpdateAutomationReconciler_deprecatedTemplateField(t *testing.T) {
+// TestImageUpdateAutomationReconciler_removedTemplateField tests removed .Updated template field usage.
+func TestImageUpdateAutomationReconciler_removedTemplateField(t *testing.T) {
 	g := NewWithT(t)
 	ctx := context.TODO()
 
@@ -860,7 +859,7 @@ func TestImageUpdateAutomationReconciler_deprecatedTemplateField(t *testing.T) {
 	fixture := "testdata/appconfig"
 	latest := "helloworld:v1.0.0"
 
-	deprecatedTemplate := `Commit summary
+	removedTemplate := `Commit summary
 
 Automation: {{ .AutomationObject }}
 
@@ -906,7 +905,7 @@ Images:
 			updateStrategy := &imagev1.UpdateStrategy{
 				Strategy: imagev1.UpdateStrategySetters,
 			}
-			err := createImageUpdateAutomation(ctx, testEnv, "update-test", s.namespace, s.gitRepoName, s.gitRepoNamespace, s.branch, s.branch, "", deprecatedTemplate, "", updateStrategy)
+			err := createImageUpdateAutomation(ctx, testEnv, "update-test", s.namespace, s.gitRepoName, s.gitRepoNamespace, s.branch, s.branch, "", removedTemplate, "", updateStrategy)
 			g.Expect(err).ToNot(HaveOccurred())
 			defer func() {
 				g.Expect(deleteImageUpdateAutomation(ctx, testEnv, "update-test", s.namespace)).To(Succeed())
@@ -923,8 +922,8 @@ Images:
 				stalledCondition := apimeta.FindStatusCondition(imageUpdate.Status.Conditions, meta.StalledCondition)
 				return stalledCondition != nil &&
 					stalledCondition.Status == metav1.ConditionTrue &&
-					stalledCondition.Reason == imagev1.DeprecatedTemplateFieldReason &&
-					strings.Contains(stalledCondition.Message, "template uses deprecated '.Updated' field")
+					stalledCondition.Reason == imagev1.RemovedTemplateFieldReason &&
+					strings.Contains(stalledCondition.Message, "template uses removed '.Updated' field")
 			}, timeout).Should(BeTrue())
 
 			head, _ := localRepo.Head()

internal/source/source.go

@@ -22,7 +22,6 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
-	"regexp"
 	"strings"
 	"text/template"
 	"time"
@@ -49,19 +48,15 @@ import (
 // ErrInvalidSourceConfiguration is an error for invalid source configuration.
 var ErrInvalidSourceConfiguration = errors.New("invalid source configuration")
 
-// ErrDeprecatedTemplateField is an error for deprecated template field usage.
-var ErrDeprecatedTemplateField = errors.New("template uses deprecated '.Updated' field. Please use '.Changed' instead. See: https://fluxcd.io/flux/components/image/imageupdateautomations/#message-template")
+// ErrRemovedTemplateField is an error for removed template field usage.
+var ErrRemovedTemplateField = errors.New("template uses removed '.Updated' field. Please use '.Changed' instead. See: https://fluxcd.io/flux/components/image/imageupdateautomations/#message-template")
 
 const defaultMessageTemplate = `Update from image update automation`
 
-// deprecatedUpdatedPattern matches deprecated .Updated field usage in templates
-var deprecatedUpdatedPattern = regexp.MustCompile(`\.Updated\.`)
-
 // 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
 	Values           map[string]string
 }
@@ -286,7 +281,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,
 	}
@@ -353,10 +347,6 @@ func templateMsg(messageTemplate string, templateValues *TemplateData) (string,
 		messageTemplate = defaultMessageTemplate
 	}
 
-	if deprecatedUpdatedPattern.MatchString(messageTemplate) {
-		return "", ErrDeprecatedTemplateField
-	}
-
 	// Includes only functions that are guaranteed to always evaluate to the same result for given input.
 	// This removes the possibility of accidentally relying on where or when the template runs.
 	// https://github.com/Masterminds/sprig/blob/3ac42c7bc5e4be6aa534e036fb19dde4a996da2e/functions.go#L70
@@ -367,6 +357,9 @@ func templateMsg(messageTemplate string, templateValues *TemplateData) (string,
 
 	b := &strings.Builder{}
 	if err := t.Execute(b, *templateValues); err != nil {
+		if strings.Contains(err.Error(), "can't evaluate field Updated in type source.TemplateData") {
+			return "", ErrRemovedTemplateField
+		}
 		return "", fmt.Errorf("failed to run template from spec: %w", err)
 	}
 	return b.String(), nil

internal/source/source_test.go

@@ -55,8 +55,8 @@ import (
 )
 
 const (
-	originRemote                 = "origin"
-	testCommitTemplateDeprecated = `Commit summary
+	originRemote              = "origin"
+	testCommitTemplateRemoved = `Commit summary
 
 Automation: {{ .AutomationObject }}
 
@@ -469,13 +469,13 @@ func test_sourceManager_CommitAndPush(t *testing.T, proto string) {
 		checkRefSpecBranch string
 	}{
 		{
-			name: "push to cloned branch with deprecated template field",
+			name: "push to cloned branch with removed template field",
 			gitSpec: &imagev1.GitSpec{
 				Push: &imagev1.PushSpec{
 					Branch: "main",
 				},
 				Commit: imagev1.CommitSpec{
-					MessageTemplate: testCommitTemplateDeprecated,
+					MessageTemplate: testCommitTemplateRemoved,
 				},
 			},
 			gitRepoReference: &sourcev1.GitRepositoryRef{
@@ -762,7 +762,7 @@ Testing: value
 			g.Expect(err != nil).To(Equal(tt.wantErr))
 			if tt.wantErr {
 				g.Expect(pushResult).To(BeNil())
-				g.Expect(err).To(MatchError(ErrDeprecatedTemplateField))
+				g.Expect(err).To(MatchError(ErrRemovedTemplateField))
 				return
 			}
 			if tt.noChange {