add EnableExternalCerts featuregate

Signed-off-by: kuromesi <[email protected]>
openkruise · Jul 11, 2024 · 68011dd · 68011dd
1 parent 88c5b73
commit 68011dd
Show file tree

Hide file tree

Showing 8 changed files with 159 additions and 87 deletions.
diff --git a/apis/apps/v1alpha1/daemonset_types.go b/apis/apps/v1alpha1/daemonset_types.go
@@ -91,7 +91,7 @@ type RollingUpdateDaemonSet struct {
 	// pod is available (Ready for at least minReadySeconds) the old DaemonSet pod
 	// on that node is marked deleted. If the old pod becomes unavailable for any
 	// reason (Ready transitions to false, is evicted, or is drained) an updated
-	// pod is immediatedly created on that node without considering surge limits.
+	// pod is immediately created on that node without considering surge limits.
 	// Allowing surge implies the possibility that the resources consumed by the
 	// daemonset on any given node can double if the readiness check fails, and
 	// so resource intensive daemonsets should take into account that they may

diff --git a/config/crd/bases/apps.kruise.io_daemonsets.yaml b/config/crd/bases/apps.kruise.io_daemonsets.yaml
@@ -253,7 +253,7 @@ spec:
                           pod is available (Ready for at least minReadySeconds) the old DaemonSet pod
                           on that node is marked deleted. If the old pod becomes unavailable for any
                           reason (Ready transitions to false, is evicted, or is drained) an updated
-                          pod is immediatedly created on that node without considering surge limits.
+                          pod is immediately created on that node without considering surge limits.
                           Allowing surge implies the possibility that the resources consumed by the
                           daemonset on any given node can double if the readiness check fails, and
                           so resource intensive daemonsets should take into account that they may

diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml
@@ -35,7 +35,7 @@ spec:
         - --enable-leader-election
         - --logtostderr=true
         - --v=5
-        - --feature-gates=AllAlpha=true
+        - --feature-gates=AllAlpha=true,EnableExternalCerts=false
         image: controller:latest
         imagePullPolicy: Always
         securityContext:

diff --git a/pkg/features/kruise_features.go b/pkg/features/kruise_features.go
@@ -122,6 +122,9 @@ const (
 
 	// Enables a StatefulSet to start from an arbitrary non zero ordinal
 	StatefulSetStartOrdinal featuregate.Feature = "StatefulSetStartOrdinal"
+
+	// Use certs generated externally
+	EnableExternalCerts featuregate.Feature = "EnableExternalCerts"
 )
 
 var defaultFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{
@@ -154,6 +157,7 @@ var defaultFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{
 	EnhancedLivenessProbeGate:              {Default: false, PreRelease: featuregate.Alpha},
 	RecreatePodWhenChangeVCTInCloneSetGate: {Default: false, PreRelease: featuregate.Alpha},
 	StatefulSetStartOrdinal:                {Default: false, PreRelease: featuregate.Alpha},
+	EnableExternalCerts:                    {Default: false, PreRelease: featuregate.Alpha},
 }
 
 func init() {

diff --git a/pkg/webhook/server.go b/pkg/webhook/server.go
@@ -113,13 +113,13 @@ func Initialize(ctx context.Context, cfg *rest.Config) error {
 		c.Start(ctx)
 	}()
 
-	timer := time.NewTimer(time.Second * 20)
+	timer := time.NewTimer(time.Second * 30)
 	defer timer.Stop()
 	select {
 	case <-webhookcontroller.Inited():
 		return nil
 	case <-timer.C:
-		return fmt.Errorf("failed to start webhook controller for waiting more than 20s")
+		return fmt.Errorf("failed to start webhook controller for waiting more than 30s")
 	}
 }
 

diff --git 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"
@@ -28,6 +29,8 @@ import (
 	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/klog/v2"
 
+	"github.com/openkruise/kruise/pkg/features"
+	utilfeature "github.com/openkruise/kruise/pkg/util/feature"
 	"github.com/openkruise/kruise/pkg/webhook/types"
 	webhookutil "github.com/openkruise/kruise/pkg/webhook/util"
 )
@@ -46,8 +49,6 @@ func Ensure(kubeClient clientset.Interface, handlers map[string]types.HandlerGet
 	if err != nil {
 		return fmt.Errorf("not found ValidatingWebhookConfiguration %s", validatingWebhookConfigurationName)
 	}
-	oldMutatingConfig := mutatingConfig.DeepCopy()
-	oldValidatingConfig := validatingConfig.DeepCopy()
 
 	mutatingTemplate, err := parseMutatingTemplate(mutatingConfig)
 	if err != nil {
@@ -58,65 +59,102 @@ func Ensure(kubeClient clientset.Interface, handlers map[string]types.HandlerGet
 		return err
 	}
 
-	var mutatingWHs []admissionregistrationv1.MutatingWebhook
-	for i := range mutatingTemplate {
-		wh := &mutatingTemplate[i]
-		wh.ClientConfig.CABundle = caBundle
-		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 utilfeature.DefaultFeatureGate.Enabled(features.EnableExternalCerts) {
+		// if using external certs, only check the caBundle of webhook
+		for i := range mutatingTemplate {
+			wh := &mutatingTemplate[i]
+			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)
+			}
 		}
-		if wh.ClientConfig.Service != nil {
-			wh.ClientConfig.Service.Namespace = webhookutil.GetNamespace()
-			wh.ClientConfig.Service.Name = webhookutil.GetServiceName()
 
-			if host := webhookutil.GetHost(); len(host) > 0 {
-				convertClientConfig(&wh.ClientConfig, host, webhookutil.GetPort())
+		for i := range validatingTemplate {
+			wh := &validatingTemplate[i]
+			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", mutatingWebhookConfigurationName)
 			}
 		}
+	} else {
+		// if using certs generated by kruise, update webhook configurations
+		oldMutatingConfig := mutatingConfig.DeepCopy()
+		oldValidatingConfig := validatingConfig.DeepCopy()
 
-		mutatingWHs = append(mutatingWHs, *wh)
-	}
-	mutatingConfig.Webhooks = mutatingWHs
+		var mutatingWHs []admissionregistrationv1.MutatingWebhook
+		for i := range mutatingTemplate {
+			wh := &mutatingTemplate[i]
+			wh.ClientConfig.CABundle = caBundle
+			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 wh.ClientConfig.Service != nil {
+				wh.ClientConfig.Service.Namespace = webhookutil.GetNamespace()
+				wh.ClientConfig.Service.Name = webhookutil.GetServiceName()
 
-	var validatingWHs []admissionregistrationv1.ValidatingWebhook
-	for i := range validatingTemplate {
-		wh := &validatingTemplate[i]
-		wh.ClientConfig.CABundle = caBundle
-		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 host := webhookutil.GetHost(); len(host) > 0 {
+					convertClientConfig(&wh.ClientConfig, host, webhookutil.GetPort())
+				}
+			}
+
+			mutatingWHs = append(mutatingWHs, *wh)
 		}
-		if wh.ClientConfig.Service != nil {
-			wh.ClientConfig.Service.Namespace = webhookutil.GetNamespace()
-			wh.ClientConfig.Service.Name = webhookutil.GetServiceName()
+		mutatingConfig.Webhooks = mutatingWHs
 
-			if host := webhookutil.GetHost(); len(host) > 0 {
-				convertClientConfig(&wh.ClientConfig, host, webhookutil.GetPort())
+		var validatingWHs []admissionregistrationv1.ValidatingWebhook
+		for i := range validatingTemplate {
+			wh := &validatingTemplate[i]
+			wh.ClientConfig.CABundle = caBundle
+			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 wh.ClientConfig.Service != nil {
+				wh.ClientConfig.Service.Namespace = webhookutil.GetNamespace()
+				wh.ClientConfig.Service.Name = webhookutil.GetServiceName()
 
-		validatingWHs = append(validatingWHs, *wh)
-	}
-	validatingConfig.Webhooks = validatingWHs
+				if host := webhookutil.GetHost(); len(host) > 0 {
+					convertClientConfig(&wh.ClientConfig, host, webhookutil.GetPort())
+				}
+			}
 
-	if !reflect.DeepEqual(mutatingConfig, oldMutatingConfig) {
-		if _, err := kubeClient.AdmissionregistrationV1().MutatingWebhookConfigurations().Update(context.TODO(), mutatingConfig, metav1.UpdateOptions{}); err != nil {
-			return fmt.Errorf("failed to update %s: %v", mutatingWebhookConfigurationName, err)
+			validatingWHs = append(validatingWHs, *wh)
+		}
+		validatingConfig.Webhooks = validatingWHs
+
+		if !reflect.DeepEqual(mutatingConfig, oldMutatingConfig) {
+			if _, err := kubeClient.AdmissionregistrationV1().MutatingWebhookConfigurations().Update(context.TODO(), mutatingConfig, metav1.UpdateOptions{}); err != nil {
+				return fmt.Errorf("failed to update %s: %v", mutatingWebhookConfigurationName, err)
+			}
 		}
-	}
 
-	if !reflect.DeepEqual(validatingConfig, oldValidatingConfig) {
-		if _, err := kubeClient.AdmissionregistrationV1().ValidatingWebhookConfigurations().Update(context.TODO(), validatingConfig, metav1.UpdateOptions{}); err != nil {
-			return fmt.Errorf("failed to update %s: %v", validatingWebhookConfigurationName, err)
+		if !reflect.DeepEqual(validatingConfig, oldValidatingConfig) {
+			if _, err := kubeClient.AdmissionregistrationV1().ValidatingWebhookConfigurations().Update(context.TODO(), validatingConfig, metav1.UpdateOptions{}); err != nil {
+				return fmt.Errorf("failed to update %s: %v", validatingWebhookConfigurationName, err)
+			}
 		}
 	}
 

diff --git a/pkg/webhook/util/controller/webhook_controller.go b/pkg/webhook/util/controller/webhook_controller.go
@@ -41,6 +41,8 @@ import (
 	"k8s.io/klog/v2"
 
 	extclient "github.com/openkruise/kruise/pkg/client"
+	"github.com/openkruise/kruise/pkg/features"
+	utilfeature "github.com/openkruise/kruise/pkg/util/feature"
 	webhooktypes "github.com/openkruise/kruise/pkg/webhook/types"
 	webhookutil "github.com/openkruise/kruise/pkg/webhook/util"
 	"github.com/openkruise/kruise/pkg/webhook/util/configuration"
@@ -53,7 +55,8 @@ const (
 	mutatingWebhookConfigurationName   = "kruise-mutating-webhook-configuration"
 	validatingWebhookConfigurationName = "kruise-validating-webhook-configuration"
 
-	defaultResyncPeriod = time.Minute
+	defaultResyncPeriod      = time.Minute
+	WaitForExternalCertsSync = 10 * time.Second
 )
 
 var (
@@ -189,6 +192,10 @@ func (c *Controller) Start(ctx context.Context) {
 		return
 	}
 
+	if utilfeature.DefaultFeatureGate.Enabled(features.EnableExternalCerts) {
+		// wait 10s for external certs to be synced
+		time.Sleep(WaitForExternalCertsSync)
+	}
 	go wait.Until(func() {
 		for c.processNextWorkItem() {
 		}
@@ -233,15 +240,15 @@ func (c *Controller) sync() error {
 	var err error
 
 	certWriterType := webhookutil.GetCertWriter()
-	if certWriterType == writer.FsCertWriter || (len(certWriterType) == 0 && len(webhookutil.GetHost()) != 0) {
-		certWriter, err = writer.NewFSCertWriter(writer.FSCertWriterOptions{
-			Path: webhookutil.GetCertDir(),
-		})
-	} else if certWriterType == writer.ExternalCertWriter {
+	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{
+			Path: webhookutil.GetCertDir(),
+		})
 	} else {
 		certWriter, err = writer.NewSecretCertWriter(writer.SecretCertWriterOptions{
 			Clientset: c.kubeClient,

diff --git 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"
@@ -32,6 +33,8 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 
 	"github.com/openkruise/kruise/apis"
+	"github.com/openkruise/kruise/pkg/features"
+	utilfeature "github.com/openkruise/kruise/pkg/util/feature"
 	webhookutil "github.com/openkruise/kruise/pkg/webhook/util"
 )
 
@@ -49,41 +52,61 @@ func Ensure(client apiextensionsclientset.Interface, lister apiextensionslisters
 		return fmt.Errorf("failed to list crds: %v", err)
 	}
 
-	webhookConfig := apiextensionsv1.WebhookClientConfig{
-		CABundle: caBundle,
-	}
-	path := "/convert"
-	if host := webhookutil.GetHost(); len(host) > 0 {
-		url := fmt.Sprintf("https://%s:%d%s", host, webhookutil.GetPort(), path)
-		webhookConfig.URL = &url
-	} else {
-		var port int32 = 443
-		webhookConfig.Service = &apiextensionsv1.ServiceReference{
-			Namespace: webhookutil.GetNamespace(),
-			Name:      webhookutil.GetServiceName(),
-			Port:      &port,
-			Path:      &path,
-		}
-	}
+	if utilfeature.DefaultFeatureGate.Enabled(features.EnableExternalCerts) {
+		for _, crd := range crdList {
+			if len(crd.Spec.Versions) == 0 || crd.Spec.Conversion == nil || crd.Spec.Conversion.Strategy != apiextensionsv1.WebhookConverter {
+				continue
+			}
+			if !kruiseScheme.Recognizes(schema.GroupVersionKind{Group: crd.Spec.Group, Version: crd.Spec.Versions[0].Name, Kind: crd.Spec.Names.Kind}) {
+				continue
+			}
+
+			if crd.Spec.Conversion.Webhook == nil || crd.Spec.Conversion.Webhook.ClientConfig == nil {
+				return fmt.Errorf("bad conversion configuration of CRD %s", crd.Name)
+			}
 
-	for _, crd := range crdList {
-		if len(crd.Spec.Versions) == 0 || crd.Spec.Conversion == nil || crd.Spec.Conversion.Strategy != apiextensionsv1.WebhookConverter {
-			continue
+			if !bytes.Equal(crd.Spec.Conversion.Webhook.ClientConfig.CABundle, caBundle) {
+				return fmt.Errorf("caBundle of CRD %s does not match external caBundle", crd.Name)
+			}
+		}
+	} else {
+		webhookConfig := apiextensionsv1.WebhookClientConfig{
+			CABundle: caBundle,
 		}
-		if !kruiseScheme.Recognizes(schema.GroupVersionKind{Group: crd.Spec.Group, Version: crd.Spec.Versions[0].Name, Kind: crd.Spec.Names.Kind}) {
-			continue
+		path := "/convert"
+		if host := webhookutil.GetHost(); len(host) > 0 {
+			url := fmt.Sprintf("https://%s:%d%s", host, webhookutil.GetPort(), path)
+			webhookConfig.URL = &url
+		} else {
+			var port int32 = 443
+			webhookConfig.Service = &apiextensionsv1.ServiceReference{
+				Namespace: webhookutil.GetNamespace(),
+				Name:      webhookutil.GetServiceName(),
+				Port:      &port,
+				Path:      &path,
+			}
 		}
 
-		if crd.Spec.Conversion.Webhook == nil || !reflect.DeepEqual(crd.Spec.Conversion.Webhook.ClientConfig, webhookConfig) {
-			newCRD := crd.DeepCopy()
-			newCRD.Spec.Conversion.Webhook = &apiextensionsv1.WebhookConversion{
-				ClientConfig:             webhookConfig.DeepCopy(),
-				ConversionReviewVersions: []string{"v1", "v1beta1"},
+		for _, crd := range crdList {
+			if len(crd.Spec.Versions) == 0 || crd.Spec.Conversion == nil || crd.Spec.Conversion.Strategy != apiextensionsv1.WebhookConverter {
+				continue
 			}
-			if _, err := client.ApiextensionsV1().CustomResourceDefinitions().Update(context.TODO(), newCRD, metav1.UpdateOptions{}); err != nil {
-				return fmt.Errorf("failed to update CRD %s: %v", newCRD.Name, err)
+			if !kruiseScheme.Recognizes(schema.GroupVersionKind{Group: crd.Spec.Group, Version: crd.Spec.Versions[0].Name, Kind: crd.Spec.Names.Kind}) {
+				continue
+			}
+
+			if crd.Spec.Conversion.Webhook == nil || !reflect.DeepEqual(crd.Spec.Conversion.Webhook.ClientConfig, webhookConfig) {
+				newCRD := crd.DeepCopy()
+				newCRD.Spec.Conversion.Webhook = &apiextensionsv1.WebhookConversion{
+					ClientConfig:             webhookConfig.DeepCopy(),
+					ConversionReviewVersions: []string{"v1", "v1beta1"},
+				}
+				if _, err := client.ApiextensionsV1().CustomResourceDefinitions().Update(context.TODO(), newCRD, metav1.UpdateOptions{}); err != nil {
+					return fmt.Errorf("failed to update CRD %s: %v", newCRD.Name, err)
+				}
 			}
 		}
 	}
+
 	return nil
 }