fix(downloader): do not error for remote chart when local repo cache is missing - #32648
Open
aadieng100 wants to merge 1 commit into
Open
aadieng100 wants to merge 1 commit into
aadieng100 wants to merge 1 commit into
Conversation
…is missing
When resolving an absolute chart URL, scanReposForURL scans known repositories to check if any match the chart URL in order to apply repository-specific TLS or authentication options.
Previously, if any repository in repositories.yaml lacked a local cached index file (for example because the cache was cleared or never updated), scanReposForURL immediately aborted with an error ('no cached repo found'). This caused ResolveChartVersion to fail completely, preventing the user from downloading or templating an unrelated remote chart even when fetching directly by URL or when another repository had a valid cache.
This change skips repositories whose index cache does not exist (fs.ErrNotExist) rather than returning an early error. If no repository matches, ErrNoOwnerRepo is returned and swallowed by ResolveChartVersion to fall back to the default HTTP client as intended.
Closes helm#11961
Signed-off-by: abdouladieng <diengabdoulaziz110@gmail.com>
pull-request-size
Bot
added
the
size/M
github-actions
Bot
added
the
v4.x
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
When resolving an absolute chart URL,
scanReposForURLscans known repositories to check if any match the chart URL in order to apply repository-specific TLS or authentication options.Previously, if any repository in
repositories.yamllacked a local cached index file (for example because the cache was cleared or never updated),scanReposForURLimmediately aborted with an error (no cached repo found. (try 'helm repo update')). This causedResolveChartVersionto fail completely, preventing the user from downloading or templating an unrelated remote chart even when fetching directly by URL or when another repository had a valid cache.This change skips repositories whose index cache does not exist (
fs.ErrNotExist) rather than returning an early error. If no repository matches,ErrNoOwnerRepois returned and swallowed byResolveChartVersionto fall back to the default HTTP client as intended.closes #11961
Special notes for your reviewer:
ErrNoOwnerRepoerror sentinel message is intentionally kept unchanged, preserving SDK error stability.pkg/downloader/chart_downloader_test.goverifying:scanReposForURLreturnsErrNoOwnerRepowhen repository cache is empty.scanReposForURLskips unrelated repos with missing cache files and successfully finds a matching repository when present.ResolveChartVersionsuccessfully resolves direct chart URLs with an empty repository cache.If applicable:
docs neededlabel should be applied if so)