[Security Solution][Exceptions] - Moves remaining exceptions builder logic into lists plugin #95266

yctercero · 2021-03-24T05:58:56Z

Summary

Moves part of the exceptions UI out of the security solution plugin and into the lists plugin. In order to keep PRs (relatively) small, I am moving single components at a time. This should also then help more easily pinpoint the source of any issues that come up along the way.

The next couple PRs will focus on the exception builder. This one in particular is focused on moving over the ExceptionBuilderComponent which deals with rendering numerous exception items and their entries.

Quick Summary:

x-pack/plugins/security_solution/public/common/components/exceptions/builder/ → x-pack/plugins/lists/public/exceptions/components/builder/
- Corresponding unit test file moved as well
- Updated security solution exception builder to pull ExceptionBuilderComponent from lists plugin

Entry Item (PR)

Exception Item (PR)

Builder (THIS PR DEALS WITH 👇🏽)

Builder Buttons (THIS PR DEALS WITH 👇🏽)

Exception Item Comments

Builder Modal

Notes

There may still be work to do to get the component working for all use cases, however, for the moment being, concentrating on moving existing functionality over

Testing

Prior to making these changes, added a number of tests to ensure coverage of existing functionality. For manual testing, please checkout PR and play around with the following:

Add/edit an exception item
- if an existing item, make sure that it populates as expected in the modal
Add multiple exception items (using or operator)
Play around with the component in storybook

Checklist

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

elasticmachine · 2021-03-26T05:44:55Z

Pinging @elastic/security-solution (Team: SecuritySolution)

elasticmachine · 2021-03-26T05:44:56Z

Pinging @elastic/security-detections-response (Team:Detections and Resp)

ghost · 2021-03-31T12:37:21Z

Hi @yctercero

we have looked in the ticket description and its on-going conversation and came up with some of the observation and Query .

Build Details:

Version: 7.13.0-SNAPSHOT
Commit:e5f116a79c0d3acb783669693b42f030d32c2ac1
Build:39905

so can you please look into our observation if we are thinking in right direction and provide update on the queries raised for this ticket.

Queries:

Query 1: On going through through the ticket details and conversion [Security Solution][Exceptions] - Moves remaining exceptions builder logic into lists plugin #95266 (comment) , apart from changing current security solution components for exception form to list plugin , there have changes done to the decreasing the elements size/limit. so can you share more details if we are required to test this out on endpoint/rule exception form fields values. (eg . adding exception with long filename)

Module Count

Fewer modules leads to a faster build time

id before after diff

lists 156 165 +9

securitySolution 2206 2201 -5

total +4

Query 2: As shared above changes are being done from old security solution components for exception form to new list plugin. However we are observed still some changed here [Security Solution][Exceptions] - Moves remaining exceptions builder logic into lists plugin #95266 (comment) being done for the security solution elements . As these are going to change in list plugin . so do we have exception reason for there updation too.
Query 3: Is there any differentiation point to check when the purposed changes mentioned in this ticket will be implemented . However this pr is still not merged but we checked for changed on 7.13.0-SNAPSHOT OR we just have check whether after the changes being merged there should not be impact on ongoing exception funtionality

Observations:

Please find below checkpoints checked from these changed done on 7.13.0-SNAPHOT having the latest commit changes.

S.No	Scenario	Status	Remark
1	Validate user is able to create endpoint exception	PASS	no alert for the saved exception received at Detection Tab alert List and Host Tab External Alert List
2	Validate Endpoint Exception with OR and AND Clause	PASS
3	Validate user is able to edit saved endpoint exception	PASS	no alert received for the updated exception parameter and alert received for old parameters which we have updated
4	Validate Endpoint Exception Form Filed should be Pre-Populated	PASS
5	Validate No Error Prompt should received on creating Endpoint Exception	PASS
6	Validate user can add multiple Exception Entries		Tested up to 20 entries including both exceptions rule and endpoint
7	Validate Bulk Alert Close state on Close All checkbox	PASS
8	Validate Case Sensitivity for Endpoint Exception Fields	PASS	Tested with `file.path.caseless` field
9	General Check for Exception	PASS	Commenting , auto value feed on typing , saving exception without any parameter
10	Validate user is able to Create Rule exception		no alert for the saved exception received at Detection Tab alert List but received at Host Tab External Alert List
11	Validate Rule Exception Form Filed should not be Pre-Populated	PASS

Found 01 Issue during testing:
#95910

Please let us known if we are missing something or to validate anything more for this ticket.

thanks !!

yctercero · 2021-03-31T15:49:35Z

Query 1: On going through through the ticket details and conversion #95266 (comment) , apart from changing current security solution components for exception form to list plugin , there have changes done to the decreasing the elements size/limit. so can you share more details if we are required to test this out on endpoint/rule exception form fields values. (eg . adding exception with long filename)

The changes in bundle size don't need to be tested. In talking with Tyler it's clear that the lists plugin bundle size should be near 0 right now as the client side code is not needed on load. That is something that will need to be worked on. For this PR we are simply increasing the lists plugin and decreasing the security_solution plugin since we're just moving code from one to the other.

Query 2: As shared above changes are being done from old security solution components for exception form to new list plugin. However we are observed still some changed here #95266 (comment) being done for the security solution elements . As these are going to change in list plugin . so do we have exception reason for there updation too.

As part of the RAC initiative we are preparing these components to be used in any plugin that may need them and as such they need to be made more generic. The changes made in to the security solution were in line with this.

Query 3: Is there any differentiation point to check when the purposed changes mentioned in this ticket will be implemented . However this pr is still not merged but we checked for changed on 7.13.0-SNAPSHOT OR we just have check whether after the changes being merged there should not be impact on ongoing exception funtionality

I pinged QA here because a file owned by QA is modified https://github.com/elastic/kibana/pull/95266/files#diff-0225f90a5daddb79c38b634443c805cf06cd83d05ebadc98c9d66883a1458049 so it needs QA code owners review. Because this work was chunked, it probably is best to wait for it all to be merged to 7.13 before testing it in 7.13. There should be no functionality changes to exceptions.

Found 01 Issue during testing:
#95910

I checked that this issue does not occur in this PR. As you can see below, field suggestions appear as expected:

ghost · 2021-04-01T10:47:34Z

Query 1: On going through through the ticket details and conversion #95266 (comment) , apart from changing current security solution components for exception form to list plugin , there have changes done to the decreasing the elements size/limit. so can you share more details if we are required to test this out on endpoint/rule exception form fields values. (eg . adding exception with long filename)

The changes in bundle size don't need to be tested. In talking with Tyler it's clear that the lists plugin bundle size should be near 0 right now as the client side code is not needed on load. That is something that will need to be worked on. For this PR we are simply increasing the lists plugin and decreasing the security_solution plugin since we're just moving code from one to the other.

Query 2: As shared above changes are being done from old security solution components for exception form to new list plugin. However we are observed still some changed here #95266 (comment) being done for the security solution elements . As these are going to change in list plugin . so do we have exception reason for there updation too.

As part of the RAC initiative we are preparing these components to be used in any plugin that may need them and as such they need to be made more generic. The changes made in to the security solution were in line with this.

Query 3: Is there any differentiation point to check when the purposed changes mentioned in this ticket will be implemented . However this pr is still not merged but we checked for changed on 7.13.0-SNAPSHOT OR we just have check whether after the changes being merged there should not be impact on ongoing exception funtionality

I pinged QA here because a file owned by QA is modified https://github.com/elastic/kibana/pull/95266/files#diff-0225f90a5daddb79c38b634443c805cf06cd83d05ebadc98c9d66883a1458049 so it needs QA code owners review. Because this work was chunked, it probably is best to wait for it all to be merged to 7.13 before testing it in 7.13. There should be no functionality changes to exceptions.

Found 01 Issue during testing:
#95910

I checked that this issue does not occur in this PR. As you can see below, field suggestions appear as expected:

thanks @yctercero for looking into out queries and providing feedback on them.

However for the issue we will keep track this PR merge , will close issue if it get resolved .

yctercero · 2021-04-05T02:49:46Z

@elasticmachine merge upstream

yctercero · 2021-04-05T03:38:09Z

@elasticmachine merge upstream

dmlemeshko · 2021-04-06T11:16:39Z

I don't see any code changes that require @elastic/kibana-qa review. Is it still relevant?

MadameSheema · 2021-04-06T12:19:13Z

@dmlemeshko is relevant because without the review of the owner the PR cannot be merged.

dmlemeshko

I finally found removed test that is related to code coverage (owned by Kibana-QA).

dmlemeshko · 2021-04-06T13:17:18Z

src/dev/code_coverage/ingest_coverage/__tests__/enumerate_patterns.test.js

@@ -35,14 +35,4 @@ describe(`enumeratePatterns`, () => {
      'src/plugins/charts/public/static/color_maps/color_maps.ts kibana-app'
    );
  });
-  it(`should resolve x-pack/plugins/security_solution/public/common/components/exceptions/builder/translations.ts to kibana-security`, () => {


Hey @dplumlee This test was "unlucky" to randomly select path that you changed. I think you just need to update it with one you have now and we are good. But please don't remove it completely

kibanamachine · 2021-04-06T17:06:25Z

💛 Build succeeded, but was flaky

continuous-integration/kibana-ci/pull-request
Commit: dfb5ce6
Storybooks Preview
Flaky suites:
- xpack-securitySolutionCypressChrome

Test Failures

Kibana Pipeline / general / sets and reads the url state for timeline by id.url state sets and reads the url state for timeline by id

Link to Jenkins

Stack Trace

Failed Tests Reporter:
  - Test has failed 20 times on tracked branches: https://github.com/elastic/kibana/issues/61612

CypressError: `cy.type()` failed because it targeted a disabled element.

The element typed into was:

  > <input type="text" id="ic2785f21-96f3-11eb-a2b3-15cee924fadd" placeholder="Untitled timeline" class="euiFieldText euiFieldText--fullWidth" data-test-subj="save-timeline-title" aria-label="Title" spellcheck="true" value="S" disabled="">

Ensure the element does not have an attribute named `disabled` before typing into it.

https://on.cypress.io/type
    at validateTyping (http://elastic:changeme@localhost:6121/__cypress/runner/cypress_runner.js:162278:65)
    at http://elastic:changeme@localhost:6121/__cypress/runner/cypress_runner.js:162573:15
    at tryCatcher (http://elastic:changeme@localhost:6121/__cypress/runner/cypress_runner.js:10609:23)
    at Function.Promise.attempt.Promise.try (http://elastic:changeme@localhost:6121/__cypress/runner/cypress_runner.js:7883:29)
    at http://elastic:changeme@localhost:6121/__cypress/runner/cypress_runner.js:162620:65
    at tryCatcher (http://elastic:changeme@localhost:6121/__cypress/runner/cypress_runner.js:10609:23)
    at Object.gotValue (http://elastic:changeme@localhost:6121/__cypress/runner/cypress_runner.js:9751:18)
    at Object.gotAccum (http://elastic:changeme@localhost:6121/__cypress/runner/cypress_runner.js:9740:25)
    at Object.tryCatcher (http://elastic:changeme@localhost:6121/__cypress/runner/cypress_runner.js:10609:23)
    at Promise._settlePromiseFromHandler (http://elastic:changeme@localhost:6121/__cypress/runner/cypress_runner.js:8544:31)
    at Promise._settlePromise (http://elastic:changeme@localhost:6121/__cypress/runner/cypress_runner.js:8601:18)
    at Promise._settlePromise0 (http://elastic:changeme@localhost:6121/__cypress/runner/cypress_runner.js:8646:10)
    at Promise._settlePromises (http://elastic:changeme@localhost:6121/__cypress/runner/cypress_runner.js:8726:18)
    at Promise._fulfill (http://elastic:changeme@localhost:6121/__cypress/runner/cypress_runner.js:8670:18)
    at Promise._resolveCallback (http://elastic:changeme@localhost:6121/__cypress/runner/cypress_runner.js:8464:57)
    at Promise._settlePromiseFromHandler (http://elastic:changeme@localhost:6121/__cypress/runner/cypress_runner.js:8556:17)
From Your Spec Code:
    at Object.addNameToTimeline (http://localhost:6121/__cypress/tests?p=cypress/integration/urls/state.spec.ts:16345:45)
    at Context.eval (http://localhost:6121/__cypress/tests?p=cypress/integration/urls/state.spec.ts:15158:20)

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`lists`	156	165	+9
`securitySolution`	2208	2203	-5
total			+4

Async chunks

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

id	before	after	diff
`securitySolution`	7.3MB	7.3MB	-16.1KB

Page load bundle

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

id	before	after	diff
`lists`	190.7KB	208.9KB	+18.3KB

History

💚 Build #117178 succeeded e4e1785
💚 Build #116892 succeeded cb9a0ca
💔 Build #116761 failed 47ea537
💔 Build #116752 failed 47ea537
💛 Build #116562 was flaky 47ea537

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

cc @yctercero

dmlemeshko

LGTM

kibanamachine · 2021-04-06T18:29:17Z

💔 Backport failed

❌ 7.x: Commit could not be cherrypicked due to conflicts

To backport manually run:
node scripts/backport --pr 95266

…logic into lists plugin (elastic#95266) ## Summary Moves part of the exceptions UI out of the security solution plugin and into the lists plugin. In order to keep PRs (relatively) small, I am moving single components at a time. This should also then help more easily pinpoint the source of any issues that come up along the way. The next couple PRs will focus on the exception builder. This one in particular is focused on moving over the `ExceptionBuilderComponent` which deals with rendering numerous exception items and their entries. Quick Summary: - `x-pack/plugins/security_solution/public/common/components/exceptions/builder/` → ` x-pack/plugins/lists/public/exceptions/components/builder/` - Corresponding unit test file moved as well - Updated security solution exception builder to pull `ExceptionBuilderComponent` from lists plugin # Conflicts: # packages/kbn-optimizer/limits.yml # src/dev/code_coverage/ingest_coverage/__tests__/enumerate_patterns.test.js

kibanamachine · 2021-04-08T18:30:18Z

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

kibanamachine · 2021-04-09T18:40:16Z

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

…ilder logic into lists plugin (#95266) (#96358) * [Security Solution][Exceptions] - Moves remaining exceptions builder logic into lists plugin (#95266) ## Summary Moves part of the exceptions UI out of the security solution plugin and into the lists plugin. In order to keep PRs (relatively) small, I am moving single components at a time. This should also then help more easily pinpoint the source of any issues that come up along the way. The next couple PRs will focus on the exception builder. This one in particular is focused on moving over the `ExceptionBuilderComponent` which deals with rendering numerous exception items and their entries. Quick Summary: - `x-pack/plugins/security_solution/public/common/components/exceptions/builder/` → ` x-pack/plugins/lists/public/exceptions/components/builder/` - Corresponding unit test file moved as well - Updated security solution exception builder to pull `ExceptionBuilderComponent` from lists plugin # Conflicts: # packages/kbn-optimizer/limits.yml # src/dev/code_coverage/ingest_coverage/__tests__/enumerate_patterns.test.js * removing file that exists in master but not backported to 7.x mistakenly included in backport upon cherry picking Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

yctercero added 11 commits March 22, 2021 14:44

forking ss autocomplete component to list component

fc1f774

fixed up tests, cypress and unit passing locally

11e2a2e

fixing import

db512ab

updating story type issues

d9606b3

moved over the and/or badge

279e511

moved over the and/or badge

f43dfdc

moved exception item

213ef13

fix jest unit test

45597a9

added logic buttons to lists plugin

b12ad8c

moved over main builder logic into lists

34ee433

cleaned up builder folder in security solution and tests

05adb77

yctercero requested review from a team as code owners March 24, 2021 05:58

yctercero marked this pull request as draft March 24, 2021 06:00

yctercero self-assigned this Mar 24, 2021

yctercero added release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detections and Resp Security Detection Response Team v7.13.0 v8.0.0 labels Mar 24, 2021

yctercero added 5 commits March 24, 2021 11:48

updating tests, lint issues, etc

ba5e490

fix ups and increase bundle size

a578547

Merge branch 'master' of github.com:elastic/kibana into rac_builder

57d86d2

fix types

f55f1b8

fix up mocks in tests

e91d08b

yctercero marked this pull request as ready for review March 26, 2021 05:44

yctercero requested review from a team as code owners March 26, 2021 05:44

Merge branch 'master' of github.com:elastic/kibana into rac_builder

cb9a0ca

yctercero enabled auto-merge (squash) April 2, 2021 03:10

yctercero added the auto-backport Deprecated - use backport:version if exact versions are needed label Apr 2, 2021

Merge branch 'master' into rac_builder

e4e1785

MadameSheema approved these changes Apr 6, 2021

View reviewed changes

dmlemeshko requested changes Apr 6, 2021

View reviewed changes

yctercero added 3 commits April 6, 2021 07:40

Merge branch 'master' of github.com:elastic/kibana into rac_builder

255419d

restoring needed qa test

4a621cb

restoring needed qa test

dfb5ce6

dmlemeshko approved these changes Apr 6, 2021

View reviewed changes

yctercero merged commit 92b9482 into elastic:master Apr 6, 2021

yctercero mentioned this pull request Apr 6, 2021

[7.x] [Security Solution][Exceptions] - Moves remaining exceptions builder logic into lists plugin (#95266) #96358

Merged

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Apr 8, 2021

kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Apr 11, 2021

dplumlee mentioned this pull request Apr 29, 2021

[Security Solution][Detections] Endpoint Exceptions no longer restrict 'file.path.*' fields in field selector #95848

Closed

peluja1012 mentioned this pull request May 4, 2021

[Security Solution][Detections][Meta] Modularize the Detection Engine #93550

Closed

13 tasks

yctercero deleted the rac_builder branch October 13, 2021 06:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution][Exceptions] - Moves remaining exceptions builder logic into lists plugin #95266

[Security Solution][Exceptions] - Moves remaining exceptions builder logic into lists plugin #95266

yctercero commented Mar 24, 2021 •

edited

Loading

elasticmachine commented Mar 26, 2021

elasticmachine commented Mar 26, 2021

ghost commented Mar 31, 2021

Module Count

yctercero commented Mar 31, 2021

ghost commented Apr 1, 2021

yctercero commented Apr 5, 2021

yctercero commented Apr 5, 2021

dmlemeshko commented Apr 6, 2021

MadameSheema commented Apr 6, 2021

dmlemeshko left a comment

dmlemeshko Apr 6, 2021

kibanamachine commented Apr 6, 2021

Stack Trace

dmlemeshko left a comment

kibanamachine commented Apr 6, 2021

kibanamachine commented Apr 8, 2021

kibanamachine commented Apr 9, 2021

[Security Solution][Exceptions] - Moves remaining exceptions builder logic into lists plugin #95266

[Security Solution][Exceptions] - Moves remaining exceptions builder logic into lists plugin #95266

Conversation

yctercero commented Mar 24, 2021 • edited Loading

Summary

Entry Item (PR)

Exception Item (PR)

Builder (THIS PR DEALS WITH 👇🏽)

Builder Buttons (THIS PR DEALS WITH 👇🏽)

Exception Item Comments

Builder Modal

Notes

Testing

Checklist

elasticmachine commented Mar 26, 2021

elasticmachine commented Mar 26, 2021

ghost commented Mar 31, 2021

Queries:

Module Count

Observations:

yctercero commented Mar 31, 2021

ghost commented Apr 1, 2021

yctercero commented Apr 5, 2021

yctercero commented Apr 5, 2021

dmlemeshko commented Apr 6, 2021

MadameSheema commented Apr 6, 2021

dmlemeshko left a comment

Choose a reason for hiding this comment

dmlemeshko Apr 6, 2021

Choose a reason for hiding this comment

kibanamachine commented Apr 6, 2021

💛 Build succeeded, but was flaky

Test Failures

Stack Trace

Metrics [docs]

Module Count

Async chunks

Page load bundle

History

dmlemeshko left a comment

Choose a reason for hiding this comment

kibanamachine commented Apr 6, 2021

💔 Backport failed

kibanamachine commented Apr 8, 2021

kibanamachine commented Apr 9, 2021

yctercero commented Mar 24, 2021 •

edited

Loading