Bug 1837739: use user creator label to identify workspace resource instead of annotation #5497
Conversation
openshift-ci-robot
added
the
component/core
|
/cc @christianvogt |
|
/retitle Bug 1837739: use user creator label to identify workspace resource instead of annotation |
openshift-ci-robot
changed the title
openshift-ci-robot
added
the
bugzilla/severity-medium
|
@vikram-raj: This pull request references Bugzilla bug 1837739, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. 3 validation(s) were run on this bug
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
openshift-ci-robot
added
the
bugzilla/valid-bug
| (d) => d?.metadata?.annotations?.[CLOUD_SHELL_USER_ANNOTATION] === username, | ||
| ); | ||
| const workspace = isKubeAdmin | ||
| ? data.find((d) => d?.metadata?.labels?.[CLOUD_SHELL_CREATOR_LABEL] === '') |
There was a problem hiding this comment.
You should be using the matchLabels selector in the useK8sWatchResource hook. Search for the label match CLOUD_SHELL_CREATOR_LABEL.
This way you no longer need to loop over the results and instead you can use the first result.
There was a problem hiding this comment.
Guys, seems we forgot to mention really important thing.
You still should filter all workspaces which are mutable. So you should check org.eclipse.che.workspace/immutable: "true" annotation.
There was a problem hiding this comment.
Just a bit more info about immutable annotation: It guarantees that nobody who has edit rights for workspace CR will patch your terminal workspace and put custom dev container or cheEditor that will steal user's token
There was a problem hiding this comment.
@vikram-raj keep the loop we had already for the username check and now add this immutable annotation check.
| apiVersion: 'workspace.che.eclipse.org/v1alpha1', | ||
| kind: 'Workspace', | ||
| metadata: { | ||
| name, | ||
| namespace, | ||
| labels: { | ||
| [CLOUD_SHELL_LABEL]: 'true', |
There was a problem hiding this comment.
My mistake. We still need this label applied.
| selector: { | ||
| matchLabels: { [CLOUD_SHELL_LABEL]: 'true' }, | ||
| }, |
There was a problem hiding this comment.
My mistake. We still need this label applied.
vikram-raj
force-pushed
the
odc-3615
branch
2 times, most recently
from
9df49ad
to
0f66042
Compare
| expect(newResource.kind).toEqual(kind); | ||
| expect(newResource.metadata.name).toEqual(name); | ||
| expect(newResource.metadata.namespace).toEqual(namespace); | ||
| expect(newResource.metadata.labels[CLOUD_SHELL_LABEL]).toEqual('true'); |
There was a problem hiding this comment.
Add this check back here.
vikram-raj
force-pushed
the
odc-3615
branch
2 times, most recently
from
f5469fa
to
111c830
Compare
|
/hold for #5428 |
openshift-ci-robot
added
the
do-not-merge/hold
|
/lgtm |
openshift-ci-robot
added
lgtm
|
/lgtm |
|
/retest |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: christianvogt, vikram-raj The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/hold cancel |
openshift-ci-robot
removed
the
do-not-merge/hold
|
@vikram-raj: All pull requests linked via external trackers have merged: openshift/console#5497. Bugzilla bug 1837739 has been moved to the MODIFIED state. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Fixes:
https://issues.redhat.com/browse/ODC-3615
Now, we have
org.eclipse.che.workspace/creator: <user UID>label in the workspace. So, this PR start using this label instead of annotationconsole.openshift.io/cloudshell-user: <username>to get the workspace.