[RAM] mark SIEM legacy actions migrated rules #154438
Labels
8.9 candidate
performance
Team:ResponseOps
Label for the ResponseOps team (formerly the Cases and Alerting teams)
Comments
|
Pinging @elastic/response-ops (Team:ResponseOps) |
2 tasks
vitaliidm
added a commit
that referenced
this issue
Apr 19, 2023
…lesClient (#153101) ## Summary - this PR is the first part of work related to conditional logic actions. The rest of PRs, will be merged after this one. As they depend on a work implemented here. [List of tickets](elastic/security-team#2894 (comment)) - addresses #151919 - moves code related to legacy actions migration from D&R to RulesClient, [details](#151919 (comment)) - similarly to D&R part, now rulesClient read APIs, would return legacy actions within rule - similarly, every mutation API in rulesClient, would migrate legacy actions, and remove sidecar SO - each migrated legacy action will have also [`frequency` object](https://github.com/elastic/kibana/blob/8.7/x-pack/plugins/alerting/server/types.ts#L234-L238), that would allow to have notifyWhen/throttle on action level once #151916 is implemented, which is targeted in 8.8, right after this PR. But before it's merged, `frequency` is getting removed in [update/bulk_edit/create APIs](https://github.com/elastic/kibana/blob/8.7/x-pack/plugins/alerting/server/rules_client/methods/update.ts#L151-L160). Hence it's not reflected in most of the tests at this point. - cleanup of legacy actions related code in D&R - adds unit tests for RulesClient - keeps functional/e2e tests in D&R Changes in behaviour, introduced in this PR: - since, migration happens within single rulesClient API call, revision in migrated rule will increment by `1` only. - legacy actions from sidecar SO, now will be merged with rules actions, if there any. Before, in the previous implementation, there was inconsistency in a way how legacy and rules actions were treated. - On read: actions existing in rule, [would take precedence over legacy ones ](https://github.com/elastic/kibana/blob/8.7/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_actions.ts#L94-L114) - On migration: SO actions [only saved](https://github.com/elastic/kibana/blob/8.7/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/rule_actions/legacy_action_migration.ts#L114). If any actions present in rule, they will be lost. Here is an example video from **main** branch <details> <summary>Here is an example video from MAIN branch, where action in rule is overwritten by legacy action</summary> https://user-images.githubusercontent.com/92328789/230397535-d3fcd644-7cf9-4970-a573-18fd8c9f2235.mov </details> So, depends on sequence of events, different actions could be saved for identical use case: rule has both legacy and existing action. - if rule migrated through update API, existing in rule action will be saved - if rule migrated through enable/disable API, bulk edit, legacy action will be saved In this implementation, both existing in rule and legacy actions will be merged, to prevent loss of actions <details> <summary>Here is an example video from this PR branch, where actions are merged</summary> <video src="https://user-images.githubusercontent.com/92328789/230405569-f1da38e9-4e36-46a8-9654-f664e0a31063.mov" /> </details> - when duplicating rule, we don't migrate source rule anymore. It can lead to unwanted API key regeneration, with possible loss of privileges, earlier associated with the source rule. As part of UX, when duplicating any entity, users would not be expecting source entity to be changed TODO: - performance improvement issue for future #154438 - currently, in main branch, when migration is performed through rule enabling, actions not showing anymore in UI. Relevant ticket is #154567 I haven't fixed it in this PR, as in[ the next one ](#153113), we will display notifyWhen/throttle on action level in UI, rather than on rule level. So, once that PR is merged, actions should be displayed in new UI ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios ### 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: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Garrett Spong <spong@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
After #151919 is implemented, we would like to add a performance improvement to the implementation.
In current implementation, to migrate a rule, we perform at least 2 requests for each SIEM rule:
siem.notificationsalert type ruleThis proposal ticket is, each rule that is migrated or doesn't have legacy actions, should be marked as migrated.
So, during the subsequent calls of rulesClient API, legacy actions API calls would be skipped for this rule.
This property can be called smth like
isSiemRuleMigratedand can be saved within rule's attributes.It does not need be indexed, as we do not plan to query rules based on this property, but only determine whether to call migration or not
cc: @XavierM
The text was updated successfully, but these errors were encountered: