-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[8.10] backport SLO fixes #168231
[8.10] backport SLO fixes #168231
Conversation
Documentation preview: |
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
Pinging @elastic/actionable-observability (Team: Actionable Observability) |
💚 Build Succeeded
Metrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: cc @kdelemme |
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.
LGTM... I tested it with an under privileged user, it worked as expected. I started ES without a transform node, the new error message handler caught it perfectly and returned me to the "Create SLO" form. I also created and SLO with a user who had the correct privileges, then edit the privileges to remove read access to high-*
and the transform continued to run but stopped indexing data due to not being able to read the data. Once I restored the privilege, it caught back up.
I am currently reviewing this. Code review looks good. I am testing it locally.
@simianhacker @kdelemme How can I do this? |
@kdelemme I created a user with read only privileges for SLO, but I don't see the 2 SLOs I have. Screen.Recording.2023-10-09.at.13.53.08.movI might probably be missing something, when creating my custom slo read role. How do you create a read only SLO role? I didn't specify anything to the |
@mgiota Yes you are missing the following index Privileges: |
@kdelemme Thanks! I added the Index Privilege and now works as expected. Corresponding actions are deactivated I played a bit more with custom roles. Here's a role I created:
When I try to edit the slo, timestamp field is not preselected and the SLI preview is empty. Not sure if a real user would create a role the way I created it, so not sure if this is a bug or not. What do you think? Screen.Recording.2023-10-10.at.14.59.20.mov |
That's expected, since you did not give any privileges (at least read) on the source index (.e.g the high-cardinality admin console index). |
@kdelemme I tested it and everything works as expected! |
# Backport This will backport the following commits from `main` to `8.11`: - [fix(slo): handle permission error (#167933)](#167933) <!--- Backport version: 8.9.8 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Kevin Delemme","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-10-05T13:17:32Z","message":"fix(slo): handle permission error (#167933)","sha":"335fc9b2409855f4aeebf360c0747141b2fcf03b","branchLabelMapping":{"^v8.12.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","backport:skip","Team: Actionable Observability","Feature:SLO","v8.12.0"],"number":167933,"url":"https://github.com/elastic/kibana/pull/167933","mergeCommit":{"message":"fix(slo): handle permission error (#167933)","sha":"335fc9b2409855f4aeebf360c0747141b2fcf03b"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.12.0","labelRegex":"^v8.12.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/167933","number":167933,"mergeCommit":{"message":"fix(slo): handle permission error (#167933)","sha":"335fc9b2409855f4aeebf360c0747141b2fcf03b"}},{"url":"https://github.com/elastic/kibana/pull/168231","number":168231,"branch":"8.10","state":"OPEN"}]}] BACKPORT-->
Those changes didn't make it on time into the latest BC for 8.10.3. Updating the labels. |
🍒 Summary
This PR backports the following commits into 8.10 for release scheduled on 8.10.4:
Release Notes
This PR is a mix of fixes and enhancements:
Testing notes
manage_transform
.slo-*
:all
high-*
:read
andread_cross_cluster
elastic
superuserelastic
user created. For the new SLO you should get an error that says you don't have the proper permissions and then end up on the create screen (not saved). When you hit save on the SLO (that theelastic
user created) you should end up with the same error without changing the original SLO.