Datasource id is required if multiple datasource is enabled and no default cluster supported

CHANGELOG.md

@@ -55,6 +55,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - [Discover] Fix missing index pattern field from breaking Discover [#5626](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5626)
 - [BUG] Remove duplicate sample data as id 90943e30-9a47-11e8-b64d-95841ca0b247 ([5668](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5668))
 - [BUG][Multiple Datasource] Fix datasource testing connection unexpectedly passed with wrong endpoint [#5663](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5663)
+- [BUG][Multiple Datasource] Datasource id is required if multiple datasource is enabled and no default cluster supported [#5751](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5751)
 
 ### 🚞 Infrastructure
 

config/opensearch_dashboards.yml

@@ -237,6 +237,8 @@
 
 # Set the value of this setting to true to enable multiple data source feature.
 #data_source.enabled: false
+# Set the value of this setting to false to disable default cluster in data source feature.
+#data_source.defaultCluster: true
 # Set the value of these settings to customize crypto materials to encryption saved credentials
 # in data sources.
 #data_source.encryption.wrappingKeyName: 'changeme'

src/plugins/data/server/search/opensearch_search/decide_client.ts

@@ -9,14 +9,14 @@ import { IOpenSearchSearchRequest } from '..';
 export const decideClient = async (
  context: RequestHandlerContext,
  request: IOpenSearchSearchRequest,
+ withDataSourceEnabled: boolean = false,
  withLongNumeralsSupport: boolean = false
 ): Promise<OpenSearchClient> => {
- // if data source feature is disabled, return default opensearch client of current user
- const client =
- request.dataSourceId && context.dataSource
- ? await context.dataSource.opensearch.getClient(request.dataSourceId)
- : withLongNumeralsSupport
- ? context.core.opensearch.client.asCurrentUserWithLongNumeralsSupport
- : context.core.opensearch.client.asCurrentUser;
- return client;
+ const defaultOpenSearchClient = withLongNumeralsSupport
+ ? context.core.opensearch.client.asCurrentUserWithLongNumeralsSupport
+ : context.core.opensearch.client.asCurrentUser;
+
+ return withDataSourceEnabled && request.dataSourceId && context.dataSource
+ ? await context.dataSource.opensearch.getClient(request.dataSourceId)
+ : defaultOpenSearchClient;
 };

src/plugins/data/server/search/opensearch_search/opensearch_search_strategy.test.ts

@@ -31,11 +31,22 @@
 import { RequestHandlerContext } from '../../../../../core/server';
 import { pluginInitializerContextConfigMock } from '../../../../../core/server/mocks';
 import { opensearchSearchStrategyProvider } from './opensearch_search_strategy';
+import { DataSourceError } from '../../../../data_source/server/lib/error';
+import { DataSourcePluginSetup } from '../../../../data_source/server';
+import { SearchUsage } from '../collectors';
 
 describe('OpenSearch search strategy', () => {
  const mockLogger: any = {
  debug: () => {},
  };
+ const mockSearchUsage: SearchUsage = {
+ trackError(): Promise<void> {
+ return Promise.resolve(undefined);
+ },
+ trackSuccess(duration: number): Promise<void> {
+ return Promise.resolve(undefined);
+ },
+ };
  const body = {
  body: {
  _shards: {
@@ -129,8 +140,21 @@ describe('OpenSearch search strategy', () => {
  expect(response).toHaveProperty('rawResponse');
  });
 
- it('dataSource enabled, send request with dataSourceId get data source client', async () => {
- const opensearchSearch = await opensearchSearchStrategyProvider(mockConfig$, mockLogger);
+ it('dataSource enabled and default cluster disabled, send request with dataSourceId get data source client', async () => {
+ const mockDataSourcePluginSetupWithDataSourceEnabled: DataSourcePluginSetup = {
+ createDataSourceError(err: any): DataSourceError {
+ return new DataSourceError({});
+ },
+ dataSourceEnabled: jest.fn(() => true),
+ defaultClusterEnabled: jest.fn(() => false),
+ };
+
+ const opensearchSearch = await opensearchSearchStrategyProvider(
+ mockConfig$,
+ mockLogger,
+ mockSearchUsage,
+ mockDataSourcePluginSetupWithDataSourceEnabled
+ );
 
  await opensearchSearch.search(
  (mockDataSourceEnabledContext as unknown) as RequestHandlerContext,
@@ -142,6 +166,35 @@ describe('OpenSearch search strategy', () => {
  expect(mockOpenSearchApiCaller).not.toBeCalled();
  });
 
+ it('dataSource enabled and default cluster disabled, send request with empty dataSourceId should throw exception', async () => {
+ const mockDataSourcePluginSetupWithDataSourceEnabled: DataSourcePluginSetup = {
+ createDataSourceError(err: any): DataSourceError {
+ return new DataSourceError({});
+ },
+ dataSourceEnabled: jest.fn(() => true),
+ defaultClusterEnabled: jest.fn(() => false),
+ };
+
+ try {
+ const opensearchSearch = opensearchSearchStrategyProvider(
+ mockConfig$,
+ mockLogger,
+ mockSearchUsage,
+ mockDataSourcePluginSetupWithDataSourceEnabled
+ );
+
+ await opensearchSearch.search(
+ (mockDataSourceEnabledContext as unknown) as RequestHandlerContext,
+ {
+ dataSourceId: '',
+ }
+ );
+ } catch (e) {
+ expect(e).toBeTruthy();
+ expect(e).toBeInstanceOf(DataSourceError);
+ }
+ });
+
  it('dataSource disabled, send request with dataSourceId get default client', async () => {
  const opensearchSearch = await opensearchSearchStrategyProvider(mockConfig$, mockLogger);
 
@@ -152,8 +205,47 @@ describe('OpenSearch search strategy', () => {
  expect(mockDataSourceApiCaller).not.toBeCalled();
  });
 
- it('dataSource enabled, send request without dataSourceId get default client', async () => {
- const opensearchSearch = await opensearchSearchStrategyProvider(mockConfig$, mockLogger);
+ it('dataSource enabled and default cluster enabled, send request with dataSourceId get datasource client', async () => {
+ const mockDataSourcePluginSetupWithDataSourceEnabled: DataSourcePluginSetup = {
+ createDataSourceError(err: any): DataSourceError {
+ return new DataSourceError({});
+ },
+ dataSourceEnabled: jest.fn(() => true),
+ defaultClusterEnabled: jest.fn(() => true),
+ };
+
+ const opensearchSearch = await opensearchSearchStrategyProvider(
+ mockConfig$,
+ mockLogger,
+ mockSearchUsage,
+ mockDataSourcePluginSetupWithDataSourceEnabled
+ );
+
+ await opensearchSearch.search(
+ (mockDataSourceEnabledContext as unknown) as RequestHandlerContext,
+ {
+ dataSourceId,
+ }
+ );
+ expect(mockDataSourceApiCaller).toBeCalled();
+ expect(mockOpenSearchApiCaller).not.toBeCalled();
+ });
+
+ it('dataSource enabled and default cluster enabled, send request without dataSourceId get default client', async () => {
+ const mockDataSourcePluginSetupWithDataSourceEnabled: DataSourcePluginSetup = {
+ createDataSourceError(err: any): DataSourceError {
+ return new DataSourceError({});
+ },
+ dataSourceEnabled: jest.fn(() => true),
+ defaultClusterEnabled: jest.fn(() => true),
+ };
+
+ const opensearchSearch = await opensearchSearchStrategyProvider(
+ mockConfig$,
+ mockLogger,
+ mockSearchUsage,
+ mockDataSourcePluginSetupWithDataSourceEnabled
+ );
 
  await opensearchSearch.search((mockContext as unknown) as RequestHandlerContext, {});
  expect(mockOpenSearchApiCaller).toBeCalled();

src/plugins/data/server/search/opensearch_search/opensearch_search_strategy.ts

@@ -73,7 +73,20 @@
  });
 
  try {
- const client = await decideClient(context, request, withLongNumeralsSupport);
+ if (
+ dataSource?.dataSourceEnabled() &&
+ !dataSource?.defaultClusterEnabled() &&
+ !request.dataSourceId
+ ) {
+ throw new Error(`Data source id is required when no openseach hosts config provided`);
+ }
+
+ const client = await decideClient(
+ context,
+ request,
+ dataSource?.dataSourceEnabled(),
+ withLongNumeralsSupport
+ );
  const promise = shimAbortSignal(client.search(params), options?.abortSignal);
 
  const { body: rawResponse } = (await promise) as ApiResponse<SearchResponse<any>>;
@@ -92,7 +105,7 @@
  } catch (e) {
  if (usage) usage.trackError();
 
- if (dataSource && request.dataSourceId) {
+ if (dataSource?.dataSourceEnabled()) {
  throw dataSource.createDataSourceError(e);
  }
  throw e;

src/plugins/data_source/config.ts

@@ -13,6 +13,7 @@ const WRAPPING_KEY_SIZE: number = 32;
 
 export const configSchema = schema.object({
  enabled: schema.boolean({ defaultValue: false }),
+ defaultCluster: schema.boolean({ defaultValue: false }),
  encryption: schema.object({
  wrappingKeyName: schema.string({
  minLength: KEY_NAME_MIN_LENGTH,

src/plugins/data_source/server/plugin.ts

@@ -111,6 +111,8 @@ export class DataSourcePlugin implements Plugin<DataSourcePluginSetup, DataSourc
 
  return {
  createDataSourceError: (e: any) => createDataSourceError(e),
+ dataSourceEnabled: () => config.enabled,
+ defaultClusterEnabled: () => config.defaultCluster,
  };
  }
 

src/plugins/data_source/server/types.ts

@@ -53,6 +53,8 @@ declare module 'src/core/server' {
 
 export interface DataSourcePluginSetup {
  createDataSourceError: (err: any) => DataSourceError;
+ dataSourceEnabled: () => boolean;
+ defaultClusterEnabled: () => boolean;
 }
 // eslint-disable-next-line @typescript-eslint/no-empty-interface
 export interface DataSourcePluginStart {}