diff --git a/plugin/pkg/admission/noderestriction/admission.go b/plugin/pkg/admission/noderestriction/admission.go index 4ec35894e8f..9618427feb9 100644 --- a/plugin/pkg/admission/noderestriction/admission.go +++ b/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() || diff --git a/plugin/pkg/admission/noderestriction/admission_test.go b/plugin/pkg/admission/noderestriction/admission_test.go index 2d0565cffca..2306ddee194 100644 --- a/plugin/pkg/admission/noderestriction/admission_test.go +++ b/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", diff --git a/test/integration/auth/node_test.go b/test/integration/auth/node_test.go index 4527bf3d198..f2622ffbf24 100644 --- a/test/integration/auth/node_test.go +++ b/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))