From 66d6096cceec516e2eaadfbf9a5cee4738fd4f2a Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 13 Mar 2024 17:33:09 +0800 Subject: [PATCH 01/41] feat: POC implementation Signed-off-by: SuZhou-Joe --- src/plugins/workspace/common/constants.ts | 2 + .../workspace/opensearch_dashboards.json | 4 +- src/plugins/workspace/server/constant.ts | 11 ++ src/plugins/workspace/server/plugin.ts | 13 ++ .../workspace_id_consumer_wrapper.ts | 117 ++++++++++++++++++ .../workspace/server/workspace_client.ts | 1 + 6 files changed, 145 insertions(+), 3 deletions(-) create mode 100644 src/plugins/workspace/server/constant.ts create mode 100644 src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts diff --git a/src/plugins/workspace/common/constants.ts b/src/plugins/workspace/common/constants.ts index 2be4a9d121db..1781b7e32771 100644 --- a/src/plugins/workspace/common/constants.ts +++ b/src/plugins/workspace/common/constants.ts @@ -20,3 +20,5 @@ export enum WorkspacePermissionMode { LibraryRead = 'library_read', LibraryWrite = 'library_write', } + +export const WORKSPACE_ID_CONSUMER_WRAPPER_ID = 'workspace_id_consumer'; diff --git a/src/plugins/workspace/opensearch_dashboards.json b/src/plugins/workspace/opensearch_dashboards.json index 7d94a7491a00..0f1d851d5d76 100644 --- a/src/plugins/workspace/opensearch_dashboards.json +++ b/src/plugins/workspace/opensearch_dashboards.json @@ -3,9 +3,7 @@ "version": "opensearchDashboards", "server": true, "ui": true, - "requiredPlugins": [ - "savedObjects" - ], + "requiredPlugins": [], "optionalPlugins": ["savedObjectsManagement"], "requiredBundles": ["opensearchDashboardsReact"] } diff --git a/src/plugins/workspace/server/constant.ts b/src/plugins/workspace/server/constant.ts new file mode 100644 index 000000000000..d3cd904b1cf5 --- /dev/null +++ b/src/plugins/workspace/server/constant.ts @@ -0,0 +1,11 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * A symbol used for imply the workspace id in original url + * + * @Internal + */ +export const workspaceIdInUrlSymbol = Symbol('workspace_id_in_url'); diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index 0fff0082476b..5c42ae476033 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -15,6 +15,7 @@ import { import { WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID, WORKSPACE_CONFLICT_CONTROL_SAVED_OBJECTS_CLIENT_WRAPPER_ID, + WORKSPACE_ID_CONSUMER_WRAPPER_ID, } from '../common/constants'; import { IWorkspaceClientImpl } from './types'; import { WorkspaceClient } from './workspace_client'; @@ -27,6 +28,10 @@ import { SavedObjectsPermissionControlContract, } from './permission_control/client'; import { WorkspacePluginConfigType } from '../config'; +import { workspaceIdInUrlSymbol } from './constant'; +import { WorkspaceIdConsumerWrapper } from './saved_objects/workspace_id_consumer_wrapper'; +// eslint-disable-next-line @osd/eslint/no-restricted-paths +import { ensureRawRequest } from '../../../core/server/http/router'; // will be an issue as the ensureRawRequest is an internal implementation export class WorkspacePlugin implements Plugin<{}, {}> { private readonly logger: Logger; @@ -47,6 +52,8 @@ export class WorkspacePlugin implements Plugin<{}, {}> { ); if (workspaceId) { + const rawRequest = ensureRawRequest(request); + rawRequest.headers[workspaceIdInUrlSymbol.toString()] = workspaceId; const requestUrl = new URL(request.url.toString()); requestUrl.pathname = cleanWorkspaceId(requestUrl.pathname); return toolkit.rewriteUrl(requestUrl.toString()); @@ -94,6 +101,12 @@ export class WorkspacePlugin implements Plugin<{}, {}> { ); } + core.savedObjects.addClientWrapper( + -2, + WORKSPACE_ID_CONSUMER_WRAPPER_ID, + new WorkspaceIdConsumerWrapper().wrapperFactory + ); + registerRoutes({ http: core.http, logger: this.logger, diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts new file mode 100644 index 000000000000..4c302fc93518 --- /dev/null +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts @@ -0,0 +1,117 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { + SavedObjectsBaseOptions, + SavedObjectsBulkCreateObject, + SavedObjectsClientWrapperFactory, + SavedObjectsCreateOptions, + SavedObjectsCheckConflictsObject, + OpenSearchDashboardsRequest, + SavedObjectsFindOptions, + SavedObjectsUpdateOptions, + SavedObjectsBulkUpdateOptions, + SavedObjectsBulkUpdateObject, + WORKSPACE_TYPE, +} from '../../../../core/server'; +import { workspaceIdInUrlSymbol } from '../constant'; + +type WorkspaceOptions = + | { + workspaces?: string[]; + } + | undefined; + +export class WorkspaceIdConsumerWrapper { + private typeRelatedToWorkspace(type: string | string[]): boolean { + if (Array.isArray(type)) { + return type.some((item) => item === WORKSPACE_TYPE); + } + + return type === WORKSPACE_TYPE; + } + private formatWorkspaceIdParams( + request: OpenSearchDashboardsRequest, + options: T + ): T { + if (!options) { + return options; + } + const { workspaces, ...others } = options; + const workspaceIdParsedFromUrl = request.headers[workspaceIdInUrlSymbol.toString()] as string; + const workspaceIdInUserOptions = options.workspaces; + let finalWorkspaces: string[] = []; + if (workspaceIdInUserOptions?.length) { + finalWorkspaces = workspaceIdInUserOptions; + } else if (workspaceIdParsedFromUrl) { + finalWorkspaces = [workspaceIdParsedFromUrl]; + } + + return { + ...(others as T), + ...(finalWorkspaces.length ? { workspaces: finalWorkspaces } : {}), + }; + } + public wrapperFactory: SavedObjectsClientWrapperFactory = (wrapperOptions) => { + return { + ...wrapperOptions.client, + create: (type: string, attributes: T, options: SavedObjectsCreateOptions = {}) => + wrapperOptions.client.create( + type, + attributes, + this.formatWorkspaceIdParams(wrapperOptions.request, options) + ), + bulkCreate: ( + objects: Array>, + options: SavedObjectsCreateOptions = {} + ) => + wrapperOptions.client.bulkCreate( + objects, + this.formatWorkspaceIdParams(wrapperOptions.request, options) + ), + checkConflicts: ( + objects: SavedObjectsCheckConflictsObject[] = [], + options: SavedObjectsBaseOptions = {} + ) => + wrapperOptions.client.checkConflicts( + objects, + this.formatWorkspaceIdParams(wrapperOptions.request, options) + ), + delete: wrapperOptions.client.delete, + find: (options: SavedObjectsFindOptions) => + wrapperOptions.client.find( + this.typeRelatedToWorkspace(options.type) + ? options + : this.formatWorkspaceIdParams(wrapperOptions.request, options) + ), + bulkGet: wrapperOptions.client.bulkGet, + get: wrapperOptions.client.get, + update: ( + type: string, + id: string, + attributes: Partial, + options: SavedObjectsUpdateOptions = {} + ) => + wrapperOptions.client.update( + type, + id, + attributes, + this.formatWorkspaceIdParams(wrapperOptions.request, options) + ), + bulkUpdate: ( + objects: Array>, + options?: SavedObjectsBulkUpdateOptions + ) => + wrapperOptions.client.bulkUpdate( + objects, + this.formatWorkspaceIdParams(wrapperOptions.request, options) + ), + addToNamespaces: wrapperOptions.client.addToNamespaces, + deleteFromNamespaces: wrapperOptions.client.deleteFromNamespaces, + }; + }; + + constructor() {} +} diff --git a/src/plugins/workspace/server/workspace_client.ts b/src/plugins/workspace/server/workspace_client.ts index e7bdf97b54ec..0ffb72f016f2 100644 --- a/src/plugins/workspace/server/workspace_client.ts +++ b/src/plugins/workspace/server/workspace_client.ts @@ -21,6 +21,7 @@ import { IWorkspaceClientImpl, WorkspaceFindOptions, IResponse, IRequestDetail } import { workspace } from './saved_objects'; import { generateRandomId } from './utils'; import { + WORKSPACE_ID_CONSUMER_WRAPPER_ID, WORKSPACE_OVERVIEW_APP_ID, WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID, WORKSPACE_UPDATE_APP_ID, From d275f6173e5e68986505e2a8c050b29682102ae1 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 13 Mar 2024 17:37:46 +0800 Subject: [PATCH 02/41] feat: add some comment Signed-off-by: SuZhou-Joe --- src/plugins/workspace/server/plugin.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index 5c42ae476033..ecea0bb84e71 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -101,6 +101,8 @@ export class WorkspacePlugin implements Plugin<{}, {}> { ); } + // TOO many client wrapper inside a single workspace plugin + // Try to combine conflict wrapper and this consumer wrapper. core.savedObjects.addClientWrapper( -2, WORKSPACE_ID_CONSUMER_WRAPPER_ID, From 65f6be3b585ea0c7e77445e2621525286567b406 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 13 Mar 2024 17:39:15 +0800 Subject: [PATCH 03/41] feat: revert dependency Signed-off-by: SuZhou-Joe --- src/plugins/workspace/opensearch_dashboards.json | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/plugins/workspace/opensearch_dashboards.json b/src/plugins/workspace/opensearch_dashboards.json index 0f1d851d5d76..7d94a7491a00 100644 --- a/src/plugins/workspace/opensearch_dashboards.json +++ b/src/plugins/workspace/opensearch_dashboards.json @@ -3,7 +3,9 @@ "version": "opensearchDashboards", "server": true, "ui": true, - "requiredPlugins": [], + "requiredPlugins": [ + "savedObjects" + ], "optionalPlugins": ["savedObjectsManagement"], "requiredBundles": ["opensearchDashboardsReact"] } From 9c5d2d9851474760abe4fde95cae9c5a321c5f59 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 13 Mar 2024 17:41:40 +0800 Subject: [PATCH 04/41] feat: update comment Signed-off-by: SuZhou-Joe --- src/plugins/workspace/server/plugin.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index ecea0bb84e71..3cedf77a4637 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -31,7 +31,7 @@ import { WorkspacePluginConfigType } from '../config'; import { workspaceIdInUrlSymbol } from './constant'; import { WorkspaceIdConsumerWrapper } from './saved_objects/workspace_id_consumer_wrapper'; // eslint-disable-next-line @osd/eslint/no-restricted-paths -import { ensureRawRequest } from '../../../core/server/http/router'; // will be an issue as the ensureRawRequest is an internal implementation +import { ensureRawRequest } from '../../../core/server/http/router'; // TODO: move the ensureRawRequest to a core module or the import will be an issue as the ensureRawRequest can only be import from core module export class WorkspacePlugin implements Plugin<{}, {}> { private readonly logger: Logger; @@ -101,7 +101,7 @@ export class WorkspacePlugin implements Plugin<{}, {}> { ); } - // TOO many client wrapper inside a single workspace plugin + // TODO: TOO many client wrapper inside a single workspace plugin // Try to combine conflict wrapper and this consumer wrapper. core.savedObjects.addClientWrapper( -2, From d5638ff248adbb6fc4d777d5fe7645c1792a5233 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Fri, 22 Mar 2024 14:53:01 +0800 Subject: [PATCH 05/41] feat: address one TODO Signed-off-by: SuZhou-Joe --- src/plugins/workspace/server/constant.ts | 11 ----------- src/plugins/workspace/server/plugin.ts | 14 ++++++++------ .../workspace_id_consumer_wrapper.ts | 15 ++++++++------- 3 files changed, 16 insertions(+), 24 deletions(-) delete mode 100644 src/plugins/workspace/server/constant.ts diff --git a/src/plugins/workspace/server/constant.ts b/src/plugins/workspace/server/constant.ts deleted file mode 100644 index d3cd904b1cf5..000000000000 --- a/src/plugins/workspace/server/constant.ts +++ /dev/null @@ -1,11 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -/** - * A symbol used for imply the workspace id in original url - * - * @Internal - */ -export const workspaceIdInUrlSymbol = Symbol('workspace_id_in_url'); diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index 3cedf77a4637..21de1deeca10 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -21,17 +21,18 @@ import { IWorkspaceClientImpl } from './types'; import { WorkspaceClient } from './workspace_client'; import { registerRoutes } from './routes'; import { WorkspaceSavedObjectsClientWrapper } from './saved_objects'; -import { cleanWorkspaceId, getWorkspaceIdFromUrl } from '../../../core/server/utils'; +import { + cleanWorkspaceId, + getWorkspaceIdFromUrl, + updateWorkspaceState, +} from '../../../core/server/utils'; import { WorkspaceConflictSavedObjectsClientWrapper } from './saved_objects/saved_objects_wrapper_for_check_workspace_conflict'; import { SavedObjectsPermissionControl, SavedObjectsPermissionControlContract, } from './permission_control/client'; import { WorkspacePluginConfigType } from '../config'; -import { workspaceIdInUrlSymbol } from './constant'; import { WorkspaceIdConsumerWrapper } from './saved_objects/workspace_id_consumer_wrapper'; -// eslint-disable-next-line @osd/eslint/no-restricted-paths -import { ensureRawRequest } from '../../../core/server/http/router'; // TODO: move the ensureRawRequest to a core module or the import will be an issue as the ensureRawRequest can only be import from core module export class WorkspacePlugin implements Plugin<{}, {}> { private readonly logger: Logger; @@ -52,8 +53,9 @@ export class WorkspacePlugin implements Plugin<{}, {}> { ); if (workspaceId) { - const rawRequest = ensureRawRequest(request); - rawRequest.headers[workspaceIdInUrlSymbol.toString()] = workspaceId; + updateWorkspaceState(request, { + id: workspaceId, + }); const requestUrl = new URL(request.url.toString()); requestUrl.pathname = cleanWorkspaceId(requestUrl.pathname); return toolkit.rewriteUrl(requestUrl.toString()); diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts index 4c302fc93518..1135b8ca2eed 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts @@ -3,6 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ +import { getWorkspaceState } from '../../../../core/server/utils'; import { SavedObjectsBaseOptions, SavedObjectsBulkCreateObject, @@ -16,7 +17,6 @@ import { SavedObjectsBulkUpdateObject, WORKSPACE_TYPE, } from '../../../../core/server'; -import { workspaceIdInUrlSymbol } from '../constant'; type WorkspaceOptions = | { @@ -40,13 +40,14 @@ export class WorkspaceIdConsumerWrapper { return options; } const { workspaces, ...others } = options; - const workspaceIdParsedFromUrl = request.headers[workspaceIdInUrlSymbol.toString()] as string; - const workspaceIdInUserOptions = options.workspaces; + const workspaceState = getWorkspaceState(request); + const workspaceIdParsedFromRequest = workspaceState?.id; + const workspaceIdsInUserOptions = options.workspaces; let finalWorkspaces: string[] = []; - if (workspaceIdInUserOptions?.length) { - finalWorkspaces = workspaceIdInUserOptions; - } else if (workspaceIdParsedFromUrl) { - finalWorkspaces = [workspaceIdParsedFromUrl]; + if (options.hasOwnProperty('workspaces')) { + finalWorkspaces = workspaceIdsInUserOptions || []; + } else if (workspaceIdParsedFromRequest) { + finalWorkspaces = [workspaceIdParsedFromRequest]; } return { From 45589cff80409190f98a403da659fa6e34d0ec40 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Fri, 22 Mar 2024 15:24:25 +0800 Subject: [PATCH 06/41] feat: address TODO Signed-off-by: SuZhou-Joe --- src/plugins/workspace/server/plugin.ts | 14 +++++------ .../workspace_id_consumer_wrapper.ts | 25 ++----------------- .../workspace/server/workspace_client.ts | 7 +----- 3 files changed, 9 insertions(+), 37 deletions(-) diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index 21de1deeca10..088096df4d35 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -88,6 +88,12 @@ export class WorkspacePlugin implements Plugin<{}, {}> { this.workspaceConflictControl.wrapperFactory ); + core.savedObjects.addClientWrapper( + -2, + WORKSPACE_ID_CONSUMER_WRAPPER_ID, + new WorkspaceIdConsumerWrapper().wrapperFactory + ); + this.logger.info('Workspace permission control enabled:' + isPermissionControlEnabled); if (isPermissionControlEnabled) { this.permissionControl = new SavedObjectsPermissionControl(this.logger); @@ -103,14 +109,6 @@ export class WorkspacePlugin implements Plugin<{}, {}> { ); } - // TODO: TOO many client wrapper inside a single workspace plugin - // Try to combine conflict wrapper and this consumer wrapper. - core.savedObjects.addClientWrapper( - -2, - WORKSPACE_ID_CONSUMER_WRAPPER_ID, - new WorkspaceIdConsumerWrapper().wrapperFactory - ); - registerRoutes({ http: core.http, logger: this.logger, diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts index 1135b8ca2eed..321a026d943e 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts @@ -12,9 +12,6 @@ import { SavedObjectsCheckConflictsObject, OpenSearchDashboardsRequest, SavedObjectsFindOptions, - SavedObjectsUpdateOptions, - SavedObjectsBulkUpdateOptions, - SavedObjectsBulkUpdateObject, WORKSPACE_TYPE, } from '../../../../core/server'; @@ -89,26 +86,8 @@ export class WorkspaceIdConsumerWrapper { ), bulkGet: wrapperOptions.client.bulkGet, get: wrapperOptions.client.get, - update: ( - type: string, - id: string, - attributes: Partial, - options: SavedObjectsUpdateOptions = {} - ) => - wrapperOptions.client.update( - type, - id, - attributes, - this.formatWorkspaceIdParams(wrapperOptions.request, options) - ), - bulkUpdate: ( - objects: Array>, - options?: SavedObjectsBulkUpdateOptions - ) => - wrapperOptions.client.bulkUpdate( - objects, - this.formatWorkspaceIdParams(wrapperOptions.request, options) - ), + update: wrapperOptions.client.update, + bulkUpdate: wrapperOptions.client.bulkUpdate, addToNamespaces: wrapperOptions.client.addToNamespaces, deleteFromNamespaces: wrapperOptions.client.deleteFromNamespaces, }; diff --git a/src/plugins/workspace/server/workspace_client.ts b/src/plugins/workspace/server/workspace_client.ts index 0ffb72f016f2..05e323f63f02 100644 --- a/src/plugins/workspace/server/workspace_client.ts +++ b/src/plugins/workspace/server/workspace_client.ts @@ -20,12 +20,7 @@ import { import { IWorkspaceClientImpl, WorkspaceFindOptions, IResponse, IRequestDetail } from './types'; import { workspace } from './saved_objects'; import { generateRandomId } from './utils'; -import { - WORKSPACE_ID_CONSUMER_WRAPPER_ID, - WORKSPACE_OVERVIEW_APP_ID, - WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID, - WORKSPACE_UPDATE_APP_ID, -} from '../common/constants'; +import { WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID } from '../common/constants'; const WORKSPACE_ID_SIZE = 6; From 5d990c8211931563612b03f1ce487fa177451e6b Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Fri, 22 Mar 2024 16:46:15 +0800 Subject: [PATCH 07/41] feat: add unit test Signed-off-by: SuZhou-Joe --- src/plugins/workspace/server/plugin.test.ts | 65 +++++++++- .../workspace_id_consumer_wrapper.test.ts | 118 ++++++++++++++++++ .../workspace_id_consumer_wrapper.ts | 25 +--- .../workspace/server/workspace_client.ts | 12 +- 4 files changed, 198 insertions(+), 22 deletions(-) create mode 100644 src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts diff --git a/src/plugins/workspace/server/plugin.test.ts b/src/plugins/workspace/server/plugin.test.ts index c448fcf209f9..300b5e982a01 100644 --- a/src/plugins/workspace/server/plugin.test.ts +++ b/src/plugins/workspace/server/plugin.test.ts @@ -3,8 +3,10 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { coreMock } from '../../../core/server/mocks'; +import { OnPreRoutingHandler } from 'src/core/server'; +import { coreMock, httpServerMock } from '../../../core/server/mocks'; import { WorkspacePlugin } from './plugin'; +import { getWorkspaceState } from '../../../core/server/utils'; describe('Workspace server plugin', () => { it('#setup', async () => { @@ -27,5 +29,66 @@ describe('Workspace server plugin', () => { }, } `); + expect(setupMock.savedObjects.addClientWrapper).toBeCalledTimes(3); + }); + + it('#proxyWorkspaceTrafficToRealHandler', async () => { + const setupMock = coreMock.createSetup(); + const initializerContextConfigMock = coreMock.createPluginInitializerContext({ + enabled: true, + permission: { + enabled: true, + }, + }); + let onPreRoutingFn: OnPreRoutingHandler = () => httpServerMock.createResponseFactory().ok(); + setupMock.http.registerOnPreRouting.mockImplementation((fn) => { + onPreRoutingFn = fn; + return fn; + }); + const workspacePlugin = new WorkspacePlugin(initializerContextConfigMock); + await workspacePlugin.setup(setupMock); + const toolKitMock = httpServerMock.createToolkit(); + + const requestWithWorkspaceInUrl = httpServerMock.createOpenSearchDashboardsRequest({ + path: '/w/foo/app', + }); + onPreRoutingFn(requestWithWorkspaceInUrl, httpServerMock.createResponseFactory(), toolKitMock); + expect(toolKitMock.rewriteUrl).toBeCalledWith('http://localhost/app'); + expect(toolKitMock.next).toBeCalledTimes(0); + expect(getWorkspaceState(requestWithWorkspaceInUrl)).toEqual({ + id: 'foo', + }); + + const requestWithoutWorkspaceInUrl = httpServerMock.createOpenSearchDashboardsRequest({ + path: '/app', + }); + onPreRoutingFn( + requestWithoutWorkspaceInUrl, + httpServerMock.createResponseFactory(), + toolKitMock + ); + expect(toolKitMock.next).toBeCalledTimes(1); + }); + + it('#start', async () => { + const setupMock = coreMock.createSetup(); + const startMock = coreMock.createStart(); + const initializerContextConfigMock = coreMock.createPluginInitializerContext({ + enabled: true, + permission: { + enabled: true, + }, + }); + + const workspacePlugin = new WorkspacePlugin(initializerContextConfigMock); + await workspacePlugin.setup(setupMock); + await workspacePlugin.start(startMock); + expect(startMock.savedObjects.createSerializer).toBeCalledTimes(1); + }); + + it('#stop', () => { + const initializerContextConfigMock = coreMock.createPluginInitializerContext(); + const workspacePlugin = new WorkspacePlugin(initializerContextConfigMock); + workspacePlugin.stop(); }); }); diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts new file mode 100644 index 000000000000..8715606da681 --- /dev/null +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts @@ -0,0 +1,118 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { updateWorkspaceState } from '../../../../core/server/utils'; +import { SavedObject } from '../../../../core/public'; +import { httpServerMock, savedObjectsClientMock, coreMock } from '../../../../core/server/mocks'; +import { WorkspaceIdConsumerWrapper } from './workspace_id_consumer_wrapper'; + +describe('WorkspaceIdConsumerWrapper', () => { + const requestHandlerContext = coreMock.createRequestHandlerContext(); + const wrapperInstance = new WorkspaceIdConsumerWrapper(); + const mockedClient = savedObjectsClientMock.create(); + const workspaceEnabledMockRequest = httpServerMock.createOpenSearchDashboardsRequest(); + updateWorkspaceState(workspaceEnabledMockRequest, { + id: 'foo', + }); + const wrapperClient = wrapperInstance.wrapperFactory({ + client: mockedClient, + typeRegistry: requestHandlerContext.savedObjects.typeRegistry, + request: workspaceEnabledMockRequest, + }); + const getSavedObject = (savedObject: Partial) => { + const payload: SavedObject = { + references: [], + id: '', + type: 'dashboard', + attributes: {}, + ...savedObject, + }; + + return payload; + }; + describe('create', () => { + beforeEach(() => { + mockedClient.create.mockClear(); + }); + it(`Should add workspaces parameters when create`, async () => { + await wrapperClient.create('dashboard', { + name: 'foo', + }); + + expect(mockedClient.create).toBeCalledWith( + expect.anything(), + expect.anything(), + expect.objectContaining({ + workspaces: ['foo'], + }) + ); + }); + + it(`Should use options.workspaces there is workspaces inside options`, async () => { + await wrapperClient.create( + 'dashboard', + { + name: 'foo', + }, + { + id: 'dashboard:foo', + overwrite: true, + workspaces: undefined, + } + ); + + expect(mockedClient.create.mock.calls[0][2]?.hasOwnProperty('workspaces')).toEqual(false); + }); + }); + + describe('bulkCreate', () => { + beforeEach(() => { + mockedClient.bulkCreate.mockClear(); + }); + it(`Should add workspaces parameters when bulk create`, async () => { + await wrapperClient.bulkCreate([ + getSavedObject({ + id: 'foo', + }), + ]); + + expect(mockedClient.bulkCreate).toBeCalledWith( + [{ attributes: {}, id: 'foo', references: [], type: 'dashboard' }], + { + workspaces: ['foo'], + } + ); + }); + }); + + describe('checkConflict', () => { + beforeEach(() => { + mockedClient.checkConflicts.mockClear(); + }); + + it(`Should add workspaces parameters when checkConflict`, async () => { + await wrapperClient.checkConflicts([]); + expect(mockedClient.checkConflicts).toBeCalledWith([], { + workspaces: ['foo'], + }); + }); + }); + + describe('find', () => { + beforeEach(() => { + mockedClient.find.mockClear(); + }); + + it(`Should add workspaces parameters when find`, async () => { + await wrapperClient.find({ + type: 'dashboard', + }); + expect(mockedClient.find).toBeCalledWith({ + type: 'dashboard', + workspaces: ['foo'], + }); + }); + }); +}); diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts index 321a026d943e..bf450841f001 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts @@ -12,7 +12,6 @@ import { SavedObjectsCheckConflictsObject, OpenSearchDashboardsRequest, SavedObjectsFindOptions, - WORKSPACE_TYPE, } from '../../../../core/server'; type WorkspaceOptions = @@ -22,26 +21,16 @@ type WorkspaceOptions = | undefined; export class WorkspaceIdConsumerWrapper { - private typeRelatedToWorkspace(type: string | string[]): boolean { - if (Array.isArray(type)) { - return type.some((item) => item === WORKSPACE_TYPE); - } - - return type === WORKSPACE_TYPE; - } private formatWorkspaceIdParams( request: OpenSearchDashboardsRequest, - options: T + options?: T ): T { - if (!options) { - return options; - } - const { workspaces, ...others } = options; + const { workspaces, ...others } = options || {}; const workspaceState = getWorkspaceState(request); const workspaceIdParsedFromRequest = workspaceState?.id; - const workspaceIdsInUserOptions = options.workspaces; + const workspaceIdsInUserOptions = options?.workspaces; let finalWorkspaces: string[] = []; - if (options.hasOwnProperty('workspaces')) { + if (options?.hasOwnProperty('workspaces')) { finalWorkspaces = workspaceIdsInUserOptions || []; } else if (workspaceIdParsedFromRequest) { finalWorkspaces = [workspaceIdParsedFromRequest]; @@ -79,11 +68,7 @@ export class WorkspaceIdConsumerWrapper { ), delete: wrapperOptions.client.delete, find: (options: SavedObjectsFindOptions) => - wrapperOptions.client.find( - this.typeRelatedToWorkspace(options.type) - ? options - : this.formatWorkspaceIdParams(wrapperOptions.request, options) - ), + wrapperOptions.client.find(this.formatWorkspaceIdParams(wrapperOptions.request, options)), bulkGet: wrapperOptions.client.bulkGet, get: wrapperOptions.client.get, update: wrapperOptions.client.update, diff --git a/src/plugins/workspace/server/workspace_client.ts b/src/plugins/workspace/server/workspace_client.ts index 05e323f63f02..3378ab650cd1 100644 --- a/src/plugins/workspace/server/workspace_client.ts +++ b/src/plugins/workspace/server/workspace_client.ts @@ -20,7 +20,10 @@ import { import { IWorkspaceClientImpl, WorkspaceFindOptions, IResponse, IRequestDetail } from './types'; import { workspace } from './saved_objects'; import { generateRandomId } from './utils'; -import { WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID } from '../common/constants'; +import { + WORKSPACE_ID_CONSUMER_WRAPPER_ID, + WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID, +} from '../common/constants'; const WORKSPACE_ID_SIZE = 6; @@ -56,6 +59,13 @@ export class WorkspaceClient implements IWorkspaceClientImpl { requestDetail: IRequestDetail ): SavedObjectsClientContract { return this.savedObjects?.getScopedClient(requestDetail.request, { + /** + * workspace object does not have workspaces field + * so need to bypass the consumer wrapper + * or it will append workspaces into the options.workspaces + * when list all the workspaces inside a workspace + */ + excludedWrappers: [WORKSPACE_ID_CONSUMER_WRAPPER_ID], includedHiddenTypes: [WORKSPACE_TYPE], }) as SavedObjectsClientContract; } From 08dc9e2be28177a037c14d4668cca0bfac042ffc Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Fri, 22 Mar 2024 18:27:09 +0800 Subject: [PATCH 08/41] feat: some special logic on specific operation Signed-off-by: SuZhou-Joe --- .../workspace_id_consumer_wrapper.ts | 4 ++-- .../workspace_saved_objects_client_wrapper.ts | 3 +++ src/plugins/workspace/server/workspace_client.ts | 16 +++++++++------- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts index bf450841f001..d24496ed31ef 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts @@ -30,8 +30,8 @@ export class WorkspaceIdConsumerWrapper { const workspaceIdParsedFromRequest = workspaceState?.id; const workspaceIdsInUserOptions = options?.workspaces; let finalWorkspaces: string[] = []; - if (options?.hasOwnProperty('workspaces')) { - finalWorkspaces = workspaceIdsInUserOptions || []; + if (workspaceIdsInUserOptions) { + finalWorkspaces = workspaceIdsInUserOptions; } else if (workspaceIdParsedFromRequest) { finalWorkspaces = [workspaceIdParsedFromRequest]; } diff --git a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts index c515f555fa4b..1a97b2dac69a 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts @@ -468,6 +468,9 @@ export class WorkspaceSavedObjectsClientWrapper { [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.LibraryWrite] ), }, + // By declaring workspaces as empty array, + // workspaces won't be appended automatically into the options. + workspaces: [], }) ).saved_objects.map((item) => item.id); diff --git a/src/plugins/workspace/server/workspace_client.ts b/src/plugins/workspace/server/workspace_client.ts index 3378ab650cd1..a8b006c99a87 100644 --- a/src/plugins/workspace/server/workspace_client.ts +++ b/src/plugins/workspace/server/workspace_client.ts @@ -50,7 +50,15 @@ export class WorkspaceClient implements IWorkspaceClientImpl { requestDetail: IRequestDetail ): SavedObjectsClientContract | undefined { return this.savedObjects?.getScopedClient(requestDetail.request, { - excludedWrappers: [WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID], + excludedWrappers: [ + WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID, + /** + * workspace object does not have workspaces field + * so need to bypass workspace id consumer wrapper + * for any kind of operation to saved objects client. + */ + WORKSPACE_ID_CONSUMER_WRAPPER_ID, + ], includedHiddenTypes: [WORKSPACE_TYPE], }); } @@ -59,12 +67,6 @@ export class WorkspaceClient implements IWorkspaceClientImpl { requestDetail: IRequestDetail ): SavedObjectsClientContract { return this.savedObjects?.getScopedClient(requestDetail.request, { - /** - * workspace object does not have workspaces field - * so need to bypass the consumer wrapper - * or it will append workspaces into the options.workspaces - * when list all the workspaces inside a workspace - */ excludedWrappers: [WORKSPACE_ID_CONSUMER_WRAPPER_ID], includedHiddenTypes: [WORKSPACE_TYPE], }) as SavedObjectsClientContract; From d3cccb8585197ab93e92a2e66f8acf354b9732b8 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Fri, 22 Mar 2024 18:27:47 +0800 Subject: [PATCH 09/41] feat: add integration test Signed-off-by: SuZhou-Joe --- .../workspace_id_consumer_wrapper.test.ts | 242 ++++++++++++++++++ 1 file changed, 242 insertions(+) create mode 100644 src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts diff --git a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts new file mode 100644 index 000000000000..381da5227d8d --- /dev/null +++ b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts @@ -0,0 +1,242 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { SavedObject } from 'src/core/types'; +import { isEqual } from 'lodash'; +import * as osdTestServer from '../../../../../core/test_helpers/osd_server'; + +const dashboard: Omit = { + type: 'dashboard', + attributes: {}, + references: [], +}; + +interface WorkspaceAttributes { + id: string; + name?: string; +} + +describe('workspace_id_consumer integration test', () => { + let root: ReturnType; + let opensearchServer: osdTestServer.TestOpenSearchUtils; + let createdFooWorkspace: WorkspaceAttributes = { + id: '', + }; + let createdBarWorkspace: WorkspaceAttributes = { + id: '', + }; + beforeAll(async () => { + const { startOpenSearch, startOpenSearchDashboards } = osdTestServer.createTestServers({ + adjustTimeout: (t: number) => jest.setTimeout(t), + settings: { + osd: { + workspace: { + enabled: true, + }, + savedObjects: { + permission: { + enabled: true, + }, + }, + migrations: { + skip: false, + }, + }, + }, + }); + opensearchServer = await startOpenSearch(); + const startOSDResp = await startOpenSearchDashboards(); + root = startOSDResp.root; + const createWorkspace = (workspaceAttribute: Omit) => + osdTestServer.request.post(root, `/api/workspaces`).send({ + attributes: workspaceAttribute, + }); + + createdFooWorkspace = await createWorkspace({ + name: 'foo', + }).then((resp) => { + return resp.body.result; + }); + createdBarWorkspace = await createWorkspace({ + name: 'bar', + }).then((resp) => resp.body.result); + }, 30000); + afterAll(async () => { + await root.shutdown(); + await opensearchServer.stop(); + }); + + const deleteItem = async (object: Pick) => { + expect( + [200, 404].includes( + (await osdTestServer.request.delete(root, `/api/saved_objects/${object.type}/${object.id}`)) + .statusCode + ) + ).toEqual(true); + }; + + const getItem = async (object: Pick) => { + return await osdTestServer.request + .get(root, `/api/saved_objects/${object.type}/${object.id}`) + .expect(200); + }; + + const clearFooAndBar = async () => { + await deleteItem({ + type: dashboard.type, + id: 'foo', + }); + await deleteItem({ + type: dashboard.type, + id: 'bar', + }); + }; + + describe('saved objects client related CRUD', () => { + it('create', async () => { + const createResult = await osdTestServer.request + .post(root, `/w/${createdFooWorkspace.id}/api/saved_objects/${dashboard.type}`) + .send({ + attributes: dashboard.attributes, + }) + .expect(200); + + expect(createResult.body.workspaces).toEqual([createdFooWorkspace.id]); + await deleteItem({ + type: dashboard.type, + id: createResult.body.id, + }); + }); + + it('bulk create', async () => { + await clearFooAndBar(); + const createResultFoo = await osdTestServer.request + .post(root, `/w/${createdFooWorkspace.id}/api/saved_objects/_bulk_create`) + .send([ + { + ...dashboard, + id: 'foo', + }, + ]) + .expect(200); + + expect((createResultFoo.body.saved_objects as any[]).some((item) => item.error)).toEqual( + false + ); + expect( + (createResultFoo.body.saved_objects as any[]).every((item) => + isEqual(item.workspaces, [createdFooWorkspace.id]) + ) + ).toEqual(true); + await Promise.all( + [...createResultFoo.body.saved_objects].map((item) => + deleteItem({ + type: item.type, + id: item.id, + }) + ) + ); + }); + + it('checkConflicts when importing ndjson', async () => { + await clearFooAndBar(); + const createResultFoo = await osdTestServer.request + .post(root, `/w/${createdFooWorkspace.id}/api/saved_objects/_bulk_create`) + .send([ + { + ...dashboard, + id: 'foo', + }, + ]) + .expect(200); + + const createResultBar = await osdTestServer.request + .post(root, `/w/${createdBarWorkspace.id}/api/saved_objects/_bulk_create`) + .send([ + { + ...dashboard, + id: 'bar', + }, + ]) + .expect(200); + + const getResultFoo = await getItem({ + type: dashboard.type, + id: 'foo', + }); + const getResultBar = await getItem({ + type: dashboard.type, + id: 'bar', + }); + + /** + * import with workspaces when conflicts + */ + const importWithWorkspacesResult = await osdTestServer.request + .post(root, `/w/${createdFooWorkspace.id}/api/saved_objects/_import?overwrite=false`) + .attach( + 'file', + Buffer.from( + [JSON.stringify(getResultFoo.body), JSON.stringify(getResultBar.body)].join('\n'), + 'utf-8' + ), + 'tmp.ndjson' + ) + .expect(200); + + expect(importWithWorkspacesResult.body.success).toEqual(false); + expect(importWithWorkspacesResult.body.errors.length).toEqual(1); + expect(importWithWorkspacesResult.body.errors[0].id).toEqual('foo'); + expect(importWithWorkspacesResult.body.errors[0].error.type).toEqual('conflict'); + + await Promise.all( + [...createResultFoo.body.saved_objects, ...createResultBar.body.saved_objects].map((item) => + deleteItem({ + type: item.type, + id: item.id, + }) + ) + ); + }); + + it('find by workspaces', async () => { + const createResultFoo = await osdTestServer.request + .post(root, `/w/${createdFooWorkspace.id}/api/saved_objects/_bulk_create`) + .send([ + { + ...dashboard, + id: 'foo', + }, + ]) + .expect(200); + + const createResultBar = await osdTestServer.request + .post(root, `/w/${createdBarWorkspace.id}/api/saved_objects/_bulk_create`) + .send([ + { + ...dashboard, + id: 'bar', + }, + ]) + .expect(200); + + const findResult = await osdTestServer.request + .get(root, `/w/${createdBarWorkspace.id}/api/saved_objects/_find?type=${dashboard.type}`) + .expect(200); + + expect(findResult.body.total).toEqual(1); + expect(findResult.body.saved_objects[0].workspaces).toEqual([createdBarWorkspace.id]); + + await Promise.all( + [...createResultFoo.body.saved_objects, ...createResultBar.body.saved_objects].map((item) => + deleteItem({ + type: item.type, + id: item.id, + }) + ) + ); + }); + }); +}); From 298c2cbd100381d681cbd1b9632019233ed58074 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Sun, 24 Mar 2024 22:58:05 +0800 Subject: [PATCH 10/41] feat: declare workspaces as empty array for advanced settings Signed-off-by: SuZhou-Joe --- .../create_or_upgrade_saved_config.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts index 639dd09249ff..10d33713c878 100644 --- a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts +++ b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts @@ -63,7 +63,12 @@ export async function createOrUpgradeSavedConfig( try { // create the new SavedConfig - await savedObjectsClient.create('config', attributes, { id: version }); + await savedObjectsClient.create('config', attributes, { + id: version, + // config is a global object that should not belong to any of the workspaces + // declare it as empty array to prevent it being created as a workspace object + workspaces: [], + }); } catch (error) { if (handleWriteErrors) { if (SavedObjectsErrorHelpers.isConflictError(error)) { From 85c522dfd3f8887e4771fa964ab39f378f114ec9 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 25 Mar 2024 10:41:25 +0800 Subject: [PATCH 11/41] feat: unified workspaces parameters when parsing from router Signed-off-by: SuZhou-Joe --- .../server/saved_objects/import/check_conflicts.ts | 2 +- src/core/server/saved_objects/routes/bulk_create.ts | 2 +- src/core/server/saved_objects/routes/create.ts | 2 +- src/core/server/saved_objects/routes/export.ts | 2 +- src/core/server/saved_objects/routes/find.ts | 2 +- .../saved_objects/service/saved_objects_client.ts | 4 ---- src/core/server/saved_objects/types.ts | 2 +- .../create_or_upgrade_saved_config.test.ts | 2 ++ .../create_or_upgrade_saved_config.ts | 4 ++-- .../workspace_id_consumer_wrapper.test.ts | 4 ++-- .../saved_objects/workspace_id_consumer_wrapper.ts | 10 +++------- 11 files changed, 15 insertions(+), 21 deletions(-) diff --git a/src/core/server/saved_objects/import/check_conflicts.ts b/src/core/server/saved_objects/import/check_conflicts.ts index f36bcf3a8a92..2c35ba147c5d 100644 --- a/src/core/server/saved_objects/import/check_conflicts.ts +++ b/src/core/server/saved_objects/import/check_conflicts.ts @@ -79,7 +79,7 @@ export async function checkConflicts({ }); const checkConflictsResult = await savedObjectsClient.checkConflicts(objectsToCheck, { namespace, - workspaces, + ...(workspaces ? { workspaces } : {}), }); const errorMap = checkConflictsResult.errors.reduce( (acc, { type, id, error }) => acc.set(`${type}:${id}`, error), diff --git a/src/core/server/saved_objects/routes/bulk_create.ts b/src/core/server/saved_objects/routes/bulk_create.ts index 056b1b795550..b8a020b4ea24 100644 --- a/src/core/server/saved_objects/routes/bulk_create.ts +++ b/src/core/server/saved_objects/routes/bulk_create.ts @@ -70,7 +70,7 @@ export const registerBulkCreateRoute = (router: IRouter) => { : undefined; const result = await context.core.savedObjects.client.bulkCreate(req.body, { overwrite, - workspaces, + ...(workspaces ? { workspaces } : {}), }); return res.ok({ body: result }); }) diff --git a/src/core/server/saved_objects/routes/create.ts b/src/core/server/saved_objects/routes/create.ts index 4d22bd244a03..2a7958aa9b78 100644 --- a/src/core/server/saved_objects/routes/create.ts +++ b/src/core/server/saved_objects/routes/create.ts @@ -71,7 +71,7 @@ export const registerCreateRoute = (router: IRouter) => { migrationVersion, references, initialNamespaces, - workspaces, + ...(workspaces ? { workspaces } : {}), }; const result = await context.core.savedObjects.client.create(type, attributes, options); return res.ok({ body: result }); diff --git a/src/core/server/saved_objects/routes/export.ts b/src/core/server/saved_objects/routes/export.ts index 9325b632e40f..4d9d5b7e8ec3 100644 --- a/src/core/server/saved_objects/routes/export.ts +++ b/src/core/server/saved_objects/routes/export.ts @@ -106,7 +106,7 @@ export const registerExportRoute = (router: IRouter, config: SavedObjectConfig) exportSizeLimit: maxImportExportSize, includeReferencesDeep, excludeExportDetails, - workspaces, + ...(workspaces ? { workspaces } : {}), }); const docsToExport: string[] = await createPromiseFromStreams([ diff --git a/src/core/server/saved_objects/routes/find.ts b/src/core/server/saved_objects/routes/find.ts index 36fa7c2cd9f5..42b47cf950fc 100644 --- a/src/core/server/saved_objects/routes/find.ts +++ b/src/core/server/saved_objects/routes/find.ts @@ -85,7 +85,7 @@ export const registerFindRoute = (router: IRouter) => { fields: typeof query.fields === 'string' ? [query.fields] : query.fields, filter: query.filter, namespaces, - workspaces, + ...(workspaces ? { workspaces } : {}), }); return res.ok({ body: result }); diff --git a/src/core/server/saved_objects/service/saved_objects_client.ts b/src/core/server/saved_objects/service/saved_objects_client.ts index e1c3d16a9258..1b268e033d97 100644 --- a/src/core/server/saved_objects/service/saved_objects_client.ts +++ b/src/core/server/saved_objects/service/saved_objects_client.ts @@ -69,10 +69,6 @@ export interface SavedObjectsCreateOptions extends SavedObjectsBaseOptions { * Note: this can only be used for multi-namespace object types. */ initialNamespaces?: string[]; - /** - * workspaces the new created objects belong to - */ - workspaces?: string[]; /** permission control describe by ACL object */ permissions?: Permissions; } diff --git a/src/core/server/saved_objects/types.ts b/src/core/server/saved_objects/types.ts index d21421dbe253..23170e0a1c47 100644 --- a/src/core/server/saved_objects/types.ts +++ b/src/core/server/saved_objects/types.ts @@ -132,7 +132,7 @@ export interface SavedObjectsBaseOptions { /** Specify the namespace for this operation */ namespace?: string; /** Specify the workspaces for this operation */ - workspaces?: string[]; + workspaces?: string[] | null; } /** diff --git a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.ts b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.ts index eb23e78b1756..fc5a55da0091 100644 --- a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.ts +++ b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.ts @@ -98,6 +98,7 @@ describe('uiSettings/createOrUpgradeSavedConfig', function () { }, { id: version, + workspaces: null, } ); }); @@ -133,6 +134,7 @@ describe('uiSettings/createOrUpgradeSavedConfig', function () { }, { id: version, + workspaces: null, } ); }); diff --git a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts index 10d33713c878..6ca3baeb7a9f 100644 --- a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts +++ b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts @@ -66,8 +66,8 @@ export async function createOrUpgradeSavedConfig( await savedObjectsClient.create('config', attributes, { id: version, // config is a global object that should not belong to any of the workspaces - // declare it as empty array to prevent it being created as a workspace object - workspaces: [], + // declare it as null to prevent it being created as a workspace object + workspaces: null, }); } catch (error) { if (handleWriteErrors) { diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts index 8715606da681..ea8445a4ccaf 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts @@ -50,7 +50,7 @@ describe('WorkspaceIdConsumerWrapper', () => { ); }); - it(`Should use options.workspaces there is workspaces inside options`, async () => { + it(`Should use options.workspaces when there is no workspaces inside options`, async () => { await wrapperClient.create( 'dashboard', { @@ -59,7 +59,7 @@ describe('WorkspaceIdConsumerWrapper', () => { { id: 'dashboard:foo', overwrite: true, - workspaces: undefined, + workspaces: null, } ); diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts index d24496ed31ef..bcf6fe4d5e8c 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts @@ -14,11 +14,7 @@ import { SavedObjectsFindOptions, } from '../../../../core/server'; -type WorkspaceOptions = - | { - workspaces?: string[]; - } - | undefined; +type WorkspaceOptions = Pick | undefined; export class WorkspaceIdConsumerWrapper { private formatWorkspaceIdParams( @@ -30,8 +26,8 @@ export class WorkspaceIdConsumerWrapper { const workspaceIdParsedFromRequest = workspaceState?.id; const workspaceIdsInUserOptions = options?.workspaces; let finalWorkspaces: string[] = []; - if (workspaceIdsInUserOptions) { - finalWorkspaces = workspaceIdsInUserOptions; + if (options?.hasOwnProperty('workspaces')) { + finalWorkspaces = workspaceIdsInUserOptions || []; } else if (workspaceIdParsedFromRequest) { finalWorkspaces = [workspaceIdParsedFromRequest]; } From cdbe9a83a5c87f4dce96e1550da94738dd86e3ac Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 25 Mar 2024 11:30:12 +0800 Subject: [PATCH 12/41] feat: improve code coverage Signed-off-by: SuZhou-Joe --- .../server/saved_objects/workspace_id_consumer_wrapper.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts index ea8445a4ccaf..d4b79868d0e5 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts @@ -93,7 +93,7 @@ describe('WorkspaceIdConsumerWrapper', () => { }); it(`Should add workspaces parameters when checkConflict`, async () => { - await wrapperClient.checkConflicts([]); + await wrapperClient.checkConflicts(); expect(mockedClient.checkConflicts).toBeCalledWith([], { workspaces: ['foo'], }); From cb7ebd96d134b6b505f544757d6535dbb2776326 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 27 Mar 2024 11:04:07 +0800 Subject: [PATCH 13/41] feat: declare workspaces as null Signed-off-by: SuZhou-Joe --- src/core/server/saved_objects/types.ts | 2 +- .../saved_objects/workspace_saved_objects_client_wrapper.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/core/server/saved_objects/types.ts b/src/core/server/saved_objects/types.ts index 23170e0a1c47..0b31baf5c111 100644 --- a/src/core/server/saved_objects/types.ts +++ b/src/core/server/saved_objects/types.ts @@ -112,7 +112,7 @@ export interface SavedObjectsFindOptions { /** An optional OpenSearch preference value to be used for the query **/ preference?: string; /** If specified, will only retrieve objects that are in the workspaces */ - workspaces?: string[]; + workspaces?: string[] | null; /** By default the operator will be 'AND' */ workspacesSearchOperator?: 'AND' | 'OR'; /** diff --git a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts index 1a97b2dac69a..48160c31af63 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts @@ -468,9 +468,9 @@ export class WorkspaceSavedObjectsClientWrapper { [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.LibraryWrite] ), }, - // By declaring workspaces as empty array, + // By declaring workspaces as null, // workspaces won't be appended automatically into the options. - workspaces: [], + workspaces: null, }) ).saved_objects.map((item) => item.id); From 294d2c557dd5d36e78455af3f3a7d2a426299584 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 27 Mar 2024 11:31:05 +0800 Subject: [PATCH 14/41] feat: use unified types Signed-off-by: SuZhou-Joe --- src/core/public/saved_objects/saved_objects_client.ts | 4 ++-- .../saved_objects/export/get_sorted_objects_for_export.ts | 4 ++-- src/core/server/saved_objects/import/check_conflicts.ts | 2 +- .../server/saved_objects/import/create_saved_objects.ts | 2 +- src/core/server/saved_objects/import/types.ts | 4 ++-- src/core/server/saved_objects/serialization/types.ts | 6 +++--- .../saved_objects/service/lib/search_dsl/query_params.ts | 2 +- .../saved_objects/service/lib/search_dsl/search_dsl.ts | 2 +- .../server/saved_objects/service/saved_objects_client.ts | 2 +- src/core/server/saved_objects/types.ts | 4 ++-- src/core/types/saved_objects.ts | 2 +- src/plugins/workspace/server/permission_control/client.ts | 2 +- 12 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/core/public/saved_objects/saved_objects_client.ts b/src/core/public/saved_objects/saved_objects_client.ts index f415e9c58596..ce3ebc710ed1 100644 --- a/src/core/public/saved_objects/saved_objects_client.ts +++ b/src/core/public/saved_objects/saved_objects_client.ts @@ -65,7 +65,7 @@ export interface SavedObjectsCreateOptions { /** {@inheritDoc SavedObjectsMigrationVersion} */ migrationVersion?: SavedObjectsMigrationVersion; references?: SavedObjectReference[]; - workspaces?: string[]; + workspaces?: SavedObject['workspaces']; } /** @@ -83,7 +83,7 @@ export interface SavedObjectsBulkCreateObject extends SavedObjectsC export interface SavedObjectsBulkCreateOptions { /** If a document with the given `id` already exists, overwrite it's contents (default=false). */ overwrite?: boolean; - workspaces?: string[]; + workspaces?: SavedObjectsCreateOptions['workspaces']; } /** @public */ diff --git a/src/core/server/saved_objects/export/get_sorted_objects_for_export.ts b/src/core/server/saved_objects/export/get_sorted_objects_for_export.ts index 0336fc702973..6c99db6c24af 100644 --- a/src/core/server/saved_objects/export/get_sorted_objects_for_export.ts +++ b/src/core/server/saved_objects/export/get_sorted_objects_for_export.ts @@ -61,7 +61,7 @@ export interface SavedObjectsExportOptions { /** optional namespace to override the namespace used by the savedObjectsClient. */ namespace?: string; /** optional workspaces to override the workspaces used by the savedObjectsClient. */ - workspaces?: string[]; + workspaces?: SavedObject['workspaces']; } /** @@ -97,7 +97,7 @@ async function fetchObjectsToExport({ exportSizeLimit: number; savedObjectsClient: SavedObjectsClientContract; namespace?: string; - workspaces?: string[]; + workspaces?: SavedObjectsExportOptions['workspaces']; }) { if ((types?.length ?? 0) > 0 && (objects?.length ?? 0) > 0) { throw Boom.badRequest(`Can't specify both "types" and "objects" properties when exporting`); diff --git a/src/core/server/saved_objects/import/check_conflicts.ts b/src/core/server/saved_objects/import/check_conflicts.ts index 2c35ba147c5d..d173af45f761 100644 --- a/src/core/server/saved_objects/import/check_conflicts.ts +++ b/src/core/server/saved_objects/import/check_conflicts.ts @@ -44,7 +44,7 @@ interface CheckConflictsParams { ignoreRegularConflicts?: boolean; retries?: SavedObjectsImportRetry[]; createNewCopies?: boolean; - workspaces?: string[]; + workspaces?: SavedObject['workspaces']; } const isUnresolvableConflict = (error: SavedObjectError) => diff --git a/src/core/server/saved_objects/import/create_saved_objects.ts b/src/core/server/saved_objects/import/create_saved_objects.ts index 6b0015851baf..f98e2e816b75 100644 --- a/src/core/server/saved_objects/import/create_saved_objects.ts +++ b/src/core/server/saved_objects/import/create_saved_objects.ts @@ -41,7 +41,7 @@ interface CreateSavedObjectsParams { overwrite?: boolean; dataSourceId?: string; dataSourceTitle?: string; - workspaces?: string[]; + workspaces?: SavedObject['workspaces']; } interface CreateSavedObjectsResult { createdObjects: Array>; diff --git a/src/core/server/saved_objects/import/types.ts b/src/core/server/saved_objects/import/types.ts index 994b7e627189..689b8222b510 100644 --- a/src/core/server/saved_objects/import/types.ts +++ b/src/core/server/saved_objects/import/types.ts @@ -190,7 +190,7 @@ export interface SavedObjectsImportOptions { dataSourceId?: string; dataSourceTitle?: string; /** if specified, will import in given workspaces */ - workspaces?: string[]; + workspaces?: SavedObject['workspaces']; } /** @@ -215,7 +215,7 @@ export interface SavedObjectsResolveImportErrorsOptions { dataSourceId?: string; dataSourceTitle?: string; /** if specified, will import in given workspaces */ - workspaces?: string[]; + workspaces?: SavedObject['workspaces']; } export type CreatedObject = SavedObject & { destinationId?: string }; diff --git a/src/core/server/saved_objects/serialization/types.ts b/src/core/server/saved_objects/serialization/types.ts index f882596ce529..6b1f025e856d 100644 --- a/src/core/server/saved_objects/serialization/types.ts +++ b/src/core/server/saved_objects/serialization/types.ts @@ -29,7 +29,7 @@ */ import { Permissions } from '../permission_control'; -import { SavedObjectsMigrationVersion, SavedObjectReference } from '../types'; +import { SavedObjectsMigrationVersion, SavedObjectReference, SavedObject } from '../types'; /** * A raw document as represented directly in the saved object index. @@ -53,7 +53,7 @@ export interface SavedObjectsRawDocSource { updated_at?: string; references?: SavedObjectReference[]; originId?: string; - workspaces?: string[]; + workspaces?: SavedObject['workspaces']; [typeMapping: string]: any; } @@ -71,7 +71,7 @@ interface SavedObjectDoc { version?: string; updated_at?: string; originId?: string; - workspaces?: string[]; + workspaces?: SavedObject['workspaces']; permissions?: Permissions; } diff --git a/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts b/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts index abbef0850dba..318768fd83c2 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts @@ -167,7 +167,7 @@ interface QueryParams { defaultSearchOperator?: string; hasReference?: HasReferenceQueryParams; kueryNode?: KueryNode; - workspaces?: string[]; + workspaces?: SavedObjectsFindOptions['workspaces']; workspacesSearchOperator?: 'AND' | 'OR'; ACLSearchParams?: SavedObjectsFindOptions['ACLSearchParams']; } diff --git a/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts b/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts index fa4311576638..626fc7efba3e 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts @@ -53,7 +53,7 @@ interface GetSearchDslOptions { id: string; }; kueryNode?: KueryNode; - workspaces?: string[]; + workspaces?: SavedObjectsFindOptions['workspaces']; workspacesSearchOperator?: 'AND' | 'OR'; ACLSearchParams?: SavedObjectsFindOptions['ACLSearchParams']; } diff --git a/src/core/server/saved_objects/service/saved_objects_client.ts b/src/core/server/saved_objects/service/saved_objects_client.ts index 1b268e033d97..be02ef247d04 100644 --- a/src/core/server/saved_objects/service/saved_objects_client.ts +++ b/src/core/server/saved_objects/service/saved_objects_client.ts @@ -97,7 +97,7 @@ export interface SavedObjectsBulkCreateObject { /** * workspaces the objects belong to, will only be used when overwrite is enabled. */ - workspaces?: string[]; + workspaces?: SavedObject['workspaces']; } /** diff --git a/src/core/server/saved_objects/types.ts b/src/core/server/saved_objects/types.ts index 0b31baf5c111..22093b90d6b3 100644 --- a/src/core/server/saved_objects/types.ts +++ b/src/core/server/saved_objects/types.ts @@ -112,7 +112,7 @@ export interface SavedObjectsFindOptions { /** An optional OpenSearch preference value to be used for the query **/ preference?: string; /** If specified, will only retrieve objects that are in the workspaces */ - workspaces?: string[] | null; + workspaces?: SavedObjectsBaseOptions['workspaces']; /** By default the operator will be 'AND' */ workspacesSearchOperator?: 'AND' | 'OR'; /** @@ -132,7 +132,7 @@ export interface SavedObjectsBaseOptions { /** Specify the namespace for this operation */ namespace?: string; /** Specify the workspaces for this operation */ - workspaces?: string[] | null; + workspaces?: SavedObject['workspaces']; } /** diff --git a/src/core/types/saved_objects.ts b/src/core/types/saved_objects.ts index b37309338c9e..e91a5012098e 100644 --- a/src/core/types/saved_objects.ts +++ b/src/core/types/saved_objects.ts @@ -115,7 +115,7 @@ export interface SavedObject { */ originId?: string; /** Workspaces that this saved object exists in. */ - workspaces?: string[]; + workspaces?: string[] | null; /** Permissions that this saved objects exists in. */ permissions?: Permissions; } diff --git a/src/plugins/workspace/server/permission_control/client.ts b/src/plugins/workspace/server/permission_control/client.ts index bad46eb156a6..03a8fb0aaf3d 100644 --- a/src/plugins/workspace/server/permission_control/client.ts +++ b/src/plugins/workspace/server/permission_control/client.ts @@ -147,7 +147,7 @@ export class SavedObjectsPermissionControl { const principals = this.getPrincipalsFromRequest(request); const deniedObjects: Array< Pick & { - workspaces?: string[]; + workspaces?: SavedObject['workspaces']; permissions?: Permissions; } > = []; From 42e0e051896c900ea9cc573c71b66f7a173c7e17 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 28 Mar 2024 10:46:18 +0800 Subject: [PATCH 15/41] feat: update comment Signed-off-by: SuZhou-Joe --- .../saved_objects/workspace_saved_objects_client_wrapper.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts index 48160c31af63..d5497bc6475c 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts @@ -470,6 +470,7 @@ export class WorkspaceSavedObjectsClientWrapper { }, // By declaring workspaces as null, // workspaces won't be appended automatically into the options. + // or workspaces can not be found because workspace object do not have `workspaces` field. workspaces: null, }) ).saved_objects.map((item) => item.id); From 1b323162ecb8e844f2f6a345c5e507c0d71491d6 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 1 Apr 2024 17:13:00 +0800 Subject: [PATCH 16/41] feat: remove null Signed-off-by: SuZhou-Joe --- .../create_or_upgrade_saved_config.test.ts | 2 -- .../create_or_upgrade_saved_config.ts | 3 --- 2 files changed, 5 deletions(-) diff --git a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.ts b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.ts index fc5a55da0091..eb23e78b1756 100644 --- a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.ts +++ b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.ts @@ -98,7 +98,6 @@ describe('uiSettings/createOrUpgradeSavedConfig', function () { }, { id: version, - workspaces: null, } ); }); @@ -134,7 +133,6 @@ describe('uiSettings/createOrUpgradeSavedConfig', function () { }, { id: version, - workspaces: null, } ); }); diff --git a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts index 6ca3baeb7a9f..6649966d8952 100644 --- a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts +++ b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts @@ -65,9 +65,6 @@ export async function createOrUpgradeSavedConfig( // create the new SavedConfig await savedObjectsClient.create('config', attributes, { id: version, - // config is a global object that should not belong to any of the workspaces - // declare it as null to prevent it being created as a workspace object - workspaces: null, }); } catch (error) { if (handleWriteErrors) { From b7a6cb7bec00944e89a9e51c25543f4be29f5ea6 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 2 Apr 2024 15:37:50 +0800 Subject: [PATCH 17/41] feat: address comments Signed-off-by: SuZhou-Joe --- .../create_or_upgrade_saved_config.ts | 4 +--- .../saved_objects/workspace_id_consumer_wrapper.test.ts | 2 +- .../server/saved_objects/workspace_id_consumer_wrapper.ts | 1 + 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts index 6649966d8952..639dd09249ff 100644 --- a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts +++ b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts @@ -63,9 +63,7 @@ export async function createOrUpgradeSavedConfig( try { // create the new SavedConfig - await savedObjectsClient.create('config', attributes, { - id: version, - }); + await savedObjectsClient.create('config', attributes, { id: version }); } catch (error) { if (handleWriteErrors) { if (SavedObjectsErrorHelpers.isConflictError(error)) { diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts index d4b79868d0e5..21ef63c04867 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts @@ -50,7 +50,7 @@ describe('WorkspaceIdConsumerWrapper', () => { ); }); - it(`Should use options.workspaces when there is no workspaces inside options`, async () => { + it(`Should not use options.workspaces when there is no workspaces inside options`, async () => { await wrapperClient.create( 'dashboard', { diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts index bcf6fe4d5e8c..8ebd6b301563 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts @@ -71,6 +71,7 @@ export class WorkspaceIdConsumerWrapper { bulkUpdate: wrapperOptions.client.bulkUpdate, addToNamespaces: wrapperOptions.client.addToNamespaces, deleteFromNamespaces: wrapperOptions.client.deleteFromNamespaces, + deleteByWorkspace: wrapperOptions.client.deleteByWorkspace, }; }; From dd6cebfca07f3f216578a407fef8cec279927337 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 3 Apr 2024 09:34:21 +0800 Subject: [PATCH 18/41] feat: use request app to store request workspace id Signed-off-by: SuZhou-Joe --- src/core/server/utils/workspace.test.ts | 4 ++-- src/core/server/utils/workspace.ts | 31 +++++++++++-------------- src/legacy/server/osd_server.d.ts | 8 ------- 3 files changed, 16 insertions(+), 27 deletions(-) diff --git a/src/core/server/utils/workspace.test.ts b/src/core/server/utils/workspace.test.ts index 49382cfac38f..7dfcff9e5d18 100644 --- a/src/core/server/utils/workspace.test.ts +++ b/src/core/server/utils/workspace.test.ts @@ -10,10 +10,10 @@ describe('updateWorkspaceState', () => { it('update with payload', () => { const requestMock = httpServerMock.createOpenSearchDashboardsRequest(); updateWorkspaceState(requestMock, { - id: 'foo', + requestWorkspaceId: 'foo', }); expect(getWorkspaceState(requestMock)).toEqual({ - id: 'foo', + requestWorkspaceId: 'foo', }); }); }); diff --git a/src/core/server/utils/workspace.ts b/src/core/server/utils/workspace.ts index c5dcf84b92d9..831755c414a5 100644 --- a/src/core/server/utils/workspace.ts +++ b/src/core/server/utils/workspace.ts @@ -3,14 +3,12 @@ * SPDX-License-Identifier: Apache-2.0 */ -/** - * This file is using {@link PluginsStates} to store workspace info into request. - * The best practice would be using {@link Server.register} to register plugins into the hapi server - * but OSD is wrappering the hapi server and the hapi server instance is hidden as internal implementation. - */ -import { PluginsStates } from '@hapi/hapi'; import { OpenSearchDashboardsRequest, ensureRawRequest } from '../http/router'; +export interface WorkspaceState { + requestWorkspaceId?: string; +} + /** * This function will be used as a proxy * because `ensureRequest` is only importable from core module. @@ -20,24 +18,23 @@ import { OpenSearchDashboardsRequest, ensureRawRequest } from '../http/router'; */ export const updateWorkspaceState = ( request: OpenSearchDashboardsRequest, - payload: Partial + payload: Partial ) => { const rawRequest = ensureRawRequest(request); - if (!rawRequest.plugins) { - rawRequest.plugins = {}; + if (!rawRequest.app) { + rawRequest.app = {}; } - if (!rawRequest.plugins.workspace) { - rawRequest.plugins.workspace = {}; - } - - rawRequest.plugins.workspace = { - ...rawRequest.plugins.workspace, + rawRequest.app = { + ...rawRequest.app, ...payload, }; }; -export const getWorkspaceState = (request: OpenSearchDashboardsRequest) => { - return ensureRawRequest(request).plugins?.workspace; +export const getWorkspaceState = (request: OpenSearchDashboardsRequest): WorkspaceState => { + const { requestWorkspaceId } = ensureRawRequest(request).app as WorkspaceState; + return { + requestWorkspaceId, + }; }; diff --git a/src/legacy/server/osd_server.d.ts b/src/legacy/server/osd_server.d.ts index 9d94349cf1c0..1f8535b59e87 100644 --- a/src/legacy/server/osd_server.d.ts +++ b/src/legacy/server/osd_server.d.ts @@ -60,14 +60,6 @@ declare module 'hapi' { } } -declare module '@hapi/hapi' { - interface PluginsStates { - workspace?: { - id?: string; - }; - } -} - type OsdMixinFunc = (osdServer: OsdServer, server: Server, config: any) => Promise | void; export interface PluginsSetup { From 7fd5160d3275661fc29ba48858e3ba1349ef75b6 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 3 Apr 2024 09:51:20 +0800 Subject: [PATCH 19/41] feat: use app state to store request workspace id Signed-off-by: SuZhou-Joe --- src/plugins/workspace/server/plugin.ts | 2 +- .../server/saved_objects/workspace_id_consumer_wrapper.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index 088096df4d35..e7029f8387f6 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -54,7 +54,7 @@ export class WorkspacePlugin implements Plugin<{}, {}> { if (workspaceId) { updateWorkspaceState(request, { - id: workspaceId, + requestWorkspaceId: workspaceId, }); const requestUrl = new URL(request.url.toString()); requestUrl.pathname = cleanWorkspaceId(requestUrl.pathname); diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts index 8ebd6b301563..74e8e99af71e 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts @@ -23,7 +23,7 @@ export class WorkspaceIdConsumerWrapper { ): T { const { workspaces, ...others } = options || {}; const workspaceState = getWorkspaceState(request); - const workspaceIdParsedFromRequest = workspaceState?.id; + const workspaceIdParsedFromRequest = workspaceState?.requestWorkspaceId; const workspaceIdsInUserOptions = options?.workspaces; let finalWorkspaces: string[] = []; if (options?.hasOwnProperty('workspaces')) { From 9d725240d536bb7e27243b0893ab9a22de523003 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 3 Apr 2024 13:53:04 +0800 Subject: [PATCH 20/41] feat: remove workspaces when listing data sources Signed-off-by: SuZhou-Joe --- .../workspace_id_consumer_wrapper.test.ts | 12 ++++++++++- .../workspace_id_consumer_wrapper.ts | 21 ++++++++++++++++++- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts index 21ef63c04867..b8d1b93f2686 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts @@ -7,6 +7,7 @@ import { updateWorkspaceState } from '../../../../core/server/utils'; import { SavedObject } from '../../../../core/public'; import { httpServerMock, savedObjectsClientMock, coreMock } from '../../../../core/server/mocks'; import { WorkspaceIdConsumerWrapper } from './workspace_id_consumer_wrapper'; +import { DATA_SOURCE_SAVED_OBJECT_TYPE } from '../../../../plugins/data_source/common'; describe('WorkspaceIdConsumerWrapper', () => { const requestHandlerContext = coreMock.createRequestHandlerContext(); @@ -14,7 +15,7 @@ describe('WorkspaceIdConsumerWrapper', () => { const mockedClient = savedObjectsClientMock.create(); const workspaceEnabledMockRequest = httpServerMock.createOpenSearchDashboardsRequest(); updateWorkspaceState(workspaceEnabledMockRequest, { - id: 'foo', + requestWorkspaceId: 'foo', }); const wrapperClient = wrapperInstance.wrapperFactory({ client: mockedClient, @@ -114,5 +115,14 @@ describe('WorkspaceIdConsumerWrapper', () => { workspaces: ['foo'], }); }); + + it(`workspaces parameters should be removed when finding data sources`, async () => { + await wrapperClient.find({ + type: DATA_SOURCE_SAVED_OBJECT_TYPE, + }); + expect(mockedClient.find).toBeCalledWith({ + type: DATA_SOURCE_SAVED_OBJECT_TYPE, + }); + }); }); }); diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts index 74e8e99af71e..9e94e8945d62 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts @@ -13,6 +13,7 @@ import { OpenSearchDashboardsRequest, SavedObjectsFindOptions, } from '../../../../core/server'; +import { DATA_SOURCE_SAVED_OBJECT_TYPE } from '../../../../plugins/data_source/common'; type WorkspaceOptions = Pick | undefined; @@ -37,6 +38,17 @@ export class WorkspaceIdConsumerWrapper { ...(finalWorkspaces.length ? { workspaces: finalWorkspaces } : {}), }; } + private isDataSourceType(type: SavedObjectsFindOptions['type']): boolean { + if (Array.isArray(type)) { + return type.every((item) => item === DATA_SOURCE_SAVED_OBJECT_TYPE); + } + + return type === DATA_SOURCE_SAVED_OBJECT_TYPE; + } + private formatFindParams(options: SavedObjectsFindOptions): SavedObjectsFindOptions { + const isListingDataSource = this.isDataSourceType(options.type); + return isListingDataSource ? { ...options, workspaces: null } : options; + } public wrapperFactory: SavedObjectsClientWrapperFactory = (wrapperOptions) => { return { ...wrapperOptions.client, @@ -64,7 +76,14 @@ export class WorkspaceIdConsumerWrapper { ), delete: wrapperOptions.client.delete, find: (options: SavedObjectsFindOptions) => - wrapperOptions.client.find(this.formatWorkspaceIdParams(wrapperOptions.request, options)), + wrapperOptions.client.find( + this.formatWorkspaceIdParams( + wrapperOptions.request, + // The `formatFindParams` is a workaroud for 2.14 to always list global data sources, + // should remove this workaround in 2.15 once readonly share is available + this.formatFindParams(options) + ) + ), bulkGet: wrapperOptions.client.bulkGet, get: wrapperOptions.client.get, update: wrapperOptions.client.update, From f65ffcfbe67f90832f47672319529531a860804a Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 10 Apr 2024 10:42:03 +0800 Subject: [PATCH 21/41] feat: remove useless code change Signed-off-by: SuZhou-Joe --- src/core/types/saved_objects.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/types/saved_objects.ts b/src/core/types/saved_objects.ts index 65ff595fa200..74890bb624a3 100644 --- a/src/core/types/saved_objects.ts +++ b/src/core/types/saved_objects.ts @@ -115,7 +115,7 @@ export interface SavedObject { */ originId?: string; /** Workspaces that this saved object exists in. */ - workspaces?: string[] | null; + workspaces?: string[]; /** Permissions that this saved objects exists in. */ permissions?: Permissions; } From 57ddd07e0d98754413f190e81e8978750b6b6cfa Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 10 Apr 2024 15:45:54 +0800 Subject: [PATCH 22/41] feat: throw error if the type is not allowed Signed-off-by: SuZhou-Joe --- src/core/server/index.ts | 1 + src/core/server/ui_settings/index.ts | 2 + .../ui_settings/saved_objects/ui_settings.ts | 4 +- .../server/ui_settings/ui_settings_service.ts | 3 +- .../workspace_id_consumer_wrapper.ts | 42 ++++++++++++++++--- 5 files changed, 45 insertions(+), 7 deletions(-) diff --git a/src/core/server/index.ts b/src/core/server/index.ts index f497bed22755..b0e1179fbc3b 100644 --- a/src/core/server/index.ts +++ b/src/core/server/index.ts @@ -341,6 +341,7 @@ export { StringValidation, StringValidationRegex, StringValidationRegexString, + UI_SETTINGS_SAVED_OBJECTS_TYPE, } from './ui_settings'; export { diff --git a/src/core/server/ui_settings/index.ts b/src/core/server/ui_settings/index.ts index 7912c0af84af..3738afef95b1 100644 --- a/src/core/server/ui_settings/index.ts +++ b/src/core/server/ui_settings/index.ts @@ -49,3 +49,5 @@ export { StringValidationRegex, StringValidationRegexString, } from './types'; + +export { UI_SETTINGS_SAVED_OBJECTS_TYPE } from './saved_objects/ui_settings'; diff --git a/src/core/server/ui_settings/saved_objects/ui_settings.ts b/src/core/server/ui_settings/saved_objects/ui_settings.ts index a56b12ed2063..968c3ceb5691 100644 --- a/src/core/server/ui_settings/saved_objects/ui_settings.ts +++ b/src/core/server/ui_settings/saved_objects/ui_settings.ts @@ -31,8 +31,10 @@ import { SavedObjectsType } from '../../saved_objects'; import { migrations } from './migrations'; +export const UI_SETTINGS_SAVED_OBJECTS_TYPE = 'config'; + export const uiSettingsType: SavedObjectsType = { - name: 'config', + name: UI_SETTINGS_SAVED_OBJECTS_TYPE, hidden: false, namespaceType: 'single', mappings: { diff --git a/src/core/server/ui_settings/ui_settings_service.ts b/src/core/server/ui_settings/ui_settings_service.ts index d20b1e964e5b..514cdca96b36 100644 --- a/src/core/server/ui_settings/ui_settings_service.ts +++ b/src/core/server/ui_settings/ui_settings_service.ts @@ -48,6 +48,7 @@ import { import { uiSettingsType } from './saved_objects'; import { registerRoutes } from './routes'; import { getCoreSettings } from './settings'; +import { UI_SETTINGS_SAVED_OBJECTS_TYPE } from './saved_objects/ui_settings'; export interface SetupDeps { http: InternalHttpServiceSetup; @@ -102,7 +103,7 @@ export class UiSettingsService const { version, buildNum } = this.coreContext.env.packageInfo; return (savedObjectsClient: SavedObjectsClientContract) => new UiSettingsClient({ - type: 'config', + type: UI_SETTINGS_SAVED_OBJECTS_TYPE, id: version, buildNum, savedObjectsClient, diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts index 9e94e8945d62..72bd6305b61d 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts @@ -12,6 +12,8 @@ import { SavedObjectsCheckConflictsObject, OpenSearchDashboardsRequest, SavedObjectsFindOptions, + UI_SETTINGS_SAVED_OBJECTS_TYPE, + SavedObjectsErrorHelpers, } from '../../../../core/server'; import { DATA_SOURCE_SAVED_OBJECT_TYPE } from '../../../../plugins/data_source/common'; @@ -45,6 +47,13 @@ export class WorkspaceIdConsumerWrapper { return type === DATA_SOURCE_SAVED_OBJECT_TYPE; } + private isConfigType(type: SavedObjectsFindOptions['type']): boolean { + if (Array.isArray(type)) { + return type.every((item) => item === UI_SETTINGS_SAVED_OBJECTS_TYPE); + } + + return type === UI_SETTINGS_SAVED_OBJECTS_TYPE; + } private formatFindParams(options: SavedObjectsFindOptions): SavedObjectsFindOptions { const isListingDataSource = this.isDataSourceType(options.type); return isListingDataSource ? { ...options, workspaces: null } : options; @@ -61,11 +70,34 @@ export class WorkspaceIdConsumerWrapper { bulkCreate: ( objects: Array>, options: SavedObjectsCreateOptions = {} - ) => - wrapperOptions.client.bulkCreate( - objects, - this.formatWorkspaceIdParams(wrapperOptions.request, options) - ), + ) => { + const { workspaces } = this.formatWorkspaceIdParams(wrapperOptions.request, options); + const disallowedSavedObjects = objects.filter((item) => { + // If create out of workspace, allow the operation + if (!workspaces?.length && !item.workspaces?.length) { + return false; + } + + // config and data-sources can not be created inside a workspace + return this.isConfigType(item.type) || this.isDataSourceType(item.type); + }); + + if (!disallowedSavedObjects?.length) { + return wrapperOptions.client.bulkCreate( + objects, + this.formatWorkspaceIdParams(wrapperOptions.request, options) + ); + } + + const disallowedTypes = [...new Set(disallowedSavedObjects.map((item) => item.type))]; + + throw SavedObjectsErrorHelpers.decorateBadRequestError( + new Error(''), + `${disallowedTypes.map((item) => `type: ${item}`).join(', ')} ${ + disallowedTypes.length > 1 ? 'are' : 'is' + } not allowed to create within a workspace.` + ); + }, checkConflicts: ( objects: SavedObjectsCheckConflictsObject[] = [], options: SavedObjectsBaseOptions = {} From d5b86768cac5691c433a3d065c14ff867351dfdb Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 10 Apr 2024 16:29:27 +0800 Subject: [PATCH 23/41] feat: add unit test Signed-off-by: SuZhou-Joe --- .../workspace_id_consumer_wrapper.test.ts | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts index b8d1b93f2686..28c9aad535af 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts @@ -8,6 +8,7 @@ import { SavedObject } from '../../../../core/public'; import { httpServerMock, savedObjectsClientMock, coreMock } from '../../../../core/server/mocks'; import { WorkspaceIdConsumerWrapper } from './workspace_id_consumer_wrapper'; import { DATA_SOURCE_SAVED_OBJECT_TYPE } from '../../../../plugins/data_source/common'; +import { UI_SETTINGS_SAVED_OBJECTS_TYPE } from '../../../../core/server'; describe('WorkspaceIdConsumerWrapper', () => { const requestHandlerContext = coreMock.createRequestHandlerContext(); @@ -86,6 +87,30 @@ describe('WorkspaceIdConsumerWrapper', () => { } ); }); + + it(`Should throw error when trying to create unallowed type within a workspace`, async () => { + expect(() => + wrapperClient.bulkCreate([ + getSavedObject({ + type: DATA_SOURCE_SAVED_OBJECT_TYPE, + id: 'foo', + }), + ]) + ).toThrow('type: data-source is not allowed to create within a workspace.'); + + expect(() => + wrapperClient.bulkCreate([ + getSavedObject({ + type: DATA_SOURCE_SAVED_OBJECT_TYPE, + id: 'foo', + }), + getSavedObject({ + type: UI_SETTINGS_SAVED_OBJECTS_TYPE, + id: 'bar', + }), + ]) + ).toThrow('type: data-source, type: config are not allowed to create within a workspace.'); + }); }); describe('checkConflict', () => { From 395f6b0ca98a1c75c454da14921bcbd7f8de2246 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 10 Apr 2024 16:48:46 +0800 Subject: [PATCH 24/41] feat: add integration test Signed-off-by: SuZhou-Joe --- .../workspace_id_consumer_wrapper.test.ts | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts index 381da5227d8d..5ca60b2a3004 100644 --- a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts @@ -6,6 +6,8 @@ import { SavedObject } from 'src/core/types'; import { isEqual } from 'lodash'; import * as osdTestServer from '../../../../../core/test_helpers/osd_server'; +import { DATA_SOURCE_SAVED_OBJECT_TYPE } from '../../../../data_source/common'; +import { UI_SETTINGS_SAVED_OBJECTS_TYPE } from '../../../../../core/server'; const dashboard: Omit = { type: 'dashboard', @@ -13,6 +15,18 @@ const dashboard: Omit = { references: [], }; +const dataSource: Omit = { + type: DATA_SOURCE_SAVED_OBJECT_TYPE, + attributes: {}, + references: [], +}; + +const advancedSettings: Omit = { + type: UI_SETTINGS_SAVED_OBJECTS_TYPE, + attributes: {}, + references: [], +}; + interface WorkspaceAttributes { id: string; name?: string; @@ -31,7 +45,14 @@ describe('workspace_id_consumer integration test', () => { const { startOpenSearch, startOpenSearchDashboards } = osdTestServer.createTestServers({ adjustTimeout: (t: number) => jest.setTimeout(t), settings: { + opensearch: { + license: 'oss', + opensearchFrom: '/Users/suzhou/Downloads/opensearch-3.0.0-SNAPSHOT', + }, osd: { + data_source: { + enabled: true, + }, workspace: { enabled: true, }, @@ -140,6 +161,47 @@ describe('workspace_id_consumer integration test', () => { ); }); + it('bulk create with disallowed types', async () => { + await clearFooAndBar(); + + // import advanced settings and data sources should throw error + const createResultFoo = await osdTestServer.request + .post(root, `/w/${createdFooWorkspace.id}/api/saved_objects/_bulk_create`) + .send([ + { + ...dataSource, + id: 'foo', + }, + { + ...advancedSettings, + id: 'bar', + }, + ]) + .expect(400); + expect(createResultFoo.body).toEqual({ + error: 'Bad Request', + message: 'type: data-source, type: config are not allowed to create within a workspace.', + statusCode: 400, + }); + + // data source and advanced settings should not be found within the workspace + const findDataSourceResult = await osdTestServer.request + .get( + root, + `/w/${createdFooWorkspace.id}/api/saved_objects/_find?type=${DATA_SOURCE_SAVED_OBJECT_TYPE}` + ) + .expect(200); + expect(findDataSourceResult.body.total).toEqual(0); + + const findAdvancedSettings = await osdTestServer.request + .get( + root, + `/w/${createdFooWorkspace.id}/api/saved_objects/_find?type=${UI_SETTINGS_SAVED_OBJECTS_TYPE}` + ) + .expect(200); + expect(findAdvancedSettings.body.total).toEqual(0); + }); + it('checkConflicts when importing ndjson', async () => { await clearFooAndBar(); const createResultFoo = await osdTestServer.request From 0fe0d372665501e3a6e5a9e9540b3101e7b8daee Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 15 Apr 2024 14:41:07 +0800 Subject: [PATCH 25/41] feat: change the implementation Signed-off-by: SuZhou-Joe --- src/core/server/index.ts | 1 - src/core/server/ui_settings/index.ts | 2 -- .../ui_settings/saved_objects/ui_settings.ts | 4 +-- ...ts_wrapper_for_check_workspace_conflict.ts | 6 ++-- .../workspace_id_consumer_wrapper.test.ts | 4 +-- .../workspace_id_consumer_wrapper.ts | 36 ++++++++----------- 6 files changed, 21 insertions(+), 32 deletions(-) diff --git a/src/core/server/index.ts b/src/core/server/index.ts index b0e1179fbc3b..f497bed22755 100644 --- a/src/core/server/index.ts +++ b/src/core/server/index.ts @@ -341,7 +341,6 @@ export { StringValidation, StringValidationRegex, StringValidationRegexString, - UI_SETTINGS_SAVED_OBJECTS_TYPE, } from './ui_settings'; export { diff --git a/src/core/server/ui_settings/index.ts b/src/core/server/ui_settings/index.ts index 3738afef95b1..7912c0af84af 100644 --- a/src/core/server/ui_settings/index.ts +++ b/src/core/server/ui_settings/index.ts @@ -49,5 +49,3 @@ export { StringValidationRegex, StringValidationRegexString, } from './types'; - -export { UI_SETTINGS_SAVED_OBJECTS_TYPE } from './saved_objects/ui_settings'; diff --git a/src/core/server/ui_settings/saved_objects/ui_settings.ts b/src/core/server/ui_settings/saved_objects/ui_settings.ts index 968c3ceb5691..a56b12ed2063 100644 --- a/src/core/server/ui_settings/saved_objects/ui_settings.ts +++ b/src/core/server/ui_settings/saved_objects/ui_settings.ts @@ -31,10 +31,8 @@ import { SavedObjectsType } from '../../saved_objects'; import { migrations } from './migrations'; -export const UI_SETTINGS_SAVED_OBJECTS_TYPE = 'config'; - export const uiSettingsType: SavedObjectsType = { - name: UI_SETTINGS_SAVED_OBJECTS_TYPE, + name: 'config', hidden: false, namespaceType: 'single', mappings: { diff --git a/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.ts b/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.ts index 298d0448031a..838b689328bf 100644 --- a/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.ts +++ b/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.ts @@ -20,8 +20,8 @@ import { const errorContent = (error: Boom.Boom) => error.output.payload; const filterWorkspacesAccordingToSourceWorkspaces = ( - targetWorkspaces?: string[], - baseWorkspaces?: string[] + targetWorkspaces?: SavedObjectsBaseOptions['workspaces'], + baseWorkspaces?: SavedObjectsBaseOptions['workspaces'] ): string[] => targetWorkspaces?.filter((item) => !baseWorkspaces?.includes(item)) || []; export class WorkspaceConflictSavedObjectsClientWrapper { @@ -110,7 +110,7 @@ export class WorkspaceConflictSavedObjectsClientWrapper { }) : []; const objectsConflictWithWorkspace: SavedObject[] = []; - const objectsMapWorkspaces: Record = {}; + const objectsMapWorkspaces: Record = {}; if (bulkGetDocs.length) { /** * Get latest status of objects diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts index 28c9aad535af..741d7ef3662e 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts @@ -92,11 +92,11 @@ describe('WorkspaceIdConsumerWrapper', () => { expect(() => wrapperClient.bulkCreate([ getSavedObject({ - type: DATA_SOURCE_SAVED_OBJECT_TYPE, + type: 'config', id: 'foo', }), ]) - ).toThrow('type: data-source is not allowed to create within a workspace.'); + ).toThrow('type: config is not allowed to create within a workspace.'); expect(() => wrapperClient.bulkCreate([ diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts index 72bd6305b61d..1466fec3897c 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts @@ -12,13 +12,13 @@ import { SavedObjectsCheckConflictsObject, OpenSearchDashboardsRequest, SavedObjectsFindOptions, - UI_SETTINGS_SAVED_OBJECTS_TYPE, - SavedObjectsErrorHelpers, } from '../../../../core/server'; import { DATA_SOURCE_SAVED_OBJECT_TYPE } from '../../../../plugins/data_source/common'; type WorkspaceOptions = Pick | undefined; +const UI_SETTINGS_SAVED_OBJECTS_TYPE = 'config'; + export class WorkspaceIdConsumerWrapper { private formatWorkspaceIdParams( request: OpenSearchDashboardsRequest, @@ -72,30 +72,24 @@ export class WorkspaceIdConsumerWrapper { options: SavedObjectsCreateOptions = {} ) => { const { workspaces } = this.formatWorkspaceIdParams(wrapperOptions.request, options); - const disallowedSavedObjects = objects.filter((item) => { - // If create out of workspace, allow the operation - if (!workspaces?.length && !item.workspaces?.length) { + const allowedSavedObjects = objects.filter((item) => { + const isImportIntoWorkspace = workspaces?.length || item.workspaces?.length; + // config can not be created inside a workspace + if (this.isConfigType(item.type) && isImportIntoWorkspace) { return false; } - // config and data-sources can not be created inside a workspace - return this.isConfigType(item.type) || this.isDataSourceType(item.type); - }); - - if (!disallowedSavedObjects?.length) { - return wrapperOptions.client.bulkCreate( - objects, - this.formatWorkspaceIdParams(wrapperOptions.request, options) - ); - } + // For 2.14, data source can only be created without workspace info + if (this.isDataSourceType(item.type) && isImportIntoWorkspace) { + return false; + } - const disallowedTypes = [...new Set(disallowedSavedObjects.map((item) => item.type))]; + return true; + }); - throw SavedObjectsErrorHelpers.decorateBadRequestError( - new Error(''), - `${disallowedTypes.map((item) => `type: ${item}`).join(', ')} ${ - disallowedTypes.length > 1 ? 'are' : 'is' - } not allowed to create within a workspace.` + return wrapperOptions.client.bulkCreate( + allowedSavedObjects, + this.formatWorkspaceIdParams(wrapperOptions.request, options) ); }, checkConflicts: ( From 5f487bcf32c0b32867e5b52a28c46f23b303bd53 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 15 Apr 2024 14:42:40 +0800 Subject: [PATCH 26/41] feat: remove useless change Signed-off-by: SuZhou-Joe --- src/core/server/ui_settings/ui_settings_service.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/core/server/ui_settings/ui_settings_service.ts b/src/core/server/ui_settings/ui_settings_service.ts index 514cdca96b36..d20b1e964e5b 100644 --- a/src/core/server/ui_settings/ui_settings_service.ts +++ b/src/core/server/ui_settings/ui_settings_service.ts @@ -48,7 +48,6 @@ import { import { uiSettingsType } from './saved_objects'; import { registerRoutes } from './routes'; import { getCoreSettings } from './settings'; -import { UI_SETTINGS_SAVED_OBJECTS_TYPE } from './saved_objects/ui_settings'; export interface SetupDeps { http: InternalHttpServiceSetup; @@ -103,7 +102,7 @@ export class UiSettingsService const { version, buildNum } = this.coreContext.env.packageInfo; return (savedObjectsClient: SavedObjectsClientContract) => new UiSettingsClient({ - type: UI_SETTINGS_SAVED_OBJECTS_TYPE, + type: 'config', id: version, buildNum, savedObjectsClient, From 4cfdfb095704478258b72ca07ff8a25a067d447e Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 15 Apr 2024 14:43:58 +0800 Subject: [PATCH 27/41] feat: remove useless change Signed-off-by: SuZhou-Joe --- .../integration_tests/workspace_id_consumer_wrapper.test.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts index ae75c5f86e63..7abbaaac6ae2 100644 --- a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts @@ -7,7 +7,6 @@ import { SavedObject } from 'src/core/types'; import { isEqual } from 'lodash'; import * as osdTestServer from '../../../../../core/test_helpers/osd_server'; import { DATA_SOURCE_SAVED_OBJECT_TYPE } from '../../../../data_source/common'; -import { UI_SETTINGS_SAVED_OBJECTS_TYPE } from '../../../../../core/server'; const dashboard: Omit = { type: 'dashboard', @@ -22,7 +21,7 @@ const dataSource: Omit = { }; const advancedSettings: Omit = { - type: UI_SETTINGS_SAVED_OBJECTS_TYPE, + type: 'config', attributes: {}, references: [], }; @@ -196,7 +195,7 @@ describe('workspace_id_consumer integration test', () => { const findAdvancedSettings = await osdTestServer.request .get( root, - `/w/${createdFooWorkspace.id}/api/saved_objects/_find?type=${UI_SETTINGS_SAVED_OBJECTS_TYPE}` + `/w/${createdFooWorkspace.id}/api/saved_objects/_find?type=config` ) .expect(200); expect(findAdvancedSettings.body.total).toEqual(0); From 2119304a7b571a078d90b4e5e2deecee5e18ab68 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 15 Apr 2024 15:26:41 +0800 Subject: [PATCH 28/41] feat: add integration test Signed-off-by: SuZhou-Joe --- .../workspace_id_consumer_wrapper.test.ts | 51 +++++++++++++------ 1 file changed, 36 insertions(+), 15 deletions(-) diff --git a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts index 7abbaaac6ae2..3e73d5a44d4a 100644 --- a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts @@ -5,6 +5,7 @@ import { SavedObject } from 'src/core/types'; import { isEqual } from 'lodash'; +import packageInfo from '../../../../../../package.json'; import * as osdTestServer from '../../../../../core/test_helpers/osd_server'; import { DATA_SOURCE_SAVED_OBJECT_TYPE } from '../../../../data_source/common'; @@ -16,7 +17,9 @@ const dashboard: Omit = { const dataSource: Omit = { type: DATA_SOURCE_SAVED_OBJECT_TYPE, - attributes: {}, + attributes: { + title: 'test data source', + }, references: [], }; @@ -44,10 +47,6 @@ describe('workspace_id_consumer integration test', () => { const { startOpenSearch, startOpenSearchDashboards } = osdTestServer.createTestServers({ adjustTimeout: (t: number) => jest.setTimeout(t), settings: { - opensearch: { - license: 'oss', - opensearchFrom: '/Users/suzhou/Downloads/opensearch-3.0.0-SNAPSHOT', - }, osd: { data_source: { enabled: true, @@ -160,7 +159,7 @@ describe('workspace_id_consumer integration test', () => { ); }); - it('bulk create with disallowed types', async () => { + it('bulk create with disallowed types in workspace', async () => { await clearFooAndBar(); // import advanced settings and data sources should throw error @@ -173,14 +172,12 @@ describe('workspace_id_consumer integration test', () => { }, { ...advancedSettings, - id: 'bar', + id: packageInfo.version, }, ]) - .expect(400); + .expect(200); expect(createResultFoo.body).toEqual({ - error: 'Bad Request', - message: 'type: data-source, type: config are not allowed to create within a workspace.', - statusCode: 400, + saved_objects: [], }); // data source and advanced settings should not be found within the workspace @@ -193,14 +190,38 @@ describe('workspace_id_consumer integration test', () => { expect(findDataSourceResult.body.total).toEqual(0); const findAdvancedSettings = await osdTestServer.request - .get( - root, - `/w/${createdFooWorkspace.id}/api/saved_objects/_find?type=config` - ) + .get(root, `/w/${createdFooWorkspace.id}/api/saved_objects/_find?type=config`) .expect(200); expect(findAdvancedSettings.body.total).toEqual(0); }); + it('bulk create with disallowed types out of workspace', async () => { + await clearFooAndBar(); + + // import advanced settings and data sources should throw error + const createResultFoo = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create`) + .send([ + { + ...advancedSettings, + id: packageInfo.version, + }, + ]) + .expect(200); + expect(createResultFoo.body).toEqual({ + saved_objects: [ + expect.objectContaining({ + type: advancedSettings.type, + }), + ], + }); + + const findAdvancedSettings = await osdTestServer.request + .get(root, `/api/saved_objects/_find?type=${advancedSettings.type}`) + .expect(200); + expect(findAdvancedSettings.body.total).toEqual(1); + }); + it('checkConflicts when importing ndjson', async () => { await clearFooAndBar(); const createResultFoo = await osdTestServer.request From b1494b1c71f9ab1c53659a94867e80ca0d0850a3 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 15 Apr 2024 18:33:34 +0800 Subject: [PATCH 29/41] fix: unit test Signed-off-by: SuZhou-Joe --- .../workspace_id_consumer_wrapper.test.ts | 35 ++++++++----------- 1 file changed, 14 insertions(+), 21 deletions(-) diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts index 741d7ef3662e..f9fa276e820e 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts @@ -88,28 +88,21 @@ describe('WorkspaceIdConsumerWrapper', () => { ); }); - it(`Should throw error when trying to create unallowed type within a workspace`, async () => { - expect(() => - wrapperClient.bulkCreate([ - getSavedObject({ - type: 'config', - id: 'foo', - }), - ]) - ).toThrow('type: config is not allowed to create within a workspace.'); + it(`Should skip the objects when trying to create unallowed type within a workspace`, async () => { + await wrapperClient.bulkCreate([ + getSavedObject({ + type: 'config', + id: 'foo', + }), + getSavedObject({ + type: DATA_SOURCE_SAVED_OBJECT_TYPE, + id: 'foo', + }), + ]); - expect(() => - wrapperClient.bulkCreate([ - getSavedObject({ - type: DATA_SOURCE_SAVED_OBJECT_TYPE, - id: 'foo', - }), - getSavedObject({ - type: UI_SETTINGS_SAVED_OBJECTS_TYPE, - id: 'bar', - }), - ]) - ).toThrow('type: data-source, type: config are not allowed to create within a workspace.'); + expect(mockedClient.bulkCreate).toBeCalledWith([], { + workspaces: ['foo'], + }); }); }); From e61910d763a99b4bffb0b90352f67b5cf7da27d7 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 16 Apr 2024 18:53:45 +0800 Subject: [PATCH 30/41] feat: add error message Signed-off-by: SuZhou-Joe --- .../workspace_id_consumer_wrapper.test.ts | 16 +++++-- .../workspace_id_consumer_wrapper.test.ts | 17 +++++++- .../workspace_id_consumer_wrapper.ts | 43 ++++++++++++++++--- 3 files changed, 65 insertions(+), 11 deletions(-) diff --git a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts index 3e73d5a44d4a..75c0fd380bcc 100644 --- a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts @@ -176,9 +176,19 @@ describe('workspace_id_consumer integration test', () => { }, ]) .expect(200); - expect(createResultFoo.body).toEqual({ - saved_objects: [], - }); + expect(createResultFoo.body.saved_objects[0].error).toEqual( + expect.objectContaining({ + message: "Unsupport type in workspace: 'config' is not allowed to import in workspace.", + statusCode: 400, + }) + ); + expect(createResultFoo.body.saved_objects[1].error).toEqual( + expect.objectContaining({ + message: + "Unsupport type in workspace: 'data-source' is not allowed to import in workspace.", + statusCode: 400, + }) + ); // data source and advanced settings should not be found within the workspace const findDataSourceResult = await osdTestServer.request diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts index f9fa276e820e..da129e1403cc 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts @@ -8,7 +8,6 @@ import { SavedObject } from '../../../../core/public'; import { httpServerMock, savedObjectsClientMock, coreMock } from '../../../../core/server/mocks'; import { WorkspaceIdConsumerWrapper } from './workspace_id_consumer_wrapper'; import { DATA_SOURCE_SAVED_OBJECT_TYPE } from '../../../../plugins/data_source/common'; -import { UI_SETTINGS_SAVED_OBJECTS_TYPE } from '../../../../core/server'; describe('WorkspaceIdConsumerWrapper', () => { const requestHandlerContext = coreMock.createRequestHandlerContext(); @@ -89,7 +88,8 @@ describe('WorkspaceIdConsumerWrapper', () => { }); it(`Should skip the objects when trying to create unallowed type within a workspace`, async () => { - await wrapperClient.bulkCreate([ + mockedClient.bulkCreate.mockResolvedValueOnce({ saved_objects: [] }); + const result = await wrapperClient.bulkCreate([ getSavedObject({ type: 'config', id: 'foo', @@ -103,6 +103,19 @@ describe('WorkspaceIdConsumerWrapper', () => { expect(mockedClient.bulkCreate).toBeCalledWith([], { workspaces: ['foo'], }); + expect(result.saved_objects[0].error).toEqual( + expect.objectContaining({ + message: "Unsupport type in workspace: 'config' is not allowed to import in workspace.", + statusCode: 400, + }) + ); + expect(result.saved_objects[1].error).toEqual( + expect.objectContaining({ + message: + "Unsupport type in workspace: 'data-source' is not allowed to import in workspace.", + statusCode: 400, + }) + ); }); }); diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts index 1466fec3897c..63472219dc1c 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts @@ -12,6 +12,7 @@ import { SavedObjectsCheckConflictsObject, OpenSearchDashboardsRequest, SavedObjectsFindOptions, + SavedObjectsErrorHelpers, } from '../../../../core/server'; import { DATA_SOURCE_SAVED_OBJECT_TYPE } from '../../../../plugins/data_source/common'; @@ -67,30 +68,60 @@ export class WorkspaceIdConsumerWrapper { attributes, this.formatWorkspaceIdParams(wrapperOptions.request, options) ), - bulkCreate: ( + bulkCreate: async ( objects: Array>, options: SavedObjectsCreateOptions = {} ) => { const { workspaces } = this.formatWorkspaceIdParams(wrapperOptions.request, options); - const allowedSavedObjects = objects.filter((item) => { + const disallowedSavedObjects: Array> = []; + const allowedSavedObjects: Array> = []; + objects.forEach((item) => { const isImportIntoWorkspace = workspaces?.length || item.workspaces?.length; // config can not be created inside a workspace if (this.isConfigType(item.type) && isImportIntoWorkspace) { - return false; + disallowedSavedObjects.push(item); + return; } // For 2.14, data source can only be created without workspace info if (this.isDataSourceType(item.type) && isImportIntoWorkspace) { - return false; + disallowedSavedObjects.push(item); + return; } - return true; + allowedSavedObjects.push(item); + return; }); - return wrapperOptions.client.bulkCreate( + if (!disallowedSavedObjects.length) { + return await wrapperOptions.client.bulkCreate( + objects, + this.formatWorkspaceIdParams(wrapperOptions.request, options) + ); + } + + const allowedSavedObjectsBulkCreateResult = await wrapperOptions.client.bulkCreate( allowedSavedObjects, this.formatWorkspaceIdParams(wrapperOptions.request, options) ); + + return { + saved_objects: [ + ...allowedSavedObjectsBulkCreateResult.saved_objects, + ...disallowedSavedObjects.map((item) => ({ + references: [], + id: '', + ...item, + error: { + ...SavedObjectsErrorHelpers.decorateBadRequestError( + new Error(`'${item.type}' is not allowed to import in workspace.`), + 'Unsupport type in workspace' + ).output.payload, + metadata: { isNotOverwritable: true }, + }, + })), + ], + }; }, checkConflicts: ( objects: SavedObjectsCheckConflictsObject[] = [], From 49c771bc8c4a6f9f0da004856a06a4d4cd933643 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 17 Apr 2024 10:17:53 +0800 Subject: [PATCH 31/41] fix: integration test Signed-off-by: SuZhou-Joe --- .../integration_tests/workspace_id_consumer_wrapper.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts index 75c0fd380bcc..e9370ba892c9 100644 --- a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts @@ -178,14 +178,14 @@ describe('workspace_id_consumer integration test', () => { .expect(200); expect(createResultFoo.body.saved_objects[0].error).toEqual( expect.objectContaining({ - message: "Unsupport type in workspace: 'config' is not allowed to import in workspace.", + message: + "Unsupport type in workspace: 'data-source' is not allowed to import in workspace.", statusCode: 400, }) ); expect(createResultFoo.body.saved_objects[1].error).toEqual( expect.objectContaining({ - message: - "Unsupport type in workspace: 'data-source' is not allowed to import in workspace.", + message: "Unsupport type in workspace: 'config' is not allowed to import in workspace.", statusCode: 400, }) ); From 402d69b297ee5ebd0b81f821474cb698c00ea92d Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 17 Apr 2024 10:59:06 +0800 Subject: [PATCH 32/41] fix: integration test Signed-off-by: SuZhou-Joe --- .../workspace_id_consumer_wrapper.test.ts | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts index e9370ba892c9..f09a4fc414c0 100644 --- a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts @@ -47,6 +47,10 @@ describe('workspace_id_consumer integration test', () => { const { startOpenSearch, startOpenSearchDashboards } = osdTestServer.createTestServers({ adjustTimeout: (t: number) => jest.setTimeout(t), settings: { + opensearch: { + license: 'oss', + opensearchFrom: '/Users/suzhou/Downloads/opensearch-3.0.0-SNAPSHOT', + }, osd: { data_source: { enabled: true, @@ -190,15 +194,15 @@ describe('workspace_id_consumer integration test', () => { }) ); - // data source and advanced settings should not be found within the workspace - const findDataSourceResult = await osdTestServer.request + // Data source should not be created + await osdTestServer.request .get( root, - `/w/${createdFooWorkspace.id}/api/saved_objects/_find?type=${DATA_SOURCE_SAVED_OBJECT_TYPE}` + `/w/${createdFooWorkspace.id}/api/saved_objects/${DATA_SOURCE_SAVED_OBJECT_TYPE}/foo` ) - .expect(200); - expect(findDataSourceResult.body.total).toEqual(0); + .expect(404); + // Advanced settings should not be found within workspace const findAdvancedSettings = await osdTestServer.request .get(root, `/w/${createdFooWorkspace.id}/api/saved_objects/_find?type=config`) .expect(200); From cd77dbacbde9bb84f0d778b16f1393c75dd68e2e Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 17 Apr 2024 10:59:24 +0800 Subject: [PATCH 33/41] feat: remove useless change Signed-off-by: SuZhou-Joe --- .../integration_tests/workspace_id_consumer_wrapper.test.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts index f09a4fc414c0..e093e3dd7f89 100644 --- a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts @@ -47,10 +47,6 @@ describe('workspace_id_consumer integration test', () => { const { startOpenSearch, startOpenSearchDashboards } = osdTestServer.createTestServers({ adjustTimeout: (t: number) => jest.setTimeout(t), settings: { - opensearch: { - license: 'oss', - opensearchFrom: '/Users/suzhou/Downloads/opensearch-3.0.0-SNAPSHOT', - }, osd: { data_source: { enabled: true, From 152263720f26a1224d0cb47dc56fd7f04d3da4b9 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 17 Apr 2024 11:38:01 +0800 Subject: [PATCH 34/41] feat: add test case and add restrict on create method Signed-off-by: SuZhou-Joe --- .../workspace_id_consumer_wrapper.test.ts | 32 +++++++++++++++++++ .../workspace_id_consumer_wrapper.test.ts | 32 +++++++++++++++++++ .../workspace_id_consumer_wrapper.ts | 17 ++++++++-- 3 files changed, 78 insertions(+), 3 deletions(-) diff --git a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts index e093e3dd7f89..de712f5ad934 100644 --- a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts @@ -129,6 +129,38 @@ describe('workspace_id_consumer integration test', () => { }); }); + it('create disallowed types within workspace', async () => { + const createDataSourceResult = await osdTestServer.request + .post(root, `/w/${createdFooWorkspace.id}/api/saved_objects/${dataSource.type}`) + .send({ + attributes: dataSource.attributes, + }) + .expect(400); + + expect(createDataSourceResult.body).toMatchInlineSnapshot(` + Object { + "error": "Bad Request", + "message": "Unsupport type in workspace: 'data-source' is not allowed to create in workspace.", + "statusCode": 400, + } + `); + + const createConfigResult = await osdTestServer.request + .post(root, `/w/${createdFooWorkspace.id}/api/saved_objects/config`) + .send({ + attributes: dataSource.attributes, + }) + .expect(400); + + expect(createConfigResult.body).toMatchInlineSnapshot(` + Object { + "error": "Bad Request", + "message": "Unsupport type in workspace: 'config' is not allowed to create in workspace.", + "statusCode": 400, + } + `); + }); + it('bulk create', async () => { await clearFooAndBar(); const createResultFoo = await osdTestServer.request diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts index da129e1403cc..88d90a3abb45 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts @@ -66,6 +66,38 @@ describe('WorkspaceIdConsumerWrapper', () => { expect(mockedClient.create.mock.calls[0][2]?.hasOwnProperty('workspaces')).toEqual(false); }); + + it(`Should throw error when trying to create disallowed type in workspace`, async () => { + expect(() => + wrapperClient.create( + DATA_SOURCE_SAVED_OBJECT_TYPE, + { + name: 'foo', + }, + + { + workspaces: ['foo'], + } + ) + ).toThrowErrorMatchingInlineSnapshot( + `"Unsupport type in workspace: 'data-source' is not allowed to create in workspace."` + ); + + expect(() => + wrapperClient.create( + 'config', + { + name: 'foo', + }, + + { + workspaces: ['foo'], + } + ) + ).toThrowErrorMatchingInlineSnapshot( + `"Unsupport type in workspace: 'config' is not allowed to create in workspace."` + ); + }); }); describe('bulkCreate', () => { diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts index 63472219dc1c..b4df79c77369 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts @@ -62,12 +62,23 @@ export class WorkspaceIdConsumerWrapper { public wrapperFactory: SavedObjectsClientWrapperFactory = (wrapperOptions) => { return { ...wrapperOptions.client, - create: (type: string, attributes: T, options: SavedObjectsCreateOptions = {}) => - wrapperOptions.client.create( + create: (type: string, attributes: T, options: SavedObjectsCreateOptions = {}) => { + const { workspaces } = this.formatWorkspaceIdParams(wrapperOptions.request, options); + if (workspaces?.length && (this.isDataSourceType(type) || this.isConfigType(type))) { + // For 2.14, data source can only be created without workspace info + // config can not be created inside a workspace + throw SavedObjectsErrorHelpers.decorateBadRequestError( + new Error(`'${type}' is not allowed to create in workspace.`), + 'Unsupport type in workspace' + ); + } + + return wrapperOptions.client.create( type, attributes, this.formatWorkspaceIdParams(wrapperOptions.request, options) - ), + ); + }, bulkCreate: async ( objects: Array>, options: SavedObjectsCreateOptions = {} From 21f73c6d5745403ea14cb89e2c247cd73c9c3504 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 17 Apr 2024 11:46:14 +0800 Subject: [PATCH 35/41] feat: change type Signed-off-by: SuZhou-Joe --- .../server/saved_objects/workspace_id_consumer_wrapper.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts index b4df79c77369..aa5e4c3b3eb2 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts @@ -13,6 +13,7 @@ import { OpenSearchDashboardsRequest, SavedObjectsFindOptions, SavedObjectsErrorHelpers, + SavedObject, } from '../../../../core/server'; import { DATA_SOURCE_SAVED_OBJECT_TYPE } from '../../../../plugins/data_source/common'; @@ -48,11 +49,7 @@ export class WorkspaceIdConsumerWrapper { return type === DATA_SOURCE_SAVED_OBJECT_TYPE; } - private isConfigType(type: SavedObjectsFindOptions['type']): boolean { - if (Array.isArray(type)) { - return type.every((item) => item === UI_SETTINGS_SAVED_OBJECTS_TYPE); - } - + private isConfigType(type: SavedObject['type']): boolean { return type === UI_SETTINGS_SAVED_OBJECTS_TYPE; } private formatFindParams(options: SavedObjectsFindOptions): SavedObjectsFindOptions { From 544083e043deb61efd39d0581f2ab0da397b81fe Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 17 Apr 2024 11:48:05 +0800 Subject: [PATCH 36/41] feat: change comment Signed-off-by: SuZhou-Joe --- .../integration_tests/workspace_id_consumer_wrapper.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts index de712f5ad934..a40baba1d556 100644 --- a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts @@ -230,7 +230,7 @@ describe('workspace_id_consumer integration test', () => { ) .expect(404); - // Advanced settings should not be found within workspace + // Advanced settings should not be created within workspace const findAdvancedSettings = await osdTestServer.request .get(root, `/w/${createdFooWorkspace.id}/api/saved_objects/_find?type=config`) .expect(200); From 093de9e5fb451b4ed4d74b18d7bd19fd78952bb1 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 17 Apr 2024 12:59:16 +0800 Subject: [PATCH 37/41] feat: optimize test Signed-off-by: SuZhou-Joe --- .../server/saved_objects/workspace_id_consumer_wrapper.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts index 88d90a3abb45..616a11491552 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts @@ -119,7 +119,7 @@ describe('WorkspaceIdConsumerWrapper', () => { ); }); - it(`Should skip the objects when trying to create unallowed type within a workspace`, async () => { + it(`Should return error when trying to create unallowed type within a workspace`, async () => { mockedClient.bulkCreate.mockResolvedValueOnce({ saved_objects: [] }); const result = await wrapperClient.bulkCreate([ getSavedObject({ @@ -182,6 +182,7 @@ describe('WorkspaceIdConsumerWrapper', () => { it(`workspaces parameters should be removed when finding data sources`, async () => { await wrapperClient.find({ type: DATA_SOURCE_SAVED_OBJECT_TYPE, + workspaces: ['foo'], }); expect(mockedClient.find).toBeCalledWith({ type: DATA_SOURCE_SAVED_OBJECT_TYPE, From 0c80e824d0e2c67ecae700b5f80e61389a7cef7c Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 17 Apr 2024 16:43:58 +0800 Subject: [PATCH 38/41] refactor: move logic to conflict check wrapper Signed-off-by: SuZhou-Joe --- ...apper_for_check_workspace_conflict.test.ts | 126 ++++++++++++++++++ .../workspace_id_consumer_wrapper.test.ts | 124 ----------------- ...apper_for_check_workspace_conflict.test.ts | 79 +++++++++++ ...ts_wrapper_for_check_workspace_conflict.ts | 71 +++++++++- .../workspace_id_consumer_wrapper.test.ts | 73 ---------- .../workspace_id_consumer_wrapper.ts | 100 ++------------ 6 files changed, 281 insertions(+), 292 deletions(-) diff --git a/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts b/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts index ec5259608c72..a004db9d8774 100644 --- a/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts +++ b/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts @@ -5,7 +5,9 @@ import { SavedObject } from 'src/core/types'; import { isEqual } from 'lodash'; +import packageInfo from '../../../../../../package.json'; import * as osdTestServer from '../../../../../core/test_helpers/osd_server'; +import { DATA_SOURCE_SAVED_OBJECT_TYPE } from '../../../../data_source/common'; const dashboard: Omit = { type: 'dashboard', @@ -13,6 +15,20 @@ const dashboard: Omit = { references: [], }; +const dataSource: Omit = { + type: DATA_SOURCE_SAVED_OBJECT_TYPE, + attributes: { + title: 'test data source', + }, + references: [], +}; + +const advancedSettings: Omit = { + type: 'config', + attributes: {}, + references: [], +}; + interface WorkspaceAttributes { id: string; name?: string; @@ -32,6 +48,9 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', adjustTimeout: (t: number) => jest.setTimeout(t), settings: { osd: { + data_source: { + enabled: true, + }, workspace: { enabled: true, }, @@ -150,6 +169,40 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', }); }); + it('create disallowed types within workspace', async () => { + const createDataSourceResult = await osdTestServer.request + .post(root, `/api/saved_objects/${dataSource.type}`) + .send({ + attributes: dataSource.attributes, + workspaces: [createdFooWorkspace.id], + }) + .expect(400); + + expect(createDataSourceResult.body).toMatchInlineSnapshot(` + Object { + "error": "Bad Request", + "message": "Unsupported type in workspace: 'data-source' is not allowed to create in workspace.", + "statusCode": 400, + } + `); + + const createConfigResult = await osdTestServer.request + .post(root, `/api/saved_objects/config`) + .send({ + attributes: advancedSettings.attributes, + workspaces: [createdFooWorkspace.id], + }) + .expect(400); + + expect(createConfigResult.body).toMatchInlineSnapshot(` + Object { + "error": "Bad Request", + "message": "Unsupported type in workspace: 'config' is not allowed to create in workspace.", + "statusCode": 400, + } + `); + }); + it('bulk create', async () => { await clearFooAndBar(); const createResultFoo = await osdTestServer.request @@ -259,6 +312,79 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', ); }); + it('bulk create with disallowed types in workspace', async () => { + await clearFooAndBar(); + + // import advanced settings and data sources should throw error + const createResultFoo = await osdTestServer.request + .post(root, `/w/${createdFooWorkspace.id}/api/saved_objects/_bulk_create`) + .send([ + { + ...dataSource, + id: 'foo', + }, + { + ...advancedSettings, + id: packageInfo.version, + }, + ]) + .expect(200); + expect(createResultFoo.body.saved_objects[0].error).toEqual( + expect.objectContaining({ + message: + "Unsupported type in workspace: 'data-source' is not allowed to import in workspace.", + statusCode: 400, + }) + ); + expect(createResultFoo.body.saved_objects[1].error).toEqual( + expect.objectContaining({ + message: "Unsupported type in workspace: 'config' is not allowed to import in workspace.", + statusCode: 400, + }) + ); + + // Data source should not be created + await osdTestServer.request + .get( + root, + `/w/${createdFooWorkspace.id}/api/saved_objects/${DATA_SOURCE_SAVED_OBJECT_TYPE}/foo` + ) + .expect(404); + + // Advanced settings should not be created within workspace + const findAdvancedSettings = await osdTestServer.request + .get(root, `/w/${createdFooWorkspace.id}/api/saved_objects/_find?type=config`) + .expect(200); + expect(findAdvancedSettings.body.total).toEqual(0); + }); + + it('bulk create with disallowed types out of workspace', async () => { + await clearFooAndBar(); + + // import advanced settings and data sources should throw error + const createResultFoo = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create`) + .send([ + { + ...advancedSettings, + id: packageInfo.version, + }, + ]) + .expect(200); + expect(createResultFoo.body).toEqual({ + saved_objects: [ + expect.objectContaining({ + type: advancedSettings.type, + }), + ], + }); + + const findAdvancedSettings = await osdTestServer.request + .get(root, `/api/saved_objects/_find?type=${advancedSettings.type}`) + .expect(200); + expect(findAdvancedSettings.body.total).toEqual(1); + }); + it('checkConflicts when importing ndjson', async () => { await clearFooAndBar(); const createResultFoo = await osdTestServer.request diff --git a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts index a40baba1d556..ac643d59e641 100644 --- a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts @@ -5,9 +5,7 @@ import { SavedObject } from 'src/core/types'; import { isEqual } from 'lodash'; -import packageInfo from '../../../../../../package.json'; import * as osdTestServer from '../../../../../core/test_helpers/osd_server'; -import { DATA_SOURCE_SAVED_OBJECT_TYPE } from '../../../../data_source/common'; const dashboard: Omit = { type: 'dashboard', @@ -15,20 +13,6 @@ const dashboard: Omit = { references: [], }; -const dataSource: Omit = { - type: DATA_SOURCE_SAVED_OBJECT_TYPE, - attributes: { - title: 'test data source', - }, - references: [], -}; - -const advancedSettings: Omit = { - type: 'config', - attributes: {}, - references: [], -}; - interface WorkspaceAttributes { id: string; name?: string; @@ -48,9 +32,6 @@ describe('workspace_id_consumer integration test', () => { adjustTimeout: (t: number) => jest.setTimeout(t), settings: { osd: { - data_source: { - enabled: true, - }, workspace: { enabled: true, }, @@ -129,38 +110,6 @@ describe('workspace_id_consumer integration test', () => { }); }); - it('create disallowed types within workspace', async () => { - const createDataSourceResult = await osdTestServer.request - .post(root, `/w/${createdFooWorkspace.id}/api/saved_objects/${dataSource.type}`) - .send({ - attributes: dataSource.attributes, - }) - .expect(400); - - expect(createDataSourceResult.body).toMatchInlineSnapshot(` - Object { - "error": "Bad Request", - "message": "Unsupport type in workspace: 'data-source' is not allowed to create in workspace.", - "statusCode": 400, - } - `); - - const createConfigResult = await osdTestServer.request - .post(root, `/w/${createdFooWorkspace.id}/api/saved_objects/config`) - .send({ - attributes: dataSource.attributes, - }) - .expect(400); - - expect(createConfigResult.body).toMatchInlineSnapshot(` - Object { - "error": "Bad Request", - "message": "Unsupport type in workspace: 'config' is not allowed to create in workspace.", - "statusCode": 400, - } - `); - }); - it('bulk create', async () => { await clearFooAndBar(); const createResultFoo = await osdTestServer.request @@ -191,79 +140,6 @@ describe('workspace_id_consumer integration test', () => { ); }); - it('bulk create with disallowed types in workspace', async () => { - await clearFooAndBar(); - - // import advanced settings and data sources should throw error - const createResultFoo = await osdTestServer.request - .post(root, `/w/${createdFooWorkspace.id}/api/saved_objects/_bulk_create`) - .send([ - { - ...dataSource, - id: 'foo', - }, - { - ...advancedSettings, - id: packageInfo.version, - }, - ]) - .expect(200); - expect(createResultFoo.body.saved_objects[0].error).toEqual( - expect.objectContaining({ - message: - "Unsupport type in workspace: 'data-source' is not allowed to import in workspace.", - statusCode: 400, - }) - ); - expect(createResultFoo.body.saved_objects[1].error).toEqual( - expect.objectContaining({ - message: "Unsupport type in workspace: 'config' is not allowed to import in workspace.", - statusCode: 400, - }) - ); - - // Data source should not be created - await osdTestServer.request - .get( - root, - `/w/${createdFooWorkspace.id}/api/saved_objects/${DATA_SOURCE_SAVED_OBJECT_TYPE}/foo` - ) - .expect(404); - - // Advanced settings should not be created within workspace - const findAdvancedSettings = await osdTestServer.request - .get(root, `/w/${createdFooWorkspace.id}/api/saved_objects/_find?type=config`) - .expect(200); - expect(findAdvancedSettings.body.total).toEqual(0); - }); - - it('bulk create with disallowed types out of workspace', async () => { - await clearFooAndBar(); - - // import advanced settings and data sources should throw error - const createResultFoo = await osdTestServer.request - .post(root, `/api/saved_objects/_bulk_create`) - .send([ - { - ...advancedSettings, - id: packageInfo.version, - }, - ]) - .expect(200); - expect(createResultFoo.body).toEqual({ - saved_objects: [ - expect.objectContaining({ - type: advancedSettings.type, - }), - ], - }); - - const findAdvancedSettings = await osdTestServer.request - .get(root, `/api/saved_objects/_find?type=${advancedSettings.type}`) - .expect(200); - expect(findAdvancedSettings.body.total).toEqual(1); - }); - it('checkConflicts when importing ndjson', async () => { await clearFooAndBar(); const createResultFoo = await osdTestServer.request diff --git a/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.test.ts b/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.test.ts index 9c29684e58e4..c242470fb9b5 100644 --- a/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.test.ts +++ b/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.test.ts @@ -7,6 +7,7 @@ import { SavedObject } from '../../../../core/public'; import { httpServerMock, savedObjectsClientMock, coreMock } from '../../../../core/server/mocks'; import { WorkspaceConflictSavedObjectsClientWrapper } from './saved_objects_wrapper_for_check_workspace_conflict'; import { SavedObjectsSerializer } from '../../../../core/server'; +import { DATA_SOURCE_SAVED_OBJECT_TYPE } from '../../../../plugins/data_source/common'; describe('WorkspaceConflictSavedObjectsClientWrapper', () => { const requestHandlerContext = coreMock.createRequestHandlerContext(); @@ -115,6 +116,38 @@ describe('WorkspaceConflictSavedObjectsClientWrapper', () => { }) ); }); + + it(`Should throw error when trying to create disallowed types in workspace`, async () => { + expect(() => + wrapperClient.create( + DATA_SOURCE_SAVED_OBJECT_TYPE, + { + name: 'foo', + }, + + { + workspaces: ['foo'], + } + ) + ).toThrowErrorMatchingInlineSnapshot( + `"Unsupported type in workspace: 'data-source' is not allowed to create in workspace."` + ); + + expect(() => + wrapperClient.create( + 'config', + { + name: 'foo', + }, + + { + workspaces: ['foo'], + } + ) + ).toThrowErrorMatchingInlineSnapshot( + `"Unsupported type in workspace: 'config' is not allowed to create in workspace."` + ); + }); }); describe('bulkCreateWithWorkspaceConflictCheck', () => { @@ -291,6 +324,36 @@ describe('WorkspaceConflictSavedObjectsClientWrapper', () => { } `); }); + it(`Should return error when trying to create disallowed types within a workspace`, async () => { + mockedClient.bulkCreate.mockResolvedValueOnce({ saved_objects: [] }); + const result = await wrapperClient.bulkCreate([ + getSavedObject({ + type: 'config', + id: 'foo', + }), + getSavedObject({ + type: DATA_SOURCE_SAVED_OBJECT_TYPE, + id: 'foo', + }), + ]); + + expect(mockedClient.bulkCreate).toBeCalledWith([], { + workspaces: ['foo'], + }); + expect(result.saved_objects[0].error).toEqual( + expect.objectContaining({ + message: "Unsupported type in workspace: 'config' is not allowed to import in workspace.", + statusCode: 400, + }) + ); + expect(result.saved_objects[1].error).toEqual( + expect.objectContaining({ + message: + "Unsupported type in workspace: 'data-source' is not allowed to import in workspace.", + statusCode: 400, + }) + ); + }); }); describe('checkConflictWithWorkspaceConflictCheck', () => { @@ -393,4 +456,20 @@ describe('WorkspaceConflictSavedObjectsClientWrapper', () => { `); }); }); + + describe('find', () => { + beforeEach(() => { + mockedClient.find.mockClear(); + }); + + it(`workspaces parameters should be removed when finding data sources`, async () => { + await wrapperClient.find({ + type: DATA_SOURCE_SAVED_OBJECT_TYPE, + workspaces: ['foo'], + }); + expect(mockedClient.find).toBeCalledWith({ + type: DATA_SOURCE_SAVED_OBJECT_TYPE, + }); + }); + }); }); diff --git a/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.ts b/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.ts index 838b689328bf..46ad3cf2ec97 100644 --- a/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.ts +++ b/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.ts @@ -15,7 +15,11 @@ import { SavedObjectsSerializer, SavedObjectsCheckConflictsObject, SavedObjectsCheckConflictsResponse, + SavedObjectsFindOptions, } from '../../../../core/server'; +import { DATA_SOURCE_SAVED_OBJECT_TYPE } from '../../../data_source/common'; + +const UI_SETTINGS_SAVED_OBJECTS_TYPE = 'config'; const errorContent = (error: Boom.Boom) => error.output.payload; @@ -36,6 +40,21 @@ export class WorkspaceConflictSavedObjectsClientWrapper { ); } + private isDataSourceType(type: SavedObjectsFindOptions['type']): boolean { + if (Array.isArray(type)) { + return type.every((item) => item === DATA_SOURCE_SAVED_OBJECT_TYPE); + } + + return type === DATA_SOURCE_SAVED_OBJECT_TYPE; + } + private isConfigType(type: SavedObject['type']): boolean { + return type === UI_SETTINGS_SAVED_OBJECTS_TYPE; + } + private formatFindParams(options: SavedObjectsFindOptions): SavedObjectsFindOptions { + const isListingDataSource = this.isDataSourceType(options.type); + return isListingDataSource ? { ...options, workspaces: null } : options; + } + /** * Workspace is a concept to manage saved objects and the `workspaces` field of each object indicates workspaces the object belongs to. * When user tries to update an existing object's attribute, workspaces field should be preserved. Below are some cases that this conflict wrapper will take effect: @@ -49,6 +68,16 @@ export class WorkspaceConflictSavedObjectsClientWrapper { options: SavedObjectsCreateOptions = {} ) => { const { workspaces, id, overwrite } = options; + + if (workspaces?.length && (this.isDataSourceType(type) || this.isConfigType(type))) { + // For 2.14, data source can only be created without workspace info + // config can not be created inside a workspace + throw SavedObjectsErrorHelpers.decorateBadRequestError( + new Error(`'${type}' is not allowed to create in workspace.`), + 'Unsupported type in workspace' + ); + } + let savedObjectWorkspaces = options?.workspaces; /** @@ -89,12 +118,33 @@ export class WorkspaceConflictSavedObjectsClientWrapper { objects: Array>, options: SavedObjectsCreateOptions = {} ): Promise> => { - const { overwrite, namespace } = options; + const { overwrite, namespace, workspaces } = options; + + const disallowedSavedObjects: Array> = []; + const allowedSavedObjects: Array> = []; + objects.forEach((item) => { + const isImportIntoWorkspace = workspaces?.length || item.workspaces?.length; + // config can not be created inside a workspace + if (this.isConfigType(item.type) && isImportIntoWorkspace) { + disallowedSavedObjects.push(item); + return; + } + + // For 2.14, data source can only be created without workspace info + if (this.isDataSourceType(item.type) && isImportIntoWorkspace) { + disallowedSavedObjects.push(item); + return; + } + + allowedSavedObjects.push(item); + return; + }); + /** * When overwrite, filter out all the objects that have ids */ const bulkGetDocs = overwrite - ? objects + ? allowedSavedObjects .filter((object) => !!object.id) .map((object) => { /** @@ -211,6 +261,18 @@ export class WorkspaceConflictSavedObjectsClientWrapper { ...realBulkCreateResult, saved_objects: [ ...objectsConflictWithWorkspace, + ...disallowedSavedObjects.map((item) => ({ + references: [], + id: '', + ...item, + error: { + ...SavedObjectsErrorHelpers.decorateBadRequestError( + new Error(`'${item.type}' is not allowed to import in workspace.`), + 'Unsupported type in workspace' + ).output.payload, + metadata: { isNotOverwritable: true }, + }, + })), ...(realBulkCreateResult?.saved_objects || []), ], } as SavedObjectsBulkResponse; @@ -316,7 +378,10 @@ export class WorkspaceConflictSavedObjectsClientWrapper { bulkCreate: bulkCreateWithWorkspaceConflictCheck, checkConflicts: checkConflictWithWorkspaceConflictCheck, delete: wrapperOptions.client.delete, - find: wrapperOptions.client.find, + find: (options: SavedObjectsFindOptions) => + // TODO: The `formatFindParams` is a workaround for 2.14 to always list global data sources, + // should remove this workaround in the upcoming release once readonly share is available. + wrapperOptions.client.find(this.formatFindParams(options)), bulkGet: wrapperOptions.client.bulkGet, get: wrapperOptions.client.get, update: wrapperOptions.client.update, diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts index 616a11491552..6d9219bef2c5 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts @@ -66,38 +66,6 @@ describe('WorkspaceIdConsumerWrapper', () => { expect(mockedClient.create.mock.calls[0][2]?.hasOwnProperty('workspaces')).toEqual(false); }); - - it(`Should throw error when trying to create disallowed type in workspace`, async () => { - expect(() => - wrapperClient.create( - DATA_SOURCE_SAVED_OBJECT_TYPE, - { - name: 'foo', - }, - - { - workspaces: ['foo'], - } - ) - ).toThrowErrorMatchingInlineSnapshot( - `"Unsupport type in workspace: 'data-source' is not allowed to create in workspace."` - ); - - expect(() => - wrapperClient.create( - 'config', - { - name: 'foo', - }, - - { - workspaces: ['foo'], - } - ) - ).toThrowErrorMatchingInlineSnapshot( - `"Unsupport type in workspace: 'config' is not allowed to create in workspace."` - ); - }); }); describe('bulkCreate', () => { @@ -118,37 +86,6 @@ describe('WorkspaceIdConsumerWrapper', () => { } ); }); - - it(`Should return error when trying to create unallowed type within a workspace`, async () => { - mockedClient.bulkCreate.mockResolvedValueOnce({ saved_objects: [] }); - const result = await wrapperClient.bulkCreate([ - getSavedObject({ - type: 'config', - id: 'foo', - }), - getSavedObject({ - type: DATA_SOURCE_SAVED_OBJECT_TYPE, - id: 'foo', - }), - ]); - - expect(mockedClient.bulkCreate).toBeCalledWith([], { - workspaces: ['foo'], - }); - expect(result.saved_objects[0].error).toEqual( - expect.objectContaining({ - message: "Unsupport type in workspace: 'config' is not allowed to import in workspace.", - statusCode: 400, - }) - ); - expect(result.saved_objects[1].error).toEqual( - expect.objectContaining({ - message: - "Unsupport type in workspace: 'data-source' is not allowed to import in workspace.", - statusCode: 400, - }) - ); - }); }); describe('checkConflict', () => { @@ -178,15 +115,5 @@ describe('WorkspaceIdConsumerWrapper', () => { workspaces: ['foo'], }); }); - - it(`workspaces parameters should be removed when finding data sources`, async () => { - await wrapperClient.find({ - type: DATA_SOURCE_SAVED_OBJECT_TYPE, - workspaces: ['foo'], - }); - expect(mockedClient.find).toBeCalledWith({ - type: DATA_SOURCE_SAVED_OBJECT_TYPE, - }); - }); }); }); diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts index aa5e4c3b3eb2..f41ff7bfba08 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts @@ -12,15 +12,10 @@ import { SavedObjectsCheckConflictsObject, OpenSearchDashboardsRequest, SavedObjectsFindOptions, - SavedObjectsErrorHelpers, - SavedObject, } from '../../../../core/server'; -import { DATA_SOURCE_SAVED_OBJECT_TYPE } from '../../../../plugins/data_source/common'; type WorkspaceOptions = Pick | undefined; -const UI_SETTINGS_SAVED_OBJECTS_TYPE = 'config'; - export class WorkspaceIdConsumerWrapper { private formatWorkspaceIdParams( request: OpenSearchDashboardsRequest, @@ -42,95 +37,23 @@ export class WorkspaceIdConsumerWrapper { ...(finalWorkspaces.length ? { workspaces: finalWorkspaces } : {}), }; } - private isDataSourceType(type: SavedObjectsFindOptions['type']): boolean { - if (Array.isArray(type)) { - return type.every((item) => item === DATA_SOURCE_SAVED_OBJECT_TYPE); - } - - return type === DATA_SOURCE_SAVED_OBJECT_TYPE; - } - private isConfigType(type: SavedObject['type']): boolean { - return type === UI_SETTINGS_SAVED_OBJECTS_TYPE; - } - private formatFindParams(options: SavedObjectsFindOptions): SavedObjectsFindOptions { - const isListingDataSource = this.isDataSourceType(options.type); - return isListingDataSource ? { ...options, workspaces: null } : options; - } public wrapperFactory: SavedObjectsClientWrapperFactory = (wrapperOptions) => { return { ...wrapperOptions.client, - create: (type: string, attributes: T, options: SavedObjectsCreateOptions = {}) => { - const { workspaces } = this.formatWorkspaceIdParams(wrapperOptions.request, options); - if (workspaces?.length && (this.isDataSourceType(type) || this.isConfigType(type))) { - // For 2.14, data source can only be created without workspace info - // config can not be created inside a workspace - throw SavedObjectsErrorHelpers.decorateBadRequestError( - new Error(`'${type}' is not allowed to create in workspace.`), - 'Unsupport type in workspace' - ); - } - - return wrapperOptions.client.create( + create: (type: string, attributes: T, options: SavedObjectsCreateOptions = {}) => + wrapperOptions.client.create( type, attributes, this.formatWorkspaceIdParams(wrapperOptions.request, options) - ); - }, + ), bulkCreate: async ( objects: Array>, options: SavedObjectsCreateOptions = {} - ) => { - const { workspaces } = this.formatWorkspaceIdParams(wrapperOptions.request, options); - const disallowedSavedObjects: Array> = []; - const allowedSavedObjects: Array> = []; - objects.forEach((item) => { - const isImportIntoWorkspace = workspaces?.length || item.workspaces?.length; - // config can not be created inside a workspace - if (this.isConfigType(item.type) && isImportIntoWorkspace) { - disallowedSavedObjects.push(item); - return; - } - - // For 2.14, data source can only be created without workspace info - if (this.isDataSourceType(item.type) && isImportIntoWorkspace) { - disallowedSavedObjects.push(item); - return; - } - - allowedSavedObjects.push(item); - return; - }); - - if (!disallowedSavedObjects.length) { - return await wrapperOptions.client.bulkCreate( - objects, - this.formatWorkspaceIdParams(wrapperOptions.request, options) - ); - } - - const allowedSavedObjectsBulkCreateResult = await wrapperOptions.client.bulkCreate( - allowedSavedObjects, + ) => + wrapperOptions.client.bulkCreate( + objects, this.formatWorkspaceIdParams(wrapperOptions.request, options) - ); - - return { - saved_objects: [ - ...allowedSavedObjectsBulkCreateResult.saved_objects, - ...disallowedSavedObjects.map((item) => ({ - references: [], - id: '', - ...item, - error: { - ...SavedObjectsErrorHelpers.decorateBadRequestError( - new Error(`'${item.type}' is not allowed to import in workspace.`), - 'Unsupport type in workspace' - ).output.payload, - metadata: { isNotOverwritable: true }, - }, - })), - ], - }; - }, + ), checkConflicts: ( objects: SavedObjectsCheckConflictsObject[] = [], options: SavedObjectsBaseOptions = {} @@ -141,14 +64,7 @@ export class WorkspaceIdConsumerWrapper { ), delete: wrapperOptions.client.delete, find: (options: SavedObjectsFindOptions) => - wrapperOptions.client.find( - this.formatWorkspaceIdParams( - wrapperOptions.request, - // The `formatFindParams` is a workaroud for 2.14 to always list global data sources, - // should remove this workaround in 2.15 once readonly share is available - this.formatFindParams(options) - ) - ), + wrapperOptions.client.find(this.formatWorkspaceIdParams(wrapperOptions.request, options)), bulkGet: wrapperOptions.client.bulkGet, get: wrapperOptions.client.get, update: wrapperOptions.client.update, From 804e50f52e5e4e592f5eef71e2bc2c70c0be9205 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 17 Apr 2024 16:45:02 +0800 Subject: [PATCH 39/41] feat: remove useless change Signed-off-by: SuZhou-Joe --- .../server/saved_objects/workspace_id_consumer_wrapper.test.ts | 1 - .../server/saved_objects/workspace_id_consumer_wrapper.ts | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts index 6d9219bef2c5..112f31baf562 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts @@ -7,7 +7,6 @@ import { updateWorkspaceState } from '../../../../core/server/utils'; import { SavedObject } from '../../../../core/public'; import { httpServerMock, savedObjectsClientMock, coreMock } from '../../../../core/server/mocks'; import { WorkspaceIdConsumerWrapper } from './workspace_id_consumer_wrapper'; -import { DATA_SOURCE_SAVED_OBJECT_TYPE } from '../../../../plugins/data_source/common'; describe('WorkspaceIdConsumerWrapper', () => { const requestHandlerContext = coreMock.createRequestHandlerContext(); diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts index f41ff7bfba08..74e8e99af71e 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts @@ -46,7 +46,7 @@ export class WorkspaceIdConsumerWrapper { attributes, this.formatWorkspaceIdParams(wrapperOptions.request, options) ), - bulkCreate: async ( + bulkCreate: ( objects: Array>, options: SavedObjectsCreateOptions = {} ) => From 0810ec5647c93e63c7713394dcce948000cd27aa Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 17 Apr 2024 17:03:34 +0800 Subject: [PATCH 40/41] fix: unit test Signed-off-by: SuZhou-Joe --- ...cts_wrapper_for_check_workspace_conflict.test.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.test.ts b/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.test.ts index c242470fb9b5..2cbfb951bbee 100644 --- a/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.test.ts +++ b/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.test.ts @@ -118,7 +118,7 @@ describe('WorkspaceConflictSavedObjectsClientWrapper', () => { }); it(`Should throw error when trying to create disallowed types in workspace`, async () => { - expect(() => + await expect( wrapperClient.create( DATA_SOURCE_SAVED_OBJECT_TYPE, { @@ -129,11 +129,11 @@ describe('WorkspaceConflictSavedObjectsClientWrapper', () => { workspaces: ['foo'], } ) - ).toThrowErrorMatchingInlineSnapshot( - `"Unsupported type in workspace: 'data-source' is not allowed to create in workspace."` + ).rejects.toMatchInlineSnapshot( + `[Error: Unsupported type in workspace: 'data-source' is not allowed to create in workspace.]` ); - expect(() => + await expect( wrapperClient.create( 'config', { @@ -144,8 +144,8 @@ describe('WorkspaceConflictSavedObjectsClientWrapper', () => { workspaces: ['foo'], } ) - ).toThrowErrorMatchingInlineSnapshot( - `"Unsupported type in workspace: 'config' is not allowed to create in workspace."` + ).rejects.toMatchInlineSnapshot( + `[Error: Unsupported type in workspace: 'config' is not allowed to create in workspace.]` ); }); }); @@ -469,6 +469,7 @@ describe('WorkspaceConflictSavedObjectsClientWrapper', () => { }); expect(mockedClient.find).toBeCalledWith({ type: DATA_SOURCE_SAVED_OBJECT_TYPE, + workspaces: null, }); }); }); From 39bb304f9d3499a306f4b981b644c32dcf8292b2 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 17 Apr 2024 17:54:26 +0800 Subject: [PATCH 41/41] fix: unit test Signed-off-by: SuZhou-Joe --- ...apper_for_check_workspace_conflict.test.ts | 25 +++++++++++-------- ...ts_wrapper_for_check_workspace_conflict.ts | 2 +- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.test.ts b/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.test.ts index 2cbfb951bbee..c6efcb832221 100644 --- a/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.test.ts +++ b/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.test.ts @@ -326,16 +326,21 @@ describe('WorkspaceConflictSavedObjectsClientWrapper', () => { }); it(`Should return error when trying to create disallowed types within a workspace`, async () => { mockedClient.bulkCreate.mockResolvedValueOnce({ saved_objects: [] }); - const result = await wrapperClient.bulkCreate([ - getSavedObject({ - type: 'config', - id: 'foo', - }), - getSavedObject({ - type: DATA_SOURCE_SAVED_OBJECT_TYPE, - id: 'foo', - }), - ]); + const result = await wrapperClient.bulkCreate( + [ + getSavedObject({ + type: 'config', + id: 'foo', + }), + getSavedObject({ + type: DATA_SOURCE_SAVED_OBJECT_TYPE, + id: 'foo', + }), + ], + { + workspaces: ['foo'], + } + ); expect(mockedClient.bulkCreate).toBeCalledWith([], { workspaces: ['foo'], diff --git a/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.ts b/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.ts index 46ad3cf2ec97..8d57226e71ab 100644 --- a/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.ts +++ b/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.ts @@ -219,7 +219,7 @@ export class WorkspaceConflictSavedObjectsClientWrapper { /** * Get all the objects that do not conflict on workspaces */ - const objectsNoWorkspaceConflictError = objects.filter( + const objectsNoWorkspaceConflictError = allowedSavedObjects.filter( (item) => !objectsConflictWithWorkspace.find( (errorItems) =>