From 25324ca8db0cbcf95296cae2f20aebfabd8f1423 Mon Sep 17 00:00:00 2001 From: Karuppiah Natarajan Date: Tue, 10 Sep 2019 09:58:17 +0530 Subject: [PATCH] fix install storing computed values in release instead of user supplied values Signed-off-by: Karuppiah Natarajan --- pkg/action/action_test.go | 7 +++++++ pkg/action/install_test.go | 27 +++++++++++++++++++++++++++ pkg/action/lint.go | 6 +----- pkg/chartutil/coalesce.go | 12 ++++++------ pkg/chartutil/coalesce_test.go | 14 ++++++++++++++ 5 files changed, 55 insertions(+), 11 deletions(-) diff --git a/pkg/action/action_test.go b/pkg/action/action_test.go index becbc9ed0..716132853 100644 --- a/pkg/action/action_test.go +++ b/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{ diff --git a/pkg/action/install_test.go b/pkg/action/install_test.go index 2315fc4fa..6ae891b42 100644 --- a/pkg/action/install_test.go +++ b/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) diff --git a/pkg/action/lint.go b/pkg/action/lint.go index c8fa4061d..a3ec1563a 100644 --- a/pkg/action/lint.go +++ b/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 } diff --git a/pkg/chartutil/coalesce.go b/pkg/chartutil/coalesce.go index 466e0d119..ea05e0b5b 100644 --- a/pkg/chartutil/coalesce.go +++ b/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 { - vals = make(map[string]interface{}) + // create a copy of vals and then pass it to coalesce + // 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 vals, err - } - return coalesceDeps(chrt, vals) + return coalesceDeps(chrt, valsCopy) } // coalesce coalesces the dest values and the chart values, giving priority to the dest values. diff --git a/pkg/chartutil/coalesce_test.go b/pkg/chartutil/coalesce_test.go index 62d9e4064..6e82de590 100644 --- a/pkg/chartutil/coalesce_test.go +++ b/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) {