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 <amenon@canarytechnologies.com> * fix: mirror reporter's bases/templates/inherit setup in issue-2424 integration test Signed-off-by: Aditya Menon <amenon@canarytechnologies.com> --------- Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>
This commit is contained in:
parent
2b0086b196
commit
69bed171ab
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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))
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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 -----------------------------------------------------------------------------------------------------------
|
||||
|
||||
|
|
|
|||
|
|
@ -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/<namespace>/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"
|
||||
|
|
@ -0,0 +1,2 @@
|
|||
helmDefaults:
|
||||
wait: false
|
||||
|
|
@ -0,0 +1,2 @@
|
|||
environments:
|
||||
default:
|
||||
|
|
@ -0,0 +1,5 @@
|
|||
templates:
|
||||
default:
|
||||
missingFileHandler: Error
|
||||
values:
|
||||
- ../config/{{ .Release.Namespace }}/values.yaml
|
||||
|
|
@ -0,0 +1,10 @@
|
|||
templates:
|
||||
- |
|
||||
apiVersion: v1
|
||||
kind: ConfigMap
|
||||
metadata:
|
||||
name: app-configmap-2424
|
||||
namespace: {{ .Release.Namespace }}
|
||||
data:
|
||||
issue: "2424"
|
||||
resolved: "true"
|
||||
|
|
@ -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
|
||||
|
|
@ -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
|
||||
Loading…
Reference in New Issue