[Detection Engine][Rule Suppression] Disable EQL sequence suppression #178531
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…afaaNasr/kibana into eql-suppression-BE-FTR-changes
…afaaNasr/kibana into eql-suppression-BE-FTR-changes
…afaaNasr/kibana into eql-suppression-BE-FTR-changes
…sequence Conflicts: x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/step_define_rule/index.tsx x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/eql/eql.ts x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_creation/event_correlation_rule_suppression_ess_basic.cy.ts
It's official, we've now got way too much happening in this component!
At its core, this is driven by `useFormData` (as I think a lot of complexity in this form could be): * We use the current, updated value of `queryBar`, along with the `groupByFields`, to determine whether the suppression fields should be disabled. The core logic here is the same as before. * We move the _validity_ of the groupByFields field into the form validations. Again, the logic here is the same, but it's now done a) only when the relevant fields change, and b) without the need for manually calling `validateFields` or `setFieldErrors`. Other notes: * Adds an `isEqlSequenceQuery` helper for explicitness. * Removes callback hook from query bar, since we now get this info directly from the form.
These were discussed in elastic/security-docs#4977
|
/ci |
|
/ci |
These were duplicated since their inception, but CI was never run so it wasn't caught until now.
|
/ci |
|
/ci |
Removes tests around deleted prop, fixes imports
rylnd
force-pushed
the
eql-suppression-disable-sequence
branch
from
21fec57 to
0d1b024
Compare
|
/ci |
|
/ci |
This one snuck through the myriad merges/conflict resolutions on this branch.
See TODO for details.
|
/ci |
input
This tooltip was previously wrapping several form fields (all of those
related to alert suppression, which:
1. Caused strange UX when attempting to hover disabled elements
* Because many EUI disabled elements have `pointer-events: none`, you
could only trigger the tooltip in certain areas of the form
* I suspect there might've been some interaction with nested tooltips,
where if you hovered the inner, the outer would also open and they
would cancel each other out.
2. Caused existing cypress tests around suppression tooltips to fail,
mainly due to the above.
For these reasons, I've limited the scope of this tooltip to only show
on hover of the alert suppression (group by) input, which is both more
consistent and much more robust, UX-wise.
The only downside here is that if you're on a basic license AND you type
a sequence query, you'll get one of the two tooltips depending on where
you hover, but that was already the case before this.
…abled" This reverts commit f974273. See previous commit about our fixing of the tooltip.
|
/ci |
💔 Build FailedFailed CI StepsMetrics [docs]
History
To update your PR or re-run it, just comment with: cc @rylnd @WafaaNasr |
rylnd
merged commit 8e3b753
into
elastic:security/eql-suppression
34 of 35 checks passed
|
This is green as of the latest commit (ignoring the merge-base issue with CI); this is now merged into the main branch: #176422 |
rylnd
added a commit
that referenced
this pull request
Apr 17, 2024
…nce based queries (#176422) # Summary - Address adding suppression to EQL rules https://github.com/elastic/security-team/issues/7773 - Milestone details https://github.com/elastic/security-team/issues/8432 ## Checklist - [x] Functional changes are hidden behind a feature flag. If not hidden, the PR explains why these changes are being implemented in a long-living feature branch. - [x] Functional changes are covered with a test plan and automated tests. [Test plan](https://github.com/elastic/security-team/pull/9155) - [ ] Stability of new and changed tests is verified using the [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner) in both ESS and Serverless. By default, use 200 runs for ESS and 200 runs for Serverless. * Cypress ESS: https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5686 * Cypress Serverless: https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5687 * FTR ESS: https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5688 * FTR Serverless: https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5689 - [x] Comprehensive manual testing is done by two engineers: the PR author and one of the PR reviewers. Changes are tested in both ESS and Serverless. - [x] Mapping changes are accompanied by a technical design document. It can be a GitHub issue or an RFC explaining the changes. The design document is shared with and approved by the appropriate teams and individual stakeholders. - [ ] (OPTIONAL) OpenAPI specs changes include detailed descriptions and examples of usage and are ready to be released on https://docs.elastic.co/api-reference. NOTE: This is optional because at the moment we don't have yet any OpenAPI specs that would be fully "documented" and "GA-ready" for publishing on https://docs.elastic.co/api-reference. - [x] Functional changes are communicated to the Docs team. A ticket is opened in https://github.com/elastic/security-docs using the [`Internal documentation request (Elastic employees)`](https://github.com/elastic/security-docs/issues/new?assignees=&labels=&projects=&template=docs-request-internal.yaml&title=%5BRequest%5D+) template. The following information is included: feature flags used, target ESS version, planned timing for ESS and Serverless releases. - [x] Check if in timeline we can show the suppression count column when the user clicks on investigate on timeline for Eql suppressed Alerts (#180976) ## Related Issues * Sub-PRs - Address EQL schema changes PR #176391 - Adding Feature flag PR and updating the Frontend Part in Rule Create/Edit #176398 - Adding Backend changes and FTR tests #176597 - Fix Investigate in Timeline for the Suppressed Alerts #177839 - Add Cypress e2e tests #177870 - Disable EQL sequence suppression in the UI and fix Cypress `after` esArchive path #178531 - Docs Issue elastic/security-docs#4977 - Test plan https://github.com/elastic/security-team/pull/9155 ## Screenshots/recordings ### Non-Sequence Suppression 1. Rule creation, Suppression based on a single value https://github.com/elastic/kibana/assets/12671903/8d168bce-15d3-45c2-a5dc-238b3ac01626 2. Rule creation, Suppression based on an array of values https://github.com/elastic/kibana/assets/12671903/0e3312a9-4eae-476b-9c1e-c68189bbaf95 3. Investigate In Timeline https://github.com/elastic/kibana/assets/12671903/e10c8668-4d5b-4748-b8a1-678603b4a8a5 ### Disabled Sequence Suppression 1. UI https://github.com/elastic/kibana/assets/12671903/01faa649-ca8b-43e4-a398-42ab242e7a72 --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Ryland Herrick <ryalnd@gmail.com> Co-authored-by: Georgii Gorbachev <georgii.gorbachev@elastic.co>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
DisableSequenceEqlsuppression.mov
TODO: