From b3ed8324f1c305bddd86d052ae683bef1a691ba7 Mon Sep 17 00:00:00 2001 From: Gidi Meir Morris Date: Thu, 2 Jul 2020 21:46:44 +0100 Subject: [PATCH] use alertsclient in task runner --- .../alerts/server/alerts_client.test.ts | 2 +- x-pack/plugins/alerts/server/alerts_client.ts | 5 +- x-pack/plugins/alerts/server/plugin.ts | 20 ++-- .../server/task_runner/task_runner.test.ts | 112 +++++++----------- .../alerts/server/task_runner/task_runner.ts | 83 +++++-------- .../task_runner/task_runner_factory.test.ts | 4 +- .../server/task_runner/task_runner_factory.ts | 4 +- 7 files changed, 90 insertions(+), 140 deletions(-) diff --git a/x-pack/plugins/alerts/server/alerts_client.test.ts b/x-pack/plugins/alerts/server/alerts_client.test.ts index dbf9258d3ba79..5194d3b6b1fb8 100644 --- a/x-pack/plugins/alerts/server/alerts_client.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client.test.ts @@ -1886,7 +1886,7 @@ describe('get()', () => { references: [], }); await expect(alertsClient.get({ id: '1' })).rejects.toThrowErrorMatchingInlineSnapshot( - `"Reference action_0 not found"` + `"Action reference \\"action_0\\" not found in alert id: 1"` ); }); diff --git a/x-pack/plugins/alerts/server/alerts_client.ts b/x-pack/plugins/alerts/server/alerts_client.ts index 0be60673881fb..9fb302193d602 100644 --- a/x-pack/plugins/alerts/server/alerts_client.ts +++ b/x-pack/plugins/alerts/server/alerts_client.ts @@ -719,13 +719,14 @@ export class AlertsClient { } private injectReferencesIntoActions( + alertId: string, actions: RawAlert['actions'], references: SavedObjectReference[] ) { return actions.map((action) => { const reference = references.find((ref) => ref.name === action.actionRef); if (!reference) { - throw new Error(`Reference ${action.actionRef} not found`); + throw new Error(`Action reference "${action.actionRef}" not found in alert id: ${alertId}`); } return { ...omit(action, 'actionRef'), @@ -759,7 +760,7 @@ export class AlertsClient { // Once we support additional types, this type signature will likely change schedule: rawAlert.schedule as IntervalSchedule, actions: rawAlert.actions - ? this.injectReferencesIntoActions(rawAlert.actions, references || []) + ? this.injectReferencesIntoActions(id, rawAlert.actions, references || []) : [], ...(updatedAt ? { updatedAt: new Date(updatedAt) } : {}), ...(createdAt ? { createdAt: new Date(createdAt) } : {}), diff --git a/x-pack/plugins/alerts/server/plugin.ts b/x-pack/plugins/alerts/server/plugin.ts index 182bde560700d..6ca65ac152ee3 100644 --- a/x-pack/plugins/alerts/server/plugin.ts +++ b/x-pack/plugins/alerts/server/plugin.ts @@ -222,9 +222,19 @@ export class AlertingPlugin { features: plugins.features, }); + const getAlertsClientWithRequest = (request: KibanaRequest) => { + if (isESOUsingEphemeralEncryptionKey === true) { + throw new Error( + `Unable to create alerts client due to the Encrypted Saved Objects plugin using an ephemeral encryption key. Please set xpack.encryptedSavedObjects.encryptionKey in kibana.yml` + ); + } + return alertsClientFactory!.create(request, core.savedObjects); + }; + taskRunnerFactory.initialize({ logger, getServices: this.getServicesFactory(core.savedObjects, core.elasticsearch), + getAlertsClientWithRequest, spaceIdToNamespace: this.spaceIdToNamespace, actionsPlugin: plugins.actions, encryptedSavedObjectsClient, @@ -236,15 +246,7 @@ export class AlertingPlugin { return { listTypes: alertTypeRegistry!.list.bind(this.alertTypeRegistry!), - // Ability to get an alerts client from legacy code - getAlertsClientWithRequest: (request: KibanaRequest) => { - if (isESOUsingEphemeralEncryptionKey === true) { - throw new Error( - `Unable to create alerts client due to the Encrypted Saved Objects plugin using an ephemeral encryption key. Please set xpack.encryptedSavedObjects.encryptionKey in kibana.yml` - ); - } - return alertsClientFactory!.create(request, core.savedObjects); - }, + getAlertsClientWithRequest, }; } 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 2373ae264c492..4abe58de5a904 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 @@ -14,7 +14,7 @@ import { encryptedSavedObjectsMock } from '../../../encrypted_saved_objects/serv import { loggingSystemMock } from '../../../../../src/core/server/mocks'; import { PluginStartContract as ActionsPluginStart } from '../../../actions/server'; import { actionsMock, actionsClientMock } from '../../../actions/server/mocks'; -import { alertsMock } from '../mocks'; +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'; @@ -56,8 +56,8 @@ describe('Task Runner', () => { const encryptedSavedObjectsClient = encryptedSavedObjectsMock.createClient(); const services = alertsMock.createAlertServices(); - const savedObjectsClient = services.savedObjectsClient; const actionsClient = actionsClientMock.create(); + const alertsClient = alertsClientMock.create(); const taskRunnerFactoryInitializerParams: jest.Mocked & { actionsPlugin: jest.Mocked; @@ -65,6 +65,7 @@ describe('Task Runner', () => { } = { getServices: jest.fn().mockReturnValue(services), actionsPlugin: actionsMock.createStart(), + getAlertsClientWithRequest: jest.fn().mockReturnValue(alertsClient), encryptedSavedObjectsClient, logger: loggingSystemMock.create().get(), spaceIdToNamespace: jest.fn().mockReturnValue(undefined), @@ -74,34 +75,31 @@ describe('Task Runner', () => { const mockedAlertTypeSavedObject = { id: '1', - type: 'alert', - attributes: { - enabled: true, - alertTypeId: '123', - schedule: { interval: '10s' }, - name: 'alert-name', - tags: ['alert-', '-tags'], - createdBy: 'alert-creator', - updatedBy: 'alert-updater', - mutedInstanceIds: [], - params: { - bar: true, - }, - actions: [ - { - group: 'default', - actionRef: 'action_0', - params: { - foo: true, - }, - }, - ], + consumer: 'bar', + createdAt: new Date('2019-02-12T21:01:22.479Z'), + updatedAt: new Date('2019-02-12T21:01:22.479Z'), + throttle: null, + muteAll: false, + enabled: true, + alertTypeId: '123', + apiKeyOwner: 'elastic', + schedule: { interval: '10s' }, + name: 'alert-name', + tags: ['alert-', '-tags'], + createdBy: 'alert-creator', + updatedBy: 'alert-updater', + mutedInstanceIds: [], + params: { + bar: true, }, - references: [ + actions: [ { - name: 'action_0', - type: 'action', + group: 'default', id: '1', + actionTypeId: 'action', + params: { + foo: true, + }, }, ], }; @@ -109,6 +107,7 @@ describe('Task Runner', () => { beforeEach(() => { jest.resetAllMocks(); taskRunnerFactoryInitializerParams.getServices.mockReturnValue(services); + taskRunnerFactoryInitializerParams.getAlertsClientWithRequest.mockReturnValue(alertsClient); taskRunnerFactoryInitializerParams.actionsPlugin.getActionsClientWithRequest.mockResolvedValue( actionsClient ); @@ -126,7 +125,7 @@ describe('Task Runner', () => { }, taskRunnerFactoryInitializerParams ); - savedObjectsClient.get.mockResolvedValueOnce(mockedAlertTypeSavedObject); + alertsClient.get.mockResolvedValueOnce(mockedAlertTypeSavedObject); encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValueOnce({ id: '1', type: 'alert', @@ -200,7 +199,7 @@ describe('Task Runner', () => { mockedTaskInstance, taskRunnerFactoryInitializerParams ); - savedObjectsClient.get.mockResolvedValueOnce(mockedAlertTypeSavedObject); + alertsClient.get.mockResolvedValueOnce(mockedAlertTypeSavedObject); encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValueOnce({ id: '1', type: 'alert', @@ -285,7 +284,7 @@ describe('Task Runner', () => { ], }, message: - "alert: test:1: 'alert-name' instanceId: '1' scheduled actionGroup: 'default' action: undefined:1", + "alert: test:1: 'alert-name' instanceId: '1' scheduled actionGroup: 'default' action: action:1", }); }); @@ -302,7 +301,7 @@ describe('Task Runner', () => { mockedTaskInstance, taskRunnerFactoryInitializerParams ); - savedObjectsClient.get.mockResolvedValueOnce(mockedAlertTypeSavedObject); + alertsClient.get.mockResolvedValueOnce(mockedAlertTypeSavedObject); encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValueOnce({ id: '1', type: 'alert', @@ -412,7 +411,7 @@ describe('Task Runner', () => { }, ], }, - "message": "alert: test:1: 'alert-name' instanceId: '1' scheduled actionGroup: 'default' action: undefined:1", + "message": "alert: test:1: 'alert-name' instanceId: '1' scheduled actionGroup: 'default' action: action:1", }, ], ] @@ -439,7 +438,7 @@ describe('Task Runner', () => { }, taskRunnerFactoryInitializerParams ); - savedObjectsClient.get.mockResolvedValueOnce(mockedAlertTypeSavedObject); + alertsClient.get.mockResolvedValueOnce(mockedAlertTypeSavedObject); encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValueOnce({ id: '1', type: 'alert', @@ -526,7 +525,7 @@ describe('Task Runner', () => { mockedTaskInstance, taskRunnerFactoryInitializerParams ); - savedObjectsClient.get.mockResolvedValueOnce(mockedAlertTypeSavedObject); + alertsClient.get.mockResolvedValueOnce(mockedAlertTypeSavedObject); encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValueOnce({ id: '1', type: 'alert', @@ -548,44 +547,13 @@ describe('Task Runner', () => { ); }); - test('throws error if reference not found', async () => { - const taskRunner = new TaskRunner( - alertType, - mockedTaskInstance, - taskRunnerFactoryInitializerParams - ); - savedObjectsClient.get.mockResolvedValueOnce({ - ...mockedAlertTypeSavedObject, - references: [], - }); - encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValueOnce({ - id: '1', - type: 'alert', - attributes: { - apiKey: Buffer.from('123:abc').toString('base64'), - }, - references: [], - }); - expect(await taskRunner.run()).toMatchInlineSnapshot(` - Object { - "runAt": 1970-01-01T00:00:10.000Z, - "state": Object { - "previousStartedAt": 1970-01-01T00:00:00.000Z, - }, - } - `); - expect(taskRunnerFactoryInitializerParams.logger.error).toHaveBeenCalledWith( - `Executing Alert \"1\" has resulted in Error: Action reference \"action_0\" not found in alert id: 1` - ); - }); - test('uses API key when provided', async () => { const taskRunner = new TaskRunner( alertType, mockedTaskInstance, taskRunnerFactoryInitializerParams ); - savedObjectsClient.get.mockResolvedValueOnce(mockedAlertTypeSavedObject); + alertsClient.get.mockResolvedValueOnce(mockedAlertTypeSavedObject); encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValueOnce({ id: '1', type: 'alert', @@ -621,7 +589,7 @@ describe('Task Runner', () => { mockedTaskInstance, taskRunnerFactoryInitializerParams ); - savedObjectsClient.get.mockResolvedValueOnce(mockedAlertTypeSavedObject); + alertsClient.get.mockResolvedValueOnce(mockedAlertTypeSavedObject); encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValueOnce({ id: '1', type: 'alert', @@ -660,7 +628,7 @@ describe('Task Runner', () => { taskRunnerFactoryInitializerParams ); - savedObjectsClient.get.mockResolvedValueOnce(mockedAlertTypeSavedObject); + alertsClient.get.mockResolvedValueOnce(mockedAlertTypeSavedObject); encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValueOnce({ id: '1', type: 'alert', @@ -722,7 +690,7 @@ describe('Task Runner', () => { taskRunnerFactoryInitializerParams ); - savedObjectsClient.get.mockResolvedValueOnce(mockedAlertTypeSavedObject); + alertsClient.get.mockResolvedValueOnce(mockedAlertTypeSavedObject); const runnerResult = await taskRunner.run(); @@ -747,7 +715,7 @@ describe('Task Runner', () => { taskRunnerFactoryInitializerParams ); - savedObjectsClient.get.mockResolvedValueOnce(mockedAlertTypeSavedObject); + alertsClient.get.mockResolvedValueOnce(mockedAlertTypeSavedObject); encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValueOnce({ id: '1', type: 'alert', @@ -770,7 +738,7 @@ describe('Task Runner', () => { }); test('recovers gracefully when the Alert Task Runner throws an exception when fetching attributes', async () => { - savedObjectsClient.get.mockImplementation(() => { + alertsClient.get.mockImplementation(() => { throw new Error('OMG'); }); @@ -802,7 +770,7 @@ describe('Task Runner', () => { }); test('avoids rescheduling a failed Alert Task Runner when it throws due to failing to fetch the alert', async () => { - savedObjectsClient.get.mockImplementation(() => { + alertsClient.get.mockImplementation(() => { throw SavedObjectsErrorHelpers.createGenericNotFoundError('task', '1'); }); 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 3512ab16a3712..90a5bc413d273 100644 --- a/x-pack/plugins/alerts/server/task_runner/task_runner.ts +++ b/x-pack/plugins/alerts/server/task_runner/task_runner.ts @@ -5,7 +5,7 @@ */ import { pick, mapValues, omit, without } from 'lodash'; -import { Logger, SavedObject, KibanaRequest } from '../../../../../src/core/server'; +import { Logger, KibanaRequest } from '../../../../../src/core/server'; import { TaskRunnerContext } from './task_runner_factory'; import { ConcreteTaskInstance } from '../../../task_manager/server'; import { createExecutionHandler } from './create_execution_handler'; @@ -17,9 +17,11 @@ import { RawAlert, IntervalSchedule, Services, - AlertInfoParams, RawAlertInstance, AlertTaskState, + Alert, + AlertExecutorOptions, + SanitizedAlert, } from '../types'; import { promiseResult, map, Resultable, asOk, asErr, resolveErr } from '../lib/result_type'; import { taskInstanceToAlertTaskInstance } from './alert_task_instance'; @@ -27,6 +29,7 @@ import { AlertInstances } from '../alert_instance/alert_instance'; import { EVENT_LOG_ACTIONS } from '../plugin'; import { IEvent, IEventLogger, SAVED_OBJECT_REL_PRIMARY } from '../../../event_log/server'; import { isAlertSavedObjectNotFoundError } from '../lib/is_alert_not_found_error'; +import { AlertsClient } from '../alerts_client'; const FALLBACK_RETRY_INTERVAL: IntervalSchedule = { interval: '5m' }; @@ -94,8 +97,12 @@ export class TaskRunner { } as unknown) as KibanaRequest; } - async getServicesWithSpaceLevelPermissions(spaceId: string, apiKey: string | null) { - return this.context.getServices(this.getFakeKibanaRequest(spaceId, apiKey)); + private getServicesWithSpaceLevelPermissions( + spaceId: string, + apiKey: string | null + ): [Services, PublicMethodsOf] { + const request = this.getFakeKibanaRequest(spaceId, apiKey); + return [this.context.getServices(request), this.context.getAlertsClientWithRequest(request)]; } private getExecutionHandler( @@ -104,21 +111,8 @@ export class TaskRunner { tags: string[] | undefined, spaceId: string, apiKey: string | null, - actions: RawAlert['actions'], - references: SavedObject['references'] + actions: Alert['actions'] ) { - // Inject ids into actions - const actionsWithIds = actions.map((action) => { - const actionReference = references.find((obj) => obj.name === action.actionRef); - if (!actionReference) { - throw new Error(`Action reference "${action.actionRef}" not found in alert id: ${alertId}`); - } - return { - ...action, - id: actionReference.id, - }; - }); - return createExecutionHandler({ alertId, alertName, @@ -126,7 +120,7 @@ export class TaskRunner { logger: this.logger, actionsPlugin: this.context.actionsPlugin, apiKey, - actions: actionsWithIds, + actions, spaceId, alertType: this.alertType, eventLogger: this.context.eventLogger, @@ -147,20 +141,12 @@ export class TaskRunner { async executeAlertInstances( services: Services, - alertInfoParams: AlertInfoParams, + alert: SanitizedAlert, + params: AlertExecutorOptions['params'], executionHandler: ReturnType, spaceId: string ): Promise { - const { - params, - throttle, - muteAll, - mutedInstanceIds, - name, - tags, - createdBy, - updatedBy, - } = alertInfoParams; + const { throttle, muteAll, mutedInstanceIds, name, tags, createdBy, updatedBy } = alert; const { params: { alertId }, state: { alertInstances: alertRawInstances = {}, alertTypeState = {}, previousStartedAt }, @@ -267,33 +253,22 @@ export class TaskRunner { }; } - async validateAndExecuteAlert( - services: Services, - apiKey: string | null, - attributes: RawAlert, - references: SavedObject['references'] - ) { + async validateAndExecuteAlert(services: Services, apiKey: string | null, alert: SanitizedAlert) { const { params: { alertId, spaceId }, } = this.taskInstance; // Validate - const params = validateAlertTypeParams(this.alertType, attributes.params); + const validatedParams = validateAlertTypeParams(this.alertType, alert.params); const executionHandler = this.getExecutionHandler( alertId, - attributes.name, - attributes.tags, + alert.name, + alert.tags, spaceId, apiKey, - attributes.actions, - references - ); - return this.executeAlertInstances( - services, - { ...attributes, params }, - executionHandler, - spaceId + alert.actions ); + return this.executeAlertInstances(services, alert, validatedParams, executionHandler, spaceId); } async loadAlertAttributesAndRun(): Promise> { @@ -302,17 +277,17 @@ export class TaskRunner { } = this.taskInstance; const apiKey = await this.getApiKeyForAlertPermissions(alertId, spaceId); - const services = await this.getServicesWithSpaceLevelPermissions(spaceId, apiKey); + const [services, alertsClient] = await this.getServicesWithSpaceLevelPermissions( + spaceId, + apiKey + ); // Ensure API key is still valid and user has access - const { attributes, references } = await services.savedObjectsClient.get( - 'alert', - alertId - ); + const alert = await alertsClient.get({ id: alertId }); return { state: await promiseResult( - this.validateAndExecuteAlert(services, apiKey, attributes, references) + this.validateAndExecuteAlert(services, apiKey, alert) ), runAt: asOk( getNextRunAt( @@ -320,7 +295,7 @@ export class TaskRunner { // we do not currently have a good way of returning the type // from SavedObjectsClient, and as we currenrtly require a schedule // and we only support `interval`, we can cast this safely - attributes.schedule as IntervalSchedule + alert.schedule ) ), }; diff --git a/x-pack/plugins/alerts/server/task_runner/task_runner_factory.test.ts b/x-pack/plugins/alerts/server/task_runner/task_runner_factory.test.ts index ba151c2356191..9af7d9ddc44eb 100644 --- a/x-pack/plugins/alerts/server/task_runner/task_runner_factory.test.ts +++ b/x-pack/plugins/alerts/server/task_runner/task_runner_factory.test.ts @@ -10,7 +10,7 @@ import { TaskRunnerContext, TaskRunnerFactory } from './task_runner_factory'; import { encryptedSavedObjectsMock } from '../../../encrypted_saved_objects/server/mocks'; import { loggingSystemMock } from '../../../../../src/core/server/mocks'; import { actionsMock } from '../../../actions/server/mocks'; -import { alertsMock } from '../mocks'; +import { alertsMock, alertsClientMock } from '../mocks'; import { eventLoggerMock } from '../../../event_log/server/event_logger.mock'; const alertType = { @@ -52,9 +52,11 @@ describe('Task Runner Factory', () => { const encryptedSavedObjectsPlugin = encryptedSavedObjectsMock.createStart(); const services = alertsMock.createAlertServices(); + const alertsClient = alertsClientMock.create(); const taskRunnerFactoryInitializerParams: jest.Mocked = { getServices: jest.fn().mockReturnValue(services), + getAlertsClientWithRequest: jest.fn().mockReturnValue(alertsClient), actionsPlugin: actionsMock.createStart(), encryptedSavedObjectsClient: encryptedSavedObjectsPlugin.getClient(), logger: loggingSystemMock.create().get(), diff --git a/x-pack/plugins/alerts/server/task_runner/task_runner_factory.ts b/x-pack/plugins/alerts/server/task_runner/task_runner_factory.ts index ca762cf2b2105..6f83e34cdbe03 100644 --- a/x-pack/plugins/alerts/server/task_runner/task_runner_factory.ts +++ b/x-pack/plugins/alerts/server/task_runner/task_runner_factory.ts @@ -3,7 +3,7 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ -import { Logger } from '../../../../../src/core/server'; +import { Logger, KibanaRequest } from '../../../../../src/core/server'; import { RunContext } from '../../../task_manager/server'; import { EncryptedSavedObjectsClient } from '../../../encrypted_saved_objects/server'; import { PluginStartContract as ActionsPluginStartContract } from '../../../actions/server'; @@ -15,10 +15,12 @@ import { } from '../types'; import { TaskRunner } from './task_runner'; import { IEventLogger } from '../../../event_log/server'; +import { AlertsClient } from '../alerts_client'; export interface TaskRunnerContext { logger: Logger; getServices: GetServicesFunction; + getAlertsClientWithRequest(request: KibanaRequest): PublicMethodsOf; actionsPlugin: ActionsPluginStartContract; eventLogger: IEventLogger; encryptedSavedObjectsClient: EncryptedSavedObjectsClient;