From 6b6baa223c2a70e2b7221e691c78acacf1c69290 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 22 Oct 2025 01:07:58 +0000 Subject: [PATCH] Fix helmBinary and kustomizeBinary being ignored when using bases - Add mergo.WithOverride to merge operations for proper precedence - Move default binary setting after base loading - Add comprehensive tests for various base scenarios Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com> --- pkg/state/create.go | 22 +++-- pkg/state/create_test.go | 206 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 221 insertions(+), 7 deletions(-) diff --git a/pkg/state/create.go b/pkg/state/create.go index 97e91823..606134fd 100644 --- a/pkg/state/create.go +++ b/pkg/state/create.go @@ -116,11 +116,19 @@ func (c *StateCreator) Parse(content []byte, baseDir, file string) (*HelmState, return nil, &StateLoadError{fmt.Sprintf("failed to read %s: reading document at index %d", file, i), err} } - if err := mergo.Merge(&state, &intermediate, mergo.WithAppendSlice); err != nil { + if err := mergo.Merge(&state, &intermediate, mergo.WithAppendSlice, mergo.WithOverride); err != nil { return nil, &StateLoadError{fmt.Sprintf("failed to read %s: merging document at index %d", file, i), err} } } + state.logger = c.logger + state.valsRuntime = c.valsRuntime + + return &state, nil +} + +// applyDefaultsAndOverrides applies default binary paths and command-line overrides +func (c *StateCreator) applyDefaultsAndOverrides(state *HelmState) { if c.overrideHelmBinary != "" && c.overrideHelmBinary != DefaultHelmBinary { state.DefaultHelmBinary = c.overrideHelmBinary } else if state.DefaultHelmBinary == "" { @@ -134,11 +142,6 @@ func (c *StateCreator) Parse(content []byte, baseDir, file string) (*HelmState, // Let `helmfile --kustomize-binary ""` not break this helmfile run state.DefaultKustomizeBinary = DefaultKustomizeBinary } - - state.logger = c.logger - state.valsRuntime = c.valsRuntime - - return &state, nil } // LoadEnvValues loads environment values files relative to the `baseDir` @@ -181,6 +184,11 @@ func (c *StateCreator) ParseAndLoad(content []byte, baseDir, file string, envNam if err != nil { return nil, err } + + // Apply default binaries and command-line overrides only for the main helmfile + // after loading and merging all bases. This ensures that values from bases are + // properly respected and that later bases/documents can override earlier ones. + c.applyDefaultsAndOverrides(state) } state, err = c.LoadEnvValues(state, envName, failOnMissingEnv, envValues, overrode) @@ -216,7 +224,7 @@ func (c *StateCreator) loadBases(envValues, overrodeEnv *environment.Environment layers = append(layers, st) for i := 1; i < len(layers); i++ { - if err := mergo.Merge(layers[0], layers[i], mergo.WithAppendSlice); err != nil { + if err := mergo.Merge(layers[0], layers[i], mergo.WithAppendSlice, mergo.WithOverride); err != nil { return nil, err } } diff --git a/pkg/state/create_test.go b/pkg/state/create_test.go index 29ac5ca8..1aa016eb 100644 --- a/pkg/state/create_test.go +++ b/pkg/state/create_test.go @@ -1,6 +1,7 @@ package state import ( + "fmt" "path/filepath" "reflect" "testing" @@ -525,3 +526,208 @@ releaseContext: t.Errorf("unexpected values: expected=%v, actual=%v", expectedValues, actualValues) } } + +// TestHelmBinaryInBases tests that helmBinary and kustomizeBinary settings +// from bases are properly merged with later values overriding earlier ones + +// TestHelmBinaryInBases tests that helmBinary and kustomizeBinary settings +// from bases are properly merged with later values overriding earlier ones +func TestHelmBinaryInBases(t *testing.T) { +tests := []struct { +name string +files map[string]string +mainFile string +expectedHelmBinary string +expectedKustomizeBinary string +}{ +{ +name: "helmBinary in second base should be used", +files: map[string]string{ +"/path/to/helmfile.yaml": `bases: + - ./bases/env.yaml +--- +bases: + - ./bases/repos.yaml +--- +bases: + - ./bases/releases.yaml +`, +"/path/to/bases/env.yaml": `environments: + default: + values: + - key: value1 +`, +"/path/to/bases/repos.yaml": `repositories: + - name: stable + url: https://charts.helm.sh/stable +helmBinary: /path/to/custom/helm +`, +"/path/to/bases/releases.yaml": `releases: + - name: myapp + chart: stable/nginx +`, +}, +mainFile: "/path/to/helmfile.yaml", +expectedHelmBinary: "/path/to/custom/helm", +expectedKustomizeBinary: DefaultKustomizeBinary, +}, +{ +name: "helmBinary in main file after bases should override", +files: map[string]string{ +"/path/to/helmfile.yaml": `bases: + - ./bases/env.yaml +--- +bases: + - ./bases/repos.yaml +--- +bases: + - ./bases/releases.yaml +helmBinary: /path/to/main/helm +`, +"/path/to/bases/env.yaml": `environments: + default: + values: + - key: value1 +`, +"/path/to/bases/repos.yaml": `repositories: + - name: stable + url: https://charts.helm.sh/stable +helmBinary: /path/to/base/helm +`, +"/path/to/bases/releases.yaml": `releases: + - name: myapp + chart: stable/nginx +`, +}, +mainFile: "/path/to/helmfile.yaml", +expectedHelmBinary: "/path/to/main/helm", +expectedKustomizeBinary: DefaultKustomizeBinary, +}, +{ +name: "helmBinary in main file between bases should override earlier bases", +files: map[string]string{ +"/path/to/helmfile.yaml": `bases: + - ./bases/env.yaml +--- +bases: + - ./bases/repos.yaml +helmBinary: /path/to/middle/helm +--- +bases: + - ./bases/releases.yaml +`, +"/path/to/bases/env.yaml": `environments: + default: + values: + - key: value1 +`, +"/path/to/bases/repos.yaml": `repositories: + - name: stable + url: https://charts.helm.sh/stable +helmBinary: /path/to/base/helm +`, +"/path/to/bases/releases.yaml": `releases: + - name: myapp + chart: stable/nginx +`, +}, +mainFile: "/path/to/helmfile.yaml", +expectedHelmBinary: "/path/to/middle/helm", +expectedKustomizeBinary: DefaultKustomizeBinary, +}, +{ +name: "kustomizeBinary in base should be used", +files: map[string]string{ +"/path/to/helmfile.yaml": `bases: + - ./bases/base.yaml +`, +"/path/to/bases/base.yaml": `kustomizeBinary: /path/to/custom/kustomize +releases: + - name: myapp + chart: mychart +`, +}, +mainFile: "/path/to/helmfile.yaml", +expectedHelmBinary: DefaultHelmBinary, +expectedKustomizeBinary: "/path/to/custom/kustomize", +}, +{ +name: "both helmBinary and kustomizeBinary in different bases", +files: map[string]string{ +"/path/to/helmfile.yaml": `bases: + - ./bases/helm.yaml +--- +bases: + - ./bases/kustomize.yaml +`, +"/path/to/bases/helm.yaml": `helmBinary: /path/to/custom/helm +`, +"/path/to/bases/kustomize.yaml": `kustomizeBinary: /path/to/custom/kustomize +`, +}, +mainFile: "/path/to/helmfile.yaml", +expectedHelmBinary: "/path/to/custom/helm", +expectedKustomizeBinary: "/path/to/custom/kustomize", +}, +{ +name: "later base overrides earlier base for helmBinary", +files: map[string]string{ +"/path/to/helmfile.yaml": `bases: + - ./bases/first.yaml +--- +bases: + - ./bases/second.yaml +`, +"/path/to/bases/first.yaml": `helmBinary: /path/to/first/helm +`, +"/path/to/bases/second.yaml": `helmBinary: /path/to/second/helm +`, +}, +mainFile: "/path/to/helmfile.yaml", +expectedHelmBinary: "/path/to/second/helm", +expectedKustomizeBinary: DefaultKustomizeBinary, +}, +} + +for _, tt := range tests { +t.Run(tt.name, func(t *testing.T) { +testFs := testhelper.NewTestFs(tt.files) +if testFs.Cwd == "" { +testFs.Cwd = "/" +} + +r := remote.NewRemote(logger, testFs.Cwd, testFs.ToFileSystem()) +creator := NewCreator(logger, testFs.ToFileSystem(), nil, nil, "", "", r, false, "") + +// Set up LoadFile for recursive base loading +creator.LoadFile = func(inheritedEnv, overrodeEnv *environment.Environment, baseDir, file string, evaluateBases bool) (*HelmState, error) { +path := filepath.Join(baseDir, file) +content, ok := tt.files[path] +if !ok { +return nil, fmt.Errorf("file not found: %s", path) +} +return creator.ParseAndLoad([]byte(content), filepath.Dir(path), path, DefaultEnv, true, evaluateBases, inheritedEnv, overrodeEnv) +} + +yamlContent, ok := tt.files[tt.mainFile] +if !ok { +t.Fatalf("no file named %q registered", tt.mainFile) +} + +state, err := creator.ParseAndLoad([]byte(yamlContent), filepath.Dir(tt.mainFile), tt.mainFile, DefaultEnv, true, true, nil, nil) +if err != nil { +t.Fatalf("unexpected error: %v", err) +} + +if state.DefaultHelmBinary != tt.expectedHelmBinary { +t.Errorf("helmBinary mismatch: expected=%s, actual=%s", +tt.expectedHelmBinary, state.DefaultHelmBinary) +} + +if state.DefaultKustomizeBinary != tt.expectedKustomizeBinary { +t.Errorf("kustomizeBinary mismatch: expected=%s, actual=%s", +tt.expectedKustomizeBinary, state.DefaultKustomizeBinary) +} +}) +} +}