Merge pull request #13655 from LuBingtan/dev

feat: support multi-document values files

pkg/chart/loader/load.go

@@ -17,14 +17,17 @@ limitations under the License.
 package loader
 
 import (
+	"bufio"
 	"bytes"
 	"encoding/json"
+	"io"
 	"log"
 	"os"
 	"path/filepath"
 	"strings"
 
 	"github.com/pkg/errors"
+	utilyaml "k8s.io/apimachinery/pkg/util/yaml"
 	"sigs.k8s.io/yaml"
 
 	"helm.sh/helm/v4/pkg/chart"
@@ -104,13 +107,11 @@ func LoadFiles(files []*BufferedFile) (*chart.Chart, error) {
 				return c, errors.Wrap(err, "cannot load Chart.lock")
 			}
 		case f.Name == "values.yaml":
-			c.Values = make(map[string]interface{})
+			values, err := LoadValues(bytes.NewReader(f.Data))
-			if err := yaml.Unmarshal(f.Data, &c.Values, func(d *json.Decoder) *json.Decoder {
+			if err != nil {
-				d.UseNumber()
-				return d
-			}); err != nil {
 				return c, errors.Wrap(err, "cannot load values.yaml")
 			}
+			c.Values = values
 		case f.Name == "values.schema.json":
 			c.Schema = f.Data
 
@@ -205,3 +206,51 @@ func LoadFiles(files []*BufferedFile) (*chart.Chart, error) {
 
 	return c, nil
 }
+
+// LoadValues loads values from a reader.
+//
+// The reader is expected to contain one or more YAML documents, the values of which are merged.
+// And the values can be either a chart's default values or a user-supplied values.
+func LoadValues(data io.Reader) (map[string]interface{}, error) {
+	values := map[string]interface{}{}
+	reader := utilyaml.NewYAMLReader(bufio.NewReader(data))
+	for {
+		currentMap := map[string]interface{}{}
+		raw, err := reader.Read()
+		if err != nil {
+			if err == io.EOF {
+				break
+			}
+			return nil, errors.Wrap(err, "error reading yaml document")
+		}
+		if err := yaml.Unmarshal(raw, &currentMap, func(d *json.Decoder) *json.Decoder {
+			d.UseNumber()
+			return d
+		}); err != nil {
+			return nil, errors.Wrap(err, "cannot unmarshal yaml document")
+		}
+		values = MergeMaps(values, currentMap)
+	}
+	return values, nil
+}
+
+// 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.
+func MergeMaps(a, b map[string]interface{}) map[string]interface{} {
+	out := make(map[string]interface{}, len(a))
+	for k, v := range a {
+		out[k] = v
+	}
+	for k, v := range b {
+		if v, 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)
+					continue
+				}
+			}
+		}
+		out[k] = v
+	}
+	return out
+}

pkg/chart/loader/load_test.go

@@ -24,6 +24,7 @@ import (
 	"log"
 	"os"
 	"path/filepath"
+	"reflect"
 	"runtime"
 	"strings"
 	"testing"
@@ -488,6 +489,115 @@ func TestLoadInvalidArchive(t *testing.T) {
 	}
 }
 
+func TestLoadValues(t *testing.T) {
+	testCases := map[string]struct {
+		data          []byte
+		expctedValues map[string]interface{}
+	}{
+		"It should load values correctly": {
+			data: []byte(`
+foo:
+  image: foo:v1
+bar:
+  version: v2
+`),
+			expctedValues: map[string]interface{}{
+				"foo": map[string]interface{}{
+					"image": "foo:v1",
+				},
+				"bar": map[string]interface{}{
+					"version": "v2",
+				},
+			},
+		},
+		"It should load values correctly with multiple documents in one file": {
+			data: []byte(`
+foo:
+  image: foo:v1
+bar:
+  version: v2
+---
+foo:
+  image: foo:v2
+`),
+			expctedValues: map[string]interface{}{
+				"foo": map[string]interface{}{
+					"image": "foo:v2",
+				},
+				"bar": map[string]interface{}{
+					"version": "v2",
+				},
+			},
+		},
+	}
+	for testName, testCase := range testCases {
+		t.Run(testName, func(tt *testing.T) {
+			values, err := LoadValues(bytes.NewReader(testCase.data))
+			if err != nil {
+				tt.Fatal(err)
+			}
+			if !reflect.DeepEqual(values, testCase.expctedValues) {
+				tt.Errorf("Expected values: %v, got %v", testCase.expctedValues, values)
+			}
+		})
+	}
+}
+
+func TestMergeValues(t *testing.T) {
+	nestedMap := map[string]interface{}{
+		"foo": "bar",
+		"baz": map[string]string{
+			"cool": "stuff",
+		},
+	}
+	anotherNestedMap := map[string]interface{}{
+		"foo": "bar",
+		"baz": map[string]string{
+			"cool":    "things",
+			"awesome": "stuff",
+		},
+	}
+	flatMap := map[string]interface{}{
+		"foo": "bar",
+		"baz": "stuff",
+	}
+	anotherFlatMap := map[string]interface{}{
+		"testing": "fun",
+	}
+
+	testMap := MergeMaps(flatMap, nestedMap)
+	equal := reflect.DeepEqual(testMap, nestedMap)
+	if !equal {
+		t.Errorf("Expected a nested map to overwrite a flat value. Expected: %v, got %v", nestedMap, testMap)
+	}
+
+	testMap = MergeMaps(nestedMap, flatMap)
+	equal = reflect.DeepEqual(testMap, flatMap)
+	if !equal {
+		t.Errorf("Expected a flat value to overwrite a map. Expected: %v, got %v", flatMap, testMap)
+	}
+
+	testMap = MergeMaps(nestedMap, anotherNestedMap)
+	equal = reflect.DeepEqual(testMap, anotherNestedMap)
+	if !equal {
+		t.Errorf("Expected a nested map to overwrite another nested map. Expected: %v, got %v", anotherNestedMap, testMap)
+	}
+
+	testMap = MergeMaps(anotherFlatMap, anotherNestedMap)
+	expectedMap := map[string]interface{}{
+		"testing": "fun",
+		"foo":     "bar",
+		"baz": map[string]string{
+			"cool":    "things",
+			"awesome": "stuff",
+		},
+	}
+	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)
+	}
+}
+
 func verifyChart(t *testing.T, c *chart.Chart) {
 	t.Helper()
 	if c.Name() == "" {

pkg/cli/values/options.go

@@ -17,6 +17,7 @@ limitations under the License.
 package values
 
 import (
+	"bytes"
 	"encoding/json"
 	"io"
 	"net/url"
@@ -24,8 +25,8 @@ import (
 	"strings"
 
 	"github.com/pkg/errors"
-	"sigs.k8s.io/yaml"
 
+	"helm.sh/helm/v4/pkg/chart/loader"
 	"helm.sh/helm/v4/pkg/getter"
 	"helm.sh/helm/v4/pkg/strvals"
 )
@@ -47,18 +48,16 @@ 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{}{}
+		raw, err := readFile(filePath, p)
-
-		bytes, err := readFile(filePath, p)
 		if err != nil {
 			return nil, err
 		}
-
+		currentMap, err := loader.LoadValues(bytes.NewReader(raw))
-		if err := yaml.Unmarshal(bytes, &currentMap); err != nil {
+		if err != nil {
 			return nil, errors.Wrapf(err, "failed to parse %s", filePath)
 		}
 		// Merge with the previous map
-		base = mergeMaps(base, currentMap)
+		base = loader.MergeMaps(base, currentMap)
 	}
 
 	// User specified a value via --set-json
@@ -70,7 +69,7 @@ func (opts *Options) MergeValues(p getter.Providers) (map[string]interface{}, er
 			if err := json.Unmarshal([]byte(trimmedValue), &jsonMap); err != nil {
 				return nil, errors.Errorf("failed parsing --set-json data JSON: %s", value)
 			}
-			base = mergeMaps(base, jsonMap)
+			base = loader.MergeMaps(base, jsonMap)
 		} else {
 			// Otherwise, parse it as key=value format
 			if err := strvals.ParseJSON(value, base); err != nil {
@@ -117,25 +116,6 @@ func (opts *Options) MergeValues(p getter.Providers) (map[string]interface{}, er
 	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 {
-		out[k] = v
-	}
-	for k, v := range b {
-		if v, 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)
-					continue
-				}
-			}
-		}
-		out[k] = v
-	}
-	return out
-}
-
 // readFile load a file from stdin, the local directory, or a remote file with a url.
 func readFile(filePath string, p getter.Providers) ([]byte, error) {
 	if strings.TrimSpace(filePath) == "-" {

pkg/cli/values/options_test.go

@@ -23,61 +23,6 @@ import (
 	"helm.sh/helm/v4/pkg/getter"
 )
 
-func TestMergeMaps(t *testing.T) {
-	nestedMap := map[string]interface{}{
-		"foo": "bar",
-		"baz": map[string]string{
-			"cool": "stuff",
-		},
-	}
-	anotherNestedMap := map[string]interface{}{
-		"foo": "bar",
-		"baz": map[string]string{
-			"cool":    "things",
-			"awesome": "stuff",
-		},
-	}
-	flatMap := map[string]interface{}{
-		"foo": "bar",
-		"baz": "stuff",
-	}
-	anotherFlatMap := map[string]interface{}{
-		"testing": "fun",
-	}
-
-	testMap := mergeMaps(flatMap, nestedMap)
-	equal := reflect.DeepEqual(testMap, nestedMap)
-	if !equal {
-		t.Errorf("Expected a nested map to overwrite a flat value. Expected: %v, got %v", nestedMap, testMap)
-	}
-
-	testMap = mergeMaps(nestedMap, flatMap)
-	equal = reflect.DeepEqual(testMap, flatMap)
-	if !equal {
-		t.Errorf("Expected a flat value to overwrite a map. Expected: %v, got %v", flatMap, testMap)
-	}
-
-	testMap = mergeMaps(nestedMap, anotherNestedMap)
-	equal = reflect.DeepEqual(testMap, anotherNestedMap)
-	if !equal {
-		t.Errorf("Expected a nested map to overwrite another nested map. Expected: %v, got %v", anotherNestedMap, testMap)
-	}
-
-	testMap = mergeMaps(anotherFlatMap, anotherNestedMap)
-	expectedMap := map[string]interface{}{
-		"testing": "fun",
-		"foo":     "bar",
-		"baz": map[string]string{
-			"cool":    "things",
-			"awesome": "stuff",
-		},
-	}
-	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)
-	}
-}
-
 func TestReadFile(t *testing.T) {
 	var p getter.Providers
 	filePath := "%a.txt"