From add3e883d789a5247351fe1e11656e7bd65ea055 Mon Sep 17 00:00:00 2001 From: Soule BA Date: Sun, 5 May 2024 21:52:52 +0200 Subject: [PATCH] fix: detect changes in spec.postRenderers Signed-off-by: Soule BA --- api/v2/helmrelease_types.go | 5 + api/v2beta1/helmrelease_types.go | 5 + api/v2beta2/helmrelease_types.go | 5 + .../helm.toolkit.fluxcd.io_helmreleases.yaml | 17 +- docs/api/v2/helm.md | 13 ++ internal/controller/helmrelease_controller.go | 16 +- .../controller/helmrelease_controller_test.go | 162 ++++++++++++++++++ internal/postrender/build.go | 12 ++ internal/reconcile/reconcile.go | 3 + internal/reconcile/state.go | 5 + 10 files changed, 239 insertions(+), 4 deletions(-) diff --git a/api/v2/helmrelease_types.go b/api/v2/helmrelease_types.go index 688db28d0..50a132bae 100644 --- a/api/v2/helmrelease_types.go +++ b/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 diff --git a/api/v2beta1/helmrelease_types.go b/api/v2beta1/helmrelease_types.go index f857baac0..508511fa5 100644 --- a/api/v2beta1/helmrelease_types.go +++ b/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. // diff --git a/api/v2beta2/helmrelease_types.go b/api/v2beta2/helmrelease_types.go index 4f2121dad..b657322b8 100644 --- a/api/v2beta2/helmrelease_types.go +++ b/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 diff --git a/config/crd/bases/helm.toolkit.fluxcd.io_helmreleases.yaml b/config/crd/bases/helm.toolkit.fluxcd.io_helmreleases.yaml index f60447460..a3fed831d 100644 --- a/config/crd/bases/helm.toolkit.fluxcd.io_helmreleases.yaml +++ b/config/crd/bases/helm.toolkit.fluxcd.io_helmreleases.yaml @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.15.0 + controller-gen.kubebuilder.io/version: v0.14.0 name: helmreleases.helm.toolkit.fluxcd.io spec: group: helm.toolkit.fluxcd.io @@ -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 diff --git a/docs/api/v2/helm.md b/docs/api/v2/helm.md index 08472f1b8..909b21dbd 100644 --- a/docs/api/v2/helm.md +++ b/docs/api/v2/helm.md @@ -1623,6 +1623,19 @@ string +lastAttemptedPostRenderersDigest
+ +string + + + +(Optional) +

LastAttemptedPostRenderersDigest is the digest for the post-renderers of +the last reconciliation attempt.

+ + + + lastHandledForceAt
string diff --git a/internal/controller/helmrelease_controller.go b/internal/controller/helmrelease_controller.go index 083a44528..044f4d101 100644 --- a/internal/controller/helmrelease_controller.go +++ b/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,11 @@ 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. + 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 +415,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 +650,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 } diff --git a/internal/controller/helmrelease_controller_test.go b/internal/controller/helmrelease_controller_test.go index a7921a358..270ff165f 100644 --- a/internal/controller/helmrelease_controller_test.go +++ b/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{}). diff --git a/internal/postrender/build.go b/internal/postrender/build.go index bac4ea48b..96ea800e9 100644 --- a/internal/postrender/build.go +++ b/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() +} diff --git a/internal/reconcile/reconcile.go b/internal/reconcile/reconcile.go index 6c51dd1cc..353dee0ff 100644 --- a/internal/reconcile/reconcile.go +++ b/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 diff --git a/internal/reconcile/state.go b/internal/reconcile/state.go index b3bf7d03e..ea8b2504c 100644 --- a/internal/reconcile/state.go +++ b/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`.