Merge pull request #49638 from liggitt/remove-nodes-binding

Automatic merge from submit-queue (batch tested with PRs 49619, 49598, 47267, 49597, 49638)

Remove default binding of system:node role to system:nodes group

part of https://github.com/kubernetes/features/issues/279

deprecation of this automatic binding announced in 1.7 in https://github.com/kubernetes/kubernetes/pull/46076

```release-note
RBAC: the `system:node` role is no longer automatically granted to the `system:nodes` group in new clusters. It is recommended that nodes be authorized using the `Node` authorization mode instead. Installations that wish to continue giving all members of the `system:nodes` group the `system:node` role (which grants broad read access, including all secrets and configmaps) must create an installation-specific `ClusterRoleBinding`.
```

pkg/kubeapiserver/authorizer/config.go

@@ -115,9 +115,6 @@ func (config AuthorizationConfig) New() (authorizer.Authorizer, error) {
 			nodeAuthorizer := node.NewAuthorizer(graph, nodeidentifier.NewDefaultNodeIdentifier(), bootstrappolicy.NodeRules())
 			authorizers = append(authorizers, nodeAuthorizer)
 
-			// Don't bind system:nodes to the system:node role
-			bootstrappolicy.AddClusterRoleBindingFilter(bootstrappolicy.OmitNodesGroupBinding)
-
 		case modes.ModeAlwaysAllow:
 			authorizers = append(authorizers, authorizerfactory.NewAlwaysAllowAuthorizer())
 		case modes.ModeAlwaysDeny:

plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go

@@ -366,38 +366,8 @@ func ClusterRoles() []rbac.ClusterRole {
 	return roles
 }
 
-// ClusterRoleBindingFilter can modify and return or omit (by returning nil) a role binding
-type ClusterRoleBindingFilter func(*rbac.ClusterRoleBinding) *rbac.ClusterRoleBinding
-
-// AddClusterRoleBindingFilter adds the given filter to the list that is invoked when determing bootstrap roles to reconcile.
-func AddClusterRoleBindingFilter(filter ClusterRoleBindingFilter) {
-	clusterRoleBindingFilters = append(clusterRoleBindingFilters, filter)
-}
-
-// ClearClusterRoleBindingFilters removes any filters added using AddClusterRoleBindingFilter
-func ClearClusterRoleBindingFilters() {
-	clusterRoleBindingFilters = nil
-}
-
 const systemNodeRoleName = "system:node"
 
-var clusterRoleBindingFilters []ClusterRoleBindingFilter
-
-// OmitNodesGroupBinding is a filter that omits the deprecated binding for the system:nodes group to the system:node role.
-var OmitNodesGroupBinding = ClusterRoleBindingFilter(func(binding *rbac.ClusterRoleBinding) *rbac.ClusterRoleBinding {
-	if binding.RoleRef.Name == systemNodeRoleName {
-		subjects := []rbac.Subject{}
-		for _, subject := range binding.Subjects {
-			if subject.Kind == rbac.GroupKind && subject.Name == user.NodesGroup {
-				continue
-			}
-			subjects = append(subjects, subject)
-		}
-		binding.Subjects = subjects
-	}
-	return binding
-})
-
 // ClusterRoleBindings return default rolebindings to the default roles
 func ClusterRoleBindings() []rbac.ClusterRoleBinding {
 	rolebindings := []rbac.ClusterRoleBinding{
@@ -409,27 +379,15 @@ func ClusterRoleBindings() []rbac.ClusterRoleBinding {
 		rbac.NewClusterBinding("system:kube-dns").SAs("kube-system", "kube-dns").BindingOrDie(),
 		rbac.NewClusterBinding("system:kube-scheduler").Users(user.KubeScheduler).BindingOrDie(),
 
-		// This default system:nodes binding is deprecated in 1.7 with the availability of the Node authorizer.
+		// This default binding of the system:node role to the system:nodes group is deprecated in 1.7 with the availability of the Node authorizer.
-		// If an admin wants to grant the system:node role (which cannot partition Node API access), they will need to create their own clusterrolebinding.
+		// This leaves the binding, but with an empty set of subjects, so that tightening reconciliation can remove the subject.
-		// TODO: Remove the subjects from this binding in 1.8 (leave the empty binding for tightening reconciliation), and remove AddClusterRoleBindingFilter()
+		{
-		rbac.NewClusterBinding(systemNodeRoleName).Groups(user.NodesGroup).BindingOrDie(),
+			ObjectMeta: metav1.ObjectMeta{Name: systemNodeRoleName},
+			RoleRef:    rbac.RoleRef{APIGroup: rbac.GroupName, Kind: "ClusterRole", Name: systemNodeRoleName},
+		},
 	}
 
 	addClusterRoleBindingLabel(rolebindings)
 
-	retval := []rbac.ClusterRoleBinding{}
+	return rolebindings
-	for i := range rolebindings {
-		binding := &rolebindings[i]
-		for _, filter := range clusterRoleBindingFilters {
-			binding = filter(binding)
-			if binding == nil {
-				break
-			}
-		}
-		if binding != nil {
-			retval = append(retval, *binding)
-		}
-	}
-
-	return retval
 }

plugin/pkg/auth/authorizer/rbac/bootstrappolicy/testdata/cluster-role-bindings.yaml

@@ -121,10 +121,7 @@ items:
     apiGroup: rbac.authorization.k8s.io
     kind: ClusterRole
     name: system:node
-  subjects:
+  subjects: []
-  - apiGroup: rbac.authorization.k8s.io
-    kind: Group
-    name: system:nodes
 - apiVersion: rbac.authorization.k8s.io/v1beta1
   kind: ClusterRoleBinding
   metadata:

test/integration/auth/BUILD

@@ -49,7 +49,6 @@ go_test(
         "//plugin/pkg/admission/noderestriction:go_default_library",
         "//plugin/pkg/auth/authenticator/token/bootstrap:go_default_library",
         "//plugin/pkg/auth/authorizer/rbac:go_default_library",
-        "//plugin/pkg/auth/authorizer/rbac/bootstrappolicy:go_default_library",
         "//test/e2e/lifecycle/bootstrap:go_default_library",
         "//test/integration:go_default_library",
         "//test/integration/framework:go_default_library",

test/integration/auth/node_test.go

@@ -38,7 +38,6 @@ import (
 	informers "k8s.io/kubernetes/pkg/client/informers/informers_generated/internalversion"
 	"k8s.io/kubernetes/pkg/kubeapiserver/authorizer"
 	"k8s.io/kubernetes/plugin/pkg/admission/noderestriction"
-	"k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac/bootstrappolicy"
 	"k8s.io/kubernetes/test/integration/framework"
 )
 
@@ -79,7 +78,6 @@ func TestNodeAuthorizer(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer bootstrappolicy.ClearClusterRoleBindingFilters()
 
 	// Set up NodeRestriction admission
 	nodeRestrictionAdmission := noderestriction.NewPlugin(nodeidentifier.NewDefaultNodeIdentifier())