[SIEM] [Detection Engine] Fixes bug when notification doesn't… (elast…

…ic#63013) 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
dhurley14 · Apr 9, 2020 · 78abbf6 · 78abbf6
1 parent 5e6c6f0
commit 78abbf6
Show file tree

Hide file tree

Showing 8 changed files with 62 additions and 4 deletions.
diff --git a/x-pack/legacy/plugins/siem/server/lib/detection_engine/signals/bulk_create_ml_signals.ts b/x-pack/legacy/plugins/siem/server/lib/detection_engine/signals/bulk_create_ml_signals.ts
@@ -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';
 
@@ -29,6 +29,7 @@ interface BulkCreateMlSignalsParams {
   updatedBy: string;
   interval: string;
   enabled: boolean;
+  refresh: RefreshTypes;
   tags: string[];
   throttle: string;
 }

diff --git a/.../legacy/plugins/siem/server/lib/detection_engine/signals/search_after_bulk_create.test.ts b/.../legacy/plugins/siem/server/lib/detection_engine/signals/search_after_bulk_create.test.ts
@@ -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',
     });
@@ -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',
     });
@@ -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',
     });
@@ -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',
     });
@@ -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',
     });
@@ -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',
     });
@@ -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',
     });
@@ -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',
     });

diff --git a/x-pack/legacy/plugins/siem/server/lib/detection_engine/signals/search_after_bulk_create.ts b/x-pack/legacy/plugins/siem/server/lib/detection_engine/signals/search_after_bulk_create.ts
@@ -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';
@@ -30,6 +30,7 @@ interface SearchAfterAndBulkCreateParams {
   enabled: boolean;
   pageSize: number;
   filter: unknown;
+  refresh: RefreshTypes;
   tags: string[];
   throttle: string;
 }
@@ -61,6 +62,7 @@ export const searchAfterAndBulkCreate = async ({
   interval,
   enabled,
   pageSize,
+  refresh,
   tags,
   throttle,
 }: SearchAfterAndBulkCreateParams): Promise<SearchAfterAndBulkCreateReturnType> => {
@@ -92,6 +94,7 @@ export const searchAfterAndBulkCreate = async ({
     updatedBy,
     interval,
     enabled,
+    refresh,
     tags,
     throttle,
   });
@@ -179,6 +182,7 @@ export const searchAfterAndBulkCreate = async ({
         updatedBy,
         interval,
         enabled,
+        refresh,
         tags,
         throttle,
       });

diff --git a/...ck/legacy/plugins/siem/server/lib/detection_engine/signals/signal_rule_alert_type.test.ts b/...ck/legacy/plugins/siem/server/lib/detection_engine/signals/signal_rule_alert_type.test.ts
@@ -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: [],
@@ -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 = [

diff --git a/x-pack/legacy/plugins/siem/server/lib/detection_engine/signals/signal_rule_alert_type.ts b/x-pack/legacy/plugins/siem/server/lib/detection_engine/signals/signal_rule_alert_type.ts
@@ -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,
@@ -181,6 +182,7 @@ export const signalRulesAlertType = ({
             updatedAt,
             interval,
             enabled,
+            refresh,
             tags,
           });
           result.success = success;
@@ -241,6 +243,7 @@ export const signalRulesAlertType = ({
             interval,
             enabled,
             pageSize: searchAfterSize,
+            refresh,
             tags,
             throttle,
           });

diff --git a/x-pack/legacy/plugins/siem/server/lib/detection_engine/signals/single_bulk_create.test.ts b/x-pack/legacy/plugins/siem/server/lib/detection_engine/signals/single_bulk_create.test.ts
@@ -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',
     });
@@ -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',
     });
@@ -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',
     });
@@ -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',
     });
@@ -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',
     });
@@ -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',
     });

diff --git a/x-pack/legacy/plugins/siem/server/lib/detection_engine/signals/single_bulk_create.ts b/x-pack/legacy/plugins/siem/server/lib/detection_engine/signals/single_bulk_create.ts
@@ -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';
@@ -31,6 +31,7 @@ interface SingleBulkCreateParams {
   enabled: boolean;
   tags: string[];
   throttle: string;
+  refresh: RefreshTypes;
 }
 
 /**
@@ -77,6 +78,7 @@ export const singleBulkCreate = async ({
   updatedBy,
   interval,
   enabled,
+  refresh,
   tags,
   throttle,
 }: SingleBulkCreateParams): Promise<SingleBulkCreateResponse> => {
@@ -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();

diff --git a/x-pack/legacy/plugins/siem/server/lib/detection_engine/types.ts b/x-pack/legacy/plugins/siem/server/lib/detection_engine/types.ts
@@ -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';