From 81ab674017791c2367d8a43d2783d2dad70cb4c3 Mon Sep 17 00:00:00 2001 From: yxxhero <11087727+yxxhero@users.noreply.github.com> Date: Wed, 20 May 2026 08:44:14 +0800 Subject: [PATCH] fix: normalize dependency chart path before DirectoryExistsAt check (#2598) When helmfile.d contains multiple release files and one release has a local chart dependency (e.g. chart: ../chart), the dependency path was passed to DirectoryExistsAt without normalizing against basePath. This caused the path to be resolved against CWD instead of the helmfile directory, so the local chart was not detected and helmfile tried to resolve it as a remote repo, failing with: 'failed reading adhoc dependencies: no helm list entry found for repository' Fixes #2596 Signed-off-by: yxxhero --- pkg/state/helmx.go | 3 +- pkg/state/issue_2596_test.go | 138 ++++++++++++++++++ test/integration/run.sh | 1 + .../issue-2596-local-deps-multiple-files.sh | 49 +++++++ .../input/chart/Chart.yaml | 24 +++ .../input/helmfile.d/release1.yaml | 8 + .../input/helmfile.d/release2.yaml | 5 + 7 files changed, 227 insertions(+), 1 deletion(-) create mode 100644 pkg/state/issue_2596_test.go create mode 100644 test/integration/test-cases/issue-2596-local-deps-multiple-files.sh create mode 100644 test/integration/test-cases/issue-2596-local-deps-multiple-files/input/chart/Chart.yaml create mode 100644 test/integration/test-cases/issue-2596-local-deps-multiple-files/input/helmfile.d/release1.yaml create mode 100644 test/integration/test-cases/issue-2596-local-deps-multiple-files/input/helmfile.d/release2.yaml 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: "*"