Merge pull request #6430 from karuppiah7890/fix-install-storing-computed-values

fix install storing computed values in release instead of user supplied values

pkg/action/action_test.go

@@ -166,6 +166,13 @@ func buildChart(opts ...chartOption) *chart.Chart {
 	return c.Chart
 }
 
+func withSampleValues() chartOption {
+	values := map[string]interface{}{"someKey": "someValue"}
+	return func(opts *chartOptions) {
+		opts.Values = values
+	}
+}
+
 func withNotes(notes string) chartOption {
 	return func(opts *chartOptions) {
 		opts.Templates = append(opts.Templates, &chart.File{

pkg/action/install_test.go

@@ -74,6 +74,33 @@ func TestInstallRelease(t *testing.T) {
 	is.Equal(rel.Info.Description, "Install complete")
 }
 
+func TestInstallReleaseWithValues(t *testing.T) {
+	is := assert.New(t)
+	instAction := installAction(t)
+	userVals := map[string]interface{}{}
+	expectedUserValues := map[string]interface{}{}
+	res, err := instAction.Run(buildChart(withSampleValues()), userVals)
+	if err != nil {
+		t.Fatalf("Failed install: %s", err)
+	}
+	is.Equal(res.Name, "test-install-release", "Expected release name.")
+	is.Equal(res.Namespace, "spaced")
+
+	rel, err := instAction.cfg.Releases.Get(res.Name, res.Version)
+	is.NoError(err)
+
+	is.Len(rel.Hooks, 1)
+	is.Equal(rel.Hooks[0].Manifest, manifestWithHook)
+	is.Equal(rel.Hooks[0].Events[0], release.HookPostInstall)
+	is.Equal(rel.Hooks[0].Events[1], release.HookPreDelete, "Expected event 0 is pre-delete")
+
+	is.NotEqual(len(res.Manifest), 0)
+	is.NotEqual(len(rel.Manifest), 0)
+	is.Contains(rel.Manifest, "---\n# Source: hello/templates/hello\nhello: world")
+	is.Equal("Install complete", rel.Info.Description)
+	is.Equal(expectedUserValues, rel.Config)
+}
+
 func TestInstallReleaseClientOnly(t *testing.T) {
 	is := assert.New(t)
 	instAction := installAction(t)

pkg/action/lint.go

@@ -83,10 +83,6 @@ func (l *Lint) Run(paths []string, vals map[string]interface{}) *LintResult {
 func lintChart(path string, vals map[string]interface{}, namespace string, strict bool) (support.Linter, error) {
 	var chartPath string
 	linter := support.Linter{}
-	currentVals := make(map[string]interface{}, len(vals))
-	for key, value := range vals {
-		currentVals[key] = value
-	}
 
 	if strings.HasSuffix(path, ".tgz") {
 		tempDir, err := ioutil.TempDir("", "helm-lint")
@@ -120,5 +116,5 @@ func lintChart(path string, vals map[string]interface{}, namespace string, stric
 		return linter, errLintNoChart
 	}
 
-	return lint.All(chartPath, currentVals, namespace, strict), nil
+	return lint.All(chartPath, vals, namespace, strict), nil
 }

pkg/chartutil/coalesce.go

@@ -34,13 +34,13 @@ import (
 //	- A chart has access to all of the variables for it, as well as all of
 //		the values destined for its dependencies.
 func CoalesceValues(chrt *chart.Chart, vals map[string]interface{}) (Values, error) {
-	if vals == nil {
+	// create a copy of vals and then pass it to coalesce
-		vals = make(map[string]interface{})
+	// and coalesceDeps, as both will mutate the passed values
+	valsCopy := copyMap(vals)
+	if _, err := coalesce(chrt, valsCopy); err != nil {
+		return valsCopy, err
 	}
-	if _, err := coalesce(chrt, vals); err != nil {
+	return coalesceDeps(chrt, valsCopy)
-		return vals, err
-	}
-	return coalesceDeps(chrt, vals)
 }
 
 // coalesce coalesces the dest values and the chart values, giving priority to the dest values.

pkg/chartutil/coalesce_test.go

@@ -19,6 +19,8 @@ package chartutil
 import (
 	"encoding/json"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
 
 // ref: http://www.yaml.org/spec/1.2/spec.html#id2803362
@@ -48,6 +50,7 @@ pequod:
 `)
 
 func TestCoalesceValues(t *testing.T) {
+	is := assert.New(t)
 	c := loadChart(t, "testdata/moby")
 
 	vals, err := ReadValues(testCoalesceValuesYaml)
@@ -55,6 +58,14 @@ func TestCoalesceValues(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	// taking a copy of the values before passing it
+	// to CoalesceValues as argument, so that we can
+	// use it for asserting later
+	valsCopy := make(Values, len(vals))
+	for key, value := range vals {
+		valsCopy[key] = value
+	}
+
 	v, err := CoalesceValues(c, vals)
 	if err != nil {
 		t.Fatal(err)
@@ -102,6 +113,9 @@ func TestCoalesceValues(t *testing.T) {
 			t.Errorf("Expected key %q to be removed, still present", nullKey)
 		}
 	}
+
+	// CoalesceValues should not mutate the passed arguments
+	is.Equal(valsCopy, vals)
 }
 
 func TestCoalesceTables(t *testing.T) {