Merge pull request #63167 from liggitt/taint-modification

Automatic merge from submit-queue (batch tested with PRs 63167, 63357). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Prevent nodes from updating taints

Prevents kubelets from modifying or removing taints on update.

Nodes can set taints when they register themselves, but do not update/remove those taints after creation (that is done by the node controller based on reported node conditions).

xref kubernetes/community#911 kubernetes/enhancements#279

/sig node
/sig auth
/sig scheduling

/assign @mikedanese @k82cn

```release-note
The NodeRestriction admission plugin now prevents kubelets from modifying/removing taints applied to their Node API object.
```

plugin/pkg/admission/noderestriction/admission.go

@@ -338,6 +338,12 @@ func (c *nodePlugin) admitNode(nodeName string, a admission.Attributes) error {
 		if node.Spec.ConfigSource != nil && !apiequality.Semantic.DeepEqual(node.Spec.ConfigSource, oldNode.Spec.ConfigSource) {
 			return admission.NewForbidden(a, fmt.Errorf("cannot update configSource to a new non-nil configSource"))
 		}
+
+		// Don't allow a node to update its own taints. This would allow a node to remove or modify its
+		// taints in a way that would let it steer disallowed workloads to itself.
+		if !apiequality.Semantic.DeepEqual(node.Spec.Taints, oldNode.Spec.Taints) {
+			return admission.NewForbidden(a, fmt.Errorf("cannot modify taints"))
+		}
 	}
 
 	return nil

plugin/pkg/admission/noderestriction/admission_test.go

@@ -105,7 +105,9 @@ func Test_nodePlugin_Admit(t *testing.T) {
 				UID:              "quxUID",
 				KubeletConfigKey: "kubelet",
 			}}}}
-		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"}}}}
+		othernodeObj    = &api.Node{ObjectMeta: metav1.ObjectMeta{Name: "othernode"}}
 
 		mymirrorpod      = makeTestPod("ns", "mymirrorpod", "mynode", true)
 		othermirrorpod   = makeTestPod("ns", "othermirrorpod", "othernode", true)
@@ -633,6 +635,12 @@ func Test_nodePlugin_Admit(t *testing.T) {
 			attributes: admission.NewAttributesRecord(mynodeObj, nil, nodeKind, mynodeObj.Namespace, "", nodeResource, "", admission.Create, mynode),
 			err:        "",
 		},
+		{
+			name:       "allow create of my node with taints",
+			podsGetter: noExistingPods,
+			attributes: admission.NewAttributesRecord(mynodeObjTaintA, nil, nodeKind, mynodeObj.Namespace, "", nodeResource, "", admission.Create, mynode),
+			err:        "",
+		},
 		{
 			name:       "allow update of my node",
 			podsGetter: existingPods,
@@ -681,6 +689,30 @@ func Test_nodePlugin_Admit(t *testing.T) {
 			attributes: admission.NewAttributesRecord(mynodeObj, mynodeObjConfigA, nodeKind, mynodeObj.Namespace, mynodeObj.Name, nodeResource, "", admission.Update, mynode),
 			err:        "",
 		},
+		{
+			name:       "allow update of my node: no change to taints",
+			podsGetter: existingPods,
+			attributes: admission.NewAttributesRecord(mynodeObjTaintA, mynodeObjTaintA, nodeKind, mynodeObj.Namespace, mynodeObj.Name, nodeResource, "", admission.Update, mynode),
+			err:        "",
+		},
+		{
+			name:       "forbid update of my node: add taints",
+			podsGetter: existingPods,
+			attributes: admission.NewAttributesRecord(mynodeObjTaintA, mynodeObj, nodeKind, mynodeObj.Namespace, mynodeObj.Name, nodeResource, "", admission.Update, mynode),
+			err:        "cannot modify taints",
+		},
+		{
+			name:       "forbid update of my node: remove taints",
+			podsGetter: existingPods,
+			attributes: admission.NewAttributesRecord(mynodeObj, mynodeObjTaintA, nodeKind, mynodeObj.Namespace, mynodeObj.Name, nodeResource, "", admission.Update, mynode),
+			err:        "cannot modify taints",
+		},
+		{
+			name:       "forbid update of my node: change taints",
+			podsGetter: existingPods,
+			attributes: admission.NewAttributesRecord(mynodeObjTaintA, mynodeObjTaintB, nodeKind, mynodeObj.Namespace, mynodeObj.Name, nodeResource, "", admission.Update, mynode),
+			err:        "cannot modify taints",
+		},
 
 		// Other node object
 		{