cosign
cosign copied to clipboard
sigstore
Inappropriate printing to STDOUT
Generally, in Cosign we try to make STDOUT have the output of the command: the (often machine-readable) result of execution. We put status messages, errors, etc. on STDERR.
This is a common convention for CLI tools and a nice thing to do for scriptability.
However, we're pretty sloppy sometimes.
I'm looking mostly at:
- https://github.com/sigstore/cosign/blob/fa8a799784cafb0779849e69cb939c8dc6132a5e/pkg/cosign/tlog.go#L170
- https://github.com/sigstore/cosign/blob/381ba64a577505d378b49482ee34d39eeab6ea50/cmd/cosign/cli/pivcli/commands.go#L152
- https://github.com/sigstore/cosign/blob/381ba64a577505d378b49482ee34d39eeab6ea50/cmd/cosign/cli/initialize/init.go#L52
- https://github.com/sigstore/cosign/blob/d720f040bf493861c1ea8280f462c14e705990e0/cmd/cosign/cli/sign/sign.go#L250
But even some of the ones that are actually correct just work by spitting out to STDOUT in the middle of command execution:
- https://github.com/sigstore/cosign/blob/381ba64a577505d378b49482ee34d39eeab6ea50/cmd/cosign/cli/pivcli/commands.go#L265
- https://github.com/sigstore/cosign/blob/d720f040bf493861c1ea8280f462c14e705990e0/cmd/cosign/cli/tree.go#L152
My proposal is the following:
- Have the CLI top-level entry point print the output of the command. That is, something like
SaveCmdwould be implemented as a function with a return value, and then we'd hook into Cobra to print the output (if we need to stream output, we could use channels or something). - Ban (using Forbidigo) any calls to
fmt.Print{f,ln}in the Cosign codebase. - Implement a "Cosign output" package. Instead of
fmt.Fprintln(os.Stderr, "warning: foo")directly, Cosign code would callWarn(). Same for any other common output patterns. (That is, calling code can assign semantics to its output which helps make sure it goes to the right place). - Ban (most) other
fmt.*calls (we can keepfmt.Sprint*).
This has a number of benefits:
- Supporting multiple formats easily and consistently (you could pass
--format=jsonlor--format=csv) without requiring support from every command - Testability: parsing STDOUT is hard. Right now that's part of the reason that the
cmd/directory is under-tested. - Enforce consistent output: of course, this fixes the STDOUT/STDERR thing. But it also starts to unify the output across the whole tool. If we wanted to start putting all STDERR output in red, we could do that in one place easily. If we want to "tee" all output to a log file, we can do that.
The way we handle this in kubectl and gitsign is by encapsulating the streams which has the added bonus of being really easy to mock in tests.
