Skip to content

Commit 36b3f76

Browse files
SergeyKanzhelevNathan Herz
authored andcommitted
do not allow the node to update it's owner reference
1 parent be53c1e commit 36b3f76

File tree

2 files changed

+35
-6
lines changed

2 files changed

+35
-6
lines changed

plugin/pkg/admission/noderestriction/admission.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -536,6 +536,11 @@ func (p *Plugin) admitNode(nodeName string, a admission.Attributes) error {
536536
return admission.NewForbidden(a, fmt.Errorf("node %q is not allowed to modify taints", nodeName))
537537
}
538538

539+
// Don't allow a node to update its own ownerReferences.
540+
if !apiequality.Semantic.DeepEqual(node.OwnerReferences, oldNode.OwnerReferences) {
541+
return admission.NewForbidden(a, fmt.Errorf("node %q is not allowed to modify ownerReferences", nodeName))
542+
}
543+
539544
// Don't allow a node to update labels outside the allowed set.
540545
// This would allow a node to add or modify its labels in a way that would let it steer privileged workloads to itself.
541546
modifiedLabels := getModifiedLabels(node.Labels, oldNode.Labels)

plugin/pkg/admission/noderestriction/admission_test.go

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -260,10 +260,14 @@ func (a *admitTestCase) run(t *testing.T) {
260260

261261
func Test_nodePlugin_Admit(t *testing.T) {
262262
var (
263-
mynode = &user.DefaultInfo{Name: "system:node:mynode", Groups: []string{"system:nodes"}}
264-
bob = &user.DefaultInfo{Name: "bob"}
263+
trueRef = true
264+
mynode = &user.DefaultInfo{Name: "system:node:mynode", Groups: []string{"system:nodes"}}
265+
bob = &user.DefaultInfo{Name: "bob"}
266+
267+
mynodeObjMeta = metav1.ObjectMeta{Name: "mynode", UID: "mynode-uid"}
268+
mynodeObjMetaOwnerRefA = metav1.ObjectMeta{Name: "mynode", UID: "mynode-uid", OwnerReferences: []metav1.OwnerReference{{Name: "fooerA", Controller: &trueRef}}}
269+
mynodeObjMetaOwnerRefB = metav1.ObjectMeta{Name: "mynode", UID: "mynode-uid", OwnerReferences: []metav1.OwnerReference{{Name: "fooerB", Controller: &trueRef}}}
265270

266-
mynodeObjMeta = metav1.ObjectMeta{Name: "mynode", UID: "mynode-uid"}
267271
mynodeObj = &api.Node{ObjectMeta: mynodeObjMeta}
268272
mynodeObjConfigA = &api.Node{ObjectMeta: mynodeObjMeta, Spec: api.NodeSpec{ConfigSource: &api.NodeConfigSource{
269273
ConfigMap: &api.ConfigMapNodeConfigSource{
@@ -280,9 +284,11 @@ func Test_nodePlugin_Admit(t *testing.T) {
280284
KubeletConfigKey: "kubelet",
281285
}}}}
282286

283-
mynodeObjTaintA = &api.Node{ObjectMeta: mynodeObjMeta, Spec: api.NodeSpec{Taints: []api.Taint{{Key: "mykey", Value: "A"}}}}
284-
mynodeObjTaintB = &api.Node{ObjectMeta: mynodeObjMeta, Spec: api.NodeSpec{Taints: []api.Taint{{Key: "mykey", Value: "B"}}}}
285-
othernodeObj = &api.Node{ObjectMeta: metav1.ObjectMeta{Name: "othernode"}}
287+
mynodeObjTaintA = &api.Node{ObjectMeta: mynodeObjMeta, Spec: api.NodeSpec{Taints: []api.Taint{{Key: "mykey", Value: "A"}}}}
288+
mynodeObjTaintB = &api.Node{ObjectMeta: mynodeObjMeta, Spec: api.NodeSpec{Taints: []api.Taint{{Key: "mykey", Value: "B"}}}}
289+
mynodeObjOwnerRefA = &api.Node{ObjectMeta: mynodeObjMetaOwnerRefA}
290+
mynodeObjOwnerRefB = &api.Node{ObjectMeta: mynodeObjMetaOwnerRefB}
291+
othernodeObj = &api.Node{ObjectMeta: metav1.ObjectMeta{Name: "othernode"}}
286292

287293
coremymirrorpod, v1mymirrorpod = makeTestPod("ns", "mymirrorpod", "mynode", true)
288294
coreothermirrorpod, v1othermirrorpod = makeTestPod("ns", "othermirrorpod", "othernode", true)
@@ -1222,6 +1228,24 @@ func Test_nodePlugin_Admit(t *testing.T) {
12221228
attributes: admission.NewAttributesRecord(setForbiddenUpdateLabels(mynodeObj, "new"), setForbiddenUpdateLabels(mynodeObj, "old"), nodeKind, mynodeObj.Namespace, mynodeObj.Name, nodeResource, "", admission.Update, &metav1.UpdateOptions{}, false, mynode),
12231229
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`,
12241230
},
1231+
{
1232+
name: "forbid update of my node: add owner reference",
1233+
podsGetter: existingPods,
1234+
attributes: admission.NewAttributesRecord(mynodeObjOwnerRefA, mynodeObj, nodeKind, mynodeObj.Namespace, mynodeObj.Name, nodeResource, "", admission.Update, &metav1.UpdateOptions{}, false, mynode),
1235+
err: "node \"mynode\" is not allowed to modify ownerReferences",
1236+
},
1237+
{
1238+
name: "forbid update of my node: remove owner reference",
1239+
podsGetter: existingPods,
1240+
attributes: admission.NewAttributesRecord(mynodeObj, mynodeObjOwnerRefA, nodeKind, mynodeObj.Namespace, mynodeObj.Name, nodeResource, "", admission.Update, &metav1.UpdateOptions{}, false, mynode),
1241+
err: "node \"mynode\" is not allowed to modify ownerReferences",
1242+
},
1243+
{
1244+
name: "forbid update of my node: change owner reference",
1245+
podsGetter: existingPods,
1246+
attributes: admission.NewAttributesRecord(mynodeObjOwnerRefA, mynodeObjOwnerRefB, nodeKind, mynodeObj.Namespace, mynodeObj.Name, nodeResource, "", admission.Update, &metav1.UpdateOptions{}, false, mynode),
1247+
err: "node \"mynode\" is not allowed to modify ownerReferences",
1248+
},
12251249

12261250
// Other node object
12271251
{

0 commit comments

Comments
 (0)