diff --git a/docs/environments.md b/docs/environments.md index 41eb7476..cdc7eee6 100644 --- a/docs/environments.md +++ b/docs/environments.md @@ -104,6 +104,21 @@ releases: ... ``` +#### Merge strategy + +By default, when several files are listed under `values:`, later files override earlier files. Set `mergeStrategy: fallback` on the environment to flip the precedence so earlier files win and later files only fill missing keys: + +```yaml +environments: + production: + mergeStrategy: fallback + values: + - cluster-specific.yaml # wins on every key it defines + - shared-defaults.yaml.gotmpl +``` + +Under `fallback`, explicit non-nil values in the earlier file (including zero values like `false`, `0`, `""`, and empty list) are preserved against any later file, while maps are deep-merged so later files may still add nested keys. A later `.gotmpl` file can also reference values from earlier files via `.Values`. See [Merge Strategy: override vs fallback](values-and-merging.md#4a-merge-strategy-override-vs-fallback) for the full semantics, including how explicit `null` is handled. + #### HCL specifications Since Helmfile v0.164.0, HCL language is supported for environment values only. diff --git a/docs/values-and-merging.md b/docs/values-and-merging.md index 5b39d80f..9237a8c3 100644 --- a/docs/values-and-merging.md +++ b/docs/values-and-merging.md @@ -116,6 +116,54 @@ environments: - secrets.yaml # Merged last (step 3) - highest priority ``` +### 4a. Merge Strategy: `override` vs `fallback` + +By default, when an environment lists multiple files under `values:`, **later files override earlier files** (the historical helmfile behavior, equivalent to `mergeStrategy: override`). + +You can flip this per environment so that **earlier files take precedence** and later files only fill in missing keys: + +```yaml +environments: + production: + mergeStrategy: fallback + values: + - cluster-specific.yaml # wins on every key it defines + - shared-defaults.yaml # only fills gaps +``` + +Under `fallback`: + +- An explicit non-nil value in an earlier file is preserved against any later file, including the zero values `false`, `0`, `""`, and empty list. An explicit `enabled: false` in `cluster-specific.yaml` is *not* silently overwritten by `enabled: true` from `shared-defaults.yaml`. +- Maps are deep-merged. An earlier map does not block later files from adding nested keys it didn't set; only the keys an earlier file explicitly defines win on conflict. +- An explicit `null` in an earlier file falls through to a later file's value, matching how `MergeMaps` treats nil from the override side elsewhere in helmfile. +- Within a single `values:` entry that expands to multiple files (e.g. via a glob), the **first** file in the expansion wins. +- A later `.gotmpl` values file can reference values from earlier files via `.Values`, so derived defaults work natively: + + ```yaml + # cluster-specific.yaml + cluster: + domain: prod.example.com + ``` + + ```yaml + # shared-defaults.yaml.gotmpl + service: + domain: "service.{{ .Values.cluster.domain }}" + ``` + + → `service.domain: service.prod.example.com`. Under `mergeStrategy: override` this cross-file template reference is not available. + +Valid values: `override` (default) and `fallback`. Any other value is rejected at load time. + +#### Interaction with `.hcl` values files + +`.hcl` files are evaluated as a single unit so that HCL `locals` and `values` blocks can reference each other across files. Helmfile collects every `.hcl` entry from the `values:` list, renders them together, and merges the combined result after the YAML pass. Two consequences worth knowing: + +- `mergeStrategy` does not reshuffle the position of HCL within the list. HCL's combined output is always merged after the YAML pass. Under `override` it overrides YAML on conflicts (the historical behavior); under `fallback` it fills gaps only. +- Among multiple `.hcl` files, the precedence is HCL's own (last-file-wins within HCL), independent of `mergeStrategy`. The strategy applies at the YAML-vs-HCL boundary, not within HCL. + +If you need first-file-wins precedence between specific HCL files, restructure them into one HCL file (or split the values into YAML). + ### 5. CLI Overrides The highest priority values come from CLI flags: diff --git a/pkg/app/desired_state_file_loader.go b/pkg/app/desired_state_file_loader.go index 8ffe3942..b57a029b 100644 --- a/pkg/app/desired_state_file_loader.go +++ b/pkg/app/desired_state_file_loader.go @@ -69,7 +69,7 @@ func (ld *desiredStateLoader) Load(f string, opts LoadOpts) (*state.HelmState, e // --state-values-file: loaded into Values so arrays replace (not merge) if len(fileArgs) > 0 { - fileVals, err := envld.LoadEnvironmentValues(&handler, fileArgs, environment.New(ld.env), ld.env) + fileVals, err := envld.LoadEnvironmentValues(&handler, fileArgs, environment.New(ld.env), ld.env, "") if err != nil { return nil, err } @@ -78,7 +78,7 @@ func (ld *desiredStateLoader) Load(f string, opts LoadOpts) (*state.HelmState, e // --state-values-set: loaded into CLIOverrides so arrays merge element-by-element if len(setArgs) > 0 { - setVals, err := envld.LoadEnvironmentValues(&handler, setArgs, environment.New(ld.env), ld.env) + setVals, err := envld.LoadEnvironmentValues(&handler, setArgs, environment.New(ld.env), ld.env, "") if err != nil { return nil, err } diff --git a/pkg/state/create.go b/pkg/state/create.go index 9058e9a7..7384596a 100644 --- a/pkg/state/create.go +++ b/pkg/state/create.go @@ -169,7 +169,7 @@ func (c *StateCreator) LoadEnvValues(target *HelmState, env string, failOnMissin return nil, &StateLoadError{fmt.Sprintf("failed to read %s", state.FilePath), err} } - newDefaults, err := state.loadValuesEntries(nil, state.DefaultValues, c.remote, ctxEnv, env) + newDefaults, err := state.loadValuesEntries(nil, state.DefaultValues, c.remote, ctxEnv, env, "") if err != nil { return nil, err } @@ -237,17 +237,12 @@ func mergeEnvironments(dst, src map[string]EnvironmentSpec) { for envName, srcEnv := range src { if dstEnv, exists := dst[envName]; exists { - // Environment exists in both - merge the Values slices - mergedValues := append([]any{}, dstEnv.Values...) - mergedValues = append(mergedValues, srcEnv.Values...) - // Merge Secrets slices mergedSecrets := append([]string{}, dstEnv.Secrets...) mergedSecrets = append(mergedSecrets, srcEnv.Secrets...) // Create merged environment merged := EnvironmentSpec{ - Values: mergedValues, Secrets: mergedSecrets, } @@ -272,6 +267,26 @@ func mergeEnvironments(dst, src map[string]EnvironmentSpec) { merged.MissingFileHandlerConfig = dstEnv.MissingFileHandlerConfig } + // Override MergeStrategy if src has it + if srcEnv.MergeStrategy != "" { + merged.MergeStrategy = srcEnv.MergeStrategy + } else { + merged.MergeStrategy = dstEnv.MergeStrategy + } + + // Concatenate Values so the later layer (src, e.g. main helmfile) + // always takes precedence over the earlier one (dst, e.g. a base + // helmfile). Under override the natural append order achieves that + // (last-file-wins). Under fallback we have to prepend src so that + // "first-file-wins" still resolves to "later layer wins". + if merged.MergeStrategy == MergeStrategyFallback { + mergedValues := append([]any{}, srcEnv.Values...) + merged.Values = append(mergedValues, dstEnv.Values...) + } else { + mergedValues := append([]any{}, dstEnv.Values...) + merged.Values = append(mergedValues, srcEnv.Values...) + } + dst[envName] = merged } else { // Environment only exists in src - just copy it @@ -394,7 +409,7 @@ func (c *StateCreator) loadEnvValues(st *HelmState, name string, failOnMissingEn if err != nil { return nil, err } - valuesVals, err = st.loadValuesEntries(envSpec.MissingFileHandler, envValuesEntries, c.remote, loadValuesEntriesEnv, name) + valuesVals, err = st.loadValuesEntries(envSpec.MissingFileHandler, envValuesEntries, c.remote, loadValuesEntriesEnv, name, envSpec.MergeStrategy) if err != nil { return nil, err } @@ -550,13 +565,13 @@ func (c *StateCreator) scatterGatherEnvSecretFiles(st *HelmState, envSecretFiles return decryptedFilesKeeper, nil } -func (st *HelmState) loadValuesEntries(missingFileHandler *string, entries []any, remote *remote.Remote, ctxEnv *environment.Environment, envName string) (map[string]any, error) { +func (st *HelmState) loadValuesEntries(missingFileHandler *string, entries []any, remote *remote.Remote, ctxEnv *environment.Environment, envName string, mergeStrategy string) (map[string]any, error) { var envVals map[string]any valuesEntries := append([]any{}, entries...) ld := NewEnvironmentValuesLoader(st.storage(), st.fs, st.logger, remote) var err error - envVals, err = ld.LoadEnvironmentValues(missingFileHandler, valuesEntries, ctxEnv, envName) + envVals, err = ld.LoadEnvironmentValues(missingFileHandler, valuesEntries, ctxEnv, envName, mergeStrategy) if err != nil { return nil, err } diff --git a/pkg/state/create_test.go b/pkg/state/create_test.go index aa6b704e..0d28753c 100644 --- a/pkg/state/create_test.go +++ b/pkg/state/create_test.go @@ -932,3 +932,96 @@ releases: }) } } + +// mergeEnvironments must keep the established "later layer wins over earlier +// layer" semantics even when the resolved strategy is fallback. Under override +// the natural append order (dst then src) achieves that because last-wins +// chooses src; under fallback we have to prepend src so that first-wins still +// resolves to src. Without this adjustment, a base helmfile's values would +// silently override values declared in the main helmfile that includes it. +func TestMergeEnvironments_LaterLayerWinsRegardlessOfStrategy(t *testing.T) { + tests := []struct { + name string + strategy string + wantOrdering []any + }{ + { + name: "override appends src after dst (last wins)", + strategy: MergeStrategyOverride, + wantOrdering: []any{"base.yaml", "main.yaml"}, + }, + { + name: "empty strategy defaults to override semantics", + strategy: "", + wantOrdering: []any{"base.yaml", "main.yaml"}, + }, + { + name: "fallback prepends src (first wins) so main still beats base", + strategy: MergeStrategyFallback, + wantOrdering: []any{"main.yaml", "base.yaml"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dst := map[string]EnvironmentSpec{ + "prod": {Values: []any{"base.yaml"}, MergeStrategy: tt.strategy}, + } + src := map[string]EnvironmentSpec{ + "prod": {Values: []any{"main.yaml"}, MergeStrategy: tt.strategy}, + } + mergeEnvironments(dst, src) + got := dst["prod"].Values + if !reflect.DeepEqual(got, tt.wantOrdering) { + t.Errorf("Values ordering: want %v, got %v", tt.wantOrdering, got) + } + }) + } +} + +// mergeEnvironments must preserve MergeStrategy when layering multiple +// helmfiles (bases). Without this, an environment that opted into +// fallback in a base helmfile would silently fall back to the default +// override behavior in any helmfile that re-declares the environment. +func TestMergeEnvironments_PreservesMergeStrategy(t *testing.T) { + tests := []struct { + name string + dst EnvironmentSpec + src EnvironmentSpec + expected string + }{ + { + name: "src declares fallback, dst empty", + dst: EnvironmentSpec{}, + src: EnvironmentSpec{MergeStrategy: MergeStrategyFallback}, + expected: MergeStrategyFallback, + }, + { + name: "dst declares fallback, src empty preserves dst", + dst: EnvironmentSpec{MergeStrategy: MergeStrategyFallback}, + src: EnvironmentSpec{}, + expected: MergeStrategyFallback, + }, + { + name: "src override wins over dst fallback", + dst: EnvironmentSpec{MergeStrategy: MergeStrategyFallback}, + src: EnvironmentSpec{MergeStrategy: MergeStrategyOverride}, + expected: MergeStrategyOverride, + }, + { + name: "both empty stays empty", + dst: EnvironmentSpec{}, + src: EnvironmentSpec{}, + expected: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dst := map[string]EnvironmentSpec{"prod": tt.dst} + src := map[string]EnvironmentSpec{"prod": tt.src} + mergeEnvironments(dst, src) + if got := dst["prod"].MergeStrategy; got != tt.expected { + t.Errorf("MergeStrategy after merge: want %q, got %q", tt.expected, got) + } + }) + } +} diff --git a/pkg/state/environment.go b/pkg/state/environment.go index c5c78876..91d997ab 100644 --- a/pkg/state/environment.go +++ b/pkg/state/environment.go @@ -15,4 +15,17 @@ type EnvironmentSpec struct { MissingFileHandler *string `yaml:"missingFileHandler,omitempty"` // MissingFileHandlerConfig is composed of various settings for the MissingFileHandler MissingFileHandlerConfig *MissingFileHandlerConfig `yaml:"missingFileHandlerConfig,omitempty"` + + // MergeStrategy controls precedence when multiple values files are listed under `values`. + // + // "override" (default): later files override earlier files (the historical helmfile behavior). + // "fallback": earlier files take precedence; later files only fill gaps. + // + // Under the "fallback" strategy, an explicit non-nil value in an earlier file (including + // the zero values false, 0, "", and empty list) is preserved against any later file. Maps + // are deep-merged, so an earlier map does not block later files from adding nested keys. + // An explicit null in an earlier file falls through to a later file's value (matching how + // helmfile's MergeMaps treats nil from the override side elsewhere). Subsequent .gotmpl + // values files can also reference values from earlier files via .Values. + MergeStrategy string `yaml:"mergeStrategy,omitempty"` } diff --git a/pkg/state/envvals_loader.go b/pkg/state/envvals_loader.go index 8764025b..048154a8 100644 --- a/pkg/state/envvals_loader.go +++ b/pkg/state/envvals_loader.go @@ -36,7 +36,13 @@ func NewEnvironmentValuesLoader(storage *Storage, fs *filesystem.FileSystem, log } } -func (ld *EnvironmentValuesLoader) LoadEnvironmentValues(missingFileHandler *string, valuesEntries []any, ctxEnv *environment.Environment, envName string) (map[string]any, error) { +func (ld *EnvironmentValuesLoader) LoadEnvironmentValues(missingFileHandler *string, valuesEntries []any, ctxEnv *environment.Environment, envName string, mergeStrategy string) (map[string]any, error) { + switch mergeStrategy { + case "", MergeStrategyOverride, MergeStrategyFallback: + default: + return nil, fmt.Errorf("environment %q: invalid mergeStrategy %q (must be %q or %q)", + envName, mergeStrategy, MergeStrategyOverride, MergeStrategyFallback) + } var ( result = map[string]any{} hclLoader = hcllang.NewHCLLoader(ld.fs, ld.logger) @@ -44,8 +50,6 @@ func (ld *EnvironmentValuesLoader) LoadEnvironmentValues(missingFileHandler *str ) for _, entry := range valuesEntries { - maps := []any{} - switch strOrMap := entry.(type) { case string: files, skipped, err := ld.storage.resolveFile(missingFileHandler, "environment values", entry.(string)) @@ -64,37 +68,56 @@ func (ld *EnvironmentValuesLoader) LoadEnvironmentValues(missingFileHandler *str } if strings.HasSuffix(f, ".hcl") { hclLoader.AddFile(f) - } else { - // Use merged values (Defaults + Values + CLIOverrides) for template rendering - // so that CLI values are accessible via .Values in environment value files. - mergedVals, err := env.GetMergedValues() - if err != nil { - return nil, fmt.Errorf("failed to get merged values for environment file \"%s\": %v", f, err) + continue + } + // Use merged values (Defaults + Values + CLIOverrides) for template rendering + // so that CLI values are accessible via .Values in environment value files. + mergedVals, err := env.GetMergedValues() + if err != nil { + return nil, fmt.Errorf("failed to get merged values for environment file \"%s\": %v", f, err) + } + // Under fallback strategy, also expose values accumulated from earlier files + // in this same `values:` list, including earlier files in this same glob + // expansion, so a later .gotmpl can reference them via .Values (e.g. + // `{{ .Values.cluster.domain }}`). Env CLI overrides and values still win, + // layered on top with WithOverride. + if mergeStrategy == MergeStrategyFallback && len(result) > 0 { + enriched := map[string]any{} + if err := mergo.Merge(&enriched, result); err != nil { + return nil, fmt.Errorf("failed to build template context for \"%s\": %v", f, err) } - tmplData := NewEnvironmentTemplateData(env, "", mergedVals) - r := tmpl.NewFileRenderer(ld.fs, filepath.Dir(f), tmplData) - bytes, err := r.RenderToBytes(f) - if err != nil { - return nil, fmt.Errorf("failed to load environment values file \"%s\": %v", f, err) + if err := mergo.Merge(&enriched, mergedVals, mergo.WithOverride); err != nil { + return nil, fmt.Errorf("failed to build template context for \"%s\": %v", f, err) } - m := map[string]any{} - if err := yaml.Unmarshal(bytes, &m); err != nil { - return nil, fmt.Errorf("failed to load environment values file \"%s\": %v\n\nOffending YAML:\n%s", f, err, bytes) - } - maps = append(maps, m) - ld.logger.Debugf("envvals_loader: loaded %s:%v", strOrMap, m) + mergedVals = enriched + } + tmplData := NewEnvironmentTemplateData(env, "", mergedVals) + r := tmpl.NewFileRenderer(ld.fs, filepath.Dir(f), tmplData) + bytes, err := r.RenderToBytes(f) + if err != nil { + return nil, fmt.Errorf("failed to load environment values file \"%s\": %v", f, err) + } + m := map[string]any{} + if err := yaml.Unmarshal(bytes, &m); err != nil { + return nil, fmt.Errorf("failed to load environment values file \"%s\": %v\n\nOffending YAML:\n%s", f, err, bytes) + } + ld.logger.Debugf("envvals_loader: loaded %s:%v", strOrMap, m) + // Merge each file into result immediately so subsequent files in the same + // entry's expansion (e.g. a glob) can see prior files' values via .Values + // when rendered as templates. + result, err = mapMerge(result, []any{m}, mergeStrategy) + if err != nil { + return nil, err } } case map[any]any, map[string]any: - maps = append(maps, strOrMap) + result, err = mapMerge(result, []any{strOrMap}, mergeStrategy) + if err != nil { + return nil, err + } default: return nil, fmt.Errorf("unexpected type of value: value=%v, type=%T", strOrMap, strOrMap) } - - result, err = mapMerge(result, maps) - if err != nil { - return nil, err - } } maps := []any{} if hclLoader.Length() > 0 { @@ -104,14 +127,14 @@ func (ld *EnvironmentValuesLoader) LoadEnvironmentValues(missingFileHandler *str } maps = append(maps, m) } - result, err = mapMerge(result, maps) + result, err = mapMerge(result, maps, mergeStrategy) if err != nil { return nil, err } return result, nil } -func mapMerge(dest map[string]any, maps []any) (map[string]any, error) { +func mapMerge(dest map[string]any, maps []any, mergeStrategy string) (map[string]any, error) { for _, m := range maps { // All the nested map key should be string. Otherwise we get strange errors due to that // mergo or reflect is unable to merge map[any]any with map[string]any or vice versa. @@ -120,6 +143,16 @@ func mapMerge(dest map[string]any, maps []any) (map[string]any, error) { if err != nil { return nil, err } + if mergeStrategy == MergeStrategyFallback { + // First-file-wins: the new file is the base and the + // accumulator overlays it, so keys already accumulated keep + // their value while keys only present in the new file fill + // in. MergeMaps is used instead of mergo because mergo's + // isEmptyValue rule would silently let a later fallback's + // `enabled: true` clobber an explicit `enabled: false`. + dest = maputil.MergeMaps(vals, dest) + continue + } if err := mergo.Merge(&dest, &vals, mergo.WithOverride); err != nil { return nil, fmt.Errorf("failed to merge %v: %v", m, err) } diff --git a/pkg/state/envvals_loader_test.go b/pkg/state/envvals_loader_test.go index 764ada78..5902ebe6 100644 --- a/pkg/state/envvals_loader_test.go +++ b/pkg/state/envvals_loader_test.go @@ -2,6 +2,7 @@ package state import ( "io" + "strings" "testing" "github.com/google/go-cmp/cmp" @@ -29,7 +30,7 @@ func newLoader() *EnvironmentValuesLoader { func TestEnvValsLoad_SingleValuesFile(t *testing.T) { l := newLoader() - actual, err := l.LoadEnvironmentValues(nil, []any{"testdata/values.5.yaml"}, nil, "") + actual, err := l.LoadEnvironmentValues(nil, []any{"testdata/values.5.yaml"}, nil, "", "") if err != nil { t.Fatal(err) } @@ -87,7 +88,7 @@ func TestEnvValsLoad_EnvironmentNameFile(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - actual, err := l.LoadEnvironmentValues(nil, []any{"testdata/values.6.yaml.gotmpl"}, tt.env, tt.envName) + actual, err := l.LoadEnvironmentValues(nil, []any{"testdata/values.6.yaml.gotmpl"}, tt.env, tt.envName, "") if err != nil { t.Fatal(err) } @@ -103,7 +104,7 @@ func TestEnvValsLoad_EnvironmentNameFile(t *testing.T) { func TestEnvValsLoad_SingleValuesFileRemote(t *testing.T) { l := newLoader() - actual, err := l.LoadEnvironmentValues(nil, []any{"git::https://github.com/helm/helm.git@cmd/helm/testdata/output/values.yaml?ref=v3.8.1"}, nil, "") + actual, err := l.LoadEnvironmentValues(nil, []any{"git::https://github.com/helm/helm.git@cmd/helm/testdata/output/values.yaml?ref=v3.8.1"}, nil, "", "") if err != nil { t.Fatal(err) } @@ -121,7 +122,7 @@ func TestEnvValsLoad_SingleValuesFileRemote(t *testing.T) { func TestEnvValsLoad_OverwriteNilValue_Issue1150(t *testing.T) { l := newLoader() - actual, err := l.LoadEnvironmentValues(nil, []any{"testdata/values.1.yaml", "testdata/values.2.yaml"}, nil, "") + actual, err := l.LoadEnvironmentValues(nil, []any{"testdata/values.1.yaml", "testdata/values.2.yaml"}, nil, "", "") if err != nil { t.Fatal(err) } @@ -143,7 +144,7 @@ func TestEnvValsLoad_OverwriteNilValue_Issue1150(t *testing.T) { func TestEnvValsLoad_OverwriteWithNilValue_Issue1154(t *testing.T) { l := newLoader() - actual, err := l.LoadEnvironmentValues(nil, []any{"testdata/values.3.yaml", "testdata/values.4.yaml"}, nil, "") + actual, err := l.LoadEnvironmentValues(nil, []any{"testdata/values.3.yaml", "testdata/values.4.yaml"}, nil, "", "") if err != nil { t.Fatal(err) } @@ -166,7 +167,7 @@ func TestEnvValsLoad_OverwriteWithNilValue_Issue1154(t *testing.T) { func TestEnvValsLoad_OverwriteEmptyValue_Issue1168(t *testing.T) { l := newLoader() - actual, err := l.LoadEnvironmentValues(nil, []any{"testdata/issues/1168/addons.yaml", "testdata/issues/1168/addons2.yaml"}, nil, "") + actual, err := l.LoadEnvironmentValues(nil, []any{"testdata/issues/1168/addons.yaml", "testdata/issues/1168/addons2.yaml"}, nil, "", "") if err != nil { t.Fatal(err) } @@ -191,7 +192,7 @@ func TestEnvValsLoad_OverwriteEmptyValue_Issue1168(t *testing.T) { func TestEnvValsLoad_MultiHCL(t *testing.T) { l := newLoader() - actual, err := l.LoadEnvironmentValues(nil, []any{"testdata/values.7.hcl", "testdata/values.8.hcl"}, nil, "") + actual, err := l.LoadEnvironmentValues(nil, []any{"testdata/values.7.hcl", "testdata/values.8.hcl"}, nil, "", "") if err != nil { t.Fatal(err) } @@ -234,7 +235,7 @@ func TestEnvValsLoad_EnvironmentValues(t *testing.T) { env := environment.New("test") env.Values["foo"] = "bar" - actual, err := l.LoadEnvironmentValues(nil, []any{"testdata/values.9.yaml.gotmpl"}, env, "") + actual, err := l.LoadEnvironmentValues(nil, []any{"testdata/values.9.yaml.gotmpl"}, env, "", "") if err != nil { t.Fatal(err) } @@ -247,3 +248,292 @@ func TestEnvValsLoad_EnvironmentValues(t *testing.T) { t.Error(diff) } } + +// --- mergeStrategy: fallback --- + +// Earlier files take precedence. Same conflicting key in two files → +// the value from default.yaml (loaded first) wins. +func TestEnvValsLoad_FallbackStrategy_EarlierWins(t *testing.T) { + l := newLoader() + + actual, err := l.LoadEnvironmentValues(nil, + []any{"testdata/mergestrategy/default.yaml", "testdata/mergestrategy/fallback.yaml"}, + nil, "", MergeStrategyFallback) + if err != nil { + t.Fatal(err) + } + + cluster := actual["cluster"].(map[string]any) + if got := cluster["domain"]; got != "example.com" { + t.Errorf("cluster.domain: want %q (from default.yaml), got %v", "example.com", got) + } +} + +// Later files only fill keys that are missing from earlier files. +func TestEnvValsLoad_FallbackStrategy_FillsGaps(t *testing.T) { + l := newLoader() + + actual, err := l.LoadEnvironmentValues(nil, + []any{"testdata/mergestrategy/default.yaml", "testdata/mergestrategy/fallback.yaml"}, + nil, "", MergeStrategyFallback) + if err != nil { + t.Fatal(err) + } + + cluster := actual["cluster"].(map[string]any) + if got := cluster["region"]; got != "us-east-1" { + t.Errorf("cluster.region: want %q (from fallback.yaml, missing in default.yaml), got %v", "us-east-1", got) + } + service := actual["service"].(map[string]any) + if got := service["port"]; got != 8080 { + t.Errorf("service.port: want 8080 (from fallback.yaml), got %v", got) + } +} + +// Nested maps merge recursively: top-level cluster is not replaced +// wholesale; both files contribute keys. +func TestEnvValsLoad_FallbackStrategy_DeepMerge(t *testing.T) { + l := newLoader() + + actual, err := l.LoadEnvironmentValues(nil, + []any{"testdata/mergestrategy/default.yaml", "testdata/mergestrategy/fallback.yaml"}, + nil, "", MergeStrategyFallback) + if err != nil { + t.Fatal(err) + } + + expected := map[string]any{ + "cluster": map[string]any{ + "domain": "example.com", // from default (wins) + "region": "us-east-1", // from fallback (gap filled) + }, + "service": map[string]any{ + "port": 8080, // from fallback (gap filled) + }, + } + if diff := cmp.Diff(expected, actual); diff != "" { + t.Errorf("deep merge mismatch (-want +got):\n%s", diff) + } +} + +// First-wins precedence holds across an arbitrarily long chain, not just +// pairwise. Three files exercise the accumulator state across iterations. +func TestEnvValsLoad_FallbackStrategy_ChainedFiles(t *testing.T) { + l := newLoader() + + actual, err := l.LoadEnvironmentValues(nil, + []any{ + "testdata/mergestrategy/chain_a.yaml", + "testdata/mergestrategy/chain_b.yaml", + "testdata/mergestrategy/chain_c.yaml", + }, + nil, "", MergeStrategyFallback) + if err != nil { + t.Fatal(err) + } + + expected := map[string]any{ + "letter": "a", // only in a + "only_a": "from-a", // only in a + "only_b": "from-b", // only in b + "only_c": "from-c", // only in c + "both_ab": "from-a", // a and b → a wins (earlier) + "both_bc": "from-b", // b and c → b wins (earlier) + "all_three": "from-a", // a, b, c → a wins (earliest) + } + if diff := cmp.Diff(expected, actual); diff != "" { + t.Errorf("chain mismatch (-want +got):\n%s", diff) + } +} + +// Explicit zero values in the earlier file MUST be preserved. Without the +// hand-rolled fallbackDeepMerge, mergo's isEmptyValue would silently let +// `enabled: true` from fallback overwrite `enabled: false` from default. +func TestEnvValsLoad_FallbackStrategy_PreservesExplicitZeroValues(t *testing.T) { + l := newLoader() + + actual, err := l.LoadEnvironmentValues(nil, + []any{"testdata/mergestrategy/zero_default.yaml", "testdata/mergestrategy/zero_fallback.yaml"}, + nil, "", MergeStrategyFallback) + if err != nil { + t.Fatal(err) + } + + expected := map[string]any{ + "enabled": false, + "replicas": 0, + "name": "", + "tags": []any{}, + } + if diff := cmp.Diff(expected, actual); diff != "" { + t.Errorf("explicit zero values not preserved (-want +got):\n%s", diff) + } +} + +// Explicit nil in the earlier file does NOT win under fallback: it falls +// through to the fallback file's value. This matches helmfile's existing +// MergeMaps treatment of nil ("nil from the override side only fills missing +// keys"; here, by argument-swap, nil from the winner is treated as +// "no preference, let the fallback fill it"). Documented as the deliberate +// difference from override mode, where mergo.WithOverride lets nil overwrite +// (see TestEnvValsLoad_OverwriteWithNilValue_Issue1154). +func TestEnvValsLoad_FallbackStrategy_NilFallsThroughToFallback(t *testing.T) { + l := newLoader() + + actual, err := l.LoadEnvironmentValues(nil, + []any{"testdata/mergestrategy/nil_default.yaml", "testdata/mergestrategy/nil_fallback.yaml"}, + nil, "", MergeStrategyFallback) + if err != nil { + t.Fatal(err) + } + + expected := map[string]any{"value": "from-fallback"} + if diff := cmp.Diff(expected, actual); diff != "" { + t.Errorf("explicit nil should fall through to fallback (-want +got):\n%s", diff) + } +} + +// Inline map entries (not file paths) also honor the fallback strategy. +func TestEnvValsLoad_FallbackStrategy_InlineMapEntry(t *testing.T) { + l := newLoader() + + inline := map[string]any{ + "cluster": map[string]any{"domain": "inline.example"}, + "extra": "from-inline", + } + + actual, err := l.LoadEnvironmentValues(nil, + []any{inline, "testdata/mergestrategy/fallback.yaml"}, + nil, "", MergeStrategyFallback) + if err != nil { + t.Fatal(err) + } + + cluster := actual["cluster"].(map[string]any) + if got := cluster["domain"]; got != "inline.example" { + t.Errorf("cluster.domain: want inline value to win, got %v", got) + } + if got := actual["extra"]; got != "from-inline" { + t.Errorf("extra: want %q, got %v", "from-inline", got) + } + // fallback.yaml still fills gaps the inline map did not set. + if got := cluster["region"]; got != "us-east-1" { + t.Errorf("cluster.region: want %q from fallback file, got %v", "us-east-1", got) + } +} + +// Regression guard: explicit "override" matches today's behavior +// (last file wins). +func TestEnvValsLoad_OverrideStrategy_PreservesCurrentBehavior(t *testing.T) { + l := newLoader() + + actual, err := l.LoadEnvironmentValues(nil, + []any{"testdata/mergestrategy/default.yaml", "testdata/mergestrategy/fallback.yaml"}, + nil, "", MergeStrategyOverride) + if err != nil { + t.Fatal(err) + } + + cluster := actual["cluster"].(map[string]any) + if got := cluster["domain"]; got != "cluster.local" { + t.Errorf("cluster.domain under override: want %q (from fallback.yaml), got %v", "cluster.local", got) + } +} + +// Empty strategy is identical to explicit "override". +func TestEnvValsLoad_DefaultStrategy_MatchesOverride(t *testing.T) { + l := newLoader() + + asDefault, err := l.LoadEnvironmentValues(nil, + []any{"testdata/mergestrategy/default.yaml", "testdata/mergestrategy/fallback.yaml"}, + nil, "", "") + if err != nil { + t.Fatal(err) + } + asOverride, err := l.LoadEnvironmentValues(nil, + []any{"testdata/mergestrategy/default.yaml", "testdata/mergestrategy/fallback.yaml"}, + nil, "", MergeStrategyOverride) + if err != nil { + t.Fatal(err) + } + if diff := cmp.Diff(asOverride, asDefault); diff != "" { + t.Errorf("default strategy diverges from override (-override +default):\n%s", diff) + } +} + +// Within a single `values:` entry that expands to multiple files (a glob), a +// later .gotmpl in the expansion can reference earlier files in that same +// expansion. Matters because the inner file loop must merge each parsed file +// into the accumulator before rendering the next, not buffer the whole +// expansion and merge once at the end. +func TestEnvValsLoad_FallbackStrategy_GlobTemplateSeesPriorFileInSameExpansion(t *testing.T) { + l := newLoader() + + actual, err := l.LoadEnvironmentValues(nil, + []any{"testdata/mergestrategy/glob_*.yaml*"}, + nil, "", MergeStrategyFallback) + if err != nil { + t.Fatal(err) + } + + service := actual["service"].(map[string]any) + if got := service["domain"]; got != "service.example.com" { + t.Errorf("service.domain: want %q (templated from sibling glob match), got %v", + "service.example.com", got) + } +} + +// The headline use case: under fallback, a later .gotmpl values file can +// reference values defined by earlier files in the same list via .Values. +// default.yaml sets cluster.domain; fallback.yaml.gotmpl renders +// `service.domain: "service.{{ .Values.cluster.domain }}"`. +func TestEnvValsLoad_FallbackStrategy_TemplateAccessesPriorFile(t *testing.T) { + l := newLoader() + + actual, err := l.LoadEnvironmentValues(nil, + []any{"testdata/mergestrategy/default.yaml", "testdata/mergestrategy/fallback.yaml.gotmpl"}, + nil, "", MergeStrategyFallback) + if err != nil { + t.Fatal(err) + } + + service := actual["service"].(map[string]any) + if got := service["domain"]; got != "service.example.com" { + t.Errorf("service.domain: want %q (templated from prior file), got %v", + "service.example.com", got) + } +} + +// Symmetric guard: under override, the same .gotmpl reference does NOT +// see prior files in the same list. Documents the deliberate scoping: +// the cross-file template enrichment is opt-in via mergeStrategy: fallback. +func TestEnvValsLoad_OverrideStrategy_TemplateContextUnchanged(t *testing.T) { + l := newLoader() + + _, err := l.LoadEnvironmentValues(nil, + []any{"testdata/mergestrategy/default.yaml", "testdata/mergestrategy/fallback.yaml.gotmpl"}, + nil, "", MergeStrategyOverride) + if err == nil { + t.Fatal("expected template render error: under override, .Values.cluster.domain should not resolve to a prior file's value") + } + // The exact error wording is owned by the template renderer; we only + // assert that we got an error rather than a bogus successful render. +} + +// Unknown strategy values produce a clear error that names both the bad +// value and the valid options. +func TestEnvValsLoad_InvalidStrategy_Errors(t *testing.T) { + l := newLoader() + + _, err := l.LoadEnvironmentValues(nil, + []any{"testdata/mergestrategy/default.yaml"}, + nil, "prod", "bogus") + if err == nil { + t.Fatal("expected error for invalid mergeStrategy, got nil") + } + for _, want := range []string{"prod", "bogus", MergeStrategyOverride, MergeStrategyFallback} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error message missing %q: %v", want, err) + } + } +} diff --git a/pkg/state/state.go b/pkg/state/state.go index cec8f890..8a82e85e 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -49,6 +49,12 @@ const ( // Valid enum for updateStrategy values UpdateStrategyReinstallIfForbidden = "reinstallIfForbidden" + + // Valid values for environment mergeStrategy. + // MergeStrategyOverride (default) makes later values files override earlier ones. + // MergeStrategyFallback flips the precedence: earlier files win and later files only fill gaps. + MergeStrategyOverride = "override" + MergeStrategyFallback = "fallback" ) // ReleaseSetSpec is release set spec diff --git a/pkg/state/testdata/mergestrategy/chain_a.yaml b/pkg/state/testdata/mergestrategy/chain_a.yaml new file mode 100644 index 00000000..4c23a0f4 --- /dev/null +++ b/pkg/state/testdata/mergestrategy/chain_a.yaml @@ -0,0 +1,4 @@ +letter: a +only_a: from-a +both_ab: from-a +all_three: from-a diff --git a/pkg/state/testdata/mergestrategy/chain_b.yaml b/pkg/state/testdata/mergestrategy/chain_b.yaml new file mode 100644 index 00000000..19d208f6 --- /dev/null +++ b/pkg/state/testdata/mergestrategy/chain_b.yaml @@ -0,0 +1,4 @@ +only_b: from-b +both_ab: from-b +both_bc: from-b +all_three: from-b diff --git a/pkg/state/testdata/mergestrategy/chain_c.yaml b/pkg/state/testdata/mergestrategy/chain_c.yaml new file mode 100644 index 00000000..2fd3686f --- /dev/null +++ b/pkg/state/testdata/mergestrategy/chain_c.yaml @@ -0,0 +1,3 @@ +only_c: from-c +both_bc: from-c +all_three: from-c diff --git a/pkg/state/testdata/mergestrategy/default.yaml b/pkg/state/testdata/mergestrategy/default.yaml new file mode 100644 index 00000000..4ad18014 --- /dev/null +++ b/pkg/state/testdata/mergestrategy/default.yaml @@ -0,0 +1,2 @@ +cluster: + domain: example.com diff --git a/pkg/state/testdata/mergestrategy/fallback.yaml b/pkg/state/testdata/mergestrategy/fallback.yaml new file mode 100644 index 00000000..8b57825e --- /dev/null +++ b/pkg/state/testdata/mergestrategy/fallback.yaml @@ -0,0 +1,5 @@ +cluster: + domain: cluster.local + region: us-east-1 +service: + port: 8080 diff --git a/pkg/state/testdata/mergestrategy/fallback.yaml.gotmpl b/pkg/state/testdata/mergestrategy/fallback.yaml.gotmpl new file mode 100644 index 00000000..0df72cba --- /dev/null +++ b/pkg/state/testdata/mergestrategy/fallback.yaml.gotmpl @@ -0,0 +1,3 @@ +service: + domain: "service.{{ .Values.cluster.domain }}" + port: 8080 diff --git a/pkg/state/testdata/mergestrategy/glob_a.yaml b/pkg/state/testdata/mergestrategy/glob_a.yaml new file mode 100644 index 00000000..4ad18014 --- /dev/null +++ b/pkg/state/testdata/mergestrategy/glob_a.yaml @@ -0,0 +1,2 @@ +cluster: + domain: example.com diff --git a/pkg/state/testdata/mergestrategy/glob_b.yaml.gotmpl b/pkg/state/testdata/mergestrategy/glob_b.yaml.gotmpl new file mode 100644 index 00000000..2d92987e --- /dev/null +++ b/pkg/state/testdata/mergestrategy/glob_b.yaml.gotmpl @@ -0,0 +1,2 @@ +service: + domain: "service.{{ .Values.cluster.domain }}" diff --git a/pkg/state/testdata/mergestrategy/nil_default.yaml b/pkg/state/testdata/mergestrategy/nil_default.yaml new file mode 100644 index 00000000..6a1a5a05 --- /dev/null +++ b/pkg/state/testdata/mergestrategy/nil_default.yaml @@ -0,0 +1 @@ +value: ~ diff --git a/pkg/state/testdata/mergestrategy/nil_fallback.yaml b/pkg/state/testdata/mergestrategy/nil_fallback.yaml new file mode 100644 index 00000000..5a7504c9 --- /dev/null +++ b/pkg/state/testdata/mergestrategy/nil_fallback.yaml @@ -0,0 +1 @@ +value: from-fallback diff --git a/pkg/state/testdata/mergestrategy/zero_default.yaml b/pkg/state/testdata/mergestrategy/zero_default.yaml new file mode 100644 index 00000000..d1fb7157 --- /dev/null +++ b/pkg/state/testdata/mergestrategy/zero_default.yaml @@ -0,0 +1,4 @@ +enabled: false +replicas: 0 +name: "" +tags: [] diff --git a/pkg/state/testdata/mergestrategy/zero_fallback.yaml b/pkg/state/testdata/mergestrategy/zero_fallback.yaml new file mode 100644 index 00000000..fc39f730 --- /dev/null +++ b/pkg/state/testdata/mergestrategy/zero_fallback.yaml @@ -0,0 +1,6 @@ +enabled: true +replicas: 3 +name: from-fallback +tags: + - a + - b