Skip to content

Commit df7ff42

Browse files
vrutkovsvrutkovs
committed
certrotation: replace JiraComponent/Description with AdditionalMetadata struct
Function signature won't require changes when new cert requirements are being created
1 parent 2ad7865 commit df7ff42

File tree

6 files changed

+41
-66
lines changed

6 files changed

+41
-66
lines changed

‎pkg/operator/certrotation/annotations.go

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,30 +5,34 @@ import (
55
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
66
)
77

8-
func NewTLSArtifactObjectMeta(name, namespace, jiraComponent, description string) metav1.ObjectMeta {
9-
return metav1.ObjectMeta{
10-
Namespace: namespace,
11-
Name: name,
12-
Annotations: map[string]string{
13-
annotations.OpenShiftComponent: jiraComponent,
14-
annotations.OpenShiftDescription: description,
15-
},
16-
}
8+
type AdditionalAnnotations struct {
9+
// JiraComponent annotates tls artifacts so that owner could be easily found
10+
JiraComponent string
11+
// Description is a human-readable one sentence description of certificate purpose
12+
Description string
1713
}
1814

19-
// EnsureTLSMetadataUpdate mutates objectMeta setting necessary annotations if unset
20-
func EnsureTLSMetadataUpdate(meta *metav1.ObjectMeta, jiraComponent, description string) bool {
15+
func (a AdditionalAnnotations) EnsureTLSMetadataUpdate(meta *metav1.ObjectMeta) bool {
2116
modified := false
2217
if meta.Annotations == nil {
2318
meta.Annotations = make(map[string]string)
2419
}
25-
if len(jiraComponent) > 0 && meta.Annotations[annotations.OpenShiftComponent] != jiraComponent {
26-
meta.Annotations[annotations.OpenShiftComponent] = jiraComponent
20+
if len(a.JiraComponent) > 0 && meta.Annotations[annotations.OpenShiftComponent] != a.JiraComponent {
21+
meta.Annotations[annotations.OpenShiftComponent] = a.JiraComponent
2722
modified = true
2823
}
29-
if len(description) > 0 && meta.Annotations[annotations.OpenShiftDescription] != description {
30-
meta.Annotations[annotations.OpenShiftDescription] = description
24+
if len(a.Description) > 0 && meta.Annotations[annotations.OpenShiftDescription] != a.Description {
25+
meta.Annotations[annotations.OpenShiftDescription] = a.Description
3126
modified = true
3227
}
3328
return modified
3429
}
30+
31+
func NewTLSArtifactObjectMeta(name, namespace string, annotations AdditionalAnnotations) metav1.ObjectMeta {
32+
meta := metav1.ObjectMeta{
33+
Namespace: namespace,
34+
Name: name,
35+
}
36+
_ = annotations.EnsureTLSMetadataUpdate(&meta)
37+
return meta
38+
}

‎pkg/operator/certrotation/cabundle.go

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,8 @@ type CABundleConfigMap struct {
3232
Name string
3333
// Owner is an optional reference to add to the secret that this rotator creates.
3434
Owner *metav1.OwnerReference
35-
// JiraComponent annotates tls artifacts so that owner could be easily found
36-
JiraComponent string
37-
// Description is a human-readable one sentence description of certificate purpose
38-
Description string
39-
35+
// AdditionalAnnotations is a collection of annotations set for the secret
36+
AdditionalAnnotations AdditionalAnnotations
4037
// Plumbing:
4138
Informer corev1informers.ConfigMapInformer
4239
Lister corev1listers.ConfigMapLister
@@ -57,18 +54,15 @@ func (c CABundleConfigMap) EnsureConfigMapCABundle(ctx context.Context, signingC
5754
caBundleConfigMap = &corev1.ConfigMap{ObjectMeta: NewTLSArtifactObjectMeta(
5855
c.Name,
5956
c.Namespace,
60-
c.JiraComponent,
61-
c.Description,
57+
c.AdditionalAnnotations,
6258
)}
6359
}
6460

6561
needsMetadataUpdate := false
6662
if c.Owner != nil {
6763
needsMetadataUpdate = ensureOwnerReference(&caBundleConfigMap.ObjectMeta, c.Owner)
6864
}
69-
if len(c.JiraComponent) > 0 || len(c.Description) > 0 {
70-
needsMetadataUpdate = EnsureTLSMetadataUpdate(&caBundleConfigMap.ObjectMeta, c.JiraComponent, c.Description) || needsMetadataUpdate
71-
}
65+
needsMetadataUpdate = c.AdditionalAnnotations.EnsureTLSMetadataUpdate(&caBundleConfigMap.ObjectMeta) || needsMetadataUpdate
7266
if needsMetadataUpdate && len(caBundleConfigMap.ResourceVersion) > 0 {
7367
_, _, err := resourceapply.ApplyConfigMap(ctx, c.Client, c.EventRecorder, caBundleConfigMap)
7468
if err != nil {

‎pkg/operator/certrotation/signer.go

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,9 @@ type RotatedSigningCASecret struct {
4444
// is used, early deletion will be catastrophic.
4545
Owner *metav1.OwnerReference
4646

47-
// JiraComponent annotates tls artifacts so that owner could be easily found
48-
JiraComponent string
47+
// AdditionalAnnotations is a collection of annotations set for the secret
48+
AdditionalAnnotations AdditionalAnnotations
4949

50-
// Description is a human-readable one sentence description of certificate purpose
51-
Description string
5250
// Plumbing:
5351
Informer corev1informers.SecretInformer
5452
Lister corev1listers.SecretLister
@@ -67,8 +65,7 @@ func (c RotatedSigningCASecret) EnsureSigningCertKeyPair(ctx context.Context) (*
6765
signingCertKeyPairSecret = &corev1.Secret{ObjectMeta: NewTLSArtifactObjectMeta(
6866
c.Name,
6967
c.Namespace,
70-
c.JiraComponent,
71-
c.Description,
68+
c.AdditionalAnnotations,
7269
)}
7370
}
7471
signingCertKeyPairSecret.Type = corev1.SecretTypeTLS
@@ -77,9 +74,7 @@ func (c RotatedSigningCASecret) EnsureSigningCertKeyPair(ctx context.Context) (*
7774
if c.Owner != nil {
7875
needsMetadataUpdate = ensureOwnerReference(&signingCertKeyPairSecret.ObjectMeta, c.Owner)
7976
}
80-
if len(c.JiraComponent) > 0 || len(c.Description) > 0 {
81-
needsMetadataUpdate = EnsureTLSMetadataUpdate(&signingCertKeyPairSecret.ObjectMeta, c.JiraComponent, c.Description) || needsMetadataUpdate
82-
}
77+
needsMetadataUpdate = c.AdditionalAnnotations.EnsureTLSMetadataUpdate(&signingCertKeyPairSecret.ObjectMeta) || needsMetadataUpdate
8378
if needsMetadataUpdate && len(signingCertKeyPairSecret.ResourceVersion) > 0 {
8479
_, _, err := resourceapply.ApplySecret(ctx, c.Client, c.EventRecorder, signingCertKeyPairSecret)
8580
if err != nil {

‎pkg/operator/certrotation/target.go

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"strings"
88
"time"
99

10-
"github.com/openshift/api/annotations"
1110
corev1 "k8s.io/api/core/v1"
1211
apierrors "k8s.io/apimachinery/pkg/api/errors"
1312
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -58,11 +57,8 @@ type RotatedSelfSignedCertKeySecret struct {
5857
// certificate is used, early deletion will be catastrophic.
5958
Owner *metav1.OwnerReference
6059

61-
// JiraComponent annotates tls artifacts so that owner could be easily found
62-
JiraComponent string
63-
64-
// Description is a human-readable one sentence description of certificate purpose
65-
Description string
60+
// AdditionalAnnotations is a collection of annotations set for the secret
61+
AdditionalAnnotations AdditionalAnnotations
6662

6763
// CertCreator does the actual cert generation.
6864
CertCreator TargetCertCreator
@@ -104,8 +100,7 @@ func (c RotatedSelfSignedCertKeySecret) EnsureTargetCertKeyPair(ctx context.Cont
104100
targetCertKeyPairSecret = &corev1.Secret{ObjectMeta: NewTLSArtifactObjectMeta(
105101
c.Name,
106102
c.Namespace,
107-
c.JiraComponent,
108-
c.Description,
103+
c.AdditionalAnnotations,
109104
)}
110105
}
111106
targetCertKeyPairSecret.Type = corev1.SecretTypeTLS
@@ -114,9 +109,7 @@ func (c RotatedSelfSignedCertKeySecret) EnsureTargetCertKeyPair(ctx context.Cont
114109
if c.Owner != nil {
115110
needsMetadataUpdate = ensureOwnerReference(&targetCertKeyPairSecret.ObjectMeta, c.Owner)
116111
}
117-
if len(c.JiraComponent) > 0 || len(c.Description) > 0 {
118-
needsMetadataUpdate = EnsureTLSMetadataUpdate(&targetCertKeyPairSecret.ObjectMeta, c.JiraComponent, c.Description) || needsMetadataUpdate
119-
}
112+
needsMetadataUpdate = c.AdditionalAnnotations.EnsureTLSMetadataUpdate(&targetCertKeyPairSecret.ObjectMeta) || needsMetadataUpdate
120113
if needsMetadataUpdate && len(targetCertKeyPairSecret.ResourceVersion) > 0 {
121114
_, _, err := resourceapply.ApplySecret(ctx, c.Client, c.EventRecorder, targetCertKeyPairSecret)
122115
if err != nil {
@@ -126,7 +119,7 @@ func (c RotatedSelfSignedCertKeySecret) EnsureTargetCertKeyPair(ctx context.Cont
126119

127120
if reason := c.CertCreator.NeedNewTargetCertKeyPair(targetCertKeyPairSecret.Annotations, signingCertKeyPair, caBundleCerts, c.Refresh, c.RefreshOnlyWhenExpired); len(reason) > 0 {
128121
c.EventRecorder.Eventf("TargetUpdateRequired", "%q in %q requires a new target cert/key pair: %v", c.Name, c.Namespace, reason)
129-
if err := setTargetCertKeyPairSecret(targetCertKeyPairSecret, c.Validity, signingCertKeyPair, c.CertCreator, c.JiraComponent, c.Description); err != nil {
122+
if err := setTargetCertKeyPairSecret(targetCertKeyPairSecret, c.Validity, signingCertKeyPair, c.CertCreator, c.AdditionalAnnotations); err != nil {
130123
return nil, err
131124
}
132125

@@ -217,7 +210,7 @@ func needNewTargetCertKeyPairForTime(annotations map[string]string, signer *cryp
217210

218211
// setTargetCertKeyPairSecret creates a new cert/key pair and sets them in the secret. Only one of client, serving, or signer rotation may be specified.
219212
// TODO refactor with an interface for actually signing and move the one-of check higher in the stack.
220-
func setTargetCertKeyPairSecret(targetCertKeyPairSecret *corev1.Secret, validity time.Duration, signer *crypto.CA, certCreator TargetCertCreator, jiraComponent, description string) error {
213+
func setTargetCertKeyPairSecret(targetCertKeyPairSecret *corev1.Secret, validity time.Duration, signer *crypto.CA, certCreator TargetCertCreator, annotations AdditionalAnnotations) error {
221214
if targetCertKeyPairSecret.Annotations == nil {
222215
targetCertKeyPairSecret.Annotations = map[string]string{}
223216
}
@@ -244,12 +237,8 @@ func setTargetCertKeyPairSecret(targetCertKeyPairSecret *corev1.Secret, validity
244237
targetCertKeyPairSecret.Annotations[CertificateNotAfterAnnotation] = certKeyPair.Certs[0].NotAfter.Format(time.RFC3339)
245238
targetCertKeyPairSecret.Annotations[CertificateNotBeforeAnnotation] = certKeyPair.Certs[0].NotBefore.Format(time.RFC3339)
246239
targetCertKeyPairSecret.Annotations[CertificateIssuer] = certKeyPair.Certs[0].Issuer.CommonName
247-
if len(jiraComponent) > 0 {
248-
targetCertKeyPairSecret.Annotations[annotations.OpenShiftComponent] = jiraComponent
249-
}
250-
if len(description) > 0 {
251-
targetCertKeyPairSecret.Annotations[annotations.OpenShiftDescription] = description
252-
}
240+
241+
_ = annotations.EnsureTLSMetadataUpdate(&targetCertKeyPairSecret.ObjectMeta)
253242
certCreator.SetAnnotations(certKeyPair, targetCertKeyPairSecret.Annotations)
254243

255244
return nil

‎pkg/operator/csr/cert_controller.go

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,8 @@ type ClientCertOption struct {
6464
SecretName string
6565
// AdditonalSecretData contains data that will be added into client certificate secret besides tls.key/tls.crt
6666
AdditonalSecretData map[string][]byte
67-
// JiraComponent annotates tls artifacts so that owner could be easily found
68-
JiraComponent string
69-
// Description is a human-readable one sentence description of certificate purpose
70-
Description string
67+
// AdditionalAnnotations is a collection of annotations set for the secret
68+
AdditionalAnnotations certrotation.AdditionalAnnotations
7169
}
7270

7371
// 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.
154152
ObjectMeta: certrotation.NewTLSArtifactObjectMeta(
155153
c.SecretName,
156154
c.SecretNamespace,
157-
c.JiraComponent,
158-
c.Description,
155+
c.AdditionalAnnotations,
159156
),
160157
}
161158
case err != nil:
162159
return fmt.Errorf("unable to get secret %q: %w", c.SecretNamespace+"/"+c.SecretName, err)
163160
}
164161

165-
needsMetadataUpdate := false
166-
if len(c.JiraComponent) > 0 || len(c.Description) > 0 {
167-
needsMetadataUpdate = certrotation.EnsureTLSMetadataUpdate(&secret.ObjectMeta, c.JiraComponent, c.Description)
168-
}
162+
needsMetadataUpdate := c.AdditionalAnnotations.EnsureTLSMetadataUpdate(&secret.ObjectMeta)
169163

170164
// reconcile pending csr if exists
171165
if len(c.csrName) > 0 {

‎pkg/operator/resourcesynccontroller/core.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import (
1414
"github.com/openshift/library-go/pkg/operator/certrotation"
1515
)
1616

17-
func CombineCABundleConfigMaps(destinationConfigMap ResourceLocation, lister corev1listers.ConfigMapLister, jiraComponent, description string, inputConfigMaps ...ResourceLocation) (*corev1.ConfigMap, error) {
17+
func CombineCABundleConfigMaps(destinationConfigMap ResourceLocation, lister corev1listers.ConfigMapLister, additionalAnnotations certrotation.AdditionalAnnotations, inputConfigMaps ...ResourceLocation) (*corev1.ConfigMap, error) {
1818
certificates := []*x509.Certificate{}
1919
for _, input := range inputConfigMaps {
2020
inputConfigMap, err := lister.ConfigMaps(input.Namespace).Get(input.Name)
@@ -62,8 +62,7 @@ func CombineCABundleConfigMaps(destinationConfigMap ResourceLocation, lister cor
6262
ObjectMeta: certrotation.NewTLSArtifactObjectMeta(
6363
destinationConfigMap.Name,
6464
destinationConfigMap.Namespace,
65-
jiraComponent,
66-
description,
65+
additionalAnnotations,
6766
),
6867
Data: map[string]string{
6968
"ca-bundle.crt": string(caBytes),

0 commit comments

Comments
 (0)