Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 12 commits into from
Sep 28, 2020
Original file line number Diff line number Diff line change
Expand Up @@ -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()', () => {
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
}
`);
});
});
61 changes: 36 additions & 25 deletions x-pack/plugins/actions/server/builtin_action_types/pagerduty.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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);
Expand All @@ -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
Expand Down Expand Up @@ -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)) {
Expand All @@ -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;

Comment on lines +278 to -269
Copy link
Contributor Author

@gmmorris gmmorris Sep 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made this change in the interest of making it easier to maintain longer term.
I spent a good few minutes tying to figure out why we were comparing to null a value that can only be String or undefined... I was convinced this was a bug... it took me embarassingly long to notice the != rather than !==.

Damn you Brendan Eich! 😩

To prevent others from going down the same rabbit hole, I've replaced this with an explicit check for undefined. :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will change the payload so that previously params that were null were not included in the payload, but now they will, with a value of null. If that's ok with the PagerDuty API, then it's fine with me :-)

return data;
}
114 changes: 114 additions & 0 deletions x-pack/plugins/alerts/server/saved_objects/migrations.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
Loading