fix(compose): reject unknown subcommands under bridge/transformations - #14143
fix(compose): reject unknown subcommands under bridge/transformations#14143glours wants to merge 1 commit into
Conversation
docker-agent
left a comment
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
The PR correctly wires rejectUnknownSubcommand as RunE on the bridgeCommand and transformersCommand parent commands so that unknown subcommands exit 1 instead of 0. The core logic is sound: cobra dispatches unknown tokens to the parent's RunE, known subcommands are dispatched directly so the new guard is never incorrectly invoked, and the tests cover all the key scenarios (unknown subcommand, no subcommand, nested transformations).
One low-severity finding below: a discarded cmd.Help() error in the non-zero-args path.
| if len(args) == 0 { | ||
| return cmd.Help() | ||
| } | ||
| _ = cmd.Help() |
There was a problem hiding this comment.
[low] Error from cmd.Help() silently discarded before returning StatusError
When args is non-empty, rejectUnknownSubcommand calls _ = cmd.Help() and discards the error before returning the cli.StatusError. If the cobra output writer returns an I/O error (e.g. a broken pipe), the help text won't render and the error will be silently swallowed — the caller only sees the StatusError, with no indication that help rendering failed and no help context displayed.
The conventional Go pattern is to propagate the error:
| _ = cmd.Help() | |
| if err := cmd.Help(); err != nil { | |
| return err | |
| } |
| Confidence | Score |
|---|---|
| 🟢 strong | 100/100 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
bridgeCommand and transformersCommand had no RunE, so an unknown subcommand (e.g. "bridge zzz") fell through cobra's default non-Runnable path: help printed, exit 0. Scripts couldn't distinguish a typo from success. Mirrors the same fix already shipped on the compose root command. Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>
d382891 to
dcea158
Compare
2 participants
What I did
bridgeCommandandtransformersCommandhad noRunE, so an unknown subcommand (e.g. "bridge zzz") fell through cobra's default non-Runnable path: help printed, exit 0. Scripts couldn't distinguish a typo from success. Mirrors the same fix already shipped on the compose root command.Related issue
https://docker.atlassian.net/browse/DDB-666
(not mandatory) A picture of a cute animal, if possible in relation to what you did
