Merge --values-dir to --values and delete --values-dir flag to simpli…

…fy usage and compatibility
Bhargav-InfraCloud · Apr 28, 2023 · 92e43e7 · 92e43e7
1 parent 87da7b1
commit 92e43e7
Show file tree

Hide file tree

Showing 3 changed files with 92 additions and 100 deletions.
diff --git a/cmd/helm/flags.go b/cmd/helm/flags.go
@@ -43,8 +43,7 @@ const (
 )
 
 func addValueOptionsFlags(f *pflag.FlagSet, v *values.Options) {
-	f.StringSliceVarP(&v.ValueFiles, "values", "f", []string{}, "specify values in a YAML file or a URL (can specify multiple)")
-	f.StringSliceVarP(&v.ValuesDirectories, "values-directory", "d", []string{}, "specify values directory with YAML files which is read recursively (can specify multiple)")
+	f.StringSliceVarP(&v.ValueFiles, "values", "f", []string{}, "specify values in a YAML file, a URL or a directory (can specify multiple) if the parameter is a directory, reads YAML files contained recursively")
 	f.StringArrayVar(&v.Values, "set", []string{}, "set values on the command line (can specify multiple or separate values with commas: key1=val1,key2=val2)")
 	f.StringArrayVar(&v.StringValues, "set-string", []string{}, "set STRING values on the command line (can specify multiple or separate values with commas: key1=val1,key2=val2)")
 	f.StringArrayVar(&v.FileValues, "set-file", []string{}, "set values from respective files specified via the command line (can specify multiple or separate values with commas: key1=path1,key2=path2)")

diff --git a/pkg/cli/values/options.go b/pkg/cli/values/options.go
@@ -34,50 +34,48 @@ const yamlExt = `.yaml`
 
 // Options captures the different ways to specify values
 type Options struct {
-	ValueFiles        []string // -f/--values
-	ValuesDirectories []string // -d/--values-directory
-	StringValues      []string // --set-string
-	Values            []string // --set
-	FileValues        []string // --set-file
-	JSONValues        []string // --set-json
+	ValueFiles   []string // -f/--values
+	StringValues []string // --set-string
+	Values       []string // --set
+	FileValues   []string // --set-file
+	JSONValues   []string // --set-json
 }
 
-// MergeValues merges values from files specified via -f/--values, files in directories
-// specified by -d/--values-directory and directly via --set-json, --set, --set-string,
+// MergeValues merges values from files or files in directories specified via -f/--values,
+// and directly via --set-json, --set, --set-string,
 // or --set-file, marshaling them to YAML
 func (opts *Options) MergeValues(p getter.Providers) (map[string]interface{}, error) {
 	base := map[string]interface{}{}
 
-	var valuesFiles []string
+	// User specified a file or directory via -f/--values
+	for _, filePath := range opts.ValueFiles {
+		var err error
 
-	// User specified values directories via -d/--values-directory
-	for _, dir := range opts.ValuesDirectories {
-		// Recursive list of YAML files in input values directory
-		files, err := recursiveListOfFiles(dir, yamlExt)
+		fileInfo, err := os.Stat(filePath)
 		if err != nil {
-			// Error already wrapped
 			return nil, err
 		}
 
-		valuesFiles = append(valuesFiles, files...)
-	}
-
-	// User specified values files via -f/--values
-	valuesFiles = append(valuesFiles, opts.ValueFiles...)
-
-	for _, filePath := range valuesFiles {
-		currentMap := map[string]interface{}{}
-
-		bytes, err := readFile(filePath, p)
-		if err != nil {
-			return nil, err
-		}
-
-		if err := yaml.Unmarshal(bytes, &currentMap); err != nil {
-			return nil, errors.Wrapf(err, "failed to parse %s", filePath)
+		// Check if file is a directory
+		if fileInfo.IsDir() {
+			// Recursive list of YAML files in input values directory
+			filesInDir, err := recursiveListOfFiles(filePath, yamlExt)
+			if err != nil {
+				// Error already wrapped
+				return nil, err
+			}
+			for _, fileInDir := range filesInDir {
+				base, err = mergeValuesFile(base, fileInDir, p)
+				if err != nil {
+					return nil, err
+				}
+			}
+		} else {
+			base, err = mergeValuesFile(base, filePath, p)
+			if err != nil {
+				return nil, err
+			}
 		}
-		// Merge with the previous map
-		base = mergeMaps(base, currentMap)
 	}
 
 	// User specified a value via --set-json
@@ -137,6 +135,22 @@ func mergeMaps(a, b map[string]interface{}) map[string]interface{} {
 	return out
 }
 
+// mergeFile load and parse a values file and merge its content with the map provided
+func mergeValuesFile(base map[string]interface{}, filePath string, p getter.Providers) (map[string]interface{}, error) {
+	currentMap := map[string]interface{}{}
+
+	bytes, err := readFile(filePath, p)
+	if err != nil {
+		return nil, err
+	}
+
+	if err := yaml.Unmarshal(bytes, &currentMap); err != nil {
+		return nil, errors.Wrapf(err, "failed to parse %s", filePath)
+	}
+	// Merge with the previous map
+	return mergeMaps(base, currentMap), err
+}
+
 // 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) == "-" {

diff --git a/pkg/cli/values/options_test.go b/pkg/cli/values/options_test.go
@@ -137,12 +137,11 @@ func TestOptions_MergeValues(t *testing.T) {
 		{
 			name: "Empty-Values",
 			opts: Options{
-				ValueFiles:        []string{},
-				ValuesDirectories: []string{},
-				StringValues:      []string{},
-				Values:            []string{},
-				FileValues:        []string{},
-				JSONValues:        []string{},
+				ValueFiles:   []string{},
+				StringValues: []string{},
+				Values:       []string{},
+				FileValues:   []string{},
+				JSONValues:   []string{},
 			},
 			args: args{
 				p: []getter.Provider{},
@@ -157,11 +156,10 @@ func TestOptions_MergeValues(t *testing.T) {
 					"testdata/noconflicts/values.yaml",
 					"testdata/noconflicts/extras.yaml",
 				},
-				ValuesDirectories: []string{},
-				StringValues:      []string{},
-				Values:            []string{},
-				FileValues:        []string{},
-				JSONValues:        []string{},
+				StringValues: []string{},
+				Values:       []string{},
+				FileValues:   []string{},
+				JSONValues:   []string{},
 			},
 			args: args{
 				p: []getter.Provider{},
@@ -175,8 +173,7 @@ func TestOptions_MergeValues(t *testing.T) {
 		{
 			name: "Values-Directories",
 			opts: Options{
-				ValueFiles: []string{},
-				ValuesDirectories: []string{
+				ValueFiles: []string{
 					"testdata/noconflicts/values.d",
 				},
 				StringValues: []string{},
@@ -202,8 +199,7 @@ func TestOptions_MergeValues(t *testing.T) {
 		{
 			name: "Values-Directories-Recursive-Read",
 			opts: Options{
-				ValueFiles: []string{},
-				ValuesDirectories: []string{
+				ValueFiles: []string{
 					"testdata/multilevelvaluesd/values.d",
 				},
 				StringValues: []string{},
@@ -231,8 +227,7 @@ func TestOptions_MergeValues(t *testing.T) {
 		{
 			name: "Values-Using-Set-String",
 			opts: Options{
-				ValueFiles:        []string{},
-				ValuesDirectories: []string{},
+				ValueFiles: []string{},
 				StringValues: []string{
 					fmt.Sprintf("%s=%s,%s=%s", adminKey, adminVal, replicasKey, replicasVal),
 				},
@@ -252,9 +247,8 @@ func TestOptions_MergeValues(t *testing.T) {
 		{
 			name: "Values-Using-Set",
 			opts: Options{
-				ValueFiles:        []string{},
-				ValuesDirectories: []string{},
-				StringValues:      []string{},
+				ValueFiles:   []string{},
+				StringValues: []string{},
 				Values: []string{
 					fmt.Sprintf("%s=%s,%s=%s", areaKey, financeAreaVal, testFlagKey, testFlagVal),
 				},
@@ -273,10 +267,9 @@ func TestOptions_MergeValues(t *testing.T) {
 		{
 			name: "Values-Using-Set-File",
 			opts: Options{
-				ValueFiles:        []string{},
-				ValuesDirectories: []string{},
-				StringValues:      []string{},
-				Values:            []string{},
+				ValueFiles:   []string{},
+				StringValues: []string{},
+				Values:       []string{},
 				FileValues: []string{
 					fmt.Sprintf("%s=%s,%s=%s", environmentKey, "testdata/noconflicts/environment",
 						nodesKey, "testdata/noconflicts/nodes"),
@@ -295,11 +288,10 @@ func TestOptions_MergeValues(t *testing.T) {
 		{
 			name: "Values-Using-Set-JSON",
 			opts: Options{
-				ValueFiles:        []string{},
-				ValuesDirectories: []string{},
-				StringValues:      []string{},
-				Values:            []string{},
-				FileValues:        []string{},
+				ValueFiles:   []string{},
+				StringValues: []string{},
+				Values:       []string{},
+				FileValues:   []string{},
 				JSONValues: []string{
 					fmt.Sprintf("%s={%q:%q,%q:%q},%s={%q:%q,%q:%q}", readKey, minKey,
 						minVal, maxKey, maxVal, writeKey, minKey, minVal, maxKey, maxVal),
@@ -326,8 +318,6 @@ func TestOptions_MergeValues(t *testing.T) {
 				ValueFiles: []string{
 					"testdata/noconflicts/values.yaml",
 					"testdata/noconflicts/extras.yaml",
-				},
-				ValuesDirectories: []string{
 					"testdata/noconflicts/values.d",
 				},
 				StringValues: []string{
@@ -380,12 +370,10 @@ func TestOptions_MergeValues(t *testing.T) {
 			name: "All-Types-of-Inputs-Overwritten-Values",
 			opts: Options{
 				ValueFiles: []string{
+					"testdata/withconflicts/values.d",
 					"testdata/withconflicts/values.yaml",
 					"testdata/withconflicts/extras.yaml",
 				},
-				ValuesDirectories: []string{
-					"testdata/withconflicts/values.d",
-				},
 				StringValues: []string{
 					fmt.Sprintf("%s=%s,%s=%s", adminKey, adminVal, replicasKey, replicasVal),
 				},
@@ -438,11 +426,10 @@ func TestOptions_MergeValues(t *testing.T) {
 				ValueFiles: []string{
 					"testdata/noconflicts/values-non-existing.yaml",
 				},
-				ValuesDirectories: []string{},
-				StringValues:      []string{},
-				Values:            []string{},
-				FileValues:        []string{},
-				JSONValues:        []string{},
+				StringValues: []string{},
+				Values:       []string{},
+				FileValues:   []string{},
+				JSONValues:   []string{},
 			},
 			args: args{
 				p: []getter.Provider{},
@@ -453,8 +440,7 @@ func TestOptions_MergeValues(t *testing.T) {
 		{
 			name: "Failure-Values-Directory-Missing",
 			opts: Options{
-				ValueFiles: []string{},
-				ValuesDirectories: []string{
+				ValueFiles: []string{
 					"testdata/noconflicts/values-non-existing.d",
 				},
 				StringValues: []string{},
@@ -474,11 +460,10 @@ func TestOptions_MergeValues(t *testing.T) {
 				ValueFiles: []string{
 					"testdata/malformed/values.yaml",
 				},
-				ValuesDirectories: []string{},
-				StringValues:      []string{},
-				Values:            []string{},
-				FileValues:        []string{},
-				JSONValues:        []string{},
+				StringValues: []string{},
+				Values:       []string{},
+				FileValues:   []string{},
+				JSONValues:   []string{},
 			},
 			args: args{
 				p: []getter.Provider{},
@@ -489,8 +474,7 @@ func TestOptions_MergeValues(t *testing.T) {
 		{
 			name: "Failure-Malformed-YAML-in-Values-Directory",
 			opts: Options{
-				ValueFiles: []string{},
-				ValuesDirectories: []string{
+				ValueFiles: []string{
 					"testdata/malformed/values.d",
 				},
 				StringValues: []string{},
@@ -507,11 +491,10 @@ func TestOptions_MergeValues(t *testing.T) {
 		{
 			name: "Failure-Malformed-JSON",
 			opts: Options{
-				ValueFiles:        []string{},
-				ValuesDirectories: []string{},
-				StringValues:      []string{},
-				Values:            []string{},
-				FileValues:        []string{},
+				ValueFiles:   []string{},
+				StringValues: []string{},
+				Values:       []string{},
+				FileValues:   []string{},
 				JSONValues: []string{
 					fmt.Sprintf("%s={%q:%q,%q:%q,%s={%q:%q,%q:%q}", readKey, minKey,
 						minVal, maxKey, maxVal, writeKey, minKey, minVal, maxKey, maxVal),
@@ -526,9 +509,8 @@ func TestOptions_MergeValues(t *testing.T) {
 		{
 			name: "Failure-Malformed-Set-Input",
 			opts: Options{
-				ValueFiles:        []string{},
-				ValuesDirectories: []string{},
-				StringValues:      []string{},
+				ValueFiles:   []string{},
+				StringValues: []string{},
 				Values: []string{
 					fmt.Sprintf("%s:%s,%s=%s", areaKey, financeAreaVal, testFlagKey, testFlagVal),
 				},
@@ -544,8 +526,7 @@ func TestOptions_MergeValues(t *testing.T) {
 		{
 			name: "Failure-Malformed-String-Input",
 			opts: Options{
-				ValueFiles:        []string{},
-				ValuesDirectories: []string{},
+				ValueFiles: []string{},
 				StringValues: []string{
 					fmt.Sprintf("%s:%s,%s=%s", adminKey, adminVal, replicasKey, replicasVal),
 				},
@@ -562,10 +543,9 @@ func TestOptions_MergeValues(t *testing.T) {
 		{
 			name: "Failure-Set-File-Missing",
 			opts: Options{
-				ValueFiles:        []string{},
-				ValuesDirectories: []string{},
-				StringValues:      []string{},
-				Values:            []string{},
+				ValueFiles:   []string{},
+				StringValues: []string{},
+				Values:       []string{},
 				FileValues: []string{
 					fmt.Sprintf("%s=%s,%s=%s", environmentKey, "testdata/noconflicts/environment-non-existing",
 						nodesKey, "testdata/noconflicts/nodes"),
@@ -581,10 +561,9 @@ func TestOptions_MergeValues(t *testing.T) {
 		{
 			name: "Failure-Malformed-File-String-Input",
 			opts: Options{
-				ValueFiles:        []string{},
-				ValuesDirectories: []string{},
-				StringValues:      []string{},
-				Values:            []string{},
+				ValueFiles:   []string{},
+				StringValues: []string{},
+				Values:       []string{},
 				FileValues: []string{
 					fmt.Sprintf("%s:%s,%s=%s", environmentKey, "testdata/noconflicts/environment",
 						nodesKey, "testdata/noconflicts/nodes"),