diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.test.ts index 8b4a9cf250226..a601f7d14528f 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.test.ts @@ -26,7 +26,7 @@ import { import { getAlertMock } from '../__mocks__/request_responses'; import { INTERNAL_IDENTIFIER } from '../../../../../common/constants'; import { PartialFilter } from '../../types'; -import { BulkError } from '../utils'; +import { BulkError, createBulkErrorObject } from '../utils'; import { getOutputRuleAlertForRest } from '../__mocks__/utils'; import { PartialAlert } from '../../../../../../alerting/server'; import { createRulesAndExceptionsStreamFromNdJson } from '../../rules/create_rules_stream_from_ndjson'; @@ -46,9 +46,24 @@ import { LegacyRulesActionsSavedObject } from '../../rule_actions/legacy_get_rul // eslint-disable-next-line no-restricted-imports import { LegacyRuleAlertAction } from '../../rule_actions/legacy_types'; import { RuleExceptionsPromiseFromStreams } from './utils/import_rules_utils'; +import { partition } from 'lodash/fp'; type PromiseFromStreams = ImportRulesSchemaDecoded | Error; +const createMockImportRule = async (rule: ReturnType) => { + const ndJsonStream = new Readable({ + read() { + this.push(`${JSON.stringify(rule)}\n`); + this.push(null); + }, + }); + const [{ rules }] = await createPromiseFromStreams([ + ndJsonStream, + ...createRulesAndExceptionsStreamFromNdJson(1000), + ]); + return rules; +}; + describe.each([ ['Legacy', false], ['RAC', true], @@ -667,12 +682,10 @@ describe.each([ soClient.find.mockClear(); }); - test('returns original action if Elasticsearch query fails', async () => { - clients.core.savedObjects - .getClient() - .find.mockRejectedValueOnce(new Error('failed to query')); + test('returns error if Elasticsearch query fails', async () => { + soClient.find.mockRejectedValue(new Error('failed to query')); const result = await swapActionIds(mockAction, soClient); - expect(result).toEqual(mockAction); + expect((result as Error).message).toEqual('failed to query'); }); test('returns original action if Elasticsearch query returns no hits', async () => { @@ -781,9 +794,50 @@ describe.each([ ]); }); + test('returns import rules schemas + one migrated action + one error', async () => { + const rule: ReturnType = { + ...getCreateRulesSchemaMock('rule-1'), + actions: [mockAction, { ...mockAction, id: 'different-id' }], + }; + const rules = await createMockImportRule(rule); + soClient.find.mockImplementationOnce(async () => ({ + total: 0, + per_page: 0, + page: 1, + saved_objects: [ + { score: 0, id: 'new-post-8.0-id', type: 'action', attributes: {}, references: [] }, + ], + })); + + soClient.find.mockRejectedValueOnce(new Error('failed to query')); + + const res = await migrateLegacyActionsIds(rules, soClient); + expect(soClient.find.mock.calls).toHaveLength(2); + const [error, ruleRes] = partition( + (item): item is Error => item instanceof Error + )(res); + + expect(ruleRes[0]).toEqual({ + ...rules[0], + actions: [{ ...mockAction, id: 'new-post-8.0-id' }], + }); + expect(error[0]).toEqual( + new Error( + JSON.stringify( + createBulkErrorObject({ + ruleId: rule.rule_id, + statusCode: 409, + message: `${[new Error('failed to query')].map((e: Error) => e.message).join(',')}`, + }) + ) + ) + ); + }); + test('returns import rules schemas + migrated action resulting in error', async () => { const rule: ReturnType = { ...getCreateRulesSchemaMock('rule-1'), + // only passing in one action here actions: [mockAction], }; soClient.find.mockImplementationOnce(async () => ({ @@ -791,6 +845,7 @@ describe.each([ per_page: 0, page: 1, saved_objects: [ + // two actions are being returned, thus resulting in a conflict { score: 0, id: 'new-post-8.0-id', type: 'action', attributes: {}, references: [] }, { score: 0, id: 'new-post-8.0-id-2', type: 'action', attributes: {}, references: [] }, ], @@ -808,6 +863,7 @@ describe.each([ error: { status_code: 409, message: + // error message for when two or more action connectors are found for a single id 'Found two action connectors with originId or _id: some-7.x-id The upload cannot be completed unless the _id or the originId of the action connector is changed. See https://www.elastic.co/guide/en/kibana/current/sharing-saved-objects.html for more details', }, }) @@ -843,8 +899,9 @@ describe.each([ soClient ); expect(res[0]).toEqual({ ...rule, actions: [{ ...mockAction, id: 'new-post-8.0-id' }] }); - expect(res[1] instanceof Error).toBeTruthy(); - expect((res[1] as unknown as Error).message).toEqual( + expect(res[1]).toEqual({ ...rule, actions: [] }); + expect(res[2] instanceof Error).toBeTruthy(); + expect((res[2] as unknown as Error).message).toEqual( JSON.stringify({ rule_id: 'rule-1', error: { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts index a52a2804d3c7e..8819b068fd5d7 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts @@ -5,7 +5,7 @@ * 2.0. */ -import { countBy } from 'lodash/fp'; +import { countBy, partition } from 'lodash/fp'; import uuid from 'uuid'; import { Action } from '@kbn/securitysolution-io-ts-alerting-types'; import { SavedObjectsClientContract } from 'kibana/server'; @@ -229,10 +229,10 @@ export const swapActionIds = async ( `Found two action connectors with originId or _id: ${action.id} The upload cannot be completed unless the _id or the originId of the action connector is changed. See https://www.elastic.co/guide/en/kibana/current/sharing-saved-objects.html for more details` ); } - } catch (exc) { return action; + } catch (exc) { + return exc; } - return action; }; /** @@ -271,7 +271,7 @@ export const migrateLegacyActionsIds = async ( ): Promise => { const isImportRule = (r: unknown): r is ImportRulesSchemaDecoded => !(r instanceof Error); - return pMap( + const toReturn = await pMap( rules, async (rule) => { if (isImportRule(rule)) { @@ -284,33 +284,32 @@ export const migrateLegacyActionsIds = async ( ); // were there any errors discovered while trying to migrate and swap the action connector ids? - const actionMigrationErrors = newActions.filter( - (action): action is Error => action instanceof Error - ); - - const newlyMigratedActions: Action[] = newActions.filter( - (action): action is Action => !(action instanceof Error) - ); + const [actionMigrationErrors, newlyMigratedActions] = partition( + (item): item is Error => item instanceof Error + )(newActions); if (actionMigrationErrors == null || actionMigrationErrors.length === 0) { return { ...rule, actions: newlyMigratedActions }; } - // return an Error object with the rule_id and the error messages - // for the actions associated with that rule. - return new Error( - JSON.stringify( - createBulkErrorObject({ - ruleId: rule.rule_id, - statusCode: 409, - message: `${actionMigrationErrors.map((error: Error) => error.message).join(',')}`, - }) - ) - ); + + return [ + { ...rule, actions: newlyMigratedActions }, + new Error( + JSON.stringify( + createBulkErrorObject({ + ruleId: rule.rule_id, + statusCode: 409, + message: `${actionMigrationErrors.map((error: Error) => error.message).join(',')}`, + }) + ) + ), + ]; } return rule; }, { concurrency: MAX_CONCURRENT_SEARCHES } ); + return toReturn.flat(); }; /**