Fall-back to username for DevWorkspace creator label if UID is not present.

[AObuchow]

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.

[AObuchow]

After further discussion with the OpenShift console team, we've decided to change the proposed fix for this issue:

• Add a new devworkspace label, something like controller.devfile.io/creator-username that will hold the username of the user who has created the devworkspace
• Require devworkspaces to use both the controller.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.
  • However, this issue could still occur if both user A and user B don't have UID's (and only usernames)...
• The OpenShift console should verify both the controller.devfile.io/creator and controller.devfile.io/creator-username labels on a DevWorkspace when opening a web terminal

What 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.

[ibuziuk]

@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]

@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:

• Using the username kube:admin is not a valid kubernetes label value as it contains the : character
  • We could encode kube:admin and it's potential variations (kubeadmin, kube-admin) to try and programatically solve this issue:
    • If we encode as base64 or base32, the padding character = must be replaced by another character, since = is not valid in kubernetes label strings
    • base64 encoding results in the use of upper character letters, which are also not valid in kubernetes label strings
    • It seems that base32, padded with a character other than = is the way to go if we want the encoding to be reversible
    • These are just implementation details however... and can be discussed once I have a PR in place.
• Finding an encoding solution just for kubeadmin's username seems a bit overkill currently (we could just hard-code something), but this may be important if we have to deal with the niche rosa HCP case later on, where any arbtirary username may be used in place of a UID.
• Unfortunately, it's not enough to encode kubeadmin's username on the DWO webhook side & in the OpenShift Console backend. The OpenShift Console's frontend is hard-coded to expect the controller.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.

[AObuchow]

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.

[AObuchow]

The OCP change that caused the bug related to this issue in WTO has now been backported to all affected versions, see redhat-developer/web-terminal-operator#162 (comment). Closing this issue.