From ba4d2aa076cd925f6659885d77f5bc2016ad57f6 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Mon, 20 Apr 2020 16:14:12 -0400 Subject: [PATCH] Restrict node labels on Node create --- plugin/pkg/admission/noderestriction/BUILD | 1 - .../admission/noderestriction/admission.go | 32 +++---------------- .../noderestriction/admission_test.go | 10 ++---- 3 files changed, 7 insertions(+), 36 deletions(-) diff --git a/plugin/pkg/admission/noderestriction/BUILD b/plugin/pkg/admission/noderestriction/BUILD index e9dfae55fe9..736a3d046b6 100644 --- a/plugin/pkg/admission/noderestriction/BUILD +++ b/plugin/pkg/admission/noderestriction/BUILD @@ -32,7 +32,6 @@ go_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/component-base/featuregate:go_default_library", - "//vendor/k8s.io/klog:go_default_library", ], ) diff --git a/plugin/pkg/admission/noderestriction/admission.go b/plugin/pkg/admission/noderestriction/admission.go index 314800b8ef1..4ec35894e8f 100644 --- a/plugin/pkg/admission/noderestriction/admission.go +++ b/plugin/pkg/admission/noderestriction/admission.go @@ -34,7 +34,6 @@ import ( "k8s.io/client-go/informers" corev1lister "k8s.io/client-go/listers/core/v1" "k8s.io/component-base/featuregate" - "k8s.io/klog" podutil "k8s.io/kubernetes/pkg/api/pod" authenticationapi "k8s.io/kubernetes/pkg/apis/authentication" coordapi "k8s.io/kubernetes/pkg/apis/coordination" @@ -406,14 +405,9 @@ func (p *Plugin) admitNode(nodeName string, a admission.Attributes) error { // 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 := p.getForbiddenCreateLabels(modifiedLabels); len(forbiddenLabels) > 0 { + if forbiddenLabels := p.getForbiddenLabels(modifiedLabels); len(forbiddenLabels) > 0 { return admission.NewForbidden(a, fmt.Errorf("node %q is not allowed to set the following labels: %s", nodeName, strings.Join(forbiddenLabels.List(), ", "))) } - // check and warn if nodes set labels on create that would have been forbidden on update - // TODO(liggitt): in 1.19, expand getForbiddenCreateLabels to match getForbiddenUpdateLabels and drop this - if forbiddenUpdateLabels := p.getForbiddenUpdateLabels(modifiedLabels); len(forbiddenUpdateLabels) > 0 { - klog.Warningf("node %q added disallowed labels on node creation: %s", nodeName, strings.Join(forbiddenUpdateLabels.List(), ", ")) - } } if requestedName != nodeName { return admission.NewForbidden(a, fmt.Errorf("node %q is not allowed to modify node %q", nodeName, requestedName)) @@ -445,7 +439,7 @@ func (p *Plugin) admitNode(nodeName string, a admission.Attributes) error { // 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 := p.getForbiddenUpdateLabels(modifiedLabels); len(forbiddenUpdateLabels) > 0 { + if forbiddenUpdateLabels := p.getForbiddenLabels(modifiedLabels); len(forbiddenUpdateLabels) > 0 { return admission.NewForbidden(a, fmt.Errorf("is not allowed to modify labels: %s", strings.Join(forbiddenUpdateLabels.List(), ", "))) } } @@ -487,26 +481,8 @@ func getLabelNamespace(key string) string { return "" } -// getForbiddenCreateLabels returns the set of labels that may not be set by the node. -// TODO(liggitt): in 1.19, expand to match getForbiddenUpdateLabels() -func (p *Plugin) 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 == v1.LabelNamespaceNodeRestriction || strings.HasSuffix(namespace, "."+v1.LabelNamespaceNodeRestriction) { - forbiddenLabels.Insert(label) - } - } - return forbiddenLabels -} - -// getForbiddenLabels returns the set of labels that may not be set by the node on update. -func (p *Plugin) getForbiddenUpdateLabels(modifiedLabels sets.String) sets.String { +// getForbiddenLabels returns the set of labels that may not be added, removed, or modified by the node on create or update. +func (p *Plugin) getForbiddenLabels(modifiedLabels sets.String) sets.String { if len(modifiedLabels) == 0 { return nil } diff --git a/plugin/pkg/admission/noderestriction/admission_test.go b/plugin/pkg/admission/noderestriction/admission_test.go index 805552e0203..2d0565cffca 100644 --- a/plugin/pkg/admission/noderestriction/admission_test.go +++ b/plugin/pkg/admission/noderestriction/admission_test.go @@ -146,9 +146,6 @@ func setAllLabels(node *api.Node, value string) *api.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 } @@ -206,9 +203,8 @@ func setForbiddenCreateLabels(node *api.Node, value string) *api.Node { // 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 + node.Labels["other.kubernetes.io/foo"] = value + node.Labels["other.k8s.io/foo"] = value return node } @@ -925,7 +921,7 @@ func Test_nodePlugin_Admit(t *testing.T) { name: "forbid create of my node with forbidden labels", podsGetter: noExistingPods, attributes: admission.NewAttributesRecord(setForbiddenCreateLabels(mynodeObj, ""), nil, nodeKind, mynodeObj.Namespace, "", nodeResource, "", admission.Create, &metav1.CreateOptions{}, false, mynode), - err: `is not allowed to set the following labels: foo.node-restriction.kubernetes.io/foo, node-restriction.kubernetes.io/foo`, + err: `is not allowed to set the following labels: foo.node-restriction.kubernetes.io/foo, node-restriction.kubernetes.io/foo, other.k8s.io/foo, other.kubernetes.io/foo`, }, { name: "allow update of my node",