diff --git a/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.test.ts b/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.test.ts index fc26c837d5e52..267d671361184 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.test.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.test.ts @@ -96,8 +96,7 @@ describe('getSearchDsl', () => { mappings, opts.type, opts.sortField, - opts.sortOrder, - opts.pit + opts.sortOrder ); }); diff --git a/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts b/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts index cae5e43897bcf..9820544f02bd1 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts @@ -78,7 +78,7 @@ export function getSearchDsl( hasReferenceOperator, kueryNode, }), - ...getSortingParams(mappings, type, sortField, sortOrder, pit), + ...getSortingParams(mappings, type, sortField, sortOrder), ...(pit ? getPitParams(pit) : {}), ...(searchAfter ? { search_after: searchAfter } : {}), }; diff --git a/src/core/server/saved_objects/service/lib/search_dsl/sorting_params.test.ts b/src/core/server/saved_objects/service/lib/search_dsl/sorting_params.test.ts index 73c7065705fc5..1376f0d50a9da 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/sorting_params.test.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/sorting_params.test.ts @@ -79,11 +79,6 @@ describe('searchDsl/getSortParams', () => { ], }); }); - it('appends tiebreaker when PIT is provided', () => { - expect(getSortingParams(MAPPINGS, 'saved', 'title', undefined, { id: 'abc' }).sort).toEqual( - expect.arrayContaining([{ _shard_doc: 'asc' }]) - ); - }); }); describe('sortField is simple root property with multiple types', () => { it('returns correct params', () => { @@ -98,11 +93,6 @@ describe('searchDsl/getSortParams', () => { ], }); }); - it('appends tiebreaker when PIT is provided', () => { - expect( - getSortingParams(MAPPINGS, ['saved', 'pending'], 'type', undefined, { id: 'abc' }).sort - ).toEqual(expect.arrayContaining([{ _shard_doc: 'asc' }])); - }); }); describe('sortField is simple non-root property with multiple types', () => { it('returns correct params', () => { @@ -124,11 +114,6 @@ describe('searchDsl/getSortParams', () => { ], }); }); - it('appends tiebreaker when PIT is provided', () => { - expect( - getSortingParams(MAPPINGS, 'saved', 'title.raw', undefined, { id: 'abc' }).sort - ).toEqual(expect.arrayContaining([{ _shard_doc: 'asc' }])); - }); }); describe('sortField is multi-field with single type as array', () => { it('returns correct params', () => { @@ -143,11 +128,6 @@ describe('searchDsl/getSortParams', () => { ], }); }); - it('appends tiebreaker when PIT is provided', () => { - expect( - getSortingParams(MAPPINGS, ['saved'], 'title.raw', undefined, { id: 'abc' }).sort - ).toEqual(expect.arrayContaining([{ _shard_doc: 'asc' }])); - }); }); describe('sortField is root multi-field with multiple types', () => { it('returns correct params', () => { @@ -162,12 +142,6 @@ describe('searchDsl/getSortParams', () => { ], }); }); - it('appends tiebreaker when PIT is provided', () => { - expect( - getSortingParams(MAPPINGS, ['saved', 'pending'], 'type.raw', undefined, { id: 'abc' }) - .sort - ).toEqual(expect.arrayContaining([{ _shard_doc: 'asc' }])); - }); }); describe('sortField is not-root multi-field with multiple types', () => { it('returns correct params', () => { diff --git a/src/core/server/saved_objects/service/lib/search_dsl/sorting_params.ts b/src/core/server/saved_objects/service/lib/search_dsl/sorting_params.ts index abef9bfa0a300..e3bfba6a80f59 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/sorting_params.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/sorting_params.ts @@ -8,12 +8,6 @@ import Boom from '@hapi/boom'; import { getProperty, IndexMapping } from '../../../mappings'; -import { SavedObjectsPitParams } from '../../../types'; - -// TODO: The plan is for ES to automatically add this tiebreaker when -// using PIT. We should remove this logic once that is resolved. -// https://github.com/elastic/elasticsearch/issues/56828 -const ES_PROVIDED_TIEBREAKER = { _shard_doc: 'asc' }; const TOP_LEVEL_FIELDS = ['_id', '_score']; @@ -21,8 +15,7 @@ export function getSortingParams( mappings: IndexMapping, type: string | string[], sortField?: string, - sortOrder?: string, - pit?: SavedObjectsPitParams + sortOrder?: string ) { if (!sortField) { return {}; @@ -38,7 +31,6 @@ export function getSortingParams( order: sortOrder, }, }, - ...(pit ? [ES_PROVIDED_TIEBREAKER] : []), ], }; } @@ -59,7 +51,6 @@ export function getSortingParams( unmapped_type: rootField.type, }, }, - ...(pit ? [ES_PROVIDED_TIEBREAKER] : []), ], }; } @@ -84,7 +75,6 @@ export function getSortingParams( unmapped_type: field.type, }, }, - ...(pit ? [ES_PROVIDED_TIEBREAKER] : []), ], }; } diff --git a/src/plugins/embeddable/README.asciidoc b/src/plugins/embeddable/README.asciidoc index daa6040eab7eb..007b16587e9f8 100644 --- a/src/plugins/embeddable/README.asciidoc +++ b/src/plugins/embeddable/README.asciidoc @@ -22,18 +22,17 @@ There is also an example of rendering dashboard container outside of dashboard a === Docs -link:./docs/README.md[Embeddable docs, guides & caveats] +link:https://github.com/elastic/kibana/blob/master/src/plugins/embeddable/docs/README.md[Embeddable docs, guides & caveats] === API docs -==== Server API -https://github.com/elastic/kibana/blob/master/docs/development/plugins/embeddable/server/kibana-plugin-plugins-embeddable-server.embeddablesetup.md[Server Setup contract] -https://github.com/elastic/kibana/blob/master/docs/development/plugins/embeddable/server/kibana-plugin-plugins-embeddable-server.embeddablestart.md[Server Start contract] - ===== Browser API https://github.com/elastic/kibana/blob/master/docs/development/plugins/embeddable/public/kibana-plugin-plugins-embeddable-public.embeddablesetup.md[Browser Setup contract] https://github.com/elastic/kibana/blob/master/docs/development/plugins/embeddable/public/kibana-plugin-plugins-embeddable-public.embeddablestart.md[Browser Start contract] +==== Server API +https://github.com/elastic/kibana/blob/master/docs/development/plugins/embeddable/server/kibana-plugin-plugins-embeddable-server.embeddablesetup.md[Server Setup contract] + === Testing Run unit tests diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.test.ts index 55d128225c555..50823ebd85d05 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.test.ts @@ -5,7 +5,6 @@ * 2.0. */ -import moment from 'moment'; import { sampleRuleAlertParams, sampleEmptyDocSearchResults, @@ -23,9 +22,10 @@ import { alertsMock, AlertServicesMock } from '../../../../../alerts/server/mock import uuid from 'uuid'; import { listMock } from '../../../../../lists/server/mocks'; import { getExceptionListItemSchemaMock } from '../../../../../lists/common/schemas/response/exception_list_item_schema.mock'; -import { BulkResponse } from './types'; +import { BulkResponse, RuleRangeTuple } from './types'; import { SearchListItemArraySchema } from '../../../../../lists/common/schemas'; import { getSearchListItemResponseMock } from '../../../../../lists/common/schemas/response/search_list_item_schema.mock'; +import { getRuleRangeTuples } from './utils'; const buildRuleMessage = buildRuleMessageFactory({ id: 'fake id', @@ -39,16 +39,26 @@ describe('searchAfterAndBulkCreate', () => { let inputIndexPattern: string[] = []; let listClient = listMock.getListClient(); const someGuids = Array.from({ length: 13 }).map(() => uuid.v4()); + const sampleParams = sampleRuleAlertParams(30); + let tuples: RuleRangeTuple[]; beforeEach(() => { jest.clearAllMocks(); listClient = listMock.getListClient(); listClient.searchListItemByValues = jest.fn().mockResolvedValue([]); inputIndexPattern = ['auditbeat-*']; mockService = alertsMock.createAlertServices(); + ({ tuples } = getRuleRangeTuples({ + logger: mockLogger, + previousStartedAt: new Date(), + from: sampleParams.from, + to: sampleParams.to, + interval: '5m', + maxSignals: sampleParams.maxSignals, + buildRuleMessage, + })); }); test('should return success with number of searches less than max signals', async () => { - const sampleParams = sampleRuleAlertParams(30); mockService.callCluster .mockResolvedValueOnce(repeatedSearchResultsWithSortId(4, 1, someGuids.slice(0, 3))) .mockResolvedValueOnce({ @@ -112,11 +122,9 @@ describe('searchAfterAndBulkCreate', () => { }, }, ]; - const { success, createdSignalsCount, lastLookBackDate } = await searchAfterAndBulkCreate({ ruleParams: sampleParams, - gap: null, - previousStartedAt: new Date(), + tuples, listClient, exceptionsList: [exceptionItem], services: mockService, @@ -147,7 +155,6 @@ describe('searchAfterAndBulkCreate', () => { }); test('should return success with number of searches less than max signals with gap', async () => { - const sampleParams = sampleRuleAlertParams(30); mockService.callCluster .mockResolvedValueOnce(repeatedSearchResultsWithSortId(4, 1, someGuids.slice(0, 3))) .mockResolvedValueOnce({ @@ -201,8 +208,7 @@ describe('searchAfterAndBulkCreate', () => { ]; const { success, createdSignalsCount, lastLookBackDate } = await searchAfterAndBulkCreate({ ruleParams: sampleParams, - gap: moment.duration(2, 'm'), - previousStartedAt: moment().subtract(10, 'm').toDate(), + tuples, listClient, exceptionsList: [exceptionItem], services: mockService, @@ -233,7 +239,6 @@ describe('searchAfterAndBulkCreate', () => { }); test('should return success when no search results are in the allowlist', async () => { - const sampleParams = sampleRuleAlertParams(30); mockService.callCluster .mockResolvedValueOnce(repeatedSearchResultsWithSortId(4, 4, someGuids.slice(0, 3))) .mockResolvedValueOnce({ @@ -278,8 +283,7 @@ describe('searchAfterAndBulkCreate', () => { ]; const { success, createdSignalsCount, lastLookBackDate } = await searchAfterAndBulkCreate({ ruleParams: sampleParams, - gap: null, - previousStartedAt: new Date(), + tuples, listClient, exceptionsList: [exceptionItem], services: mockService, @@ -305,7 +309,7 @@ describe('searchAfterAndBulkCreate', () => { }); expect(success).toEqual(true); expect(mockService.callCluster).toHaveBeenCalledTimes(3); - expect(createdSignalsCount).toEqual(4); // should not create any signals because all events were in the allowlist + expect(createdSignalsCount).toEqual(4); expect(lastLookBackDate).toEqual(new Date('2020-04-20T21:27:45+0000')); }); @@ -316,7 +320,6 @@ describe('searchAfterAndBulkCreate', () => { { ...getSearchListItemResponseMock(), value: ['3.3.3.3'] }, ]; listClient.searchListItemByValues = jest.fn().mockResolvedValue(searchListItems); - const sampleParams = sampleRuleAlertParams(30); mockService.callCluster .mockResolvedValueOnce( repeatedSearchResultsWithSortId(4, 4, someGuids.slice(0, 3), [ @@ -342,8 +345,7 @@ describe('searchAfterAndBulkCreate', () => { ]; const { success, createdSignalsCount, lastLookBackDate } = await searchAfterAndBulkCreate({ ruleParams: sampleParams, - gap: null, - previousStartedAt: new Date(), + tuples, listClient, exceptionsList: [exceptionItem], services: mockService, @@ -382,7 +384,6 @@ describe('searchAfterAndBulkCreate', () => { ]; listClient.searchListItemByValues = jest.fn().mockResolvedValue(searchListItems); - const sampleParams = sampleRuleAlertParams(30); mockService.callCluster.mockResolvedValueOnce( repeatedSearchResultsWithNoSortId(4, 4, someGuids.slice(0, 3), [ '1.1.1.1', @@ -406,8 +407,7 @@ describe('searchAfterAndBulkCreate', () => { ]; const { success, createdSignalsCount, lastLookBackDate } = await searchAfterAndBulkCreate({ ruleParams: sampleParams, - gap: null, - previousStartedAt: new Date(), + tuples, listClient, exceptionsList: [exceptionItem], services: mockService, @@ -437,13 +437,12 @@ describe('searchAfterAndBulkCreate', () => { expect(lastLookBackDate).toEqual(new Date('2020-04-20T21:27:45+0000')); // 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[8][0]).toContain( + expect(((mockLogger.debug as unknown) as jest.Mock).mock.calls[7][0]).toContain( 'ran out of sort ids to sort on name: "fake name" id: "fake id" rule id: "fake rule id" signals index: "fakeindex"' ); }); test('should return success when no sortId present but search results are in the allowlist', async () => { - const sampleParams = sampleRuleAlertParams(30); mockService.callCluster .mockResolvedValueOnce(repeatedSearchResultsWithNoSortId(4, 4, someGuids.slice(0, 3))) .mockResolvedValueOnce({ @@ -487,8 +486,7 @@ describe('searchAfterAndBulkCreate', () => { ]; const { success, createdSignalsCount, lastLookBackDate } = await searchAfterAndBulkCreate({ ruleParams: sampleParams, - gap: null, - previousStartedAt: new Date(), + tuples, listClient, exceptionsList: [exceptionItem], services: mockService, @@ -514,17 +512,16 @@ describe('searchAfterAndBulkCreate', () => { }); expect(success).toEqual(true); expect(mockService.callCluster).toHaveBeenCalledTimes(2); - expect(createdSignalsCount).toEqual(4); // should not create any signals because all events were in the allowlist + expect(createdSignalsCount).toEqual(4); expect(lastLookBackDate).toEqual(new Date('2020-04-20T21:27:45+0000')); // 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( + expect(((mockLogger.debug as unknown) as jest.Mock).mock.calls[14][0]).toContain( 'ran out of sort ids to sort on name: "fake name" id: "fake id" rule id: "fake rule id" signals index: "fakeindex"' ); }); test('should return success when no exceptions list provided', async () => { - const sampleParams = sampleRuleAlertParams(30); mockService.callCluster .mockResolvedValueOnce(repeatedSearchResultsWithSortId(4, 4, someGuids.slice(0, 3))) .mockResolvedValueOnce({ @@ -565,8 +562,7 @@ describe('searchAfterAndBulkCreate', () => { ); const { success, createdSignalsCount, lastLookBackDate } = await searchAfterAndBulkCreate({ ruleParams: sampleParams, - gap: null, - previousStartedAt: new Date(), + tuples, listClient, exceptionsList: [], services: mockService, @@ -592,7 +588,7 @@ describe('searchAfterAndBulkCreate', () => { }); expect(success).toEqual(true); expect(mockService.callCluster).toHaveBeenCalledTimes(3); - expect(createdSignalsCount).toEqual(4); // should not create any signals because all events were in the allowlist + expect(createdSignalsCount).toEqual(4); expect(lastLookBackDate).toEqual(new Date('2020-04-20T21:27:45+0000')); }); @@ -609,15 +605,13 @@ describe('searchAfterAndBulkCreate', () => { }, }, ]; - const sampleParams = sampleRuleAlertParams(10); mockService.callCluster .mockResolvedValueOnce(repeatedSearchResultsWithSortId(4, 1, someGuids.slice(0, 3))) .mockRejectedValue(new Error('bulk failed')); // Added this recently const { success, createdSignalsCount, lastLookBackDate } = await searchAfterAndBulkCreate({ listClient, exceptionsList: [exceptionItem], - gap: null, - previousStartedAt: new Date(), + tuples, ruleParams: sampleParams, services: mockService, logger: mockLogger, @@ -659,7 +653,6 @@ describe('searchAfterAndBulkCreate', () => { }, }, ]; - const sampleParams = sampleRuleAlertParams(30); mockService.callCluster.mockResolvedValueOnce(sampleEmptyDocSearchResults()); listClient.searchListItemByValues = jest.fn(({ value }) => Promise.resolve( @@ -672,8 +665,7 @@ describe('searchAfterAndBulkCreate', () => { const { success, createdSignalsCount, lastLookBackDate } = await searchAfterAndBulkCreate({ listClient, exceptionsList: [exceptionItem], - gap: null, - previousStartedAt: new Date(), + tuples, ruleParams: sampleParams, services: mockService, logger: mockLogger, @@ -702,7 +694,6 @@ describe('searchAfterAndBulkCreate', () => { }); test('if returns false when singleSearchAfter throws an exception', async () => { - const sampleParams = sampleRuleAlertParams(10); mockService.callCluster .mockResolvedValueOnce({ took: 100, @@ -741,8 +732,7 @@ describe('searchAfterAndBulkCreate', () => { const { success, createdSignalsCount, lastLookBackDate } = await searchAfterAndBulkCreate({ listClient, exceptionsList: [exceptionItem], - gap: null, - previousStartedAt: new Date(), + tuples, ruleParams: sampleParams, services: mockService, logger: mockLogger, @@ -771,7 +761,6 @@ describe('searchAfterAndBulkCreate', () => { }); test('it returns error array when singleSearchAfter returns errors', async () => { - const sampleParams = sampleRuleAlertParams(30); const bulkItem: BulkResponse = { took: 100, errors: true, @@ -832,7 +821,6 @@ describe('searchAfterAndBulkCreate', () => { ], }) .mockResolvedValueOnce(sampleDocSearchResultsNoSortIdNoHits()); - const { success, createdSignalsCount, @@ -840,8 +828,7 @@ describe('searchAfterAndBulkCreate', () => { errors, } = await searchAfterAndBulkCreate({ ruleParams: sampleParams, - gap: null, - previousStartedAt: new Date(), + tuples, listClient, exceptionsList: [], services: mockService, @@ -873,7 +860,6 @@ describe('searchAfterAndBulkCreate', () => { }); it('invokes the enrichment callback with signal search results', async () => { - const sampleParams = sampleRuleAlertParams(30); mockService.callCluster .mockResolvedValueOnce(repeatedSearchResultsWithSortId(4, 1, someGuids.slice(0, 3))) .mockResolvedValueOnce({ @@ -917,8 +903,7 @@ describe('searchAfterAndBulkCreate', () => { const { success, createdSignalsCount, lastLookBackDate } = await searchAfterAndBulkCreate({ enrichment: mockEnrichment, ruleParams: sampleParams, - gap: moment.duration(2, 'm'), - previousStartedAt: moment().subtract(10, 'm').toDate(), + tuples, listClient, exceptionsList: [], services: mockService, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.ts index 061aa4bba5a41..1dd3a2d2173a8 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.ts @@ -17,7 +17,6 @@ import { createSearchResultReturnType, createSearchAfterReturnTypeFromResponse, createTotalHitsFromSearchResult, - getSignalTimeTuples, mergeReturns, mergeSearchResults, } from './utils'; @@ -25,8 +24,7 @@ import { SearchAfterAndBulkCreateParams, SearchAfterAndBulkCreateReturnType } fr // search_after through documents and re-index using bulk endpoint. export const searchAfterAndBulkCreate = async ({ - gap, - previousStartedAt, + tuples: totalToFromTuples, ruleParams, exceptionsList, services, @@ -64,16 +62,6 @@ export const searchAfterAndBulkCreate = async ({ // to ensure we don't exceed maxSignals let signalsCreatedCount = 0; - const totalToFromTuples = getSignalTimeTuples({ - logger, - ruleParamsFrom: ruleParams.from, - ruleParamsTo: ruleParams.to, - ruleParamsMaxSignals: ruleParams.maxSignals, - gap, - previousStartedAt, - interval, - buildRuleMessage, - }); const tuplesToBeLogged = [...totalToFromTuples]; logger.debug(buildRuleMessage(`totalToFromTuples: ${totalToFromTuples.length}`)); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.test.ts index a79961eb716fd..cadc6d0c5b7c0 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.test.ts @@ -11,14 +11,7 @@ import { getResult, getMlResult } from '../routes/__mocks__/request_responses'; import { signalRulesAlertType } from './signal_rule_alert_type'; import { alertsMock, AlertServicesMock } from '../../../../../alerts/server/mocks'; import { ruleStatusServiceFactory } from './rule_status_service'; -import { - getGapBetweenRuns, - getGapMaxCatchupRatio, - getListsClient, - getExceptions, - sortExceptionItems, - checkPrivileges, -} from './utils'; +import { getListsClient, getExceptions, sortExceptionItems, checkPrivileges } from './utils'; import { parseScheduleDates } from '../../../../common/detection_engine/parse_schedule_dates'; import { RuleExecutorOptions, SearchAfterAndBulkCreateReturnType } from './types'; import { searchAfterAndBulkCreate } from './search_after_bulk_create'; @@ -40,8 +33,6 @@ jest.mock('./utils', () => { const original = jest.requireActual('./utils'); return { ...original, - getGapBetweenRuns: jest.fn(), - getGapMaxCatchupRatio: jest.fn(), getListsClient: jest.fn(), getExceptions: jest.fn(), sortExceptionItems: jest.fn(), @@ -113,7 +104,6 @@ describe('rules_notification_alert_type', () => { warning: jest.fn(), }; (ruleStatusServiceFactory as jest.Mock).mockReturnValue(ruleStatusService); - (getGapBetweenRuns as jest.Mock).mockReturnValue(moment.duration(0)); (getListsClient as jest.Mock).mockReturnValue({ listClient: getListClientMock(), exceptionsClient: getExceptionListClientMock(), @@ -124,7 +114,6 @@ describe('rules_notification_alert_type', () => { exceptionsWithValueLists: [], }); (searchAfterAndBulkCreate as jest.Mock).mockClear(); - (getGapMaxCatchupRatio as jest.Mock).mockClear(); (searchAfterAndBulkCreate as jest.Mock).mockResolvedValue({ success: true, searchAfterTimes: [], @@ -187,23 +176,12 @@ describe('rules_notification_alert_type', () => { describe('executor', () => { it('should warn about the gap between runs if gap is very large', async () => { - (getGapBetweenRuns as jest.Mock).mockReturnValue(moment.duration(100, 'm')); - (getGapMaxCatchupRatio as jest.Mock).mockReturnValue({ - maxCatchup: 4, - ratio: 20, - gapDiffInUnits: 95, - }); + payload.previousStartedAt = moment().subtract(100, 'm').toDate(); await alert.executor(payload); expect(logger.warn).toHaveBeenCalled(); - expect(logger.warn.mock.calls[0][0]).toContain( - '2 hours (6000000ms) has passed since last rule execution, and signals may have been missed.' - ); expect(ruleStatusService.error).toHaveBeenCalled(); - expect(ruleStatusService.error.mock.calls[0][0]).toContain( - '2 hours (6000000ms) has passed since last rule execution, and signals may have been missed.' - ); expect(ruleStatusService.error.mock.calls[0][1]).toEqual({ - gap: '2 hours', + gap: 'an hour', }); }); @@ -257,12 +235,7 @@ describe('rules_notification_alert_type', () => { }); it('should NOT warn about the gap between runs if gap small', async () => { - (getGapBetweenRuns as jest.Mock).mockReturnValue(moment.duration(1, 'm')); - (getGapMaxCatchupRatio as jest.Mock).mockReturnValue({ - maxCatchup: 1, - ratio: 1, - gapDiffInUnits: 1, - }); + payload.previousStartedAt = moment().subtract(10, 'm').toDate(); await alert.executor(payload); expect(logger.warn).toHaveBeenCalledTimes(0); expect(ruleStatusService.error).toHaveBeenCalledTimes(0); @@ -450,6 +423,7 @@ describe('rules_notification_alert_type', () => { const ruleAlert = getMlResult(); ruleAlert.params.anomalyThreshold = undefined; payload = getPayload(ruleAlert, alertServices) as jest.Mocked; + payload.previousStartedAt = null; await alert.executor(payload); expect(logger.error).toHaveBeenCalled(); expect(logger.error.mock.calls[0][0]).toContain( @@ -460,6 +434,7 @@ describe('rules_notification_alert_type', () => { it('should throw an error if Machine learning job summary was null', async () => { const ruleAlert = getMlResult(); payload = getPayload(ruleAlert, alertServices) as jest.Mocked; + payload.previousStartedAt = null; jobsSummaryMock.mockResolvedValue([]); await alert.executor(payload); expect(logger.warn).toHaveBeenCalled(); @@ -473,6 +448,7 @@ describe('rules_notification_alert_type', () => { it('should log an error if Machine learning job was not started', async () => { const ruleAlert = getMlResult(); payload = getPayload(ruleAlert, alertServices) as jest.Mocked; + payload.previousStartedAt = null; jobsSummaryMock.mockResolvedValue([ { id: 'some_job_id', @@ -518,6 +494,7 @@ describe('rules_notification_alert_type', () => { it('should call ruleStatusService.success if signals were created', async () => { const ruleAlert = getMlResult(); payload = getPayload(ruleAlert, alertServices) as jest.Mocked; + payload.previousStartedAt = null; jobsSummaryMock.mockResolvedValue([ { id: 'some_job_id', @@ -544,6 +521,7 @@ describe('rules_notification_alert_type', () => { it('should not call checkPrivileges if ML rule', async () => { const ruleAlert = getMlResult(); payload = getPayload(ruleAlert, alertServices) as jest.Mocked; + payload.previousStartedAt = null; jobsSummaryMock.mockResolvedValue([ { id: 'some_job_id', diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.ts index 98c9dd41d179c..2025ba512cb65 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.ts @@ -37,11 +37,8 @@ import { WrappedSignalHit, } from './types'; import { - getGapBetweenRuns, getListsClient, getExceptions, - getGapMaxCatchupRatio, - MAX_RULE_GAP_RATIO, wrapSignal, createErrorsFromShard, createSearchAfterReturnType, @@ -50,6 +47,7 @@ import { checkPrivileges, hasTimestampFields, hasReadIndexPrivileges, + getRuleRangeTuples, } from './utils'; import { signalParamsSchema } from './signal_params_schema'; import { siemRuleActionGroups } from './siem_rule_action_groups'; @@ -230,29 +228,24 @@ export const signalRulesAlertType = ({ } catch (exc) { logger.error(buildRuleMessage(`Check privileges failed to execute ${exc}`)); } - - const gap = getGapBetweenRuns({ previousStartedAt, interval, from, to }); - if (gap != null && gap.asMilliseconds() > 0) { - const fromUnit = from[from.length - 1]; - const { ratio } = getGapMaxCatchupRatio({ - logger, - buildRuleMessage, - previousStartedAt, - ruleParamsFrom: from, - interval, - unit: fromUnit, - }); - if (ratio && ratio >= MAX_RULE_GAP_RATIO) { - const gapString = gap.humanize(); - const gapMessage = buildRuleMessage( - `${gapString} (${gap.asMilliseconds()}ms) has passed since last rule execution, and signals may have been missed.`, - 'Consider increasing your look behind time or adding more Kibana instances.' - ); - logger.warn(gapMessage); - - hasError = true; - await ruleStatusService.error(gapMessage, { gap: gapString }); - } + const { tuples, remainingGap } = getRuleRangeTuples({ + logger, + previousStartedAt, + from, + to, + interval, + maxSignals, + buildRuleMessage, + }); + if (remainingGap.asMilliseconds() > 0) { + const gapString = remainingGap.humanize(); + const gapMessage = buildRuleMessage( + `${gapString} (${remainingGap.asMilliseconds()}ms) were not queried between this rule execution and the last execution, so signals may have been missed.`, + 'Consider increasing your look behind time or adding more Kibana instances.' + ); + logger.warn(gapMessage); + hasError = true; + await ruleStatusService.error(gapMessage, { gap: gapString }); } try { const { listClient, exceptionsClient } = getListsClient({ @@ -479,6 +472,7 @@ export const signalRulesAlertType = ({ } const inputIndex = await getInputIndex(services, version, index); result = await createThreatSignals({ + tuples, threatMapping, query, inputIndex, @@ -489,8 +483,6 @@ export const signalRulesAlertType = ({ savedId, services, exceptionItems: exceptionItems ?? [], - gap, - previousStartedAt, listClient, logger, eventsTelemetry, @@ -531,8 +523,7 @@ export const signalRulesAlertType = ({ }); result = await searchAfterAndBulkCreate({ - gap, - previousStartedAt, + tuples, listClient, exceptionsList: exceptionItems ?? [], ruleParams: params, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threat_mapping/create_threat_signal.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threat_mapping/create_threat_signal.ts index ba428bc077125..d9c72f7f95679 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threat_mapping/create_threat_signal.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threat_mapping/create_threat_signal.ts @@ -13,6 +13,7 @@ import { CreateThreatSignalOptions } from './types'; import { SearchAfterAndBulkCreateReturnType } from '../types'; export const createThreatSignal = async ({ + tuples, threatMapping, threatEnrichment, query, @@ -23,8 +24,6 @@ export const createThreatSignal = async ({ savedId, services, exceptionItems, - gap, - previousStartedAt, listClient, logger, eventsTelemetry, @@ -80,8 +79,7 @@ export const createThreatSignal = async ({ ); const result = await searchAfterAndBulkCreate({ - gap, - previousStartedAt, + tuples, listClient, exceptionsList: exceptionItems, ruleParams: params, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threat_mapping/create_threat_signals.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threat_mapping/create_threat_signals.ts index e45aea29c423f..854c2b8f3fdc1 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threat_mapping/create_threat_signals.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threat_mapping/create_threat_signals.ts @@ -15,6 +15,7 @@ import { combineConcurrentResults } from './utils'; import { buildThreatEnrichment } from './build_threat_enrichment'; export const createThreatSignals = async ({ + tuples, threatMapping, query, inputIndex, @@ -24,8 +25,6 @@ export const createThreatSignals = async ({ savedId, services, exceptionItems, - gap, - previousStartedAt, listClient, logger, eventsTelemetry, @@ -111,6 +110,7 @@ export const createThreatSignals = async ({ const concurrentSearchesPerformed = chunks.map>( (slicedChunk) => createThreatSignal({ + tuples, threatEnrichment, threatMapping, query, @@ -121,8 +121,6 @@ export const createThreatSignals = async ({ savedId, services, exceptionItems, - gap, - previousStartedAt, listClient, logger, eventsTelemetry, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threat_mapping/types.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threat_mapping/types.ts index a022cbbdd4042..1c35a5af09b38 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threat_mapping/types.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threat_mapping/types.ts @@ -6,7 +6,6 @@ */ import { SearchResponse } from 'elasticsearch'; -import { Duration } from 'moment'; import { ListClient } from '../../../../../../lists/server'; import { @@ -34,11 +33,12 @@ import { ILegacyScopedClusterClient, Logger } from '../../../../../../../../src/ import { RuleAlertAction } from '../../../../../common/detection_engine/types'; import { TelemetryEventsSender } from '../../../telemetry/sender'; import { BuildRuleMessage } from '../rule_messages'; -import { SearchAfterAndBulkCreateReturnType, SignalsEnrichment } from '../types'; +import { RuleRangeTuple, SearchAfterAndBulkCreateReturnType, SignalsEnrichment } from '../types'; export type SortOrderOrUndefined = 'asc' | 'desc' | undefined; export interface CreateThreatSignalsOptions { + tuples: RuleRangeTuple[]; threatMapping: ThreatMapping; query: string; inputIndex: string[]; @@ -48,8 +48,6 @@ export interface CreateThreatSignalsOptions { savedId: string | undefined; services: AlertServices; exceptionItems: ExceptionListItemSchema[]; - gap: Duration | null; - previousStartedAt: Date | null; listClient: ListClient; logger: Logger; eventsTelemetry: TelemetryEventsSender | undefined; @@ -79,6 +77,7 @@ export interface CreateThreatSignalsOptions { } export interface CreateThreatSignalOptions { + tuples: RuleRangeTuple[]; threatMapping: ThreatMapping; threatEnrichment: SignalsEnrichment; query: string; @@ -89,8 +88,6 @@ export interface CreateThreatSignalOptions { savedId: string | undefined; services: AlertServices; exceptionItems: ExceptionListItemSchema[]; - gap: Duration | null; - previousStartedAt: Date | null; listClient: ListClient; logger: Logger; eventsTelemetry: TelemetryEventsSender | undefined; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/types.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/types.ts index e5ca1f6a60456..f759da31566e2 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/types.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/types.ts @@ -73,6 +73,12 @@ export interface ThresholdSignalHistory { [hash: string]: ThresholdSignalHistoryRecord; } +export interface RuleRangeTuple { + to: moment.Moment; + from: moment.Moment; + maxSignals: number; +} + export interface SignalSource { [key: string]: SearchTypes; // TODO: SignalSource is being used as the type for documents matching detection engine queries, but they may not @@ -251,8 +257,11 @@ export interface QueryFilter { export type SignalsEnrichment = (signals: SignalSearchResponse) => Promise; export interface SearchAfterAndBulkCreateParams { - gap: moment.Duration | null; - previousStartedAt: Date | null | undefined; + tuples: Array<{ + to: moment.Moment; + from: moment.Moment; + maxSignals: number; + }>; ruleParams: RuleTypeParams; services: AlertServices; listClient: ListClient; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.test.ts index 7888bb6deaab7..0a581816ee82f 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.test.ts @@ -25,10 +25,10 @@ import { parseInterval, getDriftTolerance, getGapBetweenRuns, - getGapMaxCatchupRatio, + getNumCatchupIntervals, errorAggregator, getListsClient, - getSignalTimeTuples, + getRuleRangeTuples, getExceptions, hasTimestampFields, wrapBuildingBlocks, @@ -109,7 +109,7 @@ describe('utils', () => { expect(duration?.asMilliseconds()).toEqual(moment.duration(5, 'minutes').asMilliseconds()); }); - test('it returns null given an invalid duration', () => { + test('it throws given an invalid duration', () => { const duration = parseInterval('junk'); expect(duration).toBeNull(); }); @@ -148,7 +148,7 @@ describe('utils', () => { const drift = getDriftTolerance({ from: 'now-6m', to: 'now', - interval: moment.duration(5, 'minutes'), + intervalDuration: moment.duration(5, 'minutes'), }); expect(drift).not.toBeNull(); expect(drift?.asMilliseconds()).toEqual(moment.duration(1, 'minute').asMilliseconds()); @@ -158,7 +158,7 @@ describe('utils', () => { const drift = getDriftTolerance({ from: 'now-5m', to: 'now', - interval: moment.duration(5, 'minutes'), + intervalDuration: moment.duration(5, 'minutes'), }); expect(drift?.asMilliseconds()).toEqual(0); }); @@ -167,7 +167,7 @@ describe('utils', () => { const drift = getDriftTolerance({ from: 'now-10m', to: 'now', - interval: moment.duration(5, 'minutes'), + intervalDuration: moment.duration(5, 'minutes'), }); expect(drift).not.toBeNull(); expect(drift?.asMilliseconds()).toEqual(moment.duration(5, 'minutes').asMilliseconds()); @@ -177,7 +177,7 @@ describe('utils', () => { const drift = getDriftTolerance({ from: 'now-10m', to: 'now', - interval: moment.duration(0, 'milliseconds'), + intervalDuration: moment.duration(0, 'milliseconds'), }); expect(drift).not.toBeNull(); expect(drift?.asMilliseconds()).toEqual(moment.duration(10, 'minutes').asMilliseconds()); @@ -187,7 +187,7 @@ describe('utils', () => { const drift = getDriftTolerance({ from: 'invalid', to: 'now', - interval: moment.duration(5, 'minutes'), + intervalDuration: moment.duration(5, 'minutes'), }); expect(drift).not.toBeNull(); expect(drift?.asMilliseconds()).toEqual(moment.duration(1, 'minute').asMilliseconds()); @@ -197,7 +197,7 @@ describe('utils', () => { const drift = getDriftTolerance({ from: '10m', to: 'now', - interval: moment.duration(5, 'minutes'), + intervalDuration: moment.duration(5, 'minutes'), }); expect(drift).not.toBeNull(); expect(drift?.asMilliseconds()).toEqual(moment.duration(1, 'minute').asMilliseconds()); @@ -207,7 +207,7 @@ describe('utils', () => { const drift = getDriftTolerance({ from: 'now-10m', to: 'now-1m', - interval: moment.duration(5, 'minutes'), + intervalDuration: moment.duration(5, 'minutes'), }); expect(drift).not.toBeNull(); expect(drift?.asMilliseconds()).toEqual(moment.duration(4, 'minutes').asMilliseconds()); @@ -217,7 +217,7 @@ describe('utils', () => { const drift = getDriftTolerance({ from: moment().subtract(10, 'minutes').toISOString(), to: 'now', - interval: moment.duration(5, 'minutes'), + intervalDuration: moment.duration(5, 'minutes'), }); expect(drift).not.toBeNull(); expect(drift?.asMilliseconds()).toEqual(moment.duration(5, 'minutes').asMilliseconds()); @@ -227,7 +227,7 @@ describe('utils', () => { const drift = getDriftTolerance({ from: 'now-6m', to: moment().toISOString(), - interval: moment.duration(5, 'minutes'), + intervalDuration: moment.duration(5, 'minutes'), }); expect(drift).not.toBeNull(); expect(drift?.asMilliseconds()).toEqual(moment.duration(1, 'minute').asMilliseconds()); @@ -238,7 +238,7 @@ describe('utils', () => { test('it returns a gap of 0 when "from" and interval match each other and the previous started was from the previous interval time', () => { const gap = getGapBetweenRuns({ previousStartedAt: nowDate.clone().subtract(5, 'minutes').toDate(), - interval: '5m', + intervalDuration: moment.duration(5, 'minutes'), from: 'now-5m', to: 'now', now: nowDate.clone(), @@ -250,7 +250,7 @@ describe('utils', () => { test('it returns a negative gap of 1 minute when "from" overlaps to by 1 minute and the previousStartedAt was 5 minutes ago', () => { const gap = getGapBetweenRuns({ previousStartedAt: nowDate.clone().subtract(5, 'minutes').toDate(), - interval: '5m', + intervalDuration: moment.duration(5, 'minutes'), from: 'now-6m', to: 'now', now: nowDate.clone(), @@ -262,7 +262,7 @@ describe('utils', () => { test('it returns a negative gap of 5 minutes when "from" overlaps to by 1 minute and the previousStartedAt was 5 minutes ago', () => { const gap = getGapBetweenRuns({ previousStartedAt: nowDate.clone().subtract(5, 'minutes').toDate(), - interval: '5m', + intervalDuration: moment.duration(5, 'minutes'), from: 'now-10m', to: 'now', now: nowDate.clone(), @@ -274,7 +274,7 @@ describe('utils', () => { test('it returns a negative gap of 1 minute when "from" overlaps to by 1 minute and the previousStartedAt was 10 minutes ago and so was the interval', () => { const gap = getGapBetweenRuns({ previousStartedAt: nowDate.clone().subtract(10, 'minutes').toDate(), - interval: '10m', + intervalDuration: moment.duration(10, 'minutes'), from: 'now-11m', to: 'now', now: nowDate.clone(), @@ -286,7 +286,7 @@ describe('utils', () => { test('it returns a gap of only -30 seconds when the from overlaps with now by 1 minute, the interval is 5 minutes but the previous started is 30 seconds more', () => { const gap = getGapBetweenRuns({ previousStartedAt: nowDate.clone().subtract(5, 'minutes').subtract(30, 'seconds').toDate(), - interval: '5m', + intervalDuration: moment.duration(5, 'minutes'), from: 'now-6m', to: 'now', now: nowDate.clone(), @@ -298,7 +298,7 @@ describe('utils', () => { test('it returns an exact 0 gap when the from overlaps with now by 1 minute, the interval is 5 minutes but the previous started is one minute late', () => { const gap = getGapBetweenRuns({ previousStartedAt: nowDate.clone().subtract(6, 'minutes').toDate(), - interval: '5m', + intervalDuration: moment.duration(5, 'minutes'), from: 'now-6m', to: 'now', now: nowDate.clone(), @@ -310,7 +310,7 @@ describe('utils', () => { test('it returns a gap of 30 seconds when the from overlaps with now by 1 minute, the interval is 5 minutes but the previous started is one minute and 30 seconds late', () => { const gap = getGapBetweenRuns({ previousStartedAt: nowDate.clone().subtract(6, 'minutes').subtract(30, 'seconds').toDate(), - interval: '5m', + intervalDuration: moment.duration(5, 'minutes'), from: 'now-6m', to: 'now', now: nowDate.clone(), @@ -322,7 +322,7 @@ describe('utils', () => { test('it returns a gap of 1 minute when the from overlaps with now by 1 minute, the interval is 5 minutes but the previous started is two minutes late', () => { const gap = getGapBetweenRuns({ previousStartedAt: nowDate.clone().subtract(7, 'minutes').toDate(), - interval: '5m', + intervalDuration: moment.duration(5, 'minutes'), from: 'now-6m', to: 'now', now: nowDate.clone(), @@ -331,32 +331,21 @@ describe('utils', () => { expect(gap?.asMilliseconds()).toEqual(moment.duration(1, 'minute').asMilliseconds()); }); - test('it returns null if given a previousStartedAt of null', () => { + test('it returns 0 if given a previousStartedAt of null', () => { const gap = getGapBetweenRuns({ previousStartedAt: null, - interval: '5m', + intervalDuration: moment.duration(5, 'minutes'), from: 'now-5m', to: 'now', now: nowDate.clone(), }); - expect(gap).toBeNull(); - }); - - test('it returns null if the interval is an invalid string such as "invalid"', () => { - const gap = getGapBetweenRuns({ - previousStartedAt: nowDate.clone().toDate(), - interval: 'invalid', // if not set to "x" where x is an interval such as 6m - from: 'now-5m', - to: 'now', - now: nowDate.clone(), - }); - expect(gap).toBeNull(); + expect(gap.asMilliseconds()).toEqual(0); }); test('it returns the expected result when "from" is an invalid string such as "invalid"', () => { const gap = getGapBetweenRuns({ previousStartedAt: nowDate.clone().subtract(7, 'minutes').toDate(), - interval: '5m', + intervalDuration: moment.duration(5, 'minutes'), from: 'invalid', to: 'now', now: nowDate.clone(), @@ -368,7 +357,7 @@ describe('utils', () => { test('it returns the expected result when "to" is an invalid string such as "invalid"', () => { const gap = getGapBetweenRuns({ previousStartedAt: nowDate.clone().subtract(7, 'minutes').toDate(), - interval: '5m', + intervalDuration: moment.duration(5, 'minutes'), from: 'now-6m', to: 'invalid', now: nowDate.clone(), @@ -609,134 +598,116 @@ describe('utils', () => { }); }); - describe('getSignalTimeTuples', () => { + describe('getRuleRangeTuples', () => { test('should return a single tuple if no gap', () => { - const someTuples = getSignalTimeTuples({ + const { tuples, remainingGap } = getRuleRangeTuples({ logger: mockLogger, - gap: null, previousStartedAt: moment().subtract(30, 's').toDate(), interval: '30s', - ruleParamsFrom: 'now-30s', - ruleParamsTo: 'now', - ruleParamsMaxSignals: 20, + from: 'now-30s', + to: 'now', + maxSignals: 20, buildRuleMessage, }); - const someTuple = someTuples[0]; + const someTuple = tuples[0]; expect(moment(someTuple.to).diff(moment(someTuple.from), 's')).toEqual(30); + expect(tuples.length).toEqual(1); + expect(remainingGap.asMilliseconds()).toEqual(0); + }); + + test('should return a single tuple if malformed interval prevents gap calculation', () => { + const { tuples, remainingGap } = getRuleRangeTuples({ + logger: mockLogger, + previousStartedAt: moment().subtract(30, 's').toDate(), + interval: 'invalid', + from: 'now-30s', + to: 'now', + maxSignals: 20, + buildRuleMessage, + }); + const someTuple = tuples[0]; + expect(moment(someTuple.to).diff(moment(someTuple.from), 's')).toEqual(30); + expect(tuples.length).toEqual(1); + expect(remainingGap.asMilliseconds()).toEqual(0); }); test('should return two tuples if gap and previouslyStartedAt', () => { - const someTuples = getSignalTimeTuples({ + const { tuples, remainingGap } = getRuleRangeTuples({ logger: mockLogger, - gap: moment.duration(10, 's'), previousStartedAt: moment().subtract(65, 's').toDate(), interval: '50s', - ruleParamsFrom: 'now-55s', - ruleParamsTo: 'now', - ruleParamsMaxSignals: 20, + from: 'now-55s', + to: 'now', + maxSignals: 20, buildRuleMessage, }); - const someTuple = someTuples[1]; - expect(moment(someTuple.to).diff(moment(someTuple.from), 's')).toEqual(10); + const someTuple = tuples[1]; + expect(moment(someTuple.to).diff(moment(someTuple.from), 's')).toEqual(55); + expect(remainingGap.asMilliseconds()).toEqual(0); }); test('should return five tuples when give long gap', () => { - const someTuples = getSignalTimeTuples({ + const { tuples, remainingGap } = getRuleRangeTuples({ logger: mockLogger, - gap: moment.duration(65, 's'), // 64 is 5 times the interval + lookback, which will trigger max lookback - previousStartedAt: moment().subtract(65, 's').toDate(), + previousStartedAt: moment().subtract(65, 's').toDate(), // 64 is 5 times the interval + lookback, which will trigger max lookback interval: '10s', - ruleParamsFrom: 'now-13s', - ruleParamsTo: 'now', - ruleParamsMaxSignals: 20, + from: 'now-13s', + to: 'now', + maxSignals: 20, buildRuleMessage, }); - expect(someTuples.length).toEqual(5); - someTuples.forEach((item, index) => { + expect(tuples.length).toEqual(5); + tuples.forEach((item, index) => { if (index === 0) { return; } - expect(moment(item.to).diff(moment(item.from), 's')).toEqual(10); + expect(moment(item.to).diff(moment(item.from), 's')).toEqual(13); + expect(item.to.diff(tuples[index - 1].to, 's')).toEqual(-10); + expect(item.from.diff(tuples[index - 1].from, 's')).toEqual(-10); }); + expect(remainingGap.asMilliseconds()).toEqual(12000); }); - // this tests if calculatedFrom in utils.ts:320 parses an int and not a float - // if we don't parse as an int, then dateMath.parse will fail - // as it doesn't support parsing `now-67.549`, it only supports ints like `now-67`. - test('should return five tuples when given a gap with a decimal to ensure no parsing errors', () => { - const someTuples = getSignalTimeTuples({ + test('should return a single tuple when give a negative gap (rule ran sooner than expected)', () => { + const { tuples, remainingGap } = getRuleRangeTuples({ logger: mockLogger, - gap: moment.duration(67549, 'ms'), // 64 is 5 times the interval + lookback, which will trigger max lookback - previousStartedAt: moment().subtract(67549, 'ms').toDate(), - interval: '10s', - ruleParamsFrom: 'now-13s', - ruleParamsTo: 'now', - ruleParamsMaxSignals: 20, - buildRuleMessage, - }); - expect(someTuples.length).toEqual(5); - }); - - test('should return single tuples when give a negative gap (rule ran sooner than expected)', () => { - const someTuples = getSignalTimeTuples({ - logger: mockLogger, - gap: moment.duration(-15, 's'), // 64 is 5 times the interval + lookback, which will trigger max lookback previousStartedAt: moment().subtract(-15, 's').toDate(), interval: '10s', - ruleParamsFrom: 'now-13s', - ruleParamsTo: 'now', - ruleParamsMaxSignals: 20, + from: 'now-13s', + to: 'now', + maxSignals: 20, buildRuleMessage, }); - expect(someTuples.length).toEqual(1); - const someTuple = someTuples[0]; + expect(tuples.length).toEqual(1); + const someTuple = tuples[0]; expect(moment(someTuple.to).diff(moment(someTuple.from), 's')).toEqual(13); + expect(remainingGap.asMilliseconds()).toEqual(0); }); }); describe('getMaxCatchupRatio', () => { - test('should return null if rule has never run before', () => { - const { maxCatchup, ratio, gapDiffInUnits } = getGapMaxCatchupRatio({ - logger: mockLogger, - previousStartedAt: null, - interval: '30s', - ruleParamsFrom: 'now-30s', - buildRuleMessage, - unit: 's', + test('should return 0 if gap is 0', () => { + const catchup = getNumCatchupIntervals({ + gap: moment.duration(0), + intervalDuration: moment.duration(11000), }); - expect(maxCatchup).toBeNull(); - expect(ratio).toBeNull(); - expect(gapDiffInUnits).toBeNull(); + expect(catchup).toEqual(0); }); - test('should should have non-null values when gap is present', () => { - const { maxCatchup, ratio, gapDiffInUnits } = getGapMaxCatchupRatio({ - logger: mockLogger, - previousStartedAt: moment().subtract(65, 's').toDate(), - interval: '50s', - ruleParamsFrom: 'now-55s', - buildRuleMessage, - unit: 's', + test('should return 1 if gap is in (0, intervalDuration]', () => { + const catchup = getNumCatchupIntervals({ + gap: moment.duration(10000), + intervalDuration: moment.duration(10000), }); - expect(maxCatchup).toEqual(0.2); - expect(ratio).toEqual(0.2); - expect(gapDiffInUnits).toEqual(10); + expect(catchup).toEqual(1); }); - // when a rule runs sooner than expected we don't - // consider that a gap as that is a very rare circumstance - test('should return null when given a negative gap (rule ran sooner than expected)', () => { - const { maxCatchup, ratio, gapDiffInUnits } = getGapMaxCatchupRatio({ - logger: mockLogger, - previousStartedAt: moment().subtract(-15, 's').toDate(), - interval: '10s', - ruleParamsFrom: 'now-13s', - buildRuleMessage, - unit: 's', + test('should round up return value', () => { + const catchup = getNumCatchupIntervals({ + gap: moment.duration(15000), + intervalDuration: moment.duration(11000), }); - expect(maxCatchup).toBeNull(); - expect(ratio).toBeNull(); - expect(gapDiffInUnits).toBeNull(); + expect(catchup).toEqual(2); }); }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts index 58bf22be97bf8..2b306cd2a8d9d 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts @@ -29,12 +29,12 @@ import { ListArray } from '../../../../common/detection_engine/schemas/types/lis import { BulkResponse, BulkResponseErrorAggregation, - isValidUnit, SignalHit, SearchAfterAndBulkCreateReturnType, SignalSearchResponse, Signal, WrappedSignalHit, + RuleRangeTuple, } from './types'; import { BuildRuleMessage } from './rule_messages'; import { parseScheduleDates } from '../../../../common/detection_engine/parse_schedule_dates'; @@ -163,82 +163,21 @@ export const checkPrivileges = async ( }, }); -export const getGapMaxCatchupRatio = ({ - logger, - previousStartedAt, - unit, - buildRuleMessage, - ruleParamsFrom, - interval, +export const getNumCatchupIntervals = ({ + gap, + intervalDuration, }: { - logger: Logger; - ruleParamsFrom: string; - previousStartedAt: Date | null | undefined; - interval: string; - buildRuleMessage: BuildRuleMessage; - unit: string; -}): { - maxCatchup: number | null; - ratio: number | null; - gapDiffInUnits: number | null; -} => { - if (previousStartedAt == null) { - return { - maxCatchup: null, - ratio: null, - gapDiffInUnits: null, - }; - } - if (!isValidUnit(unit)) { - logger.error(buildRuleMessage(`unit: ${unit} failed isValidUnit check`)); - return { - maxCatchup: null, - ratio: null, - gapDiffInUnits: null, - }; - } - /* - we need the total duration from now until the last time the rule ran. - the next few lines can be summed up as calculating - "how many second | minutes | hours have passed since the last time this ran?" - */ - const nowToGapDiff = moment.duration(moment().diff(previousStartedAt)); - // rule ran early, no gap - if (shorthandMap[unit].asFn(nowToGapDiff) < 0) { - // rule ran early, no gap - return { - maxCatchup: null, - ratio: null, - gapDiffInUnits: null, - }; - } - const calculatedFrom = `now-${ - parseInt(shorthandMap[unit].asFn(nowToGapDiff).toString(), 10) + unit - }`; - logger.debug(buildRuleMessage(`calculatedFrom: ${calculatedFrom}`)); - - const intervalMoment = moment.duration(parseInt(interval, 10), unit); - logger.debug(buildRuleMessage(`intervalMoment: ${shorthandMap[unit].asFn(intervalMoment)}`)); - const calculatedFromAsMoment = dateMath.parse(calculatedFrom); - const dateMathRuleParamsFrom = dateMath.parse(ruleParamsFrom); - if (dateMathRuleParamsFrom != null && intervalMoment != null) { - const momentUnit = shorthandMap[unit].momentString as moment.DurationInputArg2; - const gapDiffInUnits = dateMathRuleParamsFrom.diff(calculatedFromAsMoment, momentUnit); - - const ratio = gapDiffInUnits / shorthandMap[unit].asFn(intervalMoment); - - // maxCatchup is to ensure we are not trying to catch up too far back. - // This allows for a maximum of 4 consecutive rule execution misses - // to be included in the number of signals generated. - const maxCatchup = ratio < MAX_RULE_GAP_RATIO ? ratio : MAX_RULE_GAP_RATIO; - return { maxCatchup, ratio, gapDiffInUnits }; + gap: moment.Duration; + intervalDuration: moment.Duration; +}): number => { + if (gap.asMilliseconds() <= 0 || intervalDuration.asMilliseconds() <= 0) { + return 0; } - logger.error(buildRuleMessage('failed to parse calculatedFrom and intervalMoment')); - return { - maxCatchup: null, - ratio: null, - gapDiffInUnits: null, - }; + const ratio = Math.ceil(gap.asMilliseconds() / intervalDuration.asMilliseconds()); + // maxCatchup is to ensure we are not trying to catch up too far back. + // This allows for a maximum of 4 consecutive rule execution misses + // to be included in the number of signals generated. + return ratio < MAX_RULE_GAP_RATIO ? ratio : MAX_RULE_GAP_RATIO; }; export const getListsClient = ({ @@ -396,50 +335,40 @@ export const parseInterval = (intervalString: string): moment.Duration | null => export const getDriftTolerance = ({ from, to, - interval, + intervalDuration, now = moment(), }: { from: string; to: string; - interval: moment.Duration; + intervalDuration: moment.Duration; now?: moment.Moment; -}): moment.Duration | null => { +}): moment.Duration => { const toDate = parseScheduleDates(to) ?? now; const fromDate = parseScheduleDates(from) ?? dateMath.parse('now-6m'); const timeSegment = toDate.diff(fromDate); const duration = moment.duration(timeSegment); - if (duration !== null) { - return duration.subtract(interval); - } else { - return null; - } + return duration.subtract(intervalDuration); }; export const getGapBetweenRuns = ({ previousStartedAt, - interval, + intervalDuration, from, to, now = moment(), }: { previousStartedAt: Date | undefined | null; - interval: string; + intervalDuration: moment.Duration; from: string; to: string; now?: moment.Moment; -}): moment.Duration | null => { +}): moment.Duration => { if (previousStartedAt == null) { - return null; - } - const intervalDuration = parseInterval(interval); - if (intervalDuration == null) { - return null; - } - const driftTolerance = getDriftTolerance({ from, to, interval: intervalDuration }); - if (driftTolerance == null) { - return null; + return moment.duration(0); } + const driftTolerance = getDriftTolerance({ from, to, intervalDuration }); + const diff = moment.duration(now.diff(previousStartedAt)); const drift = diff.subtract(intervalDuration); return drift.subtract(driftTolerance); @@ -489,135 +418,103 @@ export const errorAggregator = ( }, Object.create(null)); }; -/** - * Determines the number of time intervals to search if gap is present - * along with new maxSignals per time interval. - * @param logger Logger - * @param ruleParamsFrom string representing the rules 'from' property - * @param ruleParamsTo string representing the rules 'to' property - * @param ruleParamsMaxSignals int representing the maxSignals property on the rule (usually unmodified at 100) - * @param gap moment.Duration representing a gap in since the last time the rule ran - * @param previousStartedAt Date at which the rule last ran - * @param interval string the interval which the rule runs - * @param buildRuleMessage function provides meta information for logged event - */ -export const getSignalTimeTuples = ({ +export const getRuleRangeTuples = ({ logger, - ruleParamsFrom, - ruleParamsTo, - ruleParamsMaxSignals, - gap, previousStartedAt, + from, + to, interval, + maxSignals, buildRuleMessage, }: { logger: Logger; - ruleParamsFrom: string; - ruleParamsTo: string; - ruleParamsMaxSignals: number; - gap: moment.Duration | null; previousStartedAt: Date | null | undefined; + from: string; + to: string; interval: string; - buildRuleMessage: BuildRuleMessage; -}): Array<{ - to: moment.Moment | undefined; - from: moment.Moment | undefined; maxSignals: number; -}> => { - let totalToFromTuples: Array<{ - to: moment.Moment | undefined; - from: moment.Moment | undefined; - maxSignals: number; - }> = []; - if (gap != null && gap.valueOf() > 0 && previousStartedAt != null) { - const fromUnit = ruleParamsFrom[ruleParamsFrom.length - 1]; - if (isValidUnit(fromUnit)) { - const unit = fromUnit; // only seconds (s), minutes (m) or hours (h) - - /* - we need the total duration from now until the last time the rule ran. - the next few lines can be summed up as calculating - "how many second | minutes | hours have passed since the last time this ran?" - */ - const nowToGapDiff = moment.duration(moment().diff(previousStartedAt)); - const calculatedFrom = `now-${ - parseInt(shorthandMap[unit].asFn(nowToGapDiff).toString(), 10) + unit - }`; - logger.debug(buildRuleMessage(`calculatedFrom: ${calculatedFrom}`)); - - const intervalMoment = moment.duration(parseInt(interval, 10), unit); - logger.debug(buildRuleMessage(`intervalMoment: ${shorthandMap[unit].asFn(intervalMoment)}`)); - const momentUnit = shorthandMap[unit].momentString as moment.DurationInputArg2; - // maxCatchup is to ensure we are not trying to catch up too far back. - // This allows for a maximum of 4 consecutive rule execution misses - // to be included in the number of signals generated. - const { maxCatchup, ratio, gapDiffInUnits } = getGapMaxCatchupRatio({ - logger, - buildRuleMessage, - previousStartedAt, - unit, - ruleParamsFrom, - interval, - }); - logger.debug(buildRuleMessage(`maxCatchup: ${maxCatchup}, ratio: ${ratio}`)); - if (maxCatchup == null || ratio == null || gapDiffInUnits == null) { - throw new Error( - buildRuleMessage('failed to calculate maxCatchup, ratio, or gapDiffInUnits') - ); - } - let tempTo = dateMath.parse(ruleParamsFrom); - if (tempTo == null) { - // return an error - throw new Error(buildRuleMessage('dateMath parse failed')); - } - - let beforeMutatedFrom: moment.Moment | undefined; - while (totalToFromTuples.length < maxCatchup) { - // if maxCatchup is less than 1, we calculate the 'from' differently - // and maxSignals becomes some less amount of maxSignals - // in order to maintain maxSignals per full rule interval. - if (maxCatchup > 0 && maxCatchup < 1) { - totalToFromTuples.push({ - to: tempTo.clone(), - from: tempTo.clone().subtract(gapDiffInUnits, momentUnit), - maxSignals: ruleParamsMaxSignals * maxCatchup, - }); - break; - } - const beforeMutatedTo = tempTo.clone(); - - // moment.subtract mutates the moment so we need to clone again.. - beforeMutatedFrom = tempTo.clone().subtract(intervalMoment, momentUnit); - const tuple = { - to: beforeMutatedTo, - from: beforeMutatedFrom, - maxSignals: ruleParamsMaxSignals, - }; - totalToFromTuples = [...totalToFromTuples, tuple]; - tempTo = beforeMutatedFrom; - } - totalToFromTuples = [ - { - to: dateMath.parse(ruleParamsTo), - from: dateMath.parse(ruleParamsFrom), - maxSignals: ruleParamsMaxSignals, - }, - ...totalToFromTuples, - ]; - } - } else { - totalToFromTuples = [ - { - to: dateMath.parse(ruleParamsTo), - from: dateMath.parse(ruleParamsFrom), - maxSignals: ruleParamsMaxSignals, - }, - ]; + buildRuleMessage: BuildRuleMessage; +}) => { + const originalTo = dateMath.parse(to); + const originalFrom = dateMath.parse(from); + if (originalTo == null || originalFrom == null) { + throw new Error(buildRuleMessage('dateMath parse failed')); + } + const tuples = [ + { + to: originalTo, + from: originalFrom, + maxSignals, + }, + ]; + const intervalDuration = parseInterval(interval); + if (intervalDuration == null) { + logger.error(`Failed to compute gap between rule runs: could not parse rule interval`); + return { tuples, remainingGap: moment.duration(0) }; } - logger.debug( - buildRuleMessage(`totalToFromTuples: ${JSON.stringify(totalToFromTuples, null, 4)}`) + const gap = getGapBetweenRuns({ previousStartedAt, intervalDuration, from, to }); + const catchup = getNumCatchupIntervals({ + gap, + intervalDuration, + }); + const catchupTuples = getCatchupTuples({ + to: originalTo, + from: originalFrom, + ruleParamsMaxSignals: maxSignals, + catchup, + intervalDuration, + }); + tuples.push(...catchupTuples); + // Each extra tuple adds one extra intervalDuration to the time range this rule will cover. + const remainingGapMilliseconds = Math.max( + gap.asMilliseconds() - catchup * intervalDuration.asMilliseconds(), + 0 ); - return totalToFromTuples; + return { tuples, remainingGap: moment.duration(remainingGapMilliseconds) }; +}; + +/** + * Creates rule range tuples needed to cover gaps since the last rule run. + * @param to moment.Moment representing the rules 'to' property + * @param from moment.Moment representing the rules 'from' property + * @param ruleParamsMaxSignals int representing the maxSignals property on the rule (usually unmodified at 100) + * @param catchup number the number of additional rule run intervals to add + * @param intervalDuration moment.Duration the interval which the rule runs + */ +export const getCatchupTuples = ({ + to, + from, + ruleParamsMaxSignals, + catchup, + intervalDuration, +}: { + to: moment.Moment; + from: moment.Moment; + ruleParamsMaxSignals: number; + catchup: number; + intervalDuration: moment.Duration; +}): RuleRangeTuple[] => { + const catchupTuples: RuleRangeTuple[] = []; + const intervalInMilliseconds = intervalDuration.asMilliseconds(); + let currentTo = to; + let currentFrom = from; + // This loop will create tuples with overlapping time ranges, the same way rule runs have overlapping time + // ranges due to the additional lookback. We could choose to create tuples that don't overlap here by using the + // "from" value from one tuple as "to" in the next one, however, the overlap matters for rule types like EQL and + // threshold rules that look for sets of documents within the query. Thus we keep the overlap so that these + // extra tuples behave as similarly to the regular rule runs as possible. + while (catchupTuples.length < catchup) { + const nextTo = currentTo.clone().subtract(intervalInMilliseconds); + const nextFrom = currentFrom.clone().subtract(intervalInMilliseconds); + catchupTuples.push({ + to: nextTo, + from: nextFrom, + maxSignals: ruleParamsMaxSignals, + }); + currentTo = nextTo; + currentFrom = nextFrom; + } + return catchupTuples; }; /**