mirror of
https://github.com/helmfile/helmfile.git
synced 2026-09-30 04:12:29 +02:00
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 <aiopsclub@163.com>
This commit is contained in:
+2
-1
@@ -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:
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -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 -----------------------------------------------------------------------------------------------------------
|
||||
|
||||
@@ -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"
|
||||
+24
@@ -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"
|
||||
+8
@@ -0,0 +1,8 @@
|
||||
---
|
||||
releases:
|
||||
- name: release1
|
||||
chart: ../chart
|
||||
version: "*"
|
||||
dependencies:
|
||||
- chart: ../chart
|
||||
version: "*"
|
||||
+5
@@ -0,0 +1,5 @@
|
||||
---
|
||||
releases:
|
||||
- name: release2
|
||||
chart: ../chart
|
||||
version: "*"
|
||||
Reference in New Issue
Block a user