node authorizer sets up access rules for dynamic config

This PR makes the node authorizer automatically set up access rules for
dynamic Kubelet config.

I also added some validation to the node strategy, which I discovered we
were missing while writing this.

pkg/apis/core/validation/validation.go

@@ -4098,6 +4098,9 @@ func ValidateNodeUpdate(node, oldNode *core.Node) field.ErrorList {
 
 	// Allow updates to Node.Spec.ConfigSource if DynamicKubeletConfig feature gate is enabled
 	if utilfeature.DefaultFeatureGate.Enabled(features.DynamicKubeletConfig) {
+		if node.Spec.ConfigSource != nil {
+			allErrs = append(allErrs, validateNodeConfigSource(node.Spec.ConfigSource, field.NewPath("spec", "configSource"))...)
+		}
 		oldNode.Spec.ConfigSource = node.Spec.ConfigSource
 	}
 
@@ -4111,6 +4114,25 @@ func ValidateNodeUpdate(node, oldNode *core.Node) field.ErrorList {
 	return allErrs
 }
 
+func validateNodeConfigSource(source *core.NodeConfigSource, fldPath *field.Path) field.ErrorList {
+	allErrs := field.ErrorList{}
+	count := int(0)
+	if ref := source.ConfigMapRef; ref != nil {
+		count++
+		// name, namespace, and UID must all be non-empty for ConfigMapRef
+		if ref.Name == "" || ref.Namespace == "" || string(ref.UID) == "" {
+			allErrs = append(allErrs, field.Invalid(fldPath.Child("configMapRef"), ref, "name, namespace, and UID must all be non-empty"))
+		}
+	}
+	// add more subfields here in the future as they are added to NodeConfigSource
+
+	// exactly one reference subfield must be non-nil
+	if count != 1 {
+		allErrs = append(allErrs, field.Invalid(fldPath, source, "exactly one reference subfield must be non-nil"))
+	}
+	return allErrs
+}
+
 // Validate compute resource typename.
 // Refer to docs/design/resources.md for more details.
 func validateResourceName(value string, fldPath *field.Path) field.ErrorList {

pkg/kubeapiserver/authorizer/config.go

@@ -68,12 +68,13 @@ func (config AuthorizationConfig) New() (authorizer.Authorizer, authorizer.RuleR
 	)
 
 	for _, authorizationMode := range config.AuthorizationModes {
-		// Keep cases in sync with constant list above.
+		// Keep cases in sync with constant list in k8s.io/kubernetes/pkg/kubeapiserver/authorizer/modes/modes.go.
 		switch authorizationMode {
 		case modes.ModeNode:
 			graph := node.NewGraph()
 			node.AddGraphEventHandlers(
 				graph,
+				config.InformerFactory.Core().InternalVersion().Nodes(),
 				config.InformerFactory.Core().InternalVersion().Pods(),
 				config.InformerFactory.Core().InternalVersion().PersistentVolumes(),
 				config.VersionedInformerFactory.Storage().V1beta1().VolumeAttachments(),

plugin/pkg/auth/authorizer/node/BUILD

@@ -8,15 +8,20 @@ load(
 
 go_test(
     name = "go_default_test",
-    srcs = ["node_authorizer_test.go"],
+    srcs = [
+        "graph_test.go",
+        "node_authorizer_test.go",
+    ],
     embed = [":go_default_library"],
     deps = [
         "//pkg/apis/core:go_default_library",
         "//pkg/auth/nodeidentifier:go_default_library",
         "//pkg/features:go_default_library",
         "//plugin/pkg/auth/authorizer/rbac/bootstrappolicy:go_default_library",
+        "//vendor/github.com/stretchr/testify/assert:go_default_library",
         "//vendor/k8s.io/api/storage/v1beta1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/types:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/authentication/user:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/authorization/authorizer:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/util/feature:go_default_library",

plugin/pkg/auth/authorizer/node/graph.go

@@ -198,6 +198,50 @@ func (g *Graph) deleteVertex_locked(vertexType vertexType, namespace, name strin
 	}
 }
 
+// must be called under write lock
+// deletes edges from a given vertex type to a specific vertex
+// will delete each orphaned "from" vertex, but will never delete the "to" vertex
+func (g *Graph) deleteEdges_locked(fromType, toType vertexType, toNamespace, toName string) {
+	// get the "to" side
+	toVert, exists := g.getVertex_rlocked(toType, toNamespace, toName)
+	if !exists {
+		return
+	}
+
+	// get potential "from" verts that match fromType
+	namespaces, exists := g.vertices[fromType]
+	if !exists {
+		return
+	}
+
+	// delete all edges between vertices of fromType and toVert
+	removeVerts := []*namedVertex{}
+	for _, vertexMapping := range namespaces {
+		for _, fromVert := range vertexMapping {
+			if g.graph.HasEdgeBetween(fromVert, toVert) {
+				// remove the edge (no-op if edge doesn't exist)
+				g.graph.RemoveEdge(newDestinationEdge(fromVert, toVert, nil))
+				// remember to clean up the fromVert if we orphaned it
+				if g.graph.Degree(fromVert) == 0 {
+					removeVerts = append(removeVerts, fromVert)
+				}
+			}
+		}
+	}
+
+	// clean up orphaned verts
+	for _, v := range removeVerts {
+		g.graph.RemoveNode(v)
+		delete(g.vertices[v.vertexType][v.namespace], v.name)
+		if len(g.vertices[v.vertexType][v.namespace]) == 0 {
+			delete(g.vertices[v.vertexType], v.namespace)
+		}
+		if len(g.vertices[v.vertexType]) == 0 {
+			delete(g.vertices, v.vertexType)
+		}
+	}
+}
+
 // AddPod should only be called once spec.NodeName is populated.
 // It sets up edges for the following relationships (which are immutable for a pod once bound to a node):
 //
@@ -301,3 +345,25 @@ func (g *Graph) DeleteVolumeAttachment(name string) {
 	defer g.lock.Unlock()
 	g.deleteVertex_locked(vaVertexType, "", name)
 }
+
+// SetNodeConfigMap sets up edges for the Node.Spec.ConfigSource.ConfigMapRef relationship:
+//
+// configmap -> node
+func (g *Graph) SetNodeConfigMap(nodeName, configMapName, configMapNamespace string) {
+	g.lock.Lock()
+	defer g.lock.Unlock()
+
+	// TODO(mtaufen): ensure len(nodeName) > 0 in all cases (would sure be nice to have a dependently-typed language here...)
+
+	// clear edges configmaps -> node where the destination is the current node *only*
+	// at present, a node can only have one *direct* configmap reference at a time
+	g.deleteEdges_locked(configMapVertexType, nodeVertexType, "", nodeName)
+
+	// establish new edges if we have a real ConfigMap to reference
+	if len(configMapName) > 0 && len(configMapNamespace) > 0 {
+		configmapVertex := g.getOrCreateVertex_locked(configMapVertexType, configMapNamespace, configMapName)
+		nodeVertex := g.getOrCreateVertex_locked(nodeVertexType, "", nodeName)
+		g.graph.SetEdge(newDestinationEdge(configmapVertex, nodeVertex, nodeVertex))
+	}
+
+}

plugin/pkg/auth/authorizer/node/graph_populator.go

@@ -17,6 +17,7 @@ limitations under the License.
 package node
 
 import (
+	"fmt"
 	"github.com/golang/glog"
 
 	storagev1beta1 "k8s.io/api/storage/v1beta1"
@@ -34,6 +35,7 @@ type graphPopulator struct {
 
 func AddGraphEventHandlers(
 	graph *Graph,
+	nodes coreinformers.NodeInformer,
 	pods coreinformers.PodInformer,
 	pvs coreinformers.PersistentVolumeInformer,
 	attachments storageinformers.VolumeAttachmentInformer,
@@ -42,6 +44,14 @@ func AddGraphEventHandlers(
 		graph: graph,
 	}
 
+	if utilfeature.DefaultFeatureGate.Enabled(features.DynamicKubeletConfig) {
+		nodes.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
+			AddFunc:    g.addNode,
+			UpdateFunc: g.updateNode,
+			DeleteFunc: g.deleteNode,
+		})
+	}
+
 	pods.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
 		AddFunc:    g.addPod,
 		UpdateFunc: g.updatePod,
@@ -63,6 +73,57 @@ func AddGraphEventHandlers(
 	}
 }
 
+func (g *graphPopulator) addNode(obj interface{}) {
+	g.updateNode(nil, obj)
+}
+
+func (g *graphPopulator) updateNode(oldObj, obj interface{}) {
+	node := obj.(*api.Node)
+	var oldNode *api.Node
+	if oldObj != nil {
+		oldNode = oldObj.(*api.Node)
+	}
+
+	// we only set up rules for ConfigMapRef today, because that is the only reference type
+
+	var name, namespace string
+	if source := node.Spec.ConfigSource; source != nil && source.ConfigMapRef != nil {
+		name = source.ConfigMapRef.Name
+		namespace = source.ConfigMapRef.Namespace
+	}
+
+	var oldName, oldNamespace string
+	if oldNode != nil {
+		if oldSource := oldNode.Spec.ConfigSource; oldSource != nil && oldSource.ConfigMapRef != nil {
+			oldName = oldSource.ConfigMapRef.Name
+			oldNamespace = oldSource.ConfigMapRef.Namespace
+		}
+	}
+
+	// if Node.Spec.ConfigSource wasn't updated, nothing for us to do
+	if name == oldName && namespace == oldNamespace {
+		return
+	}
+
+	path := "nil"
+	if node.Spec.ConfigSource != nil {
+		path = fmt.Sprintf("%s/%s", namespace, name)
+	}
+	glog.V(4).Infof("updateNode configSource reference to %s for node %s", path, node.Name)
+	g.graph.SetNodeConfigMap(node.Name, name, namespace)
+}
+
+func (g *graphPopulator) deleteNode(obj interface{}) {
+	node := obj.(*api.Node)
+
+	// TODO(mtaufen): ensure len(nodeName) > 0 in all cases (would sure be nice to have a dependently-typed language here...)
+
+	// NOTE: We don't remove the node, because if the node is re-created not all pod -> node
+	// links are re-established (we don't get relevant events because the no mutations need
+	// to happen in the API; the state is already there).
+	g.graph.SetNodeConfigMap(node.Name, "", "")
+}
+
 func (g *graphPopulator) addPod(obj interface{}) {
 	g.updatePod(nil, obj)
 }

plugin/pkg/auth/authorizer/node/graph_test.go

@@ -0,0 +1,176 @@
+/*
+Copyright 2018 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 node
+
+import (
+	"sort"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestDeleteEdges_locked(t *testing.T) {
+	cases := []struct {
+		desc        string
+		fromType    vertexType
+		toType      vertexType
+		toNamespace string
+		toName      string
+		start       *Graph
+		expect      *Graph
+	}{
+		{
+			// single edge from a configmap to a node, will delete edge and orphaned configmap
+			desc:        "edges and source orphans are deleted, destination orphans are preserved",
+			fromType:    configMapVertexType,
+			toType:      nodeVertexType,
+			toNamespace: "",
+			toName:      "node1",
+			start: func() *Graph {
+				g := NewGraph()
+				nodeVertex := g.getOrCreateVertex_locked(nodeVertexType, "", "node1")
+				configmapVertex := g.getOrCreateVertex_locked(configMapVertexType, "namespace1", "configmap1")
+				g.graph.SetEdge(newDestinationEdge(configmapVertex, nodeVertex, nodeVertex))
+				return g
+			}(),
+			expect: func() *Graph {
+				g := NewGraph()
+				g.getOrCreateVertex_locked(nodeVertexType, "", "node1")
+				return g
+			}(),
+		},
+		{
+			// two edges from the same configmap to distinct nodes, will delete one of the edges
+			desc:        "edges are deleted, non-orphans and destination orphans are preserved",
+			fromType:    configMapVertexType,
+			toType:      nodeVertexType,
+			toNamespace: "",
+			toName:      "node2",
+			start: func() *Graph {
+				g := NewGraph()
+				nodeVertex1 := g.getOrCreateVertex_locked(nodeVertexType, "", "node1")
+				nodeVertex2 := g.getOrCreateVertex_locked(nodeVertexType, "", "node2")
+				configmapVertex := g.getOrCreateVertex_locked(configMapVertexType, "namespace1", "configmap1")
+				g.graph.SetEdge(newDestinationEdge(configmapVertex, nodeVertex1, nodeVertex1))
+				g.graph.SetEdge(newDestinationEdge(configmapVertex, nodeVertex2, nodeVertex2))
+				return g
+			}(),
+			expect: func() *Graph {
+				g := NewGraph()
+				nodeVertex1 := g.getOrCreateVertex_locked(nodeVertexType, "", "node1")
+				g.getOrCreateVertex_locked(nodeVertexType, "", "node2")
+				configmapVertex := g.getOrCreateVertex_locked(configMapVertexType, "namespace1", "configmap1")
+				g.graph.SetEdge(newDestinationEdge(configmapVertex, nodeVertex1, nodeVertex1))
+				return g
+			}(),
+		},
+		{
+			desc:        "no edges to delete",
+			fromType:    configMapVertexType,
+			toType:      nodeVertexType,
+			toNamespace: "",
+			toName:      "node1",
+			start: func() *Graph {
+				g := NewGraph()
+				g.getOrCreateVertex_locked(nodeVertexType, "", "node1")
+				g.getOrCreateVertex_locked(configMapVertexType, "namespace1", "configmap1")
+				return g
+			}(),
+			expect: func() *Graph {
+				g := NewGraph()
+				g.getOrCreateVertex_locked(nodeVertexType, "", "node1")
+				g.getOrCreateVertex_locked(configMapVertexType, "namespace1", "configmap1")
+				return g
+			}(),
+		},
+		{
+			desc:        "destination vertex does not exist",
+			fromType:    configMapVertexType,
+			toType:      nodeVertexType,
+			toNamespace: "",
+			toName:      "node1",
+			start: func() *Graph {
+				g := NewGraph()
+				g.getOrCreateVertex_locked(configMapVertexType, "namespace1", "configmap1")
+				return g
+			}(),
+			expect: func() *Graph {
+				g := NewGraph()
+				g.getOrCreateVertex_locked(configMapVertexType, "namespace1", "configmap1")
+				return g
+			}(),
+		},
+		{
+			desc:        "source vertex type doesn't exist",
+			fromType:    configMapVertexType,
+			toType:      nodeVertexType,
+			toNamespace: "",
+			toName:      "node1",
+			start: func() *Graph {
+				g := NewGraph()
+				g.getOrCreateVertex_locked(nodeVertexType, "", "node1")
+				return g
+			}(),
+			expect: func() *Graph {
+				g := NewGraph()
+				g.getOrCreateVertex_locked(nodeVertexType, "", "node1")
+				return g
+			}(),
+		},
+	}
+	for _, c := range cases {
+		t.Run(c.desc, func(t *testing.T) {
+			c.start.deleteEdges_locked(c.fromType, c.toType, c.toNamespace, c.toName)
+
+			// Note: We assert on substructures (graph.Nodes(), graph.Edges()) because the graph tracks
+			// freed IDs for reuse, which results in an irrelevant inequality between start and expect.
+
+			// sort the nodes by ID
+			// (the slices we get back are from map iteration, where order is not guaranteed)
+			expectNodes := c.expect.graph.Nodes()
+			sort.Slice(expectNodes, func(i, j int) bool {
+				return expectNodes[i].ID() < expectNodes[j].ID()
+			})
+			startNodes := c.start.graph.Nodes()
+			sort.Slice(expectNodes, func(i, j int) bool {
+				return startNodes[i].ID() < startNodes[j].ID()
+			})
+			assert.Equal(t, expectNodes, startNodes)
+
+			// sort the edges by from ID, then to ID
+			// (the slices we get back are from map iteration, where order is not guaranteed)
+			expectEdges := c.expect.graph.Edges()
+			sort.Slice(expectEdges, func(i, j int) bool {
+				if expectEdges[i].From().ID() == expectEdges[j].From().ID() {
+					return expectEdges[i].To().ID() < expectEdges[j].To().ID()
+				}
+				return expectEdges[i].From().ID() < expectEdges[j].From().ID()
+			})
+			startEdges := c.start.graph.Edges()
+			sort.Slice(expectEdges, func(i, j int) bool {
+				if startEdges[i].From().ID() == startEdges[j].From().ID() {
+					return startEdges[i].To().ID() < startEdges[j].To().ID()
+				}
+				return startEdges[i].From().ID() < startEdges[j].From().ID()
+			})
+			assert.Equal(t, expectEdges, startEdges)
+
+			// vertices is a recursive map, no need to sort
+			assert.Equal(t, c.expect.vertices, c.start.vertices)
+		})
+	}
+}

plugin/pkg/auth/authorizer/node/node_authorizer.go

@@ -38,6 +38,7 @@ import (
 // 1. If a request is not from a node (NodeIdentity() returns isNode=false), reject
 // 2. If a specific node cannot be identified (NodeIdentity() returns nodeName=""), reject
 // 3. If a request is for a secret, configmap, persistent volume or persistent volume claim, reject unless the verb is get, and the requested object is related to the requesting node:
+//    node <- configmap
 //    node <- pod
 //    node <- pod <- secret
 //    node <- pod <- configmap

plugin/pkg/auth/authorizer/node/node_authorizer_test.go

@@ -26,6 +26,7 @@ import (
 
 	storagev1beta1 "k8s.io/api/storage/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apiserver/pkg/authentication/user"
 	"k8s.io/apiserver/pkg/authorization/authorizer"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
@@ -72,8 +73,8 @@ func TestAuthorizer(t *testing.T) {
 		sharedPVCsPerPod:       0,
 		uniquePVCsPerPod:       1,
 	}
-	pods, pvs, attachments := generate(opts)
+	nodes, pods, pvs, attachments := generate(opts)
-	populate(g, pods, pvs, attachments)
+	populate(g, nodes, pods, pvs, attachments)
 
 	identifier := nodeidentifier.NewDefaultNodeIdentifier()
 	authz := NewAuthorizer(g, identifier, bootstrappolicy.NodeRules()).(*NodeAuthorizer)
@@ -86,6 +87,11 @@ func TestAuthorizer(t *testing.T) {
 		expect   authorizer.Decision
 		features utilfeature.FeatureGate
 	}{
+		{
+			name:   "allowed node configmap",
+			attrs:  authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "configmaps", Name: "node0-configmap", Namespace: "ns0"},
+			expect: authorizer.DecisionAllow,
+		},
 		{
 			name:   "allowed configmap",
 			attrs:  authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "configmaps", Name: "configmap0-pod0-node0", Namespace: "ns0"},
@@ -117,6 +123,11 @@ func TestAuthorizer(t *testing.T) {
 			expect: authorizer.DecisionAllow,
 		},
 
+		{
+			name:   "disallowed node configmap",
+			attrs:  authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "configmaps", Name: "node1-configmap", Namespace: "ns0"},
+			expect: authorizer.DecisionNoOpinion,
+		},
 		{
 			name:   "disallowed configmap",
 			attrs:  authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "configmaps", Name: "configmap0-pod0-node1", Namespace: "ns0"},
@@ -247,9 +258,14 @@ func TestAuthorizerSharedResources(t *testing.T) {
 		},
 	})
 
+	g.SetNodeConfigMap("node1", "shared-configmap", "ns1")
+	g.SetNodeConfigMap("node2", "shared-configmap", "ns1")
+	g.SetNodeConfigMap("node3", "configmap", "ns1")
+
 	testcases := []struct {
 		User          user.Info
 		Secret        string
+		ConfigMap     string
 		ExpectAllowed bool
 	}{
 		{User: node1, ExpectAllowed: true, Secret: "node1-only"},
@@ -263,14 +279,39 @@ func TestAuthorizerSharedResources(t *testing.T) {
 		{User: node3, ExpectAllowed: false, Secret: "node1-only"},
 		{User: node3, ExpectAllowed: false, Secret: "node1-node2-only"},
 		{User: node3, ExpectAllowed: true, Secret: "shared-all"},
+
+		{User: node1, ExpectAllowed: true, ConfigMap: "shared-configmap"},
+		{User: node1, ExpectAllowed: false, ConfigMap: "configmap"},
+
+		{User: node2, ExpectAllowed: true, ConfigMap: "shared-configmap"},
+		{User: node2, ExpectAllowed: false, ConfigMap: "configmap"},
+
+		{User: node3, ExpectAllowed: false, ConfigMap: "shared-configmap"},
+		{User: node3, ExpectAllowed: true, ConfigMap: "configmap"},
 	}
 
 	for i, tc := range testcases {
-		decision, _, err := authz.Authorize(authorizer.AttributesRecord{User: tc.User, ResourceRequest: true, Verb: "get", Resource: "secrets", Namespace: "ns1", Name: tc.Secret})
+		var (
-		if err != nil {
+			decision authorizer.Decision
-			t.Errorf("%d: unexpected error: %v", i, err)
+			err      error
-			continue
+		)
+
+		if len(tc.Secret) > 0 {
+			decision, _, err = authz.Authorize(authorizer.AttributesRecord{User: tc.User, ResourceRequest: true, Verb: "get", Resource: "secrets", Namespace: "ns1", Name: tc.Secret})
+			if err != nil {
+				t.Errorf("%d: unexpected error: %v", i, err)
+				continue
+			}
+		} else if len(tc.ConfigMap) > 0 {
+			decision, _, err = authz.Authorize(authorizer.AttributesRecord{User: tc.User, ResourceRequest: true, Verb: "get", Resource: "configmaps", Namespace: "ns1", Name: tc.ConfigMap})
+			if err != nil {
+				t.Errorf("%d: unexpected error: %v", i, err)
+				continue
+			}
+		} else {
+			t.Fatalf("test case must include a request for a Secret or ConfigMap")
 		}
+
 		if (decision == authorizer.DecisionAllow) != tc.ExpectAllowed {
 			t.Errorf("%d: expected %v, got %v", i, tc.ExpectAllowed, decision)
 		}
@@ -309,12 +350,12 @@ func BenchmarkPopulationAllocation(b *testing.B) {
 		uniquePVCsPerPod:       1,
 	}
 
-	pods, pvs, attachments := generate(opts)
+	nodes, pods, pvs, attachments := generate(opts)
 	b.ResetTimer()
 
 	for i := 0; i < b.N; i++ {
 		g := NewGraph()
-		populate(g, pods, pvs, attachments)
+		populate(g, nodes, pods, pvs, attachments)
 	}
 }
 
@@ -340,14 +381,14 @@ func BenchmarkPopulationRetention(b *testing.B) {
 		uniquePVCsPerPod:       1,
 	}
 
-	pods, pvs, attachments := generate(opts)
+	nodes, pods, pvs, attachments := generate(opts)
 	// Garbage collect before the first iteration
 	runtime.GC()
 	b.ResetTimer()
 
 	for i := 0; i < b.N; i++ {
 		g := NewGraph()
-		populate(g, pods, pvs, attachments)
+		populate(g, nodes, pods, pvs, attachments)
 
 		if i == 0 {
 			f, _ := os.Create("BenchmarkPopulationRetention.profile")
@@ -375,8 +416,8 @@ func BenchmarkAuthorization(b *testing.B) {
 		sharedPVCsPerPod:       0,
 		uniquePVCsPerPod:       1,
 	}
-	pods, pvs, attachments := generate(opts)
+	nodes, pods, pvs, attachments := generate(opts)
-	populate(g, pods, pvs, attachments)
+	populate(g, nodes, pods, pvs, attachments)
 
 	identifier := nodeidentifier.NewDefaultNodeIdentifier()
 	authz := NewAuthorizer(g, identifier, bootstrappolicy.NodeRules()).(*NodeAuthorizer)
@@ -389,6 +430,11 @@ func BenchmarkAuthorization(b *testing.B) {
 		expect   authorizer.Decision
 		features utilfeature.FeatureGate
 	}{
+		{
+			name:   "allowed node configmap",
+			attrs:  authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "configmaps", Name: "node0-configmap", Namespace: "ns0"},
+			expect: authorizer.DecisionAllow,
+		},
 		{
 			name:   "allowed configmap",
 			attrs:  authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "configmaps", Name: "configmap0-pod0-node0", Namespace: "ns0"},
@@ -404,6 +450,12 @@ func BenchmarkAuthorization(b *testing.B) {
 			attrs:  authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "secrets", Name: "secret0-shared", Namespace: "ns0"},
 			expect: authorizer.DecisionAllow,
 		},
+
+		{
+			name:   "disallowed node configmap",
+			attrs:  authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "configmaps", Name: "node1-configmap", Namespace: "ns0"},
+			expect: authorizer.DecisionNoOpinion,
+		},
 		{
 			name:   "disallowed configmap",
 			attrs:  authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "configmaps", Name: "configmap0-pod0-node1", Namespace: "ns0"},
@@ -467,9 +519,12 @@ func BenchmarkAuthorization(b *testing.B) {
 	}
 }
 
-func populate(graph *Graph, pods []*api.Pod, pvs []*api.PersistentVolume, attachments []*storagev1beta1.VolumeAttachment) {
+func populate(graph *Graph, nodes []*api.Node, pods []*api.Pod, pvs []*api.PersistentVolume, attachments []*storagev1beta1.VolumeAttachment) {
 	p := &graphPopulator{}
 	p.graph = graph
+	for _, node := range nodes {
+		p.addNode(node)
+	}
 	for _, pod := range pods {
 		p.addPod(pod)
 	}
@@ -485,7 +540,8 @@ func populate(graph *Graph, pods []*api.Pod, pvs []*api.PersistentVolume, attach
 // the secret/configmap/pvc/node references in the pod and pv objects are named to indicate the connections between the objects.
 // for example, secret0-pod0-node0 is a secret referenced by pod0 which is bound to node0.
 // when populated into the graph, the node authorizer should allow node0 to access that secret, but not node1.
-func generate(opts sampleDataOpts) ([]*api.Pod, []*api.PersistentVolume, []*storagev1beta1.VolumeAttachment) {
+func generate(opts sampleDataOpts) ([]*api.Node, []*api.Pod, []*api.PersistentVolume, []*storagev1beta1.VolumeAttachment) {
+	nodes := make([]*api.Node, 0, opts.nodes)
 	pods := make([]*api.Pod, 0, opts.nodes*opts.podsPerNode)
 	pvs := make([]*api.PersistentVolume, 0, (opts.nodes*opts.podsPerNode*opts.uniquePVCsPerPod)+(opts.sharedPVCsPerPod*opts.namespaces))
 	attachments := make([]*storagev1beta1.VolumeAttachment, 0, opts.nodes*opts.attachmentsPerNode)
@@ -552,6 +608,20 @@ func generate(opts sampleDataOpts) ([]*api.Pod, []*api.PersistentVolume, []*stor
 			attachment.Spec.NodeName = nodeName
 			attachments = append(attachments, attachment)
 		}
+
+		name := fmt.Sprintf("%s-configmap", nodeName)
+		nodes = append(nodes, &api.Node{
+			ObjectMeta: metav1.ObjectMeta{Name: nodeName},
+			Spec: api.NodeSpec{
+				ConfigSource: &api.NodeConfigSource{
+					ConfigMapRef: &api.ObjectReference{
+						Name:      name,
+						Namespace: "ns0",
+						UID:       types.UID(fmt.Sprintf("ns0-%s", name)),
+					},
+				},
+			},
+		})
 	}
-	return pods, pvs, attachments
+	return nodes, pods, pvs, attachments
 }

test/e2e/auth/node_authz.go

@@ -74,11 +74,33 @@ var _ = SIGDescribe("[Feature:NodeAuthorizer]", func() {
 		Expect(apierrors.IsForbidden(err)).Should(Equal(true))
 	})
 
-	It("Getting an existent secret should exit with the Forbidden error", func() {
+	It("Getting an existing secret should exit with the Forbidden error", func() {
 		_, err := c.CoreV1().Secrets(ns).Get(defaultSaSecret, metav1.GetOptions{})
 		Expect(apierrors.IsForbidden(err)).Should(Equal(true))
 	})
 
+	It("Getting a non-existent configmap should exit with the Forbidden error, not a NotFound error", func() {
+		_, err := c.CoreV1().ConfigMaps(ns).Get("foo", metav1.GetOptions{})
+		Expect(apierrors.IsForbidden(err)).Should(Equal(true))
+	})
+
+	It("Getting an existing configmap should exit with the Forbidden error", func() {
+		By("Create a configmap for testing")
+		configmap := &v1.ConfigMap{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: ns,
+				Name:      "node-auth-configmap",
+			},
+			Data: map[string]string{
+				"data": "content",
+			},
+		}
+		_, err := f.ClientSet.CoreV1().ConfigMaps(ns).Create(configmap)
+		Expect(err).NotTo(HaveOccurred())
+		_, err = c.CoreV1().ConfigMaps(ns).Get(configmap.Name, metav1.GetOptions{})
+		Expect(apierrors.IsForbidden(err)).Should(Equal(true))
+	})
+
 	It("Getting a secret for a workload the node has access to should succeed", func() {
 		By("Create a secret for testing")
 		secret := &v1.Secret{
@@ -138,7 +160,7 @@ var _ = SIGDescribe("[Feature:NodeAuthorizer]", func() {
 		Expect(err).NotTo(HaveOccurred())
 	})
 
-	It("A node shouldn't be able to create an other node", func() {
+	It("A node shouldn't be able to create another node", func() {
 		node := &v1.Node{
 			ObjectMeta: metav1.ObjectMeta{Name: "foo"},
 			TypeMeta: metav1.TypeMeta{
@@ -151,7 +173,7 @@ var _ = SIGDescribe("[Feature:NodeAuthorizer]", func() {
 		Expect(apierrors.IsForbidden(err)).Should(Equal(true))
 	})
 
-	It("A node shouldn't be able to delete an other node", func() {
+	It("A node shouldn't be able to delete another node", func() {
 		By(fmt.Sprintf("Create node foo by user: %v", asUser))
 		err := c.CoreV1().Nodes().Delete("foo", &metav1.DeleteOptions{})
 		Expect(apierrors.IsForbidden(err)).Should(Equal(true))

test/e2e_node/dynamic_kubelet_config_test.go

@@ -19,6 +19,7 @@ package e2e_node
 import (
 	"fmt"
 	"reflect"
+	"strings"
 	"time"
 
 	"github.com/davecgh/go-spew/spew"
@@ -40,6 +41,8 @@ type configState struct {
 	configSource   *apiv1.NodeConfigSource
 	expectConfigOk *apiv1.NodeCondition
 	expectConfig   *kubeletconfig.KubeletConfiguration
+	// whether to expect this substring in an error returned from the API server when updating the config source
+	apierr string
 	// whether the state would cause a config change event as a result of the update to Node.Spec.ConfigSource,
 	// assuming that the current source would have also caused a config change event.
 	// for example, some malformed references may result in a download failure, in which case the Kubelet
@@ -132,25 +135,16 @@ var _ = framework.KubeDescribe("DynamicKubeletConfiguration [Feature:DynamicKube
 
 					// Node.Spec.ConfigSource has all nil subfields
 					{desc: "Node.Spec.ConfigSource has all nil subfields",
-						configSource: &apiv1.NodeConfigSource{ConfigMapRef: nil},
+						configSource: &apiv1.NodeConfigSource{},
-						expectConfigOk: &apiv1.NodeCondition{Type: apiv1.NodeKubeletConfigOk, Status: apiv1.ConditionFalse,
+						apierr:       "exactly one reference subfield must be non-nil",
-							Message: "",
-							Reason:  fmt.Sprintf(status.FailSyncReasonFmt, status.FailSyncReasonAllNilSubfields)},
-						expectConfig: nil,
-						event:        false,
 					},
 
 					// Node.Spec.ConfigSource.ConfigMapRef is partial
 					{desc: "Node.Spec.ConfigSource.ConfigMapRef is partial",
-						// TODO(mtaufen): check the other 7 partials in a unit test
 						configSource: &apiv1.NodeConfigSource{ConfigMapRef: &apiv1.ObjectReference{
 							UID:  "foo",
 							Name: "bar"}}, // missing Namespace
-						expectConfigOk: &apiv1.NodeCondition{Type: apiv1.NodeKubeletConfigOk, Status: apiv1.ConditionFalse,
+						apierr: "name, namespace, and UID must all be non-empty",
-							Message: "",
-							Reason:  fmt.Sprintf(status.FailSyncReasonFmt, status.FailSyncReasonPartialObjectReference)},
-						expectConfig: nil,
-						event:        false,
 					},
 
 					// Node.Spec.ConfigSource's UID does not align with namespace/name
@@ -348,10 +342,20 @@ func setAndTestKubeletConfigState(f *framework.Framework, state *configState, ex
 	// set the desired state, retry a few times in case we are competing with other editors
 	Eventually(func() error {
 		if err := setNodeConfigSource(f, state.configSource); err != nil {
-			return fmt.Errorf("case %s: error setting Node.Spec.ConfigSource", err)
+			if len(state.apierr) == 0 {
+				return fmt.Errorf("case %s: expect nil error but got %q", state.desc, err.Error())
+			} else if !strings.Contains(err.Error(), state.apierr) {
+				return fmt.Errorf("case %s: expect error to contain %q but got %q", state.desc, state.apierr, err.Error())
+			}
+		} else if len(state.apierr) > 0 {
+			return fmt.Errorf("case %s: expect error to contain %q but got nil error", state.desc, state.apierr)
 		}
 		return nil
 	}, time.Minute, time.Second).Should(BeNil())
+	// skip further checks if we expected an API error
+	if len(state.apierr) > 0 {
+		return
+	}
 	// check that config source actually got set to what we expect
 	checkNodeConfigSource(f, state.desc, state.configSource)
 	// check condition
@@ -391,7 +395,6 @@ func checkConfigOkCondition(f *framework.Framework, desc string, expect *apiv1.N
 		timeout  = time.Minute
 		interval = time.Second
 	)
-
 	Eventually(func() error {
 		node, err := f.ClientSet.CoreV1().Nodes().Get(framework.TestContext.NodeName, metav1.GetOptions{})
 		if err != nil {

test/integration/auth/node_test.go

@@ -81,6 +81,9 @@ func TestNodeAuthorizer(t *testing.T) {
 	// Enabled CSIPersistentVolume feature at startup so volumeattachments get watched
 	defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIPersistentVolume, true)()
 
+	// Enable DynamicKubeletConfig feature so that Node.Spec.ConfigSource can be set
+	defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DynamicKubeletConfig, true)()
+
 	// Set up Node+RBAC authorizer
 	authorizerConfig := &authorizer.AuthorizationConfig{
 		AuthorizationModes:       []string{"Node", "RBAC"},
@@ -135,6 +138,9 @@ func TestNodeAuthorizer(t *testing.T) {
 	if _, err := superuserClient.Core().ConfigMaps("ns").Create(&api.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "myconfigmap"}}); err != nil {
 		t.Fatal(err)
 	}
+	if _, err := superuserClient.Core().ConfigMaps("ns").Create(&api.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "myconfigmapconfigsource"}}); err != nil {
+		t.Fatal(err)
+	}
 	pvName := "mypv"
 	if _, err := superuserClientExternal.StorageV1beta1().VolumeAttachments().Create(&storagev1beta1.VolumeAttachment{
 		ObjectMeta: metav1.ObjectMeta{Name: "myattachment"},
@@ -186,6 +192,12 @@ func TestNodeAuthorizer(t *testing.T) {
 			return err
 		}
 	}
+	getConfigMapConfigSource := func(client clientset.Interface) func() error {
+		return func() error {
+			_, err := client.Core().ConfigMaps("ns").Get("myconfigmapconfigsource", metav1.GetOptions{})
+			return err
+		}
+	}
 	getPVC := func(client clientset.Interface) func() error {
 		return func() error {
 			_, err := client.Core().PersistentVolumeClaims("ns").Get("mypvc", metav1.GetOptions{})
@@ -267,6 +279,36 @@ func TestNodeAuthorizer(t *testing.T) {
 			return err
 		}
 	}
+	setNode2ConfigSource := func(client clientset.Interface) func() error {
+		return func() error {
+			node2, err := client.Core().Nodes().Get("node2", metav1.GetOptions{})
+			if err != nil {
+				return err
+			}
+			node2.Spec.ConfigSource = &api.NodeConfigSource{
+				ConfigMapRef: &api.ObjectReference{
+					Namespace: "ns",
+					Name:      "myconfigmapconfigsource",
+					// validation just requires UID to be non-empty and it isn't necessary for GET,
+					// so we just use a bogus one for the test
+					UID: "uid",
+				},
+			}
+			_, err = client.Core().Nodes().Update(node2)
+			return err
+		}
+	}
+	unsetNode2ConfigSource := func(client clientset.Interface) func() error {
+		return func() error {
+			node2, err := client.Core().Nodes().Get("node2", metav1.GetOptions{})
+			if err != nil {
+				return err
+			}
+			node2.Spec.ConfigSource = nil
+			_, err = client.Core().Nodes().Update(node2)
+			return err
+		}
+	}
 	updateNode2Status := func(client clientset.Interface) func() error {
 		return func() error {
 			_, err := client.Core().Nodes().UpdateStatus(&api.Node{
@@ -420,6 +462,7 @@ func TestNodeAuthorizer(t *testing.T) {
 	expectAllowed(t, deleteNode2NormalPod(node2Client))
 	expectAllowed(t, createNode2MirrorPod(node2Client))
 	expectAllowed(t, deleteNode2MirrorPod(node2Client))
+
 	// recreate as an admin to test eviction
 	expectAllowed(t, createNode2NormalPod(superuserClient))
 	expectAllowed(t, createNode2MirrorPod(superuserClient))
@@ -449,6 +492,25 @@ func TestNodeAuthorizer(t *testing.T) {
 	expectForbidden(t, getVolumeAttachment(node1ClientExternal))
 	expectAllowed(t, getVolumeAttachment(node2ClientExternal))
 
+	// create node2 again
+	expectAllowed(t, createNode2(node2Client))
+	// node2 can not set its own config source
+	expectForbidden(t, setNode2ConfigSource(node2Client))
+	// node2 can not access the configmap config source yet
+	expectForbidden(t, getConfigMapConfigSource(node2Client))
+	// superuser can access the configmap config source
+	expectAllowed(t, getConfigMapConfigSource(superuserClient))
+	// superuser can set node2's config source
+	expectAllowed(t, setNode2ConfigSource(superuserClient))
+	// node2 can now get the configmap assigned as its config source
+	expectAllowed(t, getConfigMapConfigSource(node2Client))
+	// superuser can unset node2's config source
+	expectAllowed(t, unsetNode2ConfigSource(superuserClient))
+	// node2 can no longer get the configmap after it is unassigned as its config source
+	expectForbidden(t, getConfigMapConfigSource(node2Client))
+	// clean up node2
+	expectAllowed(t, deleteNode2(node2Client))
+
 	//TODO(mikedanese): integration test node restriction of TokenRequest
 }