From b3841902e2f7f044afac60025a873582513a869d Mon Sep 17 00:00:00 2001 From: James Payne Date: Sun, 16 Oct 2022 23:09:28 +0100 Subject: [PATCH] Stop CoalesceValues from trimming nil values from the values. Creating new public function TrimNilValues which does the final nil value removal. This is called in ToRenderValues before final schema validation. Signed-off-by: James Payne --- pkg/chartutil/coalesce.go | 32 ++++++++++++----- pkg/chartutil/coalesce_test.go | 66 +++++++++++++++++++++++++++------- pkg/chartutil/values.go | 3 ++ 3 files changed, 80 insertions(+), 21 deletions(-) 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())