:warn: Allow passing a custom webhook server

Currently it is impossible to pass a custom webhook server, because we
reference the concrete type rather than an interface. Change this to an
interface.

pkg/builder/webhook.go

@@ -229,10 +229,10 @@ func (blder *WebhookBuilder) getType() (runtime.Object, error) {
 }
 
 func (blder *WebhookBuilder) isAlreadyHandled(path string) bool {
-	if blder.mgr.GetWebhookServer().WebhookMux == nil {
+	if blder.mgr.GetWebhookServer().WebhookMux() == nil {
 		return false
 	}
-	h, p := blder.mgr.GetWebhookServer().WebhookMux.Handler(&http.Request{URL: &url.URL{Path: path}})
+	h, p := blder.mgr.GetWebhookServer().WebhookMux().Handler(&http.Request{URL: &url.URL{Path: path}})
 	if p == path && h != nil {
 		return true
 	}

pkg/builder/webhook_test.go

@@ -125,7 +125,7 @@ func runTests(admissionReviewVersion string) {
 		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)
+		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`))
@@ -139,7 +139,7 @@ func runTests(admissionReviewVersion string) {
 		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)
+		svr.WebhookMux().ServeHTTP(w, req)
 		ExpectWithOffset(1, w.Code).To(Equal(http.StatusNotFound))
 	})
 
@@ -199,7 +199,7 @@ func runTests(admissionReviewVersion string) {
 		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)
+		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":false`))
@@ -266,7 +266,7 @@ func runTests(admissionReviewVersion string) {
 		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)
+		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`))
@@ -281,7 +281,7 @@ func runTests(admissionReviewVersion string) {
 		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)
+		svr.WebhookMux().ServeHTTP(w, req)
 		ExpectWithOffset(1, w.Code).To(Equal(http.StatusNotFound))
 	})
 
@@ -341,7 +341,7 @@ func runTests(admissionReviewVersion string) {
 		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)
+		svr.WebhookMux().ServeHTTP(w, req)
 		ExpectWithOffset(1, w.Code).To(Equal(http.StatusNotFound))
 
 		By("sending a request to a validating webhook path")
@@ -351,7 +351,7 @@ func runTests(admissionReviewVersion string) {
 		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)
+		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`))
@@ -415,7 +415,7 @@ func runTests(admissionReviewVersion string) {
 		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)
+		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`))
@@ -484,7 +484,7 @@ func runTests(admissionReviewVersion string) {
 		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)
+		svr.WebhookMux().ServeHTTP(w, req)
 		ExpectWithOffset(1, w.Code).To(Equal(http.StatusNotFound))
 
 		By("sending a request to a validating webhook path")
@@ -494,7 +494,7 @@ func runTests(admissionReviewVersion string) {
 		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)
+		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`))
@@ -556,7 +556,7 @@ func runTests(admissionReviewVersion string) {
 		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)
+		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":true`))
@@ -570,7 +570,7 @@ func runTests(admissionReviewVersion string) {
 		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)
+		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":true`))
@@ -632,7 +632,7 @@ func runTests(admissionReviewVersion string) {
 		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)
+		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`))
@@ -666,7 +666,7 @@ func runTests(admissionReviewVersion string) {
 		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)
+		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":true`))

pkg/manager/internal.go

@@ -130,7 +130,7 @@ type controllerManager struct {
 	// election was configured.
 	elected chan struct{}
 
-	webhookServer *webhook.Server
+	webhookServer webhook.Server
 	// webhookServerOnce will be called in GetWebhookServer() to optionally initialize
 	// webhookServer if unset, and Add() it to controllerManager.
 	webhookServerOnce sync.Once
@@ -277,7 +277,7 @@ func (cm *controllerManager) GetAPIReader() client.Reader {
 	return cm.cluster.GetAPIReader()
 }
 
-func (cm *controllerManager) GetWebhookServer() *webhook.Server {
+func (cm *controllerManager) GetWebhookServer() webhook.Server {
 	cm.webhookServerOnce.Do(func() {
 		if cm.webhookServer == nil {
 			panic("webhook should not be nil")

pkg/manager/manager.go

@@ -86,7 +86,7 @@ type Manager interface {
 	Start(ctx context.Context) error
 
 	// GetWebhookServer returns a webhook.Server
-	GetWebhookServer() *webhook.Server
+	GetWebhookServer() webhook.Server
 
 	// GetLogger returns this manager's logger.
 	GetLogger() logr.Logger
@@ -306,7 +306,7 @@ type Options struct {
 	// WebhookServer is an externally configured webhook.Server. By default,
 	// a Manager will create a default server using Port, Host, and CertDir;
 	// if this is set, the Manager will use this server instead.
-	WebhookServer *webhook.Server
+	WebhookServer webhook.Server
 
 	// BaseContext is the function that provides Context values to Runnables
 	// managed by the Manager. If a BaseContext function isn't provided, Runnables
@@ -556,11 +556,11 @@ func (o Options) AndFrom(loader config.ControllerManagerConfiguration) (Options,
 		o.CertDir = newObj.Webhook.CertDir
 	}
 	if o.WebhookServer == nil {
-		o.WebhookServer = &webhook.Server{
+		o.WebhookServer = webhook.New(webhook.Options{
 			Port:    o.Port,
 			Host:    o.Host,
 			CertDir: o.CertDir,
-		}
+		})
 	}
 
 	if newObj.Controller != nil {
@@ -733,12 +733,12 @@ func setOptionsDefaults(options Options) Options {
 	}
 
 	if options.WebhookServer == nil {
-		options.WebhookServer = &webhook.Server{
+		options.WebhookServer = webhook.New(webhook.Options{
 			Host:    options.Host,
 			Port:    options.Port,
 			CertDir: options.CertDir,
 			TLSOpts: options.TLSOpts,
-		}
+		})
 	}
 
 	return options

pkg/manager/manager_test.go

@@ -226,12 +226,12 @@ var _ = Describe("manger.Manager", func() {
 				HealthProbeBindAddress:     "5000",
 				ReadinessEndpointName:      "/readiness",
 				LivenessEndpointName:       "/liveness",
-				WebhookServer: &webhook.Server{
+				WebhookServer: webhook.New(webhook.Options{
 					Port:    8080,
 					Host:    "example.com",
 					CertDir: "/pki",
 					TLSOpts: optionsTlSOptsFuncs,
-				},
+				}),
 			}.AndFrom(&fakeDeferredLoader{ccfg})
 			Expect(err).To(BeNil())
 
@@ -248,23 +248,23 @@ var _ = Describe("manger.Manager", func() {
 			Expect(m.HealthProbeBindAddress).To(Equal("5000"))
 			Expect(m.ReadinessEndpointName).To(Equal("/readiness"))
 			Expect(m.LivenessEndpointName).To(Equal("/liveness"))
-			Expect(m.WebhookServer.Port).To(Equal(8080))
-			Expect(m.WebhookServer.Host).To(Equal("example.com"))
-			Expect(m.WebhookServer.CertDir).To(Equal("/pki"))
-			Expect(m.WebhookServer.TLSOpts).To(Equal(optionsTlSOptsFuncs))
+			Expect(m.WebhookServer.(*webhook.DefaultServer).Port).To(Equal(8080))
+			Expect(m.WebhookServer.(*webhook.DefaultServer).Host).To(Equal("example.com"))
+			Expect(m.WebhookServer.(*webhook.DefaultServer).CertDir).To(Equal("/pki"))
+			Expect(m.WebhookServer.(*webhook.DefaultServer).TLSOpts).To(Equal(optionsTlSOptsFuncs))
 		})
 
 		It("should lazily initialize a webhook server if needed", func() {
 			By("creating a manager with options")
-			m, err := New(cfg, Options{WebhookServer: &webhook.Server{Port: 9440, Host: "foo.com"}})
+			m, err := New(cfg, Options{WebhookServer: webhook.New(webhook.Options{Port: 9440, Host: "foo.com"})})
 			Expect(err).NotTo(HaveOccurred())
 			Expect(m).NotTo(BeNil())
 
 			By("checking options are passed to the webhook server")
 			svr := m.GetWebhookServer()
 			Expect(svr).NotTo(BeNil())
-			Expect(svr.Port).To(Equal(9440))
-			Expect(svr.Host).To(Equal("foo.com"))
+			Expect(svr.(*webhook.DefaultServer).Port).To(Equal(9440))
+			Expect(svr.(*webhook.DefaultServer).Host).To(Equal("foo.com"))
 		})
 
 		It("should lazily initialize a webhook server if needed (deprecated)", func() {
@@ -276,13 +276,13 @@ var _ = Describe("manger.Manager", func() {
 			By("checking options are passed to the webhook server")
 			svr := m.GetWebhookServer()
 			Expect(svr).NotTo(BeNil())
-			Expect(svr.Port).To(Equal(9440))
-			Expect(svr.Host).To(Equal("foo.com"))
+			Expect(svr.(*webhook.DefaultServer).Port).To(Equal(9440))
+			Expect(svr.(*webhook.DefaultServer).Host).To(Equal("foo.com"))
 		})
 
 		It("should not initialize a webhook server if Options.WebhookServer is set", func() {
 			By("creating a manager with options")
-			srv := &webhook.Server{Port: 9440}
+			srv := webhook.New(webhook.Options{Port: 9440})
 			m, err := New(cfg, Options{Port: 9441, WebhookServer: srv})
 			Expect(err).NotTo(HaveOccurred())
 			Expect(m).NotTo(BeNil())
@@ -291,7 +291,22 @@ var _ = Describe("manger.Manager", func() {
 			svr := m.GetWebhookServer()
 			Expect(svr).NotTo(BeNil())
 			Expect(svr).To(Equal(srv))
-			Expect(svr.Port).To(Equal(9440))
+			Expect(svr.(*webhook.DefaultServer).Port).To(Equal(9440))
+		})
+
+		It("should allow passing a custom webhook.Server implementation", func() {
+			type customWebhook struct {
+				webhook.Server
+			}
+			m, err := New(cfg, Options{WebhookServer: customWebhook{}})
+			Expect(err).NotTo(HaveOccurred())
+			Expect(m).NotTo(BeNil())
+
+			svr := m.GetWebhookServer()
+			Expect(svr).NotTo(BeNil())
+
+			_, isCustomWebhook := svr.(customWebhook)
+			Expect(isCustomWebhook).To(BeTrue())
 		})
 
 		Context("with leader election enabled", func() {

pkg/manager/runnable_group.go

@@ -56,7 +56,7 @@ func (r *runnables) Add(fn Runnable) error {
 		return r.Caches.Add(fn, func(ctx context.Context) bool {
 			return runnable.GetCache().WaitForCacheSync(ctx)
 		})
-	case *webhook.Server:
+	case webhook.Server:
 		return r.Webhooks.Add(fn, nil)
 	case LeaderElectionRunnable:
 		if !runnable.NeedLeaderElection() {

pkg/manager/runnable_group_test.go

@@ -29,7 +29,7 @@ var _ = Describe("runnables", func() {
 	})
 
 	It("should add webhooks to the appropriate group", func() {
-		webhook := &webhook.Server{}
+		webhook := webhook.New(webhook.Options{})
 		r := newRunnables(defaultBaseContext, errCh)
 		Expect(r.Add(webhook)).To(Succeed())
 		Expect(r.Webhooks.startQueue).To(HaveLen(1))

pkg/webhook/example_test.go

@@ -61,9 +61,9 @@ func Example() {
 	}
 
 	// Create a webhook server.
-	hookServer := &Server{
+	hookServer := New(Options{
 		Port: 8443,
-	}
+	})
 	if err := mgr.Add(hookServer); err != nil {
 		panic(err)
 	}
@@ -88,9 +88,9 @@ func Example() {
 // tls.crt and tls.key.
 func ExampleServer_Start() {
 	// Create a webhook server
-	hookServer := &Server{
+	hookServer := New(Options{
 		Port: 8443,
-	}
+	})
 
 	// Register the webhooks in the server.
 	hookServer.Register("/mutating", mutatingHook)