Skip to content
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

[ResponseOps] Add revision field support to Alerting framework rules #137164

Closed
Tracked by #174166 ...
xcrzx opened this issue Jul 26, 2022 · 3 comments · Fixed by #147398
Closed
Tracked by #174166 ...

[ResponseOps] Add revision field support to Alerting framework rules #137164

xcrzx opened this issue Jul 26, 2022 · 3 comments · Fixed by #147398
Assignees
Labels
8.8 candidate Feature:Alerting/RulesFramework Issues related to the Alerting Rules Framework Feature:Detection Rules Anything related to Security Solution's Detection Rules Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.8.0

Comments

@xcrzx
Copy link
Contributor

xcrzx commented Jul 26, 2022

Parent ticket: #136213

Summary

Implement a new rule attribute revision of type long:

  • All newly created rules should have revision set to 0
  • On rule save, if rule params have been modified, increase the revision number by 1
  • Revisions should not be modifiable by users, i.e., they are increased automatically and ignored when passed as arguments to functions that mutate rules
  • Rule changes like activation/deactivation should not lead to revision increase
  • Changes to dynamic rule attributes (rule state) like executionStatus, or monitoring should not lead to revision increase. See x-pack/plugins/alerting/server/saved_objects/mappings.ts for more examples.

Migrate all existing rules to the new revision field:

  • For all existing rules, set revision to 0
@xcrzx xcrzx added Feature:Detection Rules Anything related to Security Solution's Detection Rules Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detection Rule Management Security Detection Rule Management Team 8.5 candidate labels Jul 26, 2022
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@banderror banderror changed the title Add revision field support to Alerting framework rules [ResponseOps] Add revision field support to Alerting framework rules Jul 28, 2022
@banderror banderror added Feature:Alerting/RulesFramework Issues related to the Alerting Rules Framework Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) labels Jul 28, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops (Team:ResponseOps)

spong added a commit that referenced this issue Mar 10, 2023
## Summary

Resolves #137164. 

This PR adds a new `revision` field (number) to Alerting Rules that
auto-increments when relevant content changes have been made via the
`Rules Client`. _Relevant content changes_ are defined as any content
change that the user may either want to be notified about, or have the
option to later revert to. This will include most all changes, except
the enabling/disabling of the rule, or updates to metadata fields like
`executionStatus` or `monitoring`. This `revision` field is not intended
to be user editable, and should be ignored if ever provided via the API.

See #136213 for additional
details. To be followed-up by
#137168, which will remove the
version bump logic from security solution.

## Details

### Migrations

Includes SO migration to default `revision` to `0` when upgrading a
cluster, or when importing `pre-8.8.0` rules via the SO Management UI.
For consistency, security rule import follows the same logic as SO
Management and will not reset the `revision` to `0` when overriding or
creating a new rule.

### Downstream Index Updates

* EventLog _has not_ been updated to include `revision` along with basic
rule fields currently being written. Should we?
* AAD Schema will be updated in
#151388 (as this one is getting
pretty big) to include `revision` so alerts written will include which
specific revision of the rule created the alert.

### Reference Fields

Any creation of or modification to `actions` will result in a revision
increment. More typical reference fields like `exception lists` on the
security side will only result in a revision increment when the list is
initially associated/deleted from the rule (as subsequent updates will
be done directly against the list).

### RuleClient Updates

The following methods within the RuleClient have been updated to support
incrementing revision when relevant field changes have been detected:

* `clone()` - resets to 0 currently (see open question)
* `update()` - increments `revision` so long a change has been made to
relevant fields (fields not in [ignore
list](https://github.com/elastic/kibana/pull/147398/files#diff-6736e143ede2dc06e825bddcdc23b4d088a6620805751db4eddc5900d586c4dfR69-R85))
* `bulkEdit()` - increments `revision` for relevant fields (all current
bulk edit fields minus api key/snooze/mute)

Mutation methods not updated to include revision log:
* `snooze()`
* `unsnooze()`
*  `clearExpiredSnoozes()`
*  `muteAll()`
*  `unmuteAll()`
*  `muteInstance()`
*  `unmuteInstance()`
* `updateApiKey()` - increments revision as rule functionality could be
impacted


## Open questions:
- [X] Should `clone()` in RulesClient reset revision to 0 as if it's a
new rule, or keep the current value? (see
[comment](https://github.com/elastic/kibana/pull/147398/files#r1106484105))
- [X] What about snooze/un-snooze, and mute/unmute? Should we update
revision on these field changes as well? (see
[comment](https://github.com/elastic/kibana/pull/147398/files#r1106431966))
- Discussed with @XavierM and determined to not update on
snooze/mute/API key changes as this actions could be plentiful and don't
necessarily represent a version of the rule a user would want to revert
to, thus polluting the revision history.
- [ ] Should we write `revision` to EventLog?


---

### Checklist

Delete any items that are not applicable to this PR.

- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
  - [ ] To work with docs team 
- [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

- [X] 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)
bmorelli25 pushed a commit to bmorelli25/kibana that referenced this issue Mar 10, 2023
## Summary

Resolves elastic#137164. 

This PR adds a new `revision` field (number) to Alerting Rules that
auto-increments when relevant content changes have been made via the
`Rules Client`. _Relevant content changes_ are defined as any content
change that the user may either want to be notified about, or have the
option to later revert to. This will include most all changes, except
the enabling/disabling of the rule, or updates to metadata fields like
`executionStatus` or `monitoring`. This `revision` field is not intended
to be user editable, and should be ignored if ever provided via the API.

See elastic#136213 for additional
details. To be followed-up by
elastic#137168, which will remove the
version bump logic from security solution.

## Details

### Migrations

Includes SO migration to default `revision` to `0` when upgrading a
cluster, or when importing `pre-8.8.0` rules via the SO Management UI.
For consistency, security rule import follows the same logic as SO
Management and will not reset the `revision` to `0` when overriding or
creating a new rule.

### Downstream Index Updates

* EventLog _has not_ been updated to include `revision` along with basic
rule fields currently being written. Should we?
* AAD Schema will be updated in
elastic#151388 (as this one is getting
pretty big) to include `revision` so alerts written will include which
specific revision of the rule created the alert.

### Reference Fields

Any creation of or modification to `actions` will result in a revision
increment. More typical reference fields like `exception lists` on the
security side will only result in a revision increment when the list is
initially associated/deleted from the rule (as subsequent updates will
be done directly against the list).

### RuleClient Updates

The following methods within the RuleClient have been updated to support
incrementing revision when relevant field changes have been detected:

* `clone()` - resets to 0 currently (see open question)
* `update()` - increments `revision` so long a change has been made to
relevant fields (fields not in [ignore
list](https://github.com/elastic/kibana/pull/147398/files#diff-6736e143ede2dc06e825bddcdc23b4d088a6620805751db4eddc5900d586c4dfR69-R85))
* `bulkEdit()` - increments `revision` for relevant fields (all current
bulk edit fields minus api key/snooze/mute)

Mutation methods not updated to include revision log:
* `snooze()`
* `unsnooze()`
*  `clearExpiredSnoozes()`
*  `muteAll()`
*  `unmuteAll()`
*  `muteInstance()`
*  `unmuteInstance()`
* `updateApiKey()` - increments revision as rule functionality could be
impacted


## Open questions:
- [X] Should `clone()` in RulesClient reset revision to 0 as if it's a
new rule, or keep the current value? (see
[comment](https://github.com/elastic/kibana/pull/147398/files#r1106484105))
- [X] What about snooze/un-snooze, and mute/unmute? Should we update
revision on these field changes as well? (see
[comment](https://github.com/elastic/kibana/pull/147398/files#r1106431966))
- Discussed with @XavierM and determined to not update on
snooze/mute/API key changes as this actions could be plentiful and don't
necessarily represent a version of the rule a user would want to revert
to, thus polluting the revision history.
- [ ] Should we write `revision` to EventLog?


---

### Checklist

Delete any items that are not applicable to this PR.

- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
  - [ ] To work with docs team 
- [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

- [X] 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)
nkhristinin pushed a commit that referenced this issue Mar 22, 2023
## Summary

Resolves #137164. 

This PR adds a new `revision` field (number) to Alerting Rules that
auto-increments when relevant content changes have been made via the
`Rules Client`. _Relevant content changes_ are defined as any content
change that the user may either want to be notified about, or have the
option to later revert to. This will include most all changes, except
the enabling/disabling of the rule, or updates to metadata fields like
`executionStatus` or `monitoring`. This `revision` field is not intended
to be user editable, and should be ignored if ever provided via the API.

See #136213 for additional
details. To be followed-up by
#137168, which will remove the
version bump logic from security solution.

## Details

### Migrations

Includes SO migration to default `revision` to `0` when upgrading a
cluster, or when importing `pre-8.8.0` rules via the SO Management UI.
For consistency, security rule import follows the same logic as SO
Management and will not reset the `revision` to `0` when overriding or
creating a new rule.

### Downstream Index Updates

* EventLog _has not_ been updated to include `revision` along with basic
rule fields currently being written. Should we?
* AAD Schema will be updated in
#151388 (as this one is getting
pretty big) to include `revision` so alerts written will include which
specific revision of the rule created the alert.

### Reference Fields

Any creation of or modification to `actions` will result in a revision
increment. More typical reference fields like `exception lists` on the
security side will only result in a revision increment when the list is
initially associated/deleted from the rule (as subsequent updates will
be done directly against the list).

### RuleClient Updates

The following methods within the RuleClient have been updated to support
incrementing revision when relevant field changes have been detected:

* `clone()` - resets to 0 currently (see open question)
* `update()` - increments `revision` so long a change has been made to
relevant fields (fields not in [ignore
list](https://github.com/elastic/kibana/pull/147398/files#diff-6736e143ede2dc06e825bddcdc23b4d088a6620805751db4eddc5900d586c4dfR69-R85))
* `bulkEdit()` - increments `revision` for relevant fields (all current
bulk edit fields minus api key/snooze/mute)

Mutation methods not updated to include revision log:
* `snooze()`
* `unsnooze()`
*  `clearExpiredSnoozes()`
*  `muteAll()`
*  `unmuteAll()`
*  `muteInstance()`
*  `unmuteInstance()`
* `updateApiKey()` - increments revision as rule functionality could be
impacted


## Open questions:
- [X] Should `clone()` in RulesClient reset revision to 0 as if it's a
new rule, or keep the current value? (see
[comment](https://github.com/elastic/kibana/pull/147398/files#r1106484105))
- [X] What about snooze/un-snooze, and mute/unmute? Should we update
revision on these field changes as well? (see
[comment](https://github.com/elastic/kibana/pull/147398/files#r1106431966))
- Discussed with @XavierM and determined to not update on
snooze/mute/API key changes as this actions could be plentiful and don't
necessarily represent a version of the rule a user would want to revert
to, thus polluting the revision history.
- [ ] Should we write `revision` to EventLog?


---

### Checklist

Delete any items that are not applicable to this PR.

- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
  - [ ] To work with docs team 
- [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

- [X] 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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.8 candidate Feature:Alerting/RulesFramework Issues related to the Alerting Rules Framework Feature:Detection Rules Anything related to Security Solution's Detection Rules Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.8.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants