Merge pull request #33294 from deads2k/rbac-08-compact-init

pkg/apis/rbac/helpers.go

@@ -17,6 +17,7 @@ limitations under the License.
 package rbac
 
 import (
+	"fmt"
 	"strings"
 
 	"k8s.io/kubernetes/pkg/api/unversioned"
@@ -94,3 +95,70 @@ func NonResourceURLMatches(rule PolicyRule, requestedURL string) bool {
 
 	return false
 }
+
+// +k8s:deepcopy-gen=false
+// PolicyRuleBuilder let's us attach methods.  A no-no for API types.
+// We use it to construct rules in code.  It's more compact than trying to write them
+// out in a literal and allows us to perform some basic checking during construction
+type PolicyRuleBuilder struct {
+	PolicyRule PolicyRule
+}
+
+func NewRule(verbs ...string) *PolicyRuleBuilder {
+	return &PolicyRuleBuilder{
+		PolicyRule: PolicyRule{Verbs: verbs},
+	}
+}
+
+func (r *PolicyRuleBuilder) Groups(groups ...string) *PolicyRuleBuilder {
+	r.PolicyRule.APIGroups = append(r.PolicyRule.APIGroups, groups...)
+	return r
+}
+
+func (r *PolicyRuleBuilder) Resources(resources ...string) *PolicyRuleBuilder {
+	r.PolicyRule.Resources = append(r.PolicyRule.Resources, resources...)
+	return r
+}
+
+func (r *PolicyRuleBuilder) Names(names ...string) *PolicyRuleBuilder {
+	r.PolicyRule.ResourceNames = append(r.PolicyRule.ResourceNames, names...)
+	return r
+}
+
+func (r *PolicyRuleBuilder) URLs(urls ...string) *PolicyRuleBuilder {
+	r.PolicyRule.NonResourceURLs = append(r.PolicyRule.NonResourceURLs, urls...)
+	return r
+}
+
+func (r *PolicyRuleBuilder) RuleOrDie() PolicyRule {
+	ret, err := r.Rule()
+	if err != nil {
+		panic(err)
+	}
+	return ret
+}
+
+func (r *PolicyRuleBuilder) Rule() (PolicyRule, error) {
+	if len(r.PolicyRule.Verbs) == 0 {
+		return PolicyRule{}, fmt.Errorf("verbs are required: %#v", r.PolicyRule)
+	}
+
+	switch {
+	case len(r.PolicyRule.NonResourceURLs) > 0:
+		if len(r.PolicyRule.APIGroups) != 0 || len(r.PolicyRule.Resources) != 0 || len(r.PolicyRule.ResourceNames) != 0 {
+			return PolicyRule{}, fmt.Errorf("non-resource rule may not have apiGroups, resources, or resourceNames: %#v", r.PolicyRule)
+		}
+	case len(r.PolicyRule.Resources) > 0:
+		if len(r.PolicyRule.NonResourceURLs) != 0 {
+			return PolicyRule{}, fmt.Errorf("resource rule may not have nonResourceURLs: %#v", r.PolicyRule)
+		}
+		if len(r.PolicyRule.APIGroups) == 0 {
+			// this a common bug
+			return PolicyRule{}, fmt.Errorf("resource rule must have apiGroups: %#v", r.PolicyRule)
+		}
+	default:
+		return PolicyRule{}, fmt.Errorf("a rule must have either nonResourceURLs or resources: %#v", r.PolicyRule)
+	}
+
+	return r.PolicyRule, nil
+}

pkg/registry/rbac/rest/storage_rbac.go

@@ -122,7 +122,7 @@ func newPostStartHook(directClusterRoleAccess *clusterroleetcd.REST) genericapis
 			return nil
 		}
 
-		for _, clusterRole := range bootstrappolicy.ClusterRoles() {
+		for _, clusterRole := range append(bootstrappolicy.ClusterRoles(), bootstrappolicy.ControllerRoles()...) {
 			if _, err := directClusterRoleAccess.Create(ctx, &clusterRole); err != nil {
 				// don't fail on failures, try to create as many as you can
 				utilruntime.HandleError(fmt.Errorf("unable to initialize clusterroles: %v", err))

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

+/*
+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 bootstrappolicy
+
+import (
+	"strings"
+
+	"github.com/golang/glog"
+
+	"k8s.io/kubernetes/pkg/api"
+	rbac "k8s.io/kubernetes/pkg/apis/rbac"
+)
+
+var (
+	// controllerRoles is a slice of roles used for controllers
+	controllerRoles = []rbac.ClusterRole{}
+)
+
+func addControllerRole(role rbac.ClusterRole) {
+	if !strings.HasPrefix(role.Name, "system:controller:") {
+		glog.Fatalf(`role %q must start with "system:controller:"`, role.Name)
+	}
+
+	for _, existingRole := range controllerRoles {
+		if role.Name == existingRole.Name {
+			glog.Fatalf("role %q was already registered", role.Name)
+		}
+	}
+
+	controllerRoles = append(controllerRoles, role)
+}
+
+func eventsRule() rbac.PolicyRule {
+	return rbac.NewRule("create", "update", "patch").Groups(legacyGroup).Resources("events").RuleOrDie()
+}
+
+func init() {
+	addControllerRole(rbac.ClusterRole{
+		ObjectMeta: api.ObjectMeta{Name: "system:controller:replication-controller"},
+		Rules: []rbac.PolicyRule{
+			rbac.NewRule("get", "list", "watch", "update").Groups(legacyGroup).Resources("replicationcontrollers").RuleOrDie(),
+			rbac.NewRule("update").Groups(legacyGroup).Resources("replicationcontrollers/status").RuleOrDie(),
+			rbac.NewRule("list", "watch", "create", "delete").Groups(legacyGroup).Resources("pods").RuleOrDie(),
+			eventsRule(),
+		},
+	})
+}
+
+// ControllerRoles returns the cluster roles used by controllers
+func ControllerRoles() []rbac.ClusterRole {
+	return controllerRoles
+}

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

@@ -18,18 +18,24 @@ package bootstrappolicy
 
 import (
 	"k8s.io/kubernetes/pkg/api"
-	rbacapi "k8s.io/kubernetes/pkg/apis/rbac"
+	rbac "k8s.io/kubernetes/pkg/apis/rbac"
+)
+
+var (
+	readWrite = []string{"get", "list", "watch", "create", "update", "patch", "delete", "deletecollection"}
+	read      = []string{"get", "list", "watch"}
+
+	legacyGroup = ""
 )
 
 // ClusterRoles returns the cluster roles to bootstrap an API server with
-func ClusterRoles() []rbacapi.ClusterRole {
-	return []rbacapi.ClusterRole{
-		// TODO update the expression of these rules to match openshift for ease of inspection
+func ClusterRoles() []rbac.ClusterRole {
+	return []rbac.ClusterRole{
 		{
 			ObjectMeta: api.ObjectMeta{Name: "cluster-admin"},
-			Rules: []rbacapi.PolicyRule{
-				{Verbs: []string{"*"}, APIGroups: []string{"*"}, Resources: []string{"*"}},
-				{Verbs: []string{"*"}, NonResourceURLs: []string{"*"}},
+			Rules: []rbac.PolicyRule{
+				rbac.NewRule("*").Groups("*").Resources("*").RuleOrDie(),
+				rbac.NewRule("*").URLs("*").RuleOrDie(),
 			},
 		},
 	}

test/integration/auth/rbac_test.go

@@ -19,8 +19,6 @@ limitations under the License.
 package auth
 
 import (
-	"bytes"
-	"encoding/json"
 	"errors"
 	"fmt"
 	"io"
@@ -35,10 +33,7 @@ import (
 
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/api/testapi"
-	"k8s.io/kubernetes/pkg/api/unversioned"
-	"k8s.io/kubernetes/pkg/api/v1"
 	rbacapi "k8s.io/kubernetes/pkg/apis/rbac"
-	"k8s.io/kubernetes/pkg/apis/rbac/v1alpha1"
 	"k8s.io/kubernetes/pkg/auth/authenticator"
 	"k8s.io/kubernetes/pkg/auth/authenticator/bearertoken"
 	"k8s.io/kubernetes/pkg/auth/authorizer"
@@ -80,6 +75,12 @@ func clientForUser(user string) *http.Client {
 	}
 }
 
+func clientsetForUser(user string, config *restclient.Config) clientset.Interface {
+	configCopy := *config
+	configCopy.BearerToken = user
+	return clientset.NewForConfigOrDie(&configCopy)
+}
+
 func newRBACAuthorizer(t *testing.T, superUser string, config *master.Config) authorizer.Authorizer {
 	newRESTOptions := func(resource string) generic.RESTOptions {
 		storageConfig, err := config.StorageFactory.NewConfig(rbacapi.Resource(resource))
@@ -98,72 +99,41 @@ func newRBACAuthorizer(t *testing.T, superUser string, config *master.Config) au
 
 // bootstrapRoles are a set of RBAC roles which will be populated before the test.
 type bootstrapRoles struct {
-	roles               []v1alpha1.Role
-	roleBindings        []v1alpha1.RoleBinding
-	clusterRoles        []v1alpha1.ClusterRole
-	clusterRoleBindings []v1alpha1.ClusterRoleBinding
+	roles               []rbacapi.Role
+	roleBindings        []rbacapi.RoleBinding
+	clusterRoles        []rbacapi.ClusterRole
+	clusterRoleBindings []rbacapi.ClusterRoleBinding
 }
 
 // bootstrap uses the provided client to create the bootstrap roles and role bindings.
 //
 // client should be authenticated as the RBAC super user.
-func (b bootstrapRoles) bootstrap(client *http.Client, serverURL string) error {
-	newReq := func(resource, name, namespace string, v interface{}) *http.Request {
-		body, err := json.Marshal(v)
-		if err != nil {
-			panic(err)
-		}
-		path := testapi.Rbac.ResourcePath(resource, namespace, name)
-		req, err := http.NewRequest("PUT", serverURL+path, bytes.NewReader(body))
+func (b bootstrapRoles) bootstrap(client clientset.Interface) error {
+	for _, r := range b.clusterRoles {
+		_, err := client.Rbac().ClusterRoles().Create(&r)
 		if err != nil {
-			panic(err)
+			return fmt.Errorf("failed to make request: %v", err)
 		}
-		req.Header.Set("Content-Type", "application/json")
-		req.ContentLength = int64(len(body))
-		return req
-	}
-
-	apiVersion := v1alpha1.SchemeGroupVersion.String()
-
-	var requests []*http.Request
-	for _, r := range b.clusterRoles {
-		r.TypeMeta = unversioned.TypeMeta{Kind: "ClusterRole", APIVersion: apiVersion}
-		requests = append(requests, newReq("clusterroles", r.Name, r.Namespace, r))
 	}
 	for _, r := range b.roles {
-		r.TypeMeta = unversioned.TypeMeta{Kind: "Role", APIVersion: apiVersion}
-		requests = append(requests, newReq("roles", r.Name, r.Namespace, r))
+		_, err := client.Rbac().Roles(r.Namespace).Create(&r)
+		if err != nil {
+			return fmt.Errorf("failed to make request: %v", err)
+		}
 	}
 	for _, r := range b.clusterRoleBindings {
-		r.TypeMeta = unversioned.TypeMeta{Kind: "ClusterRoleBinding", APIVersion: apiVersion}
-		requests = append(requests, newReq("clusterrolebindings", r.Name, r.Namespace, r))
+		_, err := client.Rbac().ClusterRoleBindings().Create(&r)
+		if err != nil {
+			return fmt.Errorf("failed to make request: %v", err)
+		}
 	}
 	for _, r := range b.roleBindings {
-		r.TypeMeta = unversioned.TypeMeta{Kind: "RoleBinding", APIVersion: apiVersion}
-		requests = append(requests, newReq("rolebindings", r.Name, r.Namespace, r))
-	}
-
-	for _, req := range requests {
-		err := func() error {
-			resp, err := client.Do(req)
-			if err != nil {
-				return fmt.Errorf("failed to make request: %v", err)
-			}
-			defer resp.Body.Close()
-
-			if resp.StatusCode != http.StatusCreated {
-				body, err := ioutil.ReadAll(resp.Body)
-				if err != nil {
-					return fmt.Errorf("failed to read body: %v", err)
-				}
-				return fmt.Errorf("POST %s: expected %d got %s\n%s", req.URL, resp.Status, body)
-			}
-			return nil
-		}()
+		_, err := client.Rbac().RoleBindings(r.Namespace).Create(&r)
 		if err != nil {
-			return err
+			return fmt.Errorf("failed to make request: %v", err)
 		}
 	}
+
 	return nil
 }
 
@@ -263,23 +233,9 @@ var (
 
 // Declare some PolicyRules beforehand.
 var (
-	ruleAllowAll = v1alpha1.PolicyRule{
-		Verbs:     []string{"*"},
-		APIGroups: []string{"*"},
-		Resources: []string{"*"},
-	}
-
-	ruleReadPods = v1alpha1.PolicyRule{
-		Verbs:     []string{"list", "get", "watch"},
-		APIGroups: []string{""},
-		Resources: []string{"pods"},
-	}
-
-	ruleWriteJobs = v1alpha1.PolicyRule{
-		Verbs:     []string{"*"},
-		APIGroups: []string{"batch"},
-		Resources: []string{"*"},
-	}
+	ruleAllowAll  = rbacapi.NewRule("*").Groups("*").Resources("*").RuleOrDie()
+	ruleReadPods  = rbacapi.NewRule("list", "get", "watch").Groups("").Resources("pods").RuleOrDie()
+	ruleWriteJobs = rbacapi.NewRule("*").Groups("batch").Resources("*").RuleOrDie()
 )
 
 func TestRBAC(t *testing.T) {
@@ -292,23 +248,23 @@ func TestRBAC(t *testing.T) {
 	}{
 		{
 			bootstrapRoles: bootstrapRoles{
-				clusterRoles: []v1alpha1.ClusterRole{
+				clusterRoles: []rbacapi.ClusterRole{
 					{
-						ObjectMeta: v1.ObjectMeta{Name: "allow-all"},
-						Rules:      []v1alpha1.PolicyRule{ruleAllowAll},
+						ObjectMeta: api.ObjectMeta{Name: "allow-all"},
+						Rules:      []rbacapi.PolicyRule{ruleAllowAll},
 					},
 					{
-						ObjectMeta: v1.ObjectMeta{Name: "read-pods"},
-						Rules:      []v1alpha1.PolicyRule{ruleReadPods},
+						ObjectMeta: api.ObjectMeta{Name: "read-pods"},
+						Rules:      []rbacapi.PolicyRule{ruleReadPods},
 					},
 				},
-				clusterRoleBindings: []v1alpha1.ClusterRoleBinding{
+				clusterRoleBindings: []rbacapi.ClusterRoleBinding{
 					{
-						ObjectMeta: v1.ObjectMeta{Name: "read-pods"},
-						Subjects: []v1alpha1.Subject{
+						ObjectMeta: api.ObjectMeta{Name: "read-pods"},
+						Subjects: []rbacapi.Subject{
 							{Kind: "User", Name: "pod-reader"},
 						},
-						RoleRef: v1alpha1.RoleRef{Kind: "ClusterRole", Name: "read-pods"},
+						RoleRef: rbacapi.RoleRef{Kind: "ClusterRole", Name: "read-pods"},
 					},
 				},
 			},
@@ -330,24 +286,24 @@ func TestRBAC(t *testing.T) {
 		},
 		{
 			bootstrapRoles: bootstrapRoles{
-				clusterRoles: []v1alpha1.ClusterRole{
+				clusterRoles: []rbacapi.ClusterRole{
 					{
-						ObjectMeta: v1.ObjectMeta{Name: "write-jobs"},
-						Rules:      []v1alpha1.PolicyRule{ruleWriteJobs},
+						ObjectMeta: api.ObjectMeta{Name: "write-jobs"},
+						Rules:      []rbacapi.PolicyRule{ruleWriteJobs},
 					},
 				},
-				clusterRoleBindings: []v1alpha1.ClusterRoleBinding{
+				clusterRoleBindings: []rbacapi.ClusterRoleBinding{
 					{
-						ObjectMeta: v1.ObjectMeta{Name: "write-jobs"},
-						Subjects:   []v1alpha1.Subject{{Kind: "User", Name: "job-writer"}},
-						RoleRef:    v1alpha1.RoleRef{Kind: "ClusterRole", Name: "write-jobs"},
+						ObjectMeta: api.ObjectMeta{Name: "write-jobs"},
+						Subjects:   []rbacapi.Subject{{Kind: "User", Name: "job-writer"}},
+						RoleRef:    rbacapi.RoleRef{Kind: "ClusterRole", Name: "write-jobs"},
 					},
 				},
-				roleBindings: []v1alpha1.RoleBinding{
+				roleBindings: []rbacapi.RoleBinding{
 					{
-						ObjectMeta: v1.ObjectMeta{Name: "write-jobs", Namespace: "job-namespace"},
-						Subjects:   []v1alpha1.Subject{{Kind: "User", Name: "job-writer-namespace"}},
-						RoleRef:    v1alpha1.RoleRef{Kind: "ClusterRole", Name: "write-jobs"},
+						ObjectMeta: api.ObjectMeta{Name: "write-jobs", Namespace: "job-namespace"},
+						Subjects:   []rbacapi.Subject{{Kind: "User", Name: "job-writer-namespace"}},
+						RoleRef:    rbacapi.RoleRef{Kind: "ClusterRole", Name: "write-jobs"},
 					},
 				},
 			},
@@ -388,8 +344,10 @@ func TestRBAC(t *testing.T) {
 		_, s := framework.RunAMaster(masterConfig)
 		defer s.Close()
 
+		clientConfig := &restclient.Config{Host: s.URL, ContentConfig: restclient.ContentConfig{NegotiatedSerializer: api.Codecs}}
+
 		// Bootstrap the API Server with the test case's initial roles.
-		if err := tc.bootstrapRoles.bootstrap(clientForUser(superUser), s.URL); err != nil {
+		if err := tc.bootstrapRoles.bootstrap(clientsetForUser(superUser, clientConfig)); err != nil {
 			t.Errorf("case %d: failed to apply initial roles: %v", i, err)
 			continue
 		}