Merge pull request #1980 from p0lyn0mial/refactor-setSigningCertKeyPairSecret

operator/certrotation/signer: decouple setting annotation from setSigningCertKeyPairSecret into a separate fn

Author: openshift-merge-bot[bot]

pkg/operator/certrotation/metadata.go

@@ -5,7 +5,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-func ensureMetadataUpdate(secret *corev1.Secret, owner *metav1.OwnerReference, additionalAnnotations AdditionalAnnotations) bool {
+func ensureOwnerRefAndTLSAnnotations(secret *corev1.Secret, owner *metav1.OwnerReference, additionalAnnotations AdditionalAnnotations) bool {
 	needsMetadataUpdate := false
 	// no ownerReference set
 	if owner != nil {

pkg/operator/certrotation/signer.go

@@ -80,7 +80,7 @@ func (c RotatedSigningCASecret) EnsureSigningCertKeyPair(ctx context.Context) (*
 
 	// run Update if metadata needs changing unless we're in RefreshOnlyWhenExpired mode
 	if !c.RefreshOnlyWhenExpired {
-		needsMetadataUpdate := ensureMetadataUpdate(signingCertKeyPairSecret, c.Owner, c.AdditionalAnnotations)
+		needsMetadataUpdate := ensureOwnerRefAndTLSAnnotations(signingCertKeyPairSecret, c.Owner, c.AdditionalAnnotations)
 		needsTypeChange := ensureSecretTLSTypeSet(signingCertKeyPairSecret)
 		updateRequired = needsMetadataUpdate || needsTypeChange
 	}
@@ -92,7 +92,7 @@ func (c RotatedSigningCASecret) EnsureSigningCertKeyPair(ctx context.Context) (*
 			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, c.AdditionalAnnotations); err != nil {
+		if err = setSigningCertKeyPairSecretAndTLSAnnotations(signingCertKeyPairSecret, c.Validity, c.AdditionalAnnotations); err != nil {
 			return nil, false, err
 		}
 
@@ -199,18 +199,30 @@ func getValidityFromAnnotations(annotations map[string]string) (notBefore time.T
 	return notBefore, notAfter, ""
 }
 
+// setSigningCertKeyPairSecretAndTLSAnnotations generates a new signing certificate and key pair,
+// stores them in the specified secret, and adds predefined TLS annotations to that secret.
+func setSigningCertKeyPairSecretAndTLSAnnotations(signingCertKeyPairSecret *corev1.Secret, validity time.Duration, tlsAnnotations AdditionalAnnotations) error {
+	ca, err := setSigningCertKeyPairSecret(signingCertKeyPairSecret, validity)
+	if err != nil {
+		return err
+	}
+
+	setTLSAnnotationsOnSigningCertKeyPairSecret(signingCertKeyPairSecret, ca, tlsAnnotations)
+	return nil
+}
+
 // setSigningCertKeyPairSecret creates a new signing cert/key pair and sets them in the secret
-func setSigningCertKeyPairSecret(signingCertKeyPairSecret *corev1.Secret, validity time.Duration, annotations AdditionalAnnotations) error {
+func setSigningCertKeyPairSecret(signingCertKeyPairSecret *corev1.Secret, validity time.Duration) (*crypto.TLSCertificateConfig, error) {
 	signerName := fmt.Sprintf("%s_%s@%d", signingCertKeyPairSecret.Namespace, signingCertKeyPairSecret.Name, time.Now().Unix())
 	ca, err := crypto.MakeSelfSignedCAConfigForDuration(signerName, validity)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	certBytes := &bytes.Buffer{}
 	keyBytes := &bytes.Buffer{}
-	if err := ca.WriteCertConfig(certBytes, keyBytes); err != nil {
-		return err
+	if err = ca.WriteCertConfig(certBytes, keyBytes); err != nil {
+		return nil, err
 	}
 
 	if signingCertKeyPairSecret.Annotations == nil {
@@ -221,11 +233,20 @@ func setSigningCertKeyPairSecret(signingCertKeyPairSecret *corev1.Secret, validi
 	}
 	signingCertKeyPairSecret.Data["tls.crt"] = certBytes.Bytes()
 	signingCertKeyPairSecret.Data["tls.key"] = keyBytes.Bytes()
-	annotations.NotBefore = ca.Certs[0].NotBefore.Format(time.RFC3339)
-	annotations.NotAfter = ca.Certs[0].NotAfter.Format(time.RFC3339)
-	signingCertKeyPairSecret.Annotations[CertificateIssuer] = ca.Certs[0].Issuer.CommonName
+	return ca, nil
+}
 
-	_ = annotations.EnsureTLSMetadataUpdate(&signingCertKeyPairSecret.ObjectMeta)
+// setTLSAnnotationsOnSigningCertKeyPairSecret applies predefined TLS annotations to the given secret.
+//
+// This function does not perform nil checks on its parameters and assumes that the
+// secret's Annotations field has already been initialized.
+//
+// These assumptions are safe because this function is only called after the secret
+// has been initialized in setSigningCertKeyPairSecret.
+func setTLSAnnotationsOnSigningCertKeyPairSecret(signingCertKeyPairSecret *corev1.Secret, ca *crypto.TLSCertificateConfig, tlsAnnotations AdditionalAnnotations) {
+	signingCertKeyPairSecret.Annotations[CertificateIssuer] = ca.Certs[0].Issuer.CommonName
 
-	return nil
+	tlsAnnotations.NotBefore = ca.Certs[0].NotBefore.Format(time.RFC3339)
+	tlsAnnotations.NotAfter = ca.Certs[0].NotAfter.Format(time.RFC3339)
+	_ = tlsAnnotations.EnsureTLSMetadataUpdate(&signingCertKeyPairSecret.ObjectMeta)
 }

pkg/operator/certrotation/target.go

@@ -114,7 +114,7 @@ func (c RotatedSelfSignedCertKeySecret) EnsureTargetCertKeyPair(ctx context.Cont
 
 	// run Update if metadata needs changing unless we're in RefreshOnlyWhenExpired mode
 	if !c.RefreshOnlyWhenExpired {
-		needsMetadataUpdate := ensureMetadataUpdate(targetCertKeyPairSecret, c.Owner, c.AdditionalAnnotations)
+		needsMetadataUpdate := ensureOwnerRefAndTLSAnnotations(targetCertKeyPairSecret, c.Owner, c.AdditionalAnnotations)
 		needsTypeChange := ensureSecretTLSTypeSet(targetCertKeyPairSecret)
 		updateRequired = needsMetadataUpdate || needsTypeChange
 	}