[Alerting & Actions] Overwrite SOs when updating instead of partially…

… updating (#73688) This PR changes the Alerts & Actions clients to ensure they require full updates (rather than partial) to SOs and overwrites the entire document when making the update. This is to prevent the situation where nested objects get _merged_ instead of replaced when a user makes an `update`. We also enhanced the EncryptedSavedObjectsClient to allow specified `id`s when overwriting an existing object.
elastic · Sep 18, 2020 · fd624b1 · fd624b1
1 parent c7abea0
commit fd624b1
Show file tree

Hide file tree

Showing 10 changed files with 402 additions and 117 deletions.
diff --git a/x-pack/plugins/actions/server/actions_client.test.ts b/x-pack/plugins/actions/server/actions_client.test.ts
@@ -893,7 +893,7 @@ describe('update()', () => {
         },
         references: [],
       });
-      unsecuredSavedObjectsClient.update.mockResolvedValueOnce({
+      unsecuredSavedObjectsClient.create.mockResolvedValueOnce({
         id: 'my-action',
         type: 'action',
         attributes: {
@@ -946,7 +946,7 @@ describe('update()', () => {
       },
       references: [],
     });
-    unsecuredSavedObjectsClient.update.mockResolvedValueOnce({
+    unsecuredSavedObjectsClient.create.mockResolvedValueOnce({
       id: 'my-action',
       type: 'action',
       attributes: {
@@ -972,17 +972,21 @@ describe('update()', () => {
       name: 'my name',
       config: {},
     });
-    expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledTimes(1);
-    expect(unsecuredSavedObjectsClient.update.mock.calls[0]).toMatchInlineSnapshot(`
+    expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledTimes(1);
+    expect(unsecuredSavedObjectsClient.create.mock.calls[0]).toMatchInlineSnapshot(`
       Array [
         "action",
-        "my-action",
         Object {
           "actionTypeId": "my-action-type",
           "config": Object {},
           "name": "my name",
           "secrets": Object {},
         },
+        Object {
+          "id": "my-action",
+          "overwrite": true,
+          "references": Array [],
+        },
       ]
     `);
     expect(unsecuredSavedObjectsClient.get).toHaveBeenCalledTimes(1);
@@ -1043,7 +1047,7 @@ describe('update()', () => {
       },
       references: [],
     });
-    unsecuredSavedObjectsClient.update.mockResolvedValueOnce({
+    unsecuredSavedObjectsClient.create.mockResolvedValueOnce({
       id: 'my-action',
       type: 'action',
       attributes: {
@@ -1081,11 +1085,10 @@ describe('update()', () => {
         c: true,
       },
     });
-    expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledTimes(1);
-    expect(unsecuredSavedObjectsClient.update.mock.calls[0]).toMatchInlineSnapshot(`
+    expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledTimes(1);
+    expect(unsecuredSavedObjectsClient.create.mock.calls[0]).toMatchInlineSnapshot(`
       Array [
         "action",
-        "my-action",
         Object {
           "actionTypeId": "my-action-type",
           "config": Object {
@@ -1096,6 +1099,11 @@ describe('update()', () => {
           "name": "my name",
           "secrets": Object {},
         },
+        Object {
+          "id": "my-action",
+          "overwrite": true,
+          "references": Array [],
+        },
       ]
     `);
   });
@@ -1118,7 +1126,7 @@ describe('update()', () => {
       },
       references: [],
     });
-    unsecuredSavedObjectsClient.update.mockResolvedValueOnce({
+    unsecuredSavedObjectsClient.create.mockResolvedValueOnce({
       id: 'my-action',
       type: 'action',
       attributes: {

diff --git a/x-pack/plugins/actions/server/actions_client.ts b/x-pack/plugins/actions/server/actions_client.ts
@@ -13,6 +13,7 @@ import {
 } from 'src/core/server';
 
 import { i18n } from '@kbn/i18n';
+import { omitBy, isUndefined } from 'lodash';
 import { ActionTypeRegistry } from './action_type_registry';
 import { validateConfig, validateSecrets, ActionExecutorContract } from './lib';
 import {
@@ -151,21 +152,36 @@ export class ActionsClient {
         'update'
       );
     }
-    const existingObject = await this.unsecuredSavedObjectsClient.get<RawAction>('action', id);
-    const { actionTypeId } = existingObject.attributes;
+    const { attributes, references, version } = await this.unsecuredSavedObjectsClient.get<
+      RawAction
+    >('action', id);
+    const { actionTypeId } = attributes;
     const { name, config, secrets } = action;
     const actionType = this.actionTypeRegistry.get(actionTypeId);
     const validatedActionTypeConfig = validateConfig(actionType, config);
     const validatedActionTypeSecrets = validateSecrets(actionType, secrets);
 
     this.actionTypeRegistry.ensureActionTypeEnabled(actionTypeId);
 
-    const result = await this.unsecuredSavedObjectsClient.update<RawAction>('action', id, {
-      actionTypeId,
-      name,
-      config: validatedActionTypeConfig as SavedObjectAttributes,
-      secrets: validatedActionTypeSecrets as SavedObjectAttributes,
-    });
+    const result = await this.unsecuredSavedObjectsClient.create<RawAction>(
+      'action',
+      {
+        ...attributes,
+        actionTypeId,
+        name,
+        config: validatedActionTypeConfig as SavedObjectAttributes,
+        secrets: validatedActionTypeSecrets as SavedObjectAttributes,
+      },
+      omitBy(
+        {
+          id,
+          overwrite: true,
+          references,
+          version,
+        },
+        isUndefined
+      )
+    );
 
     return {
       id,