Mirror pod without OwnerReference should not be created

Signed-off-by: Ted Yu <[email protected]>

Author: tedyu

plugin/pkg/admission/noderestriction/admission.go

@@ -224,6 +224,9 @@ func (p *Plugin) admitPodCreate(nodeName string, a admission.Attributes) error {
 	if len(pod.OwnerReferences) > 1 {
 		return admission.NewForbidden(a, fmt.Errorf("node %q can only create pods with a single owner reference set to itself", nodeName))
 	}
+	if len(pod.OwnerReferences) == 0 {
+		return admission.NewForbidden(a, fmt.Errorf("node %q can only create pods with an owner reference set to itself", nodeName))
+	}
 	if len(pod.OwnerReferences) == 1 {
 		owner := pod.OwnerReferences[0]
 		if owner.APIVersion != v1.SchemeGroupVersion.String() ||

plugin/pkg/admission/noderestriction/admission_test.go

@@ -1314,8 +1314,9 @@ func Test_nodePlugin_Admit_OwnerReference(t *testing.T) {
 		expectErr   string
 	}{
 		{
-			name:   "no owner",
-			owners: nil,
+			name:      "no owner",
+			owners:    nil,
+			expectErr: "pods \"test\" is forbidden: node \"mynode\" can only create pods with an owner reference set to itself",
 		},
 		{
 			name:   "valid owner",

test/integration/auth/node_test.go

@@ -224,13 +224,26 @@ func TestNodeAuthorizer(t *testing.T) {
 
 	createNode2MirrorPod := func(client clientset.Interface) func() error {
 		return func() error {
-			_, err := client.CoreV1().Pods("ns").Create(context.TODO(), &corev1.Pod{
+			const nodeName = "node2"
+			node, err := client.CoreV1().Nodes().Get(context.TODO(), nodeName, metav1.GetOptions{})
+			if err != nil {
+				return err
+			}
+			controller := true
+			_, err = client.CoreV1().Pods("ns").Create(context.TODO(), &corev1.Pod{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:        "node2mirrorpod",
 					Annotations: map[string]string{corev1.MirrorPodAnnotationKey: "true"},
+					OwnerReferences: []metav1.OwnerReference{{
+						APIVersion: corev1.SchemeGroupVersion.String(),
+						Kind:       "Node",
+						Name:       nodeName,
+						UID:        node.UID,
+						Controller: &controller,
+					}},
 				},
 				Spec: corev1.PodSpec{
-					NodeName:   "node2",
+					NodeName:   nodeName,
 					Containers: []corev1.Container{{Name: "image", Image: "busybox"}},
 				},
 			}, metav1.CreateOptions{})
@@ -462,9 +475,7 @@ func TestNodeAuthorizer(t *testing.T) {
 	expectForbidden(t, getPVC(nodeanonClient))
 	expectForbidden(t, getPV(nodeanonClient))
 	expectForbidden(t, createNode2NormalPod(nodeanonClient))
-	expectForbidden(t, createNode2MirrorPod(nodeanonClient))
 	expectForbidden(t, deleteNode2NormalPod(nodeanonClient))
-	expectForbidden(t, deleteNode2MirrorPod(nodeanonClient))
 	expectForbidden(t, createNode2MirrorPodEviction(nodeanonClient))
 	expectForbidden(t, createNode2(nodeanonClient))
 	expectForbidden(t, updateNode2Status(nodeanonClient))
@@ -476,8 +487,6 @@ func TestNodeAuthorizer(t *testing.T) {
 	expectForbidden(t, getPVC(node1Client))
 	expectForbidden(t, getPV(node1Client))
 	expectForbidden(t, createNode2NormalPod(nodeanonClient))
-	expectForbidden(t, createNode2MirrorPod(node1Client))
-	expectNotFound(t, deleteNode2MirrorPod(node1Client))
 	expectNotFound(t, createNode2MirrorPodEviction(node1Client))
 	expectForbidden(t, createNode2(node1Client))
 	expectNotFound(t, updateNode2Status(node1Client))
@@ -492,21 +501,23 @@ func TestNodeAuthorizer(t *testing.T) {
 
 	expectForbidden(t, createNode2NormalPod(nodeanonClient))
 	// mirror pod and self node lifecycle is allowed
+	expectAllowed(t, createNode2(node2Client))
+	expectAllowed(t, updateNode2Status(node2Client))
+	expectForbidden(t, createNode2MirrorPod(nodeanonClient))
+	expectForbidden(t, deleteNode2MirrorPod(nodeanonClient))
+	expectForbidden(t, createNode2MirrorPod(node1Client))
+	expectNotFound(t, deleteNode2MirrorPod(node1Client))
+	// create a pod as an admin to add object references
+	expectAllowed(t, createNode2NormalPod(superuserClient))
+
 	expectAllowed(t, createNode2MirrorPod(node2Client))
 	expectAllowed(t, deleteNode2MirrorPod(node2Client))
 	expectAllowed(t, createNode2MirrorPod(node2Client))
 	expectAllowed(t, createNode2MirrorPodEviction(node2Client))
-	expectAllowed(t, createNode2(node2Client))
-	expectAllowed(t, updateNode2Status(node2Client))
 	// self deletion is not allowed
 	expectForbidden(t, deleteNode2(node2Client))
 	// modification of another node's status is not allowed
 	expectForbidden(t, updateNode2Status(node1Client))
-	// clean up node2
-	expectAllowed(t, deleteNode2(superuserClient))
-
-	// create a pod as an admin to add object references
-	expectAllowed(t, createNode2NormalPod(superuserClient))
 
 	// unidentifiable node and node1 are still forbidden
 	expectForbidden(t, getSecret(nodeanonClient))
@@ -553,6 +564,8 @@ func TestNodeAuthorizer(t *testing.T) {
 	expectAllowed(t, createNode2MirrorPod(superuserClient))
 	expectAllowed(t, createNode2NormalPodEviction(node2Client))
 	expectAllowed(t, createNode2MirrorPodEviction(node2Client))
+	// clean up node2
+	expectAllowed(t, deleteNode2(superuserClient))
 
 	// re-create a pod as an admin to add object references
 	expectAllowed(t, createNode2NormalPod(superuserClient))