-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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][Alerts] Alert suppression per rule execution #142686
Merged
Merged
Changes from all commits
Commits
Show all changes
89 commits
Select commit
Hold shift + click to select a range
41144fa
Initial commit
madirey 781cf26
Merge branch 'main' of github.com:elastic/kibana into alert-per-entit…
madirey 8e6aa9e
No value list exceptions for now
madirey 3c9a1b5
Merge branch 'main' of github.com:elastic/kibana into alert-per-entit…
madirey de903ec
Gather alerts from groups for indexing
madirey 00a817a
Merge branch 'main' of github.com:elastic/kibana into alert-per-entit…
madirey aa6dec8
Refine algorithm
madirey 8eea5a8
Merge branch 'main' of github.com:elastic/kibana into alert-per-entit…
madirey 2e01a48
Fix algorithm
madirey 903fa61
Merge branch 'main' of github.com:elastic/kibana into alert-per-entit…
madirey a3bf7af
Clean up prototype
madirey 7e10630
Merge branch 'main' of github.com:elastic/kibana into alert-per-entit…
madirey d2f356c
add comments
madirey 20c4745
Merge branch 'main' of github.com:elastic/kibana into alert-per-entit…
madirey 8b764b9
Merge branch 'main' of github.com:elastic/kibana into alert-per-entit…
madirey 20e5c12
Merge branch 'main' of github.com:elastic/kibana into alert-per-entit…
madirey 9206c6c
Refactor
madirey 05fedc3
Refactor
madirey 04b3a27
More refactoring
madirey 95a96be
Merge branch 'main' of github.com:elastic/kibana into alert-per-entit…
madirey 674776c
Some tests
madirey 85c8cfc
Integration test
madirey bf17d52
Fixes
madirey e793e93
Add create/patch params
madirey dadc419
Merge branch 'main' of github.com:elastic/kibana into alert-per-entit…
madirey 71f26c3
Type fixes
madirey efbe457
Integration test
madirey 32bc5bc
Merge branch 'main' of github.com:elastic/kibana into alert-per-entit…
madirey 81fb0e5
Initial commit - UI
madirey 131a9fa
type fixes
madirey ab4bd0e
Merge branch 'main' of github.com:elastic/kibana into alert-per-entit…
madirey efc015b
UI fixes
madirey 141d7e4
Merge branch 'main' of github.com:elastic/kibana into alert-per-entit…
madirey 90d95c9
Merge branch 'main' of github.com:elastic/kibana into alert-per-entit…
madirey 98c54be
cleanup
madirey a309d05
Merge branch 'main' of github.com:elastic/kibana into alert-per-entit…
madirey 2fc1d10
cleanup
madirey 6fb6416
Default
madirey 73f5ff3
Feature flag
madirey cba60bf
Merge branch 'main' of github.com:elastic/kibana into alert-per-entit…
madirey 105f364
comments
madirey 7933ba4
Do more things
madirey 1d852bc
WIP
marshallmain ff176f2
Merge branch 'main' of github.com:elastic/kibana into alert-throttling
marshallmain 6cf5285
WIP
marshallmain a2c1629
First implementation of backend
marshallmain 39a94b6
Investigate throttled alert in timeline, add field to UI
marshallmain f1c3ff6
[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…
kibanamachine 6fa3d29
Update AAD throttling field names
marshallmain fc1caa6
Merge branch 'main' of github.com:elastic/kibana into alert-throttling
marshallmain 241487d
Move alertGrouping schema to correct file
marshallmain 438dfcc
Merge branch 'main' of github.com:elastic/kibana into alert-throttling
marshallmain 02f3e53
Add some tests
marshallmain 9f265e8
Add test snapshot
marshallmain 669920b
Fix saved query rule type
marshallmain 8a1275e
Add alert_grouping in remaining places
marshallmain c18b67b
Remove old grouping integration test
marshallmain 395df7a
Fix another type
marshallmain f7e6817
Misc fixes
marshallmain f315467
Fix state returning from query executor
marshallmain 75551bc
Merge branch 'main' into alert-throttling
marshallmain 7f87e7b
Fix state return, alertTimestampOverride merge conflicts
marshallmain 665142e
Merge branch 'main' into alert-throttling
marshallmain 4350c77
Combine query and saved query files
marshallmain 16bfd4f
Move alert throttle fields to rule registry
marshallmain d714131
Fix imports
marshallmain 82c1bae
Fix more imports
marshallmain 79e77e3
Update test
marshallmain 93566e8
Merge branch 'main' into alert-throttling
marshallmain 7452fd7
Change throttling references to suppression
marshallmain d4e4e6c
Add jsdoc comment
marshallmain dad1f69
Rename throttlign to suppression in more places
marshallmain 0650f11
Update snapshot
marshallmain 079e934
Fix rule suppression bucket state and add grouping fields limit
marshallmain 1b45b4b
Add server side license check for alert suppression
marshallmain 310146b
Merge branch 'main' into alert-throttling
marshallmain b84b679
Renaming suppression.count to suppression.docs_count
marshallmain 9929a13
Subtract 1 from bucket count
marshallmain a0e2619
Merge branch 'main' into alert-throttling
marshallmain 66fda9d
[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…
kibanamachine d757582
Add runtime license check for suppression
marshallmain 2df10e9
Fix test snapshot
marshallmain 65cc647
Rule create/edit/details licensing UX
marshallmain 454d672
Revert "Add server side license check for alert suppression"
marshallmain 3823357
Merge branch 'main' into alert-throttling
marshallmain f8b4250
Fix description step tests
marshallmain 135944e
Limit group by fields to aggregatable fields
marshallmain 534c896
Add layers icon in rule name, fix details
marshallmain 5d88e35
Fix incorrect i18n names
marshallmain File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
29 changes: 29 additions & 0 deletions
29
...olution/common/detection_engine/rule_schema/model/specific_attributes/query_attributes.ts
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
|
||
import * as t from 'io-ts'; | ||
import { LimitedSizeArray } from '@kbn/securitysolution-io-ts-types'; | ||
|
||
export const AlertSuppressionGroupBy = LimitedSizeArray({ | ||
codec: t.string, | ||
minSize: 1, | ||
maxSize: 3, | ||
}); | ||
|
||
/** | ||
* Schema for fields relating to alert suppression, which enables limiting the number of alerts per entity. | ||
* e.g. group_by: ['host.name'] would create only one alert per value of host.name. The created alert | ||
* contains metadata about how many other candidate alerts with the same host.name value were suppressed. | ||
*/ | ||
export type AlertSuppression = t.TypeOf<typeof AlertSuppression>; | ||
export const AlertSuppression = t.exact( | ||
t.type({ | ||
group_by: AlertSuppressionGroupBy, | ||
}) | ||
); | ||
|
||
export const minimumLicenseForSuppression = 'platinum'; | ||
35 changes: 35 additions & 0 deletions
35
x-pack/plugins/security_solution/common/detection_engine/schemas/alerts/8.6.0/index.ts
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
|
||
import type { AlertWithCommonFields800 } from '@kbn/rule-registry-plugin/common/schemas/8.0.0'; | ||
import type { | ||
ALERT_SUPPRESSION_TERMS, | ||
ALERT_SUPPRESSION_START, | ||
ALERT_SUPPRESSION_END, | ||
ALERT_SUPPRESSION_DOCS_COUNT, | ||
} from '@kbn/rule-data-utils'; | ||
|
||
import type { BaseFields840, DetectionAlert840 } from '../8.4.0'; | ||
|
||
/* DO NOT MODIFY THIS SCHEMA TO ADD NEW FIELDS. These types represent the alerts that shipped in 8.6.0. | ||
Any changes to these types should be bug fixes so the types more accurately represent the alerts from 8.6.0. | ||
If you are adding new fields for a new release of Kibana, create a new sibling folder to this one | ||
for the version to be released and add the field(s) to the schema in that folder. | ||
Then, update `../index.ts` to import from the new folder that has the latest schemas, add the | ||
new schemas to the union of all alert schemas, and re-export the new schemas as the `*Latest` schemas. | ||
*/ | ||
|
||
export interface SuppressionFields860 extends BaseFields840 { | ||
[ALERT_SUPPRESSION_TERMS]: Array<{ field: string; value: string | number | null }>; | ||
[ALERT_SUPPRESSION_START]: Date; | ||
[ALERT_SUPPRESSION_END]: Date; | ||
[ALERT_SUPPRESSION_DOCS_COUNT]: number; | ||
} | ||
|
||
export type SuppressionAlert860 = AlertWithCommonFields800<SuppressionFields860>; | ||
|
||
export type DetectionAlert860 = DetectionAlert840 | SuppressionAlert860; |
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
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can see check for license is only present in UI.
What about having it on API level of Security and Platform? Is there a consideration to have it in 8.6?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I switched from validating the params on create/edit of a rule to checking the license at runtime in the executor to avoid edge cases around license expiration/degradation. E.g. if the license expires, rules would be exportable but then would fail to import,
patch
would allow editing a rule while leaving suppression params untouched butupdate
would not, etc.The behavior now is that if a license is insufficient and suppression is enabled then rules will continue to work, but the suppression configuration will be ignored. If the platinum license is restored, then suppression will start to work again on those rules automatically. This way we can keep the functionality behind the license appropriately but not cause issues with rule management if the license level does drop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for explaining, that makes sense 👍
Btw, can we use there
minimumLicenseForSuppression
instead of hardcodedplatinum
value?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, yeah that should be
minimumLicenseForSuppression
as well. I'll add it to the follow up list.