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

Do not allow to change webhooks enabled on the fly #16980

Closed
sleshchenko opened this issue May 20, 2020 · 3 comments · Fixed by devfile/devworkspace-operator#126
Closed

Do not allow to change webhooks enabled on the fly #16980

sleshchenko opened this issue May 20, 2020 · 3 comments · Fixed by devfile/devworkspace-operator#126
Assignees
Labels
engine/devworkspace Issues related to Che configured to use the devworkspace controller as workspace engine. kind/task Internal things, technical debt, and to-do tasks to be performed. new&noteworthy For new and/or noteworthy issues that deserve a blog post, new docs, or emphasis in release notes severity/P1 Has a major impact to usage or development of the system.

Comments

@sleshchenko
Copy link
Member

sleshchenko commented May 20, 2020

Is your task related to a problem? Please describe.

Currently, there two aspects that could lead to privileges escalation:

  1. Operator is reconfigured with webhooks.enabled: false and all tokens previously injected into workspaces are not secured anymore.
  2. Operator is removed and webhooks are removed transitively. So, all tokens previously injected into workspaces are not secured anymore.

Despite the fact it was handy for local development - it's not safe things to do, so it's proposed to:

  1. Do not set ownerRef for workspaces-related webhooks to avoid transitive removing.
    Then once operator is removed - it won't be possible to do any exec in any pods (even not related to workspaces since exec subresource does not support labels). So, admin is supposed to make sure that all workspace-related resources are removed and after that manually remove webhooks.
    ⚠️ It might make uninstallation process not convenient but otherwise it will be unsafe, unless OLM is able to remove all CR along with operator.
implemented
  1. Once webhooks are created - operator should not remove them if webhooks.enabled is reconfigured to false. Controller should fail in such case.
    If an admin really needs to disable webhooks - it's their responsibility to make sure that all workspaces are stopped, remove webhooks and then reconfigure and restart controller.
@sleshchenko sleshchenko added kind/task Internal things, technical debt, and to-do tasks to be performed. severity/P1 Has a major impact to usage or development of the system. engine/devworkspace Issues related to Che configured to use the devworkspace controller as workspace engine. labels May 20, 2020
@amisevsk
Copy link
Contributor

Makes sense to me, the only concern is if there's an issue with OLM.

In terms of local development, we shouldn't be impacted too heavily -- make remove webhooks would be enough.

I don't know if you meant to link to the first two PRs in the Che repo, though 😄

@JPinkney
Copy link
Contributor

If I'm understanding correctly you're trying to find out if uninstalling the operator will remove all the workspaces? I've just tried it out using https://github.com/davidfestal/summi-at-sites-workspace-operator-bundle and it looks like the workspaces themselves stay and an error message pops up when uninstalling that says: "This will remove operator Che Workspace Operator from all namespaces. Your application will keep running, but it will no longer receive updates or configuration changes."

I wonder if there's any way from inside the operator that we can detect if we are in the process of being uninstalled so that we can clean up any existing workspaces

@benoitf benoitf added the new&noteworthy For new and/or noteworthy issues that deserve a blog post, new docs, or emphasis in release notes label May 29, 2020
@JPinkney JPinkney self-assigned this Jun 3, 2020
@sleshchenko

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
engine/devworkspace Issues related to Che configured to use the devworkspace controller as workspace engine. kind/task Internal things, technical debt, and to-do tasks to be performed. new&noteworthy For new and/or noteworthy issues that deserve a blog post, new docs, or emphasis in release notes severity/P1 Has a major impact to usage or development of the system.
Projects
None yet
4 participants