Merge remote-tracking branch 'jamoflaw/fix-null-removal-in-nested-subcharts-alt' into main

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) {

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{}{

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())