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

Default Che Operator OLM installation grants create/update/patch permissions for CheClusters to all users with the edit role #21695

Closed
Tracked by #21653 ...
amisevsk opened this issue Sep 10, 2022 · 4 comments
Labels
area/che-operator Issues and PRs related to Eclipse Che Kubernetes Operator kind/bug Outline of a bug - must adhere to the bug report template. sprint/current
Milestone

Comments

@amisevsk
Copy link
Contributor

Describe the bug

The default Che Operator install provisions aggregate roles

checlusters.org.eclipse.che-v1-admin
checlusters.org.eclipse.che-v1-crdview
checlusters.org.eclipse.che-v1-edit
checlusters.org.eclipse.che-v1-view
checlusters.org.eclipse.che-v2-admin
checlusters.org.eclipse.che-v2-crdview
checlusters.org.eclipse.che-v2-edit
checlusters.org.eclipse.che-v2-view

These clusterroles are specified to aggregate with the default admin, crdview, edit and view roles in the cluster, granting those permissions for CheClusters to users with that Kubernetes role (e.g. if I have edit permissions in a namespace, the aggregated edit roles will allow me to edit checlusters in that namespace).

We should consider restricting this, as regular users are not intended to create or edit CheClusters, and that permission should be granted more specifically. In particular, the Che Operator does not generally support more than one CheCluster being installed in a cluster and will fail to start workspaces in that case.

The severity of this issue is increased in clusters where #21694 is present, as creating a second CheCluster can trigger workspace failures that require manual workarounds to resolve across the cluster, even after the CheCluster has been cleaned up.

Che version

next (development version)

Steps to reproduce

  1. Install Che via OLM
  2. Log in as a regular, non-cluster admin user and create a namespace
  3. kubectl auth can-i create checlusters returns yes

Expected behavior

Regular users should be restricted from creating CheClusters.

Runtime

OpenShift

Screenshots

No response

Installation method

OperatorHub

Environment

Linux, Dev Sandbox (workspaces.openshift.com)

Eclipse Che Logs

No response

Additional context

No response

@amisevsk amisevsk added kind/bug Outline of a bug - must adhere to the bug report template. area/che-operator Issues and PRs related to Eclipse Che Kubernetes Operator labels Sep 10, 2022
@che-bot che-bot added the status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. label Sep 10, 2022
@tolusha
Copy link
Contributor

tolusha commented Sep 12, 2022

Investigation shows that, those ClusterRoles are created by OperatorGroup [1] and are not managed by Che operator.
As a result, we can't restrict access to CheCluster resources in a user's namespace. But Che operator can ignore those CheCluster CRs in users` namespaces by checking namespace labels and (or) annotations [2].

While investigating I found that setting annotations in a Kubernetes namespace overrides previously set labels [3].
The right soution must be the [4]

[1] https://olm.operatorframework.io/docs/concepts/crds/operatorgroup/#rbac
[2] https://github.com/eclipse-che/che-server/blob/971acbd7a26c9e3de9984fcbd0420b948dbde08b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties#L334-L352
[3] https://github.com/eclipse-che/che-server/blob/388a5183be0366a5d48c3845c35c618b5dbaeac9/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespace.java#L158-L159
[4] https://github.com/eclipse-che/che-server/blob/388a5183be0366a5d48c3845c35c618b5dbaeac9/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProject.java#L156-L157

@tolusha
Copy link
Contributor

tolusha commented Sep 12, 2022

Another solution will be to delegate only needed roles [1] like we do for Kubernentes namespace

[1] https://github.com/eclipse-che/che-operator/blob/7e63fdb7cf5b1683052b76a552f28f6343b2fd4b/pkg/deploy/rbac/workspace_permissions.go#L262

@nickboldt
Copy link
Contributor

Downstream: https://issues.redhat.com/browse/CRW-3328

@l0rd l0rd added status/analyzing An issue has been proposed and it is currently being analyzed for effort and implementation approach and removed status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. labels Sep 12, 2022
@ibuziuk ibuziuk mentioned this issue Sep 13, 2022
82 tasks
@amisevsk
Copy link
Contributor Author

I've only looked at it briefly, but the main issue I noticed when there are multiple Che clusters is that the Che routing solver cannot figure out which gateway to use. This might be fixable by always setting the annotations

che.routing.controller.devfile.io/che-name
che.routing.controller.devfile.io/che-namespace

on the DevWorkspace that Che creates. This should mean that the Che Operator can resolve the correct routing configuration [1], but I don't know if there are further issues with having multiple CheClusters.

[1] - https://github.com/eclipse-che/che-operator/blob/23c5dadd013da66236df9bc81012f7be48c0d4be/controllers/devworkspace/solver/solver.go#L167-L172

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/che-operator Issues and PRs related to Eclipse Che Kubernetes Operator kind/bug Outline of a bug - must adhere to the bug report template. sprint/current
Projects
None yet
Development

No branches or pull requests

5 participants