From 1d8088ea0ccb467f8c8e29553f7c3a05a6f8a363 Mon Sep 17 00:00:00 2001 From: gaobinlong Date: Fri, 29 Sep 2023 07:38:42 +0800 Subject: [PATCH] [Workspace] Add optional workspaces parameter to all saved objects API (#185) * [Workspace] Add workspaces parameters to all saved objects API Signed-off-by: gaobinlong * feat: update snapshot Signed-off-by: SuZhou-Joe * feat: optimize logic when checkConflict and bulkCreate (#189) * feat: optimize logic when checkConflict and bulkCreate Signed-off-by: SuZhou-Joe * feat: add options.workspace check Signed-off-by: SuZhou-Joe * feat: throw error when workspace check error in repository create Signed-off-by: SuZhou-Joe * feat: modify judgement Signed-off-by: SuZhou-Joe * feat: always get objects from DB when create-with-override Signed-off-by: SuZhou-Joe --------- Signed-off-by: SuZhou-Joe * feat: call get when create with override Signed-off-by: SuZhou-Joe * feat: update test according to count Signed-off-by: SuZhou-Joe * feat: add integration test Signed-off-by: SuZhou-Joe * fix: unit test Signed-off-by: SuZhou-Joe * feat: regenerate ids when import Signed-off-by: SuZhou-Joe * feat: add more unit test Signed-off-by: SuZhou-Joe * feat: minor changes logic on repository Signed-off-by: SuZhou-Joe * feat: update unit test Signed-off-by: SuZhou-Joe * feat: update test Signed-off-by: SuZhou-Joe * feat: optimization according to comments Signed-off-by: SuZhou-Joe * feat: update test Signed-off-by: SuZhou-Joe * feat: optimize code Signed-off-by: SuZhou-Joe --------- Signed-off-by: gaobinlong Signed-off-by: SuZhou-Joe Co-authored-by: SuZhou-Joe --- .../export/get_sorted_objects_for_export.ts | 4 +- .../import/import_saved_objects.test.ts | 12 +- .../import/import_saved_objects.ts | 12 +- .../import/regenerate_ids.test.ts | 71 +++- .../saved_objects/import/regenerate_ids.ts | 35 +- .../build_active_mappings.test.ts.snap | 8 + .../migrations/core/build_active_mappings.ts | 3 + .../migrations/core/index_migrator.test.ts | 12 + ...pensearch_dashboards_migrator.test.ts.snap | 4 + .../routes/integration_tests/find.test.ts | 34 ++ .../routes/resolve_import_errors.ts | 1 + .../lib/integration_tests/repository.test.ts | 310 ++++++++++++++++++ .../service/lib/repository.test.js | 147 ++++++++- .../saved_objects/service/lib/repository.ts | 47 ++- .../service/lib/search_dsl/query_params.ts | 14 + .../server/saved_objects/service/lib/utils.ts | 7 + .../service/saved_objects_client.ts | 2 +- 17 files changed, 701 insertions(+), 22 deletions(-) create mode 100644 src/core/server/saved_objects/service/lib/integration_tests/repository.test.ts 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 ea944ff3307a..660f86846137 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 @@ -109,7 +109,9 @@ async function fetchObjectsToExport({ if (typeof search === 'string') { throw Boom.badRequest(`Can't specify both "search" and "objects" properties when exporting`); } - const bulkGetResult = await savedObjectsClient.bulkGet(objects, { namespace }); + const bulkGetResult = await savedObjectsClient.bulkGet(objects, { + namespace, + }); const erroredObjects = bulkGetResult.saved_objects.filter((obj) => !!obj.error); if (erroredObjects.length) { const err = Boom.badRequest(); diff --git a/src/core/server/saved_objects/import/import_saved_objects.test.ts b/src/core/server/saved_objects/import/import_saved_objects.test.ts index 3dda6931bd1e..dcb8d685d42c 100644 --- a/src/core/server/saved_objects/import/import_saved_objects.test.ts +++ b/src/core/server/saved_objects/import/import_saved_objects.test.ts @@ -42,7 +42,7 @@ import { typeRegistryMock } from '../saved_objects_type_registry.mock'; import { importSavedObjectsFromStream } from './import_saved_objects'; import { collectSavedObjects } from './collect_saved_objects'; -import { regenerateIds } from './regenerate_ids'; +import { regenerateIds, regenerateIdsWithReference } from './regenerate_ids'; import { validateReferences } from './validate_references'; import { checkConflicts } from './check_conflicts'; import { checkOriginConflicts } from './check_origin_conflicts'; @@ -70,6 +70,7 @@ describe('#importSavedObjectsFromStream', () => { importIdMap: new Map(), }); getMockFn(regenerateIds).mockReturnValue(new Map()); + getMockFn(regenerateIdsWithReference).mockReturnValue(Promise.resolve(new Map())); getMockFn(validateReferences).mockResolvedValue([]); getMockFn(checkConflicts).mockResolvedValue({ errors: [], @@ -278,6 +279,15 @@ describe('#importSavedObjectsFromStream', () => { ]), }); getMockFn(validateReferences).mockResolvedValue([errors[1]]); + getMockFn(regenerateIdsWithReference).mockResolvedValue( + Promise.resolve( + new Map([ + ['foo', {}], + ['bar', {}], + ['baz', {}], + ]) + ) + ); getMockFn(checkConflicts).mockResolvedValue({ errors: [errors[2]], filteredObjects, diff --git a/src/core/server/saved_objects/import/import_saved_objects.ts b/src/core/server/saved_objects/import/import_saved_objects.ts index f2833c198e1b..0222223a1751 100644 --- a/src/core/server/saved_objects/import/import_saved_objects.ts +++ b/src/core/server/saved_objects/import/import_saved_objects.ts @@ -38,7 +38,7 @@ import { validateReferences } from './validate_references'; import { checkOriginConflicts } from './check_origin_conflicts'; import { createSavedObjects } from './create_saved_objects'; import { checkConflicts } from './check_conflicts'; -import { regenerateIds } from './regenerate_ids'; +import { regenerateIds, regenerateIdsWithReference } from './regenerate_ids'; import { checkConflictsForDataSource } from './check_conflict_for_data_source'; /** @@ -87,6 +87,16 @@ export async function importSavedObjectsFromStream({ importIdMap = regenerateIds(collectSavedObjectsResult.collectedObjects, dataSourceId); } else { // in check conclict and override mode + + if (workspaces) { + importIdMap = await regenerateIdsWithReference({ + savedObjects: collectSavedObjectsResult.collectedObjects, + savedObjectsClient, + workspaces, + objectLimit, + importIdMap, + }); + } // Check single-namespace objects for conflicts in this namespace, and check multi-namespace objects for conflicts across all namespaces const checkConflictsParams = { objects: collectSavedObjectsResult.collectedObjects, diff --git a/src/core/server/saved_objects/import/regenerate_ids.test.ts b/src/core/server/saved_objects/import/regenerate_ids.test.ts index c7dbfb8b50bc..895b90a89324 100644 --- a/src/core/server/saved_objects/import/regenerate_ids.test.ts +++ b/src/core/server/saved_objects/import/regenerate_ids.test.ts @@ -29,8 +29,10 @@ */ import { mockUuidv4 } from './__mocks__'; -import { regenerateIds } from './regenerate_ids'; +import { regenerateIds, regenerateIdsWithReference } from './regenerate_ids'; import { SavedObject } from '../types'; +import { savedObjectsClientMock } from '../service/saved_objects_client.mock'; +import { SavedObjectsBulkResponse } from '../service'; describe('#regenerateIds', () => { const objects = ([ @@ -68,3 +70,70 @@ describe('#regenerateIds', () => { `); }); }); + +describe('#regenerateIdsWithReference', () => { + const objects = ([ + { type: 'foo', id: '1' }, + { type: 'bar', id: '2' }, + { type: 'baz', id: '3' }, + ] as any) as SavedObject[]; + + test('returns expected values', async () => { + const mockedSavedObjectsClient = savedObjectsClientMock.create(); + mockUuidv4.mockReturnValueOnce('uuidv4 #1'); + const result: SavedObjectsBulkResponse = { + saved_objects: [ + { + error: { + statusCode: 404, + error: '', + message: '', + }, + id: '1', + type: 'foo', + attributes: {}, + references: [], + }, + { + id: '2', + type: 'bar', + attributes: {}, + references: [], + workspaces: ['bar'], + }, + { + id: '3', + type: 'baz', + attributes: {}, + references: [], + workspaces: ['foo'], + }, + ], + }; + mockedSavedObjectsClient.bulkGet.mockResolvedValue(result); + expect( + await regenerateIdsWithReference({ + savedObjects: objects, + savedObjectsClient: mockedSavedObjectsClient, + workspaces: ['bar'], + objectLimit: 1000, + importIdMap: new Map(), + }) + ).toMatchInlineSnapshot(` + Map { + "foo:1" => Object { + "id": "1", + "omitOriginId": true, + }, + "bar:2" => Object { + "id": "2", + "omitOriginId": false, + }, + "baz:3" => Object { + "id": "uuidv4 #1", + "omitOriginId": true, + }, + } + `); + }); +}); diff --git a/src/core/server/saved_objects/import/regenerate_ids.ts b/src/core/server/saved_objects/import/regenerate_ids.ts index f1092bed7f55..bf89fad824d6 100644 --- a/src/core/server/saved_objects/import/regenerate_ids.ts +++ b/src/core/server/saved_objects/import/regenerate_ids.ts @@ -29,7 +29,8 @@ */ import { v4 as uuidv4 } from 'uuid'; -import { SavedObject } from '../types'; +import { SavedObject, SavedObjectsClientContract } from '../types'; +import { SavedObjectsUtils } from '../service'; /** * Takes an array of saved objects and returns an importIdMap of randomly-generated new IDs. @@ -47,3 +48,35 @@ export const regenerateIds = (objects: SavedObject[], dataSourceId: string | und }, new Map()); return importIdMap; }; + +export const regenerateIdsWithReference = async (props: { + savedObjects: SavedObject[]; + savedObjectsClient: SavedObjectsClientContract; + workspaces: string[]; + objectLimit: number; + importIdMap: Map; +}): Promise> => { + const { savedObjects, savedObjectsClient, workspaces, importIdMap } = props; + + const bulkGetResult = await savedObjectsClient.bulkGet( + savedObjects.map((item) => ({ type: item.type, id: item.id })) + ); + + return bulkGetResult.saved_objects.reduce((acc, object) => { + if (object.error?.statusCode === 404) { + acc.set(`${object.type}:${object.id}`, { id: object.id, omitOriginId: true }); + return acc; + } + + const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToBaseWorkspaces( + workspaces, + object.workspaces + ); + if (filteredWorkspaces.length) { + acc.set(`${object.type}:${object.id}`, { id: uuidv4(), omitOriginId: true }); + } else { + acc.set(`${object.type}:${object.id}`, { id: object.id, omitOriginId: false }); + } + return acc; + }, importIdMap); +}; diff --git a/src/core/server/saved_objects/migrations/core/__snapshots__/build_active_mappings.test.ts.snap b/src/core/server/saved_objects/migrations/core/__snapshots__/build_active_mappings.test.ts.snap index 6f67893104e7..09e8ad8b5407 100644 --- a/src/core/server/saved_objects/migrations/core/__snapshots__/build_active_mappings.test.ts.snap +++ b/src/core/server/saved_objects/migrations/core/__snapshots__/build_active_mappings.test.ts.snap @@ -14,6 +14,7 @@ Object { "references": "7997cf5a56cc02bdc9c93361bde732b0", "type": "2f4316de49999235636386fe51dc06c1", "updated_at": "00da57df13e94e9d98437d13ace4bfe0", + "workspaces": "2f4316de49999235636386fe51dc06c1", }, }, "dynamic": "strict", @@ -111,6 +112,9 @@ Object { "updated_at": Object { "type": "date", }, + "workspaces": Object { + "type": "keyword", + }, }, } `; @@ -130,6 +134,7 @@ Object { "thirdType": "510f1f0adb69830cf8a1c5ce2923ed82", "type": "2f4316de49999235636386fe51dc06c1", "updated_at": "00da57df13e94e9d98437d13ace4bfe0", + "workspaces": "2f4316de49999235636386fe51dc06c1", }, }, "dynamic": "strict", @@ -244,6 +249,9 @@ Object { "updated_at": Object { "type": "date", }, + "workspaces": Object { + "type": "keyword", + }, }, } `; diff --git a/src/core/server/saved_objects/migrations/core/build_active_mappings.ts b/src/core/server/saved_objects/migrations/core/build_active_mappings.ts index efedd9351a22..fac64bf78b01 100644 --- a/src/core/server/saved_objects/migrations/core/build_active_mappings.ts +++ b/src/core/server/saved_objects/migrations/core/build_active_mappings.ts @@ -219,6 +219,9 @@ function defaultMapping(): IndexMapping { }, }, }, + workspaces: { + type: 'keyword', + }, permissions: { properties: { read: principals, diff --git a/src/core/server/saved_objects/migrations/core/index_migrator.test.ts b/src/core/server/saved_objects/migrations/core/index_migrator.test.ts index f22234fc8996..de2e14572278 100644 --- a/src/core/server/saved_objects/migrations/core/index_migrator.test.ts +++ b/src/core/server/saved_objects/migrations/core/index_migrator.test.ts @@ -231,6 +231,7 @@ describe('IndexMigrator', () => { references: '7997cf5a56cc02bdc9c93361bde732b0', type: '2f4316de49999235636386fe51dc06c1', updated_at: '00da57df13e94e9d98437d13ace4bfe0', + workspaces: '2f4316de49999235636386fe51dc06c1', }, }, properties: { @@ -241,6 +242,9 @@ describe('IndexMigrator', () => { originId: { type: 'keyword' }, type: { type: 'keyword' }, updated_at: { type: 'date' }, + workspaces: { + type: 'keyword', + }, permissions: { properties: { library_read: { @@ -383,6 +387,7 @@ describe('IndexMigrator', () => { references: '7997cf5a56cc02bdc9c93361bde732b0', type: '2f4316de49999235636386fe51dc06c1', updated_at: '00da57df13e94e9d98437d13ace4bfe0', + workspaces: '2f4316de49999235636386fe51dc06c1', }, }, properties: { @@ -394,6 +399,9 @@ describe('IndexMigrator', () => { originId: { type: 'keyword' }, type: { type: 'keyword' }, updated_at: { type: 'date' }, + workspaces: { + type: 'keyword', + }, permissions: { properties: { library_read: { @@ -479,6 +487,7 @@ describe('IndexMigrator', () => { references: '7997cf5a56cc02bdc9c93361bde732b0', type: '2f4316de49999235636386fe51dc06c1', updated_at: '00da57df13e94e9d98437d13ace4bfe0', + workspaces: '2f4316de49999235636386fe51dc06c1', }, }, properties: { @@ -490,6 +499,9 @@ describe('IndexMigrator', () => { originId: { type: 'keyword' }, type: { type: 'keyword' }, updated_at: { type: 'date' }, + workspaces: { + type: 'keyword', + }, permissions: { properties: { library_read: { diff --git a/src/core/server/saved_objects/migrations/opensearch_dashboards/__snapshots__/opensearch_dashboards_migrator.test.ts.snap b/src/core/server/saved_objects/migrations/opensearch_dashboards/__snapshots__/opensearch_dashboards_migrator.test.ts.snap index 5e39af788d79..2748ad2eaf6a 100644 --- a/src/core/server/saved_objects/migrations/opensearch_dashboards/__snapshots__/opensearch_dashboards_migrator.test.ts.snap +++ b/src/core/server/saved_objects/migrations/opensearch_dashboards/__snapshots__/opensearch_dashboards_migrator.test.ts.snap @@ -14,6 +14,7 @@ Object { "references": "7997cf5a56cc02bdc9c93361bde732b0", "type": "2f4316de49999235636386fe51dc06c1", "updated_at": "00da57df13e94e9d98437d13ace4bfe0", + "workspaces": "2f4316de49999235636386fe51dc06c1", }, }, "dynamic": "strict", @@ -119,6 +120,9 @@ Object { "updated_at": Object { "type": "date", }, + "workspaces": Object { + "type": "keyword", + }, }, } `; diff --git a/src/core/server/saved_objects/routes/integration_tests/find.test.ts b/src/core/server/saved_objects/routes/integration_tests/find.test.ts index fc21eefed434..b21425386400 100644 --- a/src/core/server/saved_objects/routes/integration_tests/find.test.ts +++ b/src/core/server/saved_objects/routes/integration_tests/find.test.ts @@ -288,4 +288,38 @@ describe('GET /api/saved_objects/_find', () => { defaultSearchOperator: 'OR', }); }); + + it('accepts the query parameter workspaces as a string', async () => { + await supertest(httpSetup.server.listener) + .get('/api/saved_objects/_find?type=index-pattern&workspaces=foo') + .expect(200); + + expect(savedObjectsClient.find).toHaveBeenCalledTimes(1); + + const options = savedObjectsClient.find.mock.calls[0][0]; + expect(options).toEqual({ + defaultSearchOperator: 'OR', + perPage: 20, + page: 1, + type: ['index-pattern'], + workspaces: ['foo'], + }); + }); + + it('accepts the query parameter workspaces as an array', async () => { + await supertest(httpSetup.server.listener) + .get('/api/saved_objects/_find?type=index-pattern&workspaces=default&workspaces=foo') + .expect(200); + + expect(savedObjectsClient.find).toHaveBeenCalledTimes(1); + + const options = savedObjectsClient.find.mock.calls[0][0]; + expect(options).toEqual({ + perPage: 20, + page: 1, + type: ['index-pattern'], + workspaces: ['default', 'foo'], + defaultSearchOperator: 'OR', + }); + }); }); diff --git a/src/core/server/saved_objects/routes/resolve_import_errors.ts b/src/core/server/saved_objects/routes/resolve_import_errors.ts index 6bc667eba0df..7d22e35a658d 100644 --- a/src/core/server/saved_objects/routes/resolve_import_errors.ts +++ b/src/core/server/saved_objects/routes/resolve_import_errors.ts @@ -135,6 +135,7 @@ export const registerResolveImportErrorsRoute = (router: IRouter, config: SavedO workspaces, dataSourceId, dataSourceTitle, + workspaces, }); return res.ok({ body: result }); diff --git a/src/core/server/saved_objects/service/lib/integration_tests/repository.test.ts b/src/core/server/saved_objects/service/lib/integration_tests/repository.test.ts new file mode 100644 index 000000000000..b601de985dc0 --- /dev/null +++ b/src/core/server/saved_objects/service/lib/integration_tests/repository.test.ts @@ -0,0 +1,310 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ +import { SavedObject } from 'src/core/types'; +import { isEqual } from 'lodash'; +import * as osdTestServer from '../../../../../test_helpers/osd_server'; +import { Readable } from 'stream'; + +const dashboard: Omit = { + type: 'dashboard', + attributes: {}, + references: [], +}; + +describe('repository integration test', () => { + let root: ReturnType; + let opensearchServer: osdTestServer.TestOpenSearchUtils; + beforeAll(async () => { + const { startOpenSearch, startOpenSearchDashboards } = osdTestServer.createTestServers({ + adjustTimeout: (t: number) => jest.setTimeout(t), + }); + opensearchServer = await startOpenSearch(); + const startOSDResp = await startOpenSearchDashboards(); + root = startOSDResp.root; + }, 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 + ) + ); + }; + + 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('workspace related CRUD', () => { + it('create', async () => { + const createResult = await osdTestServer.request + .post(root, `/api/saved_objects/${dashboard.type}`) + .send({ + attributes: dashboard.attributes, + workspaces: ['foo'], + }) + .expect(200); + + expect(createResult.body.workspaces).toEqual(['foo']); + await deleteItem({ + type: dashboard.type, + id: createResult.body.id, + }); + }); + + it('create-with-override', async () => { + const createResult = await osdTestServer.request + .post(root, `/api/saved_objects/${dashboard.type}`) + .send({ + attributes: dashboard.attributes, + workspaces: ['foo'], + }) + .expect(200); + + await osdTestServer.request + .post(root, `/api/saved_objects/${dashboard.type}/${createResult.body.id}?overwrite=true`) + .send({ + attributes: dashboard.attributes, + workspaces: ['bar'], + }) + .expect(409); + + await deleteItem({ + type: dashboard.type, + id: createResult.body.id, + }); + }); + + it('bulk create', async () => { + await clearFooAndBar(); + const createResultFoo = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=foo`) + .send([ + { + ...dashboard, + id: 'foo', + }, + ]) + .expect(200); + + const createResultBar = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=bar`) + .send([ + { + ...dashboard, + id: 'bar', + }, + ]) + .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, ['foo']) + ) + ).toEqual(true); + expect((createResultBar.body.saved_objects as any[]).some((item) => item.error)).toEqual( + false + ); + expect( + (createResultBar.body.saved_objects as any[]).every((item) => + isEqual(item.workspaces, ['bar']) + ) + ).toEqual(true); + await Promise.all( + [...createResultFoo.body.saved_objects, ...createResultBar.body.saved_objects].map((item) => + deleteItem({ + type: item.type, + id: item.id, + }) + ) + ); + }); + + it('bulk create with conflict', async () => { + await clearFooAndBar(); + const createResultFoo = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=foo`) + .send([ + { + ...dashboard, + id: 'foo', + }, + ]) + .expect(200); + + const createResultBar = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=bar`) + .send([ + { + ...dashboard, + id: 'bar', + }, + ]) + .expect(200); + + /** + * overwrite with workspaces + */ + const overwriteWithWorkspacesResult = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?overwrite=true&workspaces=foo`) + .send([ + { + ...dashboard, + id: 'bar', + }, + { + ...dashboard, + id: 'foo', + attributes: { + title: 'foo', + }, + }, + ]) + .expect(200); + + expect(overwriteWithWorkspacesResult.body.saved_objects[0].error.statusCode).toEqual(409); + expect(overwriteWithWorkspacesResult.body.saved_objects[1].attributes.title).toEqual('foo'); + expect(overwriteWithWorkspacesResult.body.saved_objects[1].workspaces).toEqual(['foo']); + + await Promise.all( + [...createResultFoo.body.saved_objects, ...createResultBar.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, `/api/saved_objects/_bulk_create?workspaces=foo`) + .send([ + { + ...dashboard, + id: 'foo', + }, + ]) + .expect(200); + + const createResultBar = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=bar`) + .send([ + { + ...dashboard, + id: 'bar', + }, + ]) + .expect(200); + + const getResultFoo = await getItem({ + type: dashboard.type, + id: 'foo', + }); + const getResultBar = await getItem({ + type: dashboard.type, + id: 'bar', + }); + + const readableStream = new Readable(); + readableStream.push( + `Content-Disposition: form-data; name="file"; filename="tmp.ndjson"\r\n\r\n` + ); + readableStream.push( + [JSON.stringify(getResultFoo.body), JSON.stringify(getResultBar.body)].join('\n') + ); + readableStream.push(null); + + /** + * import with workspaces when conflicts + */ + const importWithWorkspacesResult = await osdTestServer.request + .post(root, `/api/saved_objects/_import?workspaces=foo&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, `/api/saved_objects/_bulk_create?workspaces=foo`) + .send([ + { + ...dashboard, + id: 'foo', + }, + ]) + .expect(200); + + const createResultBar = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=bar`) + .send([ + { + ...dashboard, + id: 'bar', + }, + ]) + .expect(200); + + const findResult = await osdTestServer.request + .get(root, `/api/saved_objects/_find?workspaces=bar&type=${dashboard.type}`) + .expect(200); + + expect(findResult.body.total).toEqual(1); + expect(findResult.body.saved_objects[0].workspaces).toEqual(['bar']); + + await Promise.all( + [...createResultFoo.body.saved_objects, ...createResultBar.body.saved_objects].map((item) => + deleteItem({ + type: item.type, + id: item.id, + }) + ) + ); + }); + }); +}); 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 96883e55320b..82bed74eca55 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.js +++ b/src/core/server/saved_objects/service/lib/repository.test.js @@ -27,7 +27,6 @@ * specific language governing permissions and limitations * under the License. */ - import { SavedObjectsRepository } from './repository'; import * as getSearchDslNS from './search_dsl/search_dsl'; import { SavedObjectsErrorHelpers } from './errors'; @@ -54,6 +53,12 @@ const createGenericNotFoundError = (...args) => const createUnsupportedTypeError = (...args) => SavedObjectsErrorHelpers.createUnsupportedTypeError(...args).output.payload; +const omitWorkspace = (object) => { + const newObject = JSON.parse(JSON.stringify(object)); + delete newObject.workspaces; + return newObject; +}; + describe('SavedObjectsRepository', () => { let client; let savedObjectsRepository; @@ -182,6 +187,7 @@ describe('SavedObjectsRepository', () => { _source: { ...(registry.isSingleNamespace(type) && { namespace: namespaceId }), ...(registry.isMultiNamespace(type) && { namespaces: [namespaceId ?? 'default'] }), + workspaces, ...(originId && { originId }), ...(permissions && { permissions }), type, @@ -493,7 +499,9 @@ describe('SavedObjectsRepository', () => { opensearchClientMock.createSuccessTransportRequestPromise(response) ); const result = await savedObjectsRepository.bulkCreate(objects, options); - expect(client.mget).toHaveBeenCalledTimes(multiNamespaceObjects?.length ? 1 : 0); + expect(client.mget).toHaveBeenCalledTimes( + multiNamespaceObjects?.length || options?.workspaces ? 1 : 0 + ); return result; }; @@ -696,6 +704,7 @@ describe('SavedObjectsRepository', () => { expect.anything() ); client.bulk.mockClear(); + client.mget.mockClear(); }; await test(undefined); await test(namespace); @@ -912,6 +921,74 @@ describe('SavedObjectsRepository', () => { const expectedError = expectErrorResult(obj3, { message: JSON.stringify(opensearchError) }); await bulkCreateError(obj3, opensearchError, expectedError); }); + + it(`returns error when there is a conflict with an existing saved object according to workspaces`, async () => { + const obj = { ...obj3, workspaces: ['foo'] }; + const response1 = { + status: 200, + docs: [ + { + found: true, + _id: `${obj1.type}:${obj1.id}`, + _source: { + type: obj1.type, + workspaces: ['bar'], + }, + }, + { + found: true, + _id: `${obj.type}:${obj.id}`, + _source: { + type: obj.type, + workspaces: obj.workspaces, + }, + }, + { + found: true, + _id: `${obj2.type}:${obj2.id}`, + _source: { + type: obj2.type, + }, + }, + ], + }; + client.mget.mockResolvedValueOnce( + opensearchClientMock.createSuccessTransportRequestPromise(response1) + ); + const response2 = getMockBulkCreateResponse([obj1, obj, obj2]); + client.bulk.mockResolvedValueOnce( + opensearchClientMock.createSuccessTransportRequestPromise(response2) + ); + + const options = { overwrite: true, workspaces: ['bar'] }; + const result = await savedObjectsRepository.bulkCreate([obj1, obj, obj2], options); + expect(client.bulk).toHaveBeenCalled(); + expect(client.mget).toHaveBeenCalled(); + + const body1 = { + docs: [ + expect.objectContaining({ _id: `${obj1.type}:${obj1.id}` }), + expect.objectContaining({ _id: `${obj.type}:${obj.id}` }), + expect.objectContaining({ _id: `${obj2.type}:${obj2.id}` }), + ], + }; + expect(client.mget).toHaveBeenCalledWith( + expect.objectContaining({ body: body1 }), + expect.anything() + ); + const body2 = [...expectObjArgs(obj1)]; + expect(client.bulk).toHaveBeenCalledWith( + expect.objectContaining({ body: body2 }), + expect.anything() + ); + expect(result).toEqual({ + saved_objects: [ + expectSuccess(obj1), + expectErrorConflict(obj, { metadata: { isNotOverwritable: true } }), + expectErrorConflict(obj2, { metadata: { isNotOverwritable: true } }), + ], + }); + }); }); describe('migration', () => { @@ -1791,6 +1868,8 @@ describe('SavedObjectsRepository', () => { const obj5 = { type: MULTI_NAMESPACE_TYPE, id: 'five' }; const obj6 = { type: NAMESPACE_AGNOSTIC_TYPE, id: 'six' }; const obj7 = { type: NAMESPACE_AGNOSTIC_TYPE, id: 'seven' }; + const obj8 = { type: 'dashboard', id: 'eight', workspaces: ['foo'] }; + const obj9 = { type: 'dashboard', id: 'nine', workspaces: ['bar'] }; const namespace = 'foo-namespace'; const checkConflicts = async (objects, options) => @@ -1882,6 +1961,8 @@ describe('SavedObjectsRepository', () => { { found: false }, getMockGetResponse(obj6), { found: false }, + getMockGetResponse(obj7), + getMockGetResponse(obj8), ], }; client.mget.mockResolvedValue( @@ -1910,6 +1991,36 @@ describe('SavedObjectsRepository', () => { ], }); }); + + it(`expected results with workspaces`, async () => { + const objects = [obj8, obj9]; + const response = { + status: 200, + docs: [getMockGetResponse(obj8), getMockGetResponse(obj9)], + }; + client.mget.mockResolvedValue( + opensearchClientMock.createSuccessTransportRequestPromise(response) + ); + + const result = await checkConflicts(objects, { + workspaces: ['foo'], + }); + expect(client.mget).toHaveBeenCalledTimes(1); + expect(result).toEqual({ + errors: [ + { ...omitWorkspace(obj8), error: createConflictError(obj8.type, obj8.id) }, + { + ...omitWorkspace(obj9), + error: { + ...createConflictError(obj9.type, obj9.id), + metadata: { + isNotOverwritable: true, + }, + }, + }, + ], + }); + }); }); }); @@ -1946,9 +2057,17 @@ describe('SavedObjectsRepository', () => { const createSuccess = async (type, attributes, options) => { const result = await savedObjectsRepository.create(type, attributes, options); - expect(client.get).toHaveBeenCalledTimes( - registry.isMultiNamespace(type) && options.overwrite ? 1 : 0 - ); + let count = 0; + if (options?.overwrite && options.id && options.workspaces) { + /** + * workspace will call extra one to get latest status of current object + */ + count++; + } + if (registry.isMultiNamespace(type) && options.overwrite) { + count++; + } + expect(client.get).toHaveBeenCalledTimes(count); return result; }; @@ -2210,6 +2329,21 @@ describe('SavedObjectsRepository', () => { expect(client.get).toHaveBeenCalled(); }); + it(`throws error when there is a conflict with an existing workspaces saved object`, async () => { + const response = getMockGetResponse({ workspaces: ['foo'], id }); + client.get.mockResolvedValueOnce( + opensearchClientMock.createSuccessTransportRequestPromise(response) + ); + await expect( + savedObjectsRepository.create('dashboard', attributes, { + id, + overwrite: true, + workspaces: ['bar'], + }) + ).rejects.toThrowError(createConflictError('dashboard', id)); + expect(client.get).toHaveBeenCalled(); + }); + it.todo(`throws when automatic index creation fails`); it.todo(`throws when an unexpected failure occurs`); @@ -2303,10 +2437,11 @@ describe('SavedObjectsRepository', () => { const type = 'index-pattern'; const id = 'logstash-*'; const namespace = 'foo-namespace'; + const workspaces = ['bar-workspace']; const deleteSuccess = async (type, id, options) => { if (registry.isMultiNamespace(type)) { - const mockGetResponse = getMockGetResponse({ type, id }, options?.namespace); + const mockGetResponse = getMockGetResponse({ type, id }, options?.namespace, workspaces); client.get.mockResolvedValueOnce( opensearchClientMock.createSuccessTransportRequestPromise(mockGetResponse) ); diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index 15dcf7a6c122..be4a0ba6f29b 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -360,15 +360,28 @@ export class SavedObjectsRepository { const method = object.id && overwrite ? 'index' : 'create'; const requiresNamespacesCheck = object.id && this._registry.isMultiNamespace(object.type); + /** + * It requires a check when overwriting objects to target workspaces + */ + const requiresWorkspaceCheck = !!(object.id && options.workspaces); if (object.id == null) object.id = uuid.v1(); + let opensearchRequestIndexPayload = {}; + + if (requiresNamespacesCheck || requiresWorkspaceCheck) { + opensearchRequestIndexPayload = { + opensearchRequestIndex: bulkGetRequestIndexCounter, + }; + bulkGetRequestIndexCounter++; + } + return { tag: 'Right' as 'Right', value: { method, object, - ...(requiresNamespacesCheck && { opensearchRequestIndex: bulkGetRequestIndexCounter++ }), + ...opensearchRequestIndexPayload, }, }; }); @@ -379,7 +392,7 @@ export class SavedObjectsRepository { .map(({ value: { object: { type, id } } }) => ({ _id: this._serializer.generateRawId(namespace, type, id), _index: this.getIndexForType(type), - _source: ['type', 'namespaces'], + _source: ['type', 'namespaces', 'workspaces'], })); const bulkGetResponse = bulkGetDocs.length ? await this.client.mget( @@ -410,7 +423,7 @@ export class SavedObjectsRepository { if (opensearchRequestIndex !== undefined) { const indexFound = bulkGetResponse?.statusCode !== 404; const actualResult = indexFound - ? bulkGetResponse?.body.docs[opensearchRequestIndex] + ? bulkGetResponse?.body.docs?.[opensearchRequestIndex] : undefined; const docFound = indexFound && actualResult?.found === true; // @ts-expect-error MultiGetHit._source is optional @@ -562,7 +575,7 @@ export class SavedObjectsRepository { const bulkGetDocs = expectedBulkGetResults.filter(isRight).map(({ value: { type, id } }) => ({ _id: this._serializer.generateRawId(namespace, type, id), _index: this.getIndexForType(type), - _source: ['type', 'namespaces'], + _source: ['type', 'namespaces', 'workspaces'], })); const bulkGetResponse = bulkGetDocs.length ? await this.client.mget( @@ -585,13 +598,24 @@ export class SavedObjectsRepository { const { type, id, opensearchRequestIndex } = expectedResult.value; const doc = bulkGetResponse?.body.docs[opensearchRequestIndex]; if (doc?.found) { + let workspaceConflict = false; + if (options.workspaces) { + const transformedObject = this._serializer.rawToSavedObject(doc as SavedObjectsRawDoc); + const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToBaseWorkspaces( + options.workspaces, + transformedObject.workspaces + ); + if (filteredWorkspaces.length) { + workspaceConflict = true; + } + } errors.push({ id, type, error: { ...errorContent(SavedObjectsErrorHelpers.createConflictError(type, id)), // @ts-expect-error MultiGetHit._source is optional - ...(!this.rawDocExistsInNamespace(doc!, namespace) && { + ...((!this.rawDocExistsInNamespace(doc!, namespace) || workspaceConflict) && { metadata: { isNotOverwritable: true }, }), }, @@ -926,7 +950,7 @@ export class SavedObjectsRepository { */ async bulkGet( objects: SavedObjectsBulkGetObject[] = [], - options: SavedObjectsBaseOptions = {} + options: Omit = {} ): Promise> { const namespace = normalizeNamespace(options.namespace); @@ -1014,7 +1038,7 @@ export class SavedObjectsRepository { async get( type: string, id: string, - options: SavedObjectsBaseOptions = {} + options: Omit = {} ): Promise> { if (!this._allowedTypes.includes(type)) { throw SavedObjectsErrorHelpers.createGenericNotFoundError(type, id); @@ -1040,7 +1064,7 @@ export class SavedObjectsRepository { throw SavedObjectsErrorHelpers.createGenericNotFoundError(type, id); } - const { originId, updated_at: updatedAt, permissions } = body._source; + const { originId, updated_at: updatedAt, permissions, workspaces } = body._source; let namespaces: string[] = []; if (!this._registry.isNamespaceAgnostic(type)) { @@ -1056,6 +1080,7 @@ export class SavedObjectsRepository { ...(originId && { originId }), ...(updatedAt && { updated_at: updatedAt }), ...(permissions && { permissions }), + ...(workspaces && { workspaces }), version: encodeHitVersion(body), attributes: body._source[type], references: body._source.references || [], @@ -1121,7 +1146,7 @@ export class SavedObjectsRepository { throw SavedObjectsErrorHelpers.createGenericNotFoundError(type, id); } - const { originId } = body.get?._source ?? {}; + const { originId, workspaces } = body.get?._source ?? {}; let namespaces: string[] = []; if (!this._registry.isNamespaceAgnostic(type)) { namespaces = body.get?._source.namespaces ?? [ @@ -1137,6 +1162,7 @@ export class SavedObjectsRepository { namespaces, ...(originId && { originId }), ...(permissions && { permissions }), + ...(workspaces && { workspaces }), references, attributes, }; @@ -1520,12 +1546,13 @@ export class SavedObjectsRepository { }; } - const { originId } = get._source; + const { originId, workspaces } = get._source; return { id, type, ...(namespaces && { namespaces }), ...(originId && { originId }), + ...(workspaces && { workspaces }), updated_at, version: encodeVersion(seqNo, primaryTerm), attributes, 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 b78c5a032992..186457145103 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 @@ -127,6 +127,20 @@ function getClauseForType( }, }; } +/** + * Gets the clause that will filter for the workspace. + */ +function getClauseForWorkspace(workspace: string) { + if (!workspace) { + return {}; + } + + return { + bool: { + must: [{ term: { workspaces: workspace } }], + }, + }; +} /** * Gets the clause that will filter for the workspace. diff --git a/src/core/server/saved_objects/service/lib/utils.ts b/src/core/server/saved_objects/service/lib/utils.ts index 4823e52d77c9..9fc4a6280b63 100644 --- a/src/core/server/saved_objects/service/lib/utils.ts +++ b/src/core/server/saved_objects/service/lib/utils.ts @@ -80,4 +80,11 @@ export class SavedObjectsUtils { total: 0, saved_objects: [], }); + + public static filterWorkspacesAccordingToBaseWorkspaces( + targetWorkspaces?: string[], + sourceWorkspaces?: string[] + ): string[] { + return targetWorkspaces?.filter((item) => !sourceWorkspaces?.includes(item)) || []; + } } 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 05069d9d887a..e1c3d16a9258 100644 --- a/src/core/server/saved_objects/service/saved_objects_client.ts +++ b/src/core/server/saved_objects/service/saved_objects_client.ts @@ -385,7 +385,7 @@ export class SavedObjectsClient { */ async bulkGet( objects: SavedObjectsBulkGetObject[] = [], - options: SavedObjectsBaseOptions = {} + options: Omit = {} ): Promise> { return await this._repository.bulkGet(objects, options); }