From 788fbc772ce6a1ca9e963085d02bbb6a14aeefdc Mon Sep 17 00:00:00 2001 From: James M Leddy Date: Mon, 22 Sep 2025 17:10:01 -0400 Subject: [PATCH] chore(merge-lists): Add tests Per the parent pull request #30632, add tests Signed-off-by: James M Leddy --- pkg/chart/v2/loader/load.go | 86 +++++----- pkg/chart/v2/loader/load_test.go | 274 ++++++++++++++++++++++++++++++- 2 files changed, 319 insertions(+), 41 deletions(-) diff --git a/pkg/chart/v2/loader/load.go b/pkg/chart/v2/loader/load.go index 9f8f2dc7e..7e80c35d9 100644 --- a/pkg/chart/v2/loader/load.go +++ b/pkg/chart/v2/loader/load.go @@ -249,40 +249,60 @@ func MergeMaps(a, b map[string]interface{}) map[string]interface{} { continue } } - } else { + } else if strings.HasPrefix(k, mergePrefix) { strippedKey := strings.TrimPrefix(k, mergePrefix) if out[k] != nil { out[strippedKey] = out[k] delete(out, k) } - if sourceList, ok := out[strippedKey].([]any); ok && strings.HasPrefix(k, mergePrefix) { - - _, isMapSlice := sourceList[0].(map[string]any) - if isMapSlice { - val, ok := v.([]any) - if !ok { - // List is explicitly made null on a subsequent file - if v == nil { - delete(out, strippedKey) - continue - } else { - log.Printf("Property \"%s\" mismatch during merge", strippedKey) - continue - } - } + // Prefer non-prefixed in case of conflict + if b[strippedKey] != nil { + out[strippedKey] = b[strippedKey] + continue + } + + // List is explicitly made null on a subsequent file + if v == nil { + delete(out, strippedKey) + continue + } + + if sourceList, ok := out[strippedKey].([]map[string]interface{}); ok { + if val, ok := v.([]map[string]interface{}); ok { out[strippedKey] = MergeMapLists(sourceList, val) continue - } else if sourceList, ok := out[strippedKey].([]any); ok { - if val, ok := v.([]any); ok { - out[strippedKey] = append(sourceList, val...) - } else { - out[strippedKey] = v - } + } + + log.Printf("Property \"%s\" mismatch during merge", strippedKey) + continue + } else if sourceList, ok := out[strippedKey].([]float64); ok { + if val, ok := v.([]float64); ok { + out[strippedKey] = append(sourceList, val...) continue } - } + log.Printf("Property \"%s\" mismatch during merge", strippedKey) + continue + } else if sourceList, ok := out[strippedKey].([]bool); ok { + if val, ok := v.([]bool); ok { + out[strippedKey] = append(sourceList, val...) + continue + } + + log.Printf("Property \"%s\" mismatch during merge", strippedKey) + continue + } else if sourceList, ok := out[strippedKey].([]string); ok { + if val, ok := v.([]string); ok { + out[strippedKey] = append(sourceList, val...) + continue + } + + log.Printf("Property \"%s\" mismatch during merge", strippedKey) + continue + } else { + out[strippedKey] = v + } } out[k] = v } @@ -291,37 +311,23 @@ func MergeMaps(a, b map[string]interface{}) map[string]interface{} { // MergeMapLists merges two lists of maps. If a prefix of * is set on a map key, // that key will be used to de-duplicate/merge with the source map -func MergeMapLists(a, b []any) []any { +func MergeMapLists(a, b []map[string]interface{}) []map[string]interface{} { out := a for j, mapEntry := range b { - mapEntry, ok := mapEntry.(map[string]any) - if !ok { - continue - } - var uniqueKey string var dedupValue interface{} for k, v := range mapEntry { if strings.HasPrefix(k, mergePrefix) { uniqueKey = k dedupValue = v - bj, ok := b[j].(map[string]any) - if !ok { - continue - } - bj[strings.TrimPrefix(uniqueKey, mergePrefix)] = v - delete(bj, uniqueKey) - break + b[j][strings.TrimPrefix(uniqueKey, mergePrefix)] = v + delete(b[j], uniqueKey) } } if len(uniqueKey) > 0 { strippedMergeKey := strings.TrimPrefix(uniqueKey, mergePrefix) for i, sourceMapEntry := range out { - sourceMapEntry, ok := sourceMapEntry.(map[string]any) - if !ok { - continue - } for k, v := range sourceMapEntry { if (k == strippedMergeKey || k == uniqueKey) && v == dedupValue { mergedMapEntry := MergeMaps(sourceMapEntry, mapEntry) diff --git a/pkg/chart/v2/loader/load_test.go b/pkg/chart/v2/loader/load_test.go index c4ae646f6..3ce9347fb 100644 --- a/pkg/chart/v2/loader/load_test.go +++ b/pkg/chart/v2/loader/load_test.go @@ -595,10 +595,282 @@ func TestMergeValuesV2(t *testing.T) { } equal = reflect.DeepEqual(testMap, expectedMap) if !equal { - t.Errorf("Expected a map with different keys to merge properly with another map. Expected: %v, got %v", expectedMap, testMap) + t.Errorf("Expected a map with different keys to merge properly with another map. Expected values: %v, got %v", expectedMap, testMap) } } +func TestMergeLists(t *testing.T) { + stringCommonMap := map[string]interface{}{ + "foo": "bar", + "baz": []string{ + "cool", + }, + } + stringOverrideMap := map[string]interface{}{ + "foo": "bar", + "\\*baz": []string{ + "stuff", + }, + } + stringComposedMap := map[string]interface{}{ + "foo": "bar", + "baz": []string{ + "cool", + "stuff", + }, + } + testMap := MergeMaps(stringCommonMap, stringOverrideMap) + equal := reflect.DeepEqual(testMap, stringComposedMap) + if !equal { + t.Errorf("Expected lists of a bool type to compose when specified. Expected values: %v, got %v", stringComposedMap, testMap) + } + + floatCommonMap := map[string]interface{}{ + "foo": "bar", + "baz": []float64{ + 1, + }, + } + floatOverrideMap := map[string]interface{}{ + "foo": "bar", + "\\*baz": []float64{ + 2, + }, + } + floatComposedMap := map[string]interface{}{ + "foo": "bar", + "baz": []float64{ + 1, + 2, + }, + } + testMap = MergeMaps(floatCommonMap, floatOverrideMap) + equal = reflect.DeepEqual(testMap, floatComposedMap) + if !equal { + t.Errorf("Expected lists of a bool type to compose when specified. Expected values: %v, got %v", floatComposedMap, testMap) + } + + boolCommonMap := map[string]interface{}{ + "foo": "bar", + "baz": []bool{ + true, + }, + } + boolOverrideMap := map[string]interface{}{ + "foo": "bar", + "\\*baz": []bool{ + false, + }, + } + boolComposedMap := map[string]interface{}{ + "foo": "bar", + "baz": []bool{ + true, + false, + }, + } + testMap = MergeMaps(boolCommonMap, boolOverrideMap) + equal = reflect.DeepEqual(testMap, boolComposedMap) + if !equal { + t.Errorf("Expected lists of a bool type to compose when specified. Expected values: %v, got %v", boolComposedMap, testMap) + } + + testMap = MergeMaps(stringCommonMap, floatOverrideMap) + equal =reflect.DeepEqual(testMap, stringCommonMap) + if !equal { + t.Errorf("Expected lists of non-matching types not to compose. Expected values: %v, got %v", stringCommonMap, testMap) + } + + nilOverrideMap := map[string]interface{}{ + "foo": "bar", + "\\*baz": nil, + } + nilComposedMap := map[string]interface{}{ + "foo": "bar", + } + testMap = MergeMaps(stringCommonMap, nilOverrideMap) + equal = reflect.DeepEqual(testMap, nilComposedMap) + if !equal { + t.Errorf("Expected nil on maps of composed lists to override values. Expected values: %v, got %v", nilOverrideMap, testMap) + } + + stringCommonMap = map[string]interface{}{ + "foo": "bar", + "baz": []string{ + "cool", + }, + "\\*baz": []string{ + "nifty", + }, + } + stringComposedMap = map[string]interface{}{ + "foo": "bar", + "baz": []string{ + "nifty", + "stuff", + }, + } + testMap = MergeMaps(stringCommonMap, stringOverrideMap) + equal = reflect.DeepEqual(testMap, stringComposedMap) + if !equal { + t.Errorf("Expected nil on maps of composed lists to override values. Expected values: %v, got %v", stringComposedMap, testMap) + } + + stringOverrideMap = map[string]interface{}{ + "foo": "bar", + "baz": []string{ + "things", + }, + "\\*baz": []string{ + "stuff", + }, + } + stringComposedMap = map[string]interface{}{ + "foo": "bar", + "baz": []string{ + "things", + }, + } + testMap = MergeMaps(stringCommonMap, stringOverrideMap) + equal = reflect.DeepEqual(testMap, stringComposedMap) + if !equal { + t.Errorf("Expected composed lists to default to old behavior in case of conflict. Expected values: %v, got %v", stringComposedMap, testMap) + } + + indexedCommonMap := map[string]interface{}{ + "foo": []map[string]interface{}{ + { + "name": "bar", + "qualities": []string{ + "cool", + }, + }, + { + "name": "baz", + "qualities": []string{ + "stuff", + }, + }, + }, + } + indexedOverrideMap := map[string]interface{}{ + "\\*foo": []map[string]interface{}{ + { + "name": "bar", + "qualities": []string{ + "nifty", + }, + }, + { + "name": "bash", + "qualities": []string{ + "things", + }, + }, + }, + } + normalOverrideMap := map[string]interface{}{ + "foo": []map[string]interface{}{ + { + "\\*name": "bar", + "qualities": []string{ + "nifty", + }, + }, + { + "name": "bash", + "qualities": []string{ + "things", + }, + }, + }, + } + indexedMergedMap := map[string]interface{}{ + "foo": []map[string]interface{}{ + { + "name": "bar", + "qualities": []string{ + "cool", + }, + }, + { + "name": "baz", + "qualities": []string{ + "stuff", + }, + }, + { + "name": "bar", + "qualities": []string{ + "nifty", + }, + }, + { + "name": "bash", + "qualities": []string{ + "things", + }, + }, + }, + } + testMap = MergeMaps(indexedCommonMap, indexedOverrideMap) + equal = reflect.DeepEqual(testMap, indexedMergedMap) + if !equal { + t.Errorf("Expected composed map with duplicate names. Expected values: %v, got %v", indexedMergedMap, testMap) + } + + testMap = MergeMaps(indexedCommonMap, normalOverrideMap) + equal = reflect.DeepEqual(testMap, normalOverrideMap) + if !equal { + t.Errorf("Expected ordinary lists to be fully overriden. Expected values: %v, got %v", indexedOverrideMap, testMap) + } + + indexedOverrideMap = map[string]interface{}{ + "\\*foo": []map[string]interface{}{ + { + "\\*name": "bar", + "qualities": []string{ + "nifty", + }, + }, + { + "name": "bash", + "qualities": []string{ + "things", + }, + }, + }, + } + indexedMergedMap = map[string]interface{}{ + "foo": []map[string]interface{}{ + { + "name": "bar", + "qualities": []string{ + "nifty", + }, + }, + { + "name": "baz", + "qualities": []string{ + "stuff", + }, + }, + { + "name": "bash", + "qualities": []string{ + "things", + }, + }, + }, + } + testMap = MergeMaps(indexedCommonMap, indexedOverrideMap) + equal = reflect.DeepEqual(testMap, indexedMergedMap) + if !equal { + t.Errorf("Expected indexed 'bar' name to be overriden. Expected values: %v, got %v", indexedMergedMap, testMap) + } + +} + func verifyChart(t *testing.T, c *chart.Chart) { t.Helper() if c.Name() == "" {