[SIEM] Check ML Job status on ML Rule execution (elastic#61715)

* Move isMlRule helper to a more general location

And use it during rule execution as well.

* Add error message back to rule error status

This was unintentionally removed in a previous merge commit.

* Expose mlClient as part of ML's Setup contract

This allows dependent plugins to leverage the exposed services without
having to define their own ml paths, e.g. "ml.jobs"

* Move ML Job predicates to common folder

These are pure functions and used on both the client and server.

* WIP: Check ML Job status on ML Rule execution

This works, but unfortunately it pushes this executor function to a
complexity of 25. We're gonna refactor this next.

* Move isMlRule and RuleType to common

These are used on both the frontend and the backend, and can be shared.

* Refactor Signal Rule executor to use RuleStatusService

RuleStatusService holds the logic for updating the current status as
well as adding an error status. It leverages a simple
RuleStatusSavedObjectClient to handle the communication with
SavedObjects.

This removes the need for our specialized 'writeError', 'writeGap', and
'writeSuccess' functions, which duplicated much of the rule status
logic and code. It also fixes a bug with gap failures, with should have
been treated the same as other failures.

NB that an error does not necessarily prevent the rule from running, as
in the case of a gap or an ML Job not running.

This also adds a buildRuleMessage helper to reduce the noise of
generating logs/messages, and to make them more consistent.

* Remove unneeded 'async' keywords

We're not awaiting here, so we can just return the promise.

* Make buildRuleStatusAttributes synchronous

We weren't doing anything async here, and in fact the returning of a
promise was causing a bug when we tried to spread it into our attributes
object.

* Fix incorrectly-named RuleStatus attributes

This mapping could be done within the ruleStatusService, but it
lives outside it for now.

Also renames the object holding these values to the more general
'result,' as creationSuccess implies it always succeeds.

* Move our rule message helpers to a separate file

Adds some tests, as well.

* Refactor how rule status objects interact

Only ruleStatusSavedObjectsClient receives a savedObjectsClient, the
other functions receive the ruleStatusSavedObjectsClient

* pluralizes savedObjects in ruleStatusSavedObjectsClient
* Backfills tests

* Handle adding multiple errors during a single rule execution

We were storing state in our RuleStatusClient, and consequently could
get into a situation where that state did not reflect reality, and we
would incorrectly try to delete a SavedObject that had already been
deleted.

Rather than try to store the _correct_ state in the service, we remove
state entirely and just fetch our statuses on each action.

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

x-pack/legacy/plugins/siem/public/components/ml/helpers/index.test.ts → x-pack/legacy/plugins/siem/common/detection_engine/ml_helpers.test.ts

@@ -4,7 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import { isJobStarted, isJobLoading, isJobFailed } from './';
+import { isJobStarted, isJobLoading, isJobFailed } from './ml_helpers';
 
 describe('isJobStarted', () => {
   test('returns false if only jobState is enabled', () => {

x-pack/legacy/plugins/siem/public/components/ml/helpers/index.ts → x-pack/legacy/plugins/siem/common/detection_engine/ml_helpers.ts

@@ -4,6 +4,8 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
+import { RuleType } from './types';
+
 // Based on ML Job/Datafeed States from x-pack/legacy/plugins/ml/common/constants/states.js
 const enabledStates = ['started', 'opened'];
 const loadingStates = ['starting', 'stopping', 'opening', 'closing'];
@@ -20,3 +22,5 @@ export const isJobLoading = (jobState: string, datafeedState: string): boolean =
 export const isJobFailed = (jobState: string, datafeedState: string): boolean => {
   return failureStates.includes(jobState) || failureStates.includes(datafeedState);
 };
+
+export const isMlRule = (ruleType: RuleType) => ruleType === 'machine_learning';

x-pack/legacy/plugins/siem/common/detection_engine/types.ts

@@ -3,9 +3,17 @@
  * or more contributor license agreements. Licensed under the Elastic License;
  * you may not use this file except in compliance with the Elastic License.
  */
+import * as t from 'io-ts';
 
 import { AlertAction } from '../../../../../plugins/alerting/common';
 
 export type RuleAlertAction = Omit<AlertAction, 'actionTypeId'> & {
   action_type_id: string;
 };
+
+export const RuleTypeSchema = t.keyof({
+  query: null,
+  saved_query: null,
+  machine_learning: null,
+});
+export type RuleType = t.TypeOf<typeof RuleTypeSchema>;

x-pack/legacy/plugins/siem/public/components/ml_popover/jobs_table/job_switch.tsx

@@ -8,7 +8,11 @@ import styled from 'styled-components';
 import React, { useState, useCallback } from 'react';
 import { EuiFlexGroup, EuiFlexItem, EuiLoadingSpinner, EuiSwitch } from '@elastic/eui';
 import { SiemJob } from '../types';
-import { isJobLoading, isJobStarted, isJobFailed } from '../../ml/helpers';
+import {
+  isJobLoading,
+  isJobFailed,
+  isJobStarted,
+} from '../../../../common/detection_engine/ml_helpers';
 
 const StaticSwitch = styled(EuiSwitch)`
   .euiSwitch__thumb,

x-pack/legacy/plugins/siem/public/containers/detection_engine/rules/types.ts

@@ -6,12 +6,7 @@
 
 import * as t from 'io-ts';
 
-export const RuleTypeSchema = t.keyof({
-  query: null,
-  saved_query: null,
-  machine_learning: null,
-});
-export type RuleType = t.TypeOf<typeof RuleTypeSchema>;
+import { RuleTypeSchema } from '../../../../common/detection_engine/types';
 
 /**
  * Params is an "record", since it is a type of AlertActionParams which is action templates.

x-pack/legacy/plugins/siem/public/containers/detection_engine/rules/use_rule_status.test.tsx

@@ -12,7 +12,7 @@ import {
   ReturnRulesStatuses,
 } from './use_rule_status';
 import * as api from './api';
-import { RuleType, Rule } from '../rules/types';
+import { Rule } from '../rules/types';
 
 jest.mock('./api');
 
@@ -57,7 +57,7 @@ const testRule: Rule = {
   threat: [],
   throttle: null,
   to: 'now',
-  type: 'query' as RuleType,
+  type: 'query',
   updated_at: 'mm/dd/yyyyTHH:MM:sssz',
   updated_by: 'mockUser',
 };

x-pack/legacy/plugins/siem/public/pages/detection_engine/rules/components/description_step/helpers.tsx

@@ -21,13 +21,13 @@ import styled from 'styled-components';
 
 import { esFilters } from '../../../../../../../../../../src/plugins/data/public';
 
+import { RuleType } from '../../../../../../common/detection_engine/types';
 import { tacticsOptions, techniquesOptions } from '../../../mitre/mitre_tactics_techniques';
 
 import * as i18n from './translations';
 import { BuildQueryBarDescription, BuildThreatDescription, ListItems } from './types';
 import { SeverityBadge } from '../severity_badge';
 import ListTreeIcon from './assets/list_tree_icon.svg';
-import { RuleType } from '../../../../../containers/detection_engine/rules';
 import { assertUnreachable } from '../../../../../lib/helpers';
 
 const NoteDescriptionContainer = styled(EuiFlexItem)`

x-pack/legacy/plugins/siem/public/pages/detection_engine/rules/components/description_step/index.tsx

@@ -15,7 +15,7 @@ import {
   esFilters,
   FilterManager,
 } from '../../../../../../../../../../src/plugins/data/public';
-import { RuleType } from '../../../../../containers/detection_engine/rules';
+import { RuleType } from '../../../../../../common/detection_engine/types';
 import { DEFAULT_TIMELINE_TITLE } from '../../../../../components/timeline/translations';
 import { useKibana } from '../../../../../lib/kibana';
 import { IMitreEnterpriseAttack } from '../../types';

x-pack/legacy/plugins/siem/public/pages/detection_engine/rules/components/description_step/ml_job_description.tsx

@@ -11,8 +11,8 @@ import { EuiBadge, EuiIcon, EuiLink, EuiToolTip } from '@elastic/eui';
 import { useKibana } from '../../../../../lib/kibana';
 import { SiemJob } from '../../../../../components/ml_popover/types';
 import { ListItems } from './types';
-import { isJobStarted } from '../../../../../components/ml/helpers';
 import { ML_JOB_STARTED, ML_JOB_STOPPED } from './translations';
+import { isJobStarted } from '../../../../../../common/detection_engine/ml_helpers';
 
 enum MessageLevels {
   info = 'info',

x-pack/legacy/plugins/siem/public/pages/detection_engine/rules/components/select_rule_type/index.tsx

@@ -16,10 +16,10 @@ import {
   EuiText,
 } from '@elastic/eui';
 
+import { isMlRule } from '../../../../../../common/detection_engine/ml_helpers';
+import { RuleType } from '../../../../../../common/detection_engine/types';
 import { FieldHook } from '../../../../../shared_imports';
-import { RuleType } from '../../../../../containers/detection_engine/rules/types';
 import * as i18n from './translations';
-import { isMlRule } from '../../helpers';
 
 const MlCardDescription = ({ hasValidLicense = false }: { hasValidLicense?: boolean }) => (
   <EuiText size="s">

x-pack/legacy/plugins/siem/public/pages/detection_engine/rules/components/step_define_rule/index.tsx

@@ -12,10 +12,11 @@ import deepEqual from 'fast-deep-equal';
 import { IIndexPattern } from '../../../../../../../../../../src/plugins/data/public';
 import { useFetchIndexPatterns } from '../../../../../containers/detection_engine/rules';
 import { DEFAULT_INDEX_KEY } from '../../../../../../common/constants';
+import { isMlRule } from '../../../../../../common/detection_engine/ml_helpers';
 import { DEFAULT_TIMELINE_TITLE } from '../../../../../components/timeline/translations';
 import { useMlCapabilities } from '../../../../../components/ml_popover/hooks/use_ml_capabilities';
 import { useUiSetting$ } from '../../../../../lib/kibana';
-import { setFieldValue, isMlRule } from '../../helpers';
+import { setFieldValue } from '../../helpers';
 import { DefineStepRule, RuleStep, RuleStepProps } from '../../types';
 import { StepRuleDescription } from '../description_step';
 import { QueryBarDefineRule } from '../query_bar';

x-pack/legacy/plugins/siem/public/pages/detection_engine/rules/components/step_define_rule/schema.tsx

@@ -10,6 +10,7 @@ import { isEmpty } from 'lodash/fp';
 import React from 'react';
 
 import { esKuery } from '../../../../../../../../../../src/plugins/data/public';
+import { isMlRule } from '../../../../../../common/detection_engine/ml_helpers';
 import { FieldValueQueryBar } from '../query_bar';
 import {
   ERROR_CODE,
@@ -19,7 +20,6 @@ import {
   ValidationFunc,
 } from '../../../../../shared_imports';
 import { CUSTOM_QUERY_REQUIRED, INVALID_CUSTOM_QUERY, INDEX_HELPER_TEXT } from './translations';
-import { isMlRule } from '../../helpers';
 
 export const schema: FormSchema = {
   index: {

x-pack/legacy/plugins/siem/public/pages/detection_engine/rules/create/helpers.ts

@@ -12,8 +12,10 @@ import {
   NOTIFICATION_THROTTLE_RULE,
   NOTIFICATION_THROTTLE_NO_ACTIONS,
 } from '../../../../../common/constants';
-import { NewRule, RuleType } from '../../../../containers/detection_engine/rules';
 import { transformAlertToRuleAction } from '../../../../../common/detection_engine/transform_actions';
+import { RuleType } from '../../../../../common/detection_engine/types';
+import { isMlRule } from '../../../../../common/detection_engine/ml_helpers';
+import { NewRule } from '../../../../containers/detection_engine/rules';
 
 import {
   AboutStepRule,
@@ -25,7 +27,6 @@ import {
   AboutStepRuleJson,
   ActionsStepRuleJson,
 } from '../types';
-import { isMlRule } from '../helpers';
 
 export const getTimeTypeValue = (time: string): { unit: string; value: number } => {
   const timeObj = {

x-pack/legacy/plugins/siem/public/pages/detection_engine/rules/helpers.tsx

@@ -10,10 +10,11 @@ import moment from 'moment';
 import memoizeOne from 'memoize-one';
 import { useLocation } from 'react-router-dom';
 
-import { RuleAlertAction } from '../../../../common/detection_engine/types';
+import { RuleAlertAction, RuleType } from '../../../../common/detection_engine/types';
+import { isMlRule } from '../../../../common/detection_engine/ml_helpers';
 import { transformRuleToAlertAction } from '../../../../common/detection_engine/transform_actions';
 import { Filter } from '../../../../../../../../src/plugins/data/public';
-import { Rule, RuleType } from '../../../containers/detection_engine/rules';
+import { Rule } from '../../../containers/detection_engine/rules';
 import { FormData, FormHook, FormSchema } from '../../../shared_imports';
 import {
   AboutStepRule,
@@ -214,8 +215,6 @@ export const setFieldValue = (
     }
   });
 
-export const isMlRule = (ruleType: RuleType) => ruleType === 'machine_learning';
-
 export const redirectToDetections = (
   isSignalIndexExists: boolean | null,
   isAuthenticated: boolean | null,

x-pack/legacy/plugins/siem/public/pages/detection_engine/rules/types.ts

@@ -5,9 +5,8 @@
  */
 
 import { AlertAction } from '../../../../../../../plugins/alerting/common';
-import { RuleAlertAction } from '../../../../common/detection_engine/types';
+import { RuleAlertAction, RuleType } from '../../../../common/detection_engine/types';
 import { Filter } from '../../../../../../../../src/plugins/data/common';
-import { RuleType } from '../../../containers/detection_engine/rules/types';
 import { FieldValueQueryBar } from './components/query_bar';
 import { FormData, FormHook } from '../../../shared_imports';
 import { FieldValueTimeline } from './components/pick_timeline';

x-pack/legacy/plugins/siem/server/lib/detection_engine/routes/rules/utils.ts

@@ -19,12 +19,7 @@ import {
   isRuleStatusFindTypes,
   isRuleStatusSavedObjectType,
 } from '../../rules/types';
-import {
-  OutputRuleAlertRest,
-  ImportRuleAlertRest,
-  RuleAlertParamsRest,
-  RuleType,
-} from '../../types';
+import { OutputRuleAlertRest, ImportRuleAlertRest, RuleAlertParamsRest } from '../../types';
 import {
   createBulkErrorObject,
   BulkError,
@@ -300,5 +295,3 @@ export const getTupleDuplicateErrorsAndUniqueRules = (
 
   return [Array.from(errors.values()), Array.from(rulesAcc.values())];
 };
-
-export const isMlRule = (ruleType: RuleType) => ruleType === 'machine_learning';

x-pack/legacy/plugins/siem/server/lib/detection_engine/routes/schemas/response/check_type_dependents.ts

@@ -8,6 +8,7 @@ import * as t from 'io-ts';
 import { Either, left, fold } from 'fp-ts/lib/Either';
 import { pipe } from 'fp-ts/lib/pipeable';
 
+import { isMlRule } from '../../../../../../common/detection_engine/ml_helpers';
 import {
   dependentRulesSchema,
   RequiredRulesSchema,
@@ -47,7 +48,7 @@ export const addQueryFields = (typeAndTimelineOnly: TypeAndTimelineOnly): t.Mixe
 };
 
 export const addMlFields = (typeAndTimelineOnly: TypeAndTimelineOnly): t.Mixed[] => {
-  if (typeAndTimelineOnly.type === 'machine_learning') {
+  if (isMlRule(typeAndTimelineOnly.type)) {
     return [
       t.exact(t.type({ anomaly_threshold: dependentRulesSchema.props.anomaly_threshold })),
       t.exact(

x-pack/legacy/plugins/siem/server/lib/detection_engine/routes/utils.ts

@@ -16,9 +16,9 @@ import {
 } from '../../../../../../../../src/core/server';
 import { ILicense } from '../../../../../../../plugins/licensing/server';
 import { MINIMUM_ML_LICENSE } from '../../../../common/constants';
+import { RuleType } from '../../../../common/detection_engine/types';
+import { isMlRule } from '../../../../common/detection_engine/ml_helpers';
 import { BadRequestError } from '../errors/bad_request_error';
-import { RuleType } from '../types';
-import { isMlRule } from './rules/utils';
 
 export interface OutputError {
   message: string;

x-pack/legacy/plugins/siem/server/lib/detection_engine/rules/types.ts

@@ -84,7 +84,7 @@ export interface IRuleStatusFindType {
   saved_objects: IRuleStatusSavedObject[];
 }
 
-export type RuleStatusString = 'succeeded' | 'failed' | 'going to run' | 'executing';
+export type RuleStatusString = 'succeeded' | 'failed' | 'going to run';
 
 export interface HapiReadableStream extends Readable {
   hapi: {

x-pack/legacy/plugins/siem/server/lib/detection_engine/signals/__mocks__/es_results.ts

@@ -5,9 +5,15 @@
  */
 
 import { SignalSourceHit, SignalSearchResponse } from '../types';
-import { Logger } from 'kibana/server';
+import {
+  Logger,
+  SavedObject,
+  SavedObjectsFindResponse,
+} from '../../../../../../../../../src/core/server';
 import { loggingServiceMock } from '../../../../../../../../../src/core/server/mocks';
 import { RuleTypeParams, OutputRuleAlertRest } from '../../types';
+import { IRuleStatusAttributes } from '../../rules/types';
+import { ruleStatusSavedObjectType } from '../../../../saved_objects';
 
 export const sampleRuleAlertParams = (
   maxSignals?: number | undefined,
@@ -373,4 +379,34 @@ export const sampleRule = (): Partial<OutputRuleAlertRest> => {
   };
 };
 
+export const exampleRuleStatus: () => SavedObject<IRuleStatusAttributes> = () => ({
+  type: ruleStatusSavedObjectType,
+  id: '042e6d90-7069-11ea-af8b-0f8ae4fa817e',
+  attributes: {
+    alertId: 'f4b8e31d-cf93-4bde-a265-298bde885cd7',
+    statusDate: '2020-03-27T22:55:59.517Z',
+    status: 'succeeded',
+    lastFailureAt: null,
+    lastSuccessAt: '2020-03-27T22:55:59.517Z',
+    lastFailureMessage: null,
+    lastSuccessMessage: 'succeeded',
+    gap: null,
+    bulkCreateTimeDurations: [],
+    searchAfterTimeDurations: [],
+    lastLookBackDate: null,
+  },
+  references: [],
+  updated_at: '2020-03-27T22:55:59.577Z',
+  version: 'WzgyMiwxXQ==',
+});
+
+export const exampleFindRuleStatusResponse: (
+  mockStatuses: Array<SavedObject<IRuleStatusAttributes>>
+) => SavedObjectsFindResponse<IRuleStatusAttributes> = (mockStatuses = [exampleRuleStatus()]) => ({
+  total: 1,
+  per_page: 6,
+  page: 1,
+  saved_objects: mockStatuses,
+});
+
 export const mockLogger: Logger = loggingServiceMock.createLogger();

x-pack/legacy/plugins/siem/server/lib/detection_engine/signals/__mocks__/rule_status_saved_objects_client.mock.ts

@@ -0,0 +1,18 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import { RuleStatusSavedObjectsClient } from '../rule_status_saved_objects_client';
+
+const createMockRuleStatusSavedObjectsClient = (): jest.Mocked<RuleStatusSavedObjectsClient> => ({
+  find: jest.fn(),
+  create: jest.fn(),
+  update: jest.fn(),
+  delete: jest.fn(),
+});
+
+export const ruleStatusSavedObjectsClientMock = {
+  create: createMockRuleStatusSavedObjectsClient,
+};