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

[ML] Fix saved object sync check for jobs which are hidden from the user #160266

Merged
merged 7 commits into from
Jun 23, 2023
Merged

[ML] Fix saved object sync check for jobs which are hidden from the user #160266

merged 7 commits into from
Jun 23, 2023

Conversation

jgowdyelastic
Copy link
Member

@jgowdyelastic jgowdyelastic commented Jun 22, 2023 •
edited by kibanamachine
Loading

Fixes a bug introduced in PR #146155
A user who cannot see all spaces will incorrectly be told that jobs which only exist in spaces they cannot see are in need of synchronisation.
The problem was caused by an accident replacement of the internalSavedObjectsClient function (which can see all spaces) with the cached saved objects client which can only see the user's allowed spaces.
The fix is to revert to the original code.

This particular scenario was not covered by API tests. The tests have also been updated in this PR.

@jgowdyelastic jgowdyelastic requested a review from a team as a code owner June 22, 2023 12:56
@jgowdyelastic jgowdyelastic self-assigned this Jun 22, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

darnautov
darnautov approved these changes Jun 22, 2023
View reviewed changes
Copy link
Contributor

@darnautov darnautov left a comment

Choose a reason for hiding this comment

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

LGTM

peteharverson
peteharverson approved these changes Jun 22, 2023
View reviewed changes
Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

Tested and LGTM

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #61 / Synthetics API Tests AddProjectMonitors handles updating package policies when project monitors are updated

Metrics [docs]

Unknown metric groups

ESLint disabled line counts

id before after diff
enterpriseSearch 13 15 +2
securitySolution 416 420 +4
total +6

Total ESLint disabled count

id before after diff
enterpriseSearch 14 16 +2
securitySolution 497 501 +4
total +6

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @jgowdyelastic

@jgowdyelastic jgowdyelastic merged commit 7aa1dca into elastic:main Jun 23, 2023
@kibanamachine kibanamachine mentioned this pull request Jun 23, 2023
Closed
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jun 23, 2023
@jgowdyelastic
[ML] Fix saved object sync check for jobs which are hidden from the u…
d0f1775 
…ser (elastic#160266)

Fixes a bug introduced in PR
elastic#146155
A user who cannot see all spaces will incorrectly be told that jobs
which only exist in spaces they cannot see are in need of
synchronisation.
The problem was caused by an accident replacement of the
`internalSavedObjectsClient` function (which can see all spaces) with
the cached saved objects client which can only see the user's allowed
spaces.
The fix is to revert to the original code.

This particular scenario was not covered by API tests. The tests have
also been updated in this PR.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

(cherry picked from commit 7aa1dca)
@kibanamachine
Copy link
Contributor

💔 Some backports could not be created

Status Branch Result
❌ 8.8 Backport failed because of merge conflicts
✅ 8.9

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

Manual backport

To create the backport manually run:

node scripts/backport --pr 160266

Questions ?

Please refer to the Backport tool documentation

@jgowdyelastic jgowdyelastic mentioned this pull request Jun 23, 2023
Merged
jgowdyelastic added a commit that referenced this pull request Jun 23, 2023
@jgowdyelastic
[ML] [8.8] fix saved object sync check for jobs in different space (#…
913ea51 
…160402)

Manual backport of #160266
@hp0620 hp0620 mentioned this pull request Oct 6, 2023
Open
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label May 5, 2024
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

7 similar comments
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@peteharverson peteharverson peteharverson approved these changes

@darnautov darnautov darnautov approved these changes

Assignees

@jgowdyelastic jgowdyelastic

Labels
backported Feature:Anomaly Detection ML anomaly detection Feature:Data Frame Analytics ML data frame analytics features Feature:3rd Party Models ML 3rd party models :ml release_note:fix v8.8.2 v8.9.0 v8.10.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@jgowdyelastic @elasticmachine @kibana-ci @kibanamachine @darnautov @peteharverson @jbudz