From f4159c45838baa4d9adccfc0e0b435dbbcf77d01 Mon Sep 17 00:00:00 2001 From: Sergi Massaneda Date: Fri, 23 Jun 2023 16:15:41 +0200 Subject: [PATCH 01/54] [Security Solution] Use CellActions registry in Discover data grid (#157201) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary closes: https://github.com/elastic/kibana/issues/157191 Enables Discover DataGrid to use registered cell actions instead of the default static actions. ### New `cellActionsTriggerId` prop This PR introduces a new `cellActionsTriggerId` _optional_ prop in the DataGrid component: https://github.com/elastic/kibana/blob/98c210f9ecce969d3621daa3b009aced80ef8f92/src/plugins/discover/public/components/discover_grid/discover_grid.tsx#L198-L201 When this prop is defined, the component queries the trigger's registry to retrieve the cellActions attached to it, using the CellActions package' `useDataGridColumnsCellActions` hook. This hook returns the cellActions array ready to be passed for each column to the EuiDataGrid component. When (non-empty) actions are found in the registry, they are used, replacing all of the default static Discover actions. Otherwise, the default cell actions are used. This new prop also allows other instances of the Discover DataGrid to be configured with custom cell actions, which will probably be needed by Security Timeline integration with Discover. ### New `SEARCH_EMBEDDABLE_CELL_ACTIONS_TRIGGER` Trigger Along with the new `cellActionsTriggerId` prop the plugin also registers a new trigger for "saved search" embeddable: https://github.com/elastic/kibana/blob/055750c8ddf6859d89c1dd89ab22bd03273a2d82/src/plugins/discover/public/plugin.tsx#L387 And it gets passed to the DataGrid component on the Embeddable creation: https://github.com/elastic/kibana/blob/055750c8ddf6859d89c1dd89ab22bd03273a2d82/src/plugins/discover/public/embeddable/saved_search_embeddable.tsx#L403 Having this new trigger available allows solutions to attach custom actions to it, in order to be displayed in the saved search embeddables. Each action will be able to implement its `isCompatible` check to determine if they are going to be displayed in the embedded saved search DataGrid field, or not. If no compatible actions are found, DataGrid will render the default static actions. ℹ️ In this implementation, the actions registered to this new "embeddable trigger" need to check if they are being rendered inside Security using the `isCompatible` function, to prevent them from being displayed in other solutions, resulting in a non-optimal architecture. This approach was needed since there's no plausible way to pass the `cellActionsTriggerId` property from the Dashboard Renderer used in Security, all the way down to the specific Discover "saved search" embeddable. However, the Dashboards team is planning to enable us to pass options to nested embeddables using a registry (https://github.com/elastic/kibana/issues/148933). When this new tool is available we will be able to delegate the trigger registering to Security and configure the "saved search" embeddables to use it. Therefore, the trigger will only be used by Security, so we won't have to worry about Security actions being rendered outside Security. ## Videos before: https://github.com/elastic/kibana/assets/17747913/de92cd74-6125-4766-8e9d-7e0985932618 after: https://github.com/elastic/kibana/assets/17747913/f9bd597a-860e-4572-aa9d-9f1c72c11a4b --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Pablo Neves Machado --- packages/kbn-cell-actions/src/constants.ts | 9 + .../src/context/cell_actions_context.tsx | 24 +- ...use_data_grid_column_cell_actions.test.tsx | 61 ++-- .../use_data_grid_column_cell_actions.tsx | 88 +++--- .../src/hooks/use_load_actions.test.ts | 19 ++ .../src/hooks/use_load_actions.ts | 6 +- packages/kbn-cell-actions/src/index.ts | 1 + packages/kbn-cell-actions/src/utils.test.ts | 36 +++ packages/kbn-cell-actions/src/utils.ts | 13 + packages/kbn-cell-actions/tsconfig.json | 1 + .../discover/public/__mocks__/data_view.ts | 2 +- .../discover/public/__mocks__/services.ts | 2 + .../context/context_app_content.test.tsx | 24 +- .../context/context_app_content.tsx | 49 ++-- .../layout/discover_documents.test.tsx | 21 +- .../components/layout/discover_documents.tsx | 73 ++--- .../layout/discover_histogram_layout.test.tsx | 7 +- .../layout/discover_main_content.test.tsx | 18 +- src/plugins/discover/public/build_services.ts | 3 + .../discover_grid/discover_grid.test.tsx | 125 +++++++- .../discover_grid/discover_grid.tsx | 68 ++++- .../discover_grid_cell_actions.tsx | 10 + .../discover_grid_columns.test.tsx | 274 ++++++------------ .../discover_grid/discover_grid_columns.tsx | 34 ++- .../discover/public/embeddable/constants.ts | 12 + .../discover/public/embeddable/index.ts | 2 +- .../embeddable/saved_search_embeddable.tsx | 9 +- src/plugins/discover/public/index.ts | 2 +- src/plugins/discover/public/plugin.tsx | 3 + src/plugins/discover/tsconfig.json | 1 + .../components/data_table/index.test.tsx | 22 +- .../components/data_table/index.tsx | 54 ++-- x-pack/plugins/security_solution/kibana.jsonc | 4 +- .../discover/add_to_timeline.test.ts | 130 +++++++++ .../discover/add_to_timeline.ts | 36 +++ .../public/actions/add_to_timeline/index.ts | 1 + .../cell_action/copy_to_clipboard.test.ts | 64 ++++ .../discover/copy_to_clipboard.test.ts | 80 +++++ .../discover/copy_to_clipboard.ts | 32 ++ .../public/actions/copy_to_clipboard/index.ts | 1 + .../filter/cell_action/filter_in.test.ts | 75 ++--- .../filter/cell_action/filter_out.test.ts | 83 ++++-- .../actions/filter/discover/filter_in.test.ts | 88 ++++++ .../actions/filter/discover/filter_in.ts | 33 +++ .../filter/discover/filter_out.test.ts | 93 ++++++ .../actions/filter/discover/filter_out.ts | 33 +++ .../public/actions/filter/index.ts | 2 + .../public/actions/register.ts | 143 +++++---- .../show_top_n/cell_action/show_top_n.tsx | 4 +- .../security_solution/public/actions/types.ts | 24 +- .../use_cell_actions.tsx | 65 +++-- .../plugins/security_solution/tsconfig.json | 1 + 52 files changed, 1501 insertions(+), 564 deletions(-) create mode 100644 packages/kbn-cell-actions/src/utils.test.ts create mode 100644 packages/kbn-cell-actions/src/utils.ts create mode 100644 x-pack/plugins/security_solution/public/actions/add_to_timeline/discover/add_to_timeline.test.ts create mode 100644 x-pack/plugins/security_solution/public/actions/add_to_timeline/discover/add_to_timeline.ts create mode 100644 x-pack/plugins/security_solution/public/actions/copy_to_clipboard/cell_action/copy_to_clipboard.test.ts create mode 100644 x-pack/plugins/security_solution/public/actions/copy_to_clipboard/discover/copy_to_clipboard.test.ts create mode 100644 x-pack/plugins/security_solution/public/actions/copy_to_clipboard/discover/copy_to_clipboard.ts create mode 100644 x-pack/plugins/security_solution/public/actions/filter/discover/filter_in.test.ts create mode 100644 x-pack/plugins/security_solution/public/actions/filter/discover/filter_in.ts create mode 100644 x-pack/plugins/security_solution/public/actions/filter/discover/filter_out.test.ts create mode 100644 x-pack/plugins/security_solution/public/actions/filter/discover/filter_out.ts diff --git a/packages/kbn-cell-actions/src/constants.ts b/packages/kbn-cell-actions/src/constants.ts index 70b9862a933de..fee20ae66ddf7 100644 --- a/packages/kbn-cell-actions/src/constants.ts +++ b/packages/kbn-cell-actions/src/constants.ts @@ -6,6 +6,8 @@ * Side Public License, v 1. */ +import { KBN_FIELD_TYPES } from '@kbn/field-types'; + export const FILTER_CELL_ACTION_TYPE = 'cellAction-filter'; export const COPY_CELL_ACTION_TYPE = 'cellAction-copy'; @@ -14,3 +16,10 @@ export enum CellActionsMode { HOVER_RIGHT = 'hover-right', INLINE = 'inline', } + +export const SUPPORTED_KBN_TYPES = [ + KBN_FIELD_TYPES.DATE, + KBN_FIELD_TYPES.IP, + KBN_FIELD_TYPES.STRING, + KBN_FIELD_TYPES.NUMBER, // Currently supported by casting https://github.com/elastic/kibana/issues/159298 +]; diff --git a/packages/kbn-cell-actions/src/context/cell_actions_context.tsx b/packages/kbn-cell-actions/src/context/cell_actions_context.tsx index af47a2fbe0ff7..4c43a9917ee26 100644 --- a/packages/kbn-cell-actions/src/context/cell_actions_context.tsx +++ b/packages/kbn-cell-actions/src/context/cell_actions_context.tsx @@ -7,26 +7,30 @@ */ import { orderBy } from 'lodash/fp'; -import React, { createContext, FC, useCallback, useContext } from 'react'; +import React, { createContext, FC, useContext, useMemo } from 'react'; import type { CellAction, CellActionsProviderProps, GetActions } from '../types'; -const CellActionsContext = createContext<{ getActions: GetActions } | null>(null); +interface CellActionsContextValue { + getActions: GetActions; +} +const CellActionsContext = createContext(null); export const CellActionsProvider: FC = ({ children, getTriggerCompatibleActions, }) => { - const getActions = useCallback( - (context) => - getTriggerCompatibleActions(context.trigger.id, context).then((actions) => - orderBy(['order', 'id'], ['asc', 'asc'], actions) - ) as Promise, + const value = useMemo( + () => ({ + getActions: (context) => + getTriggerCompatibleActions(context.trigger.id, context).then((actions) => + orderBy(['order', 'id'], ['asc', 'asc'], actions) + ) as Promise, + }), [getTriggerCompatibleActions] ); - return ( - {children} - ); + // make sure that provider's value does not change + return {children}; }; export const useCellActionsContext = () => { diff --git a/packages/kbn-cell-actions/src/hooks/use_data_grid_column_cell_actions.test.tsx b/packages/kbn-cell-actions/src/hooks/use_data_grid_column_cell_actions.test.tsx index 625cd3406dd75..136ca441588f4 100644 --- a/packages/kbn-cell-actions/src/hooks/use_data_grid_column_cell_actions.test.tsx +++ b/packages/kbn-cell-actions/src/hooks/use_data_grid_column_cell_actions.test.tsx @@ -31,30 +31,21 @@ const mockGetActions = jest.fn(async () => actions); jest.mock('../context/cell_actions_context', () => ({ useCellActionsContext: () => ({ getActions: mockGetActions }), })); -const values1 = ['0.0', '0.1', '0.2', '0.3']; -const field1 = { - name: 'column1', - type: 'string', - searchable: true, - aggregatable: true, -}; - -const values2 = ['1.0', '1.1', '1.2', '1.3']; -const field2 = { - name: 'column2', - - type: 'string', - searchable: true, - aggregatable: true, +const fieldValues: Record = { + column1: ['0.0', '0.1', '0.2', '0.3'], + column2: ['1.0', '1.1', '1.2', '1.3'], }; +const mockGetCellValue = jest.fn( + (field: string, rowIndex: number) => fieldValues[field]?.[rowIndex % fieldValues[field].length] +); +const field1 = { name: 'column1', type: 'text', searchable: true, aggregatable: true }; +const field2 = { name: 'column2', type: 'keyword', searchable: true, aggregatable: true }; const columns = [{ id: field1.name }, { id: field2.name }]; const mockCloseCellPopover = jest.fn(); const useDataGridColumnsCellActionsProps: UseDataGridColumnsCellActionsProps = { - data: [ - { field: field1, values: values1 }, - { field: field2, values: values2 }, - ], + fields: [field1, field2], + getCellValue: mockGetCellValue, triggerId: 'testTriggerId', metadata: { some: 'value' }, dataGridRef: { @@ -101,13 +92,31 @@ describe('useDataGridColumnsCellActions', () => { const { result } = renderHook(useDataGridColumnsCellActions, { initialProps: useDataGridColumnsCellActionsProps, }); - await act(async () => { const cellAction = renderCellAction(result.current[0][0]); expect(cellAction.getByTestId('dataGridColumnCellAction-loading')).toBeInTheDocument(); }); }); + it('should call getCellValue with the proper params', async () => { + const { result, waitForNextUpdate } = renderHook(useDataGridColumnsCellActions, { + initialProps: useDataGridColumnsCellActionsProps, + }); + + await waitForNextUpdate(); + + renderCellAction(result.current[0][0], { rowIndex: 0 }); + renderCellAction(result.current[0][1], { rowIndex: 1 }); + renderCellAction(result.current[1][0], { rowIndex: 0 }); + renderCellAction(result.current[1][1], { rowIndex: 1 }); + + expect(mockGetCellValue).toHaveBeenCalledTimes(4); + expect(mockGetCellValue).toHaveBeenCalledWith(field1.name, 0); + expect(mockGetCellValue).toHaveBeenCalledWith(field1.name, 1); + expect(mockGetCellValue).toHaveBeenCalledWith(field2.name, 0); + expect(mockGetCellValue).toHaveBeenCalledWith(field2.name, 1); + }); + it('should render the cell actions', async () => { const { result, waitForNextUpdate } = renderHook(useDataGridColumnsCellActions, { initialProps: useDataGridColumnsCellActionsProps, @@ -156,7 +165,7 @@ describe('useDataGridColumnsCellActions', () => { expect.objectContaining({ data: [ { - value: values1[1], + value: fieldValues[field1.name][1], field: { name: field1.name, type: field1.type, @@ -179,7 +188,7 @@ describe('useDataGridColumnsCellActions', () => { expect.objectContaining({ data: [ { - value: values2[2], + value: fieldValues[field2.name][2], field: { name: field2.name, type: field2.type, @@ -204,12 +213,14 @@ describe('useDataGridColumnsCellActions', () => { cellAction.getByTestId(`dataGridColumnCellAction-${action1.id}`).click(); + expect(mockGetCellValue).toHaveBeenCalledWith(field1.name, 25); + await waitFor(() => { expect(action1.execute).toHaveBeenCalledWith( expect.objectContaining({ data: [ { - value: values1[1], + value: fieldValues[field1.name][1], field: { name: field1.name, type: field1.type, @@ -242,7 +253,7 @@ describe('useDataGridColumnsCellActions', () => { const { result, waitForNextUpdate } = renderHook(useDataGridColumnsCellActions, { initialProps: { ...useDataGridColumnsCellActionsProps, - data: [], + fields: [], }, }); @@ -256,7 +267,7 @@ describe('useDataGridColumnsCellActions', () => { const { result, waitForNextUpdate } = renderHook(useDataGridColumnsCellActions, { initialProps: { ...useDataGridColumnsCellActionsProps, - data: undefined, + fields: undefined, }, }); diff --git a/packages/kbn-cell-actions/src/hooks/use_data_grid_column_cell_actions.tsx b/packages/kbn-cell-actions/src/hooks/use_data_grid_column_cell_actions.tsx index 3a0de0cc90eb1..5877added643f 100644 --- a/packages/kbn-cell-actions/src/hooks/use_data_grid_column_cell_actions.tsx +++ b/packages/kbn-cell-actions/src/hooks/use_data_grid_column_cell_actions.tsx @@ -12,47 +12,66 @@ import { EuiLoadingSpinner, type EuiDataGridColumnCellAction, } from '@elastic/eui'; +import { FieldSpec } from '@kbn/data-views-plugin/common'; import type { CellAction, CellActionCompatibilityContext, CellActionExecutionContext, - CellActionsData, CellActionsProps, + CellActionFieldValue, } from '../types'; import { useBulkLoadActions } from './use_load_actions'; -interface BulkData extends Omit { +export interface UseDataGridColumnsCellActionsProps + extends Pick { /** - * Array containing all the values of the field in the visible page, indexed by rowIndex + * Optional trigger ID to used to retrieve the cell actions. + * returns empty array if not provided + */ + triggerId?: string; + /** + * fields array, used to determine which actions to load. + * returns empty array if not provided + */ + fields?: FieldSpec[]; + /** + * Function to get the cell value for a given field name and row index. + * the `rowIndex` parameter is absolute, not relative to the current page + */ + getCellValue: (fieldName: string, rowIndex: number) => CellActionFieldValue; + /** + * ref to the EuiDataGrid instance */ - values: Array; -} - -export interface UseDataGridColumnsCellActionsProps - extends Pick { - data?: BulkData[]; dataGridRef: MutableRefObject; } export type UseDataGridColumnsCellActions< P extends UseDataGridColumnsCellActionsProps = UseDataGridColumnsCellActionsProps > = (props: P) => EuiDataGridColumnCellAction[][]; +// static actions array references to prevent React updates +const loadingColumnActions: EuiDataGridColumnCellAction[] = [ + () => , +]; +const emptyActions: EuiDataGridColumnCellAction[][] = []; + export const useDataGridColumnsCellActions: UseDataGridColumnsCellActions = ({ - data, + fields, + getCellValue, triggerId, metadata, dataGridRef, disabledActionTypes = [], }) => { - const bulkContexts: CellActionCompatibilityContext[] = useMemo( - () => - data?.map(({ field }) => ({ - data: [{ field }], // we are getting the actions for the whole column field, so the compatibility check will be done without the value - trigger: { id: triggerId }, - metadata, - })) ?? [], - [triggerId, metadata, data] - ); + const bulkContexts: CellActionCompatibilityContext[] | undefined = useMemo(() => { + if (!triggerId || !fields?.length) { + return undefined; + } + return fields.map((field) => ({ + data: [{ field }], + trigger: { id: triggerId }, + metadata, + })); + }, [fields, triggerId, metadata]); const { loading, value: columnsActions } = useBulkLoadActions(bulkContexts, { disabledActionTypes, @@ -60,46 +79,45 @@ export const useDataGridColumnsCellActions: UseDataGridColumnsCellActions = ({ const columnsCellActions = useMemo(() => { if (loading) { - return ( - data?.map(() => [ - () => , - ]) ?? [] - ); + return fields?.length ? fields.map(() => loadingColumnActions) : emptyActions; } - if (!columnsActions || !data || data.length === 0) { - return []; + if (!triggerId || !columnsActions?.length || !fields?.length) { + return emptyActions; } // Check for a temporary inconsistency because `useBulkLoadActions` takes one render loop before setting `loading` to true. // It will eventually update to a consistent state - if (columnsActions.length !== data.length) { - return []; + if (columnsActions.length !== fields.length) { + return emptyActions; } return columnsActions.map((actions, columnIndex) => actions.map((action) => createColumnCellAction({ action, + field: fields[columnIndex], + getCellValue, metadata, triggerId, - data: data[columnIndex], dataGridRef, }) ) ); - }, [loading, columnsActions, data, metadata, triggerId, dataGridRef]); + }, [columnsActions, fields, getCellValue, loading, metadata, triggerId, dataGridRef]); return columnsCellActions; }; interface CreateColumnCellActionParams - extends Pick { - data: BulkData; + extends Pick { + field: FieldSpec; + triggerId: string; action: CellAction; } const createColumnCellAction = ({ - data: { field, values }, action, + field, + getCellValue, metadata, triggerId, dataGridRef, @@ -109,8 +127,8 @@ const createColumnCellAction = ({ const buttonRef = useRef(null); const actionContext: CellActionExecutionContext = useMemo(() => { - // rowIndex refers to all pages, we need to use the row index relative to the page to get the value - const value = values[rowIndex % values.length]; + const { name } = field; + const value = getCellValue(name, rowIndex); return { data: [ { diff --git a/packages/kbn-cell-actions/src/hooks/use_load_actions.test.ts b/packages/kbn-cell-actions/src/hooks/use_load_actions.test.ts index 11ec2f0984199..9a2377decf8d6 100644 --- a/packages/kbn-cell-actions/src/hooks/use_load_actions.test.ts +++ b/packages/kbn-cell-actions/src/hooks/use_load_actions.test.ts @@ -199,5 +199,24 @@ describe('loadActions hooks', () => { expect(result.current.value).toHaveLength(0); expect(result.current.loading).toBe(false); }); + + it('should return the same array after re-render when contexts is undefined', async () => { + const { result, rerender, waitFor } = renderHook(useBulkLoadActions, { + initialProps: undefined, + }); + + await waitFor(() => expect(result.current.value).toEqual([])); + expect(result.current.loading).toBe(false); + expect(mockGetActions).not.toHaveBeenCalled(); + + const initialResultValue = result.current.value; + + rerender(undefined); + + await waitFor(() => expect(result.current.value).toEqual([])); + expect(result.current.value).toBe(initialResultValue); + expect(result.current.loading).toBe(false); + expect(mockGetActions).not.toHaveBeenCalled(); + }); }); }); diff --git a/packages/kbn-cell-actions/src/hooks/use_load_actions.ts b/packages/kbn-cell-actions/src/hooks/use_load_actions.ts index 6eece48655be3..f32f9c5f2df9a 100644 --- a/packages/kbn-cell-actions/src/hooks/use_load_actions.ts +++ b/packages/kbn-cell-actions/src/hooks/use_load_actions.ts @@ -53,18 +53,18 @@ interface LoadActionsOptions { * Groups getActions calls for an array of contexts in one async bulk operation */ export const useBulkLoadActions = ( - contexts: CellActionCompatibilityContext[], + contexts: CellActionCompatibilityContext[] | undefined, options: LoadActionsOptions = {} ): AsyncActions => { const { getActions } = useCellActionsContext(); const { error, ...actionsState } = useAsync( () => Promise.all( - contexts.map((context) => + contexts?.map((context) => getActions(context).then( (actions) => filteredActions(actions, options.disabledActionTypes) ?? [] ) - ) + ) ?? [] ), [contexts] ); diff --git a/packages/kbn-cell-actions/src/index.ts b/packages/kbn-cell-actions/src/index.ts index 7c13b97adf276..dfd1d83937c0f 100644 --- a/packages/kbn-cell-actions/src/index.ts +++ b/packages/kbn-cell-actions/src/index.ts @@ -9,6 +9,7 @@ // Types and enums export type { CellAction, + CellActionFieldValue, CellActionsProps, CellActionExecutionContext, CellActionCompatibilityContext, diff --git a/packages/kbn-cell-actions/src/utils.test.ts b/packages/kbn-cell-actions/src/utils.test.ts new file mode 100644 index 0000000000000..df941a495d73e --- /dev/null +++ b/packages/kbn-cell-actions/src/utils.test.ts @@ -0,0 +1,36 @@ +/* + * 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. + */ + +import { KBN_FIELD_TYPES } from '@kbn/field-types'; +import { isTypeSupportedByCellActions } from './utils'; + +describe('isTypeSupportedByCellActions', () => { + it('returns true if the type is number', () => { + expect(isTypeSupportedByCellActions(KBN_FIELD_TYPES.NUMBER)).toBe(true); + }); + + it('returns true if the type is string', () => { + expect(isTypeSupportedByCellActions(KBN_FIELD_TYPES.STRING)).toBe(true); + }); + + it('returns true if the type is ip', () => { + expect(isTypeSupportedByCellActions(KBN_FIELD_TYPES.IP)).toBe(true); + }); + + it('returns true if the type is date', () => { + expect(isTypeSupportedByCellActions(KBN_FIELD_TYPES.DATE)).toBe(true); + }); + + it('returns false if the type is boolean', () => { + expect(isTypeSupportedByCellActions(KBN_FIELD_TYPES.BOOLEAN)).toBe(false); + }); + + it('returns false if the type is unknown', () => { + expect(isTypeSupportedByCellActions(KBN_FIELD_TYPES.BOOLEAN)).toBe(false); + }); +}); diff --git a/packages/kbn-cell-actions/src/utils.ts b/packages/kbn-cell-actions/src/utils.ts new file mode 100644 index 0000000000000..ef0a7c8dfb004 --- /dev/null +++ b/packages/kbn-cell-actions/src/utils.ts @@ -0,0 +1,13 @@ +/* + * 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. + */ + +import { KBN_FIELD_TYPES } from '@kbn/field-types'; +import { SUPPORTED_KBN_TYPES } from './constants'; + +export const isTypeSupportedByCellActions = (kbnFieldType: KBN_FIELD_TYPES) => + SUPPORTED_KBN_TYPES.includes(kbnFieldType); diff --git a/packages/kbn-cell-actions/tsconfig.json b/packages/kbn-cell-actions/tsconfig.json index b2d41295e44e6..c504b32bac30d 100644 --- a/packages/kbn-cell-actions/tsconfig.json +++ b/packages/kbn-cell-actions/tsconfig.json @@ -19,6 +19,7 @@ "@kbn/data-plugin", "@kbn/es-query", "@kbn/ui-actions-plugin", + "@kbn/field-types", "@kbn/data-views-plugin", ], "exclude": ["target/**/*"] diff --git a/src/plugins/discover/public/__mocks__/data_view.ts b/src/plugins/discover/public/__mocks__/data_view.ts index 67950cd9f5e15..d660eef2b9b38 100644 --- a/src/plugins/discover/public/__mocks__/data_view.ts +++ b/src/plugins/discover/public/__mocks__/data_view.ts @@ -8,7 +8,7 @@ import { DataView } from '@kbn/data-views-plugin/public'; -const fields = [ +export const fields = [ { name: '_source', type: '_source', diff --git a/src/plugins/discover/public/__mocks__/services.ts b/src/plugins/discover/public/__mocks__/services.ts index 77aa943a8f5fd..764b1dcd566ba 100644 --- a/src/plugins/discover/public/__mocks__/services.ts +++ b/src/plugins/discover/public/__mocks__/services.ts @@ -9,6 +9,7 @@ import { Observable, of } from 'rxjs'; import { EUI_CHARTS_THEME_LIGHT } from '@elastic/eui/dist/eui_charts_theme'; import { DiscoverServices } from '../build_services'; import { dataPluginMock } from '@kbn/data-plugin/public/mocks'; +import { uiActionsPluginMock } from '@kbn/ui-actions-plugin/public/mocks'; import { expressionsPluginMock } from '@kbn/expressions-plugin/public/mocks'; import { savedSearchPluginMock } from '@kbn/saved-search-plugin/public/mocks'; import { chromeServiceMock, coreMock, docLinksServiceMock } from '@kbn/core/public/mocks'; @@ -120,6 +121,7 @@ export function createDiscoverServicesMock(): DiscoverServices { inspector: { open: jest.fn(), }, + uiActions: uiActionsPluginMock.createStartContract(), uiSettings: { get: jest.fn((key: string) => { if (key === 'fields:popularLimit') { diff --git a/src/plugins/discover/public/application/context/context_app_content.test.tsx b/src/plugins/discover/public/application/context/context_app_content.test.tsx index dc937d329c415..c4c22fc472bd4 100644 --- a/src/plugins/discover/public/application/context/context_app_content.test.tsx +++ b/src/plugins/discover/public/application/context/context_app_content.test.tsx @@ -15,14 +15,15 @@ import { SortDirection } from '@kbn/data-plugin/public'; import { ContextAppContent, ContextAppContentProps } from './context_app_content'; import { LoadingStatus } from './services/context_query_state'; import { dataViewMock } from '../../__mocks__/data_view'; -import { DiscoverGrid } from '../../components/discover_grid/discover_grid'; import { discoverServiceMock } from '../../__mocks__/services'; +import { DiscoverGrid } from '../../components/discover_grid/discover_grid'; import { DocTableWrapper } from '../../components/doc_table/doc_table_wrapper'; import { KibanaContextProvider } from '@kbn/kibana-react-plugin/public'; import { buildDataTableRecord } from '../../utils/build_data_record'; +import { act } from 'react-dom/test-utils'; describe('ContextAppContent test', () => { - const mountComponent = ({ + const mountComponent = async ({ anchorStatus, isLegacy, }: { @@ -73,30 +74,35 @@ describe('ContextAppContent test', () => { addFilter: () => {}, } as unknown as ContextAppContentProps; - return mountWithIntl( + const component = mountWithIntl( ); + await act(async () => { + // needed by cell actions to complete async loading + component.update(); + }); + return component; }; - it('should render legacy table correctly', () => { - const component = mountComponent({}); + it('should render legacy table correctly', async () => { + const component = await mountComponent({}); expect(component.find(DocTableWrapper).length).toBe(1); const loadingIndicator = findTestSubject(component, 'contextApp_loadingIndicator'); expect(loadingIndicator.length).toBe(0); expect(component.find(ActionBar).length).toBe(2); }); - it('renders loading indicator', () => { - const component = mountComponent({ anchorStatus: LoadingStatus.LOADING }); + it('renders loading indicator', async () => { + const component = await mountComponent({ anchorStatus: LoadingStatus.LOADING }); const loadingIndicator = findTestSubject(component, 'contextApp_loadingIndicator'); expect(component.find(DocTableWrapper).length).toBe(1); expect(loadingIndicator.length).toBe(1); }); - it('should render discover grid correctly', () => { - const component = mountComponent({ isLegacy: false }); + it('should render discover grid correctly', async () => { + const component = await mountComponent({ isLegacy: false }); expect(component.find(DiscoverGrid).length).toBe(1); }); }); diff --git a/src/plugins/discover/public/application/context/context_app_content.tsx b/src/plugins/discover/public/application/context/context_app_content.tsx index 6b65bec06bdcf..62c3a0e1ca863 100644 --- a/src/plugins/discover/public/application/context/context_app_content.tsx +++ b/src/plugins/discover/public/application/context/context_app_content.tsx @@ -12,6 +12,7 @@ import { EuiHorizontalRule, EuiText } from '@elastic/eui'; import type { DataView } from '@kbn/data-views-plugin/public'; import { SortDirection } from '@kbn/data-plugin/public'; import type { SortOrder } from '@kbn/saved-search-plugin/public'; +import { CellActionsProvider } from '@kbn/cell-actions'; import { CONTEXT_STEP_SETTING, DOC_HIDE_TIME_COLUMN_SETTING } from '../../../common'; import { LoadingStatus } from './services/context_query_state'; import { ActionBar } from './components/action_bar/action_bar'; @@ -75,7 +76,7 @@ export function ContextAppContent({ setAppState, addFilter, }: ContextAppContentProps) { - const { uiSettings: config } = useDiscoverServices(); + const { uiSettings: config, uiActions } = useDiscoverServices(); const services = useDiscoverServices(); const [expandedDoc, setExpandedDoc] = useState(); @@ -145,28 +146,30 @@ export function ContextAppContent({ )} {!isLegacy && (
- + + +
)} diff --git a/src/plugins/discover/public/application/main/components/layout/discover_documents.test.tsx b/src/plugins/discover/public/application/main/components/layout/discover_documents.test.tsx index 1360a49676895..39fa270a8c9b7 100644 --- a/src/plugins/discover/public/application/main/components/layout/discover_documents.test.tsx +++ b/src/plugins/discover/public/application/main/components/layout/discover_documents.test.tsx @@ -7,6 +7,7 @@ */ import React from 'react'; +import { act } from 'react-dom/test-utils'; import { BehaviorSubject } from 'rxjs'; import { mountWithIntl } from '@kbn/test-jest-helpers'; import { setHeaderActionMenuMounter } from '../../../../kibana_services'; @@ -25,7 +26,7 @@ import { DiscoverAppState } from '../../services/discover_app_state_container'; setHeaderActionMenuMounter(jest.fn()); -function mountComponent(fetchStatus: FetchStatus, hits: EsHitRecord[]) { +async function mountComponent(fetchStatus: FetchStatus, hits: EsHitRecord[]) { const services = discoverServiceMock; services.data.query.timefilter.timefilter.getTime = () => { return { from: '2020-05-14T11:05:13.590', to: '2020-05-14T11:20:13.590' }; @@ -46,30 +47,34 @@ function mountComponent(fetchStatus: FetchStatus, hits: EsHitRecord[]) { onFieldEdited: jest.fn(), }; - return mountWithIntl( + const component = mountWithIntl( ); + await act(async () => { + component.update(); + }); + return component; } describe('Discover documents layout', () => { - test('render loading when loading and no documents', () => { - const component = mountComponent(FetchStatus.LOADING, []); + test('render loading when loading and no documents', async () => { + const component = await mountComponent(FetchStatus.LOADING, []); expect(component.find('.dscDocuments__loading').exists()).toBeTruthy(); expect(component.find('.dscTable').exists()).toBeFalsy(); }); - test('render complete when loading but documents were already fetched', () => { - const component = mountComponent(FetchStatus.LOADING, esHits); + test('render complete when loading but documents were already fetched', async () => { + const component = await mountComponent(FetchStatus.LOADING, esHits); expect(component.find('.dscDocuments__loading').exists()).toBeFalsy(); expect(component.find('.dscTable').exists()).toBeTruthy(); }); - test('render complete', () => { - const component = mountComponent(FetchStatus.COMPLETE, esHits); + test('render complete', async () => { + const component = await mountComponent(FetchStatus.COMPLETE, esHits); expect(component.find('.dscDocuments__loading').exists()).toBeFalsy(); expect(component.find('.dscTable').exists()).toBeTruthy(); }); diff --git a/src/plugins/discover/public/application/main/components/layout/discover_documents.tsx b/src/plugins/discover/public/application/main/components/layout/discover_documents.tsx index 64f857d0d9aed..65e2099984384 100644 --- a/src/plugins/discover/public/application/main/components/layout/discover_documents.tsx +++ b/src/plugins/discover/public/application/main/components/layout/discover_documents.tsx @@ -18,6 +18,7 @@ import { FormattedMessage } from '@kbn/i18n-react'; import { css } from '@emotion/react'; import { DataView } from '@kbn/data-views-plugin/public'; import { SortOrder } from '@kbn/saved-search-plugin/public'; +import { CellActionsProvider } from '@kbn/cell-actions'; import { useInternalStateSelector } from '../../services/discover_internal_state_container'; import { useAppStateSelector } from '../../services/discover_app_state_container'; import { useDiscoverServices } from '../../../../hooks/use_discover_services'; @@ -85,7 +86,7 @@ function DiscoverDocumentsComponent({ const services = useDiscoverServices(); const documents$ = stateContainer.dataState.data$.documents$; const savedSearch = useSavedSearchInitial(); - const { dataViews, capabilities, uiSettings } = services; + const { dataViews, capabilities, uiSettings, uiActions } = services; const [query, sort, rowHeight, rowsPerPage, grid, columns, index] = useAppStateSelector( (state) => { return [ @@ -228,39 +229,43 @@ function DiscoverDocumentsComponent({ )}
- + + +
)} diff --git a/src/plugins/discover/public/application/main/components/layout/discover_histogram_layout.test.tsx b/src/plugins/discover/public/application/main/components/layout/discover_histogram_layout.test.tsx index ce7c0c9460f73..829bde038f15a 100644 --- a/src/plugins/discover/public/application/main/components/layout/discover_histogram_layout.test.tsx +++ b/src/plugins/discover/public/application/main/components/layout/discover_histogram_layout.test.tsx @@ -21,7 +21,8 @@ import { } from '../../services/discover_data_state_container'; import { discoverServiceMock } from '../../../../__mocks__/services'; import { FetchStatus } from '../../../types'; -import { KibanaContextProvider, KibanaThemeProvider } from '@kbn/kibana-react-plugin/public'; +import { KibanaThemeProvider } from '@kbn/kibana-react-plugin/public'; +import { KibanaContextProvider } from '@kbn/kibana-react-plugin/public'; import { buildDataTableRecord } from '../../../../utils/build_data_record'; import { DiscoverHistogramLayout, DiscoverHistogramLayoutProps } from './discover_histogram_layout'; import { SavedSearch, VIEW_MODE } from '@kbn/saved-search-plugin/public'; @@ -141,7 +142,9 @@ const mountComponent = async ({ // wait for lazy modules await act(() => new Promise((resolve) => setTimeout(resolve, 0))); - component.update(); + await act(async () => { + component.update(); + }); return { component, stateContainer }; }; diff --git a/src/plugins/discover/public/application/main/components/layout/discover_main_content.test.tsx b/src/plugins/discover/public/application/main/components/layout/discover_main_content.test.tsx index 5ab1a6e4d6e3b..1ca0e6124006a 100644 --- a/src/plugins/discover/public/application/main/components/layout/discover_main_content.test.tsx +++ b/src/plugins/discover/public/application/main/components/layout/discover_main_content.test.tsx @@ -8,6 +8,7 @@ import React from 'react'; import { BehaviorSubject, of } from 'rxjs'; +import { act } from 'react-dom/test-utils'; import { mountWithIntl } from '@kbn/test-jest-helpers'; import { esHits } from '../../../../__mocks__/es_hits'; import { dataViewMock } from '../../../../__mocks__/data_view'; @@ -20,7 +21,8 @@ import { } from '../../services/discover_data_state_container'; import { createDiscoverServicesMock } from '../../../../__mocks__/services'; import { FetchStatus } from '../../../types'; -import { KibanaContextProvider, KibanaThemeProvider } from '@kbn/kibana-react-plugin/public'; +import { KibanaThemeProvider } from '@kbn/kibana-react-plugin/public'; +import { KibanaContextProvider } from '@kbn/kibana-react-plugin/public'; import { buildDataTableRecord } from '../../../../utils/build_data_record'; import { DiscoverMainContent, DiscoverMainContentProps } from './discover_main_content'; import { SavedSearch, VIEW_MODE } from '@kbn/saved-search-plugin/public'; @@ -33,7 +35,7 @@ import { DiscoverMainProvider } from '../../services/discover_state_provider'; import { getDiscoverStateMock } from '../../../../__mocks__/discover_state.mock'; import type { Storage } from '@kbn/kibana-utils-plugin/public'; -const mountComponent = ({ +const mountComponent = async ({ hideChart = false, isPlainRecord = false, viewMode = VIEW_MODE.DOCUMENT_LEVEL, @@ -116,31 +118,35 @@ const mountComponent = ({ ); + await act(async () => { + component.update(); + }); + return component; }; describe('Discover main content component', () => { describe('DocumentViewModeToggle', () => { it('should show DocumentViewModeToggle when isPlainRecord is false', async () => { - const component = mountComponent(); + const component = await mountComponent(); expect(component.find(DocumentViewModeToggle).exists()).toBe(true); }); it('should not show DocumentViewModeToggle when isPlainRecord is true', async () => { - const component = mountComponent({ isPlainRecord: true }); + const component = await mountComponent({ isPlainRecord: true }); expect(component.find(DocumentViewModeToggle).exists()).toBe(false); }); }); describe('Document view', () => { it('should show DiscoverDocuments when VIEW_MODE is DOCUMENT_LEVEL', async () => { - const component = mountComponent(); + const component = await mountComponent(); expect(component.find(DiscoverDocuments).exists()).toBe(true); expect(component.find(FieldStatisticsTab).exists()).toBe(false); }); it('should show FieldStatisticsTableMemoized when VIEW_MODE is not DOCUMENT_LEVEL', async () => { - const component = mountComponent({ viewMode: VIEW_MODE.AGGREGATED_LEVEL }); + const component = await mountComponent({ viewMode: VIEW_MODE.AGGREGATED_LEVEL }); expect(component.find(DiscoverDocuments).exists()).toBe(false); expect(component.find(FieldStatisticsTab).exists()).toBe(true); }); diff --git a/src/plugins/discover/public/build_services.ts b/src/plugins/discover/public/build_services.ts index f3cda1707d4a9..6390dc9a07871 100644 --- a/src/plugins/discover/public/build_services.ts +++ b/src/plugins/discover/public/build_services.ts @@ -48,6 +48,7 @@ import type { SavedObjectsTaggingApi } from '@kbn/saved-objects-tagging-oss-plug import type { SavedObjectsManagementPluginStart } from '@kbn/saved-objects-management-plugin/public'; import type { UnifiedSearchPublicPluginStart } from '@kbn/unified-search-plugin/public'; import type { LensPublicStart } from '@kbn/lens-plugin/public'; +import type { UiActionsStart } from '@kbn/ui-actions-plugin/public'; import type { SettingsStart } from '@kbn/core-ui-settings-browser'; import { getHistory } from './kibana_services'; import { DiscoverStartPlugins } from './plugin'; @@ -103,6 +104,7 @@ export interface DiscoverServices { savedSearch: SavedSearchPublicPluginStart; unifiedSearch: UnifiedSearchPublicPluginStart; lens: LensPublicStart; + uiActions: UiActionsStart; } export const buildServices = memoize(function ( @@ -159,5 +161,6 @@ export const buildServices = memoize(function ( savedSearch: plugins.savedSearch, unifiedSearch: plugins.unifiedSearch, lens: plugins.lens, + uiActions: plugins.uiActions, }; }); diff --git a/src/plugins/discover/public/components/discover_grid/discover_grid.test.tsx b/src/plugins/discover/public/components/discover_grid/discover_grid.test.tsx index 33f507b43333b..c6d0bb4e0f48b 100644 --- a/src/plugins/discover/public/components/discover_grid/discover_grid.test.tsx +++ b/src/plugins/discover/public/components/discover_grid/discover_grid.test.tsx @@ -11,7 +11,7 @@ import { EuiCopy } from '@elastic/eui'; import { act } from 'react-dom/test-utils'; import { findTestSubject } from '@elastic/eui/lib/test'; import { esHits } from '../../__mocks__/es_hits'; -import { dataViewMock } from '../../__mocks__/data_view'; +import { buildDataViewMock, fields } from '../../__mocks__/data_view'; import { mountWithIntl } from '@kbn/test-jest-helpers'; import { DiscoverGrid, DiscoverGridProps } from './discover_grid'; import { KibanaContextProvider } from '@kbn/kibana-react-plugin/public'; @@ -20,6 +20,18 @@ import { buildDataTableRecord } from '../../utils/build_data_record'; import { getDocId } from '../../utils/get_doc_id'; import { EsHitRecord } from '../../types'; +const mockUseDataGridColumnsCellActions = jest.fn((prop: unknown) => []); +jest.mock('@kbn/cell-actions', () => ({ + ...jest.requireActual('@kbn/cell-actions'), + useDataGridColumnsCellActions: (prop: unknown) => mockUseDataGridColumnsCellActions(prop), +})); + +export const dataViewMock = buildDataViewMock({ + name: 'the-data-view', + fields, + timeFieldName: '@timestamp', +}); + function getProps() { const services = discoverServiceMock; services.dataViewFieldEditor.userPermissions.editIndexPattern = jest.fn().mockReturnValue(true); @@ -49,14 +61,19 @@ function getProps() { }; } -function getComponent(props: DiscoverGridProps = getProps()) { +async function getComponent(props: DiscoverGridProps = getProps()) { const Proxy = (innerProps: DiscoverGridProps) => ( ); - return mountWithIntl(); + const component = mountWithIntl(); + await act(async () => { + // needed by cell actions to complete async loading + component.update(); + }); + return component; } function getSelectedDocNr(component: ReactWrapper) { @@ -89,10 +106,14 @@ async function toggleDocSelection( } describe('DiscoverGrid', () => { + afterEach(async () => { + jest.clearAllMocks(); + }); + describe('Document selection', () => { let component: ReactWrapper; - beforeEach(() => { - component = getComponent(); + beforeEach(async () => { + component = await getComponent(); }); test('no documents are selected initially', async () => { @@ -178,8 +199,8 @@ describe('DiscoverGrid', () => { }); describe('edit field button', () => { - it('should render the edit field button if onFieldEdited is provided', () => { - const component = getComponent({ + it('should render the edit field button if onFieldEdited is provided', async () => { + const component = await getComponent({ ...getProps(), columns: ['message'], onFieldEdited: jest.fn(), @@ -194,8 +215,8 @@ describe('DiscoverGrid', () => { expect(findTestSubject(component, 'gridEditFieldButton').exists()).toBe(true); }); - it('should not render the edit field button if onFieldEdited is not provided', () => { - const component = getComponent({ + it('should not render the edit field button if onFieldEdited is not provided', async () => { + const component = await getComponent({ ...getProps(), columns: ['message'], }); @@ -210,9 +231,91 @@ describe('DiscoverGrid', () => { }); }); + describe('cellActionsTriggerId', () => { + it('should call useDataGridColumnsCellActions with empty params when no cellActionsTriggerId is provided', async () => { + await getComponent({ + ...getProps(), + columns: ['message'], + onFieldEdited: jest.fn(), + }); + expect(mockUseDataGridColumnsCellActions).toHaveBeenCalledWith( + expect.objectContaining({ + triggerId: undefined, + getCellValue: expect.any(Function), + fields: undefined, + }) + ); + }); + + it('should call useDataGridColumnsCellActions properly when cellActionsTriggerId defined', async () => { + await getComponent({ + ...getProps(), + columns: ['message'], + onFieldEdited: jest.fn(), + cellActionsTriggerId: 'test', + }); + expect(mockUseDataGridColumnsCellActions).toHaveBeenCalledWith( + expect.objectContaining({ + triggerId: 'test', + getCellValue: expect.any(Function), + fields: [ + { + name: '@timestamp', + type: 'date', + aggregatable: true, + searchable: undefined, + }, + { + name: 'message', + type: 'string', + aggregatable: false, + searchable: undefined, + }, + ], + }) + ); + }); + + it('should call useDataGridColumnsCellActions with empty field name and type for unsupported field types', async () => { + await getComponent({ + ...getProps(), + columns: ['message', '_source'], + onFieldEdited: jest.fn(), + cellActionsTriggerId: 'test', + }); + + expect(mockUseDataGridColumnsCellActions).toHaveBeenCalledWith( + expect.objectContaining({ + triggerId: 'test', + getCellValue: expect.any(Function), + fields: [ + { + name: '@timestamp', + type: 'date', + aggregatable: true, + searchable: undefined, + }, + { + name: 'message', + type: 'string', + aggregatable: false, + searchable: undefined, + }, + { + searchable: false, + aggregatable: false, + name: '', + type: '', + }, + ], + }) + ); + }); + }); + describe('sorting', () => { - it('should enable in memory sorting with plain records', () => { - const component = getComponent({ + it('should enable in memory sorting with plain records', async () => { + const component = await getComponent({ ...getProps(), columns: ['message'], isPlainRecord: true, diff --git a/src/plugins/discover/public/components/discover_grid/discover_grid.tsx b/src/plugins/discover/public/components/discover_grid/discover_grid.tsx index 69748b449b05c..3265584f37303 100644 --- a/src/plugins/discover/public/components/discover_grid/discover_grid.tsx +++ b/src/plugins/discover/public/components/discover_grid/discover_grid.tsx @@ -25,10 +25,17 @@ import { } from '@elastic/eui'; import type { DataView } from '@kbn/data-views-plugin/public'; import type { SortOrder } from '@kbn/saved-search-plugin/public'; +import { + useDataGridColumnsCellActions, + type UseDataGridColumnsCellActionsProps, + type CellActionFieldValue, +} from '@kbn/cell-actions'; import type { AggregateQuery, Filter, Query } from '@kbn/es-query'; import { FieldFormatsStart } from '@kbn/field-formats-plugin/public'; import type { ToastsStart, IUiSettingsClient, HttpStart, CoreStart } from '@kbn/core/public'; import { DataViewFieldEditorStart } from '@kbn/data-view-field-editor-plugin/public'; +import { KBN_FIELD_TYPES } from '@kbn/data-plugin/common'; +import { isTypeSupportedByCellActions } from '@kbn/cell-actions/src/utils'; import { DocViewFilterFn } from '../../services/doc_views/doc_views_types'; import { getSchemaDetectors } from './discover_grid_schema'; import { DiscoverGridFlyout } from './discover_grid_flyout'; @@ -53,6 +60,7 @@ import type { DataTableRecord, ValueToStringConverter } from '../../types'; import { useRowHeightsOptions } from '../../hooks/use_row_heights_options'; import { convertValueToString } from '../../utils/convert_value_to_string'; import { getRowsPerPageOptions, getDefaultRowsPerPage } from '../../utils/rows_per_page'; +import { convertCellActionValue } from './discover_grid_cell_actions'; const themeDefault = { darkMode: false }; @@ -203,6 +211,10 @@ export interface DiscoverGridProps { * Document detail view component */ DocumentView?: typeof DiscoverGridFlyout; + /** + * Optional triggerId to retrieve the column cell actions that will override the default ones + */ + cellActionsTriggerId?: string; /** * Service dependencies */ @@ -248,6 +260,7 @@ export const DiscoverGrid = ({ isSortEnabled = true, isPaginationEnabled = true, controlColumnIds = CONTROL_COLUMN_IDS_DEFAULT, + cellActionsTriggerId, className, rowHeightState, onUpdateRowHeight, @@ -432,14 +445,57 @@ export const DiscoverGrid = ({ [dataView, onFieldEdited, services.dataViewFieldEditor] ); + const visibleColumns = useMemo( + () => getVisibleColumns(displayedColumns, dataView, showTimeCol) as string[], + [dataView, displayedColumns, showTimeCol] + ); + + const getCellValue = useCallback( + (fieldName, rowIndex): CellActionFieldValue => + convertCellActionValue(displayedRows[rowIndex % displayedRows.length].flattened[fieldName]), + [displayedRows] + ); + + const cellActionsFields = useMemo( + () => + cellActionsTriggerId && !isPlainRecord + ? visibleColumns.map((columnName) => { + const field = dataView.getFieldByName(columnName); + if (!field || !isTypeSupportedByCellActions(field.type as KBN_FIELD_TYPES)) { + // disable custom actions on object columns + return { + name: '', + type: '', + aggregatable: false, + searchable: false, + }; + } + return { + name: columnName, + type: field.type, + aggregatable: field.aggregatable, + searchable: field.searchable, + }; + }) + : undefined, + [cellActionsTriggerId, isPlainRecord, visibleColumns, dataView] + ); + + const columnsCellActions = useDataGridColumnsCellActions({ + fields: cellActionsFields, + getCellValue, + triggerId: cellActionsTriggerId, + dataGridRef, + }); + const euiGridColumns = useMemo( () => getEuiGridColumns({ - columns: displayedColumns, + columns: visibleColumns, + columnsCellActions, rowsCount: displayedRows.length, settings, dataView, - showTimeCol, defaultColumns, isSortEnabled, isPlainRecord, @@ -454,10 +510,10 @@ export const DiscoverGrid = ({ }), [ onFilter, - displayedColumns, + visibleColumns, + columnsCellActions, displayedRows, dataView, - showTimeCol, settings, defaultColumns, isSortEnabled, @@ -477,12 +533,12 @@ export const DiscoverGrid = ({ const schemaDetectors = useMemo(() => getSchemaDetectors(), []); const columnsVisibility = useMemo( () => ({ - visibleColumns: getVisibleColumns(displayedColumns, dataView, showTimeCol) as string[], + visibleColumns, setVisibleColumns: (newColumns: string[]) => { onSetColumns(newColumns, hideTimeColumn); }, }), - [displayedColumns, dataView, showTimeCol, hideTimeColumn, onSetColumns] + [visibleColumns, hideTimeColumn, onSetColumns] ); const sorting = useMemo(() => { if (isSortEnabled) { diff --git a/src/plugins/discover/public/components/discover_grid/discover_grid_cell_actions.tsx b/src/plugins/discover/public/components/discover_grid/discover_grid_cell_actions.tsx index 59cd130277f90..f570851ea528c 100644 --- a/src/plugins/discover/public/components/discover_grid/discover_grid_cell_actions.tsx +++ b/src/plugins/discover/public/components/discover_grid/discover_grid_cell_actions.tsx @@ -10,6 +10,7 @@ import React, { useContext } from 'react'; import { EuiDataGridColumnCellActionProps } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { DataViewField } from '@kbn/data-views-plugin/public'; +import type { CellActionFieldValue } from '@kbn/cell-actions'; import { DocViewFilterFn } from '../../services/doc_views/doc_views_types'; import { DiscoverGridContext, GridContext } from './discover_grid_context'; import { useDiscoverServices } from '../../hooks/use_discover_services'; @@ -120,3 +121,12 @@ export const CopyBtn = ({ Component, rowIndex, columnId }: EuiDataGridColumnCell export function buildCellActions(field: DataViewField, onFilter?: DocViewFilterFn) { return [...(onFilter && field.filterable ? [FilterInBtn, FilterOutBtn] : []), CopyBtn]; } + +// Converts the cell action value to the type expected by CellActions component +export const convertCellActionValue = (rawValue: unknown): CellActionFieldValue => { + const value = rawValue as CellActionFieldValue | number | number[]; + if (Array.isArray(value)) { + return value.map((val) => (val != null ? val.toString() : val)); + } + return value != null ? value.toString() : value; +}; diff --git a/src/plugins/discover/public/components/discover_grid/discover_grid_columns.test.tsx b/src/plugins/discover/public/components/discover_grid/discover_grid_columns.test.tsx index c4c68bf0132d0..ea22d8b130835 100644 --- a/src/plugins/discover/public/components/discover_grid/discover_grid_columns.test.tsx +++ b/src/plugins/discover/public/components/discover_grid/discover_grid_columns.test.tsx @@ -7,152 +7,39 @@ */ import { dataViewMock } from '../../__mocks__/data_view'; -import { getEuiGridColumns } from './discover_grid_columns'; +import { getEuiGridColumns, getVisibleColumns } from './discover_grid_columns'; import { dataViewWithTimefieldMock } from '../../__mocks__/data_view_with_timefield'; import { discoverGridContextMock } from '../../__mocks__/grid_context'; import { discoverServiceMock } from '../../__mocks__/services'; +const columns = ['extension', 'message']; +const columnsWithTimeCol = getVisibleColumns( + ['extension', 'message'], + dataViewWithTimefieldMock, + true +) as string[]; + describe('Discover grid columns', function () { - it('returns eui grid columns without time column', async () => { - const actual = getEuiGridColumns({ - columns: ['extension', 'message'], - settings: {}, - dataView: dataViewMock, - showTimeCol: false, - defaultColumns: false, - isSortEnabled: true, - isPlainRecord: false, - valueToStringConverter: discoverGridContextMock.valueToStringConverter, - rowsCount: 100, - services: { - uiSettings: discoverServiceMock.uiSettings, - toastNotifications: discoverServiceMock.toastNotifications, - }, - hasEditDataViewPermission: () => - discoverServiceMock.dataViewFieldEditor.userPermissions.editIndexPattern(), - onFilter: () => {}, - }); - expect(actual).toMatchInlineSnapshot(` - Array [ - Object { - "actions": Object { - "additional": Array [ - Object { - "data-test-subj": "gridCopyColumnNameToClipBoardButton", - "iconProps": Object { - "size": "m", - }, - "iconType": "copyClipboard", - "label": , - "onClick": [Function], - "size": "xs", - }, - Object { - "data-test-subj": "gridCopyColumnValuesToClipBoardButton", - "iconProps": Object { - "size": "m", - }, - "iconType": "copyClipboard", - "label": , - "onClick": [Function], - "size": "xs", - }, - ], - "showHide": Object { - "iconType": "cross", - "label": "Remove column", - }, - "showMoveLeft": true, - "showMoveRight": true, - }, - "cellActions": Array [ - [Function], - [Function], - [Function], - ], - "displayAsText": "extension", - "id": "extension", - "isSortable": false, - "schema": "string", + describe('getEuiGridColumns', () => { + it('returns eui grid columns showing default columns', async () => { + const actual = getEuiGridColumns({ + columns, + settings: {}, + dataView: dataViewWithTimefieldMock, + defaultColumns: true, + isSortEnabled: true, + isPlainRecord: false, + valueToStringConverter: discoverGridContextMock.valueToStringConverter, + rowsCount: 100, + services: { + uiSettings: discoverServiceMock.uiSettings, + toastNotifications: discoverServiceMock.toastNotifications, }, - Object { - "actions": Object { - "additional": Array [ - Object { - "data-test-subj": "gridCopyColumnNameToClipBoardButton", - "iconProps": Object { - "size": "m", - }, - "iconType": "copyClipboard", - "label": , - "onClick": [Function], - "size": "xs", - }, - Object { - "data-test-subj": "gridCopyColumnValuesToClipBoardButton", - "iconProps": Object { - "size": "m", - }, - "iconType": "copyClipboard", - "label": , - "onClick": [Function], - "size": "xs", - }, - ], - "showHide": Object { - "iconType": "cross", - "label": "Remove column", - }, - "showMoveLeft": true, - "showMoveRight": true, - }, - "cellActions": Array [ - [Function], - ], - "displayAsText": "message", - "id": "message", - "isSortable": false, - "schema": "string", - }, - ] - `); - }); - it('returns eui grid columns without time column showing default columns', async () => { - const actual = getEuiGridColumns({ - columns: ['extension', 'message'], - settings: {}, - dataView: dataViewWithTimefieldMock, - showTimeCol: false, - defaultColumns: true, - isSortEnabled: true, - isPlainRecord: false, - valueToStringConverter: discoverGridContextMock.valueToStringConverter, - rowsCount: 100, - services: { - uiSettings: discoverServiceMock.uiSettings, - toastNotifications: discoverServiceMock.toastNotifications, - }, - hasEditDataViewPermission: () => - discoverServiceMock.dataViewFieldEditor.userPermissions.editIndexPattern(), - onFilter: () => {}, - }); - expect(actual).toMatchInlineSnapshot(` + hasEditDataViewPermission: () => + discoverServiceMock.dataViewFieldEditor.userPermissions.editIndexPattern(), + onFilter: () => {}, + }); + expect(actual).toMatchInlineSnapshot(` Array [ Object { "actions": Object { @@ -246,27 +133,27 @@ describe('Discover grid columns', function () { }, ] `); - }); - it('returns eui grid columns with time column', async () => { - const actual = getEuiGridColumns({ - columns: ['extension', 'message'], - settings: {}, - dataView: dataViewWithTimefieldMock, - showTimeCol: true, - defaultColumns: false, - isSortEnabled: true, - isPlainRecord: false, - valueToStringConverter: discoverGridContextMock.valueToStringConverter, - rowsCount: 100, - services: { - uiSettings: discoverServiceMock.uiSettings, - toastNotifications: discoverServiceMock.toastNotifications, - }, - hasEditDataViewPermission: () => - discoverServiceMock.dataViewFieldEditor.userPermissions.editIndexPattern(), - onFilter: () => {}, }); - expect(actual).toMatchInlineSnapshot(` + + it('returns eui grid columns with time column', async () => { + const actual = getEuiGridColumns({ + columns: columnsWithTimeCol, + settings: {}, + dataView: dataViewWithTimefieldMock, + defaultColumns: false, + isSortEnabled: true, + isPlainRecord: false, + valueToStringConverter: discoverGridContextMock.valueToStringConverter, + rowsCount: 100, + services: { + uiSettings: discoverServiceMock.uiSettings, + toastNotifications: discoverServiceMock.toastNotifications, + }, + hasEditDataViewPermission: () => + discoverServiceMock.dataViewFieldEditor.userPermissions.editIndexPattern(), + onFilter: () => {}, + }); + expect(actual).toMatchInlineSnapshot(` Array [ Object { "actions": Object { @@ -431,28 +318,27 @@ describe('Discover grid columns', function () { }, ] `); - }); - - it('returns eui grid with inmemory sorting', async () => { - const actual = getEuiGridColumns({ - columns: ['extension', 'message'], - settings: {}, - dataView: dataViewWithTimefieldMock, - showTimeCol: true, - defaultColumns: false, - isSortEnabled: true, - isPlainRecord: true, - valueToStringConverter: discoverGridContextMock.valueToStringConverter, - rowsCount: 100, - services: { - uiSettings: discoverServiceMock.uiSettings, - toastNotifications: discoverServiceMock.toastNotifications, - }, - hasEditDataViewPermission: () => - discoverServiceMock.dataViewFieldEditor.userPermissions.editIndexPattern(), - onFilter: () => {}, }); - expect(actual).toMatchInlineSnapshot(` + + it('returns eui grid with in memory sorting', async () => { + const actual = getEuiGridColumns({ + columns: columnsWithTimeCol, + settings: {}, + dataView: dataViewWithTimefieldMock, + defaultColumns: false, + isSortEnabled: true, + isPlainRecord: true, + valueToStringConverter: discoverGridContextMock.valueToStringConverter, + rowsCount: 100, + services: { + uiSettings: discoverServiceMock.uiSettings, + toastNotifications: discoverServiceMock.toastNotifications, + }, + hasEditDataViewPermission: () => + discoverServiceMock.dataViewFieldEditor.userPermissions.editIndexPattern(), + onFilter: () => {}, + }); + expect(actual).toMatchInlineSnapshot(` Array [ Object { "actions": Object { @@ -617,5 +503,31 @@ describe('Discover grid columns', function () { }, ] `); + }); + }); + + describe('getVisibleColumns', () => { + it('returns grid columns without time column when data view has no timestamp field', () => { + const actual = getVisibleColumns(['extension', 'message'], dataViewMock, true) as string[]; + expect(actual).toEqual(['extension', 'message']); + }); + + it('returns grid columns without time column when showTimeCol is falsy', () => { + const actual = getVisibleColumns( + ['extension', 'message'], + dataViewWithTimefieldMock, + false + ) as string[]; + expect(actual).toEqual(['extension', 'message']); + }); + + it('returns grid columns with time column when data view has timestamp field', () => { + const actual = getVisibleColumns( + ['extension', 'message'], + dataViewWithTimefieldMock, + true + ) as string[]; + expect(actual).toEqual(['timestamp', 'extension', 'message']); + }); }); }); diff --git a/src/plugins/discover/public/components/discover_grid/discover_grid_columns.tsx b/src/plugins/discover/public/components/discover_grid/discover_grid_columns.tsx index b341d6236d235..a93b8a9ff1464 100644 --- a/src/plugins/discover/public/components/discover_grid/discover_grid_columns.tsx +++ b/src/plugins/discover/public/components/discover_grid/discover_grid_columns.tsx @@ -8,7 +8,13 @@ import React from 'react'; import { i18n } from '@kbn/i18n'; -import { EuiDataGridColumn, EuiIcon, EuiScreenReaderOnly, EuiToolTip } from '@elastic/eui'; +import { + type EuiDataGridColumn, + type EuiDataGridColumnCellAction, + EuiIcon, + EuiScreenReaderOnly, + EuiToolTip, +} from '@elastic/eui'; import type { DataView } from '@kbn/data-views-plugin/public'; import { ToastsStart, IUiSettingsClient } from '@kbn/core/public'; import { DocViewFilterFn } from '../../services/doc_views/doc_views_types'; @@ -72,6 +78,7 @@ function buildEuiGridColumn({ rowsCount, onFilter, editField, + columnCellActions, }: { columnName: string; columnWidth: number | undefined; @@ -85,6 +92,7 @@ function buildEuiGridColumn({ rowsCount: number; onFilter?: DocViewFilterFn; editField?: (fieldName: string) => void; + columnCellActions?: EuiDataGridColumnCellAction[]; }) { const dataViewField = dataView.getFieldByName(columnName); const editFieldButton = @@ -98,6 +106,13 @@ function buildEuiGridColumn({ }) : dataViewField?.displayName || columnName; + let cellActions: EuiDataGridColumnCellAction[]; + if (columnCellActions?.length) { + cellActions = columnCellActions; + } else { + cellActions = dataViewField ? buildCellActions(dataViewField, onFilter) : []; + } + const column: EuiDataGridColumn = { id: columnName, schema: getSchemaByKbnType(dataViewField?.type), @@ -134,7 +149,7 @@ function buildEuiGridColumn({ ...(editFieldButton ? [editFieldButton] : []), ], }, - cellActions: dataViewField ? buildCellActions(dataViewField, onFilter) : [], + cellActions, }; if (column.id === dataView.timeFieldName) { @@ -172,10 +187,10 @@ function buildEuiGridColumn({ export function getEuiGridColumns({ columns, + columnsCellActions, rowsCount, settings, dataView, - showTimeCol, defaultColumns, isSortEnabled, isPlainRecord, @@ -186,10 +201,10 @@ export function getEuiGridColumns({ editField, }: { columns: string[]; + columnsCellActions?: EuiDataGridColumnCellAction[][]; rowsCount: number; settings: DiscoverGridSettings | undefined; dataView: DataView; - showTimeCol: boolean; defaultColumns: boolean; isSortEnabled: boolean; isPlainRecord?: boolean; @@ -202,17 +217,12 @@ export function getEuiGridColumns({ onFilter: DocViewFilterFn; editField?: (fieldName: string) => void; }) { - const timeFieldName = dataView.timeFieldName; const getColWidth = (column: string) => settings?.columns?.[column]?.width ?? 0; - let visibleColumns = columns; - if (showTimeCol && dataView.timeFieldName && !columns.find((col) => col === timeFieldName)) { - visibleColumns = [dataView.timeFieldName, ...columns]; - } - - return visibleColumns.map((column) => + return columns.map((column, columnIndex) => buildEuiGridColumn({ columnName: column, + columnCellActions: columnsCellActions?.[columnIndex], columnWidth: getColWidth(column), dataView, defaultColumns, @@ -231,7 +241,7 @@ export function getEuiGridColumns({ export function getVisibleColumns(columns: string[], dataView: DataView, showTimeCol: boolean) { const timeFieldName = dataView.timeFieldName; - if (showTimeCol && !columns.find((col) => col === timeFieldName)) { + if (showTimeCol && timeFieldName && !columns.find((col) => col === timeFieldName)) { return [timeFieldName, ...columns]; } diff --git a/src/plugins/discover/public/embeddable/constants.ts b/src/plugins/discover/public/embeddable/constants.ts index 6c9de47a0f710..be717e18cfb5a 100644 --- a/src/plugins/discover/public/embeddable/constants.ts +++ b/src/plugins/discover/public/embeddable/constants.ts @@ -6,4 +6,16 @@ * Side Public License, v 1. */ +import type { Trigger } from '@kbn/ui-actions-plugin/public'; + export { SEARCH_EMBEDDABLE_TYPE } from '../../common'; + +export const SEARCH_EMBEDDABLE_CELL_ACTIONS_TRIGGER_ID = + 'SEARCH_EMBEDDABLE_CELL_ACTIONS_TRIGGER_ID'; + +export const SEARCH_EMBEDDABLE_CELL_ACTIONS_TRIGGER: Trigger = { + id: SEARCH_EMBEDDABLE_CELL_ACTIONS_TRIGGER_ID, + title: 'Discover saved searches embeddable cell actions', + description: + 'This trigger is used to replace the cell actions for Discover saved search embeddable grid.', +} as const; diff --git a/src/plugins/discover/public/embeddable/index.ts b/src/plugins/discover/public/embeddable/index.ts index 77182c67a4f3b..d07f90866b3b3 100644 --- a/src/plugins/discover/public/embeddable/index.ts +++ b/src/plugins/discover/public/embeddable/index.ts @@ -6,6 +6,6 @@ * Side Public License, v 1. */ -export { SEARCH_EMBEDDABLE_TYPE } from './constants'; +export { SEARCH_EMBEDDABLE_TYPE, SEARCH_EMBEDDABLE_CELL_ACTIONS_TRIGGER_ID } from './constants'; export * from './types'; export * from './search_embeddable_factory'; diff --git a/src/plugins/discover/public/embeddable/saved_search_embeddable.tsx b/src/plugins/discover/public/embeddable/saved_search_embeddable.tsx index eb6788f9bfe7e..e0d80e05bc969 100644 --- a/src/plugins/discover/public/embeddable/saved_search_embeddable.tsx +++ b/src/plugins/discover/public/embeddable/saved_search_embeddable.tsx @@ -35,12 +35,13 @@ import { UiActionsStart } from '@kbn/ui-actions-plugin/public'; import { KibanaContextProvider, KibanaThemeProvider } from '@kbn/kibana-react-plugin/public'; import { SavedSearch } from '@kbn/saved-search-plugin/public'; import { METRIC_TYPE } from '@kbn/analytics'; +import { CellActionsProvider } from '@kbn/cell-actions'; import { VIEW_MODE } from '../../common/constants'; import { getSortForEmbeddable, SortPair } from '../utils/sorting'; import { buildDataTableRecord } from '../utils/build_data_record'; import { DataTableRecord, EsHitRecord } from '../types'; import { ISearchEmbeddable, SearchInput, SearchOutput } from './types'; -import { SEARCH_EMBEDDABLE_TYPE } from './constants'; +import { SEARCH_EMBEDDABLE_TYPE, SEARCH_EMBEDDABLE_CELL_ACTIONS_TRIGGER_ID } from './constants'; import { DiscoverServices } from '../build_services'; import { SavedSearchEmbeddableComponent } from './saved_search_embeddable_component'; import { @@ -399,6 +400,7 @@ export class SavedSearchEmbeddable onUpdateRowsPerPage: (rowsPerPage) => { this.updateInput({ rowsPerPage }); }, + cellActionsTriggerId: SEARCH_EMBEDDABLE_CELL_ACTIONS_TRIGGER_ID, }; const timeRangeSearchSource = searchSource.create(); @@ -565,11 +567,14 @@ export class SavedSearchEmbeddable query, }; if (searchProps.services) { + const { getTriggerCompatibleActions } = searchProps.services.uiActions; ReactDOM.render( - + + + , diff --git a/src/plugins/discover/public/index.ts b/src/plugins/discover/public/index.ts index 3cd8586756c07..099a610953490 100644 --- a/src/plugins/discover/public/index.ts +++ b/src/plugins/discover/public/index.ts @@ -15,5 +15,5 @@ export function plugin(initializerContext: PluginInitializerContext) { } export type { ISearchEmbeddable, SearchInput } from './embeddable'; -export { SEARCH_EMBEDDABLE_TYPE } from './embeddable'; +export { SEARCH_EMBEDDABLE_TYPE, SEARCH_EMBEDDABLE_CELL_ACTIONS_TRIGGER_ID } from './embeddable'; export { loadSharingDataHelpers } from './utils'; diff --git a/src/plugins/discover/public/plugin.tsx b/src/plugins/discover/public/plugin.tsx index 0336b5e2ec597..aa3f80d4330c4 100644 --- a/src/plugins/discover/public/plugin.tsx +++ b/src/plugins/discover/public/plugin.tsx @@ -74,6 +74,7 @@ import { import { DiscoverAppLocator, DiscoverAppLocatorDefinition } from '../common'; import type { CustomizationCallback } from './customizations'; import { createCustomizeFunction, createProfileRegistry } from './customizations/profile_registry'; +import { SEARCH_EMBEDDABLE_CELL_ACTIONS_TRIGGER } from './embeddable/constants'; const DocViewerLegacyTable = React.lazy( () => import('./services/doc_views/components/doc_viewer_table/legacy') @@ -405,6 +406,8 @@ export class DiscoverPlugin const { uiActions } = plugins; + uiActions.registerTrigger(SEARCH_EMBEDDABLE_CELL_ACTIONS_TRIGGER); + const viewSavedSearchAction = new ViewSavedSearchAction(core.application); uiActions.addTriggerAction('CONTEXT_MENU_TRIGGER', viewSavedSearchAction); setUiActions(plugins.uiActions); diff --git a/src/plugins/discover/tsconfig.json b/src/plugins/discover/tsconfig.json index 0bacbf94a2731..df2760e4afb2f 100644 --- a/src/plugins/discover/tsconfig.json +++ b/src/plugins/discover/tsconfig.json @@ -53,6 +53,7 @@ "@kbn/dom-drag-drop", "@kbn/unified-field-list", "@kbn/core-saved-objects-api-server", + "@kbn/cell-actions", ], "exclude": [ "target/**/*", diff --git a/x-pack/packages/security-solution/data_table/components/data_table/index.test.tsx b/x-pack/packages/security-solution/data_table/components/data_table/index.test.tsx index 64140f71a62ea..42523385b5ace 100644 --- a/x-pack/packages/security-solution/data_table/components/data_table/index.test.tsx +++ b/x-pack/packages/security-solution/data_table/components/data_table/index.test.tsx @@ -171,20 +171,17 @@ describe('DataTable', () => { expect(mockUseDataGridColumnsCellActions).toHaveBeenCalledWith({ triggerId: 'mockCellActionsTrigger', - data: [ + fields: [ { - values: [data[0]?.data[0]?.value], - field: { - name: '@timestamp', - type: 'date', - aggregatable: true, - esTypes: ['date'], - searchable: true, - subType: undefined, - }, + name: '@timestamp', + type: 'date', + aggregatable: true, + esTypes: ['date'], + searchable: true, + subType: undefined, }, ], - + getCellValue: expect.any(Function), metadata: { scopeId: 'table-test', }, @@ -202,7 +199,8 @@ describe('DataTable', () => { expect(mockUseDataGridColumnsCellActions).toHaveBeenCalledWith( expect.objectContaining({ - data: [], + triggerId: undefined, + fields: undefined, }) ); }); diff --git a/x-pack/packages/security-solution/data_table/components/data_table/index.tsx b/x-pack/packages/security-solution/data_table/components/data_table/index.tsx index 7f21e8e3bca6a..b11c932b60b58 100644 --- a/x-pack/packages/security-solution/data_table/components/data_table/index.tsx +++ b/x-pack/packages/security-solution/data_table/components/data_table/index.tsx @@ -38,7 +38,10 @@ import { DeprecatedRowRenderer, TimelineItem, } from '@kbn/timelines-plugin/common'; -import { useDataGridColumnsCellActions } from '@kbn/cell-actions'; +import { + useDataGridColumnsCellActions, + UseDataGridColumnsCellActionsProps, +} from '@kbn/cell-actions'; import { DataTableModel, DataTableState } from '../../store/data_table/types'; import { getColumnHeader, getColumnHeaders } from './column_headers/helpers'; @@ -327,36 +330,39 @@ export const DataTableComponent = React.memo( [dispatch, id] ); - const columnsCellActionsProps = useMemo(() => { - const columnsCellActionData = !cellActionsTriggerId - ? [] - : columnHeaders.map((column) => ({ - // TODO use FieldSpec object instead of column - field: { + const cellActionsMetadata = useMemo(() => ({ scopeId: id }), [id]); + + const cellActionsFields = useMemo( + () => + cellActionsTriggerId + ? // TODO use FieldSpec object instead of column + columnHeaders.map((column) => ({ name: column.id, type: column.type ?? 'keyword', aggregatable: column.aggregatable ?? false, searchable: column.searchable ?? false, esTypes: column.esTypes ?? [], subType: column.subType, - }, - values: data.map( - ({ data: columnData }) => - columnData.find((rowData) => rowData.field === column.id)?.value - ), - })); + })) + : undefined, + [cellActionsTriggerId, columnHeaders] + ); - return { - triggerId: cellActionsTriggerId || '', - data: columnsCellActionData, - metadata: { - scopeId: id, - }, - dataGridRef, - }; - }, [columnHeaders, cellActionsTriggerId, id, data]); + const getCellValue = useCallback( + (fieldName, rowIndex) => { + const pageIndex = rowIndex % data.length; + return data[pageIndex].data.find((rowData) => rowData.field === fieldName)?.value; + }, + [data] + ); - const columnsCellActions = useDataGridColumnsCellActions(columnsCellActionsProps); + const columnsCellActions = useDataGridColumnsCellActions({ + triggerId: cellActionsTriggerId, + fields: cellActionsFields, + getCellValue, + metadata: cellActionsMetadata, + dataGridRef, + }); const columnsWithCellActions: EuiDataGridColumn[] = useMemo( () => @@ -469,5 +475,3 @@ export const DataTableComponent = React.memo( ); } ); - -DataTableComponent.displayName = 'DataTableComponent'; diff --git a/x-pack/plugins/security_solution/kibana.jsonc b/x-pack/plugins/security_solution/kibana.jsonc index f24dffb58edc6..04808cbc1f137 100644 --- a/x-pack/plugins/security_solution/kibana.jsonc +++ b/x-pack/plugins/security_solution/kibana.jsonc @@ -19,8 +19,9 @@ "cloudSecurityPosture", "dashboard", "data", - "ecsDataQualityDashboard", "dataViews", + "discover", + "ecsDataQualityDashboard", "embeddable", "eventLog", "features", @@ -41,7 +42,6 @@ "unifiedSearch", "files", "controls", - "dataViews", "savedObjectsManagement", "stackConnectors", ], diff --git a/x-pack/plugins/security_solution/public/actions/add_to_timeline/discover/add_to_timeline.test.ts b/x-pack/plugins/security_solution/public/actions/add_to_timeline/discover/add_to_timeline.test.ts new file mode 100644 index 0000000000000..d61a1ab64bdb8 --- /dev/null +++ b/x-pack/plugins/security_solution/public/actions/add_to_timeline/discover/add_to_timeline.test.ts @@ -0,0 +1,130 @@ +/* + * 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 type { SecurityAppStore } from '../../../common/store/types'; +import { TimelineId } from '../../../../common/types'; +import { addProvider } from '../../../timelines/store/timeline/actions'; +import { createAddToTimelineDiscoverCellActionFactory } from './add_to_timeline'; +import type { CellActionExecutionContext } from '@kbn/cell-actions'; +import { GEO_FIELD_TYPE } from '../../../timelines/components/timeline/body/renderers/constants'; +import { createStartServicesMock } from '../../../common/lib/kibana/kibana_react.mock'; +import { APP_UI_ID } from '../../../../common'; +import { BehaviorSubject } from 'rxjs'; + +const services = createStartServicesMock(); +const mockWarningToast = services.notifications.toasts.addWarning; + +const currentAppIdSubject$ = new BehaviorSubject(APP_UI_ID); +services.application.currentAppId$ = currentAppIdSubject$.asObservable(); + +const mockDispatch = jest.fn(); +const store = { + dispatch: mockDispatch, +} as unknown as SecurityAppStore; + +const value = 'the-value'; + +const context = { + data: [{ field: { name: 'user.name', type: 'text' }, value }], +} as CellActionExecutionContext; + +const defaultDataProvider = { + type: addProvider.type, + payload: { + id: TimelineId.active, + providers: [ + { + and: [], + enabled: true, + excluded: false, + id: 'event-field-default-timeline-1-user_name-0-the-value', + kqlQuery: '', + name: 'user.name', + queryMatch: { + field: 'user.name', + operator: ':', + value: 'the-value', + }, + }, + ], + }, +}; + +describe('createAddToTimelineDiscoverCellActionFactory', () => { + const addToTimelineDiscoverCellActionFactory = createAddToTimelineDiscoverCellActionFactory({ + store, + services, + }); + const addToTimelineAction = addToTimelineDiscoverCellActionFactory({ + id: 'testAddToTimeline', + order: 1, + }); + + beforeEach(() => { + currentAppIdSubject$.next(APP_UI_ID); + jest.clearAllMocks(); + }); + + it('should return display name', () => { + expect(addToTimelineAction.getDisplayName(context)).toEqual('Add to timeline'); + }); + + it('should return icon type', () => { + expect(addToTimelineAction.getIconType(context)).toEqual('timeline'); + }); + + describe('isCompatible', () => { + it('should return true if everything is okay', async () => { + expect(await addToTimelineAction.isCompatible(context)).toEqual(true); + }); + + it('should return false if not in security', async () => { + currentAppIdSubject$.next('not-security'); + expect(await addToTimelineAction.isCompatible(context)).toEqual(false); + }); + + it('should return false if field not allowed', async () => { + expect( + await addToTimelineAction.isCompatible({ + ...context, + data: [ + { + ...context.data[0], + field: { ...context.data[0].field, name: 'signal.reason' }, + }, + ], + }) + ).toEqual(false); + }); + }); + + describe('execute', () => { + it('should execute normally', async () => { + await addToTimelineAction.execute(context); + expect(mockDispatch).toHaveBeenCalledWith(defaultDataProvider); + expect(mockWarningToast).not.toHaveBeenCalled(); + }); + + it('should show warning if no provider added', async () => { + await addToTimelineAction.execute({ + ...context, + data: [ + { + ...context.data[0], + field: { + ...context.data[0].field, + type: GEO_FIELD_TYPE, + }, + value, + }, + ], + }); + expect(mockDispatch).not.toHaveBeenCalled(); + expect(mockWarningToast).toHaveBeenCalled(); + }); + }); +}); diff --git a/x-pack/plugins/security_solution/public/actions/add_to_timeline/discover/add_to_timeline.ts b/x-pack/plugins/security_solution/public/actions/add_to_timeline/discover/add_to_timeline.ts new file mode 100644 index 0000000000000..859f14652810b --- /dev/null +++ b/x-pack/plugins/security_solution/public/actions/add_to_timeline/discover/add_to_timeline.ts @@ -0,0 +1,36 @@ +/* + * 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 type { CellAction, CellActionFactory } from '@kbn/cell-actions'; +import type { SecurityAppStore } from '../../../common/store'; +import { isInSecurityApp } from '../../utils'; +import type { StartServices } from '../../../types'; +import { createAddToTimelineCellActionFactory } from '../cell_action/add_to_timeline'; + +export const createAddToTimelineDiscoverCellActionFactory = ({ + store, + services, +}: { + store: SecurityAppStore; + services: StartServices; +}): CellActionFactory => { + const { application } = services; + + let currentAppId: string | undefined; + application.currentAppId$.subscribe((appId) => { + currentAppId = appId; + }); + + const securityAddToTimelineActionFactory = createAddToTimelineCellActionFactory({ + store, + services, + }); + + return securityAddToTimelineActionFactory.combine({ + isCompatible: async () => isInSecurityApp(currentAppId), + }); +}; diff --git a/x-pack/plugins/security_solution/public/actions/add_to_timeline/index.ts b/x-pack/plugins/security_solution/public/actions/add_to_timeline/index.ts index 5aeaa99800829..4f7ac7c1385bc 100644 --- a/x-pack/plugins/security_solution/public/actions/add_to_timeline/index.ts +++ b/x-pack/plugins/security_solution/public/actions/add_to_timeline/index.ts @@ -8,3 +8,4 @@ export { createAddToTimelineCellActionFactory } from './cell_action/add_to_timeline'; export { createInvestigateInNewTimelineCellActionFactory } from './cell_action/investigate_in_new_timeline'; export { createAddToTimelineLensAction } from './lens/add_to_timeline'; +export { createAddToTimelineDiscoverCellActionFactory } from './discover/add_to_timeline'; diff --git a/x-pack/plugins/security_solution/public/actions/copy_to_clipboard/cell_action/copy_to_clipboard.test.ts b/x-pack/plugins/security_solution/public/actions/copy_to_clipboard/cell_action/copy_to_clipboard.test.ts new file mode 100644 index 0000000000000..6df6f6b00b177 --- /dev/null +++ b/x-pack/plugins/security_solution/public/actions/copy_to_clipboard/cell_action/copy_to_clipboard.test.ts @@ -0,0 +1,64 @@ +/* + * 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 { createCopyToClipboardCellActionFactory } from './copy_to_clipboard'; +import { createStartServicesMock } from '../../../common/lib/kibana/kibana_react.mock'; +import type { CellActionExecutionContext } from '@kbn/cell-actions'; + +const services = createStartServicesMock(); +const mockSuccessToast = services.notifications.toasts.addSuccess; + +const mockCopy = jest.fn((text: string) => true); +jest.mock('copy-to-clipboard', () => (text: string) => mockCopy(text)); + +describe('createCopyToClipboardCellActionFactory', () => { + const copyToClipboardActionFactory = createCopyToClipboardCellActionFactory({ services }); + const copyToClipboardAction = copyToClipboardActionFactory({ id: 'testAction' }); + const context = { + data: [{ field: { name: 'user.name', type: 'text' }, value: 'the value' }], + } as CellActionExecutionContext; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('should return display name', () => { + expect(copyToClipboardAction.getDisplayName(context)).toEqual('Copy to Clipboard'); + }); + + it('should return icon type', () => { + expect(copyToClipboardAction.getIconType(context)).toEqual('copyClipboard'); + }); + + describe('isCompatible', () => { + it('should return true if everything is okay', async () => { + expect(await copyToClipboardAction.isCompatible(context)).toEqual(true); + }); + + it('should return false if field not allowed', async () => { + expect( + await copyToClipboardAction.isCompatible({ + ...context, + data: [ + { + ...context.data[0].field, + field: { ...context.data[0].field, name: 'signal.reason' }, + }, + ], + }) + ).toEqual(false); + }); + }); + + describe('execute', () => { + it('should execute normally', async () => { + await copyToClipboardAction.execute(context); + expect(mockCopy).toHaveBeenCalledWith('user.name: "the value"'); + expect(mockSuccessToast).toHaveBeenCalled(); + }); + }); +}); diff --git a/x-pack/plugins/security_solution/public/actions/copy_to_clipboard/discover/copy_to_clipboard.test.ts b/x-pack/plugins/security_solution/public/actions/copy_to_clipboard/discover/copy_to_clipboard.test.ts new file mode 100644 index 0000000000000..23ca470e11d22 --- /dev/null +++ b/x-pack/plugins/security_solution/public/actions/copy_to_clipboard/discover/copy_to_clipboard.test.ts @@ -0,0 +1,80 @@ +/* + * 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 { createCopyToClipboardDiscoverCellActionFactory } from './copy_to_clipboard'; +import { createStartServicesMock } from '../../../common/lib/kibana/kibana_react.mock'; +import type { CellActionExecutionContext } from '@kbn/cell-actions'; +import { BehaviorSubject } from 'rxjs'; +import { APP_UI_ID } from '../../../../common'; + +const services = createStartServicesMock(); +const mockSuccessToast = services.notifications.toasts.addSuccess; + +const currentAppIdSubject$ = new BehaviorSubject(APP_UI_ID); +services.application.currentAppId$ = currentAppIdSubject$.asObservable(); + +const mockCopy = jest.fn((text: string) => true); +jest.mock('copy-to-clipboard', () => (text: string) => mockCopy(text)); + +describe('createCopyToClipboardDiscoverCellActionFactory', () => { + const copyToClipboardActionFactory = createCopyToClipboardDiscoverCellActionFactory({ services }); + const copyToClipboardAction = copyToClipboardActionFactory({ id: 'testAction' }); + const context = { + data: [ + { + field: { name: 'user.name', type: 'text' }, + value: 'the value', + }, + ], + } as CellActionExecutionContext; + + beforeEach(() => { + currentAppIdSubject$.next(APP_UI_ID); + jest.clearAllMocks(); + }); + + it('should return display name', () => { + expect(copyToClipboardAction.getDisplayName(context)).toEqual('Copy to Clipboard'); + }); + + it('should return icon type', () => { + expect(copyToClipboardAction.getIconType(context)).toEqual('copyClipboard'); + }); + + describe('isCompatible', () => { + it('should return true if everything is okay', async () => { + expect(await copyToClipboardAction.isCompatible(context)).toEqual(true); + }); + + it('should return false if not in security', async () => { + currentAppIdSubject$.next('not-security'); + expect(await copyToClipboardAction.isCompatible(context)).toEqual(false); + }); + + it('should return false if field not allowed', async () => { + expect( + await copyToClipboardAction.isCompatible({ + ...context, + data: [ + { + ...context.data[0], + field: { ...context.data[0].field, name: 'signal.reason' }, + }, + ], + }) + ).toEqual(false); + }); + }); + + describe('execute', () => { + it('should execute normally', async () => { + await copyToClipboardAction.execute(context); + expect(mockCopy).toHaveBeenCalledWith('user.name: "the value"'); + expect(mockSuccessToast).toHaveBeenCalled(); + }); + }); +}); diff --git a/x-pack/plugins/security_solution/public/actions/copy_to_clipboard/discover/copy_to_clipboard.ts b/x-pack/plugins/security_solution/public/actions/copy_to_clipboard/discover/copy_to_clipboard.ts new file mode 100644 index 0000000000000..ac3e077597afd --- /dev/null +++ b/x-pack/plugins/security_solution/public/actions/copy_to_clipboard/discover/copy_to_clipboard.ts @@ -0,0 +1,32 @@ +/* + * 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 type { CellAction, CellActionFactory } from '@kbn/cell-actions'; +import { isInSecurityApp } from '../../utils'; +import type { StartServices } from '../../../types'; +import { createCopyToClipboardCellActionFactory } from '../cell_action/copy_to_clipboard'; + +export const createCopyToClipboardDiscoverCellActionFactory = ({ + services, +}: { + services: StartServices; +}): CellActionFactory => { + const { application } = services; + + let currentAppId: string | undefined; + application.currentAppId$.subscribe((appId) => { + currentAppId = appId; + }); + + const genericCopyToClipboardActionFactory = createCopyToClipboardCellActionFactory({ + services, + }); + + return genericCopyToClipboardActionFactory.combine({ + isCompatible: async () => isInSecurityApp(currentAppId), + }); +}; diff --git a/x-pack/plugins/security_solution/public/actions/copy_to_clipboard/index.ts b/x-pack/plugins/security_solution/public/actions/copy_to_clipboard/index.ts index 601e23086b422..575420e170669 100644 --- a/x-pack/plugins/security_solution/public/actions/copy_to_clipboard/index.ts +++ b/x-pack/plugins/security_solution/public/actions/copy_to_clipboard/index.ts @@ -7,3 +7,4 @@ export { createCopyToClipboardLensAction } from './lens/copy_to_clipboard'; export { createCopyToClipboardCellActionFactory } from './cell_action/copy_to_clipboard'; +export { createCopyToClipboardDiscoverCellActionFactory } from './discover/copy_to_clipboard'; diff --git a/x-pack/plugins/security_solution/public/actions/filter/cell_action/filter_in.test.ts b/x-pack/plugins/security_solution/public/actions/filter/cell_action/filter_in.test.ts index 61ba26cfd5086..2e2a511933476 100644 --- a/x-pack/plugins/security_solution/public/actions/filter/cell_action/filter_in.test.ts +++ b/x-pack/plugins/security_solution/public/actions/filter/cell_action/filter_in.test.ts @@ -20,7 +20,8 @@ import { TableId } from '@kbn/securitysolution-data-table'; import { TimelineId } from '../../../../common/types'; const services = createStartServicesMock(); -const mockFilterManager = services.data.query.filterManager; +const mockGlobalFilterManager = services.data.query.filterManager; +const mockTimelineFilterManager = createFilterManagerMock(); const mockState = { ...mockGlobalState, @@ -30,7 +31,7 @@ const mockState = { ...mockGlobalState.timeline.timelineById, [TimelineId.active]: { ...mockGlobalState.timeline.timelineById[TimelineId.active], - filterManager: createFilterManagerMock(), + filterManager: mockTimelineFilterManager, }, }, }, @@ -88,56 +89,64 @@ describe('createFilterInCellActionFactory', () => { }); }); - describe('generic scope execution', () => { - const dataTableContext = { - ...context, - metadata: { scopeId: TableId.alertsOnAlertsPage }, - } as SecurityCellActionExecutionContext; - - it('should execute using generic filterManager', async () => { - await filterInAction.execute(dataTableContext); - expect(mockFilterManager.addFilters).toHaveBeenCalled(); - expect( - mockState.timeline.timelineById[TimelineId.active].filterManager?.addFilters - ).not.toHaveBeenCalled(); + describe('execute', () => { + describe('generic scope execution', () => { + const dataTableContext = { + ...context, + metadata: { scopeId: TableId.alertsOnAlertsPage }, + } as SecurityCellActionExecutionContext; + + it('should execute using generic filterManager', async () => { + await filterInAction.execute(dataTableContext); + expect(mockGlobalFilterManager.addFilters).toHaveBeenCalled(); + expect(mockTimelineFilterManager.addFilters).not.toHaveBeenCalled(); + }); }); - }); - describe('timeline scope execution', () => { - const timelineContext = { - ...context, - metadata: { scopeId: TimelineId.active }, - } as SecurityCellActionExecutionContext; + describe('timeline scope execution', () => { + const timelineContext = { + ...context, + metadata: { scopeId: TimelineId.active }, + } as SecurityCellActionExecutionContext; - it('should execute using timeline filterManager', async () => { - await filterInAction.execute(timelineContext); - expect( - mockState.timeline.timelineById[TimelineId.active].filterManager?.addFilters - ).toHaveBeenCalled(); - expect(mockFilterManager.addFilters).not.toHaveBeenCalled(); + it('should execute using timeline filterManager', async () => { + await filterInAction.execute(timelineContext); + expect(mockTimelineFilterManager.addFilters).toHaveBeenCalled(); + expect(mockGlobalFilterManager.addFilters).not.toHaveBeenCalled(); + }); }); - describe('should execute correctly when negateFilters is provided', () => { - it('if negateFilters is false, negate should be false (do not exclude)', async () => { + describe('negateFilters', () => { + it('if negateFilters is false, negate should be false (include)', async () => { await filterInAction.execute({ ...context, metadata: { negateFilters: false, }, }); - expect(mockFilterManager.addFilters).toHaveBeenCalled(); - // expect(mockCreateFilter).toBeCalledWith(context.field.name, context.field.value, false); + expect(mockGlobalFilterManager.addFilters).toHaveBeenCalledWith( + expect.objectContaining({ + meta: expect.objectContaining({ + negate: false, + }), + }) + ); }); - it('if negateFilters is true, negate should be true (exclude)', async () => { + it('if negateFilters is true, negate should be true (do not include)', async () => { await filterInAction.execute({ ...context, metadata: { negateFilters: true, }, }); - expect(mockFilterManager.addFilters).toHaveBeenCalled(); - // expect(mockCreateFilter).toBeCalledWith(context.field.name, context.field.value, true); + expect(mockGlobalFilterManager.addFilters).toHaveBeenCalledWith( + expect.objectContaining({ + meta: expect.objectContaining({ + negate: true, + }), + }) + ); }); }); }); diff --git a/x-pack/plugins/security_solution/public/actions/filter/cell_action/filter_out.test.ts b/x-pack/plugins/security_solution/public/actions/filter/cell_action/filter_out.test.ts index 09f7c9ffbecde..b4c855c3a4509 100644 --- a/x-pack/plugins/security_solution/public/actions/filter/cell_action/filter_out.test.ts +++ b/x-pack/plugins/security_solution/public/actions/filter/cell_action/filter_out.test.ts @@ -20,7 +20,8 @@ import { TimelineId } from '../../../../common/types'; import { TableId } from '@kbn/securitysolution-data-table'; const services = createStartServicesMock(); -const mockFilterManager = services.data.query.filterManager; +const mockGlobalFilterManager = services.data.query.filterManager; +const mockTimelineFilterManager = createFilterManagerMock(); const mockState = { ...mockGlobalState, @@ -30,7 +31,7 @@ const mockState = { ...mockGlobalState.timeline.timelineById, [TimelineId.active]: { ...mockGlobalState.timeline.timelineById[TimelineId.active], - filterManager: createFilterManagerMock(), + filterManager: mockTimelineFilterManager, }, }, }, @@ -82,33 +83,65 @@ describe('createFilterOutCellActionFactory', () => { }); }); - describe('generic scope execution', () => { - const dataTableContext = { - ...context, - metadata: { scopeId: TableId.alertsOnAlertsPage }, - } as SecurityCellActionExecutionContext; + describe('execute', () => { + describe('generic scope execution', () => { + const dataTableContext = { + ...context, + metadata: { scopeId: TableId.alertsOnAlertsPage }, + } as SecurityCellActionExecutionContext; - it('should execute using generic filterManager', async () => { - await filterOutAction.execute(dataTableContext); - expect(mockFilterManager.addFilters).toHaveBeenCalled(); - expect( - mockState.timeline.timelineById[TimelineId.active].filterManager?.addFilters - ).not.toHaveBeenCalled(); + it('should execute using generic filterManager', async () => { + await filterOutAction.execute(dataTableContext); + expect(mockGlobalFilterManager.addFilters).toHaveBeenCalled(); + expect(mockTimelineFilterManager.addFilters).not.toHaveBeenCalled(); + }); }); - }); - describe('timeline scope execution', () => { - const timelineContext = { - ...context, - metadata: { scopeId: TimelineId.active }, - } as SecurityCellActionExecutionContext; + describe('timeline scope execution', () => { + const timelineContext = { + ...context, + metadata: { scopeId: TimelineId.active }, + } as SecurityCellActionExecutionContext; - it('should execute using timeline filterManager', async () => { - await filterOutAction.execute(timelineContext); - expect( - mockState.timeline.timelineById[TimelineId.active].filterManager?.addFilters - ).toHaveBeenCalled(); - expect(mockFilterManager.addFilters).not.toHaveBeenCalled(); + it('should execute using timeline filterManager', async () => { + await filterOutAction.execute(timelineContext); + expect(mockTimelineFilterManager.addFilters).toHaveBeenCalled(); + expect(mockGlobalFilterManager.addFilters).not.toHaveBeenCalled(); + }); + }); + + describe('negateFilters', () => { + it('if negateFilters is false, negate should be true (exclude)', async () => { + await filterOutAction.execute({ + ...context, + metadata: { + negateFilters: false, + }, + }); + expect(mockGlobalFilterManager.addFilters).toHaveBeenCalledWith( + expect.objectContaining({ + meta: expect.objectContaining({ + negate: true, + }), + }) + ); + }); + + it('if negateFilters is true, negate should be false (do not exclude)', async () => { + await filterOutAction.execute({ + ...context, + metadata: { + negateFilters: true, + }, + }); + expect(mockGlobalFilterManager.addFilters).toHaveBeenCalledWith( + expect.objectContaining({ + meta: expect.objectContaining({ + negate: false, + }), + }) + ); + }); }); }); }); diff --git a/x-pack/plugins/security_solution/public/actions/filter/discover/filter_in.test.ts b/x-pack/plugins/security_solution/public/actions/filter/discover/filter_in.test.ts new file mode 100644 index 0000000000000..aefe4698ed09e --- /dev/null +++ b/x-pack/plugins/security_solution/public/actions/filter/discover/filter_in.test.ts @@ -0,0 +1,88 @@ +/* + * 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 { + createSecuritySolutionStorageMock, + kibanaObservable, + mockGlobalState, + SUB_PLUGINS_REDUCER, +} from '../../../common/mock'; +import { createStore } from '../../../common/store'; +import { createFilterInDiscoverCellActionFactory } from './filter_in'; +import type { SecurityCellActionExecutionContext } from '../../types'; +import { createStartServicesMock } from '../../../common/lib/kibana/kibana_react.mock'; +import { BehaviorSubject } from 'rxjs'; +import { APP_UI_ID } from '../../../../common'; + +const services = createStartServicesMock(); +const mockGlobalFilterManager = services.data.query.filterManager; + +const currentAppIdSubject$ = new BehaviorSubject(APP_UI_ID); +services.application.currentAppId$ = currentAppIdSubject$.asObservable(); + +jest.mock('@kbn/ui-actions-plugin/public', () => ({ + ...jest.requireActual('@kbn/ui-actions-plugin/public'), + addFilterIn: () => {}, + addFilterOut: () => {}, +})); + +const { storage } = createSecuritySolutionStorageMock(); +const mockStore = createStore(mockGlobalState, SUB_PLUGINS_REDUCER, kibanaObservable, storage); + +describe('createFilterInDiscoverCellActionFactory', () => { + const createFilterInCellAction = createFilterInDiscoverCellActionFactory({ + store: mockStore, + services, + }); + const filterInAction = createFilterInCellAction({ id: 'testAction' }); + + beforeEach(() => { + currentAppIdSubject$.next(APP_UI_ID); + jest.clearAllMocks(); + }); + + const context = { + data: [{ field: { name: 'user.name', type: 'text' }, value: 'the value' }], + } as SecurityCellActionExecutionContext; + + it('should return display name', () => { + expect(filterInAction.getDisplayName(context)).toEqual('Filter In'); + }); + + it('should return icon type', () => { + expect(filterInAction.getIconType(context)).toEqual('plusInCircle'); + }); + + describe('isCompatible', () => { + it('should return true if everything is okay', async () => { + expect(await filterInAction.isCompatible(context)).toEqual(true); + }); + + it('should return false if not in security', async () => { + currentAppIdSubject$.next('not-security'); + expect(await filterInAction.isCompatible(context)).toEqual(false); + }); + + it('should return false if field not allowed', async () => { + expect( + await filterInAction.isCompatible({ + ...context, + data: [ + { ...context.data[0], field: { ...context.data[0].field, name: 'signal.reason' } }, + ], + }) + ).toEqual(false); + }); + }); + + describe('execution', () => { + it('should execute using generic filterManager', async () => { + await filterInAction.execute(context); + expect(mockGlobalFilterManager.addFilters).toHaveBeenCalled(); + }); + }); +}); diff --git a/x-pack/plugins/security_solution/public/actions/filter/discover/filter_in.ts b/x-pack/plugins/security_solution/public/actions/filter/discover/filter_in.ts new file mode 100644 index 0000000000000..d9d575ef5cbd6 --- /dev/null +++ b/x-pack/plugins/security_solution/public/actions/filter/discover/filter_in.ts @@ -0,0 +1,33 @@ +/* + * 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 type { CellAction, CellActionFactory } from '@kbn/cell-actions'; +import type { SecurityAppStore } from '../../../common/store'; +import { isInSecurityApp } from '../../utils'; +import type { StartServices } from '../../../types'; +import { createFilterInCellActionFactory } from '../cell_action/filter_in'; + +export const createFilterInDiscoverCellActionFactory = ({ + store, + services, +}: { + store: SecurityAppStore; + services: StartServices; +}): CellActionFactory => { + const { application } = services; + + let currentAppId: string | undefined; + application.currentAppId$.subscribe((appId) => { + currentAppId = appId; + }); + + const securityFilterInActionFactory = createFilterInCellActionFactory({ store, services }); + + return securityFilterInActionFactory.combine({ + isCompatible: async () => isInSecurityApp(currentAppId), + }); +}; diff --git a/x-pack/plugins/security_solution/public/actions/filter/discover/filter_out.test.ts b/x-pack/plugins/security_solution/public/actions/filter/discover/filter_out.test.ts new file mode 100644 index 0000000000000..42f6bad3040b8 --- /dev/null +++ b/x-pack/plugins/security_solution/public/actions/filter/discover/filter_out.test.ts @@ -0,0 +1,93 @@ +/* + * 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 { + createSecuritySolutionStorageMock, + kibanaObservable, + mockGlobalState, + SUB_PLUGINS_REDUCER, +} from '../../../common/mock'; +import { createStore } from '../../../common/store'; +import { createFilterOutDiscoverCellActionFactory } from './filter_out'; +import type { SecurityCellActionExecutionContext } from '../../types'; +import { createStartServicesMock } from '../../../common/lib/kibana/kibana_react.mock'; +import { BehaviorSubject } from 'rxjs'; +import { APP_UI_ID } from '../../../../common'; + +const services = createStartServicesMock(); +const mockGlobalFilterManager = services.data.query.filterManager; + +const currentAppIdSubject$ = new BehaviorSubject(APP_UI_ID); +services.application.currentAppId$ = currentAppIdSubject$.asObservable(); + +jest.mock('@kbn/ui-actions-plugin/public', () => ({ + ...jest.requireActual('@kbn/ui-actions-plugin/public'), + addFilterIn: () => {}, + addFilterOut: () => {}, +})); + +const { storage } = createSecuritySolutionStorageMock(); +const mockStore = createStore(mockGlobalState, SUB_PLUGINS_REDUCER, kibanaObservable, storage); + +describe('createFilterOutDiscoverCellActionFactory', () => { + const createFilterOutCellAction = createFilterOutDiscoverCellActionFactory({ + store: mockStore, + services, + }); + const filterOutAction = createFilterOutCellAction({ id: 'testAction' }); + + beforeEach(() => { + currentAppIdSubject$.next(APP_UI_ID); + jest.clearAllMocks(); + }); + + const context = { + data: [ + { + field: { name: 'user.name', type: 'text' }, + value: 'the value', + }, + ], + } as SecurityCellActionExecutionContext; + + it('should return display name', () => { + expect(filterOutAction.getDisplayName(context)).toEqual('Filter Out'); + }); + + it('should return icon type', () => { + expect(filterOutAction.getIconType(context)).toEqual('minusInCircle'); + }); + + describe('isCompatible', () => { + it('should return true if everything is okay', async () => { + expect(await filterOutAction.isCompatible(context)).toEqual(true); + }); + + it('should return false if not in security', async () => { + currentAppIdSubject$.next('not-security'); + expect(await filterOutAction.isCompatible(context)).toEqual(false); + }); + + it('should return false if field not allowed', async () => { + expect( + await filterOutAction.isCompatible({ + ...context, + data: [ + { ...context.data[0], field: { ...context.data[0].field, name: 'signal.reason' } }, + ], + }) + ).toEqual(false); + }); + }); + + describe('execution', () => { + it('should execute using generic filterManager', async () => { + await filterOutAction.execute(context); + expect(mockGlobalFilterManager.addFilters).toHaveBeenCalled(); + }); + }); +}); diff --git a/x-pack/plugins/security_solution/public/actions/filter/discover/filter_out.ts b/x-pack/plugins/security_solution/public/actions/filter/discover/filter_out.ts new file mode 100644 index 0000000000000..07b8d1e61b37a --- /dev/null +++ b/x-pack/plugins/security_solution/public/actions/filter/discover/filter_out.ts @@ -0,0 +1,33 @@ +/* + * 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 type { CellActionFactory, CellAction } from '@kbn/cell-actions'; +import { isInSecurityApp } from '../../utils'; +import type { SecurityAppStore } from '../../../common/store'; +import type { StartServices } from '../../../types'; +import { createFilterOutCellActionFactory } from '../cell_action/filter_out'; + +export const createFilterOutDiscoverCellActionFactory = ({ + store, + services, +}: { + store: SecurityAppStore; + services: StartServices; +}): CellActionFactory => { + const { application } = services; + + let currentAppId: string | undefined; + application.currentAppId$.subscribe((appId) => { + currentAppId = appId; + }); + + const genericFilterOutActionFactory = createFilterOutCellActionFactory({ store, services }); + + return genericFilterOutActionFactory.combine({ + isCompatible: async () => isInSecurityApp(currentAppId), + }); +}; diff --git a/x-pack/plugins/security_solution/public/actions/filter/index.ts b/x-pack/plugins/security_solution/public/actions/filter/index.ts index a6e782d261db7..3eadceb7d55b9 100644 --- a/x-pack/plugins/security_solution/public/actions/filter/index.ts +++ b/x-pack/plugins/security_solution/public/actions/filter/index.ts @@ -7,3 +7,5 @@ export { createFilterInCellActionFactory } from './cell_action/filter_in'; export { createFilterOutCellActionFactory } from './cell_action/filter_out'; +export { createFilterInDiscoverCellActionFactory } from './discover/filter_in'; +export { createFilterOutDiscoverCellActionFactory } from './discover/filter_out'; diff --git a/x-pack/plugins/security_solution/public/actions/register.ts b/x-pack/plugins/security_solution/public/actions/register.ts index 9b6453c9dc8d0..2b11301ff64ae 100644 --- a/x-pack/plugins/security_solution/public/actions/register.ts +++ b/x-pack/plugins/security_solution/public/actions/register.ts @@ -6,35 +6,49 @@ */ import { CELL_VALUE_TRIGGER } from '@kbn/embeddable-plugin/public'; -import type * as H from 'history'; +import type { History } from 'history'; +import { SEARCH_EMBEDDABLE_CELL_ACTIONS_TRIGGER_ID } from '@kbn/discover-plugin/public'; import type { SecurityAppStore } from '../common/store/types'; import type { StartServices } from '../types'; -import { createFilterInCellActionFactory, createFilterOutCellActionFactory } from './filter'; +import { + createFilterInCellActionFactory, + createFilterInDiscoverCellActionFactory, + createFilterOutCellActionFactory, + createFilterOutDiscoverCellActionFactory, +} from './filter'; import { createAddToTimelineLensAction, createAddToTimelineCellActionFactory, createInvestigateInNewTimelineCellActionFactory, + createAddToTimelineDiscoverCellActionFactory, } from './add_to_timeline'; import { createShowTopNCellActionFactory } from './show_top_n'; import { createCopyToClipboardLensAction, createCopyToClipboardCellActionFactory, + createCopyToClipboardDiscoverCellActionFactory, } from './copy_to_clipboard'; import { createToggleColumnCellActionFactory } from './toggle_column'; import { SecurityCellActionsTrigger } from './constants'; -import type { SecurityCellActionName, SecurityCellActions } from './types'; +import type { + DiscoverCellActionName, + DiscoverCellActions, + SecurityCellActionName, + SecurityCellActions, +} from './types'; import { enhanceActionWithTelemetry } from './telemetry'; export const registerUIActions = ( store: SecurityAppStore, - history: H.History, + history: History, services: StartServices ) => { - registerLensActions(store, services); + registerLensEmbeddableActions(store, services); + registerDiscoverCellActions(store, services); registerCellActions(store, history, services); }; -const registerLensActions = (store: SecurityAppStore, services: StartServices) => { +const registerLensEmbeddableActions = (store: SecurityAppStore, services: StartServices) => { const { uiActions } = services; const addToTimelineAction = createAddToTimelineLensAction({ store, order: 1 }); @@ -44,12 +58,47 @@ const registerLensActions = (store: SecurityAppStore, services: StartServices) = uiActions.addTriggerAction(CELL_VALUE_TRIGGER, copyToClipboardAction); }; +const registerDiscoverCellActions = (store: SecurityAppStore, services: StartServices) => { + const { uiActions } = services; + + const DiscoverCellActionsFactories: DiscoverCellActions = { + filterIn: createFilterInDiscoverCellActionFactory({ store, services }), + filterOut: createFilterOutDiscoverCellActionFactory({ store, services }), + addToTimeline: createAddToTimelineDiscoverCellActionFactory({ store, services }), + copyToClipboard: createCopyToClipboardDiscoverCellActionFactory({ services }), + }; + + const addDiscoverEmbeddableCellActions = ( + triggerId: string, + actionsOrder: DiscoverCellActionName[] + ) => { + actionsOrder.forEach((actionName, order) => { + const actionFactory = DiscoverCellActionsFactories[actionName]; + if (actionFactory) { + const action = actionFactory({ id: `${triggerId}-${actionName}`, order }); + const actionWithTelemetry = enhanceActionWithTelemetry(action, services); + uiActions.addTriggerAction(triggerId, actionWithTelemetry); + } + }); + }; + + // this trigger is already registered by discover search embeddable + addDiscoverEmbeddableCellActions(SEARCH_EMBEDDABLE_CELL_ACTIONS_TRIGGER_ID, [ + 'filterIn', + 'filterOut', + 'addToTimeline', + 'copyToClipboard', + ]); +}; + const registerCellActions = ( store: SecurityAppStore, - history: H.History, + history: History, services: StartServices ) => { - const cellActions: SecurityCellActions = { + const { uiActions } = services; + + const cellActionsFactories: SecurityCellActions = { filterIn: createFilterInCellActionFactory({ store, services }), filterOut: createFilterOutCellActionFactory({ store, services }), addToTimeline: createAddToTimelineCellActionFactory({ store, services }), @@ -59,55 +108,37 @@ const registerCellActions = ( toggleColumn: createToggleColumnCellActionFactory({ store }), }; - registerCellActionsTrigger({ - triggerId: SecurityCellActionsTrigger.DEFAULT, - cellActions, - actionsOrder: ['filterIn', 'filterOut', 'addToTimeline', 'showTopN', 'copyToClipboard'], - services, - }); - - registerCellActionsTrigger({ - triggerId: SecurityCellActionsTrigger.DETAILS_FLYOUT, - cellActions, - actionsOrder: [ - 'filterIn', - 'filterOut', - 'addToTimeline', - 'toggleColumn', - 'showTopN', - 'copyToClipboard', - ], - services, - }); - - registerCellActionsTrigger({ - triggerId: SecurityCellActionsTrigger.ALERTS_COUNT, - cellActions, - actionsOrder: ['investigateInNewTimeline'], - services, - }); -}; + const registerCellActionsTrigger = ( + triggerId: SecurityCellActionsTrigger, + actionsOrder: SecurityCellActionName[] + ) => { + uiActions.registerTrigger({ id: triggerId }); + actionsOrder.forEach((actionName, order) => { + const actionFactory = cellActionsFactories[actionName]; + if (actionFactory) { + const action = actionFactory({ id: `${triggerId}-${actionName}`, order }); + const actionWithTelemetry = enhanceActionWithTelemetry(action, services); + uiActions.addTriggerAction(triggerId, actionWithTelemetry); + } + }); + }; -const registerCellActionsTrigger = ({ - triggerId, - cellActions, - actionsOrder, - services, -}: { - triggerId: SecurityCellActionsTrigger; - cellActions: SecurityCellActions; - actionsOrder: SecurityCellActionName[]; - services: StartServices; -}) => { - const { uiActions } = services; - uiActions.registerTrigger({ id: triggerId }); + registerCellActionsTrigger(SecurityCellActionsTrigger.DEFAULT, [ + 'filterIn', + 'filterOut', + 'addToTimeline', + 'showTopN', + 'copyToClipboard', + ]); - actionsOrder.forEach((actionName, order) => { - const actionFactory = cellActions[actionName]; - if (actionFactory) { - const action = actionFactory({ id: `${triggerId}-${actionName}`, order }); + registerCellActionsTrigger(SecurityCellActionsTrigger.DETAILS_FLYOUT, [ + 'filterIn', + 'filterOut', + 'addToTimeline', + 'toggleColumn', + 'showTopN', + 'copyToClipboard', + ]); - uiActions.addTriggerAction(triggerId, enhanceActionWithTelemetry(action, services)); - } - }); + registerCellActionsTrigger(SecurityCellActionsTrigger.ALERTS_COUNT, ['investigateInNewTimeline']); }; diff --git a/x-pack/plugins/security_solution/public/actions/show_top_n/cell_action/show_top_n.tsx b/x-pack/plugins/security_solution/public/actions/show_top_n/cell_action/show_top_n.tsx index fa67739ef8f4b..0d3501df767bd 100644 --- a/x-pack/plugins/security_solution/public/actions/show_top_n/cell_action/show_top_n.tsx +++ b/x-pack/plugins/security_solution/public/actions/show_top_n/cell_action/show_top_n.tsx @@ -6,7 +6,7 @@ */ import React from 'react'; import ReactDOM, { unmountComponentAtNode } from 'react-dom'; -import type * as H from 'history'; +import type { History } from 'history'; import { Provider } from 'react-redux'; import { Router } from 'react-router-dom'; import { i18n } from '@kbn/i18n'; @@ -38,7 +38,7 @@ export const createShowTopNCellActionFactory = createCellActionFactory( services, }: { store: SecurityAppStore; - history: H.History; + history: History; services: StartServices; }): CellActionTemplate => ({ type: SecurityCellActionType.SHOW_TOP_N, diff --git a/x-pack/plugins/security_solution/public/actions/types.ts b/x-pack/plugins/security_solution/public/actions/types.ts index d1e25987205b8..a4c992632b7d6 100644 --- a/x-pack/plugins/security_solution/public/actions/types.ts +++ b/x-pack/plugins/security_solution/public/actions/types.ts @@ -54,14 +54,24 @@ export interface SecurityCellActionExecutionContext extends CellActionExecutionC export type SecurityCellAction = CellAction; export interface SecurityCellActions { - filterIn?: CellActionFactory; - filterOut?: CellActionFactory; - addToTimeline?: CellActionFactory; - investigateInNewTimeline?: CellActionFactory; - showTopN?: CellActionFactory; - copyToClipboard?: CellActionFactory; - toggleColumn?: CellActionFactory; + filterIn: CellActionFactory; + filterOut: CellActionFactory; + addToTimeline: CellActionFactory; + investigateInNewTimeline: CellActionFactory; + showTopN: CellActionFactory; + copyToClipboard: CellActionFactory; + toggleColumn: CellActionFactory; } // All security cell actions names export type SecurityCellActionName = keyof SecurityCellActions; + +export interface DiscoverCellActions { + filterIn: CellActionFactory; + filterOut: CellActionFactory; + addToTimeline: CellActionFactory; + copyToClipboard: CellActionFactory; +} + +// All Discover search embeddable cell actions names +export type DiscoverCellActionName = keyof DiscoverCellActions; diff --git a/x-pack/plugins/security_solution/public/detections/hooks/trigger_actions_alert_table/use_cell_actions.tsx b/x-pack/plugins/security_solution/public/detections/hooks/trigger_actions_alert_table/use_cell_actions.tsx index eb3aa7f714f40..393232924ed3c 100644 --- a/x-pack/plugins/security_solution/public/detections/hooks/trigger_actions_alert_table/use_cell_actions.tsx +++ b/x-pack/plugins/security_solution/public/detections/hooks/trigger_actions_alert_table/use_cell_actions.tsx @@ -61,40 +61,41 @@ export const getUseCellActionsHook = (tableId: TableId) => { useShallowEqualSelector((state) => (getTable(state, tableId) ?? tableDefaults).viewMode) ?? tableDefaults.viewMode; - const cellActionProps = useMemo(() => { - const cellActionsData = - viewMode === VIEW_SELECTION.eventRenderedView - ? [] - : columns.map((col) => { - const fieldMeta: Partial | undefined = browserFieldsByName[col.id]; - return { - // TODO use FieldSpec object instead of browserField - field: { - name: col.id, - type: fieldMeta?.type ?? 'keyword', - esTypes: fieldMeta?.esTypes ?? [], - aggregatable: fieldMeta?.aggregatable ?? false, - searchable: fieldMeta?.searchable ?? false, - subType: fieldMeta?.subType, - }, - values: (finalData as TimelineNonEcsData[][]).map( - (row) => row.find((rowData) => rowData.field === col.id)?.value ?? [] - ), - }; - }); + const cellActionsMetadata = useMemo(() => ({ scopeId: tableId }), []); + + const cellActionsFields = useMemo(() => { + if (viewMode === VIEW_SELECTION.eventRenderedView) { + return undefined; + } + return columns.map((column) => { + // TODO use FieldSpec object instead of browserField + const browserField: Partial | undefined = browserFieldsByName[column.id]; + return { + name: column.id, + type: browserField?.type ?? 'keyword', + esTypes: browserField?.esTypes ?? [], + aggregatable: browserField?.aggregatable ?? false, + searchable: browserField?.searchable ?? false, + subType: browserField?.subType, + }; + }); + }, [browserFieldsByName, columns, viewMode]); - return { - triggerId: SecurityCellActionsTrigger.DEFAULT, - data: cellActionsData, - metadata: { - // cell actions scope - scopeId: tableId, - }, - dataGridRef, - }; - }, [viewMode, browserFieldsByName, columns, finalData, dataGridRef]); + const getCellValue = useCallback( + (fieldName, rowIndex) => { + const pageRowIndex = rowIndex % finalData.length; + return finalData[pageRowIndex].find((rowData) => rowData.field === fieldName)?.value ?? []; + }, + [finalData] + ); - const cellActions = useDataGridColumnsSecurityCellActions(cellActionProps); + const cellActions = useDataGridColumnsSecurityCellActions({ + triggerId: SecurityCellActionsTrigger.DEFAULT, + fields: cellActionsFields, + getCellValue, + metadata: cellActionsMetadata, + dataGridRef, + }); const getCellActions = useCallback( (_columnId: string, columnIndex: number) => { diff --git a/x-pack/plugins/security_solution/tsconfig.json b/x-pack/plugins/security_solution/tsconfig.json index a4d2e2873a072..bb7c6ce5ea4b6 100644 --- a/x-pack/plugins/security_solution/tsconfig.json +++ b/x-pack/plugins/security_solution/tsconfig.json @@ -159,6 +159,7 @@ "@kbn/ecs", "@kbn/url-state", "@kbn/ml-anomaly-utils", + "@kbn/discover-plugin", "@kbn/field-formats-plugin", "@kbn/dev-proc-runner", "@kbn/cloud-chat-plugin" From 498e8a625b863c956111f94257f08a87fdcd7122 Mon Sep 17 00:00:00 2001 From: Carlos Crespo Date: Fri, 23 Jun 2023 16:41:35 +0200 Subject: [PATCH 02/54] [Stack Monitoring] pipeline_viewer test - retry expected validations (#160162) closes: [#156552](https://github.com/elastic/kibana/issues/156552) ## Summary This PR fixes a problem that rarely occurs when the test validates the information on Logstash Pipelines page. Despite showing [correct values on the screen ](https://buildkiteartifacts.com/e0f3970e-3a75-4621-919f-e6c773e2bb12/b1106efc-ba5e-4e90-a857-11c4a899c05d/01889078-e04d-4aba-af2b-20750be02e1d/0188907a-dabf-46fb-80a2-b2cf17423cb2/x-pack/test/functional/screenshots/failure/Monitoring%20app%20Logstash%20pipeline%20viewer%20Should%20change%20the%20pipeline%20data%20when%20dat-fcb1376d2ab5ef50c3db3c8ed141d694094152ee82d117c6238c7010cdf2d45a.png?response-content-type=image%2Fpng&X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=ASIAQPCP3C7LT46Z72NL%2F20230621%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20230621T153732Z&X-Amz-Expires=600&X-Amz-Security-Token=IQoJb3JpZ2luX2VjEGYaCXVzLWVhc3QtMSJIMEYCIQCfvEYxUA85BiK5lIHfTw%2F7XOg3IYibuqN3BzfSuodh6AIhAIAHNIJVCq2YtaV6g0rIpRK2krPD%2FvO5EmJUNiB7O8miKvoDCL%2F%2F%2F%2F%2F%2F%2F%2F%2F%2F%2FwEQABoMMDMyMzc5NzA1MzAzIgyN%2F7Q4xkvrfgzG37cqzgM9arF2HpAZ4WBbBWvOQEINgBAbuk7ZSbuqOTof3CpEEp%2Bnq7%2B%2FHEAClbl5ky1KQcoPU7OTe8BLZsGc8HcDijsGQUkgHwtPIU64u8OCuYs0Bql%2F1ZCfaXM%2FeuMnlnw85C98nn4hlJDFMFb874LHTqxwDlQBC3EMdvp%2FycSuZKii7taE3ouP3Nasx7VY5RrXP4jdW6%2BJlM12Rkiiylaond088pfI7tlURK00Bwk26yNrvNV3LmgHwz4JFFXuNIT0vmKpJMEPStrTd43Ed3rffP7DESw4GwNrZehI6WZeOVfP2k7EYOhDQZ1iHOU8L%2FbbYwZeNpTtSofhSZWFdhpnB8R8RvWbAQMzqqJ9KNiAnbTezZL0Y3aMRsei3d88z7XFufETlUGygUNYcHFLfxFg%2BCxRnDFvTxLJO86RM2dxUM%2BvTQ5hOQUG2%2FhApLF7wtYfuc2npbO0Sdtr0ejSAHf8Pg72zzWPkMKTBoFsEzzLfF%2BF1GH7ZLL2SYyBfUk5kJ5w%2BoXMQNESlbUX1lcwqJIla8gEtVvEBfQWYVT830iQ0xwTq60cwRxBYBcDqGi4IbUSMz3yhUNVvQYhKkvoghLEcLLlfp58GTim%2BYCMoynwK4ww%2Fv3LpAY6pAFX3eWSDHctpVoWwNhI%2FCgMiBSkpZhIj%2Fy3%2BYwq2NTp7Ydj5beZKMBuRfOhbFby7uiHoBIdOiYFKl8AGke0U1%2FOPWGZcPVeCfV96AhVYnJs20kAKjYV79s9ZkbEQf69z43aVyL6mFDg052dhX78nmkI5drcxO9GTMVQ98U7KdKzxDvg4846cxePX8UWzLl2Xj96TS05Jx3dl2mu6Bw8uKj58eKq7g%3D%3D&X-Amz-SignedHeaders=host&X-Amz-Signature=3b3fdc513ac1b376f93a28de38b97fd154a3b0edaa2ef417167492471471ad57), the test still fails ### How to test - `yarn test:ftr:server --config x-pack/test/functional/apps/monitoring/config.ts` - `yarn test:ftr:runner --config x-pack/test/functional/apps/monitoring/config.ts --include x-pack/test/functional/apps/monitoring/logstash/pipeline_viewer.js` Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../monitoring/logstash/pipeline_viewer.js | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/x-pack/test/functional/apps/monitoring/logstash/pipeline_viewer.js b/x-pack/test/functional/apps/monitoring/logstash/pipeline_viewer.js index 205674ea3ee44..bc9e366f18fe2 100644 --- a/x-pack/test/functional/apps/monitoring/logstash/pipeline_viewer.js +++ b/x-pack/test/functional/apps/monitoring/logstash/pipeline_viewer.js @@ -13,6 +13,7 @@ export default function ({ getService, getPageObjects }) { const pipelinesList = getService('monitoringLogstashPipelines'); const pipelineViewer = getService('monitoringLogstashPipelineViewer'); const pageObjects = getPageObjects(['timePicker']); + const retry = getService('retry'); describe('Logstash pipeline viewer', () => { const { setup, tearDown } = getLifecycleMethods(getService, getPageObjects); @@ -52,15 +53,17 @@ export default function ({ getService, getPageObjects }) { 'Jan 22, 2018 @ 10:00:00.000' ); - const { inputs, filters, outputs } = await pipelineViewer.getPipelineDefinition(); + await retry.try(async () => { + const { inputs, filters, outputs } = await pipelineViewer.getPipelineDefinition(); - expect(inputs).to.eql([{ name: 'generator', metrics: ['mygen01', '643.75 e/s emitted'] }]); - expect(filters).to.eql([ - { name: 'sleep', metrics: ['1%', '96.37 ms/e', '643.75 e/s received'] }, - ]); - expect(outputs).to.eql([ - { name: 'stdout', metrics: ['0%', '0 ms/e', '643.75 e/s received'] }, - ]); + expect(inputs).to.eql([{ name: 'generator', metrics: ['mygen01', '643.75 e/s emitted'] }]); + expect(filters).to.eql([ + { name: 'sleep', metrics: ['1%', '96.37 ms/e', '643.75 e/s received'] }, + ]); + expect(outputs).to.eql([ + { name: 'stdout', metrics: ['0%', '0 ms/e', '643.75 e/s received'] }, + ]); + }); }); }); } From 40c2afdc5894dc163c8e196d3011293875b36f6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20C=C3=B4t=C3=A9?= Date: Fri, 23 Jun 2023 10:41:55 -0400 Subject: [PATCH 03/54] Add support to Task Manager for validating and versioning the task state objects (#159048) Part of https://github.com/elastic/kibana/issues/155764. In this PR, I'm modifying task manager to allow task types to report a versioned schema for the `state` object. When defining `stateSchemaByVersion`, the following will happen: - The `state` returned from the task runner will get validated against the latest version and throw an error if ever it is invalid (to capture mismatches at development and testing time) - When task manager reads a task, it will migrate the task state to the latest version (if necessary) and validate against the latest schema, dropping any unknown fields (in the scenario of a downgrade). By default, Task Manager will validate the state on write once a versioned schema is provided, however the following config must be enabled for errors to be thrown on read: `xpack.task_manager.allow_reading_invalid_state: true`. We plan to enable this in serverless by default but be cautious on existing deployments and wait for telemetry to show no issues. I've onboarded the `alerts_invalidate_api_keys` task type which can be used as an example to onboard others. See [this commit](https://github.com/elastic/kibana/pull/159048/commits/214bae38d89409d4f5527887f46ce9c4988146d1). ### How to configure a task type to version and validate The structure is defined as: ``` taskManager.registerTaskDefinitions({ ... stateSchemaByVersion: { 1: { // All existing tasks don't have a version so will get `up` migrated to 1 up: (state: Record) => ({ runs: state.runs || 0, total_invalidated: state.total_invalidated || 0, }), schema: schema.object({ runs: schema.number(), total_invalidated: schema.number(), }), }, }, ... }); ``` However, look at [this commit](https://github.com/elastic/kibana/pull/159048/commits/214bae38d89409d4f5527887f46ce9c4988146d1) for an example that you can leverage type safety from the schema. ### Follow up issues - Onboard non-alerting task types to have a versioned state schema (https://github.com/elastic/kibana/issues/159342) - Onboard alerting task types to have a versioned state schema for the framework fields (https://github.com/elastic/kibana/issues/159343) - Onboard alerting task types to have a versioned rule and alert state schema within the task state (https://github.com/elastic/kibana/issues/159344) - Telemetry on the validation failures (https://github.com/elastic/kibana/issues/159345) - Remove feature flag so `allow_reading_invalid_state` is always `false` (https://github.com/elastic/kibana/issues/159346) - Force validation on all tasks using state by removing the exemption code (https://github.com/elastic/kibana/issues/159347) - Release tasks when encountering a validation failure after run (https://github.com/elastic/kibana/issues/159964) ### To Verify NOTE: I have the following verification scenarios in a jest integration test as well => https://github.com/elastic/kibana/pull/159048/files#diff-5f06228df58fa74d5a0f2722c30f1f4bee2ee9df7a14e0700b9aa9bc3864a858. You will need to log the state when the task runs to observe what the task runner receives in different scenarios. ``` diff --git a/x-pack/plugins/alerting/server/invalidate_pending_api_keys/task.ts b/x-pack/plugins/alerting/server/invalidate_pending_api_keys/task.ts index 1e624bcd807..4aa4c2c7805 100644 --- a/x-pack/plugins/alerting/server/invalidate_pending_api_keys/task.ts +++ b/x-pack/plugins/alerting/server/invalidate_pending_api_keys/task.ts @@ -140,6 +140,7 @@ function taskRunner( ) { return ({ taskInstance }: RunContext) => { const state = taskInstance.state as LatestTaskStateSchema; + console.log('*** Running task with the following state:', JSON.stringify(state)); return { async run() { let totalInvalidated = 0; ``` #### Scenario 1: Adding an unknown field to the task saved-object gets dropped 1. Startup a fresh Kibana instance 2. Make the following call to Elasticsearch (I used postman). This call adds an unknown property (`foo`) to the task state and makes the task run right away. ``` POST http://kibana_system:changeme@localhost:9200/.kibana_task_manager/_update/task:Alerts-alerts_invalidate_api_keys { "doc": { "task": { "runAt": "2023-06-08T00:00:00.000Z", "state": "{\"runs\":1,\"total_invalidated\":0,\"foo\":true}" } } } ``` 3. Observe the task run log message, with state not containing `foo`. #### Scenario 2: Task running returning an unknown property causes the task to fail to update 1. Apply the following changes to the code (and ignore TypeScript issues) ``` diff --git a/x-pack/plugins/alerting/server/invalidate_pending_api_keys/task.ts b/x-pack/plugins/alerting/server/invalidate_pending_api_keys/task.ts index 1e624bcd807..b15d4a4f478 100644 --- a/x-pack/plugins/alerting/server/invalidate_pending_api_keys/task.ts +++ b/x-pack/plugins/alerting/server/invalidate_pending_api_keys/task.ts @@ -183,6 +183,7 @@ function taskRunner( const updatedState: LatestTaskStateSchema = { runs: (state.runs || 0) + 1, + foo: true, total_invalidated: totalInvalidated, }; return { ``` 2. Make the task run right away by calling Elasticsearch with the following ``` POST http://kibana_system:changeme@localhost:9200/.kibana_task_manager/_update/task:Alerts-alerts_invalidate_api_keys { "doc": { "task": { "runAt": "2023-06-08T00:00:00.000Z" } } } ``` 3. Notice the validation errors logged as debug ``` [ERROR][plugins.taskManager] Task alerts_invalidate_api_keys "Alerts-alerts_invalidate_api_keys" failed: Error: [foo]: definition for this key is missing ``` #### Scenario 3: Task state gets migrated 1. Apply the following code change ``` diff --git a/x-pack/plugins/alerting/server/invalidate_pending_api_keys/task.ts b/x-pack/plugins/alerting/server/invalidate_pending_api_keys/task.ts index 1e624bcd807..338f21bed5b 100644 --- a/x-pack/plugins/alerting/server/invalidate_pending_api_keys/task.ts +++ b/x-pack/plugins/alerting/server/invalidate_pending_api_keys/task.ts @@ -41,6 +41,18 @@ const stateSchemaByVersion = { total_invalidated: schema.number(), }), }, + 2: { + up: (state: Record) => ({ + runs: state.runs, + total_invalidated: state.total_invalidated, + foo: true, + }), + schema: schema.object({ + runs: schema.number(), + total_invalidated: schema.number(), + foo: schema.boolean(), + }), + }, }; const latestSchema = stateSchemaByVersion[1].schema; ``` 2. Make the task run right away by calling Elasticsearch with the following ``` POST http://kibana_system:changeme@localhost:9200/.kibana_task_manager/_update/task:Alerts-alerts_invalidate_api_keys { "doc": { "task": { "runAt": "2023-06-08T00:00:00.000Z" } } } ``` 3. Observe the state now contains `foo` property when the task runs. #### Scenario 4: Reading invalid state causes debug logs 1. Run the following request to Elasticsearch ``` POST http://kibana_system:changeme@localhost:9200/.kibana_task_manager/_update/task:Alerts-alerts_invalidate_api_keys { "doc": { "task": { "runAt": "2023-06-08T00:00:00.000Z", "state": "{}" } } } ``` 2. Observe the Kibana debug log mentioning the validation failure while letting the task through ``` [DEBUG][plugins.taskManager] [alerts_invalidate_api_keys][Alerts-alerts_invalidate_api_keys] Failed to validate the task's state. Allowing read operation to proceed because allow_reading_invalid_state is true. Error: [runs]: expected value of type [number] but got [undefined] ``` #### Scenario 5: Reading invalid state when setting `allow_reading_invalid_state: false` causes tasks to fail to run 1. Set `xpack.task_manager.allow_reading_invalid_state: false` in your kibana.yml settings 2. Run the following request to Elasticsearch ``` POST http://kibana_system:changeme@localhost:9200/.kibana_task_manager/_update/task:Alerts-alerts_invalidate_api_keys { "doc": { "task": { "runAt": "2023-06-08T00:00:00.000Z", "state": "{}" } } } ``` 3. Observe the Kibana error log mentioning the validation failure ``` [ERROR][plugins.taskManager] Failed to poll for work: Error: [runs]: expected value of type [number] but got [undefined] ``` NOTE: While corrupting the task directly is rare, we plan to re-queue the tasks that failed to read, leveraging work from https://github.com/elastic/kibana/issues/159302 in a future PR (hence why the yml config is enabled by default, allowing invalid reads). --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Ying Mao --- .../invalidate_pending_api_keys/task.ts | 46 +- x-pack/plugins/task_manager/README.md | 14 + .../server/buffered_task_store.mock.ts | 21 + .../server/buffered_task_store.test.ts | 46 +- .../server/buffered_task_store.ts | 29 +- .../task_manager/server/config.test.ts | 3 + x-pack/plugins/task_manager/server/config.ts | 1 + .../server/ephemeral_task_lifecycle.test.ts | 1 + .../server/integration_tests/lib/index.ts | 10 + .../integration_tests/lib/inject_task.ts | 32 ++ .../server/integration_tests/lib/retry.ts | 29 ++ .../lib/setup_test_servers.ts | 56 +++ .../managed_configuration.test.ts | 1 + .../task_state_validation.test.ts | 326 ++++++++++++++ .../server/lib/bulk_operation_buffer.test.ts | 6 +- .../server/lib/bulk_operation_buffer.ts | 2 +- .../server/lib/retryable_bulk_update.test.ts | 19 +- .../server/lib/retryable_bulk_update.ts | 4 +- .../configuration_statistics.test.ts | 1 + .../monitoring_stats_stream.test.ts | 1 + .../task_manager/server/plugin.test.ts | 1 + x-pack/plugins/task_manager/server/plugin.ts | 2 + .../server/polling_lifecycle.test.ts | 1 + x-pack/plugins/task_manager/server/task.ts | 19 +- .../server/task_running/task_runner.test.ts | 41 +- .../server/task_running/task_runner.ts | 69 +-- .../server/task_scheduling.test.ts | 6 +- .../task_manager/server/task_scheduling.ts | 18 +- .../task_manager/server/task_store.mock.ts | 4 + .../task_manager/server/task_store.test.ts | 206 ++++++++- .../plugins/task_manager/server/task_store.ts | 98 +++-- .../server/task_type_dictionary.ts | 8 + .../server/task_validator.test.ts | 397 ++++++++++++++++++ .../task_manager/server/task_validator.ts | 205 +++++++++ x-pack/plugins/task_manager/tsconfig.json | 3 +- 35 files changed, 1595 insertions(+), 131 deletions(-) create mode 100644 x-pack/plugins/task_manager/server/buffered_task_store.mock.ts create mode 100644 x-pack/plugins/task_manager/server/integration_tests/lib/index.ts create mode 100644 x-pack/plugins/task_manager/server/integration_tests/lib/inject_task.ts create mode 100644 x-pack/plugins/task_manager/server/integration_tests/lib/retry.ts create mode 100644 x-pack/plugins/task_manager/server/integration_tests/lib/setup_test_servers.ts create mode 100644 x-pack/plugins/task_manager/server/integration_tests/task_state_validation.test.ts create mode 100644 x-pack/plugins/task_manager/server/task_validator.test.ts create mode 100644 x-pack/plugins/task_manager/server/task_validator.ts diff --git a/x-pack/plugins/alerting/server/invalidate_pending_api_keys/task.ts b/x-pack/plugins/alerting/server/invalidate_pending_api_keys/task.ts index f1e97c9fd93d0..1e624bcd807cf 100644 --- a/x-pack/plugins/alerting/server/invalidate_pending_api_keys/task.ts +++ b/x-pack/plugins/alerting/server/invalidate_pending_api_keys/task.ts @@ -12,6 +12,7 @@ import { KibanaRequest, SavedObjectsClientContract, } from '@kbn/core/server'; +import { schema, TypeOf } from '@kbn/config-schema'; import { EncryptedSavedObjectsClient } from '@kbn/encrypted-saved-objects-plugin/server'; import { InvalidateAPIKeysParams, SecurityPluginStart } from '@kbn/security-plugin/server'; import { @@ -29,6 +30,22 @@ import { InvalidatePendingApiKey } from '../types'; const TASK_TYPE = 'alerts_invalidate_api_keys'; export const TASK_ID = `Alerts-${TASK_TYPE}`; +const stateSchemaByVersion = { + 1: { + up: (state: Record) => ({ + runs: state.runs || 0, + total_invalidated: state.total_invalidated || 0, + }), + schema: schema.object({ + runs: schema.number(), + total_invalidated: schema.number(), + }), + }, +}; + +const latestSchema = stateSchemaByVersion[1].schema; +type LatestTaskStateSchema = TypeOf; + const invalidateAPIKeys = async ( params: InvalidateAPIKeysParams, securityPluginStart?: SecurityPluginStart @@ -65,17 +82,21 @@ export async function scheduleApiKeyInvalidatorTask( ) { const interval = config.invalidateApiKeysTask.interval; try { + const state: LatestTaskStateSchema = { + runs: 0, + total_invalidated: 0, + }; await taskManager.ensureScheduled({ id: TASK_ID, taskType: TASK_TYPE, schedule: { interval, }, - state: {}, + state, params: {}, }); } catch (e) { - logger.debug(`Error scheduling task, received ${e.message}`); + logger.error(`Error scheduling ${TASK_ID} task, received ${e.message}`); } } @@ -88,6 +109,7 @@ function registerApiKeyInvalidatorTaskDefinition( taskManager.registerTaskDefinitions({ [TASK_TYPE]: { title: 'Invalidate alert API Keys', + stateSchemaByVersion, createTaskRunner: taskRunner(logger, coreStartServices, config), }, }); @@ -117,7 +139,7 @@ function taskRunner( config: AlertingConfig ) { return ({ taskInstance }: RunContext) => { - const { state } = taskInstance; + const state = taskInstance.state as LatestTaskStateSchema; return { async run() { let totalInvalidated = 0; @@ -159,22 +181,24 @@ function taskRunner( hasApiKeysPendingInvalidation = apiKeysToInvalidate.total > PAGE_SIZE; } while (hasApiKeysPendingInvalidation); + const updatedState: LatestTaskStateSchema = { + runs: (state.runs || 0) + 1, + total_invalidated: totalInvalidated, + }; return { - state: { - runs: (state.runs || 0) + 1, - total_invalidated: totalInvalidated, - }, + state: updatedState, schedule: { interval: config.invalidateApiKeysTask.interval, }, }; } catch (e) { logger.warn(`Error executing alerting apiKey invalidation task: ${e.message}`); + const updatedState: LatestTaskStateSchema = { + runs: state.runs + 1, + total_invalidated: totalInvalidated, + }; return { - state: { - runs: (state.runs || 0) + 1, - total_invalidated: totalInvalidated, - }, + state: updatedState, schedule: { interval: config.invalidateApiKeysTask.interval, }, diff --git a/x-pack/plugins/task_manager/README.md b/x-pack/plugins/task_manager/README.md index dd6565ae16d52..c7ca9e5eeea08 100644 --- a/x-pack/plugins/task_manager/README.md +++ b/x-pack/plugins/task_manager/README.md @@ -92,6 +92,20 @@ export class Plugin { // can add significant load to the ES cluster, so please use this configuration only when absolutly necesery. maxConcurrency: 1, + // To ensure the validity of task state during read and write operations, utilize the stateSchemaByVersion configuration. This functionality validates the state before executing a task. Make sure to define the schema property using the @kbn/config-schema plugin, specifically as an ObjectType (schema.object) at the top level. + stateSchemaByVersion: { + 1: { + schema: schema.object({ + count: schema.number(), + }), + up: (state) => { + return { + count: state.count || 0, + }; + }, + } + } + // The createTaskRunner function / method returns an object that is responsible for // performing the work of the task. context: { taskInstance }, is documented below. createTaskRunner(context) { diff --git a/x-pack/plugins/task_manager/server/buffered_task_store.mock.ts b/x-pack/plugins/task_manager/server/buffered_task_store.mock.ts new file mode 100644 index 0000000000000..efdea5a49bc38 --- /dev/null +++ b/x-pack/plugins/task_manager/server/buffered_task_store.mock.ts @@ -0,0 +1,21 @@ +/* + * 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 type { PublicMethodsOf } from '@kbn/utility-types'; +import { BufferedTaskStore } from './buffered_task_store'; + +const createBufferedTaskStoreMock = () => { + const mocked: jest.Mocked> = { + update: jest.fn(), + remove: jest.fn(), + }; + return mocked; +}; + +export const bufferedTaskStoreMock = { + create: createBufferedTaskStoreMock, +}; diff --git a/x-pack/plugins/task_manager/server/buffered_task_store.test.ts b/x-pack/plugins/task_manager/server/buffered_task_store.test.ts index 935192f7cc48e..08d84c8236cf1 100644 --- a/x-pack/plugins/task_manager/server/buffered_task_store.test.ts +++ b/x-pack/plugins/task_manager/server/buffered_task_store.test.ts @@ -29,8 +29,36 @@ describe('Buffered Task Store', () => { taskStore.bulkUpdate.mockResolvedValue([asOk(task)]); - expect(await bufferedStore.update(task)).toMatchObject(task); - expect(taskStore.bulkUpdate).toHaveBeenCalledWith([task]); + expect(await bufferedStore.update(task, { validate: true })).toMatchObject(task); + expect(taskStore.bulkUpdate).toHaveBeenCalledWith([task], { validate: false }); + + expect(taskStore.taskValidator.getValidatedTaskInstanceForUpdating).toHaveBeenCalledTimes(1); + expect(taskStore.taskValidator.getValidatedTaskInstanceFromReading).toHaveBeenCalledTimes(1); + expect(taskStore.taskValidator.getValidatedTaskInstanceForUpdating).toHaveBeenCalledWith( + task, + { validate: true } + ); + expect(taskStore.taskValidator.getValidatedTaskInstanceFromReading).toHaveBeenCalledWith( + task, + { validate: true } + ); + }); + + test(`doesn't validate when specified`, async () => { + const taskStore = taskStoreMock.create(); + const bufferedStore = new BufferedTaskStore(taskStore, {}); + + const task = taskManagerMock.createTask(); + + taskStore.bulkUpdate.mockResolvedValue([asOk(task)]); + + expect(await bufferedStore.update(task, { validate: false })).toMatchObject(task); + expect(taskStore.bulkUpdate).toHaveBeenCalledWith([task], { validate: false }); + + expect(taskStore.taskValidator.getValidatedTaskInstanceForUpdating).toHaveBeenCalledWith( + task, + { validate: false } + ); }); test('handles partially successfull bulkUpdates resolving each call appropriately', async () => { @@ -58,9 +86,9 @@ describe('Buffered Task Store', () => { ]); const results = [ - bufferedStore.update(tasks[0]), - bufferedStore.update(tasks[1]), - bufferedStore.update(tasks[2]), + bufferedStore.update(tasks[0], { validate: true }), + bufferedStore.update(tasks[1], { validate: true }), + bufferedStore.update(tasks[2], { validate: true }), ]; expect(await results[0]).toMatchObject(tasks[0]); expect(results[1]).rejects.toMatchInlineSnapshot(` @@ -105,10 +133,10 @@ describe('Buffered Task Store', () => { ]); const results = [ - bufferedStore.update(tasks[0]), - bufferedStore.update(tasks[1]), - bufferedStore.update(tasks[2]), - bufferedStore.update(tasks[3]), + bufferedStore.update(tasks[0], { validate: true }), + bufferedStore.update(tasks[1], { validate: true }), + bufferedStore.update(tasks[2], { validate: true }), + bufferedStore.update(tasks[3], { validate: true }), ]; expect(await results[0]).toMatchObject(tasks[0]); expect(results[1]).rejects.toMatchInlineSnapshot(` diff --git a/x-pack/plugins/task_manager/server/buffered_task_store.ts b/x-pack/plugins/task_manager/server/buffered_task_store.ts index 0e24e8e49e43f..4135164c1e442 100644 --- a/x-pack/plugins/task_manager/server/buffered_task_store.ts +++ b/x-pack/plugins/task_manager/server/buffered_task_store.ts @@ -17,14 +17,31 @@ const DEFAULT_BUFFER_MAX_DURATION = 50; export class BufferedTaskStore implements Updatable { private bufferedUpdate: Operation; constructor(private readonly taskStore: TaskStore, options: BufferOptions) { - this.bufferedUpdate = createBuffer((docs) => taskStore.bulkUpdate(docs), { - bufferMaxDuration: DEFAULT_BUFFER_MAX_DURATION, - ...options, - }); + this.bufferedUpdate = createBuffer( + // Setting validate: false because we'll validate per update call + // + // Ideally we could accumulate the "validate" options and pass them + // to .bulkUpdate per doc, but the required changes to the bulk_operation_buffer + // to track the values are high and deffered for now. + (docs) => taskStore.bulkUpdate(docs, { validate: false }), + { + bufferMaxDuration: DEFAULT_BUFFER_MAX_DURATION, + ...options, + } + ); } - public async update(doc: ConcreteTaskInstance): Promise { - return unwrapPromise(this.bufferedUpdate(doc)); + public async update( + doc: ConcreteTaskInstance, + options: { validate: boolean } + ): Promise { + const docToUpdate = this.taskStore.taskValidator.getValidatedTaskInstanceForUpdating(doc, { + validate: options.validate, + }); + const result = await unwrapPromise(this.bufferedUpdate(docToUpdate)); + return this.taskStore.taskValidator.getValidatedTaskInstanceFromReading(result, { + validate: options.validate, + }); } public async remove(id: string): Promise { diff --git a/x-pack/plugins/task_manager/server/config.test.ts b/x-pack/plugins/task_manager/server/config.test.ts index d2679efc5042a..4eca3e971de54 100644 --- a/x-pack/plugins/task_manager/server/config.test.ts +++ b/x-pack/plugins/task_manager/server/config.test.ts @@ -12,6 +12,7 @@ describe('config validation', () => { const config: Record = {}; expect(configSchema.validate(config)).toMatchInlineSnapshot(` Object { + "allow_reading_invalid_state": true, "ephemeral_tasks": Object { "enabled": false, "request_capacity": 10, @@ -64,6 +65,7 @@ describe('config validation', () => { const config: Record = {}; expect(configSchema.validate(config)).toMatchInlineSnapshot(` Object { + "allow_reading_invalid_state": true, "ephemeral_tasks": Object { "enabled": false, "request_capacity": 10, @@ -114,6 +116,7 @@ describe('config validation', () => { }; expect(configSchema.validate(config)).toMatchInlineSnapshot(` Object { + "allow_reading_invalid_state": true, "ephemeral_tasks": Object { "enabled": false, "request_capacity": 10, diff --git a/x-pack/plugins/task_manager/server/config.ts b/x-pack/plugins/task_manager/server/config.ts index dffc9677c7c77..2025b44d5b50d 100644 --- a/x-pack/plugins/task_manager/server/config.ts +++ b/x-pack/plugins/task_manager/server/config.ts @@ -137,6 +137,7 @@ export const configSchema = schema.object( exclude_task_types: schema.arrayOf(schema.string(), { defaultValue: [] }), authenticate_background_task_utilization: schema.boolean({ defaultValue: true }), }), + allow_reading_invalid_state: schema.boolean({ defaultValue: true }), }, { validate: (config) => { diff --git a/x-pack/plugins/task_manager/server/ephemeral_task_lifecycle.test.ts b/x-pack/plugins/task_manager/server/ephemeral_task_lifecycle.test.ts index 586fa9db5d446..a12ce476ec2f7 100644 --- a/x-pack/plugins/task_manager/server/ephemeral_task_lifecycle.test.ts +++ b/x-pack/plugins/task_manager/server/ephemeral_task_lifecycle.test.ts @@ -50,6 +50,7 @@ describe('EphemeralTaskLifecycle', () => { poll_interval: 6000000, version_conflict_threshold: 80, request_capacity: 1000, + allow_reading_invalid_state: false, monitored_aggregated_stats_refresh_rate: 5000, monitored_stats_required_freshness: 5000, monitored_stats_running_average_window: 50, diff --git a/x-pack/plugins/task_manager/server/integration_tests/lib/index.ts b/x-pack/plugins/task_manager/server/integration_tests/lib/index.ts new file mode 100644 index 0000000000000..ab7f23c98e06f --- /dev/null +++ b/x-pack/plugins/task_manager/server/integration_tests/lib/index.ts @@ -0,0 +1,10 @@ +/* + * 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. + */ + +export { injectTask } from './inject_task'; +export { setupTestServers } from './setup_test_servers'; +export { retry } from './retry'; diff --git a/x-pack/plugins/task_manager/server/integration_tests/lib/inject_task.ts b/x-pack/plugins/task_manager/server/integration_tests/lib/inject_task.ts new file mode 100644 index 0000000000000..2624c234dd526 --- /dev/null +++ b/x-pack/plugins/task_manager/server/integration_tests/lib/inject_task.ts @@ -0,0 +1,32 @@ +/* + * 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 { type ElasticsearchClient } from '@kbn/core/server'; +import { type ConcreteTaskInstance } from '../../task'; + +export async function injectTask( + esClient: ElasticsearchClient, + { id, ...task }: ConcreteTaskInstance +) { + const soId = `task:${id}`; + await esClient.index({ + id: soId, + index: '.kibana_task_manager', + document: { + references: [], + type: 'task', + updated_at: new Date().toISOString(), + task: { + ...task, + state: JSON.stringify(task.state), + params: JSON.stringify(task.params), + runAt: task.runAt.toISOString(), + scheduledAt: task.scheduledAt.toISOString(), + }, + }, + }); +} diff --git a/x-pack/plugins/task_manager/server/integration_tests/lib/retry.ts b/x-pack/plugins/task_manager/server/integration_tests/lib/retry.ts new file mode 100644 index 0000000000000..17aa83c6479bf --- /dev/null +++ b/x-pack/plugins/task_manager/server/integration_tests/lib/retry.ts @@ -0,0 +1,29 @@ +/* + * 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. + */ + +interface RetryOpts { + times: number; + intervalMs: number; +} + +export async function retry( + cb: () => Promise, + options: RetryOpts = { times: 60, intervalMs: 500 } +) { + let attempt = 1; + while (true) { + try { + return await cb(); + } catch (e) { + if (attempt >= options.times) { + throw e; + } + } + attempt++; + await new Promise((resolve) => setTimeout(resolve, options.intervalMs)); + } +} diff --git a/x-pack/plugins/task_manager/server/integration_tests/lib/setup_test_servers.ts b/x-pack/plugins/task_manager/server/integration_tests/lib/setup_test_servers.ts new file mode 100644 index 0000000000000..7ded9629eb31e --- /dev/null +++ b/x-pack/plugins/task_manager/server/integration_tests/lib/setup_test_servers.ts @@ -0,0 +1,56 @@ +/* + * 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 deepmerge from 'deepmerge'; +import { createTestServers, createRootWithCorePlugins } from '@kbn/core-test-helpers-kbn-server'; + +export async function setupTestServers(settings = {}) { + const { startES } = createTestServers({ + adjustTimeout: (t) => jest.setTimeout(t), + settings: { + es: { + license: 'trial', + }, + }, + }); + + const esServer = await startES(); + + const root = createRootWithCorePlugins( + deepmerge( + { + logging: { + root: { + level: 'warn', + }, + loggers: [ + { + name: 'plugins.taskManager', + level: 'all', + }, + ], + }, + }, + settings + ), + { oss: false } + ); + + await root.preboot(); + const coreSetup = await root.setup(); + const coreStart = await root.start(); + + return { + esServer, + kibanaServer: { + root, + coreSetup, + coreStart, + stop: async () => await root.shutdown(), + }, + }; +} diff --git a/x-pack/plugins/task_manager/server/integration_tests/managed_configuration.test.ts b/x-pack/plugins/task_manager/server/integration_tests/managed_configuration.test.ts index 4b024ac1e7e1b..5ba49664895cd 100644 --- a/x-pack/plugins/task_manager/server/integration_tests/managed_configuration.test.ts +++ b/x-pack/plugins/task_manager/server/integration_tests/managed_configuration.test.ts @@ -43,6 +43,7 @@ describe('managed configuration', () => { max_workers: 10, max_attempts: 9, poll_interval: 3000, + allow_reading_invalid_state: false, version_conflict_threshold: 80, monitored_aggregated_stats_refresh_rate: 60000, monitored_stats_health_verbose_log: { diff --git a/x-pack/plugins/task_manager/server/integration_tests/task_state_validation.test.ts b/x-pack/plugins/task_manager/server/integration_tests/task_state_validation.test.ts new file mode 100644 index 0000000000000..ac72a726850ea --- /dev/null +++ b/x-pack/plugins/task_manager/server/integration_tests/task_state_validation.test.ts @@ -0,0 +1,326 @@ +/* + * 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 { + type TestElasticsearchUtils, + type TestKibanaUtils, +} from '@kbn/core-test-helpers-kbn-server'; +import { schema } from '@kbn/config-schema'; +import { TaskStatus } from '../task'; +import { type TaskPollingLifecycleOpts } from '../polling_lifecycle'; +import { type TaskClaimingOpts } from '../queries/task_claiming'; +import { TaskManagerPlugin, type TaskManagerStartContract } from '../plugin'; +import { injectTask, setupTestServers, retry } from './lib'; + +const { TaskPollingLifecycle: TaskPollingLifecycleMock } = jest.requireMock('../polling_lifecycle'); +jest.mock('../polling_lifecycle', () => { + const actual = jest.requireActual('../polling_lifecycle'); + return { + ...actual, + TaskPollingLifecycle: jest.fn().mockImplementation((opts) => { + return new actual.TaskPollingLifecycle(opts); + }), + }; +}); + +const mockTaskTypeRunFn = jest.fn(); +const mockCreateTaskRunner = jest.fn(); +const mockTaskType = { + title: '', + description: '', + stateSchemaByVersion: { + 1: { + up: (state: Record) => ({ foo: state.foo || '' }), + schema: schema.object({ + foo: schema.string(), + }), + }, + 2: { + up: (state: Record) => ({ ...state, bar: state.bar || '' }), + schema: schema.object({ + foo: schema.string(), + bar: schema.string(), + }), + }, + 3: { + up: (state: Record) => ({ ...state, baz: state.baz || '' }), + schema: schema.object({ + foo: schema.string(), + bar: schema.string(), + baz: schema.string(), + }), + }, + }, + createTaskRunner: mockCreateTaskRunner.mockImplementation(() => ({ + run: mockTaskTypeRunFn, + })), +}; +jest.mock('../queries/task_claiming', () => { + const actual = jest.requireActual('../queries/task_claiming'); + return { + ...actual, + TaskClaiming: jest.fn().mockImplementation((opts: TaskClaimingOpts) => { + // We need to register here because once the class is instantiated, adding + // definitions won't get claimed because of "partitionIntoClaimingBatches". + opts.definitions.registerTaskDefinitions({ + fooType: mockTaskType, + }); + return new actual.TaskClaiming(opts); + }), + }; +}); + +const taskManagerStartSpy = jest.spyOn(TaskManagerPlugin.prototype, 'start'); + +describe('task state validation', () => { + describe('allow_reading_invalid_state: true', () => { + let esServer: TestElasticsearchUtils; + let kibanaServer: TestKibanaUtils; + let taskManagerPlugin: TaskManagerStartContract; + let pollingLifecycleOpts: TaskPollingLifecycleOpts; + + beforeAll(async () => { + const setupResult = await setupTestServers(); + esServer = setupResult.esServer; + kibanaServer = setupResult.kibanaServer; + + expect(taskManagerStartSpy).toHaveBeenCalledTimes(1); + taskManagerPlugin = taskManagerStartSpy.mock.results[0].value; + + expect(TaskPollingLifecycleMock).toHaveBeenCalledTimes(1); + pollingLifecycleOpts = TaskPollingLifecycleMock.mock.calls[0][0]; + }); + + afterAll(async () => { + if (kibanaServer) { + await kibanaServer.stop(); + } + if (esServer) { + await esServer.stop(); + } + }); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + afterEach(async () => { + await taskManagerPlugin.removeIfExists('foo'); + }); + + it('should drop unknown fields from the task state', async () => { + const taskRunnerPromise = new Promise((resolve) => { + mockTaskTypeRunFn.mockImplementation(() => { + setTimeout(resolve, 0); + return { state: {} }; + }); + }); + + await injectTask(kibanaServer.coreStart.elasticsearch.client.asInternalUser, { + id: 'foo', + taskType: 'fooType', + params: { foo: true }, + state: { foo: 'test', bar: 'test', baz: 'test', invalidProperty: 'invalid' }, + stateVersion: 4, + runAt: new Date(), + enabled: true, + scheduledAt: new Date(), + attempts: 0, + status: TaskStatus.Idle, + startedAt: null, + retryAt: null, + ownerId: null, + }); + + await taskRunnerPromise; + + expect(mockCreateTaskRunner).toHaveBeenCalledTimes(1); + const call = mockCreateTaskRunner.mock.calls[0][0]; + expect(call.taskInstance.state).toEqual({ + foo: 'test', + bar: 'test', + baz: 'test', + }); + }); + + it('should fail to update the task if the task runner returns an unknown property in the state', async () => { + const errorLogSpy = jest.spyOn(pollingLifecycleOpts.logger, 'error'); + const taskRunnerPromise = new Promise((resolve) => { + mockTaskTypeRunFn.mockImplementation(() => { + setTimeout(resolve, 0); + return { state: { invalidField: true, foo: 'test', bar: 'test', baz: 'test' } }; + }); + }); + + await taskManagerPlugin.schedule({ + id: 'foo', + taskType: 'fooType', + params: {}, + state: { foo: 'test', bar: 'test', baz: 'test' }, + schedule: { interval: '1d' }, + }); + + await taskRunnerPromise; + + expect(mockCreateTaskRunner).toHaveBeenCalledTimes(1); + const call = mockCreateTaskRunner.mock.calls[0][0]; + expect(call.taskInstance.state).toEqual({ + foo: 'test', + bar: 'test', + baz: 'test', + }); + expect(errorLogSpy).toHaveBeenCalledWith( + 'Task fooType "foo" failed: Error: [invalidField]: definition for this key is missing', + expect.anything() + ); + }); + + it('should migrate the task state', async () => { + const taskRunnerPromise = new Promise((resolve) => { + mockTaskTypeRunFn.mockImplementation(() => { + setTimeout(resolve, 0); + return { state: {} }; + }); + }); + + await injectTask(kibanaServer.coreStart.elasticsearch.client.asInternalUser, { + id: 'foo', + taskType: 'fooType', + params: { foo: true }, + state: {}, + runAt: new Date(), + enabled: true, + scheduledAt: new Date(), + attempts: 0, + status: TaskStatus.Idle, + startedAt: null, + retryAt: null, + ownerId: null, + }); + + await taskRunnerPromise; + + expect(mockCreateTaskRunner).toHaveBeenCalledTimes(1); + const call = mockCreateTaskRunner.mock.calls[0][0]; + expect(call.taskInstance.state).toEqual({ + foo: '', + bar: '', + baz: '', + }); + }); + + it('should debug log by default when reading an invalid task state', async () => { + const debugLogSpy = jest.spyOn(pollingLifecycleOpts.logger, 'debug'); + const taskRunnerPromise = new Promise((resolve) => { + mockTaskTypeRunFn.mockImplementation(() => { + setTimeout(resolve, 0); + return { state: {} }; + }); + }); + + await injectTask(kibanaServer.coreStart.elasticsearch.client.asInternalUser, { + id: 'foo', + taskType: 'fooType', + params: { foo: true }, + state: { foo: true, bar: 'test', baz: 'test' }, + stateVersion: 4, + runAt: new Date(), + enabled: true, + scheduledAt: new Date(), + attempts: 0, + status: TaskStatus.Idle, + startedAt: null, + retryAt: null, + ownerId: null, + }); + + await taskRunnerPromise; + + expect(mockCreateTaskRunner).toHaveBeenCalledTimes(1); + const call = mockCreateTaskRunner.mock.calls[0][0]; + expect(call.taskInstance.state).toEqual({ + foo: true, + bar: 'test', + baz: 'test', + }); + + expect(debugLogSpy).toHaveBeenCalledWith( + `[fooType][foo] Failed to validate the task's state. Allowing read operation to proceed because allow_reading_invalid_state is true. Error: [foo]: expected value of type [string] but got [boolean]` + ); + }); + }); + + describe('allow_reading_invalid_state: false', () => { + let esServer: TestElasticsearchUtils; + let kibanaServer: TestKibanaUtils; + let taskManagerPlugin: TaskManagerStartContract; + let pollingLifecycleOpts: TaskPollingLifecycleOpts; + + beforeAll(async () => { + const setupResult = await setupTestServers({ + xpack: { + task_manager: { + allow_reading_invalid_state: false, + }, + }, + }); + esServer = setupResult.esServer; + kibanaServer = setupResult.kibanaServer; + + expect(taskManagerStartSpy).toHaveBeenCalledTimes(1); + taskManagerPlugin = taskManagerStartSpy.mock.results[0].value; + + expect(TaskPollingLifecycleMock).toHaveBeenCalledTimes(1); + pollingLifecycleOpts = TaskPollingLifecycleMock.mock.calls[0][0]; + }); + + afterAll(async () => { + if (kibanaServer) { + await kibanaServer.stop(); + } + if (esServer) { + await esServer.stop(); + } + }); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + afterEach(async () => { + await taskManagerPlugin.removeIfExists('foo'); + }); + + it('should fail the task run when setting allow_reading_invalid_state:false and reading an invalid state', async () => { + const errorLogSpy = jest.spyOn(pollingLifecycleOpts.logger, 'error'); + + await injectTask(kibanaServer.coreStart.elasticsearch.client.asInternalUser, { + id: 'foo', + taskType: 'fooType', + params: { foo: true }, + state: { foo: true, bar: 'test', baz: 'test' }, + stateVersion: 4, + runAt: new Date(), + enabled: true, + scheduledAt: new Date(), + attempts: 0, + status: TaskStatus.Idle, + startedAt: null, + retryAt: null, + ownerId: null, + }); + + await retry(async () => { + expect(errorLogSpy).toHaveBeenCalledWith( + `Failed to poll for work: Error: [foo]: expected value of type [string] but got [boolean]` + ); + }); + + expect(mockCreateTaskRunner).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/x-pack/plugins/task_manager/server/lib/bulk_operation_buffer.test.ts b/x-pack/plugins/task_manager/server/lib/bulk_operation_buffer.test.ts index 1832e1235aaf6..99bb7c5b84274 100644 --- a/x-pack/plugins/task_manager/server/lib/bulk_operation_buffer.test.ts +++ b/x-pack/plugins/task_manager/server/lib/bulk_operation_buffer.test.ts @@ -44,7 +44,7 @@ describe('Bulk Operation Buffer', () => { return Promise.resolve([incrementAttempts(task1), incrementAttempts(task2)]); }); - const bufferedUpdate = createBuffer(bulkUpdate); + const bufferedUpdate = createBuffer(bulkUpdate, {}); const task1 = createTask(); const task2 = createTask(); @@ -173,7 +173,7 @@ describe('Bulk Operation Buffer', () => { } ); - const bufferedUpdate = createBuffer(bulkUpdate); + const bufferedUpdate = createBuffer(bulkUpdate, {}); const task1 = createTask(); const task2 = createTask(); @@ -195,7 +195,7 @@ describe('Bulk Operation Buffer', () => { return Promise.reject(new Error('bulkUpdate is an illusion')); }); - const bufferedUpdate = createBuffer(bulkUpdate); + const bufferedUpdate = createBuffer(bulkUpdate, {}); const task1 = createTask(); const task2 = createTask(); diff --git a/x-pack/plugins/task_manager/server/lib/bulk_operation_buffer.ts b/x-pack/plugins/task_manager/server/lib/bulk_operation_buffer.ts index 10bec75c84684..874592c4e5d01 100644 --- a/x-pack/plugins/task_manager/server/lib/bulk_operation_buffer.ts +++ b/x-pack/plugins/task_manager/server/lib/bulk_operation_buffer.ts @@ -39,7 +39,7 @@ const FLUSH = true; export function createBuffer( bulkOperation: BulkOperation, - { bufferMaxDuration = 0, bufferMaxOperations = Number.MAX_VALUE, logger }: BufferOptions = {} + { bufferMaxDuration = 0, bufferMaxOperations = Number.MAX_VALUE, logger }: BufferOptions ): Operation { const flushBuffer = new Subject(); diff --git a/x-pack/plugins/task_manager/server/lib/retryable_bulk_update.test.ts b/x-pack/plugins/task_manager/server/lib/retryable_bulk_update.test.ts index aefcae67ecdd5..1eada39cb3539 100644 --- a/x-pack/plugins/task_manager/server/lib/retryable_bulk_update.test.ts +++ b/x-pack/plugins/task_manager/server/lib/retryable_bulk_update.test.ts @@ -32,25 +32,26 @@ describe('retryableBulkUpdate()', () => { }); it('should call getTasks with taskIds', async () => { - await retryableBulkUpdate({ taskIds, getTasks, filter, map, store }); + await retryableBulkUpdate({ taskIds, getTasks, filter, map, store, validate: false }); expect(getTasks).toHaveBeenCalledWith(taskIds); }); it('should filter tasks returned from getTasks', async () => { filter.mockImplementation((task) => task.id === '2'); - await retryableBulkUpdate({ taskIds, getTasks, filter, map, store }); + await retryableBulkUpdate({ taskIds, getTasks, filter, map, store, validate: false }); expect(filter).toHaveBeenCalledTimes(3); // Map happens after filter expect(map).toHaveBeenCalledTimes(1); - expect(store.bulkUpdate).toHaveBeenCalledWith([tasks[1]]); + expect(store.bulkUpdate).toHaveBeenCalledWith([tasks[1]], { validate: false }); }); it('should map tasks returned from getTasks', async () => { map.mockImplementation((task) => ({ ...task, status: TaskStatus.Claiming })); - await retryableBulkUpdate({ taskIds, getTasks, filter, map, store }); + await retryableBulkUpdate({ taskIds, getTasks, filter, map, store, validate: false }); expect(map).toHaveBeenCalledTimes(3); expect(store.bulkUpdate).toHaveBeenCalledWith( - tasks.map((task) => ({ ...task, status: TaskStatus.Claiming })) + tasks.map((task) => ({ ...task, status: TaskStatus.Claiming })), + { validate: false } ); }); @@ -71,9 +72,9 @@ describe('retryableBulkUpdate()', () => { ]); getTasks.mockResolvedValueOnce([tasks[0]].map((task) => asOk(task))); store.bulkUpdate.mockResolvedValueOnce(tasks.map((task) => asOk(task))); - await retryableBulkUpdate({ taskIds, getTasks, filter, map, store }); + await retryableBulkUpdate({ taskIds, getTasks, filter, map, store, validate: false }); expect(store.bulkUpdate).toHaveBeenCalledTimes(2); - expect(store.bulkUpdate).toHaveBeenNthCalledWith(2, [tasks[0]]); + expect(store.bulkUpdate).toHaveBeenNthCalledWith(2, [tasks[0]], { validate: false }); }); it('should skip updating tasks that cannot be found', async () => { @@ -86,7 +87,7 @@ describe('retryableBulkUpdate()', () => { }), asOk(tasks[2]), ]); - await retryableBulkUpdate({ taskIds, getTasks, filter, map, store }); - expect(store.bulkUpdate).toHaveBeenCalledWith([tasks[0], tasks[2]]); + await retryableBulkUpdate({ taskIds, getTasks, filter, map, store, validate: false }); + expect(store.bulkUpdate).toHaveBeenCalledWith([tasks[0], tasks[2]], { validate: false }); }); }); diff --git a/x-pack/plugins/task_manager/server/lib/retryable_bulk_update.ts b/x-pack/plugins/task_manager/server/lib/retryable_bulk_update.ts index 3c832e31af4da..d50fb88d909c3 100644 --- a/x-pack/plugins/task_manager/server/lib/retryable_bulk_update.ts +++ b/x-pack/plugins/task_manager/server/lib/retryable_bulk_update.ts @@ -19,6 +19,7 @@ export interface RetryableBulkUpdateOpts { filter: (task: ConcreteTaskInstance) => boolean; map: (task: ConcreteTaskInstance) => ConcreteTaskInstance; store: TaskStore; + validate: boolean; } export async function retryableBulkUpdate({ @@ -27,6 +28,7 @@ export async function retryableBulkUpdate({ filter, map, store, + validate, }: RetryableBulkUpdateOpts): Promise { const resultMap: Record = {}; @@ -42,7 +44,7 @@ export async function retryableBulkUpdate({ }, []) .filter(filter) .map(map); - const bulkUpdateResult = await store.bulkUpdate(tasksToUpdate); + const bulkUpdateResult = await store.bulkUpdate(tasksToUpdate, { validate }); for (const result of bulkUpdateResult) { const taskId = getId(result); resultMap[taskId] = result; diff --git a/x-pack/plugins/task_manager/server/monitoring/configuration_statistics.test.ts b/x-pack/plugins/task_manager/server/monitoring/configuration_statistics.test.ts index 665b212d951c0..62303b925fb3f 100644 --- a/x-pack/plugins/task_manager/server/monitoring/configuration_statistics.test.ts +++ b/x-pack/plugins/task_manager/server/monitoring/configuration_statistics.test.ts @@ -16,6 +16,7 @@ describe('Configuration Statistics Aggregator', () => { max_workers: 10, max_attempts: 9, poll_interval: 6000000, + allow_reading_invalid_state: false, version_conflict_threshold: 80, monitored_stats_required_freshness: 6000000, request_capacity: 1000, diff --git a/x-pack/plugins/task_manager/server/monitoring/monitoring_stats_stream.test.ts b/x-pack/plugins/task_manager/server/monitoring/monitoring_stats_stream.test.ts index 23312b79cec8b..0dcd87ab150bf 100644 --- a/x-pack/plugins/task_manager/server/monitoring/monitoring_stats_stream.test.ts +++ b/x-pack/plugins/task_manager/server/monitoring/monitoring_stats_stream.test.ts @@ -20,6 +20,7 @@ describe('createMonitoringStatsStream', () => { max_workers: 10, max_attempts: 9, poll_interval: 6000000, + allow_reading_invalid_state: false, version_conflict_threshold: 80, monitored_stats_required_freshness: 6000000, request_capacity: 1000, diff --git a/x-pack/plugins/task_manager/server/plugin.test.ts b/x-pack/plugins/task_manager/server/plugin.test.ts index 637f8a2b952f4..aca39e1678a23 100644 --- a/x-pack/plugins/task_manager/server/plugin.test.ts +++ b/x-pack/plugins/task_manager/server/plugin.test.ts @@ -43,6 +43,7 @@ const pluginInitializerContextParams = { poll_interval: 3000, version_conflict_threshold: 80, request_capacity: 1000, + allow_reading_invalid_state: false, monitored_aggregated_stats_refresh_rate: 5000, monitored_stats_health_verbose_log: { enabled: false, diff --git a/x-pack/plugins/task_manager/server/plugin.ts b/x-pack/plugins/task_manager/server/plugin.ts index 64da6d6f84f95..e65574cef779a 100644 --- a/x-pack/plugins/task_manager/server/plugin.ts +++ b/x-pack/plugins/task_manager/server/plugin.ts @@ -227,6 +227,8 @@ export class TaskManagerPlugin definitions: this.definitions, taskManagerId: `kibana:${this.taskManagerId!}`, adHocTaskCounter: this.adHocTaskCounter, + allowReadingInvalidState: this.config.allow_reading_invalid_state, + logger: this.logger, }); const managedConfiguration = createManagedConfiguration({ diff --git a/x-pack/plugins/task_manager/server/polling_lifecycle.test.ts b/x-pack/plugins/task_manager/server/polling_lifecycle.test.ts index fc72c950a4034..a18a458ad6510 100644 --- a/x-pack/plugins/task_manager/server/polling_lifecycle.test.ts +++ b/x-pack/plugins/task_manager/server/polling_lifecycle.test.ts @@ -48,6 +48,7 @@ describe('TaskPollingLifecycle', () => { poll_interval: 6000000, version_conflict_threshold: 80, request_capacity: 1000, + allow_reading_invalid_state: false, monitored_aggregated_stats_refresh_rate: 5000, monitored_stats_health_verbose_log: { enabled: false, diff --git a/x-pack/plugins/task_manager/server/task.ts b/x-pack/plugins/task_manager/server/task.ts index a9f49b661a700..edde074ca4e02 100644 --- a/x-pack/plugins/task_manager/server/task.ts +++ b/x-pack/plugins/task_manager/server/task.ts @@ -5,7 +5,7 @@ * 2.0. */ -import { schema, TypeOf } from '@kbn/config-schema'; +import { schema, TypeOf, ObjectType } from '@kbn/config-schema'; import { Interval, isInterval, parseIntervalAsMillisecond } from './lib/intervals'; import { isErr, tryAsResult } from './lib/result_type'; @@ -138,6 +138,15 @@ export const taskDefinitionSchema = schema.object( min: 0, }) ), + stateSchemaByVersion: schema.maybe( + schema.recordOf( + schema.string(), + schema.object({ + schema: schema.any(), + up: schema.any(), + }) + ) + ), }, { validate({ timeout }) { @@ -158,6 +167,13 @@ export type TaskDefinition = TypeOf & { * and an optional cancel function which cancels the task. */ createTaskRunner: TaskRunCreatorFunction; + stateSchemaByVersion?: Record< + number, + { + schema: ObjectType; + up: (state: Record) => Record; + } + >; }; export enum TaskStatus { @@ -248,6 +264,7 @@ export interface TaskInstance { // this can be fixed by supporting generics in the future // eslint-disable-next-line @typescript-eslint/no-explicit-any state: Record; + stateVersion?: number; /** * The serialized traceparent string of the current APM transaction or span. diff --git a/x-pack/plugins/task_manager/server/task_running/task_runner.test.ts b/x-pack/plugins/task_manager/server/task_running/task_runner.test.ts index 5812384c66c8e..be47be3a84266 100644 --- a/x-pack/plugins/task_manager/server/task_running/task_runner.test.ts +++ b/x-pack/plugins/task_manager/server/task_running/task_runner.test.ts @@ -23,10 +23,10 @@ import moment from 'moment'; import { TaskDefinitionRegistry, TaskTypeDictionary } from '../task_type_dictionary'; import { mockLogger } from '../test_utils'; import { throwRetryableError, throwUnrecoverableError } from './errors'; -import { taskStoreMock } from '../task_store.mock'; import apm from 'elastic-apm-node'; import { executionContextServiceMock } from '@kbn/core/server/mocks'; import { usageCountersServiceMock } from '@kbn/usage-collection-plugin/server/usage_counters/usage_counters_service.mock'; +import { bufferedTaskStoreMock } from '../buffered_task_store.mock'; import { calculateDelay, TASK_MANAGER_RUN_TRANSACTION_TYPE, @@ -432,17 +432,20 @@ describe('TaskManagerRunner', () => { `[Error: type: Bad Request]` ); - expect(store.update).toHaveBeenCalledWith({ - ...mockInstance({ - id, - attempts: initialAttempts + 1, - schedule: undefined, - }), - status: TaskStatus.Idle, - startedAt: null, - retryAt: null, - ownerId: null, - }); + expect(store.update).toHaveBeenCalledWith( + { + ...mockInstance({ + id, + attempts: initialAttempts + 1, + schedule: undefined, + }), + status: TaskStatus.Idle, + startedAt: null, + retryAt: null, + ownerId: null, + }, + { validate: false } + ); }); test(`it doesnt try to increment a task's attempts when markTaskAsRunning fails for version conflict`, async () => { @@ -834,7 +837,9 @@ describe('TaskManagerRunner', () => { await runner.run(); expect(store.update).toHaveBeenCalledTimes(1); - expect(store.update).toHaveBeenCalledWith(expect.objectContaining({ runAt })); + expect(store.update).toHaveBeenCalledWith(expect.objectContaining({ runAt }), { + validate: true, + }); }); test('reschedules tasks that return a schedule', async () => { @@ -862,7 +867,9 @@ describe('TaskManagerRunner', () => { await runner.run(); expect(store.update).toHaveBeenCalledTimes(1); - expect(store.update).toHaveBeenCalledWith(expect.objectContaining({ runAt })); + expect(store.update).toHaveBeenCalledWith(expect.objectContaining({ runAt }), { + validate: true, + }); }); test(`doesn't reschedule recurring tasks that throw an unrecoverable error`, async () => { @@ -936,7 +943,9 @@ describe('TaskManagerRunner', () => { await runner.run(); expect(store.update).toHaveBeenCalledTimes(1); - expect(store.update).toHaveBeenCalledWith(expect.objectContaining({ runAt })); + expect(store.update).toHaveBeenCalledWith(expect.objectContaining({ runAt }), { + validate: true, + }); }); test('removes non-recurring tasks after they complete', async () => { @@ -1654,7 +1663,7 @@ describe('TaskManagerRunner', () => { const instance = mockInstance(opts.instance); - const store = taskStoreMock.create(); + const store = bufferedTaskStoreMock.create(); const usageCounter = usageCountersServiceMock.createSetupContract().createUsageCounter('test'); store.update.mockResolvedValue(instance); diff --git a/x-pack/plugins/task_manager/server/task_running/task_runner.ts b/x-pack/plugins/task_manager/server/task_running/task_runner.ts index 858d89f46d70a..0252c565c5d0f 100644 --- a/x-pack/plugins/task_manager/server/task_running/task_runner.ts +++ b/x-pack/plugins/task_manager/server/task_running/task_runner.ts @@ -89,7 +89,7 @@ export interface TaskRunning { } export interface Updatable { - update(doc: ConcreteTaskInstance): Promise; + update(doc: ConcreteTaskInstance, options: { validate: boolean }): Promise; remove(id: string): Promise; } @@ -395,27 +395,30 @@ export class TaskManagerRunner implements TaskRunner { } this.instance = asReadyToRun( - (await this.bufferedTaskStore.update({ - ...taskWithoutEnabled(taskInstance), - status: TaskStatus.Running, - startedAt: now, - attempts, - retryAt: - (this.instance.task.schedule - ? maxIntervalFromDate( - now, - this.instance.task.schedule.interval, - this.definition.timeout - ) - : this.getRetryDelay({ - attempts, - // Fake an error. This allows retry logic when tasks keep timing out - // and lets us set a proper "retryAt" value each time. - error: new Error('Task timeout'), - addDuration: this.definition.timeout, - })) ?? null, - // This is a safe convertion as we're setting the startAt above - })) as ConcreteTaskInstanceWithStartedAt + (await this.bufferedTaskStore.update( + { + ...taskWithoutEnabled(taskInstance), + status: TaskStatus.Running, + startedAt: now, + attempts, + retryAt: + (this.instance.task.schedule + ? maxIntervalFromDate( + now, + this.instance.task.schedule.interval, + this.definition.timeout + ) + : this.getRetryDelay({ + attempts, + // Fake an error. This allows retry logic when tasks keep timing out + // and lets us set a proper "retryAt" value each time. + error: new Error('Task timeout'), + addDuration: this.definition.timeout, + })) ?? null, + // This is a safe convertion as we're setting the startAt above + }, + { validate: false } + )) as ConcreteTaskInstanceWithStartedAt ); const timeUntilClaimExpiresAfterUpdate = @@ -476,14 +479,17 @@ export class TaskManagerRunner implements TaskRunner { private async releaseClaimAndIncrementAttempts(): Promise> { return promiseResult( - this.bufferedTaskStore.update({ - ...taskWithoutEnabled(this.instance.task), - status: TaskStatus.Idle, - attempts: this.instance.task.attempts + 1, - startedAt: null, - retryAt: null, - ownerId: null, - }) + this.bufferedTaskStore.update( + { + ...taskWithoutEnabled(this.instance.task), + status: TaskStatus.Idle, + attempts: this.instance.task.attempts + 1, + startedAt: null, + retryAt: null, + ownerId: null, + }, + { validate: false } + ) ); } @@ -580,7 +586,8 @@ export class TaskManagerRunner implements TaskRunner { ownerId: null, }, taskWithoutEnabled(this.instance.task) - ) + ), + { validate: true } ) ); } diff --git a/x-pack/plugins/task_manager/server/task_scheduling.test.ts b/x-pack/plugins/task_manager/server/task_scheduling.test.ts index 7f59f9f362554..1a87bd59ec036 100644 --- a/x-pack/plugins/task_manager/server/task_scheduling.test.ts +++ b/x-pack/plugins/task_manager/server/task_scheduling.test.ts @@ -538,7 +538,8 @@ describe('TaskScheduling', () => { status: TaskStatus.Idle, runAt: expect.any(Date), scheduledAt: expect.any(Date), - }) + }), + { validate: false } ); expect(mockTaskStore.get).toHaveBeenCalledWith(id); expect(result).toEqual({ id }); @@ -560,7 +561,8 @@ describe('TaskScheduling', () => { status: TaskStatus.Idle, runAt: expect.any(Date), scheduledAt: expect.any(Date), - }) + }), + { validate: false } ); expect(mockTaskStore.get).toHaveBeenCalledWith(id); expect(result).toEqual({ id }); diff --git a/x-pack/plugins/task_manager/server/task_scheduling.ts b/x-pack/plugins/task_manager/server/task_scheduling.ts index 86bf91048253f..85c346d52da05 100644 --- a/x-pack/plugins/task_manager/server/task_scheduling.ts +++ b/x-pack/plugins/task_manager/server/task_scheduling.ts @@ -159,6 +159,7 @@ export class TaskScheduling { getTasks: async (ids) => await this.bulkGetTasksHelper(ids), filter: (task) => !!task.enabled, map: (task) => ({ ...task, enabled: false }), + validate: false, }); } @@ -174,6 +175,7 @@ export class TaskScheduling { } return { ...task, enabled: true }; }, + validate: false, }); } @@ -208,6 +210,7 @@ export class TaskScheduling { return { ...task, schedule, runAt: new Date(newRunAtInMs) }; }, + validate: false, }); } @@ -229,12 +232,15 @@ export class TaskScheduling { public async runSoon(taskId: string): Promise { const task = await this.getNonRunningTask(taskId); try { - await this.store.update({ - ...task, - status: TaskStatus.Idle, - scheduledAt: new Date(), - runAt: new Date(), - }); + await this.store.update( + { + ...task, + status: TaskStatus.Idle, + scheduledAt: new Date(), + runAt: new Date(), + }, + { validate: false } + ); } catch (e) { if (e.statusCode === 409) { this.logger.debug( diff --git a/x-pack/plugins/task_manager/server/task_store.mock.ts b/x-pack/plugins/task_manager/server/task_store.mock.ts index 23818bc943508..861f7d60bd221 100644 --- a/x-pack/plugins/task_manager/server/task_store.mock.ts +++ b/x-pack/plugins/task_manager/server/task_store.mock.ts @@ -14,6 +14,10 @@ interface TaskStoreOptions { export const taskStoreMock = { create({ index = '', taskManagerId = '' }: TaskStoreOptions = {}) { const mocked = { + taskValidator: { + getValidatedTaskInstanceFromReading: jest.fn().mockImplementation((task) => task), + getValidatedTaskInstanceForUpdating: jest.fn().mockImplementation((task) => task), + }, convertToSavedObjectIds: jest.fn(), update: jest.fn(), remove: jest.fn(), diff --git a/x-pack/plugins/task_manager/server/task_store.test.ts b/x-pack/plugins/task_manager/server/task_store.test.ts index aa194fc2231e2..2261b8ac8f2ee 100644 --- a/x-pack/plugins/task_manager/server/task_store.test.ts +++ b/x-pack/plugins/task_manager/server/task_store.test.ts @@ -5,6 +5,7 @@ * 2.0. */ +import { schema } from '@kbn/config-schema'; import { Client } from '@elastic/elasticsearch'; import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey'; import _ from 'lodash'; @@ -25,13 +26,36 @@ import { mockLogger } from './test_utils'; import { AdHocTaskCounter } from './lib/adhoc_task_counter'; import { asErr } from './lib/result_type'; +const mockGetValidatedTaskInstanceFromReading = jest.fn(); +const mockGetValidatedTaskInstanceForUpdating = jest.fn(); +jest.mock('./task_validator', () => { + return { + TaskValidator: jest.fn().mockImplementation(() => { + return { + getValidatedTaskInstanceFromReading: mockGetValidatedTaskInstanceFromReading, + getValidatedTaskInstanceForUpdating: mockGetValidatedTaskInstanceForUpdating, + }; + }), + }; +}); + const savedObjectsClient = savedObjectsRepositoryMock.create(); const serializer = savedObjectsServiceMock.createSerializer(); const adHocTaskCounter = new AdHocTaskCounter(); const randomId = () => `id-${_.random(1, 20)}`; -beforeEach(() => jest.resetAllMocks()); +beforeEach(() => { + jest.resetAllMocks(); + jest.requireMock('./task_validator').TaskValidator.mockImplementation(() => { + return { + getValidatedTaskInstanceFromReading: mockGetValidatedTaskInstanceFromReading, + getValidatedTaskInstanceForUpdating: mockGetValidatedTaskInstanceForUpdating, + }; + }); + mockGetValidatedTaskInstanceFromReading.mockImplementation((task) => task); + mockGetValidatedTaskInstanceForUpdating.mockImplementation((task) => task); +}); const mockedDate = new Date('2019-02-12T21:01:22.479Z'); // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -49,6 +73,14 @@ const taskDefinitions = new TaskTypeDictionary(mockLogger()); taskDefinitions.registerTaskDefinitions({ report: { title: 'report', + stateSchemaByVersion: { + 1: { + schema: schema.object({ + foo: schema.string(), + }), + up: (doc) => doc, + }, + }, createTaskRunner: jest.fn(), }, dernstraight: { @@ -67,6 +99,7 @@ describe('TaskStore', () => { beforeAll(() => { store = new TaskStore({ + logger: mockLogger(), index: 'tasky', taskManagerId: '', serializer, @@ -74,6 +107,7 @@ describe('TaskStore', () => { definitions: taskDefinitions, savedObjectsRepository: savedObjectsClient, adHocTaskCounter, + allowReadingInvalidState: false, }); }); @@ -237,6 +271,7 @@ describe('TaskStore', () => { childEsClient = elasticsearchServiceMock.createClusterClient().asInternalUser; esClient.child.mockReturnValue(childEsClient as unknown as Client); store = new TaskStore({ + logger: mockLogger(), index: 'tasky', taskManagerId: '', serializer, @@ -244,6 +279,7 @@ describe('TaskStore', () => { definitions: taskDefinitions, savedObjectsRepository: savedObjectsClient, adHocTaskCounter, + allowReadingInvalidState: false, }); }); @@ -306,6 +342,7 @@ describe('TaskStore', () => { beforeAll(() => { esClient = elasticsearchServiceMock.createClusterClient().asInternalUser; store = new TaskStore({ + logger: mockLogger(), index: 'tasky', taskManagerId: '', serializer, @@ -313,6 +350,7 @@ describe('TaskStore', () => { definitions: taskDefinitions, savedObjectsRepository: savedObjectsClient, adHocTaskCounter, + allowReadingInvalidState: false, }); }); @@ -404,6 +442,7 @@ describe('TaskStore', () => { beforeAll(() => { esClient = elasticsearchServiceMock.createClusterClient().asInternalUser; store = new TaskStore({ + logger: mockLogger(), index: 'tasky', taskManagerId: '', serializer, @@ -411,6 +450,7 @@ describe('TaskStore', () => { definitions: taskDefinitions, savedObjectsRepository: savedObjectsClient, adHocTaskCounter, + allowReadingInvalidState: false, }); }); @@ -443,8 +483,16 @@ describe('TaskStore', () => { } ); - const result = await store.update(task); + const result = await store.update(task, { validate: true }); + expect(mockGetValidatedTaskInstanceForUpdating).toHaveBeenCalledTimes(1); + expect(mockGetValidatedTaskInstanceFromReading).toHaveBeenCalledTimes(1); + expect(mockGetValidatedTaskInstanceForUpdating).toHaveBeenCalledWith(task, { + validate: true, + }); + expect(mockGetValidatedTaskInstanceFromReading).toHaveBeenCalledWith(task, { + validate: true, + }); expect(savedObjectsClient.update).toHaveBeenCalledWith( 'task', task.id, @@ -478,6 +526,42 @@ describe('TaskStore', () => { }); }); + test(`doesn't go through validation process to inject stateVersion when validate:false`, async () => { + const task = { + runAt: mockedDate, + scheduledAt: mockedDate, + startedAt: null, + retryAt: null, + id: 'task:324242', + params: { hello: 'world' }, + state: { foo: 'bar' }, + taskType: 'report', + attempts: 3, + status: 'idle' as TaskStatus, + version: '123', + ownerId: null, + traceparent: 'myTraceparent', + }; + + savedObjectsClient.update.mockImplementation( + async (type: string, id: string, attributes: SavedObjectAttributes) => { + return { + id, + type, + attributes, + references: [], + version: '123', + }; + } + ); + + await store.update(task, { validate: false }); + + expect(mockGetValidatedTaskInstanceForUpdating).toHaveBeenCalledWith(task, { + validate: false, + }); + }); + test('pushes error from saved objects client to errors$', async () => { const task = { runAt: mockedDate, @@ -497,7 +581,9 @@ describe('TaskStore', () => { const firstErrorPromise = store.errors$.pipe(first()).toPromise(); savedObjectsClient.update.mockRejectedValue(new Error('Failure')); - await expect(store.update(task)).rejects.toThrowErrorMatchingInlineSnapshot(`"Failure"`); + await expect( + store.update(task, { validate: true }) + ).rejects.toThrowErrorMatchingInlineSnapshot(`"Failure"`); expect(await firstErrorPromise).toMatchInlineSnapshot(`[Error: Failure]`); }); }); @@ -507,6 +593,7 @@ describe('TaskStore', () => { beforeAll(() => { store = new TaskStore({ + logger: mockLogger(), index: 'tasky', taskManagerId: '', serializer, @@ -514,6 +601,47 @@ describe('TaskStore', () => { definitions: taskDefinitions, savedObjectsRepository: savedObjectsClient, adHocTaskCounter, + allowReadingInvalidState: false, + }); + }); + + test(`doesn't validate whenever validate:false is passed-in`, async () => { + const task = { + runAt: mockedDate, + scheduledAt: mockedDate, + startedAt: null, + retryAt: null, + id: 'task:324242', + params: { hello: 'world' }, + state: { foo: 'bar' }, + taskType: 'report', + attempts: 3, + status: 'idle' as TaskStatus, + version: '123', + ownerId: null, + traceparent: '', + }; + + savedObjectsClient.bulkUpdate.mockResolvedValue({ + saved_objects: [ + { + id: '324242', + type: 'task', + attributes: { + ...task, + state: '{"foo":"bar"}', + params: '{"hello":"world"}', + }, + references: [], + version: '123', + }, + ], + }); + + await store.bulkUpdate([task], { validate: false }); + + expect(mockGetValidatedTaskInstanceForUpdating).toHaveBeenCalledWith(task, { + validate: false, }); }); @@ -536,9 +664,9 @@ describe('TaskStore', () => { const firstErrorPromise = store.errors$.pipe(first()).toPromise(); savedObjectsClient.bulkUpdate.mockRejectedValue(new Error('Failure')); - await expect(store.bulkUpdate([task])).rejects.toThrowErrorMatchingInlineSnapshot( - `"Failure"` - ); + await expect( + store.bulkUpdate([task], { validate: true }) + ).rejects.toThrowErrorMatchingInlineSnapshot(`"Failure"`); expect(await firstErrorPromise).toMatchInlineSnapshot(`[Error: Failure]`); }); }); @@ -548,6 +676,7 @@ describe('TaskStore', () => { beforeAll(() => { store = new TaskStore({ + logger: mockLogger(), index: 'tasky', taskManagerId: '', serializer, @@ -555,6 +684,7 @@ describe('TaskStore', () => { definitions: taskDefinitions, savedObjectsRepository: savedObjectsClient, adHocTaskCounter, + allowReadingInvalidState: false, }); }); @@ -582,6 +712,7 @@ describe('TaskStore', () => { beforeAll(() => { store = new TaskStore({ + logger: mockLogger(), index: 'tasky', taskManagerId: '', serializer, @@ -589,6 +720,7 @@ describe('TaskStore', () => { definitions: taskDefinitions, savedObjectsRepository: savedObjectsClient, adHocTaskCounter, + allowReadingInvalidState: false, }); }); @@ -616,6 +748,7 @@ describe('TaskStore', () => { beforeAll(() => { store = new TaskStore({ + logger: mockLogger(), index: 'tasky', taskManagerId: '', serializer, @@ -623,6 +756,7 @@ describe('TaskStore', () => { definitions: taskDefinitions, savedObjectsRepository: savedObjectsClient, adHocTaskCounter, + allowReadingInvalidState: false, }); }); @@ -635,6 +769,7 @@ describe('TaskStore', () => { id: randomId(), params: { hello: 'world' }, state: { foo: 'bar' }, + stateVersion: 1, taskType: 'report', attempts: 3, status: 'idle' as TaskStatus, @@ -673,6 +808,7 @@ describe('TaskStore', () => { beforeAll(() => { store = new TaskStore({ + logger: mockLogger(), index: 'tasky', taskManagerId: '', serializer, @@ -680,6 +816,7 @@ describe('TaskStore', () => { definitions: taskDefinitions, savedObjectsRepository: savedObjectsClient, adHocTaskCounter, + allowReadingInvalidState: false, }); }); @@ -745,6 +882,7 @@ describe('TaskStore', () => { id: randomId(), params: { hello: 'world' }, state: { foo: 'bar' }, + stateVersion: 1, taskType: 'report', attempts: 3, status: status as TaskStatus, @@ -765,6 +903,7 @@ describe('TaskStore', () => { })); const store = new TaskStore({ + logger: mockLogger(), index: 'tasky', taskManagerId: '', serializer, @@ -772,6 +911,7 @@ describe('TaskStore', () => { definitions: taskDefinitions, savedObjectsRepository: savedObjectsClient, adHocTaskCounter, + allowReadingInvalidState: false, }); expect(await store.getLifecycle(task.id)).toEqual(status); @@ -785,6 +925,7 @@ describe('TaskStore', () => { ); const store = new TaskStore({ + logger: mockLogger(), index: 'tasky', taskManagerId: '', serializer, @@ -792,6 +933,7 @@ describe('TaskStore', () => { definitions: taskDefinitions, savedObjectsRepository: savedObjectsClient, adHocTaskCounter, + allowReadingInvalidState: false, }); expect(await store.getLifecycle(randomId())).toEqual(TaskLifecycleResult.NotFound); @@ -803,6 +945,7 @@ describe('TaskStore', () => { ); const store = new TaskStore({ + logger: mockLogger(), index: 'tasky', taskManagerId: '', serializer, @@ -810,6 +953,7 @@ describe('TaskStore', () => { definitions: taskDefinitions, savedObjectsRepository: savedObjectsClient, adHocTaskCounter, + allowReadingInvalidState: false, }); return expect(store.getLifecycle(randomId())).rejects.toThrow('Bad Request'); @@ -821,6 +965,7 @@ describe('TaskStore', () => { beforeAll(() => { store = new TaskStore({ + logger: mockLogger(), index: 'tasky', taskManagerId: '', serializer, @@ -828,6 +973,7 @@ describe('TaskStore', () => { definitions: taskDefinitions, savedObjectsRepository: savedObjectsClient, adHocTaskCounter, + allowReadingInvalidState: false, }); }); @@ -849,6 +995,7 @@ describe('TaskStore', () => { scheduledAt: '2019-02-12T21:01:22.479Z', startedAt: null, state: '{"foo":"bar"}', + stateVersion: 1, status: 'idle', taskType: 'report', traceparent: 'apmTraceparent', @@ -909,6 +1056,7 @@ describe('TaskStore', () => { scope: undefined, startedAt: null, state: { foo: 'bar' }, + stateVersion: 1, status: 'idle', taskType: 'report', user: undefined, @@ -981,4 +1129,50 @@ describe('TaskStore', () => { expect(adHocTaskCounter.count).toEqual(0); }); }); + + describe('TaskValidator', () => { + test(`should pass allowReadingInvalidState:false accordingly`, () => { + const logger = mockLogger(); + + new TaskStore({ + logger, + index: 'tasky', + taskManagerId: '', + serializer, + esClient: elasticsearchServiceMock.createClusterClient().asInternalUser, + definitions: taskDefinitions, + savedObjectsRepository: savedObjectsClient, + adHocTaskCounter, + allowReadingInvalidState: false, + }); + + expect(jest.requireMock('./task_validator').TaskValidator).toHaveBeenCalledWith({ + logger, + definitions: taskDefinitions, + allowReadingInvalidState: false, + }); + }); + + test(`should pass allowReadingInvalidState:true accordingly`, () => { + const logger = mockLogger(); + + new TaskStore({ + logger, + index: 'tasky', + taskManagerId: '', + serializer, + esClient: elasticsearchServiceMock.createClusterClient().asInternalUser, + definitions: taskDefinitions, + savedObjectsRepository: savedObjectsClient, + adHocTaskCounter, + allowReadingInvalidState: true, + }); + + expect(jest.requireMock('./task_validator').TaskValidator).toHaveBeenCalledWith({ + logger, + definitions: taskDefinitions, + allowReadingInvalidState: true, + }); + }); + }); }); diff --git a/x-pack/plugins/task_manager/server/task_store.ts b/x-pack/plugins/task_manager/server/task_store.ts index a06ee7b918a7b..d3c84e2c4e561 100644 --- a/x-pack/plugins/task_manager/server/task_store.ts +++ b/x-pack/plugins/task_manager/server/task_store.ts @@ -13,7 +13,7 @@ import { omit, defaults, get } from 'lodash'; import { SavedObjectError } from '@kbn/core-saved-objects-common'; import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey'; -import type { SavedObjectsBulkDeleteResponse } from '@kbn/core/server'; +import type { SavedObjectsBulkDeleteResponse, Logger } from '@kbn/core/server'; import { SavedObject, @@ -36,6 +36,7 @@ import { import { TaskTypeDictionary } from './task_type_dictionary'; import { AdHocTaskCounter } from './lib/adhoc_task_counter'; +import { TaskValidator } from './task_validator'; export interface StoreOpts { esClient: ElasticsearchClient; @@ -45,6 +46,8 @@ export interface StoreOpts { savedObjectsRepository: ISavedObjectsRepository; serializer: ISavedObjectsSerializer; adHocTaskCounter: AdHocTaskCounter; + allowReadingInvalidState: boolean; + logger: Logger; } export interface SearchOpts { @@ -97,6 +100,7 @@ export class TaskStore { public readonly index: string; public readonly taskManagerId: string; public readonly errors$ = new Subject(); + public readonly taskValidator: TaskValidator; private esClient: ElasticsearchClient; private esClientWithoutRetries: ElasticsearchClient; @@ -122,6 +126,11 @@ export class TaskStore { this.serializer = opts.serializer; this.savedObjectsRepository = opts.savedObjectsRepository; this.adHocTaskCounter = opts.adHocTaskCounter; + this.taskValidator = new TaskValidator({ + logger: opts.logger, + definitions: opts.definitions, + allowReadingInvalidState: opts.allowReadingInvalidState, + }); this.esClientWithoutRetries = opts.esClient.child({ // Timeouts are retried and make requests timeout after (requestTimeout * (1 + maxRetries)) // The poller doesn't need retry logic because it will try again at the next polling cycle @@ -150,9 +159,11 @@ export class TaskStore { let savedObject; try { + const validatedTaskInstance = + this.taskValidator.getValidatedTaskInstanceForUpdating(taskInstance); savedObject = await this.savedObjectsRepository.create( 'task', - taskInstanceToAttributes(taskInstance), + taskInstanceToAttributes(validatedTaskInstance), { id: taskInstance.id, refresh: false } ); if (get(taskInstance, 'schedule.interval', null) == null) { @@ -163,7 +174,8 @@ export class TaskStore { throw e; } - return savedObjectToConcreteTaskInstance(savedObject); + const result = savedObjectToConcreteTaskInstance(savedObject); + return this.taskValidator.getValidatedTaskInstanceFromReading(result); } /** @@ -174,9 +186,11 @@ export class TaskStore { public async bulkSchedule(taskInstances: TaskInstance[]): Promise { const objects = taskInstances.map((taskInstance) => { this.definitions.ensureHas(taskInstance.taskType); + const validatedTaskInstance = + this.taskValidator.getValidatedTaskInstanceForUpdating(taskInstance); return { type: 'task', - attributes: taskInstanceToAttributes(taskInstance), + attributes: taskInstanceToAttributes(validatedTaskInstance), id: taskInstance.id, }; }); @@ -197,7 +211,10 @@ export class TaskStore { throw e; } - return savedObjects.saved_objects.map((so) => savedObjectToConcreteTaskInstance(so)); + return savedObjects.saved_objects.map((so) => { + const taskInstance = savedObjectToConcreteTaskInstance(so); + return this.taskValidator.getValidatedTaskInstanceFromReading(taskInstance); + }); } /** @@ -222,8 +239,14 @@ export class TaskStore { * @param {TaskDoc} doc * @returns {Promise} */ - public async update(doc: ConcreteTaskInstance): Promise { - const attributes = taskInstanceToAttributes(doc); + public async update( + doc: ConcreteTaskInstance, + options: { validate: boolean } + ): Promise { + const taskInstance = this.taskValidator.getValidatedTaskInstanceForUpdating(doc, { + validate: options.validate, + }); + const attributes = taskInstanceToAttributes(taskInstance); let updatedSavedObject; try { @@ -241,13 +264,16 @@ export class TaskStore { throw e; } - return savedObjectToConcreteTaskInstance( + const result = savedObjectToConcreteTaskInstance( // The SavedObjects update api forces a Partial on the `attributes` on the response, // but actually returns the whole object that is passed to it, so as we know we're // passing in the whole object, this is safe to do. // This is far from ideal, but unless we change the SavedObjectsClient this is the best we can do { ...updatedSavedObject, attributes: defaults(updatedSavedObject.attributes, attributes) } ); + return this.taskValidator.getValidatedTaskInstanceFromReading(result, { + validate: options.validate, + }); } /** @@ -257,9 +283,15 @@ export class TaskStore { * @param {Array} docs * @returns {Promise>} */ - public async bulkUpdate(docs: ConcreteTaskInstance[]): Promise { + public async bulkUpdate( + docs: ConcreteTaskInstance[], + options: { validate: boolean } + ): Promise { const attributesByDocId = docs.reduce((attrsById, doc) => { - attrsById.set(doc.id, taskInstanceToAttributes(doc)); + const taskInstance = this.taskValidator.getValidatedTaskInstanceForUpdating(doc, { + validate: options.validate, + }); + attrsById.set(doc.id, taskInstanceToAttributes(taskInstance)); return attrsById; }, new Map()); @@ -283,21 +315,25 @@ export class TaskStore { } return updatedSavedObjects.map((updatedSavedObject) => { - return updatedSavedObject.error !== undefined - ? asErr({ - type: 'task', - id: updatedSavedObject.id, - error: updatedSavedObject.error, - }) - : asOk( - savedObjectToConcreteTaskInstance({ - ...updatedSavedObject, - attributes: defaults( - updatedSavedObject.attributes, - attributesByDocId.get(updatedSavedObject.id)! - ), - }) - ); + if (updatedSavedObject.error !== undefined) { + return asErr({ + type: 'task', + id: updatedSavedObject.id, + error: updatedSavedObject.error, + }); + } + + const taskInstance = savedObjectToConcreteTaskInstance({ + ...updatedSavedObject, + attributes: defaults( + updatedSavedObject.attributes, + attributesByDocId.get(updatedSavedObject.id)! + ), + }); + const result = this.taskValidator.getValidatedTaskInstanceFromReading(taskInstance, { + validate: options.validate, + }); + return asOk(result); }); } @@ -346,7 +382,8 @@ export class TaskStore { this.errors$.next(e); throw e; } - return savedObjectToConcreteTaskInstance(result); + const taskInstance = savedObjectToConcreteTaskInstance(result); + return this.taskValidator.getValidatedTaskInstanceFromReading(taskInstance); } /** @@ -369,7 +406,10 @@ export class TaskStore { if (task.error) { return asErr({ id: task.id, type: task.type, error: task.error }); } - return asOk(savedObjectToConcreteTaskInstance(task)); + const taskInstance = savedObjectToConcreteTaskInstance(task); + const validatedTaskInstance = + this.taskValidator.getValidatedTaskInstanceFromReading(taskInstance); + return asOk(validatedTaskInstance); }); } @@ -413,7 +453,9 @@ export class TaskStore { // @ts-expect-error @elastic/elasticsearch _source is optional .map((doc) => this.serializer.rawToSavedObject(doc)) .map((doc) => omit(doc, 'namespace') as SavedObject) - .map(savedObjectToConcreteTaskInstance), + .map((doc) => savedObjectToConcreteTaskInstance(doc)) + .map((doc) => this.taskValidator.getValidatedTaskInstanceFromReading(doc)) + .filter((doc): doc is ConcreteTaskInstance => !!doc), }; } catch (e) { this.errors$.next(e); diff --git a/x-pack/plugins/task_manager/server/task_type_dictionary.ts b/x-pack/plugins/task_manager/server/task_type_dictionary.ts index f2b6d5b9153b1..30a4ee169f4e6 100644 --- a/x-pack/plugins/task_manager/server/task_type_dictionary.ts +++ b/x-pack/plugins/task_manager/server/task_type_dictionary.ts @@ -5,6 +5,7 @@ * 2.0. */ +import { ObjectType } from '@kbn/config-schema'; import { Logger } from '@kbn/core/server'; import { TaskDefinition, taskDefinitionSchema, TaskRunCreatorFunction } from './task'; import { CONCURRENCY_ALLOW_LIST_BY_TASK_TYPE } from './constants'; @@ -65,6 +66,13 @@ export interface TaskRegisterDefinition { * The default value, if not given, is 0. */ maxConcurrency?: number; + stateSchemaByVersion?: Record< + number, + { + schema: ObjectType; + up: (state: Record) => Record; + } + >; } /** diff --git a/x-pack/plugins/task_manager/server/task_validator.test.ts b/x-pack/plugins/task_manager/server/task_validator.test.ts new file mode 100644 index 0000000000000..52822adf6f49f --- /dev/null +++ b/x-pack/plugins/task_manager/server/task_validator.test.ts @@ -0,0 +1,397 @@ +/* + * 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 { schema } from '@kbn/config-schema'; +import { taskManagerMock } from './mocks'; +import { mockLogger } from './test_utils'; +import { TaskValidator } from './task_validator'; +import { TaskTypeDictionary } from './task_type_dictionary'; + +const fooTaskDefinition = { + title: 'Foo', + description: '', + createTaskRunner() { + return { + async run() { + return { + state: {}, + }; + }, + }; + }, +}; + +describe('TaskValidator', () => { + describe('getValidatedTaskInstanceFromReading()', () => { + it(`should return the task as-is whenever the task definition isn't defined`, () => { + const definitions = new TaskTypeDictionary(mockLogger()); + const taskValidator = new TaskValidator({ + logger: mockLogger(), + definitions, + allowReadingInvalidState: false, + }); + const task = taskManagerMock.createTask(); + const result = taskValidator.getValidatedTaskInstanceFromReading(task); + expect(result).toEqual(task); + }); + + it(`should return the task as-is whenever the validate:false option is passed-in`, () => { + const definitions = new TaskTypeDictionary(mockLogger()); + definitions.registerTaskDefinitions({ + foo: { + ...fooTaskDefinition, + stateSchemaByVersion: { + 1: { + up: (state) => state, + schema: schema.object({ + foo: schema.string(), + }), + }, + }, + }, + }); + const taskValidator = new TaskValidator({ + logger: mockLogger(), + definitions, + allowReadingInvalidState: false, + }); + const task = taskManagerMock.createTask(); + const result = taskValidator.getValidatedTaskInstanceFromReading(task, { validate: false }); + expect(result).toEqual(task); + }); + + // TODO: Remove skip once all task types have defined their state schema. + // https://github.com/elastic/kibana/issues/159347 + it.skip(`should fail to validate the state schema when the task type doesn't have stateSchemaByVersion defined`, () => { + const definitions = new TaskTypeDictionary(mockLogger()); + definitions.registerTaskDefinitions({ + foo: fooTaskDefinition, + }); + const taskValidator = new TaskValidator({ + logger: mockLogger(), + definitions, + allowReadingInvalidState: false, + }); + const task = taskManagerMock.createTask({ state: { foo: 'bar' } }); + expect(() => + taskValidator.getValidatedTaskInstanceFromReading(task) + ).toThrowErrorMatchingInlineSnapshot( + `"[TaskValidator] stateSchemaByVersion not defined for task type: foo"` + ); + }); + + it(`should validate the state schema`, () => { + const definitions = new TaskTypeDictionary(mockLogger()); + definitions.registerTaskDefinitions({ + foo: { + ...fooTaskDefinition, + stateSchemaByVersion: { + 1: { + up: (state) => state, + schema: schema.object({ + foo: schema.string(), + }), + }, + }, + }, + }); + const taskValidator = new TaskValidator({ + logger: mockLogger(), + definitions, + allowReadingInvalidState: false, + }); + const task = taskManagerMock.createTask({ stateVersion: 1, state: { foo: 'bar' } }); + const result = taskValidator.getValidatedTaskInstanceFromReading(task); + expect(result).toEqual(task); + }); + + it(`should fail validation when the state schema doesn't match the state data`, () => { + const definitions = new TaskTypeDictionary(mockLogger()); + definitions.registerTaskDefinitions({ + foo: { + ...fooTaskDefinition, + stateSchemaByVersion: { + 1: { + up: (state) => state, + schema: schema.object({ + foo: schema.string(), + }), + }, + }, + }, + }); + const taskValidator = new TaskValidator({ + logger: mockLogger(), + definitions, + allowReadingInvalidState: false, + }); + const task = taskManagerMock.createTask({ stateVersion: 1, state: { foo: true } }); + expect(() => + taskValidator.getValidatedTaskInstanceFromReading(task) + ).toThrowErrorMatchingInlineSnapshot( + `"[foo]: expected value of type [string] but got [boolean]"` + ); + }); + + it(`should return original state when the state is invalid and allowReadingInvalidState is true`, () => { + const definitions = new TaskTypeDictionary(mockLogger()); + definitions.registerTaskDefinitions({ + foo: { + ...fooTaskDefinition, + stateSchemaByVersion: { + 1: { + up: (state) => state, + schema: schema.object({ + foo: schema.string(), + }), + }, + }, + }, + }); + const taskValidator = new TaskValidator({ + logger: mockLogger(), + definitions, + allowReadingInvalidState: true, + }); + const task = taskManagerMock.createTask({ stateVersion: 1, state: { foo: true } }); + const result = taskValidator.getValidatedTaskInstanceFromReading(task); + expect(result.state).toEqual({ foo: true }); + }); + + it(`should remove unknown fields`, () => { + const definitions = new TaskTypeDictionary(mockLogger()); + definitions.registerTaskDefinitions({ + foo: { + ...fooTaskDefinition, + stateSchemaByVersion: { + 1: { + up: (state) => state, + schema: schema.object({ + foo: schema.string(), + }), + }, + }, + }, + }); + const taskValidator = new TaskValidator({ + logger: mockLogger(), + definitions, + allowReadingInvalidState: false, + }); + const task = taskManagerMock.createTask({ + stateVersion: 1, + state: { foo: 'foo', bar: 'bar' }, + }); + const result = taskValidator.getValidatedTaskInstanceFromReading(task); + expect(result.state).toEqual({ foo: 'foo' }); + }); + + it(`should migrate state when reading from a document without stateVersion`, () => { + const definitions = new TaskTypeDictionary(mockLogger()); + definitions.registerTaskDefinitions({ + foo: { + ...fooTaskDefinition, + stateSchemaByVersion: { + 1: { + up: (state) => ({ ...state, baz: 'baz' }), + schema: schema.object({ + foo: schema.string(), + baz: schema.string(), + }), + }, + }, + }, + }); + const taskValidator = new TaskValidator({ + logger: mockLogger(), + definitions, + allowReadingInvalidState: false, + }); + const task = taskManagerMock.createTask({ stateVersion: undefined, state: { foo: 'foo' } }); + const result = taskValidator.getValidatedTaskInstanceFromReading(task); + expect(result.state).toEqual({ foo: 'foo', baz: 'baz' }); + }); + + it(`should migrate state when reading from an older version`, () => { + const definitions = new TaskTypeDictionary(mockLogger()); + definitions.registerTaskDefinitions({ + foo: { + ...fooTaskDefinition, + stateSchemaByVersion: { + 1: { + up: (state) => state, + schema: schema.object({ + foo: schema.string(), + }), + }, + 2: { + up: (state) => ({ ...state, baz: 'baz' }), + schema: schema.object({ + foo: schema.string(), + baz: schema.string(), + }), + }, + }, + }, + }); + const taskValidator = new TaskValidator({ + logger: mockLogger(), + definitions, + allowReadingInvalidState: false, + }); + const task = taskManagerMock.createTask({ stateVersion: 1, state: { foo: 'foo' } }); + const result = taskValidator.getValidatedTaskInstanceFromReading(task); + expect(result.state).toEqual({ foo: 'foo', baz: 'baz' }); + }); + + it(`should throw during the migration phase if a schema version is missing`, () => { + const definitions = new TaskTypeDictionary(mockLogger()); + definitions.registerTaskDefinitions({ + foo: { + ...fooTaskDefinition, + stateSchemaByVersion: { + 1: { + up: (state) => state, + schema: schema.object({ + foo: schema.string(), + }), + }, + 3: { + up: (state) => ({ ...state, baz: 'baz' }), + schema: schema.object({ + foo: schema.string(), + baz: schema.string(), + }), + }, + }, + }, + }); + const taskValidator = new TaskValidator({ + logger: mockLogger(), + definitions, + allowReadingInvalidState: false, + }); + const task = taskManagerMock.createTask({ stateVersion: 1, state: { foo: 'foo' } }); + expect(() => + taskValidator.getValidatedTaskInstanceFromReading(task) + ).toThrowErrorMatchingInlineSnapshot( + `"[TaskValidator] state schema for foo missing version: 2"` + ); + }); + }); + + describe('getValidatedTaskInstanceForUpdating()', () => { + it(`should return the task as-is whenever the task definition isn't defined`, () => { + const definitions = new TaskTypeDictionary(mockLogger()); + const taskValidator = new TaskValidator({ + logger: mockLogger(), + definitions, + allowReadingInvalidState: false, + }); + const task = taskManagerMock.createTask(); + const result = taskValidator.getValidatedTaskInstanceForUpdating(task); + expect(result).toEqual(task); + }); + + it(`should return the task as-is whenever the validate:false option is passed-in`, () => { + const definitions = new TaskTypeDictionary(mockLogger()); + definitions.registerTaskDefinitions({ + foo: { + ...fooTaskDefinition, + stateSchemaByVersion: { + 1: { + up: (state) => state, + schema: schema.object({ + foo: schema.string(), + }), + }, + }, + }, + }); + const taskValidator = new TaskValidator({ + logger: mockLogger(), + definitions, + allowReadingInvalidState: false, + }); + const task = taskManagerMock.createTask(); + const result = taskValidator.getValidatedTaskInstanceForUpdating(task, { validate: false }); + expect(result).toEqual(task); + }); + + // TODO: Remove skip once all task types have defined their state schema. + // https://github.com/elastic/kibana/issues/159347 + it.skip(`should fail to validate the state schema when the task type doesn't have stateSchemaByVersion defined`, () => { + const definitions = new TaskTypeDictionary(mockLogger()); + definitions.registerTaskDefinitions({ + foo: fooTaskDefinition, + }); + const taskValidator = new TaskValidator({ + logger: mockLogger(), + definitions, + allowReadingInvalidState: false, + }); + const task = taskManagerMock.createTask({ state: { foo: 'bar' } }); + expect(() => + taskValidator.getValidatedTaskInstanceForUpdating(task) + ).toThrowErrorMatchingInlineSnapshot( + `"[TaskValidator] stateSchemaByVersion not defined for task type: foo"` + ); + }); + + it(`should validate the state schema`, () => { + const definitions = new TaskTypeDictionary(mockLogger()); + definitions.registerTaskDefinitions({ + foo: { + ...fooTaskDefinition, + stateSchemaByVersion: { + 1: { + up: (state) => state, + schema: schema.object({ + foo: schema.string(), + }), + }, + }, + }, + }); + const taskValidator = new TaskValidator({ + logger: mockLogger(), + definitions, + allowReadingInvalidState: false, + }); + const task = taskManagerMock.createTask({ state: { foo: 'bar' } }); + const { stateVersion, ...result } = taskValidator.getValidatedTaskInstanceForUpdating(task); + expect(result).toEqual(task); + expect(stateVersion).toEqual(1); + }); + + it(`should fail to validate the state schema when unknown fields are present`, () => { + const definitions = new TaskTypeDictionary(mockLogger()); + definitions.registerTaskDefinitions({ + foo: { + ...fooTaskDefinition, + stateSchemaByVersion: { + 1: { + up: (state) => state, + schema: schema.object({ + foo: schema.string(), + }), + }, + }, + }, + }); + const taskValidator = new TaskValidator({ + logger: mockLogger(), + definitions, + allowReadingInvalidState: false, + }); + const task = taskManagerMock.createTask({ state: { foo: 'foo', bar: 'bar' } }); + expect(() => + taskValidator.getValidatedTaskInstanceForUpdating(task) + ).toThrowErrorMatchingInlineSnapshot(`"[bar]: definition for this key is missing"`); + }); + }); +}); diff --git a/x-pack/plugins/task_manager/server/task_validator.ts b/x-pack/plugins/task_manager/server/task_validator.ts new file mode 100644 index 0000000000000..61d9a903dd5b4 --- /dev/null +++ b/x-pack/plugins/task_manager/server/task_validator.ts @@ -0,0 +1,205 @@ +/* + * 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 { max, memoize } from 'lodash'; +import type { Logger } from '@kbn/core/server'; +import type { ObjectType } from '@kbn/config-schema'; +import { TaskTypeDictionary } from './task_type_dictionary'; +import type { TaskInstance, ConcreteTaskInstance, TaskDefinition } from './task'; + +interface TaskValidatorOpts { + allowReadingInvalidState: boolean; + definitions: TaskTypeDictionary; + logger: Logger; +} + +type LatestStateSchema = + | undefined + | { + schema: ObjectType; + version: number; + up: (state: Record) => Record; + }; + +export class TaskValidator { + private readonly logger: Logger; + private readonly definitions: TaskTypeDictionary; + private readonly allowReadingInvalidState: boolean; + private readonly cachedGetLatestStateSchema: (taskTypeDef: TaskDefinition) => LatestStateSchema; + private readonly cachedExtendSchema: typeof extendSchema; + + constructor({ definitions, allowReadingInvalidState, logger }: TaskValidatorOpts) { + this.logger = logger; + this.definitions = definitions; + this.allowReadingInvalidState = allowReadingInvalidState; + this.cachedGetLatestStateSchema = memoize( + getLatestStateSchema, + (taskTypeDef) => taskTypeDef.type + ); + this.cachedExtendSchema = memoize( + extendSchema, + // We need to cache two outcomes per task type (unknowns: ignore and unknowns: forbid) + (options) => `${options.taskType}|unknowns:${options.unknowns}` + ); + } + + public getValidatedTaskInstanceFromReading( + task: T, + options: { validate: boolean } = { validate: true } + ): T { + if (!options.validate) { + return task; + } + + // In the scenario the task is unused / deprecated and Kibana needs to manipulate the task, + // we'll do a pass-through for those + if (!this.definitions.has(task.taskType)) { + return task; + } + + const taskTypeDef = this.definitions.get(task.taskType); + const latestStateSchema = this.cachedGetLatestStateSchema(taskTypeDef); + + // TODO: Remove once all task types have defined their state schema. + // https://github.com/elastic/kibana/issues/159347 + // Otherwise, failures on read / write would occur. (don't forget to unskip test) + if (!latestStateSchema) { + return task; + } + + let state = task.state; + try { + state = this.getValidatedStateSchema( + this.migrateTaskState(task.state, task.stateVersion, taskTypeDef, latestStateSchema), + task.taskType, + latestStateSchema, + 'ignore' + ); + } catch (e) { + if (!this.allowReadingInvalidState) { + throw e; + } + this.logger.debug( + `[${task.taskType}][${task.id}] Failed to validate the task's state. Allowing read operation to proceed because allow_reading_invalid_state is true. Error: ${e.message}` + ); + } + + return { + ...task, + state, + }; + } + + public getValidatedTaskInstanceForUpdating( + task: T, + options: { validate: boolean } = { validate: true } + ): T { + if (!options.validate) { + return task; + } + + // In the scenario the task is unused / deprecated and Kibana needs to manipulate the task, + // we'll do a pass-through for those + if (!this.definitions.has(task.taskType)) { + return task; + } + + const taskTypeDef = this.definitions.get(task.taskType); + const latestStateSchema = this.cachedGetLatestStateSchema(taskTypeDef); + + // TODO: Remove once all task types have defined their state schema. + // https://github.com/elastic/kibana/issues/159347 + // Otherwise, failures on read / write would occur. (don't forget to unskip test) + if (!latestStateSchema) { + return task; + } + + // We are doing a write operation which must validate against the latest state schema + return { + ...task, + state: this.getValidatedStateSchema(task.state, task.taskType, latestStateSchema, 'forbid'), + stateVersion: latestStateSchema?.version, + }; + } + + private migrateTaskState( + state: ConcreteTaskInstance['state'], + currentVersion: number | undefined, + taskTypeDef: TaskDefinition, + latestStateSchema: LatestStateSchema + ) { + if (!latestStateSchema || (currentVersion && currentVersion >= latestStateSchema.version)) { + return state; + } + + let migratedState = state; + for (let i = currentVersion || 1; i <= latestStateSchema.version; i++) { + if (!taskTypeDef.stateSchemaByVersion || !taskTypeDef.stateSchemaByVersion[`${i}`]) { + throw new Error( + `[TaskValidator] state schema for ${taskTypeDef.type} missing version: ${i}` + ); + } + migratedState = taskTypeDef.stateSchemaByVersion[i].up(migratedState); + try { + taskTypeDef.stateSchemaByVersion[i].schema.validate(migratedState); + } catch (e) { + throw new Error( + `[TaskValidator] failed to migrate to version ${i} because the data returned from the up migration doesn't match the schema: ${e.message}` + ); + } + } + + return migratedState; + } + + private getValidatedStateSchema( + state: ConcreteTaskInstance['state'], + taskType: string, + latestStateSchema: LatestStateSchema, + unknowns: 'forbid' | 'ignore' + ): ConcreteTaskInstance['state'] { + if (!latestStateSchema) { + throw new Error( + `[TaskValidator] stateSchemaByVersion not defined for task type: ${taskType}` + ); + } + + return this.cachedExtendSchema({ unknowns, taskType, latestStateSchema }).validate(state); + } +} + +function extendSchema(options: { + latestStateSchema: LatestStateSchema; + unknowns: 'forbid' | 'ignore'; + taskType: string; +}) { + if (!options.latestStateSchema) { + throw new Error( + `[TaskValidator] stateSchemaByVersion not defined for task type: ${options.taskType}` + ); + } + return options.latestStateSchema.schema.extendsDeep({ unknowns: options.unknowns }); +} + +function getLatestStateSchema(taskTypeDef: TaskDefinition): LatestStateSchema { + if (!taskTypeDef.stateSchemaByVersion) { + return; + } + + const versions = Object.keys(taskTypeDef.stateSchemaByVersion).map((v) => parseInt(v, 10)); + const latest = max(versions); + + if (latest === undefined) { + return; + } + + return { + version: latest, + schema: taskTypeDef.stateSchemaByVersion[latest].schema, + up: taskTypeDef.stateSchemaByVersion[latest].up, + }; +} diff --git a/x-pack/plugins/task_manager/tsconfig.json b/x-pack/plugins/task_manager/tsconfig.json index 35e024db1f87d..20fdb90611518 100644 --- a/x-pack/plugins/task_manager/tsconfig.json +++ b/x-pack/plugins/task_manager/tsconfig.json @@ -19,7 +19,8 @@ "@kbn/es-types", "@kbn/apm-utils", "@kbn/core-saved-objects-common", - "@kbn/core-saved-objects-utils-server" + "@kbn/core-saved-objects-utils-server", + "@kbn/core-test-helpers-kbn-server" ], "exclude": [ "target/**/*", From bb3aa54c862f3e5c1c9384bf71be834441595a53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Efe=20G=C3=BCrkan=20YALAMAN?= Date: Fri, 23 Jun 2023 16:46:24 +0200 Subject: [PATCH 04/54] [Enterprise Search] Update last step of configuration (#160408) ## Summary Update last step of the configuration to expose one time sync ![Screenshot 2023-06-23 at 15 26 50](https://github.com/elastic/kibana/assets/1410658/eb286aaa-f225-4fdd-9f13-2ff0bf488cab) ![Screenshot 2023-06-23 at 15 26 53](https://github.com/elastic/kibana/assets/1410658/af5d0166-16ce-469b-abc8-776f82e8427b) ### Checklist Delete any items that are not applicable to this PR. - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [x] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) --- .../search_index/connector/connector_configuration.tsx | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/enterprise_search/public/applications/enterprise_search_content/components/search_index/connector/connector_configuration.tsx b/x-pack/plugins/enterprise_search/public/applications/enterprise_search_content/components/search_index/connector/connector_configuration.tsx index f6b8f97d03073..81bd97840b190 100644 --- a/x-pack/plugins/enterprise_search/public/applications/enterprise_search_content/components/search_index/connector/connector_configuration.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/enterprise_search_content/components/search_index/connector/connector_configuration.tsx @@ -36,6 +36,7 @@ import { GenerateConnectorApiKeyApiLogic } from '../../../api/connector/generate import { SEARCH_INDEX_TAB_PATH } from '../../../routes'; import { isConnectorIndex } from '../../../utils/indices'; +import { SyncsContextMenu } from '../components/header_actions/syncs_context_menu'; import { IndexNameLogic } from '../index_name_logic'; import { IndexViewLogic } from '../index_view_logic'; @@ -257,7 +258,7 @@ service_type: "${index.connector.service_type || 'changeme'}" 'xpack.enterpriseSearch.content.indices.configurationConnector.scheduleSync.description', { defaultMessage: - 'Once configured, set a recurring sync schedule to keep your documents in sync over time. You can also simply trigger a one-time sync.', + 'Finalize your connector by triggering a one-time sync, or setting a recurring sync to keep your data source in sync over time', } )} @@ -281,6 +282,9 @@ service_type: "${index.connector.service_type || 'changeme'}" )} + + + @@ -289,7 +293,7 @@ service_type: "${index.connector.service_type || 'changeme'}" title: i18n.translate( 'xpack.enterpriseSearch.content.indices.configurationConnector.steps.schedule.title', { - defaultMessage: 'Set a recurring sync schedule', + defaultMessage: 'Advanced configuration', } ), titleSize: 'xs', From a1d02824f1680e5a9da3f9d6520716435270f808 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20Kopyci=C5=84ski?= Date: Fri, 23 Jun 2023 17:02:06 +0200 Subject: [PATCH 05/54] [shared-ux-router] Add Router and Routes components (#159834) ## Summary Why? To simplify the process of migration to react-router@6. https://github.com/remix-run/react-router/discussions/8753 What problems exactly it solves? - In my previous PR I added `CompatRouter` https://github.com/elastic/kibana/pull/159173, which caused changes in ~50 files and pinged 15 Teams. And this is just meant to be a temporary change, so when we're done with the migration I would have to revert these changes and engage everyone to review the PR again. And it is just a single step in the migration strategy. So to make our lives easier I think it would be better to have a common place where we do import our router components because it will allow us to surface some extra logic in single place instead of going through the whole source code again. - `react-router@6` doesn't support a custom `Route` component, so that means our custom `Route` component that we're using almost everywhere today, will need to be replaced by a different solution. I have decided to add `Routes` component, which will be responsible for rendering the proper component (`react-router@6` renamed `Switch` to `Routes`, so I have named this component to align with the dictionary of the new router) and also is going to add the logic that today is done in `Route` (moving logic to `Routes` will be done in the follow-up PR, here I just wanted to focus on using the common router components to make the review process easier) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> --- .eslintrc.js | 2 +- .../public/containers/app/index.tsx | 21 +- .../public/examples/index.tsx | 93 ++- .../content_management_examples/tsconfig.json | 1 + examples/embeddable_explorer/public/app.tsx | 15 +- .../public/components/app.tsx | 42 +- examples/locator_examples/public/app.tsx | 8 +- .../public/app.tsx | 30 +- .../public/containers/app/index.tsx | 21 +- .../public/components/app.tsx | 109 ++-- .../screenshot_mode_example/tsconfig.json | 1 + .../search_examples/public/application.tsx | 9 +- .../public/todo/todo.tsx | 109 ++-- .../public/with_data_services/app.tsx | 65 +- .../src/ui/app_router.tsx | 111 ++-- .../tsconfig.json | 1 + .../src/ui/project/header.tsx | 51 +- .../tsconfig.json | 1 + .../template/public/components/app.tsx.ejs | 112 ++-- .../src/router_provider.tsx | 8 +- .../tsconfig.json | 3 +- ...uter.test.tsx.snap => route.test.tsx.snap} | 0 packages/shared-ux/router/impl/index.ts | 4 +- .../impl/{router.test.tsx => route.test.tsx} | 2 +- .../shared-ux/router/impl/route.tsx | 41 +- packages/shared-ux/router/impl/router.tsx | 102 +-- packages/shared-ux/router/impl/routes.tsx | 56 ++ .../mount_management_section.tsx | 51 +- .../public/dashboard_app/dashboard_router.tsx | 30 +- .../mount_management_section.tsx | 37 +- src/plugins/dev_tools/public/application.tsx | 57 +- .../public/application/discover_router.tsx | 27 +- .../main/discover_main_app.test.tsx | 2 +- .../public/mount_management_section.tsx | 2 +- .../public/application/components/home_app.js | 9 +- src/plugins/home/tsconfig.json | 1 + .../kibana_overview/public/components/app.tsx | 8 +- src/plugins/kibana_react/public/index.ts | 2 - .../kibana_react/public/router/index.ts | 9 - .../management_app/management_router.tsx | 8 +- .../management_section/mount_section.tsx | 7 +- .../public/visualize_app/app.tsx | 8 +- .../public/visualize_app/index.tsx | 2 +- .../plugins/core_history_block/public/app.tsx | 8 +- .../plugins/core_history_block/tsconfig.json | 3 +- .../core_plugin_a/public/application.tsx | 4 +- .../core_plugin_b/public/application.tsx | 4 +- .../public/application.tsx | 4 +- .../management_test_plugin/public/plugin.tsx | 8 +- .../reporting_example/public/application.tsx | 7 +- .../application/maintenance_windows.tsx | 7 +- ...ervice_transactions_overview_link.test.tsx | 11 +- .../apm/transaction_overview_link.test.tsx | 7 +- .../shared/search_bar/search_bar.test.tsx | 15 +- .../unified_search_bar.test.tsx | 8 +- .../url_params_context.test.tsx | 2 +- x-pack/plugins/canvas/public/routes/index.tsx | 8 +- .../public/routes/workpad/workpad_route.tsx | 8 +- x-pack/plugins/cases/public/application.tsx | 2 +- .../cases/public/components/app/routes.tsx | 8 +- .../visualizations/actions/action_wrapper.tsx | 2 +- .../public/application/router.test.tsx | 2 +- .../public/application/router.tsx | 8 +- .../public/test/test_provider.tsx | 7 +- .../public/application/csp_router.test.tsx | 2 +- .../public/application/csp_router.tsx | 12 +- .../pages/configurations/configurations.tsx | 8 +- .../latest_findings_container.test.tsx | 2 +- .../findings_by_resource_container.tsx | 7 +- .../public/pages/findings/findings.tsx | 8 +- .../pages/vulnerabilities/vulnerabilities.tsx | 7 +- .../public/test/test_provider.tsx | 9 +- .../public/app/app.tsx | 8 +- .../public/app/sections/home/home.js | 7 +- .../analytics_collection_view.tsx | 7 +- .../public/applications/analytics/index.tsx | 7 +- .../analytics/analytics_router.test.tsx | 6 +- .../components/analytics/analytics_router.tsx | 8 +- .../components/crawler/crawler_router.tsx | 7 +- .../curations/curations_router.test.tsx | 5 +- .../components/curations/curations_router.tsx | 7 +- .../components/engine/engine_router.test.tsx | 6 +- .../components/engine/engine_router.tsx | 8 +- .../components/schema/schema_router.test.tsx | 5 +- .../components/schema/schema_router.tsx | 7 +- .../public/applications/app_search/index.tsx | 16 +- .../search_application_router.tsx | 8 +- .../search_application_view.tsx | 8 +- .../search_applications_router.tsx | 7 +- .../applications/applications/index.tsx | 8 +- .../applications/elasticsearch/index.tsx | 7 +- .../components/new_index/new_index_router.tsx | 7 +- .../search_index/search_index_router.tsx | 8 +- .../search_indices_router.test.tsx | 5 +- .../search_indices/search_indices_router.tsx | 7 +- .../enterprise_search_content/index.tsx | 16 +- .../enterprise_search_overview/index.tsx | 7 +- .../public/applications/esre/index.tsx | 7 +- .../public/applications/index.tsx | 2 +- .../applications/search_experiences/index.tsx | 7 +- .../applications/workplace_search/index.tsx | 16 +- .../display_settings_router.test.tsx | 5 +- .../display_settings_router.tsx | 7 +- .../synchronization_router.test.tsx | 6 +- .../synchronization_router.tsx | 8 +- .../views/content_sources/source_router.tsx | 8 +- .../views/content_sources/sources_router.tsx | 8 +- .../views/groups/group_router.test.tsx | 5 +- .../views/groups/group_router.tsx | 8 +- .../views/groups/groups_router.test.tsx | 5 +- .../views/groups/groups_router.tsx | 7 +- .../views/settings/settings_router.test.tsx | 6 +- .../views/settings/settings_router.tsx | 8 +- .../public/application/index.tsx | 7 +- .../shared/date_picker/date_picker.test.tsx | 3 +- .../hooks/use_series_storage.test.tsx | 3 +- .../shared/exploratory_view/rtl_helpers.tsx | 3 +- .../fleet/public/applications/fleet/app.tsx | 8 +- .../single_page_layout/index.test.tsx | 1 + .../agent_policy/details_page/index.tsx | 8 +- .../fleet/sections/agent_policy/index.tsx | 38 +- .../agents/agent_details_page/index.tsx | 8 +- .../fleet/sections/agents/index.tsx | 34 +- .../fleet/sections/data_stream/index.tsx | 21 +- .../fleet/sections/settings/index.tsx | 194 +++--- .../public/applications/integrations/app.tsx | 8 +- .../integrations/sections/epm/index.tsx | 7 +- .../sections/epm/screens/detail/index.tsx | 8 +- .../sections/epm/screens/home/index.tsx | 7 +- .../public/mock/create_test_renderer.tsx | 2 +- x-pack/plugins/graph/public/router.tsx | 8 +- .../public/application/app.tsx | 8 +- .../public/application/app.tsx | 8 +- .../public/application/sections/home/home.tsx | 8 +- .../index_list/detail_panel/detail_panel.js | 3 +- .../index_list/index_table/index_table.js | 3 +- .../plugins/infra/public/apps/legacy_app.tsx | 8 +- x-pack/plugins/infra/public/apps/logs_app.tsx | 7 +- .../plugins/infra/public/apps/metrics_app.tsx | 7 +- .../public/pages/link_to/link_to_logs.tsx | 8 +- .../public/pages/link_to/link_to_metrics.tsx | 8 +- .../infra/public/pages/logs/page_content.tsx | 7 +- .../infra/public/pages/metrics/index.tsx | 8 +- .../metric_detail/hooks/metrics_time.test.tsx | 2 +- .../public/application/app.tsx | 8 +- .../kubernetes_security_routes/index.tsx | 7 +- .../kubernetes_security/public/test/index.tsx | 2 +- .../lens/public/app_plugin/mounter.tsx | 8 +- .../public/application/app.js | 7 +- .../public/application/index.tsx | 2 +- .../logstash/public/application/index.tsx | 8 +- x-pack/plugins/maps/public/render_app.tsx | 8 +- .../anomaly_results_view_selector.test.tsx | 2 +- .../components/ml_page/ml_page.tsx | 10 +- .../jobs_list_page/jobs_list_page.tsx | 2 +- .../ml/public/application/routing/router.tsx | 3 +- .../monitoring/public/application/index.tsx | 8 +- .../public/application/index.tsx | 7 +- .../has_data_context.test.tsx | 2 +- .../date_picker/date_picker.test.tsx | 3 +- .../public/application/app.tsx | 13 +- .../public/hooks/use_link_props.test.tsx | 2 +- .../observability_shared/tsconfig.json | 1 + x-pack/plugins/osquery/public/application.tsx | 2 +- .../plugins/osquery/public/routes/index.tsx | 8 +- .../public/routes/live_queries/index.tsx | 8 +- .../osquery/public/routes/packs/index.tsx | 8 +- .../public/routes/saved_queries/index.tsx | 8 +- .../remote_clusters/public/application/app.js | 8 +- x-pack/plugins/remote_clusters/tsconfig.json | 1 + x-pack/plugins/rollup/public/crud_app/app.js | 8 +- x-pack/plugins/rollup/tsconfig.json | 1 + .../account_management_app.tsx | 2 +- .../api_keys/api_keys_management_app.tsx | 2 +- .../role_mappings_management_app.tsx | 4 +- .../management/roles/roles_management_app.tsx | 4 +- .../management/users/users_management_app.tsx | 8 +- .../show_top_n/cell_action/show_top_n.tsx | 2 +- .../security_solution/public/app/index.tsx | 7 +- .../security_solution/public/app/routes.tsx | 7 +- .../public/cases/pages/index.test.tsx | 2 +- .../ml_host_conditional_container.tsx | 8 +- .../ml_network_conditional_container.tsx | 8 +- .../index.test.tsx | 26 +- .../mock/endpoint/app_root_provider.tsx | 2 +- .../dashboards/pages/details/index.test.tsx | 2 +- .../public/dashboards/pages/index.tsx | 8 +- .../pages/rule_details/index.tsx | 8 +- .../detections/pages/alert_details/index.tsx | 8 +- .../alerts/alert_details_redirect.test.tsx | 2 +- .../public/detections/pages/alerts/index.tsx | 7 +- .../public/exceptions/routes.tsx | 7 +- .../hosts/pages/details/details_tabs.tsx | 7 +- .../public/explore/hosts/pages/hosts.test.tsx | 2 +- .../public/explore/hosts/pages/hosts_tabs.tsx | 7 +- .../public/explore/hosts/pages/index.tsx | 8 +- .../network/pages/details/details_tabs.tsx | 7 +- .../public/explore/network/pages/index.tsx | 8 +- .../pages/navigation/network_routes.tsx | 7 +- .../explore/network/pages/network.test.tsx | 2 +- .../users/pages/details/details_tabs.tsx | 8 +- .../public/explore/users/pages/index.tsx | 8 +- .../explore/users/pages/users_tabs.test.tsx | 2 +- .../public/explore/users/pages/users_tabs.tsx | 7 +- .../privileged_route/privileged_route.tsx | 2 +- .../management/pages/blocklist/index.tsx | 7 +- .../management/pages/endpoint_hosts/index.tsx | 7 +- .../management/pages/event_filters/index.tsx | 7 +- .../pages/host_isolation_exceptions/index.tsx | 7 +- .../public/management/pages/index.tsx | 8 +- .../public/management/pages/policy/index.tsx | 8 +- .../pages/response_actions/index.tsx | 8 +- .../management/pages/trusted_apps/index.tsx | 7 +- .../public/overview/pages/landing.test.tsx | 2 +- .../simulator/mock_resolver.tsx | 2 +- .../security_solution/public/rules/routes.tsx | 8 +- .../public/timelines/pages/index.tsx | 8 +- .../session_view/public/test/index.tsx | 2 +- x-pack/plugins/session_view/tsconfig.json | 1 + .../public/application/app.tsx | 8 +- .../public/application/index.tsx | 2 +- .../public/application/sections/home/home.tsx | 8 +- .../management/spaces_management_app.tsx | 8 +- .../public/apps/synthetics/routes.tsx | 8 +- .../public/apps/synthetics/synthetics_app.tsx | 2 +- .../synthetics/utils/testing/rtl_helpers.tsx | 2 +- .../public/legacy_uptime/app/uptime_app.tsx | 2 +- .../__snapshots__/cert_monitors.test.tsx.snap | 44 +- .../__snapshots__/cert_search.test.tsx.snap | 8 +- .../__snapshots__/cert_status.test.tsx.snap | 38 +- .../certificates/cert_monitors.test.tsx | 3 +- .../certificates/cert_search.test.tsx | 3 +- .../certificates/cert_status.test.tsx | 3 +- .../__snapshots__/monitor_tags.test.tsx.snap | 510 +++++++-------- .../monitor_bar_series.test.tsx.snap | 46 +- .../common/charts/monitor_bar_series.test.tsx | 3 +- .../components/common/monitor_tags.test.tsx | 6 +- .../monitor_charts.test.tsx.snap | 8 +- .../ml_integerations.test.tsx.snap | 4 +- .../monitor/ml/ml_integerations.test.tsx | 3 +- .../monitor/monitor_charts.test.tsx | 3 +- .../ssl_certificate.test.tsx.snap | 16 +- .../status_details/ssl_certificate.test.tsx | 3 +- .../filter_status_button.test.tsx.snap | 16 +- .../__snapshots__/status_filter.test.tsx.snap | 4 +- .../filter_status_button.test.tsx | 3 +- .../monitor_list_drawer.test.tsx.snap | 250 +++---- .../monitor_list_drawer.test.tsx | 6 +- .../monitor_list/status_filter.test.tsx | 3 +- .../certificate_form.test.tsx.snap | 28 +- .../__snapshots__/indices_form.test.tsx.snap | 28 +- .../settings/certificate_form.test.tsx | 3 +- .../components/settings/indices_form.test.tsx | 3 +- .../use_url_params.test.tsx.snap | 610 +++++------------- .../hooks/use_url_params.test.tsx | 6 +- .../lib/helper/enzyme_helpers.tsx | 2 +- .../legacy_uptime/lib/helper/rtl_helpers.tsx | 2 +- .../public/legacy_uptime/routes.tsx | 7 +- x-pack/plugins/transform/public/app/app.tsx | 7 +- .../public/application/app.tsx | 8 +- .../public/application/connectors_app.tsx | 8 +- .../public/application/home.test.tsx | 3 +- .../public/application/home.tsx | 8 +- .../components/actions_connectors_home.tsx | 8 +- .../public/application/app.tsx | 14 +- .../app/rum_dashboard/utils/test_helper.tsx | 15 +- .../url_params_context.test.tsx | 44 +- x-pack/plugins/ux/tsconfig.json | 1 + .../watcher/public/application/app.tsx | 22 +- .../plugins/alerts/public/application.tsx | 27 +- .../plugins/cases/public/application.tsx | 11 +- .../plugins/cases/tsconfig.json | 1 + .../applications/resolver_test/index.tsx | 32 +- .../plugins/resolver_test/tsconfig.json | 1 + 274 files changed, 2103 insertions(+), 2479 deletions(-) rename packages/shared-ux/router/impl/__snapshots__/{router.test.tsx.snap => route.test.tsx.snap} (100%) rename packages/shared-ux/router/impl/{router.test.tsx => route.test.tsx} (97%) rename src/plugins/kibana_react/public/router/router.tsx => packages/shared-ux/router/impl/route.tsx (66%) create mode 100644 packages/shared-ux/router/impl/routes.tsx delete mode 100644 src/plugins/kibana_react/public/router/index.ts diff --git a/.eslintrc.js b/.eslintrc.js index 299e3f41d4c7b..15d4492040efc 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -245,7 +245,7 @@ const RESTRICTED_IMPORTS = [ }, { name: 'react-router-dom', - importNames: ['Route'], + importNames: ['Router', 'Switch', 'Route'], message: 'Please use @kbn/shared-ux-router instead', }, { diff --git a/examples/bfetch_explorer/public/containers/app/index.tsx b/examples/bfetch_explorer/public/containers/app/index.tsx index 25678b2128d1c..54395ff4eb46d 100644 --- a/examples/bfetch_explorer/public/containers/app/index.tsx +++ b/examples/bfetch_explorer/public/containers/app/index.tsx @@ -7,9 +7,8 @@ */ import React from 'react'; -import { BrowserRouter as Router, Redirect, Switch } from 'react-router-dom'; -import { CompatRouter } from 'react-router-dom-v5-compat'; -import { Route } from '@kbn/shared-ux-router'; +import { Redirect } from 'react-router-dom'; +import { BrowserRouter as Router, Route, Routes } from '@kbn/shared-ux-router'; import { EuiPage } from '@elastic/eui'; import { useDeps } from '../../hooks/use_deps'; import { Sidebar } from './sidebar'; @@ -27,15 +26,13 @@ export const App: React.FC = () => { return ( - - - - - {routeElements} - - - - + + + + {routeElements} + + + ); }; diff --git a/examples/content_management_examples/public/examples/index.tsx b/examples/content_management_examples/public/examples/index.tsx index bb002510cb88a..dbf1ed1bc01e2 100644 --- a/examples/content_management_examples/public/examples/index.tsx +++ b/examples/content_management_examples/public/examples/index.tsx @@ -8,9 +8,8 @@ import React from 'react'; import ReactDOM from 'react-dom'; -// eslint-disable-next-line no-restricted-imports -import { Router, Switch, Route, Redirect } from 'react-router-dom'; -import { CompatRouter } from 'react-router-dom-v5-compat'; +import { Redirect } from 'react-router-dom'; +import { Router, Routes, Route } from '@kbn/shared-ux-router'; import { RedirectAppLinks } from '@kbn/shared-ux-link-redirect-app'; import { EuiPageTemplate, EuiSideNav } from '@elastic/eui'; import { AppMountParameters, CoreStart } from '@kbn/core/public'; @@ -25,52 +24,50 @@ export const renderApp = ( ) => { ReactDOM.render( - - - - - - + + + + + - - - - - - - - - - - - - - + + + + + + + + + + + + + , element ); diff --git a/examples/content_management_examples/tsconfig.json b/examples/content_management_examples/tsconfig.json index ba1c3f19850c6..a9f20f45792a6 100644 --- a/examples/content_management_examples/tsconfig.json +++ b/examples/content_management_examples/tsconfig.json @@ -28,5 +28,6 @@ "@kbn/core-saved-objects-api-browser", "@kbn/content-management-table-list-view-table", "@kbn/content-management-table-list-view", + "@kbn/shared-ux-router", ] } diff --git a/examples/embeddable_explorer/public/app.tsx b/examples/embeddable_explorer/public/app.tsx index 20b8c024849b3..ac24e45818bf5 100644 --- a/examples/embeddable_explorer/public/app.tsx +++ b/examples/embeddable_explorer/public/app.tsx @@ -9,7 +9,6 @@ import React from 'react'; import ReactDOM from 'react-dom'; import { BrowserRouter as Router, withRouter, RouteComponentProps } from 'react-router-dom'; -import { CompatRouter } from 'react-router-dom-v5-compat'; import { Route } from '@kbn/shared-ux-router'; import { EuiPage, EuiPageSideBar_Deprecated as EuiPageSideBar, EuiSideNav } from '@elastic/eui'; @@ -127,14 +126,12 @@ const EmbeddableExplorerApp = ({ return ( - - - -