From 3bbdb16149747c9ebf6b4979ba6d687db498902c Mon Sep 17 00:00:00 2001 From: Solly Ross Date: Tue, 11 Aug 2015 16:41:00 -0400 Subject: [PATCH] Merge NamespaceExists into NamespaceLifecycle This commit merges the NamespaceExists admission controller into the NamespaceLifecycle admission controller. New tests were added to the NamespaceLifecycle addmission controller tests, and the test case was fixed so that it runs without panicing. Additionally, the NamespaceExists admission controller was marked as deprecated in the docs. Closes #12053 --- docs/admin/admission-controllers.md | 16 +++---- .../namespace/lifecycle/admission.go | 36 +++++++++----- .../namespace/lifecycle/admission_test.go | 47 +++++++++++++++++-- 3 files changed, 75 insertions(+), 24 deletions(-) diff --git a/docs/admin/admission-controllers.md b/docs/admin/admission-controllers.md index a0dda55a8da..9a4bfe41e57 100644 --- a/docs/admin/admission-controllers.md +++ b/docs/admin/admission-controllers.md @@ -48,7 +48,7 @@ Documentation for other releases can be found at - [SecurityContextDeny](#securitycontextdeny) - [ResourceQuota](#resourcequota) - [LimitRanger](#limitranger) - - [NamespaceExists](#namespaceexists) + - [NamespaceExists (deprecated)](#namespaceexists-deprecated) - [NamespaceAutoProvision (deprecated)](#namespaceautoprovision-deprecated) - [NamespaceLifecycle](#namespacelifecycle) - [Is there a recommended set of plug-ins to use?](#is-there-a-recommended-set-of-plug-ins-to-use) @@ -129,29 +129,29 @@ applies a 0.1 CPU requirement to all Pods in the `default` namespace. See the [limitRange design doc](../design/admission_control_limit_range.md) and the [example of Limit Range](limitrange/) for more details. -### NamespaceExists +### NamespaceExists (deprecated) This plug-in will observe all incoming requests that attempt to create a resource in a Kubernetes `Namespace` and reject the request if the `Namespace` was not previously created. We strongly recommend running this plug-in to ensure integrity of your data. +The functionality of this admission controller has been merged into `NamespaceLifecycle` + ### NamespaceAutoProvision (deprecated) This plug-in will observe all incoming requests that attempt to create a resource in a Kubernetes `Namespace` and create a new `Namespace` if one did not already exist previously. -We strongly recommend `NamespaceExists` over `NamespaceAutoProvision`. +We strongly recommend `NamespaceLifecycle` over `NamespaceAutoProvision`. ### NamespaceLifecycle -This plug-in enforces that a `Namespace` that is undergoing termination cannot have new objects created in it. +This plug-in enforces that a `Namespace` that is undergoing termination cannot have new objects created in it, +and ensures that requests in a non-existant `Namespace` are rejected. A `Namespace` deletion kicks off a sequence of operations that remove all objects (pods, services, etc.) in that namespace. In order to enforce integrity of that process, we strongly recommend running this plug-in. -Once `NamespaceAutoProvision` is deprecated, we anticipate `NamespaceLifecycle` and `NamespaceExists` will -be merged into a single plug-in that enforces the life-cycle of a `Namespace` in Kubernetes. - ## Is there a recommended set of plug-ins to use? Yes. @@ -159,7 +159,7 @@ Yes. For Kubernetes 1.0, we strongly recommend running the following set of admission control plug-ins (order matters): ``` ---admission-control=NamespaceLifecycle,NamespaceExists,LimitRanger,SecurityContextDeny,ServiceAccount,ResourceQuota +--admission-control=NamespaceLifecycle,LimitRanger,SecurityContextDeny,ServiceAccount,ResourceQuota ``` diff --git a/plugin/pkg/admission/namespace/lifecycle/admission.go b/plugin/pkg/admission/namespace/lifecycle/admission.go index 9ff9d09cb35..df9eae8eedd 100644 --- a/plugin/pkg/admission/namespace/lifecycle/admission.go +++ b/plugin/pkg/admission/namespace/lifecycle/admission.go @@ -19,6 +19,7 @@ package lifecycle import ( "fmt" "io" + "time" "k8s.io/kubernetes/pkg/admission" "k8s.io/kubernetes/pkg/api" @@ -51,11 +52,8 @@ type lifecycle struct { func (l *lifecycle) Admit(a admission.Attributes) (err error) { // prevent deletion of immortal namespaces - if a.GetOperation() == admission.Delete { - if a.GetKind() == "Namespace" && l.immortalNamespaces.Has(a.GetName()) { - return errors.NewForbidden(a.GetKind(), a.GetName(), fmt.Errorf("namespace can never be deleted")) - } - return nil + if a.GetOperation() == admission.Delete && a.GetKind() == "Namespace" && l.immortalNamespaces.Has(a.GetName()) { + return errors.NewForbidden(a.GetKind(), a.GetName(), fmt.Errorf("namespace can never be deleted")) } defaultVersion, kind, err := api.RESTMapper.VersionAndKindForResource(a.GetResource()) @@ -78,15 +76,27 @@ func (l *lifecycle) Admit(a admission.Attributes) (err error) { if err != nil { return admission.NewForbidden(a, err) } + + // refuse to operate on non-existent namespaces if !exists { - return nil - } - namespace := namespaceObj.(*api.Namespace) - if namespace.Status.Phase != api.NamespaceTerminating { - return nil + // in case of latency in our caches, make a call direct to storage to verify that it truly exists or not + namespaceObj, err = l.client.Namespaces().Get(a.GetNamespace()) + if err != nil { + return admission.NewForbidden(a, fmt.Errorf("Namespace %s does not exist", a.GetNamespace())) + } } - return admission.NewForbidden(a, fmt.Errorf("Unable to create new content in namespace %s because it is being terminated.", a.GetNamespace())) + // ensure that we're not trying to create objects in terminating namespaces + if a.GetOperation() == admission.Create { + namespace := namespaceObj.(*api.Namespace) + if namespace.Status.Phase != api.NamespaceTerminating { + return nil + } + + return admission.NewForbidden(a, fmt.Errorf("Unable to create new content in namespace %s because it is being terminated.", a.GetNamespace())) + } + + return nil } // NewLifecycle creates a new namespace lifecycle admission control handler @@ -103,11 +113,11 @@ func NewLifecycle(c client.Interface) admission.Interface { }, &api.Namespace{}, store, - 0, + 5*time.Minute, ) reflector.Run() return &lifecycle{ - Handler: admission.NewHandler(admission.Create, admission.Delete), + Handler: admission.NewHandler(admission.Create, admission.Update, admission.Delete), client: c, store: store, immortalNamespaces: util.NewStringSet(api.NamespaceDefault), diff --git a/plugin/pkg/admission/namespace/lifecycle/admission_test.go b/plugin/pkg/admission/namespace/lifecycle/admission_test.go index 2465d9d1384..470580c8cc2 100644 --- a/plugin/pkg/admission/namespace/lifecycle/admission_test.go +++ b/plugin/pkg/admission/namespace/lifecycle/admission_test.go @@ -17,12 +17,15 @@ limitations under the License. package lifecycle import ( + "fmt" "testing" "k8s.io/kubernetes/pkg/admission" "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/client/unversioned/cache" "k8s.io/kubernetes/pkg/client/unversioned/testclient" + "k8s.io/kubernetes/pkg/runtime" + "k8s.io/kubernetes/pkg/watch" ) // TestAdmission @@ -36,14 +39,37 @@ func TestAdmission(t *testing.T) { Phase: api.NamespaceActive, }, } - store := cache.NewStore(cache.IndexFuncToKeyFuncAdapter(cache.MetaNamespaceIndexFunc)) + + reactFunc := func(action testclient.Action) (runtime.Object, error) { + switch { + case action.Matches("get", "namespaces"): + if getAction, ok := action.(testclient.GetAction); ok && getAction.GetName() == namespaceObj.Name { + return namespaceObj, nil + } + case action.Matches("list", "namespaces"): + return &api.NamespaceList{Items: []api.Namespace{*namespaceObj}}, nil + + } + + return nil, fmt.Errorf("No result for action %v", action) + } + + store := cache.NewStore(cache.MetaNamespaceKeyFunc) store.Add(namespaceObj) - mockClient := &testclient.Fake{} + fakeWatch := watch.NewFake() + mockClient := &testclient.Fake{Watch: fakeWatch, ReactFn: reactFunc} lfhandler := NewLifecycle(mockClient).(*lifecycle) lfhandler.store = store handler := admission.NewChainHandler(lfhandler) pod := api.Pod{ - ObjectMeta: api.ObjectMeta{Name: "123", Namespace: namespaceObj.Namespace}, + ObjectMeta: api.ObjectMeta{Name: "123", Namespace: namespaceObj.Name}, + Spec: api.PodSpec{ + Volumes: []api.Volume{{Name: "vol"}}, + Containers: []api.Container{{Name: "ctr", Image: "image"}}, + }, + } + badPod := api.Pod{ + ObjectMeta: api.ObjectMeta{Name: "456", Namespace: "doesnotexist"}, Spec: api.PodSpec{ Volumes: []api.Volume{{Name: "vol"}}, Containers: []api.Container{{Name: "ctr", Image: "image"}}, @@ -88,4 +114,19 @@ func TestAdmission(t *testing.T) { t.Errorf("Did not expect an error %v", err) } + // verify create/update/delete of object in non-existant namespace throws error + err = handler.Admit(admission.NewAttributesRecord(&badPod, "Pod", badPod.Namespace, badPod.Name, "pods", "", admission.Create, nil)) + if err == nil { + t.Errorf("Expected an aerror that objects cannot be created in non-existant namespaces", err) + } + + err = handler.Admit(admission.NewAttributesRecord(&badPod, "Pod", badPod.Namespace, badPod.Name, "pods", "", admission.Update, nil)) + if err == nil { + t.Errorf("Expected an aerror that objects cannot be updated in non-existant namespaces", err) + } + + err = handler.Admit(admission.NewAttributesRecord(&badPod, "Pod", badPod.Namespace, badPod.Name, "pods", "", admission.Delete, nil)) + if err == nil { + t.Errorf("Expected an aerror that objects cannot be deleted in non-existant namespaces", err) + } }