Open
Conversation
2608132 to
bd3a0d2
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds support for nested ref+ expressions, allowing inner expressions to be resolved before outer ones. For example, ref+echo://ref+envsubst://$VAR/path will first resolve the inner ref+envsubst expression, then use the result in the outer ref+echo expression. This enables dynamic parameterization of provider URIs, addressing issue #794.
Changes:
- Added
resolveInnerRefsfunction to detect and resolve nested expressions inside-out before normal expansion - Integrated nested resolution into
InStringandInValuemethods with zero-overhead fast path for non-nested cases - Added comprehensive unit and integration tests covering basic nesting, triple nesting, and practical use cases
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| pkg/expansion/expand_match.go | Core implementation: added resolveInnerRefs function with depth-limited recursion and integration into existing expansion methods |
| pkg/expansion/expand_match_test.go | Added unit tests for nested expressions including depth limit protection and direct resolver testing |
| vals_test.go | Added integration tests TestGetNested and TestEvalNested covering practical nested expression scenarios |
bd3a0d2 to
5af31ca
Compare
5af31ca to
e41dfeb
Compare
Contributor
Author
|
@yxxhero PR ready for review |
Member
|
@aditmeno please rebase the code. |
Resolve nested ref+ expressions like ref+echo://ref+envsubst://$VAR/path by pre-processing inside-out before the main expansion loop runs. Closes helmfile#794 Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>
e41dfeb to
30161ec
Compare
Contributor
Author
Rebased |
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.
Summary
Resolves #794 — adds support for nested
ref+expressions likeref+echo://ref+envsubst://$VAR/path, where inner expressions resolve first (inside-out) before the outer expression uses the resolved value.This enables dynamic parameterization of provider URIs — e.g., resolving a Vault path from an environment variable before fetching the secret.
Approach
A pre-processing step (
resolveInnerRefs) runs beforeInStringandInValue:ref+/secretref+prefix positionsref+prefixesLookup, substitute into the stringInString/InValueflowExample trace
What does NOT change
DefaultRefRegexp— unchangedLookupsignature — stillfunc(string) (interface{}, error)InValuetype preservation — full-match detection intactInMap— unchanged (delegates toInValue)vals.go— no changes at allshouldExpand/Onlyfilter — respected in nested resolutionFiles changed
pkg/expansion/expand_match.goresolveInnerRefs, integrate intoInStringandInValuepkg/expansion/expand_match_test.govals_test.goTestGetNestedandTestEvalNestedintegration testsTest plan
TestGet,TestExec,TestEvalNodes, etc.)TestResolveInnerRefs(direct resolver tests),TestResolveInnerRefsDepthLimitTestGetNested(echo, triple echo, envsubst-in-echo),TestEvalNested(map path viaEval())