-
Notifications
You must be signed in to change notification settings - Fork 508
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
Moving cluster indices permission to cluster section #1656 #7161
Moving cluster indices permission to cluster section #1656 #7161
Conversation
…ct#1656 Signed-off-by: AntonEliatra <anton.rubin@eliatra.com>
Converted to draft, as found permissions in Dashboards UI that user can only add as cluster, but should be added as index permission, indices:data/write/bulk* |
Signed-off-by: AntonEliatra <anton.rubin@eliatra.com>
Signed-off-by: AntonEliatra <anton.rubin@eliatra.com>
Signed-off-by: AntonEliatra <anton.rubin@eliatra.com>
Signed-off-by: AntonEliatra <anton.rubin@eliatra.com>
While working on this PR I came across incorrect/outdated API documentation and raised issue to fix it: |
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.
Looks good thanks Anton!
Co-authored-by: Naarcha-AWS <97990722+Naarcha-AWS@users.noreply.github.com> Signed-off-by: AntonEliatra <anton.rubin@eliatra.com>
@Naarcha-AWS , if this looks good, it can go to Nathan. An editorial request has already been submitted. |
Signed-off-by: Naarcha-AWS <97990722+Naarcha-AWS@users.noreply.github.com>
@natebower: This is ready for editorial review. |
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.
@AntonEliatra @Naarcha-AWS Please see my changes and confirm the phrasing I've flagged on lines 193 and 514. Thanks!
| `indices:data/read/scroll` | Permission to scroll data. | | ||
| `indices:data/read/scroll/clear` | Permission to clear read scroll data. | | ||
| `indices:data/read/scroll` | Permission to scroll data. This setting needs to be configured as both a cluster and index level permission. | | ||
| `indices:data/read/scroll/clear` | Permission to clear read scroll data. This setting needs to be configured as both a cluster and index level permission. | |
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.
Is "read scroll data" a thing?
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.
hey @natebower I'm not sure I follow, You mean read/scroll
to scroll through data/results or read/scroll/data
- which I haven't come across
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.
@AntonEliatra I suspect that "read scroll data" is meant to be past tense, but it reads as though "read scroll data" is a noun. Can we rephrase for clarity?
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.
Ah I see what you mean, will fix now
Co-authored-by: Nathan Bower <nbower@amazon.com> Signed-off-by: AntonEliatra <anton.rubin@eliatra.com>
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.
Please rephrase lines 193 and 514. Thanks!
Signed-off-by: AntonEliatra <anton.rubin@eliatra.com>
Co-authored-by: Nathan Bower <nbower@amazon.com> Signed-off-by: AntonEliatra <anton.rubin@eliatra.com>
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.
Thanks @AntonEliatra! LGTM
* Moving cluster indices permission to cluster section #1656 Signed-off-by: AntonEliatra <anton.rubin@eliatra.com> * updating index and cluster permissions Signed-off-by: AntonEliatra <anton.rubin@eliatra.com> * removing empty space Signed-off-by: AntonEliatra <anton.rubin@eliatra.com> * fixing vale errors Signed-off-by: AntonEliatra <anton.rubin@eliatra.com> * adding more permissions to the list Signed-off-by: AntonEliatra <anton.rubin@eliatra.com> * Apply suggestions from code review Co-authored-by: Naarcha-AWS <97990722+Naarcha-AWS@users.noreply.github.com> Signed-off-by: AntonEliatra <anton.rubin@eliatra.com> * Apply suggestions from code review Signed-off-by: Naarcha-AWS <97990722+Naarcha-AWS@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: Nathan Bower <nbower@amazon.com> Signed-off-by: AntonEliatra <anton.rubin@eliatra.com> * Update permissions.md Signed-off-by: AntonEliatra <anton.rubin@eliatra.com> * Update _security/access-control/permissions.md Co-authored-by: Nathan Bower <nbower@amazon.com> Signed-off-by: AntonEliatra <anton.rubin@eliatra.com> --------- Signed-off-by: AntonEliatra <anton.rubin@eliatra.com> Signed-off-by: Naarcha-AWS <97990722+Naarcha-AWS@users.noreply.github.com> Co-authored-by: Naarcha-AWS <97990722+Naarcha-AWS@users.noreply.github.com> Co-authored-by: Nathan Bower <nbower@amazon.com> (cherry picked from commit 88bab65) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Description
Moving cluster wide Index permissions to cluster permission section
Issues Resolved
Closes #2359
Closes #1656
Closes #621
Checklist
For more information on following Developer Certificate of Origin and signing off your commits, please check here.