Skip to content

Commit 8547b32

Browse files
authored
[Actions] avoids setting a default dedupKey on PagerDuty (#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.
1 parent 8841757 commit 8547b32

File tree

12 files changed

+470
-66
lines changed

12 files changed

+470
-66
lines changed

x-pack/plugins/actions/server/builtin_action_types/pagerduty.test.ts

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,16 @@ describe('validateParams()', () => {
169169
});
170170
}).toThrowError(`error validating action params: error parsing timestamp "${timestamp}"`);
171171
});
172+
173+
test('should validate and throw error when dedupKey is missing on resolve', () => {
174+
expect(() => {
175+
validateParams(actionType, {
176+
eventAction: 'resolve',
177+
});
178+
}).toThrowError(
179+
`error validating action params: DedupKey is required when eventAction is "resolve"`
180+
);
181+
});
172182
});
173183

174184
describe('execute()', () => {
@@ -199,7 +209,6 @@ describe('execute()', () => {
199209
Object {
200210
"apiUrl": "https://events.pagerduty.com/v2/enqueue",
201211
"data": Object {
202-
"dedup_key": "action:some-action-id",
203212
"event_action": "trigger",
204213
"payload": Object {
205214
"severity": "info",
@@ -509,4 +518,61 @@ describe('execute()', () => {
509518
}
510519
`);
511520
});
521+
522+
test('should not set a default dedupkey to ensure each execution is a unique PagerDuty incident', async () => {
523+
const randoDate = new Date('1963-09-23T01:23:45Z').toISOString();
524+
const secrets = {
525+
routingKey: 'super-secret',
526+
};
527+
const config = {
528+
apiUrl: 'the-api-url',
529+
};
530+
const params: ActionParamsType = {
531+
eventAction: 'trigger',
532+
summary: 'the summary',
533+
source: 'the-source',
534+
severity: 'critical',
535+
timestamp: randoDate,
536+
};
537+
538+
postPagerdutyMock.mockImplementation(() => {
539+
return { status: 202, data: 'data-here' };
540+
});
541+
542+
const actionId = 'some-action-id';
543+
const executorOptions: PagerDutyActionTypeExecutorOptions = {
544+
actionId,
545+
config,
546+
params,
547+
secrets,
548+
services,
549+
};
550+
const actionResponse = await actionType.executor(executorOptions);
551+
const { apiUrl, data, headers } = postPagerdutyMock.mock.calls[0][0];
552+
expect({ apiUrl, data, headers }).toMatchInlineSnapshot(`
553+
Object {
554+
"apiUrl": "the-api-url",
555+
"data": Object {
556+
"event_action": "trigger",
557+
"payload": Object {
558+
"severity": "critical",
559+
"source": "the-source",
560+
"summary": "the summary",
561+
"timestamp": "1963-09-23T01:23:45.000Z",
562+
},
563+
},
564+
"headers": Object {
565+
"Content-Type": "application/json",
566+
"X-Routing-Key": "super-secret",
567+
},
568+
}
569+
`);
570+
expect(actionResponse).toMatchInlineSnapshot(`
571+
Object {
572+
"actionId": "some-action-id",
573+
"data": "data-here",
574+
"status": "ok",
575+
}
576+
`);
577+
});
512578
});

x-pack/plugins/actions/server/builtin_action_types/pagerduty.ts

Lines changed: 36 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* you may not use this file except in compliance with the Elastic License.
55
*/
66

7-
import { curry } from 'lodash';
7+
import { curry, isUndefined, pick, omitBy } from 'lodash';
88
import { i18n } from '@kbn/i18n';
99
import { schema, TypeOf } from '@kbn/config-schema';
1010
import { postPagerduty } from './lib/post_pagerduty';
@@ -51,6 +51,10 @@ export type ActionParamsType = TypeOf<typeof ParamsSchema>;
5151
const EVENT_ACTION_TRIGGER = 'trigger';
5252
const EVENT_ACTION_RESOLVE = 'resolve';
5353
const EVENT_ACTION_ACKNOWLEDGE = 'acknowledge';
54+
const EVENT_ACTIONS_WITH_REQUIRED_DEDUPKEY = new Set([
55+
EVENT_ACTION_RESOLVE,
56+
EVENT_ACTION_ACKNOWLEDGE,
57+
]);
5458

5559
const EventActionSchema = schema.oneOf([
5660
schema.literal(EVENT_ACTION_TRIGGER),
@@ -81,7 +85,7 @@ const ParamsSchema = schema.object(
8185
);
8286

8387
function validateParams(paramsObject: unknown): string | void {
84-
const { timestamp } = paramsObject as ActionParamsType;
88+
const { timestamp, eventAction, dedupKey } = paramsObject as ActionParamsType;
8589
if (timestamp != null) {
8690
try {
8791
const date = Date.parse(timestamp);
@@ -103,6 +107,14 @@ function validateParams(paramsObject: unknown): string | void {
103107
});
104108
}
105109
}
110+
if (eventAction && EVENT_ACTIONS_WITH_REQUIRED_DEDUPKEY.has(eventAction) && !dedupKey) {
111+
return i18n.translate('xpack.actions.builtin.pagerduty.missingDedupkeyErrorMessage', {
112+
defaultMessage: `DedupKey is required when eventAction is "{eventAction}"`,
113+
values: {
114+
eventAction,
115+
},
116+
});
117+
}
106118
}
107119

108120
// action type definition
@@ -230,26 +242,29 @@ async function executor(
230242

231243
const AcknowledgeOrResolve = new Set([EVENT_ACTION_ACKNOWLEDGE, EVENT_ACTION_RESOLVE]);
232244

233-
function getBodyForEventAction(actionId: string, params: ActionParamsType): unknown {
234-
const eventAction = params.eventAction || EVENT_ACTION_TRIGGER;
235-
const dedupKey = params.dedupKey || `action:${actionId}`;
236-
237-
const data: {
238-
event_action: ActionParamsType['eventAction'];
239-
dedup_key: string;
240-
payload?: {
241-
summary: string;
242-
source: string;
243-
severity: string;
244-
timestamp?: string;
245-
component?: string;
246-
group?: string;
247-
class?: string;
248-
};
249-
} = {
245+
interface PagerDutyPayload {
246+
event_action: ActionParamsType['eventAction'];
247+
dedup_key?: string;
248+
payload?: {
249+
summary: string;
250+
source: string;
251+
severity: string;
252+
timestamp?: string;
253+
component?: string;
254+
group?: string;
255+
class?: string;
256+
};
257+
}
258+
259+
function getBodyForEventAction(actionId: string, params: ActionParamsType): PagerDutyPayload {
260+
const eventAction = params.eventAction ?? EVENT_ACTION_TRIGGER;
261+
262+
const data: PagerDutyPayload = {
250263
event_action: eventAction,
251-
dedup_key: dedupKey,
252264
};
265+
if (params.dedupKey) {
266+
data.dedup_key = params.dedupKey;
267+
}
253268

254269
// for acknowledge / resolve, just send the dedup key
255270
if (AcknowledgeOrResolve.has(eventAction)) {
@@ -260,12 +275,8 @@ function getBodyForEventAction(actionId: string, params: ActionParamsType): unkn
260275
summary: params.summary || 'No summary provided.',
261276
source: params.source || `Kibana Action ${actionId}`,
262277
severity: params.severity || 'info',
278+
...omitBy(pick(params, ['timestamp', 'component', 'group', 'class']), isUndefined),
263279
};
264280

265-
if (params.timestamp != null) data.payload.timestamp = params.timestamp;
266-
if (params.component != null) data.payload.component = params.component;
267-
if (params.group != null) data.payload.group = params.group;
268-
if (params.class != null) data.payload.class = params.class;
269-
270281
return data;
271282
}

x-pack/plugins/alerts/server/saved_objects/migrations.test.ts

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,120 @@ describe('7.10.0', () => {
8585
},
8686
});
8787
});
88+
89+
test('migrates PagerDuty actions to set a default dedupkey of the AlertId', () => {
90+
const migration710 = getMigrations(encryptedSavedObjectsSetup)['7.10.0'];
91+
const alert = getMockData({
92+
actions: [
93+
{
94+
actionTypeId: '.pagerduty',
95+
group: 'default',
96+
params: {
97+
summary: 'fired {{alertInstanceId}}',
98+
eventAction: 'resolve',
99+
component: '',
100+
},
101+
id: 'b62ea790-5366-4abc-a7df-33db1db78410',
102+
},
103+
],
104+
});
105+
expect(migration710(alert, { log })).toMatchObject({
106+
...alert,
107+
attributes: {
108+
...alert.attributes,
109+
actions: [
110+
{
111+
actionTypeId: '.pagerduty',
112+
group: 'default',
113+
params: {
114+
summary: 'fired {{alertInstanceId}}',
115+
eventAction: 'resolve',
116+
dedupKey: '{{alertId}}',
117+
component: '',
118+
},
119+
id: 'b62ea790-5366-4abc-a7df-33db1db78410',
120+
},
121+
],
122+
},
123+
});
124+
});
125+
126+
test('skips PagerDuty actions with a specified dedupkey', () => {
127+
const migration710 = getMigrations(encryptedSavedObjectsSetup)['7.10.0'];
128+
const alert = getMockData({
129+
actions: [
130+
{
131+
actionTypeId: '.pagerduty',
132+
group: 'default',
133+
params: {
134+
summary: 'fired {{alertInstanceId}}',
135+
eventAction: 'trigger',
136+
dedupKey: '{{alertInstanceId}}',
137+
component: '',
138+
},
139+
id: 'b62ea790-5366-4abc-a7df-33db1db78410',
140+
},
141+
],
142+
});
143+
expect(migration710(alert, { log })).toMatchObject({
144+
...alert,
145+
attributes: {
146+
...alert.attributes,
147+
actions: [
148+
{
149+
actionTypeId: '.pagerduty',
150+
group: 'default',
151+
params: {
152+
summary: 'fired {{alertInstanceId}}',
153+
eventAction: 'trigger',
154+
dedupKey: '{{alertInstanceId}}',
155+
component: '',
156+
},
157+
id: 'b62ea790-5366-4abc-a7df-33db1db78410',
158+
},
159+
],
160+
},
161+
});
162+
});
163+
164+
test('skips PagerDuty actions with an eventAction of "trigger"', () => {
165+
const migration710 = getMigrations(encryptedSavedObjectsSetup)['7.10.0'];
166+
const alert = getMockData({
167+
actions: [
168+
{
169+
actionTypeId: '.pagerduty',
170+
group: 'default',
171+
params: {
172+
summary: 'fired {{alertInstanceId}}',
173+
eventAction: 'trigger',
174+
component: '',
175+
},
176+
id: 'b62ea790-5366-4abc-a7df-33db1db78410',
177+
},
178+
],
179+
});
180+
expect(migration710(alert, { log })).toEqual({
181+
...alert,
182+
attributes: {
183+
...alert.attributes,
184+
meta: {
185+
versionApiKeyLastmodified: 'pre-7.10.0',
186+
},
187+
actions: [
188+
{
189+
actionTypeId: '.pagerduty',
190+
group: 'default',
191+
params: {
192+
summary: 'fired {{alertInstanceId}}',
193+
eventAction: 'trigger',
194+
component: '',
195+
},
196+
id: 'b62ea790-5366-4abc-a7df-33db1db78410',
197+
},
198+
],
199+
},
200+
});
201+
});
88202
});
89203

90204
describe('7.10.0 migrates with failure', () => {

0 commit comments

Comments
 (0)