-
Notifications
You must be signed in to change notification settings - Fork 97
SREP-2508 - Revert openshift.io/cluster-monitoring label enforcement #437
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
base: master
Are you sure you want to change the base?
SREP-2508 - Revert openshift.io/cluster-monitoring label enforcement #437
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tnierman 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 |
Co-Authored-By: Claude <[email protected]>
b3d5c90 to
8ef8df9
Compare
|
@tnierman: 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. |
|
Hi @tnierman Correct me if I am wrong, the goal is to allow non-admins to be able to add/remove/modify the |
|
reached out for clarification in https://redhat-internal.slack.com/archives/C035W96HKN3/p1763355409770629?thread_ts=1761320979.484619&cid=C035W96HKN3 |
| // Check labels. | ||
| unauthorized, err := s.unauthorizedLabelChanges(request) | ||
| if unauthorized { | ||
| if !amIAdmin(request) && unauthorized { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am trying to understand why we add !amIAdmin(request) in this condition. It seems line 186 should already checked amIAdmin(request)? or is there anything specific here?
Reverts logic preventing non-admins from adding the
openshift.io/cluster-monitoringlabel to non-platform namespaces, as this change ultimately proved too disruptive for customers and RH alike.