From bf9821d4025cd04b06e0679f7a6b834d70a553ba Mon Sep 17 00:00:00 2001 From: Hailong Cui Date: Fri, 13 Oct 2023 09:55:19 +0800 Subject: [PATCH] [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,