Skip to content

Commit

Permalink
[Security Solution] Write and read Rule Execution Logs from rule inst…
Browse files Browse the repository at this point in the history
…ead of saved object (elastic#147035)

**Addresses:** elastic#130966
**Based on:** elastic#135127

## Summary

This PR deprecates the Sidecar SO of type `siem-detection-engine-rule-execution-info` in favour of storing Rule Execution Logging data within the rule itself, making use of the work previously done in the Alerting Framework:
- elastic#140882
- elastic#147278

Work done:
- **Pass execution statuses and metrics from rule executors to the Framework:** through the use of `RuleMonitoringService` and `RuleResultService` from within the rule execution log client for executor. `x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/client_for_executors/client.ts`
- **Fetch execution statuses and metrics from rules themselves instead of the sidecar `siem-detection-engine-rule-execution-info` saved objects**: through the use of the new function `createRuleExecutionSummary` in `x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/rule_execution_log/create_rule_execution_summary.ts`, which extracts last execution information from the rule itself.
- **Remove the siem-detection-engine-rule-execution-info saved objects type from the codebase. Mark it as deleted in Kibana Core:** added `siem-detection-engine-rule-execution-info` to `packages/core/saved-objects/core-saved-objects-migration-server-internal/src/core/unused_types.ts`; and got rid of the related Saved Object client.
- **Make sure to keep backward compatibility in the Detection API endpoints and rule execution events we write into the Event Log**: API compatibility is maintained. No breaking changes.


### Checklist

- [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials
- [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
jpdjere authored and darnautov committed Feb 7, 2023
1 parent a873621 commit 94e3c34
Show file tree
Hide file tree
Showing 44 changed files with 221 additions and 564 deletions.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ export const REMOVED_TYPES: string[] = [
'osquery-usage-metric',
// Was removed in 8.1 https://github.com/elastic/kibana/issues/91265
'siem-detection-engine-rule-status',
// Was removed in 8.7 https://github.com/elastic/kibana/issues/130966
'siem-detection-engine-rule-execution-info',
// Was removed in 7.16
'timelion-sheet',
// Removed in 8.3 https://github.com/elastic/kibana/issues/127745
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,11 @@ describe('createInitialState', () => {
"type": "server",
},
},
Object {
"term": Object {
"type": "siem-detection-engine-rule-execution-info",
},
},
Object {
"term": Object {
"type": "siem-detection-engine-rule-status",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,6 @@ describe('checking migration metadata changes on all registered SO types', () =>
"security-rule": "e0dfdba5d66139d0300723b2e6672993cd4a11f3",
"security-solution-signals-migration": "e65933e32926e0ca385415bd44fc6da0b6d3d419",
"siem-detection-engine-rule-actions": "d4b5934c0c0e4ccdf509a41000eb0bee07be0c28",
"siem-detection-engine-rule-execution-info": "b92d51db7b7d591758d3e85892a91064aff01ff8",
"siem-ui-timeline": "95474f10662802e2f9ea068b45bf69212a2f5842",
"siem-ui-timeline-note": "08c71dc0b8b8018a67e80beb4659a078404c223d",
"siem-ui-timeline-pinned-event": "e2697b38751506c7fce6e8b7207a830483dc4283",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import type * as t from 'io-ts';
import { enumeration, PositiveInteger } from '@kbn/securitysolution-io-ts-types';
import type { RuleLastRunOutcomes } from '@kbn/alerting-plugin/common';
import { assertUnreachable } from '../../../utility_types';

/**
Expand Down Expand Up @@ -78,3 +79,19 @@ export const ruleExecutionStatusToNumber = (
return 0;
}
};

export const ruleLastRunOutcomeToExecutionStatus = (
outcome: RuleLastRunOutcomes
): RuleExecutionStatus => {
switch (outcome) {
case 'succeeded':
return RuleExecutionStatus.succeeded;
case 'warning':
return RuleExecutionStatus['partial failure'];
case 'failed':
return RuleExecutionStatus.failed;
default:
assertUnreachable(outcome);
return RuleExecutionStatus.failed;
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ const RuleStatusBadgeComponent = ({ status, message }: RuleStatusBadgeProps) =>
const statusColor = getStatusColor(status);
return (
<HealthTruncateText
tooltipContent={statusTooltip}
tooltipContent={statusTooltip?.split('\n').map((line) => (
<p>{line}</p>
))}
healthColor={statusColor}
dataTestSubj="ruleExecutionStatus"
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,9 @@ const RuleStatusFailedCallOutComponent: React.FC<RuleStatusFailedCallOutProps> =
iconType="alert"
data-test-subj="ruleStatusFailedCallOut"
>
<p>{message}</p>
{message.split('\n').map((line) => (
<p>{line}</p>
))}
</EuiCallOut>
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ const createPromises = (
await deleteRules({
ruleId: migratedRule.id,
rulesClient,
ruleExecutionLog,
});

return createRules({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,6 @@ export const performBulkActionRoute = (
]);

const rulesClient = ctx.alerting.getRulesClient();
const ruleExecutionLog = ctx.securitySolution.getRuleExecutionLog();
const exceptionsClient = ctx.lists?.getExceptionListClient();
const savedObjectsClient = ctx.core.savedObjects.client;

Expand Down Expand Up @@ -481,7 +480,6 @@ export const performBulkActionRoute = (
await deleteRules({
ruleId: migratedRule.id,
rulesClient,
ruleExecutionLog,
});

return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ export const bulkCreateRulesRoute = (
params: payloadRule,
});

return transformValidateBulkError(createdRule.params.ruleId, createdRule, null);
return transformValidateBulkError(createdRule.params.ruleId, createdRule);
} catch (err) {
return transformBulkError(
payloadRule.rule_id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ export const bulkDeleteRulesRoute = (router: SecuritySolutionPluginRouter, logge
const ctx = await context.resolve(['core', 'securitySolution', 'alerting']);

const rulesClient = ctx.alerting.getRulesClient();
const ruleExecutionLog = ctx.securitySolution.getRuleExecutionLog();
const savedObjectsClient = ctx.core.savedObjects.client;

const rules = await Promise.all(
Expand All @@ -91,19 +90,12 @@ export const bulkDeleteRulesRoute = (router: SecuritySolutionPluginRouter, logge
return getIdBulkError({ id, ruleId });
}

const ruleExecutionSummary = await ruleExecutionLog.getExecutionSummary(migratedRule.id);

await deleteRules({
ruleId: migratedRule.id,
rulesClient,
ruleExecutionLog,
});

return transformValidateBulkError(
idOrRuleIdOrUnknown,
migratedRule,
ruleExecutionSummary
);
return transformValidateBulkError(idOrRuleIdOrUnknown, migratedRule);
} catch (err) {
return transformBulkError(idOrRuleIdOrUnknown, err);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ export const bulkPatchRulesRoute = (
const ctx = await context.resolve(['core', 'securitySolution', 'alerting', 'licensing']);

const rulesClient = ctx.alerting.getRulesClient();
const ruleExecutionLog = ctx.securitySolution.getRuleExecutionLog();
const savedObjectsClient = ctx.core.savedObjects.client;

const mlAuthz = buildMlAuthz({
Expand Down Expand Up @@ -111,8 +110,7 @@ export const bulkPatchRulesRoute = (
nextParams: payloadRule,
});
if (rule != null && rule.enabled != null && rule.name != null) {
const ruleExecutionSummary = await ruleExecutionLog.getExecutionSummary(rule.id);
return transformValidateBulkError(rule.id, rule, ruleExecutionSummary);
return transformValidateBulkError(rule.id, rule);
} else {
return getIdBulkError({ id: payloadRule.id, ruleId: payloadRule.rule_id });
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ export const bulkUpdateRulesRoute = (
const ctx = await context.resolve(['core', 'securitySolution', 'alerting', 'licensing']);

const rulesClient = ctx.alerting.getRulesClient();
const ruleExecutionLog = ctx.securitySolution.getRuleExecutionLog();
const savedObjectsClient = ctx.core.savedObjects.client;

const mlAuthz = buildMlAuthz({
Expand Down Expand Up @@ -116,8 +115,7 @@ export const bulkUpdateRulesRoute = (
ruleUpdate: payloadRule,
});
if (rule != null) {
const ruleExecutionSummary = await ruleExecutionLog.getExecutionSummary(rule.id);
return transformValidateBulkError(rule.id, rule, ruleExecutionSummary);
return transformValidateBulkError(rule.id, rule);
} else {
return getIdBulkError({ id: payloadRule.id, ruleId: payloadRule.rule_id });
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ export const createRuleRoute = (
]);

const rulesClient = ctx.alerting.getRulesClient();
const ruleExecutionLog = ctx.securitySolution.getRuleExecutionLog();
const savedObjectsClient = ctx.core.savedObjects.client;
const exceptionsClient = ctx.lists?.getExceptionListClient();

Expand Down Expand Up @@ -99,9 +98,7 @@ export const createRuleRoute = (
params: request.body,
});

const ruleExecutionSummary = await ruleExecutionLog.getExecutionSummary(createdRule.id);

const [validated, errors] = transformValidate(createdRule, ruleExecutionSummary);
const [validated, errors] = transformValidate(createdRule);
if (errors != null) {
return siemResponse.error({ statusCode: 500, body: errors });
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ export const deleteRuleRoute = (router: SecuritySolutionPluginRouter) => {

const ctx = await context.resolve(['core', 'securitySolution', 'alerting']);
const rulesClient = ctx.alerting.getRulesClient();
const ruleExecutionLog = ctx.securitySolution.getRuleExecutionLog();
const savedObjectsClient = ctx.core.savedObjects.client;

const rule = await readRules({ rulesClient, id, ruleId });
Expand All @@ -64,15 +63,12 @@ export const deleteRuleRoute = (router: SecuritySolutionPluginRouter) => {
});
}

const ruleExecutionSummary = await ruleExecutionLog.getExecutionSummary(migratedRule.id);

await deleteRules({
ruleId: migratedRule.id,
rulesClient,
ruleExecutionLog,
});

const transformed = transform(migratedRule, ruleExecutionSummary);
const transformed = transform(migratedRule);
if (transformed == null) {
return siemResponse.error({ statusCode: 500, body: 'failed to transform alert' });
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ export const findRulesRoute = (router: SecuritySolutionPluginRouter, logger: Log
const { query } = request;
const ctx = await context.resolve(['core', 'securitySolution', 'alerting']);
const rulesClient = ctx.alerting.getRulesClient();
const ruleExecutionLog = ctx.securitySolution.getRuleExecutionLog();
const savedObjectsClient = ctx.core.savedObjects.client;

const rules = await findRules({
Expand All @@ -64,12 +63,13 @@ export const findRulesRoute = (router: SecuritySolutionPluginRouter, logger: Log

const ruleIds = rules.data.map((rule) => rule.id);

const [ruleExecutionSummaries, ruleActions] = await Promise.all([
ruleExecutionLog.getExecutionSummariesBulk(ruleIds),
legacyGetBulkRuleActionsSavedObject({ alertIds: ruleIds, savedObjectsClient, logger }),
]);
const ruleActions = await legacyGetBulkRuleActionsSavedObject({
alertIds: ruleIds,
savedObjectsClient,
logger,
});

const transformed = transformFindAlerts(rules, ruleExecutionSummaries, ruleActions);
const transformed = transformFindAlerts(rules, ruleActions);
if (transformed == null) {
return siemResponse.error({ statusCode: 500, body: 'Internal error transforming' });
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ export const patchRuleRoute = (router: SecuritySolutionPluginRouter, ml: SetupPl
try {
const params = request.body;
const rulesClient = (await context.alerting).getRulesClient();
const ruleExecutionLog = (await context.securitySolution).getRuleExecutionLog();
const savedObjectsClient = (await context.core).savedObjects.client;

const mlAuthz = buildMlAuthz({
Expand Down Expand Up @@ -96,9 +95,7 @@ export const patchRuleRoute = (router: SecuritySolutionPluginRouter, ml: SetupPl
nextParams: params,
});
if (rule != null && rule.enabled != null && rule.name != null) {
const ruleExecutionSummary = await ruleExecutionLog.getExecutionSummary(rule.id);

const [validated, errors] = transformValidate(rule, ruleExecutionSummary);
const [validated, errors] = transformValidate(rule);
if (errors != null) {
return siemResponse.error({ statusCode: 500, body: errors });
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ export const readRuleRoute = (router: SecuritySolutionPluginRouter, logger: Logg

try {
const rulesClient = (await context.alerting).getRulesClient();
const ruleExecutionLog = (await context.securitySolution).getRuleExecutionLog();
const savedObjectsClient = (await context.core).savedObjects.client;

const rule = await readRules({
Expand All @@ -60,9 +59,7 @@ export const readRuleRoute = (router: SecuritySolutionPluginRouter, logger: Logg
logger,
});

const ruleExecutionSummary = await ruleExecutionLog.getExecutionSummary(rule.id);

const transformed = transform(rule, ruleExecutionSummary, legacyRuleActions);
const transformed = transform(rule, legacyRuleActions);
if (transformed == null) {
return siemResponse.error({ statusCode: 500, body: 'Internal error transforming' });
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,7 @@ export const updateRuleRoute = (router: SecuritySolutionPluginRouter, ml: SetupP
});

if (rule != null) {
const ruleExecutionLog = ctx.securitySolution.getRuleExecutionLog();
const ruleExecutionSummary = await ruleExecutionLog.getExecutionSummary(rule.id);
const [validated, errors] = transformValidate(rule, ruleExecutionSummary);
const [validated, errors] = transformValidate(rule);
if (errors != null) {
return siemResponse.error({ statusCode: 500, body: errors });
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,29 +6,24 @@
*/

import { rulesClientMock } from '@kbn/alerting-plugin/server/mocks';
import { ruleExecutionLogMock } from '../../../rule_monitoring/mocks';
import type { DeleteRuleOptions } from './delete_rules';
import { deleteRules } from './delete_rules';

describe('deleteRules', () => {
let rulesClient: ReturnType<typeof rulesClientMock.create>;
let ruleExecutionLog: ReturnType<typeof ruleExecutionLogMock.forRoutes.create>;

beforeEach(() => {
rulesClient = rulesClientMock.create();
ruleExecutionLog = ruleExecutionLogMock.forRoutes.create();
});

it('should delete the rule along with its actions, and statuses', async () => {
const options: DeleteRuleOptions = {
ruleId: 'ruleId',
rulesClient,
ruleExecutionLog,
};

await deleteRules(options);

expect(rulesClient.delete).toHaveBeenCalledWith({ id: options.ruleId });
expect(ruleExecutionLog.clearExecutionSummary).toHaveBeenCalledWith(options.ruleId);
});
});
Loading

0 comments on commit 94e3c34

Please sign in to comment.