From 1a1d84ce4c68b834ccf77b26f0aa0aded1598a77 Mon Sep 17 00:00:00 2001 From: Taylor Thomas 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, ¤tMap); 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, ¤tMap); 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 {