[Security Solution] Restructuring folders of Detection Engine + refactoring Rule Management #142950
There are no files selected for viewing
Validating CODEOWNERS rules …
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -105,7 +105,7 @@ pageLoadAssetSize: | |
| screenshotting: 22870 | ||
| searchprofiler: 67080 | ||
| security: 65433 | ||
| securitySolution: 339077 | ||
|
There was a problem hiding this comment. Will be addressed shortly in a follow-up PR. #143532 There was a problem hiding this comment. Can resolving #95870 please be prioritized? It's affecting the end-user performance of Kibana as a whole and just appears to be getting worse, not better. There was a problem hiding this comment. That will come with this PR There was a problem hiding this comment. I think after updating this PR with the main, which contains the recent bundle changes, we can set securitySolution: 66738 back. There was a problem hiding this comment. @YulNaumenko @tylersmalley I managed to keep 66738 in this PR. Will see if I can reduce it a bit more by doing #143532 |
||
| sessionView: 77750 | ||
| share: 71239 | ||
| snapshotRestore: 79032 | ||
|
|
||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,42 +6,47 @@ | |
| * Side Public License, v 1. | ||
| */ | ||
|
|
||
| import * as t from 'io-ts'; | ||
| import { saved_object_attributes } from '../saved_object_attributes'; | ||
|
|
||
| export type RuleActionGroup = t.TypeOf<typeof RuleActionGroup>; | ||
| export const RuleActionGroup = t.string; | ||
|
There was a problem hiding this comment. What's the reasoning behind using PascalCase for the io-ts runtime values now? In the past we've used camelCase for objects and PascalCase for types, which generally makes it simple to distinguish between types and concrete instances of that type. There was a problem hiding this comment. @marshallmain This is a TypeScript feature called aliases. In this case, it's handy because it allows to write only one JSDoc comment which will be applied to both the TS type and the io-ts schema. So when you hover over them in the IDE you will see the same comment. Without that, we'd need to either duplicate the comment or leave either the TS type or the io-ts schema uncommented. Personally, I also like the fact that it's 1 import instead of 2 when you need both (e.g. in routes). There was a problem hiding this comment. One of the examples of how JSDoc comments for rule attributes could look like: |
||
|
|
||
| export type RuleActionId = t.TypeOf<typeof RuleActionId>; | ||
| export const RuleActionId = t.string; | ||
|
|
||
| export type RuleActionTypeId = t.TypeOf<typeof RuleActionTypeId>; | ||
| export const RuleActionTypeId = t.string; | ||
|
|
||
| /** | ||
| * Params is an "object", since it is a type of RuleActionParams which is action templates. | ||
| * @see x-pack/plugins/alerting/common/rule.ts | ||
| */ | ||
| export type RuleActionParams = t.TypeOf<typeof RuleActionParams>; | ||
| export const RuleActionParams = saved_object_attributes; | ||
|
|
||
| export type RuleAction = t.TypeOf<typeof RuleAction>; | ||
| export const RuleAction = t.exact( | ||
| t.type({ | ||
| group: RuleActionGroup, | ||
| id: RuleActionId, | ||
| action_type_id: RuleActionTypeId, | ||
| params: RuleActionParams, | ||
| }) | ||
| ); | ||
|
|
||
| export type RuleActionArray = t.TypeOf<typeof RuleActionArray>; | ||
| export const RuleActionArray = t.array(RuleAction); | ||
|
|
||
| export type RuleActionCamel = t.TypeOf<typeof RuleActionCamel>; | ||
| export const RuleActionCamel = t.exact( | ||
| t.type({ | ||
| group: RuleActionGroup, | ||
| id: RuleActionId, | ||
| actionTypeId: RuleActionTypeId, | ||
| params: RuleActionParams, | ||
| }) | ||
| ); | ||
|
|
||
| export type RuleActionArrayCamel = t.TypeOf<typeof RuleActionArrayCamel>; | ||
| export const RuleActionArrayCamel = t.array(RuleActionCamel); | ||
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| /* | ||
| * 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 and the Server Side Public License, v 1; you may not use this file except | ||
| * in compliance with, at your election, the Elastic License 2.0 or the Server | ||
| * Side Public License, v 1. | ||
| */ | ||
|
|
||
| import * as t from 'io-ts'; | ||
| import { From } from '../from'; | ||
|
|
||
| export type RuleInterval = t.TypeOf<typeof RuleInterval>; | ||
| export const RuleInterval = t.string; // we need a more specific schema | ||
|
|
||
| export type RuleIntervalFrom = t.TypeOf<typeof RuleIntervalFrom>; | ||
| export const RuleIntervalFrom = From; | ||
|
|
||
| /** | ||
| * TODO: Create a regular expression type or custom date math part type here | ||
| */ | ||
| export type RuleIntervalTo = t.TypeOf<typeof RuleIntervalTo>; | ||
| export const RuleIntervalTo = t.string; // we need a more specific schema |
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'd like to add
@elastic/security-detections-response-alertshere and on/x-pack/plugins/security_solution/server/lib/detection_engine/rule_schemasince our area does a fair amount of work adding fields to the schema, migrating rules, etc. The rule schemas are one of the few places I think shared ownership makes sense so both teams get pinged.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.
@marshallmain I thought you would like to stop owning it, but apparently, I misunderstood you. Shared ownership makes total sense to me here. Will add.