From 9aca526de665b8670f8032ce71b911822cb89083 Mon Sep 17 00:00:00 2001 From: jennypavlova Date: Thu, 21 Mar 2024 17:55:51 +0100 Subject: [PATCH 1/7] [APM] Add small fixes to APM service overview dashboards tab (#179084) ## Summary While working on [#176070](https://github.com/elastic/kibana/issues/176070) I found some small things we can improve/fix in the APM service overview dashboards tab. This PR: - fixes typos - adds missing translation - improves loading state when linking a dashboard ( disables the cancel button and puts the save button in the loading state) https://github.com/elastic/kibana/assets/14139027/7790841c-4ade-46fc-a9f1-0239c0e385cd --- .../actions/goto_dashboard.tsx | 2 +- .../actions/link_dashboard.tsx | 2 +- .../actions/save_dashboard_modal.tsx | 38 ++++++++++--------- .../actions/unlink_dashboard.tsx | 2 +- .../app/service_dashboards/context_menu.tsx | 14 +++++-- .../service_dashboards/dashboard_selector.tsx | 4 +- .../app/service_dashboards/index.tsx | 8 ++-- 7 files changed, 40 insertions(+), 30 deletions(-) diff --git a/x-pack/plugins/observability_solution/apm/public/components/app/service_dashboards/actions/goto_dashboard.tsx b/x-pack/plugins/observability_solution/apm/public/components/app/service_dashboards/actions/goto_dashboard.tsx index f196077c41a4d..1d1c923e6a597 100644 --- a/x-pack/plugins/observability_solution/apm/public/components/app/service_dashboards/actions/goto_dashboard.tsx +++ b/x-pack/plugins/observability_solution/apm/public/components/app/service_dashboards/actions/goto_dashboard.tsx @@ -30,7 +30,7 @@ export function GotoDashboard({ data-test-subj="apmGotoDashboardGoToDashboardButton" color="text" size="s" - iconType={'visGauge'} + iconType="visGauge" href={url} > {i18n.translate('xpack.apm.serviceDashboards.contextMenu.goToDashboard', { diff --git a/x-pack/plugins/observability_solution/apm/public/components/app/service_dashboards/actions/link_dashboard.tsx b/x-pack/plugins/observability_solution/apm/public/components/app/service_dashboards/actions/link_dashboard.tsx index 0db2654c1d66b..03dc390fc93ce 100644 --- a/x-pack/plugins/observability_solution/apm/public/components/app/service_dashboards/actions/link_dashboard.tsx +++ b/x-pack/plugins/observability_solution/apm/public/components/app/service_dashboards/actions/link_dashboard.tsx @@ -29,7 +29,7 @@ export function LinkDashboard({ setIsModalVisible(true)} > diff --git a/x-pack/plugins/observability_solution/apm/public/components/app/service_dashboards/actions/save_dashboard_modal.tsx b/x-pack/plugins/observability_solution/apm/public/components/app/service_dashboards/actions/save_dashboard_modal.tsx index 6872b0d2cc805..406a1b3e43190 100644 --- a/x-pack/plugins/observability_solution/apm/public/components/app/service_dashboards/actions/save_dashboard_modal.tsx +++ b/x-pack/plugins/observability_solution/apm/public/components/app/service_dashboards/actions/save_dashboard_modal.tsx @@ -53,29 +53,29 @@ export function SaveDashboardModal({ const { data: allAvailableDashboards, status } = useDashboardFetcher(); const history = useHistory(); - let defaultOption: EuiComboBoxOptionOption | undefined; + const [isLoading, setIsLoading] = useState(false); - const [serviceFiltersEnabled, setserviceFiltersEnabled] = useState( + const [serviceFiltersEnabled, setServiceFiltersEnabled] = useState( (currentDashboard?.serviceEnvironmentFilterEnabled && currentDashboard?.serviceNameFilterEnabled) ?? true ); - if (currentDashboard) { - const { title, dashboardSavedObjectId } = currentDashboard; - defaultOption = { label: title, value: dashboardSavedObjectId }; - } - - const [selectedDashboard, setSelectedDashboard] = useState( - defaultOption ? [defaultOption] : [] + const [selectedDashboard, setSelectedDashboard] = useState< + Array> + >( + currentDashboard + ? [ + { + label: currentDashboard.title, + value: currentDashboard.dashboardSavedObjectId, + }, + ] + : [] ); const isEditMode = !!currentDashboard?.id; - const reloadCustomDashboards = useCallback(() => { - onRefresh(); - }, [onRefresh]); - const options = allAvailableDashboards?.map( (dashboardItem: DashboardItem) => ({ label: dashboardItem.attributes.title, @@ -92,6 +92,7 @@ export function SaveDashboardModal({ const [newDashboard] = selectedDashboard; try { if (newDashboard.value) { + setIsLoading(true); await callApmApi('POST /internal/apm/custom-dashboard', { params: { query: { customDashboardId: currentDashboard?.id }, @@ -117,7 +118,7 @@ export function SaveDashboardModal({ dashboardId: newDashboard.value, }), }); - reloadCustomDashboards(); + onRefresh(); } } catch (error) { console.error(error); @@ -132,6 +133,7 @@ export function SaveDashboardModal({ text: error.body.message, }); } + setIsLoading(false); onClose(); }, [ @@ -139,7 +141,7 @@ export function SaveDashboardModal({ notifications.toasts, serviceFiltersEnabled, onClose, - reloadCustomDashboards, + onRefresh, isEditMode, serviceName, currentDashboard, @@ -170,7 +172,7 @@ export function SaveDashboardModal({

} - onChange={() => setserviceFiltersEnabled(!serviceFiltersEnabled)} + onChange={() => setServiceFiltersEnabled(!serviceFiltersEnabled)} checked={serviceFiltersEnabled} /> @@ -220,6 +222,7 @@ export function SaveDashboardModal({ {i18n.translate( 'xpack.apm.serviceDashboards.selectDashboard.cancel', @@ -231,6 +234,7 @@ export function SaveDashboardModal({ {isEditMode diff --git a/x-pack/plugins/observability_solution/apm/public/components/app/service_dashboards/actions/unlink_dashboard.tsx b/x-pack/plugins/observability_solution/apm/public/components/app/service_dashboards/actions/unlink_dashboard.tsx index c43d3d289b767..3e68e8f54ebef 100644 --- a/x-pack/plugins/observability_solution/apm/public/components/app/service_dashboards/actions/unlink_dashboard.tsx +++ b/x-pack/plugins/observability_solution/apm/public/components/app/service_dashboards/actions/unlink_dashboard.tsx @@ -85,7 +85,7 @@ export function UnlinkDashboard({ setIsModalVisible(true)} > diff --git a/x-pack/plugins/observability_solution/apm/public/components/app/service_dashboards/context_menu.tsx b/x-pack/plugins/observability_solution/apm/public/components/app/service_dashboards/context_menu.tsx index 2eb48b7f66848..bf7da532054bf 100644 --- a/x-pack/plugins/observability_solution/apm/public/components/app/service_dashboards/context_menu.tsx +++ b/x-pack/plugins/observability_solution/apm/public/components/app/service_dashboards/context_menu.tsx @@ -4,7 +4,7 @@ * 2.0; you may not use this file except in compliance with the Elastic License * 2.0. */ - +import { i18n } from '@kbn/i18n'; import React, { useState } from 'react'; import { EuiButtonIcon, @@ -36,7 +36,10 @@ export function ContextMenu({ items }: Props) { display="base" size="s" iconType="boxesVertical" - aria-label="More" + aria-label={i18n.translate( + 'xpack.apm.serviceDashboards.contextMenu.moreLabel', + { defaultMessage: 'More' } + )} onClick={onButtonClick} /> } @@ -47,8 +50,11 @@ export function ContextMenu({ items }: Props) { > ( - {item} + items={items.map((item, index) => ( + + {' '} + {item} + ))} /> diff --git a/x-pack/plugins/observability_solution/apm/public/components/app/service_dashboards/dashboard_selector.tsx b/x-pack/plugins/observability_solution/apm/public/components/app/service_dashboards/dashboard_selector.tsx index 7dbcbe714a428..5925340151a7d 100644 --- a/x-pack/plugins/observability_solution/apm/public/components/app/service_dashboards/dashboard_selector.tsx +++ b/x-pack/plugins/observability_solution/apm/public/components/app/service_dashboards/dashboard_selector.tsx @@ -90,8 +90,8 @@ export function DashboardSelector({ selectedDashboard ? [ { - value: selectedDashboard?.dashboardSavedObjectId, - label: selectedDashboard?.title, + value: selectedDashboard.dashboardSavedObjectId, + label: selectedDashboard.title, }, ] : [] diff --git a/x-pack/plugins/observability_solution/apm/public/components/app/service_dashboards/index.tsx b/x-pack/plugins/observability_solution/apm/public/components/app/service_dashboards/index.tsx index 7426ec382b87f..4bfe04df040b3 100644 --- a/x-pack/plugins/observability_solution/apm/public/components/app/service_dashboards/index.tsx +++ b/x-pack/plugins/observability_solution/apm/public/components/app/service_dashboards/index.tsx @@ -83,7 +83,7 @@ export function ServiceDashboards() { ); useEffect(() => { - const filteredServiceDashbords = (data?.serviceDashboards ?? []).reduce( + const filteredServiceDashboards = (data?.serviceDashboards ?? []).reduce( ( result: MergedServiceDashboard[], serviceDashboard: SavedApmCustomDashboard @@ -102,8 +102,8 @@ export function ServiceDashboards() { [] ); - setServiceDashboards(filteredServiceDashbords); - }, [allAvailableDashboards, data?.serviceDashboards]); + setServiceDashboards(filteredServiceDashboards); + }, [allAvailableDashboards, data]); const getCreationOptions = useCallback((): Promise => { @@ -127,7 +127,6 @@ export function ServiceDashboards() { timeRange: { from: rangeFrom, to: rangeTo }, query: { query: kuery, language: 'kuery' }, }); - // eslint-disable-next-line react-hooks/exhaustive-deps }, [ dataView, serviceName, @@ -136,6 +135,7 @@ export function ServiceDashboards() { dashboard, rangeFrom, rangeTo, + currentDashboard, ]); const getLocatorParams = useCallback( From 7ca14caca7d463a3a4e75fe54e4f9635b8d40ed7 Mon Sep 17 00:00:00 2001 From: Milton Hultgren Date: Thu, 21 Mar 2024 18:12:19 +0100 Subject: [PATCH 2/7] [Obs AI Assistant] Fix error in reranking prompt (#179164) --- .../observability_ai_assistant/server/functions/context.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/observability_solution/observability_ai_assistant/server/functions/context.ts b/x-pack/plugins/observability_solution/observability_ai_assistant/server/functions/context.ts index 422c0287e15d7..7db8f63e273f9 100644 --- a/x-pack/plugins/observability_solution/observability_ai_assistant/server/functions/context.ts +++ b/x-pack/plugins/observability_solution/observability_ai_assistant/server/functions/context.ts @@ -216,7 +216,7 @@ async function scoreSuggestions({ const newUserMessageContent = dedent(`Given the following question, score the documents that are relevant to the question. on a scale from 0 to 7, - 0 being completely relevant, and 7 being extremely relevant. Information is relevant to the question if it helps in + 0 being completely irrelevant, and 7 being extremely relevant. Information is relevant to the question if it helps in answering the question. Judge it according to the following criteria: - The document is relevant to the question, and the rest of the conversation From 386c29094df6f189da57b1a7c5ddc839e610e559 Mon Sep 17 00:00:00 2001 From: Pierre Gayvallet Date: Thu, 21 Mar 2024 18:31:58 +0100 Subject: [PATCH 3/7] status service: get rid of `calculateDepthRecursive` (#179160) ## Summary We were spending a lot of time in `calculateDepthRecursive` Screenshot 2024-03-21 at 13 46 00 And it could fully be avoided given the plugin service already sorts the plugins. This PR changes the way the plugin system exposes its `PluginDependencies` so that the keys are topologically ordered (same order we use to setup / start the plugins) and then use that ordering directly from the status service. --- .../src/plugins_system.test.ts | 6 +- .../src/plugins_system.ts | 162 ++++++++++-------- .../core-plugins-server-internal/src/types.ts | 10 ++ .../src/plugins_status.ts | 62 ++----- 4 files changed, 113 insertions(+), 127 deletions(-) diff --git a/packages/core/plugins/core-plugins-server-internal/src/plugins_system.test.ts b/packages/core/plugins/core-plugins-server-internal/src/plugins_system.test.ts index 1ddc4c08f915c..e99ed47ca43f0 100644 --- a/packages/core/plugins/core-plugins-server-internal/src/plugins_system.test.ts +++ b/packages/core/plugins/core-plugins-server-internal/src/plugins_system.test.ts @@ -128,7 +128,7 @@ test('getPlugins returns the list of plugins', () => { expect(pluginsSystem.getPlugins()).toEqual([pluginA, pluginB]); }); -test('getPluginDependencies returns dependency tree of symbols', () => { +test('getPluginDependencies returns dependency tree with keys topologically sorted', () => { pluginsSystem.addPlugin(createPlugin('plugin-a', { required: ['no-dep'] })); pluginsSystem.addPlugin( createPlugin('plugin-b', { required: ['plugin-a'], optional: ['no-dep', 'other'] }) @@ -138,6 +138,7 @@ test('getPluginDependencies returns dependency tree of symbols', () => { expect(pluginsSystem.getPluginDependencies()).toMatchInlineSnapshot(` Object { "asNames": Map { + "no-dep" => Array [], "plugin-a" => Array [ "no-dep", ], @@ -145,9 +146,9 @@ test('getPluginDependencies returns dependency tree of symbols', () => { "plugin-a", "no-dep", ], - "no-dep" => Array [], }, "asOpaqueIds": Map { + Symbol(no-dep) => Array [], Symbol(plugin-a) => Array [ Symbol(no-dep), ], @@ -155,7 +156,6 @@ test('getPluginDependencies returns dependency tree of symbols', () => { Symbol(plugin-a), Symbol(no-dep), ], - Symbol(no-dep) => Array [], }, } `); diff --git a/packages/core/plugins/core-plugins-server-internal/src/plugins_system.ts b/packages/core/plugins/core-plugins-server-internal/src/plugins_system.ts index a47c82f73aca7..a929044cd6037 100644 --- a/packages/core/plugins/core-plugins-server-internal/src/plugins_system.ts +++ b/packages/core/plugins/core-plugins-server-internal/src/plugins_system.ts @@ -34,6 +34,7 @@ export class PluginsSystem { private readonly log: Logger; // `satup`, the past-tense version of the noun `setup`. private readonly satupPlugins: PluginName[] = []; + private sortedPluginNames?: Set; constructor(private readonly coreContext: CoreContext, public readonly type: T) { this.log = coreContext.logger.get('plugins-system', this.type); @@ -47,6 +48,9 @@ export class PluginsSystem { } this.plugins.set(plugin.name, plugin); + + // clear sorted plugin name cache on addition + this.sortedPluginNames = undefined; } public getPlugins() { @@ -54,32 +58,31 @@ export class PluginsSystem { } /** - * @returns a ReadonlyMap of each plugin and an Array of its available dependencies + * @returns a Map of each plugin and an Array of its available dependencies * @internal */ public getPluginDependencies(): PluginDependencies { - const asNames = new Map( - [...this.plugins].map(([name, plugin]) => [ + const asNames = new Map(); + const asOpaqueIds = new Map(); + + for (const pluginName of this.getTopologicallySortedPluginNames()) { + const plugin = this.plugins.get(pluginName)!; + const dependencies = [ + ...new Set([ + ...plugin.requiredPlugins, + ...plugin.optionalPlugins.filter((optPlugin) => this.plugins.has(optPlugin)), + ]), + ]; + + asNames.set( plugin.name, - [ - ...new Set([ - ...plugin.requiredPlugins, - ...plugin.optionalPlugins.filter((optPlugin) => this.plugins.has(optPlugin)), - ]), - ].map((depId) => this.plugins.get(depId)!.name), - ]) - ); - const asOpaqueIds = new Map( - [...this.plugins].map(([name, plugin]) => [ + dependencies.map((depId) => this.plugins.get(depId)!.name) + ); + asOpaqueIds.set( plugin.opaqueId, - [ - ...new Set([ - ...plugin.requiredPlugins, - ...plugin.optionalPlugins.filter((optPlugin) => this.plugins.has(optPlugin)), - ]), - ].map((depId) => this.plugins.get(depId)!.opaqueId), - ]) - ); + dependencies.map((depId) => this.plugins.get(depId)!.opaqueId) + ); + } return { asNames, asOpaqueIds }; } @@ -298,67 +301,74 @@ export class PluginsSystem { return publicPlugins; } - /** - * Gets topologically sorted plugin names that are registered with the plugin system. - * Ordering is possible if and only if the plugins graph has no directed cycles, - * that is, if it is a directed acyclic graph (DAG). If plugins cannot be ordered - * an error is thrown. - * - * Uses Kahn's Algorithm to sort the graph. - */ private getTopologicallySortedPluginNames() { - // We clone plugins so we can remove handled nodes while we perform the - // topological ordering. If the cloned graph is _not_ empty at the end, we - // know we were not able to topologically order the graph. We exclude optional - // dependencies that are not present in the plugins graph. - const pluginsDependenciesGraph = new Map( - [...this.plugins.entries()].map(([pluginName, plugin]) => { - return [ - pluginName, - new Set([ - ...plugin.requiredPlugins, - ...plugin.optionalPlugins.filter((dependency) => this.plugins.has(dependency)), - ]), - ] as [PluginName, Set]; - }) - ); - - // First, find a list of "start nodes" which have no outgoing edges. At least - // one such node must exist in a non-empty acyclic graph. - const pluginsWithAllDependenciesSorted = [...pluginsDependenciesGraph.keys()].filter( - (pluginName) => pluginsDependenciesGraph.get(pluginName)!.size === 0 - ); - - const sortedPluginNames = new Set(); - while (pluginsWithAllDependenciesSorted.length > 0) { - const sortedPluginName = pluginsWithAllDependenciesSorted.pop()!; - - // We know this plugin has all its dependencies sorted, so we can remove it - // and include into the final result. - pluginsDependenciesGraph.delete(sortedPluginName); - sortedPluginNames.add(sortedPluginName); - - // Go through the rest of the plugins and remove `sortedPluginName` from their - // unsorted dependencies. - for (const [pluginName, dependencies] of pluginsDependenciesGraph) { - // If we managed delete `sortedPluginName` from dependencies let's check - // whether it was the last one and we can mark plugin as sorted. - if (dependencies.delete(sortedPluginName) && dependencies.size === 0) { - pluginsWithAllDependenciesSorted.push(pluginName); - } - } + if (!this.sortedPluginNames) { + this.sortedPluginNames = getTopologicallySortedPluginNames(this.plugins); } + return this.sortedPluginNames; + } +} - if (pluginsDependenciesGraph.size > 0) { - const edgesLeft = JSON.stringify([...pluginsDependenciesGraph.keys()]); - throw new Error( - `Topological ordering of plugins did not complete, these plugins have cyclic or missing dependencies: ${edgesLeft}` - ); +/** + * Gets topologically sorted plugin names that are registered with the plugin system. + * Ordering is possible if and only if the plugins graph has no directed cycles, + * that is, if it is a directed acyclic graph (DAG). If plugins cannot be ordered + * an error is thrown. + * + * Uses Kahn's Algorithm to sort the graph. + */ +const getTopologicallySortedPluginNames = (plugins: Map) => { + // We clone plugins so we can remove handled nodes while we perform the + // topological ordering. If the cloned graph is _not_ empty at the end, we + // know we were not able to topologically order the graph. We exclude optional + // dependencies that are not present in the plugins graph. + const pluginsDependenciesGraph = new Map( + [...plugins.entries()].map(([pluginName, plugin]) => { + return [ + pluginName, + new Set([ + ...plugin.requiredPlugins, + ...plugin.optionalPlugins.filter((dependency) => plugins.has(dependency)), + ]), + ] as [PluginName, Set]; + }) + ); + + // First, find a list of "start nodes" which have no outgoing edges. At least + // one such node must exist in a non-empty acyclic graph. + const pluginsWithAllDependenciesSorted = [...pluginsDependenciesGraph.keys()].filter( + (pluginName) => pluginsDependenciesGraph.get(pluginName)!.size === 0 + ); + + const sortedPluginNames = new Set(); + while (pluginsWithAllDependenciesSorted.length > 0) { + const sortedPluginName = pluginsWithAllDependenciesSorted.pop()!; + + // We know this plugin has all its dependencies sorted, so we can remove it + // and include into the final result. + pluginsDependenciesGraph.delete(sortedPluginName); + sortedPluginNames.add(sortedPluginName); + + // Go through the rest of the plugins and remove `sortedPluginName` from their + // unsorted dependencies. + for (const [pluginName, dependencies] of pluginsDependenciesGraph) { + // If we managed delete `sortedPluginName` from dependencies let's check + // whether it was the last one and we can mark plugin as sorted. + if (dependencies.delete(sortedPluginName) && dependencies.size === 0) { + pluginsWithAllDependenciesSorted.push(pluginName); + } } + } - return sortedPluginNames; + if (pluginsDependenciesGraph.size > 0) { + const edgesLeft = JSON.stringify([...pluginsDependenciesGraph.keys()]); + throw new Error( + `Topological ordering of plugins did not complete, these plugins have cyclic or missing dependencies: ${edgesLeft}` + ); } -} + + return sortedPluginNames; +}; const buildReverseDependencyMap = ( pluginMap: Map diff --git a/packages/core/plugins/core-plugins-server-internal/src/types.ts b/packages/core/plugins/core-plugins-server-internal/src/types.ts index def1a27a4c26f..195b4910f71dd 100644 --- a/packages/core/plugins/core-plugins-server-internal/src/types.ts +++ b/packages/core/plugins/core-plugins-server-internal/src/types.ts @@ -10,6 +10,16 @@ import type { PluginName, PluginOpaqueId } from '@kbn/core-base-common'; /** @internal */ export interface PluginDependencies { + /** + * Plugin to dependencies map with plugin names as key/values. + * + * Keys sorted by plugin topological order (root plugins first, leaf plugins last). + */ asNames: ReadonlyMap; + /** + * Plugin to dependencies map with plugin opaque ids as key/values. + * + * Keys sorted by plugin topological order (root plugins first, leaf plugins last). + */ asOpaqueIds: ReadonlyMap; } diff --git a/packages/core/status/core-status-server-internal/src/plugins_status.ts b/packages/core/status/core-status-server-internal/src/plugins_status.ts index 7de22e23cc97e..a34fb8a98d794 100644 --- a/packages/core/status/core-status-server-internal/src/plugins_status.ts +++ b/packages/core/status/core-status-server-internal/src/plugins_status.ts @@ -23,7 +23,6 @@ import { takeUntil, delay, } from 'rxjs/operators'; -import { sortBy } from 'lodash'; import { isDeepStrictEqual } from 'util'; import type { PluginName } from '@kbn/core-base-common'; import { ServiceStatusLevels, type CoreStatus, type ServiceStatus } from '@kbn/core-status-common'; @@ -45,7 +44,6 @@ export interface Deps { interface PluginData { [name: PluginName]: { name: PluginName; - depth: number; // depth of this plugin in the dependency tree (root plugins will have depth = 1) dependencies: PluginName[]; reverseDependencies: PluginName[]; reportedStatus?: PluginStatus; @@ -81,7 +79,8 @@ export class PluginsStatusService { constructor(deps: Deps, private readonly statusTimeoutMs: number = STATUS_TIMEOUT_MS) { this.pluginData = this.initPluginData(deps.pluginDependencies); this.rootPlugins = this.getRootPlugins(); - this.orderedPluginNames = this.getOrderedPluginNames(); + // plugin dependencies keys are already sorted + this.orderedPluginNames = [...deps.pluginDependencies.keys()]; this.coreSubscription = deps.core$ .pipe( @@ -216,23 +215,20 @@ export class PluginsStatusService { private initPluginData(pluginDependencies: ReadonlyMap): PluginData { const pluginData: PluginData = {}; - if (pluginDependencies) { - pluginDependencies.forEach((dependencies, name) => { - pluginData[name] = { - name, - depth: 0, - dependencies, - reverseDependencies: [], - derivedStatus: defaultStatus, - }; - }); + pluginDependencies.forEach((dependencies, name) => { + pluginData[name] = { + name, + dependencies, + reverseDependencies: [], + derivedStatus: defaultStatus, + }; + }); - pluginDependencies.forEach((dependencies, name) => { - dependencies.forEach((dependency) => { - pluginData[dependency].reverseDependencies.push(name); - }); + pluginDependencies.forEach((dependencies, name) => { + dependencies.forEach((dependency) => { + pluginData[dependency].reverseDependencies.push(name); }); - } + }); return pluginData; } @@ -248,36 +244,6 @@ export class PluginsStatusService { ); } - /** - * Obtain a list of plugins names, ordered by depth. - * @see {calculateDepthRecursive} - * @returns {PluginName[]} a list of plugins, ordered by depth + name - */ - private getOrderedPluginNames(): PluginName[] { - this.rootPlugins.forEach((plugin) => { - this.calculateDepthRecursive(plugin, 1); - }); - - return sortBy(Object.values(this.pluginData), ['depth', 'name']).map(({ name }) => name); - } - - /** - * Calculate the depth of the given plugin, knowing that it's has at least the specified depth - * The depth of a plugin is determined by how many levels of dependencies the plugin has above it. - * We define root plugins as depth = 1, plugins that only depend on root plugins will have depth = 2 - * and so on so forth - * @param {PluginName} plugin the name of the plugin whose depth must be calculated - * @param {number} depth the minimum depth that we know for sure this plugin has - */ - private calculateDepthRecursive(plugin: PluginName, depth: number): void { - const pluginData = this.pluginData[plugin]; - pluginData.depth = Math.max(pluginData.depth, depth); - const newDepth = depth + 1; - pluginData.reverseDependencies.forEach((revDep) => - this.calculateDepthRecursive(revDep, newDepth) - ); - } - /** * Updates the root plugins statuses according to the current core services status */ From 771d97e8b95813672edeb0c518b25ba99783343a Mon Sep 17 00:00:00 2001 From: Ramon Butter Date: Thu, 21 Mar 2024 18:39:47 +0100 Subject: [PATCH 4/7] deepmerge in merging apm-configuration (#179048) We faced an issue where APM global labels sourced from the kibana configuration were overriden when `ELASTIC_APM_GLOBAL_LABELS` was set in the run-time. I suspect this is due to the fact hat the use a shallow merge at this point, and the labels are nested inside both configuration ( `configFromKibanaConfig`, and `configFromEnv` ). ``` config = Object { ... "globalLabels": Object { .... }, ... } ``` Didn't set up my IDE to actually test this. But added a unit test which will hopefully give some feedback --------- Co-authored-by: Jean-Louis Leysens --- .../kbn-apm-config-loader/src/config.test.ts | 20 +++++++++++++++++-- packages/kbn-apm-config-loader/src/config.ts | 6 +++++- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/packages/kbn-apm-config-loader/src/config.test.ts b/packages/kbn-apm-config-loader/src/config.test.ts index ac139b220768c..caf3b800b444b 100644 --- a/packages/kbn-apm-config-loader/src/config.test.ts +++ b/packages/kbn-apm-config-loader/src/config.test.ts @@ -7,9 +7,9 @@ */ import type { AgentConfigOptions, Labels } from 'elastic-apm-node'; import { - packageMock, - mockedRootDir, gitRevExecMock, + mockedRootDir, + packageMock, readUuidFileMock, resetAllMocks, } from './config.test.mocks'; @@ -152,6 +152,7 @@ describe('ApmConfiguration', () => { delete process.env.ELASTIC_APM_SECRET_TOKEN; delete process.env.ELASTIC_APM_API_KEY; delete process.env.ELASTIC_APM_SERVER_URL; + delete process.env.ELASTIC_APM_GLOBAL_LABELS; delete process.env.NODE_ENV; }); @@ -184,6 +185,21 @@ describe('ApmConfiguration', () => { }) ); }); + + it('ELASTIC_APM_GLOBAL_LABELS', () => { + process.env.ELASTIC_APM_GLOBAL_LABELS = 'test1=1,test2=2'; + const config = new ApmConfiguration(mockedRootDir, {}, true); + + expect(config.getConfig('serviceName')).toEqual( + expect.objectContaining({ + globalLabels: { + git_rev: 'sha', + test1: '1', + test2: '2', + }, + }) + ); + }); }); it('does not override the environment from NODE_ENV if already set in the config file', () => { diff --git a/packages/kbn-apm-config-loader/src/config.ts b/packages/kbn-apm-config-loader/src/config.ts index f25feafe90eb7..efe228be8157e 100644 --- a/packages/kbn-apm-config-loader/src/config.ts +++ b/packages/kbn-apm-config-loader/src/config.ts @@ -7,6 +7,7 @@ */ import { join } from 'path'; +import deepmerge from 'deepmerge'; import { merge } from 'lodash'; import { execSync } from 'child_process'; import { getDataPath } from '@kbn/utils'; @@ -295,7 +296,10 @@ export class ApmConfiguration { const { servicesOverrides, redactUsers, ...configFromKibanaConfig } = this.getConfigFromKibanaConfig(); const configFromEnv = this.getConfigFromEnv(configFromKibanaConfig); - const config = merge({}, configFromKibanaConfig, configFromEnv); + const config = [configFromKibanaConfig, configFromEnv].reduce( + (acc, conf) => deepmerge(acc, conf), + {} + ); if (config.active === false && config.contextPropagationOnly !== false) { throw new Error( From 4d51cad82fdbbe5b958c51d12d9070db8a023a1b Mon Sep 17 00:00:00 2001 From: amyjtechwriter <61687663+amyjtechwriter@users.noreply.github.com> Date: Thu, 21 Mar 2024 17:40:52 +0000 Subject: [PATCH 5/7] [DOCS] Discover troubleshooting blog link (#179109) Request to link from Discover page to a blog [troubleshooting guide for Discover](https://www.elastic.co/blog/troubleshooting-guide-common-issues-kibana-discover-load). I've put it under a Troubleshooting heading on the [Discover index page](https://www.elastic.co/guide/en/kibana/current/discover.html), but if you think it would be better included in the What's next? section just let me know and I'll move it. Screenshot 2024-03-20 at 20 18 45 Relates to:#178046 --- docs/user/discover.asciidoc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/user/discover.asciidoc b/docs/user/discover.asciidoc index d60ccb2ccd0de..ddd06b06c9cd8 100644 --- a/docs/user/discover.asciidoc +++ b/docs/user/discover.asciidoc @@ -334,6 +334,11 @@ For more about this and other rules provided in {alert-features}, go to <> to better meet your needs. +[float] +=== Troubleshooting + +* {blog-ref}troubleshooting-guide-common-issues-kibana-discover-load[Learn how to resolve common issues with Discover.] + -- include::{kibana-root}/docs/discover/document-explorer.asciidoc[] From 3be6e23ae756971011414f8ac33f0d75ad9efdea Mon Sep 17 00:00:00 2001 From: Gloria Hornero Date: Thu, 21 Mar 2024 19:39:58 +0100 Subject: [PATCH 6/7] [Security Solution] [Cypress] Changes the way to delete all the timelines (#178935) ## Summary After investigating the failure of the test `should save timelines as new` inside `x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/creation.cy.ts` spec file in MKI environment, we found out the issue was that the timelines created by the previous tests were not deleted. That was the reason why the assertion `cy.get(ROWS).should('have.length', '2');` fails. `deleteTimelines()` was not failing in Cypress, but when we executed the query using the developer tools on an MKI environment, we were getting a 403 because we were trying to access an internal index, something that is completely forbidden in MKI environments. The reason why this query was not failing in Cypress, is because we are using a pattern doing so at the developer tools, does not throw any error, but targeting a specific index does. To solve the issue around the timelines not being deleted, we are using our APIs. --- .../sourcerer/sourcerer_timeline.cy.ts | 3 +-- .../e2e/explore/cases/attach_timeline.cy.ts | 3 +-- .../cypress/e2e/explore/cases/creation.cy.ts | 3 +-- .../alerts/ransomware_detection.cy.ts | 2 +- .../alerts/ransomware_prevention.cy.ts | 2 +- .../timeline_templates/creation.cy.ts | 2 +- .../timeline_templates/export.cy.ts | 3 +-- .../timelines/correlation_tab.cy.ts | 2 +- .../investigations/timelines/creation.cy.ts | 3 +-- .../timelines/esql/search_filter.cy.ts | 4 ++-- .../e2e/investigations/timelines/export.cy.ts | 2 +- .../investigations/timelines/notes_tab.cy.ts | 4 +--- .../timelines/open_timeline.cy.ts | 2 +- .../investigations/timelines/overview.cy.tsx | 7 ++++-- .../investigations/timelines/query_tab.cy.ts | 2 +- .../timelines/row_renderers.cy.ts | 2 +- .../timelines/search_or_filter.cy.ts | 2 +- .../investigations/timelines/url_state.cy.ts | 2 +- .../cypress/tasks/api_calls/common.ts | 21 ---------------- .../cypress/tasks/api_calls/timelines.ts | 24 ++++++++++++++++++- 20 files changed, 46 insertions(+), 49 deletions(-) diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/sourcerer/sourcerer_timeline.cy.ts b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/sourcerer/sourcerer_timeline.cy.ts index f720d11689d1a..bb83d4cfbe007 100644 --- a/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/sourcerer/sourcerer_timeline.cy.ts +++ b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/sourcerer/sourcerer_timeline.cy.ts @@ -10,7 +10,6 @@ import { DEFAULT_INDEX_PATTERN, } from '@kbn/security-solution-plugin/common/constants'; -import { deleteTimelines } from '../../../../tasks/api_calls/common'; import { login } from '../../../../tasks/login'; import { visitWithTimeRange } from '../../../../tasks/navigation'; @@ -33,7 +32,7 @@ import { } from '../../../../tasks/sourcerer'; import { openTimelineUsingToggle } from '../../../../tasks/security_main'; import { SOURCERER } from '../../../../screens/sourcerer'; -import { createTimeline } from '../../../../tasks/api_calls/timelines'; +import { createTimeline, deleteTimelines } from '../../../../tasks/api_calls/timelines'; import { getTimeline, getTimelineModifiedSourcerer } from '../../../../objects/timeline'; import { closeTimeline, openTimelineById } from '../../../../tasks/timeline'; diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/explore/cases/attach_timeline.cy.ts b/x-pack/test/security_solution_cypress/cypress/e2e/explore/cases/attach_timeline.cy.ts index 0dfa0620acccb..a56390010b882 100644 --- a/x-pack/test/security_solution_cypress/cypress/e2e/explore/cases/attach_timeline.cy.ts +++ b/x-pack/test/security_solution_cypress/cypress/e2e/explore/cases/attach_timeline.cy.ts @@ -16,8 +16,7 @@ import { import { DESCRIPTION_INPUT, ADD_COMMENT_INPUT } from '../../../screens/create_new_case'; import { getCase1 } from '../../../objects/case'; import { getTimeline } from '../../../objects/timeline'; -import { createTimeline } from '../../../tasks/api_calls/timelines'; -import { deleteTimelines } from '../../../tasks/api_calls/common'; +import { createTimeline, deleteTimelines } from '../../../tasks/api_calls/timelines'; import { createCase, deleteCases } from '../../../tasks/api_calls/cases'; describe('attach timeline to case', { tags: ['@ess', '@serverless'] }, () => { diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/explore/cases/creation.cy.ts b/x-pack/test/security_solution_cypress/cypress/e2e/explore/cases/creation.cy.ts index d76f8e9fb9eec..9da58cc8e406e 100644 --- a/x-pack/test/security_solution_cypress/cypress/e2e/explore/cases/creation.cy.ts +++ b/x-pack/test/security_solution_cypress/cypress/e2e/explore/cases/creation.cy.ts @@ -39,7 +39,7 @@ import { TIMELINE_QUERY, TIMELINE_TITLE } from '../../../screens/timeline'; import { OVERVIEW_CASE_DESCRIPTION, OVERVIEW_CASE_NAME } from '../../../screens/overview'; import { goToCaseDetails, goToCreateNewCase } from '../../../tasks/all_cases'; -import { createTimeline } from '../../../tasks/api_calls/timelines'; +import { createTimeline, deleteTimelines } from '../../../tasks/api_calls/timelines'; import { openCaseTimeline } from '../../../tasks/case_details'; import { attachTimeline, @@ -53,7 +53,6 @@ import { visit, visitWithTimeRange } from '../../../tasks/navigation'; import { CASES_URL, OVERVIEW_URL } from '../../../urls/navigation'; import { ELASTICSEARCH_USERNAME } from '../../../env_var_names_constants'; -import { deleteTimelines } from '../../../tasks/api_calls/common'; import { deleteCases } from '../../../tasks/api_calls/cases'; // Tracked by https://github.com/elastic/security-team/issues/7696 diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/investigations/alerts/ransomware_detection.cy.ts b/x-pack/test/security_solution_cypress/cypress/e2e/investigations/alerts/ransomware_detection.cy.ts index a225d08780166..a4908a1035f39 100644 --- a/x-pack/test/security_solution_cypress/cypress/e2e/investigations/alerts/ransomware_detection.cy.ts +++ b/x-pack/test/security_solution_cypress/cypress/e2e/investigations/alerts/ransomware_detection.cy.ts @@ -14,7 +14,7 @@ import { ALERTS_HISTOGRAM_SERIES, ALERT_RULE_NAME, MESSAGE } from '../../../scre import { TIMELINE_QUERY, TIMELINE_VIEW_IN_ANALYZER } from '../../../screens/timeline'; import { selectAlertsHistogram } from '../../../tasks/alerts'; import { openTimelineUsingToggle } from '../../../tasks/security_main'; -import { deleteTimelines } from '../../../tasks/api_calls/common'; +import { deleteTimelines } from '../../../tasks/api_calls/timelines'; describe('Ransomware Detection Alerts', { tags: ['@ess', '@serverless'] }, () => { before(() => { diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/investigations/alerts/ransomware_prevention.cy.ts b/x-pack/test/security_solution_cypress/cypress/e2e/investigations/alerts/ransomware_prevention.cy.ts index abc00b14c466a..c7078783466e9 100644 --- a/x-pack/test/security_solution_cypress/cypress/e2e/investigations/alerts/ransomware_prevention.cy.ts +++ b/x-pack/test/security_solution_cypress/cypress/e2e/investigations/alerts/ransomware_prevention.cy.ts @@ -12,7 +12,7 @@ import { ALERTS_URL } from '../../../urls/navigation'; import { ALERTS_HISTOGRAM_SERIES, ALERT_RULE_NAME, MESSAGE } from '../../../screens/alerts'; import { TIMELINE_VIEW_IN_ANALYZER } from '../../../screens/timeline'; import { selectAlertsHistogram } from '../../../tasks/alerts'; -import { deleteTimelines } from '../../../tasks/api_calls/common'; +import { deleteTimelines } from '../../../tasks/api_calls/timelines'; import { createTimeline } from '../../../tasks/api_calls/timelines'; import { getTimeline } from '../../../objects/timeline'; diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timeline_templates/creation.cy.ts b/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timeline_templates/creation.cy.ts index 2c640bf3d4492..40efaa1f20883 100644 --- a/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timeline_templates/creation.cy.ts +++ b/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timeline_templates/creation.cy.ts @@ -16,7 +16,7 @@ import { } from '../../../screens/timeline'; import { TIMELINES_DESCRIPTION, TIMELINES_USERNAME } from '../../../screens/timelines'; import { createTimeline } from '../../../tasks/api_calls/timelines'; -import { deleteTimelines } from '../../../tasks/api_calls/common'; +import { deleteTimelines } from '../../../tasks/api_calls/timelines'; import { login } from '../../../tasks/login'; import { visit } from '../../../tasks/navigation'; diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timeline_templates/export.cy.ts b/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timeline_templates/export.cy.ts index caf9271adb19b..2f32c15ac8b86 100644 --- a/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timeline_templates/export.cy.ts +++ b/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timeline_templates/export.cy.ts @@ -14,9 +14,8 @@ import { } from '../../../objects/timeline'; import { TIMELINE_TEMPLATES_URL } from '../../../urls/navigation'; -import { createTimelineTemplate } from '../../../tasks/api_calls/timelines'; +import { createTimelineTemplate, deleteTimelines } from '../../../tasks/api_calls/timelines'; import { searchByTitle } from '../../../tasks/table_pagination'; -import { deleteTimelines } from '../../../tasks/api_calls/common'; describe('Export timelines', { tags: ['@ess', '@serverless'] }, () => { beforeEach(() => { diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/correlation_tab.cy.ts b/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/correlation_tab.cy.ts index 2318a4c4ef452..70f7d82e34629 100644 --- a/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/correlation_tab.cy.ts +++ b/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/correlation_tab.cy.ts @@ -21,7 +21,7 @@ import { addEqlToTimeline, saveTimeline, clearEqlInTimeline } from '../../../tas import { TIMELINES_URL } from '../../../urls/navigation'; import { EQL_QUERY_VALIDATION_ERROR } from '../../../screens/create_new_rule'; -import { deleteTimelines } from '../../../tasks/api_calls/common'; +import { deleteTimelines } from '../../../tasks/api_calls/timelines'; describe('Correlation tab', { tags: ['@ess', '@serverless'] }, () => { const eql = 'any where process.name == "zsh"'; diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/creation.cy.ts b/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/creation.cy.ts index 88fbf7a238aeb..b11fb0b8cf88f 100644 --- a/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/creation.cy.ts +++ b/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/creation.cy.ts @@ -23,9 +23,8 @@ import { } from '../../../screens/timeline'; import { LOADING_INDICATOR } from '../../../screens/security_header'; import { ROWS } from '../../../screens/timelines'; -import { createTimelineTemplate } from '../../../tasks/api_calls/timelines'; +import { createTimelineTemplate, deleteTimelines } from '../../../tasks/api_calls/timelines'; -import { deleteTimelines } from '../../../tasks/api_calls/common'; import { login } from '../../../tasks/login'; import { visit, visitWithTimeRange } from '../../../tasks/navigation'; import { openTimelineUsingToggle } from '../../../tasks/security_main'; diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/esql/search_filter.cy.ts b/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/esql/search_filter.cy.ts index 73bb13b20236b..15263ff5d6b83 100644 --- a/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/esql/search_filter.cy.ts +++ b/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/esql/search_filter.cy.ts @@ -5,7 +5,6 @@ * 2.0. */ -import { deleteTimelines } from '../../../../tasks/api_calls/common'; import { GET_LOCAL_DATE_PICKER_START_DATE_POPOVER_BUTTON } from '../../../../screens/date_picker'; import { setStartDate, @@ -25,10 +24,11 @@ import { addFieldToTable, convertEditorNonBreakingSpaceToSpace, } from '../../../../tasks/discover'; -import { createNewTimeline, goToEsqlTab, openActiveTimeline } from '../../../../tasks/timeline'; import { login } from '../../../../tasks/login'; import { visitWithTimeRange } from '../../../../tasks/navigation'; import { ALERTS_URL } from '../../../../urls/navigation'; +import { deleteTimelines } from '../../../../tasks/api_calls/timelines'; +import { openActiveTimeline, createNewTimeline, goToEsqlTab } from '../../../../tasks/timeline'; const DEFAULT_DATE = '~ 15 minutes ago'; const INITIAL_START_DATE = 'Jan 18, 2021 @ 20:33:29.186'; diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/export.cy.ts b/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/export.cy.ts index cbcc6fc758963..4c98e9d2f9241 100644 --- a/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/export.cy.ts +++ b/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/export.cy.ts @@ -13,7 +13,7 @@ import { } from '../../../tasks/timelines'; import { login } from '../../../tasks/login'; import { visit } from '../../../tasks/navigation'; -import { deleteTimelines } from '../../../tasks/api_calls/common'; +import { deleteTimelines } from '../../../tasks/api_calls/timelines'; import { TIMELINES_URL } from '../../../urls/navigation'; import { TOASTER } from '../../../screens/alerts_detection_rules'; diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/notes_tab.cy.ts b/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/notes_tab.cy.ts index dd7c775a32118..c9ff65129bfe1 100644 --- a/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/notes_tab.cy.ts +++ b/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/notes_tab.cy.ts @@ -18,14 +18,12 @@ import { MARKDOWN_INVESTIGATE_BUTTON, } from '../../../screens/timeline'; import { MODAL_CONFIRMATION_BTN } from '../../../screens/alerts_detection_rules'; -import { createTimeline } from '../../../tasks/api_calls/timelines'; +import { createTimeline, deleteTimelines } from '../../../tasks/api_calls/timelines'; import { login } from '../../../tasks/login'; import { visitTimeline } from '../../../tasks/navigation'; import { addNotesToTimeline, goToNotesTab } from '../../../tasks/timeline'; -import { deleteTimelines } from '../../../tasks/api_calls/common'; - const author = Cypress.env('ELASTICSEARCH_USERNAME'); const link = 'https://www.elastic.co/'; diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/open_timeline.cy.ts b/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/open_timeline.cy.ts index 6c1ec9b093df7..1d6b54e8339e6 100644 --- a/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/open_timeline.cy.ts +++ b/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/open_timeline.cy.ts @@ -25,7 +25,7 @@ import { refreshTimelinesUntilTimeLinePresent, } from '../../../tasks/timeline'; import { TIMELINES_URL } from '../../../urls/navigation'; -import { deleteTimelines } from '../../../tasks/api_calls/common'; +import { deleteTimelines } from '../../../tasks/api_calls/timelines'; describe('Open timeline modal', { tags: ['@serverless', '@ess'] }, () => { beforeEach(function () { diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/overview.cy.tsx b/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/overview.cy.tsx index 16b6c58146915..ec8583c287de5 100644 --- a/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/overview.cy.tsx +++ b/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/overview.cy.tsx @@ -19,10 +19,13 @@ import { import { login } from '../../../tasks/login'; import { visit } from '../../../tasks/navigation'; -import { createTimeline, favoriteTimeline } from '../../../tasks/api_calls/timelines'; +import { + createTimeline, + deleteTimelines, + favoriteTimeline, +} from '../../../tasks/api_calls/timelines'; import { TIMELINES_URL } from '../../../urls/navigation'; -import { deleteTimelines } from '../../../tasks/api_calls/common'; describe('timeline overview search', { tags: ['@ess', 'serverless'] }, () => { beforeEach(() => { diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/query_tab.cy.ts b/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/query_tab.cy.ts index acd3f4eb2176e..3cd3f1148f471 100644 --- a/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/query_tab.cy.ts +++ b/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/query_tab.cy.ts @@ -14,7 +14,7 @@ import { TIMELINE_QUERY, NOTE_CARD_CONTENT, } from '../../../screens/timeline'; -import { deleteTimelines } from '../../../tasks/api_calls/common'; +import { deleteTimelines } from '../../../tasks/api_calls/timelines'; import { addNoteToTimeline } from '../../../tasks/api_calls/notes'; import { createTimeline } from '../../../tasks/api_calls/timelines'; diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/row_renderers.cy.ts b/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/row_renderers.cy.ts index 4867ba79183f3..09e04c28e4975 100644 --- a/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/row_renderers.cy.ts +++ b/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/row_renderers.cy.ts @@ -16,7 +16,7 @@ import { TIMELINE_ROW_RENDERERS_SURICATA_LINK_TOOLTIP, TIMELINE_ROW_RENDERERS_MODAL_CLOSE_BUTTON, } from '../../../screens/timeline'; -import { deleteTimelines } from '../../../tasks/api_calls/common'; +import { deleteTimelines } from '../../../tasks/api_calls/timelines'; import { waitForWelcomePanelToBeLoaded } from '../../../tasks/common'; import { waitForAllHostsToBeLoaded } from '../../../tasks/hosts/all_hosts'; diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/search_or_filter.cy.ts b/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/search_or_filter.cy.ts index ad72dfd2f6ca3..1c55f606d4a72 100644 --- a/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/search_or_filter.cy.ts +++ b/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/search_or_filter.cy.ts @@ -20,7 +20,7 @@ import { selectKqlSearchMode, } from '../../../tasks/timeline'; import { waitForTimelinesPanelToBeLoaded } from '../../../tasks/timelines'; -import { deleteTimelines } from '../../../tasks/api_calls/common'; +import { deleteTimelines } from '../../../tasks/api_calls/timelines'; import { hostsUrl, TIMELINES_URL } from '../../../urls/navigation'; diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/url_state.cy.ts b/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/url_state.cy.ts index ddd65d69c5fed..23403fc7b101d 100644 --- a/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/url_state.cy.ts +++ b/x-pack/test/security_solution_cypress/cypress/e2e/investigations/timelines/url_state.cy.ts @@ -16,7 +16,7 @@ import { getNewRule } from '../../../objects/rule'; import { login } from '../../../tasks/login'; import { visit, visitWithTimeRange } from '../../../tasks/navigation'; import { TIMELINES_URL } from '../../../urls/navigation'; -import { deleteTimelines } from '../../../tasks/api_calls/common'; +import { deleteTimelines } from '../../../tasks/api_calls/timelines'; describe('Open timeline', { tags: ['@serverless', '@ess'] }, () => { let timelineSavedObjectId: string | null = null; diff --git a/x-pack/test/security_solution_cypress/cypress/tasks/api_calls/common.ts b/x-pack/test/security_solution_cypress/cypress/tasks/api_calls/common.ts index 194bfd2a4129f..897a0a3a0ec8e 100644 --- a/x-pack/test/security_solution_cypress/cypress/tasks/api_calls/common.ts +++ b/x-pack/test/security_solution_cypress/cypress/tasks/api_calls/common.ts @@ -95,27 +95,6 @@ export const deleteEndpointExceptionList = () => { }); }; -export const deleteTimelines = () => { - const kibanaIndexUrl = `${Cypress.env('ELASTICSEARCH_URL')}/.kibana_\*`; - rootRequest({ - method: 'POST', - url: `${kibanaIndexUrl}/_delete_by_query?conflicts=proceed&refresh`, - body: { - query: { - bool: { - filter: [ - { - match: { - type: 'siem-ui-timeline', - }, - }, - ], - }, - }, - }, - }); -}; - export const getConnectors = () => rootRequest({ method: 'GET', diff --git a/x-pack/test/security_solution_cypress/cypress/tasks/api_calls/timelines.ts b/x-pack/test/security_solution_cypress/cypress/tasks/api_calls/timelines.ts index 620a105a2b98e..e6c13eb4b5988 100644 --- a/x-pack/test/security_solution_cypress/cypress/tasks/api_calls/timelines.ts +++ b/x-pack/test/security_solution_cypress/cypress/tasks/api_calls/timelines.ts @@ -5,7 +5,10 @@ * 2.0. */ -import type { TimelineResponse } from '@kbn/security-solution-plugin/common/api/timeline'; +import type { + AllTimelinesResponse, + TimelineResponse, +} from '@kbn/security-solution-plugin/common/api/timeline'; import type { CompleteTimeline } from '../../objects/timeline'; import { rootRequest } from './common'; @@ -141,3 +144,22 @@ export const favoriteTimeline = ({ templateTimelineVersion: templateTimelineVersion || null, }, }); + +export const getAllTimelines = () => + rootRequest({ + method: 'GET', + url: 'api/timelines?page_size=100&page_index=1&sort_field=updated&sort_order=desc&timeline_type=default', + }); + +export const deleteTimelines = () => { + getAllTimelines().then(($timelines) => { + const savedObjectIds = $timelines.body.timeline.map((timeline) => timeline.savedObjectId); + rootRequest({ + method: 'DELETE', + url: 'api/timeline', + body: { + savedObjectIds, + }, + }); + }); +}; From c81301bd5666bff4d28e8d72227f288a57be0658 Mon Sep 17 00:00:00 2001 From: Ying Mao Date: Thu, 21 Mar 2024 15:16:16 -0400 Subject: [PATCH 7/7] [Response Ops][Actions] Adding `getAll` to UnsecuredActionsClient (#179090) ## Summary Adds unsecured version of the `getAll` function that is exposed via the `unsecuredActionsClient` on the actions plugin start contract. This version doesn't expect any user request and doesn't check RBAC. Actions saved objects are accessed via the internal saved objects repository (instead of a saved objects client scoped to the request) and an ES client that uses the internal user (instead of scoped to the request). This function returns all connectors for the give space ID and preconfigured connectors but does not return system actions. ## To Verify 1. Run this PR and create a connector via the UI or API in multiple different spaces. 2. In `x-pack/plugins/alerting/server/plugin.ts` in the start function, add the following: ``` const unsecuredActionsClient = plugins.actions.getUnsecuredActionsClient(); unsecuredActionsClient.getAll('default').then((results) => { console.log('default connectors'); console.log(JSON.stringify(results)); }); unsecuredActionsClient.getAll().then((results) => { console.log(JSON.stringify(results)); }); ``` 3. Restart Kibana and verify that the appropriate connectors were returned and logged. --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> --- .../connector/methods/get_all/get_all.test.ts | 451 +++++++++++++++++- .../connector/methods/get_all/get_all.ts | 96 +++- .../connector/methods/get_all/index.ts | 2 +- .../connector/methods/get_all/types/params.ts | 17 +- .../data/connector/find_connectors_so.ts | 6 +- .../data/connector/search_connectors_so.ts | 4 +- .../server/data/connector/types/params.ts | 9 +- x-pack/plugins/actions/server/plugin.ts | 6 +- .../unsecured_actions_client.mock.ts | 1 + .../unsecured_actions_client.test.ts | 105 +++- .../unsecured_actions_client.ts | 41 +- x-pack/plugins/actions/tsconfig.json | 3 +- .../server/unsecured_actions_simulation.ts | 29 ++ .../actions/get_all_unsecured_actions.ts | 209 ++++++++ .../spaces_only/tests/actions/index.ts | 1 + 15 files changed, 927 insertions(+), 53 deletions(-) create mode 100644 x-pack/test/alerting_api_integration/spaces_only/tests/actions/get_all_unsecured_actions.ts diff --git a/x-pack/plugins/actions/server/application/connector/methods/get_all/get_all.test.ts b/x-pack/plugins/actions/server/application/connector/methods/get_all/get_all.test.ts index 2e264300490f8..2032653712a59 100644 --- a/x-pack/plugins/actions/server/application/connector/methods/get_all/get_all.test.ts +++ b/x-pack/plugins/actions/server/application/connector/methods/get_all/get_all.test.ts @@ -10,7 +10,10 @@ import { ActionsAuthorization } from '../../../../authorization/actions_authoriz import { connectorTokenClientMock } from '../../../../lib/connector_token_client.mock'; import { getOAuthJwtAccessToken } from '../../../../lib/get_oauth_jwt_access_token'; import { getOAuthClientCredentialsAccessToken } from '../../../../lib/get_oauth_client_credentials_access_token'; -import { savedObjectsClientMock } from '@kbn/core-saved-objects-api-server-mocks'; +import { + savedObjectsClientMock, + savedObjectsRepositoryMock, +} from '@kbn/core-saved-objects-api-server-mocks'; import { actionsAuthorizationMock } from '../../../../authorization/actions_authorization.mock'; import { elasticsearchServiceMock } from '@kbn/core-elasticsearch-server-mocks'; import { actionExecutorMock } from '../../../../lib/action_executor.mock'; @@ -21,6 +24,7 @@ import { loggingSystemMock } from '@kbn/core-logging-server-mocks'; import { Logger } from '@kbn/logging'; import { eventLogClientMock } from '@kbn/event-log-plugin/server/event_log_client.mock'; import { ActionTypeRegistry } from '../../../../action_type_registry'; +import { getAllUnsecured } from './get_all'; jest.mock('@kbn/core-saved-objects-utils-server', () => { const actual = jest.requireActual('@kbn/core-saved-objects-utils-server'); @@ -77,6 +81,7 @@ const logger = loggingSystemMock.create().get() as jest.Mocked; const eventLogClient = eventLogClientMock.create(); const getEventLogClient = jest.fn(); const connectorTokenClient = connectorTokenClientMock.create(); +const internalSavedObjectsRepository = savedObjectsRepositoryMock.create(); let actionsClient: ActionsClient; let actionTypeRegistry: ActionTypeRegistry; @@ -551,3 +556,447 @@ describe('getAll()', () => { ); }); }); + +describe('getAllUnsecured()', () => { + beforeEach(() => { + jest.resetAllMocks(); + jest.clearAllMocks(); + }); + + test('calls internalSavedObjectRepository with parameters and returns inMemoryConnectors correctly', async () => { + const expectedResult = { + total: 1, + per_page: 10, + page: 1, + saved_objects: [ + { + id: '1', + type: 'type', + attributes: { + name: 'test', + isMissingSecrets: false, + config: { + foo: 'bar', + }, + secrets: 'this should not be returned', + }, + score: 1, + references: [], + }, + ], + }; + internalSavedObjectsRepository.find.mockResolvedValueOnce(expectedResult); + scopedClusterClient.asInternalUser.search.mockResponse( + // @ts-expect-error not full search response + { + aggregations: { + '1': { doc_count: 6 }, + testPreconfigured: { doc_count: 2 }, + 'system-connector-.cases': { doc_count: 2 }, + }, + } + ); + + const result = await getAllUnsecured({ + esClient: scopedClusterClient.asInternalUser, + inMemoryConnectors: [ + { + id: 'testPreconfigured', + actionTypeId: '.slack', + secrets: {}, + isPreconfigured: true, + isDeprecated: false, + isSystemAction: false, + name: 'test', + config: { + foo: 'bar', + }, + }, + /** + * System actions will not + * be returned from getAllUnsecured + */ + { + id: 'system-connector-.cases', + actionTypeId: '.cases', + name: 'System action: .cases', + config: {}, + secrets: {}, + isDeprecated: false, + isMissingSecrets: false, + isPreconfigured: false, + isSystemAction: true, + }, + ], + internalSavedObjectsRepository, + kibanaIndices, + logger, + spaceId: 'default', + }); + + expect(result).toEqual([ + { + id: '1', + name: 'test', + isMissingSecrets: false, + config: { foo: 'bar' }, + isPreconfigured: false, + isDeprecated: false, + isSystemAction: false, + referencedByCount: 6, + }, + { + id: 'testPreconfigured', + actionTypeId: '.slack', + name: 'test', + isPreconfigured: true, + isSystemAction: false, + isDeprecated: false, + referencedByCount: 2, + }, + ]); + + expect(internalSavedObjectsRepository.find).toHaveBeenCalledWith({ + perPage: 10000, + type: 'action', + }); + + expect(scopedClusterClient.asInternalUser.search).toHaveBeenCalledWith({ + index: kibanaIndices, + ignore_unavailable: true, + body: { + aggs: { + '1': { + filter: { + bool: { + must: { + nested: { + path: 'references', + query: { + bool: { + filter: { + bool: { + must: [ + { + term: { + 'references.id': '1', + }, + }, + { + term: { + 'references.type': 'action', + }, + }, + ], + }, + }, + }, + }, + }, + }, + }, + }, + }, + testPreconfigured: { + filter: { + bool: { + must: { + nested: { + path: 'references', + query: { + bool: { + filter: { + bool: { + must: [ + { + term: { + 'references.id': 'testPreconfigured', + }, + }, + { + term: { + 'references.type': 'action', + }, + }, + ], + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + size: 0, + query: { + match_all: {}, + }, + }, + }); + + expect(auditLogger.log).not.toHaveBeenCalled(); + expect(authorization.ensureAuthorized).not.toHaveBeenCalled(); + }); + + test('passed custom space id if defined', async () => { + const expectedResult = { + total: 1, + per_page: 10, + page: 1, + saved_objects: [ + { + id: '1', + type: 'type', + attributes: { + name: 'test', + isMissingSecrets: false, + config: { + foo: 'bar', + }, + secrets: 'this should not be returned', + }, + score: 1, + references: [], + }, + ], + }; + internalSavedObjectsRepository.find.mockResolvedValueOnce(expectedResult); + scopedClusterClient.asInternalUser.search.mockResponse( + // @ts-expect-error not full search response + { + aggregations: { + '1': { doc_count: 6 }, + testPreconfigured: { doc_count: 2 }, + 'system-connector-.cases': { doc_count: 2 }, + }, + } + ); + + const result = await getAllUnsecured({ + esClient: scopedClusterClient.asInternalUser, + inMemoryConnectors: [ + { + id: 'testPreconfigured', + actionTypeId: '.slack', + secrets: {}, + isPreconfigured: true, + isDeprecated: false, + isSystemAction: false, + name: 'test', + config: { + foo: 'bar', + }, + }, + /** + * System actions will not + * be returned from getAllUnsecured + */ + { + id: 'system-connector-.cases', + actionTypeId: '.cases', + name: 'System action: .cases', + config: {}, + secrets: {}, + isDeprecated: false, + isMissingSecrets: false, + isPreconfigured: false, + isSystemAction: true, + }, + ], + internalSavedObjectsRepository, + kibanaIndices, + logger, + spaceId: 'custom', + }); + + expect(result).toEqual([ + { + id: '1', + name: 'test', + isMissingSecrets: false, + config: { foo: 'bar' }, + isPreconfigured: false, + isDeprecated: false, + isSystemAction: false, + referencedByCount: 6, + }, + { + id: 'testPreconfigured', + actionTypeId: '.slack', + name: 'test', + isPreconfigured: true, + isSystemAction: false, + isDeprecated: false, + referencedByCount: 2, + }, + ]); + + expect(internalSavedObjectsRepository.find).toHaveBeenCalledWith({ + perPage: 10000, + type: 'action', + namespaces: ['custom'], + }); + + expect(scopedClusterClient.asInternalUser.search).toHaveBeenCalledWith({ + index: kibanaIndices, + ignore_unavailable: true, + body: { + aggs: { + '1': { + filter: { + bool: { + must: { + nested: { + path: 'references', + query: { + bool: { + filter: { + bool: { + must: [ + { + term: { + 'references.id': '1', + }, + }, + { + term: { + 'references.type': 'action', + }, + }, + ], + }, + }, + }, + }, + }, + }, + }, + }, + }, + testPreconfigured: { + filter: { + bool: { + must: { + nested: { + path: 'references', + query: { + bool: { + filter: { + bool: { + must: [ + { + term: { + 'references.id': 'testPreconfigured', + }, + }, + { + term: { + 'references.type': 'action', + }, + }, + ], + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + size: 0, + query: { + match_all: {}, + }, + }, + }); + + expect(auditLogger.log).not.toHaveBeenCalled(); + expect(authorization.ensureAuthorized).not.toHaveBeenCalled(); + }); + + test('validates connectors before return', async () => { + internalSavedObjectsRepository.find.mockResolvedValueOnce({ + total: 1, + per_page: 10, + page: 1, + saved_objects: [ + { + id: '1', + type: 'type', + attributes: { + name: 'test', + isMissingSecrets: false, + config: { + foo: 'bar', + }, + }, + score: 1, + references: [], + }, + ], + }); + scopedClusterClient.asInternalUser.search.mockResponse( + // @ts-expect-error not full search response + { + aggregations: { + '1': { doc_count: 6 }, + testPreconfigured: { doc_count: 2 }, + }, + } + ); + + const result = await getAllUnsecured({ + esClient: scopedClusterClient.asInternalUser, + inMemoryConnectors: [ + { + id: 'testPreconfigured', + actionTypeId: '.slack', + secrets: {}, + isPreconfigured: true, + isDeprecated: false, + isSystemAction: false, + name: 'test', + config: { + foo: 'bar', + }, + }, + ], + internalSavedObjectsRepository, + kibanaIndices, + logger, + spaceId: 'default', + }); + + expect(result).toEqual([ + { + config: { + foo: 'bar', + }, + id: '1', + isDeprecated: false, + isMissingSecrets: false, + isPreconfigured: false, + isSystemAction: false, + name: 'test', + referencedByCount: 6, + }, + { + actionTypeId: '.slack', + id: 'testPreconfigured', + isDeprecated: false, + isPreconfigured: true, + isSystemAction: false, + name: 'test', + referencedByCount: 2, + }, + ]); + + expect(logger.warn).toHaveBeenCalledWith( + 'Error validating connector: 1, Error: [actionTypeId]: expected value of type [string] but got [undefined]' + ); + }); +}); diff --git a/x-pack/plugins/actions/server/application/connector/methods/get_all/get_all.ts b/x-pack/plugins/actions/server/application/connector/methods/get_all/get_all.ts index 8d764a9c632e0..9c3b9c13924fd 100644 --- a/x-pack/plugins/actions/server/application/connector/methods/get_all/get_all.ts +++ b/x-pack/plugins/actions/server/application/connector/methods/get_all/get_all.ts @@ -9,12 +9,28 @@ * Get all actions with in-memory connectors */ import * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey'; +import { AuditLogger } from '@kbn/security-plugin-types-server'; +import { ElasticsearchClient, Logger } from '@kbn/core/server'; +import { omit } from 'lodash'; +import { InMemoryConnector } from '../../../..'; +import { SavedObjectClientForFind } from '../../../../data/connector/types/params'; import { connectorWithExtraFindDataSchema } from '../../schemas'; import { findConnectorsSo, searchConnectorsSo } from '../../../../data/connector'; import { GetAllParams, InjectExtraFindDataParams } from './types'; import { ConnectorAuditAction, connectorAuditEvent } from '../../../../lib/audit_events'; import { connectorFromSavedObject, isConnectorDeprecated } from '../../lib'; import { ConnectorWithExtraFindData } from '../../types'; +import { GetAllUnsecuredParams } from './types/params'; + +interface GetAllHelperOpts { + auditLogger?: AuditLogger; + esClient: ElasticsearchClient; + inMemoryConnectors: InMemoryConnector[]; + kibanaIndices: string[]; + logger: Logger; + namespace?: string; + savedObjectsClient: SavedObjectClientForFind; +} export async function getAll({ context, @@ -32,28 +48,70 @@ export async function getAll({ throw error; } + return await getAllHelper({ + auditLogger: context.auditLogger, + esClient: context.scopedClusterClient.asInternalUser, + inMemoryConnectors: includeSystemActions + ? context.inMemoryConnectors + : context.inMemoryConnectors.filter((connector) => !connector.isSystemAction), + kibanaIndices: context.kibanaIndices, + logger: context.logger, + savedObjectsClient: context.unsecuredSavedObjectsClient, + }); +} + +export async function getAllUnsecured({ + esClient, + inMemoryConnectors, + internalSavedObjectsRepository, + kibanaIndices, + logger, + spaceId, +}: GetAllUnsecuredParams): Promise { + const namespace = spaceId && spaceId !== 'default' ? spaceId : undefined; + + const connectors = await getAllHelper({ + esClient, + // Unsecured execution does not currently support system actions so we filter them out + inMemoryConnectors: inMemoryConnectors.filter((connector) => !connector.isSystemAction), + kibanaIndices, + logger, + namespace, + savedObjectsClient: internalSavedObjectsRepository, + }); + + return connectors.map((connector) => omit(connector, 'secrets')); +} + +async function getAllHelper({ + auditLogger, + esClient, + inMemoryConnectors, + kibanaIndices, + logger, + namespace, + savedObjectsClient, +}: GetAllHelperOpts): Promise { const savedObjectsActions = ( - await findConnectorsSo({ unsecuredSavedObjectsClient: context.unsecuredSavedObjectsClient }) + await findConnectorsSo({ savedObjectsClient, namespace }) ).saved_objects.map((rawAction) => connectorFromSavedObject(rawAction, isConnectorDeprecated(rawAction.attributes)) ); - savedObjectsActions.forEach(({ id }) => - context.auditLogger?.log( - connectorAuditEvent({ - action: ConnectorAuditAction.FIND, - savedObject: { type: 'action', id }, - }) - ) - ); - - const inMemoryConnectorsFiltered = includeSystemActions - ? context.inMemoryConnectors - : context.inMemoryConnectors.filter((connector) => !connector.isSystemAction); + if (auditLogger) { + savedObjectsActions.forEach(({ id }) => + auditLogger.log( + connectorAuditEvent({ + action: ConnectorAuditAction.FIND, + savedObject: { type: 'action', id }, + }) + ) + ); + } const mergedResult = [ ...savedObjectsActions, - ...inMemoryConnectorsFiltered.map((inMemoryConnector) => ({ + ...inMemoryConnectors.map((inMemoryConnector) => ({ id: inMemoryConnector.id, actionTypeId: inMemoryConnector.actionTypeId, name: inMemoryConnector.name, @@ -64,8 +122,8 @@ export async function getAll({ ].sort((a, b) => a.name.localeCompare(b.name)); const connectors = await injectExtraFindData({ - kibanaIndices: context.kibanaIndices, - scopedClusterClient: context.scopedClusterClient, + kibanaIndices, + esClient, connectors: mergedResult, }); @@ -74,7 +132,7 @@ export async function getAll({ try { connectorWithExtraFindDataSchema.validate(connector); } catch (e) { - context.logger.warn(`Error validating connector: ${connector.id}, ${e}`); + logger.warn(`Error validating connector: ${connector.id}, ${e}`); } }); @@ -83,7 +141,7 @@ export async function getAll({ async function injectExtraFindData({ kibanaIndices, - scopedClusterClient, + esClient, connectors, }: InjectExtraFindDataParams): Promise { const aggs: Record = {}; @@ -121,7 +179,7 @@ async function injectExtraFindData({ }; } - const aggregationResult = await searchConnectorsSo({ scopedClusterClient, kibanaIndices, aggs }); + const aggregationResult = await searchConnectorsSo({ esClient, kibanaIndices, aggs }); return connectors.map((connector) => ({ ...connector, diff --git a/x-pack/plugins/actions/server/application/connector/methods/get_all/index.ts b/x-pack/plugins/actions/server/application/connector/methods/get_all/index.ts index dcbc4c6fbc957..5b3da65578d65 100644 --- a/x-pack/plugins/actions/server/application/connector/methods/get_all/index.ts +++ b/x-pack/plugins/actions/server/application/connector/methods/get_all/index.ts @@ -5,4 +5,4 @@ * 2.0. */ -export { getAll } from './get_all'; +export { getAll, getAllUnsecured } from './get_all'; diff --git a/x-pack/plugins/actions/server/application/connector/methods/get_all/types/params.ts b/x-pack/plugins/actions/server/application/connector/methods/get_all/types/params.ts index 4e5157a1fdce0..ca0afdb782f7d 100644 --- a/x-pack/plugins/actions/server/application/connector/methods/get_all/types/params.ts +++ b/x-pack/plugins/actions/server/application/connector/methods/get_all/types/params.ts @@ -5,7 +5,10 @@ * 2.0. */ -import { IScopedClusterClient } from '@kbn/core-elasticsearch-server'; +import { ElasticsearchClient } from '@kbn/core-elasticsearch-server'; +import { ISavedObjectsRepository, Logger } from '@kbn/core/server'; +import { AuditLogger } from '@kbn/security-plugin/server'; +import { InMemoryConnector } from '../../../../..'; import { ActionsClientContext } from '../../../../../actions_client'; import { Connector } from '../../../types'; @@ -14,8 +17,18 @@ export interface GetAllParams { context: ActionsClientContext; } +export interface GetAllUnsecuredParams { + auditLogger?: AuditLogger; + esClient: ElasticsearchClient; + inMemoryConnectors: InMemoryConnector[]; + internalSavedObjectsRepository: ISavedObjectsRepository; + kibanaIndices: string[]; + logger: Logger; + spaceId: string; +} + export interface InjectExtraFindDataParams { kibanaIndices: string[]; - scopedClusterClient: IScopedClusterClient; + esClient: ElasticsearchClient; connectors: Connector[]; } diff --git a/x-pack/plugins/actions/server/data/connector/find_connectors_so.ts b/x-pack/plugins/actions/server/data/connector/find_connectors_so.ts index da232f5b2aa83..238ae18a1b62b 100644 --- a/x-pack/plugins/actions/server/data/connector/find_connectors_so.ts +++ b/x-pack/plugins/actions/server/data/connector/find_connectors_so.ts @@ -9,10 +9,12 @@ import { FindConnectorsSoResult, FindConnectorsSoParams } from './types'; import { MAX_ACTIONS_RETURNED } from './constants'; export const findConnectorsSo = async ({ - unsecuredSavedObjectsClient, + savedObjectsClient, + namespace, }: FindConnectorsSoParams): Promise => { - return unsecuredSavedObjectsClient.find({ + return savedObjectsClient.find({ perPage: MAX_ACTIONS_RETURNED, type: 'action', + ...(namespace ? { namespaces: [namespace] } : {}), }); }; diff --git a/x-pack/plugins/actions/server/data/connector/search_connectors_so.ts b/x-pack/plugins/actions/server/data/connector/search_connectors_so.ts index 09d3ae3b532d9..ab549899348ae 100644 --- a/x-pack/plugins/actions/server/data/connector/search_connectors_so.ts +++ b/x-pack/plugins/actions/server/data/connector/search_connectors_so.ts @@ -8,11 +8,11 @@ import { SearchConnectorsSoParams } from './types'; export const searchConnectorsSo = async ({ - scopedClusterClient, + esClient, kibanaIndices, aggs, }: SearchConnectorsSoParams) => { - return scopedClusterClient.asInternalUser.search({ + return esClient.search({ index: kibanaIndices, ignore_unavailable: true, body: { diff --git a/x-pack/plugins/actions/server/data/connector/types/params.ts b/x-pack/plugins/actions/server/data/connector/types/params.ts index 73d8ea6dadd14..c23447fb37486 100644 --- a/x-pack/plugins/actions/server/data/connector/types/params.ts +++ b/x-pack/plugins/actions/server/data/connector/types/params.ts @@ -5,18 +5,21 @@ * 2.0. */ -import { IScopedClusterClient } from '@kbn/core-elasticsearch-server'; +import { ElasticsearchClient } from '@kbn/core-elasticsearch-server'; import * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey'; import { SavedObjectsClientContract } from '@kbn/core-saved-objects-api-server'; +import { SavedObjectsClient } from '@kbn/core/server'; +export type SavedObjectClientForFind = Pick; export interface SearchConnectorsSoParams { kibanaIndices: string[]; - scopedClusterClient: IScopedClusterClient; + esClient: ElasticsearchClient; aggs: Record; } export interface FindConnectorsSoParams { - unsecuredSavedObjectsClient: SavedObjectsClientContract; + savedObjectsClient: SavedObjectClientForFind; + namespace?: string; } export interface GetConnectorSoParams { diff --git a/x-pack/plugins/actions/server/plugin.ts b/x-pack/plugins/actions/server/plugin.ts index 33383e526e36d..1ef28b10e6440 100644 --- a/x-pack/plugins/actions/server/plugin.ts +++ b/x-pack/plugins/actions/server/plugin.ts @@ -486,18 +486,22 @@ export class ActionsPlugin implements Plugin { const internalSavedObjectsRepository = core.savedObjects.createInternalRepository([ + ACTION_SAVED_OBJECT_TYPE, ACTION_TASK_PARAMS_SAVED_OBJECT_TYPE, ]); return new UnsecuredActionsClient({ actionExecutor: actionExecutor!, - internalSavedObjectsRepository, + clusterClient: core.elasticsearch.client, executionEnqueuer: createBulkUnsecuredExecutionEnqueuerFunction({ taskManager: plugins.taskManager, connectorTypeRegistry: actionTypeRegistry!, inMemoryConnectors: this.inMemoryConnectors, configurationUtilities: actionsConfigUtils, }), + inMemoryConnectors: this.inMemoryConnectors, + internalSavedObjectsRepository, + kibanaIndices: core.savedObjects.getAllIndices(), logger: this.logger, }); }; diff --git a/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.mock.ts b/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.mock.ts index 4cbbfa1604dc1..748847d579eeb 100644 --- a/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.mock.ts +++ b/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.mock.ts @@ -11,6 +11,7 @@ export type UnsecuredActionsClientMock = jest.Mocked; const createUnsecuredActionsClientMock = () => { const mocked: UnsecuredActionsClientMock = { + getAll: jest.fn(), execute: jest.fn(), bulkEnqueueExecution: jest.fn(), }; diff --git a/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.test.ts b/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.test.ts index 5df39e28fcbc1..89145d80eea19 100644 --- a/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.test.ts +++ b/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.test.ts @@ -6,28 +6,125 @@ */ import { v4 as uuidv4 } from 'uuid'; -import { loggingSystemMock, savedObjectsRepositoryMock } from '@kbn/core/server/mocks'; +import { + elasticsearchServiceMock, + loggingSystemMock, + savedObjectsRepositoryMock, +} from '@kbn/core/server/mocks'; import { asNotificationExecutionSource } from '../lib'; import { actionExecutorMock } from '../lib/action_executor.mock'; import { UnsecuredActionsClient } from './unsecured_actions_client'; +import { Logger } from '@kbn/core/server'; +import { getAllUnsecured } from '../application/connector/methods/get_all/get_all'; + +jest.mock('../application/connector/methods/get_all/get_all'); + +const mockGetAllUnsecured = getAllUnsecured as jest.MockedFunction; const internalSavedObjectsRepository = savedObjectsRepositoryMock.create(); const actionExecutor = actionExecutorMock.create(); const executionEnqueuer = jest.fn(); -const logger = loggingSystemMock.create().get(); - +const logger = loggingSystemMock.create().get() as jest.Mocked; +const clusterClient = elasticsearchServiceMock.createClusterClient(); +const inMemoryConnectors = [ + { + id: 'testPreconfigured', + actionTypeId: '.slack', + secrets: {}, + isPreconfigured: true, + isDeprecated: false, + isSystemAction: false, + name: 'test', + config: { + foo: 'bar', + }, + }, + /** + * System actions will not + * be returned from getAllUnsecured + */ + { + id: 'system-connector-.cases', + actionTypeId: '.cases', + name: 'System action: .cases', + config: {}, + secrets: {}, + isDeprecated: false, + isMissingSecrets: false, + isPreconfigured: false, + isSystemAction: true, + }, +]; let unsecuredActionsClient: UnsecuredActionsClient; beforeEach(() => { jest.resetAllMocks(); unsecuredActionsClient = new UnsecuredActionsClient({ actionExecutor, - internalSavedObjectsRepository, + clusterClient, executionEnqueuer, + inMemoryConnectors, + internalSavedObjectsRepository, + kibanaIndices: ['.kibana'], logger, }); }); +describe('getAll()', () => { + test('calls getAllUnsecured library method with appropriate parameters', async () => { + const expectedResult = [ + { + actionTypeId: 'test', + id: '1', + name: 'test', + isMissingSecrets: false, + config: { foo: 'bar' }, + isPreconfigured: false, + isDeprecated: false, + isSystemAction: false, + referencedByCount: 6, + }, + { + id: 'testPreconfigured', + actionTypeId: '.slack', + name: 'test', + isPreconfigured: true, + isSystemAction: false, + isDeprecated: false, + referencedByCount: 2, + }, + ]; + mockGetAllUnsecured.mockResolvedValueOnce(expectedResult); + const result = await unsecuredActionsClient.getAll('default'); + expect(result).toEqual(expectedResult); + expect(mockGetAllUnsecured).toHaveBeenCalledWith({ + esClient: clusterClient.asInternalUser, + inMemoryConnectors, + kibanaIndices: ['.kibana'], + logger, + internalSavedObjectsRepository, + spaceId: 'default', + }); + }); + + test('throws error if getAllUnsecured throws errors', async () => { + mockGetAllUnsecured.mockImplementationOnce(() => { + throw new Error('failfail'); + }); + await expect( + unsecuredActionsClient.getAll('customSpace') + ).rejects.toThrowErrorMatchingInlineSnapshot(`"failfail"`); + expect(mockGetAllUnsecured).toHaveBeenCalledWith({ + esClient: clusterClient.asInternalUser, + inMemoryConnectors, + kibanaIndices: ['.kibana'], + logger, + internalSavedObjectsRepository, + spaceId: 'customSpace', + }); + }); +}); + describe('execute()', () => { test('throws error when executing action with not allowed requester id', async () => { await expect( diff --git a/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.ts b/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.ts index 96449380a82cd..8331f6890486c 100644 --- a/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.ts +++ b/x-pack/plugins/actions/server/unsecured_actions_client/unsecured_actions_client.ts @@ -6,7 +6,7 @@ */ import { v4 as uuidv4 } from 'uuid'; -import { ISavedObjectsRepository, Logger } from '@kbn/core/server'; +import { IClusterClient, ISavedObjectsRepository, Logger } from '@kbn/core/server'; import { BulkUnsecuredExecutionEnqueuer, ExecuteOptions, @@ -17,8 +17,10 @@ import { asNotificationExecutionSource, type RelatedSavedObjects, } from '../lib'; -import { ActionTypeExecutorResult } from '../types'; +import { ActionTypeExecutorResult, InMemoryConnector } from '../types'; import { asBackgroundTaskExecutionSource } from '../lib/action_execution_source'; +import { ConnectorWithExtraFindData } from '../application/connector/types'; +import { getAllUnsecured } from '../application/connector/methods/get_all/get_all'; // requests from the notification service (for system notification) const NOTIFICATION_REQUESTER_ID = 'notifications'; @@ -37,8 +39,11 @@ const ALLOWED_REQUESTER_IDS = [ export interface UnsecuredActionsClientOpts { actionExecutor: ActionExecutorContract; - internalSavedObjectsRepository: ISavedObjectsRepository; + clusterClient: IClusterClient; executionEnqueuer: BulkUnsecuredExecutionEnqueuer; + inMemoryConnectors: InMemoryConnector[]; + internalSavedObjectsRepository: ISavedObjectsRepository; + kibanaIndices: string[]; logger: Logger; } @@ -48,6 +53,7 @@ type UnsecuredExecuteOptions = Omit & { }; export interface IUnsecuredActionsClient { + getAll: (spaceId: string) => Promise; execute: (opts: UnsecuredExecuteOptions) => Promise>; bulkEnqueueExecution: ( requesterId: string, @@ -56,17 +62,7 @@ export interface IUnsecuredActionsClient { } export class UnsecuredActionsClient { - private readonly actionExecutor: ActionExecutorContract; - private readonly internalSavedObjectsRepository: ISavedObjectsRepository; - private readonly executionEnqueuer: BulkUnsecuredExecutionEnqueuer; - private readonly logger: Logger; - - constructor(params: UnsecuredActionsClientOpts) { - this.actionExecutor = params.actionExecutor; - this.executionEnqueuer = params.executionEnqueuer; - this.internalSavedObjectsRepository = params.internalSavedObjectsRepository; - this.logger = params.logger; - } + constructor(private readonly opts: UnsecuredActionsClientOpts) {} public async execute({ requesterId, @@ -83,14 +79,14 @@ export class UnsecuredActionsClient { } if (!relatedSavedObjects) { - this.logger.warn( + this.opts.logger.warn( `Calling "execute" in UnsecuredActionsClient without any relatedSavedObjects data. Consider including this for traceability.` ); } const source = this.getSourceFromRequester(requesterId, id, relatedSavedObjects); - return this.actionExecutor.executeUnsecured({ + return this.opts.actionExecutor.executeUnsecured({ actionExecutionId: uuidv4(), actionId: id, params, @@ -123,7 +119,18 @@ export class UnsecuredActionsClient { ...source, }; }); - return this.executionEnqueuer(this.internalSavedObjectsRepository, actionsToEnqueue); + return this.opts.executionEnqueuer(this.opts.internalSavedObjectsRepository, actionsToEnqueue); + } + + public async getAll(spaceId: string): Promise { + return getAllUnsecured({ + esClient: this.opts.clusterClient.asInternalUser, + inMemoryConnectors: this.opts.inMemoryConnectors, + kibanaIndices: this.opts.kibanaIndices, + logger: this.opts.logger, + internalSavedObjectsRepository: this.opts.internalSavedObjectsRepository, + spaceId, + }); } private getSourceFromRequester( diff --git a/x-pack/plugins/actions/tsconfig.json b/x-pack/plugins/actions/tsconfig.json index d30c8bdde7d60..aae2d31c7aa09 100644 --- a/x-pack/plugins/actions/tsconfig.json +++ b/x-pack/plugins/actions/tsconfig.json @@ -44,7 +44,8 @@ "@kbn/core-logging-server-mocks", "@kbn/serverless", "@kbn/actions-types", - "@kbn/core-test-helpers-kbn-server" + "@kbn/core-test-helpers-kbn-server", + "@kbn/security-plugin-types-server" ], "exclude": [ "target/**/*", diff --git a/x-pack/test/alerting_api_integration/common/plugins/actions_simulators/server/unsecured_actions_simulation.ts b/x-pack/test/alerting_api_integration/common/plugins/actions_simulators/server/unsecured_actions_simulation.ts index 675dbe50afd54..536abe2d3526e 100644 --- a/x-pack/test/alerting_api_integration/common/plugins/actions_simulators/server/unsecured_actions_simulation.ts +++ b/x-pack/test/alerting_api_integration/common/plugins/actions_simulators/server/unsecured_actions_simulation.ts @@ -94,4 +94,33 @@ export function initPlugin(router: IRouter, coreSetup: CoreSetup, + res: KibanaResponseFactory + ): Promise> { + const [_, { actions }] = await coreSetup.getStartServices(); + const { body } = req; + + try { + const unsecuredActionsClient = actions.getUnsecuredActionsClient(); + const { spaceId } = body; + const result = await unsecuredActionsClient.getAll(spaceId); + + return res.ok({ body: { status: 'success', result } }); + } catch (err) { + return res.ok({ body: { status: 'error', error: `${err}` } }); + } + } + ); } diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/actions/get_all_unsecured_actions.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/actions/get_all_unsecured_actions.ts new file mode 100644 index 0000000000000..3183f19771f16 --- /dev/null +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/actions/get_all_unsecured_actions.ts @@ -0,0 +1,209 @@ +/* + * 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; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import expect from '@kbn/expect'; +import { FtrProviderContext } from '../../../../common/ftr_provider_context'; +import { getUrlPrefix, ObjectRemover } from '../../../common/lib'; +import { Spaces } from '../../scenarios'; + +// eslint-disable-next-line import/no-default-export +export default function createUnsecuredActionTests({ getService }: FtrProviderContext) { + const supertest = getService('supertest'); + const kibanaServer = getService('kibanaServer'); + + const preconfiguredConnectors = [ + { + id: 'preconfigured-alert-history-es-index', + actionTypeId: '.index', + name: 'Alert history Elasticsearch index', + isPreconfigured: true, + isDeprecated: false, + isSystemAction: false, + referencedByCount: 0, + }, + { + id: 'notification-email', + actionTypeId: '.email', + name: 'Notification Email Connector', + isPreconfigured: true, + isDeprecated: false, + isSystemAction: false, + referencedByCount: 0, + }, + { + id: 'preconfigured-es-index-action', + actionTypeId: '.index', + name: 'preconfigured_es_index_action', + isPreconfigured: true, + isDeprecated: false, + isSystemAction: false, + referencedByCount: 0, + }, + { + id: 'my-deprecated-servicenow', + actionTypeId: '.servicenow', + name: 'ServiceNow#xyz', + isPreconfigured: true, + isDeprecated: true, + isSystemAction: false, + referencedByCount: 0, + }, + { + id: 'my-deprecated-servicenow-default', + actionTypeId: '.servicenow', + name: 'ServiceNow#xyz', + isPreconfigured: true, + isDeprecated: true, + isSystemAction: false, + referencedByCount: 0, + }, + { + id: 'my-slack1', + actionTypeId: '.slack', + name: 'Slack#xyz', + isPreconfigured: true, + isDeprecated: false, + isSystemAction: false, + referencedByCount: 0, + }, + { + id: 'custom-system-abc-connector', + actionTypeId: 'system-abc-action-type', + name: 'SystemABC', + isPreconfigured: true, + isDeprecated: false, + isSystemAction: false, + referencedByCount: 0, + }, + { + id: 'preconfigured.test.index-record', + actionTypeId: 'test.index-record', + name: 'Test:_Preconfigured_Index_Record', + isPreconfigured: true, + isDeprecated: false, + isSystemAction: false, + referencedByCount: 0, + }, + { + id: 'my-test-email', + actionTypeId: '.email', + name: 'TestEmail#xyz', + isPreconfigured: true, + isDeprecated: false, + isSystemAction: false, + referencedByCount: 0, + }, + ]; + + describe('get all unsecured actions', () => { + const objectRemover = new ObjectRemover(supertest); + + // need to wait for kibanaServer to settle ... + before(() => { + kibanaServer.resolveUrl(`/api/get_all_unsecured_actions`); + }); + + after(() => objectRemover.removeAll()); + + it('should successfully get all actions', async () => { + // Create a connector + const { body: createdConnector1 } = await supertest + .post(`${getUrlPrefix(Spaces.space1.id)}/api/actions/connector`) + .set('kbn-xsrf', 'foo') + .send({ + name: 'zzz - My action1', + connector_type_id: 'test.index-record', + config: { + unencrypted: `This value shouldn't get encrypted`, + }, + secrets: { + encrypted: 'This value should be encrypted', + }, + }) + .expect(200); + objectRemover.add(Spaces.space1.id, createdConnector1.id, 'action', 'actions'); + + const { body: createdConnector2 } = await supertest + .post(`${getUrlPrefix(Spaces.other.id)}/api/actions/connector`) + .set('kbn-xsrf', 'foo') + .send({ + name: 'zzz - My action2', + connector_type_id: 'test.index-record', + config: { + unencrypted: `This value shouldn't get encrypted`, + }, + secrets: { + encrypted: 'This value should be encrypted', + }, + }) + .expect(200); + objectRemover.add(Spaces.other.id, createdConnector2.id, 'action', 'actions'); + + const space1SpaceResponse = await supertest + .post(`/api/get_all_unsecured_actions`) + .set('kbn-xsrf', 'xxx') + .send({ + spaceId: Spaces.space1.id, + }) + .expect(200); + expect(space1SpaceResponse.body.status).to.eql('success'); + + // the custom ssl connectors have dynamic ports, so remove them before + // comparing to what we expect + const preconfiguredWithSpace1Connector = space1SpaceResponse.body.result.filter( + (conn: { id: string }) => !conn.id.startsWith('custom.ssl.') + ); + expect(preconfiguredWithSpace1Connector).to.eql([ + ...preconfiguredConnectors, + { + id: createdConnector1.id, + isPreconfigured: false, + isDeprecated: false, + name: 'zzz - My action1', + actionTypeId: 'test.index-record', + isMissingSecrets: false, + isSystemAction: false, + config: { + unencrypted: `This value shouldn't get encrypted`, + }, + referencedByCount: 0, + }, + ]); + + const otherSpaceResponse = await supertest + .post(`/api/get_all_unsecured_actions`) + .set('kbn-xsrf', 'xxx') + .send({ + spaceId: Spaces.other.id, + }) + .expect(200); + expect(otherSpaceResponse.body.status).to.eql('success'); + + // the custom ssl connectors have dynamic ports, so remove them before + // comparing to what we expect + const preconfiguredWithOtherSpaceConnector = otherSpaceResponse.body.result.filter( + (conn: { id: string }) => !conn.id.startsWith('custom.ssl.') + ); + expect(preconfiguredWithOtherSpaceConnector).to.eql([ + ...preconfiguredConnectors, + { + id: createdConnector2.id, + isPreconfigured: false, + isDeprecated: false, + name: 'zzz - My action2', + actionTypeId: 'test.index-record', + isMissingSecrets: false, + isSystemAction: false, + config: { + unencrypted: `This value shouldn't get encrypted`, + }, + referencedByCount: 0, + }, + ]); + }); + }); +} diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/actions/index.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/actions/index.ts index 89f1d48285ae2..4f5832debebda 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/actions/index.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/actions/index.ts @@ -31,6 +31,7 @@ export default function actionsTests({ loadTestFile, getService }: FtrProviderCo loadTestFile(require.resolve('./type_not_enabled')); loadTestFile(require.resolve('./schedule_unsecured_action')); loadTestFile(require.resolve('./execute_unsecured_action')); + loadTestFile(require.resolve('./get_all_unsecured_actions')); loadTestFile(require.resolve('./check_registered_connector_types')); loadTestFile(require.resolve('./max_queued_actions_circuit_breaker'));