Skip to content

Commit

Permalink
Datasource id is required if multiple datasource is enabled and no de…
Browse files Browse the repository at this point in the history
…fault cluster supported (#5751)

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

Signed-off-by: Xinrui Bai <xinruiba@amazon.com>

---------

Signed-off-by: Xinrui Bai <xinruiba@amazon.com>
(cherry picked from commit bbd40e1)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
github-actions[bot] committed Feb 7, 2024
1 parent 040a7f0 commit ee6aa79
Show file tree
Hide file tree
Showing 7 changed files with 126 additions and 14 deletions.
2 changes: 2 additions & 0 deletions config/opensearch_dashboards.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down Expand Up @@ -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,
Expand All @@ -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);

Expand All @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,20 @@ export const opensearchSearchStrategyProvider = (
});

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`);

Check warning on line 81 in src/plugins/data/server/search/opensearch_search/opensearch_search_strategy.ts

View check run for this annotation

Codecov / codecov/patch

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

Added line #L81 was not covered by tests
}

const client = await decideClient(

Check warning on line 84 in src/plugins/data/server/search/opensearch_search/opensearch_search_strategy.ts

View check run for this annotation

Codecov / codecov/patch

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

Added line #L84 was not covered by tests
context,
request,
dataSource?.dataSourceEnabled(),
withLongNumeralsSupport
);
const promise = shimAbortSignal(client.search(params), options?.abortSignal);

const { body: rawResponse } = (await promise) as ApiResponse<SearchResponse<any>>;
Expand All @@ -92,7 +105,7 @@ export const opensearchSearchStrategyProvider = (
} catch (e) {
if (usage) usage.trackError();

if (dataSource && request.dataSourceId) {
if (dataSource?.dataSourceEnabled()) {
throw dataSource.createDataSourceError(e);
}
throw e;
Expand Down
1 change: 1 addition & 0 deletions src/plugins/data_source/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions src/plugins/data_source/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ export class DataSourcePlugin implements Plugin<DataSourcePluginSetup, DataSourc

return {
createDataSourceError: (e: any) => createDataSourceError(e),
dataSourceEnabled: () => config.enabled,
defaultClusterEnabled: () => config.defaultCluster,
};
}

Expand Down
2 changes: 2 additions & 0 deletions src/plugins/data_source/server/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}

0 comments on commit ee6aa79

Please sign in to comment.