Skip to content

Commit 88c177b

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#92696 from gongguan/callhook
skip mismatched webhookAccessor and object
2 parents 5d2907a + 850a913 commit 88c177b

File tree

3 files changed

+248
-10
lines changed

3 files changed

+248
-10
lines changed

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,11 @@ go_test(
5353
embed = [":go_default_library"],
5454
deps = [
5555
"//staging/src/k8s.io/api/admissionregistration/v1:go_default_library",
56+
"//staging/src/k8s.io/api/core/v1:go_default_library",
57+
"//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
5658
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
5759
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured:go_default_library",
60+
"//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library",
5861
"//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
5962
"//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
6063
"//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library",

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

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,18 @@ func (a *Webhook) ValidateInitialization() error {
141141
// ShouldCallHook returns invocation details if the webhook should be called, nil if the webhook should not be called,
142142
// or an error if an error was encountered during evaluation.
143143
func (a *Webhook) ShouldCallHook(h webhook.WebhookAccessor, attr admission.Attributes, o admission.ObjectInterfaces) (*WebhookInvocation, *apierrors.StatusError) {
144-
var err *apierrors.StatusError
144+
matches, matchNsErr := a.namespaceMatcher.MatchNamespaceSelector(h, attr)
145+
// Should not return an error here for webhooks which do not apply to the request, even if err is an unexpected scenario.
146+
if !matches && matchNsErr == nil {
147+
return nil, nil
148+
}
149+
150+
// Should not return an error here for webhooks which do not apply to the request, even if err is an unexpected scenario.
151+
matches, matchObjErr := a.objectMatcher.MatchObjectSelector(h, attr)
152+
if !matches && matchObjErr == nil {
153+
return nil, nil
154+
}
155+
145156
var invocation *WebhookInvocation
146157
for _, r := range h.GetRules() {
147158
m := rules.Matcher{Rule: r, Attr: attr}
@@ -189,15 +200,11 @@ func (a *Webhook) ShouldCallHook(h webhook.WebhookAccessor, attr admission.Attri
189200
if invocation == nil {
190201
return nil, nil
191202
}
192-
193-
matches, err := a.namespaceMatcher.MatchNamespaceSelector(h, attr)
194-
if !matches || err != nil {
195-
return nil, err
203+
if matchNsErr != nil {
204+
return nil, matchNsErr
196205
}
197-
198-
matches, err = a.objectMatcher.MatchObjectSelector(h, attr)
199-
if !matches || err != nil {
200-
return nil, err
206+
if matchObjErr != nil {
207+
return nil, matchObjErr
201208
}
202209

203210
return invocation, nil

‎staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/webhook_test.go‎

Lines changed: 229 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,10 @@ import (
2222
"testing"
2323

2424
"k8s.io/api/admissionregistration/v1"
25+
corev1 "k8s.io/api/core/v1"
26+
"k8s.io/apimachinery/pkg/api/errors"
2527
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
28+
"k8s.io/apimachinery/pkg/labels"
2629
"k8s.io/apimachinery/pkg/runtime"
2730
"k8s.io/apimachinery/pkg/runtime/schema"
2831
"k8s.io/apiserver/pkg/admission"
@@ -75,7 +78,7 @@ func TestShouldCallHook(t *testing.T) {
7578
}{
7679
{
7780
name: "no rules (just write)",
78-
webhook: &v1.ValidatingWebhook{Rules: []v1.RuleWithOperations{}},
81+
webhook: &v1.ValidatingWebhook{NamespaceSelector: &metav1.LabelSelector{}, Rules: []v1.RuleWithOperations{}},
7982
attrs: admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{"apps", "v1", "Deployment"}, "ns", "name", schema.GroupVersionResource{"apps", "v1", "deployments"}, "", admission.Create, &metav1.CreateOptions{}, false, nil),
8083
expectCall: false,
8184
},
@@ -329,3 +332,228 @@ func TestShouldCallHook(t *testing.T) {
329332
})
330333
}
331334
}
335+
336+
type fakeNamespaceLister struct {
337+
namespaces map[string]*corev1.Namespace
338+
}
339+
340+
func (f fakeNamespaceLister) List(selector labels.Selector) (ret []*corev1.Namespace, err error) {
341+
return nil, nil
342+
}
343+
func (f fakeNamespaceLister) Get(name string) (*corev1.Namespace, error) {
344+
ns, ok := f.namespaces[name]
345+
if ok {
346+
return ns, nil
347+
}
348+
return nil, errors.NewNotFound(corev1.Resource("namespaces"), name)
349+
}
350+
351+
func BenchmarkShouldCallHookWithComplexSelector(b *testing.B) {
352+
allScopes := v1.AllScopes
353+
equivalentMatch := v1.Equivalent
354+
355+
namespace1Labels := map[string]string{"ns": "ns1"}
356+
namespace1 := corev1.Namespace{
357+
ObjectMeta: metav1.ObjectMeta{
358+
Name: "ns1",
359+
Labels: namespace1Labels,
360+
},
361+
}
362+
namespaceLister := fakeNamespaceLister{map[string]*corev1.Namespace{"ns": &namespace1}}
363+
364+
mapper := runtime.NewEquivalentResourceRegistryWithIdentity(func(resource schema.GroupResource) string {
365+
if resource.Resource == "deployments" {
366+
// co-locate deployments in all API groups
367+
return "/deployments"
368+
}
369+
return ""
370+
})
371+
mapper.RegisterKindFor(schema.GroupVersionResource{"extensions", "v1beta1", "deployments"}, "", schema.GroupVersionKind{"extensions", "v1beta1", "Deployment"})
372+
mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1", "deployments"}, "", schema.GroupVersionKind{"apps", "v1", "Deployment"})
373+
mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1beta1", "deployments"}, "", schema.GroupVersionKind{"apps", "v1beta1", "Deployment"})
374+
mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1alpha1", "deployments"}, "", schema.GroupVersionKind{"apps", "v1alpha1", "Deployment"})
375+
376+
mapper.RegisterKindFor(schema.GroupVersionResource{"extensions", "v1beta1", "deployments"}, "scale", schema.GroupVersionKind{"extensions", "v1beta1", "Scale"})
377+
mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1", "deployments"}, "scale", schema.GroupVersionKind{"autoscaling", "v1", "Scale"})
378+
mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1beta1", "deployments"}, "scale", schema.GroupVersionKind{"apps", "v1beta1", "Scale"})
379+
mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1alpha1", "deployments"}, "scale", schema.GroupVersionKind{"apps", "v1alpha1", "Scale"})
380+
381+
mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1", "statefulset"}, "", schema.GroupVersionKind{"apps", "v1", "StatefulSet"})
382+
mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1beta1", "statefulset"}, "", schema.GroupVersionKind{"apps", "v1beta1", "StatefulSet"})
383+
mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1beta2", "statefulset"}, "", schema.GroupVersionKind{"apps", "v1beta2", "StatefulSet"})
384+
385+
mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1", "statefulset"}, "scale", schema.GroupVersionKind{"apps", "v1", "Scale"})
386+
mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1beta1", "statefulset"}, "scale", schema.GroupVersionKind{"apps", "v1beta1", "Scale"})
387+
mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1alpha2", "statefulset"}, "scale", schema.GroupVersionKind{"apps", "v1beta2", "Scale"})
388+
389+
nsSelector := make(map[string]string)
390+
for i := 0; i < 100; i++ {
391+
nsSelector[fmt.Sprintf("key-%d", i)] = fmt.Sprintf("val-%d", i)
392+
}
393+
394+
wb := &v1.ValidatingWebhook{
395+
MatchPolicy: &equivalentMatch,
396+
NamespaceSelector: &metav1.LabelSelector{MatchLabels: nsSelector},
397+
ObjectSelector: &metav1.LabelSelector{},
398+
Rules: []v1.RuleWithOperations{
399+
{
400+
Operations: []v1.OperationType{"*"},
401+
Rule: v1.Rule{APIGroups: []string{"apps"}, APIVersions: []string{"v1beta1"}, Resources: []string{"deployments", "deployments/scale"}, Scope: &allScopes},
402+
},
403+
{
404+
Operations: []v1.OperationType{"*"},
405+
Rule: v1.Rule{APIGroups: []string{"extensions"}, APIVersions: []string{"v1beta1"}, Resources: []string{"deployments", "deployments/scale"}, Scope: &allScopes},
406+
},
407+
},
408+
}
409+
410+
wbAccessor := webhook.NewValidatingWebhookAccessor("webhook", "webhook-cfg", wb)
411+
attrs := admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{"autoscaling", "v1", "Scale"}, "ns", "name", schema.GroupVersionResource{"apps", "v1", "deployments"}, "scale", admission.Create, &metav1.CreateOptions{}, false, nil)
412+
interfaces := &admission.RuntimeObjectInterfaces{EquivalentResourceMapper: mapper}
413+
a := &Webhook{namespaceMatcher: &namespace.Matcher{NamespaceLister: namespaceLister}, objectMatcher: &object.Matcher{}}
414+
415+
for i := 0; i < b.N; i++ {
416+
a.ShouldCallHook(wbAccessor, attrs, interfaces)
417+
}
418+
}
419+
420+
func BenchmarkShouldCallHookWithComplexRule(b *testing.B) {
421+
allScopes := v1.AllScopes
422+
equivalentMatch := v1.Equivalent
423+
424+
namespace1Labels := map[string]string{"ns": "ns1"}
425+
namespace1 := corev1.Namespace{
426+
ObjectMeta: metav1.ObjectMeta{
427+
Name: "ns1",
428+
Labels: namespace1Labels,
429+
},
430+
}
431+
namespaceLister := fakeNamespaceLister{map[string]*corev1.Namespace{"ns": &namespace1}}
432+
433+
mapper := runtime.NewEquivalentResourceRegistryWithIdentity(func(resource schema.GroupResource) string {
434+
if resource.Resource == "deployments" {
435+
// co-locate deployments in all API groups
436+
return "/deployments"
437+
}
438+
return ""
439+
})
440+
mapper.RegisterKindFor(schema.GroupVersionResource{"extensions", "v1beta1", "deployments"}, "", schema.GroupVersionKind{"extensions", "v1beta1", "Deployment"})
441+
mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1", "deployments"}, "", schema.GroupVersionKind{"apps", "v1", "Deployment"})
442+
mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1beta1", "deployments"}, "", schema.GroupVersionKind{"apps", "v1beta1", "Deployment"})
443+
mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1alpha1", "deployments"}, "", schema.GroupVersionKind{"apps", "v1alpha1", "Deployment"})
444+
445+
mapper.RegisterKindFor(schema.GroupVersionResource{"extensions", "v1beta1", "deployments"}, "scale", schema.GroupVersionKind{"extensions", "v1beta1", "Scale"})
446+
mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1", "deployments"}, "scale", schema.GroupVersionKind{"autoscaling", "v1", "Scale"})
447+
mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1beta1", "deployments"}, "scale", schema.GroupVersionKind{"apps", "v1beta1", "Scale"})
448+
mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1alpha1", "deployments"}, "scale", schema.GroupVersionKind{"apps", "v1alpha1", "Scale"})
449+
450+
mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1", "statefulset"}, "", schema.GroupVersionKind{"apps", "v1", "StatefulSet"})
451+
mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1beta1", "statefulset"}, "", schema.GroupVersionKind{"apps", "v1beta1", "StatefulSet"})
452+
mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1beta2", "statefulset"}, "", schema.GroupVersionKind{"apps", "v1beta2", "StatefulSet"})
453+
454+
mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1", "statefulset"}, "scale", schema.GroupVersionKind{"apps", "v1", "Scale"})
455+
mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1beta1", "statefulset"}, "scale", schema.GroupVersionKind{"apps", "v1beta1", "Scale"})
456+
mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1alpha2", "statefulset"}, "scale", schema.GroupVersionKind{"apps", "v1beta2", "Scale"})
457+
458+
wb := &v1.ValidatingWebhook{
459+
MatchPolicy: &equivalentMatch,
460+
NamespaceSelector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "b"}},
461+
ObjectSelector: &metav1.LabelSelector{},
462+
Rules: []v1.RuleWithOperations{},
463+
}
464+
465+
for i := 0; i < 100; i++ {
466+
rule := v1.RuleWithOperations{
467+
Operations: []v1.OperationType{"*"},
468+
Rule: v1.Rule{
469+
APIGroups: []string{fmt.Sprintf("app-%d", i)},
470+
APIVersions: []string{fmt.Sprintf("v%d", i)},
471+
Resources: []string{fmt.Sprintf("resource%d", i), fmt.Sprintf("resource%d/scale", i)},
472+
Scope: &allScopes,
473+
},
474+
}
475+
wb.Rules = append(wb.Rules, rule)
476+
}
477+
478+
wbAccessor := webhook.NewValidatingWebhookAccessor("webhook", "webhook-cfg", wb)
479+
attrs := admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{"autoscaling", "v1", "Scale"}, "ns", "name", schema.GroupVersionResource{"apps", "v1", "deployments"}, "scale", admission.Create, &metav1.CreateOptions{}, false, nil)
480+
interfaces := &admission.RuntimeObjectInterfaces{EquivalentResourceMapper: mapper}
481+
a := &Webhook{namespaceMatcher: &namespace.Matcher{NamespaceLister: namespaceLister}, objectMatcher: &object.Matcher{}}
482+
483+
for i := 0; i < b.N; i++ {
484+
a.ShouldCallHook(wbAccessor, attrs, interfaces)
485+
}
486+
}
487+
488+
func BenchmarkShouldCallHookWithComplexSelectorAndRule(b *testing.B) {
489+
allScopes := v1.AllScopes
490+
equivalentMatch := v1.Equivalent
491+
492+
namespace1Labels := map[string]string{"ns": "ns1"}
493+
namespace1 := corev1.Namespace{
494+
ObjectMeta: metav1.ObjectMeta{
495+
Name: "ns1",
496+
Labels: namespace1Labels,
497+
},
498+
}
499+
namespaceLister := fakeNamespaceLister{map[string]*corev1.Namespace{"ns": &namespace1}}
500+
501+
mapper := runtime.NewEquivalentResourceRegistryWithIdentity(func(resource schema.GroupResource) string {
502+
if resource.Resource == "deployments" {
503+
// co-locate deployments in all API groups
504+
return "/deployments"
505+
}
506+
return ""
507+
})
508+
mapper.RegisterKindFor(schema.GroupVersionResource{"extensions", "v1beta1", "deployments"}, "", schema.GroupVersionKind{"extensions", "v1beta1", "Deployment"})
509+
mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1", "deployments"}, "", schema.GroupVersionKind{"apps", "v1", "Deployment"})
510+
mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1beta1", "deployments"}, "", schema.GroupVersionKind{"apps", "v1beta1", "Deployment"})
511+
mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1alpha1", "deployments"}, "", schema.GroupVersionKind{"apps", "v1alpha1", "Deployment"})
512+
513+
mapper.RegisterKindFor(schema.GroupVersionResource{"extensions", "v1beta1", "deployments"}, "scale", schema.GroupVersionKind{"extensions", "v1beta1", "Scale"})
514+
mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1", "deployments"}, "scale", schema.GroupVersionKind{"autoscaling", "v1", "Scale"})
515+
mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1beta1", "deployments"}, "scale", schema.GroupVersionKind{"apps", "v1beta1", "Scale"})
516+
mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1alpha1", "deployments"}, "scale", schema.GroupVersionKind{"apps", "v1alpha1", "Scale"})
517+
518+
mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1", "statefulset"}, "", schema.GroupVersionKind{"apps", "v1", "StatefulSet"})
519+
mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1beta1", "statefulset"}, "", schema.GroupVersionKind{"apps", "v1beta1", "StatefulSet"})
520+
mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1beta2", "statefulset"}, "", schema.GroupVersionKind{"apps", "v1beta2", "StatefulSet"})
521+
522+
mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1", "statefulset"}, "scale", schema.GroupVersionKind{"apps", "v1", "Scale"})
523+
mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1beta1", "statefulset"}, "scale", schema.GroupVersionKind{"apps", "v1beta1", "Scale"})
524+
mapper.RegisterKindFor(schema.GroupVersionResource{"apps", "v1alpha2", "statefulset"}, "scale", schema.GroupVersionKind{"apps", "v1beta2", "Scale"})
525+
526+
nsSelector := make(map[string]string)
527+
for i := 0; i < 100; i++ {
528+
nsSelector[fmt.Sprintf("key-%d", i)] = fmt.Sprintf("val-%d", i)
529+
}
530+
531+
wb := &v1.ValidatingWebhook{
532+
MatchPolicy: &equivalentMatch,
533+
NamespaceSelector: &metav1.LabelSelector{MatchLabels: nsSelector},
534+
ObjectSelector: &metav1.LabelSelector{},
535+
Rules: []v1.RuleWithOperations{},
536+
}
537+
538+
for i := 0; i < 100; i++ {
539+
rule := v1.RuleWithOperations{
540+
Operations: []v1.OperationType{"*"},
541+
Rule: v1.Rule{
542+
APIGroups: []string{fmt.Sprintf("app-%d", i)},
543+
APIVersions: []string{fmt.Sprintf("v%d", i)},
544+
Resources: []string{fmt.Sprintf("resource%d", i), fmt.Sprintf("resource%d/scale", i)},
545+
Scope: &allScopes,
546+
},
547+
}
548+
wb.Rules = append(wb.Rules, rule)
549+
}
550+
551+
wbAccessor := webhook.NewValidatingWebhookAccessor("webhook", "webhook-cfg", wb)
552+
attrs := admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{"autoscaling", "v1", "Scale"}, "ns", "name", schema.GroupVersionResource{"apps", "v1", "deployments"}, "scale", admission.Create, &metav1.CreateOptions{}, false, nil)
553+
interfaces := &admission.RuntimeObjectInterfaces{EquivalentResourceMapper: mapper}
554+
a := &Webhook{namespaceMatcher: &namespace.Matcher{NamespaceLister: namespaceLister}, objectMatcher: &object.Matcher{}}
555+
556+
for i := 0; i < b.N; i++ {
557+
a.ShouldCallHook(wbAccessor, attrs, interfaces)
558+
}
559+
}

0 commit comments

Comments
 (0)