Merge pull request #1982 from p0lyn0mial/refactor-setTargetCertKeyPairSecret

operator/certrotation/target: decouple setting annotation from setTargetCertKeyPairSecret into a seperate fn

Author: openshift-merge-bot[bot]

pkg/operator/certrotation/target.go

@@ -121,7 +121,7 @@ func (c RotatedSelfSignedCertKeySecret) EnsureTargetCertKeyPair(ctx context.Cont
 
 	if reason := c.CertCreator.NeedNewTargetCertKeyPair(targetCertKeyPairSecret, signingCertKeyPair, caBundleCerts, c.Refresh, c.RefreshOnlyWhenExpired, creationRequired); len(reason) > 0 {
 		c.EventRecorder.Eventf("TargetUpdateRequired", "%q in %q requires a new target cert/key pair: %v", c.Name, c.Namespace, reason)
-		if err := setTargetCertKeyPairSecret(targetCertKeyPairSecret, c.Validity, signingCertKeyPair, c.CertCreator, c.AdditionalAnnotations); err != nil {
+		if err = setTargetCertKeyPairSecretAndTLSAnnotations(targetCertKeyPairSecret, c.Validity, signingCertKeyPair, c.CertCreator, c.AdditionalAnnotations); err != nil {
 			return nil, err
 		}
 
@@ -232,9 +232,21 @@ func needNewTargetCertKeyPairForTime(annotations map[string]string, signer *cryp
 	return ""
 }
 
+// setTargetCertKeyPairSecretAndTLSAnnotations generates a new cert/key pair,
+// stores them in the specified secret, and adds predefined TLS annotations to that secret.
+func setTargetCertKeyPairSecretAndTLSAnnotations(targetCertKeyPairSecret *corev1.Secret, validity time.Duration, signer *crypto.CA, certCreator TargetCertCreator, tlsAnnotations AdditionalAnnotations) error {
+	certKeyPair, err := setTargetCertKeyPairSecret(targetCertKeyPairSecret, validity, signer, certCreator)
+	if err != nil {
+		return err
+	}
+
+	setTLSAnnotationsOnTargetCertKeyPairSecret(targetCertKeyPairSecret, certKeyPair, certCreator, tlsAnnotations)
+	return nil
+}
+
 // setTargetCertKeyPairSecret creates a new cert/key pair and sets them in the secret.  Only one of client, serving, or signer rotation may be specified.
 // TODO refactor with an interface for actually signing and move the one-of check higher in the stack.
-func setTargetCertKeyPairSecret(targetCertKeyPairSecret *corev1.Secret, validity time.Duration, signer *crypto.CA, certCreator TargetCertCreator, annotations AdditionalAnnotations) error {
+func setTargetCertKeyPairSecret(targetCertKeyPairSecret *corev1.Secret, validity time.Duration, signer *crypto.CA, certCreator TargetCertCreator) (*crypto.TLSCertificateConfig, error) {
 	if targetCertKeyPairSecret.Annotations == nil {
 		targetCertKeyPairSecret.Annotations = map[string]string{}
 	}
@@ -251,21 +263,28 @@ func setTargetCertKeyPairSecret(targetCertKeyPairSecret *corev1.Secret, validity
 
 	certKeyPair, err := certCreator.NewCertificate(signer, targetValidity)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	targetCertKeyPairSecret.Data["tls.crt"], targetCertKeyPairSecret.Data["tls.key"], err = certKeyPair.GetPEMBytes()
-	if err != nil {
-		return err
-	}
-	annotations.NotBefore = certKeyPair.Certs[0].NotBefore.Format(time.RFC3339)
-	annotations.NotAfter = certKeyPair.Certs[0].NotAfter.Format(time.RFC3339)
+	return certKeyPair, err
+}
+
+// setTLSAnnotationsOnTargetCertKeyPairSecret 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 setTargetCertKeyPairSecret.
+func setTLSAnnotationsOnTargetCertKeyPairSecret(targetCertKeyPairSecret *corev1.Secret, certKeyPair *crypto.TLSCertificateConfig, certCreator TargetCertCreator, tlsAnnotations AdditionalAnnotations) {
 	targetCertKeyPairSecret.Annotations[CertificateIssuer] = certKeyPair.Certs[0].Issuer.CommonName
 
-	_ = annotations.EnsureTLSMetadataUpdate(&targetCertKeyPairSecret.ObjectMeta)
-	certCreator.SetAnnotations(certKeyPair, targetCertKeyPairSecret.Annotations)
+	tlsAnnotations.NotBefore = certKeyPair.Certs[0].NotBefore.Format(time.RFC3339)
+	tlsAnnotations.NotAfter = certKeyPair.Certs[0].NotAfter.Format(time.RFC3339)
+	_ = tlsAnnotations.EnsureTLSMetadataUpdate(&targetCertKeyPairSecret.ObjectMeta)
 
-	return nil
+	certCreator.SetAnnotations(certKeyPair, targetCertKeyPairSecret.Annotations)
 }
 
 type ClientRotation struct {