Fix custom path for webhooks conflict

Signed-off-by: Damien Dassieu <[email protected]>

Author: damsien

pkg/builder/webhook.go

@@ -41,6 +41,7 @@ type WebhookBuilder struct {
 	customDefaulter      admission.CustomDefaulter
 	customDefaulterOpts  []admission.DefaulterOption
 	customValidator      admission.CustomValidator
+	customPath           string
 	validatingCustomPath string
 	defaultingCustomPath string
 	gvk                  schema.GroupVersionKind
@@ -96,6 +97,14 @@ func (blder *WebhookBuilder) RecoverPanic(recoverPanic bool) *WebhookBuilder {
 	return blder
 }
 
+// WithCustomPath overrides the webhook's default path by the customPath
+// Deprecated: WithCustomPath should not be used anymore.
+// Please use WithValidatingCustomPath or WithDefaultingCustomPath instead.
+func (blder *WebhookBuilder) WithCustomPath(customPath string) *WebhookBuilder {
+	blder.customPath = customPath
+	return blder
+}
+
 // WithValidatingCustomPath overrides the webhook's default validating path by the customPath
 func (blder *WebhookBuilder) WithValidatingCustomPath(customPath string) *WebhookBuilder {
 	blder.validatingCustomPath = customPath
@@ -146,6 +155,10 @@ func (blder *WebhookBuilder) setLogConstructor() {
 	}
 }
 
+func (blder *WebhookBuilder) isThereCustomPathConflict() bool {
+	return (blder.customPath != "" && blder.customDefaulter != nil && blder.customValidator != nil) || (blder.customPath != "" && blder.defaultingCustomPath != "") || (blder.customPath != "" && blder.validatingCustomPath != "")
+}
+
 func (blder *WebhookBuilder) registerWebhooks() error {
 	typ, err := blder.getType()
 	if err != nil {
@@ -157,6 +170,18 @@ func (blder *WebhookBuilder) registerWebhooks() error {
 		return err
 	}
 
+	if blder.isThereCustomPathConflict() {
+		return errors.New("only one of the custom defaulter or custom validtor should be set when using WithCustomPath. Otherwise, WithDefaultingCustomPath() and WithValidatingCustomPath() should be used.")
+	} else {
+		if blder.customPath != "" {
+			// isThereCustomPathConflict() already checks for potential conflicts.
+			// Since we are sure that only one of customDefaulter or customValidator will be used,
+			// we can set set both defaultingCustomPath and validatingCustomPath.
+			blder.defaultingCustomPath = blder.customPath
+			blder.validatingCustomPath = blder.customPath
+		}
+	}
+
 	// Register webhook(s) for type
 	err = blder.registerDefaultingWebhook()
 	if err != nil {

pkg/builder/webhook_test.go

@@ -817,6 +817,78 @@ func runTests(admissionReviewVersion string) {
 		svr.WebhookMux().ServeHTTP(w, req)
 		ExpectWithOffset(1, w.Code).To(Equal(http.StatusNotFound))
 	})
+
+	It("should not scaffold a custom defaulting and a custom validating webhook with the same custom path", func() {
+		By("creating a controller manager")
+		m, err := manager.New(cfg, manager.Options{})
+		ExpectWithOffset(1, err).NotTo(HaveOccurred())
+
+		By("registering the type in the Scheme")
+		builder := scheme.Builder{GroupVersion: testValidatorGVK.GroupVersion()}
+		builder.Register(&TestDefaultValidator{}, &TestDefaultValidatorList{})
+		err = builder.AddToScheme(m.GetScheme())
+		ExpectWithOffset(1, err).NotTo(HaveOccurred())
+
+		customPath := "/custom-path"
+		err = WebhookManagedBy(m).
+			For(&TestDefaultValidator{}).
+			WithDefaulter(&TestCustomDefaultValidator{}).
+			WithValidator(&TestCustomDefaultValidator{}).
+			WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger {
+				return admission.DefaultLogConstructor(testingLogger, req)
+			}).
+			WithCustomPath(customPath).
+			Complete()
+		ExpectWithOffset(1, err).To(HaveOccurred())
+	})
+
+	It("should not scaffold a custom defaulting when setting a custom path and a defaulting custom path", func() {
+		By("creating a controller manager")
+		m, err := manager.New(cfg, manager.Options{})
+		ExpectWithOffset(1, err).NotTo(HaveOccurred())
+
+		By("registering the type in the Scheme")
+		builder := scheme.Builder{GroupVersion: testValidatorGVK.GroupVersion()}
+		builder.Register(&TestDefaultValidator{}, &TestDefaultValidatorList{})
+		err = builder.AddToScheme(m.GetScheme())
+		ExpectWithOffset(1, err).NotTo(HaveOccurred())
+
+		customPath := "/custom-path"
+		err = WebhookManagedBy(m).
+			For(&TestDefaulter{}).
+			WithDefaulter(&TestCustomDefaulter{}).
+			WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger {
+				return admission.DefaultLogConstructor(testingLogger, req)
+			}).
+			WithDefaultingCustomPath(customPath).
+			WithCustomPath(customPath).
+			Complete()
+		ExpectWithOffset(1, err).To(HaveOccurred())
+	})
+
+	It("should not scaffold a custom defaulting when setting a custom path and a validating custom path", func() {
+		By("creating a controller manager")
+		m, err := manager.New(cfg, manager.Options{})
+		ExpectWithOffset(1, err).NotTo(HaveOccurred())
+
+		By("registering the type in the Scheme")
+		builder := scheme.Builder{GroupVersion: testValidatorGVK.GroupVersion()}
+		builder.Register(&TestDefaultValidator{}, &TestDefaultValidatorList{})
+		err = builder.AddToScheme(m.GetScheme())
+		ExpectWithOffset(1, err).NotTo(HaveOccurred())
+
+		customPath := "/custom-path"
+		err = WebhookManagedBy(m).
+			For(&TestValidator{}).
+			WithValidator(&TestCustomValidator{}).
+			WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger {
+				return admission.DefaultLogConstructor(testingLogger, req)
+			}).
+			WithDefaultingCustomPath(customPath).
+			WithCustomPath(customPath).
+			Complete()
+		ExpectWithOffset(1, err).To(HaveOccurred())
+	})
 }
 
 // TestDefaulter.