From 20daeedae600468b12200913967b34c7094af7bb Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 29 Apr 2024 11:03:48 +0800 Subject: [PATCH 1/4] fix: keep disallowed types when importing with overwrite Signed-off-by: SuZhou-Joe --- ...apper_for_check_workspace_conflict.test.ts | 94 +++++++++++++++++++ ...apper_for_check_workspace_conflict.test.ts | 37 ++++++++ ...ts_wrapper_for_check_workspace_conflict.ts | 45 ++++++++- 3 files changed, 171 insertions(+), 5 deletions(-) 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 0942556b521d..2958d8b9fe0e 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 @@ -23,6 +23,12 @@ const dataSource: Omit = { references: [], }; +const indexPattern: Omit = { + type: 'index-pattern', + attributes: {}, + references: [], +}; + const advancedSettings: Omit = { type: 'config', attributes: {}, @@ -445,6 +451,94 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', ); }); + it('checkConflicts when importing disallowed types', async () => { + await clearFooAndBar(); + // Create data source through saved objects client will do a connection validation + // Use OpenSearch API to create the saved objects directly + await osdTestServer.request + .post(root, `/api/console/proxy?path=.kibana%2F_doc%2Fdata-source%3Afoo&method=PUT`) + .send({ + type: dataSource.type, + [dataSource.type]: {}, + }) + .expect(201); + + await osdTestServer.request + .post(root, `/api/saved_objects/${indexPattern.type}/foo`) + .send({ + attributes: indexPattern.attributes, + references: [ + { + id: 'foo', + type: dataSource.type, + name: 'dataSource', + }, + ], + }) + .expect(200); + + const getResultFoo = await getItem({ + type: dataSource.type, + id: 'foo', + }); + const getResultBar = await getItem({ + type: indexPattern.type, + id: 'foo', + }); + + /** + * import with workspaces when conflicts + */ + const importWithWorkspacesResult = await osdTestServer.request + .post( + root, + `/api/saved_objects/_import?workspaces=${createdFooWorkspace.id}&overwrite=true&dataSourceEnabled=true` + ) + .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('unknown'); + expect(importWithWorkspacesResult.body.successCount).toEqual(1); + + const [indexPatternImportResult] = importWithWorkspacesResult.body.successResults; + const getImportIndexPattern = await getItem({ + type: indexPatternImportResult.type, + id: indexPatternImportResult.destinationId, + }); + + // The references to disallowed types should be kept + expect(getImportIndexPattern.body.references).toEqual([ + { + id: 'foo', + type: dataSource.type, + name: 'dataSource', + }, + ]); + + await Promise.all( + [ + { id: 'foo', type: indexPattern.type }, + { id: 'foo', type: dataSource.type }, + { id: indexPatternImportResult.destinationId, type: indexPattern.type }, + ].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=${createdFooWorkspace.id}`) 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 70b571ebc3e3..f5613550a541 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 @@ -461,6 +461,43 @@ describe('WorkspaceConflictSavedObjectsClientWrapper', () => { } `); }); + + it(`Should return error when trying to check conflict on disallowed types within a workspace`, async () => { + mockedClient.bulkCreate.mockResolvedValueOnce({ saved_objects: [] }); + const result = await wrapperClient.checkConflicts( + [ + getSavedObject({ + type: 'config', + id: 'foo', + }), + getSavedObject({ + type: DATA_SOURCE_SAVED_OBJECT_TYPE, + id: 'foo', + }), + ], + { + workspaces: ['foo'], + } + ); + + expect(mockedClient.bulkCreate).toBeCalledWith([], { + workspaces: ['foo'], + }); + expect(result.errors[0].error).toEqual( + expect.objectContaining({ + message: + "Unsupported type in workspace: 'config' is not allowed to be imported in workspace.", + statusCode: 400, + }) + ); + expect(result.errors[1].error).toEqual( + expect.objectContaining({ + message: + "Unsupported type in workspace: 'data-source' is not allowed to be imported in workspace.", + statusCode: 400, + }) + ); + }); }); describe('find', () => { 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 5f293580153d..b34c88b47fbc 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 @@ -289,11 +289,33 @@ export class WorkspaceConflictSavedObjectsClientWrapper { return { errors: [] }; } + const { workspaces } = options; + + const disallowedSavedObjects: SavedObjectsCheckConflictsObject[] = []; + const allowedSavedObjects: SavedObjectsCheckConflictsObject[] = []; + objects.forEach((item) => { + const isImportIntoWorkspace = 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; + }); + /** * Workspace conflict only happens when target workspaces params present. */ if (options.workspaces) { - const bulkGetDocs: any[] = objects.map((object) => { + const bulkGetDocs: any[] = allowedSavedObjects.map((object) => { const { type, id } = object; return { @@ -335,7 +357,7 @@ export class WorkspaceConflictSavedObjectsClientWrapper { } } - const objectsNoWorkspaceConflictError = objects.filter( + const objectsNoWorkspaceConflictError = allowedSavedObjects.filter( (item) => !objectsConflictWithWorkspace.find( (errorItems) => @@ -355,7 +377,7 @@ export class WorkspaceConflictSavedObjectsClientWrapper { /** * Bypass those objects that are not conflict on workspaces */ - const realBulkCreateResult = await wrapperOptions.client.checkConflicts( + const realCheckConflictsResult = await wrapperOptions.client.checkConflicts( objectsNoWorkspaceConflictError, options ); @@ -364,8 +386,21 @@ export class WorkspaceConflictSavedObjectsClientWrapper { * Merge results from two conflict check. */ const result: SavedObjectsCheckConflictsResponse = { - ...realBulkCreateResult, - errors: [...objectsConflictWithWorkspace, ...realBulkCreateResult.errors], + ...realCheckConflictsResult, + errors: [ + ...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 }, + }, + })), + ...(realCheckConflictsResult?.errors || []), + ], }; return result; From fb89f0e3a8ce9baf111783d335246ca4751c0dd1 Mon Sep 17 00:00:00 2001 From: "opensearch-changeset-bot[bot]" <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Date: Mon, 29 Apr 2024 03:11:13 +0000 Subject: [PATCH 2/4] Changeset file for PR #6668 created/updated --- changelogs/fragments/6668.yml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelogs/fragments/6668.yml diff --git a/changelogs/fragments/6668.yml b/changelogs/fragments/6668.yml new file mode 100644 index 000000000000..239ee0257197 --- /dev/null +++ b/changelogs/fragments/6668.yml @@ -0,0 +1,2 @@ +fix: +- Keep disallowed types when importing with overwrite ([#6668](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6668)) \ No newline at end of file From 7f2fe361d4edf005a9ed326b3b4b6be856a0e377 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 29 Apr 2024 13:01:03 +0800 Subject: [PATCH 3/4] feat: update comment Signed-off-by: SuZhou-Joe --- ...aved_objects_wrapper_for_check_workspace_conflict.test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) 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 2958d8b9fe0e..7967992690d0 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 @@ -453,8 +453,9 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', it('checkConflicts when importing disallowed types', async () => { await clearFooAndBar(); - // Create data source through saved objects client will do a connection validation - // Use OpenSearch API to create the saved objects directly + // Create data source through OpenSearch API directly + // or the saved objects API will do connection validation on the data source + // which will not pass as it is a dummy data source without endpoint and credentials await osdTestServer.request .post(root, `/api/console/proxy?path=.kibana%2F_doc%2Fdata-source%3Afoo&method=PUT`) .send({ From 1df34d167f25a47eaff4a257fb7bff46819ca9da Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 29 Apr 2024 16:48:54 +0800 Subject: [PATCH 4/4] feat: address some minor concern Signed-off-by: SuZhou-Joe --- .../saved_objects_wrapper_for_check_workspace_conflict.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 b34c88b47fbc..0c4447e69e21 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 @@ -294,7 +294,7 @@ export class WorkspaceConflictSavedObjectsClientWrapper { const disallowedSavedObjects: SavedObjectsCheckConflictsObject[] = []; const allowedSavedObjects: SavedObjectsCheckConflictsObject[] = []; objects.forEach((item) => { - const isImportIntoWorkspace = workspaces?.length; + const isImportIntoWorkspace = !!workspaces?.length; // config can not be created inside a workspace if (this.isConfigType(item.type) && isImportIntoWorkspace) { disallowedSavedObjects.push(item);