From b33d556dd871e7284b900739e2eea72b284d43bb Mon Sep 17 00:00:00 2001 From: Patrick Mueller Date: Fri, 4 Sep 2020 17:47:13 -0400 Subject: [PATCH 1/7] [Alerting] formalize alert status and add status fields to alert saved object resolves https://github.com/elastic/kibana/issues/51099 This formalizes the concept of "alert status", in terms of it's execution, with some new fields in the alert saved object and types used with the alert client and http APIs. These fields are read-only from the client point-of-view; they are provided in the alert structures, but are only updated by the alerting framework itself. The values will be updated after each run of the alert type executor. The data is added to the alert as the `executionStatus` field, with the following shape: ```ts interface AlertExecutionStatus { status: 'ok' | 'active' | 'error' | 'unknown'; date: Date; error?: { reason: 'read' | 'decrypt' | 'execute' | 'unknown'; message: string; }; } ``` interim commits: calculate the execution status, some refactoring write the execution status to the alert after execution use real date in execution status on create add an await to an async fn comment out status update to see if SIEM FT succeeds fix SIEM FT alert deletion issue use partial updates and retries in alerts clients to avoid conflicts fix jest tests clean up conflict-fixin code moar conflict-prevention fixing fix type error with find result add reasons to alert execution errors add some jest tests add some function tests fix status update to use alert namespace fix function test finish function tests more fixes after rebase fix type checks and jest tests after rebase add migration and find functional tests fix relative import --- x-pack/plugins/alerts/common/alert.ts | 13 + .../alerts/server/alerts_client.test.ts | 20 ++ x-pack/plugins/alerts/server/alerts_client.ts | 16 +- .../server/lib/alert_execution_status.test.ts | 163 +++++++++ .../server/lib/alert_execution_status.ts | 60 ++++ ...rt_instance_summary_from_event_log.test.ts | 4 + .../server/lib/error_with_reason.test.ts | 28 ++ .../alerts/server/lib/error_with_reason.ts | 29 ++ x-pack/plugins/alerts/server/lib/index.ts | 7 + .../lib/is_alert_not_found_error.test.ts | 22 +- .../server/lib/is_alert_not_found_error.ts | 5 + x-pack/plugins/alerts/server/plugin.ts | 1 + .../alerts/server/routes/create.test.ts | 7 +- .../plugins/alerts/server/routes/get.test.ts | 7 +- .../alerts/server/saved_objects/index.ts | 13 +- .../alerts/server/saved_objects/mappings.json | 20 ++ .../server/saved_objects/migrations.test.ts | 28 +- .../alerts/server/saved_objects/migrations.ts | 23 +- .../saved_objects/partially_update_alert.ts | 4 +- .../task_runner/alert_task_instance.test.ts | 4 + .../server/task_runner/task_runner.test.ts | 15 +- .../alerts/server/task_runner/task_runner.ts | 59 ++- .../task_runner/task_runner_factory.test.ts | 6 +- .../server/task_runner/task_runner_factory.ts | 3 +- x-pack/plugins/alerts/server/types.ts | 12 + .../routes/__mocks__/request_responses.ts | 8 + .../rules/patch_rules.mock.ts | 4 + .../public/application/lib/alert_api.test.ts | 8 + .../public/application/lib/alert_api.ts | 5 +- .../components/alert_details.test.tsx | 4 + .../components/alert_details_route.test.tsx | 4 + .../components/alert_instances.test.tsx | 4 + .../components/alert_instances_route.test.tsx | 4 + .../components/view_in_app.test.tsx | 4 + .../sections/alert_form/alert_edit.test.tsx | 9 +- .../with_bulk_alert_api_operations.test.tsx | 4 + .../common/lib/test_assertions.ts | 18 + .../tests/alerting/create.ts | 1 + .../tests/alerting/execution_status.ts | 87 +++++ .../tests/alerting/find.ts | 81 +++++ .../security_and_spaces/tests/alerting/get.ts | 1 + .../tests/alerting/index.ts | 1 + .../tests/alerting/update.ts | 5 + .../spaces_only/tests/alerting/create.ts | 1 + .../tests/alerting/execution_status.ts | 340 ++++++++++++++++++ .../spaces_only/tests/alerting/find.ts | 1 + .../spaces_only/tests/alerting/get.ts | 1 + .../spaces_only/tests/alerting/index.ts | 3 +- .../spaces_only/tests/alerting/update.ts | 1 + .../spaces_only/tests/index.ts | 8 + .../detection_engine_api_integration/utils.ts | 13 +- 51 files changed, 1142 insertions(+), 47 deletions(-) create mode 100644 x-pack/plugins/alerts/server/lib/alert_execution_status.test.ts create mode 100644 x-pack/plugins/alerts/server/lib/alert_execution_status.ts create mode 100644 x-pack/plugins/alerts/server/lib/error_with_reason.test.ts create mode 100644 x-pack/plugins/alerts/server/lib/error_with_reason.ts create mode 100644 x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/execution_status.ts create mode 100644 x-pack/test/alerting_api_integration/spaces_only/tests/alerting/execution_status.ts diff --git a/x-pack/plugins/alerts/common/alert.ts b/x-pack/plugins/alerts/common/alert.ts index 3ff7ed742e810f..7e5d25b36361c4 100644 --- a/x-pack/plugins/alerts/common/alert.ts +++ b/x-pack/plugins/alerts/common/alert.ts @@ -15,6 +15,18 @@ export interface IntervalSchedule extends SavedObjectAttributes { interval: string; } +export type AlertExecutionStatuses = 'ok' | 'active' | 'error' | 'unknown'; +export type AlertExecutionStatusErrorReasons = 'read' | 'decrypt' | 'execute' | 'unknown'; + +export interface AlertExecutionStatus { + status: AlertExecutionStatuses; + date: Date; + error?: { + reason: AlertExecutionStatusErrorReasons; + message: string; + }; +} + export type AlertActionParams = SavedObjectAttributes; export interface AlertAction { @@ -44,6 +56,7 @@ export interface Alert { throttle: string | null; muteAll: boolean; mutedInstanceIds: string[]; + executionStatus: AlertExecutionStatus; } export type SanitizedAlert = Omit; diff --git a/x-pack/plugins/alerts/server/alerts_client.test.ts b/x-pack/plugins/alerts/server/alerts_client.test.ts index d4817eab64acbe..27410e3b579c82 100644 --- a/x-pack/plugins/alerts/server/alerts_client.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client.test.ts @@ -393,6 +393,11 @@ describe('create()', () => { "createdAt": "2019-02-12T21:01:22.479Z", "createdBy": "elastic", "enabled": true, + "executionStatus": Object { + "date": "2019-02-12T21:01:22.479Z", + "error": null, + "status": "unknown", + }, "meta": Object { "versionApiKeyLastmodified": "v7.10.0", }, @@ -1029,6 +1034,11 @@ describe('create()', () => { muteAll: false, mutedInstanceIds: [], tags: ['foo'], + executionStatus: { + date: '2019-02-12T21:01:22.479Z', + status: 'unknown', + error: null, + }, }, { references: [ @@ -1145,6 +1155,11 @@ describe('create()', () => { muteAll: false, mutedInstanceIds: [], tags: ['foo'], + executionStatus: { + date: '2019-02-12T21:01:22.479Z', + status: 'unknown', + error: null, + }, }, { references: [ @@ -2496,6 +2511,11 @@ const BaseAlertInstanceSummarySavedObject: SavedObject = { throttle: null, muteAll: false, mutedInstanceIds: [], + executionStatus: { + status: 'unknown', + date: '2020-08-20T19:23:38Z', + error: null, + }, }, references: [], }; diff --git a/x-pack/plugins/alerts/server/alerts_client.ts b/x-pack/plugins/alerts/server/alerts_client.ts index 033fdd752c6958..5e7d6e9c5e3bb6 100644 --- a/x-pack/plugins/alerts/server/alerts_client.ts +++ b/x-pack/plugins/alerts/server/alerts_client.ts @@ -28,7 +28,7 @@ import { AlertTaskState, AlertInstanceSummary, } from './types'; -import { validateAlertTypeParams } from './lib'; +import { validateAlertTypeParams, alertExecutionStatusFromRaw } from './lib'; import { InvalidateAPIKeyParams, GrantAPIKeyResult as SecurityPluginGrantAPIKeyResult, @@ -122,6 +122,7 @@ export interface CreateOptions { | 'muteAll' | 'mutedInstanceIds' | 'actions' + | 'executionStatus' > & { actions: NormalizedAlertAction[] }; options?: { migrationVersion?: Record; @@ -228,6 +229,11 @@ export class AlertsClient { params: validatedAlertTypeParams as RawAlert['params'], muteAll: false, mutedInstanceIds: [], + executionStatus: { + status: 'unknown', + date: new Date().toISOString(), + error: null, + }, }; const createdAlert = await this.unsecuredSavedObjectsClient.create( 'alert', @@ -961,9 +967,14 @@ export class AlertsClient { updatedAt: SavedObject['updated_at'] = createdAt, references: SavedObjectReference[] | undefined ): PartialAlert { + const rawAlertWithoutExecutionStatus: Partial> = { + ...rawAlert, + }; + delete rawAlertWithoutExecutionStatus.executionStatus; + const executionStatus = alertExecutionStatusFromRaw(rawAlert.executionStatus); return { id, - ...rawAlert, + ...rawAlertWithoutExecutionStatus, // we currently only support the Interval Schedule type // Once we support additional types, this type signature will likely change schedule: rawAlert.schedule as IntervalSchedule, @@ -973,6 +984,7 @@ export class AlertsClient { ...(updatedAt ? { updatedAt: new Date(updatedAt) } : {}), ...(createdAt ? { createdAt: new Date(createdAt) } : {}), ...(scheduledTaskId ? { scheduledTaskId } : {}), + ...(executionStatus ? { executionStatus } : {}), }; } diff --git a/x-pack/plugins/alerts/server/lib/alert_execution_status.test.ts b/x-pack/plugins/alerts/server/lib/alert_execution_status.test.ts new file mode 100644 index 00000000000000..4d7982bd8b5d96 --- /dev/null +++ b/x-pack/plugins/alerts/server/lib/alert_execution_status.test.ts @@ -0,0 +1,163 @@ +/* + * 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 { AlertExecutionStatusErrorReasons } from '../types'; +import { + executionStatusFromState, + executionStatusFromError, + alertExecutionStatusToRaw, + alertExecutionStatusFromRaw, +} from './alert_execution_status'; +import { ErrorWithReason } from './error_with_reason'; + +describe('AlertExecutionStatus', () => { + describe('executionStatusFromState()', () => { + test('empty task state', () => { + const status = executionStatusFromState({}); + checkDateIsNearNow(status.date); + expect(status.status).toBe('ok'); + expect(status.error).toBe(undefined); + }); + + test('task state with no instances', () => { + const status = executionStatusFromState({ alertInstances: {} }); + checkDateIsNearNow(status.date); + expect(status.status).toBe('ok'); + expect(status.error).toBe(undefined); + }); + + test('task state with one instance', () => { + const status = executionStatusFromState({ alertInstances: { a: {} } }); + checkDateIsNearNow(status.date); + expect(status.status).toBe('active'); + expect(status.error).toBe(undefined); + }); + }); + + describe('executionStatusFromError()', () => { + test('error with no reason', () => { + const status = executionStatusFromError(new Error('boo!')); + expect(status.status).toBe('error'); + expect(status.error).toMatchInlineSnapshot(` + Object { + "message": "boo!", + "reason": "unknown", + } + `); + }); + + test('error with a reason', () => { + const status = executionStatusFromError(new ErrorWithReason('execute', new Error('hoo!'))); + expect(status.status).toBe('error'); + expect(status.error).toMatchInlineSnapshot(` + Object { + "message": "hoo!", + "reason": "execute", + } + `); + }); + }); + + describe('alertExecutionStatusToRaw()', () => { + const date = new Date('2020-09-03T16:26:58Z'); + const status = 'ok'; + const reason: AlertExecutionStatusErrorReasons = 'decrypt'; + const error = { reason, message: 'wops' }; + + test('status without an error', () => { + expect(alertExecutionStatusToRaw({ date, status })).toMatchInlineSnapshot(` + Object { + "date": "2020-09-03T16:26:58.000Z", + "error": null, + "status": "ok", + } + `); + }); + + test('status with an error', () => { + expect(alertExecutionStatusToRaw({ date, status, error })).toMatchInlineSnapshot(` + Object { + "date": "2020-09-03T16:26:58.000Z", + "error": Object { + "message": "wops", + "reason": "decrypt", + }, + "status": "ok", + } + `); + }); + }); + + describe('alertExecutionStatusFromRaw()', () => { + const date = new Date('2020-09-03T16:26:58Z').toISOString(); + const status = 'active'; + const reason: AlertExecutionStatusErrorReasons = 'execute'; + const error = { reason, message: 'wops' }; + + test('no input', () => { + const result = alertExecutionStatusFromRaw(); + expect(result).toBe(undefined); + }); + + test('undefined input', () => { + const result = alertExecutionStatusFromRaw(undefined); + expect(result).toBe(undefined); + }); + + test('null input', () => { + const result = alertExecutionStatusFromRaw(null); + expect(result).toBe(undefined); + }); + + test('invalid date', () => { + const result = alertExecutionStatusFromRaw({ date: 'an invalid date' })!; + checkDateIsNearNow(result.date); + expect(result.status).toBe('unknown'); + expect(result.error).toBe(undefined); + }); + + test('valid date', () => { + const result = alertExecutionStatusFromRaw({ date }); + expect(result).toMatchInlineSnapshot(` + Object { + "date": 2020-09-03T16:26:58.000Z, + "status": "unknown", + } + `); + }); + + test('valid status and date', () => { + const result = alertExecutionStatusFromRaw({ status, date }); + expect(result).toMatchInlineSnapshot(` + Object { + "date": 2020-09-03T16:26:58.000Z, + "status": "active", + } + `); + }); + + test('valid status, date and error', () => { + const result = alertExecutionStatusFromRaw({ status, date, error }); + expect(result).toMatchInlineSnapshot(` + Object { + "date": 2020-09-03T16:26:58.000Z, + "error": Object { + "message": "wops", + "reason": "execute", + }, + "status": "active", + } + `); + }); + }); +}); + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +function checkDateIsNearNow(date: any) { + expect(date instanceof Date).toBe(true); + // allow for lots of slop in the time difference + expect(Date.now() - date.valueOf()).toBeLessThanOrEqual(10000); +} diff --git a/x-pack/plugins/alerts/server/lib/alert_execution_status.ts b/x-pack/plugins/alerts/server/lib/alert_execution_status.ts new file mode 100644 index 00000000000000..25d4115acadcb2 --- /dev/null +++ b/x-pack/plugins/alerts/server/lib/alert_execution_status.ts @@ -0,0 +1,60 @@ +/* + * 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 { AlertTaskState, AlertExecutionStatus, RawAlertExecutionStatus } from '../types'; +import { getReasonFromError } from './error_with_reason'; + +export function executionStatusFromState(state: AlertTaskState): AlertExecutionStatus { + const instanceIds = Object.keys(state.alertInstances ?? {}); + return { + date: new Date(), + status: instanceIds.length === 0 ? 'ok' : 'active', + }; +} + +export function executionStatusFromError(error: Error): AlertExecutionStatus { + return { + date: new Date(), + status: 'error', + error: { + reason: getReasonFromError(error), + message: error.message, + }, + }; +} + +export function alertExecutionStatusToRaw({ + date, + status, + error, +}: AlertExecutionStatus): RawAlertExecutionStatus { + return { + date: date.toISOString(), + status, + error: error ?? null, + }; +} + +export function alertExecutionStatusFromRaw( + rawAlertExecutionStatus?: Partial | null | undefined +): AlertExecutionStatus | undefined { + if (!rawAlertExecutionStatus) return undefined; + + const { date, status = 'unknown', error } = rawAlertExecutionStatus; + + let parsedDateMillis = date ? Date.parse(date) : Date.now(); + if (isNaN(parsedDateMillis)) { + // TODO: log a message? + parsedDateMillis = Date.now(); + } + + const parsedDate = new Date(parsedDateMillis); + if (error) { + return { date: parsedDate, status, error }; + } else { + return { date: parsedDate, status }; + } +} diff --git a/x-pack/plugins/alerts/server/lib/alert_instance_summary_from_event_log.test.ts b/x-pack/plugins/alerts/server/lib/alert_instance_summary_from_event_log.test.ts index b5936cf3577b36..f3cb48176d829a 100644 --- a/x-pack/plugins/alerts/server/lib/alert_instance_summary_from_event_log.test.ts +++ b/x-pack/plugins/alerts/server/lib/alert_instance_summary_from_event_log.test.ts @@ -511,4 +511,8 @@ const BaseAlert: SanitizedAlert = { createdAt: new Date(), updatedAt: new Date(), apiKeyOwner: null, + executionStatus: { + status: 'unknown', + date: new Date('2020-08-20T19:23:38Z'), + }, }; diff --git a/x-pack/plugins/alerts/server/lib/error_with_reason.test.ts b/x-pack/plugins/alerts/server/lib/error_with_reason.test.ts new file mode 100644 index 00000000000000..f31f5844003086 --- /dev/null +++ b/x-pack/plugins/alerts/server/lib/error_with_reason.test.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; + * you may not use this file except in compliance with the Elastic License. + */ + +import { ErrorWithReason, getReasonFromError, isErrorWithReason } from './error_with_reason'; + +describe('ErrorWithReason', () => { + const plainError = new Error('well, actually'); + const errorWithReason = new ErrorWithReason('decrypt', plainError); + + test('ErrorWithReason class', () => { + expect(errorWithReason.message).toBe(plainError.message); + expect(errorWithReason.error).toBe(plainError); + expect(errorWithReason.reason).toBe('decrypt'); + }); + + test('getReasonFromError()', () => { + expect(getReasonFromError(plainError)).toBe('unknown'); + expect(getReasonFromError(errorWithReason)).toBe('decrypt'); + }); + + test('isErrorWithReason()', () => { + expect(isErrorWithReason(plainError)).toBe(false); + expect(isErrorWithReason(errorWithReason)).toBe(true); + }); +}); diff --git a/x-pack/plugins/alerts/server/lib/error_with_reason.ts b/x-pack/plugins/alerts/server/lib/error_with_reason.ts new file mode 100644 index 00000000000000..29eb666e644272 --- /dev/null +++ b/x-pack/plugins/alerts/server/lib/error_with_reason.ts @@ -0,0 +1,29 @@ +/* + * 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 { AlertExecutionStatusErrorReasons } from '../types'; + +export class ErrorWithReason extends Error { + public readonly reason: AlertExecutionStatusErrorReasons; + public readonly error: Error; + + constructor(reason: AlertExecutionStatusErrorReasons, error: Error) { + super(error.message); + this.error = error; + this.reason = reason; + } +} + +export function getReasonFromError(error: Error): AlertExecutionStatusErrorReasons { + if (isErrorWithReason(error)) { + return error.reason; + } + return 'unknown'; +} + +export function isErrorWithReason(error: Error | ErrorWithReason): error is ErrorWithReason { + return error instanceof ErrorWithReason; +} diff --git a/x-pack/plugins/alerts/server/lib/index.ts b/x-pack/plugins/alerts/server/lib/index.ts index 2f610aafd8c31f..32047ae5cbfa83 100644 --- a/x-pack/plugins/alerts/server/lib/index.ts +++ b/x-pack/plugins/alerts/server/lib/index.ts @@ -7,3 +7,10 @@ export { parseDuration, validateDurationSchema } from '../../common/parse_duration'; export { LicenseState } from './license_state'; export { validateAlertTypeParams } from './validate_alert_type_params'; +export { ErrorWithReason, getReasonFromError, isErrorWithReason } from './error_with_reason'; +export { + executionStatusFromState, + executionStatusFromError, + alertExecutionStatusToRaw, + alertExecutionStatusFromRaw, +} from './alert_execution_status'; diff --git a/x-pack/plugins/alerts/server/lib/is_alert_not_found_error.test.ts b/x-pack/plugins/alerts/server/lib/is_alert_not_found_error.test.ts index 46ceee3ce420ba..b570957d82de4a 100644 --- a/x-pack/plugins/alerts/server/lib/is_alert_not_found_error.test.ts +++ b/x-pack/plugins/alerts/server/lib/is_alert_not_found_error.test.ts @@ -5,27 +5,27 @@ */ import { isAlertSavedObjectNotFoundError } from './is_alert_not_found_error'; +import { ErrorWithReason } from './error_with_reason'; import { SavedObjectsErrorHelpers } from '../../../../../src/core/server'; import uuid from 'uuid'; describe('isAlertSavedObjectNotFoundError', () => { + const id = uuid.v4(); + const errorSONF = SavedObjectsErrorHelpers.createGenericNotFoundError('alert', id); + test('identifies SavedObjects Not Found errors', () => { - const id = uuid.v4(); // ensure the error created by SO parses as a string with the format we expect - expect( - `${SavedObjectsErrorHelpers.createGenericNotFoundError('alert', id)}`.includes(`alert/${id}`) - ).toBe(true); - - const errorBySavedObjectsHelper = SavedObjectsErrorHelpers.createGenericNotFoundError( - 'alert', - id - ); + expect(`${errorSONF}`.includes(`alert/${id}`)).toBe(true); - expect(isAlertSavedObjectNotFoundError(errorBySavedObjectsHelper, id)).toBe(true); + expect(isAlertSavedObjectNotFoundError(errorSONF, id)).toBe(true); }); test('identifies generic errors', () => { - const id = uuid.v4(); expect(isAlertSavedObjectNotFoundError(new Error(`not found`), id)).toBe(false); }); + + test('identifies SavedObjects Not Found errors wrapped in an ErrorWithReason', () => { + const error = new ErrorWithReason('read', errorSONF); + expect(isAlertSavedObjectNotFoundError(error, id)).toBe(true); + }); }); diff --git a/x-pack/plugins/alerts/server/lib/is_alert_not_found_error.ts b/x-pack/plugins/alerts/server/lib/is_alert_not_found_error.ts index 0aa83ad0e883ce..3ec946495f0335 100644 --- a/x-pack/plugins/alerts/server/lib/is_alert_not_found_error.ts +++ b/x-pack/plugins/alerts/server/lib/is_alert_not_found_error.ts @@ -5,7 +5,12 @@ */ import { SavedObjectsErrorHelpers } from '../../../../../src/core/server'; +import { isErrorWithReason } from './error_with_reason'; export function isAlertSavedObjectNotFoundError(err: Error, alertId: string) { + // if this is an error with a reason, the actual error needs to be extracted + if (isErrorWithReason(err)) { + err = err.error; + } return SavedObjectsErrorHelpers.isNotFoundError(err) && `${err}`.includes(alertId); } diff --git a/x-pack/plugins/alerts/server/plugin.ts b/x-pack/plugins/alerts/server/plugin.ts index 8f09d55c9a0e01..e9b8c30e55d6b5 100644 --- a/x-pack/plugins/alerts/server/plugin.ts +++ b/x-pack/plugins/alerts/server/plugin.ts @@ -264,6 +264,7 @@ export class AlertingPlugin { encryptedSavedObjectsClient, getBasePath: this.getBasePath, eventLogger: this.eventLogger!, + internalSavedObjectsRepository: core.savedObjects.createInternalRepository(['alert']), }); this.eventLogService!.registerSavedObjectProvider('alert', (request) => { diff --git a/x-pack/plugins/alerts/server/routes/create.test.ts b/x-pack/plugins/alerts/server/routes/create.test.ts index 274acaf01c4751..5a528819c8d1a1 100644 --- a/x-pack/plugins/alerts/server/routes/create.test.ts +++ b/x-pack/plugins/alerts/server/routes/create.test.ts @@ -10,6 +10,7 @@ import { mockLicenseState } from '../lib/license_state.mock'; import { verifyApiAccess } from '../lib/license_api_access'; import { mockHandlerArguments } from './_mock_handler_arguments'; import { alertsClientMock } from '../alerts_client.mock'; +import { Alert } from '../../common/alert'; const alertsClient = alertsClientMock.create(); @@ -46,7 +47,7 @@ describe('createAlertRoute', () => { ], }; - const createResult = { + const createResult: Alert = { ...mockedAlert, enabled: true, muteAll: false, @@ -64,6 +65,10 @@ describe('createAlertRoute', () => { actionTypeId: 'test', }, ], + executionStatus: { + status: 'unknown', + date: new Date('2020-08-20T19:23:38Z'), + }, }; it('creates an alert with proper parameters', async () => { diff --git a/x-pack/plugins/alerts/server/routes/get.test.ts b/x-pack/plugins/alerts/server/routes/get.test.ts index 8c4b06adf70f76..7bf72aab8792cc 100644 --- a/x-pack/plugins/alerts/server/routes/get.test.ts +++ b/x-pack/plugins/alerts/server/routes/get.test.ts @@ -10,6 +10,7 @@ import { mockLicenseState } from '../lib/license_state.mock'; import { verifyApiAccess } from '../lib/license_api_access'; import { mockHandlerArguments } from './_mock_handler_arguments'; import { alertsClientMock } from '../alerts_client.mock'; +import { Alert } from '../../common'; const alertsClient = alertsClientMock.create(); jest.mock('../lib/license_api_access.ts', () => ({ @@ -21,7 +22,7 @@ beforeEach(() => { }); describe('getAlertRoute', () => { - const mockedAlert = { + const mockedAlert: Alert = { id: '1', alertTypeId: '1', schedule: { interval: '10s' }, @@ -51,6 +52,10 @@ describe('getAlertRoute', () => { apiKeyOwner: '', throttle: '30s', mutedInstanceIds: [], + executionStatus: { + status: 'unknown', + date: new Date('2020-08-20T19:23:38Z'), + }, }; it('gets an alert with proper parameters', async () => { diff --git a/x-pack/plugins/alerts/server/saved_objects/index.ts b/x-pack/plugins/alerts/server/saved_objects/index.ts index 51ac68b5899776..9aa1f86676eaa6 100644 --- a/x-pack/plugins/alerts/server/saved_objects/index.ts +++ b/x-pack/plugins/alerts/server/saved_objects/index.ts @@ -16,15 +16,19 @@ export const AlertAttributesExcludedFromAAD = [ 'muteAll', 'mutedInstanceIds', 'updatedBy', + 'executionStatus', ]; // useful for Pick which is a // type which is a subset of RawAlert with just attributes excluded from AAD + +// useful for Pick export type AlertAttributesExcludedFromAADType = | 'scheduledTaskId' | 'muteAll' | 'mutedInstanceIds' - | 'updatedBy'; + | 'updatedBy' + | 'executionStatus'; export function setupSavedObjects( savedObjects: SavedObjectsServiceSetup, @@ -42,11 +46,6 @@ export function setupSavedObjects( encryptedSavedObjects.registerType({ type: 'alert', attributesToEncrypt: new Set(['apiKey']), - attributesToExcludeFromAAD: new Set([ - 'scheduledTaskId', - 'muteAll', - 'mutedInstanceIds', - 'updatedBy', - ]), + attributesToExcludeFromAAD: new Set(AlertAttributesExcludedFromAAD), }); } diff --git a/x-pack/plugins/alerts/server/saved_objects/mappings.json b/x-pack/plugins/alerts/server/saved_objects/mappings.json index 8440b963975ffc..5e1ada42a2d45b 100644 --- a/x-pack/plugins/alerts/server/saved_objects/mappings.json +++ b/x-pack/plugins/alerts/server/saved_objects/mappings.json @@ -83,6 +83,26 @@ "type": "keyword" } } + }, + "executionStatus": { + "properties": { + "status": { + "type": "keyword" + }, + "date": { + "type": "date" + }, + "error": { + "properties": { + "reason": { + "type": "keyword" + }, + "message": { + "type": "keyword" + } + } + } + } } } } diff --git a/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts b/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts index 10e1a9ae421b7d..5305432f4ce730 100644 --- a/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts +++ b/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts @@ -177,7 +177,7 @@ describe('7.10.0', () => { }, ], }); - expect(migration710(alert, { log })).toEqual({ + expect(migration710(alert, { log })).toMatchObject({ ...alert, attributes: { ...alert.attributes, @@ -199,6 +199,30 @@ describe('7.10.0', () => { }, }); }); + + test('creates execution status', () => { + const migration710 = getMigrations(encryptedSavedObjectsSetup)['7.10.0']; + const alert = getMockData(); + const dateStart = Date.now(); + const migratedAlert = migration710(alert, { log }); + const dateStop = Date.now(); + const dateExecutionStatus = Date.parse(migratedAlert.attributes.executionStatus.date); + + expect(dateStart).toBeLessThanOrEqual(dateExecutionStatus); + expect(dateStop).toBeGreaterThanOrEqual(dateExecutionStatus); + + expect(migratedAlert).toMatchObject({ + ...alert, + attributes: { + ...alert.attributes, + executionStatus: { + date: migratedAlert.attributes.executionStatus.date, + status: 'unknown', + error: null, + }, + }, + }); + }); }); describe('7.10.0 migrates with failure', () => { @@ -237,7 +261,7 @@ describe('7.10.0 migrates with failure', () => { function getMockData( overwrites: Record = {} -): SavedObjectUnsanitizedDoc { +): SavedObjectUnsanitizedDoc> { return { attributes: { enabled: true, diff --git a/x-pack/plugins/alerts/server/saved_objects/migrations.ts b/x-pack/plugins/alerts/server/saved_objects/migrations.ts index 537c21e85c0bd7..030df9b2ec5a0e 100644 --- a/x-pack/plugins/alerts/server/saved_objects/migrations.ts +++ b/x-pack/plugins/alerts/server/saved_objects/migrations.ts @@ -30,7 +30,11 @@ export function getMigrations( // migrate all documents in 7.10 in order to add the "meta" RBAC field return true; }, - pipeMigrations(markAsLegacyAndChangeConsumer, setAlertIdAsDefaultDedupkeyOnPagerDutyActions) + pipeMigrations( + markAsLegacyAndChangeConsumer, + setAlertIdAsDefaultDedupkeyOnPagerDutyActions, + initializeExecutionStatus + ) ); return { @@ -110,6 +114,23 @@ function setAlertIdAsDefaultDedupkeyOnPagerDutyActions( }; } +function initializeExecutionStatus( + doc: SavedObjectUnsanitizedDoc +): SavedObjectUnsanitizedDoc { + const { attributes } = doc; + return { + ...doc, + attributes: { + ...attributes, + executionStatus: { + status: 'unknown', + date: new Date().toISOString(), + error: null, + }, + }, + }; +} + function pipeMigrations(...migrations: AlertMigration[]): AlertMigration { return (doc: SavedObjectUnsanitizedDoc) => migrations.reduce((migratedDoc, nextMigration) => nextMigration(migratedDoc), doc); diff --git a/x-pack/plugins/alerts/server/saved_objects/partially_update_alert.ts b/x-pack/plugins/alerts/server/saved_objects/partially_update_alert.ts index cc25aaba357981..b829a6788a3dd3 100644 --- a/x-pack/plugins/alerts/server/saved_objects/partially_update_alert.ts +++ b/x-pack/plugins/alerts/server/saved_objects/partially_update_alert.ts @@ -15,7 +15,9 @@ import { import { AlertAttributesExcludedFromAAD, AlertAttributesExcludedFromAADType } from './index'; -export type PartiallyUpdateableAlertAttributes = Pick; +export type PartiallyUpdateableAlertAttributes = Partial< + Pick +>; export interface PartiallyUpdateAlertSavedObjectOptions { version?: string; diff --git a/x-pack/plugins/alerts/server/task_runner/alert_task_instance.test.ts b/x-pack/plugins/alerts/server/task_runner/alert_task_instance.test.ts index efac4c5dcdc01f..0220f6d81f9a76 100644 --- a/x-pack/plugins/alerts/server/task_runner/alert_task_instance.test.ts +++ b/x-pack/plugins/alerts/server/task_runner/alert_task_instance.test.ts @@ -29,6 +29,10 @@ const alert: SanitizedAlert = { throttle: null, muteAll: false, mutedInstanceIds: [], + executionStatus: { + status: 'unknown', + date: new Date('2020-08-20T19:23:38Z'), + }, }; describe('Alert Task Instance', () => { 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 801d30b6406ee5..4f0c8941d6318c 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 @@ -11,14 +11,17 @@ import { ConcreteTaskInstance, TaskStatus } from '../../../task_manager/server'; import { TaskRunnerContext } from './task_runner_factory'; import { TaskRunner } from './task_runner'; import { encryptedSavedObjectsMock } from '../../../encrypted_saved_objects/server/mocks'; -import { loggingSystemMock } from '../../../../../src/core/server/mocks'; +import { + loggingSystemMock, + savedObjectsRepositoryMock, +} from '../../../../../src/core/server/mocks'; import { PluginStartContract as ActionsPluginStart } from '../../../actions/server'; import { actionsMock, actionsClientMock } from '../../../actions/server/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'; - +import { Alert } from '../../common'; const alertType = { id: 'test', name: 'My test alert', @@ -71,9 +74,10 @@ describe('Task Runner', () => { spaceIdToNamespace: jest.fn().mockReturnValue(undefined), getBasePath: jest.fn().mockReturnValue(undefined), eventLogger: eventLoggerMock.create(), + internalSavedObjectsRepository: savedObjectsRepositoryMock.create(), }; - const mockedAlertTypeSavedObject = { + const mockedAlertTypeSavedObject: Alert = { id: '1', consumer: 'bar', createdAt: new Date('2019-02-12T21:01:22.479Z'), @@ -82,6 +86,7 @@ describe('Task Runner', () => { muteAll: false, enabled: true, alertTypeId: '123', + apiKey: '', apiKeyOwner: 'elastic', schedule: { interval: '10s' }, name: 'alert-name', @@ -102,6 +107,10 @@ describe('Task Runner', () => { }, }, ], + executionStatus: { + status: 'unknown', + date: new Date('2020-08-20T19:23:38Z'), + }, }; beforeEach(() => { 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 7ea3f83d747c0c..1ccf14a3a53340 100644 --- a/x-pack/plugins/alerts/server/task_runner/task_runner.ts +++ b/x-pack/plugins/alerts/server/task_runner/task_runner.ts @@ -11,7 +11,13 @@ import { ConcreteTaskInstance } from '../../../task_manager/server'; import { createExecutionHandler } from './create_execution_handler'; import { AlertInstance, createAlertInstanceFactory } from '../alert_instance'; import { getNextRunAt } from './get_next_run_at'; -import { validateAlertTypeParams } from '../lib'; +import { + validateAlertTypeParams, + executionStatusFromState, + executionStatusFromError, + alertExecutionStatusToRaw, + ErrorWithReason, +} from '../lib'; import { AlertType, RawAlert, @@ -22,6 +28,7 @@ import { Alert, AlertExecutorOptions, SanitizedAlert, + AlertExecutionStatus, } from '../types'; import { promiseResult, map, Resultable, asOk, asErr, resolveErr } from '../lib/result_type'; import { taskInstanceToAlertTaskInstance } from './alert_task_instance'; @@ -29,6 +36,7 @@ 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'; +import { partiallyUpdateAlert } from '../saved_objects'; const FALLBACK_RETRY_INTERVAL: IntervalSchedule = { interval: '5m' }; @@ -204,7 +212,7 @@ export class TaskRunner { event.event = event.event || {}; event.event.outcome = 'failure'; eventLogger.logEvent(event); - throw err; + throw new ErrorWithReason('execute', err); } eventLogger.stopTiming(event); @@ -278,15 +286,22 @@ export class TaskRunner { const { params: { alertId, spaceId }, } = this.taskInstance; + let apiKey: string | null; + try { + apiKey = await this.getApiKeyForAlertPermissions(alertId, spaceId); + } catch (err) { + throw new ErrorWithReason('decrypt', err); + } + const [services, alertsClient] = this.getServicesWithSpaceLevelPermissions(spaceId, apiKey); - const apiKey = await this.getApiKeyForAlertPermissions(alertId, spaceId); - const [services, alertsClient] = await this.getServicesWithSpaceLevelPermissions( - spaceId, - apiKey - ); + let alert: SanitizedAlert; // Ensure API key is still valid and user has access - const alert = await alertsClient.get({ id: alertId }); + try { + alert = await alertsClient.get({ id: alertId }); + } catch (err) { + throw new ErrorWithReason('read', err); + } return { state: await promiseResult( @@ -306,12 +321,38 @@ export class TaskRunner { async run(): Promise { const { - params: { alertId }, + params: { alertId, spaceId }, startedAt: previousStartedAt, state: originalState, } = this.taskInstance; const { state, runAt } = await errorAsAlertTaskRunResult(this.loadAlertAttributesAndRun()); + const namespace = spaceId === 'default' ? undefined : spaceId; + + const executionStatus: AlertExecutionStatus = map( + state, + (alertTaskState: AlertTaskState) => executionStatusFromState(alertTaskState), + (err: Error) => executionStatusFromError(err) + ); + this.logger.debug( + `alertExecutionStatus for ${this.alertType.id}:${alertId}: ${JSON.stringify(executionStatus)}` + ); + + const client = this.context.internalSavedObjectsRepository; + const attributes = { + executionStatus: alertExecutionStatusToRaw(executionStatus), + }; + + try { + await partiallyUpdateAlert(client, alertId, attributes, { + ignore404: true, + namespace, + }); + } catch (err) { + this.logger.error( + `error updating alert execution status for ${this.alertType.id}:${alertId} ${err.message}` + ); + } return { state: map( 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 9af7d9ddc44eb3..5da8e4296f4dd1 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 @@ -8,7 +8,10 @@ import sinon from 'sinon'; import { ConcreteTaskInstance, TaskStatus } from '../../../task_manager/server'; import { TaskRunnerContext, TaskRunnerFactory } from './task_runner_factory'; import { encryptedSavedObjectsMock } from '../../../encrypted_saved_objects/server/mocks'; -import { loggingSystemMock } from '../../../../../src/core/server/mocks'; +import { + loggingSystemMock, + savedObjectsRepositoryMock, +} from '../../../../../src/core/server/mocks'; import { actionsMock } from '../../../actions/server/mocks'; import { alertsMock, alertsClientMock } from '../mocks'; import { eventLoggerMock } from '../../../event_log/server/event_logger.mock'; @@ -63,6 +66,7 @@ describe('Task Runner Factory', () => { spaceIdToNamespace: jest.fn().mockReturnValue(undefined), getBasePath: jest.fn().mockReturnValue(undefined), eventLogger: eventLoggerMock.create(), + internalSavedObjectsRepository: savedObjectsRepositoryMock.create(), }; beforeEach(() => { 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 6f83e34cdbe031..944c4dc64ce7a0 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, KibanaRequest } from '../../../../../src/core/server'; +import { Logger, KibanaRequest, ISavedObjectsRepository } from '../../../../../src/core/server'; import { RunContext } from '../../../task_manager/server'; import { EncryptedSavedObjectsClient } from '../../../encrypted_saved_objects/server'; import { PluginStartContract as ActionsPluginStartContract } from '../../../actions/server'; @@ -26,6 +26,7 @@ export interface TaskRunnerContext { encryptedSavedObjectsClient: EncryptedSavedObjectsClient; spaceIdToNamespace: SpaceIdToNamespaceFunction; getBasePath: GetBasePathFunction; + internalSavedObjectsRepository: ISavedObjectsRepository; } export class TaskRunnerFactory { diff --git a/x-pack/plugins/alerts/server/types.ts b/x-pack/plugins/alerts/server/types.ts index 8d568e8b7ecd1b..97d37b3e41adb8 100644 --- a/x-pack/plugins/alerts/server/types.ts +++ b/x-pack/plugins/alerts/server/types.ts @@ -24,6 +24,8 @@ import { AlertTypeState, AlertInstanceContext, AlertInstanceState, + AlertExecutionStatuses, + AlertExecutionStatusErrorReasons, } from '../common'; export type WithoutQueryAndParams = Pick>; @@ -115,6 +117,15 @@ export interface AlertMeta extends SavedObjectAttributes { versionApiKeyLastmodified?: string; } +export interface RawAlertExecutionStatus extends SavedObjectAttributes { + status: AlertExecutionStatuses; + date: string; + error: null | { + reason: AlertExecutionStatusErrorReasons; + message: string; + }; +} + export type PartialAlert = Pick & Partial>; export interface RawAlert extends SavedObjectAttributes { @@ -136,6 +147,7 @@ export interface RawAlert extends SavedObjectAttributes { muteAll: boolean; mutedInstanceIds: string[]; meta?: AlertMeta; + executionStatus: RawAlertExecutionStatus; } export type AlertInfoParams = Pick< 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 5d9cfb4bb44928..fdc2d839e27f67 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 @@ -419,6 +419,10 @@ export const getResult = (): RuleAlertType => ({ muteAll: false, mutedInstanceIds: [], scheduledTaskId: '2dabe330-0702-11ea-8b50-773b89126888', + executionStatus: { + status: 'unknown', + date: new Date('2020-08-20T19:23:38Z'), + }, }); export const getMlResult = (): RuleAlertType => { @@ -630,6 +634,10 @@ export const getNotificationResult = (): RuleNotificationAlertType => ({ mutedInstanceIds: [], scheduledTaskId: '62b3a130-6b70-11ea-9ce9-6b9818c4cbd7', updatedAt: new Date('2020-03-21T12:37:08.730Z'), + executionStatus: { + status: 'unknown', + date: new Date('2020-08-20T19:23:38Z'), + }, }); export const getFindNotificationsResultWithSingleHit = (): FindHit => ({ diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/patch_rules.mock.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/patch_rules.mock.ts index aeb136a969aa18..0259a75e3e99d3 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/patch_rules.mock.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/patch_rules.mock.ts @@ -110,6 +110,10 @@ const rule: SanitizedAlert = { muteAll: false, mutedInstanceIds: [], scheduledTaskId: '2dabe330-0702-11ea-8b50-773b89126888', + executionStatus: { + status: 'unknown', + date: new Date('2020-08-20T19:23:38Z'), + }, }; export const getPatchRulesOptionsMock = (): PatchRulesOptions => ({ diff --git a/x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api.test.ts b/x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api.test.ts index fc5d301cb7cd03..a7d70a1e8640da 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api.test.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api.test.ts @@ -398,6 +398,10 @@ describe('createAlert', () => { updatedBy: null, muteAll: false, mutedInstanceIds: [], + executionStatus: { + status: 'unknown', + date: new Date('2020-08-20T19:23:38Z'), + }, }; http.post.mockResolvedValueOnce(resolvedValue); @@ -440,6 +444,10 @@ describe('updateAlert', () => { updatedBy: null, muteAll: false, mutedInstanceIds: [], + executionStatus: { + status: 'unknown', + date: new Date('2020-08-20T19:23:38Z'), + }, }; http.put.mockResolvedValueOnce(resolvedValue); 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 97feea6ba8a0f8..d5711a3e8c919a 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 @@ -136,7 +136,10 @@ export async function createAlert({ alert, }: { http: HttpSetup; - alert: Omit; + alert: Omit< + AlertWithoutId, + 'createdBy' | 'updatedBy' | 'muteAll' | 'mutedInstanceIds' | 'executionStatus' + >; }): Promise { return await http.post(`${BASE_ALERT_API_PATH}/alert`, { body: JSON.stringify(alert), diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_details.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_details.test.tsx index 16d1a5c7c9c652..6cff022b4beed6 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_details.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_details.test.tsx @@ -757,6 +757,10 @@ function mockAlert(overloads: Partial = {}): Alert { throttle: null, muteAll: false, mutedInstanceIds: [], + executionStatus: { + status: 'unknown', + date: new Date('2020-08-20T19:23:38Z'), + }, ...overloads, }; } diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_details_route.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_details_route.test.tsx index 7a40104e97d9fd..390e54441cfe49 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_details_route.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_details_route.test.tsx @@ -404,6 +404,10 @@ function mockAlert(overloads: Partial = {}): Alert { throttle: null, muteAll: false, mutedInstanceIds: [], + executionStatus: { + status: 'unknown', + date: new Date('2020-08-20T19:23:38Z'), + }, ...overloads, }; } diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_instances.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_instances.test.tsx index f59b836a7936e0..60b1e68ba05dc2 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_instances.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_instances.test.tsx @@ -254,6 +254,10 @@ function mockAlert(overloads: Partial = {}): Alert { throttle: null, muteAll: false, mutedInstanceIds: [], + executionStatus: { + status: 'unknown', + date: new Date('2020-08-20T19:23:38Z'), + }, ...overloads, }; } diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_instances_route.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_instances_route.test.tsx index d92148a8fea531..e679166233cfd2 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_instances_route.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_instances_route.test.tsx @@ -132,6 +132,10 @@ function mockAlert(overloads: Partial = {}): Alert { throttle: null, muteAll: false, mutedInstanceIds: [], + executionStatus: { + status: 'unknown', + date: new Date('2020-08-20T19:23:38Z'), + }, ...overloads, }; } diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/view_in_app.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/view_in_app.test.tsx index 54d335aaba5aa9..9facb453bae615 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/view_in_app.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/view_in_app.test.tsx @@ -105,6 +105,10 @@ function mockAlert(overloads: Partial = {}): Alert { throttle: null, muteAll: false, mutedInstanceIds: [], + executionStatus: { + status: 'unknown', + date: new Date('2020-08-20T19:23:38Z'), + }, ...overloads, }; } diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_edit.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_edit.test.tsx index e408c7fcb81441..65c9f92b8c8afd 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_edit.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_edit.test.tsx @@ -8,7 +8,7 @@ import { mountWithIntl, nextTick } from 'test_utils/enzyme_helpers'; import { act } from 'react-dom/test-utils'; import { coreMock } from '../../../../../../../src/core/public/mocks'; import { actionTypeRegistryMock } from '../../action_type_registry.mock'; -import { ValidationResult } from '../../../types'; +import { ValidationResult, Alert } from '../../../types'; import { AlertsContextProvider } from '../../context/alerts_context'; import { alertTypeRegistryMock } from '../../alert_type_registry.mock'; import { ReactWrapper } from 'enzyme'; @@ -73,7 +73,7 @@ describe('alert_edit', () => { actionParamsFields: null, }; - const alert = { + const alert: Alert = { id: 'ab5661e0-197e-45ee-b477-302d89193b5e', params: { aggType: 'average', @@ -93,7 +93,6 @@ describe('alert_edit', () => { actionTypeId: 'my-action-type', group: 'threshold met', params: { message: 'Alert [{{ctx.metadata.name}}] has exceeded the threshold' }, - message: 'Alert [{{ctx.metadata.name}}] has exceeded the threshold', id: '917f5d41-fbc4-4056-a8ad-ac592f7dcee2', }, ], @@ -107,6 +106,10 @@ describe('alert_edit', () => { muteAll: false, mutedInstanceIds: [], updatedAt: new Date(), + executionStatus: { + status: 'unknown', + date: new Date('2020-08-20T19:23:38Z'), + }, }; actionTypeRegistry.get.mockReturnValueOnce(actionTypeModel); actionTypeRegistry.has.mockReturnValue(true); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/common/components/with_bulk_alert_api_operations.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/common/components/with_bulk_alert_api_operations.test.tsx index 074e2d5147b5ec..4f0e02b0f1f6a6 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/common/components/with_bulk_alert_api_operations.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/common/components/with_bulk_alert_api_operations.test.tsx @@ -264,6 +264,10 @@ function mockAlert(overloads: Partial = {}): Alert { throttle: null, muteAll: false, mutedInstanceIds: [], + executionStatus: { + status: 'unknown', + date: new Date('2020-08-20T19:23:38Z'), + }, ...overloads, }; } diff --git a/x-pack/test/alerting_api_integration/common/lib/test_assertions.ts b/x-pack/test/alerting_api_integration/common/lib/test_assertions.ts index 9495dd4cfae82a..6124a5fb7c358e 100644 --- a/x-pack/test/alerting_api_integration/common/lib/test_assertions.ts +++ b/x-pack/test/alerting_api_integration/common/lib/test_assertions.ts @@ -15,3 +15,21 @@ export function ensureDatetimeIsWithinRange( expect(diff).to.be.greaterThan(expectedDiff - buffer); expect(diff).to.be.lessThan(expectedDiff + buffer); } + +export function ensureDatetimesAreOrdered(dates: Array) { + const dateStrings = dates.map(normalizeDate); + const sortedDateStrings = dateStrings.slice().sort(); + expect(dateStrings).to.eql(sortedDateStrings); +} + +function normalizeDate(date: Date | string | number): string { + if (typeof date === 'number') return new Date(date).toISOString(); + if (date instanceof Date) return date.toISOString(); + + const dateString = `${date}`; + const dateNumber = Date.parse(dateString); + if (isNaN(dateNumber)) { + throw new Error(`invalid date string: "${dateString}"`); + } + return new Date(dateNumber).toISOString(); +} diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/create.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/create.ts index 983f87405a1a64..19d90378e8b7a8 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/create.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/create.ts @@ -119,6 +119,7 @@ export default function createAlertTests({ getService }: FtrProviderContext) { apiKeyOwner: user.username, muteAll: false, mutedInstanceIds: [], + executionStatus: response.body.executionStatus, }); expect(typeof response.body.scheduledTaskId).to.be('string'); expect(Date.parse(response.body.createdAt)).to.be.greaterThan(0); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/execution_status.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/execution_status.ts new file mode 100644 index 00000000000000..8fb89042e4a903 --- /dev/null +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/execution_status.ts @@ -0,0 +1,87 @@ +/* + * 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 expect from '@kbn/expect'; +import { Spaces } from '../../scenarios'; +import { getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; +import { FtrProviderContext } from '../../../common/ftr_provider_context'; + +// eslint-disable-next-line import/no-default-export +export default function executionStatusAlertTests({ getService }: FtrProviderContext) { + const supertest = getService('supertest'); + const spaceId = Spaces[0].id; + + // the only tests here are those that can't be run in spaces_only + describe('executionStatus', () => { + const objectRemover = new ObjectRemover(supertest); + + after(async () => await objectRemover.removeAll()); + + it('should eventually have error reason "decrypt" when appropriate', async () => { + const response = await supertest + .post(`${getUrlPrefix(spaceId)}/api/alerts/alert`) + .set('kbn-xsrf', 'foo') + .send( + getTestAlertData({ + alertTypeId: 'test.noop', + schedule: { interval: '1s' }, + }) + ); + expect(response.status).to.eql(200); + const alertId = response.body.id; + objectRemover.add(spaceId, alertId, 'alert', 'alerts'); + + let executionStatus = await waitForStatus(alertId, new Set(['ok']), 10000); + + // break AAD + await supertest + .put(`${getUrlPrefix(spaceId)}/api/alerts_fixture/saved_object/alert/${alertId}`) + .set('kbn-xsrf', 'foo') + .send({ + attributes: { + name: 'bar', + }, + }) + .expect(200); + + executionStatus = await waitForStatus(alertId, new Set(['error'])); + expect(executionStatus.error).to.be.ok(); + expect(executionStatus.error.reason).to.be('decrypt'); + expect(executionStatus.error.message).to.be('Unable to decrypt attribute "apiKey"'); + }); + }); + + const WaitForStatusIncrement = 500; + + async function waitForStatus( + id: string, + statuses: Set, + waitMillis: number = 10000 + ): Promise> { + if (waitMillis < 0) { + expect().fail(`waiting for alert ${id} statuses ${Array.from(statuses)} timed out`); + } + + const response = await supertest.get(`${getUrlPrefix(spaceId)}/api/alerts/alert/${id}`); + expect(response.status).to.eql(200); + const { status } = response.body.executionStatus; + if (statuses.has(status)) return response.body.executionStatus; + + // eslint-disable-next-line no-console + console.log( + `waitForStatus(${Array.from(statuses)}): got ${JSON.stringify( + response.body.executionStatus + )}, retrying` + ); + + await delay(WaitForStatusIncrement); + return await waitForStatus(id, statuses, waitMillis - WaitForStatusIncrement); + } +} + +async function delay(millis: number): Promise { + await new Promise((resolve) => setTimeout(resolve, millis)); +} diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/find.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/find.ts index 268212d4294d0c..adfe5cd27b33a1 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/find.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/find.ts @@ -79,6 +79,7 @@ export default function createFindTests({ getService }: FtrProviderContext) { apiKeyOwner: 'elastic', muteAll: false, mutedInstanceIds: [], + executionStatus: match.executionStatus, }); expect(Date.parse(match.createdAt)).to.be.greaterThan(0); expect(Date.parse(match.updatedAt)).to.be.greaterThan(0); @@ -273,6 +274,7 @@ export default function createFindTests({ getService }: FtrProviderContext) { mutedInstanceIds: [], createdAt: match.createdAt, updatedAt: match.updatedAt, + executionStatus: match.executionStatus, }); expect(Date.parse(match.createdAt)).to.be.greaterThan(0); expect(Date.parse(match.updatedAt)).to.be.greaterThan(0); @@ -359,6 +361,85 @@ export default function createFindTests({ getService }: FtrProviderContext) { } }); + it('should handle find alert request with executionStatus field appropriately', async () => { + const myTag = uuid.v4(); + const { body: createdAlert } = await supertest + .post(`${getUrlPrefix(space.id)}/api/alerts/alert`) + .set('kbn-xsrf', 'foo') + .send( + getTestAlertData({ + enabled: false, + tags: [myTag], + alertTypeId: 'test.restricted-noop', + consumer: 'alertsRestrictedFixture', + }) + ) + .expect(200); + objectRemover.add(space.id, createdAlert.id, 'alert', 'alerts'); + + // create another type with same tag + const { body: createdSecondAlert } = await supertest + .post(`${getUrlPrefix(space.id)}/api/alerts/alert`) + .set('kbn-xsrf', 'foo') + .send( + getTestAlertData({ + tags: [myTag], + alertTypeId: 'test.restricted-noop', + consumer: 'alertsRestrictedFixture', + }) + ) + .expect(200); + objectRemover.add(space.id, createdSecondAlert.id, 'alert', 'alerts'); + + const response = await supertestWithoutAuth + .get( + `${getUrlPrefix( + space.id + )}/api/alerts/_find?filter=alert.attributes.alertTypeId:test.restricted-noop&fields=["tags","executionStatus"]&sort_field=createdAt` + ) + .auth(user.username, user.password); + + switch (scenario.id) { + case 'no_kibana_privileges at space1': + case 'space_1_all at space2': + expect(response.statusCode).to.eql(403); + expect(response.body).to.eql({ + error: 'Forbidden', + message: `Unauthorized to find any alert types`, + statusCode: 403, + }); + break; + case 'space_1_all at space1': + case 'space_1_all_alerts_none_actions at space1': + expect(response.statusCode).to.eql(200); + expect(response.body.data).to.eql([]); + break; + case 'global_read at space1': + case 'superuser at space1': + case 'space_1_all_with_restricted_fixture at space1': + expect(response.statusCode).to.eql(200); + expect(response.body.page).to.equal(1); + expect(response.body.perPage).to.be.greaterThan(0); + expect(response.body.total).to.be.greaterThan(0); + const [matchFirst, matchSecond] = response.body.data; + expect(omit(matchFirst, 'updatedAt')).to.eql({ + id: createdAlert.id, + actions: [], + tags: [myTag], + executionStatus: matchFirst.executionStatus, + }); + expect(omit(matchSecond, 'updatedAt')).to.eql({ + id: createdSecondAlert.id, + actions: [], + tags: [myTag], + executionStatus: matchSecond.executionStatus, + }); + break; + default: + throw new Error(`Scenario untested: ${JSON.stringify(scenario)}`); + } + }); + it(`shouldn't find alert from another space`, async () => { const { body: createdAlert } = await supertest .post(`${getUrlPrefix(space.id)}/api/alerts/alert`) diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/get.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/get.ts index 1043ece08a2ac0..93e9be771ab5c6 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/get.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/get.ts @@ -75,6 +75,7 @@ export default function createGetTests({ getService }: FtrProviderContext) { apiKeyOwner: 'elastic', muteAll: false, mutedInstanceIds: [], + executionStatus: response.body.executionStatus, }); expect(Date.parse(response.body.createdAt)).to.be.greaterThan(0); expect(Date.parse(response.body.updatedAt)).to.be.greaterThan(0); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/index.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/index.ts index fa0130780cb692..1fbee9e18fdaa2 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/index.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/index.ts @@ -14,6 +14,7 @@ export default function alertingTests({ loadTestFile }: FtrProviderContext) { loadTestFile(require.resolve('./delete')); loadTestFile(require.resolve('./disable')); loadTestFile(require.resolve('./enable')); + loadTestFile(require.resolve('./execution_status')); loadTestFile(require.resolve('./get')); loadTestFile(require.resolve('./get_alert_state')); loadTestFile(require.resolve('./get_alert_instance_summary')); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update.ts index 48269cc1c4498f..d75aa868253deb 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update.ts @@ -129,6 +129,7 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { scheduledTaskId: createdAlert.scheduledTaskId, createdAt: response.body.createdAt, updatedAt: response.body.updatedAt, + executionStatus: response.body.executionStatus, }); expect(Date.parse(response.body.createdAt)).to.be.greaterThan(0); expect(Date.parse(response.body.updatedAt)).to.be.greaterThan(0); @@ -211,6 +212,7 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { scheduledTaskId: createdAlert.scheduledTaskId, createdAt: response.body.createdAt, updatedAt: response.body.updatedAt, + executionStatus: response.body.executionStatus, }); expect(Date.parse(response.body.createdAt)).to.be.greaterThan(0); expect(Date.parse(response.body.updatedAt)).to.be.greaterThan(0); @@ -304,6 +306,7 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { scheduledTaskId: createdAlert.scheduledTaskId, createdAt: response.body.createdAt, updatedAt: response.body.updatedAt, + executionStatus: response.body.executionStatus, }); expect(Date.parse(response.body.createdAt)).to.be.greaterThan(0); expect(Date.parse(response.body.updatedAt)).to.be.greaterThan(0); @@ -397,6 +400,7 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { scheduledTaskId: createdAlert.scheduledTaskId, createdAt: response.body.createdAt, updatedAt: response.body.updatedAt, + executionStatus: response.body.executionStatus, }); expect(Date.parse(response.body.createdAt)).to.be.greaterThan(0); expect(Date.parse(response.body.updatedAt)).to.be.greaterThan(0); @@ -486,6 +490,7 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { scheduledTaskId: createdAlert.scheduledTaskId, createdAt: response.body.createdAt, updatedAt: response.body.updatedAt, + executionStatus: response.body.executionStatus, }); expect(Date.parse(response.body.createdAt)).to.be.greaterThan(0); expect(Date.parse(response.body.updatedAt)).to.be.greaterThan(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 86775f77a7671c..41f6b66c30aafa 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 @@ -87,6 +87,7 @@ export default function createAlertTests({ getService }: FtrProviderContext) { mutedInstanceIds: [], createdAt: response.body.createdAt, updatedAt: response.body.updatedAt, + executionStatus: response.body.executionStatus, }); expect(Date.parse(response.body.createdAt)).to.be.greaterThan(0); expect(Date.parse(response.body.updatedAt)).to.be.greaterThan(0); diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/execution_status.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/execution_status.ts new file mode 100644 index 00000000000000..94471576c2242d --- /dev/null +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/execution_status.ts @@ -0,0 +1,340 @@ +/* + * 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 expect from '@kbn/expect'; +import { Spaces } from '../../scenarios'; +import { + checkAAD, + getUrlPrefix, + getTestAlertData, + ObjectRemover, + ensureDatetimesAreOrdered, +} from '../../../common/lib'; +import { FtrProviderContext } from '../../../common/ftr_provider_context'; + +// eslint-disable-next-line import/no-default-export +export default function executionStatusAlertTests({ getService }: FtrProviderContext) { + const supertest = getService('supertest'); + + describe('executionStatus', () => { + const objectRemover = new ObjectRemover(supertest); + + after(async () => await objectRemover.removeAll()); + + it('should be "unknown" for newly created alert', async () => { + const dateStart = Date.now(); + const response = await supertest + .post(`${getUrlPrefix(Spaces.space1.id)}/api/alerts/alert`) + .set('kbn-xsrf', 'foo') + .send(getTestAlertData()); + const dateEnd = Date.now(); + expect(response.status).to.eql(200); + objectRemover.add(Spaces.space1.id, response.body.id, 'alert', 'alerts'); + + expect(response.body.executionStatus).to.be.ok(); + const { status, date, error } = response.body.executionStatus; + expect(status).to.be('unknown'); + ensureDatetimesAreOrdered([dateStart, date, dateEnd]); + expect(error).not.to.be.ok(); + + // Ensure AAD isn't broken + await checkAAD({ + supertest, + spaceId: Spaces.space1.id, + type: 'alert', + id: response.body.id, + }); + }); + + it('should eventually be "ok" for no-op alert', async () => { + const dates = []; + dates.push(Date.now()); + const response = await supertest + .post(`${getUrlPrefix(Spaces.space1.id)}/api/alerts/alert`) + .set('kbn-xsrf', 'foo') + .send( + getTestAlertData({ + alertTypeId: 'test.noop', + schedule: { interval: '1s' }, + }) + ); + expect(response.status).to.eql(200); + const alertId = response.body.id; + dates.push(response.body.executionStatus.date); + dates.push(Date.now()); + objectRemover.add(Spaces.space1.id, alertId, 'alert', 'alerts'); + + const executionStatus = await waitForStatus(alertId, new Set(['ok'])); + dates.push(executionStatus.date); + dates.push(Date.now()); + ensureDatetimesAreOrdered(dates); + + // Ensure AAD isn't broken + await checkAAD({ + supertest, + spaceId: Spaces.space1.id, + type: 'alert', + id: response.body.id, + }); + }); + + it('should eventually be "active" for firing alert', async () => { + const dates = []; + dates.push(Date.now()); + const response = await supertest + .post(`${getUrlPrefix(Spaces.space1.id)}/api/alerts/alert`) + .set('kbn-xsrf', 'foo') + .send( + getTestAlertData({ + alertTypeId: 'test.patternFiring', + schedule: { interval: '1s' }, + params: { + pattern: { instance: trues(100) }, + }, + }) + ); + expect(response.status).to.eql(200); + const alertId = response.body.id; + dates.push(response.body.executionStatus.date); + dates.push(Date.now()); + objectRemover.add(Spaces.space1.id, alertId, 'alert', 'alerts'); + + const executionStatus = await waitForStatus(alertId, new Set(['active'])); + dates.push(executionStatus.date); + dates.push(Date.now()); + ensureDatetimesAreOrdered(dates); + + // Ensure AAD isn't broken + await checkAAD({ + supertest, + spaceId: Spaces.space1.id, + type: 'alert', + id: response.body.id, + }); + }); + + it('should eventually be "error" for an error alert', async () => { + const dates = []; + dates.push(Date.now()); + const response = await supertest + .post(`${getUrlPrefix(Spaces.space1.id)}/api/alerts/alert`) + .set('kbn-xsrf', 'foo') + .send( + getTestAlertData({ + alertTypeId: 'test.throw', + schedule: { interval: '1s' }, + }) + ); + expect(response.status).to.eql(200); + const alertId = response.body.id; + dates.push(response.body.executionStatus.date); + dates.push(Date.now()); + objectRemover.add(Spaces.space1.id, alertId, 'alert', 'alerts'); + + const executionStatus = await waitForStatus(alertId, new Set(['error'])); + dates.push(executionStatus.date); + dates.push(Date.now()); + ensureDatetimesAreOrdered(dates); + + // Ensure AAD isn't broken + await checkAAD({ + supertest, + spaceId: Spaces.space1.id, + type: 'alert', + id: response.body.id, + }); + }); + + // not sure how to test the read error reason! + + // note the decrypt error reason is tested in security_and_spaces, can't be tested + // without security on + + it('should eventually have error reason "execute" when appropriate', async () => { + const response = await supertest + .post(`${getUrlPrefix(Spaces.space1.id)}/api/alerts/alert`) + .set('kbn-xsrf', 'foo') + .send( + getTestAlertData({ + alertTypeId: 'test.throw', + schedule: { interval: '1s' }, + }) + ); + expect(response.status).to.eql(200); + const alertId = response.body.id; + objectRemover.add(Spaces.space1.id, alertId, 'alert', 'alerts'); + + const executionStatus = await waitForStatus(alertId, new Set(['error'])); + expect(executionStatus.error).to.be.ok(); + expect(executionStatus.error.reason).to.be('execute'); + expect(executionStatus.error.message).to.be('this alert is intended to fail'); + }); + + it('should eventually have error reason "unknown" when appropriate', async () => { + const response = await supertest + .post(`${getUrlPrefix(Spaces.space1.id)}/api/alerts/alert`) + .set('kbn-xsrf', 'foo') + .send( + getTestAlertData({ + alertTypeId: 'test.validation', + schedule: { interval: '1s' }, + params: { param1: 'valid now, but will change to a number soon!' }, + }) + ); + expect(response.status).to.eql(200); + const alertId = response.body.id; + objectRemover.add(Spaces.space1.id, alertId, 'alert', 'alerts'); + + let executionStatus = await waitForStatus(alertId, new Set(['ok'])); + + // break the validation of the params + await supertest + .put(`${getUrlPrefix(Spaces.space1.id)}/api/alerts_fixture/saved_object/alert/${alertId}`) + .set('kbn-xsrf', 'foo') + .send({ + attributes: { + params: { param1: 42 }, + }, + }) + .expect(200); + + executionStatus = await waitForStatus(alertId, new Set(['error'])); + expect(executionStatus.error).to.be.ok(); + expect(executionStatus.error.reason).to.be('unknown'); + + const message = 'params invalid: [param1]: expected value of type [string] but got [number]'; + expect(executionStatus.error.message).to.be(message); + }); + + it('should be able to find over all the fields', async () => { + const startDate = Date.now(); + const createResponse = await supertest + .post(`${getUrlPrefix(Spaces.space1.id)}/api/alerts/alert`) + .set('kbn-xsrf', 'foo') + .send( + getTestAlertData({ + alertTypeId: 'test.throw', + schedule: { interval: '1s' }, + }) + ); + expect(createResponse.status).to.eql(200); + const alertId = createResponse.body.id; + objectRemover.add(Spaces.space1.id, alertId, 'alert', 'alerts'); + + await waitForStatus(alertId, new Set(['error'])); + + let filter = `date>${startDate}`; + let executionStatus = await waitForFindStatus(alertId, new Set(['error']), filter); + expectErrorExecutionStatus(executionStatus, startDate); + + filter = `status:error`; + executionStatus = await waitForFindStatus(alertId, new Set(['error']), filter); + expectErrorExecutionStatus(executionStatus, startDate); + + filter = `error.message:*intended*`; + executionStatus = await waitForFindStatus(alertId, new Set(['error']), filter); + expectErrorExecutionStatus(executionStatus, startDate); + + filter = `error.reason:execute`; + executionStatus = await waitForFindStatus(alertId, new Set(['error']), filter); + expectErrorExecutionStatus(executionStatus, startDate); + }); + }); + + const WaitForStatusIncrement = 500; + + async function waitForStatus( + id: string, + statuses: Set, + waitMillis: number = 10000 + ): Promise> { + if (waitMillis < 0) { + expect().fail(`waiting for alert ${id} statuses ${Array.from(statuses)} timed out`); + } + + const response = await supertest.get( + `${getUrlPrefix(Spaces.space1.id)}/api/alerts/alert/${id}` + ); + expect(response.status).to.eql(200); + const { status } = response.body.executionStatus; + + const message = `waitForStatus(${Array.from(statuses)}): got ${JSON.stringify( + response.body.executionStatus + )}`; + + if (statuses.has(status)) { + return response.body.executionStatus; + } + + // eslint-disable-next-line no-console + console.log(`${message}, retrying`); + + await delay(WaitForStatusIncrement); + return await waitForStatus(id, statuses, waitMillis - WaitForStatusIncrement); + } + + async function waitForFindStatus( + id: string, + statuses: Set, + filter: string, + waitMillis: number = 10000 + ): Promise> { + if (waitMillis < 0) { + expect().fail(`waiting for find alert ${id} statuses ${Array.from(statuses)} timed out`); + } + + const findUri = getFindUri(filter); + const response = await supertest.get(`${getUrlPrefix(Spaces.space1.id)}/${findUri}`); + + expect(response.status).to.eql(200); + const { executionStatus } = response.body.data.find((obj: any) => obj.id === id); + + const message = `waitForFindStatus(${Array.from(statuses)}): got ${JSON.stringify( + executionStatus + )}`; + + if (statuses.has(executionStatus.status)) { + return executionStatus; + } + + // eslint-disable-next-line no-console + console.log(`${message}, retrying`); + + await delay(WaitForStatusIncrement); + return await waitForStatus(id, statuses, waitMillis - WaitForStatusIncrement); + } +} + +function expectErrorExecutionStatus(executionStatus: Record, startDate: number) { + expect(executionStatus.status).to.equal('error'); + + const statusDate = Date.parse(executionStatus.date); + const stopDate = Date.now(); + expect(startDate).to.be.lessThan(statusDate); + expect(stopDate).to.be.greaterThan(statusDate); + + expect(executionStatus.error.message).to.equal('this alert is intended to fail'); + expect(executionStatus.error.reason).to.equal('execute'); +} + +function getFindUri(filter: string) { + return `api/alerts/_find?filter=alert.attributes.executionStatus.${filter}`; +} + +function trues(length: number): boolean[] { + return booleans(length, true); +} + +function booleans(length: number, value: boolean): boolean[] { + return '' + .padStart(length) + .split('') + .map((e) => value); +} + +async function delay(millis: number): Promise { + await new Promise((resolve) => setTimeout(resolve, millis)); +} 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 b28ce89b304724..850ec24789f5b1 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 @@ -56,6 +56,7 @@ export default function createFindTests({ getService }: FtrProviderContext) { mutedInstanceIds: [], createdAt: match.createdAt, updatedAt: match.updatedAt, + executionStatus: match.executionStatus, }); expect(Date.parse(match.createdAt)).to.be.greaterThan(0); expect(Date.parse(match.updatedAt)).to.be.greaterThan(0); 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 165eaa09126a81..14a57f57c9237d 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 @@ -50,6 +50,7 @@ export default function createGetTests({ getService }: FtrProviderContext) { mutedInstanceIds: [], createdAt: response.body.createdAt, updatedAt: response.body.updatedAt, + executionStatus: response.body.executionStatus, }); expect(Date.parse(response.body.createdAt)).to.be.greaterThan(0); expect(Date.parse(response.body.updatedAt)).to.be.greaterThan(0); diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/index.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/index.ts index 3a3fed22f02066..b2a8b31c5fd212 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/index.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/index.ts @@ -8,7 +8,7 @@ import { FtrProviderContext } from '../../../common/ftr_provider_context'; // eslint-disable-next-line import/no-default-export export default function alertingTests({ loadTestFile }: FtrProviderContext) { - describe('Alerting', () => { + describe('Alerts', () => { loadTestFile(require.resolve('./create')); loadTestFile(require.resolve('./delete')); loadTestFile(require.resolve('./disable')); @@ -19,6 +19,7 @@ export default function alertingTests({ loadTestFile }: FtrProviderContext) { loadTestFile(require.resolve('./get_alert_instance_summary')); loadTestFile(require.resolve('./list_alert_types')); loadTestFile(require.resolve('./event_log')); + loadTestFile(require.resolve('./execution_status')); loadTestFile(require.resolve('./mute_all')); loadTestFile(require.resolve('./mute_instance')); loadTestFile(require.resolve('./unmute_all')); 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 9c8e6f6b8d94c8..f44a7d71318799 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 @@ -57,6 +57,7 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { scheduledTaskId: createdAlert.scheduledTaskId, createdAt: response.body.createdAt, updatedAt: response.body.updatedAt, + executionStatus: response.body.executionStatus, }); expect(Date.parse(response.body.createdAt)).to.be.greaterThan(0); expect(Date.parse(response.body.updatedAt)).to.be.greaterThan(0); diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/index.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/index.ts index 281096f8a3592e..46a6bf2b366e03 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/index.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/index.ts @@ -23,6 +23,14 @@ export default function alertingApiIntegrationTests({ if (space.id === 'default') continue; const { id, name, disabledFeatures } = space; + + try { + await spacesService.delete(id); + } catch (err) { + // try deleting before creating, in case it already exists, + // but obviously it's ok if it fails (because it doesn't yet exist) + } + await spacesService.create({ id, name, disabledFeatures }); } }); diff --git a/x-pack/test/detection_engine_api_integration/utils.ts b/x-pack/test/detection_engine_api_integration/utils.ts index 1dba1a154373b3..5d82eed41d3c5a 100644 --- a/x-pack/test/detection_engine_api_integration/utils.ts +++ b/x-pack/test/detection_engine_api_integration/utils.ts @@ -248,16 +248,25 @@ export const getSimpleMlRuleOutput = (ruleId = 'rule-1'): Partial = export const deleteAllAlerts = async (es: Client, retryCount = 20): Promise => { if (retryCount > 0) { try { - await es.deleteByQuery({ + const result = await es.deleteByQuery({ index: '.kibana', q: 'type:alert', wait_for_completion: true, refresh: true, + conflicts: 'proceed', body: {}, }); + // deleteByQuery will cause version conflicts as alerts are being updated + // by background processes; the code below accounts for that + if (result.body.version_conflicts !== 0) { + throw new Error(`Version conflicts for ${result.body.version_conflicts} alerts`); + } } catch (err) { // eslint-disable-next-line no-console - console.log(`Failure trying to deleteAllAlerts, retries left are: ${retryCount - 1}`, err); + console.log(`Error in deleteAllAlerts(), retries left: ${retryCount - 1}`, err); + + // retry, counting down, and delay a bit before + await new Promise((resolve) => setTimeout(resolve, 250)); await deleteAllAlerts(es, retryCount - 1); } } else { From 5edaccaf4bea03e941c3da8fb4cd4480e528345e Mon Sep 17 00:00:00 2001 From: Patrick Mueller Date: Tue, 29 Sep 2020 14:38:42 -0400 Subject: [PATCH 2/7] add string array consts for execution status and reason values --- x-pack/plugins/alerts/common/alert.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/alerts/common/alert.ts b/x-pack/plugins/alerts/common/alert.ts index 7e5d25b36361c4..5fe71e7c4fbfe6 100644 --- a/x-pack/plugins/alerts/common/alert.ts +++ b/x-pack/plugins/alerts/common/alert.ts @@ -15,8 +15,18 @@ export interface IntervalSchedule extends SavedObjectAttributes { interval: string; } -export type AlertExecutionStatuses = 'ok' | 'active' | 'error' | 'unknown'; -export type AlertExecutionStatusErrorReasons = 'read' | 'decrypt' | 'execute' | 'unknown'; +// for the `typeof ThingValues[number]` types below, become string types that +// only accept the values in the associated string arrays +export const AlertExecutionStatusValues = ['ok', 'active', 'error', 'unknown'] as const; +export type AlertExecutionStatuses = typeof AlertExecutionStatusValues[number]; + +export const AlertExecutionStatusErrorReasonValues = [ + 'read', + 'decrypt', + 'execute', + 'unknown', +] as const; +export type AlertExecutionStatusErrorReasons = typeof AlertExecutionStatusErrorReasonValues[number]; export interface AlertExecutionStatus { status: AlertExecutionStatuses; From 95fcc5a6809e001a51a4a130f21f70c183532831 Mon Sep 17 00:00:00 2001 From: Patrick Mueller Date: Tue, 29 Sep 2020 16:35:56 -0400 Subject: [PATCH 3/7] fix merge conflicts with master --- .../spaces_only/tests/alerting/index.ts | 8 +++- .../spaces_only/tests/index.ts | 42 +++++++------------ 2 files changed, 22 insertions(+), 28 deletions(-) diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/index.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/index.ts index b2a8b31c5fd212..a80970788e517d 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/index.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/index.ts @@ -5,10 +5,14 @@ */ import { FtrProviderContext } from '../../../common/ftr_provider_context'; +import { buildUp, tearDown } from '..'; // eslint-disable-next-line import/no-default-export -export default function alertingTests({ loadTestFile }: FtrProviderContext) { - describe('Alerts', () => { +export default function alertingTests({ loadTestFile, getService }: FtrProviderContext) { + describe('Alerting', () => { + before(async () => buildUp(getService)); + after(async () => tearDown(getService)); + loadTestFile(require.resolve('./create')); loadTestFile(require.resolve('./delete')); loadTestFile(require.resolve('./disable')); diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/index.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/index.ts index 46a6bf2b366e03..49227c2d5cdf8c 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/index.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/index.ts @@ -8,36 +8,26 @@ import { FtrProviderContext } from '../../common/ftr_provider_context'; import { Spaces } from '../scenarios'; // eslint-disable-next-line import/no-default-export -export default function alertingApiIntegrationTests({ - loadTestFile, - getService, -}: FtrProviderContext) { - const spacesService = getService('spaces'); - const esArchiver = getService('esArchiver'); - +export default function alertingApiIntegrationTests({ loadTestFile }: FtrProviderContext) { describe('alerting api integration spaces only', function () { this.tags('ciGroup9'); - before(async () => { - for (const space of Object.values(Spaces)) { - if (space.id === 'default') continue; - - const { id, name, disabledFeatures } = space; - - try { - await spacesService.delete(id); - } catch (err) { - // try deleting before creating, in case it already exists, - // but obviously it's ok if it fails (because it doesn't yet exist) - } - - await spacesService.create({ id, name, disabledFeatures }); - } - }); - - after(async () => await esArchiver.unload('empty_kibana')); - loadTestFile(require.resolve('./actions')); loadTestFile(require.resolve('./alerting')); }); } + +export async function buildUp(getService: FtrProviderContext['getService']) { + const spacesService = getService('spaces'); + for (const space of Object.values(Spaces)) { + if (space.id === 'default') continue; + + const { id, name, disabledFeatures } = space; + await spacesService.create({ id, name, disabledFeatures }); + } +} + +export async function tearDown(getService: FtrProviderContext['getService']) { + const esArchiver = getService('esArchiver'); + await esArchiver.unload('empty_kibana'); +} From c366d4b9c3c727bb86da886661a3a0616379334e Mon Sep 17 00:00:00 2001 From: Patrick Mueller Date: Wed, 30 Sep 2020 12:54:30 -0400 Subject: [PATCH 4/7] fix an issue with a prior rebase --- .../spaces_only/tests/actions/index.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/actions/index.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/actions/index.ts index 75544b7fd41697..007ea7f44a42f1 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/actions/index.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/actions/index.ts @@ -5,10 +5,14 @@ */ import { FtrProviderContext } from '../../../common/ftr_provider_context'; +import { buildUp, tearDown } from '..'; // eslint-disable-next-line import/no-default-export -export default function actionsTests({ loadTestFile }: FtrProviderContext) { +export default function actionsTests({ loadTestFile, getService }: FtrProviderContext) { describe('Actions', () => { + before(async () => buildUp(getService)); + after(async () => tearDown(getService)); + loadTestFile(require.resolve('./create')); loadTestFile(require.resolve('./delete')); loadTestFile(require.resolve('./get_all')); From 860647fc3ab681c800bb4a99c200a37d59028490 Mon Sep 17 00:00:00 2001 From: Patrick Mueller Date: Thu, 1 Oct 2020 09:55:05 -0400 Subject: [PATCH 5/7] change initial/migration status from unknown to waiting --- x-pack/plugins/alerts/common/alert.ts | 2 +- x-pack/plugins/alerts/server/alerts_client.test.ts | 6 +++--- x-pack/plugins/alerts/server/alerts_client.ts | 2 +- .../plugins/alerts/server/saved_objects/migrations.test.ts | 2 +- x-pack/plugins/alerts/server/saved_objects/migrations.ts | 2 +- .../spaces_only/tests/alerting/execution_status.ts | 4 ++-- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/x-pack/plugins/alerts/common/alert.ts b/x-pack/plugins/alerts/common/alert.ts index 5fe71e7c4fbfe6..9566537ca0539c 100644 --- a/x-pack/plugins/alerts/common/alert.ts +++ b/x-pack/plugins/alerts/common/alert.ts @@ -17,7 +17,7 @@ export interface IntervalSchedule extends SavedObjectAttributes { // for the `typeof ThingValues[number]` types below, become string types that // only accept the values in the associated string arrays -export const AlertExecutionStatusValues = ['ok', 'active', 'error', 'unknown'] as const; +export const AlertExecutionStatusValues = ['ok', 'active', 'error', 'waiting', 'unknown'] as const; export type AlertExecutionStatuses = typeof AlertExecutionStatusValues[number]; export const AlertExecutionStatusErrorReasonValues = [ diff --git a/x-pack/plugins/alerts/server/alerts_client.test.ts b/x-pack/plugins/alerts/server/alerts_client.test.ts index 27410e3b579c82..e6941838d0e9ea 100644 --- a/x-pack/plugins/alerts/server/alerts_client.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client.test.ts @@ -396,7 +396,7 @@ describe('create()', () => { "executionStatus": Object { "date": "2019-02-12T21:01:22.479Z", "error": null, - "status": "unknown", + "status": "waiting", }, "meta": Object { "versionApiKeyLastmodified": "v7.10.0", @@ -1036,7 +1036,7 @@ describe('create()', () => { tags: ['foo'], executionStatus: { date: '2019-02-12T21:01:22.479Z', - status: 'unknown', + status: 'waiting', error: null, }, }, @@ -1157,7 +1157,7 @@ describe('create()', () => { tags: ['foo'], executionStatus: { date: '2019-02-12T21:01:22.479Z', - status: 'unknown', + status: 'waiting', error: null, }, }, diff --git a/x-pack/plugins/alerts/server/alerts_client.ts b/x-pack/plugins/alerts/server/alerts_client.ts index 5e7d6e9c5e3bb6..35ff3f5a110a86 100644 --- a/x-pack/plugins/alerts/server/alerts_client.ts +++ b/x-pack/plugins/alerts/server/alerts_client.ts @@ -230,7 +230,7 @@ export class AlertsClient { muteAll: false, mutedInstanceIds: [], executionStatus: { - status: 'unknown', + status: 'waiting', date: new Date().toISOString(), error: null, }, diff --git a/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts b/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts index 5305432f4ce730..99774564bbb6ce 100644 --- a/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts +++ b/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts @@ -217,7 +217,7 @@ describe('7.10.0', () => { ...alert.attributes, executionStatus: { date: migratedAlert.attributes.executionStatus.date, - status: 'unknown', + status: 'waiting', error: null, }, }, diff --git a/x-pack/plugins/alerts/server/saved_objects/migrations.ts b/x-pack/plugins/alerts/server/saved_objects/migrations.ts index 030df9b2ec5a0e..cabab24ce7596e 100644 --- a/x-pack/plugins/alerts/server/saved_objects/migrations.ts +++ b/x-pack/plugins/alerts/server/saved_objects/migrations.ts @@ -123,7 +123,7 @@ function initializeExecutionStatus( attributes: { ...attributes, executionStatus: { - status: 'unknown', + status: 'waiting', date: new Date().toISOString(), error: null, }, diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/execution_status.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/execution_status.ts index 94471576c2242d..466568ffce9e1f 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/execution_status.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/execution_status.ts @@ -24,7 +24,7 @@ export default function executionStatusAlertTests({ getService }: FtrProviderCon after(async () => await objectRemover.removeAll()); - it('should be "unknown" for newly created alert', async () => { + it('should be "waiting" for newly created alert', async () => { const dateStart = Date.now(); const response = await supertest .post(`${getUrlPrefix(Spaces.space1.id)}/api/alerts/alert`) @@ -36,7 +36,7 @@ export default function executionStatusAlertTests({ getService }: FtrProviderCon expect(response.body.executionStatus).to.be.ok(); const { status, date, error } = response.body.executionStatus; - expect(status).to.be('unknown'); + expect(status).to.be('waiting'); ensureDatetimesAreOrdered([dateStart, date, dateEnd]); expect(error).not.to.be.ok(); From 877561028f37b7668e8d98269a469599a0306aa7 Mon Sep 17 00:00:00 2001 From: Patrick Mueller Date: Thu, 1 Oct 2020 14:41:17 -0400 Subject: [PATCH 6/7] changes from PR review comments --- x-pack/plugins/alerts/common/alert.ts | 2 +- .../alerts/server/alerts_client.test.ts | 8 +-- x-pack/plugins/alerts/server/alerts_client.ts | 9 ++- .../server/lib/alert_execution_status.test.ts | 58 +++++++++++++------ .../server/lib/alert_execution_status.ts | 24 +++++--- ...rt_instance_summary_from_event_log.test.ts | 2 +- .../server/lib/is_alert_not_found_error.ts | 9 +-- .../alerts/server/routes/create.test.ts | 2 +- .../plugins/alerts/server/routes/get.test.ts | 2 +- .../server/saved_objects/migrations.test.ts | 6 +- .../alerts/server/saved_objects/migrations.ts | 2 +- .../task_runner/alert_task_instance.test.ts | 2 +- .../server/task_runner/task_runner.test.ts | 2 +- x-pack/plugins/alerts/server/types.ts | 5 +- .../routes/__mocks__/request_responses.ts | 4 +- .../rules/patch_rules.mock.ts | 2 +- .../public/application/lib/alert_api.test.ts | 4 +- .../components/alert_details.test.tsx | 2 +- .../components/alert_details_route.test.tsx | 2 +- .../components/alert_instances.test.tsx | 2 +- .../components/alert_instances_route.test.tsx | 2 +- .../components/view_in_app.test.tsx | 2 +- .../sections/alert_form/alert_edit.test.tsx | 2 +- .../with_bulk_alert_api_operations.test.tsx | 2 +- .../tests/alerting/execution_status.ts | 23 +++----- 25 files changed, 106 insertions(+), 74 deletions(-) diff --git a/x-pack/plugins/alerts/common/alert.ts b/x-pack/plugins/alerts/common/alert.ts index 9566537ca0539c..d9b3b50f32e4e0 100644 --- a/x-pack/plugins/alerts/common/alert.ts +++ b/x-pack/plugins/alerts/common/alert.ts @@ -30,7 +30,7 @@ export type AlertExecutionStatusErrorReasons = typeof AlertExecutionStatusErrorR export interface AlertExecutionStatus { status: AlertExecutionStatuses; - date: Date; + lastExecutionDate: Date; error?: { reason: AlertExecutionStatusErrorReasons; message: string; diff --git a/x-pack/plugins/alerts/server/alerts_client.test.ts b/x-pack/plugins/alerts/server/alerts_client.test.ts index e6941838d0e9ea..caac6e2c03a8d2 100644 --- a/x-pack/plugins/alerts/server/alerts_client.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client.test.ts @@ -394,8 +394,8 @@ describe('create()', () => { "createdBy": "elastic", "enabled": true, "executionStatus": Object { - "date": "2019-02-12T21:01:22.479Z", "error": null, + "lastExecutionDate": "2019-02-12T21:01:22.479Z", "status": "waiting", }, "meta": Object { @@ -1035,7 +1035,7 @@ describe('create()', () => { mutedInstanceIds: [], tags: ['foo'], executionStatus: { - date: '2019-02-12T21:01:22.479Z', + lastExecutionDate: '2019-02-12T21:01:22.479Z', status: 'waiting', error: null, }, @@ -1156,7 +1156,7 @@ describe('create()', () => { mutedInstanceIds: [], tags: ['foo'], executionStatus: { - date: '2019-02-12T21:01:22.479Z', + lastExecutionDate: '2019-02-12T21:01:22.479Z', status: 'waiting', error: null, }, @@ -2513,7 +2513,7 @@ const BaseAlertInstanceSummarySavedObject: SavedObject = { mutedInstanceIds: [], executionStatus: { status: 'unknown', - date: '2020-08-20T19:23:38Z', + lastExecutionDate: '2020-08-20T19:23:38Z', error: null, }, }, diff --git a/x-pack/plugins/alerts/server/alerts_client.ts b/x-pack/plugins/alerts/server/alerts_client.ts index 35ff3f5a110a86..3932e1d6243dc7 100644 --- a/x-pack/plugins/alerts/server/alerts_client.ts +++ b/x-pack/plugins/alerts/server/alerts_client.ts @@ -231,7 +231,7 @@ export class AlertsClient { mutedInstanceIds: [], executionStatus: { status: 'waiting', - date: new Date().toISOString(), + lastExecutionDate: new Date().toISOString(), error: null, }, }; @@ -967,11 +967,16 @@ export class AlertsClient { updatedAt: SavedObject['updated_at'] = createdAt, references: SavedObjectReference[] | undefined ): PartialAlert { + // Not the prettiest code here, but if we want to use most of the + // alert fields from the rawAlert using `...rawAlert` kind of access, we + // need to specifically delete the executionStatus as it's a different type + // in RawAlert and Alert. Probably next time we need to do something similar + // here, we should look at redesigning the implementation of this method. const rawAlertWithoutExecutionStatus: Partial> = { ...rawAlert, }; delete rawAlertWithoutExecutionStatus.executionStatus; - const executionStatus = alertExecutionStatusFromRaw(rawAlert.executionStatus); + const executionStatus = alertExecutionStatusFromRaw(this.logger, id, rawAlert.executionStatus); return { id, ...rawAlertWithoutExecutionStatus, diff --git a/x-pack/plugins/alerts/server/lib/alert_execution_status.test.ts b/x-pack/plugins/alerts/server/lib/alert_execution_status.test.ts index 4d7982bd8b5d96..3372d19cd40901 100644 --- a/x-pack/plugins/alerts/server/lib/alert_execution_status.test.ts +++ b/x-pack/plugins/alerts/server/lib/alert_execution_status.test.ts @@ -4,6 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ +import { loggingSystemMock } from '../../../../../src/core/server/mocks'; import { AlertExecutionStatusErrorReasons } from '../types'; import { executionStatusFromState, @@ -13,25 +14,31 @@ import { } from './alert_execution_status'; import { ErrorWithReason } from './error_with_reason'; +const MockLogger = loggingSystemMock.create().get(); + describe('AlertExecutionStatus', () => { + beforeEach(() => { + jest.resetAllMocks(); + }); + describe('executionStatusFromState()', () => { test('empty task state', () => { const status = executionStatusFromState({}); - checkDateIsNearNow(status.date); + checkDateIsNearNow(status.lastExecutionDate); expect(status.status).toBe('ok'); expect(status.error).toBe(undefined); }); test('task state with no instances', () => { const status = executionStatusFromState({ alertInstances: {} }); - checkDateIsNearNow(status.date); + checkDateIsNearNow(status.lastExecutionDate); expect(status.status).toBe('ok'); expect(status.error).toBe(undefined); }); test('task state with one instance', () => { const status = executionStatusFromState({ alertInstances: { a: {} } }); - checkDateIsNearNow(status.date); + checkDateIsNearNow(status.lastExecutionDate); expect(status.status).toBe('active'); expect(status.error).toBe(undefined); }); @@ -68,23 +75,24 @@ describe('AlertExecutionStatus', () => { const error = { reason, message: 'wops' }; test('status without an error', () => { - expect(alertExecutionStatusToRaw({ date, status })).toMatchInlineSnapshot(` + expect(alertExecutionStatusToRaw({ lastExecutionDate: date, status })).toMatchInlineSnapshot(` Object { - "date": "2020-09-03T16:26:58.000Z", "error": null, + "lastExecutionDate": "2020-09-03T16:26:58.000Z", "status": "ok", } `); }); test('status with an error', () => { - expect(alertExecutionStatusToRaw({ date, status, error })).toMatchInlineSnapshot(` + expect(alertExecutionStatusToRaw({ lastExecutionDate: date, status, error })) + .toMatchInlineSnapshot(` Object { - "date": "2020-09-03T16:26:58.000Z", "error": Object { "message": "wops", "reason": "decrypt", }, + "lastExecutionDate": "2020-09-03T16:26:58.000Z", "status": "ok", } `); @@ -98,56 +106,70 @@ describe('AlertExecutionStatus', () => { const error = { reason, message: 'wops' }; test('no input', () => { - const result = alertExecutionStatusFromRaw(); + const result = alertExecutionStatusFromRaw(MockLogger, 'alert-id'); expect(result).toBe(undefined); }); test('undefined input', () => { - const result = alertExecutionStatusFromRaw(undefined); + const result = alertExecutionStatusFromRaw(MockLogger, 'alert-id', undefined); expect(result).toBe(undefined); }); test('null input', () => { - const result = alertExecutionStatusFromRaw(null); + const result = alertExecutionStatusFromRaw(MockLogger, 'alert-id', null); expect(result).toBe(undefined); }); test('invalid date', () => { - const result = alertExecutionStatusFromRaw({ date: 'an invalid date' })!; - checkDateIsNearNow(result.date); + const result = alertExecutionStatusFromRaw(MockLogger, 'alert-id', { + lastExecutionDate: 'an invalid date', + })!; + checkDateIsNearNow(result.lastExecutionDate); expect(result.status).toBe('unknown'); expect(result.error).toBe(undefined); + expect(MockLogger.debug).toBeCalledWith( + 'invalid alertExecutionStatus lastExecutionDate "an invalid date" in raw alert alert-id' + ); }); test('valid date', () => { - const result = alertExecutionStatusFromRaw({ date }); + const result = alertExecutionStatusFromRaw(MockLogger, 'alert-id', { + lastExecutionDate: date, + }); expect(result).toMatchInlineSnapshot(` Object { - "date": 2020-09-03T16:26:58.000Z, + "lastExecutionDate": 2020-09-03T16:26:58.000Z, "status": "unknown", } `); }); test('valid status and date', () => { - const result = alertExecutionStatusFromRaw({ status, date }); + const result = alertExecutionStatusFromRaw(MockLogger, 'alert-id', { + status, + lastExecutionDate: date, + }); expect(result).toMatchInlineSnapshot(` Object { - "date": 2020-09-03T16:26:58.000Z, + "lastExecutionDate": 2020-09-03T16:26:58.000Z, "status": "active", } `); }); test('valid status, date and error', () => { - const result = alertExecutionStatusFromRaw({ status, date, error }); + const result = alertExecutionStatusFromRaw(MockLogger, 'alert-id', { + status, + lastExecutionDate: date, + error, + }); expect(result).toMatchInlineSnapshot(` Object { - "date": 2020-09-03T16:26:58.000Z, "error": Object { "message": "wops", "reason": "execute", }, + "lastExecutionDate": 2020-09-03T16:26:58.000Z, "status": "active", } `); diff --git a/x-pack/plugins/alerts/server/lib/alert_execution_status.ts b/x-pack/plugins/alerts/server/lib/alert_execution_status.ts index 25d4115acadcb2..9eb0c8817f28c6 100644 --- a/x-pack/plugins/alerts/server/lib/alert_execution_status.ts +++ b/x-pack/plugins/alerts/server/lib/alert_execution_status.ts @@ -4,20 +4,21 @@ * you may not use this file except in compliance with the Elastic License. */ +import { Logger } from 'src/core/server'; import { AlertTaskState, AlertExecutionStatus, RawAlertExecutionStatus } from '../types'; import { getReasonFromError } from './error_with_reason'; export function executionStatusFromState(state: AlertTaskState): AlertExecutionStatus { const instanceIds = Object.keys(state.alertInstances ?? {}); return { - date: new Date(), + lastExecutionDate: new Date(), status: instanceIds.length === 0 ? 'ok' : 'active', }; } export function executionStatusFromError(error: Error): AlertExecutionStatus { return { - date: new Date(), + lastExecutionDate: new Date(), status: 'error', error: { reason: getReasonFromError(error), @@ -27,34 +28,39 @@ export function executionStatusFromError(error: Error): AlertExecutionStatus { } export function alertExecutionStatusToRaw({ - date, + lastExecutionDate, status, error, }: AlertExecutionStatus): RawAlertExecutionStatus { return { - date: date.toISOString(), + lastExecutionDate: lastExecutionDate.toISOString(), status, + // explicitly setting to null (in case undefined) due to partial update concerns error: error ?? null, }; } export function alertExecutionStatusFromRaw( + logger: Logger, + alertId: string, rawAlertExecutionStatus?: Partial | null | undefined ): AlertExecutionStatus | undefined { if (!rawAlertExecutionStatus) return undefined; - const { date, status = 'unknown', error } = rawAlertExecutionStatus; + const { lastExecutionDate, status = 'unknown', error } = rawAlertExecutionStatus; - let parsedDateMillis = date ? Date.parse(date) : Date.now(); + let parsedDateMillis = lastExecutionDate ? Date.parse(lastExecutionDate) : Date.now(); if (isNaN(parsedDateMillis)) { - // TODO: log a message? + logger.debug( + `invalid alertExecutionStatus lastExecutionDate "${lastExecutionDate}" in raw alert ${alertId}` + ); parsedDateMillis = Date.now(); } const parsedDate = new Date(parsedDateMillis); if (error) { - return { date: parsedDate, status, error }; + return { lastExecutionDate: parsedDate, status, error }; } else { - return { date: parsedDate, status }; + return { lastExecutionDate: parsedDate, status }; } } diff --git a/x-pack/plugins/alerts/server/lib/alert_instance_summary_from_event_log.test.ts b/x-pack/plugins/alerts/server/lib/alert_instance_summary_from_event_log.test.ts index f3cb48176d829a..566a1770c0658e 100644 --- a/x-pack/plugins/alerts/server/lib/alert_instance_summary_from_event_log.test.ts +++ b/x-pack/plugins/alerts/server/lib/alert_instance_summary_from_event_log.test.ts @@ -513,6 +513,6 @@ const BaseAlert: SanitizedAlert = { apiKeyOwner: null, executionStatus: { status: 'unknown', - date: new Date('2020-08-20T19:23:38Z'), + lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, }; diff --git a/x-pack/plugins/alerts/server/lib/is_alert_not_found_error.ts b/x-pack/plugins/alerts/server/lib/is_alert_not_found_error.ts index 3ec946495f0335..038e234586688a 100644 --- a/x-pack/plugins/alerts/server/lib/is_alert_not_found_error.ts +++ b/x-pack/plugins/alerts/server/lib/is_alert_not_found_error.ts @@ -9,8 +9,9 @@ import { isErrorWithReason } from './error_with_reason'; export function isAlertSavedObjectNotFoundError(err: Error, alertId: string) { // if this is an error with a reason, the actual error needs to be extracted - if (isErrorWithReason(err)) { - err = err.error; - } - return SavedObjectsErrorHelpers.isNotFoundError(err) && `${err}`.includes(alertId); + const actualError = isErrorWithReason(err) ? err.error : err; + + return ( + SavedObjectsErrorHelpers.isNotFoundError(actualError) && `${actualError}`.includes(alertId) + ); } diff --git a/x-pack/plugins/alerts/server/routes/create.test.ts b/x-pack/plugins/alerts/server/routes/create.test.ts index 5a528819c8d1a1..51c5d2525631d6 100644 --- a/x-pack/plugins/alerts/server/routes/create.test.ts +++ b/x-pack/plugins/alerts/server/routes/create.test.ts @@ -67,7 +67,7 @@ describe('createAlertRoute', () => { ], executionStatus: { status: 'unknown', - date: new Date('2020-08-20T19:23:38Z'), + lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, }; diff --git a/x-pack/plugins/alerts/server/routes/get.test.ts b/x-pack/plugins/alerts/server/routes/get.test.ts index 7bf72aab8792cc..c60177e90b79d8 100644 --- a/x-pack/plugins/alerts/server/routes/get.test.ts +++ b/x-pack/plugins/alerts/server/routes/get.test.ts @@ -54,7 +54,7 @@ describe('getAlertRoute', () => { mutedInstanceIds: [], executionStatus: { status: 'unknown', - date: new Date('2020-08-20T19:23:38Z'), + lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, }; diff --git a/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts b/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts index 99774564bbb6ce..2ae42a7cb61e60 100644 --- a/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts +++ b/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts @@ -206,7 +206,9 @@ describe('7.10.0', () => { const dateStart = Date.now(); const migratedAlert = migration710(alert, { log }); const dateStop = Date.now(); - const dateExecutionStatus = Date.parse(migratedAlert.attributes.executionStatus.date); + const dateExecutionStatus = Date.parse( + migratedAlert.attributes.executionStatus.lastExecutionDate + ); expect(dateStart).toBeLessThanOrEqual(dateExecutionStatus); expect(dateStop).toBeGreaterThanOrEqual(dateExecutionStatus); @@ -216,7 +218,7 @@ describe('7.10.0', () => { attributes: { ...alert.attributes, executionStatus: { - date: migratedAlert.attributes.executionStatus.date, + lastExecutionDate: migratedAlert.attributes.executionStatus.lastExecutionDate, status: 'waiting', error: null, }, diff --git a/x-pack/plugins/alerts/server/saved_objects/migrations.ts b/x-pack/plugins/alerts/server/saved_objects/migrations.ts index cabab24ce7596e..8d611890bad4b6 100644 --- a/x-pack/plugins/alerts/server/saved_objects/migrations.ts +++ b/x-pack/plugins/alerts/server/saved_objects/migrations.ts @@ -124,7 +124,7 @@ function initializeExecutionStatus( ...attributes, executionStatus: { status: 'waiting', - date: new Date().toISOString(), + lastExecutionDate: new Date().toISOString(), error: null, }, }, diff --git a/x-pack/plugins/alerts/server/task_runner/alert_task_instance.test.ts b/x-pack/plugins/alerts/server/task_runner/alert_task_instance.test.ts index 0220f6d81f9a76..cf0dd9d135e275 100644 --- a/x-pack/plugins/alerts/server/task_runner/alert_task_instance.test.ts +++ b/x-pack/plugins/alerts/server/task_runner/alert_task_instance.test.ts @@ -31,7 +31,7 @@ const alert: SanitizedAlert = { mutedInstanceIds: [], executionStatus: { status: 'unknown', - date: new Date('2020-08-20T19:23:38Z'), + lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, }; 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 4f0c8941d6318c..d9af3d0ae6d5bd 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 @@ -109,7 +109,7 @@ describe('Task Runner', () => { ], executionStatus: { status: 'unknown', - date: new Date('2020-08-20T19:23:38Z'), + lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, }; diff --git a/x-pack/plugins/alerts/server/types.ts b/x-pack/plugins/alerts/server/types.ts index 97d37b3e41adb8..03d41724213ce6 100644 --- a/x-pack/plugins/alerts/server/types.ts +++ b/x-pack/plugins/alerts/server/types.ts @@ -117,9 +117,12 @@ export interface AlertMeta extends SavedObjectAttributes { versionApiKeyLastmodified?: string; } +// note that the `error` property is "null-able", as we're doing a partial +// update on the alert when we update this data, but need to ensure we +// delete any previous error if the current status has no error export interface RawAlertExecutionStatus extends SavedObjectAttributes { status: AlertExecutionStatuses; - date: string; + lastExecutionDate: string; error: null | { reason: AlertExecutionStatusErrorReasons; message: string; 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 fdc2d839e27f67..9081831c454978 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 @@ -421,7 +421,7 @@ export const getResult = (): RuleAlertType => ({ scheduledTaskId: '2dabe330-0702-11ea-8b50-773b89126888', executionStatus: { status: 'unknown', - date: new Date('2020-08-20T19:23:38Z'), + lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, }); @@ -636,7 +636,7 @@ export const getNotificationResult = (): RuleNotificationAlertType => ({ updatedAt: new Date('2020-03-21T12:37:08.730Z'), executionStatus: { status: 'unknown', - date: new Date('2020-08-20T19:23:38Z'), + lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/patch_rules.mock.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/patch_rules.mock.ts index 0259a75e3e99d3..8672c85f984269 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/patch_rules.mock.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/patch_rules.mock.ts @@ -112,7 +112,7 @@ const rule: SanitizedAlert = { scheduledTaskId: '2dabe330-0702-11ea-8b50-773b89126888', executionStatus: { status: 'unknown', - date: new Date('2020-08-20T19:23:38Z'), + lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, }; diff --git a/x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api.test.ts b/x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api.test.ts index a7d70a1e8640da..f6cefb77a240e8 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api.test.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api.test.ts @@ -400,7 +400,7 @@ describe('createAlert', () => { mutedInstanceIds: [], executionStatus: { status: 'unknown', - date: new Date('2020-08-20T19:23:38Z'), + lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, }; http.post.mockResolvedValueOnce(resolvedValue); @@ -446,7 +446,7 @@ describe('updateAlert', () => { mutedInstanceIds: [], executionStatus: { status: 'unknown', - date: new Date('2020-08-20T19:23:38Z'), + lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, }; http.put.mockResolvedValueOnce(resolvedValue); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_details.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_details.test.tsx index 6cff022b4beed6..5c9969221cfc35 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_details.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_details.test.tsx @@ -759,7 +759,7 @@ function mockAlert(overloads: Partial = {}): Alert { mutedInstanceIds: [], executionStatus: { status: 'unknown', - date: new Date('2020-08-20T19:23:38Z'), + lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, ...overloads, }; diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_details_route.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_details_route.test.tsx index 390e54441cfe49..5ed924c37fe7a5 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_details_route.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_details_route.test.tsx @@ -406,7 +406,7 @@ function mockAlert(overloads: Partial = {}): Alert { mutedInstanceIds: [], executionStatus: { status: 'unknown', - date: new Date('2020-08-20T19:23:38Z'), + lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, ...overloads, }; diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_instances.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_instances.test.tsx index 60b1e68ba05dc2..2c1020ff1d5b36 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_instances.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_instances.test.tsx @@ -256,7 +256,7 @@ function mockAlert(overloads: Partial = {}): Alert { mutedInstanceIds: [], executionStatus: { status: 'unknown', - date: new Date('2020-08-20T19:23:38Z'), + lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, ...overloads, }; diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_instances_route.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_instances_route.test.tsx index e679166233cfd2..603f06d0bbae44 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_instances_route.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_instances_route.test.tsx @@ -134,7 +134,7 @@ function mockAlert(overloads: Partial = {}): Alert { mutedInstanceIds: [], executionStatus: { status: 'unknown', - date: new Date('2020-08-20T19:23:38Z'), + lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, ...overloads, }; diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/view_in_app.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/view_in_app.test.tsx index 9facb453bae615..7e43fd22ff8c8e 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/view_in_app.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/view_in_app.test.tsx @@ -107,7 +107,7 @@ function mockAlert(overloads: Partial = {}): Alert { mutedInstanceIds: [], executionStatus: { status: 'unknown', - date: new Date('2020-08-20T19:23:38Z'), + lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, ...overloads, }; diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_edit.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_edit.test.tsx index 65c9f92b8c8afd..24eb7aabb95493 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_edit.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_edit.test.tsx @@ -108,7 +108,7 @@ describe('alert_edit', () => { updatedAt: new Date(), executionStatus: { status: 'unknown', - date: new Date('2020-08-20T19:23:38Z'), + lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, }; actionTypeRegistry.get.mockReturnValueOnce(actionTypeModel); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/common/components/with_bulk_alert_api_operations.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/common/components/with_bulk_alert_api_operations.test.tsx index 4f0e02b0f1f6a6..72d4f8857a610a 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/common/components/with_bulk_alert_api_operations.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/common/components/with_bulk_alert_api_operations.test.tsx @@ -266,7 +266,7 @@ function mockAlert(overloads: Partial = {}): Alert { mutedInstanceIds: [], executionStatus: { status: 'unknown', - date: new Date('2020-08-20T19:23:38Z'), + lastExecutionDate: new Date('2020-08-20T19:23:38Z'), }, ...overloads, }; diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/execution_status.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/execution_status.ts index 466568ffce9e1f..62e14b3cfd8117 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/execution_status.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/execution_status.ts @@ -63,12 +63,12 @@ export default function executionStatusAlertTests({ getService }: FtrProviderCon ); expect(response.status).to.eql(200); const alertId = response.body.id; - dates.push(response.body.executionStatus.date); + dates.push(response.body.executionStatus.lastExecutionDate); dates.push(Date.now()); objectRemover.add(Spaces.space1.id, alertId, 'alert', 'alerts'); const executionStatus = await waitForStatus(alertId, new Set(['ok'])); - dates.push(executionStatus.date); + dates.push(executionStatus.lastExecutionDate); dates.push(Date.now()); ensureDatetimesAreOrdered(dates); @@ -98,12 +98,12 @@ export default function executionStatusAlertTests({ getService }: FtrProviderCon ); expect(response.status).to.eql(200); const alertId = response.body.id; - dates.push(response.body.executionStatus.date); + dates.push(response.body.executionStatus.lastExecutionDate); dates.push(Date.now()); objectRemover.add(Spaces.space1.id, alertId, 'alert', 'alerts'); const executionStatus = await waitForStatus(alertId, new Set(['active'])); - dates.push(executionStatus.date); + dates.push(executionStatus.lastExecutionDate); dates.push(Date.now()); ensureDatetimesAreOrdered(dates); @@ -130,12 +130,12 @@ export default function executionStatusAlertTests({ getService }: FtrProviderCon ); expect(response.status).to.eql(200); const alertId = response.body.id; - dates.push(response.body.executionStatus.date); + dates.push(response.body.executionStatus.lastExecutionDate); dates.push(Date.now()); objectRemover.add(Spaces.space1.id, alertId, 'alert', 'alerts'); const executionStatus = await waitForStatus(alertId, new Set(['error'])); - dates.push(executionStatus.date); + dates.push(executionStatus.lastExecutionDate); dates.push(Date.now()); ensureDatetimesAreOrdered(dates); @@ -311,7 +311,7 @@ export default function executionStatusAlertTests({ getService }: FtrProviderCon function expectErrorExecutionStatus(executionStatus: Record, startDate: number) { expect(executionStatus.status).to.equal('error'); - const statusDate = Date.parse(executionStatus.date); + const statusDate = Date.parse(executionStatus.lastExecutionDate); const stopDate = Date.now(); expect(startDate).to.be.lessThan(statusDate); expect(stopDate).to.be.greaterThan(statusDate); @@ -325,14 +325,7 @@ function getFindUri(filter: string) { } function trues(length: number): boolean[] { - return booleans(length, true); -} - -function booleans(length: number, value: boolean): boolean[] { - return '' - .padStart(length) - .split('') - .map((e) => value); + return new Array(length).fill(true); } async function delay(millis: number): Promise { From 3d68a1e36891fdf06f00684b02422d22b8f6e755 Mon Sep 17 00:00:00 2001 From: Patrick Mueller Date: Thu, 1 Oct 2020 16:29:55 -0400 Subject: [PATCH 7/7] change unknown status from waiting to pending, more lastExecutionDate fixes --- x-pack/plugins/alerts/common/alert.ts | 2 +- x-pack/plugins/alerts/server/alerts_client.test.ts | 6 +++--- x-pack/plugins/alerts/server/alerts_client.ts | 2 +- .../plugins/alerts/server/saved_objects/mappings.json | 2 +- .../alerts/server/saved_objects/migrations.test.ts | 2 +- .../plugins/alerts/server/saved_objects/migrations.ts | 2 +- .../spaces_only/tests/alerting/execution_status.ts | 10 +++++----- 7 files changed, 13 insertions(+), 13 deletions(-) diff --git a/x-pack/plugins/alerts/common/alert.ts b/x-pack/plugins/alerts/common/alert.ts index d9b3b50f32e4e0..4ebe66f7b7c9f2 100644 --- a/x-pack/plugins/alerts/common/alert.ts +++ b/x-pack/plugins/alerts/common/alert.ts @@ -17,7 +17,7 @@ export interface IntervalSchedule extends SavedObjectAttributes { // for the `typeof ThingValues[number]` types below, become string types that // only accept the values in the associated string arrays -export const AlertExecutionStatusValues = ['ok', 'active', 'error', 'waiting', 'unknown'] as const; +export const AlertExecutionStatusValues = ['ok', 'active', 'error', 'pending', 'unknown'] as const; export type AlertExecutionStatuses = typeof AlertExecutionStatusValues[number]; export const AlertExecutionStatusErrorReasonValues = [ diff --git a/x-pack/plugins/alerts/server/alerts_client.test.ts b/x-pack/plugins/alerts/server/alerts_client.test.ts index 6e5a9e6d538f9b..59d76ef8bbff09 100644 --- a/x-pack/plugins/alerts/server/alerts_client.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client.test.ts @@ -396,7 +396,7 @@ describe('create()', () => { "executionStatus": Object { "error": null, "lastExecutionDate": "2019-02-12T21:01:22.479Z", - "status": "waiting", + "status": "pending", }, "meta": Object { "versionApiKeyLastmodified": "v7.10.0", @@ -1041,7 +1041,7 @@ describe('create()', () => { tags: ['foo'], executionStatus: { lastExecutionDate: '2019-02-12T21:01:22.479Z', - status: 'waiting', + status: 'pending', error: null, }, }, @@ -1162,7 +1162,7 @@ describe('create()', () => { tags: ['foo'], executionStatus: { lastExecutionDate: '2019-02-12T21:01:22.479Z', - status: 'waiting', + status: 'pending', error: null, }, }, diff --git a/x-pack/plugins/alerts/server/alerts_client.ts b/x-pack/plugins/alerts/server/alerts_client.ts index 82e99db88c789f..bd278d39c62294 100644 --- a/x-pack/plugins/alerts/server/alerts_client.ts +++ b/x-pack/plugins/alerts/server/alerts_client.ts @@ -230,7 +230,7 @@ export class AlertsClient { muteAll: false, mutedInstanceIds: [], executionStatus: { - status: 'waiting', + status: 'pending', lastExecutionDate: new Date().toISOString(), error: null, }, diff --git a/x-pack/plugins/alerts/server/saved_objects/mappings.json b/x-pack/plugins/alerts/server/saved_objects/mappings.json index 5e1ada42a2d45b..a6c92080f18be5 100644 --- a/x-pack/plugins/alerts/server/saved_objects/mappings.json +++ b/x-pack/plugins/alerts/server/saved_objects/mappings.json @@ -89,7 +89,7 @@ "status": { "type": "keyword" }, - "date": { + "lastExecutionDate": { "type": "date" }, "error": { diff --git a/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts b/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts index 2ae42a7cb61e60..8c9d10769b18a1 100644 --- a/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts +++ b/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts @@ -219,7 +219,7 @@ describe('7.10.0', () => { ...alert.attributes, executionStatus: { lastExecutionDate: migratedAlert.attributes.executionStatus.lastExecutionDate, - status: 'waiting', + status: 'pending', error: null, }, }, diff --git a/x-pack/plugins/alerts/server/saved_objects/migrations.ts b/x-pack/plugins/alerts/server/saved_objects/migrations.ts index 8d611890bad4b6..0b2c86b84f67b3 100644 --- a/x-pack/plugins/alerts/server/saved_objects/migrations.ts +++ b/x-pack/plugins/alerts/server/saved_objects/migrations.ts @@ -123,7 +123,7 @@ function initializeExecutionStatus( attributes: { ...attributes, executionStatus: { - status: 'waiting', + status: 'pending', lastExecutionDate: new Date().toISOString(), error: null, }, diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/execution_status.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/execution_status.ts index 62e14b3cfd8117..ac63fe8faadc7c 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/execution_status.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/execution_status.ts @@ -24,7 +24,7 @@ export default function executionStatusAlertTests({ getService }: FtrProviderCon after(async () => await objectRemover.removeAll()); - it('should be "waiting" for newly created alert', async () => { + it('should be "pending" for newly created alert', async () => { const dateStart = Date.now(); const response = await supertest .post(`${getUrlPrefix(Spaces.space1.id)}/api/alerts/alert`) @@ -35,9 +35,9 @@ export default function executionStatusAlertTests({ getService }: FtrProviderCon objectRemover.add(Spaces.space1.id, response.body.id, 'alert', 'alerts'); expect(response.body.executionStatus).to.be.ok(); - const { status, date, error } = response.body.executionStatus; - expect(status).to.be('waiting'); - ensureDatetimesAreOrdered([dateStart, date, dateEnd]); + const { status, lastExecutionDate, error } = response.body.executionStatus; + expect(status).to.be('pending'); + ensureDatetimesAreOrdered([dateStart, lastExecutionDate, dateEnd]); expect(error).not.to.be.ok(); // Ensure AAD isn't broken @@ -226,7 +226,7 @@ export default function executionStatusAlertTests({ getService }: FtrProviderCon await waitForStatus(alertId, new Set(['error'])); - let filter = `date>${startDate}`; + let filter = `lastExecutionDate>${startDate}`; let executionStatus = await waitForFindStatus(alertId, new Set(['error']), filter); expectErrorExecutionStatus(executionStatus, startDate);