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.

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

Add ClusterRbacConfig and deprecate the RbacConfig #8825

Closed
2 of 3 tasks
yangminzhu opened this issue Sep 19, 2018 · 9 comments
Closed
2 of 3 tasks

Add ClusterRbacConfig and deprecate the RbacConfig #8825

yangminzhu opened this issue Sep 19, 2018 · 9 comments
Assignees
Milestone

Comments

@yangminzhu
Copy link
Contributor

yangminzhu commented Sep 19, 2018

Describe the bug
RbacConfig is incorrectly scoped to Namespace but should be Cluster scoped instead. As the scope field of a CRD is immutable, we have to introduce a new ClusterRbacConfig with cluster scope and deprecate the old RbacConfig.

  • Add ClusterRbacConfig in pilot and crd.yaml. The logic in pilot should be:
    1. If there is a ClusterRbacConfig, then use it and ignore any RbacConfigs
    2. Fallback to check if there is a RbacConfig, if found then use it
    3. None of ClusterRbacConfig/RbacConfig found, then the RBAC is disabled.

  • Update the documentation about the deprecate of RbacConfig and guides how to migrate to ClusterRbacConfig.
    We can provide a subcommand in istioctl to do the migration automatically for the user. (Basically it should copy the content from RbacConfig to ClusterRbacConfig and then remove the RbacConfig CRD)

  • After another two releases of Istio, we could remove the RbacConfig completely in the code. There is no formal API deprecate policy for now but it should be fine for RBAC as the API is still in alpha stage.

/cc @ayj @ozevren @diemtvu @liminw

@yangminzhu yangminzhu self-assigned this Sep 19, 2018
@ayj ayj added this to the 1.1 milestone Sep 20, 2018
@yangminzhu yangminzhu changed the title Add MeshRbacConfig and deprecate the RbacConfig Add ClusterRbacConfig and deprecate the RbacConfig Oct 5, 2018
@ayj ayj modified the milestones: 1.2, 1.1 Oct 15, 2018
@ayj
Copy link
Contributor

ayj commented Oct 15, 2018

We should be sure to document the deprecation of RBACConfig in the 1.1 release notes.

@munrodg munrodg modified the milestones: 1.1, 1.2 Nov 2, 2018
@stale
Copy link

stale bot commented Feb 8, 2019

This issue has been automatically marked as stale because it has not had activity in the last 90 days. It will be closed in the next 30 days unless it is tagged "help wanted" or other activity occurs. Thank you for your contributions.

@stale
Copy link

stale bot commented May 9, 2019

This issue has been automatically marked as stale because it has not had activity in the last 90 days. It will be closed in the next 30 days unless it is tagged "help wanted" or other activity occurs. Thank you for your contributions.

@stale stale bot added the stale label May 9, 2019
@yangminzhu yangminzhu removed the stale label May 9, 2019
@rcernich
Copy link
Contributor

rcernich commented Jun 4, 2019

Can you provide a link to any discussion around this? Moving to cluster scoped resources effectively prevents multiple installations of Istio (all control planes would share the same default cluster resources; same applies to MeshPolicy), which prevents supporting multitenant installations.

Given the way these are used (providing default behavior for the mesh), I fail to see why they need to be cluster scoped, especially since they need to be named default to begin with. To better support multiple installations and multitenancy, it would be preferable if these remained namespace scoped and the controllers only listed RbacConfig (and Policy) resources in the local namespace (e.g. istio-system) named default.

@yangminzhu
Copy link
Contributor Author

@rcernich In the next version of Istio authorization, we're changing to put it (also rename to AuthorizationConfig) in the Istio root namespace which means it will be namespace-scoped resource, not cluster-scoped resource. We may still need more efforts to support multi-tenant installations though.

For more information about the next version of Istio authorization, please see https://docs.google.com/document/d/1diwa9oYVwmLtarXb-kfWPq-Ul3O2Ic3b8Y21Z3R8DzQ (accessible in Istio community group)

cc @liminw @diemtvu

@rcernich
Copy link
Contributor

rcernich commented Jun 4, 2019

Sounds great. Thanks for the info!

@knrc, fyi

@knrc
Copy link

knrc commented Jun 5, 2019

@yangminzhu We've been doing a lot of work on soft multi tenancy and have some updates to the Istio control plane components which would be of interest, this is what sparked our interest in the remaining cluster scoped resources (ClusterRbacConfig and MeshPolicy). We're trying to work out what we should do with these resources to support multiple tenants within the same cluster.

I was talking to @duderino earlier today and mentioned this.

We still have a number of issues to address including how to have the control plane components react better to our dynamic updates for the namespaces (defined in a custom resource), they often don't take too kindly to namespaces disappearing.

@rlenglet rlenglet modified the milestones: 1.4, 1.3 Jul 9, 2019
@ozevren ozevren modified the milestones: 1.3, 1.4 Sep 3, 2019
@howardjohn
Copy link
Member

howardjohn commented Oct 12, 2019

Are you planning to remove the deprecated CRD in 1.4?

@yangminzhu
Copy link
Contributor Author

yangminzhu commented Oct 18, 2019

@howardjohn
No, there is not much need to remove the RbacConfig in 1.4 as its successor ClusterRbacConfig is also deprecated by the new AuthorizationPolicy in 1.4.

The next step is to deprecate the old RBAC policy (RbacConfig, ClusterRbacConfig, ServiceRole, ServiceRoleBinding) all together which could be done in 1.5 or 1.6 to allow customer to have enough time to upgrade to AuthorizationPolicy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants