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

Specify public key for inactive shards in shard config #746

Merged
merged 5 commits into from
Mar 31, 2022

Conversation

priyawadhwa
Copy link
Contributor

This is pretty much the last item we need for sharding! Specify a public key for inactive shards in the sharding config in case we are rotating signers; this way users can request the public key for a specific shard from the API

cc @lkatalin and @asraa

Summary

Ticket Link

Fixes

Release Note


Signed-off-by: Priya Wadhwa <priya@chainguard.dev>
Signed-off-by: Priya Wadhwa <priya@chainguard.dev>
@priyawadhwa priyawadhwa requested a review from a team as a code owner March 29, 2022 05:40
@codecov-commenter
Copy link

Codecov Report

Merging #746 (116282a) into main (befbcc0) will increase coverage by 0.17%.
The diff coverage is 76.92%.

@@            Coverage Diff             @@
##             main     #746      +/-   ##
==========================================
+ Coverage   48.43%   48.60%   +0.17%     
==========================================
  Files          61       61              
  Lines        5492     5518      +26     
==========================================
+ Hits         2660     2682      +22     
- Misses       2544     2546       +2     
- Partials      288      290       +2     
Impacted Files Coverage Δ
pkg/sharding/ranges.go 53.75% <76.92%> (+11.15%) ⬆️
pkg/types/rekord/v0.0.1/entry.go 51.51% <0.00%> (+0.60%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update befbcc0...116282a. Read the comment docs.

Signed-off-by: Priya Wadhwa <priya@chainguard.dev>
@priyawadhwa priyawadhwa force-pushed the publickey branch 3 times, most recently from 686ff2c to b22ef56 Compare March 29, 2022 19:55
Copy link
Contributor

@lkatalin lkatalin left a comment

Choose a reason for hiding this comment

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

This looks good! I had a question around the pubkey API endpoint because I don't see any documentation but maybe it's only internal?

pkg/sharding/ranges_test.go Outdated Show resolved Hide resolved
pkg/sharding/ranges.go Show resolved Hide resolved
Comment on lines +96 to +101
parameters:
- in: query
name: treeID
type: string
pattern: '^[0-9]+$'
description: The tree ID of the tree you wish to get a public key for
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this endpoint external for the user like loginfo or just internal?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's external! you can query it like this:

curl "http://localhost:3000/api/v1/log/publicKey?treeID=383271703219633232"

Copy link
Contributor

Choose a reason for hiding this comment

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

Is that documented anywhere? Maybe I missed it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to look at the openAPI docs here to figure out the different types of params they accept. I went with query because that's what we do for a bunch of other api calls as well (/api/v1/log/retrieve, /api/v1/log/proof etc)

rekor/openapi.yaml

Lines 113 to 132 in 035b262

parameters:
- in: query
name: firstSize
type: integer
default: 1
minimum: 1
description: >
The size of the tree that you wish to prove consistency from (1 means the beginning of the log)
Defaults to 1 if not specified
- in: query
name: lastSize
type: integer
required: true
minimum: 1
description: The size of the tree that you wish to prove consistency to
- in: query
name: treeID
type: string
pattern: '^[0-9]+$'
description: The tree ID of the tree that you wish to prove consistency for

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh okay, thank you, I meant documented in Rekor for the user to be able to use some of these commands, but we can do that later if we want to. The link is helpful to me as I'm also trying to figure it out!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah gotcha, yah docs would be useful for sure!

@lkatalin
Copy link
Contributor

Also I love the e2e tests! 💯

Signed-off-by: Priya Wadhwa <priya@chainguard.dev>
@dlorenc dlorenc merged commit f525de8 into sigstore:main Mar 31, 2022
@github-actions github-actions bot added this to the v1.0.0 milestone Mar 31, 2022
@priyawadhwa priyawadhwa deleted the publickey branch March 31, 2022 23:39
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.

4 participants