diff --git a/x-pack/plugins/alerting/common/rule.ts b/x-pack/plugins/alerting/common/rule.ts index fdd0705629486..d315c923eca79 100644 --- a/x-pack/plugins/alerting/common/rule.ts +++ b/x-pack/plugins/alerting/common/rule.ts @@ -174,6 +174,7 @@ export interface Rule { revision: number; running?: boolean | null; viewInAppRelativeUrl?: string; + typeVersion?: number; } export interface SanitizedAlertsFilter extends AlertsFilter { diff --git a/x-pack/plugins/alerting/server/application/rule/methods/create/create_rule.test.ts b/x-pack/plugins/alerting/server/application/rule/methods/create/create_rule.test.ts index ca2ccc98ea292..cd3192c21d2b7 100644 --- a/x-pack/plugins/alerting/server/application/rule/methods/create/create_rule.test.ts +++ b/x-pack/plugins/alerting/server/application/rule/methods/create/create_rule.test.ts @@ -165,6 +165,7 @@ describe('create()', () => { ownerId: null, }); rulesClientParams.getActionsClient.mockResolvedValue(actionsClient); + ruleTypeRegistry.getLatestRuleVersion.mockReturnValue(1); }); describe('authorization', () => { @@ -500,6 +501,7 @@ describe('create()', () => { "foo", ], "throttle": null, + "typeVersion": 1, "updatedAt": "2019-02-12T21:01:22.479Z", "updatedBy": "elastic", } @@ -728,6 +730,7 @@ describe('create()', () => { "foo", ], "throttle": null, + "typeVersion": 1, "updatedAt": "2019-02-12T21:01:22.479Z", "updatedBy": "elastic", } @@ -1168,6 +1171,7 @@ describe('create()', () => { schedule: { interval: '1m' }, tags: ['foo'], throttle: null, + typeVersion: 1, updatedAt: '2019-02-12T21:01:22.479Z', updatedBy: 'elastic', }, @@ -1427,6 +1431,7 @@ describe('create()', () => { schedule: { interval: '1m' }, tags: ['foo'], throttle: null, + typeVersion: 1, updatedAt: '2019-02-12T21:01:22.479Z', updatedBy: 'elastic', }, @@ -1649,6 +1654,7 @@ describe('create()', () => { schedule: { interval: '1m' }, tags: ['foo'], throttle: null, + typeVersion: 1, updatedAt: '2019-02-12T21:01:22.479Z', updatedBy: 'elastic', }, @@ -1838,6 +1844,7 @@ describe('create()', () => { schedule: { interval: '1m' }, tags: ['foo'], throttle: null, + typeVersion: 1, updatedAt: '2019-02-12T21:01:22.479Z', updatedBy: 'elastic', }, @@ -2005,6 +2012,7 @@ describe('create()', () => { }, schedule: { interval: '1m' }, throttle: '10m', + typeVersion: 1, notifyWhen: 'onActionGroupChange', muteAll: false, snoozeSchedule: [], @@ -2146,6 +2154,7 @@ describe('create()', () => { }, schedule: { interval: '1m' }, throttle: '10m', + typeVersion: 1, notifyWhen: 'onThrottleInterval', muteAll: false, snoozeSchedule: [], @@ -2287,6 +2296,7 @@ describe('create()', () => { }, schedule: { interval: '1m' }, throttle: null, + typeVersion: 1, notifyWhen: null, muteAll: false, snoozeSchedule: [], @@ -2480,6 +2490,7 @@ describe('create()', () => { }, revision: 0, running: false, + typeVersion: 1, }, { references: [ @@ -2837,6 +2848,7 @@ describe('create()', () => { }, schedule: { interval: '1m' }, throttle: null, + typeVersion: 1, notifyWhen: null, muteAll: false, snoozeSchedule: [], @@ -2943,6 +2955,7 @@ describe('create()', () => { }, schedule: { interval: '1m' }, throttle: null, + typeVersion: 1, notifyWhen: null, muteAll: false, snoozeSchedule: [], @@ -3822,6 +3835,7 @@ describe('create()', () => { }, schedule: { interval: '1m' }, throttle: null, + typeVersion: 1, notifyWhen: null, muteAll: false, snoozeSchedule: [], diff --git a/x-pack/plugins/alerting/server/application/rule/methods/create/create_rule.ts b/x-pack/plugins/alerting/server/application/rule/methods/create/create_rule.ts index bdd11da2483f7..eac05024992d7 100644 --- a/x-pack/plugins/alerting/server/application/rule/methods/create/create_rule.ts +++ b/x-pack/plugins/alerting/server/application/rule/methods/create/create_rule.ts @@ -188,6 +188,7 @@ export async function createRule( monitoring: getDefaultMonitoringRuleDomainProperties(lastRunTimestamp.toISOString()), revision: 0, running: false, + typeVersion: context.ruleTypeRegistry.getLatestRuleVersion(), }, { legacyId, diff --git a/x-pack/plugins/alerting/server/application/rule/schemas/rule_schemas.ts b/x-pack/plugins/alerting/server/application/rule/schemas/rule_schemas.ts index ef8f1dc652bff..a80b62a1dcd2e 100644 --- a/x-pack/plugins/alerting/server/application/rule/schemas/rule_schemas.ts +++ b/x-pack/plugins/alerting/server/application/rule/schemas/rule_schemas.ts @@ -168,6 +168,7 @@ export const ruleDomainSchema = schema.object({ revision: schema.number(), running: schema.maybe(schema.nullable(schema.boolean())), viewInAppRelativeUrl: schema.maybe(schema.nullable(schema.string())), + typeVersion: schema.maybe(schema.number()), }); /** diff --git a/x-pack/plugins/alerting/server/application/rule/transforms/transform_rule_attributes_to_rule_domain.ts b/x-pack/plugins/alerting/server/application/rule/transforms/transform_rule_attributes_to_rule_domain.ts index 26831b9dff81c..b475f8f93a38f 100644 --- a/x-pack/plugins/alerting/server/application/rule/transforms/transform_rule_attributes_to_rule_domain.ts +++ b/x-pack/plugins/alerting/server/application/rule/transforms/transform_rule_attributes_to_rule_domain.ts @@ -216,6 +216,7 @@ export const transformRuleAttributesToRuleDomain = { revision: RuleDomainSchemaType['revision']; running?: RuleDomainSchemaType['running']; viewInAppRelativeUrl?: RuleDomainSchemaType['viewInAppRelativeUrl']; + typeVersion?: RuleDomainSchemaType['typeVersion']; } diff --git a/x-pack/plugins/alerting/server/data/rule/types/rule_attributes.ts b/x-pack/plugins/alerting/server/data/rule/types/rule_attributes.ts index 19a669e6bd33e..79230019b6379 100644 --- a/x-pack/plugins/alerting/server/data/rule/types/rule_attributes.ts +++ b/x-pack/plugins/alerting/server/data/rule/types/rule_attributes.ts @@ -174,4 +174,5 @@ export interface RuleAttributes { nextRun?: string | null; revision: number; running?: boolean | null; + typeVersion?: number; } diff --git a/x-pack/plugins/alerting/server/rule_type_registry.ts b/x-pack/plugins/alerting/server/rule_type_registry.ts index 2ed57e878291a..4ab4f1369fbc0 100644 --- a/x-pack/plugins/alerting/server/rule_type_registry.ts +++ b/x-pack/plugins/alerting/server/rule_type_registry.ts @@ -39,7 +39,6 @@ import { AlertingRulesConfig } from '.'; import { AlertsService } from './alerts_service/alerts_service'; import { getRuleTypeIdValidLegacyConsumers } from './rule_type_registry_deprecated_consumers'; import { AlertingConfig } from './config'; -import { rawRuleSchemaV1 } from './saved_objects/schemas/raw_rule'; export interface ConstructorOptions { config: AlertingConfig; @@ -315,7 +314,8 @@ export class RuleTypeRegistry { spaceId: schema.string(), consumer: schema.maybe(schema.string()), }), - indirectParamsSchema: rawRuleSchemaV1, + indirectParamsSchema: ruleType.validate.params, + latestTypeVersion: this.getLatestRuleVersion(), }, }); diff --git a/x-pack/plugins/alerting/server/rules_client/lib/get_alert_from_raw.ts b/x-pack/plugins/alerting/server/rules_client/lib/get_alert_from_raw.ts index ddf1e40728b28..117448b53f958 100644 --- a/x-pack/plugins/alerting/server/rules_client/lib/get_alert_from_raw.ts +++ b/x-pack/plugins/alerting/server/rules_client/lib/get_alert_from_raw.ts @@ -93,6 +93,7 @@ export function getPartialRuleFromRaw( actions, snoozeSchedule, lastRun, + typeVersion, ...partialRawRule }: Partial, references: SavedObjectReference[] | undefined, diff --git a/x-pack/plugins/alerting/server/rules_client/methods/update.ts b/x-pack/plugins/alerting/server/rules_client/methods/update.ts index 8fb0de5f3519c..3e8cb274ac43d 100644 --- a/x-pack/plugins/alerting/server/rules_client/methods/update.ts +++ b/x-pack/plugins/alerting/server/rules_client/methods/update.ts @@ -275,6 +275,7 @@ async function updateAlert( revision, updatedBy: username, updatedAt: new Date().toISOString(), + typeVersion: context.ruleTypeRegistry.getLatestRuleVersion(), }); const mappedParams = getMappedParams(updatedParams); diff --git a/x-pack/plugins/alerting/server/rules_client/rules_client.ts b/x-pack/plugins/alerting/server/rules_client/rules_client.ts index 0a2da42d7e424..369937f484ed8 100644 --- a/x-pack/plugins/alerting/server/rules_client/rules_client.ts +++ b/x-pack/plugins/alerting/server/rules_client/rules_client.ts @@ -80,6 +80,7 @@ const fieldsToExcludeFromPublicApi: Array = [ 'mapped_params', 'snoozeSchedule', 'activeSnoozes', + 'typeVersion', ]; export const fieldsToExcludeFromRevisionUpdates: ReadonlySet = new Set([ diff --git a/x-pack/plugins/alerting/server/rules_client/tests/update.test.ts b/x-pack/plugins/alerting/server/rules_client/tests/update.test.ts index aa03a71d93e14..77a38cfdc515f 100644 --- a/x-pack/plugins/alerting/server/rules_client/tests/update.test.ts +++ b/x-pack/plugins/alerting/server/rules_client/tests/update.test.ts @@ -192,6 +192,7 @@ describe('update()', () => { }, validLegacyConsumers: [], }); + ruleTypeRegistry.getLatestRuleVersion.mockReturnValue(1); (migrateLegacyActions as jest.Mock).mockResolvedValue({ hasLegacyActions: false, resultedActions: [], @@ -449,6 +450,7 @@ describe('update()', () => { "foo", ], "throttle": null, + "typeVersion": 1, "updatedAt": "2019-02-12T21:01:22.479Z", "updatedBy": "elastic", } @@ -679,6 +681,7 @@ describe('update()', () => { scheduledTaskId: 'task-123', tags: ['foo'], throttle: null, + typeVersion: 1, updatedAt: '2019-02-12T21:01:22.479Z', updatedBy: 'elastic', }, @@ -923,6 +926,7 @@ describe('update()', () => { scheduledTaskId: 'task-123', tags: ['foo'], throttle: null, + typeVersion: 1, updatedAt: '2019-02-12T21:01:22.479Z', updatedBy: 'elastic', }, @@ -1116,6 +1120,7 @@ describe('update()', () => { scheduledTaskId: 'task-123', tags: ['foo'], throttle: null, + typeVersion: 1, updatedAt: '2019-02-12T21:01:22.479Z', updatedBy: 'elastic', }, @@ -1304,6 +1309,7 @@ describe('update()', () => { "foo", ], "throttle": "5m", + "typeVersion": 1, "updatedAt": "2019-02-12T21:01:22.479Z", "updatedBy": "elastic", } @@ -1457,6 +1463,7 @@ describe('update()', () => { "foo", ], "throttle": "5m", + "typeVersion": 1, "updatedAt": "2019-02-12T21:01:22.479Z", "updatedBy": "elastic", } @@ -2477,6 +2484,7 @@ describe('update()', () => { scheduledTaskId: 'task-123', tags: ['foo'], throttle: null, + typeVersion: 1, updatedAt: '2019-02-12T21:01:22.479Z', updatedBy: 'elastic', }, @@ -3034,6 +3042,7 @@ describe('update()', () => { tags: ['foo'], updatedAt: '2019-02-12T21:01:22.479Z', updatedBy: 'elastic', + typeVersion: 1, }, { id: '1', @@ -3244,6 +3253,7 @@ describe('update()', () => { "foo", ], "throttle": "5m", + "typeVersion": 1, "updatedAt": "2019-02-12T21:01:22.479Z", "updatedBy": "elastic", } diff --git a/x-pack/plugins/alerting/server/saved_objects/index.ts b/x-pack/plugins/alerting/server/saved_objects/index.ts index 385a5dd25d6bf..fc65ef445a283 100644 --- a/x-pack/plugins/alerting/server/saved_objects/index.ts +++ b/x-pack/plugins/alerting/server/saved_objects/index.ts @@ -51,6 +51,7 @@ export const RuleAttributesExcludedFromAAD = [ 'nextRun', 'revision', 'running', + 'typeVersion', ]; // useful for Pick which is a @@ -71,7 +72,8 @@ export type RuleAttributesExcludedFromAADType = | 'lastRun' | 'nextRun' | 'revision' - | 'running'; + | 'running' + | 'typeVersion'; export function setupSavedObjects( savedObjects: SavedObjectsServiceSetup, diff --git a/x-pack/plugins/alerting/server/saved_objects/rule_model_versions.test.ts b/x-pack/plugins/alerting/server/saved_objects/rule_model_versions.test.ts index 9afcdaad8e2f4..481dbc9a210f4 100644 --- a/x-pack/plugins/alerting/server/saved_objects/rule_model_versions.test.ts +++ b/x-pack/plugins/alerting/server/saved_objects/rule_model_versions.test.ts @@ -14,7 +14,7 @@ import { schema } from '@kbn/config-schema'; import { RawRule } from '../types'; describe('rule model versions', () => { - const ruleModelVersions: CustomSavedObjectsModelVersionMap = { + const modelVersions: CustomSavedObjectsModelVersionMap = { '1': { changes: [], schemas: { @@ -58,14 +58,22 @@ describe('rule model versions', () => { describe('getMinimumCompatibleVersion', () => { it('should return the minimum compatible version for the matching rawRule', () => { - expect(getMinimumCompatibleVersion(ruleModelVersions, 1, rawRule)).toBe(1); - expect(getMinimumCompatibleVersion(ruleModelVersions, 2, rawRule)).toBe(2); - expect(getMinimumCompatibleVersion(ruleModelVersions, 3, rawRule)).toBe(2); - expect(getMinimumCompatibleVersion(ruleModelVersions, 4, rawRule)).toBe(2); + expect(getMinimumCompatibleVersion({ modelVersions, version: 1, rawRule })).toBe(1); + expect(getMinimumCompatibleVersion({ modelVersions, version: 2, rawRule })).toBe(2); + expect(getMinimumCompatibleVersion({ modelVersions, version: 3, rawRule })).toBe(2); + expect(getMinimumCompatibleVersion({ modelVersions, version: 4, rawRule })).toBe(2); }); it('should return the minimum compatible version for the mismatching rawRule', () => { - expect(getMinimumCompatibleVersion(ruleModelVersions, 3, mismatchingRawRule)).toBe(3); - expect(getMinimumCompatibleVersion(ruleModelVersions, 4, mismatchingRawRule)).toBe(4); + expect( + getMinimumCompatibleVersion({ modelVersions, version: 3, rawRule: mismatchingRawRule }) + ).toBe(3); + expect( + getMinimumCompatibleVersion({ modelVersions, version: 4, rawRule: mismatchingRawRule }) + ).toBe(4); + }); + + it('should return the given version number if the number does not exist in ruleModelVersions', () => { + expect(getMinimumCompatibleVersion({ version: 999, rawRule: mismatchingRawRule })).toBe(999); }); }); diff --git a/x-pack/plugins/alerting/server/saved_objects/rule_model_versions.ts b/x-pack/plugins/alerting/server/saved_objects/rule_model_versions.ts index 38adc17389b23..2e08b07f97b0e 100644 --- a/x-pack/plugins/alerting/server/saved_objects/rule_model_versions.ts +++ b/x-pack/plugins/alerting/server/saved_objects/rule_model_versions.ts @@ -32,17 +32,25 @@ export const ruleModelVersions: CustomSavedObjectsModelVersionMap = { export const getLatestRuleVersion = () => Math.max(...Object.keys(ruleModelVersions).map(Number)); -export function getMinimumCompatibleVersion( - modelVersions: CustomSavedObjectsModelVersionMap, - version: number, - rawRule: RawRule -): number { +export function getMinimumCompatibleVersion({ + modelVersions = ruleModelVersions, + version = getLatestRuleVersion(), + rawRule, +}: { + modelVersions?: CustomSavedObjectsModelVersionMap; + version?: number; + rawRule: RawRule; +}): number { if (version === 1) { return 1; } - if (modelVersions[version].isCompatibleWithPreviousVersion(rawRule)) { - return getMinimumCompatibleVersion(modelVersions, version - 1, rawRule); + if (modelVersions[version] && modelVersions[version].isCompatibleWithPreviousVersion(rawRule)) { + return getMinimumCompatibleVersion({ + modelVersions, + version: version - 1, + rawRule, + }); } return version; diff --git a/x-pack/plugins/alerting/server/task_runner/fixtures.ts b/x-pack/plugins/alerting/server/task_runner/fixtures.ts index b975f21304013..f1877288c725d 100644 --- a/x-pack/plugins/alerting/server/task_runner/fixtures.ts +++ b/x-pack/plugins/alerting/server/task_runner/fixtures.ts @@ -127,6 +127,7 @@ export const generateSavedObjectParams = ({ }, nextRun, running: false, + typeVersion: 1, }, { refresh: false, namespace: undefined }, ]; @@ -359,26 +360,6 @@ export const generateRunnerResult = ({ taskRunError, }: GeneratorParams = {}) => { return { - monitoring: { - run: { - calculated_metrics: { - success_ratio: successRatio, - }, - // @ts-ignore - history: history.map((success) => ({ success, timestamp: 0 })), - last_run: { - metrics: { - duration: 0, - gap_duration_s: null, - total_alerts_created: null, - total_alerts_detected: null, - total_indexing_duration_ms: null, - total_search_duration_ms: null, - }, - timestamp: '1970-01-01T00:00:00.000Z', - }, - }, - }, schedule: { interval, }, diff --git a/x-pack/plugins/alerting/server/task_runner/rule_loader.test.ts b/x-pack/plugins/alerting/server/task_runner/rule_loader.test.ts index 380d436c95e65..cdc31f338d513 100644 --- a/x-pack/plugins/alerting/server/task_runner/rule_loader.test.ts +++ b/x-pack/plugins/alerting/server/task_runner/rule_loader.test.ts @@ -13,7 +13,7 @@ import { getRuleAttributes, getFakeKibanaRequest, validateRule } from './rule_lo import { TaskRunnerContext } from './task_runner_factory'; import { ruleTypeRegistryMock } from '../rule_type_registry.mock'; import { rulesClientMock } from '../rules_client.mock'; -import { Rule } from '../types'; +import { Rule, RuleTypeParams, RuleTypeParamsValidator } from '../types'; import { MONITORING_HISTORY_LIMIT, RuleExecutionStatusErrorReasons } from '../../common'; import { ErrorWithReason, getReasonFromError } from '../lib/error_with_reason'; import { alertingEventLoggerMock } from '../lib/alerting_event_logger/alerting_event_logger.mock'; @@ -43,7 +43,7 @@ describe('rule_loader', () => { const paramValidator = schema.object({ bar: schema.boolean(), - }); + }) as RuleTypeParamsValidator; const getDefaultValidateRuleParams = ({ fakeRequest, @@ -65,11 +65,14 @@ describe('rule_loader', () => { ? { error } : { data: { - indirectParams: { ...mockedRawRuleSO.attributes, enabled: ruleEnabled }, - rule: { ...mockedRule, params }, + indirectParams: mockedRawRuleSO.attributes.params, + rule: { ...mockedRule, params, enabled: ruleEnabled }, rulesClient, version: '1', fakeRequest, + apiKey: mockedRawRuleSO.attributes.apiKey, + typeVersion: 1, + latestTypeVersion: 1, }, }, }); @@ -107,7 +110,7 @@ describe('rule_loader', () => { expect(result.rule.alertTypeId).toBe(ruleTypeId); expect(result.rule.name).toBe(ruleName); expect(result.rule.params).toBe(ruleParams); - expect(result.indirectParams).toEqual(mockedRawRuleSO.attributes); + expect(result.indirectParams).toEqual(mockedRawRuleSO.attributes.params); expect(result.version).toBe('1'); expect(result.rulesClient).toBe(rulesClient); }); @@ -190,12 +193,7 @@ describe('rule_loader', () => { expect(result.fakeRequest).toEqual(expect.any(CoreKibanaRequest)); expect(result.rule.alertTypeId).toBe(ruleTypeId); - expect(result.indirectParams).toEqual({ - ...mockedRawRuleSO.attributes, - apiKey, - enabled, - consumer, - }); + expect(result.indirectParams).toEqual(mockedRawRuleSO.attributes.params); expect(result.rulesClient).toBeTruthy(); expect(contextMock.spaceIdToNamespace.mock.calls[0]).toEqual(['default']); @@ -211,12 +209,7 @@ describe('rule_loader', () => { expect(result.rule.alertTypeId).toBe(ruleTypeId); expect(result.rulesClient).toBeTruthy(); expect(contextMock.spaceIdToNamespace.mock.calls[0]).toEqual([spaceId]); - expect(result.indirectParams).toEqual({ - ...mockedRawRuleSO.attributes, - apiKey, - enabled, - consumer, - }); + expect(result.indirectParams).toEqual(mockedRawRuleSO.attributes.params); const esoArgs = encryptedSavedObjects.getDecryptedAsInternalUser.mock.calls[0]; expect(esoArgs).toEqual([RULE_SAVED_OBJECT_TYPE, ruleId, { namespace: spaceId }]); diff --git a/x-pack/plugins/alerting/server/task_runner/rule_loader.ts b/x-pack/plugins/alerting/server/task_runner/rule_loader.ts index fb037b802ec9b..a5348b1938142 100644 --- a/x-pack/plugins/alerting/server/task_runner/rule_loader.ts +++ b/x-pack/plugins/alerting/server/task_runner/rule_loader.ts @@ -25,13 +25,16 @@ import { import { MONITORING_HISTORY_LIMIT, RuleTypeParams } from '../../common'; import { AlertingEventLogger } from '../lib/alerting_event_logger/alerting_event_logger'; import { RULE_SAVED_OBJECT_TYPE } from '../saved_objects'; +import { getMinimumCompatibleVersion } from '../saved_objects'; -export interface RuleData extends LoadedIndirectParams { - indirectParams: RawRule; +export interface RuleData extends LoadedIndirectParams { + indirectParams: Params; rule: SanitizedRule; version: string | undefined; + typeVersion: number; fakeRequest: CoreKibanaRequest; rulesClient: RulesClientApi; + apiKey: string | null; } export type RuleDataResult = LoadIndirectParamsResult; @@ -60,16 +63,14 @@ export function validateRule( const { ruleData: { - data: { indirectParams, rule, fakeRequest, rulesClient, version }, + data: { indirectParams, rule, fakeRequest, rulesClient, version, apiKey, typeVersion }, }, ruleTypeRegistry, paramValidator, alertingEventLogger, } = params; - const { enabled, apiKey } = indirectParams; - - if (!enabled) { + if (!rule.enabled) { throw new ErrorWithReason( RuleExecutionStatusErrorReasons.Disabled, new Error(`Rule failed to execute because rule ran after it was disabled.`) @@ -104,6 +105,7 @@ export function validateRule( rulesClient, validatedParams, version, + typeVersion, }; } @@ -131,12 +133,19 @@ export async function getRuleAttributes( omitGeneratedValues: false, }); + const typeVersion = getMinimumCompatibleVersion({ + version: rawRule.attributes.typeVersion, + rawRule: rawRule.attributes, + }); + return { rule, version: rawRule.version, - indirectParams: rawRule.attributes, + indirectParams: rule.params, fakeRequest, rulesClient, + apiKey: rawRule.attributes.apiKey, + typeVersion, }; } diff --git a/x-pack/plugins/alerting/server/task_runner/task_runner.test.ts b/x-pack/plugins/alerting/server/task_runner/task_runner.test.ts index 23d5d5f576ce3..b51190b7b14bf 100644 --- a/x-pack/plugins/alerting/server/task_runner/task_runner.test.ts +++ b/x-pack/plugins/alerting/server/task_runner/task_runner.test.ts @@ -82,6 +82,7 @@ import { alertsServiceMock } from '../alerts_service/alerts_service.mock'; import { getMockMaintenanceWindow } from '../data/maintenance_window/test_helpers'; import { alertsClientMock } from '../alerts_client/alerts_client.mock'; import { MaintenanceWindow } from '../application/maintenance_window/types'; +import { getLatestRuleVersion } from '../saved_objects'; import { RULE_SAVED_OBJECT_TYPE } from '../saved_objects'; jest.mock('uuid', () => ({ @@ -221,6 +222,7 @@ describe('Task Runner', () => { (actionTypeId, actionId, params) => params ); ruleTypeRegistry.get.mockReturnValue(ruleType); + ruleTypeRegistry.getLatestRuleVersion.mockReturnValue(getLatestRuleVersion()); taskRunnerFactoryInitializerParams.executionContext.withContext.mockImplementation((ctx, fn) => fn() ); @@ -2815,26 +2817,6 @@ describe('Task Runner', () => { ); }); - test('caps monitoring history at 200', async () => { - const taskRunner = new TaskRunner({ - ruleType, - taskInstance: mockedTaskInstance, - - context: taskRunnerFactoryInitializerParams, - inMemoryMetrics, - }); - expect(AlertingEventLogger).toHaveBeenCalled(); - - rulesClient.getAlertFromRaw.mockReturnValue(mockedRuleTypeSavedObject as Rule); - encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValue(mockedRawRuleSO); - - for (let i = 0; i < 300; i++) { - await taskRunner.run(); - } - const runnerResult = await taskRunner.run(); - expect(runnerResult.monitoring?.run.history.length).toBe(200); - }); - test('Actions circuit breaker kicked in, should set status as warning and log a message in event log', async () => { const actionsConfigMap = { default: { @@ -3285,6 +3267,7 @@ describe('Task Runner', () => { test('loadIndirectParams Fetches the ruleData and returns the indirectParams', async () => { encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValue(mockedRawRuleSO); + rulesClient.getAlertFromRaw.mockReturnValue(mockedRuleTypeSavedObject as Rule); const taskRunner = new TaskRunner({ ruleType, taskInstance: { @@ -3302,7 +3285,7 @@ describe('Task Runner', () => { expect(encryptedSavedObjectsClient.getDecryptedAsInternalUser).toHaveBeenCalledTimes(1); expect(result).toEqual({ - data: expect.objectContaining({ indirectParams: mockedRawRuleSO.attributes }), + data: expect.objectContaining({ indirectParams: mockedRawRuleSO.attributes.params }), }); }); diff --git a/x-pack/plugins/alerting/server/task_runner/task_runner.ts b/x-pack/plugins/alerting/server/task_runner/task_runner.ts index 532dd7b1e12ba..bb9b68775e8fb 100644 --- a/x-pack/plugins/alerting/server/task_runner/task_runner.ts +++ b/x-pack/plugins/alerting/server/task_runner/task_runner.ts @@ -208,6 +208,7 @@ export class TaskRunner< monitoring?: RawRuleMonitoring; nextRun?: string | null; lastRun?: RawRuleLastRun | null; + typeVersion: number; } ) { const client = this.context.internalSavedObjectsRepository; @@ -822,6 +823,7 @@ export class TaskRunner< nextRun, lastRun: lastRunToRaw(lastRun), monitoring: this.ruleMonitoring.getMonitoring() as RawRuleMonitoring, + typeVersion: this.context.ruleTypeRegistry.getLatestRuleVersion(), }); } @@ -976,7 +978,6 @@ export class TaskRunner< return { interval: retryInterval }; }), - monitoring: this.ruleMonitoring.getMonitoring(), ...(isErr(schedule) ? { taskRunError: createTaskRunError(schedule.error, TaskErrorSource.FRAMEWORK) } : {}), @@ -1048,6 +1049,7 @@ export class TaskRunner< }, monitoring: this.ruleMonitoring.getMonitoring() as RawRuleMonitoring, nextRun: nextRun && new Date(nextRun).getTime() > date.getTime() ? nextRun : null, + typeVersion: this.context.ruleTypeRegistry.getLatestRuleVersion(), }); } } diff --git a/x-pack/plugins/alerting/server/task_runner/task_runner_alerts_client.test.ts b/x-pack/plugins/alerting/server/task_runner/task_runner_alerts_client.test.ts index 2cf4174d7bb28..37e9468832003 100644 --- a/x-pack/plugins/alerting/server/task_runner/task_runner_alerts_client.test.ts +++ b/x-pack/plugins/alerting/server/task_runner/task_runner_alerts_client.test.ts @@ -97,6 +97,7 @@ import { TAGS, VERSION, } from '@kbn/rule-data-utils'; +import { getLatestRuleVersion } from '../saved_objects'; jest.mock('uuid', () => ({ v4: () => '5f6aa57d-3e22-484e-bae8-cbed868f4d28', @@ -236,6 +237,7 @@ describe('Task Runner', () => { (actionTypeId, actionId, params) => params ); ruleTypeRegistry.get.mockReturnValue(ruleTypeWithAlerts); + ruleTypeRegistry.getLatestRuleVersion.mockReturnValue(getLatestRuleVersion()); taskRunnerFactoryInitializerParams.executionContext.withContext.mockImplementation( (ctx, fn) => fn() ); diff --git a/x-pack/plugins/alerting/server/task_runner/task_runner_cancel.test.ts b/x-pack/plugins/alerting/server/task_runner/task_runner_cancel.test.ts index 0ce2f758ade39..6689a53fcf068 100644 --- a/x-pack/plugins/alerting/server/task_runner/task_runner_cancel.test.ts +++ b/x-pack/plugins/alerting/server/task_runner/task_runner_cancel.test.ts @@ -175,6 +175,7 @@ describe('Task Runner Cancel', () => { (actionTypeId, actionId, params) => params ); ruleTypeRegistry.get.mockReturnValue(ruleType); + ruleTypeRegistry.getLatestRuleVersion.mockReturnValue(1); taskRunnerFactoryInitializerParams.executionContext.withContext.mockImplementation((ctx, fn) => fn() ); @@ -266,6 +267,7 @@ describe('Task Runner Cancel', () => { }, nextRun: '1970-01-01T00:00:10.000Z', running: false, + typeVersion: 1, }, { refresh: false, namespace: undefined } ); diff --git a/x-pack/plugins/alerting/server/task_runner/types.ts b/x-pack/plugins/alerting/server/task_runner/types.ts index 33417bcfe9c44..eb71e64a6e9f6 100644 --- a/x-pack/plugins/alerting/server/task_runner/types.ts +++ b/x-pack/plugins/alerting/server/task_runner/types.ts @@ -17,7 +17,6 @@ import { AlertInstanceState, RuleTypeParams, IntervalSchedule, - RuleMonitoring, RuleTaskState, SanitizedRule, RuleTypeState, @@ -31,7 +30,6 @@ import { AlertingEventLogger } from '../lib/alerting_event_logger/alerting_event export interface RuleTaskRunResult { state: RuleTaskState; - monitoring: RuleMonitoring | undefined; schedule: IntervalSchedule | undefined; taskRunError?: DecoratedError; } diff --git a/x-pack/plugins/alerting/server/types.ts b/x-pack/plugins/alerting/server/types.ts index 4867adaf9b9a0..63cf82242f364 100644 --- a/x-pack/plugins/alerting/server/types.ts +++ b/x-pack/plugins/alerting/server/types.ts @@ -479,6 +479,7 @@ export interface RawRule extends SavedObjectAttributes { nextRun?: string | null; revision: number; running?: boolean | null; + typeVersion?: number; } export type { DataStreamAdapter } from './alerts_service/lib/data_stream_adapter'; diff --git a/x-pack/plugins/task_manager/server/task.ts b/x-pack/plugins/task_manager/server/task.ts index c71f8b42185ca..27fc87e7b72b6 100644 --- a/x-pack/plugins/task_manager/server/task.ts +++ b/x-pack/plugins/task_manager/server/task.ts @@ -96,6 +96,7 @@ export interface LoadedIndirectParams< > { [key: string]: unknown; indirectParams: IndirectParams; + typeVersion?: number; } export type LoadIndirectParamsResult = @@ -176,6 +177,7 @@ export const taskDefinitionSchema = schema.object( paramsSchema: schema.maybe(schema.any()), // schema of the data fetched by the task runner (in loadIndirectParams) e.g. rule, action etc. indirectParamsSchema: schema.maybe(schema.any()), + latestTypeVersion: schema.maybe(schema.number()), }, { validate({ timeout }) { diff --git a/x-pack/plugins/task_manager/server/task_running/task_runner.ts b/x-pack/plugins/task_manager/server/task_running/task_runner.ts index f67d8a22db81d..cd891f5c81a67 100644 --- a/x-pack/plugins/task_manager/server/task_running/task_runner.ts +++ b/x-pack/plugins/task_manager/server/task_running/task_runner.ts @@ -341,15 +341,9 @@ export class TaskManagerRunner implements TaskRunner { description: 'run task', }; - let taskParamsValidation; - if (this.requeueInvalidTasksConfig.enabled) { - taskParamsValidation = this.validateTaskParams(modifiedContext); - if (!taskParamsValidation.error) { - taskParamsValidation = await this.validateIndirectTaskParams(modifiedContext); - } - } + const taskValidationResult = await this.validateTask(modifiedContext); - const hasSkipError = !isUndefined(taskParamsValidation?.error); + const hasSkipError = !isUndefined(taskValidationResult?.error); let shouldSkip = false; let shouldKeepSkipAttempts = false; @@ -360,7 +354,7 @@ export class TaskManagerRunner implements TaskRunner { } const result = shouldSkip - ? taskParamsValidation + ? taskValidationResult : await this.executionContext.withContext(ctx, () => withSpan({ name: 'run', type: 'task manager' }, () => this.task!.run()) ); @@ -391,23 +385,6 @@ export class TaskManagerRunner implements TaskRunner { } } - private validateTaskParams({ taskInstance }: RunContext) { - let error; - const { state, taskType, params, id } = taskInstance; - - try { - const paramsSchema = this.definition.paramsSchema; - if (paramsSchema) { - paramsSchema.validate(params); - } - } catch (err) { - this.logger.warn(`Task (${taskType}/${id}) has a validation error: ${err.message}`); - error = createSkipError(err); - } - - return { ...(error ? { error } : {}), state }; - } - private validateTaskState(taskInstance: ConcreteTaskInstance) { const { taskType, id } = taskInstance; try { @@ -419,28 +396,54 @@ export class TaskManagerRunner implements TaskRunner { } } - private async validateIndirectTaskParams({ taskInstance }: RunContext) { - let error; - const { state, taskType, id } = taskInstance; - const indirectParamsSchema = this.definition.indirectParamsSchema; + private async validateTask({ taskInstance }: RunContext) { + const { state, taskType, params, id } = taskInstance; - if (this.task?.loadIndirectParams && !!indirectParamsSchema) { + if (!this.requeueInvalidTasksConfig.enabled) { + return { state }; + } + const { paramsSchema, indirectParamsSchema, latestTypeVersion } = this.definition; + + // validate task params + if (paramsSchema) { + try { + paramsSchema.validate(params); + } catch (err) { + this.logger.warn(`Task (${taskType}/${id}) has a validation error: ${err.message}`); + return { error: createSkipError(err), state }; + } + } + + if (this.task?.loadIndirectParams) { const { data } = await this.task.loadIndirectParams(); - if (data) { + const indirectParams = data?.indirectParams; + const typeVersion = data?.typeVersion; + + // validate runtime version + if (typeVersion && latestTypeVersion && typeVersion > latestTypeVersion) { + this.logger.warn( + `Task (${taskType}/${id}) has a newer version(${typeVersion}) than expected((${latestTypeVersion}))` + ); + return { + error: createSkipError(new Error('Task has already been run by a newer Kibana version')), + state, + }; + } + + // validate indirect params e.g. connector or rule.params + if (indirectParamsSchema && indirectParams) { try { - if (indirectParamsSchema) { - indirectParamsSchema.validate(data.indirectParams); - } + indirectParamsSchema.validate(indirectParams); } catch (err) { this.logger.warn( `Task (${taskType}/${id}) has a validation error in its indirect params: ${err.message}` ); - error = createSkipError(err); + return { error: createSkipError(err), state }; } } } - return { ...(error ? { error } : {}), state }; + return { state }; } public async removeTask(): Promise { diff --git a/x-pack/plugins/task_manager/server/task_type_dictionary.ts b/x-pack/plugins/task_manager/server/task_type_dictionary.ts index 7a99be8612620..9e6da424d13e0 100644 --- a/x-pack/plugins/task_manager/server/task_type_dictionary.ts +++ b/x-pack/plugins/task_manager/server/task_type_dictionary.ts @@ -75,7 +75,8 @@ export interface TaskRegisterDefinition { >; paramsSchema?: ObjectType; - indirectParamsSchema?: ObjectType; + indirectParamsSchema?: Partial; + latestTypeVersion?: number; } /** diff --git a/x-pack/test/alerting_api_integration/common/config.ts b/x-pack/test/alerting_api_integration/common/config.ts index 88f3cce226a70..cd2455a4073a3 100644 --- a/x-pack/test/alerting_api_integration/common/config.ts +++ b/x-pack/test/alerting_api_integration/common/config.ts @@ -348,6 +348,7 @@ export function createTestConfig(name: string, options: CreateTestConfigOptions) '--notifications.connectors.default.email=notification-email', '--xpack.task_manager.allow_reading_invalid_state=false', '--xpack.task_manager.requeue_invalid_tasks.enabled=true', + '--xpack.task_manager.requeue_invalid_tasks.max_attempts=2', '--xpack.actions.queued.max=500', `--xpack.stack_connectors.enableExperimental=${JSON.stringify(experimentalFeatures)}`, ], diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group2/execution_status.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group2/execution_status.ts index 9f2dc2ff77eb6..293f41ed14d97 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group2/execution_status.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group2/execution_status.ts @@ -213,7 +213,7 @@ export default function executionStatusAlertTests({ getService }: FtrProviderCon }) .expect(200); - executionStatus = await waitForStatus(alertId, new Set(['error'])); + executionStatus = await waitForStatus(alertId, new Set(['error']), 50000); expect(executionStatus.error).to.be.ok(); expect(executionStatus.error.reason).to.be('validate'); await ensureAlertUpdatedAtHasNotChanged(alertId, alertUpdatedAt); diff --git a/x-pack/test/plugin_api_integration/plugins/sample_task_plugin/server/plugin.ts b/x-pack/test/plugin_api_integration/plugins/sample_task_plugin/server/plugin.ts index d7bb483fcac01..1d4137e68858b 100644 --- a/x-pack/test/plugin_api_integration/plugins/sample_task_plugin/server/plugin.ts +++ b/x-pack/test/plugin_api_integration/plugins/sample_task_plugin/server/plugin.ts @@ -233,6 +233,18 @@ export class SampleTaskManagerFixturePlugin }, }), }, + sampleTaskWithInvalidVersion: { + title: 'Sample Task That has invalid type version', + description: 'Task that has he latestTypeVersion', + maxAttempts: 1, + latestTypeVersion: 1, + createTaskRunner: () => ({ + async loadIndirectParams() { + return { data: { indirectParams: {}, typeVersion: 2 } }; // greater then latestTypeVersion:1 + }, + async run() {}, + }), + }, taskToDisable: { title: 'Task used for testing it being disabled', description: '', diff --git a/x-pack/test/plugin_api_integration/test_suites/task_manager/check_registered_task_types.ts b/x-pack/test/plugin_api_integration/test_suites/task_manager/check_registered_task_types.ts index f56e5667df331..a3f47a7ec06fe 100644 --- a/x-pack/test/plugin_api_integration/test_suites/task_manager/check_registered_task_types.ts +++ b/x-pack/test/plugin_api_integration/test_suites/task_manager/check_registered_task_types.ts @@ -35,6 +35,7 @@ export default function ({ getService }: FtrProviderContext) { 'sampleOneTimeTaskWithInvalidIndirectParam', 'sampleTaskWithParamsSchema', 'taskToDisable', + 'sampleTaskWithInvalidVersion', ]; // This test is meant to fail when any change is made in task manager registered types. diff --git a/x-pack/test/plugin_api_integration/test_suites/task_manager/skip.ts b/x-pack/test/plugin_api_integration/test_suites/task_manager/skip.ts index 6c5f4239436a4..cf8625662096f 100644 --- a/x-pack/test/plugin_api_integration/test_suites/task_manager/skip.ts +++ b/x-pack/test/plugin_api_integration/test_suites/task_manager/skip.ts @@ -140,5 +140,33 @@ export default function ({ getService }: FtrProviderContext) { expect(task.numSkippedRuns).to.eql(2); }); }); + + it('Skips the tasks with invalid version', async () => { + const createdTask = await supertest + .post('/api/sample_tasks/schedule') + .set('kbn-xsrf', 'xxx') + .send({ + task: { + taskType: 'sampleTaskWithInvalidVersion', + }, + }) + .expect(200) + .then((response: { body: SerializedConcreteTaskInstance }) => { + log.debug(`Task Scheduled: ${response.body.id}`); + return response.body; + }); + + await retry.try(async () => { + const task = await currentTask(createdTask.id); + // skips 2 times + expect(task.numSkippedRuns).to.eql(2); + }); + + await retry.try(async () => { + const task = await currentTask(createdTask.id); + // runs successfully after 2 skips + expect(task.status).to.eql(TaskStatus.Idle); + }); + }); }); }