diff --git a/pkg/operator/certrotation/annotations.go b/pkg/operator/certrotation/annotations.go index 3c051c88e6..2f1667669d 100644 --- a/pkg/operator/certrotation/annotations.go +++ b/pkg/operator/certrotation/annotations.go @@ -1,10 +1,8 @@ package certrotation import ( - "github.com/google/go-cmp/cmp" "github.com/openshift/api/annotations" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/klog/v2" ) const ( @@ -48,44 +46,30 @@ func (a AdditionalAnnotations) EnsureTLSMetadataUpdate(meta *metav1.ObjectMeta) meta.Annotations = make(map[string]string) } if len(a.JiraComponent) > 0 && meta.Annotations[annotations.OpenShiftComponent] != a.JiraComponent { - diff := cmp.Diff(meta.Annotations[annotations.OpenShiftComponent], a.JiraComponent) - klog.V(2).Infof("Updating %q annotation for %s/%s, diff: %s", annotations.OpenShiftComponent, meta.Namespace, meta.Name, diff) meta.Annotations[annotations.OpenShiftComponent] = a.JiraComponent modified = true } if len(a.Description) > 0 && meta.Annotations[annotations.OpenShiftDescription] != a.Description { - diff := cmp.Diff(meta.Annotations[annotations.OpenShiftDescription], a.Description) - klog.V(2).Infof("Updating %q annotation for %s/%s, diff: %s", annotations.OpenShiftDescription, meta.Namespace, meta.Name, diff) meta.Annotations[annotations.OpenShiftDescription] = a.Description modified = true } if len(a.TestName) > 0 && meta.Annotations[CertificateTestNameAnnotation] != a.TestName { - diff := cmp.Diff(meta.Annotations[CertificateTestNameAnnotation], a.TestName) - klog.V(2).Infof("Updating %q annotation for %s/%s, diff: %s", CertificateTestNameAnnotation, meta.Name, meta.Namespace, diff) meta.Annotations[CertificateTestNameAnnotation] = a.TestName modified = true } if len(a.AutoRegenerateAfterOfflineExpiry) > 0 && meta.Annotations[CertificateAutoRegenerateAfterOfflineExpiryAnnotation] != a.AutoRegenerateAfterOfflineExpiry { - diff := cmp.Diff(meta.Annotations[CertificateAutoRegenerateAfterOfflineExpiryAnnotation], a.AutoRegenerateAfterOfflineExpiry) - klog.V(2).Infof("Updating %q annotation for %s/%s, diff: %s", CertificateAutoRegenerateAfterOfflineExpiryAnnotation, meta.Namespace, meta.Name, diff) meta.Annotations[CertificateAutoRegenerateAfterOfflineExpiryAnnotation] = a.AutoRegenerateAfterOfflineExpiry modified = true } if len(a.NotBefore) > 0 && meta.Annotations[CertificateNotBeforeAnnotation] != a.NotBefore { - diff := cmp.Diff(meta.Annotations[CertificateNotBeforeAnnotation], a.NotBefore) - klog.V(2).Infof("Updating %q annotation for %s/%s, diff: %s", CertificateNotBeforeAnnotation, meta.Name, meta.Namespace, diff) meta.Annotations[CertificateNotBeforeAnnotation] = a.NotBefore modified = true } if len(a.NotAfter) > 0 && meta.Annotations[CertificateNotAfterAnnotation] != a.NotAfter { - diff := cmp.Diff(meta.Annotations[CertificateNotAfterAnnotation], a.NotAfter) - klog.V(2).Infof("Updating %q annotation for %s/%s, diff: %s", CertificateNotAfterAnnotation, meta.Name, meta.Namespace, diff) meta.Annotations[CertificateNotAfterAnnotation] = a.NotAfter modified = true } if len(a.RefreshPeriod) > 0 && meta.Annotations[CertificateRefreshPeriodAnnotation] != a.RefreshPeriod { - diff := cmp.Diff(meta.Annotations[CertificateRefreshPeriodAnnotation], a.RefreshPeriod) - klog.V(2).Infof("Updating %q annotation for %s/%s, diff: %s", CertificateRefreshPeriodAnnotation, meta.Name, meta.Namespace, diff) meta.Annotations[CertificateRefreshPeriodAnnotation] = a.RefreshPeriod modified = true } diff --git a/pkg/operator/certrotation/cabundle.go b/pkg/operator/certrotation/cabundle.go index 447b1e0e31..e38f4f57e4 100644 --- a/pkg/operator/certrotation/cabundle.go +++ b/pkg/operator/certrotation/cabundle.go @@ -8,6 +8,8 @@ import ( "reflect" "sort" + "github.com/google/go-cmp/cmp" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -101,7 +103,14 @@ func (c CABundleConfigMap) EnsureConfigMapCABundle(ctx context.Context, signingC if err != nil { return nil, err } - klog.V(2).Infof("Created ca-bundle.crt configmap %s/%s with:\n%s", certs.CertificateBundleToString(updatedCerts), caBundleConfigMap.Namespace, caBundleConfigMap.Name) + + if klog.V(2).Enabled() { + klog.V(2).Infof("Created ca-bundle.crt configmap %s/%s with:\n%s", caBundleConfigMap.Namespace, caBundleConfigMap.Name, certs.CertificateBundleToString(updatedCerts)) + if diff := cmp.Diff(nil, actualCABundleConfigMap.Annotations); len(diff) > 0 { + klog.V(2).Infof("ConfigMap %s/%s annotations diff: %s", actualCABundleConfigMap.Namespace, actualCABundleConfigMap.Name, diff) + } + } + caBundleConfigMap = actualCABundleConfigMap } else if updateRequired { actualCABundleConfigMap, err := c.Client.ConfigMaps(c.Namespace).Update(ctx, caBundleConfigMap, metav1.UpdateOptions{}) @@ -113,7 +122,14 @@ func (c CABundleConfigMap) EnsureConfigMapCABundle(ctx context.Context, signingC if err != nil { return nil, err } - klog.V(2).Infof("Updated ca-bundle.crt configmap %s/%s with:\n%s", certs.CertificateBundleToString(updatedCerts), caBundleConfigMap.Namespace, caBundleConfigMap.Name) + + if klog.V(2).Enabled() { + klog.V(2).Infof("Updated ca-bundle.crt configmap %s/%s with:\n%s", caBundleConfigMap.Namespace, caBundleConfigMap.Name, certs.CertificateBundleToString(updatedCerts)) + if diff := cmp.Diff(originalCABundleConfigMap.Annotations, actualCABundleConfigMap.Annotations); len(diff) > 0 { + klog.V(2).Infof("ConfigMap %s/%s annotations diff: %s", actualCABundleConfigMap.Namespace, actualCABundleConfigMap.Name, diff) + } + } + caBundleConfigMap = actualCABundleConfigMap } diff --git a/pkg/operator/certrotation/signer.go b/pkg/operator/certrotation/signer.go index c2c8b8368f..89f95ec260 100644 --- a/pkg/operator/certrotation/signer.go +++ b/pkg/operator/certrotation/signer.go @@ -6,6 +6,8 @@ import ( "fmt" "time" + "github.com/google/go-cmp/cmp" + "github.com/openshift/library-go/pkg/crypto" "github.com/openshift/library-go/pkg/operator/events" "github.com/openshift/library-go/pkg/operator/resource/resourcehelper" @@ -64,6 +66,7 @@ func (c RotatedSigningCASecret) EnsureSigningCertKeyPair(ctx context.Context) (* if err != nil && !apierrors.IsNotFound(err) { return nil, false, err } + signingCertKeyPairSecret := originalSigningCertKeyPairSecret.DeepCopy() if apierrors.IsNotFound(err) { // create an empty one @@ -108,7 +111,14 @@ func (c RotatedSigningCASecret) EnsureSigningCertKeyPair(ctx context.Context) (* if err != nil { return nil, false, err } - klog.V(2).Infof("Created secret %s/%s", actualSigningCertKeyPairSecret.Namespace, actualSigningCertKeyPairSecret.Name) + + if klog.V(2).Enabled() { + klog.V(2).Infof("Created secret %s/%s", actualSigningCertKeyPairSecret.Namespace, actualSigningCertKeyPairSecret.Name) + if diff := cmp.Diff(nil, actualSigningCertKeyPairSecret.Annotations); len(diff) > 0 { + klog.V(2).Infof("Secret %s/%s annotations diff: %s", actualSigningCertKeyPairSecret.Namespace, actualSigningCertKeyPairSecret.Name, diff) + } + } + signingCertKeyPairSecret = actualSigningCertKeyPairSecret } else if updateRequired { actualSigningCertKeyPairSecret, err := c.Client.Secrets(c.Namespace).Update(ctx, signingCertKeyPairSecret, metav1.UpdateOptions{}) @@ -120,7 +130,14 @@ func (c RotatedSigningCASecret) EnsureSigningCertKeyPair(ctx context.Context) (* if err != nil { return nil, false, err } - klog.V(2).Infof("Updated secret %s/%s", actualSigningCertKeyPairSecret.Namespace, actualSigningCertKeyPairSecret.Name) + + if klog.V(2).Enabled() { + klog.V(2).Infof("Updated secret %s/%s", actualSigningCertKeyPairSecret.Namespace, actualSigningCertKeyPairSecret.Name) + if diff := cmp.Diff(originalSigningCertKeyPairSecret.Annotations, actualSigningCertKeyPairSecret.Annotations); len(diff) > 0 { + klog.V(2).Infof("Secret %s/%s annotations diff: %s", actualSigningCertKeyPairSecret.Namespace, actualSigningCertKeyPairSecret.Name, diff) + } + } + signingCertKeyPairSecret = actualSigningCertKeyPairSecret } diff --git a/pkg/operator/certrotation/target.go b/pkg/operator/certrotation/target.go index 88cd41189e..a3eb44616a 100644 --- a/pkg/operator/certrotation/target.go +++ b/pkg/operator/certrotation/target.go @@ -7,6 +7,8 @@ import ( "strings" "time" + "github.com/google/go-cmp/cmp" + corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -135,7 +137,14 @@ func (c RotatedSelfSignedCertKeySecret) EnsureTargetCertKeyPair(ctx context.Cont if err != nil { return nil, err } - klog.V(2).Infof("Created secret %s/%s", actualTargetCertKeyPairSecret.Namespace, actualTargetCertKeyPairSecret.Name) + + if klog.V(2).Enabled() { + klog.V(2).Infof("Created secret %s/%s", actualTargetCertKeyPairSecret.Namespace, actualTargetCertKeyPairSecret.Name) + if diff := cmp.Diff(nil, actualTargetCertKeyPairSecret.Annotations); len(diff) > 0 { + klog.V(2).Infof("Secret %s/%s annotations diff: %s", actualTargetCertKeyPairSecret.Namespace, actualTargetCertKeyPairSecret.Name, diff) + } + } + targetCertKeyPairSecret = actualTargetCertKeyPairSecret } else if updateRequired { actualTargetCertKeyPairSecret, err := c.Client.Secrets(c.Namespace).Update(ctx, targetCertKeyPairSecret, metav1.UpdateOptions{}) @@ -147,7 +156,14 @@ func (c RotatedSelfSignedCertKeySecret) EnsureTargetCertKeyPair(ctx context.Cont if err != nil { return nil, err } - klog.V(2).Infof("Updated secret %s/%s", actualTargetCertKeyPairSecret.Namespace, actualTargetCertKeyPairSecret.Name) + + if klog.V(2).Enabled() { + klog.V(2).Infof("Updated secret %s/%s", actualTargetCertKeyPairSecret.Namespace, actualTargetCertKeyPairSecret.Name) + if diff := cmp.Diff(originalTargetCertKeyPairSecret.Annotations, actualTargetCertKeyPairSecret.Annotations); len(diff) > 0 { + klog.V(2).Infof("Secret %s/%s annotations diff: %s", actualTargetCertKeyPairSecret.Namespace, actualTargetCertKeyPairSecret.Name, diff) + } + } + targetCertKeyPairSecret = actualTargetCertKeyPairSecret } diff --git a/pkg/operator/csr/cert_controller.go b/pkg/operator/csr/cert_controller.go index ba4f1b358f..514f7f193e 100644 --- a/pkg/operator/csr/cert_controller.go +++ b/pkg/operator/csr/cert_controller.go @@ -10,6 +10,8 @@ import ( "math/rand" "time" + "github.com/google/go-cmp/cmp" + "github.com/openshift/library-go/pkg/controller/factory" "github.com/openshift/library-go/pkg/operator/certrotation" "github.com/openshift/library-go/pkg/operator/events" @@ -151,6 +153,10 @@ func NewClientCertificateController( func (c *clientCertificateController) sync(ctx context.Context, syncCtx factory.SyncContext) error { // get secret containing client certificate secret, err := c.spokeCoreClient.Secrets(c.SecretNamespace).Get(ctx, c.SecretName, metav1.GetOptions{}) + + // Capture original annotations for diff logging. + var originalAnnotations map[string]string + switch { case errors.IsNotFound(err): secret = &corev1.Secret{ @@ -162,6 +168,12 @@ func (c *clientCertificateController) sync(ctx context.Context, syncCtx factory. } case err != nil: return fmt.Errorf("unable to get secret %q: %w", c.SecretNamespace+"/"+c.SecretName, err) + default: + // Copy original annotations before any modifications + originalAnnotations = make(map[string]string, len(secret.Annotations)) + for k, v := range secret.Annotations { + originalAnnotations[k] = v + } } needsMetadataUpdate := c.AdditionalAnnotations.EnsureTLSMetadataUpdate(&secret.ObjectMeta) @@ -188,14 +200,14 @@ func (c *clientCertificateController) sync(ctx context.Context, syncCtx factory. _ = c.AdditionalAnnotations.EnsureTLSMetadataUpdate(&secret.ObjectMeta) // save the changes into secret - if err := c.saveSecret(secret); err != nil { + if err := c.saveSecret(secret, originalAnnotations); err != nil { return err } syncCtx.Recorder().Eventf("ClientCertificateCreated", "A new client certificate for %s is available", c.controllerName) c.reset() return nil } else if needsMetadataUpdate && len(secret.ResourceVersion) > 0 { - if err := c.saveSecret(secret); err != nil { + if err := c.saveSecret(secret, originalAnnotations); err != nil { return err } } @@ -326,14 +338,36 @@ func (c *clientCertificateController) createCSR(ctx context.Context) (string, er return req.Name, nil } -func (c *clientCertificateController) saveSecret(secret *corev1.Secret) error { - var err error +func (c *clientCertificateController) saveSecret(secret *corev1.Secret, oldAnnotations map[string]string) error { if secret.ResourceVersion == "" { - _, err = c.spokeCoreClient.Secrets(c.SecretNamespace).Create(context.Background(), secret, metav1.CreateOptions{}) + actualSecret, err := c.spokeCoreClient.Secrets(c.SecretNamespace).Create(context.Background(), secret, metav1.CreateOptions{}) + if err != nil { + return err + } + + if klog.V(2).Enabled() { + klog.V(2).Infof("Created secret %s/%s", actualSecret.Namespace, actualSecret.Name) + if diff := cmp.Diff(nil, actualSecret.Annotations); len(diff) > 0 { + klog.V(2).Infof("Secret %s/%s annotations diff: %s", actualSecret.Namespace, actualSecret.Name, diff) + } + } + + return nil + } + + actualSecret, err := c.spokeCoreClient.Secrets(c.SecretNamespace).Update(context.Background(), secret, metav1.UpdateOptions{}) + if err != nil { return err } - _, err = c.spokeCoreClient.Secrets(c.SecretNamespace).Update(context.Background(), secret, metav1.UpdateOptions{}) - return err + + if klog.V(2).Enabled() { + klog.V(2).Infof("Updated secret %s/%s", actualSecret.Namespace, actualSecret.Name) + if diff := cmp.Diff(oldAnnotations, actualSecret.Annotations); len(diff) > 0 { + klog.V(2).Infof("Secret %s/%s annotations diff: %s", actualSecret.Namespace, actualSecret.Name, diff) + } + } + + return nil } func (c *clientCertificateController) reset() {