[Security Solution] Migrate Rules schema to OpenAPI #167999

xcrzx · 2023-10-04T14:00:30Z

Parent meta ticket: https://github.com/elastic/security-team/issues/7491

Resolves: https://github.com/elastic/security-team/issues/7582
Resolves: https://github.com/elastic/security-team/issues/7580
Resolves: https://github.com/elastic/security-team/issues/7581

Summary

This PR migrates the rules schema to OpenAPI, Zod, and code generation.

The following APIs now have complete OpenAPI specifications and are enabled for code generation:

Method	Endpoint	OpenAPI spec	Fully migrated
POST	/api/detection_engine/rules	✅	✅
GET	/api/detection_engine/rules	✅	✅
PUT	/api/detection_engine/rules	✅	✅
PATCH	/api/detection_engine/rules	✅	✅
DELETE	/api/detection_engine/rules	✅	✅
POST	/api/detection_engine/rules/_bulk_create	✅	✅
PUT	/api/detection_engine/rules/_bulk_update	✅	✅
PATCH	/api/detection_engine/rules/_bulk_update	✅	✅
DELETE	/api/detection_engine/rules/_bulk_delete	✅	✅
POST	/api/detection_engine/rules/_bulk_delete	✅	✅

Rule schemas are now forward-compatible

We now allow extra fields in schemas for forward compatibility, but we remove them from the payload during parsing. So from now on, extra fields are simply ignored and won't lead to validation errors.

vitaliidm

Comment not actual anymore, refer to #167999 (review)

michaelolo24

Investigations codeowners review. Thanks for updating the types!

.../security_solution/common/api/detection_engine/model/rule_schema/rule_request_schema.test.ts

x-pack/plugins/security_solution/common/detection_engine/transform_actions.ts

...blic/detection_engine/rule_exceptions/components/flyout_components/item_conditions/index.tsx

rylnd

Risk engine schema changes (updating an import, adding an eslint-disable look good to me.

rylnd · 2023-10-24T19:23:30Z

x-pack/plugins/security_solution/common/risk_engine/risk_score_calculation/request_schema.ts

-import { DataViewId } from '../../api/detection_engine';
+// TODO https://github.com/elastic/security-team/issues/7491
+// eslint-disable-next-line no-restricted-imports
+import { DataViewId } from '../../api/detection_engine/model/rule_schema_legacy';


Do you happen to know why this is a restricted import? They're both common files in the same plugin.

The path is restricted because it includes _legacy in it. We have plans to remove those legacy schemas, and in the meantime, we don't want people to use them accidentally. So adding legacy to the path helps with that.

maximpn · 2023-10-25T17:51:14Z

...ecurity_solution/common/api/detection_engine/model/rule_schema/common_attributes.schema.yaml

@@ -6,13 +6,19 @@ paths: {}
 components:
  x-codegen-enabled: true
  schemas:
+    NonEmptyString:
+      type: string
+      pattern: ^(?! *$).+$


Just curious why using this relatively complex regex while it could be ^[^\s]+$ (require one ore more non space characters)? While it's stricter than NonEmptyString defined previously in via io-ts but we still don't expect any kind of space characters as a rule name.

In my honest opinion, we need neither of those regexes 🙂 I'd define a non-empty string as a string with length > 0, regardless of its content.

banderror

Fantastic and absolutely humongous work @xcrzx! Thank you for addressing so many comments and moving our architecture in the right direction. This is so, so much better than our legacy schemas written with io-ts. I'm excited about all the other improvements that we can and will do based on this work!

banderror · 2023-10-26T13:46:43Z

...ugins/security_solution/public/detection_engine/rule_details_ui/pages/rule_details/index.tsx

@@ -74,6 +73,7 @@ import {
 import { RuleSwitch } from '../../../../detections/components/rules/rule_switch';
 import { StepPanel } from '../../../../detections/components/rules/step_panel';
 import {
+  getMachineLearningJobId,


Nit: the function should be called getMachineLearningJobIds because it returns an array. We could rename it later

banderror · 2023-10-26T13:51:56Z

...on/public/detection_engine/rule_details_ui/pages/rule_details/use_rule_details_tabs.test.tsx

@@ -32,6 +32,7 @@ const mockRule: Rule = {
  severity: 'low',
  type: 'query',
  query: 'some query',
+  language: 'kuery',


Thank you for getting rid of the frontend-side Rule "schema"! I guess this diff shows that this highlighted some bugs in our code where we forgot to specify all the fields required by RuleResponse.

banderror · 2023-10-26T13:58:56Z

x-pack/plugins/security_solution/public/detection_engine/rule_management/logic/types.ts

-export const RuleSchema = t.intersection([
-  t.type({
-    author: RuleAuthorArray,
-    created_at: t.string,


No more frontend-side (duplicated and inaccurate) rule schema anymore! 🙌
cc @elastic/security-detection-rule-management @elastic/security-detection-engine

banderror · 2023-10-26T14:10:19Z

...solution/server/lib/detection_engine/prebuilt_rules/model/rule_assets/prebuilt_rule_asset.ts

+export type PrebuiltRuleAsset = z.infer<typeof PrebuiltRuleAsset>;
+export const PrebuiltRuleAsset = BaseCreateProps.and(TypeSpecificCreateProps).and(
+  z.object({
+    rule_id: RuleSignatureId,
+    version: RuleVersion,
+    related_integrations: RelatedIntegrationArray.optional(),
+    required_fields: RequiredFieldArray.optional(),
+    setup: SetupGuide.optional(),
+  })
+);


Awesome!! 🙌

Rule schema validation should be addressed in subsequent PRs: #167999 (comment)

kibana-ci · 2023-10-27T15:41:01Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 5c2609e

Failed CI Steps

Test Failures

[job] [logs] Security Solution Cypress Tests #10 / Changing alert status Opening alerts can bulk open alerts can bulk open alerts

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`securitySolution`	4640	4651	+11

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`@kbn/securitysolution-es-utils`	78	76	-2
`@kbn/zod-helpers`	-	9	+9
total			+7

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`securitySolution`	12.8MB	12.8MB	+13.8KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`securitySolution`	64.7KB	64.8KB	+94.0B

Unknown metric groups

API count

id	before	after	diff
`@kbn/securitysolution-es-utils`	89	87	-2
`@kbn/zod-helpers`	-	12	+12
total			+10

ESLint disabled line counts

id	before	after	diff
`securitySolution`	453	472	+19

Total ESLint disabled count

id	before	after	diff
`securitySolution`	521	540	+19

History

💔 Build #171554 failed 92851af1d9569ebc8e3d3b2d06a88a56662b29bf
💔 Build #171519 failed 2eb70ecb46fb8a5f664cea3f806831725c6a2488
💔 Build #171500 failed 1b9c918e4c5890e2015bd41535d52fbd65df762a
💔 Build #171249 failed dbcce09c105ccb744cec7bb9b14c5153d6b552a0
💚 Build #171164 succeeded 62e5aac237068f21eb496d1095df672b2898fa2e
💛 Build #170766 was flaky 5aa692f7953a4dfb1b6b44c2252937312fc307a8

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @xcrzx

xcrzx self-assigned this Oct 4, 2023

xcrzx force-pushed the rules-schema-openapi branch 16 times, most recently from 52d456e to 2ba0fc7 Compare October 10, 2023 06:53

xcrzx added Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detection Rule Management Security Detection Rule Management Team labels Oct 13, 2023

xcrzx force-pushed the rules-schema-openapi branch 7 times, most recently from 64cf59b to 5f80ffe Compare October 18, 2023 15:26

xcrzx added v8.12.0 release_note:skip Skip the PR/issue when compiling release notes labels Oct 19, 2023

xcrzx force-pushed the rules-schema-openapi branch from 5f80ffe to 1083511 Compare October 19, 2023 10:41

vitaliidm previously requested changes Oct 24, 2023

View reviewed changes

vitaliidm self-requested a review October 24, 2023 16:21

michaelolo24 approved these changes Oct 24, 2023

View reviewed changes

vitaliidm reviewed Oct 24, 2023

View reviewed changes

.../security_solution/common/api/detection_engine/model/rule_schema/rule_request_schema.test.ts Show resolved Hide resolved

vitaliidm self-requested a review October 24, 2023 16:48

vitaliidm reviewed Oct 24, 2023

View reviewed changes

x-pack/plugins/security_solution/common/detection_engine/transform_actions.ts Show resolved Hide resolved

...blic/detection_engine/rule_exceptions/components/flyout_components/item_conditions/index.tsx Show resolved Hide resolved

rylnd approved these changes Oct 24, 2023

View reviewed changes

xcrzx force-pushed the rules-schema-openapi branch 2 times, most recently from 062b2c8 to 5aa692f Compare October 25, 2023 11:36

xcrzx requested review from vitaliidm and banderror October 25, 2023 12:26

maximpn reviewed Oct 25, 2023

View reviewed changes

xcrzx force-pushed the rules-schema-openapi branch from 5aa692f to 62e5aac Compare October 26, 2023 12:18

banderror approved these changes Oct 26, 2023

View reviewed changes

xcrzx force-pushed the rules-schema-openapi branch from 62e5aac to dbcce09 Compare October 26, 2023 14:58

vitaliidm approved these changes Oct 26, 2023

View reviewed changes

xcrzx force-pushed the rules-schema-openapi branch 3 times, most recently from 2eb70ec to 92851af Compare October 27, 2023 13:13

Migrate Rules schema to OpenAPI

5c2609e

xcrzx force-pushed the rules-schema-openapi branch from 92851af to 5c2609e Compare October 27, 2023 14:29

xcrzx merged commit 3a18916 into elastic:main Oct 27, 2023

kibanamachine added the backport:skip This commit does not require backporting label Oct 27, 2023

vitaliidm mentioned this pull request Nov 9, 2023

[Security Solution] Discuss rule fields in the context of the prebuilt rule upgrade workflow #147239

Closed

vitaliidm mentioned this pull request Dec 1, 2023

[Security Solution][Detection Engine] adds alerts Suppression to threshold rule #171423

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution] Migrate Rules schema to OpenAPI #167999

[Security Solution] Migrate Rules schema to OpenAPI #167999

xcrzx commented Oct 4, 2023 •

edited

Loading

vitaliidm left a comment •

edited

Loading

michaelolo24 left a comment

rylnd left a comment

rylnd Oct 24, 2023

xcrzx Oct 25, 2023 •

edited

Loading

maximpn Oct 25, 2023 •

edited

Loading

xcrzx Oct 25, 2023

banderror left a comment

banderror Oct 26, 2023

banderror Oct 26, 2023

banderror Oct 26, 2023

banderror Oct 26, 2023

kibana-ci commented Oct 27, 2023

API count

ESLint disabled line counts

Total ESLint disabled count

[Security Solution] Migrate Rules schema to OpenAPI #167999

[Security Solution] Migrate Rules schema to OpenAPI #167999

Conversation

xcrzx commented Oct 4, 2023 • edited Loading

Summary

Rule schemas are now forward-compatible

vitaliidm left a comment • edited Loading

Choose a reason for hiding this comment

michaelolo24 left a comment

Choose a reason for hiding this comment

rylnd left a comment

Choose a reason for hiding this comment

rylnd Oct 24, 2023

Choose a reason for hiding this comment

xcrzx Oct 25, 2023 • edited Loading

Choose a reason for hiding this comment

maximpn Oct 25, 2023 • edited Loading

Choose a reason for hiding this comment

xcrzx Oct 25, 2023

Choose a reason for hiding this comment

banderror left a comment

Choose a reason for hiding this comment

banderror Oct 26, 2023

Choose a reason for hiding this comment

banderror Oct 26, 2023

Choose a reason for hiding this comment

banderror Oct 26, 2023

Choose a reason for hiding this comment

banderror Oct 26, 2023

Choose a reason for hiding this comment

kibana-ci commented Oct 27, 2023

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Page load bundle

API count

ESLint disabled line counts

Total ESLint disabled count

History

xcrzx commented Oct 4, 2023 •

edited

Loading

vitaliidm left a comment •

edited

Loading

xcrzx Oct 25, 2023 •

edited

Loading

maximpn Oct 25, 2023 •

edited

Loading