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

[Backport 2.x] [MD] Support legacy client for data source (#2204) #2484

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
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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is changing the valid call signature of this class backwards compatible? Is it possible it would break any plugins attempting to call the IndexPatternsFetcher with anOpenSearchClient? Or was this change only introduced as part of the initial MD PR?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah it's mostly a follow-up to the initial MD PR. At that time, since legacy client is not supported by MD, but index_pattern_fetcher is still using legacy client. We had a workaround to enforce index_pattern_fetcher to use new Client when talking to data sources, use legacy client when talk to default cluster. Now with the support of legacy client, we can get rid of the workaround, and keep the type declaration clean, to only use legacy client for any scenario

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';
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