Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use the upstream RBAC roles for reconciliation #20638

Merged
merged 2 commits into from
Aug 20, 2018
Merged

use the upstream RBAC roles for reconciliation #20638

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
160ad45
generated
deads2k Aug 14, 2018
fa149e8
use the upstream RBAC roles for reconciliation
deads2k Aug 14, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions pkg/cmd/server/bootstrappolicy/constants.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@ const (

// groups
const (
UnauthenticatedUsername = "system:anonymous"

AuthenticatedGroup = "system:authenticated"
AuthenticatedOAuthGroup = "system:authenticated:oauth"
UnauthenticatedGroup = "system:unauthenticated"
Expand All @@ -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
Expand Down Expand Up @@ -96,7 +93,7 @@ const (
SDNManagerRoleName = "system:sdn-manager"
OAuthTokenDeleterRoleName = "system:oauth-token-deleter"
WebHooksRoleName = "system:webhook"
DiscoveryRoleName = "system:discovery"
DiscoveryRoleName = "system:openshift:discovery"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drop the duplicate rules from this cluster role?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drop the duplicate rules from this cluster role?

That would affect the scope authorizer amongst other spots. This solves 90% of our problem. I'd live with extra rules we can transition at some later point.


// NodeAdmin has full access to the API provided by the kubelet
NodeAdminRoleName = "system:node-admin"
Expand Down Expand Up @@ -127,7 +124,6 @@ const (
NodeAdminRoleBindingName = NodeAdminRoleName + "s"
SDNReaderRoleBindingName = SDNReaderRoleName + "s"
WebHooksRoleBindingName = WebHooksRoleName + "s"
DiscoveryRoleBindingName = DiscoveryRoleName + "-binding"

OpenshiftSharedResourceViewRoleBindingName = OpenshiftSharedResourceViewRoleName + "s"

Expand Down
3 changes: 3 additions & 0 deletions pkg/cmd/server/bootstrappolicy/dead.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
65 changes: 6 additions & 59 deletions pkg/cmd/server/bootstrappolicy/policy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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",
Expand Down
1 change: 1 addition & 0 deletions pkg/cmd/server/bootstrappolicy/web_console_role_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
)
Expand Down
4 changes: 0 additions & 4 deletions pkg/cmd/server/kubernetes/master/master_config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions test/integration/master_routes_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Loading