Make NodeRestriction admission allow evictions for bounded pods

plugin/pkg/admission/noderestriction/BUILD

@@ -15,6 +15,7 @@ go_library(
     deps = [
         "//pkg/api:go_default_library",
         "//pkg/api/pod:go_default_library",
+        "//pkg/apis/policy:go_default_library",
         "//pkg/auth/nodeidentifier:go_default_library",
         "//pkg/client/clientset_generated/internalclientset:go_default_library",
         "//pkg/client/clientset_generated/internalclientset/typed/core/internalversion:go_default_library",
@@ -32,6 +33,7 @@ go_test(
     tags = ["automanaged"],
     deps = [
         "//pkg/api:go_default_library",
+        "//pkg/apis/policy:go_default_library",
         "//pkg/auth/nodeidentifier:go_default_library",
         "//pkg/client/clientset_generated/internalclientset/fake:go_default_library",
         "//pkg/client/clientset_generated/internalclientset/typed/core/internalversion:go_default_library",

plugin/pkg/admission/noderestriction/admission.go

@@ -25,6 +25,7 @@ import (
 	"k8s.io/apiserver/pkg/admission"
 	"k8s.io/kubernetes/pkg/api"
 	podutil "k8s.io/kubernetes/pkg/api/pod"
+	"k8s.io/kubernetes/pkg/apis/policy"
 	"k8s.io/kubernetes/pkg/auth/nodeidentifier"
 	"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
 	coreinternalversion "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/core/internalversion"
@@ -102,6 +103,8 @@ func (c *nodePlugin) Admit(a admission.Attributes) error {
 			return c.admitPod(nodeName, a)
 		case "status":
 			return c.admitPodStatus(nodeName, a)
+		case "eviction":
+			return c.admitPodEviction(nodeName, a)
 		default:
 			return admission.NewForbidden(a, fmt.Errorf("unexpected pod subresource %q", a.GetSubresource()))
 		}
@@ -161,6 +164,9 @@ func (c *nodePlugin) admitPod(nodeName string, a admission.Attributes) error {
 		if errors.IsNotFound(err) {
 			// wasn't found in the server cache, do a live lookup before forbidding
 			existingPod, err = c.podsGetter.Pods(a.GetNamespace()).Get(a.GetName(), v1.GetOptions{})
+			if errors.IsNotFound(err) {
+				return err
+			}
 		}
 		if err != nil {
 			return admission.NewForbidden(a, err)
@@ -195,6 +201,45 @@ func (c *nodePlugin) admitPodStatus(nodeName string, a admission.Attributes) err
 	}
 }
 
+func (c *nodePlugin) admitPodEviction(nodeName string, a admission.Attributes) error {
+	switch a.GetOperation() {
+	case admission.Create:
+		// require eviction to an existing pod object
+		eviction, ok := a.GetObject().(*policy.Eviction)
+		if !ok {
+			return admission.NewForbidden(a, fmt.Errorf("unexpected type %T", a.GetObject()))
+		}
+		// use pod name from the admission attributes, if set, rather than from the submitted Eviction object
+		podName := a.GetName()
+		if len(podName) == 0 {
+			if len(eviction.Name) == 0 {
+				return admission.NewForbidden(a, fmt.Errorf("could not determine pod from request data"))
+			}
+			podName = eviction.Name
+		}
+		// get the existing pod from the server cache
+		existingPod, err := c.podsGetter.Pods(a.GetNamespace()).Get(podName, v1.GetOptions{ResourceVersion: "0"})
+		if errors.IsNotFound(err) {
+			// wasn't found in the server cache, do a live lookup before forbidding
+			existingPod, err = c.podsGetter.Pods(a.GetNamespace()).Get(podName, v1.GetOptions{})
+			if errors.IsNotFound(err) {
+				return err
+			}
+		}
+		if err != nil {
+			return admission.NewForbidden(a, err)
+		}
+		// only allow a node to evict a pod bound to itself
+		if existingPod.Spec.NodeName != nodeName {
+			return admission.NewForbidden(a, fmt.Errorf("node %s can only evict pods with spec.nodeName set to itself", nodeName))
+		}
+		return nil
+
+	default:
+		return admission.NewForbidden(a, fmt.Errorf("unexpected operation %s", a.GetOperation()))
+	}
+}
+
 func (c *nodePlugin) admitNode(nodeName string, a admission.Attributes) error {
 	requestedName := a.GetName()
 

plugin/pkg/admission/noderestriction/admission_test.go

@@ -24,6 +24,8 @@ import (
 	"k8s.io/apiserver/pkg/admission"
 	"k8s.io/apiserver/pkg/authentication/user"
 	"k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/apis/policy"
+	policyapi "k8s.io/kubernetes/pkg/apis/policy"
 	"k8s.io/kubernetes/pkg/auth/nodeidentifier"
 	"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake"
 	coreinternalversion "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/core/internalversion"
@@ -40,6 +42,12 @@ func makeTestPod(namespace, name, node string, mirror bool) *api.Pod {
 	return pod
 }
 
+func makeTestPodEviction(name string) *policy.Eviction {
+	eviction := &policy.Eviction{}
+	eviction.Name = name
+	return eviction
+}
+
 func Test_nodePlugin_Admit(t *testing.T) {
 	var (
 		mynode = &user.DefaultInfo{Name: "system:node:mynode", Groups: []string{"system:nodes"}}
@@ -54,12 +62,22 @@ func Test_nodePlugin_Admit(t *testing.T) {
 		mypod            = makeTestPod("ns", "mypod", "mynode", false)
 		otherpod         = makeTestPod("ns", "otherpod", "othernode", false)
 		unboundpod       = makeTestPod("ns", "unboundpod", "", false)
+		unnamedpod       = makeTestPod("ns", "", "mynode", false)
+
+		mymirrorpodEviction      = makeTestPodEviction("mymirrorpod")
+		othermirrorpodEviction   = makeTestPodEviction("othermirrorpod")
+		unboundmirrorpodEviction = makeTestPodEviction("unboundmirrorpod")
+		mypodEviction            = makeTestPodEviction("mypod")
+		otherpodEviction         = makeTestPodEviction("otherpod")
+		unboundpodEviction       = makeTestPodEviction("unboundpod")
+		unnamedEviction          = makeTestPodEviction("")
 
 		configmapResource = api.Resource("configmap").WithVersion("v1")
 		configmapKind     = api.Kind("ConfigMap").WithVersion("v1")
 
-		podResource = api.Resource("pods").WithVersion("v1")
-		podKind     = api.Kind("Pod").WithVersion("v1")
+		podResource  = api.Resource("pods").WithVersion("v1")
+		podKind      = api.Kind("Pod").WithVersion("v1")
+		evictionKind = policyapi.Kind("Eviction").WithVersion("v1beta1")
 
 		nodeResource = api.Resource("nodes").WithVersion("v1")
 		nodeKind     = api.Kind("Node").WithVersion("v1")
@@ -123,6 +141,30 @@ func Test_nodePlugin_Admit(t *testing.T) {
 			attributes: admission.NewAttributesRecord(nil, nil, podKind, mymirrorpod.Namespace, mymirrorpod.Name, podResource, "status", admission.Delete, mynode),
 			err:        "forbidden: unexpected operation",
 		},
+		{
+			name:       "allow create of eviction for mirror pod bound to self",
+			podsGetter: existingPods,
+			attributes: admission.NewAttributesRecord(mymirrorpodEviction, nil, evictionKind, mymirrorpod.Namespace, mymirrorpod.Name, podResource, "eviction", admission.Create, mynode),
+			err:        "",
+		},
+		{
+			name:       "forbid update of eviction for mirror pod bound to self",
+			podsGetter: existingPods,
+			attributes: admission.NewAttributesRecord(mymirrorpodEviction, nil, evictionKind, mymirrorpod.Namespace, mymirrorpod.Name, podResource, "eviction", admission.Update, mynode),
+			err:        "forbidden: unexpected operation",
+		},
+		{
+			name:       "forbid delete of eviction for mirror pod bound to self",
+			podsGetter: existingPods,
+			attributes: admission.NewAttributesRecord(mymirrorpodEviction, nil, evictionKind, mymirrorpod.Namespace, mymirrorpod.Name, podResource, "eviction", admission.Delete, mynode),
+			err:        "forbidden: unexpected operation",
+		},
+		{
+			name:       "allow create of unnamed eviction for mirror pod bound to self",
+			podsGetter: existingPods,
+			attributes: admission.NewAttributesRecord(unnamedEviction, nil, evictionKind, mymirrorpod.Namespace, mymirrorpod.Name, podResource, "eviction", admission.Create, mynode),
+			err:        "",
+		},
 
 		// Mirror pods bound to another node
 		{
@@ -161,6 +203,30 @@ func Test_nodePlugin_Admit(t *testing.T) {
 			attributes: admission.NewAttributesRecord(nil, nil, podKind, othermirrorpod.Namespace, othermirrorpod.Name, podResource, "status", admission.Delete, mynode),
 			err:        "forbidden: unexpected operation",
 		},
+		{
+			name:       "forbid create of eviction for mirror pod bound to another",
+			podsGetter: existingPods,
+			attributes: admission.NewAttributesRecord(othermirrorpodEviction, nil, evictionKind, othermirrorpod.Namespace, othermirrorpod.Name, podResource, "eviction", admission.Create, mynode),
+			err:        "spec.nodeName set to itself",
+		},
+		{
+			name:       "forbid update of eviction for mirror pod bound to another",
+			podsGetter: existingPods,
+			attributes: admission.NewAttributesRecord(othermirrorpodEviction, nil, evictionKind, othermirrorpod.Namespace, othermirrorpod.Name, podResource, "eviction", admission.Update, mynode),
+			err:        "forbidden: unexpected operation",
+		},
+		{
+			name:       "forbid delete of eviction for mirror pod bound to another",
+			podsGetter: existingPods,
+			attributes: admission.NewAttributesRecord(othermirrorpodEviction, nil, evictionKind, othermirrorpod.Namespace, othermirrorpod.Name, podResource, "eviction", admission.Delete, mynode),
+			err:        "forbidden: unexpected operation",
+		},
+		{
+			name:       "forbid create of unnamed eviction for mirror pod to another",
+			podsGetter: existingPods,
+			attributes: admission.NewAttributesRecord(unnamedEviction, nil, evictionKind, othermirrorpod.Namespace, othermirrorpod.Name, podResource, "eviction", admission.Create, mynode),
+			err:        "spec.nodeName set to itself",
+		},
 
 		// Mirror pods not bound to any node
 		{
@@ -199,6 +265,30 @@ func Test_nodePlugin_Admit(t *testing.T) {
 			attributes: admission.NewAttributesRecord(nil, nil, podKind, unboundmirrorpod.Namespace, unboundmirrorpod.Name, podResource, "status", admission.Delete, mynode),
 			err:        "forbidden: unexpected operation",
 		},
+		{
+			name:       "forbid create of eviction for mirror pod unbound",
+			podsGetter: existingPods,
+			attributes: admission.NewAttributesRecord(unboundmirrorpodEviction, nil, evictionKind, unboundmirrorpod.Namespace, unboundmirrorpod.Name, podResource, "eviction", admission.Create, mynode),
+			err:        "spec.nodeName set to itself",
+		},
+		{
+			name:       "forbid update of eviction for mirror pod unbound",
+			podsGetter: existingPods,
+			attributes: admission.NewAttributesRecord(unboundmirrorpodEviction, nil, evictionKind, unboundmirrorpod.Namespace, unboundmirrorpod.Name, podResource, "eviction", admission.Update, mynode),
+			err:        "forbidden: unexpected operation",
+		},
+		{
+			name:       "forbid delete of eviction for mirror pod unbound",
+			podsGetter: existingPods,
+			attributes: admission.NewAttributesRecord(unboundmirrorpodEviction, nil, evictionKind, unboundmirrorpod.Namespace, unboundmirrorpod.Name, podResource, "eviction", admission.Delete, mynode),
+			err:        "forbidden: unexpected operation",
+		},
+		{
+			name:       "forbid create of unnamed eviction for mirror pod unbound",
+			podsGetter: existingPods,
+			attributes: admission.NewAttributesRecord(unnamedEviction, nil, evictionKind, unboundmirrorpod.Namespace, unboundmirrorpod.Name, podResource, "eviction", admission.Create, mynode),
+			err:        "spec.nodeName set to itself",
+		},
 
 		// Normal pods bound to us
 		{
@@ -237,6 +327,24 @@ func Test_nodePlugin_Admit(t *testing.T) {
 			attributes: admission.NewAttributesRecord(nil, nil, podKind, mypod.Namespace, mypod.Name, podResource, "status", admission.Delete, mynode),
 			err:        "forbidden: unexpected operation",
 		},
+		{
+			name:       "forbid update of eviction for normal pod bound to self",
+			podsGetter: existingPods,
+			attributes: admission.NewAttributesRecord(mypodEviction, nil, evictionKind, mypod.Namespace, mypod.Name, podResource, "eviction", admission.Update, mynode),
+			err:        "forbidden: unexpected operation",
+		},
+		{
+			name:       "forbid delete of eviction for normal pod bound to self",
+			podsGetter: existingPods,
+			attributes: admission.NewAttributesRecord(mypodEviction, nil, evictionKind, mypod.Namespace, mypod.Name, podResource, "eviction", admission.Delete, mynode),
+			err:        "forbidden: unexpected operation",
+		},
+		{
+			name:       "allow create of unnamed eviction for normal pod bound to self",
+			podsGetter: existingPods,
+			attributes: admission.NewAttributesRecord(unnamedEviction, nil, evictionKind, mypod.Namespace, mypod.Name, podResource, "eviction", admission.Create, mynode),
+			err:        "",
+		},
 
 		// Normal pods bound to another
 		{
@@ -275,6 +383,30 @@ func Test_nodePlugin_Admit(t *testing.T) {
 			attributes: admission.NewAttributesRecord(nil, nil, podKind, otherpod.Namespace, otherpod.Name, podResource, "status", admission.Delete, mynode),
 			err:        "forbidden: unexpected operation",
 		},
+		{
+			name:       "forbid create of eviction for normal pod bound to another",
+			podsGetter: existingPods,
+			attributes: admission.NewAttributesRecord(otherpodEviction, nil, evictionKind, otherpodEviction.Namespace, otherpodEviction.Name, podResource, "eviction", admission.Create, mynode),
+			err:        "spec.nodeName set to itself",
+		},
+		{
+			name:       "forbid update of eviction for normal pod bound to another",
+			podsGetter: existingPods,
+			attributes: admission.NewAttributesRecord(otherpodEviction, nil, evictionKind, otherpodEviction.Namespace, otherpodEviction.Name, podResource, "eviction", admission.Update, mynode),
+			err:        "forbidden: unexpected operation",
+		},
+		{
+			name:       "forbid delete of eviction for normal pod bound to another",
+			podsGetter: existingPods,
+			attributes: admission.NewAttributesRecord(otherpodEviction, nil, evictionKind, otherpodEviction.Namespace, otherpodEviction.Name, podResource, "eviction", admission.Delete, mynode),
+			err:        "forbidden: unexpected operation",
+		},
+		{
+			name:       "forbid create of eviction for normal pod bound to another",
+			podsGetter: existingPods,
+			attributes: admission.NewAttributesRecord(unnamedEviction, nil, evictionKind, otherpod.Namespace, otherpod.Name, podResource, "eviction", admission.Create, mynode),
+			err:        "spec.nodeName set to itself",
+		},
 
 		// Normal pods not bound to any node
 		{
@@ -313,6 +445,30 @@ func Test_nodePlugin_Admit(t *testing.T) {
 			attributes: admission.NewAttributesRecord(nil, nil, podKind, unboundpod.Namespace, unboundpod.Name, podResource, "status", admission.Delete, mynode),
 			err:        "forbidden: unexpected operation",
 		},
+		{
+			name:       "forbid create of eviction for normal pod unbound",
+			podsGetter: existingPods,
+			attributes: admission.NewAttributesRecord(unboundpodEviction, nil, evictionKind, unboundpod.Namespace, unboundpod.Name, podResource, "eviction", admission.Create, mynode),
+			err:        "spec.nodeName set to itself",
+		},
+		{
+			name:       "forbid update of eviction for normal pod unbound",
+			podsGetter: existingPods,
+			attributes: admission.NewAttributesRecord(unboundpodEviction, nil, evictionKind, unboundpod.Namespace, unboundpod.Name, podResource, "eviction", admission.Update, mynode),
+			err:        "forbidden: unexpected operation",
+		},
+		{
+			name:       "forbid delete of eviction for normal pod unbound",
+			podsGetter: existingPods,
+			attributes: admission.NewAttributesRecord(unboundpodEviction, nil, evictionKind, unboundpod.Namespace, unboundpod.Name, podResource, "eviction", admission.Delete, mynode),
+			err:        "forbidden: unexpected operation",
+		},
+		{
+			name:       "forbid create of unnamed eviction for normal unbound",
+			podsGetter: existingPods,
+			attributes: admission.NewAttributesRecord(unnamedEviction, nil, evictionKind, unboundpod.Namespace, unboundpod.Name, podResource, "eviction", admission.Create, mynode),
+			err:        "spec.nodeName set to itself",
+		},
 
 		// Missing pod
 		{
@@ -321,6 +477,57 @@ func Test_nodePlugin_Admit(t *testing.T) {
 			attributes: admission.NewAttributesRecord(nil, nil, podKind, unboundpod.Namespace, unboundpod.Name, podResource, "", admission.Delete, mynode),
 			err:        "not found",
 		},
+		{
+			name:       "forbid create of eviction for unknown pod",
+			podsGetter: noExistingPods,
+			attributes: admission.NewAttributesRecord(mypodEviction, nil, evictionKind, mypod.Namespace, mypod.Name, podResource, "eviction", admission.Create, mynode),
+			err:        "not found",
+		},
+		{
+			name:       "forbid update of eviction for unknown pod",
+			podsGetter: noExistingPods,
+			attributes: admission.NewAttributesRecord(mypodEviction, nil, evictionKind, mypod.Namespace, mypod.Name, podResource, "eviction", admission.Update, mynode),
+			err:        "forbidden: unexpected operation",
+		},
+		{
+			name:       "forbid delete of eviction for unknown pod",
+			podsGetter: noExistingPods,
+			attributes: admission.NewAttributesRecord(mypodEviction, nil, evictionKind, mypod.Namespace, mypod.Name, podResource, "eviction", admission.Delete, mynode),
+			err:        "forbidden: unexpected operation",
+		},
+		{
+			name:       "forbid create of unnamed eviction for unknown pod",
+			podsGetter: noExistingPods,
+			attributes: admission.NewAttributesRecord(unnamedEviction, nil, evictionKind, mypod.Namespace, mypod.Name, podResource, "eviction", admission.Create, mynode),
+			err:        "not found",
+		},
+
+		// Eviction for unnamed pod
+		{
+			name:       "allow create of eviction for unnamed pod",
+			podsGetter: existingPods,
+			attributes: admission.NewAttributesRecord(mypodEviction, nil, evictionKind, unnamedpod.Namespace, unnamedpod.Name, podResource, "eviction", admission.Create, mynode),
+			// use the submitted eviction resource name as the pod name
+			err: "",
+		},
+		{
+			name:       "forbid update of eviction for unnamed pod",
+			podsGetter: existingPods,
+			attributes: admission.NewAttributesRecord(mypodEviction, nil, evictionKind, unnamedpod.Namespace, unnamedpod.Name, podResource, "eviction", admission.Update, mynode),
+			err:        "forbidden: unexpected operation",
+		},
+		{
+			name:       "forbid delete of eviction for unnamed pod",
+			podsGetter: existingPods,
+			attributes: admission.NewAttributesRecord(mypodEviction, nil, evictionKind, unnamedpod.Namespace, unnamedpod.Name, podResource, "eviction", admission.Delete, mynode),
+			err:        "forbidden: unexpected operation",
+		},
+		{
+			name:       "forbid create of unnamed eviction for unnamed pod",
+			podsGetter: existingPods,
+			attributes: admission.NewAttributesRecord(unnamedEviction, nil, evictionKind, unnamedpod.Namespace, unnamedpod.Name, podResource, "eviction", admission.Create, mynode),
+			err:        "could not determine pod from request data",
+		},
 
 		// Resource pods
 		{

plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go

@@ -113,6 +113,9 @@ func NodeRules() []rbac.PolicyRule {
 		// Needed for the node to report status of pods it is running.
 		// Use the NodeRestriction admission plugin to limit a node to updating status of pods bound to itself.
 		rbac.NewRule("update").Groups(legacyGroup).Resources("pods/status").RuleOrDie(),
+		// Needed for the node to create pod evictions.
+		// Use the NodeRestriction admission plugin to limit a node to creating evictions for pods bound to itself.
+		rbac.NewRule("create").Groups(legacyGroup).Resources("pods/eviction").RuleOrDie(),
 
 		// Needed for imagepullsecrets, rbd/ceph and secret volumes, and secrets in envs
 		// Needed for configmap volume and envs

plugin/pkg/auth/authorizer/rbac/bootstrappolicy/testdata/cluster-roles.yaml

@@ -686,6 +686,12 @@ items:
     - pods/status
     verbs:
     - update
+  - apiGroups:
+    - ""
+    resources:
+    - pods/eviction
+    verbs:
+    - create
   - apiGroups:
     - ""
     resources: