-
Notifications
You must be signed in to change notification settings - Fork 363
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
Use access to Alertmanager CR to gate access to AM #502
Conversation
/test e2e-aws |
thanks @cblecker, the e2e test failure is valid here. The following alerts are being fired:
The resolution should be failry easy. We need to give Prometheus the permissions expressed in the oauth proxy SAR: |
I believe adding another cluster-monitoring-operator/jsonnet/prometheus.jsonnet Lines 11 to 33 in 3387268
|
and referencing it here: cluster-monitoring-operator/jsonnet/prometheus.jsonnet Lines 114 to 115 in 3387268
|
@s-urbaniak Thanks for helping to identify the errors. Should this in in addition to the |
@s-urbaniak I'm guessing instead of, and running tests now. If it turns out that prometheus needs both permissions, then I'll add namespaces back in. |
Turns out it needed to be an in addition to. I've updated the PR, as well as updated the docs to indicate how to grant permissions. |
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.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cblecker, s-urbaniak 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 |
This adds the alertmanager policy rule to the existing cluster-monitoring-view cluster role such that users can access alertmanager with the new subject access review policy added in openshift#502.
This changes the permissions needed to access Alertmanager's UI, and the ability to view its config or create silences. Instead of having to rely in the ability to see all namespaces, this changes the permissions to the ability to view the Alertmanager CR. This way, access can be specifically granted to users based on the ability to view that CR.
Also fix a makefile syntax issue on systems (such as Darwin) that use a different file path for the make binary.
cc: @RiRa12621 @brancz @sichvoge
ref: https://jira.coreos.com/browse/SREP-1983