Skip to content

Commit efb9793

Browse files
[Alerting] Improve validation and errors handling in PagerDuty action (#63954) (#64060)
Improves the messaging when the Pager Duty action has trouble parsing the timestamp field and adds trimming on the timestamp's field to make us more flexible in handling the parsing and hence more likely to be forgiving of the input by the user. As the timestamp relies on context variables provided via mustcahe templates, we can't reliably validate this field at alert creation time. We address by: 1. Trimming the edges, which is required when parsing a date, should help prevent accidental spaces from breaking the parsing. 2. Checking for a mustache template on the client side and if there are none - we validate for a valid timestamp when the action is created. Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
1 parent f570c0d commit efb9793

File tree

6 files changed

+106
-12
lines changed

6 files changed

+106
-12
lines changed

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,25 @@ describe('validateParams()', () => {
142142
- [eventAction.2]: expected value to equal [acknowledge]"
143143
`);
144144
});
145+
146+
test('should validate and throw error when timestamp has spaces', () => {
147+
const randoDate = new Date('1963-09-23T01:23:45Z').toISOString();
148+
const timestamp = ` ${randoDate}`;
149+
expect(() => {
150+
validateParams(actionType, {
151+
timestamp,
152+
});
153+
}).toThrowError(`error validating action params: error parsing timestamp "${timestamp}"`);
154+
});
155+
156+
test('should validate and throw error when timestamp is invalid', () => {
157+
const timestamp = `1963-09-55 90:23:45`;
158+
expect(() => {
159+
validateParams(actionType, {
160+
timestamp,
161+
});
162+
}).toThrowError(`error validating action params: error parsing timestamp "${timestamp}"`);
163+
});
145164
});
146165

147166
describe('execute()', () => {

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

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -70,18 +70,26 @@ const ParamsSchema = schema.object(
7070

7171
function validateParams(paramsObject: any): string | void {
7272
const params: ActionParamsType = paramsObject;
73-
7473
const { timestamp } = params;
7574
if (timestamp != null) {
76-
let date;
7775
try {
78-
date = Date.parse(timestamp);
76+
const date = Date.parse(timestamp);
77+
if (isNaN(date)) {
78+
return i18n.translate('xpack.actions.builtin.pagerduty.invalidTimestampErrorMessage', {
79+
defaultMessage: `error parsing timestamp "{timestamp}"`,
80+
values: {
81+
timestamp,
82+
},
83+
});
84+
}
7985
} catch (err) {
80-
return 'error parsing timestamp: ${err.message}';
81-
}
82-
83-
if (isNaN(date)) {
84-
return 'error parsing timestamp';
86+
return i18n.translate('xpack.actions.builtin.pagerduty.timestampParsingFailedErrorMessage', {
87+
defaultMessage: `error parsing timestamp "{timestamp}": {message}`,
88+
values: {
89+
timestamp,
90+
message: err.message,
91+
},
92+
});
8593
}
8694
}
8795
}

x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/pagerduty.test.tsx

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ describe('pagerduty action params validation', () => {
9090
summary: '2323',
9191
source: 'source',
9292
severity: 'critical',
93-
timestamp: '234654564654',
93+
timestamp: new Date().toISOString(),
9494
component: 'test',
9595
group: 'group',
9696
class: 'test class',
@@ -99,6 +99,7 @@ describe('pagerduty action params validation', () => {
9999
expect(actionTypeModel.validateParams(actionParams)).toEqual({
100100
errors: {
101101
summary: [],
102+
timestamp: [],
102103
},
103104
});
104105
});
@@ -156,15 +157,15 @@ describe('PagerDutyParamsFields renders', () => {
156157
summary: '2323',
157158
source: 'source',
158159
severity: SeverityActionOptions.CRITICAL,
159-
timestamp: '234654564654',
160+
timestamp: new Date().toISOString(),
160161
component: 'test',
161162
group: 'group',
162163
class: 'test class',
163164
};
164165
const wrapper = mountWithIntl(
165166
<ParamsFields
166167
actionParams={actionParams}
167-
errors={{ summary: [] }}
168+
errors={{ summary: [], timestamp: [] }}
168169
editAction={() => {}}
169170
index={0}
170171
/>

x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/pagerduty.tsx

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
} from '@elastic/eui';
1515
import { i18n } from '@kbn/i18n';
1616
import { FormattedMessage } from '@kbn/i18n/react';
17+
import moment from 'moment';
1718
import {
1819
ActionTypeModel,
1920
ActionConnectorFieldsProps,
@@ -23,6 +24,7 @@ import {
2324
import { PagerDutyActionParams, PagerDutyActionConnector } from './types';
2425
import pagerDutySvg from './pagerduty.svg';
2526
import { AddMessageVariables } from '../add_message_variables';
27+
import { hasMustacheTokens } from '../../lib/has_mustache_tokens';
2628

2729
export function getActionType(): ActionTypeModel {
2830
return {
@@ -62,6 +64,7 @@ export function getActionType(): ActionTypeModel {
6264
const validationResult = { errors: {} };
6365
const errors = {
6466
summary: new Array<string>(),
67+
timestamp: new Array<string>(),
6568
};
6669
validationResult.errors = errors;
6770
if (!actionParams.summary?.length) {
@@ -74,6 +77,24 @@ export function getActionType(): ActionTypeModel {
7477
)
7578
);
7679
}
80+
if (actionParams.timestamp && !hasMustacheTokens(actionParams.timestamp)) {
81+
if (isNaN(Date.parse(actionParams.timestamp))) {
82+
const { nowShortFormat, nowLongFormat } = getValidTimestampExamples();
83+
errors.timestamp.push(
84+
i18n.translate(
85+
'xpack.triggersActionsUI.components.builtinActionTypes.pagerDutyAction.error.invalidTimestamp',
86+
{
87+
defaultMessage:
88+
'Timestamp must be a valid date, such as {nowShortFormat} or {nowLongFormat}.',
89+
values: {
90+
nowShortFormat,
91+
nowLongFormat,
92+
},
93+
}
94+
)
95+
);
96+
}
97+
}
7798
return validationResult;
7899
},
79100
actionConnectorFields: PagerDutyActionConnectorFields,
@@ -334,6 +355,8 @@ const PagerDutyParamsFields: React.FunctionComponent<ActionParamsProps<PagerDuty
334355
<EuiFlexItem>
335356
<EuiFormRow
336357
fullWidth
358+
error={errors.timestamp}
359+
isInvalid={errors.timestamp.length > 0 && timestamp !== undefined}
337360
label={i18n.translate(
338361
'xpack.triggersActionsUI.components.builtinActionTypes.pagerDutyAction.timestampTextFieldLabel',
339362
{
@@ -355,11 +378,14 @@ const PagerDutyParamsFields: React.FunctionComponent<ActionParamsProps<PagerDuty
355378
name="timestamp"
356379
data-test-subj="timestampInput"
357380
value={timestamp || ''}
381+
isInvalid={errors.timestamp.length > 0 && timestamp !== undefined}
358382
onChange={(e: React.ChangeEvent<HTMLInputElement>) => {
359383
editAction('timestamp', e.target.value, index);
360384
}}
361385
onBlur={() => {
362-
if (!timestamp) {
386+
if (timestamp?.trim()) {
387+
editAction('timestamp', timestamp.trim(), index);
388+
} else {
363389
editAction('timestamp', '', index);
364390
}
365391
}}
@@ -534,3 +560,11 @@ const PagerDutyParamsFields: React.FunctionComponent<ActionParamsProps<PagerDuty
534560
</Fragment>
535561
);
536562
};
563+
564+
function getValidTimestampExamples() {
565+
const now = moment();
566+
return {
567+
nowShortFormat: now.format('YYYY-MM-DD'),
568+
nowLongFormat: now.format('YYYY-MM-DD h:mm:ss'),
569+
};
570+
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
import uuid from 'uuid';
8+
9+
import { hasMustacheTokens } from './has_mustache_tokens';
10+
11+
describe('hasMustacheTokens', () => {
12+
test('returns false for empty string', () => {
13+
expect(hasMustacheTokens('')).toBe(false);
14+
});
15+
16+
test('returns false for string without tokens', () => {
17+
expect(hasMustacheTokens(`some random string ${uuid.v4()}`)).toBe(false);
18+
});
19+
20+
test('returns true when a template token is present', () => {
21+
expect(hasMustacheTokens('{{context.timestamp}}')).toBe(true);
22+
});
23+
});
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
export function hasMustacheTokens(str: string): boolean {
8+
return null !== str.match(/{{.*}}/);
9+
}

0 commit comments

Comments
 (0)