Simplify design and error on multiple mutation mechanisms

Author: alculquicondor

pkg/builder/webhook.go

@@ -37,7 +37,7 @@ import (
 // WebhookBuilder builds a Webhook.
 type WebhookBuilder struct {
 	apiType         runtime.Object
-	mutatorFactory  admission.HandlerFactory
+	mutationHandler admission.Handler
 	customDefaulter admission.CustomDefaulter
 	customValidator admission.CustomValidator
 	gvk             schema.GroupVersionKind
@@ -66,9 +66,9 @@ func (blder *WebhookBuilder) For(apiType runtime.Object) *WebhookBuilder {
 	return blder
 }
 
-// WithMutatorFactory takes an admission.HandlerFactory, a MutatingWebhook will be wired for the handler that this factory creates.
-func (blder *WebhookBuilder) WithMutatorFactory(factory admission.HandlerFactory) *WebhookBuilder {
-	blder.mutatorFactory = factory
+// WithMutationHandler takes an admission.Handler, a MutatingWebhook will be wired for it.
+func (blder *WebhookBuilder) WithMutationHandler(h admission.Handler) *WebhookBuilder {
+	blder.mutationHandler = h
 	return blder
 }
 
@@ -147,7 +147,9 @@ func (blder *WebhookBuilder) registerWebhooks() error {
 	}
 
 	// Register webhook(s) for type
-	blder.registerDefaultingWebhook()
+	if err := blder.registerDefaultingWebhook(); err != nil {
+		return err
+	}
 	blder.registerValidatingWebhook()
 
 	err = blder.registerConversionWebhook()
@@ -158,8 +160,11 @@ func (blder *WebhookBuilder) registerWebhooks() error {
 }
 
 // registerDefaultingWebhook registers a defaulting webhook if necessary.
-func (blder *WebhookBuilder) registerDefaultingWebhook() {
-	mwh := blder.getDefaultingWebhook()
+func (blder *WebhookBuilder) registerDefaultingWebhook() error {
+	mwh, err := blder.getDefaultingWebhook()
+	if err != nil {
+		return err
+	}
 	if mwh != nil {
 		mwh.LogConstructor = blder.logConstructor
 		path := generateMutatePath(blder.gvk)
@@ -173,21 +178,27 @@ func (blder *WebhookBuilder) registerDefaultingWebhook() {
 			blder.mgr.GetWebhookServer().Register(path, mwh)
 		}
 	}
+	return nil
 }
 
-func (blder *WebhookBuilder) getDefaultingWebhook() *admission.Webhook {
+func (blder *WebhookBuilder) getDefaultingWebhook() (*admission.Webhook, error) {
 	var w *admission.Webhook
-	if factory := blder.mutatorFactory; factory != nil {
-		w = admission.WithHandlerFactory(blder.mgr.GetScheme(), blder.apiType, factory)
-	} else if defaulter := blder.customDefaulter; defaulter != nil {
+	if handler := blder.mutationHandler; handler != nil {
+		w = &admission.Webhook{Handler: handler}
+	}
+	if defaulter := blder.customDefaulter; defaulter != nil {
+		if w != nil {
+			return nil, errors.New("a WebhookBuilder can only define a MutationHandler or a Defaulter, but not both")
+		}
 		w = admission.WithCustomDefaulter(blder.mgr.GetScheme(), blder.apiType, defaulter)
-	} else {
-		return nil
+	}
+	if w == nil {
+		return nil, nil
 	}
 	if blder.recoverPanic != nil {
 		w = w.WithRecoverPanic(*blder.recoverPanic)
 	}
-	return w
+	return w, nil
 }
 
 // registerValidatingWebhook registers a validating webhook if necessary.

pkg/builder/webhook_test.go

@@ -229,8 +229,8 @@ func runTests(admissionReviewVersion string) {
 		ExpectWithOffset(1, err).NotTo(HaveOccurred())
 
 		err = WebhookManagedBy(m).
-			WithMutatorFactory(mutatorFactoryForTestDefaulter(m.GetScheme())).
 			For(&TestDefaulter{}).
+			WithMutationHandler(admission.WithCustomDefaulter(m.GetScheme(), &TestDefaulter{}, &TestCustomDefaulter{})).
 			WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger {
 				return admission.DefaultLogConstructor(testingLogger, req)
 			}).
@@ -668,12 +668,6 @@ func (*TestCustomDefaulter) Default(ctx context.Context, obj runtime.Object) err
 
 var _ admission.CustomDefaulter = &TestCustomDefaulter{}
 
-func mutatorFactoryForTestDefaulter(scheme *runtime.Scheme) admission.HandlerFactory {
-	return func(obj runtime.Object, _ admission.Decoder) admission.Handler {
-		return admission.WithCustomDefaulter(scheme, obj, &TestCustomDefaulter{}).Handler
-	}
-}
-
 // TestCustomValidator.
 
 type TestCustomValidator struct{}

pkg/webhook/admission/webhook.go

@@ -27,7 +27,6 @@ import (
 	"gomodules.xyz/jsonpatch/v2"
 	admissionv1 "k8s.io/api/admission/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/json"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/klog/v2"
@@ -96,9 +95,6 @@ func (r *Response) Complete(req Request) error {
 	return nil
 }
 
-// HandlerFactory can create a Handler for the given type.
-type HandlerFactory func(obj runtime.Object, decoder Decoder) Handler
-
 // Handler can handle an AdmissionRequest.
 type Handler interface {
 	// Handle yields a response to an AdmissionRequest.
@@ -118,13 +114,6 @@ func (f HandlerFunc) Handle(ctx context.Context, req Request) Response {
 	return f(ctx, req)
 }
 
-// WithHandlerFactory creates a new Webhook for a handler factory.
-func WithHandlerFactory(scheme *runtime.Scheme, obj runtime.Object, factory HandlerFactory) *Webhook {
-	return &Webhook{
-		Handler: factory(obj, NewDecoder(scheme)),
-	}
-}
-
 // Webhook represents each individual webhook.
 //
 // It must be registered with a webhook.Server or