From 1a1d84ce4c68b834ccf77b26f0aa0aded1598a77 Mon Sep 17 00:00:00 2001
From: Taylor Thomas <taylor.l.thomas@intel.com>
Date: Wed, 7 Dec 2016 14:50:49 -0800
Subject: [PATCH] feat(helm): add support for multiple values files

You can now specify the `-f` flag multiple times to include multiple
values files. The priority will be given to the last (right-most)
file specified.

Closes #1620
---
 cmd/helm/install.go                           | 72 +++++++++++++++++--
 cmd/helm/install_test.go                      | 72 +++++++++++++++++++
 .../testcharts/alpine/extra_values.yaml       |  2 +
 .../testcharts/alpine/more_values.yaml        |  2 +
 .../alpine/templates/alpine-pod.yaml          |  1 +
 cmd/helm/upgrade.go                           | 25 ++++---
 6 files changed, 159 insertions(+), 15 deletions(-)
 create mode 100644 cmd/helm/testdata/testcharts/alpine/extra_values.yaml
 create mode 100644 cmd/helm/testdata/testcharts/alpine/more_values.yaml

diff --git a/cmd/helm/install.go b/cmd/helm/install.go
index d0c919285..ab6e67e7e 100644
--- a/cmd/helm/install.go
+++ b/cmd/helm/install.go
@@ -55,6 +55,12 @@ or
 
 	$ helm install --set name=prod ./redis
 
+You can specify the '--values'/'-f' flag multiple times. The priority will be given to the
+last (right-most) file specified. For example, if both myvalues.yaml and override.yaml 
+contained a key called 'Test', the value set in override.yaml would take precedence:
+
+	$ helm install -f myvalues.yaml -f override.yaml ./redis
+
 To check the generated manifests of a release without installing the chart,
 the '--debug' and '--dry-run' flags can be combined. This will still require a
 round-trip to the Tiller server.
@@ -86,7 +92,7 @@ charts in a repository, use 'helm search'.
 type installCmd struct {
 	name         string
 	namespace    string
-	valuesFile   string
+	valueFiles   valueFiles
 	chartPath    string
 	dryRun       bool
 	disableHooks bool
@@ -100,6 +106,23 @@ type installCmd struct {
 	version      string
 }
 
+type valueFiles []string
+
+func (v *valueFiles) String() string {
+	return fmt.Sprint(*v)
+}
+
+func (v *valueFiles) Type() string {
+	return "valueFiles"
+}
+
+func (v *valueFiles) Set(value string) error {
+	for _, filePath := range strings.Split(value, ",") {
+		*v = append(*v, filePath)
+	}
+	return nil
+}
+
 func newInstallCmd(c helm.Interface, out io.Writer) *cobra.Command {
 	inst := &installCmd{
 		out:    out,
@@ -126,7 +149,7 @@ func newInstallCmd(c helm.Interface, out io.Writer) *cobra.Command {
 	}
 
 	f := cmd.Flags()
-	f.StringVarP(&inst.valuesFile, "values", "f", "", "specify values in a YAML file")
+	f.VarP(&inst.valueFiles, "values", "f", "specify values in a YAML file (can specify multiple)")
 	f.StringVarP(&inst.name, "name", "n", "", "release name. If unspecified, it will autogenerate one for you")
 	f.StringVar(&inst.namespace, "namespace", "", "namespace to install the release into")
 	f.BoolVar(&inst.dryRun, "dry-run", false, "simulate an install")
@@ -197,19 +220,54 @@ func (i *installCmd) run() error {
 	return nil
 }
 
+// Merges source and destination map, preferring values from the source map
+func mergeValues(dest map[string]interface{}, src map[string]interface{}) map[string]interface{} {
+	for k, v := range src {
+		// If the key doesn't exist already, then just set the key to that value
+		if _, exists := dest[k]; !exists {
+			dest[k] = v
+			continue
+		}
+		nextMap, ok := v.(map[string]interface{})
+		// If it isn't another map, overwrite the value
+		if !ok {
+			dest[k] = v
+			continue
+		}
+		// If the key doesn't exist already, then just set the key to that value
+		if _, exists := dest[k]; !exists {
+			dest[k] = nextMap
+			continue
+		}
+		// Edge case: If the key exists in the destination, but isn't a map
+		destMap, isMap := dest[k].(map[string]interface{})
+		// If the source map has a map for this key, prefer it
+		if !isMap {
+			dest[k] = v
+			continue
+		}
+		// If we got to this point, it is a map in both, so merge them
+		dest[k] = mergeValues(destMap, nextMap)
+	}
+	return dest
+}
+
 func (i *installCmd) vals() ([]byte, error) {
 	base := map[string]interface{}{}
 
-	// User specified a values file via -f/--values
-	if i.valuesFile != "" {
-		bytes, err := ioutil.ReadFile(i.valuesFile)
+	// User specified a values files via -f/--values
+	for _, filePath := range i.valueFiles {
+		currentMap := map[string]interface{}{}
+		bytes, err := ioutil.ReadFile(filePath)
 		if err != nil {
 			return []byte{}, err
 		}
 
-		if err := yaml.Unmarshal(bytes, &base); err != nil {
-			return []byte{}, fmt.Errorf("failed to parse %s: %s", i.valuesFile, err)
+		if err := yaml.Unmarshal(bytes, &currentMap); err != nil {
+			return []byte{}, fmt.Errorf("failed to parse %s: %s", filePath, err)
 		}
+		// Merge with the previous map
+		base = mergeValues(base, currentMap)
 	}
 
 	if err := strvals.ParseInto(i.values, base); err != nil {
diff --git a/cmd/helm/install_test.go b/cmd/helm/install_test.go
index e2e5e1e8b..452c73b26 100644
--- a/cmd/helm/install_test.go
+++ b/cmd/helm/install_test.go
@@ -18,6 +18,7 @@ package main
 
 import (
 	"io"
+	"reflect"
 	"regexp"
 	"strings"
 	"testing"
@@ -51,6 +52,22 @@ func TestInstall(t *testing.T) {
 			resp:     releaseMock(&releaseOptions{name: "virgil"}),
 			expected: "virgil",
 		},
+		// Install, values from yaml
+		{
+			name:     "install with values",
+			args:     []string{"testdata/testcharts/alpine"},
+			flags:    strings.Split("-f testdata/testcharts/alpine/extra_values.yaml", " "),
+			resp:     releaseMock(&releaseOptions{name: "virgil"}),
+			expected: "virgil",
+		},
+		// Install, values from multiple yaml
+		{
+			name:     "install with values",
+			args:     []string{"testdata/testcharts/alpine"},
+			flags:    strings.Split("-f testdata/testcharts/alpine/extra_values.yaml -f testdata/testcharts/alpine/more_values.yaml", " "),
+			resp:     releaseMock(&releaseOptions{name: "virgil"}),
+			expected: "virgil",
+		},
 		// Install, no charts
 		{
 			name: "install with no chart specified",
@@ -172,3 +189,58 @@ func TestNameTemplate(t *testing.T) {
 		}
 	}
 }
+
+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 := mergeValues(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 = mergeValues(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 = mergeValues(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 = mergeValues(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)
+	}
+}
diff --git a/cmd/helm/testdata/testcharts/alpine/extra_values.yaml b/cmd/helm/testdata/testcharts/alpine/extra_values.yaml
new file mode 100644
index 000000000..468bbacbc
--- /dev/null
+++ b/cmd/helm/testdata/testcharts/alpine/extra_values.yaml
@@ -0,0 +1,2 @@
+test:
+  Name: extra-values
diff --git a/cmd/helm/testdata/testcharts/alpine/more_values.yaml b/cmd/helm/testdata/testcharts/alpine/more_values.yaml
new file mode 100644
index 000000000..3d21e1fed
--- /dev/null
+++ b/cmd/helm/testdata/testcharts/alpine/more_values.yaml
@@ -0,0 +1,2 @@
+test:
+  Name: more-values
diff --git a/cmd/helm/testdata/testcharts/alpine/templates/alpine-pod.yaml b/cmd/helm/testdata/testcharts/alpine/templates/alpine-pod.yaml
index c15ab8efc..424920782 100644
--- a/cmd/helm/testdata/testcharts/alpine/templates/alpine-pod.yaml
+++ b/cmd/helm/testdata/testcharts/alpine/templates/alpine-pod.yaml
@@ -12,6 +12,7 @@ metadata:
     release: {{.Release.Name | quote }}
     # This makes it easy to audit chart usage.
     chart: "{{.Chart.Name}}-{{.Chart.Version}}"
+    values: {{.Values.test.Name}}
   annotations:
     "helm.sh/created": {{.Release.Time.Seconds | quote }}
 spec:
diff --git a/cmd/helm/upgrade.go b/cmd/helm/upgrade.go
index 4d42c1d36..7fd8a273c 100644
--- a/cmd/helm/upgrade.go
+++ b/cmd/helm/upgrade.go
@@ -40,6 +40,12 @@ version will be specified unless the '--version' flag is set.
 
 To override values in a chart, use either the '--values' flag and pass in a file
 or use the '--set' flag and pass configuration from the command line.
+
+You can specify the '--values'/'-f' flag multiple times. The priority will be given to the
+last (right-most) file specified. For example, if both myvalues.yaml and override.yaml 
+contained a key called 'Test', the value set in override.yaml would take precedence:
+
+	$ helm install -f myvalues.yaml -f override.yaml ./redis
 `
 
 type upgradeCmd struct {
@@ -49,7 +55,7 @@ type upgradeCmd struct {
 	client       helm.Interface
 	dryRun       bool
 	disableHooks bool
-	valuesFile   string
+	valueFiles   valueFiles
 	values       string
 	verify       bool
 	keyring      string
@@ -84,7 +90,7 @@ func newUpgradeCmd(client helm.Interface, out io.Writer) *cobra.Command {
 	}
 
 	f := cmd.Flags()
-	f.StringVarP(&upgrade.valuesFile, "values", "f", "", "path to a values YAML file")
+	f.VarP(&upgrade.valueFiles, "values", "f", "specify values in a YAML file (can specify multiple)")
 	f.BoolVar(&upgrade.dryRun, "dry-run", false, "simulate an upgrade")
 	f.StringVar(&upgrade.values, "set", "", "set values on the command line. Separate values with commas: key1=val1,key2=val2")
 	f.BoolVar(&upgrade.disableHooks, "disable-hooks", false, "disable pre/post upgrade hooks. DEPRECATED. Use no-hooks")
@@ -121,7 +127,7 @@ func (u *upgradeCmd) run() error {
 				client:       u.client,
 				out:          u.out,
 				name:         u.release,
-				valuesFile:   u.valuesFile,
+				valueFiles:   u.valueFiles,
 				dryRun:       u.dryRun,
 				verify:       u.verify,
 				disableHooks: u.disableHooks,
@@ -164,16 +170,19 @@ func (u *upgradeCmd) run() error {
 func (u *upgradeCmd) vals() ([]byte, error) {
 	base := map[string]interface{}{}
 
-	// User specified a values file via -f/--values
-	if u.valuesFile != "" {
-		bytes, err := ioutil.ReadFile(u.valuesFile)
+	// User specified a values files via -f/--values
+	for _, filePath := range u.valueFiles {
+		currentMap := map[string]interface{}{}
+		bytes, err := ioutil.ReadFile(filePath)
 		if err != nil {
 			return []byte{}, err
 		}
 
-		if err := yaml.Unmarshal(bytes, &base); err != nil {
-			return []byte{}, fmt.Errorf("failed to parse %s: %s", u.valuesFile, err)
+		if err := yaml.Unmarshal(bytes, &currentMap); err != nil {
+			return []byte{}, fmt.Errorf("failed to parse %s: %s", filePath, err)
 		}
+		// Merge with the previous map
+		base = mergeValues(base, currentMap)
 	}
 
 	if err := strvals.ParseInto(u.values, base); err != nil {
-- 
GitLab