feat: update state values handling to replace arrays instead of merging
Signed-off-by: Vojta Polak <vojta.polak@gmail.com>
This commit is contained in:
parent
fc6cf5d2cc
commit
bca4e96bf0
|
|
@ -1362,14 +1362,12 @@ func (a *App) visitStatesWithSelectorsAndRemoteSupportWithContext(fileOrDir stri
|
|||
for _, v := range a.ValuesFiles {
|
||||
envvals = append(envvals, v)
|
||||
}
|
||||
|
||||
if len(a.Set) > 0 {
|
||||
envvals = append(envvals, a.Set)
|
||||
}
|
||||
|
||||
if len(envvals) > 0 {
|
||||
opts.Environment.OverrideValues = envvals
|
||||
opts.Environment.OverrideValuesAreCLI = true
|
||||
}
|
||||
|
||||
if len(a.Set) > 0 {
|
||||
opts.Environment.OverrideCLISetValues = []any{a.Set}
|
||||
}
|
||||
|
||||
a.remote = remote.NewRemote(a.Logger, "", a.fs)
|
||||
|
|
|
|||
|
|
@ -1283,6 +1283,67 @@ x:
|
|||
}
|
||||
}
|
||||
|
||||
// TestStateValuesFileArrayReplace verifies that arrays in --state-values-file
|
||||
// replace environment default arrays entirely rather than merging element-by-element.
|
||||
// Regression test for https://github.com/helmfile/helmfile/issues/2536
|
||||
func TestStateValuesFileArrayReplace(t *testing.T) {
|
||||
files := map[string]string{
|
||||
"/path/to/helmfile.yaml.gotmpl": `
|
||||
environments:
|
||||
default:
|
||||
values:
|
||||
- env-defaults.yaml
|
||||
---
|
||||
releases:
|
||||
- name: {{ .Values.list | join "," }}
|
||||
chart: stable/noop
|
||||
`,
|
||||
// Environment default defines a list with two elements
|
||||
"/path/to/env-defaults.yaml": `
|
||||
list:
|
||||
- first
|
||||
- second
|
||||
`,
|
||||
// --state-values-file overrides the list with a single element
|
||||
// This should REPLACE the list, not merge element-by-element.
|
||||
"/path/to/overrides.yaml": `
|
||||
list:
|
||||
- second
|
||||
`,
|
||||
}
|
||||
|
||||
actual := []state.ReleaseSpec{}
|
||||
collectReleases := func(run *Run) (bool, []error) {
|
||||
actual = append(actual, run.state.Releases...)
|
||||
return false, []error{}
|
||||
}
|
||||
|
||||
app := appWithFs(&App{
|
||||
OverrideHelmBinary: DefaultHelmBinary,
|
||||
OverrideKubeContext: "default",
|
||||
DisableKubeVersionAutoDetection: true,
|
||||
Logger: newAppTestLogger(),
|
||||
Selectors: []string{},
|
||||
Env: "default",
|
||||
ValuesFiles: []string{"overrides.yaml"},
|
||||
FileOrDir: "helmfile.yaml.gotmpl",
|
||||
}, files)
|
||||
|
||||
expectNoCallsToHelm(app)
|
||||
|
||||
err := app.ForEachState(collectReleases, false, SetFilter(true))
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if len(actual) != 1 {
|
||||
t.Fatalf("expected 1 release, got %d", len(actual))
|
||||
}
|
||||
// list should be ["second"] (replaced), not ["second","second"] (merged by position)
|
||||
if actual[0].Name != "second" {
|
||||
t.Errorf("expected release name %q (list replaced), got %q (list was merged)", "second", actual[0].Name)
|
||||
}
|
||||
}
|
||||
|
||||
func TestVisitDesiredStatesWithReleasesFiltered_ChartAtAbsPath(t *testing.T) {
|
||||
files := map[string]string{
|
||||
"/path/to/helmfile.yaml": `
|
||||
|
|
|
|||
|
|
@ -50,30 +50,39 @@ type desiredStateLoader struct {
|
|||
func (ld *desiredStateLoader) Load(f string, opts LoadOpts) (*state.HelmState, error) {
|
||||
var overrodeEnv *environment.Environment
|
||||
|
||||
args := opts.Environment.OverrideValues
|
||||
fileArgs := opts.Environment.OverrideValues
|
||||
setArgs := opts.Environment.OverrideCLISetValues
|
||||
|
||||
if len(args) > 0 {
|
||||
if len(fileArgs) > 0 || len(setArgs) > 0 {
|
||||
if opts.CalleePath == "" {
|
||||
return nil, fmt.Errorf("bug: opts.CalleePath was nil: f=%s, opts=%v", f, opts)
|
||||
}
|
||||
storage := state.NewStorage(opts.CalleePath, ld.logger, ld.fs)
|
||||
envld := state.NewEnvironmentValuesLoader(storage, ld.fs, ld.logger, ld.remote)
|
||||
handler := state.MissingFileHandlerError
|
||||
vals, err := envld.LoadEnvironmentValues(&handler, args, environment.New(ld.env), ld.env)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
|
||||
overrodeEnv = &environment.Environment{
|
||||
Name: ld.env,
|
||||
Values: map[string]any{},
|
||||
CLIOverrides: map[string]any{},
|
||||
}
|
||||
|
||||
if opts.Environment.OverrideValuesAreCLI {
|
||||
overrodeEnv = &environment.Environment{
|
||||
Name: ld.env,
|
||||
CLIOverrides: vals,
|
||||
// --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)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
} else {
|
||||
overrodeEnv = &environment.Environment{
|
||||
Name: ld.env,
|
||||
Values: vals,
|
||||
overrodeEnv.Values = fileVals
|
||||
}
|
||||
|
||||
// --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)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
overrodeEnv.CLIOverrides = setVals
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -30,7 +30,7 @@ func (o LoadOpts) DeepCopy() LoadOpts {
|
|||
panic(err)
|
||||
}
|
||||
|
||||
new.Environment.OverrideValuesAreCLI = o.Environment.OverrideValuesAreCLI
|
||||
new.Environment.OverrideCLISetValues = o.Environment.OverrideCLISetValues
|
||||
|
||||
return new
|
||||
}
|
||||
|
|
|
|||
|
|
@ -21,16 +21,16 @@ func TestLoadOptsDeepCopy(t *testing.T) {
|
|||
require.Equal(t, lOld, lNew, "DeepCopy should return a copy of the LoadOpts struct")
|
||||
}
|
||||
|
||||
// TestLoadOptsDeepCopyPreservesOverrideValuesAreCLI verifies that DeepCopy
|
||||
// preserves the OverrideValuesAreCLI flag which is tagged yaml:"-".
|
||||
func TestLoadOptsDeepCopyPreservesOverrideValuesAreCLI(t *testing.T) {
|
||||
// TestLoadOptsDeepCopyPreservesOverrideCLISetValues verifies that DeepCopy
|
||||
// preserves the OverrideCLISetValues field which is tagged yaml:"-".
|
||||
func TestLoadOptsDeepCopyPreservesOverrideCLISetValues(t *testing.T) {
|
||||
lOld := LoadOpts{
|
||||
Selectors: []string{"test"},
|
||||
CalleePath: "test",
|
||||
}
|
||||
lOld.Environment.OverrideValuesAreCLI = true
|
||||
lOld.Environment.OverrideCLISetValues = []any{map[string]any{"key": "value"}}
|
||||
|
||||
lNew := lOld.DeepCopy()
|
||||
|
||||
require.True(t, lNew.Environment.OverrideValuesAreCLI, "DeepCopy should preserve OverrideValuesAreCLI flag")
|
||||
require.Equal(t, lOld.Environment.OverrideCLISetValues, lNew.Environment.OverrideCLISetValues, "DeepCopy should preserve OverrideCLISetValues field")
|
||||
}
|
||||
|
|
|
|||
|
|
@ -156,7 +156,7 @@ type SubHelmfileSpec struct {
|
|||
// SubhelmfileEnvironmentSpec is the environment spec for a subhelmfile
|
||||
type SubhelmfileEnvironmentSpec struct {
|
||||
OverrideValues []any `yaml:"values,omitempty"`
|
||||
OverrideValuesAreCLI bool `yaml:"-"`
|
||||
OverrideCLISetValues []any `yaml:"-"` // CLI --state-values-set values only, merged element-by-element
|
||||
}
|
||||
|
||||
// HelmSpec to defines helmDefault values
|
||||
|
|
|
|||
Loading…
Reference in New Issue