fix: drop null-valued labels/annotations keys in metadata tidy

.github/workflows/ci.yaml

@@ -196,3 +196,6 @@ jobs:
           set -o pipefail
           helm diff upgrade storage-diff ./helm-diff -n prod-apps --storage-namespace flux-system --three-way-merge --set replicaCount=2 | tee /tmp/three-way.out
           grep -q 'replicas: 2' /tmp/three-way.out
+
+      - name: Reproduce issue 1064 (labels-only diff noise)
+        run: scripts/repro-issue-1064.sh

manifest/parse.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io"
 	"log"
+	"regexp"
 	"strings"
 
 	jsoniter "github.com/json-iterator/go"
@@ -20,6 +21,105 @@ const (
 
 var yamlSeparator = []byte("\n---\n")
 
+// metadataLineRegex matches the top-level `metadata:` key of a manifest.
+var metadataLineRegex = regexp.MustCompile(`^metadata:[ \t]*(#.*)?$`)
+
+// stripEmptyMetadataKeys removes `labels:`/`annotations:` lines from content
+// when their value is null or an empty mapping.
+//
+// Kubernetes treats a null or empty labels/annotations map exactly like an
+// absent one, but charts frequently render the (empty) key anyway, e.g. via a
+// conditional block:
+//
+//	metadata:
+//	  name: example
+//	  labels:
+//
+// A raw textual diff between such a manifest and one that omits the key
+// reports a meaningless `- labels:` change. The removal is done line-based so
+// the surrounding text keeps its original formatting; the parsed document is
+// only consulted to confirm that the key really is null/empty (a `labels:`
+// line followed by indented entries is left alone).
+//
+// See https://github.com/databus23/helm-diff/issues/1064
+func stripEmptyMetadataKeys(content []byte) []byte {
+	var doc map[interface{}]interface{}
+	if err := yaml.Unmarshal(content, &doc); err != nil {
+		return content
+	}
+
+	metadata, ok := doc["metadata"].(map[interface{}]interface{})
+	if !ok {
+		return content
+	}
+
+	strip := map[string]bool{}
+	for _, key := range []string{"labels", "annotations"} {
+		value, exists := metadata[key]
+		if !exists {
+			continue
+		}
+		if value == nil {
+			strip[key] = true
+			continue
+		}
+		if m, ok := value.(map[interface{}]interface{}); ok && len(m) == 0 {
+			strip[key] = true
+		}
+	}
+	if len(strip) == 0 {
+		return content
+	}
+
+	lines := strings.Split(string(content), "\n")
+
+	// Determine the indentation of metadata's direct children by looking at
+	// the first non-blank line below the top-level `metadata:` key.
+	childIndent := -1
+	for i, line := range lines {
+		if !metadataLineRegex.MatchString(line) {
+			continue
+		}
+		for j := i + 1; j < len(lines); j++ {
+			if strings.TrimSpace(lines[j]) == "" {
+				continue
+			}
+			if indent := len(lines[j]) - len(strings.TrimLeft(lines[j], " \t")); indent > 0 {
+				childIndent = indent
+			}
+			break
+		}
+		break
+	}
+	if childIndent <= 0 {
+		return content
+	}
+
+	keyLine := func(key string) *regexp.Regexp {
+		return regexp.MustCompile(fmt.Sprintf(`^ {%d}%s:[ \t]*(\{\}[ \t]*)?(#.*)?$`, childIndent, key))
+	}
+	regexes := make([]*regexp.Regexp, 0, len(strip))
+	for key := range strip {
+		regexes = append(regexes, keyLine(key))
+	}
+
+	kept := make([]string, 0, len(lines))
+	for _, line := range lines {
+		drop := false
+		for _, re := range regexes {
+			if re.MatchString(line) {
+				drop = true
+				break
+			}
+		}
+		if !drop {
+			kept = append(kept, line)
+		}
+	}
+
+	return []byte(strings.Join(kept, "\n"))
+}
+
 // MappingResult to store result of diff
 type MappingResult struct {
 	Name           string
@@ -197,6 +297,12 @@ func parseContent(content []byte, defaultNamespace string, normalizeManifests bo
 		}
 	}
 
+	// Remove `labels:`/`annotations:` keys that are null or empty: they are
+	// semantically identical to an absent key, yet a textual diff between a
+	// manifest rendering the empty key and one omitting it would otherwise
+	// report a meaningless change (#1064).
+	content = stripEmptyMetadataKeys(content)
+
 	if isHook(parsedMetadata, excludedHooks...) {
 		return nil, nil
 	}

manifest/parse_test.go

@@ -3,6 +3,7 @@ package manifest_test
 import (
 	"os"
 	"sort"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -31,6 +32,96 @@ func TestPod(t *testing.T) {
 	)
 }
 
+func TestParseStripsEmptyMetadataKeys(t *testing.T) {
+	tests := []struct {
+		name     string
+		manifest string
+		wantName string
+		want     []string
+	}{
+		{
+			name: "null labels line is removed",
+			manifest: `# Source: chart/templates/cm.yaml
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: example
+  labels:
+data:
+  foo: bar
+`,
+			wantName: "default, example, ConfigMap (v1)",
+			want:     []string{"# Source: chart/templates/cm.yaml", "apiVersion: v1", "kind: ConfigMap", "metadata:", "  name: example", "data:", "  foo: bar"},
+		},
+		{
+			name: "empty flow labels map is removed",
+			manifest: `apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: example
+  labels: {}
+data:
+  foo: bar
+`,
+			wantName: "default, example, ConfigMap (v1)",
+			want:     []string{"apiVersion: v1", "kind: ConfigMap", "metadata:", "  name: example", "data:", "  foo: bar"},
+		},
+		{
+			name: "null annotations line is removed",
+			manifest: `apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: example
+  annotations:
+data:
+  foo: bar
+`,
+			wantName: "default, example, ConfigMap (v1)",
+			want:     []string{"apiVersion: v1", "kind: ConfigMap", "metadata:", "  name: example", "data:", "  foo: bar"},
+		},
+		{
+			name: "labels with content are kept",
+			manifest: `apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: example
+  labels:
+    app: kept
+data:
+  foo: bar
+`,
+			wantName: "default, example, ConfigMap (v1)",
+			want:     []string{"apiVersion: v1", "kind: ConfigMap", "metadata:", "  name: example", "  labels:", "    app: kept", "data:", "  foo: bar"},
+		},
+		{
+			name: "nested spec template labels are kept",
+			manifest: `apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: example
+  labels:
+spec:
+  template:
+    metadata:
+      labels:
+        app: kept
+`,
+			wantName: "default, example, Deployment (apps)",
+			want:     []string{"apiVersion: apps/v1", "kind: Deployment", "metadata:", "  name: example", "spec:", "  template:", "    metadata:", "      labels:", "        app: kept"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := Parse([]byte(tt.manifest), "default", false)
+			require.Len(t, result, 1)
+			mapping, ok := result[tt.wantName]
+			require.True(t, ok, "expected resource %q in %v", tt.wantName, foundObjects(result))
+			require.Equal(t, tt.want, strings.Split(mapping.Content, "\n"))
+		})
+	}
+}
+
 func TestPodNamespace(t *testing.T) {
 	spec, err := os.ReadFile("testdata/pod_namespace.yaml")
 	require.NoError(t, err)

manifest/util.go

@@ -42,10 +42,19 @@ func deleteStatusAndTidyMetadata(obj []byte) (map[string]interface{}, error) {
 
 // pruneNestedMap removes the given fields from the nested map found at key in
 // target. If the nested map ends up empty afterwards, key itself is removed
-// from target.
+// from target. A null-valued key (e.g. a chart template rendering "labels:"
+// with nothing under it) is removed as well, so it does not show up as a
+// confusing "- labels:" diff entry. See
+// https://github.com/databus23/helm-diff/issues/1064
 func pruneNestedMap(target map[string]interface{}, key string, fields ...string) {
 	sub, ok := target[key].(map[string]interface{})
 	if !ok {
+		// The key is either absent or explicitly null. A null value (JSON
+		// "labels": null) would otherwise survive as an empty "labels:" entry
+		// in the rendered YAML and produce a meaningless diff.
+		if target[key] == nil {
+			delete(target, key)
+		}
 		return
 	}
 

manifest/util_test.go

@@ -111,6 +111,75 @@ func Test_deleteStatusAndTidyMetadata(t *testing.T) {
 			},
 			wantErr: false,
 		},
+		{
+			name: "null labels are removed (chart renders bare labels: key)",
+			obj: []byte(`
+{
+    "kind": "ConfigMap",
+    "metadata": {
+        "labels": null,
+        "name": "example"
+    }
+}
+`),
+			want: map[string]interface{}{
+				"kind": "ConfigMap",
+				"metadata": map[string]interface{}{
+					"name": "example",
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "null annotations are removed",
+			obj: []byte(`
+{
+    "kind": "ConfigMap",
+    "metadata": {
+        "annotations": null,
+        "labels": {
+            "app": "kept"
+        },
+        "name": "example"
+    }
+}
+`),
+			want: map[string]interface{}{
+				"kind": "ConfigMap",
+				"metadata": map[string]interface{}{
+					"labels": map[string]interface{}{
+						"app": "kept",
+					},
+					"name": "example",
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "labels with other keys are kept",
+			obj: []byte(`
+{
+    "kind": "ConfigMap",
+    "metadata": {
+        "labels": {
+            "app.kubernetes.io/managed-by": "Helm",
+            "app.kubernetes.io/name": "myapp"
+        },
+        "name": "example"
+    }
+}
+`),
+			want: map[string]interface{}{
+				"kind": "ConfigMap",
+				"metadata": map[string]interface{}{
+					"labels": map[string]interface{}{
+						"app.kubernetes.io/name": "myapp",
+					},
+					"name": "example",
+				},
+			},
+			wantErr: false,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {