fix(requirements): refactor to use common codepath for table coalescing (#7047)

* align both formats behaviors and now they will just differ in how to discover their paths
* add coverage for exports format and fix expected assertions for parent-child format to match the logic child values always wins
* just partially revert dda8497, this way parents values could be overridden when coalescing
* after getting better coverage we were able to refact both formats behaviors by merging their propagation logics into a single code path.

pkg/chartutil/requirements.go

@@ -396,7 +396,7 @@ func processImportValues(c *chart.Chart) error {
 	if err != nil {
 		return err
 	}
-	b := cvals.AsMap()
+	b := make(map[string]interface{}, 0)
 	// import values from each dependency if specified in import-values
 	for _, r := range reqs.Dependencies {
 		// only process raw requirement that is found in chart's dependencies (enabled)
@@ -417,42 +417,34 @@ func processImportValues(c *chart.Chart) error {
 		if len(r.ImportValues) > 0 {
 			var outiv []interface{}
 			for _, riv := range r.ImportValues {
+				nm := make(map[string]string, 0)
 				switch iv := riv.(type) {
 				case map[string]interface{}:
-					nm := map[string]string{
-						"child":  iv["child"].(string),
-						"parent": iv["parent"].(string),
-					}
-					outiv = append(outiv, nm)
-					s := name + "." + nm["child"]
-					// get child table
-					vv, err := cvals.Table(s)
-					if err != nil {
-						log.Printf("Warning: ImportValues missing table: %v", err)
-						continue
-					}
-					// create value map from child to be merged into parent
-					vm := pathToMap(nm["parent"], vv.AsMap())
-					b = coalesceTables(b, vm, c.Metadata.Name)
+					nm["child"] = iv["child"].(string)
+					nm["parent"] = iv["parent"].(string)
 				case string:
-					nm := map[string]string{
-						"child":  "exports." + iv,
-						"parent": ".",
-					}
-					outiv = append(outiv, nm)
-					s := name + "." + nm["child"]
-					vm, err := cvals.Table(s)
-					if err != nil {
-						log.Printf("Warning: ImportValues missing table: %v", err)
-						continue
-					}
-					b = coalesceTables(b, vm.AsMap(), c.Metadata.Name)
+					nm["child"] = "exports." + iv
+					nm["parent"] = "."
 				}
+
+				outiv = append(outiv, nm)
+				s := name + "." + nm["child"]
+				// get child table
+				vv, err := cvals.Table(s)
+				if err != nil {
+					log.Printf("Warning: ImportValues missing table: %v", err)
+					continue
+				}
+				// create value map from child to be merged into parent
+				vm := pathToMap(nm["parent"], vv.AsMap())
+				b = coalesceTables(b, vm, c.Metadata.Name)
+
 			}
 			// set our formatted import values
 			r.ImportValues = outiv
 		}
 	}
+	b = coalesceTables(b, cvals, c.Metadata.Name)
 	y, err := yaml.Marshal(b)
 	if err != nil {
 		return err

pkg/chartutil/requirements_test.go

@@ -259,10 +259,10 @@ func TestProcessRequirementsImportValues(t *testing.T) {
 	e["imported-chartA-B.SPextra5"] = "k8s"
 	e["imported-chartA-B.SC1extra5"] = "tiller"
 
-	e["overridden-chart1.SC1bool"] = "false"
-	e["overridden-chart1.SC1float"] = "3.141592"
-	e["overridden-chart1.SC1int"] = "99"
-	e["overridden-chart1.SC1string"] = "pollywog"
+	e["overridden-chart1.SC1bool"] = "true"
+	e["overridden-chart1.SC1float"] = "3.14"
+	e["overridden-chart1.SC1int"] = "100"
+	e["overridden-chart1.SC1string"] = "dollywood"
 	e["overridden-chart1.SPextra2"] = "42"
 
 	e["overridden-chartA.SCAbool"] = "true"
@@ -272,13 +272,13 @@ func TestProcessRequirementsImportValues(t *testing.T) {
 	e["overridden-chartA.SPextra4"] = "true"
 
 	e["overridden-chartA-B.SCAbool"] = "true"
-	e["overridden-chartA-B.SCAfloat"] = "41.3"
-	e["overridden-chartA-B.SCAint"] = "808"
-	e["overridden-chartA-B.SCAstring"] = "jaberwocky"
-	e["overridden-chartA-B.SCBbool"] = "false"
-	e["overridden-chartA-B.SCBfloat"] = "1.99"
-	e["overridden-chartA-B.SCBint"] = "77"
-	e["overridden-chartA-B.SCBstring"] = "jango"
+	e["overridden-chartA-B.SCAfloat"] = "3.33"
+	e["overridden-chartA-B.SCAint"] = "555"
+	e["overridden-chartA-B.SCAstring"] = "wormwood"
+	e["overridden-chartA-B.SCBbool"] = "true"
+	e["overridden-chartA-B.SCBfloat"] = "0.25"
+	e["overridden-chartA-B.SCBint"] = "98"
+	e["overridden-chartA-B.SCBstring"] = "murkwood"
 	e["overridden-chartA-B.SPextra6"] = "111"
 	e["overridden-chartA-B.SCAextra1"] = "23"
 	e["overridden-chartA-B.SCBextra1"] = "13"
@@ -290,6 +290,9 @@ func TestProcessRequirementsImportValues(t *testing.T) {
 	e["SCBexported2A"] = "blaster"
 	e["global.SC1exported2.all.SC1exported3"] = "SC1expstr"
 
+	e["SCCdata.SCCstring"] = "mugwort"
+	e["SCCdata.SCCint"] = "42"
+
 	verifyRequirementsImportValues(t, c, v, e)
 }
 func verifyRequirementsImportValues(t *testing.T, c *chart.Chart, v *chart.Config, e map[string]string) {
@@ -314,18 +317,18 @@ func verifyRequirementsImportValues(t *testing.T, c *chart.Chart, v *chart.Confi
 		case float64:
 			s := strconv.FormatFloat(pv.(float64), 'f', -1, 64)
 			if s != vv {
-				t.Errorf("Failed to match imported float value %v with expected %v", s, vv)
+				t.Errorf("Failed to match imported float field %v with value %v with expected %v", kk, s, vv)
 				return
 			}
 		case bool:
 			b := strconv.FormatBool(pv.(bool))
 			if b != vv {
-				t.Errorf("Failed to match imported bool value %v with expected %v", b, vv)
+				t.Errorf("Failed to match imported bool field %v with value %v with expected %v", kk, b, vv)
 				return
 			}
 		default:
 			if pv.(string) != vv {
-				t.Errorf("Failed to match imported string value %v with expected %v", pv, vv)
+				t.Errorf("Failed to match imported string field %v with value %v with expected %v", kk, pv, vv)
 				return
 			}
 		}

pkg/chartutil/testdata/subpop/charts/subchart3/.helmignore

@@ -0,0 +1,22 @@
+# Patterns to ignore when building packages.
+# This supports shell glob matching, relative path matching, and
+# negation (prefixed with !). Only one pattern per line.
+.DS_Store
+# Common VCS dirs
+.git/
+.gitignore
+.bzr/
+.bzrignore
+.hg/
+.hgignore
+.svn/
+# Common backup files
+*.swp
+*.bak
+*.tmp
+*~
+# Various IDEs
+.project
+.idea/
+*.tmproj
+.vscode/

pkg/chartutil/testdata/subpop/charts/subchart3/Chart.yaml

@@ -0,0 +1,5 @@
+apiVersion: v1
+appVersion: "1.0"
+description: A Helm chart for Kubernetes
+name: subchart3
+version: 0.1.0

pkg/chartutil/testdata/subpop/charts/subchart3/values.yaml

@@ -0,0 +1,4 @@
+exports:
+  data:
+    SCCdata:
+      SCCstring: "mugwort"

pkg/chartutil/testdata/subpop/requirements.yaml

@@ -35,3 +35,10 @@ dependencies:
         repository: http://localhost:10191
         version: 0.1.0
         condition: subchart2alias.enabled
+
+      - name: subchart3
+        repository: http://localhost:10191
+        version: 0.1.0
+        condition: subchart3.enabled
+        import-values:
+          - data

pkg/chartutil/testdata/subpop/values.yaml

@@ -35,9 +35,16 @@ overridden-chartA-B:
   SCBstring: "jango"
   SPextra6: 111
 
+SCCdata:
+  SCCstring: "jaberwocky"
+  SCCint: 42
+
 tags:
   front-end: true
   back-end: false
 
+subchart3:
+  enabled: false
+
 subchart2alias:
   enabled: false