fix: detect changes in spec.postRenderers

Signed-off-by: Soule BA <[email protected]>

api/v2/helmrelease_types.go

@@ -1008,6 +1008,11 @@ type HelmReleaseStatus struct {
 	// +optional
 	LastAttemptedConfigDigest string `json:"lastAttemptedConfigDigest,omitempty"`
 
+	// LastAttemptedPostRenderersDigest is the digest for the post-renderers of
+	// the last reconciliation attempt.
+	// +optional
+	LastAttemptedPostRenderersDigest string `json:"lastAttemptedPostRenderersDigest,omitempty"`
+
 	// LastHandledForceAt holds the value of the most recent force request
 	// value, so a change of the annotation value can be detected.
 	// +optional

api/v2beta1/helmrelease_types.go

@@ -949,6 +949,11 @@ type HelmReleaseStatus struct {
 	// +optional
 	LastAttemptedConfigDigest string `json:"lastAttemptedConfigDigest,omitempty"`
 
+	// LastAttemptedPostRenderersDigest is the digest for the post-renderers of
+	// the last reconciliation attempt.
+	// +optional
+	LastAttemptedPostRenderersDigest string `json:"lastAttemptedPostRenderersDigest,omitempty"`
+
 	// LastAttemptedReleaseAction is the last release action performed for this
 	// HelmRelease. It is used to determine the active remediation strategy.
 	//

api/v2beta2/helmrelease_types.go

@@ -1032,6 +1032,11 @@ type HelmReleaseStatus struct {
 	// +optional
 	LastAttemptedConfigDigest string `json:"lastAttemptedConfigDigest,omitempty"`
 
+	// LastAttemptedPostRenderersDigest is the digest for the post-renderers of
+	// the last reconciliation attempt.
+	// +optional
+	LastAttemptedPostRenderersDigest string `json:"lastAttemptedPostRenderersDigest,omitempty"`
+
 	// LastHandledForceAt holds the value of the most recent force request
 	// value, so a change of the annotation value can be detected.
 	// +optional

config/crd/bases/helm.toolkit.fluxcd.io_helmreleases.yaml

@@ -1106,6 +1106,11 @@ spec:
                   to reconcile.
                 format: int64
                 type: integer
+              lastAttemptedPostRenderersDigest:
+                description: |-
+                  LastAttemptedPostRenderersDigest is the digest for the post-renderers of
+                  the last reconciliation attempt.
+                type: string
               lastAttemptedReleaseAction:
                 description: |-
                   LastAttemptedReleaseAction is the last release action performed for this
@@ -2336,6 +2341,11 @@ spec:
                   by v2beta1 HelmReleases.
                 format: int64
                 type: integer
+              lastAttemptedPostRenderersDigest:
+                description: |-
+                  LastAttemptedPostRenderersDigest is the digest for the post-renderers of
+                  the last reconciliation attempt.
+                type: string
               lastAttemptedReleaseAction:
                 description: |-
                   LastAttemptedReleaseAction is the last release action performed for this
@@ -3615,6 +3625,11 @@ spec:
                   to reconcile.
                 format: int64
                 type: integer
+              lastAttemptedPostRenderersDigest:
+                description: |-
+                  LastAttemptedPostRenderersDigest is the digest for the post-renderers of
+                  the last reconciliation attempt.
+                type: string
               lastAttemptedReleaseAction:
                 description: |-
                   LastAttemptedReleaseAction is the last release action performed for this

docs/api/v2/helm.md

@@ -1623,6 +1623,19 @@ string
 </tr>
 <tr>
 <td>
+<code>lastAttemptedPostRenderersDigest</code><br>
+<em>
+string
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>LastAttemptedPostRenderersDigest is the digest for the post-renderers of
+the last reconciliation attempt.</p>
+</td>
+</tr>
+<tr>
+<td>
 <code>lastHandledForceAt</code><br>
 <em>
 string

internal/controller/helmrelease_controller.go

@@ -67,6 +67,7 @@ import (
 	"github.com/fluxcd/helm-controller/internal/features"
 	"github.com/fluxcd/helm-controller/internal/kube"
 	"github.com/fluxcd/helm-controller/internal/loader"
+	"github.com/fluxcd/helm-controller/internal/postrender"
 	intpredicates "github.com/fluxcd/helm-controller/internal/predicates"
 	intreconcile "github.com/fluxcd/helm-controller/internal/reconcile"
 	"github.com/fluxcd/helm-controller/internal/release"
@@ -391,6 +392,13 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, patchHelpe
 	obj.Status.LastAttemptedRevisionDigest = ociDigest
 	obj.Status.LastAttemptedConfigDigest = chartutil.DigestValues(digest.Canonical, values).String()
 	obj.Status.LastAttemptedValuesChecksum = ""
+	// Keep track of the post-renderers digest used during the last reconciliation.
+	// This is used to determine if the post-renderers have changed.
+	oldPostRenderersDigest := obj.Status.LastAttemptedPostRenderersDigest
+	// Update the post-renderers digest with the current post-renderers in the spec.
+	if obj.Spec.PostRenderers != nil {
+		obj.Status.LastAttemptedPostRenderersDigest = postrender.Digest(digest.Canonical, obj.Spec.PostRenderers).String()
+	}
 	obj.Status.LastReleaseRevision = 0
 
 	// Construct config factory for any further Helm actions.
@@ -409,9 +417,10 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, patchHelpe
 
 	// Off we go!
 	if err = intreconcile.NewAtomicRelease(patchHelper, cfg, r.EventRecorder, r.FieldManager).Reconcile(ctx, &intreconcile.Request{
-		Object: obj,
-		Chart:  loadedChart,
-		Values: values,
+		Object:                    obj,
+		Chart:                     loadedChart,
+		Values:                    values,
+		PreviousPostrendersDigest: oldPostRenderersDigest,
 	}); err != nil {
 		if errors.Is(err, intreconcile.ErrMustRequeue) {
 			return ctrl.Result{Requeue: true}, nil
@@ -643,6 +652,9 @@ func (r *HelmReleaseReconciler) adoptLegacyRelease(ctx context.Context, getter g
 	// Erase the last release revision from the status.
 	obj.Status.LastReleaseRevision = 0
 
+	// Set the last attempted post-renderers digest.
+	obj.Status.LastAttemptedPostRenderersDigest = postrender.Digest(digest.Canonical, obj.Spec.PostRenderers).String()
+
 	return nil
 }
 

internal/controller/helmrelease_controller_test.go

@@ -60,9 +60,11 @@ import (
 	"github.com/fluxcd/helm-controller/internal/chartutil"
 	"github.com/fluxcd/helm-controller/internal/features"
 	"github.com/fluxcd/helm-controller/internal/kube"
+	"github.com/fluxcd/helm-controller/internal/postrender"
 	intreconcile "github.com/fluxcd/helm-controller/internal/reconcile"
 	"github.com/fluxcd/helm-controller/internal/release"
 	"github.com/fluxcd/helm-controller/internal/testutil"
+	"github.com/fluxcd/pkg/apis/kustomize"
 )
 
 func TestHelmReleaseReconciler_reconcileRelease(t *testing.T) {
@@ -790,6 +792,7 @@ func TestHelmReleaseReconciler_reconcileRelease(t *testing.T) {
 				},
 			},
 		}
+		obj.Status.LastAttemptedPostRenderersDigest = postrender.Digest(digest.Canonical, obj.Spec.PostRenderers).String()
 
 		c := fake.NewClientBuilder().
 			WithScheme(NewTestScheme()).
@@ -1161,6 +1164,163 @@ func TestHelmReleaseReconciler_reconcileReleaseFromHelmChartSource(t *testing.T)
 			*conditions.FalseCondition(meta.ReadyCondition, v2.ArtifactFailedReason, "Source not ready"),
 		}))
 	})
+
+	t.Run("reports postrenderer changes", func(t *testing.T) {
+		g := NewWithT(t)
+
+		patches := `
+- target:
+    version: v1
+    kind: ConfigMap
+    name: cm
+  patch: |
+    - op: add
+      path: /metadata/annotations/foo
+      value: bar
+`
+
+		patches2 := `
+- target:
+    version: v1
+    kind: ConfigMap
+    name: cm
+  patch: |
+    - op: add
+      path: /metadata/annotations/foo2
+      value: bar2
+`
+
+		var targeted []kustomize.Patch
+		err := yaml.Unmarshal([]byte(patches), &targeted)
+		g.Expect(err).ToNot(HaveOccurred())
+
+		// Create HelmChart mock.
+		chartMock := testutil.BuildChart()
+		chartArtifact, err := testutil.SaveChartAsArtifact(chartMock, digest.SHA256, testServer.URL(), testServer.Root())
+		g.Expect(err).ToNot(HaveOccurred())
+		// copy the artifact to mutate the revision
+		ociArtifact := chartArtifact.DeepCopy()
+		ociArtifact.Revision += "@" + chartArtifact.Digest
+
+		ns, err := testEnv.CreateNamespace(context.TODO(), "mock")
+		g.Expect(err).ToNot(HaveOccurred())
+		t.Cleanup(func() {
+			_ = testEnv.Delete(context.TODO(), ns)
+		})
+
+		hc := &sourcev1.HelmChart{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:       "chart",
+				Namespace:  ns.Name,
+				Generation: 1,
+			},
+			Spec: sourcev1.HelmChartSpec{
+				Chart:   "testdata/test-helmrepo",
+				Version: "0.1.0",
+				SourceRef: sourcev1.LocalHelmChartSourceReference{
+					Kind: sourcev1.HelmRepositoryKind,
+					Name: "test-helmrepo",
+				},
+			},
+			Status: sourcev1.HelmChartStatus{
+				ObservedGeneration: 1,
+				Artifact:           chartArtifact,
+				Conditions: []metav1.Condition{
+					{
+						Type:   meta.ReadyCondition,
+						Status: metav1.ConditionTrue,
+					},
+				},
+			},
+		}
+
+		// Create a test Helm release storage mock.
+		rls := testutil.BuildRelease(&helmrelease.MockReleaseOptions{
+			Name:      "release",
+			Namespace: ns.Name,
+			Version:   1,
+			Chart:     chartMock,
+			Status:    helmrelease.StatusDeployed,
+		})
+
+		obj := &v2.HelmRelease{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "release",
+				Namespace: ns.Name,
+			},
+			Spec: v2.HelmReleaseSpec{
+				ChartRef: &v2.CrossNamespaceSourceReference{
+					Kind: sourcev1.HelmChartKind,
+					Name: hc.Name,
+				},
+				PostRenderers: []v2.PostRenderer{
+					{
+						Kustomize: &v2.Kustomize{
+							Patches: targeted,
+						},
+					},
+				},
+			},
+			Status: v2.HelmReleaseStatus{
+				StorageNamespace: ns.Name,
+				History: v2.Snapshots{
+					release.ObservedToSnapshot(release.ObserveRelease(rls)),
+				},
+				HelmChart: hc.Namespace + "/" + hc.Name,
+			},
+		}
+
+		obj.Status.LastAttemptedPostRenderersDigest = postrender.Digest(digest.Canonical, obj.Spec.PostRenderers).String()
+		obj.Status.LastAttemptedConfigDigest = chartutil.DigestValues(digest.Canonical, chartMock.Values).String()
+
+		c := fake.NewClientBuilder().
+			WithScheme(NewTestScheme()).
+			WithStatusSubresource(&v2.HelmRelease{}).
+			WithObjects(hc, obj).
+			Build()
+
+		r := &HelmReleaseReconciler{
+			Client:           c,
+			GetClusterConfig: GetTestClusterConfig,
+			EventRecorder:    record.NewFakeRecorder(32),
+		}
+
+		//Store the Helm release mock in the test namespace.
+		getter, err := r.buildRESTClientGetter(context.TODO(), obj)
+		g.Expect(err).ToNot(HaveOccurred())
+
+		cfg, err := action.NewConfigFactory(getter, action.WithStorage(helmdriver.SecretsDriverName, obj.Status.StorageNamespace))
+		g.Expect(err).ToNot(HaveOccurred())
+
+		store := helmstorage.Init(cfg.Driver)
+		g.Expect(store.Create(rls)).To(Succeed())
+
+		// update the postrenderers
+		err = yaml.Unmarshal([]byte(patches2), &targeted)
+		g.Expect(err).ToNot(HaveOccurred())
+		obj.Spec.PostRenderers[0].Kustomize.Patches = targeted
+
+		_, err = r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, r.Client), obj)
+		g.Expect(err).ToNot(HaveOccurred())
+
+		// Verify attempted values are set.
+		g.Expect(obj.Status.LastAttemptedGeneration).To(Equal(obj.Generation))
+		g.Expect(obj.Status.LastAttemptedPostRenderersDigest).To(Equal(postrender.Digest(digest.Canonical, obj.Spec.PostRenderers).String()))
+
+		// verify upgrade succeeded
+		g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{
+			*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingWithRetryReason, "Helm upgrade succeeded for release %s with chart %s",
+				fmt.Sprintf("%s/%s.v%d", rls.Namespace, rls.Name, rls.Version+1), fmt.Sprintf("%s@%s", chartMock.Name(),
+					chartMock.Metadata.Version)),
+			*conditions.TrueCondition(meta.ReadyCondition, v2.UpgradeSucceededReason, "Helm upgrade succeeded for release %s with chart %s",
+				fmt.Sprintf("%s/%s.v%d", rls.Namespace, rls.Name, rls.Version+1), fmt.Sprintf("%s@%s", chartMock.Name(),
+					chartMock.Metadata.Version)),
+			*conditions.TrueCondition(v2.ReleasedCondition, v2.UpgradeSucceededReason, "Helm upgrade succeeded for release %s with chart %s",
+				fmt.Sprintf("%s/%s.v%d", rls.Namespace, rls.Name, rls.Version+1), fmt.Sprintf("%s@%s", chartMock.Name(),
+					chartMock.Metadata.Version)),
+		}))
+
+	})
 }
 
 func TestHelmReleaseReconciler_reconcileReleaseFromOCIRepositorySource(t *testing.T) {
@@ -1718,6 +1878,8 @@ func TestHelmReleaseReconciler_reconcileReleaseFromOCIRepositorySource(t *testin
 			},
 		}
 
+		obj.Status.LastAttemptedPostRenderersDigest = postrender.Digest(digest.Canonical, obj.Spec.PostRenderers).String()
+
 		c := fake.NewClientBuilder().
 			WithScheme(NewTestScheme()).
 			WithStatusSubresource(&v2.HelmRelease{}).

internal/postrender/build.go

@@ -17,9 +17,12 @@ limitations under the License.
 package postrender
 
 import (
+	"encoding/json"
+
 	helmpostrender "helm.sh/helm/v3/pkg/postrender"
 
 	v2 "github.com/fluxcd/helm-controller/api/v2"
+	"github.com/opencontainers/go-digest"
 )
 
 // BuildPostRenderers creates the post-renderer instances from a HelmRelease
@@ -43,3 +46,12 @@ func BuildPostRenderers(rel *v2.HelmRelease) helmpostrender.PostRenderer {
 	}
 	return NewCombined(renderers...)
 }
+
+func Digest(algo digest.Algorithm, postrenders []v2.PostRenderer) digest.Digest {
+	digester := algo.Digester()
+	enc := json.NewEncoder(digester.Hash())
+	if err := enc.Encode(postrenders); err != nil {
+		return ""
+	}
+	return digester.Digest()
+}

internal/reconcile/reconcile.go

@@ -83,6 +83,9 @@ type Request struct {
 	// Values is the Helm chart values to be used for the installation or
 	// upgrade.
 	Values helmchartutil.Values
+	// PreviousPostrendersDigest is the digest of the post-renderers that were used
+	// during the last reconciliation.
+	PreviousPostrendersDigest string
 }
 
 // ActionReconciler is an interface which defines the methods that a reconciler

internal/reconcile/state.go

@@ -141,6 +141,11 @@ func DetermineReleaseState(ctx context.Context, cfg *action.ConfigFactory, req *
 			}
 		}
 
+		// Verify if postrender digest has changed
+		if req.PreviousPostrendersDigest != req.Object.Status.LastAttemptedPostRenderersDigest {
+			return ReleaseState{Status: ReleaseStatusOutOfSync, Reason: "postrender digest has changed"}, nil
+		}
+
 		// For the further determination of test results, we look at the
 		// observed state of the object. As tests can be run manually by
 		// users running e.g. `helm test`.