Repository: helm/helm (go)

Issue:
Global values defined in a subchart can affect global values in parent chart and sibling charts
The issue can be reproduced with a chart that contains 2 subcharts and the following values:

1. toplevel values:
    ```
    global:
      nested:
        v1: chart
    ```

2. values in subchart alpha:
    ```
    global:
      nested:
        v2: alpha
        v3: alpha
    ```
3. values in subchart beta:
    ```
    global:
      nested:
        v2: beta
        v4: beta
    ```

After installing such a chart, `helm get values -a` produced the following output:

```
COMPUTED VALUES:
alpha:
  global:
    nested:
      v1: chart
      v2: alpha
      v3: alpha
      v4: beta
beta:
  global:
    nested:
      v1: chart
      v2: alpha
      v3: alpha
      v4: beta
global:
  nested:
    v1: chart
    v2: alpha
    v3: alpha
    v4: beta

```

This shows that values from the subcharts become visible in the toplevel chart and in other subcharts,

In my opinion, this behavior contradicts the following statement, which can be found in https://helm.sh/docs/topics/charts/:
> "There is no way for a subchart to influence the values of the parent chart."

Here is also a zip file that contains my example chart to reproduce the issue: [chart.zip](https://github.com/helm/helm/files/5933020/chart.zip)

Output of `helm version`:
```
version.BuildInfo{Version:"v3.3.0", GitCommit:"8a4aeec08d67a7b84472007529e8097ec3742105", GitTreeState:"dirty", GoVersion:"go1.14.7"}
```

Output of `kubectl version`:
```
Client Version: version.Info{Major:"1", Minor:"19", GitVersion:"v1.19.0", GitCommit:"e19964183377d0ec2052d1f1fa930c4d7575bd50", GitTreeState:"clean", BuildDate:"2020-08-26T14:30:33Z", GoVersion:"go1.15", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"1", Minor:"19", GitVersion:"v1.19.0", GitCommit:"e19964183377d0ec2052d1f1fa930c4d7575bd50", GitTreeState:"clean", BuildDate:"2020-08-26T14:23:04Z", GoVersion:"go1.15", Compiler:"gc", Platform:"linux/amd64"}
```

Cloud Provider/Platform (AKS, GKE, Minikube etc.): N/A




Pull request:
Fix 9332
<!--  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/master/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**:

This PR fixes issue 9332, which is about global values defined in subchart can affect globals in parent chart or sibling charts.

**Special notes for your reviewer**:

The fix consists of 3 commits.

- a unit test enhancement, which failed before the fix was implemented.
- the actual fix, which was just a missing 'continue' statement, such that the intended code was later overrule
- an additional code improvement of `coalesce.go to` get rid or continue statement and using appropriate else
  blocks instead. This is not actually necessary for the fix, but should improve the code to make a similar issue
  less likely.

**If applicable**:
- [X] this PR contains unit tests

closes #9332

Patch:
diff --git a/pkg/chartutil/coalesce.go b/pkg/chartutil/coalesce.go
index 7b1ee1d80..b49a31b01 100644
--- a/pkg/chartutil/coalesce.go
+++ b/pkg/chartutil/coalesce.go
@@ -120,13 +120,11 @@ func coalesceGlobals(dest, src map[string]interface{}) {
 					// top-down.
 					CoalesceTables(vv, destvmap)
 					dg[key] = vv
-					continue
 				}
 			}
 		} else if dv, ok := dg[key]; ok && istable(dv) {
 			// It's not clear if this condition can actually ever trigger.
 			log.Printf("key %s is table. Skipping", key)
-			continue
 		} else {
 			// TODO: Do we need to do any additional checking on the value?
 			dg[key] = val
@@ -159,15 +157,15 @@ func coalesceValues(c *chart.Chart, v map[string]interface{}) {
 				src, ok := val.(map[string]interface{})
 				if !ok {
 					// If the original value is nil, there is nothing to coalesce, so we don't print
-					// the warning but simply continue
+					// the warning
 					if val != nil {
 						log.Printf("warning: skipped value for %s: Not a table.", key)
 					}
-					continue
+				} else {
+					// Because v has higher precedence than nv, dest values override src
+					// values.
+					CoalesceTables(dest, src)
 				}
-				// Because v has higher precedence than nv, dest values override src
-				// values.
-				CoalesceTables(dest, src)
 			}
 		} else {
 			// If the key is not in v, copy it from nv.


Test patch:
diff --git a/pkg/chartutil/coalesce_test.go b/pkg/chartutil/coalesce_test.go
index fc55e0a25..6f7c37483 100644
--- a/pkg/chartutil/coalesce_test.go
+++ b/pkg/chartutil/coalesce_test.go
@@ -78,17 +78,27 @@ func TestCoalesceValues(t *testing.T) {
 			"right":    "exists",
 			"scope":    "moby",
 			"top":      "nope",
+			"global": map[string]interface{}{
+				"nested2": map[string]interface{}{"l0": "moby"},
+			},
 		},
 	},
 		withDeps(&chart.Chart{
 			Metadata: &chart.Metadata{Name: "pequod"},
-			Values:   map[string]interface{}{"name": "pequod", "scope": "pequod"},
+			Values: map[string]interface{}{
+				"name":  "pequod",
+				"scope": "pequod",
+				"global": map[string]interface{}{
+					"nested2": map[string]interface{}{"l1": "pequod"},
+				},
+			},
 		},
 			&chart.Chart{
 				Metadata: &chart.Metadata{Name: "ahab"},
 				Values: map[string]interface{}{
 					"global": map[string]interface{}{
-						"nested": map[string]interface{}{"foo": "bar"},
+						"nested":  map[string]interface{}{"foo": "bar"},
+						"nested2": map[string]interface{}{"l2": "ahab"},
 					},
 					"scope":  "ahab",
 					"name":   "ahab",
@@ -99,7 +109,12 @@ func TestCoalesceValues(t *testing.T) {
 		),
 		&chart.Chart{
 			Metadata: &chart.Metadata{Name: "spouter"},
-			Values:   map[string]interface{}{"scope": "spouter"},
+			Values: map[string]interface{}{
+				"scope": "spouter",
+				"global": map[string]interface{}{
+					"nested2": map[string]interface{}{"l1": "spouter"},
+				},
+			},
 		},
 	)
 
@@ -152,6 +167,19 @@ func TestCoalesceValues(t *testing.T) {
 		{"{{.spouter.global.nested.boat}}", "true"},
 		{"{{.pequod.global.nested.sail}}", "true"},
 		{"{{.spouter.global.nested.sail}}", "<no value>"},
+
+		{"{{.global.nested2.l0}}", "moby"},
+		{"{{.global.nested2.l1}}", "<no value>"},
+		{"{{.global.nested2.l2}}", "<no value>"},
+		{"{{.pequod.global.nested2.l0}}", "moby"},
+		{"{{.pequod.global.nested2.l1}}", "pequod"},
+		{"{{.pequod.global.nested2.l2}}", "<no value>"},
+		{"{{.pequod.ahab.global.nested2.l0}}", "moby"},
+		{"{{.pequod.ahab.global.nested2.l1}}", "pequod"},
+		{"{{.pequod.ahab.global.nested2.l2}}", "ahab"},
+		{"{{.spouter.global.nested2.l0}}", "moby"},
+		{"{{.spouter.global.nested2.l1}}", "spouter"},
+		{"{{.spouter.global.nested2.l2}}", "<no value>"},
 	}
 
 	for _, tt := range tests {