Fix concurrent-map-iteration-and-write errors while running release hooks (#1534)

Fixes #1495

pkg/app/desired_state_file_loader.go

@@ -219,6 +219,8 @@ func (ld *desiredStateLoader) renderAndLoad(env, overrodeEnv *environment.Enviro
 			if err := mergo.Merge(&finalState.ReleaseSetSpec, &currentState.ReleaseSetSpec, mergo.WithOverride); err != nil {
 				return nil, err
 			}
+
+			finalState.RenderedValues = currentState.RenderedValues
 		}
 
 		env = &finalState.Env

pkg/state/create.go

@@ -145,11 +145,15 @@ func (c *StateCreator) LoadEnvValues(target *HelmState, env string, ctxEnv *envi
 		return nil, &StateLoadError{fmt.Sprintf("failed to read %s", state.FilePath), err}
 	}
 
-	e.Defaults, err = state.loadValuesEntries(nil, state.DefaultValues, c.remote)
+	newDefaults, err := state.loadValuesEntries(nil, state.DefaultValues, c.remote)
 	if err != nil {
 		return nil, err
 	}
 
+	if err := mergo.Merge(&e.Defaults, newDefaults, mergo.WithOverride, mergo.WithOverwriteWithEmptyValue); err != nil {
+		return nil, err
+	}
+
 	state.Env = *e
 
 	return &state, nil
@@ -181,6 +185,12 @@ func (c *StateCreator) ParseAndLoad(content []byte, baseDir, file string, envNam
 
 	state.FilePath = file
 
+	vals, err := state.Env.GetMergedValues()
+	if err != nil {
+		return nil, fmt.Errorf("rendering values: %w", err)
+	}
+	state.RenderedValues = vals
+
 	return state, nil
 }
 

pkg/state/create_test.go

@@ -63,6 +63,35 @@ func TestReadFromYaml_NonexistentEnv(t *testing.T) {
 	}
 }
 
+type stateTestEnv struct {
+	Files   map[string]string
+	WorkDir string
+}
+
+func (testEnv stateTestEnv) MustLoadState(t *testing.T, file, envName string) *HelmState {
+	t.Helper()
+
+	testFs := testhelper.NewTestFs(testEnv.Files)
+
+	if testFs.Cwd == "" {
+		testFs.Cwd = "/"
+	}
+
+	yamlContent, ok := testEnv.Files[file]
+	if !ok {
+		t.Fatalf("no file named %q registered", file)
+	}
+
+	r := remote.NewRemote(logger, testFs.Cwd, testFs.ReadFile, testFs.DirectoryExistsAt, testFs.FileExistsAt)
+	state, err := NewCreator(logger, testFs.ReadFile, testFs.FileExists, testFs.Abs, testFs.Glob, nil, nil, "", r).
+		ParseAndLoad([]byte(yamlContent), filepath.Dir(file), file, envName, true, nil)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	return state
+}
+
 func TestReadFromYaml_NonDefaultEnv(t *testing.T) {
 	yamlFile := "/example/path/to/helmfile.yaml"
 	yamlContent := []byte(`environments:

pkg/state/selector_test.go

@@ -2,7 +2,6 @@ package state
 
 import (
 	"github.com/google/go-cmp/cmp"
-	"gopkg.in/yaml.v2"
 	"testing"
 )
 
@@ -70,11 +69,12 @@ func TestSelectReleasesWithOverrides(t *testing.T) {
     type: bar
 `)
 
-	var state HelmState
+	state := stateTestEnv{
-
+		Files: map[string]string{
-	if err := yaml.Unmarshal(example, &state); err != nil {
+			"/helmfile.yaml": string(example),
-		t.Fatal(err)
+		},
-	}
+		WorkDir: "/",
+	}.MustLoadState(t, "/helmfile.yaml", "default")
 
 	for _, tc := range testcases {
 		state.Selectors = tc.selector

pkg/state/state.go

@@ -91,6 +91,11 @@ type HelmState struct {
 
 	runner      helmexec.Runner
 	valsRuntime vals.Evaluator
+
+	// RenderedValues is the helmfile-wide values that is `.Values`
+	// which is accessible from within the whole helmfile go template.
+	// Note that this is usually computed by DesiredStateLoader from ReleaseSetSpec.Env
+	RenderedValues map[string]interface{}
 }
 
 // SubHelmfileSpec defines the subhelmfile path and options
@@ -1683,10 +1688,7 @@ func (st *HelmState) GetReleasesWithOverrides() []ReleaseSpec {
 }
 
 func (st *HelmState) SelectReleasesWithOverrides() ([]Release, error) {
-	values, err := st.Values()
+	values := st.Values()
-	if err != nil {
-		return nil, err
-	}
 	rs, err := markExcludedReleases(st.GetReleasesWithOverrides(), st.Selectors, st.CommonLabels, values)
 	if err != nil {
 		return nil, err
@@ -1823,10 +1825,7 @@ func (st *HelmState) triggerReleaseEvent(evt string, evtErr error, r *ReleaseSpe
 		Logger:        st.logger,
 		ReadFile:      st.readFile,
 	}
-	vals, err := st.Values()
+	vals := st.Values()
-	if err != nil {
-		return false, err
-	}
 	data := map[string]interface{}{
 		"Values":          vals,
 		"Release":         r,
@@ -2147,10 +2146,7 @@ func (st *HelmState) flagsForLint(helm helmexec.Interface, release *ReleaseSpec,
 }
 
 func (st *HelmState) RenderReleaseValuesFileToBytes(release *ReleaseSpec, path string) ([]byte, error) {
-	vals, err := st.Values()
+	vals := st.Values()
-	if err != nil {
-		return nil, err
-	}
 	templateData := st.createReleaseTemplateData(release, vals)
 
 	r := tmpl.NewFileRenderer(st.readFile, filepath.Dir(path), templateData)

pkg/state/state_exec_tmpl.go

@@ -8,8 +8,12 @@ import (
 	"gopkg.in/yaml.v2"
 )
 
-func (st *HelmState) Values() (map[string]interface{}, error) {
+func (st *HelmState) Values() map[string]interface{} {
-	return st.Env.GetMergedValues()
+	if st.RenderedValues == nil {
+		panic("[bug] RenderedValues is nil")
+	}
+
+	return st.RenderedValues
 }
 
 func (st *HelmState) createReleaseTemplateData(release *ReleaseSpec, vals map[string]interface{}) releaseTemplateData {
@@ -78,10 +82,7 @@ func updateBoolTemplatedValues(r *ReleaseSpec) error {
 func (st *HelmState) ExecuteTemplates() (*HelmState, error) {
 	r := *st
 
-	vals, err := st.Values()
+	vals := st.Values()
-	if err != nil {
-		return nil, err
-	}
 
 	for i, rt := range st.Releases {
 		if rt.Labels == nil {

pkg/state/state_exec_tmpl_test.go

@@ -146,6 +146,7 @@ func TestHelmState_executeTemplates(t *testing.T) {
 						tt.input,
 					},
 				},
+				RenderedValues: map[string]interface{}{},
 			}
 
 			r, err := state.ExecuteTemplates()
@@ -248,6 +249,7 @@ func TestHelmState_recursiveRefsTemplates(t *testing.T) {
 						tt.input,
 					},
 				},
+				RenderedValues: map[string]interface{}{},
 			}
 
 			r, err := state.ExecuteTemplates()

pkg/state/state_test.go

@@ -1030,8 +1030,9 @@ func TestHelmState_SyncReleases(t *testing.T) {
 				ReleaseSetSpec: ReleaseSetSpec{
 					Releases: tt.releases,
 				},
-				logger:      logger,
+				logger:         logger,
-				valsRuntime: valsRuntime,
+				valsRuntime:    valsRuntime,
+				RenderedValues: map[string]interface{}{},
 			}
 			if errs := state.SyncReleases(&AffectedReleases{}, tt.helm, []string{}, 1); errs != nil && len(errs) > 0 {
 				if len(errs) != len(tt.wantErrorMsgs) {
@@ -1136,8 +1137,9 @@ func TestHelmState_SyncReleases_MissingValuesFileForUndesiredRelease(t *testing.
 				ReleaseSetSpec: ReleaseSetSpec{
 					Releases: []ReleaseSpec{tt.release},
 				},
-				logger:      logger,
+				logger:         logger,
-				valsRuntime: valsRuntime,
+				valsRuntime:    valsRuntime,
+				RenderedValues: map[string]interface{}{},
 			}
 			fs := testhelper.NewTestFs(map[string]string{})
 			state = injectFs(state, fs)
@@ -1283,8 +1285,9 @@ func TestHelmState_SyncReleasesAffectedRealeases(t *testing.T) {
 				ReleaseSetSpec: ReleaseSetSpec{
 					Releases: tt.releases,
 				},
-				logger:      logger,
+				logger:         logger,
-				valsRuntime: valsRuntime,
+				valsRuntime:    valsRuntime,
+				RenderedValues: map[string]interface{}{},
 			}
 			helm := &exectest.Helm{
 				Lists: map[exectest.ListKey]string{},
@@ -1387,8 +1390,9 @@ func TestGetDeployedVersion(t *testing.T) {
 				ReleaseSetSpec: ReleaseSetSpec{
 					Releases: []ReleaseSpec{tt.release},
 				},
-				logger:      logger,
+				logger:         logger,
-				valsRuntime: valsRuntime,
+				valsRuntime:    valsRuntime,
+				RenderedValues: map[string]interface{}{},
 			}
 			helm := &exectest.Helm{
 				Lists: map[exectest.ListKey]string{},
@@ -1516,8 +1520,9 @@ func TestHelmState_DiffReleases(t *testing.T) {
 				ReleaseSetSpec: ReleaseSetSpec{
 					Releases: tt.releases,
 				},
-				logger:      logger,
+				logger:         logger,
-				valsRuntime: valsRuntime,
+				valsRuntime:    valsRuntime,
+				RenderedValues: map[string]interface{}{},
 			}
 			_, errs := state.DiffReleases(tt.helm, []string{}, 1, false, false, false, false, false)
 			if errs != nil && len(errs) > 0 {
@@ -1596,6 +1601,7 @@ func TestHelmState_SyncReleasesCleanup(t *testing.T) {
 					numRemovedFiles += 1
 					return nil
 				},
+				RenderedValues: map[string]interface{}{},
 			}
 			testfs := testhelper.NewTestFs(map[string]string{
 				"/path/to/someFile": `foo: FOO`,
@@ -1682,6 +1688,7 @@ func TestHelmState_DiffReleasesCleanup(t *testing.T) {
 					numRemovedFiles += 1
 					return nil
 				},
+				RenderedValues: map[string]interface{}{},
 			}
 			testfs := testhelper.NewTestFs(map[string]string{
 				"/path/to/someFile": `foo: bar
@@ -2062,7 +2069,8 @@ func TestHelmState_NoReleaseMatched(t *testing.T) {
 				ReleaseSetSpec: ReleaseSetSpec{
 					Releases: releases,
 				},
-				logger: logger,
+				logger:         logger,
+				RenderedValues: map[string]interface{}{},
 			}
 			state.Selectors = []string{tt.labels}
 			errs := state.FilterReleases()
@@ -2233,7 +2241,8 @@ func TestHelmState_Delete(t *testing.T) {
 					},
 					Releases: releases,
 				},
-				logger: logger,
+				logger:         logger,
+				RenderedValues: map[string]interface{}{},
 			}
 			helm := &exectest.Helm{
 				Lists:   map[exectest.ListKey]string{},