diff --git a/pkg/state/issue_2296_test.go b/pkg/state/issue_2296_test.go index 549ad7c9..ebcce047 100644 --- a/pkg/state/issue_2296_test.go +++ b/pkg/state/issue_2296_test.go @@ -149,60 +149,74 @@ func (h *updateRepoTracker) UpdateRepo() error { // 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) { tests := []struct { name string optsSkipRefresh bool helmDefaultsSkipRefresh bool - hasRepos bool + repos []RepositorySpec expectUpdateRepo bool }{ { name: "no skip flags and repos exist - UpdateRepo called", optsSkipRefresh: false, helmDefaultsSkipRefresh: false, - hasRepos: true, + repos: []RepositorySpec{{Name: "stable", URL: "https://example.com"}}, expectUpdateRepo: true, }, { name: "opts.SkipRefresh=true - UpdateRepo skipped", optsSkipRefresh: true, helmDefaultsSkipRefresh: false, - hasRepos: true, + repos: []RepositorySpec{{Name: "stable", URL: "https://example.com"}}, expectUpdateRepo: false, }, { name: "helmDefaults.skipRefresh=true - UpdateRepo skipped", optsSkipRefresh: false, helmDefaultsSkipRefresh: true, - hasRepos: true, + repos: []RepositorySpec{{Name: "stable", URL: "https://example.com"}}, expectUpdateRepo: false, }, { name: "no repos configured - UpdateRepo skipped", optsSkipRefresh: false, helmDefaultsSkipRefresh: false, - hasRepos: false, + repos: nil, expectUpdateRepo: false, }, + { + name: "only OCI repos configured - UpdateRepo skipped", + optsSkipRefresh: false, + helmDefaultsSkipRefresh: false, + repos: []RepositorySpec{{Name: "karpenter", URL: "public.ecr.aws/karpenter", OCI: true}}, + expectUpdateRepo: false, + }, + { + name: "mixed repos (OCI + non-OCI) - UpdateRepo called", + optsSkipRefresh: false, + helmDefaultsSkipRefresh: false, + repos: []RepositorySpec{ + {Name: "karpenter", URL: "public.ecr.aws/karpenter", OCI: true}, + {Name: "stable", URL: "https://charts.helm.sh/stable"}, + }, + expectUpdateRepo: true, + }, } 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, + Repositories: tt.repos, }, } @@ -284,3 +298,58 @@ func TestSkipReposLogic(t *testing.T) { }) } } + +// TestNeedsRepoUpdate tests the NeedsRepoUpdate function. +// This is a regression test for issue #2418. +func TestNeedsRepoUpdate(t *testing.T) { + tests := []struct { + name string + repos []RepositorySpec + expected bool + }{ + { + name: "no repos configured", + repos: nil, + expected: false, + }, + { + name: "only non-OCI repos", + repos: []RepositorySpec{{Name: "stable", URL: "https://charts.helm.sh/stable"}}, + expected: true, + }, + { + name: "only OCI repos", + repos: []RepositorySpec{{Name: "karpenter", URL: "public.ecr.aws/karpenter", OCI: true}}, + expected: false, + }, + { + name: "mixed repos (OCI + non-OCI)", + repos: []RepositorySpec{ + {Name: "karpenter", URL: "public.ecr.aws/karpenter", OCI: true}, + {Name: "stable", URL: "https://charts.helm.sh/stable"}, + }, + expected: true, + }, + { + name: "multiple OCI repos", + repos: []RepositorySpec{ + {Name: "karpenter", URL: "public.ecr.aws/karpenter", OCI: true}, + {Name: "nginx", URL: "registry.example.com/nginx", OCI: true}, + }, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + st := &HelmState{ + ReleaseSetSpec: ReleaseSetSpec{ + Repositories: tt.repos, + }, + } + result := st.NeedsRepoUpdate() + assert.Equal(t, tt.expected, result, + "NeedsRepoUpdate mismatch: expected %v, got %v", tt.expected, result) + }) + } +} diff --git a/pkg/state/state.go b/pkg/state/state.go index 28967852..5b1c1ade 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -1860,8 +1860,9 @@ func (st *HelmState) runHelmDepBuilds(helm helmexec.Interface, concurrency int, // 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 repositories configured. - if len(builds) > 0 && !opts.SkipRefresh && !st.HelmDefaults.SkipRefresh && len(st.Repositories) > 0 { + // 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. + if len(builds) > 0 && !opts.SkipRefresh && !st.HelmDefaults.SkipRefresh && st.NeedsRepoUpdate() { if err := helm.UpdateRepo(); err != nil { return fmt.Errorf("updating repo: %w", err) } @@ -5081,6 +5082,17 @@ func (st *HelmState) IsOCIChart(chart string) bool { return repo.OCI } +// NeedsRepoUpdate returns true if there are any repositories that require `helm repo update`. +// OCI repositories don't need `helm repo update` as they use `helm registry login` instead. +func (st *HelmState) NeedsRepoUpdate() bool { + for _, repo := range st.Repositories { + if !repo.OCI { + return true + } + } + return false +} + // parseOCIChartRef parses an OCI chart URL into base URL, version tag, and digest. // Examples: // diff --git a/test/integration/run.sh b/test/integration/run.sh index c6480baf..7661ae70 100755 --- a/test/integration/run.sh +++ b/test/integration/run.sh @@ -132,6 +132,7 @@ ${kubectl} create namespace ${test_ns} || fail "Could not create namespace ${tes . ${dir}/test-cases/unittest.sh . ${dir}/test-cases/issue-2409-sequential-kubecontext.sh . ${dir}/test-cases/issue-2269.sh +. ${dir}/test-cases/issue-2418.sh # ALL DONE ----------------------------------------------------------------------------------------------------------- diff --git a/test/integration/test-cases/issue-2418.sh b/test/integration/test-cases/issue-2418.sh new file mode 100755 index 00000000..77e81598 --- /dev/null +++ b/test/integration/test-cases/issue-2418.sh @@ -0,0 +1,38 @@ +#!/usr/bin/env bash + +# Test for issue #2418: Skip helm repo update when only OCI repos are configured +# When using only OCI repositories combined with a local chart, helmfile should NOT +# attempt to run `helm repo update` which would fail with "no repositories found". + +issue_2418_input_dir="${cases_dir}/issue-2418/input" +issue_2418_tmp=$(mktemp -d) +issue_2418_output="${issue_2418_tmp}/template.log" + +cleanup_issue_2418() { + rm -rf "${issue_2418_tmp}" +} +trap cleanup_issue_2418 EXIT + +test_start "issue-2418: OCI repos + local chart should skip helm repo update" + +# Run helmfile template - this would fail with "no repositories found" before the fix +# because it would attempt to run `helm repo update` for OCI repos +info "Running helmfile template with OCI repo + local chart" +${helmfile} -f "${issue_2418_input_dir}/helmfile.yaml" template > "${issue_2418_output}" 2>&1 || { + code=$? + cat "${issue_2418_output}" + + # Check if the failure is due to "no repositories found" error + if grep -q "no repositories found" "${issue_2418_output}"; then + fail "Issue #2418 regression: helm repo update was called for OCI-only repos" + fi + + fail "helmfile template failed with exit code ${code}" +} + +info "SUCCESS: helmfile template completed without 'no repositories found' error" +info "Template output:" +cat "${issue_2418_output}" + +trap - EXIT +test_pass "issue-2418: OCI repos + local chart should skip helm repo update" diff --git a/test/integration/test-cases/issue-2418/input/customresources/Chart.yaml b/test/integration/test-cases/issue-2418/input/customresources/Chart.yaml new file mode 100644 index 00000000..f03842cf --- /dev/null +++ b/test/integration/test-cases/issue-2418/input/customresources/Chart.yaml @@ -0,0 +1,6 @@ +apiVersion: v2 +name: customresources +description: A Helm chart for custom resources +type: application +version: 0.1.0 +appVersion: "1.0" diff --git a/test/integration/test-cases/issue-2418/input/customresources/templates/notes.txt b/test/integration/test-cases/issue-2418/input/customresources/templates/notes.txt new file mode 100644 index 00000000..be5f6927 --- /dev/null +++ b/test/integration/test-cases/issue-2418/input/customresources/templates/notes.txt @@ -0,0 +1 @@ +{{- /* Empty templates */ -}} diff --git a/test/integration/test-cases/issue-2418/input/customresources/values.yaml b/test/integration/test-cases/issue-2418/input/customresources/values.yaml new file mode 100644 index 00000000..4874d147 --- /dev/null +++ b/test/integration/test-cases/issue-2418/input/customresources/values.yaml @@ -0,0 +1 @@ +# Default values for customresources. diff --git a/test/integration/test-cases/issue-2418/input/helmfile.yaml b/test/integration/test-cases/issue-2418/input/helmfile.yaml new file mode 100644 index 00000000..bc884542 --- /dev/null +++ b/test/integration/test-cases/issue-2418/input/helmfile.yaml @@ -0,0 +1,9 @@ +repositories: + - name: karpenter + url: public.ecr.aws/karpenter + oci: true + +releases: +- name: custom-resources + chart: ./customresources + namespace: default