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 @@ -199,7 +199,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 +508,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",
}
`);
});
});
47 changes: 23 additions & 24 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 @@ -230,26 +230,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 +263,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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export function initPlugin(router: IRouter, path: string) {
validate: {
body: schema.object(
{
dedup_key: schema.string(),
dedup_key: schema.maybe(schema.string()),
payload: schema.object(
{
summary: schema.string(),
Expand All @@ -48,12 +48,7 @@ export function initPlugin(router: IRouter, path: string) {
res: KibanaResponseFactory
): Promise<IKibanaResponse<any>> {
const { body } = req;
let dedupKey = body && body.dedup_key;
const summary = body && body.payload && body.payload.summary;

if (dedupKey == null) {
dedupKey = `kibana-ft-simulator-dedup-key-${new Date().toISOString()}`;
}
const summary = body?.payload?.summary;

switch (summary) {
case 'respond-with-429':
Expand All @@ -67,7 +62,7 @@ export function initPlugin(router: IRouter, path: string) {
return jsonResponse(res, 202, {
status: 'success',
message: 'Event processed',
dedup_key: dedupKey,
...(body?.dedup_key ? { dedup_key: body?.dedup_key } : {}),
});
}
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,6 @@ export default function pagerdutyTest({ getService }: FtrProviderContext) {
status: 'ok',
actionId: simulatedActionId,
data: {
dedup_key: `action:${simulatedActionId}`,
message: 'Event processed',
status: 'success',
},
Expand Down