Merge pull request #84260 from tallclair/status-restrict

Forbid label updates by nodes through pod/status

plugin/pkg/admission/noderestriction/BUILD

@@ -24,6 +24,7 @@ go_library(
         "//staging/src/k8s.io/apimachinery/pkg/api/equality:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/meta:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library",

plugin/pkg/admission/noderestriction/admission.go

@@ -22,10 +22,11 @@ import (
 	"io"
 	"strings"
 
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
+	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/util/diff"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apiserver/pkg/admission"
@@ -235,14 +236,21 @@ func (p *Plugin) admitPodStatus(nodeName string, a admission.Attributes) error {
 	switch a.GetOperation() {
 	case admission.Update:
 		// require an existing pod
-		pod, ok := a.GetOldObject().(*api.Pod)
+		oldPod, ok := a.GetOldObject().(*api.Pod)
 		if !ok {
 			return admission.NewForbidden(a, fmt.Errorf("unexpected type %T", a.GetOldObject()))
 		}
 		// only allow a node to update status of a pod bound to itself
-		if pod.Spec.NodeName != nodeName {
+		if oldPod.Spec.NodeName != nodeName {
 			return admission.NewForbidden(a, fmt.Errorf("node %q can only update pod status for pods with spec.nodeName set to itself", nodeName))
 		}
+		newPod, ok := a.GetObject().(*api.Pod)
+		if !ok {
+			return admission.NewForbidden(a, fmt.Errorf("unexpected type %T", a.GetObject()))
+		}
+		if !labels.Equals(oldPod.Labels, newPod.Labels) {
+			return admission.NewForbidden(a, fmt.Errorf("node %q cannot update labels through pod status", nodeName))
+		}
 		return nil
 
 	default:

plugin/pkg/admission/noderestriction/admission_test.go

@@ -93,6 +93,20 @@ func makeTestPod(namespace, name, node string, mirror bool) (*api.Pod, *corev1.P
 	return corePod, v1Pod
 }
 
+func withLabels(pod *api.Pod, labels map[string]string) *api.Pod {
+	labeledPod := pod.DeepCopy()
+	if labels == nil {
+		labeledPod.Labels = nil
+		return labeledPod
+	}
+	// Clone.
+	labeledPod.Labels = map[string]string{}
+	for key, value := range labels {
+		labeledPod.Labels[key] = value
+	}
+	return labeledPod
+}
+
 func makeTestPodEviction(name string) *policy.Eviction {
 	eviction := &policy.Eviction{}
 	eviction.Name = name
@@ -337,6 +351,16 @@ func Test_nodePlugin_Admit(t *testing.T) {
 
 		existingPodsIndex = cache.NewIndexer(cache.MetaNamespaceKeyFunc, nil)
 		existingPods      = corev1lister.NewPodLister(existingPodsIndex)
+
+		labelsA = map[string]string{
+			"label-a": "value-a",
+		}
+		labelsAB = map[string]string{
+			"label-a": "value-a",
+			"label-b": "value-b",
+		}
+		aLabeledPod  = withLabels(coremypod, labelsA)
+		abLabeledPod = withLabels(coremypod, labelsAB)
 	)
 
 	existingPodsIndex.Add(v1mymirrorpod)
@@ -588,6 +612,30 @@ func Test_nodePlugin_Admit(t *testing.T) {
 			attributes: admission.NewAttributesRecord(nil, nil, podKind, coremypod.Namespace, coremypod.Name, podResource, "status", admission.Delete, &metav1.DeleteOptions{}, false, mynode),
 			err:        "forbidden: unexpected operation",
 		},
+		{
+			name:       "forbid addition of pod status preexisting labels",
+			podsGetter: noExistingPods,
+			attributes: admission.NewAttributesRecord(abLabeledPod, aLabeledPod, podKind, coremypod.Namespace, coremypod.Name, podResource, "status", admission.Update, &metav1.UpdateOptions{}, false, mynode),
+			err:        "cannot update labels through pod status",
+		},
+		{
+			name:       "forbid deletion of pod status preexisting labels",
+			podsGetter: noExistingPods,
+			attributes: admission.NewAttributesRecord(aLabeledPod, abLabeledPod, podKind, coremypod.Namespace, coremypod.Name, podResource, "status", admission.Update, &metav1.UpdateOptions{}, false, mynode),
+			err:        "cannot update labels through pod status",
+		},
+		{
+			name:       "forbid deletion of all pod status preexisting labels",
+			podsGetter: noExistingPods,
+			attributes: admission.NewAttributesRecord(aLabeledPod, coremypod, podKind, coremypod.Namespace, coremypod.Name, podResource, "status", admission.Update, &metav1.UpdateOptions{}, false, mynode),
+			err:        "cannot update labels through pod status",
+		},
+		{
+			name:       "forbid addition of pod status labels",
+			podsGetter: noExistingPods,
+			attributes: admission.NewAttributesRecord(coremypod, aLabeledPod, podKind, coremypod.Namespace, coremypod.Name, podResource, "status", admission.Update, &metav1.UpdateOptions{}, false, mynode),
+			err:        "cannot update labels through pod status",
+		},
 		{
 			name:       "forbid update of eviction for normal pod bound to self",
 			podsGetter: existingPods,