From 8c10d929cac13dc50ca4ffaca83e7ae5c8e41292 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Sat, 24 Aug 2019 17:12:14 -0400 Subject: [PATCH] Use cached selectors/client for webhooks --- .../pkg/admission/plugin/webhook/BUILD | 3 ++ .../pkg/admission/plugin/webhook/accessors.go | 31 +++++++++++++++++-- .../admission/plugin/webhook/mutating/BUILD | 1 - .../plugin/webhook/mutating/dispatcher.go | 3 +- .../plugin/webhook/namespace/matcher.go | 3 +- .../pkg/admission/plugin/webhook/object/BUILD | 1 - .../plugin/webhook/object/matcher.go | 4 +-- .../pkg/admission/plugin/webhook/util/BUILD | 5 +-- .../plugin/webhook/util/client_config.go | 27 ---------------- .../admission/plugin/webhook/validating/BUILD | 1 - .../plugin/webhook/validating/dispatcher.go | 3 +- vendor/modules.txt | 1 - 12 files changed, 36 insertions(+), 47 deletions(-) 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 9c91594167d..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 @@ -18,7 +18,6 @@ package webhook import ( "sync" - "fmt" "k8s.io/api/admissionregistration/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -104,7 +103,7 @@ func (m *mutatingWebhookAccessor) GetConfigurationName() string { func (m *mutatingWebhookAccessor) GetRESTClient(clientManager *webhookutil.ClientManager) (*rest.RESTClient, error) { m.initClient.Do(func() { - m.client, m.clientErr = nil, fmt.Errorf("unimplemented") + m.client, m.clientErr = clientManager.HookClient(hookClientConfigForWebhook(m)) }) return m.client, m.clientErr } @@ -204,7 +203,7 @@ func (v *validatingWebhookAccessor) GetConfigurationName() string { func (v *validatingWebhookAccessor) GetRESTClient(clientManager *webhookutil.ClientManager) (*rest.RESTClient, error) { v.initClient.Do(func() { - v.client, v.clientErr = nil, fmt.Errorf("unimplemented") + v.client, v.clientErr = clientManager.HookClient(hookClientConfigForWebhook(v)) }) return v.client, v.clientErr } @@ -270,3 +269,29 @@ func (v *validatingWebhookAccessor) GetMutatingWebhook() (*v1beta1.MutatingWebho 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/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/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/vendor/modules.txt b/vendor/modules.txt index 4de00e10515..0d2339149b3 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1260,7 +1260,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