Merge pull request #12605 from DirectXMan12/refactor/merge-namespace-admission-controllers

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,12 +52,9 @@ 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()) {
+	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"))
 	}
-		return nil
-	}
 
 	defaultVersion, kind, err := api.RESTMapper.VersionAndKindForResource(a.GetResource())
 	if err != nil {
@@ -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
+		// 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()))
 		}
+	}
+
+	// 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)
+	}
 }