Support overwritting values from `import-values` dependencies

Alternative to https://github.com/helm/helm/issues/12466

This allows the above requirement to be implemented in terms of existing
Helm functionality. Instead of built in support for a "bundled value
set", a chart can expose a dependency on a chart which provides some
sets of values. These can be conditionally enabled with `import-values`
and `conditions` on the dependency.

The one gap in this approach is the parent values take precedence. This
allows a user to *opt in* to making children high precedence.

Additionally, a unit test and additional validation for the field is
added.

Note: overall, this is a worse experience than #12466 could provide for
this use case. So I would prefer to still do that if feasible. However,
this is a low-cost approach

Signed-off-by: John Howard <howardjohn@google.com>

pkg/chart/dependency.go

@@ -66,6 +66,31 @@ func (d *Dependency) Validate() error {
 	if d.Alias != "" && !aliasNameFormat.MatchString(d.Alias) {
 		return ValidationErrorf("dependency %q has disallowed characters in the alias", d.Name)
 	}
+	for _, riv := range d.ImportValues {
+		switch iv := riv.(type) {
+		case map[string]interface{}:
+			for k, v := range iv {
+				switch k {
+				case "child":
+					if _, ok := v.(string); !ok {
+						return ValidationErrorf("dependency %q has invalid importValues; child must be a string", d.Name)
+					}
+				case "parent":
+					if _, ok := v.(string); !ok {
+						return ValidationErrorf("dependency %q has invalid importValues; parent must be a string", d.Name)
+					}
+				case "overwrite":
+					if _, ok := v.(bool); !ok {
+						return ValidationErrorf("dependency %q has invalid importValues; overwrite must be a boolean", d.Name)
+					}
+				}
+			}
+		case string:
+			// String is always valid
+		default:
+			return ValidationErrorf("dependency %q has invalid importValues of type %T", d.Name, iv)
+		}
+	}
 	return nil
 }
 

pkg/chartutil/dependencies.go

@@ -249,7 +249,11 @@ func processImportValues(c *chart.Chart, merge bool) error {
 	if err != nil {
 		return err
 	}
-	b := make(map[string]interface{})
+	// Imports can be lower or higher precedence from the primary chart
+	// beforeParent are applied first, with lower precedence
+	beforeParent := []map[string]interface{}{}
+	// afterParent are applied last, with higher precedence
+	afterParent := []map[string]interface{}{}
 	// import values from each dependency if specified in import-values
 	for _, r := range c.Metadata.Dependencies {
 		var outiv []interface{}
@@ -258,11 +262,18 @@ func processImportValues(c *chart.Chart, merge bool) error {
 			case map[string]interface{}:
 				child := iv["child"].(string)
 				parent := iv["parent"].(string)
-
-				outiv = append(outiv, map[string]string{
+				var overwrite bool
+				if o, f := iv["overwrite"]; f {
+					overwrite = o.(bool)
+				}
+				m := map[string]interface{}{
 					"child":  child,
 					"parent": parent,
-				})
+				}
+				if overwrite {
+					m["overwrite"] = overwrite
+				}
+				outiv = append(outiv, m)
 
 				// get child table
 				vv, err := cvals.Table(r.Name + "." + child)
@@ -270,15 +281,14 @@ func processImportValues(c *chart.Chart, merge bool) error {
 					log.Printf("Warning: ImportValues missing table from chart %s: %v", r.Name, err)
 					continue
 				}
-				// create value map from child to be merged into parent
-				if merge {
-					b = MergeTables(b, pathToMap(parent, vv.AsMap()))
+				if overwrite {
+					afterParent = append(afterParent, pathToMap(parent, vv.AsMap()))
 				} else {
-					b = CoalesceTables(b, pathToMap(parent, vv.AsMap()))
+					beforeParent = append(beforeParent, pathToMap(parent, vv.AsMap()))
 				}
 			case string:
 				child := "exports." + iv
-				outiv = append(outiv, map[string]string{
+				outiv = append(outiv, map[string]interface{}{
 					"child":  child,
 					"parent": ".",
 				})
@@ -287,16 +297,20 @@ func processImportValues(c *chart.Chart, merge bool) error {
 					log.Printf("Warning: ImportValues missing table: %v", err)
 					continue
 				}
-				if merge {
-					b = MergeTables(b, vm.AsMap())
-				} else {
-					b = CoalesceTables(b, vm.AsMap())
-				}
+				beforeParent = append(beforeParent, vm.AsMap())
 			}
 		}
 		r.ImportValues = outiv
 	}
 
+	b := make(map[string]interface{})
+	for _, mv := range beforeParent {
+		if merge {
+			b = MergeTables(b, mv)
+		} else {
+			b = CoalesceTables(b, mv)
+		}
+	}
 	// Imported values from a child to a parent chart have a lower priority than
 	// the parents values. This enables parent charts to import a large section
 	// from a child and then override select parts. This is why b is merged into
@@ -305,15 +319,23 @@ func processImportValues(c *chart.Chart, merge bool) error {
 		// deep copying the cvals as there are cases where pointers can end
 		// up in the cvals when they are copied onto b in ways that break things.
 		cvals = deepCopyMap(cvals)
-		c.Values = MergeTables(cvals, b)
+		b = MergeTables(cvals, b)
 	} else {
 		// Trimming the nil values from cvals is needed for backwards compatibility.
 		// Previously, the b value had been populated with cvals along with some
 		// overrides. This caused the coalescing functionality to remove the
 		// nil/null values. This trimming is for backwards compat.
 		cvals = trimNilValues(cvals)
-		c.Values = CoalesceTables(cvals, b)
+		b = CoalesceTables(cvals, b)
+	}
+	for _, mv := range afterParent {
+		if merge {
+			b = MergeTables(mv, b)
+		} else {
+			b = CoalesceTables(mv, b)
+		}
 	}
+	c.Values = b
 
 	return nil
 }

pkg/chartutil/dependencies_test.go

@@ -190,6 +190,15 @@ func TestProcessDependencyImportValues(t *testing.T) {
 	e["overridden-chart1.SC1string"] = "pollywog"
 	e["overridden-chart1.SPextra2"] = "42"
 
+	// These values are imported from the child chart to the parent. Parent values are present.
+	// Child is configred to take precedence
+	e["child-overridden-chart1.SC1bool"] = "true"
+	e["child-overridden-chart1.SC1float"] = "3.14"
+	e["child-overridden-chart1.SC1int"] = "100"
+	e["child-overridden-chart1.SC1string"] = "dollywood"
+	e["child-overridden-chart1.SC1extra1"] = "11"
+	e["child-overridden-chart1.SPextra2"] = "42"
+
 	e["overridden-chartA.SCAbool"] = "true"
 	e["overridden-chartA.SCAfloat"] = "41.3"
 	e["overridden-chartA.SCAint"] = "808"

pkg/chartutil/testdata/subpop/Chart.yaml

@@ -15,6 +15,9 @@ dependencies:
         parent: imported-chart1
       - child: SC1data
         parent: overridden-chart1
+      - child: SC1data
+        parent: child-overridden-chart1
+        overwrite: true
       - child: imported-chartA
         parent: imported-chartA
       - child: imported-chartA-B

pkg/chartutil/testdata/subpop/values.yaml

@@ -10,6 +10,12 @@ overridden-chart1:
   SC1string: "pollywog"
   SPextra2: 42
 
+child-overridden-chart1:
+  SC1bool: false
+  SC1float: 3.141592
+  SC1int: 99
+  SC1string: "pollywog"
+  SPextra2: 42
 
 imported-chartA:
   SPextra3: 1.337