Integrate builder and validator #3869
Conversation
azdagron
requested review from
evan2645,
amartinezfayo,
MarcosDY and
rturner3
as code owners
Also: - moves validator into own package - deletes unused signing code from jwtsvid package Signed-off-by: Andrew Harding <[email protected]>
azdagron
force-pushed
the
credentialcomposer-next
branch
from
1f47635 to
41ec0d4
Compare
| @@ -0,0 +1,227 @@ | |||
| package jwtsvid | |||
There was a problem hiding this comment.
This is just a renamed token_test.go with just the validation test portion remaining.
| @@ -40,31 +40,28 @@ type Config struct { | |||
| Clock clock.Clock | |||
| DataStore datastore.DataStore | |||
| ServerCA ca.ServerCA | |||
| AgentTTL time.Duration | |||
There was a problem hiding this comment.
AgentTTL is now handled in the builder.
| @@ -398,93 +398,6 @@ func (b *Builder) buildX509SVIDTemplate(spiffeID spiffeid.ID, publicKey crypto.P | |||
| return tmpl, nil | |||
| } | |||
|
|
|||
| func (b *Builder) ValidateX509CA(ca *x509.Certificate) error { | |||
There was a problem hiding this comment.
These moved into the credvalidator package.
| @@ -0,0 +1,176 @@ | |||
| package credvalidator | |||
There was a problem hiding this comment.
This is just existing code split out from the credtemplate.Builder.
| @@ -0,0 +1,395 @@ | |||
| package credvalidator_test | |||
There was a problem hiding this comment.
These tests are just existing code moved out from the credtemplate.Builder
| @@ -86,6 +86,7 @@ func (ca *UpstreamCA) SignCSR(ctx context.Context, csrDER []byte, preferredTTL t | |||
| x509.KeyUsageCRLSign, | |||
| BasicConstraintsValid: true, | |||
| IsCA: true, | |||
| ExtraExtensions: csr.ExtraExtensions, | |||
There was a problem hiding this comment.
is it a change to what we have on previous versions? why is it required?
There was a problem hiding this comment.
The CredentialComposers can add extra extensions to the CSR that is signed upstream.
pkg/server/ca/ca.go
Outdated
| } | ||
| telemetry_server.IncrServerCASignX509CACounter(ca.c.Metrics) |
There was a problem hiding this comment.
signing X509 SVIDS will increment ServerCASignX509CA is that ok?
| @@ -250,3 +253,27 @@ func makePublicKey(t *testing.T, kid string) *common.PublicKey { | |||
| PkixBytes: pkixBytes, | |||
| } | |||
| } | |||
|
|
|||
| func generateServerCACSR() ([]byte, error) { | |||
There was a problem hiding this comment.
since error is not used, may we fatal here? or just ignore and depends on getting an empty csr is enough?
There was a problem hiding this comment.
Sure. I changed it to panic since it is only used in tests and run at init time.
Signed-off-by: Andrew Harding <[email protected]>
* Integrate builder and validator Also: - moves validator into own package - deletes unused signing code from jwtsvid package Signed-off-by: Andrew Harding <[email protected]>
Also: