Fix | Stream branch root for local Helm charts with out-of-chart value files#444
Merged
Merged
Conversation
Owner
Author
|
Added an end-to-end integration test for this regression (commit 5201b29):
Verified locally:
So the new integration test reproduces the regression and guards against it. |
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
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.
Problem
v0.2.9 (PR #443) regressed
--render-method repo-server-apifor local Helm charts whosehelm.valueFilesreference files outside the chart directory. Rendering fails with:PR #443 added an optimization: for any local source directory containing
Chart.yaml, the repo-server stream is narrowed to only the chart directory (andsource.Pathis cleared) to avoid sending unrelated monorepo files (symlink safety checks).That optimization assumes the chart is self-contained. It is not when a chart pulls value files from a sibling directory tree. Argo CD resolves a leading-slash value file relative to the repository root (
util/io/path/resolved.go->ResolveValueFilePathOrUrl), and a relative value file relative to the chart directory. So a value file like:lives outside the narrowed stream root, so the file is no longer in the tarball ->
no such file or directory. This is the Helm mirror image of the kustomize../../basesibling-reference case the original comment already warned about.Fix
buildStreamDirForLocalSourcenow only narrows to the chart directory when nohelm.valueFilesescape the chart directory. The newhasOutOfChartValueFilehelper mirrors Argo CD's resolution rules:https://...) are remote and ignored,and a value file "escapes" when its resolved path is not within the chart directory. When any value file escapes, the whole branch folder is streamed (with
Pathretained) so the out-of-chart files remain reachable. The PR #443 optimization (and its symlink-scoping behaviour) is preserved for self-contained charts.Tests
Added unit tests in
pkg/reposerverextract/extract_test.go:../out-of-chart value file -> streams branch rootBoth regression tests fail on
mainand pass with this change. Full unit suite is green andgolangci-lintreports 0 issues. PR #443's symlink-scoping test still passes.