[Response Ops][Alerting] Delete unrecognized tasks when enabling a rule (#152975)

This commit is contained in:
Ying Mao 2023-03-10 22:22:19 -05:00 committed by GitHub
parent 01ba0270d9
commit c875a284af
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 435 additions and 8 deletions

View file

@ -10,7 +10,7 @@ import { KueryNode, nodeBuilder } from '@kbn/es-query';
import { SavedObjectsBulkUpdateObject } from '@kbn/core/server';
import { withSpan } from '@kbn/apm-utils';
import { Logger } from '@kbn/core/server';
import { TaskManagerStartContract } from '@kbn/task-manager-plugin/server';
import { TaskManagerStartContract, TaskStatus } from '@kbn/task-manager-plugin/server';
import { RawRule, IntervalSchedule } from '../../types';
import { convertRuleIdsToKueryNode } from '../../lib';
import { ruleAuditEvent, RuleAuditAction } from '../common/audit_events';
@ -36,10 +36,18 @@ const getShouldScheduleTask = async (
if (!scheduledTaskId) return true;
try {
// make sure scheduledTaskId exist
await withSpan({ name: 'getShouldScheduleTask', type: 'rules' }, () =>
context.taskManager.get(scheduledTaskId)
);
return false;
return await withSpan({ name: 'getShouldScheduleTask', type: 'rules' }, async () => {
const task = await context.taskManager.get(scheduledTaskId);
// Check whether task status is unrecognized. If so, we want to delete
// this task and create a fresh one
if (task.status === TaskStatus.Unrecognized) {
await context.taskManager.removeIfExists(scheduledTaskId);
return true;
}
return false;
});
} catch (err) {
return true;
}

View file

@ -5,6 +5,7 @@
* 2.0.
*/
import { TaskStatus } from '@kbn/task-manager-plugin/server';
import { RawRule, IntervalSchedule } from '../../types';
import { resetMonitoringLastRun, getNextRun } from '../../lib';
import { WriteOperations, AlertingAuthorizationEntity } from '../../authorization';
@ -110,7 +111,14 @@ async function enableWithOCC(context: RulesClientContext, { id }: { id: string }
if (attributes.scheduledTaskId) {
// If scheduledTaskId defined in rule SO, make sure it exists
try {
await context.taskManager.get(attributes.scheduledTaskId);
const task = await context.taskManager.get(attributes.scheduledTaskId);
// Check whether task status is unrecognized. If so, we want to delete
// this task and create a fresh one
if (task.status === TaskStatus.Unrecognized) {
await context.taskManager.removeIfExists(attributes.scheduledTaskId);
scheduledTaskIdToCreate = id;
}
} catch (err) {
scheduledTaskIdToCreate = id;
}

View file

@ -30,6 +30,7 @@ import {
returnedRule1,
returnedRule2,
} from './test_helpers';
import { TaskStatus } from '@kbn/task-manager-plugin/server';
jest.mock('../../invalidate_pending_api_keys/bulk_mark_api_keys_for_invalidation', () => ({
bulkMarkApiKeysForInvalidation: jest.fn(),
@ -379,7 +380,7 @@ describe('bulkEnableRules', () => {
});
describe('taskManager', () => {
test('should return task id if deleting task failed', async () => {
test('should return task id if enabling task failed', async () => {
unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({
saved_objects: [enabledRule1, enabledRule2],
});
@ -487,6 +488,245 @@ describe('bulkEnableRules', () => {
);
expect(logger.error).toBeCalledTimes(0);
});
test('should schedule task when scheduledTaskId is defined but task with that ID does not', async () => {
// One rule gets the task successfully, one rule doesn't so only one task should be scheduled
taskManager.get.mockRejectedValueOnce(new Error('Failed to get task!'));
taskManager.schedule.mockResolvedValueOnce({
id: 'id1',
taskType: 'alerting:fakeType',
scheduledAt: new Date(),
attempts: 1,
status: TaskStatus.Idle,
runAt: new Date(),
startedAt: null,
retryAt: null,
state: {},
params: {},
ownerId: null,
});
unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({
saved_objects: [enabledRule1, enabledRule2],
});
const result = await rulesClient.bulkEnableRules({ ids: ['id1', 'id2'] });
expect(taskManager.schedule).toHaveBeenCalledTimes(1);
expect(taskManager.schedule).toHaveBeenCalledWith({
id: 'id1',
taskType: `alerting:fakeType`,
params: {
alertId: 'id1',
spaceId: 'default',
consumer: 'fakeConsumer',
},
schedule: {
interval: '5m',
},
enabled: true,
state: {
alertInstances: {},
alertTypeState: {},
previousStartedAt: null,
},
scope: ['alerting'],
});
expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(1);
expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledWith(
expect.arrayContaining([
expect.objectContaining({
id: 'id1',
attributes: expect.objectContaining({
enabled: true,
}),
}),
expect.objectContaining({
id: 'id2',
attributes: expect.objectContaining({
enabled: true,
}),
}),
]),
{ overwrite: true }
);
expect(result).toStrictEqual({
errors: [],
rules: [returnedRule1, returnedRule2],
total: 2,
taskIdsFailedToBeEnabled: [],
});
});
test('should schedule task when scheduledTaskId is not defined', async () => {
encryptedSavedObjects.createPointInTimeFinderDecryptedAsInternalUser = jest
.fn()
.mockResolvedValueOnce({
close: jest.fn(),
find: function* asyncGenerator() {
yield {
saved_objects: [
{
...disabledRule1,
attributes: { ...disabledRule1.attributes, scheduledTaskId: null },
},
disabledRule2,
],
};
},
});
taskManager.schedule.mockResolvedValueOnce({
id: 'id1',
taskType: 'alerting:fakeType',
scheduledAt: new Date(),
attempts: 1,
status: TaskStatus.Idle,
runAt: new Date(),
startedAt: null,
retryAt: null,
state: {},
params: {},
ownerId: null,
});
unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({
saved_objects: [enabledRule1, enabledRule2],
});
const result = await rulesClient.bulkEnableRules({ ids: ['id1', 'id2'] });
expect(taskManager.schedule).toHaveBeenCalledTimes(1);
expect(taskManager.schedule).toHaveBeenCalledWith({
id: 'id1',
taskType: `alerting:fakeType`,
params: {
alertId: 'id1',
spaceId: 'default',
consumer: 'fakeConsumer',
},
schedule: {
interval: '5m',
},
enabled: true,
state: {
alertInstances: {},
alertTypeState: {},
previousStartedAt: null,
},
scope: ['alerting'],
});
expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(1);
expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledWith(
expect.arrayContaining([
expect.objectContaining({
id: 'id1',
attributes: expect.objectContaining({
enabled: true,
}),
}),
expect.objectContaining({
id: 'id2',
attributes: expect.objectContaining({
enabled: true,
}),
}),
]),
{ overwrite: true }
);
expect(result).toStrictEqual({
errors: [],
rules: [returnedRule1, returnedRule2],
total: 2,
taskIdsFailedToBeEnabled: [],
});
});
test('should schedule task when task with scheduledTaskId exists but is unrecognized', async () => {
taskManager.get.mockResolvedValueOnce({
id: 'task-123',
taskType: 'alerting:123',
scheduledAt: new Date(),
attempts: 1,
status: TaskStatus.Unrecognized,
runAt: new Date(),
startedAt: null,
retryAt: null,
state: {},
params: {
alertId: '1',
},
ownerId: null,
enabled: false,
});
taskManager.schedule.mockResolvedValueOnce({
id: 'id1',
taskType: 'alerting:fakeType',
scheduledAt: new Date(),
attempts: 1,
status: TaskStatus.Idle,
runAt: new Date(),
startedAt: null,
retryAt: null,
state: {},
params: {},
ownerId: null,
});
unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({
saved_objects: [enabledRule1, enabledRule2],
});
const result = await rulesClient.bulkEnableRules({ ids: ['id1', 'id2'] });
expect(taskManager.removeIfExists).toHaveBeenCalledTimes(1);
expect(taskManager.removeIfExists).toHaveBeenCalledWith('id1');
expect(taskManager.schedule).toHaveBeenCalledTimes(1);
expect(taskManager.schedule).toHaveBeenCalledWith({
id: 'id1',
taskType: `alerting:fakeType`,
params: {
alertId: 'id1',
spaceId: 'default',
consumer: 'fakeConsumer',
},
schedule: {
interval: '5m',
},
enabled: true,
state: {
alertInstances: {},
alertTypeState: {},
previousStartedAt: null,
},
scope: ['alerting'],
});
expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(1);
expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledWith(
expect.arrayContaining([
expect.objectContaining({
id: 'id1',
attributes: expect.objectContaining({
enabled: true,
}),
}),
expect.objectContaining({
id: 'id2',
attributes: expect.objectContaining({
enabled: true,
}),
}),
]),
{ overwrite: true }
);
expect(result).toStrictEqual({
errors: [],
rules: [returnedRule1, returnedRule2],
total: 2,
taskIdsFailedToBeEnabled: [],
});
});
});
describe('auditLogger', () => {

View file

@ -536,6 +536,53 @@ describe('enable()', () => {
});
});
test('schedules task when task with scheduledTaskId exists but is unrecognized', async () => {
taskManager.schedule.mockResolvedValueOnce({
id: '1',
taskType: 'alerting:123',
scheduledAt: new Date(),
attempts: 1,
status: TaskStatus.Idle,
runAt: new Date(),
startedAt: null,
retryAt: null,
state: {},
params: {},
ownerId: null,
});
taskManager.get.mockResolvedValue({ ...mockTask, status: TaskStatus.Unrecognized });
await rulesClient.enable({ id: '1' });
expect(unsecuredSavedObjectsClient.get).not.toHaveBeenCalled();
expect(encryptedSavedObjects.getDecryptedAsInternalUser).toHaveBeenCalledWith('alert', '1', {
namespace: 'default',
});
expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledTimes(2);
expect(taskManager.bulkEnable).not.toHaveBeenCalled();
expect(taskManager.removeIfExists).toHaveBeenCalledWith('task-123');
expect(taskManager.schedule).toHaveBeenCalledWith({
id: '1',
taskType: `alerting:myType`,
params: {
alertId: '1',
spaceId: 'default',
consumer: 'myApp',
},
schedule: {
interval: '10s',
},
enabled: true,
state: {
alertInstances: {},
alertTypeState: {},
previousStartedAt: null,
},
scope: ['alerting'],
});
expect(unsecuredSavedObjectsClient.update).toHaveBeenNthCalledWith(2, 'alert', '1', {
scheduledTaskId: '1',
});
});
test('throws error when scheduling task fails', async () => {
encryptedSavedObjects.getDecryptedAsInternalUser.mockResolvedValueOnce({
...existingRule,

View file

@ -9,6 +9,7 @@ import { taskManagerMock } from '@kbn/task-manager-plugin/server/mocks';
import { IEventLogClient } from '@kbn/event-log-plugin/server';
import { actionsClientMock } from '@kbn/actions-plugin/server/mocks';
import { eventLogClientMock } from '@kbn/event-log-plugin/server/mocks';
import { TaskStatus } from '@kbn/task-manager-plugin/server';
import { ConstructorOptions } from '../rules_client';
import { RuleTypeRegistry } from '../../rule_type_registry';
import { RecoveredActionGroup } from '../../../common';
@ -51,6 +52,22 @@ export function getBeforeSetup(
rulesClientParams.createAPIKey.mockResolvedValue({ apiKeysEnabled: false });
rulesClientParams.getUserName.mockResolvedValue('elastic');
taskManager.runSoon.mockResolvedValue({ id: '' });
taskManager.get.mockResolvedValue({
id: 'task-123',
taskType: 'alerting:123',
scheduledAt: new Date(),
attempts: 1,
status: TaskStatus.Idle,
runAt: new Date(),
startedAt: null,
retryAt: null,
state: {},
params: {
alertId: '1',
},
ownerId: null,
enabled: false,
});
taskManager.bulkRemoveIfExist.mockResolvedValue({
statuses: [{ id: 'taskId', type: 'alert', success: true }],
});

View file

@ -23,6 +23,7 @@ export default function createScheduledTaskIdTests({ getService }: FtrProviderCo
const supertest = getService('supertest');
const supertestWithoutAuth = getService('supertestWithoutAuth');
const esArchiver = getService('esArchiver');
const retry = getService('retry');
describe('scheduled task id', () => {
const objectRemover = new ObjectRemover(supertest);
@ -116,5 +117,36 @@ export default function createScheduledTaskIdTests({ getService }: FtrProviderCo
});
expect(taskRecord.task.enabled).to.eql(true);
});
it('deletes associated task for rule if task is unrecognized', async () => {
const RULE_ID = '46be60d4-ae63-48ed-ab6f-f4d9b4defacf';
// We've archived a disabled rule with a scheduled task ID that references
// a task with a removed task type. Task manager will mark the task as unrecognized.
// When we enable the rule, the unrecognized task should be removed and a new
// task created in its place
// scheduled task should exist and be unrecognized
await retry.try(async () => {
const taskRecordLoaded = await getScheduledTask(RULE_ID);
expect(taskRecordLoaded.task.status).to.equal('unrecognized');
});
// enable the rule
await supertestWithoutAuth
.post(`${getUrlPrefix(``)}/api/alerting/rule/${RULE_ID}/_enable`)
.set('kbn-xsrf', 'foo');
await retry.try(async () => {
const response = await supertestWithoutAuth.get(
`${getUrlPrefix(``)}/api/alerting/rule/${RULE_ID}`
);
expect(response.status).to.eql(200);
expect(response.body.enabled).to.be(true);
});
// new scheduled task should exist with ID and status should not be unrecognized
const newTaskRecordLoaded = await getScheduledTask(RULE_ID);
expect(newTaskRecordLoaded.task.status).not.to.equal('unrecognized');
});
});
}

View file

@ -71,4 +71,79 @@
"updated_at": "2021-11-05T16:21:37.629Z"
}
}
}
}
{
"type": "doc",
"value": {
"id": "alert:46be60d4-ae63-48ed-ab6f-f4d9b4defacf",
"index": ".kibana_1",
"source": {
"alert": {
"actions": [
],
"alertTypeId": "example.always-firing",
"apiKey": "QIUT8u0/kbOakEHSj50jDpVR90MrqOxanEscboYOoa8PxQvcA5jfHash+fqH3b+KNjJ1LpnBcisGuPkufY9j1e32gKzwGZV5Bfys87imHvygJvIM8uKiFF8bQ8Y4NTaxOJO9fAmZPrFy07ZcQMCAQz+DUTgBFqs=",
"apiKeyOwner": "elastic",
"consumer": "alerts",
"createdAt": "2020-06-17T15:35:38.497Z",
"createdBy": "elastic",
"enabled": false,
"muteAll": false,
"mutedInstanceIds": [
],
"name": "always-firing-alert",
"params": {
},
"schedule": {
"interval": "1m"
},
"scheduledTaskId": "46be60d4-ae63-48ed-ab6f-f4d9b4defacf",
"tags": [
],
"throttle": null,
"updatedBy": "elastic"
},
"migrationVersion": {
"alert": "7.16.0"
},
"references": [
],
"type": "alert",
"updated_at": "2020-06-17T15:35:39.839Z"
}
}
}
{
"type": "doc",
"value": {
"id": "task:46be60d4-ae63-48ed-ab6f-f4d9b4defacf",
"index": ".kibana_task_manager_1",
"source": {
"migrationVersion": {
"task": "7.16.0"
},
"task": {
"attempts": 0,
"ownerId": null,
"params": "{\"alertId\":\"46be60d4-ae63-48ed-ab6f-f4d9b4defacf\",\"spaceId\":\"default\"}",
"retryAt": null,
"runAt": "2021-11-05T16:21:52.148Z",
"schedule": {
"interval": "1m"
},
"scheduledAt": "2021-11-05T15:28:42.055Z",
"scope": [
"alerting"
],
"startedAt": null,
"status": "idle",
"taskType": "sampleTaskRemovedType"
},
"references": [],
"type": "task",
"updated_at": "2021-11-05T16:21:37.629Z"
}
}
}