diff --git a/pkg/chart/v2/loader/load.go b/pkg/chart/v2/loader/load.go index 028d59e82..eaabb1a24 100644 --- a/pkg/chart/v2/loader/load.go +++ b/pkg/chart/v2/loader/load.go @@ -36,6 +36,8 @@ import ( chart "helm.sh/helm/v4/pkg/chart/v2" ) +const mergePrefix = "\\*" + // ChartLoader loads a chart. type ChartLoader interface { Load() (*chart.Chart, error) @@ -222,6 +224,7 @@ func LoadValues(data io.Reader) (map[string]interface{}, error) { if err := yaml.Unmarshal(raw, ¤tMap); err != nil { return nil, fmt.Errorf("cannot unmarshal yaml document: %w", err) } + values = MergeMaps(values, currentMap) } return values, nil @@ -229,19 +232,129 @@ func LoadValues(data io.Reader) (map[string]interface{}, error) { // MergeMaps merges two maps. If a key exists in both maps, the value from b will be used. // If the value is a map, the maps will be merged recursively. +// If the value is a list, the lists will be merged func MergeMaps(a, b map[string]interface{}) map[string]interface{} { out := make(map[string]interface{}, len(a)) maps.Copy(out, a) for k, v := range b { - if v, ok := v.(map[string]interface{}); ok { + if val, ok := v.(map[string]interface{}); ok { if bv, ok := out[k]; ok { if bv, ok := bv.(map[string]interface{}); ok { - out[k] = MergeMaps(bv, v) + out[k] = MergeMaps(bv, val) continue } } + } else if strings.HasPrefix(k, mergePrefix) { + strippedKey := strings.TrimPrefix(k, mergePrefix) + if out[k] != nil { + out[strippedKey] = out[k] + delete(out, k) + } + + // 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 + } + + log.Printf("Warning: 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("Warning: 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("Warning: 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("Warning: Property \"%s\" mismatch during merge", strippedKey) + continue + } else { + out[strippedKey] = v + } } out[k] = v } return out } + +// 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 []map[string]interface{}) []map[string]interface{} { + out := make([]map[string]interface{}, len(a)) + copy(out, a) +OuterList: + for j, mapEntry := range b { + var uniqueKey string + var dedupValue interface{} + alreadyHasPrefix := false + for k, v := range mapEntry { + switch v.(type) { + case float64: + break + case string: + break + default: + continue + } + if strings.HasPrefix(k, mergePrefix) { + if alreadyHasPrefix { + duplicateKey := "\\*" + uniqueKey + log.Printf("Warning: Can't index on multiple keys \"%s: %v\" and \"%s: %v\" during list merge", duplicateKey, dedupValue, k, v) + // undo prior step + b[j][duplicateKey] = dedupValue + delete(b[j], uniqueKey) + out = append(out, mapEntry) + continue OuterList + } + + alreadyHasPrefix = true + uniqueKey = strings.TrimPrefix(k, mergePrefix) + dedupValue = v + b[j][uniqueKey] = v + delete(b[j], k) + } + } + if len(uniqueKey) > 0 { + for i, sourceMapEntry := range out { + for k, v := range sourceMapEntry { + if k == uniqueKey && v == dedupValue { + mergedMapEntry := MergeMaps(sourceMapEntry, mapEntry) + out[i] = mergedMapEntry + break + } + } + } + } else { + out = append(out, mapEntry) + } + } + return out + +} diff --git a/pkg/chart/v2/loader/load_test.go b/pkg/chart/v2/loader/load_test.go index 7eca5f402..d16edc4c9 100644 --- a/pkg/chart/v2/loader/load_test.go +++ b/pkg/chart/v2/loader/load_test.go @@ -596,7 +596,440 @@ 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 TestMergeListsDeep(t *testing.T) { + indexedCommonMap := map[string]interface{}{ + "groups": []map[string]interface{}{ + { + "name": "commonRules", + "id": float64(1331), + "rules": []map[string]interface{}{ + { + "name": "firstRule", + "expr": "commonFirstExpr", + "for": "3m", + }, + }, + }, + }, + } + indexedOverrideMap := map[string]interface{}{ + "\\*groups": []map[string]interface{}{ + { + "\\*name": "commonRules", + "\\*rules": []map[string]interface{}{ + { + "\\*name": "firstRule", + "for": "4m", + }, + { + "name": "uncommonRule", + "expr": "secondExpr", + }, + }, + }, + { + "name": "appRules", + "rules": []map[string]interface{}{ + { + "name": "appRule", + "expr": "appExpr", + "for": "4m", + }, + }, + }, + }, + } + indexedMergedMap := map[string]interface{}{ + "groups": []map[string]interface{}{ + { + "name": "commonRules", + "id": float64(1331), + "rules": []map[string]interface{}{ + { + "name": "firstRule", + "expr": "commonFirstExpr", + "for": "4m", + }, + { + "name": "uncommonRule", + "expr": "secondExpr", + }, + }, + }, + { + "name": "appRules", + "rules": []map[string]interface{}{ + { + "name": "appRule", + "expr": "appExpr", + "for": "4m", + }, + }, + }, + }, + } + testMap := MergeMaps(indexedCommonMap, indexedOverrideMap) + equal := reflect.DeepEqual(testMap, indexedMergedMap) + if !equal { + t.Errorf("Expected two level deep merged list. Expected values: %v, got %v", indexedMergedMap, testMap) + } + + indexedOverrideMap = map[string]interface{}{ + "\\*groups": []map[string]interface{}{ + { + "\\*id": float64(1331), + "\\*rules": []map[string]interface{}{ + { + "\\*name": "firstRule", + "for": "4m", + }, + { + "name": "uncommonRule", + "expr": "secondExpr", + }, + }, + }, + }, + } + indexedMergedMap = map[string]interface{}{ + "groups": []map[string]interface{}{ + { + "name": "commonRules", + "id": float64(1331), + "rules": []map[string]interface{}{ + { + "name": "firstRule", + "expr": "commonFirstExpr", + "for": "4m", + }, + { + "name": "uncommonRule", + "expr": "secondExpr", + }, + }, + }, + }, + } + testMap = MergeMaps(indexedCommonMap, indexedOverrideMap) + equal = reflect.DeepEqual(testMap, indexedMergedMap) + if !equal { + t.Errorf("Expected deep merged list on float64 key. Expected values: %v, got %v", indexedMergedMap, testMap) + } + + indexedOverrideMap = map[string]interface{}{ + "\\*groups": []map[string]interface{}{ + { + "\\*name": "commonRules", + "\\*rules": []map[string]interface{}{ + { + "\\*name": "firstRule", + "\\*for": "3m", + "expr": "OverrideFirstExpr", + }, + }, + }, + }, + } + indexedMergedMap = map[string]interface{}{ + "groups": []map[string]interface{}{ + { + "name": "commonRules", + "id": float64(1331), + "rules": []map[string]interface{}{ + { + "name": "firstRule", + "expr": "commonFirstExpr", + "for": "3m", + }, + { + "\\*name": "firstRule", + "\\*for": "3m", + "expr": "OverrideFirstExpr", + }, + }, + }, + }, + } + testMap = MergeMaps(indexedCommonMap, indexedOverrideMap) + equal = reflect.DeepEqual(testMap, indexedMergedMap) + if !equal { + t.Errorf("Expected composed list where we used two erroneously indicies. Expected values: %v, got %v", indexedMergedMap, testMap) } } @@ -646,7 +1079,6 @@ func verifyChart(t *testing.T, c *chart.Chart) { t.Errorf("Expected %s version %s, got %s", dep.Name(), exp["version"], dep.Metadata.Version) } } - } func verifyDependencies(t *testing.T, c *chart.Chart) {