diff --git a/pkg/webhook/util/configuration/configuration.go b/pkg/webhook/util/configuration/configuration.go index e9d985f771..d141e3a2a8 100644 --- a/pkg/webhook/util/configuration/configuration.go +++ b/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 diff --git a/pkg/webhook/util/controller/webhook_controller.go b/pkg/webhook/util/controller/webhook_controller.go index 2c5435e7e7..981e94702d 100644 --- a/pkg/webhook/util/controller/webhook_controller.go +++ b/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{ diff --git a/pkg/webhook/util/crd/crd.go b/pkg/webhook/util/crd/crd.go index 2949679f3a..94bd83c682 100644 --- a/pkg/webhook/util/crd/crd.go +++ b/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 } diff --git a/pkg/webhook/util/writer/external.go b/pkg/webhook/util/writer/external.go index 7330b5f251..87403701c8 100644 --- a/pkg/webhook/util/writer/external.go +++ b/pkg/webhook/util/writer/external.go @@ -20,9 +20,11 @@ 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" @@ -30,19 +32,26 @@ import ( ) 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 { diff --git a/pkg/webhook/util/writer/external_test.go b/pkg/webhook/util/writer/external_test.go index d6afaecdd9..87faa69bd5 100644 --- a/pkg/webhook/util/writer/external_test.go +++ b/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 }