[RAM] Add modal when disabling rule to prompt user to untrack alerts (#…

…175363) ## Summary Adds a modal to all of the stack management disable flows to ask the user if they want to untrack orphaned alerts - Rules list -> bulk disable - Rules list -> State dropdown - Rules list -> Action items dropdown - Rule details ### To Test 1. Navigate to rules list or rule details 2. Try to create a rule that generate alerts 3. Disable the rule, use the modal to determine if the alerts should be untracked 4. Check the alerts to see if they were untracked or still tracked (depending on step 3) ![image](https://github.com/elastic/kibana/assets/74562234/1d980f51-d243-4742-b856-12a58369c0f9) ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Xavier Mouligneau <[email protected]>
elastic · Feb 12, 2024 · 7a5fcc9 · 7a5fcc9
1 parent 955dd69
commit 7a5fcc9
Show file tree

Hide file tree

Showing 35 changed files with 1,100 additions and 390 deletions.
diff --git a/x-pack/plugins/alerting/common/routes/rule/apis/bulk_disable/schemas/v1.ts b/x-pack/plugins/alerting/common/routes/rule/apis/bulk_disable/schemas/v1.ts
@@ -10,4 +10,5 @@ import { schema } from '@kbn/config-schema';
 export const bulkDisableRulesRequestBodySchema = schema.object({
   filter: schema.maybe(schema.string()),
   ids: schema.maybe(schema.arrayOf(schema.string(), { minSize: 1, maxSize: 1000 })),
+  untrack: schema.maybe(schema.boolean({ defaultValue: false })),
 });
diff --git a/.../plugins/alerting/server/application/rule/methods/bulk_disable/bulk_disable_rules.test.ts b/.../plugins/alerting/server/application/rule/methods/bulk_disable/bulk_disable_rules.test.ts
@@ -41,14 +41,6 @@ import {
 import { migrateLegacyActions } from '../../../../rules_client/lib';
 import { RULE_SAVED_OBJECT_TYPE } from '../../../../saved_objects';
 
-jest.mock('../../../../task_runner/alert_task_instance', () => ({
-  taskInstanceToAlertTaskInstance: jest.fn(),
-}));
-
-const { taskInstanceToAlertTaskInstance } = jest.requireMock(
-  '../../../../task_runner/alert_task_instance'
-);
-
 jest.mock('../../../../rules_client/lib/siem_legacy_actions/migrate_legacy_actions', () => {
   return {
     migrateLegacyActions: jest.fn().mockResolvedValue({
@@ -63,6 +55,12 @@ jest.mock('../../../../invalidate_pending_api_keys/bulk_mark_api_keys_for_invali
   bulkMarkApiKeysForInvalidation: jest.fn(),
 }));
 
+jest.mock('../../../../rules_client/lib/untrack_rule_alerts', () => ({
+  untrackRuleAlerts: jest.fn(),
+}));
+
+const { untrackRuleAlerts } = jest.requireMock('../../../../rules_client/lib/untrack_rule_alerts');
+
 const taskManager = taskManagerMock.createStart();
 const ruleTypeRegistry = ruleTypeRegistryMock.create();
 const unsecuredSavedObjectsClient = savedObjectsClientMock.create();
@@ -192,6 +190,76 @@ describe('bulkDisableRules', () => {
     });
   });
 
+  test('should call untrack alert if untrack is true', async () => {
+    unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({
+      saved_objects: [disabledRuleForBulkDisable1, disabledRuleForBulkDisable2],
+    });
+
+    const result = await rulesClient.bulkDisableRules({ filter: 'fake_filter', untrack: true });
+
+    expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(1);
+    expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledWith(
+      expect.arrayContaining([
+        expect.objectContaining({
+          id: 'id1',
+          attributes: expect.objectContaining({
+            enabled: false,
+          }),
+        }),
+        expect.objectContaining({
+          id: 'id2',
+          attributes: expect.objectContaining({
+            enabled: false,
+          }),
+        }),
+      ]),
+      { overwrite: true }
+    );
+
+    expect(result).toStrictEqual({
+      errors: [],
+      rules: [returnedRuleForBulkDisable1, returnedRuleForBulkDisable2],
+      total: 2,
+    });
+
+    expect(untrackRuleAlerts).toHaveBeenCalled();
+  });
+
+  test('should not call untrack alert if untrack is false', async () => {
+    unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({
+      saved_objects: [disabledRuleForBulkDisable1, disabledRuleForBulkDisable2],
+    });
+
+    const result = await rulesClient.bulkDisableRules({ filter: 'fake_filter', untrack: true });
+
+    expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(1);
+    expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledWith(
+      expect.arrayContaining([
+        expect.objectContaining({
+          id: 'id1',
+          attributes: expect.objectContaining({
+            enabled: false,
+          }),
+        }),
+        expect.objectContaining({
+          id: 'id2',
+          attributes: expect.objectContaining({
+            enabled: false,
+          }),
+        }),
+      ]),
+      { overwrite: true }
+    );
+
+    expect(result).toStrictEqual({
+      errors: [],
+      rules: [returnedRuleForBulkDisable1, returnedRuleForBulkDisable2],
+      total: 2,
+    });
+
+    expect(untrackRuleAlerts).toHaveBeenCalled();
+  });
+
   test('should try to disable rules, one successful and one with 500 error', async () => {
     unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({
       saved_objects: [disabledRuleForBulkDisable1, savedObjectWith500Error],
@@ -585,51 +653,6 @@ describe('bulkDisableRules', () => {
     });
   });
 
-  describe('recoverRuleAlerts', () => {
-    beforeEach(() => {
-      taskInstanceToAlertTaskInstance.mockImplementation(() => ({
-        state: {
-          alertInstances: {
-            '1': {
-              meta: {
-                lastScheduledActions: {
-                  group: 'default',
-                  date: new Date().toISOString(),
-                },
-              },
-              state: { bar: false },
-            },
-          },
-        },
-      }));
-    });
-    test('should call logEvent', async () => {
-      unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({
-        saved_objects: [disabledRuleForBulkDisable1, disabledRuleForBulkDisable2],
-      });
-
-      await rulesClient.bulkDisableRules({ filter: 'fake_filter' });
-
-      expect(eventLogger.logEvent).toHaveBeenCalledTimes(2);
-    });
-
-    test('should call logger.warn', async () => {
-      eventLogger.logEvent.mockImplementation(() => {
-        throw new Error('UPS');
-      });
-      unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({
-        saved_objects: [disabledRuleForBulkDisable1, disabledRuleForBulkDisable2],
-      });
-
-      await rulesClient.bulkDisableRules({ filter: 'fake_filter' });
-
-      expect(logger.warn).toHaveBeenCalledTimes(2);
-      expect(logger.warn).toHaveBeenLastCalledWith(
-        "rulesClient.disable('id2') - Could not write untrack events - UPS"
-      );
-    });
-  });
-
   describe('legacy actions migration for SIEM', () => {
     test('should call migrateLegacyActions', async () => {
       encryptedSavedObjects.createPointInTimeFinderDecryptedAsInternalUser = jest

diff --git a/x-pack/plugins/alerting/server/application/rule/methods/bulk_disable/bulk_disable_rules.ts b/x-pack/plugins/alerting/server/application/rule/methods/bulk_disable/bulk_disable_rules.ts
@@ -50,7 +50,7 @@ export const bulkDisableRules = async <Params extends RuleParams>(
     throw Boom.badRequest(`Error validating bulk disable data - ${error.message}`);
   }
 
-  const { ids, filter } = options;
+  const { ids, filter, untrack = false } = options;
 
   const kueryNodeFilter = ids ? convertRuleIdsToKueryNode(ids) : buildKueryNodeFilter(filter);
   const authorizationFilter = await getAuthorizationFilter(context, { action: 'DISABLE' });
@@ -72,7 +72,7 @@ export const bulkDisableRules = async <Params extends RuleParams>(
         action: 'DISABLE',
         logger: context.logger,
         bulkOperation: (filterKueryNode: KueryNode | null) =>
-          bulkDisableRulesWithOCC(context, { filter: filterKueryNode }),
+          bulkDisableRulesWithOCC(context, { filter: filterKueryNode, untrack }),
         filter: kueryNodeFilterWithAuth,
       })
   );
@@ -120,7 +120,13 @@ export const bulkDisableRules = async <Params extends RuleParams>(
 
 const bulkDisableRulesWithOCC = async (
   context: RulesClientContext,
-  { filter }: { filter: KueryNode | null }
+  {
+    filter,
+    untrack = false,
+  }: {
+    filter: KueryNode | null;
+    untrack: boolean;
+  }
 ) => {
   const additionalFilter = nodeBuilder.is('alert.attributes.enabled', 'true');
 
@@ -151,7 +157,9 @@ const bulkDisableRulesWithOCC = async (
       for await (const response of rulesFinder.find()) {
         await pMap(response.saved_objects, async (rule) => {
           try {
-            await untrackRuleAlerts(context, rule.id, rule.attributes);
+            if (untrack) {
+              await untrackRuleAlerts(context, rule.id, rule.attributes);
+            }
 
             if (rule.attributes.name) {
               ruleNameToRuleIdMapping[rule.id] = rule.attributes.name;

diff --git a/x-pack/plugins/alerting/server/application/rule/methods/bulk_disable/schemas/index.ts b/x-pack/plugins/alerting/server/application/rule/methods/bulk_disable/schemas/index.ts
@@ -10,4 +10,5 @@ import { schema } from '@kbn/config-schema';
 export const bulkDisableRulesRequestBodySchema = schema.object({
   filter: schema.maybe(schema.string()),
   ids: schema.maybe(schema.arrayOf(schema.string(), { minSize: 1, maxSize: 1000 })),
+  untrack: schema.maybe(schema.boolean({ defaultValue: false })),
 });
diff --git a/x-pack/plugins/alerting/server/routes/disable_rule.test.ts b/x-pack/plugins/alerting/server/routes/disable_rule.test.ts
@@ -52,6 +52,7 @@ describe('disableRuleRoute', () => {
       Array [
         Object {
           "id": "1",
+          "untrack": false,
         },
       ]
     `);

diff --git a/x-pack/plugins/alerting/server/routes/disable_rule.ts b/x-pack/plugins/alerting/server/routes/disable_rule.ts
@@ -15,6 +15,14 @@ const paramSchema = schema.object({
   id: schema.string(),
 });
 
+const bodySchema = schema.nullable(
+  schema.maybe(
+    schema.object({
+      untrack: schema.maybe(schema.boolean({ defaultValue: false })),
+    })
+  )
+);
+
 export const disableRuleRoute = (
   router: IRouter<AlertingRequestHandlerContext>,
   licenseState: ILicenseState
@@ -24,14 +32,16 @@ export const disableRuleRoute = (
       path: `${BASE_ALERTING_API_PATH}/rule/{id}/_disable`,
       validate: {
         params: paramSchema,
+        body: bodySchema,
       },
     },
     router.handleLegacyErrors(
       verifyAccessAndContext(licenseState, async function (context, req, res) {
         const rulesClient = (await context.alerting).getRulesClient();
         const { id } = req.params;
+        const { untrack = false } = req.body || {};
         try {
-          await rulesClient.disable({ id });
+          await rulesClient.disable({ id, untrack });
           return res.noContent();
         } catch (e) {
           if (e instanceof RuleTypeDisabledError) {

diff --git a/x-pack/plugins/alerting/server/routes/rule/apis/bulk_disable/bulk_disable_rules_route.ts b/x-pack/plugins/alerting/server/routes/rule/apis/bulk_disable/bulk_disable_rules_route.ts
@@ -39,10 +39,10 @@ export const bulkDisableRulesRoute = ({
           const rulesClient = (await context.alerting).getRulesClient();
 
           const body: BulkDisableRulesRequestBodyV1 = req.body;
-          const { filter, ids } = body;
+          const { filter, ids, untrack } = body;
 
           try {
-            const bulkDisableResults = await rulesClient.bulkDisableRules({ filter, ids });
+            const bulkDisableResults = await rulesClient.bulkDisableRules({ filter, ids, untrack });
 
             const resultBody: BulkDisableRulesResponseV1<RuleParamsV1> = {
               body: {

diff --git a/x-pack/plugins/alerting/server/rules_client/methods/disable.ts b/x-pack/plugins/alerting/server/rules_client/methods/disable.ts
@@ -15,15 +15,33 @@ import { untrackRuleAlerts, updateMeta, migrateLegacyActions } from '../lib';
 import { RuleAttributes } from '../../data/rule/types';
 import { RULE_SAVED_OBJECT_TYPE } from '../../saved_objects';
 
-export async function disable(context: RulesClientContext, { id }: { id: string }): Promise<void> {
+export async function disable(
+  context: RulesClientContext,
+  {
+    id,
+    untrack = false,
+  }: {
+    id: string;
+    untrack?: boolean;
+  }
+): Promise<void> {
   return await retryIfConflicts(
     context.logger,
     `rulesClient.disable('${id}')`,
-    async () => await disableWithOCC(context, { id })
+    async () => await disableWithOCC(context, { id, untrack })
   );
 }
 
-async function disableWithOCC(context: RulesClientContext, { id }: { id: string }) {
+async function disableWithOCC(
+  context: RulesClientContext,
+  {
+    id,
+    untrack = false,
+  }: {
+    id: string;
+    untrack?: boolean;
+  }
+) {
   let attributes: RawRule;
   let version: string | undefined;
   let references: SavedObjectReference[];
@@ -70,7 +88,9 @@ async function disableWithOCC(context: RulesClientContext, { id }: { id: string
     throw error;
   }
 
-  await untrackRuleAlerts(context, id, attributes as RuleAttributes);
+  if (untrack) {
+    await untrackRuleAlerts(context, id, attributes as RuleAttributes);
+  }
 
   context.auditLogger?.log(
     ruleAuditEvent({

diff --git a/x-pack/plugins/alerting/server/rules_client/rules_client.ts b/x-pack/plugins/alerting/server/rules_client/rules_client.ts
@@ -160,7 +160,7 @@ export class RulesClient {
   public updateApiKey = (options: { id: string }) => updateApiKey(this.context, options);
 
   public enable = (options: { id: string }) => enable(this.context, options);
-  public disable = (options: { id: string }) => disable(this.context, options);
+  public disable = (options: { id: string; untrack?: boolean }) => disable(this.context, options);
 
   public snooze = (options: SnoozeRuleOptions) => snoozeRule(this.context, options);
   public unsnooze = (options: UnsnoozeParams) => unsnoozeRule(this.context, options);

diff --git a/x-pack/plugins/alerting/server/rules_client/tests/disable.test.ts b/x-pack/plugins/alerting/server/rules_client/tests/disable.test.ts
@@ -299,7 +299,7 @@ describe('disable()', () => {
       },
       ownerId: null,
     });
-    await rulesClient.disable({ id: '1' });
+    await rulesClient.disable({ id: '1', untrack: true });
     expect(unsecuredSavedObjectsClient.get).not.toHaveBeenCalled();
     expect(encryptedSavedObjects.getDecryptedAsInternalUser).toHaveBeenCalledWith(
       RULE_SAVED_OBJECT_TYPE,
@@ -388,7 +388,7 @@ describe('disable()', () => {
 
   test('disables the rule even if unable to retrieve task manager doc to generate untrack event log events', async () => {
     taskManager.get.mockRejectedValueOnce(new Error('Fail'));
-    await rulesClient.disable({ id: '1' });
+    await rulesClient.disable({ id: '1', untrack: true });
     expect(unsecuredSavedObjectsClient.get).not.toHaveBeenCalled();
     expect(encryptedSavedObjects.getDecryptedAsInternalUser).toHaveBeenCalledWith(
       RULE_SAVED_OBJECT_TYPE,
@@ -440,6 +440,55 @@ describe('disable()', () => {
     );
   });
 
+  test('should not untrack rule alert if untrack is false', async () => {
+    await rulesClient.disable({ id: '1', untrack: false });
+    expect(unsecuredSavedObjectsClient.get).not.toHaveBeenCalled();
+    expect(encryptedSavedObjects.getDecryptedAsInternalUser).toHaveBeenCalledWith(
+      RULE_SAVED_OBJECT_TYPE,
+      '1',
+      {
+        namespace: 'default',
+      }
+    );
+    expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledWith(
+      RULE_SAVED_OBJECT_TYPE,
+      '1',
+      {
+        consumer: 'myApp',
+        schedule: { interval: '10s' },
+        alertTypeId: 'myType',
+        enabled: false,
+        meta: {
+          versionApiKeyLastmodified: 'v7.10.0',
+        },
+        revision: 0,
+        scheduledTaskId: '1',
+        apiKey: 'MTIzOmFiYw==',
+        apiKeyOwner: 'elastic',
+        updatedAt: '2019-02-12T21:01:22.479Z',
+        updatedBy: 'elastic',
+        actions: [
+          {
+            group: 'default',
+            id: '1',
+            actionTypeId: '1',
+            actionRef: '1',
+            params: {
+              foo: true,
+            },
+          },
+        ],
+        nextRun: null,
+      },
+      {
+        version: '123',
+      }
+    );
+    expect(taskManager.bulkDisable).toHaveBeenCalledWith(['1'], false);
+    expect(taskManager.get).not.toHaveBeenCalled();
+    expect(taskManager.removeIfExists).not.toHaveBeenCalled();
+  });
+
   test('falls back when getDecryptedAsInternalUser throws an error', async () => {
     encryptedSavedObjects.getDecryptedAsInternalUser.mockRejectedValueOnce(new Error('Fail'));
     await rulesClient.disable({ id: '1' });
-Original file line number
+Diff line change
@@ Expand Up / @@ -52,6 +52,7 @@ describe('disableRuleRoute', () => { @@
           Array [
             Object {
               "id": "1",
+              "untrack": false,
             },
           ]
         `);
@@ Expand Down @@