-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Security Solution] PoC of the rule upgrade and installation workflows #144060
Conversation
829b248
to
65dd3ac
Compare
65dd3ac
to
0f95ae4
Compare
ffe26ab
to
311d70f
Compare
b868dec
to
6a31992
Compare
7ca12fe
to
d6f263f
Compare
d6f263f
to
ee5b361
Compare
b34bf23
to
98c5710
Compare
98c5710
to
0800399
Compare
💔 Build FailedFailed CI Steps
Test Failures
Metrics [docs]
History
To update your PR or re-run it, just comment with: cc @banderror |
@xcrzx I completely agree with your conclusions. Let's proceed with the flat data model and try to fix the issues on the Fleet side that are currently preventing us from picking the flat model. If it doesn't work out we can return back and use the composite model v2. I updated the Summary in the PR description with some notes and links. At this point, we consider this PoC as completed and I'm closing this PR. Follow-up implementation work will be done in #148392. |
… initial implementation (#148392) **Addresses:** #148181, #148182, #148185 **Partially addresses:** #148183, #148189 ## Summary Based on the [POC](#144060), this PR adds 4 endpoints for the new upgrade and installation workflows for prebuilt rules: - `GET /internal/detection_engine/prebuilt_rules/status` - `POST /internal/detection_engine/prebuilt_rules/upgrade/_review` - `POST /internal/detection_engine/prebuilt_rules/installation/_review` - `POST /internal/detection_engine/prebuilt_rules/_generate_assets` (temporary helper endpoint for development and testing) The new endpoints are hidden behind a feature flag and can be enabled by the following config setting: ```yaml xpack.securitySolution.enableExperimental: ['prebuiltRulesNewUpgradeAndInstallationWorkflowsEnabled'] ``` ## In the next episodes Will be done later in follow-up PRs: - Implementation of some additional response properties for the `upgrade/_review` endpoint: - #148183 - Making base versions optional for diff calculation (we need to support this in order to be able to still show diffs for rule assets coming from packages without historical versions): - #148189 - Further development of the diff algorithm: - #148191 - Test coverage: - #148192
… initial implementation (elastic#148392) **Addresses:** elastic#148181, elastic#148182, elastic#148185 **Partially addresses:** elastic#148183, elastic#148189 ## Summary Based on the [POC](elastic#144060), this PR adds 4 endpoints for the new upgrade and installation workflows for prebuilt rules: - `GET /internal/detection_engine/prebuilt_rules/status` - `POST /internal/detection_engine/prebuilt_rules/upgrade/_review` - `POST /internal/detection_engine/prebuilt_rules/installation/_review` - `POST /internal/detection_engine/prebuilt_rules/_generate_assets` (temporary helper endpoint for development and testing) The new endpoints are hidden behind a feature flag and can be enabled by the following config setting: ```yaml xpack.securitySolution.enableExperimental: ['prebuiltRulesNewUpgradeAndInstallationWorkflowsEnabled'] ``` ## In the next episodes Will be done later in follow-up PRs: - Implementation of some additional response properties for the `upgrade/_review` endpoint: - elastic#148183 - Making base versions optional for diff calculation (we need to support this in order to be able to still show diffs for rule assets coming from packages without historical versions): - elastic#148189 - Further development of the diff algorithm: - elastic#148191 - Test coverage: - elastic#148192
…tract migrating to Zod (#189790) Partially addresses (contract change only): #166376 Created in favour of: #189187 (closed) ## Summary - Extends contract as described in the [POC](#144060), migrating from `io-ts` to Zod (search for `Perform rule upgrade`) - Uses new types in endpoint, but functionality remains unchaged. ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
Fixes: #166376 (main ticket) Fixes: #186544 (handling of specific fields) Fixes: #180195 (replace PATCH with PUT logic on rule upgrade) ## Summary - Enhances the `/upgrade/_perform` endpoint to upgrade rules in a way that works with prebuilt rules customized by users and resolve conflicts between user customizations and updates from Elastic. - Handles special fields under the hood (see below) - Replaces the update prebuilt rule logic to work with PUT instead of PATCH. ### Rough implementation plan - For each `upgradeableRule`, we attempt to build the payload necessary to pass to `upgradePrebuiltRules()`, which is of type `PrebuiltRuleAsset`. So we retrieve the field names from `FIELDS_PAYLOAD_BY_RULE_TYPE` and loop through them. - If any of those `field`s are non-upgreadable, (i.e. its value needs to be handled under the hood) we do so in `determineFieldUpgradeStatus`. - Otherwise, we continue to build a `FieldUpgradeSpecifier` for each field, which will help us determine if that field needs to be set to the base, current, target version, OR if it needs to be calculated as a MERGED value, or it is passed in the request payload as a RESOLVED value. - Notice that we are iterating over "flat" (non-grouped) fields which are part of the `PrebuiltRuleAsset` schema. This means that mapping is necessary between these flat fields and the diffable (grouped) fields that are used in the API contract, part of `DiffableRule`. For example, if we try to determine the value for the `query` field, we will need to look up for its value in the `eql_query` field if the target rule is `eql` or in `esql_query` if the target rule is `esql`. All these mappings can be found in `diffable_rule_fields_mappings.ts`. - Once a `FieldUpgradeSpecifier` has been retrieved for each field of the payload we are building, retrieve its actual value: either fetching it from the base, current or target versions of the rule, from the three way diff calculation, or retrieving it from the request payload if it resolved. - Do this for all upgreadable rules, and the pass the payload array into `upgradePrebuiltRules()`. - **IMPORTANT:** The upgrade prebuilt rules logic has been changed from PATCH to PUT. That means that if the next version of a rule removes a field, and the user updates to that target version, those fields will be undefined in the resulting rule. **Additional example:** a installs a rule, and creates a `timeline_id` for it rule by modifying it. If neither the next version (target version) still does not have a `timeline_id` field for it, and the user updates to that target version fully (without resolving the conflict), that field will not exist anymore in the resulting rule. ## Acceptance criteria - [x] Extend the contract of the API endpoint according to the [POC](#144060): - [x] Add the ability to pick the `MERGED` version for rule upgrades. If the `MERGED` version is selected, the diffs are recalculated and the rule fields are updated to the result of the diff calculation. This is only possible if all field diffs return a `conflict` value of either `NO`. If any fields returns a value of `NON_SOLVABLE` or `SOLVABLE`, reject the request with an error specifying that there are conflicts, and that they must be resolved on a per-field basis. - [x] Calculate diffs inside this endpoint, when the value of `pick_version` is `MERGED`. - [x] Add the ability to specify rule field versions, to update specific fields to different `pick_versions`: `BASE' | 'CURRENT' | 'TARGET' | 'MERGED' | 'RESOLVED'` (See `FieldUpgradeRequest` in [PoC](#144060) for details) ## Handling of special fields Specific fields are handled under the hood based on #186544 See implementation in `x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/api/perform_rule_upgrade/determine_field_upgrade_status.ts`, which imports fields to handle under the hood: - `DiffableFieldsToOmit` - `FieldsToUpdateToCurrentVersion` ## Edge cases - [x] If target version of rule has a **rule type change**, check that all `pick_version`, at all levels, match `TARGET`. Otherwise, create new error and add to ruleErrors array. - [x] if a rule has a specific `targetVersion.type` (for example, EQL) and the user includes in its `fields` object of the request payload any fields which do not match that rule type (in this case, for example, sending in `machine_learning_job_id` as part of `fields`), throw an error for that rule. - [x] Calculation of field diffs: what happens if some fields have a conflict value of `NON_SOLVABLE`: - [x] If the whole rule is being updated to `MERGED`, and **ANY** fields return with a `NON_SOLVABLE` conflict, reject the whole update for that rule: create new error and add to ruleErrors array. - [x] **EXCEPTION** for case above: the whole rule is being updated to `MERGED`, and one or more of the fields return with a `NON_SOLVABLE` conflict, BUT those same fields have a specific `pick_version` for them in the `fields` object which **ARE NOT** `MERGED`. No error should be reported in this case. - [x] The whole rule is being updated to any `pick_version` other than MERGED, but any specific field in the `fields` object is set to upgrade to `MERGED`, and the diff for that fields returns a `NON_SOLVABLE` conflict. In that case, create new error and add to ruleErrors array. ### TODO - [[Security Solution] Add InvestigationFields and AlertSuppression fields to the upgrade workflow [#190597]](#190597): InvestigationFields is already working, but AlertSuppression is still currently handled under the hood to update to current version. ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: Maxim Palenov <[email protected]>
…1439) Fixes: elastic#166376 (main ticket) Fixes: elastic#186544 (handling of specific fields) Fixes: elastic#180195 (replace PATCH with PUT logic on rule upgrade) ## Summary - Enhances the `/upgrade/_perform` endpoint to upgrade rules in a way that works with prebuilt rules customized by users and resolve conflicts between user customizations and updates from Elastic. - Handles special fields under the hood (see below) - Replaces the update prebuilt rule logic to work with PUT instead of PATCH. ### Rough implementation plan - For each `upgradeableRule`, we attempt to build the payload necessary to pass to `upgradePrebuiltRules()`, which is of type `PrebuiltRuleAsset`. So we retrieve the field names from `FIELDS_PAYLOAD_BY_RULE_TYPE` and loop through them. - If any of those `field`s are non-upgreadable, (i.e. its value needs to be handled under the hood) we do so in `determineFieldUpgradeStatus`. - Otherwise, we continue to build a `FieldUpgradeSpecifier` for each field, which will help us determine if that field needs to be set to the base, current, target version, OR if it needs to be calculated as a MERGED value, or it is passed in the request payload as a RESOLVED value. - Notice that we are iterating over "flat" (non-grouped) fields which are part of the `PrebuiltRuleAsset` schema. This means that mapping is necessary between these flat fields and the diffable (grouped) fields that are used in the API contract, part of `DiffableRule`. For example, if we try to determine the value for the `query` field, we will need to look up for its value in the `eql_query` field if the target rule is `eql` or in `esql_query` if the target rule is `esql`. All these mappings can be found in `diffable_rule_fields_mappings.ts`. - Once a `FieldUpgradeSpecifier` has been retrieved for each field of the payload we are building, retrieve its actual value: either fetching it from the base, current or target versions of the rule, from the three way diff calculation, or retrieving it from the request payload if it resolved. - Do this for all upgreadable rules, and the pass the payload array into `upgradePrebuiltRules()`. - **IMPORTANT:** The upgrade prebuilt rules logic has been changed from PATCH to PUT. That means that if the next version of a rule removes a field, and the user updates to that target version, those fields will be undefined in the resulting rule. **Additional example:** a installs a rule, and creates a `timeline_id` for it rule by modifying it. If neither the next version (target version) still does not have a `timeline_id` field for it, and the user updates to that target version fully (without resolving the conflict), that field will not exist anymore in the resulting rule. ## Acceptance criteria - [x] Extend the contract of the API endpoint according to the [POC](elastic#144060): - [x] Add the ability to pick the `MERGED` version for rule upgrades. If the `MERGED` version is selected, the diffs are recalculated and the rule fields are updated to the result of the diff calculation. This is only possible if all field diffs return a `conflict` value of either `NO`. If any fields returns a value of `NON_SOLVABLE` or `SOLVABLE`, reject the request with an error specifying that there are conflicts, and that they must be resolved on a per-field basis. - [x] Calculate diffs inside this endpoint, when the value of `pick_version` is `MERGED`. - [x] Add the ability to specify rule field versions, to update specific fields to different `pick_versions`: `BASE' | 'CURRENT' | 'TARGET' | 'MERGED' | 'RESOLVED'` (See `FieldUpgradeRequest` in [PoC](elastic#144060) for details) ## Handling of special fields Specific fields are handled under the hood based on elastic#186544 See implementation in `x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/api/perform_rule_upgrade/determine_field_upgrade_status.ts`, which imports fields to handle under the hood: - `DiffableFieldsToOmit` - `FieldsToUpdateToCurrentVersion` ## Edge cases - [x] If target version of rule has a **rule type change**, check that all `pick_version`, at all levels, match `TARGET`. Otherwise, create new error and add to ruleErrors array. - [x] if a rule has a specific `targetVersion.type` (for example, EQL) and the user includes in its `fields` object of the request payload any fields which do not match that rule type (in this case, for example, sending in `machine_learning_job_id` as part of `fields`), throw an error for that rule. - [x] Calculation of field diffs: what happens if some fields have a conflict value of `NON_SOLVABLE`: - [x] If the whole rule is being updated to `MERGED`, and **ANY** fields return with a `NON_SOLVABLE` conflict, reject the whole update for that rule: create new error and add to ruleErrors array. - [x] **EXCEPTION** for case above: the whole rule is being updated to `MERGED`, and one or more of the fields return with a `NON_SOLVABLE` conflict, BUT those same fields have a specific `pick_version` for them in the `fields` object which **ARE NOT** `MERGED`. No error should be reported in this case. - [x] The whole rule is being updated to any `pick_version` other than MERGED, but any specific field in the `fields` object is set to upgrade to `MERGED`, and the diff for that fields returns a `NON_SOLVABLE` conflict. In that case, create new error and add to ruleErrors array. ### TODO - [[Security Solution] Add InvestigationFields and AlertSuppression fields to the upgrade workflow [elastic#190597]](elastic#190597): InvestigationFields is already working, but AlertSuppression is still currently handled under the hood to update to current version. ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: Maxim Palenov <[email protected]> (cherry picked from commit 7c38873)
) (#196471) # Backport This will backport the following commits from `main` to `8.x`: - [[Security Solution] Extend upgrade perform endpoint logic (#191439)](#191439) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Juan Pablo Djeredjian","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-16T01:51:25Z","message":"[Security Solution] Extend upgrade perform endpoint logic (#191439)\n\nFixes: #166376 (main ticket)\r\nFixes: #186544 (handling of\r\nspecific fields)\r\nFixes: #180195 (replace PATCH\r\nwith PUT logic on rule upgrade)\r\n\r\n## Summary\r\n\r\n- Enhances the `/upgrade/_perform` endpoint to upgrade rules in a way\r\nthat works with prebuilt rules customized by users and resolve conflicts\r\nbetween user customizations and updates from Elastic.\r\n- Handles special fields under the hood (see below)\r\n- Replaces the update prebuilt rule logic to work with PUT instead of\r\nPATCH.\r\n\r\n### Rough implementation plan\r\n- For each `upgradeableRule`, we attempt to build the payload necessary\r\nto pass to `upgradePrebuiltRules()`, which is of type\r\n`PrebuiltRuleAsset`. So we retrieve the field names from\r\n`FIELDS_PAYLOAD_BY_RULE_TYPE` and loop through them.\r\n- If any of those `field`s are non-upgreadable, (i.e. its value needs to\r\nbe handled under the hood) we do so in `determineFieldUpgradeStatus`.\r\n- Otherwise, we continue to build a `FieldUpgradeSpecifier` for each\r\nfield, which will help us determine if that field needs to be set to the\r\nbase, current, target version, OR if it needs to be calculated as a\r\nMERGED value, or it is passed in the request payload as a RESOLVED\r\nvalue.\r\n- Notice that we are iterating over \"flat\" (non-grouped) fields which\r\nare part of the `PrebuiltRuleAsset` schema. This means that mapping is\r\nnecessary between these flat fields and the diffable (grouped) fields\r\nthat are used in the API contract, part of `DiffableRule`. For example,\r\nif we try to determine the value for the `query` field, we will need to\r\nlook up for its value in the `eql_query` field if the target rule is\r\n`eql` or in `esql_query` if the target rule is `esql`. All these\r\nmappings can be found in `diffable_rule_fields_mappings.ts`.\r\n- Once a `FieldUpgradeSpecifier` has been retrieved for each field of\r\nthe payload we are building, retrieve its actual value: either fetching\r\nit from the base, current or target versions of the rule, from the three\r\nway diff calculation, or retrieving it from the request payload if it\r\nresolved.\r\n- Do this for all upgreadable rules, and the pass the payload array into\r\n`upgradePrebuiltRules()`.\r\n- **IMPORTANT:** The upgrade prebuilt rules logic has been changed from\r\nPATCH to PUT. That means that if the next version of a rule removes a\r\nfield, and the user updates to that target version, those fields will be\r\nundefined in the resulting rule. **Additional example:** a installs a\r\nrule, and creates a `timeline_id` for it rule by modifying it. If\r\nneither the next version (target version) still does not have a\r\n`timeline_id` field for it, and the user updates to that target version\r\nfully (without resolving the conflict), that field will not exist\r\nanymore in the resulting rule.\r\n\r\n## Acceptance criteria\r\n\r\n- [x] Extend the contract of the API endpoint according to the\r\n[POC](https://github.com/elastic/kibana/pull/144060):\r\n- [x] Add the ability to pick the `MERGED` version for rule upgrades. If\r\nthe `MERGED` version is selected, the diffs are recalculated and the\r\nrule fields are updated to the result of the diff calculation. This is\r\nonly possible if all field diffs return a `conflict` value of either\r\n`NO`. If any fields returns a value of `NON_SOLVABLE` or `SOLVABLE`,\r\nreject the request with an error specifying that there are conflicts,\r\nand that they must be resolved on a per-field basis.\r\n- [x] Calculate diffs inside this endpoint, when the value of\r\n`pick_version` is `MERGED`.\r\n- [x] Add the ability to specify rule field versions, to update specific\r\nfields to different `pick_versions`: `BASE' | 'CURRENT' | 'TARGET' |\r\n'MERGED' | 'RESOLVED'` (See `FieldUpgradeRequest` in\r\n[PoC](#144060) for details)\r\n\r\n## Handling of special fields\r\n\r\nSpecific fields are handled under the hood based on\r\nhttps://github.com//issues/186544\r\n\r\nSee implementation in\r\n`x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/api/perform_rule_upgrade/determine_field_upgrade_status.ts`,\r\nwhich imports fields to handle under the hood:\r\n- `DiffableFieldsToOmit`\r\n- `FieldsToUpdateToCurrentVersion`\r\n\r\n## Edge cases\r\n\r\n- [x] If target version of rule has a **rule type change**, check that\r\nall `pick_version`, at all levels, match `TARGET`. Otherwise, create new\r\nerror and add to ruleErrors array.\r\n- [x] if a rule has a specific `targetVersion.type` (for example, EQL)\r\nand the user includes in its `fields` object of the request payload any\r\nfields which do not match that rule type (in this case, for example,\r\nsending in `machine_learning_job_id` as part of `fields`), throw an\r\nerror for that rule.\r\n- [x] Calculation of field diffs: what happens if some fields have a\r\nconflict value of `NON_SOLVABLE`:\r\n- [x] If the whole rule is being updated to `MERGED`, and **ANY** fields\r\nreturn with a `NON_SOLVABLE` conflict, reject the whole update for that\r\nrule: create new error and add to ruleErrors array.\r\n- [x] **EXCEPTION** for case above: the whole rule is being updated to\r\n`MERGED`, and one or more of the fields return with a `NON_SOLVABLE`\r\nconflict, BUT those same fields have a specific `pick_version` for them\r\nin the `fields` object which **ARE NOT** `MERGED`. No error should be\r\nreported in this case.\r\n- [x] The whole rule is being updated to any `pick_version` other than\r\nMERGED, but any specific field in the `fields` object is set to upgrade\r\nto `MERGED`, and the diff for that fields returns a `NON_SOLVABLE`\r\nconflict. In that case, create new error and add to ruleErrors array.\r\n\r\n### TODO\r\n\r\n- [[Security Solution] Add InvestigationFields and AlertSuppression\r\nfields to the upgrade workflow\r\n[#190597]](https://github.com/elastic/kibana/issues/190597):\r\nInvestigationFields is already working, but AlertSuppression is still\r\ncurrently handled under the hood to update to current version.\r\n\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for breaking API changes and was [labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by: Maxim Palenov <[email protected]>","sha":"7c3887309cec54cc21e1abf8a2522afa49147712","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:enhancement","Team:Fleet","v9.0.0","Team:Detections and Resp","Team: SecuritySolution","Team:Detection Rule Management","Feature:Prebuilt Detection Rules","backport:prev-minor","v8.16.0"],"title":"[Security Solution] Extend upgrade perform endpoint logic","number":191439,"url":"https://github.com/elastic/kibana/pull/191439","mergeCommit":{"message":"[Security Solution] Extend upgrade perform endpoint logic (#191439)\n\nFixes: #166376 (main ticket)\r\nFixes: #186544 (handling of\r\nspecific fields)\r\nFixes: #180195 (replace PATCH\r\nwith PUT logic on rule upgrade)\r\n\r\n## Summary\r\n\r\n- Enhances the `/upgrade/_perform` endpoint to upgrade rules in a way\r\nthat works with prebuilt rules customized by users and resolve conflicts\r\nbetween user customizations and updates from Elastic.\r\n- Handles special fields under the hood (see below)\r\n- Replaces the update prebuilt rule logic to work with PUT instead of\r\nPATCH.\r\n\r\n### Rough implementation plan\r\n- For each `upgradeableRule`, we attempt to build the payload necessary\r\nto pass to `upgradePrebuiltRules()`, which is of type\r\n`PrebuiltRuleAsset`. So we retrieve the field names from\r\n`FIELDS_PAYLOAD_BY_RULE_TYPE` and loop through them.\r\n- If any of those `field`s are non-upgreadable, (i.e. its value needs to\r\nbe handled under the hood) we do so in `determineFieldUpgradeStatus`.\r\n- Otherwise, we continue to build a `FieldUpgradeSpecifier` for each\r\nfield, which will help us determine if that field needs to be set to the\r\nbase, current, target version, OR if it needs to be calculated as a\r\nMERGED value, or it is passed in the request payload as a RESOLVED\r\nvalue.\r\n- Notice that we are iterating over \"flat\" (non-grouped) fields which\r\nare part of the `PrebuiltRuleAsset` schema. This means that mapping is\r\nnecessary between these flat fields and the diffable (grouped) fields\r\nthat are used in the API contract, part of `DiffableRule`. For example,\r\nif we try to determine the value for the `query` field, we will need to\r\nlook up for its value in the `eql_query` field if the target rule is\r\n`eql` or in `esql_query` if the target rule is `esql`. All these\r\nmappings can be found in `diffable_rule_fields_mappings.ts`.\r\n- Once a `FieldUpgradeSpecifier` has been retrieved for each field of\r\nthe payload we are building, retrieve its actual value: either fetching\r\nit from the base, current or target versions of the rule, from the three\r\nway diff calculation, or retrieving it from the request payload if it\r\nresolved.\r\n- Do this for all upgreadable rules, and the pass the payload array into\r\n`upgradePrebuiltRules()`.\r\n- **IMPORTANT:** The upgrade prebuilt rules logic has been changed from\r\nPATCH to PUT. That means that if the next version of a rule removes a\r\nfield, and the user updates to that target version, those fields will be\r\nundefined in the resulting rule. **Additional example:** a installs a\r\nrule, and creates a `timeline_id` for it rule by modifying it. If\r\nneither the next version (target version) still does not have a\r\n`timeline_id` field for it, and the user updates to that target version\r\nfully (without resolving the conflict), that field will not exist\r\nanymore in the resulting rule.\r\n\r\n## Acceptance criteria\r\n\r\n- [x] Extend the contract of the API endpoint according to the\r\n[POC](https://github.com/elastic/kibana/pull/144060):\r\n- [x] Add the ability to pick the `MERGED` version for rule upgrades. If\r\nthe `MERGED` version is selected, the diffs are recalculated and the\r\nrule fields are updated to the result of the diff calculation. This is\r\nonly possible if all field diffs return a `conflict` value of either\r\n`NO`. If any fields returns a value of `NON_SOLVABLE` or `SOLVABLE`,\r\nreject the request with an error specifying that there are conflicts,\r\nand that they must be resolved on a per-field basis.\r\n- [x] Calculate diffs inside this endpoint, when the value of\r\n`pick_version` is `MERGED`.\r\n- [x] Add the ability to specify rule field versions, to update specific\r\nfields to different `pick_versions`: `BASE' | 'CURRENT' | 'TARGET' |\r\n'MERGED' | 'RESOLVED'` (See `FieldUpgradeRequest` in\r\n[PoC](#144060) for details)\r\n\r\n## Handling of special fields\r\n\r\nSpecific fields are handled under the hood based on\r\nhttps://github.com//issues/186544\r\n\r\nSee implementation in\r\n`x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/api/perform_rule_upgrade/determine_field_upgrade_status.ts`,\r\nwhich imports fields to handle under the hood:\r\n- `DiffableFieldsToOmit`\r\n- `FieldsToUpdateToCurrentVersion`\r\n\r\n## Edge cases\r\n\r\n- [x] If target version of rule has a **rule type change**, check that\r\nall `pick_version`, at all levels, match `TARGET`. Otherwise, create new\r\nerror and add to ruleErrors array.\r\n- [x] if a rule has a specific `targetVersion.type` (for example, EQL)\r\nand the user includes in its `fields` object of the request payload any\r\nfields which do not match that rule type (in this case, for example,\r\nsending in `machine_learning_job_id` as part of `fields`), throw an\r\nerror for that rule.\r\n- [x] Calculation of field diffs: what happens if some fields have a\r\nconflict value of `NON_SOLVABLE`:\r\n- [x] If the whole rule is being updated to `MERGED`, and **ANY** fields\r\nreturn with a `NON_SOLVABLE` conflict, reject the whole update for that\r\nrule: create new error and add to ruleErrors array.\r\n- [x] **EXCEPTION** for case above: the whole rule is being updated to\r\n`MERGED`, and one or more of the fields return with a `NON_SOLVABLE`\r\nconflict, BUT those same fields have a specific `pick_version` for them\r\nin the `fields` object which **ARE NOT** `MERGED`. No error should be\r\nreported in this case.\r\n- [x] The whole rule is being updated to any `pick_version` other than\r\nMERGED, but any specific field in the `fields` object is set to upgrade\r\nto `MERGED`, and the diff for that fields returns a `NON_SOLVABLE`\r\nconflict. In that case, create new error and add to ruleErrors array.\r\n\r\n### TODO\r\n\r\n- [[Security Solution] Add InvestigationFields and AlertSuppression\r\nfields to the upgrade workflow\r\n[#190597]](https://github.com/elastic/kibana/issues/190597):\r\nInvestigationFields is already working, but AlertSuppression is still\r\ncurrently handled under the hood to update to current version.\r\n\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for breaking API changes and was [labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by: Maxim Palenov <[email protected]>","sha":"7c3887309cec54cc21e1abf8a2522afa49147712"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/191439","number":191439,"mergeCommit":{"message":"[Security Solution] Extend upgrade perform endpoint logic (#191439)\n\nFixes: #166376 (main ticket)\r\nFixes: #186544 (handling of\r\nspecific fields)\r\nFixes: #180195 (replace PATCH\r\nwith PUT logic on rule upgrade)\r\n\r\n## Summary\r\n\r\n- Enhances the `/upgrade/_perform` endpoint to upgrade rules in a way\r\nthat works with prebuilt rules customized by users and resolve conflicts\r\nbetween user customizations and updates from Elastic.\r\n- Handles special fields under the hood (see below)\r\n- Replaces the update prebuilt rule logic to work with PUT instead of\r\nPATCH.\r\n\r\n### Rough implementation plan\r\n- For each `upgradeableRule`, we attempt to build the payload necessary\r\nto pass to `upgradePrebuiltRules()`, which is of type\r\n`PrebuiltRuleAsset`. So we retrieve the field names from\r\n`FIELDS_PAYLOAD_BY_RULE_TYPE` and loop through them.\r\n- If any of those `field`s are non-upgreadable, (i.e. its value needs to\r\nbe handled under the hood) we do so in `determineFieldUpgradeStatus`.\r\n- Otherwise, we continue to build a `FieldUpgradeSpecifier` for each\r\nfield, which will help us determine if that field needs to be set to the\r\nbase, current, target version, OR if it needs to be calculated as a\r\nMERGED value, or it is passed in the request payload as a RESOLVED\r\nvalue.\r\n- Notice that we are iterating over \"flat\" (non-grouped) fields which\r\nare part of the `PrebuiltRuleAsset` schema. This means that mapping is\r\nnecessary between these flat fields and the diffable (grouped) fields\r\nthat are used in the API contract, part of `DiffableRule`. For example,\r\nif we try to determine the value for the `query` field, we will need to\r\nlook up for its value in the `eql_query` field if the target rule is\r\n`eql` or in `esql_query` if the target rule is `esql`. All these\r\nmappings can be found in `diffable_rule_fields_mappings.ts`.\r\n- Once a `FieldUpgradeSpecifier` has been retrieved for each field of\r\nthe payload we are building, retrieve its actual value: either fetching\r\nit from the base, current or target versions of the rule, from the three\r\nway diff calculation, or retrieving it from the request payload if it\r\nresolved.\r\n- Do this for all upgreadable rules, and the pass the payload array into\r\n`upgradePrebuiltRules()`.\r\n- **IMPORTANT:** The upgrade prebuilt rules logic has been changed from\r\nPATCH to PUT. That means that if the next version of a rule removes a\r\nfield, and the user updates to that target version, those fields will be\r\nundefined in the resulting rule. **Additional example:** a installs a\r\nrule, and creates a `timeline_id` for it rule by modifying it. If\r\nneither the next version (target version) still does not have a\r\n`timeline_id` field for it, and the user updates to that target version\r\nfully (without resolving the conflict), that field will not exist\r\nanymore in the resulting rule.\r\n\r\n## Acceptance criteria\r\n\r\n- [x] Extend the contract of the API endpoint according to the\r\n[POC](https://github.com/elastic/kibana/pull/144060):\r\n- [x] Add the ability to pick the `MERGED` version for rule upgrades. If\r\nthe `MERGED` version is selected, the diffs are recalculated and the\r\nrule fields are updated to the result of the diff calculation. This is\r\nonly possible if all field diffs return a `conflict` value of either\r\n`NO`. If any fields returns a value of `NON_SOLVABLE` or `SOLVABLE`,\r\nreject the request with an error specifying that there are conflicts,\r\nand that they must be resolved on a per-field basis.\r\n- [x] Calculate diffs inside this endpoint, when the value of\r\n`pick_version` is `MERGED`.\r\n- [x] Add the ability to specify rule field versions, to update specific\r\nfields to different `pick_versions`: `BASE' | 'CURRENT' | 'TARGET' |\r\n'MERGED' | 'RESOLVED'` (See `FieldUpgradeRequest` in\r\n[PoC](#144060) for details)\r\n\r\n## Handling of special fields\r\n\r\nSpecific fields are handled under the hood based on\r\nhttps://github.com//issues/186544\r\n\r\nSee implementation in\r\n`x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/api/perform_rule_upgrade/determine_field_upgrade_status.ts`,\r\nwhich imports fields to handle under the hood:\r\n- `DiffableFieldsToOmit`\r\n- `FieldsToUpdateToCurrentVersion`\r\n\r\n## Edge cases\r\n\r\n- [x] If target version of rule has a **rule type change**, check that\r\nall `pick_version`, at all levels, match `TARGET`. Otherwise, create new\r\nerror and add to ruleErrors array.\r\n- [x] if a rule has a specific `targetVersion.type` (for example, EQL)\r\nand the user includes in its `fields` object of the request payload any\r\nfields which do not match that rule type (in this case, for example,\r\nsending in `machine_learning_job_id` as part of `fields`), throw an\r\nerror for that rule.\r\n- [x] Calculation of field diffs: what happens if some fields have a\r\nconflict value of `NON_SOLVABLE`:\r\n- [x] If the whole rule is being updated to `MERGED`, and **ANY** fields\r\nreturn with a `NON_SOLVABLE` conflict, reject the whole update for that\r\nrule: create new error and add to ruleErrors array.\r\n- [x] **EXCEPTION** for case above: the whole rule is being updated to\r\n`MERGED`, and one or more of the fields return with a `NON_SOLVABLE`\r\nconflict, BUT those same fields have a specific `pick_version` for them\r\nin the `fields` object which **ARE NOT** `MERGED`. No error should be\r\nreported in this case.\r\n- [x] The whole rule is being updated to any `pick_version` other than\r\nMERGED, but any specific field in the `fields` object is set to upgrade\r\nto `MERGED`, and the diff for that fields returns a `NON_SOLVABLE`\r\nconflict. In that case, create new error and add to ruleErrors array.\r\n\r\n### TODO\r\n\r\n- [[Security Solution] Add InvestigationFields and AlertSuppression\r\nfields to the upgrade workflow\r\n[#190597]](https://github.com/elastic/kibana/issues/190597):\r\nInvestigationFields is already working, but AlertSuppression is still\r\ncurrently handled under the hood to update to current version.\r\n\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for breaking API changes and was [labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by: Maxim Palenov <[email protected]>","sha":"7c3887309cec54cc21e1abf8a2522afa49147712"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Juan Pablo Djeredjian <[email protected]>
Resolves: #137446
NOTE: You can comment this PR description line-by-line in
security_solution/common/detection_engine/prebuilt_rules/poc/README.md
.Summary
PrebuiltRuleToInstall
schemastatus
and two_review
endpoints. I think we could skip implementing the_perform
endpoints within this POC because they seem to be less risky (given the_review
endpoints).TODO: https://github.com/elastic/kibana/pull/144060
comments.Open questions to clarify in parallel or later:
server/fleet_integration/handlers/install_prepackaged_rules.ts
to install only the promotion "Endpoint Security" rule if it's not installed. Don't install all the rules and don't upgrade rules.Workflows
Stage 1 (both workflows):
GET /internal/detection_engine/prebuilt_rules/status
.Stage 2 (upgrade workflow):
POST /internal/detection_engine/prebuilt_rules/upgrade/_review
.POST /internal/detection_engine/prebuilt_rules/upgrade/_perform
.Stage 2 (installation workflow):
POST /internal/detection_engine/prebuilt_rules/installation/_review
.POST /internal/detection_engine/prebuilt_rules/installation/_perform
.Data models
This POC implements and compares 3 new data models for historical versioned rule asset saved objects.
See the implementation in
server/lib/detection_engine/prebuilt_rules/logic/poc/saved_objects
.We will need to choose the one we will proceed with. Criteria considered so far:
Flat model
Every object is a historical rule version that contains the rule id, the content version and the content itself.
server/lib/detection_engine/prebuilt_rules/logic/poc/saved_objects/rule_asset_flat_saved_objects_type.ts
:Composite model v1
Every object is a rule, all historical content is stored in its nested field (an array).
server/lib/detection_engine/prebuilt_rules/logic/poc/saved_objects/rule_asset_composite_saved_objects_type.ts
:Composite model v2
Every object is a rule. Historical version information is stored as an array of small objects
which is mapped as a nested field. Historical content is stored in a map where keys are formed
in a special way so that we can fetch individual content versions for many rules in bulk.
server/lib/detection_engine/prebuilt_rules/logic/poc/saved_objects/rule_asset_composite2_saved_objects_type.ts
:API endpoints
See the implementation in
x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/api
.Get status of prebuilt rules
Response body:
Implementation:
server/lib/detection_engine/prebuilt_rules/api/get_prebuilt_rules_status/route.ts
.Review rules that can be upgraded
Response body:
Implementation:
server/lib/detection_engine/prebuilt_rules/api/review_rule_upgrade/route.ts
.Perform rule upgrade
Request body:
Response body:
Implementation: not implemented in this POC.
Review rules that can be installed
Response body:
Implementation:
server/lib/detection_engine/prebuilt_rules/api/review_rule_installation/route.ts
.Perform rule installation
Request body:
Response body:
Implementation: not implemented in this POC.
API performance considerations
Should be fast and lightweight (< 1 second). Should:
POST /internal/detection_engine/prebuilt_rules/*/_review
Could be slightly slow (1 to 5 seconds). Can:
POST /internal/detection_engine/prebuilt_rules/*/_perform
Could be moderately slow (< 1 minute). Can:
API testing
All the 3 added endpoints accept a
data_model
parameter so we could test their work and performancein different conditions and with different data models.
Generate test prebuilt rule assets (the endpoint will do it for all 3 data models).
Pick whatever number of versions you want to be generated per each rule.
Test get status endpoint
Test review installation endpoint
Test review upgrade endpoint
Rule fields
I did some research on rule fields to be able to determine which rule fields will be "customizable" and which will be "technical".
Please find the result and follow-up work to do in a dedicated ticket:
#147239
Diff algorithm
This section describes an algorithm that returns a 3-way diff between 3 rule versions: base, current, target.
Definition: diffable rule
We have two data structures that represent a prebuilt rule:
PrebuiltRuleToInstall
: schema for a prebuilt rule asset (filesystem or fleet package based).RuleResponse
: schema for an Alerting Framework's rule.These data structures are similar but different. In order to be able to run a diff between
an already installed prebuilt rule (
RuleResponse
) and its next version shipped by Elastic(
PrebuiltRuleToInstall
) we would first need to normalize both of them to a common interfacethat would be suitable for passing to the diff algorithm. This common interface is
DiffableRule
.common/detection_engine/prebuilt_rules/poc/diffable_rule_model/diffable_rule.ts
:Definition: 3-way diff
We will use a 3-way diff algorithm for two things:
DiffableRule
).In order to calculate a 3-way diff result for a field, we will need 3 input values:
And, our goal will be:
Potential reasons for a conflict:
rule's behavior or introducing unintended side-effects in the behavior from the user's point of view
Below is a
ThreeWayDiff
result's interface.common/detection_engine/prebuilt_rules/poc/diff_model/three_way_diff.ts
:The algorithm itself
GIVEN a list of prebuilt rules that can be upgraded (
currentVersion[]
)AND a list of the corresponding base asset saved objects (
baseVersion[]
)AND a list of the corresponding target asset saved objects (
targetVersion[]
)DO run the diff algorithm for every match of these 3 versions.
common/detection_engine/prebuilt_rules/poc/diff_algorithm/calculate_rule_diff.ts
:The algorithm's overall structure is fully implemented and works, but it uses a simple diff algorithm
for calculating field diffs. This algorithm is kinda nasty: it doesn't try to merge any values
and marks a diff as conflict if base version != current version != target version.
common/detection_engine/prebuilt_rules/poc/diff_algorithm/calculation/algorithms/simple_diff_algorithm.ts
.The idea is to write more flexible algorithms for different rule fields that would generate fewer
conflicts and would try to automatically merge changes when it can be technically done and it won't
result in inintended changes in the rule from the user standpoint.