v1: Fix --state-values-set to override values of environments colocated with releases (#705)

pkg/app/desired_state_file_loader.go

@@ -100,7 +100,7 @@ func (ld *desiredStateLoader) Load(f string, opts LoadOpts) (*state.HelmState, e
 	return st, nil
 }
 
-func (ld *desiredStateLoader) loadFile(inheritedEnv *environment.Environment, baseDir, file string, evaluateBases bool) (*state.HelmState, error) {
+func (ld *desiredStateLoader) loadFile(inheritedEnv, overrodeEnv *environment.Environment, baseDir, file string, evaluateBases bool) (*state.HelmState, error) {
 	path, err := ld.remote.Locate(file)
 	if err != nil {
 		return nil, fmt.Errorf("locate: %v", err)
@@ -109,7 +109,7 @@ func (ld *desiredStateLoader) loadFile(inheritedEnv *environment.Environment, ba
 		ld.logger.Debugf("fetched remote \"%s\" to local cache \"%s\" and loading the latter...", file, path)
 	}
 	file = path
-	return ld.loadFileWithOverrides(inheritedEnv, nil, baseDir, file, evaluateBases)
+	return ld.loadFileWithOverrides(inheritedEnv, overrodeEnv, baseDir, file, evaluateBases)
 }
 
 func (ld *desiredStateLoader) loadFileWithOverrides(inheritedEnv, overrodeEnv *environment.Environment, baseDir, file string, evaluateBases bool) (*state.HelmState, error) {
@@ -157,16 +157,24 @@ func (a *desiredStateLoader) underlying() *state.StateCreator {
 }
 
 func (a *desiredStateLoader) rawLoad(yaml []byte, baseDir, file string, evaluateBases bool, env, overrodeEnv *environment.Environment) (*state.HelmState, error) {
-	merged, err := env.Merge(overrodeEnv)
-	if err != nil {
-		return nil, err
-	}
+	var st *state.HelmState
+	var err error
+	if runtime.V1Mode {
+		st, err = a.underlying().ParseAndLoad(yaml, baseDir, file, a.env, evaluateBases, env, overrodeEnv)
+		if err != nil {
+			return nil, err
+		}
+	} else {
+		merged, err := env.Merge(overrodeEnv)
+		if err != nil {
+			return nil, err
+		}
 
-	st, err := a.underlying().ParseAndLoad(yaml, baseDir, file, a.env, evaluateBases, merged)
-	if err != nil {
-		return nil, err
+		st, err = a.underlying().ParseAndLoad(yaml, baseDir, file, a.env, evaluateBases, merged, nil)
+		if err != nil {
+			return nil, err
+		}
 	}
-
 	helmfiles, err := st.ExpandedHelmfiles()
 	if err != nil {
 		return nil, err

pkg/app/two_pass_renderer.go

@@ -22,8 +22,12 @@ func prependLineNumbers(text string) string {
 	return buf.String()
 }
 
-func (r *desiredStateLoader) renderPrestate(firstPassEnv *environment.Environment, baseDir, filename string, content []byte) (*environment.Environment, *state.HelmState) {
-	tmplData := state.NewEnvironmentTemplateData(*firstPassEnv, r.namespace, map[string]interface{}{})
+func (r *desiredStateLoader) renderPrestate(firstPassEnv, overrode *environment.Environment, baseDir, filename string, content []byte) (*environment.Environment, *state.HelmState) {
+	initEnv, err := firstPassEnv.Merge(overrode)
+	if err != nil {
+		return firstPassEnv, nil
+	}
+	tmplData := state.NewEnvironmentTemplateData(*initEnv, r.namespace, map[string]interface{}{})
 	firstPassRenderer := tmpl.NewFirstPassRenderer(baseDir, tmplData)
 
 	// parse as much as we can, tolerate errors, this is a preparse
@@ -49,7 +53,7 @@ func (r *desiredStateLoader) renderPrestate(firstPassEnv *environment.Environmen
 	c := r.underlying()
 	c.Strict = false
 	// create preliminary state, as we may have an environment. Tolerate errors.
-	prestate, err := c.ParseAndLoad([]byte(sanitized), baseDir, filename, r.env, false, firstPassEnv)
+	prestate, err := c.ParseAndLoad([]byte(sanitized), baseDir, filename, r.env, false, firstPassEnv, overrode)
 	if err != nil {
 		if _, ok := err.(*state.StateLoadError); ok {
 			r.logger.Debugf("could not deduce `environment:` block, configuring only .Environment.Name. error: %v", err)
@@ -85,7 +89,7 @@ func (r *desiredStateLoader) twoPassRenderTemplateToYaml(inherited, overrode *en
 	}
 	r.logger.Debugf("%srendering starting for \"%s\": inherited=%v, overrode=%v", phase, filename, inherited, overrode)
 
-	initEnv, err := inherited.Merge(overrode)
+	initEnv, err := inherited.Merge(nil)
 	if err != nil {
 		return nil, err
 	}
@@ -99,7 +103,10 @@ func (r *desiredStateLoader) twoPassRenderTemplateToYaml(inherited, overrode *en
 	if runtime.V1Mode {
 		var err error
 
-		finalEnv = initEnv
+		finalEnv, err = initEnv.Merge(overrode)
+		if err != nil {
+			return nil, err
+		}
 
 		vals, err = finalEnv.GetMergedValues()
 		if err != nil {
@@ -107,8 +114,11 @@ func (r *desiredStateLoader) twoPassRenderTemplateToYaml(inherited, overrode *en
 		}
 	} else {
 		r.logger.Debugf("first-pass uses: %v", initEnv)
-
-		renderedEnv, prestate := r.renderPrestate(initEnv, baseDir, filename, content)
+		firstPassEnv, err := initEnv.Merge(nil)
+		if err != nil {
+			return nil, err
+		}
+		renderedEnv, prestate := r.renderPrestate(firstPassEnv, overrode, baseDir, filename, content)
 
 		r.logger.Debugf("first-pass produced: %v", renderedEnv)
 

pkg/state/create.go

@@ -47,7 +47,7 @@ type StateCreator struct {
 
 	Strict bool
 
-	LoadFile func(inheritedEnv *environment.Environment, baseDir, file string, evaluateBases bool) (*HelmState, error)
+	LoadFile func(inheritedEnv, overrodeEnv *environment.Environment, baseDir, file string, evaluateBases bool) (*HelmState, error)
 
 	getHelm func(*HelmState) helmexec.Interface
 
@@ -138,10 +138,10 @@ func (c *StateCreator) Parse(content []byte, baseDir, file string) (*HelmState,
 }
 
 // LoadEnvValues loads environment values files relative to the `baseDir`
-func (c *StateCreator) LoadEnvValues(target *HelmState, env string, ctxEnv *environment.Environment, failOnMissingEnv bool) (*HelmState, error) {
+func (c *StateCreator) LoadEnvValues(target *HelmState, env string, ctxEnv, overrode *environment.Environment, failOnMissingEnv bool) (*HelmState, error) {
 	state := *target
 
-	e, err := c.loadEnvValues(&state, env, failOnMissingEnv, ctxEnv)
+	e, err := c.loadEnvValues(&state, env, failOnMissingEnv, ctxEnv, overrode)
 	if err != nil {
 		return nil, &StateLoadError{fmt.Sprintf("failed to read %s", state.FilePath), err}
 	}
@@ -162,7 +162,7 @@ func (c *StateCreator) LoadEnvValues(target *HelmState, env string, ctxEnv *envi
 
 // Parses YAML into HelmState, while loading environment values files relative to the `baseDir`
 // evaluateBases=true means that this is NOT a base helmfile
-func (c *StateCreator) ParseAndLoad(content []byte, baseDir, file string, envName string, evaluateBases bool, envValues *environment.Environment) (*HelmState, error) {
+func (c *StateCreator) ParseAndLoad(content []byte, baseDir, file string, envName string, evaluateBases bool, envValues, overrode *environment.Environment) (*HelmState, error) {
 	state, err := c.Parse(content, baseDir, file)
 	if err != nil {
 		return nil, err
@@ -173,13 +173,13 @@ func (c *StateCreator) ParseAndLoad(content []byte, baseDir, file string, envNam
 			return nil, errors.New("nested `base` helmfile is unsupported. please submit a feature request if you need this!")
 		}
 	} else {
-		state, err = c.loadBases(envValues, state, baseDir)
+		state, err = c.loadBases(envValues, overrode, state, baseDir)
 		if err != nil {
 			return nil, err
 		}
 	}
 
-	state, err = c.LoadEnvValues(state, envName, envValues, evaluateBases)
+	state, err = c.LoadEnvValues(state, envName, envValues, overrode, evaluateBases)
 	if err != nil {
 		return nil, err
 	}
@@ -195,10 +195,10 @@ func (c *StateCreator) ParseAndLoad(content []byte, baseDir, file string, envNam
 	return state, nil
 }
 
-func (c *StateCreator) loadBases(envValues *environment.Environment, st *HelmState, baseDir string) (*HelmState, error) {
+func (c *StateCreator) loadBases(envValues, overrodeEnv *environment.Environment, st *HelmState, baseDir string) (*HelmState, error) {
 	layers := []*HelmState{}
 	for _, b := range st.Bases {
-		base, err := c.LoadFile(envValues, baseDir, b, false)
+		base, err := c.LoadFile(envValues, overrodeEnv, baseDir, b, false)
 		if err != nil {
 			return nil, err
 		}
@@ -216,7 +216,7 @@ func (c *StateCreator) loadBases(envValues *environment.Environment, st *HelmSta
 }
 
 // nolint: unparam
-func (c *StateCreator) loadEnvValues(st *HelmState, name string, failOnMissingEnv bool, ctxEnv *environment.Environment) (*environment.Environment, error) {
+func (c *StateCreator) loadEnvValues(st *HelmState, name string, failOnMissingEnv bool, ctxEnv, overrode *environment.Environment) (*environment.Environment, error) {
 	envVals := map[string]interface{}{}
 	envSpec, ok := st.Environments[name]
 	if ok {
@@ -250,13 +250,23 @@ func (c *StateCreator) loadEnvValues(st *HelmState, name string, failOnMissingEn
 	newEnv := &environment.Environment{Name: name, Values: envVals}
 
 	if ctxEnv != nil {
-		intEnv := *ctxEnv
+		intCtxEnv := *ctxEnv
 
-		if err := mergo.Merge(&intEnv, newEnv, mergo.WithOverride, mergo.WithOverwriteWithEmptyValue); err != nil {
+		if err := mergo.Merge(&intCtxEnv, newEnv, mergo.WithOverride, mergo.WithOverwriteWithEmptyValue); err != nil {
 			return nil, fmt.Errorf("error while merging environment values for \"%s\": %v", name, err)
 		}
 
-		newEnv = &intEnv
+		newEnv = &intCtxEnv
+	}
+
+	if overrode != nil {
+		intOverrodeEnv := *newEnv
+
+		if err := mergo.Merge(&intOverrodeEnv, overrode, mergo.WithOverride, mergo.WithOverwriteWithEmptyValue); err != nil {
+			return nil, fmt.Errorf("error while merging environment overrode values for \"%s\": %v", name, err)
+		}
+
+		newEnv = &intOverrodeEnv
 	}
 
 	return newEnv, nil

pkg/state/create_test.go

@@ -20,7 +20,7 @@ func createFromYaml(content []byte, file string, env string, logger *zap.Sugared
 		fs:     filesystem.DefaultFileSystem(),
 		Strict: true,
 	}
-	return c.ParseAndLoad(content, filepath.Dir(file), file, env, true, nil)
+	return c.ParseAndLoad(content, filepath.Dir(file), file, env, true, nil, nil)
 }
 
 func TestReadFromYaml(t *testing.T) {
@@ -84,7 +84,7 @@ func (testEnv stateTestEnv) MustLoadStateWithEnableLiveOutput(t *testing.T, file
 
 	r := remote.NewRemote(logger, testFs.Cwd, testFs.ToFileSystem())
 	state, err := NewCreator(logger, testFs.ToFileSystem(), nil, nil, "", r, enableLiveOutput, "").
-		ParseAndLoad([]byte(yamlContent), filepath.Dir(file), file, envName, true, nil)
+		ParseAndLoad([]byte(yamlContent), filepath.Dir(file), file, envName, true, nil, nil)
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
@@ -154,7 +154,7 @@ releaseNamespace: mynamespace
 		Name: "production",
 	}
 	state, err := NewCreator(logger, testFs.ToFileSystem(), nil, nil, "", r, false, "").
-		ParseAndLoad(yamlContent, filepath.Dir(yamlFile), yamlFile, "production", true, &env)
+		ParseAndLoad(yamlContent, filepath.Dir(yamlFile), yamlFile, "production", true, &env, nil)
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
@@ -241,7 +241,7 @@ overrideNamespace: myns
 
 	r := remote.NewRemote(logger, testFs.Cwd, testFs.ToFileSystem())
 	state, err := NewCreator(logger, testFs.ToFileSystem(), nil, nil, "", r, false, "").
-		ParseAndLoad(yamlContent, filepath.Dir(yamlFile), yamlFile, "production", true, nil)
+		ParseAndLoad(yamlContent, filepath.Dir(yamlFile), yamlFile, "production", true, nil, nil)
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}