From fa149e87718632ce9cb2ab90e0bdad71e8476c28 Mon Sep 17 00:00:00 2001 From: David Eads Date: Tue, 14 Aug 2018 11:42:06 -0400 Subject: [PATCH] use the upstream RBAC roles for reconciliation --- pkg/cmd/server/bootstrappolicy/constants.go | 6 +- pkg/cmd/server/bootstrappolicy/dead.go | 3 + pkg/cmd/server/bootstrappolicy/policy.go | 65 ++----------------- .../bootstrappolicy/web_console_role_test.go | 1 + .../server/kubernetes/master/master_config.go | 4 -- test/integration/master_routes_test.go | 1 + 6 files changed, 12 insertions(+), 68 deletions(-) diff --git a/pkg/cmd/server/bootstrappolicy/constants.go b/pkg/cmd/server/bootstrappolicy/constants.go index bdbb3ff7d870..d9089d914df7 100644 --- a/pkg/cmd/server/bootstrappolicy/constants.go +++ b/pkg/cmd/server/bootstrappolicy/constants.go @@ -33,8 +33,6 @@ const ( // groups const ( - UnauthenticatedUsername = "system:anonymous" - AuthenticatedGroup = "system:authenticated" AuthenticatedOAuthGroup = "system:authenticated:oauth" UnauthenticatedGroup = "system:unauthenticated" @@ -43,7 +41,6 @@ const ( MastersGroup = "system:masters" NodesGroup = "system:nodes" NodeAdminsGroup = "system:node-admins" - NodeReadersGroup = "system:node-readers" ) // Service Account Names that are not controller related @@ -96,7 +93,7 @@ const ( SDNManagerRoleName = "system:sdn-manager" OAuthTokenDeleterRoleName = "system:oauth-token-deleter" WebHooksRoleName = "system:webhook" - DiscoveryRoleName = "system:discovery" + DiscoveryRoleName = "system:openshift:discovery" // NodeAdmin has full access to the API provided by the kubelet NodeAdminRoleName = "system:node-admin" @@ -127,7 +124,6 @@ const ( NodeAdminRoleBindingName = NodeAdminRoleName + "s" SDNReaderRoleBindingName = SDNReaderRoleName + "s" WebHooksRoleBindingName = WebHooksRoleName + "s" - DiscoveryRoleBindingName = DiscoveryRoleName + "-binding" OpenshiftSharedResourceViewRoleBindingName = OpenshiftSharedResourceViewRoleName + "s" diff --git a/pkg/cmd/server/bootstrappolicy/dead.go b/pkg/cmd/server/bootstrappolicy/dead.go index 8737fd671d2f..4dee5e2fcbb8 100644 --- a/pkg/cmd/server/bootstrappolicy/dead.go +++ b/pkg/cmd/server/bootstrappolicy/dead.go @@ -76,4 +76,7 @@ func init() { // this was replaced by the node authorizer addDeadClusterRoleBinding("system:nodes", "system:node") + + // this was replaced by an openshift specific role and binding + addDeadClusterRoleBinding("system:discovery-binding", "system:discovery") } diff --git a/pkg/cmd/server/bootstrappolicy/policy.go b/pkg/cmd/server/bootstrappolicy/policy.go index 4c3b5ca0efed..9b270de83d2d 100644 --- a/pkg/cmd/server/bootstrappolicy/policy.go +++ b/pkg/cmd/server/bootstrappolicy/policy.go @@ -102,18 +102,6 @@ func GetOpenshiftBootstrapClusterRoles() []rbacv1.ClusterRole { // four resource can be a single line // up to ten-ish resources per line otherwise clusterRoles := []rbacv1.ClusterRole{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: ClusterAdminRoleName, - Annotations: map[string]string{ - oapi.OpenShiftDescription: "A super-user that can perform any action in the cluster. When granted to a user within a project, they have full control over quota and membership and can perform every action on every resource in the project.", - }, - }, - Rules: []rbacv1.PolicyRule{ - rbacv1helpers.NewRule(rbacv1.VerbAll).Groups(rbacv1.APIGroupAll).Resources(rbacv1.ResourceAll).RuleOrDie(), - rbacv1helpers.NewRule(rbacv1.VerbAll).URLs(rbacv1.NonResourceAll).RuleOrDie(), - }, - }, { ObjectMeta: metav1.ObjectMeta{ Name: SudoerRoleName, @@ -741,7 +729,6 @@ func GetBootstrapClusterRoles() []rbacv1.ClusterRole { // so add them to this list. openshiftClusterRoles = append(openshiftClusterRoles, GetDeadClusterRoles()...) kubeClusterRoles := bootstrappolicy.ClusterRoles() - kubeSAClusterRoles := bootstrappolicy.ControllerRoles() openshiftControllerRoles := ControllerRoles() // Eventually openshift controllers and kube controllers have different prefixes @@ -757,26 +744,14 @@ func GetBootstrapClusterRoles() []rbacv1.ClusterRole { } conflictingNames := kubeClusterRoleNames.Intersection(openshiftClusterRoleNames) - extraRBACConflicts := conflictingNames.Difference(clusterRoleConflicts) - extraWhitelistEntries := clusterRoleConflicts.Difference(conflictingNames) - switch { - case len(extraRBACConflicts) > 0 && len(extraWhitelistEntries) > 0: - panic(fmt.Sprintf("kube ClusterRoles conflict with openshift ClusterRoles: %v and ClusterRole whitelist contains a extraneous entries: %v ", extraRBACConflicts.List(), extraWhitelistEntries.List())) - case len(extraRBACConflicts) > 0: - panic(fmt.Sprintf("kube ClusterRoles conflict with openshift ClusterRoles: %v", extraRBACConflicts.List())) - case len(extraWhitelistEntries) > 0: - panic(fmt.Sprintf("ClusterRole whitelist contains a extraneous entries: %v", extraWhitelistEntries.List())) + if len(conflictingNames) > 0 { + panic(fmt.Sprintf("kube ClusterRoles conflict with openshift ClusterRoles: %v", conflictingNames.List())) } finalClusterRoles := []rbacv1.ClusterRole{} finalClusterRoles = append(finalClusterRoles, openshiftClusterRoles...) finalClusterRoles = append(finalClusterRoles, openshiftControllerRoles...) - finalClusterRoles = append(finalClusterRoles, kubeSAClusterRoles...) - for i := range kubeClusterRoles { - if !clusterRoleConflicts.Has(kubeClusterRoles[i].Name) { - finalClusterRoles = append(finalClusterRoles, kubeClusterRoles[i]) - } - } + finalClusterRoles = append(finalClusterRoles, kubeClusterRoles...) // TODO we should not do this for kube cluster roles since we cannot control them once we run on top of kube // conditionally add the web console annotations @@ -876,7 +851,7 @@ func GetOpenshiftBootstrapClusterRoleBindings() []rbacv1.ClusterRoleBinding { newOriginClusterBinding(WebHooksRoleBindingName, WebHooksRoleName). Groups(AuthenticatedGroup, UnauthenticatedGroup). BindingOrDie(), - newOriginClusterBinding(DiscoveryRoleBindingName, DiscoveryRoleName). + rbacv1helpers.NewClusterBinding(DiscoveryRoleName). Groups(AuthenticatedGroup, UnauthenticatedGroup). BindingOrDie(), // Allow all build strategies by default. @@ -915,7 +890,6 @@ func GetBootstrapClusterRoleBindings() []rbacv1.ClusterRoleBinding { openshiftClusterRoleBindings = append(openshiftClusterRoleBindings, GetDeadClusterRoleBindings()...) kubeClusterRoleBindings := bootstrappolicy.ClusterRoleBindings() - kubeControllerClusterRoleBindings := bootstrappolicy.ControllerRoleBindings() openshiftControllerClusterRoleBindings := ControllerRoleBindings() // openshift controllers and kube controllers have different prefixes @@ -930,44 +904,17 @@ func GetBootstrapClusterRoleBindings() []rbacv1.ClusterRoleBinding { } conflictingNames := kubeClusterRoleBindingNames.Intersection(openshiftClusterRoleBindingNames) - extraRBACConflicts := conflictingNames.Difference(clusterRoleBindingConflicts) - extraWhitelistEntries := clusterRoleBindingConflicts.Difference(conflictingNames) - switch { - case len(extraRBACConflicts) > 0 && len(extraWhitelistEntries) > 0: - panic(fmt.Sprintf("kube ClusterRoleBindings conflict with openshift ClusterRoleBindings: %v and ClusterRoleBinding whitelist contains a extraneous entries: %v ", extraRBACConflicts.List(), extraWhitelistEntries.List())) - case len(extraRBACConflicts) > 0: - panic(fmt.Sprintf("kube ClusterRoleBindings conflict with openshift ClusterRoleBindings: %v", extraRBACConflicts.List())) - case len(extraWhitelistEntries) > 0: - panic(fmt.Sprintf("ClusterRoleBinding whitelist contains a extraneous entries: %v", extraWhitelistEntries.List())) + if len(conflictingNames) > 0 { + panic(fmt.Sprintf("kube ClusterRoleBindings conflict with openshift ClusterRoleBindings: %v", conflictingNames.List())) } finalClusterRoleBindings := []rbacv1.ClusterRoleBinding{} finalClusterRoleBindings = append(finalClusterRoleBindings, openshiftClusterRoleBindings...) - finalClusterRoleBindings = append(finalClusterRoleBindings, kubeControllerClusterRoleBindings...) finalClusterRoleBindings = append(finalClusterRoleBindings, openshiftControllerClusterRoleBindings...) - for i := range kubeClusterRoleBindings { - if !clusterRoleBindingConflicts.Has(kubeClusterRoleBindings[i].Name) { - finalClusterRoleBindings = append(finalClusterRoleBindings, kubeClusterRoleBindings[i]) - } - } return finalClusterRoleBindings } -// clusterRoleConflicts lists the roles which are known to conflict with upstream and which we have manually -// deconflicted with our own. -var clusterRoleConflicts = sets.NewString( - // TODO this should probably be re-swizzled to be the delta on top of the kube role - "system:discovery", - - // TODO these should be reconsidered - "cluster-admin", -) - -// clusterRoleBindingConflicts lists the roles which are known to conflict with upstream and which we have manually -// deconflicted with our own. -var clusterRoleBindingConflicts = sets.NewString() - // The current list of roles considered useful for normal users (non-admin) var rolesToShow = sets.NewString( "admin", diff --git a/pkg/cmd/server/bootstrappolicy/web_console_role_test.go b/pkg/cmd/server/bootstrappolicy/web_console_role_test.go index 63f0b1583407..6d4b9418be6d 100644 --- a/pkg/cmd/server/bootstrappolicy/web_console_role_test.go +++ b/pkg/cmd/server/bootstrappolicy/web_console_role_test.go @@ -66,6 +66,7 @@ var rolesToHide = sets.NewString( "system:openshift:aggregate-to-edit", "system:openshift:aggregate-to-view", "system:openshift:aggregate-to-cluster-reader", + "system:openshift:discovery", "system:kubelet-api-admin", "system:volume-scheduler", ) diff --git a/pkg/cmd/server/kubernetes/master/master_config.go b/pkg/cmd/server/kubernetes/master/master_config.go index 72351ec2cd77..6d04007a4a74 100644 --- a/pkg/cmd/server/kubernetes/master/master_config.go +++ b/pkg/cmd/server/kubernetes/master/master_config.go @@ -51,7 +51,6 @@ import ( "k8s.io/kubernetes/pkg/registry/cachesize" "k8s.io/kubernetes/pkg/registry/core/endpoint" endpointsstorage "k8s.io/kubernetes/pkg/registry/core/endpoint/storage" - rbacrest "k8s.io/kubernetes/pkg/registry/rbac/rest" kversion "k8s.io/kubernetes/pkg/version" "github.com/openshift/library-go/pkg/crypto" @@ -412,9 +411,6 @@ func (rc *incompleteKubeMasterConfig) Complete( genericConfig.PublicAddress = publicAddress genericConfig.Authentication.Authenticator = originAuthenticator // this is used to fulfill the tokenreviews endpoint which is used by node authentication genericConfig.Authorization.Authorizer = kubeAuthorizer // this is used to fulfill the kube SAR endpoints - genericConfig.DisabledPostStartHooks.Insert(rbacrest.PostStartHookName) - // This disables the ThirdPartyController which removes handlers from our go-restful containers. The remove functionality is broken and destroys the serve mux. - genericConfig.DisabledPostStartHooks.Insert("extensions/third-party-resources") genericConfig.AdmissionControl = admissionControl genericConfig.RequestInfoResolver = configprocessing.OpenshiftRequestInfoResolver() genericConfig.OpenAPIConfig = configprocessing.DefaultOpenAPIConfig(masterConfig) diff --git a/test/integration/master_routes_test.go b/test/integration/master_routes_test.go index a3b3a65d590d..f940c92cf808 100644 --- a/test/integration/master_routes_test.go +++ b/test/integration/master_routes_test.go @@ -125,6 +125,7 @@ var expectedIndex = []string{ "/healthz/poststarthook/project.openshift.io-projectauthorizationcache", "/healthz/poststarthook/project.openshift.io-projectcache", "/healthz/poststarthook/quota.openshift.io-clusterquotamapping", + "/healthz/poststarthook/rbac/bootstrap-roles", "/healthz/poststarthook/scheduling/bootstrap-system-priority-classes", "/healthz/poststarthook/security.openshift.io-bootstrapscc", "/healthz/poststarthook/start-apiextensions-controllers",