🐛 stop registering webhooks with same path when adding multiple controllers

If a kind has implemented Defaulter and(or) Validator interface(s),
multiple controllers for this kind should be able to be added to the
same controller manager.

Author: Mengqi Yu

pkg/builder/build.go

@@ -18,6 +18,8 @@ package builder
 
 import (
 	"fmt"
+	"net/http"
+	"net/url"
 	"strings"
 
 	"k8s.io/apimachinery/pkg/runtime"
@@ -280,22 +282,31 @@ func (blder *Builder) doWebhook() error {
 		mwh := admission.DefaultingWebhookFor(defaulter)
 		if mwh != nil {
 			path := generateMutatePath(gvk)
-			log.Info("Registering a mutating webhook",
-				"GVK", gvk,
-				"path", path)
 
-			blder.mgr.GetWebhookServer().Register(path, mwh)
+			// Checking if the path is already registered.
+			// If so, just skip it.
+			if !blder.isAlreadyHandled(path) {
+				log.Info("Registering a mutating webhook",
+					"GVK", gvk,
+					"path", path)
+				blder.mgr.GetWebhookServer().Register(path, mwh)
+			}
 		}
 	}
 
 	if validator, isValidator := blder.apiType.(admission.Validator); isValidator {
 		vwh := admission.ValidatingWebhookFor(validator)
 		if vwh != nil {
 			path := generateValidatePath(gvk)
-			log.Info("Registering a validating webhook",
-				"GVK", gvk,
-				"path", path)
-			blder.mgr.GetWebhookServer().Register(path, vwh)
+
+			// Checking if the path is already registered.
+			// If so, just skip it.
+			if !blder.isAlreadyHandled(path) {
+				log.Info("Registering a validating webhook",
+					"GVK", gvk,
+					"path", path)
+				blder.mgr.GetWebhookServer().Register(path, vwh)
+			}
 		}
 	}
 
@@ -306,6 +317,14 @@ func (blder *Builder) doWebhook() error {
 	return nil
 }
 
+func (blder *Builder) isAlreadyHandled(path string) bool {
+	h, p := blder.mgr.GetWebhookServer().WebhookMux.Handler(&http.Request{URL: &url.URL{Path: path}})
+	if p == path && h != nil {
+		return true
+	}
+	return false
+}
+
 func generateMutatePath(gvk schema.GroupVersionKind) string {
 	return "/mutate-" + strings.Replace(gvk.Group, ".", "-", -1) + "-" +
 		gvk.Version + "-" + strings.ToLower(gvk.Kind)

pkg/builder/build_test.go

@@ -350,6 +350,34 @@ var _ = Describe("application", func() {
 			Expect(w.Body).To(ContainSubstring(`"allowed":true`))
 			Expect(w.Body).To(ContainSubstring(`"code":200`))
 		})
+
+		It("should allow multiple controllers for the same kind", func() {
+			By("creating a controller manager")
+			m, err := manager.New(cfg, manager.Options{})
+			Expect(err).NotTo(HaveOccurred())
+
+			By("registering the type in the Scheme")
+			builder := scheme.Builder{GroupVersion: testDefaultValidatorGVK.GroupVersion()}
+			builder.Register(&TestDefaultValidator{}, &TestDefaultValidatorList{})
+			err = builder.AddToScheme(m.GetScheme())
+			Expect(err).NotTo(HaveOccurred())
+
+			By("creating the 1st controller")
+			ctrl1, err := ControllerManagedBy(m).
+				For(&TestDefaultValidator{}).
+				Owns(&appsv1.ReplicaSet{}).
+				Build(noop)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(ctrl1).NotTo(BeNil())
+
+			By("creating the 2nd controller")
+			ctrl2, err := ControllerManagedBy(m).
+				For(&TestDefaultValidator{}).
+				Owns(&appsv1.ReplicaSet{}).
+				Build(noop)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(ctrl2).NotTo(BeNil())
+		})
 	})
 
 	Describe("Start with SimpleController", func() {