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

Verifying that chectl can deploy multiuser Che without cluster admin privileges #14662

Closed
ibuziuk opened this issue Sep 25, 2019 · 16 comments
Closed
Labels
area/install Issues related to installation, including offline/air gap and initial setup kind/task Internal things, technical debt, and to-do tasks to be performed. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. severity/P1 Has a major impact to usage or development of the system.

Comments

@ibuziuk
Copy link
Member

ibuziuk commented Sep 25, 2019

Currently, cluster-admin is a requirement according to the chectl error message:

[ibuziuk@fedora ~]$ chectl server:start --platform=minishift --multiuser
✔ airplane Minishift preflight checklist
✔ Verify if oc is installed...done.
✔ Verify if minishift is installed...done.
✔ Verify if minishift is running...done.
❯ 🏃‍ Running the Che Operator
✔ Copying operator resources...done.
✔ Create Namespace (che)...done.
✔ Create ServiceAccount che-operator in namespace che...done.
✖ Create Role che-operator in namespace che
→ ERROR: It looks like you don't have enough privileges. You need to grant

Create ClusterRole che-operator
Create RoleBinding che-operator in namespace che
Create ClusterRoleBinding che-operator
Create CRD checlusters.org.eclipse.che
Waiting 5 seconds for the new Kubernetes resources to get flushed
Create deployment che-operator in namespace che
Create Che Cluster eclipse-che in namespace che
› Error: ERROR: It looks like you don't have enough privileges. You need to
› grant more privileges to current user or use a different user. If you are
› using minishift you can "oc login -u system:admin"

admin rights should not be required (except if the user wants to configure Keycloak to use OpenShift OAuth). Today for some reason we create a che-operator ClusterRole. Why do we need that? Can we avoid it?

Usecase:

  • OSD - operator in the whitelist
  • OCP
  • minishift
  • crc
  • minikube
@ibuziuk ibuziuk added the kind/task Internal things, technical debt, and to-do tasks to be performed. label Sep 25, 2019
@ibuziuk ibuziuk added this to the 7.3.0 milestone Sep 25, 2019
@ibuziuk ibuziuk added team/osio severity/P1 Has a major impact to usage or development of the system. labels Sep 25, 2019
@sleshchenko
Copy link
Member

@ibuziuk

admin rights should not be required (except if the user wants to configure Keycloak to use OpenShift OAuth)

If we want to configure OpenShift OAuth or create console link - then we need cluster role https://github.com/eclipse/che-operator/blob/master/deploy/cluster_role.yaml

But from your logs, we can see that it failed during role creating and AFAIU it's about granting operator an ability to created pods, routes, services ... https://github.com/eclipse/che-operator/blob/master/deploy/role.yaml

But your issue totally makes sense if we can not avoid admin rights at all - maybe we can improve UX, like make chectl interactive - ask the user to type credentials that we will use only for those operations that we can not do with default context credentials.

@ibuziuk ibuziuk modified the milestones: 7.3.0, Backlog - Hosted Che, 7.4.0 Oct 14, 2019
@amisevsk
Copy link
Contributor

amisevsk commented Oct 17, 2019

Testing now on

minishift v1.34.1+c2ff9cb
chectl/0.0.20191011-next.7cfccb7 linux-x64 node-v10.16.3

I see

chectl server:start -a operator -m -p minishift 
  ✖ Verify Kubernetes API
    → Failed to connect to Kubernetes API. serviceaccounts is forbidden: User "developer" cannot list serviceaccounts in the namespace "default": no RBAC policy matched
    👀  Looking for an already existing Che instance
 ›   Error: Failed to connect to Kubernetes API. serviceaccounts is forbidden: User "developer" cannot list serviceaccounts in the namespace "default": no RBAC policy matched

And running as system:admin results in the eclipse-che namespace being created by system:admin, so inaccessible to a regular account. I need to oc policy add-role-to-user admin developer -n che to access Che.

@sleshchenko
Copy link
Member

@amisevsk It's not possible to deploy operator without admin rights yet.
There is a PR that make chectl does not require admin rights on this phase che-incubator/chectl#340. But after it's merged - it would fail on clusterrole creation phase.

  1. Instead of adding admin rights for developer you can apply minishift addon admin-user and loggin to console with admin/admin.
  2. Also, you should be able precreate namespace with developer and then deploy che with admin user, so developer will still have access to Che objects like deployment, ConfigMap, CR.

I just shared different alternatives for using chectl+minishift and I definitely agree that chectl workflow should be improved.

@amisevsk
Copy link
Contributor

Thanks @sleshchenko -- I realize that the operator requires admin, it was just a little confusing. Mostly the issue is one you've described elsewhere of trying to get serviceaccounts in the default namespace causing a confusing message (but I think you've got a PR for this already).

@ibuziuk Regarding the scope of this issue, what actual testing is required? We currently have five platforms listed in the issue description, for each of those do we need to just test the "main" installer for that platform (e.g. helm for k8s clusters, etc)?

As it stands, my opinion would be

  • minishift/minikube/crc - the question of needing admin privileges is not that important, since users do have admin access (the chectl issue would be one of flow/showing information to the user, which we have other issues for)
  • k8s cluster - I haven't tested since I don't have access to one
  • OCP cluster (3.11) admin access is required since I think the option there is to use the operator
  • OSD with operator enabled - the only unknown IMO, in which case this issue really is "can we deploy the whitelisted operator without admin privileges?.

@tomgeorge
Copy link
Contributor

These are the minimum permissions required to install che via `chectl:
https://gist.github.com/tomgeorge/a1691328efd48735e6c7207008a03104

I bound those roles to the developer user in crc and installed che.

The main admin-y thing is being able to create clusterroles/roles. This requires a decent level of permissions, but not cluster-admin. If you are able to create cluster roles you can create the roles required to access oauthclients and consolelinks, while still not allowing access to those objects as the installing user.

@tomgeorge
Copy link
Contributor

tomgeorge commented Nov 5, 2019

Updated Roles required for a normal user to install che via chectl (without creating the clusterrole using this branch che-incubator/chectl#381):

https://gist.github.com/tomgeorge/3e6d9cefad635160a022fc1a8f929e1c

Testing matrix:

  • minikube
  • minishift
  • crc
  • kubernetes
  • OCP 4.X

@ibuziuk ibuziuk modified the milestones: 7.4.0, 7.5.0 Nov 6, 2019
@l0rd
Copy link
Contributor

l0rd commented Nov 25, 2019

Creating a CRD requires cluster wide privileges and I think we should process that in a different way compared to the creation of namespaced resourced. An idea would be to add the option chectl server:start --create-crd-only and use the following workflow:

  1. when the CRD already exist we should skip creating it
  2. if the CRD doesn't exist we should check if user has the right privileges to create it CRD and if he doesn't:
    a. on minishift/minikube/crc check the default system:admin user and use that for the creation of the CRD only
    b. on other platforms request the user to switch to a contex with cluster-wide privieges and use chectl server:start --create-crd-only and then come back to the current context to deploy che.

@tomgeorge about your testing matrix: are the default user privileges enough to deploy Che (if we exclude the CRD) on minikube/minishift/crc?

@tomgeorge
Copy link
Contributor

tomgeorge commented Nov 25, 2019

They are not, I had to apply the roles listed above. If I disable --os-oauth then a regular user can install Che after the che-minimal role is applied in CRC. This is also the case for minishift.

Minikube does not have any identity provider and users are typically in the admin context, so that is less of an issue.

@tomgeorge
Copy link
Contributor

tomgeorge commented Nov 26, 2019

for point 2a: I believe that you would need permission to impersonate system:admin which a regular user probably won't have

@tomgeorge
Copy link
Contributor

tomgeorge commented Nov 26, 2019

@l0rd @davidfestal what if the che operator created a che-installer service account with appropriate permissions, and cluster administrators or OLM allowed an identity provider group to impersonate that service account?

@ibuziuk ibuziuk modified the milestones: 7.5.0, 7.6.0 Nov 27, 2019
@sunix
Copy link
Contributor

sunix commented Dec 16, 2019

hello @sleshchenko @amisevsk could we update the doc accordingly ? if anything is needed to install Che, should be specified in https://www.eclipse.org/che/docs/che-7/running-che-locally/#using-minishift-to-set-up-openshift-3_running-che-locally

@sleshchenko
Copy link
Member

hello @sleshchenko @amisevsk could we update the doc accordingly ?

@sunix Sorry, accordingly to what?
It's better to sync with Michal, he has PR[1] to che-operator and it seems after that we will always require cluster-admin context while deploying Che.
There is a thread[2] in che-dev mailing list and there are no objections since now.

[1] eclipse-che/che-operator#137
[2] https://www.eclipse.org/lists/che-dev/msg03498.html

@nickboldt
Copy link
Contributor

Is this issue done? If so, please close. If not, please move to backlog or scheduled milestone.

@nickboldt nickboldt removed this from the 7.6.0 milestone Mar 3, 2020
@nickboldt
Copy link
Contributor

Tom said: "No it probably should go back in the backlog"

@nickboldt nickboldt added this to the Backlog - Deploy milestone Mar 3, 2020
@tomgeorge
Copy link
Contributor

As it stands, a "regular" user would not be able to install che via chectl without the intervention of an administrator. We had a couple of ideas to proceed but never started work on them. @tolusha @l0rd I believe this is under the deploy team now.

Also, is this a P1 issue anymore?

@tolusha tolusha added team/deploy area/install Issues related to installation, including offline/air gap and initial setup and removed team/hosted-che labels Mar 4, 2020
@tolusha tolusha mentioned this issue May 8, 2020
56 tasks
@tolusha tolusha mentioned this issue Jun 1, 2020
34 tasks
@tolusha tolusha modified the milestones: Backlog - Deploy, 7.15 Jun 3, 2020
@mmorhun mmorhun mentioned this issue Jun 23, 2020
14 tasks
@tolusha tolusha removed this from the 7.15 milestone Jun 30, 2020
@che-bot
Copy link
Contributor

che-bot commented Jan 4, 2021

Issues go stale after 180 days of inactivity. lifecycle/stale issues rot after an additional 7 days of inactivity and eventually close.

Mark the issue as fresh with /remove-lifecycle stale in a new comment.

If this issue is safe to close now please do so.

Moderators: Add lifecycle/frozen label to avoid stale mode.

@che-bot che-bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 4, 2021
@che-bot che-bot closed this as completed Jan 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/install Issues related to installation, including offline/air gap and initial setup kind/task Internal things, technical debt, and to-do tasks to be performed. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. severity/P1 Has a major impact to usage or development of the system.
Projects
None yet
Development

No branches or pull requests

9 participants