Merge pull request #16045 from mrogers950/rbac-15826

Automatic merge from submit-queue (batch tested with PRs 16228, 16200, 16253, 16249, 16045) Use rbac.PolicyRule directly for DiscoveryRule Fixes #15826
openshift · Sep 9, 2017 · a3d15d4 · a3d15d4
2 parents c02d392 + c69ca42
commit a3d15d4
Show file tree

Hide file tree

Showing 5 changed files with 28 additions and 35 deletions.
diff --git a/pkg/authorization/apis/authorization/types.go b/pkg/authorization/apis/authorization/types.go
@@ -40,9 +40,9 @@ const (
 )
 
 // DiscoveryRule is a rule that allows a client to discover the API resources available on this server
-var DiscoveryRule = PolicyRule{
-	Verbs: sets.NewString("get"),
-	NonResourceURLs: sets.NewString(
+var DiscoveryRule = rbac.PolicyRule{
+	Verbs: []string{"get"},
+	NonResourceURLs: []string{
 		// Server version checking
 		"/version", "/version/*",
 
@@ -56,14 +56,7 @@ var DiscoveryRule = PolicyRule{
 
 		// we intentionally allow all to here
 		"/",
-	),
-}
-
-// The Kubernetes Rbac version
-// TODO make this the authoritative rule
-var RbacDiscoveryRule = rbac.PolicyRule{
-	Verbs:           DiscoveryRule.Verbs.List(),
-	NonResourceURLs: DiscoveryRule.NonResourceURLs.List(),
+	},
 }
 
 // PolicyRule holds information that describes a policy rule, but does not contain information

diff --git a/pkg/authorization/authorizer/scope/converter.go b/pkg/authorization/authorizer/scope/converter.go
@@ -25,7 +25,7 @@ import (
 // ScopesToRules takes the scopes and return the rules back.  We ALWAYS add the discovery rules and it is possible to get some rules and and
 // an error since errors aren't fatal to evaluation
 func ScopesToRules(scopes []string, namespace string, clusterRoleGetter rbaclisters.ClusterRoleLister) ([]rbac.PolicyRule, error) {
-	rules := append([]rbac.PolicyRule{}, authorizationapi.RbacDiscoveryRule)
+	rules := append([]rbac.PolicyRule{}, authorizationapi.DiscoveryRule)
 
 	errors := []error{}
 	for _, scope := range scopes {

diff --git a/pkg/authorization/authorizer/scope/converter_test.go b/pkg/authorization/authorizer/scope/converter_test.go
@@ -222,7 +222,7 @@ func TestEscalationProtection(t *testing.T) {
 					Rules:      []rbac.PolicyRule{{APIGroups: []string{""}, Resources: []string{"pods", "secrets"}}},
 				},
 			},
-			expectedRules: []rbac.PolicyRule{authorizationapi.RbacDiscoveryRule, {APIGroups: []string{""}, Resources: []string{"pods"}}},
+			expectedRules: []rbac.PolicyRule{authorizationapi.DiscoveryRule, {APIGroups: []string{""}, Resources: []string{"pods"}}},
 			scopes:        []string{ClusterRoleIndicator + "admin:*"},
 		},
 		{
@@ -233,7 +233,7 @@ func TestEscalationProtection(t *testing.T) {
 					Rules:      []rbac.PolicyRule{{APIGroups: []string{}, Resources: []string{"pods", "secrets"}}},
 				},
 			},
-			expectedRules: []rbac.PolicyRule{authorizationapi.RbacDiscoveryRule, {APIGroups: []string{}, Resources: []string{"pods", "secrets"}}},
+			expectedRules: []rbac.PolicyRule{authorizationapi.DiscoveryRule, {APIGroups: []string{}, Resources: []string{"pods", "secrets"}}},
 			scopes:        []string{ClusterRoleIndicator + "admin:*"},
 		},
 		{
@@ -244,7 +244,7 @@ func TestEscalationProtection(t *testing.T) {
 					Rules:      []rbac.PolicyRule{{APIGroups: []string{"foo"}, Resources: []string{"pods", "secrets"}}},
 				},
 			},
-			expectedRules: []rbac.PolicyRule{authorizationapi.RbacDiscoveryRule, {APIGroups: []string{"foo"}, Resources: []string{"pods", "secrets"}}},
+			expectedRules: []rbac.PolicyRule{authorizationapi.DiscoveryRule, {APIGroups: []string{"foo"}, Resources: []string{"pods", "secrets"}}},
 			scopes:        []string{ClusterRoleIndicator + "admin:*"},
 		},
 		{
@@ -255,7 +255,7 @@ func TestEscalationProtection(t *testing.T) {
 					Rules:      []rbac.PolicyRule{{APIGroups: []string{"", "and-foo"}, Resources: []string{"pods", "oauthaccesstokens"}}},
 				},
 			},
-			expectedRules: []rbac.PolicyRule{authorizationapi.RbacDiscoveryRule, {APIGroups: []string{"", "and-foo"}, Resources: []string{"pods"}}},
+			expectedRules: []rbac.PolicyRule{authorizationapi.DiscoveryRule, {APIGroups: []string{"", "and-foo"}, Resources: []string{"pods"}}},
 			scopes:        []string{ClusterRoleIndicator + "admin:*"},
 		},
 		{
@@ -266,7 +266,7 @@ func TestEscalationProtection(t *testing.T) {
 					Rules:      []rbac.PolicyRule{{APIGroups: []string{""}, Resources: []string{"pods", "secrets"}}},
 				},
 			},
-			expectedRules: []rbac.PolicyRule{authorizationapi.RbacDiscoveryRule, {APIGroups: []string{""}, Resources: []string{"pods", "secrets"}}},
+			expectedRules: []rbac.PolicyRule{authorizationapi.DiscoveryRule, {APIGroups: []string{""}, Resources: []string{"pods", "secrets"}}},
 			scopes:        []string{ClusterRoleIndicator + "admin:*:!"},
 		},
 	}

diff --git a/pkg/cmd/server/bootstrappolicy/policy.go b/pkg/cmd/server/bootstrappolicy/policy.go
@@ -512,7 +512,7 @@ func GetOpenshiftBootstrapClusterRoles() []rbac.ClusterRole {
 			Rules: []rbac.PolicyRule{
 				// Health
 				rbac.NewRule("get").URLs("/healthz", "/healthz/*").RuleOrDie(),
-				authorizationapi.RbacDiscoveryRule,
+				authorizationapi.DiscoveryRule,
 			},
 		},
 		{
@@ -772,7 +772,7 @@ func GetOpenshiftBootstrapClusterRoles() []rbac.ClusterRole {
 				Name: DiscoveryRoleName,
 			},
 			Rules: []rbac.PolicyRule{
-				authorizationapi.RbacDiscoveryRule,
+				authorizationapi.DiscoveryRule,
 			},
 		},
 		{

diff --git a/test/testdata/bootstrappolicy/bootstrap_cluster_roles.yaml b/test/testdata/bootstrappolicy/bootstrap_cluster_roles.yaml
@@ -1699,22 +1699,22 @@ items:
     verbs:
     - get
   - nonResourceURLs:
-    - /
-    - /.well-known
-    - /.well-known/*
+    - /version
+    - /version/*
     - /api
     - /api/*
     - /apis
     - /apis/*
     - /oapi
     - /oapi/*
-    - /osapi
-    - /osapi/
-    - /swagger.json
     - /swaggerapi
     - /swaggerapi/*
-    - /version
-    - /version/*
+    - /swagger.json
+    - /osapi
+    - /osapi/
+    - /.well-known
+    - /.well-known/*
+    - /
     verbs:
     - get
 - apiVersion: rbac.authorization.k8s.io/v1beta1
@@ -2356,22 +2356,22 @@ items:
     name: system:discovery
   rules:
   - nonResourceURLs:
-    - /
-    - /.well-known
-    - /.well-known/*
+    - /version
+    - /version/*
     - /api
     - /api/*
     - /apis
     - /apis/*
     - /oapi
     - /oapi/*
-    - /osapi
-    - /osapi/
-    - /swagger.json
     - /swaggerapi
     - /swaggerapi/*
-    - /version
-    - /version/*
+    - /swagger.json
+    - /osapi
+    - /osapi/
+    - /.well-known
+    - /.well-known/*
+    - /
     verbs:
     - get
 - apiVersion: rbac.authorization.k8s.io/v1beta1