Skip to content

Commit

Permalink
[RAM] System action in bulk disable api (#170229)
Browse files Browse the repository at this point in the history
Fix: #170097
Meta: #160367


## Summary

This PR enables system actions for the Bulk Disable Rule API.

### 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
  • Loading branch information
guskovaue authored Nov 6, 2023
1 parent f7ecb3b commit 62ba292
Show file tree
Hide file tree
Showing 5 changed files with 280 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,20 @@ import {
savedObjectWith500Error,
disabledRuleForBulkDisable1,
disabledRuleForBulkDisable2,
returnedRuleForBulkDisableWithActions1,
returnedRuleForBulkDisableWithActions2,
enabledRuleForBulkOps1,
enabledRuleForBulkOps2,
disabledRuleForBulkOpsWithActions1,
disabledRuleForBulkOpsWithActions2,
returnedRuleForBulkDisable1,
returnedRuleForBulkDisable2,
siemRuleForBulkOps1,
siemRuleForBulkOps2,
} from '../../../../rules_client/tests/test_helpers';
import { migrateLegacyActions } from '../../../../rules_client/lib';
import { ConnectorAdapterRegistry } from '../../../../connector_adapters/connector_adapter_registry';
import { ActionsClient } from '@kbn/actions-plugin/server';

jest.mock('../../../../task_runner/alert_task_instance', () => ({
taskInstanceToAlertTaskInstance: jest.fn(),
Expand Down Expand Up @@ -95,6 +100,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 @@ -108,6 +114,8 @@ setGlobalDate();

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

const mockCreatePointInTimeFinderAsInternalUser = (
response = { saved_objects: [enabledRule1, enabledRule2] }
) => {
Expand Down Expand Up @@ -143,6 +151,9 @@ describe('bulkDisableRules', () => {

beforeEach(async () => {
rulesClient = new RulesClient(rulesClientParams);
actionsClient = (await rulesClientParams.getActionsClient()) as jest.Mocked<ActionsClient>;
rulesClientParams.getActionsClient.mockResolvedValue(actionsClient);

authorization.getFindAuthorizationFilter.mockResolvedValue({
ensureRuleTypeIsAuthorized() {},
});
Expand All @@ -153,6 +164,7 @@ describe('bulkDisableRules', () => {
resultedActions: [],
resultedReferences: [],
});
actionsClient.isSystemAction.mockImplementation((id: string) => id === 'system_action:id');
});

test('should disable two rule', async () => {
Expand Down Expand Up @@ -188,6 +200,39 @@ describe('bulkDisableRules', () => {
});
});

test('should disable two rule and return right actions', async () => {
unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({
saved_objects: [disabledRuleForBulkOpsWithActions1, disabledRuleForBulkOpsWithActions2],
});

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

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: [returnedRuleForBulkDisableWithActions1, returnedRuleForBulkDisableWithActions2],
total: 2,
});
});

test('should try to disable rules, one successful and one with 500 error', async () => {
unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({
saved_objects: [disabledRuleForBulkDisable1, savedObjectWith500Error],
Expand Down Expand Up @@ -656,25 +701,25 @@ describe('bulkDisableRules', () => {
await rulesClient.bulkDisableRules({ filter: 'fake_filter' });

expect(migrateLegacyActions).toHaveBeenCalledTimes(4);
expect(migrateLegacyActions).toHaveBeenCalledWith(expect.any(Object), {
expect(migrateLegacyActions).toHaveBeenNthCalledWith(1, expect.any(Object), {
attributes: enabledRuleForBulkOps1.attributes,
ruleId: enabledRuleForBulkOps1.id,
actions: [],
references: [],
});
expect(migrateLegacyActions).toHaveBeenCalledWith(expect.any(Object), {
expect(migrateLegacyActions).toHaveBeenNthCalledWith(2, expect.any(Object), {
attributes: enabledRuleForBulkOps2.attributes,
ruleId: enabledRuleForBulkOps2.id,
actions: [],
references: [],
});
expect(migrateLegacyActions).toHaveBeenCalledWith(expect.any(Object), {
expect(migrateLegacyActions).toHaveBeenNthCalledWith(3, expect.any(Object), {
attributes: expect.objectContaining({ consumer: AlertConsumers.SIEM }),
ruleId: siemRuleForBulkOps1.id,
actions: [],
references: [],
});
expect(migrateLegacyActions).toHaveBeenCalledWith(expect.any(Object), {
expect(migrateLegacyActions).toHaveBeenNthCalledWith(4, expect.any(Object), {
attributes: expect.objectContaining({ consumer: AlertConsumers.SIEM }),
ruleId: siemRuleForBulkOps2.id,
actions: [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export const bulkDisableRules = 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: 'DISABLE' });
Expand Down Expand Up @@ -93,13 +94,17 @@ export const bulkDisableRules = async <Params extends RuleParams>(
// fix the type cast from SavedObjectsBulkUpdateObject to SavedObjectsBulkUpdateObject
// when we are doing the bulk disable 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 @@ -13,6 +13,13 @@ 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 { actionsClientMock } from '@kbn/actions-plugin/server/mocks';
import {
RuleActionTypes,
RuleDefaultAction,
RuleSystemAction,
SanitizedRule,
} from '../../../../types';

const rulesClient = rulesClientMock.create();

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

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: false,
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 bulkDisableActionsResult = {
rules: mockedRules,
errors: [],
total: 1,
skipped: [],
};

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');

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

rulesClient.bulkDisableRules.mockResolvedValueOnce(bulkDisableActionsResult);

const [context, req, res] = mockHandlerArguments(
{ rulesClient, actionsClient },
{
body: bulkDisableRequest,
},
['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',
},
]);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,13 @@

import Boom from '@hapi/boom';
import { i18n } from '@kbn/i18n';

import { AlertConsumers } from '@kbn/rule-data-utils';

import type { SavedObjectReference } from '@kbn/core/server';
import type { RulesClientContext } from '../..';
import { RawRuleAction, RawRule } from '../../../types';
import { validateActions } from '../validate_actions';
import { injectReferencesIntoActions } from '../../common';
import { retrieveMigratedLegacyActions } from './retrieve_migrated_legacy_actions';
import { transformRawActionsToDomainActions } from '../../../application/rule/transforms/transform_raw_actions_to_domain_actions';

type MigrateLegacyActions = (
context: RulesClientContext,
Expand Down Expand Up @@ -66,7 +64,12 @@ export const migrateLegacyActions: MigrateLegacyActions = async (
// set to undefined to avoid both per-actin and rule level values clashing
throttle: undefined,
notifyWhen: undefined,
actions: injectReferencesIntoActions(ruleId, legacyActions, legacyActionsReferences),
actions: transformRawActionsToDomainActions({
ruleId,
actions: legacyActions,
references: legacyActionsReferences,
isSystemAction: context.isSystemAction,
}),
});
};

Expand Down
Loading

0 comments on commit 62ba292

Please sign in to comment.