From 6510e005821d4cac79b2550806b74f2e20ea1276 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 4 Mar 2024 10:14:05 +0800 Subject: [PATCH 1/6] feat: cherry-pick pr #5949 Signed-off-by: SuZhou-Joe --- CHANGELOG.md | 6 ++ config/opensearch_dashboards.yml | 2 +- .../import/create_saved_objects.ts | 3 +- .../import/import_saved_objects.ts | 2 +- .../import/resolve_import_errors.ts | 3 +- src/core/server/saved_objects/import/types.ts | 4 ++ .../core/build_active_mappings.test.ts | 7 +++ .../migrations/core/build_active_mappings.ts | 17 ++++-- .../migrations/core/index_migrator.test.ts | 55 +++++++++++++++++++ .../migrations/core/migration_context.ts | 16 ++++-- .../opensearch_dashboards_migrator.test.ts | 15 ++++- .../opensearch_dashboards_migrator.ts | 11 +++- .../server/saved_objects/routes/import.ts | 4 ++ .../routes/resolve_import_errors.ts | 3 + .../saved_objects/saved_objects_service.ts | 8 +++ .../service/saved_objects_client.ts | 4 ++ ...apper_for_check_workspace_conflict.test.ts | 21 +++++++ ...apper_for_check_workspace_conflict.test.ts | 51 +++++++++++++++++ ...ts_wrapper_for_check_workspace_conflict.ts | 21 +++++-- 19 files changed, 234 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d49655b69e70..b16ab9a73eac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,12 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Multiple Datasource] Add interfaces to register add-on authentication method from plug-in module ([#5851](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5851)) - [Multiple Datasource] Able to Hide "Local Cluster" option from datasource DropDown ([#5827](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5827)) - [Multiple Datasource] Add api registry and allow it to be added into client config in data source plugin ([#5895](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5895)) +- [Multiple Datasource] Concatenate data source name with index pattern name and change delimiter to double colon ([#5907](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5907)) +- [Multiple Datasource] Refactor client and legacy client to use authentication registry ([#5881](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5881)) +- [Multiple Datasource] Improved error handling for the search API when a null value is passed for the dataSourceId ([#5882](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5882)) +- [Multiple Datasource] Hide/Show authentication method in multi data source plugin based on configuration ([#5916](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5916)) +- [[Dynamic Configurations] Add support for dynamic application configurations ([#5855](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5855)) +- [Workspace] Optional workspaces params in repository ([#5949](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5949)) ### 🐛 Bug Fixes diff --git a/config/opensearch_dashboards.yml b/config/opensearch_dashboards.yml index 0e5beac120c0..523a483d1cec 100644 --- a/config/opensearch_dashboards.yml +++ b/config/opensearch_dashboards.yml @@ -276,7 +276,7 @@ # Set the value of this setting to true to enable plugin augmentation on Dashboard # vis_augmenter.pluginAugmentationEnabled: true -# Set the value to true enable workspace feature +# Set the value to true to enable workspace feature # workspace.enabled: false # Set the value to false to disable permission check on workspace # Permission check depends on OpenSearch Dashboards has authentication enabled, set it to false if no authentication is configured diff --git a/src/core/server/saved_objects/import/create_saved_objects.ts b/src/core/server/saved_objects/import/create_saved_objects.ts index 67bc8dfc6227..3bc970dfa358 100644 --- a/src/core/server/saved_objects/import/create_saved_objects.ts +++ b/src/core/server/saved_objects/import/create_saved_objects.ts @@ -42,6 +42,7 @@ interface CreateSavedObjectsParams { workspaces?: string[]; dataSourceId?: string; dataSourceTitle?: string; + workspaces?: string[]; } interface CreateSavedObjectsResult { createdObjects: Array>; @@ -59,9 +60,9 @@ export const createSavedObjects = async ({ importIdMap, namespace, overwrite, - workspaces, dataSourceId, dataSourceTitle, + workspaces, }: CreateSavedObjectsParams): Promise> => { // filter out any objects that resulted in errors const errorSet = accumulatedErrors.reduce( diff --git a/src/core/server/saved_objects/import/import_saved_objects.ts b/src/core/server/saved_objects/import/import_saved_objects.ts index 7cdb6970ca9d..fa97d3f55b43 100644 --- a/src/core/server/saved_objects/import/import_saved_objects.ts +++ b/src/core/server/saved_objects/import/import_saved_objects.ts @@ -55,9 +55,9 @@ export async function importSavedObjectsFromStream({ savedObjectsClient, typeRegistry, namespace, - workspaces, dataSourceId, dataSourceTitle, + workspaces, }: SavedObjectsImportOptions): Promise { let errorAccumulator: SavedObjectsImportError[] = []; const supportedTypes = typeRegistry.getImportableAndExportableTypes().map((type) => type.name); diff --git a/src/core/server/saved_objects/import/resolve_import_errors.ts b/src/core/server/saved_objects/import/resolve_import_errors.ts index a3d10c6f1ace..0c07acd6c9f0 100644 --- a/src/core/server/saved_objects/import/resolve_import_errors.ts +++ b/src/core/server/saved_objects/import/resolve_import_errors.ts @@ -59,9 +59,9 @@ export async function resolveSavedObjectsImportErrors({ typeRegistry, namespace, createNewCopies, - workspaces, dataSourceId, dataSourceTitle, + workspaces, }: SavedObjectsResolveImportErrorsOptions): Promise { // throw a BadRequest error if we see invalid retries validateRetries(retries); @@ -166,6 +166,7 @@ export async function resolveSavedObjectsImportErrors({ workspaces, dataSourceId, dataSourceTitle, + workspaces, }; const { createdObjects, errors: bulkCreateErrors } = await createSavedObjects( createSavedObjectsParams diff --git a/src/core/server/saved_objects/import/types.ts b/src/core/server/saved_objects/import/types.ts index d0433b72766a..08c0f23b4331 100644 --- a/src/core/server/saved_objects/import/types.ts +++ b/src/core/server/saved_objects/import/types.ts @@ -191,6 +191,8 @@ export interface SavedObjectsImportOptions { workspaces?: string[]; dataSourceId?: string; dataSourceTitle?: string; + /** if specified, will import in given workspaces */ + workspaces?: string[]; } /** @@ -216,6 +218,8 @@ export interface SavedObjectsResolveImportErrorsOptions { workspaces?: string[]; dataSourceId?: string; dataSourceTitle?: string; + /** if specified, will import in given workspaces */ + workspaces?: string[]; } export type CreatedObject = SavedObject & { destinationId?: string }; diff --git a/src/core/server/saved_objects/migrations/core/build_active_mappings.test.ts b/src/core/server/saved_objects/migrations/core/build_active_mappings.test.ts index c70dbbb241bc..4acc161c4bab 100644 --- a/src/core/server/saved_objects/migrations/core/build_active_mappings.test.ts +++ b/src/core/server/saved_objects/migrations/core/build_active_mappings.test.ts @@ -30,6 +30,7 @@ import { IndexMapping, SavedObjectsTypeMappingDefinitions } from './../../mappings'; import { buildActiveMappings, diffMappings } from './build_active_mappings'; +import { configMock } from '../../../config/mocks'; describe('buildActiveMappings', () => { test('creates a strict mapping', () => { @@ -91,6 +92,12 @@ describe('buildActiveMappings', () => { expect(hashes.aaa).toEqual(hashes.bbb); expect(hashes.aaa).not.toEqual(hashes.ccc); }); + + test('workspaces field is added when workspace feature flag is enabled', () => { + const rawConfig = configMock.create(); + rawConfig.get.mockReturnValue(true); + expect(buildActiveMappings({}, rawConfig)).toHaveProperty('properties.workspaces'); + }); }); describe('diffMappings', () => { diff --git a/src/core/server/saved_objects/migrations/core/build_active_mappings.ts b/src/core/server/saved_objects/migrations/core/build_active_mappings.ts index 05fb534f7a11..8f301debf6f7 100644 --- a/src/core/server/saved_objects/migrations/core/build_active_mappings.ts +++ b/src/core/server/saved_objects/migrations/core/build_active_mappings.ts @@ -34,6 +34,7 @@ import crypto from 'crypto'; import { cloneDeep, mapValues } from 'lodash'; +import { Config } from 'packages/osd-config/target'; import { IndexMapping, SavedObjectsFieldMapping, @@ -48,11 +49,20 @@ import { * @param typeDefinitions - the type definitions to build mapping from. */ export function buildActiveMappings( - typeDefinitions: SavedObjectsTypeMappingDefinitions | SavedObjectsMappingProperties + typeDefinitions: SavedObjectsTypeMappingDefinitions | SavedObjectsMappingProperties, + opensearchDashboardsRawConfig?: Config ): IndexMapping { const mapping = defaultMapping(); - const mergedProperties = validateAndMerge(mapping.properties, typeDefinitions); + let mergedProperties = validateAndMerge(mapping.properties, typeDefinitions); + // if permission control for saved objects is enabled, the permissions field should be added to the mapping + if (opensearchDashboardsRawConfig?.get('workspace.enabled')) { + mergedProperties = validateAndMerge(mapping.properties, { + workspaces: { + type: 'keyword', + }, + }); + } return cloneDeep({ ...mapping, @@ -186,9 +196,6 @@ function defaultMapping(): IndexMapping { }, }, }, - workspaces: { - type: 'keyword', - }, permissions: { properties: { read: principals, diff --git a/src/core/server/saved_objects/migrations/core/index_migrator.test.ts b/src/core/server/saved_objects/migrations/core/index_migrator.test.ts index 08bc4162a807..3188ff87afe8 100644 --- a/src/core/server/saved_objects/migrations/core/index_migrator.test.ts +++ b/src/core/server/saved_objects/migrations/core/index_migrator.test.ts @@ -36,6 +36,7 @@ import { SavedObjectTypeRegistry } from '../../saved_objects_type_registry'; import { IndexMigrator } from './index_migrator'; import { MigrationOpts } from './migration_context'; import { loggingSystemMock } from '../../../logging/logging_system.mock'; +import { configMock } from '../../../config/mocks'; describe('IndexMigrator', () => { let testOpts: jest.Mocked & { @@ -59,6 +60,60 @@ describe('IndexMigrator', () => { }; }); + test('creates the index when workspaces feature flag is enabled', async () => { + const { client } = testOpts; + + testOpts.mappingProperties = { foo: { type: 'long' } as any }; + const rawConfig = configMock.create(); + rawConfig.get.mockReturnValue(true); + testOpts.opensearchDashboardsRawConfig = rawConfig; + + withIndex(client, { index: { statusCode: 404 }, alias: { statusCode: 404 } }); + + await new IndexMigrator(testOpts).migrate(); + + expect(client.indices.create).toHaveBeenCalledWith({ + body: { + mappings: { + dynamic: 'strict', + _meta: { + migrationMappingPropertyHashes: { + foo: '18c78c995965207ed3f6e7fc5c6e55fe', + migrationVersion: '4a1746014a75ade3a714e1db5763276f', + namespace: '2f4316de49999235636386fe51dc06c1', + namespaces: '2f4316de49999235636386fe51dc06c1', + originId: '2f4316de49999235636386fe51dc06c1', + references: '7997cf5a56cc02bdc9c93361bde732b0', + type: '2f4316de49999235636386fe51dc06c1', + updated_at: '00da57df13e94e9d98437d13ace4bfe0', + workspaces: '2f4316de49999235636386fe51dc06c1', + }, + }, + properties: { + foo: { type: 'long' }, + migrationVersion: { dynamic: 'true', type: 'object' }, + namespace: { type: 'keyword' }, + namespaces: { type: 'keyword' }, + originId: { type: 'keyword' }, + type: { type: 'keyword' }, + updated_at: { type: 'date' }, + references: { + type: 'nested', + properties: { + name: { type: 'keyword' }, + type: { type: 'keyword' }, + id: { type: 'keyword' }, + }, + }, + workspaces: { type: 'keyword' }, + }, + }, + settings: { number_of_shards: 1, auto_expand_replicas: '0-1' }, + }, + index: '.kibana_1', + }); + }); + test('creates the index if it does not exist', async () => { const { client } = testOpts; diff --git a/src/core/server/saved_objects/migrations/core/migration_context.ts b/src/core/server/saved_objects/migrations/core/migration_context.ts index 82001f7ed4c4..8a1e9b648bce 100644 --- a/src/core/server/saved_objects/migrations/core/migration_context.ts +++ b/src/core/server/saved_objects/migrations/core/migration_context.ts @@ -36,6 +36,7 @@ */ import { Logger } from 'src/core/server/logging'; +import { Config } from 'packages/osd-config/target'; import { MigrationOpenSearchClient } from './migration_opensearch_client'; import { SavedObjectsSerializer } from '../../serialization'; import { @@ -65,6 +66,7 @@ export interface MigrationOpts { * prior to running migrations. For example: 'opensearch_dashboards_index_template*' */ obsoleteIndexTemplatePattern?: string; + opensearchDashboardsRawConfig?: Config; } /** @@ -90,10 +92,15 @@ export interface Context { * and various info needed to migrate the source index. */ export async function migrationContext(opts: MigrationOpts): Promise { - const { log, client } = opts; + const { log, client, opensearchDashboardsRawConfig } = opts; const alias = opts.index; const source = createSourceContext(await Index.fetchInfo(client, alias), alias); - const dest = createDestContext(source, alias, opts.mappingProperties); + const dest = createDestContext( + source, + alias, + opts.mappingProperties, + opensearchDashboardsRawConfig + ); return { client, @@ -125,10 +132,11 @@ function createSourceContext(source: Index.FullIndexInfo, alias: string) { function createDestContext( source: Index.FullIndexInfo, alias: string, - typeMappingDefinitions: SavedObjectsTypeMappingDefinitions + typeMappingDefinitions: SavedObjectsTypeMappingDefinitions, + opensearchDashboardsRawConfig?: Config ): Index.FullIndexInfo { const targetMappings = disableUnknownTypeMappingFields( - buildActiveMappings(typeMappingDefinitions), + buildActiveMappings(typeMappingDefinitions, opensearchDashboardsRawConfig), source.mappings ); diff --git a/src/core/server/saved_objects/migrations/opensearch_dashboards/opensearch_dashboards_migrator.test.ts b/src/core/server/saved_objects/migrations/opensearch_dashboards/opensearch_dashboards_migrator.test.ts index 32a1bc51a554..b0350a00b211 100644 --- a/src/core/server/saved_objects/migrations/opensearch_dashboards/opensearch_dashboards_migrator.test.ts +++ b/src/core/server/saved_objects/migrations/opensearch_dashboards/opensearch_dashboards_migrator.test.ts @@ -37,6 +37,7 @@ import { import { loggingSystemMock } from '../../../logging/logging_system.mock'; import { SavedObjectTypeRegistry } from '../../saved_objects_type_registry'; import { SavedObjectsType } from '../../types'; +import { configMock } from '../../../config/mocks'; const createRegistry = (types: Array>) => { const registry = new SavedObjectTypeRegistry(); @@ -76,6 +77,12 @@ describe('OpenSearchDashboardsMigrator', () => { const mappings = new OpenSearchDashboardsMigrator(options).getActiveMappings(); expect(mappings).toMatchSnapshot(); }); + + it('workspaces field exists in the mappings when the feature is enabled', () => { + const options = mockOptions(true); + const mappings = new OpenSearchDashboardsMigrator(options).getActiveMappings(); + expect(mappings).toHaveProperty('properties.workspaces'); + }); }); describe('runMigrations', () => { @@ -146,7 +153,12 @@ type MockedOptions = OpenSearchDashboardsMigratorOptions & { client: ReturnType; }; -const mockOptions = () => { +const mockOptions = (isWorkspaceEnabled?: boolean) => { + const rawConfig = configMock.create(); + rawConfig.get.mockReturnValue(false); + if (isWorkspaceEnabled) { + rawConfig.get.mockReturnValue(true); + } const options: MockedOptions = { logger: loggingSystemMock.create().get(), opensearchDashboardsVersion: '8.2.3', @@ -186,6 +198,7 @@ const mockOptions = () => { skip: false, }, client: opensearchClientMock.createOpenSearchClient(), + opensearchDashboardsRawConfig: rawConfig, }; return options; }; diff --git a/src/core/server/saved_objects/migrations/opensearch_dashboards/opensearch_dashboards_migrator.ts b/src/core/server/saved_objects/migrations/opensearch_dashboards/opensearch_dashboards_migrator.ts index 284615083af3..468aea3e905d 100644 --- a/src/core/server/saved_objects/migrations/opensearch_dashboards/opensearch_dashboards_migrator.ts +++ b/src/core/server/saved_objects/migrations/opensearch_dashboards/opensearch_dashboards_migrator.ts @@ -36,6 +36,7 @@ import { OpenSearchDashboardsConfigType } from 'src/core/server/opensearch_dashboards_config'; import { BehaviorSubject } from 'rxjs'; +import { Config } from 'packages/osd-config/target'; import { Logger } from '../../../logging'; import { IndexMapping, SavedObjectsTypeMappingDefinitions } from '../../mappings'; import { SavedObjectUnsanitizedDoc, SavedObjectsSerializer } from '../../serialization'; @@ -54,6 +55,7 @@ export interface OpenSearchDashboardsMigratorOptions { opensearchDashboardsConfig: OpenSearchDashboardsConfigType; opensearchDashboardsVersion: string; logger: Logger; + opensearchDashboardsRawConfig: Config; } export type IOpenSearchDashboardsMigrator = Pick< @@ -83,6 +85,7 @@ export class OpenSearchDashboardsMigrator { status: 'waiting', }); private readonly activeMappings: IndexMapping; + private readonly opensearchDashboardsRawConfig: Config; /** * Creates an instance of OpenSearchDashboardsMigrator. @@ -94,6 +97,7 @@ export class OpenSearchDashboardsMigrator { savedObjectsConfig, opensearchDashboardsVersion, logger, + opensearchDashboardsRawConfig, }: OpenSearchDashboardsMigratorOptions) { this.client = client; this.opensearchDashboardsConfig = opensearchDashboardsConfig; @@ -107,9 +111,13 @@ export class OpenSearchDashboardsMigrator { typeRegistry, log: this.log, }); + this.opensearchDashboardsRawConfig = opensearchDashboardsRawConfig; // Building the active mappings (and associated md5sums) is an expensive // operation so we cache the result - this.activeMappings = buildActiveMappings(this.mappingProperties); + this.activeMappings = buildActiveMappings( + this.mappingProperties, + this.opensearchDashboardsRawConfig + ); } /** @@ -181,6 +189,7 @@ export class OpenSearchDashboardsMigrator { ? 'opensearch_dashboards_index_template*' : undefined, convertToAliasScript: indexMap[index].script, + opensearchDashboardsRawConfig: this.opensearchDashboardsRawConfig, }); }); diff --git a/src/core/server/saved_objects/routes/import.ts b/src/core/server/saved_objects/routes/import.ts index 6c7cc77d9b84..c8878439b6c8 100644 --- a/src/core/server/saved_objects/routes/import.ts +++ b/src/core/server/saved_objects/routes/import.ts @@ -64,6 +64,9 @@ export const registerImportRoute = (router: IRouter, config: SavedObjectConfig) schema.oneOf([schema.string(), schema.arrayOf(schema.string())]) ), dataSourceId: schema.maybe(schema.string({ defaultValue: '' })), + workspaces: schema.maybe( + schema.oneOf([schema.string(), schema.arrayOf(schema.string())]) + ), }, { validate: (object) => { @@ -126,6 +129,7 @@ export const registerImportRoute = (router: IRouter, config: SavedObjectConfig) workspaces, dataSourceId, dataSourceTitle, + workspaces, }); return res.ok({ body: result }); diff --git a/src/core/server/saved_objects/routes/resolve_import_errors.ts b/src/core/server/saved_objects/routes/resolve_import_errors.ts index dedcc960a675..0ea91e43a777 100644 --- a/src/core/server/saved_objects/routes/resolve_import_errors.ts +++ b/src/core/server/saved_objects/routes/resolve_import_errors.ts @@ -62,6 +62,9 @@ export const registerResolveImportErrorsRoute = (router: IRouter, config: SavedO schema.oneOf([schema.string(), schema.arrayOf(schema.string())]) ), dataSourceId: schema.maybe(schema.string({ defaultValue: '' })), + workspaces: schema.maybe( + schema.oneOf([schema.string(), schema.arrayOf(schema.string())]) + ), }), body: schema.object({ file: schema.stream(), diff --git a/src/core/server/saved_objects/saved_objects_service.ts b/src/core/server/saved_objects/saved_objects_service.ts index 64c8b6a5fbc8..2a1cff648fae 100644 --- a/src/core/server/saved_objects/saved_objects_service.ts +++ b/src/core/server/saved_objects/saved_objects_service.ts @@ -67,6 +67,7 @@ import { registerRoutes } from './routes'; import { ServiceStatus, ServiceStatusLevels } from '../status'; import { calculateStatus$ } from './status'; import { createMigrationOpenSearchClient } from './migrations/core/'; +import { Config } from '../config'; /** * Saved Objects is OpenSearchDashboards's data persistence mechanism allowing plugins to * use OpenSearch for storing and querying state. The SavedObjectsServiceSetup API exposes methods @@ -315,6 +316,8 @@ export class SavedObjectsService summary: `waiting`, }); + private opensearchDashboardsRawConfig?: Config; + constructor(private readonly coreContext: CoreContext) { this.logger = coreContext.logger.get('savedobjects-service'); } @@ -332,6 +335,10 @@ export class SavedObjectsService .atPath('migrations') .pipe(first()) .toPromise(); + this.opensearchDashboardsRawConfig = await this.coreContext.configService + .getConfig$() + .pipe(first()) + .toPromise(); this.config = new SavedObjectConfig(savedObjectsConfig, savedObjectsMigrationConfig); registerRoutes({ @@ -559,6 +566,7 @@ export class SavedObjectsService this.logger, migrationsRetryDelay ), + opensearchDashboardsRawConfig: this.opensearchDashboardsRawConfig as Config, }); } } diff --git a/src/core/server/saved_objects/service/saved_objects_client.ts b/src/core/server/saved_objects/service/saved_objects_client.ts index a53684c833e2..c9990977bb48 100644 --- a/src/core/server/saved_objects/service/saved_objects_client.ts +++ b/src/core/server/saved_objects/service/saved_objects_client.ts @@ -71,6 +71,10 @@ export interface SavedObjectsCreateOptions extends SavedObjectsBaseOptions { initialNamespaces?: string[]; /** permission control describe by ACL object */ permissions?: Permissions; + /** + * workspaces the new created objects belong to + */ + workspaces?: string[]; } /** diff --git a/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts b/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts index 147f0ab159d3..062aee299758 100644 --- a/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts +++ b/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts @@ -38,6 +38,9 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', enabled: true, }, }, + migrations: { + skip: false, + }, }, }, }); @@ -104,6 +107,24 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test', }); }); + it('create-with-override with unexisting object id', async () => { + const createResult = await osdTestServer.request + .post(root, `/api/saved_objects/${dashboard.type}/foo?overwrite=true`) + .send({ + attributes: dashboard.attributes, + workspaces: [createdFooWorkspace.id], + }) + .expect(200); + + expect(createResult.body.id).toEqual('foo'); + expect(createResult.body.workspaces).toEqual([createdFooWorkspace.id]); + + await deleteItem({ + type: dashboard.type, + id: createResult.body.id, + }); + }); + it('create-with-override', async () => { const createResult = await osdTestServer.request .post(root, `/api/saved_objects/${dashboard.type}`) diff --git a/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.test.ts b/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.test.ts index cac06d789822..961accac262f 100644 --- a/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.test.ts +++ b/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.test.ts @@ -33,6 +33,9 @@ describe('WorkspaceConflictSavedObjectsClientWrapper', () => { }; wrapperInstance.setSerializer(savedObjectsSerializer); describe('createWithWorkspaceConflictCheck', () => { + beforeEach(() => { + mockedClient.create.mockClear(); + }); it(`Should reserve the workspace params when overwrite with empty workspaces`, async () => { mockedClient.get.mockResolvedValueOnce( getSavedObject({ @@ -84,6 +87,40 @@ describe('WorkspaceConflictSavedObjectsClientWrapper', () => { ) ).rejects.toThrowError('Saved object [dashboard/dashboard:foo] conflict'); }); + + it(`Should use options.workspaces when get throws error`, async () => { + mockedClient.get.mockRejectedValueOnce( + getSavedObject({ + id: 'dashboard:foo', + workspaces: ['foo'], + error: { + statusCode: 404, + error: 'Not found', + message: 'Not found', + }, + }) + ); + + await wrapperClient.create( + 'dashboard', + { + name: 'foo', + }, + { + id: 'dashboard:foo', + overwrite: true, + workspaces: ['bar'], + } + ); + + expect(mockedClient.create).toBeCalledWith( + expect.anything(), + expect.anything(), + expect.objectContaining({ + workspaces: ['bar'], + }) + ); + }); }); describe('bulkCreateWithWorkspaceConflictCheck', () => { @@ -136,6 +173,10 @@ describe('WorkspaceConflictSavedObjectsClientWrapper', () => { id: 'bar', workspaces: ['foo', 'bar'], }), + getSavedObject({ + id: 'qux', + workspaces: ['foo'], + }), ], }); mockedClient.bulkGet.mockResolvedValueOnce({ @@ -199,6 +240,7 @@ describe('WorkspaceConflictSavedObjectsClientWrapper', () => { id: 'qux', references: [], type: 'dashboard', + workspaces: ['foo'], }, ], { @@ -242,6 +284,15 @@ describe('WorkspaceConflictSavedObjectsClientWrapper', () => { "bar", ], }, + Object { + "attributes": Object {}, + "id": "qux", + "references": Array [], + "type": "dashboard", + "workspaces": Array [ + "foo", + ], + }, ], } `); diff --git a/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.ts b/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.ts index f2d0eb2c732c..a190fcc88613 100644 --- a/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.ts +++ b/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.ts @@ -12,7 +12,6 @@ import { SavedObjectsClientWrapperFactory, SavedObjectsCreateOptions, SavedObjectsErrorHelpers, - SavedObjectsUtils, SavedObjectsSerializer, SavedObjectsCheckConflictsObject, SavedObjectsCheckConflictsResponse, @@ -53,8 +52,14 @@ export class WorkspaceConflictSavedObjectsClientWrapper { let currentItem; try { currentItem = await wrapperOptions.client.get(type, id); - } catch (e) { - // If item can not be found, supress the error and create the object + } catch (e: unknown) { + const error = e as Boom.Boom; + if (error?.output?.statusCode === 404) { + // If item can not be found, supress the error and create the object + } else { + // Throw other error + throw e; + } } if (currentItem) { if ( @@ -105,6 +110,15 @@ export class WorkspaceConflictSavedObjectsClientWrapper { const bulkGetResult = await wrapperOptions.client.bulkGet(bulkGetDocs); bulkGetResult.saved_objects.forEach((object) => { + const { id, type } = object; + + /** + * If the object can not be found, create object by using options.workspaces + */ + if (object.error && object.error.statusCode === 404) { + objectsMapWorkspaces[this.getRawId({ namespace, type, id })] = options.workspaces; + } + /** * Skip the items with error, wrapperOptions.client will handle the error */ @@ -118,7 +132,6 @@ export class WorkspaceConflictSavedObjectsClientWrapper { options.workspaces, object.workspaces ); - const { id, type } = object; if (filteredWorkspaces.length) { /** * options.workspaces is not a subset of object.workspaces, From 07f362d346134e4165a09ea4a2bed2b50f0b1ed2 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 4 Mar 2024 11:51:32 +0800 Subject: [PATCH 2/6] fix: bootstrap error Signed-off-by: SuZhou-Joe --- src/core/server/saved_objects/import/create_saved_objects.ts | 1 - src/core/server/saved_objects/import/resolve_import_errors.ts | 1 - src/core/server/saved_objects/import/types.ts | 4 ---- src/core/server/saved_objects/routes/import.ts | 4 ---- src/core/server/saved_objects/routes/resolve_import_errors.ts | 3 --- 5 files changed, 13 deletions(-) diff --git a/src/core/server/saved_objects/import/create_saved_objects.ts b/src/core/server/saved_objects/import/create_saved_objects.ts index 3bc970dfa358..4bb07150aef8 100644 --- a/src/core/server/saved_objects/import/create_saved_objects.ts +++ b/src/core/server/saved_objects/import/create_saved_objects.ts @@ -39,7 +39,6 @@ interface CreateSavedObjectsParams { importIdMap: Map; namespace?: string; overwrite?: boolean; - workspaces?: string[]; dataSourceId?: string; dataSourceTitle?: string; workspaces?: string[]; diff --git a/src/core/server/saved_objects/import/resolve_import_errors.ts b/src/core/server/saved_objects/import/resolve_import_errors.ts index 0c07acd6c9f0..24bbc1934de3 100644 --- a/src/core/server/saved_objects/import/resolve_import_errors.ts +++ b/src/core/server/saved_objects/import/resolve_import_errors.ts @@ -163,7 +163,6 @@ export async function resolveSavedObjectsImportErrors({ importIdMap, namespace, overwrite, - workspaces, dataSourceId, dataSourceTitle, workspaces, diff --git a/src/core/server/saved_objects/import/types.ts b/src/core/server/saved_objects/import/types.ts index 08c0f23b4331..994b7e627189 100644 --- a/src/core/server/saved_objects/import/types.ts +++ b/src/core/server/saved_objects/import/types.ts @@ -187,8 +187,6 @@ export interface SavedObjectsImportOptions { namespace?: string; /** If true, will create new copies of import objects, each with a random `id` and undefined `originId`. */ createNewCopies: boolean; - /** if specified, will import in given workspaces */ - workspaces?: string[]; dataSourceId?: string; dataSourceTitle?: string; /** if specified, will import in given workspaces */ @@ -214,8 +212,6 @@ export interface SavedObjectsResolveImportErrorsOptions { namespace?: string; /** If true, will create new copies of import objects, each with a random `id` and undefined `originId`. */ createNewCopies: boolean; - /** if specified, will import in given workspaces */ - workspaces?: string[]; dataSourceId?: string; dataSourceTitle?: string; /** if specified, will import in given workspaces */ diff --git a/src/core/server/saved_objects/routes/import.ts b/src/core/server/saved_objects/routes/import.ts index c8878439b6c8..1fc739ea168c 100644 --- a/src/core/server/saved_objects/routes/import.ts +++ b/src/core/server/saved_objects/routes/import.ts @@ -60,9 +60,6 @@ export const registerImportRoute = (router: IRouter, config: SavedObjectConfig) { overwrite: schema.boolean({ defaultValue: false }), createNewCopies: schema.boolean({ defaultValue: false }), - workspaces: schema.maybe( - schema.oneOf([schema.string(), schema.arrayOf(schema.string())]) - ), dataSourceId: schema.maybe(schema.string({ defaultValue: '' })), workspaces: schema.maybe( schema.oneOf([schema.string(), schema.arrayOf(schema.string())]) @@ -126,7 +123,6 @@ export const registerImportRoute = (router: IRouter, config: SavedObjectConfig) objectLimit: maxImportExportSize, overwrite, createNewCopies, - workspaces, dataSourceId, dataSourceTitle, workspaces, diff --git a/src/core/server/saved_objects/routes/resolve_import_errors.ts b/src/core/server/saved_objects/routes/resolve_import_errors.ts index 0ea91e43a777..6bc667eba0df 100644 --- a/src/core/server/saved_objects/routes/resolve_import_errors.ts +++ b/src/core/server/saved_objects/routes/resolve_import_errors.ts @@ -58,9 +58,6 @@ export const registerResolveImportErrorsRoute = (router: IRouter, config: SavedO validate: { query: schema.object({ createNewCopies: schema.boolean({ defaultValue: false }), - workspaces: schema.maybe( - schema.oneOf([schema.string(), schema.arrayOf(schema.string())]) - ), dataSourceId: schema.maybe(schema.string({ defaultValue: '' })), workspaces: schema.maybe( schema.oneOf([schema.string(), schema.arrayOf(schema.string())]) From bed5936029fbbf6db29b8befa9bbf3f220e81db6 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 4 Mar 2024 12:27:48 +0800 Subject: [PATCH 3/6] fix: unit test error Signed-off-by: SuZhou-Joe --- .../migrations/core/index_migrator.test.ts | 67 +++++++++++++++---- 1 file changed, 55 insertions(+), 12 deletions(-) diff --git a/src/core/server/saved_objects/migrations/core/index_migrator.test.ts b/src/core/server/saved_objects/migrations/core/index_migrator.test.ts index 3188ff87afe8..728e508e3c23 100644 --- a/src/core/server/saved_objects/migrations/core/index_migrator.test.ts +++ b/src/core/server/saved_objects/migrations/core/index_migrator.test.ts @@ -87,6 +87,7 @@ describe('IndexMigrator', () => { type: '2f4316de49999235636386fe51dc06c1', updated_at: '00da57df13e94e9d98437d13ace4bfe0', workspaces: '2f4316de49999235636386fe51dc06c1', + permissions: '07c04cdd060494956fdddaa7ef86e8ac', }, }, properties: { @@ -106,6 +107,60 @@ describe('IndexMigrator', () => { }, }, workspaces: { type: 'keyword' }, + permissions: { + properties: { + library_read: { + properties: { + groups: { + type: 'keyword', + }, + users: { + type: 'keyword', + }, + }, + }, + library_write: { + properties: { + groups: { + type: 'keyword', + }, + users: { + type: 'keyword', + }, + }, + }, + management: { + properties: { + groups: { + type: 'keyword', + }, + users: { + type: 'keyword', + }, + }, + }, + read: { + properties: { + groups: { + type: 'keyword', + }, + users: { + type: 'keyword', + }, + }, + }, + write: { + properties: { + groups: { + type: 'keyword', + }, + users: { + type: 'keyword', + }, + }, + }, + }, + }, }, }, settings: { number_of_shards: 1, auto_expand_replicas: '0-1' }, @@ -137,7 +192,6 @@ describe('IndexMigrator', () => { references: '7997cf5a56cc02bdc9c93361bde732b0', type: '2f4316de49999235636386fe51dc06c1', updated_at: '00da57df13e94e9d98437d13ace4bfe0', - workspaces: '2f4316de49999235636386fe51dc06c1', permissions: '07c04cdd060494956fdddaa7ef86e8ac', }, }, @@ -149,9 +203,6 @@ describe('IndexMigrator', () => { originId: { type: 'keyword' }, type: { type: 'keyword' }, updated_at: { type: 'date' }, - workspaces: { - type: 'keyword', - }, permissions: { properties: { library_read: { @@ -313,7 +364,6 @@ describe('IndexMigrator', () => { references: '7997cf5a56cc02bdc9c93361bde732b0', type: '2f4316de49999235636386fe51dc06c1', updated_at: '00da57df13e94e9d98437d13ace4bfe0', - workspaces: '2f4316de49999235636386fe51dc06c1', permissions: '07c04cdd060494956fdddaa7ef86e8ac', }, }, @@ -326,9 +376,6 @@ describe('IndexMigrator', () => { originId: { type: 'keyword' }, type: { type: 'keyword' }, updated_at: { type: 'date' }, - workspaces: { - type: 'keyword', - }, permissions: { properties: { library_read: { @@ -433,7 +480,6 @@ describe('IndexMigrator', () => { references: '7997cf5a56cc02bdc9c93361bde732b0', type: '2f4316de49999235636386fe51dc06c1', updated_at: '00da57df13e94e9d98437d13ace4bfe0', - workspaces: '2f4316de49999235636386fe51dc06c1', permissions: '07c04cdd060494956fdddaa7ef86e8ac', }, }, @@ -446,9 +492,6 @@ describe('IndexMigrator', () => { originId: { type: 'keyword' }, type: { type: 'keyword' }, updated_at: { type: 'date' }, - workspaces: { - type: 'keyword', - }, permissions: { properties: { library_read: { From b38f8a0d4108b4901f7ffed048cb5b335327493a Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 4 Mar 2024 12:31:50 +0800 Subject: [PATCH 4/6] fix: unit test Signed-off-by: SuZhou-Joe --- ..._wrapper_for_check_workspace_conflict.test.ts | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.test.ts b/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.test.ts index 961accac262f..9c29684e58e4 100644 --- a/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.test.ts +++ b/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.test.ts @@ -89,17 +89,11 @@ describe('WorkspaceConflictSavedObjectsClientWrapper', () => { }); it(`Should use options.workspaces when get throws error`, async () => { - mockedClient.get.mockRejectedValueOnce( - getSavedObject({ - id: 'dashboard:foo', - workspaces: ['foo'], - error: { - statusCode: 404, - error: 'Not found', - message: 'Not found', - }, - }) - ); + mockedClient.get.mockRejectedValueOnce({ + output: { + statusCode: 404, + }, + }); await wrapperClient.create( 'dashboard', From 5f7e962111ab158b0cf1680e071ccd17f4f48b2a Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 4 Mar 2024 13:20:22 +0800 Subject: [PATCH 5/6] feat: remove useless CHANGELOG Signed-off-by: SuZhou-Joe --- CHANGELOG.md | 5 ----- 1 file changed, 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b16ab9a73eac..d48fef6e3efa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,11 +41,6 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Multiple Datasource] Add interfaces to register add-on authentication method from plug-in module ([#5851](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5851)) - [Multiple Datasource] Able to Hide "Local Cluster" option from datasource DropDown ([#5827](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5827)) - [Multiple Datasource] Add api registry and allow it to be added into client config in data source plugin ([#5895](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5895)) -- [Multiple Datasource] Concatenate data source name with index pattern name and change delimiter to double colon ([#5907](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5907)) -- [Multiple Datasource] Refactor client and legacy client to use authentication registry ([#5881](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5881)) -- [Multiple Datasource] Improved error handling for the search API when a null value is passed for the dataSourceId ([#5882](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5882)) -- [Multiple Datasource] Hide/Show authentication method in multi data source plugin based on configuration ([#5916](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5916)) -- [[Dynamic Configurations] Add support for dynamic application configurations ([#5855](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5855)) - [Workspace] Optional workspaces params in repository ([#5949](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5949)) ### 🐛 Bug Fixes From 7aa6580077269d820ca27d1f5935662d17a2fd4f Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 4 Mar 2024 13:23:22 +0800 Subject: [PATCH 6/6] feat: update snapshot Signed-off-by: SuZhou-Joe --- .../core/__snapshots__/build_active_mappings.test.ts.snap | 8 -------- .../opensearch_dashboards_migrator.test.ts.snap | 4 ---- 2 files changed, 12 deletions(-) diff --git a/src/core/server/saved_objects/migrations/core/__snapshots__/build_active_mappings.test.ts.snap b/src/core/server/saved_objects/migrations/core/__snapshots__/build_active_mappings.test.ts.snap index 09e8ad8b5407..6f67893104e7 100644 --- a/src/core/server/saved_objects/migrations/core/__snapshots__/build_active_mappings.test.ts.snap +++ b/src/core/server/saved_objects/migrations/core/__snapshots__/build_active_mappings.test.ts.snap @@ -14,7 +14,6 @@ Object { "references": "7997cf5a56cc02bdc9c93361bde732b0", "type": "2f4316de49999235636386fe51dc06c1", "updated_at": "00da57df13e94e9d98437d13ace4bfe0", - "workspaces": "2f4316de49999235636386fe51dc06c1", }, }, "dynamic": "strict", @@ -112,9 +111,6 @@ Object { "updated_at": Object { "type": "date", }, - "workspaces": Object { - "type": "keyword", - }, }, } `; @@ -134,7 +130,6 @@ Object { "thirdType": "510f1f0adb69830cf8a1c5ce2923ed82", "type": "2f4316de49999235636386fe51dc06c1", "updated_at": "00da57df13e94e9d98437d13ace4bfe0", - "workspaces": "2f4316de49999235636386fe51dc06c1", }, }, "dynamic": "strict", @@ -249,9 +244,6 @@ Object { "updated_at": Object { "type": "date", }, - "workspaces": Object { - "type": "keyword", - }, }, } `; diff --git a/src/core/server/saved_objects/migrations/opensearch_dashboards/__snapshots__/opensearch_dashboards_migrator.test.ts.snap b/src/core/server/saved_objects/migrations/opensearch_dashboards/__snapshots__/opensearch_dashboards_migrator.test.ts.snap index 2748ad2eaf6a..5e39af788d79 100644 --- a/src/core/server/saved_objects/migrations/opensearch_dashboards/__snapshots__/opensearch_dashboards_migrator.test.ts.snap +++ b/src/core/server/saved_objects/migrations/opensearch_dashboards/__snapshots__/opensearch_dashboards_migrator.test.ts.snap @@ -14,7 +14,6 @@ Object { "references": "7997cf5a56cc02bdc9c93361bde732b0", "type": "2f4316de49999235636386fe51dc06c1", "updated_at": "00da57df13e94e9d98437d13ace4bfe0", - "workspaces": "2f4316de49999235636386fe51dc06c1", }, }, "dynamic": "strict", @@ -120,9 +119,6 @@ Object { "updated_at": Object { "type": "date", }, - "workspaces": Object { - "type": "keyword", - }, }, } `;