drop KVMap, cleanup godoc and tweak defaulting

Author: Mengqi Yu

example/main.go

@@ -102,7 +102,6 @@ func main() {
 	as, err := webhook.NewServer("foo-admission-server", mgr, webhook.ServerOptions{
 		Port:    9876,
 		CertDir: "/tmp/cert",
-		KVMap:   map[string]interface{}{"foo": "bar"},
 		BootstrapOptions: &webhook.BootstrapOptions{
 			Secret: &apitypes.NamespacedName{
 				Namespace: "default",

example/mutatingwebhook.go

@@ -18,11 +18,11 @@ package main
 
 import (
 	"context"
-	"fmt"
 	"net/http"
 
 	corev1 "k8s.io/api/core/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/runtime/inject"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission/types"
 )
@@ -46,24 +46,38 @@ func (a *podAnnotator) Handle(ctx context.Context, req types.Request) types.Resp
 	}
 	copy := pod.DeepCopy()
 
-	err = mutatePodsFn(ctx, copy)
+	err = a.mutatePodsFn(ctx, copy)
 	if err != nil {
 		return admission.ErrorResponse(http.StatusInternalServerError, err)
 	}
 	return admission.PatchResponse(pod, copy)
 }
 
 // mutatePodsFn add an annotation to the given pod
-func mutatePodsFn(ctx context.Context, pod *corev1.Pod) error {
-	v, ok := ctx.Value(admission.StringKey("foo")).(string)
-	if !ok {
-		return fmt.Errorf("the value associated with %v is expected to be a string", "foo")
+func (a *podAnnotator) mutatePodsFn(ctx context.Context, pod *corev1.Pod) error {
+	if pod.Annotations == nil {
+		pod.Annotations = map[string]string{}
 	}
-	anno := pod.Annotations
-	if anno == nil {
-		anno = map[string]string{}
-	}
-	anno["example-mutating-admission-webhook"] = v
-	pod.Annotations = anno
+	pod.Annotations["example-mutating-admission-webhook"] = "foo"
+	return nil
+}
+
+// podValidator implements inject.Client.
+// A client will be automatically injected.
+var _ inject.Client = &podValidator{}
+
+// InjectClient injects the client.
+func (v *podAnnotator) InjectClient(c client.Client) error {
+	v.client = c
+	return nil
+}
+
+// podValidator implements inject.Decoder.
+// A decoder will be automatically injected.
+var _ inject.Decoder = &podValidator{}
+
+// InjectDecoder injects the decoder.
+func (v *podAnnotator) InjectDecoder(d types.Decoder) error {
+	v.decoder = d
 	return nil
 }

example/validatingwebhook.go

@@ -23,6 +23,7 @@ import (
 
 	corev1 "k8s.io/api/core/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/runtime/inject"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission/types"
 )
@@ -45,30 +46,44 @@ func (v *podValidator) Handle(ctx context.Context, req types.Request) types.Resp
 		return admission.ErrorResponse(http.StatusBadRequest, err)
 	}
 
-	allowed, reason, err := validatePodsFn(ctx, pod)
+	allowed, reason, err := v.validatePodsFn(ctx, pod)
 	if err != nil {
 		return admission.ErrorResponse(http.StatusInternalServerError, err)
 	}
 	return admission.ValidationResponse(allowed, reason)
 }
 
-func validatePodsFn(ctx context.Context, pod *corev1.Pod) (bool, string, error) {
-	v, ok := ctx.Value(admission.StringKey("foo")).(string)
-	if !ok {
-		return false, "",
-			fmt.Errorf("the value associated with key %q is expected to be a string", v)
-	}
-	annotations := pod.GetAnnotations()
+func (v *podValidator) validatePodsFn(ctx context.Context, pod *corev1.Pod) (bool, string, error) {
 	key := "example-mutating-admission-webhook"
-	anno, found := annotations[key]
+	anno, found := pod.Annotations[key]
 	switch {
 	case !found:
 		return found, fmt.Sprintf("failed to find annotation with key: %q", key), nil
-	case found && anno == v:
+	case found && anno == "foo":
 		return found, "", nil
-	case found && anno != v:
+	case found && anno != "foo":
 		return false,
-			fmt.Sprintf("the value associate with key %q is expected to be %q, but got %q", "foo", v, anno), nil
+			fmt.Sprintf("the value associate with key %q is expected to be %q, but got %q", key, "foo", anno), nil
 	}
 	return false, "", nil
 }
+
+// podValidator implements inject.Client.
+// A client will be automatically injected.
+var _ inject.Client = &podValidator{}
+
+// InjectClient injects the client.
+func (v *podValidator) InjectClient(c client.Client) error {
+	v.client = c
+	return nil
+}
+
+// podValidator implements inject.Decoder.
+// A decoder will be automatically injected.
+var _ inject.Decoder = &podValidator{}
+
+// InjectDecoder injects the decoder.
+func (v *podValidator) InjectDecoder(d types.Decoder) error {
+	v.decoder = d
+	return nil
+}

pkg/webhook/admission/builder/builder.go

@@ -31,33 +31,40 @@ import (
 
 // WebhookBuilder builds a webhook based on the provided options.
 type WebhookBuilder struct {
-	// name specifies the Name of the webhook. It must be unique in the http
-	// server that serves all the webhooks.
+	// name specifies the name of the webhook. It must be unique among all webhooks.
 	name string
 
-	// path is the URL Path to register this webhook. e.g. "/feature-foo-mutating-pods".
+	// path is the URL Path to register this webhook. e.g. "/mutate-pods".
 	path string
 
-	// handlers are handlers for handling admission request.
+	// handlers handle admission requests.
+	// A WebhookBuilder may have multiple handlers.
+	// For example, handlers[0] mutates a pod for feature foo.
+	// handlers[1] mutates a pod for a different feature bar.
 	handlers []admission.Handler
 
-	// t specifies the type of the webhook
+	// t specifies the type of the webhook.
+	// Currently, Mutating and Validating are supported.
 	t *types.WebhookType
+
+	// operations define the operations this webhook cares.
 	// only one of operations and Rules can be set.
 	operations []admissionregistrationv1beta1.OperationType
-
-	// resources that this webhook cares.
+	// apiType represents the resource that this webhook cares.
 	// Only one of apiType and Rules can be set.
 	apiType runtime.Object
-	rules   []admissionregistrationv1beta1.RuleWithOperations
+	// rules contain a list of admissionregistrationv1beta1.RuleWithOperations
+	// It overrides operations and apiType.
+	rules []admissionregistrationv1beta1.RuleWithOperations
 
-	// This field maps to the FailurePolicy in the admissionregistrationv1beta1.Webhook
+	// failurePolicy maps to the FailurePolicy in the admissionregistrationv1beta1.Webhook
 	failurePolicy *admissionregistrationv1beta1.FailurePolicyType
 
-	// This field maps to the NamespaceSelector in the admissionregistrationv1beta1.Webhook
+	// namespaceSelector maps to the NamespaceSelector in the admissionregistrationv1beta1.Webhook
 	namespaceSelector *metav1.LabelSelector
 
 	// manager is the manager for the webhook.
+	// It is used for provisioning various dependencies for the webhook. e.g. RESTMapper.
 	manager manager.Manager
 }
 
@@ -66,7 +73,7 @@ func NewWebhookBuilder() *WebhookBuilder {
 	return &WebhookBuilder{}
 }
 
-// Name sets the Name of the webhook.
+// Name sets the name of the webhook.
 // This is optional
 func (b *WebhookBuilder) Name(name string) *WebhookBuilder {
 	b.name = name
@@ -89,8 +96,11 @@ func (b *WebhookBuilder) Validating() *WebhookBuilder {
 	return b
 }
 
-// Path sets the Path for the webhook.
-// This is optional
+// Path sets the path for the webhook.
+// Path needs to be unique among different webhooks.
+// This is optional. If not set, it will be built from the type and resource name.
+// For example, a webhook that mutates pods has a default path of "/mutate-pods"
+// If the defaulting logic can't find a unique path for it, user need to set it manually.
 func (b *WebhookBuilder) Path(path string) *WebhookBuilder {
 	b.path = path
 	return b
@@ -105,15 +115,15 @@ func (b *WebhookBuilder) Operations(ops ...admissionregistrationv1beta1.Operatio
 }
 
 // ForType sets the type of resources that the webhook will operate.
-// This cannot be use with Rules.
+// It will be overridden by Rules if Rules are not empty.
 func (b *WebhookBuilder) ForType(obj runtime.Object) *WebhookBuilder {
 	b.apiType = obj
 	return b
 }
 
 // Rules sets the RuleWithOperations for the webhook.
 // It overrides ForType and Operations.
-// This is optional and for advanced user
+// This is optional and for advanced user.
 func (b *WebhookBuilder) Rules(rules ...admissionregistrationv1beta1.RuleWithOperations) *WebhookBuilder {
 	b.rules = rules
 	return b
@@ -134,7 +144,7 @@ func (b *WebhookBuilder) NamespaceSelector(namespaceSelector *metav1.LabelSelect
 	return b
 }
 
-// WithManager set the manager for the webhook for provisioning client etc.
+// WithManager set the manager for the webhook for provisioning various dependencies. e.g. client etc.
 func (b *WebhookBuilder) WithManager(mgr manager.Manager) *WebhookBuilder {
 	b.manager = mgr
 	return b
@@ -208,14 +218,5 @@ func (b *WebhookBuilder) Build() (*admission.Webhook, error) {
 		}
 	}
 
-	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.Path = b.path
-
 	return w, nil
 }

pkg/webhook/admission/http.go

@@ -29,6 +29,7 @@ import (
 	admissionv1beta1 "k8s.io/api/admission/v1beta1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/serializer"
+	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission/types"
 )
 
@@ -40,20 +41,11 @@ func init() {
 }
 
 func addToScheme(scheme *runtime.Scheme) {
-	err := admissionv1beta1.AddToScheme(scheme)
-	// TODO: switch to use Must in
-	// https://github.com/kubernetes/kubernetes/blob/fbb2dfcc6ad345b0ca3fe09cb4bc2a23ec0781d5/staging/src/k8s.io/apimachinery/pkg/util/runtime/runtime.go#L164-L169
-	// after the apimachinery repo in vendor has been updated to 1.11 or later.
-	if err != nil {
-		panic(err)
-	}
+	utilruntime.Must(admissionv1beta1.AddToScheme(scheme))
 }
 
 var _ http.Handler = &Webhook{}
 
-// ContextKey is a type alias of string and is used as the key in context.
-type ContextKey string
-
 func (wh *Webhook) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	var body []byte
 	var err error
@@ -93,11 +85,7 @@ func (wh *Webhook) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	}
 
 	// TODO: add panic-recovery for Handle
-	ctx := context.Background()
-	for k := range wh.KVMap {
-		ctx = context.WithValue(ctx, ContextKey(k), wh.KVMap[k])
-	}
-	reviewResponse = wh.Handle(ctx, types.Request{AdmissionRequest: ar.Request})
+	reviewResponse = wh.Handle(context.Background(), types.Request{AdmissionRequest: ar.Request})
 	writeResponse(w, reviewResponse)
 }
 

pkg/webhook/admission/http_test.go

@@ -133,15 +133,13 @@ var _ = Describe("admission webhook http handler", func() {
 		wh := &Webhook{
 			Type:     types.WebhookTypeValidating,
 			Handlers: []Handler{h},
-			KVMap:    map[string]interface{}{"foo": "bar"},
 		}
 		expected := []byte(`{"response":{"uid":"","allowed":true}}
 `)
 		It("should return a response successfully", func() {
 			wh.ServeHTTP(w, req)
 			Expect(w.Body.Bytes()).To(Equal(expected))
 			Expect(h.invoked).To(BeTrue())
-			Expect(h.valueFromContext).To(Equal("bar"))
 		})
 	})
 })
@@ -153,19 +151,11 @@ type nopCloser struct {
 func (nopCloser) Close() error { return nil }
 
 type fakeHandler struct {
-	invoked          bool
-	valueFromContext string
-	fn               func(context.Context, atypes.Request) atypes.Response
+	invoked bool
+	fn      func(context.Context, atypes.Request) atypes.Response
 }
 
 func (h *fakeHandler) Handle(ctx context.Context, req atypes.Request) atypes.Response {
-	v := ctx.Value(ContextKey("foo"))
-	if v != nil {
-		typed, ok := v.(string)
-		if ok {
-			h.valueFromContext = typed
-		}
-	}
 	h.invoked = true
 	if h.fn != nil {
 		return h.fn(ctx, req)

pkg/webhook/admission/response.go

@@ -26,7 +26,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission/types"
 )
 
-// ErrorResponse creates a new Response for an error handling the request
+// ErrorResponse creates a new Response for error-handling a request.
 func ErrorResponse(code int32, err error) types.Response {
 	return types.Response{
 		Response: &admissionv1beta1.AdmissionResponse{
@@ -39,7 +39,7 @@ func ErrorResponse(code int32, err error) types.Response {
 	}
 }
 
-// ValidationResponse returns a response for admitting a request
+// ValidationResponse returns a response for admitting a request.
 func ValidationResponse(allowed bool, reason string) types.Response {
 	resp := types.Response{
 		Response: &admissionv1beta1.AdmissionResponse{
@@ -54,7 +54,7 @@ func ValidationResponse(allowed bool, reason string) types.Response {
 	return resp
 }
 
-// PatchResponse returns a new response with json patch
+// PatchResponse returns a new response with json patch.
 func PatchResponse(original, current runtime.Object) types.Response {
 	patches, err := patch.NewJSONPatch(original, current)
 	if err != nil {