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] Add host isolation exceptions UI #111253

Merged
merged 40 commits into from Sep 30, 2021

Conversation

academo
Copy link
Contributor

@academo academo commented Sep 6, 2021

Summary

Adds host isolation exceptions. This is the first of a series of PR to introduce the full functionality.

This first PR:

  • Adds a new exception list_id to store the host isolation exceptions
  • Adds a new security sidebar menu entry for host isolation exceptions
  • Allows to list existing host isolation exceptions as cards (see image)
  • Host isolation exceptions for now, will only support to except IPs.

Note: No adding/editing/deleting functionality is introduced in this PR. These features will come in subsequent ones.

Notes to reviewers:

This PR consists of the following main parts:

  • Code to add a new menu entry found in these files:
    • x-pack/plugins/security_solution/public/app/deep_links/index.ts
    • x-pack/plugins/security_solution/public/app/home/home_navigations.ts
    • x-pack/plugins/security_solution/common/constants.ts
    • x-pack/plugins/security_solution/public/management/common/constants.ts
    • x-pack/plugins/security_solution/public/common/components/navigation/types.ts
    • x-pack/plugins/security_solution/public/common/components/navigation/use_security_solution_navigation/index.test.tsx
    • x-pack/plugins/security_solution/public/management/common/breadcrumbs.ts
    • x-pack/plugins/security_solution/public/common/components/navigation/use_security_solution_navigation/use_navigation_items.tsx
  • Code to add the list_id in the exceptions item, found in these files:
    • packages/kbn-securitysolution-io-ts-list-types/src/common/exception_list/index.ts
    • packages/kbn-securitysolution-io-ts-list-types/src/common/lists/index.test.ts
  • Code to add the actual host isolation exceptions listing, including all redux boilerplate:
    • All the code inside x-pack/plugins/security_solution/public/management/pages/host_isolation_exceptions/**

image

Checklist

Delete any items that are not applicable to this PR.

@academo academo changed the title Add isolation_ips_whitelist to the exception list endpoint types Add host isolation exceptions to the exception list endpoint types Sep 13, 2021
@academo academo force-pushed the feature/host_isolation_ips_whitelist branch 3 times, most recently from 8fea6e8 to 611319a Compare September 16, 2021 08:54
@academo academo force-pushed the feature/host_isolation_ips_whitelist branch from ef07fca to 0ce0223 Compare September 20, 2021 09:18
Copy link
Member

@ashokaditya ashokaditya left a comment

Choose a reason for hiding this comment

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

🚀 🐑

Copy link
Contributor

@dasansol92 dasansol92 left a comment

Choose a reason for hiding this comment

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

🔥 🔥 🔥
Suggestion: As there are some changes in the io-ts-list package I would suggest you to add someone from security-detections-response team as a reviewer

@academo academo requested a review from a team September 29, 2021 08:55
page?: number;
perPage?: number;
sortField?: keyof ExceptionListItemSchema;
sortOrder?: 'asc' | 'desc';
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: We have a SortOrder type that should be the same as re-doing this here but if you look across the code base the asc | desc has been retyped so many times you can pull from any number of duplicate types or just leave this as is here.

@kibanamachine
Copy link
Contributor

💛 Build succeeded, but was flaky


Test Failures

Kibana Pipeline / general / Allows the rule to be duplicated from the edit screen.indicator match Detection rules, Indicator Match Duplicates the indicator rule Allows the rule to be duplicated from the edit screen

Link to Jenkins

Stack Trace

Failed Tests Reporter:
  - Test has not failed recently on tracked branches

AssertionError: Timed out retrying after 60000ms: expected '<button.euiContextMenuItem>' to be 'visible'

This element `<button.euiContextMenuItem>` is not visible because its parent `<div>` has CSS property: `visibility: hidden`
    at Context.eval (http://localhost:61221/__cypress/tests?p=cypress/integration/detection_rules/indicator_match_rule.spec.ts:31198:58)

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 2253 2263 +10

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
lists 125.2KB 125.3KB +114.0B
securitySolution 4.3MB 4.3MB +7.3KB
total +7.4KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
securitySolution 109.8KB 109.8KB +80.0B

History

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

@academo academo merged commit a667038 into elastic:master Sep 30, 2021
@academo academo deleted the feature/host_isolation_ips_whitelist branch September 30, 2021 11:43
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Sep 30, 2021
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated: Automatically backport this PR after it's merged release_note:feature Makes this part of the condensed 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.

None yet

6 participants