[Security Solution] Add Alert Suppression editable component #198673
Merged
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
maximpn
added
release_note:skip
maximpn
force-pushed
the
add-alert-supression-editable-component
branch
from
1a42a9c
to
ad5db96
Compare
nikitaindik
reviewed
Nov 1, 2024
| fieldName: UpgradeableEsqlFields; | ||
| } | ||
|
|
||
| export function EsQlRuleFieldEdit({ fieldName }: EqQlRuleFieldEditProps) { |
There was a problem hiding this comment.
Shall we make it Esql with a lowercase "q" for consistency? I see that in other parts of the codebase it's written as Esql.
nikitaindik
reviewed
Nov 1, 2024
| export function AlertSuppressionEditAdapter({ | ||
| finalDiffableRule, | ||
| }: RuleFieldEditComponentProps): JSX.Element { | ||
| const { indexPattern: dataView } = useRuleIndexPattern({ |
There was a problem hiding this comment.
There's a getRuleIndexPatternParameters function in kql_query_edit.tsx. We might reuse it to simplify this conditional logic.
const defaultIndexPattern = useDefaultIndexPattern();
const indexPatternParameters = getRuleIndexPatternParameters(
finalDiffableRule,
defaultIndexPattern
);
const { indexPattern: dataView } = useRuleIndexPattern(indexPatternParameters);There was a problem hiding this comment.
I finally decided to refactor the implementation to get rid of useRuleIndexPattern().
nikitaindik
reviewed
Nov 1, 2024
| ? GroupByOptions.PerTimePeriod | ||
| : GroupByOptions.PerRuleExecution, | ||
| [SUPPRESSION_DURATION]: alertSuppression?.duration ?? { | ||
| value: 5, |
There was a problem hiding this comment.
Do you think it would add value if we extract this default object into a constant and then reuse it here, on the rule editing page?
There was a problem hiding this comment.
Yes, finally extracted it into a constant.
nikitaindik
reviewed
Nov 1, 2024
| disabledText, | ||
| }: SuppressionFieldsSelectorProps): JSX.Element { | ||
| return ( | ||
| <> |
There was a problem hiding this comment.
Can fragment wrappers (<>) here and around UseField be removed? Do they serve any purpose?
There was a problem hiding this comment.
It was a refactoring artefact.
maximpn
force-pushed
the
add-alert-supression-editable-component
branch
from
8806cfc
to
5eb6aa4
Compare
|
Pinging @elastic/security-detections-response (Team:Detections and Resp) |
|
Pinging @elastic/security-solution (Team: SecuritySolution) |
|
Pinging @elastic/security-detection-rule-management (Team:Detection Rule Management) |
maximpn
force-pushed
the
add-alert-supression-editable-component
branch
from
b48f32b
to
b9ae744
Compare
maximpn
force-pushed
the
add-alert-supression-editable-component
branch
from
9f4d426
to
68f8fe6
Compare
|
@vitaliidm The behavior you discovered in #198673 (comment) is technically correct. User doesn't select an existing value so empty alert suppression fields array is submitted leading to removing alert suppression in the upgrade. Additionally this behavor matches with existing on rule creation/editing pages. Though I agree it doesn't look like the best UX. And we should consider improving it before the Prebuilt Rules Customization feature release. It doesn't look like a critical issue and shouldn't block this PR. WDYT? UPD: I created a ticket to collect improvement tasks. |
I have discussed this with the team and consensus was we should not introduce any potential issue in this PR and should handle it separately. cc: @yctercero |
maximpn
force-pushed
the
add-alert-supression-editable-component
branch
from
30ae31d
to
162b3a9
Compare
@vitaliidm I rolled back terms aggregation fields for threshold rules in 162b3a9. |
vitaliidm
approved these changes
Nov 12, 2024
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
Unknown metric groupsESLint disabled in files
ESLint disabled line counts
Total ESLint disabled count
History
cc @maximpn |
PhilippeOberti
approved these changes
Nov 12, 2024
|
Starting backport for target branches: 8.x https://github.com/elastic/kibana/actions/runs/11799662644 |
💔 All backports failed
Manual backportTo create the backport manually run: Questions ?Please refer to the Backport tool documentation |
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
maximpn
added a commit
that referenced
this pull request
Nov 12, 2024
…198673) (#199809) # Backport This will backport the following commits from `main` to `8.x`: - [[Security Solution] Add Alert Suppression editable component (#198673)](#198673) <!--- Backport version: 8.9.8 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Maxim Palenov","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-12T14:46:39Z","message":"[Security Solution] Add Alert Suppression editable component (#198673)\n\n**Partially addresses:** https://github.com/elastic/kibana/issues/171520\r\n\r\n## Summary\r\n\r\nThis PR adds is built on top of #193828 and #196948 and adds an Alert Suppression editable component for Three Way Diff tab's final edit side of the upgrade prebuilt rule workflow.\r\n\r\n## Details\r\n\r\nhttps://github.com//issues/171520 required adding editable components for each field diffable rule field. Alert Suppression edit component was extracted from Define Rule Step Component into a separate reusable component. To simplify the logic it was split into common Alert Suppression and Threshold Alert Suppression since the latter is a specific use case.\r\n\r\n## Caveats\r\n\r\nUpgrade prebuilt rules workflow is quite different from rule creation and editing. In create and edit rule forms users are capable to change any field at their will. Upgrade prebuilt rules workflow allow to modify only specific fields having diff in the current rule upgrade.\r\n\r\nThere are fields which depend on each other. In particular Alert Suppression isn't supported for EQL sequence though it's addressed in #189725. \r\n\r\n- Alert Suppression editable component in Three Way Diff workflow isn't disabled EQL sequence rule queries. Alert suppression support for rules with EQL sequence queries is implemented in #189725. \r\n\r\n- Machine learning rule type require running selected machine learning jobs otherwise input could be disabled in case of there are no fields to pick from otherwise a warning message below the combobox is shown.\r\n\r\n## How to test\r\n\r\nThe simplest way to test is via patching installed prebuilt rules via Rule Patch API. Please follow steps below\r\n\r\n- Enable Prebuilt rule customization feature by adding a `prebuiltRulesCustomizationEnabled` feature flag\r\n- Run Kibana locally\r\n- Install a prebuilt rule, e.g. `Potential Code Execution via Postgresql` with rule_id `2a692072-d78d-42f3-a48a-775677d79c4e`\r\n- Patch the installed rule by running a query below\r\n\r\n```bash\r\ncurl -X PATCH --user elastic:changeme -H 'Content-Type: application/json' -H 'kbn-xsrf: 123' -H \"elastic-api-version: 2023-10-31\" -d '{\"rule_id\":\"2a692072-d78d-42f3-a48a-775677d79c4e\",\"version\":1,\"alert_suppression\":{\"group_by\":[\"host.name\"]}}' http://localhost:5601/kbn/api/detection_engine/rules\r\n```\r\n\r\n- Open `Detection Rules (SIEM)` Page -> `Rule Updates` -> click on `Potential Code Execution via Postgresql` rule -> expand `EQL Query` to see EQL Query -> press `Edit` button\r\n\r\n## Screenshots\r\n\r\nCustom query prebuilt rule (UI looks similar for EQL, Indicator Match, New Terms and ES|QL rule types)\r\n\r\n\r\n\r\nMachine learning prebuilt rule with a diff in alert suppression\r\n\r\n\r\n\r\nThreshold prebuilt rule\r\n\r\n","sha":"06986e4a86a0fa3c3951fcb6b2ba34ebe2769820","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Team:Detections and Resp","Team: SecuritySolution","Team:Detection Rule Management","Feature:Prebuilt Detection Rules","backport:prev-minor","v8.17.0"],"number":198673,"url":"https://github.com/elastic/kibana/pull/198673","mergeCommit":{"message":"[Security Solution] Add Alert Suppression editable component (#198673)\n\n**Partially addresses:** https://github.com/elastic/kibana/issues/171520\r\n\r\n## Summary\r\n\r\nThis PR adds is built on top of #193828 and #196948 and adds an Alert Suppression editable component for Three Way Diff tab's final edit side of the upgrade prebuilt rule workflow.\r\n\r\n## Details\r\n\r\nhttps://github.com//issues/171520 required adding editable components for each field diffable rule field. Alert Suppression edit component was extracted from Define Rule Step Component into a separate reusable component. To simplify the logic it was split into common Alert Suppression and Threshold Alert Suppression since the latter is a specific use case.\r\n\r\n## Caveats\r\n\r\nUpgrade prebuilt rules workflow is quite different from rule creation and editing. In create and edit rule forms users are capable to change any field at their will. Upgrade prebuilt rules workflow allow to modify only specific fields having diff in the current rule upgrade.\r\n\r\nThere are fields which depend on each other. In particular Alert Suppression isn't supported for EQL sequence though it's addressed in #189725. \r\n\r\n- Alert Suppression editable component in Three Way Diff workflow isn't disabled EQL sequence rule queries. Alert suppression support for rules with EQL sequence queries is implemented in #189725. \r\n\r\n- Machine learning rule type require running selected machine learning jobs otherwise input could be disabled in case of there are no fields to pick from otherwise a warning message below the combobox is shown.\r\n\r\n## How to test\r\n\r\nThe simplest way to test is via patching installed prebuilt rules via Rule Patch API. Please follow steps below\r\n\r\n- Enable Prebuilt rule customization feature by adding a `prebuiltRulesCustomizationEnabled` feature flag\r\n- Run Kibana locally\r\n- Install a prebuilt rule, e.g. `Potential Code Execution via Postgresql` with rule_id `2a692072-d78d-42f3-a48a-775677d79c4e`\r\n- Patch the installed rule by running a query below\r\n\r\n```bash\r\ncurl -X PATCH --user elastic:changeme -H 'Content-Type: application/json' -H 'kbn-xsrf: 123' -H \"elastic-api-version: 2023-10-31\" -d '{\"rule_id\":\"2a692072-d78d-42f3-a48a-775677d79c4e\",\"version\":1,\"alert_suppression\":{\"group_by\":[\"host.name\"]}}' http://localhost:5601/kbn/api/detection_engine/rules\r\n```\r\n\r\n- Open `Detection Rules (SIEM)` Page -> `Rule Updates` -> click on `Potential Code Execution via Postgresql` rule -> expand `EQL Query` to see EQL Query -> press `Edit` button\r\n\r\n## Screenshots\r\n\r\nCustom query prebuilt rule (UI looks similar for EQL, Indicator Match, New Terms and ES|QL rule types)\r\n\r\n\r\n\r\nMachine learning prebuilt rule with a diff in alert suppression\r\n\r\n\r\n\r\nThreshold prebuilt rule\r\n\r\n","sha":"06986e4a86a0fa3c3951fcb6b2ba34ebe2769820"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/198673","number":198673,"mergeCommit":{"message":"[Security Solution] Add Alert Suppression editable component (#198673)\n\n**Partially addresses:** https://github.com/elastic/kibana/issues/171520\r\n\r\n## Summary\r\n\r\nThis PR adds is built on top of #193828 and #196948 and adds an Alert Suppression editable component for Three Way Diff tab's final edit side of the upgrade prebuilt rule workflow.\r\n\r\n## Details\r\n\r\nhttps://github.com//issues/171520 required adding editable components for each field diffable rule field. Alert Suppression edit component was extracted from Define Rule Step Component into a separate reusable component. To simplify the logic it was split into common Alert Suppression and Threshold Alert Suppression since the latter is a specific use case.\r\n\r\n## Caveats\r\n\r\nUpgrade prebuilt rules workflow is quite different from rule creation and editing. In create and edit rule forms users are capable to change any field at their will. Upgrade prebuilt rules workflow allow to modify only specific fields having diff in the current rule upgrade.\r\n\r\nThere are fields which depend on each other. In particular Alert Suppression isn't supported for EQL sequence though it's addressed in #189725. \r\n\r\n- Alert Suppression editable component in Three Way Diff workflow isn't disabled EQL sequence rule queries. Alert suppression support for rules with EQL sequence queries is implemented in #189725. \r\n\r\n- Machine learning rule type require running selected machine learning jobs otherwise input could be disabled in case of there are no fields to pick from otherwise a warning message below the combobox is shown.\r\n\r\n## How to test\r\n\r\nThe simplest way to test is via patching installed prebuilt rules via Rule Patch API. Please follow steps below\r\n\r\n- Enable Prebuilt rule customization feature by adding a `prebuiltRulesCustomizationEnabled` feature flag\r\n- Run Kibana locally\r\n- Install a prebuilt rule, e.g. `Potential Code Execution via Postgresql` with rule_id `2a692072-d78d-42f3-a48a-775677d79c4e`\r\n- Patch the installed rule by running a query below\r\n\r\n```bash\r\ncurl -X PATCH --user elastic:changeme -H 'Content-Type: application/json' -H 'kbn-xsrf: 123' -H \"elastic-api-version: 2023-10-31\" -d '{\"rule_id\":\"2a692072-d78d-42f3-a48a-775677d79c4e\",\"version\":1,\"alert_suppression\":{\"group_by\":[\"host.name\"]}}' http://localhost:5601/kbn/api/detection_engine/rules\r\n```\r\n\r\n- Open `Detection Rules (SIEM)` Page -> `Rule Updates` -> click on `Potential Code Execution via Postgresql` rule -> expand `EQL Query` to see EQL Query -> press `Edit` button\r\n\r\n## Screenshots\r\n\r\nCustom query prebuilt rule (UI looks similar for EQL, Indicator Match, New Terms and ES|QL rule types)\r\n\r\n\r\n\r\nMachine learning prebuilt rule with a diff in alert suppression\r\n\r\n\r\n\r\nThreshold prebuilt rule\r\n\r\n","sha":"06986e4a86a0fa3c3951fcb6b2ba34ebe2769820"}},{"branch":"8.x","label":"v8.17.0","labelRegex":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT-->
tkajtoch
pushed a commit
to tkajtoch/kibana
that referenced
this pull request
Nov 12, 2024
…#198673) **Partially addresses:** elastic#171520 ## Summary This PR adds is built on top of elastic#193828 and elastic#196948 and adds an Alert Suppression editable component for Three Way Diff tab's final edit side of the upgrade prebuilt rule workflow. ## Details elastic#171520 required adding editable components for each field diffable rule field. Alert Suppression edit component was extracted from Define Rule Step Component into a separate reusable component. To simplify the logic it was split into common Alert Suppression and Threshold Alert Suppression since the latter is a specific use case. ## Caveats Upgrade prebuilt rules workflow is quite different from rule creation and editing. In create and edit rule forms users are capable to change any field at their will. Upgrade prebuilt rules workflow allow to modify only specific fields having diff in the current rule upgrade. There are fields which depend on each other. In particular Alert Suppression isn't supported for EQL sequence though it's addressed in elastic#189725. - Alert Suppression editable component in Three Way Diff workflow isn't disabled EQL sequence rule queries. Alert suppression support for rules with EQL sequence queries is implemented in elastic#189725. - Machine learning rule type require running selected machine learning jobs otherwise input could be disabled in case of there are no fields to pick from otherwise a warning message below the combobox is shown. ## How to test The simplest way to test is via patching installed prebuilt rules via Rule Patch API. Please follow steps below - Enable Prebuilt rule customization feature by adding a `prebuiltRulesCustomizationEnabled` feature flag - Run Kibana locally - Install a prebuilt rule, e.g. `Potential Code Execution via Postgresql` with rule_id `2a692072-d78d-42f3-a48a-775677d79c4e` - Patch the installed rule by running a query below ```bash curl -X PATCH --user elastic:changeme -H 'Content-Type: application/json' -H 'kbn-xsrf: 123' -H "elastic-api-version: 2023-10-31" -d '{"rule_id":"2a692072-d78d-42f3-a48a-775677d79c4e","version":1,"alert_suppression":{"group_by":["host.name"]}}' http://localhost:5601/kbn/api/detection_engine/rules ``` - Open `Detection Rules (SIEM)` Page -> `Rule Updates` -> click on `Potential Code Execution via Postgresql` rule -> expand `EQL Query` to see EQL Query -> press `Edit` button ## Screenshots Custom query prebuilt rule (UI looks similar for EQL, Indicator Match, New Terms and ES|QL rule types)  Machine learning prebuilt rule with a diff in alert suppression  Threshold prebuilt rule 
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.
Partially addresses: #171520
Summary
This PR adds is built on top of #193828 and #196948 and adds an Alert Suppression editable component for Three Way Diff tab's final edit side of the upgrade prebuilt rule workflow.
Details
#171520 required adding editable components for each field diffable rule field. Alert Suppression edit component was extracted from Define Rule Step Component into a separate reusable component. To simplify the logic it was split into common Alert Suppression and Threshold Alert Suppression since the latter is a specific use case.
Caveats
Upgrade prebuilt rules workflow is quite different from rule creation and editing. In create and edit rule forms users are capable to change any field at their will. Upgrade prebuilt rules workflow allow to modify only specific fields having diff in the current rule upgrade.
There are fields which depend on each other. In particular Alert Suppression isn't supported for EQL sequence though it's addressed in #189725.
Alert Suppression editable component in Three Way Diff workflow isn't disabled EQL sequence rule queries. Alert suppression support for rules with EQL sequence queries is implemented in [Security Solution][Detection Engine] Adds support for suppressing EQL sequence alerts #189725.
Machine learning rule type require running selected machine learning jobs otherwise input could be disabled in case of there are no fields to pick from otherwise a warning message below the combobox is shown.
How to test
The simplest way to test is via patching installed prebuilt rules via Rule Patch API. Please follow steps below
prebuiltRulesCustomizationEnabledfeature flagPotential Code Execution via Postgresqlwith rule_id2a692072-d78d-42f3-a48a-775677d79c4eDetection Rules (SIEM)Page ->Rule Updates-> click onPotential Code Execution via Postgresqlrule -> expandEQL Queryto see EQL Query -> pressEditbuttonScreenshots
Custom query prebuilt rule (UI looks similar for EQL, Indicator Match, New Terms and ES|QL rule types)
Machine learning prebuilt rule with a diff in alert suppression
Threshold prebuilt rule