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

@@ -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
 ```
 
 

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),

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)
+	}
 }