Skip to content
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

[rancher-monitoring] Correct list permission with get #2753

Merged
merged 3 commits into from
Jul 11, 2023
Merged

[rancher-monitoring] Correct list permission with get #2753

merged 3 commits into from
Jul 11, 2023

Conversation

vatsalparekh
Copy link

Issue:

SURE-6299

Problem

When accessing the endpoints object called rancher-monitoring-alertmanager, either from HTTP url or from kubectl, for a user which has the view monitoring permission only, it resulted in 403 error as followed

Error from server (Forbidden): endpoints is forbidden: User "u-rb45j" cannot list resource "endpoints" in API group "" in the namespace "cattle-monitoring-system"

Solution

This error occurs because the clusterrole monitoring-ui-view gives list permission with resouceName mentioned. Which means a user with this permission cannot list all the endpoints or even get the specified resourceName due to lack of get permission. I think just giving it get permission will fix this.

> k get endpoints rancher-monitoring-alertmanager  -n cattle-monitoring-system
Error from server (Forbidden): endpoints "rancher-monitoring-alertmanager" is forbidden: User "u-rb45j" cannot get resource "endpoints" in API group "" in the namespace "cattle-monitoring-system" 

Testing

Manually checked after applying this fix

> k get endpoints rancher-monitoring-alertmanager  -n cattle-monitoring-system
NAME                              ENDPOINTS           AGE
rancher-monitoring-alertmanager   10.42.136.29:9093   3h39m
> k get endpoints -n cattle-monitoring-system
Error from server (Forbidden): endpoints is forbidden: User "u-26md8" cannot list resource "endpoints" in API group "" in the namespace "cattle-monitoring-system"

Engineering Testing

Manual Testing

Automated Testing

QA Testing Considerations

Regressions Considerations

Backporting considerations

This might be happening in previous versions also

@joshmeranda
Copy link
Contributor

This fix looks ok to me.

@vatsalparekh can you update the commits here to follow the steps in https://github.com/rancher/charts/blob/dev-v2.7/docs/developing.md#making-changes-to-packages?

@geethub97 can you take a quick look?

Copy link
Contributor

@geethub97 geethub97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change itself looks fine, but can you please split the commit into two and also bump the version for monitoring and add that new version to the release yaml?
In the end you should have 3 commits:

  • one with changes to the packages directory
  • one for make charts
  • one to update the release yaml

Signed-off-by: Vatsal Parekh <vatsalparekh@outlook.com>
Signed-off-by: Vatsal Parekh <vatsalparekh@outlook.com>
Signed-off-by: Vatsal Parekh <vatsalparekh@outlook.com>
@geethub97 geethub97 merged commit d623750 into rancher:dev-v2.7 Jul 11, 2023
2 checks passed
@vatsalparekh vatsalparekh deleted the monitoring-ui-view-fix branch July 11, 2023 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants