diff --git a/pkg/state/helmx.go b/pkg/state/helmx.go index 62eb7bea..a0e0d606 100644 --- a/pkg/state/helmx.go +++ b/pkg/state/helmx.go @@ -914,6 +914,34 @@ func (st *HelmState) downloadChartWithGoGetter(r *ReleaseSpec) (string, error) { return st.goGetterChart(r.Chart, r.Directory, cacheDir, r.ForceGoGetter) } +// downloadAdhocDepChartWithGoGetter fetches a go-getter URL referenced by an +// ad-hoc release dependency (release.dependencies[].chart) to a local cache +// directory and returns the local path. +// +// Ad-hoc dependencies were previously passed to chartify as-is. chartify then +// tried to resolve non-local, non-OCI values via `helm repo list`, which fails +// for go-getter URLs like "git::https://host/repo.git@path?ref=tag" with +// "no helm list entry found for repository". This mirrors the primary-chart +// fetch path (downloadChartWithGoGetter) so go-getter URLs work uniformly. +// See issue #821. +func (st *HelmState) downloadAdhocDepChartWithGoGetter(release *ReleaseSpec, chart string) (string, error) { + var pathElems []string + + if release.Namespace != "" { + pathElems = append(pathElems, release.Namespace) + } + + if release.KubeContext != "" { + pathElems = append(pathElems, release.KubeContext) + } + + pathElems = append(pathElems, release.Name, "deps") + + cacheDir := filepath.Join(pathElems...) + + return st.goGetterChart(chart, "", cacheDir, release.ForceGoGetter) +} + func (st *HelmState) goGetterChart(chart, dir, cacheDir string, force bool) (string, error) { if dir != "" && chart == "" { chart = dir @@ -985,6 +1013,20 @@ func (st *HelmState) PrepareChartify(helm helmexec.Interface, release *ReleaseSp } else if rewritten, ok := st.resolveOCIAdhocDepChart(d.Chart); ok { st.logger.Debugf("ad-hoc dependency %q rewritten to %q (matched OCI repo entry)", d.Chart, rewritten) chart = rewritten + } else if remote.IsRemote(chart) { + // Ad-hoc dependency uses a go-getter URL (e.g. + // "git::https://host/repo.git@path?ref=tag"). Fetch it to a local + // cache directory so chartify treats it as a local chart instead of + // trying to resolve it via `helm repo list`, which fails with + // "no helm list entry found for repository" for go-getter URLs. + // The primary chart already does this via downloadChartWithGoGetter; + // this mirrors that path for ad-hoc deps. See issue #821. + fetched, err := st.downloadAdhocDepChartWithGoGetter(release, chart) + if err != nil { + return nil, clean, fmt.Errorf("ad-hoc dependency %q: %w", d.Chart, err) + } + st.logger.Debugf("ad-hoc dependency %q fetched to %q via go-getter", d.Chart, fetched) + chart = fetched } c.Opts.AdhocChartDependencies = append(c.Opts.AdhocChartDependencies, chartify.ChartDependency{ diff --git a/pkg/state/issue_821_test.go b/pkg/state/issue_821_test.go new file mode 100644 index 00000000..1df03bd5 --- /dev/null +++ b/pkg/state/issue_821_test.go @@ -0,0 +1,102 @@ +package state + +import ( + "io" + "os" + "os/exec" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/helmfile/helmfile/pkg/filesystem" + "github.com/helmfile/helmfile/pkg/helmexec" + "github.com/helmfile/helmfile/pkg/remote" +) + +// initGitRepo creates a throwaway git repository at dir containing a chart at +// chartRel (e.g. "charts/mychart") and returns the go-getter URL that targets +// it on the given ref. It is used to exercise the go-getter fetch path without +// hitting the network. +func initGitRepo(t *testing.T, dir, chartRel, ref string) string { + t.Helper() + if _, err := exec.LookPath("git"); err != nil { + t.Skipf("git not available: %v", err) + } + run := func(args ...string) { + c := exec.Command("git", args...) + c.Dir = dir + out, err := c.CombinedOutput() + if err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } + } + run("init", "-b", ref) + run("config", "user.email", "test@example.com") + run("config", "user.name", "test") + chartDir := filepath.Join(dir, filepath.FromSlash(chartRel)) + require.NoError(t, os.MkdirAll(chartDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(chartDir, "Chart.yaml"), + []byte("apiVersion: v2\nname: mychart\nversion: 0.1.0\n"), 0o644)) + run("add", ".") + run("commit", "-m", "init") + return "git::file://" + dir + "@" + chartRel + "?ref=" + ref +} + +// TestAdhocDependencyGoGetterDetection verifies that the go-getter URL shapes +// used in ad-hoc dependencies (release.dependencies[].chart) are detected as +// remote sources by remote.IsRemote, which is the predicate gating the fetch +// branch added for issue #821. +func TestAdhocDependencyGoGetterDetection(t *testing.T) { + t.Parallel() + cases := []struct { + name string + chart string + remote bool + }{ + {name: "forced git getter", chart: "git::https://github.com/example/repo.git@charts/app?ref=v1.0.0", remote: true}, + {name: "forced git getter file", chart: "git::file:///tmp/repo@charts/app?ref=main", remote: true}, + {name: "https url", chart: "https://example.com/charts/app-0.1.0.tgz", remote: true}, + {name: "named repo passthrough", chart: "prometheus-community/kube-prometheus-stack", remote: false}, + {name: "oci url passthrough", chart: "oci://registry.example.com/charts/app", remote: false}, + {name: "local relative path", chart: "../charts/app", remote: false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tc.remote, remote.IsRemote(tc.chart)) + }) + } +} + +// TestAdhocDependencyGoGetterFetch is the regression test for issue #821: an +// ad-hoc dependency using a go-getter URL must be fetched to a local directory +// via downloadAdhocDepChartWithGoGetter instead of being passed through to +// chartify (which would then fail with "no helm list entry found for +// repository"). It uses a local file:// git repo so no network access is +// required. +func TestAdhocDependencyGoGetterFetch(t *testing.T) { + repo := t.TempDir() + url := initGitRepo(t, repo, "charts/mychart", "main") + + logger := helmexec.NewLogger(io.Discard, "warn") + st := &HelmState{ + logger: logger, + fs: filesystem.DefaultFileSystem(), + basePath: repo, + } + release := &ReleaseSpec{ + Name: "test-release", + Namespace: "test-ns", + } + + got, err := st.downloadAdhocDepChartWithGoGetter(release, url) + require.NoError(t, err) + + info, err := os.Stat(got) + require.NoError(t, err, "fetched dependency path must exist on disk") + assert.True(t, info.IsDir(), "fetched dependency must be a directory") + _, err = os.Stat(filepath.Join(got, "Chart.yaml")) + assert.NoError(t, err, "fetched dependency must contain the chart contents") +} diff --git a/test/integration/run.sh b/test/integration/run.sh index 3aa92922..c613f3d9 100755 --- a/test/integration/run.sh +++ b/test/integration/run.sh @@ -144,6 +144,7 @@ ${kubectl} create namespace ${test_ns} || fail "Could not create namespace ${tes . ${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/issue-821-adhoc-dep-go-getter.sh . ${dir}/test-cases/issue-2599-default-inherit.sh . ${dir}/test-cases/kubedog-tracking.sh . ${dir}/test-cases/lookup.sh diff --git a/test/integration/test-cases/issue-821-adhoc-dep-go-getter.sh b/test/integration/test-cases/issue-821-adhoc-dep-go-getter.sh new file mode 100644 index 00000000..e431b162 --- /dev/null +++ b/test/integration/test-cases/issue-821-adhoc-dep-go-getter.sh @@ -0,0 +1,94 @@ +# Integration test for issue #821: go-getter URL in ad-hoc dependencies +# https://github.com/helmfile/helmfile/issues/821 +# +# Before the fix, an ad-hoc dependency using a go-getter URL like +# git::https://github.com/org/repo.git@path?ref=tag +# was passed to chartify as-is, which then tried to resolve it via +# `helm repo list` and failed with: +# "failed reading adhoc dependencies: no helm list entry found for repository" +# +# This test commits a chart to a throwaway local git repo and references it via +# a "git::file://..." go-getter URL. Using file:// (instead of https://) keeps +# the test deterministic and network-free while exercising the exact fix path +# (remote.IsRemote + downloadAdhocDepChartWithGoGetter in PrepareChartify). + +# Resolve to an absolute path: the helmfile.yaml is generated in a temp +# directory below, and helmfile resolves `chart:` relative to that directory +# (its basePath), not the integration CWD. A relative case_dir here would make +# the main chart unresolvable. +issue_821_case_dir="$(cd "${cases_dir}/issue-821-adhoc-dep-go-getter" && pwd)" +issue_821_tmp="" + +cleanup_issue_821() { + if [ -n "${issue_821_tmp}" ] && [ -d "${issue_821_tmp}" ]; then + rm -rf "${issue_821_tmp}" + fi +} +trap cleanup_issue_821 EXIT + +issue_821_tmp=$(mktemp -d) + +test_start "issue #821: go-getter URL in ad-hoc dependencies" + +info "Setting up throwaway git repo for the ad-hoc dependency chart" + +# Build a git repo containing the dependency chart at charts/crd, mirroring the +# issue's "@" go-getter syntax. +dep_repo="${issue_821_tmp}/dep-repo" +mkdir -p "${dep_repo}/charts/crd" +cp -R "${issue_821_case_dir}/input/dep-chart/." "${dep_repo}/charts/crd/" + +git -C "${dep_repo}" init -q -b main +git -C "${dep_repo}" config user.email "test@example.com" +git -C "${dep_repo}" config user.name "helmfile-tests" +git -C "${dep_repo}" add . +git -C "${dep_repo}" commit -q -m "issue-821 dep chart" + +# Render the helmfile with absolute paths so it is independent of CWD. +cat > "${issue_821_tmp}/helmfile.yaml" < "${issue_821_out}" 2>&1 || issue_821_rc=$? + +if [ ${issue_821_rc} -ne 0 ]; then + cat "${issue_821_out}" + fail "helmfile template should not fail for a go-getter ad-hoc dependency" +fi + +# Guard against the pre-fix regression: the old error must never appear. +if grep -q "no helm list entry found for repository" "${issue_821_out}"; then + cat "${issue_821_out}" + fail "output should not contain the pre-fix 'no helm list entry found' error" +fi + +# The go-getter-fetched dependency must be materialized and rendered: its +# ConfigMap appears in the template output alongside the main chart's. +if ! grep -q "issue-821-adhoc-dep-cm" "${issue_821_out}"; then + cat "${issue_821_out}" + fail "expected the go-getter ad-hoc dependency resource 'issue-821-adhoc-dep-cm' in the template output" +fi + +if ! grep -q "issue-821-main-cm" "${issue_821_out}"; then + cat "${issue_821_out}" + fail "expected the main chart resource 'issue-821-main-cm' in the template output" +fi + +info "go-getter ad-hoc dependency was fetched and rendered correctly" + +cleanup_issue_821 +trap - EXIT + +test_pass "issue #821: go-getter URL in ad-hoc dependencies" diff --git a/test/integration/test-cases/issue-821-adhoc-dep-go-getter/input/dep-chart/Chart.yaml b/test/integration/test-cases/issue-821-adhoc-dep-go-getter/input/dep-chart/Chart.yaml new file mode 100644 index 00000000..6ed9a47e --- /dev/null +++ b/test/integration/test-cases/issue-821-adhoc-dep-go-getter/input/dep-chart/Chart.yaml @@ -0,0 +1,9 @@ +apiVersion: v2 +name: crd +version: 0.1.0 +description: >- + Ad-hoc dependency chart for issue #821. This is committed to a throwaway git + repo by the test script and referenced via a "git::file://..." go-getter URL. + The name "crd" matches the go-getter "@charts/crd" subdir basename, which is + what chartify derives as the dependency name (filepath.Base of the fetched + path) so helm's dependency build and template steps stay aligned. diff --git a/test/integration/test-cases/issue-821-adhoc-dep-go-getter/input/dep-chart/templates/dep-cm.yaml b/test/integration/test-cases/issue-821-adhoc-dep-go-getter/input/dep-chart/templates/dep-cm.yaml new file mode 100644 index 00000000..429bc927 --- /dev/null +++ b/test/integration/test-cases/issue-821-adhoc-dep-go-getter/input/dep-chart/templates/dep-cm.yaml @@ -0,0 +1,8 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: issue-821-adhoc-dep-cm + labels: + app.kubernetes.io/managed-by: {{ .Release.Service }} +data: + source: git-getter-adhoc-dep diff --git a/test/integration/test-cases/issue-821-adhoc-dep-go-getter/input/main-chart/Chart.yaml b/test/integration/test-cases/issue-821-adhoc-dep-go-getter/input/main-chart/Chart.yaml new file mode 100644 index 00000000..1708973a --- /dev/null +++ b/test/integration/test-cases/issue-821-adhoc-dep-go-getter/input/main-chart/Chart.yaml @@ -0,0 +1,4 @@ +apiVersion: v2 +name: issue-821-main +version: 0.1.0 +description: Main chart for issue #821 integration test (host of the ad-hoc go-getter dependency). diff --git a/test/integration/test-cases/issue-821-adhoc-dep-go-getter/input/main-chart/templates/main-cm.yaml b/test/integration/test-cases/issue-821-adhoc-dep-go-getter/input/main-chart/templates/main-cm.yaml new file mode 100644 index 00000000..500160bb --- /dev/null +++ b/test/integration/test-cases/issue-821-adhoc-dep-go-getter/input/main-chart/templates/main-cm.yaml @@ -0,0 +1,8 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: issue-821-main-cm + labels: + app.kubernetes.io/managed-by: {{ .Release.Service }} +data: + source: main-chart