Skip to content

Commit

Permalink
expose more funcs for etcd cert_rotation
Browse files Browse the repository at this point in the history
In CEO we need to mix and match different CAs with their respective
certs which requires us to open up a couple functions and struct fields.

Another approach would also enable us to run a
client_cert_rotation_controller per node, which would need to open a few
functions and struct fields too.

This commit also adds bundle sorting to avoid CA cert ordering issues causing
constant pod revision rollouts.
  • Loading branch information
tjungblu committed Jan 12, 2024
1 parent 50a38a0 commit 434fa21
Show file tree
Hide file tree
Showing 7 changed files with 29 additions and 23 deletions.
8 changes: 7 additions & 1 deletion pkg/operator/certrotation/cabundle.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package certrotation

import (
"bytes"
"context"
"crypto/x509"
"fmt"
"reflect"
"sort"

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/equality"
Expand Down Expand Up @@ -42,7 +44,7 @@ type CABundleConfigMap struct {
EventRecorder events.Recorder
}

func (c CABundleConfigMap) ensureConfigMapCABundle(ctx context.Context, signingCertKeyPair *crypto.CA) ([]*x509.Certificate, error) {
func (c CABundleConfigMap) EnsureConfigMapCABundle(ctx context.Context, signingCertKeyPair *crypto.CA) ([]*x509.Certificate, error) {
// 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
originalCABundleConfigMap, err := c.Lister.ConfigMaps(c.Namespace).Get(c.Name)
Expand Down Expand Up @@ -139,6 +141,10 @@ func manageCABundleConfigMap(caBundleConfigMap *corev1.ConfigMap, currentSigner
}
}

// sorting ensures we don't continuously swap the certificates in the bundle, which might cause revision rollouts
sort.SliceStable(finalCertificates, func(i, j int) bool {
return bytes.Compare(finalCertificates[i].Raw, finalCertificates[j].Raw) < 0
})
caBytes, err := crypto.EncodeCertificates(finalCertificates...)
if err != nil {
return nil, err
Expand Down
2 changes: 1 addition & 1 deletion pkg/operator/certrotation/cabundle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ func TestEnsureConfigMapCABundle(t *testing.T) {
if err != nil {
t.Fatal(err)
}
_, err = c.ensureConfigMapCABundle(context.TODO(), newCA)
_, err = c.EnsureConfigMapCABundle(context.TODO(), newCA)
switch {
case err != nil && len(test.expectedError) == 0:
t.Error(err)
Expand Down
22 changes: 11 additions & 11 deletions pkg/operator/certrotation/client_cert_rotation_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,9 @@ func (s *StaticPodConditionStatusReporter) Report(ctx context.Context, controlle
// 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 string
// rotatedSigningCASecret rotates a self-signed signing CA stored in a secret.
rotatedSigningCASecret RotatedSigningCASecret
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.
Expand All @@ -81,8 +81,8 @@ func NewCertRotationController(
reporter StatusReporter,
) factory.Controller {
c := &CertRotationController{
name: name,
rotatedSigningCASecret: rotatedSigningCASecret,
Name: name,
RotatedSigningCASecret: rotatedSigningCASecret,
CABundleConfigMap: caBundleConfigMap,
RotatedSelfSignedCertKeySecret: rotatedSelfSignedCertKeySecret,
StatusReporter: reporter,
Expand All @@ -102,15 +102,15 @@ func NewCertRotationController(
}

func (c CertRotationController) Sync(ctx context.Context, syncCtx factory.SyncContext) error {
syncErr := c.syncWorker(ctx)
syncErr := c.SyncWorker(ctx)

// running this function with RunOnceContextKey value context will make this "run-once" without updating status.
isRunOnce, ok := ctx.Value(RunOnceContextKey).(bool)
if ok && isRunOnce {
return syncErr
}

updated, updateErr := c.StatusReporter.Report(ctx, c.name, syncErr)
updated, updateErr := c.StatusReporter.Report(ctx, c.Name, syncErr)
if updateErr != nil {
return updateErr
}
Expand All @@ -121,18 +121,18 @@ func (c CertRotationController) Sync(ctx context.Context, syncCtx factory.SyncCo
return syncErr
}

func (c CertRotationController) syncWorker(ctx context.Context) error {
signingCertKeyPair, err := c.rotatedSigningCASecret.ensureSigningCertKeyPair(ctx)
func (c CertRotationController) SyncWorker(ctx context.Context) error {
signingCertKeyPair, err := c.RotatedSigningCASecret.EnsureSigningCertKeyPair(ctx)
if err != nil {
return err
}

cabundleCerts, err := c.CABundleConfigMap.ensureConfigMapCABundle(ctx, signingCertKeyPair)
cabundleCerts, err := c.CABundleConfigMap.EnsureConfigMapCABundle(ctx, signingCertKeyPair)
if err != nil {
return err
}

if err := c.RotatedSelfSignedCertKeySecret.ensureTargetCertKeyPair(ctx, signingCertKeyPair, cabundleCerts); err != nil {
if _, err := c.RotatedSelfSignedCertKeySecret.EnsureTargetCertKeyPair(ctx, signingCertKeyPair, cabundleCerts); err != nil {
return err
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/operator/certrotation/signer.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ type RotatedSigningCASecret struct {
EventRecorder events.Recorder
}

func (c RotatedSigningCASecret) ensureSigningCertKeyPair(ctx context.Context) (*crypto.CA, error) {
func (c RotatedSigningCASecret) EnsureSigningCertKeyPair(ctx context.Context) (*crypto.CA, error) {
originalSigningCertKeyPairSecret, err := c.Lister.Secrets(c.Namespace).Get(c.Name)
if err != nil && !apierrors.IsNotFound(err) {
return nil, err
Expand Down
2 changes: 1 addition & 1 deletion pkg/operator/certrotation/signer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func TestEnsureSigningCertKeyPair(t *testing.T) {
EventRecorder: events.NewInMemoryRecorder("test"),
}

_, err := c.ensureSigningCertKeyPair(context.TODO())
_, err := c.EnsureSigningCertKeyPair(context.TODO())
switch {
case err != nil && len(test.expectedError) == 0:
t.Error(err)
Expand Down
12 changes: 6 additions & 6 deletions pkg/operator/certrotation/target.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,14 +89,14 @@ type TargetCertRechecker interface {
RecheckChannel() <-chan struct{}
}

func (c RotatedSelfSignedCertKeySecret) ensureTargetCertKeyPair(ctx context.Context, signingCertKeyPair *crypto.CA, caBundleCerts []*x509.Certificate) error {
func (c RotatedSelfSignedCertKeySecret) 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.
originalTargetCertKeyPairSecret, err := c.Lister.Secrets(c.Namespace).Get(c.Name)
if err != nil && !apierrors.IsNotFound(err) {
return err
return nil, err
}
targetCertKeyPairSecret := originalTargetCertKeyPairSecret.DeepCopy()
if apierrors.IsNotFound(err) {
Expand All @@ -120,26 +120,26 @@ func (c RotatedSelfSignedCertKeySecret) ensureTargetCertKeyPair(ctx context.Cont
if needsMetadataUpdate && len(targetCertKeyPairSecret.ResourceVersion) > 0 {
_, _, err := resourceapply.ApplySecret(ctx, c.Client, c.EventRecorder, targetCertKeyPairSecret)
if err != nil {
return err
return nil, err
}
}

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

LabelAsManagedSecret(targetCertKeyPairSecret, CertificateTypeTarget)

actualTargetCertKeyPairSecret, _, err := resourceapply.ApplySecret(ctx, c.Client, c.EventRecorder, targetCertKeyPairSecret)
if err != nil {
return err
return nil, err
}
targetCertKeyPairSecret = actualTargetCertKeyPairSecret
}

return nil
return targetCertKeyPairSecret, nil
}

func needNewTargetCertKeyPair(annotations map[string]string, signer *crypto.CA, caBundleCerts []*x509.Certificate, refresh time.Duration, refreshOnlyWhenExpired bool) string {
Expand Down
4 changes: 2 additions & 2 deletions pkg/operator/certrotation/target_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ func TestEnsureTargetCertKeyPair(t *testing.T) {
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)
Expand Down Expand Up @@ -423,7 +423,7 @@ func TestEnsureTargetSignerCertKeyPair(t *testing.T) {
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)
Expand Down

0 comments on commit 434fa21

Please sign in to comment.