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

Invert upload flag to allow for not uploading attestation #979

Merged
merged 1 commit into from
Oct 31, 2021

Conversation

luhring
Copy link
Contributor

@luhring luhring commented Oct 31, 2021

Summary

The cosign attest command has an --upload flag, designed to tell cosign to upload the generated attestation to the registry.

However, this bool value is set to true by default, which ends up meaning that there's no way to instruct cosign not to upload the generated attestation (IIUC).

This PR changes the flag to --no-upload, which inverts the logic and behaves as you'd expect. Without the flag, the attestation is uploaded. With the flag, the attestation is not uploaded, and instead, the attestation DSSE is sent to stdout.

Note: This is a breaking change for the CLI, since it removes an existing flag. The flag hadn't been affecting cosign's execution, so I'm not sure how many folks were using it. But regardless, I'm happy to approach this a different way to make the change non-breaking, just let me know!

Ticket Link

N/A

Release Note

attest: replaces '--upload' flag with a '--no-upload' flag

@dlorenc
Copy link
Member

dlorenc commented Oct 31, 2021

Note: This is a breaking change for the CLI, since it removes an existing flag. The flag hadn't been affecting cosign's execution, so I'm not sure how many folks were using it. But regardless, I'm happy to approach this a different way to make the change non-breaking, just let me know!

If the flag was broken I see no problem fixing it like this!

@luhring luhring force-pushed the enable-not-uploading-attestation branch from fe3d5eb to b65ce9d Compare October 31, 2021 19:03
@luhring luhring force-pushed the enable-not-uploading-attestation branch from b65ce9d to 66d9d93 Compare October 31, 2021 19:21
@dlorenc dlorenc merged commit 58f8d20 into sigstore:main Oct 31, 2021
@github-actions github-actions bot added this to the v1.3.0 milestone Oct 31, 2021
@luhring luhring deleted the enable-not-uploading-attestation branch November 1, 2021 23:23
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.

2 participants