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

[WIP] Skip creating the clusterrole/clusterrolebinding if we are not enabling openshif OAuth #381

Closed

Conversation

tomgeorge
Copy link
Contributor

Signed-off-by: Tom George tg82490@gmail.com

What does this PR do?

Skip creating the che-operator clusterrole if --os-auth is not enabled.

What issues does this PR fix or reference?

Part of eclipse-che/che#14662 that I changed as part of my testing.

…ng openshift oauth so a normal user can run chectl

Signed-off-by: Tom George <tg82490@gmail.com>
@@ -120,6 +125,11 @@ export class OperatorTasks {
},
{
title: `Create ClusterRoleBinding ${this.operatorClusterRoleBinding}`,
skip: () => {
if (!flags['os-oauth']) {
return `No need for ClusterRole ${this.operatorClusterRole}`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously it was implemented in such way, but then I removed such check during console link support implementing.

How consolelink supposed to work after your changes? https://github.com/eclipse/che-operator/blob/master/deploy/cluster_role.yaml#L41

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@sleshchenko sleshchenko Nov 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think chectl should be be smarter - it can check if the current user has admin rights:

  • if has - create cluster role, everything is OK
  • if it does not - OAuth flag is not permitted, during deploying chectl prints warning that consolelink won't be created.

It's needed to check what che-operator will do on OpenShift 4.2 when he does not permissions to create console link, probably it fails to deploy Che...

Copy link
Contributor

@sleshchenko sleshchenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably che-operator should be modified, and ClusterRoles for OAuth and Console links should be separate.
Then chectl is able to create OauthClusterRole only when Oauth is configured, and consoleLink might be optional in case when user is not a cluster admin.

@tomgeorge tomgeorge changed the title Skip creating the clusterrole/clusterrolebinding if we are not enabling openshif OAuth [WIP] Skip creating the clusterrole/clusterrolebinding if we are not enabling openshif OAuth Nov 6, 2019
@tomgeorge
Copy link
Contributor Author

tomgeorge commented Nov 15, 2019

Would you say this is better suited as an operator change, to separate those clusterroles?

Then on the chectl side, if --os-oauth false skip creation of those roles?

@tomgeorge tomgeorge closed this Feb 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants