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

Fix resolve index API to not throw 500 when encountering no_such_remote_cluster_exception #204802

Merged
merged 10 commits into from
Dec 19, 2024

Conversation

lukasolson
Copy link
Member

@lukasolson lukasolson commented Dec 18, 2024

Summary

Fixes #197747.

Updates the /internal/index-pattern-management/resolve_index/{query} route to properly handle no_such_remote_cluster_exception and return 404 rather than 500 server error.

Adds unit tests for the route handler.

Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

@lukasolson lukasolson marked this pull request as ready for review December 18, 2024 19:47
@lukasolson lukasolson requested a review from a team as a code owner December 18, 2024 19:47
@lukasolson lukasolson self-assigned this Dec 18, 2024
@lukasolson lukasolson added Feature:Data Views Data Views code and UI - index patterns before 8.0 release_note:skip Skip the PR/issue when compiling release notes Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Dec 18, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

Copy link
Contributor

@jughosta jughosta left a comment

Choose a reason for hiding this comment

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

Overall looks good!
Added a suggestion for having API integration tests too if posible.

},
};

describe('preview_scripted_field route', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
describe('preview_scripted_field route', () => {
describe('resolve_index route', () => {

Copy link
Member Author

@lukasolson lukasolson Dec 19, 2024

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we also extend the integration tests with this case?
test/api_integration/apis/data_views/resolve_index/resolve_index.ts

Copy link
Member Author

@lukasolson lukasolson Dec 19, 2024

Choose a reason for hiding this comment

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

Added in 322412c. (We already had a test for should return 404 for an exact match index but I added one for should return 200 for a search for indices with wildcard.)

@lukasolson lukasolson requested a review from jughosta December 19, 2024 17:17
Copy link
Contributor

@jughosta jughosta left a comment

Choose a reason for hiding this comment

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

Great, thanks!

@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

✅ unchanged

History

cc @lukasolson

@lukasolson lukasolson merged commit 0952f6e into elastic:main Dec 19, 2024
9 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

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

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Dec 19, 2024
…ote_cluster_exception` (elastic#204802)

## Summary

Fixes elastic#197747.

Updates the `/internal/index-pattern-management/resolve_index/{query}`
route to properly handle `no_such_remote_cluster_exception` and return
`404` rather than `500` server error.

Adds unit tests for the route handler.

### Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

- [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

---------

Co-authored-by: Julia Rechkunova <[email protected]>
(cherry picked from commit 0952f6e)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.x

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

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Dec 19, 2024
…no_such_remote_cluster_exception&#x60; (#204802) (#205016)

# Backport

This will backport the following commits from `main` to `8.x`:
- [Fix resolve index API to not throw 500 when encountering
&#x60;no_such_remote_cluster_exception&#x60;
(#204802)](#204802)

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

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

<!--BACKPORT [{"author":{"name":"Lukas
Olson","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-12-19T21:55:29Z","message":"Fix
resolve index API to not throw 500 when encountering
`no_such_remote_cluster_exception` (#204802)\n\n## Summary\r\n\r\nFixes
https://github.com/elastic/kibana/issues/197747.\r\n\r\nUpdates the
`/internal/index-pattern-management/resolve_index/{query}`\r\nroute to
properly handle `no_such_remote_cluster_exception` and return\r\n`404`
rather than `500` server error.\r\n\r\nAdds unit tests for the route
handler.\r\n\r\n### Checklist\r\n\r\nCheck the PR satisfies following
conditions. \r\n\r\nReviewers should verify this PR satisfies this list
as well.\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: Julia Rechkunova
<[email protected]>","sha":"0952f6e0f52771b3503c9ddd2afd793d8ed86709","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Data
Views","release_note:skip","v9.0.0","Team:DataDiscovery","backport:prev-minor"],"title":"Fix
resolve index API to not throw 500 when encountering
`no_such_remote_cluster_exception`","number":204802,"url":"https://github.com/elastic/kibana/pull/204802","mergeCommit":{"message":"Fix
resolve index API to not throw 500 when encountering
`no_such_remote_cluster_exception` (#204802)\n\n## Summary\r\n\r\nFixes
https://github.com/elastic/kibana/issues/197747.\r\n\r\nUpdates the
`/internal/index-pattern-management/resolve_index/{query}`\r\nroute to
properly handle `no_such_remote_cluster_exception` and return\r\n`404`
rather than `500` server error.\r\n\r\nAdds unit tests for the route
handler.\r\n\r\n### Checklist\r\n\r\nCheck the PR satisfies following
conditions. \r\n\r\nReviewers should verify this PR satisfies this list
as well.\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: Julia Rechkunova
<[email protected]>","sha":"0952f6e0f52771b3503c9ddd2afd793d8ed86709"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/204802","number":204802,"mergeCommit":{"message":"Fix
resolve index API to not throw 500 when encountering
`no_such_remote_cluster_exception` (#204802)\n\n## Summary\r\n\r\nFixes
https://github.com/elastic/kibana/issues/197747.\r\n\r\nUpdates the
`/internal/index-pattern-management/resolve_index/{query}`\r\nroute to
properly handle `no_such_remote_cluster_exception` and return\r\n`404`
rather than `500` server error.\r\n\r\nAdds unit tests for the route
handler.\r\n\r\n### Checklist\r\n\r\nCheck the PR satisfies following
conditions. \r\n\r\nReviewers should verify this PR satisfies this list
as well.\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: Julia Rechkunova
<[email protected]>","sha":"0952f6e0f52771b3503c9ddd2afd793d8ed86709"}}]}]
BACKPORT-->

Co-authored-by: Lukas Olson <[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:Data Views Data Views code and UI - index patterns before 8.0 release_note:skip Skip the PR/issue when compiling release notes Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. v8.18.0 v9.0.0
Projects
None yet
4 participants