Skip to content

Commit

Permalink
Delete saved object when deleting workspace (#84) (#150)
Browse files Browse the repository at this point in the history
* remove objects from workspace



* delete saved object when deleting workspace



* fix osd bootstrap issue



* add force option to delete saved objects when deleting workspace



* remove force delete option



* redirect only delete workspace successfuly



* delete saved object when deleting workspace



* delete saved object when deleting workspace



* Add more unit test cases



---------


(cherry picked from commit f465403)

Signed-off-by: Hailong Cui <ihailong@amazon.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
1 parent af9d698 commit 0f704e6
Show file tree
Hide file tree
Showing 9 changed files with 258 additions and 15 deletions.
1 change: 1 addition & 0 deletions src/core/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,7 @@ export {
SavedObjectsShareObjects,
SavedObjectsAddToWorkspacesOptions,
SavedObjectsAddToWorkspacesResponse,
SavedObjectsDeleteByWorkspaceOptions,
Permissions,
ACL,
SavedObjectsPermissionControlContract,
Expand Down
2 changes: 2 additions & 0 deletions src/core/server/saved_objects/service/lib/repository.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ const create = (): jest.Mocked<ISavedObjectsRepository> => ({
addToWorkspaces: jest.fn(),
getPermissionQuery: jest.fn(),
processFindOptions: jest.fn(),
deleteByWorkspace: jest.fn(),
deleteFromWorkspaces: jest.fn(),
});

export const savedObjectsRepositoryMock = { create };
45 changes: 35 additions & 10 deletions src/core/server/saved_objects/service/lib/repository.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ describe('SavedObjectsRepository', () => {
});

const getMockGetResponse = (
{ type, id, references, namespace: objectNamespace, originId },
{ type, id, references, namespace: objectNamespace, originId, workspaces },
namespace
) => {
const namespaceId = objectNamespace === 'default' ? undefined : objectNamespace ?? namespace;
Expand All @@ -182,6 +182,7 @@ describe('SavedObjectsRepository', () => {
_source: {
...(registry.isSingleNamespace(type) && { namespace: namespaceId }),
...(registry.isMultiNamespace(type) && { namespaces: [namespaceId ?? 'default'] }),
workspaces,
...(originId && { originId }),
type,
[type]: { title: 'Testing' },
Expand Down Expand Up @@ -2213,32 +2214,33 @@ describe('SavedObjectsRepository', () => {
const type = 'index-pattern';
const id = 'logstash-*';
const namespace = 'foo-namespace';
const workspaces = ['bar-workspace'];

const mockGet = async (type, id, options) => {
const mockGetResponse = getMockGetResponse({ type, id }, options?.namespace, workspaces);
client.get.mockResolvedValue(
opensearchClientMock.createSuccessTransportRequestPromise(mockGetResponse)
);
};

const deleteSuccess = async (type, id, options) => {
if (registry.isMultiNamespace(type)) {
const mockGetResponse = getMockGetResponse({ type, id }, options?.namespace);
client.get.mockResolvedValueOnce(
opensearchClientMock.createSuccessTransportRequestPromise(mockGetResponse)
);
}
mockGet(type, id, options);
client.delete.mockResolvedValueOnce(
opensearchClientMock.createSuccessTransportRequestPromise({ result: 'deleted' })
);
const result = await savedObjectsRepository.delete(type, id, options);
expect(client.get).toHaveBeenCalledTimes(registry.isMultiNamespace(type) ? 1 : 0);
expect(client.get).toHaveBeenCalledTimes(registry.isMultiNamespace(type) ? 2 : 1);
return result;
};

describe('client calls', () => {
it(`should use the OpenSearch delete action when not using a multi-namespace type`, async () => {
await deleteSuccess(type, id);
expect(client.get).not.toHaveBeenCalled();
expect(client.delete).toHaveBeenCalledTimes(1);
});

it(`should use OpenSearch get action then delete action when using a multi-namespace type`, async () => {
await deleteSuccess(MULTI_NAMESPACE_TYPE, id);
expect(client.get).toHaveBeenCalledTimes(1);
expect(client.delete).toHaveBeenCalledTimes(1);
});

Expand Down Expand Up @@ -2294,6 +2296,7 @@ describe('SavedObjectsRepository', () => {
);

client.delete.mockClear();
client.get.mockClear();
await deleteSuccess(MULTI_NAMESPACE_TYPE, id, { namespace });
expect(client.delete).toHaveBeenCalledWith(
expect.objectContaining({ id: `${MULTI_NAMESPACE_TYPE}:${id}` }),
Expand Down Expand Up @@ -2364,6 +2367,25 @@ describe('SavedObjectsRepository', () => {
expect(client.get).toHaveBeenCalledTimes(1);
});

it(`throws when the document has multiple workspaces and the force option is not enabled`, async () => {
const workspaces = ['foo-workspace', 'bar-workspace'];
const response = getMockGetResponse({
type: NAMESPACE_AGNOSTIC_TYPE,
id,
namespace,
workspaces,
});
client.get.mockResolvedValueOnce(
opensearchClientMock.createSuccessTransportRequestPromise(response)
);
await expect(
savedObjectsRepository.delete(NAMESPACE_AGNOSTIC_TYPE, id, { namespace })
).rejects.toThrowError(
'Unable to delete saved object that exists in multiple workspaces, use the `force` option to delete it anyway'
);
expect(client.get).toHaveBeenCalledTimes(1);
});

it(`throws when the type is multi-namespace and the document has all namespaces and the force option is not enabled`, async () => {
const response = getMockGetResponse({ type: MULTI_NAMESPACE_TYPE, id, namespace });
response._source.namespaces = ['*'];
Expand All @@ -2379,6 +2401,7 @@ describe('SavedObjectsRepository', () => {
});

it(`throws when OpenSearch is unable to find the document during delete`, async () => {
mockGet(type, id);
client.delete.mockResolvedValueOnce(
opensearchClientMock.createSuccessTransportRequestPromise({ result: 'not_found' })
);
Expand All @@ -2387,6 +2410,7 @@ describe('SavedObjectsRepository', () => {
});

it(`throws when OpenSearch is unable to find the index during delete`, async () => {
mockGet(type, id);
client.delete.mockResolvedValueOnce(
opensearchClientMock.createSuccessTransportRequestPromise({
error: { type: 'index_not_found_exception' },
Expand All @@ -2397,6 +2421,7 @@ describe('SavedObjectsRepository', () => {
});

it(`throws when OpenSearch returns an unexpected response`, async () => {
mockGet(type, id);
client.delete.mockResolvedValueOnce(
opensearchClientMock.createSuccessTransportRequestPromise({
result: 'something unexpected',
Expand Down
155 changes: 155 additions & 0 deletions src/core/server/saved_objects/service/lib/repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,11 @@ import {
SavedObjectsCheckConflictsObject,
SavedObjectsCheckConflictsResponse,
SavedObjectsCreateOptions,
SavedObjectsDeleteByWorkspaceOptions,
SavedObjectsDeleteFromNamespacesOptions,
SavedObjectsDeleteFromNamespacesResponse,
SavedObjectsDeleteFromWorkspacesOptions,
SavedObjectsDeleteFromWorkspacesResponse,
SavedObjectsDeleteOptions,
SavedObjectsFindResponse,
SavedObjectsFindResult,
Expand Down Expand Up @@ -727,6 +730,14 @@ export class SavedObjectsRepository {
}
}

const obj = await this.get(type, id, { namespace });
const existingWorkspace = obj.workspaces || [];
if (!force && existingWorkspace.length > 1) {
throw SavedObjectsErrorHelpers.createBadRequestError(
'Unable to delete saved object that exists in multiple workspaces, use the `force` option to delete it anyway'
);
}

const { body, statusCode } = await this.client.delete<DeleteDocumentResponse>(
{
id: rawId,
Expand Down Expand Up @@ -807,6 +818,55 @@ export class SavedObjectsRepository {
return body;
}

/**
* Deletes all objects from the provided workspace.
*
* @param {string} workspace
* @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 || workspace === '*') {
throw new TypeError(`workspace is required, and must be a string that is not equal to '*'`);
}

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

const { body } = await this.client.updateByQuery(
{
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;
}

/**
* @param {object} [options={}]
* @property {(string|Array<string>)} [options.type]
Expand Down Expand Up @@ -1459,6 +1519,101 @@ export class SavedObjectsRepository {
});
}

/**
* Removes one or more workspace from a given saved object. If no workspace remain, the saved object is deleted
* entirely. This method and [`addToWorkspaces`]{@link SavedObjectsRepository.addToWorkspaces} are the only ways to change which workspace a
* saved object is shared to.
*/
async deleteFromWorkspaces(
type: string,
id: string,
workspaces: string[],
options: SavedObjectsDeleteFromWorkspacesOptions = {}
): Promise<SavedObjectsDeleteFromWorkspacesResponse> {
if (!this._allowedTypes.includes(type)) {
throw SavedObjectsErrorHelpers.createGenericNotFoundError(type, id);
}

if (!workspaces.length) {
throw SavedObjectsErrorHelpers.createBadRequestError(
'workspaces must be a non-empty array of strings'
);
}

const { refresh = DEFAULT_REFRESH_SETTING } = options;

const rawId = this._serializer.generateRawId(undefined, type, id);
const savedObject = await this.get(type, id);
const existingWorkspaces = savedObject.workspaces;
// if there are somehow no existing workspaces, allow the operation to proceed and delete this saved object
const remainingWorkspaces = existingWorkspaces?.filter((x) => !workspaces.includes(x));

if (remainingWorkspaces?.length) {
// if there is 1 or more workspace remaining, update the saved object
const time = this._getCurrentTime();

const doc = {
updated_at: time,
workspaces: remainingWorkspaces,
};

const { statusCode } = await this.client.update(
{
id: rawId,
index: this.getIndexForType(type),
...decodeRequestVersion(savedObject.version),
refresh,

body: {
doc,
},
},
{
ignore: [404],
}
);

if (statusCode === 404) {
// see "404s from missing index" above
throw SavedObjectsErrorHelpers.createGenericNotFoundError(type, id);
}
return { workspaces: doc.workspaces };
} else {
// if there are no namespaces remaining, delete the saved object
const { body, statusCode } = await this.client.delete<DeleteDocumentResponse>(
{
id: rawId,
index: this.getIndexForType(type),
refresh,
...decodeRequestVersion(savedObject.version),
},
{
ignore: [404],
}
);

const deleted = body.result === 'deleted';
if (deleted) {
return { workspaces: [] };
}

const deleteDocNotFound = body.result === 'not_found';
const deleteIndexNotFound = body.error && body.error.type === 'index_not_found_exception';
if (deleteDocNotFound || deleteIndexNotFound) {
// see "404s from missing index" above
throw SavedObjectsErrorHelpers.createGenericNotFoundError(type, id);
}

throw new Error(
`Unexpected OpenSearch DELETE response: ${JSON.stringify({
type,
id,
response: { body, statusCode },
})}`
);
}
}

/**
* Updates multiple objects in bulk
*
Expand Down
27 changes: 27 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 @@ -228,6 +228,16 @@ export interface SavedObjectsDeleteFromNamespacesOptions extends SavedObjectsBas
refresh?: MutatingOperationRefreshSetting;
}

export interface SavedObjectsDeleteFromWorkspacesOptions {
/** The OpenSearch Refresh setting for this operation */
refresh?: MutatingOperationRefreshSetting;
}

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

/**
*
* @public
Expand All @@ -237,6 +247,11 @@ export interface SavedObjectsDeleteFromNamespacesResponse {
namespaces: string[];
}

export interface SavedObjectsDeleteFromWorkspacesResponse {
/** The workspaces the object exists in after this operation is complete. An empty array indicates the object was deleted. */
workspaces: string[];
}

/**
*
* @public
Expand Down Expand Up @@ -485,6 +500,18 @@ export class SavedObjectsClient {
return await this._repository.processFindOptions(props);
};

/**
* 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);
};

/**
* Bulk Updates multiple SavedObject at once
*
Expand Down
1 change: 1 addition & 0 deletions src/core/types/workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,5 @@ export interface WorkspaceAttribute {
color?: string;
icon?: string;
defaultVISTheme?: string;
reserved?: boolean;
}
11 changes: 7 additions & 4 deletions src/plugins/workspace/server/saved_objects/workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,19 @@ export const workspace: SavedObjectsType = {
* In opensearch, string[] is also mapped to text
*/
features: {
type: 'text',
type: 'keyword',
},
color: {
type: 'text',
type: 'keyword',
},
icon: {
type: 'text',
type: 'keyword',
},
defaultVISTheme: {
type: 'text',
type: 'keyword',
},
reserved: {
type: 'boolean',
},
},
},
Expand Down
Loading

0 comments on commit 0f704e6

Please sign in to comment.