From 018982f177fcc123877307d9cadce3d2e361db0c Mon Sep 17 00:00:00 2001 From: Elliot Kennedy Date: Thu, 17 Dec 2020 04:00:03 +0000 Subject: [PATCH] [#9136] - Deep copy chart values before coalescing to prevent mutating the chart Signed-off-by: Elliot Kennedy --- pkg/chartutil/coalesce.go | 7 ++++++- pkg/chartutil/coalesce_test.go | 21 ++++++++++++++------- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/pkg/chartutil/coalesce.go b/pkg/chartutil/coalesce.go index e086d8b6e..938e12bc5 100644 --- a/pkg/chartutil/coalesce.go +++ b/pkg/chartutil/coalesce.go @@ -146,7 +146,12 @@ func copyMap(src map[string]interface{}) map[string]interface{} { // // Values in v will override the values in the chart. func coalesceValues(c *chart.Chart, v map[string]interface{}) { - for key, val := range c.Values { + valsCopy, err := copystructure.Copy(c.Values) + if err != nil { + log.Printf("warning: could not copy values %+v. Using raw values from chart. Exception:\n%s", c.Values, err) + valsCopy = c.Values + } + for key, val := range valsCopy.(map[string]interface{}) { if value, ok := v[key]; ok { if value == nil { // When the YAML value is null, we remove the value's key. diff --git a/pkg/chartutil/coalesce_test.go b/pkg/chartutil/coalesce_test.go index 5a4656d71..6aee16933 100644 --- a/pkg/chartutil/coalesce_test.go +++ b/pkg/chartutil/coalesce_test.go @@ -18,6 +18,7 @@ package chartutil import ( "encoding/json" + "github.com/mitchellh/copystructure" "testing" "github.com/stretchr/testify/assert" @@ -78,11 +79,12 @@ func TestCoalesceValues(t *testing.T) { "right": "exists", "scope": "moby", "top": "nope", + "pequod": map[string]interface{}{"nested": map[string]interface{}{"bar": nil}}, }, }, withDeps(&chart.Chart{ Metadata: &chart.Metadata{Name: "pequod"}, - Values: map[string]interface{}{"name": "pequod", "scope": "pequod"}, + Values: map[string]interface{}{"name": "pequod", "scope": "pequod", "nested": map[string]interface{}{"bar": "true"}}, }, &chart.Chart{ Metadata: &chart.Metadata{Name: "ahab"}, @@ -105,12 +107,15 @@ 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 + // take a copy of the values and chart values before passing + // to CoalesceValues, so that we can use for asserting later + valsCopy, err := copystructure.Copy(vals) + if err != nil { + t.Fatal(err) + } + chartValsCopy, err := copystructure.Copy(c.Values) + if err != nil { + t.Fatal(err) } v, err := CoalesceValues(c, vals) @@ -131,6 +136,7 @@ func TestCoalesceValues(t *testing.T) { {"{{.global.subject}}", "Queequeg"}, {"{{.global.harpooner}}", ""}, {"{{.pequod.name}}", "pequod"}, + {"{{.pequod.nested.bar}}", ""}, {"{{.pequod.ahab.name}}", "ahab"}, {"{{.pequod.ahab.scope}}", "whale"}, {"{{.pequod.ahab.nested.foo}}", "true"}, @@ -177,6 +183,7 @@ func TestCoalesceValues(t *testing.T) { // CoalesceValues should not mutate the passed arguments is.Equal(valsCopy, vals) + is.Equal(chartValsCopy, c.Values) } func TestCoalesceTables(t *testing.T) {