read certs from secrets to support external certs (#1677)

Signed-off-by: Kuromesi <blackfacepan@163.com>

pkg/webhook/util/configuration/configuration.go

@@ -17,6 +17,7 @@ limitations under the License.
 package configuration
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
 	"fmt"
@@ -52,15 +53,30 @@ func Ensure(kubeClient clientset.Interface, handlers map[string]types.HandlerGet
 	if utilfeature.DefaultFeatureGate.Enabled(features.EnableExternalCerts) {
 		// if using external certs, only check the caBundle of webhook
 		for _, wh := range mutatingConfig.Webhooks {
-			if wh.ClientConfig.CABundle == nil {
-				return fmt.Errorf("caBundle of MutatingWebhookConfiguration %s is nil", mutatingWebhookConfigurationName)
-
+			path, err := getPath(&wh.ClientConfig)
+			if err != nil {
+				return err
+			}
+			if _, ok := handlers[path]; !ok {
+				klog.Warningf("Ignore webhook for %s in configuration", path)
+				continue
+			}
+			if !bytes.Equal(wh.ClientConfig.CABundle, caBundle) {
+				return fmt.Errorf("caBundle of MutatingWebhookConfiguration %s does not match the external caBundle", mutatingWebhookConfigurationName)
 			}
 		}
 
 		for _, wh := range validatingConfig.Webhooks {
-			if wh.ClientConfig.CABundle == nil {
-				return fmt.Errorf("caBundle of ValidatingWebhookConfiguration %s is nil", mutatingWebhookConfigurationName)
+			path, err := getPath(&wh.ClientConfig)
+			if err != nil {
+				return err
+			}
+			if _, ok := handlers[path]; !ok {
+				klog.Warningf("Ignore webhook for %s in configuration", path)
+				continue
+			}
+			if !bytes.Equal(wh.ClientConfig.CABundle, caBundle) {
+				return fmt.Errorf("caBundle of ValidatingWebhookConfiguration %s does not match the external caBundle", validatingWebhookConfigurationName)
 			}
 		}
 		return nil

pkg/webhook/util/controller/webhook_controller.go

@@ -238,6 +238,7 @@ func (c *Controller) sync() error {
 	if utilfeature.DefaultFeatureGate.Enabled(features.EnableExternalCerts) {
 		certWriter, err = writer.NewExternalCertWriter(writer.ExternalCertWriterOptions{
 			Clientset: c.kubeClient,
+			Secret:    &types.NamespacedName{Namespace: webhookutil.GetNamespace(), Name: webhookutil.GetSecretName()},
 		})
 	} else if certWriterType == writer.FsCertWriter || (len(certWriterType) == 0 && len(webhookutil.GetHost()) != 0) {
 		certWriter, err = writer.NewFSCertWriter(writer.FSCertWriterOptions{

pkg/webhook/util/crd/crd.go

@@ -17,6 +17,7 @@ limitations under the License.
 package crd
 
 import (
+	"bytes"
 	"context"
 	"fmt"
 	"reflect"
@@ -60,9 +61,13 @@ func Ensure(client apiextensionsclientset.Interface, lister apiextensionslisters
 				continue
 			}
 
-			if crd.Spec.Conversion.Webhook == nil || crd.Spec.Conversion.Webhook.ClientConfig == nil || crd.Spec.Conversion.Webhook.ClientConfig.CABundle == nil {
+			if crd.Spec.Conversion.Webhook == nil || crd.Spec.Conversion.Webhook.ClientConfig == nil {
 				return fmt.Errorf("bad conversion configuration of CRD %s", crd.Name)
 			}
+
+			if !bytes.Equal(crd.Spec.Conversion.Webhook.ClientConfig.CABundle, caBundle) {
+				return fmt.Errorf("caBundle of CRD %s does not match external caBundle", crd.Name)
+			}
 		}
 		return nil
 	}

pkg/webhook/util/writer/external.go

@@ -20,29 +20,38 @@ import (
 	"bytes"
 	"context"
 	"errors"
-	"fmt"
 
+	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/types"
 	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/klog/v2"
 
 	"github.com/openkruise/kruise/pkg/webhook/util/generator"
 )
 
 const (
-	mutatingWebhookConfigurationName = "kruise-mutating-webhook-configuration"
+	ExternalCertWriter = "external"
+	ExternalCACert     = "ca.crt"
+	ExternalCAKey      = "ca.key"
+	ExternalServerCert = "tls.crt"
+	ExternalServerKey  = "tls.key"
 )
 
 var currentExternalCerts *generator.Artifacts
 
-// externalCertWriter provisions the certificate by reading from the k8s mutating webhook configuration.
+// externalCertWriter provisions the certificate by reading from the k8s secrets.
 type externalCertWriter struct {
 	*ExternalCertWriterOptions
 }
 
 // ExternalCertWriterOptions is options for constructing a externalCertWriter.
 type ExternalCertWriterOptions struct {
+	// client talks to a kubernetes cluster for creating the secret.
 	Clientset clientset.Interface
+	// secret points the secret that contains certificates that written by the CertWriter.
+	Secret *types.NamespacedName
 }
 
 var _ CertWriter = &externalCertWriter{}
@@ -51,9 +60,13 @@ func (ops *ExternalCertWriterOptions) validate() error {
 	if ops.Clientset == nil {
 		return errors.New("client must be set in externalCertWriterOptions")
 	}
+	if ops.Secret == nil {
+		return errors.New("secret must be set in externalCertWriterOptions")
+	}
 	return nil
 }
 
+// NewExternalCertWriter constructs a CertWriter that persists the certificate in a k8s secret.
 func NewExternalCertWriter(ops ExternalCertWriterOptions) (CertWriter, error) {
 	err := ops.validate()
 	if err != nil {
@@ -62,12 +75,14 @@ func NewExternalCertWriter(ops ExternalCertWriterOptions) (CertWriter, error) {
 	return &externalCertWriter{ExternalCertWriterOptions: &ops}, nil
 }
 
+// EnsureCert read and validate certs from k8s secret.
 func (s *externalCertWriter) EnsureCert(dnsName string) (*generator.Artifacts, bool, error) {
-	// Read certs from mutating webhook configuration generated by external
+	// Read certs from secrets generated externally
 	certs, err := s.read()
 	if err != nil {
 		return nil, false, err
 	}
+
 	// check if the certs are updated since last read
 	if currentExternalCerts != nil && compareCerts(certs, currentExternalCerts) {
 		klog.Info("external certs are not updated")
@@ -89,16 +104,28 @@ func (s *externalCertWriter) overwrite(resourceVersion string) (*generator.Artif
 }
 
 func (s *externalCertWriter) read() (*generator.Artifacts, error) {
-	mutatingConfig, err := s.Clientset.AdmissionregistrationV1().MutatingWebhookConfigurations().Get(context.TODO(), mutatingWebhookConfigurationName, metav1.GetOptions{})
-	if err != nil {
-		return nil, fmt.Errorf("not found MutatingWebhookConfiguration %s", mutatingWebhookConfigurationName)
+	secret, err := s.Clientset.CoreV1().Secrets(s.Secret.Namespace).Get(context.TODO(), s.Secret.Name, metav1.GetOptions{})
+	if apierrors.IsNotFound(err) {
+		return nil, notFoundError{err}
 	}
-	if len(mutatingConfig.Webhooks) == 0 {
-		return nil, fmt.Errorf("not found webhook in MutatingWebhookConfiguration %s", mutatingWebhookConfigurationName)
+	if err != nil {
+		return nil, err
 	}
+	certs := externalSecretToCerts(secret)
+	return certs, nil
+}
 
-	caBundle := mutatingConfig.Webhooks[0].ClientConfig.CABundle
-	return &generator.Artifacts{CACert: caBundle}, nil
+func externalSecretToCerts(secret *corev1.Secret) *generator.Artifacts {
+	ret := &generator.Artifacts{
+		ResourceVersion: secret.ResourceVersion,
+	}
+	if secret.Data != nil {
+		ret.CACert = secret.Data[ExternalCACert]
+		ret.CAKey = secret.Data[ExternalCAKey]
+		ret.Cert = secret.Data[ExternalServerCert]
+		ret.Key = secret.Data[ExternalServerKey]
+	}
+	return ret
 }
 
 func compareCerts(certsA, certsB *generator.Artifacts) bool {

pkg/webhook/util/writer/external_test.go

@@ -21,9 +21,11 @@ import (
 	"testing"
 
 	"github.com/onsi/gomega"
+	webhookutil "github.com/openkruise/kruise/pkg/webhook/util"
 	"github.com/openkruise/kruise/pkg/webhook/util/generator"
-	v1 "k8s.io/api/admissionregistration/v1"
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes/fake"
 )
 
@@ -36,11 +38,13 @@ func TestEnsureCert(t *testing.T) {
 	teatCases := []struct {
 		name                 string
 		needRefreshCertCache bool
+		secret               types.NamespacedName
 		expect               ExpectOut
 	}{
 		{
-			name:                 "test get certs from mutating webhook configuration",
+			name:                 "test get certs from secret",
 			needRefreshCertCache: true,
+			secret:               types.NamespacedName{Namespace: webhookutil.GetNamespace(), Name: webhookutil.GetSecretName()},
 			expect: ExpectOut{
 				changed:      true,
 				errorHappens: false,
@@ -49,11 +53,21 @@ func TestEnsureCert(t *testing.T) {
 		{
 			name:                 "test get certs from cache",
 			needRefreshCertCache: false,
+			secret:               types.NamespacedName{Namespace: webhookutil.GetNamespace(), Name: webhookutil.GetSecretName()},
 			expect: ExpectOut{
 				changed:      false,
 				errorHappens: false,
 			},
 		},
+		{
+			name:                 "test secret not found",
+			needRefreshCertCache: true,
+			secret:               types.NamespacedName{Namespace: "default", Name: "not-existed-secret"},
+			expect: ExpectOut{
+				changed:      false,
+				errorHappens: true,
+			},
+		},
 	}
 	dnsName := "kruise-webhook-service.svc"
 	client := fake.NewSimpleClientset()
@@ -63,20 +77,23 @@ func TestEnsureCert(t *testing.T) {
 	// certs expire after 10 years
 	certs, err := certGenerator.Generate(dnsName)
 	g.Expect(err).NotTo(gomega.HaveOccurred())
-	wh := &v1.MutatingWebhookConfiguration{
+	secret := corev1.Secret{
+		TypeMeta: metav1.TypeMeta{
+			APIVersion: "v1",
+			Kind:       "Secret",
+		},
 		ObjectMeta: metav1.ObjectMeta{
-			Name: mutatingWebhookConfigurationName,
+			Namespace: webhookutil.GetNamespace(),
+			Name:      webhookutil.GetSecretName(),
 		},
-		Webhooks: []v1.MutatingWebhook{
-			{
-				ClientConfig: v1.WebhookClientConfig{
-					CABundle: certs.CACert,
-				},
-			},
+		Data: map[string][]byte{
+			ExternalCAKey:      certs.CAKey,
+			ExternalCACert:     certs.CACert,
+			ExternalServerKey:  certs.Key,
+			ExternalServerCert: certs.Cert,
 		},
 	}
-
-	_, err = client.AdmissionregistrationV1().MutatingWebhookConfigurations().Create(context.TODO(), wh, metav1.CreateOptions{})
+	_, err = client.CoreV1().Secrets(webhookutil.GetNamespace()).Create(context.TODO(), &secret, metav1.CreateOptions{})
 	g.Expect(err).NotTo(gomega.HaveOccurred())
 
 	for _, tc := range teatCases {
@@ -89,10 +106,10 @@ func TestEnsureCert(t *testing.T) {
 
 			externalCertWriter, err := NewExternalCertWriter(ExternalCertWriterOptions{
 				Clientset: client,
+				Secret:    &types.NamespacedName{Namespace: tc.secret.Namespace, Name: tc.secret.Name},
 			})
 			g.Expect(err).NotTo(gomega.HaveOccurred())
-			externalCerts, changed, err := externalCertWriter.EnsureCert(dnsName)
-			g.Expect(externalCerts.CACert).Should(gomega.Equal(certs.CACert))
+			_, changed, err := externalCertWriter.EnsureCert(dnsName)
 			g.Expect(changed).Should(gomega.Equal(tc.expect.changed))
 			g.Expect(err != nil).Should(gomega.Equal(tc.expect.errorHappens))
 		})
@@ -104,5 +121,5 @@ func RefreshCurrentExternalCertsCache() {
 }
 
 func UpdateCurrentExternalCertsCache(externalCerts *generator.Artifacts) {
-	currentExternalCerts.CACert = externalCerts.CACert
+	currentExternalCerts = externalCerts
 }