-
Notifications
You must be signed in to change notification settings - Fork 783
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
Add (skopeo generate-sigstore-key) #1869
Conversation
cmd/skopeo/generate_sigstore_key.go
Outdated
func generateSigstoreKeyCmd() *cobra.Command { | ||
var opts generateSigstoreKeyOptions | ||
cmd := &cobra.Command{ | ||
Use: "generate-sigstore-key --output-prefix PREFIX", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m not sure about the --output-prefix
approach — an alternative is to have --output-public
and --output--private
, which is more explicit, but also much more typing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea 👍 Naming is always hard and I do not have another idea.
} | ||
|
||
pubKeyPath := opts.outputPrefix + ".pub" | ||
privateKeyPath := opts.outputPrefix + ".private" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.private
or just .key
?
.key
is somewhat traditional, OTOH users publish private keys often enough that making that harder seems worthwhile.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 on .private
. I am very likely such a user that would do it on low caffeine.
747ef09
to
e3682f6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (minus docs)
cmd/skopeo/generate_sigstore_key.go
Outdated
func generateSigstoreKeyCmd() *cobra.Command { | ||
var opts generateSigstoreKeyOptions | ||
cmd := &cobra.Command{ | ||
Use: "generate-sigstore-key --output-prefix PREFIX", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea 👍 Naming is always hard and I do not have another idea.
} | ||
|
||
pubKeyPath := opts.outputPrefix + ".pub" | ||
privateKeyPath := opts.outputPrefix + ".private" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 on .private
. I am very likely such a user that would do it on low caffeine.
Now vendors a merged c/image commit, and includes a man page. Ready for review and possible merging. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
besides the nit LGTM
|
||
## OPTIONS | ||
|
||
**--output=prefix** _prefix_ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
**--output=prefix** _prefix_ | |
**--output-prefix=prefix** _prefix_ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! Fixed I think, not quite as suggested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
But you may need a last wrestle with CI:
hack/man-page-checker
WARNING: skopeo generate-sigstore-key: man page shows '[options]', help does not show [command options]
make: *** [Makefile:245: validate-docs] Error 1
ad7bf29
to
f72493f
Compare
Sorry about that, the script doesn’t run on macOS. It is passing now. |
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Depends on unmerged containers/image#1810 , missing documentation - see FIXMEs
@vrothberg PTAL.