Merge pull request #81896 from liggitt/webhook-efficiency

Compute webhook selectors and client once per webhookconfig revision

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/BUILD

@@ -9,6 +9,9 @@ go_library(
     deps = [
         "//staging/src/k8s.io/api/admissionregistration/v1beta1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library",
+        "//staging/src/k8s.io/apiserver/pkg/util/webhook:go_default_library",
+        "//staging/src/k8s.io/client-go/rest:go_default_library",
     ],
 )
 

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/accessors.go

@@ -17,8 +17,13 @@ limitations under the License.
 package webhook
 
 import (
+	"sync"
+
 	"k8s.io/api/admissionregistration/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
+	webhookutil "k8s.io/apiserver/pkg/util/webhook"
+	"k8s.io/client-go/rest"
 )
 
 // WebhookAccessor provides a common interface to both mutating and validating webhook types.
@@ -29,6 +34,13 @@ type WebhookAccessor interface {
 	// GetConfigurationName gets the name of the webhook configuration that owns this webhook.
 	GetConfigurationName() string
 
+	// GetRESTClient gets the webhook client
+	GetRESTClient(clientManager *webhookutil.ClientManager) (*rest.RESTClient, error)
+	// GetParsedNamespaceSelector gets the webhook NamespaceSelector field.
+	GetParsedNamespaceSelector() (labels.Selector, error)
+	// GetParsedObjectSelector gets the webhook ObjectSelector field.
+	GetParsedObjectSelector() (labels.Selector, error)
+
 	// GetName gets the webhook Name field. Note that the name is scoped to the webhook
 	// configuration and does not provide a globally unique identity, if a unique identity is
 	// needed, use GetUID.
@@ -60,134 +72,226 @@ type WebhookAccessor interface {
 
 // NewMutatingWebhookAccessor creates an accessor for a MutatingWebhook.
 func NewMutatingWebhookAccessor(uid, configurationName string, h *v1beta1.MutatingWebhook) WebhookAccessor {
-	return mutatingWebhookAccessor{uid: uid, configurationName: configurationName, MutatingWebhook: h}
+	return &mutatingWebhookAccessor{uid: uid, configurationName: configurationName, MutatingWebhook: h}
 }
 
 type mutatingWebhookAccessor struct {
 	*v1beta1.MutatingWebhook
 	uid               string
 	configurationName string
+
+	initObjectSelector sync.Once
+	objectSelector     labels.Selector
+	objectSelectorErr  error
+
+	initNamespaceSelector sync.Once
+	namespaceSelector     labels.Selector
+	namespaceSelectorErr  error
+
+	initClient sync.Once
+	client     *rest.RESTClient
+	clientErr  error
 }
 
-func (m mutatingWebhookAccessor) GetUID() string {
+func (m *mutatingWebhookAccessor) GetUID() string {
 	return m.uid
 }
 
-func (m mutatingWebhookAccessor) GetConfigurationName() string {
+func (m *mutatingWebhookAccessor) GetConfigurationName() string {
 	return m.configurationName
 }
 
-func (m mutatingWebhookAccessor) GetName() string {
+func (m *mutatingWebhookAccessor) GetRESTClient(clientManager *webhookutil.ClientManager) (*rest.RESTClient, error) {
+	m.initClient.Do(func() {
+		m.client, m.clientErr = clientManager.HookClient(hookClientConfigForWebhook(m))
+	})
+	return m.client, m.clientErr
+}
+
+func (m *mutatingWebhookAccessor) GetParsedNamespaceSelector() (labels.Selector, error) {
+	m.initNamespaceSelector.Do(func() {
+		m.namespaceSelector, m.namespaceSelectorErr = metav1.LabelSelectorAsSelector(m.NamespaceSelector)
+	})
+	return m.namespaceSelector, m.namespaceSelectorErr
+}
+
+func (m *mutatingWebhookAccessor) GetParsedObjectSelector() (labels.Selector, error) {
+	m.initObjectSelector.Do(func() {
+		m.objectSelector, m.objectSelectorErr = metav1.LabelSelectorAsSelector(m.ObjectSelector)
+	})
+	return m.objectSelector, m.objectSelectorErr
+}
+
+func (m *mutatingWebhookAccessor) GetName() string {
 	return m.Name
 }
 
-func (m mutatingWebhookAccessor) GetClientConfig() v1beta1.WebhookClientConfig {
+func (m *mutatingWebhookAccessor) GetClientConfig() v1beta1.WebhookClientConfig {
 	return m.ClientConfig
 }
 
-func (m mutatingWebhookAccessor) GetRules() []v1beta1.RuleWithOperations {
+func (m *mutatingWebhookAccessor) GetRules() []v1beta1.RuleWithOperations {
 	return m.Rules
 }
 
-func (m mutatingWebhookAccessor) GetFailurePolicy() *v1beta1.FailurePolicyType {
+func (m *mutatingWebhookAccessor) GetFailurePolicy() *v1beta1.FailurePolicyType {
 	return m.FailurePolicy
 }
 
-func (m mutatingWebhookAccessor) GetMatchPolicy() *v1beta1.MatchPolicyType {
+func (m *mutatingWebhookAccessor) GetMatchPolicy() *v1beta1.MatchPolicyType {
 	return m.MatchPolicy
 }
 
-func (m mutatingWebhookAccessor) GetNamespaceSelector() *metav1.LabelSelector {
+func (m *mutatingWebhookAccessor) GetNamespaceSelector() *metav1.LabelSelector {
 	return m.NamespaceSelector
 }
 
-func (m mutatingWebhookAccessor) GetObjectSelector() *metav1.LabelSelector {
+func (m *mutatingWebhookAccessor) GetObjectSelector() *metav1.LabelSelector {
 	return m.ObjectSelector
 }
 
-func (m mutatingWebhookAccessor) GetSideEffects() *v1beta1.SideEffectClass {
+func (m *mutatingWebhookAccessor) GetSideEffects() *v1beta1.SideEffectClass {
 	return m.SideEffects
 }
 
-func (m mutatingWebhookAccessor) GetTimeoutSeconds() *int32 {
+func (m *mutatingWebhookAccessor) GetTimeoutSeconds() *int32 {
 	return m.TimeoutSeconds
 }
 
-func (m mutatingWebhookAccessor) GetAdmissionReviewVersions() []string {
+func (m *mutatingWebhookAccessor) GetAdmissionReviewVersions() []string {
 	return m.AdmissionReviewVersions
 }
 
-func (m mutatingWebhookAccessor) GetMutatingWebhook() (*v1beta1.MutatingWebhook, bool) {
+func (m *mutatingWebhookAccessor) GetMutatingWebhook() (*v1beta1.MutatingWebhook, bool) {
 	return m.MutatingWebhook, true
 }
 
-func (m mutatingWebhookAccessor) GetValidatingWebhook() (*v1beta1.ValidatingWebhook, bool) {
+func (m *mutatingWebhookAccessor) GetValidatingWebhook() (*v1beta1.ValidatingWebhook, bool) {
 	return nil, false
 }
 
 // NewValidatingWebhookAccessor creates an accessor for a ValidatingWebhook.
 func NewValidatingWebhookAccessor(uid, configurationName string, h *v1beta1.ValidatingWebhook) WebhookAccessor {
-	return validatingWebhookAccessor{uid: uid, configurationName: configurationName, ValidatingWebhook: h}
+	return &validatingWebhookAccessor{uid: uid, configurationName: configurationName, ValidatingWebhook: h}
 }
 
 type validatingWebhookAccessor struct {
 	*v1beta1.ValidatingWebhook
 	uid               string
 	configurationName string
+
+	initObjectSelector sync.Once
+	objectSelector     labels.Selector
+	objectSelectorErr  error
+
+	initNamespaceSelector sync.Once
+	namespaceSelector     labels.Selector
+	namespaceSelectorErr  error
+
+	initClient sync.Once
+	client     *rest.RESTClient
+	clientErr  error
 }
 
-func (v validatingWebhookAccessor) GetUID() string {
+func (v *validatingWebhookAccessor) GetUID() string {
 	return v.uid
 }
 
-func (v validatingWebhookAccessor) GetConfigurationName() string {
+func (v *validatingWebhookAccessor) GetConfigurationName() string {
 	return v.configurationName
 }
 
-func (v validatingWebhookAccessor) GetName() string {
+func (v *validatingWebhookAccessor) GetRESTClient(clientManager *webhookutil.ClientManager) (*rest.RESTClient, error) {
+	v.initClient.Do(func() {
+		v.client, v.clientErr = clientManager.HookClient(hookClientConfigForWebhook(v))
+	})
+	return v.client, v.clientErr
+}
+
+func (v *validatingWebhookAccessor) GetParsedNamespaceSelector() (labels.Selector, error) {
+	v.initNamespaceSelector.Do(func() {
+		v.namespaceSelector, v.namespaceSelectorErr = metav1.LabelSelectorAsSelector(v.NamespaceSelector)
+	})
+	return v.namespaceSelector, v.namespaceSelectorErr
+}
+
+func (v *validatingWebhookAccessor) GetParsedObjectSelector() (labels.Selector, error) {
+	v.initObjectSelector.Do(func() {
+		v.objectSelector, v.objectSelectorErr = metav1.LabelSelectorAsSelector(v.ObjectSelector)
+	})
+	return v.objectSelector, v.objectSelectorErr
+}
+
+func (v *validatingWebhookAccessor) GetName() string {
 	return v.Name
 }
 
-func (v validatingWebhookAccessor) GetClientConfig() v1beta1.WebhookClientConfig {
+func (v *validatingWebhookAccessor) GetClientConfig() v1beta1.WebhookClientConfig {
 	return v.ClientConfig
 }
 
-func (v validatingWebhookAccessor) GetRules() []v1beta1.RuleWithOperations {
+func (v *validatingWebhookAccessor) GetRules() []v1beta1.RuleWithOperations {
 	return v.Rules
 }
 
-func (v validatingWebhookAccessor) GetFailurePolicy() *v1beta1.FailurePolicyType {
+func (v *validatingWebhookAccessor) GetFailurePolicy() *v1beta1.FailurePolicyType {
 	return v.FailurePolicy
 }
 
-func (v validatingWebhookAccessor) GetMatchPolicy() *v1beta1.MatchPolicyType {
+func (v *validatingWebhookAccessor) GetMatchPolicy() *v1beta1.MatchPolicyType {
 	return v.MatchPolicy
 }
 
-func (v validatingWebhookAccessor) GetNamespaceSelector() *metav1.LabelSelector {
+func (v *validatingWebhookAccessor) GetNamespaceSelector() *metav1.LabelSelector {
 	return v.NamespaceSelector
 }
 
-func (v validatingWebhookAccessor) GetObjectSelector() *metav1.LabelSelector {
+func (v *validatingWebhookAccessor) GetObjectSelector() *metav1.LabelSelector {
 	return v.ObjectSelector
 }
 
-func (v validatingWebhookAccessor) GetSideEffects() *v1beta1.SideEffectClass {
+func (v *validatingWebhookAccessor) GetSideEffects() *v1beta1.SideEffectClass {
 	return v.SideEffects
 }
 
-func (v validatingWebhookAccessor) GetTimeoutSeconds() *int32 {
+func (v *validatingWebhookAccessor) GetTimeoutSeconds() *int32 {
 	return v.TimeoutSeconds
 }
 
-func (v validatingWebhookAccessor) GetAdmissionReviewVersions() []string {
+func (v *validatingWebhookAccessor) GetAdmissionReviewVersions() []string {
 	return v.AdmissionReviewVersions
 }
 
-func (v validatingWebhookAccessor) GetMutatingWebhook() (*v1beta1.MutatingWebhook, bool) {
+func (v *validatingWebhookAccessor) GetMutatingWebhook() (*v1beta1.MutatingWebhook, bool) {
 	return nil, false
 }
 
-func (v validatingWebhookAccessor) GetValidatingWebhook() (*v1beta1.ValidatingWebhook, bool) {
+func (v *validatingWebhookAccessor) GetValidatingWebhook() (*v1beta1.ValidatingWebhook, bool) {
 	return v.ValidatingWebhook, true
 }
+
+// hookClientConfigForWebhook construct a webhookutil.ClientConfig using a WebhookAccessor to access
+// v1beta1.MutatingWebhook and v1beta1.ValidatingWebhook API objects.  webhookutil.ClientConfig is used
+// to create a HookClient and the purpose of the config struct is to share that with other packages
+// that need to create a HookClient.
+func hookClientConfigForWebhook(w WebhookAccessor) webhookutil.ClientConfig {
+	ret := webhookutil.ClientConfig{Name: w.GetName(), CABundle: w.GetClientConfig().CABundle}
+	if w.GetClientConfig().URL != nil {
+		ret.URL = *w.GetClientConfig().URL
+	}
+	if w.GetClientConfig().Service != nil {
+		ret.Service = &webhookutil.ClientConfigService{
+			Name:      w.GetClientConfig().Service.Name,
+			Namespace: w.GetClientConfig().Service.Namespace,
+		}
+		if w.GetClientConfig().Service.Port != nil {
+			ret.Service.Port = *w.GetClientConfig().Service.Port
+		} else {
+			ret.Service.Port = 443
+		}
+		if w.GetClientConfig().Service.Path != nil {
+			ret.Service.Path = *w.GetClientConfig().Service.Path
+		}
+	}
+	return ret
+}

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/BUILD

@@ -28,7 +28,6 @@ go_library(
         "//staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/errors:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/request:go_default_library",
-        "//staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/util:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/apis/audit:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/util/webhook:go_default_library",
         "//vendor/github.com/evanphx/json-patch:go_default_library",

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/dispatcher.go

@@ -40,7 +40,6 @@ import (
 	webhookerrors "k8s.io/apiserver/pkg/admission/plugin/webhook/errors"
 	"k8s.io/apiserver/pkg/admission/plugin/webhook/generic"
 	webhookrequest "k8s.io/apiserver/pkg/admission/plugin/webhook/request"
-	"k8s.io/apiserver/pkg/admission/plugin/webhook/util"
 	auditinternal "k8s.io/apiserver/pkg/apis/audit"
 	webhookutil "k8s.io/apiserver/pkg/util/webhook"
 	utiltrace "k8s.io/utils/trace"
@@ -197,7 +196,7 @@ func (a *mutatingDispatcher) callAttrMutatingHook(ctx context.Context, h *v1beta
 		return false, &webhookutil.ErrCallingWebhook{WebhookName: h.Name, Reason: err}
 	}
 	// Make the webhook request
-	client, err := a.cm.HookClient(util.HookClientConfigForWebhook(invocation.Webhook))
+	client, err := invocation.Webhook.GetRESTClient(a.cm)
 	if err != nil {
 		return false, &webhookutil.ErrCallingWebhook{WebhookName: h.Name, Reason: err}
 	}

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/plugin_test.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"fmt"
 	"net/url"
+	"os"
 	"reflect"
 	"strings"
 	"testing"
@@ -33,6 +34,75 @@ import (
 	auditinternal "k8s.io/apiserver/pkg/apis/audit"
 )
 
+// BenchmarkAdmit tests the performance cost of invoking a mutating webhook
+func BenchmarkAdmit(b *testing.B) {
+	testServerURL := os.Getenv("WEBHOOK_TEST_SERVER_URL")
+	if len(testServerURL) == 0 {
+		b.Log("warning, WEBHOOK_TEST_SERVER_URL not set, starting in-process server, benchmarks will include webhook cost.")
+		b.Log("to run a standalone server, run:")
+		b.Log("go run ./vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/main/main.go")
+		testServer := webhooktesting.NewTestServer(b)
+		testServer.StartTLS()
+		defer testServer.Close()
+		testServerURL = testServer.URL
+	}
+
+	serverURL, err := url.ParseRequestURI(testServerURL)
+	if err != nil {
+		b.Fatalf("this should never happen? %v", err)
+	}
+
+	objectInterfaces := webhooktesting.NewObjectInterfacesForTest()
+
+	stopCh := make(chan struct{})
+	defer close(stopCh)
+
+	testCases := append(webhooktesting.NewMutatingTestCases(serverURL, "test-webhooks"),
+		webhooktesting.ConvertToMutatingTestCases(webhooktesting.NewNonMutatingTestCases(serverURL), "test-webhooks")...)
+
+	for _, tt := range testCases {
+		// For now, skip failure cases or tests that explicitly skip benchmarking
+		if !tt.ExpectAllow || tt.SkipBenchmark {
+			continue
+		}
+		b.Run(tt.Name, func(b *testing.B) {
+			wh, err := NewMutatingWebhook(nil)
+			if err != nil {
+				b.Errorf("failed to create mutating webhook: %v", err)
+				return
+			}
+
+			ns := "webhook-test"
+			client, informer := webhooktesting.NewFakeMutatingDataSource(ns, tt.Webhooks, stopCh)
+
+			wh.SetAuthenticationInfoResolverWrapper(webhooktesting.Wrapper(webhooktesting.NewAuthenticationInfoResolver(new(int32))))
+			wh.SetServiceResolver(webhooktesting.NewServiceResolver(*serverURL))
+			wh.SetExternalKubeClientSet(client)
+			wh.SetExternalKubeInformerFactory(informer)
+
+			informer.Start(stopCh)
+			informer.WaitForCacheSync(stopCh)
+
+			if err = wh.ValidateInitialization(); err != nil {
+				b.Errorf("failed to validate initialization: %v", err)
+				return
+			}
+
+			var attr admission.Attributes
+			if tt.IsCRD {
+				attr = webhooktesting.NewAttributeUnstructured(ns, tt.AdditionalLabels, tt.IsDryRun)
+			} else {
+				attr = webhooktesting.NewAttribute(ns, tt.AdditionalLabels, tt.IsDryRun)
+			}
+
+			b.ResetTimer()
+			for i := 0; i < b.N; i++ {
+				wh.Admit(context.TODO(), attr, objectInterfaces)
+			}
+		})
+	}
+}
+
 // TestAdmit tests that MutatingWebhook#Admit works as expected
 func TestAdmit(t *testing.T) {
 	testServer := webhooktesting.NewTestServer(t)

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/namespace/matcher.go

@@ -95,8 +95,7 @@ func (m *Matcher) MatchNamespaceSelector(h webhook.WebhookAccessor, attr admissi
 		// Also update the comment in types.go
 		return true, nil
 	}
-	// TODO: adding an LRU cache to cache the translation
+	selector, err := h.GetParsedNamespaceSelector()
-	selector, err := metav1.LabelSelectorAsSelector(h.GetNamespaceSelector())
 	if err != nil {
 		return false, apierrors.NewInternalError(err)
 	}

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/object/BUILD

@@ -12,7 +12,6 @@ go_library(
     deps = [
         "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/meta:go_default_library",
-        "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library",

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/object/matcher.go

@@ -19,7 +19,6 @@ package object
 import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apiserver/pkg/admission"
@@ -47,8 +46,7 @@ func matchObject(obj runtime.Object, selector labels.Selector) bool {
 // MatchObjectSelector decideds whether the request matches the ObjectSelector
 // of the webhook. Only when they match, the webhook is called.
 func (m *Matcher) MatchObjectSelector(h webhook.WebhookAccessor, attr admission.Attributes) (bool, *apierrors.StatusError) {
-	// TODO: adding an LRU cache to cache the translation
+	selector, err := h.GetParsedObjectSelector()
-	selector, err := metav1.LabelSelectorAsSelector(h.GetObjectSelector())
 	if err != nil {
 		return false, apierrors.NewInternalError(err)
 	}

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/BUILD

@@ -40,7 +40,10 @@ filegroup(
 
 filegroup(
     name = "all-srcs",
-    srcs = [":package-srcs"],
+    srcs = [
+        ":package-srcs",
+        "//staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/main:all-srcs",
+    ],
     tags = ["automanaged"],
     visibility = ["//visibility:public"],
 )

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/main/BUILD

@@ -0,0 +1,30 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library")
+
+go_library(
+    name = "go_default_library",
+    srcs = ["main.go"],
+    importmap = "k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/main",
+    importpath = "k8s.io/apiserver/pkg/admission/plugin/webhook/testing/main",
+    visibility = ["//visibility:private"],
+    deps = ["//staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing:go_default_library"],
+)
+
+go_binary(
+    name = "main",
+    embed = [":go_default_library"],
+    visibility = ["//visibility:public"],
+)
+
+filegroup(
+    name = "package-srcs",
+    srcs = glob(["**"]),
+    tags = ["automanaged"],
+    visibility = ["//visibility:private"],
+)
+
+filegroup(
+    name = "all-srcs",
+    srcs = [":package-srcs"],
+    tags = ["automanaged"],
+    visibility = ["//visibility:public"],
+)

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/main/main.go

@@ -0,0 +1,30 @@
+/*
+Copyright 2019 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package main
+
+import (
+	"fmt"
+
+	webhooktesting "k8s.io/apiserver/pkg/admission/plugin/webhook/testing"
+)
+
+func main() {
+	server := webhooktesting.NewTestServer(nil)
+	server.StartTLS()
+	fmt.Println("serving on", server.URL)
+	select {}
+}

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/testcase.go

@@ -217,6 +217,7 @@ type ValidatingTest struct {
 	IsCRD                  bool
 	IsDryRun               bool
 	AdditionalLabels       map[string]string
+	SkipBenchmark          bool
 	ExpectLabels           map[string]string
 	ExpectAllow            bool
 	ErrorContains          string
@@ -233,6 +234,7 @@ type MutatingTest struct {
 	IsCRD                  bool
 	IsDryRun               bool
 	AdditionalLabels       map[string]string
+	SkipBenchmark          bool
 	ExpectLabels           map[string]string
 	ExpectAllow            bool
 	ErrorContains          string
@@ -262,7 +264,7 @@ func ConvertToMutatingTestCases(tests []ValidatingTest, configurationName string
 				break
 			}
 		}
-		r[i] = MutatingTest{t.Name, ConvertToMutatingWebhooks(t.Webhooks), t.Path, t.IsCRD, t.IsDryRun, t.AdditionalLabels, t.ExpectLabels, t.ExpectAllow, t.ErrorContains, t.ExpectAnnotations, t.ExpectStatusCode, t.ExpectReinvokeWebhooks}
+		r[i] = MutatingTest{t.Name, ConvertToMutatingWebhooks(t.Webhooks), t.Path, t.IsCRD, t.IsDryRun, t.AdditionalLabels, t.SkipBenchmark, t.ExpectLabels, t.ExpectAllow, t.ErrorContains, t.ExpectAnnotations, t.ExpectStatusCode, t.ExpectReinvokeWebhooks}
 	}
 	return r
 }
@@ -404,7 +406,8 @@ func NewNonMutatingTestCases(url *url.URL) []ValidatingTest {
 				AdmissionReviewVersions: []string{"v1beta1"},
 			}},
 
-			ExpectAllow: true,
+			SkipBenchmark: true,
+			ExpectAllow:   true,
 		},
 		{
 			Name: "match & fail (but disallow because fail close on nil FailurePolicy)",
@@ -499,7 +502,8 @@ func NewNonMutatingTestCases(url *url.URL) []ValidatingTest {
 				ObjectSelector:          &metav1.LabelSelector{},
 				AdmissionReviewVersions: []string{"v1beta1"},
 			}},
-			ExpectAllow: true,
+			SkipBenchmark: true,
+			ExpectAllow:   true,
 		},
 		{
 			Name: "absent response and fail closed",

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/webhook_server.go

@@ -20,7 +20,6 @@ import (
 	"crypto/tls"
 	"crypto/x509"
 	"encoding/json"
-	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"testing"
@@ -31,11 +30,12 @@ import (
 )
 
 // NewTestServer returns a webhook test HTTPS server with fixed webhook test certs.
-func NewTestServer(t *testing.T) *httptest.Server {
+func NewTestServer(t testing.TB) *httptest.Server {
 	// Create the test webhook server
 	sCert, err := tls.X509KeyPair(testcerts.ServerCert, testcerts.ServerKey)
 	if err != nil {
-		t.Fatal(err)
+		t.Error(err)
+		t.FailNow()
 	}
 	rootCAs := x509.NewCertPool()
 	rootCAs.AppendCertsFromPEM(testcerts.CACert)
@@ -49,7 +49,7 @@ func NewTestServer(t *testing.T) *httptest.Server {
 }
 
 func webhookHandler(w http.ResponseWriter, r *http.Request) {
-	fmt.Printf("got req: %v\n", r.URL.Path)
+	// fmt.Printf("got req: %v\n", r.URL.Path)
 	switch r.URL.Path {
 	case "/internalErr":
 		http.Error(w, "webhook internal server error", http.StatusInternalServerError)

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/util/BUILD

@@ -6,10 +6,7 @@ go_library(
     importmap = "k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/util",
     importpath = "k8s.io/apiserver/pkg/admission/plugin/webhook/util",
     visibility = ["//visibility:public"],
-    deps = [
+    deps = ["//staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook:go_default_library"],
-        "//staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook:go_default_library",
-        "//staging/src/k8s.io/apiserver/pkg/util/webhook:go_default_library",
-    ],
 )
 
 filegroup(

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/util/client_config.go

@@ -18,35 +18,8 @@ package util
 
 import (
 	"k8s.io/apiserver/pkg/admission/plugin/webhook"
-	webhookutil "k8s.io/apiserver/pkg/util/webhook"
 )
 
-// HookClientConfigForWebhook construct a webhookutil.ClientConfig using a WebhookAccessor to access
-// v1beta1.MutatingWebhook and v1beta1.ValidatingWebhook API objects.  webhookutil.ClientConfig is used
-// to create a HookClient and the purpose of the config struct is to share that with other packages
-// that need to create a HookClient.
-func HookClientConfigForWebhook(w webhook.WebhookAccessor) webhookutil.ClientConfig {
-	ret := webhookutil.ClientConfig{Name: w.GetName(), CABundle: w.GetClientConfig().CABundle}
-	if w.GetClientConfig().URL != nil {
-		ret.URL = *w.GetClientConfig().URL
-	}
-	if w.GetClientConfig().Service != nil {
-		ret.Service = &webhookutil.ClientConfigService{
-			Name:      w.GetClientConfig().Service.Name,
-			Namespace: w.GetClientConfig().Service.Namespace,
-		}
-		if w.GetClientConfig().Service.Port != nil {
-			ret.Service.Port = *w.GetClientConfig().Service.Port
-		} else {
-			ret.Service.Port = 443
-		}
-		if w.GetClientConfig().Service.Path != nil {
-			ret.Service.Path = *w.GetClientConfig().Service.Path
-		}
-	}
-	return ret
-}
-
 // HasAdmissionReviewVersion check whether a version is accepted by a given webhook.
 func HasAdmissionReviewVersion(a string, w webhook.WebhookAccessor) bool {
 	for _, b := range w.GetAdmissionReviewVersions() {

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/BUILD

@@ -22,7 +22,6 @@ go_library(
         "//staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/errors:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/request:go_default_library",
-        "//staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/util:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/util/webhook:go_default_library",
         "//vendor/k8s.io/klog:go_default_library",
         "//vendor/k8s.io/utils/trace:go_default_library",

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/dispatcher.go

@@ -32,7 +32,6 @@ import (
 	webhookerrors "k8s.io/apiserver/pkg/admission/plugin/webhook/errors"
 	"k8s.io/apiserver/pkg/admission/plugin/webhook/generic"
 	webhookrequest "k8s.io/apiserver/pkg/admission/plugin/webhook/request"
-	"k8s.io/apiserver/pkg/admission/plugin/webhook/util"
 	webhookutil "k8s.io/apiserver/pkg/util/webhook"
 	"k8s.io/klog"
 	utiltrace "k8s.io/utils/trace"
@@ -158,7 +157,7 @@ func (d *validatingDispatcher) callHook(ctx context.Context, h *v1beta1.Validati
 		return &webhookutil.ErrCallingWebhook{WebhookName: h.Name, Reason: err}
 	}
 	// Make the webhook request
-	client, err := d.cm.HookClient(util.HookClientConfigForWebhook(invocation.Webhook))
+	client, err := invocation.Webhook.GetRESTClient(d.cm)
 	if err != nil {
 		return &webhookutil.ErrCallingWebhook{WebhookName: h.Name, Reason: err}
 	}

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/plugin_test.go

@@ -19,6 +19,7 @@ package validating
 import (
 	"context"
 	"net/url"
+	"os"
 	"strings"
 	"testing"
 
@@ -29,6 +30,68 @@ import (
 	auditinternal "k8s.io/apiserver/pkg/apis/audit"
 )
 
+// BenchmarkValidate tests that ValidatingWebhook#Validate works as expected
+func BenchmarkValidate(b *testing.B) {
+	testServerURL := os.Getenv("WEBHOOK_TEST_SERVER_URL")
+	if len(testServerURL) == 0 {
+		b.Log("warning, WEBHOOK_TEST_SERVER_URL not set, starting in-process server, benchmarks will include webhook cost.")
+		b.Log("to run a standalone server, run:")
+		b.Log("go run ./vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/main/main.go")
+		testServer := webhooktesting.NewTestServer(b)
+		testServer.StartTLS()
+		defer testServer.Close()
+		testServerURL = testServer.URL
+	}
+
+	objectInterfaces := webhooktesting.NewObjectInterfacesForTest()
+
+	serverURL, err := url.ParseRequestURI(testServerURL)
+	if err != nil {
+		b.Fatalf("this should never happen? %v", err)
+	}
+
+	stopCh := make(chan struct{})
+	defer close(stopCh)
+
+	for _, tt := range webhooktesting.NewNonMutatingTestCases(serverURL) {
+		// For now, skip failure cases or tests that explicitly skip benchmarking
+		if !tt.ExpectAllow || tt.SkipBenchmark {
+			continue
+		}
+
+		b.Run(tt.Name, func(b *testing.B) {
+			wh, err := NewValidatingAdmissionWebhook(nil)
+			if err != nil {
+				b.Errorf("%s: failed to create validating webhook: %v", tt.Name, err)
+				return
+			}
+
+			ns := "webhook-test"
+			client, informer := webhooktesting.NewFakeValidatingDataSource(ns, tt.Webhooks, stopCh)
+
+			wh.SetAuthenticationInfoResolverWrapper(webhooktesting.Wrapper(webhooktesting.NewAuthenticationInfoResolver(new(int32))))
+			wh.SetServiceResolver(webhooktesting.NewServiceResolver(*serverURL))
+			wh.SetExternalKubeClientSet(client)
+			wh.SetExternalKubeInformerFactory(informer)
+
+			informer.Start(stopCh)
+			informer.WaitForCacheSync(stopCh)
+
+			if err = wh.ValidateInitialization(); err != nil {
+				b.Errorf("%s: failed to validate initialization: %v", tt.Name, err)
+				return
+			}
+
+			attr := webhooktesting.NewAttribute(ns, nil, tt.IsDryRun)
+
+			b.ResetTimer()
+			for i := 0; i < b.N; i++ {
+				wh.Validate(context.TODO(), attr, objectInterfaces)
+			}
+		})
+	}
+}
+
 // TestValidate tests that ValidatingWebhook#Validate works as expected
 func TestValidate(t *testing.T) {
 	testServer := webhooktesting.NewTestServer(t)

vendor/modules.txt

@@ -1282,7 +1282,6 @@ k8s.io/apiserver/pkg/admission/plugin/webhook/namespace
 k8s.io/apiserver/pkg/admission/plugin/webhook/object
 k8s.io/apiserver/pkg/admission/plugin/webhook/request
 k8s.io/apiserver/pkg/admission/plugin/webhook/rules
-k8s.io/apiserver/pkg/admission/plugin/webhook/util
 k8s.io/apiserver/pkg/admission/plugin/webhook/validating
 k8s.io/apiserver/pkg/admission/testing
 k8s.io/apiserver/pkg/apis/apiserver