[RAC][Security Solution] Adds Threshold rule type and removes reliance on outputIndex #111437

madirey · 2021-09-07T17:08:14Z

Summary

Creates a working Threshold rule using new RAC framework
Sorts threshold search groups by cardinality, fixing [Security Solution][Detections][Threshold Rules] Filtering by Cardinality may miss alerts when bucket count is high #95258
Updates threshold rule to use persisted rule state rather than querying for past alerts
Handles upgrades to prevent a flurry of duplicate threshold signals

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space.	Low	High	Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks.	High	Low	Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled.	Medium	High	Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

This was checked for breaking API changes and was labeled appropriately

…-type-threshold

elasticmachine · 2021-09-09T15:48:45Z

Pinging @elastic/security-solution (Team: SecuritySolution)

…-type-threshold

marshallmain

Looks like the ruleTypeId for the new threshold rules needs to be updated from siem.signals to siem.thresholdRule in the ruleTypeMappings. But since this is feature flagged and there will be major follow ups to this I don't think we need to hold up merging this PR.

marshallmain · 2021-09-14T06:25:14Z

...urity_solution/server/lib/detection_engine/rule_types/factories/utils/flatten_with_prefix.ts

 export const flattenWithPrefix = (
  prefix: string,
-  obj: Record<string, SearchTypes>
+  maybeObj: unknown
 ): Record<string, SearchTypes> => {
-  return Object.keys(obj).reduce((acc: Record<string, SearchTypes>, key) => {
+  if (maybeObj != null && isPlainObject(maybeObj)) {
+    return Object.keys(maybeObj as Record<string, SearchTypes>).reduce(
+      (acc: Record<string, SearchTypes>, key) => {
+        return {
+          ...acc,
+          ...flattenWithPrefix(`${prefix}.${key}`, (maybeObj as Record<string, SearchTypes>)[key]),
+        };
+      },
+      {}
+    );
+  } else {
    return {
-      ...acc,
-      [`${prefix}.${key}`]: obj[key],
+      [prefix]: maybeObj as SearchTypes,
    };
-  }, {});
+  }


Since this function is used in both the real alert generation and the alert generation tests, it would be nice to test this function explicitly - especially now that it's not as trivial to see that it works.

marshallmain · 2021-09-14T06:34:04Z

When we're updating the API integration tests to work with the new rule implementations, we should also add additional API integration tests for all rule types around duplicate mitigation.

…-type-threshold

kibanamachine · 2021-09-14T18:41:26Z

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

💔 Build #153219 failed 9339cee
💔 Build #152972 failed 04a024d
💛 Build #152948 was flaky a352cbb
💔 Build #152866 failed 2082b3b
💚 Build #152705 succeeded 6057c94

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

…e on outputIndex (elastic#111437) * Initial commit * Properly handle signal history * Fix elastic#95258 - cardinality sort bug * Init threshold rule * Create working threshold rule * Fix threshold signal generation * Fix tests * Update mappings * ALERT_TYPE_ID => RULE_TYPE_ID * Add tests * Fix types

kibanamachine · 2021-09-16T18:44:03Z