Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cli: silence usage on errors #265

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cli/slsa-verifier/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ For more information on SLSA, visit https://slsa.dev`,
c.AddCommand(verifyImageCmd())
// We print our own errors and usage in the check function.
c.SilenceErrors = true
c.SilenceUsage = true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this also silence usage when there is actually a flag parsing error? or just when there is an error returned by RunE?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me see: so if a flag is parsed incorrectly with this change it'll currently print

$ go run ./cli/slsa-verifier/ verify-artifact --provenance testdata/go/v1.2.0/binary-linux-amd64-workflow_dispatch.intoto.jsonl --source-uri github.com/slsa-framework/example-package testdata/go/v1.2.0/binary-linux-amd64-workflow_dispatch
unknown flag: --provenance

The problem is, that it can't distinguish between error types from flag parsing or generally from RunE. You'll see the usage string only with --help.

This seems like others must have run into this issue. Looking into it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update: There's no way to distinguish a flag parsing error from a RunE error, unless we handle them separately: spf13/cobra#340

I think the better thing to do is to switch from RunE to Run and use our own custom error handler. Will update today

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SG, i think this is what I've done in the past for this reason but my memory was fuzzy.

return c
}

Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ require (
github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826
github.com/sigstore/cosign v1.11.1
github.com/slsa-framework/slsa-github-generator v1.2.0
github.com/spf13/cobra v1.5.0
github.com/transparency-dev/merkle v0.0.1
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4
)
Expand Down Expand Up @@ -168,7 +169,6 @@ require (
github.com/soheilhy/cmux v0.1.5 // indirect
github.com/spf13/afero v1.8.2 // indirect
github.com/spf13/cast v1.5.0 // indirect
github.com/spf13/cobra v1.5.0 // indirect
github.com/spf13/jwalterweatherman v1.1.0 // indirect
github.com/spf13/pflag v1.0.5 // indirect
github.com/spf13/viper v1.12.0 // indirect
Expand Down