Merge pull request #965 from fluxcd/fix-964

Track changes in `.spec.postRenderers`

api/v2/helmrelease_types.go

@@ -933,6 +933,11 @@ type HelmReleaseStatus struct {
 	// +optional
 	ObservedGeneration int64 `json:"observedGeneration,omitempty"`
 
+	// ObservedPostRenderersDigest is the digest for the post-renderers of
+	// the last successful reconciliation attempt.
+	// +optional
+	ObservedPostRenderersDigest string `json:"observedPostRenderersDigest,omitempty"`
+
 	// LastAttemptedGeneration is the last generation the controller attempted
 	// to reconcile.
 	// +optional

api/v2beta1/helmrelease_types.go

@@ -875,6 +875,11 @@ type HelmReleaseStatus struct {
 	// +optional
 	ObservedGeneration int64 `json:"observedGeneration,omitempty"`
 
+	// ObservedPostRenderersDigest is the digest for the post-renderers of
+	// the last successful reconciliation attempt.
+	// +optional
+	ObservedPostRenderersDigest string `json:"observedPostRenderersDigest,omitempty"`
+
 	meta.ReconcileRequestStatus `json:",inline"`
 
 	// Conditions holds the conditions for the HelmRelease.

api/v2beta2/helmrelease_types.go

@@ -953,6 +953,11 @@ type HelmReleaseStatus struct {
 	// +optional
 	ObservedGeneration int64 `json:"observedGeneration,omitempty"`
 
+	// ObservedPostRenderersDigest is the digest for the post-renderers of
+	// the last successful reconciliation attempt.
+	// +optional
+	ObservedPostRenderersDigest string `json:"observedPostRenderersDigest,omitempty"`
+
 	// LastAttemptedGeneration is the last generation the controller attempted
 	// to reconcile.
 	// +optional

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

@@ -1160,6 +1160,11 @@ spec:
                 description: ObservedGeneration is the last observed generation.
                 format: int64
                 type: integer
+              observedPostRenderersDigest:
+                description: |-
+                  ObservedPostRenderersDigest is the digest for the post-renderers of
+                  the last successful reconciliation attempt.
+                type: string
               storageNamespace:
                 description: |-
                   StorageNamespace is the namespace of the Helm release storage for the
@@ -2394,6 +2399,11 @@ spec:
                 description: ObservedGeneration is the last observed generation.
                 format: int64
                 type: integer
+              observedPostRenderersDigest:
+                description: |-
+                  ObservedPostRenderersDigest is the digest for the post-renderers of
+                  the last successful reconciliation attempt.
+                type: string
               storageNamespace:
                 description: |-
                   StorageNamespace is the namespace of the Helm release storage for the
@@ -3677,6 +3687,11 @@ spec:
                 description: ObservedGeneration is the last observed generation.
                 format: int64
                 type: integer
+              observedPostRenderersDigest:
+                description: |-
+                  ObservedPostRenderersDigest is the digest for the post-renderers of
+                  the last successful reconciliation attempt.
+                type: string
               storageNamespace:
                 description: |-
                   StorageNamespace is the namespace of the Helm release storage for the

docs/api/v2/helm.md

@@ -1434,6 +1434,19 @@ int64
 </tr>
 <tr>
 <td>
+<code>observedPostRenderersDigest</code><br>
+<em>
+string
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>ObservedPostRenderersDigest is the digest for the post-renderers of
+the last successful reconciliation attempt.</p>
+</td>
+</tr>
+<tr>
+<td>
 <code>lastAttemptedGeneration</code><br>
 <em>
 int64

docs/spec/v2/helmreleases.md

@@ -1666,6 +1666,16 @@ The helm-controller reports an observed generation in the HelmRelease's
 `.metadata.generation` which resulted in either a [ready state](#ready-helmrelease),
 or stalled due to error it can not recover from without human intervention.
 
+### Observed Post Renderers Digest
+
+The helm-controller reports the digest for the [post renderers](#post-renderers)
+it last rendered the Helm chart with in the for a successful Helm install or
+upgrade in the `.status.observedPostRenderersDigest` field.
+
+This field is used by the controller to determine if a deployed Helm release
+is in sync with the HelmRelease `spec.postRenderers` configuration and whether
+it should trigger a Helm upgrade.
+
 ### Last Attempted Config Digest
 
 The helm-controller reports the digest for the [values](#values) it last

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"
@@ -352,14 +353,17 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, patchHelpe
 		conditions.MarkUnknown(obj, meta.ReadyCondition, meta.ProgressingReason, "reconciliation in progress")
 	}
 
-	// Attempt to adopt "legacy" v2beta1 release state on a best-effort basis.
-	// If this fails, the controller will fall back to performing an upgrade
-	// to settle on the desired state.
-	// TODO(hidde): remove this in a future release.
+	// Keep feature flagged code paths separate from the main reconciliation
+	// logic to ensure easy removal when the feature flag is removed.
 	if ok, _ := features.Enabled(features.AdoptLegacyReleases); ok {
+		// Attempt to adopt "legacy" v2beta1 release state on a best-effort basis.
+		// If this fails, the controller will fall back to performing an upgrade
+		// to settle on the desired state.
+		// TODO(hidde): remove this in a future release.
 		if err := r.adoptLegacyRelease(ctx, getter, obj); err != nil {
 			log.Error(err, "failed to adopt v2beta1 release state")
 		}
+		r.adoptPostRenderersStatus(obj)
 	}
 
 	// If the release target configuration has changed, we need to uninstall the
@@ -646,6 +650,20 @@ func (r *HelmReleaseReconciler) adoptLegacyRelease(ctx context.Context, getter g
 	return nil
 }
 
+// adoptPostRenderersStatus attempts to set obj.Status.ObservedPostRenderersDigest
+// for v2beta1 and v2beta2 HelmReleases.
+func (*HelmReleaseReconciler) adoptPostRenderersStatus(obj *v2.HelmRelease) {
+	if obj.GetGeneration() != obj.Status.ObservedGeneration {
+		return
+	}
+
+	// if we have a reconciled object with PostRenderers not reflected in the
+	// status, we need to update the status.
+	if obj.Spec.PostRenderers != nil && obj.Status.ObservedPostRenderersDigest == "" {
+		obj.Status.ObservedPostRenderersDigest = postrender.Digest(digest.Canonical, obj.Spec.PostRenderers).String()
+	}
+}
+
 func (r *HelmReleaseReconciler) buildRESTClientGetter(ctx context.Context, obj *v2.HelmRelease) (genericclioptions.RESTClientGetter, error) {
 	opts := []kube.Option{
 		kube.WithNamespace(obj.GetReleaseNamespace()),

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) {
@@ -1161,6 +1163,157 @@ 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())
+
+		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.ObservedPostRenderersDigest = 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.ObservedPostRenderersDigest).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.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) {

internal/postrender/build.go

@@ -17,6 +17,9 @@ limitations under the License.
 package postrender
 
 import (
+	"encoding/json"
+
+	"github.com/opencontainers/go-digest"
 	helmpostrender "helm.sh/helm/v3/pkg/postrender"
 
 	v2 "github.com/fluxcd/helm-controller/api/v2"
@@ -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/release.go

@@ -28,6 +28,8 @@ import (
 
 	v2 "github.com/fluxcd/helm-controller/api/v2"
 	"github.com/fluxcd/helm-controller/internal/action"
+	"github.com/fluxcd/helm-controller/internal/digest"
+	"github.com/fluxcd/helm-controller/internal/postrender"
 	"github.com/fluxcd/helm-controller/internal/release"
 	"github.com/fluxcd/helm-controller/internal/storage"
 )
@@ -137,6 +139,8 @@ func observeRelease(observed observedReleases) storage.ObserveFunc {
 // tests are not enabled, and excluding it when failures must be ignored.
 //
 // If Ready=True, any Stalled condition is removed.
+//
+// The ObservedPostRenderersDigest is updated if the post-renderers exist.
 func summarize(req *Request) {
 	var sumConds = []string{v2.RemediatedCondition, v2.ReleasedCondition}
 	if req.Object.GetTest().Enable && !req.Object.GetTest().IgnoreFailures {
@@ -186,6 +190,15 @@ func summarize(req *Request) {
 		Message:            conds[0].Message,
 		ObservedGeneration: req.Object.Generation,
 	})
+
+	// remove stale post-renderers digest
+	if conditions.Get(req.Object, meta.ReadyCondition).Status == metav1.ConditionTrue {
+		req.Object.Status.ObservedPostRenderersDigest = ""
+		if req.Object.Spec.PostRenderers != nil {
+			// Update the post-renderers digest if the post-renderers exist.
+			req.Object.Status.ObservedPostRenderersDigest = postrender.Digest(digest.Canonical, req.Object.Spec.PostRenderers).String()
+		}
+	}
 }
 
 // eventMessageWithLog returns an event message composed out of the given