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

action: download default release assets to sign #46

Merged
merged 5 commits into from
Feb 15, 2023

Conversation

tnytown
Copy link
Collaborator

@tnytown tnytown commented Feb 15, 2023

Summary

This PR resolves #44. We now do the following on releases if release-signing-artifacts is enabled:

  1. Download the default release artifacts, i.e.{{ref}}.tar.gz and {{ref}}.zip;
  2. Sign for them;
  3. Upload them back to the release as {{ref}}-signed.{{extension}}, along with their signatures.

Release Note

The release-signing-artifacts setting was extended to re-upload and sign default assets. Previously, the stability of default asset hashes were not guaranteed, as they are generated on-the-fly by GitHub.

Documentation

https://docs.sigstore.dev does not seem to include documentation on the actions, but I will add a commit documenting the change to release-signing-artifacts in this repo's README.

Signed-off-by: Andrew Pan <a@tny.town>
@tnytown
Copy link
Collaborator Author

tnytown commented Feb 15, 2023

Doing this also significantly increases the number of assets attached to a release by default: https://github.com/tnytown/gh-action-sigstore-python/releases/tag/TEST_RELEASE

Signed-off-by: Andrew Pan <a@tny.town>
@woodruffw
Copy link
Member

3. Upload them back to the release as {{ref}}-signed.{{extension}}, along with their signatures.

What happens if we name it {{ref}}.{{extension}}, like the default artifacts? I'd partially expect that to fail, but if it works it might be slightly nicer from a consumer perspective (no need to understand why archives have "signed" in the name).

https://docs.sigstore.dev/ does not seem to include documentation on the actions, but I will add a commit documenting the change to release-signing-artifacts in this repo's README.

Yep, the README is good -- this action isn't documented at all on the main Sigstore website. That's something we should consider in the future, though!

Doing this also significantly increases the number of assets attached to a release by default: https://github.com/tnytown/gh-action-sigstore-python/releases/tag/TEST_RELEASE

Yeah -- the .crt and .sig outputs are technically redundant now that .sigstore is present, so I think we can add an option like bundle-only: bool with a default of false that'll control whether we emit those.

Then, with a 2.0 release, we can switch that setting to true.

Copy link
Member

@woodruffw woodruffw left a comment

Choose a reason for hiding this comment

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

LGTM overall! Two small nitpicks.

action.py Outdated Show resolved Hide resolved
action.py Outdated Show resolved Hide resolved
@woodruffw
Copy link
Member

Yeah -- the .crt and .sig outputs are technically redundant now that .sigstore is present, so I think we can add an option like bundle-only: bool with a default of false that'll control whether we emit those.

Then, with a 2.0 release, we can switch that setting to true.

Just to clarify: We can do this with a follow-up PR. I'll make an issue to track that.

@tnytown
Copy link
Collaborator Author

tnytown commented Feb 15, 2023

What happens if we name it {{ref}}.{{extension}}, like the default artifacts? I'd partially expect that to fail, but if it works it might be slightly nicer from a consumer perspective (no need to understand why archives have "signed" in the name).

I swapped it to {{ref}}-signed.{{extension}} when trying to fix another issue with artifact upload. I'll try {{ref}}.{{extension}} again to see if it works!

@woodruffw
Copy link
Member

Cool! If it doesn't, we could always do something generic like release.{{extension}} -- the stable URL contains the {{ref}} anyways 🙂

Signed-off-by: Andrew Pan <a@tny.town>
@tnytown tnytown marked this pull request as ready for review February 15, 2023 20:33
woodruffw
woodruffw previously approved these changes Feb 15, 2023
Copy link
Member

@woodruffw woodruffw left a comment

Choose a reason for hiding this comment

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

LGTM! Small tweaks but I'll merge those in and get this merged.

action.py Outdated Show resolved Hide resolved
action.py Outdated Show resolved Hide resolved
action.py Outdated Show resolved Hide resolved
Signed-off-by: William Woodruff <william@yossarian.net>
@woodruffw
Copy link
Member

CI is broken because of third-party PRs not having access to OIDC, sigh. I'll tweak all-tests-pass so that it allows skippage.

@woodruffw woodruffw merged commit 7643db0 into sigstore:main Feb 15, 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.

Figure out how to sign for the default release assets
2 participants