fix: Duplicate env vars are now treated as errors in helm v4

Signed-off-by: Kunal Jain <qlapon@gmail.com>
pull/32061/head
Kunal Jain 3 weeks ago committed by Kunal Jain
parent 29d309e56b
commit 5b5d82d74e

@ -1217,6 +1217,19 @@ func patchResourceServerSide(target *resource.Info, dryRun bool, forceConflicts
WithFieldManager(getManagedFieldsManager()).
WithFieldValidation(string(fieldValidationDirective))
// Deduplicate list-map entries (e.g. env vars) before server-side apply.
// Server-side apply rejects manifests with duplicate keys in list-maps, but
// Kubernetes client-side apply previously allowed them (last value wins).
// We preserve that behavior by deduplicating here, keeping the last occurrence.
if u, ok := target.Object.(*unstructured.Unstructured); ok {
if deduplicateListMaps(u.Object) {
slog.Warn("deduplicated list-map entries in manifest; please remove duplicates from the chart",
slog.String("name", target.Name),
slog.String("namespace", target.Namespace),
slog.String("gvk", target.Mapping.GroupVersionKind.String()))
}
}
// Send the full object to be applied on the server side.
data, err := runtime.Encode(unstructured.UnstructuredJSONScheme, target.Object)
if err != nil {
@ -1286,6 +1299,86 @@ func copyRequestStreamToWriter(request *rest.Request, podName, containerName str
return nil
}
// 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
// deduplication occurred.
func deduplicateListMaps(obj map[string]interface{}) bool {
deduped := false
for key, val := range obj {
switch v := val.(type) {
case map[string]interface{}:
if deduplicateListMaps(v) {
deduped = true
}
case []interface{}:
newList, changed := processNamedList(v)
if changed {
obj[key] = newList
deduped = true
}
}
}
return deduped
}
// processNamedList recurses into list items and deduplicates the list if every
// item is a map with a "name" key. The last occurrence of each name is kept and
// the relative order of surviving items is preserved. Returns the (possibly
// modified) list and whether any change was made.
func processNamedList(list []interface{}) ([]interface{}, bool) {
changed := false
// Recurse into each map item first.
for i, item := range list {
if m, ok := item.(map[string]interface{}); ok {
if deduplicateListMaps(m) {
list[i] = m
changed = true
}
}
}
if len(list) < 2 {
return list, changed
}
// Only deduplicate if every item is a map with a "name" key.
for _, item := range list {
m, ok := item.(map[string]interface{})
if !ok {
return list, changed
}
if _, hasName := m["name"]; !hasName {
return list, changed
}
}
// Build deduplicated list keeping the last occurrence of each name.
seen := make(map[string]bool)
result := make([]interface{}, 0, len(list))
hasDups := false
for i := len(list) - 1; i >= 0; i-- {
m := list[i].(map[string]interface{})
name, _ := m["name"].(string)
if !seen[name] {
seen[name] = true
result = append(result, list[i])
} else {
hasDups = true
}
}
if !hasDups {
return list, changed
}
// Reverse to restore the original relative order of surviving items.
for i, j := 0, len(result)-1; i < j; i, j = i+1, j-1 {
result[i], result[j] = result[j], result[i]
}
return result, true
}
// scrubValidationError removes kubectl info from the message.
func scrubValidationError(err error) error {
if err == nil {

@ -1859,6 +1859,96 @@ func TestDetermineFieldValidationDirective(t *testing.T) {
assert.Equal(t, FieldValidationDirectiveStrict, determineFieldValidationDirective(true))
}
func TestDeduplicateListMaps(t *testing.T) {
tests := []struct {
name string
input map[string]interface{}
expected map[string]interface{}
changed bool
}{
{
name: "no duplicates",
input: map[string]interface{}{
"env": []interface{}{
map[string]interface{}{"name": "FOO", "value": "1"},
map[string]interface{}{"name": "BAR", "value": "2"},
},
},
expected: map[string]interface{}{
"env": []interface{}{
map[string]interface{}{"name": "FOO", "value": "1"},
map[string]interface{}{"name": "BAR", "value": "2"},
},
},
changed: false,
},
{
name: "duplicate env var keeps last value",
input: map[string]interface{}{
"env": []interface{}{
map[string]interface{}{"name": "FOO", "value": "first"},
map[string]interface{}{"name": "BAR", "value": "2"},
map[string]interface{}{"name": "FOO", "value": "last"},
},
},
expected: map[string]interface{}{
"env": []interface{}{
map[string]interface{}{"name": "BAR", "value": "2"},
map[string]interface{}{"name": "FOO", "value": "last"},
},
},
changed: true,
},
{
name: "nested container env dedup",
input: map[string]interface{}{
"spec": map[string]interface{}{
"containers": []interface{}{
map[string]interface{}{
"name": "app",
"env": []interface{}{
map[string]interface{}{"name": "X", "value": "a"},
map[string]interface{}{"name": "X", "value": "b"},
},
},
},
},
},
expected: map[string]interface{}{
"spec": map[string]interface{}{
"containers": []interface{}{
map[string]interface{}{
"name": "app",
"env": []interface{}{
map[string]interface{}{"name": "X", "value": "b"},
},
},
},
},
},
changed: true,
},
{
name: "non-named list not deduplicated",
input: map[string]interface{}{
"args": []interface{}{"--flag=a", "--flag=b"},
},
expected: map[string]interface{}{
"args": []interface{}{"--flag=a", "--flag=b"},
},
changed: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := deduplicateListMaps(tt.input)
assert.Equal(t, tt.changed, got)
assert.Equal(t, tt.expected, tt.input)
})
}
}
func TestClientWaitContextCancellationLegacy(t *testing.T) {
podList := newPodList("starfish", "otter")

Loading…
Cancel
Save