diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/BUILD b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/BUILD index 7bfb60fc731..6ac42aec05a 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/BUILD +++ b/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", ], ) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/accessors.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/accessors.go index 4442861234d..4d5243ead6d 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/accessors.go +++ b/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 +} diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/BUILD b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/BUILD index 366b917332b..791f55207d2 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/BUILD +++ b/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", diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/dispatcher.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/dispatcher.go index 83cba52e9cc..b69b5fa274a 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/dispatcher.go +++ b/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} } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/plugin_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/plugin_test.go index c036c9fd882..ff48879ec3b 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/plugin_test.go +++ b/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) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/namespace/matcher.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/namespace/matcher.go index 4530de3e608..eb7d5ec4c86 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/namespace/matcher.go +++ b/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 := metav1.LabelSelectorAsSelector(h.GetNamespaceSelector()) + selector, err := h.GetParsedNamespaceSelector() if err != nil { return false, apierrors.NewInternalError(err) } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/object/BUILD b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/object/BUILD index 6a069ce315b..06460672963 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/object/BUILD +++ b/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", diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/object/matcher.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/object/matcher.go index be341dd95c9..da1b1e0394c 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/object/matcher.go +++ b/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 := metav1.LabelSelectorAsSelector(h.GetObjectSelector()) + selector, err := h.GetParsedObjectSelector() if err != nil { return false, apierrors.NewInternalError(err) } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/BUILD b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/BUILD index c55234a7333..9578fd8ff38 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/BUILD +++ b/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"], ) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/main/BUILD b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/main/BUILD new file mode 100644 index 00000000000..cd67fcb97f5 --- /dev/null +++ b/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"], +) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/main/main.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/main/main.go new file mode 100644 index 00000000000..32b9da6157f --- /dev/null +++ b/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 {} +} diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/testcase.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/testcase.go index cbcee41db3e..03025462b6a 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/testcase.go +++ b/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", diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/webhook_server.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/webhook_server.go index 6a9ee5ff144..b6d446005b0 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/webhook_server.go +++ b/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) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/util/BUILD b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/util/BUILD index 3a9b036f76e..085d7ed0b8b 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/util/BUILD +++ b/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 = [ - "//staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook:go_default_library", - "//staging/src/k8s.io/apiserver/pkg/util/webhook:go_default_library", - ], + deps = ["//staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook:go_default_library"], ) filegroup( diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/util/client_config.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/util/client_config.go index b9907286fc5..2ed342321ea 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/util/client_config.go +++ b/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() { diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/BUILD b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/BUILD index 7c75e50e5ee..6f2fd166a7e 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/BUILD +++ b/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", diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/dispatcher.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/dispatcher.go index 3e1cb589aaf..653cf5a9de9 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/dispatcher.go +++ b/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} } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/plugin_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/plugin_test.go index f09c88e3865..fc7a604e17a 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/plugin_test.go +++ b/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) diff --git a/vendor/modules.txt b/vendor/modules.txt index a51b567a042..a46fcd6c464 100644 --- a/vendor/modules.txt +++ b/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