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

[Backport 2.x] [Workspace] Delete saved objects by workspace #6424

Merged
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions src/core/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,7 @@ export {
ISavedObjectsRepository,
SavedObjectsRepository,
SavedObjectsDeleteByNamespaceOptions,
SavedObjectsDeleteByWorkspaceOptions,
SavedObjectsIncrementCounterOptions,
SavedObjectsFieldMapping,
SavedObjectsTypeMappingDefinition,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ const create = (): jest.Mocked<ISavedObjectsRepository> => ({
deleteFromNamespaces: jest.fn(),
deleteByNamespace: jest.fn(),
incrementCounter: jest.fn(),
deleteByWorkspace: jest.fn(),
});

export const savedObjectsRepositoryMock = { create };
79 changes: 79 additions & 0 deletions src/core/server/saved_objects/service/lib/repository.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2585,6 +2585,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 {
Expand Down
50 changes: 50 additions & 0 deletions src/core/server/saved_objects/service/lib/repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
SavedObjectsAddToNamespacesResponse,
SavedObjectsDeleteFromNamespacesOptions,
SavedObjectsDeleteFromNamespacesResponse,
SavedObjectsDeleteByWorkspaceOptions,
} from '../saved_objects_client';
import {
SavedObject,
Expand Down Expand Up @@ -714,6 +715,55 @@
return body;
}

/**
* Deletes all objects from the provided workspace.
*
* @param {string} workspace - workspace id
* @param options SavedObjectsDeleteByWorkspaceOptions
* @returns {promise} - { took, timed_out, total, deleted, batches, version_conflicts, noops, retries, failures }
*/
async deleteByWorkspace(
workspace: string,
options: SavedObjectsDeleteByWorkspaceOptions = {}
): Promise<any> {
if (!workspace || typeof workspace !== 'string' || workspace === '*') {
throw new TypeError(`workspace is required, and must be a string that is not equal to '*'`);

Check warning on line 730 in src/core/server/saved_objects/service/lib/repository.ts

View check run for this annotation

Codecov / codecov/patch

src/core/server/saved_objects/service/lib/repository.ts#L730

Added line #L730 was not covered by tests
}

const allTypes = Object.keys(getRootPropertiesObjects(this._mappings));

Check warning on line 733 in src/core/server/saved_objects/service/lib/repository.ts

View check run for this annotation

Codecov / codecov/patch

src/core/server/saved_objects/service/lib/repository.ts#L733

Added line #L733 was not covered by tests

const { body } = await this.client.updateByQuery(

Check warning on line 735 in src/core/server/saved_objects/service/lib/repository.ts

View check run for this annotation

Codecov / codecov/patch

src/core/server/saved_objects/service/lib/repository.ts#L735

Added line #L735 was not covered by tests
{
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;

Check warning on line 764 in src/core/server/saved_objects/service/lib/repository.ts

View check run for this annotation

Codecov / codecov/patch

src/core/server/saved_objects/service/lib/repository.ts#L764

Added line #L764 was not covered by tests
}

/**
* @param {object} [options={}]
* @property {(string|Array<string>)} [options.type]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
21 changes: 21 additions & 0 deletions src/core/server/saved_objects/service/saved_objects_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,15 @@
references: SavedObjectReference[] | undefined;
}

/**
*
* @public
*/
export interface SavedObjectsDeleteByWorkspaceOptions extends SavedObjectsBaseOptions {
/** The OpenSearch supports only boolean flag for this operation */
refresh?: boolean;
}

/**
*
* @public
Expand Down Expand Up @@ -446,6 +455,18 @@
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<any> => {
return await this._repository.deleteByWorkspace(workspace, options);

Check warning on line 467 in src/core/server/saved_objects/service/saved_objects_client.ts

View check run for this annotation

Codecov / codecov/patch

src/core/server/saved_objects/service/saved_objects_client.ts#L467

Added line #L467 was not covered by tests
};

/**
* Bulk Updates multiple SavedObject at once
*
Expand Down
46 changes: 43 additions & 3 deletions src/plugins/workspace/server/integration_tests/routes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = <T extends { id?: string }>(object: T): Omit<T, 'id'> => {
const { id, ...others } = object;
Expand All @@ -20,6 +21,7 @@ const testWorkspace: WorkspaceAttribute = {
describe('workspace service', () => {
let root: ReturnType<typeof osdTestServer.createRoot>;
let opensearchServer: osdTestServer.TestOpenSearchUtils;
let osd: osdTestServer.TestOpenSearchDashboardsUtils;
beforeAll(async () => {
const { startOpenSearch, startOpenSearchDashboards } = osdTestServer.createTestServers({
adjustTimeout: (t: number) => jest.setTimeout(t),
Expand All @@ -28,12 +30,13 @@ describe('workspace service', () => {
workspace: {
enabled: true,
},
migrations: { skip: false },
},
},
});
opensearchServer = await startOpenSearch();
const startOSDResp = await startOpenSearchDashboards();
root = startOSDResp.root;
osd = await startOpenSearchDashboards();
root = osd.root;
});
afterAll(async () => {
await root.shutdown();
Expand All @@ -47,9 +50,13 @@ describe('workspace service', () => {
page: 1,
})
.expect(200);
const savedObjectsRepository = osd.coreStart.savedObjects.createInternalRepository([
WORKSPACE_TYPE,
]);
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
savedObjectsRepository.delete(WORKSPACE_TYPE, item.id)
)
);
});
Expand Down Expand Up @@ -119,6 +126,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);
Expand All @@ -129,6 +146,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: omitId(reservedWorkspace),
})
.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
Expand Down
1 change: 1 addition & 0 deletions src/plugins/workspace/server/routes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
18 changes: 17 additions & 1 deletion src/plugins/workspace/server/workspace_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,23 @@ export class WorkspaceClient implements IWorkspaceClientImpl {
}
public async delete(requestDetail: IRequestDetail, id: string): Promise<IResponse<boolean>> {
try {
await this.getSavedObjectClientsFromRequestDetail(requestDetail).delete(WORKSPACE_TYPE, id);
const savedObjectClient = this.getSavedObjectClientsFromRequestDetail(requestDetail);
const workspaceInDB: SavedObject<WorkspaceAttribute> = 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,
Expand Down
Loading