From ed9889263c431a9907663bc8a7a076d18670de90 Mon Sep 17 00:00:00 2001 From: Luis Sanchez Date: Thu, 2 Apr 2026 22:04:59 -0400 Subject: [PATCH 1/2] certrotation: restructure controller to own plumbing and cert creation Replace hybrid data+behavior structs with pure config types: - RotatedSigningCASecret -> SigningCAConfig - CABundleConfigMap -> CABundleConfig - RotatedSelfSignedCertKeySecret -> TargetCertKeyPairConfig Flatten TargetCertCreator interface into sealed TargetCertConfig with typed variants (ClientCertConfig, ServingCertConfig, SignerCertConfig). The controller type-switches on the variant to create certs. Move all Ensure/set/needNew methods from data structs to CertRotationController. The controller now takes kubernetes.Interface and KubeInformersForNamespaces, deriving informers and listers from the config struct Namespace+Name fields. This eliminates the per-struct Informer/Lister/Client/EventRecorder plumbing and removes the TargetCertCreator, TargetCertRechecker, and KeyPairGeneratorConfigurable interfaces along with the ClientRotation, ServingRotation, and SignerRotation types. --- pkg/operator/certrotation/cabundle.go | 45 ++- pkg/operator/certrotation/cabundle_test.go | 34 +- .../client_cert_rotation_controller.go | 81 +++-- .../client_cert_rotation_controller_test.go | 90 +++--- pkg/operator/certrotation/signer.go | 61 ++-- pkg/operator/certrotation/signer_test.go | 47 +-- pkg/operator/certrotation/target.go | 305 +++++++----------- pkg/operator/certrotation/target_test.go | 97 +++--- 8 files changed, 349 insertions(+), 411 deletions(-) diff --git a/pkg/operator/certrotation/cabundle.go b/pkg/operator/certrotation/cabundle.go index 447b1e0e31..ba4e304671 100644 --- a/pkg/operator/certrotation/cabundle.go +++ b/pkg/operator/certrotation/cabundle.go @@ -12,20 +12,16 @@ import ( "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - corev1informers "k8s.io/client-go/informers/core/v1" - corev1client "k8s.io/client-go/kubernetes/typed/core/v1" - corev1listers "k8s.io/client-go/listers/core/v1" "k8s.io/client-go/util/cert" "k8s.io/klog/v2" "github.com/openshift/library-go/pkg/certs" "github.com/openshift/library-go/pkg/crypto" - "github.com/openshift/library-go/pkg/operator/events" "github.com/openshift/library-go/pkg/operator/resource/resourcehelper" ) -// CABundleConfigMap maintains a CA bundle config map, by adding new CA certs coming from RotatedSigningCASecret, and by removing expired old ones. -type CABundleConfigMap struct { +// CABundleConfig holds the configuration for a CA bundle config map, by adding new CA certs coming from SigningCAConfig, and by removing expired old ones. +type CABundleConfig struct { // Namespace is the namespace of the ConfigMap to maintain. Namespace string // Name is the name of the ConfigMap to maintain. @@ -38,20 +34,17 @@ type CABundleConfigMap struct { Owner *metav1.OwnerReference // AdditionalAnnotations is a collection of annotations set for the secret AdditionalAnnotations AdditionalAnnotations - // Plumbing: - Informer corev1informers.ConfigMapInformer - Lister corev1listers.ConfigMapLister - Client corev1client.ConfigMapsGetter - EventRecorder events.Recorder } -func (c CABundleConfigMap) EnsureConfigMapCABundle(ctx context.Context, signingCertKeyPair *crypto.CA, signingCertKeyPairLocation string) ([]*x509.Certificate, error) { +func (c CertRotationController) ensureConfigMapCABundle(ctx context.Context, signingCertKeyPair *crypto.CA) ([]*x509.Certificate, error) { + signingCertKeyPairLocation := c.getSigningCertKeyPairLocation() + // by this point we have current signing cert/key pair. We now need to make sure that the ca-bundle configmap has this cert and // doesn't have any expired certs updateRequired := false creationRequired := false - originalCABundleConfigMap, err := c.Lister.ConfigMaps(c.Namespace).Get(c.Name) + originalCABundleConfigMap, err := c.kubeInformers.ConfigMapLister().ConfigMaps(c.CABundle.Namespace).Get(c.CABundle.Name) if err != nil && !apierrors.IsNotFound(err) { return nil, err } @@ -59,20 +52,20 @@ func (c CABundleConfigMap) EnsureConfigMapCABundle(ctx context.Context, signingC if apierrors.IsNotFound(err) { // create an empty one caBundleConfigMap = &corev1.ConfigMap{ObjectMeta: NewTLSArtifactObjectMeta( - c.Name, - c.Namespace, - c.AdditionalAnnotations, + c.CABundle.Name, + c.CABundle.Namespace, + c.CABundle.AdditionalAnnotations, )} creationRequired = true } // run Update if metadata needs changing unless running in RefreshOnlyWhenExpired mode - if !c.RefreshOnlyWhenExpired { + if !c.CABundle.RefreshOnlyWhenExpired { needsOwnerUpdate := false - if c.Owner != nil { - needsOwnerUpdate = ensureOwnerReference(&caBundleConfigMap.ObjectMeta, c.Owner) + if c.CABundle.Owner != nil { + needsOwnerUpdate = ensureOwnerReference(&caBundleConfigMap.ObjectMeta, c.CABundle.Owner) } - needsMetadataUpdate := c.AdditionalAnnotations.EnsureTLSMetadataUpdate(&caBundleConfigMap.ObjectMeta) + needsMetadataUpdate := c.CABundle.AdditionalAnnotations.EnsureTLSMetadataUpdate(&caBundleConfigMap.ObjectMeta) updateRequired = needsOwnerUpdate || needsMetadataUpdate } @@ -89,27 +82,27 @@ func (c CABundleConfigMap) EnsureConfigMapCABundle(ctx context.Context, signingC } else if !equality.Semantic.DeepEqual(originalCABundleConfigMap.Data, caBundleConfigMap.Data) { reason = fmt.Sprintf("signer update %s", signingCertKeyPairLocation) } - c.EventRecorder.Eventf("CABundleUpdateRequired", "%q in %q requires a new cert: %s", c.Name, c.Namespace, reason) + c.eventRecorder.Eventf("CABundleUpdateRequired", "%q in %q requires a new cert: %s", c.CABundle.Name, c.CABundle.Namespace, reason) LabelAsManagedConfigMap(caBundleConfigMap, CertificateTypeCABundle) updateRequired = true } if creationRequired { - actualCABundleConfigMap, err := c.Client.ConfigMaps(c.Namespace).Create(ctx, caBundleConfigMap, metav1.CreateOptions{}) - resourcehelper.ReportCreateEvent(c.EventRecorder, actualCABundleConfigMap, err) + actualCABundleConfigMap, err := c.kubeClient.CoreV1().ConfigMaps(c.CABundle.Namespace).Create(ctx, caBundleConfigMap, metav1.CreateOptions{}) + resourcehelper.ReportCreateEvent(c.eventRecorder, actualCABundleConfigMap, err) if err != nil { return nil, err } klog.V(2).Infof("Created ca-bundle.crt configmap %s/%s with:\n%s", certs.CertificateBundleToString(updatedCerts), caBundleConfigMap.Namespace, caBundleConfigMap.Name) caBundleConfigMap = actualCABundleConfigMap } else if updateRequired { - actualCABundleConfigMap, err := c.Client.ConfigMaps(c.Namespace).Update(ctx, caBundleConfigMap, metav1.UpdateOptions{}) + actualCABundleConfigMap, err := c.kubeClient.CoreV1().ConfigMaps(c.CABundle.Namespace).Update(ctx, caBundleConfigMap, metav1.UpdateOptions{}) if apierrors.IsConflict(err) { // ignore error if its attempting to update outdated version of the configmap return nil, nil } - resourcehelper.ReportUpdateEvent(c.EventRecorder, actualCABundleConfigMap, err) + resourcehelper.ReportUpdateEvent(c.eventRecorder, actualCABundleConfigMap, err) if err != nil { return nil, err } @@ -175,4 +168,4 @@ func manageCABundleConfigMap(caBundleConfigMap *corev1.ConfigMap, currentSigner caBundleConfigMap.Data["ca-bundle.crt"] = string(caBytes) return finalCertificates, nil -} +} \ No newline at end of file diff --git a/pkg/operator/certrotation/cabundle_test.go b/pkg/operator/certrotation/cabundle_test.go index cf15bafff1..e174704ac9 100644 --- a/pkg/operator/certrotation/cabundle_test.go +++ b/pkg/operator/certrotation/cabundle_test.go @@ -20,13 +20,13 @@ import ( "github.com/openshift/library-go/pkg/crypto" "github.com/openshift/library-go/pkg/operator/events" + "github.com/openshift/library-go/pkg/operator/v1helpers" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/informers" kubefake "k8s.io/client-go/kubernetes/fake" - corev1listers "k8s.io/client-go/listers/core/v1" clienttesting "k8s.io/client-go/testing" - "k8s.io/client-go/tools/cache" ) func TestEnsureConfigMapCABundle(t *testing.T) { @@ -195,28 +195,38 @@ func TestEnsureConfigMapCABundle(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc}) - client := kubefake.NewSimpleClientset() + informerFactory := informers.NewSharedInformerFactory(client, 0) if startingObj := test.initialConfigMapFn(); startingObj != nil { - indexer.Add(startingObj) client = kubefake.NewSimpleClientset(startingObj) + informerFactory = informers.NewSharedInformerFactory(client, 0) + informerFactory.Core().V1().ConfigMaps().Informer().GetStore().Add(startingObj) } - c := &CABundleConfigMap{ - Namespace: "ns", - Name: "trust-bundle", + kubeInformers := v1helpers.NewFakeKubeInformersForNamespaces(map[string]informers.SharedInformerFactory{ + "ns": informerFactory, + }) - Client: client.CoreV1(), - Lister: corev1listers.NewConfigMapLister(indexer), - EventRecorder: events.NewInMemoryRecorder("test", clocktesting.NewFakePassiveClock(time.Now())), + c := &CertRotationController{ + Name: "test-cabundle", + SigningCA: SigningCAConfig{ + Namespace: "ns", + Name: "signer-secret", + }, + CABundle: CABundleConfig{ + Namespace: "ns", + Name: "trust-bundle", + }, + kubeClient: client, + kubeInformers: kubeInformers, + eventRecorder: events.NewInMemoryRecorder("test", clocktesting.NewFakePassiveClock(time.Now())), } newCA, err := test.caFn() if err != nil { t.Fatal(err) } - _, err = c.EnsureConfigMapCABundle(context.TODO(), newCA, "signer-secret") + _, err = c.ensureConfigMapCABundle(context.TODO(), newCA) switch { case err != nil && len(test.expectedError) == 0: t.Error(err) diff --git a/pkg/operator/certrotation/client_cert_rotation_controller.go b/pkg/operator/certrotation/client_cert_rotation_controller.go index f9ad1fc14b..ab007b8ca4 100644 --- a/pkg/operator/certrotation/client_cert_rotation_controller.go +++ b/pkg/operator/certrotation/client_cert_rotation_controller.go @@ -8,6 +8,7 @@ import ( operatorv1 "github.com/openshift/api/operator/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/kubernetes" "github.com/openshift/library-go/pkg/controller/factory" "github.com/openshift/library-go/pkg/operator/condition" @@ -48,60 +49,74 @@ func (s *StaticPodConditionStatusReporter) Report(ctx context.Context, controlle // CertRotationController does: // -// 1) continuously create a self-signed signing CA (via RotatedSigningCASecret) and store it in a secret. +// 1) continuously create a self-signed signing CA (via SigningCAConfig) and store it in a secret. // 2) maintain a CA bundle ConfigMap with all not yet expired CA certs. // 3) continuously create a target cert and key signed by the latest signing CA and store it in a secret. type CertRotationController struct { - // controller name + // Name is the controller name. Name string - // RotatedSigningCASecret rotates a self-signed signing CA stored in a secret. - RotatedSigningCASecret RotatedSigningCASecret - // CABundleConfigMap maintains a CA bundle config map, by adding new CA certs coming from rotatedSigningCASecret, and by removing expired old ones. - CABundleConfigMap CABundleConfigMap - // RotatedSelfSignedCertKeySecret rotates a key and cert signed by a signing CA and stores it in a secret. - RotatedSelfSignedCertKeySecret RotatedSelfSignedCertKeySecret - - // Plumbing: + // SigningCA holds the configuration for the signing CA secret. + SigningCA SigningCAConfig + // CABundle holds the configuration for the CA bundle config map. + CABundle CABundleConfig + // TargetCert holds the configuration for the target certificate secret. + TargetCert TargetCertKeyPairConfig + + // StatusReporter reports the status of cert rotation. StatusReporter StatusReporter + + kubeClient kubernetes.Interface + kubeInformers v1helpers.KubeInformersForNamespaces + eventRecorder events.Recorder } func NewCertRotationController( name string, - rotatedSigningCASecret RotatedSigningCASecret, - caBundleConfigMap CABundleConfigMap, - rotatedSelfSignedCertKeySecret RotatedSelfSignedCertKeySecret, + signingCA SigningCAConfig, + caBundle CABundleConfig, + targetCert TargetCertKeyPairConfig, + kubeClient kubernetes.Interface, + kubeInformers v1helpers.KubeInformersForNamespaces, recorder events.Recorder, reporter StatusReporter, ) factory.Controller { c := &CertRotationController{ - Name: name, - RotatedSigningCASecret: rotatedSigningCASecret, - CABundleConfigMap: caBundleConfigMap, - RotatedSelfSignedCertKeySecret: rotatedSelfSignedCertKeySecret, - StatusReporter: reporter, + Name: name, + SigningCA: signingCA, + CABundle: caBundle, + TargetCert: targetCert, + StatusReporter: reporter, + kubeClient: kubeClient, + kubeInformers: kubeInformers, + eventRecorder: recorder, } + + signerSecretInformer := kubeInformers.InformersFor(signingCA.Namespace).Core().V1().Secrets() + caBundleConfigMapInformer := kubeInformers.InformersFor(caBundle.Namespace).Core().V1().ConfigMaps() + targetSecretInformer := kubeInformers.InformersFor(targetCert.Namespace).Core().V1().Secrets() + return factory.New(). ResyncEvery(time.Minute). WithSync(c.Sync). WithFilteredEventsInformers( func(obj interface{}) bool { if cm, ok := obj.(*corev1.ConfigMap); ok { - return cm.Namespace == caBundleConfigMap.Namespace && cm.Name == caBundleConfigMap.Name + return cm.Namespace == caBundle.Namespace && cm.Name == caBundle.Name } if secret, ok := obj.(*corev1.Secret); ok { - if secret.Namespace == rotatedSigningCASecret.Namespace && secret.Name == rotatedSigningCASecret.Name { + if secret.Namespace == signingCA.Namespace && secret.Name == signingCA.Name { return true } - if secret.Namespace == rotatedSelfSignedCertKeySecret.Namespace && secret.Name == rotatedSelfSignedCertKeySecret.Name { + if secret.Namespace == targetCert.Namespace && secret.Name == targetCert.Name { return true } return false } return true }, - rotatedSigningCASecret.Informer.Informer(), - caBundleConfigMap.Informer.Informer(), - rotatedSelfSignedCertKeySecret.Informer.Informer(), + signerSecretInformer.Informer(), + caBundleConfigMapInformer.Informer(), + targetSecretInformer.Informer(), ). WithPostStartHooks( c.targetCertRecheckerPostRunHook, @@ -133,11 +148,11 @@ func (c CertRotationController) Sync(ctx context.Context, syncCtx factory.SyncCo } func (c CertRotationController) getSigningCertKeyPairLocation() string { - return fmt.Sprintf("%s/%s", c.RotatedSelfSignedCertKeySecret.Namespace, c.RotatedSelfSignedCertKeySecret.Name) + return fmt.Sprintf("%s/%s", c.SigningCA.Namespace, c.SigningCA.Name) } func (c CertRotationController) SyncWorker(ctx context.Context) error { - signingCertKeyPair, _, err := c.RotatedSigningCASecret.EnsureSigningCertKeyPair(ctx) + signingCertKeyPair, _, err := c.ensureSigningCertKeyPair(ctx) if err != nil { return err } @@ -146,7 +161,7 @@ func (c CertRotationController) SyncWorker(ctx context.Context) error { return fmt.Errorf("signingCertKeyPair is nil") } - cabundleCerts, err := c.CABundleConfigMap.EnsureConfigMapCABundle(ctx, signingCertKeyPair, c.getSigningCertKeyPairLocation()) + cabundleCerts, err := c.ensureConfigMapCABundle(ctx, signingCertKeyPair) if err != nil { return err } @@ -155,7 +170,7 @@ func (c CertRotationController) SyncWorker(ctx context.Context) error { return fmt.Errorf("cabundleCerts is nil") } - if _, err := c.RotatedSelfSignedCertKeySecret.EnsureTargetCertKeyPair(ctx, signingCertKeyPair, cabundleCerts); err != nil { + if _, err := c.ensureTargetCertKeyPair(ctx, signingCertKeyPair, cabundleCerts); err != nil { return err } @@ -163,16 +178,14 @@ func (c CertRotationController) SyncWorker(ctx context.Context) error { } func (c CertRotationController) targetCertRecheckerPostRunHook(ctx context.Context, syncCtx factory.SyncContext) error { - // If we have a need to force rechecking the cert, use this channel to do it. - refresher, ok := c.RotatedSelfSignedCertKeySecret.CertCreator.(TargetCertRechecker) - if !ok { + serving, ok := c.TargetCert.CertConfig.(ServingCertConfig) + if !ok || serving.HostnamesChanged == nil { return nil } - targetRefresh := refresher.RecheckChannel() go wait.Until(func() { for { select { - case <-targetRefresh: + case <-serving.HostnamesChanged: syncCtx.Queue().Add(factory.DefaultQueueKey) case <-ctx.Done(): return @@ -182,4 +195,4 @@ func (c CertRotationController) targetCertRecheckerPostRunHook(ctx context.Conte <-ctx.Done() return nil -} +} \ No newline at end of file diff --git a/pkg/operator/certrotation/client_cert_rotation_controller_test.go b/pkg/operator/certrotation/client_cert_rotation_controller_test.go index 38f5e947ce..6ea6b52c53 100644 --- a/pkg/operator/certrotation/client_cert_rotation_controller_test.go +++ b/pkg/operator/certrotation/client_cert_rotation_controller_test.go @@ -2,7 +2,6 @@ package certrotation import ( "context" - "crypto/x509" "fmt" "strings" "testing" @@ -11,6 +10,7 @@ import ( "github.com/openshift/api/annotations" "github.com/openshift/library-go/pkg/crypto" "github.com/openshift/library-go/pkg/operator/events" + "github.com/openshift/library-go/pkg/operator/v1helpers" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -36,7 +36,7 @@ func TestCertRotationController_SyncWorker(t *testing.T) { SerialGenerator: &crypto.RandomSerialGenerator{}, } // Create target certificate signed by the CA - targetCert, err := ca.MakeServerCert(sets.New("test.example.com", "localhost"), time.Hour*12) + targetCert, err := ca.MakeServerCert(sets.New("test-cert", "localhost"), time.Hour*12) if err != nil { panic(fmt.Sprintf("failed to create server cert: %v", err)) } @@ -260,42 +260,39 @@ func TestCertRotationController_SyncWorker(t *testing.T) { } } + kubeInformers := v1helpers.NewFakeKubeInformersForNamespaces(map[string]informers.SharedInformerFactory{ + "test-namespace": informerFactory, + }) + // Create the controller controller := &CertRotationController{ Name: "test-controller", - RotatedSigningCASecret: RotatedSigningCASecret{ - Namespace: "test-namespace", - Name: "test-signer-cert", - Validity: 24 * time.Hour, - Refresh: 12 * time.Hour, - Informer: informerFactory.Core().V1().Secrets(), - Lister: informerFactory.Core().V1().Secrets().Lister(), - Client: fakeClient.CoreV1(), - EventRecorder: events.NewInMemoryRecorder("test", clock.RealClock{}), + SigningCA: SigningCAConfig{ + Namespace: "test-namespace", + Name: "test-signer-cert", + Validity: 24 * time.Hour, + Refresh: 12 * time.Hour, }, - CABundleConfigMap: CABundleConfigMap{ - Namespace: "test-namespace", - Name: "test-ca-bundle", - Informer: informerFactory.Core().V1().ConfigMaps(), - Lister: informerFactory.Core().V1().ConfigMaps().Lister(), - Client: fakeClient.CoreV1(), - EventRecorder: events.NewInMemoryRecorder("test", clock.RealClock{}), + CABundle: CABundleConfig{ + Namespace: "test-namespace", + Name: "test-ca-bundle", AdditionalAnnotations: AdditionalAnnotations{ JiraComponent: "test", }, }, - RotatedSelfSignedCertKeySecret: RotatedSelfSignedCertKeySecret{ - Namespace: "test-namespace", - Name: "test-target-cert", - Validity: 24 * time.Hour, - Refresh: 12 * time.Hour, - CertCreator: &mockTargetCertCreator{}, - Informer: informerFactory.Core().V1().Secrets(), - Lister: informerFactory.Core().V1().Secrets().Lister(), - Client: fakeClient.CoreV1(), - EventRecorder: events.NewInMemoryRecorder("test", clock.RealClock{}), + TargetCert: TargetCertKeyPairConfig{ + Namespace: "test-namespace", + Name: "test-target-cert", + Validity: 24 * time.Hour, + Refresh: 12 * time.Hour, + CertConfig: ServingCertConfig{ + Hostnames: func() []string { return []string{"test-cert", "localhost"} }, + }, }, StatusReporter: &testStatusReporter{}, + kubeClient: fakeClient, + kubeInformers: kubeInformers, + eventRecorder: events.NewInMemoryRecorder("test", clock.RealClock{}), } // Run SyncWorker @@ -329,32 +326,6 @@ func (t *testStatusReporter) Report(ctx context.Context, controllerName string, return false, nil } -// mockTargetCertCreator is a mock implementation of TargetCertCreator for testing -type mockTargetCertCreator struct{} - -func (m *mockTargetCertCreator) NewCertificate(signer *crypto.CA, validity time.Duration) (*crypto.TLSCertificateConfig, error) { - // Use the provided signer to create a real certificate with matching key - certConfig, err := signer.MakeServerCert(sets.New("test-cert", "localhost"), validity) - if err != nil { - return nil, fmt.Errorf("failed to create server certificate: %v", err) - } - - return certConfig, nil -} - -func (m *mockTargetCertCreator) NeedNewTargetCertKeyPair(currentCertSecret *corev1.Secret, signer *crypto.CA, caBundleCerts []*x509.Certificate, refresh time.Duration, refreshOnlyWhenExpired, creationRequired bool) string { - if creationRequired { - return "creation required" - } - // For testing, we don't need rotation unless explicitly required - return "" -} - -func (m *mockTargetCertCreator) SetAnnotations(cert *crypto.TLSCertificateConfig, annotations map[string]string) map[string]string { - // Just return the annotations as-is for testing - return annotations -} - func createValidSigningCASecret(namespace, name string, signer *crypto.TLSCertificateConfig) *corev1.Secret { certBytes, keyBytes, err := signer.GetPEMBytes() if err != nil { @@ -463,6 +434,15 @@ func createValidTargetCertSecret(namespace, name string, serverCert *crypto.TLSC panic(fmt.Sprintf("failed to get PEM bytes: %v", err)) } + // Build hostnames from the certificate's DNS names and IP addresses + hostnameSet := sets.New[string]() + for _, dnsName := range serverCert.Certs[0].DNSNames { + hostnameSet.Insert(dnsName) + } + for _, ip := range serverCert.Certs[0].IPAddresses { + hostnameSet.Insert(ip.String()) + } + return &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Namespace: namespace, @@ -470,6 +450,8 @@ func createValidTargetCertSecret(namespace, name string, serverCert *crypto.TLSC Annotations: map[string]string{ "auth.openshift.io/certificate-not-after": time.Now().Add(24 * time.Hour).Format(time.RFC3339), "auth.openshift.io/certificate-not-before": time.Now().Add(-1 * time.Hour).Format(time.RFC3339), + "auth.openshift.io/certificate-issuer": serverCert.Certs[0].Issuer.CommonName, + "auth.openshift.io/certificate-hostnames": strings.Join(sets.List(hostnameSet), ","), }, }, Type: corev1.SecretTypeTLS, diff --git a/pkg/operator/certrotation/signer.go b/pkg/operator/certrotation/signer.go index c2c8b8368f..b09a10e9b6 100644 --- a/pkg/operator/certrotation/signer.go +++ b/pkg/operator/certrotation/signer.go @@ -7,22 +7,18 @@ import ( "time" "github.com/openshift/library-go/pkg/crypto" - "github.com/openshift/library-go/pkg/operator/events" "github.com/openshift/library-go/pkg/operator/resource/resourcehelper" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - corev1informers "k8s.io/client-go/informers/core/v1" - corev1client "k8s.io/client-go/kubernetes/typed/core/v1" - corev1listers "k8s.io/client-go/listers/core/v1" "k8s.io/klog/v2" ) -// RotatedSigningCASecret rotates a self-signed signing CA stored in a secret. It creates a new one when +// SigningCAConfig holds the configuration for a self-signed signing CA stored in a secret. It creates a new one when // - refresh duration is over // - or 80% of validity is over (if RefreshOnlyWhenExpired is false) // - or the CA is expired. -type RotatedSigningCASecret struct { +type SigningCAConfig struct { // Namespace is the namespace of the Secret. Namespace string // Name is the name of the Secret. @@ -47,20 +43,14 @@ type RotatedSigningCASecret struct { // AdditionalAnnotations is a collection of annotations set for the secret AdditionalAnnotations AdditionalAnnotations - - // Plumbing: - Informer corev1informers.SecretInformer - Lister corev1listers.SecretLister - Client corev1client.SecretsGetter - EventRecorder events.Recorder } -// EnsureSigningCertKeyPair manages the entire lifecycle of a signer cert as a secret, from creation to continued rotation. +// ensureSigningCertKeyPair manages the entire lifecycle of a signer cert as a secret, from creation to continued rotation. // It always returns the currently used CA pair, a bool indicating whether it was created/updated within this function call and an error. -func (c RotatedSigningCASecret) EnsureSigningCertKeyPair(ctx context.Context) (*crypto.CA, bool, error) { +func (c CertRotationController) ensureSigningCertKeyPair(ctx context.Context) (*crypto.CA, bool, error) { creationRequired := false updateRequired := false - originalSigningCertKeyPairSecret, err := c.Lister.Secrets(c.Namespace).Get(c.Name) + originalSigningCertKeyPairSecret, err := c.kubeInformers.SecretLister().Secrets(c.SigningCA.Namespace).Get(c.SigningCA.Name) if err != nil && !apierrors.IsNotFound(err) { return nil, false, err } @@ -69,9 +59,9 @@ func (c RotatedSigningCASecret) EnsureSigningCertKeyPair(ctx context.Context) (* // create an empty one signingCertKeyPairSecret = &corev1.Secret{ ObjectMeta: NewTLSArtifactObjectMeta( - c.Name, - c.Namespace, - c.AdditionalAnnotations, + c.SigningCA.Name, + c.SigningCA.Namespace, + c.SigningCA.AdditionalAnnotations, ), Type: corev1.SecretTypeTLS, } @@ -79,22 +69,25 @@ func (c RotatedSigningCASecret) EnsureSigningCertKeyPair(ctx context.Context) (* } // run Update if metadata needs changing unless we're in RefreshOnlyWhenExpired mode - if !c.RefreshOnlyWhenExpired { - needsMetadataUpdate := ensureOwnerRefAndTLSAnnotations(signingCertKeyPairSecret, c.Owner, c.AdditionalAnnotations) + if !c.SigningCA.RefreshOnlyWhenExpired { + needsMetadataUpdate := ensureOwnerRefAndTLSAnnotations(signingCertKeyPairSecret, c.SigningCA.Owner, c.SigningCA.AdditionalAnnotations) needsTypeChange := ensureSecretTLSTypeSet(signingCertKeyPairSecret) updateRequired = needsMetadataUpdate || needsTypeChange } // run Update if signer content needs changing signerUpdated := false - if needed, reason := needNewSigningCertKeyPair(signingCertKeyPairSecret, c.Refresh, c.RefreshOnlyWhenExpired); needed || creationRequired { + if needed, reason := needNewSigningCertKeyPair(signingCertKeyPairSecret, c.SigningCA.Refresh, c.SigningCA.RefreshOnlyWhenExpired); needed || creationRequired { if creationRequired { 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 = setSigningCertKeyPairSecretAndTLSAnnotations(signingCertKeyPairSecret, c.Validity, c.Refresh, c.AdditionalAnnotations); err != nil { + c.eventRecorder.Eventf("SignerUpdateRequired", "%q in %q requires a new signing cert/key pair: %v", c.SigningCA.Name, c.SigningCA.Namespace, reason) + + ca, err := setSigningCertKeyPairSecret(signingCertKeyPairSecret, c.SigningCA.Validity) + if err != nil { return nil, false, err } + setTLSAnnotationsOnSigningCertKeyPairSecret(signingCertKeyPairSecret, ca, c.SigningCA.Refresh, c.SigningCA.AdditionalAnnotations) LabelAsManagedSecret(signingCertKeyPairSecret, CertificateTypeSigner) @@ -103,20 +96,20 @@ func (c RotatedSigningCASecret) EnsureSigningCertKeyPair(ctx context.Context) (* } if creationRequired { - actualSigningCertKeyPairSecret, err := c.Client.Secrets(c.Namespace).Create(ctx, signingCertKeyPairSecret, metav1.CreateOptions{}) - resourcehelper.ReportCreateEvent(c.EventRecorder, actualSigningCertKeyPairSecret, err) + actualSigningCertKeyPairSecret, err := c.kubeClient.CoreV1().Secrets(c.SigningCA.Namespace).Create(ctx, signingCertKeyPairSecret, metav1.CreateOptions{}) + resourcehelper.ReportCreateEvent(c.eventRecorder, actualSigningCertKeyPairSecret, err) if err != nil { return nil, false, err } klog.V(2).Infof("Created secret %s/%s", actualSigningCertKeyPairSecret.Namespace, actualSigningCertKeyPairSecret.Name) signingCertKeyPairSecret = actualSigningCertKeyPairSecret } else if updateRequired { - actualSigningCertKeyPairSecret, err := c.Client.Secrets(c.Namespace).Update(ctx, signingCertKeyPairSecret, metav1.UpdateOptions{}) + actualSigningCertKeyPairSecret, err := c.kubeClient.CoreV1().Secrets(c.SigningCA.Namespace).Update(ctx, signingCertKeyPairSecret, metav1.UpdateOptions{}) if apierrors.IsConflict(err) { // ignore error if its attempting to update outdated version of the secret return nil, false, nil } - resourcehelper.ReportUpdateEvent(c.EventRecorder, actualSigningCertKeyPairSecret, err) + resourcehelper.ReportUpdateEvent(c.eventRecorder, actualSigningCertKeyPairSecret, err) if err != nil { return nil, false, err } @@ -199,18 +192,6 @@ 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, refresh time.Duration, tlsAnnotations AdditionalAnnotations) error { - ca, err := setSigningCertKeyPairSecret(signingCertKeyPairSecret, validity) - if err != nil { - return err - } - - setTLSAnnotationsOnSigningCertKeyPairSecret(signingCertKeyPairSecret, ca, refresh, 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) (*crypto.TLSCertificateConfig, error) { signerName := fmt.Sprintf("%s_%s@%d", signingCertKeyPairSecret.Namespace, signingCertKeyPairSecret.Name, time.Now().Unix()) @@ -250,4 +231,4 @@ func setTLSAnnotationsOnSigningCertKeyPairSecret(signingCertKeyPairSecret *corev tlsAnnotations.NotAfter = ca.Certs[0].NotAfter.Format(time.RFC3339) tlsAnnotations.RefreshPeriod = refresh.String() _ = tlsAnnotations.EnsureTLSMetadataUpdate(&signingCertKeyPairSecret.ObjectMeta) -} +} \ No newline at end of file diff --git a/pkg/operator/certrotation/signer_test.go b/pkg/operator/certrotation/signer_test.go index 66c9c5a289..82b4ee9683 100644 --- a/pkg/operator/certrotation/signer_test.go +++ b/pkg/operator/certrotation/signer_test.go @@ -14,14 +14,14 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/client-go/informers" kubefake "k8s.io/client-go/kubernetes/fake" - corev1listers "k8s.io/client-go/listers/core/v1" clienttesting "k8s.io/client-go/testing" - "k8s.io/client-go/tools/cache" clocktesting "k8s.io/utils/clock/testing" "github.com/openshift/api/annotations" "github.com/openshift/library-go/pkg/operator/events" + "github.com/openshift/library-go/pkg/operator/v1helpers" ) func TestEnsureSigningCertKeyPair(t *testing.T) { @@ -242,34 +242,41 @@ func TestEnsureSigningCertKeyPair(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc}) - client := kubefake.NewClientset() + informerFactory := informers.NewSharedInformerFactory(client, 0) if test.initialSecret != nil { - indexer.Add(test.initialSecret) client = kubefake.NewClientset(test.initialSecret) + informerFactory = informers.NewSharedInformerFactory(client, 0) + informerFactory.Core().V1().Secrets().Informer().GetStore().Add(test.initialSecret) } + kubeInformers := v1helpers.NewFakeKubeInformersForNamespaces(map[string]informers.SharedInformerFactory{ + "ns": informerFactory, + }) + recorder := events.NewInMemoryRecorder("test", clocktesting.NewFakePassiveClock(time.Now())) - c := &RotatedSigningCASecret{ - Namespace: "ns", - Name: "signer", - Validity: 24 * time.Hour, - Refresh: 12 * time.Hour, - Client: client.CoreV1(), - Lister: corev1listers.NewSecretLister(indexer), - EventRecorder: recorder, - AdditionalAnnotations: AdditionalAnnotations{ - JiraComponent: "test", - }, - Owner: &metav1.OwnerReference{ - Name: "operator", + c := &CertRotationController{ + Name: "test-signer", + SigningCA: SigningCAConfig{ + Namespace: "ns", + Name: "signer", + Validity: 24 * time.Hour, + Refresh: 12 * time.Hour, + AdditionalAnnotations: AdditionalAnnotations{ + JiraComponent: "test", + }, + Owner: &metav1.OwnerReference{ + Name: "operator", + }, + RefreshOnlyWhenExpired: test.RefreshOnlyWhenExpired, }, - RefreshOnlyWhenExpired: test.RefreshOnlyWhenExpired, + kubeClient: client, + kubeInformers: kubeInformers, + eventRecorder: recorder, } - _, updated, err := c.EnsureSigningCertKeyPair(context.TODO()) + _, updated, err := c.ensureSigningCertKeyPair(context.TODO()) switch { case err != nil && len(test.expectedError) == 0: t.Error(err) diff --git a/pkg/operator/certrotation/target.go b/pkg/operator/certrotation/target.go index 88cd41189e..7671db77a6 100644 --- a/pkg/operator/certrotation/target.go +++ b/pkg/operator/certrotation/target.go @@ -16,21 +16,11 @@ import ( "github.com/openshift/library-go/pkg/certs" "github.com/openshift/library-go/pkg/crypto" - "github.com/openshift/library-go/pkg/operator/events" "github.com/openshift/library-go/pkg/operator/resource/resourcehelper" - corev1informers "k8s.io/client-go/informers/core/v1" - corev1client "k8s.io/client-go/kubernetes/typed/core/v1" - corev1listers "k8s.io/client-go/listers/core/v1" ) -// RotatedSelfSignedCertKeySecret rotates a key and cert signed by a signing CA and stores it in a secret. -// -// It creates a new one when -// - refresh duration is over -// - or 80% of validity is over (if RefreshOnlyWhenExpired is false) -// - or the cert is expired. -// - or the signing CA changes. -type RotatedSelfSignedCertKeySecret struct { +// TargetCertKeyPairConfig holds the configuration for a target certificate key pair. +type TargetCertKeyPairConfig struct { // Namespace is the namespace of the Secret. Namespace string // Name is the name of the Secret. @@ -43,9 +33,6 @@ type RotatedSelfSignedCertKeySecret struct { // Refresh is ignored until the signing CA at least 10% in its life-time to ensure it is deployed // through-out the cluster. Refresh time.Duration - // RefreshOnlyWhenExpired allows rotating only certs that are already expired. (for autorecovery) - // If false (regular flow) it rotates at the refresh interval but no later then 4/5 of the cert lifetime. - // RefreshOnlyWhenExpired set to true means to ignore 80% of validity and the Refresh duration for rotation, // but only rotate when the certificate expires. This is useful for auto-recovery when we want to enforce // rotation on expiration only, but not interfere with the ordinary rotation controller. @@ -61,32 +48,40 @@ type RotatedSelfSignedCertKeySecret struct { // AdditionalAnnotations is a collection of annotations set for the secret AdditionalAnnotations AdditionalAnnotations - // CertCreator does the actual cert generation. - CertCreator TargetCertCreator + // CertConfig specifies the type of certificate to create. + CertConfig TargetCertConfig +} + +// TargetCertConfig is a sealed interface for target certificate configuration. +// Implementations: ClientCertConfig, ServingCertConfig, SignerCertConfig. +type TargetCertConfig interface { + targetCertConfig() +} - // Plumbing: - Informer corev1informers.SecretInformer - Lister corev1listers.SecretLister - Client corev1client.SecretsGetter - EventRecorder events.Recorder +// ClientCertConfig configures a client certificate. +type ClientCertConfig struct { + UserInfo user.Info } -type TargetCertCreator interface { - // NewCertificate creates a new key-cert pair with the given signer. - NewCertificate(signer *crypto.CA, validity time.Duration) (*crypto.TLSCertificateConfig, error) - // NeedNewTargetCertKeyPair decides whether a new cert-key pair is needed. It returns a non-empty reason if it is the case. - NeedNewTargetCertKeyPair(currentCertSecret *corev1.Secret, signer *crypto.CA, caBundleCerts []*x509.Certificate, refresh time.Duration, refreshOnlyWhenExpired, creationRequired bool) string - // SetAnnotations gives an option to override or set additional annotations - SetAnnotations(cert *crypto.TLSCertificateConfig, annotations map[string]string) map[string]string +func (ClientCertConfig) targetCertConfig() {} + +// ServingCertConfig configures a serving certificate. +type ServingCertConfig struct { + Hostnames func() []string + HostnamesChanged <-chan struct{} + CertificateExtensionFn []crypto.CertificateExtensionFunc } -// TargetCertRechecker is an optional interface to be implemented by the TargetCertCreator to enforce -// a controller run. -type TargetCertRechecker interface { - RecheckChannel() <-chan struct{} +func (ServingCertConfig) targetCertConfig() {} + +// SignerCertConfig configures an intermediate signer certificate. +type SignerCertConfig struct { + SignerName string } -func (c RotatedSelfSignedCertKeySecret) EnsureTargetCertKeyPair(ctx context.Context, signingCertKeyPair *crypto.CA, caBundleCerts []*x509.Certificate) (*corev1.Secret, error) { +func (SignerCertConfig) targetCertConfig() {} + +func (c CertRotationController) ensureTargetCertKeyPair(ctx context.Context, signingCertKeyPair *crypto.CA, caBundleCerts []*x509.Certificate) (*corev1.Secret, error) { // at this point our trust bundle has been updated. We don't know for sure that consumers have updated, but that's why we have a second // validity percentage. We always check to see if we need to sign. Often we are signing with an old key or we have no target // and need to mint one @@ -94,7 +89,7 @@ func (c RotatedSelfSignedCertKeySecret) EnsureTargetCertKeyPair(ctx context.Cont creationRequired := false updateRequired := false - originalTargetCertKeyPairSecret, err := c.Lister.Secrets(c.Namespace).Get(c.Name) + originalTargetCertKeyPairSecret, err := c.kubeInformers.SecretLister().Secrets(c.TargetCert.Namespace).Get(c.TargetCert.Name) if err != nil && !apierrors.IsNotFound(err) { return nil, err } @@ -103,9 +98,9 @@ func (c RotatedSelfSignedCertKeySecret) EnsureTargetCertKeyPair(ctx context.Cont // create an empty one targetCertKeyPairSecret = &corev1.Secret{ ObjectMeta: NewTLSArtifactObjectMeta( - c.Name, - c.Namespace, - c.AdditionalAnnotations, + c.TargetCert.Name, + c.TargetCert.Namespace, + c.TargetCert.AdditionalAnnotations, ), Type: corev1.SecretTypeTLS, } @@ -113,15 +108,22 @@ func (c RotatedSelfSignedCertKeySecret) EnsureTargetCertKeyPair(ctx context.Cont } // run Update if metadata needs changing unless we're in RefreshOnlyWhenExpired mode - if !c.RefreshOnlyWhenExpired { - needsMetadataUpdate := ensureOwnerRefAndTLSAnnotations(targetCertKeyPairSecret, c.Owner, c.AdditionalAnnotations) + if !c.TargetCert.RefreshOnlyWhenExpired { + needsMetadataUpdate := ensureOwnerRefAndTLSAnnotations(targetCertKeyPairSecret, c.TargetCert.Owner, c.TargetCert.AdditionalAnnotations) needsTypeChange := ensureSecretTLSTypeSet(targetCertKeyPairSecret) updateRequired = needsMetadataUpdate || needsTypeChange } - 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 = setTargetCertKeyPairSecretAndTLSAnnotations(targetCertKeyPairSecret, c.Validity, c.Refresh, signingCertKeyPair, c.CertCreator, c.AdditionalAnnotations); err != nil { + // Determine hostnames function for serving certs + var hostnames func() []string + if serving, ok := c.TargetCert.CertConfig.(ServingCertConfig); ok { + hostnames = serving.Hostnames + } + + if reason := needNewTargetCertKeyPair(targetCertKeyPairSecret, signingCertKeyPair, caBundleCerts, c.TargetCert.Refresh, c.TargetCert.RefreshOnlyWhenExpired, creationRequired, hostnames); len(reason) > 0 { + c.eventRecorder.Eventf("TargetUpdateRequired", "%q in %q requires a new target cert/key pair: %v", c.TargetCert.Name, c.TargetCert.Namespace, reason) + + if err := c.setTargetCertKeyPairSecret(targetCertKeyPairSecret, signingCertKeyPair); err != nil { return nil, err } @@ -130,20 +132,20 @@ func (c RotatedSelfSignedCertKeySecret) EnsureTargetCertKeyPair(ctx context.Cont updateRequired = true } if creationRequired { - actualTargetCertKeyPairSecret, err := c.Client.Secrets(c.Namespace).Create(ctx, targetCertKeyPairSecret, metav1.CreateOptions{}) - resourcehelper.ReportCreateEvent(c.EventRecorder, actualTargetCertKeyPairSecret, err) + actualTargetCertKeyPairSecret, err := c.kubeClient.CoreV1().Secrets(c.TargetCert.Namespace).Create(ctx, targetCertKeyPairSecret, metav1.CreateOptions{}) + resourcehelper.ReportCreateEvent(c.eventRecorder, actualTargetCertKeyPairSecret, err) if err != nil { return nil, err } klog.V(2).Infof("Created secret %s/%s", actualTargetCertKeyPairSecret.Namespace, actualTargetCertKeyPairSecret.Name) targetCertKeyPairSecret = actualTargetCertKeyPairSecret } else if updateRequired { - actualTargetCertKeyPairSecret, err := c.Client.Secrets(c.Namespace).Update(ctx, targetCertKeyPairSecret, metav1.UpdateOptions{}) + actualTargetCertKeyPairSecret, err := c.kubeClient.CoreV1().Secrets(c.TargetCert.Namespace).Update(ctx, targetCertKeyPairSecret, metav1.UpdateOptions{}) if apierrors.IsConflict(err) { // ignore error if its attempting to update outdated version of the secret return nil, nil } - resourcehelper.ReportUpdateEvent(c.EventRecorder, actualTargetCertKeyPairSecret, err) + resourcehelper.ReportUpdateEvent(c.eventRecorder, actualTargetCertKeyPairSecret, err) if err != nil { return nil, err } @@ -154,7 +156,73 @@ func (c RotatedSelfSignedCertKeySecret) EnsureTargetCertKeyPair(ctx context.Cont return targetCertKeyPairSecret, nil } -func needNewTargetCertKeyPair(secret *corev1.Secret, signer *crypto.CA, caBundleCerts []*x509.Certificate, refresh time.Duration, refreshOnlyWhenExpired, creationRequired bool) string { +// setTargetCertKeyPairSecret creates a new cert/key pair, sets them in the secret, and applies TLS annotations. +func (c CertRotationController) setTargetCertKeyPairSecret(targetCertKeyPairSecret *corev1.Secret, signer *crypto.CA) error { + if targetCertKeyPairSecret.Annotations == nil { + targetCertKeyPairSecret.Annotations = map[string]string{} + } + if targetCertKeyPairSecret.Data == nil { + targetCertKeyPairSecret.Data = map[string][]byte{} + } + + // our annotation is based on our cert validity, so we want to make sure that we don't specify something past our signer + targetValidity := c.TargetCert.Validity + remainingSignerValidity := signer.Config.Certs[0].NotAfter.Sub(time.Now()) + if remainingSignerValidity < targetValidity { + targetValidity = remainingSignerValidity + } + + var certKeyPair *crypto.TLSCertificateConfig + var err error + switch cfg := c.TargetCert.CertConfig.(type) { + case ClientCertConfig: + certKeyPair, err = signer.MakeClientCertificateForDuration(cfg.UserInfo, targetValidity) + case ServingCertConfig: + if len(cfg.Hostnames()) == 0 { + return fmt.Errorf("no hostnames set") + } + certKeyPair, err = signer.MakeServerCertForDuration(sets.New(cfg.Hostnames()...), targetValidity, cfg.CertificateExtensionFn...) + case SignerCertConfig: + signerName := fmt.Sprintf("%s_@%d", cfg.SignerName, time.Now().Unix()) + certKeyPair, err = crypto.MakeCAConfigForDuration(signerName, targetValidity, signer) + default: + return fmt.Errorf("unknown target cert config type: %T", cfg) + } + if err != nil { + return err + } + + targetCertKeyPairSecret.Data["tls.crt"], targetCertKeyPairSecret.Data["tls.key"], err = certKeyPair.GetPEMBytes() + if err != nil { + return err + } + + // Set TLS annotations + targetCertKeyPairSecret.Annotations[CertificateIssuer] = certKeyPair.Certs[0].Issuer.CommonName + + tlsAnnotations := c.TargetCert.AdditionalAnnotations + tlsAnnotations.NotBefore = certKeyPair.Certs[0].NotBefore.Format(time.RFC3339) + tlsAnnotations.NotAfter = certKeyPair.Certs[0].NotAfter.Format(time.RFC3339) + tlsAnnotations.RefreshPeriod = c.TargetCert.Refresh.String() + _ = tlsAnnotations.EnsureTLSMetadataUpdate(&targetCertKeyPairSecret.ObjectMeta) + + // Set hostname annotations for serving certs + if _, ok := c.TargetCert.CertConfig.(ServingCertConfig); ok { + hostnames := sets.Set[string]{} + for _, ip := range certKeyPair.Certs[0].IPAddresses { + hostnames.Insert(ip.String()) + } + for _, dnsName := range certKeyPair.Certs[0].DNSNames { + hostnames.Insert(dnsName) + } + // List does a sort so that we have a consistent representation + targetCertKeyPairSecret.Annotations[CertificateHostnames] = strings.Join(sets.List(hostnames), ",") + } + + return nil +} + +func needNewTargetCertKeyPair(secret *corev1.Secret, signer *crypto.CA, caBundleCerts []*x509.Certificate, refresh time.Duration, refreshOnlyWhenExpired, creationRequired bool, hostnames func() []string) string { if creationRequired { return "secret doesn't exist" } @@ -176,6 +244,10 @@ func needNewTargetCertKeyPair(secret *corev1.Secret, signer *crypto.CA, caBundle } for _, caCert := range caBundleCerts { if signerCommonName == caCert.Subject.CommonName { + // Signer is in the CA bundle. Check hostnames if applicable. + if hostnames != nil { + return missingHostnames(secret.Annotations, hostnames) + } return "" } } @@ -237,107 +309,9 @@ 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, refresh 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, refresh, 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) (*crypto.TLSCertificateConfig, error) { - if targetCertKeyPairSecret.Annotations == nil { - targetCertKeyPairSecret.Annotations = map[string]string{} - } - if targetCertKeyPairSecret.Data == nil { - targetCertKeyPairSecret.Data = map[string][]byte{} - } - - // our annotation is based on our cert validity, so we want to make sure that we don't specify something past our signer - targetValidity := validity - remainingSignerValidity := signer.Config.Certs[0].NotAfter.Sub(time.Now()) - if remainingSignerValidity < validity { - targetValidity = remainingSignerValidity - } - - certKeyPair, err := certCreator.NewCertificate(signer, targetValidity) - if err != nil { - return nil, err - } - - targetCertKeyPairSecret.Data["tls.crt"], targetCertKeyPairSecret.Data["tls.key"], err = certKeyPair.GetPEMBytes() - 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, refresh time.Duration, tlsAnnotations AdditionalAnnotations) { - targetCertKeyPairSecret.Annotations[CertificateIssuer] = certKeyPair.Certs[0].Issuer.CommonName - - tlsAnnotations.NotBefore = certKeyPair.Certs[0].NotBefore.Format(time.RFC3339) - tlsAnnotations.NotAfter = certKeyPair.Certs[0].NotAfter.Format(time.RFC3339) - tlsAnnotations.RefreshPeriod = refresh.String() - _ = tlsAnnotations.EnsureTLSMetadataUpdate(&targetCertKeyPairSecret.ObjectMeta) - - certCreator.SetAnnotations(certKeyPair, targetCertKeyPairSecret.Annotations) -} - -type ClientRotation struct { - UserInfo user.Info -} - -func (r *ClientRotation) NewCertificate(signer *crypto.CA, validity time.Duration) (*crypto.TLSCertificateConfig, error) { - return signer.MakeClientCertificateForDuration(r.UserInfo, validity) -} - -func (r *ClientRotation) NeedNewTargetCertKeyPair(currentCertSecret *corev1.Secret, signer *crypto.CA, caBundleCerts []*x509.Certificate, refresh time.Duration, refreshOnlyWhenExpired, exists bool) string { - return needNewTargetCertKeyPair(currentCertSecret, signer, caBundleCerts, refresh, refreshOnlyWhenExpired, exists) -} - -func (r *ClientRotation) SetAnnotations(cert *crypto.TLSCertificateConfig, annotations map[string]string) map[string]string { - return annotations -} - -type ServingRotation struct { - Hostnames ServingHostnameFunc - CertificateExtensionFn []crypto.CertificateExtensionFunc - HostnamesChanged <-chan struct{} -} - -func (r *ServingRotation) NewCertificate(signer *crypto.CA, validity time.Duration) (*crypto.TLSCertificateConfig, error) { - if len(r.Hostnames()) == 0 { - return nil, fmt.Errorf("no hostnames set") - } - return signer.MakeServerCertForDuration(sets.New(r.Hostnames()...), validity, r.CertificateExtensionFn...) -} - -func (r *ServingRotation) RecheckChannel() <-chan struct{} { - return r.HostnamesChanged -} - -func (r *ServingRotation) NeedNewTargetCertKeyPair(currentCertSecret *corev1.Secret, signer *crypto.CA, caBundleCerts []*x509.Certificate, refresh time.Duration, refreshOnlyWhenExpired, creationRequired bool) string { - reason := needNewTargetCertKeyPair(currentCertSecret, signer, caBundleCerts, refresh, refreshOnlyWhenExpired, creationRequired) - if len(reason) > 0 { - return reason - } - - return r.missingHostnames(currentCertSecret.Annotations) -} - -func (r *ServingRotation) missingHostnames(annotations map[string]string) string { +func missingHostnames(annotations map[string]string, hostnames func() []string) string { existingHostnames := sets.New(strings.Split(annotations[CertificateHostnames], ",")...) - requiredHostnames := sets.New(r.Hostnames()...) + requiredHostnames := sets.New(hostnames()...) if !existingHostnames.Equal(requiredHostnames) { existingNotRequired := existingHostnames.Difference(requiredHostnames) requiredNotExisting := requiredHostnames.Difference(existingHostnames) @@ -345,37 +319,4 @@ func (r *ServingRotation) missingHostnames(annotations map[string]string) string } return "" -} - -func (r *ServingRotation) SetAnnotations(cert *crypto.TLSCertificateConfig, annotations map[string]string) map[string]string { - hostnames := sets.Set[string]{} - for _, ip := range cert.Certs[0].IPAddresses { - hostnames.Insert(ip.String()) - } - for _, dnsName := range cert.Certs[0].DNSNames { - hostnames.Insert(dnsName) - } - - // List does a sort so that we have a consistent representation - annotations[CertificateHostnames] = strings.Join(sets.List(hostnames), ",") - return annotations -} - -type ServingHostnameFunc func() []string - -type SignerRotation struct { - SignerName string -} - -func (r *SignerRotation) NewCertificate(signer *crypto.CA, validity time.Duration) (*crypto.TLSCertificateConfig, error) { - signerName := fmt.Sprintf("%s_@%d", r.SignerName, time.Now().Unix()) - return crypto.MakeCAConfigForDuration(signerName, validity, signer) -} - -func (r *SignerRotation) NeedNewTargetCertKeyPair(currentCertSecret *corev1.Secret, signer *crypto.CA, caBundleCerts []*x509.Certificate, refresh time.Duration, refreshOnlyWhenExpired, exists bool) string { - return needNewTargetCertKeyPair(currentCertSecret, signer, caBundleCerts, refresh, refreshOnlyWhenExpired, exists) -} - -func (r *SignerRotation) SetAnnotations(cert *crypto.TLSCertificateConfig, annotations map[string]string) map[string]string { - return annotations -} +} \ No newline at end of file diff --git a/pkg/operator/certrotation/target_test.go b/pkg/operator/certrotation/target_test.go index 1df8cf856f..39703368cd 100644 --- a/pkg/operator/certrotation/target_test.go +++ b/pkg/operator/certrotation/target_test.go @@ -15,13 +15,13 @@ import ( "github.com/openshift/api/annotations" "github.com/openshift/library-go/pkg/crypto" "github.com/openshift/library-go/pkg/operator/events" + "github.com/openshift/library-go/pkg/operator/v1helpers" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/informers" kubefake "k8s.io/client-go/kubernetes/fake" - corev1listers "k8s.io/client-go/listers/core/v1" clienttesting "k8s.io/client-go/testing" - "k8s.io/client-go/tools/cache" ) func TestNeedNewTargetCertKeyPairForTime(t *testing.T) { @@ -268,40 +268,46 @@ func TestEnsureTargetCertKeyPair(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc}) - client := kubefake.NewSimpleClientset() + informerFactory := informers.NewSharedInformerFactory(client, 0) if startingObj := test.initialSecretFn(); startingObj != nil { - indexer.Add(startingObj) client = kubefake.NewSimpleClientset(startingObj) + informerFactory = informers.NewSharedInformerFactory(client, 0) + informerFactory.Core().V1().Secrets().Informer().GetStore().Add(startingObj) } - c := &RotatedSelfSignedCertKeySecret{ - Namespace: "ns", - Validity: 24 * time.Hour, - Refresh: 12 * time.Hour, - Name: "target-secret", - CertCreator: &ServingRotation{ - Hostnames: func() []string { return []string{"foo", "bar"} }, - }, - - Client: client.CoreV1(), - Lister: corev1listers.NewSecretLister(indexer), - EventRecorder: events.NewInMemoryRecorder("test", clocktesting.NewFakePassiveClock(time.Now())), - AdditionalAnnotations: AdditionalAnnotations{ - JiraComponent: "test", - }, - Owner: &metav1.OwnerReference{ - Name: "operator", + kubeInformers := v1helpers.NewFakeKubeInformersForNamespaces(map[string]informers.SharedInformerFactory{ + "ns": informerFactory, + }) + + c := &CertRotationController{ + Name: "test-target", + TargetCert: TargetCertKeyPairConfig{ + Namespace: "ns", + Name: "target-secret", + Validity: 24 * time.Hour, + Refresh: 12 * time.Hour, + CertConfig: ServingCertConfig{ + Hostnames: func() []string { return []string{"foo", "bar"} }, + }, + AdditionalAnnotations: AdditionalAnnotations{ + JiraComponent: "test", + }, + Owner: &metav1.OwnerReference{ + Name: "operator", + }, + RefreshOnlyWhenExpired: test.RefreshOnlyWhenExpired, }, - RefreshOnlyWhenExpired: test.RefreshOnlyWhenExpired, + kubeClient: client, + kubeInformers: kubeInformers, + eventRecorder: events.NewInMemoryRecorder("test", clocktesting.NewFakePassiveClock(time.Now())), } newCA, err := test.caFn() if err != nil { t.Fatal(err) } - _, err = c.EnsureTargetCertKeyPair(context.TODO(), newCA, newCA.Config.Certs) + _, err = c.ensureTargetCertKeyPair(context.TODO(), newCA, newCA.Config.Certs) switch { case err != nil && len(test.expectedError) == 0: t.Error(err) @@ -359,10 +365,8 @@ func TestServerHostnameCheck(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - r := &ServingRotation{ - Hostnames: func() []string { return test.requiredHostnames }, - } - actual := r.missingHostnames(map[string]string{CertificateHostnames: test.existingHostnames}) + hostnames := func() []string { return test.requiredHostnames } + actual := missingHostnames(map[string]string{CertificateHostnames: test.existingHostnames}, hostnames) if actual != test.expected { t.Fatal(actual) } @@ -466,33 +470,39 @@ func TestEnsureTargetSignerCertKeyPair(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc}) - client := kubefake.NewSimpleClientset() + informerFactory := informers.NewSharedInformerFactory(client, 0) if startingObj := test.initialSecretFn(); startingObj != nil { - indexer.Add(startingObj) client = kubefake.NewSimpleClientset(startingObj) + informerFactory = informers.NewSharedInformerFactory(client, 0) + informerFactory.Core().V1().Secrets().Informer().GetStore().Add(startingObj) } - c := &RotatedSelfSignedCertKeySecret{ - Namespace: "ns", - Validity: 24 * time.Hour, - Refresh: 12 * time.Hour, - Name: "target-secret", - CertCreator: &SignerRotation{ - SignerName: "lower-signer", + kubeInformers := v1helpers.NewFakeKubeInformersForNamespaces(map[string]informers.SharedInformerFactory{ + "ns": informerFactory, + }) + + c := &CertRotationController{ + Name: "test-target-signer", + TargetCert: TargetCertKeyPairConfig{ + Namespace: "ns", + Name: "target-secret", + Validity: 24 * time.Hour, + Refresh: 12 * time.Hour, + CertConfig: SignerCertConfig{ + SignerName: "lower-signer", + }, }, - - Client: client.CoreV1(), - Lister: corev1listers.NewSecretLister(indexer), - EventRecorder: events.NewInMemoryRecorder("test", clocktesting.NewFakePassiveClock(time.Now())), + kubeClient: client, + kubeInformers: kubeInformers, + eventRecorder: events.NewInMemoryRecorder("test", clocktesting.NewFakePassiveClock(time.Now())), } newCA, err := test.caFn() if err != nil { t.Fatal(err) } - _, err = c.EnsureTargetCertKeyPair(context.TODO(), newCA, newCA.Config.Certs) + _, err = c.ensureTargetCertKeyPair(context.TODO(), newCA, newCA.Config.Certs) switch { case err != nil && len(test.expectedError) == 0: t.Error(err) @@ -690,6 +700,7 @@ func TestNeedNewTargetCertKeyPair(t *testing.T) { test.refresh, test.refreshOnlyWhenExpired, test.creationRequired, + nil, ) if test.expected == "" { From a957edb574ab21331810c8fd25a92d07f7509ee8 Mon Sep 17 00:00:00 2001 From: Luis Sanchez Date: Fri, 3 Apr 2026 16:24:28 -0400 Subject: [PATCH 2/2] certrotation: move ensure methods into controller file Move ensureSigningCertKeyPair, ensureConfigMapCABundle, ensureTargetCertKeyPair, and setTargetCertKeyPairSecret from signer.go, cabundle.go, and target.go into client_cert_rotation_controller.go. This consolidates all controller sync logic in one file, leaving only config types and pure helper functions in the per-artifact files. --- pkg/operator/certrotation/cabundle.go | 95 +----- .../client_cert_rotation_controller.go | 321 +++++++++++++++++- pkg/operator/certrotation/signer.go | 87 +---- pkg/operator/certrotation/target.go | 149 +------- 4 files changed, 324 insertions(+), 328 deletions(-) diff --git a/pkg/operator/certrotation/cabundle.go b/pkg/operator/certrotation/cabundle.go index ba4e304671..4817f21ad5 100644 --- a/pkg/operator/certrotation/cabundle.go +++ b/pkg/operator/certrotation/cabundle.go @@ -2,22 +2,15 @@ package certrotation import ( "bytes" - "context" "crypto/x509" - "fmt" "reflect" "sort" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/equality" - apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/util/cert" - "k8s.io/klog/v2" - "github.com/openshift/library-go/pkg/certs" "github.com/openshift/library-go/pkg/crypto" - "github.com/openshift/library-go/pkg/operator/resource/resourcehelper" ) // CABundleConfig holds the configuration for a CA bundle config map, by adding new CA certs coming from SigningCAConfig, and by removing expired old ones. @@ -36,92 +29,6 @@ type CABundleConfig struct { AdditionalAnnotations AdditionalAnnotations } -func (c CertRotationController) ensureConfigMapCABundle(ctx context.Context, signingCertKeyPair *crypto.CA) ([]*x509.Certificate, error) { - signingCertKeyPairLocation := c.getSigningCertKeyPairLocation() - - // by this point we have current signing cert/key pair. We now need to make sure that the ca-bundle configmap has this cert and - // doesn't have any expired certs - updateRequired := false - creationRequired := false - - originalCABundleConfigMap, err := c.kubeInformers.ConfigMapLister().ConfigMaps(c.CABundle.Namespace).Get(c.CABundle.Name) - if err != nil && !apierrors.IsNotFound(err) { - return nil, err - } - caBundleConfigMap := originalCABundleConfigMap.DeepCopy() - if apierrors.IsNotFound(err) { - // create an empty one - caBundleConfigMap = &corev1.ConfigMap{ObjectMeta: NewTLSArtifactObjectMeta( - c.CABundle.Name, - c.CABundle.Namespace, - c.CABundle.AdditionalAnnotations, - )} - creationRequired = true - } - - // run Update if metadata needs changing unless running in RefreshOnlyWhenExpired mode - if !c.CABundle.RefreshOnlyWhenExpired { - needsOwnerUpdate := false - if c.CABundle.Owner != nil { - needsOwnerUpdate = ensureOwnerReference(&caBundleConfigMap.ObjectMeta, c.CABundle.Owner) - } - needsMetadataUpdate := c.CABundle.AdditionalAnnotations.EnsureTLSMetadataUpdate(&caBundleConfigMap.ObjectMeta) - updateRequired = needsOwnerUpdate || needsMetadataUpdate - } - - updatedCerts, err := manageCABundleConfigMap(caBundleConfigMap, signingCertKeyPair.Config.Certs[0]) - if err != nil { - return nil, err - } - if originalCABundleConfigMap == nil || originalCABundleConfigMap.Data == nil || !equality.Semantic.DeepEqual(originalCABundleConfigMap.Data, caBundleConfigMap.Data) { - reason := "" - if creationRequired { - reason = "configmap doesn't exist" - } else if originalCABundleConfigMap.Data == nil { - reason = "configmap is empty" - } else if !equality.Semantic.DeepEqual(originalCABundleConfigMap.Data, caBundleConfigMap.Data) { - reason = fmt.Sprintf("signer update %s", signingCertKeyPairLocation) - } - c.eventRecorder.Eventf("CABundleUpdateRequired", "%q in %q requires a new cert: %s", c.CABundle.Name, c.CABundle.Namespace, reason) - LabelAsManagedConfigMap(caBundleConfigMap, CertificateTypeCABundle) - - updateRequired = true - } - - if creationRequired { - actualCABundleConfigMap, err := c.kubeClient.CoreV1().ConfigMaps(c.CABundle.Namespace).Create(ctx, caBundleConfigMap, metav1.CreateOptions{}) - resourcehelper.ReportCreateEvent(c.eventRecorder, actualCABundleConfigMap, err) - if err != nil { - return nil, err - } - klog.V(2).Infof("Created ca-bundle.crt configmap %s/%s with:\n%s", certs.CertificateBundleToString(updatedCerts), caBundleConfigMap.Namespace, caBundleConfigMap.Name) - caBundleConfigMap = actualCABundleConfigMap - } else if updateRequired { - actualCABundleConfigMap, err := c.kubeClient.CoreV1().ConfigMaps(c.CABundle.Namespace).Update(ctx, caBundleConfigMap, metav1.UpdateOptions{}) - if apierrors.IsConflict(err) { - // ignore error if its attempting to update outdated version of the configmap - return nil, nil - } - resourcehelper.ReportUpdateEvent(c.eventRecorder, actualCABundleConfigMap, err) - if err != nil { - return nil, err - } - klog.V(2).Infof("Updated ca-bundle.crt configmap %s/%s with:\n%s", certs.CertificateBundleToString(updatedCerts), caBundleConfigMap.Namespace, caBundleConfigMap.Name) - caBundleConfigMap = actualCABundleConfigMap - } - - caBundle := caBundleConfigMap.Data["ca-bundle.crt"] - if len(caBundle) == 0 { - return nil, fmt.Errorf("configmap/%s -n%s missing ca-bundle.crt", caBundleConfigMap.Name, caBundleConfigMap.Namespace) - } - certificates, err := cert.ParseCertsPEM([]byte(caBundle)) - if err != nil { - return nil, err - } - - return certificates, nil -} - // manageCABundleConfigMap adds the new certificate to the list of cabundles, eliminates duplicates, and prunes the list of expired // certs to trust as signers func manageCABundleConfigMap(caBundleConfigMap *corev1.ConfigMap, currentSigner *x509.Certificate) ([]*x509.Certificate, error) { @@ -168,4 +75,4 @@ func manageCABundleConfigMap(caBundleConfigMap *corev1.ConfigMap, currentSigner caBundleConfigMap.Data["ca-bundle.crt"] = string(caBytes) return finalCertificates, nil -} \ No newline at end of file +} diff --git a/pkg/operator/certrotation/client_cert_rotation_controller.go b/pkg/operator/certrotation/client_cert_rotation_controller.go index ab007b8ca4..100262520f 100644 --- a/pkg/operator/certrotation/client_cert_rotation_controller.go +++ b/pkg/operator/certrotation/client_cert_rotation_controller.go @@ -2,18 +2,29 @@ package certrotation import ( "context" + "crypto/x509" "fmt" + "strings" "time" operatorv1 "github.com/openshift/api/operator/v1" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/kubernetes" + "k8s.io/client-go/util/cert" + "k8s.io/klog/v2" + "github.com/openshift/library-go/pkg/certs" "github.com/openshift/library-go/pkg/controller/factory" + "github.com/openshift/library-go/pkg/crypto" "github.com/openshift/library-go/pkg/operator/condition" "github.com/openshift/library-go/pkg/operator/events" + "github.com/openshift/library-go/pkg/operator/resource/resourcehelper" "github.com/openshift/library-go/pkg/operator/v1helpers" + "k8s.io/apimachinery/pkg/api/equality" ) const ( @@ -177,6 +188,314 @@ func (c CertRotationController) SyncWorker(ctx context.Context) error { return nil } +// ensureSigningCertKeyPair manages the entire lifecycle of a signer cert as a secret, from creation to continued rotation. +// It always returns the currently used CA pair, a bool indicating whether it was created/updated within this function call and an error. +func (c CertRotationController) ensureSigningCertKeyPair(ctx context.Context) (*crypto.CA, bool, error) { + creationRequired := false + updateRequired := false + originalSigningCertKeyPairSecret, err := c.kubeInformers.SecretLister().Secrets(c.SigningCA.Namespace).Get(c.SigningCA.Name) + if err != nil && !apierrors.IsNotFound(err) { + return nil, false, err + } + signingCertKeyPairSecret := originalSigningCertKeyPairSecret.DeepCopy() + if apierrors.IsNotFound(err) { + // create an empty one + signingCertKeyPairSecret = &corev1.Secret{ + ObjectMeta: NewTLSArtifactObjectMeta( + c.SigningCA.Name, + c.SigningCA.Namespace, + c.SigningCA.AdditionalAnnotations, + ), + Type: corev1.SecretTypeTLS, + } + creationRequired = true + } + + // run Update if metadata needs changing unless we're in RefreshOnlyWhenExpired mode + if !c.SigningCA.RefreshOnlyWhenExpired { + needsMetadataUpdate := ensureOwnerRefAndTLSAnnotations(signingCertKeyPairSecret, c.SigningCA.Owner, c.SigningCA.AdditionalAnnotations) + needsTypeChange := ensureSecretTLSTypeSet(signingCertKeyPairSecret) + updateRequired = needsMetadataUpdate || needsTypeChange + } + + // run Update if signer content needs changing + signerUpdated := false + if needed, reason := needNewSigningCertKeyPair(signingCertKeyPairSecret, c.SigningCA.Refresh, c.SigningCA.RefreshOnlyWhenExpired); needed || creationRequired { + if creationRequired { + reason = "secret doesn't exist" + } + c.eventRecorder.Eventf("SignerUpdateRequired", "%q in %q requires a new signing cert/key pair: %v", c.SigningCA.Name, c.SigningCA.Namespace, reason) + + ca, err := setSigningCertKeyPairSecret(signingCertKeyPairSecret, c.SigningCA.Validity) + if err != nil { + return nil, false, err + } + setTLSAnnotationsOnSigningCertKeyPairSecret(signingCertKeyPairSecret, ca, c.SigningCA.Refresh, c.SigningCA.AdditionalAnnotations) + + LabelAsManagedSecret(signingCertKeyPairSecret, CertificateTypeSigner) + + updateRequired = true + signerUpdated = true + } + + if creationRequired { + actualSigningCertKeyPairSecret, err := c.kubeClient.CoreV1().Secrets(c.SigningCA.Namespace).Create(ctx, signingCertKeyPairSecret, metav1.CreateOptions{}) + resourcehelper.ReportCreateEvent(c.eventRecorder, actualSigningCertKeyPairSecret, err) + if err != nil { + return nil, false, err + } + klog.V(2).Infof("Created secret %s/%s", actualSigningCertKeyPairSecret.Namespace, actualSigningCertKeyPairSecret.Name) + signingCertKeyPairSecret = actualSigningCertKeyPairSecret + } else if updateRequired { + actualSigningCertKeyPairSecret, err := c.kubeClient.CoreV1().Secrets(c.SigningCA.Namespace).Update(ctx, signingCertKeyPairSecret, metav1.UpdateOptions{}) + if apierrors.IsConflict(err) { + // ignore error if its attempting to update outdated version of the secret + return nil, false, nil + } + resourcehelper.ReportUpdateEvent(c.eventRecorder, actualSigningCertKeyPairSecret, err) + if err != nil { + return nil, false, err + } + klog.V(2).Infof("Updated secret %s/%s", actualSigningCertKeyPairSecret.Namespace, actualSigningCertKeyPairSecret.Name) + signingCertKeyPairSecret = actualSigningCertKeyPairSecret + } + + // at this point, the secret has the correct signer, so we should read that signer to be able to sign + signingCertKeyPair, err := crypto.GetCAFromBytes(signingCertKeyPairSecret.Data["tls.crt"], signingCertKeyPairSecret.Data["tls.key"]) + if err != nil { + return nil, signerUpdated, err + } + + return signingCertKeyPair, signerUpdated, nil +} + +func (c CertRotationController) ensureConfigMapCABundle(ctx context.Context, signingCertKeyPair *crypto.CA) ([]*x509.Certificate, error) { + signingCertKeyPairLocation := c.getSigningCertKeyPairLocation() + + // by this point we have current signing cert/key pair. We now need to make sure that the ca-bundle configmap has this cert and + // doesn't have any expired certs + updateRequired := false + creationRequired := false + + originalCABundleConfigMap, err := c.kubeInformers.ConfigMapLister().ConfigMaps(c.CABundle.Namespace).Get(c.CABundle.Name) + if err != nil && !apierrors.IsNotFound(err) { + return nil, err + } + caBundleConfigMap := originalCABundleConfigMap.DeepCopy() + if apierrors.IsNotFound(err) { + // create an empty one + caBundleConfigMap = &corev1.ConfigMap{ObjectMeta: NewTLSArtifactObjectMeta( + c.CABundle.Name, + c.CABundle.Namespace, + c.CABundle.AdditionalAnnotations, + )} + creationRequired = true + } + + // run Update if metadata needs changing unless running in RefreshOnlyWhenExpired mode + if !c.CABundle.RefreshOnlyWhenExpired { + needsOwnerUpdate := false + if c.CABundle.Owner != nil { + needsOwnerUpdate = ensureOwnerReference(&caBundleConfigMap.ObjectMeta, c.CABundle.Owner) + } + needsMetadataUpdate := c.CABundle.AdditionalAnnotations.EnsureTLSMetadataUpdate(&caBundleConfigMap.ObjectMeta) + updateRequired = needsOwnerUpdate || needsMetadataUpdate + } + + updatedCerts, err := manageCABundleConfigMap(caBundleConfigMap, signingCertKeyPair.Config.Certs[0]) + if err != nil { + return nil, err + } + if originalCABundleConfigMap == nil || originalCABundleConfigMap.Data == nil || !equality.Semantic.DeepEqual(originalCABundleConfigMap.Data, caBundleConfigMap.Data) { + reason := "" + if creationRequired { + reason = "configmap doesn't exist" + } else if originalCABundleConfigMap.Data == nil { + reason = "configmap is empty" + } else if !equality.Semantic.DeepEqual(originalCABundleConfigMap.Data, caBundleConfigMap.Data) { + reason = fmt.Sprintf("signer update %s", signingCertKeyPairLocation) + } + c.eventRecorder.Eventf("CABundleUpdateRequired", "%q in %q requires a new cert: %s", c.CABundle.Name, c.CABundle.Namespace, reason) + LabelAsManagedConfigMap(caBundleConfigMap, CertificateTypeCABundle) + + updateRequired = true + } + + if creationRequired { + actualCABundleConfigMap, err := c.kubeClient.CoreV1().ConfigMaps(c.CABundle.Namespace).Create(ctx, caBundleConfigMap, metav1.CreateOptions{}) + resourcehelper.ReportCreateEvent(c.eventRecorder, actualCABundleConfigMap, err) + if err != nil { + return nil, err + } + klog.V(2).Infof("Created ca-bundle.crt configmap %s/%s with:\n%s", certs.CertificateBundleToString(updatedCerts), caBundleConfigMap.Namespace, caBundleConfigMap.Name) + caBundleConfigMap = actualCABundleConfigMap + } else if updateRequired { + actualCABundleConfigMap, err := c.kubeClient.CoreV1().ConfigMaps(c.CABundle.Namespace).Update(ctx, caBundleConfigMap, metav1.UpdateOptions{}) + if apierrors.IsConflict(err) { + // ignore error if its attempting to update outdated version of the configmap + return nil, nil + } + resourcehelper.ReportUpdateEvent(c.eventRecorder, actualCABundleConfigMap, err) + if err != nil { + return nil, err + } + klog.V(2).Infof("Updated ca-bundle.crt configmap %s/%s with:\n%s", certs.CertificateBundleToString(updatedCerts), caBundleConfigMap.Namespace, caBundleConfigMap.Name) + caBundleConfigMap = actualCABundleConfigMap + } + + caBundle := caBundleConfigMap.Data["ca-bundle.crt"] + if len(caBundle) == 0 { + return nil, fmt.Errorf("configmap/%s -n%s missing ca-bundle.crt", caBundleConfigMap.Name, caBundleConfigMap.Namespace) + } + certificates, err := cert.ParseCertsPEM([]byte(caBundle)) + if err != nil { + return nil, err + } + + return certificates, nil +} + +func (c CertRotationController) ensureTargetCertKeyPair(ctx context.Context, signingCertKeyPair *crypto.CA, caBundleCerts []*x509.Certificate) (*corev1.Secret, error) { + // at this point our trust bundle has been updated. We don't know for sure that consumers have updated, but that's why we have a second + // validity percentage. We always check to see if we need to sign. Often we are signing with an old key or we have no target + // and need to mint one + // TODO do the cross signing thing, but this shows the API consumers want and a very simple impl. + + creationRequired := false + updateRequired := false + originalTargetCertKeyPairSecret, err := c.kubeInformers.SecretLister().Secrets(c.TargetCert.Namespace).Get(c.TargetCert.Name) + if err != nil && !apierrors.IsNotFound(err) { + return nil, err + } + targetCertKeyPairSecret := originalTargetCertKeyPairSecret.DeepCopy() + if apierrors.IsNotFound(err) { + // create an empty one + targetCertKeyPairSecret = &corev1.Secret{ + ObjectMeta: NewTLSArtifactObjectMeta( + c.TargetCert.Name, + c.TargetCert.Namespace, + c.TargetCert.AdditionalAnnotations, + ), + Type: corev1.SecretTypeTLS, + } + creationRequired = true + } + + // run Update if metadata needs changing unless we're in RefreshOnlyWhenExpired mode + if !c.TargetCert.RefreshOnlyWhenExpired { + needsMetadataUpdate := ensureOwnerRefAndTLSAnnotations(targetCertKeyPairSecret, c.TargetCert.Owner, c.TargetCert.AdditionalAnnotations) + needsTypeChange := ensureSecretTLSTypeSet(targetCertKeyPairSecret) + updateRequired = needsMetadataUpdate || needsTypeChange + } + + // Determine hostnames function for serving certs + var hostnames func() []string + if serving, ok := c.TargetCert.CertConfig.(ServingCertConfig); ok { + hostnames = serving.Hostnames + } + + if reason := needNewTargetCertKeyPair(targetCertKeyPairSecret, signingCertKeyPair, caBundleCerts, c.TargetCert.Refresh, c.TargetCert.RefreshOnlyWhenExpired, creationRequired, hostnames); len(reason) > 0 { + c.eventRecorder.Eventf("TargetUpdateRequired", "%q in %q requires a new target cert/key pair: %v", c.TargetCert.Name, c.TargetCert.Namespace, reason) + + if err := c.setTargetCertKeyPairSecret(targetCertKeyPairSecret, signingCertKeyPair); err != nil { + return nil, err + } + + LabelAsManagedSecret(targetCertKeyPairSecret, CertificateTypeTarget) + + updateRequired = true + } + if creationRequired { + actualTargetCertKeyPairSecret, err := c.kubeClient.CoreV1().Secrets(c.TargetCert.Namespace).Create(ctx, targetCertKeyPairSecret, metav1.CreateOptions{}) + resourcehelper.ReportCreateEvent(c.eventRecorder, actualTargetCertKeyPairSecret, err) + if err != nil { + return nil, err + } + klog.V(2).Infof("Created secret %s/%s", actualTargetCertKeyPairSecret.Namespace, actualTargetCertKeyPairSecret.Name) + targetCertKeyPairSecret = actualTargetCertKeyPairSecret + } else if updateRequired { + actualTargetCertKeyPairSecret, err := c.kubeClient.CoreV1().Secrets(c.TargetCert.Namespace).Update(ctx, targetCertKeyPairSecret, metav1.UpdateOptions{}) + if apierrors.IsConflict(err) { + // ignore error if its attempting to update outdated version of the secret + return nil, nil + } + resourcehelper.ReportUpdateEvent(c.eventRecorder, actualTargetCertKeyPairSecret, err) + if err != nil { + return nil, err + } + klog.V(2).Infof("Updated secret %s/%s", actualTargetCertKeyPairSecret.Namespace, actualTargetCertKeyPairSecret.Name) + targetCertKeyPairSecret = actualTargetCertKeyPairSecret + } + + return targetCertKeyPairSecret, nil +} + +// setTargetCertKeyPairSecret creates a new cert/key pair, sets them in the secret, and applies TLS annotations. +func (c CertRotationController) setTargetCertKeyPairSecret(targetCertKeyPairSecret *corev1.Secret, signer *crypto.CA) error { + if targetCertKeyPairSecret.Annotations == nil { + targetCertKeyPairSecret.Annotations = map[string]string{} + } + if targetCertKeyPairSecret.Data == nil { + targetCertKeyPairSecret.Data = map[string][]byte{} + } + + // our annotation is based on our cert validity, so we want to make sure that we don't specify something past our signer + targetValidity := c.TargetCert.Validity + remainingSignerValidity := signer.Config.Certs[0].NotAfter.Sub(time.Now()) + if remainingSignerValidity < targetValidity { + targetValidity = remainingSignerValidity + } + + var certKeyPair *crypto.TLSCertificateConfig + var err error + switch cfg := c.TargetCert.CertConfig.(type) { + case ClientCertConfig: + certKeyPair, err = signer.MakeClientCertificateForDuration(cfg.UserInfo, targetValidity) + case ServingCertConfig: + if len(cfg.Hostnames()) == 0 { + return fmt.Errorf("no hostnames set") + } + certKeyPair, err = signer.MakeServerCertForDuration(sets.New(cfg.Hostnames()...), targetValidity, cfg.CertificateExtensionFn...) + case SignerCertConfig: + signerName := fmt.Sprintf("%s_@%d", cfg.SignerName, time.Now().Unix()) + certKeyPair, err = crypto.MakeCAConfigForDuration(signerName, targetValidity, signer) + default: + return fmt.Errorf("unknown target cert config type: %T", cfg) + } + if err != nil { + return err + } + + targetCertKeyPairSecret.Data["tls.crt"], targetCertKeyPairSecret.Data["tls.key"], err = certKeyPair.GetPEMBytes() + if err != nil { + return err + } + + // Set TLS annotations + targetCertKeyPairSecret.Annotations[CertificateIssuer] = certKeyPair.Certs[0].Issuer.CommonName + + tlsAnnotations := c.TargetCert.AdditionalAnnotations + tlsAnnotations.NotBefore = certKeyPair.Certs[0].NotBefore.Format(time.RFC3339) + tlsAnnotations.NotAfter = certKeyPair.Certs[0].NotAfter.Format(time.RFC3339) + tlsAnnotations.RefreshPeriod = c.TargetCert.Refresh.String() + _ = tlsAnnotations.EnsureTLSMetadataUpdate(&targetCertKeyPairSecret.ObjectMeta) + + // Set hostname annotations for serving certs + if _, ok := c.TargetCert.CertConfig.(ServingCertConfig); ok { + hostnames := sets.Set[string]{} + for _, ip := range certKeyPair.Certs[0].IPAddresses { + hostnames.Insert(ip.String()) + } + for _, dnsName := range certKeyPair.Certs[0].DNSNames { + hostnames.Insert(dnsName) + } + // List does a sort so that we have a consistent representation + targetCertKeyPairSecret.Annotations[CertificateHostnames] = strings.Join(sets.List(hostnames), ",") + } + + return nil +} + func (c CertRotationController) targetCertRecheckerPostRunHook(ctx context.Context, syncCtx factory.SyncContext) error { serving, ok := c.TargetCert.CertConfig.(ServingCertConfig) if !ok || serving.HostnamesChanged == nil { @@ -195,4 +514,4 @@ func (c CertRotationController) targetCertRecheckerPostRunHook(ctx context.Conte <-ctx.Done() return nil -} \ No newline at end of file +} diff --git a/pkg/operator/certrotation/signer.go b/pkg/operator/certrotation/signer.go index b09a10e9b6..f0fe75ab76 100644 --- a/pkg/operator/certrotation/signer.go +++ b/pkg/operator/certrotation/signer.go @@ -2,16 +2,12 @@ package certrotation import ( "bytes" - "context" "fmt" "time" "github.com/openshift/library-go/pkg/crypto" - "github.com/openshift/library-go/pkg/operator/resource/resourcehelper" corev1 "k8s.io/api/core/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/klog/v2" ) // SigningCAConfig holds the configuration for a self-signed signing CA stored in a secret. It creates a new one when @@ -45,87 +41,6 @@ type SigningCAConfig struct { AdditionalAnnotations AdditionalAnnotations } -// ensureSigningCertKeyPair manages the entire lifecycle of a signer cert as a secret, from creation to continued rotation. -// It always returns the currently used CA pair, a bool indicating whether it was created/updated within this function call and an error. -func (c CertRotationController) ensureSigningCertKeyPair(ctx context.Context) (*crypto.CA, bool, error) { - creationRequired := false - updateRequired := false - originalSigningCertKeyPairSecret, err := c.kubeInformers.SecretLister().Secrets(c.SigningCA.Namespace).Get(c.SigningCA.Name) - if err != nil && !apierrors.IsNotFound(err) { - return nil, false, err - } - signingCertKeyPairSecret := originalSigningCertKeyPairSecret.DeepCopy() - if apierrors.IsNotFound(err) { - // create an empty one - signingCertKeyPairSecret = &corev1.Secret{ - ObjectMeta: NewTLSArtifactObjectMeta( - c.SigningCA.Name, - c.SigningCA.Namespace, - c.SigningCA.AdditionalAnnotations, - ), - Type: corev1.SecretTypeTLS, - } - creationRequired = true - } - - // run Update if metadata needs changing unless we're in RefreshOnlyWhenExpired mode - if !c.SigningCA.RefreshOnlyWhenExpired { - needsMetadataUpdate := ensureOwnerRefAndTLSAnnotations(signingCertKeyPairSecret, c.SigningCA.Owner, c.SigningCA.AdditionalAnnotations) - needsTypeChange := ensureSecretTLSTypeSet(signingCertKeyPairSecret) - updateRequired = needsMetadataUpdate || needsTypeChange - } - - // run Update if signer content needs changing - signerUpdated := false - if needed, reason := needNewSigningCertKeyPair(signingCertKeyPairSecret, c.SigningCA.Refresh, c.SigningCA.RefreshOnlyWhenExpired); needed || creationRequired { - if creationRequired { - reason = "secret doesn't exist" - } - c.eventRecorder.Eventf("SignerUpdateRequired", "%q in %q requires a new signing cert/key pair: %v", c.SigningCA.Name, c.SigningCA.Namespace, reason) - - ca, err := setSigningCertKeyPairSecret(signingCertKeyPairSecret, c.SigningCA.Validity) - if err != nil { - return nil, false, err - } - setTLSAnnotationsOnSigningCertKeyPairSecret(signingCertKeyPairSecret, ca, c.SigningCA.Refresh, c.SigningCA.AdditionalAnnotations) - - LabelAsManagedSecret(signingCertKeyPairSecret, CertificateTypeSigner) - - updateRequired = true - signerUpdated = true - } - - if creationRequired { - actualSigningCertKeyPairSecret, err := c.kubeClient.CoreV1().Secrets(c.SigningCA.Namespace).Create(ctx, signingCertKeyPairSecret, metav1.CreateOptions{}) - resourcehelper.ReportCreateEvent(c.eventRecorder, actualSigningCertKeyPairSecret, err) - if err != nil { - return nil, false, err - } - klog.V(2).Infof("Created secret %s/%s", actualSigningCertKeyPairSecret.Namespace, actualSigningCertKeyPairSecret.Name) - signingCertKeyPairSecret = actualSigningCertKeyPairSecret - } else if updateRequired { - actualSigningCertKeyPairSecret, err := c.kubeClient.CoreV1().Secrets(c.SigningCA.Namespace).Update(ctx, signingCertKeyPairSecret, metav1.UpdateOptions{}) - if apierrors.IsConflict(err) { - // ignore error if its attempting to update outdated version of the secret - return nil, false, nil - } - resourcehelper.ReportUpdateEvent(c.eventRecorder, actualSigningCertKeyPairSecret, err) - if err != nil { - return nil, false, err - } - klog.V(2).Infof("Updated secret %s/%s", actualSigningCertKeyPairSecret.Namespace, actualSigningCertKeyPairSecret.Name) - signingCertKeyPairSecret = actualSigningCertKeyPairSecret - } - - // at this point, the secret has the correct signer, so we should read that signer to be able to sign - signingCertKeyPair, err := crypto.GetCAFromBytes(signingCertKeyPairSecret.Data["tls.crt"], signingCertKeyPairSecret.Data["tls.key"]) - if err != nil { - return nil, signerUpdated, err - } - - return signingCertKeyPair, signerUpdated, nil -} - // ensureOwnerReference adds the owner to the list of owner references in meta, if necessary func ensureOwnerReference(meta *metav1.ObjectMeta, owner *metav1.OwnerReference) bool { var found bool @@ -231,4 +146,4 @@ func setTLSAnnotationsOnSigningCertKeyPairSecret(signingCertKeyPairSecret *corev tlsAnnotations.NotAfter = ca.Certs[0].NotAfter.Format(time.RFC3339) tlsAnnotations.RefreshPeriod = refresh.String() _ = tlsAnnotations.EnsureTLSMetadataUpdate(&signingCertKeyPairSecret.ObjectMeta) -} \ No newline at end of file +} diff --git a/pkg/operator/certrotation/target.go b/pkg/operator/certrotation/target.go index 7671db77a6..bb06594a4d 100644 --- a/pkg/operator/certrotation/target.go +++ b/pkg/operator/certrotation/target.go @@ -1,22 +1,18 @@ package certrotation import ( - "context" "crypto/x509" "fmt" "strings" "time" corev1 "k8s.io/api/core/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apiserver/pkg/authentication/user" - "k8s.io/klog/v2" "github.com/openshift/library-go/pkg/certs" "github.com/openshift/library-go/pkg/crypto" - "github.com/openshift/library-go/pkg/operator/resource/resourcehelper" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // TargetCertKeyPairConfig holds the configuration for a target certificate key pair. @@ -81,147 +77,6 @@ type SignerCertConfig struct { func (SignerCertConfig) targetCertConfig() {} -func (c CertRotationController) ensureTargetCertKeyPair(ctx context.Context, signingCertKeyPair *crypto.CA, caBundleCerts []*x509.Certificate) (*corev1.Secret, error) { - // at this point our trust bundle has been updated. We don't know for sure that consumers have updated, but that's why we have a second - // validity percentage. We always check to see if we need to sign. Often we are signing with an old key or we have no target - // and need to mint one - // TODO do the cross signing thing, but this shows the API consumers want and a very simple impl. - - creationRequired := false - updateRequired := false - originalTargetCertKeyPairSecret, err := c.kubeInformers.SecretLister().Secrets(c.TargetCert.Namespace).Get(c.TargetCert.Name) - if err != nil && !apierrors.IsNotFound(err) { - return nil, err - } - targetCertKeyPairSecret := originalTargetCertKeyPairSecret.DeepCopy() - if apierrors.IsNotFound(err) { - // create an empty one - targetCertKeyPairSecret = &corev1.Secret{ - ObjectMeta: NewTLSArtifactObjectMeta( - c.TargetCert.Name, - c.TargetCert.Namespace, - c.TargetCert.AdditionalAnnotations, - ), - Type: corev1.SecretTypeTLS, - } - creationRequired = true - } - - // run Update if metadata needs changing unless we're in RefreshOnlyWhenExpired mode - if !c.TargetCert.RefreshOnlyWhenExpired { - needsMetadataUpdate := ensureOwnerRefAndTLSAnnotations(targetCertKeyPairSecret, c.TargetCert.Owner, c.TargetCert.AdditionalAnnotations) - needsTypeChange := ensureSecretTLSTypeSet(targetCertKeyPairSecret) - updateRequired = needsMetadataUpdate || needsTypeChange - } - - // Determine hostnames function for serving certs - var hostnames func() []string - if serving, ok := c.TargetCert.CertConfig.(ServingCertConfig); ok { - hostnames = serving.Hostnames - } - - if reason := needNewTargetCertKeyPair(targetCertKeyPairSecret, signingCertKeyPair, caBundleCerts, c.TargetCert.Refresh, c.TargetCert.RefreshOnlyWhenExpired, creationRequired, hostnames); len(reason) > 0 { - c.eventRecorder.Eventf("TargetUpdateRequired", "%q in %q requires a new target cert/key pair: %v", c.TargetCert.Name, c.TargetCert.Namespace, reason) - - if err := c.setTargetCertKeyPairSecret(targetCertKeyPairSecret, signingCertKeyPair); err != nil { - return nil, err - } - - LabelAsManagedSecret(targetCertKeyPairSecret, CertificateTypeTarget) - - updateRequired = true - } - if creationRequired { - actualTargetCertKeyPairSecret, err := c.kubeClient.CoreV1().Secrets(c.TargetCert.Namespace).Create(ctx, targetCertKeyPairSecret, metav1.CreateOptions{}) - resourcehelper.ReportCreateEvent(c.eventRecorder, actualTargetCertKeyPairSecret, err) - if err != nil { - return nil, err - } - klog.V(2).Infof("Created secret %s/%s", actualTargetCertKeyPairSecret.Namespace, actualTargetCertKeyPairSecret.Name) - targetCertKeyPairSecret = actualTargetCertKeyPairSecret - } else if updateRequired { - actualTargetCertKeyPairSecret, err := c.kubeClient.CoreV1().Secrets(c.TargetCert.Namespace).Update(ctx, targetCertKeyPairSecret, metav1.UpdateOptions{}) - if apierrors.IsConflict(err) { - // ignore error if its attempting to update outdated version of the secret - return nil, nil - } - resourcehelper.ReportUpdateEvent(c.eventRecorder, actualTargetCertKeyPairSecret, err) - if err != nil { - return nil, err - } - klog.V(2).Infof("Updated secret %s/%s", actualTargetCertKeyPairSecret.Namespace, actualTargetCertKeyPairSecret.Name) - targetCertKeyPairSecret = actualTargetCertKeyPairSecret - } - - return targetCertKeyPairSecret, nil -} - -// setTargetCertKeyPairSecret creates a new cert/key pair, sets them in the secret, and applies TLS annotations. -func (c CertRotationController) setTargetCertKeyPairSecret(targetCertKeyPairSecret *corev1.Secret, signer *crypto.CA) error { - if targetCertKeyPairSecret.Annotations == nil { - targetCertKeyPairSecret.Annotations = map[string]string{} - } - if targetCertKeyPairSecret.Data == nil { - targetCertKeyPairSecret.Data = map[string][]byte{} - } - - // our annotation is based on our cert validity, so we want to make sure that we don't specify something past our signer - targetValidity := c.TargetCert.Validity - remainingSignerValidity := signer.Config.Certs[0].NotAfter.Sub(time.Now()) - if remainingSignerValidity < targetValidity { - targetValidity = remainingSignerValidity - } - - var certKeyPair *crypto.TLSCertificateConfig - var err error - switch cfg := c.TargetCert.CertConfig.(type) { - case ClientCertConfig: - certKeyPair, err = signer.MakeClientCertificateForDuration(cfg.UserInfo, targetValidity) - case ServingCertConfig: - if len(cfg.Hostnames()) == 0 { - return fmt.Errorf("no hostnames set") - } - certKeyPair, err = signer.MakeServerCertForDuration(sets.New(cfg.Hostnames()...), targetValidity, cfg.CertificateExtensionFn...) - case SignerCertConfig: - signerName := fmt.Sprintf("%s_@%d", cfg.SignerName, time.Now().Unix()) - certKeyPair, err = crypto.MakeCAConfigForDuration(signerName, targetValidity, signer) - default: - return fmt.Errorf("unknown target cert config type: %T", cfg) - } - if err != nil { - return err - } - - targetCertKeyPairSecret.Data["tls.crt"], targetCertKeyPairSecret.Data["tls.key"], err = certKeyPair.GetPEMBytes() - if err != nil { - return err - } - - // Set TLS annotations - targetCertKeyPairSecret.Annotations[CertificateIssuer] = certKeyPair.Certs[0].Issuer.CommonName - - tlsAnnotations := c.TargetCert.AdditionalAnnotations - tlsAnnotations.NotBefore = certKeyPair.Certs[0].NotBefore.Format(time.RFC3339) - tlsAnnotations.NotAfter = certKeyPair.Certs[0].NotAfter.Format(time.RFC3339) - tlsAnnotations.RefreshPeriod = c.TargetCert.Refresh.String() - _ = tlsAnnotations.EnsureTLSMetadataUpdate(&targetCertKeyPairSecret.ObjectMeta) - - // Set hostname annotations for serving certs - if _, ok := c.TargetCert.CertConfig.(ServingCertConfig); ok { - hostnames := sets.Set[string]{} - for _, ip := range certKeyPair.Certs[0].IPAddresses { - hostnames.Insert(ip.String()) - } - for _, dnsName := range certKeyPair.Certs[0].DNSNames { - hostnames.Insert(dnsName) - } - // List does a sort so that we have a consistent representation - targetCertKeyPairSecret.Annotations[CertificateHostnames] = strings.Join(sets.List(hostnames), ",") - } - - return nil -} - func needNewTargetCertKeyPair(secret *corev1.Secret, signer *crypto.CA, caBundleCerts []*x509.Certificate, refresh time.Duration, refreshOnlyWhenExpired, creationRequired bool, hostnames func() []string) string { if creationRequired { return "secret doesn't exist" @@ -319,4 +174,4 @@ func missingHostnames(annotations map[string]string, hostnames func() []string) } return "" -} \ No newline at end of file +}