Bump sprig to v3.1.0 and mergo 3.11 (#1456)

* Bump sprig to v3.1.0 test for mergeOverwrite * Let mergo not (accidentally) try to merge unexported fields This is also a good chance separate `HelmState` with the config loaded from YAML, which I had been wanting to do for a long time. Co-authored-by: Johannes Alkjær <johannes.alkjaer@wunderman.com> Co-authored-by: Yusuke Kuoka <ykuoka@gmail.com>
2020-09-04 02:58:54 +02:00 · 2020-09-04 02:58:54 +02:00 · 9d2c0d4285
parent efd26f288a
commit 9d2c0d4285
8 changed files with 188 additions and 112 deletions
--- a/go.mod
+++ b/go.mod
@ -15,7 +15,7 @@ require (
 	github.com/hashicorp/go-retryablehttp v0.6.3 // indirect
 	github.com/hashicorp/go-version v1.2.0
 	github.com/howeyc/gopass v0.0.0-20190910152052-7cb4b85ec19c // indirect
-	github.com/imdario/mergo v0.3.9
+	github.com/imdario/mergo v0.3.11
 	github.com/konsorten/go-windows-terminal-sequences v1.0.2 // indirect
 	github.com/pierrec/lz4 v2.3.0+incompatible // indirect
 	github.com/r3labs/diff v0.0.0-20190801153147-a71de73c46ad
@ -29,7 +29,7 @@ require (
 	go.uber.org/zap v1.9.1
 	golang.org/x/sync v0.0.0-20200317015054-43a5402ce75a
 	gopkg.in/square/go-jose.v2 v2.4.0 // indirect
-	gopkg.in/yaml.v2 v2.2.4
+	gopkg.in/yaml.v2 v2.3.0
 	gotest.tools v2.2.0+incompatible
 	gotest.tools/v3 v3.0.3-0.20200410202438-4e4a41b7851a
 	k8s.io/apimachinery v0.0.0-20190409092423-760d1845f48b
--- a/go.sum
+++ b/go.sum
@ -528,6 +528,8 @@ github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1:
 github.com/imdario/mergo v0.3.8/go.mod h1:2EnlNZ0deacrJVfApfmtdGgDfMuh/nq6Ok1EcJh5FfA=
 github.com/imdario/mergo v0.3.9 h1:UauaLniWCFHWd+Jp9oCEkTBj8VO/9DKg3PV3VCNMDIg=
 github.com/imdario/mergo v0.3.9/go.mod h1:2EnlNZ0deacrJVfApfmtdGgDfMuh/nq6Ok1EcJh5FfA=
+github.com/imdario/mergo v0.3.11 h1:3tnifQM4i+fbajXKBHXWEH+KvNHqojZ778UH75j3bGA=
+github.com/imdario/mergo v0.3.11/go.mod h1:jmQim1M+e3UYxmgPu/WyfjB3N3VflVyUjjjwH0dnCYA=
 github.com/inconshreveable/mousetrap v1.0.0 h1:Z8tu5sraLXCXIcARxBp/8cbvlwVa7Z1NHg9XEKhtSvM=
 github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8=
 github.com/influxdata/influxdb v0.0.0-20190411212539-d24b7ba8c4c4 h1:3K3KcD4S6/Y2hevi70EzUTNKOS3cryQyhUnkjE6Tz0w=
@ -1099,6 +1101,7 @@ google.golang.org/api v0.20.0/go.mod h1:BwFmGc8tA3vsd7r/7kR8DY7iEEGSU04BFxCo5jP/
 google.golang.org/api v0.21.0 h1:zS+Q/CJJnVlXpXQVIz+lH0ZT2lBuT2ac7XD8Y/3w6hY=
 google.golang.org/api v0.26.0 h1:VJZ8h6E8ip82FRpQl848c5vAadxlTXrUh8RzQzSRm08=
 google.golang.org/api v0.30.0 h1:yfrXXP61wVuLb0vBcG6qaOoIoqYEzOQS8jum51jkv2w=
+google.golang.org/api v0.31.0 h1:1w5Sz/puhxFo9lTtip2n47k7toB/U2nCqOKNHd3Yrbo=
 google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM=
 google.golang.org/appengine v1.2.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4=
 google.golang.org/appengine v1.3.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4=
@ -1189,6 +1192,8 @@ gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v2 v2.2.4 h1:/eiJrUcujPVeJ3xlSWaiNi3uSVmDGBK1pDHUHAnao1I=
 gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
+gopkg.in/yaml.v2 v2.3.0 h1:clyUAQHOM3G0M3f5vQj7LuJrETvjVot3Z5el9nffUtU=
+gopkg.in/yaml.v2 v2.3.0/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v3 v3.0.0-20190709130402-674ba3eaed22 h1:0efs3hwEZhFKsCoP8l6dDB1AZWMgnEl3yWXWRZTOaEA=
 gopkg.in/yaml.v3 v3.0.0-20190709130402-674ba3eaed22/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 gopkg.in/yaml.v3 v3.0.0-20191026110619-0b21df46bc1d h1:LCPbGQ34PMrwad11aMZ+dbz5SAsq/0ySjRwQ8I9Qwd8=
--- a/pkg/app/desired_state_file_loader.go
+++ b/pkg/app/desired_state_file_loader.go
@ -216,7 +216,7 @@ func (ld *desiredStateLoader) renderAndLoad(env, overrodeEnv *environment.Enviro
 		if finalState == nil {
 			finalState = currentState
 		} else {
-			if err := mergo.Merge(finalState, currentState, mergo.WithOverride); err != nil {
+			if err := mergo.Merge(&finalState.ReleaseSetSpec, &currentState.ReleaseSetSpec, mergo.WithOverride); err != nil {
 				return nil, err
 			}
 		}
--- a/pkg/argparser/args_test.go
+++ b/pkg/argparser/args_test.go
@ -10,7 +10,11 @@ func TestGetArgs(t *testing.T) {
 	args := "--timeout=3600 --set app1.bootstrap=true --set app2.bootstrap=false --tiller-namespace ns"
 	defaultArgs := []string{"--recreate-pods", "--force"}
 	Helmdefaults := state.HelmSpec{KubeContext: "test", TillerNamespace: "test-namespace", Args: defaultArgs}
-	testState := &state.HelmState{HelmDefaults: Helmdefaults}
+	testState := &state.HelmState{
+		ReleaseSetSpec: state.ReleaseSetSpec{
+			HelmDefaults: Helmdefaults,
+		},
+	}
 	receivedArgs := GetArgs(args, testState)

 	expectedOutput := "--timeout=3600 --set app1.bootstrap=true --set app2.bootstrap=false --tiller-namespace ns --recreate-pods --force"
@ -24,7 +28,11 @@ func Test2(t *testing.T) {
 	args := "--timeout=3600 --set app1.bootstrap=true --set app2.bootstrap=false,app3.bootstrap=true --tiller-namespace ns"
 	defaultArgs := []string{"--recreate-pods", "--force"}
 	Helmdefaults := state.HelmSpec{KubeContext: "test", TillerNamespace: "test-namespace", Args: defaultArgs}
-	testState := &state.HelmState{HelmDefaults: Helmdefaults}
+	testState := &state.HelmState{
+		ReleaseSetSpec: state.ReleaseSetSpec{
+			HelmDefaults: Helmdefaults,
+		},
+	}
 	receivedArgs := GetArgs(args, testState)

 	expectedOutput := "--timeout=3600 --set app1.bootstrap=true --set app2.bootstrap=false,app3.bootstrap=true --tiller-namespace ns --recreate-pods --force"
--- a/pkg/state/state.go
+++ b/pkg/state/state.go
@ -40,11 +40,7 @@ const (
 	EmptyTimeout = -1
 )

-// HelmState structure for the helmfile
-type HelmState struct {
-	basePath string
-	FilePath string
-
+type ReleaseSetSpec struct {
 	DefaultHelmBinary string `yaml:"helmBinary,omitempty"`

 	// DefaultValues is the default values to be overrode by environment values and command-line overrides
@ -71,6 +67,19 @@ type HelmState struct {

 	Env environment.Environment `yaml:"-"`

+	// If set to "Error", return an error when a subhelmfile points to a
+	// non-existent path. The default behavior is to print a warning. Note the
+	// differing default compared to other MissingFileHandlers.
+	MissingFileHandler string `yaml:"missingFileHandler"`
+}
+
+// HelmState structure for the helmfile
+type HelmState struct {
+	basePath string
+	FilePath string
+
+	ReleaseSetSpec `yaml:",inline"`
+
 	logger *zap.SugaredLogger

 	readFile func(string) ([]byte, error)
@ -82,11 +91,6 @@ type HelmState struct {

 	runner      helmexec.Runner
 	valsRuntime vals.Evaluator
-
-	// If set to "Error", return an error when a subhelmfile points to a
-	// non-existent path. The default behavior is to print a warning. Note the
-	// differing default compared to other MissingFileHandlers.
-	MissingFileHandler string `yaml:"missingFileHandler"`
 }

 // SubHelmfileSpec defines the subhelmfile path and options
--- a/pkg/state/state_exec_tmpl_test.go
+++ b/pkg/state/state_exec_tmpl_test.go
@ -135,14 +135,16 @@ func TestHelmState_executeTemplates(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			state := &HelmState{
 				basePath: ".",
-				HelmDefaults: HelmSpec{
-					KubeContext: "test_context",
-				},
-				Env:               environment.Environment{Name: "test_env"},
-				OverrideNamespace: "test-namespace_",
-				Repositories:      nil,
-				Releases: []ReleaseSpec{
-					tt.input,
+				ReleaseSetSpec: ReleaseSetSpec{
+					HelmDefaults: HelmSpec{
+						KubeContext: "test_context",
+					},
+					Env:               environment.Environment{Name: "test_env"},
+					OverrideNamespace: "test-namespace_",
+					Repositories:      nil,
+					Releases: []ReleaseSpec{
+						tt.input,
+					},
 				},
 			}

@ -235,14 +237,16 @@ func TestHelmState_recursiveRefsTemplates(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			state := &HelmState{
 				basePath: ".",
-				HelmDefaults: HelmSpec{
-					KubeContext: "test_context",
-				},
-				Env:               environment.Environment{Name: "test_env"},
-				OverrideNamespace: "test-namespace_",
-				Repositories:      nil,
-				Releases: []ReleaseSpec{
-					tt.input,
+				ReleaseSetSpec: ReleaseSetSpec{
+					HelmDefaults: HelmSpec{
+						KubeContext: "test_context",
+					},
+					Env:               environment.Environment{Name: "test_env"},
+					OverrideNamespace: "test-namespace_",
+					Repositories:      nil,
+					Releases: []ReleaseSpec{
+						tt.input,
+					},
 				},
 			}

--- a/pkg/state/state_test.go
+++ b/pkg/state/state_test.go
@ -127,12 +127,14 @@ func TestHelmState_applyDefaultsTo(t *testing.T) {
 		tt := tests[i]
 		t.Run(tt.name, func(t *testing.T) {
 			state := &HelmState{
-				basePath:           tt.fields.BaseChartPath,
-				DeprecatedContext:  tt.fields.Context,
-				DeprecatedReleases: tt.fields.DeprecatedReleases,
-				OverrideNamespace:  tt.fields.Namespace,
-				Repositories:       tt.fields.Repositories,
-				Releases:           tt.fields.Releases,
+				basePath: tt.fields.BaseChartPath,
+				ReleaseSetSpec: ReleaseSetSpec{
+					DeprecatedContext:  tt.fields.Context,
+					DeprecatedReleases: tt.fields.DeprecatedReleases,
+					OverrideNamespace:  tt.fields.Namespace,
+					Repositories:       tt.fields.Repositories,
+					Releases:           tt.fields.Releases,
+				},
 			}
 			if state.ApplyOverrides(&tt.args.spec); !reflect.DeepEqual(tt.args.spec, tt.want) {
 				t.Errorf("HelmState.ApplyOverrides() = %v, want %v", tt.args.spec, tt.want)
@ -695,11 +697,13 @@ func TestHelmState_flagsForUpgrade(t *testing.T) {
 		tt := tests[i]
 		t.Run(tt.name, func(t *testing.T) {
 			state := &HelmState{
-				basePath:          "./",
-				DeprecatedContext: "default",
-				Releases:          []ReleaseSpec{*tt.release},
-				HelmDefaults:      tt.defaults,
-				valsRuntime:       valsRuntime,
+				basePath: "./",
+				ReleaseSetSpec: ReleaseSetSpec{
+					DeprecatedContext: "default",
+					Releases:          []ReleaseSpec{*tt.release},
+					HelmDefaults:      tt.defaults,
+				},
+				valsRuntime: valsRuntime,
 			}
 			helm := &exectest.Helm{
 				Version: tt.version,
@ -921,7 +925,9 @@ func TestHelmState_SyncRepos(t *testing.T) {
 				}
 			}
 			state := &HelmState{
-				Repositories: tt.repos,
+				ReleaseSetSpec: ReleaseSetSpec{
+					Repositories: tt.repos,
+				},
 			}
 			if _, _ = state.SyncRepos(tt.helm, map[string]bool{}); !reflect.DeepEqual(tt.helm.Repo, tt.want) {
 				t.Errorf("HelmState.SyncRepos() for [%s] = %v, want %v", tt.name, tt.helm.Repo, tt.want)
@ -1032,7 +1038,9 @@ func TestHelmState_SyncReleases(t *testing.T) {
 		tt := tests[i]
 		t.Run(tt.name, func(t *testing.T) {
 			state := &HelmState{
-				Releases:    tt.releases,
+				ReleaseSetSpec: ReleaseSetSpec{
+					Releases: tt.releases,
+				},
 				logger:      logger,
 				valsRuntime: valsRuntime,
 			}
@ -1135,8 +1143,10 @@ func TestHelmState_SyncReleases_MissingValuesFileForUndesiredRelease(t *testing.
 		tt := tests[i]
 		t.Run(tt.name, func(t *testing.T) {
 			state := &HelmState{
-				basePath:    ".",
-				Releases:    []ReleaseSpec{tt.release},
+				basePath: ".",
+				ReleaseSetSpec: ReleaseSetSpec{
+					Releases: []ReleaseSpec{tt.release},
+				},
 				logger:      logger,
 				valsRuntime: valsRuntime,
 			}
@ -1281,7 +1291,9 @@ func TestHelmState_SyncReleasesAffectedRealeases(t *testing.T) {
 		tt := tests[i]
 		t.Run(tt.name, func(t *testing.T) {
 			state := &HelmState{
-				Releases:    tt.releases,
+				ReleaseSetSpec: ReleaseSetSpec{
+					Releases: tt.releases,
+				},
 				logger:      logger,
 				valsRuntime: valsRuntime,
 			}
@ -1383,7 +1395,9 @@ func TestGetDeployedVersion(t *testing.T) {
 		tt := tests[i]
 		t.Run(tt.name, func(t *testing.T) {
 			state := &HelmState{
-				Releases:    []ReleaseSpec{tt.release},
+				ReleaseSetSpec: ReleaseSetSpec{
+					Releases: []ReleaseSpec{tt.release},
+				},
 				logger:      logger,
 				valsRuntime: valsRuntime,
 			}
@ -1510,7 +1524,9 @@ func TestHelmState_DiffReleases(t *testing.T) {
 		tt := tests[i]
 		t.Run(tt.name, func(t *testing.T) {
 			state := &HelmState{
-				Releases:    tt.releases,
+				ReleaseSetSpec: ReleaseSetSpec{
+					Releases: tt.releases,
+				},
 				logger:      logger,
 				valsRuntime: valsRuntime,
 			}
@ -1582,7 +1598,9 @@ func TestHelmState_SyncReleasesCleanup(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			numRemovedFiles := 0
 			state := &HelmState{
-				Releases:    tt.releases,
+				ReleaseSetSpec: ReleaseSetSpec{
+					Releases: tt.releases,
+				},
 				logger:      logger,
 				valsRuntime: valsRuntime,
 				removeFile: func(f string) error {
@ -1666,7 +1684,9 @@ func TestHelmState_DiffReleasesCleanup(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			numRemovedFiles := 0
 			state := &HelmState{
-				Releases:    tt.releases,
+				ReleaseSetSpec: ReleaseSetSpec{
+					Releases: tt.releases,
+				},
 				logger:      logger,
 				valsRuntime: valsRuntime,
 				removeFile: func(f string) error {
@ -1728,35 +1748,37 @@ generated: 2019-05-16T15:42:45.50486+09:00
 	state := &HelmState{
 		basePath: "/src",
 		FilePath: "/src/helmfile.yaml",
-		Releases: []ReleaseSpec{
-			{
-				Chart: "./..",
+		ReleaseSetSpec: ReleaseSetSpec{
+			Releases: []ReleaseSpec{
+				{
+					Chart: "./..",
+				},
+				{
+					Chart: "../examples",
+				},
+				{
+					Chart: "../../helmfile",
+				},
+				{
+					Chart: "published",
+				},
+				{
+					Chart: "published/deeper",
+				},
+				{
+					Chart:   "stable/envoy",
+					Version: "1.5.0",
+				},
+				{
+					Chart:   "stable/envoy",
+					Version: "1.4.0",
+				},
 			},
-			{
-				Chart: "../examples",
-			},
-			{
-				Chart: "../../helmfile",
-			},
-			{
-				Chart: "published",
-			},
-			{
-				Chart: "published/deeper",
-			},
-			{
-				Chart:   "stable/envoy",
-				Version: "1.5.0",
-			},
-			{
-				Chart:   "stable/envoy",
-				Version: "1.4.0",
-			},
-		},
-		Repositories: []RepositorySpec{
-			{
-				Name: "stable",
-				URL:  "https://kubernetes-charts.storage.googleapis.com",
+			Repositories: []RepositorySpec{
+				{
+					Name: "stable",
+					URL:  "https://kubernetes-charts.storage.googleapis.com",
+				},
 			},
 		},
 		tempDir: tempDir,
@ -1790,30 +1812,32 @@ func TestHelmState_ResolveDeps_NoLockFile(t *testing.T) {
 	state := &HelmState{
 		basePath: "/src",
 		FilePath: "/src/helmfile.yaml",
-		Releases: []ReleaseSpec{
-			{
-				Chart: "./..",
+		ReleaseSetSpec: ReleaseSetSpec{
+			Releases: []ReleaseSpec{
+				{
+					Chart: "./..",
+				},
+				{
+					Chart: "../examples",
+				},
+				{
+					Chart: "../../helmfile",
+				},
+				{
+					Chart: "published",
+				},
+				{
+					Chart: "published/deeper",
+				},
+				{
+					Chart: "stable/envoy",
+				},
 			},
-			{
-				Chart: "../examples",
-			},
-			{
-				Chart: "../../helmfile",
-			},
-			{
-				Chart: "published",
-			},
-			{
-				Chart: "published/deeper",
-			},
-			{
-				Chart: "stable/envoy",
-			},
-		},
-		Repositories: []RepositorySpec{
-			{
-				Name: "stable",
-				URL:  "https://kubernetes-charts.storage.googleapis.com",
+			Repositories: []RepositorySpec{
+				{
+					Name: "stable",
+					URL:  "https://kubernetes-charts.storage.googleapis.com",
+				},
 			},
 		},
 		logger: logger,
@ -1906,8 +1930,10 @@ func TestHelmState_ReleaseStatuses(t *testing.T) {
 		tt := tests[i]
 		f := func(t *testing.T) {
 			state := &HelmState{
-				Releases: tt.releases,
-				logger:   logger,
+				ReleaseSetSpec: ReleaseSetSpec{
+					Releases: tt.releases,
+				},
+				logger: logger,
 				fileExists: func(f string) (bool, error) {
 					if f != "foo.yaml" {
 						return false, fmt.Errorf("unexpected file: %s", f)
@ -1991,8 +2017,10 @@ func TestHelmState_TestReleasesNoCleanUp(t *testing.T) {
 		tt := tests[i]
 		f := func(t *testing.T) {
 			state := &HelmState{
-				Releases: tt.releases,
-				logger:   logger,
+				ReleaseSetSpec: ReleaseSetSpec{
+					Releases: tt.releases,
+				},
+				logger: logger,
 			}
 			errs := state.TestReleases(tt.helm, tt.cleanup, 1, 1)
 			if (errs != nil) != tt.wantErr {
@ -2042,8 +2070,10 @@ func TestHelmState_NoReleaseMatched(t *testing.T) {
 		tt := tests[i]
 		f := func(t *testing.T) {
 			state := &HelmState{
-				Releases: releases,
-				logger:   logger,
+				ReleaseSetSpec: ReleaseSetSpec{
+					Releases: releases,
+				},
+				logger: logger,
 			}
 			state.Selectors = []string{tt.labels}
 			errs := state.FilterReleases()
@ -2208,11 +2238,13 @@ func TestHelmState_Delete(t *testing.T) {
 				release,
 			}
 			state := &HelmState{
-				HelmDefaults: HelmSpec{
-					KubeContext: tt.defKubeContext,
+				ReleaseSetSpec: ReleaseSetSpec{
+					HelmDefaults: HelmSpec{
+						KubeContext: tt.defKubeContext,
+					},
+					Releases: releases,
 				},
-				Releases: releases,
-				logger:   logger,
+				logger: logger,
 			}
 			helm := &exectest.Helm{
 				Lists:   map[exectest.ListKey]string{},
--- a/pkg/tmpl/sprig_functions_test.go
+++ b/pkg/tmpl/sprig_functions_test.go
@ -0,0 +1,23 @@
+package tmpl
+
+import (
+	"testing"
+)
+
+func TestMergeOverwrite(t *testing.T) {
+	ctx := &Context{}
+	buf, err := ctx.RenderTemplateToBuffer(`
+		{{- $v1 := dict "bool" true  "int" 2 "str" "v1" "str2" "v1" -}}
+		{{- $v2 := dict "bool" false "int" 0 "str" "v2" "str2" "" -}}
+		{{- $mo1 := mergeOverwrite (dict) $v1 $v2 }}
+		{{- $mo1 -}}
+	`)
+	expected := "map[bool:false int:0 str:v2 str2:]"
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	actual := buf.String()
+	if actual != expected {
+		t.Errorf("unexpected result: expected=%v, actual=%v", expected, actual)
+	}
+}