feat: specify env values from the parent to the nested state (#622)

* feat: specify env values from the parent to the nested state

Adds the `helmfiles[].environment.values` that accepts a mix of file pathes and inline dictes:

```yaml
helmfiles:
- path: path/to/nested/helmfile.yaml
  environment:
    values:
    - key1: val1
    - values.yaml
```

The values files are loaded in the context of the parent state file. For example, in case the above state file is located at `/path/to/helmfile.yaml`,
`values.yaml` is located at `/path/to/values.yaml` instead of `/path/to/nested/values.yaml`.

Resolves #523

* fix: multiple "bases" declarations yields duplicate releases

Fixes #615

* fix regression in double-rendering with env value overrides

The latest commit broke any state files like the below to NOT pass env value overrides at all:

```
helmfiles:
- path: nested/state.yaml
  environment:
    values:
    - overrides.yaml
```

This fixes the issue.

state/create.go

@@ -4,17 +4,13 @@ import (
 	"bytes"
 	"errors"
 	"fmt"
-	"io"
-	"os"
-	"path/filepath"
-	"sort"
-
 	"github.com/imdario/mergo"
 	"github.com/roboll/helmfile/environment"
 	"github.com/roboll/helmfile/helmexec"
-	"github.com/roboll/helmfile/tmpl"
 	"go.uber.org/zap"
 	"gopkg.in/yaml.v2"
+	"io"
+	"os"
 )
 
 type StateLoadError struct {
@@ -164,68 +160,19 @@ func (c *StateCreator) loadBases(envValues *environment.Environment, st *HelmSta
 	return layers[0], nil
 }
 
-func (st *HelmState) ExpandPaths(globPattern string) ([]string, error) {
-	result := []string{}
-	absPathPattern := st.normalizePath(globPattern)
-	matches, err := st.glob(absPathPattern)
-	if err != nil {
-		return nil, fmt.Errorf("failed processing %s: %v", globPattern, err)
-	}
-
-	sort.Strings(matches)
-
-	result = append(result, matches...)
-	return result, nil
-}
-
 func (st *HelmState) loadEnvValues(name string, ctxEnv *environment.Environment, readFile func(string) ([]byte, error), glob func(string) ([]string, error)) (*environment.Environment, error) {
 	envVals := map[string]interface{}{}
 	envSpec, ok := st.Environments[name]
 	if ok {
-		for _, v := range envSpec.Values {
-			switch typedValue := v.(type) {
-			case string:
-				urlOrPath := typedValue
-				resolved, skipped, err := st.resolveFile(envSpec.MissingFileHandler, "environment values", urlOrPath)
-				if err != nil {
-					return nil, err
-				}
-				if skipped {
-					continue
-				}
-
-				for _, envvalFullPath := range resolved {
-					tmplData := EnvironmentTemplateData{Environment: environment.EmptyEnvironment, Namespace: ""}
-					r := tmpl.NewFileRenderer(readFile, filepath.Dir(envvalFullPath), tmplData)
-					bytes, err := r.RenderToBytes(envvalFullPath)
-					if err != nil {
-						return nil, fmt.Errorf("failed to load environment values file \"%s\": %v", envvalFullPath, err)
-					}
-					m := map[string]interface{}{}
-					if err := yaml.Unmarshal(bytes, &m); err != nil {
-						return nil, fmt.Errorf("failed to load environment values file \"%s\": %v", envvalFullPath, err)
-					}
-					if err := mergo.Merge(&envVals, &m, mergo.WithOverride); err != nil {
-						return nil, fmt.Errorf("failed to load \"%s\": %v", envvalFullPath, err)
-					}
-				}
-			case map[interface{}]interface{}:
-				m := map[string]interface{}{}
-				for k, v := range typedValue {
-					switch typedKey := k.(type) {
-					case string:
-						m[typedKey] = v
-					default:
-						return nil, fmt.Errorf("unexpected type of key in inline environment values %v: expected string, got %T", typedValue, typedKey)
-					}
-				}
-				if err := mergo.Merge(&envVals, &m, mergo.WithOverride); err != nil {
-					return nil, fmt.Errorf("failed to merge %v: %v", typedValue, err)
-				}
-				continue
-			default:
-				return nil, fmt.Errorf("unexpected type of values entry: %T", typedValue)
-			}
+		envValues := append([]interface{}{}, envSpec.Values...)
+		ld := &EnvironmentValuesLoader{
+			storage:  st.storage(),
+			readFile: st.readFile,
+		}
+		var err error
+		envVals, err = ld.LoadEnvironmentValues(envSpec.MissingFileHandler, envValues)
+		if err != nil {
+			return nil, err
 		}
 
 		if len(envSpec.Secrets) > 0 {
@@ -233,7 +180,7 @@ func (st *HelmState) loadEnvValues(name string, ctxEnv *environment.Environment,
 
 			var envSecretFiles []string
 			for _, urlOrPath := range envSpec.Secrets {
-				resolved, skipped, err := st.resolveFile(envSpec.MissingFileHandler, "environment values", urlOrPath)
+				resolved, skipped, err := st.storage().resolveFile(envSpec.MissingFileHandler, "environment values", urlOrPath)
 				if err != nil {
 					return nil, err
 				}
@@ -281,7 +228,7 @@ func (st *HelmState) loadEnvValues(name string, ctxEnv *environment.Environment,
 	if ctxEnv != nil {
 		intEnv := *ctxEnv
 
-		if err := mergo.Merge(&intEnv, newEnv, mergo.WithAppendSlice); err != nil {
+		if err := mergo.Merge(&intEnv, newEnv, mergo.WithOverride); err != nil {
 			return nil, fmt.Errorf("error while merging environment values for \"%s\": %v", name, err)
 		}
 

state/environment_values_loader.go

@@ -0,0 +1,75 @@
+package state
+
+import (
+	"fmt"
+	"github.com/imdario/mergo"
+	"github.com/roboll/helmfile/environment"
+	"github.com/roboll/helmfile/tmpl"
+	"gopkg.in/yaml.v2"
+	"path/filepath"
+)
+
+type EnvironmentValuesLoader struct {
+	storage *Storage
+
+	readFile func(string) ([]byte, error)
+}
+
+func NewEnvironmentValuesLoader(storage *Storage, readFile func(string) ([]byte, error)) *EnvironmentValuesLoader {
+	return &EnvironmentValuesLoader{
+		storage:  storage,
+		readFile: readFile,
+	}
+}
+
+func (ld *EnvironmentValuesLoader) LoadEnvironmentValues(missingFileHandler *string, envValues []interface{}) (map[string]interface{}, error) {
+	envVals := map[string]interface{}{}
+
+	for _, v := range envValues {
+		switch typedValue := v.(type) {
+		case string:
+			urlOrPath := typedValue
+			resolved, skipped, err := ld.storage.resolveFile(missingFileHandler, "environment values", urlOrPath)
+			if err != nil {
+				return nil, err
+			}
+			if skipped {
+				continue
+			}
+
+			for _, envvalFullPath := range resolved {
+				tmplData := EnvironmentTemplateData{Environment: environment.EmptyEnvironment, Namespace: ""}
+				r := tmpl.NewFileRenderer(ld.readFile, filepath.Dir(envvalFullPath), tmplData)
+				bytes, err := r.RenderToBytes(envvalFullPath)
+				if err != nil {
+					return nil, fmt.Errorf("failed to load environment values file \"%s\": %v", envvalFullPath, err)
+				}
+				m := map[string]interface{}{}
+				if err := yaml.Unmarshal(bytes, &m); err != nil {
+					return nil, fmt.Errorf("failed to load environment values file \"%s\": %v", envvalFullPath, err)
+				}
+				if err := mergo.Merge(&envVals, &m, mergo.WithOverride); err != nil {
+					return nil, fmt.Errorf("failed to load \"%s\": %v", envvalFullPath, err)
+				}
+			}
+		case map[interface{}]interface{}:
+			m := map[string]interface{}{}
+			for k, v := range typedValue {
+				switch typedKey := k.(type) {
+				case string:
+					m[typedKey] = v
+				default:
+					return nil, fmt.Errorf("unexpected type of key in inline environment values %v: expected string, got %T", typedValue, typedKey)
+				}
+			}
+			if err := mergo.Merge(&envVals, &m, mergo.WithOverride); err != nil {
+				return nil, fmt.Errorf("failed to merge %v: %v", typedValue, err)
+			}
+			continue
+		default:
+			return nil, fmt.Errorf("unexpected type of values entry: %T", typedValue)
+		}
+	}
+
+	return envVals, nil
+}

state/state.go

@@ -15,8 +15,6 @@ import (
 
 	"regexp"
 
-	"net/url"
-
 	"github.com/roboll/helmfile/environment"
 	"github.com/roboll/helmfile/event"
 	"github.com/roboll/helmfile/tmpl"
@@ -63,6 +61,12 @@ type SubHelmfileSpec struct {
 	Path               string   //path or glob pattern for the sub helmfiles
 	Selectors          []string //chosen selectors for the sub helmfiles
 	SelectorsInherited bool     //do the sub helmfiles inherits from parent selectors
+
+	Environment SubhelmfileEnvironmentSpec
+}
+
+type SubhelmfileEnvironmentSpec struct {
+	OverrideValues []interface{} `yaml:"values"`
 }
 
 // HelmSpec to defines helmDefault values
@@ -1040,21 +1044,6 @@ func (st *HelmState) BuildDeps(helm helmexec.Interface) []error {
 	return nil
 }
 
-// JoinBase returns an absolute path in the form basePath/relative
-func (st *HelmState) JoinBase(relPath string) string {
-	return filepath.Join(st.basePath, relPath)
-}
-
-// normalizes relative path to absolute one
-func (st *HelmState) normalizePath(path string) string {
-	u, _ := url.Parse(path)
-	if u.Scheme != "" || filepath.IsAbs(path) {
-		return path
-	} else {
-		return st.JoinBase(path)
-	}
-}
-
 // normalizeChart allows for the distinction between a file path reference and repository references.
 // - Any single (or double character) followed by a `/` will be considered a local file reference and
 // 	 be constructed relative to the `base path`.
@@ -1253,13 +1242,42 @@ func (st *HelmState) RenderValuesFileToBytes(path string) ([]byte, error) {
 	return r.RenderToBytes(path)
 }
 
+func (st *HelmState) storage() *Storage {
+	return &Storage{
+		FilePath: st.FilePath,
+		basePath: st.basePath,
+		glob:     st.glob,
+		logger:   st.logger,
+	}
+}
+
+func (st *HelmState) ExpandedHelmfiles() ([]SubHelmfileSpec, error) {
+	helmfiles := []SubHelmfileSpec{}
+	for _, hf := range st.Helmfiles {
+		matches, err := st.storage().ExpandPaths(hf.Path)
+		if err != nil {
+			return nil, err
+		}
+		if len(matches) == 0 {
+			return nil, fmt.Errorf("no file matching %s found", hf.Path)
+		}
+		for _, match := range matches {
+			newHelmfile := hf
+			newHelmfile.Path = match
+			helmfiles = append(helmfiles, newHelmfile)
+		}
+	}
+
+	return helmfiles, nil
+}
+
 func (st *HelmState) generateTemporaryValuesFiles(values []interface{}, missingFileHandler *string) ([]string, error) {
 	generatedFiles := []string{}
 
 	for _, value := range values {
 		switch typedValue := value.(type) {
 		case string:
-			paths, skip, err := st.resolveFile(missingFileHandler, "values", typedValue)
+			paths, skip, err := st.storage().resolveFile(missingFileHandler, "values", typedValue)
 			if err != nil {
 				return nil, err
 			}
@@ -1317,7 +1335,7 @@ func (st *HelmState) namespaceAndValuesFlags(helm helmexec.Interface, release *R
 	for _, v := range release.Values {
 		switch typedValue := v.(type) {
 		case string:
-			path := st.normalizePath(release.ValuesPathPrefix + typedValue)
+			path := st.storage().normalizePath(release.ValuesPathPrefix + typedValue)
 			values = append(values, path)
 		default:
 			values = append(values, v)
@@ -1336,7 +1354,7 @@ func (st *HelmState) namespaceAndValuesFlags(helm helmexec.Interface, release *R
 	release.generatedValues = append(release.generatedValues, generatedFiles...)
 
 	for _, value := range release.Secrets {
-		paths, skip, err := st.resolveFile(release.MissingFileHandler, "secrets", release.ValuesPathPrefix+value)
+		paths, skip, err := st.storage().resolveFile(release.MissingFileHandler, "secrets", release.ValuesPathPrefix+value)
 		if err != nil {
 			return nil, err
 		}
@@ -1363,7 +1381,7 @@ func (st *HelmState) namespaceAndValuesFlags(helm helmexec.Interface, release *R
 			if set.Value != "" {
 				flags = append(flags, "--set", fmt.Sprintf("%s=%s", escape(set.Name), escape(set.Value)))
 			} else if set.File != "" {
-				flags = append(flags, "--set-file", fmt.Sprintf("%s=%s", escape(set.Name), st.normalizePath(set.File)))
+				flags = append(flags, "--set-file", fmt.Sprintf("%s=%s", escape(set.Name), st.storage().normalizePath(set.File)))
 			} else if len(set.Values) > 0 {
 				items := make([]string, len(set.Values))
 				for i, raw := range set.Values {
@@ -1405,49 +1423,6 @@ func (st *HelmState) namespaceAndValuesFlags(helm helmexec.Interface, release *R
 	return flags, nil
 }
 
-func (st *HelmState) resolveFile(missingFileHandler *string, tpe, path string) ([]string, bool, error) {
-	title := fmt.Sprintf("%s file", tpe)
-
-	files, err := st.ExpandPaths(path)
-	if err != nil {
-		return nil, false, err
-	}
-
-	var handlerId string
-
-	if missingFileHandler != nil {
-		handlerId = *missingFileHandler
-	} else {
-		handlerId = MissingFileHandlerError
-	}
-
-	if len(files) == 0 {
-		switch handlerId {
-		case MissingFileHandlerError:
-			return nil, false, fmt.Errorf("%s matching \"%s\" does not exist", title, path)
-		case MissingFileHandlerWarn:
-			st.logger.Warnf("skipping missing %s matching \"%s\"", title, path)
-			return nil, true, nil
-		case MissingFileHandlerInfo:
-			st.logger.Infof("skipping missing %s matching \"%s\"", title, path)
-			return nil, true, nil
-		case MissingFileHandlerDebug:
-			st.logger.Debugf("skipping missing %s matching \"%s\"", title, path)
-			return nil, true, nil
-		default:
-			available := []string{
-				MissingFileHandlerError,
-				MissingFileHandlerWarn,
-				MissingFileHandlerInfo,
-				MissingFileHandlerDebug,
-			}
-			return nil, false, fmt.Errorf("invalid missing file handler \"%s\" while processing \"%s\" in \"%s\": it must be one of %s", handlerId, path, st.FilePath, available)
-		}
-	}
-
-	return files, false, nil
-}
-
 // DisplayAffectedReleases logs the upgraded, deleted and in error releases
 func (ar *AffectedReleases) DisplayAffectedReleases(logger *zap.SugaredLogger) {
 	if ar.Upgraded != nil {
@@ -1501,6 +1476,8 @@ func (hf *SubHelmfileSpec) UnmarshalYAML(unmarshal func(interface{}) error) erro
 			Path               string   `yaml:"path"`
 			Selectors          []string `yaml:"selectors"`
 			SelectorsInherited bool     `yaml:"selectorsInherited"`
+
+			Environment SubhelmfileEnvironmentSpec `yaml:"environment"`
 		}
 		if err := unmarshal(&subHelmfileSpecTmp); err != nil {
 			return err
@@ -1508,6 +1485,7 @@ func (hf *SubHelmfileSpec) UnmarshalYAML(unmarshal func(interface{}) error) erro
 		hf.Path = subHelmfileSpecTmp.Path
 		hf.Selectors = subHelmfileSpecTmp.Selectors
 		hf.SelectorsInherited = subHelmfileSpecTmp.SelectorsInherited
+		hf.Environment = subHelmfileSpecTmp.Environment
 	}
 	//since we cannot make sur the "console" string can be red after the "path" we must check we don't have
 	//a SubHelmfileSpec with only selector and no path

state/state_test.go

@@ -1001,7 +1001,7 @@ func TestHelmState_SyncReleases_MissingValuesFileForUndesiredRelease(t *testing.
 				Values: []interface{}{"noexistent.values.yaml"},
 			},
 			listResult:    ``,
-			expectedError: `failed processing release foo: values file matching "noexistent.values.yaml" does not exist`,
+			expectedError: `failed processing release foo: values file matching "noexistent.values.yaml" does not exist in "."`,
 		},
 		{
 			name: "should fail upgrading due to missing values file",
@@ -1012,7 +1012,7 @@ func TestHelmState_SyncReleases_MissingValuesFileForUndesiredRelease(t *testing.
 			},
 			listResult: `NAME 	REVISION	UPDATED                 	STATUS  	CHART                      	APP VERSION	NAMESPACE
 										foo	1       	Wed Apr 17 17:39:04 2019	DEPLOYED	foo-bar-2.0.4	0.1.0      	default`,
-			expectedError: `failed processing release foo: values file matching "noexistent.values.yaml" does not exist`,
+			expectedError: `failed processing release foo: values file matching "noexistent.values.yaml" does not exist in "."`,
 		},
 		{
 			name: "should uninstall even when there is a missing values file",
@@ -1031,6 +1031,7 @@ func TestHelmState_SyncReleases_MissingValuesFileForUndesiredRelease(t *testing.
 		tt := tests[i]
 		t.Run(tt.name, func(t *testing.T) {
 			state := &HelmState{
+				basePath: ".",
 				Releases: []ReleaseSpec{tt.release},
 				logger:   logger,
 			}

state/storage.go

@@ -0,0 +1,99 @@
+package state
+
+import (
+	"fmt"
+	"go.uber.org/zap"
+	"net/url"
+	"path/filepath"
+	"sort"
+)
+
+type Storage struct {
+	logger *zap.SugaredLogger
+
+	FilePath string
+
+	basePath string
+	glob     func(string) ([]string, error)
+}
+
+func NewStorage(forFile string, logger *zap.SugaredLogger, glob func(string) ([]string, error)) *Storage {
+	return &Storage{
+		FilePath: forFile,
+		basePath: filepath.Dir(forFile),
+		logger:   logger,
+		glob:     glob,
+	}
+}
+
+func (st *Storage) resolveFile(missingFileHandler *string, tpe, path string) ([]string, bool, error) {
+	title := fmt.Sprintf("%s file", tpe)
+
+	files, err := st.ExpandPaths(path)
+	if err != nil {
+		return nil, false, err
+	}
+
+	var handlerId string
+
+	if missingFileHandler != nil {
+		handlerId = *missingFileHandler
+	} else {
+		handlerId = MissingFileHandlerError
+	}
+
+	if len(files) == 0 {
+		switch handlerId {
+		case MissingFileHandlerError:
+			return nil, false, fmt.Errorf("%s matching \"%s\" does not exist in \"%s\"", title, path, st.basePath)
+		case MissingFileHandlerWarn:
+			st.logger.Warnf("skipping missing %s matching \"%s\"", title, path)
+			return nil, true, nil
+		case MissingFileHandlerInfo:
+			st.logger.Infof("skipping missing %s matching \"%s\"", title, path)
+			return nil, true, nil
+		case MissingFileHandlerDebug:
+			st.logger.Debugf("skipping missing %s matching \"%s\"", title, path)
+			return nil, true, nil
+		default:
+			available := []string{
+				MissingFileHandlerError,
+				MissingFileHandlerWarn,
+				MissingFileHandlerInfo,
+				MissingFileHandlerDebug,
+			}
+			return nil, false, fmt.Errorf("invalid missing file handler \"%s\" while processing \"%s\" in \"%s\": it must be one of %s", handlerId, path, st.FilePath, available)
+		}
+	}
+
+	return files, false, nil
+}
+
+func (st *Storage) ExpandPaths(globPattern string) ([]string, error) {
+	result := []string{}
+	absPathPattern := st.normalizePath(globPattern)
+	matches, err := st.glob(absPathPattern)
+	if err != nil {
+		return nil, fmt.Errorf("failed processing %s: %v", globPattern, err)
+	}
+
+	sort.Strings(matches)
+
+	result = append(result, matches...)
+	return result, nil
+}
+
+// normalizes relative path to absolute one
+func (st *Storage) normalizePath(path string) string {
+	u, _ := url.Parse(path)
+	if u.Scheme != "" || filepath.IsAbs(path) {
+		return path
+	} else {
+		return st.JoinBase(path)
+	}
+}
+
+// JoinBase returns an absolute path in the form basePath/relative
+func (st *Storage) JoinBase(relPath string) string {
+	return filepath.Join(st.basePath, relPath)
+}