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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 38 commits into
base: main
Choose a base branch
from
Open

Add Rekor v2 client #1422

wants to merge 38 commits into from

Conversation

jku
Copy link
Member

@jku jku commented Jun 4, 2025 •
edited
Loading

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
@ramonpetgrave64
add RekorV2Client and types
376361a 
Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
@ramonpetgrave64
add rekorv2 client + tests
2983b4a 
Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
@ramonpetgrave64
add lots of docstrings
3f94e7a 
Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
@ramonpetgrave64
xfail on local and staging
d043256 
Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
@ramonpetgrave64
add cahngelog
1a55117 
Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
@ramonpetgrave64
Merge branch 'main' into rekov2-client
65ecf4e
@ramonpetgrave64
remove staging and production methods
3730364 
Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
@ramonpetgrave64
add @pytest.mark.ambient_oidc
1e78607 
Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
@ramonpetgrave64
send the cert, not only the public key
53c9113 
Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
@ramonpetgrave64
abstract the signer fixture
79f967c 
Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
@ramonpetgrave64
reorganize fixtures
832f87d 
Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
@ramonpetgrave64
add tiemout
e6a6fe3 
Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
@ramonpetgrave64
Merge branch 'main' into rekov2-client
c546aea
@ramonpetgrave64
no V002 workaround
5baeb8f 
Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
@ramonpetgrave64
merge updates
4b2a03a 
Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
@ramonpetgrave64
no V002 workaround
73eaf0e 
Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
@ramonpetgrave64
regorganize tests
34043a2 
Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
@ramonpetgrave64
use new methods for building requests
a733d5a 
Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
@ramonpetgrave64
changelog
90d8244 
Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
@ramonpetgrave64
cleanup comment
b49d8a7 
Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
@ramonpetgrave64
future import
80422f2 
Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
@jku
Rekor: Tweak the log submitter abstraction
9b60fb6 
* 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 <jkukkonen@google.com>
@jku
tests: Simplify rekorv2 tests
d80c25c 
* 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 <jkukkonen@google.com>
@jku jku mentioned this pull request Jun 4, 2025
Closed
6 tasks
jku added 3 commits June 4, 2025 16:48
@jku
rekor: remove timeout for now
5746d0c 
* 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 <jkukkonen@google.com>
@jku
Use rekor types from (unreleased) protobuf-specs
cba9507 
As of right now this is not in a released protobuf-specs: just preparing
for when it is

Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
@jku
Merge remote-tracking branch 'origin/main' into rekov2-client
5c503a6
@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

jku
jku commented Jun 6, 2025
View reviewed changes
@jku
rekor v2: Cope without CreateEntryRequest
3d9a1b8 
This was dropped from generated protobuf code

Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
jku
jku commented Jun 6, 2025
View reviewed changes
@jku
Copy link
Member Author

jku commented Jun 6, 2025 •
edited
Loading

I'm marking this ready for review: it depends on a protobuf-specs PR so is not ready to merge but should be otherwise reasonable

@jku jku marked this pull request as ready for review June 6, 2025 12:05
jku
jku commented Jun 6, 2025
View reviewed changes
@jku jku force-pushed the rekov2-client branch from 8de5278 to 393275b Compare June 7, 2025 09:09
jku added 3 commits June 7, 2025 12:12
@jku
Revert "rekor v2: Cope without CreateEntryRequest"
b942f90 
This reverts commit 3d9a1b8.

protobuf-specs now contains CreateEntryRequest: no need to workaround

Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
@jku
test: remove unnecessary change
4e9c733 
Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
@jku
rekor: Re-use the error class for both clients
afdebab 
Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
@jku jku force-pushed the rekov2-client branch from 393275b to b6b2ad5 Compare June 7, 2025 09:12
@jku
sign: Tweak rekor annotation
f0b79f9 
This is a backwards compatible change (RekorClient is a
RekorLogSumbitter), but also allows using a RekorV2Client.

Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
@jku jku force-pushed the rekov2-client branch from b6b2ad5 to 90d1ff7 Compare June 7, 2025 09:21
@jku jku requested review from ramonpetgrave64 and woodruffw June 9, 2025 07:15
This was referenced Jun 9, 2025
Closed
Closed
Draft
woodruffw
woodruffw reviewed Jun 9, 2025
View reviewed changes
woodruffw
woodruffw reviewed Jun 9, 2025
View reviewed changes
woodruffw
woodruffw reviewed Jun 9, 2025
View reviewed changes
woodruffw
woodruffw reviewed Jun 9, 2025
View reviewed changes
ramonpetgrave64
ramonpetgrave64 suggested changes Jun 9, 2025
View reviewed changes
test/unit/internal/rekor/test_client_v2.py
).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.

@ramonpetgrave64 ramonpetgrave64 mentioned this pull request Jun 9, 2025
Closed
@ramonpetgrave64 @jku
change PR number
1f71c8b 
Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
@ramonpetgrave64 @jku
ranem to EntryRequestBody
29ba622 
Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
@ramonpetgrave64 @jku
docstring formatting
d5b407b 
Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
@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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@woodruffw woodruffw woodruffw left review comments

@ramonpetgrave64 ramonpetgrave64 ramonpetgrave64 approved these changes

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jku @woodruffw @ramonpetgrave64