[9.0] [Security Solution][Rules Management] Separate actions import logic from rules import (#216380)#218467
Merged
Conversation
…rom rules import (elastic#216380) ## Summary Redo of elastic#193471 Closes https://github.com/elastic/security-team/issues/8644 > Fixes a bug where importing a rule fails with a connector into a space where (1) the connector already exists, and (2) the existing connector was exported and re-imported from another space. The import logic in this scenario effectively tries to convert the action ID on the rule import twice. The second conversion attempt tries to use the old action ID to look up the correct new action ID in a map, however, in this test scenario the action ID has already been updated by legacy SO ID migration logic and there is no map entry with the new ID as a key. The result is that the second attempt sets the action ID to undefined, resulting in an import failure. The root cause of the bug is that we have two different places in the rule import logic where action IDs are migrated. The first ID migration was done by `migrateLegacyActionsIds` prior to importing rule actions, and the second migration was done by `importRuleActionConnectors` after importing the actions. `importRuleActionConnectors` used a lookup table to convert old IDs to new IDs, but if the connector already existed and had an `originId` then the rule action would already be migrated by `migrateLegacyActionsIds`. The lookup table used by `importRuleActionConnectors` does not have entries for migrated IDs, only the original IDs, so in that case the result of the lookup is `undefined` which we assign to the action ID. This PR reworks the logic to create a clean separation between action and rule import. We now import the connectors first, ignoring the rules, then migrate action IDs on the rules afterwards. This handles connectors changing IDs in any way, either through the 7.x->8.0 migration long ago or IDs changing on import if there are ID conflicts. Only after the connectors are imported and rule actions are migrated do we then verify if each rule action references a connector ID that actually exists with the new `checkRuleActions` function, replacing `checkIfActionsHaveMissingConnectors` and related functions that were also buggy. Finally, as a nice side effect this rework removes "rule action connector missing" errors out of the `action_connector_errors` part of the response. `action_connector_errors` is reserved for errors importing connectors specifically. If a rule action is missing a connector and therefore we don't import the rule, that's a rule error and it's represented in the `errors` part of the response. Since the shape of the response is not changing, I don't consider this a breaking change but rather a bug fix. ## Repro Steps Repro Steps 1. Download the export file below and change the extension back to .ndjson from .json (github does not allow .ndjson files [rules_export.json](https://github.com/user-attachments/files/17065272/rules_export.json) 2. Import the rule and connector into a space (default is fine) 3. Create a new space 4. Import the rule and connector into the new space 5. Import the rule and connector into the new space again, but check the `Overwrite existing connectors with conflicting action "id"` box. Observe the failure. --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit 52ecdd0)
Contributor
⏳ Build in-progress, with failures
Failed CI StepsTest Failures |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport
This will backport the following commits from
mainto9.0:Questions ?
Please refer to the Backport tool documentation