Support workspace namespace configuration in che-operator

[metlos]

Is your enhancement related to a problem? Please describe.

Che server can be configured to use custom namespaces used for workspace deployment but che-operator currently hardcodes such configuration to the same namespace the che server is deployed to.

This wasn't configurable before because Che server didn't support safely changing this configuration without damaging the existing workspaces (the workspaces would loose their data, because the PVs are bound to a namespace). As of 7.5.0 this is no longer the case and the target namespace can be safely reconfigured. The workspaces now "remember" the namespace they should be deployed to.

Describe the solution you'd like

Che operator should enable configuration of the following 2 config properties in the Che CR:

• che.infra.kuberentes.namespace.default - this is currently just hardcoded to the same namespace as the che server CR
• che.infra.kubernetes.namespace.allow_user_defined - this is a new property in Che 7.5.0 that allows the users to specify a custom namespace where a workspace should be deployed.

This depends on #15040 to be merged in Che server.

[sparkoo]

@metlos does this task include updating default namespace strategy to <username>-che ?

[sparkoo]

@metlos default for che.infra.kubernetes.namespace.allow_user_defined is false ?

[skabashnyuk]

@metlos does this task include updating default namespace strategy to <username>-che ?

I would say - yes. We should be aligned with #14795

[sparkoo]

with <username>-che as default, we don't have permissions to create new namespace. For that, we need ClusterRole for che service account:

apiGroups:
  - project.openshift.io
resources:
  - projectrequests
verbs:
  - create

Do we want to include that or do we require admins to pre-create namespaces? Is it in the scope of this task?

cc: @metlos

[metlos]

@metlos does this task include updating default namespace strategy to <username>-che ?

https://github.com/eclipse/che-operator/blob/master/pkg/deploy/che_configmap.go#L88

@metlos default for che.infra.kubernetes.namespace.allow_user_defined is false ?

yes

As for the missing perms for the che service account - the logic in the operator is currently kinda simplistic - it just creates both the che and che-workspace service accounts in the same namespace as the CR. This needs to change in this PR.

The che-workspace service account should be created in the target namespace of a given workspace (if not already present) - and so this is not a job for the che operator - che server does that on demand.

As for the missing privs on the che service account - the thing is that the service account is not always required - if we use openshift oauth to authenticate, it is the user who needs to have such privs. If we don't use it, we use the che service account. This service account then optionally needs privs for namespace/project creation and also optionally needs privs for service account creation - this in case it needs to create the che-workspace service account in the target namespace of a workspace - but this only happens if the che server doesn't find such service account in the target namespace already.

I didn't know how che operator handled the service accounts before so this aspect didn't come to my mind during the initial formulation of this issue. My apologies.

[sparkoo]

@metlos does this task include updating default namespace strategy to <username>-che ?

https://github.com/eclipse/che-operator/blob/master/pkg/deploy/che_configmap.go#L88

yes, but I'm asking whether we should change the default value to <username>-che in the scope of this task?

The che-workspace service account should be created in the target namespace of a given workspace (if not already present) - and so this is not a job for the che operator - che server does that on demand.

If che-server does it, I would remove it. But I think this will complicate it :/ https://github.com/eclipse/che-operator/blob/master/pkg/controller/che/che_controller.go#L415

As for the missing privs on the che service account ...

something like this ?

if !oauth
  create `che` SA with current ClusterRoleBinding
  if defaultNamespace != cheNamespace
    allow `che` create project/namespace
    allow `che` create serviceaccounts

However, I'm not sure whether it make sense to have logic like this, or just simply always create che SA with permissions to create projects/namespaces and serviceaccounts.

[metlos]

yes, but I'm asking whether we should change the default value to -che in the scope of this task?

Well, currently, the CR namespace is the default, unless openshift oauth is active, at which point the default is <username>-che. IMHO we should keep it like that in absence of any other configuration because such configuration requires the least permissions on the operator.

[metlos]

If che-server does it, I would remove it. But I think this will complicate it :/ https://github.com/eclipse/che-operator/blob/master/pkg/controller/che/che_controller.go#L415

I think this serves the same purpose as che.infra.kubernetes.cluster_role_name config property.

something like this ?

if !oauth
 create `che` SA with current ClusterRoleBinding
 if defaultNamespace != cheNamespace
   allow `che` create project/namespace
   allow `che` create serviceaccounts

However, I'm not sure whether it make sense to have logic like this, or just simply always create che SA with permissions to create projects/namespaces and serviceaccounts.

It is just my opinion, but I believe we should require the minimum permissions given a configuration, so I would not just simply always require the create permissions.

[sparkoo]

I've created PR with new properties and not messing with defaults or service accounts eclipse-che/che-operator#136.

I will do default/serviceaccounts in second PR. Or even new issue for that? It will definitely need some investigation and discussion.
WDYT? @skabashnyuk @metlos

[sparkoo]

There is an issue with service accounts handling from che-operator. First of all, it is questionable, whether it's responsibility of che-operator to have any service accounts logic.

we need to support these scenarios (without oauth)

1. everything in one namespace (default for che-operator)
2. precreated namespaces (with and without che-workspace serviceaccout ?)
3. create namespaces (is this desired default ? The default workspaces namespace/project should be <username>-che #14795)

current state of che-operator

we always create 2 service accounts in che namespace:

• che
  • edit ClusterRole
• che-workspace
  • exec Role
    • pods/exec; *
  • view Role
    • pods; list
      With these, we support scenarios 1 and 2

requirements

To support scenario 3, che sa will need to create new namespaces and serviceaccounts in them. However, che-operator can't grant such permission, because first it would need to have it on it's own (https://github.com/eclipse/che-operator/blob/master/deploy/role.yaml).
On the other hand, when we're not using che namespace for workspaces, we don't need to create che-workspace sa.
Che-operator is basically tuned only to scenario 1 and is ok for 2 (works, but creates unnecessary che-workspace sa). For 3 it has insufficient permissions.

oauth

It's a bit easier with OpenShift oauth, because we're using user account to create the namespace. Thus user account has to have enough permissions to create namespace or have pre-created namespace for it's workspaces and we can't do anything about it as we're not owners. In current state, this scenario is working ok. We only again create unnecessary che-workspace sa.

next?

I see few options for insufficient permissions:

• increase permissions for che-operator to be able to create namespaces and serviceaccounts, so it can grant these permissions to che sa
  • with this, we will be able to control what permissions che sa really have depending on namespace strategy
• don't create che sa from che-operator, but include it in che-operator deployment
• document different kind of namespace strategies and leave it to cluster admins

We probably should always use as less permissions as we need, but it is tricky to control it programatically, because we don't know needed permissions before che-operator deployment.

Second issue with che-workspace sa. AFAIK we don't need to create this sa at all. It is managed with che server. We should simply not create it from che-operator.

• There is CheWorkspaceClusterRole property, that allows to give custom ClusterRole to che-workspace sa. It has it's equivalent on che server (https://github.com/eclipse/che/blob/master/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties#L263), so we should just set this as a variable in configmap.

[sparkoo]

cc @metlos

[metlos]

IMHO #15300 (comment) is a great summary of the requirements we have on the SAs.

I think it also highlights the fact that we have a little bit "blurred" permission story - we either delegate the infra permissions to openshift oauth or we require quite strong permissions ourselves and provide user separation using our own mechanisms.

@davidfestal, @l0rd would you please comment on the solution you'd favor given the broader picture of Che installation conforming to the Kubernetes/OpenShift best practices for operator and operatee deployment and permissions for doing that and also the "Che permissions story" - how we want the admins and end users to grant the permissions to the che server for its correct function?

[l0rd]

I agree that #15300 (comment) is a great summary.

I think we should agree that scenario n.1 should be avoided as all users will have access to the namespace objects (secrets with ssh keys etc...).

At the same time we cannot risk that, once Che is installed on a cluster, users fail to create workspaces because of not enough privileges.

Hence we should provide the following options to the admin that install CheCluster:

• @sparkoo scenario n.3 (wsmaster creates users namespaces and associated sa): it requires wsmaster to have more privileges that it currently has today but it should be the default. Some admins won't buy it so we need another option...
• @sparkoo scenario n.2 (admin pre-create users namespaces and associated sa): that's a safer option in terms of security exposure but workspace creation may fail if the admin hasn't pre-created the namespaces properly. Providing a clear self-explanatory error message when a workspace creation fails is critical in this scenario.

In scenario n.3 I would prefer if we could decouple namespace and workspace creation. In other words I would create the namespace as soon as possible (when the Che user is provisioned) and delete it as late as possible (at user deprovisioning). That's because we will fail earlier if something goes wrong and because it removes one responsibility from workspace creation flow and makes it simpler.

@alexeykazakov I would like to know you opinion

[alexeykazakov]

For hosted Toolchain, scenario n.2 is what we need. Namespace provisioning there is similar to OSIO.
During user provisioning Toolchain creates <username>-code namespace which the user has access to. So, by the time we need to create a workspace for the user the namespace already exists.

There are a couple of important details though:

• Currently, Hosted Toolchain uses -code suffix for the namespaces (vs. -che in OSIO). And it can change in the future. So, it's important to allow the operator admin to configure the namespace name template and do not hardcode <username>-code or something else.
• username in the <username>-code template is not actually the OpenShift username as is. The problem here is that namespace names have to be DNS-1123 compliant. But OpenShift usernames do not. For example OpenShift allows users with emails as usernames: john@domain.com. But you can't create a namespace with john@domain.com-che name. To workaround this problem in Hosted Toolchain we transform the original username if it's not DNS-1123 compliant and use this transformed and compliant username as names of other relevant resources like namespaces. For example that john@domain.com user can be transformed to john-at-domain-com (or john-at-domain-com-1(2,3,4,...) if john-at-domain-com is already taken).
  In such case the john@domain.com user will get john-at-domain-com-code (or john-at-domain-com-<N>-code!) namespace provisioned.

While supporting a custom suffix should be pretty easy I guess, the second problem with compliant username is harder to solve. Basically you can't map the same username always to the same namespace just using the username itself. But we could relay on another source of that mapping which should be provided by the operator/cluster admin. For example it could be a specific annotation in the User.meta.annotations of the OpenShift user which will tell Che what namespace to use for workspaces for this user by default. Or something like that. If not defined than use the default <username>-che for example...

[sparkoo]

scenario n.3 (wsmaster creates users namespaces and associated sa): it requires wsmaster to have more privileges that it currently has today but it should be the default.

@l0rd That would mean that we need grant more privileges to che-operator by default. Even if we use pre-created namespaces, che-operator will have permissions to create namespaces. It is more tricky to control che-operator permissions, especially with one-click install from OperatorHub. We don't know the minimum permissions needed at deploy time so we would have to set maximum permissions. Is it ok? Or do you see any other option here?

So, it's important to allow the operator admin to configure the namespace name template and do not hardcode -code or something else.

@alexeykazakov PR is currently in review for this eclipse-che/che-operator#136

username in the -code template is not actually the OpenShift username as is. The problem here is that namespace names have to be DNS-1123 compliant.

@alexeykazakov yes, the issue is on our radar #15323. I think you should comment there.

[sparkoo]

@metlos @l0rd @alexeykazakov I've mystified you. The current state of che-operator is that it's able to run only scenario 1 - all in che namespace. It's not even possible to run workspaces in pre-created namespace, because che sa don't have get projects cluster wide permission.

We're ok with os oauth. Last thing I want to test is if we need any permissions for che sa at all, when using oauth (eclipse-che/che-operator#137 (comment)).

Question is, whether and how we want to increase permissions of che-operator to be able to grant cluster wide permissions to che sa?

[l0rd]

@sparkoo yes it's ok to add get projects cluster wide permissions to the operator. Even if it's not ideal I don't think we have other options. As I said, scenario 1 is acceptable for demo purpose but not acceptable for an enterprise customer installation.

[sparkoo]

@l0rd I was very hasty saying that we need just get projects. To start/stop/delete workspace in pre-created namespace, I've end-up with following ClusterRole for che sa:

ClusterRole for `che`

kind: ClusterRole
apiVersion: rbac.authorization.k8s.io/v1
metadata:
  name: che
  selfLink: /apis/rbac.authorization.k8s.io/v1/clusterroles/che
  uid: 86568a37-1762-11ea-b11a-52fdfc072182
  resourceVersion: '106835'
  creationTimestamp: '2019-12-05T13:24:11Z'
  labels:
    app: che
    component: che
rules:
  - verbs:
      - get
      - create
    apiGroups:
      - authorization.openshift.io
      - rbac.authorization.k8s.io
    resources:
      - roles
  - verbs:
      - get
      - update
      - create
    apiGroups:
      - authorization.openshift.io
      - rbac.authorization.k8s.io
    resources:
      - rolebindings
  - verbs:
      - get
    apiGroups:
      - project.openshift.io
    resources:
      - projects
  - verbs:
      - get
      - create
    apiGroups:
      - ''
    resources:
      - serviceaccounts
  - verbs:
      - create
    apiGroups:
      - ''
    resources:
      - pods/exec
  - verbs:
      - list
    apiGroups:
      - ''
    resources:
      - persistentvolumeclaims
      - configmaps
  - verbs:
      - list
    apiGroups:
      - apps
    resources:
      - secrets
  - verbs:
      - list
      - create
      - delete
    apiGroups:
      - ''
    resources:
      - secrets
  - verbs:
      - create
      - get
      - watch
    apiGroups:
      - ''
    resources:
      - persistentvolumeclaims
  - verbs:
      - get
      - list
      - create
      - watch
      - delete
    apiGroups:
      - ''
    resources:
      - pods
  - verbs:
      - get
      - list
      - create
      - patch
      - watch
      - delete
    apiGroups:
      - apps
    resources:
      - deployments
  - verbs:
      - list
      - create
      - delete
    apiGroups:
      - ''
    resources:
      - services
  - verbs:
      - create
      - delete
    apiGroups:
      - ''
    resources:
      - configmaps
  - verbs:
      - list
      - create
      - delete
    apiGroups:
      - route.openshift.io
    resources:
      - routes
  - verbs:
      - watch
    apiGroups:
      - ''
    resources:
      - events
  - verbs:
      - list
      - get
      - patch
      - delete
    apiGroups:
      - apps
    resources:
      - replicasets

[sparkoo]

I've updated the PR (eclipse-che/che-operator#137). It gives full permissions (to create namespace and to manage workspaces in that namespace) when disabled oauth and using different namespace for workspaces. Please check if it is acceptable. There are still few issues remaining to solve. Should be in description.

cc @l0rd @metlos @skabashnyuk @sleshchenko @davidfestal

[alexeykazakov]

I've mystified you. The current state of che-operator is that it's able to run only scenario 1 - all in che namespace. It's not even possible to run workspaces in pre-created namespace, because che sa don't have get projects cluster wide permission.

@sparkoo does it mean that https://www.eclipse.org/che/docs/che-7/advanced-configuration-options/#one-namespace-per-user-strategy doesn't actually work in Che operator because lack of permissions for the che SA?

[sparkoo]

@sparkoo does it mean that https://www.eclipse.org/che/docs/che-7/advanced-configuration-options/#one-namespace-per-user-strategy doesn't actually work in Che operator because lack of permissions for the che SA?
@alexeykazakov Yes. If you want to use different namespace than che-operator is deployed to, you have to manually extend permissions for che sa or use oauth.

[alexeykazakov]

or use oauth.

What exactly do you mean by using oauth? Do you mean enabling OpenShift OAuth (openShiftoAuth: true)?

[sparkoo]

What exactly do you mean by using oauth? Do you mean enabling OpenShift OAuth (openShiftoAuth: true)?

yes

[sparkoo]

@l0rd full list of needed permissions is here https://gist.github.com/sparkoo/624bbd1e10c88b8ad8719b93bc847920. It's for both OpenShift and Kubernetes.
I think it is quite important decision so I've started mail thread on che-dev https://www.eclipse.org/lists/che-dev/msg03491.html

[sparkoo]

scope of this issue is implemented. I've created separate issue for <username>-che as default namespace + permissions troubles #15493.