Fixing precedence issue with the import of values.

The ordering should be:
1. User specified values (e.g CLI)
2. Parent chart values
3. Imported values
4. Sub-chart values

This enables parnet charts to import large set of values from a
child and then override select values.

This change is needed for backwards compatibility.

Fixes #12460

Signed-off-by: Matt Farina <matt.farina@suse.com>
(cherry picked from commit 25371e2f0d)

cmd/helm/template_test.go

@@ -142,7 +142,7 @@ func TestTemplateCmd(t *testing.T) {
 			golden: "output/issue-9027.txt",
 		},
 		{
-			// Ensure that imported values take precedence over parent chart values
+			// Ensure that parent chart values take precedence over imported values
 			name:   "template with imported subchart values ensuring import",
 			cmd:    fmt.Sprintf("template '%s' --set configmap.enabled=true --set subchartb.enabled=true", chartPath),
 			golden: "output/template-subchart-cm.txt",

cmd/helm/testdata/output/template-subchart-cm.txt

@@ -11,7 +11,7 @@ kind: ConfigMap
 metadata:
   name: subchart-cm
 data:
-  value: bar
+  value: foo
 ---
 # Source: subchart/templates/subdir/role.yaml
 apiVersion: rbac.authorization.k8s.io/v1

pkg/chartutil/dependencies.go

@@ -297,20 +297,22 @@ func processImportValues(c *chart.Chart, merge bool) error {
 		r.ImportValues = outiv
 	}
 
-	// Imported values from a child to a parent chart have a higher priority than
-	// values specified in the parent chart.
+	// 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
+	// cvals in the code below and not the other way around.
 	if merge {
 		// 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(b, cvals)
+		c.Values = 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(b, cvals)
+		c.Values = CoalesceTables(cvals, b)
 	}
 
 	return nil

pkg/chartutil/dependencies_test.go

@@ -181,13 +181,13 @@ func TestProcessDependencyImportValues(t *testing.T) {
 	e["imported-chartA-B.SPextra5"] = "k8s"
 	e["imported-chartA-B.SC1extra5"] = "tiller"
 
-	// These values are imported from the child chart to the parent. Imported
-	// values take precedence over those in the parent so these should be the
-	// values from the child chart.
-	e["overridden-chart1.SC1bool"] = "true"
-	e["overridden-chart1.SC1float"] = "3.14"
-	e["overridden-chart1.SC1int"] = "100"
-	e["overridden-chart1.SC1string"] = "dollywood"
+	// These values are imported from the child chart to the parent. Parent
+	// values take precedence over imported values. This enables importing a
+	// large section from a child chart and overriding a selection from it.
+	e["overridden-chart1.SC1bool"] = "false"
+	e["overridden-chart1.SC1float"] = "3.141592"
+	e["overridden-chart1.SC1int"] = "99"
+	e["overridden-chart1.SC1string"] = "pollywog"
 	e["overridden-chart1.SPextra2"] = "42"
 
 	e["overridden-chartA.SCAbool"] = "true"
@@ -196,17 +196,17 @@ func TestProcessDependencyImportValues(t *testing.T) {
 	e["overridden-chartA.SCAstring"] = "jabberwocky"
 	e["overridden-chartA.SPextra4"] = "true"
 
-	// These values are imported from the child chart to the parent. Imported
-	// values take precedence over those in the parent so these should be the
-	// values from the child chart.
+	// These values are imported from the child chart to the parent. Parent
+	// values take precedence over imported values. This enables importing a
+	// large section from a child chart and overriding a selection from it.
 	e["overridden-chartA-B.SCAbool"] = "true"
-	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.SCAfloat"] = "41.3"
+	e["overridden-chartA-B.SCAint"] = "808"
+	e["overridden-chartA-B.SCAstring"] = "jabberwocky"
+	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.SPextra6"] = "111"
 	e["overridden-chartA-B.SCAextra1"] = "23"
 	e["overridden-chartA-B.SCBextra1"] = "13"
@@ -278,20 +278,20 @@ func TestProcessDependencyImportValuesMultiLevelPrecedence(t *testing.T) {
 
 	// The order of precedence should be:
 	// 1. User specified values (e.g CLI)
-	// 2. Imported values
-	// 3. Parent chart values
+	// 2. Parent chart values
+	// 3. Imported values
 	// 4. Sub-chart values
 	// The 4 app charts here deal with things differently:
 	// - app1 has a port value set in the umbrella chart. It does not import any
 	//   values so the value from the umbrella chart should be used.
 	// - app2 has a value in the app chart and imports from the library. The
-	//   library chart value should take precedence.
+	//   app chart value should take precedence.
 	// - app3 has no value in the app chart and imports the value from the library
 	//   chart. The library chart value should be used.
 	// - app4 has a value in the app chart and does not import the value from the
 	//   library chart. The app charts value should be used.
 	e["app1.service.port"] = "3456"
-	e["app2.service.port"] = "9090"
+	e["app2.service.port"] = "8080"
 	e["app3.service.port"] = "9090"
 	e["app4.service.port"] = "1234"
 	if err := processDependencyImportValues(c, true); err != nil {

pkg/chartutil/testdata/three-level-dependent-chart/README.md

@@ -5,7 +5,7 @@ This chart is for testing the processing of multi-level dependencies.
 Consists of the following charts:
 
 - Library Chart
-- App Chart (Uses Library Chart as dependecy, 2x: app1/app2)
+- App Chart (Uses Library Chart as dependency, 2x: app1/app2)
 - Umbrella Chart (Has all the app charts as dependencies)
 
 The precedence is as follows: `library < app < umbrella`