Skip to content

Commit

Permalink
[Response Ops][Alerting] Fixing bug with using runSoon on pre-8.x r…
Browse files Browse the repository at this point in the history
…ule (#142505)

* Running task using scheduled task id. Adding functional test

* dont run if rule is disable

* Fixing i18n
  • Loading branch information
ymao1 authored Oct 4, 2022
1 parent 999bc84 commit e007ad6
Show file tree
Hide file tree
Showing 4 changed files with 171 additions and 7 deletions.
41 changes: 36 additions & 5 deletions x-pack/plugins/alerting/server/rules_client/rules_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -2977,9 +2981,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)
Expand All @@ -2989,7 +3011,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() {
Expand Down
32 changes: 30 additions & 2 deletions x-pack/plugins/alerting/server/rules_client/tests/run_soon.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ describe('runSoon()', () => {
consumer: 'myApp',
schedule: { interval: '10s' },
alertTypeId: 'myType',
enabled: false,
enabled: true,
apiKey: 'MTIzOmFiYw==',
apiKeyOwner: 'elastic',
actions: [
Expand Down Expand Up @@ -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,
Expand All @@ -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();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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`);
});
});
}

0 comments on commit e007ad6

Please sign in to comment.