From 512274139c85e414aee5b872acd50c38919bd64d Mon Sep 17 00:00:00 2001 From: Chao Xu Date: Fri, 27 Oct 2017 14:42:09 -0700 Subject: [PATCH] add NamespaceSelector to the api business logic in webhook plugin and unit test add a e2e test for namespace selector Kubernetes-commit: 7006d224bebb5a1aee9c70387a8584e0a0e8b10f --- pkg/admission/plugin/webhook/admission.go | 84 ++++++++ .../plugin/webhook/admission_test.go | 194 ++++++++++++++++-- 2 files changed, 264 insertions(+), 14 deletions(-) diff --git a/pkg/admission/plugin/webhook/admission.go b/pkg/admission/plugin/webhook/admission.go index af4c3a9c2..96f5b1ea3 100644 --- a/pkg/admission/plugin/webhook/admission.go +++ b/pkg/admission/plugin/webhook/admission.go @@ -32,7 +32,9 @@ import ( admissionv1alpha1 "k8s.io/api/admission/v1alpha1" "k8s.io/api/admissionregistration/v1alpha1" 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/apimachinery/pkg/runtime/serializer" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -41,7 +43,9 @@ import ( "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/admission/configuration" genericadmissioninit "k8s.io/apiserver/pkg/admission/initializer" + "k8s.io/client-go/informers" clientset "k8s.io/client-go/kubernetes" + corelisters "k8s.io/client-go/listers/core/v1" "k8s.io/client-go/rest" ) @@ -123,6 +127,8 @@ type GenericAdmissionWebhook struct { hookSource WebhookSource serviceResolver ServiceResolver negotiatedSerializer runtime.NegotiatedSerializer + namespaceLister corelisters.NamespaceLister + client clientset.Interface authInfoResolver AuthenticationInfoResolver cache *lru.Cache @@ -163,9 +169,17 @@ func (a *GenericAdmissionWebhook) SetScheme(scheme *runtime.Scheme) { // WantsExternalKubeClientSet defines a function which sets external ClientSet for admission plugins that need it func (a *GenericAdmissionWebhook) SetExternalKubeClientSet(client clientset.Interface) { + a.client = client a.hookSource = configuration.NewValidatingWebhookConfigurationManager(client.AdmissionregistrationV1alpha1().ValidatingWebhookConfigurations()) } +// SetExternalKubeInformerFactory implements the WantsExternalKubeInformerFactory interface. +func (a *GenericAdmissionWebhook) SetExternalKubeInformerFactory(f informers.SharedInformerFactory) { + namespaceInformer := f.Core().V1().Namespaces() + a.namespaceLister = namespaceInformer.Lister() + a.SetReadyFunc(namespaceInformer.Informer().HasSynced) +} + // ValidateInitialization implements the InitializationValidator interface. func (a *GenericAdmissionWebhook) ValidateInitialization() error { if a.hookSource == nil { @@ -174,6 +188,9 @@ func (a *GenericAdmissionWebhook) ValidateInitialization() error { if a.negotiatedSerializer == nil { return fmt.Errorf("the GenericAdmissionWebhook admission plugin requires a runtime.Scheme to be provided to derive a serializer") } + if a.namespaceLister == nil { + return fmt.Errorf("the GenericAdmissionWebhook admission plugin requires a namespaceLister") + } go a.hookSource.Run(wait.NeverStop) return nil } @@ -255,7 +272,74 @@ func (a *GenericAdmissionWebhook) Admit(attr admission.Attributes) error { return errs[0] } +func (a *GenericAdmissionWebhook) getNamespaceLabels(attr admission.Attributes) (map[string]string, error) { + // If the request itself is creating or updating a namespace, then get the + // labels from attr.Object, because namespaceLister doesn't have the latest + // namespace yet. + // + // However, if the request is deleting a namespace, then get the label from + // the namespace in the namespaceLister, because a delete request is not + // going to change the object, and attr.Object will be a DeleteOptions + // rather than a namespace object. + if attr.GetResource().Resource == "namespaces" && + len(attr.GetSubresource()) == 0 && + (attr.GetOperation() == admission.Create || attr.GetOperation() == admission.Update) { + accessor, err := meta.Accessor(attr.GetObject()) + if err != nil { + return nil, err + } + return accessor.GetLabels(), nil + } + + namespaceName := attr.GetNamespace() + namespace, err := a.namespaceLister.Get(namespaceName) + if err != nil && !apierrors.IsNotFound(err) { + return nil, err + } + if apierrors.IsNotFound(err) { + // in case of latency in our caches, make a call direct to storage to verify that it truly exists or not + namespace, err = a.client.Core().Namespaces().Get(namespaceName, metav1.GetOptions{}) + if err != nil { + return nil, err + } + } + return namespace.Labels, nil +} + +// whether the request is exempted by the webhook because of the +// namespaceSelector of the webhook. +func (a *GenericAdmissionWebhook) exemptedByNamespaceSelector(h *v1alpha1.Webhook, attr admission.Attributes) (bool, error) { + namespaceName := attr.GetNamespace() + if len(namespaceName) == 0 && attr.GetResource().Resource != "namespaces" { + // If the request is about a cluster scoped resource, and it is not a + // namespace, it is exempted from all webhooks for now. + // TODO: figure out a way selective exempt cluster scoped resources. + // Also update the comment in types.go + return true, nil + } + namespaceLabels, err := a.getNamespaceLabels(attr) + if apierrors.IsNotFound(err) { + return false, err + } + if err != nil { + return false, apierrors.NewInternalError(err) + } + // TODO: adding an LRU cache to cache the translation + selector, err := metav1.LabelSelectorAsSelector(h.NamespaceSelector) + if err != nil { + return false, apierrors.NewInternalError(err) + } + return !selector.Matches(labels.Set(namespaceLabels)), nil +} + func (a *GenericAdmissionWebhook) callHook(ctx context.Context, h *v1alpha1.Webhook, attr admission.Attributes) error { + excluded, err := a.exemptedByNamespaceSelector(h, attr) + if err != nil { + return err + } + if excluded { + return nil + } matches := false for _, r := range h.Rules { m := RuleMatcher{Rule: r, Attr: attr} diff --git a/pkg/admission/plugin/webhook/admission_test.go b/pkg/admission/plugin/webhook/admission_test.go index 39ad0c34f..25308fa23 100644 --- a/pkg/admission/plugin/webhook/admission_test.go +++ b/pkg/admission/plugin/webhook/admission_test.go @@ -24,16 +24,20 @@ import ( "net/http" "net/http/httptest" "net/url" + "reflect" "strings" "sync/atomic" "testing" "k8s.io/api/admission/v1alpha1" registrationv1alpha1 "k8s.io/api/admissionregistration/v1alpha1" - api "k8s.io/api/core/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" apierrors "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" "k8s.io/apiserver/pkg/authentication/user" "k8s.io/client-go/rest" @@ -48,6 +52,11 @@ func (f *fakeHookSource) Webhooks() (*registrationv1alpha1.ValidatingWebhookConf if f.err != nil { return nil, f.err } + for i, h := range f.hooks { + if h.NamespaceSelector == nil { + f.hooks[i].NamespaceSelector = &metav1.LabelSelector{} + } + } return ®istrationv1alpha1.ValidatingWebhookConfiguration{Webhooks: f.hooks}, nil } @@ -65,11 +74,26 @@ func (f fakeServiceResolver) ResolveEndpoint(namespace, name string) (*url.URL, return &u, nil } +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) +} + // TestAdmit tests that GenericAdmissionWebhook#Admit works as expected func TestAdmit(t *testing.T) { scheme := runtime.NewScheme() v1alpha1.AddToScheme(scheme) - api.AddToScheme(scheme) + corev1.AddToScheme(scheme) testServer := newTestServer(t) testServer.StartTLS() @@ -85,12 +109,22 @@ func TestAdmit(t *testing.T) { wh.authInfoResolver = newFakeAuthenticationInfoResolver() wh.serviceResolver = fakeServiceResolver{base: *serverURL} wh.SetScheme(scheme) + namespace := "webhook-test" + wh.namespaceLister = fakeNamespaceLister{map[string]*corev1.Namespace{ + namespace: { + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "runlevel": "0", + }, + }, + }, + }, + } // Set up a test object for the call - kind := api.SchemeGroupVersion.WithKind("Pod") + kind := corev1.SchemeGroupVersion.WithKind("Pod") name := "my-pod" - namespace := "webhook-test" - object := api.Pod{ + object := corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ "pod.name": name, @@ -103,11 +137,11 @@ func TestAdmit(t *testing.T) { Kind: "Pod", }, } - oldObject := api.Pod{ + oldObject := corev1.Pod{ ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace}, } operation := admission.Update - resource := api.Resource("pods").WithVersion("v1") + resource := corev1.Resource("pods").WithVersion("v1") subResource := "" userInfo := user.DefaultInfo{ Name: "webhook-test", @@ -167,6 +201,40 @@ func TestAdmit(t *testing.T) { }, errorContains: "you shall not pass", }, + "match & disallow & but allowed because namespaceSelector exempt the namespace": { + hookSource: fakeHookSource{ + hooks: []registrationv1alpha1.Webhook{{ + Name: "disallow", + ClientConfig: newFakeHookClientConfig("disallow"), + Rules: newMatchEverythingRules(), + NamespaceSelector: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{{ + Key: "runlevel", + Values: []string{"1"}, + Operator: metav1.LabelSelectorOpIn, + }}, + }, + }}, + }, + expectAllow: true, + }, + "match & disallow & but allowed because namespaceSelector exempt the namespace ii": { + hookSource: fakeHookSource{ + hooks: []registrationv1alpha1.Webhook{{ + Name: "disallow", + ClientConfig: newFakeHookClientConfig("disallow"), + Rules: newMatchEverythingRules(), + NamespaceSelector: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{{ + Key: "runlevel", + Values: []string{"0"}, + Operator: metav1.LabelSelectorOpNotIn, + }}, + }, + }}, + }, + expectAllow: true, + }, "match & fail (but allow because fail open)": { hookSource: fakeHookSource{ hooks: []registrationv1alpha1.Webhook{{ @@ -230,9 +298,11 @@ func TestAdmit(t *testing.T) { } for name, tt := range table { + if !strings.Contains(name, "no match") { + continue + } t.Run(name, func(t *testing.T) { wh.hookSource = &tt.hookSource - err = wh.Admit(admission.NewAttributesRecord(&object, &oldObject, kind, namespace, name, resource, subResource, operation, &userInfo)) if tt.expectAllow != (err == nil) { t.Errorf("expected allowed=%v, but got err=%v", tt.expectAllow, err) @@ -254,7 +324,7 @@ func TestAdmit(t *testing.T) { func TestAdmitCachedClient(t *testing.T) { scheme := runtime.NewScheme() v1alpha1.AddToScheme(scheme) - api.AddToScheme(scheme) + corev1.AddToScheme(scheme) testServer := newTestServer(t) testServer.StartTLS() @@ -270,12 +340,22 @@ func TestAdmitCachedClient(t *testing.T) { wh.authInfoResolver = newFakeAuthenticationInfoResolver() wh.serviceResolver = fakeServiceResolver{base: *serverURL} wh.SetScheme(scheme) + namespace := "webhook-test" + wh.namespaceLister = fakeNamespaceLister{map[string]*corev1.Namespace{ + namespace: { + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "runlevel": "0", + }, + }, + }, + }, + } // Set up a test object for the call - kind := api.SchemeGroupVersion.WithKind("Pod") + kind := corev1.SchemeGroupVersion.WithKind("Pod") name := "my-pod" - namespace := "webhook-test" - object := api.Pod{ + object := corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ "pod.name": name, @@ -288,11 +368,11 @@ func TestAdmitCachedClient(t *testing.T) { Kind: "Pod", }, } - oldObject := api.Pod{ + oldObject := corev1.Pod{ ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace}, } operation := admission.Update - resource := api.Resource("pods").WithVersion("v1") + resource := corev1.Resource("pods").WithVersion("v1") subResource := "" userInfo := user.DefaultInfo{ Name: "webhook-test", @@ -522,3 +602,89 @@ func newMatchEverythingRules() []registrationv1alpha1.RuleWithOperations { }, }} } + +func TestGetNamespaceLabels(t *testing.T) { + namespace1Labels := map[string]string{ + "runlevel": "1", + } + namespace1 := corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "1", + Labels: namespace1Labels, + }, + } + namespace2Labels := map[string]string{ + "runlevel": "2", + } + namespace2 := corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "2", + Labels: namespace2Labels, + }, + } + namespaceLister := fakeNamespaceLister{map[string]*corev1.Namespace{ + "1": &namespace1, + }, + } + + tests := []struct { + name string + attr admission.Attributes + expectedLabels map[string]string + }{ + { + name: "request is for creating namespace, the labels should be from the object itself", + attr: admission.NewAttributesRecord(&namespace2, nil, schema.GroupVersionKind{}, "", namespace2.Name, schema.GroupVersionResource{Resource: "namespaces"}, "", admission.Create, nil), + expectedLabels: namespace2Labels, + }, + { + name: "request is for updating namespace, the labels should be from the new object", + attr: admission.NewAttributesRecord(&namespace2, nil, schema.GroupVersionKind{}, namespace2.Name, namespace2.Name, schema.GroupVersionResource{Resource: "namespaces"}, "", admission.Update, nil), + expectedLabels: namespace2Labels, + }, + { + name: "request is for deleting namespace, the labels should be from the cache", + attr: admission.NewAttributesRecord(&namespace2, nil, schema.GroupVersionKind{}, namespace1.Name, namespace1.Name, schema.GroupVersionResource{Resource: "namespaces"}, "", admission.Delete, nil), + expectedLabels: namespace1Labels, + }, + { + name: "request is for namespace/finalizer", + attr: admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{}, namespace1.Name, "mock-name", schema.GroupVersionResource{Resource: "namespaces"}, "finalizers", admission.Create, nil), + expectedLabels: namespace1Labels, + }, + { + name: "request is for pod", + attr: admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{}, namespace1.Name, "mock-name", schema.GroupVersionResource{Resource: "pods"}, "", admission.Create, nil), + expectedLabels: namespace1Labels, + }, + } + wh, err := NewGenericAdmissionWebhook(nil) + if err != nil { + t.Fatal(err) + } + wh.namespaceLister = namespaceLister + for _, tt := range tests { + actualLabels, err := wh.getNamespaceLabels(tt.attr) + if err != nil { + t.Error(err) + } + if !reflect.DeepEqual(actualLabels, tt.expectedLabels) { + t.Errorf("expected labels to be %#v, got %#v", tt.expectedLabels, actualLabels) + } + } +} + +func TestExemptClusterScopedResource(t *testing.T) { + hook := ®istrationv1alpha1.Webhook{ + NamespaceSelector: &metav1.LabelSelector{}, + } + attr := admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{}, "", "mock-name", schema.GroupVersionResource{Version: "v1", Resource: "nodes"}, "", admission.Create, nil) + g := GenericAdmissionWebhook{} + exempted, err := g.exemptedByNamespaceSelector(hook, attr) + if err != nil { + t.Fatal(err) + } + if !exempted { + t.Errorf("cluster scoped resources (but not a namespace) should be exempted from all webhooks") + } +}