[Response Ops][Alerting] Fixing bug with using runSoon on pre-8.x rule (#142505) (#142550)

* Running task using scheduled task id. Adding functional test

* dont run if rule is disable

* Fixing i18n

(cherry picked from commit e007ad6df5)

Co-authored-by: Ying Mao <ying.mao@elastic.co>

x-pack/plugins/alerting/server/rules_client/rules_client.ts

@@ -41,7 +41,11 @@ import {
   InvalidateAPIKeyResult as SecurityPluginInvalidateAPIKeyResult,
 } from '@kbn/security-plugin/server';
 import { EncryptedSavedObjectsClient } from '@kbn/encrypted-saved-objects-plugin/server';
-import { TaskManagerStartContract, TaskStatus } from '@kbn/task-manager-plugin/server';
+import {
+  ConcreteTaskInstance,
+  TaskManagerStartContract,
+  TaskStatus,
+} from '@kbn/task-manager-plugin/server';
 import {
   IEvent,
   IEventLogClient,
@@ -2978,9 +2982,27 @@ export class RulesClient {
 
     this.ruleTypeRegistry.ensureRuleTypeEnabled(attributes.alertTypeId);
 
-    const taskDoc = attributes.scheduledTaskId
-      ? await this.taskManager.get(attributes.scheduledTaskId)
-      : null;
+    // Check that the rule is enabled
+    if (!attributes.enabled) {
+      return i18n.translate('xpack.alerting.rulesClient.runSoon.disabledRuleError', {
+        defaultMessage: 'Error running rule: rule is disabled',
+      });
+    }
+
+    let taskDoc: ConcreteTaskInstance | null = null;
+    try {
+      taskDoc = attributes.scheduledTaskId
+        ? await this.taskManager.get(attributes.scheduledTaskId)
+        : null;
+    } catch (err) {
+      return i18n.translate('xpack.alerting.rulesClient.runSoon.getTaskError', {
+        defaultMessage: 'Error running rule: {errMessage}',
+        values: {
+          errMessage: err.message,
+        },
+      });
+    }
+
     if (
       taskDoc &&
       (taskDoc.status === TaskStatus.Claiming || taskDoc.status === TaskStatus.Running)
@@ -2990,7 +3012,16 @@ export class RulesClient {
       });
     }
 
-    await this.taskManager.runSoon(id);
+    try {
+      await this.taskManager.runSoon(attributes.scheduledTaskId ? attributes.scheduledTaskId : id);
+    } catch (err) {
+      return i18n.translate('xpack.alerting.rulesClient.runSoon.runSoonError', {
+        defaultMessage: 'Error running rule: {errMessage}',
+        values: {
+          errMessage: err.message,
+        },
+      });
+    }
   }
 
   public async listAlertTypes() {

x-pack/plugins/alerting/server/rules_client/tests/run_soon.test.ts

@@ -59,7 +59,7 @@ describe('runSoon()', () => {
       consumer: 'myApp',
       schedule: { interval: '10s' },
       alertTypeId: 'myType',
-      enabled: false,
+      enabled: true,
       apiKey: 'MTIzOmFiYw==',
       apiKeyOwner: 'elastic',
       actions: [
@@ -179,7 +179,7 @@ describe('runSoon()', () => {
   });
 
   test('does not run a rule if that rule is already running', async () => {
-    taskManager.get.mockResolvedValue({
+    taskManager.get.mockResolvedValueOnce({
       id: '1',
       scheduledAt: new Date(),
       attempts: 0,
@@ -196,4 +196,32 @@ describe('runSoon()', () => {
     expect(message).toBe('Rule is already running');
     expect(taskManager.runSoon).not.toHaveBeenCalled();
   });
+
+  test('does not run a rule if that rule is disabled', async () => {
+    unsecuredSavedObjectsClient.get.mockResolvedValue({
+      ...existingRule,
+      attributes: {
+        ...existingRule.attributes,
+        enabled: false,
+      },
+    });
+    const message = await rulesClient.runSoon({ id: '1' });
+    expect(message).toBe('Error running rule: rule is disabled');
+    expect(taskManager.get).not.toHaveBeenCalled();
+    expect(taskManager.runSoon).not.toHaveBeenCalled();
+  });
+
+  test('gracefully handles errors getting task document', async () => {
+    taskManager.get.mockRejectedValueOnce(new Error('oh no!'));
+    const message = await rulesClient.runSoon({ id: '1' });
+    expect(message).toBe('Error running rule: oh no!');
+    expect(taskManager.runSoon).not.toHaveBeenCalled();
+  });
+
+  test('gracefully handles errors calling runSoon', async () => {
+    taskManager.runSoon.mockRejectedValueOnce(new Error('fail!'));
+    const message = await rulesClient.runSoon({ id: '1' });
+    expect(message).toBe('Error running rule: fail!');
+    expect(taskManager.runSoon).toHaveBeenCalled();
+  });
 });

x-pack/test/alerting_api_integration/spaces_only/tests/alerting/index.ts

@@ -49,6 +49,7 @@ export default function alertingTests({ loadTestFile, getService }: FtrProviderC
     loadTestFile(require.resolve('./bulk_edit'));
     loadTestFile(require.resolve('./capped_action_type'));
     loadTestFile(require.resolve('./scheduled_task_id'));
+    loadTestFile(require.resolve('./run_soon'));
     // Do not place test files here, due to https://github.com/elastic/kibana/issues/123059
 
     // note that this test will destroy existing spaces

x-pack/test/alerting_api_integration/spaces_only/tests/alerting/run_soon.ts

@@ -0,0 +1,104 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+import expect from '@kbn/expect';
+import { getUrlPrefix, getTestRuleData, ObjectRemover } from '../../../common/lib';
+import { FtrProviderContext } from '../../../common/ftr_provider_context';
+
+const LOADED_RULE_ID = '74f3e6d7-b7bb-477d-ac28-92ee22728e6e';
+
+// eslint-disable-next-line import/no-default-export
+export default function createRunSoonTests({ getService }: FtrProviderContext) {
+  const supertest = getService('supertest');
+  const retry = getService('retry');
+  const es = getService('es');
+  const esArchiver = getService('esArchiver');
+
+  describe('runSoon', () => {
+    const objectRemover = new ObjectRemover(supertest);
+
+    before(async () => {
+      await esArchiver.load('x-pack/test/functional/es_archives/rules_scheduled_task_id');
+    });
+
+    afterEach(async () => {
+      await objectRemover.removeAll();
+    });
+
+    after(async () => {
+      await esArchiver.unload('x-pack/test/functional/es_archives/rules_scheduled_task_id');
+    });
+
+    it('should successfully run rule where scheduled task id is different than rule id', async () => {
+      await retry.try(async () => {
+        // Sometimes the rule may already be running. Try until it isn't
+        const response = await supertest
+          .post(`${getUrlPrefix(``)}/internal/alerting/rule/${LOADED_RULE_ID}/_run_soon`)
+          .set('kbn-xsrf', 'foo');
+        expect(response.status).to.eql(204);
+      });
+    });
+
+    it('should successfully run rule where scheduled task id is same as rule id', async () => {
+      const response = await supertest
+        .post(`${getUrlPrefix(``)}/api/alerting/rule`)
+        .set('kbn-xsrf', 'foo')
+        .send(getTestRuleData());
+
+      expect(response.status).to.eql(200);
+      objectRemover.add('default', response.body.id, 'rule', 'alerting');
+
+      const runSoonResponse = await supertest
+        .post(`${getUrlPrefix(``)}/internal/alerting/rule/${response.body.id}/_run_soon`)
+        .set('kbn-xsrf', 'foo');
+      expect(runSoonResponse.status).to.eql(204);
+    });
+
+    it('should return message when task does not exist for rule', async () => {
+      const response = await supertest
+        .post(`${getUrlPrefix(``)}/api/alerting/rule`)
+        .set('kbn-xsrf', 'foo')
+        .send(getTestRuleData());
+
+      expect(response.status).to.eql(200);
+      objectRemover.add('default', response.body.id, 'rule', 'alerting');
+
+      await es.delete({
+        id: `task:${response.body.id}`,
+        index: '.kibana_task_manager',
+      });
+
+      const runSoonResponse = await supertest
+        .post(`${getUrlPrefix(``)}/internal/alerting/rule/${response.body.id}/_run_soon`)
+        .set('kbn-xsrf', 'foo');
+      expect(runSoonResponse.status).to.eql(200);
+      expect(runSoonResponse.text).to.eql(
+        `Error running rule: Saved object [task/${response.body.id}] not found`
+      );
+    });
+
+    it('should return message when rule is disabled', async () => {
+      const response = await supertest
+        .post(`${getUrlPrefix(``)}/api/alerting/rule`)
+        .set('kbn-xsrf', 'foo')
+        .send(getTestRuleData());
+
+      expect(response.status).to.eql(200);
+      objectRemover.add('default', response.body.id, 'rule', 'alerting');
+
+      await supertest
+        .post(`${getUrlPrefix(``)}/api/alerting/rule/${response.body.id}/_disable`)
+        .set('kbn-xsrf', 'foo');
+
+      const runSoonResponse = await supertest
+        .post(`${getUrlPrefix(``)}/internal/alerting/rule/${response.body.id}/_run_soon`)
+        .set('kbn-xsrf', 'foo');
+      expect(runSoonResponse.status).to.eql(200);
+      expect(runSoonResponse.text).to.eql(`Error running rule: rule is disabled`);
+    });
+  });
+}