✨ Allow webhooks to register custom validators/defaulter types

pkg/builder/webhook.go

@@ -17,6 +17,7 @@ limitations under the License.
 package builder
 
 import (
+	"errors"
 	"net/http"
 	"net/url"
 	"strings"
@@ -32,10 +33,12 @@ import (
 
 // WebhookBuilder builds a Webhook.
 type WebhookBuilder struct {
-	apiType runtime.Object
-	gvk     schema.GroupVersionKind
-	mgr     manager.Manager
-	config  *rest.Config
+	apiType       runtime.Object
+	withDefaulter admission.CustomDefaulter
+	withValidator admission.CustomValidator
+	gvk           schema.GroupVersionKind
+	mgr           manager.Manager
+	config        *rest.Config
 }
 
 // WebhookManagedBy allows inform its manager.Manager.
@@ -53,6 +56,18 @@ func (blder *WebhookBuilder) For(apiType runtime.Object) *WebhookBuilder {
 	return blder
 }
 
+// WithDefaulter takes a admission.WithDefaulter interface, a MutatingWebhook will be wired for this type.
+func (blder *WebhookBuilder) WithDefaulter(defaulter admission.CustomDefaulter) *WebhookBuilder {
+	blder.withDefaulter = defaulter
+	return blder
+}
+
+// WithValidator takes a admission.WithValidator interface, a ValidatingWebhook will be wired for this type.
+func (blder *WebhookBuilder) WithValidator(validator admission.CustomValidator) *WebhookBuilder {
+	blder.withValidator = validator
+	return blder
+}
+
 // Complete builds the webhook.
 func (blder *WebhookBuilder) Complete() error {
 	// Set the Config
@@ -69,9 +84,13 @@ func (blder *WebhookBuilder) loadRestConfig() {
 }
 
 func (blder *WebhookBuilder) registerWebhooks() error {
+	typ, err := blder.getType()
+	if err != nil {
+		return err
+	}
+
 	// Create webhook(s) for each type
-	var err error
-	blder.gvk, err = apiutil.GVKForObject(blder.apiType, blder.mgr.GetScheme())
+	blder.gvk, err = apiutil.GVKForObject(typ, blder.mgr.GetScheme())
 	if err != nil {
 		return err
 	}
@@ -88,12 +107,7 @@ func (blder *WebhookBuilder) registerWebhooks() error {
 
 // registerDefaultingWebhook registers a defaulting webhook if th.
 func (blder *WebhookBuilder) registerDefaultingWebhook() {
-	defaulter, isDefaulter := blder.apiType.(admission.Defaulter)
-	if !isDefaulter {
-		log.Info("skip registering a mutating webhook, admission.Defaulter interface is not implemented", "GVK", blder.gvk)
-		return
-	}
-	mwh := admission.DefaultingWebhookFor(defaulter)
+	mwh := blder.getDefaultingWebhook()
 	if mwh != nil {
 		path := generateMutatePath(blder.gvk)
 
@@ -108,13 +122,21 @@ func (blder *WebhookBuilder) registerDefaultingWebhook() {
 	}
 }
 
-func (blder *WebhookBuilder) registerValidatingWebhook() {
-	validator, isValidator := blder.apiType.(admission.Validator)
-	if !isValidator {
-		log.Info("skip registering a validating webhook, admission.Validator interface is not implemented", "GVK", blder.gvk)
-		return
+func (blder *WebhookBuilder) getDefaultingWebhook() *admission.Webhook {
+	if defaulter := blder.withDefaulter; defaulter != nil {
+		return admission.WithCustomDefaulter(blder.apiType, defaulter)
+	}
+	if defaulter, ok := blder.apiType.(admission.Defaulter); ok {
+		return admission.DefaultingWebhookFor(defaulter)
 	}
-	vwh := admission.ValidatingWebhookFor(validator)
+	log.Info(
+		"skip registering a mutating webhook, object does not implement admission.Defaulter or WithDefaulter wasn't called",
+		"GVK", blder.gvk)
+	return nil
+}
+
+func (blder *WebhookBuilder) registerValidatingWebhook() {
+	vwh := blder.getValidatingWebhook()
 	if vwh != nil {
 		path := generateValidatePath(blder.gvk)
 
@@ -129,6 +151,19 @@ func (blder *WebhookBuilder) registerValidatingWebhook() {
 	}
 }
 
+func (blder *WebhookBuilder) getValidatingWebhook() *admission.Webhook {
+	if validator := blder.withValidator; validator != nil {
+		return admission.WithCustomValidator(blder.apiType, validator)
+	}
+	if validator, ok := blder.apiType.(admission.Validator); ok {
+		return admission.ValidatingWebhookFor(validator)
+	}
+	log.Info(
+		"skip registering a validating webhook, object does not implement admission.Validator or WithValidator wasn't called",
+		"GVK", blder.gvk)
+	return nil
+}
+
 func (blder *WebhookBuilder) registerConversionWebhook() error {
 	ok, err := conversion.IsConvertible(blder.mgr.GetScheme(), blder.apiType)
 	if err != nil {
@@ -145,6 +180,13 @@ func (blder *WebhookBuilder) registerConversionWebhook() error {
 	return nil
 }
 
+func (blder *WebhookBuilder) getType() (runtime.Object, error) {
+	if blder.apiType != nil {
+		return blder.apiType, nil
+	}
+	return nil, errors.New("For() must be called with a valid object")
+}
+
 func (blder *WebhookBuilder) isAlreadyHandled(path string) bool {
 	if blder.mgr.GetWebhookServer().WebhookMux == nil {
 		return false

pkg/builder/webhook_test.go

@@ -134,6 +134,81 @@ func runTests(admissionReviewVersion string) {
 		ExpectWithOffset(1, w.Code).To(Equal(http.StatusNotFound))
 	})
 
+	It("should scaffold a defaulting webhook with a custom defaulter", func() {
+		By("creating a controller manager")
+		m, err := manager.New(cfg, manager.Options{})
+		ExpectWithOffset(1, err).NotTo(HaveOccurred())
+
+		By("registering the type in the Scheme")
+		builder := scheme.Builder{GroupVersion: testDefaulterGVK.GroupVersion()}
+		builder.Register(&TestDefaulter{}, &TestDefaulterList{})
+		err = builder.AddToScheme(m.GetScheme())
+		ExpectWithOffset(1, err).NotTo(HaveOccurred())
+
+		err = WebhookManagedBy(m).
+			WithDefaulter(&TestCustomDefaulter{}).
+			For(&TestDefaulter{}).
+			Complete()
+		ExpectWithOffset(1, err).NotTo(HaveOccurred())
+		svr := m.GetWebhookServer()
+		ExpectWithOffset(1, svr).NotTo(BeNil())
+
+		reader := strings.NewReader(`{
+  "kind":"AdmissionReview",
+  "apiVersion":"admission.k8s.io/` + admissionReviewVersion + `",
+  "request":{
+    "uid":"07e52e8d-4513-11e9-a716-42010a800270",
+    "kind":{
+      "group":"",
+      "version":"v1",
+      "kind":"TestDefaulter"
+    },
+    "resource":{
+      "group":"",
+      "version":"v1",
+      "resource":"testdefaulter"
+    },
+    "namespace":"default",
+    "operation":"CREATE",
+    "object":{
+      "replica":1
+    },
+    "oldObject":null
+  }
+}`)
+
+		ctx, cancel := context.WithCancel(context.Background())
+		cancel()
+		// TODO: we may want to improve it to make it be able to inject dependencies,
+		// but not always try to load certs and return not found error.
+		err = svr.Start(ctx)
+		if err != nil && !os.IsNotExist(err) {
+			ExpectWithOffset(1, err).NotTo(HaveOccurred())
+		}
+
+		By("sending a request to a mutating webhook path")
+		path := generateMutatePath(testDefaulterGVK)
+		req := httptest.NewRequest("POST", "http://svc-name.svc-ns.svc"+path, reader)
+		req.Header.Add("Content-Type", "application/json")
+		w := httptest.NewRecorder()
+		svr.WebhookMux.ServeHTTP(w, req)
+		ExpectWithOffset(1, w.Code).To(Equal(http.StatusOK))
+		By("sanity checking the response contains reasonable fields")
+		ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":true`))
+		ExpectWithOffset(1, w.Body).To(ContainSubstring(`"patch":`))
+		ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":200`))
+
+		By("sending a request to a validating webhook path that doesn't exist")
+		path = generateValidatePath(testDefaulterGVK)
+		_, err = reader.Seek(0, 0)
+		ExpectWithOffset(1, err).NotTo(HaveOccurred())
+		req = httptest.NewRequest("POST", "http://svc-name.svc-ns.svc"+path, reader)
+		req.Header.Add("Content-Type", "application/json")
+		w = httptest.NewRecorder()
+		svr.WebhookMux.ServeHTTP(w, req)
+		ExpectWithOffset(1, w.Code).To(Equal(http.StatusNotFound))
+	})
+
 	It("should scaffold a validating webhook if the type implements the Validator interface", func() {
 		By("creating a controller manager")
 		m, err := manager.New(cfg, manager.Options{})
@@ -209,6 +284,82 @@ func runTests(admissionReviewVersion string) {
 		ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":403`))
 	})
 
+	It("should scaffold a validating webhook with a custom validator", func() {
+		By("creating a controller manager")
+		m, err := manager.New(cfg, manager.Options{})
+		ExpectWithOffset(1, err).NotTo(HaveOccurred())
+
+		By("registering the type in the Scheme")
+		builder := scheme.Builder{GroupVersion: testValidatorGVK.GroupVersion()}
+		builder.Register(&TestValidator{}, &TestValidatorList{})
+		err = builder.AddToScheme(m.GetScheme())
+		ExpectWithOffset(1, err).NotTo(HaveOccurred())
+
+		err = WebhookManagedBy(m).
+			WithValidator(&TestCustomValidator{}).
+			For(&TestValidator{}).
+			Complete()
+		ExpectWithOffset(1, err).NotTo(HaveOccurred())
+		svr := m.GetWebhookServer()
+		ExpectWithOffset(1, svr).NotTo(BeNil())
+
+		reader := strings.NewReader(`{
+  "kind":"AdmissionReview",
+  "apiVersion":"admission.k8s.io/` + admissionReviewVersion + `",
+  "request":{
+    "uid":"07e52e8d-4513-11e9-a716-42010a800270",
+    "kind":{
+      "group":"",
+      "version":"v1",
+      "kind":"TestValidator"
+    },
+    "resource":{
+      "group":"",
+      "version":"v1",
+      "resource":"testvalidator"
+    },
+    "namespace":"default",
+    "operation":"UPDATE",
+    "object":{
+      "replica":1
+    },
+    "oldObject":{
+      "replica":2
+    }
+  }
+}`)
+
+		ctx, cancel := context.WithCancel(context.Background())
+		cancel()
+		// TODO: we may want to improve it to make it be able to inject dependencies,
+		// but not always try to load certs and return not found error.
+		err = svr.Start(ctx)
+		if err != nil && !os.IsNotExist(err) {
+			ExpectWithOffset(1, err).NotTo(HaveOccurred())
+		}
+
+		By("sending a request to a mutating webhook path that doesn't exist")
+		path := generateMutatePath(testValidatorGVK)
+		req := httptest.NewRequest("POST", "http://svc-name.svc-ns.svc"+path, reader)
+		req.Header.Add("Content-Type", "application/json")
+		w := httptest.NewRecorder()
+		svr.WebhookMux.ServeHTTP(w, req)
+		ExpectWithOffset(1, w.Code).To(Equal(http.StatusNotFound))
+
+		By("sending a request to a validating webhook path")
+		path = generateValidatePath(testValidatorGVK)
+		_, err = reader.Seek(0, 0)
+		ExpectWithOffset(1, err).NotTo(HaveOccurred())
+		req = httptest.NewRequest("POST", "http://svc-name.svc-ns.svc"+path, reader)
+		req.Header.Add("Content-Type", "application/json")
+		w = httptest.NewRecorder()
+		svr.WebhookMux.ServeHTTP(w, req)
+		ExpectWithOffset(1, w.Code).To(Equal(http.StatusOK))
+		By("sanity checking the response contains reasonable field")
+		ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":false`))
+		ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":403`))
+	})
+
 	It("should scaffold defaulting and validating webhooks if the type implements both Defaulter and Validator interfaces", func() {
 		By("creating a controller manager")
 		m, err := manager.New(cfg, manager.Options{})
@@ -537,3 +688,51 @@ func (dv *TestDefaultValidator) ValidateDelete() error {
 	}
 	return nil
 }
+
+// TestCustomDefaulter.
+
+type TestCustomDefaulter struct{}
+
+func (*TestCustomDefaulter) Default(ctx context.Context, obj runtime.Object) error {
+	d := obj.(*TestDefaulter) //nolint:ifshort
+	if d.Replica < 2 {
+		d.Replica = 2
+	}
+	return nil
+}
+
+var _ admission.CustomDefaulter = &TestCustomDefaulter{}
+
+// TestCustomValidator.
+
+type TestCustomValidator struct{}
+
+func (*TestCustomValidator) ValidateCreate(ctx context.Context, obj runtime.Object) error {
+	v := obj.(*TestValidator) //nolint:ifshort
+	if v.Replica < 0 {
+		return errors.New("number of replica should be greater than or equal to 0")
+	}
+	return nil
+}
+
+func (*TestCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) error {
+	v := newObj.(*TestValidator)
+	old := oldObj.(*TestValidator) //nolint:ifshort
+	if v.Replica < 0 {
+		return errors.New("number of replica should be greater than or equal to 0")
+	}
+	if v.Replica < old.Replica {
+		return fmt.Errorf("new replica %v should not be fewer than old replica %v", v.Replica, old.Replica)
+	}
+	return nil
+}
+
+func (*TestCustomValidator) ValidateDelete(ctx context.Context, obj runtime.Object) error {
+	v := obj.(*TestValidator) //nolint:ifshort
+	if v.Replica > 0 {
+		return errors.New("number of replica should be less than or equal to 0 to delete")
+	}
+	return nil
+}
+
+var _ admission.CustomValidator = &TestCustomValidator{}