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 <amenon@canarytechnologies.com>

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.

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.

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)
 		}