Skip to content

fix(repo-server): expire cached Helm chart archives using repo cache expiration#28061

Open
Mangaal wants to merge 11 commits into
argoproj:masterfrom
Mangaal:helm-cache-expire
Open

fix(repo-server): expire cached Helm chart archives using repo cache expiration#28061
Mangaal wants to merge 11 commits into
argoproj:masterfrom
Mangaal:helm-cache-expire

Conversation

@Mangaal
Copy link
Copy Markdown
Member

@Mangaal Mangaal commented May 29, 2026

This PR makes the repo server Helm chart cache respect the configured repo cache expiration.

Currently, downloaded Helm chart archives could remain in the on-disk chart cache even after the repo cache expires. This meant Argo CD could continue to extract a stale cached chart instead of refreshing it according to --repo-cache-expiration.

The change adds a chartCacheExpiration option to the Helm client and wires it from the repo server’s existing repo cache expiration setting. During chart extraction, the Helm client checks the cached chart archive’s modification time. If the archive is older than the configured expiration, it is removed and fetched again before extraction.

Closes #4002.

This PR doesn't fully address the issue where the Helm chart doesn't detect changes unless the version is updated. This can be used as a workaround for the root issue #24139.
Checklist:

  • Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
  • The title of the PR states what changed and the related issues number (used for the release note).
  • The title of the PR conforms to the Title of the PR
  • I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
  • I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
  • Does this PR require documentation updates?
  • I've updated documentation as required by this PR.
  • I have signed off all my commits as required by DCO
  • I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
  • My build is green (troubleshooting builds).
  • My new feature complies with the feature status guidelines.
  • I have added a brief description of why this PR is necessary and/or what this PR solves.

@Mangaal Mangaal requested a review from a team as a code owner May 29, 2026 10:40
@bunnyshell
Copy link
Copy Markdown

bunnyshell Bot commented May 29, 2026

✅ Preview Environment deployed on Bunnyshell

Component Endpoints
argocd https://argocd-px5li6.bunnyenv.com/
argocd-ttyd https://argocd-web-cli-px5li6.bunnyenv.com/

See: Environment Details | Pipeline Logs

Available commands (reply to this comment):

  • 🔴 /bns:stop to stop the environment
  • 🚀 /bns:deploy to redeploy the environment
  • /bns:delete to remove the environment

@Mangaal Mangaal requested a review from Copilot May 29, 2026 10:40
@Mangaal Mangaal changed the title fix(repo-server): expire cached Helm chart archives using repo cache expiration(#4002) fix(repo-server): expire cached Helm chart archives using repo cache expiration May 29, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds configurable Helm chart cache expiration and wires it through repo-server so cached chart archives are evicted after a TTL, with a regression test confirming expiration behavior.

Changes:

  • Introduces WithChartCacheExpiration(time.Duration) and enforces expiration in nativeHelmChart.ExtractChart.
  • Plumbs cache expiration from repo-server cache config into Helm client initialization.
  • Adds a unit test + helper to validate cache eviction based on file mtime.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
util/helm/client.go Adds chart-cache TTL option + eviction logic during chart extraction.
util/helm/client_test.go Adds coverage ensuring cached charts are not reused past expiration.
reposerver/repository/repository.go Wires chart-cache expiration into Helm client opts via init constants.
reposerver/cache/cache.go Exposes repo cache expiration via a getter for wiring.
cmd/argocd-repo-server/commands/argocd_repo_server.go Reads repo cache expiration and passes it into repo-server init constants.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread util/helm/client.go
Comment thread reposerver/cache/cache.go
Comment thread util/helm/client.go Outdated
Comment thread util/helm/client.go Outdated
@Mangaal Mangaal marked this pull request as draft May 29, 2026 11:40
@codecov
Copy link
Copy Markdown

codecov Bot commented May 29, 2026

Codecov Report

❌ Patch coverage is 65.21739% with 8 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (master@19d99f5). Learn more about missing BASE report.

Files with missing lines Patch % Lines
util/helm/client.go 55.55% 6 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##             master   #28061   +/-   ##
=========================================
  Coverage          ?   64.58%           
=========================================
  Files             ?      423           
  Lines             ?    58247           
  Branches          ?        0           
=========================================
  Hits              ?    37617           
  Misses            ?    17112           
  Partials          ?     3518           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Mangaal added 11 commits May 30, 2026 09:19
Signed-off-by: Mangaal <angommeeteimangaal@gmail.com>
Signed-off-by: Mangaal <angommeeteimangaal@gmail.com>
Signed-off-by: Mangaal <angommeeteimangaal@gmail.com>
Signed-off-by: Mangaal <angommeeteimangaal@gmail.com>
Signed-off-by: Mangaal <angommeeteimangaal@gmail.com>
Signed-off-by: Mangaal <angommeeteimangaal@gmail.com>
Signed-off-by: Mangaal <angommeeteimangaal@gmail.com>
Signed-off-by: Mangaal <angommeeteimangaal@gmail.com>
Signed-off-by: Mangaal <angommeeteimangaal@gmail.com>
Signed-off-by: Mangaal <angommeeteimangaal@gmail.com>
Signed-off-by: Mangaal <angommeeteimangaal@gmail.com>
@Mangaal Mangaal force-pushed the helm-cache-expire branch from 8409f47 to c212500 Compare May 30, 2026 03:53
@Mangaal Mangaal marked this pull request as ready for review May 30, 2026 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

repo-cache-expiration is ignored for helm repositories

2 participants