diff --git a/pkg/chartutil/coalesce.go b/pkg/chartutil/coalesce.go index f634d6425..ca5aef37e 100644 --- a/pkg/chartutil/coalesce.go +++ b/pkg/chartutil/coalesce.go @@ -56,6 +56,27 @@ func CoalesceValues(chrt *chart.Chart, vals map[string]interface{}) (Values, err return coalesce(log.Printf, chrt, valsCopy, "") } +// TrimNilValues removes nil/null values from maps. +// 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 { + return vals + } + valsCopyMap := valsCopy.(map[string]interface{}) + for key, val := range valsCopyMap { + if val == nil { + // Iterate over the values and remove nil keys + delete(valsCopyMap, key) + } else if istable(val) { + // Recursively call into ourselves to remove keys from inner tables + valsCopyMap[key] = TrimNilValues(val.(map[string]interface{})) + } + } + + return valsCopyMap +} + type printFn func(format string, v ...interface{}) // coalesce coalesces the dest values and the chart values, giving priority to the dest values. @@ -160,12 +181,7 @@ func coalesceValues(printf printFn, c *chart.Chart, v map[string]interface{}, pr subPrefix := concatPrefix(prefix, c.Metadata.Name) for key, val := range c.Values { if value, ok := v[key]; ok { - if value == nil { - // When the YAML value is null, we remove the value's key. - // This allows Helm's various sources of values (value files or --set) to - // remove incompatible keys from any previous chart, file, or set values. - delete(v, key) - } else if dest, ok := value.(map[string]interface{}); ok { + if dest, ok := value.(map[string]interface{}); ok { // if v[key] is a table, merge nv's val table into v[key]. src, ok := val.(map[string]interface{}) if !ok { @@ -209,9 +225,7 @@ func coalesceTablesFullKey(printf printFn, dst, src map[string]interface{}, pref // values. for key, val := range src { fullkey := concatPrefix(prefix, key) - if dv, ok := dst[key]; ok && dv == nil { - delete(dst, key) - } else if !ok { + if dv, ok := dst[key]; !ok { dst[key] = val } else if istable(val) { if istable(dv) { diff --git a/pkg/chartutil/coalesce_test.go b/pkg/chartutil/coalesce_test.go index 3fe93f5ff..360e36bf5 100644 --- a/pkg/chartutil/coalesce_test.go +++ b/pkg/chartutil/coalesce_test.go @@ -63,6 +63,45 @@ func withDeps(c *chart.Chart, deps ...*chart.Chart) *chart.Chart { return c } +func TestTrimNilValues(t *testing.T) { + values := map[string]interface{}{ + "top": "exists", + "bottom": "exists", + "left": nil, + "right": "exists", + "nested": map[string]interface{}{ + "left": true, + "right": nil, + }, + } + + values = TrimNilValues(values) + + j, _ := json.MarshalIndent(values, "", " ") + t.Logf("Trimmed Values: %s", string(j)) + + if val, ok := values["top"]; !ok || val != "exists" { + t.Errorf("Key top was expected to be 'exists' but it is %s", val) + } + if val, ok := values["bottom"]; !ok || val != "exists" { + t.Errorf("Key bottom was expected to be 'exists' but it is %s", val) + } + if val, ok := values["left"]; ok || val != nil { + t.Errorf("Key left was expected to be missing but it is %s", val) + } + if val, ok := values["right"]; !ok || val != "exists" { + t.Errorf("Key right was expected to be 'exists' but it is %s", val) + } + + nested := values["nested"].(map[string]interface{}) + if val, ok := nested["left"]; !ok || val != true { + t.Errorf("Key nested.left was expected to be true but it is %s", val) + } + if val, ok := nested["right"]; ok || val != nil { + t.Errorf("Key nested.right was expected to be missing but it is %s", val) + } +} + func TestCoalesceValues(t *testing.T) { is := assert.New(t) @@ -82,6 +121,9 @@ func TestCoalesceValues(t *testing.T) { "global": map[string]interface{}{ "nested2": map[string]interface{}{"l0": "moby"}, }, + "pequod": map[string]interface{}{ + "nested": map[string]interface{}{"foo": false, "bar": true}, + }, }, }, withDeps(&chart.Chart{ @@ -191,22 +233,22 @@ func TestCoalesceValues(t *testing.T) { 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 val, ok := v[nullKey]; !ok || val != nil { + t.Errorf("Expected key %q to be null", nullKey) } } - if _, ok := v["nested"].(map[string]interface{})["boat"]; ok { - t.Error("Expected nested boat key to be removed, still present") + if val, ok := v["nested"].(map[string]interface{})["boat"]; !ok || val != nil { + t.Error("Expected nested boat key to be null") } 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 val, ok := subchart["boat"]; !ok || val != nil { + t.Error("Expected subchart boat key to be null") } - if _, ok := subchart["nested"].(map[string]interface{})["bar"]; ok { - t.Error("Expected subchart nested bar key to be removed, still present") + 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 @@ -269,8 +311,8 @@ func TestCoalesceTables(t *testing.T) { t.Errorf("Unexpected state: %v", addr["state"]) } - if _, ok = addr["country"]; ok { - t.Error("The country is not left out.") + if val, ok := addr["country"]; !ok || val != nil { + t.Error("The country should be null") } if det, ok := dst["details"].(map[string]interface{}); !ok { @@ -283,8 +325,8 @@ func TestCoalesceTables(t *testing.T) { t.Errorf("Expected boat string, got %v", dst["boat"]) } - if _, ok = dst["hole"]; ok { - t.Error("The hole still exists.") + if val, ok := dst["hole"]; !ok || val != nil { + t.Error("The hole should be null") } dst2 := map[string]interface{}{ diff --git a/pkg/chartutil/values.go b/pkg/chartutil/values.go index 97bf44217..591199b7d 100644 --- a/pkg/chartutil/values.go +++ b/pkg/chartutil/values.go @@ -156,6 +156,9 @@ func ToRenderValues(chrt *chart.Chart, chrtVals map[string]interface{}, options return top, err } + // Remove nil values from the coalesced map + vals = TrimNilValues(vals) + if err := ValidateAgainstSchema(chrt, vals); err != nil { errFmt := "values don't meet the specifications of the schema(s) in the following chart(s):\n%s" return top, fmt.Errorf(errFmt, err.Error())