-
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
[Security Solution][Endpoint] Update host isolation pending status API to work with new endpoint #115441
[Security Solution][Endpoint] Update host isolation pending status API to work with new endpoint #115441
Conversation
@elasticmachine merge upstream |
Pinging @elastic/security-onboarding-and-lifecycle-mgt (Team:Onboarding and Lifecycle Mgt) |
x-pack/plugins/security_solution/server/endpoint/services/actions.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/security_solution/server/endpoint/services/actions.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/security_solution/server/endpoint/services/actions.ts
Outdated
Show resolved
Hide resolved
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.
Left some code suggestions, let me know if something doesn't work on your side 🙂
Btw, it looks great! 🔥
x-pack/plugins/security_solution/server/endpoint/services/actions.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/security_solution/server/endpoint/services/actions.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/security_solution/server/endpoint/services/actions.ts
Outdated
Show resolved
Hide resolved
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.
there is a problem with async code mixing with sync code.
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.
the logic is there
some names were not very clear to me, but that's a nitpick at best
x-pack/plugins/security_solution/server/endpoint/services/actions.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/security_solution/server/endpoint/services/actions.ts
Outdated
Show resolved
Hide resolved
@@ -146,6 +147,45 @@ const getActivityLog = async ({ | |||
return sortedData; | |||
}; | |||
|
|||
const hasAckInResponse = (response: EndpointActionResponse): boolean => { |
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.
💯
x-pack/plugins/security_solution/server/endpoint/services/actions.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/security_solution/server/endpoint/services/actions.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/security_solution/server/endpoint/services/actions.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/security_solution/server/endpoint/services/actions.ts
Outdated
Show resolved
Hide resolved
return ackResponseActionIdList.includes(action.action_id) // if has ack | ||
? isLogsEndpointFilter({ action, agentId, indexedActionIds }) // then find responses in new index | ||
: legacyFilterCallback({ | ||
// else use the legacy way |
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.
comments here are 🔥
x-pack/plugins/security_solution/server/endpoint/services/actions.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/security_solution/server/endpoint/services/actions.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/security_solution/server/endpoint/services/actions.ts
Outdated
Show resolved
Hide resolved
review changes
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.
🐑 🐑 🐑
💚 Build SucceededMetrics [docs]
History
To update your PR or re-run it, just comment with: cc @ashokaditya |
…I to work with new endpoint (elastic#115441) * get the whole response when fetching responses by agentIds fixes elastic/security-team/issues/1705 * search new response index with actionId fixes elastic/security-team/issues/1705 * Find matching responses in new response index if fleet action response has an `ack` * review changes * hasIndexedDoc fix fetch logic * remove file * simplify code * add some acks to generator responses * meaningful names review changes Co-authored-by: Esteban Beltran <[email protected]>
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
…I to work with new endpoint (elastic#115441) * get the whole response when fetching responses by agentIds fixes elastic/security-team/issues/1705 * search new response index with actionId fixes elastic/security-team/issues/1705 * Find matching responses in new response index if fleet action response has an `ack` * review changes * hasIndexedDoc fix fetch logic * remove file * simplify code * add some acks to generator responses * meaningful names review changes Co-authored-by: Esteban Beltran <[email protected]>
…tus API to work with new endpoint (#115441) (#115691) * [Security Solution][Endpoint] Update host isolation pending status API to work with new endpoint (#115441) * get the whole response when fetching responses by agentIds fixes elastic/security-team/issues/1705 * search new response index with actionId fixes elastic/security-team/issues/1705 * Find matching responses in new response index if fleet action response has an `ack` * review changes * hasIndexedDoc fix fetch logic * remove file * simplify code * add some acks to generator responses * meaningful names review changes Co-authored-by: Esteban Beltran <[email protected]> * skip flakey test Co-authored-by: Ashokaditya <[email protected]> Co-authored-by: Esteban Beltran <[email protected]>
#115998) * add tests for pending status api changes related to /pull/115441 refs elastic/security-team/issues/1705 * update mock refs /pull/116214 Co-authored-by: Kibana Machine <[email protected]>
elastic#115998) * add tests for pending status api changes related to elastic/pull/115441 refs elastic/security-team/issues/1705 * update mock refs elastic/pull/116214 Co-authored-by: Kibana Machine <[email protected]>
elastic#115998) * add tests for pending status api changes related to elastic/pull/115441 refs elastic/security-team/issues/1705 * update mock refs elastic/pull/116214 Co-authored-by: Kibana Machine <[email protected]>
#115998) (#116924) * add tests for pending status api changes related to /pull/115441 refs elastic/security-team/issues/1705 * update mock refs /pull/116214 Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Ashokaditya <[email protected]>
#115998) (#116925) * add tests for pending status api changes related to /pull/115441 refs elastic/security-team/issues/1705 * update mock refs /pull/116214 Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Ashokaditya <[email protected]>
When looking for completed responses for responses with ACK. refs elastic/pull/115441
Summary
Adds logic to finding pending isolation/release status for new (
>=v7.16
) endpoints.Essentially, for
<=v71.6
endpoints, the legacy logic follows for pending action status, where the action is considered pending when there is no response yet. For>=v7.16
endpoints, action is pending if there is a response withack
in it, but there's no endpoint response.Checklist
Delete any items that are not applicable to this PR.