From 69bed171abeb0451bd28977a18b302c46d2787a3 Mon Sep 17 00:00:00 2001 From: Aditya Menon Date: Thu, 26 Feb 2026 05:17:39 +0530 Subject: [PATCH] fix: use absolute baseDir in sequential helmfiles for correct values path resolution (#2425) * fix: use absolute baseDir in sequential helmfiles for correct values path resolution (#2424) PR #2410 introduced a regression where a relative directory was passed as baseDir instead of an absolute one, causing values and secrets file paths to resolve incorrectly when using --sequential-helmfiles with helmfile.d/. Signed-off-by: Aditya Menon * fix: mirror reporter's bases/templates/inherit setup in issue-2424 integration test Signed-off-by: Aditya Menon --------- Signed-off-by: Aditya Menon --- pkg/app/app.go | 9 +- pkg/app/app_sequential_test.go | 85 +++++++++++++++++++ test/integration/run.sh | 1 + .../issue-2424-sequential-values-paths.sh | 50 +++++++++++ .../input/bases/defaults.yaml.gotmpl | 2 + .../input/bases/environments.yaml | 2 + .../input/bases/templates.yaml | 5 ++ .../input/config/default/values.yaml | 10 +++ .../input/helmfile.d/01-app.yaml.gotmpl | 13 +++ .../input/helmfile.d/02-other.yaml.gotmpl | 13 +++ 10 files changed, 184 insertions(+), 6 deletions(-) create mode 100644 test/integration/test-cases/issue-2424-sequential-values-paths.sh create mode 100644 test/integration/test-cases/issue-2424-sequential-values-paths/input/bases/defaults.yaml.gotmpl create mode 100644 test/integration/test-cases/issue-2424-sequential-values-paths/input/bases/environments.yaml create mode 100644 test/integration/test-cases/issue-2424-sequential-values-paths/input/bases/templates.yaml create mode 100644 test/integration/test-cases/issue-2424-sequential-values-paths/input/config/default/values.yaml create mode 100644 test/integration/test-cases/issue-2424-sequential-values-paths/input/helmfile.d/01-app.yaml.gotmpl create mode 100644 test/integration/test-cases/issue-2424-sequential-values-paths/input/helmfile.d/02-other.yaml.gotmpl diff --git a/pkg/app/app.go b/pkg/app/app.go index bd1e2fc0..459d95a3 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -1080,13 +1080,10 @@ func (a *App) visitStatesWithContext(fileOrDir string, defOpts LoadOpts, converg var loadErr error if useBaseDir { - // Multi-file sequential: use baseDir for path resolution + // Multi-file sequential: use absolute baseDir for path resolution // instead of os.Chdir to avoid breaking relative env var paths. - baseDir := dir - if dir == "." { - baseDir = "" - } - st, loadErr = a.loadDesiredStateFromYamlWithBaseDir(file, baseDir, opts) + // Must use absd (absolute dir) to correctly resolve relative values/secrets paths. + st, loadErr = a.loadDesiredStateFromYamlWithBaseDir(file, absd, opts) } else { // Single file: CWD is set by within(), load without baseDir st, loadErr = a.loadDesiredStateFromYaml(file, opts) diff --git a/pkg/app/app_sequential_test.go b/pkg/app/app_sequential_test.go index 8dc28618..101a4b5c 100644 --- a/pkg/app/app_sequential_test.go +++ b/pkg/app/app_sequential_test.go @@ -4,6 +4,7 @@ import ( "bytes" "fmt" "reflect" + "strings" "testing" "github.com/helmfile/vals" @@ -415,3 +416,87 @@ releases: t.Errorf("expected error to contain 'simulated converge failure', got: %v", err) } } + +// TestSequentialHelmfilesValuesFileResolution verifies that relative values file +// paths (e.g., ../config/myapp/values.yaml) are resolved correctly in sequential +// mode. This is a regression test for issue #2424 where a relative baseDir caused +// values paths to be resolved incorrectly. +func TestSequentialHelmfilesValuesFileResolution(t *testing.T) { + files := map[string]string{ + "/path/to/helmfile.d/01-app.yaml": ` +releases: +- name: myapp + chart: stable/myapp + namespace: default + values: + - ../config/myapp/values.yaml +`, + "/path/to/helmfile.d/02-other.yaml": ` +releases: +- name: other + chart: stable/other + namespace: default +`, + "/path/to/config/myapp/values.yaml": ` +replicaCount: 3 +`, + } + + testFs := testhelper.NewTestFs(files) + + valsRuntime, err := vals.New(vals.Options{CacheSize: 32}) + if err != nil { + t.Fatalf("unexpected error creating vals runtime: %v", err) + } + + app := &App{ + OverrideHelmBinary: DefaultHelmBinary, + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Env: "default", + Logger: newAppTestLogger(), + valsRuntime: valsRuntime, + FileOrDir: "/path/to/helmfile.d", + SequentialHelmfiles: true, + } + + app = injectFs(app, testFs) + expectNoCallsToHelm(app) + + var stateFilePaths []string + captureState := func(run *Run) (bool, []error) { + stateFilePaths = append(stateFilePaths, run.state.FilePath) + return false, []error{} + } + + err = app.ForEachState( + captureState, + false, + SetFilter(true), + ) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Verify that state FilePaths are absolute (which means basePath is also + // absolute, ensuring relative values paths like ../config/myapp/values.yaml + // resolve correctly). Before the fix, a relative baseDir was passed causing + // values path resolution to fail. + for _, fp := range stateFilePaths { + if !strings.HasPrefix(fp, "/") { + t.Errorf("expected absolute FilePath, got relative: %s", fp) + } + } + + // Verify the values file was found and can be read (proving correct path resolution). + // With an absolute basePath, ../config/myapp/values.yaml resolves to + // /path/to/config/myapp/values.yaml which exists in our test filesystem. + valuesPath := "/path/to/config/myapp/values.yaml" + content, err := testFs.ToFileSystem().ReadFile(valuesPath) + if err != nil { + t.Fatalf("values file should be readable at %s: %v", valuesPath, err) + } + if !strings.Contains(string(content), "replicaCount") { + t.Errorf("values file content mismatch: %s", string(content)) + } +} diff --git a/test/integration/run.sh b/test/integration/run.sh index 7661ae70..bd2d93b6 100755 --- a/test/integration/run.sh +++ b/test/integration/run.sh @@ -133,6 +133,7 @@ ${kubectl} create namespace ${test_ns} || fail "Could not create namespace ${tes . ${dir}/test-cases/issue-2409-sequential-kubecontext.sh . ${dir}/test-cases/issue-2269.sh . ${dir}/test-cases/issue-2418.sh +. ${dir}/test-cases/issue-2424-sequential-values-paths.sh # ALL DONE ----------------------------------------------------------------------------------------------------------- diff --git a/test/integration/test-cases/issue-2424-sequential-values-paths.sh b/test/integration/test-cases/issue-2424-sequential-values-paths.sh new file mode 100644 index 00000000..242b9f1d --- /dev/null +++ b/test/integration/test-cases/issue-2424-sequential-values-paths.sh @@ -0,0 +1,50 @@ +# Issue #2424: Test that --sequential-helmfiles resolves relative values file paths correctly +# https://github.com/helmfile/helmfile/issues/2424 +# +# This test replicates a regression introduced in #2410 where a relative baseDir +# was passed instead of an absolute one, breaking values/secrets path resolution +# when using --sequential-helmfiles with helmfile.d/ containing multiple files. +# +# The setup mirrors the reporter's structure: +# - helmfile.d/ with two .yaml.gotmpl files (to trigger multi-file sequential mode) +# - bases/ with environments, defaults, and templates (values: ../config/{{ .Release.Namespace }}/values.yaml) +# - Releases using inherit: [template: default] to pick up values via template +# - config//values.yaml resolved via the template's relative path + +issue_2424_input_dir="${cases_dir}/issue-2424-sequential-values-paths/input/helmfile.d" +issue_2424_tmp=$(mktemp -d) + +test_start "issue 2424 sequential helmfiles values path resolution" + +# Run template with --sequential-helmfiles to verify values paths resolve correctly +info "Running helmfile --sequential-helmfiles template" +${helmfile} --sequential-helmfiles -f ${issue_2424_input_dir} template \ + > ${issue_2424_tmp}/sequential.log 2>&1 \ + || fail "\"helmfile --sequential-helmfiles template\" shouldn't fail" + +# Verify the values from config/default/values.yaml appear in template output +grep -q "app-configmap-2424" ${issue_2424_tmp}/sequential.log \ + || fail "values from ../config/default/values.yaml should be resolved in sequential mode" + +grep -q "issue.*2424" ${issue_2424_tmp}/sequential.log \ + || fail "data from values file should appear in template output" + +# Verify both releases are processed +grep -q "test-app-2424" ${issue_2424_tmp}/sequential.log \ + || fail "release test-app-2424 from 01-app.yaml.gotmpl should be in output" + +grep -q "test-other-2424" ${issue_2424_tmp}/sequential.log \ + || fail "release test-other-2424 from 02-other.yaml.gotmpl should be in output" + +# Run without --sequential-helmfiles and confirm same result +info "Running helmfile template without --sequential-helmfiles" +${helmfile} -f ${issue_2424_input_dir} template \ + > ${issue_2424_tmp}/parallel.log 2>&1 \ + || fail "\"helmfile template\" (parallel) shouldn't fail" + +grep -q "app-configmap-2424" ${issue_2424_tmp}/parallel.log \ + || fail "values should resolve in parallel mode too" + +rm -rf ${issue_2424_tmp} + +test_pass "issue 2424 sequential helmfiles values path resolution" diff --git a/test/integration/test-cases/issue-2424-sequential-values-paths/input/bases/defaults.yaml.gotmpl b/test/integration/test-cases/issue-2424-sequential-values-paths/input/bases/defaults.yaml.gotmpl new file mode 100644 index 00000000..5dd90139 --- /dev/null +++ b/test/integration/test-cases/issue-2424-sequential-values-paths/input/bases/defaults.yaml.gotmpl @@ -0,0 +1,2 @@ +helmDefaults: + wait: false diff --git a/test/integration/test-cases/issue-2424-sequential-values-paths/input/bases/environments.yaml b/test/integration/test-cases/issue-2424-sequential-values-paths/input/bases/environments.yaml new file mode 100644 index 00000000..f737d6af --- /dev/null +++ b/test/integration/test-cases/issue-2424-sequential-values-paths/input/bases/environments.yaml @@ -0,0 +1,2 @@ +environments: + default: diff --git a/test/integration/test-cases/issue-2424-sequential-values-paths/input/bases/templates.yaml b/test/integration/test-cases/issue-2424-sequential-values-paths/input/bases/templates.yaml new file mode 100644 index 00000000..d6204f7b --- /dev/null +++ b/test/integration/test-cases/issue-2424-sequential-values-paths/input/bases/templates.yaml @@ -0,0 +1,5 @@ +templates: + default: + missingFileHandler: Error + values: + - ../config/{{ .Release.Namespace }}/values.yaml diff --git a/test/integration/test-cases/issue-2424-sequential-values-paths/input/config/default/values.yaml b/test/integration/test-cases/issue-2424-sequential-values-paths/input/config/default/values.yaml new file mode 100644 index 00000000..afcde2ff --- /dev/null +++ b/test/integration/test-cases/issue-2424-sequential-values-paths/input/config/default/values.yaml @@ -0,0 +1,10 @@ +templates: +- | + apiVersion: v1 + kind: ConfigMap + metadata: + name: app-configmap-2424 + namespace: {{ .Release.Namespace }} + data: + issue: "2424" + resolved: "true" diff --git a/test/integration/test-cases/issue-2424-sequential-values-paths/input/helmfile.d/01-app.yaml.gotmpl b/test/integration/test-cases/issue-2424-sequential-values-paths/input/helmfile.d/01-app.yaml.gotmpl new file mode 100644 index 00000000..3ade90a3 --- /dev/null +++ b/test/integration/test-cases/issue-2424-sequential-values-paths/input/helmfile.d/01-app.yaml.gotmpl @@ -0,0 +1,13 @@ +bases: + - ../bases/environments.yaml +--- +bases: + - ../bases/defaults.yaml.gotmpl + - ../bases/templates.yaml +--- +releases: +- name: test-app-2424 + chart: ../../../../charts/raw + namespace: default + inherit: + - template: default diff --git a/test/integration/test-cases/issue-2424-sequential-values-paths/input/helmfile.d/02-other.yaml.gotmpl b/test/integration/test-cases/issue-2424-sequential-values-paths/input/helmfile.d/02-other.yaml.gotmpl new file mode 100644 index 00000000..e66a8cfb --- /dev/null +++ b/test/integration/test-cases/issue-2424-sequential-values-paths/input/helmfile.d/02-other.yaml.gotmpl @@ -0,0 +1,13 @@ +bases: + - ../bases/environments.yaml +--- +bases: + - ../bases/defaults.yaml.gotmpl + - ../bases/templates.yaml +--- +releases: +- name: test-other-2424 + chart: ../../../../charts/raw + namespace: default + inherit: + - template: default