forked from elastic/kibana
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[RAM] Adds auto-incrementing revision field to rules (elastic#147398)
## 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)
- Loading branch information
1 parent
51d50c2
commit 2c71631
Showing
112 changed files
with
663 additions
and
20 deletions.
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
100 changes: 100 additions & 0 deletions
100
x-pack/plugins/alerting/server/rules_client/lib/increment_revision.test.ts
This file contains 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,100 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the Elastic License | ||
| * 2.0; you may not use this file except in compliance with the Elastic License | ||
| * 2.0. | ||
| */ | ||
|
|
||
| import { UpdateOptions } from '..'; | ||
| import { mockedDateString } from '../tests/lib'; | ||
| import { incrementRevision } from './increment_revision'; | ||
| import { SavedObject } from '@kbn/core/server'; | ||
| import { RawRule, RuleTypeParams } from '../../types'; | ||
|
|
||
| describe('incrementRevision', () => { | ||
| const currentRule: SavedObject<RawRule> = { | ||
| id: '1', | ||
| type: 'alert', | ||
| attributes: { | ||
| enabled: true, | ||
| name: 'rule-name', | ||
| tags: ['tag-1', 'tag-2'], | ||
| alertTypeId: '123', | ||
| consumer: 'rule-consumer', | ||
| legacyId: null, | ||
| schedule: { interval: '1s' }, | ||
| actions: [], | ||
| params: {}, | ||
| createdBy: null, | ||
| updatedBy: null, | ||
| createdAt: mockedDateString, | ||
| updatedAt: mockedDateString, | ||
| apiKey: null, | ||
| apiKeyOwner: null, | ||
| throttle: null, | ||
| notifyWhen: null, | ||
| muteAll: false, | ||
| mutedInstanceIds: [], | ||
| executionStatus: { | ||
| status: 'unknown', | ||
| lastExecutionDate: '2020-08-20T19:23:38Z', | ||
| error: null, | ||
| warning: null, | ||
| }, | ||
| revision: 0, | ||
| }, | ||
| references: [], | ||
| }; | ||
|
|
||
| const updateOptions: UpdateOptions<RuleTypeParams> = { | ||
| id: '1', | ||
| data: { | ||
| schedule: { | ||
| interval: '1m', | ||
| }, | ||
| name: 'abc', | ||
| tags: ['foo'], | ||
| params: { | ||
| bar: true, | ||
| risk_score: 40, | ||
| severity: 'low', | ||
| }, | ||
| throttle: null, | ||
| notifyWhen: 'onActiveAlert', | ||
| actions: [], | ||
| }, | ||
| }; | ||
| const updatedParams: RuleTypeParams = { bar: true, risk_score: 40, severity: 'low' }; | ||
|
|
||
| it('should return the current revision if no attrs or params are updated', () => { | ||
| // @ts-expect-error | ||
| expect(incrementRevision(currentRule, { data: {} }, {})).toBe(0); | ||
| }); | ||
|
|
||
| it('should increment the revision if a root level attr is updated', () => { | ||
| expect(incrementRevision(currentRule, updateOptions, {})).toBe(1); | ||
| }); | ||
|
|
||
| it('should increment the revision if a rule param is updated', () => { | ||
| // @ts-expect-error | ||
| expect(incrementRevision(currentRule, { data: {} }, updatedParams)).toBe(1); | ||
| }); | ||
|
|
||
| it('should not increment the revision if an excluded attr is updated', () => { | ||
| // @ts-expect-error | ||
| expect(incrementRevision(currentRule, { data: { activeSnoozes: 'excludedValue' } }, {})).toBe( | ||
| 0 | ||
| ); | ||
| }); | ||
|
|
||
| it('should not increment the revision if an excluded param is updated', () => { | ||
| expect( | ||
| incrementRevision( | ||
| currentRule, | ||
| // @ts-expect-error | ||
| { data: {} }, | ||
| { isSnoozedUntil: '1970-01-02T00:00:00.000Z' } | ||
| ) | ||
| ).toBe(0); | ||
| }); | ||
| }); |
38 changes: 38 additions & 0 deletions
38
x-pack/plugins/alerting/server/rules_client/lib/increment_revision.ts
This file contains 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the Elastic License | ||
| * 2.0; you may not use this file except in compliance with the Elastic License | ||
| * 2.0. | ||
| */ | ||
|
|
||
| import { SavedObject } from '@kbn/core/server'; | ||
| import { get, isEqual } from 'lodash'; | ||
| import { RawRule, RuleTypeParams } from '../../types'; | ||
| import { fieldsToExcludeFromRevisionUpdates, UpdateOptions } from '..'; | ||
|
|
||
| export function incrementRevision<Params extends RuleTypeParams>( | ||
| currentRule: SavedObject<RawRule>, | ||
| { data }: UpdateOptions<Params>, | ||
| updatedParams: RuleTypeParams | ||
| ): number { | ||
| // Diff root level attrs | ||
| for (const [field, value] of Object.entries(data).filter(([key]) => key !== 'params')) { | ||
| if ( | ||
| !fieldsToExcludeFromRevisionUpdates.has(field) && | ||
| !isEqual(value, get(currentRule.attributes, field)) | ||
| ) { | ||
| return currentRule.attributes.revision + 1; | ||
| } | ||
| } | ||
|
|
||
| // Diff rule params | ||
| for (const [field, value] of Object.entries(updatedParams)) { | ||
| if ( | ||
| !fieldsToExcludeFromRevisionUpdates.has(field) && | ||
| !isEqual(value, get(currentRule.attributes.params, field)) | ||
| ) { | ||
| return currentRule.attributes.revision + 1; | ||
| } | ||
| } | ||
| return currentRule.attributes.revision; | ||
| } |
This file contains 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
Oops, something went wrong.