Skip to content

Commit

Permalink
[Workspace] Fix: keep disallowed types when importing with overwrite (#…
Browse files Browse the repository at this point in the history
…6668)

* fix: keep disallowed types when importing with overwrite

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>

* Changeset file for PR #6668 created/updated

* feat: update comment

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>

* feat: address some minor concern

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>

---------

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>
Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
(cherry picked from commit de6a889)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
1 parent fcc06de commit a59c616
Show file tree
Hide file tree
Showing 4 changed files with 174 additions and 5 deletions.
2 changes: 2 additions & 0 deletions changelogs/fragments/6668.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
fix:
- Keep disallowed types when importing with overwrite ([#6668](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6668))
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@ const dataSource: Omit<SavedObject, 'id'> = {
references: [],
};

const indexPattern: Omit<SavedObject, 'id'> = {
type: 'index-pattern',
attributes: {},
references: [],
};

const advancedSettings: Omit<SavedObject, 'id'> = {
type: 'config',
attributes: {},
Expand Down Expand Up @@ -445,6 +451,95 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test',
);
});

it('checkConflicts when importing disallowed types', async () => {
await clearFooAndBar();
// 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({
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}`)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -335,7 +357,7 @@ export class WorkspaceConflictSavedObjectsClientWrapper {
}
}

const objectsNoWorkspaceConflictError = objects.filter(
const objectsNoWorkspaceConflictError = allowedSavedObjects.filter(
(item) =>
!objectsConflictWithWorkspace.find(
(errorItems) =>
Expand All @@ -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
);
Expand All @@ -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;
Expand Down

0 comments on commit a59c616

Please sign in to comment.