Skip to content

Commit

Permalink
[SIEM] [Detection Engine] Fixes bug when notification doesn't… (#63013)
Browse files Browse the repository at this point in the history
Set refresh on bulk create to 'wait_for' when actions are present, so we do not respond until the newly indexed signals are searchable.

* set refresh on bulk create to 'wait_for' when actions are present, so we do not respond until the newly indexed signals are searchable

* fix types in tests
  • Loading branch information
dhurley14 committed Apr 8, 2020
1 parent c643148 commit 274cb80
Show file tree
Hide file tree
Showing 8 changed files with 62 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { SearchResponse } from 'elasticsearch';
import { Logger } from '../../../../../../../../src/core/server';
import { AlertServices } from '../../../../../../../plugins/alerting/server';
import { RuleAlertAction } from '../../../../common/detection_engine/types';
import { RuleTypeParams } from '../types';
import { RuleTypeParams, RefreshTypes } from '../types';
import { singleBulkCreate, SingleBulkCreateResponse } from './single_bulk_create';
import { AnomalyResults, Anomaly } from '../../machine_learning';

Expand All @@ -29,6 +29,7 @@ interface BulkCreateMlSignalsParams {
updatedBy: string;
interval: string;
enabled: boolean;
refresh: RefreshTypes;
tags: string[];
throttle: string;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ describe('searchAfterAndBulkCreate', () => {
enabled: true,
pageSize: 1,
filter: undefined,
refresh: false,
tags: ['some fake tag 1', 'some fake tag 2'],
throttle: 'no_actions',
});
Expand Down Expand Up @@ -126,6 +127,7 @@ describe('searchAfterAndBulkCreate', () => {
enabled: true,
pageSize: 1,
filter: undefined,
refresh: false,
tags: ['some fake tag 1', 'some fake tag 2'],
throttle: 'no_actions',
});
Expand Down Expand Up @@ -156,6 +158,7 @@ describe('searchAfterAndBulkCreate', () => {
enabled: true,
pageSize: 1,
filter: undefined,
refresh: false,
tags: ['some fake tag 1', 'some fake tag 2'],
throttle: 'no_actions',
});
Expand Down Expand Up @@ -198,6 +201,7 @@ describe('searchAfterAndBulkCreate', () => {
enabled: true,
pageSize: 1,
filter: undefined,
refresh: false,
tags: ['some fake tag 1', 'some fake tag 2'],
throttle: 'no_actions',
});
Expand Down Expand Up @@ -240,6 +244,7 @@ describe('searchAfterAndBulkCreate', () => {
enabled: true,
pageSize: 1,
filter: undefined,
refresh: false,
tags: ['some fake tag 1', 'some fake tag 2'],
throttle: 'no_actions',
});
Expand Down Expand Up @@ -284,6 +289,7 @@ describe('searchAfterAndBulkCreate', () => {
enabled: true,
pageSize: 1,
filter: undefined,
refresh: false,
tags: ['some fake tag 1', 'some fake tag 2'],
throttle: 'no_actions',
});
Expand Down Expand Up @@ -328,6 +334,7 @@ describe('searchAfterAndBulkCreate', () => {
enabled: true,
pageSize: 1,
filter: undefined,
refresh: false,
tags: ['some fake tag 1', 'some fake tag 2'],
throttle: 'no_actions',
});
Expand Down Expand Up @@ -374,6 +381,7 @@ describe('searchAfterAndBulkCreate', () => {
enabled: true,
pageSize: 1,
filter: undefined,
refresh: false,
tags: ['some fake tag 1', 'some fake tag 2'],
throttle: 'no_actions',
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import { AlertServices } from '../../../../../../../plugins/alerting/server';
import { RuleAlertAction } from '../../../../common/detection_engine/types';
import { RuleTypeParams } from '../types';
import { RuleTypeParams, RefreshTypes } from '../types';
import { Logger } from '../../../../../../../../src/core/server';
import { singleSearchAfter } from './single_search_after';
import { singleBulkCreate } from './single_bulk_create';
Expand All @@ -30,6 +30,7 @@ interface SearchAfterAndBulkCreateParams {
enabled: boolean;
pageSize: number;
filter: unknown;
refresh: RefreshTypes;
tags: string[];
throttle: string;
}
Expand Down Expand Up @@ -61,6 +62,7 @@ export const searchAfterAndBulkCreate = async ({
interval,
enabled,
pageSize,
refresh,
tags,
throttle,
}: SearchAfterAndBulkCreateParams): Promise<SearchAfterAndBulkCreateReturnType> => {
Expand Down Expand Up @@ -92,6 +94,7 @@ export const searchAfterAndBulkCreate = async ({
updatedBy,
interval,
enabled,
refresh,
tags,
throttle,
});
Expand Down Expand Up @@ -179,6 +182,7 @@ export const searchAfterAndBulkCreate = async ({
updatedBy,
interval,
enabled,
refresh,
tags,
throttle,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ describe('rules_notification_alert_type', () => {
};
(ruleStatusServiceFactory as jest.Mock).mockReturnValue(ruleStatusService);
(getGapBetweenRuns as jest.Mock).mockReturnValue(moment.duration(0));
(searchAfterAndBulkCreate as jest.Mock).mockClear();
(searchAfterAndBulkCreate as jest.Mock).mockResolvedValue({
success: true,
searchAfterTimes: [],
Expand Down Expand Up @@ -149,6 +150,37 @@ describe('rules_notification_alert_type', () => {
});
});

it("should set refresh to 'wait_for' when actions are present", async () => {
const ruleAlert = getResult();
ruleAlert.actions = [
{
actionTypeId: '.slack',
params: {
message:
'Rule generated {{state.signals_count}} signals\n\n{{context.rule.name}}\n{{{context.results_link}}}',
},
group: 'default',
id: '99403909-ca9b-49ba-9d7a-7e5320e68d05',
},
];

savedObjectsClient.get.mockResolvedValue({
id: 'id',
type: 'type',
references: [],
attributes: ruleAlert,
});
await alert.executor(payload);
expect((searchAfterAndBulkCreate as jest.Mock).mock.calls[0][0].refresh).toEqual('wait_for');
(searchAfterAndBulkCreate as jest.Mock).mockClear();
});

it('should set refresh to false when actions are not present', async () => {
await alert.executor(payload);
expect((searchAfterAndBulkCreate as jest.Mock).mock.calls[0][0].refresh).toEqual(false);
(searchAfterAndBulkCreate as jest.Mock).mockClear();
});

it('should call scheduleActions if signalsCount was greater than 0 and rule has actions defined', async () => {
const ruleAlert = getResult();
ruleAlert.actions = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ export const signalRulesAlertType = ({
params: ruleParams,
} = savedObject.attributes;
const updatedAt = savedObject.updated_at ?? '';
const refresh = actions.length ? 'wait_for' : false;
const buildRuleMessage = buildRuleMessageFactory({
id: alertId,
ruleId,
Expand Down Expand Up @@ -181,6 +182,7 @@ export const signalRulesAlertType = ({
updatedAt,
interval,
enabled,
refresh,
tags,
});
result.success = success;
Expand Down Expand Up @@ -241,6 +243,7 @@ export const signalRulesAlertType = ({
interval,
enabled,
pageSize: searchAfterSize,
refresh,
tags,
throttle,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ describe('singleBulkCreate', () => {
updatedBy: 'elastic',
interval: '5m',
enabled: true,
refresh: false,
tags: ['some fake tag 1', 'some fake tag 2'],
throttle: 'no_actions',
});
Expand Down Expand Up @@ -192,6 +193,7 @@ describe('singleBulkCreate', () => {
updatedBy: 'elastic',
interval: '5m',
enabled: true,
refresh: false,
tags: ['some fake tag 1', 'some fake tag 2'],
throttle: 'no_actions',
});
Expand All @@ -217,6 +219,7 @@ describe('singleBulkCreate', () => {
updatedBy: 'elastic',
interval: '5m',
enabled: true,
refresh: false,
tags: ['some fake tag 1', 'some fake tag 2'],
throttle: 'no_actions',
});
Expand All @@ -243,6 +246,7 @@ describe('singleBulkCreate', () => {
updatedBy: 'elastic',
interval: '5m',
enabled: true,
refresh: false,
tags: ['some fake tag 1', 'some fake tag 2'],
throttle: 'no_actions',
});
Expand Down Expand Up @@ -271,6 +275,7 @@ describe('singleBulkCreate', () => {
updatedBy: 'elastic',
interval: '5m',
enabled: true,
refresh: false,
tags: ['some fake tag 1', 'some fake tag 2'],
throttle: 'no_actions',
});
Expand Down Expand Up @@ -365,6 +370,7 @@ describe('singleBulkCreate', () => {
updatedBy: 'elastic',
interval: '5m',
enabled: true,
refresh: false,
tags: ['some fake tag 1', 'some fake tag 2'],
throttle: 'no_actions',
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { performance } from 'perf_hooks';
import { AlertServices } from '../../../../../../../plugins/alerting/server';
import { SignalSearchResponse, BulkResponse } from './types';
import { RuleAlertAction } from '../../../../common/detection_engine/types';
import { RuleTypeParams } from '../types';
import { RuleTypeParams, RefreshTypes } from '../types';
import { generateId, makeFloatString } from './utils';
import { buildBulkBody } from './build_bulk_body';
import { Logger } from '../../../../../../../../src/core/server';
Expand All @@ -31,6 +31,7 @@ interface SingleBulkCreateParams {
enabled: boolean;
tags: string[];
throttle: string;
refresh: RefreshTypes;
}

/**
Expand Down Expand Up @@ -77,6 +78,7 @@ export const singleBulkCreate = async ({
updatedBy,
interval,
enabled,
refresh,
tags,
throttle,
}: SingleBulkCreateParams): Promise<SingleBulkCreateResponse> => {
Expand Down Expand Up @@ -124,7 +126,7 @@ export const singleBulkCreate = async ({
const start = performance.now();
const response: BulkResponse = await services.callCluster('bulk', {
index: signalsIndex,
refresh: false,
refresh,
body: bulkBody,
});
const end = performance.now();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,3 +149,5 @@ export type CallWithRequest<T extends Record<string, any>, V> = (
params: T,
options?: CallAPIOptions
) => Promise<V>;

export type RefreshTypes = false | 'wait_for';

0 comments on commit 274cb80

Please sign in to comment.