[Security Solution][Legacy Actions] - Update legacy action migration …

…to account for more edge cases (#130511)

## Summary

Updates the legacy actions migration code to account for edge cases we had not initially caught. Thanks to testing from some teammates, they reported seeing the following behavior:

- Rules created pre 7.16 with no actions still create the legacy action sidecar (but not a `siem.notifications` legacy actions alert) which upon migration to 7.16+ was not being deleted
- Rules created pre 7.16 with actions that run on every rule run create the legacy action sidecar(but not a `siem.notifications` legacy actions alert) which upon migration to 7.16+ was not being deleted
- Rules created pre 7.16 with actions that were never enabled until 8.x did not have a `siem.notifications` legacy actions alert type created

Because the legacy migration code relied on checking if a corresponding `siem.notifications` SO existed to kick off the necessary cleanup/migration, the above edge cases  were not being caught.

x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/add_prepackaged_rules_route.test.ts

@@ -21,6 +21,15 @@ import { installPrepackagedTimelines } from '../../../timeline/routes/prepackage
 // eslint-disable-next-line @kbn/eslint/no-restricted-paths
 import { elasticsearchClientMock } from '@kbn/core/server/elasticsearch/client/mocks';
 import { getQueryRuleParams } from '../../schemas/rule_schemas.mock';
+import { legacyMigrate } from '../../rules/utils';
+
+jest.mock('../../rules/utils', () => {
+  const actual = jest.requireActual('../../rules/utils');
+  return {
+    ...actual,
+    legacyMigrate: jest.fn(),
+  };
+});
 
 jest.mock('../../rules/get_prepackaged_rules', () => {
   return {
@@ -92,6 +101,8 @@ describe('add_prepackaged_rules_route', () => {
       errors: [],
     });
 
+    (legacyMigrate as jest.Mock).mockResolvedValue(getRuleMock(getQueryRuleParams()));
+
     context.core.elasticsearch.client.asCurrentUser.search.mockResolvedValue(
       elasticsearchClientMock.createSuccessTransportRequestPromise(getBasicEmptySearchResponse())
     );

x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/delete_rules_route.test.ts

@@ -13,10 +13,20 @@ import {
   getFindResultWithSingleHit,
   getDeleteRequestById,
   getEmptySavedObjectsResponse,
+  getRuleMock,
 } from '../__mocks__/request_responses';
 import { requestContextMock, serverMock, requestMock } from '../__mocks__';
 import { deleteRulesRoute } from './delete_rules_route';
 import { getQueryRuleParams } from '../../schemas/rule_schemas.mock';
+import { legacyMigrate } from '../../rules/utils';
+
+jest.mock('../../rules/utils', () => {
+  const actual = jest.requireActual('../../rules/utils');
+  return {
+    ...actual,
+    legacyMigrate: jest.fn(),
+  };
+});
 
 describe('delete_rules', () => {
   let server: ReturnType<typeof serverMock.create>;
@@ -29,6 +39,8 @@ describe('delete_rules', () => {
     clients.rulesClient.find.mockResolvedValue(getFindResultWithSingleHit());
     clients.savedObjectsClient.find.mockResolvedValue(getEmptySavedObjectsResponse());
 
+    (legacyMigrate as jest.Mock).mockResolvedValue(getRuleMock(getQueryRuleParams()));
+
     deleteRulesRoute(server.router);
   });
 
@@ -54,6 +66,7 @@ describe('delete_rules', () => {
 
     test('returns 404 when deleting a single rule that does not exist with a valid actionClient and alertClient', async () => {
       clients.rulesClient.find.mockResolvedValue(getEmptyFindResult());
+      (legacyMigrate as jest.Mock).mockResolvedValue(null);
       const response = await server.inject(
         getDeleteRequest(),
         requestContextMock.convertContext(context)

x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_bulk_route.test.ts

@@ -23,9 +23,18 @@ import { patchRulesBulkRoute } from './patch_rules_bulk_route';
 import { getCreateRulesSchemaMock } from '../../../../../common/detection_engine/schemas/request/rule_schemas.mock';
 import { getQueryRuleParams } from '../../schemas/rule_schemas.mock';
 import { loggingSystemMock } from '@kbn/core/server/mocks';
+import { legacyMigrate } from '../../rules/utils';
 
 jest.mock('../../../machine_learning/authz', () => mockMlAuthzFactory.create());
 
+jest.mock('../../rules/utils', () => {
+  const actual = jest.requireActual('../../rules/utils');
+  return {
+    ...actual,
+    legacyMigrate: jest.fn(),
+  };
+});
+
 describe('patch_rules_bulk', () => {
   let server: ReturnType<typeof serverMock.create>;
   let { clients, context } = requestContextMock.createTools();
@@ -40,6 +49,8 @@ describe('patch_rules_bulk', () => {
     clients.rulesClient.find.mockResolvedValue(getFindResultWithSingleHit()); // rule exists
     clients.rulesClient.update.mockResolvedValue(getRuleMock(getQueryRuleParams())); // update succeeds
 
+    (legacyMigrate as jest.Mock).mockResolvedValue(getRuleMock(getQueryRuleParams()));
+
     patchRulesBulkRoute(server.router, ml, logger);
   });
 
@@ -54,6 +65,7 @@ describe('patch_rules_bulk', () => {
 
     test('returns an error in the response when updating a single rule that does not exist', async () => {
       clients.rulesClient.find.mockResolvedValue(getEmptyFindResult());
+      (legacyMigrate as jest.Mock).mockResolvedValue(null);
       const response = await server.inject(
         getPatchBulkRequest(),
         requestContextMock.convertContext(context)
@@ -148,6 +160,8 @@ describe('patch_rules_bulk', () => {
 
   describe('request validation', () => {
     test('rejects payloads with no ID', async () => {
+      (legacyMigrate as jest.Mock).mockResolvedValue(null);
+
       const request = requestMock.create({
         method: 'patch',
         path: DETECTION_ENGINE_RULES_BULK_UPDATE,

x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_route.test.ts

@@ -21,9 +21,18 @@ import { requestContextMock, serverMock, requestMock } from '../__mocks__';
 import { patchRulesRoute } from './patch_rules_route';
 import { getPatchRulesSchemaMock } from '../../../../../common/detection_engine/schemas/request/patch_rules_schema.mock';
 import { getQueryRuleParams } from '../../schemas/rule_schemas.mock';
+import { legacyMigrate } from '../../rules/utils';
 
 jest.mock('../../../machine_learning/authz', () => mockMlAuthzFactory.create());
 
+jest.mock('../../rules/utils', () => {
+  const actual = jest.requireActual('../../rules/utils');
+  return {
+    ...actual,
+    legacyMigrate: jest.fn(),
+  };
+});
+
 describe('patch_rules', () => {
   let server: ReturnType<typeof serverMock.create>;
   let { clients, context } = requestContextMock.createTools();
@@ -41,6 +50,8 @@ describe('patch_rules', () => {
       getRuleExecutionSummarySucceeded()
     );
 
+    (legacyMigrate as jest.Mock).mockResolvedValue(getRuleMock(getQueryRuleParams()));
+
     patchRulesRoute(server.router, ml);
   });
 
@@ -55,6 +66,7 @@ describe('patch_rules', () => {
 
     test('returns 404 when updating a single rule that does not exist', async () => {
       clients.rulesClient.find.mockResolvedValue(getEmptyFindResult());
+      (legacyMigrate as jest.Mock).mockResolvedValue(null);
       const response = await server.inject(
         getPatchRequest(),
         requestContextMock.convertContext(context)
@@ -67,6 +79,7 @@ describe('patch_rules', () => {
     });
 
     test('returns error if requesting a non-rule', async () => {
+      (legacyMigrate as jest.Mock).mockResolvedValue(null);
       clients.rulesClient.find.mockResolvedValue(nonRuleFindResult());
       const response = await server.inject(
         getPatchRequest(),

x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/perform_bulk_action_route.test.ts

@@ -14,6 +14,7 @@ import {
   getBulkActionEditRequest,
   getFindResultWithSingleHit,
   getFindResultWithMultiHits,
+  getRuleMock,
 } from '../__mocks__/request_responses';
 import { requestContextMock, serverMock, requestMock } from '../__mocks__';
 import { performBulkActionRoute } from './perform_bulk_action_route';
@@ -23,10 +24,20 @@ import {
 } from '../../../../../common/detection_engine/schemas/request/perform_bulk_action_schema.mock';
 import { loggingSystemMock } from '@kbn/core/server/mocks';
 import { readRules } from '../../rules/read_rules';
+import { legacyMigrate } from '../../rules/utils';
+import { getQueryRuleParams } from '../../schemas/rule_schemas.mock';
 
 jest.mock('../../../machine_learning/authz', () => mockMlAuthzFactory.create());
 jest.mock('../../rules/read_rules', () => ({ readRules: jest.fn() }));
 
+jest.mock('../../rules/utils', () => {
+  const actual = jest.requireActual('../../rules/utils');
+  return {
+    ...actual,
+    legacyMigrate: jest.fn(),
+  };
+});
+
 describe('perform_bulk_action', () => {
   const readRulesMock = readRules as jest.Mock;
   let server: ReturnType<typeof serverMock.create>;
@@ -40,6 +51,8 @@ describe('perform_bulk_action', () => {
     logger = loggingSystemMock.createLogger();
     ({ clients, context } = requestContextMock.createTools());
     ml = mlServicesMock.createSetupContract();
+    (legacyMigrate as jest.Mock).mockResolvedValue(getRuleMock(getQueryRuleParams()));
+
     clients.rulesClient.find.mockResolvedValue(getFindResultWithSingleHit());
     performBulkActionRoute(server.router, ml, logger);
   });
@@ -220,7 +233,10 @@ describe('perform_bulk_action', () => {
       readRulesMock.mockImplementationOnce(() =>
         Promise.resolve({ ...mockRule, params: { ...mockRule.params, type: 'machine_learning' } })
       );
-
+      (legacyMigrate as jest.Mock).mockResolvedValue({
+        ...mockRule,
+        params: { ...mockRule.params, type: 'machine_learning' },
+      });
       const request = requestMock.create({
         method: 'patch',
         path: DETECTION_ENGINE_RULES_BULK_ACTION,
@@ -271,7 +287,10 @@ describe('perform_bulk_action', () => {
       readRulesMock.mockImplementationOnce(() =>
         Promise.resolve({ ...mockRule, params: { ...mockRule.params, index: ['index-*'] } })
       );
-
+      (legacyMigrate as jest.Mock).mockResolvedValue({
+        ...mockRule,
+        params: { ...mockRule.params, index: ['index-*'] },
+      });
       const request = requestMock.create({
         method: 'patch',
         path: DETECTION_ENGINE_RULES_BULK_ACTION,

x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/update_rules_bulk_route.test.ts

@@ -21,9 +21,18 @@ import { BulkError } from '../utils';
 import { getCreateRulesSchemaMock } from '../../../../../common/detection_engine/schemas/request/rule_schemas.mock';
 import { getQueryRuleParams } from '../../schemas/rule_schemas.mock';
 import { loggingSystemMock } from '@kbn/core/server/mocks';
+import { legacyMigrate } from '../../rules/utils';
 
 jest.mock('../../../machine_learning/authz', () => mockMlAuthzFactory.create());
 
+jest.mock('../../rules/utils', () => {
+  const actual = jest.requireActual('../../rules/utils');
+  return {
+    ...actual,
+    legacyMigrate: jest.fn(),
+  };
+});
+
 describe('update_rules_bulk', () => {
   let server: ReturnType<typeof serverMock.create>;
   let { clients, context } = requestContextMock.createTools();
@@ -40,6 +49,8 @@ describe('update_rules_bulk', () => {
 
     clients.appClient.getSignalsIndex.mockReturnValue('.siem-signals-test-index');
 
+    (legacyMigrate as jest.Mock).mockResolvedValue(getRuleMock(getQueryRuleParams()));
+
     updateRulesBulkRoute(server.router, ml, logger);
   });
 
@@ -54,6 +65,8 @@ describe('update_rules_bulk', () => {
 
     test('returns 200 as a response when updating a single rule that does not exist', async () => {
       clients.rulesClient.find.mockResolvedValue(getEmptyFindResult());
+      (legacyMigrate as jest.Mock).mockResolvedValue(null);
+
       const expected: BulkError[] = [
         {
           error: { message: 'rule_id: "rule-1" not found', status_code: 404 },
@@ -116,6 +129,8 @@ describe('update_rules_bulk', () => {
 
   describe('request validation', () => {
     test('rejects payloads with no ID', async () => {
+      (legacyMigrate as jest.Mock).mockResolvedValue(null);
+
       const noIdRequest = requestMock.create({
         method: 'put',
         path: DETECTION_ENGINE_RULES_BULK_UPDATE,

x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/update_rules_route.test.ts

@@ -21,9 +21,18 @@ import { DETECTION_ENGINE_RULES_URL } from '../../../../../common/constants';
 import { updateRulesRoute } from './update_rules_route';
 import { getUpdateRulesSchemaMock } from '../../../../../common/detection_engine/schemas/request/rule_schemas.mock';
 import { getQueryRuleParams } from '../../schemas/rule_schemas.mock';
+import { legacyMigrate } from '../../rules/utils';
 
 jest.mock('../../../machine_learning/authz', () => mockMlAuthzFactory.create());
 
+jest.mock('../../rules/utils', () => {
+  const actual = jest.requireActual('../../rules/utils');
+  return {
+    ...actual,
+    legacyMigrate: jest.fn(),
+  };
+});
+
 describe('update_rules', () => {
   let server: ReturnType<typeof serverMock.create>;
   let { clients, context } = requestContextMock.createTools();
@@ -42,6 +51,8 @@ describe('update_rules', () => {
     );
     clients.appClient.getSignalsIndex.mockReturnValue('.siem-signals-test-index');
 
+    (legacyMigrate as jest.Mock).mockResolvedValue(getRuleMock(getQueryRuleParams()));
+
     updateRulesRoute(server.router, ml);
   });
 
@@ -56,6 +67,7 @@ describe('update_rules', () => {
 
     test('returns 404 when updating a single rule that does not exist', async () => {
       clients.rulesClient.find.mockResolvedValue(getEmptyFindResult());
+      (legacyMigrate as jest.Mock).mockResolvedValue(null);
       const response = await server.inject(
         getUpdateRequest(),
         requestContextMock.convertContext(context)
@@ -69,6 +81,7 @@ describe('update_rules', () => {
     });
 
     test('returns error when updating non-rule', async () => {
+      (legacyMigrate as jest.Mock).mockResolvedValue(null);
       clients.rulesClient.find.mockResolvedValue(nonRuleFindResult());
       const response = await server.inject(
         getUpdateRequest(),

x-pack/plugins/security_solution/server/lib/detection_engine/rules/update_prepacked_rules.test.ts

@@ -7,14 +7,24 @@
 
 import { rulesClientMock } from '@kbn/alerting-plugin/server/mocks';
 import { savedObjectsClientMock } from '@kbn/core/server/mocks';
-import { getFindResultWithSingleHit } from '../routes/__mocks__/request_responses';
+import { getRuleMock, getFindResultWithSingleHit } from '../routes/__mocks__/request_responses';
 import { updatePrepackagedRules } from './update_prepacked_rules';
 import { patchRules } from './patch_rules';
 import { getAddPrepackagedRulesSchemaDecodedMock } from '../../../../common/detection_engine/schemas/request/add_prepackaged_rules_schema.mock';
 import { ruleExecutionLogMock } from '../rule_execution_log/__mocks__';
+import { legacyMigrate } from './utils';
+import { getQueryRuleParams } from '../schemas/rule_schemas.mock';
 
 jest.mock('./patch_rules');
 
+jest.mock('./utils', () => {
+  const actual = jest.requireActual('./utils');
+  return {
+    ...actual,
+    legacyMigrate: jest.fn(),
+  };
+});
+
 describe('updatePrepackagedRules', () => {
   let rulesClient: ReturnType<typeof rulesClientMock.create>;
   let savedObjectsClient: ReturnType<typeof savedObjectsClientMock.create>;
@@ -24,6 +34,8 @@ describe('updatePrepackagedRules', () => {
     rulesClient = rulesClientMock.create();
     savedObjectsClient = savedObjectsClientMock.create();
     ruleExecutionLog = ruleExecutionLogMock.forRoutes.create();
+
+    (legacyMigrate as jest.Mock).mockResolvedValue(getRuleMock(getQueryRuleParams()));
   });
 
   it('should omit actions and enabled when calling patchRules', async () => {