Skip to content

Commit

Permalink
certrotation: improve error messages
Browse files Browse the repository at this point in the history 
* Explicitly log "secret/configmap doesn't exist" events
* when secret is refreshed differenciate between refresh on 80% of validity
  and custom refresh time
* when CA bundle is updated log which signer update caused it
  • Loading branch information
@vrutkovs
vrutkovs committed Apr 17, 2024
1 parent eb2f24c commit f2f23fb
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 12 deletions.
12 changes: 10 additions & 2 deletions pkg/operator/certrotation/cabundle.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ type CABundleConfigMap struct {
EventRecorder events.Recorder
}

func (c CABundleConfigMap) EnsureConfigMapCABundle(ctx context.Context, signingCertKeyPair *crypto.CA) ([]*x509.Certificate, error) {
func (c CABundleConfigMap) EnsureConfigMapCABundle(ctx context.Context, signingCertKeyPair *crypto.CA, signingCertKeyPairLocation string) ([]*x509.Certificate, error) {
// by this point we have current signing cert/key pair. We now need to make sure that the ca-bundle configmap has this cert and
// doesn't have any expired certs
originalCABundleConfigMap, err := c.Lister.ConfigMaps(c.Namespace).Get(c.Name)
Expand Down Expand Up @@ -75,7 +75,15 @@ func (c CABundleConfigMap) EnsureConfigMapCABundle(ctx context.Context, signingC
return nil, err
}
if originalCABundleConfigMap == nil || originalCABundleConfigMap.Data == nil || !equality.Semantic.DeepEqual(originalCABundleConfigMap.Data, caBundleConfigMap.Data) {
c.EventRecorder.Eventf("CABundleUpdateRequired", "%q in %q requires a new cert", c.Name, c.Namespace)
reason := ""
if originalCABundleConfigMap == nil {
reason = "configmap doesn't exist"
} else if originalCABundleConfigMap.Data == nil {
reason = "configmap is empty"
} else if !equality.Semantic.DeepEqual(originalCABundleConfigMap.Data, caBundleConfigMap.Data) {
reason = fmt.Sprintf("signer update %s", signingCertKeyPairLocation)
}
c.EventRecorder.Eventf("CABundleUpdateRequired", "%q in %q requires a new cert: %s", c.Name, c.Namespace, reason)
LabelAsManagedConfigMap(caBundleConfigMap, CertificateTypeCABundle)

actualCABundleConfigMap, modified, err := resourceapply.ApplyConfigMap(ctx, c.Client, c.EventRecorder, caBundleConfigMap)
Expand Down
2 changes: 1 addition & 1 deletion pkg/operator/certrotation/cabundle_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ func TestEnsureConfigMapCABundle(t *testing.T) {
if err != nil {
t.Fatal(err)
}
_, err = c.EnsureConfigMapCABundle(context.TODO(), newCA)
_, err = c.EnsureConfigMapCABundle(context.TODO(), newCA, "signer-secret")
switch {
case err != nil && len(test.expectedError) == 0:
t.Error(err)
Expand Down
6 changes: 5 additions & 1 deletion pkg/operator/certrotation/client_cert_rotation_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,17 @@ func (c CertRotationController) Sync(ctx context.Context, syncCtx factory.SyncCo
return syncErr
}

func (c CertRotationController) getSigningCertKeyPairLocation() string {
return fmt.Sprintf("%s/%s", c.RotatedSelfSignedCertKeySecret.Namespace, c.RotatedSelfSignedCertKeySecret.Name)
}

func (c CertRotationController) SyncWorker(ctx context.Context) error {
signingCertKeyPair, _, err := c.RotatedSigningCASecret.EnsureSigningCertKeyPair(ctx)
if err != nil {
return err
}

cabundleCerts, err := c.CABundleConfigMap.EnsureConfigMapCABundle(ctx, signingCertKeyPair)
cabundleCerts, err := c.CABundleConfigMap.EnsureConfigMapCABundle(ctx, signingCertKeyPair, c.getSigningCertKeyPairLocation())
if err != nil {
return err
}
Expand Down
12 changes: 9 additions & 3 deletions pkg/operator/certrotation/signer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ func (c RotatedSigningCASecret) EnsureSigningCertKeyPair(ctx context.Context) (*
if err != nil && !apierrors.IsNotFound(err) {
return nil, false, err
}
var signerExists = true
signingCertKeyPairSecret := originalSigningCertKeyPairSecret.DeepCopy()
if apierrors.IsNotFound(err) {
// create an empty one
Expand All @@ -78,6 +79,7 @@ func (c RotatedSigningCASecret) EnsureSigningCertKeyPair(ctx context.Context) (*
),
Type: corev1.SecretTypeTLS,
}
signerExists = false
}

applyFn := resourceapply.ApplySecret
Expand All @@ -96,7 +98,10 @@ func (c RotatedSigningCASecret) EnsureSigningCertKeyPair(ctx context.Context) (*
}

signerUpdated := false
if needed, reason := needNewSigningCertKeyPair(signingCertKeyPairSecret.Annotations, c.Refresh, c.RefreshOnlyWhenExpired); needed {
if needed, reason := needNewSigningCertKeyPair(signingCertKeyPairSecret, c.Refresh, c.RefreshOnlyWhenExpired, signerExists); needed || !signerExists {
if !signerExists {
reason = "secret doesn't exist"
}
c.EventRecorder.Eventf("SignerUpdateRequired", "%q in %q requires a new signing cert/key pair: %v", c.Name, c.Namespace, reason)
if err := setSigningCertKeyPairSecret(signingCertKeyPairSecret, c.Validity); err != nil {
return nil, false, err
Expand Down Expand Up @@ -136,7 +141,8 @@ func ensureOwnerReference(meta *metav1.ObjectMeta, owner *metav1.OwnerReference)
return false
}

func needNewSigningCertKeyPair(annotations map[string]string, refresh time.Duration, refreshOnlyWhenExpired bool) (bool, string) {
func needNewSigningCertKeyPair(secret *corev1.Secret, refresh time.Duration, refreshOnlyWhenExpired, signerExists bool) (bool, string) {
annotations := secret.Annotations
notBefore, notAfter, reason := getValidityFromAnnotations(annotations)
if len(reason) > 0 {
return true, reason
Expand All @@ -153,7 +159,7 @@ func needNewSigningCertKeyPair(annotations map[string]string, refresh time.Durat
validity := notAfter.Sub(notBefore)
at80Percent := notAfter.Add(-validity / 5)
if time.Now().After(at80Percent) {
return true, fmt.Sprintf("past its latest possible time %v", at80Percent)
return true, fmt.Sprintf("past refresh time (80%% of validity): %v", at80Percent)
}

developerSpecifiedRefresh := notBefore.Add(refresh)
Expand Down
15 changes: 10 additions & 5 deletions pkg/operator/certrotation/target.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,12 @@ func (c RotatedSelfSignedCertKeySecret) EnsureTargetCertKeyPair(ctx context.Cont
return targetCertKeyPairSecret, nil
}

func needNewTargetCertKeyPair(annotations map[string]string, signer *crypto.CA, caBundleCerts []*x509.Certificate, refresh time.Duration, refreshOnlyWhenExpired bool) string {
func needNewTargetCertKeyPair(secret *corev1.Secret, signer *crypto.CA, caBundleCerts []*x509.Certificate, refresh time.Duration, refreshOnlyWhenExpired bool) string {
if secret.ResourceVersion == "" {
return "secret doesn't exist"
}

annotations := secret.Annotations
if reason := needNewTargetCertKeyPairForTime(annotations, signer, refresh, refreshOnlyWhenExpired); len(reason) > 0 {
return reason
}
Expand Down Expand Up @@ -203,7 +208,7 @@ func needNewTargetCertKeyPairForTime(annotations map[string]string, signer *cryp
validity := notAfter.Sub(notBefore)
at80Percent := notAfter.Add(-validity / 5)
if time.Now().After(at80Percent) {
return fmt.Sprintf("past its latest possible time %v", at80Percent)
return fmt.Sprintf("past refresh time (80%% of validity): %v", at80Percent)
}

// If Certificate is past its refresh time, we may have action to take. We only do this if the signer is old enough.
Expand Down Expand Up @@ -264,7 +269,7 @@ func (r *ClientRotation) NewCertificate(signer *crypto.CA, validity time.Duratio
}

func (r *ClientRotation) NeedNewTargetCertKeyPair(currentCertSecret *corev1.Secret, signer *crypto.CA, caBundleCerts []*x509.Certificate, refresh time.Duration, refreshOnlyWhenExpired bool) string {
return needNewTargetCertKeyPair(currentCertSecret.Annotations, signer, caBundleCerts, refresh, refreshOnlyWhenExpired)
return needNewTargetCertKeyPair(currentCertSecret, signer, caBundleCerts, refresh, refreshOnlyWhenExpired)
}

func (r *ClientRotation) SetAnnotations(cert *crypto.TLSCertificateConfig, annotations map[string]string) map[string]string {
Expand All @@ -289,7 +294,7 @@ func (r *ServingRotation) RecheckChannel() <-chan struct{} {
}

func (r *ServingRotation) NeedNewTargetCertKeyPair(currentCertSecret *corev1.Secret, signer *crypto.CA, caBundleCerts []*x509.Certificate, refresh time.Duration, refreshOnlyWhenExpired bool) string {
reason := needNewTargetCertKeyPair(currentCertSecret.Annotations, signer, caBundleCerts, refresh, refreshOnlyWhenExpired)
reason := needNewTargetCertKeyPair(currentCertSecret, signer, caBundleCerts, refresh, refreshOnlyWhenExpired)
if len(reason) > 0 {
return reason
}
Expand Down Expand Up @@ -335,7 +340,7 @@ func (r *SignerRotation) NewCertificate(signer *crypto.CA, validity time.Duratio
}

func (r *SignerRotation) NeedNewTargetCertKeyPair(currentCertSecret *corev1.Secret, signer *crypto.CA, caBundleCerts []*x509.Certificate, refresh time.Duration, refreshOnlyWhenExpired bool) string {
return needNewTargetCertKeyPair(currentCertSecret.Annotations, signer, caBundleCerts, refresh, refreshOnlyWhenExpired)
return needNewTargetCertKeyPair(currentCertSecret, signer, caBundleCerts, refresh, refreshOnlyWhenExpired)
}

func (r *SignerRotation) SetAnnotations(cert *crypto.TLSCertificateConfig, annotations map[string]string) map[string]string {
Expand Down

0 comments on commit f2f23fb

Please sign in to comment.