[8.x] [Response Ops][Alerting] Use ES client to update rule SO at end…

… of rule run instead of SO client. (#193341) (#194444) # Backport This will backport the following commits from `main` to `8.x`: - [[Response Ops][Alerting] Use ES client to update rule SO at end of rule run instead of SO client. (#193341)](#193341)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Ying Mao <[email protected]>
elastic · Sep 30, 2024 · ce42c68 · ce42c68
1 parent 464430d
commit ce42c68
Show file tree

Hide file tree

Showing 9 changed files with 340 additions and 141 deletions.
diff --git a/x-pack/plugins/alerting/server/saved_objects/index.ts b/x-pack/plugins/alerting/server/saved_objects/index.ts
@@ -23,7 +23,7 @@ import { RawRule } from '../types';
 import { getImportWarnings } from './get_import_warnings';
 import { isRuleExportable } from './is_rule_exportable';
 import { RuleTypeRegistry } from '../rule_type_registry';
-export { partiallyUpdateRule } from './partially_update_rule';
+export { partiallyUpdateRule, partiallyUpdateRuleWithEs } from './partially_update_rule';
 import {
   RULES_SETTINGS_SAVED_OBJECT_TYPE,
   MAINTENANCE_WINDOW_SAVED_OBJECT_TYPE,

diff --git a/x-pack/plugins/alerting/server/saved_objects/partially_update_rule.test.ts b/x-pack/plugins/alerting/server/saved_objects/partially_update_rule.test.ts
@@ -10,16 +10,23 @@ import {
   ISavedObjectsRepository,
   SavedObjectsErrorHelpers,
 } from '@kbn/core/server';
-
-import { PartiallyUpdateableRuleAttributes, partiallyUpdateRule } from './partially_update_rule';
-import { savedObjectsClientMock } from '@kbn/core/server/mocks';
+import {
+  PartiallyUpdateableRuleAttributes,
+  partiallyUpdateRule,
+  partiallyUpdateRuleWithEs,
+} from './partially_update_rule';
+import { elasticsearchServiceMock, savedObjectsClientMock } from '@kbn/core/server/mocks';
 import { RULE_SAVED_OBJECT_TYPE } from '.';
+import { ALERTING_CASES_SAVED_OBJECT_INDEX } from '@kbn/core-saved-objects-server';
+import { estypes } from '@elastic/elasticsearch';
+import { RuleExecutionStatuses } from '@kbn/alerting-types';
 
 const MockSavedObjectsClientContract = savedObjectsClientMock.create();
 const MockISavedObjectsRepository =
   MockSavedObjectsClientContract as unknown as jest.Mocked<ISavedObjectsRepository>;
+const esClient = elasticsearchServiceMock.createClusterClient().asInternalUser;
 
-describe('partially_update_rule', () => {
+describe('partiallyUpdateRule', () => {
   beforeEach(() => {
     jest.resetAllMocks();
   });
@@ -104,6 +111,101 @@ describe('partially_update_rule', () => {
     });
 });
 
+describe('partiallyUpdateRuleWithEs', () => {
+  beforeEach(() => {
+    jest.resetAllMocks();
+    jest.clearAllMocks();
+  });
+
+  test('should work with no options', async () => {
+    esClient.update.mockResolvedValueOnce(MockEsUpdateResponse(MockRuleId));
+
+    await partiallyUpdateRuleWithEs(esClient, MockRuleId, DefaultAttributesForEsUpdate);
+    expect(esClient.update).toHaveBeenCalledTimes(1);
+    expect(esClient.update).toHaveBeenCalledWith({
+      id: `alert:${MockRuleId}`,
+      index: ALERTING_CASES_SAVED_OBJECT_INDEX,
+      doc: {
+        alert: DefaultAttributesForEsUpdate,
+      },
+    });
+  });
+
+  test('should strip unallowed attributes ', async () => {
+    const attributes =
+      AttributesForEsUpdateWithUnallowedFields as unknown as PartiallyUpdateableRuleAttributes;
+    esClient.update.mockResolvedValueOnce(MockEsUpdateResponse(MockRuleId));
+
+    await partiallyUpdateRuleWithEs(esClient, MockRuleId, attributes);
+    expect(esClient.update).toHaveBeenCalledWith({
+      id: `alert:${MockRuleId}`,
+      index: ALERTING_CASES_SAVED_OBJECT_INDEX,
+      doc: {
+        alert: DefaultAttributesForEsUpdate,
+      },
+    });
+  });
+
+  test('should handle ES errors', async () => {
+    esClient.update.mockRejectedValueOnce(new Error('wops'));
+
+    await expect(
+      partiallyUpdateRuleWithEs(esClient, MockRuleId, DefaultAttributes)
+    ).rejects.toThrowError('wops');
+  });
+
+  test('should handle the version option', async () => {
+    esClient.update.mockResolvedValueOnce(MockEsUpdateResponse(MockRuleId));
+
+    await partiallyUpdateRuleWithEs(esClient, MockRuleId, DefaultAttributesForEsUpdate, {
+      version: 'WzQsMV0=',
+    });
+    expect(esClient.update).toHaveBeenCalledWith({
+      id: `alert:${MockRuleId}`,
+      index: ALERTING_CASES_SAVED_OBJECT_INDEX,
+      if_primary_term: 1,
+      if_seq_no: 4,
+      doc: {
+        alert: DefaultAttributesForEsUpdate,
+      },
+    });
+  });
+
+  test('should handle the ignore404 option', async () => {
+    esClient.update.mockResolvedValueOnce(MockEsUpdateResponse(MockRuleId));
+
+    await partiallyUpdateRuleWithEs(esClient, MockRuleId, DefaultAttributesForEsUpdate, {
+      ignore404: true,
+    });
+    expect(esClient.update).toHaveBeenCalledWith(
+      {
+        id: `alert:${MockRuleId}`,
+        index: ALERTING_CASES_SAVED_OBJECT_INDEX,
+        doc: {
+          alert: DefaultAttributesForEsUpdate,
+        },
+      },
+      { ignore: [404] }
+    );
+  });
+
+  test('should handle the refresh option', async () => {
+    esClient.update.mockResolvedValueOnce(MockEsUpdateResponse(MockRuleId));
+
+    await partiallyUpdateRuleWithEs(esClient, MockRuleId, DefaultAttributesForEsUpdate, {
+      refresh: 'wait_for',
+    });
+    expect(esClient.update).toHaveBeenCalledWith({
+      id: `alert:${MockRuleId}`,
+      index: ALERTING_CASES_SAVED_OBJECT_INDEX,
+      doc: {
+        alert: DefaultAttributesForEsUpdate,
+      },
+      refresh: 'wait_for',
+    });
+  });
+});
+
 function getMockSavedObjectClients(): Record<
   string,
   jest.Mocked<SavedObjectsClientContract | ISavedObjectsRepository>
@@ -126,6 +228,50 @@ const DefaultAttributes = {
 
 const ExtraneousAttributes = { ...DefaultAttributes, foo: 'bar' };
 
+const DefaultAttributesForEsUpdate = {
+  running: false,
+  executionStatus: {
+    status: 'active' as RuleExecutionStatuses,
+    lastExecutionDate: '2023-01-01T08:44:40.000Z',
+    lastDuration: 12,
+    error: null,
+    warning: null,
+  },
+  monitoring: {
+    run: {
+      calculated_metrics: {
+        success_ratio: 20,
+      },
+      history: [
+        {
+          success: true,
+          timestamp: 1640991880000,
+          duration: 12,
+          outcome: 'success',
+        },
+      ],
+      last_run: {
+        timestamp: '2023-01-01T08:44:40.000Z',
+        metrics: {
+          duration: 12,
+          gap_duration_s: null,
+          total_alerts_created: null,
+          total_alerts_detected: null,
+          total_indexing_duration_ms: null,
+          total_search_duration_ms: null,
+        },
+      },
+    },
+  },
+};
+
+const AttributesForEsUpdateWithUnallowedFields = {
+  ...DefaultAttributesForEsUpdate,
+  alertTypeId: 'foo',
+  consumer: 'consumer',
+  randomField: 'bar',
+};
+
 const MockRuleId = 'rule-id';
 
 const MockUpdateValue = {
@@ -137,3 +283,13 @@ const MockUpdateValue = {
   },
   references: [],
 };
+
+const MockEsUpdateResponse = (id: string) => ({
+  _index: '.kibana_alerting_cases_9.0.0_001',
+  _id: `alert:${id}`,
+  _version: 3,
+  result: 'updated' as estypes.Result,
+  _shards: { total: 1, successful: 1, failed: 0 },
+  _seq_no: 5,
+  _primary_term: 1,
+});
diff --git a/x-pack/plugins/alerting/server/saved_objects/partially_update_rule.ts b/x-pack/plugins/alerting/server/saved_objects/partially_update_rule.ts
@@ -7,10 +7,13 @@
 
 import { omit, pick } from 'lodash';
 import {
+  ElasticsearchClient,
   SavedObjectsClient,
   SavedObjectsErrorHelpers,
   SavedObjectsUpdateOptions,
 } from '@kbn/core/server';
+import { decodeRequestVersion } from '@kbn/core-saved-objects-base-server-internal';
+import { ALERTING_CASES_SAVED_OBJECT_INDEX } from '@kbn/core-saved-objects-server';
 import { RawRule } from '../types';
 
 import {
@@ -67,3 +70,50 @@ export async function partiallyUpdateRule(
     throw err;
   }
 }
+
+// Explicit list of attributes that we allow to be partially updated
+// There should be no overlap between this list and RuleAttributesIncludedInAAD or RuleAttributesToEncrypt
+const RuleAttributesAllowedForPartialUpdate = [
+  'executionStatus',
+  'lastRun',
+  'monitoring',
+  'nextRun',
+  'running',
+];
+
+// direct, partial update to a rule saved object via ElasticsearchClient
+
+// we do this direct partial update to avoid the overhead of the SavedObjectsClient for
+// only these allow-listed fields which don't impact encryption. in addition, because these
+// fields are only updated by the system user at the end of a rule run, they should not
+// need to be included in any (user-centric) audit logs.
+export async function partiallyUpdateRuleWithEs(
+  esClient: ElasticsearchClient,
+  id: string,
+  attributes: PartiallyUpdateableRuleAttributes,
+  options: PartiallyUpdateRuleSavedObjectOptions = {}
+): Promise<void> {
+  // ensure we only have the valid attributes that are not encrypted and are excluded from AAD
+  const attributeUpdates = omit(attributes, [
+    ...RuleAttributesToEncrypt,
+    ...RuleAttributesIncludedInAAD,
+  ]);
+  // ensure we only have attributes that we explicitly allow to be updated
+  const attributesAllowedForUpdate = pick(attributeUpdates, RuleAttributesAllowedForPartialUpdate);
+
+  const updateParams = {
+    id: `alert:${id}`,
+    index: ALERTING_CASES_SAVED_OBJECT_INDEX,
+    ...(options.version ? decodeRequestVersion(options.version) : {}),
+    doc: {
+      alert: attributesAllowedForUpdate,
+    },
+    ...(options.refresh ? { refresh: options.refresh } : {}),
+  };
+
+  if (options.ignore404) {
+    await esClient.update(updateParams, { ignore: [404] });
+  } else {
+    await esClient.update(updateParams);
+  }
+}
diff --git a/x-pack/plugins/alerting/server/task_runner/fixtures.ts b/x-pack/plugins/alerting/server/task_runner/fixtures.ts
@@ -7,6 +7,7 @@
 
 import { TaskStatus } from '@kbn/task-manager-plugin/server';
 import { SavedObject } from '@kbn/core/server';
+import { ALERTING_CASES_SAVED_OBJECT_INDEX } from '@kbn/core-saved-objects-server';
 import {
   Rule,
   RuleTypeParams,
@@ -64,7 +65,7 @@ const defaultHistory = [
   },
 ];
 
-export const generateSavedObjectParams = ({
+export const generateRuleUpdateParams = ({
   error = null,
   warning = null,
   status = 'ok',
@@ -83,53 +84,59 @@ export const generateSavedObjectParams = ({
   history?: RuleMonitoring['run']['history'];
   alertsCount?: Record<string, number>;
 }) => [
-  RULE_SAVED_OBJECT_TYPE,
-  '1',
   {
-    monitoring: {
-      run: {
-        calculated_metrics: {
-          success_ratio: successRatio,
+    id: `alert:1`,
+    index: ALERTING_CASES_SAVED_OBJECT_INDEX,
+    doc: {
+      alert: {
+        monitoring: {
+          run: {
+            calculated_metrics: {
+              success_ratio: successRatio,
+            },
+            history,
+            last_run: {
+              timestamp: '1970-01-01T00:00:00.000Z',
+              metrics: {
+                duration: 0,
+                gap_duration_s: null,
+                total_alerts_created: null,
+                total_alerts_detected: null,
+                total_indexing_duration_ms: null,
+                total_search_duration_ms: null,
+              },
+            },
+          },
         },
-        history,
-        last_run: {
-          timestamp: '1970-01-01T00:00:00.000Z',
-          metrics: {
-            duration: 0,
-            gap_duration_s: null,
-            total_alerts_created: null,
-            total_alerts_detected: null,
-            total_indexing_duration_ms: null,
-            total_search_duration_ms: null,
+        executionStatus: {
+          error,
+          lastDuration: 0,
+          lastExecutionDate: '1970-01-01T00:00:00.000Z',
+          status,
+          warning,
+        },
+        lastRun: {
+          outcome,
+          outcomeOrder: RuleLastRunOutcomeOrderMap[outcome],
+          outcomeMsg:
+            (error?.message && [error?.message]) ||
+            (warning?.message && [warning?.message]) ||
+            null,
+          warning: error?.reason || warning?.reason || null,
+          alertsCount: {
+            active: 0,
+            ignored: 0,
+            new: 0,
+            recovered: 0,
+            ...(alertsCount || {}),
           },
         },
+        nextRun,
+        running: false,
       },
     },
-    executionStatus: {
-      error,
-      lastDuration: 0,
-      lastExecutionDate: '1970-01-01T00:00:00.000Z',
-      status,
-      warning,
-    },
-    lastRun: {
-      outcome,
-      outcomeOrder: RuleLastRunOutcomeOrderMap[outcome],
-      outcomeMsg:
-        (error?.message && [error?.message]) || (warning?.message && [warning?.message]) || null,
-      warning: error?.reason || warning?.reason || null,
-      alertsCount: {
-        active: 0,
-        ignored: 0,
-        new: 0,
-        recovered: 0,
-        ...(alertsCount || {}),
-      },
-    },
-    nextRun,
-    running: false,
   },
-  { refresh: false, namespace: undefined },
+  { ignore: [404] },
 ];
 
 export const GENERIC_ERROR_MESSAGE = 'GENERIC ERROR MESSAGE';