Auto-rotate serving certificate

The rotation is forced by a new controller that deletes the serving cert
secret, as other controllers will see this deletion and ensure that a new
serving cert is created.

Note that the integration tests now have an addition worst case runtime of
60 seconds. This is because of the way that the aggregated API server code
reloads certificates. We will fix this in a future story. Then, the
integration tests should hopefully get much faster.

Signed-off-by: Andrew Keesler <akeesler@vmware.com>

internal/controller/apicerts/certs_expirer.go

@@ -0,0 +1,128 @@
+/*
+Copyright 2020 VMware, Inc.
+SPDX-License-Identifier: Apache-2.0
+*/
+
+package apicerts
+
+import (
+	"crypto/x509"
+	"encoding/pem"
+	"fmt"
+	"time"
+
+	corev1 "k8s.io/api/core/v1"
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	corev1informers "k8s.io/client-go/informers/core/v1"
+	"k8s.io/client-go/kubernetes"
+	"k8s.io/klog/v2"
+
+	"github.com/suzerain-io/controller-go"
+	"github.com/suzerain-io/placeholder-name/internal/constable"
+	placeholdernamecontroller "github.com/suzerain-io/placeholder-name/internal/controller"
+)
+
+type certsExpirerController struct {
+	namespace      string
+	k8sClient      kubernetes.Interface
+	secretInformer corev1informers.SecretInformer
+
+	// ageThreshold is a percentage (i.e., a real number between 0 and 1,
+	// inclusive) indicating the point in a certificate's lifetime where this
+	// controller will start to try to rotate it.
+	//
+	// Said another way, once ageThreshold % of a certificate's lifetime has
+	// passed, this controller will try to delete it to force a new certificate
+	// to be created.
+	ageThreshold float32
+}
+
+// NewCertsExpirerController returns a controller.Controller that will delete a
+// CA once it gets within some threshold of its expiration time.
+func NewCertsExpirerController(
+	namespace string,
+	k8sClient kubernetes.Interface,
+	secretInformer corev1informers.SecretInformer,
+	withInformer placeholdernamecontroller.WithInformerOptionFunc,
+	ageThreshold float32,
+) controller.Controller {
+	return controller.New(
+		controller.Config{
+			Name: "certs-expirer-controller",
+			Syncer: &certsExpirerController{
+				namespace:      namespace,
+				k8sClient:      k8sClient,
+				secretInformer: secretInformer,
+				ageThreshold:   ageThreshold,
+			},
+		},
+		withInformer(
+			secretInformer,
+			placeholdernamecontroller.NameAndNamespaceExactMatchFilterFactory(certsSecretName, namespace),
+			controller.InformerOption{},
+		),
+	)
+}
+
+// Sync implements controller.Syncer.Sync.
+func (c *certsExpirerController) Sync(ctx controller.Context) error {
+	secret, err := c.secretInformer.Lister().Secrets(c.namespace).Get(certsSecretName)
+	notFound := k8serrors.IsNotFound(err)
+	if err != nil && !notFound {
+		return fmt.Errorf("failed to get %s/%s secret: %w", c.namespace, certsSecretName, err)
+	}
+	if notFound {
+		klog.Info("certsExpirerController Sync() found that the secret does not exist yet or was deleted")
+		return nil
+	}
+
+	notBefore, notAfter, err := getCABounds(secret)
+	if err != nil {
+		// If we can't get the CA, then really all we can do is log something, since
+		// if we returned an error then the controller lib would just call us again
+		// and again, which would probably yield the same results.
+		klog.Warningf("certsExpirerController Sync() found that the secret is malformed: %s", err.Error())
+		return nil
+	}
+
+	caLifetime := notAfter.Sub(notBefore)
+	caAge := time.Since(notBefore)
+	thresholdDelta := (float32(caAge) / float32(caLifetime)) - c.ageThreshold
+	klog.Infof("certsExpirerController Sync() found a CA age threshold delta of %.2f", thresholdDelta)
+	if thresholdDelta > 0 {
+		err := c.k8sClient.
+			CoreV1().
+			Secrets(c.namespace).
+			Delete(ctx.Context, certsSecretName, metav1.DeleteOptions{})
+		if err != nil {
+			// Do return an error here so that the controller library will reschedule
+			// us to try deleting this cert again.
+			return err
+		}
+	}
+
+	return nil
+}
+
+// getCABounds returns the NotBefore and NotAfter fields of the CA certificate
+// in the provided secret, or an error. Not that it expects the provided secret
+// to contain the well-known data keys from this package (see certs_manager.go).
+func getCABounds(secret *corev1.Secret) (time.Time, time.Time, error) {
+	caPEM := secret.Data[caCertificateSecretKey]
+	if caPEM == nil {
+		return time.Time{}, time.Time{}, constable.Error("failed to find CA")
+	}
+
+	caBlock, _ := pem.Decode(caPEM)
+	if caBlock == nil {
+		return time.Time{}, time.Time{}, constable.Error("failed to decode CA PEM")
+	}
+
+	caCrt, err := x509.ParseCertificate(caBlock.Bytes)
+	if err != nil {
+		return time.Time{}, time.Time{}, fmt.Errorf("failed to parse CA: %w", err)
+	}
+
+	return caCrt.NotBefore, caCrt.NotAfter, nil
+}

internal/controller/apicerts/certs_expirer_test.go

@@ -0,0 +1,269 @@
+/*
+Copyright 2020 VMware, Inc.
+SPDX-License-Identifier: Apache-2.0
+*/
+
+package apicerts
+
+import (
+	"context"
+	"crypto/rand"
+	"crypto/rsa"
+	"crypto/x509"
+	"errors"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	kubeinformers "k8s.io/client-go/informers"
+	kubernetesfake "k8s.io/client-go/kubernetes/fake"
+	kubetesting "k8s.io/client-go/testing"
+
+	"github.com/suzerain-io/controller-go"
+	"github.com/suzerain-io/placeholder-name/internal/testutil"
+)
+
+func TestExpirerControllerFilters(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name      string
+		namespace string
+		secret    corev1.Secret
+		want      bool
+	}{
+		{
+			name:      "good name, good namespace",
+			namespace: "good-namespace",
+			secret: corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "api-serving-cert",
+					Namespace: "good-namespace",
+				},
+			},
+			want: true,
+		},
+		{
+			name:      "bad name, good namespace",
+			namespace: "good-namespacee",
+			secret: corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "bad-name",
+					Namespace: "good-namespace",
+				},
+			},
+			want: false,
+		},
+		{
+			name:      "good name, bad namespace",
+			namespace: "good-namespacee",
+			secret: corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "api-serving-cert",
+					Namespace: "bad-namespace",
+				},
+			},
+			want: false,
+		},
+		{
+			name:      "bad name, bad namespace",
+			namespace: "good-namespacee",
+			secret: corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "bad-name",
+					Namespace: "bad-namespace",
+				},
+			},
+			want: false,
+		},
+	}
+	for _, test := range tests {
+		test := test
+		t.Run(test.name+"-"+test.namespace, func(t *testing.T) {
+			t.Parallel()
+
+			secretsInformer := kubeinformers.NewSharedInformerFactory(
+				kubernetesfake.NewSimpleClientset(),
+				0,
+			).Core().V1().Secrets()
+			withInformer := testutil.NewObservableWithInformerOption()
+			_ = NewCertsExpirerController(
+				test.namespace,
+				nil, // k8sClient, not needed
+				secretsInformer,
+				withInformer.WithInformer,
+				0, // ageThreshold, not needed
+			)
+
+			unrelated := corev1.Secret{}
+			filter := withInformer.GetFilterForInformer(secretsInformer)
+			require.Equal(t, test.want, filter.Add(&test.secret))
+			require.Equal(t, test.want, filter.Update(&unrelated, &test.secret))
+			require.Equal(t, test.want, filter.Update(&test.secret, &unrelated))
+			require.Equal(t, test.want, filter.Delete(&test.secret))
+		})
+	}
+}
+
+func TestExpirerControllerSync(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name                string
+		ageThreshold        float32
+		fillSecretData      func(*testing.T, map[string][]byte)
+		configKubeAPIClient func(*kubernetesfake.Clientset)
+		wantDelete          bool
+		wantError           string
+	}{
+		{
+			name:       "secret does not exist",
+			wantDelete: false,
+		},
+		{
+			name:           "secret missing key",
+			fillSecretData: func(t *testing.T, m map[string][]byte) {},
+			wantDelete:     false,
+		},
+		{
+			name:         "lifetime below threshold",
+			ageThreshold: 0.7,
+			fillSecretData: func(t *testing.T, m map[string][]byte) {
+				caPEM, err := testutil.CreateCertificate(
+					time.Now().Add(-5*time.Hour),
+					time.Now().Add(5*time.Hour),
+				)
+				require.NoError(t, err)
+
+				// See cert_manager.go for this constant.
+				m["caCertificate"] = caPEM
+			},
+			wantDelete: false,
+		},
+		{
+			name:         "lifetime above threshold",
+			ageThreshold: 0.3,
+			fillSecretData: func(t *testing.T, m map[string][]byte) {
+				caPEM, err := testutil.CreateCertificate(
+					time.Now().Add(-5*time.Hour),
+					time.Now().Add(5*time.Hour),
+				)
+				require.NoError(t, err)
+
+				// See cert_manager.go for this constant.
+				m["caCertificate"] = caPEM
+			},
+			wantDelete: true,
+		},
+		{
+			name:         "delete failure",
+			ageThreshold: 0.3,
+			fillSecretData: func(t *testing.T, m map[string][]byte) {
+				caPEM, err := testutil.CreateCertificate(
+					time.Now().Add(-5*time.Hour),
+					time.Now().Add(5*time.Hour),
+				)
+				require.NoError(t, err)
+
+				// See cert_manager.go for this constant.
+				m["caCertificate"] = caPEM
+			},
+			configKubeAPIClient: func(c *kubernetesfake.Clientset) {
+				c.PrependReactor("delete", "secrets", func(_ kubetesting.Action) (bool, runtime.Object, error) {
+					return true, nil, errors.New("delete failed: some delete error")
+				})
+			},
+			wantError: "delete failed: some delete error",
+		},
+		{
+			name: "parse cert failure",
+			fillSecretData: func(t *testing.T, m map[string][]byte) {
+				privateKey, err := rsa.GenerateKey(rand.Reader, 2048)
+				require.NoError(t, err)
+
+				// See cert_manager.go for this constant.
+				m["caCertificate"] = x509.MarshalPKCS1PrivateKey(privateKey)
+			},
+			wantDelete: false,
+		},
+	}
+	for _, test := range tests {
+		test := test
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+
+			ctx, cancel := context.WithTimeout(context.Background(), time.Second*3)
+			defer cancel()
+
+			kubeAPIClient := kubernetesfake.NewSimpleClientset()
+			if test.configKubeAPIClient != nil {
+				test.configKubeAPIClient(kubeAPIClient)
+			}
+
+			kubeInformerClient := kubernetesfake.NewSimpleClientset()
+			name := "api-serving-cert" // See cert_manager.go.
+			namespace := "some-namespace"
+			if test.fillSecretData != nil {
+				secret := &corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      name,
+						Namespace: namespace,
+					},
+					Data: map[string][]byte{},
+				}
+				test.fillSecretData(t, secret.Data)
+
+				require.NoError(t, kubeAPIClient.Tracker().Add(secret))
+				require.NoError(t, kubeInformerClient.Tracker().Add(secret))
+			}
+
+			kubeInformers := kubeinformers.NewSharedInformerFactory(
+				kubeInformerClient,
+				0,
+			)
+
+			c := NewCertsExpirerController(
+				namespace,
+				kubeAPIClient,
+				kubeInformers.Core().V1().Secrets(),
+				controller.WithInformer,
+				test.ageThreshold,
+			)
+
+			// Must start informers before calling TestRunSynchronously().
+			kubeInformers.Start(ctx.Done())
+			controller.TestRunSynchronously(t, c)
+
+			err := controller.TestSync(t, c, controller.Context{
+				Context: ctx,
+			})
+			if test.wantError != "" {
+				require.EqualError(t, err, test.wantError)
+				return
+			}
+			require.NoError(t, err)
+
+			exActions := []kubetesting.Action{}
+			if test.wantDelete {
+				exActions = append(
+					exActions,
+					kubetesting.NewDeleteAction(
+						schema.GroupVersionResource{
+							Group:    "",
+							Version:  "v1",
+							Resource: "secrets",
+						},
+						namespace,
+						name,
+					),
+				)
+			}
+			acActions := kubeAPIClient.Actions()
+			require.Equal(t, exActions, acActions)
+		})
+	}
+}

internal/controller/apicerts/doc.go

@@ -0,0 +1,7 @@
+/*
+Copyright 2020 VMware, Inc.
+SPDX-License-Identifier: Apache-2.0
+*/
+
+// Package apicerts contains controllers that work together to provide rotating API certs.
+package apicerts