Files
helmfile/pkg/app/desired_state_file_loader.go
yxxhero ca8fc293e9 fix: helmBinary setting ignored in multi-document YAML files (#2414)
* fix: helmBinary setting ignored in multi-document YAML files

The helmBinary setting in helmfile.yaml was being ignored when using
multi-document YAML files (files with --- separators).

Root Cause:
When processing multi-document YAML files, the load() function splits
the file into parts and processes each part separately. Each part was
calling applyDefaultsAndOverrides() which would set an empty helmBinary
to the default 'helm'. When merging parts, the default value from a
later part would override the correct value from an earlier part.

Fix:
- Added a new applyDefaults parameter to ParseAndLoad() to control when
  defaults are applied
- Modified rawLoad() to pass applyDefaults=false when processing
  individual parts
- Added a call to ApplyDefaultsAndOverrides() after all parts are merged
  to apply defaults once on the final merged state
- Exported ApplyDefaultsAndOverrides() method for use by the app package

Fixes: #2319
Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: update comment per PR review

Change 're-apply' to 'apply' since defaults are never applied during
part processing (applyDefaults=false is passed), so this is the first
and only time defaults are applied to the merged state.

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: clarify applyDefaults logic in test LoadFile callbacks

Add explicit applyDefaults variable with comment explaining why it
equals evaluateBases: base files shouldn't apply defaults, only the
main file should after all parts/bases are merged.

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: address PR review comments

- Remove applyDefaults parameter from rawLoad() since it's always false
- Add regression test for multi-document YAML with helmBinary (issue #2319)

Signed-off-by: yxxhero <aiopsclub@163.com>

* test: add integration test for helmBinary in multi-document YAML

Add TestHelmBinaryPreservedInMultiDocumentYAML that exercises the full
loadDesiredStateFromYaml path to ensure helmBinary from the first
document is preserved when merging multi-document YAML files.

This is a regression test at the load() orchestration level for issue #2319.

Signed-off-by: yxxhero <aiopsclub@163.com>

---------

Signed-off-by: yxxhero <aiopsclub@163.com>
2026-02-20 22:12:38 +08:00

333 lines
9.5 KiB
Go

package app
import (
"bytes"
"errors"
"fmt"
"os"
"path/filepath"
"slices"
"dario.cat/mergo"
"github.com/helmfile/vals"
"go.uber.org/zap"
"github.com/helmfile/helmfile/pkg/environment"
"github.com/helmfile/helmfile/pkg/envvar"
"github.com/helmfile/helmfile/pkg/filesystem"
"github.com/helmfile/helmfile/pkg/helmexec"
"github.com/helmfile/helmfile/pkg/policy"
"github.com/helmfile/helmfile/pkg/remote"
"github.com/helmfile/helmfile/pkg/state"
)
const (
DefaultHelmBinary = state.DefaultHelmBinary
DefaultKustomizeBinary = state.DefaultKustomizeBinary
)
type desiredStateLoader struct {
overrideKubeContext string
overrideHelmBinary string
overrideKustomizeBinary string
enableLiveOutput bool
env string
namespace string
chart string
fs *filesystem.FileSystem
baseDir string // Base directory for resolving relative paths, empty means use cwd
getHelm func(*state.HelmState) (helmexec.Interface, error)
remote *remote.Remote
logger *zap.SugaredLogger
valsRuntime vals.Evaluator
lockFilePath string
}
func (ld *desiredStateLoader) Load(f string, opts LoadOpts) (*state.HelmState, error) {
var overrodeEnv *environment.Environment
args := opts.Environment.OverrideValues
if len(args) > 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,
CLIOverrides: vals,
}
}
// Resolve file path relative to baseDir if provided
var dir, file string
if ld.baseDir != "" {
// If baseDir is set, resolve all paths relative to it
if !filepath.IsAbs(f) {
f = filepath.Join(ld.baseDir, f)
}
dir = filepath.Dir(f)
file = filepath.Base(f)
} else {
// Use original behavior
dir = filepath.Dir(f)
file = filepath.Base(f)
}
st, err := ld.loadFileWithOverrides(nil, overrodeEnv, dir, file, true)
if err != nil {
return nil, err
}
if opts.Reverse {
st.Reverse()
}
if ld.overrideKubeContext != "" {
if st.OverrideKubeContext != "" {
return nil, errors.New("err: Cannot use option --kube-context and set attribute kubeContext.")
}
st.OverrideKubeContext = ld.overrideKubeContext
// HelmDefaults.KubeContext is also overridden in here
// to set default release value properly.
st.HelmDefaults.KubeContext = ld.overrideKubeContext
}
if ld.namespace != "" {
if st.OverrideNamespace != "" {
return nil, errors.New("err: Cannot use option --namespace and set attribute namespace.")
}
st.OverrideNamespace = ld.namespace
}
if ld.chart != "" {
if st.OverrideChart != "" {
return nil, errors.New("err: Cannot use option --chart and set attribute chart.")
}
st.OverrideChart = ld.chart
}
return st, nil
}
func (ld *desiredStateLoader) loadFile(inheritedEnv, overrodeEnv *environment.Environment, baseDir, file string, evaluateBases bool) (*state.HelmState, error) {
path, err := ld.remote.Locate(file, "states")
if err != nil {
return nil, fmt.Errorf("locate: %v", err)
}
if file != path {
ld.logger.Debugf("fetched remote \"%s\" to local cache \"%s\" and loading the latter...", file, path)
}
file = path
return ld.loadFileWithOverrides(inheritedEnv, overrodeEnv, baseDir, file, evaluateBases)
}
func (ld *desiredStateLoader) loadFileWithOverrides(inheritedEnv, overrodeEnv *environment.Environment, baseDir, file string, evaluateBases bool) (*state.HelmState, error) {
var f string
if filepath.IsAbs(file) {
f = file
} else {
f = filepath.Join(baseDir, file)
}
fileBytes, err := ld.fs.ReadFile(f)
if err != nil {
return nil, err
}
self, err := ld.load(
inheritedEnv,
overrodeEnv,
baseDir,
f,
fileBytes,
evaluateBases,
)
if err != nil {
return nil, err
}
for i, h := range self.Helmfiles {
if h.Path == f {
return nil, fmt.Errorf("%s contains a recursion into the same sub-helmfile at helmfiles[%d]", f, i)
}
if h.Path == "." {
return nil, fmt.Errorf("%s contains a recursion into the the directory containing this helmfile at helmfiles[%d]", f, i)
}
}
return self, nil
}
func (a *desiredStateLoader) underlying() *state.StateCreator {
c := state.NewCreator(a.logger, a.fs, a.valsRuntime, a.getHelm, a.overrideHelmBinary, a.overrideKustomizeBinary, a.remote, a.enableLiveOutput, a.lockFilePath)
c.LoadFile = a.loadFile
return c
}
func (a *desiredStateLoader) rawLoad(yaml []byte, baseDir, file string, evaluateBases bool, env, overrodeEnv *environment.Environment) (*state.HelmState, error) {
var st *state.HelmState
var err error
merged, err := env.Merge(overrodeEnv)
if err != nil {
return nil, err
}
// applyDefaults is always false here - defaults are applied after all parts are merged
st, err = a.underlying().ParseAndLoad(yaml, baseDir, file, a.env, false, evaluateBases, false, merged, nil)
if err != nil {
return nil, err
}
helmfiles, err := st.ExpandedHelmfiles()
if err != nil {
return nil, err
}
st.Helmfiles = helmfiles
return st, nil
}
func (ld *desiredStateLoader) load(env, overrodeEnv *environment.Environment, baseDir, filename string, content []byte, evaluateBases bool) (*state.HelmState, error) {
// Allows part-splitting to work with CLRF-ed content
normalizedContent := bytes.ReplaceAll(content, []byte("\r\n"), []byte("\n"))
isStrict, err := policy.Checker(filename, normalizedContent)
if err != nil {
if isStrict {
return nil, err
}
ld.logger.Warnf("WARNING: %v", err)
}
parts := bytes.Split(normalizedContent, []byte("\n---\n"))
hasEnv := env != nil || overrodeEnv != nil
var finalState *state.HelmState
for i, part := range parts {
id := fmt.Sprintf("%s.part.%d", filename, i)
var rawContent []byte
shouldRender := filepath.Ext(filename) == ".gotmpl" || os.Getenv(envvar.RenderYaml) == "true"
if shouldRender {
var yamlBuf *bytes.Buffer
var err error
if env == nil && overrodeEnv == nil {
yamlBuf, err = ld.renderTemplatesToYaml(baseDir, id, part)
if err != nil {
return nil, fmt.Errorf("error during %s parsing: %v", id, err)
}
} else {
yamlBuf, err = ld.renderTemplatesToYamlWithEnv(baseDir, id, part, env, overrodeEnv)
if err != nil {
return nil, fmt.Errorf("error during %s parsing: %v", id, err)
}
}
rawContent = yamlBuf.Bytes()
} else {
rawContent = part
}
currentState, err := ld.rawLoad(
rawContent,
baseDir,
filename,
evaluateBases,
env,
overrodeEnv,
)
if err != nil {
return nil, err
}
if finalState == nil {
finalState = currentState
} else {
if err := mergo.Merge(&finalState.ReleaseSetSpec, &currentState.ReleaseSetSpec, mergo.WithOverride); err != nil {
return nil, err
}
finalState.RenderedValues = currentState.RenderedValues
}
if len(finalState.HelmDefaults.PostRendererArgs) > 0 {
for i := range finalState.Releases {
if len(finalState.Releases[i].PostRendererArgs) == 0 {
finalState.Releases[i].PostRendererArgs = finalState.HelmDefaults.PostRendererArgs
}
}
finalState.HelmDefaults.PostRendererArgs = nil
}
env = &finalState.Env
ld.logger.Debugf("merged environment: %v", env)
if len(finalState.Environments) == 0 {
continue
}
// At this point, we are sure that the env has been
// read from the vanilla or rendered YAML document.
// We can now check if the env is defined in it and fail accordingly.
// See https://github.com/helmfile/helmfile/issues/913
// We defer the missing env detection and failure until
// all the helmfile parts are loaded and merged.
// Otherwise, any single helmfile part missing the env would fail the whole helmfile run.
// That's problematic, because each helmfile part is supposed to be incomplete, and
// they become complete only after merging all the parts.
// See https://github.com/helmfile/helmfile/issues/807 for the rationale of this.
if _, ok := finalState.Environments[env.Name]; evaluateBases && env.Name != state.DefaultEnv && !ok {
return nil, &state.StateLoadError{
Msg: fmt.Sprintf("failed to read %s", finalState.FilePath),
Cause: &state.UndefinedEnvError{Env: env.Name},
}
}
}
// After all parts are merged, apply defaults and overrides to ensure
// that values from earlier parts (like helmBinary) are preserved correctly
// in the merged state.
// See https://github.com/helmfile/helmfile/issues/2319
if evaluateBases {
ld.underlying().ApplyDefaultsAndOverrides(finalState)
}
// If environments are not defined in the helmfile at all although the env is specified,
// it's a missing env situation. Let's fail.
if len(finalState.Environments) == 0 && evaluateBases && !hasEnv && env.Name != state.DefaultEnv {
return nil, &state.StateLoadError{
Msg: fmt.Sprintf("failed to read %s", finalState.FilePath),
Cause: &state.UndefinedEnvError{Env: env.Name},
}
}
// Validate updateStrategy value if set in the releases
for i := range finalState.Releases {
if finalState.Releases[i].UpdateStrategy != "" {
if !slices.Contains(state.ValidUpdateStrategyValues, finalState.Releases[i].UpdateStrategy) {
return nil, &state.StateLoadError{
Msg: fmt.Sprintf("failed to read %s", finalState.FilePath),
Cause: &state.InvalidUpdateStrategyError{UpdateStrategy: finalState.Releases[i].UpdateStrategy},
}
}
}
}
finalState.OrginReleases = finalState.Releases
return finalState, nil
}