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][Exceptions] - New add/edit exception item flyouts #140643

Closed
wants to merge 95 commits into from

Conversation

yctercero
Copy link
Contributor

@yctercero yctercero commented Sep 13, 2022

Summary

Addresses #131668, #131671, #143014, #143015.

Updates the exception item add/edit flyout to match new designs and flows. While this can only be tested in our current UI from the rule details page, alerts, alert details flyout, it's been built out for the other cases where it will be used - rule bulk actions and exception list management.

To Do

  • Some of the hooks need unit tests
  • Update to use the components from the exception package, to start deleting them from the rule_exceptions/components
  • Add back in 2 error states - when version conflict (not super common) and list does not exist and user needs to remove association from rule, again, shouldn't be super common error

Breaking down the changes

  • packages/kbn-securitysolution-autocomplete/
    • Updates the field combo box to allow user to input a custom option, no longer restricted to selecting a prefilled value
    • There is a boolean added, that is set to default false, so this custom option functionality is optional
  • x-pack/plugins/lists/public/exceptions/components/builder/
    • Update UI to show text under field combo box if it accepts a custom option to let user know that this is the case
    • Updates the builder to take an exception item name from user input
  • x-pack/plugins/security_solution/cypress/e2e/exceptions/
    • Updates and adds cypress tests related to adding/editing exceptions from the different existing flows
  • x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/add_exception_flyout/
    • Update the add exception flyout to use the new components added in PR
    • Tried to move out submission logic into hook to clean things up a bit
  • x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/edit_exception_flyout/
    • Update the add exception flyout to use the new components added in PR
    • Tried to move out submission logic into hook to clean things up a bit
  • x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/all_exception_items_table/
    • Updates the all items view to take in an array of listTypes since now the table isn't just endpoint exceptions or detection exceptions, it can include rule_default list type
  • x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/components/exception_item_card
    • Now includes affected rules and affected lists link in the card, so it's easier to tell which items are rule only vs shared list
  • x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/logic/
    • Updated the hooks to try and simplify them and separate concerns a bit
    • TODO: these need updated tests
  • x-pack/plugins/security_solution/public/detection_engine/rule_exceptions/logic/use_exception_flyout_data.tsx
    • New hook to try and manage the logic around the add/edit exception flyout indices
      • If a single rule is passed into the flyout, it uses the data_view_id if one is defined or the index to fetch the fields
      • If multiple rules are passed in, it defaults to using the default security data view (right now there's no UI supporting this use case, but it would be part of the bulk rule action flow)
      • If no rules are passed in, it defaults to using the default security data view (right now there's no UI supporting this use case, but it would be part of the create exception item from the list management flow)
  • x-pack/plugins/security_solution/public/detections/components/alerts_table/timeline_actions/
    • Updates logic around adding exception from alert and alert detail flyout to fit with new add exception modal

Existing questions

  • When add modal is dealing with multiple rules (bulk action or when adding an exception from the list management page where you can choose what rules to add the exception to) - should large value lists be allowed with some sort of warning text that any rules of type x, y, z will ignore any exceptions of type list?
    • In this PR, I am allowing large value lists unless a single rule is passed in where we can be sure of whether large value lists are supported by it

Addresses

#127696, #99597, #117289, #99022, #135254, #102460, #116586

Screenshots

Shows exception item that is part of a shared list
Screen Shot 2022-10-10 at 1 34 31 PM

Add endpoint exception

Screen Shot 2022-10-10 at 1 39 39 PM

Edit endpoint exception

Screen Shot 2022-10-10 at 1 37 04 PM

Add rule exception

Screen Shot 2022-10-10 at 1 39 18 PM

Edit rule exception

Screen Shot 2022-10-11 at 11 26 23 AM

Checklist

For maintainers

yctercero and others added 30 commits August 12, 2022 13:29
@yctercero yctercero force-pushed the use_me_exceptions_flyouts branch from 1e60825 to a249725 Compare October 4, 2022 17:14
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 3165 3191 +26

Async chunks

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

id before after diff
lists 152.7KB 153.7KB +1.1KB
securitySolution 6.6MB 12.4MB ⚠️ +5.8MB
total ⚠️ +5.8MB

Page load bundle

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

id before after diff
securitySolution 265.1KB 264.9KB -172.0B
Unknown metric groups

API count

id before after diff
@kbn/securitysolution-list-utils 192 194 +2

ESLint disabled in files

id before after diff
securitySolution 73 72 -1

ESLint disabled line counts

id before after diff
lists 13 18 +5

Total ESLint disabled count

id before after diff
lists 18 23 +5
securitySolution 484 483 -1
total +4

History

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

@legrego legrego closed this Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants