From fad1fe2412c2702102babe0c266337db2f12e088 Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Thu, 5 Nov 2020 17:14:10 -0800 Subject: [PATCH 1/8] Added ability to fire actions when an alert instance is resolved --- .../alerts/common/builtin_action_groups.ts | 14 +++ x-pack/plugins/alerts/common/index.ts | 6 +- .../alerts/server/alert_type_registry.test.ts | 35 ++++++++ .../alerts/server/alert_type_registry.ts | 28 ++++++ .../server/task_runner/task_runner.test.ts | 87 ++++++++++++++++++- .../alerts/server/task_runner/task_runner.ts | 38 ++++++++ .../public/application/lib/alert_api.ts | 3 + .../spaces_only/tests/alerting/alerts_base.ts | 87 +++++++++++++++++++ 8 files changed, 290 insertions(+), 8 deletions(-) create mode 100644 x-pack/plugins/alerts/common/builtin_action_groups.ts diff --git a/x-pack/plugins/alerts/common/builtin_action_groups.ts b/x-pack/plugins/alerts/common/builtin_action_groups.ts new file mode 100644 index 0000000000000..8da874c348285 --- /dev/null +++ b/x-pack/plugins/alerts/common/builtin_action_groups.ts @@ -0,0 +1,14 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +import { i18n } from '@kbn/i18n'; +import { ActionGroup } from './alert_type'; + +export const ResolvedActionGroup: ActionGroup = { + id: 'resolved', + name: i18n.translate('xpack.alerts.builtinActionGroups.resolved', { + defaultMessage: 'Resolved', + }), +}; diff --git a/x-pack/plugins/alerts/common/index.ts b/x-pack/plugins/alerts/common/index.ts index ab71f77a049f6..13ac89153c114 100644 --- a/x-pack/plugins/alerts/common/index.ts +++ b/x-pack/plugins/alerts/common/index.ts @@ -10,11 +10,7 @@ export * from './alert_instance'; export * from './alert_task_instance'; export * from './alert_navigation'; export * from './alert_instance_summary'; - -export interface ActionGroup { - id: string; - name: string; -} +export * from './builtin_action_groups'; export interface AlertingFrameworkHealth { isSufficientlySecure: boolean; diff --git a/x-pack/plugins/alerts/server/alert_type_registry.test.ts b/x-pack/plugins/alerts/server/alert_type_registry.test.ts index 9e1545bae5384..fee32ab498399 100644 --- a/x-pack/plugins/alerts/server/alert_type_registry.test.ts +++ b/x-pack/plugins/alerts/server/alert_type_registry.test.ts @@ -95,6 +95,33 @@ describe('register()', () => { ); }); + test('throws if AlertType action groups contains reserved group id', () => { + const alertType = { + id: 'test', + name: 'Test', + actionGroups: [ + { + id: 'default', + name: 'Default', + }, + { + id: 'resolved', + name: 'Resolved', + }, + ], + defaultActionGroupId: 'default', + executor: jest.fn(), + producer: 'alerts', + }; + const registry = new AlertTypeRegistry(alertTypeRegistryParams); + + expect(() => registry.register(alertType)).toThrowError( + new Error( + `Alert type by id ${alertType.id} cannot be registered. Action groups [resolved] is reserved by framework.` + ) + ); + }); + test('registers the executor with the task manager', () => { const alertType = { id: 'test', @@ -201,6 +228,10 @@ describe('get()', () => { "id": "default", "name": "Default", }, + Object { + "id": "resolved", + "name": "Resolved", + }, ], "actionVariables": Object { "context": Array [], @@ -255,6 +286,10 @@ describe('list()', () => { "id": "testActionGroup", "name": "Test Action Group", }, + Object { + "id": "resolved", + "name": "Resolved", + }, ], "actionVariables": Object { "context": Array [], diff --git a/x-pack/plugins/alerts/server/alert_type_registry.ts b/x-pack/plugins/alerts/server/alert_type_registry.ts index 1d2d9981faeaa..a037bdd4fac69 100644 --- a/x-pack/plugins/alerts/server/alert_type_registry.ts +++ b/x-pack/plugins/alerts/server/alert_type_registry.ts @@ -8,6 +8,7 @@ import Boom from '@hapi/boom'; import { i18n } from '@kbn/i18n'; import { schema } from '@kbn/config-schema'; import typeDetect from 'type-detect'; +import { intersection } from 'lodash'; import { RunContext, TaskManagerSetupContract } from '../../task_manager/server'; import { TaskRunnerFactory } from './task_runner'; import { @@ -16,7 +17,9 @@ import { AlertTypeState, AlertInstanceState, AlertInstanceContext, + ActionGroup, } from './types'; +import { ResolvedActionGroup } from '../common'; interface ConstructorOptions { taskManager: TaskManagerSetupContract; @@ -82,6 +85,8 @@ export class AlertTypeRegistry { ); } alertType.actionVariables = normalizedActionVariables(alertType.actionVariables); + validateActionGroups(alertType.id, alertType.actionGroups); + alertType.actionGroups = [...alertType.actionGroups, ...getBuiltinActionGroups()]; this.alertTypes.set(alertIdSchema.validate(alertType.id), { ...alertType } as AlertType); this.taskManager.registerTaskDefinitions({ [`alerting:${alertType.id}`]: { @@ -137,3 +142,26 @@ function normalizedActionVariables(actionVariables: AlertType['actionVariables'] params: actionVariables?.params ?? [], }; } + +function validateActionGroups(alertTypeId: string, actionGroups: ActionGroup[]) { + const reservedActionGroups = intersection( + actionGroups.map((item) => item.id), + getBuiltinActionGroups().map((item) => item.id) + ); + if (reservedActionGroups.length > 0) { + throw Boom.badRequest( + i18n.translate('xpack.alerts.alertTypeRegistry.register.reservedActionGroupUsageError', { + defaultMessage: + 'Alert type by id {alertTypeId} cannot be registered. Action groups [{actionGroups}] is reserved by framework.', + values: { + actionGroups: reservedActionGroups.join(', '), + alertTypeId, + }, + }) + ); + } +} + +function getBuiltinActionGroups(): ActionGroup[] { + return [ResolvedActionGroup]; +} diff --git a/x-pack/plugins/alerts/server/task_runner/task_runner.test.ts b/x-pack/plugins/alerts/server/task_runner/task_runner.test.ts index 86e78dea66a09..6a113ad9f7957 100644 --- a/x-pack/plugins/alerts/server/task_runner/task_runner.test.ts +++ b/x-pack/plugins/alerts/server/task_runner/task_runner.test.ts @@ -25,12 +25,12 @@ import { alertsMock, alertsClientMock } from '../mocks'; import { eventLoggerMock } from '../../../event_log/server/event_logger.mock'; import { IEventLogger } from '../../../event_log/server'; import { SavedObjectsErrorHelpers } from '../../../../../src/core/server'; -import { Alert } from '../../common'; +import { Alert, ResolvedActionGroup } from '../../common'; import { omit } from 'lodash'; const alertType = { id: 'test', name: 'My test alert', - actionGroups: [{ id: 'default', name: 'Default' }], + actionGroups: [{ id: 'default', name: 'Default' }, ResolvedActionGroup], defaultActionGroupId: 'default', executor: jest.fn(), producer: 'alerts', @@ -91,7 +91,7 @@ describe('Task Runner', () => { throttle: null, muteAll: false, enabled: true, - alertTypeId: '123', + alertTypeId: alertType.id, apiKey: '', apiKeyOwner: 'elastic', schedule: { interval: '10s' }, @@ -112,6 +112,14 @@ describe('Task Runner', () => { foo: true, }, }, + { + group: ResolvedActionGroup.id, + id: '2', + actionTypeId: 'action', + params: { + isResolved: true, + }, + }, ], executionStatus: { status: 'unknown', @@ -489,6 +497,79 @@ describe('Task Runner', () => { `); }); + test('fire resolved actions for execution for the alertInstances which is in the resolved state', async () => { + taskRunnerFactoryInitializerParams.actionsPlugin.isActionTypeEnabled.mockReturnValue(true); + taskRunnerFactoryInitializerParams.actionsPlugin.isActionExecutable.mockReturnValue(true); + + alertType.executor.mockImplementation( + ({ services: executorServices }: AlertExecutorOptions) => { + executorServices.alertInstanceFactory('1').scheduleActions('default'); + } + ); + const taskRunner = new TaskRunner( + alertType, + { + ...mockedTaskInstance, + state: { + ...mockedTaskInstance.state, + alertInstances: { + '1': { meta: {}, state: { bar: false } }, + '2': { meta: {}, state: { bar: false } }, + }, + }, + }, + taskRunnerFactoryInitializerParams + ); + alertsClient.get.mockResolvedValue(mockedAlertTypeSavedObject); + encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValue({ + id: '1', + type: 'alert', + attributes: { + apiKey: Buffer.from('123:abc').toString('base64'), + }, + references: [], + }); + const runnerResult = await taskRunner.run(); + expect(runnerResult.state.alertInstances).toMatchInlineSnapshot(` + Object { + "1": Object { + "meta": Object { + "lastScheduledActions": Object { + "date": 1970-01-01T00:00:00.000Z, + "group": "default", + }, + }, + "state": Object { + "bar": false, + }, + }, + } + `); + + const eventLogger = taskRunnerFactoryInitializerParams.eventLogger; + expect(eventLogger.logEvent).toHaveBeenCalledTimes(5); + expect(actionsClient.enqueueExecution).toHaveBeenCalledTimes(2); + expect(actionsClient.enqueueExecution.mock.calls[0]).toMatchInlineSnapshot(` + Array [ + Object { + "apiKey": "MTIzOmFiYw==", + "id": "2", + "params": Object { + "isResolved": true, + }, + "source": Object { + "source": Object { + "id": "1", + "type": "alert", + }, + "type": "SAVED_OBJECT", + }, + "spaceId": undefined, + }, + ] + `); + }); + test('persists alertInstances passed in from state, only if they are scheduled for execution', async () => { alertType.executor.mockImplementation( ({ services: executorServices }: AlertExecutorOptions) => { diff --git a/x-pack/plugins/alerts/server/task_runner/task_runner.ts b/x-pack/plugins/alerts/server/task_runner/task_runner.ts index 2611ba766173b..91547830468d8 100644 --- a/x-pack/plugins/alerts/server/task_runner/task_runner.ts +++ b/x-pack/plugins/alerts/server/task_runner/task_runner.ts @@ -36,6 +36,7 @@ import { IEvent, IEventLogger, SAVED_OBJECT_REL_PRIMARY } from '../../../event_l import { isAlertSavedObjectNotFoundError } from '../lib/is_alert_not_found_error'; import { AlertsClient } from '../alerts_client'; import { partiallyUpdateAlert } from '../saved_objects'; +import { ResolvedActionGroup } from '../../common'; const FALLBACK_RETRY_INTERVAL = '5m'; @@ -225,6 +226,14 @@ export class TaskRunner { alertInstance.hasScheduledActions() ); const currentAlertInstanceIds = Object.keys(instancesWithScheduledActions); + + scheduleActionsForResolvedInstances( + alertInstances, + executionHandler, + originalAlertInstanceIds, + currentAlertInstanceIds + ); + generateNewAndResolvedInstanceEvents({ eventLogger, originalAlertInstanceIds, @@ -439,6 +448,35 @@ function generateNewAndResolvedInstanceEvents(params: GenerateNewAndResolvedInst } } +function scheduleActionsForResolvedInstances( + alertInstancesMap: { + [x: string]: AlertInstance< + { + [x: string]: unknown; + }, + { + [x: string]: unknown; + } + >; + }, + executionHandler: ReturnType, + originalAlertInstanceIds: string[], + currentAlertInstanceIds: string[] +) { + const resolvedIds = without(originalAlertInstanceIds, ...currentAlertInstanceIds); + for (const id of resolvedIds) { + alertInstancesMap[id].updateLastScheduledActions(ResolvedActionGroup.id); + alertInstancesMap[id].unscheduleActions(); + executionHandler({ + actionGroup: ResolvedActionGroup.id, + context: {}, + state: {}, + alertInstanceId: id, + }); + alertInstancesMap[id].scheduleActions(ResolvedActionGroup.id); + } +} + /** * If an error is thrown, wrap it in an AlertTaskRunResult * so that we can treat each field independantly diff --git a/x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api.ts b/x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api.ts index 7c2f50211d4af..5e61ef4fa4c4e 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api.ts @@ -184,6 +184,9 @@ export async function createAlert({ 'createdBy' | 'updatedBy' | 'muteAll' | 'mutedInstanceIds' | 'executionStatus' >; }): Promise { + if (alert.actions.length === 2) { + alert.actions[1].group = 'resolved'; + } return await http.post(`${BASE_ALERT_API_PATH}/alert`, { body: JSON.stringify(alert), }); diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/alerts_base.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/alerts_base.ts index 75628f6c72487..57c2a6ac81c22 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/alerts_base.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/alerts_base.ts @@ -6,6 +6,7 @@ import expect from '@kbn/expect'; import { Response as SupertestResponse } from 'supertest'; +import { ResolvedActionGroup } from '../../../../../plugins/alerts/common'; import { Space } from '../../../common/types'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; import { @@ -135,6 +136,92 @@ instanceStateValue: true await taskManagerUtils.waitForActionTaskParamsToBeCleanedUp(testStart); }); + it('should fire actions when an alert instance is resolved', async () => { + const reference = alertUtils.generateReference(); + + const { body: createdAction } = await supertestWithoutAuth + .post(`${getUrlPrefix(space.id)}/api/actions/action`) + .set('kbn-xsrf', 'foo') + .send({ + name: 'MY action', + actionTypeId: 'test.noop', + config: {}, + secrets: {}, + }) + .expect(200); + + // pattern of when the alert should fire + const pattern = { + instance: [false, true, true], + }; + + const createdAlert = await supertestWithoutAuth + .post(`${getUrlPrefix(space.id)}/api/alerts/alert`) + .set('kbn-xsrf', 'foo') + .send( + getTestAlertData({ + alertTypeId: 'test.patternFiring', + schedule: { interval: '1s' }, + throttle: null, + params: { + pattern, + }, + actions: [ + { + id: createdAction.id, + group: 'default', + params: {}, + }, + { + group: ResolvedActionGroup.id, + id: indexRecordActionId, + params: { + index: ES_TEST_INDEX_NAME, + reference, + message: 'Resolved message', + }, + }, + ], + }) + ); + + expect(createdAlert.status).to.eql(200); + const alertId = createdAlert.body.id; + objectRemover.add(space.id, alertId, 'alert', 'alerts'); + + const instancesSummary = await supertestWithoutAuth.post( + `${getUrlPrefix(space.id)}/api/alerts/alert/${createdAlert.id}/_instance_summary` + ); + + expect(instancesSummary.status).to.eql(200); + expect(instancesSummary.body.instances).to.eql({ + '1': { + status: 'OK', + muted: true, + }, + }); + + const actionTestRecord = ( + await esTestIndexTool.waitForDocs('action:test.index-record', reference) + )[0]; + + expect(actionTestRecord._source).to.eql({ + config: { + unencrypted: `This value shouldn't get encrypted`, + }, + secrets: { + encrypted: 'This value should be encrypted', + }, + params: { + index: ES_TEST_INDEX_NAME, + reference, + message: 'Resolved message', + }, + reference, + source: 'action:test.index-record', + }); + }); + it('should reschedule failing alerts using the Task Manager retry logic with alert schedule interval', async () => { /* Alerts should set the Task Manager schedule interval with initial value. From 378fa7ced7927f54f759a0d6fe7ff4490dba4db1 Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Tue, 10 Nov 2020 12:25:00 -0800 Subject: [PATCH 2/8] Fixed due to comments --- x-pack/plugins/alerts/common/builtin_action_groups.ts | 4 ++++ x-pack/plugins/alerts/server/alert_type_registry.ts | 6 +----- .../triggers_actions_ui/public/application/lib/alert_api.ts | 3 --- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/x-pack/plugins/alerts/common/builtin_action_groups.ts b/x-pack/plugins/alerts/common/builtin_action_groups.ts index 8da874c348285..d31f75357d370 100644 --- a/x-pack/plugins/alerts/common/builtin_action_groups.ts +++ b/x-pack/plugins/alerts/common/builtin_action_groups.ts @@ -12,3 +12,7 @@ export const ResolvedActionGroup: ActionGroup = { defaultMessage: 'Resolved', }), }; + +export function getBuiltinActionGroups(): ActionGroup[] { + return [ResolvedActionGroup]; +} diff --git a/x-pack/plugins/alerts/server/alert_type_registry.ts b/x-pack/plugins/alerts/server/alert_type_registry.ts index a037bdd4fac69..070cfcd66995f 100644 --- a/x-pack/plugins/alerts/server/alert_type_registry.ts +++ b/x-pack/plugins/alerts/server/alert_type_registry.ts @@ -19,7 +19,7 @@ import { AlertInstanceContext, ActionGroup, } from './types'; -import { ResolvedActionGroup } from '../common'; +import { getBuiltinActionGroups } from '../common'; interface ConstructorOptions { taskManager: TaskManagerSetupContract; @@ -161,7 +161,3 @@ function validateActionGroups(alertTypeId: string, actionGroups: ActionGroup[]) ); } } - -function getBuiltinActionGroups(): ActionGroup[] { - return [ResolvedActionGroup]; -} diff --git a/x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api.ts b/x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api.ts index 5e61ef4fa4c4e..7c2f50211d4af 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api.ts @@ -184,9 +184,6 @@ export async function createAlert({ 'createdBy' | 'updatedBy' | 'muteAll' | 'mutedInstanceIds' | 'executionStatus' >; }): Promise { - if (alert.actions.length === 2) { - alert.actions[1].group = 'resolved'; - } return await http.post(`${BASE_ALERT_API_PATH}/alert`, { body: JSON.stringify(alert), }); From 8f48a3921329dffa6e42c7e2b686b2a1fd4d27b8 Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Tue, 10 Nov 2020 12:33:37 -0800 Subject: [PATCH 3/8] Fixed merge issue --- x-pack/plugins/alerts/server/task_runner/task_runner.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/alerts/server/task_runner/task_runner.ts b/x-pack/plugins/alerts/server/task_runner/task_runner.ts index 1d39cef7a74ac..c19e967e25e81 100644 --- a/x-pack/plugins/alerts/server/task_runner/task_runner.ts +++ b/x-pack/plugins/alerts/server/task_runner/task_runner.ts @@ -231,7 +231,7 @@ export class TaskRunner { alertInstances, executionHandler, originalAlertInstanceIds, - currentAlertInstanceIds + instancesWithScheduledActions ); generateNewAndResolvedInstanceEvents({ @@ -464,8 +464,9 @@ function scheduleActionsForResolvedInstances( }, executionHandler: ReturnType, originalAlertInstanceIds: string[], - currentAlertInstanceIds: string[] + currentAlertInstances: Dictionary ) { + const currentAlertInstanceIds = Object.keys(currentAlertInstances); const resolvedIds = without(originalAlertInstanceIds, ...currentAlertInstanceIds); for (const id of resolvedIds) { alertInstancesMap[id].updateLastScheduledActions(ResolvedActionGroup.id); From 676d8f126df00c2f36c8cbdb214a6973bc392fb5 Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Tue, 10 Nov 2020 21:55:08 -0800 Subject: [PATCH 4/8] Fixed tests and added skip for muted resolve --- .../alerts/server/task_runner/task_runner.ts | 30 ++++++++++++------- .../tests/alerting/list_alert_types.ts | 10 +++++-- .../spaces_only/tests/alerting/alerts_base.ts | 14 +-------- .../tests/alerting/list_alert_types.ts | 5 +++- 4 files changed, 32 insertions(+), 27 deletions(-) diff --git a/x-pack/plugins/alerts/server/task_runner/task_runner.ts b/x-pack/plugins/alerts/server/task_runner/task_runner.ts index c19e967e25e81..0bed249a1cb17 100644 --- a/x-pack/plugins/alerts/server/task_runner/task_runner.ts +++ b/x-pack/plugins/alerts/server/task_runner/task_runner.ts @@ -227,12 +227,15 @@ export class TaskRunner { alertInstance.hasScheduledActions() ); - scheduleActionsForResolvedInstances( - alertInstances, - executionHandler, - originalAlertInstanceIds, - instancesWithScheduledActions - ); + if (!alert.muteAll) { + scheduleActionsForResolvedInstances( + alertInstances, + executionHandler, + originalAlertInstanceIds, + instancesWithScheduledActions, + alert.mutedInstanceIds + ); + } generateNewAndResolvedInstanceEvents({ eventLogger, @@ -464,20 +467,25 @@ function scheduleActionsForResolvedInstances( }, executionHandler: ReturnType, originalAlertInstanceIds: string[], - currentAlertInstances: Dictionary + currentAlertInstances: Dictionary, + mutedInstanceIds: string[] ) { const currentAlertInstanceIds = Object.keys(currentAlertInstances); - const resolvedIds = without(originalAlertInstanceIds, ...currentAlertInstanceIds); + const resolvedIds = without( + originalAlertInstanceIds, + ...[...currentAlertInstanceIds, ...mutedInstanceIds] + ); for (const id of resolvedIds) { - alertInstancesMap[id].updateLastScheduledActions(ResolvedActionGroup.id); - alertInstancesMap[id].unscheduleActions(); + const instance = alertInstancesMap[id]; + instance.updateLastScheduledActions(ResolvedActionGroup.id); + instance.unscheduleActions(); executionHandler({ actionGroup: ResolvedActionGroup.id, context: {}, state: {}, alertInstanceId: id, }); - alertInstancesMap[id].scheduleActions(ResolvedActionGroup.id); + instance.scheduleActions(ResolvedActionGroup.id); } } diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/list_alert_types.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/list_alert_types.ts index ad60ed6941caf..b3635b9f40e27 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/list_alert_types.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/list_alert_types.ts @@ -15,7 +15,10 @@ export default function listAlertTypes({ getService }: FtrProviderContext) { const supertestWithoutAuth = getService('supertestWithoutAuth'); const expectedNoOpType = { - actionGroups: [{ id: 'default', name: 'Default' }], + actionGroups: [ + { id: 'default', name: 'Default' }, + { id: 'resolved', name: 'Resolved' }, + ], defaultActionGroupId: 'default', id: 'test.noop', name: 'Test: Noop', @@ -28,7 +31,10 @@ export default function listAlertTypes({ getService }: FtrProviderContext) { }; const expectedRestrictedNoOpType = { - actionGroups: [{ id: 'default', name: 'Default' }], + actionGroups: [ + { id: 'default', name: 'Default' }, + { id: 'resolved', name: 'Resolved' }, + ], defaultActionGroupId: 'default', id: 'test.restricted-noop', name: 'Test: Restricted Noop', diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/alerts_base.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/alerts_base.ts index 57c2a6ac81c22..b01166132b482 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/alerts_base.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/alerts_base.ts @@ -150,7 +150,7 @@ instanceStateValue: true }) .expect(200); - // pattern of when the alert should fire + // pattern of when the alert should fire. const pattern = { instance: [false, true, true], }; @@ -189,18 +189,6 @@ instanceStateValue: true const alertId = createdAlert.body.id; objectRemover.add(space.id, alertId, 'alert', 'alerts'); - const instancesSummary = await supertestWithoutAuth.post( - `${getUrlPrefix(space.id)}/api/alerts/alert/${createdAlert.id}/_instance_summary` - ); - - expect(instancesSummary.status).to.eql(200); - expect(instancesSummary.body.instances).to.eql({ - '1': { - status: 'OK', - muted: true, - }, - }); - const actionTestRecord = ( await esTestIndexTool.waitForDocs('action:test.index-record', reference) )[0]; diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/list_alert_types.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/list_alert_types.ts index 6fb573c7344b3..3fb2cc40437d8 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/list_alert_types.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/list_alert_types.ts @@ -23,7 +23,10 @@ export default function listAlertTypes({ getService }: FtrProviderContext) { (alertType: any) => alertType.id === 'test.noop' ); expect(fixtureAlertType).to.eql({ - actionGroups: [{ id: 'default', name: 'Default' }], + actionGroups: [ + { id: 'default', name: 'Default' }, + { id: 'resolved', name: 'Resolved' }, + ], defaultActionGroupId: 'default', id: 'test.noop', name: 'Test: Noop', From f781dcb59df51206f507d76da046ff1dc82b5e3c Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Wed, 11 Nov 2020 22:10:13 -0800 Subject: [PATCH 5/8] added test for muted alert --- .../spaces_only/tests/alerting/alerts_base.ts | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/alerts_base.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/alerts_base.ts index b01166132b482..232542e351c82 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/alerts_base.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/alerts_base.ts @@ -210,6 +210,67 @@ instanceStateValue: true }); }); + it('should not fire actions when an alert instance is resolved, but alert is muted', async () => { + const reference = alertUtils.generateReference(); + + const { body: createdAction } = await supertestWithoutAuth + .post(`${getUrlPrefix(space.id)}/api/actions/action`) + .set('kbn-xsrf', 'foo') + .send({ + name: 'MY action', + actionTypeId: 'test.noop', + config: {}, + secrets: {}, + }) + .expect(200); + + // pattern of when the alert should fire. + const pattern = { + instance: [false, true, true], + }; + + const createdAlert = await supertestWithoutAuth + .post(`${getUrlPrefix(space.id)}/api/alerts/alert`) + .set('kbn-xsrf', 'foo') + .send( + getTestAlertData({ + alertTypeId: 'test.patternFiring', + schedule: { interval: '1s' }, + throttle: null, + params: { + pattern, + }, + actions: [ + { + id: createdAction.id, + group: 'default', + params: {}, + }, + { + group: ResolvedActionGroup.id, + id: indexRecordActionId, + params: { + index: ES_TEST_INDEX_NAME, + reference, + message: 'Resolved message', + }, + }, + ], + }) + ); + + await alertUtils.muteAll(createdAlert.id); + + expect(createdAlert.status).to.eql(200); + const alertId = createdAlert.body.id; + objectRemover.add(space.id, alertId, 'alert', 'alerts'); + + const actionTestRecord = ( + await esTestIndexTool.waitForDocs('action:test.index-record', reference, 0) + )[0]; + expect(actionTestRecord).to.eql(0); + }); + it('should reschedule failing alerts using the Task Manager retry logic with alert schedule interval', async () => { /* Alerts should set the Task Manager schedule interval with initial value. From 53ac140a7cef94d51ab9f275bdc7e64b2151297b Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Thu, 12 Nov 2020 15:37:16 -0800 Subject: [PATCH 6/8] Fixed due to comments --- .../alerts/server/alert_type_registry.ts | 3 +- .../alerts/server/task_runner/task_runner.ts | 35 ++++++--------- .../plugins/alerts/server/alert_types.ts | 14 ++++++ .../spaces_only/tests/alerting/alerts_base.ts | 44 ++++++++----------- 4 files changed, 47 insertions(+), 49 deletions(-) diff --git a/x-pack/plugins/alerts/server/alert_type_registry.ts b/x-pack/plugins/alerts/server/alert_type_registry.ts index 070cfcd66995f..2898c827755f7 100644 --- a/x-pack/plugins/alerts/server/alert_type_registry.ts +++ b/x-pack/plugins/alerts/server/alert_type_registry.ts @@ -9,6 +9,7 @@ import { i18n } from '@kbn/i18n'; import { schema } from '@kbn/config-schema'; import typeDetect from 'type-detect'; import { intersection } from 'lodash'; +import _ from 'lodash'; import { RunContext, TaskManagerSetupContract } from '../../task_manager/server'; import { TaskRunnerFactory } from './task_runner'; import { @@ -86,7 +87,7 @@ export class AlertTypeRegistry { } alertType.actionVariables = normalizedActionVariables(alertType.actionVariables); validateActionGroups(alertType.id, alertType.actionGroups); - alertType.actionGroups = [...alertType.actionGroups, ...getBuiltinActionGroups()]; + alertType.actionGroups = [...alertType.actionGroups, ..._.cloneDeep(getBuiltinActionGroups())]; this.alertTypes.set(alertIdSchema.validate(alertType.id), { ...alertType } as AlertType); this.taskManager.registerTaskDefinitions({ [`alerting:${alertType.id}`]: { diff --git a/x-pack/plugins/alerts/server/task_runner/task_runner.ts b/x-pack/plugins/alerts/server/task_runner/task_runner.ts index c981c132416b0..0dad952a86590 100644 --- a/x-pack/plugins/alerts/server/task_runner/task_runner.ts +++ b/x-pack/plugins/alerts/server/task_runner/task_runner.ts @@ -212,16 +212,6 @@ export class TaskRunner { alertInstance.hasScheduledActions() ); - if (!alert.muteAll) { - scheduleActionsForResolvedInstances( - alertInstances, - executionHandler, - originalAlertInstanceIds, - instancesWithScheduledActions, - alert.mutedInstanceIds - ); - } - generateNewAndResolvedInstanceEvents({ eventLogger, originalAlertInstances, @@ -232,6 +222,14 @@ export class TaskRunner { }); if (!muteAll) { + scheduleActionsForResolvedInstances( + alertInstances, + executionHandler, + originalAlertInstances, + instancesWithScheduledActions, + alert.mutedInstanceIds + ); + const mutedInstanceIdsSet = new Set(mutedInstanceIds); await Promise.all( @@ -492,25 +490,18 @@ function generateNewAndResolvedInstanceEvents(params: GenerateNewAndResolvedInst } function scheduleActionsForResolvedInstances( - alertInstancesMap: { - [x: string]: AlertInstance< - { - [x: string]: unknown; - }, - { - [x: string]: unknown; - } - >; - }, + alertInstancesMap: Record, executionHandler: ReturnType, - originalAlertInstanceIds: string[], + originalAlertInstances: Record, currentAlertInstances: Dictionary, mutedInstanceIds: string[] ) { const currentAlertInstanceIds = Object.keys(currentAlertInstances); + const originalAlertInstanceIds = Object.keys(originalAlertInstances); const resolvedIds = without( originalAlertInstanceIds, - ...[...currentAlertInstanceIds, ...mutedInstanceIds] + ...currentAlertInstanceIds, + ...mutedInstanceIds ); for (const id of resolvedIds) { const instance = alertInstancesMap[id]; diff --git a/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/alert_types.ts b/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/alert_types.ts index d43c3363f86b1..7ed864afac4cc 100644 --- a/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/alert_types.ts +++ b/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/alert_types.ts @@ -7,6 +7,7 @@ import { CoreSetup } from 'src/core/server'; import { schema, TypeOf } from '@kbn/config-schema'; import { times } from 'lodash'; +import { ES_TEST_INDEX_NAME } from '../../../../lib'; import { FixtureStartDeps, FixtureSetupDeps } from './plugin'; import { AlertType, @@ -330,6 +331,7 @@ function getValidationAlertType() { function getPatternFiringAlertType() { const paramsSchema = schema.object({ pattern: schema.recordOf(schema.string(), schema.arrayOf(schema.boolean())), + reference: schema.maybe(schema.string()), }); type ParamsType = TypeOf; interface State { @@ -353,6 +355,18 @@ function getPatternFiringAlertType() { maxPatternLength = Math.max(maxPatternLength, instancePattern.length); } + if (params.reference) { + await services.scopedClusterClient.index({ + index: ES_TEST_INDEX_NAME, + refresh: 'wait_for', + body: { + reference: params.reference, + source: 'alert:test.patternFiring', + ...alertExecutorOptions, + }, + }); + } + // get the pattern index, return if past it const patternIndex = state.patternIndex ?? 0; if (patternIndex >= maxPatternLength) { diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/alerts_base.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/alerts_base.ts index 232542e351c82..26f52475a2d4e 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/alerts_base.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/alerts_base.ts @@ -152,7 +152,7 @@ instanceStateValue: true // pattern of when the alert should fire. const pattern = { - instance: [false, true, true], + instance: [true, true], }; const createdAlert = await supertestWithoutAuth @@ -193,24 +193,11 @@ instanceStateValue: true await esTestIndexTool.waitForDocs('action:test.index-record', reference) )[0]; - expect(actionTestRecord._source).to.eql({ - config: { - unencrypted: `This value shouldn't get encrypted`, - }, - secrets: { - encrypted: 'This value should be encrypted', - }, - params: { - index: ES_TEST_INDEX_NAME, - reference, - message: 'Resolved message', - }, - reference, - source: 'action:test.index-record', - }); + expect(actionTestRecord._source.params.message).to.eql('Resolved message'); }); it('should not fire actions when an alert instance is resolved, but alert is muted', async () => { + const testStart = new Date(); const reference = alertUtils.generateReference(); const { body: createdAction } = await supertestWithoutAuth @@ -226,9 +213,9 @@ instanceStateValue: true // pattern of when the alert should fire. const pattern = { - instance: [false, true, true], + instance: [true, true], }; - + // created disabled alert const createdAlert = await supertestWithoutAuth .post(`${getUrlPrefix(space.id)}/api/alerts/alert`) .set('kbn-xsrf', 'foo') @@ -236,9 +223,11 @@ instanceStateValue: true getTestAlertData({ alertTypeId: 'test.patternFiring', schedule: { interval: '1s' }, + enabled: false, throttle: null, params: { pattern, + reference, }, actions: [ { @@ -258,17 +247,20 @@ instanceStateValue: true ], }) ); - - await alertUtils.muteAll(createdAlert.id); - expect(createdAlert.status).to.eql(200); const alertId = createdAlert.body.id; - objectRemover.add(space.id, alertId, 'alert', 'alerts'); - const actionTestRecord = ( - await esTestIndexTool.waitForDocs('action:test.index-record', reference, 0) - )[0]; - expect(actionTestRecord).to.eql(0); + await alertUtils.muteAll(alertId); + + await alertUtils.enable(alertId); + + await esTestIndexTool.search('alert:test.patternFiring', reference); + + await taskManagerUtils.waitForActionTaskParamsToBeCleanedUp(testStart); + + const actionTestRecord = await esTestIndexTool.search('action:test.index-record', reference); + expect(actionTestRecord.hits.total.value).to.eql(0); + objectRemover.add(space.id, alertId, 'alert', 'alerts'); }); it('should reschedule failing alerts using the Task Manager retry logic with alert schedule interval', async () => { From b4dc1f5b2a75a80cd625703da505df82be3fb3de Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Thu, 12 Nov 2020 15:42:59 -0800 Subject: [PATCH 7/8] Fixed registry error message --- x-pack/plugins/alerts/server/alert_type_registry.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/alerts/server/alert_type_registry.ts b/x-pack/plugins/alerts/server/alert_type_registry.ts index 2898c827755f7..8fe2ab06acd9a 100644 --- a/x-pack/plugins/alerts/server/alert_type_registry.ts +++ b/x-pack/plugins/alerts/server/alert_type_registry.ts @@ -150,10 +150,10 @@ function validateActionGroups(alertTypeId: string, actionGroups: ActionGroup[]) getBuiltinActionGroups().map((item) => item.id) ); if (reservedActionGroups.length > 0) { - throw Boom.badRequest( + throw new Error( i18n.translate('xpack.alerts.alertTypeRegistry.register.reservedActionGroupUsageError', { defaultMessage: - 'Alert type by id {alertTypeId} cannot be registered. Action groups [{actionGroups}] is reserved by framework.', + 'Alert type [id="{alertTypeId}"] cannot be registered. Action groups [{actionGroups}] are reserved by the framework.', values: { actionGroups: reservedActionGroups.join(', '), alertTypeId, From 78a15c904fde7ca60c861e054977d2f6cc8f641e Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Fri, 13 Nov 2020 14:55:09 -0800 Subject: [PATCH 8/8] Fixed jest test --- x-pack/plugins/alerts/server/alert_type_registry.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/alerts/server/alert_type_registry.test.ts b/x-pack/plugins/alerts/server/alert_type_registry.test.ts index fee32ab498399..8dc387f96addb 100644 --- a/x-pack/plugins/alerts/server/alert_type_registry.test.ts +++ b/x-pack/plugins/alerts/server/alert_type_registry.test.ts @@ -117,7 +117,7 @@ describe('register()', () => { expect(() => registry.register(alertType)).toThrowError( new Error( - `Alert type by id ${alertType.id} cannot be registered. Action groups [resolved] is reserved by framework.` + `Alert type [id="${alertType.id}"] cannot be registered. Action groups [resolved] are reserved by the framework.` ) ); });