<username>-che as default namespace #156

AndrienkoAleksandr · 2020-01-27T14:42:57Z

Referenced issue
issue: eclipse-che/che#15493

What does this PR do?
Continue work started #137 :

don't create che-workspace SA
check what namespace we're using for workspaces (workspaceNamespaceDefault)
- if it is different than che-operator is deployed to, create all ClusterRoles and ClusterRoleBindings needed to create new namespaces and to create workspaces in them
- if it is same as che-operator's namespace, create just Role and RoleBinding needed to manage workspaces in same namespace
if oauth is enabled, just create che sa with no granted permissions
finalize cluster roles and cluster role bindings created by che-operator for Che server
make working Che server property CHE_INFRA_KUBERNETES_CLUSTER__ROLE__NAME

Related pr(s)
che-incubator/chectl#469

TODO:

test and fix permissions for k8s (currently it works for openshift)
test with OS Oauth. PR does not grant any permissions for che sa
check chectl
test installation with olm

… to che serviceaccount Signed-off-by: Michal Vala <mvala@redhat.com>

Signed-off-by: Michal Vala <mvala@redhat.com>

…amespaces Signed-off-by: Michal Vala <mvala@redhat.com>

…rent namespace than che Signed-off-by: Michal Vala <mvala@redhat.com>

Signed-off-by: Michal Vala <mvala@redhat.com>

… pr/137

Fix failing workspace start due 'update' namespace permission. Handle todo about saving Che worksapce cluster role property to config map. Rename che cluster role to che-manage-namespaces Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

sparkoo

few nitpicks

sparkoo · 2020-01-27T17:42:03Z

deploy_k8s.sh

+# sometimes the operator cannot get CRD right away
+sleep 2
+
+# uncomment if you need Login with OpenShift


this is irrelevant here

sparkoo · 2020-01-27T17:43:09Z

deploy_k8s.sh

+
+BASE_DIR=$(cd "$(dirname "$0")"; pwd)
+
+kubectl apply -f "${BASE_DIR}"/deploy/service_account.yaml -n che


would be nice if script accepts one arg with namespace, defaulting to che

sparkoo · 2020-01-27T17:56:48Z

pkg/controller/che/che_controller.go

+			}
+
+			// this binding is needed to manage che workspaces out of che namespace
+			// `che` ClusterRole should be created during che-operator deploy


the comment is no longer valid. you're binding che-manage-namespace role

sparkoo · 2020-01-27T17:57:06Z

pkg/controller/che/che_controller.go

+			}
+		} else {
+			// this binding is needed to create new namespaces for workspaces
+			// `che` ClusterRole should be created during che-operator deploy


the comment is no longer valid. you're binding che-create-namespaces role

sparkoo · 2020-01-27T18:00:12Z

pkg/deploy/che_configmap.go

@@ -164,6 +166,7 @@ func GetConfigMapData(cr *orgv1.CheCluster) (cheEnv map[string]string) {
 		CheDebugServer:                       cheDebug,
 		CheInfrastructureActive:              infra,
 		CheInfraKubernetesServiceAccountName: "che-workspace",
+		CheWorkspaceClusterRole:			  cheWorkspaceClusterRole,


formatting looks bit off here

sparkoo · 2020-01-28T07:22:12Z

When used with chectl, I'm getting:

E0128 07:19:57.695018       1 reflector.go:134] sigs.k8s.io/controller-runtime/pkg/cache/internal/informers_map.go:196: Failed to list *v1.ClusterRoleBinding: clusterrolebindings.rbac.authorization.k8s.io is forbidden: User "system:serviceaccount:che:che-operator" cannot list resource "clusterrolebindings" in API group "rbac.authorization.k8s.io" at the cluster scope

you can try with

chectl server:start --installer=operator --platform=crc --che-operator-image=quay.io/mvala/che-operator:namespace

sparkoo

currently does not work with chectl

sparkoo · 2020-01-28T07:33:37Z

It looks like the patch is not actually changing default namespace. It has to be defined in CheCluster CR yaml, whitch is now empty https://github.com/eclipse/che-operator/blob/pr/137/deploy/crds/org_v1_che_cr.yaml#L57
It can be done as separate PR. This PR will make it ready for it.

AndrienkoAleksandr · 2020-01-28T07:36:01Z

currently does not work with chectl

pr for chectl che-incubator/chectl#469 . @sparkoo Did you tested this one?

sparkoo · 2020-01-28T08:23:04Z

currently does not work with chectl

pr for chectl che-incubator/chectl#469 . @sparkoo Did you tested this one?

no :) will do

sparkoo · 2020-01-28T09:01:09Z

works with che-incubator/chectl#469. Just need to properly handle templates.

…the minikube: create own unique clusterrole and clusterrolebinding for Che server. We need it if we want to have working few che in the same cluster in the different namespaces. Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

che-bot · 2020-02-04T09:24:03Z

Can one of the admins verify this patch?

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

…r roles Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

… pr/137

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

AndrienkoAleksandr · 2020-02-05T10:29:41Z

Hello, @davidfestal, @tolusha , @mmorhun . I completed testing current pr with OLM on the minikube and CRC clutster. Looks like everything works just fine. Could you review pr, please?

sparkoo · 2020-02-05T12:24:55Z

pkg/controller/che/che_controller.go

+			}
+
+			// this binding is needed to manage che workspaces out of che namespace
+			// `${namespace}-clusterrole-manage-namespaces` ClusterRole should be created during che-operator deploy


If ClusterRole is created during che-operator deploy, like this comment say, why it is created again here https://github.com/eclipse/che-operator/pull/156/files#diff-d9445e614f0d728fd140771b0fd29c50R1315 ?

OLM auto-generate unique names for che-operator ClusterRoles. ClusterRole object is not namespaced it has wider scope. That's why olm generate name to have ability to use few operators in the same cluster with own specific version of the ClusterRoles and their rules. We can not find and reuse this roles for che-server by name, That's why I created for che-server own Cluster roles.
P.S.: Theoretically we can reuse che-operator cluster role using labels, but I used current approach.

I see, ok. thanks

ibuziuk

@nickboldt @l0rd is it planned to have this change in 7.9.0 ? this would affect defaults for CRW I think

mmorhun · 2020-02-05T14:56:13Z

README.md

@@ -131,6 +131,7 @@ Or you can build in a container:
 docker run -ti -v /tmp:/tmp -v ${OPERATOR_REPO}:/opt/app-root/src/go/src/github.com/eclipse/che-operator registry.redhat.io/devtools/go-toolset-rhel7:1.11.5-3 sh -c "OOS=linux GOARCH=amd64 CGO_ENABLED=0 go build -o /tmp/run-tests /opt/app-root/src/go/src/github.com/eclipse/che-operator/e2e/*.go"
 cp /tmp/run-tests ${OPERATOR_REPO}/run-tests
 ```
+> Notice: if you don't have redhat subscription, use public image registry.access.redhat.com/devtools/go-toolset-rhel7:latest


I think it is not right that we have links to private images in Che project.
But this is out of scope of this PR.

mmorhun

LGTM.
Tested with default che namespace and everything worked for me.

AndrienkoAleksandr · 2020-02-05T15:37:53Z

Sorry guys, pr moved #166, because Ci become crazy with branch name pr/137 ... And github doesn't provide ability to rename target branch for pr. I can change base branch, but not target.

sparkoo and others added 10 commits December 3, 2019 16:19

don't create che-workspace serviceaccount and give needed permissions…

83d2544

… to che serviceaccount Signed-off-by: Michal Vala <mvala@redhat.com>

don't create roles and rolebindings when oauth enabled

3ade531

Signed-off-by: Michal Vala <mvala@redhat.com>

Merge branch 'master' into che-workspace-rem

9e5bce3

grant more permissions to support running workspaces in pre-created n…

3b1e777

…amespaces Signed-off-by: Michal Vala <mvala@redhat.com>

add needed permissions to create namespace and run workspace in diffe…

6786dd5

…rent namespace than che Signed-off-by: Michal Vala <mvala@redhat.com>

license headers, cleanup

cd48a3c

Signed-off-by: Michal Vala <mvala@redhat.com>

update permissions

aca6124

Signed-off-by: Michal Vala <mvala@redhat.com>

Merge branch 'master' of https://github.com/eclipse/che-operator into…

092a2d2

… pr/137

Fix issue with clusterrolebindings and clusterrole finalizers

9390c16

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

AndrienkoAleksandr requested review from metlos, davidfestal, sparkoo, tolusha, sleshchenko and mmorhun January 27, 2020 14:42

AndrienkoAleksandr requested review from l0rd, nickboldt, rhopp and vparfonov as code owners January 27, 2020 14:42

AndrienkoAleksandr mentioned this pull request Jan 27, 2020

minishift/CRC fail to start workspace created with namespace.default=<workspaceid>-che eclipse-che/che#15780

Closed

23 tasks

sparkoo reviewed Jan 27, 2020

View reviewed changes

sparkoo self-requested a review January 28, 2020 07:23

sparkoo requested changes Jan 28, 2020

View reviewed changes

AndrienkoAleksandr added 4 commits February 4, 2020 22:35

Address changes.

6baaa73

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

Fix service account wrongly generated by operator-sdk for some cluste…

1922ae6

…r roles Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

Merge branch 'master' of https://github.com/eclipse/che-operator into…

caf9bd8

… pr/137

Clean up.

aa4635f

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

sparkoo reviewed Feb 5, 2020

View reviewed changes

sparkoo approved these changes Feb 5, 2020

View reviewed changes

ibuziuk reviewed Feb 5, 2020

View reviewed changes

mmorhun reviewed Feb 5, 2020

View reviewed changes

mmorhun approved these changes Feb 5, 2020

View reviewed changes

AndrienkoAleksandr closed this Feb 5, 2020

AndrienkoAleksandr deleted the pr/137 branch February 6, 2020 07:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

<username>-che as default namespace #156

<username>-che as default namespace #156

AndrienkoAleksandr commented Jan 27, 2020

sparkoo left a comment

sparkoo Jan 27, 2020

sparkoo Jan 27, 2020

sparkoo Jan 27, 2020

sparkoo Jan 27, 2020

sparkoo Jan 27, 2020

sparkoo commented Jan 28, 2020 •

edited

Loading

sparkoo left a comment

sparkoo commented Jan 28, 2020

AndrienkoAleksandr commented Jan 28, 2020

sparkoo commented Jan 28, 2020

sparkoo commented Jan 28, 2020

che-bot commented Feb 4, 2020

AndrienkoAleksandr commented Feb 5, 2020

sparkoo Feb 5, 2020

AndrienkoAleksandr Feb 5, 2020 •

edited

Loading

sparkoo Feb 5, 2020

ibuziuk left a comment

mmorhun Feb 5, 2020 •

edited

Loading

mmorhun left a comment

AndrienkoAleksandr commented Feb 5, 2020


		BASE_DIR=$(cd "$(dirname "$0")"; pwd)

		kubectl apply -f "${BASE_DIR}"/deploy/service_account.yaml -n che

<username>-che as default namespace #156

<username>-che as default namespace #156

Conversation

AndrienkoAleksandr commented Jan 27, 2020

sparkoo left a comment

Choose a reason for hiding this comment

sparkoo Jan 27, 2020

Choose a reason for hiding this comment

sparkoo Jan 27, 2020

Choose a reason for hiding this comment

sparkoo Jan 27, 2020

Choose a reason for hiding this comment

sparkoo Jan 27, 2020

Choose a reason for hiding this comment

sparkoo Jan 27, 2020

Choose a reason for hiding this comment

sparkoo commented Jan 28, 2020 • edited Loading

sparkoo left a comment

Choose a reason for hiding this comment

sparkoo commented Jan 28, 2020

AndrienkoAleksandr commented Jan 28, 2020

sparkoo commented Jan 28, 2020

sparkoo commented Jan 28, 2020

che-bot commented Feb 4, 2020

AndrienkoAleksandr commented Feb 5, 2020

sparkoo Feb 5, 2020

Choose a reason for hiding this comment

AndrienkoAleksandr Feb 5, 2020 • edited Loading

Choose a reason for hiding this comment

sparkoo Feb 5, 2020

Choose a reason for hiding this comment

ibuziuk left a comment

Choose a reason for hiding this comment

mmorhun Feb 5, 2020 • edited Loading

Choose a reason for hiding this comment

mmorhun left a comment

Choose a reason for hiding this comment

AndrienkoAleksandr commented Feb 5, 2020

sparkoo commented Jan 28, 2020 •

edited

Loading

AndrienkoAleksandr Feb 5, 2020 •

edited

Loading

mmorhun Feb 5, 2020 •

edited

Loading