From 7d6d7d067f784a2ceb0401c9970ca61bc51cc26a Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Tue, 23 Apr 2024 18:07:14 +0800 Subject: [PATCH] [Workspace] Handle data sources and advanced settings as global object. (#6524) (#6611) * Handle data sources and advanced settings as global object. (#313) * feat: POC implementation * feat: add some comment * feat: revert dependency * feat: update comment * feat: address one TODO * feat: address TODO * feat: add unit test * feat: some special logic on specific operation * feat: add integration test * feat: declare workspaces as empty array for advanced settings * feat: unified workspaces parameters when parsing from router * feat: improve code coverage * feat: declare workspaces as null * feat: use unified types * feat: update comment * feat: remove null * feat: address comments * feat: use request app to store request workspace id * feat: use app state to store request workspace id * feat: remove workspaces when listing data sources * feat: remove useless code change * feat: throw error if the type is not allowed * feat: add unit test * feat: add integration test * feat: change the implementation * feat: remove useless change * feat: remove useless change * feat: add integration test * fix: unit test * feat: add error message * fix: integration test * fix: integration test * feat: remove useless change * feat: add test case and add restrict on create method * feat: change type * feat: change comment * feat: optimize test * refactor: move logic to conflict check wrapper * feat: remove useless change * fix: unit test * fix: unit test --------- * Changeset file for PR #6524 created/updated * Apply suggestions from code review * feat: optimize based on comment * feat: remove useless default value --------- (cherry picked from commit da88296dea21e4b411c16e40f7d89b12cb09058d) Signed-off-by: SuZhou-Joe Signed-off-by: github-actions[bot] Co-authored-by: github-actions[bot] Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Co-authored-by: Lu Yu --- changelogs/fragments/6524.yml | 2 + ...apper_for_check_workspace_conflict.test.ts | 127 ++++++++++++++++++ ...apper_for_check_workspace_conflict.test.ts | 85 ++++++++++++ ...ts_wrapper_for_check_workspace_conflict.ts | 72 +++++++++- 4 files changed, 282 insertions(+), 4 deletions(-) create mode 100644 changelogs/fragments/6524.yml diff --git a/changelogs/fragments/6524.yml b/changelogs/fragments/6524.yml new file mode 100644 index 000000000000..1c7c99bb0145 --- /dev/null +++ b/changelogs/fragments/6524.yml @@ -0,0 +1,2 @@ +feat: +- [Workspace] Handle data sources and advanced settings as global object. ([#6524](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6524)) \ No newline at end of file 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 75b19bb225b0..0942556b521d 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, }, @@ -145,6 +164,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 be created 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 be created in workspace.", + "statusCode": 400, + } + `); + }); + it('bulk create', async () => { await clearFooAndBar(); const createResultFoo = await osdTestServer.request @@ -254,6 +307,80 @@ 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 be imported in workspace.", + statusCode: 400, + }) + ); + expect(createResultFoo.body.saved_objects[1].error).toEqual( + expect.objectContaining({ + message: + "Unsupported type in workspace: 'config' is not allowed to be imported 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..70b571ebc3e3 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 () => { + await expect( + wrapperClient.create( + DATA_SOURCE_SAVED_OBJECT_TYPE, + { + name: 'foo', + }, + + { + workspaces: ['foo'], + } + ) + ).rejects.toMatchInlineSnapshot( + `[Error: Unsupported type in workspace: 'data-source' is not allowed to be created in workspace.]` + ); + + await expect( + wrapperClient.create( + 'config', + { + name: 'foo', + }, + + { + workspaces: ['foo'], + } + ) + ).rejects.toMatchInlineSnapshot( + `[Error: Unsupported type in workspace: 'config' is not allowed to be created in workspace.]` + ); + }); }); describe('bulkCreateWithWorkspaceConflictCheck', () => { @@ -291,6 +324,42 @@ 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', + }), + ], + { + workspaces: ['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 be imported in workspace.", + statusCode: 400, + }) + ); + expect(result.saved_objects[1].error).toEqual( + expect.objectContaining({ + message: + "Unsupported type in workspace: 'data-source' is not allowed to be imported in workspace.", + statusCode: 400, + }) + ); + }); }); describe('checkConflictWithWorkspaceConflictCheck', () => { @@ -393,4 +462,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..5f293580153d 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,22 @@ 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); + const { workspaces, ...otherOptions } = options; + return isListingDataSource ? otherOptions : 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 +69,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 be created in workspace.`), + 'Unsupported type in workspace' + ); + } + let savedObjectWorkspaces = options?.workspaces; /** @@ -89,12 +119,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) => { /** @@ -169,7 +220,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) => @@ -211,6 +262,16 @@ export class WorkspaceConflictSavedObjectsClientWrapper { ...realBulkCreateResult, saved_objects: [ ...objectsConflictWithWorkspace, + ...disallowedSavedObjects.map((item) => ({ + ...item, + error: { + ...SavedObjectsErrorHelpers.decorateBadRequestError( + new Error(`'${item.type}' is not allowed to be imported in workspace.`), + 'Unsupported type in workspace' + ).output.payload, + metadata: { isNotOverwritable: true }, + }, + })), ...(realBulkCreateResult?.saved_objects || []), ], } as SavedObjectsBulkResponse; @@ -316,7 +377,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,