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

[Security Solution][Endpoint] Host Isolation API changes #113621

Merged
merged 18 commits into from
Oct 13, 2021
Merged

[Security Solution][Endpoint] Host Isolation API changes #113621

merged 18 commits into from
Oct 13, 2021

Conversation

ashokaditya
Copy link
Member

@ashokaditya ashokaditya commented Oct 1, 2021

Summary

Host isolation API changes that take into account if the new endpoint data streams are present.
If yes then add an action request doc to the new data stream as the user before writing to the .fleet-actions index as Kibana.
If no then the host isolation API works as usual.

Checklist

Delete any items that are not applicable to this PR.

@ashokaditya ashokaditya self-assigned this Oct 1, 2021
@ashokaditya ashokaditya added auto-backport Deprecated - use backport:version if exact versions are needed release_note:breaking release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v7.16.0 v8.0.0 labels Oct 1, 2021
@ashokaditya ashokaditya marked this pull request as ready for review October 5, 2021 14:52
@ashokaditya ashokaditya requested a review from a team as a code owner October 5, 2021 14:52
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-onboarding-and-lifecycle-mgt (Team:Onboarding and Lifecycle Mgt)

@ashokaditya ashokaditya requested a review from pzl October 5, 2021 14:52
@academo
Copy link
Contributor

academo commented Oct 6, 2021

I am not fully familiar with the way the indices work. I only had nitpick comments about improving the code. Better if someone else that understand that logic also approves it.

@academo academo requested review from academo and removed request for academo October 6, 2021 07:36
@ashokaditya
Copy link
Member Author

@elasticmachine merge upstream

Copy link
Member

@pzl pzl left a comment

Choose a reason for hiding this comment

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

minor revisions

{ index: string; body: EndpointAction }
] = [
(ctx.core.elasticsearch.client.asCurrentUser.index as jest.Mock).mock.calls[0][0],
(ctx.core.elasticsearch.client.asInternalUser.index as jest.Mock).mock.calls[1][0],
Copy link
Member

Choose a reason for hiding this comment

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

So this is getting the first argument of the second call to asInternalUser.index(). What's the first thing we index as internal user via this path? Are we guaranteeing that it will always happen in that order?

Copy link
Member Author

@ashokaditya ashokaditya Oct 12, 2021

Choose a reason for hiding this comment

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

The first thing that is indexed is .logs-endpoint.actions and then .fleet-actions. This is because of the way I mocked the client to use internalUser when the index exists.

Comment on lines 14 to 18
code: string;
id: string;
code?: string;
id?: string;
message: string;
stack_trace: string;
type: string;
stack_trace?: string;
type?: string;
Copy link
Member Author

Choose a reason for hiding this comment

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

would be nice to know if some of these can be more specific! ECS doesn't tell much more than some of them being keywords. Can we decide on some error codes for the failed action request errors that we discussed? @pzl

https://www.elastic.co/guide/en/ecs/current/ecs-error.html#field-error-code

body: {
...doc,
error: {
code: '424',
Copy link
Member Author

Choose a reason for hiding this comment

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

This translates to a Failed Dependency http error code. Using a code would be nice to filter out these records quickly

Copy link
Member Author

Choose a reason for hiding this comment

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

What do you think @pzl

Copy link
Member

Choose a reason for hiding this comment

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

Do the error codes in these responses correlate to HTTP error elsewhere? If we don't have that established, 424 won't mean much the next time we see it until we look it up

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, not in Kibana that I know of. Should I add a comment here about the specific code? Or should I remove it altogether?

Copy link
Member

Choose a reason for hiding this comment

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

doesn't have a big impact, just leave it

review comment
@ashokaditya
Copy link
Member Author

@elasticmachine merge upstream

@ashokaditya
Copy link
Member Author

@elasticmachine merge upstream

@ashokaditya
Copy link
Member Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

cc @ashokaditya

@ashokaditya ashokaditya merged commit 1d71d42 into elastic:master Oct 13, 2021
@ashokaditya ashokaditya deleted the feat/olm-host_isolation_api_changes-1704 branch October 13, 2021 07:25
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Oct 13, 2021
)

* Use the new data stream (if exists) to write action request to
and then the fleet index. Else do as usual.

fixes elastic/security-team/issues/1704

* fix legacy tests

* add relevant additional tests

* remove duplicate test

* update tests

* cleanup

review changes
refs elastic/security-team/issues/1704

* fix lint

* Use correct mapping keys when writing to index

* write record on new index when action request fails to write to `.fleet-actions`

review comments

* better error message

review comment

Co-authored-by: Kibana Machine <[email protected]>
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Oct 13, 2021
…114757)

* Use the new data stream (if exists) to write action request to
and then the fleet index. Else do as usual.

fixes elastic/security-team/issues/1704

* fix legacy tests

* add relevant additional tests

* remove duplicate test

* update tests

* cleanup

review changes
refs elastic/security-team/issues/1704

* fix lint

* Use correct mapping keys when writing to index

* write record on new index when action request fails to write to `.fleet-actions`

review comments

* better error message

review comment

Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Ashokaditya <[email protected]>
jloleysens added a commit to jloleysens/kibana that referenced this pull request Oct 13, 2021
…ide-users-to-saving-ux

* 'master' of github.com:elastic/kibana: (133 commits)
  [DOCS] Indicate reports are a subscription feature (elastic#114653)
  Update namespace for indices (elastic#114612)
  [DOCS] Adds Logstash pipeline settings (elastic#114648)
  Bump EPR snapshot version used for tests (elastic#114529)
  [Security Solution] [Endpoint] Fleet summary card adjustments (elastic#114291)
  skip flaky suite (elastic#68400)
  [Visualizations] fix usage of optional dependencies (elastic#114286)
  [Security Solution] [Detections] Improves custom query rule upgrade test (elastic#114454)
  [fleet] Add Integration Preference selector (elastic#114432)
  [Reporting] Add new `data-render-error` attribute (elastic#114472)
  Replace EuiCodeEditor with CodeEditor in app-services code (elastic#114316)
  [data views] add getDefaultDataView method  (elastic#113891)
  [Security Solution] [Endpoint] Event filters uses the new card design (elastic#114126)
  [fleet] Tweak Header UI (elastic#114704)
  [APM] Filter on tx metrics for instance stats (elastic#114758)
  [APM] Fix typo in linting docs (elastic#114764)
  [Discover] Removing SavedObject usage for savedSearch (elastic#112983)
  [Fleet] Add Integration Policy Page Improvements (elastic#114556)
  [Lens] Keep the custom label when transitioning to/from Formula (elastic#114270)
  [Security Solution][Endpoint] Host Isolation API changes (elastic#113621)
  ...
ashokaditya added a commit that referenced this pull request Oct 19, 2021
* rename legacy actions/responses

fixes elastic/security-team/issues/1702

* use correct name for responses index

refs /pull/113621

* extract helper method to utils

* append endpoint responses docs to activity log

* Show completed responses on activity log

fixes elastic/security-team/issues/1703

* remove width restriction on date picker

* add a simple test to verify endpoint responses

fixes elastic/security-team/issues/1702

* find unique action_ids from `.fleet-actions` and `.logs-endpoint.actions-default` indices

fixes elastic/security-team/issues/1702

* do not filter out endpoint only actions/responses that did not make it to Fleet

review comments

* use a constant to manage various doc types

review comments

* refactor `getActivityLog`

Simplify `getActivityLog` so it is easier to reason with.
review comments

* skip this for now

will mock this better in a new PR

* improve types

* display endpoint actions similar to fleet actions, but with success icon color

* Correctly do mocks for tests

* Include only errored endpoint actions, remove successful duplicates

fixes elastic/security-team/issues/1703

* Update tests to use non duplicate action_ids

review comments
fixes elastic/security-team/issues/1703

* show correct action title

review fixes

* statusCode constant

review change

* rename

review changes

* Update translations.ts

refs 74a8340

Co-authored-by: Kibana Machine <[email protected]>
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Oct 19, 2021
)

* rename legacy actions/responses

fixes elastic/security-team/issues/1702

* use correct name for responses index

refs elastic/pull/113621

* extract helper method to utils

* append endpoint responses docs to activity log

* Show completed responses on activity log

fixes elastic/security-team/issues/1703

* remove width restriction on date picker

* add a simple test to verify endpoint responses

fixes elastic/security-team/issues/1702

* find unique action_ids from `.fleet-actions` and `.logs-endpoint.actions-default` indices

fixes elastic/security-team/issues/1702

* do not filter out endpoint only actions/responses that did not make it to Fleet

review comments

* use a constant to manage various doc types

review comments

* refactor `getActivityLog`

Simplify `getActivityLog` so it is easier to reason with.
review comments

* skip this for now

will mock this better in a new PR

* improve types

* display endpoint actions similar to fleet actions, but with success icon color

* Correctly do mocks for tests

* Include only errored endpoint actions, remove successful duplicates

fixes elastic/security-team/issues/1703

* Update tests to use non duplicate action_ids

review comments
fixes elastic/security-team/issues/1703

* show correct action title

review fixes

* statusCode constant

review change

* rename

review changes

* Update translations.ts

refs elastic@74a8340

Co-authored-by: Kibana Machine <[email protected]>
kibanamachine added a commit that referenced this pull request Oct 19, 2021
…115492)

* rename legacy actions/responses

fixes elastic/security-team/issues/1702

* use correct name for responses index

refs /pull/113621

* extract helper method to utils

* append endpoint responses docs to activity log

* Show completed responses on activity log

fixes elastic/security-team/issues/1703

* remove width restriction on date picker

* add a simple test to verify endpoint responses

fixes elastic/security-team/issues/1702

* find unique action_ids from `.fleet-actions` and `.logs-endpoint.actions-default` indices

fixes elastic/security-team/issues/1702

* do not filter out endpoint only actions/responses that did not make it to Fleet

review comments

* use a constant to manage various doc types

review comments

* refactor `getActivityLog`

Simplify `getActivityLog` so it is easier to reason with.
review comments

* skip this for now

will mock this better in a new PR

* improve types

* display endpoint actions similar to fleet actions, but with success icon color

* Correctly do mocks for tests

* Include only errored endpoint actions, remove successful duplicates

fixes elastic/security-team/issues/1703

* Update tests to use non duplicate action_ids

review comments
fixes elastic/security-team/issues/1703

* show correct action title

review fixes

* statusCode constant

review change

* rename

review changes

* Update translations.ts

refs 74a8340

Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Ashokaditya <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v7.16.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants