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.

Sign up for GitHub

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

Jump to bottom

Docs: Update RBAC documentation #75869

Merged
merged 5 commits into from Oct 4, 2023
Merged

Docs: Update RBAC documentation #75869

merged 5 commits into from Oct 4, 2023

Conversation

mgyongyosi
Copy link
Contributor

What is this feature?
This PR updated the RBAC related documentation:

  • update the permission list with accepted actions and scopes
  • add example validation error response
  • small fixes

Why do we need this feature?

Who is this feature for?

Which issue(s) does this PR fix?:

Fixes #

Special notes for your reviewer:

Please check that:

  • It works as expected from a user's perspective.
  • If this is a pre-GA feature, it is behind a feature toggle.
  • The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

@mgyongyosi mgyongyosi added this to the 10.2.x milestone Oct 3, 2023
chri2547
chri2547 reviewed Oct 3, 2023
View reviewed changes
docs/sources/developers/http_api/access_control.md
> It has been enabled by default since Grafana 10.2

##### Invalid action

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add a single introduction sentence here that describes what the reader is seeing in this section. Something like "The following example shows...".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

chri2547
chri2547 reviewed Oct 3, 2023
View reviewed changes
docs/sources/developers/http_api/access_control.md
```

##### Invalid scope

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add a single introduction sentence here that describes what the reader is seeing in this section. Something like "The following example shows...".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

chri2547
chri2547 reviewed Oct 3, 2023
View reviewed changes
docs/sources/developers/http_api/access_control.md Outdated Show resolved Hide resolved
docs/sources/developers/http_api/access_control.md Outdated

> It has been enabled by default since Grafana 10.2

[Same as the create role validation errors.]({{< relref "#create-role-validation-errors" >}})
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't recommend linking an entire sentence. Write link text as follows:

For more information, refer to Create role validation errors.

Please note that you don't need a relref for anchors.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done and changed relref to ref ;)

@chri2547
Copy link
Collaborator

chri2547 commented Oct 3, 2023

@mgyongyosi - thanks so much for the contribution. Please ping me for another review. Thank you!

ethirolle
ethirolle approved these changes Oct 3, 2023
View reviewed changes
Copy link

@ethirolle ethirolle left a comment

Choose a reason for hiding this comment

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

LGTM!

mgyongyosi and others added 2 commits October 4, 2023 09:26
@mgyongyosi @chri2547
Apply suggestions from code review
28a3b4d 
Co-authored-by: Christopher Moyer <35463610+chri2547@users.noreply.github.com>
@mgyongyosi
Update based on reviews
8da139b
chri2547
chri2547 approved these changes Oct 4, 2023
View reviewed changes
Copy link
Collaborator

@chri2547 chri2547 left a comment

Choose a reason for hiding this comment

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

LGTM!

@mgyongyosi mgyongyosi merged commit e1aad9c into main Oct 4, 2023
12 checks passed
@mgyongyosi mgyongyosi deleted the mgyongyosi/update-rbac-permission-table branch October 4, 2023 15:53
@zerok zerok modified the milestones: 10.2.x, 10.2.0 Oct 23, 2023
@dnhn dnhn mentioned this pull request Oct 24, 2023
Closed
@BrewTestBot BrewTestBot mentioned this pull request Oct 25, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jguer Jguer Jguer approved these changes

@ethirolle ethirolle ethirolle approved these changes

@chri2547 chri2547 chri2547 approved these changes

@IevaVasiljeva IevaVasiljeva Awaiting requested review from IevaVasiljeva

@Eve832 Eve832 Awaiting requested review from Eve832 Eve832 is a code owner

@GrafanaWriter GrafanaWriter Awaiting requested review from GrafanaWriter

Assignees
No one assigned
Labels
add to changelog area/auth/rbac Grafana role-based access control no-backport Skip backport of PR team/identity-access type/docs
Projects
None yet
Milestone
10.2.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@mgyongyosi @chri2547 @Jguer @ethirolle @zerok