-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[7.x] [Fleet] Add test/fix for invalid/missing ids in bulk agent reassign (#94632) #94774
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…94632) ## Problem While working on changes for bulk reassign #90437, I found that the server has a runtime error and returns a 500 if given an invalid or missing id. <details><summary>server error stack trace</summary> ``` │ proc [kibana] server log [12:21:48.953] [error][fleet][plugins] TypeError: Cannot read property 'policy_revision_idx' of undefined │ proc [kibana] at map (/Users/jfsiii/work/kibana/x-pack/plugins/fleet/server/services/agents/helpers.ts:15:34) │ proc [kibana] at Array.map (<anonymous>) │ proc [kibana] at getAgents (/Users/jfsiii/work/kibana/x-pack/plugins/fleet/server/services/agents/crud.ts:191:32) │ proc [kibana] at runMicrotasks (<anonymous>) │ proc [kibana] at processTicksAndRejections (internal/process/task_queues.js:93:5) │ proc [kibana] at Object.reassignAgents (/Users/jfsiii/work/kibana/x-pack/plugins/fleet/server/services/agents/reassign.ts:91:9) │ proc [kibana] at postBulkAgentsReassignHandler (/Users/jfsiii/work/kibana/x-pack/plugins/fleet/server/routes/agent/handlers.ts:314:21) │ proc [kibana] at Router.handle (/Users/jfsiii/work/kibana/src/core/server/http/router/router.ts:272:30) │ proc [kibana] at handler (/Users/jfsiii/work/kibana/src/core/server/http/router/router.ts:227:11) │ proc [kibana] at exports.Manager.execute (/Users/jfsiii/work/kibana/node_modules/@hapi/hapi/lib/toolkit.js:60:28) │ proc [kibana] at Object.internals.handler (/Users/jfsiii/work/kibana/node_modules/@hapi/hapi/lib/handler.js:46:20) │ proc [kibana] at exports.execute (/Users/jfsiii/work/kibana/node_modules/@hapi/hapi/lib/handler.js:31:20) │ proc [kibana] at Request._lifecycle (/Users/jfsiii/work/kibana/node_modules/@hapi/hapi/lib/request.js:370:32) │ proc [kibana] at Request._execute (/Users/jfsiii/work/kibana/node_modules/@hapi/hapi/lib/request.js:279:9) ``` </details> <details><summary>see test added in this PR fail on master</summary> ``` 1) Fleet Endpoints reassign agent(s) bulk reassign agents should allow to reassign multiple agents by id -- some invalid: Error: expected 200 "OK", got 500 "Internal Server Error" ``` </details> ## Root cause Debugging runtime error in `searchHitToAgent` found some TS type mismatches for the ES values being returned. Perhaps from one or more of the recent changes to ES client & Fleet Server. Based on `test:jest` and `test:ftr`, it appears the possible types are `GetResponse` or `SearchResponse`, instead of only an `ESSearchHit`. https://github.com/elastic/kibana/pull/94632/files#diff-254d0f427979efc3b442f78762302eb28fb9c8857df68ea04f8d411e052f939cL11 While a `.search` result will include return matched values, a `.get` or `.mget` will return a row for each input and a `found: boolean`. e.g. `{ _id: "does-not-exist", found: false }`. The error occurs when [`searchHitToAgent`](https://github.com/jfsiii/kibana/blob/1702cf98f018c41ec0a080d829a12403168ac242/x-pack/plugins/fleet/server/services/agents/helpers.ts#L11) is run on a get miss instead of a search hit. ## PR Changes * Added a test to ensure it doesn't fail if invalid or missing IDs are given * Moved the `bulk_reassign` tests to their own test section * Filter out any missing results before calling `searchHitToAgent`, to match current behavior * Consolidate repeated arguments into and code for getting agents into single [function](https://github.com/elastic/kibana/pull/94632/files#diff-f7377ed9ad56eaa8ea188b64e957e771ccc7a7652fd1eaf44251c25b930f8448R70-R87): and [TS type](https://github.com/elastic/kibana/pull/94632/files#diff-f7377ed9ad56eaa8ea188b64e957e771ccc7a7652fd1eaf44251c25b930f8448R61-R68) * Rename some agent service functions to be more explicit (IMO) but behavior maintained. Same API names exported. This moves toward the "one result (success or error) per given id" approach for #90437 ### Checklist - [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 # Conflicts: # x-pack/plugins/fleet/server/services/agents/crud.ts # x-pack/plugins/fleet/server/services/index.ts
💛 Build succeeded, but was flaky
Test FailuresKibana Pipeline / general / Chrome UI Functional Tests.test/functional/apps/management/_runtime_fields·js.management runtime fields create runtime field should create runtime fieldStandard Out
Stack Trace
Metrics [docs]
To update your PR or re-run it, just comment with: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backports the following commits to 7.x: