From 3547c6c4f1f86dca33095a9c702a7e9c56e06f5d Mon Sep 17 00:00:00 2001
From: Mengqi Yu <mengqiy@google.com>
Date: Tue, 11 Jun 2019 16:06:02 -0700
Subject: [PATCH] :bug: 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.
---
 pkg/builder/build.go      | 35 +++++++++++++++++++++++++++--------
 pkg/builder/build_test.go | 28 ++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 8 deletions(-)

diff --git a/pkg/builder/build.go b/pkg/builder/build.go
index 487786586b..5bb70e8ceb 100644
--- a/pkg/builder/build.go
+++ b/pkg/builder/build.go
@@ -18,6 +18,8 @@ package builder
 
 import (
 	"fmt"
+	"net/http"
+	"net/url"
 	"strings"
 
 	"k8s.io/apimachinery/pkg/runtime"
@@ -280,11 +282,15 @@ 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)
+			}
 		}
 	}
 
@@ -292,10 +298,15 @@ func (blder *Builder) doWebhook() error {
 		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)
diff --git a/pkg/builder/build_test.go b/pkg/builder/build_test.go
index 241f51c3f1..545f222a9e 100644
--- a/pkg/builder/build_test.go
+++ b/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() {