feat: support spec:inputs for root pipeline via --input and --inputs-file - #1814
Conversation
There was a problem hiding this comment.
4 issues found across 15 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-includes.ts">
<violation number="1" location="src/parser-includes.ts:177">
P1: Component input merge order is incorrect: trailing `cliGlobalInputs` overrides component-specific CLI inputs on key conflicts.</violation>
</file>
<file name="src/argv.ts">
<violation number="1" location="src/argv.ts:226">
P2: Component-specific `--input` parsing is too restrictive (`[\w-]+`) and can misparse valid component names containing `/`, causing inputs to be dropped or assigned to the wrong component.</violation>
<violation number="2" location="src/argv.ts:243">
P1: Global and component inputs share one object namespace, allowing key collisions that can cause runtime errors or overwrite component data.</violation>
<violation number="3" location="src/argv.ts:243">
P1: User-controlled `component`/`key` are assigned into a plain object, allowing `__proto__`-based prototype pollution via `--input` parsing.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
@firecow could you please review this PR when you get a chance? This adds support for root |
|
Nice feature — root 1. Precedence order in the description doesn't match the code The description says global CLI (#2) beats component-specific file (#3), but const mergedInputs = {...(value.inputs ?? {}), ...globalInputs, ...fileComponentInputs, ...cliComponentSpecificInputs};The test 2. Global and component inputs share one object in At Unlikely in practice (requires naming a global input the same as a component), but the fix is straightforward — separate the namespaces. 3. Duplicated structured-format detection The 4. At |
|
Thanks for the review! Pushed a fix commit (0362bc8) addressing your feedback: #1 — Precedence docs: You're right, the code behavior (more specific wins) is correct. Updated the PR description to match:
#2 — Namespace collision: Separated #3 — Duplicated isStructured detection: Extracted #4 — JSON.parse coercion: Noted — will leave as-is for now since it's not a blocker. Can add a help text note in a follow-up if you'd like. All 29 input-related tests pass + 487 non-Docker tests pass with zero regressions. |
0362bc8 to
d219942
Compare
Kewl, god job.. Test job is failing. |
…file Add support for providing spec:inputs values to the root .gitlab-ci.yml pipeline file, not just included files. This closes firecow#1720. Changes: - Add --input KEY=value CLI flag (repeatable) for global inputs - Add --input component:KEY=value syntax for component-specific inputs - Add --inputs-file flag (default: .gitlab-ci-local-inputs.yml) - Pass inputs to root loadYaml() call in parser.ts (was hardcoded {}) - Merge external inputs into all include types (local, project, component, template, remote) - Support structured inputs file with _global and component sections - Fix absolute path handling for --inputs-file Priority order (highest to lowest): 1. Component-specific CLI inputs (--input component:key=value) 2. Global CLI inputs (--input key=value) 3. Component-specific file inputs 4. Global file inputs (_global or flat format) 5. Inline inputs in .gitlab-ci.yml (include: inputs:) 6. Spec defaults Fixes: firecow#1720
- Fix component input merge order: remove duplicate cliGlobalInputs spread that caused global CLI to override component-specific CLI inputs on key conflicts (P1) - Broaden component name regex to allow '/' for paths like templates/deploy (P2) - Add prototype pollution guard for __proto__, constructor, prototype keys in --input parsing (P1) - Add 7 new tests covering edge cases - Fix existing test that relied on buggy merge order
…uctured helper Addresses review feedback: - Separate global and component CLI inputs into _global/_components namespaces in argv.input to prevent key collisions (firecow#2) - Extract isStructuredInputsFile() and getGlobalFileInputs() to Utils to deduplicate detection logic in parser.ts and parser-includes.ts (firecow#3) - Add test for namespace collision scenario
d219942 to
4c3c13f
Compare
|
Rebased onto latest master (
Verified locally that these are not caused by our changes — ran both test suites on clean master and on our branch back-to-back, got identical results (same 8 Docker timeouts on both, predefined-variables normal/custom-ports pass on both). Our branch adds 16 tests across 6 new test files (552 → 568 total). All lightweight parsing tests, but the extra concurrency might be enough to push the service tests past their 60s timeout on a slower runner. The branch diff touches zero lines in the failing test files. Could you re-run the CI on this latest push? Should be clean on a fresh runner. |
Oh, I didn't notice it was exiting unchanged tests that failed. Sorry about that, I'll take care of those flaky tests. |
|
Seems like failing to me on with: Job is: build-backend:
extends:
- .base-backend-job
- .base-docker-job
stage: build
needs:
- unit-test-backend
script:
- /bin/bash scripts/buidscript.sh backend database $[[ inputs.push_containers ]]spec is: spec:
inputs:
push_containers:
type: boolean
default: false
description: "Allow containers to be pushed to registry"
|
|
@WatchTh1s Thanks for reporting this. A few things to check:
|
This MR contains the following updates: | Package | Change | [Age](https://docs.renovatebot.com/merge-confidence/) | [Adoption](https://docs.renovatebot.com/merge-confidence/) | [Passing](https://docs.renovatebot.com/merge-confidence/) | [Confidence](https://docs.renovatebot.com/merge-confidence/) | |---|---|---|---|---|---| | [npm:gitlab-ci-local](https://github.com/firecow/gitlab-ci-local) | `4.71.0` → `4.72.0` |  |  |  |  | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>firecow/gitlab-ci-local (npm:gitlab-ci-local)</summary> ### [`v4.72.0`](https://github.com/firecow/gitlab-ci-local/releases/tag/4.72.0) [Compare Source](firecow/gitlab-ci-local@4.71.0...4.72.0) #### What's Changed - feat: support spec:inputs for root pipeline via --input and --inputs-file by [@​gyanranjan](https://github.com/gyanranjan) in [#​1814](firecow/gitlab-ci-local#1814) - feat: implement workflow:rules:variables support ([#​1832](firecow/gitlab-ci-local#1832)) by [@​bcouetil](https://github.com/bcouetil) in [#​1833](firecow/gitlab-ci-local#1833) - chore(deps): lock file maintenance by [@​renovate](https://github.com/renovate)\[bot] in [#​1834](firecow/gitlab-ci-local#1834) - chore(deps): update all non-major by [@​renovate](https://github.com/renovate)\[bot] in [#​1835](firecow/gitlab-ci-local#1835) - fix: pass artifact paths to rsync via `--files-from` by [@​Paul-Goulpie](https://github.com/Paul-Goulpie) in [#​1825](firecow/gitlab-ci-local#1825) - chore(deps): update sonarsource/sonarqube-scan-action action to v8 by [@​renovate](https://github.com/renovate)\[bot] in [#​1842](firecow/gitlab-ci-local#1842) - fix: connect service containers to local registry network by [@​firecow](https://github.com/firecow) in [#​1847](firecow/gitlab-ci-local#1847) - test: ignore 'still running' heartbeat in stdout assertions by [@​firecow](https://github.com/firecow) in [#​1851](firecow/gitlab-ci-local#1851) - chore(deps): update github/codeql-action action to v4.35.4 by [@​renovate](https://github.com/renovate)\[bot] in [#​1843](firecow/gitlab-ci-local#1843) - chore(deps): lock file maintenance by [@​renovate](https://github.com/renovate)\[bot] in [#​1845](firecow/gitlab-ci-local#1845) - feat: add environment column to --list and --list-csv ([#​1837](firecow/gitlab-ci-local#1837)) by [@​bcouetil](https://github.com/bcouetil) in [#​1838](firecow/gitlab-ci-local#1838) - feat: support needs\[].parallel.matrix and matrix expressions by [@​inistor](https://github.com/inistor) in [#​1848](firecow/gitlab-ci-local#1848) - fix: reject empty rules array instead of silently skipping job by [@​firecow](https://github.com/firecow) in [#​1852](firecow/gitlab-ci-local#1852) - fix: respect IGNORE\_PREDEFINED\_VARS in .gitlab-ci-local-env by [@​firecow](https://github.com/firecow) in [#​1853](firecow/gitlab-ci-local#1853) - fix: reject ${VAR} in rules:if by [@​firecow](https://github.com/firecow) in [#​1854](firecow/gitlab-ci-local#1854) - fix: wait for child stdio close before resolving exec by [@​firecow](https://github.com/firecow) in [#​1855](firecow/gitlab-ci-local#1855) - chore: pin third-party actions to commit SHAs by [@​firecow](https://github.com/firecow) in [#​1857](firecow/gitlab-ci-local#1857) #### New Contributors - [@​gyanranjan](https://github.com/gyanranjan) made their first contribution in [#​1814](firecow/gitlab-ci-local#1814) - [@​inistor](https://github.com/inistor) made their first contribution in [#​1848](firecow/gitlab-ci-local#1848) **Full Changelog**: <firecow/gitlab-ci-local@4.71.0...4.72.0> </details> --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - At any time (no schedule defined) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Mend Renovate](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xNzMuNSIsInVwZGF0ZWRJblZlciI6IjQzLjE3My41IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiLCJhdXRvbWF0aW9uOmJvdC1hdXRob3JlZCIsImRlcGVuZGVuY3ktdHlwZTo6bWlub3IiXX0=-->
Summary
Adds support for providing
spec:inputsvalues to the root.gitlab-ci.ymlpipeline file — not just included files. This closes #1720.Problem
When the root
.gitlab-ci.ymlusesspec:inputs, there was no way to pass values for required inputs (those without adefault). The root file was always loaded with an empty context:Inputs on included files worked fine because
parser-includes.tsforwardedvalue.inputsfrom theinclude:block. But the root file had no equivalent mechanism.Solution
New CLI options
--input KEY=value(repeatable) — provide global inputs--input component:KEY=value— provide component-specific inputs--inputs-file PATH— path to inputs YAML file (default:.gitlab-ci-local-inputs.yml)Inputs file formats
Flat (all inputs are global):
Structured (global + component-specific):
Priority order (highest to lowest)
--input component:key=value)--input key=value)_global:or flat format).gitlab-ci.yml(include: inputs:)Changes
src/argv.tsinputsFilegetter,inputgetter withkey=valueandcomponent:key=valueparsingsrc/index.ts--inputs-fileand--inputyargs optionssrc/parser.tsloadInputs()method; rootloadYaml()now passes{inputs: rootInputs}instead of{}; absolute path support for--inputs-filesrc/parser-includes.tsinputsin opts type; merge logic for global/component-specific inputs across all include typestests/test-cases/component-inputs-cli/tests/test-cases/component-inputs-multiple/tests/test-cases/.gitignore.gitlab-ci-local-inputs.ymlfixture filesTesting
Corner cases verified
--input✅_global:✅--inputs-filepath ✅Example
Fixes #1720
Summary by cubic
Adds support for passing
spec:inputsto the root.gitlab-ci.yml, with values from CLI or an inputs file. Inputs are applied to the root and all include types.New Features
--input KEY=value(repeatable) and--input component:KEY=valuefor global and component inputs.--inputs-file PATH(default:.gitlab-ci-local-inputs.yml) with flat or structured formats (_global+ components).Bug Fixes
/in component names and blocked prototype-pollution keys (__proto__,constructor,prototype) in--input.--inputs-file; ESLint cleanups (no functional changes).Written for commit 4c3c13f. Summary will update on new commits.