diff --git a/cmd/helm/status.go b/cmd/helm/status.go index aa22aa02a..46939230c 100644 --- a/cmd/helm/status.go +++ b/cmd/helm/status.go @@ -191,7 +191,7 @@ func (s statusPrinter) WriteTable(out io.Writer) error { // Print an extra newline fmt.Fprintln(out) - cfg, err := chartutil.CoalesceValues(s.release.Chart, s.release.Config) + cfg, err := chartutil.MergeValues(s.release.Chart, s.release.Config) if err != nil { return err } diff --git a/pkg/action/get_values.go b/pkg/action/get_values.go index 9c32db213..ecfd4055e 100644 --- a/pkg/action/get_values.go +++ b/pkg/action/get_values.go @@ -50,7 +50,7 @@ func (g *GetValues) Run(name string) (map[string]interface{}, error) { // If the user wants all values, compute the values and return. if g.AllValues { - cfg, err := chartutil.CoalesceValues(rel.Chart, rel.Config) + cfg, err := chartutil.MergeValues(rel.Chart, rel.Config) if err != nil { return nil, err } diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 8fb895df0..8d64133f5 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -510,7 +510,7 @@ func (u *Upgrade) reuseValues(chart *chart.Chart, current *release.Release, newV u.cfg.Log("reusing the old release's values") // We have to regenerate the old coalesced values: - oldVals, err := chartutil.CoalesceValues(current.Chart, current.Config) + oldVals, err := chartutil.MergeValues(current.Chart, current.Config) if err != nil { return nil, errors.Wrap(err, "failed to rebuild old values") } diff --git a/pkg/chartutil/coalesce.go b/pkg/chartutil/coalesce.go index ca5aef37e..25ced5985 100644 --- a/pkg/chartutil/coalesce.go +++ b/pkg/chartutil/coalesce.go @@ -37,17 +37,48 @@ func concatPrefix(a, b string) string { // // Values are coalesced together using the following rules: // -// - Values in a higher level chart always override values in a lower-level -// dependency chart -// - Scalar values and arrays are replaced, maps are merged -// - A chart has access to all of the variables for it, as well as all of -// the values destined for its dependencies. +// - Values in a higher level chart always override values in a lower-level +// dependency chart +// +// - Scalar values and arrays are replaced, maps are merged +// +// - A chart has access to all of the variables for it, as well as all of +// the values destined for its dependencies. +// +// Note that CoalesceValues automatically removes nil values func CoalesceValues(chrt *chart.Chart, vals map[string]interface{}) (Values, error) { v, err := copystructure.Copy(vals) if err != nil { return vals, err } + valsCopy := v.(map[string]interface{}) + // if we have an empty map, make sure it is initialized + if valsCopy == nil { + valsCopy = make(map[string]interface{}) + } + + valsResult, err := coalesce(log.Printf, chrt, valsCopy, "") + return TrimNilValues(valsResult), err +} + +// MergeValues coalesces all of the values in a chart (and its subcharts). +// +// Values are coalesced together using the following rules: +// +// - Values in a higher level chart always override values in a lower-level +// dependency chart +// - Scalar values and arrays are replaced, maps are merged +// - A chart has access to all of the variables for it, as well as all of +// the values destined for its dependencies. +// Note that MergeValues does NOT automatically remove nil values, it is up +// to the caller to ensure that TrimNilValues is called at the appropriate time +func MergeValues(chrt *chart.Chart, vals map[string]interface{}) (Values, error) { + v, err := copystructure.Copy(vals) + if err != nil { + return vals, err + } + valsCopy := v.(map[string]interface{}) // if we have an empty map, make sure it is initialized if valsCopy == nil { @@ -57,7 +88,8 @@ func CoalesceValues(chrt *chart.Chart, vals map[string]interface{}) (Values, err } // TrimNilValues removes nil/null values from maps. -// To be used after CoalesceValues to remove keys as needed +// +// To be used after CoalesceValues to remove keys as needed func TrimNilValues(vals map[string]interface{}) map[string]interface{} { valsCopy, err := copystructure.Copy(vals) if err != nil { diff --git a/pkg/chartutil/coalesce_test.go b/pkg/chartutil/coalesce_test.go index 360e36bf5..cc3d1a456 100644 --- a/pkg/chartutil/coalesce_test.go +++ b/pkg/chartutil/coalesce_test.go @@ -65,12 +65,12 @@ func withDeps(c *chart.Chart, deps ...*chart.Chart) *chart.Chart { func TestTrimNilValues(t *testing.T) { values := map[string]interface{}{ - "top": "exists", + "top": "exists", "bottom": "exists", - "left": nil, - "right": "exists", + "left": nil, + "right": "exists", "nested": map[string]interface{}{ - "left": true, + "left": true, "right": nil, }, } @@ -102,10 +102,8 @@ func TestTrimNilValues(t *testing.T) { } } -func TestCoalesceValues(t *testing.T) { - is := assert.New(t) - - c := withDeps(&chart.Chart{ +func getValuesChart() *chart.Chart { + return withDeps(&chart.Chart{ Metadata: &chart.Metadata{Name: "moby"}, Values: map[string]interface{}{ "back": "exists", @@ -160,6 +158,12 @@ func TestCoalesceValues(t *testing.T) { }, }, ) +} + +func TestCoalesceValues(t *testing.T) { + is := assert.New(t) + + c := getValuesChart() vals, err := ReadValues(testCoalesceValuesYaml) if err != nil { @@ -181,6 +185,92 @@ func TestCoalesceValues(t *testing.T) { j, _ := json.MarshalIndent(v, "", " ") t.Logf("Coalesced Values: %s", string(j)) + tests := []struct { + tpl string + expect string + }{ + {"{{.top}}", "yup"}, + {"{{.back}}", ""}, + {"{{.name}}", "moby"}, + {"{{.global.name}}", "Ishmael"}, + {"{{.global.subject}}", "Queequeg"}, + {"{{.global.harpooner}}", ""}, + {"{{.pequod.name}}", "pequod"}, + {"{{.pequod.ahab.name}}", "ahab"}, + {"{{.pequod.ahab.scope}}", "whale"}, + {"{{.pequod.ahab.nested.foo}}", "true"}, + {"{{.pequod.ahab.global.name}}", "Ishmael"}, + {"{{.pequod.ahab.global.nested.foo}}", "bar"}, + {"{{.pequod.ahab.global.subject}}", "Queequeg"}, + {"{{.pequod.ahab.global.harpooner}}", "Tashtego"}, + {"{{.pequod.global.name}}", "Ishmael"}, + {"{{.pequod.global.nested.foo}}", ""}, + {"{{.pequod.global.subject}}", "Queequeg"}, + {"{{.spouter.global.name}}", "Ishmael"}, + {"{{.spouter.global.harpooner}}", ""}, + + {"{{.global.nested.boat}}", "true"}, + {"{{.pequod.global.nested.boat}}", "true"}, + {"{{.spouter.global.nested.boat}}", "true"}, + {"{{.pequod.global.nested.sail}}", "true"}, + {"{{.spouter.global.nested.sail}}", ""}, + } + + for _, tt := range tests { + if o, err := ttpl(tt.tpl, v); err != nil || o != tt.expect { + t.Errorf("Expected %q to expand to %q, got %q", tt.tpl, tt.expect, o) + } + } + + nullKeys := []string{"bottom", "right", "left", "front"} + for _, nullKey := range nullKeys { + if _, ok := v[nullKey]; ok { + t.Errorf("Expected key %q to be removed, still present", nullKey) + } + } + + if _, ok := v["nested"].(map[string]interface{})["boat"]; ok { + t.Error("Expected nested boat key to be removed, still present") + } + + subchart := v["pequod"].(map[string]interface{})["ahab"].(map[string]interface{}) + if _, ok := subchart["boat"]; ok { + t.Error("Expected subchart boat key to be removed, still present") + } + + if _, ok := subchart["nested"].(map[string]interface{})["bar"]; ok { + t.Error("Expected subchart nested bar key to be removed, still present") + } + + // CoalesceValues should not mutate the passed arguments + is.Equal(valsCopy, vals) +} + +func TestMergeValues(t *testing.T) { + is := assert.New(t) + + c := getValuesChart() + + vals, err := ReadValues(testCoalesceValuesYaml) + if err != nil { + t.Fatal(err) + } + + // taking a copy of the values before passing it + // to MergeValues as argument, so that we can + // use it for asserting later + valsCopy := make(Values, len(vals)) + for key, value := range vals { + valsCopy[key] = value + } + + v, err := MergeValues(c, vals) + if err != nil { + t.Fatal(err) + } + j, _ := json.MarshalIndent(v, "", " ") + t.Logf("Coalesced Values: %s", string(j)) + tests := []struct { tpl string expect string @@ -247,11 +337,12 @@ func TestCoalesceValues(t *testing.T) { t.Error("Expected subchart boat key to be null") } + // MergeValues should not mutate / remove nil values if val, ok := subchart["nested"].(map[string]interface{})["bar"]; !ok || val != nil { t.Error("Expected subchart nested bar key to be null") } - // CoalesceValues should not mutate the passed arguments + // MergeValues should not mutate the passed arguments is.Equal(valsCopy, vals) } diff --git a/pkg/chartutil/dependencies.go b/pkg/chartutil/dependencies.go index e01b95bf7..7abd13f23 100644 --- a/pkg/chartutil/dependencies.go +++ b/pkg/chartutil/dependencies.go @@ -150,7 +150,7 @@ Loop: for _, lr := range c.Metadata.Dependencies { lr.Enabled = true } - cvals, err := CoalesceValues(c, v) + cvals, err := MergeValues(c, v) if err != nil { return err } @@ -222,7 +222,7 @@ func processImportValues(c *chart.Chart) error { return nil } // combine chart values and empty config to get Values - cvals, err := CoalesceValues(c, nil) + cvals, err := MergeValues(c, nil) if err != nil { return err } diff --git a/pkg/chartutil/values.go b/pkg/chartutil/values.go index 591199b7d..62a28484d 100644 --- a/pkg/chartutil/values.go +++ b/pkg/chartutil/values.go @@ -151,7 +151,7 @@ func ToRenderValues(chrt *chart.Chart, chrtVals map[string]interface{}, options }, } - vals, err := CoalesceValues(chrt, chrtVals) + vals, err := MergeValues(chrt, chrtVals) if err != nil { return top, err } diff --git a/pkg/engine/engine_test.go b/pkg/engine/engine_test.go index 434b939dc..ebd39c02e 100644 --- a/pkg/engine/engine_test.go +++ b/pkg/engine/engine_test.go @@ -105,7 +105,7 @@ func TestRender(t *testing.T) { }, } - v, err := chartutil.CoalesceValues(c, vals) + v, err := chartutil.MergeValues(c, vals) if err != nil { t.Fatalf("Failed to coalesce values: %s", err) } @@ -219,7 +219,7 @@ func TestRenderWIthDNS(t *testing.T) { "Values": map[string]interface{}{}, } - v, err := chartutil.CoalesceValues(c, vals) + v, err := chartutil.MergeValues(c, vals) if err != nil { t.Fatalf("Failed to coalesce values: %s", err) } @@ -525,7 +525,7 @@ func TestRenderNestedValues(t *testing.T) { }, } - tmp, err := chartutil.CoalesceValues(outer, injValues) + tmp, err := chartutil.MergeValues(outer, injValues) if err != nil { t.Fatalf("Failed to coalesce values: %s", err) } diff --git a/pkg/lint/rules/template.go b/pkg/lint/rules/template.go index 61425f92e..34c677371 100644 --- a/pkg/lint/rules/template.go +++ b/pkg/lint/rules/template.go @@ -76,7 +76,7 @@ func Templates(linter *support.Linter, values map[string]interface{}, namespace return } - cvals, err := chartutil.CoalesceValues(chart, values) + cvals, err := chartutil.MergeValues(chart, values) if err != nil { return } diff --git a/pkg/lint/rules/values.go b/pkg/lint/rules/values.go index 79a294326..1551fa570 100644 --- a/pkg/lint/rules/values.go +++ b/pkg/lint/rules/values.go @@ -70,7 +70,7 @@ func validateValuesFile(valuesPath string, overrides map[string]interface{}) err // level values against the top-level expectations. Subchart values are not linted. // We could change that. For now, though, we retain that strategy, and thus can // coalesce tables (like reuse-values does) instead of doing the full chart - // CoalesceValues + // MergeValues coalescedValues := chartutil.CoalesceTables(make(map[string]interface{}, len(overrides)), overrides) coalescedValues = chartutil.CoalesceTables(coalescedValues, values)