-
Notifications
You must be signed in to change notification settings - Fork 279
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
fix: update internal labels endpoints to work with RBAC #5099
fix: update internal labels endpoints to work with RBAC #5099
Conversation
@@ -545,7 +546,7 @@ def web_link(self) -> str: | |||
@property | |||
def declare_incident_link(self) -> str: | |||
"""Generate a link for AlertGroup to declare Grafana Incident by click""" | |||
incident_link = urljoin(self.channel.organization.grafana_url, "a/grafana-incident-app/incidents/declare/") | |||
incident_link = urljoin(self.channel.organization.grafana_url, f"a/{PluginID.INCIDENT}/incidents/declare/") |
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'll finish renaming the hardcoded plugin IDs, to use PluginID
, in a subsequent PR
LABEL_CREATE = LegacyAccessControlCompatiblePermission( | ||
Resources.LABEL, Actions.CREATE, LegacyAccessControlRole.EDITOR, prefix=PluginID.LABELS | ||
) |
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 only see the Labels Reader
and Labels Writer
roles in the existing Grafana RBAC UI, is this PR introducing a new one for create? maybe it's a good idea to only keep read and write (so create is part of write)?
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.
tldr; no it's not introducing any new RBAC roles. The LABEL_CREATE
, LABEL_READ
, and LABEL_WRITE
variables are just helpers which translate to the permission strings grafana-labels-app:label:create
, grafana-labels-app:label:read
, and grafana-labels-app:label:write
, respectively.
Those are the RBAC roles that're available from Labels. Roles grant >= 1 permission(s) . These are the permissions granted by those labels RBAC roles:
"roles": [
{
"role": {
"name": "Labels Writer",
"description": "Read/write/create/delete Labels",
"permissions": [
{ "action": "plugins.app:access", "scope": "plugins:id:grafana-labels-app" },
{ "action": "grafana-labels-app.label:read" },
{ "action": "grafana-labels-app.label:write" },
{ "action": "grafana-labels-app.label:create" },
{ "action": "grafana-labels-app.label:delete" }
]
},
"grants": ["Admin", "Editor"]
},
{
"role": {
"name": "Labels Reader",
"description": "Read-only access to Labels",
"permissions": [
{ "action": "plugins.app:access", "scope": "plugins:id:grafana-labels-app" },
{ "action": "grafana-labels-app.label:read" }
]
},
"grants": ["Viewer"]
}
]
so in OnCall we're basically validating that the user has the necessary permission (action), we don't necessarily care about the role names themselves.
I agree that :write
and :create
should be consolidated over in labels but that's the current state of things. If you're curious, see these resources for more context:
Co-authored-by: Vadim Stepanov <vadimkerr@gmail.com>
What this PR does
Related to:
Checklist
pr:no public docs
PR label added if not required)release:
). These labels dictate how your PR willshow up in the autogenerated release notes.