From abfe73fa6fbf70a855a576fe0c444f45cac11865 Mon Sep 17 00:00:00 2001 From: Aditya Menon Date: Sun, 22 Feb 2026 14:13:05 +0530 Subject: [PATCH] fix: helmDefaults.skipRefresh ignored in runHelmDepBuilds (#2415) fix: helmDefaults.skipRefresh ignored in runHelmDepBuilds (#2269) `runHelmDepBuilds()` only checked the CLI flag (`opts.SkipRefresh`) when deciding whether to run `helm repo update` before building dependencies. This meant that setting `helmDefaults.skipRefresh: true` in helmfile.yaml had no effect on the repo update call inside dep builds. Add `!st.HelmDefaults.SkipRefresh` to the guard condition so that `helmDefaults.skipRefresh: true` is respected alongside the CLI flag. Signed-off-by: Aditya Menon --- pkg/app/app_test.go | 53 +++++++++++ pkg/state/issue_2296_test.go | 90 +++++++++++++++++++ pkg/state/state.go | 2 +- test/integration/run.sh | 1 + test/integration/test-cases/issue-2269.sh | 57 ++++++++++++ .../issue-2269/input/chart/Chart.yaml | 4 + .../input/chart/templates/configmap.yaml | 6 ++ .../test-cases/issue-2269/input/helmfile.yaml | 13 +++ 8 files changed, 225 insertions(+), 1 deletion(-) create mode 100644 test/integration/test-cases/issue-2269.sh create mode 100644 test/integration/test-cases/issue-2269/input/chart/Chart.yaml create mode 100644 test/integration/test-cases/issue-2269/input/chart/templates/configmap.yaml create mode 100644 test/integration/test-cases/issue-2269/input/helmfile.yaml diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go index bd42a3b3..07e9e6f1 100644 --- a/pkg/app/app_test.go +++ b/pkg/app/app_test.go @@ -4619,6 +4619,59 @@ func TestRenderYamlEnvVar(t *testing.T) { } } +// TestTemplate_HelmDefaultsSkipDepsSkipRefresh verifies that helmDefaults.skipDeps +// and helmDefaults.skipRefresh cause SyncReposOnce to be skipped, so no AddRepo +// calls are made. This is a regression test for issue #2269/#2296. +func TestTemplate_HelmDefaultsSkipDepsSkipRefresh(t *testing.T) { + files := map[string]string{ + "/path/to/helmfile.yaml": ` +repositories: +- name: stable + url: https://kubernetes-charts.storage.googleapis.com + +helmDefaults: + skipDeps: true + skipRefresh: true + +releases: +- name: myrelease1 + chart: stable/mychart1 +`, + } + + var helm = &mockHelmExec{} + + var buffer bytes.Buffer + syncWriter := testhelper.NewSyncWriter(&buffer) + logger := helmexec.NewLogger(syncWriter, "debug") + + valsRuntime, err := vals.New(vals.Options{CacheSize: 32}) + require.NoError(t, err) + + app := appWithFs(&App{ + OverrideHelmBinary: DefaultHelmBinary, + fs: ffs.DefaultFileSystem(), + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Env: "default", + Logger: logger, + helms: map[helmKey]helmexec.Interface{ + createHelmKey("helm", "default"): helm, + }, + Namespace: "testNamespace", + valsRuntime: valsRuntime, + }, files) + + // Run Template with skipDeps=false to simulate no CLI flags being passed. + // The helmDefaults should still cause repos to be skipped. + err = app.Template(configImpl{set: []string{"foo=a"}, skipDeps: false}) + require.NoError(t, err) + + // The key assertion: repos should be empty because helmDefaults.skipDeps + // and helmDefaults.skipRefresh caused SyncReposOnce to be skipped. + assert.Empty(t, helm.repos, "expected no AddRepo calls when helmDefaults.skipDeps and helmDefaults.skipRefresh are true") +} + // TestHelmBinaryPreservedInMultiDocumentYAML tests that helmBinary is preserved // when processing multi-document YAML files at the loadDesiredStateFromYaml level. // This is a regression test for issue #2319. diff --git a/pkg/state/issue_2296_test.go b/pkg/state/issue_2296_test.go index 98bd7a3e..549ad7c9 100644 --- a/pkg/state/issue_2296_test.go +++ b/pkg/state/issue_2296_test.go @@ -7,6 +7,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/helmfile/helmfile/pkg/exectest" ) // Note: boolPtr helper is already defined in skip_test.go and shared across test files in this package. @@ -133,6 +135,94 @@ version: 0.1.0 } } +// updateRepoTracker wraps exectest.Helm to track whether UpdateRepo was called. +type updateRepoTracker struct { + exectest.Helm + updateRepoCalled bool +} + +func (h *updateRepoTracker) UpdateRepo() error { + h.updateRepoCalled = true + 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. +func TestRunHelmDepBuilds_HelmDefaultsSkipRefresh(t *testing.T) { + tests := []struct { + name string + optsSkipRefresh bool + helmDefaultsSkipRefresh bool + hasRepos bool + expectUpdateRepo bool + }{ + { + name: "no skip flags and repos exist - UpdateRepo called", + optsSkipRefresh: false, + helmDefaultsSkipRefresh: false, + hasRepos: true, + expectUpdateRepo: true, + }, + { + name: "opts.SkipRefresh=true - UpdateRepo skipped", + optsSkipRefresh: true, + helmDefaultsSkipRefresh: false, + hasRepos: true, + expectUpdateRepo: false, + }, + { + name: "helmDefaults.skipRefresh=true - UpdateRepo skipped", + optsSkipRefresh: false, + helmDefaultsSkipRefresh: true, + hasRepos: true, + expectUpdateRepo: false, + }, + { + name: "no repos configured - UpdateRepo skipped", + optsSkipRefresh: false, + helmDefaultsSkipRefresh: false, + hasRepos: false, + expectUpdateRepo: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + helm := &updateRepoTracker{} + + var repos []RepositorySpec + if tt.hasRepos { + repos = []RepositorySpec{{Name: "stable", URL: "https://example.com"}} + } + + st := &HelmState{ + logger: logger, + ReleaseSetSpec: ReleaseSetSpec{ + HelmDefaults: HelmSpec{ + SkipRefresh: tt.helmDefaultsSkipRefresh, + }, + Repositories: repos, + }, + } + + builds := []*chartPrepareResult{ + {releaseName: "test", chartPath: "/tmp/chart", buildDeps: true}, + } + + opts := ChartPrepareOptions{ + SkipRefresh: tt.optsSkipRefresh, + } + + err := st.runHelmDepBuilds(helm, 1, builds, opts) + require.NoError(t, err) + + assert.Equal(t, tt.expectUpdateRepo, helm.updateRepoCalled, + "UpdateRepo called mismatch: expected %v, got %v", tt.expectUpdateRepo, helm.updateRepoCalled) + }) + } +} + // 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. diff --git a/pkg/state/state.go b/pkg/state/state.go index 212ce22e..28967852 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -1861,7 +1861,7 @@ func (st *HelmState) runHelmDepBuilds(helm helmexec.Interface, concurrency int, // 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 repositories configured. - if len(builds) > 0 && !opts.SkipRefresh && len(st.Repositories) > 0 { + if len(builds) > 0 && !opts.SkipRefresh && !st.HelmDefaults.SkipRefresh && len(st.Repositories) > 0 { if err := helm.UpdateRepo(); err != nil { return fmt.Errorf("updating repo: %w", err) } diff --git a/test/integration/run.sh b/test/integration/run.sh index 5fa5e626..c6480baf 100755 --- a/test/integration/run.sh +++ b/test/integration/run.sh @@ -131,6 +131,7 @@ ${kubectl} create namespace ${test_ns} || fail "Could not create namespace ${tes . ${dir}/test-cases/issue-2103.sh . ${dir}/test-cases/unittest.sh . ${dir}/test-cases/issue-2409-sequential-kubecontext.sh +. ${dir}/test-cases/issue-2269.sh # ALL DONE ----------------------------------------------------------------------------------------------------------- diff --git a/test/integration/test-cases/issue-2269.sh b/test/integration/test-cases/issue-2269.sh new file mode 100644 index 00000000..060250c4 --- /dev/null +++ b/test/integration/test-cases/issue-2269.sh @@ -0,0 +1,57 @@ +# Issue #2269: helmDefaults.skipDeps and helmDefaults.skipRefresh should be respected +# https://github.com/helmfile/helmfile/issues/2269 +# +# When helmDefaults.skipDeps=true and helmDefaults.skipRefresh=true are set in +# helmfile.yaml, helmfile should NOT attempt to add repos or update them. +# Previously, only the CLI flags --skip-deps and --skip-refresh worked; +# the helmDefaults equivalents were ignored in withPreparedCharts() and +# runHelmDepBuilds(). + +issue_2269_input_dir="${cases_dir}/issue-2269/input" +issue_2269_tmp=$(mktemp -d) +issue_2269_output="${issue_2269_tmp}/template.log" + +test_start "issue-2269: helmDefaults.skipDeps and skipRefresh prevent repo operations" + +# Run helmfile template WITHOUT any CLI --skip-deps/--skip-refresh flags. +# The helmDefaults in helmfile.yaml should be sufficient to skip repo operations. +# We use a fake repo URL that would fail if helmfile tried to contact it. +info "Running helmfile template with helmDefaults.skipDeps=true and skipRefresh=true" +${helmfile} -f "${issue_2269_input_dir}/helmfile.yaml" template > "${issue_2269_output}" 2>&1 +code=$? + +if [ $code -ne 0 ]; then + cat "${issue_2269_output}" + rm -rf "${issue_2269_tmp}" + fail "helmfile template failed - helmDefaults.skipDeps/skipRefresh were likely ignored and helmfile tried to contact the fake repo" +fi + +# Verify no repo operations occurred +if grep -q "Adding repo" "${issue_2269_output}"; then + cat "${issue_2269_output}" + rm -rf "${issue_2269_tmp}" + fail "Issue #2269 regression: 'Adding repo' found in output despite helmDefaults.skipDeps=true and skipRefresh=true" +fi + +info "No 'Adding repo' messages found" + +if grep -q "Updating repo" "${issue_2269_output}" || grep -q "helm repo update" "${issue_2269_output}"; then + cat "${issue_2269_output}" + rm -rf "${issue_2269_tmp}" + fail "Issue #2269 regression: repo update found in output despite helmDefaults.skipRefresh=true" +fi + +info "No repo update messages found" + +# Verify the template actually produced output (sanity check) +if ! grep -q "test-2269" "${issue_2269_output}"; then + cat "${issue_2269_output}" + rm -rf "${issue_2269_tmp}" + fail "Template output missing expected content" +fi + +info "Template produced expected output" + +rm -rf "${issue_2269_tmp}" + +test_pass "issue-2269: helmDefaults.skipDeps and skipRefresh prevent repo operations" diff --git a/test/integration/test-cases/issue-2269/input/chart/Chart.yaml b/test/integration/test-cases/issue-2269/input/chart/Chart.yaml new file mode 100644 index 00000000..610dc56c --- /dev/null +++ b/test/integration/test-cases/issue-2269/input/chart/Chart.yaml @@ -0,0 +1,4 @@ +apiVersion: v2 +name: test-chart-2269 +version: 0.1.0 +description: Minimal chart for issue #2269 integration test diff --git a/test/integration/test-cases/issue-2269/input/chart/templates/configmap.yaml b/test/integration/test-cases/issue-2269/input/chart/templates/configmap.yaml new file mode 100644 index 00000000..6dadf874 --- /dev/null +++ b/test/integration/test-cases/issue-2269/input/chart/templates/configmap.yaml @@ -0,0 +1,6 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: test-2269 +data: + key: value diff --git a/test/integration/test-cases/issue-2269/input/helmfile.yaml b/test/integration/test-cases/issue-2269/input/helmfile.yaml new file mode 100644 index 00000000..365690e5 --- /dev/null +++ b/test/integration/test-cases/issue-2269/input/helmfile.yaml @@ -0,0 +1,13 @@ +# Issue #2269: helmDefaults.skipDeps and helmDefaults.skipRefresh should prevent +# repo operations (Adding repo, helm repo update) without needing CLI flags. +repositories: +- name: fake-repo + url: https://fake-repo.example.com + +helmDefaults: + skipDeps: true + skipRefresh: true + +releases: +- name: test-release-2269 + chart: ./chart