Repository: helm/helm (go)

Issue:
Helm 3.13 is not backward compatible with 3.12
<!-- If you need help or think you have found a bug, please help us with your issue by entering the following information (otherwise you can delete this text): -->

Output of `helm version`: 
```
version.BuildInfo{Version:"v3.13.0", GitCommit:"825e86f6a7a38cef1112bfa606e4127a706749b1", GitTreeState:"clean", GoVersion:"go1.20.8"}
```

Output of `kubectl version`: 
```
Client Version: v1.28.2
Kustomize Version: v5.0.4-0.20230601165947-6ce0bf390ce3
Server Version: v1.27.3-gke.100
```

Cloud Provider/Platform (AKS, GKE, Minikube etc.): `Not applicable`

---

I have found in a few projects that helm 3.13 is not rendering the same output as 3.12. I've set up a simple minimal reproducible example at https://github.com/zevisert/helm-3-13-rendering-mre

Either way, here's some console output showing what I'm running into: 


```console
$ helm version
version.BuildInfo{Version:"v3.12.3", GitCommit:"3a31588ad33fe3b89af5a2a54ee1d25bfe6eaa5e", GitTreeState:"clean", GoVersion:"go1.20.7"}

$ helm create example
Creating example

$ rm -r example/charts/ example/templates/*

$ echo '{{ include "helmet.app" . }}' > example/templates/app.yaml

$ yq e '. += ({"dependencies": [{"name": "helmet", "version": "~0.9.1", "repository": "https://companyinfo.github.io/helm-charts", "import-values": ["defaults"]}]})' < example/Chart.yaml | sponge example/Chart.yaml

$ helm dependency build example
Hang tight while we grab the latest from your chart repositories...
...Successfully got an update from the "helmet" chart repository
Update Complete. ⎈Happy Helming!⎈
Saving 1 charts
Downloading helmet from repo https://companyinfo.github.io/helm-charts
Deleting outdated charts

$ helm template example > templated-3.12.yaml

$ curl -fsSL -o get_helm.sh https://raw.githubusercontent.com/helm/helm/main/scripts/get-helm-3
$ chmod 700 get_helm.sh
$ ./get_helm.sh
Helm v3.13.0 is available. Changing from version v3.12.3.
Downloading https://get.helm.sh/helm-v3.13.0-linux-amd64.tar.gz
Verifying checksum... Done.
Preparing to install helm into /opt/tooling/helm/bin
helm installed into /opt/tooling/helm/bin/helm

$ helm version
version.BuildInfo{Version:"v3.13.0", GitCommit:"825e86f6a7a38cef1112bfa606e4127a706749b1", GitTreeState:"clean", GoVersion:"go1.20.8"}

$ helm dependency build example
Hang tight while we grab the latest from your chart repositories...
...Successfully got an update from the "helmet" chart repository
Update Complete. ⎈Happy Helming!⎈
Saving 1 charts
Downloading helmet from repo https://companyinfo.github.io/helm-charts
Deleting outdated charts

$ helm template example > templated-3.13.yaml
```

Here is the side-by-side diff between the two outputs (templated-3.12.yaml vs templated 3.13.yaml)

```
---                                                             ---
# Source: example/templates/app.yaml                            # Source: example/templates/app.yaml
apiVersion: v1                                                  apiVersion: v1
kind: Service                                                   kind: Service
metadata:                                                       metadata:
  name: release-name-example                                      name: release-name-example
  namespace: "default"                                            namespace: "default"
  labels:                                                         labels:
    app.kubernetes.io/name: example                                 app.kubernetes.io/name: example
    helm.sh/chart: example-0.1.0                                    helm.sh/chart: example-0.1.0
    app.kubernetes.io/instance: release-name                        app.kubernetes.io/instance: release-name
    app.kubernetes.io/managed-by: Helm                              app.kubernetes.io/managed-by: Helm
spec:                                                           spec:
  type: ClusterIP                                                 type: ClusterIP
  sessionAffinity: None                                           sessionAffinity: None
  ports:                                                          ports:
    - name: http                                                    - name: http
      port: 80                                                        port: 80
      protocol: TCP                                                   protocol: TCP
      targetPort: http                                                targetPort: http
  selector:                                                       selector:
    app.kubernetes.io/name: example                                 app.kubernetes.io/name: example
    app.kubernetes.io/instance: release-name                        app.kubernetes.io/instance: release-name
---                                                           <
# Source: example/templates/app.yaml                          <
apiVersion: apps/v1                                           <
kind: Deployment                                              <
metadata:                                                     <
  name: release-name-example                                  <
  namespace: "default"                                        <
  labels:                                                     <
    app.kubernetes.io/name: example                           <
    helm.sh/chart: example-0.1.0                              <
    app.kubernetes.io/instance: release-name                  <
    app.kubernetes.io/managed-by: Helm                        <
spec:                                                         <
  replicas: 1                                                 <
  revisionHistoryLimit: 10                                    <
  selector:                                                   <
    matchLabels:                                              <
      app.kubernetes.io/name: example                         <
      app.kubernetes.io/instance: release-name                <
  strategy:                                                   <
    type: RollingUpdate                                       <
  template:                                                   <
    metadata:                                                 <
      annotations:                                            <
        helm.sh/revision: "1"                                 <
      labels:                                                 <
        app.kubernetes.io/name: example                       <
        helm.sh/chart: example-0.1.0                          <
        app.kubernetes.io/instance: release-name              <
        app.kubernetes.io/managed-by: Helm                    <
    spec:                                                     <
      restartPolicy: Always                                   <
      containers:                                             <
        - name: example                                       <
          image: docker.io/hello-world:latest                 <
          imagePullPolicy: Always                             <
          ports:                                              <
            - containerPort: 8080                             <
              name: http                                      <
              protocol: TCP                                   <
          resources:                                          <
            limits: {}                                        <
            requests: {}                                      <
      affinity:                                               <
        podAntiAffinity:                                      <
          preferredDuringSchedulingIgnoredDuringExecution:    <
            - podAffinityTerm:                                <
                labelSelector:                                <
                  matchLabels:                                <
                    app.kubernetes.io/name: example           <
                    app.kubernetes.io/instance: release-name  <
                topologyKey: kubernetes.io/hostname           <
              weight: 1                                       <
      serviceAccountName: default                             <
```






Pull request description:
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 parent charts to import large set of values from a child and then override select values.

This change is needed for backwards compatibility.

Fixes #12460

<!--  Thanks for sending a pull request!  Here are some tips for you:
1. Make sure to read the Contributing Guide before submitting your PR: https://github.com/helm/helm/blob/main/CONTRIBUTING.md
2. If this PR closes another issue, add 'closes #<issue number>' somewhere in the PR summary. GitHub will automatically close that issue when this PR gets merged. Alternatively, adding 'refs #<issue number>' will not close the issue, but help provide the reviewer more context.-->

**What this PR does / why we need it**: The values fixes in 3.13.0 had the wrong prioritization for values which broke backwards compatibility and did not make sense. Imported values should be able to be used (e.g., with a library chart) while being able override select values.

**Special notes for your reviewer**:
- Tests that change here are restored to 3.12.x values for the tests.
- This restores the previous ordering. [See 3.12.3 here](https://github.com/helm/helm/blob/v3.12.3/pkg/chartutil/dependencies.go#L271).

**If applicable**:
- [ ] this PR contains documentation
- [x] this PR contains unit tests
- [x] this PR has been tested for backwards compatibility


Patch:
diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml
index 0a32365db..58ef76781 100644
--- a/.github/workflows/release.yml
+++ b/.github/workflows/release.yml
@@ -19,8 +19,7 @@ jobs:
     steps:
       - name: Checkout source code
         uses: actions/checkout@f43a0e5ff2bd294095638e18286ca9a3d1956744 # pin@v3.6.0
-        with:
-          fetch-depth: 0
+        fetch-depth: 0
 
       - name: Setup Go
         uses: actions/setup-go@93397bea11091df50f3d7e59dc26a7711a8bcfbe # pin@4.1.0
diff --git a/pkg/chartutil/dependencies.go b/pkg/chartutil/dependencies.go
index c38a8b6c4..a87f0fbe8 100644
--- a/pkg/chartutil/dependencies.go
+++ b/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


Test patch:
diff --git a/cmd/helm/template_test.go b/cmd/helm/template_test.go
index 123a4c9bc..e5b939879 100644
--- a/cmd/helm/template_test.go
+++ b/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",
diff --git a/cmd/helm/testdata/output/template-subchart-cm.txt b/cmd/helm/testdata/output/template-subchart-cm.txt
index f7e7b3d37..9cc9e2296 100644
--- a/cmd/helm/testdata/output/template-subchart-cm.txt
+++ b/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
diff --git a/pkg/chartutil/dependencies_test.go b/pkg/chartutil/dependencies_test.go
index 34ae12f95..7e035be5f 100644
--- a/pkg/chartutil/dependencies_test.go
+++ b/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 {
diff --git a/pkg/chartutil/testdata/three-level-dependent-chart/README.md b/pkg/chartutil/testdata/three-level-dependent-chart/README.md
index e6f586a5c..536bb9792 100644
--- a/pkg/chartutil/testdata/three-level-dependent-chart/README.md
+++ b/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`