[Alerting] Improve validation and errors handling in PagerDuty action (#63954)

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.
This commit is contained in:
Gidi Meir Morris 2020-04-21 12:32:14 +01:00 committed by GitHub
parent 1deb5d63eb
commit 02d55db6cd
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 106 additions and 12 deletions

View file

@ -142,6 +142,25 @@ describe('validateParams()', () => {
- [eventAction.2]: expected value to equal [acknowledge]"
`);
});
test('should validate and throw error when timestamp has spaces', () => {
const randoDate = new Date('1963-09-23T01:23:45Z').toISOString();
const timestamp = ` ${randoDate}`;
expect(() => {
validateParams(actionType, {
timestamp,
});
}).toThrowError(`error validating action params: error parsing timestamp "${timestamp}"`);
});
test('should validate and throw error when timestamp is invalid', () => {
const timestamp = `1963-09-55 90:23:45`;
expect(() => {
validateParams(actionType, {
timestamp,
});
}).toThrowError(`error validating action params: error parsing timestamp "${timestamp}"`);
});
});
describe('execute()', () => {

View file

@ -70,18 +70,26 @@ const ParamsSchema = schema.object(
function validateParams(paramsObject: any): string | void {
const params: ActionParamsType = paramsObject;
const { timestamp } = params;
if (timestamp != null) {
let date;
try {
date = Date.parse(timestamp);
const date = Date.parse(timestamp);
if (isNaN(date)) {
return i18n.translate('xpack.actions.builtin.pagerduty.invalidTimestampErrorMessage', {
defaultMessage: `error parsing timestamp "{timestamp}"`,
values: {
timestamp,
},
});
}
} catch (err) {
return 'error parsing timestamp: ${err.message}';
}
if (isNaN(date)) {
return 'error parsing timestamp';
return i18n.translate('xpack.actions.builtin.pagerduty.timestampParsingFailedErrorMessage', {
defaultMessage: `error parsing timestamp "{timestamp}": {message}`,
values: {
timestamp,
message: err.message,
},
});
}
}
}

View file

@ -90,7 +90,7 @@ describe('pagerduty action params validation', () => {
summary: '2323',
source: 'source',
severity: 'critical',
timestamp: '234654564654',
timestamp: new Date().toISOString(),
component: 'test',
group: 'group',
class: 'test class',
@ -99,6 +99,7 @@ describe('pagerduty action params validation', () => {
expect(actionTypeModel.validateParams(actionParams)).toEqual({
errors: {
summary: [],
timestamp: [],
},
});
});
@ -156,7 +157,7 @@ describe('PagerDutyParamsFields renders', () => {
summary: '2323',
source: 'source',
severity: SeverityActionOptions.CRITICAL,
timestamp: '234654564654',
timestamp: new Date().toISOString(),
component: 'test',
group: 'group',
class: 'test class',
@ -164,7 +165,7 @@ describe('PagerDutyParamsFields renders', () => {
const wrapper = mountWithIntl(
<ParamsFields
actionParams={actionParams}
errors={{ summary: [] }}
errors={{ summary: [], timestamp: [] }}
editAction={() => {}}
index={0}
/>

View file

@ -14,6 +14,7 @@ import {
} from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n/react';
import moment from 'moment';
import {
ActionTypeModel,
ActionConnectorFieldsProps,
@ -23,6 +24,7 @@ import {
import { PagerDutyActionParams, PagerDutyActionConnector } from './types';
import pagerDutySvg from './pagerduty.svg';
import { AddMessageVariables } from '../add_message_variables';
import { hasMustacheTokens } from '../../lib/has_mustache_tokens';
export function getActionType(): ActionTypeModel {
return {
@ -62,6 +64,7 @@ export function getActionType(): ActionTypeModel {
const validationResult = { errors: {} };
const errors = {
summary: new Array<string>(),
timestamp: new Array<string>(),
};
validationResult.errors = errors;
if (!actionParams.summary?.length) {
@ -74,6 +77,24 @@ export function getActionType(): ActionTypeModel {
)
);
}
if (actionParams.timestamp && !hasMustacheTokens(actionParams.timestamp)) {
if (isNaN(Date.parse(actionParams.timestamp))) {
const { nowShortFormat, nowLongFormat } = getValidTimestampExamples();
errors.timestamp.push(
i18n.translate(
'xpack.triggersActionsUI.components.builtinActionTypes.pagerDutyAction.error.invalidTimestamp',
{
defaultMessage:
'Timestamp must be a valid date, such as {nowShortFormat} or {nowLongFormat}.',
values: {
nowShortFormat,
nowLongFormat,
},
}
)
);
}
}
return validationResult;
},
actionConnectorFields: PagerDutyActionConnectorFields,
@ -334,6 +355,8 @@ const PagerDutyParamsFields: React.FunctionComponent<ActionParamsProps<PagerDuty
<EuiFlexItem>
<EuiFormRow
fullWidth
error={errors.timestamp}
isInvalid={errors.timestamp.length > 0 && timestamp !== undefined}
label={i18n.translate(
'xpack.triggersActionsUI.components.builtinActionTypes.pagerDutyAction.timestampTextFieldLabel',
{
@ -355,11 +378,14 @@ const PagerDutyParamsFields: React.FunctionComponent<ActionParamsProps<PagerDuty
name="timestamp"
data-test-subj="timestampInput"
value={timestamp || ''}
isInvalid={errors.timestamp.length > 0 && timestamp !== undefined}
onChange={(e: React.ChangeEvent<HTMLInputElement>) => {
editAction('timestamp', e.target.value, index);
}}
onBlur={() => {
if (!timestamp) {
if (timestamp?.trim()) {
editAction('timestamp', timestamp.trim(), index);
} else {
editAction('timestamp', '', index);
}
}}
@ -534,3 +560,11 @@ const PagerDutyParamsFields: React.FunctionComponent<ActionParamsProps<PagerDuty
</Fragment>
);
};
function getValidTimestampExamples() {
const now = moment();
return {
nowShortFormat: now.format('YYYY-MM-DD'),
nowLongFormat: now.format('YYYY-MM-DD h:mm:ss'),
};
}

View file

@ -0,0 +1,23 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import uuid from 'uuid';
import { hasMustacheTokens } from './has_mustache_tokens';
describe('hasMustacheTokens', () => {
test('returns false for empty string', () => {
expect(hasMustacheTokens('')).toBe(false);
});
test('returns false for string without tokens', () => {
expect(hasMustacheTokens(`some random string ${uuid.v4()}`)).toBe(false);
});
test('returns true when a template token is present', () => {
expect(hasMustacheTokens('{{context.timestamp}}')).toBe(true);
});
});

View file

@ -0,0 +1,9 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
export function hasMustacheTokens(str: string): boolean {
return null !== str.match(/{{.*}}/);
}