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

Granting kibana_system reserved role access to "all" privileges to .internal.preview.alerts* index #80889

Merged
merged 4 commits into from
Nov 22, 2021

Conversation

dplumlee
Copy link
Contributor

@dplumlee dplumlee commented Nov 19, 2021

Required for: elastic/kibana#116374

Summary
An extension of #76624 and #80746. Adding for the new rule preview feature that utilizes alerts as data and a reserved index for kibana_system user to read/write alerts. We are writing to a separate internal index than normal alerts so they won't show up with standard .internal.alerts* queries, but still need the same permissions as "normal" alert indices

@dplumlee dplumlee added >enhancement :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC v8.0.0 Team:Security Meta label for security team external-contributor Pull request authored by a developer outside the Elasticsearch team auto-backport Automatically create backport pull requests when merged labels Nov 19, 2021
@dplumlee dplumlee self-assigned this Nov 19, 2021
@dplumlee dplumlee marked this pull request as ready for review November 19, 2021 21:28
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security (Team:Security)

Copy link
Member

@ywangd ywangd left a comment

Choose a reason for hiding this comment

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

LGTM

I am not very familiar with the alerts as data work. So this might not be the right question. But why do we have to prepend different strings to the .alerts namespace? We already granted Kibana all access to .alerts*, which seems to be a viable top-level namespace for covering all usages required by alerts-as-data? That is, is it more organised and easier if we use suffix instead of prefix for sub namespaces? i.e. instead of .internal.alerts*, .preview.alerts* and .internal.preview.alerts*, we replace them with .alerts.internal.*, .alerts.preview.*, .alerts.internal_preview.*?

@dplumlee
Copy link
Contributor Author

dplumlee commented Nov 22, 2021

@ywangd The way the new alerts as data feature is set up, it was intentional to separate the fields from the general .alerts* namespace as you mentioned. However, I definitely agree that it's something we should revisit later, this is just the consensus that the RAC team came to for the time being. Whether it be separating the fields using datasets or some other naming convention I think there's some room for improvement with fewer index permissions needed

@dplumlee dplumlee merged commit 46128da into master Nov 22, 2021
@dplumlee dplumlee deleted the alerts-as-data-internal-rule-preview branch November 22, 2021 20:01
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

@marshallmain
Copy link
Contributor

This appears not to have made it back to 8.0 and is required for #81379. Is there a backport script we can run to easily backport it?

ywangd pushed a commit to ywangd/elasticsearch that referenced this pull request Dec 6, 2021
elasticsearchmachine pushed a commit that referenced this pull request Dec 6, 2021
…nternal.preview.alerts* index (#80889) (#81392)

Co-authored-by: Davis Plumlee <56367316+dplumlee@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Automatically create backport pull requests when merged >enhancement external-contributor Pull request authored by a developer outside the Elasticsearch team :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC Team:Security Meta label for security team v8.0.0 v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants