diff --git a/pkg/cli/values/options.go b/pkg/cli/values/options.go index a28ffa99fd5..95cd118d9b1 100644 --- a/pkg/cli/values/options.go +++ b/pkg/cli/values/options.go @@ -17,13 +17,15 @@ limitations under the License. package values import ( + "bytes" + "io" "io/ioutil" "net/url" "os" "strings" "github.com/pkg/errors" - "sigs.k8s.io/yaml" + "k8s.io/apimachinery/pkg/util/yaml" "helm.sh/helm/v3/pkg/getter" "helm.sh/helm/v3/pkg/strvals" @@ -43,18 +45,15 @@ func (opts *Options) MergeValues(p getter.Providers) (map[string]interface{}, er // User specified a values files via -f/--values for _, filePath := range opts.ValueFiles { - currentMap := map[string]interface{}{} - - bytes, err := readFile(filePath, p) + data, err := readFile(filePath, p) if err != nil { return nil, err } - if err := yaml.Unmarshal(bytes, ¤tMap); err != nil { + base, err = mergeYaml(base, data) + if err != nil { return nil, errors.Wrapf(err, "failed to parse %s", filePath) } - // Merge with the previous map - base = mergeMaps(base, currentMap) } // User specified a value via --set @@ -74,11 +73,11 @@ func (opts *Options) MergeValues(p getter.Providers) (map[string]interface{}, er // User specified a value via --set-file for _, value := range opts.FileValues { reader := func(rs []rune) (interface{}, error) { - bytes, err := readFile(string(rs), p) + data, err := readFile(string(rs), p) if err != nil { return nil, err } - return string(bytes), err + return string(data), err } if err := strvals.ParseIntoFile(value, base, reader); err != nil { return nil, errors.Wrap(err, "failed parsing --set-file data") @@ -88,6 +87,28 @@ func (opts *Options) MergeValues(p getter.Providers) (map[string]interface{}, er return base, nil } +// mergeYaml merges data into an existing Go map. data must be a YAML format. +// If the file contains multiple documents, each document will be merged separately. +func mergeYaml(base map[string]interface{}, data []byte) (map[string]interface{}, error) { + decoder := yaml.NewYAMLOrJSONDecoder(bytes.NewReader(data), 4096) + + for { + currentMap := map[string]interface{}{} + err := decoder.Decode(¤tMap) + + if err == io.EOF { + break + } else if err != nil { + return nil, err + } + + // Merge with the previous map + base = mergeMaps(base, currentMap) + } + + return base, nil +} + func mergeMaps(a, b map[string]interface{}) map[string]interface{} { out := make(map[string]interface{}, len(a)) for k, v := range a { diff --git a/pkg/cli/values/options_test.go b/pkg/cli/values/options_test.go index d988274bfaf..901d72c4f92 100644 --- a/pkg/cli/values/options_test.go +++ b/pkg/cli/values/options_test.go @@ -17,7 +17,9 @@ limitations under the License. package values import ( + "fmt" "reflect" + "strings" "testing" ) @@ -75,3 +77,117 @@ func TestMergeValues(t *testing.T) { t.Errorf("Expected a map with different keys to merge properly with another map. Expected: %v, got %v", expectedMap, testMap) } } + +func TestMergeYaml(t *testing.T) { + testCases := []struct { + desc string + input string + err bool + expect map[string]interface{} + }{ + {"string value", "key: value1", false, map[string]interface{}{ + "key": "value1", + }}, + {"int value", "key: 1000", false, map[string]interface{}{ + "key": 1000, + }}, + {"nil value", "key: ~", false, map[string]interface{}{ + "key": nil, + }}, + + {"sub map", "key: {subkey: sub value}", false, map[string]interface{}{ + "key": map[string]interface{}{ + "subkey": "sub value", + }, + }}, + + {"empty document", "---", false, map[string]interface{}{}}, + {"empty document with newline", "---\n", false, map[string]interface{}{}}, + + {"empty documents", "\n---\n", false, map[string]interface{}{}}, + + {"multiple documents w/o first separator", "key1: value1\n---\nkey2: value2", false, map[string]interface{}{ + "key1": "value1", + "key2": "value2", + }}, + {"multiple documents w/ first separator", "---\nkey1: value1\n---\nkey2: value2", false, map[string]interface{}{ + "key1": "value1", + "key2": "value2", + }}, + {"override keys with string", "---\nkey1: value1\n---\nkey1: value2", false, map[string]interface{}{ + "key1": "value2", + }}, + {"override keys with nil", "key1: value1\n---\nkey2: ", false, map[string]interface{}{ + "key1": "value1", + "key2": nil, + }}, + {"override keys with nil + additional separator", "key1: value1\n---\nkey2: \n---", false, map[string]interface{}{ + "key1": "value1", + "key2": nil, + }}, + {"override with map", "---\nfoo: {key: value}\n#---\nfoo: {key: value2}", false, map[string]interface{}{ + "foo": map[string]interface{}{ + "key": "value2", + }, + }}, + + {"yaml syntax error in 2nd document", "key1: value1\n---\nkey2 ", true, nil}, + + {"7 yaml documents", "key1: value1\n---\nkey2: value2\n---\nkey3: value3\n---\nkey4: value4\n---\nkey5: value5\n---\nkey6: value6\n---\nkey7: value7\n---", false, map[string]interface{}{ + "key1": "value1", + "key2": "value2", + "key3": "value3", + "key4": "value4", + "key5": "value5", + "key6": "value6", + "key7": "value7", + }}, + + {"comment in yaml separator", "---\nfoo: bar\n--- # with Comment\nbaz: biz", false, map[string]interface{}{ + "foo": "bar", + "baz": "biz", + }}, + + {"yaml anchors", "---\nkey1: &value value\nkey2:\n- *value", false, map[string]interface{}{ + "key1": "value", + "key2": []interface{}{"value"}, + }}, + + {"separator in multiline", "key: >-\n hello\n ---\n world", false, map[string]interface{}{ + "key": "hello --- world", + }}, + + {"multiple json", "{\"1\":2}{\"3\":4}", false, map[string]interface{}{ + "1": 2, + "3": 4, + }}, + {"json with spaces", " {\"1\":2} ", false, map[string]interface{}{ + "1": 2, + }}, + + {"big yaml", strings.Repeat("---\nkey1: value1\n---\nkey2: value2\n", 10*1024), false, map[string]interface{}{ + "key1": "value1", + "key2": "value2", + }}, + } + + for _, testCase := range testCases { + base := map[string]interface{}{} + base, err := mergeYaml(base, []byte(testCase.input)) + + switch { + case testCase.err && err == nil: + t.Errorf("%s: unexpected non-error", testCase.desc) + continue + case !testCase.err && err != nil: + t.Errorf("%s: unexpected error: %v", testCase.desc, err) + continue + case err != nil: + continue + } + + if fmt.Sprintf("%#v", testCase.expect) != fmt.Sprintf("%#v", base) { + t.Errorf("%s: objects were not equal: \n%#v\n%#v", testCase.desc, testCase.expect, base) + } + } +}