diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/BUILD b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/BUILD index edb175111eb..6fd9b8b3b50 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/BUILD @@ -53,8 +53,11 @@ go_test( embed = [":go_default_library"], deps = [ "//staging/src/k8s.io/api/admissionregistration/v1:go_default_library", + "//staging/src/k8s.io/api/core/v1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured: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/apimachinery/pkg/runtime/schema:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library", diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/webhook.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/webhook.go index 6fc0eff297d..c04225e94f7 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/webhook.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/webhook.go @@ -141,7 +141,18 @@ func (a *Webhook) ValidateInitialization() error { // ShouldCallHook returns invocation details if the webhook should be called, nil if the webhook should not be called, // or an error if an error was encountered during evaluation. func (a *Webhook) ShouldCallHook(h webhook.WebhookAccessor, attr admission.Attributes, o admission.ObjectInterfaces) (*WebhookInvocation, *apierrors.StatusError) { - var err *apierrors.StatusError + matches, matchNsErr := a.namespaceMatcher.MatchNamespaceSelector(h, attr) + // Should not return an error here for webhooks which do not apply to the request, even if err is an unexpected scenario. + if !matches && matchNsErr == nil { + return nil, nil + } + + // Should not return an error here for webhooks which do not apply to the request, even if err is an unexpected scenario. + matches, matchObjErr := a.objectMatcher.MatchObjectSelector(h, attr) + if !matches && matchObjErr == nil { + return nil, nil + } + var invocation *WebhookInvocation for _, r := range h.GetRules() { m := rules.Matcher{Rule: r, Attr: attr} @@ -189,15 +200,11 @@ func (a *Webhook) ShouldCallHook(h webhook.WebhookAccessor, attr admission.Attri if invocation == nil { return nil, nil } - - matches, err := a.namespaceMatcher.MatchNamespaceSelector(h, attr) - if !matches || err != nil { - return nil, err + if matchNsErr != nil { + return nil, matchNsErr } - - matches, err = a.objectMatcher.MatchObjectSelector(h, attr) - if !matches || err != nil { - return nil, err + if matchObjErr != nil { + return nil, matchObjErr } return invocation, nil diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/webhook_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/webhook_test.go index 1f34f1242ab..26dbefc1991 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/webhook_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/webhook_test.go @@ -22,7 +22,10 @@ import ( "testing" "k8s.io/api/admissionregistration/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apiserver/pkg/admission" @@ -75,7 +78,7 @@ func TestShouldCallHook(t *testing.T) { }{ { name: "no rules (just write)", - webhook: &v1.ValidatingWebhook{Rules: []v1.RuleWithOperations{}}, + webhook: &v1.ValidatingWebhook{NamespaceSelector: &metav1.LabelSelector{}, Rules: []v1.RuleWithOperations{}}, attrs: admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{"apps", "v1", "Deployment"}, "ns", "name", schema.GroupVersionResource{"apps", "v1", "deployments"}, "", admission.Create, &metav1.CreateOptions{}, false, nil), expectCall: false, }, @@ -329,3 +332,228 @@ func TestShouldCallHook(t *testing.T) { }) } } + +type fakeNamespaceLister struct { + namespaces map[string]*corev1.Namespace +} + +func (f fakeNamespaceLister) List(selector labels.Selector) (ret []*corev1.Namespace, err error) { + return nil, nil +} +func (f fakeNamespaceLister) Get(name string) (*corev1.Namespace, error) { + ns, ok := f.namespaces[name] + if ok { + return ns, nil + } + return nil, errors.NewNotFound(corev1.Resource("namespaces"), name) +} + +func BenchmarkShouldCallHookWithComplexSelector(b *testing.B) { + allScopes := v1.AllScopes + equivalentMatch := v1.Equivalent + + namespace1Labels := map[string]string{"ns": "ns1"} + namespace1 := corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ns1", + Labels: namespace1Labels, + }, + } + namespaceLister := fakeNamespaceLister{map[string]*corev1.Namespace{"ns": &namespace1}} + + mapper := runtime.NewEquivalentResourceRegistryWithIdentity(func(resource schema.GroupResource) string { + if resource.Resource == "deployments" { + // co-locate deployments in all API groups + return "/deployments" + } + return "" + }) + mapper.RegisterKindFor(schema.GroupVersionResource{"extensions", "v1beta1", "deployments"}, "", schema.GroupVersionKind{"extensions", "v1beta1", "Deployment"}) + mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1", "deployments"}, "", schema.GroupVersionKind{"apps", "v1", "Deployment"}) + mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1beta1", "deployments"}, "", schema.GroupVersionKind{"apps", "v1beta1", "Deployment"}) + mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1alpha1", "deployments"}, "", schema.GroupVersionKind{"apps", "v1alpha1", "Deployment"}) + + mapper.RegisterKindFor(schema.GroupVersionResource{"extensions", "v1beta1", "deployments"}, "scale", schema.GroupVersionKind{"extensions", "v1beta1", "Scale"}) + mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1", "deployments"}, "scale", schema.GroupVersionKind{"autoscaling", "v1", "Scale"}) + mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1beta1", "deployments"}, "scale", schema.GroupVersionKind{"apps", "v1beta1", "Scale"}) + mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1alpha1", "deployments"}, "scale", schema.GroupVersionKind{"apps", "v1alpha1", "Scale"}) + + mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1", "statefulset"}, "", schema.GroupVersionKind{"apps", "v1", "StatefulSet"}) + mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1beta1", "statefulset"}, "", schema.GroupVersionKind{"apps", "v1beta1", "StatefulSet"}) + mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1beta2", "statefulset"}, "", schema.GroupVersionKind{"apps", "v1beta2", "StatefulSet"}) + + mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1", "statefulset"}, "scale", schema.GroupVersionKind{"apps", "v1", "Scale"}) + mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1beta1", "statefulset"}, "scale", schema.GroupVersionKind{"apps", "v1beta1", "Scale"}) + mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1alpha2", "statefulset"}, "scale", schema.GroupVersionKind{"apps", "v1beta2", "Scale"}) + + nsSelector := make(map[string]string) + for i := 0; i < 100; i++ { + nsSelector[fmt.Sprintf("key-%d", i)] = fmt.Sprintf("val-%d", i) + } + + wb := &v1.ValidatingWebhook{ + MatchPolicy: &equivalentMatch, + NamespaceSelector: &metav1.LabelSelector{MatchLabels: nsSelector}, + ObjectSelector: &metav1.LabelSelector{}, + Rules: []v1.RuleWithOperations{ + { + Operations: []v1.OperationType{"*"}, + Rule: v1.Rule{APIGroups: []string{"apps"}, APIVersions: []string{"v1beta1"}, Resources: []string{"deployments", "deployments/scale"}, Scope: &allScopes}, + }, + { + Operations: []v1.OperationType{"*"}, + Rule: v1.Rule{APIGroups: []string{"extensions"}, APIVersions: []string{"v1beta1"}, Resources: []string{"deployments", "deployments/scale"}, Scope: &allScopes}, + }, + }, + } + + wbAccessor := webhook.NewValidatingWebhookAccessor("webhook", "webhook-cfg", wb) + attrs := admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{"autoscaling", "v1", "Scale"}, "ns", "name", schema.GroupVersionResource{"apps", "v1", "deployments"}, "scale", admission.Create, &metav1.CreateOptions{}, false, nil) + interfaces := &admission.RuntimeObjectInterfaces{EquivalentResourceMapper: mapper} + a := &Webhook{namespaceMatcher: &namespace.Matcher{NamespaceLister: namespaceLister}, objectMatcher: &object.Matcher{}} + + for i := 0; i < b.N; i++ { + a.ShouldCallHook(wbAccessor, attrs, interfaces) + } +} + +func BenchmarkShouldCallHookWithComplexRule(b *testing.B) { + allScopes := v1.AllScopes + equivalentMatch := v1.Equivalent + + namespace1Labels := map[string]string{"ns": "ns1"} + namespace1 := corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ns1", + Labels: namespace1Labels, + }, + } + namespaceLister := fakeNamespaceLister{map[string]*corev1.Namespace{"ns": &namespace1}} + + mapper := runtime.NewEquivalentResourceRegistryWithIdentity(func(resource schema.GroupResource) string { + if resource.Resource == "deployments" { + // co-locate deployments in all API groups + return "/deployments" + } + return "" + }) + mapper.RegisterKindFor(schema.GroupVersionResource{"extensions", "v1beta1", "deployments"}, "", schema.GroupVersionKind{"extensions", "v1beta1", "Deployment"}) + mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1", "deployments"}, "", schema.GroupVersionKind{"apps", "v1", "Deployment"}) + mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1beta1", "deployments"}, "", schema.GroupVersionKind{"apps", "v1beta1", "Deployment"}) + mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1alpha1", "deployments"}, "", schema.GroupVersionKind{"apps", "v1alpha1", "Deployment"}) + + mapper.RegisterKindFor(schema.GroupVersionResource{"extensions", "v1beta1", "deployments"}, "scale", schema.GroupVersionKind{"extensions", "v1beta1", "Scale"}) + mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1", "deployments"}, "scale", schema.GroupVersionKind{"autoscaling", "v1", "Scale"}) + mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1beta1", "deployments"}, "scale", schema.GroupVersionKind{"apps", "v1beta1", "Scale"}) + mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1alpha1", "deployments"}, "scale", schema.GroupVersionKind{"apps", "v1alpha1", "Scale"}) + + mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1", "statefulset"}, "", schema.GroupVersionKind{"apps", "v1", "StatefulSet"}) + mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1beta1", "statefulset"}, "", schema.GroupVersionKind{"apps", "v1beta1", "StatefulSet"}) + mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1beta2", "statefulset"}, "", schema.GroupVersionKind{"apps", "v1beta2", "StatefulSet"}) + + mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1", "statefulset"}, "scale", schema.GroupVersionKind{"apps", "v1", "Scale"}) + mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1beta1", "statefulset"}, "scale", schema.GroupVersionKind{"apps", "v1beta1", "Scale"}) + mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1alpha2", "statefulset"}, "scale", schema.GroupVersionKind{"apps", "v1beta2", "Scale"}) + + wb := &v1.ValidatingWebhook{ + MatchPolicy: &equivalentMatch, + NamespaceSelector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "b"}}, + ObjectSelector: &metav1.LabelSelector{}, + Rules: []v1.RuleWithOperations{}, + } + + for i := 0; i < 100; i++ { + rule := v1.RuleWithOperations{ + Operations: []v1.OperationType{"*"}, + Rule: v1.Rule{ + APIGroups: []string{fmt.Sprintf("app-%d", i)}, + APIVersions: []string{fmt.Sprintf("v%d", i)}, + Resources: []string{fmt.Sprintf("resource%d", i), fmt.Sprintf("resource%d/scale", i)}, + Scope: &allScopes, + }, + } + wb.Rules = append(wb.Rules, rule) + } + + wbAccessor := webhook.NewValidatingWebhookAccessor("webhook", "webhook-cfg", wb) + attrs := admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{"autoscaling", "v1", "Scale"}, "ns", "name", schema.GroupVersionResource{"apps", "v1", "deployments"}, "scale", admission.Create, &metav1.CreateOptions{}, false, nil) + interfaces := &admission.RuntimeObjectInterfaces{EquivalentResourceMapper: mapper} + a := &Webhook{namespaceMatcher: &namespace.Matcher{NamespaceLister: namespaceLister}, objectMatcher: &object.Matcher{}} + + for i := 0; i < b.N; i++ { + a.ShouldCallHook(wbAccessor, attrs, interfaces) + } +} + +func BenchmarkShouldCallHookWithComplexSelectorAndRule(b *testing.B) { + allScopes := v1.AllScopes + equivalentMatch := v1.Equivalent + + namespace1Labels := map[string]string{"ns": "ns1"} + namespace1 := corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ns1", + Labels: namespace1Labels, + }, + } + namespaceLister := fakeNamespaceLister{map[string]*corev1.Namespace{"ns": &namespace1}} + + mapper := runtime.NewEquivalentResourceRegistryWithIdentity(func(resource schema.GroupResource) string { + if resource.Resource == "deployments" { + // co-locate deployments in all API groups + return "/deployments" + } + return "" + }) + mapper.RegisterKindFor(schema.GroupVersionResource{"extensions", "v1beta1", "deployments"}, "", schema.GroupVersionKind{"extensions", "v1beta1", "Deployment"}) + mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1", "deployments"}, "", schema.GroupVersionKind{"apps", "v1", "Deployment"}) + mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1beta1", "deployments"}, "", schema.GroupVersionKind{"apps", "v1beta1", "Deployment"}) + mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1alpha1", "deployments"}, "", schema.GroupVersionKind{"apps", "v1alpha1", "Deployment"}) + + mapper.RegisterKindFor(schema.GroupVersionResource{"extensions", "v1beta1", "deployments"}, "scale", schema.GroupVersionKind{"extensions", "v1beta1", "Scale"}) + mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1", "deployments"}, "scale", schema.GroupVersionKind{"autoscaling", "v1", "Scale"}) + mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1beta1", "deployments"}, "scale", schema.GroupVersionKind{"apps", "v1beta1", "Scale"}) + mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1alpha1", "deployments"}, "scale", schema.GroupVersionKind{"apps", "v1alpha1", "Scale"}) + + mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1", "statefulset"}, "", schema.GroupVersionKind{"apps", "v1", "StatefulSet"}) + mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1beta1", "statefulset"}, "", schema.GroupVersionKind{"apps", "v1beta1", "StatefulSet"}) + mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1beta2", "statefulset"}, "", schema.GroupVersionKind{"apps", "v1beta2", "StatefulSet"}) + + mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1", "statefulset"}, "scale", schema.GroupVersionKind{"apps", "v1", "Scale"}) + mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1beta1", "statefulset"}, "scale", schema.GroupVersionKind{"apps", "v1beta1", "Scale"}) + mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1alpha2", "statefulset"}, "scale", schema.GroupVersionKind{"apps", "v1beta2", "Scale"}) + + nsSelector := make(map[string]string) + for i := 0; i < 100; i++ { + nsSelector[fmt.Sprintf("key-%d", i)] = fmt.Sprintf("val-%d", i) + } + + wb := &v1.ValidatingWebhook{ + MatchPolicy: &equivalentMatch, + NamespaceSelector: &metav1.LabelSelector{MatchLabels: nsSelector}, + ObjectSelector: &metav1.LabelSelector{}, + Rules: []v1.RuleWithOperations{}, + } + + for i := 0; i < 100; i++ { + rule := v1.RuleWithOperations{ + Operations: []v1.OperationType{"*"}, + Rule: v1.Rule{ + APIGroups: []string{fmt.Sprintf("app-%d", i)}, + APIVersions: []string{fmt.Sprintf("v%d", i)}, + Resources: []string{fmt.Sprintf("resource%d", i), fmt.Sprintf("resource%d/scale", i)}, + Scope: &allScopes, + }, + } + wb.Rules = append(wb.Rules, rule) + } + + wbAccessor := webhook.NewValidatingWebhookAccessor("webhook", "webhook-cfg", wb) + attrs := admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{"autoscaling", "v1", "Scale"}, "ns", "name", schema.GroupVersionResource{"apps", "v1", "deployments"}, "scale", admission.Create, &metav1.CreateOptions{}, false, nil) + interfaces := &admission.RuntimeObjectInterfaces{EquivalentResourceMapper: mapper} + a := &Webhook{namespaceMatcher: &namespace.Matcher{NamespaceLister: namespaceLister}, objectMatcher: &object.Matcher{}} + + for i := 0; i < b.N; i++ { + a.ShouldCallHook(wbAccessor, attrs, interfaces) + } +}