From b902f857c6bbd4e50d0d257915bd0f268bd74ba7 Mon Sep 17 00:00:00 2001 From: Avesh Agarwal Date: Fri, 30 Sep 2016 16:58:01 -0400 Subject: [PATCH 1/2] Add support for admission controller based on namespace node selectors. This work is to upstream openshift's project node selectors based admission controller. Addresses #17151. --- cmd/kube-apiserver/app/plugins.go | 1 + pkg/labels/labels.go | 101 ++++++++ .../admission/podnodeselector/admission.go | 216 ++++++++++++++++++ 3 files changed, 318 insertions(+) create mode 100644 plugin/pkg/admission/podnodeselector/admission.go diff --git a/cmd/kube-apiserver/app/plugins.go b/cmd/kube-apiserver/app/plugins.go index c685428fbee..a2888393c11 100644 --- a/cmd/kube-apiserver/app/plugins.go +++ b/cmd/kube-apiserver/app/plugins.go @@ -36,6 +36,7 @@ import ( _ "k8s.io/kubernetes/plugin/pkg/admission/namespace/exists" _ "k8s.io/kubernetes/plugin/pkg/admission/namespace/lifecycle" _ "k8s.io/kubernetes/plugin/pkg/admission/persistentvolume/label" + _ "k8s.io/kubernetes/plugin/pkg/admission/podnodeselector" _ "k8s.io/kubernetes/plugin/pkg/admission/resourcequota" _ "k8s.io/kubernetes/plugin/pkg/admission/security/podsecuritypolicy" _ "k8s.io/kubernetes/plugin/pkg/admission/securitycontext/scdeny" diff --git a/pkg/labels/labels.go b/pkg/labels/labels.go index 822b137a9e7..0d0caa77dcb 100644 --- a/pkg/labels/labels.go +++ b/pkg/labels/labels.go @@ -17,6 +17,7 @@ limitations under the License. package labels import ( + "fmt" "sort" "strings" ) @@ -78,3 +79,103 @@ func FormatLabels(labelMap map[string]string) string { } return l } + +// Conflicts takes 2 maps and returns true if there a key match between +// the maps but the value doesn't match, and returns false in other cases +func Conflicts(labels1, labels2 Set) bool { + small := labels1 + big := labels2 + if len(labels2) < len(labels1) { + small = labels2 + big = labels1 + } + + for k, v := range small { + if val, match := big[k]; match { + if val != v { + return true + } + } + } + + return false +} + +// Merge combines given maps, and does not check for any conflicts +// between the maps. In case of conflicts, second map (labels2) wins +func Merge(labels1, labels2 Set) Set { + mergedMap := Set{} + + for k, v := range labels1 { + mergedMap[k] = v + } + for k, v := range labels2 { + mergedMap[k] = v + } + return mergedMap +} + +// Equals returns true if the given maps are equal +func Equals(labels1, labels2 Set) bool { + if len(labels1) != len(labels2) { + return false + } + + for k, v := range labels1 { + value, ok := labels2[k] + if !ok { + return false + } + if value != v { + return false + } + } + return true +} + +// AreLabelsInWhiteList verifies if the provided label list +// is in the provided whitelist and returns true, otherwise false. +func AreLabelsInWhiteList(labels, whitelist Set) bool { + if len(whitelist) == 0 { + return true + } + + for k, v := range labels { + value, ok := whitelist[k] + if !ok { + return false + } + if value != v { + return false + } + } + return true +} + +// ConvertSelectorToLabelsMap converts selector string to labels map +// and validates keys and values +func ConvertSelectorToLabelsMap(selector string) (Set, error) { + labelsMap := Set{} + + if len(selector) == 0 { + return labelsMap, nil + } + + labels := strings.Split(selector, ",") + for _, label := range labels { + l := strings.Split(label, "=") + if len(l) != 2 { + return labelsMap, fmt.Errorf("invalid selector: %s", l) + } + key := strings.TrimSpace(l[0]) + if err := validateLabelKey(key); err != nil { + return labelsMap, err + } + value := strings.TrimSpace(l[1]) + if err := validateLabelValue(value); err != nil { + return labelsMap, err + } + labelsMap[key] = value + } + return labelsMap, nil +} diff --git a/plugin/pkg/admission/podnodeselector/admission.go b/plugin/pkg/admission/podnodeselector/admission.go new file mode 100644 index 00000000000..0201d8f28d4 --- /dev/null +++ b/plugin/pkg/admission/podnodeselector/admission.go @@ -0,0 +1,216 @@ +/* +Copyright 2016 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package podnodeselector + +import ( + "fmt" + "io" + "reflect" + + "github.com/golang/glog" + + "k8s.io/kubernetes/pkg/admission" + "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/api/errors" + "k8s.io/kubernetes/pkg/client/cache" + clientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset" + "k8s.io/kubernetes/pkg/controller/informers" + "k8s.io/kubernetes/pkg/labels" + "k8s.io/kubernetes/pkg/util/yaml" +) + +// The annotation key scheduler.alpha.kubernetes.io/node-selector is for assigning +// node selectors labels to namespaces +var NamespaceNodeSelectors = []string{"scheduler.alpha.kubernetes.io/node-selector"} + +func init() { + admission.RegisterPlugin("PodNodeSelector", func(client clientset.Interface, config io.Reader) (admission.Interface, error) { + pluginConfig := readConfig(config) + plugin := NewPodNodeSelector(client, pluginConfig.PodNodeSelectorPluginConfig) + return plugin, nil + }) +} + +// podNodeSelector is an implementation of admission.Interface. +type podNodeSelector struct { + *admission.Handler + client clientset.Interface + namespaceInformer cache.SharedIndexInformer + // global default node selector and namespace whitelists in a cluster. + clusterNodeSelectors map[string]string +} + +type pluginConfig struct { + PodNodeSelectorPluginConfig map[string]string +} + +// readConfig reads default value of clusterDefaultNodeSelector +// from the file provided with --admission-control-config-file +// If the file is not supplied, it defaults to "" +// The format in a file: +// podNodeSelectorPluginConfig: +// clusterDefaultNodeSelector: +// namespace1: +// namespace2: +func readConfig(config io.Reader) *pluginConfig { + defaultConfig := &pluginConfig{} + if config == nil || reflect.ValueOf(config).IsNil() { + return defaultConfig + } + d := yaml.NewYAMLOrJSONDecoder(config, 4096) + for { + if err := d.Decode(defaultConfig); err != nil { + if err != io.EOF { + continue + } + } + break + } + return defaultConfig +} + +// Admit enforces that pod and its namespace node label selectors matches at least a node in the cluster. +func (p *podNodeSelector) Admit(a admission.Attributes) error { + resource := a.GetResource().GroupResource() + if resource != api.Resource("pods") { + return nil + } + if a.GetSubresource() != "" { + // only run the checks below on pods proper and not subresources + return nil + } + + obj := a.GetObject() + pod, ok := obj.(*api.Pod) + if !ok { + glog.Errorf("expected pod but got %s", a.GetKind().Kind) + return nil + } + + if !p.WaitForReady() { + return admission.NewForbidden(a, fmt.Errorf("not yet ready to handle request")) + } + + name := pod.Name + nsName := a.GetNamespace() + var namespace *api.Namespace + + namespaceObj, exists, err := p.namespaceInformer.GetStore().Get(&api.Namespace{ + ObjectMeta: api.ObjectMeta{ + Name: nsName, + Namespace: "", + }, + }) + if err != nil { + return errors.NewInternalError(err) + } + if exists { + namespace = namespaceObj.(*api.Namespace) + } else { + namespace, err = p.defaultGetNamespace(nsName) + if err != nil { + if errors.IsNotFound(err) { + return err + } + return errors.NewInternalError(err) + } + } + namespaceNodeSelector, err := p.getNodeSelectorMap(namespace) + if err != nil { + return err + } + + if labels.Conflicts(namespaceNodeSelector, labels.Set(pod.Spec.NodeSelector)) { + return errors.NewForbidden(resource, name, fmt.Errorf("pod node label selector conflicts with its namespace node label selector")) + } + + whitelist, err := labels.ConvertSelectorToLabelsMap(p.clusterNodeSelectors[namespace.Name]) + if err != nil { + return err + } + + // Merge pod node selector = namespace node selector + current pod node selector + podNodeSelectorLabels := labels.Merge(namespaceNodeSelector, pod.Spec.NodeSelector) + + // whitelist verification + if !labels.AreLabelsInWhiteList(podNodeSelectorLabels, whitelist) { + return errors.NewForbidden(resource, name, fmt.Errorf("pod node label selector labels conflict with its namespace whitelist")) + } + + // Updated pod node selector = namespace node selector + current pod node selector + pod.Spec.NodeSelector = map[string]string(podNodeSelectorLabels) + return nil +} + +func NewPodNodeSelector(client clientset.Interface, clusterNodeSelectors map[string]string) *podNodeSelector { + return &podNodeSelector{ + Handler: admission.NewHandler(admission.Create), + client: client, + clusterNodeSelectors: clusterNodeSelectors, + } +} + +func (p *podNodeSelector) SetInformerFactory(f informers.SharedInformerFactory) { + p.namespaceInformer = f.Namespaces().Informer() + p.SetReadyFunc(p.namespaceInformer.HasSynced) +} + +func (p *podNodeSelector) Validate() error { + if p.namespaceInformer == nil { + return fmt.Errorf("missing namespaceInformer") + } + return nil +} + +func (p *podNodeSelector) defaultGetNamespace(name string) (*api.Namespace, error) { + namespace, err := p.client.Core().Namespaces().Get(name) + if err != nil { + return nil, fmt.Errorf("namespace %s does not exist", name) + } + return namespace, nil +} + +func (p *podNodeSelector) getNodeSelectorMap(namespace *api.Namespace) (labels.Set, error) { + selector := labels.Set{} + labelsMap := labels.Set{} + var err error + found := false + if len(namespace.ObjectMeta.Annotations) > 0 { + for _, annotation := range NamespaceNodeSelectors { + if ns, ok := namespace.ObjectMeta.Annotations[annotation]; ok { + labelsMap, err = labels.ConvertSelectorToLabelsMap(ns) + if err != nil { + return labels.Set{}, err + } + + if labels.Conflicts(selector, labelsMap) { + nsName := namespace.ObjectMeta.Name + return labels.Set{}, fmt.Errorf("%s annotations' node label selectors conflict", nsName) + } + selector = labels.Merge(selector, labelsMap) + found = true + } + } + } + if !found { + selector, err = labels.ConvertSelectorToLabelsMap(p.clusterNodeSelectors["clusterDefaultNodeSelector"]) + if err != nil { + return labels.Set{}, err + } + } + return selector, nil +} From 5bb2cb82490c970260ec75fe4f5395574552b305 Mon Sep 17 00:00:00 2001 From: Avesh Agarwal Date: Fri, 30 Sep 2016 16:58:38 -0400 Subject: [PATCH 2/2] Added new unit tests. --- pkg/labels/labels_test.go | 171 ++++++++++++++++ .../podnodeselector/admission_test.go | 188 ++++++++++++++++++ 2 files changed, 359 insertions(+) create mode 100644 plugin/pkg/admission/podnodeselector/admission_test.go diff --git a/pkg/labels/labels_test.go b/pkg/labels/labels_test.go index a9f19309880..2d4d761bc2b 100644 --- a/pkg/labels/labels_test.go +++ b/pkg/labels/labels_test.go @@ -58,3 +58,174 @@ func TestLabelGet(t *testing.T) { t.Errorf("Set.Get is broken") } } + +func TestLabelConflict(t *testing.T) { + tests := []struct { + labels1 map[string]string + labels2 map[string]string + conflict bool + }{ + { + labels1: map[string]string{}, + labels2: map[string]string{}, + conflict: false, + }, + { + labels1: map[string]string{"env": "test"}, + labels2: map[string]string{"infra": "true"}, + conflict: false, + }, + { + labels1: map[string]string{"env": "test"}, + labels2: map[string]string{"infra": "true", "env": "test"}, + conflict: false, + }, + { + labels1: map[string]string{"env": "test"}, + labels2: map[string]string{"env": "dev"}, + conflict: true, + }, + { + labels1: map[string]string{"env": "test", "infra": "false"}, + labels2: map[string]string{"infra": "true", "color": "blue"}, + conflict: true, + }, + } + for _, test := range tests { + conflict := Conflicts(Set(test.labels1), Set(test.labels2)) + if conflict != test.conflict { + t.Errorf("expected: %v but got: %v", test.conflict, conflict) + } + } +} + +func TestLabelMerge(t *testing.T) { + tests := []struct { + labels1 map[string]string + labels2 map[string]string + mergedLabels map[string]string + }{ + { + labels1: map[string]string{}, + labels2: map[string]string{}, + mergedLabels: map[string]string{}, + }, + { + labels1: map[string]string{"infra": "true"}, + labels2: map[string]string{}, + mergedLabels: map[string]string{"infra": "true"}, + }, + { + labels1: map[string]string{"infra": "true"}, + labels2: map[string]string{"env": "test", "color": "blue"}, + mergedLabels: map[string]string{"infra": "true", "env": "test", "color": "blue"}, + }, + } + for _, test := range tests { + mergedLabels := Merge(Set(test.labels1), Set(test.labels2)) + if !Equals(mergedLabels, test.mergedLabels) { + t.Errorf("expected: %v but got: %v", test.mergedLabels, mergedLabels) + } + } +} + +func TestLabelSelectorParse(t *testing.T) { + tests := []struct { + selector string + labels map[string]string + valid bool + }{ + { + selector: "", + labels: map[string]string{}, + valid: true, + }, + { + selector: "x=a", + labels: map[string]string{"x": "a"}, + valid: true, + }, + { + selector: "x=a,y=b,z=c", + labels: map[string]string{"x": "a", "y": "b", "z": "c"}, + valid: true, + }, + { + selector: " x = a , y = b , z = c ", + labels: map[string]string{"x": "a", "y": "b", "z": "c"}, + valid: true, + }, + { + selector: "color=green,env=test,service=front", + labels: map[string]string{"color": "green", "env": "test", "service": "front"}, + valid: true, + }, + { + selector: "color=green, env=test, service=front", + labels: map[string]string{"color": "green", "env": "test", "service": "front"}, + valid: true, + }, + { + selector: ",", + labels: map[string]string{}, + valid: false, + }, + { + selector: "x", + labels: map[string]string{}, + valid: false, + }, + { + selector: "x,y", + labels: map[string]string{}, + valid: false, + }, + { + selector: "x=$y", + labels: map[string]string{}, + valid: false, + }, + { + selector: "x!=y", + labels: map[string]string{}, + valid: false, + }, + { + selector: "x==y", + labels: map[string]string{}, + valid: false, + }, + { + selector: "x=a||y=b", + labels: map[string]string{}, + valid: false, + }, + { + selector: "x in (y)", + labels: map[string]string{}, + valid: false, + }, + { + selector: "x notin (y)", + labels: map[string]string{}, + valid: false, + }, + { + selector: "x y", + labels: map[string]string{}, + valid: false, + }, + } + for _, test := range tests { + labels, err := ConvertSelectorToLabelsMap(test.selector) + if test.valid && err != nil { + t.Errorf("selector: %s, expected no error but got: %s", test.selector, err) + } else if !test.valid && err == nil { + t.Errorf("selector: %s, expected an error", test.selector) + } + + if !Equals(Set(labels), test.labels) { + t.Errorf("expected: %s but got: %s", test.labels, labels) + } + } +} diff --git a/plugin/pkg/admission/podnodeselector/admission_test.go b/plugin/pkg/admission/podnodeselector/admission_test.go new file mode 100644 index 00000000000..dee7fd39533 --- /dev/null +++ b/plugin/pkg/admission/podnodeselector/admission_test.go @@ -0,0 +1,188 @@ +/* +Copyright 2016 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package podnodeselector + +import ( + "testing" + "time" + + "k8s.io/kubernetes/pkg/admission" + "k8s.io/kubernetes/pkg/api" + clientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset" + "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake" + "k8s.io/kubernetes/pkg/controller/informers" + "k8s.io/kubernetes/pkg/labels" + "k8s.io/kubernetes/pkg/util/wait" +) + +// TestPodAdmission verifies various scenarios involving pod/namespace/global node label selectors +func TestPodAdmission(t *testing.T) { + namespace := &api.Namespace{ + ObjectMeta: api.ObjectMeta{ + Name: "testNamespace", + Namespace: "", + }, + } + + mockClient := &fake.Clientset{} + handler, informerFactory, err := newHandlerForTest(mockClient) + if err != nil { + t.Errorf("unexpected error initializing handler: %v", err) + } + informerFactory.Start(wait.NeverStop) + + pod := &api.Pod{ + ObjectMeta: api.ObjectMeta{Name: "testPod", Namespace: "testNamespace"}, + } + + tests := []struct { + defaultNodeSelector string + namespaceNodeSelector string + whitelist string + podNodeSelector map[string]string + mergedNodeSelector labels.Set + ignoreTestNamespaceNodeSelector bool + admit bool + testName string + }{ + { + defaultNodeSelector: "", + podNodeSelector: map[string]string{}, + mergedNodeSelector: labels.Set{}, + ignoreTestNamespaceNodeSelector: true, + admit: true, + testName: "No node selectors", + }, + { + defaultNodeSelector: "infra = false", + podNodeSelector: map[string]string{}, + mergedNodeSelector: labels.Set{"infra": "false"}, + ignoreTestNamespaceNodeSelector: true, + admit: true, + testName: "Default node selector and no conflicts", + }, + { + defaultNodeSelector: "", + namespaceNodeSelector: " infra = false ", + podNodeSelector: map[string]string{}, + mergedNodeSelector: labels.Set{"infra": "false"}, + admit: true, + testName: "TestNamespace node selector with whitespaces and no conflicts", + }, + { + defaultNodeSelector: "infra = false", + namespaceNodeSelector: "infra=true", + podNodeSelector: map[string]string{}, + mergedNodeSelector: labels.Set{"infra": "true"}, + admit: true, + testName: "Default and namespace node selector, no conflicts", + }, + { + defaultNodeSelector: "infra = false", + namespaceNodeSelector: "", + podNodeSelector: map[string]string{}, + mergedNodeSelector: labels.Set{}, + admit: true, + testName: "Empty namespace node selector and no conflicts", + }, + { + defaultNodeSelector: "infra = false", + namespaceNodeSelector: "infra=true", + podNodeSelector: map[string]string{"env": "test"}, + mergedNodeSelector: labels.Set{"infra": "true", "env": "test"}, + admit: true, + testName: "TestNamespace and pod node selector, no conflicts", + }, + { + defaultNodeSelector: "env = test", + namespaceNodeSelector: "infra=true", + podNodeSelector: map[string]string{"infra": "false"}, + admit: false, + testName: "Conflicting pod and namespace node selector, one label", + }, + { + defaultNodeSelector: "env=dev", + namespaceNodeSelector: "infra=false, env = test", + podNodeSelector: map[string]string{"env": "dev", "color": "blue"}, + admit: false, + testName: "Conflicting pod and namespace node selector, multiple labels", + }, + { + defaultNodeSelector: "env=dev", + namespaceNodeSelector: "infra=false, env = dev", + whitelist: "env=dev, infra=false, color=blue", + podNodeSelector: map[string]string{"env": "dev", "color": "blue"}, + mergedNodeSelector: labels.Set{"infra": "false", "env": "dev", "color": "blue"}, + admit: true, + testName: "Merged pod node selectors satisfy the whitelist", + }, + { + defaultNodeSelector: "env=dev", + namespaceNodeSelector: "infra=false, env = dev", + whitelist: "env=dev, infra=true, color=blue", + podNodeSelector: map[string]string{"env": "dev", "color": "blue"}, + admit: false, + testName: "Merged pod node selectors conflict with the whitelist", + }, + } + for _, test := range tests { + if !test.ignoreTestNamespaceNodeSelector { + namespace.ObjectMeta.Annotations = map[string]string{"scheduler.alpha.kubernetes.io/node-selector": test.namespaceNodeSelector} + handler.namespaceInformer.GetStore().Update(namespace) + } + handler.clusterNodeSelectors = make(map[string]string) + handler.clusterNodeSelectors["clusterDefaultNodeSelector"] = test.defaultNodeSelector + handler.clusterNodeSelectors[namespace.Name] = test.whitelist + pod.Spec = api.PodSpec{NodeSelector: test.podNodeSelector} + + err := handler.Admit(admission.NewAttributesRecord(pod, nil, api.Kind("Pod").WithVersion("version"), "testNamespace", namespace.ObjectMeta.Name, api.Resource("pods").WithVersion("version"), "", admission.Create, nil)) + if test.admit && err != nil { + t.Errorf("Test: %s, expected no error but got: %s", test.testName, err) + } else if !test.admit && err == nil { + t.Errorf("Test: %s, expected an error", test.testName) + } + + if test.admit && !labels.Equals(test.mergedNodeSelector, labels.Set(pod.Spec.NodeSelector)) { + t.Errorf("Test: %s, expected: %s but got: %s", test.testName, test.mergedNodeSelector, pod.Spec.NodeSelector) + } + } +} + +func TestHandles(t *testing.T) { + for op, shouldHandle := range map[admission.Operation]bool{ + admission.Create: true, + admission.Update: false, + admission.Connect: false, + admission.Delete: false, + } { + nodeEnvionment := NewPodNodeSelector(nil, nil) + if e, a := shouldHandle, nodeEnvionment.Handles(op); e != a { + t.Errorf("%v: shouldHandle=%t, handles=%t", op, e, a) + } + } +} + +// newHandlerForTest returns the admission controller configured for testing. +func newHandlerForTest(c clientset.Interface) (*podNodeSelector, informers.SharedInformerFactory, error) { + f := informers.NewSharedInformerFactory(c, 5*time.Minute) + handler := NewPodNodeSelector(c, nil) + plugins := []admission.Interface{handler} + pluginInitializer := admission.NewPluginInitializer(f, nil) + pluginInitializer.Initialize(plugins) + err := admission.Validate(plugins) + return handler, f, err +}