Repository: helm/helm (go)

Issue:
Helm lint not pulling values outside of the default values file for schema validation
<!-- 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): -->

Hi 

Im not 100% if this is a bug or the intended functionality of the helm v3 lint command but I noticed an issue when I uplifted our ci pipeline from helm v2 to helm v3 (see actual versions below)

What is currently happening?
when I run `helm lint . --values=second_values.yaml` any value that is in second_values.yaml that is also in the values.yaml file of the chart will not be overridden with the values of second_values.yaml. 
Example
![image](https://user-images.githubusercontent.com/22730434/95833505-a3f27080-0d33-11eb-90a1-fe040a46d2c7.png)


I was able to recreate the issue with a test chart (also attached as [testchart-0.1.0.zip](https://github.com/helm/helm/files/5369833/testchart-0.1.0.zip))

I dont know if this is a change in functionality. I am aware that the helm lint command handles schema checks differently between helm v2 and helm v3 however the schema used to validate the helm v3 lint command should be correct provided the second_values.yaml file is used.

Any help on this issue/question would be helpful as the only way we can work around this is by either disabling the helm lint command in our ci pipelines or removing some validation from our values.schema.json file

Output of `helm version`:
helm 3 installation
`helm version
version.BuildInfo{Version:"v3.3.4", GitCommit:"a61ce5633af99708171414353ed49547cf05013d", GitTreeState:"clean", GoVersion:"go1.14.9"}
`
helm 2 installation
`
helm2 version
Client: &version.Version{SemVer:"v2.16.5", GitCommit:"89bd14c1541fa93a09492010030fd3699ca65a97", GitTreeState:"clean"}
Server: &version.Version{SemVer:"v2.16.1", GitCommit:"65cac393eeadea00384ea7108296ee6e5509c4de", GitTreeState:"clean"}
`

Output of `kubectl version`:
`kubectl version
Client Version: version.Info{Major:"1", Minor:"14", GitVersion:"v1.14.6", GitCommit:"96fac5cd13a5dc064f7d9f4f23030a6aeface6cc", GitTreeState:"clean", BuildDate:"2019-08-19T11:13:49Z", GoVersion:"go1.12.9", Compiler:"gc", Platform:"windows/amd64"}
Server Version: version.Info{Major:"1", Minor:"17", GitVersion:"v1.17.3", GitCommit:"06ad960bfd03b39c8310aaf92d1e7c12ce618213", GitTreeState:"clean", BuildDate:"2020-02-16T17:47:13Z", GoVersion:"go1.13.6", Compiler:"gc", Platform:"linux/amd64"}`

Cloud Provider/Platform (AKS, GKE, Minikube etc.):  N/A




Pull request description:
corrected order of helm lint coalescing of multiple values files
<!--  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**:

**Example**

The following command is run with the below value files and a json schema specifying service.port is required.

```bash
helm lint ./testchart --values ./testchart/second_values.yaml
```

_values.yaml_
```yaml
{
...
service:
  type: ClusterIP
  port:
...
}
```

_second_values.yaml_
```yaml
{
...
service:
  port: 80
...
}
```

The result is an error.
```bash
helm lint ./testchart --values ./testchart/second_values.yaml
==> Linting ./testchart
[INFO] Chart.yaml: icon is recommended
[ERROR] values.yaml: - service: port is required


Error: 1 chart(s) linted, 1 chart(s) failed
```

See the attached chart below for the full, runnable example.
[testchart-0.1.0.zip](https://github.com/helm/helm/files/5850689/testchart-0.1.0.zip)


**Problem**

Running helm lint with the flag -f/--values specifies a user-provided values file.  When using helm lint if a user-provided values file exists, then the user-provided values file and chart provided values.yaml are coalesced to create a single values file.  When coalescing the values files if a value exists, see port above, in both the chart provided values.yaml and a user-provided values file, see second_values.yaml above, helm lint uses the value in the chart provided values.yaml.

This behavior is backwards as the user-provided values file should take precedent over the chart provided values.yaml.


**Solution**

The values files are coalesced in the following line of _pkg/lint/rules/values.go_
```go
values = chartutil.CoalesceTables(values, overrides)
```
The CoalesceTables function states the following
```go
// CoalesceTables merges a source map into a destination map.
//
// dest is considered authoritative.
func CoalesceTables(dst, src map[string]interface{}) map[string]interface{
```
Since dst is authoritative, overrides should be dst.  Also overrides is passed in so in order to not manipulate it, a deep copy of overrides should be used.

**Special notes for your reviewer**:
Fixes #8880 
Fixes #8892 

**If applicable**:
- [ ] this PR contains documentation
- [*] this PR contains unit tests
- [ ] this PR has been tested for backwards compatibility


Patch:
diff --git a/pkg/lint/rules/values.go b/pkg/lint/rules/values.go
index c596687c5..79a294326 100644
--- a/pkg/lint/rules/values.go
+++ b/pkg/lint/rules/values.go
@@ -70,8 +70,9 @@ func validateValuesFile(valuesPath string, overrides map[string]interface{}) err
 	// level values against the top-level expectations. Subchart values are not linted.
 	// We could change that. For now, though, we retain that strategy, and thus can
 	// coalesce tables (like reuse-values does) instead of doing the full chart
-	// CoalesceValues.
-	values = chartutil.CoalesceTables(values, overrides)
+	// CoalesceValues
+	coalescedValues := chartutil.CoalesceTables(make(map[string]interface{}, len(overrides)), overrides)
+	coalescedValues = chartutil.CoalesceTables(coalescedValues, values)
 
 	ext := filepath.Ext(valuesPath)
 	schemaPath := valuesPath[:len(valuesPath)-len(ext)] + ".schema.json"
@@ -82,5 +83,5 @@ func validateValuesFile(valuesPath string, overrides map[string]interface{}) err
 	if err != nil {
 		return err
 	}
-	return chartutil.ValidateAgainstSingleSchema(values, schema)
+	return chartutil.ValidateAgainstSingleSchema(coalescedValues, schema)
 }


Test patch:
diff --git a/pkg/lint/rules/values_test.go b/pkg/lint/rules/values_test.go
index 6d93d630e..23335cc01 100644
--- a/pkg/lint/rules/values_test.go
+++ b/pkg/lint/rules/values_test.go
@@ -118,6 +118,53 @@ func TestValidateValuesFileSchemaOverrides(t *testing.T) {
 	}
 }
 
+func TestValidateValuesFile(t *testing.T) {
+	tests := []struct {
+		name         string
+		yaml         string
+		overrides    map[string]interface{}
+		errorMessage string
+	}{
+		{
+			name:      "value added",
+			yaml:      "username: admin",
+			overrides: map[string]interface{}{"password": "swordfish"},
+		},
+		{
+			name:         "value not overridden",
+			yaml:         "username: admin\npassword:",
+			overrides:    map[string]interface{}{"username": "anotherUser"},
+			errorMessage: "Expected: string, given: null",
+		},
+		{
+			name:      "value overridden",
+			yaml:      "username: admin\npassword:",
+			overrides: map[string]interface{}{"username": "anotherUser", "password": "swordfish"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			tmpdir := ensure.TempFile(t, "values.yaml", []byte(tt.yaml))
+			defer os.RemoveAll(tmpdir)
+			createTestingSchema(t, tmpdir)
+
+			valfile := filepath.Join(tmpdir, "values.yaml")
+
+			err := validateValuesFile(valfile, tt.overrides)
+
+			switch {
+			case err != nil && tt.errorMessage == "":
+				t.Errorf("Failed validation with %s", err)
+			case err == nil && tt.errorMessage != "":
+				t.Error("expected values file to fail parsing")
+			case err != nil && tt.errorMessage != "":
+				assert.Contains(t, err.Error(), tt.errorMessage, "Failed with unexpected error")
+			}
+		})
+	}
+}
+
 func createTestingSchema(t *testing.T, dir string) string {
 	t.Helper()
 	schemafile := filepath.Join(dir, "values.schema.json")