From bc82f2e18b31a45d51c0538452ec76e68ad70ffa Mon Sep 17 00:00:00 2001
From: Kunal Jain <kunaljain@adobe.com>
Date: Mon, 20 Apr 2026 14:34:17 -0700
Subject: [PATCH] fix: narrow SSA dedup to name-keyed fields, guard empty name

- Add nameKeyedFields allowlist (env, containers, initContainers,
  ephemeralContainers, volumes, imagePullSecrets) so deduplicateListMaps
  only touches list fields that are actually keyed by "name" under
  server-side apply merge semantics; volumeMounts and other lists that
  incidentally contain a "name" field are no longer collapsed
- processNamedList now skips deduplication when any item's "name" is a
  missing or empty string, preventing unrelated entries from being
  treated as duplicates
- Add regression tests: volumeMounts with duplicate names preserved,
  env list with empty name not deduplicated

Addresses Copilot review comments on PR #32061.

Signed-off-by: Kunal Jain <qlapon@gmail.com>
---
 pkg/kube/client.go      | 27 ++++++++++++++++++++++-----
 pkg/kube/client_test.go | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 5 deletions(-)

diff --git a/pkg/kube/client.go b/pkg/kube/client.go
index b5ffb3c7a..4cbfa947e 100644
--- a/pkg/kube/client.go
+++ b/pkg/kube/client.go
@@ -1299,10 +1299,23 @@ func copyRequestStreamToWriter(request *rest.Request, podName, containerName str
 	return nil
 }
 
+// nameKeyedFields is the set of Kubernetes list fields that are keyed by the
+// "name" field under server-side apply merge semantics. Only these fields are
+// candidates for deduplication; other lists that incidentally contain a "name"
+// field (e.g. volumeMounts, which is keyed by mountPath) must not be touched.
+var nameKeyedFields = map[string]bool{
+	"containers":          true,
+	"initContainers":      true,
+	"ephemeralContainers": true,
+	"env":                 true,
+	"volumes":             true,
+	"imagePullSecrets":    true,
+}
+
 // deduplicateListMaps walks an unstructured Kubernetes object and deduplicates
-// any list field where every item is a map containing a "name" key (e.g. env
-// vars, volumes, containers). When duplicates are found the last occurrence
-// wins, matching Kubernetes client-side-apply semantics. Returns true if any
+// list fields that are keyed by "name" under server-side apply merge semantics
+// (see nameKeyedFields). When duplicates are found the last occurrence wins,
+// matching Kubernetes client-side-apply semantics. Returns true if any
 // deduplication occurred.
 func deduplicateListMaps(obj map[string]interface{}) bool {
 	deduped := false
@@ -1313,6 +1326,9 @@ func deduplicateListMaps(obj map[string]interface{}) bool {
 				deduped = true
 			}
 		case []interface{}:
+			if !nameKeyedFields[key] {
+				continue
+			}
 			newList, changed := processNamedList(v)
 			if changed {
 				obj[key] = newList
@@ -1344,13 +1360,14 @@ func processNamedList(list []interface{}) ([]interface{}, bool) {
 		return list, changed
 	}
 
-	// Only deduplicate if every item is a map with a "name" key.
+	// Only deduplicate if every item is a map with a non-empty string "name".
 	for _, item := range list {
 		m, ok := item.(map[string]interface{})
 		if !ok {
 			return list, changed
 		}
-		if _, hasName := m["name"]; !hasName {
+		nameVal, isString := m["name"].(string)
+		if !isString || nameVal == "" {
 			return list, changed
 		}
 	}
diff --git a/pkg/kube/client_test.go b/pkg/kube/client_test.go
index 82d9e0e05..09f302d19 100644
--- a/pkg/kube/client_test.go
+++ b/pkg/kube/client_test.go
@@ -1938,6 +1938,40 @@ func TestDeduplicateListMaps(t *testing.T) {
 			},
 			changed: false,
 		},
+		{
+			// volumeMounts is keyed by mountPath, not name — same name with
+			// different mountPaths must NOT be collapsed.
+			name: "volumeMounts with duplicate names not deduplicated",
+			input: map[string]interface{}{
+				"volumeMounts": []interface{}{
+					map[string]interface{}{"name": "data", "mountPath": "/data"},
+					map[string]interface{}{"name": "data", "mountPath": "/backup"},
+				},
+			},
+			expected: map[string]interface{}{
+				"volumeMounts": []interface{}{
+					map[string]interface{}{"name": "data", "mountPath": "/data"},
+					map[string]interface{}{"name": "data", "mountPath": "/backup"},
+				},
+			},
+			changed: false,
+		},
+		{
+			name: "list with empty name not deduplicated",
+			input: map[string]interface{}{
+				"env": []interface{}{
+					map[string]interface{}{"name": "", "value": "a"},
+					map[string]interface{}{"name": "", "value": "b"},
+				},
+			},
+			expected: map[string]interface{}{
+				"env": []interface{}{
+					map[string]interface{}{"name": "", "value": "a"},
+					map[string]interface{}{"name": "", "value": "b"},
+				},
+			},
+			changed: false,
+		},
 	}
 
 	for _, tt := range tests {