fix(chartutil): Uses copystructure for deep copy to avoid using gob

We already had the copystructure library in our dependencies transitively
through sprig. This solves a gob encoding bug that was causing issues with
chart testing

Signed-off-by: Taylor Thomas <taylor.thomas@microsoft.com>
pull/6691/head
Taylor Thomas 5 years ago
parent 7892a36cb3
commit a758490f4d

@ -28,6 +28,7 @@ require (
github.com/gregjones/httpcache v0.0.0-20181110185634-c63ab54fda8f // indirect github.com/gregjones/httpcache v0.0.0-20181110185634-c63ab54fda8f // indirect
github.com/mattn/go-runewidth v0.0.4 // indirect github.com/mattn/go-runewidth v0.0.4 // indirect
github.com/mattn/go-shellwords v1.0.5 github.com/mattn/go-shellwords v1.0.5
github.com/mitchellh/copystructure v1.0.0
github.com/opencontainers/go-digest v1.0.0-rc1 github.com/opencontainers/go-digest v1.0.0-rc1
github.com/opencontainers/image-spec v1.0.1 github.com/opencontainers/image-spec v1.0.1
github.com/pkg/errors v0.8.1 github.com/pkg/errors v0.8.1

@ -1,39 +0,0 @@
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
}

@ -1,130 +0,0 @@
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)
}
}

@ -19,8 +19,7 @@ package chartutil
import ( import (
"log" "log"
"helm.sh/helm/v3/internal/third_party/deepcopy" "github.com/mitchellh/copystructure"
"github.com/pkg/errors" "github.com/pkg/errors"
"helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chart"
@ -38,10 +37,16 @@ import (
func CoalesceValues(chrt *chart.Chart, vals map[string]interface{}) (Values, error) { func CoalesceValues(chrt *chart.Chart, vals map[string]interface{}) (Values, error) {
// create a copy of vals and then pass it to coalesce // create a copy of vals and then pass it to coalesce
// and coalesceDeps, as both will mutate the passed values // and coalesceDeps, as both will mutate the passed values
valsCopy, err := deepcopy.Map(vals) v, err := copystructure.Copy(vals)
if err != nil { if err != nil {
return vals, err return vals, err
} }
valsCopy := v.(map[string]interface{})
// if we have an empty map, make sure it is initialized
if valsCopy == nil {
valsCopy = make(map[string]interface{})
}
if _, err := coalesce(chrt, valsCopy); err != nil { if _, err := coalesce(chrt, valsCopy); err != nil {
return valsCopy, err return valsCopy, err
} }

Loading…
Cancel
Save