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

Use Issuer interface to allow for custom issuers #1008

Merged
merged 5 commits into from
Mar 7, 2023

Conversation

priyawadhwa
Copy link
Contributor

@priyawadhwa priyawadhwa commented Feb 16, 2023

This plumbs through IssuerPool and starts using it to set up Fulcio.

@priyawadhwa priyawadhwa changed the title WIP: User Issuer interface to allow for custom issuers WIP: Use Issuer interface to allow for custom issuers Feb 16, 2023
@haydentherapper
Copy link
Contributor

LGTM with the concept, as we chatted about, server implementers who want to use private implementations of an issuer will instantiate the gRPC server with their own issuer pool.

@haydentherapper
Copy link
Contributor

Will take a look Monday!

@priyawadhwa
Copy link
Contributor Author

@haydentherapper just so you know it's still a WIP, have a couple more things to add!

@priyawadhwa priyawadhwa changed the title WIP: Use Issuer interface to allow for custom issuers Use Issuer interface to allow for custom issuers Mar 6, 2023
This plumbs through IssuerPool and starts using it to set up Fulcio.

Signed-off-by: Priya Wadhwa <[email protected]>
Signed-off-by: Priya Wadhwa <[email protected]>
@codecov
Copy link

codecov bot commented Mar 6, 2023

Codecov Report

Merging #1008 (1bc60cd) into main (d15bdf8) will increase coverage by 0.30%.
The diff coverage is 90.66%.

@@            Coverage Diff             @@
##             main    #1008      +/-   ##
==========================================
+ Coverage   54.47%   54.77%   +0.30%     
==========================================
  Files          46       48       +2     
  Lines        2480     2521      +41     
==========================================
+ Hits         1351     1381      +30     
- Misses       1030     1039       +9     
- Partials       99      101       +2     
Impacted Files Coverage Δ
cmd/app/serve.go 26.73% <40.00%> (-0.19%) ⬇️
pkg/identity/base/issuer.go 88.46% <88.46%> (ø)
pkg/server/issuer_pool.go 96.66% <96.66%> (ø)
cmd/app/grpc.go 41.30% <100.00%> (ø)
pkg/identity/buildkite/issuer.go 62.50% <100.00%> (-7.50%) ⬇️
pkg/identity/email/issuer.go 62.50% <100.00%> (-7.50%) ⬇️
pkg/identity/github/issuer.go 62.50% <100.00%> (-7.50%) ⬇️
pkg/identity/kubernetes/issuer.go 62.50% <100.00%> (-7.50%) ⬇️
pkg/identity/spiffe/issuer.go 62.50% <100.00%> (-7.50%) ⬇️
pkg/identity/uri/issuer.go 62.50% <100.00%> (-7.50%) ⬇️
... and 5 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

pkg/server/issuer_pool.go Outdated Show resolved Hide resolved
pkg/server/grpc_server_test.go Outdated Show resolved Hide resolved
pkg/identity/base/issuer.go Show resolved Hide resolved
Signed-off-by: Priya Wadhwa <[email protected]>
@priyawadhwa
Copy link
Contributor Author

@haydentherapper this should be RFAL! Thanks :)

Copy link
Contributor

@haydentherapper haydentherapper left a comment

Choose a reason for hiding this comment

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

pkg/server/grpc_server_test.go Outdated Show resolved Hide resolved
Signed-off-by: Priya Wadhwa <[email protected]>
@priyawadhwa
Copy link
Contributor Author

priyawadhwa commented Mar 7, 2023

@haydentherapper i was planning on deleting everything in a follow-up! wanted to try pulling in this commit and make sure everything worked before deleting anything.

Copy link
Contributor

@haydentherapper haydentherapper left a comment

Choose a reason for hiding this comment

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

Sounds good! Thanks!

@priyawadhwa priyawadhwa merged commit 4d9d96a into sigstore:main Mar 7, 2023
@priyawadhwa priyawadhwa deleted the custom-issuer branch March 7, 2023 01:54
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.

2 participants