Skip to content

Commit

Permalink
read certs from secrets to support external certs (openkruise#1677)
Browse files Browse the repository at this point in the history
Signed-off-by: Kuromesi <[email protected]>
  • Loading branch information
Kuromesi authored Aug 1, 2024
1 parent b19c4d8 commit ee572bf
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 32 deletions.
26 changes: 21 additions & 5 deletions pkg/webhook/util/configuration/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package configuration

import (
"bytes"
"context"
"encoding/json"
"fmt"
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions pkg/webhook/util/controller/webhook_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
7 changes: 6 additions & 1 deletion pkg/webhook/util/crd/crd.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package crd

import (
"bytes"
"context"
"fmt"
"reflect"
Expand Down Expand Up @@ -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
}
Expand Down
49 changes: 38 additions & 11 deletions pkg/webhook/util/writer/external.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand All @@ -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 {
Expand All @@ -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")
Expand All @@ -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 {
Expand Down
47 changes: 32 additions & 15 deletions pkg/webhook/util/writer/external_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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,
Expand All @@ -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()
Expand All @@ -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 {
Expand All @@ -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))
})
Expand All @@ -104,5 +121,5 @@ func RefreshCurrentExternalCertsCache() {
}

func UpdateCurrentExternalCertsCache(externalCerts *generator.Artifacts) {
currentExternalCerts.CACert = externalCerts.CACert
currentExternalCerts = externalCerts
}

0 comments on commit ee572bf

Please sign in to comment.