Skip to content

fix(#212): space separated inputs are now supported#213

Merged
Joseph94m merged 2 commits into
getplumber:mainfrom
p1-dta:fix/template-quotes
Jun 4, 2026
Merged

fix(#212): space separated inputs are now supported#213
Joseph94m merged 2 commits into
getplumber:mainfrom
p1-dta:fix/template-quotes

Conversation

@p1-dta
Copy link
Copy Markdown
Contributor

@p1-dta p1-dta commented May 28, 2026

See issue #212

I took the liberty to remove unnecessary quotes. Only required quotes are left so all quotes are meaningful and purposefully placed.

@Joseph94m
Copy link
Copy Markdown
Collaborator

Joseph94m commented Jun 1, 2026

@p1-dta Thanks for taking this on. I ran the PR template end-to-end against a sandbox project (gitlab.com/getplumber/examples/go-build-test-compliant) before commenting, and I want to walk through what I found, because I think the diagnosis lands on a different line than the one this PR rewrites.

Live runs against the PR template

Three pipelines, same component reference (@quotes-unquotes), three input shapes:

A. Defaults only, no overrides

https://gitlab.com/getplumber/examples/go-build-test-compliant/-/jobs/14627158012

/bin/sh: eval: line 200: plumber-report.json: not found
WARNING: plumber-report.json: no matching files
WARNING: plumber-pbom.json: no matching files
WARNING: plumber-cyclonedx-sbom.json: no matching files
WARNING: gl-sast-report.json: no matching files
ERROR: Job failed: exit code 127

No spaces in any filename. Plain default usage. Exit 127, all four reports missing.

B. The original #212 scenario reproduced

https://gitlab.com/getplumber/examples/go-build-test-compliant/-/jobs/14627249681

Inputs:

output_file: "plumber report.json"
/bin/sh: eval: line 200: plumber report.json: not found
WARNING: plumber report.json: no matching files
WARNING: plumber-pbom.json: no matching files
WARNING: plumber-cyclonedx-sbom.json: no matching files
WARNING: gl-sast-report.json: no matching files
ERROR: Job failed: exit code 127

Same warning fingerprint as the original bug report. The other three artifacts are also missing despite their defaults being untouched, which tells me the regression is per value-flag, not per overridden input.

C. All value-flag inputs explicitly empty

https://gitlab.com/getplumber/examples/go-build-test-compliant/-/jobs/14627307256

Inputs:

output_file: ""
pbom_file: ""
pbom_cyclonedx_file: ""
glsast_file: ""

Exit 0. /plumber analyze runs to completion, compliance table renders. No not found lines anywhere. The artifact uploader still warns on the four empty paths but the job succeeds.

This is the only input shape I found where the PR's template runs cleanly, and it's the shape where every report artifact is disabled.

What I think is happening

The image is alpine:3.22, so /bin/sh is busybox ash. In ash (and any POSIX shell), this line:

OUTPUT_FLAG=--output "plumber-report.json"

parses as a transient assignment (OUTPUT_FLAG=--output, scoped to one command) followed by an attempt to execute "plumber-report.json" as a command. That's where the not found and the exit 127 come from. OUTPUT_FLAG never gets set, the --output flag never reaches /plumber analyze, no report is written.

The deeper issue: the actual root cause of #212 wasn't the assignment form, it was the unquoted $OUTPUT_FLAG at the call site word-splitting on whitespace. That's why the original bug split --output plumber report.json into three argv entries. This PR rewrites the assignment but leaves the call site unquoted, so the bug just changes shape from "wrong split" to "flag silently dropped", and now it fires on the default case too.

Pipeline C confirms this: when every value flag is empty, the if [ -n "..." ] guards short-circuit and the broken assignment line never runs, so the script makes it to /plumber analyze without tripping. Anything else breaks.

A direction worth considering

The shape that fixes both the assignment and the call site in POSIX (so it works under ash without bash, which the image doesn't ship) is the positional accumulator:

set --
[ -n "$[[ inputs.output_file ]]" ]   && set -- "$@" --output "$[[ inputs.output_file ]]"
[ -n "$[[ inputs.pbom_file ]]" ]     && set -- "$@" --pbom   "$[[ inputs.pbom_file ]]"
[ "$[[ inputs.verbose ]]" = "true" ] && set -- "$@" --verbose
#
/plumber analyze --gitlab-url "$[[ inputs.server_url ]]" ... "$@"

Populated flags get appended as distinct argv entries that "$@" preserves verbatim; empty flags simply aren't appended, so there's no stray-empty-argv to leak. Does that fit the style you were going for?

One meta-point on test coverage

make test passes on this branch because templates/plumber.yml isn't exercised by anything in the Go suite. That's how a regression this wide would land unnoticed. Would it be worth a small CI job that runs the component end-to-end against a fixture project (default inputs + output_file: "plumber report.json"), so #212 and its cousins get caught automatically next time?

Happy to be wrong if you can share a green pipeline using the PR template with non-empty defaults. Otherwise, want to push a follow-up using set -- so the call-site word-splitting and the assignment both land in one go? Thanks again for digging into this.

@p1-dta p1-dta force-pushed the fix/template-quotes branch from 847e965 to 4a802ed Compare June 2, 2026 09:49
@p1-dta
Copy link
Copy Markdown
Contributor Author

p1-dta commented Jun 2, 2026

@p1-dta Thanks for taking this on. I ran the PR template end-to-end against a sandbox project (gitlab.com/getplumber/examples/go-build-test-compliant) before commenting, and I want to walk through what I found, because I think the diagnosis lands on a different line than the one this PR rewrites.

Live runs against the PR template

Three pipelines, same component reference (@quotes-unquotes), three input shapes:

A. Defaults only, no overrides

https://gitlab.com/getplumber/examples/go-build-test-compliant/-/jobs/14627158012

/bin/sh: eval: line 200: plumber-report.json: not found
WARNING: plumber-report.json: no matching files
WARNING: plumber-pbom.json: no matching files
WARNING: plumber-cyclonedx-sbom.json: no matching files
WARNING: gl-sast-report.json: no matching files
ERROR: Job failed: exit code 127

No spaces in any filename. Plain default usage. Exit 127, all four reports missing.

B. The original #212 scenario reproduced

https://gitlab.com/getplumber/examples/go-build-test-compliant/-/jobs/14627249681

Inputs:

output_file: "plumber report.json"
/bin/sh: eval: line 200: plumber report.json: not found
WARNING: plumber report.json: no matching files
WARNING: plumber-pbom.json: no matching files
WARNING: plumber-cyclonedx-sbom.json: no matching files
WARNING: gl-sast-report.json: no matching files
ERROR: Job failed: exit code 127

Same warning fingerprint as the original bug report. The other three artifacts are also missing despite their defaults being untouched, which tells me the regression is per value-flag, not per overridden input.

C. All value-flag inputs explicitly empty

https://gitlab.com/getplumber/examples/go-build-test-compliant/-/jobs/14627307256

Inputs:

output_file: ""
pbom_file: ""
pbom_cyclonedx_file: ""
glsast_file: ""

Exit 0. /plumber analyze runs to completion, compliance table renders. No not found lines anywhere. The artifact uploader still warns on the four empty paths but the job succeeds.

This is the only input shape I found where the PR's template runs cleanly, and it's the shape where every report artifact is disabled.

What I think is happening

The image is alpine:3.22, so /bin/sh is busybox ash. In ash (and any POSIX shell), this line:

OUTPUT_FLAG=--output "plumber-report.json"

parses as a transient assignment (OUTPUT_FLAG=--output, scoped to one command) followed by an attempt to execute "plumber-report.json" as a command. That's where the not found and the exit 127 come from. OUTPUT_FLAG never gets set, the --output flag never reaches /plumber analyze, no report is written.

The deeper issue: the actual root cause of #212 wasn't the assignment form, it was the unquoted $OUTPUT_FLAG at the call site word-splitting on whitespace. That's why the original bug split --output plumber report.json into three argv entries. This PR rewrites the assignment but leaves the call site unquoted, so the bug just changes shape from "wrong split" to "flag silently dropped", and now it fires on the default case too.

Pipeline C confirms this: when every value flag is empty, the if [ -n "..." ] guards short-circuit and the broken assignment line never runs, so the script makes it to /plumber analyze without tripping. Anything else breaks.

A direction worth considering

The shape that fixes both the assignment and the call site in POSIX (so it works under ash without bash, which the image doesn't ship) is the positional accumulator:

set --
[ -n "$[[ inputs.output_file ]]" ]   && set -- "$@" --output "$[[ inputs.output_file ]]"
[ -n "$[[ inputs.pbom_file ]]" ]     && set -- "$@" --pbom   "$[[ inputs.pbom_file ]]"
[ "$[[ inputs.verbose ]]" = "true" ] && set -- "$@" --verbose
#
/plumber analyze --gitlab-url "$[[ inputs.server_url ]]" ... "$@"

Populated flags get appended as distinct argv entries that "$@" preserves verbatim; empty flags simply aren't appended, so there's no stray-empty-argv to leak. Does that fit the style you were going for?

One meta-point on test coverage

make test passes on this branch because templates/plumber.yml isn't exercised by anything in the Go suite. That's how a regression this wide would land unnoticed. Would it be worth a small CI job that runs the component end-to-end against a fixture project (default inputs + output_file: "plumber report.json"), so #212 and its cousins get caught automatically next time?

Happy to be wrong if you can share a green pipeline using the PR template with non-empty defaults. Otherwise, want to push a follow-up using set -- so the call-site word-splitting and the assignment both land in one go? Thanks again for digging into this.

Hi, thanks for the feedback. I replicated your errors, you are correct. I am confused as I remember doing some testing and it was working, but maybe it was an already better version of the template ? Since the plumber CICD doesn't check the template, I have multiple other repo for testing and I might have confuse which version I wanted to use. Your suggestion to fix all issues what my original solution but I looked for an intermediary step, trying to first address the quote error before performing a refactoring for readability. Then, I got confuse in my testing.

I updated the MR with my refactoring code, originally planned for a subsequent MR, and from my own testing, this one should works.

@Joseph94m Joseph94m force-pushed the fix/template-quotes branch from 4a802ed to ce36bee Compare June 3, 2026 12:56
@Joseph94m
Copy link
Copy Markdown
Collaborator

Joseph94m commented Jun 3, 2026

@p1-dta Done reviewing. Rebased and resolved a conflict with main.
Can merge when pipeline is green.

I tested on the same project as I did before - all the scenarios and it works.

Thanks for the PR.

@Joseph94m Joseph94m merged commit d1c90c7 into getplumber:main Jun 4, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants