[Security Solution] Switches remaining rule types to use new Rule Preview API by dplumlee · Pull Request #116374 · elastic/kibana

dplumlee · 2021-10-26T22:11:01Z

Summary

Overview

This PR switches the rule preview feature over to our new rule preview API that uses the actual rule executors instead of solely relying on a limited search strategy call over existing data. By using the actual executors, we are able to return real warnings and errors that occur in the execution process that previously couldn't be seen until the rule was created and actually ran. It also allows us to display a more accurate preview akin to what the real rule execution would show and gives us the ability to add more rule types and features to the preview functionality down the line.

Remaining tasks

Note

Cypress tests related to threshold preview were disabled in this PR because of the way we implement the es archive data. The current method for testing the preview functionality is hard coding the look back time for the rule to 50,000 hours so it can get the data (timestamped 2019) to show up. With the new updates added in this PR, we no longer have the ability to hard code a look back time. While this is planned to be added in a future PR, it's still not a great way to maintain the tests and so I've opened an issue around updating the data generation for these (and perhaps more) cypress tests that will align with unskipping those specific cypress tests

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

For maintainers

This was checked for breaking API changes and was labeled appropriately

ecezalp

I was testing this PR locally and I came across a critical issue (409 w/ version_conflict_engine_exception) - please let me know if you are not able to reproduce it.

create a test-index with @timestamp field with date mapping
post an event to test-index, with current timestamp and test_id: 123
create a custom query rule for test-index with test_id: * query
observe that the preview returns no results on the UI (expecting 1)
observe that inspecting matrix histogram query there are no results (expecting 1)
observe that once the rule is created 1 alert is generated as expected
observe via devtools that there is a preview event on the preview index as expected

this was the error was in the console

[2021-12-06T14:48:35.044-05:00][ERROR][plugins.ruleRegistry] ResponseError: {"took":0,"errors":true,"items":[{"create":{"_index":".internal.preview.alerts-security.alerts-default-000001","_id":"80c7e30e7572dc2fa9ef510d5c3747fafda0a10dec8f0c5afa7e03817840841a","status":409,"error":{"type":"version_conflict_engine_exception","reason":"[80c7e30e7572dc2fa9ef510d5c3747fafda0a10dec8f0c5afa7e03817840841a]: version conflict, document already exists (current version [1])","index_uuid":"sKKOrC7YSySyt16-FqFNQg","shard":"0","index":".internal.preview.alerts-security.alerts-default-000001"}}}]}
    at /Users/eceozalp/Workspace/kibana/x-pack/plugins/rule_registry/server/rule_data_client/rule_data_client.ts:194:31
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at alertWithPersistence (/Users/eceozalp/Workspace/kibana/x-pack/plugins/rule_registry/server/utils/create_persistence_rule_type_wrapper.ts:94:34)
    at /Users/eceozalp/Workspace/kibana/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/factories/bulk_create_factory.ts:47:31
    at searchAfterAndBulkCreate (/Users/eceozalp/Workspace/kibana/x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.ts:158:13)
    at Object.executor (/Users/eceozalp/Workspace/kibana/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/query/create_query_alert_type.ts:66:22)
    at Object.executor (/Users/eceozalp/Workspace/kibana/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_wrapper.ts:246:51)
    at executor (/Users/eceozalp/Workspace/kibana/x-pack/plugins/rule_registry/server/utils/create_persistence_rule_type_wrapper.ts:20:23)
    at lists (/Users/eceozalp/Workspace/kibana/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/preview_rules_route.ts:140:11)
    at /Users/eceozalp/Workspace/kibana/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/preview_rules_route.ts:178:11
    at Router.handle (/Users/eceozalp/Workspace/kibana/src/core/server/http/router/router.ts:275:30)
    at handler (/Users/eceozalp/Workspace/kibana/src/core/server/http/router/router.ts:230:13)
    at exports.Manager.execute (/Users/eceozalp/Workspace/kibana/node_modules/@hapi/hapi/lib/toolkit.js:60:28)
    at Object.internals.handler (/Users/eceozalp/Workspace/kibana/node_modules/@hapi/hapi/lib/handler.js:46:20)
    at exports.execute (/Users/eceozalp/Workspace/kibana/node_modules/@hapi/hapi/lib/handler.js:31:20)
    at Request._lifecycle (/Users/eceozalp/Workspace/kibana/node_modules/@hapi/hapi/lib/request.js:371:32)
    at Request._execute (/Users/eceozalp/Workspace/kibana/node_modules/@hapi/hapi/lib/request.js:281:9)

I deleted my preview index and started kibana again, and tried to preview the same event with a different new rule, and the problem persisted.

dplumlee · 2021-12-07T00:04:57Z

@ecezalp The error you're seeing in the console is a side effect of the new dedup logic we're using for the rule registry executors. It's not actually impacting the data we're getting back and displaying in the preview index/histogram. The real issue was the index having a bit of a race condition between refreshing and the read call the histogram is sending, the data was not always being indexed fully thus making it unable to be read by the histogram. In the latest commit, we add an await clause to refresh the alias + namespace before returning the initial preview call so that the UI should always be able to read the entire index correctly. As for the console error itself, @marshallmain has this PR that addresses logging the message instead of throwing the error, but from my understanding it's more of an indication that the dedup is working and not causing data to not be written properly

ecezalp · 2021-12-07T02:27:33Z

following the instructions from my previous comment, I am still seeing no preview results when trying to preview a custom query rule. preview index seems to get populated, but the matrix histogram query doesn't return anything. I have tested by pulling the latest changes & deleting the data directory & bootstrapping & following the steps shared above

…eview

kibana-ci · 2021-12-07T08:41:12Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`securitySolution`	2807	2798	-9

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
`ruleRegistry`	145	148	+3

Async chunks

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

id	before	after	diff
`securitySolution`	4.6MB	4.5MB	-9.6KB

Page load bundle

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

id	before	after	diff
`securitySolution`	243.0KB	242.7KB	-326.0B

Unknown metric groups

API count

id	before	after	diff
`ruleRegistry`	171	175	+4

History

💔 Build #11604 failed 5f62ed2
💔 Build #11595 failed 75fd94c
💚 Build #11490 succeeded a4e2aff
💚 Build #11108 succeeded 5b25b3f
💚 Build #10854 succeeded 5aaf91f

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

cc @dplumlee

madirey · 2021-12-07T20:22:02Z

x-pack/plugins/rule_registry/server/rule_data_client/rule_data_client.ts

    return this.options.indexInfo.kibanaVersion;
  }

+  public indexNameWithNamespace(namespace: string): string {


ecezalp · 2021-12-07T21:03:25Z

Removed the blocker as Davis pointed out that the bug identified is an existing one, and is logged here

ecezalp

LGTM, thank you for the hard work!

I was able to locally run the code and confirm that custom query rules, threshold rules, and EQL rules can be previewed as expected.

There is still some relevant work that needs to be completed, which we should do during the FF

fix matrix histogram bug
unskip tests
do not ignore 404
get product approval around what noisy is in a preview context

ecezalp · 2021-12-07T22:31:01Z

x-pack/plugins/security_solution/public/detections/components/rules/rule_preview/helpers.ts

 export const isNoisy = (hits: number, timeframe: Unit): boolean => {
  if (timeframe === 'h') {
-    return hits > 1;
+    return hits > 20;


was there a particular reason for this change? as far as I know, the noisiness criteria was 1 alert per hour for all scenarios

Ah maybe i misunderstood the code prior to then, I thought we were going based of the invocations themselves, we can switch that back and get a more solidified answer for noisiness in all rule scenarios

ecezalp · 2021-12-07T22:35:30Z

...ck/plugins/security_solution/server/lib/detection_engine/routes/rules/preview_rules_route.ts

        }

+        if (request.body.type === 'threat_match') {
+          return response.ok({ body: { errors: ['Preview for rule type not supported'] } });


ecezalp · 2021-12-07T22:40:31Z

...ck/plugins/security_solution/server/lib/detection_engine/routes/rules/preview_rules_route.ts

+          {
+            index: previewRuleDataClient.indexNameWithNamespace(spaceId),
+          },
+          { ignore: [404] }


hmm why are we ignoring 404 here? doesn't that mean that the preview index hasn't been properly created & functionality won't work?

At this point, the 404 error would show up if there were no alerts written to the index and we wouldn't have created it because no alerts were found with the preview query. I believe in the event of the index not being properly created, there are other error handlers that would throw still

so if the first preview ever doesn't have any results, you get a 404, which is then suppressed? that's sounds like an edge case. I think it's conceptually not very good to suppress errors like this in general, so maybe we should let the users see the error in that case.. we could ask product

We can definitely ask, but as I understand it, this is expected behavior as it wouldn't be an error from the user's perspective. It's more of an internal error dealing with the rule registry's design behavior of creating an index only if alerts exist. Users would get no data back and the resulting histogram would be empty which would be accurate from their POV and they need not know if the index was created or not. We can for sure discuss it further though

marshallmain

LGTM
Nice work

marshallmain · 2021-12-07T23:05:05Z

...gins/security_solution/public/detections/components/rules/rule_preview/use_preview_route.tsx

  threatMapping: ThreatMapping;
+  threshold: FieldValueThreshold;
+  machineLearningJobId: string[];
+  anomalyThreshold: number;


Mixing params from different rule types is something of an anti-pattern IMO.

From a UI perspective, these are always present since it's just the form state. They do get separated into their different rule types later. There's probably better ways to pass that through though

kibanamachine · 2021-12-07T23:27:08Z

The following labels were identified as gaps in your version labels and will be added automatically:

v8.1.0

If any of these should not be on your pull request, please manually remove them.

kibanamachine · 2021-12-07T23:29:29Z

💔 Backport failed

Status	Branch	Result
❌	8.0	Commit could not be cherrypicked due to conflicts

To backport manually run:
node scripts/backport --pr 116374

…view API (elastic#116374) # Conflicts: # x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/preview_rules_route.ts

…le Preview API (#116374) (#120700) * [Security Solution] Switches remaining rule types to use new Rule Preview API (#116374) # Conflicts: # x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/preview_rules_route.ts * fix conflicts Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

…view API (elastic#116374)

dplumlee self-assigned this Oct 26, 2021

dplumlee force-pushed the update-rule-type-preview branch from cfaff97 to c3441ec Compare October 26, 2021 22:13

dplumlee added the Team:Detections and Resp Security Detection Response Team label Oct 26, 2021

dplumlee force-pushed the update-rule-type-preview branch from 44971a8 to a624960 Compare October 28, 2021 19:15

dplumlee force-pushed the update-rule-type-preview branch 3 times, most recently from 0d47948 to 1656bdf Compare November 9, 2021 23:25

elastic deleted a comment from kibanamachine Nov 12, 2021

dplumlee mentioned this pull request Nov 15, 2021

Granting kibana_system reserved role access to "all" privileges to .preview.alerts* index elastic/elasticsearch#80746

Merged

dplumlee force-pushed the update-rule-type-preview branch 2 times, most recently from 4dbfc45 to dd39c1e Compare November 18, 2021 14:30

ywangd mentioned this pull request Nov 18, 2021

Grants kibana_system reserved role access to .preview.alerts* index (#80746) elastic/elasticsearch#80861

Merged

dplumlee mentioned this pull request Nov 19, 2021

Granting kibana_system reserved role access to "all" privileges to .internal.preview.alerts* index elastic/elasticsearch#80889

Merged

dplumlee force-pushed the update-rule-type-preview branch from dd39c1e to 7adb090 Compare November 22, 2021 20:07

dplumlee added 10 commits November 23, 2021 12:42

switches old preview to new api

70e0bdb

fixes types

5ef4af0

updates eql and query rule type previews

1965bb8

fixes types

6e28d0f

fixes types

963ed0d

adds query to preview

0798046

fixes text errors

dac95eb

updates react tests

e435c5d

adds integration tests

889c057

ready for es update

929cd9e

ecezalp self-requested a review December 6, 2021 19:53

ecezalp suggested changes Dec 6, 2021

View reviewed changes

ywangd mentioned this pull request Dec 6, 2021

Granting kibana_system reserved role access to "all" privileges to .internal.preview.alerts* index (#80889) elastic/elasticsearch#81392

Merged

adds ml rule preview and fixes refresh error

75fd94c

dplumlee requested a review from a team December 7, 2021 00:20

dplumlee added 2 commits December 6, 2021 19:27

updates jest tests for ML previews

a60e3c7

updates rule data client mock

5f62ed2

dplumlee added 2 commits December 7, 2021 00:16

Merge remote-tracking branch 'upstream/main' into update-rule-type-pr…

1505e01

…eview

fixes 404 error

0fa27da

madirey reviewed Dec 7, 2021

View reviewed changes

ecezalp self-requested a review December 7, 2021 21:02

ecezalp approved these changes Dec 7, 2021

View reviewed changes

marshallmain approved these changes Dec 7, 2021

View reviewed changes

dplumlee merged commit 5d44d79 into elastic:main Dec 7, 2021

kibanamachine added the v8.1.0 label Dec 7, 2021

dplumlee mentioned this pull request Dec 7, 2021

[8.0] [Security Solution] Switches remaining rule types to use new Rule Preview API (#116374) #120700

Merged

dplumlee deleted the update-rule-type-preview branch December 7, 2021 23:48

dplumlee mentioned this pull request Dec 14, 2021

[Security Solution] Rule Preview Follow-up #121249

Merged

4 tasks

TinLe pushed a commit to TinLe/kibana that referenced this pull request Dec 22, 2021

[Security Solution] Switches remaining rule types to use new Rule Pre…

246aee4

…view API (elastic#116374)

nastasha-solomon mentioned this pull request Aug 1, 2022

[BUG] Mention limited preview options for Threshold and Event Correlation rules elastic/security-docs#2238

Closed

Conversation

dplumlee commented Oct 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Overview

Note

Checklist

For maintainers

Uh oh!

ecezalp left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dplumlee commented Dec 7, 2021

Uh oh!

ecezalp commented Dec 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kibana-ci commented Dec 7, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Page load bundle

API count

History

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ecezalp commented Dec 7, 2021

Uh oh!

ecezalp left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ecezalp Dec 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marshallmain left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dplumlee Dec 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Dec 7, 2021

Uh oh!

kibanamachine commented Dec 7, 2021

💔 Backport failed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

dplumlee commented Oct 26, 2021 •

edited

Loading

ecezalp left a comment •

edited

Loading

ecezalp commented Dec 7, 2021 •

edited

Loading

ecezalp left a comment •

edited

Loading

ecezalp Dec 7, 2021 •

edited

Loading

dplumlee Dec 7, 2021 •

edited

Loading