Closed AObuchow closed 1 week ago
After further discussion with the OpenShift console team, we've decided to change the proposed fix for this issue:
controller.devfile.io/creator-username
that will hold the username of the user who has created the devworkspacecontroller.devfile.io/creator
and controller.devfile.io/creator-username` label. This is important IMO, consider the case of two users A and B. User A might get deleted and user B might re-use the same username as user A. We want to prevent user B from having access to user A's devworkspaces.
controller.devfile.io/creator
and controller.devfile.io/creator-username
labels on a DevWorkspace when opening a web terminalWhat is not clear yet is how to safely require that devworkspaces have both labels without breaking existing devworkspaces that only have the original controller.devfile.io/creator
label.
We could allow for the mutating webhook to verify on DevWorkspace update that a DevWorkspace with the controller.devfile.io/creator
still has the same UID, and retrieve the username from the webhook request to populate the controller.devfile.io/creator-username
label. However, on clusters with DWO installed where users only have usernames and no UID's, existing devworkspaces will have the controller.devfile.io/creator
set to an empty string (their non-existing UID): a user that does not own a devworkspace could then potentially modify the devworkspace and claim wrongful ownership of the devworkspace.
It's much safer to require devworkspaces to be deleted and re-created with both the controller.devfile.io/creator
and controller.devfile.io/creator-username
labels applied. We already required devworkspaces to be deleted if the controller.devfile.io/creator
label is missing.
@ibuziuk IMO we should go with the second approach of requiring users to delete and re-create their workspaces, but this may cause issues for users who haven't backed up all the data on their PVCs. They'd have to perform some manual PVC backup workaround, which could cause frustration. If you have any thoughts, please share them with me on this issue, offline or on a call sometime soon.
@AObuchow let's keep it simple and just fix kubeadmin (aka kube:admin / kube-admin) case. To my knowledge, this is the only 4.15 case where controller.devfile.io/creator
label will be blank.
Regarding the niche rosa HCP case related to https://issues.redhat.com/browse/XCMSTRAT-365 let's handle it in another PR / issue in the future if will be needed. I requested more information / docs about this feature, but we should not handle it for now.
@AObuchow let's keep it simple and just fix kubeadmin (aka kube:admin / kube-admin) case. To my knowledge, this is the only 4.15 case where
controller.devfile.io/creator
label will be blank.
If we're only concerned with the kubeadmin case, then it might make more sense to keep things as simple as possible and go with my original proposal: in the case the creator of a workspace is kubeadmin, we encode their username and use it to populate the controller.devfile.io/creator
label.
I began testing this approach and ran into a few things:
kube:admin
is not a valid kubernetes label value as it contains the :
character
kube:admin
and it's potential variations (kubeadmin
, kube-admin
) to try and programatically solve this issue:
=
must be replaced by another character, since =
is not valid in kubernetes label strings=
is the way to go if we want the encoding to be reversiblecontroller.devfile.io/creator
to be blank, so changes need to be made there as well. I was able to build and test the OpenShift Console backend, but couldn't pull the required images to build the frontend, so I'll have to get some help from the OpenShift Console team.When I return from PTO, I'll have to think more of the pro's and con's of re-using the existing label for this specific case versus introducing a new label.
After discussion with the OpenShift Console team, it seems like we are going to revert the change that caused a bug in WTO which led to the creation of this DWO issue. We have gotten approval from our PM regarding the revert, and are currently waiting for the revert to happen before closing this issue.
The OCP change that caused the bug related to this issue in WTO has now been backported to all affected versions, see https://github.com/redhat-developer/web-terminal-operator/issues/162#issuecomment-2206400246. Closing this issue.
OpenShift 4.15 allows an OIDC provider to be configured directly with the kube-apiserver options, which removes the usage of OpenShift oauth-server and results in no OpenShift UID being provided when interacting with the cluster.
Thus, we can no longer expect that a UID will be provided when a DevWorkspace is created by a client. In order to ensure the creator of a DevWorkspace is still uniquely identifiable, we should fall-back to using the requesting client's username for the
controller.devfile.io/creator
label when a DevWorkspace is created. This is the approach that the OpenShift console has taken for OCP >= 4.15.Another option would be to have a new creator label that uses the username, and to continue using the old (current) label for the UID.