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>
pull/32061/head
Kunal Jain 2 months ago committed by Kunal Jain
parent 5b5d82d74e
commit bc82f2e18b

@ -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
}
}

@ -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 {

Loading…
Cancel
Save