-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[OpenShift OAuth] Idling doesn't have enough permissions to stop the workspace. #15906
Comments
I assume such a scenario of this problem.
|
@davidfestal is this task for deploy team now ? cc @tolusha |
@ibuziuk I don't think so. To me it's a wsmaster problem, if I understand correctly. It's not especially related to installation afaik. BTW it doesn't seem a new issue to me. Don't know why it pops up again right now though. |
@davidfestal when you implement this functionality what was the plan for the idler? How does it suppose to work in upstream? |
setting P1 since it affects Toolchain team |
@skabashnyuk I'm not sure what exact part of code you're speaking about, when mentioning my implementation. But I seem to remember that at some point there was a cache to map subjects associated to started workspaces (this cache I had implemented). This used to help retrieving the right subject when idling. |
@davidfestal I was referring to your code that was added #9577 and related to OSIO team issue #8178. Can you remind me: is that correct that to get Openshift OAuth token we need Keycloak token that is not available for the idler and we can cache it since it can expire?
@ibuziuk do we have someone else from your team who remembers implementation details? |
@skabashnyuk @davidfestal sorry, folks but I do not understand how come this issue is related to Hosted Che - this is a pure upstream issue with 0 dependency on downstream (Hosted Che) |
@skabashnyuk I see the PR you're referring to. But let me remind that this PR has been created when the work on Che 7 wasn't even there. So a number of things might have changed in the meantime which I didn't control.
And yes, afair at this point of time this implied using a cache. But Keycloak token expiration was not a problem since the cached subject for a user was systematically updated when a user did any action on the workspace (either Dashboard or Workspace).
The only one I'm finding back for now is this PR: https://github.com/eclipse/che/pull/7243/files#diff-a7b479c3e74a881033d3835df47948d6R98 Instead of that, the equivalent changes were added into rh-che: https://github.com/redhat-developer/rh-che/commits/3cd8ced08128d5263301cd81a8b6b3c49e29cf93/plugins/ls-bayesian-agent/src/main/java/com/redhat/bayesian/agent/WorkspaceSubjectsRegistry.java So on the rh-che side I don't see any reason for this not to work. To summarize, the cache mechanism I had once implemented used to provide (afaict) a consistent solution to support idling in this use-case, at least at this time. @ibuziuk we should probably setup some sort of sync or meeting to understand why this probem popped up again in hosted Che. |
Sorry @ibuziuk according to the first comments in this issue, I initially thought that the problem was occurring in Hosted Che currently, but obviously I was mistaken. This doesn't change the fact that my reasoning is mainly the following: If it's something that can be fixed through idling a workspace by using the Openshift user that created it (which I believe might doable - cf the cache mechanism I had setup long ago), then no additional permission would be required, and this issue would more likely become a platform issue. In a general way, it seems to me that requiring additional permissions instead of fixing design itself is usually not the way to go. |
Both #15906 and #15904 (comment) clearly says that none of the caching techniques (keycloak or OpenShift OAuth) doesn't provide 100% guarantee that start or stop(idling) operation would be successfully performed.
There is plan B that will grant additional permissions for che-server Note:
|
I was able to reproduce this. It seems like the way forward is to change the RBAC rules around allowed actions in developer namespaces. Is there any reason why we cannot do this? |
@tomgeorge yes I think we agreed that we should grant Che SA privileges to stop workspaces in users namespaces (@ibuziuk and @davidfestal can confirm). |
AFAIK it would not work in a multi-cluster environment. |
@skabashnyuk there is no official support / documentation for multi-cluster environment. If a user tries to use Che in a multi cluster scenario it will need to use some external tools to simulate a single cluster. hosted-che uses oso-proxy to simulate single cluster. |
ok. Does this also mean that Che SA privileges would be enough to start the workspace? |
Yes that's the idea: Che SA should be granted edit privileges that allow objects creation/deletion |
…ty in order to have a possibility to disable the 'OpenShiftStopWorkspaceRoleProvisioner' Signed-off-by: Ilya Buziuk <ibuziuk@redhat.com>
che eclipse-che#15906 Adding 'che.workspace.stop.role.enabled' property in order to have a possibility to disable the 'OpenShiftStopWorkspaceRoleProvisioner'
PR is merged - #16532 |
Thank you guys for fixing that! Will definitely verify when the operator is available. |
…default Signed-off-by: Ilya Buziuk <ibuziuk@redhat.com>
Signed-off-by: Ilya Buziuk <ibuziuk@redhat.com>
Signed-off-by: Ilya Buziuk <ibuziuk@redhat.com>
@ibuziuk after upgrading the operator to 7.12.0 I still see the same error with not enough permissions to stop workspaces: https://gist.github.com/alexeykazakov/5e66598e93d8cbed8dcbdd9ff7b23e63 |
@alexeykazakov I think che-operator 7.12.0 still uses 7.12.0 of che-server, and this fix went into 7.12.1 |
Hm.. OK. Ilya mentioned 7.12.0 version though:
Will wait for 7.12.1 then. Any time frame for 7.12.1 release? |
Actually setting |
The fix went into 7.12.0, but it caused an issue in non-multiuser deployments. With |
What is a single user deployment? |
Like running Edit: when no identity provider is given |
No keycloak, no postgres, no authentication. Used for local development because it requires less resources. |
@alexeykazakov correct. Idling will work OOTB in 7.13.0 |
Signed-off-by: Ilya Buziuk <ibuziuk@redhat.com>
Signed-off-by: Ilya Buziuk <ibuziuk@redhat.com>
Signed-off-by: Ilya Buziuk <ibuziuk@redhat.com>
Describe the bug
Workspace
stopped
(actually not stopped) incorrectly by inactive timeout if OpenShift OAuth enabled.Che version
Steps to reproduce
chectl server:start --platform=crc --installer=operator --os-oauth --tls --self-signed-cert
Expected behavior
Two variants:
Runtime
kubectl version
)oc version
)minikube version
andkubectl version
)minishift version
andoc version
)docker version
andkubectl version
)Screenshots
Installation method
Environment
Additional context
N/A
The text was updated successfully, but these errors were encountered: