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

[Alerts] Jira: Disallow labels with spaces #90548

Merged
merged 5 commits into from
Feb 8, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion x-pack/plugins/actions/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -657,7 +657,7 @@ The following table describes the properties of the `incident` object.
| externalId | The id of the issue in Jira. If presented the incident will be update. Otherwise a new incident will be created. | string _(optional)_ |
| issueType | The id of the issue type in Jira. | string _(optional)_ |
| priority | The name of the priority in Jira. Example: `Medium`. | string _(optional)_ |
| labels | An array of labels. | string[] _(optional)_ |
| labels | An array of labels. Labels cannot contain spaces. | string[] _(optional)_ |
| parent | The parent issue id or key. Only for `Sub-task` issue types. | string _(optional)_ |

#### `subActionParams (getIncident)`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,14 @@ export const ExecutorSubActionPushParamsSchema = schema.object({
externalId: schema.nullable(schema.string()),
issueType: schema.nullable(schema.string()),
priority: schema.nullable(schema.string()),
labels: schema.nullable(schema.arrayOf(schema.string())),
labels: schema.nullable(
schema.arrayOf(
schema.string({
validate: (label) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you try to use RegExp here for the validation?

Copy link
Member Author

Choose a reason for hiding this comment

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

No 😄 . Do you want me to?

Copy link
Contributor

Choose a reason for hiding this comment

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

It is not a blocker, but could be more extensible.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done!

label.includes(' ') ? `The label ${label} cannot contain spaces` : undefined,
})
)
),
parent: schema.nullable(schema.string()),
}),
comments: schema.nullable(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ describe('jira action params validation', () => {
};

expect(actionTypeModel.validateParams(actionParams)).toEqual({
errors: { 'subActionParams.incident.summary': [] },
errors: { 'subActionParams.incident.summary': [], 'subActionParams.incident.labels': [] },
});
});

Expand All @@ -108,6 +108,23 @@ describe('jira action params validation', () => {
expect(actionTypeModel.validateParams(actionParams)).toEqual({
errors: {
'subActionParams.incident.summary': ['Summary is required.'],
'subActionParams.incident.labels': [],
},
});
});

test('params validation fails when labels contain spaces', () => {
const actionParams = {
subActionParams: {
incident: { summary: 'some title', labels: ['label with spaces'] },
comments: [],
},
};

expect(actionTypeModel.validateParams(actionParams)).toEqual({
errors: {
'subActionParams.incident.summary': [],
'subActionParams.incident.labels': ['Labels cannot contain spaces.'],
},
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ export function getActionType(): ActionTypeModel<JiraConfig, JiraSecrets, JiraAc
validateParams: (actionParams: JiraActionParams): GenericValidationResult<unknown> => {
const errors = {
'subActionParams.incident.summary': new Array<string>(),
'subActionParams.incident.labels': new Array<string>(),
};
const validationResult = {
errors,
Expand All @@ -83,6 +84,12 @@ export function getActionType(): ActionTypeModel<JiraConfig, JiraSecrets, JiraAc
) {
errors['subActionParams.incident.summary'].push(i18n.SUMMARY_REQUIRED);
}

if (actionParams.subActionParams?.incident?.labels?.length) {
// Jira do not allows empty spaces on labels. If the label includes a whitespace show an error.
if (actionParams.subActionParams.incident.labels.some((label) => label.includes(' ')))
errors['subActionParams.incident.labels'].push(i18n.LABELS_WHITE_SPACES);
}
return validationResult;
},
actionParamsFields: lazy(() => import('./jira_params')),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,11 @@ const JiraParamsFields: React.FunctionComponent<ActionParamsProps<JiraActionPara
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [actionParams]);

const areLabelsInvalid =
errors['subActionParams.incident.labels'] != null &&
errors['subActionParams.incident.labels'].length > 0 &&
incident.labels !== undefined;

return (
<Fragment>
<>
Expand Down Expand Up @@ -304,6 +309,8 @@ const JiraParamsFields: React.FunctionComponent<ActionParamsProps<JiraActionPara
defaultMessage: 'Labels',
}
)}
error={errors['subActionParams.incident.labels'] as string[]}
isInvalid={areLabelsInvalid}
>
<EuiComboBox
noSuggestions
Expand Down Expand Up @@ -331,6 +338,7 @@ const JiraParamsFields: React.FunctionComponent<ActionParamsProps<JiraActionPara
}}
isClearable={true}
data-test-subj="labelsComboBox"
isInvalid={areLabelsInvalid}
/>
</EuiFormRow>
</EuiFlexItem>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,3 +199,10 @@ export const SEARCH_ISSUES_LOADING = i18n.translate(
defaultMessage: 'Loading...',
}
);

export const LABELS_WHITE_SPACES = i18n.translate(
'xpack.triggersActionsUI.components.builtinActionTypes.jira.labelsSpacesErrorMessage',
{
defaultMessage: 'Labels cannot contain spaces.',
}
);
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,34 @@ export default function jiraTest({ getService }: FtrProviderContext) {
});
});
});

it('should handle failing with a simulated success when labels containing a space', async () => {
await supertest
.post(`/api/actions/action/${simulatedActionId}/_execute`)
.set('kbn-xsrf', 'foo')
.send({
params: {
...mockJira.params,
subActionParams: {
incident: {
...mockJira.params.subActionParams.incident,
issueType: '10006',
labels: ['label with spaces'],
},
comments: [],
},
},
})
.then((resp: any) => {
expect(resp.body).to.eql({
actionId: simulatedActionId,
status: 'error',
retry: false,
message:
'error validating action params: types that failed validation:\n- [0.subAction]: expected value to equal [getFields]\n- [1.subAction]: expected value to equal [getIncident]\n- [2.subAction]: expected value to equal [handshake]\n- [3.subActionParams.incident.labels]: types that failed validation:\n - [subActionParams.incident.labels.0.0]: The label label with spaces cannot contain spaces\n - [subActionParams.incident.labels.1]: expected value to equal [null]\n- [4.subAction]: expected value to equal [issueTypes]\n- [5.subAction]: expected value to equal [fieldsByIssueType]\n- [6.subAction]: expected value to equal [issues]\n- [7.subAction]: expected value to equal [issue]',
});
});
});
});

describe('Execution', () => {
Expand Down