Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RAM] System action in bulk delete #170741

Merged
merged 10 commits into from
Nov 14, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { actionsAuthorizationMock } from '@kbn/actions-plugin/server/mocks';
import { ActionsAuthorization } from '@kbn/actions-plugin/server';
import { auditLoggerMock } from '@kbn/security-plugin/server/audit/mocks';
import { loggerMock } from '@kbn/logging-mocks';
import { ActionsClient } from '@kbn/actions-plugin/server';
import { ruleTypeRegistryMock } from '../../../../rule_type_registry.mock';
import { alertingAuthorizationMock } from '../../../../authorization/alerting_authorization.mock';
import { RecoveredActionGroup } from '../../../../../common';
Expand All @@ -24,10 +25,14 @@ import {
enabledRuleForBulkOps1,
enabledRuleForBulkOps2,
enabledRuleForBulkOps3,
returnedRuleForBulkDelete1,
returnedRuleForBulkDelete2,
returnedRuleForBulkDelete3,
returnedRuleForBulkOps1,
returnedRuleForBulkOps2,
returnedRuleForBulkOps3,
siemRuleForBulkOps1,
enabledRuleForBulkOpsWithActions1,
enabledRuleForBulkOpsWithActions2,
returnedRuleForBulkEnableWithActions1,
returnedRuleForBulkEnableWithActions2,
} from '../../../../rules_client/tests/test_helpers';
import { migrateLegacyActions } from '../../../../rules_client/lib';
import { ConnectorAdapterRegistry } from '../../../../connector_adapters/connector_adapter_registry';
Expand Down Expand Up @@ -81,6 +86,7 @@ const rulesClientParams: jest.Mocked<ConstructorOptions> = {
isAuthenticationTypeAPIKey: jest.fn(),
getAuthenticationAPIKey: jest.fn(),
connectorAdapterRegistry: new ConnectorAdapterRegistry(),
isSystemAction: jest.fn(),
getAlertIndicesAlias: jest.fn(),
alertsService: null,
};
Expand All @@ -105,11 +111,12 @@ setGlobalDate();

describe('bulkDelete', () => {
let rulesClient: RulesClient;
let actionsClient: jest.Mocked<ActionsClient>;

const mockCreatePointInTimeFinderAsInternalUser = (
response = {
saved_objects: [enabledRuleForBulkOps1, enabledRuleForBulkOps2, enabledRuleForBulkOps3],
}
} as unknown
guskovaue marked this conversation as resolved.
Show resolved Hide resolved
) => {
encryptedSavedObjects.createPointInTimeFinderDecryptedAsInternalUser = jest
.fn()
Expand Down Expand Up @@ -161,6 +168,48 @@ describe('bulkDelete', () => {
},
validLegacyConsumers: [],
});

actionsClient = (await rulesClientParams.getActionsClient()) as jest.Mocked<ActionsClient>;
actionsClient.isSystemAction.mockImplementation((id: string) => id === 'system_action:id');
rulesClientParams.getActionsClient.mockResolvedValue(actionsClient);
});

test('should successfully delete two rule and return right actions', async () => {
mockCreatePointInTimeFinderAsInternalUser({
saved_objects: [enabledRuleForBulkOpsWithActions1, enabledRuleForBulkOpsWithActions2],
});
unsecuredSavedObjectsClient.bulkDelete.mockResolvedValue({
statuses: [
{ id: 'id1', type: 'alert', success: true },
{ id: 'id2', type: 'alert', success: true },
],
});

const result = await rulesClient.bulkDeleteRules({ filter: 'fake_filter' });

expect(unsecuredSavedObjectsClient.bulkDelete).toHaveBeenCalledTimes(1);
expect(unsecuredSavedObjectsClient.bulkDelete).toHaveBeenCalledWith(
[enabledRuleForBulkOps1, enabledRuleForBulkOps2].map(({ id }) => ({
id,
type: 'alert',
})),
undefined
);

expect(taskManager.bulkRemove).toHaveBeenCalledTimes(1);
expect(taskManager.bulkRemove).toHaveBeenCalledWith(['id1', 'id2']);
expect(bulkMarkApiKeysForInvalidation).toHaveBeenCalledTimes(1);
expect(bulkMarkApiKeysForInvalidation).toHaveBeenCalledWith(
{ apiKeys: ['MTIzOmFiYw==', 'MzIxOmFiYw=='] },
expect.anything(),
expect.anything()
);
expect(result).toStrictEqual({
rules: [returnedRuleForBulkEnableWithActions1, returnedRuleForBulkEnableWithActions2],
errors: [],
total: 2,
taskIdsFailedToBeDeleted: [],
});
});

test('should try to delete rules, two successful and one with 500 error', async () => {
Expand Down Expand Up @@ -192,7 +241,7 @@ describe('bulkDelete', () => {
expect.anything()
);
expect(result).toStrictEqual({
rules: [returnedRuleForBulkDelete1, returnedRuleForBulkDelete3],
rules: [returnedRuleForBulkOps1, returnedRuleForBulkOps3],
errors: [{ message: 'UPS', rule: { id: 'id2', name: 'fakeName' }, status: 500 }],
total: 2,
taskIdsFailedToBeDeleted: [],
Expand Down Expand Up @@ -256,7 +305,7 @@ describe('bulkDelete', () => {
expect.anything()
);
expect(result).toStrictEqual({
rules: [returnedRuleForBulkDelete1],
rules: [returnedRuleForBulkOps1],
errors: [{ message: 'UPS', rule: { id: 'id2', name: 'fakeName' }, status: 409 }],
total: 2,
taskIdsFailedToBeDeleted: [],
Expand Down Expand Up @@ -314,7 +363,7 @@ describe('bulkDelete', () => {
expect.anything()
);
expect(result).toStrictEqual({
rules: [returnedRuleForBulkDelete1, returnedRuleForBulkDelete2],
rules: [returnedRuleForBulkOps1, returnedRuleForBulkOps2],
errors: [],
total: 2,
taskIdsFailedToBeDeleted: [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export const bulkDeleteRules = async <Params extends RuleParams>(
}

const { ids, filter } = options;
const actionsClient = await context.getActionsClient();

const kueryNodeFilter = ids ? convertRuleIdsToKueryNode(ids) : buildKueryNodeFilter(filter);
const authorizationFilter = await getAuthorizationFilter(context, { action: 'DELETE' });
Expand Down Expand Up @@ -95,13 +96,17 @@ export const bulkDeleteRules = async <Params extends RuleParams>(
// fix the type cast from SavedObjectsBulkUpdateObject to SavedObjectsBulkUpdateObject
// when we are doing the bulk delete and this should fix itself
const ruleType = context.ruleTypeRegistry.get(attributes.alertTypeId!);
const ruleDomain = transformRuleAttributesToRuleDomain<Params>(attributes as RuleAttributes, {
id,
logger: context.logger,
ruleType,
references,
omitGeneratedValues: false,
});
const ruleDomain = transformRuleAttributesToRuleDomain<Params>(
attributes as RuleAttributes,
{
id,
logger: context.logger,
ruleType,
references,
omitGeneratedValues: false,
},
(connectorId: string) => actionsClient.isSystemAction(connectorId)
);

try {
ruleDomainSchema.validate(ruleDomain);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@ export interface RRuleAttributes {
byweekday?: Array<string | number>;
bymonth?: number[];
bysetpos?: number[];
bymonthday: number[];
byyearday: number[];
byweekno: number[];
byhour: number[];
byminute: number[];
bysecond: number[];
bymonthday?: number[];
XavierM marked this conversation as resolved.
Show resolved Hide resolved
byyearday?: number[];
byweekno?: number[];
byhour?: number[];
byminute?: number[];
bysecond?: number[];
}

export interface RuleSnoozeScheduleAttributes {
Expand Down
119 changes: 118 additions & 1 deletion x-pack/plugins/alerting/server/routes/bulk_enable_rules.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@
*/

import { httpServiceMock } from '@kbn/core/server/mocks';

import { actionsClientMock } from '@kbn/actions-plugin/server/mocks';
import { bulkEnableRulesRoute } from './bulk_enable_rules';
import { licenseStateMock } from '../lib/license_state.mock';
import { mockHandlerArguments } from './_mock_handler_arguments';
import { rulesClientMock } from '../rules_client.mock';
import { RuleTypeDisabledError } from '../lib/errors/rule_type_disabled';
import { verifyApiAccess } from '../lib/license_api_access';
import { RuleActionTypes, RuleDefaultAction, RuleSystemAction, SanitizedRule } from '../types';

const rulesClient = rulesClientMock.create();

Expand Down Expand Up @@ -123,4 +124,120 @@ describe('bulkEnableRulesRoute', () => {

expect(res.forbidden).toHaveBeenCalledWith({ body: { message: 'Fail' } });
});

describe('actions', () => {
const mockedRule: SanitizedRule<{}> = {
id: '1',
alertTypeId: '1',
schedule: { interval: '10s' },
params: {
bar: true,
},
createdAt: new Date(),
updatedAt: new Date(),
actions: [
{
group: 'default',
id: '2',
actionTypeId: 'test',
params: {
foo: true,
},
uuid: '123-456',
type: RuleActionTypes.DEFAULT,
},
],
consumer: 'bar',
name: 'abc',
tags: ['foo'],
enabled: true,
muteAll: false,
notifyWhen: 'onActionGroupChange',
createdBy: '',
updatedBy: '',
apiKeyOwner: '',
throttle: '30s',
mutedInstanceIds: [],
executionStatus: {
status: 'unknown',
lastExecutionDate: new Date('2020-08-20T19:23:38Z'),
},
revision: 0,
};

const action: RuleDefaultAction = {
actionTypeId: 'test',
group: 'default',
id: '2',
params: {
foo: true,
},
uuid: '123-456',
type: RuleActionTypes.DEFAULT,
};

const systemAction: RuleSystemAction = {
actionTypeId: 'test-2',
id: 'system_action-id',
params: {
foo: true,
},
uuid: '123-456',
type: RuleActionTypes.SYSTEM,
};

const mockedRules: Array<SanitizedRule<{}>> = [
{ ...mockedRule, actions: [action, systemAction] },
];

const bulkEnableActionsResult = {
rules: mockedRules,
errors: [],
total: 1,
taskIdsFailedToBeEnabled: [],
};

it('removes the type from the actions correctly before sending the response', async () => {
const licenseState = licenseStateMock.create();
const router = httpServiceMock.createRouter();
const actionsClient = actionsClientMock.create();
actionsClient.isSystemAction.mockImplementation((id: string) => id === 'system_action-id');

bulkEnableRulesRoute({ router, licenseState });
const [_, handler] = router.patch.mock.calls[0];

rulesClient.bulkEnableRules.mockResolvedValueOnce(bulkEnableActionsResult);

const [context, req, res] = mockHandlerArguments(
{ rulesClient, actionsClient },
{
body: bulkEnableRequest,
},
['ok']
);

const routeRes = await handler(context, req, res);

// @ts-expect-error: body exists
expect(routeRes.body.rules[0].actions).toEqual([
{
connector_type_id: 'test',
group: 'default',
id: '2',
params: {
foo: true,
},
uuid: '123-456',
},
{
connector_type_id: 'test-2',
id: 'system_action-id',
params: {
foo: true,
},
uuid: '123-456',
},
]);
});
});
});
18 changes: 16 additions & 2 deletions x-pack/plugins/alerting/server/routes/bulk_enable_rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import { IRouter } from '@kbn/core/server';
import { verifyAccessAndContext, handleDisabledApiKeysError } from './lib';
import { ILicenseState, RuleTypeDisabledError } from '../lib';
import { AlertingRequestHandlerContext, INTERNAL_BASE_ALERTING_API_PATH } from '../types';
import { transformRuleToRuleResponseV1 } from './rule/transforms';
import { Rule } from '../application/rule/types';

export const bulkEnableRulesRoute = ({
router,
Expand All @@ -35,8 +37,20 @@ export const bulkEnableRulesRoute = ({
const { filter, ids } = req.body;

try {
const result = await rulesClient.bulkEnableRules({ filter, ids });
return res.ok({ body: result });
const bulkEnableResults = await rulesClient.bulkEnableRules({ filter, ids });

const resultBody = {
body: {
...bulkEnableResults,
rules: bulkEnableResults.rules.map((rule) => {
// TODO (http-versioning): Remove this cast, this enables us to move forward
// without fixing all of other solution types
return transformRuleToRuleResponseV1(rule as Rule);
}),
},
};

return res.ok(resultBody);
} catch (e) {
if (e instanceof RuleTypeDisabledError) {
return e.sendResponse(res);
Expand Down
Loading