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

🌊 Play nice with ES #200253

Merged
merged 11 commits into from
Nov 26, 2024
Merged

Conversation

flash1293
Copy link
Contributor

@flash1293 flash1293 commented Nov 14, 2024

This PR implements two changes:

  • When syncing a stream, try to PUT the current mappings to the data stream - if this fails with illegal_argument_exception, do a rollover instead. This is similar to how fleet handles this situation
  • Before accessing streams, check whether the current user can read the current data stream and only return it if this is the case. Users with partial read access will only see a partial tree. This doesn't apply to writing changes as the user needs to be able to change index templates, pipelines and so on which requires admin privileges anyway

@flash1293 flash1293 added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting v9.0.0 labels Nov 14, 2024
@flash1293 flash1293 marked this pull request as draft November 14, 2024 20:46
@flash1293 flash1293 marked this pull request as ready for review November 14, 2024 20:47
@flash1293 flash1293 changed the title [Streams] Play nice with ES 🌊 Play nice with ES Nov 20, 2024
Copy link
Member

@simianhacker simianhacker 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 endable streams, created 3 forks (logs.mongodb, logs.nginx, logs.nginx.errors), create a user that only had read and view_index_metadata for both logs and logs.mongodb, logged in as that user to ensure I could only see logs and logs.mongodb when calling GET /api/streams.

@flash1293
Copy link
Contributor Author

I think we will need to get back to https://github.com/elastic/kibana/pull/200253/files#diff-e247f45f35fcc0ed73b1992cf1c79d1b49257ea2fb8b763188d79458c22c1557R101 at some point - this strategy won't scale well. I will check with the Elasticsearch team.

It should be fine for a first version though.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

@flash1293 flash1293 merged commit 8e67172 into elastic:main Nov 26, 2024
21 checks passed
paulinashakirova pushed a commit to paulinashakirova/kibana that referenced this pull request Nov 26, 2024
This PR implements two changes:
* When syncing a stream, try to PUT the current mappings to the data
stream - if this fails with `illegal_argument_exception`, do a rollover
instead. This is similar to how fleet handles this situation
* Before accessing streams, check whether the current user can read the
current data stream and only return it if this is the case. Users with
partial read access will only see a partial tree. This doesn't apply to
writing changes as the user needs to be able to change index templates,
pipelines and so on which requires admin privileges anyway

---------

Co-authored-by: kibanamachine <[email protected]>
@flash1293 flash1293 added backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) v8.17.0 v8.18.0 and removed backport:skip This commit does not require backporting labels Dec 2, 2024
@flash1293 flash1293 removed the v8.17.0 label Dec 2, 2024
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.17, 8.x

https://github.com/elastic/kibana/actions/runs/12122747001

@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.17, 8.x

https://github.com/elastic/kibana/actions/runs/12122747095

@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.17 Backport failed because of merge conflicts
8.x An unhandled error occurred. Please see the logs for details

Manual backport

To create the backport manually run:

node scripts/backport --pr 200253

Questions ?

Please refer to the Backport tool documentation

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Dec 2, 2024
This PR implements two changes:
* When syncing a stream, try to PUT the current mappings to the data
stream - if this fails with `illegal_argument_exception`, do a rollover
instead. This is similar to how fleet handles this situation
* Before accessing streams, check whether the current user can read the
current data stream and only return it if this is the case. Users with
partial read access will only see a partial tree. This doesn't apply to
writing changes as the user needs to be able to change index templates,
pipelines and so on which requires admin privileges anyway

---------

Co-authored-by: kibanamachine <[email protected]>
(cherry picked from commit 8e67172)
@kibanamachine
Copy link
Contributor

💔 Some backports could not be created

Status Branch Result
8.17 Backport failed because of merge conflicts
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Manual backport

To create the backport manually run:

node scripts/backport --pr 200253

Questions ?

Please refer to the Backport tool documentation

@flash1293 flash1293 added the Feature:Streams This is the label for the Streams Project label Dec 2, 2024
kibanamachine added a commit that referenced this pull request Dec 2, 2024
# Backport

This will backport the following commits from `main` to `8.x`:
- [🌊 Play nice with ES
(#200253)](#200253)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Joe
Reuter","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-26T06:46:36Z","message":"🌊
Play nice with ES (#200253)\n\nThis PR implements two changes:\r\n* When
syncing a stream, try to PUT the current mappings to the data\r\nstream
- if this fails with `illegal_argument_exception`, do a
rollover\r\ninstead. This is similar to how fleet handles this
situation\r\n* Before accessing streams, check whether the current user
can read the\r\ncurrent data stream and only return it if this is the
case. Users with\r\npartial read access will only see a partial tree.
This doesn't apply to\r\nwriting changes as the user needs to be able to
change index templates,\r\npipelines and so on which requires admin
privileges anyway\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<[email protected]>","sha":"8e671728614012a5f5c5efc68975d9c0139ccfb7","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","backport:prev-minor","v8.18.0"],"title":"🌊
Play nice with
ES","number":200253,"url":"https://github.com/elastic/kibana/pull/200253","mergeCommit":{"message":"🌊
Play nice with ES (#200253)\n\nThis PR implements two changes:\r\n* When
syncing a stream, try to PUT the current mappings to the data\r\nstream
- if this fails with `illegal_argument_exception`, do a
rollover\r\ninstead. This is similar to how fleet handles this
situation\r\n* Before accessing streams, check whether the current user
can read the\r\ncurrent data stream and only return it if this is the
case. Users with\r\npartial read access will only see a partial tree.
This doesn't apply to\r\nwriting changes as the user needs to be able to
change index templates,\r\npipelines and so on which requires admin
privileges anyway\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<[email protected]>","sha":"8e671728614012a5f5c5efc68975d9c0139ccfb7"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/200253","number":200253,"mergeCommit":{"message":"🌊
Play nice with ES (#200253)\n\nThis PR implements two changes:\r\n* When
syncing a stream, try to PUT the current mappings to the data\r\nstream
- if this fails with `illegal_argument_exception`, do a
rollover\r\ninstead. This is similar to how fleet handles this
situation\r\n* Before accessing streams, check whether the current user
can read the\r\ncurrent data stream and only return it if this is the
case. Users with\r\npartial read access will only see a partial tree.
This doesn't apply to\r\nwriting changes as the user needs to be able to
change index templates,\r\npipelines and so on which requires admin
privileges anyway\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<[email protected]>","sha":"8e671728614012a5f5c5efc68975d9c0139ccfb7"}},{"branch":"8.x","label":"v8.18.0","branchLabelMappingKey":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Joe Reuter <[email protected]>
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Dec 12, 2024
This PR implements two changes:
* When syncing a stream, try to PUT the current mappings to the data
stream - if this fails with `illegal_argument_exception`, do a rollover
instead. This is similar to how fleet handles this situation
* Before accessing streams, check whether the current user can read the
current data stream and only return it if this is the case. Users with
partial read access will only see a partial tree. This doesn't apply to
writing changes as the user needs to be able to change index templates,
pipelines and so on which requires admin privileges anyway

---------

Co-authored-by: kibanamachine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) Feature:Streams This is the label for the Streams Project release_note:skip Skip the PR/issue when compiling release notes v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants