From 542bf21dd0eebb33d5d4a853a79db8e1d2456204 Mon Sep 17 00:00:00 2001 From: Garrett Spong Date: Mon, 12 Dec 2022 15:01:14 -0700 Subject: [PATCH 01/28] Initial plumbing of revision through alerting and security --- x-pack/plugins/alerting/common/rule.ts | 1 + .../rules_client/lib/increment_revision.ts | 18 ++++++++++++++++++ .../alerting/server/rules_client/lib/index.ts | 1 + .../server/rules_client/methods/bulk_edit.ts | 12 +++++++++++- .../server/rules_client/methods/clone.ts | 1 + .../server/rules_client/methods/create.ts | 2 ++ .../server/rules_client/methods/update.ts | 15 +++++++++++++-- .../alerting/server/saved_objects/index.ts | 2 ++ .../alerting/server/saved_objects/mappings.ts | 3 +++ x-pack/plugins/alerting/server/types.ts | 1 + .../rule_schema/model/rule_schemas.ts | 2 ++ .../detection_engine/schemas/common/schemas.ts | 1 + .../logic/actions/duplicate_rule.ts | 3 +++ .../normalization/rule_converters.ts | 1 + 14 files changed, 60 insertions(+), 3 deletions(-) create mode 100644 x-pack/plugins/alerting/server/rules_client/lib/increment_revision.ts diff --git a/x-pack/plugins/alerting/common/rule.ts b/x-pack/plugins/alerting/common/rule.ts index b13c996e5bf78..e2abda3cc4ca5 100644 --- a/x-pack/plugins/alerting/common/rule.ts +++ b/x-pack/plugins/alerting/common/rule.ts @@ -139,6 +139,7 @@ export interface Rule { isSnoozedUntil?: Date | null; lastRun?: RuleLastRun | null; nextRun?: Date | null; + revision: number; } export type SanitizedRule = Omit, 'apiKey'>; diff --git a/x-pack/plugins/alerting/server/rules_client/lib/increment_revision.ts b/x-pack/plugins/alerting/server/rules_client/lib/increment_revision.ts new file mode 100644 index 0000000000000..d17b56861d040 --- /dev/null +++ b/x-pack/plugins/alerting/server/rules_client/lib/increment_revision.ts @@ -0,0 +1,18 @@ +/* + * 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 { RawRule, RuleTypeParams } from '../../types'; + +export function incrementRevision( + currentRule: SavedObject, + updatedParams: RuleTypeParams +): number { + // TODO: Update logic as outlined in https://github.com/elastic/kibana/issues/137164 + // TODO: Potentially streamline with 'skipped logic' being introduced in https://github.com/elastic/kibana/pull/144461 + return currentRule.attributes.revision + 1; +} diff --git a/x-pack/plugins/alerting/server/rules_client/lib/index.ts b/x-pack/plugins/alerting/server/rules_client/lib/index.ts index 1f9534a5c6da2..847e5a612715c 100644 --- a/x-pack/plugins/alerting/server/rules_client/lib/index.ts +++ b/x-pack/plugins/alerting/server/rules_client/lib/index.ts @@ -15,3 +15,4 @@ export { checkAuthorizationAndGetTotal } from './check_authorization_and_get_tot export { scheduleTask } from './schedule_task'; export { createNewAPIKeySet } from './create_new_api_key_set'; export { recoverRuleAlerts } from './recover_rule_alerts'; +export { incrementRevision } from './increment_revision'; diff --git a/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts b/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts index 0ac7c8d24d0fe..e85c659660e2c 100644 --- a/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts +++ b/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts @@ -40,7 +40,13 @@ import { API_KEY_GENERATE_CONCURRENCY, } from '../common/constants'; import { getMappedParams } from '../common/mapped_params_utils'; -import { getAlertFromRaw, extractReferences, validateActions, updateMeta } from '../lib'; +import { + getAlertFromRaw, + extractReferences, + validateActions, + updateMeta, + incrementRevision, +} from '../lib'; import { NormalizedAlertAction, BulkOperationError, @@ -463,12 +469,16 @@ async function bulkEditOcc( attributes.throttle ?? null ); + // Increment revision if applicable field has changed + const revision = incrementRevision(rule, updatedParams); + const updatedAttributes = updateMeta(context, { ...attributes, ...apiKeyAttributes, params: updatedParams as RawRule['params'], actions: rawAlertActions, notifyWhen, + revision, updatedBy: username, updatedAt: new Date().toISOString(), }); diff --git a/x-pack/plugins/alerting/server/rules_client/methods/clone.ts b/x-pack/plugins/alerting/server/rules_client/methods/clone.ts index b4ebe5891885c..0aadbab760e18 100644 --- a/x-pack/plugins/alerting/server/rules_client/methods/clone.ts +++ b/x-pack/plugins/alerting/server/rules_client/methods/clone.ts @@ -111,6 +111,7 @@ export async function clone( mutedInstanceIds: [], executionStatus: getRuleExecutionStatusPending(lastRunTimestamp.toISOString()), monitoring: getDefaultMonitoring(lastRunTimestamp.toISOString()), + revision: 0, // TODO: Clarify if we're resetting revision since it's a new rule, or carrying over from previous rule (existing security solution behavior) scheduledTaskId: null, }; diff --git a/x-pack/plugins/alerting/server/rules_client/methods/create.ts b/x-pack/plugins/alerting/server/rules_client/methods/create.ts index 31707726b4e24..34f350594d1c0 100644 --- a/x-pack/plugins/alerting/server/rules_client/methods/create.ts +++ b/x-pack/plugins/alerting/server/rules_client/methods/create.ts @@ -41,6 +41,7 @@ export interface CreateOptions { | 'isSnoozedUntil' | 'lastRun' | 'nextRun' + | 'revision' > & { actions: NormalizedAlertAction[] }; options?: SavedObjectOptions; } @@ -129,6 +130,7 @@ export async function create( throttle, executionStatus: getRuleExecutionStatusPending(lastRunTimestamp.toISOString()), monitoring: getDefaultMonitoring(lastRunTimestamp.toISOString()), + revision: 0, }; const mappedParams = getMappedParams(updatedParams); diff --git a/x-pack/plugins/alerting/server/rules_client/methods/update.ts b/x-pack/plugins/alerting/server/rules_client/methods/update.ts index 289f5fe007874..74a1d28e589a1 100644 --- a/x-pack/plugins/alerting/server/rules_client/methods/update.ts +++ b/x-pack/plugins/alerting/server/rules_client/methods/update.ts @@ -23,7 +23,13 @@ import { bulkMarkApiKeysForInvalidation } from '../../invalidate_pending_api_key import { ruleAuditEvent, RuleAuditAction } from '../common/audit_events'; import { getMappedParams } from '../common/mapped_params_utils'; import { NormalizedAlertAction, RulesClientContext } from '../types'; -import { validateActions, extractReferences, updateMeta, getPartialRuleFromRaw } from '../lib'; +import { + validateActions, + extractReferences, + updateMeta, + getPartialRuleFromRaw, + incrementRevision, +} from '../lib'; import { generateAPIKeyName, apiKeyAsAlertAttributes } from '../common'; export interface UpdateOptions { @@ -138,8 +144,9 @@ async function updateWithOCC( async function updateAlert( context: RulesClientContext, { id, data }: UpdateOptions, - { attributes, version }: SavedObject + currentRule: SavedObject ): Promise> { + const { attributes, version } = currentRule; const ruleType = context.ruleTypeRegistry.get(attributes.alertTypeId); // Validate @@ -178,6 +185,9 @@ async function updateAlert( const apiKeyAttributes = apiKeyAsAlertAttributes(createdAPIKey, username); const notifyWhen = getRuleNotifyWhenType(data.notifyWhen ?? null, data.throttle ?? null); + // Increment revision if applicable field has changed + const revision = incrementRevision(currentRule, updatedParams); + let updatedObject: SavedObject; const createAttributes = updateMeta(context, { ...attributes, @@ -186,6 +196,7 @@ async function updateAlert( params: updatedParams as RawRule['params'], actions, notifyWhen, + revision, updatedBy: username, updatedAt: new Date().toISOString(), }); diff --git a/x-pack/plugins/alerting/server/saved_objects/index.ts b/x-pack/plugins/alerting/server/saved_objects/index.ts index 76fc730409294..4363cc3a6fc5e 100644 --- a/x-pack/plugins/alerting/server/saved_objects/index.ts +++ b/x-pack/plugins/alerting/server/saved_objects/index.ts @@ -38,6 +38,7 @@ export const AlertAttributesExcludedFromAAD = [ 'isSnoozedUntil', 'lastRun', 'nextRun', + 'revision', ]; // useful for Pick which is a @@ -56,6 +57,7 @@ export type AlertAttributesExcludedFromAADType = | 'snoozeSchedule' | 'isSnoozedUntil' | 'lastRun' + | 'revision' | 'nextRun'; export function setupSavedObjects( diff --git a/x-pack/plugins/alerting/server/saved_objects/mappings.ts b/x-pack/plugins/alerting/server/saved_objects/mappings.ts index a45dc78170ddb..c276fd31cd837 100644 --- a/x-pack/plugins/alerting/server/saved_objects/mappings.ts +++ b/x-pack/plugins/alerting/server/saved_objects/mappings.ts @@ -197,6 +197,9 @@ export const alertMappings: SavedObjectsTypeMappingDefinition = { }, }, }, + revision: { + type: 'long', + }, snoozeSchedule: { type: 'nested', properties: { diff --git a/x-pack/plugins/alerting/server/types.ts b/x-pack/plugins/alerting/server/types.ts index c2af1555cfa36..f399dfa8810db 100644 --- a/x-pack/plugins/alerting/server/types.ts +++ b/x-pack/plugins/alerting/server/types.ts @@ -284,6 +284,7 @@ export interface RawRule extends SavedObjectAttributes { isSnoozedUntil?: string | null; lastRun?: RawRuleLastRun | null; nextRun?: string | null; + revision: number; } export interface AlertingPlugin { diff --git a/x-pack/plugins/security_solution/common/detection_engine/rule_schema/model/rule_schemas.ts b/x-pack/plugins/security_solution/common/detection_engine/rule_schema/model/rule_schemas.ts index 5d35811368b39..e6e1a384df183 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/rule_schema/model/rule_schemas.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/rule_schema/model/rule_schemas.ts @@ -36,6 +36,7 @@ import { updated_by, created_at, created_by, + revision, } from '../../schemas/common'; import { @@ -154,6 +155,7 @@ const responseRequiredFields = { updated_by, created_at, created_by, + revision, // NOTE: For now, Related Integrations, Required Fields and Setup Guide are supported for prebuilt // rules only. We don't want to allow users to edit these 3 fields via the API. If we added them diff --git a/x-pack/plugins/security_solution/common/detection_engine/schemas/common/schemas.ts b/x-pack/plugins/security_solution/common/detection_engine/schemas/common/schemas.ts index 52ba9e06622d4..b6f90f4cf7025 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/schemas/common/schemas.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/schemas/common/schemas.ts @@ -59,6 +59,7 @@ export const status_code = PositiveInteger; export const message = t.string; export const perPage = PositiveInteger; export const total = PositiveInteger; +export const revision = PositiveInteger; export const success = t.boolean; export const success_count = PositiveInteger; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.ts index f5ee4fc8ae35d..394aa5c296c1a 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.ts @@ -34,6 +34,9 @@ export const duplicateRule = async ({ rule }: DuplicateRuleParams): Promise Date: Mon, 12 Dec 2022 18:25:15 -0700 Subject: [PATCH 02/28] Adding revision to FTRs --- .../security_and_spaces/group1/tests/alerting/create.ts | 1 + .../security_and_spaces/group1/tests/alerting/find.ts | 2 ++ .../security_and_spaces/group1/tests/alerting/get.ts | 1 + .../security_and_spaces/group3/tests/alerting/bulk_enable.ts | 1 + .../spaces_only/tests/alerting/create.ts | 1 + .../alerting_api_integration/spaces_only/tests/alerting/find.ts | 1 + .../alerting_api_integration/spaces_only/tests/alerting/get.ts | 2 ++ .../spaces_only/tests/alerting/update.ts | 1 + 8 files changed, 10 insertions(+) diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/group1/tests/alerting/create.ts b/x-pack/test/alerting_api_integration/security_and_spaces/group1/tests/alerting/create.ts index cebfe68e279b0..191a67c1f6edc 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/group1/tests/alerting/create.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/group1/tests/alerting/create.ts @@ -124,6 +124,7 @@ export default function createAlertTests({ getService }: FtrProviderContext) { mute_all: false, muted_alert_ids: [], execution_status: response.body.execution_status, + revision: 0, ...(response.body.next_run ? { next_run: response.body.next_run } : {}), ...(response.body.last_run ? { last_run: response.body.last_run } : {}), }); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/group1/tests/alerting/find.ts b/x-pack/test/alerting_api_integration/security_and_spaces/group1/tests/alerting/find.ts index b8460e84202c9..cb867af42d1c0 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/group1/tests/alerting/find.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/group1/tests/alerting/find.ts @@ -83,6 +83,7 @@ const findTestUtils = ( api_key_owner: 'elastic', mute_all: false, muted_alert_ids: [], + revision: 0, execution_status: match.execution_status, ...(match.next_run ? { next_run: match.next_run } : {}), ...(match.last_run ? { last_run: match.last_run } : {}), @@ -293,6 +294,7 @@ const findTestUtils = ( created_at: match.created_at, updated_at: match.updated_at, execution_status: match.execution_status, + revision: 0, ...(match.next_run ? { next_run: match.next_run } : {}), ...(match.last_run ? { last_run: match.last_run } : {}), ...(describeType === 'internal' diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/group1/tests/alerting/get.ts b/x-pack/test/alerting_api_integration/security_and_spaces/group1/tests/alerting/get.ts index b0900f74993cb..b18eafc099665 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/group1/tests/alerting/get.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/group1/tests/alerting/get.ts @@ -81,6 +81,7 @@ const getTestUtils = ( mute_all: false, muted_alert_ids: [], execution_status: response.body.execution_status, + revision: 0, ...(response.body.next_run ? { next_run: response.body.next_run } : {}), ...(response.body.last_run ? { last_run: response.body.last_run } : {}), ...(describeType === 'internal' diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/group3/tests/alerting/bulk_enable.ts b/x-pack/test/alerting_api_integration/security_and_spaces/group3/tests/alerting/bulk_enable.ts index eaabc53cd1dc9..c75df9bfa57e2 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/group3/tests/alerting/bulk_enable.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/group3/tests/alerting/bulk_enable.ts @@ -97,6 +97,7 @@ export default ({ getService }: FtrProviderContext) => { snoozeSchedule: [], updatedAt: response.body.rules[0].updatedAt, createdAt: response.body.rules[0].createdAt, + revision: 0, scheduledTaskId: response.body.rules[0].scheduledTaskId, executionStatus: { lastDuration: 0, diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/create.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/create.ts index 5cc7316f1c6e3..4b76acd51f88b 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/create.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/create.ts @@ -495,6 +495,7 @@ export default function createAlertTests({ getService }: FtrProviderContext) { createdAt: response.body.createdAt, updatedAt: response.body.updatedAt, executionStatus: response.body.executionStatus, + revision: 0, ...(response.body.next_run ? { next_run: response.body.next_run } : {}), ...(response.body.last_run ? { last_run: response.body.last_run } : {}), }); diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/find.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/find.ts index d10518aca575f..00fc2312c553b 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/find.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/find.ts @@ -327,6 +327,7 @@ export default function createFindTests({ getService }: FtrProviderContext) { createdAt: match.createdAt, updatedAt: match.updatedAt, executionStatus: match.executionStatus, + revision: 0, ...(match.nextRun ? { nextRun: match.nextRun } : {}), ...(match.lastRun ? { lastRun: match.lastRun } : {}), }); diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/get.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/get.ts index c91467f698dc1..447ab8412ae4e 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/get.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/get.ts @@ -54,6 +54,7 @@ const getTestUtils = ( created_at: response.body.created_at, updated_at: response.body.updated_at, execution_status: response.body.execution_status, + revision: 0, ...(response.body.next_run ? { next_run: response.body.next_run } : {}), ...(response.body.last_run ? { last_run: response.body.last_run } : {}), ...(describeType === 'internal' @@ -154,6 +155,7 @@ export default function createGetTests({ getService }: FtrProviderContext) { createdAt: response.body.createdAt, updatedAt: response.body.updatedAt, executionStatus: response.body.executionStatus, + revision: 0, ...(response.body.nextRun ? { nextRun: response.body.nextRun } : {}), ...(response.body.lastRun ? { lastRun: response.body.lastRun } : {}), }); diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/update.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/update.ts index 4c740b3be9b97..7e0ae082d335f 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/update.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/update.ts @@ -168,6 +168,7 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { createdAt: response.body.createdAt, updatedAt: response.body.updatedAt, executionStatus: response.body.executionStatus, + revision: 1, ...(response.body.nextRun ? { nextRun: response.body.nextRun } : {}), ...(response.body.lastRun ? { lastRun: response.body.lastRun } : {}), }); From 6d3aa3d051da18af11c4e44567b4be7f5fa250a8 Mon Sep 17 00:00:00 2001 From: Garrett Spong Date: Mon, 12 Dec 2022 20:00:23 -0700 Subject: [PATCH 03/28] Fixing typechecks and more FTRs --- x-pack/plugins/alerting/public/alert_api.test.ts | 1 + .../alerting/public/lib/common_transformations.test.ts | 2 ++ .../alerting/server/lib/alert_summary_from_event_log.test.ts | 1 + .../plugins/alerting/server/routes/bulk_edit_rules.test.ts | 1 + x-pack/plugins/alerting/server/routes/clone_rule.test.ts | 2 ++ x-pack/plugins/alerting/server/routes/create_rule.test.ts | 2 ++ x-pack/plugins/alerting/server/routes/get_rule.test.ts | 2 ++ x-pack/plugins/alerting/server/routes/legacy/create.test.ts | 1 + x-pack/plugins/alerting/server/routes/legacy/get.test.ts | 1 + x-pack/plugins/alerting/server/routes/resolve_rule.test.ts | 2 ++ .../server/rules_client/tests/get_action_error_log.test.ts | 1 + .../server/rules_client/tests/get_alert_summary.test.ts | 1 + .../server/rules_client/tests/get_execution_log.test.ts | 1 + .../alerting/server/task_runner/alert_task_instance.test.ts | 1 + x-pack/plugins/alerting/server/task_runner/fixtures.ts | 1 + .../rule_schema/model/rule_response_schema.mock.ts | 1 + .../security_and_spaces/group2/tests/alerting/update.ts | 5 +++++ .../security_and_spaces/group3/tests/alerting/bulk_delete.ts | 1 + .../group3/tests/alerting/bulk_disable.ts | 1 + .../spaces_only/tests/alerting/create.ts | 1 + .../spaces_only/tests/alerting/find.ts | 1 + 21 files changed, 30 insertions(+) diff --git a/x-pack/plugins/alerting/public/alert_api.test.ts b/x-pack/plugins/alerting/public/alert_api.test.ts index 2f09643e499e3..429e5f1c44e0c 100644 --- a/x-pack/plugins/alerting/public/alert_api.test.ts +++ b/x-pack/plugins/alerting/public/alert_api.test.ts @@ -330,5 +330,6 @@ function getRule(): Rule<{ x: number }> { lastExecutionDate: RuleExecuteDate, lastDuration: 1194, }, + revision: 0, }; } diff --git a/x-pack/plugins/alerting/public/lib/common_transformations.test.ts b/x-pack/plugins/alerting/public/lib/common_transformations.test.ts index 6b7026f1ea593..69fd0b7a6d94d 100644 --- a/x-pack/plugins/alerting/public/lib/common_transformations.test.ts +++ b/x-pack/plugins/alerting/public/lib/common_transformations.test.ts @@ -45,6 +45,7 @@ describe('common_transformations', () => { notify_when: 'onActiveAlert', mute_all: false, muted_alert_ids: ['bob', 'jim'], + revision: 0, execution_status: { last_execution_date: dateExecuted.toISOString(), last_duration: 42, @@ -222,6 +223,7 @@ describe('common_transformations', () => { notify_when: 'onActiveAlert', mute_all: false, muted_alert_ids: ['bob', 'jim'], + revision: 0, execution_status: { last_execution_date: dateExecuted.toISOString(), status: 'error', diff --git a/x-pack/plugins/alerting/server/lib/alert_summary_from_event_log.test.ts b/x-pack/plugins/alerting/server/lib/alert_summary_from_event_log.test.ts index 3bf01caaead1a..fc6518a5d0960 100644 --- a/x-pack/plugins/alerting/server/lib/alert_summary_from_event_log.test.ts +++ b/x-pack/plugins/alerting/server/lib/alert_summary_from_event_log.test.ts @@ -784,4 +784,5 @@ const BaseRule: SanitizedRule<{ bar: boolean }> = { status: 'unknown', lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, + revision: 0, }; diff --git a/x-pack/plugins/alerting/server/routes/bulk_edit_rules.test.ts b/x-pack/plugins/alerting/server/routes/bulk_edit_rules.test.ts index b70e4734ab4ff..9a531ff9c19af 100644 --- a/x-pack/plugins/alerting/server/routes/bulk_edit_rules.test.ts +++ b/x-pack/plugins/alerting/server/routes/bulk_edit_rules.test.ts @@ -58,6 +58,7 @@ describe('bulkEditInternalRulesRoute', () => { status: 'unknown', lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, + revision: 0, }; const mockedAlerts: Array> = [mockedAlert]; diff --git a/x-pack/plugins/alerting/server/routes/clone_rule.test.ts b/x-pack/plugins/alerting/server/routes/clone_rule.test.ts index 7b89f217f9c8f..ec5f963b67e6e 100644 --- a/x-pack/plugins/alerting/server/routes/clone_rule.test.ts +++ b/x-pack/plugins/alerting/server/routes/clone_rule.test.ts @@ -64,6 +64,7 @@ describe('cloneRuleRoute', () => { status: 'unknown', lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, + revision: 0, }; const ruleToClone: AsApiContract['data']> = { @@ -89,6 +90,7 @@ describe('cloneRuleRoute', () => { created_at: mockedRule.createdAt, updated_at: mockedRule.updatedAt, id: mockedRule.id, + revision: 0, // TODO: Finalize clone rule behavior. Clone or reset revision? execution_status: { status: mockedRule.executionStatus.status, last_execution_date: mockedRule.executionStatus.lastExecutionDate, diff --git a/x-pack/plugins/alerting/server/routes/create_rule.test.ts b/x-pack/plugins/alerting/server/routes/create_rule.test.ts index cbb7965cab081..8704220373dd4 100644 --- a/x-pack/plugins/alerting/server/routes/create_rule.test.ts +++ b/x-pack/plugins/alerting/server/routes/create_rule.test.ts @@ -67,6 +67,7 @@ describe('createRuleRoute', () => { status: 'unknown', lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, + revision: 0, }; const ruleToCreate: AsApiContract['data']> = { @@ -92,6 +93,7 @@ describe('createRuleRoute', () => { created_at: mockedAlert.createdAt, updated_at: mockedAlert.updatedAt, id: mockedAlert.id, + revision: mockedAlert.revision, execution_status: { status: mockedAlert.executionStatus.status, last_execution_date: mockedAlert.executionStatus.lastExecutionDate, diff --git a/x-pack/plugins/alerting/server/routes/get_rule.test.ts b/x-pack/plugins/alerting/server/routes/get_rule.test.ts index 065cd567f1f69..6f83e4cc6ce44 100644 --- a/x-pack/plugins/alerting/server/routes/get_rule.test.ts +++ b/x-pack/plugins/alerting/server/routes/get_rule.test.ts @@ -61,6 +61,7 @@ describe('getRuleRoute', () => { status: 'unknown', lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, + revision: 0, }; const getResult: AsApiContract> = { @@ -75,6 +76,7 @@ describe('getRuleRoute', () => { created_at: mockedAlert.createdAt, updated_at: mockedAlert.updatedAt, id: mockedAlert.id, + revision: mockedAlert.revision, execution_status: { status: mockedAlert.executionStatus.status, last_execution_date: mockedAlert.executionStatus.lastExecutionDate, diff --git a/x-pack/plugins/alerting/server/routes/legacy/create.test.ts b/x-pack/plugins/alerting/server/routes/legacy/create.test.ts index 60b299be6db97..548015ef55f03 100644 --- a/x-pack/plugins/alerting/server/routes/legacy/create.test.ts +++ b/x-pack/plugins/alerting/server/routes/legacy/create.test.ts @@ -80,6 +80,7 @@ describe('createAlertRoute', () => { status: 'unknown', lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, + revision: 0, }; it('creates an alert with proper parameters', async () => { diff --git a/x-pack/plugins/alerting/server/routes/legacy/get.test.ts b/x-pack/plugins/alerting/server/routes/legacy/get.test.ts index 6860140ce2540..403f7a5b42ac8 100644 --- a/x-pack/plugins/alerting/server/routes/legacy/get.test.ts +++ b/x-pack/plugins/alerting/server/routes/legacy/get.test.ts @@ -66,6 +66,7 @@ describe('getAlertRoute', () => { status: 'unknown', lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, + revision: 0, }; it('gets an alert with proper parameters', async () => { diff --git a/x-pack/plugins/alerting/server/routes/resolve_rule.test.ts b/x-pack/plugins/alerting/server/routes/resolve_rule.test.ts index 656874b5cf332..646ea3f5ec135 100644 --- a/x-pack/plugins/alerting/server/routes/resolve_rule.test.ts +++ b/x-pack/plugins/alerting/server/routes/resolve_rule.test.ts @@ -63,6 +63,7 @@ describe('resolveRuleRoute', () => { }, outcome: 'aliasMatch', alias_target_id: '2', + revision: 0, }; const resolveResult: AsApiContract> = { @@ -87,6 +88,7 @@ describe('resolveRuleRoute', () => { created_at: mockedRule.createdAt, updated_at: mockedRule.updatedAt, id: mockedRule.id, + revision: mockedRule.revision, execution_status: { status: mockedRule.executionStatus.status, last_execution_date: mockedRule.executionStatus.lastExecutionDate, diff --git a/x-pack/plugins/alerting/server/rules_client/tests/get_action_error_log.test.ts b/x-pack/plugins/alerting/server/rules_client/tests/get_action_error_log.test.ts index 672281469d18b..90d68301f97aa 100644 --- a/x-pack/plugins/alerting/server/rules_client/tests/get_action_error_log.test.ts +++ b/x-pack/plugins/alerting/server/rules_client/tests/get_action_error_log.test.ts @@ -90,6 +90,7 @@ const BaseRuleSavedObject: SavedObject = { error: null, warning: null, }, + revision: 0, }, references: [], }; diff --git a/x-pack/plugins/alerting/server/rules_client/tests/get_alert_summary.test.ts b/x-pack/plugins/alerting/server/rules_client/tests/get_alert_summary.test.ts index f0f634538d6f7..b8932d5a29071 100644 --- a/x-pack/plugins/alerting/server/rules_client/tests/get_alert_summary.test.ts +++ b/x-pack/plugins/alerting/server/rules_client/tests/get_alert_summary.test.ts @@ -94,6 +94,7 @@ const BaseRuleSavedObject: SavedObject = { error: null, warning: null, }, + revision: 0, }, references: [], }; diff --git a/x-pack/plugins/alerting/server/rules_client/tests/get_execution_log.test.ts b/x-pack/plugins/alerting/server/rules_client/tests/get_execution_log.test.ts index ffc40cd705abd..3f8dfd92ee894 100644 --- a/x-pack/plugins/alerting/server/rules_client/tests/get_execution_log.test.ts +++ b/x-pack/plugins/alerting/server/rules_client/tests/get_execution_log.test.ts @@ -91,6 +91,7 @@ const BaseRuleSavedObject: SavedObject = { error: null, warning: null, }, + revision: 0, }, references: [], }; diff --git a/x-pack/plugins/alerting/server/task_runner/alert_task_instance.test.ts b/x-pack/plugins/alerting/server/task_runner/alert_task_instance.test.ts index 3faea1a4d2261..b5cef774d9a1c 100644 --- a/x-pack/plugins/alerting/server/task_runner/alert_task_instance.test.ts +++ b/x-pack/plugins/alerting/server/task_runner/alert_task_instance.test.ts @@ -37,6 +37,7 @@ const alert: SanitizedRule<{ status: 'unknown', lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, + revision: 0, }; describe('Alert Task Instance', () => { diff --git a/x-pack/plugins/alerting/server/task_runner/fixtures.ts b/x-pack/plugins/alerting/server/task_runner/fixtures.ts index 38ebd756d0aeb..7850134c5b5b3 100644 --- a/x-pack/plugins/alerting/server/task_runner/fixtures.ts +++ b/x-pack/plugins/alerting/server/task_runner/fixtures.ts @@ -191,6 +191,7 @@ export const mockedRuleTypeSavedObject: Rule = { lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, monitoring: getDefaultMonitoring('2020-08-20T19:23:38Z'), + revision: 0, }; export const mockTaskInstance = () => ({ diff --git a/x-pack/plugins/security_solution/common/detection_engine/rule_schema/model/rule_response_schema.mock.ts b/x-pack/plugins/security_solution/common/detection_engine/rule_schema/model/rule_response_schema.mock.ts index 86018ff1a7b88..7ccdd03b1355c 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/rule_schema/model/rule_response_schema.mock.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/rule_schema/model/rule_response_schema.mock.ts @@ -31,6 +31,7 @@ const getResponseBaseParams = (anchorDate: string = ANCHOR_DATE): SharedResponse immutable: false, name: 'Query with a rule id', references: ['test 1', 'test 2'], + revision: 0, severity: 'high' as const, severity_mapping: [], updated_by: 'elastic_kibana', diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/update.ts b/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/update.ts index 430d69274041a..23335b565fc9c 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/update.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/update.ts @@ -132,6 +132,7 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { created_at: response.body.created_at, updated_at: response.body.updated_at, execution_status: response.body.execution_status, + revision: 1, ...(response.body.next_run ? { next_run: response.body.next_run } : {}), ...(response.body.last_run ? { last_run: response.body.last_run } : {}), }); @@ -221,6 +222,7 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { created_at: response.body.created_at, updated_at: response.body.updated_at, execution_status: response.body.execution_status, + revision: 1, ...(response.body.next_run ? { next_run: response.body.next_run } : {}), ...(response.body.last_run ? { last_run: response.body.last_run } : {}), }); @@ -321,6 +323,7 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { created_at: response.body.created_at, updated_at: response.body.updated_at, execution_status: response.body.execution_status, + revision: 1, ...(response.body.next_run ? { next_run: response.body.next_run } : {}), ...(response.body.last_run ? { last_run: response.body.last_run } : {}), }); @@ -421,6 +424,7 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { created_at: response.body.created_at, updated_at: response.body.updated_at, execution_status: response.body.execution_status, + revision: 1, ...(response.body.next_run ? { next_run: response.body.next_run } : {}), ...(response.body.last_run ? { last_run: response.body.last_run } : {}), }); @@ -519,6 +523,7 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { created_at: response.body.created_at, updated_at: response.body.updated_at, execution_status: response.body.execution_status, + revision: 1, ...(response.body.next_run ? { next_run: response.body.next_run } : {}), ...(response.body.last_run ? { last_run: response.body.last_run } : {}), }); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/group3/tests/alerting/bulk_delete.ts b/x-pack/test/alerting_api_integration/security_and_spaces/group3/tests/alerting/bulk_delete.ts index b69b62e4b4a40..61bdfa2f78717 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/group3/tests/alerting/bulk_delete.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/group3/tests/alerting/bulk_delete.ts @@ -34,6 +34,7 @@ const getDefaultRules = (response: any) => ({ scheduledTaskId: response.body.rules[0].scheduledTaskId, executionStatus: response.body.rules[0].executionStatus, monitoring: response.body.rules[0].monitoring, + revision: 0, ...(response.body.rules[0].nextRun ? { nextRun: response.body.rules[0].nextRun } : {}), ...(response.body.rules[0].lastRun ? { lastRun: response.body.rules[0].lastRun } : {}), }); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/group3/tests/alerting/bulk_disable.ts b/x-pack/test/alerting_api_integration/security_and_spaces/group3/tests/alerting/bulk_disable.ts index 74fd9d6a695ba..29d45a3b495a3 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/group3/tests/alerting/bulk_disable.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/group3/tests/alerting/bulk_disable.ts @@ -33,6 +33,7 @@ const getDefaultRules = (response: any) => ({ scheduledTaskId: response.body.rules[0].scheduledTaskId, executionStatus: response.body.rules[0].executionStatus, monitoring: response.body.rules[0].monitoring, + revision: 0, ...(response.body.rules[0].nextRun ? { nextRun: response.body.rules[0].nextRun } : {}), ...(response.body.rules[0].lastRun ? { lastRun: response.body.rules[0].lastRun } : {}), }); diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/create.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/create.ts index 4b76acd51f88b..b4e2073321d29 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/create.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/create.ts @@ -94,6 +94,7 @@ export default function createAlertTests({ getService }: FtrProviderContext) { created_at: response.body.created_at, updated_at: response.body.updated_at, execution_status: response.body.execution_status, + revision: 0, ...(response.body.next_run ? { next_run: response.body.next_run } : {}), ...(response.body.last_run ? { last_run: response.body.last_run } : {}), }); diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/find.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/find.ts index 00fc2312c553b..e81e485d3d87b 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/find.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/find.ts @@ -75,6 +75,7 @@ const findTestUtils = ( created_at: match.created_at, updated_at: match.updated_at, execution_status: match.execution_status, + revision: 0, ...(match.next_run ? { next_run: match.next_run } : {}), ...(match.last_run ? { last_run: match.last_run } : {}), ...(describeType === 'internal' From 9b2813436ea4124881fde7f6a20ca60339b71905 Mon Sep 17 00:00:00 2001 From: Garrett Spong Date: Tue, 13 Dec 2022 12:18:09 -0700 Subject: [PATCH 04/28] More typecheck fixes throughout triggers and actions ui --- .../rule_management/logic/actions/duplicate_rule.test.ts | 1 + .../public/application/lib/rule_api/create.test.ts | 1 + .../public/application/lib/rule_api/update.test.ts | 2 ++ .../public/application/lib/value_validators.test.ts | 4 ++++ .../application/mock/rule_details/alert_summary/index.ts | 1 + .../common/components/with_bulk_rule_api_operations.test.tsx | 1 + .../rule_details/components/rule_actions_popover.test.tsx | 1 + .../sections/rule_details/components/rule_definition.test.tsx | 1 + .../sections/rule_details/components/rule_details.test.tsx | 1 + .../rule_details/components/rule_details_route.test.tsx | 1 + .../sections/rule_details/components/rule_error_log.test.tsx | 1 + .../sections/rule_details/components/rule_route.test.tsx | 1 + .../sections/rule_details/components/test_helpers.ts | 1 + .../sections/rule_details/components/view_in_app.test.tsx | 1 + .../public/application/sections/rule_form/rule_add.test.tsx | 1 + .../public/application/sections/rule_form/rule_edit.test.tsx | 1 + .../application/sections/rule_form/rule_errors.test.tsx | 1 + .../rules_list/components/collapsed_item_actions.test.tsx | 1 + .../rules_list/components/rule_enabled_switch.test.tsx | 2 ++ .../rules_list/components/rules_list_notify_badge.test.tsx | 1 + 20 files changed, 25 insertions(+) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.test.ts index a36746623dbcd..ea6159197a1db 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.test.ts @@ -76,6 +76,7 @@ describe('duplicateRule', () => { mutedInstanceIds: [], updatedAt: new Date(2021, 0), createdAt: new Date(2021, 0), + revision: 0, scheduledTaskId: undefined, executionStatus: { lastExecutionDate: new Date(2021, 0), diff --git a/x-pack/plugins/triggers_actions_ui/public/application/lib/rule_api/create.test.ts b/x-pack/plugins/triggers_actions_ui/public/application/lib/rule_api/create.test.ts index d3138a115a27b..7e7f01877f123 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/lib/rule_api/create.test.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/lib/rule_api/create.test.ts @@ -87,6 +87,7 @@ describe('createRule', () => { createdAt: new Date('2021-04-01T21:33:13.247Z'), updatedAt: new Date('2021-04-01T21:33:13.247Z'), apiKeyOwner: '', + revision: 0, }; http.post.mockResolvedValueOnce(resolvedValue); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/lib/rule_api/update.test.ts b/x-pack/plugins/triggers_actions_ui/public/application/lib/rule_api/update.test.ts index 29bf8027bfe47..a313fed9e4ea2 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/lib/rule_api/update.test.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/lib/rule_api/update.test.ts @@ -28,6 +28,7 @@ describe('updateRule', () => { apiKey: null, apiKeyOwner: null, notifyWhen: 'onThrottleInterval' as RuleNotifyWhenType, + revision: 0, }; const resolvedValue: Rule = { ...ruleToUpdate, @@ -42,6 +43,7 @@ describe('updateRule', () => { status: 'unknown', lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, + revision: 1, }; http.put.mockResolvedValueOnce(resolvedValue); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/lib/value_validators.test.ts b/x-pack/plugins/triggers_actions_ui/public/application/lib/value_validators.test.ts index 6587fc669b97e..50abb717e30e6 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/lib/value_validators.test.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/lib/value_validators.test.ts @@ -275,6 +275,7 @@ describe('getRuleWithInvalidatedFields', () => { throttle: '', updatedAt: new Date(), updatedBy: '', + revision: 0, }; const baseAlertErrors = {}; const actionsErrors: IErrorObject[] = []; @@ -313,6 +314,7 @@ describe('getRuleWithInvalidatedFields', () => { throttle: '', updatedAt: new Date(), updatedBy: '', + revision: 0, }; const baseAlertErrors = {}; const actionsErrors: IErrorObject[] = []; @@ -362,6 +364,7 @@ describe('getRuleWithInvalidatedFields', () => { throttle: '', updatedAt: new Date(), updatedBy: '', + revision: 0, }; const baseAlertErrors = {}; const actionsErrors = [{ 'incident.field.name': ['Name is required.'] }]; @@ -419,6 +422,7 @@ describe('getRuleWithInvalidatedFields', () => { throttle: '', updatedAt: new Date(), updatedBy: '', + revision: 0, }; const baseAlertErrors = {}; const actionsErrors = [ diff --git a/x-pack/plugins/triggers_actions_ui/public/application/mock/rule_details/alert_summary/index.ts b/x-pack/plugins/triggers_actions_ui/public/application/mock/rule_details/alert_summary/index.ts index 0c00abae8d855..6645531ab223e 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/mock/rule_details/alert_summary/index.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/mock/rule_details/alert_summary/index.ts @@ -27,6 +27,7 @@ export const mockRule = (): Rule => { updatedAt: new Date(), consumer: 'alerts', notifyWhen: 'onActiveAlert', + revision: 0, executionStatus: { status: 'active', lastDuration: 500, diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/common/components/with_bulk_rule_api_operations.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/common/components/with_bulk_rule_api_operations.test.tsx index 989a27905cf51..f0d0be3dd9517 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/common/components/with_bulk_rule_api_operations.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/common/components/with_bulk_rule_api_operations.test.tsx @@ -342,6 +342,7 @@ function mockRule(overloads: Partial = {}): Rule { status: 'unknown', lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, + revision: 0, ...overloads, }; } diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule_actions_popover.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule_actions_popover.test.tsx index 6fa62883e3aff..f981fd07a1b54 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule_actions_popover.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule_actions_popover.test.tsx @@ -40,6 +40,7 @@ describe('rule_actions_popover', () => { status: 'unknown', lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, + revision: 0, ...overloads, }; } diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule_definition.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule_definition.test.tsx index 6f215e139adf0..148d0994fc497 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule_definition.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule_definition.test.tsx @@ -215,6 +215,7 @@ function mockRule(overwrite = {}): Rule { updatedAt: new Date(), consumer: 'alerts', notifyWhen: 'onActiveAlert', + revision: 0, executionStatus: { status: 'active', lastDuration: 500, diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule_details.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule_details.test.tsx index 40893f892cd9e..f95348d1a86ec 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule_details.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule_details.test.tsx @@ -813,6 +813,7 @@ describe('rule_details', () => { status: 'unknown', lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, + revision: 0, ...overloads, }; } diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule_details_route.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule_details_route.test.tsx index 46d127479031f..e510384a333dc 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule_details_route.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule_details_route.test.tsx @@ -494,6 +494,7 @@ function mockRule(overloads: Partial = {}): Rule { status: 'unknown', lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, + revision: 0, ...overloads, }; } diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule_error_log.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule_error_log.test.tsx index 192cf39929c88..8aa8ad0e5bf44 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule_error_log.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule_error_log.test.tsx @@ -117,6 +117,7 @@ const mockRule: Rule = { status: 'unknown', lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, + revision: 0, }; const loadActionErrorLogMock = jest.fn(); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule_route.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule_route.test.tsx index a29fc2a2ee9a3..96f0ec60107ff 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule_route.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule_route.test.tsx @@ -125,6 +125,7 @@ function mockRule(overloads: Partial = {}): Rule { status: 'unknown', lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, + revision: 0, ...overloads, }; } diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/test_helpers.ts b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/test_helpers.ts index b7a6876535a64..26aec74dce6fe 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/test_helpers.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/test_helpers.ts @@ -54,6 +54,7 @@ export function mockRule(overloads: Partial = {}): Rule { notifyWhen: null, muteAll: false, mutedInstanceIds: [], + revision: 0, executionStatus: { status: 'unknown', lastExecutionDate: new Date('2020-08-20T19:23:38Z'), diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/view_in_app.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/view_in_app.test.tsx index ee29facf60579..e3c52f7814011 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/view_in_app.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/view_in_app.test.tsx @@ -93,6 +93,7 @@ function mockRule(overloads: Partial = {}): Rule { status: 'unknown', lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, + revision: 0, ...overloads, }; } diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_add.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_add.test.tsx index 6735bb5637e74..378087c0eb356 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_add.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_add.test.tsx @@ -390,6 +390,7 @@ function mockRule(overloads: Partial = {}): Rule { status: 'unknown', lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, + revision: 0, ...overloads, }; } diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_edit.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_edit.test.tsx index 5830ab2bd9312..9306d9375722c 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_edit.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_edit.test.tsx @@ -170,6 +170,7 @@ describe('rule_edit', () => { status: 'unknown', lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, + revision: 0, ...initialRuleFields, }; actionTypeRegistry.get.mockReturnValueOnce(actionTypeModel); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_errors.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_errors.test.tsx index 3453728862b5e..0e99a2b0ba8a7 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_errors.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_errors.test.tsx @@ -267,6 +267,7 @@ function mockRule(overloads: Partial = {}): Rule { status: 'unknown', lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, + revision: 0, ...overloads, }; } diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rules_list/components/collapsed_item_actions.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rules_list/components/collapsed_item_actions.test.tsx index 5d01ebd3600d3..e79c2236e1910 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rules_list/components/collapsed_item_actions.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rules_list/components/collapsed_item_actions.test.tsx @@ -83,6 +83,7 @@ describe('CollapsedItemActions', () => { ruleType: 'Test Rule Type', isEditable: true, enabledInLicense: true, + revision: 0, ...overrides, }; diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rules_list/components/rule_enabled_switch.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rules_list/components/rule_enabled_switch.test.tsx index 418b6931ca1a3..6796b741fc0c1 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rules_list/components/rule_enabled_switch.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rules_list/components/rule_enabled_switch.test.tsx @@ -42,6 +42,7 @@ describe('RuleEnabledSwitch', () => { notifyWhen: null, index: 0, updatedAt: new Date('2020-08-20T19:23:38Z'), + revision: 0, }, onRuleChanged: jest.fn(), }; @@ -86,6 +87,7 @@ describe('RuleEnabledSwitch', () => { notifyWhen: null, index: 0, updatedAt: new Date('2020-08-20T19:23:38Z'), + revision: 0, }, }} /> diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rules_list/components/rules_list_notify_badge.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rules_list/components/rules_list_notify_badge.test.tsx index ca22f042ca329..1a0894e40a879 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rules_list/components/rules_list_notify_badge.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rules_list/components/rules_list_notify_badge.test.tsx @@ -53,6 +53,7 @@ const getRule = (overrides = {}): RuleTableItem => ({ ruleType: 'Test Rule Type', isEditable: true, enabledInLicense: true, + revision: 0, ...overrides, }); From 96f386b63a32155196eb86468e180d7cf14f8dc5 Mon Sep 17 00:00:00 2001 From: Garrett Spong Date: Tue, 13 Dec 2022 12:53:27 -0700 Subject: [PATCH 05/28] More typecheck fixes throughout security --- .../detection_engine/routes/__mocks__/request_responses.ts | 5 +++++ .../server/lib/detection_engine/routes/__mocks__/utils.ts | 1 + .../logic/export/get_export_by_object_ids.test.ts | 1 + .../detection_engine/rule_management/utils/validate.test.ts | 1 + .../lib/detection_engine/signals/__mocks__/es_results.ts | 1 + 5 files changed, 9 insertions(+) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/request_responses.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/request_responses.ts index 7b6c85d486638..bfa4413a06004 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/request_responses.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/request_responses.ts @@ -367,6 +367,7 @@ export const getRuleMock = (params: T): SanitizedRule = status: 'unknown', lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, + revision: 0, }); export const resolveRuleMock = (params: T): ResolvedSanitizedRule => ({ @@ -544,6 +545,7 @@ export const legacyGetNotificationResult = ({ status: 'unknown', lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, + revision: 0, }); /** @@ -592,6 +594,7 @@ export const legacyGetHourlyNotificationResult = ( status: 'unknown', lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, + revision: 0, }); /** @@ -640,6 +643,7 @@ export const legacyGetDailyNotificationResult = ( status: 'unknown', lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, + revision: 0, }); /** @@ -688,6 +692,7 @@ export const legacyGetWeeklyNotificationResult = ( status: 'unknown', lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, + revision: 0, }); /** diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/utils.ts index edb7c2c699882..7df109deb6f3b 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/utils.ts @@ -86,6 +86,7 @@ export const getOutputRuleAlertForRest = (): RuleResponse => ({ type: 'query', note: '# Investigative notes', version: 1, + revision: 0, execution_summary: undefined, related_integrations: [], required_fields: [], diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_by_object_ids.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_by_object_ids.test.ts index 02b83342cd846..e7fae09cb1d38 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_by_object_ids.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_by_object_ids.test.ts @@ -212,6 +212,7 @@ describe('get_export_by_object_ids', () => { throttle: 'no_actions', note: '# Investigative notes', version: 1, + revision: 0, exceptions_list: getListArrayMock(), execution_summary: undefined, outcome: undefined, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/validate.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/validate.test.ts index f90ce4a33b573..eee0ec955b2b9 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/validate.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/validate.test.ts @@ -47,6 +47,7 @@ export const ruleOutput = (): RuleResponse => ({ throttle: 'no_actions', threat: getThreatMock(), version: 1, + revision: 0, filters: [ { query: { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/__mocks__/es_results.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/__mocks__/es_results.ts index 5bf47929cffa5..9593dd5624c22 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/__mocks__/es_results.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/__mocks__/es_results.ts @@ -497,6 +497,7 @@ export const sampleSignalHit = (): SignalHit => ({ related_integrations: [], required_fields: [], response_actions: undefined, + revision: 0, setup: '', throttle: 'no_actions', actions: [], From 31b143ac62eddeaa2d8b8e3840936168930ac0fd Mon Sep 17 00:00:00 2001 From: Garrett Spong Date: Tue, 13 Dec 2022 15:12:58 -0700 Subject: [PATCH 06/28] Additional typecheck and FTR fixes --- .../components/rules_list_notify_badge_sandbox.tsx | 1 + .../public/lib/common_transformations.test.ts | 1 + .../alerting/server/rules_client/tests/create.test.ts | 11 +++++++++++ .../alerts/lib/get_alert_panels_by_category.test.tsx | 1 + .../alerts/lib/get_alert_panels_by_node.test.tsx | 1 + .../public/legacy_uptime/state/api/alerts.ts | 1 + .../group3/tests/alerting/bulk_delete.ts | 1 + .../group3/tests/alerting/clone.ts | 1 + .../basic/tests/create_rules.ts | 1 + .../utils/get_simple_rule_output.ts | 1 + 10 files changed, 20 insertions(+) diff --git a/x-pack/examples/triggers_actions_ui_example/public/components/rules_list_notify_badge_sandbox.tsx b/x-pack/examples/triggers_actions_ui_example/public/components/rules_list_notify_badge_sandbox.tsx index 86f929853dc8a..9fb188ce648ba 100644 --- a/x-pack/examples/triggers_actions_ui_example/public/components/rules_list_notify_badge_sandbox.tsx +++ b/x-pack/examples/triggers_actions_ui_example/public/components/rules_list_notify_badge_sandbox.tsx @@ -45,6 +45,7 @@ const mockRule: RuleTableItem = { ruleType: 'Test Rule Type', isEditable: true, enabledInLicense: true, + revision: 0, }; export const RulesListNotifyBadgeSandbox = ({ triggersActionsUi }: SandboxProps) => { diff --git a/x-pack/plugins/alerting/public/lib/common_transformations.test.ts b/x-pack/plugins/alerting/public/lib/common_transformations.test.ts index 69fd0b7a6d94d..5443727f16dd8 100644 --- a/x-pack/plugins/alerting/public/lib/common_transformations.test.ts +++ b/x-pack/plugins/alerting/public/lib/common_transformations.test.ts @@ -180,6 +180,7 @@ describe('common_transformations', () => { ], }, }, + "revision": 0, "schedule": Object { "interval": "1s", }, diff --git a/x-pack/plugins/alerting/server/rules_client/tests/create.test.ts b/x-pack/plugins/alerting/server/rules_client/tests/create.test.ts index 6205b4bb4ba6a..7bfad0fb01c2b 100644 --- a/x-pack/plugins/alerting/server/rules_client/tests/create.test.ts +++ b/x-pack/plugins/alerting/server/rules_client/tests/create.test.ts @@ -446,6 +446,7 @@ describe('create()', () => { "params": Object { "bar": true, }, + "revision": 0, "schedule": Object { "interval": "1m", }, @@ -663,6 +664,7 @@ describe('create()', () => { "params": Object { "bar": true, }, + "revision": 0, "schedule": Object { "interval": "1m", }, @@ -1085,6 +1087,7 @@ describe('create()', () => { name: 'abc', notifyWhen: 'onActiveAlert', params: { bar: true }, + revision: 0, schedule: { interval: '1m' }, tags: ['foo'], throttle: null, @@ -1284,6 +1287,7 @@ describe('create()', () => { name: 'abc', notifyWhen: 'onActiveAlert', params: { bar: true, parameterThatIsSavedObjectRef: 'soRef_0' }, + revision: 0, schedule: { interval: '1m' }, tags: ['foo'], throttle: null, @@ -1452,6 +1456,7 @@ describe('create()', () => { name: 'abc', notifyWhen: 'onActiveAlert', params: { bar: true, parameterThatIsSavedObjectRef: 'action_0' }, + revision: 0, schedule: { interval: '1m' }, tags: ['foo'], throttle: null, @@ -1623,6 +1628,7 @@ describe('create()', () => { warning: null, }, monitoring: getDefaultMonitoring('2019-02-12T21:01:22.479Z'), + revision: 0, }, { id: 'mock-saved-object-id', @@ -1755,6 +1761,7 @@ describe('create()', () => { warning: null, }, monitoring: getDefaultMonitoring('2019-02-12T21:01:22.479Z'), + revision: 0, }, { id: 'mock-saved-object-id', @@ -1887,6 +1894,7 @@ describe('create()', () => { warning: null, }, monitoring: getDefaultMonitoring('2019-02-12T21:01:22.479Z'), + revision: 0, }, { id: 'mock-saved-object-id', @@ -2027,6 +2035,7 @@ describe('create()', () => { muteAll: false, snoozeSchedule: [], mutedInstanceIds: [], + revision: 0, executionStatus: { status: 'pending', lastExecutionDate: '2019-02-12T21:01:22.479Z', @@ -2407,6 +2416,7 @@ describe('create()', () => { warning: null, }, monitoring: getDefaultMonitoring('2019-02-12T21:01:22.479Z'), + revision: 0, }, { id: 'mock-saved-object-id', @@ -2509,6 +2519,7 @@ describe('create()', () => { warning: null, }, monitoring: getDefaultMonitoring('2019-02-12T21:01:22.479Z'), + revision: 0, }, { id: 'mock-saved-object-id', diff --git a/x-pack/plugins/monitoring/public/alerts/lib/get_alert_panels_by_category.test.tsx b/x-pack/plugins/monitoring/public/alerts/lib/get_alert_panels_by_category.test.tsx index 445006ecf715e..367b48f5a6786 100644 --- a/x-pack/plugins/monitoring/public/alerts/lib/get_alert_panels_by_category.test.tsx +++ b/x-pack/plugins/monitoring/public/alerts/lib/get_alert_panels_by_category.test.tsx @@ -58,6 +58,7 @@ const mockAlert = { lastExecutionDate: new Date('2020-12-08'), }, notifyWhen: null, + revision: 0, }; describe('getAlertPanelsByCategory', () => { diff --git a/x-pack/plugins/monitoring/public/alerts/lib/get_alert_panels_by_node.test.tsx b/x-pack/plugins/monitoring/public/alerts/lib/get_alert_panels_by_node.test.tsx index 7e7cd96f75aba..9103eaa0c3207 100644 --- a/x-pack/plugins/monitoring/public/alerts/lib/get_alert_panels_by_node.test.tsx +++ b/x-pack/plugins/monitoring/public/alerts/lib/get_alert_panels_by_node.test.tsx @@ -57,6 +57,7 @@ const mockAlert = { lastExecutionDate: new Date('2020-12-08'), }, notifyWhen: null, + revision: 0, }; describe('getAlertPanelsByNode', () => { diff --git a/x-pack/plugins/synthetics/public/legacy_uptime/state/api/alerts.ts b/x-pack/plugins/synthetics/public/legacy_uptime/state/api/alerts.ts index 5513369d5c10f..068b79d57bd43 100644 --- a/x-pack/plugins/synthetics/public/legacy_uptime/state/api/alerts.ts +++ b/x-pack/plugins/synthetics/public/legacy_uptime/state/api/alerts.ts @@ -62,6 +62,7 @@ type NewMonitorStatusAlert = Omit< | 'muteAll' | 'mutedInstanceIds' | 'executionStatus' + | 'revision' | 'ruleTypeId' | 'notifyWhen' | 'actions' diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/group3/tests/alerting/bulk_delete.ts b/x-pack/test/alerting_api_integration/security_and_spaces/group3/tests/alerting/bulk_delete.ts index 61bdfa2f78717..9bbd7fc375891 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/group3/tests/alerting/bulk_delete.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/group3/tests/alerting/bulk_delete.ts @@ -66,6 +66,7 @@ const getThreeRules = (response: any) => { scheduledTaskId: response.body.rules[i].scheduledTaskId, executionStatus: response.body.rules[i].executionStatus, monitoring: response.body.rules[i].monitoring, + revision: 0, ...(response.body.rules[i].nextRun ? { nextRun: response.body.rules[i].nextRun } : {}), ...(response.body.rules[i].lastRun ? { lastRun: response.body.rules[i].lastRun } : {}), }); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/group3/tests/alerting/clone.ts b/x-pack/test/alerting_api_integration/security_and_spaces/group3/tests/alerting/clone.ts index e99aa177d8a78..023f07de687e7 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/group3/tests/alerting/clone.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/group3/tests/alerting/clone.ts @@ -163,6 +163,7 @@ export default function createAlertTests({ getService }: FtrProviderContext) { mute_all: false, muted_alert_ids: [], execution_status: response.body.execution_status, + revision: 0, // TODO: Finalize proper clone behavior (copy previous revision, or reset to initial default?) last_run: { alerts_count: { active: 0, diff --git a/x-pack/test/detection_engine_api_integration/basic/tests/create_rules.ts b/x-pack/test/detection_engine_api_integration/basic/tests/create_rules.ts index 55183b48d78f9..d67c10c8b17bd 100644 --- a/x-pack/test/detection_engine_api_integration/basic/tests/create_rules.ts +++ b/x-pack/test/detection_engine_api_integration/basic/tests/create_rules.ts @@ -102,6 +102,7 @@ export default ({ getService }: FtrProviderContext) => { throttle: 'no_actions', exceptions_list: [], version: 1, + revision: 0, }; const { body } = await supertest diff --git a/x-pack/test/detection_engine_api_integration/utils/get_simple_rule_output.ts b/x-pack/test/detection_engine_api_integration/utils/get_simple_rule_output.ts index 39b8d2acf088e..9826d9a2b98b4 100644 --- a/x-pack/test/detection_engine_api_integration/utils/get_simple_rule_output.ts +++ b/x-pack/test/detection_engine_api_integration/utils/get_simple_rule_output.ts @@ -43,6 +43,7 @@ export const getMockSharedResponseSchema = ( throttle: 'no_actions', exceptions_list: [], version: 1, + revision: 0, id: 'id', updated_at: '2020-07-08T16:36:32.377Z', created_at: '2020-07-08T16:36:32.377Z', From bdd528586271c4b18a9562ba0a73cc3a835026b5 Mon Sep 17 00:00:00 2001 From: Garrett Spong Date: Tue, 13 Dec 2022 17:41:51 -0700 Subject: [PATCH 07/28] Jest fixes and detections FTR fixes --- x-pack/plugins/alerting/public/alert_api.test.ts | 2 ++ .../server/rules_client/tests/update.test.ts | 8 ++++++++ .../get_alert_panels_by_category.test.tsx.snap | 15 +++++++++++++++ .../get_alert_panels_by_node.test.tsx.snap | 8 ++++++++ .../security_solution/cypress/objects/rule.ts | 1 + .../logic/export/get_export_all.test.ts | 1 + .../logic/export/get_export_by_object_ids.test.ts | 1 + .../group10/update_rules_bulk.ts | 13 +++++++++++++ 8 files changed, 49 insertions(+) diff --git a/x-pack/plugins/alerting/public/alert_api.test.ts b/x-pack/plugins/alerting/public/alert_api.test.ts index 429e5f1c44e0c..cff785d55417f 100644 --- a/x-pack/plugins/alerting/public/alert_api.test.ts +++ b/x-pack/plugins/alerting/public/alert_api.test.ts @@ -131,6 +131,7 @@ describe('loadRule', () => { "params": Object { "x": 42, }, + "revision": 0, "schedule": Object { "interval": "1s", }, @@ -267,6 +268,7 @@ function getApiRule() { updated_by: '2889684073', mute_all: false, muted_alert_ids: [], + revision: 0, schedule: { interval: '1s', }, diff --git a/x-pack/plugins/alerting/server/rules_client/tests/update.test.ts b/x-pack/plugins/alerting/server/rules_client/tests/update.test.ts index ae566c107862a..52af00ab59f29 100644 --- a/x-pack/plugins/alerting/server/rules_client/tests/update.test.ts +++ b/x-pack/plugins/alerting/server/rules_client/tests/update.test.ts @@ -89,6 +89,7 @@ describe('update()', () => { params: {}, throttle: null, notifyWhen: null, + revision: 0, actions: [ { group: 'default', @@ -382,6 +383,7 @@ describe('update()', () => { "risk_score": 40, "severity": "low", }, + "revision": 1, "schedule": Object { "interval": "1m", }, @@ -568,6 +570,7 @@ describe('update()', () => { }, }); + // TODO: Review update results once increment logic is finalized expect(unsecuredSavedObjectsClient.create).toHaveBeenNthCalledWith( 1, 'alert', @@ -607,6 +610,7 @@ describe('update()', () => { name: 'abc', notifyWhen: 'onActiveAlert', params: { bar: true }, + revision: 1, schedule: { interval: '1m' }, scheduledTaskId: 'task-123', tags: ['foo'], @@ -767,6 +771,7 @@ describe('update()', () => { }, }); + // TODO: Review update results once increment logic is finalized expect(extractReferencesFn).toHaveBeenCalledWith(ruleParams); expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith( 'alert', @@ -783,6 +788,7 @@ describe('update()', () => { name: 'abc', notifyWhen: 'onActiveAlert', params: { bar: true, parameterThatIsSavedObjectRef: 'soRef_0' }, + revision: 1, schedule: { interval: '1m' }, scheduledTaskId: 'task-123', tags: ['foo'], @@ -961,6 +967,7 @@ describe('update()', () => { "params": Object { "bar": true, }, + "revision": 1, "schedule": Object { "interval": "1m", }, @@ -1109,6 +1116,7 @@ describe('update()', () => { "params": Object { "bar": true, }, + "revision": 1, "schedule": Object { "interval": "1m", }, diff --git a/x-pack/plugins/monitoring/public/alerts/lib/__snapshots__/get_alert_panels_by_category.test.tsx.snap b/x-pack/plugins/monitoring/public/alerts/lib/__snapshots__/get_alert_panels_by_category.test.tsx.snap index c2d9d0df2d4d1..1ed3850227b76 100644 --- a/x-pack/plugins/monitoring/public/alerts/lib/__snapshots__/get_alert_panels_by_category.test.tsx.snap +++ b/x-pack/plugins/monitoring/public/alerts/lib/__snapshots__/get_alert_panels_by_category.test.tsx.snap @@ -87,6 +87,7 @@ Array [ "name": "monitoring_alert_cpu_usage_label", "notifyWhen": null, "params": Object {}, + "revision": 0, "schedule": Object { "interval": "1m", }, @@ -250,6 +251,7 @@ Array [ "name": "monitoring_alert_jvm_memory_usage_label", "notifyWhen": null, "params": Object {}, + "revision": 0, "schedule": Object { "interval": "1m", }, @@ -310,6 +312,7 @@ Array [ "name": "monitoring_alert_jvm_memory_usage_label", "notifyWhen": null, "params": Object {}, + "revision": 0, "schedule": Object { "interval": "1m", }, @@ -602,6 +605,7 @@ Array [ "name": "monitoring_alert_nodes_changed_label", "notifyWhen": null, "params": Object {}, + "revision": 0, "schedule": Object { "interval": "1m", }, @@ -662,6 +666,7 @@ Array [ "name": "monitoring_alert_nodes_changed_label", "notifyWhen": null, "params": Object {}, + "revision": 0, "schedule": Object { "interval": "1m", }, @@ -722,6 +727,7 @@ Array [ "name": "monitoring_alert_disk_usage_label", "notifyWhen": null, "params": Object {}, + "revision": 0, "schedule": Object { "interval": "1m", }, @@ -782,6 +788,7 @@ Array [ "name": "monitoring_alert_license_expiration_label", "notifyWhen": null, "params": Object {}, + "revision": 0, "schedule": Object { "interval": "1m", }, @@ -842,6 +849,7 @@ Array [ "name": "monitoring_alert_license_expiration_label", "notifyWhen": null, "params": Object {}, + "revision": 0, "schedule": Object { "interval": "1m", }, @@ -931,6 +939,7 @@ Array [ "name": "monitoring_alert_jvm_memory_usage_label", "notifyWhen": null, "params": Object {}, + "revision": 0, "schedule": Object { "interval": "1m", }, @@ -1032,6 +1041,7 @@ Array [ "name": "monitoring_alert_nodes_changed_label", "notifyWhen": null, "params": Object {}, + "revision": 0, "schedule": Object { "interval": "1m", }, @@ -1068,6 +1078,7 @@ Array [ "name": "monitoring_alert_disk_usage_label", "notifyWhen": null, "params": Object {}, + "revision": 0, "schedule": Object { "interval": "1m", }, @@ -1104,6 +1115,7 @@ Array [ "name": "monitoring_alert_license_expiration_label", "notifyWhen": null, "params": Object {}, + "revision": 0, "schedule": Object { "interval": "1m", }, @@ -1205,6 +1217,7 @@ Array [ "name": "monitoring_alert_logstash_version_mismatch_label", "notifyWhen": null, "params": Object {}, + "revision": 0, "schedule": Object { "interval": "1m", }, @@ -1241,6 +1254,7 @@ Array [ "name": "monitoring_alert_cpu_usage_label", "notifyWhen": null, "params": Object {}, + "revision": 0, "schedule": Object { "interval": "1m", }, @@ -1277,6 +1291,7 @@ Array [ "name": "monitoring_alert_thread_pool_write_rejections_label", "notifyWhen": null, "params": Object {}, + "revision": 0, "schedule": Object { "interval": "1m", }, diff --git a/x-pack/plugins/monitoring/public/alerts/lib/__snapshots__/get_alert_panels_by_node.test.tsx.snap b/x-pack/plugins/monitoring/public/alerts/lib/__snapshots__/get_alert_panels_by_node.test.tsx.snap index 9bb6d1c979571..b576b029f1a70 100644 --- a/x-pack/plugins/monitoring/public/alerts/lib/__snapshots__/get_alert_panels_by_node.test.tsx.snap +++ b/x-pack/plugins/monitoring/public/alerts/lib/__snapshots__/get_alert_panels_by_node.test.tsx.snap @@ -114,6 +114,7 @@ Array [ "name": "monitoring_alert_jvm_memory_usage_label", "notifyWhen": null, "params": Object {}, + "revision": 0, "schedule": Object { "interval": "1m", }, @@ -174,6 +175,7 @@ Array [ "name": "monitoring_alert_jvm_memory_usage_label", "notifyWhen": null, "params": Object {}, + "revision": 0, "schedule": Object { "interval": "1m", }, @@ -407,6 +409,7 @@ Array [ "name": "monitoring_alert_nodes_changed_label", "notifyWhen": null, "params": Object {}, + "revision": 0, "schedule": Object { "interval": "1m", }, @@ -467,6 +470,7 @@ Array [ "name": "monitoring_alert_disk_usage_label", "notifyWhen": null, "params": Object {}, + "revision": 0, "schedule": Object { "interval": "1m", }, @@ -527,6 +531,7 @@ Array [ "name": "monitoring_alert_license_expiration_label", "notifyWhen": null, "params": Object {}, + "revision": 0, "schedule": Object { "interval": "1m", }, @@ -587,6 +592,7 @@ Array [ "name": "monitoring_alert_nodes_changed_label", "notifyWhen": null, "params": Object {}, + "revision": 0, "schedule": Object { "interval": "1m", }, @@ -647,6 +653,7 @@ Array [ "name": "monitoring_alert_license_expiration_label", "notifyWhen": null, "params": Object {}, + "revision": 0, "schedule": Object { "interval": "1m", }, @@ -707,6 +714,7 @@ Array [ "name": "monitoring_alert_nodes_changed_label_2", "notifyWhen": null, "params": Object {}, + "revision": 0, "schedule": Object { "interval": "1m", }, diff --git a/x-pack/plugins/security_solution/cypress/objects/rule.ts b/x-pack/plugins/security_solution/cypress/objects/rule.ts index d60ef8dd54d51..663ab0fa3c715 100644 --- a/x-pack/plugins/security_solution/cypress/objects/rule.ts +++ b/x-pack/plugins/security_solution/cypress/objects/rule.ts @@ -531,6 +531,7 @@ export const expectedExportedRule = (ruleResponse: Cypress.Response { references: ['http://example.com', 'https://example.com'], related_integrations: [], required_fields: [], + revision: 0, setup: '', timeline_id: 'some-timeline-id', timeline_title: 'some-timeline-title', diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_by_object_ids.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_by_object_ids.test.ts index e7fae09cb1d38..50e16207255d1 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_by_object_ids.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_by_object_ids.test.ts @@ -85,6 +85,7 @@ describe('get_export_by_object_ids', () => { references: ['http://example.com', 'https://example.com'], related_integrations: [], required_fields: [], + revision: 0, setup: '', timeline_id: 'some-timeline-id', timeline_title: 'some-timeline-title', diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/update_rules_bulk.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/update_rules_bulk.ts index 04f3eb2536a41..3d0535bf6731a 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/update_rules_bulk.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/update_rules_bulk.ts @@ -79,6 +79,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutput(); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body[0]); expect(bodyToCompare).to.eql(outputRule); }); @@ -109,10 +110,12 @@ export default ({ getService }: FtrProviderContext) => { const outputRule1 = getSimpleRuleOutput(); outputRule1.name = 'some other name'; outputRule1.version = 2; + outputRule1.revision = 1; const outputRule2 = getSimpleRuleOutput('rule-2'); outputRule2.name = 'some other name'; outputRule2.version = 2; + outputRule1.revision = 1; const bodyToCompare1 = removeServerGeneratedProperties(body[0]); const bodyToCompare2 = removeServerGeneratedProperties(body[1]); @@ -169,6 +172,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutput(response.rule_id); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 2; outputRule.actions = [ { action_type_id: '.slack', @@ -231,6 +235,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutput(response.rule_id); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 2; outputRule.actions = []; outputRule.throttle = 'no_actions'; const bodyToCompare = removeServerGeneratedProperties(response); @@ -256,6 +261,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutput(); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body[0]); expect(bodyToCompare).to.eql(outputRule); }); @@ -284,10 +290,12 @@ export default ({ getService }: FtrProviderContext) => { const outputRule1 = getSimpleRuleOutput('rule-1'); outputRule1.name = 'some other name'; outputRule1.version = 2; + outputRule1.revision = 1; const outputRule2 = getSimpleRuleOutput('rule-2'); outputRule2.name = 'some other name'; outputRule2.version = 2; + outputRule1.revision = 1; const bodyToCompare1 = removeServerGeneratedProperties(body[0]); const bodyToCompare2 = removeServerGeneratedProperties(body[1]); @@ -313,6 +321,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutput(); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body[0]); expect(bodyToCompare).to.eql(outputRule); }); @@ -335,6 +344,7 @@ export default ({ getService }: FtrProviderContext) => { outputRule.enabled = false; outputRule.severity = 'low'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body[0]); expect(bodyToCompare).to.eql(outputRule); @@ -367,6 +377,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutput(); outputRule.name = 'some other name'; outputRule.version = 3; + outputRule.revision = 2; const bodyToCompare = removeServerGeneratedProperties(body[0]); expect(bodyToCompare).to.eql(outputRule); @@ -434,6 +445,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutput(); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body[0]); expect([bodyToCompare, body[1]]).to.eql([ @@ -471,6 +483,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutput(); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body[0]); expect([bodyToCompare, body[1]]).to.eql([ From 396ccc12e8a943ebe5abd5b8aa51cb71c2c2507b Mon Sep 17 00:00:00 2001 From: Garrett Spong Date: Wed, 14 Dec 2022 11:51:24 -0700 Subject: [PATCH 08/28] Fixes additional detections FTRs --- .../basic/tests/patch_rules.ts | 5 +++++ .../basic/tests/patch_rules_bulk.ts | 11 +++++++++++ .../basic/tests/update_rules_bulk.ts | 2 ++ .../security_and_spaces/group10/patch_rules.ts | 7 +++++++ .../security_and_spaces/group10/patch_rules_bulk.ts | 11 +++++++++++ .../security_and_spaces/group10/update_rules.ts | 9 +++++++++ .../security_and_spaces/group10/update_rules_bulk.ts | 4 ++-- 7 files changed, 47 insertions(+), 2 deletions(-) diff --git a/x-pack/test/detection_engine_api_integration/basic/tests/patch_rules.ts b/x-pack/test/detection_engine_api_integration/basic/tests/patch_rules.ts index ae92c294867ce..9bc9894f5d30f 100644 --- a/x-pack/test/detection_engine_api_integration/basic/tests/patch_rules.ts +++ b/x-pack/test/detection_engine_api_integration/basic/tests/patch_rules.ts @@ -50,6 +50,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutput(); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body); expect(bodyToCompare).to.eql(outputRule); }); @@ -86,6 +87,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutputWithoutRuleId(); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedPropertiesIncludingRuleId(body); expect(bodyToCompare).to.eql(outputRule); }); @@ -103,6 +105,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutput(); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body); expect(bodyToCompare).to.eql(outputRule); }); @@ -138,6 +141,7 @@ export default ({ getService }: FtrProviderContext) => { outputRule.enabled = false; outputRule.severity = 'low'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body); expect(bodyToCompare).to.eql(outputRule); @@ -165,6 +169,7 @@ export default ({ getService }: FtrProviderContext) => { outputRule.timeline_title = 'some title'; outputRule.timeline_id = 'some id'; outputRule.version = 3; + outputRule.revision = 2; const bodyToCompare = removeServerGeneratedProperties(body); expect(bodyToCompare).to.eql(outputRule); diff --git a/x-pack/test/detection_engine_api_integration/basic/tests/patch_rules_bulk.ts b/x-pack/test/detection_engine_api_integration/basic/tests/patch_rules_bulk.ts index a79c9e19857b8..cf0506f44c278 100644 --- a/x-pack/test/detection_engine_api_integration/basic/tests/patch_rules_bulk.ts +++ b/x-pack/test/detection_engine_api_integration/basic/tests/patch_rules_bulk.ts @@ -50,6 +50,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutput(); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body[0]); expect(bodyToCompare).to.eql(outputRule); }); @@ -71,10 +72,12 @@ export default ({ getService }: FtrProviderContext) => { const outputRule1 = getSimpleRuleOutput(); outputRule1.name = 'some other name'; outputRule1.version = 2; + outputRule1.revision = 1; const outputRule2 = getSimpleRuleOutput('rule-2'); outputRule2.name = 'some other name'; outputRule2.version = 2; + outputRule2.revision = 1; const bodyToCompare1 = removeServerGeneratedProperties(body[0]); const bodyToCompare2 = removeServerGeneratedProperties(body[1]); @@ -95,6 +98,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutput(); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body[0]); expect(bodyToCompare).to.eql(outputRule); }); @@ -116,10 +120,12 @@ export default ({ getService }: FtrProviderContext) => { const outputRule1 = getSimpleRuleOutputWithoutRuleId('rule-1'); outputRule1.name = 'some other name'; outputRule1.version = 2; + outputRule1.revision = 1; const outputRule2 = getSimpleRuleOutputWithoutRuleId('rule-2'); outputRule2.name = 'some other name'; outputRule2.version = 2; + outputRule2.revision = 1; const bodyToCompare1 = removeServerGeneratedPropertiesIncludingRuleId(body[0]); const bodyToCompare2 = removeServerGeneratedPropertiesIncludingRuleId(body[1]); @@ -140,6 +146,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutput(); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body[0]); expect(bodyToCompare).to.eql(outputRule); }); @@ -175,6 +182,7 @@ export default ({ getService }: FtrProviderContext) => { outputRule.enabled = false; outputRule.severity = 'low'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body[0]); expect(bodyToCompare).to.eql(outputRule); @@ -202,6 +210,7 @@ export default ({ getService }: FtrProviderContext) => { outputRule.timeline_title = 'some title'; outputRule.timeline_id = 'some id'; outputRule.version = 3; + outputRule.revision = 2; const bodyToCompare = removeServerGeneratedProperties(body[0]); expect(bodyToCompare).to.eql(outputRule); @@ -256,6 +265,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutput(); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body[0]); expect([bodyToCompare, body[1]]).to.eql([ @@ -286,6 +296,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutput(); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body[0]); expect([bodyToCompare, body[1]]).to.eql([ diff --git a/x-pack/test/detection_engine_api_integration/basic/tests/update_rules_bulk.ts b/x-pack/test/detection_engine_api_integration/basic/tests/update_rules_bulk.ts index b614c1defa761..2c02e09a5cfa1 100644 --- a/x-pack/test/detection_engine_api_integration/basic/tests/update_rules_bulk.ts +++ b/x-pack/test/detection_engine_api_integration/basic/tests/update_rules_bulk.ts @@ -195,6 +195,7 @@ export default ({ getService }: FtrProviderContext) => { outputRule.enabled = false; outputRule.severity = 'low'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body[0]); expect(bodyToCompare).to.eql(outputRule); @@ -227,6 +228,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutput(); outputRule.name = 'some other name'; outputRule.version = 3; + outputRule.revision = 2; const bodyToCompare = removeServerGeneratedProperties(body[0]); expect(bodyToCompare).to.eql(outputRule); diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/patch_rules.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/patch_rules.ts index 2ec13365eb1d1..bc6221f061cb2 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/patch_rules.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/patch_rules.ts @@ -54,6 +54,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutput(); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body); expect(bodyToCompare).to.eql(outputRule); }); @@ -70,6 +71,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleMlRuleOutput(); outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body); expect(bodyToCompare).to.eql(outputRule); }); @@ -87,6 +89,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleMlRuleOutput(); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body); expect(bodyToCompare).to.eql(outputRule); }); @@ -106,6 +109,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutputWithoutRuleId(); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedPropertiesIncludingRuleId(body); expect(bodyToCompare).to.eql(outputRule); }); @@ -123,6 +127,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutput(); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body); expect(bodyToCompare).to.eql(outputRule); }); @@ -158,6 +163,7 @@ export default ({ getService }: FtrProviderContext) => { outputRule.enabled = false; outputRule.severity = 'low'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body); expect(bodyToCompare).to.eql(outputRule); @@ -185,6 +191,7 @@ export default ({ getService }: FtrProviderContext) => { outputRule.timeline_title = 'some title'; outputRule.timeline_id = 'some id'; outputRule.version = 3; + outputRule.revision = 2; const bodyToCompare = removeServerGeneratedProperties(body); expect(bodyToCompare).to.eql(outputRule); diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/patch_rules_bulk.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/patch_rules_bulk.ts index b844e9500e126..ad6e22e042da3 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/patch_rules_bulk.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/patch_rules_bulk.ts @@ -73,6 +73,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutput(); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body[0]); expect(bodyToCompare).to.eql(outputRule); }); @@ -94,10 +95,12 @@ export default ({ getService }: FtrProviderContext) => { const outputRule1 = getSimpleRuleOutput(); outputRule1.name = 'some other name'; outputRule1.version = 2; + outputRule1.revision = 1; const outputRule2 = getSimpleRuleOutput('rule-2'); outputRule2.name = 'some other name'; outputRule2.version = 2; + outputRule2.revision = 1; const bodyToCompare1 = removeServerGeneratedProperties(body[0]); const bodyToCompare2 = removeServerGeneratedProperties(body[1]); @@ -118,6 +121,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutput(); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body[0]); expect(bodyToCompare).to.eql(outputRule); }); @@ -139,10 +143,12 @@ export default ({ getService }: FtrProviderContext) => { const outputRule1 = getSimpleRuleOutputWithoutRuleId('rule-1'); outputRule1.name = 'some other name'; outputRule1.version = 2; + outputRule1.revision = 1; const outputRule2 = getSimpleRuleOutputWithoutRuleId('rule-2'); outputRule2.name = 'some other name'; outputRule2.version = 2; + outputRule2.revision = 1; const bodyToCompare1 = removeServerGeneratedPropertiesIncludingRuleId(body[0]); const bodyToCompare2 = removeServerGeneratedPropertiesIncludingRuleId(body[1]); @@ -212,6 +218,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutput(); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body[0]); expect(bodyToCompare).to.eql(outputRule); }); @@ -247,6 +254,7 @@ export default ({ getService }: FtrProviderContext) => { outputRule.enabled = false; outputRule.severity = 'low'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body[0]); expect(bodyToCompare).to.eql(outputRule); @@ -274,6 +282,7 @@ export default ({ getService }: FtrProviderContext) => { outputRule.timeline_title = 'some title'; outputRule.timeline_id = 'some id'; outputRule.version = 3; + outputRule.revision = 2; const bodyToCompare = removeServerGeneratedProperties(body[0]); expect(bodyToCompare).to.eql(outputRule); @@ -328,6 +337,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutput(); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body[0]); expect([bodyToCompare, body[1]]).to.eql([ @@ -358,6 +368,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutput(); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body[0]); expect([bodyToCompare, body[1]]).to.eql([ diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/update_rules.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/update_rules.ts index a33aacd26bb8a..5248c5625bcbe 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/update_rules.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/update_rules.ts @@ -63,6 +63,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutput(); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body); expect(bodyToCompare).to.eql(outputRule); }); @@ -86,6 +87,7 @@ export default ({ getService }: FtrProviderContext) => { // @ts-expect-error type narrowing is lost due to Omit<> outputRule.machine_learning_job_id = ['legacy_job_id']; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body); expect(bodyToCompare).to.eql(outputRule); }); @@ -108,6 +110,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleMlRuleOutput(); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body); expect(bodyToCompare).to.eql(outputRule); }); @@ -132,6 +135,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutputWithoutRuleId(); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedPropertiesIncludingRuleId(body); expect(bodyToCompare).to.eql(outputRule); }); @@ -181,6 +185,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutputWithoutRuleId(); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 1; // Expect an empty array outputRule.actions = []; // Expect "no_actions" @@ -232,6 +237,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutputWithoutRuleId(); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 1; outputRule.actions = [ { action_type_id: '.slack', @@ -266,6 +272,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutput(); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body); expect(bodyToCompare).to.eql(outputRule); }); @@ -288,6 +295,7 @@ export default ({ getService }: FtrProviderContext) => { outputRule.enabled = false; outputRule.severity = 'low'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body); expect(bodyToCompare).to.eql(outputRule); @@ -320,6 +328,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutput(); outputRule.name = 'some other name'; outputRule.version = 3; + outputRule.revision = 2; const bodyToCompare = removeServerGeneratedProperties(body); expect(bodyToCompare).to.eql(outputRule); diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/update_rules_bulk.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/update_rules_bulk.ts index 3d0535bf6731a..4c054d4983705 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/update_rules_bulk.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/update_rules_bulk.ts @@ -115,7 +115,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule2 = getSimpleRuleOutput('rule-2'); outputRule2.name = 'some other name'; outputRule2.version = 2; - outputRule1.revision = 1; + outputRule2.revision = 1; const bodyToCompare1 = removeServerGeneratedProperties(body[0]); const bodyToCompare2 = removeServerGeneratedProperties(body[1]); @@ -295,7 +295,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule2 = getSimpleRuleOutput('rule-2'); outputRule2.name = 'some other name'; outputRule2.version = 2; - outputRule1.revision = 1; + outputRule2.revision = 1; const bodyToCompare1 = removeServerGeneratedProperties(body[0]); const bodyToCompare2 = removeServerGeneratedProperties(body[1]); From 98098b5dfb1be0ce6828931e052624020f722bd5 Mon Sep 17 00:00:00 2001 From: Garrett Spong Date: Tue, 7 Feb 2023 11:47:34 -0700 Subject: [PATCH 09/28] Reuse incrementVersion in update() --- .../plugins/alerting/server/rules_client/methods/update.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/alerting/server/rules_client/methods/update.ts b/x-pack/plugins/alerting/server/rules_client/methods/update.ts index 8e303cf4fd306..7a39aa4eca194 100644 --- a/x-pack/plugins/alerting/server/rules_client/methods/update.ts +++ b/x-pack/plugins/alerting/server/rules_client/methods/update.ts @@ -150,8 +150,9 @@ async function updateWithOCC( async function updateAlert( context: RulesClientContext, { id, data, allowMissingConnectorSecrets }: UpdateOptions, - { attributes, version }: SavedObject + currentRule: SavedObject ): Promise> { + const { attributes, version } = currentRule; const ruleType = context.ruleTypeRegistry.get(attributes.alertTypeId); // TODO https://github.com/elastic/kibana/issues/148414 @@ -202,7 +203,7 @@ async function updateAlert( const notifyWhen = getRuleNotifyWhenType(data.notifyWhen ?? null, data.throttle ?? null); // Increment revision if applicable field has changed - // const revision = incrementRevision(currentRule, updatedParams); + const revision = incrementRevision(currentRule, updatedParams); let updatedObject: SavedObject; const createAttributes = updateMeta(context, { @@ -212,7 +213,7 @@ async function updateAlert( params: updatedParams as RawRule['params'], actions, notifyWhen, - revision: 0, // TODO: Resolve after merge + revision, updatedBy: username, updatedAt: new Date().toISOString(), }); From 40fa9b04f20916b46eab3436c9fcdc808ea789cd Mon Sep 17 00:00:00 2001 From: Garrett Spong Date: Tue, 7 Feb 2023 12:12:03 -0700 Subject: [PATCH 10/28] Adds migration and bulk edit support --- .../server/rules_client/methods/bulk_edit.ts | 15 +++------- .../server/rules_client/rules_client.ts | 19 ++++++++++++- .../saved_objects/migrations/8.8/index.ts | 28 +++++++++++++++++++ 3 files changed, 50 insertions(+), 12 deletions(-) create mode 100644 x-pack/plugins/alerting/server/saved_objects/migrations/8.8/index.ts diff --git a/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts b/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts index 1eaa94d94d922..9787762645e41 100644 --- a/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts +++ b/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts @@ -54,13 +54,7 @@ import { API_KEY_GENERATE_CONCURRENCY, } from '../common/constants'; import { getMappedParams } from '../common/mapped_params_utils'; -import { - getAlertFromRaw, - extractReferences, - validateActions, - updateMeta, - incrementRevision, -} from '../lib'; +import { getAlertFromRaw, extractReferences, validateActions, updateMeta } from '../lib'; import { NormalizedAlertAction, BulkOperationError, @@ -441,6 +435,9 @@ async function updateRuleAttributesAndParamsInMemory = [ 'activeSnoozes', ]; +export const fieldsToExcludeFromRevisionUpdates: Array = [ + 'id', + 'enabled', + 'consumer', + 'alertTypeId', + 'createdBy', + 'updatedBy', + 'createdAt', + 'updatedAt', + 'executionStatus', + 'monitoring', + 'lastRun', + 'nextRun', + 'revision', + 'running', +]; + export class RulesClient { private readonly context: RulesClientContext; diff --git a/x-pack/plugins/alerting/server/saved_objects/migrations/8.8/index.ts b/x-pack/plugins/alerting/server/saved_objects/migrations/8.8/index.ts new file mode 100644 index 0000000000000..23aa21c02b69a --- /dev/null +++ b/x-pack/plugins/alerting/server/saved_objects/migrations/8.8/index.ts @@ -0,0 +1,28 @@ +/* + * 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 { SavedObjectUnsanitizedDoc } from '@kbn/core-saved-objects-server'; +import { EncryptedSavedObjectsPluginSetup } from '@kbn/encrypted-saved-objects-plugin/server'; +import { createEsoMigration, pipeMigrations } from '../utils'; +import { RawRule } from '../../../types'; + +function addRevision(doc: SavedObjectUnsanitizedDoc): SavedObjectUnsanitizedDoc { + return { + ...doc, + attributes: { + ...doc.attributes, + revision: 0, + }, + }; +} + +export const getMigrations880 = (encryptedSavedObjects: EncryptedSavedObjectsPluginSetup) => + createEsoMigration( + encryptedSavedObjects, + (doc: SavedObjectUnsanitizedDoc): doc is SavedObjectUnsanitizedDoc => true, + pipeMigrations(addRevision) + ); From e122adb1806d7d633f566afe51f4896395000b60 Mon Sep 17 00:00:00 2001 From: Garrett Spong Date: Tue, 7 Feb 2023 20:11:32 -0700 Subject: [PATCH 11/28] Adds incrementRevision logic and fixing tests --- .../migrations/check_registered_types.test.ts | 2 +- .../public/lib/common_transformations.test.ts | 1 + .../rules_client/lib/increment_revision.ts | 28 ++++++++++++++++--- .../server/rules_client/methods/bulk_edit.ts | 2 +- .../server/rules_client/methods/update.ts | 6 +++- .../tests/alerting/group2/update.ts | 3 ++ .../basic/tests/import_rules.ts | 1 + .../utils/get_complex_rule_output.ts | 1 + 8 files changed, 37 insertions(+), 7 deletions(-) diff --git a/src/core/server/integration_tests/saved_objects/migrations/check_registered_types.test.ts b/src/core/server/integration_tests/saved_objects/migrations/check_registered_types.test.ts index bafc64776b6dd..791d0f1aded59 100644 --- a/src/core/server/integration_tests/saved_objects/migrations/check_registered_types.test.ts +++ b/src/core/server/integration_tests/saved_objects/migrations/check_registered_types.test.ts @@ -57,7 +57,7 @@ describe('checking migration metadata changes on all registered SO types', () => Object { "action": "6cfc277ed3211639e37546ac625f4a68f2494215", "action_task_params": "db2afea7d78e00e725486b791554d0d4e81956ef", - "alert": "492f749af60bed1cc7022519ddf5fe4c482b62c4", + "alert": "12dc3b249eab1e03eae23eadb05f5662edc11620", "api_key_pending_invalidation": "16e7bcf8e78764102d7f525542d5b616809a21ee", "apm-indices": "d19dd7fb51f2d2cbc1f8769481721e0953f9a6d2", "apm-server-schema": "1d42f17eff9ec6c16d3a9324d9539e2d123d0a9a", diff --git a/x-pack/plugins/alerting/public/lib/common_transformations.test.ts b/x-pack/plugins/alerting/public/lib/common_transformations.test.ts index 9929d882577e7..59bd58dd84768 100644 --- a/x-pack/plugins/alerting/public/lib/common_transformations.test.ts +++ b/x-pack/plugins/alerting/public/lib/common_transformations.test.ts @@ -343,6 +343,7 @@ describe('common_transformations', () => { "nextRun": 2021-12-15T12:34:55.789Z, "notifyWhen": "onActiveAlert", "params": Object {}, + "revision": 0, "schedule": Object { "interval": "1s", }, diff --git a/x-pack/plugins/alerting/server/rules_client/lib/increment_revision.ts b/x-pack/plugins/alerting/server/rules_client/lib/increment_revision.ts index d17b56861d040..9f5a075506eec 100644 --- a/x-pack/plugins/alerting/server/rules_client/lib/increment_revision.ts +++ b/x-pack/plugins/alerting/server/rules_client/lib/increment_revision.ts @@ -6,13 +6,33 @@ */ import { SavedObject } from '@kbn/core/server'; +import { get, isEqual } from 'lodash'; import { RawRule, RuleTypeParams } from '../../types'; +import { fieldsToExcludeFromRevisionUpdates, UpdateOptions } from '..'; -export function incrementRevision( +export function incrementRevision( currentRule: SavedObject, + { data }: UpdateOptions, updatedParams: RuleTypeParams ): number { - // TODO: Update logic as outlined in https://github.com/elastic/kibana/issues/137164 - // TODO: Potentially streamline with 'skipped logic' being introduced in https://github.com/elastic/kibana/pull/144461 - return currentRule.attributes.revision + 1; + // Diff root level fields + for (const [field, value] of Object.entries(data).filter(([key]) => key !== 'params')) { + if (!fieldsToExcludeFromRevisionUpdates.map(toString).includes(field)) { + if (!isEqual(value, get(currentRule.attributes, field))) { + return currentRule.attributes.revision + 1; + } + } + } + + // Diff rule params + for (const [field, value] of Object.entries(updatedParams)) { + // TODO: Remove 'version' from SecuritySolution (and maybe have a way for + // TODO: RuleTypes to declare fields that can be revision skipped as well?) + if (!fieldsToExcludeFromRevisionUpdates.map(toString).includes(field) && field !== 'version') { + if (!isEqual(value, get(currentRule.attributes.params, field))) { + return currentRule.attributes.revision + 1; + } + } + } + return currentRule.attributes.revision; } diff --git a/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts b/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts index 9787762645e41..daa3817a1abae 100644 --- a/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts +++ b/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts @@ -436,7 +436,7 @@ async function updateRuleAttributesAndParamsInMemory( const notifyWhen = getRuleNotifyWhenType(data.notifyWhen ?? null, data.throttle ?? null); // Increment revision if applicable field has changed - const revision = incrementRevision(currentRule, updatedParams); + const revision = incrementRevision( + currentRule, + { id, data, allowMissingConnectorSecrets }, + updatedParams + ); let updatedObject: SavedObject; const createAttributes = updateMeta(context, { diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group2/update.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group2/update.ts index 0764ae70d6df1..80580e3c8953e 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group2/update.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group2/update.ts @@ -60,6 +60,7 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { mute_all: false, muted_alert_ids: [], notify_when: 'onThrottleInterval', + revision: 1, scheduled_task_id: createdAlert.scheduled_task_id, created_at: response.body.created_at, updated_at: response.body.updated_at, @@ -125,6 +126,8 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { }); }); + // TODO: Port over SecuritySolution `version` update() FTR tests + describe('legacy', () => { it('should handle update alert request appropriately', async () => { const { body: createdAlert } = await supertest diff --git a/x-pack/test/detection_engine_api_integration/basic/tests/import_rules.ts b/x-pack/test/detection_engine_api_integration/basic/tests/import_rules.ts index 2630174529b75..90a75b0d1146d 100644 --- a/x-pack/test/detection_engine_api_integration/basic/tests/import_rules.ts +++ b/x-pack/test/detection_engine_api_integration/basic/tests/import_rules.ts @@ -361,6 +361,7 @@ export default ({ getService }: FtrProviderContext): void => { }; ruleOutput.name = 'some other name'; ruleOutput.version = 2; + ruleOutput.revision = 1; expect(bodyToCompare).to.eql(ruleOutput); }); diff --git a/x-pack/test/detection_engine_api_integration/utils/get_complex_rule_output.ts b/x-pack/test/detection_engine_api_integration/utils/get_complex_rule_output.ts index a8f5916c3598d..af66123014383 100644 --- a/x-pack/test/detection_engine_api_integration/utils/get_complex_rule_output.ts +++ b/x-pack/test/detection_engine_api_integration/utils/get_complex_rule_output.ts @@ -51,6 +51,7 @@ export const getComplexRuleOutput = (ruleId = 'rule-1'): Partial = tags: ['tag 1', 'tag 2', 'any tag you want'], to: 'now', from: 'now-6m', + revision: 0, severity: 'high', severity_mapping: [], language: 'kuery', From 4a80747b35eb8ef4b8af08c5c09c5b064f775429 Mon Sep 17 00:00:00 2001 From: Garrett Spong Date: Wed, 8 Feb 2023 16:31:08 -0700 Subject: [PATCH 12/28] Resolving remaining test failures --- .../server/rules_client/tests/update.test.ts | 17 ++++++++++++++--- .../logic/export/get_export_all.test.ts | 1 + .../export/get_export_by_object_ids.test.ts | 1 + .../apis/synthetics/enable_default_alerting.ts | 1 + .../basic/tests/update_rules.ts | 5 +++++ .../basic/tests/update_rules_bulk.ts | 9 +++++++++ .../security_and_spaces/group1/create_rules.ts | 1 + .../group1/update_actions.ts | 10 ++++++---- .../security_and_spaces/group10/import_rules.ts | 1 + .../security_and_spaces/group10/patch_rules.ts | 4 ++-- .../group10/patch_rules_bulk.ts | 1 + .../security_and_spaces/group10/update_rules.ts | 2 +- .../utils/rule_to_update_schema.ts | 1 + .../security_solution/legacy_actions/data.json | 8 ++++++++ 14 files changed, 52 insertions(+), 10 deletions(-) diff --git a/x-pack/plugins/alerting/server/rules_client/tests/update.test.ts b/x-pack/plugins/alerting/server/rules_client/tests/update.test.ts index f2f6d86ee14a2..c352b45e7dc99 100644 --- a/x-pack/plugins/alerting/server/rules_client/tests/update.test.ts +++ b/x-pack/plugins/alerting/server/rules_client/tests/update.test.ts @@ -85,6 +85,7 @@ describe('update()', () => { alertTypeId: 'myType', schedule: { interval: '1m' }, consumer: 'myApp', + revision: 0, scheduledTaskId: 'task-123', params: {}, actions: [ @@ -105,7 +106,6 @@ describe('update()', () => { ], }, references: [], - revision: 0, version: '123', }; const existingDecryptedAlert = { @@ -231,6 +231,7 @@ describe('update()', () => { }, ], notifyWhen: 'onActiveAlert', + revision: 1, scheduledTaskId: 'task-123', createdAt: new Date().toISOString(), updatedAt: new Date().toISOString(), @@ -326,6 +327,7 @@ describe('update()', () => { "params": Object { "bar": true, }, + "revision": 1, "schedule": Object { "interval": "1m", }, @@ -521,6 +523,7 @@ describe('update()', () => { }, ], notifyWhen: 'onActiveAlert', + revision: 1, scheduledTaskId: 'task-123', createdAt: new Date().toISOString(), updatedAt: new Date().toISOString(), @@ -575,7 +578,6 @@ describe('update()', () => { }, }); - // TODO: Review update results once increment logic is finalized expect(unsecuredSavedObjectsClient.create).toHaveBeenNthCalledWith( 1, 'alert', @@ -666,6 +668,7 @@ describe('update()', () => { "params": Object { "bar": true, }, + "revision": 1, "schedule": Object { "interval": "1m", }, @@ -740,6 +743,7 @@ describe('update()', () => { }, ], notifyWhen: 'onActiveAlert', + revision: 1, scheduledTaskId: 'task-123', createdAt: new Date().toISOString(), updatedAt: new Date().toISOString(), @@ -778,7 +782,6 @@ describe('update()', () => { }, }); - // TODO: Review update results once increment logic is finalized expect(extractReferencesFn).toHaveBeenCalledWith(ruleParams); expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith( 'alert', @@ -841,6 +844,7 @@ describe('update()', () => { "bar": true, "parameterThatIsSavedObjectId": "9", }, + "revision": 1, "schedule": Object { "interval": "1m", }, @@ -878,6 +882,7 @@ describe('update()', () => { }, ], apiKey: Buffer.from('123:abc').toString('base64'), + revision: 1, scheduledTaskId: 'task-123', }, updated_at: new Date().toISOString(), @@ -931,6 +936,7 @@ describe('update()', () => { "params": Object { "bar": true, }, + "revision": 1, "schedule": Object { "interval": "1m", }, @@ -1033,6 +1039,7 @@ describe('update()', () => { }, }, ], + revision: 1, scheduledTaskId: 'task-123', apiKey: null, }, @@ -1088,6 +1095,7 @@ describe('update()', () => { "params": Object { "bar": true, }, + "revision": 1, "schedule": Object { "interval": "1m", }, @@ -2068,6 +2076,7 @@ describe('update()', () => { }, ], notifyWhen: 'onActiveAlert', + revision: 1, scheduledTaskId: 'task-123', createdAt: new Date().toISOString(), updatedAt: new Date().toISOString(), @@ -2132,6 +2141,7 @@ describe('update()', () => { name: 'abc', notifyWhen: 'onActiveAlert', params: { bar: true }, + revision: 1, schedule: { interval: '1m' }, scheduledTaskId: 'task-123', tags: ['foo'], @@ -2166,6 +2176,7 @@ describe('update()', () => { "params": Object { "bar": true, }, + "revision": 1, "schedule": Object { "interval": "1m", }, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_all.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_all.test.ts index eabf7fb8c95e8..b38329bdab3da 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_all.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_all.test.ts @@ -288,6 +288,7 @@ describe('getExportAll', () => { throttle: 'rule', note: '# Investigative notes', version: 1, + revision: 0, exceptions_list: getListArrayMock(), }); expect(detailsJson).toEqual({ diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_by_object_ids.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_by_object_ids.test.ts index f0e00ed5c19d2..40a1435b2d64f 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_by_object_ids.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_by_object_ids.test.ts @@ -298,6 +298,7 @@ describe('get_export_by_object_ids', () => { throttle: 'rule', note: '# Investigative notes', version: 1, + revision: 0, exceptions_list: getListArrayMock(), }); expect(detailsJson).toEqual({ diff --git a/x-pack/test/api_integration/apis/synthetics/enable_default_alerting.ts b/x-pack/test/api_integration/apis/synthetics/enable_default_alerting.ts index ad47a031f0628..b1e587eedb5ea 100644 --- a/x-pack/test/api_integration/apis/synthetics/enable_default_alerting.ts +++ b/x-pack/test/api_integration/apis/synthetics/enable_default_alerting.ts @@ -81,6 +81,7 @@ export default function ({ getService }: FtrProviderContext) { executionStatus: { status: 'pending', lastExecutionDate: '2022-12-20T09:10:15.500Z' }, ruleTypeId: 'xpack.synthetics.alerts.monitorStatus', running: false, + revision: 0, }, omitFields ) diff --git a/x-pack/test/detection_engine_api_integration/basic/tests/update_rules.ts b/x-pack/test/detection_engine_api_integration/basic/tests/update_rules.ts index 0a8ac78019630..6f913739b6f18 100644 --- a/x-pack/test/detection_engine_api_integration/basic/tests/update_rules.ts +++ b/x-pack/test/detection_engine_api_integration/basic/tests/update_rules.ts @@ -57,6 +57,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutput(); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body); expect(bodyToCompare).to.eql(outputRule); }); @@ -102,6 +103,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutputWithoutRuleId(); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedPropertiesIncludingRuleId(body); expect(bodyToCompare).to.eql(outputRule); }); @@ -124,6 +126,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutput(); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body); expect(bodyToCompare).to.eql(outputRule); }); @@ -146,6 +149,7 @@ export default ({ getService }: FtrProviderContext) => { outputRule.enabled = false; outputRule.severity = 'low'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body); expect(bodyToCompare).to.eql(outputRule); @@ -178,6 +182,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutput(); outputRule.name = 'some other name'; outputRule.version = 3; + outputRule.revision = 2; const bodyToCompare = removeServerGeneratedProperties(body); expect(bodyToCompare).to.eql(outputRule); diff --git a/x-pack/test/detection_engine_api_integration/basic/tests/update_rules_bulk.ts b/x-pack/test/detection_engine_api_integration/basic/tests/update_rules_bulk.ts index 2c02e09a5cfa1..d2dee431fc2cd 100644 --- a/x-pack/test/detection_engine_api_integration/basic/tests/update_rules_bulk.ts +++ b/x-pack/test/detection_engine_api_integration/basic/tests/update_rules_bulk.ts @@ -57,6 +57,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutput(); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body[0]); expect(bodyToCompare).to.eql(outputRule); }); @@ -87,10 +88,12 @@ export default ({ getService }: FtrProviderContext) => { const outputRule1 = getSimpleRuleOutput(); outputRule1.name = 'some other name'; outputRule1.version = 2; + outputRule1.revision = 1; const outputRule2 = getSimpleRuleOutput('rule-2'); outputRule2.name = 'some other name'; outputRule2.version = 2; + outputRule2.revision = 1; const bodyToCompare1 = removeServerGeneratedProperties(body[0]); const bodyToCompare2 = removeServerGeneratedProperties(body[1]); @@ -116,6 +119,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutput(); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body[0]); expect(bodyToCompare).to.eql(outputRule); }); @@ -144,10 +148,12 @@ export default ({ getService }: FtrProviderContext) => { const outputRule1 = getSimpleRuleOutputWithoutRuleId('rule-1'); outputRule1.name = 'some other name'; outputRule1.version = 2; + outputRule1.revision = 1; const outputRule2 = getSimpleRuleOutputWithoutRuleId('rule-2'); outputRule2.name = 'some other name'; outputRule2.version = 2; + outputRule2.revision = 1; const bodyToCompare1 = removeServerGeneratedPropertiesIncludingRuleId(body[0]); const bodyToCompare2 = removeServerGeneratedPropertiesIncludingRuleId(body[1]); @@ -173,6 +179,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutput(); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body[0]); expect(bodyToCompare).to.eql(outputRule); }); @@ -296,6 +303,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutput(); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body[0]); expect([bodyToCompare, body[1]]).to.eql([ @@ -333,6 +341,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutput(); outputRule.name = 'some other name'; outputRule.version = 2; + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body[0]); expect([bodyToCompare, body[1]]).to.eql([ diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/group1/create_rules.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/group1/create_rules.ts index 6ddbe82e7d5cf..8abd4cd741bbe 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/group1/create_rules.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/group1/create_rules.ts @@ -192,6 +192,7 @@ export default ({ getService }: FtrProviderContext) => { references: [], related_integrations: [], required_fields: [], + revision: 0, setup: '', severity: 'high', severity_mapping: [], diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/group1/update_actions.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/group1/update_actions.ts index 4897805e09eb2..7930bf7433da0 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/group1/update_actions.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/group1/update_actions.ts @@ -72,6 +72,7 @@ export default ({ getService }: FtrProviderContext) => { const expected = { ...getSimpleRuleOutputWithWebHookAction(`${bodyToCompare.actions?.[0].id}`), + revision: 1, // version bump is required since this is an updated rule and this is part of the testing that we do bump the version number on update version: 2, // version bump is required since this is an updated rule and this is part of the testing that we do bump the version number on update }; expect(bodyToCompare).to.eql(expected); @@ -87,6 +88,7 @@ export default ({ getService }: FtrProviderContext) => { const bodyToCompare = removeServerGeneratedProperties(ruleAfterActionRemoved); const expected = { ...getSimpleRuleOutput(), + revision: 2, // version bump is required since this is an updated rule and this is part of the testing that we do bump the version number on update version: 3, // version bump is required since this is an updated rule and this is part of the testing that we do bump the version number on update }; expect(bodyToCompare).to.eql(expected); @@ -127,10 +129,10 @@ export default ({ getService }: FtrProviderContext) => { const updatedRule = await updateRule(supertest, log, ruleToUpdate); const expected = omit(removeServerGeneratedProperties(updatedRule), actionsProps); - const immutableRuleToAssert = omit( - removeServerGeneratedProperties(immutableRule), - actionsProps - ); + const immutableRuleToAssert = { + ...omit(removeServerGeneratedProperties(immutableRule), actionsProps), + revision: 1, // Unlike `version` which is static for immutable rules, `revision` will increment when an action/exception is added + }; expect(immutableRuleToAssert).to.eql(expected); expect(expected.immutable).to.be(true); // It should stay immutable true when returning diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_rules.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_rules.ts index 169fe074604fa..bc8ea846345fc 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_rules.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_rules.ts @@ -637,6 +637,7 @@ export default ({ getService }: FtrProviderContext): void => { }; ruleOutput.name = 'some other name'; ruleOutput.version = 2; + ruleOutput.revision = 1; // TODO: Should we match `version` logic or reset to `0` as it's technically a new rule? expect(bodyToCompare).to.eql(ruleOutput); }); diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/patch_rules.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/patch_rules.ts index bc6221f061cb2..65ee926d2b57b 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/patch_rules.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/patch_rules.ts @@ -70,8 +70,7 @@ export default ({ getService }: FtrProviderContext) => { .expect(200); const outputRule = getSimpleMlRuleOutput(); - outputRule.version = 2; - outputRule.revision = 1; + outputRule.version = 2; // NOTE: Revision is not updated as `machine_learning_job_id` value doesn't actually change const bodyToCompare = removeServerGeneratedProperties(body); expect(bodyToCompare).to.eql(outputRule); }); @@ -379,6 +378,7 @@ export default ({ getService }: FtrProviderContext) => { }, ]; outputRule.throttle = '1h'; + outputRule.revision = 2; // Expected revision is 2 as call to `createLegacyRuleAction()` does two separate rules updates for `notifyWhen` & `actions` field const bodyToCompare = removeServerGeneratedProperties(patchResponse.body); expect(bodyToCompare).to.eql(outputRule); }); diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/patch_rules_bulk.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/patch_rules_bulk.ts index ad6e22e042da3..374437b50e7a8 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/patch_rules_bulk.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/patch_rules_bulk.ts @@ -200,6 +200,7 @@ export default ({ getService }: FtrProviderContext) => { }, ]; outputRule.throttle = '1h'; + outputRule.revision = 2; // Expected revision is 2 as call to `createLegacyRuleAction()` does two separate rules updates for `notifyWhen` & `actions` field const bodyToCompare = removeServerGeneratedProperties(response); expect(bodyToCompare).to.eql(outputRule); }); diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/update_rules.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/update_rules.ts index 5248c5625bcbe..7bd459890fad1 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/update_rules.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/update_rules.ts @@ -237,7 +237,7 @@ export default ({ getService }: FtrProviderContext) => { const outputRule = getSimpleRuleOutputWithoutRuleId(); outputRule.name = 'some other name'; outputRule.version = 2; - outputRule.revision = 1; + outputRule.revision = 2; // Migration of action results in additional revision increment (change to `notifyWhen`), so expected revision is 2 outputRule.actions = [ { action_type_id: '.slack', diff --git a/x-pack/test/detection_engine_api_integration/utils/rule_to_update_schema.ts b/x-pack/test/detection_engine_api_integration/utils/rule_to_update_schema.ts index 7c10c98c105d6..b4d2759ccf67d 100644 --- a/x-pack/test/detection_engine_api_integration/utils/rule_to_update_schema.ts +++ b/x-pack/test/detection_engine_api_integration/utils/rule_to_update_schema.ts @@ -20,6 +20,7 @@ const propertiesToRemove = [ 'created_by', 'related_integrations', 'required_fields', + 'revision', 'setup', 'execution_summary', ]; diff --git a/x-pack/test/functional/es_archives/security_solution/legacy_actions/data.json b/x-pack/test/functional/es_archives/security_solution/legacy_actions/data.json index f0c883c6b3756..8dbc0fbe883e8 100644 --- a/x-pack/test/functional/es_archives/security_solution/legacy_actions/data.json +++ b/x-pack/test/functional/es_archives/security_solution/legacy_actions/data.json @@ -74,6 +74,7 @@ ], "alertTypeId" : "siem.queryRule", "consumer" : "siem", + "revision": 0, "params" : { "author" : [ ], "description" : "a", @@ -168,6 +169,7 @@ ], "alertTypeId" : "siem.queryRule", "consumer" : "siem", + "revision": 0, "params" : { "author" : [ ], "description" : "a", @@ -281,6 +283,7 @@ ], "alertTypeId" : "siem.queryRule", "consumer" : "siem", + "revision": 0, "params" : { "author" : [ ], "description" : "a", @@ -375,6 +378,7 @@ ], "alertTypeId" : "siem.queryRule", "consumer" : "siem", + "revision": 0, "params" : { "author" : [ ], "description" : "a", @@ -469,6 +473,7 @@ ], "alertTypeId" : "siem.queryRule", "consumer" : "siem", + "revision": 0, "params" : { "author" : [ ], "description" : "a", @@ -561,6 +566,7 @@ ], "alertTypeId" : "siem.notifications", "consumer" : "siem", + "revision": 0, "params" : { "ruleAlertId" : "61ec7a40-b076-11ec-bb3f-1f063f8e06cf" }, @@ -641,6 +647,7 @@ ], "alertTypeId" : "siem.notifications", "consumer" : "siem", + "revision": 0, "params" : { "ruleAlertId" : "064e3160-b076-11ec-bb3f-1f063f8e06cf" }, @@ -733,6 +740,7 @@ ], "alertTypeId" : "siem.notifications", "consumer" : "siem", + "revision": 0, "params" : { "ruleAlertId" : "27639570-b076-11ec-bb3f-1f063f8e06cf" }, From e8f40c66f270d13f1dd4c7a41ab9ce0e1315d050 Mon Sep 17 00:00:00 2001 From: Garrett Spong Date: Fri, 10 Feb 2023 11:11:21 -0700 Subject: [PATCH 13/28] Update alert SO hash --- .../migrations/group2/check_registered_types.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts b/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts index 0f6b7729cf17e..9451089138315 100644 --- a/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts +++ b/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts @@ -57,7 +57,7 @@ describe('checking migration metadata changes on all registered SO types', () => Object { "action": "6cfc277ed3211639e37546ac625f4a68f2494215", "action_task_params": "db2afea7d78e00e725486b791554d0d4e81956ef", - "alert": "12dc3b249eab1e03eae23eadb05f5662edc11620", + "alert": "96b59d2343980c7c11a7dc701fe2270877aa0c4e", "api_key_pending_invalidation": "16e7bcf8e78764102d7f525542d5b616809a21ee", "apm-indices": "d19dd7fb51f2d2cbc1f8769481721e0953f9a6d2", "apm-server-schema": "1d42f17eff9ec6c16d3a9324d9539e2d123d0a9a", From d162cd877cf164568223b0162a26940ad118cd96 Mon Sep 17 00:00:00 2001 From: Garrett Spong Date: Tue, 14 Feb 2023 14:13:04 -0700 Subject: [PATCH 14/28] Remove specific version check from increment logic --- .../alerting/server/rules_client/lib/increment_revision.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/alerting/server/rules_client/lib/increment_revision.ts b/x-pack/plugins/alerting/server/rules_client/lib/increment_revision.ts index 9f5a075506eec..37db70bdbf55e 100644 --- a/x-pack/plugins/alerting/server/rules_client/lib/increment_revision.ts +++ b/x-pack/plugins/alerting/server/rules_client/lib/increment_revision.ts @@ -26,9 +26,8 @@ export function incrementRevision( // Diff rule params for (const [field, value] of Object.entries(updatedParams)) { - // TODO: Remove 'version' from SecuritySolution (and maybe have a way for - // TODO: RuleTypes to declare fields that can be revision skipped as well?) - if (!fieldsToExcludeFromRevisionUpdates.map(toString).includes(field) && field !== 'version') { + // TODO: Should RuleTypes have a way to declare fields that should be revision skipped as well? + if (!fieldsToExcludeFromRevisionUpdates.map(toString).includes(field)) { if (!isEqual(value, get(currentRule.attributes.params, field))) { return currentRule.attributes.revision + 1; } From a71a309b155640fec58e2269553fbf4d4b95723a Mon Sep 17 00:00:00 2001 From: Garrett Spong Date: Tue, 14 Feb 2023 18:55:54 -0700 Subject: [PATCH 15/28] Adds migration tests, updates bulk edit snooze tests, update_api_key logic, and spaces_only update tests --- .../server/routes/lib/rewrite_rule.test.ts | 1 + .../server/rules_client/methods/bulk_edit.ts | 1 + .../rules_client/methods/update_api_key.ts | 1 + .../saved_objects/migrations/index.test.ts | 10 ++++++++ .../server/saved_objects/migrations/index.ts | 2 ++ .../group2/tests/alerting/update_api_key.ts | 10 ++++++++ .../tests/alerting/group1/disable.ts | 24 ++++++++++++++++++ .../tests/alerting/group1/enable.ts | 6 +++++ .../tests/alerting/group2/update.ts | 2 -- .../tests/alerting/group2/update_api_key.ts | 6 +++++ .../tests/alerting/group4/bulk_edit.ts | 25 +++++++++++++++++++ .../group10/patch_rules.ts | 5 +++- 12 files changed, 90 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/alerting/server/routes/lib/rewrite_rule.test.ts b/x-pack/plugins/alerting/server/routes/lib/rewrite_rule.test.ts index b4d1962931218..64326075b495a 100644 --- a/x-pack/plugins/alerting/server/routes/lib/rewrite_rule.test.ts +++ b/x-pack/plugins/alerting/server/routes/lib/rewrite_rule.test.ts @@ -60,6 +60,7 @@ const sampleRule: SanitizedRule & { activeSnoozes?: string[] } = }, }, nextRun: DATE_2020, + revision: 0, }; describe('rewriteRule', () => { diff --git a/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts b/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts index daa3817a1abae..3d2d1ccb54368 100644 --- a/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts +++ b/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts @@ -437,6 +437,7 @@ async function updateRuleAttributesAndParamsInMemory { }); }); + describe('8.8.0', () => { + test('migrates rule to include revision and defaults revision to 0', () => { + const migration880 = getMigrations(encryptedSavedObjectsSetup, {}, isPreconfigured)['8.8.0']; + + const rule = getMockData(); + const migratedAlert880 = migration880(rule, migrationContext); + expect(migratedAlert880.attributes.revision).toEqual(0); + }); + }); + describe('Metrics Inventory Threshold rule', () => { test('Migrates incorrect action group spelling', () => { const migration800 = getMigrations(encryptedSavedObjectsSetup, {}, isPreconfigured)['8.0.0']; diff --git a/x-pack/plugins/alerting/server/saved_objects/migrations/index.ts b/x-pack/plugins/alerting/server/saved_objects/migrations/index.ts index 5a88af4062b19..b94fb907d8275 100644 --- a/x-pack/plugins/alerting/server/saved_objects/migrations/index.ts +++ b/x-pack/plugins/alerting/server/saved_objects/migrations/index.ts @@ -30,6 +30,7 @@ import { getMigrations841 } from './8.4'; import { getMigrations850 } from './8.5'; import { getMigrations860 } from './8.6'; import { getMigrations870 } from './8.7'; +import { getMigrations880 } from './8.8'; import { AlertLogMeta, AlertMigration } from './types'; import { MINIMUM_SS_MIGRATION_VERSION } from './constants'; import { createEsoMigration, isEsQueryRuleType, pipeMigrations } from './utils'; @@ -79,6 +80,7 @@ export function getMigrations( '8.5.0': executeMigrationWithErrorHandling(getMigrations850(encryptedSavedObjects), '8.5.0'), '8.6.0': executeMigrationWithErrorHandling(getMigrations860(encryptedSavedObjects), '8.6.0'), '8.7.0': executeMigrationWithErrorHandling(getMigrations870(encryptedSavedObjects), '8.7.0'), + '8.8.0': executeMigrationWithErrorHandling(getMigrations880(encryptedSavedObjects), '8.8.0'), }, getSearchSourceMigrations(encryptedSavedObjects, searchSourceMigrations) ); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/update_api_key.ts b/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/update_api_key.ts index a0d1eb4dd0756..417b5d1e95845 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/update_api_key.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/alerting/update_api_key.ts @@ -98,6 +98,8 @@ export default function createUpdateApiKeyTests({ getService }: FtrProviderConte .auth(user.username, user.password) .expect(200); expect(updatedAlert.api_key_owner).to.eql(user.username); + // Ensure revision is incremented when API key is updated + expect(updatedAlert.revision).to.eql(1); // Ensure AAD isn't broken await checkAAD({ supertest, @@ -152,6 +154,8 @@ export default function createUpdateApiKeyTests({ getService }: FtrProviderConte .auth(user.username, user.password) .expect(200); expect(updatedAlert.api_key_owner).to.eql(user.username); + // Ensure revision is incremented when API key is updated + expect(updatedAlert.revision).to.eql(1); // Ensure AAD isn't broken await checkAAD({ supertest, @@ -217,6 +221,8 @@ export default function createUpdateApiKeyTests({ getService }: FtrProviderConte .auth(user.username, user.password) .expect(200); expect(updatedAlert.api_key_owner).to.eql(user.username); + // Ensure revision is incremented when API key is updated + expect(updatedAlert.revision).to.eql(1); // Ensure AAD isn't broken await checkAAD({ supertest, @@ -282,6 +288,8 @@ export default function createUpdateApiKeyTests({ getService }: FtrProviderConte .auth(user.username, user.password) .expect(200); expect(updatedAlert.api_key_owner).to.eql(user.username); + // Ensure revision is incremented when API key is updated + expect(updatedAlert.revision).to.eql(1); // Ensure AAD isn't broken await checkAAD({ supertest, @@ -346,6 +354,8 @@ export default function createUpdateApiKeyTests({ getService }: FtrProviderConte .auth(user.username, user.password) .expect(200); expect(updatedAlert.api_key_owner).to.eql(user.username); + // Ensure revision is incremented when API key is updated + expect(updatedAlert.revision).to.eql(1); // Ensure AAD isn't broken await checkAAD({ supertest, diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/disable.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/disable.ts index 1ff7aab8f4ca6..c4620416f4bd6 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/disable.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/disable.ts @@ -63,6 +63,14 @@ export default function createDisableRuleTests({ getService }: FtrProviderContex expect(taskRecord.task.enabled).to.eql(false); }); + const { body: disabledRule } = await supertestWithoutAuth + .get(`${getUrlPrefix(Spaces.space1.id)}/api/alerting/rule/${createdRule.id}`) + .set('kbn-xsrf', 'foo') + .expect(200); + + // Ensure revision was not updated + expect(disabledRule.revision).to.eql(0); + // Ensure AAD isn't broken await checkAAD({ supertest: supertestWithoutAuth, @@ -173,6 +181,14 @@ export default function createDisableRuleTests({ getService }: FtrProviderContex }); await ruleUtils.disable(createdRule.id); + const { body: disabledRule } = await supertestWithoutAuth + .get(`${getUrlPrefix(Spaces.space1.id)}/api/alerting/rule/${createdRule.id}`) + .set('kbn-xsrf', 'foo') + .expect(200); + + // Ensure revision was not updated + expect(disabledRule.revision).to.eql(0); + // Ensure AAD isn't broken await checkAAD({ supertest: supertestWithoutAuth, @@ -209,6 +225,14 @@ export default function createDisableRuleTests({ getService }: FtrProviderContex expect(taskRecord.task.enabled).to.eql(false); }); + const { body: disabledRule } = await supertestWithoutAuth + .get(`${getUrlPrefix(Spaces.space1.id)}/api/alerting/rule/${createdRule.id}`) + .set('kbn-xsrf', 'foo') + .expect(200); + + // Ensure revision was not updated + expect(disabledRule.revision).to.eql(0); + // Ensure AAD isn't broken await checkAAD({ supertest: supertestWithoutAuth, diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/enable.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/enable.ts index f4ad874d3357e..b9538c786a496 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/enable.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/enable.ts @@ -61,6 +61,9 @@ export default function createEnableAlertTests({ getService }: FtrProviderContex }); expect(taskRecord.task.enabled).to.eql(true); + // Ensure revision was not updated + expect(updatedAlert.revision).to.eql(0); + // Ensure AAD isn't broken await checkAAD({ supertest: supertestWithoutAuth, @@ -114,6 +117,9 @@ export default function createEnableAlertTests({ getService }: FtrProviderContex }); expect(taskRecord.task.enabled).to.eql(true); + // Ensure revision was not updated + expect(updatedAlert.revision).to.eql(0); + // Ensure AAD isn't broken await checkAAD({ supertest: supertestWithoutAuth, diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group2/update.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group2/update.ts index 80580e3c8953e..d3ef53d5d4bd8 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group2/update.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group2/update.ts @@ -126,8 +126,6 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { }); }); - // TODO: Port over SecuritySolution `version` update() FTR tests - describe('legacy', () => { it('should handle update alert request appropriately', async () => { const { body: createdAlert } = await supertest diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group2/update_api_key.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group2/update_api_key.ts index a57d9dc90fd6d..96c5de71afd9f 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group2/update_api_key.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group2/update_api_key.ts @@ -46,6 +46,9 @@ export default function createUpdateApiKeyTests({ getService }: FtrProviderConte .expect(200); expect(updatedAlert.api_key_owner).to.eql(null); + // Ensure revision is incremented when API key is updated + expect(updatedAlert.revision).to.eql(1); + // Ensure AAD isn't broken await checkAAD({ supertest: supertestWithoutAuth, @@ -92,6 +95,9 @@ export default function createUpdateApiKeyTests({ getService }: FtrProviderConte .expect(200); expect(updatedAlert.api_key_owner).to.eql(null); + // Ensure revision is incremented when API key is updated + expect(updatedAlert.revision).to.eql(1); + // Ensure AAD isn't broken await checkAAD({ supertest: supertestWithoutAuth, diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group4/bulk_edit.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group4/bulk_edit.ts index fa039cc3f0533..f8b318777092b 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group4/bulk_edit.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group4/bulk_edit.ts @@ -76,6 +76,9 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { expect(updatedRule.tags).to.eql(['default', 'tag-1']); + // Ensure revision is updated + expect(updatedRule.revision).to.eql(1); + // Ensure AAD isn't broken await checkAAD({ supertest, @@ -137,6 +140,7 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { updatedRules.forEach((rule) => { expect(rule.tags).to.eql([`rewritten`]); + expect(rule.revision).to.eql(1); }); }); @@ -174,6 +178,9 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { expect(updatedRule.schedule).to.eql({ interval: '1h' }); + // Ensure revision is updated + expect(updatedRule.revision).to.eql(1); + // Ensure AAD isn't broken await checkAAD({ supertest, @@ -217,6 +224,9 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { expect(updatedRule).property('throttle', '1h'); + // Ensure revision is updated + expect(updatedRule.revision).to.eql(1); + // Ensure AAD isn't broken await checkAAD({ supertest, @@ -260,6 +270,9 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { expect(updatedRule).property('notify_when', 'onActionGroupChange'); + // Ensure revision is updated + expect(updatedRule.revision).to.eql(1); + // Ensure AAD isn't broken await checkAAD({ supertest, @@ -305,6 +318,9 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { expect(bulkSnoozeResponse.body.rules[0].snooze_schedule.length).to.eql(1); expect(bulkSnoozeResponse.body.rules[0].snooze_schedule[0].duration).to.eql(28800000); + // Ensure revision is updated TODO: TBD on if snooze actually should increment revision + expect(bulkSnoozeResponse.body.rules[0].revision).to.eql(1); + const bulkUnsnoozeResponse = await supertest .post(`${getUrlPrefix(Spaces.space1.id)}/internal/alerting/rules/_bulk_edit`) .set('kbn-xsrf', 'foo') @@ -322,6 +338,9 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { expect(bulkUnsnoozeResponse.body.rules).to.have.length(1); expect(bulkUnsnoozeResponse.body.rules[0].snooze_schedule).empty(); + // Ensure revision is updated TODO: TBD on if snooze actually should increment revision + expect(bulkSnoozeResponse.body.rules[0].revision).to.eql(2); + // Ensure AAD isn't broken await checkAAD({ supertest, @@ -378,6 +397,8 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { expect(bulkSnoozeResponse.body.errors).to.have.length(0); expect(bulkSnoozeResponse.body.rules).to.have.length(1); expect(bulkSnoozeResponse.body.rules[0].snooze_schedule.length).to.eql(5); + // Ensure revision is updated TODO: TBD on if snooze actually should increment revision + expect(bulkSnoozeResponse.body.rules[0].revision).to.eql(1); // Try adding more than 5 schedules const bulkSnoozeError = await supertest @@ -433,6 +454,8 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { expect(bulkSnoozeResponse.body.errors).to.have.length(0); expect(bulkSnoozeResponse.body.rules).to.have.length(1); expect(bulkSnoozeResponse.body.rules[0].snooze_schedule).empty(); + // Ensure revision isn't updated + expect(bulkSnoozeResponse.body.rules[0].revision).to.eql(0); // Ensure AAD isn't broken await checkAAD({ @@ -473,6 +496,8 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { expect(bulkApiKeyResponse.body.errors).to.have.length(0); expect(bulkApiKeyResponse.body.rules).to.have.length(1); expect(bulkApiKeyResponse.body.rules[0].api_key_owner).to.eql(null); + // Ensure revision is updated + expect(bulkApiKeyResponse.body.rules[0].revision).to.eql(1); }); it(`shouldn't bulk edit rule from another space`, async () => { diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/patch_rules.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/patch_rules.ts index 65ee926d2b57b..7263d85459f88 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/patch_rules.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/patch_rules.ts @@ -70,7 +70,10 @@ export default ({ getService }: FtrProviderContext) => { .expect(200); const outputRule = getSimpleMlRuleOutput(); - outputRule.version = 2; // NOTE: Revision is not updated as `machine_learning_job_id` value doesn't actually change + outputRule.version = 2; + // TODO: Followup to #147398 + // NOTE: Once we remove `version` increment, revision will not be updated as `machine_learning_job_id` value doesn't actually change + outputRule.revision = 1; const bodyToCompare = removeServerGeneratedProperties(body); expect(bodyToCompare).to.eql(outputRule); }); From a1a21e65e806fc5289f772693c1e21ff044e6249 Mon Sep 17 00:00:00 2001 From: Garrett Spong Date: Tue, 14 Feb 2023 21:05:48 -0700 Subject: [PATCH 16/28] Fixes updateAPIKey tests and reset SO registered type hash --- .../migrations/group2/check_registered_types.test.ts | 2 +- .../alerting/server/rules_client/tests/update_api_key.test.ts | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts b/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts index 0deda19ec53ac..828252d7a4485 100644 --- a/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts +++ b/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts @@ -57,7 +57,7 @@ describe('checking migration metadata changes on all registered SO types', () => Object { "action": "6cfc277ed3211639e37546ac625f4a68f2494215", "action_task_params": "db2afea7d78e00e725486b791554d0d4e81956ef", - "alert": "96b59d2343980c7c11a7dc701fe2270877aa0c4e", + "alert": "38213efd0ec097dcf9a0d9d608f957ae8724e387", "api_key_pending_invalidation": "16e7bcf8e78764102d7f525542d5b616809a21ee", "apm-indices": "d19dd7fb51f2d2cbc1f8769481721e0953f9a6d2", "apm-server-schema": "1d42f17eff9ec6c16d3a9324d9539e2d123d0a9a", diff --git a/x-pack/plugins/alerting/server/rules_client/tests/update_api_key.test.ts b/x-pack/plugins/alerting/server/rules_client/tests/update_api_key.test.ts index e2841ba4927c6..1738a424205fc 100644 --- a/x-pack/plugins/alerting/server/rules_client/tests/update_api_key.test.ts +++ b/x-pack/plugins/alerting/server/rules_client/tests/update_api_key.test.ts @@ -64,6 +64,7 @@ describe('updateApiKey()', () => { id: '1', type: 'alert', attributes: { + revision: 0, schedule: { interval: '10s' }, alertTypeId: 'myType', consumer: 'myApp', @@ -117,6 +118,7 @@ describe('updateApiKey()', () => { enabled: true, apiKey: Buffer.from('234:abc').toString('base64'), apiKeyOwner: 'elastic', + revision: 1, updatedBy: 'elastic', updatedAt: '2019-02-12T21:01:22.479Z', actions: [ @@ -177,6 +179,7 @@ describe('updateApiKey()', () => { enabled: true, apiKey: Buffer.from('234:abc').toString('base64'), apiKeyOwner: 'elastic', + revision: 1, updatedAt: '2019-02-12T21:01:22.479Z', updatedBy: 'elastic', actions: [ From 88a18f8458c17aa3c1bb5d38a3227d191e30eae1 Mon Sep 17 00:00:00 2001 From: Garrett Spong Date: Wed, 15 Feb 2023 15:45:41 -0700 Subject: [PATCH 17/28] Removes revision update on snooze/mute/apiKeyUpdate and updates bulkEdit logic --- .../server/rules_client/methods/bulk_edit.ts | 6 ++---- .../rules_client/methods/update_api_key.ts | 1 - .../server/rules_client/rules_client.ts | 21 ++++++++++++------- .../rules_client/tests/update_api_key.test.ts | 4 ++-- .../logic/actions/duplicate_rule.ts | 3 --- .../group2/tests/alerting/update_api_key.ts | 20 +++++++++--------- .../tests/alerting/group2/update_api_key.ts | 8 +++---- .../tests/alerting/group4/bulk_edit.ts | 10 ++++----- 8 files changed, 37 insertions(+), 36 deletions(-) diff --git a/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts b/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts index 3d2d1ccb54368..64cffb4bb0ee8 100644 --- a/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts +++ b/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts @@ -435,10 +435,6 @@ async function updateRuleAttributesAndParamsInMemory = [ ]; export const fieldsToExcludeFromRevisionUpdates: Array = [ - 'id', - 'enabled', - 'consumer', + 'activeSnoozes', 'alertTypeId', - 'createdBy', - 'updatedBy', + 'apiKey', + 'apiKeyOwner', + 'consumer', 'createdAt', - 'updatedAt', + 'createdBy', + 'enabled', 'executionStatus', - 'monitoring', + 'id', + 'isSnoozedUntil', 'lastRun', + 'monitoring', + 'muteAll', + 'mutedInstanceIds', 'nextRun', 'revision', 'running', + 'snoozeSchedule', + 'updatedBy', + 'updatedAt', ]; export class RulesClient { diff --git a/x-pack/plugins/alerting/server/rules_client/tests/update_api_key.test.ts b/x-pack/plugins/alerting/server/rules_client/tests/update_api_key.test.ts index 1738a424205fc..3051a39f238c1 100644 --- a/x-pack/plugins/alerting/server/rules_client/tests/update_api_key.test.ts +++ b/x-pack/plugins/alerting/server/rules_client/tests/update_api_key.test.ts @@ -118,7 +118,7 @@ describe('updateApiKey()', () => { enabled: true, apiKey: Buffer.from('234:abc').toString('base64'), apiKeyOwner: 'elastic', - revision: 1, + revision: 0, updatedBy: 'elastic', updatedAt: '2019-02-12T21:01:22.479Z', actions: [ @@ -179,7 +179,7 @@ describe('updateApiKey()', () => { enabled: true, apiKey: Buffer.from('234:abc').toString('base64'), apiKeyOwner: 'elastic', - revision: 1, + revision: 0, updatedAt: '2019-02-12T21:01:22.479Z', updatedBy: 'elastic', actions: [ diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.ts index 9973f279a251a..4a99085123b41 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.ts @@ -34,9 +34,6 @@ export const duplicateRule = async ({ rule }: DuplicateRuleParams): Promise Date: Tue, 21 Feb 2023 17:08:45 -0700 Subject: [PATCH 18/28] Comments from review and expanding tests --- .../lib/increment_revision.test.ts | 100 ++++++++++++++++++ .../rules_client/lib/increment_revision.ts | 6 +- .../server/rules_client/methods/bulk_edit.ts | 19 +++- .../server/rules_client/rules_client.ts | 6 +- .../rules_client/tests/bulk_edit.test.ts | 35 +++++- 5 files changed, 154 insertions(+), 12 deletions(-) create mode 100644 x-pack/plugins/alerting/server/rules_client/lib/increment_revision.test.ts diff --git a/x-pack/plugins/alerting/server/rules_client/lib/increment_revision.test.ts b/x-pack/plugins/alerting/server/rules_client/lib/increment_revision.test.ts new file mode 100644 index 0000000000000..aed29b5996003 --- /dev/null +++ b/x-pack/plugins/alerting/server/rules_client/lib/increment_revision.test.ts @@ -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 = { + 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 = { + 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); + }); +}); diff --git a/x-pack/plugins/alerting/server/rules_client/lib/increment_revision.ts b/x-pack/plugins/alerting/server/rules_client/lib/increment_revision.ts index 37db70bdbf55e..ebff8f6a3f75a 100644 --- a/x-pack/plugins/alerting/server/rules_client/lib/increment_revision.ts +++ b/x-pack/plugins/alerting/server/rules_client/lib/increment_revision.ts @@ -15,9 +15,9 @@ export function incrementRevision( { data }: UpdateOptions, updatedParams: RuleTypeParams ): number { - // Diff root level fields + // Diff root level attrs for (const [field, value] of Object.entries(data).filter(([key]) => key !== 'params')) { - if (!fieldsToExcludeFromRevisionUpdates.map(toString).includes(field)) { + if (!fieldsToExcludeFromRevisionUpdates.has(field)) { if (!isEqual(value, get(currentRule.attributes, field))) { return currentRule.attributes.revision + 1; } @@ -27,7 +27,7 @@ export function incrementRevision( // Diff rule params for (const [field, value] of Object.entries(updatedParams)) { // TODO: Should RuleTypes have a way to declare fields that should be revision skipped as well? - if (!fieldsToExcludeFromRevisionUpdates.map(toString).includes(field)) { + if (!fieldsToExcludeFromRevisionUpdates.has(field)) { if (!isEqual(value, get(currentRule.attributes.params, field))) { return currentRule.attributes.revision + 1; } diff --git a/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts b/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts index 64cffb4bb0ee8..291bf11de1a56 100644 --- a/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts +++ b/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts @@ -68,6 +68,11 @@ export type BulkEditFields = keyof Pick< 'actions' | 'tags' | 'schedule' | 'throttle' | 'notifyWhen' | 'snoozeSchedule' | 'apiKey' >; +export const bulkEditFieldsToExcludeFromRevisionUpdates: Set = new Set([ + 'snoozeSchedule', + 'apiKey', +]); + export type BulkEditOperation = | { operation: 'add' | 'delete' | 'set'; @@ -426,6 +431,11 @@ async function updateRuleAttributesAndParamsInMemory = [ 'activeSnoozes', ]; -export const fieldsToExcludeFromRevisionUpdates: Array = [ +export const fieldsToExcludeFromRevisionUpdates: Set = new Set([ 'activeSnoozes', 'alertTypeId', 'apiKey', @@ -88,7 +88,7 @@ export const fieldsToExcludeFromRevisionUpdates: Array = [ 'snoozeSchedule', 'updatedBy', 'updatedAt', -]; +]); export class RulesClient { private readonly context: RulesClientContext; diff --git a/x-pack/plugins/alerting/server/rules_client/tests/bulk_edit.test.ts b/x-pack/plugins/alerting/server/rules_client/tests/bulk_edit.test.ts index 9f302d178b394..78676855560ec 100644 --- a/x-pack/plugins/alerting/server/rules_client/tests/bulk_edit.test.ts +++ b/x-pack/plugins/alerting/server/rules_client/tests/bulk_edit.test.ts @@ -88,6 +88,7 @@ describe('bulkEdit()', () => { notifyWhen: null, actions: [], name: 'my rule name', + revision: 0, }, references: [], version: '123', @@ -192,6 +193,7 @@ describe('bulkEdit()', () => { throttle: null, notifyWhen: null, actions: [], + revision: 0, }, references: [], version: '123', @@ -223,6 +225,7 @@ describe('bulkEdit()', () => { type: 'alert', attributes: expect.objectContaining({ tags: ['foo', 'test-1'], + revision: 1, }), }), ], @@ -247,6 +250,7 @@ describe('bulkEdit()', () => { throttle: null, notifyWhen: null, actions: [], + revision: 0, }, references: [], version: '123', @@ -274,6 +278,7 @@ describe('bulkEdit()', () => { type: 'alert', attributes: expect.objectContaining({ tags: [], + revision: 1, }), }), ], @@ -298,6 +303,7 @@ describe('bulkEdit()', () => { throttle: null, notifyWhen: null, actions: [], + revision: 0, }, references: [], version: '123', @@ -326,6 +332,7 @@ describe('bulkEdit()', () => { type: 'alert', attributes: expect.objectContaining({ tags: ['test-1', 'test-2'], + revision: 1, }), }), ], @@ -460,6 +467,7 @@ describe('bulkEdit()', () => { throttle: null, notifyWhen: null, actions: [], + revision: 0, }, references: [], version: '123', @@ -497,6 +505,7 @@ describe('bulkEdit()', () => { params: expect.objectContaining({ index: ['test-1', 'test-2', 'test-4', 'test-5'], }), + revision: 1, }), }), ], @@ -523,6 +532,7 @@ describe('bulkEdit()', () => { throttle: null, notifyWhen: null, actions: [], + revision: 0, }, references: [], version: '123', @@ -555,6 +565,7 @@ describe('bulkEdit()', () => { params: expect.objectContaining({ index: ['test-1'], }), + revision: 1, }), }), ], @@ -651,6 +662,7 @@ describe('bulkEdit()', () => { type: 'alert', attributes: expect.objectContaining({ snoozeSchedule: [snoozePayload], + revision: 0, }), }), ], @@ -680,6 +692,7 @@ describe('bulkEdit()', () => { id: '1', type: 'alert', attributes: expect.objectContaining({ + revision: 0, snoozeSchedule: [snoozePayload], }), }), @@ -725,6 +738,7 @@ describe('bulkEdit()', () => { id: '1', type: 'alert', attributes: expect.objectContaining({ + revision: 0, snoozeSchedule: [...existingSnooze, snoozePayload], }), }), @@ -770,6 +784,7 @@ describe('bulkEdit()', () => { type: 'alert', attributes: expect.objectContaining({ muteAll: true, + revision: 0, snoozeSchedule: [snoozePayload], }), }), @@ -813,6 +828,7 @@ describe('bulkEdit()', () => { id: '1', type: 'alert', attributes: expect.objectContaining({ + revision: 0, snoozeSchedule: [existingSnooze[1], existingSnooze[2]], }), }), @@ -857,6 +873,7 @@ describe('bulkEdit()', () => { id: '1', type: 'alert', attributes: expect.objectContaining({ + revision: 0, snoozeSchedule: [], }), }), @@ -901,6 +918,7 @@ describe('bulkEdit()', () => { id: '1', type: 'alert', attributes: expect.objectContaining({ + revision: 0, snoozeSchedule: [existingSnooze[0]], }), }), @@ -1013,7 +1031,7 @@ describe('bulkEdit()', () => { expect(createAPIKeyMock).not.toHaveBeenCalled(); // Explicitly bulk editing the apiKey will set the api key, even if the rule is disabled - await rulesClient.bulkEdit({ + const result = await rulesClient.bulkEdit({ filter: 'alert.attributes.tags: "APM"', operations: [ { @@ -1024,6 +1042,9 @@ describe('bulkEdit()', () => { }); expect(createAPIKeyMock).toHaveBeenCalled(); + + // Just API key updates do not result in an increment to revision + expect(result.rules[0]).toHaveProperty('revision', 0); }); }); @@ -1043,7 +1064,7 @@ describe('bulkEdit()', () => { }); }); - it('should succesfully update tags and index patterns and return updated rule', async () => { + it('should successfully update tags and index patterns and return updated rule', async () => { unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ saved_objects: [ { @@ -1062,6 +1083,7 @@ describe('bulkEdit()', () => { throttle: null, notifyWhen: null, actions: [], + revision: 0, }, references: [], version: '123', @@ -1102,6 +1124,7 @@ describe('bulkEdit()', () => { params: { index: ['index-1', 'index-2', 'index-3'], }, + revision: 1, }), }), ], @@ -1109,7 +1132,7 @@ describe('bulkEdit()', () => { ); }); - it('should succesfully update rule if tags are updated but index patterns are not', async () => { + it('should successfully update rule if tags are updated but index patterns are not', async () => { unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ saved_objects: [ { @@ -1128,6 +1151,7 @@ describe('bulkEdit()', () => { throttle: null, notifyWhen: null, actions: [], + revision: 0, }, references: [], version: '123', @@ -1169,6 +1193,7 @@ describe('bulkEdit()', () => { params: { index: ['index-1', 'index-2'], }, + revision: 1, }), }), ], @@ -1176,7 +1201,7 @@ describe('bulkEdit()', () => { ); }); - it('should succesfully update rule if index patterns are updated but tags are not', async () => { + it('should successfully update rule if index patterns are updated but tags are not', async () => { unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ saved_objects: [ { @@ -1195,6 +1220,7 @@ describe('bulkEdit()', () => { throttle: null, notifyWhen: null, actions: [], + revision: 0, }, references: [], version: '123', @@ -1236,6 +1262,7 @@ describe('bulkEdit()', () => { params: { index: ['index-1', 'index-2', 'index-3'], }, + revision: 1, }), }), ], From 2b3ea72c294cd7ae073c6afb15b7381ab0ab5f71 Mon Sep 17 00:00:00 2001 From: Garrett Spong Date: Wed, 22 Feb 2023 09:32:47 -0700 Subject: [PATCH 19/28] Updates SO hash since last update from merge from main --- .../migrations/group2/check_registered_types.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts b/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts index 7d9e4bae782e2..408adc43ae25c 100644 --- a/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts +++ b/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts @@ -57,7 +57,7 @@ describe('checking migration metadata changes on all registered SO types', () => Object { "action": "6cfc277ed3211639e37546ac625f4a68f2494215", "action_task_params": "db2afea7d78e00e725486b791554d0d4e81956ef", - "alert": "2568bf6d8ba0876441c61c9e58e08016c1dc1617", + "alert": "b5e89cf14a72ec1d8ff81c58e71188f6c5bd812a", "api_key_pending_invalidation": "16e7bcf8e78764102d7f525542d5b616809a21ee", "apm-indices": "d19dd7fb51f2d2cbc1f8769481721e0953f9a6d2", "apm-server-schema": "1d42f17eff9ec6c16d3a9324d9539e2d123d0a9a", From 1784b142573af8e0214533b4c282debf0186f5f8 Mon Sep 17 00:00:00 2001 From: Garrett Spong Date: Wed, 22 Feb 2023 10:40:13 -0700 Subject: [PATCH 20/28] Updating new uuid tests to include revision --- .../plugins/alerting/server/rules_client/tests/bulk_edit.test.ts | 1 + x-pack/plugins/alerting/server/rules_client/tests/update.test.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/x-pack/plugins/alerting/server/rules_client/tests/bulk_edit.test.ts b/x-pack/plugins/alerting/server/rules_client/tests/bulk_edit.test.ts index 1cf75d4f16484..fc82638396537 100644 --- a/x-pack/plugins/alerting/server/rules_client/tests/bulk_edit.test.ts +++ b/x-pack/plugins/alerting/server/rules_client/tests/bulk_edit.test.ts @@ -586,6 +586,7 @@ describe('bulkEdit()', () => { updatedAt: '2019-02-12T21:01:22.479Z', updatedBy: 'elastic', tags: ['foo'], + revision: 1, }, references: [{ id: '1', name: 'action_0', type: 'action' }], }, diff --git a/x-pack/plugins/alerting/server/rules_client/tests/update.test.ts b/x-pack/plugins/alerting/server/rules_client/tests/update.test.ts index bac0481e62385..074bbfa3552b1 100644 --- a/x-pack/plugins/alerting/server/rules_client/tests/update.test.ts +++ b/x-pack/plugins/alerting/server/rules_client/tests/update.test.ts @@ -2709,6 +2709,7 @@ describe('update()', () => { name: 'abc', notifyWhen: null, params: { bar: true, risk_score: 40, severity: 'low' }, + revision: 1, schedule: { interval: '1m' }, scheduledTaskId: 'task-123', tags: ['foo'], From f3b5c4e580b6c5935ec3a42eaf513d0e2bcd38bd Mon Sep 17 00:00:00 2001 From: Garrett Spong Date: Mon, 27 Feb 2023 14:47:06 -0700 Subject: [PATCH 21/28] Adds shouldIncrementRevision bypass, fixes imports, and clarifies mapping for revision --- .../alerting/server/routes/clone_rule.test.ts | 2 +- .../server/rules_client/methods/bulk_edit.ts | 20 +++++++++-- .../server/rules_client/methods/clone.ts | 2 +- .../server/rules_client/methods/update.ts | 35 +++++++++++++------ .../alerting/server/saved_objects/mappings.ts | 1 + .../model/import/rule_to_import.ts | 3 +- .../api/rules/bulk_actions/route.ts | 1 + .../rule_management/logic/crud/patch_rules.ts | 4 +++ .../logic/import/import_rules_utils.ts | 1 + .../group10/import_rules.ts | 2 +- 10 files changed, 55 insertions(+), 16 deletions(-) diff --git a/x-pack/plugins/alerting/server/routes/clone_rule.test.ts b/x-pack/plugins/alerting/server/routes/clone_rule.test.ts index cff5d85ca09b2..035e5fa5885b8 100644 --- a/x-pack/plugins/alerting/server/routes/clone_rule.test.ts +++ b/x-pack/plugins/alerting/server/routes/clone_rule.test.ts @@ -91,7 +91,7 @@ describe('cloneRuleRoute', () => { created_at: mockedRule.createdAt, updated_at: mockedRule.updatedAt, id: mockedRule.id, - revision: 0, // TODO: Finalize clone rule behavior. Clone or reset revision? + revision: 0, execution_status: { status: mockedRule.executionStatus.status, last_execution_date: mockedRule.executionStatus.lastExecutionDate, diff --git a/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts b/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts index decd419dbf127..1a0ea0caedda8 100644 --- a/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts +++ b/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts @@ -131,16 +131,22 @@ export type RuleParamsModifier = ( params: Params ) => Promise>; +export type ShouldIncrementRevision = ( + params?: RuleTypeParams +) => boolean; + export interface BulkEditOptionsFilter { filter?: string | KueryNode; operations: BulkEditOperation[]; paramsModifier?: RuleParamsModifier; + shouldIncrementRevision?: ShouldIncrementRevision; } export interface BulkEditOptionsIds { ids: string[]; operations: BulkEditOperation[]; paramsModifier?: RuleParamsModifier; + shouldIncrementRevision?: ShouldIncrementRevision; } export type BulkEditOptions = @@ -249,12 +255,13 @@ export async function bulkEdit( context.logger, `rulesClient.update('operations=${JSON.stringify(options.operations)}, paramsModifier=${ options.paramsModifier ? '[Function]' : undefined - }')`, + }', shouldIncrementRevision=${options.shouldIncrementRevision ? '[Function]' : undefined}')`, (filterKueryNode: KueryNode | null) => bulkEditOcc(context, { filter: filterKueryNode, operations: options.operations, paramsModifier: options.paramsModifier, + shouldIncrementRevision: options.shouldIncrementRevision, }), qNodeFilterWithAuth ); @@ -289,10 +296,12 @@ async function bulkEditOcc( filter, operations, paramsModifier, + shouldIncrementRevision, }: { filter: KueryNode | null; operations: BulkEditOptions['operations']; paramsModifier: BulkEditOptions['paramsModifier']; + shouldIncrementRevision?: BulkEditOptions['shouldIncrementRevision']; } ): Promise<{ apiKeysToInvalidate: string[]; @@ -331,6 +340,7 @@ async function bulkEditOcc( skipped, errors, username, + shouldIncrementRevision, }), { concurrency: API_KEY_GENERATE_CONCURRENCY } ); @@ -400,6 +410,7 @@ async function updateRuleAttributesAndParamsInMemory true, }: { context: RulesClientContext; rule: SavedObjectsFindResult; @@ -410,6 +421,7 @@ async function updateRuleAttributesAndParamsInMemory['shouldIncrementRevision']; }): Promise { try { if (rule.attributes.apiKey) { @@ -433,7 +445,11 @@ async function updateRuleAttributesAndParamsInMemory( mutedInstanceIds: [], executionStatus: getRuleExecutionStatusPending(lastRunTimestamp.toISOString()), monitoring: getDefaultMonitoring(lastRunTimestamp.toISOString()), - revision: 0, // TODO: Clarify if we're resetting revision since it's a new rule, or carrying over from previous rule (existing security solution behavior) + revision: 0, scheduledTaskId: null, running: false, }; diff --git a/x-pack/plugins/alerting/server/rules_client/methods/update.ts b/x-pack/plugins/alerting/server/rules_client/methods/update.ts index 9cbd0f3231e16..0e1ecd40da43c 100644 --- a/x-pack/plugins/alerting/server/rules_client/methods/update.ts +++ b/x-pack/plugins/alerting/server/rules_client/methods/update.ts @@ -9,6 +9,7 @@ import Boom from '@hapi/boom'; import { isEqual, omit } from 'lodash'; import { SavedObject } from '@kbn/core/server'; import { AlertConsumers } from '@kbn/rule-data-utils'; +import type { ShouldIncrementRevision } from './bulk_edit'; import { PartialRule, RawRule, @@ -46,22 +47,29 @@ export interface UpdateOptions { notifyWhen?: RuleNotifyWhenType | null; }; allowMissingConnectorSecrets?: boolean; + shouldIncrementRevision?: ShouldIncrementRevision; } export async function update( context: RulesClientContext, - { id, data, allowMissingConnectorSecrets }: UpdateOptions + { id, data, allowMissingConnectorSecrets, shouldIncrementRevision }: UpdateOptions ): Promise> { return await retryIfConflicts( context.logger, `rulesClient.update('${id}')`, - async () => await updateWithOCC(context, { id, data, allowMissingConnectorSecrets }) + async () => + await updateWithOCC(context, { + id, + data, + allowMissingConnectorSecrets, + shouldIncrementRevision, + }) ); } async function updateWithOCC( context: RulesClientContext, - { id, data, allowMissingConnectorSecrets }: UpdateOptions + { id, data, allowMissingConnectorSecrets, shouldIncrementRevision }: UpdateOptions ): Promise> { let alertSavedObject: SavedObject; @@ -109,7 +117,7 @@ async function updateWithOCC( const updateResult = await updateAlert( context, - { id, data, allowMissingConnectorSecrets }, + { id, data, allowMissingConnectorSecrets, shouldIncrementRevision }, alertSavedObject ); @@ -150,7 +158,12 @@ async function updateWithOCC( async function updateAlert( context: RulesClientContext, - { id, data: initialData, allowMissingConnectorSecrets }: UpdateOptions, + { + id, + data: initialData, + allowMissingConnectorSecrets, + shouldIncrementRevision = () => true, + }: UpdateOptions, currentRule: SavedObject ): Promise> { const { attributes, version } = currentRule; @@ -206,11 +219,13 @@ async function updateAlert( const notifyWhen = getRuleNotifyWhenType(data.notifyWhen ?? null, data.throttle ?? null); // Increment revision if applicable field has changed - const revision = incrementRevision( - currentRule, - { id, data, allowMissingConnectorSecrets }, - updatedParams - ); + const revision = shouldIncrementRevision(updatedParams) + ? incrementRevision( + currentRule, + { id, data, allowMissingConnectorSecrets }, + updatedParams + ) + : currentRule.attributes.revision; let updatedObject: SavedObject; const createAttributes = updateMeta(context, { diff --git a/x-pack/plugins/alerting/server/saved_objects/mappings.ts b/x-pack/plugins/alerting/server/saved_objects/mappings.ts index e27bfc7939996..9e3bc06e7ca39 100644 --- a/x-pack/plugins/alerting/server/saved_objects/mappings.ts +++ b/x-pack/plugins/alerting/server/saved_objects/mappings.ts @@ -199,6 +199,7 @@ export const alertMappings: SavedObjectsTypeMappingDefinition = { }, }, revision: { + index: true, // Explicitly setting to `true` as there is need to query for a rule by a specific revision type: 'long', }, snoozeSchedule: { diff --git a/x-pack/plugins/security_solution/common/detection_engine/rule_management/model/import/rule_to_import.ts b/x-pack/plugins/security_solution/common/detection_engine/rule_management/model/import/rule_to_import.ts index b40bed8ce65c5..8d9cee09a9f7b 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/rule_management/model/import/rule_to_import.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/rule_management/model/import/rule_to_import.ts @@ -17,7 +17,7 @@ import { BaseCreateProps, TypeSpecificCreateProps, } from '../../../rule_schema'; -import { created_at, updated_at, created_by, updated_by } from '../../../schemas/common'; +import { created_at, updated_at, created_by, updated_by, revision } from '../../../schemas/common'; /** * Differences from this and the createRulesSchema are @@ -44,6 +44,7 @@ export const RuleToImport = t.intersection([ created_by, related_integrations: RelatedIntegrationArray, required_fields: RequiredFieldArray, + revision, setup: SetupGuide, }) ), diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_actions/route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_actions/route.ts index e3fabc5366980..8779c19e71988 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_actions/route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_actions/route.ts @@ -544,6 +544,7 @@ export const performBulkActionRoute = ( exceptionsList: exceptions, }, }, + shouldIncrementRevision: () => false, }); // TODO: figureout why types can't return just updatedRule diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/crud/patch_rules.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/crud/patch_rules.ts index 45d1a21325abd..3ce61fcc9800b 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/crud/patch_rules.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/crud/patch_rules.ts @@ -17,6 +17,8 @@ export interface PatchRulesOptions { nextParams: PatchRuleRequestBody; existingRule: RuleAlertType | null | undefined; allowMissingConnectorSecrets?: boolean; + + shouldIncrementRevision?: boolean; } export const patchRules = async ({ @@ -24,6 +26,7 @@ export const patchRules = async ({ existingRule, nextParams, allowMissingConnectorSecrets, + shouldIncrementRevision = true, }: PatchRulesOptions): Promise | null> => { if (existingRule == null) { return null; @@ -35,6 +38,7 @@ export const patchRules = async ({ id: existingRule.id, data: patchedRule, allowMissingConnectorSecrets, + shouldIncrementRevision: () => shouldIncrementRevision, }); if (nextParams.throttle !== undefined) { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rules_utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rules_utils.ts index 5a8d6f9a34e87..89ec68ff79bb1 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rules_utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rules_utils.ts @@ -141,6 +141,7 @@ export const importRules = async ({ exceptions_list: [...exceptions], }, allowMissingConnectorSecrets, + shouldIncrementRevision: false, }); resolve({ rule_id: parsedRule.rule_id, diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_rules.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_rules.ts index e7b2a0fdbf604..21b18b7a70763 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_rules.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_rules.ts @@ -637,7 +637,7 @@ export default ({ getService }: FtrProviderContext): void => { }; ruleOutput.name = 'some other name'; ruleOutput.version = 2; - ruleOutput.revision = 1; // TODO: Should we match `version` logic or reset to `0` as it's technically a new rule? + ruleOutput.revision = 0; expect(bodyToCompare).to.eql(ruleOutput); }); From 53a5941ad9ea950e9bc2cdee543d4fa73f4d229f Mon Sep 17 00:00:00 2001 From: Garrett Spong Date: Mon, 27 Feb 2023 16:56:54 -0700 Subject: [PATCH 22/28] Updating alert SO hash and fixing basic import test --- .../migrations/group2/check_registered_types.test.ts | 2 +- .../basic/tests/import_rules.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts b/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts index b068925f5edbb..0cf1599888969 100644 --- a/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts +++ b/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts @@ -57,7 +57,7 @@ describe('checking migration metadata changes on all registered SO types', () => Object { "action": "6cfc277ed3211639e37546ac625f4a68f2494215", "action_task_params": "db2afea7d78e00e725486b791554d0d4e81956ef", - "alert": "b5e89cf14a72ec1d8ff81c58e71188f6c5bd812a", + "alert": "1e4cd6941f1eb39c729c646e91fbfb9700de84b9", "api_key_pending_invalidation": "16e7bcf8e78764102d7f525542d5b616809a21ee", "apm-indices": "d19dd7fb51f2d2cbc1f8769481721e0953f9a6d2", "apm-server-schema": "1d42f17eff9ec6c16d3a9324d9539e2d123d0a9a", diff --git a/x-pack/test/detection_engine_api_integration/basic/tests/import_rules.ts b/x-pack/test/detection_engine_api_integration/basic/tests/import_rules.ts index e269c8273339f..5b8508ffe480d 100644 --- a/x-pack/test/detection_engine_api_integration/basic/tests/import_rules.ts +++ b/x-pack/test/detection_engine_api_integration/basic/tests/import_rules.ts @@ -361,7 +361,7 @@ export default ({ getService }: FtrProviderContext): void => { }; ruleOutput.name = 'some other name'; ruleOutput.version = 2; - ruleOutput.revision = 1; + ruleOutput.revision = 0; expect(bodyToCompare).to.eql(ruleOutput); }); From e6f5a7bf38e3534e4ef53f362442bdee543f2b3d Mon Sep 17 00:00:00 2001 From: Garrett Spong Date: Mon, 27 Feb 2023 20:58:00 -0700 Subject: [PATCH 23/28] Removing remaining todo's --- .../alerting/server/rules_client/lib/increment_revision.ts | 1 - .../security_and_spaces/group3/tests/alerting/clone.ts | 2 +- .../spaces_only/tests/alerting/group4/bulk_edit.ts | 3 --- 3 files changed, 1 insertion(+), 5 deletions(-) diff --git a/x-pack/plugins/alerting/server/rules_client/lib/increment_revision.ts b/x-pack/plugins/alerting/server/rules_client/lib/increment_revision.ts index ebff8f6a3f75a..fb46b57a6736d 100644 --- a/x-pack/plugins/alerting/server/rules_client/lib/increment_revision.ts +++ b/x-pack/plugins/alerting/server/rules_client/lib/increment_revision.ts @@ -26,7 +26,6 @@ export function incrementRevision( // Diff rule params for (const [field, value] of Object.entries(updatedParams)) { - // TODO: Should RuleTypes have a way to declare fields that should be revision skipped as well? if (!fieldsToExcludeFromRevisionUpdates.has(field)) { if (!isEqual(value, get(currentRule.attributes.params, field))) { return currentRule.attributes.revision + 1; diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/group3/tests/alerting/clone.ts b/x-pack/test/alerting_api_integration/security_and_spaces/group3/tests/alerting/clone.ts index 56baa7fa8f947..d9fc773d5d12a 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/group3/tests/alerting/clone.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/group3/tests/alerting/clone.ts @@ -165,7 +165,7 @@ export default function createAlertTests({ getService }: FtrProviderContext) { mute_all: false, muted_alert_ids: [], execution_status: response.body.execution_status, - revision: 0, // TODO: Finalize proper clone behavior (copy previous revision, or reset to initial default?) + revision: 0, last_run: { alerts_count: { active: 0, diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group4/bulk_edit.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group4/bulk_edit.ts index c41238b9f6ab4..cfd8be8d5b181 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group4/bulk_edit.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group4/bulk_edit.ts @@ -338,9 +338,6 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { expect(bulkUnsnoozeResponse.body.rules).to.have.length(1); expect(bulkUnsnoozeResponse.body.rules[0].snooze_schedule).empty(); - // Ensure revision is updated TODO: TBD on if snooze actually should increment revision - expect(bulkSnoozeResponse.body.rules[0].revision).to.eql(2); - // Ensure AAD isn't broken await checkAAD({ supertest, From 24eae014d00f5a7ba5952414c042f897d84b9b32 Mon Sep 17 00:00:00 2001 From: Garrett Spong Date: Tue, 28 Feb 2023 19:49:49 -0700 Subject: [PATCH 24/28] Addressing PR comments --- .../rules_client/lib/increment_revision.ts | 18 ++++++++++-------- .../server/rules_client/methods/bulk_edit.ts | 6 ++---- .../server/rules_client/rules_client.ts | 2 +- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/x-pack/plugins/alerting/server/rules_client/lib/increment_revision.ts b/x-pack/plugins/alerting/server/rules_client/lib/increment_revision.ts index fb46b57a6736d..5bf3ae02cedc8 100644 --- a/x-pack/plugins/alerting/server/rules_client/lib/increment_revision.ts +++ b/x-pack/plugins/alerting/server/rules_client/lib/increment_revision.ts @@ -17,19 +17,21 @@ export function incrementRevision( ): number { // Diff root level attrs for (const [field, value] of Object.entries(data).filter(([key]) => key !== 'params')) { - if (!fieldsToExcludeFromRevisionUpdates.has(field)) { - if (!isEqual(value, get(currentRule.attributes, field))) { - return currentRule.attributes.revision + 1; - } + 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)) { - if (!isEqual(value, get(currentRule.attributes.params, field))) { - return currentRule.attributes.revision + 1; - } + if ( + !fieldsToExcludeFromRevisionUpdates.has(field) && + !isEqual(value, get(currentRule.attributes.params, field)) + ) { + return currentRule.attributes.revision + 1; } } return currentRule.attributes.revision; diff --git a/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts b/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts index 1a0ea0caedda8..74124da20ea65 100644 --- a/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts +++ b/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts @@ -69,10 +69,8 @@ export type BulkEditFields = keyof Pick< 'actions' | 'tags' | 'schedule' | 'throttle' | 'notifyWhen' | 'snoozeSchedule' | 'apiKey' >; -export const bulkEditFieldsToExcludeFromRevisionUpdates: Set = new Set([ - 'snoozeSchedule', - 'apiKey', -]); +export const bulkEditFieldsToExcludeFromRevisionUpdates: ReadonlySet = + new Set(['snoozeSchedule', 'apiKey']); export type BulkEditOperation = | { diff --git a/x-pack/plugins/alerting/server/rules_client/rules_client.ts b/x-pack/plugins/alerting/server/rules_client/rules_client.ts index 1ecb6bea85d1c..bc14cd0673570 100644 --- a/x-pack/plugins/alerting/server/rules_client/rules_client.ts +++ b/x-pack/plugins/alerting/server/rules_client/rules_client.ts @@ -66,7 +66,7 @@ const fieldsToExcludeFromPublicApi: Array = [ 'activeSnoozes', ]; -export const fieldsToExcludeFromRevisionUpdates: Set = new Set([ +export const fieldsToExcludeFromRevisionUpdates: ReadonlySet = new Set([ 'activeSnoozes', 'alertTypeId', 'apiKey', From d89473688f9a2cd9b5bf1de32febafaa0ab93a68 Mon Sep 17 00:00:00 2001 From: Garrett Spong Date: Wed, 1 Mar 2023 17:42:40 -0700 Subject: [PATCH 25/28] Fixes multi increment when adding bulk actions --- .../server/rules_client/methods/bulk_edit.ts | 3 +- .../rules_client/tests/bulk_edit.test.ts | 44 +++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts b/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts index 74124da20ea65..4bbe493d93c6b 100644 --- a/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts +++ b/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts @@ -673,7 +673,8 @@ async function getUpdatedAttributesFromOperations( // Only increment revision if update wasn't skipped and `operation.field` should result in a revision increment if ( !isAttributesUpdateSkipped && - !bulkEditFieldsToExcludeFromRevisionUpdates.has(operation.field) + !bulkEditFieldsToExcludeFromRevisionUpdates.has(operation.field) && + rule.attributes.revision - attributes.revision === 0 ) { attributes.revision += 1; } diff --git a/x-pack/plugins/alerting/server/rules_client/tests/bulk_edit.test.ts b/x-pack/plugins/alerting/server/rules_client/tests/bulk_edit.test.ts index fc82638396537..7ae9d4fcf041b 100644 --- a/x-pack/plugins/alerting/server/rules_client/tests/bulk_edit.test.ts +++ b/x-pack/plugins/alerting/server/rules_client/tests/bulk_edit.test.ts @@ -600,6 +600,50 @@ describe('bulkEdit()', () => { snoozeSchedule: [], }); }); + + test('should only increment revision once for multiple operations', async () => { + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ + saved_objects: [ + { + ...existingRule, + attributes: { + ...existingRule.attributes, + revision: 1, + }, + }, + ], + }); + const result = await rulesClient.bulkEdit({ + filter: '', + operations: [ + { + field: 'actions', + operation: 'add', + value: [ + { + id: '687300e0-b882-11ed-ad70-c74a8cf8f386', + group: 'default', + params: { + message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + }, + ], + }, + { + field: 'throttle', + operation: 'set', + value: null, + }, + { + field: 'notifyWhen', + operation: 'set', + value: 'onActiveAlert', + }, + ], + }); + + expect(result.rules[0]).toHaveProperty('revision', 1); + }); }); describe('index pattern operations', () => { From 41d32b3429c18aa71f2076f1cf20f1fe0d462217 Mon Sep 17 00:00:00 2001 From: Garrett Spong Date: Wed, 8 Mar 2023 11:52:28 -0700 Subject: [PATCH 26/28] Updating SO hash --- .../migrations/group2/check_registered_types.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts b/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts index d3f1bb72142dc..061d9d281dc54 100644 --- a/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts +++ b/src/core/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts @@ -57,7 +57,7 @@ describe('checking migration metadata changes on all registered SO types', () => Object { "action": "6cfc277ed3211639e37546ac625f4a68f2494215", "action_task_params": "db2afea7d78e00e725486b791554d0d4e81956ef", - "alert": "785240e3137f5eb1a0f8986e5b8eff99780fc04f", + "alert": "1e4cd6941f1eb39c729c646e91fbfb9700de84b9", "api_key_pending_invalidation": "16e7bcf8e78764102d7f525542d5b616809a21ee", "apm-indices": "d19dd7fb51f2d2cbc1f8769481721e0953f9a6d2", "apm-server-schema": "1d42f17eff9ec6c16d3a9324d9539e2d123d0a9a", From a575eb8184eb2009169a03bc4a8c695cf03ebd4f Mon Sep 17 00:00:00 2001 From: Garrett Spong Date: Fri, 10 Mar 2023 11:35:19 -0700 Subject: [PATCH 27/28] Skips synthetics MonitorSummaryTab flakey test --- x-pack/plugins/synthetics/e2e/journeys/synthetics/index.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/synthetics/e2e/journeys/synthetics/index.ts b/x-pack/plugins/synthetics/e2e/journeys/synthetics/index.ts index bf4ecd526e911..1e182d06fefdc 100644 --- a/x-pack/plugins/synthetics/e2e/journeys/synthetics/index.ts +++ b/x-pack/plugins/synthetics/e2e/journeys/synthetics/index.ts @@ -20,7 +20,8 @@ export * from './detail_flyout'; export * from './alert_rules/default_status_alert.journey'; export * from './test_now_mode.journey'; export * from './data_retention.journey'; -export * from './monitor_details_page/monitor_summary.journey'; +// Additional flake skip along with https://github.com/elastic/kibana/pull/151936 +// export * from './monitor_details_page/monitor_summary.journey'; export * from './test_run_details.journey'; export * from './step_details.journey'; export * from './project_monitor_read_only.journey'; From d2366cf7d25f6f6b017ad5c3c6a73c33dfa1da7b Mon Sep 17 00:00:00 2001 From: Garrett Spong Date: Fri, 10 Mar 2023 13:05:20 -0700 Subject: [PATCH 28/28] Updates new export tests to exclude revision --- .../security_and_spaces/group1/export_rules.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/group1/export_rules.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/group1/export_rules.ts index 0946852e6a117..3340c700ed95a 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/group1/export_rules.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/group1/export_rules.ts @@ -606,6 +606,7 @@ function expectToMatchRuleSchema(obj: unknown): void { false_positives: expect.arrayContaining([]), from: expect.any(String), max_signals: expect.any(Number), + revision: expect.any(Number), risk_score_mapping: expect.arrayContaining([]), severity_mapping: expect.arrayContaining([]), threat: expect.arrayContaining([]),