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] Delete the virtual global workspace #7165

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
2 changes: 2 additions & 0 deletions changelogs/fragments/7165.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
feat:
- [Workspace] Delete the virtual global workspace ([#7165](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7165))
2 changes: 0 additions & 2 deletions src/core/public/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,6 @@ export {
DEFAULT_APP_CATEGORIES,
WORKSPACE_TYPE,
cleanWorkspaceId,
PUBLIC_WORKSPACE_ID,
PUBLIC_WORKSPACE_NAME,
DEFAULT_NAV_GROUPS,
} from '../utils';
export {
Expand Down
7 changes: 1 addition & 6 deletions src/core/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -357,12 +357,7 @@ export {
} from './metrics';

export { AppCategory, WorkspaceAttribute } from '../types';
export {
DEFAULT_APP_CATEGORIES,
PUBLIC_WORKSPACE_ID,
PUBLIC_WORKSPACE_NAME,
WORKSPACE_TYPE,
} from '../utils';
export { DEFAULT_APP_CATEGORIES, WORKSPACE_TYPE } from '../utils';

export {
SavedObject,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,16 +134,6 @@ function getClauseForType(
* Gets the clause that will filter for the workspace.
*/
function getClauseForWorkspace(workspace: string) {
if (workspace === '*') {
return {
bool: {
must: {
match_all: {},
},
},
};
}

return {
bool: {
must: [{ term: { workspaces: workspace } }],
Expand Down
12 changes: 0 additions & 12 deletions src/core/utils/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,6 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { i18n } from '@osd/i18n';

export const WORKSPACE_TYPE = 'workspace';

export const WORKSPACE_PATH_PREFIX = '/w';

/**
* public workspace has parity with global tenant,
* it includes saved objects with `public` as its workspace or without any workspce info
*/
export const PUBLIC_WORKSPACE_ID = 'public';

export const PUBLIC_WORKSPACE_NAME = i18n.translate('workspaces.public.workspace.default.name', {
defaultMessage: 'Global workspace',
});
7 changes: 1 addition & 6 deletions src/core/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,6 @@ export {
IContextProvider,
} from './context';
export { DEFAULT_APP_CATEGORIES } from './default_app_categories';
export {
WORKSPACE_PATH_PREFIX,
WORKSPACE_TYPE,
PUBLIC_WORKSPACE_ID,
PUBLIC_WORKSPACE_NAME,
} from './constants';
export { WORKSPACE_PATH_PREFIX, WORKSPACE_TYPE } from './constants';
export { getWorkspaceIdFromUrl, formatUrlWithWorkspaceId, cleanWorkspaceId } from './workspace';
export { DEFAULT_NAV_GROUPS } from './default_nav_groups';
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ import {
import { Flyout, Relationships } from './components';
import { SavedObjectWithMetadata } from '../../types';
import { WorkspaceObject } from 'opensearch-dashboards/public';
import { PUBLIC_WORKSPACE_NAME, PUBLIC_WORKSPACE_ID } from '../../../../../core/public';
import { TableProps } from './components/table';

const allowedTypes = ['index-pattern', 'visualization', 'dashboard', 'search'];
Expand Down Expand Up @@ -413,10 +412,17 @@ describe('SavedObjectsTable', () => {
});

it('should export all, accounting for the current workspace criteria', async () => {
const component = shallowRender();
const workspaceList: WorkspaceObject[] = [
{
id: 'workspace1',
name: 'foo',
},
];
workspaces.workspaceList$.next(workspaceList);
const component = shallowRender({ workspaces });

component.instance().onQueryChange({
query: Query.parse(`test workspaces:("${PUBLIC_WORKSPACE_NAME}")`),
query: Query.parse(`test workspaces:("foo")`),
});

// Ensure all promises resolve
Expand All @@ -435,7 +441,7 @@ describe('SavedObjectsTable', () => {
allowedTypes,
'test*',
true,
[PUBLIC_WORKSPACE_ID]
['workspace1']
);
expect(saveAsMock).toHaveBeenCalledWith(blob, 'export.ndjson');
expect(notifications.toasts.addSuccess).toHaveBeenCalledWith({
Expand Down Expand Up @@ -708,10 +714,9 @@ describe('SavedObjectsTable', () => {
expect(filters.length).toBe(2);
expect(filters[0].field).toBe('type');
expect(filters[1].field).toBe('workspaces');
expect(filters[1].options.length).toBe(3);
expect(filters[1].options.length).toBe(2);
expect(filters[1].options[0].value).toBe('foo');
expect(filters[1].options[1].value).toBe('bar');
expect(filters[1].options[2].value).toBe(PUBLIC_WORKSPACE_NAME);
});

it('workspace filter only include current workspaces when in a workspace', async () => {
Expand Down Expand Up @@ -847,7 +852,7 @@ describe('SavedObjectsTable', () => {
expect(findObjectsMock).toBeCalledWith(
http,
expect.objectContaining({
workspaces: expect.arrayContaining(['workspace1', 'workspace2', PUBLIC_WORKSPACE_ID]),
workspaces: expect.arrayContaining(['workspace1', 'workspace2']),
})
);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ import {
WorkspaceAttribute,
} from 'src/core/public';
import { Subscription } from 'rxjs';
import { PUBLIC_WORKSPACE_ID, PUBLIC_WORKSPACE_NAME } from '../../../../../core/public';
import { RedirectAppLinks } from '../../../../opensearch_dashboards_react/public';
import { IndexPatternsContract } from '../../../../data/public';
import {
Expand Down Expand Up @@ -195,7 +194,7 @@ export class SavedObjectsTable extends Component<SavedObjectsTableProps, SavedOb
// application home
if (!currentWorkspaceId) {
// public workspace is virtual at this moment
return availableWorkspaces?.map((ws) => ws.id).concat(PUBLIC_WORKSPACE_ID);
return availableWorkspaces?.map((ws) => ws.id);
} else {
return [currentWorkspaceId];
}
Expand All @@ -205,7 +204,6 @@ export class SavedObjectsTable extends Component<SavedObjectsTableProps, SavedOb
private get workspaceNameIdLookup() {
const { availableWorkspaces } = this.state;
const workspaceNameIdMap = new Map<string, string>();
workspaceNameIdMap.set(PUBLIC_WORKSPACE_NAME, PUBLIC_WORKSPACE_ID);
// workspace name is unique across the system
availableWorkspaces?.forEach((workspace) => {
workspaceNameIdMap.set(workspace.name, workspace.id);
Expand Down Expand Up @@ -956,8 +954,6 @@ export class SavedObjectsTable extends Component<SavedObjectsTableProps, SavedOb
// Add workspace filter
if (workspaceEnabled && availableWorkspaces?.length) {
const wsCounts = savedObjectCounts.workspaces || {};
const publicWorkspaceExists =
availableWorkspaces.findIndex((workspace) => workspace.id === PUBLIC_WORKSPACE_ID) > -1;
const wsFilterOptions = availableWorkspaces
.filter((ws) => {
return this.workspaceIdQuery?.includes(ws.id);
Expand All @@ -970,15 +966,6 @@ export class SavedObjectsTable extends Component<SavedObjectsTableProps, SavedOb
};
});

// add public workspace option only if we don't have it as real workspace
if (!currentWorkspaceId && !publicWorkspaceExists) {
wsFilterOptions.push({
name: PUBLIC_WORKSPACE_NAME,
value: PUBLIC_WORKSPACE_NAME,
view: `${PUBLIC_WORKSPACE_NAME} (${wsCounts[PUBLIC_WORKSPACE_ID] || 0})`,
});
}

filters.push({
type: 'field_value_selection',
field: 'workspaces',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@

import { schema } from '@osd/config-schema';
import { IRouter, SavedObjectsFindOptions } from 'src/core/server';
import { PUBLIC_WORKSPACE_ID } from '../../../../core/server';
import { findAll } from '../lib';

export const registerScrollForCountRoute = (router: IRouter) => {
Expand Down Expand Up @@ -92,7 +91,7 @@ export const registerScrollForCountRoute = (router: IRouter) => {
});
}
if (requestHasWorkspaces) {
const resultWorkspaces = result.workspaces || [PUBLIC_WORKSPACE_ID];
const resultWorkspaces = result.workspaces || [];
resultWorkspaces.forEach((ws) => {
counts.workspaces[ws] = counts.workspaces[ws] || 0;
counts.workspaces[ws]++;
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/workspace/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ export class WorkspacePlugin implements Plugin<WorkspacePluginSetup, WorkspacePl
core.savedObjects.addClientWrapper(
PRIORITY_FOR_WORKSPACE_ID_CONSUMER_WRAPPER,
WORKSPACE_ID_CONSUMER_WRAPPER_ID,
new WorkspaceIdConsumerWrapper(isPermissionControlEnabled).wrapperFactory
new WorkspaceIdConsumerWrapper().wrapperFactory
);

const maxImportExportSize = core.savedObjects.getImportExportObjectLimit();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

import { updateWorkspaceState } from '../../../../core/server/utils';
import { SavedObject } from '../../../../core/public';
import { PUBLIC_WORKSPACE_ID } from '../../../../core/server';
import { httpServerMock, savedObjectsClientMock, coreMock } from '../../../../core/server/mocks';
import { WorkspaceIdConsumerWrapper } from './workspace_id_consumer_wrapper';

Expand Down Expand Up @@ -116,69 +115,8 @@ describe('WorkspaceIdConsumerWrapper', () => {
});
});

it(`Should set workspacesSearchOperator to OR when search with public workspace`, async () => {
await wrapperClient.find({
type: 'dashboard',
workspaces: [PUBLIC_WORKSPACE_ID],
});
expect(mockedClient.find).toBeCalledWith({
type: 'dashboard',
workspaces: [PUBLIC_WORKSPACE_ID],
workspacesSearchOperator: 'OR',
});
});

it(`Should set workspace as pubic when workspace is not specified`, async () => {
const mockRequest = httpServerMock.createOpenSearchDashboardsRequest();
updateWorkspaceState(mockRequest, {});
const mockedWrapperClient = wrapperInstance.wrapperFactory({
client: mockedClient,
typeRegistry: requestHandlerContext.savedObjects.typeRegistry,
request: mockRequest,
});
await mockedWrapperClient.find({
type: ['dashboard', 'visualization'],
});
expect(mockedClient.find).toBeCalledWith({
type: ['dashboard', 'visualization'],
workspaces: [PUBLIC_WORKSPACE_ID],
workspacesSearchOperator: 'OR',
});
});

it(`Should remove public workspace when permission control is enabled`, async () => {
const consumer = new WorkspaceIdConsumerWrapper(true);
const client = consumer.wrapperFactory({
client: mockedClient,
typeRegistry: requestHandlerContext.savedObjects.typeRegistry,
request: workspaceEnabledMockRequest,
});
await client.find({
type: 'dashboard',
workspaces: ['bar', PUBLIC_WORKSPACE_ID],
});
expect(mockedClient.find).toBeCalledWith({
type: 'dashboard',
workspaces: ['bar'],
workspacesSearchOperator: 'OR',
});
});

it(`Should not override workspacesSearchOperator when workspacesSearchOperator is specified`, async () => {
await wrapperClient.find({
type: 'dashboard',
workspaces: [PUBLIC_WORKSPACE_ID],
workspacesSearchOperator: 'AND',
});
expect(mockedClient.find).toBeCalledWith({
type: 'dashboard',
workspaces: [PUBLIC_WORKSPACE_ID],
workspacesSearchOperator: 'AND',
});
});

it(`Should not pass a empty workspace array`, async () => {
const workspaceIdConsumerWrapper = new WorkspaceIdConsumerWrapper(true);
it(`Should pass a empty workspace array`, async () => {
const workspaceIdConsumerWrapper = new WorkspaceIdConsumerWrapper();
const mockRequest = httpServerMock.createOpenSearchDashboardsRequest();
updateWorkspaceState(mockRequest, {});
const mockedWrapperClient = workspaceIdConsumerWrapper.wrapperFactory({
Expand All @@ -189,10 +127,8 @@ describe('WorkspaceIdConsumerWrapper', () => {
await mockedWrapperClient.find({
type: ['dashboard', 'visualization'],
});
// empty workspace array will get deleted
expect(mockedClient.find).toBeCalledWith({
type: ['dashboard', 'visualization'],
workspacesSearchOperator: 'OR',
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ import {
SavedObjectsCheckConflictsObject,
OpenSearchDashboardsRequest,
SavedObjectsFindOptions,
PUBLIC_WORKSPACE_ID,
WORKSPACE_TYPE,
} from '../../../../core/server';

type WorkspaceOptions = Pick<SavedObjectsBaseOptions, 'workspaces'> | undefined;
Expand Down Expand Up @@ -41,14 +39,6 @@ export class WorkspaceIdConsumerWrapper {
};
}

private isWorkspaceType(type: SavedObjectsFindOptions['type']): boolean {
if (Array.isArray(type)) {
return type.every((item) => item === WORKSPACE_TYPE);
}

return type === WORKSPACE_TYPE;
}

public wrapperFactory: SavedObjectsClientWrapperFactory = (wrapperOptions) => {
return {
...wrapperOptions.client,
Expand Down Expand Up @@ -76,32 +66,9 @@ export class WorkspaceIdConsumerWrapper {
),
delete: wrapperOptions.client.delete,
find: (options: SavedObjectsFindOptions) => {
const findOptions = this.formatWorkspaceIdParams(wrapperOptions.request, options);
if (this.isWorkspaceType(findOptions.type)) {
return wrapperOptions.client.find(findOptions);
}

// if workspace is enabled, we always find by workspace
if (!findOptions.workspaces || findOptions.workspaces.length === 0) {
findOptions.workspaces = [PUBLIC_WORKSPACE_ID];
}

// `PUBLIC_WORKSPACE_ID` includes both saved objects without any workspace and with `PUBLIC_WORKSPACE_ID` workspace
const index = findOptions.workspaces
? findOptions.workspaces.indexOf(PUBLIC_WORKSPACE_ID)
: -1;
if (!findOptions.workspacesSearchOperator && findOptions.workspaces && index !== -1) {
findOptions.workspacesSearchOperator = 'OR';
// remove this deletion logic when public workspace becomes to real
if (this.isPermissionControlEnabled) {
// remove public workspace to make sure we can pass permission control validation, more details in `WorkspaceSavedObjectsClientWrapper`
findOptions.workspaces.splice(index, 1);
}
}
if (findOptions.workspaces && findOptions.workspaces.length === 0) {
delete findOptions.workspaces;
}
return wrapperOptions.client.find(findOptions);
return wrapperOptions.client.find(
this.formatWorkspaceIdParams(wrapperOptions.request, options)
);
},
bulkGet: wrapperOptions.client.bulkGet,
get: wrapperOptions.client.get,
Expand All @@ -113,5 +80,5 @@ export class WorkspaceIdConsumerWrapper {
};
};

constructor(private isPermissionControlEnabled?: boolean) {}
constructor() {}
}
Loading