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

fix install storing computed values in release

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
+}

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)
+	}
+}

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
 	}

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)

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)

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
 	}