Fix admission webhook injection, no mgr, no client

The webhook server logic was not handling injection correctly -- it
implemented injection for a couple of specific fields, rather than
receiving and dealing with an injectfunc.  This fixes that.

As a side effect, it also removes the handle to manager and client from
the webhook server, since those aren't used by the server, and should
thus be injected from the manager.

Author: DirectXMan12

example/main.go

@@ -92,14 +92,15 @@ func main() {
 		Build()
 
 	entryLog.Info("setting up webhook server")
-	as, err := webhook.NewServer(mgr, webhook.ServerOptions{
+	as, err := webhook.NewServer(webhook.ServerOptions{
 		Port:    9876,
 		CertDir: "/tmp/cert",
 	})
 	if err != nil {
 		entryLog.Error(err, "unable to create a new webhook server")
 		os.Exit(1)
 	}
+	mgr.Add(as)
 
 	entryLog.Info("registering webhooks to the webhook server")
 	err = as.Register(mutatingWebhook, validatingWebhook)

pkg/webhook/admission/webhook.go

@@ -209,10 +209,12 @@ func (w *Webhook) Validate() error {
 	return nil
 }
 
-var _ inject.Injector = &Webhook{}
-
-// InjectFunc injects dependencies into the handlers.
+// InjectFunc injects the field setter into the webhook.
 func (w *Webhook) InjectFunc(f inject.Func) error {
+	// inject directly into the handlers.  It would be more correct
+	// to do this in a sync.Once in Handle (since we don't have some
+	// other start/finalize-type method), but it's more efficient to
+	// do it here, presumably.
 	for _, handler := range w.Handlers {
 		if err := f(handler); err != nil {
 			return err

pkg/webhook/server.go

@@ -25,7 +25,6 @@ import (
 	"strconv"
 	"sync"
 
-	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/runtime/inject"
 )
 
@@ -62,11 +61,8 @@ type Server struct {
 	// registry maps a path to a http.Handler.
 	registry map[string]http.Handler
 
-	// setFields is used to inject dependencies into webhooks
-	setFields func(i interface{}) error
-
-	// manager is the manager that this webhook server will be registered.
-	manager manager.Manager
+	// setFields allows injecting dependencies from an external source
+	setFields inject.Func
 
 	once sync.Once
 }
@@ -85,12 +81,11 @@ type Webhook interface {
 }
 
 // NewServer creates a new admission webhook server.
-func NewServer(mgr manager.Manager, options ServerOptions) (*Server, error) {
+func NewServer(options ServerOptions) (*Server, error) {
 	as := &Server{
 		sMux:          http.NewServeMux(),
 		registry:      map[string]http.Handler{},
 		ServerOptions: options,
-		manager:       mgr,
 	}
 
 	return as, nil
@@ -130,23 +125,27 @@ func (s *Server) Register(webhooks ...Webhook) error {
 		}
 	}
 
-	// Lazily add Server to manager.
-	// Because the all webhook handlers to be in place, so we can inject the things they need.
-	return s.manager.Add(s)
+	return nil
 }
 
 // Handle registers a http.Handler for the given pattern.
 func (s *Server) Handle(pattern string, handler http.Handler) {
 	s.sMux.Handle(pattern, handler)
 }
 
-var _ manager.Runnable = &Server{}
-
 // Start runs the server.
 // It will install the webhook related resources depend on the server configuration.
 func (s *Server) Start(stop <-chan struct{}) error {
 	s.once.Do(s.setDefault)
 
+	// inject fields here as opposed to in Register so that we're certain to have our setFields
+	// function available.
+	for _, webhook := range s.registry {
+		if err := s.setFields(webhook); err != nil {
+			return err
+		}
+	}
+
 	// TODO: watch the cert dir. Reload the cert if it changes
 	cert, err := tls.LoadX509KeyPair(path.Join(s.CertDir, certName), path.Join(s.CertDir, keyName))
 	if err != nil {
@@ -187,7 +186,7 @@ func (s *Server) Start(stop <-chan struct{}) error {
 	return nil
 }
 
-// InjectFunc injects dependencies into the handlers.
+// InjectFunc injects the field setter into the server.
 func (s *Server) InjectFunc(f inject.Func) error {
 	s.setFields = f
 	return nil