[Security Solution] Batch prebuilt rule installation#214441
[Security Solution] Batch prebuilt rule installation#214441xcrzx merged 1 commit intoelastic:mainfrom xcrzx:batch-rule-install
Conversation
|
Pinging @elastic/security-detections-response (Team:Detections and Resp) |
|
Pinging @elastic/security-detection-rule-management (Team:Detection Rule Management) |
|
Pinging @elastic/security-solution (Team: SecuritySolution) |
💚 Build Succeeded
Metrics [docs]
cc @xcrzx |
maximpn
left a comment
There was a problem hiding this comment.
@xcrzx Thanks for reducing memory consumption in prebuilt rules installation workflow 🙏
I tested the changes locally. Rules installation works as expected.
The actual fix is much smaller that the shown diff since you moved business logic to a separate function. I left nit comments to highlight improvement points.
| context: SecuritySolutionRequestHandlerContext, | ||
| request: KibanaRequest<unknown, unknown, PerformRuleInstallationRequestBody>, | ||
| response: KibanaResponseFactory | ||
| ) => { |
There was a problem hiding this comment.
nit: Specifying result type would be hight appreciated to reason on what's expected result. TS will help to make sure returned data matches expectations.
There was a problem hiding this comment.
Yes, that would help. The only problem is that this method can return errors along with a response, so the type there would be a bit bulky. I think the best way is to refactor our route handlers a bit and extract error handler and request wrapping into separate wrappers to leave handler implementations clean. This is related to your other comment. We can find some time to clean up route implementations, maybe during On week, to address this.
| const currentRuleVersions = await ruleObjectsClient.fetchInstalledRuleVersions(); | ||
| const currentRuleVersionsMap = new Map( | ||
| currentRuleVersions.map((version) => [version.rule_id, version]) | ||
| ); |
There was a problem hiding this comment.
nit: This code chunk is repeated 3 times (here, in prebuilt_rules/status and in reviewRuleInstallationHandler()).
It looks beneficial to have ruleObjectsClient.fetchInstalledRuleVersions() returning Map<RuleSignatureId, RuleVersion>.
| const allInstallableRules = allLatestVersions.filter((latestVersion) => { | ||
| const currentVersion = currentRuleVersionsMap.get(latestVersion.rule_id); | ||
| return !currentVersion; | ||
| }); |
There was a problem hiding this comment.
nit: installableRules calculation repeats in 3 different endpoint handlers as well. Should we have a separate method to return installableRules?
| if (mode === 'SPECIFIC_RULES') { | ||
| const installableRuleIds = new Set(allInstallableRules.map((rule) => rule.rule_id)); | ||
| request.body.rules.forEach((rule) => { | ||
| // Check that the requested rule is not installed yet | ||
| if (currentRuleVersionsMap.has(rule.rule_id)) { | ||
| skippedRules.push({ | ||
| rule_id: rule.rule_id, | ||
| reason: SkipRuleInstallReason.ALREADY_INSTALLED, | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| // Check that the requested rule is installable | ||
| if (!installableRuleIds.has(rule.rule_id)) { | ||
| ruleErrors.push({ | ||
| error: new Error( | ||
| `Rule with ID "${rule.rule_id}" and version "${rule.version}" not found` | ||
| ), | ||
| item: rule, | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| ruleInstallQueue.push(rule); | ||
| }); | ||
| } else if (mode === 'ALL_RULES') { | ||
| ruleInstallQueue.push(...allInstallableRules); | ||
| } |
There was a problem hiding this comment.
nit: It looks like this code chunk could be moved to a separate function returning ruleInstallQueue.
| while (ruleInstallQueue.length > 0) { | ||
| const rulesToInstall = ruleInstallQueue.splice(0, BATCH_SIZE); |
There was a problem hiding this comment.
nit: I just gave it a second thought. We could use lodash chunk to make it even simpler
for (const rulesToInstall of chunk(ruleInstallQueue, BATCH_SIZE)) {
...
}
WDYT?
| } catch (err) { | ||
| const error = transformError(err); | ||
| return siemResponse.error({ | ||
| body: error.message, | ||
| statusCode: error.statusCode, | ||
| }); | ||
| } |
There was a problem hiding this comment.
nit: Catch handler is usually identical for multiple API endpoint handlers. Maybe we could use a hight-order function with that standard logic to even further simplify handlers.
|
Starting backport for target branches: 8.18, 8.x, 9.0 https://github.com/elastic/kibana/actions/runs/13857835391 |
**This is a follow-up to:elastic#211045 ## Summary This PR removes inefficiencies in prebuilt rule installation memory consumption. ### Before In the worst-case scenario: 1. All currently installed prebuilt rules were fully loaded into memory. 2. All latest rule versions from the rule packages were fully loaded into memory. 3. All base rule versions were pulled into memory, even though they were not used. 4. The algorithm then checked which rules could be installed and installed them all at once. ### After 1. Pull only aggregated information about installed rules (only `rule_id` and `versions`). 2. Pull the same lightweight aggregated info about the latest rules in the package. 3. Using the collected `rule_id`s, fetch rule assets and install them in small batches of up to 100 rules. (cherry picked from commit 6d9fc21)
**This is a follow-up to:elastic#211045 ## Summary This PR removes inefficiencies in prebuilt rule installation memory consumption. ### Before In the worst-case scenario: 1. All currently installed prebuilt rules were fully loaded into memory. 2. All latest rule versions from the rule packages were fully loaded into memory. 3. All base rule versions were pulled into memory, even though they were not used. 4. The algorithm then checked which rules could be installed and installed them all at once. ### After 1. Pull only aggregated information about installed rules (only `rule_id` and `versions`). 2. Pull the same lightweight aggregated info about the latest rules in the package. 3. Using the collected `rule_id`s, fetch rule assets and install them in small batches of up to 100 rules. (cherry picked from commit 6d9fc21)
**This is a follow-up to:elastic#211045 ## Summary This PR removes inefficiencies in prebuilt rule installation memory consumption. ### Before In the worst-case scenario: 1. All currently installed prebuilt rules were fully loaded into memory. 2. All latest rule versions from the rule packages were fully loaded into memory. 3. All base rule versions were pulled into memory, even though they were not used. 4. The algorithm then checked which rules could be installed and installed them all at once. ### After 1. Pull only aggregated information about installed rules (only `rule_id` and `versions`). 2. Pull the same lightweight aggregated info about the latest rules in the package. 3. Using the collected `rule_id`s, fetch rule assets and install them in small batches of up to 100 rules. (cherry picked from commit 6d9fc21)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
…214578) # Backport This will backport the following commits from `main` to `9.0`: - [[Security Solution] Batch prebuilt rule installation (#214441)](#214441) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Dmitrii Shevchenko","email":"dmitrii.shevchenko@elastic.co"},"sourceCommit":{"committedDate":"2025-03-14T13:39:32Z","message":"[Security Solution] Batch prebuilt rule installation (#214441)\n\n**This is a follow-up to:https://github.com/elastic/kibana/pull/211045**\n\n## Summary \n\nThis PR removes inefficiencies in prebuilt rule installation memory\nconsumption.\n\n### Before\n\nIn the worst-case scenario: \n\n1. All currently installed prebuilt rules were fully loaded into memory.\n2. All latest rule versions from the rule packages were fully loaded\ninto memory.\n3. All base rule versions were pulled into memory, even though they were\nnot used.\n4. The algorithm then checked which rules could be installed and\ninstalled them all at once.\n\n### After\n\n1. Pull only aggregated information about installed rules (only\n`rule_id` and `versions`).\n2. Pull the same lightweight aggregated info about the latest rules in\nthe package.\n3. Using the collected `rule_id`s, fetch rule assets and install them in\nsmall batches of up to 100 rules.","sha":"6d9fc21db92dd5e02d930971a5a1cf32a97386f0","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","performance","release_note:skip","v9.0.0","Team:Detections and Resp","Team: SecuritySolution","Team:Detection Rule Management","Feature:Prebuilt Detection Rules","backport:version","v8.18.0","v9.1.0","v8.19.0"],"title":"[Security Solution] Batch prebuilt rule installation","number":214441,"url":"https://github.com/elastic/kibana/pull/214441","mergeCommit":{"message":"[Security Solution] Batch prebuilt rule installation (#214441)\n\n**This is a follow-up to:https://github.com/elastic/kibana/pull/211045**\n\n## Summary \n\nThis PR removes inefficiencies in prebuilt rule installation memory\nconsumption.\n\n### Before\n\nIn the worst-case scenario: \n\n1. All currently installed prebuilt rules were fully loaded into memory.\n2. All latest rule versions from the rule packages were fully loaded\ninto memory.\n3. All base rule versions were pulled into memory, even though they were\nnot used.\n4. The algorithm then checked which rules could be installed and\ninstalled them all at once.\n\n### After\n\n1. Pull only aggregated information about installed rules (only\n`rule_id` and `versions`).\n2. Pull the same lightweight aggregated info about the latest rules in\nthe package.\n3. Using the collected `rule_id`s, fetch rule assets and install them in\nsmall batches of up to 100 rules.","sha":"6d9fc21db92dd5e02d930971a5a1cf32a97386f0"}},"sourceBranch":"main","suggestedTargetBranches":["9.0","8.18","8.x"],"targetPullRequestStates":[{"branch":"9.0","label":"v9.0.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.18","label":"v8.18.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/214441","number":214441,"mergeCommit":{"message":"[Security Solution] Batch prebuilt rule installation (#214441)\n\n**This is a follow-up to:https://github.com/elastic/kibana/pull/211045**\n\n## Summary \n\nThis PR removes inefficiencies in prebuilt rule installation memory\nconsumption.\n\n### Before\n\nIn the worst-case scenario: \n\n1. All currently installed prebuilt rules were fully loaded into memory.\n2. All latest rule versions from the rule packages were fully loaded\ninto memory.\n3. All base rule versions were pulled into memory, even though they were\nnot used.\n4. The algorithm then checked which rules could be installed and\ninstalled them all at once.\n\n### After\n\n1. Pull only aggregated information about installed rules (only\n`rule_id` and `versions`).\n2. Pull the same lightweight aggregated info about the latest rules in\nthe package.\n3. Using the collected `rule_id`s, fetch rule assets and install them in\nsmall batches of up to 100 rules.","sha":"6d9fc21db92dd5e02d930971a5a1cf32a97386f0"}},{"branch":"8.x","label":"v8.19.0","branchLabelMappingKey":"^v8.19.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Dmitrii Shevchenko <dmitrii.shevchenko@elastic.co>
…214576) # Backport This will backport the following commits from `main` to `8.x`: - [[Security Solution] Batch prebuilt rule installation (#214441)](#214441) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Dmitrii Shevchenko","email":"dmitrii.shevchenko@elastic.co"},"sourceCommit":{"committedDate":"2025-03-14T13:39:32Z","message":"[Security Solution] Batch prebuilt rule installation (#214441)\n\n**This is a follow-up to:https://github.com/elastic/kibana/pull/211045**\n\n## Summary \n\nThis PR removes inefficiencies in prebuilt rule installation memory\nconsumption.\n\n### Before\n\nIn the worst-case scenario: \n\n1. All currently installed prebuilt rules were fully loaded into memory.\n2. All latest rule versions from the rule packages were fully loaded\ninto memory.\n3. All base rule versions were pulled into memory, even though they were\nnot used.\n4. The algorithm then checked which rules could be installed and\ninstalled them all at once.\n\n### After\n\n1. Pull only aggregated information about installed rules (only\n`rule_id` and `versions`).\n2. Pull the same lightweight aggregated info about the latest rules in\nthe package.\n3. Using the collected `rule_id`s, fetch rule assets and install them in\nsmall batches of up to 100 rules.","sha":"6d9fc21db92dd5e02d930971a5a1cf32a97386f0","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","performance","release_note:skip","v9.0.0","Team:Detections and Resp","Team: SecuritySolution","Team:Detection Rule Management","Feature:Prebuilt Detection Rules","backport:version","v8.18.0","v9.1.0","v8.19.0"],"title":"[Security Solution] Batch prebuilt rule installation","number":214441,"url":"https://github.com/elastic/kibana/pull/214441","mergeCommit":{"message":"[Security Solution] Batch prebuilt rule installation (#214441)\n\n**This is a follow-up to:https://github.com/elastic/kibana/pull/211045**\n\n## Summary \n\nThis PR removes inefficiencies in prebuilt rule installation memory\nconsumption.\n\n### Before\n\nIn the worst-case scenario: \n\n1. All currently installed prebuilt rules were fully loaded into memory.\n2. All latest rule versions from the rule packages were fully loaded\ninto memory.\n3. All base rule versions were pulled into memory, even though they were\nnot used.\n4. The algorithm then checked which rules could be installed and\ninstalled them all at once.\n\n### After\n\n1. Pull only aggregated information about installed rules (only\n`rule_id` and `versions`).\n2. Pull the same lightweight aggregated info about the latest rules in\nthe package.\n3. Using the collected `rule_id`s, fetch rule assets and install them in\nsmall batches of up to 100 rules.","sha":"6d9fc21db92dd5e02d930971a5a1cf32a97386f0"}},"sourceBranch":"main","suggestedTargetBranches":["9.0","8.18","8.x"],"targetPullRequestStates":[{"branch":"9.0","label":"v9.0.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.18","label":"v8.18.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/214441","number":214441,"mergeCommit":{"message":"[Security Solution] Batch prebuilt rule installation (#214441)\n\n**This is a follow-up to:https://github.com/elastic/kibana/pull/211045**\n\n## Summary \n\nThis PR removes inefficiencies in prebuilt rule installation memory\nconsumption.\n\n### Before\n\nIn the worst-case scenario: \n\n1. All currently installed prebuilt rules were fully loaded into memory.\n2. All latest rule versions from the rule packages were fully loaded\ninto memory.\n3. All base rule versions were pulled into memory, even though they were\nnot used.\n4. The algorithm then checked which rules could be installed and\ninstalled them all at once.\n\n### After\n\n1. Pull only aggregated information about installed rules (only\n`rule_id` and `versions`).\n2. Pull the same lightweight aggregated info about the latest rules in\nthe package.\n3. Using the collected `rule_id`s, fetch rule assets and install them in\nsmall batches of up to 100 rules.","sha":"6d9fc21db92dd5e02d930971a5a1cf32a97386f0"}},{"branch":"8.x","label":"v8.19.0","branchLabelMappingKey":"^v8.19.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Dmitrii Shevchenko <dmitrii.shevchenko@elastic.co>
…#214575) # Backport This will backport the following commits from `main` to `8.18`: - [[Security Solution] Batch prebuilt rule installation (#214441)](#214441) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Dmitrii Shevchenko","email":"dmitrii.shevchenko@elastic.co"},"sourceCommit":{"committedDate":"2025-03-14T13:39:32Z","message":"[Security Solution] Batch prebuilt rule installation (#214441)\n\n**This is a follow-up to:https://github.com/elastic/kibana/pull/211045**\n\n## Summary \n\nThis PR removes inefficiencies in prebuilt rule installation memory\nconsumption.\n\n### Before\n\nIn the worst-case scenario: \n\n1. All currently installed prebuilt rules were fully loaded into memory.\n2. All latest rule versions from the rule packages were fully loaded\ninto memory.\n3. All base rule versions were pulled into memory, even though they were\nnot used.\n4. The algorithm then checked which rules could be installed and\ninstalled them all at once.\n\n### After\n\n1. Pull only aggregated information about installed rules (only\n`rule_id` and `versions`).\n2. Pull the same lightweight aggregated info about the latest rules in\nthe package.\n3. Using the collected `rule_id`s, fetch rule assets and install them in\nsmall batches of up to 100 rules.","sha":"6d9fc21db92dd5e02d930971a5a1cf32a97386f0","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","performance","release_note:skip","v9.0.0","Team:Detections and Resp","Team: SecuritySolution","Team:Detection Rule Management","Feature:Prebuilt Detection Rules","backport:version","v8.18.0","v9.1.0","v8.19.0"],"title":"[Security Solution] Batch prebuilt rule installation","number":214441,"url":"https://github.com/elastic/kibana/pull/214441","mergeCommit":{"message":"[Security Solution] Batch prebuilt rule installation (#214441)\n\n**This is a follow-up to:https://github.com/elastic/kibana/pull/211045**\n\n## Summary \n\nThis PR removes inefficiencies in prebuilt rule installation memory\nconsumption.\n\n### Before\n\nIn the worst-case scenario: \n\n1. All currently installed prebuilt rules were fully loaded into memory.\n2. All latest rule versions from the rule packages were fully loaded\ninto memory.\n3. All base rule versions were pulled into memory, even though they were\nnot used.\n4. The algorithm then checked which rules could be installed and\ninstalled them all at once.\n\n### After\n\n1. Pull only aggregated information about installed rules (only\n`rule_id` and `versions`).\n2. Pull the same lightweight aggregated info about the latest rules in\nthe package.\n3. Using the collected `rule_id`s, fetch rule assets and install them in\nsmall batches of up to 100 rules.","sha":"6d9fc21db92dd5e02d930971a5a1cf32a97386f0"}},"sourceBranch":"main","suggestedTargetBranches":["9.0","8.18","8.x"],"targetPullRequestStates":[{"branch":"9.0","label":"v9.0.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.18","label":"v8.18.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/214441","number":214441,"mergeCommit":{"message":"[Security Solution] Batch prebuilt rule installation (#214441)\n\n**This is a follow-up to:https://github.com/elastic/kibana/pull/211045**\n\n## Summary \n\nThis PR removes inefficiencies in prebuilt rule installation memory\nconsumption.\n\n### Before\n\nIn the worst-case scenario: \n\n1. All currently installed prebuilt rules were fully loaded into memory.\n2. All latest rule versions from the rule packages were fully loaded\ninto memory.\n3. All base rule versions were pulled into memory, even though they were\nnot used.\n4. The algorithm then checked which rules could be installed and\ninstalled them all at once.\n\n### After\n\n1. Pull only aggregated information about installed rules (only\n`rule_id` and `versions`).\n2. Pull the same lightweight aggregated info about the latest rules in\nthe package.\n3. Using the collected `rule_id`s, fetch rule assets and install them in\nsmall batches of up to 100 rules.","sha":"6d9fc21db92dd5e02d930971a5a1cf32a97386f0"}},{"branch":"8.x","label":"v8.19.0","branchLabelMappingKey":"^v8.19.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Dmitrii Shevchenko <dmitrii.shevchenko@elastic.co>
**This is a follow-up to:elastic#211045 ## Summary This PR removes inefficiencies in prebuilt rule installation memory consumption. ### Before In the worst-case scenario: 1. All currently installed prebuilt rules were fully loaded into memory. 2. All latest rule versions from the rule packages were fully loaded into memory. 3. All base rule versions were pulled into memory, even though they were not used. 4. The algorithm then checked which rules could be installed and installed them all at once. ### After 1. Pull only aggregated information about installed rules (only `rule_id` and `versions`). 2. Pull the same lightweight aggregated info about the latest rules in the package. 3. Using the collected `rule_id`s, fetch rule assets and install them in small batches of up to 100 rules.
**This is a follow-up to:elastic#211045 ## Summary This PR removes inefficiencies in prebuilt rule installation memory consumption. ### Before In the worst-case scenario: 1. All currently installed prebuilt rules were fully loaded into memory. 2. All latest rule versions from the rule packages were fully loaded into memory. 3. All base rule versions were pulled into memory, even though they were not used. 4. The algorithm then checked which rules could be installed and installed them all at once. ### After 1. Pull only aggregated information about installed rules (only `rule_id` and `versions`). 2. Pull the same lightweight aggregated info about the latest rules in the package. 3. Using the collected `rule_id`s, fetch rule assets and install them in small batches of up to 100 rules.
This is a follow-up to:#211045
Summary
This PR removes inefficiencies in prebuilt rule installation memory consumption.
Before
In the worst-case scenario:
After
rule_idandversions).rule_ids, fetch rule assets and install them in small batches of up to 100 rules.