[WIP] Move config and challenge result to legacy issuer package #596
Conversation
Signed-off-by: Nathan Smith <nathan@chainguard.dev>
Moves config and challenge result into legacy issuer package. API servers now use issuer pool containing the legacy issuer. Signed-off-by: Nathan Smith <nathan@chainguard.dev>
nsmith5
force-pushed
the
fulcio-config-as-issuer
branch
from
ad7d4a2
to
3f9af12
Compare
Signed-off-by: Nathan Smith <nathan@chainguard.dev>
|
Hmm I see I've broken the stuff under |
| TypeVal: challenges.EmailValue, | ||
| Value: "foo@example.com", | ||
| }, priv.Public()) | ||
| precsc, err := ica.CreatePrecertificate(context.TODO(), testPrincipal{}, priv.Public()) |
There was a problem hiding this comment.
Can we move away from fakes to use real instances?
Can we instead use legacyPrincipal{} on line 177 instead of testPrincipal{} and populate it with test values, so that we can use the real implementation?
There was a problem hiding this comment.
We could, but this test doesn't check anything that actually changes if you change the implementation of identity.Principal used from what I can see.
| } | ||
|
|
||
| func NewGRPCCAServer(ct *ctclient.LogClient, ca certauth.CertificateAuthority) fulciogrpc.CAServer { | ||
| func NewGRPCCAServer(ct *ctclient.LogClient, ca certauth.CertificateAuthority, ip identity.IssuerPool) fulciogrpc.CAServer { |
There was a problem hiding this comment.
nit: I'd pick a different var name since ip is typically used for networking
| // top-level and second-level domain | ||
| const minimumHostnameLength = 2 | ||
|
|
||
| type legacyIssuer struct { |
There was a problem hiding this comment.
So this configuration goes away after the refactor?
It's a bit hard to tell why we're moving this, especially given this file has gotten huge. Should this be split up?
| @@ -47,35 +47,19 @@ type grpcServer struct { | |||
| caService gw.CAServer | |||
| } | |||
|
|
|||
| func passFulcioConfigThruContext(cfg *config.FulcioConfig) grpc.UnaryServerInterceptor { | |||
There was a problem hiding this comment.
To confirm, this removal is because we're now adding the config to the context in Authenticate?
|
I think we should explore that more - I think the FulcioConfig is a good top-level abstraction over all supported configs. On the note of LRU, we should still maintain the cache and make sure we're preloading the keys, etc. |
Summary
This work finally pushes the existing identity issuer logic behind the
identity.IssuerPoolabstraction. This is done bychallenges.ChallengeResulttype to the legacy issuer package. This is renamedlegacyPrincipaland all implementation details are unexported. ChallengeResult implementes the identity.Principal interface.config.FulcioConfiginto the legacy issuer package. This is renamedlegacyIssuerand all implementation details are unexported. It now implements the identity.Issuer interface.Ticket Link
Related to #275