Fix helmBinary and kustomizeBinary being ignored when using bases (#2228)
* 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> * Fix code formatting in create_test.go Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com> * Remove duplicate comment block in create_test.go Removed duplicate comment lines (530-532) as identified by code review. Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>
This commit is contained in:
Copilot
GitHub
parent
daebbfb0ad
commit
d3908e6a3c
|
|
@ -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
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
package state
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"path/filepath"
|
||||
"reflect"
|
||||
"testing"
|
||||
|
|
@ -525,3 +526,205 @@ 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
|
||||
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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue