Skip to content

Commit

Permalink
[Security Solution] Restructuring folders of Detection Engine + refac…
Browse files Browse the repository at this point in the history
…toring Rule Management (#142950)

**Partially addresses:** #138600, #92169, #138606
**Addresses:** #136957, #136962, #138614

## Summary

In this PR we are:

- Splitting the Detection Engine into subdomains ([ticket](#138600)). Every subdomain got its own folder under `detection_engine`, and we moved some (not all) code into them. More on that is below. New subdomains introduced:
  - `fleet_integrations`
  - `prebuilt_rules`
  - `rule_actions_legacy`
  - `rule_exceptions`
  - `rule_management`
  - `rule_preview`
  - `rule_schema`
  - `rule_creation_ui`
  - `rule_details_ui`
  - `rule_management_ui`
  - `rule_exceptions_ui`
- Updating the CODEOWNERS file accordingly.
- Refactoring the Rule Management page and the Rules table. Our main focus was on the way how we communicate with the API endpoints, how we cache and invalidate the fetched data, and how this code is organized in the codebase. More on that is below.
- Increasing the bundle size limit. This is going to be decreased back in a follow-up PR ([ticket](#143532))

## Restructuring folders into subdomains

For the background and problem statement, please refer to #138600

We were focusing on code that is closely related to the Rules area: either owned by us de facto (we work on it) or owned by us de jure (according to the CODEOWNERS file). Or goal was to explicitly extract code that we don't own de facto into separate subdomains, transfer ownership to other area teams, and reflect this in the CODEOWNERS file. On the other hand, we wanted the code that we own to also be organized in clear subdomains that we could easily own via CODEOWNERS. We didn't touch the code that is already explicitly owned by other area teams, e.g. `x-pack/plugins/security_solution/server/lib/detection_engine/rule_types`.

This is a draft "domain map" - an architectural diagram that shows how the Detection Engine _could_ be split into subdomains. It's more a TO-BE idea/aspiration rather than an AS-IS statement. Any feedback, critiques, and suggestions would be extremely appreciated!

<img width="2592" alt="Screenshot 2022-10-18 at 16 08 40" src="https://user-images.githubusercontent.com/7359339/196453965-b65f5b49-9a33-4d90-bb48-1347e9576223.png">

It shows the flow of dependencies between subdomains and proposes some rules:

- The whole graph of dependencies between all subdomains should be a DAG. There should not be bi-directional or circular dependencies between them.
- **Generic subdomains** represent some general knowledge that can be used/applied outside of the Detection Engine.
  - Can depend on some generic kbn packages, npm packages or utils.
  - Can't depend on any other Detection Engine subdomains.
- **Crosscutting subdomains** represent some code that can be common to / shared between many other subdomains. This could be some very common domain models and API schemas.
  - Can depend on generic subdomains.
  - Can depend on other crosscutting subdomains (dependencies between them must form a DAG).
  - Can't depend on core or UI subdomains.
- **Core subdomains** contain most of the "meat" of the Detection Engine: domain models, server-side and client-side business logic, server-side API endpoints, client-side UI (potentially shareable between several pages).
  - Can depend on crosscutting and generic subdomains.
  - Can depend on other core subdomains (dependencies between them must form a DAG).
  - Can't depend on UI subdomains.
- **UI subdomains** contain the implementation of pages related to the Detection Engine. Every page can easily depend on several core subdomains, so these subdomain are on top of everything.
  - Can depend on any other subdomains. Dependencies must form a DAG.

Dashed lines show some existing dependencies that we think should be eliminated.

Ownership TO-BE is color-coded. We updated the CODEOWNERS file according to the new folders.

The folder restructuring is not 100% finished but we did a big part of it. Most of the FE code continues to live in legacy folders, e.g. see `x-pack/plugins/security_solution/public/detections`. So this work is to be continued...

## Refactoring of Rule Management FE

- [x] #136957 For effective HTTP requests caching and deduplication, we've migrated all data fetching logic to `useQuery` and `useMutation` hooks from `react-query`. That allowed us to introduce the following improvements to our codebase:
  * All outgoing HTTP requests are now automatically deduplicated. That means that data fetching hooks like `useRule` could be used on any level in the component tree to access response data directly. So, no need to put the hook on the top level anymore and use prop-drilling to make the response data available to all children components that require it.
  * All HTTP responses are now cached with the default TTL of 5 minutes—no more redundant requests. With a hot cache, transitions to some pages now happen immediately. 
- [x] #136962 Data fetching hooks of the Rules Area are now organized in one place. `security_solution/public/detection_engine/rule_management/api/hooks` contains abstraction layer on top of the Kibana's HTTP client. All data fetching should happen exclusively through that layer to ensure that:
  * Mutation queries automatically invalidate associated cache entries.
  * Optimistic updates or updates from mutation responses could be implemented centrally where possible.
- [x] #92169 From some of the Rule Management components, logic was extracted to hooks located in `security_solution/public/detection_engine/rule_management/logic`. 

### Checklist

- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
  • Loading branch information
banderror authored Oct 21, 2022
1 parent 220f867 commit 21de750
Show file tree
Hide file tree
Showing 1,423 changed files with 9,852 additions and 10,901 deletions.
37 changes: 21 additions & 16 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -477,26 +477,31 @@ x-pack/examples/files_example @elastic/kibana-app-services
/x-pack/plugins/security_solution/common/detection_engine/schemas/alerts @elastic/security-detections-response-alerts
/x-pack/plugins/security_solution/common/field_maps @elastic/security-detections-response-alerts

/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui @elastic/security-detections-response-alerts
/x-pack/plugins/security_solution/public/detections/pages/alerts @elastic/security-detections-response-alerts

/x-pack/plugins/security_solution/server/lib/detection_engine/migrations @elastic/security-detections-response-alerts
/x-pack/plugins/security_solution/server/lib/detection_engine/notifications @elastic/security-detections-response-alerts
/x-pack/plugins/security_solution/server/lib/detection_engine/schemas @elastic/security-detections-response-alerts
/x-pack/plugins/security_solution/server/lib/detection_engine/rule_preview @elastic/security-detections-response-alerts
/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types @elastic/security-detections-response-alerts
/x-pack/plugins/security_solution/server/lib/detection_engine/signals @elastic/security-detections-response-alerts
/x-pack/plugins/security_solution/server/lib/detection_engine/routes/index @elastic/security-detections-response-alerts
/x-pack/plugins/security_solution/server/lib/detection_engine/routes/signals @elastic/security-detections-response-alerts

## Security Solution sub teams - Detections and Response Rules
/x-pack/plugins/security_solution/common/detection_engine/fleet_integrations @elastic/security-detections-response-rules
/x-pack/plugins/security_solution/common/detection_engine/prebuilt_rules @elastic/security-detections-response-rules
/x-pack/plugins/security_solution/common/detection_engine/rule_management @elastic/security-detections-response-rules
/x-pack/plugins/security_solution/common/detection_engine/rule_monitoring @elastic/security-detections-response-rules
/x-pack/plugins/security_solution/common/detection_engine/schemas/common @elastic/security-detections-response-rules
/x-pack/plugins/security_solution/common/detection_engine/schemas/request @elastic/security-detections-response-rules
/x-pack/plugins/security_solution/common/detection_engine/schemas/response @elastic/security-detections-response-rules
/x-pack/plugins/security_solution/common/detection_engine/rule_schema @elastic/security-detections-response-rules @elastic/security-detections-response-alerts

/x-pack/plugins/security_solution/public/common/components/health_truncate_text @elastic/security-detections-response-rules
/x-pack/plugins/security_solution/public/common/components/links_to_docs @elastic/security-detections-response-rules
/x-pack/plugins/security_solution/public/common/components/ml_popover @elastic/security-detections-response-rules
/x-pack/plugins/security_solution/public/common/components/popover_items @elastic/security-detections-response-rules
/x-pack/plugins/security_solution/public/detection_engine/fleet_integrations @elastic/security-detections-response-rules
/x-pack/plugins/security_solution/public/detection_engine/rule_details_ui @elastic/security-detections-response-rules
/x-pack/plugins/security_solution/public/detection_engine/rule_management @elastic/security-detections-response-rules
/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui @elastic/security-detections-response-rules
/x-pack/plugins/security_solution/public/detection_engine/rule_monitoring @elastic/security-detections-response-rules
/x-pack/plugins/security_solution/public/detections/components/callouts @elastic/security-detections-response-rules
/x-pack/plugins/security_solution/public/detections/components/modals/ml_job_upgrade_modal @elastic/security-detections-response-rules
Expand All @@ -507,17 +512,12 @@ x-pack/examples/files_example @elastic/kibana-app-services
/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules @elastic/security-detections-response-rules
/x-pack/plugins/security_solution/public/rules @elastic/security-detections-response-rules

/x-pack/plugins/security_solution/server/lib/detection_engine/routes/fleet @elastic/security-detections-response-rules
/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules @elastic/security-detections-response-rules
/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/create_rule_exceptions_route* @elastic/security-solution-platform
/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/find_rule_exceptions_route* @elastic/security-solution-platform
/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route* @elastic/security-solution-platform
/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/preview_rules_route* @elastic/security-detections-response-alerts
/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils @elastic/security-solution-platform
/x-pack/plugins/security_solution/server/lib/detection_engine/routes/tags @elastic/security-detections-response-rules
/x-pack/plugins/security_solution/server/lib/detection_engine/fleet_integrations @elastic/security-detections-response-rules
/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules @elastic/security-detections-response-rules
/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management @elastic/security-detections-response-rules
/x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring @elastic/security-detections-response-rules
/x-pack/plugins/security_solution/server/lib/detection_engine/rules @elastic/security-detections-response-rules
/x-pack/plugins/security_solution/server/lib/detection_engine/tags @elastic/security-detections-response-rules
/x-pack/plugins/security_solution/server/lib/detection_engine/rule_schema @elastic/security-detections-response-rules @elastic/security-detections-response-alerts

/x-pack/plugins/security_solution/server/utils @elastic/security-detections-response-rules

## Security Solution sub teams - Security Platform
Expand All @@ -527,12 +527,17 @@ x-pack/examples/files_example @elastic/kibana-app-services
/x-pack/plugins/security_solution/cypress/e2e/exceptions @elastic/security-solution-platform
/x-pack/plugins/security_solution/cypress/e2e/value_lists @elastic/security-solution-platform

/x-pack/plugins/security_solution/common/detection_engine/rule_exceptions @elastic/security-solution-platform

/x-pack/plugins/security_solution/public/detection_engine/rule_exceptions @elastic/security-solution-platform
/x-pack/plugins/security_solution/public/detection_engine/rule_exceptions_ui @elastic/security-solution-platform
/x-pack/plugins/security_solution/public/common/components/exceptions @elastic/security-solution-platform
/x-pack/plugins/security_solution/public/exceptions @elastic/security-solution-platform
/x-pack/plugins/security_solution/public/detections/containers/detection_engine/lists @elastic/security-solution-platform
/x-pack/plugins/security_solution/public/common/components/sourcerer @elastic/security-solution-platform

/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions_legacy @elastic/security-solution-platform
/x-pack/plugins/security_solution/server/lib/detection_engine/rule_exceptions @elastic/security-solution-platform
/x-pack/plugins/security_solution/server/lib/sourcerer @elastic/security-solution-platform

## Security Threat Intelligence - Under Security Platform
Expand Down Expand Up @@ -595,7 +600,7 @@ x-pack/test/threat_intelligence_cypress @elastic/protections-experience


# Security Intelligence And Analytics
/x-pack/plugins/security_solution/server/lib/detection_engine/rules/prepackaged_rules @elastic/security-intelligence-analytics
/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/content/prepackaged_rules @elastic/security-intelligence-analytics


# Security Asset Management
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@ export * from './src/default_severity_mapping_array';
export * from './src/default_threat_array';
export * from './src/default_to_string';
export * from './src/default_uuid';
export * from './src/from';
export * from './src/language';
export * from './src/machine_learning_job_id';
export * from './src/max_signals';
export * from './src/normalized_ml_job_id';
export * from './src/references_default_array';
export * from './src/risk_score';
export * from './src/risk_score_mapping';
export * from './src/rule_schedule';
export * from './src/saved_object_attributes';
export * from './src/severity';
export * from './src/severity_mapping';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,42 +6,47 @@
* Side Public License, v 1.
*/

/* eslint-disable @typescript-eslint/naming-convention */

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;

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 const action_group = t.string;
export const action_id = t.string;
export const action_action_type_id = t.string;
export const action_params = saved_object_attributes;
export type RuleActionParams = t.TypeOf<typeof RuleActionParams>;
export const RuleActionParams = saved_object_attributes;

export const action = t.exact(
export type RuleAction = t.TypeOf<typeof RuleAction>;
export const RuleAction = t.exact(
t.type({
group: action_group,
id: action_id,
action_type_id: action_action_type_id,
params: action_params,
group: RuleActionGroup,
id: RuleActionId,
action_type_id: RuleActionTypeId,
params: RuleActionParams,
})
);

export type Action = t.TypeOf<typeof action>;
export type RuleActionArray = t.TypeOf<typeof RuleActionArray>;
export const RuleActionArray = t.array(RuleAction);

export const actions = t.array(action);
export type Actions = t.TypeOf<typeof actions>;

export const actionsCamel = t.array(
t.exact(
t.type({
group: action_group,
id: action_id,
actionTypeId: action_action_type_id,
params: action_params,
})
)
export type RuleActionCamel = t.TypeOf<typeof RuleActionCamel>;
export const RuleActionCamel = t.exact(
t.type({
group: RuleActionGroup,
id: RuleActionId,
actionTypeId: RuleActionTypeId,
params: RuleActionParams,
})
);
export type ActionsCamel = t.TypeOf<typeof actions>;

export type RuleActionArrayCamel = t.TypeOf<typeof RuleActionArrayCamel>;
export const RuleActionArrayCamel = t.array(RuleActionCamel);
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,16 @@

import * as t from 'io-ts';
import { Either } from 'fp-ts/lib/Either';
import { actions, Actions } from '../actions';
import { RuleActionArray } from '../actions';

export const DefaultActionsArray = new t.Type<Actions, Actions | undefined, unknown>(
export const DefaultActionsArray = new t.Type<
RuleActionArray,
RuleActionArray | undefined,
unknown
>(
'DefaultActionsArray',
actions.is,
(input, context): Either<t.Errors, Actions> =>
input == null ? t.success([]) : actions.validate(input, context),
RuleActionArray.is,
(input, context): Either<t.Errors, RuleActionArray> =>
input == null ? t.success([]) : RuleActionArray.validate(input, context),
t.identity
);
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import * as t from 'io-ts';
import { Either } from 'fp-ts/lib/Either';
import { from } from '../from';
import { From } from '../from';

/**
* Types the DefaultFromString as:
Expand All @@ -21,7 +21,7 @@ export const DefaultFromString = new t.Type<string, string | undefined, unknown>
if (input == null) {
return t.success('now-6m');
}
return from.validate(input, context);
return From.validate(input, context);
},
t.identity
);
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,20 @@

import * as t from 'io-ts';
import { Either } from 'fp-ts/lib/Either';
import { RiskScoreMapping, risk_score_mapping } from '../risk_score_mapping';
import { RiskScoreMapping } from '../risk_score_mapping';

/**
* Types the DefaultStringArray as:
* - If null or undefined, then a default risk_score_mapping array will be set
* - If null or undefined, then a default RiskScoreMapping array will be set
*/
export const DefaultRiskScoreMappingArray = new t.Type<
RiskScoreMapping,
RiskScoreMapping | undefined,
unknown
>(
'DefaultRiskScoreMappingArray',
risk_score_mapping.is,
RiskScoreMapping.is,
(input, context): Either<t.Errors, RiskScoreMapping> =>
input == null ? t.success([]) : risk_score_mapping.validate(input, context),
input == null ? t.success([]) : RiskScoreMapping.validate(input, context),
t.identity
);
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,20 @@

import * as t from 'io-ts';
import { Either } from 'fp-ts/lib/Either';
import { SeverityMapping, severity_mapping } from '../severity_mapping';
import { SeverityMapping } from '../severity_mapping';

/**
* Types the DefaultStringArray as:
* - If null or undefined, then a default severity_mapping array will be set
* - If null or undefined, then a default SeverityMapping array will be set
*/
export const DefaultSeverityMappingArray = new t.Type<
SeverityMapping,
SeverityMapping | undefined,
unknown
>(
'DefaultSeverityMappingArray',
severity_mapping.is,
SeverityMapping.is,
(input, context): Either<t.Errors, SeverityMapping> =>
input == null ? t.success([]) : severity_mapping.validate(input, context),
input == null ? t.success([]) : SeverityMapping.validate(input, context),
t.identity
);
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ import { parseScheduleDates } from '@kbn/securitysolution-io-ts-utils';

const stringValidator = (input: unknown): input is string => typeof input === 'string';

export const from = new t.Type<string, string, unknown>(
export type From = t.TypeOf<typeof From>;
export const From = new t.Type<string, string, unknown>(
'From',
t.string.is,
(input, context): Either<t.Errors, string> => {
Expand All @@ -23,7 +24,3 @@ export const from = new t.Type<string, string, unknown>(
},
t.identity
);
export type From = t.TypeOf<typeof from>;

export const fromOrUndefined = t.union([from, t.undefined]);
export type FromOrUndefined = t.TypeOf<typeof fromOrUndefined>;
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
* Side Public License, v 1.
*/

/* eslint-disable @typescript-eslint/naming-convention */

import * as t from 'io-ts';
import { Either } from 'fp-ts/lib/Either';

Expand All @@ -16,6 +14,7 @@ import { Either } from 'fp-ts/lib/Either';
* - Natural Number (positive integer and not a float),
* - Between the values [0 and 100] inclusive.
*/
export type RiskScore = t.TypeOf<typeof RiskScore>;
export const RiskScore = new t.Type<number, number, unknown>(
'RiskScore',
t.number.is,
Expand All @@ -26,11 +25,3 @@ export const RiskScore = new t.Type<number, number, unknown>(
},
t.identity
);

export type RiskScoreC = typeof RiskScore;

export const risk_score = RiskScore;
export type RiskScore = t.TypeOf<typeof risk_score>;

export const riskScoreOrUndefined = t.union([risk_score, t.undefined]);
export type RiskScoreOrUndefined = t.TypeOf<typeof riskScoreOrUndefined>;
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,19 @@
* Side Public License, v 1.
*/

/* eslint-disable @typescript-eslint/naming-convention */

import * as t from 'io-ts';
import { operator } from '@kbn/securitysolution-io-ts-types';
import { riskScoreOrUndefined } from '../risk_score';
import { RiskScore } from '../risk_score';

export const risk_score_mapping_field = t.string;
export const risk_score_mapping_value = t.string;
export const risk_score_mapping_item = t.exact(
export type RiskScoreMappingItem = t.TypeOf<typeof RiskScoreMappingItem>;
export const RiskScoreMappingItem = t.exact(
t.type({
field: risk_score_mapping_field,
value: risk_score_mapping_value,
field: t.string,
value: t.string,
operator,
risk_score: riskScoreOrUndefined,
risk_score: t.union([RiskScore, t.undefined]),
})
);

export const risk_score_mapping = t.array(risk_score_mapping_item);
export type RiskScoreMapping = t.TypeOf<typeof risk_score_mapping>;

export const riskScoreMappingOrUndefined = t.union([risk_score_mapping, t.undefined]);
export type RiskScoreMappingOrUndefined = t.TypeOf<typeof riskScoreMappingOrUndefined>;
export type RiskScoreMapping = t.TypeOf<typeof RiskScoreMapping>;
export const RiskScoreMapping = t.array(RiskScoreMappingItem);
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
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,5 @@

import * as t from 'io-ts';

export const severity = t.keyof({ low: null, medium: null, high: null, critical: null });
export type Severity = t.TypeOf<typeof severity>;

export const severityOrUndefined = t.union([severity, t.undefined]);
export type SeverityOrUndefined = t.TypeOf<typeof severityOrUndefined>;
export type Severity = t.TypeOf<typeof Severity>;
export const Severity = t.keyof({ low: null, medium: null, high: null, critical: null });
Loading

0 comments on commit 21de750

Please sign in to comment.