public_git
/
helmfile
mirror of https://github.com/helmfile/helmfile.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

fix: only pass --skip-refresh to helm dep build when helm repo update was run (#2419) 

When no repositories are defined in helmfile.yaml, local charts with
external dependencies need to refresh the repo cache. Previously, we
always passed --skip-refresh to helm dep build, which broke this case.

Now --skip-refresh is only passed when we actually ran helm repo update,
meaning repos are configured AND no skip refresh flags are set. This
preserves the precomputed skipRefresh value from prepareChartForRelease
which accounts for CLI flags, helmDefaults.skipRefresh, and release.skipRefresh.

Fixes #2417

Signed-off-by: yxxhero <aiopsclub@163.com>
This commit is contained in:
yxxhero 2026-02-25 13:03:30 +08:00 committed by GitHub
parent 27c78a123e
commit 2b0086b196
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 131 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

133
pkg/state/issue_2296_test.go → pkg/state/run_helm_dep_builds_skip_refresh_test.go
View File

@ -139,6 +139,7 @@ version: 0.1.0
type updateRepoTracker struct {
exectest.Helm
updateRepoCalled bool
buildDepsFlags []string
}
func (h *updateRepoTracker) UpdateRepo() error {
@ -146,53 +147,88 @@ func (h *updateRepoTracker) UpdateRepo() error {
return nil
}
// TestRunHelmDepBuilds_HelmDefaultsSkipRefresh verifies that when
// helmDefaults.skipRefresh=true, runHelmDepBuilds skips the UpdateRepo call
// even when opts.SkipRefresh is false. This is a regression test for issue #2269.
// It also verifies that UpdateRepo is skipped when only OCI repos are configured,
// as OCI repos don't need `helm repo update` (they use `helm registry login` instead).
func TestRunHelmDepBuilds_HelmDefaultsSkipRefresh(t *testing.T) {
func (h *updateRepoTracker) BuildDeps(name, chart string, flags ...string) error {
h.buildDepsFlags = flags
return nil
}
// TestRunHelmDepBuilds_SkipRefreshBehaviors verifies multiple behaviors around
// skipRefresh handling in runHelmDepBuilds:
//
// 1. When helmDefaults.skipRefresh=true, runHelmDepBuilds skips the UpdateRepo call
// even when opts.SkipRefresh is false (regression test for issue #2269).
// 2. When no repos are configured, helm dep build should not receive --skip-refresh
// so it can refresh repos for local charts with external dependencies
// (regression test for issue #2417).
// 3. UpdateRepo is skipped when only OCI repos are configured,
// as OCI repos don't need `helm repo update` (they use `helm registry login` instead).
//
// The precomputedSkipRefresh field simulates the skipRefresh value computed in
// prepareChartForRelease, which accounts for CLI flags, helmDefaults.skipRefresh,
// and release-level skipRefresh settings.
func TestRunHelmDepBuilds_SkipRefreshBehaviors(t *testing.T) {
tests := []struct {
name string
optsSkipRefresh bool
helmDefaultsSkipRefresh bool
repos []RepositorySpec
precomputedSkipRefresh bool
expectUpdateRepo bool
expectSkipRefreshFlag bool
}{
{
name: "no skip flags and repos exist - UpdateRepo called",
name: "no skip flags and repos exist - UpdateRepo called, skip-refresh passed",
optsSkipRefresh: false,
helmDefaultsSkipRefresh: false,
repos: []RepositorySpec{{Name: "stable", URL: "https://example.com"}},
precomputedSkipRefresh: false,
expectUpdateRepo: true,
expectSkipRefreshFlag: true,
},
{
name: "opts.SkipRefresh=true - UpdateRepo skipped",
name: "opts.SkipRefresh=true - UpdateRepo skipped, skip-refresh flag preserved from precomputed value",
optsSkipRefresh: true,
helmDefaultsSkipRefresh: false,
repos: []RepositorySpec{{Name: "stable", URL: "https://example.com"}},
precomputedSkipRefresh: true,
expectUpdateRepo: false,
expectSkipRefreshFlag: true,
},
{
name: "helmDefaults.skipRefresh=true - UpdateRepo skipped",
name: "helmDefaults.skipRefresh=true - UpdateRepo skipped, skip-refresh flag preserved from precomputed value",
optsSkipRefresh: false,
helmDefaultsSkipRefresh: true,
repos: []RepositorySpec{{Name: "stable", URL: "https://example.com"}},
precomputedSkipRefresh: true,
expectUpdateRepo: false,
expectSkipRefreshFlag: true,
},
{
name: "no repos configured - UpdateRepo skipped",
name: "release-level skipRefresh=true - UpdateRepo called, skip-refresh flag preserved from precomputed value",
optsSkipRefresh: false,
helmDefaultsSkipRefresh: false,
repos: []RepositorySpec{{Name: "stable", URL: "https://example.com"}},
precomputedSkipRefresh: true,
expectUpdateRepo: true,
expectSkipRefreshFlag: true,
},
{
name: "no repos configured - UpdateRepo skipped, no skip-refresh flag (issue #2417)",
optsSkipRefresh: false,
helmDefaultsSkipRefresh: false,
repos: nil,
precomputedSkipRefresh: false,
expectUpdateRepo: false,
expectSkipRefreshFlag: false,
},
{
name: "only OCI repos configured - UpdateRepo skipped",
name: "only OCI repos configured - UpdateRepo skipped, no skip-refresh flag",
optsSkipRefresh: false,
helmDefaultsSkipRefresh: false,
repos: []RepositorySpec{{Name: "karpenter", URL: "public.ecr.aws/karpenter", OCI: true}},
precomputedSkipRefresh: false,
expectUpdateRepo: false,
expectSkipRefreshFlag: false,
},
{
name: "mixed repos (OCI + non-OCI) - UpdateRepo called",
@ -202,7 +238,9 @@ func TestRunHelmDepBuilds_HelmDefaultsSkipRefresh(t *testing.T) {
{Name: "karpenter", URL: "public.ecr.aws/karpenter", OCI: true},
{Name: "stable", URL: "https://charts.helm.sh/stable"},
},
expectUpdateRepo: true,
precomputedSkipRefresh: false,
expectUpdateRepo: true,
expectSkipRefreshFlag: true,
},
}
@ -221,7 +259,7 @@ func TestRunHelmDepBuilds_HelmDefaultsSkipRefresh(t *testing.T) {
}
builds := []*chartPrepareResult{
{releaseName: "test", chartPath: "/tmp/chart", buildDeps: true},
{releaseName: "test", chartPath: "/tmp/chart", buildDeps: true, skipRefresh: tt.precomputedSkipRefresh},
}
opts := ChartPrepareOptions{
@ -231,12 +269,81 @@ func TestRunHelmDepBuilds_HelmDefaultsSkipRefresh(t *testing.T) {
err := st.runHelmDepBuilds(helm, 1, builds, opts)
require.NoError(t, err)
assert.NotNil(t, helm.buildDepsFlags, "BuildDeps should have been called")
assert.Equal(t, tt.expectUpdateRepo, helm.updateRepoCalled,
"UpdateRepo called mismatch: expected %v, got %v", tt.expectUpdateRepo, helm.updateRepoCalled)
hasSkipRefreshFlag := false
for _, f := range helm.buildDepsFlags {
if f == "--skip-refresh" {
hasSkipRefreshFlag = true
break
}
}
assert.Equal(t, tt.expectSkipRefreshFlag, hasSkipRefreshFlag,
"--skip-refresh flag mismatch: expected %v, got %v (flags: %v)", tt.expectSkipRefreshFlag, hasSkipRefreshFlag, helm.buildDepsFlags)
})
}
}
// multiBuildTracker tracks multiple BuildDeps calls for testing scenarios with
// multiple builds that have different skipRefresh values.
type multiBuildTracker struct {
exectest.Helm
updateRepoCalled bool
buildDepsCalls [][]string
}
func (h *multiBuildTracker) UpdateRepo() error {
h.updateRepoCalled = true
return nil
}
func (h *multiBuildTracker) BuildDeps(name, chart string, flags ...string) error {
h.buildDepsCalls = append(h.buildDepsCalls, flags)
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.
func TestRunHelmDepBuilds_MultipleBuilds(t *testing.T) {
helm := &multiBuildTracker{}
st := &HelmState{
logger: logger,
ReleaseSetSpec: ReleaseSetSpec{
HelmDefaults: HelmSpec{SkipRefresh: false},
Repositories: []RepositorySpec{{Name: "stable", URL: "https://example.com"}},
},
}
builds := []*chartPrepareResult{
{releaseName: "release-a", chartPath: "/tmp/chart-a", buildDeps: true, skipRefresh: false},
{releaseName: "release-b", chartPath: "/tmp/chart-b", buildDeps: true, skipRefresh: true},
}
opts := ChartPrepareOptions{SkipRefresh: false}
err := st.runHelmDepBuilds(helm, 1, builds, opts)
require.NoError(t, err)
assert.True(t, helm.updateRepoCalled, "UpdateRepo should have been called")
assert.Len(t, helm.buildDepsCalls, 2, "BuildDeps should have been called twice")
for i, flags := range helm.buildDepsCalls {
hasSkipRefresh := false
for _, f := range flags {
if f == "--skip-refresh" {
hasSkipRefresh = true
break
}
}
assert.True(t, hasSkipRefresh, "build %d should have --skip-refresh flag (flags: %v)", i, flags)
}
}
// TestSkipReposLogic tests the skipRepos calculation used in pkg/app/run.go.
// This documents and verifies the expected behavior: both helmDefaults.skipDeps
// and helmDefaults.skipRefresh should cause repo sync to be skipped.

14
pkg/state/state.go
View File

@ -1862,16 +1862,24 @@ func (st *HelmState) runHelmDepBuilds(helm helmexec.Interface, concurrency int,
// 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() {
if err := helm.UpdateRepo(); err != nil {
return fmt.Errorf("updating repo: %w", err)
}
didUpdateRepo = true
}
for _, r := range builds {
// Never update the local repository cache here, since we've already
// updated it before entering the loop
r.skipRefresh = true
// 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
}
buildDepsFlags := getBuildDepsFlags(r)
if err := helm.BuildDeps(r.releaseName, r.chartPath, buildDepsFlags...); err != nil {