Skip to content

[Security Solution][Alerts] Remove legacy rules schema #137605

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.

Sign up for GitHub

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

Merged
merged 19 commits into from
Sep 13, 2022
Merged

[Security Solution][Alerts] Remove legacy rules schema #137605

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
845f152
Remove legacy rules schema
marshallmain Jul 29, 2022
38317c1
Remove RulesSchema from cypress test
marshallmain Aug 1, 2022
07c2e3c
Merge branch 'main' into remove-legacy-rules-schema
marshallmain Aug 1, 2022
4c01f5b
Fix ml rule API test mock
marshallmain Aug 1, 2022
7a79e6e
Fix type error
marshallmain Aug 1, 2022
44ab4a9
Merge branch 'main' into remove-legacy-rules-schema
marshallmain Aug 22, 2022
5a0a141
Explicitly specify undefined fields in response mocks
marshallmain Aug 22, 2022
63f7ed1
Merge branch 'main' into remove-legacy-rules-schema
marshallmain Aug 22, 2022
b64f9fa
Fix types
marshallmain Aug 22, 2022
9b8ffc6
Remove undefined properties so expect doesn't mess it up
marshallmain Aug 22, 2022
9ca181f
Use more specific types and rename newTransformValidate
marshallmain Aug 24, 2022
17cddda
Fix types in schema tests
marshallmain Aug 24, 2022
147b8bd
Merge branch 'main' into remove-legacy-rules-schema
marshallmain Aug 24, 2022
f43bedc
Merge branch 'main' into remove-legacy-rules-schema
marshallmain Aug 30, 2022
2e7f0bc
Merge branch 'main' into remove-legacy-rules-schema
kibanamachine Sep 6, 2022
5d3bd8a
Review comments - explicit return types, better removeServerGenerated…
marshallmain Sep 7, 2022
0ecb956
Re-add deleted test with modified assertion
marshallmain Sep 7, 2022
560bb8e
Merge branch 'main' into remove-legacy-rules-schema
marshallmain Sep 9, 2022
1649b84
Merge branch 'main' of github.com:elastic/kibana into remove-legacy-r…
marshallmain Sep 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 21 additions & 8 deletions x-pack/plugins/security_solution/common/detection_engine/schemas/request/rule_schemas.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ const {
patch: threatMatchPatchParams,
response: threatMatchResponseParams,
} = buildAPISchemas(threatMatchRuleParams);
export { threatMatchCreateParams };
export { threatMatchCreateParams, threatMatchResponseParams };

const queryRuleParams = {
required: {
Expand All @@ -307,7 +307,7 @@ const {
response: queryResponseParams,
} = buildAPISchemas(queryRuleParams);

export { queryCreateParams };
export { queryCreateParams, queryResponseParams };

const savedQueryRuleParams = {
required: {
Expand All @@ -332,7 +332,7 @@ const {
response: savedQueryResponseParams,
} = buildAPISchemas(savedQueryRuleParams);

export { savedQueryCreateParams };
export { savedQueryCreateParams, savedQueryResponseParams };

const thresholdRuleParams = {
required: {
Expand All @@ -356,7 +356,7 @@ const {
response: thresholdResponseParams,
} = buildAPISchemas(thresholdRuleParams);

export { thresholdCreateParams };
export { thresholdCreateParams, thresholdResponseParams };

const machineLearningRuleParams = {
required: {
Expand All @@ -373,7 +373,7 @@ const {
response: machineLearningResponseParams,
} = buildAPISchemas(machineLearningRuleParams);

export { machineLearningCreateParams };
export { machineLearningCreateParams, machineLearningResponseParams };

const newTermsRuleParams = {
required: {
Expand All @@ -397,7 +397,7 @@ const {
response: newTermsResponseParams,
} = buildAPISchemas(newTermsRuleParams);

export { newTermsCreateParams };
export { newTermsCreateParams, newTermsResponseParams };
// ---------------------------------------
// END type specific parameter definitions

Expand Down Expand Up @@ -503,14 +503,27 @@ const responseOptionalFields = {
execution_summary: RuleExecutionSummary,
};

export const fullResponseSchema = t.intersection([
const sharedResponseSchema = t.intersection([
baseResponseParams,
responseTypeSpecific,
t.exact(t.type(responseRequiredFields)),
t.exact(t.partial(responseOptionalFields)),
]);
type SharedResponseSchema = t.TypeOf<typeof sharedResponseSchema>;
Copy link
Contributor

@banderror banderror Sep 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: let's use either base or shared naming but not both

Copy link
Contributor Author

@marshallmain marshallmain Sep 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sharedResponseSchema here is equivalent to sharedCreateSchema or sharedUpdateSchema but for the response instead, whereas base comes from the baseParams that form the basis of the create, update, patch, and response schemas. What do you think we should rename here?

Copy link
Contributor

@banderror banderror Sep 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd probably rename baseParams to sharedParams etc. But I'm nitpicking here, the naming as it is now is clear enough, so feel free to ignore the suggestion.

export const fullResponseSchema = t.intersection([sharedResponseSchema, responseTypeSpecific]);
export type FullResponseSchema = t.TypeOf<typeof fullResponseSchema>;

// Convenience types for type specific responses
type ResponseSchema<T> = SharedResponseSchema & T;
export type EqlResponseSchema = ResponseSchema<t.TypeOf<typeof eqlResponseParams>>;
export type ThreatMatchResponseSchema = ResponseSchema<t.TypeOf<typeof threatMatchResponseParams>>;
export type QueryResponseSchema = ResponseSchema<t.TypeOf<typeof queryResponseParams>>;
export type SavedQueryResponseSchema = ResponseSchema<t.TypeOf<typeof savedQueryResponseParams>>;
export type ThresholdResponseSchema = ResponseSchema<t.TypeOf<typeof thresholdResponseParams>>;
export type MachineLearningResponseSchema = ResponseSchema<
t.TypeOf<typeof machineLearningResponseParams>
>;
export type NewTermsResponseSchema = ResponseSchema<t.TypeOf<typeof newTermsResponseParams>>;

export interface RulePreviewLogs {
errors: string[];
warnings: string[];
Expand Down
2 changes: 0 additions & 2 deletions x-pack/plugins/security_solution/common/detection_engine/schemas/response/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,3 @@ export * from './import_rules_schema';
export * from './prepackaged_rules_schema';
export * from './prepackaged_rules_status_schema';
export * from './rules_bulk_schema';
export * from './rules_schema';
export * from './type_timeline_only_schema';
22 changes: 3 additions & 19 deletions ...gins/security_solution/common/detection_engine/schemas/response/rules_bulk_schema.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ import { pipe } from 'fp-ts/lib/pipeable';

import type { RulesBulkSchema } from './rules_bulk_schema';
import { rulesBulkSchema } from './rules_bulk_schema';
import type { RulesSchema } from './rules_schema';
import type { ErrorSchema } from './error_schema';
import { exactCheck, foldLeftRight, getPaths } from '@kbn/securitysolution-io-ts-utils';

import { getRulesSchemaMock } from './rules_schema.mocks';
import { getErrorSchemaMock } from './error_schema.mocks';
import type { FullResponseSchema } from '../request';

describe('prepackaged_rule_schema', () => {
test('it should validate a regular message and and error together with a uuid', () => {
Expand Down Expand Up @@ -64,24 +64,8 @@ describe('prepackaged_rule_schema', () => {
expect(message.schema).toEqual({});
});

test('it should NOT validate an invalid error message with a deleted value', () => {
const error = getErrorSchemaMock('fake id');
// @ts-expect-error
delete error.error;
const payload: RulesBulkSchema = [error];
const decoded = rulesBulkSchema.decode(payload);
const checked = exactCheck(payload, decoded);
const message = pipe(checked, foldLeftRight);

expect(getPaths(left(message.errors))).toEqual([
'Invalid value "undefined" supplied to "type"',
'Invalid value "undefined" supplied to "error"',
]);
expect(message.schema).toEqual({});
});

test('it should NOT validate a type of "query" when it has extra data', () => {
const rule: RulesSchema & { invalid_extra_data?: string } = getRulesSchemaMock();
const rule: FullResponseSchema & { invalid_extra_data?: string } = getRulesSchemaMock();
rule.invalid_extra_data = 'invalid_extra_data';
const payload: RulesBulkSchema = [rule];
const decoded = rulesBulkSchema.decode(payload);
Expand All @@ -93,7 +77,7 @@ describe('prepackaged_rule_schema', () => {
});

test('it should NOT validate a type of "query" when it has extra data next to a valid error', () => {
const rule: RulesSchema & { invalid_extra_data?: string } = getRulesSchemaMock();
const rule: FullResponseSchema & { invalid_extra_data?: string } = getRulesSchemaMock();
rule.invalid_extra_data = 'invalid_extra_data';
const payload: RulesBulkSchema = [getErrorSchemaMock(), rule];
const decoded = rulesBulkSchema.decode(payload);
Expand Down
4 changes: 2 additions & 2 deletions ...k/plugins/security_solution/common/detection_engine/schemas/response/rules_bulk_schema.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@

import * as t from 'io-ts';

import { rulesSchema } from './rules_schema';
import { fullResponseSchema } from '../request';
import { errorSchema } from './error_schema';

export const rulesBulkSchema = t.array(t.union([rulesSchema, errorSchema]));
export const rulesBulkSchema = t.array(t.union([fullResponseSchema, errorSchema]));
export type RulesBulkSchema = t.TypeOf<typeof rulesBulkSchema>;
91 changes: 75 additions & 16 deletions .../plugins/security_solution/common/detection_engine/schemas/response/rules_schema.mocks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,18 @@
*/

import { DEFAULT_INDICATOR_SOURCE_PATH } from '../../../constants';
import type {
Copy link
Contributor

@banderror banderror Sep 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After the changes, now we have rules_schema.mocks.ts and rules_schema.test.ts under both directories: request and response. Some of the tests under response have been deleted.

  • What's the difference between those tests and mocks that are still there?
  • Can we consolidate all of them under the request folder?

Copy link
Contributor Author

@marshallmain marshallmain Sep 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the difference between those tests and mocks that are still there?

The tests and mocks in each folder still correspond to the request and response schemas respectively. It definitely deserves to be reorganized, but I tried to avoid doing reorganization at the same time as the more technical changes. request/rule_schemas.test.ts is focused on testing the createRulesSchema - we don't have independent tests for updateRulesSchema at this point, but it's almost identical to createRulesSchema. Similarly the mocks in request/rule_schemas.mock.ts are all for the create schemas.

Since request/rule_schemas.ts is where all of the core schema logic lives, it ends up being the source for the new response schemas and replaces response/rules_schema.ts - which is simply deleted. However, I kept response/rules_schema.test.ts and response/rules_schema.mock.ts and updated them to use the new schemas for better assurance that the switch over to the new schema isn't breaking things.

Can we consolidate all of them under the request folder?

I think we can consolidate by moving most of request/rule_schemas.ts to common/rule_schemas.ts and better separating the "base" and "type specific" schema logic from create, update, response, etc. This way we can keep request and response schemas organized into their own sections, and also remove the dependency of response on request/rule_schemas.ts.

Copy link
Contributor

@banderror banderror Sep 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thank you for clarifying this! Could you please add a few comments to those files to explain this:

The tests and mocks in each folder still correspond to the request and response schemas respectively. It definitely deserves to be reorganized, but I tried to avoid doing reorganization at the same time as the more technical changes. request/rule_schemas.test.ts is focused on testing the createRulesSchema - we don't have independent tests for updateRulesSchema at this point, but it's almost identical to createRulesSchema. Similarly the mocks in request/rule_schemas.mock.ts are all for the create schemas.

Since request/rule_schemas.ts is where all of the core schema logic lives, it ends up being the source for the new response schemas and replaces response/rules_schema.ts - which is simply deleted. However, I kept response/rules_schema.test.ts and response/rules_schema.mock.ts and updated them to use the new schemas for better assurance that the switch over to the new schema isn't breaking things.

Copy link
Contributor

@banderror banderror Sep 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can consolidate by moving most of request/rule_schemas.ts to common/rule_schemas.ts and better separating the "base" and "type specific" schema logic from create, update, response, etc. This way we can keep request and response schemas organized into their own sections, and also remove the dependency of response on request/rule_schemas.ts.

++ Makes total sense. I added this to #138606 as a sub-task.

EqlResponseSchema,
MachineLearningResponseSchema,
QueryResponseSchema,
SavedQueryResponseSchema,
ThreatMatchResponseSchema,
} from '../request';
import { getListArrayMock } from '../types/lists.mock';

import type { RulesSchema } from './rules_schema';

export const ANCHOR_DATE = '2020-02-20T03:57:54.037Z';

export const getRulesSchemaMock = (anchorDate: string = ANCHOR_DATE): RulesSchema => ({
const getResponseBaseParams = (anchorDate: string = ANCHOR_DATE) => ({
author: [],
id: '7a7065d7-6e8b-4aae-8d20-c93613dec9f9',
created_at: new Date(anchorDate).toISOString(),
Expand All @@ -24,45 +29,83 @@ export const getRulesSchemaMock = (anchorDate: string = ANCHOR_DATE): RulesSchem
from: 'now-6m',
immutable: false,
name: 'Query with a rule id',
query: 'user.name: root or user.name: admin',
references: ['test 1', 'test 2'],
severity: 'high',
severity: 'high' as const,
severity_mapping: [],
updated_by: 'elastic_kibana',
tags: ['some fake tag 1', 'some fake tag 2'],
to: 'now',
type: 'query',
threat: [],
version: 1,
output_index: '.siem-signals-default',
max_signals: 100,
risk_score: 55,
risk_score_mapping: [],
language: 'kuery',
rule_id: 'query-rule-id',
interval: '5m',
exceptions_list: getListArrayMock(),
related_integrations: [],
required_fields: [],
setup: '',
throttle: 'no_actions',
actions: [],
building_block_type: undefined,
note: undefined,
license: undefined,
outcome: undefined,
alias_target_id: undefined,
alias_purpose: undefined,
timeline_id: undefined,
timeline_title: undefined,
meta: undefined,
rule_name_override: undefined,
timestamp_override: undefined,
timestamp_override_fallback_disabled: undefined,
namespace: undefined,
});

export const getRulesMlSchemaMock = (anchorDate: string = ANCHOR_DATE): RulesSchema => {
const basePayload = getRulesSchemaMock(anchorDate);
const { filters, index, query, language, ...rest } = basePayload;
export const getRulesSchemaMock = (anchorDate: string = ANCHOR_DATE): QueryResponseSchema => ({
...getResponseBaseParams(anchorDate),
query: 'user.name: root or user.name: admin',
type: 'query',
language: 'kuery',
index: undefined,
data_view_id: undefined,
filters: undefined,
saved_id: undefined,
});
export const getSavedQuerySchemaMock = (
anchorDate: string = ANCHOR_DATE
): SavedQueryResponseSchema => ({
...getResponseBaseParams(anchorDate),
query: 'user.name: root or user.name: admin',
type: 'saved_query',
saved_id: 'save id 123',
language: 'kuery',
index: undefined,
data_view_id: undefined,
filters: undefined,
});

export const getRulesMlSchemaMock = (
anchorDate: string = ANCHOR_DATE
): MachineLearningResponseSchema => {
return {
...rest,
...getResponseBaseParams(anchorDate),
type: 'machine_learning',
anomaly_threshold: 59,
machine_learning_job_id: 'some_machine_learning_job_id',
};
};

export const getThreatMatchingSchemaMock = (anchorDate: string = ANCHOR_DATE): RulesSchema => {
export const getThreatMatchingSchemaMock = (
anchorDate: string = ANCHOR_DATE
): ThreatMatchResponseSchema => {
return {
...getRulesSchemaMock(anchorDate),
...getResponseBaseParams(anchorDate),
type: 'threat_match',
query: 'user.name: root or user.name: admin',
language: 'kuery',
threat_index: ['index-123'],
threat_mapping: [{ entries: [{ field: 'host.name', type: 'mapping', value: 'host.name' }] }],
threat_query: '*:*',
Expand All @@ -84,14 +127,24 @@ export const getThreatMatchingSchemaMock = (anchorDate: string = ANCHOR_DATE): R
},
},
],
index: undefined,
data_view_id: undefined,
filters: undefined,
saved_id: undefined,
threat_indicator_path: undefined,
threat_language: undefined,
concurrent_searches: undefined,
items_per_search: undefined,
};
};

/**
* Useful for e2e backend tests where it doesn't have date time and other
* server side properties attached to it.
*/
export const getThreatMatchingSchemaPartialMock = (enabled = false): Partial<RulesSchema> => {
export const getThreatMatchingSchemaPartialMock = (
enabled = false
): Partial<ThreatMatchResponseSchema> => {
return {
author: [],
created_by: 'elastic',
Expand Down Expand Up @@ -160,11 +213,17 @@ export const getThreatMatchingSchemaPartialMock = (enabled = false): Partial<Rul
};
};

export const getRulesEqlSchemaMock = (anchorDate: string = ANCHOR_DATE): RulesSchema => {
export const getRulesEqlSchemaMock = (anchorDate: string = ANCHOR_DATE): EqlResponseSchema => {
return {
...getRulesSchemaMock(anchorDate),
...getResponseBaseParams(anchorDate),
language: 'eql',
type: 'eql',
query: 'process where true',
index: undefined,
data_view_id: undefined,
filters: undefined,
timestamp_field: undefined,
event_category_override: undefined,
tiebreaker_field: undefined,
};
};
Loading