OCPBUGS-32321: Add username/uid label and annotation for the user setting ConfigMap, Role and RoleBinding
This change adds 3 new labels and 2 annotations for newly created user settings resources.
- Labels
- A new label that identifies that this resource is a user-settings resource.
- A label with the UID
- A label with the "resource identifier" (part of the resourcename itself) because the username can contain characters that are not supported as label-value.
- Annotations:
- UID again
- The origin username because annotations also allow @-signs etc.
@stlaz this should work for the different auth variants we support, or?
For example for a kube:admin ConfigMap:
apiVersion: v1
kind: ConfigMap
metadata:
name: user-settings-kubeadmin
labels:
console.openshift.io/user-settings: "true"
console.openshift.io/user-settings-uid: ""
console.openshift.io/user-settings-resource-identifier: "kubeadmin"
annotations:
console.openshift.io/user-settings-uid: ""
console.openshift.io/user-settings-username: "kube:admin"
For a developer with a UID:
apiVersion: v1
kind: ConfigMap
metadata:
name: user-settings-1234-1234...
labels:
console.openshift.io/user-settings: "true"
console.openshift.io/user-settings-uid: "1234-1234..."
console.openshift.io/user-settings-resource-identifier: "1234-1234..."
annotations:
console.openshift.io/user-settings-uid: "1234-1234..."
console.openshift.io/user-settings-username: "consoledeveloper"
For a developer without an UID but with an email as username:
apiVersion: v1
kind: ConfigMap
metadata:
name: user-settings-e3b0c442...
labels:
console.openshift.io/user-settings: "true"
console.openshift.io/user-settings-uid: ""
console.openshift.io/user-settings-resource-identifier: "e3b0c442..."
annotations:
console.openshift.io/user-settings-uid: ""
console.openshift.io/user-settings-username: "[email protected]"
/cc @stlaz @jhadvig @TheRealJon
@jerolimov: This pull request references Jira Issue OCPBUGS-32321, which is invalid:
- expected the bug to target the "4.16.0" version, but no target version was set
Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.
The bug has been updated to refer to the pull request using the external bug tracker.
In response to this:
This change adds 3 new labels and 2 annotations for newly created user settings resources.
- Labels
- A new label that identifies that this resource is a user-settings resource.
- A label with the UID
- A label with the "resource identifier" (part of the resourcename itself) because the username can contain characters that are not supported as label-value.
- Annotations:
- UID again
- The origin username because annotations also allow @-signs etc.
@stlaz this should work for the different auth variants we support, or?
For example for a kube:admin ConfigMap:
apiVersion: v1 kind: ConfigMap metadata: name: user-settings-kubeadmin labels: console.openshift.io/user-settings: "true" console.openshift.io/user-settings-uid: "" console.openshift.io/user-settings-resource-identifier: "kubeadmin" annotations: console.openshift.io/user-settings-uid: "" console.openshift.io/user-settings-username: "kube:admin"For a developer with a UID:
apiVersion: v1 kind: ConfigMap metadata: name: user-settings-1234-1234... labels: console.openshift.io/user-settings: "true" console.openshift.io/user-settings-uid: "1234-1234..." console.openshift.io/user-settings-resource-identifier: "1234-1234..." annotations: console.openshift.io/user-settings-uid: "1234-1234..." console.openshift.io/user-settings-username: "consoledeveloper"For a developer without an UID but with an email as username:
apiVersion: v1 kind: ConfigMap metadata: name: user-settings-e3b0c442... labels: console.openshift.io/user-settings: "true" console.openshift.io/user-settings-uid: "" console.openshift.io/user-settings-resource-identifier: "e3b0c442..." annotations: console.openshift.io/user-settings-uid: "" console.openshift.io/user-settings-username: "[email protected]"/cc @stlaz @jhadvig @TheRealJon
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 openshift-eng/jira-lifecycle-plugin repository.
/jira refresh
@jerolimov: This pull request references Jira Issue OCPBUGS-32321, which is valid. The bug has been moved to the POST state.
3 validation(s) were run on this bug
- bug is open, matching expected state (open)
- bug target version (4.16.0) matches configured target version for branch (4.16.0)
- bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)
Requesting review from QA contact: /cc @sanketpathak
In response to this:
/jira refresh
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 openshift-eng/jira-lifecycle-plugin repository.
/retest
Hello Thank you for this PR, have a few questions:
- When looking for resources created by the console, is one label enough to identify? I see that the label
console.openshift.io/user-settings: "true"is present in all the examples you've mentioned and seems like a good option to use that. - Out of the three examples mentioned, it looks like resources created for users in developer sandbox would be of the second example - "developer with UID". If yes, this PR uses the two labels
console.openshift.io/user-settings: "true" , console.openshift.io/user-settings-uid:to identify the resources created by console for user, to clean it up, do you think these would be enough to identify them? - Would it be possible to extend these labels to existing resources as well, maybe like a migration code? It would be easier to identify these resources instead of searching by name. Currently I've divided the code to look for resources by name first and then if not found look by labels - which isn't pretty.
/retest
@jerolimov: This pull request references Jira Issue OCPBUGS-32321, which is invalid:
- expected the bug to target either version "4.17." or "openshift-4.17.", but it targets "4.16.0" instead
Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.
In response to this:
This change adds 3 new labels and 2 annotations for newly created user settings resources.
- Labels
- A new label that identifies this resource is a user-settings resource.
- A new label with the UID, if the user-settings refers a local OpenShift user,
kubeadmin, or empty for OIDC users.
- Annotations:
- The origin username, as annotations also allow email addresses when an OIDC user is used.
@stlaz this should work for the different auth variants we support, or?
For example for a kube:admin ConfigMap:
apiVersion: v1 kind: ConfigMap metadata: name: user-settings-kubeadmin labels: console.openshift.io/user-settings: "true" console.openshift.io/user-settings-uid: "kubeadmin" annotations: console.openshift.io/user-settings-username: "kube:admin"For a local User with a UID:
apiVersion: v1 kind: ConfigMap metadata: name: user-settings-1234-1234... labels: console.openshift.io/user-settings: "true" console.openshift.io/user-settings-uid: "1234-1234..." annotations: console.openshift.io/user-settings-username: "consoledeveloper"For an OIDC user without an UID but with an email as username:
apiVersion: v1 kind: ConfigMap metadata: name: user-settings-e3b0c442... labels: console.openshift.io/user-settings: "true" console.openshift.io/user-settings-uid: "" annotations: console.openshift.io/user-settings-username: "[email protected]"/cc @stlaz @jhadvig @TheRealJon
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 openshift-eng/jira-lifecycle-plugin repository.
/jira refresh
@jerolimov: This pull request references Jira Issue OCPBUGS-32321, which is valid.
3 validation(s) were run on this bug
- bug is open, matching expected state (open)
- bug target version (4.17.0) matches configured target version for branch (4.17.0)
- bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)
Requesting review from QA contact: /cc @sanketpathak
In response to this:
/jira refresh
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 openshift-eng/jira-lifecycle-plugin repository.
Hi, @ranakan19. Thanks for your patience and first review. I've updated the PR. Can you take a fresh look?
/cc @stlaz as auth expert /cc @jhadvig @TheRealJon as console backend experts /cc @vikram-raj @lokanandaprabhu @Lucifergene as ODC reviewers /cc @sanketpathak as ODC QE
Can some more people please take a look into that PR? Thanks :smile:
I changed the PR so that it now creates only 2 labels and 1 annotation. See my updated initial comment.
I also created a small Node.js based script to apply the missing labels to all user-setting ConfigMaps:
git clone https://github.com/openshift-dev-console/scripts odc-scripts
cd odc-scripts
npm install
./odc user-settings migrate
The labels are not consumed at the moment. But they might help other teams like Dev Sandbox to cleanup the right resources in the future.
@jerolimov: GitHub didn't allow me to request PR reviews from the following users: backend, as, console, ODC, reviewers, QE, experts, expert.
Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs.
In response to this:
Hi, @ranakan19. Thanks for your patience and first review. I've updated the PR. Can you take a fresh look?
/cc @stlaz as auth expert /cc @jhadvig @TheRealJon as console backend experts /cc @vikram-raj @lokanandaprabhu @Lucifergene as ODC reviewers /cc @sanketpathak as ODC QE
Can some more people please take a look into that PR? Thanks :smile:
I changed the PR so that it now creates only 2 labels and 1 annotation. See my updated initial comment.
I also created a small Node.js based script to apply the missing labels to all user-setting ConfigMaps:
git clone https://github.com/openshift-dev-console/scripts odc-scripts cd odc-scripts npm install ./odc user-settings migrateThe labels are not consumed at the moment. But they might help other teams like Dev Sandbox to cleanup the right resources in the future.
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-sigs/prow repository.
Hello Thank you for this PR, have a few questions:
- When looking for resources created by the console, is one label enough to identify? I see that the label
console.openshift.io/user-settings: "true"is present in all the examples you've mentioned and seems like a good option to use that.
That's enough to identify that this is a user-settings ConfigMap. But this isn't the case for old ConfigMaps.
There is currently no migration code (as part of the console or console-operator) that applies this to old resources.
- Out of the three examples mentioned, it looks like resources created for users in developer sandbox would be of the second example - "developer with UID". If yes, this PR uses the two labels
console.openshift.io/user-settings: "true" , console.openshift.io/user-settings-uid:to identify the resources created by console for user, to clean it up, do you think these would be enough to identify them?
Nice that you already picked up this work on your side. Yeah checking for both labels with a uid would be fine. It's also okay to just check for console.openshift.io/user-settings-uid.
My idea was that you can check with one label for "all user settings ConfigMaps" and with the other one for the cases that you want remove a specific ConfigMap for a specific user (uid)
- Would it be possible to extend these labels to existing resources as well, maybe like a migration code? It would be easier to identify these resources instead of searching by name. Currently I've divided the code to look for resources by name first and then if not found look by labels - which isn't pretty.
Yep, I shared a Node.js script above that you can run once. We're not sure if we will add this migration also to our operator as part of 4.17 because not all clusters users local OpenShift Users CR... :man_shrugging:
Wdyt about the node.js migration script? I can create also shell version for adding new labels if this is helpful.
Verified this on a cluster bot (with normal users, no OIDC):
Then I manually removed the labels and annotations and run the migration script. It restores the exact same labels and annotations. :heavy_check_mark:
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: jerolimov, TheRealJon
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~pkg/OWNERS~~ [TheRealJon,jerolimov]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
/retest-required
Remaining retests: 0 against base HEAD 821b63e21a3770a68df6739d18a3d7ffd8596dc4 and 2 for PR HEAD 254e6d8b4e41a29856c81230027e6b4dd36b9ab6 in total
/retest-required
Remaining retests: 0 against base HEAD e1d700dd839cc512001e57cc68b157e777d24c6e and 1 for PR HEAD 254e6d8b4e41a29856c81230027e6b4dd36b9ab6 in total
@jerolimov: all tests passed!
Full PR test history. Your PR dashboard.
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-sigs/prow repository. I understand the commands that are listed here.
@jerolimov: Jira Issue OCPBUGS-32321: All pull requests linked via external trackers have merged:
Jira Issue OCPBUGS-32321 has been moved to the MODIFIED state.
In response to this:
This change adds 3 new labels and 2 annotations for newly created user settings resources.
- Labels
- A new label that identifies this resource is a user-settings resource.
- A new label with the UID, if the user-settings refers a local OpenShift user,
kubeadmin, or empty for OIDC users.
- Annotations:
- The origin username, as annotations also allow email addresses when an OIDC user is used.
@stlaz this should work for the different auth variants we support, or?
For example for a kube:admin ConfigMap:
apiVersion: v1 kind: ConfigMap metadata: name: user-settings-kubeadmin labels: console.openshift.io/user-settings: "true" console.openshift.io/user-settings-uid: "kubeadmin" annotations: console.openshift.io/user-settings-username: "kube:admin"For a local User with a UID:
apiVersion: v1 kind: ConfigMap metadata: name: user-settings-1234-1234... labels: console.openshift.io/user-settings: "true" console.openshift.io/user-settings-uid: "1234-1234..." annotations: console.openshift.io/user-settings-username: "consoledeveloper"For an OIDC user without an UID but with an email as username:
apiVersion: v1 kind: ConfigMap metadata: name: user-settings-e3b0c442... labels: console.openshift.io/user-settings: "true" console.openshift.io/user-settings-uid: "" annotations: console.openshift.io/user-settings-username: "[email protected]"/cc @stlaz @jhadvig @TheRealJon
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 openshift-eng/jira-lifecycle-plugin repository.
[ART PR BUILD NOTIFIER]
This PR has been included in build openshift-enterprise-console-container-v4.17.0-202405290542.p0.g5a96204.assembly.stream.el9 for distgit openshift-enterprise-console. All builds following this will include this PR.
@jerolimov Thank You for your responses!
There is currently no migration code (as part of the console or console-operator) that applies this to old resources.
Is that something we can consider for future? Not just migration but the ability to clean up these resources by console. I am confident that this does not just impact sandbox. Any customers with openshift clusters irrespective of how they use User CR can have a negative impact on the performance, by leaving roles, rolebindings and configmap without cleaning up. Isn't it?
Nice that you already picked up this work on your side. Yeah checking for both labels with a uid would be fine. It's also okay to just check for
console.openshift.io/user-settings-uid.
Gotcha, Thanks! I'll continue with both labels.
Yep, I shared a Node.js script above that you can run once. We're not sure if we will add this migration also to our operator as part of 4.17 because not all clusters users local OpenShift Users CR... 🤷♂️ Wdyt about the node.js migration script? I can create also shell version for adding new labels if this is helpful.
Yes please! the node.js script looks good but a shell version would be even better.
Also, since this PR got reverted, is their anyone working on fixing it?