From 91cdd8c9539c4860c5bacad36ebf75e4271f726d Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Mon, 1 May 2023 11:10:11 -0400 Subject: [PATCH] :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 | 4 +- pkg/builder/webhook_test.go | 28 +++---- pkg/manager/internal.go | 4 +- pkg/manager/manager.go | 12 +-- pkg/manager/manager_test.go | 41 +++++++--- pkg/manager/runnable_group.go | 2 +- pkg/manager/runnable_group_test.go | 2 +- pkg/webhook/example_test.go | 8 +- pkg/webhook/server.go | 103 +++++++++++++++++------- pkg/webhook/server_test.go | 14 ++-- pkg/webhook/webhook_integration_test.go | 4 +- 11 files changed, 142 insertions(+), 80 deletions(-) diff --git a/pkg/builder/webhook.go b/pkg/builder/webhook.go index 4cb971cea4..d2fd310e89 100644 --- a/pkg/builder/webhook.go +++ b/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 } diff --git a/pkg/builder/webhook_test.go b/pkg/builder/webhook_test.go index fee86562bc..54df3919cc 100644 --- a/pkg/builder/webhook_test.go +++ b/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`)) diff --git a/pkg/manager/internal.go b/pkg/manager/internal.go index 06a11c6f11..9313c143db 100644 --- a/pkg/manager/internal.go +++ b/pkg/manager/internal.go @@ -129,7 +129,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 @@ -276,7 +276,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") diff --git a/pkg/manager/manager.go b/pkg/manager/manager.go index 0fa8162a08..e1e92a30c7 100644 --- a/pkg/manager/manager.go +++ b/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.NewServer(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.NewServer(webhook.Options{ Host: options.Host, Port: options.Port, CertDir: options.CertDir, TLSOpts: options.TLSOpts, - } + }) } return options diff --git a/pkg/manager/manager_test.go b/pkg/manager/manager_test.go index fe97b1aa7e..1a7c257c25 100644 --- a/pkg/manager/manager_test.go +++ b/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.NewServer(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).Options.Port).To(Equal(8080)) + Expect(m.WebhookServer.(*webhook.DefaultServer).Options.Host).To(Equal("example.com")) + Expect(m.WebhookServer.(*webhook.DefaultServer).Options.CertDir).To(Equal("/pki")) + Expect(m.WebhookServer.(*webhook.DefaultServer).Options.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.NewServer(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).Options.Port).To(Equal(9440)) + Expect(svr.(*webhook.DefaultServer).Options.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).Options.Port).To(Equal(9440)) + Expect(svr.(*webhook.DefaultServer).Options.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.NewServer(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).Options.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() { diff --git a/pkg/manager/runnable_group.go b/pkg/manager/runnable_group.go index f7b91a209f..549741e6e5 100644 --- a/pkg/manager/runnable_group.go +++ b/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() { diff --git a/pkg/manager/runnable_group_test.go b/pkg/manager/runnable_group_test.go index 2122f23656..456b8d7ac0 100644 --- a/pkg/manager/runnable_group_test.go +++ b/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.NewServer(webhook.Options{}) r := newRunnables(defaultBaseContext, errCh) Expect(r.Add(webhook)).To(Succeed()) Expect(r.Webhooks.startQueue).To(HaveLen(1)) diff --git a/pkg/webhook/example_test.go b/pkg/webhook/example_test.go index 7c9fbfe24b..f68008755d 100644 --- a/pkg/webhook/example_test.go +++ b/pkg/webhook/example_test.go @@ -61,9 +61,9 @@ func Example() { } // Create a webhook server. - hookServer := &Server{ + hookServer := NewServer(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 := NewServer(Options{ Port: 8443, - } + }) // Register the webhooks in the server. hookServer.Register("/mutating", mutatingHook) diff --git a/pkg/webhook/server.go b/pkg/webhook/server.go index 1e21da71d2..23d5bf4350 100644 --- a/pkg/webhook/server.go +++ b/pkg/webhook/server.go @@ -46,7 +46,29 @@ var DefaultPort = 9443 // at the default locations (tls.crt and tls.key). If you do not // want to configure TLS (i.e for testing purposes) run an // admission.StandaloneWebhook in your own server. -type Server struct { +type Server interface { + // NeedLeaderElection implements the LeaderElectionRunnable interface, which indicates + // the webhook server doesn't need leader election. + NeedLeaderElection() bool + + // Register marks the given webhook as being served at the given path. + // It panics if two hooks are registered on the same path. + Register(path string, hook http.Handler) + + // Start runs the server. + // It will install the webhook related resources depend on the server configuration. + Start(ctx context.Context) error + + // StartedChecker returns an healthz.Checker which is healthy after the + // server has been started. + StartedChecker() healthz.Checker + + // WebhookMux returns the servers WebhookMux + WebhookMux() *http.ServeMux +} + +// Options are all the available options for a webhook.Server +type Options struct { // Host is the address that the server will listen on. // Defaults to "" - all addresses. Host string @@ -83,6 +105,18 @@ type Server struct { // WebhookMux is the multiplexer that handles different webhooks. WebhookMux *http.ServeMux +} + +// NewServer constructs a new Server from the provided options. +func NewServer(o Options) Server { + return &DefaultServer{ + Options: o, + } +} + +// DefaultServer is the default implementation used for Server. +type DefaultServer struct { + Options Options // webhooks keep track of all registered webhooks webhooks map[string]http.Handler @@ -96,41 +130,49 @@ type Server struct { // mu protects access to the webhook map & setFields for Start, Register, etc mu sync.Mutex + + webhookMux *http.ServeMux } // setDefaults does defaulting for the Server. -func (s *Server) setDefaults() { - s.webhooks = map[string]http.Handler{} - if s.WebhookMux == nil { - s.WebhookMux = http.NewServeMux() +func (o *Options) setDefaults() { + if o.WebhookMux == nil { + o.WebhookMux = http.NewServeMux() } - if s.Port <= 0 { - s.Port = DefaultPort + if o.Port <= 0 { + o.Port = DefaultPort } - if len(s.CertDir) == 0 { - s.CertDir = filepath.Join(os.TempDir(), "k8s-webhook-server", "serving-certs") + if len(o.CertDir) == 0 { + o.CertDir = filepath.Join(os.TempDir(), "k8s-webhook-server", "serving-certs") } - if len(s.CertName) == 0 { - s.CertName = "tls.crt" + if len(o.CertName) == 0 { + o.CertName = "tls.crt" } - if len(s.KeyName) == 0 { - s.KeyName = "tls.key" + if len(o.KeyName) == 0 { + o.KeyName = "tls.key" } } +func (s *DefaultServer) setDefaults() { + s.webhooks = map[string]http.Handler{} + s.Options.setDefaults() + + s.webhookMux = s.Options.WebhookMux +} + // NeedLeaderElection implements the LeaderElectionRunnable interface, which indicates // the webhook server doesn't need leader election. -func (*Server) NeedLeaderElection() bool { +func (*DefaultServer) NeedLeaderElection() bool { return false } // Register marks the given webhook as being served at the given path. // It panics if two hooks are registered on the same path. -func (s *Server) Register(path string, hook http.Handler) { +func (s *DefaultServer) Register(path string, hook http.Handler) { s.mu.Lock() defer s.mu.Unlock() @@ -139,7 +181,7 @@ func (s *Server) Register(path string, hook http.Handler) { panic(fmt.Errorf("can't register duplicate path: %v", path)) } s.webhooks[path] = hook - s.WebhookMux.Handle(path, metrics.InstrumentedHook(path, hook)) + s.webhookMux.Handle(path, metrics.InstrumentedHook(path, hook)) regLog := log.WithValues("path", path) regLog.Info("Registering webhook") @@ -167,13 +209,13 @@ func tlsVersion(version string) (uint16, error) { // Start runs the server. // It will install the webhook related resources depend on the server configuration. -func (s *Server) Start(ctx context.Context) error { +func (s *DefaultServer) Start(ctx context.Context) error { s.defaultingOnce.Do(s.setDefaults) baseHookLog := log.WithName("webhooks") baseHookLog.Info("Starting webhook server") - tlsMinVersion, err := tlsVersion(s.TLSMinVersion) + tlsMinVersion, err := tlsVersion(s.Options.TLSMinVersion) if err != nil { return err } @@ -183,13 +225,13 @@ func (s *Server) Start(ctx context.Context) error { MinVersion: tlsMinVersion, } // fallback TLS config ready, will now mutate if passer wants full control over it - for _, op := range s.TLSOpts { + for _, op := range s.Options.TLSOpts { op(cfg) } if cfg.GetCertificate == nil { - certPath := filepath.Join(s.CertDir, s.CertName) - keyPath := filepath.Join(s.CertDir, s.KeyName) + certPath := filepath.Join(s.Options.CertDir, s.Options.CertName) + keyPath := filepath.Join(s.Options.CertDir, s.Options.KeyName) // Create the certificate watcher and // set the config's GetCertificate on the TLSConfig @@ -207,9 +249,9 @@ func (s *Server) Start(ctx context.Context) error { } // Load CA to verify client certificate, if configured. - if s.ClientCAName != "" { + if s.Options.ClientCAName != "" { certPool := x509.NewCertPool() - clientCABytes, err := os.ReadFile(filepath.Join(s.CertDir, s.ClientCAName)) + clientCABytes, err := os.ReadFile(filepath.Join(s.Options.CertDir, s.Options.ClientCAName)) if err != nil { return fmt.Errorf("failed to read client CA cert: %w", err) } @@ -223,14 +265,14 @@ func (s *Server) Start(ctx context.Context) error { cfg.ClientAuth = tls.RequireAndVerifyClientCert } - listener, err := tls.Listen("tcp", net.JoinHostPort(s.Host, strconv.Itoa(s.Port)), cfg) + listener, err := tls.Listen("tcp", net.JoinHostPort(s.Options.Host, strconv.Itoa(s.Options.Port)), cfg) if err != nil { return err } - log.Info("Serving webhook server", "host", s.Host, "port", s.Port) + log.Info("Serving webhook server", "host", s.Options.Host, "port", s.Options.Port) - srv := httpserver.New(s.WebhookMux) + srv := httpserver.New(s.webhookMux) idleConnsClosed := make(chan struct{}) go func() { @@ -259,7 +301,7 @@ func (s *Server) Start(ctx context.Context) error { // StartedChecker returns an healthz.Checker which is healthy after the // server has been started. -func (s *Server) StartedChecker() healthz.Checker { +func (s *DefaultServer) StartedChecker() healthz.Checker { config := &tls.Config{ InsecureSkipVerify: true, //nolint:gosec // config is used to connect to our own webhook port. } @@ -272,7 +314,7 @@ func (s *Server) StartedChecker() healthz.Checker { } d := &net.Dialer{Timeout: 10 * time.Second} - conn, err := tls.DialWithDialer(d, "tcp", net.JoinHostPort(s.Host, strconv.Itoa(s.Port)), config) + conn, err := tls.DialWithDialer(d, "tcp", net.JoinHostPort(s.Options.Host, strconv.Itoa(s.Options.Port)), config) if err != nil { return fmt.Errorf("webhook server is not reachable: %w", err) } @@ -284,3 +326,8 @@ func (s *Server) StartedChecker() healthz.Checker { return nil } } + +// WebhookMux returns the servers WebhookMux +func (s *DefaultServer) WebhookMux() *http.ServeMux { + return s.webhookMux +} diff --git a/pkg/webhook/server_test.go b/pkg/webhook/server_test.go index 7a79c2f32f..9702b0fd6e 100644 --- a/pkg/webhook/server_test.go +++ b/pkg/webhook/server_test.go @@ -39,7 +39,7 @@ var _ = Describe("Webhook Server", func() { ctxCancel context.CancelFunc testHostPort string client *http.Client - server *webhook.Server + server webhook.Server servingOpts envtest.WebhookInstallOptions ) @@ -61,11 +61,11 @@ var _ = Describe("Webhook Server", func() { Transport: clientTransport, } - server = &webhook.Server{ + server = webhook.NewServer(webhook.Options{ Host: servingOpts.LocalServingHost, Port: servingOpts.LocalServingPort, CertDir: servingOpts.LocalServingCertDir, - } + }) }) AfterEach(func() { Expect(servingOpts.Cleanup()).To(Succeed()) @@ -172,7 +172,7 @@ var _ = Describe("Webhook Server", func() { // save cfg after changes to test against finalCfg = cfg } - server = &webhook.Server{ + server = webhook.NewServer(webhook.Options{ Host: servingOpts.LocalServingHost, Port: servingOpts.LocalServingPort, CertDir: servingOpts.LocalServingCertDir, @@ -180,7 +180,7 @@ var _ = Describe("Webhook Server", func() { TLSOpts: []func(*tls.Config){ tlsCfgFunc, }, - } + }) server.Register("/somepath", &testHandler{}) doneCh := genericStartServer(func(ctx context.Context) { Expect(server.Start(ctx)).To(Succeed()) @@ -212,7 +212,7 @@ var _ = Describe("Webhook Server", func() { finalGetCertificate := func(_ *tls.ClientHelloInfo) (*tls.Certificate, error) { //nolint:unparam return &finalCert, nil } - server = &webhook.Server{ + server = &webhook.DefaultServer{Options: webhook.Options{ Host: servingOpts.LocalServingHost, Port: servingOpts.LocalServingPort, CertDir: servingOpts.LocalServingCertDir, @@ -224,7 +224,7 @@ var _ = Describe("Webhook Server", func() { finalCfg = cfg }, }, - } + }} server.Register("/somepath", &testHandler{}) doneCh := genericStartServer(func(ctx context.Context) { Expect(server.Start(ctx)).To(Succeed()) diff --git a/pkg/webhook/webhook_integration_test.go b/pkg/webhook/webhook_integration_test.go index 60f6b2d6be..c91ad699e0 100644 --- a/pkg/webhook/webhook_integration_test.go +++ b/pkg/webhook/webhook_integration_test.go @@ -122,11 +122,11 @@ var _ = Describe("Webhook", func() { }) Context("when running a webhook server without a manager", func() { It("should reject create request for webhook that rejects all requests", func() { - server := webhook.Server{ + server := webhook.NewServer(webhook.Options{ Port: testenv.WebhookInstallOptions.LocalServingPort, Host: testenv.WebhookInstallOptions.LocalServingHost, CertDir: testenv.WebhookInstallOptions.LocalServingCertDir, - } + }) server.Register("/failing", &webhook.Admission{Handler: &rejectingValidator{d: admission.NewDecoder(testenv.Scheme)}}) ctx, cancel := context.WithCancel(context.Background())