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 <[email protected]>
fluxcd · Nov 29, 2023 · 04cb4f8 · 04cb4f8
1 parent ccd8f88
commit 04cb4f8
Show file tree

Hide file tree

Showing 3 changed files with 107 additions and 34 deletions.
diff --git 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
@@ -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
@@ -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())
 				}
 			}