fix(cue): accept '-' and '.' in variable references - #235
Open
s3onghyun wants to merge 1 commit into
Open
Conversation
A variable name is allowed to contain '-' and '.' (name rule "^[a-zA-Z0-9_.-]+$"), but #variableSyntaxRegex only accepted word characters. As a result a datasource referencing a validly-named variable such as $my-datasource failed CUE validation with `out of bound =~"^\$\w+$"`. Widen the reference character set to match the variable name rule. #variableSyntaxRegex is an anchored full-match validator, so this only accepts more values and is not a breaking change. Fixes perses/perses#4327 Signed-off-by: s3onghyun <s3onghyun@users.noreply.github.com>
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.
What / Why
Fixes the validation side of perses/perses#4327.
A datasource can reference a variable by name (
datasource: "$myVar"). The reference is validated incue/common/datasource.cueagainst#variableSyntaxRegex, which was"^\\$\\w+$"— i.e.$followed by word characters only.But a variable name is allowed to contain
-and.(the name rule is"^[a-zA-Z0-9_.-]+$"). So a perfectly valid variable such asmy-datasourcecan be created, yet referencing it as$my-datasourcefails CUE validation:which is exactly the error reported in perses/perses#4327 (reproduced on
v0.54.0).Change
Widen
#variableSyntaxRegexso the reference character set matches the variable name rule:This is not a breaking change:
#variableSyntaxRegexis an anchored full-match validator (^…$) on the datasource field value, so widening it only accepts more strings — nothing that validated before stops validating. Because it's a full-match validator (not an extraction regex over a larger string), there's no greedy-match ambiguity here.Tests
Added regression cases to
cue-test/common/datasource.cuefor$ds-varand$ds.var. Verified withmake test-cue:out of bound =~"^\$\w+$"(reproducing the bug);$ds/var) is still rejected, so the set isn't over-widened.Scope note
The report's root cause is a mismatch between the name rule and the reference grammar. This PR fixes the CUE validator, which is what produces the user-visible error. There is a second, independent reference parser in the main repo —
variableSyntaxRegexp(\$(\w+)) inpkg/model/api/v1/utils/variable_build_order.go, used for dependency ordering. Widening that one needs more care because it extracts from arbitrary query strings and deliberately skips purely-numeric matches to avoid colliding with PromQL positional refs ($1,$2); a naive[\w.-]+there would capture things like$1-foo. I'm happy to do that as a separate follow-up in perses/perses if you'd like it kept consistent.cc @Nexucis — @Labiote endorsed the non-breaking direction on the issue and asked for your confirmation; opening this as the concrete proposal.