From 04cb4f82a4aad379f0dd43ca82e33c047d78ec5e Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Wed, 29 Nov 2023 23:01:54 +0100 Subject: [PATCH] action/diff: include Helm metadata in objects This ensures that the metadata labels and annotations Helm adds during the creation of resources are included while diffing them. As they are not part of the manifest but should be restored in case they are e.g. removed or modified. Signed-off-by: Hidde Beydals --- internal/action/diff.go | 34 +++++++++++++ internal/action/diff_test.go | 86 +++++++++++++++++++------------- internal/reconcile/state_test.go | 21 ++++++++ 3 files changed, 107 insertions(+), 34 deletions(-) diff --git a/internal/action/diff.go b/internal/action/diff.go index e0b835a25..a3eef10c0 100644 --- a/internal/action/diff.go +++ b/internal/action/diff.go @@ -68,6 +68,11 @@ func Diff(ctx context.Context, config *helmaction.Configuration, rls *helmreleas errs []error ) for _, obj := range objects { + // Set the Helm metadata on the object which is normally set by Helm + // during object creation. + setHelmMetadata(obj, rls) + + // Set the namespace of the object if it is not set. if obj.GetNamespace() == "" { // Manifest does not contain the namespace of the release. // Figure out if the object is namespaced if the namespace is not @@ -188,6 +193,35 @@ func ApplyDiff(ctx context.Context, config *helmaction.Configuration, diffSet js return changeSet, apierrutil.NewAggregate(errs) } +const ( + appManagedByLabel = "app.kubernetes.io/managed-by" + appManagedByHelm = "Helm" + helmReleaseNameAnnotation = "meta.helm.sh/release-name" + helmReleaseNamespaceAnnotation = "meta.helm.sh/release-namespace" +) + +// setHelmMetadata sets the metadata on the given object to indicate that it is +// managed by Helm. This is safe to do, because we apply it to objects that +// originate from the Helm release itself. +// xref: https://github.com/helm/helm/blob/v3.13.2/pkg/action/validate.go +// xref: https://github.com/helm/helm/blob/main/pkg/action/rollback.go#L186-L191 +func setHelmMetadata(obj client.Object, rls *helmrelease.Release) { + labels := obj.GetLabels() + if labels == nil { + labels = make(map[string]string, 1) + } + labels[appManagedByLabel] = appManagedByHelm + obj.SetLabels(labels) + + annotations := obj.GetAnnotations() + if annotations == nil { + annotations = make(map[string]string, 2) + } + annotations[helmReleaseNameAnnotation] = rls.Name + annotations[helmReleaseNamespaceAnnotation] = rls.Namespace + obj.SetAnnotations(annotations) +} + // objectToChangeSetEntry returns a ssa.ChangeSetEntry for the given object and // action. func objectToChangeSetEntry(obj client.Object, action ssa.Action) ssa.ChangeSetEntry { diff --git a/internal/action/diff_test.go b/internal/action/diff_test.go index 8face3197..493ce7fb5 100644 --- a/internal/action/diff_test.go +++ b/internal/action/diff_test.go @@ -162,38 +162,6 @@ metadata: name: disabled annotations: %[1]s: %[2]s -data: - key: value`, v2.DriftDetectionMetadataKey, v2.DriftDetectionDisabledValue), - mutateCluster: func(objs []*unstructured.Unstructured, namespace string) ([]*unstructured.Unstructured, error) { - var clusterObjs []*unstructured.Unstructured - for _, obj := range objs { - obj := obj.DeepCopy() - obj.SetNamespace(namespace) - if err := unstructured.SetNestedField(obj.Object, "changed", "data", "key"); err != nil { - return nil, fmt.Errorf("failed to set nested field: %w", err) - } - clusterObjs = append(clusterObjs, obj) - } - return clusterObjs, nil - }, - want: func(namespace string, desired, cluster []*unstructured.Unstructured) jsondiff.DiffSet { - return jsondiff.DiffSet{ - { - Type: jsondiff.DiffTypeExclude, - DesiredObject: namespacedUnstructured(desired[0], namespace), - }, - } - }, - }, - { - name: "manifest with disabled annotation", - manifest: fmt.Sprintf(`--- -apiVersion: v1 -kind: ConfigMap -metadata: - name: disabled - labels: - %[1]s: %[2]s data: key: value`, v2.DriftDetectionMetadataKey, v2.DriftDetectionDisabledValue), mutateCluster: func(objs []*unstructured.Unstructured, namespace string) ([]*unstructured.Unstructured, error) { @@ -373,6 +341,53 @@ data: } }, }, + { + name: "configures Helm metadata", + manifest: `--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: without-helm-metadata +data: + key: value`, + mutateCluster: func(objs []*unstructured.Unstructured, namespace string) ([]*unstructured.Unstructured, error) { + var clusterObjs []*unstructured.Unstructured + for _, obj := range objs { + obj := obj.DeepCopy() + if obj.GetNamespace() == "" { + obj.SetNamespace(namespace) + } + obj.SetAnnotations(nil) + obj.SetLabels(nil) + clusterObjs = append(clusterObjs, obj) + } + return clusterObjs, nil + }, + want: func(namespace string, desired, cluster []*unstructured.Unstructured) jsondiff.DiffSet { + return jsondiff.DiffSet{ + { + Type: jsondiff.DiffTypeUpdate, + DesiredObject: namespacedUnstructured(desired[0], namespace), + ClusterObject: cluster[0], + Patch: extjsondiff.Patch{ + { + Type: extjsondiff.OperationAdd, + Path: "/metadata", + Value: map[string]interface{}{ + "labels": map[string]interface{}{ + appManagedByLabel: appManagedByHelm, + }, + "annotations": map[string]interface{}{ + helmReleaseNameAnnotation: "configures Helm metadata", + helmReleaseNamespaceAnnotation: namespace, + }, + }, + }, + }, + }, + } + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -389,10 +404,15 @@ data: } }) + rls := &helmrelease.Release{Name: tt.name, Namespace: ns.Name, Manifest: tt.manifest} + objs, err := ssa.ReadObjects(strings.NewReader(tt.manifest)) if err != nil { t.Fatalf("Failed to read release objects: %v", err) } + for _, obj := range objs { + setHelmMetadata(obj, rls) + } clusterObjs := objs if tt.mutateCluster != nil { @@ -418,8 +438,6 @@ data: } } - rls := &helmrelease.Release{Namespace: ns.Name, Manifest: tt.manifest} - got, err := Diff(ctx, &helmaction.Configuration{RESTClientGetter: getter}, rls, testOwner, tt.ignoreRules...) if (err != nil) != tt.wantErr { t.Errorf("Diff() error = %v, wantErr %v", err, tt.wantErr) diff --git a/internal/reconcile/state_test.go b/internal/reconcile/state_test.go index 286b8e1b2..abc25ef3e 100644 --- a/internal/reconcile/state_test.go +++ b/internal/reconcile/state_test.go @@ -525,6 +525,13 @@ func TestDetermineReleaseState_DriftDetection(t *testing.T) { "name": "fixture", "namespace": namespace, "creationTimestamp": nil, + "labels": map[string]interface{}{ + "app.kubernetes.io/managed-by": "Helm", + }, + "annotations": map[string]interface{}{ + "meta.helm.sh/release-name": mockReleaseName, + "meta.helm.sh/release-namespace": namespace, + }, }, }, }, @@ -558,6 +565,13 @@ func TestDetermineReleaseState_DriftDetection(t *testing.T) { "name": "fixture", "namespace": namespace, "creationTimestamp": nil, + "labels": map[string]interface{}{ + "app.kubernetes.io/managed-by": "Helm", + }, + "annotations": map[string]interface{}{ + "meta.helm.sh/release-name": mockReleaseName, + "meta.helm.sh/release-namespace": namespace, + }, }, }, }, @@ -611,6 +625,13 @@ func TestDetermineReleaseState_DriftDetection(t *testing.T) { for _, obj := range objs { g.Expect(ssa.NormalizeUnstructured(obj)).To(Succeed()) obj.SetNamespace(releaseNamespace) + obj.SetLabels(map[string]string{ + "app.kubernetes.io/managed-by": "Helm", + }) + obj.SetAnnotations(map[string]string{ + "meta.helm.sh/release-name": rls.Name, + "meta.helm.sh/release-namespace": rls.Namespace, + }) g.Expect(testEnv.Create(context.Background(), obj)).To(Succeed()) } }