diff --git a/x-pack/plugins/alerting/server/rules_client/lib/siem_legacy_actions/migrate_legacy_actions.test.ts b/x-pack/plugins/alerting/server/rules_client/lib/siem_legacy_actions/migrate_legacy_actions.test.ts index 50848be43a73f..3e6b010380751 100644 --- a/x-pack/plugins/alerting/server/rules_client/lib/siem_legacy_actions/migrate_legacy_actions.test.ts +++ b/x-pack/plugins/alerting/server/rules_client/lib/siem_legacy_actions/migrate_legacy_actions.test.ts @@ -124,18 +124,17 @@ describe('migrateLegacyActions', () => { jest.clearAllMocks(); }); - it('should return empty migratedActions when error is thrown within method', async () => { + it('should throw an exception when error is thrown within method', async () => { (retrieveMigratedLegacyActions as jest.Mock).mockRejectedValueOnce(new Error('test failure')); - const migratedActions = await migrateLegacyActions(context, { - ruleId, - attributes, - }); + await expect( + migrateLegacyActions(context, { + ruleId, + attributes, + }) + ).rejects.toThrowError( + `Failed to migrate legacy actions for SIEM rule ${ruleId}: test failure` + ); - expect(migratedActions).toEqual({ - resultedActions: [], - hasLegacyActions: false, - resultedReferences: [], - }); expect(context.logger.error).toHaveBeenCalledWith( `migrateLegacyActions(): Failed to migrate legacy actions for SIEM rule ${ruleId}: test failure` ); @@ -168,7 +167,11 @@ describe('migrateLegacyActions', () => { }); await migrateLegacyActions(context, { ruleId, attributes }); - expect(retrieveMigratedLegacyActions).toHaveBeenCalledWith(context, { ruleId }); + expect(retrieveMigratedLegacyActions).toHaveBeenCalledWith( + context, + { ruleId }, + expect.any(Function) + ); }); it('should not call validateActions and injectReferencesIntoActions if skipActionsValidation=true', async () => { @@ -178,44 +181,6 @@ describe('migrateLegacyActions', () => { expect(injectReferencesIntoActions).not.toHaveBeenCalled(); }); - it('should call validateActions and injectReferencesIntoActions if attributes provided', async () => { - (retrieveMigratedLegacyActions as jest.Mock).mockResolvedValueOnce({ - legacyActions: legacyActionsMock, - legacyActionsReferences: legacyReferencesMock, - }); - - (injectReferencesIntoActions as jest.Mock).mockReturnValue('actions-with-references'); - await migrateLegacyActions(context, { ruleId, attributes }); - - expect(validateActions).toHaveBeenCalledWith(context, ruleType, { - ...attributes, - actions: 'actions-with-references', - }); - - expect(injectReferencesIntoActions).toHaveBeenCalledWith( - 'rule_id_1', - [ - { - actionRef: 'action_0', - actionTypeId: '.email', - group: 'default', - params: { - message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', - subject: 'Test Actions', - to: ['test@test.com'], - }, - uuid: '11403909-ca9b-49ba-9d7a-7e5320e68d05', - frequency: { - notifyWhen: 'onThrottleInterval', - summary: true, - throttle: '1d', - }, - }, - ], - [{ id: 'cc85da20-d480-11ed-8e69-1df522116c28', name: 'action_0', type: 'action' }] - ); - }); - it('should set frequency props from rule level to existing actions', async () => { const result = await migrateLegacyActions(context, { ruleId, diff --git a/x-pack/plugins/alerting/server/rules_client/lib/siem_legacy_actions/migrate_legacy_actions.ts b/x-pack/plugins/alerting/server/rules_client/lib/siem_legacy_actions/migrate_legacy_actions.ts index c092e3fed982b..75bcb39e522b9 100644 --- a/x-pack/plugins/alerting/server/rules_client/lib/siem_legacy_actions/migrate_legacy_actions.ts +++ b/x-pack/plugins/alerting/server/rules_client/lib/siem_legacy_actions/migrate_legacy_actions.ts @@ -5,6 +5,9 @@ * 2.0. */ +import Boom from '@hapi/boom'; +import { i18n } from '@kbn/i18n'; + import { AlertConsumers } from '@kbn/rule-data-utils'; import type { SavedObjectReference } from '@kbn/core/server'; @@ -49,15 +52,14 @@ export const migrateLegacyActions: MigrateLegacyActions = async ( }; } - const { legacyActions, legacyActionsReferences } = await retrieveMigratedLegacyActions( - context, - { - ruleId, + const validateLegacyActions = async ( + legacyActions: RawRuleAction[], + legacyActionsReferences: SavedObjectReference[] + ) => { + // sometimes we don't need to validate legacy actions. For example, when delete rules or update rule from payload + if (skipActionsValidation === true) { + return; } - ); - - // sometimes we don't need to validate legacy actions. For example, when delete rules or update rule from payload - if (skipActionsValidation !== true) { const ruleType = context.ruleTypeRegistry.get(attributes.alertTypeId); await validateActions(context, ruleType, { ...attributes, @@ -66,7 +68,15 @@ export const migrateLegacyActions: MigrateLegacyActions = async ( notifyWhen: undefined, actions: injectReferencesIntoActions(ruleId, legacyActions, legacyActionsReferences), }); - } + }; + + const { legacyActions, legacyActionsReferences } = await retrieveMigratedLegacyActions( + context, + { + ruleId, + }, + validateLegacyActions + ); // fix references for a case when actions present in a rule if (actions.length) { @@ -103,11 +113,14 @@ export const migrateLegacyActions: MigrateLegacyActions = async ( context.logger.error( `migrateLegacyActions(): Failed to migrate legacy actions for SIEM rule ${ruleId}: ${e.message}` ); - - return { - resultedActions: [], - hasLegacyActions: false, - resultedReferences: [], - }; + throw Boom.badRequest( + i18n.translate('xpack.alerting.rulesClient.validateLegacyActions.errorSummary', { + defaultMessage: 'Failed to migrate legacy actions for SIEM rule {ruleId}: {errorMessage}', + values: { + ruleId, + errorMessage: e.message, + }, + }) + ); } }; diff --git a/x-pack/plugins/alerting/server/rules_client/lib/siem_legacy_actions/retrieve_migrated_legacy_actions.test.ts b/x-pack/plugins/alerting/server/rules_client/lib/siem_legacy_actions/retrieve_migrated_legacy_actions.test.ts index b49497d086efe..043754dff226e 100644 --- a/x-pack/plugins/alerting/server/rules_client/lib/siem_legacy_actions/retrieve_migrated_legacy_actions.test.ts +++ b/x-pack/plugins/alerting/server/rules_client/lib/siem_legacy_actions/retrieve_migrated_legacy_actions.test.ts @@ -69,7 +69,8 @@ describe('Legacy rule action migration logic', () => { unsecuredSavedObjectsClient: savedObjectsClient, logger, } as unknown as RulesClientContext, - { ruleId } + { ruleId }, + () => Promise.resolve() ); expect(deleteRuleMock).not.toHaveBeenCalled(); @@ -92,7 +93,8 @@ describe('Legacy rule action migration logic', () => { unsecuredSavedObjectsClient: savedObjectsClient, logger, } as unknown as RulesClientContext, - { ruleId } + { ruleId }, + () => Promise.resolve() ); expect(deleteRuleMock).not.toHaveBeenCalled(); @@ -116,7 +118,8 @@ describe('Legacy rule action migration logic', () => { unsecuredSavedObjectsClient: savedObjectsClient, logger, } as unknown as RulesClientContext, - { ruleId } + { ruleId }, + () => Promise.resolve() ); expect(deleteRuleMock).not.toHaveBeenCalled(); @@ -146,7 +149,8 @@ describe('Legacy rule action migration logic', () => { unsecuredSavedObjectsClient: savedObjectsClient, logger, } as unknown as RulesClientContext, - { ruleId } + { ruleId }, + () => Promise.resolve() ); expect(deleteRuleMock).toHaveBeenCalledWith(expect.any(Object), { id: '456' }); @@ -192,7 +196,8 @@ describe('Legacy rule action migration logic', () => { unsecuredSavedObjectsClient: savedObjectsClient, logger, } as unknown as RulesClientContext, - { ruleId } + { ruleId }, + () => Promise.resolve() ); expect(deleteRuleMock).toHaveBeenCalledWith(expect.any(Object), { id: '456' }); @@ -237,7 +242,8 @@ describe('Legacy rule action migration logic', () => { unsecuredSavedObjectsClient: savedObjectsClient, logger, } as unknown as RulesClientContext, - { ruleId } + { ruleId }, + () => Promise.resolve() ); expect(deleteRuleMock).toHaveBeenCalledWith(expect.any(Object), { id: '456' }); @@ -263,5 +269,47 @@ describe('Legacy rule action migration logic', () => { legacyActionsReferences: [{ id: '456', name: 'action_0', type: 'action' }], }); }); + + test('it calls validateLegacyActions on migration a rule with legacy actions', async () => { + // siem.notifications is not created for a rule with no actions + findMock.mockResolvedValueOnce({ + page: 1, + perPage: 1, + total: 1, + data: [legacyGetDailyNotificationResult(connectorId, ruleId)], + }); + // siem-detection-engine-rule-actions SO is still created + savedObjectsClient.find.mockResolvedValueOnce( + legacyGetSiemNotificationRuleActionsSOResultWithSingleHit(['daily'], ruleId, connectorId) + ); + + const validateLegacyActions = jest.fn(); + await retrieveMigratedLegacyActions( + { + unsecuredSavedObjectsClient: savedObjectsClient, + logger, + } as unknown as RulesClientContext, + { ruleId }, + validateLegacyActions + ); + + expect(validateLegacyActions).toHaveBeenCalledWith( + [ + { + actionRef: 'action_0', + actionTypeId: '.email', + frequency: { notifyWhen: 'onThrottleInterval', summary: true, throttle: '1d' }, + group: 'default', + params: { + message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', + subject: 'Test Actions', + to: ['test@test.com'], + }, + uuid: expect.any(String), + }, + ], + [{ id: '456', name: 'action_0', type: 'action' }] + ); + }); }); }); diff --git a/x-pack/plugins/alerting/server/rules_client/lib/siem_legacy_actions/retrieve_migrated_legacy_actions.ts b/x-pack/plugins/alerting/server/rules_client/lib/siem_legacy_actions/retrieve_migrated_legacy_actions.ts index 83cd315aaa5a5..9678dff6898c8 100644 --- a/x-pack/plugins/alerting/server/rules_client/lib/siem_legacy_actions/retrieve_migrated_legacy_actions.ts +++ b/x-pack/plugins/alerting/server/rules_client/lib/siem_legacy_actions/retrieve_migrated_legacy_actions.ts @@ -15,7 +15,11 @@ import { transformFromLegacyActions } from './transform_legacy_actions'; type RetrieveMigratedLegacyActions = ( context: RulesClientContext, - { ruleId }: { ruleId: string } + { ruleId }: { ruleId: string }, + validateLegacyActions: ( + legacyActions: RawRuleAction[], + legacyActionsReferences: SavedObjectReference[] + ) => Promise ) => Promise<{ legacyActions: RawRuleAction[]; legacyActionsReferences: SavedObjectReference[] }>; /** @@ -27,7 +31,8 @@ type RetrieveMigratedLegacyActions = ( */ export const retrieveMigratedLegacyActions: RetrieveMigratedLegacyActions = async ( context, - { ruleId } + { ruleId }, + validateLegacyActions ) => { const { unsecuredSavedObjectsClient } = context; try { @@ -71,17 +76,19 @@ export const retrieveMigratedLegacyActions: RetrieveMigratedLegacyActions = asyn return { legacyActions: [], legacyActionsReferences: [] }; } - await Promise.all([ - // If the legacy notification rule type ("siem.notification") exist, - // migration and cleanup are needed - siemNotificationsExist && deleteRule(context, { id: siemNotification.data[0].id }), - // Delete the legacy sidecar SO if it exists - legacyRuleNotificationSOsExist && - unsecuredSavedObjectsClient.delete( - legacyRuleActionsSavedObjectType, - legacyRuleActionsSO.saved_objects[0].id - ), - ]); + const deleteLegacyActions = async () => { + await Promise.all([ + // If the legacy notification rule type ("siem.notification") exist, + // migration and cleanup are needed + siemNotificationsExist && deleteRule(context, { id: siemNotification.data[0].id }), + // Delete the legacy sidecar SO if it exists + legacyRuleNotificationSOsExist && + unsecuredSavedObjectsClient.delete( + legacyRuleActionsSavedObjectType, + legacyRuleActionsSO.saved_objects[0].id + ), + ]); + }; // If legacy notification sidecar ("siem-detection-engine-rule-actions") // exist, migration is needed @@ -95,22 +102,28 @@ export const retrieveMigratedLegacyActions: RetrieveMigratedLegacyActions = asyn legacyRuleActionsSO.saved_objects[0].attributes.ruleThrottle === 'no_actions' || legacyRuleActionsSO.saved_objects[0].attributes.ruleThrottle === 'rule' ) { + await deleteLegacyActions(); return { legacyActions: [], legacyActionsReferences: [] }; } - return { - legacyActions: transformFromLegacyActions( - legacyRuleActionsSO.saved_objects[0].attributes, - legacyRuleActionsSO.saved_objects[0].references - ), - legacyActionsReferences: - // only action references need to be saved - legacyRuleActionsSO.saved_objects[0].references.filter(({ type }) => type === 'action') ?? - [], - }; + const legacyActions = transformFromLegacyActions( + legacyRuleActionsSO.saved_objects[0].attributes, + legacyRuleActionsSO.saved_objects[0].references + ); + // only action references need to be saved + const legacyActionsReferences = + legacyRuleActionsSO.saved_objects[0].references.filter(({ type }) => type === 'action') ?? + []; + await validateLegacyActions(legacyActions, legacyActionsReferences); + + // Delete legacy actions only after the validation + await deleteLegacyActions(); + return { legacyActions, legacyActionsReferences }; } + await deleteLegacyActions(); } catch (e) { context.logger.debug(`Migration has failed for rule ${ruleId}: ${e.message}`); + throw e; } return { legacyActions: [], legacyActionsReferences: [] };