fix: skip helm repo update when only OCI repos are configured (#2420)
When using only OCI repositories, helmfile would attempt to run 'helm repo update' which fails with 'no repositories found' error. OCI repositories don't need 'helm repo update' as they use 'helm registry login' instead. This fix adds a HasNonOCIRepositories() helper function and uses it to determine whether to run 'helm repo update'. Fixes #2418 Signed-off-by: yxxhero <aiopsclub@163.com>
This commit is contained in:
parent
04299856ef
commit
27c78a123e
|
|
@ -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)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
//
|
||||
|
|
|
|||
|
|
@ -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 -----------------------------------------------------------------------------------------------------------
|
||||
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
@ -0,0 +1,6 @@
|
|||
apiVersion: v2
|
||||
name: customresources
|
||||
description: A Helm chart for custom resources
|
||||
type: application
|
||||
version: 0.1.0
|
||||
appVersion: "1.0"
|
||||
|
|
@ -0,0 +1 @@
|
|||
{{- /* Empty templates */ -}}
|
||||
|
|
@ -0,0 +1 @@
|
|||
# Default values for customresources.
|
||||
|
|
@ -0,0 +1,9 @@
|
|||
repositories:
|
||||
- name: karpenter
|
||||
url: public.ecr.aws/karpenter
|
||||
oci: true
|
||||
|
||||
releases:
|
||||
- name: custom-resources
|
||||
chart: ./customresources
|
||||
namespace: default
|
||||
Loading…
Reference in New Issue