From b68b5788c2f96f60d4b3455898e642b1d4289acf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loix?= Date: Mon, 29 Oct 2018 16:13:16 +0100 Subject: [PATCH] fix(watch): Add WatchErrors to capture invalid watches (#23887) Instead of immediately throwing an error if a watch Action is invalid (an email for example), there is now an `option` object that we can pass to the `fromUpstreamJson()` method and receive back any error that might exist in a Watch. The Watch has a new "watchErrors" property to display configuration error in the UI. fixes #20305 fixes #20970 --- .../watcher/common/constants/action_states.js | 5 + .../watcher/common/constants/error_codes.js | 11 ++ .../plugins/watcher/common/constants/index.js | 1 + .../watcher/common/constants/watch_states.js | 4 + .../action_state_icon/action_state_icon.html | 2 +- .../errors_display_modal.html | 18 +++ .../errors_display_modal.js | 18 +++ .../components/errors_display_modal/index.js | 7 + .../watch_state_icon/watch_state_icon.html | 2 +- .../public/models/action/email_action.js | 5 +- .../public/models/action/logging_action.js | 6 +- .../public/models/action/slack_action.js | 27 +++- .../watcher/public/models/watch/base_watch.js | 44 ++++++ .../public/models/watch_errors/index.js | 7 + .../models/watch_errors/watch_errors.js | 17 +++ .../action_status_table.html | 11 ++ .../action_status_table.js | 20 ++- .../components/watch_detail/watch_detail.html | 2 + .../components/watch_detail/watch_detail.js | 59 ++++++-- .../json_watch_edit/json_watch_edit.js | 77 +++++++++- .../server/models/action/__tests__/action.js | 108 -------------- .../watcher/server/models/action/action.js | 44 +++++- .../server/models/action/action.test.js | 136 ++++++++++++++++++ .../server/models/action/base_action.js | 3 +- .../server/models/action/email_action.js | 76 ++++++---- .../server/models/action/logging_action.js | 50 ++++--- .../server/models/action/slack_action.js | 69 +++++---- .../server/models/action/unknown_action.js | 33 +++-- .../action_status/__tests__/action_status.js | 12 +- .../models/action_status/action_status.js | 5 + .../models/watch/__tests__/base_watch.js | 30 +++- .../watcher/server/models/watch/base_watch.js | 36 ++++- .../watcher/server/models/watch/json_watch.js | 4 +- .../watcher/server/models/watch/watch.js | 4 +- .../server/models/watch_errors/index.js | 7 + .../models/watch_errors/watch_errors.js | 25 ++++ .../models/watch_errors/watch_errors.test.js | 34 +++++ .../__tests__/watch_history_item.js | 3 +- .../watch_status/__tests__/watch_status.js | 11 ++ .../models/watch_status/watch_status.js | 11 +- .../routes/api/watch/register_load_route.js | 12 +- .../routes/api/watches/register_list_route.js | 17 ++- 42 files changed, 825 insertions(+), 248 deletions(-) create mode 100644 x-pack/plugins/watcher/common/constants/error_codes.js create mode 100644 x-pack/plugins/watcher/public/components/errors_display_modal/errors_display_modal.html create mode 100644 x-pack/plugins/watcher/public/components/errors_display_modal/errors_display_modal.js create mode 100644 x-pack/plugins/watcher/public/components/errors_display_modal/index.js create mode 100644 x-pack/plugins/watcher/public/models/watch_errors/index.js create mode 100644 x-pack/plugins/watcher/public/models/watch_errors/watch_errors.js delete mode 100644 x-pack/plugins/watcher/server/models/action/__tests__/action.js create mode 100644 x-pack/plugins/watcher/server/models/action/action.test.js create mode 100644 x-pack/plugins/watcher/server/models/watch_errors/index.js create mode 100644 x-pack/plugins/watcher/server/models/watch_errors/watch_errors.js create mode 100644 x-pack/plugins/watcher/server/models/watch_errors/watch_errors.test.js diff --git a/x-pack/plugins/watcher/common/constants/action_states.js b/x-pack/plugins/watcher/common/constants/action_states.js index 7b8ac59ff336e..e34a5fd37ff3a 100644 --- a/x-pack/plugins/watcher/common/constants/action_states.js +++ b/x-pack/plugins/watcher/common/constants/action_states.js @@ -33,4 +33,9 @@ export const ACTION_STATES = { defaultMessage: 'Error' }), + // Action has a configuration error + CONFIG_ERROR: i18n.translate('xpack.watcher.constants.actionStates.configErrorStateText', { + defaultMessage: 'Config error' + }), + }; diff --git a/x-pack/plugins/watcher/common/constants/error_codes.js b/x-pack/plugins/watcher/common/constants/error_codes.js new file mode 100644 index 0000000000000..2fa875549358f --- /dev/null +++ b/x-pack/plugins/watcher/common/constants/error_codes.js @@ -0,0 +1,11 @@ +/* + * 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 const ERROR_CODES = { + + // Property missing on object + ERR_PROP_MISSING: 'ERR_PROP_MISSING', +}; diff --git a/x-pack/plugins/watcher/common/constants/index.js b/x-pack/plugins/watcher/common/constants/index.js index 3324b9f3cf427..4260688c6eb45 100644 --- a/x-pack/plugins/watcher/common/constants/index.js +++ b/x-pack/plugins/watcher/common/constants/index.js @@ -22,3 +22,4 @@ export { WATCH_STATE_COMMENTS } from './watch_state_comments'; export { WATCH_HISTORY } from './watch_history'; export { WATCH_STATES } from './watch_states'; export { WATCH_TYPES } from './watch_types'; +export { ERROR_CODES } from './error_codes'; diff --git a/x-pack/plugins/watcher/common/constants/watch_states.js b/x-pack/plugins/watcher/common/constants/watch_states.js index e9916aceec1c1..1c546139b688c 100644 --- a/x-pack/plugins/watcher/common/constants/watch_states.js +++ b/x-pack/plugins/watcher/common/constants/watch_states.js @@ -24,4 +24,8 @@ export const WATCH_STATES = { defaultMessage: 'Error!' }), + CONFIG_ERROR: i18n.translate('xpack.watcher.constants.watchStates.configErrorStateText', { + defaultMessage: 'Config error' + }), + }; diff --git a/x-pack/plugins/watcher/public/components/action_state_icon/action_state_icon.html b/x-pack/plugins/watcher/public/components/action_state_icon/action_state_icon.html index 05fbcededf4a7..67c8c703aecf0 100644 --- a/x-pack/plugins/watcher/public/components/action_state_icon/action_state_icon.html +++ b/x-pack/plugins/watcher/public/components/action_state_icon/action_state_icon.html @@ -17,6 +17,6 @@ >
diff --git a/x-pack/plugins/watcher/public/components/errors_display_modal/errors_display_modal.html b/x-pack/plugins/watcher/public/components/errors_display_modal/errors_display_modal.html new file mode 100644 index 0000000000000..3d4f25e7abc7d --- /dev/null +++ b/x-pack/plugins/watcher/public/components/errors_display_modal/errors_display_modal.html @@ -0,0 +1,18 @@ +
+
+

{{ vm.title }}

+
+
+
    +
  • {{ error.message }}
    • +
+
+
+ +
+
\ No newline at end of file diff --git a/x-pack/plugins/watcher/public/components/errors_display_modal/errors_display_modal.js b/x-pack/plugins/watcher/public/components/errors_display_modal/errors_display_modal.js new file mode 100644 index 0000000000000..1e17774193ec4 --- /dev/null +++ b/x-pack/plugins/watcher/public/components/errors_display_modal/errors_display_modal.js @@ -0,0 +1,18 @@ +/* + * 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 { uiModules } from 'ui/modules'; + +const app = uiModules.get('xpack/watcher'); + +app.controller('WatcherErrorsDisplayController', function WatcherErrorsDisplayController($scope, $modalInstance, params) { + this.title = params.title; + this.errors = params.errors; + + this.close = function close() { + $modalInstance.close(); + }; +}); diff --git a/x-pack/plugins/watcher/public/components/errors_display_modal/index.js b/x-pack/plugins/watcher/public/components/errors_display_modal/index.js new file mode 100644 index 0000000000000..07daabc6bf185 --- /dev/null +++ b/x-pack/plugins/watcher/public/components/errors_display_modal/index.js @@ -0,0 +1,7 @@ +/* + * 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 './errors_display_modal'; diff --git a/x-pack/plugins/watcher/public/components/watch_state_icon/watch_state_icon.html b/x-pack/plugins/watcher/public/components/watch_state_icon/watch_state_icon.html index 5cb1b6fd492a8..0f50077157811 100644 --- a/x-pack/plugins/watcher/public/components/watch_state_icon/watch_state_icon.html +++ b/x-pack/plugins/watcher/public/components/watch_state_icon/watch_state_icon.html @@ -13,6 +13,6 @@ >
diff --git a/x-pack/plugins/watcher/public/models/action/email_action.js b/x-pack/plugins/watcher/public/models/action/email_action.js index fbcfdaa4123e8..31fe6a9405e0b 100644 --- a/x-pack/plugins/watcher/public/models/action/email_action.js +++ b/x-pack/plugins/watcher/public/models/action/email_action.js @@ -24,7 +24,10 @@ export class EmailAction extends BaseAction { Object.assign(result, { to: this.to, subject: this.subject, - body: this.body + body: this.body, + email: { + to: this.to.length ? this.to : undefined, + } }); return result; diff --git a/x-pack/plugins/watcher/public/models/action/logging_action.js b/x-pack/plugins/watcher/public/models/action/logging_action.js index 41fd86d668979..603784053e717 100644 --- a/x-pack/plugins/watcher/public/models/action/logging_action.js +++ b/x-pack/plugins/watcher/public/models/action/logging_action.js @@ -17,9 +17,13 @@ export class LoggingAction extends BaseAction { get upstreamJson() { const result = super.upstreamJson; + const text = !!this.text.trim() ? this.text : undefined; Object.assign(result, { - text: this.text + text, + logging: { + text, + } }); return result; diff --git a/x-pack/plugins/watcher/public/models/action/slack_action.js b/x-pack/plugins/watcher/public/models/action/slack_action.js index 6f8146c24d354..f94bc2f06b19d 100644 --- a/x-pack/plugins/watcher/public/models/action/slack_action.js +++ b/x-pack/plugins/watcher/public/models/action/slack_action.js @@ -17,12 +17,35 @@ export class SlackAction extends BaseAction { this.text = props.text; } + validate() { + const errors = []; + + if (!this.to.length) { + errors.push({ + message: i18n.translate('xpack.watcher.sections.watchEdit.json.warningPossibleInvalidSlackAction.description', { + // eslint-disable-next-line max-len + defaultMessage: 'This watch has a Slack action without a "to" property. This watch will only be valid if you specified the "to" property in the Slack "message_default" setting in Elasticsearch.' + }) + }); + } + + return { errors: errors.length ? errors : null }; + } + get upstreamJson() { const result = super.upstreamJson; - + const message = this.text || this.to.length + ? { + text: this.text, + to: this.to.length ? this.to : undefined + } + : undefined; Object.assign(result, { to: this.to, - text: this.text + text: this.text, + slack: { + message + } }); return result; diff --git a/x-pack/plugins/watcher/public/models/watch/base_watch.js b/x-pack/plugins/watcher/public/models/watch/base_watch.js index 27bf94d14f9c8..8363ebaf05cc3 100644 --- a/x-pack/plugins/watcher/public/models/watch/base_watch.js +++ b/x-pack/plugins/watcher/public/models/watch/base_watch.js @@ -8,6 +8,7 @@ import { getSearchValue } from 'plugins/watcher/lib/get_search_value'; import { get, isEqual, remove, map, merge } from 'lodash'; import { Action } from '../action'; import { WatchStatus } from '../watch_status'; +import { WatchErrors } from '../watch_errors'; import { createActionId } from './lib/create_action_id'; import { checkActionIdCollision } from './lib/check_action_id_collision'; import { i18n } from '@kbn/i18n'; @@ -31,6 +32,7 @@ export class BaseWatch { this.name = get(props, 'name', ''); this.isSystemWatch = Boolean(get(props, 'isSystemWatch')); this.watchStatus = WatchStatus.fromUpstreamJson(get(props, 'watchStatus')); + this.watchErrors = WatchErrors.fromUpstreamJson(get(props, 'watchErrors')); const actions = get(props, 'actions', []); this.actions = actions.map(Action.fromUpstreamJson); @@ -76,6 +78,10 @@ export class BaseWatch { remove(this.actions, action); } + resetActions = () => { + this.actions = []; + }; + get displayName() { if (this.isNew) { return i18n.translate('xpack.watcher.models.baseWatch.displayName', { @@ -130,6 +136,44 @@ export class BaseWatch { return isEqual(cleanWatch, cleanOtherWatch); } + /** + * Client validation of the Watch. + * Currently we are *only* validating the Watch "Actions" + */ + validate() { + + // Get the errors from each watch action + const actionsErrors = this.actions.reduce((actionsErrors, action) => { + if (action.validate) { + const { errors } = action.validate(); + if (!errors) { + return actionsErrors; + } + return [...actionsErrors, ...errors]; + } + return actionsErrors; + }, []); + + if (!actionsErrors.length) { + return { warning: null }; + } + + // Concatenate their message + const warningMessage = actionsErrors.reduce((message, error) => ( + !!message + ? `${message}, ${error.message}` + : error.message + ), ''); + + // We are not doing any *blocking* validation in the client, + // so we return the errors as a _warning_ + return { + warning: { + message: warningMessage, + } + }; + } + static typeName = i18n.translate('xpack.watcher.models.baseWatch.typeName', { defaultMessage: 'Watch', }); diff --git a/x-pack/plugins/watcher/public/models/watch_errors/index.js b/x-pack/plugins/watcher/public/models/watch_errors/index.js new file mode 100644 index 0000000000000..dec5e51d74e31 --- /dev/null +++ b/x-pack/plugins/watcher/public/models/watch_errors/index.js @@ -0,0 +1,7 @@ +/* + * 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 { WatchErrors } from './watch_errors'; diff --git a/x-pack/plugins/watcher/public/models/watch_errors/watch_errors.js b/x-pack/plugins/watcher/public/models/watch_errors/watch_errors.js new file mode 100644 index 0000000000000..6668481f13fdb --- /dev/null +++ b/x-pack/plugins/watcher/public/models/watch_errors/watch_errors.js @@ -0,0 +1,17 @@ +/* + * 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 { get } from 'lodash'; + +export class WatchErrors { + constructor(props = {}) { + this.actionErrors = get(props, 'actions'); + } + + static fromUpstreamJson(upstreamWatchErrors) { + return new WatchErrors(upstreamWatchErrors); + } +} diff --git a/x-pack/plugins/watcher/public/sections/watch_detail/components/action_status_table/action_status_table.html b/x-pack/plugins/watcher/public/sections/watch_detail/components/action_status_table/action_status_table.html index 1e89ebd099807..3cf5b7c5e6304 100644 --- a/x-pack/plugins/watcher/public/sections/watch_detail/components/action_status_table/action_status_table.html +++ b/x-pack/plugins/watcher/public/sections/watch_detail/components/action_status_table/action_status_table.html @@ -21,6 +21,9 @@ {{ 'xpack.watcher.sections.watchDetail.actionStatusTable.stateColumnLabel' | i18n: { defaultMessage: 'State' } }} + + Errors + @@ -39,6 +42,14 @@ {{ actionStatus.state }} + +
+ {{actionStatusTable.getLabelErrors(actionStatus.id)}} +
+
diff --git a/x-pack/plugins/watcher/public/sections/watch_detail/components/action_status_table/action_status_table.js b/x-pack/plugins/watcher/public/sections/watch_detail/components/action_status_table/action_status_table.js index 1994c88737876..db058c107d225 100644 --- a/x-pack/plugins/watcher/public/sections/watch_detail/components/action_status_table/action_status_table.js +++ b/x-pack/plugins/watcher/public/sections/watch_detail/components/action_status_table/action_status_table.js @@ -9,20 +9,36 @@ import template from './action_status_table.html'; const app = uiModules.get('xpack/watcher'); -app.directive('actionStatusTable', function () { +app.directive('actionStatusTable', function ($injector, i18n) { return { restrict: 'E', replace: true, template: template, scope: { actionStatuses: '=', + actionErrors: '=', sortField: '=', sortReverse: '=', onSortChange: '=', onActionAcknowledge: '=', + showErrors: '=' }, bindToController: true, controllerAs: 'actionStatusTable', - controller: class ActionStatusTableController {} + controller: class ActionStatusTableController { + getLabelErrors(actionId) { + const errors = this.actionErrors[actionId]; + const total = errors.length; + + const label = i18n('xpack.watcher.sections.watchDetail.actionStatusTotalErrors', { + defaultMessage: '{total, number} {total, plural, one {error} other {errors}}', + values: { + total, + } + }); + + return label; + } + } }; }); diff --git a/x-pack/plugins/watcher/public/sections/watch_detail/components/watch_detail/watch_detail.html b/x-pack/plugins/watcher/public/sections/watch_detail/components/watch_detail/watch_detail.html index 5e4397a825c63..fa46a0b12097a 100644 --- a/x-pack/plugins/watcher/public/sections/watch_detail/components/watch_detail/watch_detail.html +++ b/x-pack/plugins/watcher/public/sections/watch_detail/components/watch_detail/watch_detail.html @@ -53,10 +53,12 @@ {{ 'xpack.watcher.sections.watchDetail.noActionsFoundText' | i18n: { defaultMessage: 'No actions found.' } }} diff --git a/x-pack/plugins/watcher/public/sections/watch_detail/components/watch_detail/watch_detail.js b/x-pack/plugins/watcher/public/sections/watch_detail/components/watch_detail/watch_detail.js index 41cb264c1d916..4d3fdea3816e1 100644 --- a/x-pack/plugins/watcher/public/sections/watch_detail/components/watch_detail/watch_detail.js +++ b/x-pack/plugins/watcher/public/sections/watch_detail/components/watch_detail/watch_detail.js @@ -14,8 +14,10 @@ import 'ui/table_info'; import 'plugins/watcher/components/tool_bar_selected_count'; import 'plugins/watcher/services/watch'; import 'plugins/watcher/services/license'; +import 'plugins/watcher/components/errors_display_modal'; import template from './watch_detail.html'; +import errorsDisplayTemplate from 'plugins/watcher/components/errors_display_modal/errors_display_modal.html'; import '../watch_history'; import '../action_status_table'; import { REFRESH_INTERVALS } from 'plugins/watcher/../common/constants'; @@ -33,6 +35,7 @@ app.directive('watchDetail', function ($injector, i18n) { const $filter = $injector.get('$filter'); const orderBy = $filter('orderBy'); + const $modal = $injector.get('$modal'); moment.tz.setDefault(config.get('dateFormat:tz')); @@ -54,38 +57,39 @@ app.directive('watchDetail', function ($injector, i18n) { this.actionStatusTableSortField = 'id'; this.actionStatusTableSortReverse = false; + this.actionErrors = (this.watch.watchErrors && this.watch.watchErrors.actionErrors) || null; - this.omitBreadcrumbPages = [ - 'watch', - this.watch.id - ]; + this.omitBreadcrumbPages = ['watch', this.watch.id]; this.breadcrumb = this.watch.displayName; // Reload watch history periodically - const refreshInterval = $interval(() => this.loadWatchHistory(), REFRESH_INTERVALS.WATCH_HISTORY); + const refreshInterval = $interval( + () => this.loadWatchHistory(), + REFRESH_INTERVALS.WATCH_HISTORY + ); $scope.$on('$destroy', () => $interval.cancel(refreshInterval)); // react to data and UI changes - $scope.$watchMulti([ - 'watchDetail.actionStatusTableSortField', - 'watchDetail.actionStatusTableSortReverse', - ], this.applySortToActionStatusTable); + $scope.$watchMulti( + ['watchDetail.actionStatusTableSortField', 'watchDetail.actionStatusTableSortReverse'], + this.applySortToActionStatusTable + ); } loadWatchHistory = () => { - return watchService.loadWatchHistory(this.watch.id, this.historyRange) + return watchService + .loadWatchHistory(this.watch.id, this.historyRange) .then(watchHistoryItems => { this.isHistoryLoading = false; this.watchHistoryItems = watchHistoryItems; }) .catch(err => { - return licenseService.checkValidity() - .then(() => toastNotifications.addDanger(err)); + return licenseService.checkValidity().then(() => toastNotifications.addDanger(err)); }); - } + }; // update the watch history items when the time range changes - onHistoryRangeChange = (range) => { + onHistoryRangeChange = range => { this.historyRange = range; this.isHistoryLoading = true; return this.loadWatchHistory(); @@ -124,6 +128,33 @@ app.directive('watchDetail', function ($injector, i18n) { }); } + showErrors = (actionId, errors) => { + const errorsModal = $modal.open({ + template: errorsDisplayTemplate, + controller: 'WatcherErrorsDisplayController', + controllerAs: 'vm', + backdrop: 'static', + keyboard: true, + ariaLabelledBy: 'watcher__error-display-modal-title', + resolve: { + params: function () { + return { + title: i18n('xpack.watcher.sections.watchDetail.errorDisplayModalTitleText', { + defaultMessage: 'Errors in the "{actionId}" action', + values: { actionId } } + ), + errors, + }; + } + } + }); + + errorsModal.result.catch(() => { + // We need to add this empty Promise catch to avoid + // a console error "Possibly unhandled rejection" + }); + } + /** * Event handler methods */ diff --git a/x-pack/plugins/watcher/public/sections/watch_edit/components/json_watch_edit/json_watch_edit.js b/x-pack/plugins/watcher/public/sections/watch_edit/components/json_watch_edit/json_watch_edit.js index 9ff9382f8a999..c78b57417200c 100644 --- a/x-pack/plugins/watcher/public/sections/watch_edit/components/json_watch_edit/json_watch_edit.js +++ b/x-pack/plugins/watcher/public/sections/watch_edit/components/json_watch_edit/json_watch_edit.js @@ -18,6 +18,7 @@ import '../watch_edit_execute_detail'; import '../watch_edit_actions_execute_summary'; import '../watch_edit_watch_execute_summary'; import 'plugins/watcher/services/license'; +import { ACTION_TYPES } from '../../../../../common/constants'; const app = uiModules.get('xpack/watcher'); @@ -100,14 +101,16 @@ app.directive('jsonWatchEdit', function ($injector, i18n) { } onWatchSave = () => { + this.createActionsForWatch(this.watch); + if (!this.watch.isNew) { - return this.saveWatch(); + return this.validateAndSaveWatch(); } return this.isExistingWatch() .then(existingWatch => { if (!existingWatch) { - return this.saveWatch(); + return this.validateAndSaveWatch(); } const confirmModalOptions = { @@ -152,6 +155,23 @@ app.directive('jsonWatchEdit', function ($injector, i18n) { }); } + validateAndSaveWatch = () => { + const { warning } = this.watch.validate(); + + if (warning) { + const confirmModalOptions = { + onConfirm: this.saveWatch, + confirmButtonText: i18n('xpack.watcher.sections.watchEdit.json.watchErrorsWarning.confirmSaveWatch', { + defaultMessage: 'Save watch', + }), + }; + + return confirmModal(warning.message, confirmModalOptions); + } + + return this.saveWatch(); + } + saveWatch = () => { return watchService.saveWatch(this.watch) .then(() => { @@ -211,6 +231,59 @@ app.directive('jsonWatchEdit', function ($injector, i18n) { // dirtyPrompt.deregister(); kbnUrl.change('/management/elasticsearch/watcher/watches', {}); } + + /** + * Actions instances are not automatically added to the Watch _actions_ Array + * when we add them in the Json editor. This method takes takes care of it. + * + * @param watchModel Watch instance + * @return Watch instance + */ + createActionsForWatch(watchInstance) { + watchInstance.resetActions(); + + let action; + let type; + let actionProps; + + Object.keys(watchInstance.watch.actions).forEach((k) => { + action = watchInstance.watch.actions[k]; + type = this.getTypeFromAction(action); + actionProps = this.getPropsFromAction(type, action); + + watchInstance.createAction(type, actionProps); + }); + + return watchInstance; + } + + /** + * Get the type from an action where a key defines its type. + * eg: { email: { ... } } | { slack: { ... } } + * + * @param action A raw action object + * @return {string} The action type + */ + getTypeFromAction(action) { + const actionKeys = Object.keys(action); + let type; + + Object.keys(ACTION_TYPES).forEach((k) => { + if (actionKeys.includes(ACTION_TYPES[k])) { + type = ACTION_TYPES[k]; + } + }); + + return type ? type : ACTION_TYPES.UNKNOWN; + } + + getPropsFromAction(type, action) { + if (type === ACTION_TYPES.SLACK) { + // Slack action has its props inside the "message" object + return action[type].message; + } + return action[type]; + } } }; }); diff --git a/x-pack/plugins/watcher/server/models/action/__tests__/action.js b/x-pack/plugins/watcher/server/models/action/__tests__/action.js deleted file mode 100644 index 747f4feac7b0f..0000000000000 --- a/x-pack/plugins/watcher/server/models/action/__tests__/action.js +++ /dev/null @@ -1,108 +0,0 @@ -/* - * 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 expect from 'expect.js'; -import { Action } from '../action'; -import { ACTION_TYPES } from '../../../../common/constants'; - -describe('action', () => { - - describe('Action', () => { - - describe('fromUpstreamJson factory method', () => { - - let upstreamJson; - beforeEach(() => { - upstreamJson = { - id: 'my-action', - actionJson: { - "logging": { - "text": "foo" - } - } - }; - }); - - it(`throws an error if no 'id' property in json`, () => { - delete upstreamJson.id; - expect(Action.fromUpstreamJson).withArgs(upstreamJson) - .to.throwError(/must contain an id property/i); - }); - - it(`throws an error if no 'actionJson' property in json`, () => { - delete upstreamJson.actionJson; - expect(Action.fromUpstreamJson).withArgs(upstreamJson) - .to.throwError(/must contain an actionJson property/i); - }); - - it('returns correct Action instance', () => { - const action = Action.fromUpstreamJson(upstreamJson); - - expect(action.id).to.be(upstreamJson.id); - }); - - }); - - describe('type getter method', () => { - - it(`returns a value from ACTION_TYPES when there is a valid model class`, () => { - const upstreamJson = { - id: 'my-action', - actionJson: { - logging: { - 'text': 'foo' - } - } - }; - const action = Action.fromUpstreamJson(upstreamJson); - - expect(action.type).to.be(ACTION_TYPES.LOGGING); - }); - - it(`returns ACTION_TYPES.UNKNOWN when there is no valid model class`, () => { - const upstreamJson = { - id: 'my-action', - actionJson: { - unknown_action_type: { - 'foo': 'bar' - } - } - }; - const action = Action.fromUpstreamJson(upstreamJson); - - expect(action.type).to.be(ACTION_TYPES.UNKNOWN); - }); - - }); - - describe('downstreamJson getter method', () => { - - let upstreamJson; - beforeEach(() => { - upstreamJson = { - id: 'my-action', - actionJson: { - "logging": { - "text": "foo" - } - } - }; - }); - - it('returns correct JSON for client', () => { - const action = Action.fromUpstreamJson(upstreamJson); - - const json = action.downstreamJson; - - expect(json.id).to.be(action.id); - expect(json.type).to.be(action.type); - }); - - }); - - }); - -}); diff --git a/x-pack/plugins/watcher/server/models/action/action.js b/x-pack/plugins/watcher/server/models/action/action.js index 1d02977d0e684..a5d677518da26 100644 --- a/x-pack/plugins/watcher/server/models/action/action.js +++ b/x-pack/plugins/watcher/server/models/action/action.js @@ -26,7 +26,18 @@ export class Action { } // From Elasticsearch - static fromUpstreamJson(json) { + static fromUpstreamJson(json, options = { throwExceptions: {} }) { + if (!json.id) { + throw badRequest( + i18n.translate('xpack.watcher.models.actionStatus.absenceOfIdPropertyBadRequestMessage', { + defaultMessage: 'json argument must contain an {id} property', + values: { + id: 'id' + } + }), + ); + } + if (!json.actionJson) { throw badRequest( i18n.translate('xpack.watcher.models.action.absenceOfActionJsonPropertyBadRequestMessage', { @@ -40,13 +51,38 @@ export class Action { const type = getActionType(json.actionJson); const ActionType = ActionTypes[type] || UnknownAction; - return ActionType.fromUpstreamJson(json); + + const { action, errors } = ActionType.fromUpstreamJson(json, options); + const doThrowException = options.throwExceptions.Action !== false; + + if (errors && doThrowException) { + this.throwErrors(errors); + } + + return action; } // From Kibana - static fromDownstreamJson(json) { + static fromDownstreamJson(json, options = { throwExceptions: {} }) { const ActionType = ActionTypes[json.type] || UnknownAction; - return ActionType.fromDownstreamJson(json); + const { action, errors } = ActionType.fromDownstreamJson(json); + const doThrowException = options.throwExceptions.Action !== false; + + if (errors && doThrowException) { + this.throwErrors(errors); + } + + return action; + } + + static throwErrors(errors) { + const allMessages = errors.reduce((message, error) => { + if (message) { + return `${message}, ${error.message}`; + } + return error.message; + }, ''); + throw badRequest(allMessages); } } diff --git a/x-pack/plugins/watcher/server/models/action/action.test.js b/x-pack/plugins/watcher/server/models/action/action.test.js new file mode 100644 index 0000000000000..8771c053b574d --- /dev/null +++ b/x-pack/plugins/watcher/server/models/action/action.test.js @@ -0,0 +1,136 @@ +/* + * 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 { Action } from './action'; +import { LoggingAction } from './logging_action'; +import { ACTION_TYPES } from '../../../common/constants'; + +jest.mock('./logging_action', () => ({ + LoggingAction: { + fromUpstreamJson: jest.fn(({ id }) => ({ + errors: null, + action: { id, type: 'logging' }, + })), + } +})); + +describe('action', () => { + + describe('Action', () => { + + describe('fromUpstreamJson factory method', () => { + + let upstreamJson; + beforeEach(() => { + upstreamJson = { + id: 'my-action', + actionJson: { + "logging": { + "text": "foo" + } + } + }; + }); + + it(`throws an error if no 'id' property in json`, () => { + delete upstreamJson.id; + expect(() => { + Action.fromUpstreamJson(upstreamJson); + }).toThrowError(/must contain an id property/i); + }); + + it(`throws an error if no 'actionJson' property in json`, () => { + delete upstreamJson.actionJson; + expect(() => { + Action.fromUpstreamJson(upstreamJson); + }).toThrowError(/must contain an actionJson property/i); + }); + + it(`throws an error if an Action is invalid`, () => { + const message = 'Missing prop in Logging Action!'; + + LoggingAction.fromUpstreamJson.mockReturnValueOnce({ + errors: [{ message }], + action: {}, + }); + + expect(() => { + Action.fromUpstreamJson(upstreamJson); + }).toThrowError(message); + }); + + it('returns correct Action instance', () => { + const action = Action.fromUpstreamJson(upstreamJson); + + expect(action.id).toBe(upstreamJson.id); + }); + + }); + + describe('type getter method', () => { + + it(`returns the correct known Action type`, () => { + const options = { throwExceptions: { Action: false } }; + + const upstreamLoggingJson = { id: 'action1', actionJson: { logging: {} } }; + const loggingAction = Action.fromUpstreamJson(upstreamLoggingJson, options); + + const upstreamEmailJson = { id: 'action2', actionJson: { email: {} } }; + const emailAction = Action.fromUpstreamJson(upstreamEmailJson, options); + + const upstreamSlackJson = { id: 'action3', actionJson: { slack: {} } }; + const slackAction = Action.fromUpstreamJson(upstreamSlackJson, options); + + expect(loggingAction.type).toBe(ACTION_TYPES.LOGGING); + expect(emailAction.type).toBe(ACTION_TYPES.EMAIL); + expect(slackAction.type).toBe(ACTION_TYPES.SLACK); + }); + + it(`returns ACTION_TYPES.UNKNOWN when there is no valid model class`, () => { + const upstreamJson = { + id: 'my-action', + actionJson: { + unknown_action_type: { + 'foo': 'bar' + } + } + }; + const action = Action.fromUpstreamJson(upstreamJson); + + expect(action.type).toBe(ACTION_TYPES.UNKNOWN); + }); + + }); + + describe('downstreamJson getter method', () => { + + let upstreamJson; + beforeEach(() => { + upstreamJson = { + id: 'my-action', + actionJson: { + "email": { + "to": "elastic@elastic.co" + } + } + }; + }); + + it('returns correct JSON for client', () => { + + const action = Action.fromUpstreamJson(upstreamJson); + + const json = action.downstreamJson; + + expect(json.id).toBe(action.id); + expect(json.type).toBe(action.type); + }); + + }); + + }); + +}); diff --git a/x-pack/plugins/watcher/server/models/action/base_action.js b/x-pack/plugins/watcher/server/models/action/base_action.js index dc03a76a28515..d85cccee28508 100644 --- a/x-pack/plugins/watcher/server/models/action/base_action.js +++ b/x-pack/plugins/watcher/server/models/action/base_action.js @@ -8,9 +8,10 @@ import { badRequest } from 'boom'; import { i18n } from '@kbn/i18n'; export class BaseAction { - constructor(props) { + constructor(props, errors) { this.id = props.id; this.type = props.type; + this.errors = errors; } get downstreamJson() { diff --git a/x-pack/plugins/watcher/server/models/action/email_action.js b/x-pack/plugins/watcher/server/models/action/email_action.js index 0e45e906238ed..fea678607d12f 100644 --- a/x-pack/plugins/watcher/server/models/action/email_action.js +++ b/x-pack/plugins/watcher/server/models/action/email_action.js @@ -4,15 +4,14 @@ * you may not use this file except in compliance with the Elastic License. */ -import { badRequest } from 'boom'; import { BaseAction } from './base_action'; -import { ACTION_TYPES } from '../../../common/constants'; +import { ACTION_TYPES, ERROR_CODES } from '../../../common/constants'; import { i18n } from '@kbn/i18n'; export class EmailAction extends BaseAction { - constructor(props) { + constructor(props, errors) { props.type = ACTION_TYPES.EMAIL; - super(props); + super(props, errors); this.to = props.to; this.subject = props.subject; @@ -34,14 +33,16 @@ export class EmailAction extends BaseAction { // From Kibana static fromDownstreamJson(json) { const props = super.getPropsFromDownstreamJson(json); + const { errors } = this.validateJson(json); Object.assign(props, { to: json.to, subject: json.subject, - body: json.body + body: json.body, }); - return new EmailAction(props); + const action = new EmailAction(props, errors); + return { action, errors }; } // To Elasticsearch @@ -70,29 +71,10 @@ export class EmailAction extends BaseAction { // From Elasticsearch static fromUpstreamJson(json) { const props = super.getPropsFromUpstreamJson(json); - - if (!json.actionJson.email) { - throw badRequest( - i18n.translate('xpack.watcher.models.emailAction.absenceOfActionJsonEmailPropertyBadRequestMessage', { - defaultMessage: 'json argument must contain an {actionJsonEmail} property', - values: { - actionJsonEmail: 'actionJson.email' - } - }), - ); - } - if (!json.actionJson.email.to) { - throw badRequest( - i18n.translate('xpack.watcher.models.emailAction.absenceOfActionJsonEmailToPropertyBadRequestMessage', { - defaultMessage: 'json argument must contain an {actionJsonEmailTo} property', - values: { - actionJsonEmailTo: 'actionJson.email.to' - } - }), - ); - } + const { errors } = this.validateJson(json.actionJson); const optionalFields = {}; + if (json.actionJson.email.subject) { optionalFields.subject = json.actionJson.email.subject; } @@ -106,7 +88,45 @@ export class EmailAction extends BaseAction { ...optionalFields, }); - return new EmailAction(props); + const action = new EmailAction(props, errors); + + return { action, errors }; + } + + static validateJson(json) { + const errors = []; + + if (!json.email) { + const message = i18n.translate('xpack.watcher.models.emailAction.absenceOfActionJsonEmailPropertyBadRequestMessage', { + defaultMessage: 'json argument must contain an {actionJsonEmail} property', + values: { + actionJsonEmail: 'actionJson.email' + } + }); + + errors.push({ + code: ERROR_CODES.ERR_PROP_MISSING, + message + }); + + json.email = {}; + } + + if (!json.email.to) { + const message = i18n.translate('xpack.watcher.models.emailAction.absenceOfActionJsonEmailToPropertyBadRequestMessage', { + defaultMessage: 'json argument must contain an {actionJsonEmailTo} property', + values: { + actionJsonEmailTo: 'actionJson.email.to' + } + }); + + errors.push({ + code: ERROR_CODES.ERR_PROP_MISSING, + message + }); + } + + return { errors: errors.length ? errors : null }; } /* diff --git a/x-pack/plugins/watcher/server/models/action/logging_action.js b/x-pack/plugins/watcher/server/models/action/logging_action.js index fc1f02ad73931..62496d2f85f7b 100644 --- a/x-pack/plugins/watcher/server/models/action/logging_action.js +++ b/x-pack/plugins/watcher/server/models/action/logging_action.js @@ -4,15 +4,14 @@ * you may not use this file except in compliance with the Elastic License. */ -import { badRequest } from 'boom'; import { BaseAction } from './base_action'; -import { ACTION_TYPES } from '../../../common/constants'; +import { ACTION_TYPES, ERROR_CODES } from '../../../common/constants'; import { i18n } from '@kbn/i18n'; export class LoggingAction extends BaseAction { - constructor(props) { + constructor(props, errors) { props.type = ACTION_TYPES.LOGGING; - super(props); + super(props, errors); this.text = props.text; } @@ -30,12 +29,14 @@ export class LoggingAction extends BaseAction { // From Kibana static fromDownstreamJson(json) { const props = super.getPropsFromDownstreamJson(json); + const { errors } = this.validateJson(json); Object.assign(props, { text: json.text }); - return new LoggingAction(props); + const action = new LoggingAction(props, errors); + return { action, errors }; } // To Elasticsearch @@ -54,33 +55,46 @@ export class LoggingAction extends BaseAction { // From Elasticsearch static fromUpstreamJson(json) { const props = super.getPropsFromUpstreamJson(json); + const { errors } = this.validateJson(json.actionJson); - if (!json.actionJson.logging) { - throw badRequest( - i18n.translate('xpack.watcher.models.loggingAction.absenceOfActionJsonLoggingPropertyBadRequestMessage', { + Object.assign(props, { + text: json.actionJson.logging.text + }); + + const action = new LoggingAction(props, errors); + return { action, errors }; + } + + static validateJson(json) { + const errors = []; + + if (!json.logging) { + errors.push({ + code: ERROR_CODES.ERR_PROP_MISSING, + message: i18n.translate('xpack.watcher.models.loggingAction.absenceOfActionJsonLoggingPropertyBadRequestMessage', { defaultMessage: 'json argument must contain an {actionJsonLogging} property', values: { actionJsonLogging: 'actionJson.logging' } }), - ); + }); + + json.logging = {}; } - if (!json.actionJson.logging.text) { - throw badRequest( - i18n.translate('xpack.watcher.models.loggingAction.absenceOfActionJsonLoggingTextPropertyBadRequestMessage', { + + if (!json.logging.text) { + errors.push({ + code: ERROR_CODES.ERR_PROP_MISSING, + message: i18n.translate('xpack.watcher.models.loggingAction.absenceOfActionJsonLoggingTextPropertyBadRequestMessage', { defaultMessage: 'json argument must contain an {actionJsonLoggingText} property', values: { actionJsonLoggingText: 'actionJson.logging.text' } }), - ); + }); } - Object.assign(props, { - text: json.actionJson.logging.text - }); - - return new LoggingAction(props); + return { errors: errors.length ? errors : null }; } /* diff --git a/x-pack/plugins/watcher/server/models/action/slack_action.js b/x-pack/plugins/watcher/server/models/action/slack_action.js index 1852cfcde8c23..469aaae7b1368 100644 --- a/x-pack/plugins/watcher/server/models/action/slack_action.js +++ b/x-pack/plugins/watcher/server/models/action/slack_action.js @@ -4,15 +4,14 @@ * you may not use this file except in compliance with the Elastic License. */ -import { badRequest } from 'boom'; import { BaseAction } from './base_action'; -import { ACTION_TYPES } from '../../../common/constants'; +import { ACTION_TYPES, ERROR_CODES } from '../../../common/constants'; import { i18n } from '@kbn/i18n'; export class SlackAction extends BaseAction { - constructor(props) { + constructor(props, errors) { props.type = ACTION_TYPES.SLACK; - super(props); + super(props, errors); this.to = props.to; this.text = props.text; @@ -33,12 +32,15 @@ export class SlackAction extends BaseAction { static fromDownstreamJson(json) { const props = super.getPropsFromDownstreamJson(json); + const { errors } = this.validateJson(json); + Object.assign(props, { to: json.to, text: json.text }); - return new SlackAction(props); + const action = new SlackAction(props, errors); + return { action, errors }; } // To Elasticsearch @@ -60,45 +62,50 @@ export class SlackAction extends BaseAction { // From Elasticsearch static fromUpstreamJson(json) { const props = super.getPropsFromUpstreamJson(json); + const { errors } = this.validateJson(json.actionJson); + + Object.assign(props, { + to: json.actionJson.slack.message.to, + text: json.actionJson.slack.message.text + }); - if (!json.actionJson.slack) { - throw badRequest( - i18n.translate('xpack.watcher.models.slackAction.absenceOfActionJsonSlackPropertyBadRequestMessage', { + const action = new SlackAction(props, errors); + + return { action, errors }; + } + + static validateJson(json) { + const errors = []; + + if (!json.slack) { + errors.push({ + code: ERROR_CODES.ERR_PROP_MISSING, + message: i18n.translate('xpack.watcher.models.slackAction.absenceOfActionJsonSlackPropertyBadRequestMessage', { defaultMessage: 'json argument must contain an {actionJsonSlack} property', values: { actionJsonSlack: 'actionJson.slack' } - }), - ); + }) + }); + + json.slack = {}; } - if (!json.actionJson.slack.message) { - throw badRequest( - i18n.translate('xpack.watcher.models.slackAction.absenceOfActionJsonSlackMessagePropertyBadRequestMessage', { + + if (!json.slack.message) { + errors.push({ + code: ERROR_CODES.ERR_PROP_MISSING, + message: i18n.translate('xpack.watcher.models.slackAction.absenceOfActionJsonSlackMessagePropertyBadRequestMessage', { defaultMessage: 'json argument must contain an {actionJsonSlackMessage} property', values: { actionJsonSlackMessage: 'actionJson.slack.message' } - - }), - ); - } - if (!json.actionJson.slack.message.to) { - throw badRequest( - i18n.translate('xpack.watcher.models.slackAction.absenceOfActionJsonSlackMessageToPropertyBadRequestMessage', { - defaultMessage: 'json argument must contain an {actionJsonSlackMessageTo} property', - values: { - actionJsonSlackMessageTo: 'actionJson.slack.message.to' - } }), - ); - } + }); - Object.assign(props, { - to: json.actionJson.slack.message.to, - text: json.actionJson.slack.message.text - }); + json.slack.message = {}; + } - return new SlackAction(props); + return { errors: errors.length ? errors : null }; } /* diff --git a/x-pack/plugins/watcher/server/models/action/unknown_action.js b/x-pack/plugins/watcher/server/models/action/unknown_action.js index 7dfe83a1e4000..af8b827f770aa 100644 --- a/x-pack/plugins/watcher/server/models/action/unknown_action.js +++ b/x-pack/plugins/watcher/server/models/action/unknown_action.js @@ -4,15 +4,14 @@ * you may not use this file except in compliance with the Elastic License. */ -import { badRequest } from 'boom'; import { BaseAction } from './base_action'; -import { ACTION_TYPES } from '../../../common/constants'; +import { ACTION_TYPES, ERROR_CODES } from '../../../common/constants'; import { i18n } from '@kbn/i18n'; export class UnknownAction extends BaseAction { - constructor(props) { + constructor(props, errors) { props.type = ACTION_TYPES.UNKNOWN; - super(props); + super(props, errors); this.actionJson = props.actionJson; } @@ -51,23 +50,33 @@ export class UnknownAction extends BaseAction { // From Elasticsearch static fromUpstreamJson(json) { const props = super.getPropsFromUpstreamJson(json); + const { errors } = this.validateJson(json); + + Object.assign(props, { + actionJson: json.actionJson + }); + + const action = new UnknownAction(props, errors); + + return { action, errors }; + } + + static validateJson(json) { + const errors = []; if (!json.actionJson) { - throw badRequest( - i18n.translate('xpack.watcher.models.unknownAction.absenceOfActionJsonPropertyBadRequestMessage', { + errors.push({ + code: ERROR_CODES.ERR_PROP_MISSING, + message: i18n.translate('xpack.watcher.models.unknownAction.absenceOfActionJsonPropertyBadRequestMessage', { defaultMessage: 'json argument must contain an {actionJson} property', values: { actionJson: 'actionJson' } }), - ); + }); } - Object.assign(props, { - actionJson: json.actionJson - }); - - return new UnknownAction(props); + return { errors: errors.length ? errors : null }; } /* diff --git a/x-pack/plugins/watcher/server/models/action_status/__tests__/action_status.js b/x-pack/plugins/watcher/server/models/action_status/__tests__/action_status.js index 460fdd1a3eaf2..b30d29af611d7 100644 --- a/x-pack/plugins/watcher/server/models/action_status/__tests__/action_status.js +++ b/x-pack/plugins/watcher/server/models/action_status/__tests__/action_status.js @@ -36,7 +36,7 @@ describe('action_status', () => { 'timestamp': '2017-03-01T20:55:49.679Z', 'successful': true } - } + }, }; }); @@ -53,7 +53,7 @@ describe('action_status', () => { }); it('returns correct ActionStatus instance', () => { - const actionStatus = ActionStatus.fromUpstreamJson(upstreamJson); + const actionStatus = ActionStatus.fromUpstreamJson({ ...upstreamJson, errors: { foo: 'bar' } }); expect(actionStatus.id).to.be(upstreamJson.id); expect(actionStatus.lastAcknowledged).to @@ -68,6 +68,8 @@ describe('action_status', () => { .eql(moment(upstreamJson.actionStatusJson.last_throttle.timestamp)); expect(actionStatus.lastSuccessfulExecution).to .eql(moment(upstreamJson.actionStatusJson.last_successful_execution.timestamp)); + expect(actionStatus.errors).to + .eql({ foo: 'bar' }); }); }); @@ -106,6 +108,12 @@ describe('action_status', () => { expect(actionStatus.state).to.be(ACTION_STATES.ERROR); }); + it('correctly calculates ACTION_STATES.CONFIG_ERROR', () => { + const actionStatus = ActionStatus.fromUpstreamJson({ ...upstreamJson, errors: { foo: 'bar' } }); + expect(actionStatus.state).to.be(ACTION_STATES.CONFIG_ERROR); + }); + + it(`correctly calculates ACTION_STATES.OK`, () => { upstreamJson.actionStatusJson.ack.state = 'awaits_successful_execution'; const actionStatus = ActionStatus.fromUpstreamJson(upstreamJson); diff --git a/x-pack/plugins/watcher/server/models/action_status/action_status.js b/x-pack/plugins/watcher/server/models/action_status/action_status.js index 30319281e541f..e6f53d0bf764b 100644 --- a/x-pack/plugins/watcher/server/models/action_status/action_status.js +++ b/x-pack/plugins/watcher/server/models/action_status/action_status.js @@ -14,6 +14,7 @@ export class ActionStatus { constructor(props) { this.id = props.id; this.actionStatusJson = props.actionStatusJson; + this.errors = props.errors; this.lastAcknowledged = getMoment(get(this.actionStatusJson, 'ack.timestamp')); this.lastExecution = getMoment(get(this.actionStatusJson, 'last_execution.timestamp')); @@ -31,6 +32,10 @@ export class ActionStatus { return ACTION_STATES.ERROR; } + if (this.errors) { + return ACTION_STATES.CONFIG_ERROR; + } + if (ackState === 'awaits_successful_execution') { return ACTION_STATES.OK; } diff --git a/x-pack/plugins/watcher/server/models/watch/__tests__/base_watch.js b/x-pack/plugins/watcher/server/models/watch/__tests__/base_watch.js index 1316559561f37..9740874cb92ac 100644 --- a/x-pack/plugins/watcher/server/models/watch/__tests__/base_watch.js +++ b/x-pack/plugins/watcher/server/models/watch/__tests__/base_watch.js @@ -11,6 +11,8 @@ import sinon from 'sinon'; const actionFromUpstreamJSONMock = sinon.stub(); const actionFromDownstreamJSONMock = sinon.stub(); const watchStatusFromUpstreamJSONMock = sinon.stub(); +const watchErrorsFromUpstreamJSONMock = sinon.stub(); + class ActionStub { static fromUpstreamJson(...args) { actionFromUpstreamJSONMock(...args); @@ -30,9 +32,17 @@ class WatchStatusStub { } } +class WatchErrorsStub { + static fromUpstreamJson(...args) { + watchErrorsFromUpstreamJSONMock(...args); + return { foo: 'bar' }; + } +} + const { BaseWatch } = proxyquire('../base_watch', { '../action': { Action: ActionStub }, - '../watch_status': { WatchStatus: WatchStatusStub } + '../watch_status': { WatchStatus: WatchStatusStub }, + '../watch_errors': { WatchErrors: WatchErrorsStub }, }); describe('BaseWatch', () => { @@ -57,6 +67,7 @@ describe('BaseWatch', () => { 'type', 'isSystemWatch', 'watchStatus', + 'watchErrors', 'actions' ]; @@ -72,6 +83,7 @@ describe('BaseWatch', () => { it('populates all expected fields', () => { props.watchStatus = 'bar'; props.actions = 'baz'; + props.watchErrors = { actions: 'email' }; const actual = new BaseWatch(props); const expected = { @@ -80,6 +92,7 @@ describe('BaseWatch', () => { type: 'logging', isSystemWatch: false, watchStatus: 'bar', + watchErrors: { actions: 'email' }, actions: 'baz' }; @@ -169,6 +182,12 @@ describe('BaseWatch', () => { prop2: 'prop2' } }, + watchErrors: { + downstreamJson: { + prop1: 'prop1', + prop2: 'prop2' + } + }, actions: [{ downstreamJson: { prop1: 'prop3', @@ -188,14 +207,16 @@ describe('BaseWatch', () => { type: props.type, isSystemWatch: false, watchStatus: props.watchStatus.downstreamJson, + watchErrors: props.watchErrors.downstreamJson, actions: props.actions.map(a => a.downstreamJson) }; expect(actual).to.eql(expected); }); - it('should respect an undefined watchStatus prop', () => { + it('should respect an undefined watchStatus & watchErrors prop', () => { delete props.watchStatus; + delete props.watchErrors; const watch = new BaseWatch(props); const actual = watch.downstreamJson; @@ -206,6 +227,7 @@ describe('BaseWatch', () => { type: props.type, isSystemWatch: false, watchStatus: undefined, + watchErrors: undefined, actions: props.actions.map(a => a.downstreamJson) }; @@ -397,6 +419,7 @@ describe('BaseWatch', () => { 'name', 'watchJson', 'watchStatus', + 'watchErrors', 'actions' ]; @@ -456,6 +479,9 @@ describe('BaseWatch', () => { state: { active: true } + }, + watchErrors: { + foo: 'bar' } })).to.be(true); }); diff --git a/x-pack/plugins/watcher/server/models/watch/base_watch.js b/x-pack/plugins/watcher/server/models/watch/base_watch.js index 1f7f24b63b5df..35c524ec8a576 100644 --- a/x-pack/plugins/watcher/server/models/watch/base_watch.js +++ b/x-pack/plugins/watcher/server/models/watch/base_watch.js @@ -9,6 +9,7 @@ import { badRequest } from 'boom'; import { Action } from '../action'; import { WatchStatus } from '../watch_status'; import { i18n } from '@kbn/i18n'; +import { WatchErrors } from '../watch_errors'; export class BaseWatch { // This constructor should not be used directly. @@ -22,6 +23,7 @@ export class BaseWatch { this.isSystemWatch = false; this.watchStatus = props.watchStatus; + this.watchErrors = props.watchErrors; this.actions = props.actions; } @@ -57,6 +59,7 @@ export class BaseWatch { type: this.type, isSystemWatch: this.isSystemWatch, watchStatus: this.watchStatus ? this.watchStatus.downstreamJson : undefined, + watchErrors: this.watchErrors ? this.watchErrors.downstreamJson : undefined, actions: map(this.actions, (action) => action.downstreamJson) }; @@ -87,7 +90,7 @@ export class BaseWatch { } // from Elasticsearch - static getPropsFromUpstreamJson(json) { + static getPropsFromUpstreamJson(json, options) { if (!json.id) { throw badRequest( i18n.translate('xpack.watcher.models.baseWatch.absenceOfIdPropertyBadRequestMessage', { @@ -135,12 +138,15 @@ export class BaseWatch { const actionsJson = get(watchJson, 'actions', {}); const actions = map(actionsJson, (actionJson, actionId) => { - return Action.fromUpstreamJson({ id: actionId, actionJson }); + return Action.fromUpstreamJson({ id: actionId, actionJson }, options); }); + const watchErrors = WatchErrors.fromUpstreamJson(this.getWatchErrors(actions)); + const watchStatus = WatchStatus.fromUpstreamJson({ id, - watchStatusJson + watchStatusJson, + watchErrors, }); return { @@ -148,7 +154,31 @@ export class BaseWatch { name, watchJson, watchStatus, + watchErrors, actions }; } + + /** + * Retrieve all the errors in the watch + * + * @param {array} actions - Watch actions + */ + static getWatchErrors(actions) { + const watchErrors = {}; + + // Check for errors in Actions + const actionsErrors = actions.reduce((acc, action) => { + if (action.errors) { + acc[action.id] = action.errors; + } + return acc; + }, {}); + + if (Object.keys(actionsErrors).length) { + watchErrors.actions = actionsErrors; + } + + return watchErrors; + } } diff --git a/x-pack/plugins/watcher/server/models/watch/json_watch.js b/x-pack/plugins/watcher/server/models/watch/json_watch.js index de28004be5252..343c76054d2e0 100644 --- a/x-pack/plugins/watcher/server/models/watch/json_watch.js +++ b/x-pack/plugins/watcher/server/models/watch/json_watch.js @@ -48,8 +48,8 @@ export class JsonWatch extends BaseWatch { } // From Elasticsearch - static fromUpstreamJson(json) { - const baseProps = super.getPropsFromUpstreamJson(json); + static fromUpstreamJson(json, options) { + const baseProps = super.getPropsFromUpstreamJson(json, options); const watch = cloneDeep(baseProps.watchJson); if (has(watch, 'metadata.name')) { diff --git a/x-pack/plugins/watcher/server/models/watch/watch.js b/x-pack/plugins/watcher/server/models/watch/watch.js index 34752b55be360..c721abad4cc64 100644 --- a/x-pack/plugins/watcher/server/models/watch/watch.js +++ b/x-pack/plugins/watcher/server/models/watch/watch.js @@ -50,7 +50,7 @@ export class Watch { } // from Elasticsearch - static fromUpstreamJson(json) { + static fromUpstreamJson(json, options) { if (!json.watchJson) { throw badRequest( i18n.translate('xpack.watcher.models.watch.absenceOfWatchJsonPropertyBadRequestMessage', { @@ -65,6 +65,6 @@ export class Watch { const type = getWatchType(json.watchJson); const WatchType = WatchTypes[type]; - return WatchType.fromUpstreamJson(json); + return WatchType.fromUpstreamJson(json, options); } } diff --git a/x-pack/plugins/watcher/server/models/watch_errors/index.js b/x-pack/plugins/watcher/server/models/watch_errors/index.js new file mode 100644 index 0000000000000..dec5e51d74e31 --- /dev/null +++ b/x-pack/plugins/watcher/server/models/watch_errors/index.js @@ -0,0 +1,7 @@ +/* + * 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 { WatchErrors } from './watch_errors'; diff --git a/x-pack/plugins/watcher/server/models/watch_errors/watch_errors.js b/x-pack/plugins/watcher/server/models/watch_errors/watch_errors.js new file mode 100644 index 0000000000000..fbbe9d5ee6d68 --- /dev/null +++ b/x-pack/plugins/watcher/server/models/watch_errors/watch_errors.js @@ -0,0 +1,25 @@ +/* + * 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 class WatchErrors { + constructor({ actions } = {}) { + this.actions = actions; + } + + // generate object to send to kibana + get downstreamJson() { + const json = { + actions: this.actions, + }; + + return json; + } + + // generate object from elasticsearch response + static fromUpstreamJson(sections) { + return new WatchErrors(sections); + } +} diff --git a/x-pack/plugins/watcher/server/models/watch_errors/watch_errors.test.js b/x-pack/plugins/watcher/server/models/watch_errors/watch_errors.test.js new file mode 100644 index 0000000000000..edf0c09aa3387 --- /dev/null +++ b/x-pack/plugins/watcher/server/models/watch_errors/watch_errors.test.js @@ -0,0 +1,34 @@ +/* + * 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 { WatchErrors } from './watch_errors'; + +describe('watch_errors', () => { + describe('WatchErrors constructor', () => { + it('should set "actions" error', () => { + const watchErrors1 = new WatchErrors(); + const watchErrors2 = new WatchErrors({ actions: { foo: 'bar' } }); + + expect(watchErrors1.actions).toEqual(undefined); + expect(watchErrors2.actions).toEqual({ foo: 'bar' }); + }); + }); + + describe('fromUpstreamJson()', () => { + it('should return WatchErrors instance', () => { + const instance = WatchErrors.fromUpstreamJson(); + + expect(instance instanceof WatchErrors).toBe(true); + }); + + it('should pass errors secctions to the constructor', () => { + const instance = WatchErrors.fromUpstreamJson({ actions: { foo: 'bar' } }); + + expect(instance.actions).toEqual({ foo: 'bar' }); + }); + }); + +}); \ No newline at end of file diff --git a/x-pack/plugins/watcher/server/models/watch_history_item/__tests__/watch_history_item.js b/x-pack/plugins/watcher/server/models/watch_history_item/__tests__/watch_history_item.js index 267a78ee0a230..2d13243f8a6d3 100644 --- a/x-pack/plugins/watcher/server/models/watch_history_item/__tests__/watch_history_item.js +++ b/x-pack/plugins/watcher/server/models/watch_history_item/__tests__/watch_history_item.js @@ -73,7 +73,8 @@ describe('watch_history_item', () => { state: { active: upstreamJson.watchHistoryItemJson.status.state.active } - } + }, + watchErrors: {} }); }); diff --git a/x-pack/plugins/watcher/server/models/watch_status/__tests__/watch_status.js b/x-pack/plugins/watcher/server/models/watch_status/__tests__/watch_status.js index 5c33c5b0239b4..4b1649ab4c1e0 100644 --- a/x-pack/plugins/watcher/server/models/watch_status/__tests__/watch_status.js +++ b/x-pack/plugins/watcher/server/models/watch_status/__tests__/watch_status.js @@ -252,6 +252,17 @@ describe('watch_status', () => { expect(watchStatus.state).to.be(WATCH_STATES.ERROR); }); + it('correctly calculates WATCH_STATE.CONFIG_ERROR', () => { + const watchStatus = WatchStatus.fromUpstreamJson(upstreamJson); + + watchStatus.actionStatuses = [ + { state: ACTION_STATES.OK }, + { state: ACTION_STATES.CONFIG_ERROR } + ]; + + expect(watchStatus.state).to.be(WATCH_STATES.CONFIG_ERROR); + }); + it(`correctly calculates WATCH_STATES.DISABLED when watch is inactive`, () => { const watchStatus = WatchStatus.fromUpstreamJson(upstreamJson); watchStatus.isActive = false; diff --git a/x-pack/plugins/watcher/server/models/watch_status/watch_status.js b/x-pack/plugins/watcher/server/models/watch_status/watch_status.js index d6e6a8dbe1d7e..cf91fe829e90e 100644 --- a/x-pack/plugins/watcher/server/models/watch_status/watch_status.js +++ b/x-pack/plugins/watcher/server/models/watch_status/watch_status.js @@ -31,6 +31,7 @@ export class WatchStatus { this.id = props.id; this.watchState = props.state; this.watchStatusJson = props.watchStatusJson; + this.watchErrors = props.watchErrors || {}; this.isActive = Boolean(get(this.watchStatusJson, 'state.active')); this.lastChecked = getMoment(get(this.watchStatusJson, 'last_checked')); @@ -38,7 +39,11 @@ export class WatchStatus { const actionStatusesJson = get(this.watchStatusJson, 'actions', {}); this.actionStatuses = map(actionStatusesJson, (actionStatusJson, id) => { - const json = { id, actionStatusJson }; + const json = { + id, + actionStatusJson, + errors: this.watchErrors.actions && this.watchErrors.actions[id], + }; return ActionStatus.fromUpstreamJson(json); }); } @@ -58,6 +63,10 @@ export class WatchStatus { return WATCH_STATES.ERROR; } + if (totals[ACTION_STATES.CONFIG_ERROR] > 0) { + return WATCH_STATES.CONFIG_ERROR; + } + const firingTotal = totals[ACTION_STATES.FIRING] + totals[ACTION_STATES.ACKNOWLEDGED] + totals[ACTION_STATES.THROTTLED]; diff --git a/x-pack/plugins/watcher/server/routes/api/watch/register_load_route.js b/x-pack/plugins/watcher/server/routes/api/watch/register_load_route.js index 96cfccba7a16d..e5210dbff3567 100644 --- a/x-pack/plugins/watcher/server/routes/api/watch/register_load_route.js +++ b/x-pack/plugins/watcher/server/routes/api/watch/register_load_route.js @@ -30,16 +30,20 @@ export function registerLoadRoute(server) { const id = request.params.id; return fetchWatch(callWithRequest, id) - .then((hit) => { + .then(hit => { const watchJson = get(hit, 'watch'); const watchStatusJson = get(hit, 'status'); const json = { id, watchJson, - watchStatusJson + watchStatusJson, }; - const watch = Watch.fromUpstreamJson(json); + const watch = Watch.fromUpstreamJson(json, { + throwExceptions: { + Action: false, + }, + }); return { watch: watch.downstreamJson }; @@ -48,7 +52,7 @@ export function registerLoadRoute(server) { // Case: Error from Elasticsearch JS client if (isEsError(err)) { const statusCodeToMessageMap = { - 404: `Watch with id = ${id} not found` + 404: `Watch with id = ${id} not found`, }; throw wrapEsError(err, statusCodeToMessageMap); } diff --git a/x-pack/plugins/watcher/server/routes/api/watches/register_list_route.js b/x-pack/plugins/watcher/server/routes/api/watches/register_list_route.js index e3424386dc705..2a617e275d1ee 100644 --- a/x-pack/plugins/watcher/server/routes/api/watches/register_list_route.js +++ b/x-pack/plugins/watcher/server/routes/api/watches/register_list_route.js @@ -44,11 +44,18 @@ export function registerListRoute(server) { const watchJson = get(hit, '_source'); const watchStatusJson = get(hit, '_source.status'); - return Watch.fromUpstreamJson({ - id, - watchJson, - watchStatusJson - }); + return Watch.fromUpstreamJson( + { + id, + watchJson, + watchStatusJson, + }, + { + throwExceptions: { + Action: false, + }, + } + ); }); return {