mirror of
https://github.com/helmfile/helmfile.git
synced 2026-10-08 04:14:45 +02:00
fix: local chart with external dependencies error when repos configured (#2433)
* fix: local chart with external dependencies error when repos configured When helm repo update was run, the code unconditionally set skipRefresh=true for all builds, causing helm dep build --skip-refresh to fail for local charts with external dependencies not listed in helmfile.yaml. Now only non-local charts (precomputed skipRefresh=true) get --skip-refresh, while local charts preserve their skipRefresh=false to allow refreshing repos for external dependencies. Fixes #2431 Signed-off-by: yxxhero <aiopsclub@163.com> * test: update snapshot tests for local chart refresh behavior Local charts now run helm repo update during helm dep build to support external dependencies not listed in helmfile.yaml (fixes #2431). Signed-off-by: yxxhero <aiopsclub@163.com> * refactor: remove redundant skipRefresh assignment The condition 'if didUpdateRepo && r.skipRefresh { r.skipRefresh = true }' was a no-op since setting true to true has no effect. The precomputed skipRefresh value from prepareChartForRelease is already correct, so we simply preserve it without modification. Signed-off-by: yxxhero <aiopsclub@163.com> * refactor: only call UpdateRepo when at least one build uses --skip-refresh Avoid redundant helm repo update when all builds have skipRefresh=false, as each helm dep build will refresh repos itself in that case. Co-authored-by: Copilot <copilot@github.com> Signed-off-by: yxxhero <aiopsclub@163.com> * test: update release_template_inheritance snapshot for skipRefresh optimization UpdateRepo is now only called when at least one build uses --skip-refresh, so local charts without skipRefresh no longer trigger the global repo update. Signed-off-by: yxxhero <aiopsclub@163.com> * test: add regression test for issue #2431 Add TestIssue2431_LocalChartWithExternalDependency to verify that local charts with external dependencies on repos NOT in helmfile.yaml work correctly. The test ensures: - UpdateRepo is NOT called when all builds have skipRefresh=false - helm dep build does NOT receive --skip-refresh flag Signed-off-by: yxxhero <aiopsclub@163.com> * test: add integration test for issue #2431 Add test case to verify that local charts with repos configured in helmfile.yaml work correctly. The test ensures that helmfile template does not fail with 'no cached repository' or 'no repository definition' errors when: - helmfile.yaml has non-OCI repos configured - Local chart is used (which may have external dependencies not in helmfile.yaml) Signed-off-by: yxxhero <aiopsclub@163.com> * test: update issue #2431 integration test to match issue scenario Add external dependency (karma chart from wiremind repo) to local chart's Chart.yaml, matching the exact scenario described in issue #2431 where: - helmfile.yaml has repos configured (vector) - Local chart depends on a repo NOT in helmfile.yaml (wiremind) Signed-off-by: yxxhero <aiopsclub@163.com> * revert: remove unit tests and restore e2e snapshot outputs Remove pkg/state/run_helm_dep_builds_skip_refresh_test.go and restore chart_need snapshot outputs to original state. The fix is verified by the integration test for issue #2431. Signed-off-by: yxxhero <aiopsclub@163.com> * test: remove snapshot outputs to regenerate them Remove chart_need snapshot outputs so they can be regenerated by tests. Signed-off-by: yxxhero <aiopsclub@163.com> * revert: restore release_template_inheritance snapshot output Signed-off-by: yxxhero <aiopsclub@163.com> * restore: add back unit tests for skipRefresh behavior Signed-off-by: yxxhero <aiopsclub@163.com> * restore: add back chart_need snapshot outputs Signed-off-by: yxxhero <aiopsclub@163.com> * test: update snapshot outputs for skipRefresh optimization - Remove TestIssue2431_LocalChartWithExternalDependency unit test - Update chart_need outputs: local chart runs helm dep build with repo refresh - Update release_template_inheritance: no deps so no repo refresh output Signed-off-by: yxxhero <aiopsclub@163.com> * fix: update test comments and names per review feedback - Update TestRunHelmDepBuilds_MultipleBuilds comment to remove reference to removed didUpdateRepo variable - Rename test case to accurately describe condition being tested (build with skipRefresh=true instead of misleading 'non-local chart') Signed-off-by: yxxhero <aiopsclub@163.com> --------- Signed-off-by: yxxhero <aiopsclub@163.com> Co-authored-by: Copilot <copilot@github.com>
This commit is contained in:
yxxhero
@@ -177,13 +177,13 @@ func TestRunHelmDepBuilds_SkipRefreshBehaviors(t *testing.T) {
|
||||
expectSkipRefreshFlag bool
|
||||
}{
|
||||
{
|
||||
name: "no skip flags and repos exist - UpdateRepo called, skip-refresh passed",
|
||||
name: "local chart with repos exist - UpdateRepo skipped (all builds have skipRefresh=false), no skip-refresh flag (issue #2431)",
|
||||
optsSkipRefresh: false,
|
||||
helmDefaultsSkipRefresh: false,
|
||||
repos: []RepositorySpec{{Name: "stable", URL: "https://example.com"}},
|
||||
precomputedSkipRefresh: false,
|
||||
expectUpdateRepo: true,
|
||||
expectSkipRefreshFlag: true,
|
||||
expectUpdateRepo: false,
|
||||
expectSkipRefreshFlag: false,
|
||||
},
|
||||
{
|
||||
name: "opts.SkipRefresh=true - UpdateRepo skipped, skip-refresh flag preserved from precomputed value",
|
||||
@@ -231,7 +231,7 @@ func TestRunHelmDepBuilds_SkipRefreshBehaviors(t *testing.T) {
|
||||
expectSkipRefreshFlag: false,
|
||||
},
|
||||
{
|
||||
name: "mixed repos (OCI + non-OCI) - UpdateRepo called",
|
||||
name: "mixed repos (OCI + non-OCI) with local chart - UpdateRepo skipped (all builds have skipRefresh=false), no skip-refresh flag",
|
||||
optsSkipRefresh: false,
|
||||
helmDefaultsSkipRefresh: false,
|
||||
repos: []RepositorySpec{
|
||||
@@ -239,8 +239,17 @@ func TestRunHelmDepBuilds_SkipRefreshBehaviors(t *testing.T) {
|
||||
{Name: "stable", URL: "https://charts.helm.sh/stable"},
|
||||
},
|
||||
precomputedSkipRefresh: false,
|
||||
expectUpdateRepo: true,
|
||||
expectSkipRefreshFlag: true,
|
||||
expectUpdateRepo: false,
|
||||
expectSkipRefreshFlag: false,
|
||||
},
|
||||
{
|
||||
name: "build with skipRefresh=true and repos exist - UpdateRepo called, skip-refresh passed",
|
||||
optsSkipRefresh: false,
|
||||
helmDefaultsSkipRefresh: false,
|
||||
repos: []RepositorySpec{{Name: "stable", URL: "https://example.com"}},
|
||||
precomputedSkipRefresh: true,
|
||||
expectUpdateRepo: true,
|
||||
expectSkipRefreshFlag: true,
|
||||
},
|
||||
}
|
||||
|
||||
@@ -305,9 +314,10 @@ func (h *multiBuildTracker) BuildDeps(name, chart string, flags ...string) error
|
||||
return nil
|
||||
}
|
||||
|
||||
// TestRunHelmDepBuilds_MultipleBuilds verifies that when didUpdateRepo=true,
|
||||
// all builds receive --skip-refresh regardless of their precomputed skipRefresh values.
|
||||
// This ensures the override applies uniformly across all builds in the loop.
|
||||
// TestRunHelmDepBuilds_MultipleBuilds verifies that when at least one build
|
||||
// has skipRefresh=true, UpdateRepo is called and only builds with skipRefresh=true
|
||||
// receive --skip-refresh. Builds with skipRefresh=false preserve their value to allow
|
||||
// refreshing repos for external dependencies not in helmfile.yaml (issue #2431).
|
||||
func TestRunHelmDepBuilds_MultipleBuilds(t *testing.T) {
|
||||
helm := &multiBuildTracker{}
|
||||
|
||||
@@ -332,6 +342,7 @@ func TestRunHelmDepBuilds_MultipleBuilds(t *testing.T) {
|
||||
assert.True(t, helm.updateRepoCalled, "UpdateRepo should have been called")
|
||||
assert.Len(t, helm.buildDepsCalls, 2, "BuildDeps should have been called twice")
|
||||
|
||||
expectedSkipRefresh := []bool{false, true}
|
||||
for i, flags := range helm.buildDepsCalls {
|
||||
hasSkipRefresh := false
|
||||
for _, f := range flags {
|
||||
@@ -340,7 +351,8 @@ func TestRunHelmDepBuilds_MultipleBuilds(t *testing.T) {
|
||||
break
|
||||
}
|
||||
}
|
||||
assert.True(t, hasSkipRefresh, "build %d should have --skip-refresh flag (flags: %v)", i, flags)
|
||||
assert.Equal(t, expectedSkipRefresh[i], hasSkipRefresh,
|
||||
"build %d skip-refresh flag mismatch: expected %v, got %v (flags: %v)", i, expectedSkipRefresh[i], hasSkipRefresh, flags)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+17
-17
@@ -1857,29 +1857,29 @@ func (st *HelmState) runHelmDepBuilds(helm helmexec.Interface, concurrency int,
|
||||
//
|
||||
// See https://github.com/roboll/helmfile/issues/1521
|
||||
|
||||
// Perform an update of repos once before running `helm dep build` so that we
|
||||
// can safely pass --skip-refresh to the command to avoid doing a repo update
|
||||
// for every iteration of the loop where charts have external dependencies.
|
||||
// Only do this if there are non-OCI repositories configured.
|
||||
// OCI repositories don't need `helm repo update` as they use `helm registry login` instead.
|
||||
didUpdateRepo := false
|
||||
if len(builds) > 0 && !opts.SkipRefresh && !st.HelmDefaults.SkipRefresh && st.NeedsRepoUpdate() {
|
||||
// Only run a global repo update when at least one build will actually use --skip-refresh.
|
||||
// If all builds have skipRefresh=false, each `helm dep build` will refresh repos itself,
|
||||
// so calling `helm.UpdateRepo()` here would be redundant.
|
||||
anySkipRefresh := false
|
||||
for _, b := range builds {
|
||||
if b.skipRefresh {
|
||||
anySkipRefresh = true
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
if anySkipRefresh && !opts.SkipRefresh && !st.HelmDefaults.SkipRefresh && st.NeedsRepoUpdate() {
|
||||
if err := helm.UpdateRepo(); err != nil {
|
||||
return fmt.Errorf("updating repo: %w", err)
|
||||
}
|
||||
didUpdateRepo = true
|
||||
}
|
||||
|
||||
for _, r := range builds {
|
||||
// If we ran helm repo update, pass --skip-refresh to helm dep build to avoid
|
||||
// redundant repo updates. Otherwise, preserve the precomputed skipRefresh value
|
||||
// which accounts for CLI flags, helmDefaults.skipRefresh, and release.skipRefresh.
|
||||
// When no repos are configured in helmfile.yaml and user hasn't set skip-refresh,
|
||||
// helm dep build will refresh repos as needed for local charts with external
|
||||
// dependencies (fixes issue #2417).
|
||||
if didUpdateRepo {
|
||||
r.skipRefresh = true
|
||||
}
|
||||
// skipRefresh for each release is precomputed in prepareChartForRelease to:
|
||||
// - avoid redundant repo updates for non-local charts when helm repo update has run
|
||||
// - preserve refresh behavior for local charts that may depend on external repos
|
||||
// not listed in helmfile.yaml (fixes issue #2431).
|
||||
// We intentionally do not modify r.skipRefresh here.
|
||||
|
||||
buildDepsFlags := getBuildDepsFlags(r)
|
||||
if err := helm.BuildDeps(r.releaseName, r.chartPath, buildDepsFlags...); err != nil {
|
||||
|
||||
@@ -1,12 +1,10 @@
|
||||
Adding repo myrepo http://localhost:18080/
|
||||
"myrepo" has been added to your repositories
|
||||
|
||||
Updating repo
|
||||
Building dependency release=foo, chart=$WD/temp1/foo
|
||||
Hang tight while we grab the latest from your chart repositories...
|
||||
...Successfully got an update from the "myrepo" chart repository
|
||||
Update Complete. ⎈Happy Helming!⎈
|
||||
|
||||
Building dependency release=foo, chart=$WD/temp1/foo
|
||||
Saving 1 charts
|
||||
Downloading raw from repo http://localhost:18080/
|
||||
Deleting outdated charts
|
||||
|
||||
+1
-3
@@ -2,12 +2,10 @@ Live output is enabled
|
||||
Adding repo myrepo http://localhost:18081/
|
||||
"myrepo" has been added to your repositories
|
||||
|
||||
Updating repo
|
||||
Building dependency release=foo, chart=$WD/temp1/foo
|
||||
Hang tight while we grab the latest from your chart repositories...
|
||||
...Successfully got an update from the "myrepo" chart repository
|
||||
Update Complete. ⎈Happy Helming!⎈
|
||||
|
||||
Building dependency release=foo, chart=$WD/temp1/foo
|
||||
Saving 1 charts
|
||||
Downloading raw from repo http://localhost:18081/
|
||||
Deleting outdated charts
|
||||
|
||||
-5
@@ -1,11 +1,6 @@
|
||||
Adding repo myrepo http://localhost:18084/
|
||||
"myrepo" has been added to your repositories
|
||||
|
||||
Updating repo
|
||||
Hang tight while we grab the latest from your chart repositories...
|
||||
...Successfully got an update from the "myrepo" chart repository
|
||||
Update Complete. ⎈Happy Helming!⎈
|
||||
|
||||
Building dependency release=foo1, chart=../../charts/raw-0.1.0
|
||||
Building dependency release=foo2, chart=../../charts/raw-0.1.0
|
||||
Templating release=foo1, chart=../../charts/raw-0.1.0
|
||||
|
||||
@@ -134,6 +134,7 @@ ${kubectl} create namespace ${test_ns} || fail "Could not create namespace ${tes
|
||||
. ${dir}/test-cases/issue-2269.sh
|
||||
. ${dir}/test-cases/issue-2418.sh
|
||||
. ${dir}/test-cases/issue-2424-sequential-values-paths.sh
|
||||
. ${dir}/test-cases/issue-2431.sh
|
||||
|
||||
# ALL DONE -----------------------------------------------------------------------------------------------------------
|
||||
|
||||
|
||||
@@ -0,0 +1,41 @@
|
||||
#!/usr/bin/env bash
|
||||
|
||||
# Test for issue #2431: Local chart with external dependencies
|
||||
#
|
||||
# helmfile.yaml has repos configured (vector), but NOT the repo that the
|
||||
# local chart depends on (wiremind). The fix ensures that:
|
||||
# 1. helm dep build does NOT receive --skip-refresh for local charts
|
||||
# 2. helmfile template succeeds without "no cached repository" error
|
||||
#
|
||||
# This replicates the exact scenario from issue #2431.
|
||||
|
||||
issue_2431_input_dir="${cases_dir}/issue-2431/input"
|
||||
issue_2431_tmp=$(mktemp -d)
|
||||
issue_2431_output="${issue_2431_tmp}/template.log"
|
||||
|
||||
cleanup_issue_2431() {
|
||||
rm -rf "${issue_2431_tmp}"
|
||||
}
|
||||
trap cleanup_issue_2431 EXIT
|
||||
|
||||
test_start "issue-2431: Local chart with external dependency not in helmfile.yaml"
|
||||
|
||||
info "Running helmfile template: local chart depends on wiremind repo (not configured in helmfile.yaml)"
|
||||
${helmfile} -f "${issue_2431_input_dir}/helmfile.yaml" -e karma template > "${issue_2431_output}" 2>&1 || {
|
||||
code=$?
|
||||
cat "${issue_2431_output}"
|
||||
|
||||
# Check if the failure is due to "no cached repository" or "no repository definition" error
|
||||
if grep -q "no cached repository" "${issue_2431_output}" || grep -q "no repository definition" "${issue_2431_output}"; then
|
||||
fail "Issue #2431 regression: helm dep build received --skip-refresh incorrectly for local chart"
|
||||
fi
|
||||
|
||||
fail "helmfile template failed with exit code ${code}"
|
||||
}
|
||||
|
||||
info "SUCCESS: helmfile template completed successfully"
|
||||
info "Template output:"
|
||||
cat "${issue_2431_output}"
|
||||
|
||||
trap - EXIT
|
||||
test_pass "issue-2431: Local chart with external dependency not in helmfile.yaml"
|
||||
@@ -0,0 +1,28 @@
|
||||
# Issue #2431: Local chart with external dependencies
|
||||
#
|
||||
# helmfile.yaml has repos configured (vector), but NOT the repo that the chart
|
||||
# depends on (wiremind). This tests that helm dep build does NOT receive
|
||||
# --skip-refresh for local charts, allowing them to refresh repos for
|
||||
# external dependencies not in helmfile.yaml.
|
||||
|
||||
repositories:
|
||||
## Everything works if the wiremind repo is added here:
|
||||
# - name: wiremind
|
||||
# url: https://wiremind.github.io/wiremind-helm-charts
|
||||
- name: vector
|
||||
url: https://helm.vector.dev
|
||||
|
||||
---
|
||||
|
||||
environments:
|
||||
karma:
|
||||
missingFileHandler: Debug
|
||||
|
||||
---
|
||||
|
||||
releases:
|
||||
- name: karma
|
||||
namespace: default
|
||||
chart: ./karma
|
||||
createNamespace: false
|
||||
missingFileHandler: "Info"
|
||||
@@ -0,0 +1,10 @@
|
||||
apiVersion: v2
|
||||
name: karma
|
||||
description: Local chart with external dependencies (issue #2431)
|
||||
type: application
|
||||
version: 1.0.0
|
||||
appVersion: "1.0.0"
|
||||
dependencies:
|
||||
- name: karma
|
||||
version: 2.11.0
|
||||
repository: https://wiremind.github.io/wiremind-helm-charts
|
||||
@@ -0,0 +1,7 @@
|
||||
apiVersion: v1
|
||||
kind: ConfigMap
|
||||
metadata:
|
||||
name: karma-cm
|
||||
namespace: {{ .Release.Namespace }}
|
||||
data:
|
||||
key: value
|
||||
Reference in New Issue
Block a user