Skip to content

Add Rekor v2 client #1422

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

Merged
merged 39 commits into from
Jun 12, 2025
Merged

Add Rekor v2 client #1422

merged 39 commits into from
Jun 12, 2025

Conversation

jku
Copy link
Member

@jku jku commented Jun 4, 2025

This is a continuation of Ramons #1400 , I'm moving it here in hopes of getting staging tests running.

The PR adds a new RekorV2Client for adding entries to a rekor-tiles log.

  • The plan is that a followup PR starts using this client in the CLI when signingconfig contains a rekor v2 instance
  • The existing RekorClient is slightly modified so that there is a common abstraction that the signing code can use
    • the abstraction contains create_entry() and methods for constructing the requests for dsse/hashedrekord
    • name of the ABC is currently RekorLogSubmitter -- we could name it RekorClient but obviously would then need to rename the original V1 client...
  • Note that the client does not include functionality to lookup entries from the log (since a sigstore client does not strictly speaking need to do that). The feature could still be added
  • testing is very light but it does show that the client can add entries to a rekor-tiles log (the staging instance)

ramonpetgrave64 and others added 23 commits May 20, 2025 16:18
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
* Make sure the exposed signatures actually are abstract:
  the request payload can be just a dict so both clients actually
  implement the same API
* There is still a "EntryRequest" NewType being used instead of dict
  just to make the intent clear

Signed-off-by: Jussi Kukkonen <[email protected]>
* Don't pretend these are unit tests: just have something simple
  that proves the client roughly does what it should
* We should have real unit tests (that don't require the whole
  staging infra as current tests do) and integration tests but this
  is what I have now

Signed-off-by: Jussi Kukkonen <[email protected]>
@jku jku mentioned this pull request Jun 4, 2025
6 tasks
jku added 3 commits June 4, 2025 16:48
* The timeout was a little misleading (since requests timeout is not the
  total maximum time of the request) and we don't have specific timeouts
  elsewhere either.
* Also remove some unused variables

Signed-off-by: Jussi Kukkonen <[email protected]>
As of right now this is not in a released protobuf-specs: just preparing
for when it is

Signed-off-by: Jussi Kukkonen <[email protected]>
@jku
Copy link
Member Author

jku commented Jun 6, 2025

Sort out the protobuf situation

I've been testing with sigstore/protobuf-specs#661 -- looks fine so far

This was dropped from generated protobuf code

Signed-off-by: Jussi Kukkonen <[email protected]>
@jku jku force-pushed the rekov2-client branch from 393275b to b6b2ad5 Compare June 7, 2025 09:12
This is a backwards compatible change (RekorClient is a
RekorLogSumbitter), but also allows using a RekorV2Client.

Signed-off-by: Jussi Kukkonen <[email protected]>
).build()

with sign_ctx.signer(identity) as signer:
bundle = signer.sign_dsse(stmt)
Copy link
Contributor

Choose a reason for hiding this comment

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

One thing I wanted to ensure that the certificate, not only the public key, was used in the request. It's no problem to not include that here, though.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah tests that really dissect both the request and response would be cool... For the request we should be able to do that without ever contacting rekor or any other service: a test that provides existing assets to the _build_*() methods and ensures the request body looks like we expect should do it -- but I'm ok leaving that for followup.

Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
@jku jku force-pushed the rekov2-client branch from 90d1ff7 to 334e1b2 Compare June 10, 2025 08:09
@jku jku force-pushed the rekov2-client branch from 334e1b2 to 983bfa6 Compare June 10, 2025 08:16
@jku
Copy link
Member Author

jku commented Jun 10, 2025

  • review fixes (thanks ramon)
  • protobuf-specs was released: removed the WIP commit and merged main (to get the new protobuf-specs version in use)

@jku jku requested a review from woodruffw June 12, 2025 16:32
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, thanks @ramonpetgrave64 and @jku!

@woodruffw woodruffw added the enhancement New feature or request label Jun 12, 2025
@woodruffw woodruffw merged commit 8faeace into main Jun 12, 2025
26 checks passed
@woodruffw woodruffw deleted the rekov2-client branch June 12, 2025 18:27
@ramonpetgrave64
Copy link
Contributor

ramonpetgrave64 commented Jun 12, 2025

Looks like some tests failed after merging. Can you run them again to make sure?
https://github.com/sigstore/sigstore-python/commits/main/

@jku
Copy link
Member Author

jku commented Jun 13, 2025

Yeah, all tests are ok now -- I think that was just part of the Great Internet Meltdown of June 12 2025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants