Add serial number and revision number to svid minting log entries #3699
Conversation
alexviktorov
requested review from
evan2645,
amartinezfayo,
azdagron,
MarcosDY and
rturner3
as code owners
There was a problem hiding this comment.
Thanks so much for this contribution @alexviktorov! Couple minor comments, should be good to go after that.
| @@ -115,11 +115,12 @@ func (s *Service) MintX509SVID(ctx context.Context, req *svidv1.MintX509SVIDRequ | |||
| } | |||
|
|
|||
| rpccontext.AddRPCAuditFields(ctx, logrus.Fields{ | |||
| telemetry.ExpiresAt: x509SVID[0].NotAfter.Unix(), | |||
| telemetry.ExpiresAt: x509SVID[0].NotAfter.Format(time.RFC3339), | |||
There was a problem hiding this comment.
Was this change intentional? I don't have a strong opinion on the formatting here, the concern from my end is more around backward compatibility for folks using this field today
There was a problem hiding this comment.
Yes, just a couple of lines below on line 122 the Expiration is logged in time.RFC3339 format so changed it for consistency's sake after discussing with Ryan. I don't have a strong opinion though, I can easily revert it.
pkg/server/api/svid/v1/service.go
Outdated
| }) | ||
|
|
||
| rpccontext.AuditRPCWithFields(ctx, commonX509SVIDLogFields) | ||
| log.WithField(telemetry.Expiration, x509SVID[0].NotAfter.Format(time.RFC3339)). | ||
| WithField(telemetry.SerialNumber, x509SVID[0].SerialNumber). |
There was a problem hiding this comment.
I think this is an x509.Certificate type, and the SerialNumber field there is actually *big.Int which is a struct. I think we need to call the String() method to get something printable
| WithField(telemetry.SerialNumber, x509SVID[0].SerialNumber). | |
| WithField(telemetry.SerialNumber, x509SVID[0].SerialNumber.String()). |
Signed-off-by: Alexander Viktorov <[email protected]>
…ries Signed-off-by: Alexander Viktorov <[email protected]>
Signed-off-by: Alexander Viktorov <[email protected]>
alexviktorov
force-pushed
the
aviktorov/3688
branch
from
62c87bb to
25c14fe
Compare
| @@ -115,11 +115,12 @@ func (s *Service) MintX509SVID(ctx context.Context, req *svidv1.MintX509SVIDRequ | |||
| } | |||
|
|
|||
| rpccontext.AddRPCAuditFields(ctx, logrus.Fields{ | |||
| telemetry.ExpiresAt: x509SVID[0].NotAfter.Unix(), | |||
| telemetry.ExpiresAt: x509SVID[0].NotAfter.Format(time.RFC3339), | |||
…iffe#3699) * Added svid serial number and entry revision number where applicable Signed-off-by: Alexander Viktorov <[email protected]> Signed-off-by: divaspathak <[email protected]>
…iffe#3699) * Added svid serial number and entry revision number where applicable Signed-off-by: Alexander Viktorov <[email protected]>
Pull Request check list
Affected functionality
Log entries will include extra fields; datetime of Svid minting audit logs changed format from Unix to RFC3339
Description of change
Added serial number and revision number to svid minting log entries: both for Batch/NewX509Svid and only serial number for MintX509Svid
Which issue this PR fixes
#3688