From 4427c20cef716477855e951389b150e79f2dc4a9 Mon Sep 17 00:00:00 2001 From: Alexandra Doak Date: Wed, 23 Nov 2022 11:48:06 -0500 Subject: [PATCH 1/4] Updating to load the rule once --- .../alerting/server/rules_client.mock.ts | 1 + .../server/rules_client/rules_client.ts | 60 +++++++++--------- .../server/task_runner/rule_loader.test.ts | 57 +++++++---------- .../server/task_runner/rule_loader.ts | 61 ++++++++++++------- .../server/task_runner/task_runner.ts | 13 +++- 5 files changed, 102 insertions(+), 90 deletions(-) diff --git a/x-pack/plugins/alerting/server/rules_client.mock.ts b/x-pack/plugins/alerting/server/rules_client.mock.ts index b3844ddd5f74b..75f4db4b143ed 100644 --- a/x-pack/plugins/alerting/server/rules_client.mock.ts +++ b/x-pack/plugins/alerting/server/rules_client.mock.ts @@ -45,6 +45,7 @@ const createRulesClientMock = () => { clearExpiredSnoozes: jest.fn(), runSoon: jest.fn(), clone: jest.fn(), + getAlertFromRaw: jest.fn(), }; return mocked; }; 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 3da3315148596..c05cf9423044f 100644 --- a/x-pack/plugins/alerting/server/rules_client/rules_client.ts +++ b/x-pack/plugins/alerting/server/rules_client/rules_client.ts @@ -3760,6 +3760,36 @@ export class RulesClient { return this.spaceId; } + public getAlertFromRaw( + id: string, + ruleTypeId: string, + rawRule: RawRule, + references: SavedObjectReference[] | undefined, + includeLegacyId: boolean = false, + excludeFromPublicApi: boolean = false, + includeSnoozeData: boolean = false + ): Rule | RuleWithLegacyId { + const ruleType = this.ruleTypeRegistry.get(ruleTypeId); + // In order to support the partial update API of Saved Objects we have to support + // partial updates of an Alert, but when we receive an actual RawRule, it is safe + // to cast the result to an Alert + const res = this.getPartialRuleFromRaw( + id, + ruleType, + rawRule, + references, + includeLegacyId, + excludeFromPublicApi, + includeSnoozeData + ); + // include to result because it is for internal rules client usage + if (includeLegacyId) { + return res as RuleWithLegacyId; + } + // exclude from result because it is an internal variable + return omit(res, ['legacyId']) as Rule; + } + private async scheduleTask(opts: ScheduleTaskOptions) { const { id, consumer, ruleTypeId, schedule, throwOnConflict } = opts; const taskInstance = { @@ -3813,36 +3843,6 @@ export class RulesClient { }) as Rule['actions']; } - private getAlertFromRaw( - id: string, - ruleTypeId: string, - rawRule: RawRule, - references: SavedObjectReference[] | undefined, - includeLegacyId: boolean = false, - excludeFromPublicApi: boolean = false, - includeSnoozeData: boolean = false - ): Rule | RuleWithLegacyId { - const ruleType = this.ruleTypeRegistry.get(ruleTypeId); - // In order to support the partial update API of Saved Objects we have to support - // partial updates of an Alert, but when we receive an actual RawRule, it is safe - // to cast the result to an Alert - const res = this.getPartialRuleFromRaw( - id, - ruleType, - rawRule, - references, - includeLegacyId, - excludeFromPublicApi, - includeSnoozeData - ); - // include to result because it is for internal rules client usage - if (includeLegacyId) { - return res as RuleWithLegacyId; - } - // exclude from result because it is an internal variable - return omit(res, ['legacyId']) as Rule; - } - private getPartialRuleFromRaw( id: string, ruleType: UntypedNormalizedRuleType, 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 1cbe2bb2ae3b9..efc9d0418ccd2 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 @@ -9,11 +9,11 @@ import { encryptedSavedObjectsMock } from '@kbn/encrypted-saved-objects-plugin/s import { CoreKibanaRequest } from '@kbn/core/server'; import { schema } from '@kbn/config-schema'; -import { getDecryptedAttributes, getFakeKibanaRequest, loadRule } from './rule_loader'; +import { getRuleAttributes, getFakeKibanaRequest, loadRule } from './rule_loader'; import { TaskRunnerContext } from './task_runner_factory'; import { ruleTypeRegistryMock } from '../rule_type_registry.mock'; import { rulesClientMock } from '../rules_client.mock'; -import { Rule, RulesClientApi } from '../types'; +import { Rule } from '../types'; import { MONITORING_HISTORY_LIMIT, RuleExecutionStatusErrorReasons } from '../../common'; import { getReasonFromError } from '../lib/error_with_reason'; import { alertingEventLoggerMock } from '../lib/alerting_event_logger/alerting_event_logger.mock'; @@ -137,25 +137,6 @@ describe('rule_loader', () => { expect(outcome).toBe('failure'); }); - test('throws when user cannot read rule', async () => { - context.getRulesClientWithRequest = function (fakeRequest: unknown): RulesClientApi { - rulesClient.get.mockImplementation(async (args: unknown) => { - throw new Error('rule-client-error: 1001'); - }); - return rulesClient; - }; - - let outcome = 'success'; - try { - await loadRule({ ...DefaultLoadRuleParams, context }); - } catch (err) { - outcome = 'failure'; - expect(err.message).toBe('rule-client-error: 1001'); - expect(getReasonFromError(err)).toBe(RuleExecutionStatusErrorReasons.Read); - } - expect(outcome).toBe('failure'); - }); - test('throws when rule type is not enabled', async () => { ruleTypeRegistry.ensureRuleTypeEnabled.mockImplementation(() => { throw new Error('rule-type-not-enabled: 2112'); @@ -196,11 +177,14 @@ describe('rule_loader', () => { describe('getDecryptedAttributes()', () => { test('succeeds with default space', async () => { contextMock.spaceIdToNamespace.mockReturnValue(undefined); - const result = await getDecryptedAttributes(context, ruleId, 'default'); + const result = await getRuleAttributes(context, ruleId, 'default'); expect(result.apiKey).toBe(apiKey); expect(result.consumer).toBe(consumer); expect(result.enabled).toBe(true); + expect(result.fakeRequest).toEqual(expect.any(CoreKibanaRequest)); + expect(result.rule.alertTypeId).toBe(ruleTypeId); + expect(result.rulesClient).toBeTruthy(); expect(contextMock.spaceIdToNamespace.mock.calls[0]).toEqual(['default']); const esoArgs = encryptedSavedObjects.getDecryptedAsInternalUser.mock.calls[0]; @@ -209,11 +193,14 @@ describe('rule_loader', () => { test('succeeds with non-default space', async () => { contextMock.spaceIdToNamespace.mockReturnValue(spaceId); - const result = await getDecryptedAttributes(context, ruleId, spaceId); + const result = await getRuleAttributes(context, ruleId, spaceId); expect(result.apiKey).toBe(apiKey); expect(result.consumer).toBe(consumer); expect(result.enabled).toBe(true); + expect(result.fakeRequest).toEqual(expect.any(CoreKibanaRequest)); + expect(result.rule.alertTypeId).toBe(ruleTypeId); + expect(result.rulesClient).toBeTruthy(); expect(contextMock.spaceIdToNamespace.mock.calls[0]).toEqual([spaceId]); const esoArgs = encryptedSavedObjects.getDecryptedAsInternalUser.mock.calls[0]; @@ -227,7 +214,7 @@ describe('rule_loader', () => { } ); - const promise = getDecryptedAttributes(context, ruleId, spaceId); + const promise = getRuleAttributes(context, ruleId, spaceId); await expect(promise).rejects.toThrow('wops'); }); }); @@ -340,20 +327,18 @@ function getTaskRunnerContext(ruleParameters: unknown, historyElements: number) getRulesClientWithRequest, }; - function getRulesClientWithRequest(fakeRequest: unknown) { + function getRulesClientWithRequest() { // only need get() mocked - rulesClient.get.mockImplementation(async (args: unknown) => { - return { - name: ruleName, - alertTypeId: ruleTypeId, - params: ruleParameters, - monitoring: { - run: { - history: new Array(historyElements), - }, + rulesClient.getAlertFromRaw.mockReturnValue({ + name: ruleName, + alertTypeId: ruleTypeId, + params: ruleParameters, + monitoring: { + run: { + history: new Array(historyElements), }, - } as Rule; - }); + }, + } as Rule); return rulesClient; } } 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 8d4b00a54e094..71402e019ed57 100644 --- a/x-pack/plugins/alerting/server/task_runner/rule_loader.ts +++ b/x-pack/plugins/alerting/server/task_runner/rule_loader.ts @@ -17,6 +17,7 @@ import { RuleTypeRegistry, RuleTypeParamsValidator, SanitizedRule, + RulesClientApi, } from '../types'; import { MONITORING_HISTORY_LIMIT, RuleTypeParams } from '../../common'; import { AlertingEventLogger } from '../lib/alerting_event_logger/alerting_event_logger'; @@ -35,11 +36,17 @@ export async function loadRule(params: LoadRulePa params; let enabled: boolean; let apiKey: string | null; + let rule: SanitizedRule; + let fakeRequest: CoreKibanaRequest; + let rulesClient: RulesClientApi; try { - const decryptedAttributes = await getDecryptedAttributes(context, ruleId, spaceId); - apiKey = decryptedAttributes.apiKey; - enabled = decryptedAttributes.enabled; + const attributes = await getRuleAttributes(context, ruleId, spaceId); + apiKey = attributes.apiKey; + enabled = attributes.enabled; + rule = attributes.rule; + fakeRequest = attributes.fakeRequest; + rulesClient = attributes.rulesClient; } catch (err) { throw new ErrorWithReason(RuleExecutionStatusErrorReasons.Decrypt, err); } @@ -51,18 +58,6 @@ export async function loadRule(params: LoadRulePa ); } - const fakeRequest = getFakeKibanaRequest(context, spaceId, apiKey); - const rulesClient = context.getRulesClientWithRequest(fakeRequest); - - let rule: SanitizedRule; - - // Ensure API key is still valid and user has access - try { - rule = await rulesClient.get({ id: ruleId }); - } catch (err) { - throw new ErrorWithReason(RuleExecutionStatusErrorReasons.Read, err); - } - alertingEventLogger.setRuleName(rule.name); try { @@ -94,24 +89,44 @@ export async function loadRule(params: LoadRulePa }; } -export async function getDecryptedAttributes( +export async function getRuleAttributes( context: TaskRunnerContext, ruleId: string, spaceId: string -): Promise<{ apiKey: string | null; enabled: boolean; consumer: string }> { +): Promise<{ + apiKey: string | null; + enabled: boolean; + consumer: string; + rule: SanitizedRule; + fakeRequest: CoreKibanaRequest; + rulesClient: RulesClientApi; +}> { const namespace = context.spaceIdToNamespace(spaceId); - // Only fetch encrypted attributes here, we'll create a saved objects client - // scoped with the API key to fetch the remaining data. - const { - attributes: { apiKey, enabled, consumer }, - } = await context.encryptedSavedObjectsClient.getDecryptedAsInternalUser( + const rawRule = await context.encryptedSavedObjectsClient.getDecryptedAsInternalUser( 'alert', ruleId, { namespace } ); - return { apiKey, enabled, consumer }; + const fakeRequest = getFakeKibanaRequest(context, spaceId, rawRule.attributes.apiKey); + const rulesClient = context.getRulesClientWithRequest(fakeRequest); + const rule = rulesClient.getAlertFromRaw( + ruleId, + rawRule.attributes.alertTypeId as string, + rawRule.attributes as RawRule, + rawRule.references, + false + ); + + return { + rule, + apiKey: rawRule.attributes.apiKey, + enabled: rawRule.attributes.enabled, + consumer: rawRule.attributes.consumer, + fakeRequest, + rulesClient, + }; } export function getFakeKibanaRequest( 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 3fc70537f1bc9..802e86e4e7dba 100644 --- a/x-pack/plugins/alerting/server/task_runner/task_runner.ts +++ b/x-pack/plugins/alerting/server/task_runner/task_runner.ts @@ -652,7 +652,18 @@ export class TaskRunner< // fetch the rule again to ensure we return the correct schedule as it may have // changed during the task execution - schedule = asOk((await preparedResult.rulesClient.get({ id: ruleId })).schedule); + schedule = asOk( + ( + await loadRule({ + paramValidator: this.ruleType.validate?.params, + ruleId, + spaceId, + context: this.context, + ruleTypeRegistry: this.ruleTypeRegistry, + alertingEventLogger: this.alertingEventLogger, + }) + ).rule.schedule + ); } catch (err) { stateWithMetrics = asErr(err); schedule = asErr(err); From 7ba1cdecb2617c8a47ed1979115b6bcf341a28ce Mon Sep 17 00:00:00 2001 From: Alexandra Doak Date: Mon, 5 Dec 2022 10:21:14 -0800 Subject: [PATCH 2/4] Fixing tests --- .../server/task_runner/task_runner.test.ts | 129 +++++++++--------- 1 file changed, 66 insertions(+), 63 deletions(-) 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 69b648f099e44..262ced2b8c32d 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 @@ -14,6 +14,8 @@ import { AlertInstanceState, AlertInstanceContext, RuleExecutionStatusWarningReasons, + Rule, + RuleAction, } from '../types'; import { ConcreteTaskInstance, isUnrecoverableError } from '@kbn/task-manager-plugin/server'; import { TaskRunnerContext } from './task_runner_factory'; @@ -220,7 +222,7 @@ describe('Task Runner', () => { ); expect(AlertingEventLogger).toHaveBeenCalledTimes(1); - rulesClient.get.mockResolvedValue(mockedRuleTypeSavedObject); + rulesClient.getAlertFromRaw.mockReturnValue(mockedRuleTypeSavedObject as Rule); encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValue(SAVED_OBJECT); const runnerResult = await taskRunner.run(); expect(runnerResult).toEqual(generateRunnerResult({ state: true, history: [true] })); @@ -325,7 +327,7 @@ describe('Task Runner', () => { ); expect(AlertingEventLogger).toHaveBeenCalledTimes(1); - rulesClient.get.mockResolvedValue(mockedRuleTypeSavedObject); + rulesClient.getAlertFromRaw.mockReturnValue(mockedRuleTypeSavedObject as Rule); encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValue(SAVED_OBJECT); await taskRunner.run(); expect(enqueueFunction).toHaveBeenCalledTimes(1); @@ -405,8 +407,8 @@ describe('Task Runner', () => { ); expect(AlertingEventLogger).toHaveBeenCalledTimes(1); - rulesClient.get.mockResolvedValue({ - ...mockedRuleTypeSavedObject, + rulesClient.getAlertFromRaw.mockReturnValue({ + ...(mockedRuleTypeSavedObject as Rule), muteAll: true, }); encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValue(SAVED_OBJECT); @@ -530,8 +532,8 @@ describe('Task Runner', () => { ); expect(AlertingEventLogger).toHaveBeenCalledTimes(1); - rulesClient.get.mockResolvedValue({ - ...mockedRuleTypeSavedObject, + rulesClient.getAlertFromRaw.mockReturnValue({ + ...(mockedRuleTypeSavedObject as Rule), muteAll, snoozeSchedule: snoozeSchedule != null ? JSON.parse(snoozeSchedule) : [], }); @@ -583,8 +585,8 @@ describe('Task Runner', () => { ); expect(AlertingEventLogger).toHaveBeenCalledTimes(1); - rulesClient.get.mockResolvedValue({ - ...mockedRuleTypeSavedObject, + rulesClient.getAlertFromRaw.mockReturnValue({ + ...(mockedRuleTypeSavedObject as Rule), mutedInstanceIds: ['2'], }); encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValue(SAVED_OBJECT); @@ -666,8 +668,8 @@ describe('Task Runner', () => { ); expect(AlertingEventLogger).toHaveBeenCalledTimes(1); - rulesClient.get.mockResolvedValue({ - ...mockedRuleTypeSavedObject, + rulesClient.getAlertFromRaw.mockReturnValue({ + ...(mockedRuleTypeSavedObject as Rule), throttle: '1d', }); encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValue(SAVED_OBJECT); @@ -710,8 +712,8 @@ describe('Task Runner', () => { ); expect(AlertingEventLogger).toHaveBeenCalledTimes(1); - rulesClient.get.mockResolvedValue({ - ...mockedRuleTypeSavedObject, + rulesClient.getAlertFromRaw.mockReturnValue({ + ...(mockedRuleTypeSavedObject as Rule), mutedInstanceIds: ['2'], notifyWhen: 'onActionGroupChange', }); @@ -767,8 +769,8 @@ describe('Task Runner', () => { ); expect(AlertingEventLogger).toHaveBeenCalledTimes(1); - rulesClient.get.mockResolvedValue({ - ...mockedRuleTypeSavedObject, + rulesClient.getAlertFromRaw.mockReturnValue({ + ...(mockedRuleTypeSavedObject as Rule), notifyWhen: 'onActionGroupChange', }); encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValue(SAVED_OBJECT); @@ -835,8 +837,8 @@ describe('Task Runner', () => { ); expect(AlertingEventLogger).toHaveBeenCalledTimes(1); - rulesClient.get.mockResolvedValue({ - ...mockedRuleTypeSavedObject, + rulesClient.getAlertFromRaw.mockReturnValue({ + ...(mockedRuleTypeSavedObject as Rule), notifyWhen: 'onActionGroupChange', }); encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValue(SAVED_OBJECT); @@ -902,8 +904,8 @@ describe('Task Runner', () => { ); expect(AlertingEventLogger).toHaveBeenCalled(); - rulesClient.get.mockResolvedValue(mockedRuleTypeSavedObject); - encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValueOnce(SAVED_OBJECT); + rulesClient.getAlertFromRaw.mockReturnValue(mockedRuleTypeSavedObject as Rule); + encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValue(SAVED_OBJECT); await taskRunner.run(); expect( customTaskRunnerFactoryInitializerParams.actionsPlugin.getActionsClientWithRequest @@ -1019,7 +1021,7 @@ describe('Task Runner', () => { ); expect(AlertingEventLogger).toHaveBeenCalled(); - rulesClient.get.mockResolvedValue(mockedRuleTypeSavedObject); + rulesClient.getAlertFromRaw.mockReturnValue(mockedRuleTypeSavedObject as Rule); encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValue(SAVED_OBJECT); const runnerResult = await taskRunner.run(); expect(runnerResult.state.alertInstances).toEqual( @@ -1140,7 +1142,7 @@ describe('Task Runner', () => { ); expect(AlertingEventLogger).toHaveBeenCalled(); - rulesClient.get.mockResolvedValue(mockedRuleTypeSavedObject); + rulesClient.getAlertFromRaw.mockReturnValue(mockedRuleTypeSavedObject as Rule); encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValue(SAVED_OBJECT); const runnerResult = await taskRunner.run(); expect(runnerResult.state.alertInstances).toEqual(generateAlertInstance()); @@ -1245,8 +1247,8 @@ describe('Task Runner', () => { ); expect(AlertingEventLogger).toHaveBeenCalled(); - rulesClient.get.mockResolvedValue({ - ...mockedRuleTypeSavedObject, + rulesClient.getAlertFromRaw.mockReturnValue({ + ...(mockedRuleTypeSavedObject as Rule), actions: [ { group: 'default', @@ -1336,7 +1338,7 @@ describe('Task Runner', () => { ); expect(AlertingEventLogger).toHaveBeenCalled(); - rulesClient.get.mockResolvedValue(mockedRuleTypeSavedObject); + rulesClient.getAlertFromRaw.mockReturnValue(mockedRuleTypeSavedObject as Rule); encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValue(SAVED_OBJECT); const runnerResult = await taskRunner.run(); expect(runnerResult.state.alertInstances).toEqual( @@ -1385,13 +1387,11 @@ describe('Task Runner', () => { inMemoryMetrics ); expect(AlertingEventLogger).toHaveBeenCalled(); - - rulesClient.get.mockResolvedValueOnce(mockedRuleTypeSavedObject); - rulesClient.get.mockResolvedValueOnce({ - ...mockedRuleTypeSavedObject, + rulesClient.getAlertFromRaw.mockReturnValue({ + ...(mockedRuleTypeSavedObject as Rule), schedule: { interval: '30s' }, }); - encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValueOnce(SAVED_OBJECT); + encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValue(SAVED_OBJECT); const runnerResult = await taskRunner.run(); expect(runnerResult).toEqual( @@ -1423,7 +1423,7 @@ describe('Task Runner', () => { ); expect(AlertingEventLogger).toHaveBeenCalled(); - rulesClient.get.mockResolvedValue(mockedRuleTypeSavedObject); + rulesClient.getAlertFromRaw.mockReturnValue(mockedRuleTypeSavedObject as Rule); encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValueOnce(SAVED_OBJECT); const runnerResult = await taskRunner.run(); @@ -1451,7 +1451,7 @@ describe('Task Runner', () => { test('recovers gracefully when the Alert Task Runner throws an exception when loading rule to prepare for run', async () => { // used in loadRule() which is called in prepareToRun() - rulesClient.get.mockImplementation(() => { + rulesClient.getAlertFromRaw.mockImplementation(() => { throw new Error(GENERIC_ERROR_MESSAGE); }); @@ -1472,7 +1472,7 @@ describe('Task Runner', () => { testAlertingEventLogCalls({ setRuleName: false, status: 'error', - errorReason: 'read', + errorReason: 'decrypt', executionStatus: 'not-reached', }); @@ -1534,7 +1534,7 @@ describe('Task Runner', () => { ); expect(AlertingEventLogger).toHaveBeenCalled(); - rulesClient.get.mockResolvedValue(mockedRuleTypeSavedObject); + rulesClient.getAlertFromRaw.mockReturnValue(mockedRuleTypeSavedObject as Rule); encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValue(SAVED_OBJECT); const runnerResult = await taskRunner.run(); @@ -1608,7 +1608,7 @@ describe('Task Runner', () => { }); test('reschedules for smaller interval if es connectivity error encountered and schedule interval is greater than connectivity retry', async () => { - rulesClient.get.mockImplementation(() => { + rulesClient.getAlertFromRaw.mockImplementation(() => { throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError('alert', '1'); }); @@ -1705,11 +1705,12 @@ describe('Task Runner', () => { ); expect(AlertingEventLogger).toHaveBeenCalled(); - rulesClient.get.mockResolvedValue({ - ...mockedRuleTypeSavedObject, + rulesClient.getAlertFromRaw.mockReturnValue({ + ...(mockedRuleTypeSavedObject as Rule), notifyWhen: 'onActionGroupChange', actions: [], }); + encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValue(SAVED_OBJECT); await taskRunner.run(); @@ -1811,11 +1812,12 @@ describe('Task Runner', () => { ); expect(AlertingEventLogger).toHaveBeenCalled(); - rulesClient.get.mockResolvedValue({ - ...mockedRuleTypeSavedObject, + rulesClient.getAlertFromRaw.mockReturnValue({ + ...(mockedRuleTypeSavedObject as Rule), notifyWhen: 'onActionGroupChange', actions: [], }); + encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValue(SAVED_OBJECT); await taskRunner.run(); @@ -1885,8 +1887,8 @@ describe('Task Runner', () => { ); expect(AlertingEventLogger).toHaveBeenCalled(); - rulesClient.get.mockResolvedValue({ - ...mockedRuleTypeSavedObject, + rulesClient.getAlertFromRaw.mockReturnValue({ + ...(mockedRuleTypeSavedObject as Rule), notifyWhen: 'onActionGroupChange', actions: [], }); @@ -1954,8 +1956,8 @@ describe('Task Runner', () => { ); expect(AlertingEventLogger).toHaveBeenCalled(); - rulesClient.get.mockResolvedValue({ - ...mockedRuleTypeSavedObject, + rulesClient.getAlertFromRaw.mockReturnValue({ + ...(mockedRuleTypeSavedObject as Rule), notifyWhen: 'onActionGroupChange', actions: [], }); @@ -2028,8 +2030,8 @@ describe('Task Runner', () => { ); expect(AlertingEventLogger).toHaveBeenCalled(); - rulesClient.get.mockResolvedValue({ - ...mockedRuleTypeSavedObject, + rulesClient.getAlertFromRaw.mockReturnValue({ + ...(mockedRuleTypeSavedObject as Rule), notifyWhen: 'onActionGroupChange', actions: [], }); @@ -2080,7 +2082,7 @@ describe('Task Runner', () => { ); expect(AlertingEventLogger).toHaveBeenCalled(); - rulesClient.get.mockResolvedValue(mockedRuleTypeSavedObject); + rulesClient.getAlertFromRaw.mockReturnValue(mockedRuleTypeSavedObject as Rule); encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValue(SAVED_OBJECT); const runnerResult = await taskRunner.run(); expect(runnerResult).toEqual(generateRunnerResult({ state: true, history: [true] })); @@ -2145,8 +2147,8 @@ describe('Task Runner', () => { ); expect(AlertingEventLogger).toHaveBeenCalled(); - rulesClient.get.mockResolvedValue(mockedRuleTypeSavedObject); - encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValueOnce(SAVED_OBJECT); + rulesClient.getAlertFromRaw.mockReturnValue(mockedRuleTypeSavedObject as Rule); + encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValue(SAVED_OBJECT); const runnerResult = await taskRunner.run(); expect(runnerResult).toEqual(generateRunnerResult({ state: true, history: [true] })); }); @@ -2160,7 +2162,7 @@ describe('Task Runner', () => { ); expect(AlertingEventLogger).toHaveBeenCalled(); - rulesClient.get.mockResolvedValue(mockedRuleTypeSavedObject); + rulesClient.getAlertFromRaw.mockReturnValue(mockedRuleTypeSavedObject as Rule); encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValueOnce(SAVED_OBJECT); ruleType.executor.mockImplementation( async ({ @@ -2188,7 +2190,7 @@ describe('Task Runner', () => { ); expect(AlertingEventLogger).toHaveBeenCalled(); - rulesClient.get.mockResolvedValue(mockedRuleTypeSavedObject); + rulesClient.getAlertFromRaw.mockReturnValue(mockedRuleTypeSavedObject as Rule); encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValue(SAVED_OBJECT); await taskRunner.run(); await taskRunner.run(); @@ -2222,11 +2224,11 @@ describe('Task Runner', () => { inMemoryMetrics ); expect(AlertingEventLogger).toHaveBeenCalled(); - rulesClient.get.mockResolvedValue({ - ...mockedRuleTypeSavedObject, + rulesClient.getAlertFromRaw.mockReturnValue({ + ...(mockedRuleTypeSavedObject as Rule), schedule: { interval: '50s' }, }); - encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValueOnce(SAVED_OBJECT); + encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValue(SAVED_OBJECT); await taskRunner.run(); expect( @@ -2247,7 +2249,7 @@ describe('Task Runner', () => { ); expect(AlertingEventLogger).toHaveBeenCalled(); - rulesClient.get.mockResolvedValue(mockedRuleTypeSavedObject); + rulesClient.getAlertFromRaw.mockReturnValue(mockedRuleTypeSavedObject as Rule); encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValue(SAVED_OBJECT); ruleType.executor.mockImplementation( @@ -2295,7 +2297,7 @@ describe('Task Runner', () => { ); expect(AlertingEventLogger).toHaveBeenCalled(); - rulesClient.get.mockResolvedValue(mockedRuleTypeSavedObject); + rulesClient.getAlertFromRaw.mockReturnValue(mockedRuleTypeSavedObject as Rule); encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValue(SAVED_OBJECT); for (let i = 0; i < 300; i++) { @@ -2362,12 +2364,13 @@ describe('Task Runner', () => { }, ]; - rulesClient.get.mockResolvedValue({ - ...mockedRuleTypeSavedObject, - actions: mockActions, - } as jest.ResolvedValue); + rulesClient.getAlertFromRaw.mockReturnValue({ + ...(mockedRuleTypeSavedObject as Rule), + actions: mockActions as RuleAction[], + }); + ruleTypeRegistry.get.mockReturnValue(ruleType); - encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValueOnce(SAVED_OBJECT); + encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValue(SAVED_OBJECT); const taskRunner = new TaskRunner( ruleType, @@ -2496,8 +2499,8 @@ describe('Task Runner', () => { } ); - rulesClient.get.mockResolvedValue({ - ...mockedRuleTypeSavedObject, + rulesClient.getAlertFromRaw.mockReturnValue({ + ...(mockedRuleTypeSavedObject as Rule), actions: [ { group: 'default', @@ -2524,8 +2527,8 @@ describe('Task Runner', () => { id: '5', actionTypeId: 'any-action', }, - ], - } as jest.ResolvedValue); + ] as RuleAction[], + }); ruleTypeRegistry.get.mockReturnValue(ruleType); encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValueOnce(SAVED_OBJECT); @@ -2621,7 +2624,7 @@ describe('Task Runner', () => { ); expect(AlertingEventLogger).toHaveBeenCalled(); - rulesClient.get.mockResolvedValue(mockedRuleTypeSavedObject); + rulesClient.getAlertFromRaw.mockReturnValue(mockedRuleTypeSavedObject as Rule); encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValue({ id: '1', type: 'alert', From 9ee02d1093293aa7b9f385fe10b65a37f1496738 Mon Sep 17 00:00:00 2001 From: Alexandra Doak Date: Mon, 5 Dec 2022 11:21:41 -0800 Subject: [PATCH 3/4] Fixing cancel tests --- .../alerting/server/task_runner/task_runner_cancel.test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) 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 33efc649add26..ebfff148a7016 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 @@ -13,6 +13,7 @@ import { RuleTypeState, AlertInstanceState, AlertInstanceContext, + Rule, } from '../types'; import { ConcreteTaskInstance } from '@kbn/task-manager-plugin/server'; import { TaskRunnerContext } from './task_runner_factory'; @@ -156,8 +157,9 @@ describe('Task Runner Cancel', () => { taskRunnerFactoryInitializerParams.executionContext.withContext.mockImplementation((ctx, fn) => fn() ); - rulesClient.get.mockResolvedValue(mockedRuleTypeSavedObject); - encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValueOnce({ + rulesClient.getAlertFromRaw.mockReturnValue(mockedRuleTypeSavedObject as Rule); + + encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValue({ id: '1', type: 'alert', attributes: { From 16f1b5c9bf1e67759cbf34d355e95ef592d70af1 Mon Sep 17 00:00:00 2001 From: Alexandra Doak Date: Mon, 12 Dec 2022 11:28:19 -0500 Subject: [PATCH 4/4] Fixing test failure from resolving merge conflicts --- x-pack/plugins/alerting/server/task_runner/task_runner.test.ts | 1 + 1 file changed, 1 insertion(+) 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 b7cbfc98af9b9..b07f29f07caf2 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 @@ -667,6 +667,7 @@ describe('Task Runner', () => { rulesClient.getAlertFromRaw.mockReturnValue({ ...(mockedRuleTypeSavedObject as Rule), + notifyWhen: 'onThrottleInterval', throttle: '1d', }); encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValue(SAVED_OBJECT);