-
Notifications
You must be signed in to change notification settings - Fork 7
refactor: drop sigstore-protobuf-specs dependency #132
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
Conversation
|
Huh, no idea why this is failing in CI but not locally for me: Either way this is an overly strict check in |
|
@woodruffw now that |
| resp = requests.get( | ||
| "https://raw.githubusercontent.com/sigstore-conformance/extremely-dangerous-public-oidc-beacon/refs/heads/current-token/oidc-token.txt" |
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.
unfortunately GitHub cached raw.githubusercontent.com quite aggressively when I last tried this -- it usually works but the error rate was still annoyingly high. This is why conformance uses the clumsy looking "git clone" approach https://github.com/sigstore/sigstore-conformance/blob/e0997c248c40ee615c1e8aa1e3ee043a62920951/test/conftest.py#L131
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.
an alternative (for CI only) would be to use the GH action from extremely-dangerous-public-oidc-beacon -- this is only feasible if the test suite is fast enough that it can be run with one token
| return Bundle._from_parts( # noqa: SLF001 | ||
| cert=certificate, | ||
| content=evp, | ||
| log_entry=log_entry, | ||
| ) |
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.
Bundle has to include a timestamp if the entry kindversion is 0.0.2 (IOW it comes from rekor2). I think including the TSA timestamp would be a good idea in general
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.
The more general way to say this is that the PyPI attestations should be modified so that they can include timestamps... but that should happen in another PR
|
I believe the TestAttestation::test_roundtrip test is now failing (after updating some minor things) since it runs using sigstore staging and sigstore staging already defaults to rekor v2. EDIT: confirmed: the test passes with |
|
There is two commits for this branch in https://github.com/jku/pypi-attestations/commits/ww/drop-protobufs/
|
|
@jku I'm probably not going to get back to this for the next week or so, so feel free to open your branch as a PR! |
|
Closing in favor of #144 |
I come bearing gifts 🙂
This is a work in progress; it shouldn't be merged until sigstore/sigstore-python#1470 lands in a release.
Key changes:
sigstore-protobuf-specsis entirely gone.Closes #131.