From 9fb2dcad5e2869b9342717d32e021a8374a82f4b Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Wed, 17 Oct 2018 12:42:59 -0400 Subject: [PATCH] Limit kubelets from updating their own labels --- cmd/kubelet/app/options/BUILD | 2 + cmd/kubelet/app/options/options.go | 36 ++- pkg/kubelet/apis/BUILD | 4 +- pkg/kubelet/apis/well_known_labels.go | 76 ++++++ plugin/pkg/admission/noderestriction/BUILD | 5 + .../admission/noderestriction/admission.go | 101 +++++++- .../noderestriction/admission_test.go | 242 +++++++++++++++++- 7 files changed, 460 insertions(+), 6 deletions(-) diff --git a/cmd/kubelet/app/options/BUILD b/cmd/kubelet/app/options/BUILD index 711883b11e..d6e0d70f4a 100644 --- a/cmd/kubelet/app/options/BUILD +++ b/cmd/kubelet/app/options/BUILD @@ -23,6 +23,7 @@ go_library( "//pkg/credentialprovider/azure:go_default_library", "//pkg/credentialprovider/gcp:go_default_library", "//pkg/features:go_default_library", + "//pkg/kubelet/apis:go_default_library", "//pkg/kubelet/apis/config:go_default_library", "//pkg/kubelet/apis/config/scheme:go_default_library", "//pkg/kubelet/apis/config/validation:go_default_library", @@ -33,6 +34,7 @@ go_library( "//pkg/util/taints:go_default_library", "//pkg/version/verflag:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", "//staging/src/k8s.io/apiserver/pkg/util/flag:go_default_library", "//staging/src/k8s.io/apiserver/pkg/util/logs:go_default_library", diff --git a/cmd/kubelet/app/options/options.go b/cmd/kubelet/app/options/options.go index ff8dccde72..47f9eda3db 100644 --- a/cmd/kubelet/app/options/options.go +++ b/cmd/kubelet/app/options/options.go @@ -27,11 +27,14 @@ import ( "github.com/spf13/pflag" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/apiserver/pkg/util/flag" + "k8s.io/klog" "k8s.io/kubelet/config/v1beta1" "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/features" + kubeletapis "k8s.io/kubernetes/pkg/kubelet/apis" kubeletconfig "k8s.io/kubernetes/pkg/kubelet/apis/config" kubeletscheme "k8s.io/kubernetes/pkg/kubelet/apis/config/scheme" kubeletconfigvalidation "k8s.io/kubernetes/pkg/kubelet/apis/config/validation" @@ -250,9 +253,40 @@ func ValidateKubeletFlags(f *KubeletFlags) error { if f.NodeStatusMaxImages < -1 { return fmt.Errorf("invalid configuration: NodeStatusMaxImages (--node-status-max-images) must be -1 or greater") } + + unknownLabels := sets.NewString() + for k := range f.NodeLabels { + if isKubernetesLabel(k) && !kubeletapis.IsKubeletLabel(k) { + unknownLabels.Insert(k) + } + } + if len(unknownLabels) > 0 { + // TODO(liggitt): in 1.15, return an error + klog.Warningf("unknown 'kubernetes.io' or 'k8s.io' labels specified with --node-labels: %v", unknownLabels.List()) + klog.Warningf("in 1.15, --node-labels in the 'kubernetes.io' namespace must begin with an allowed prefix (%s) or be in the specifically allowed set (%s)", strings.Join(kubeletapis.KubeletLabelNamespaces(), ", "), strings.Join(kubeletapis.KubeletLabels(), ", ")) + } + return nil } +func isKubernetesLabel(key string) bool { + namespace := getLabelNamespace(key) + if namespace == "kubernetes.io" || strings.HasSuffix(namespace, ".kubernetes.io") { + return true + } + if namespace == "k8s.io" || strings.HasSuffix(namespace, ".k8s.io") { + return true + } + return false +} + +func getLabelNamespace(key string) string { + if parts := strings.SplitN(key, "/", 2); len(parts) == 2 { + return parts[0] + } + return "" +} + // NewKubeletConfiguration will create a new KubeletConfiguration with default values func NewKubeletConfiguration() (*kubeletconfig.KubeletConfiguration, error) { scheme, _, err := kubeletscheme.NewSchemeAndCodecs() @@ -381,7 +415,7 @@ func (f *KubeletFlags) AddFlags(mainfs *pflag.FlagSet) { fs.BoolVar(&f.ExperimentalCheckNodeCapabilitiesBeforeMount, "experimental-check-node-capabilities-before-mount", f.ExperimentalCheckNodeCapabilitiesBeforeMount, "[Experimental] if set true, the kubelet will check the underlying node for required components (binaries, etc.) before performing the mount") fs.BoolVar(&f.ExperimentalNodeAllocatableIgnoreEvictionThreshold, "experimental-allocatable-ignore-eviction", f.ExperimentalNodeAllocatableIgnoreEvictionThreshold, "When set to 'true', Hard Eviction Thresholds will be ignored while calculating Node Allocatable. See https://kubernetes.io/docs/tasks/administer-cluster/reserve-compute-resources/ for more details. [default=false]") bindableNodeLabels := flag.ConfigurationMap(f.NodeLabels) - fs.Var(&bindableNodeLabels, "node-labels", " Labels to add when registering the node in the cluster. Labels must be key=value pairs separated by ','.") + fs.Var(&bindableNodeLabels, "node-labels", fmt.Sprintf(" Labels to add when registering the node in the cluster. Labels must be key=value pairs separated by ','. Labels in the 'kubernetes.io' namespace must begin with an allowed prefix (%s) or be in the specifically allowed set (%s)", strings.Join(kubeletapis.KubeletLabelNamespaces(), ", "), strings.Join(kubeletapis.KubeletLabels(), ", "))) fs.StringVar(&f.VolumePluginDir, "volume-plugin-dir", f.VolumePluginDir, "The full path of the directory in which to search for additional third party volume plugins") fs.StringVar(&f.LockFilePath, "lock-file", f.LockFilePath, " The path to file for kubelet to use as a lock file.") fs.BoolVar(&f.ExitOnLockContention, "exit-on-lock-contention", f.ExitOnLockContention, "Whether kubelet should exit upon lock-file contention.") diff --git a/pkg/kubelet/apis/BUILD b/pkg/kubelet/apis/BUILD index c4a2fb5087..f6c4fb222b 100644 --- a/pkg/kubelet/apis/BUILD +++ b/pkg/kubelet/apis/BUILD @@ -13,7 +13,9 @@ go_library( "well_known_labels.go", ], importpath = "k8s.io/kubernetes/pkg/kubelet/apis", - deps = select({ + deps = [ + "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", + ] + select({ "@io_bazel_rules_go//go/platform:windows": [ "//pkg/features:go_default_library", "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", diff --git a/pkg/kubelet/apis/well_known_labels.go b/pkg/kubelet/apis/well_known_labels.go index 5a0db552c8..869952556d 100644 --- a/pkg/kubelet/apis/well_known_labels.go +++ b/pkg/kubelet/apis/well_known_labels.go @@ -16,6 +16,12 @@ limitations under the License. package apis +import ( + "strings" + + "k8s.io/apimachinery/pkg/util/sets" +) + const ( LabelHostname = "kubernetes.io/hostname" LabelZoneFailureDomain = "failure-domain.beta.kubernetes.io/zone" @@ -26,8 +32,78 @@ const ( LabelOS = "beta.kubernetes.io/os" LabelArch = "beta.kubernetes.io/arch" + + // GA versions of the legacy beta labels. + // TODO: update kubelet and controllers to set both beta and GA labels, then export these constants + labelZoneFailureDomainGA = "failure-domain.kubernetes.io/zone" + labelZoneRegionGA = "failure-domain.kubernetes.io/region" + labelInstanceTypeGA = "kubernetes.io/instance-type" + labelOSGA = "kubernetes.io/os" + labelArchGA = "kubernetes.io/arch" + + // LabelNamespaceSuffixKubelet is an allowed label namespace suffix kubelets can self-set ([*.]kubelet.kubernetes.io/*) + LabelNamespaceSuffixKubelet = "kubelet.kubernetes.io" + // LabelNamespaceSuffixNode is an allowed label namespace suffix kubelets can self-set ([*.]node.kubernetes.io/*) + LabelNamespaceSuffixNode = "node.kubernetes.io" + + // LabelNamespaceNodeRestriction is a forbidden label namespace that kubelets may not self-set when the NodeRestriction admission plugin is enabled + LabelNamespaceNodeRestriction = "node-restriction.kubernetes.io" ) // When the --failure-domains scheduler flag is not specified, // DefaultFailureDomains defines the set of label keys used when TopologyKey is empty in PreferredDuringScheduling anti-affinity. var DefaultFailureDomains string = LabelHostname + "," + LabelZoneFailureDomain + "," + LabelZoneRegion + +var kubeletLabels = sets.NewString( + LabelHostname, + LabelZoneFailureDomain, + LabelZoneRegion, + LabelInstanceType, + LabelOS, + LabelArch, + + labelZoneFailureDomainGA, + labelZoneRegionGA, + labelInstanceTypeGA, + labelOSGA, + labelArchGA, +) + +var kubeletLabelNamespaces = sets.NewString( + LabelNamespaceSuffixKubelet, + LabelNamespaceSuffixNode, +) + +// KubeletLabels returns the list of label keys kubelets are allowed to set on their own Node objects +func KubeletLabels() []string { + return kubeletLabels.List() +} + +// KubeletLabelNamespaces returns the list of label key namespaces kubelets are allowed to set on their own Node objects +func KubeletLabelNamespaces() []string { + return kubeletLabelNamespaces.List() +} + +// IsKubeletLabel returns true if the label key is one that kubelets are allowed to set on their own Node object. +// This checks if the key is in the KubeletLabels() list, or has a namespace in the KubeletLabelNamespaces() list. +func IsKubeletLabel(key string) bool { + if kubeletLabels.Has(key) { + return true + } + + namespace := getLabelNamespace(key) + for allowedNamespace := range kubeletLabelNamespaces { + if namespace == allowedNamespace || strings.HasSuffix(namespace, "."+allowedNamespace) { + return true + } + } + + return false +} + +func getLabelNamespace(key string) string { + if parts := strings.SplitN(key, "/", 2); len(parts) == 2 { + return parts[0] + } + return "" +} diff --git a/plugin/pkg/admission/noderestriction/BUILD b/plugin/pkg/admission/noderestriction/BUILD index 9d6850b5b2..5c3bd1e93d 100644 --- a/plugin/pkg/admission/noderestriction/BUILD +++ b/plugin/pkg/admission/noderestriction/BUILD @@ -18,16 +18,19 @@ go_library( "//pkg/apis/policy:go_default_library", "//pkg/auth/nodeidentifier:go_default_library", "//pkg/features:go_default_library", + "//pkg/kubelet/apis:go_default_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/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", "//staging/src/k8s.io/apiserver/pkg/admission/initializer:go_default_library", "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", "//staging/src/k8s.io/client-go/informers:go_default_library", "//staging/src/k8s.io/client-go/listers/core/v1:go_default_library", "//staging/src/k8s.io/csi-api/pkg/apis/csi/v1alpha1:go_default_library", + "//vendor/k8s.io/klog:go_default_library", ], ) @@ -42,10 +45,12 @@ go_test( "//pkg/apis/policy:go_default_library", "//pkg/auth/nodeidentifier:go_default_library", "//pkg/features:go_default_library", + "//pkg/kubelet/apis:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library", "//staging/src/k8s.io/apiserver/pkg/authentication/user:go_default_library", "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", diff --git a/plugin/pkg/admission/noderestriction/admission.go b/plugin/pkg/admission/noderestriction/admission.go index 7434ffb589..222156a100 100644 --- a/plugin/pkg/admission/noderestriction/admission.go +++ b/plugin/pkg/admission/noderestriction/admission.go @@ -19,17 +19,20 @@ package noderestriction import ( "fmt" "io" + "strings" apiequality "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/util/diff" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apiserver/pkg/admission" apiserveradmission "k8s.io/apiserver/pkg/admission/initializer" utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/informers" corev1lister "k8s.io/client-go/listers/core/v1" csiv1alpha1 "k8s.io/csi-api/pkg/apis/csi/v1alpha1" + "k8s.io/klog" podutil "k8s.io/kubernetes/pkg/api/pod" authenticationapi "k8s.io/kubernetes/pkg/apis/authentication" coordapi "k8s.io/kubernetes/pkg/apis/coordination" @@ -37,6 +40,7 @@ import ( "k8s.io/kubernetes/pkg/apis/policy" "k8s.io/kubernetes/pkg/auth/nodeidentifier" "k8s.io/kubernetes/pkg/features" + kubeletapis "k8s.io/kubernetes/pkg/kubelet/apis" ) const ( @@ -330,6 +334,18 @@ func (c *nodePlugin) admitNode(nodeName string, a admission.Attributes) error { return admission.NewForbidden(a, fmt.Errorf("cannot create with non-nil configSource")) } + // Don't allow a node to register with labels outside the allowed set. + // This would allow a node to add or modify its labels in a way that would let it steer privileged workloads to itself. + modifiedLabels := getModifiedLabels(node.Labels, nil) + if forbiddenLabels := c.getForbiddenCreateLabels(modifiedLabels); len(forbiddenLabels) > 0 { + return admission.NewForbidden(a, fmt.Errorf("cannot set labels: %s", strings.Join(forbiddenLabels.List(), ", "))) + } + // check and warn if nodes set labels on create that would have been forbidden on update + // TODO(liggitt): in 1.17, expand getForbiddenCreateLabels to match getForbiddenUpdateLabels and drop this + if forbiddenUpdateLabels := c.getForbiddenUpdateLabels(modifiedLabels); len(forbiddenUpdateLabels) > 0 { + klog.Warningf("node %q added disallowed labels on node creation: %s", nodeName, strings.Join(forbiddenUpdateLabels.List(), ", ")) + } + // On create, get name from new object if unset in admission if len(requestedName) == 0 { requestedName = node.Name @@ -353,19 +369,100 @@ func (c *nodePlugin) admitNode(nodeName string, a admission.Attributes) error { // We scope node access to things listed in the Node.Spec, so allowing this would allow a view escalation. // We only do the check if the new node's configSource is non-nil; old kubelets might drop the field during a status update. 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")) + return admission.NewForbidden(a, fmt.Errorf("node %q cannot update configSource to a new non-nil configSource", nodeName)) } // 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 admission.NewForbidden(a, fmt.Errorf("node %q cannot modify taints", nodeName)) + } + + // Don't allow a node to update labels outside the allowed set. + // This would allow a node to add or modify its labels in a way that would let it steer privileged workloads to itself. + modifiedLabels := getModifiedLabels(node.Labels, oldNode.Labels) + if forbiddenUpdateLabels := c.getForbiddenUpdateLabels(modifiedLabels); len(forbiddenUpdateLabels) > 0 { + return admission.NewForbidden(a, fmt.Errorf("cannot modify labels: %s", strings.Join(forbiddenUpdateLabels.List(), ", "))) } } return nil } +// getModifiedLabels returns the set of label keys that are different between the two maps +func getModifiedLabels(a, b map[string]string) sets.String { + modified := sets.NewString() + for k, v1 := range a { + if v2, ok := b[k]; !ok || v1 != v2 { + modified.Insert(k) + } + } + for k, v1 := range b { + if v2, ok := a[k]; !ok || v1 != v2 { + modified.Insert(k) + } + } + return modified +} + +func isKubernetesLabel(key string) bool { + namespace := getLabelNamespace(key) + if namespace == "kubernetes.io" || strings.HasSuffix(namespace, ".kubernetes.io") { + return true + } + if namespace == "k8s.io" || strings.HasSuffix(namespace, ".k8s.io") { + return true + } + return false +} + +func getLabelNamespace(key string) string { + if parts := strings.SplitN(key, "/", 2); len(parts) == 2 { + return parts[0] + } + return "" +} + +// getForbiddenCreateLabels returns the set of labels that may not be set by the node. +// TODO(liggitt): in 1.17, expand to match getForbiddenUpdateLabels() +func (c *nodePlugin) getForbiddenCreateLabels(modifiedLabels sets.String) sets.String { + if len(modifiedLabels) == 0 { + return nil + } + + forbiddenLabels := sets.NewString() + for label := range modifiedLabels { + namespace := getLabelNamespace(label) + // forbid kubelets from setting node-restriction labels + if namespace == kubeletapis.LabelNamespaceNodeRestriction || strings.HasSuffix(namespace, "."+kubeletapis.LabelNamespaceNodeRestriction) { + forbiddenLabels.Insert(label) + } + } + return forbiddenLabels +} + +// getForbiddenLabels returns the set of labels that may not be set by the node on update. +func (c *nodePlugin) getForbiddenUpdateLabels(modifiedLabels sets.String) sets.String { + if len(modifiedLabels) == 0 { + return nil + } + + forbiddenLabels := sets.NewString() + for label := range modifiedLabels { + namespace := getLabelNamespace(label) + // forbid kubelets from setting node-restriction labels + if namespace == kubeletapis.LabelNamespaceNodeRestriction || strings.HasSuffix(namespace, "."+kubeletapis.LabelNamespaceNodeRestriction) { + forbiddenLabels.Insert(label) + } + // forbid kubelets from setting unknown kubernetes.io and k8s.io labels on update + if isKubernetesLabel(label) && !kubeletapis.IsKubeletLabel(label) { + // TODO: defer to label policy once available + forbiddenLabels.Insert(label) + } + } + return forbiddenLabels +} + func (c *nodePlugin) admitServiceAccount(nodeName string, a admission.Attributes) error { if a.GetOperation() != admission.Create { return nil diff --git a/plugin/pkg/admission/noderestriction/admission_test.go b/plugin/pkg/admission/noderestriction/admission_test.go index 79fd77c4a5..b271109e2a 100644 --- a/plugin/pkg/admission/noderestriction/admission_test.go +++ b/plugin/pkg/admission/noderestriction/admission_test.go @@ -17,16 +17,17 @@ limitations under the License. package noderestriction import ( + "fmt" + "reflect" "strings" "testing" "time" - "fmt" - corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/authentication/user" utilfeature "k8s.io/apiserver/pkg/util/feature" @@ -39,6 +40,7 @@ import ( "k8s.io/kubernetes/pkg/apis/policy" "k8s.io/kubernetes/pkg/auth/nodeidentifier" "k8s.io/kubernetes/pkg/features" + kubeletapis "k8s.io/kubernetes/pkg/kubelet/apis" "k8s.io/utils/pointer" ) @@ -120,6 +122,99 @@ func makeTokenRequest(podname string, poduid types.UID) *authenticationapi.Token return tr } +func setAllLabels(node *api.Node, value string) *api.Node { + node = setAllowedCreateLabels(node, value) + node = setAllowedUpdateLabels(node, value) + node = setForbiddenCreateLabels(node, value) + node = setForbiddenUpdateLabels(node, value) + return node +} + +func setAllowedCreateLabels(node *api.Node, value string) *api.Node { + node = setAllowedUpdateLabels(node, value) + // also allow other kubernetes labels on create until 1.17 (TODO: remove this in 1.17) + node.Labels["other.kubernetes.io/foo"] = value + node.Labels["other.k8s.io/foo"] = value + return node +} + +func setAllowedUpdateLabels(node *api.Node, value string) *api.Node { + node = node.DeepCopy() + if node.Labels == nil { + node.Labels = map[string]string{} + } + if value == "" { + value = "value" + } + // non-kube labels + node.Labels["foo"] = value + node.Labels["example.com/foo"] = value + + // kubelet labels + node.Labels["kubernetes.io/hostname"] = value + node.Labels["failure-domain.beta.kubernetes.io/zone"] = value + node.Labels["failure-domain.beta.kubernetes.io/region"] = value + node.Labels["beta.kubernetes.io/instance-type"] = value + node.Labels["beta.kubernetes.io/os"] = value + node.Labels["beta.kubernetes.io/arch"] = value + node.Labels["failure-domain.kubernetes.io/zone"] = value + node.Labels["failure-domain.kubernetes.io/region"] = value + node.Labels["kubernetes.io/instance-type"] = value + node.Labels["kubernetes.io/os"] = value + node.Labels["kubernetes.io/arch"] = value + + // kubelet label prefixes + node.Labels["kubelet.kubernetes.io/foo"] = value + node.Labels["foo.kubelet.kubernetes.io/foo"] = value + node.Labels["node.kubernetes.io/foo"] = value + node.Labels["foo.node.kubernetes.io/foo"] = value + + // test all explicitly allowed labels and prefixes + for _, key := range kubeletapis.KubeletLabels() { + node.Labels[key] = value + } + for _, namespace := range kubeletapis.KubeletLabelNamespaces() { + node.Labels[namespace+"/foo"] = value + node.Labels["foo."+namespace+"/foo"] = value + } + + return node +} + +func setForbiddenCreateLabels(node *api.Node, value string) *api.Node { + node = node.DeepCopy() + if node.Labels == nil { + node.Labels = map[string]string{} + } + if value == "" { + value = "value" + } + // node restriction labels are forbidden + node.Labels["node-restriction.kubernetes.io/foo"] = value + node.Labels["foo.node-restriction.kubernetes.io/foo"] = value + // TODO: in 1.17, forbid arbitrary kubernetes labels on create + // node.Labels["other.kubernetes.io/foo"] = value + // node.Labels["other.k8s.io/foo"] = value + return node +} + +func setForbiddenUpdateLabels(node *api.Node, value string) *api.Node { + node = node.DeepCopy() + if node.Labels == nil { + node.Labels = map[string]string{} + } + if value == "" { + value = "value" + } + // node restriction labels are forbidden + node.Labels["node-restriction.kubernetes.io/foo"] = value + node.Labels["foo.node-restriction.kubernetes.io/foo"] = value + // arbitrary kubernetes labels are forbidden on update + node.Labels["other.kubernetes.io/foo"] = value + node.Labels["other.k8s.io/foo"] = value + return node +} + func Test_nodePlugin_Admit(t *testing.T) { var ( mynode = &user.DefaultInfo{Name: "system:node:mynode", Groups: []string{"system:nodes"}} @@ -763,6 +858,18 @@ func Test_nodePlugin_Admit(t *testing.T) { attributes: admission.NewAttributesRecord(mynodeObjTaintA, nil, nodeKind, mynodeObj.Namespace, "", nodeResource, "", admission.Create, false, mynode), err: "", }, + { + name: "allow create of my node with labels", + podsGetter: noExistingPods, + attributes: admission.NewAttributesRecord(setAllowedCreateLabels(mynodeObj, ""), nil, nodeKind, mynodeObj.Namespace, "", nodeResource, "", admission.Create, false, mynode), + err: "", + }, + { + name: "forbid create of my node with forbidden labels", + podsGetter: noExistingPods, + attributes: admission.NewAttributesRecord(setForbiddenCreateLabels(mynodeObj, ""), nil, nodeKind, mynodeObj.Namespace, "", nodeResource, "", admission.Create, false, mynode), + err: `cannot set labels: foo.node-restriction.kubernetes.io/foo, node-restriction.kubernetes.io/foo`, + }, { name: "allow update of my node", podsGetter: existingPods, @@ -817,6 +924,42 @@ func Test_nodePlugin_Admit(t *testing.T) { attributes: admission.NewAttributesRecord(mynodeObjTaintA, mynodeObjTaintA, nodeKind, mynodeObj.Namespace, mynodeObj.Name, nodeResource, "", admission.Update, false, mynode), err: "", }, + { + name: "allow update of my node: add allowed labels", + podsGetter: existingPods, + attributes: admission.NewAttributesRecord(setAllowedUpdateLabels(mynodeObj, ""), mynodeObj, nodeKind, mynodeObj.Namespace, mynodeObj.Name, nodeResource, "", admission.Update, false, mynode), + err: "", + }, + { + name: "allow update of my node: remove allowed labels", + podsGetter: existingPods, + attributes: admission.NewAttributesRecord(mynodeObj, setAllowedUpdateLabels(mynodeObj, ""), nodeKind, mynodeObj.Namespace, mynodeObj.Name, nodeResource, "", admission.Update, false, mynode), + err: "", + }, + { + name: "allow update of my node: modify allowed labels", + podsGetter: existingPods, + attributes: admission.NewAttributesRecord(setAllowedUpdateLabels(mynodeObj, "b"), setAllowedUpdateLabels(mynodeObj, "a"), nodeKind, mynodeObj.Namespace, mynodeObj.Name, nodeResource, "", admission.Update, false, mynode), + err: "", + }, + { + name: "allow update of my node: no change to labels", + podsGetter: existingPods, + attributes: admission.NewAttributesRecord(setAllLabels(mynodeObj, ""), setAllLabels(mynodeObj, ""), nodeKind, mynodeObj.Namespace, mynodeObj.Name, nodeResource, "", admission.Update, false, mynode), + err: "", + }, + { + name: "allow update of my node: add allowed labels while forbidden labels exist unmodified", + podsGetter: existingPods, + attributes: admission.NewAttributesRecord(setAllLabels(mynodeObj, ""), setForbiddenUpdateLabels(mynodeObj, ""), nodeKind, mynodeObj.Namespace, mynodeObj.Name, nodeResource, "", admission.Update, false, mynode), + err: "", + }, + { + name: "allow update of my node: remove allowed labels while forbidden labels exist unmodified", + podsGetter: existingPods, + attributes: admission.NewAttributesRecord(setForbiddenUpdateLabels(mynodeObj, ""), setAllLabels(mynodeObj, ""), nodeKind, mynodeObj.Namespace, mynodeObj.Name, nodeResource, "", admission.Update, false, mynode), + err: "", + }, { name: "forbid update of my node: add taints", podsGetter: existingPods, @@ -835,6 +978,24 @@ func Test_nodePlugin_Admit(t *testing.T) { attributes: admission.NewAttributesRecord(mynodeObjTaintA, mynodeObjTaintB, nodeKind, mynodeObj.Namespace, mynodeObj.Name, nodeResource, "", admission.Update, false, mynode), err: "cannot modify taints", }, + { + name: "forbid update of my node: add labels", + podsGetter: existingPods, + attributes: admission.NewAttributesRecord(setForbiddenUpdateLabels(mynodeObj, ""), mynodeObj, nodeKind, mynodeObj.Namespace, mynodeObj.Name, nodeResource, "", admission.Update, false, mynode), + err: `cannot modify labels: foo.node-restriction.kubernetes.io/foo, node-restriction.kubernetes.io/foo, other.k8s.io/foo, other.kubernetes.io/foo`, + }, + { + name: "forbid update of my node: remove labels", + podsGetter: existingPods, + attributes: admission.NewAttributesRecord(mynodeObj, setForbiddenUpdateLabels(mynodeObj, ""), nodeKind, mynodeObj.Namespace, mynodeObj.Name, nodeResource, "", admission.Update, false, mynode), + err: `cannot modify labels: foo.node-restriction.kubernetes.io/foo, node-restriction.kubernetes.io/foo, other.k8s.io/foo, other.kubernetes.io/foo`, + }, + { + name: "forbid update of my node: change labels", + podsGetter: existingPods, + attributes: admission.NewAttributesRecord(setForbiddenUpdateLabels(mynodeObj, "new"), setForbiddenUpdateLabels(mynodeObj, "old"), nodeKind, mynodeObj.Namespace, mynodeObj.Name, nodeResource, "", admission.Update, false, mynode), + err: `cannot modify labels: foo.node-restriction.kubernetes.io/foo, node-restriction.kubernetes.io/foo, other.k8s.io/foo, other.kubernetes.io/foo`, + }, // Other node object { @@ -1084,3 +1245,80 @@ func Test_nodePlugin_Admit(t *testing.T) { }) } } + +func Test_getModifiedLabels(t *testing.T) { + tests := []struct { + name string + a map[string]string + b map[string]string + want sets.String + }{ + { + name: "empty", + a: nil, + b: nil, + want: sets.NewString(), + }, + { + name: "no change", + a: map[string]string{"x": "1", "y": "2", "z": "3"}, + b: map[string]string{"x": "1", "y": "2", "z": "3"}, + want: sets.NewString(), + }, + { + name: "added", + a: map[string]string{}, + b: map[string]string{"a": "0"}, + want: sets.NewString("a"), + }, + { + name: "removed", + a: map[string]string{"z": "3"}, + b: map[string]string{}, + want: sets.NewString("z"), + }, + { + name: "changed", + a: map[string]string{"z": "3"}, + b: map[string]string{"z": "4"}, + want: sets.NewString("z"), + }, + { + name: "added empty", + a: map[string]string{}, + b: map[string]string{"a": ""}, + want: sets.NewString("a"), + }, + { + name: "removed empty", + a: map[string]string{"z": ""}, + b: map[string]string{}, + want: sets.NewString("z"), + }, + { + name: "changed to empty", + a: map[string]string{"z": "3"}, + b: map[string]string{"z": ""}, + want: sets.NewString("z"), + }, + { + name: "changed from empty", + a: map[string]string{"z": ""}, + b: map[string]string{"z": "3"}, + want: sets.NewString("z"), + }, + { + name: "added, removed, and changed", + a: map[string]string{"a": "1", "b": "2"}, + b: map[string]string{"a": "2", "c": "3"}, + want: sets.NewString("a", "b", "c"), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := getModifiedLabels(tt.a, tt.b); !reflect.DeepEqual(got, tt.want) { + t.Errorf("getModifiedLabels() = %v, want %v", got, tt.want) + } + }) + } +}