[Actions] avoids setting a default dedupKey on PagerDuty (elastic#77773)

The PagerDuty Action currently defaults to a dedupKey that's shared between all action executions of the same connector. To ensure we don't group unrelated executions together this PR avoids setting a default, which means each execution will result in its own incident in PD. As part of this change we've also made the `dedupKey` a required field whenever a `resolve` or `acknowledge` event_action is chosen. This ensure we don't try to resolve without a dedupKey, which would result in an error in PD. A migration has been introduced to migrate existing alerts which might not have a `dedupKey` configured.
gmmorris · Sep 28, 2020 · 1705370 · 1705370
1 parent c39243c
commit 1705370
Show file tree

Hide file tree

Showing 12 changed files with 470 additions and 66 deletions.
diff --git a/x-pack/plugins/actions/server/builtin_action_types/pagerduty.test.ts b/x-pack/plugins/actions/server/builtin_action_types/pagerduty.test.ts
@@ -169,6 +169,16 @@ describe('validateParams()', () => {
       });
     }).toThrowError(`error validating action params: error parsing timestamp "${timestamp}"`);
   });
+
+  test('should validate and throw error when dedupKey is missing on resolve', () => {
+    expect(() => {
+      validateParams(actionType, {
+        eventAction: 'resolve',
+      });
+    }).toThrowError(
+      `error validating action params: DedupKey is required when eventAction is "resolve"`
+    );
+  });
 });
 
 describe('execute()', () => {
@@ -199,7 +209,6 @@ describe('execute()', () => {
       Object {
         "apiUrl": "https://events.pagerduty.com/v2/enqueue",
         "data": Object {
-          "dedup_key": "action:some-action-id",
           "event_action": "trigger",
           "payload": Object {
             "severity": "info",
@@ -509,4 +518,61 @@ describe('execute()', () => {
       }
     `);
   });
+
+  test('should not set a default dedupkey to ensure each execution is a unique PagerDuty incident', async () => {
+    const randoDate = new Date('1963-09-23T01:23:45Z').toISOString();
+    const secrets = {
+      routingKey: 'super-secret',
+    };
+    const config = {
+      apiUrl: 'the-api-url',
+    };
+    const params: ActionParamsType = {
+      eventAction: 'trigger',
+      summary: 'the summary',
+      source: 'the-source',
+      severity: 'critical',
+      timestamp: randoDate,
+    };
+
+    postPagerdutyMock.mockImplementation(() => {
+      return { status: 202, data: 'data-here' };
+    });
+
+    const actionId = 'some-action-id';
+    const executorOptions: PagerDutyActionTypeExecutorOptions = {
+      actionId,
+      config,
+      params,
+      secrets,
+      services,
+    };
+    const actionResponse = await actionType.executor(executorOptions);
+    const { apiUrl, data, headers } = postPagerdutyMock.mock.calls[0][0];
+    expect({ apiUrl, data, headers }).toMatchInlineSnapshot(`
+      Object {
+        "apiUrl": "the-api-url",
+        "data": Object {
+          "event_action": "trigger",
+          "payload": Object {
+            "severity": "critical",
+            "source": "the-source",
+            "summary": "the summary",
+            "timestamp": "1963-09-23T01:23:45.000Z",
+          },
+        },
+        "headers": Object {
+          "Content-Type": "application/json",
+          "X-Routing-Key": "super-secret",
+        },
+      }
+    `);
+    expect(actionResponse).toMatchInlineSnapshot(`
+      Object {
+        "actionId": "some-action-id",
+        "data": "data-here",
+        "status": "ok",
+      }
+    `);
+  });
 });
diff --git a/x-pack/plugins/actions/server/builtin_action_types/pagerduty.ts b/x-pack/plugins/actions/server/builtin_action_types/pagerduty.ts
@@ -4,7 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import { curry } from 'lodash';
+import { curry, isUndefined, pick, omitBy } from 'lodash';
 import { i18n } from '@kbn/i18n';
 import { schema, TypeOf } from '@kbn/config-schema';
 import { postPagerduty } from './lib/post_pagerduty';
@@ -51,6 +51,10 @@ export type ActionParamsType = TypeOf<typeof ParamsSchema>;
 const EVENT_ACTION_TRIGGER = 'trigger';
 const EVENT_ACTION_RESOLVE = 'resolve';
 const EVENT_ACTION_ACKNOWLEDGE = 'acknowledge';
+const EVENT_ACTIONS_WITH_REQUIRED_DEDUPKEY = new Set([
+  EVENT_ACTION_RESOLVE,
+  EVENT_ACTION_ACKNOWLEDGE,
+]);
 
 const EventActionSchema = schema.oneOf([
   schema.literal(EVENT_ACTION_TRIGGER),
@@ -81,7 +85,7 @@ const ParamsSchema = schema.object(
 );
 
 function validateParams(paramsObject: unknown): string | void {
-  const { timestamp } = paramsObject as ActionParamsType;
+  const { timestamp, eventAction, dedupKey } = paramsObject as ActionParamsType;
   if (timestamp != null) {
     try {
       const date = Date.parse(timestamp);
@@ -103,6 +107,14 @@ function validateParams(paramsObject: unknown): string | void {
       });
     }
   }
+  if (eventAction && EVENT_ACTIONS_WITH_REQUIRED_DEDUPKEY.has(eventAction) && !dedupKey) {
+    return i18n.translate('xpack.actions.builtin.pagerduty.missingDedupkeyErrorMessage', {
+      defaultMessage: `DedupKey is required when eventAction is "{eventAction}"`,
+      values: {
+        eventAction,
+      },
+    });
+  }
 }
 
 // action type definition
@@ -230,26 +242,29 @@ async function executor(
 
 const AcknowledgeOrResolve = new Set([EVENT_ACTION_ACKNOWLEDGE, EVENT_ACTION_RESOLVE]);
 
-function getBodyForEventAction(actionId: string, params: ActionParamsType): unknown {
-  const eventAction = params.eventAction || EVENT_ACTION_TRIGGER;
-  const dedupKey = params.dedupKey || `action:${actionId}`;
-
-  const data: {
-    event_action: ActionParamsType['eventAction'];
-    dedup_key: string;
-    payload?: {
-      summary: string;
-      source: string;
-      severity: string;
-      timestamp?: string;
-      component?: string;
-      group?: string;
-      class?: string;
-    };
-  } = {
+interface PagerDutyPayload {
+  event_action: ActionParamsType['eventAction'];
+  dedup_key?: string;
+  payload?: {
+    summary: string;
+    source: string;
+    severity: string;
+    timestamp?: string;
+    component?: string;
+    group?: string;
+    class?: string;
+  };
+}
+
+function getBodyForEventAction(actionId: string, params: ActionParamsType): PagerDutyPayload {
+  const eventAction = params.eventAction ?? EVENT_ACTION_TRIGGER;
+
+  const data: PagerDutyPayload = {
     event_action: eventAction,
-    dedup_key: dedupKey,
   };
+  if (params.dedupKey) {
+    data.dedup_key = params.dedupKey;
+  }
 
   // for acknowledge / resolve, just send the dedup key
   if (AcknowledgeOrResolve.has(eventAction)) {
@@ -260,12 +275,8 @@ function getBodyForEventAction(actionId: string, params: ActionParamsType): unkn
     summary: params.summary || 'No summary provided.',
     source: params.source || `Kibana Action ${actionId}`,
     severity: params.severity || 'info',
+    ...omitBy(pick(params, ['timestamp', 'component', 'group', 'class']), isUndefined),
   };
 
-  if (params.timestamp != null) data.payload.timestamp = params.timestamp;
-  if (params.component != null) data.payload.component = params.component;
-  if (params.group != null) data.payload.group = params.group;
-  if (params.class != null) data.payload.class = params.class;
-
   return data;
 }
diff --git a/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts b/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts
@@ -85,6 +85,120 @@ describe('7.10.0', () => {
       },
     });
   });
+
+  test('migrates PagerDuty actions to set a default dedupkey of the AlertId', () => {
+    const migration710 = getMigrations(encryptedSavedObjectsSetup)['7.10.0'];
+    const alert = getMockData({
+      actions: [
+        {
+          actionTypeId: '.pagerduty',
+          group: 'default',
+          params: {
+            summary: 'fired {{alertInstanceId}}',
+            eventAction: 'resolve',
+            component: '',
+          },
+          id: 'b62ea790-5366-4abc-a7df-33db1db78410',
+        },
+      ],
+    });
+    expect(migration710(alert, { log })).toMatchObject({
+      ...alert,
+      attributes: {
+        ...alert.attributes,
+        actions: [
+          {
+            actionTypeId: '.pagerduty',
+            group: 'default',
+            params: {
+              summary: 'fired {{alertInstanceId}}',
+              eventAction: 'resolve',
+              dedupKey: '{{alertId}}',
+              component: '',
+            },
+            id: 'b62ea790-5366-4abc-a7df-33db1db78410',
+          },
+        ],
+      },
+    });
+  });
+
+  test('skips PagerDuty actions with a specified dedupkey', () => {
+    const migration710 = getMigrations(encryptedSavedObjectsSetup)['7.10.0'];
+    const alert = getMockData({
+      actions: [
+        {
+          actionTypeId: '.pagerduty',
+          group: 'default',
+          params: {
+            summary: 'fired {{alertInstanceId}}',
+            eventAction: 'trigger',
+            dedupKey: '{{alertInstanceId}}',
+            component: '',
+          },
+          id: 'b62ea790-5366-4abc-a7df-33db1db78410',
+        },
+      ],
+    });
+    expect(migration710(alert, { log })).toMatchObject({
+      ...alert,
+      attributes: {
+        ...alert.attributes,
+        actions: [
+          {
+            actionTypeId: '.pagerduty',
+            group: 'default',
+            params: {
+              summary: 'fired {{alertInstanceId}}',
+              eventAction: 'trigger',
+              dedupKey: '{{alertInstanceId}}',
+              component: '',
+            },
+            id: 'b62ea790-5366-4abc-a7df-33db1db78410',
+          },
+        ],
+      },
+    });
+  });
+
+  test('skips PagerDuty actions with an eventAction of "trigger"', () => {
+    const migration710 = getMigrations(encryptedSavedObjectsSetup)['7.10.0'];
+    const alert = getMockData({
+      actions: [
+        {
+          actionTypeId: '.pagerduty',
+          group: 'default',
+          params: {
+            summary: 'fired {{alertInstanceId}}',
+            eventAction: 'trigger',
+            component: '',
+          },
+          id: 'b62ea790-5366-4abc-a7df-33db1db78410',
+        },
+      ],
+    });
+    expect(migration710(alert, { log })).toEqual({
+      ...alert,
+      attributes: {
+        ...alert.attributes,
+        meta: {
+          versionApiKeyLastmodified: 'pre-7.10.0',
+        },
+        actions: [
+          {
+            actionTypeId: '.pagerduty',
+            group: 'default',
+            params: {
+              summary: 'fired {{alertInstanceId}}',
+              eventAction: 'trigger',
+              component: '',
+            },
+            id: 'b62ea790-5366-4abc-a7df-33db1db78410',
+          },
+        ],
+      },
+    });
+  });
 });
 
 describe('7.10.0 migrates with failure', () => {