From 52d78c02f501e5c1f61613520613e8153144fdc9 Mon Sep 17 00:00:00 2001 From: Jonathan Buttner Date: Thu, 10 Nov 2022 16:40:00 -0500 Subject: [PATCH 1/7] Working rule.url need to write tests --- packages/kbn-rule-data-utils/index.ts | 1 + packages/kbn-rule-data-utils/src/paths.ts | 12 +++++++++++ .../server/task_runner/execution_handler.ts | 21 ++++++++++++++++++- .../task_runner/transform_action_params.ts | 3 +++ .../public/application/app.tsx | 10 +++------ .../public/application/constants/index.ts | 1 - .../application/lib/action_variables.ts | 9 ++++++++ .../rule_details/components/rule_details.tsx | 5 +++-- .../rule_event_log_list_cell_renderer.tsx | 6 ++++-- .../rules_list/components/rules_list.tsx | 5 +++-- .../triggers_actions_ui/public/plugin.ts | 5 +++-- 11 files changed, 61 insertions(+), 17 deletions(-) create mode 100644 packages/kbn-rule-data-utils/src/paths.ts diff --git a/packages/kbn-rule-data-utils/index.ts b/packages/kbn-rule-data-utils/index.ts index ddf6215aaba90..8c9fe22f6ff80 100644 --- a/packages/kbn-rule-data-utils/index.ts +++ b/packages/kbn-rule-data-utils/index.ts @@ -10,3 +10,4 @@ export * from './src/technical_field_names'; export * from './src/alerts_as_data_rbac'; export * from './src/alerts_as_data_severity'; export * from './src/alerts_as_data_status'; +export * from './src/paths'; diff --git a/packages/kbn-rule-data-utils/src/paths.ts b/packages/kbn-rule-data-utils/src/paths.ts new file mode 100644 index 0000000000000..8a4623f9c6c41 --- /dev/null +++ b/packages/kbn-rule-data-utils/src/paths.ts @@ -0,0 +1,12 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +export const ruleDetailsRoute = '/rule/:ruleId'; +export const triggersActionsRoute = '/app/management/insightsAndAlerting/triggersActions'; + +export const getRuleDetailsRoute = (ruleId: string) => ruleDetailsRoute.replace(':ruleId', ruleId); diff --git a/x-pack/plugins/alerting/server/task_runner/execution_handler.ts b/x-pack/plugins/alerting/server/task_runner/execution_handler.ts index 2d72bcd284a66..aefd0b0c9dfff 100644 --- a/x-pack/plugins/alerting/server/task_runner/execution_handler.ts +++ b/x-pack/plugins/alerting/server/task_runner/execution_handler.ts @@ -7,11 +7,12 @@ import type { PublicMethodsOf } from '@kbn/utility-types'; import { Logger } from '@kbn/core/server'; +import { getRuleDetailsRoute, triggersActionsRoute } from '@kbn/rule-data-utils'; import { asSavedObjectExecutionSource } from '@kbn/actions-plugin/server'; import { isEphemeralTaskRejectedDueToCapacityError } from '@kbn/task-manager-plugin/server'; import { ExecuteOptions as EnqueueExecutionOptions } from '@kbn/actions-plugin/server/create_execute_function'; import { ActionsClient } from '@kbn/actions-plugin/server/actions_client'; -import { chunk } from 'lodash'; +import { chunk, isString } from 'lodash'; import { AlertingEventLogger } from '../lib/alerting_event_logger/alerting_event_logger'; import { RawRule } from '../types'; import { RuleRunMetricsStore } from '../lib/rule_run_metrics_store'; @@ -208,6 +209,7 @@ export class ExecutionHandler< kibanaBaseUrl: this.taskRunnerContext.kibanaBaseUrl, alertParams: this.rule.params, actionParams: action.params, + ruleUrl: this.buildRuleUrl(), }), }), }; @@ -282,6 +284,23 @@ export class ExecutionHandler< return executables; } + private buildRuleUrl(): string | undefined { + if (!this.taskRunnerContext.kibanaBaseUrl || !isString(this.taskInstance.params.alertId)) { + return; + } + + try { + const ruleUrl = new URL( + `${triggersActionsRoute}${getRuleDetailsRoute(this.taskInstance.params.alertId)}`, + this.taskRunnerContext.kibanaBaseUrl + ); + + return ruleUrl.toString(); + } catch (error) { + return; + } + } + private async actionRunOrAddToBulk({ enqueueOptions, bulkActions, diff --git a/x-pack/plugins/alerting/server/task_runner/transform_action_params.ts b/x-pack/plugins/alerting/server/task_runner/transform_action_params.ts index 61ec54c48886f..1b44b534a4271 100644 --- a/x-pack/plugins/alerting/server/task_runner/transform_action_params.ts +++ b/x-pack/plugins/alerting/server/task_runner/transform_action_params.ts @@ -30,6 +30,7 @@ interface TransformActionParamsOptions { state: AlertInstanceState; kibanaBaseUrl?: string; context: AlertInstanceContext; + ruleUrl?: string; } export function transformActionParams({ @@ -49,6 +50,7 @@ export function transformActionParams({ state, kibanaBaseUrl, alertParams, + ruleUrl, }: TransformActionParamsOptions): RuleActionParams { // when the list of variables we pass in here changes, // the UI will need to be updated as well; see: @@ -72,6 +74,7 @@ export function transformActionParams({ type: alertType, spaceId, tags, + url: ruleUrl, }, alert: { id: alertInstanceId, diff --git a/x-pack/plugins/triggers_actions_ui/public/application/app.tsx b/x-pack/plugins/triggers_actions_ui/public/application/app.tsx index d0c61c884e528..b7bc34819836d 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/app.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/app.tsx @@ -25,18 +25,14 @@ import type { SpacesPluginStart } from '@kbn/spaces-plugin/public'; import { Storage } from '@kbn/kibana-utils-plugin/public'; import { EuiThemeProvider } from '@kbn/kibana-react-plugin/common'; import { ActionsPublicPluginSetup } from '@kbn/actions-plugin/public'; +import { ruleDetailsRoute } from '@kbn/rule-data-utils'; import { suspendedComponentWithProps } from './lib/suspended_component_with_props'; import { ActionTypeRegistryContract, AlertsTableConfigurationRegistryContract, RuleTypeRegistryContract, } from '../types'; -import { - Section, - routeToRuleDetails, - legacyRouteToRuleDetails, - routeToConnectors, -} from './constants'; +import { Section, legacyRouteToRuleDetails, routeToConnectors } from './constants'; import { setDataViewsService } from '../common/lib/data_apis'; import { KibanaContextProvider, useKibana } from '../common/lib/kibana'; @@ -113,7 +109,7 @@ export const AppWithoutRouter = ({ sectionsRegex }: { sectionsRegex: string }) = component={suspendedComponentWithProps(TriggersActionsUIHome, 'xl')} /> = ({ }, [rule.schedule.interval, config.minimumScheduleInterval, toasts, hasEditButton]); const setRule = async () => { - history.push(routeToRuleDetails.replace(`:ruleId`, rule.id)); + history.push(getRuleDetailsRoute(rule.id)); }; const goToRulesList = () => { diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule_event_log_list_cell_renderer.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule_event_log_list_cell_renderer.tsx index bcca56ad0027e..b39d480b42245 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule_event_log_list_cell_renderer.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule_event_log_list_cell_renderer.tsx @@ -10,7 +10,7 @@ import moment from 'moment'; import { EuiLink } from '@elastic/eui'; import { RuleAlertingOutcome } from '@kbn/alerting-plugin/common'; import { useHistory } from 'react-router-dom'; -import { routeToRuleDetails } from '../../../constants'; +import { getRuleDetailsRoute } from '@kbn/rule-data-utils'; import { formatRuleAlertCount } from '../../../../common/lib/format_rule_alert_count'; import { useKibana, useSpacesData } from '../../../../common/lib/kibana'; import { RuleEventLogListStatus } from './rule_event_log_list_status'; @@ -53,7 +53,9 @@ export const RuleEventLogListCellRenderer = (props: RuleEventLogListCellRenderer const ruleNamePathname = useMemo(() => { if (!ruleId) return ''; - const ruleRoute = routeToRuleDetails.replace(':ruleId', ruleId); + + const ruleRoute = getRuleDetailsRoute(ruleId); + if (ruleOnDifferentSpace) { const [linkedSpaceId] = spaceIds ?? []; const basePath = http.basePath.get(); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rules_list/components/rules_list.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rules_list/components/rules_list.tsx index ea042c1f8b782..abf12f7a94db3 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rules_list/components/rules_list.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rules_list/components/rules_list.tsx @@ -39,6 +39,7 @@ import { RuleExecutionStatusErrorReasons, } from '@kbn/alerting-plugin/common'; import { AlertingConnectorFeatureId } from '@kbn/actions-plugin/common'; +import { ruleDetailsRoute as commonRuleDetailsRoute } from '@kbn/rule-data-utils'; import { ActionType, Rule, @@ -69,7 +70,7 @@ import { } from '../../../lib/rule_api'; import { loadActionTypes } from '../../../lib/action_connector_api'; import { hasAllPrivilege, hasExecuteActionsCapability } from '../../../lib/capabilities'; -import { routeToRuleDetails, DEFAULT_SEARCH_PAGE_SIZE } from '../../../constants'; +import { DEFAULT_SEARCH_PAGE_SIZE } from '../../../constants'; import { DeleteModalConfirmation } from '../../../components/delete_modal_confirmation'; import { EmptyPrompt } from '../../../components/prompts/empty_prompt'; import { ALERT_STATUS_LICENSE_ERROR } from '../translations'; @@ -870,7 +871,7 @@ export const RulesList = ({ onPage={setPage} onRuleChanged={() => loadData()} onRuleClick={(rule) => { - const detailsRoute = ruleDetailsRoute ? ruleDetailsRoute : routeToRuleDetails; + const detailsRoute = ruleDetailsRoute ? ruleDetailsRoute : commonRuleDetailsRoute; history.push(detailsRoute.replace(`:ruleId`, rule.id)); }} onRuleEditClick={(rule) => { diff --git a/x-pack/plugins/triggers_actions_ui/public/plugin.ts b/x-pack/plugins/triggers_actions_ui/public/plugin.ts index d8ebc2298dca9..9bfc31db44c55 100644 --- a/x-pack/plugins/triggers_actions_ui/public/plugin.ts +++ b/x-pack/plugins/triggers_actions_ui/public/plugin.ts @@ -23,6 +23,7 @@ import type { DataViewEditorStart } from '@kbn/data-view-editor-plugin/public'; import { Storage } from '@kbn/kibana-utils-plugin/public'; import type { SpacesPluginStart } from '@kbn/spaces-plugin/public'; import type { UnifiedSearchPublicPluginStart } from '@kbn/unified-search-plugin/public'; +import { triggersActionsRoute } from '@kbn/rule-data-utils'; import type { AlertsSearchBarProps } from './application/sections/alerts_search_bar'; import { TypeRegistry } from './application/type_registry'; @@ -212,7 +213,7 @@ export class Plugin title: featureTitle, description: featureDescription, icon: 'watchesApp', - path: '/app/management/insightsAndAlerting/triggersActions', + path: triggersActionsRoute, showOnHomePage: false, category: 'admin', }); @@ -221,7 +222,7 @@ export class Plugin title: connectorsFeatureTitle, description: connectorsFeatureDescription, icon: 'watchesApp', - path: '/app/management/insightsAndAlerting/triggersActions', + path: triggersActionsRoute, showOnHomePage: false, category: 'admin', }); From 86c6000957547cfaf5d6ef85daa45c597f4522aa Mon Sep 17 00:00:00 2001 From: Jonathan Buttner Date: Mon, 14 Nov 2022 11:13:33 -0500 Subject: [PATCH 2/7] Adding tests for rule.url --- .../task_runner/execution_handler.test.ts | 517 +++++++++++++----- 1 file changed, 383 insertions(+), 134 deletions(-) diff --git a/x-pack/plugins/alerting/server/task_runner/execution_handler.test.ts b/x-pack/plugins/alerting/server/task_runner/execution_handler.test.ts index c75107c52cb81..2b8f816369664 100644 --- a/x-pack/plugins/alerting/server/task_runner/execution_handler.test.ts +++ b/x-pack/plugins/alerting/server/task_runner/execution_handler.test.ts @@ -13,7 +13,7 @@ import { renderActionParameterTemplatesDefault, } from '@kbn/actions-plugin/server/mocks'; import { KibanaRequest } from '@kbn/core/server'; -import { InjectActionParamsOpts } from './inject_action_params'; +import { InjectActionParamsOpts, injectActionParams } from './inject_action_params'; import { NormalizedRuleType } from '../rule_type_registry'; import { ActionsCompletion, RuleTypeParams, RuleTypeState, SanitizedRule } from '../types'; import { RuleRunMetricsStore } from '../lib/rule_run_metrics_store'; @@ -29,6 +29,8 @@ jest.mock('./inject_action_params', () => ({ injectActionParams: jest.fn(), })); +const injectActionParamsMock = injectActionParams as jest.Mock; + const alertingEventLogger = alertingEventLoggerMock.create(); const actionsClient = actionsClientMock.create(); const mockActionsPlugin = actionsMock.createStart(); @@ -184,39 +186,39 @@ describe('Execution Handler', () => { expect(ruleRunMetricsStore.getNumberOfGeneratedActions()).toBe(1); expect(actionsClient.bulkEnqueueExecution).toHaveBeenCalledTimes(1); expect(actionsClient.bulkEnqueueExecution.mock.calls[0]).toMatchInlineSnapshot(` - Array [ - Array [ - Object { - "apiKey": "MTIzOmFiYw==", - "consumer": "rule-consumer", - "executionId": "5f6aa57d-3e22-484e-bae8-cbed868f4d28", - "id": "1", - "params": Object { - "alertVal": "My 1 name-of-alert test1 tag-A,tag-B 1 goes here", - "contextVal": "My goes here", - "foo": true, - "stateVal": "My goes here", - }, - "relatedSavedObjects": Array [ - Object { - "id": "1", - "namespace": "test1", - "type": "alert", - "typeId": "test", - }, - ], - "source": Object { - "source": Object { - "id": "1", - "type": "alert", - }, - "type": "SAVED_OBJECT", - }, - "spaceId": "test1", - }, - ], - ] - `); + Array [ + Array [ + Object { + "apiKey": "MTIzOmFiYw==", + "consumer": "rule-consumer", + "executionId": "5f6aa57d-3e22-484e-bae8-cbed868f4d28", + "id": "1", + "params": Object { + "alertVal": "My 1 name-of-alert test1 tag-A,tag-B 1 goes here", + "contextVal": "My goes here", + "foo": true, + "stateVal": "My goes here", + }, + "relatedSavedObjects": Array [ + Object { + "id": "1", + "namespace": "test1", + "type": "alert", + "typeId": "test", + }, + ], + "source": Object { + "source": Object { + "id": "1", + "type": "alert", + }, + "type": "SAVED_OBJECT", + }, + "spaceId": "test1", + }, + ], + ] + `); expect(alertingEventLogger.logAction).toHaveBeenCalledTimes(1); expect(alertingEventLogger.logAction).toHaveBeenNthCalledWith(1, { @@ -308,7 +310,7 @@ describe('Execution Handler', () => { ]); }); - test('trow error error message when action type is disabled', async () => { + test('throw error message when action type is disabled', async () => { mockActionsPlugin.preconfiguredActions = []; mockActionsPlugin.isActionExecutable.mockReturnValue(false); mockActionsPlugin.isActionTypeEnabled.mockReturnValue(false); @@ -370,39 +372,39 @@ describe('Execution Handler', () => { expect(ruleRunMetricsStore.getNumberOfGeneratedActions()).toBe(1); expect(actionsClient.bulkEnqueueExecution).toHaveBeenCalledTimes(1); expect(actionsClient.bulkEnqueueExecution.mock.calls[0]).toMatchInlineSnapshot(` - Array [ - Array [ - Object { - "apiKey": "MTIzOmFiYw==", - "consumer": "rule-consumer", - "executionId": "5f6aa57d-3e22-484e-bae8-cbed868f4d28", - "id": "1", - "params": Object { - "alertVal": "My 1 name-of-alert test1 tag-A,tag-B 2 goes here", - "contextVal": "My context-val goes here", - "foo": true, - "stateVal": "My goes here", - }, - "relatedSavedObjects": Array [ - Object { - "id": "1", - "namespace": "test1", - "type": "alert", - "typeId": "test", - }, - ], - "source": Object { - "source": Object { - "id": "1", - "type": "alert", - }, - "type": "SAVED_OBJECT", - }, - "spaceId": "test1", - }, - ], - ] - `); + Array [ + Array [ + Object { + "apiKey": "MTIzOmFiYw==", + "consumer": "rule-consumer", + "executionId": "5f6aa57d-3e22-484e-bae8-cbed868f4d28", + "id": "1", + "params": Object { + "alertVal": "My 1 name-of-alert test1 tag-A,tag-B 2 goes here", + "contextVal": "My context-val goes here", + "foo": true, + "stateVal": "My goes here", + }, + "relatedSavedObjects": Array [ + Object { + "id": "1", + "namespace": "test1", + "type": "alert", + "typeId": "test", + }, + ], + "source": Object { + "source": Object { + "id": "1", + "type": "alert", + }, + "type": "SAVED_OBJECT", + }, + "spaceId": "test1", + }, + ], + ] + `); }); test('state attribute gets parameterized', async () => { @@ -410,39 +412,39 @@ describe('Execution Handler', () => { await executionHandler.run(generateAlert({ id: 2, state: { value: 'state-val' } })); expect(actionsClient.bulkEnqueueExecution).toHaveBeenCalledTimes(1); expect(actionsClient.bulkEnqueueExecution.mock.calls[0]).toMatchInlineSnapshot(` - Array [ - Array [ - Object { - "apiKey": "MTIzOmFiYw==", - "consumer": "rule-consumer", - "executionId": "5f6aa57d-3e22-484e-bae8-cbed868f4d28", - "id": "1", - "params": Object { - "alertVal": "My 1 name-of-alert test1 tag-A,tag-B 2 goes here", - "contextVal": "My goes here", - "foo": true, - "stateVal": "My state-val goes here", - }, - "relatedSavedObjects": Array [ - Object { - "id": "1", - "namespace": "test1", - "type": "alert", - "typeId": "test", - }, - ], - "source": Object { - "source": Object { - "id": "1", - "type": "alert", - }, - "type": "SAVED_OBJECT", - }, - "spaceId": "test1", - }, - ], - ] - `); + Array [ + Array [ + Object { + "apiKey": "MTIzOmFiYw==", + "consumer": "rule-consumer", + "executionId": "5f6aa57d-3e22-484e-bae8-cbed868f4d28", + "id": "1", + "params": Object { + "alertVal": "My 1 name-of-alert test1 tag-A,tag-B 2 goes here", + "contextVal": "My goes here", + "foo": true, + "stateVal": "My state-val goes here", + }, + "relatedSavedObjects": Array [ + Object { + "id": "1", + "namespace": "test1", + "type": "alert", + "typeId": "test", + }, + ], + "source": Object { + "source": Object { + "id": "1", + "type": "alert", + }, + "type": "SAVED_OBJECT", + }, + "spaceId": "test1", + }, + ], + ] + `); }); test(`logs an error when action group isn't part of actionGroups available for the ruleType`, async () => { @@ -622,39 +624,39 @@ describe('Execution Handler', () => { await executionHandler.run(generateAlert({ id: 1, scheduleActions: false }), true); expect(actionsClient.bulkEnqueueExecution).toHaveBeenCalledTimes(1); expect(actionsClient.bulkEnqueueExecution.mock.calls[0]).toMatchInlineSnapshot(` - Array [ - Array [ - Object { - "apiKey": "MTIzOmFiYw==", - "consumer": "rule-consumer", - "executionId": "5f6aa57d-3e22-484e-bae8-cbed868f4d28", - "id": "1", - "params": Object { - "alertVal": "My 1 name-of-alert test1 tag-A,tag-B 1 goes here", - "contextVal": "My goes here", - "foo": true, - "stateVal": "My goes here", - }, - "relatedSavedObjects": Array [ - Object { - "id": "1", - "namespace": "test1", - "type": "alert", - "typeId": "test", - }, - ], - "source": Object { - "source": Object { - "id": "1", - "type": "alert", - }, - "type": "SAVED_OBJECT", - }, - "spaceId": "test1", - }, - ], - ] - `); + Array [ + Array [ + Object { + "apiKey": "MTIzOmFiYw==", + "consumer": "rule-consumer", + "executionId": "5f6aa57d-3e22-484e-bae8-cbed868f4d28", + "id": "1", + "params": Object { + "alertVal": "My 1 name-of-alert test1 tag-A,tag-B 1 goes here", + "contextVal": "My goes here", + "foo": true, + "stateVal": "My goes here", + }, + "relatedSavedObjects": Array [ + Object { + "id": "1", + "namespace": "test1", + "type": "alert", + "typeId": "test", + }, + ], + "source": Object { + "source": Object { + "id": "1", + "type": "alert", + }, + "type": "SAVED_OBJECT", + }, + "spaceId": "test1", + }, + ], + ] + `); }); test('does not schedule alerts with recovered actions that are muted', async () => { @@ -729,4 +731,251 @@ describe('Execution Handler', () => { `skipping scheduling of actions for '1' in rule ${defaultExecutionParams.ruleLabel}: rule is muted` ); }); + + describe('rule url', () => { + const ruleWithUrl = { + ...rule, + actions: [ + { + id: '1', + group: 'default', + actionTypeId: 'test', + params: { + val: 'rule url: {{rule.url}}', + }, + }, + ], + } as unknown as SanitizedRule; + + it('populates the rule.url in the action params when the base url and alert id are specified', async () => { + const execParams = { + ...defaultExecutionParams, + rule: ruleWithUrl, + taskRunnerContext: { + ...defaultExecutionParams.taskRunnerContext, + kibanaBaseUrl: 'http://localhost:12345', + }, + }; + + const executionHandler = new ExecutionHandler(generateExecutionParams(execParams)); + await executionHandler.run(generateAlert({ id: 1 })); + + expect(injectActionParamsMock.mock.calls[0]).toMatchInlineSnapshot(` + Array [ + Object { + "actionParams": Object { + "val": "rule url: http://localhost:12345/app/management/insightsAndAlerting/triggersActions/rule/1", + }, + "actionTypeId": "test", + "ruleId": "1", + "spaceId": "test1", + }, + ] + `); + }); + + it('populates the rule.url in the action params when the base url has a trailing slash and removes the trailing slash', async () => { + const execParams = { + ...defaultExecutionParams, + rule: ruleWithUrl, + taskRunnerContext: { + ...defaultExecutionParams.taskRunnerContext, + kibanaBaseUrl: 'http://localhost:12345/', + }, + }; + + const executionHandler = new ExecutionHandler(generateExecutionParams(execParams)); + await executionHandler.run(generateAlert({ id: 1 })); + + expect(injectActionParamsMock.mock.calls[0]).toMatchInlineSnapshot(` + Array [ + Object { + "actionParams": Object { + "val": "rule url: http://localhost:12345/app/management/insightsAndAlerting/triggersActions/rule/1", + }, + "actionTypeId": "test", + "ruleId": "1", + "spaceId": "test1", + }, + ] + `); + }); + + it('does not populate the rule.url when the base url is not specified', async () => { + const execParams = { + ...defaultExecutionParams, + rule: ruleWithUrl, + taskRunnerContext: { + ...defaultExecutionParams.taskRunnerContext, + kibanaBaseUrl: undefined, + }, + }; + + const executionHandler = new ExecutionHandler(generateExecutionParams(execParams)); + await executionHandler.run(generateAlert({ id: 1 })); + + expect(injectActionParamsMock.mock.calls[0]).toMatchInlineSnapshot(` + Array [ + Object { + "actionParams": Object { + "val": "rule url: ", + }, + "actionTypeId": "test", + "ruleId": "1", + "spaceId": "test1", + }, + ] + `); + }); + + it('does not populate the rule.url when the alert id is not defined', async () => { + const execParams = { + ...defaultExecutionParams, + rule: ruleWithUrl, + taskRunnerContext: { + ...defaultExecutionParams.taskRunnerContext, + kibanaBaseUrl: 'http://localhost:12345', + }, + taskInstance: { + params: { spaceId: 'test1' }, + } as unknown as ConcreteTaskInstance, + }; + + const executionHandler = new ExecutionHandler(generateExecutionParams(execParams)); + await executionHandler.run(generateAlert({ id: 1 })); + + expect(injectActionParamsMock.mock.calls[0]).toMatchInlineSnapshot(` + Array [ + Object { + "actionParams": Object { + "val": "rule url: ", + }, + "actionTypeId": "test", + "ruleId": undefined, + "spaceId": "test1", + }, + ] + `); + }); + + it('does not populate the rule.url when the alert id is set to undefined', async () => { + const execParams = { + ...defaultExecutionParams, + rule: ruleWithUrl, + taskRunnerContext: { + ...defaultExecutionParams.taskRunnerContext, + kibanaBaseUrl: 'http://localhost:12345', + }, + taskInstance: { + params: { spaceId: 'test1', alertId: undefined }, + } as unknown as ConcreteTaskInstance, + }; + + const executionHandler = new ExecutionHandler(generateExecutionParams(execParams)); + await executionHandler.run(generateAlert({ id: 1 })); + + expect(injectActionParamsMock.mock.calls[0]).toMatchInlineSnapshot(` + Array [ + Object { + "actionParams": Object { + "val": "rule url: ", + }, + "actionTypeId": "test", + "ruleId": undefined, + "spaceId": "test1", + }, + ] + `); + }); + + it('does not populate the rule.url when the alert id is a number', async () => { + const execParams = { + ...defaultExecutionParams, + rule: ruleWithUrl, + taskRunnerContext: { + ...defaultExecutionParams.taskRunnerContext, + kibanaBaseUrl: 'http://localhost:12345', + }, + taskInstance: { + params: { spaceId: 'test1', alertId: 1 }, + } as unknown as ConcreteTaskInstance, + }; + + const executionHandler = new ExecutionHandler(generateExecutionParams(execParams)); + await executionHandler.run(generateAlert({ id: 1 })); + + expect(injectActionParamsMock.mock.calls[0]).toMatchInlineSnapshot(` + Array [ + Object { + "actionParams": Object { + "val": "rule url: ", + }, + "actionTypeId": "test", + "ruleId": 1, + "spaceId": "test1", + }, + ] + `); + }); + + it('does not populate the rule.url when base url is not a valid url', async () => { + const execParams = { + ...defaultExecutionParams, + rule: ruleWithUrl, + taskRunnerContext: { + ...defaultExecutionParams.taskRunnerContext, + kibanaBaseUrl: 'localhost12345', + }, + taskInstance: { + params: { spaceId: 'test1', alertId: '1' }, + } as unknown as ConcreteTaskInstance, + }; + + const executionHandler = new ExecutionHandler(generateExecutionParams(execParams)); + await executionHandler.run(generateAlert({ id: 1 })); + + expect(injectActionParamsMock.mock.calls[0]).toMatchInlineSnapshot(` + Array [ + Object { + "actionParams": Object { + "val": "rule url: ", + }, + "actionTypeId": "test", + "ruleId": "1", + "spaceId": "test1", + }, + ] + `); + }); + + it('does not populate the rule.url when base url is a number', async () => { + const execParams = { + ...defaultExecutionParams, + rule: ruleWithUrl, + taskRunnerContext: { + ...defaultExecutionParams.taskRunnerContext, + kibanaBaseUrl: 1, + }, + taskInstance: { + params: { spaceId: 'test1', alertId: '1' }, + } as unknown as ConcreteTaskInstance, + }; + + const executionHandler = new ExecutionHandler(generateExecutionParams(execParams)); + await executionHandler.run(generateAlert({ id: 1 })); + + expect(injectActionParamsMock.mock.calls[0]).toMatchInlineSnapshot(` + Array [ + Object { + "actionParams": Object { + "val": "rule url: ", + }, + "actionTypeId": "test", + "ruleId": "1", + "spaceId": "test1", + }, + ] + `); + }); + }); }); From d7edbb857410512f381434916705b63fbd9cd2cd Mon Sep 17 00:00:00 2001 From: Jonathan Buttner Date: Mon, 14 Nov 2022 12:58:04 -0500 Subject: [PATCH 3/7] Fixing tests --- .../application/lib/action_variables.test.ts | 5 +++++ .../public/application/lib/action_variables.ts | 16 ++++++++-------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/x-pack/plugins/triggers_actions_ui/public/application/lib/action_variables.test.ts b/x-pack/plugins/triggers_actions_ui/public/application/lib/action_variables.test.ts index 9c123a822a541..4b36a8e8aeca3 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/lib/action_variables.test.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/lib/action_variables.test.ts @@ -43,6 +43,11 @@ const expectedTransformResult = [ { description: 'The space ID of the rule.', name: 'rule.spaceId' }, { description: 'The tags of the rule.', name: 'rule.tags' }, { description: 'The type of rule.', name: 'rule.type' }, + { + description: + 'The URL to the Stack Management rule page that generated the alert. This will be an empty string if the server.publicBaseUrl is not configured.', + name: 'rule.url', + }, { description: 'The date the rule scheduled the action.', name: 'date' }, { description: 'The ID of the alert that scheduled actions for the rule.', name: 'alert.id' }, { diff --git a/x-pack/plugins/triggers_actions_ui/public/application/lib/action_variables.ts b/x-pack/plugins/triggers_actions_ui/public/application/lib/action_variables.ts index 5947c203fdd63..42ae5bedc0747 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/lib/action_variables.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/lib/action_variables.ts @@ -106,6 +106,14 @@ function getAlwaysProvidedActionVariables(): ActionVariable[] { }), }); + result.push({ + name: AlertProvidedActionVariables.ruleUrl, + description: i18n.translate('xpack.triggersActionsUI.actionVariables.ruleUrlLabel', { + defaultMessage: + 'The URL to the Stack Management rule page that generated the alert. This will be an empty string if the server.publicBaseUrl is not configured.', + }), + }); + result.push({ name: AlertProvidedActionVariables.date, description: i18n.translate('xpack.triggersActionsUI.actionVariables.dateLabel', { @@ -156,14 +164,6 @@ function getAlwaysProvidedActionVariables(): ActionVariable[] { }), }); - result.push({ - name: AlertProvidedActionVariables.ruleUrl, - description: i18n.translate('xpack.triggersActionsUI.actionVariables.ruleUrlLabel', { - defaultMessage: - 'The URL to the Stack Management rule page that generated the alert. This will be an empty string if the server.publicBaseUrl is not configured.', - }), - }); - result.push({ name: LegacyAlertProvidedActionVariables.alertId, deprecated: true, From 542554367b6a28263ca7809ca228f155ff68b99d Mon Sep 17 00:00:00 2001 From: Jonathan Buttner Date: Mon, 14 Nov 2022 16:59:57 -0500 Subject: [PATCH 4/7] Addressing feedback --- .../alerting/server/task_runner/execution_handler.test.ts | 2 +- .../plugins/alerting/server/task_runner/execution_handler.ts | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/alerting/server/task_runner/execution_handler.test.ts b/x-pack/plugins/alerting/server/task_runner/execution_handler.test.ts index 2b8f816369664..9e7d9859b0553 100644 --- a/x-pack/plugins/alerting/server/task_runner/execution_handler.test.ts +++ b/x-pack/plugins/alerting/server/task_runner/execution_handler.test.ts @@ -858,7 +858,7 @@ describe('Execution Handler', () => { `); }); - it('does not populate the rule.url when the alert id is set to undefined', async () => { + it('does not populate the rule.url when the rule id is set to undefined', async () => { const execParams = { ...defaultExecutionParams, rule: ruleWithUrl, diff --git a/x-pack/plugins/alerting/server/task_runner/execution_handler.ts b/x-pack/plugins/alerting/server/task_runner/execution_handler.ts index aefd0b0c9dfff..c6836f35115dd 100644 --- a/x-pack/plugins/alerting/server/task_runner/execution_handler.ts +++ b/x-pack/plugins/alerting/server/task_runner/execution_handler.ts @@ -297,6 +297,9 @@ export class ExecutionHandler< return ruleUrl.toString(); } catch (error) { + this.logger.debug( + `Rule "${this.rule.id}" encountered an error while constructing the rule.url variable: ${error.message}` + ); return; } } From ae4a93e4e02760e255658130a49f457e887bc77c Mon Sep 17 00:00:00 2001 From: Jonathan Buttner Date: Tue, 15 Nov 2022 12:16:55 -0500 Subject: [PATCH 5/7] Removing unneeded tests --- packages/kbn-rule-data-utils/src/paths.ts | 4 +- .../task_runner/execution_handler.test.ts | 92 +------------------ .../server/task_runner/execution_handler.ts | 6 +- 3 files changed, 6 insertions(+), 96 deletions(-) diff --git a/packages/kbn-rule-data-utils/src/paths.ts b/packages/kbn-rule-data-utils/src/paths.ts index 8a4623f9c6c41..49ba239829b24 100644 --- a/packages/kbn-rule-data-utils/src/paths.ts +++ b/packages/kbn-rule-data-utils/src/paths.ts @@ -6,7 +6,7 @@ * Side Public License, v 1. */ -export const ruleDetailsRoute = '/rule/:ruleId'; -export const triggersActionsRoute = '/app/management/insightsAndAlerting/triggersActions'; +export const ruleDetailsRoute = '/rule/:ruleId' as const; +export const triggersActionsRoute = '/app/management/insightsAndAlerting/triggersActions' as const; export const getRuleDetailsRoute = (ruleId: string) => ruleDetailsRoute.replace(':ruleId', ruleId); diff --git a/x-pack/plugins/alerting/server/task_runner/execution_handler.test.ts b/x-pack/plugins/alerting/server/task_runner/execution_handler.test.ts index 9e7d9859b0553..100971dbd848d 100644 --- a/x-pack/plugins/alerting/server/task_runner/execution_handler.test.ts +++ b/x-pack/plugins/alerting/server/task_runner/execution_handler.test.ts @@ -747,7 +747,7 @@ describe('Execution Handler', () => { ], } as unknown as SanitizedRule; - it('populates the rule.url in the action params when the base url and alert id are specified', async () => { + it('populates the rule.url in the action params when the base url and rule id are specified', async () => { const execParams = { ...defaultExecutionParams, rule: ruleWithUrl, @@ -828,96 +828,6 @@ describe('Execution Handler', () => { `); }); - it('does not populate the rule.url when the alert id is not defined', async () => { - const execParams = { - ...defaultExecutionParams, - rule: ruleWithUrl, - taskRunnerContext: { - ...defaultExecutionParams.taskRunnerContext, - kibanaBaseUrl: 'http://localhost:12345', - }, - taskInstance: { - params: { spaceId: 'test1' }, - } as unknown as ConcreteTaskInstance, - }; - - const executionHandler = new ExecutionHandler(generateExecutionParams(execParams)); - await executionHandler.run(generateAlert({ id: 1 })); - - expect(injectActionParamsMock.mock.calls[0]).toMatchInlineSnapshot(` - Array [ - Object { - "actionParams": Object { - "val": "rule url: ", - }, - "actionTypeId": "test", - "ruleId": undefined, - "spaceId": "test1", - }, - ] - `); - }); - - it('does not populate the rule.url when the rule id is set to undefined', async () => { - const execParams = { - ...defaultExecutionParams, - rule: ruleWithUrl, - taskRunnerContext: { - ...defaultExecutionParams.taskRunnerContext, - kibanaBaseUrl: 'http://localhost:12345', - }, - taskInstance: { - params: { spaceId: 'test1', alertId: undefined }, - } as unknown as ConcreteTaskInstance, - }; - - const executionHandler = new ExecutionHandler(generateExecutionParams(execParams)); - await executionHandler.run(generateAlert({ id: 1 })); - - expect(injectActionParamsMock.mock.calls[0]).toMatchInlineSnapshot(` - Array [ - Object { - "actionParams": Object { - "val": "rule url: ", - }, - "actionTypeId": "test", - "ruleId": undefined, - "spaceId": "test1", - }, - ] - `); - }); - - it('does not populate the rule.url when the alert id is a number', async () => { - const execParams = { - ...defaultExecutionParams, - rule: ruleWithUrl, - taskRunnerContext: { - ...defaultExecutionParams.taskRunnerContext, - kibanaBaseUrl: 'http://localhost:12345', - }, - taskInstance: { - params: { spaceId: 'test1', alertId: 1 }, - } as unknown as ConcreteTaskInstance, - }; - - const executionHandler = new ExecutionHandler(generateExecutionParams(execParams)); - await executionHandler.run(generateAlert({ id: 1 })); - - expect(injectActionParamsMock.mock.calls[0]).toMatchInlineSnapshot(` - Array [ - Object { - "actionParams": Object { - "val": "rule url: ", - }, - "actionTypeId": "test", - "ruleId": 1, - "spaceId": "test1", - }, - ] - `); - }); - it('does not populate the rule.url when base url is not a valid url', async () => { const execParams = { ...defaultExecutionParams, diff --git a/x-pack/plugins/alerting/server/task_runner/execution_handler.ts b/x-pack/plugins/alerting/server/task_runner/execution_handler.ts index c6836f35115dd..43246a185bd8d 100644 --- a/x-pack/plugins/alerting/server/task_runner/execution_handler.ts +++ b/x-pack/plugins/alerting/server/task_runner/execution_handler.ts @@ -12,7 +12,7 @@ import { asSavedObjectExecutionSource } from '@kbn/actions-plugin/server'; import { isEphemeralTaskRejectedDueToCapacityError } from '@kbn/task-manager-plugin/server'; import { ExecuteOptions as EnqueueExecutionOptions } from '@kbn/actions-plugin/server/create_execute_function'; import { ActionsClient } from '@kbn/actions-plugin/server/actions_client'; -import { chunk, isString } from 'lodash'; +import { chunk } from 'lodash'; import { AlertingEventLogger } from '../lib/alerting_event_logger/alerting_event_logger'; import { RawRule } from '../types'; import { RuleRunMetricsStore } from '../lib/rule_run_metrics_store'; @@ -285,13 +285,13 @@ export class ExecutionHandler< } private buildRuleUrl(): string | undefined { - if (!this.taskRunnerContext.kibanaBaseUrl || !isString(this.taskInstance.params.alertId)) { + if (!this.taskRunnerContext.kibanaBaseUrl) { return; } try { const ruleUrl = new URL( - `${triggersActionsRoute}${getRuleDetailsRoute(this.taskInstance.params.alertId)}`, + `${triggersActionsRoute}${getRuleDetailsRoute(this.rule.id)}`, this.taskRunnerContext.kibanaBaseUrl ); From b0a1c95f15a401dfde3393344eb98606f85a1e8c Mon Sep 17 00:00:00 2001 From: Xavier Mouligneau Date: Tue, 15 Nov 2022 15:51:09 -0500 Subject: [PATCH 6/7] spaces are back --- packages/kbn-rule-data-utils/index.ts | 2 +- .../src/{paths.ts => routes/stack_rule_paths.ts} | 0 .../alerting/server/task_runner/execution_handler.ts | 8 +++++--- 3 files changed, 6 insertions(+), 4 deletions(-) rename packages/kbn-rule-data-utils/src/{paths.ts => routes/stack_rule_paths.ts} (100%) diff --git a/packages/kbn-rule-data-utils/index.ts b/packages/kbn-rule-data-utils/index.ts index 8c9fe22f6ff80..897e5609a8347 100644 --- a/packages/kbn-rule-data-utils/index.ts +++ b/packages/kbn-rule-data-utils/index.ts @@ -10,4 +10,4 @@ export * from './src/technical_field_names'; export * from './src/alerts_as_data_rbac'; export * from './src/alerts_as_data_severity'; export * from './src/alerts_as_data_status'; -export * from './src/paths'; +export * from './src/routes/stack_rule_paths'; diff --git a/packages/kbn-rule-data-utils/src/paths.ts b/packages/kbn-rule-data-utils/src/routes/stack_rule_paths.ts similarity index 100% rename from packages/kbn-rule-data-utils/src/paths.ts rename to packages/kbn-rule-data-utils/src/routes/stack_rule_paths.ts diff --git a/x-pack/plugins/alerting/server/task_runner/execution_handler.ts b/x-pack/plugins/alerting/server/task_runner/execution_handler.ts index 43246a185bd8d..7c993879779e4 100644 --- a/x-pack/plugins/alerting/server/task_runner/execution_handler.ts +++ b/x-pack/plugins/alerting/server/task_runner/execution_handler.ts @@ -209,7 +209,7 @@ export class ExecutionHandler< kibanaBaseUrl: this.taskRunnerContext.kibanaBaseUrl, alertParams: this.rule.params, actionParams: action.params, - ruleUrl: this.buildRuleUrl(), + ruleUrl: this.buildRuleUrl(spaceId), }), }), }; @@ -284,14 +284,16 @@ export class ExecutionHandler< return executables; } - private buildRuleUrl(): string | undefined { + private buildRuleUrl(spaceId: string): string | undefined { if (!this.taskRunnerContext.kibanaBaseUrl) { return; } try { const ruleUrl = new URL( - `${triggersActionsRoute}${getRuleDetailsRoute(this.rule.id)}`, + `${ + spaceId !== 'default' ? `/s/${spaceId}` : '' + }${triggersActionsRoute}${getRuleDetailsRoute(this.rule.id)}`, this.taskRunnerContext.kibanaBaseUrl ); From bccf5ae389c63544e308782e5498c1e91a008811 Mon Sep 17 00:00:00 2001 From: Jonathan Buttner Date: Tue, 15 Nov 2022 16:42:23 -0500 Subject: [PATCH 7/7] Fixing tests --- .../task_runner/execution_handler.test.ts | 34 +++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/alerting/server/task_runner/execution_handler.test.ts b/x-pack/plugins/alerting/server/task_runner/execution_handler.test.ts index 100971dbd848d..7a01338108e7c 100644 --- a/x-pack/plugins/alerting/server/task_runner/execution_handler.test.ts +++ b/x-pack/plugins/alerting/server/task_runner/execution_handler.test.ts @@ -764,7 +764,7 @@ describe('Execution Handler', () => { Array [ Object { "actionParams": Object { - "val": "rule url: http://localhost:12345/app/management/insightsAndAlerting/triggersActions/rule/1", + "val": "rule url: http://localhost:12345/s/test1/app/management/insightsAndAlerting/triggersActions/rule/1", }, "actionTypeId": "test", "ruleId": "1", @@ -774,6 +774,36 @@ describe('Execution Handler', () => { `); }); + it('populates the rule.url without the space specifier when the spaceId is the string "default"', async () => { + const execParams = { + ...defaultExecutionParams, + rule: ruleWithUrl, + taskRunnerContext: { + ...defaultExecutionParams.taskRunnerContext, + kibanaBaseUrl: 'http://localhost:12345', + }, + taskInstance: { + params: { spaceId: 'default', alertId: '1' }, + } as unknown as ConcreteTaskInstance, + }; + + const executionHandler = new ExecutionHandler(generateExecutionParams(execParams)); + await executionHandler.run(generateAlert({ id: 1 })); + + expect(injectActionParamsMock.mock.calls[0]).toMatchInlineSnapshot(` + Array [ + Object { + "actionParams": Object { + "val": "rule url: http://localhost:12345/app/management/insightsAndAlerting/triggersActions/rule/1", + }, + "actionTypeId": "test", + "ruleId": "1", + "spaceId": "default", + }, + ] + `); + }); + it('populates the rule.url in the action params when the base url has a trailing slash and removes the trailing slash', async () => { const execParams = { ...defaultExecutionParams, @@ -791,7 +821,7 @@ describe('Execution Handler', () => { Array [ Object { "actionParams": Object { - "val": "rule url: http://localhost:12345/app/management/insightsAndAlerting/triggersActions/rule/1", + "val": "rule url: http://localhost:12345/s/test1/app/management/insightsAndAlerting/triggersActions/rule/1", }, "actionTypeId": "test", "ruleId": "1",