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

Add --yes flag cosign import-key-pair to skip the overwrite confirmation. #3383

Merged
merged 2 commits into from Nov 28, 2023
Merged

Add --yes flag cosign import-key-pair to skip the overwrite confirmation. #3383

merged 2 commits into from Nov 28, 2023

Conversation

zhaoyonghe
Copy link
Contributor

This PR closes #3382.

Summary

Users may call cosign import-key-pair and then cosign sign multiple times to sign multiple images in CI/CD pipelines. And cannot confirm overwrite:

WARNING: File imported.key already exists. Overwrite?
22:27:00 Are you sure you would like to continue? [y/N] 2023/11/20 22:27:00 unable to import key pair: user declined the prompt
22:27:00 ERROR: exit status 1

It is better to add a --yes flag to skip the overwrite confirmation.

Release Note

Added --yes flag in cosign import-key-pair to skip the overwrite confirmation.

Documentation

Already updated doc/cosign_import-key-pair.md.

Signed-off-by: zhaoyonghe <yonghe.zhao@yahoo.com>
@@ -39,4 +41,7 @@ func (o *ImportKeyPairOptions) AddFlags(cmd *cobra.Command) {
cmd.Flags().StringVarP(&o.OutputKeyPrefix, "output-key-prefix", "o", "import-cosign",
"name used for outputted key pairs")
_ = cmd.Flags().SetAnnotation("output-key-prefix", cobra.BashCompFilenameExt, []string{})

cmd.Flags().BoolVarP(&o.SkipConfirmation, "yes", "y", false,
"skip confirmation prompts for non-destructive operations")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Question: is file overwrite considered as destructive operation? Should we change it to a different name?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, this should say destructive. Given the simplicity of this command, you could just say "skip confirmation prompts for overwriting existing key"

@zhaoyonghe zhaoyonghe marked this pull request as ready for review November 21, 2023 20:45
Copy link
Contributor

@haydentherapper haydentherapper left a comment

Choose a reason for hiding this comment

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

LGTM, just one comment on the flag desc

Signed-off-by: zhaoyonghe <yonghe.zhao@yahoo.com>
Copy link

codecov bot commented Nov 27, 2023

Codecov Report

Attention: 8 lines in your changes are missing coverage. Please review.

Comparison is base (1a8fa53) 30.23% compared to head (6058971) 30.22%.
Report is 3 commits behind head on main.

Files Patch % Lines
cmd/cosign/cli/importkeypair/import_key_pair.go 50.00% 4 Missing ⚠️
cmd/cosign/cli/options/import_key_pair.go 0.00% 3 Missing ⚠️
cmd/cosign/cli/import_key_pair.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3383      +/-   ##
==========================================
- Coverage   30.23%   30.22%   -0.02%     
==========================================
  Files         155      155              
  Lines        9958     9962       +4     
==========================================
  Hits         3011     3011              
- Misses       6497     6501       +4     
  Partials      450      450              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@haydentherapper haydentherapper enabled auto-merge (squash) November 27, 2023 19:36
@haydentherapper haydentherapper merged commit 878b6c7 into sigstore:main Nov 28, 2023
28 checks passed
@github-actions github-actions bot added this to the v2.3.0 milestone Nov 28, 2023
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.

Add --yes flag in cosign import-key-pair to skip the overwrite confirmation.
2 participants