feat: added parsing and interpolation for component.* variables#1839
feat: added parsing and interpolation for component.* variables#1839N1lzh wants to merge 7 commits into
Conversation
|
I just noticed that while I worked and procrastinated this, another PR was opened for a similar topic. This implementation here, however, allows component.* interpolation in the whole file and not only the includes. |
There was a problem hiding this comment.
4 issues found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/parser.ts">
<violation number="1" location="src/parser.ts:397">
P1: Rejects valid `component.version` values when they are null by asserting on the resolved value’s truthiness.</violation>
<violation number="2" location="src/parser.ts:398">
P2: Component interpolation injects unescaped raw string values into JSON text before `JSON.parse`, which can break parsing or alter values when component fields contain JSON-significant characters (e.g., `"`).</violation>
</file>
<file name="src/parser-includes.ts">
<violation number="1" location="src/parser-includes.ts:263">
P2: Unconditionally resolving the component SHA forces a remote `git ls-remote` during parsing, which breaks offline/cached runs and adds avoidable latency.</violation>
<violation number="2" location="src/parser-includes.ts:503">
P1: `getComponentSha` cannot resolve commit-SHA component refs and returns the wrong SHA for annotated tags.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
2 issues found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/parser.ts">
<violation number="1" location="src/parser.ts:397">
P2: Unsupported `component.*` keys are no longer validated and can serialize as `undefined`, breaking config parsing or producing malformed output.</violation>
<violation number="2" location="src/parser.ts:397">
P1: Component interpolation stringifies an already-serialized JSON string, which can produce invalid JSON with doubled quotes.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
Well this other MR #1836 is also for the whole file, included the spec document where the component could also be reference. (I amend it) But there should also be other ways to do it, maybe better. I’m not attached to my code in #1836 and any merge version is ok with me. I had simply preferred contributing directly rather than opening another issue… But it seems we concur to same code resolution :-) |
@jrd Not attached to my code either, I just need this to work ;) What I noticed on your code is, though, that you do not handle |
|
Yes my implementation is a bit crude about So yes having a more complete Can you amend your commit that way? Then I delete my PR. |
| // is specified in the specs via component: [{interpolationKey}, ...] | ||
| const configurationWithInterpolatedInputsAndComponent = configurationWithInterpolatedInputs | ||
| .replaceAll( | ||
| /(?<firstChar>.)?(?<secondChar>.)?\$\[\[\s*component.(?<interpolationKey>[\w-]+)\s*\]\](?<lastChar>[^$])?/g, |
There was a problem hiding this comment.
firstChar, secondChar and lastChar are useless for component interpolation as this is just a raw replace and there is no array or object type. This makes it more complicated that it should.
| @@ -328,7 +328,7 @@ export class Parser { | |||
| const inputsSpecification: any = fileData[0]; | |||
There was a problem hiding this comment.
This should be also be parsed for component.XXX interpolation.
I ran into the problem that while input interpolations like
$[[ input.variable ]]were replaced successfully in the yml files, component interpolations were not.Gitlab component interpolations allows four different variables,
name,reference,versionandsha, with the following characteristics:As the current
ParsedComponenttype differed a bit from these characteristics, I made the following changes to it:nametocomponentPathas this property is the full path to the component starting from the project path.reftoeffectiveRefas this is the already resolved reference, meaning either the reference specified or the concrete semantic version if existing.name,reference,versionandshathat mimic the characteristics of the gitlab properties.These properties get populated in the
parseIncludeComponentfunction inparser-includes.ts.In the
parser.tsI added the replacement for the component variables in the same way as it was done for input variables. However, the logic is a bit easier, since there are only four different variables that all have the same type.Summary by cubic
Adds parsing and interpolation for GitLab component variables so
$[[ component.name|reference|version|sha ]]resolves in included templates. Also improves version/ref/sha resolution and uses the resolved ref to locate and expand includes.New Features
componentPath,effectiveRef,name,reference,version(nullable), andsha.~latest,x,x.y) and compute SHAs (remote viagit ls-remote, local viagit rev-parse).$[[ component.* ]]during YAML load using acomponentcontext; usecomponentPath/effectiveReffor downloading and locating includes and expanding inner local includes.Bug Fixes
component.versionvalues per GitLab spec.Written for commit d9852a3. Summary will update on new commits.