-
Notifications
You must be signed in to change notification settings - Fork 62
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
Add Rekor v2 client #1422
Conversation
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]>
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]>
* 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]>
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]>
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) |
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.
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.
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.
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]>
|
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.
LGTM, thanks @ramonpetgrave64 and @jku!
Looks like some tests failed after merging. Can you run them again to make sure? |
Yeah, all tests are ok now -- I think that was just part of the Great Internet Meltdown of June 12 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.
create_entry()
and methods for constructing the requests for dsse/hashedrekordRekorLogSubmitter
-- we could name it RekorClient but obviously would then need to rename the original V1 client...