diff --git a/pkg/operator/certrotation/annotations.go b/pkg/operator/certrotation/annotations.go index 92de468603..dd81000ca0 100644 --- a/pkg/operator/certrotation/annotations.go +++ b/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 +} diff --git a/pkg/operator/certrotation/cabundle.go b/pkg/operator/certrotation/cabundle.go index e564cdf2c4..7ec91f7863 100644 --- a/pkg/operator/certrotation/cabundle.go +++ b/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,8 +54,7 @@ func (c CABundleConfigMap) EnsureConfigMapCABundle(ctx context.Context, signingC caBundleConfigMap = &corev1.ConfigMap{ObjectMeta: NewTLSArtifactObjectMeta( c.Name, c.Namespace, - c.JiraComponent, - c.Description, + c.AdditionalAnnotations, )} } @@ -66,9 +62,7 @@ func (c CABundleConfigMap) EnsureConfigMapCABundle(ctx context.Context, signingC 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 { diff --git a/pkg/operator/certrotation/signer.go b/pkg/operator/certrotation/signer.go index aa24e72672..d143dc3056 100644 --- a/pkg/operator/certrotation/signer.go +++ b/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 { diff --git a/pkg/operator/certrotation/target.go b/pkg/operator/certrotation/target.go index 8c112c45b2..ad1caa6379 100644 --- a/pkg/operator/certrotation/target.go +++ b/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 diff --git a/pkg/operator/csr/cert_controller.go b/pkg/operator/csr/cert_controller.go index 6e3b857386..84ae75e1d5 100644 --- a/pkg/operator/csr/cert_controller.go +++ b/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 { diff --git a/pkg/operator/resourcesynccontroller/core.go b/pkg/operator/resourcesynccontroller/core.go index 0548be9249..9711348b33 100644 --- a/pkg/operator/resourcesynccontroller/core.go +++ b/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),