Skip to content

Commit f4c5429

Browse files
committed
certrotation: issue more specific update events
When secret/configmap is being update an event is being emitted. This event should include a more specific reason why this resource was updated: content change, new annotation etc. This ensures duplicate events are not being issued by the controller
1 parent e696d07 commit f4c5429

File tree

3 files changed

+28
-7
lines changed

3 files changed

+28
-7
lines changed

pkg/operator/certrotation/cabundle.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,12 @@ func (c CABundleConfigMap) EnsureConfigMapCABundle(ctx context.Context, signingC
6767
}
6868

6969
// run Update if metadata needs changing unless running in RefreshOnlyWhenExpired mode
70+
updateDetail := ""
7071
if !c.RefreshOnlyWhenExpired {
7172
updateRequired = ensureOwnerRefAndTLSAnnotationsForConfigMap(caBundleConfigMap, c.Owner, c.AdditionalAnnotations)
73+
if updateRequired {
74+
updateDetail = fmt.Sprintf("annotations set to %#v", c.AdditionalAnnotations)
75+
}
7276
}
7377

7478
updatedCerts, err := manageCABundleConfigMap(caBundleConfigMap, signingCertKeyPair.Config.Certs[0])
@@ -85,6 +89,7 @@ func (c CABundleConfigMap) EnsureConfigMapCABundle(ctx context.Context, signingC
8589
reason = fmt.Sprintf("signer update %s", signingCertKeyPairLocation)
8690
}
8791
c.EventRecorder.Eventf("CABundleUpdateRequired", "%q in %q requires a new cert: %s", c.Name, c.Namespace, reason)
92+
updateDetail = fmt.Sprintf("content change: %s", reason)
8893
LabelAsManagedConfigMap(caBundleConfigMap, CertificateTypeCABundle)
8994

9095
updateRequired = true
@@ -96,19 +101,19 @@ func (c CABundleConfigMap) EnsureConfigMapCABundle(ctx context.Context, signingC
96101
if err != nil {
97102
return nil, err
98103
}
99-
klog.V(2).Infof("Created ca-bundle.crt configmap %s/%s with:\n%s", certs.CertificateBundleToString(updatedCerts), caBundleConfigMap.Namespace, caBundleConfigMap.Name)
104+
klog.V(2).Infof("Created ca-bundle.crt configmap %s/%s with:\n%s", caBundleConfigMap.Namespace, caBundleConfigMap.Name, certs.CertificateBundleToString(updatedCerts))
100105
caBundleConfigMap = actualCABundleConfigMap
101106
} else if updateRequired {
102107
actualCABundleConfigMap, err := c.Client.ConfigMaps(c.Namespace).Update(ctx, caBundleConfigMap, metav1.UpdateOptions{})
103108
if apierrors.IsConflict(err) {
104109
// ignore error if its attempting to update outdated version of the configmap
105110
return nil, nil
106111
}
107-
resourcehelper.ReportUpdateEvent(c.EventRecorder, actualCABundleConfigMap, err)
112+
resourcehelper.ReportUpdateEvent(c.EventRecorder, actualCABundleConfigMap, err, updateDetail)
108113
if err != nil {
109114
return nil, err
110115
}
111-
klog.V(2).Infof("Updated ca-bundle.crt configmap %s/%s with:\n%s", certs.CertificateBundleToString(updatedCerts), caBundleConfigMap.Namespace, caBundleConfigMap.Name)
116+
klog.V(2).Infof("Updated ca-bundle.crt configmap %s/%s due to %s with:\n%s", caBundleConfigMap.Namespace, caBundleConfigMap.Name, updateDetail, certs.CertificateBundleToString(updatedCerts))
112117
caBundleConfigMap = actualCABundleConfigMap
113118
}
114119

pkg/operator/certrotation/signer.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,16 @@ func (c RotatedSigningCASecret) EnsureSigningCertKeyPair(ctx context.Context) (*
7979
}
8080

8181
// run Update if metadata needs changing unless we're in RefreshOnlyWhenExpired mode
82+
updateDetail := ""
8283
if !c.RefreshOnlyWhenExpired {
8384
needsMetadataUpdate := ensureOwnerRefAndTLSAnnotationsForSecret(signingCertKeyPairSecret, c.Owner, c.AdditionalAnnotations)
85+
if needsMetadataUpdate {
86+
updateDetail = fmt.Sprintf("annotations set to %#v", c.AdditionalAnnotations)
87+
}
8488
needsTypeChange := ensureSecretTLSTypeSet(signingCertKeyPairSecret)
89+
if needsTypeChange {
90+
updateDetail = "updated secret type"
91+
}
8592
updateRequired = needsMetadataUpdate || needsTypeChange
8693
}
8794

@@ -92,6 +99,7 @@ func (c RotatedSigningCASecret) EnsureSigningCertKeyPair(ctx context.Context) (*
9299
reason = "secret doesn't exist"
93100
}
94101
c.EventRecorder.Eventf("SignerUpdateRequired", "%q in %q requires a new signing cert/key pair: %v", c.Name, c.Namespace, reason)
102+
updateDetail = fmt.Sprintf("signer updated: %s", reason)
95103
if err = setSigningCertKeyPairSecretAndTLSAnnotations(signingCertKeyPairSecret, c.Validity, c.Refresh, c.AdditionalAnnotations); err != nil {
96104
return nil, false, err
97105
}
@@ -116,11 +124,11 @@ func (c RotatedSigningCASecret) EnsureSigningCertKeyPair(ctx context.Context) (*
116124
// ignore error if its attempting to update outdated version of the secret
117125
return nil, false, nil
118126
}
119-
resourcehelper.ReportUpdateEvent(c.EventRecorder, actualSigningCertKeyPairSecret, err)
127+
resourcehelper.ReportUpdateEvent(c.EventRecorder, actualSigningCertKeyPairSecret, err, updateDetail)
120128
if err != nil {
121129
return nil, false, err
122130
}
123-
klog.V(2).Infof("Updated secret %s/%s", actualSigningCertKeyPairSecret.Namespace, actualSigningCertKeyPairSecret.Name)
131+
klog.V(2).Infof("Updated secret %s/%s, reason: %s", actualSigningCertKeyPairSecret.Namespace, actualSigningCertKeyPairSecret.Name, updateDetail)
124132
signingCertKeyPairSecret = actualSigningCertKeyPairSecret
125133
}
126134

pkg/operator/certrotation/target.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,14 +113,22 @@ func (c RotatedSelfSignedCertKeySecret) EnsureTargetCertKeyPair(ctx context.Cont
113113
}
114114

115115
// run Update if metadata needs changing unless we're in RefreshOnlyWhenExpired mode
116+
updateDetail := ""
116117
if !c.RefreshOnlyWhenExpired {
117118
needsMetadataUpdate := ensureOwnerRefAndTLSAnnotationsForSecret(targetCertKeyPairSecret, c.Owner, c.AdditionalAnnotations)
119+
if needsMetadataUpdate {
120+
updateDetail = fmt.Sprintf("annotations set to %#v", c.AdditionalAnnotations)
121+
}
118122
needsTypeChange := ensureSecretTLSTypeSet(targetCertKeyPairSecret)
123+
if needsTypeChange {
124+
updateDetail = "updated secret type"
125+
}
119126
updateRequired = needsMetadataUpdate || needsTypeChange
120127
}
121128

122129
if reason := c.CertCreator.NeedNewTargetCertKeyPair(targetCertKeyPairSecret, signingCertKeyPair, caBundleCerts, c.Refresh, c.RefreshOnlyWhenExpired, creationRequired); len(reason) > 0 {
123130
c.EventRecorder.Eventf("TargetUpdateRequired", "%q in %q requires a new target cert/key pair: %v", c.Name, c.Namespace, reason)
131+
updateDetail = fmt.Sprintf("content change: %s", reason)
124132
if err = setTargetCertKeyPairSecretAndTLSAnnotations(targetCertKeyPairSecret, c.Validity, c.Refresh, signingCertKeyPair, c.CertCreator, c.AdditionalAnnotations); err != nil {
125133
return nil, err
126134
}
@@ -143,11 +151,11 @@ func (c RotatedSelfSignedCertKeySecret) EnsureTargetCertKeyPair(ctx context.Cont
143151
// ignore error if its attempting to update outdated version of the secret
144152
return nil, nil
145153
}
146-
resourcehelper.ReportUpdateEvent(c.EventRecorder, actualTargetCertKeyPairSecret, err)
154+
resourcehelper.ReportUpdateEvent(c.EventRecorder, actualTargetCertKeyPairSecret, err, updateDetail)
147155
if err != nil {
148156
return nil, err
149157
}
150-
klog.V(2).Infof("Updated secret %s/%s", actualTargetCertKeyPairSecret.Namespace, actualTargetCertKeyPairSecret.Name)
158+
klog.V(2).Infof("Updated secret %s/%s, reason: %s", actualTargetCertKeyPairSecret.Namespace, actualTargetCertKeyPairSecret.Name, updateDetail)
151159
targetCertKeyPairSecret = actualTargetCertKeyPairSecret
152160
}
153161

0 commit comments

Comments
 (0)