Skip to content

Commit

Permalink
Support legacy client for data source (opensearch-project#2204)
Browse files Browse the repository at this point in the history
* support legacy client for data source
* not wrap 401 error for data source client

Signed-off-by: Su <szhongna@amazon.com>
Signed-off-by: Sergey V. Osipov <sipopo@yandex.ru>
  • Loading branch information
zhongnansu authored and sipopo committed Dec 16, 2022
1 parent fc62286 commit 6e10927
Show file tree
Hide file tree
Showing 23 changed files with 577 additions and 134 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)

### 📈 Features/Enhancements

* [MD] Support legacy client for data source ([#2204](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2204))

### 🐛 Bug Fixes
* [Vis Builder] Fixes auto bounds for timeseries bar chart visualization ([2401](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2401))
* [Vis Builder] Fixes visualization shift when editing agg ([2401](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2401))
Expand Down
1 change: 0 additions & 1 deletion src/core/server/http/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,6 @@ export class Router implements IRouter {
opensearchDashboardsResponseFactory.badRequest({ body: e.message })
);
}
// TODO: add legacy data source client config error handling

return hapiResponseAdapter.toInternalError();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
* under the License.
*/

import { LegacyAPICaller, OpenSearchClient } from 'opensearch-dashboards/server';
import { LegacyAPICaller } from 'opensearch-dashboards/server';

import { getFieldCapabilities, resolveTimePattern, createNoMatchingIndicesError } from './lib';

Expand All @@ -48,9 +48,9 @@ interface FieldSubType {
}

export class IndexPatternsFetcher {
private _callDataCluster: LegacyAPICaller | OpenSearchClient;
private _callDataCluster: LegacyAPICaller;

constructor(callDataCluster: LegacyAPICaller | OpenSearchClient) {
constructor(callDataCluster: LegacyAPICaller) {
this._callDataCluster = callDataCluster;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@

import { defaults, keyBy, sortBy } from 'lodash';

import { LegacyAPICaller, OpenSearchClient } from 'opensearch-dashboards/server';
import { LegacyAPICaller } from 'opensearch-dashboards/server';
import { callFieldCapsApi } from '../opensearch_api';
import { FieldCapsResponse, readFieldCapsResponse } from './field_caps_response';
import { mergeOverrides } from './overrides';
Expand All @@ -47,7 +47,7 @@ import { FieldDescriptor } from '../../index_patterns_fetcher';
* @return {Promise<Array<FieldDescriptor>>}
*/
export async function getFieldCapabilities(
callCluster: LegacyAPICaller | OpenSearchClient,
callCluster: LegacyAPICaller,
indices: string | string[] = [],
metaFields: string[] = [],
fieldCapsOptions?: { allowNoIndices: boolean }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,23 +57,10 @@ export interface IndexAliasResponse {
* @return {Promise<IndexAliasResponse>}
*/
export async function callIndexAliasApi(
callCluster: LegacyAPICaller | OpenSearchClient,
callCluster: LegacyAPICaller,
indices: string[] | string
): Promise<IndicesAliasResponse> {
try {
// This approach of identify between OpenSearchClient vs LegacyAPICaller
// will be deprecated after support data client with legacy client
// https://github.com/opensearch-project/OpenSearch-Dashboards/issues/2133
if ('transport' in callCluster) {
return (
await callCluster.indices.getAlias({
index: indices,
ignore_unavailable: true,
allow_no_indices: true,
})
).body as IndicesAliasResponse;
}

return (await callCluster('indices.getAlias', {
index: indices,
ignoreUnavailable: true,
Expand All @@ -97,25 +84,11 @@ export async function callIndexAliasApi(
* @return {Promise<FieldCapsResponse>}
*/
export async function callFieldCapsApi(
callCluster: LegacyAPICaller | OpenSearchClient,
callCluster: LegacyAPICaller,
indices: string[] | string,
fieldCapsOptions: { allowNoIndices: boolean } = { allowNoIndices: false }
) {
try {
// This approach of identify between OpenSearchClient vs LegacyAPICaller
// will be deprecated after support data client with legacy client
// https://github.com/opensearch-project/OpenSearch-Dashboards/issues/2133
if ('transport' in callCluster) {
return (
await callCluster.fieldCaps({
index: indices,
fields: '*',
ignore_unavailable: true,
allow_no_indices: fieldCapsOptions.allowNoIndices,
})
).body as FieldCapsResponse;
}

return (await callCluster('fieldCaps', {
index: indices,
fields: '*',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,7 @@ import { callIndexAliasApi, IndicesAliasResponse } from './opensearch_api';
* and the indices that actually match the time
* pattern (matches);
*/
export async function resolveTimePattern(
callCluster: LegacyAPICaller | OpenSearchClient,
timePattern: string
) {
export async function resolveTimePattern(callCluster: LegacyAPICaller, timePattern: string) {
const aliases = await callIndexAliasApi(callCluster, timePatternToWildcard(timePattern));

const allIndexDetails = chain<IndicesAliasResponse>(aliases)
Expand Down
19 changes: 12 additions & 7 deletions src/plugins/data/server/index_patterns/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,11 @@
*/

import { schema } from '@osd/config-schema';
import { HttpServiceSetup, RequestHandlerContext } from 'opensearch-dashboards/server';
import {
HttpServiceSetup,
LegacyAPICaller,
RequestHandlerContext,
} from 'opensearch-dashboards/server';
import { IndexPatternsFetcher } from './fetcher';

export function registerRoutes(http: HttpServiceSetup) {
Expand Down Expand Up @@ -151,11 +155,12 @@ export function registerRoutes(http: HttpServiceSetup) {
);
}

const decideClient = async (context: RequestHandlerContext, request: any) => {
const decideClient = async (
context: RequestHandlerContext,
request: any
): Promise<LegacyAPICaller> => {
const dataSourceId = request.query.data_source;
if (dataSourceId) {
return await context.dataSource.opensearch.getClient(dataSourceId);
}

return context.core.opensearch.legacy.client.callAsCurrentUser;
return dataSourceId
? (context.dataSource.opensearch.legacy.getClient(dataSourceId).callAPI as LegacyAPICaller)
: context.core.opensearch.legacy.client.callAsCurrentUser;
};
17 changes: 9 additions & 8 deletions src/plugins/data_source/server/client/client_pool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,31 +4,32 @@
*/

import { Client } from '@opensearch-project/opensearch';
import { Client as LegacyClient } from 'elasticsearch';
import LRUCache from 'lru-cache';
import { Logger } from 'src/core/server';
import { DataSourcePluginConfigType } from '../../config';

export interface OpenSearchClientPoolSetup {
getClientFromPool: (id: string) => Client | undefined;
addClientToPool: (endpoint: string, client: Client) => void;
getClientFromPool: (id: string) => Client | LegacyClient | undefined;
addClientToPool: (endpoint: string, client: Client | LegacyClient) => void;
}

/**
* OpenSearch client pool.
* OpenSearch client pool for data source.
*
* This client pool uses an LRU cache to manage OpenSearch Js client objects.
* It reuse TPC connections for each OpenSearch endpoint.
*/
export class OpenSearchClientPool {
// LRU cache
// key: data source endpoint url
// value: OpenSearch client object
private cache?: LRUCache<string, Client>;
// key: data source endpoint
// value: OpenSearch client object | Legacy client object
private cache?: LRUCache<string, Client | LegacyClient>;
private isClosed = false;

constructor(private logger: Logger) {}

public async setup(config: DataSourcePluginConfigType) {
public async setup(config: DataSourcePluginConfigType): Promise<OpenSearchClientPoolSetup> {
const logger = this.logger;
const { size } = config.clientPool;

Expand All @@ -53,7 +54,7 @@ export class OpenSearchClientPool {
return this.cache!.get(endpoint);
};

const addClientToPool = (endpoint: string, client: Client) => {
const addClientToPool = (endpoint: string, client: Client | LegacyClient) => {
this.cache!.set(endpoint, client);
};

Expand Down
30 changes: 11 additions & 19 deletions src/plugins/data_source/server/client/configure_client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { ClientOptions } from '@opensearch-project/opensearch';
// eslint-disable-next-line @osd/eslint/no-restricted-paths
import { opensearchClientMock } from '../../../../core/server/opensearch/client/mocks';
import { CryptographyClient } from '../cryptography';
import { DataSourceClientParams } from '../types';

const DATA_SOURCE_ID = 'a54b76ec86771ee865a0f74a305dfff8';
const cryptoClient = new CryptographyClient('test', 'test', new Array(32).fill(0));
Expand All @@ -28,6 +29,7 @@ describe('configureClient', () => {
let clientOptions: ClientOptions;
let dataSourceAttr: DataSourceAttributes;
let dsClient: ReturnType<typeof opensearchClientMock.createInternalClient>;
let dataSourceClientParams: DataSourceClientParams;

beforeEach(() => {
dsClient = opensearchClientMock.createInternalClient();
Expand Down Expand Up @@ -70,9 +72,13 @@ describe('configureClient', () => {
references: [],
});

ClientMock.mockImplementation(() => {
return dsClient;
});
dataSourceClientParams = {
dataSourceId: DATA_SOURCE_ID,
savedObjects: savedObjectsMock,
cryptographyClient: cryptoClient,
};

ClientMock.mockImplementation(() => dsClient);
});

afterEach(() => {
Expand All @@ -94,14 +100,7 @@ describe('configureClient', () => {

parseClientOptionsMock.mockReturnValue(clientOptions);

const client = await configureClient(
DATA_SOURCE_ID,
savedObjectsMock,
cryptoClient,
clientPoolSetup,
config,
logger
);
const client = await configureClient(dataSourceClientParams, clientPoolSetup, config, logger);

expect(parseClientOptionsMock).toHaveBeenCalled();
expect(ClientMock).toHaveBeenCalledTimes(1);
Expand All @@ -113,14 +112,7 @@ describe('configureClient', () => {
test('configure client with auth.type == username_password, will first call decrypt()', async () => {
const spy = jest.spyOn(cryptoClient, 'decodeAndDecrypt').mockResolvedValue('password');

const client = await configureClient(
DATA_SOURCE_ID,
savedObjectsMock,
cryptoClient,
clientPoolSetup,
config,
logger
);
const client = await configureClient(dataSourceClientParams, clientPoolSetup, config, logger);

expect(ClientMock).toHaveBeenCalledTimes(1);
expect(savedObjectsMock.get).toHaveBeenCalledTimes(1);
Expand Down
25 changes: 15 additions & 10 deletions src/plugins/data_source/server/client/configure_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,12 @@ import {
import { DataSourcePluginConfigType } from '../../config';
import { CryptographyClient } from '../cryptography';
import { DataSourceConfigError } from '../lib/error';
import { DataSourceClientParams } from '../types';
import { parseClientOptions } from './client_config';
import { OpenSearchClientPoolSetup } from './client_pool';

export const configureClient = async (
dataSourceId: string,
savedObjects: SavedObjectsClientContract,
cryptographyClient: CryptographyClient,
{ dataSourceId, savedObjects, cryptographyClient }: DataSourceClientParams,
openSearchClientPoolSetup: OpenSearchClientPoolSetup,
config: DataSourcePluginConfigType,
logger: Logger
Expand Down Expand Up @@ -68,20 +67,26 @@ export const getCredential = async (
*
* @param rootClient root client for the connection with given data source endpoint.
* @param dataSource data source saved object
* @param cryptographyClient cryptography client for password encryption / decrpytion
* @param cryptographyClient cryptography client for password encryption / decryption
* @returns child client.
*/
const getQueryClient = async (
rootClient: Client,
dataSource: SavedObject<DataSourceAttributes>,
cryptographyClient: CryptographyClient
): Promise<Client> => {
if (AuthType.NoAuth === dataSource.attributes.auth.type) {
return rootClient.child();
} else {
const credential = await getCredential(dataSource, cryptographyClient);
const authType = dataSource.attributes.auth.type;

switch (authType) {
case AuthType.NoAuth:
return rootClient.child();

case AuthType.UsernamePasswordType:
const credential = await getCredential(dataSource, cryptographyClient);
return getBasicAuthClient(rootClient, credential);

return getBasicAuthClient(rootClient, credential);
default:
throw Error(`${authType} is not a supported auth type for data source`);
}
};

Expand All @@ -101,7 +106,7 @@ const getRootClient = (
const endpoint = dataSourceAttr.endpoint;
const cachedClient = getClientFromPool(endpoint);
if (cachedClient) {
return cachedClient;
return cachedClient as Client;
} else {
const clientOptions = parseClientOptions(config, endpoint);

Expand Down
4 changes: 2 additions & 2 deletions src/plugins/data_source/server/client/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@
* SPDX-License-Identifier: Apache-2.0
*/

export { OpenSearchClientPool } from './client_pool';
export { configureClient } from './configure_client';
export { OpenSearchClientPool, OpenSearchClientPoolSetup } from './client_pool';
export { configureClient, getDataSource, getCredential } from './configure_client';
1 change: 1 addition & 0 deletions src/plugins/data_source/server/data_source_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ describe('Data Source Service', () => {
test('exposes proper contract', async () => {
const setup = await service.setup(config);
expect(setup).toHaveProperty('getDataSourceClient');
expect(setup).toHaveProperty('getDataSourceLegacyClient');
});
});
});
Loading

0 comments on commit 6e10927

Please sign in to comment.