From afb3701038a2c63ca526cae724e8b4bb782e9cdd Mon Sep 17 00:00:00 2001 From: Hailong Cui Date: Fri, 13 Oct 2023 09:55:19 +0800 Subject: [PATCH 1/5] [API] Delete saved objects by workspace (#216) * Delete saved objects by workspace Signed-off-by: Hailong Cui fix osd boostrap Signed-off-by: Hailong Cui * add unit test Signed-off-by: Hailong Cui * fix can't delete workspace due to invalid permission Signed-off-by: Hailong Cui --------- Signed-off-by: Hailong Cui --- src/core/server/index.ts | 1 + .../service/lib/repository.mock.ts | 1 + .../service/lib/repository.test.js | 79 +++++++++++++++++++ .../saved_objects/service/lib/repository.ts | 50 ++++++++++++ .../service/saved_objects_client.test.js | 15 ++++ .../service/saved_objects_client.ts | 21 +++++ .../server/integration_tests/routes.test.ts | 40 +++++++++- src/plugins/workspace/server/routes/index.ts | 1 + .../workspace/server/workspace_client.ts | 18 ++++- 9 files changed, 224 insertions(+), 2 deletions(-) diff --git a/src/core/server/index.ts b/src/core/server/index.ts index f8526a96a7c2..d3df420710e5 100644 --- a/src/core/server/index.ts +++ b/src/core/server/index.ts @@ -321,6 +321,7 @@ export { exportSavedObjectsToStream, importSavedObjectsFromStream, resolveSavedObjectsImportErrors, + SavedObjectsDeleteByWorkspaceOptions, } from './saved_objects'; export { diff --git a/src/core/server/saved_objects/service/lib/repository.mock.ts b/src/core/server/saved_objects/service/lib/repository.mock.ts index b9436b364f05..1271bca35129 100644 --- a/src/core/server/saved_objects/service/lib/repository.mock.ts +++ b/src/core/server/saved_objects/service/lib/repository.mock.ts @@ -44,6 +44,7 @@ const create = (): jest.Mocked => ({ deleteFromNamespaces: jest.fn(), deleteByNamespace: jest.fn(), incrementCounter: jest.fn(), + deleteByWorkspace: jest.fn(), }); export const savedObjectsRepositoryMock = { create }; diff --git a/src/core/server/saved_objects/service/lib/repository.test.js b/src/core/server/saved_objects/service/lib/repository.test.js index e50332ae514a..cab8c4b613db 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.js +++ b/src/core/server/saved_objects/service/lib/repository.test.js @@ -2479,6 +2479,85 @@ describe('SavedObjectsRepository', () => { }); }); + describe('#deleteByWorkspace', () => { + const workspace = 'bar-workspace'; + const mockUpdateResults = { + took: 15, + timed_out: false, + total: 3, + updated: 2, + deleted: 1, + batches: 1, + version_conflicts: 0, + noops: 0, + retries: { bulk: 0, search: 0 }, + throttled_millis: 0, + requests_per_second: -1.0, + throttled_until_millis: 0, + failures: [], + }; + + const deleteByWorkspaceSuccess = async (workspace, options) => { + client.updateByQuery.mockResolvedValueOnce( + opensearchClientMock.createSuccessTransportRequestPromise(mockUpdateResults) + ); + const result = await savedObjectsRepository.deleteByWorkspace(workspace, options); + expect(getSearchDslNS.getSearchDsl).toHaveBeenCalledTimes(1); + expect(client.updateByQuery).toHaveBeenCalledTimes(1); + return result; + }; + + describe('client calls', () => { + it(`should use the OpenSearch updateByQuery action`, async () => { + await deleteByWorkspaceSuccess(workspace); + expect(client.updateByQuery).toHaveBeenCalledTimes(1); + }); + + it(`should use all indices for all types`, async () => { + await deleteByWorkspaceSuccess(workspace); + expect(client.updateByQuery).toHaveBeenCalledWith( + expect.objectContaining({ index: ['.opensearch_dashboards_test', 'custom'] }), + expect.anything() + ); + }); + }); + + describe('errors', () => { + it(`throws when workspace is not a string or is '*'`, async () => { + const test = async (workspace) => { + await expect(savedObjectsRepository.deleteByWorkspace(workspace)).rejects.toThrowError( + `workspace is required, and must be a string that is not equal to '*'` + ); + expect(client.updateByQuery).not.toHaveBeenCalled(); + }; + await test(undefined); + await test(null); + await test(['foo-workspace']); + await test(123); + await test(true); + await test(ALL_NAMESPACES_STRING); + }); + }); + + describe('returns', () => { + it(`returns the query results on success`, async () => { + const result = await deleteByWorkspaceSuccess(workspace); + expect(result).toEqual(mockUpdateResults); + }); + }); + + describe('search dsl', () => { + it(`constructs a query that have workspace as search critieria`, async () => { + await deleteByWorkspaceSuccess(workspace); + const allTypes = registry.getAllTypes().map((type) => type.name); + expect(getSearchDslNS.getSearchDsl).toHaveBeenCalledWith(mappings, registry, { + workspaces: [workspace], + type: allTypes, + }); + }); + }); + }); + describe('#find', () => { const generateSearchResults = (namespace) => { return { diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index 1a4feab322b3..7738082245cd 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -68,6 +68,7 @@ import { SavedObjectsAddToNamespacesResponse, SavedObjectsDeleteFromNamespacesOptions, SavedObjectsDeleteFromNamespacesResponse, + SavedObjectsDeleteByWorkspaceOptions, } from '../saved_objects_client'; import { SavedObject, @@ -711,6 +712,55 @@ export class SavedObjectsRepository { return body; } + /** + * Deletes all objects from the provided workspace. It used when deleting a workspace. + * + * @param {string} workspace + * @param options SavedObjectsDeleteByWorkspaceOptions + * @returns {promise} - { took, timed_out, total, deleted, batches, version_conflicts, noops, retries, failures } + */ + async deleteByWorkspace( + workspace: string, + options: SavedObjectsDeleteByWorkspaceOptions = {} + ): Promise { + if (!workspace || typeof workspace !== 'string' || workspace === '*') { + throw new TypeError(`workspace is required, and must be a string that is not equal to '*'`); + } + + const allTypes = Object.keys(getRootPropertiesObjects(this._mappings)); + + const { body } = await this.client.updateByQuery( + { + index: this.getIndicesForTypes(allTypes), + refresh: options.refresh, + body: { + script: { + source: ` + if (!ctx._source.containsKey('workspaces')) { + ctx.op = "delete"; + } else { + ctx._source['workspaces'].removeAll(Collections.singleton(params['workspace'])); + if (ctx._source['workspaces'].empty) { + ctx.op = "delete"; + } + } + `, + lang: 'painless', + params: { workspace }, + }, + conflicts: 'proceed', + ...getSearchDsl(this._mappings, this._registry, { + workspaces: [workspace], + type: allTypes, + }), + }, + }, + { ignore: [404] } + ); + + return body; + } + /** * @param {object} [options={}] * @property {(string|Array)} [options.type] diff --git a/src/core/server/saved_objects/service/saved_objects_client.test.js b/src/core/server/saved_objects/service/saved_objects_client.test.js index d22ffa502f79..676b1a37e051 100644 --- a/src/core/server/saved_objects/service/saved_objects_client.test.js +++ b/src/core/server/saved_objects/service/saved_objects_client.test.js @@ -207,3 +207,18 @@ test(`#deleteFromNamespaces`, async () => { expect(mockRepository.deleteFromNamespaces).toHaveBeenCalledWith(type, id, namespaces, options); expect(result).toBe(returnValue); }); + +test(`#deleteByWorkspace`, async () => { + const returnValue = Symbol(); + const mockRepository = { + deleteByWorkspace: jest.fn().mockResolvedValue(returnValue), + }; + const client = new SavedObjectsClient(mockRepository); + + const workspace = Symbol(); + const options = Symbol(); + const result = await client.deleteByWorkspace(workspace, options); + + expect(mockRepository.deleteByWorkspace).toHaveBeenCalledWith(workspace, options); + expect(result).toBe(returnValue); +}); 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 d3edb0d98845..da62df384c1c 100644 --- a/src/core/server/saved_objects/service/saved_objects_client.ts +++ b/src/core/server/saved_objects/service/saved_objects_client.ts @@ -285,6 +285,15 @@ export interface SavedObjectsUpdateResponse references: SavedObjectReference[] | undefined; } +/** + * + * @public + */ +export interface SavedObjectsDeleteByWorkspaceOptions extends SavedObjectsBaseOptions { + /** The OpenSearch supports only boolean flag for this operation */ + refresh?: boolean; +} + /** * * @public @@ -441,6 +450,18 @@ export class SavedObjectsClient { return await this._repository.deleteFromNamespaces(type, id, namespaces, options); } + /** + * delete saved objects by workspace id + * @param workspace + * @param options + */ + deleteByWorkspace = async ( + workspace: string, + options: SavedObjectsDeleteByWorkspaceOptions = {} + ): Promise => { + return await this._repository.deleteByWorkspace(workspace, options); + }; + /** * Bulk Updates multiple SavedObject at once * diff --git a/src/plugins/workspace/server/integration_tests/routes.test.ts b/src/plugins/workspace/server/integration_tests/routes.test.ts index e14aa3de16a3..a36d187b405f 100644 --- a/src/plugins/workspace/server/integration_tests/routes.test.ts +++ b/src/plugins/workspace/server/integration_tests/routes.test.ts @@ -5,6 +5,7 @@ import { WorkspaceAttribute } from 'src/core/types'; import * as osdTestServer from '../../../../core/test_helpers/osd_server'; +import { WORKSPACE_TYPE } from '../../../../core/server'; const omitId = (object: T): Omit => { const { id, ...others } = object; @@ -28,6 +29,7 @@ describe('workspace service', () => { workspace: { enabled: true, }, + migrations: { skip: false }, }, }, }); @@ -49,7 +51,10 @@ describe('workspace service', () => { .expect(200); await Promise.all( listResult.body.result.workspaces.map((item: WorkspaceAttribute) => - osdTestServer.request.delete(root, `/api/workspaces/${item.id}`).expect(200) + // this will delete reserved workspace + osdTestServer.request + .delete(root, `/api/saved_objects/${WORKSPACE_TYPE}/${item.id}`) + .expect(200) ) ); }); @@ -119,6 +124,16 @@ describe('workspace service', () => { }) .expect(200); + await osdTestServer.request + .post(root, `/api/saved_objects/index-pattern/logstash-*`) + .send({ + attributes: { + title: 'logstash-*', + }, + workspaces: [result.body.result.id], + }) + .expect(200); + await osdTestServer.request .delete(root, `/api/workspaces/${result.body.result.id}`) .expect(200); @@ -129,6 +144,29 @@ describe('workspace service', () => { ); expect(getResult.body.success).toEqual(false); + + // saved objects been deleted + await osdTestServer.request + .get(root, `/api/saved_objects/index-pattern/logstash-*`) + .expect(404); + }); + it('delete reserved workspace', async () => { + const reservedWorkspace: WorkspaceAttribute = { ...testWorkspace, reserved: true }; + const result: any = await osdTestServer.request + .post(root, `/api/workspaces`) + .send({ + attributes: omit(reservedWorkspace, 'id'), + }) + .expect(200); + + const deleteResult = await osdTestServer.request + .delete(root, `/api/workspaces/${result.body.result.id}`) + .expect(200); + + expect(deleteResult.body.success).toEqual(false); + expect(deleteResult.body.error).toEqual( + `Reserved workspace ${result.body.result.id} is not allowed to delete.` + ); }); it('list', async () => { await osdTestServer.request diff --git a/src/plugins/workspace/server/routes/index.ts b/src/plugins/workspace/server/routes/index.ts index 5789aa0481fa..7c090be675f8 100644 --- a/src/plugins/workspace/server/routes/index.ts +++ b/src/plugins/workspace/server/routes/index.ts @@ -16,6 +16,7 @@ const workspaceAttributesSchema = schema.object({ color: schema.maybe(schema.string()), icon: schema.maybe(schema.string()), defaultVISTheme: schema.maybe(schema.string()), + reserved: schema.maybe(schema.boolean()), }); export function registerRoutes({ diff --git a/src/plugins/workspace/server/workspace_client.ts b/src/plugins/workspace/server/workspace_client.ts index 890cf9bdd8a0..092f3e528d4f 100644 --- a/src/plugins/workspace/server/workspace_client.ts +++ b/src/plugins/workspace/server/workspace_client.ts @@ -186,7 +186,23 @@ export class WorkspaceClient implements IWorkspaceClientImpl { } public async delete(requestDetail: IRequestDetail, id: string): Promise> { try { - await this.getSavedObjectClientsFromRequestDetail(requestDetail).delete(WORKSPACE_TYPE, id); + const savedObjectClient = this.getSavedObjectClientsFromRequestDetail(requestDetail); + const workspaceInDB: SavedObject = await savedObjectClient.get( + WORKSPACE_TYPE, + id + ); + if (workspaceInDB.attributes.reserved) { + return { + success: false, + error: i18n.translate('workspace.deleteReservedWorkspace.errorMessage', { + defaultMessage: 'Reserved workspace {id} is not allowed to delete.', + values: { id: workspaceInDB.id }, + }), + }; + } + await savedObjectClient.deleteByWorkspace(id); + // delete workspace itself at last, deleteByWorkspace depends on the workspace to do permission check + await savedObjectClient.delete(WORKSPACE_TYPE, id); return { success: true, result: true, From 15b93b3db8fb0665f0958fb156e54d84458c4871 Mon Sep 17 00:00:00 2001 From: Hailong Cui Date: Mon, 4 Mar 2024 15:02:04 +0800 Subject: [PATCH 2/5] update test case Signed-off-by: Hailong Cui --- .../server/integration_tests/routes.test.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/plugins/workspace/server/integration_tests/routes.test.ts b/src/plugins/workspace/server/integration_tests/routes.test.ts index a36d187b405f..061d0f3c4064 100644 --- a/src/plugins/workspace/server/integration_tests/routes.test.ts +++ b/src/plugins/workspace/server/integration_tests/routes.test.ts @@ -21,6 +21,7 @@ const testWorkspace: WorkspaceAttribute = { describe('workspace service', () => { let root: ReturnType; let opensearchServer: osdTestServer.TestOpenSearchUtils; + let osd: osdTestServer.TestOpenSearchDashboardsUtils; beforeAll(async () => { const { startOpenSearch, startOpenSearchDashboards } = osdTestServer.createTestServers({ adjustTimeout: (t: number) => jest.setTimeout(t), @@ -34,8 +35,8 @@ describe('workspace service', () => { }, }); opensearchServer = await startOpenSearch(); - const startOSDResp = await startOpenSearchDashboards(); - root = startOSDResp.root; + osd = await startOpenSearchDashboards(); + root = osd.root; }); afterAll(async () => { await root.shutdown(); @@ -49,12 +50,13 @@ describe('workspace service', () => { page: 1, }) .expect(200); + const savedObjectsRepository = osd.coreStart.savedObjects.createInternalRepository([ + WORKSPACE_TYPE, + ]); await Promise.all( listResult.body.result.workspaces.map((item: WorkspaceAttribute) => // this will delete reserved workspace - osdTestServer.request - .delete(root, `/api/saved_objects/${WORKSPACE_TYPE}/${item.id}`) - .expect(200) + savedObjectsRepository.delete(WORKSPACE_TYPE, item.id) ) ); }); @@ -155,7 +157,7 @@ describe('workspace service', () => { const result: any = await osdTestServer.request .post(root, `/api/workspaces`) .send({ - attributes: omit(reservedWorkspace, 'id'), + attributes: omitId(reservedWorkspace), }) .expect(200); From c82eae27765687d8bc5225a460626b6abf7caf6f Mon Sep 17 00:00:00 2001 From: Hailong Cui Date: Mon, 4 Mar 2024 15:14:23 +0800 Subject: [PATCH 3/5] Add change log Signed-off-by: Hailong Cui --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 14fba16ff10a..f8d651dc68ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,8 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ### 📈 Features/Enhancements +[Workspace] Add delete saved objects by workspace functionality([#6013](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6013)) + ### 🐛 Bug Fixes - [BUG][Discover] Allow saved sort from search embeddable to load in Dashboard ([#5934](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5934)) From f1fcfd4cf58889048a1c97739d7256226b67be24 Mon Sep 17 00:00:00 2001 From: Hailong Cui Date: Mon, 4 Mar 2024 15:16:07 +0800 Subject: [PATCH 4/5] update change log format Signed-off-by: Hailong Cui --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f8d651dc68ba..5d61b80f28a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ### 📈 Features/Enhancements -[Workspace] Add delete saved objects by workspace functionality([#6013](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6013)) +- [Workspace] Add delete saved objects by workspace functionality([#6013](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6013)) ### 🐛 Bug Fixes From 99ea7188e2fee8f8cf6c4e95e062ce79484083de Mon Sep 17 00:00:00 2001 From: Hailong Cui Date: Thu, 7 Mar 2024 10:42:45 +0800 Subject: [PATCH 5/5] update method comments to make it more clear Signed-off-by: Hailong Cui --- src/core/server/saved_objects/service/lib/repository.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index 95f529e1683c..15dcf7a6c122 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -716,9 +716,9 @@ export class SavedObjectsRepository { } /** - * Deletes all objects from the provided workspace. It used when deleting a workspace. + * Deletes all objects from the provided workspace. * - * @param {string} workspace + * @param {string} workspace - workspace id * @param options SavedObjectsDeleteByWorkspaceOptions * @returns {promise} - { took, timed_out, total, deleted, batches, version_conflicts, noops, retries, failures } */