From f0ad14134a36a8ced395852f1601818bb0f71052 Mon Sep 17 00:00:00 2001 From: Hailong Cui Date: Mon, 11 Mar 2024 13:23:20 +0800 Subject: [PATCH] [Workspace] Delete saved objects by workspace (#6013) * [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 * update test case Signed-off-by: Hailong Cui * Add change log Signed-off-by: Hailong Cui * update change log format Signed-off-by: Hailong Cui * update method comments to make it more clear Signed-off-by: Hailong Cui --------- Signed-off-by: Hailong Cui (cherry picked from commit 735424bcaabfc752453c4d514ddd5d4b4f1b6d6d) --- CHANGELOG.md | 2 + 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 | 46 ++++++++++- src/plugins/workspace/server/routes/index.ts | 1 + .../workspace/server/workspace_client.ts | 18 ++++- 10 files changed, 230 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 539c5aa3de5..7a536f59f2f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,8 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Multiple Datasource] Add TLS configuration for multiple data sources ([#6171](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6171)) +- [Workspace] Add delete saved objects by workspace functionality([#6013](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6013)) + ### 🐛 Bug Fixes ### 🚞 Infrastructure diff --git a/src/core/server/index.ts b/src/core/server/index.ts index 8b101ba9cf3..a1e44c959ec 100644 --- a/src/core/server/index.ts +++ b/src/core/server/index.ts @@ -306,6 +306,7 @@ export { ISavedObjectsRepository, SavedObjectsRepository, SavedObjectsDeleteByNamespaceOptions, + SavedObjectsDeleteByWorkspaceOptions, SavedObjectsIncrementCounterOptions, SavedObjectsFieldMapping, SavedObjectsTypeMappingDefinition, 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 b9436b364f0..1271bca3512 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 b793046d9a9..96883e55320 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.js +++ b/src/core/server/saved_objects/service/lib/repository.test.js @@ -2585,6 +2585,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 5340008f06a..15dcf7a6c12 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, @@ -714,6 +715,55 @@ export class SavedObjectsRepository { return body; } + /** + * Deletes all objects from the provided workspace. + * + * @param {string} workspace - workspace id + * @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 d22ffa502f7..676b1a37e05 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 49ce55c824d..05069d9d887 100644 --- a/src/core/server/saved_objects/service/saved_objects_client.ts +++ b/src/core/server/saved_objects/service/saved_objects_client.ts @@ -290,6 +290,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 @@ -446,6 +455,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 e14aa3de16a..061d0f3c406 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; @@ -20,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), @@ -28,12 +30,13 @@ describe('workspace service', () => { workspace: { enabled: true, }, + migrations: { skip: false }, }, }, }); opensearchServer = await startOpenSearch(); - const startOSDResp = await startOpenSearchDashboards(); - root = startOSDResp.root; + osd = await startOpenSearchDashboards(); + root = osd.root; }); afterAll(async () => { await root.shutdown(); @@ -47,9 +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) => - osdTestServer.request.delete(root, `/api/workspaces/${item.id}`).expect(200) + // this will delete reserved workspace + savedObjectsRepository.delete(WORKSPACE_TYPE, item.id) ) ); }); @@ -119,6 +126,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 +146,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: omitId(reservedWorkspace), + }) + .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 5789aa0481f..7c090be675f 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 890cf9bdd8a..092f3e528d4 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,