certrotation: replace JiraComponent/Description with AdditionalMetada…

…ta struct

Function signature won't require changes when new cert requirements are being created

pkg/operator/certrotation/annotations.go

@@ -5,30 +5,34 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-func NewTLSArtifactObjectMeta(name, namespace, jiraComponent, description string) metav1.ObjectMeta {
-	return metav1.ObjectMeta{
-		Namespace: namespace,
-		Name:      name,
-		Annotations: map[string]string{
-			annotations.OpenShiftComponent:   jiraComponent,
-			annotations.OpenShiftDescription: description,
-		},
-	}
+type AdditionalAnnotations struct {
+	// JiraComponent annotates tls artifacts so that owner could be easily found
+	JiraComponent string
+	// Description is a human-readable one sentence description of certificate purpose
+	Description string
 }
 
-// EnsureTLSMetadataUpdate mutates objectMeta setting necessary annotations if unset
-func EnsureTLSMetadataUpdate(meta *metav1.ObjectMeta, jiraComponent, description string) bool {
+func (a AdditionalAnnotations) EnsureTLSMetadataUpdate(meta *metav1.ObjectMeta) bool {
 	modified := false
 	if meta.Annotations == nil {
 		meta.Annotations = make(map[string]string)
 	}
-	if len(jiraComponent) > 0 && meta.Annotations[annotations.OpenShiftComponent] != jiraComponent {
-		meta.Annotations[annotations.OpenShiftComponent] = jiraComponent
+	if len(a.JiraComponent) > 0 && meta.Annotations[annotations.OpenShiftComponent] != a.JiraComponent {
+		meta.Annotations[annotations.OpenShiftComponent] = a.JiraComponent
 		modified = true
 	}
-	if len(description) > 0 && meta.Annotations[annotations.OpenShiftDescription] != description {
-		meta.Annotations[annotations.OpenShiftDescription] = description
+	if len(a.Description) > 0 && meta.Annotations[annotations.OpenShiftDescription] != a.Description {
+		meta.Annotations[annotations.OpenShiftDescription] = a.Description
 		modified = true
 	}
 	return modified
 }
+
+func NewTLSArtifactObjectMeta(name, namespace string, annotations AdditionalAnnotations) metav1.ObjectMeta {
+	meta := metav1.ObjectMeta{
+		Namespace: namespace,
+		Name:      name,
+	}
+	_ = annotations.EnsureTLSMetadataUpdate(&meta)
+	return meta
+}

pkg/operator/certrotation/cabundle.go

@@ -32,11 +32,8 @@ type CABundleConfigMap struct {
 	Name string
 	// Owner is an optional reference to add to the secret that this rotator creates.
 	Owner *metav1.OwnerReference
-	// JiraComponent annotates tls artifacts so that owner could be easily found
-	JiraComponent string
-	// Description is a human-readable one sentence description of certificate purpose
-	Description string
-
+	// AdditionalAnnotations is a collection of annotations set for the secret
+	AdditionalAnnotations AdditionalAnnotations
 	// Plumbing:
 	Informer      corev1informers.ConfigMapInformer
 	Lister        corev1listers.ConfigMapLister
@@ -57,18 +54,15 @@ func (c CABundleConfigMap) EnsureConfigMapCABundle(ctx context.Context, signingC
 		caBundleConfigMap = &corev1.ConfigMap{ObjectMeta: NewTLSArtifactObjectMeta(
 			c.Name,
 			c.Namespace,
-			c.JiraComponent,
-			c.Description,
+			c.AdditionalAnnotations,
 		)}
 	}
 
 	needsMetadataUpdate := false
 	if c.Owner != nil {
 		needsMetadataUpdate = ensureOwnerReference(&caBundleConfigMap.ObjectMeta, c.Owner)
 	}
-	if len(c.JiraComponent) > 0 || len(c.Description) > 0 {
-		needsMetadataUpdate = EnsureTLSMetadataUpdate(&caBundleConfigMap.ObjectMeta, c.JiraComponent, c.Description) || needsMetadataUpdate
-	}
+	needsMetadataUpdate = c.AdditionalAnnotations.EnsureTLSMetadataUpdate(&caBundleConfigMap.ObjectMeta) || needsMetadataUpdate
 	if needsMetadataUpdate && len(caBundleConfigMap.ResourceVersion) > 0 {
 		_, _, err := resourceapply.ApplyConfigMap(ctx, c.Client, c.EventRecorder, caBundleConfigMap)
 		if err != nil {

pkg/operator/certrotation/signer.go

@@ -44,11 +44,9 @@ type RotatedSigningCASecret struct {
 	// is used, early deletion will be catastrophic.
 	Owner *metav1.OwnerReference
 
-	// JiraComponent annotates tls artifacts so that owner could be easily found
-	JiraComponent string
+	// AdditionalAnnotations is a collection of annotations set for the secret
+	AdditionalAnnotations AdditionalAnnotations
 
-	// Description is a human-readable one sentence description of certificate purpose
-	Description string
 	// Plumbing:
 	Informer      corev1informers.SecretInformer
 	Lister        corev1listers.SecretLister
@@ -67,8 +65,7 @@ func (c RotatedSigningCASecret) EnsureSigningCertKeyPair(ctx context.Context) (*
 		signingCertKeyPairSecret = &corev1.Secret{ObjectMeta: NewTLSArtifactObjectMeta(
 			c.Name,
 			c.Namespace,
-			c.JiraComponent,
-			c.Description,
+			c.AdditionalAnnotations,
 		)}
 	}
 	signingCertKeyPairSecret.Type = corev1.SecretTypeTLS
@@ -77,9 +74,7 @@ func (c RotatedSigningCASecret) EnsureSigningCertKeyPair(ctx context.Context) (*
 	if c.Owner != nil {
 		needsMetadataUpdate = ensureOwnerReference(&signingCertKeyPairSecret.ObjectMeta, c.Owner)
 	}
-	if len(c.JiraComponent) > 0 || len(c.Description) > 0 {
-		needsMetadataUpdate = EnsureTLSMetadataUpdate(&signingCertKeyPairSecret.ObjectMeta, c.JiraComponent, c.Description) || needsMetadataUpdate
-	}
+	needsMetadataUpdate = c.AdditionalAnnotations.EnsureTLSMetadataUpdate(&signingCertKeyPairSecret.ObjectMeta) || needsMetadataUpdate
 	if needsMetadataUpdate && len(signingCertKeyPairSecret.ResourceVersion) > 0 {
 		_, _, err := resourceapply.ApplySecret(ctx, c.Client, c.EventRecorder, signingCertKeyPairSecret)
 		if err != nil {

pkg/operator/certrotation/target.go

@@ -7,7 +7,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/openshift/api/annotations"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -58,11 +57,8 @@ type RotatedSelfSignedCertKeySecret struct {
 	// certificate is used, early deletion will be catastrophic.
 	Owner *metav1.OwnerReference
 
-	// JiraComponent annotates tls artifacts so that owner could be easily found
-	JiraComponent string
-
-	// Description is a human-readable one sentence description of certificate purpose
-	Description string
+	// AdditionalAnnotations is a collection of annotations set for the secret
+	AdditionalAnnotations AdditionalAnnotations
 
 	// CertCreator does the actual cert generation.
 	CertCreator TargetCertCreator
@@ -104,8 +100,7 @@ func (c RotatedSelfSignedCertKeySecret) EnsureTargetCertKeyPair(ctx context.Cont
 		targetCertKeyPairSecret = &corev1.Secret{ObjectMeta: NewTLSArtifactObjectMeta(
 			c.Name,
 			c.Namespace,
-			c.JiraComponent,
-			c.Description,
+			c.AdditionalAnnotations,
 		)}
 	}
 	targetCertKeyPairSecret.Type = corev1.SecretTypeTLS
@@ -114,9 +109,7 @@ func (c RotatedSelfSignedCertKeySecret) EnsureTargetCertKeyPair(ctx context.Cont
 	if c.Owner != nil {
 		needsMetadataUpdate = ensureOwnerReference(&targetCertKeyPairSecret.ObjectMeta, c.Owner)
 	}
-	if len(c.JiraComponent) > 0 || len(c.Description) > 0 {
-		needsMetadataUpdate = EnsureTLSMetadataUpdate(&targetCertKeyPairSecret.ObjectMeta, c.JiraComponent, c.Description) || needsMetadataUpdate
-	}
+	needsMetadataUpdate = c.AdditionalAnnotations.EnsureTLSMetadataUpdate(&targetCertKeyPairSecret.ObjectMeta) || needsMetadataUpdate
 	if needsMetadataUpdate && len(targetCertKeyPairSecret.ResourceVersion) > 0 {
 		_, _, err := resourceapply.ApplySecret(ctx, c.Client, c.EventRecorder, targetCertKeyPairSecret)
 		if err != nil {
@@ -126,7 +119,7 @@ func (c RotatedSelfSignedCertKeySecret) EnsureTargetCertKeyPair(ctx context.Cont
 
 	if reason := c.CertCreator.NeedNewTargetCertKeyPair(targetCertKeyPairSecret.Annotations, signingCertKeyPair, caBundleCerts, c.Refresh, c.RefreshOnlyWhenExpired); 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.JiraComponent, c.Description); err != nil {
+		if err := setTargetCertKeyPairSecret(targetCertKeyPairSecret, c.Validity, signingCertKeyPair, c.CertCreator, c.AdditionalAnnotations); err != nil {
 			return nil, err
 		}
 
@@ -217,7 +210,7 @@ func needNewTargetCertKeyPairForTime(annotations map[string]string, signer *cryp
 
 // 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, jiraComponent, description string) error {
+func setTargetCertKeyPairSecret(targetCertKeyPairSecret *corev1.Secret, validity time.Duration, signer *crypto.CA, certCreator TargetCertCreator, annotations AdditionalAnnotations) error {
 	if targetCertKeyPairSecret.Annotations == nil {
 		targetCertKeyPairSecret.Annotations = map[string]string{}
 	}
@@ -244,12 +237,8 @@ func setTargetCertKeyPairSecret(targetCertKeyPairSecret *corev1.Secret, validity
 	targetCertKeyPairSecret.Annotations[CertificateNotAfterAnnotation] = certKeyPair.Certs[0].NotAfter.Format(time.RFC3339)
 	targetCertKeyPairSecret.Annotations[CertificateNotBeforeAnnotation] = certKeyPair.Certs[0].NotBefore.Format(time.RFC3339)
 	targetCertKeyPairSecret.Annotations[CertificateIssuer] = certKeyPair.Certs[0].Issuer.CommonName
-	if len(jiraComponent) > 0 {
-		targetCertKeyPairSecret.Annotations[annotations.OpenShiftComponent] = jiraComponent
-	}
-	if len(description) > 0 {
-		targetCertKeyPairSecret.Annotations[annotations.OpenShiftDescription] = description
-	}
+
+	_ = annotations.EnsureTLSMetadataUpdate(&targetCertKeyPairSecret.ObjectMeta)
 	certCreator.SetAnnotations(certKeyPair, targetCertKeyPairSecret.Annotations)
 
 	return nil

pkg/operator/csr/cert_controller.go

@@ -64,10 +64,8 @@ type ClientCertOption struct {
 	SecretName string
 	// AdditonalSecretData contains data that will be added into client certificate secret besides tls.key/tls.crt
 	AdditonalSecretData map[string][]byte
-	// JiraComponent annotates tls artifacts so that owner could be easily found
-	JiraComponent string
-	// Description is a human-readable one sentence description of certificate purpose
-	Description string
+	// AdditionalAnnotations is a collection of annotations set for the secret
+	AdditionalAnnotations certrotation.AdditionalAnnotations
 }
 
 // clientCertificateController implements the common logic of hub client certification creation/rotation. It
@@ -154,18 +152,14 @@ func (c *clientCertificateController) sync(ctx context.Context, syncCtx factory.
 			ObjectMeta: certrotation.NewTLSArtifactObjectMeta(
 				c.SecretName,
 				c.SecretNamespace,
-				c.JiraComponent,
-				c.Description,
+				c.AdditionalAnnotations,
 			),
 		}
 	case err != nil:
 		return fmt.Errorf("unable to get secret %q: %w", c.SecretNamespace+"/"+c.SecretName, err)
 	}
 
-	needsMetadataUpdate := false
-	if len(c.JiraComponent) > 0 || len(c.Description) > 0 {
-		needsMetadataUpdate = certrotation.EnsureTLSMetadataUpdate(&secret.ObjectMeta, c.JiraComponent, c.Description)
-	}
+	needsMetadataUpdate := c.AdditionalAnnotations.EnsureTLSMetadataUpdate(&secret.ObjectMeta)
 
 	// reconcile pending csr if exists
 	if len(c.csrName) > 0 {

pkg/operator/resourcesynccontroller/core.go

@@ -14,7 +14,7 @@ import (
 	"github.com/openshift/library-go/pkg/operator/certrotation"
 )
 
-func CombineCABundleConfigMaps(destinationConfigMap ResourceLocation, lister corev1listers.ConfigMapLister, jiraComponent, description string, inputConfigMaps ...ResourceLocation) (*corev1.ConfigMap, error) {
+func CombineCABundleConfigMaps(destinationConfigMap ResourceLocation, lister corev1listers.ConfigMapLister, additionalAnnotations certrotation.AdditionalAnnotations, inputConfigMaps ...ResourceLocation) (*corev1.ConfigMap, error) {
 	certificates := []*x509.Certificate{}
 	for _, input := range inputConfigMaps {
 		inputConfigMap, err := lister.ConfigMaps(input.Namespace).Get(input.Name)
@@ -62,8 +62,7 @@ func CombineCABundleConfigMaps(destinationConfigMap ResourceLocation, lister cor
 		ObjectMeta: certrotation.NewTLSArtifactObjectMeta(
 			destinationConfigMap.Name,
 			destinationConfigMap.Namespace,
-			jiraComponent,
-			description,
+			additionalAnnotations,
 		),
 		Data: map[string]string{
 			"ca-bundle.crt": string(caBytes),