diff --git a/pkg/state/helmx.go b/pkg/state/helmx.go index bf7bd46a..d8edd80a 100644 --- a/pkg/state/helmx.go +++ b/pkg/state/helmx.go @@ -448,7 +448,8 @@ func (st *HelmState) PrepareChartify(helm helmexec.Interface, release *ReleaseSp for _, d := range release.Dependencies { chart := d.Chart - if st.fs.DirectoryExistsAt(chart) { + normalizedChart := normalizeChart(st.basePath, chart) + if st.fs.DirectoryExistsAt(normalizedChart) { var err error // Otherwise helm-dependency-up on the temporary chart generated by chartify ends up errors like: diff --git a/pkg/state/issue_2596_test.go b/pkg/state/issue_2596_test.go new file mode 100644 index 00000000..f8685bce --- /dev/null +++ b/pkg/state/issue_2596_test.go @@ -0,0 +1,138 @@ +package state + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/helmfile/helmfile/pkg/filesystem" +) + +// TestLocalDependencyChartPathNormalization tests that relative chart paths in +// release dependencies (like "../chart") are normalized to absolute paths +// relative to basePath before checking if the directory exists. +// This is a regression test for issue #2596. +// +// Background: When helmfile.d/ contains multiple release files and one release +// has a local chart dependency (chart: ../chart), the dependency chart path was +// passed to DirectoryExistsAt without normalization, causing it to be resolved +// relative to the CWD instead of basePath. This made helmfile fail to detect +// the local chart and instead try to resolve it as a remote repo, resulting in +// "failed reading adhoc dependencies: no helm list entry found for repository". +func TestLocalDependencyChartPathNormalization(t *testing.T) { + tempDir := t.TempDir() + + chartDir := filepath.Join(tempDir, "chart") + require.NoError(t, os.MkdirAll(chartDir, 0755)) + require.NoError(t, os.WriteFile(filepath.Join(chartDir, "Chart.yaml"), []byte(` +apiVersion: v2 +name: test-chart +version: 0.1.0 +`), 0644)) + + helmfileDir := filepath.Join(tempDir, "helmfile.d") + require.NoError(t, os.MkdirAll(helmfileDir, 0755)) + + tests := []struct { + name string + chartPath string + basePath string + expectLocal bool + }{ + { + name: "relative path ../chart normalized from helmfile.d", + chartPath: "../chart", + basePath: helmfileDir, + expectLocal: true, + }, + { + name: "absolute path works unchanged", + chartPath: chartDir, + basePath: helmfileDir, + expectLocal: true, + }, + { + name: "non-existent relative path not detected as local", + chartPath: "../nonexistent", + basePath: helmfileDir, + expectLocal: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + normalizedChart := normalizeChart(tt.basePath, tt.chartPath) + fs := filesystem.DefaultFileSystem() + isLocal := fs.DirectoryExistsAt(normalizedChart) + assert.Equal(t, tt.expectLocal, isLocal, + "normalizeChart(%q, %q) = %q, DirectoryExistsAt = %v, want %v", + tt.basePath, tt.chartPath, normalizedChart, isLocal, tt.expectLocal) + }) + } +} + +// TestDependencyChartPathResolutionWithPrepareChartify verifies that the dependency +// chart path is normalized using basePath before calling DirectoryExistsAt, +// which is the core of the fix for issue #2596. +func TestDependencyChartPathResolutionWithPrepareChartify(t *testing.T) { + tempDir := t.TempDir() + + chartDir := filepath.Join(tempDir, "chart") + require.NoError(t, os.MkdirAll(chartDir, 0755)) + require.NoError(t, os.WriteFile(filepath.Join(chartDir, "Chart.yaml"), []byte(` +apiVersion: v2 +name: test-chart +version: 0.1.0 +`), 0644)) + + helmfileDir := filepath.Join(tempDir, "helmfile.d") + require.NoError(t, os.MkdirAll(helmfileDir, 0755)) + + fs := filesystem.DefaultFileSystem() + + tests := []struct { + name string + depChartPath string + basePath string + expectDetected bool + }{ + { + name: "relative ../chart from helmfile.d detected as local", + depChartPath: "../chart", + basePath: helmfileDir, + expectDetected: true, + }, + { + name: "absolute path detected as local", + depChartPath: chartDir, + basePath: helmfileDir, + expectDetected: true, + }, + { + name: "non-existent relative path not detected", + depChartPath: "../nonexistent", + basePath: helmfileDir, + expectDetected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + normalizedChart := normalizeChart(tt.basePath, tt.depChartPath) + isLocal := fs.DirectoryExistsAt(normalizedChart) + assert.Equal(t, tt.expectDetected, isLocal, + "normalizeChart(%q, %q) = %q, DirectoryExistsAt = %v, want %v", + tt.basePath, tt.depChartPath, normalizedChart, isLocal, tt.expectDetected) + + if tt.expectDetected && !filepath.IsAbs(tt.depChartPath) { + absChart, err := filepath.Abs(filepath.Join(tt.basePath, tt.depChartPath)) + require.NoError(t, err) + assert.Equal(t, absChart, normalizedChart, + "normalized path should match expected absolute path") + } + }) + } +} diff --git a/test/integration/run.sh b/test/integration/run.sh index be0a9c26..4f44bf12 100755 --- a/test/integration/run.sh +++ b/test/integration/run.sh @@ -143,6 +143,7 @@ ${kubectl} create namespace ${test_ns} || fail "Could not create namespace ${tes . ${dir}/test-cases/issue-2424-sequential-values-paths.sh . ${dir}/test-cases/issue-2431.sh . ${dir}/test-cases/issue-2544.sh +. ${dir}/test-cases/issue-2596-local-deps-multiple-files.sh . ${dir}/test-cases/kubedog-tracking.sh # ALL DONE ----------------------------------------------------------------------------------------------------------- diff --git a/test/integration/test-cases/issue-2596-local-deps-multiple-files.sh b/test/integration/test-cases/issue-2596-local-deps-multiple-files.sh new file mode 100644 index 00000000..84a06443 --- /dev/null +++ b/test/integration/test-cases/issue-2596-local-deps-multiple-files.sh @@ -0,0 +1,49 @@ +# Integration test for issue #2596: Local dependencies with multiple release files +# https://github.com/helmfile/helmfile/issues/2596 +# Reproduction: https://github.com/vgivanov/helmfile-deps-local-chart +# +# This test uses the exact same structure as the reproduction repo: +# chart/Chart.yaml +# helmfile.d/release1.yaml (chart: ../chart with dependencies) +# helmfile.d/release2.yaml (chart: ../chart without dependencies) +# +# Before the fix, running `helmfile template` from this directory would fail with: +# "failed reading adhoc dependencies: no helm list entry found for repository" +# because the relative dependency chart path "../chart" was not normalized against +# basePath before calling DirectoryExistsAt. + +issue_2596_input_dir="${cases_dir}/issue-2596-local-deps-multiple-files/input" +issue_2596_tmp="" + +cleanup_issue_2596() { + if [ -n "${issue_2596_tmp}" ] && [ -d "${issue_2596_tmp}" ]; then + rm -rf "${issue_2596_tmp}" + fi +} +trap cleanup_issue_2596 EXIT + +issue_2596_tmp=$(mktemp -d) +helmfile_real="$(pwd)/${helmfile}" + +test_start "issue #2596: local deps with multiple release files" + +info "Testing helmfile template with local chart dependencies across multiple release files" + +cd "${issue_2596_input_dir}" + +${helmfile_real} template > "${issue_2596_tmp}/output.yaml" 2>&1 +result=$? + +cd - > /dev/null + +if [ $result -ne 0 ]; then + cat "${issue_2596_tmp}/output.yaml" + fail "helmfile template with local chart dependencies should not fail" +fi + +info "Local chart dependencies with multiple release files works correctly" + +cleanup_issue_2596 +trap - EXIT + +test_pass "issue #2596: local deps with multiple release files" diff --git a/test/integration/test-cases/issue-2596-local-deps-multiple-files/input/chart/Chart.yaml b/test/integration/test-cases/issue-2596-local-deps-multiple-files/input/chart/Chart.yaml new file mode 100644 index 00000000..df2d97f9 --- /dev/null +++ b/test/integration/test-cases/issue-2596-local-deps-multiple-files/input/chart/Chart.yaml @@ -0,0 +1,24 @@ +apiVersion: v2 +name: chart +description: A Helm chart for Kubernetes + +# A chart can be either an 'application' or a 'library' chart. +# +# Application charts are a collection of templates that can be packaged into versioned archives +# to be deployed. +# +# Library charts provide useful utilities or functions for the chart developer. They're included as +# a dependency of application charts to inject those utilities and functions into the rendering +# pipeline. Library charts do not define any templates and therefore cannot be deployed. +type: application + +# This is the chart version. This version number should be incremented each time you make changes +# to the chart and its templates, including the app version. +# Versions are expected to follow Semantic Versioning (https://semver.org/) +version: 0.1.0 + +# This is the version number of the application being deployed. This version number should be +# incremented each time you make changes to the application. Versions are not expected to +# follow Semantic Versioning. They should reflect the version the application is using. +# It is recommended to use it with quotes. +appVersion: "1.16.0" diff --git a/test/integration/test-cases/issue-2596-local-deps-multiple-files/input/helmfile.d/release1.yaml b/test/integration/test-cases/issue-2596-local-deps-multiple-files/input/helmfile.d/release1.yaml new file mode 100644 index 00000000..f2fbfe41 --- /dev/null +++ b/test/integration/test-cases/issue-2596-local-deps-multiple-files/input/helmfile.d/release1.yaml @@ -0,0 +1,8 @@ +--- +releases: + - name: release1 + chart: ../chart + version: "*" + dependencies: + - chart: ../chart + version: "*" diff --git a/test/integration/test-cases/issue-2596-local-deps-multiple-files/input/helmfile.d/release2.yaml b/test/integration/test-cases/issue-2596-local-deps-multiple-files/input/helmfile.d/release2.yaml new file mode 100644 index 00000000..4182b9c6 --- /dev/null +++ b/test/integration/test-cases/issue-2596-local-deps-multiple-files/input/helmfile.d/release2.yaml @@ -0,0 +1,5 @@ +--- +releases: + - name: release2 + chart: ../chart + version: "*"