-
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][Detections] Create Threshold-based Rule type #71371
[Security][Detections] Create Threshold-based Rule type #71371
Conversation
…-rule-type # Conflicts: # x-pack/plugins/security_solution/public/graphql/introspection.json # x-pack/plugins/security_solution/public/graphql/types.ts # x-pack/plugins/security_solution/public/timelines/containers/index.gql_query.ts # x-pack/plugins/security_solution/server/graphql/ecs/schema.gql.ts # x-pack/plugins/security_solution/server/graphql/types.ts # x-pack/plugins/security_solution/server/lib/ecs_fields/index.ts
…-rule-type # Conflicts: # x-pack/plugins/security_solution/public/detections/components/rules/threshold_input/index.tsx
I think we'll want to add the |
if (rule.type === 'threshold') { | ||
if (!rule.threshold) { | ||
return ['when "type" is "threshold", "threshold" is required']; | ||
} else if (rule.threshold.value <= 0) { |
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.
Optionally you can use the io-ts type to verify the value is in bound -- either:
threshold.is(rule.threshold)
(as imported from detection_engine/schemas/common/schemas.ts
)
-or-
PositiveIntegerGreaterThanZero.is(rule.threshold.value)
@@ -4,8 +4,10 @@ | |||
* you may not use this file except in compliance with the Elastic License. | |||
*/ | |||
|
|||
/* eslint-disable complexity */ |
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.
Oooweee! Did we finally make it? Haha 😅
I removed this locally and wasn't running into the error, so perhaps we're still good? Either way, I think the Alert Actions code has grown enough now that we can probably restructure and break it apart a bit further. This area of the codebase is only going to grow more and more now that we can programmatically declare actions on a per alert basis... 🙂
I've added this to my list of tech debt items for 7.10+, so nothing to worry about now.
threshold?: Maybe<ToAny>; | ||
|
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.
Where is the source declaration that results in these generated types for timeline? Looks like I need to add the new rule fields from #70288. This ensures these fields show up in the field browser for timeline, correct?
@@ -201,6 +197,18 @@ export const EventColumnView = React.memo<Props>( | |||
: grouped.icon; | |||
}, [button, closePopover, id, onClickCb, data, ecsData, timelineActions, isPopoverOpen]); | |||
|
|||
const handlePinClicked = useCallback( |
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.
@@ -210,6 +210,8 @@ export const timelineQuery = gql` | |||
to | |||
filters | |||
note | |||
type | |||
threshold |
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.
Ahh, here it is (in reference to https://github.com/elastic/kibana/pull/71371/files#r453934881) -- question still though, what features are gained by adding the additional fields to the timeline gql, is it just the FieldBrowser
/Event Details
, or more?
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.
Checked out, tested locally and reviewed code. Couple nits/optionals, and we'll definitely want to add the threshold_fields
to the singals_mapping.json
, but other than that LAFTM (Looks Absolutely Fantastic To Me!).
This is going to be such a great feature for our users -- in testing it was so awesome to be able to analyze a bundle of failed auth events by host within Timeline. Great work here @patrykkopycinski, really appreciate it! 🙂 🚀 🎉
@elasticmachine merge upstream |
💚 Build SucceededBuild metrics@kbn/optimizer bundle module count
History
To update your PR or re-run it, just comment with: |
Pinging @elastic/security-solution (Team: SecuritySolution) |
Summary
Implements #68409
All results
as well if they were aggregatedChecklist