From 944b0a492356894a24dac5facfe5b9ad7be43b08 Mon Sep 17 00:00:00 2001 From: Sergey Kanzhelev Date: Thu, 22 May 2025 17:54:10 +0000 Subject: [PATCH] do not allow the node to update it's owner reference --- .../admission/noderestriction/admission.go | 5 +++ .../noderestriction/admission_test.go | 36 +++++++++++++++---- 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/plugin/pkg/admission/noderestriction/admission.go b/plugin/pkg/admission/noderestriction/admission.go index 33a76d0f979..cf4bd8a970e 100644 --- a/plugin/pkg/admission/noderestriction/admission.go +++ b/plugin/pkg/admission/noderestriction/admission.go @@ -548,6 +548,11 @@ func (p *Plugin) admitNode(nodeName string, a admission.Attributes) error { return admission.NewForbidden(a, fmt.Errorf("node %q is not allowed to modify taints", nodeName)) } + // Don't allow a node to update its own ownerReferences. + if !apiequality.Semantic.DeepEqual(node.OwnerReferences, oldNode.OwnerReferences) { + return admission.NewForbidden(a, fmt.Errorf("node %q is not allowed to modify ownerReferences", nodeName)) + } + // Don't allow a node to update labels outside the allowed set. // This would allow a node to add or modify its labels in a way that would let it steer privileged workloads to itself. modifiedLabels := getModifiedLabels(node.Labels, oldNode.Labels) diff --git a/plugin/pkg/admission/noderestriction/admission_test.go b/plugin/pkg/admission/noderestriction/admission_test.go index 53cef1fdb4a..8ef0e5af68d 100644 --- a/plugin/pkg/admission/noderestriction/admission_test.go +++ b/plugin/pkg/admission/noderestriction/admission_test.go @@ -275,10 +275,14 @@ func (a *admitTestCase) run(t *testing.T) { func Test_nodePlugin_Admit(t *testing.T) { var ( - mynode = &user.DefaultInfo{Name: "system:node:mynode", Groups: []string{"system:nodes"}} - bob = &user.DefaultInfo{Name: "bob"} + trueRef = true + mynode = &user.DefaultInfo{Name: "system:node:mynode", Groups: []string{"system:nodes"}} + bob = &user.DefaultInfo{Name: "bob"} + + mynodeObjMeta = metav1.ObjectMeta{Name: "mynode", UID: "mynode-uid"} + mynodeObjMetaOwnerRefA = metav1.ObjectMeta{Name: "mynode", UID: "mynode-uid", OwnerReferences: []metav1.OwnerReference{{Name: "fooerA", Controller: &trueRef}}} + mynodeObjMetaOwnerRefB = metav1.ObjectMeta{Name: "mynode", UID: "mynode-uid", OwnerReferences: []metav1.OwnerReference{{Name: "fooerB", Controller: &trueRef}}} - mynodeObjMeta = metav1.ObjectMeta{Name: "mynode", UID: "mynode-uid"} mynodeObj = &api.Node{ObjectMeta: mynodeObjMeta} mynodeObjConfigA = &api.Node{ObjectMeta: mynodeObjMeta, Spec: api.NodeSpec{ConfigSource: &api.NodeConfigSource{ ConfigMap: &api.ConfigMapNodeConfigSource{ @@ -295,9 +299,11 @@ func Test_nodePlugin_Admit(t *testing.T) { KubeletConfigKey: "kubelet", }}}} - mynodeObjTaintA = &api.Node{ObjectMeta: mynodeObjMeta, Spec: api.NodeSpec{Taints: []api.Taint{{Key: "mykey", Value: "A"}}}} - mynodeObjTaintB = &api.Node{ObjectMeta: mynodeObjMeta, Spec: api.NodeSpec{Taints: []api.Taint{{Key: "mykey", Value: "B"}}}} - othernodeObj = &api.Node{ObjectMeta: metav1.ObjectMeta{Name: "othernode"}} + mynodeObjTaintA = &api.Node{ObjectMeta: mynodeObjMeta, Spec: api.NodeSpec{Taints: []api.Taint{{Key: "mykey", Value: "A"}}}} + mynodeObjTaintB = &api.Node{ObjectMeta: mynodeObjMeta, Spec: api.NodeSpec{Taints: []api.Taint{{Key: "mykey", Value: "B"}}}} + mynodeObjOwnerRefA = &api.Node{ObjectMeta: mynodeObjMetaOwnerRefA} + mynodeObjOwnerRefB = &api.Node{ObjectMeta: mynodeObjMetaOwnerRefB} + othernodeObj = &api.Node{ObjectMeta: metav1.ObjectMeta{Name: "othernode"}} coremymirrorpod, v1mymirrorpod = makeTestPod("ns", "mymirrorpod", "mynode", true) coreothermirrorpod, v1othermirrorpod = makeTestPod("ns", "othermirrorpod", "othernode", true) @@ -1280,6 +1286,24 @@ func Test_nodePlugin_Admit(t *testing.T) { attributes: admission.NewAttributesRecord(setForbiddenUpdateLabels(mynodeObj, "new"), setForbiddenUpdateLabels(mynodeObj, "old"), nodeKind, mynodeObj.Namespace, mynodeObj.Name, nodeResource, "", admission.Update, &metav1.UpdateOptions{}, false, mynode), err: `is not allowed to modify labels: foo.node-restriction.kubernetes.io/foo, node-restriction.kubernetes.io/foo, other.k8s.io/foo, other.kubernetes.io/foo`, }, + { + name: "forbid update of my node: add owner reference", + podsGetter: existingPods, + attributes: admission.NewAttributesRecord(mynodeObjOwnerRefA, mynodeObj, nodeKind, mynodeObj.Namespace, mynodeObj.Name, nodeResource, "", admission.Update, &metav1.UpdateOptions{}, false, mynode), + err: "node \"mynode\" is not allowed to modify ownerReferences", + }, + { + name: "forbid update of my node: remove owner reference", + podsGetter: existingPods, + attributes: admission.NewAttributesRecord(mynodeObj, mynodeObjOwnerRefA, nodeKind, mynodeObj.Namespace, mynodeObj.Name, nodeResource, "", admission.Update, &metav1.UpdateOptions{}, false, mynode), + err: "node \"mynode\" is not allowed to modify ownerReferences", + }, + { + name: "forbid update of my node: change owner reference", + podsGetter: existingPods, + attributes: admission.NewAttributesRecord(mynodeObjOwnerRefA, mynodeObjOwnerRefB, nodeKind, mynodeObj.Namespace, mynodeObj.Name, nodeResource, "", admission.Update, &metav1.UpdateOptions{}, false, mynode), + err: "node \"mynode\" is not allowed to modify ownerReferences", + }, // Other node object {