Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Workspace] Fix: keep disallowed types when importing with overwrite #6668

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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;
SuZhou-Joe marked this conversation as resolved.
Show resolved Hide resolved
// 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;
Copy link
Collaborator

@Hailong-am Hailong-am Apr 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: return is not needed here

});

/**
* 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
Loading