github-mirrors/kibana
1
0
Fork 0
mirror of https://github.com/elastic/kibana.git synced 2025-04-24 09:48:58 -04:00
Code Issues Projects Releases Packages Wiki Activity

[Security Solution][Detection Engine] Fixes degraded state when signals detected are between 0 and 100 (#77658)

Browse source 
## Summary

Fixes: https://github.com/elastic/kibana/issues/77342

No easy way to unit test/end to end test this as it was operating correctly before and passed all of our tests, it was just running in a slow state if you had between 0 and 100 signals. The best bet is that you hand run the tests from 77342 or use a large look back time to ensure de-duplicate does not run as outlined in 77342.


Also this PR removes a TODO block, a complexity linter issue we had, a few await that were there by accident, and pushes down arrays to make things to be cleaner.
This commit is contained in:
Frank Hassanabad 2020-09-17 11:41:57 -06:00 • committed by GitHub
parent d88b3a6dde
commit 9497d62d4a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 36 additions and 94 deletions
Download patch file Download diff file Expand all files Collapse all files

2
x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.test.ts
View file

@ -559,7 +559,7 @@ describe('searchAfterAndBulkCreate', () => {
// I don't like testing log statements since logs change but this is the best
// way I can think of to ensure this section is getting hit with this test case.
expect(((mockLogger.debug as unknown) as jest.Mock).mock.calls[15][0]).toContain(
'sortIds was empty on filteredEvents'
'sortIds was empty on searchResult name: "fake name" id: "fake id" rule id: "fake rule id" signals index: "fakeindex"'
);
});

60
x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.ts
View file

@ -3,8 +3,6 @@
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
/* eslint-disable complexity */
import moment from 'moment';
import { AlertServices } from '../../../../../alerts/server';
@ -25,7 +23,7 @@ interface SearchAfterAndBulkCreateParams {
previousStartedAt: Date | null | undefined;
ruleParams: RuleTypeParams;
services: AlertServices;
listClient: ListClient | undefined; // TODO: undefined is for temporary development, remove before merged
listClient: ListClient;
exceptionsList: ExceptionListItemSchema[];
logger: Logger;
id: string;
@ -91,7 +89,7 @@ export const searchAfterAndBulkCreate = async ({
};
// sortId tells us where to start our next consecutive search_after query
let sortId;
let sortId: string | undefined;
// signalsCreatedCount keeps track of how many signals we have created,
// to ensure we don't exceed maxSignals
@ -155,7 +153,7 @@ export const searchAfterAndBulkCreate = async ({
// yields zero hits, but there were hits using the previous
// sortIds.
// e.g. totalHits was 156, index 50 of 100 results, do another search-after
// this time with a new sortId, index 22 of the remainding 56, get another sortId
// this time with a new sortId, index 22 of the remaining 56, get another sortId
// search with that sortId, total is still 156 but the hits.hits array is empty.
if (totalHits === 0 || searchResult.hits.hits.length === 0) {
logger.debug(
@ -178,16 +176,13 @@ export const searchAfterAndBulkCreate = async ({
// filter out the search results that match with the values found in the list.
// the resulting set are signals to be indexed, given they are not duplicates
// of signals already present in the signals index.
const filteredEvents: SignalSearchResponse =
listClient != null
? await filterEventsAgainstList({
listClient,
exceptionsList,
logger,
eventSearchResult: searchResult,
buildRuleMessage,
})
: searchResult;
const filteredEvents: SignalSearchResponse = await filterEventsAgainstList({
listClient,
exceptionsList,
logger,
eventSearchResult: searchResult,
buildRuleMessage,
});
// only bulk create if there are filteredEvents leftover
// if there isn't anything after going through the value list filter
@ -234,33 +229,18 @@ export const searchAfterAndBulkCreate = async ({
logger.debug(
buildRuleMessage(`filteredEvents.hits.hits: ${filteredEvents.hits.hits.length}`)
);
}
if (
filteredEvents.hits.hits[0].sort != null &&
filteredEvents.hits.hits[0].sort.length !== 0
) {
sortId = filteredEvents.hits.hits[0].sort
? filteredEvents.hits.hits[0].sort[0]
: undefined;
} else {
logger.debug(buildRuleMessage('sortIds was empty on filteredEvents'));
toReturn.success = true;
break;
}
// we are guaranteed to have searchResult hits at this point
// because we check before if the totalHits or
// searchResult.hits.hits.length is 0
const lastSortId = searchResult.hits.hits[searchResult.hits.hits.length - 1].sort;
if (lastSortId != null && lastSortId.length !== 0) {
sortId = lastSortId[0];
} else {
// we are guaranteed to have searchResult hits at this point
// because we check before if the totalHits or
// searchResult.hits.hits.length is 0
if (
searchResult.hits.hits[0].sort != null &&
searchResult.hits.hits[0].sort.length !== 0
) {
sortId = searchResult.hits.hits[0].sort ? searchResult.hits.hits[0].sort[0] : undefined;
} else {
logger.debug(buildRuleMessage('sortIds was empty on searchResult'));
toReturn.success = true;
break;
}
logger.debug(buildRuleMessage('sortIds was empty on searchResult'));
toReturn.success = true;
break;
}
} catch (exc) {
logger.error(buildRuleMessage(`[-] search_after and bulk threw an error ${exc}`));

4
x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.ts
View file

@ -159,7 +159,7 @@ export const signalRulesAlertType = ({
}
}
try {
const { listClient, exceptionsClient } = await getListsClient({
const { listClient, exceptionsClient } = getListsClient({
services,
updatedByUser,
spaceId,
@ -168,7 +168,7 @@ export const signalRulesAlertType = ({
});
const exceptionItems = await getExceptions({
client: exceptionsClient,
lists: exceptionsList,
lists: exceptionsList ?? [],
});
if (isMlRule(type)) {

34
x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.test.ts
View file

@ -558,7 +558,7 @@ describe('utils', () => {
});
test('it successfully returns list and exceptions list client', async () => {
const { listClient, exceptionsClient } = await getListsClient({
const { listClient, exceptionsClient } = getListsClient({
services: alertServices,
savedObjectClient: alertServices.savedObjectsClient,
updatedByUser: 'some_user',
@ -569,18 +569,6 @@ describe('utils', () => {
expect(listClient).toBeDefined();
expect(exceptionsClient).toBeDefined();
});
test('it throws if "lists" is undefined', async () => {
await expect(() =>
getListsClient({
services: alertServices,
savedObjectClient: alertServices.savedObjectsClient,
updatedByUser: 'some_user',
spaceId: '',
lists: undefined,
})
).rejects.toThrowError('lists plugin unavailable during rule execution');
});
});
describe('getSignalTimeTuples', () => {
@ -743,24 +731,6 @@ describe('utils', () => {
expect(exceptions).toEqual([getExceptionListItemSchemaMock()]);
});
test('it throws if "client" is undefined', async () => {
await expect(() =>
getExceptions({
client: undefined,
lists: getListArrayMock(),
})
).rejects.toThrowError('lists plugin unavailable during rule execution');
});
test('it returns empty array if "lists" is undefined', async () => {
const exceptions = await getExceptions({
client: listMock.getExceptionListClient(),
lists: undefined,
});
expect(exceptions).toEqual([]);
});
test('it throws if "getExceptionListClient" fails', async () => {
const err = new Error('error fetching list');
listMock.getExceptionListClient = () =>
@ -799,7 +769,7 @@ describe('utils', () => {
const exceptions = await getExceptions({
client: listMock.getExceptionListClient(),
lists: undefined,
lists: [],
});
expect(exceptions).toEqual([]);

30
x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts
View file

@ -11,7 +11,7 @@ import { Logger, SavedObjectsClientContract } from '../../../../../../../src/cor
import { AlertServices, parseDuration } from '../../../../../alerts/server';
import { ExceptionListClient, ListClient, ListPluginSetup } from '../../../../../lists/server';
import { ExceptionListItemSchema } from '../../../../../lists/common/schemas';
import { ListArrayOrUndefined } from '../../../../common/detection_engine/schemas/types/lists';
import { ListArray } from '../../../../common/detection_engine/schemas/types/lists';
import { BulkResponse, BulkResponseErrorAggregation, isValidUnit } from './types';
import { BuildRuleMessage } from './rule_messages';
import { parseScheduleDates } from '../../../../common/detection_engine/parse_schedule_dates';
@ -118,7 +118,7 @@ export const getGapMaxCatchupRatio = ({
};
};
export const getListsClient = async ({
export const getListsClient = ({
lists,
spaceId,
updatedByUser,
@ -130,20 +130,16 @@ export const getListsClient = async ({
updatedByUser: string | null;
services: AlertServices;
savedObjectClient: SavedObjectsClientContract;
}): Promise<{
listClient: ListClient | undefined;
exceptionsClient: ExceptionListClient | undefined;
}> => {
}): {
listClient: ListClient;
exceptionsClient: ExceptionListClient;
} => {
if (lists == null) {
throw new Error('lists plugin unavailable during rule execution');
}
const listClient = await lists.getListClient(
services.callCluster,
spaceId,
updatedByUser ?? 'elastic'
);
const exceptionsClient = await lists.getExceptionListClient(
const listClient = lists.getListClient(services.callCluster, spaceId, updatedByUser ?? 'elastic');
const exceptionsClient = lists.getExceptionListClient(
savedObjectClient,
updatedByUser ?? 'elastic'
);
@ -155,14 +151,10 @@ export const getExceptions = async ({
client,
lists,
}: {
client: ExceptionListClient | undefined;
lists: ListArrayOrUndefined;
client: ExceptionListClient;
lists: ListArray;
}): Promise<ExceptionListItemSchema[] | undefined> => {
if (client == null) {
throw new Error('lists plugin unavailable during rule execution');
}
if (lists != null && lists.length > 0) {
if (lists.length > 0) {
try {
const listIds = lists.map(({ list_id: listId }) => listId);
const namespaceTypes = lists.map(({ namespace_type: namespaceType }) => namespaceType);