change how to set webhook type and how we default path in builder

Author: Mengqi Yu

example/main.go

@@ -34,7 +34,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/source"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission/builder"
-	"sigs.k8s.io/controller-runtime/pkg/webhook/types"
 )
 
 var log = logf.Log.WithName("example-controller")
@@ -76,8 +75,7 @@ func main() {
 	// Setup webhooks
 	mutatingWebhook, err := builder.NewWebhookBuilder().
 		Name("mutating.k8s.io").
-		Type(types.WebhookTypeMutating).
-		Path("/mutating-pods").
+		Mutating().
 		Operations(admissionregistrationv1beta1.Create, admissionregistrationv1beta1.Update).
 		WithManager(mgr).
 		ForType(&corev1.Pod{}).
@@ -90,8 +88,7 @@ func main() {
 
 	validatingWebhook, err := builder.NewWebhookBuilder().
 		Name("validating.k8s.io").
-		Type(types.WebhookTypeValidating).
-		Path("/validating-pods").
+		Validating().
 		Operations(admissionregistrationv1beta1.Create, admissionregistrationv1beta1.Update).
 		WithManager(mgr).
 		ForType(&corev1.Pod{}).
@@ -103,7 +100,7 @@ func main() {
 	}
 
 	as, err := webhook.NewServer("foo-admission-server", mgr, webhook.ServerOptions{
-		Port:    443,
+		Port:    9876,
 		CertDir: "/tmp/cert",
 		KVMap:   map[string]interface{}{"foo": "bar"},
 		BootstrapOptions: &webhook.BootstrapOptions{

example/mutatingwebhook.go

@@ -59,8 +59,11 @@ func mutatePodsFn(ctx context.Context, pod *corev1.Pod) error {
 	if !ok {
 		return fmt.Errorf("the value associated with %v is expected to be a string", "foo")
 	}
-	anno := pod.GetAnnotations()
+	anno := pod.Annotations
+	if anno == nil {
+		anno = map[string]string{}
+	}
 	anno["example-mutating-admission-webhook"] = v
-	pod.SetAnnotations(anno)
+	pod.Annotations = anno
 	return nil
 }

pkg/webhook/admission/builder/builder.go

@@ -73,10 +73,19 @@ func (b *WebhookBuilder) Name(name string) *WebhookBuilder {
 	return b
 }
 
-// Type sets the type of the admission webhook
-// This is required
-func (b *WebhookBuilder) Type(t types.WebhookType) *WebhookBuilder {
-	b.t = &t
+// Mutating sets the type to mutating admission webhook
+// Only one of Mutating and Validating can be invoked.
+func (b *WebhookBuilder) Mutating() *WebhookBuilder {
+	m := types.WebhookTypeMutating
+	b.t = &m
+	return b
+}
+
+// Validating sets the type to validating admission webhook
+// Only one of Mutating and Validating can be invoked.
+func (b *WebhookBuilder) Validating() *WebhookBuilder {
+	m := types.WebhookTypeValidating
+	b.t = &m
 	return b
 }
 
@@ -165,48 +174,48 @@ func (b *WebhookBuilder) Build() (*admission.Webhook, error) {
 		Handlers:          b.handlers,
 	}
 
-	if len(b.path) == 0 {
-		if *b.t == types.WebhookTypeMutating {
-			b.path = "/mutatingwebhook"
-		} else if *b.t == types.WebhookTypeValidating {
-			b.path = "/validatingwebhook"
-		}
-	}
-	w.Path = b.path
-
 	if b.rules != nil {
 		w.Rules = b.rules
-		return w, nil
-	}
+	} else {
+		if b.manager == nil {
+			return nil, errors.New("manager should be set using WithManager")
+		}
+		gvk, err := apiutil.GVKForObject(b.apiType, b.manager.GetScheme())
+		if err != nil {
+			return nil, err
+		}
+		mapper := b.manager.GetRESTMapper()
+		mapping, err := mapper.RESTMapping(gvk.GroupKind(), gvk.Version)
+		if err != nil {
+			return nil, err
+		}
 
-	if b.manager == nil {
-		return nil, errors.New("manager should be set using WithManager")
-	}
-	gvk, err := apiutil.GVKForObject(b.apiType, b.manager.GetScheme())
-	if err != nil {
-		return nil, err
-	}
-	mapper := b.manager.GetRESTMapper()
-	mapping, err := mapper.RESTMapping(gvk.GroupKind(), gvk.Version)
-	if err != nil {
-		return nil, err
+		if b.operations == nil {
+			b.operations = []admissionregistrationv1beta1.OperationType{
+				admissionregistrationv1beta1.Create,
+				admissionregistrationv1beta1.Update,
+			}
+		}
+		w.Rules = []admissionregistrationv1beta1.RuleWithOperations{
+			{
+				Operations: b.operations,
+				Rule: admissionregistrationv1beta1.Rule{
+					APIGroups:   []string{gvk.Group},
+					APIVersions: []string{gvk.Version},
+					Resources:   []string{mapping.Resource.Resource},
+				},
+			},
+		}
 	}
 
-	if b.operations == nil {
-		b.operations = []admissionregistrationv1beta1.OperationType{
-			admissionregistrationv1beta1.Create,
-			admissionregistrationv1beta1.Update,
+	if len(b.path) == 0 {
+		if *b.t == types.WebhookTypeMutating {
+			b.path = "/mutate-" + w.Rules[0].Resources[0]
+		} else if *b.t == types.WebhookTypeValidating {
+			b.path = "/validate-" + w.Rules[0].Resources[0]
 		}
 	}
-	w.Rules = []admissionregistrationv1beta1.RuleWithOperations{
-		{
-			Operations: b.operations,
-			Rule: admissionregistrationv1beta1.Rule{
-				APIGroups:   []string{gvk.Group},
-				APIVersions: []string{gvk.Version},
-				Resources:   []string{mapping.Resource.Resource},
-			},
-		},
-	}
+	w.Path = b.path
+
 	return w, nil
 }

pkg/webhook/admission/doc.go

@@ -21,8 +21,7 @@ Build webhooks
 
 	webhook1, err := NewWebhookBuilder().
 		Name("foo.k8s.io").
-		Type(WebhookTypeMutating).
-		Path("/mutating-pods").
+		Mutating().
 		Operations(admissionregistrationv1beta1.Create).
 		ForType(&corev1.Pod{}).
 		WithManager(mgr).
@@ -33,8 +32,7 @@ Build webhooks
 
 	webhook2, err := NewWebhookBuilder().
 		Name("bar.k8s.io").
-		Type(WebhookTypeValidating).
-		Path("/validating-deployment").
+		Validating().
 		Operations(admissionregistrationv1beta1.Create, admissionregistrationv1beta1.Update).
 		ForType(&appsv1.Deployment{}).
 		WithManager(mgr).