Keep the last CA when creating a new one (hashicorp#287)

Keeps the last CA in the K8s API caBundle when generating a new
CA. Should help cover when follower replicas haven't received a new
leaf cert from the new CA.

helper/cert/source_gen.go

@@ -8,6 +8,7 @@ import (
 	"crypto/elliptic"
 	"crypto/rand"
 	"crypto/sha256"
+	"crypto/tls"
 	"crypto/x509"
 	"crypto/x509/pkix"
 	"encoding/pem"
@@ -21,6 +22,8 @@ import (
 	"github.com/cenkalti/backoff/v4"
 	"github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/vault-k8s/leader"
+	adminv1 "k8s.io/api/admissionregistration/v1"
+	adminv1beta "k8s.io/api/admissionregistration/v1beta1"
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -57,10 +60,12 @@ type GenSource struct {
 	caCertTemplate *x509.Certificate
 	caSigner       crypto.Signer
 
-	K8sClient     kubernetes.Interface
-	Namespace     string
-	SecretsCache  informerv1.SecretInformer
-	LeaderElector *leader.LeaderElector
+	K8sClient       kubernetes.Interface
+	Namespace       string
+	SecretsCache    informerv1.SecretInformer
+	LeaderElector   *leader.LeaderElector
+	WebhookName     string
+	AdminAPIVersion string
 
 	Log hclog.Logger
 }
@@ -102,6 +107,19 @@ func (s *GenSource) Certificate(ctx context.Context, last *Bundle) (Bundle, erro
 		last = nil
 
 		s.Log.Info("Generated CA")
+
+		// Check if there's an existing caBundle on the webhook config
+		oldCAs := s.getExistingCA(ctx)
+		if len(oldCAs) > 0 {
+			bothCerts, err := prependLastCA(s.caCert, oldCAs, s.Log)
+			if err != nil {
+				// If there's an error, don't set s.caCert; just log a warning
+				// and continue on, so that the caCert will be replaced
+				s.Log.Warn("failed to append previous CA cert to new CA cert", "err", err)
+			} else {
+				s.caCert = bothCerts
+			}
+		}
 	}
 
 	// Set the CA cert
@@ -252,6 +270,37 @@ func (s *GenSource) getBundleFromSecret() (Bundle, error) {
 	return bundle, nil
 }
 
+func (s *GenSource) getExistingCA(ctx context.Context) []byte {
+	switch s.AdminAPIVersion {
+	case adminv1.SchemeGroupVersion.Version:
+		cfg, err := s.K8sClient.AdmissionregistrationV1().
+			MutatingWebhookConfigurations().
+			Get(ctx, s.WebhookName, metav1.GetOptions{})
+		if err != nil {
+			s.Log.Warn("failed to fetch v1 mutating webhook config", "WebhookName", s.WebhookName, "err", err)
+			return []byte{}
+		}
+		if len(cfg.Webhooks) > 0 {
+			return cfg.Webhooks[0].ClientConfig.CABundle
+		}
+	case adminv1beta.SchemeGroupVersion.Version:
+		cfg, err := s.K8sClient.AdmissionregistrationV1beta1().
+			MutatingWebhookConfigurations().
+			Get(ctx, s.WebhookName, metav1.GetOptions{})
+		if err != nil {
+			s.Log.Warn("failed to fetch v1beta mutating webhook config", "WebhookName", s.WebhookName, "err", err)
+			return []byte{}
+		}
+		if len(cfg.Webhooks) > 0 {
+			return cfg.Webhooks[0].ClientConfig.CABundle
+		}
+	}
+
+	// At this point either the AdminAPIVersion was unknown, or the CABundle was
+	// empty, so just return an empty slice
+	return []byte{}
+}
+
 func (s *GenSource) expiry() time.Duration {
 	if s.Expiry > 0 {
 		return s.Expiry
@@ -428,3 +477,43 @@ func parseCert(pemValue []byte) (*x509.Certificate, error) {
 
 	return x509.ParseCertificate(block.Bytes)
 }
+
+// decodeCerts decodes a caBundle ([]byte) into a list of certs ([][]byte)
+func decodeCerts(caBundle []byte, log hclog.Logger) tls.Certificate {
+	certs := tls.Certificate{}
+	remainder := caBundle
+	var next *pem.Block
+	for {
+		next, remainder = pem.Decode(remainder)
+		if next == nil {
+			break
+		}
+		if next.Type == "CERTIFICATE" {
+			certs.Certificate = append(certs.Certificate, next.Bytes)
+		} else {
+			log.Warn("unexpected pem block type in caBundle, ignoring", "type", next.Type)
+		}
+	}
+	return certs
+}
+
+// prependLastCA returns a new CA bundle:
+// [0] last CA cert from oldCABundle
+// [1] newCACert
+func prependLastCA(newCACert, oldCABundle []byte, log hclog.Logger) ([]byte, error) {
+	// Decode the certs from the old CA bundle
+	oldCerts := decodeCerts(oldCABundle, log)
+	// Append the old CA if it exists
+	newBundle := []byte{}
+	if len(oldCerts.Certificate) > 0 {
+		last := oldCerts.Certificate[len(oldCerts.Certificate)-1]
+		var buf bytes.Buffer
+		if err := pem.Encode(&buf, &pem.Block{Type: "CERTIFICATE", Bytes: last}); err != nil {
+			return nil, fmt.Errorf("failed to encode old CA cert: %w", err)
+		}
+		newBundle = append(newBundle, buf.Bytes()...)
+	}
+	// Then append the new CA
+	newBundle = append(newBundle, newCACert...)
+	return newBundle, nil
+}

helper/cert/source_gen_test.go

@@ -14,7 +14,10 @@ import (
 
 	"github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/vault-k8s/leader"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	adminv1 "k8s.io/api/admissionregistration/v1"
+	adminv1beta "k8s.io/api/admissionregistration/v1beta1"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/informers"
@@ -237,3 +240,140 @@ func testGetHostname(t *testing.T) string {
 	}
 	return host
 }
+
+func TestGensource_prependLastCA(t *testing.T) {
+	// Construct caBundle's (old and new) to use in the test cases
+	new1 := testGenSource()
+	newBundle1, err := new1.Certificate(context.Background(), nil)
+	require.NoError(t, err)
+	new2 := testGenSource()
+	newBundle2, err := new2.Certificate(context.Background(), nil)
+	require.NoError(t, err)
+
+	old1 := testGenSource()
+	oldBundle1, err := old1.Certificate(context.Background(), nil)
+	require.NoError(t, err)
+	old2 := testGenSource()
+	oldBundle2, err := old2.Certificate(context.Background(), nil)
+	require.NoError(t, err)
+
+	tests := map[string]struct {
+		oldCAs   []byte
+		expected []byte
+	}{
+		"no old CAs": {
+			oldCAs:   nil,
+			expected: newBundle1.CACert,
+		},
+		"one old CA": {
+			oldCAs:   oldBundle1.CACert,
+			expected: append(oldBundle1.CACert, newBundle1.CACert...),
+		},
+		"two old CAs": {
+			oldCAs:   append(oldBundle1.CACert, oldBundle2.CACert...),
+			expected: append(oldBundle2.CACert, newBundle1.CACert...),
+		},
+		"invalid old CAs": {
+			oldCAs:   []byte("not a cert"),
+			expected: newBundle1.CACert,
+		},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			result, err := prependLastCA(newBundle1.CACert, tc.oldCAs, hclog.Default())
+			require.NoError(t, err)
+			assert.Equal(t, tc.expected, result)
+
+			// Run again with the output from previous
+			result2, err := prependLastCA(newBundle2.CACert, result, hclog.Default())
+			require.NoError(t, err)
+			assert.Equal(t, append(newBundle1.CACert, newBundle2.CACert...), result2)
+		})
+	}
+}
+
+func TestGensource_getExistingCA(t *testing.T) {
+	tests := map[string]struct {
+		existingBundle []byte
+		expectBundle   []byte
+	}{
+		"no existing CA": {
+			existingBundle: nil,
+			expectBundle:   nil,
+		},
+		"one existing CA": {
+			existingBundle: []byte("exists"),
+			expectBundle:   []byte("exists"),
+		},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			s := testGenSource()
+			s.WebhookName = "test"
+
+			betaCfg := &adminv1beta.MutatingWebhookConfiguration{
+				ObjectMeta: metav1.ObjectMeta{Name: s.WebhookName},
+				Webhooks: []adminv1beta.MutatingWebhook{
+					{
+						ClientConfig: adminv1beta.WebhookClientConfig{
+							CABundle: tc.existingBundle,
+						},
+					},
+				},
+			}
+			v1Cfg := &adminv1.MutatingWebhookConfiguration{
+				ObjectMeta: metav1.ObjectMeta{Name: s.WebhookName},
+				Webhooks: []adminv1.MutatingWebhook{
+					{
+						ClientConfig: adminv1.WebhookClientConfig{
+							CABundle: tc.existingBundle,
+						},
+					},
+				},
+			}
+			t.Run("v1", func(t *testing.T) {
+				s.AdminAPIVersion = adminv1.SchemeGroupVersion.Version
+				s.K8sClient = fake.NewSimpleClientset(v1Cfg)
+				result := s.getExistingCA(context.Background())
+				assert.Equal(t, tc.expectBundle, result)
+			})
+			t.Run("v1beta1", func(t *testing.T) {
+				s.AdminAPIVersion = adminv1beta.SchemeGroupVersion.Version
+				s.K8sClient = fake.NewSimpleClientset(betaCfg)
+				result := s.getExistingCA(context.Background())
+				assert.Equal(t, tc.expectBundle, result)
+			})
+		})
+	}
+
+	t.Run("unknown admin API version", func(t *testing.T) {
+		s := testGenSource()
+		s.AdminAPIVersion = "invalid"
+		s.K8sClient = fake.NewSimpleClientset()
+		result := s.getExistingCA(context.Background())
+		assert.Empty(t, result)
+	})
+	t.Run("no caBundle v1", func(t *testing.T) {
+		s := testGenSource()
+		s.WebhookName = "test"
+		s.AdminAPIVersion = adminv1.SchemeGroupVersion.Version
+		s.K8sClient = fake.NewSimpleClientset(&adminv1.MutatingWebhookConfiguration{
+			ObjectMeta: metav1.ObjectMeta{Name: s.WebhookName},
+		})
+		result := s.getExistingCA(context.Background())
+		assert.Empty(t, result)
+	})
+	t.Run("no caBundle v1beta1", func(t *testing.T) {
+		s := testGenSource()
+		s.WebhookName = "test"
+		s.AdminAPIVersion = adminv1beta.SchemeGroupVersion.Version
+		s.K8sClient = fake.NewSimpleClientset(&adminv1beta.MutatingWebhookConfiguration{
+			ObjectMeta: metav1.ObjectMeta{Name: s.WebhookName},
+		})
+		result := s.getExistingCA(context.Background())
+		assert.Empty(t, result)
+	})
+
+}

subcommand/injector/command.go

@@ -21,6 +21,8 @@ import (
 	"github.com/hashicorp/vault-k8s/leader"
 	"github.com/mitchellh/cli"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
+	adminv1 "k8s.io/api/admissionregistration/v1"
+	adminv1beta "k8s.io/api/admissionregistration/v1beta1"
 	k8sErrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -135,15 +137,22 @@ func (c *Command) Run(args []string) int {
 		Level:      level,
 		JSONFormat: (c.flagLogFormat == "json")})
 
+	adminAPIVersion, err := getAdminAPIVersion(ctx, clientset)
+	if err != nil {
+		logger.Warn(fmt.Sprintf("failed to determine Admissionregistration API version, defaulting to %s", adminAPIVersion), "error", err)
+	}
+
 	// Determine where to source the certificates from
 	var certSource cert.Source = &cert.GenSource{
-		Name:          "Agent Inject",
-		Hosts:         strings.Split(c.flagAutoHosts, ","),
-		K8sClient:     clientset,
-		Namespace:     namespace,
-		SecretsCache:  secrets,
-		LeaderElector: leaderElector,
-		Log:           logger.Named("auto-tls"),
+		Name:            "Agent Inject",
+		Hosts:           strings.Split(c.flagAutoHosts, ","),
+		K8sClient:       clientset,
+		Namespace:       namespace,
+		SecretsCache:    secrets,
+		LeaderElector:   leaderElector,
+		WebhookName:     c.flagAutoName,
+		AdminAPIVersion: adminAPIVersion,
+		Log:             logger.Named("auto-tls"),
 	}
 	if c.flagCertFile != "" {
 		certSource = &cert.DiskSource{
@@ -157,7 +166,7 @@ func (c *Command) Run(args []string) int {
 	certCh := make(chan cert.Bundle)
 	certNotify := cert.NewNotify(ctx, certCh, certSource, logger.Named("notify"))
 	go certNotify.Run()
-	go c.certWatcher(ctx, certCh, clientset, logger.Named("certwatcher"))
+	go c.certWatcher(ctx, certCh, clientset, adminAPIVersion, logger.Named("certwatcher"))
 
 	// Build the HTTP handler and server
 	injector := agentInject.Handler{
@@ -255,22 +264,18 @@ func (c *Command) getCertificate(*tls.ClientHelloInfo) (*tls.Certificate, error)
 }
 
 func getAdminAPIVersion(ctx context.Context, clientset *kubernetes.Clientset) (string, error) {
-	adminAPIVersion := "v1"
+	adminAPIVersion := adminv1.SchemeGroupVersion.Version
 	_, err := clientset.AdmissionregistrationV1().
 		MutatingWebhookConfigurations().
 		List(ctx, metav1.ListOptions{})
 	if k8sErrors.IsNotFound(err) {
-		adminAPIVersion = "v1beta1"
+		adminAPIVersion = adminv1beta.SchemeGroupVersion.Version
 	}
 	return adminAPIVersion, err
 }
 
-func (c *Command) certWatcher(ctx context.Context, ch <-chan cert.Bundle, clientset *kubernetes.Clientset, log hclog.Logger) {
+func (c *Command) certWatcher(ctx context.Context, ch <-chan cert.Bundle, clientset *kubernetes.Clientset, adminAPIVersion string, log hclog.Logger) {
 	var bundle cert.Bundle
-	adminAPIVersion, err := getAdminAPIVersion(ctx, clientset)
-	if err != nil {
-		log.Warn(fmt.Sprintf("failed to determine Admissionregistration API version, defaulting to %s", adminAPIVersion), "error", err)
-	}
 
 	for {
 		select {