diff --git a/internal/third_party/deepcopy/util.go b/internal/third_party/deepcopy/util.go new file mode 100644 index 000000000..0316d6c19 --- /dev/null +++ b/internal/third_party/deepcopy/util.go @@ -0,0 +1,39 @@ +package deepcopy + +// DeepCopyMap a function for deep copying map[string]interface{} +// values. Inspired by: +// https://gist.github.com/soroushjp/0ec92102641ddfc3ad5515ca76405f4d by Soroush Pour +// The gist code is MIT licensed +// which in turn has been inspired by the StackOverflow answer at: +// http://stackoverflow.com/a/28579297/1366283 +// +// Uses the golang.org/pkg/encoding/gob package to do this and therefore has the +// same caveats. +// See: https://blog.golang.org/gobs-of-data +// See: https://golang.org/pkg/encoding/gob/ + +import ( + "bytes" + "encoding/gob" +) + +func init() { + gob.Register(map[string]interface{}{}) +} + +// Map performs a deep copy of the given map m. +func Map(m map[string]interface{}) (map[string]interface{}, error) { + var buf bytes.Buffer + enc := gob.NewEncoder(&buf) + dec := gob.NewDecoder(&buf) + err := enc.Encode(m) + if err != nil { + return nil, err + } + var mapCopy map[string]interface{} + err = dec.Decode(&mapCopy) + if err != nil { + return nil, err + } + return mapCopy, nil +} diff --git a/internal/third_party/deepcopy/util_test.go b/internal/third_party/deepcopy/util_test.go new file mode 100644 index 000000000..71bec127a --- /dev/null +++ b/internal/third_party/deepcopy/util_test.go @@ -0,0 +1,130 @@ +package deepcopy_test + +import ( + "helm.sh/helm/v3/internal/third_party/deepcopy" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestCopyMap(t *testing.T) { + testCases := []struct { + // original and expectedOriginal are the same value in each test case. We do + // this to avoid unintentionally asserting against a mutated + // expectedOriginal and having the test pass erroneously. We also do not + // want to rely on the deep copy function we are testing to ensure this does + // not happen. + original map[string]interface{} + transformer func(m map[string]interface{}) map[string]interface{} + expectedCopy map[string]interface{} + expectedOriginal map[string]interface{} + }{ + // reassignment of entire map, should be okay even without deep copy. + { + original: nil, + transformer: func(m map[string]interface{}) map[string]interface{} { + return map[string]interface{}{} + }, + expectedCopy: map[string]interface{}{}, + expectedOriginal: nil, + }, + { + original: map[string]interface{}{}, + transformer: func(m map[string]interface{}) map[string]interface{} { + return nil + }, + expectedCopy: nil, + expectedOriginal: map[string]interface{}{}, + }, + // mutation of map + { + original: map[string]interface{}{}, + transformer: func(m map[string]interface{}) map[string]interface{} { + m["foo"] = "bar" + return m + }, + expectedCopy: map[string]interface{}{ + "foo": "bar", + }, + expectedOriginal: map[string]interface{}{}, + }, + { + original: map[string]interface{}{ + "foo": "bar", + }, + transformer: func(m map[string]interface{}) map[string]interface{} { + m["foo"] = "car" + return m + }, + expectedCopy: map[string]interface{}{ + "foo": "car", + }, + expectedOriginal: map[string]interface{}{ + "foo": "bar", + }, + }, + // mutation of nested maps + { + original: map[string]interface{}{}, + transformer: func(m map[string]interface{}) map[string]interface{} { + m["foo"] = map[string]interface{}{ + "biz": "baz", + } + return m + }, + expectedCopy: map[string]interface{}{ + "foo": map[string]interface{}{ + "biz": "baz", + }, + }, + expectedOriginal: map[string]interface{}{}, + }, + { + original: map[string]interface{}{ + "foo": map[string]interface{}{ + "biz": "booz", + "gaz": "gooz", + }, + }, + transformer: func(m map[string]interface{}) map[string]interface{} { + m["foo"] = map[string]interface{}{ + "biz": "baz", + } + return m + }, + expectedCopy: map[string]interface{}{ + "foo": map[string]interface{}{ + "biz": "baz", + }, + }, + expectedOriginal: map[string]interface{}{ + "foo": map[string]interface{}{ + "biz": "booz", + "gaz": "gooz", + }, + }, + }, + // mutation of slice values + { + original: map[string]interface{}{ + "foo": []string{"biz", "baz"}, + }, + transformer: func(m map[string]interface{}) map[string]interface{} { + m["foo"].([]string)[0] = "hiz" + return m + }, + expectedCopy: map[string]interface{}{ + "foo": []string{"hiz", "baz"}, + }, + expectedOriginal: map[string]interface{}{ + "foo": []string{"biz", "baz"}, + }, + }, + } + for i, tc := range testCases { + mapCopy, err := deepcopy.Map(tc.original) + assert.NoError(t, err) + assert.Exactly(t, tc.expectedCopy, tc.transformer(mapCopy), "copy was not mutated. test case: %d", i) + assert.Exactly(t, tc.expectedOriginal, tc.original, "original was mutated. test case: %d", i) + } +} diff --git a/pkg/action/action_test.go b/pkg/action/action_test.go index 49d1bad41..e1d2cca6e 100644 --- a/pkg/action/action_test.go +++ b/pkg/action/action_test.go @@ -173,7 +173,17 @@ func withName(name string) chartOption { } func withSampleValues() chartOption { - values := map[string]interface{}{"someKey": "someValue"} + values := map[string]interface{}{ + "someKey": "someValue", + "nestedKey": map[string]interface{}{ + "simpleKey": "simpleValue", + "anotherNestedKey": map[string]interface{}{ + "yetAnotherNestedKey": map[string]interface{}{ + "youReadyForAnotherNestedKey": "No", + }, + }, + }, + } return func(opts *chartOptions) { opts.Values = values } diff --git a/pkg/action/install_test.go b/pkg/action/install_test.go index de03a5a71..406574995 100644 --- a/pkg/action/install_test.go +++ b/pkg/action/install_test.go @@ -78,8 +78,16 @@ func TestInstallRelease(t *testing.T) { func TestInstallReleaseWithValues(t *testing.T) { is := assert.New(t) instAction := installAction(t) - userVals := map[string]interface{}{} - expectedUserValues := map[string]interface{}{} + userVals := map[string]interface{}{ + "nestedKey": map[string]interface{}{ + "simpleKey": "simpleValue", + }, + } + expectedUserValues := map[string]interface{}{ + "nestedKey": map[string]interface{}{ + "simpleKey": "simpleValue", + }, + } res, err := instAction.Run(buildChart(withSampleValues()), userVals) if err != nil { t.Fatalf("Failed install: %s", err) diff --git a/pkg/action/upgrade_test.go b/pkg/action/upgrade_test.go index 69ee25cd6..0c37ec158 100644 --- a/pkg/action/upgrade_test.go +++ b/pkg/action/upgrade_test.go @@ -223,7 +223,6 @@ func TestUpgradeRelease_ReuseValues(t *testing.T) { expectedValues := map[string]interface{}{ "subchart": map[string]interface{}{ "enabled": false, - "global": map[string]interface{}{}, }, } is.Equal(expectedValues, updatedRes.Config) diff --git a/pkg/chartutil/coalesce.go b/pkg/chartutil/coalesce.go index f18d520a4..bd491da42 100644 --- a/pkg/chartutil/coalesce.go +++ b/pkg/chartutil/coalesce.go @@ -19,6 +19,8 @@ package chartutil import ( "log" + "helm.sh/helm/v3/internal/third_party/deepcopy" + "github.com/pkg/errors" "helm.sh/helm/v3/pkg/chart" @@ -36,7 +38,10 @@ import ( func CoalesceValues(chrt *chart.Chart, vals map[string]interface{}) (Values, error) { // create a copy of vals and then pass it to coalesce // and coalesceDeps, as both will mutate the passed values - valsCopy := copyMap(vals) + valsCopy, err := deepcopy.Map(vals) + if err != nil { + return vals, err + } if _, err := coalesce(chrt, valsCopy); err != nil { return valsCopy, err }