UPSTREAM: <carry>: openshift-kube-apiserver: add kube-apiserver patches

pod .spec.nodeName should not override project node selector

openshift-kube-apiserver/admission/scheduler/nodeenv/admission.go

@@ -9,6 +9,7 @@ import (
 
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apiserver/pkg/admission"
 	"k8s.io/apiserver/pkg/admission/initializer"
 	"k8s.io/client-go/informers"
@@ -34,8 +35,10 @@ const (
 // podNodeEnvironment is an implementation of admission.MutationInterface.
 type podNodeEnvironment struct {
 	*admission.Handler
-	nsLister       corev1listers.NamespaceLister
-	nsListerSynced func() bool
+	nsLister         corev1listers.NamespaceLister
+	nsListerSynced   func() bool
+	nodeLister       corev1listers.NodeLister
+	nodeListerSynced func() bool
 	// TODO this should become a piece of config passed to the admission plugin
 	defaultNodeSelector string
 }
@@ -72,7 +75,7 @@ func (p *podNodeEnvironment) admit(ctx context.Context, a admission.Attributes,
 		return apierrors.NewForbidden(resource, name, err)
 	}
 
-	// If scheduler.alpha.kubernetes.io/node-selector is set on the pod,
+	// If scheduler.alpha.kubernetes.io/node-selector is set on the namespace,
 	// do not process the pod further.
 	if _, ok := namespace.ObjectMeta.Annotations[kubeProjectNodeSelector]; ok {
 		return nil
@@ -96,6 +99,16 @@ func (p *podNodeEnvironment) admit(ctx context.Context, a admission.Attributes,
 		return apierrors.NewForbidden(resource, name, fmt.Errorf("pod node label selector does not extend project node label selector"))
 	}
 
+	if len(pod.Spec.NodeName) > 0 && len(projectNodeSelector) > 0 {
+		node, err := p.nodeLister.Get(pod.Spec.NodeName)
+		if err != nil {
+			return apierrors.NewForbidden(resource, name, fmt.Errorf("cannot validate project node label selector: %v", err))
+		}
+		if !isSubset(projectNodeSelector, node.Labels) {
+			return apierrors.NewForbidden(resource, name, fmt.Errorf("pod node name conflicts with project node label selector"))
+		}
+	}
+
 	// modify pod node selector = project node selector + current pod node selector
 	pod.Spec.NodeSelector = labelselector.Merge(projectNodeSelector, pod.Spec.NodeSelector)
 
@@ -117,14 +130,16 @@ func (p *podNodeEnvironment) SetDefaultNodeSelector(in string) {
 func (p *podNodeEnvironment) SetExternalKubeInformerFactory(kubeInformers informers.SharedInformerFactory) {
 	p.nsLister = kubeInformers.Core().V1().Namespaces().Lister()
 	p.nsListerSynced = kubeInformers.Core().V1().Namespaces().Informer().HasSynced
+	p.nodeLister = kubeInformers.Core().V1().Nodes().Lister()
+	p.nodeListerSynced = kubeInformers.Core().V1().Nodes().Informer().HasSynced
 }
 
 func (p *podNodeEnvironment) waitForSyncedStore(timeout <-chan time.Time) bool {
-	for !p.nsListerSynced() {
+	for !p.nsListerSynced() || !p.nodeListerSynced() {
 		select {
 		case <-time.After(100 * time.Millisecond):
 		case <-timeout:
-			return p.nsListerSynced()
+			return p.nsListerSynced() && p.nodeListerSynced()
 		}
 	}
 
@@ -138,6 +153,12 @@ func (p *podNodeEnvironment) ValidateInitialization() error {
 	if p.nsListerSynced == nil {
 		return fmt.Errorf("project node environment plugin needs a namespace lister synced")
 	}
+	if p.nodeLister == nil {
+		return fmt.Errorf("project node environment plugin needs a node lister")
+	}
+	if p.nodeListerSynced == nil {
+		return fmt.Errorf("project node environment plugin needs a node lister synced")
+	}
 	return nil
 }
 
@@ -146,3 +167,20 @@ func NewPodNodeEnvironment() (admission.Interface, error) {
 		Handler: admission.NewHandler(admission.Create),
 	}, nil
 }
+
+func isSubset(subSet, superSet labels.Set) bool {
+	if len(superSet) == 0 {
+		return true
+	}
+
+	for k, v := range subSet {
+		value, ok := superSet[k]
+		if !ok {
+			return false
+		}
+		if value != v {
+			return false
+		}
+	}
+	return true
+}

openshift-kube-apiserver/admission/scheduler/nodeenv/admission_test.go

@@ -25,6 +25,16 @@ func TestPodAdmission(t *testing.T) {
 		},
 	}
 
+	node := &corev1.Node{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "worker-1",
+			Namespace: "",
+			Labels: map[string]string{
+				"worker": "true",
+			},
+		},
+	}
+
 	handler := &podNodeEnvironment{}
 	pod := &kapi.Pod{
 		ObjectMeta: metav1.ObjectMeta{Name: "testPod"},
@@ -34,6 +44,7 @@ func TestPodAdmission(t *testing.T) {
 		defaultNodeSelector       string
 		projectNodeSelector       string
 		podNodeSelector           map[string]string
+		podNodeName               string
 		mergedNodeSelector        map[string]string
 		ignoreProjectNodeSelector bool
 		admit                     bool
@@ -103,18 +114,58 @@ func TestPodAdmission(t *testing.T) {
 			admit:               false,
 			testName:            "Conflicting pod and project node selector, multiple labels",
 		},
+		{
+			defaultNodeSelector: "",
+			projectNodeSelector: "worker=true",
+			podNodeName:         "worker-1",
+			podNodeSelector:     nil,
+			mergedNodeSelector:  map[string]string{"worker": "true"},
+			admit:               true,
+			testName:            "node referenced in pod.nodeName does not conflict with project node selector",
+		},
+		{
+			defaultNodeSelector: "",
+			projectNodeSelector: "",
+			podNodeName:         "worker-1",
+			podNodeSelector:     map[string]string{"worker": "false"},
+			mergedNodeSelector:  map[string]string{"worker": "false"},
+			admit:               true,
+			// default to kube behavior: let this fail by kubelet
+			testName: "node referenced in pod spec.nodeName can conflict with its own node selector when no project node selector is specified",
+		},
+		{
+			defaultNodeSelector: "worker = true",
+			projectNodeSelector: "worker=false",
+			podNodeName:         "worker-1",
+			podNodeSelector:     nil,
+			mergedNodeSelector:  nil,
+			admit:               false,
+			testName:            "node referenced in pod spec.nodeName conflicts with project node selector",
+		},
+		{
+			defaultNodeSelector: "",
+			projectNodeSelector: "worker=true",
+			podNodeName:         "worker-2",
+			podNodeSelector:     nil,
+			mergedNodeSelector:  nil,
+			admit:               false,
+			testName:            "missing node referenced in pod spec.nodeName does not admit",
+		},
 	}
 	for _, test := range tests {
 		indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{})
 		indexer.Add(namespace)
+		indexer.Add(node)
 		handler.nsLister = corev1listers.NewNamespaceLister(indexer)
 		handler.nsListerSynced = func() bool { return true }
+		handler.nodeLister = corev1listers.NewNodeLister(indexer)
+		handler.nodeListerSynced = func() bool { return true }
 		handler.defaultNodeSelector = test.defaultNodeSelector
 
 		if !test.ignoreProjectNodeSelector {
 			namespace.ObjectMeta.Annotations = map[string]string{projectv1.ProjectNodeSelector: test.projectNodeSelector}
 		}
-		pod.Spec = kapi.PodSpec{NodeSelector: test.podNodeSelector}
+		pod.Spec = kapi.PodSpec{NodeSelector: test.podNodeSelector, NodeName: test.podNodeName}
 
 		attrs := admission.NewAttributesRecord(pod, nil, kapi.Kind("Pod").WithVersion("version"), "testProject", namespace.ObjectMeta.Name, kapi.Resource("pods").WithVersion("version"), "", admission.Create, nil, false, nil)
 		err := handler.Admit(context.TODO(), attrs, nil)