[Detections][Discuss] Detection engine tech debt/refactor opportunities

[marshallmain]

Summary

Now with 6 different rule types in the detection engine we've accumulated some tech debt. Below are areas that I think we can refactor to reduce maintenance burden and make the detection engine easier to reason about.

High level goals:

• Reduce data structure duplication by moving towards a single source of truth for schemas

  • We have 3 different schemas that each redefine parts of the rule schema and therefore must remain in sync - signalSchema, RuleTypeParams, and RulesSchema. A single schema should represent rules and should be used as the basis for derivative schemas (for example, the rule APIs return a flattened version of the rule in the response - this flattened schema can be defined in terms of the source of truth so we avoid duplication)
  • When validating rule params in the executor, we use signalSchema for validation, but then use the params as type RuleTypeParams which is defined independently. Some fields are defined with schema.nullable in signalSchema, but the corresponding field in RuleTypeParams is possibly undefined not null. This can lead to some unexpected behavior with null values showing up in rules and eventually in signals created by those rules.
  • Most or all of the rule params schema is copied in 30+ places as parameters for various routes and functions, e.g. create/update/patch rule. This makes it tedious and error prone to add new fields to the rule schema. Additionally, it's not always obvious when certain fields are purposely excluded from the list of parameters. In general I propose defining the parameters by using Pick or Omit on the source of truth so it's easy to update and clear when fields are included/omitted.
  • Rules are destructured and passed as a long list of parameters into functions such as searchAfterAndBulkCreate, among others. Often many of these parameters are then passed through to other functions inside. This makes it hard to add new fields to rules since every function definition on the call stack has to be modified to pass each field through. Passing the entire rule through as a single unit makes it easier to add new fields, and IMO easier to reason about where each parameter comes from.

• Establish clear divisions between rule types by creating separate schemas for each rule type, merged (via union) into the single source of truth

  • Each rule type could have its own params schema, e.g. KQLRuleParams, EQLRuleParams, ThreatRuleParams, etc. The overall rule params schema then becomes KQLRuleParams | EQLRuleParams | ThreatRuleParams | ....
  • This would eliminate the need for extra functions that do validation that depends on the type of the rule
  • Currently, the general design is one large params schema that covers all rule types. Fields that are specific to some subset of rule types are optional in the complete schema, even if they are required for the rule type (e.g. threshold is required for threshold rules, therefore it is optional in the complete rule schema). This means we have a number of functions that do some additional validation based on the type when creating/updating rules, ensuring that these optional fields are there when required. However, these functions have to be maintained in sync with the schema and add complexity which is hard to reason about.

I propose that the single source of truth is the rule SO data structure, which in turn is represented using a union of the individual rule types. The schemas for the various create/update/patch requests would be defined separately and should be the few places where we see some schema duplication due to the need for setting defaults for certain fields in the routes. However, unit tests can verify that the output from decoding with the create/update/patch schemas is a valid rule by following up with a decode using the source of truth rule schema. This ensures that the route schemas stay in sync with the source of truth.

Prior refactoring art

• https://github.com/elastic/kibana/pull/82462 (on the API schemas side)
• https://github.com/elastic/kibana/pull/96186 (on the BE schemas side)

[elasticmachine]

Pinging @elastic/siem (Team:SIEM)

[peluja1012]

Related PR: https://github.com/elastic/kibana/pull/82462

[marshallmain]

Remaining work on this issue is to remove the legacy RulesSchema (kibana/x-pack/plugins/security_solution/common/detection_engine/schemas/response/rules_schema.ts) in favor of the new schemas introduced in https://github.com/elastic/kibana/pull/82462.