Skip to content

Commit

Permalink
[Security Solution] Rename rulesManagementClient -> `detectionRules…
Browse files Browse the repository at this point in the history
…Client` (elastic#184598)

**Partially addresses: elastic#184364

## Summary
As a first step in refactoring our newly added `rulesManagementClient`,
we are renaming it from `rulesManagementClient` to
`detectionRulesClient`.

`rulesManagementClient` looks and sounds too similar to a long existing
`rulesClient`, so it can be confusing to new devs or anyone quickly
digging through the code.

This PR just updates the name. Other refactorings will be arriving in
the upcoming PRs.
  • Loading branch information
nikitaindik committed Jun 3, 2024
1 parent 9bb0018 commit 10f6174
Show file tree
Hide file tree
Showing 39 changed files with 116 additions and 116 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ export const createPrepackagedRules = async (
const savedObjectsClient = context.core.savedObjects.client;
const siemClient = context.getAppClient();
const exceptionsListClient = context.getExceptionListClient() ?? exceptionsClient;
const rulesManagementClient = context.getRulesManagementClient();
const detectionRulesClient = context.getDetectionRulesClient();
const ruleAssetsClient = createPrebuiltRuleAssetsClient(savedObjectsClient);

if (!siemClient || !rulesClient) {
Expand All @@ -107,14 +107,14 @@ export const createPrepackagedRules = async (
const rulesToInstall = getRulesToInstall(latestPrebuiltRules, installedPrebuiltRules);
const rulesToUpdate = getRulesToUpdate(latestPrebuiltRules, installedPrebuiltRules);

const result = await createPrebuiltRules(rulesManagementClient, rulesToInstall);
const result = await createPrebuiltRules(detectionRulesClient, rulesToInstall);
if (result.errors.length > 0) {
throw new AggregateError(result.errors, 'Error installing new prebuilt rules');
}

const { result: timelinesResult } = await performTimelinesInstallation(context);

await upgradePrebuiltRules(rulesManagementClient, rulesToUpdate);
await upgradePrebuiltRules(detectionRulesClient, rulesToUpdate);

const prebuiltRulesOutput: InstallPrebuiltRulesAndTimelinesResponse = {
rules_installed: rulesToInstall.length,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export const performRuleInstallationRoute = (router: SecuritySolutionPluginRoute
const config = ctx.securitySolution.getConfig();
const soClient = ctx.core.savedObjects.client;
const rulesClient = ctx.alerting.getRulesClient();
const rulesManagementClient = ctx.securitySolution.getRulesManagementClient();
const detectionRulesClient = ctx.securitySolution.getDetectionRulesClient();
const ruleAssetsClient = createPrebuiltRuleAssetsClient(soClient);
const ruleObjectsClient = createPrebuiltRuleObjectsClient(rulesClient);
const exceptionsListClient = ctx.securitySolution.getExceptionListClient();
Expand Down Expand Up @@ -110,7 +110,7 @@ export const performRuleInstallationRoute = (router: SecuritySolutionPluginRoute
}

const { results: installedRules, errors: installationErrors } = await createPrebuiltRules(
rulesManagementClient,
detectionRulesClient,
installableRules
);
const ruleErrors = [...fetchErrors, ...installationErrors];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export const performRuleUpgradeRoute = (router: SecuritySolutionPluginRouter) =>
const ctx = await context.resolve(['core', 'alerting', 'securitySolution']);
const soClient = ctx.core.savedObjects.client;
const rulesClient = ctx.alerting.getRulesClient();
const rulesManagementClient = ctx.securitySolution.getRulesManagementClient();
const detectionRulesClient = ctx.securitySolution.getDetectionRulesClient();
const ruleAssetsClient = createPrebuiltRuleAssetsClient(soClient);
const ruleObjectsClient = createPrebuiltRuleObjectsClient(rulesClient);

Expand Down Expand Up @@ -157,7 +157,7 @@ export const performRuleUpgradeRoute = (router: SecuritySolutionPluginRouter) =>

// Perform the upgrade
const { results: updatedRules, errors: installationErrors } = await upgradePrebuiltRules(
rulesManagementClient,
detectionRulesClient,
targetRules
);
const ruleErrors = [...fetchErrors, ...installationErrors];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,18 @@ import { MAX_RULES_TO_UPDATE_IN_PARALLEL } from '../../../../../../common/consta
import { initPromisePool } from '../../../../../utils/promise_pool';
import { withSecuritySpan } from '../../../../../utils/with_security_span';
import type { PrebuiltRuleAsset } from '../../model/rule_assets/prebuilt_rule_asset';
import type { IRulesManagementClient } from '../../../rule_management/logic/rule_management/rules_management_client';
import type { IDetectionRulesClient } from '../../../rule_management/logic/rule_management/detection_rules_client';

export const createPrebuiltRules = (
rulesManagementClient: IRulesManagementClient,
detectionRulesClient: IDetectionRulesClient,
rules: PrebuiltRuleAsset[]
) => {
return withSecuritySpan('createPrebuiltRules', async () => {
const result = await initPromisePool({
concurrency: MAX_RULES_TO_UPDATE_IN_PARALLEL,
items: rules,
executor: async (rule) => {
return rulesManagementClient.createPrebuiltRule({
return detectionRulesClient.createPrebuiltRule({
ruleAsset: rule,
});
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,25 +9,25 @@ import { MAX_RULES_TO_UPDATE_IN_PARALLEL } from '../../../../../../common/consta
import { initPromisePool } from '../../../../../utils/promise_pool';
import { withSecuritySpan } from '../../../../../utils/with_security_span';
import type { PrebuiltRuleAsset } from '../../model/rule_assets/prebuilt_rule_asset';
import type { IRulesManagementClient } from '../../../rule_management/logic/rule_management/rules_management_client';
import type { IDetectionRulesClient } from '../../../rule_management/logic/rule_management/detection_rules_client';

/**
* Upgrades existing prebuilt rules given a set of rules and output index.
* This implements a chunked approach to not saturate network connections and
* avoid being a "noisy neighbor".
* @param rulesManagementClient RulesManagementClient
* @param detectionRulesClient IDetectionRulesClient
* @param rules The rules to apply the update for
*/
export const upgradePrebuiltRules = async (
rulesManagementClient: IRulesManagementClient,
detectionRulesClient: IDetectionRulesClient,
rules: PrebuiltRuleAsset[]
) =>
withSecuritySpan('upgradePrebuiltRules', async () => {
const result = await initPromisePool({
concurrency: MAX_RULES_TO_UPDATE_IN_PARALLEL,
items: rules,
executor: async (rule) => {
return rulesManagementClient.upgradePrebuiltRule({ ruleAsset: rule });
return detectionRulesClient.upgradePrebuiltRule({ ruleAsset: rule });
},
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import { riskEngineDataClientMock } from '../../../entity_analytics/risk_engine/
import { riskScoreDataClientMock } from '../../../entity_analytics/risk_score/risk_score_data_client.mock';
import { assetCriticalityDataClientMock } from '../../../entity_analytics/asset_criticality/asset_criticality_data_client.mock';
import { auditLoggerMock } from '@kbn/security-plugin/server/audit/mocks';
import { rulesManagementClientMock } from '../../rule_management/logic/rule_management/__mocks__/rules_management_client';
import { detectionRulesClientMock } from '../../rule_management/logic/rule_management/__mocks__/detection_rules_client';

export const createMockClients = () => {
const core = coreMock.createRequestHandlerContext();
Expand All @@ -58,7 +58,7 @@ export const createMockClients = () => {
exceptionListClient: listMock.getExceptionListClient(core.savedObjects.client),
},
rulesClient: rulesClientMock.create(),
rulesManagementClient: rulesManagementClientMock.create(),
detectionRulesClient: detectionRulesClientMock.create(),
actionsClient: actionsClientMock.create(),
ruleDataService: ruleRegistryMocks.createRuleDataService(),

Expand Down Expand Up @@ -142,7 +142,7 @@ const createSecuritySolutionRequestContextMock = (
}),
getSpaceId: jest.fn(() => 'default'),
getRuleDataService: jest.fn(() => clients.ruleDataService),
getRulesManagementClient: jest.fn(() => clients.rulesManagementClient),
getDetectionRulesClient: jest.fn(() => clients.detectionRulesClient),
getDetectionEngineHealthClient: jest.fn(() => clients.detectionEngineHealthClient),
getRuleExecutionLog: jest.fn(() => clients.ruleExecutionLog),
getExceptionListClient: jest.fn(() => clients.lists.exceptionListClient),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ import type { RuleParams } from '../../../rule_schema';
import type { SecuritySolutionPluginRouter } from '../../../../../types';
import { buildSiemResponse } from '../../../routes/utils';
import { buildRouteValidation } from '../../../../../utils/build_validation/route_validation';
import type { IRulesManagementClient } from '../../../rule_management/logic/rule_management/rules_management_client';
import type { IDetectionRulesClient } from '../../../rule_management/logic/rule_management/detection_rules_client';

export const createRuleExceptionsRoute = (router: SecuritySolutionPluginRouter) => {
router.versioned
Expand Down Expand Up @@ -83,7 +83,7 @@ export const createRuleExceptionsRoute = (router: SecuritySolutionPluginRouter)
]);
const rulesClient = ctx.alerting.getRulesClient();
const listsClient = ctx.securitySolution.getExceptionListClient();
const rulesManagementClient = ctx.securitySolution.getRulesManagementClient();
const detectionRulesClient = ctx.securitySolution.getDetectionRulesClient();

const { items } = request.body;
const { id: ruleId } = request.params;
Expand All @@ -106,7 +106,7 @@ export const createRuleExceptionsRoute = (router: SecuritySolutionPluginRouter)
items,
rule,
listsClient,
rulesManagementClient,
detectionRulesClient,
});

const [validated, errors] = validate(createdItems, t.array(exceptionListItemSchema));
Expand All @@ -130,11 +130,11 @@ export const createRuleExceptions = async ({
items,
rule,
listsClient,
rulesManagementClient,
detectionRulesClient,
}: {
items: CreateRuleExceptionListItemSchemaDecoded[];
listsClient: ExceptionListClient | null;
rulesManagementClient: IRulesManagementClient;
detectionRulesClient: IDetectionRulesClient;
rule: SanitizedRule<RuleParams>;
}) => {
const ruleDefaultLists = rule.params.exceptionsList.filter(
Expand Down Expand Up @@ -169,7 +169,7 @@ export const createRuleExceptions = async ({
const defaultList = await createAndAssociateDefaultExceptionList({
rule,
listsClient,
rulesManagementClient,
detectionRulesClient,
removeOldAssociation: true,
});

Expand All @@ -179,7 +179,7 @@ export const createRuleExceptions = async ({
const defaultList = await createAndAssociateDefaultExceptionList({
rule,
listsClient,
rulesManagementClient,
detectionRulesClient,
removeOldAssociation: false,
});

Expand Down Expand Up @@ -272,12 +272,12 @@ export const createExceptionList = async ({
export const createAndAssociateDefaultExceptionList = async ({
rule,
listsClient,
rulesManagementClient,
detectionRulesClient,
removeOldAssociation,
}: {
rule: SanitizedRule<RuleParams>;
listsClient: ExceptionListClient | null;
rulesManagementClient: IRulesManagementClient;
detectionRulesClient: IDetectionRulesClient;
removeOldAssociation: boolean;
}): Promise<ExceptionListSchema> => {
const exceptionListToAssociate = await createExceptionList({ rule, listsClient });
Expand All @@ -294,7 +294,7 @@ export const createAndAssociateDefaultExceptionList = async ({
? existingRuleExceptionLists.filter((list) => list.type !== ExceptionListTypeEnum.RULE_DEFAULT)
: existingRuleExceptionLists;

await rulesManagementClient.patchRule({
await detectionRulesClient.patchRule({
nextParams: {
rule_id: rule.params.ruleId,
...rule.params,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ export const performBulkActionRoute = (
const exceptionsClient = ctx.lists?.getExceptionListClient();
const savedObjectsClient = ctx.core.savedObjects.client;
const actionsClient = ctx.actions.getActionsClient();
const rulesManagementClient = ctx.securitySolution.getRulesManagementClient();
const detectionRulesClient = ctx.securitySolution.getDetectionRulesClient();

const { getExporter, getClient } = ctx.core.savedObjects;
const client = getClient({ includedHiddenTypes: ['action'] });
Expand Down Expand Up @@ -203,7 +203,7 @@ export const performBulkActionRoute = (
return null;
}

await rulesManagementClient.deleteRule({
await detectionRulesClient.deleteRule({
ruleId: rule.id,
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ describe('Bulk create rules route', () => {

clients.rulesClient.find.mockResolvedValue(getEmptyFindResult()); // no existing rules
clients.rulesClient.create.mockResolvedValue(getRuleMock(getQueryRuleParams())); // successful creation
clients.rulesManagementClient.createCustomRule.mockResolvedValue(
clients.detectionRulesClient.createCustomRule.mockResolvedValue(
getRuleMock(getQueryRuleParams())
);
context.core.elasticsearch.client.asCurrentUser.search.mockResolvedValue(
Expand All @@ -55,7 +55,7 @@ describe('Bulk create rules route', () => {

describe('unhappy paths', () => {
test('returns a 403 error object if ML Authz fails', async () => {
clients.rulesManagementClient.createCustomRule.mockImplementationOnce(async () => {
clients.detectionRulesClient.createCustomRule.mockImplementationOnce(async () => {
throw new HttpAuthzError('mocked validation message');
});

Expand Down Expand Up @@ -108,7 +108,7 @@ describe('Bulk create rules route', () => {
});

test('catches error if creation throws', async () => {
clients.rulesManagementClient.createCustomRule.mockImplementation(async () => {
clients.detectionRulesClient.createCustomRule.mockImplementation(async () => {
throw new Error('Test error');
});
const response = await server.inject(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export const bulkCreateRulesRoute = (router: SecuritySolutionPluginRouter, logge
try {
const ctx = await context.resolve(['core', 'securitySolution', 'licensing', 'alerting']);
const rulesClient = ctx.alerting.getRulesClient();
const rulesManagementClient = ctx.securitySolution.getRulesManagementClient();
const detectionRulesClient = ctx.securitySolution.getDetectionRulesClient();

const ruleDefinitions = request.body;
const dupes = getDuplicates(ruleDefinitions, 'rule_id');
Expand Down Expand Up @@ -109,7 +109,7 @@ export const bulkCreateRulesRoute = (router: SecuritySolutionPluginRouter, logge
});
}

const createdRule = await rulesManagementClient.createCustomRule({
const createdRule = await detectionRulesClient.createCustomRule({
params: payloadRule,
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export const bulkDeleteRulesRoute = (router: SecuritySolutionPluginRouter, logge
const ctx = await context.resolve(['core', 'securitySolution', 'alerting']);

const rulesClient = ctx.alerting.getRulesClient();
const rulesManagementClient = ctx.securitySolution.getRulesManagementClient();
const detectionRulesClient = ctx.securitySolution.getDetectionRulesClient();

const rules = await Promise.all(
request.body.map(async (payloadRule) => {
Expand All @@ -76,7 +76,7 @@ export const bulkDeleteRulesRoute = (router: SecuritySolutionPluginRouter, logge
return getIdBulkError({ id, ruleId });
}

await rulesManagementClient.deleteRule({
await detectionRulesClient.deleteRule({
ruleId: rule.id,
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ describe('Bulk patch rules route', () => {

clients.rulesClient.find.mockResolvedValue(getFindResultWithSingleHit()); // rule exists
clients.rulesClient.update.mockResolvedValue(getRuleMock(getQueryRuleParams())); // update succeeds
clients.rulesManagementClient.patchRule.mockResolvedValue(getRuleMock(getQueryRuleParams()));
clients.detectionRulesClient.patchRule.mockResolvedValue(getRuleMock(getQueryRuleParams()));

bulkPatchRulesRoute(server.router, logger);
});
Expand Down Expand Up @@ -72,7 +72,7 @@ describe('Bulk patch rules route', () => {
...getFindResultWithSingleHit(),
data: [getRuleMock(getMlRuleParams())],
});
clients.rulesManagementClient.patchRule.mockResolvedValueOnce(
clients.detectionRulesClient.patchRule.mockResolvedValueOnce(
getRuleMock(
getMlRuleParams({
anomalyThreshold,
Expand Down Expand Up @@ -101,7 +101,7 @@ describe('Bulk patch rules route', () => {
});

it('rejects patching a rule to ML if mlAuthz fails', async () => {
clients.rulesManagementClient.patchRule.mockImplementationOnce(async () => {
clients.detectionRulesClient.patchRule.mockImplementationOnce(async () => {
throw new HttpAuthzError('mocked validation message');
});

Expand All @@ -125,7 +125,7 @@ describe('Bulk patch rules route', () => {
});

it('rejects patching an existing ML rule if mlAuthz fails', async () => {
clients.rulesManagementClient.patchRule.mockImplementationOnce(async () => {
clients.detectionRulesClient.patchRule.mockImplementationOnce(async () => {
throw new HttpAuthzError('mocked validation message');
});
const { type, ...payloadWithoutType } = typicalMlRulePayload();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export const bulkPatchRulesRoute = (router: SecuritySolutionPluginRouter, logger
try {
const ctx = await context.resolve(['core', 'securitySolution', 'alerting', 'licensing']);
const rulesClient = ctx.alerting.getRulesClient();
const rulesManagementClient = ctx.securitySolution.getRulesManagementClient();
const detectionRulesClient = ctx.securitySolution.getDetectionRulesClient();

const rules = await Promise.all(
request.body.map(async (payloadRule) => {
Expand Down Expand Up @@ -87,7 +87,7 @@ export const bulkPatchRulesRoute = (router: SecuritySolutionPluginRouter, logger
ruleId: payloadRule.id,
});

const rule = await rulesManagementClient.patchRule({
const rule = await detectionRulesClient.patchRule({
nextParams: payloadRule,
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ describe('Bulk update rules route', () => {

clients.rulesClient.find.mockResolvedValue(getFindResultWithSingleHit());
clients.rulesClient.update.mockResolvedValue(getRuleMock(getQueryRuleParams()));
clients.rulesManagementClient.updateRule.mockResolvedValue(getRuleMock(getQueryRuleParams()));
clients.detectionRulesClient.updateRule.mockResolvedValue(getRuleMock(getQueryRuleParams()));
clients.appClient.getSignalsIndex.mockReturnValue('.siem-signals-test-index');

bulkUpdateRulesRoute(server.router, logger);
Expand Down Expand Up @@ -66,7 +66,7 @@ describe('Bulk update rules route', () => {
});

test('returns an error if update throws', async () => {
clients.rulesManagementClient.updateRule.mockImplementation(() => {
clients.detectionRulesClient.updateRule.mockImplementation(() => {
throw new Error('Test error');
});

Expand All @@ -85,7 +85,7 @@ describe('Bulk update rules route', () => {
});

it('returns a 403 error object if mlAuthz fails', async () => {
clients.rulesManagementClient.updateRule.mockImplementationOnce(async () => {
clients.detectionRulesClient.updateRule.mockImplementationOnce(async () => {
throw new HttpAuthzError('mocked validation message');
});

Expand Down
Loading

0 comments on commit 10f6174

Please sign in to comment.