[Security Solution] Remove references to ruleRegistryEnabled feature flag #128913

madirey · 2022-03-30T14:41:01Z

Summary

Since rule_registry is now enabled by default, legacy rule executors have now been disabled. This removes all references to the feature flag in areas where legacy code was running or legacy alerts were written.

Fixes #128918

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

elasticmachine · 2022-03-30T14:41:04Z

Pinging @elastic/security-detections-response (Team:Detections and Resp)

madirey · 2022-04-14T19:01:35Z

@elasticmachine merge upstream

…cleanup

cnasikas

Cases code LTGM

madirey · 2022-04-20T15:05:22Z

@elasticmachine merge upstream

spong · 2022-04-20T18:27:23Z

...ugins/security_solution/server/lib/detection_engine/routes/rules/utils/import_rules_utils.ts

@@ -190,7 +181,7 @@ export const importRules = async ({
                    language,
                    license,
                    machineLearningJobId,
-                    outputIndex: signalsIndex,
+                    outputIndex: '',


This field is deprecated, no? We just need to remove it at some point, right?

@spong Yep, good point. We left it because the feature flag was still there and so it may have still been needed. I can follow up to remove it!

spong · 2022-04-20T18:32:59Z

...ns/security_solution/server/lib/detection_engine/routes/rules/add_prepackaged_rules_route.ts

-      throw new PrepackagedRulesError(
-        `Pre-packaged rules cannot be installed until the signals index is created: ${signalsIndex}`,
-        400
-      );


Is this error still possible even with the RuleRegistry? Like if you create a new space, but don't go to the security app, and then hit this API directly the indices won't have been created yet, no? RuleRegistry index creation happens on Kibana startup for existing spaces, but new spaces require an alert to be generated, is that correct?

@spong I'll have to test this scenario, but you may be right. I think I'll follow up with an additional PR/issue to address this and the output_index stuff.

spong

Did a quick scan over code changes and LGTM! Couple questions around test changes, but everything else looks good -- thanks for all the cleanup here @madirey! 🙂 🙌 🚀

madirey · 2022-04-20T18:48:10Z

@elasticmachine merge upstream

…cleanup

michaelolo24

Thanks for removing the flags! LGTM

kibana-ci · 2022-04-25T20:08:45Z

💛 Build succeeded, but was flaky

Test Failures

[job] [logs] Security Solution Tests / Alerts timeline Privileges: can crud should allow a user with crud privileges to attach alerts to cases

Metrics [docs]

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.8MB	4.8MB	-107.0B

Page load bundle

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

id	before	after	diff
`securitySolution`	248.0KB	248.0KB	-23.0B

History

💔 Build #40278 failed d18c331
💚 Build #39820 succeeded d7a6234
💔 Build #39574 failed 9185f06
💔 Build #39488 failed 756b17d
💚 Build #38672 succeeded fcd8469

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

…flag (elastic#128913) * Remove references to ruleRegistryEnabled feature flag * Fix remaining tests using describe.each * Test fixes * alert -> rule * Fix import rule tests * Fix output_index in tests * Tryin' again * Another test fix Co-authored-by: Kibana Machine <[email protected]>

Remove references to ruleRegistryEnabled feature flag

e0b160f

madirey added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting Team:Detections and Resp Security Detection Response Team Feature:Detection Alerts Security Solution Detection Alerts Feature 8.3 candidate v8.3.0 labels Mar 30, 2022

madirey requested review from a team as code owners March 30, 2022 14:41

madirey added 11 commits March 30, 2022 10:49

Fix remaining tests using describe.each

9f0c52d

Merge branch 'main' of github.com:elastic/kibana into rr-ff-cleanup

67d4267

Merge branch 'main' of github.com:elastic/kibana into rr-ff-cleanup

c26c19b

Test fixes

6fd569c

alert -> rule

f63ad23

Fix import rule tests

27d03dc

Merge branch 'main' of github.com:elastic/kibana into rr-ff-cleanup

0d77524

Fix output_index in tests

fb479ee

Tryin' again

6dc7556

Merge branch 'main' of github.com:elastic/kibana into rr-ff-cleanup

ed3382c

Another test fix

55efe9b

marshallmain approved these changes Apr 12, 2022

View reviewed changes

Merge branch 'main' of github.com:elastic/kibana into rr-ff-cleanup

aaeef0d

MadameSheema approved these changes Apr 12, 2022

View reviewed changes

Merge branch 'main' into rr-ff-cleanup

5536b5f

madirey requested review from a team as code owners April 16, 2022 06:39

madirey added 2 commits April 18, 2022 10:13

Merge branch 'main' of github.com:elastic/kibana into rr-ff-cleanup

e228c68

Merge branch 'rr-ff-cleanup' of github.com:madirey/kibana into rr-ff-…

fcd8469

…cleanup

cnasikas approved these changes Apr 19, 2022

View reviewed changes

marshallmain approved these changes Apr 19, 2022

View reviewed changes

Merge branch 'main' into rr-ff-cleanup

756b17d

madirey enabled auto-merge (squash) April 20, 2022 15:36

spong reviewed Apr 20, 2022

View reviewed changes

spong approved these changes Apr 20, 2022

View reviewed changes

kibanamachine and others added 4 commits April 20, 2022 13:48

Merge branch 'main' into rr-ff-cleanup

9185f06

Merge branch 'main' of github.com:elastic/kibana into rr-ff-cleanup

acb9b61

Merge branch 'rr-ff-cleanup' of github.com:madirey/kibana into rr-ff-…

d7a6234

…cleanup

Merge branch 'main' of github.com:elastic/kibana into rr-ff-cleanup

d18c331

YulNaumenko self-requested a review April 25, 2022 17:20

michaelolo24 approved these changes Apr 25, 2022

View reviewed changes

madirey added 2 commits April 25, 2022 13:53

Another fix from conflict

f9714c6

Merge branch 'main' of github.com:elastic/kibana into rr-ff-cleanup

611e53a

madirey removed the request for review from YulNaumenko April 25, 2022 17:54

madirey merged commit 414ad78 into elastic:main Apr 25, 2022

madirey deleted the rr-ff-cleanup branch April 26, 2022 01:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution] Remove references to ruleRegistryEnabled feature flag #128913

[Security Solution] Remove references to ruleRegistryEnabled feature flag #128913

madirey commented Mar 30, 2022 •

edited

Loading

elasticmachine commented Mar 30, 2022

madirey commented Apr 14, 2022

cnasikas left a comment

madirey commented Apr 20, 2022

spong Apr 20, 2022

madirey Apr 20, 2022

spong Apr 20, 2022

madirey Apr 20, 2022

spong left a comment

madirey commented Apr 20, 2022

michaelolo24 left a comment

kibana-ci commented Apr 25, 2022

[Security Solution] Remove references to ruleRegistryEnabled feature flag #128913

[Security Solution] Remove references to ruleRegistryEnabled feature flag #128913

Conversation

madirey commented Mar 30, 2022 • edited Loading

Summary

Risk Matrix

For maintainers

elasticmachine commented Mar 30, 2022

madirey commented Apr 14, 2022

cnasikas left a comment

Choose a reason for hiding this comment

madirey commented Apr 20, 2022

spong Apr 20, 2022

Choose a reason for hiding this comment

madirey Apr 20, 2022

Choose a reason for hiding this comment

spong Apr 20, 2022

Choose a reason for hiding this comment

madirey Apr 20, 2022

Choose a reason for hiding this comment

spong left a comment

Choose a reason for hiding this comment

madirey commented Apr 20, 2022

michaelolo24 left a comment

Choose a reason for hiding this comment

kibana-ci commented Apr 25, 2022

💛 Build succeeded, but was flaky

Test Failures

Metrics [docs]

Async chunks

Page load bundle

History

madirey commented Mar 30, 2022 •

edited

Loading