From 2c3edfa6b33fd8609d2b581df50f7c2c4785fb17 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Sat, 7 Jan 2023 00:40:47 +0000 Subject: [PATCH] [MD] Improve test connection (#3110) Signed-off-by: Su Signed-off-by: Su (cherry picked from commit 5719fb9e527db9f79a84cfe2d67ca835fff4e2d4) Signed-off-by: github-actions[bot] # Conflicts: # CHANGELOG.md --- .../data_source/common/data_sources/types.ts | 1 - .../server/client/configure_client.ts | 19 +++---- .../data_source/server/data_source_service.ts | 4 +- .../legacy/configure_legacy_client.test.ts | 2 +- .../server/legacy/configure_legacy_client.ts | 8 +-- src/plugins/data_source/server/lib/error.ts | 13 ++++- .../data_source_connection_validator.ts | 6 +-- .../server/routes/test_connection.ts | 52 ++++++++++--------- src/plugins/data_source/server/types.ts | 3 +- .../public/components/utils.test.ts | 4 +- .../public/components/utils.ts | 12 +++-- 11 files changed, 64 insertions(+), 60 deletions(-) diff --git a/src/plugins/data_source/common/data_sources/types.ts b/src/plugins/data_source/common/data_sources/types.ts index 29b4e3c3128f..afcf3d662fed 100644 --- a/src/plugins/data_source/common/data_sources/types.ts +++ b/src/plugins/data_source/common/data_sources/types.ts @@ -6,7 +6,6 @@ import { SavedObjectAttributes } from 'src/core/types'; export interface DataSourceAttributes extends SavedObjectAttributes { - id?: string; title: string; description?: string; endpoint: string; diff --git a/src/plugins/data_source/server/client/configure_client.ts b/src/plugins/data_source/server/client/configure_client.ts index 3f8a64f71d86..cc9bcfd9b361 100644 --- a/src/plugins/data_source/server/client/configure_client.ts +++ b/src/plugins/data_source/server/client/configure_client.ts @@ -25,7 +25,7 @@ export const configureClient = async ( logger: Logger ): Promise => { try { - const { attributes: dataSource } = await getDataSource(dataSourceId, savedObjects); + const { attributes: dataSource } = await getDataSource(dataSourceId!, savedObjects); const rootClient = getRootClient(dataSource, config, openSearchClientPoolSetup); return await getQueryClient(rootClient, dataSource, cryptography); @@ -38,7 +38,7 @@ export const configureClient = async ( }; export const configureTestClient = async ( - { savedObjects, cryptography }: DataSourceClientParams, + { savedObjects, cryptography, dataSourceId }: DataSourceClientParams, dataSource: DataSourceAttributes, openSearchClientPoolSetup: OpenSearchClientPoolSetup, config: DataSourcePluginConfigType, @@ -46,28 +46,21 @@ export const configureTestClient = async ( ): Promise => { try { const { - id, auth: { type, credentials }, } = dataSource; let requireDecryption = false; const rootClient = getRootClient(dataSource, config, openSearchClientPoolSetup); - if (type === AuthType.UsernamePasswordType && !credentials?.password && id) { - const { attributes: fetchedDataSource } = await getDataSource(id || '', savedObjects); - dataSource.auth = { - type, - credentials: { - username: credentials?.username || '', - password: fetchedDataSource.auth.credentials?.password || '', - }, - }; + if (type === AuthType.UsernamePasswordType && !credentials?.password && dataSourceId) { + const dataSourceSavedObject = await getDataSource(dataSourceId, savedObjects); + dataSource = dataSourceSavedObject.attributes; requireDecryption = true; } return getQueryClient(rootClient, dataSource, cryptography, requireDecryption); } catch (error: any) { - logger.error(`Failed to get data source client for dataSource: ${dataSource}`); + logger.error(`Failed to get test client for dataSource: ${dataSource}`); logger.error(error); // Re-throw as DataSourceError throw createDataSourceError(error); diff --git a/src/plugins/data_source/server/data_source_service.ts b/src/plugins/data_source/server/data_source_service.ts index f841c2ec3067..798fce739216 100644 --- a/src/plugins/data_source/server/data_source_service.ts +++ b/src/plugins/data_source/server/data_source_service.ts @@ -40,8 +40,8 @@ export class DataSourceService { } async setup(config: DataSourcePluginConfigType): Promise { - const opensearchClientPoolSetup = await this.openSearchClientPool.setup(config); - const legacyClientPoolSetup = await this.legacyClientPool.setup(config); + const opensearchClientPoolSetup = this.openSearchClientPool.setup(config); + const legacyClientPoolSetup = this.legacyClientPool.setup(config); const getDataSourceClient = async ( params: DataSourceClientParams diff --git a/src/plugins/data_source/server/legacy/configure_legacy_client.test.ts b/src/plugins/data_source/server/legacy/configure_legacy_client.test.ts index f5a192a1cae5..bfdf0ce585f0 100644 --- a/src/plugins/data_source/server/legacy/configure_legacy_client.test.ts +++ b/src/plugins/data_source/server/legacy/configure_legacy_client.test.ts @@ -19,7 +19,7 @@ import { configureLegacyClient } from './configure_legacy_client'; const DATA_SOURCE_ID = 'a54b76ec86771ee865a0f74a305dfff8'; // TODO: improve UT -describe.skip('configureLegacyClient', () => { +describe('configureLegacyClient', () => { let logger: ReturnType; let config: DataSourcePluginConfigType; let savedObjectsMock: jest.Mocked; diff --git a/src/plugins/data_source/server/legacy/configure_legacy_client.ts b/src/plugins/data_source/server/legacy/configure_legacy_client.ts index 94e6b40a4d2e..137d5b506fb3 100644 --- a/src/plugins/data_source/server/legacy/configure_legacy_client.ts +++ b/src/plugins/data_source/server/legacy/configure_legacy_client.ts @@ -33,10 +33,10 @@ export const configureLegacyClient = async ( logger: Logger ) => { try { - const dataSource = await getDataSource(dataSourceId, savedObjects); + const dataSource = await getDataSource(dataSourceId!, savedObjects); const rootClient = getRootClient(dataSource.attributes, config, openSearchClientPoolSetup); - return await getQueryClient(rootClient, dataSource, cryptography, callApiParams); + return await getQueryClient(rootClient, dataSource.attributes, cryptography, callApiParams); } catch (error: any) { logger.error(`Failed to get data source client for dataSourceId: [${dataSourceId}]`); logger.error(error); @@ -55,11 +55,11 @@ export const configureLegacyClient = async ( */ const getQueryClient = async ( rootClient: Client, - dataSource: SavedObject, + dataSource: DataSourceAttributes, cryptography: CryptographyServiceSetup, { endpoint, clientParams, options }: LegacyClientCallAPIParams ) => { - const authType = dataSource.attributes.auth.type; + const authType = dataSource.auth.type; switch (authType) { case AuthType.NoAuth: diff --git a/src/plugins/data_source/server/lib/error.ts b/src/plugins/data_source/server/lib/error.ts index 9a9866a072d6..9d1108e13e16 100644 --- a/src/plugins/data_source/server/lib/error.ts +++ b/src/plugins/data_source/server/lib/error.ts @@ -11,9 +11,18 @@ import { OsdError } from '../../../opensearch_dashboards_utils/common'; export class DataSourceError extends OsdError { // must have statusCode to avoid route handler in search.ts to return 500 statusCode: number; - constructor(error: any, message?: string, statusCode?: number) { - message = message ? message : error.message; + constructor(error: any, context?: string, statusCode?: number) { + let message: string; + if (context) { + message = context; + } else if (isResponseError(error)) { + message = JSON.stringify(error.meta.body); + } else { + message = error.message; + } + super('Data Source Error: ' + message); + if (statusCode) { this.statusCode = statusCode; } else if (error.statusCode) { diff --git a/src/plugins/data_source/server/routes/data_source_connection_validator.ts b/src/plugins/data_source/server/routes/data_source_connection_validator.ts index b23a92624d2a..ecec07dafcc4 100644 --- a/src/plugins/data_source/server/routes/data_source_connection_validator.ts +++ b/src/plugins/data_source/server/routes/data_source_connection_validator.ts @@ -13,11 +13,7 @@ export class DataSourceConnectionValidator { try { return await this.callDataCluster.info(); } catch (e) { - if (e.statusCode === 403) { - return true; - } else { - throw createDataSourceError(e); - } + throw createDataSourceError(e); } } } diff --git a/src/plugins/data_source/server/routes/test_connection.ts b/src/plugins/data_source/server/routes/test_connection.ts index edebd4feb91f..8f90577b045f 100644 --- a/src/plugins/data_source/server/routes/test_connection.ts +++ b/src/plugins/data_source/server/routes/test_connection.ts @@ -20,36 +20,40 @@ export const registerTestConnectionRoute = ( path: '/internal/data-source-management/validate', validate: { body: schema.object({ - id: schema.string(), - endpoint: schema.string(), - auth: schema.maybe( - schema.object({ - type: schema.oneOf([schema.literal('username_password'), schema.literal('no_auth')]), - credentials: schema.oneOf([ - schema.object({ - username: schema.string(), - password: schema.string(), - }), - schema.literal(null), - ]), - }) - ), + id: schema.maybe(schema.string()), + dataSourceAttr: schema.object({ + endpoint: schema.string(), + auth: schema.maybe( + schema.object({ + type: schema.oneOf([ + schema.literal('username_password'), + schema.literal('no_auth'), + ]), + credentials: schema.oneOf([ + schema.object({ + username: schema.string(), + password: schema.string(), + }), + schema.literal(null), + ]), + }) + ), + }), }), }, }, async (context, request, response) => { - const dataSource: DataSourceAttributes = request.body as DataSourceAttributes; - - const dataSourceClient: OpenSearchClient = await dataSourceServiceSetup.getTestingClient( - { - dataSourceId: dataSource.id || '', - savedObjects: context.core.savedObjects.client, - cryptography, - }, - dataSource - ); + const { dataSourceAttr, id: dataSourceId } = request.body; try { + const dataSourceClient: OpenSearchClient = await dataSourceServiceSetup.getTestingClient( + { + dataSourceId, + savedObjects: context.core.savedObjects.client, + cryptography, + }, + dataSourceAttr as DataSourceAttributes + ); const dsValidator = new DataSourceConnectionValidator(dataSourceClient); await dsValidator.validate(); diff --git a/src/plugins/data_source/server/types.ts b/src/plugins/data_source/server/types.ts index cf2748d046d7..913218e40d4b 100644 --- a/src/plugins/data_source/server/types.ts +++ b/src/plugins/data_source/server/types.ts @@ -19,7 +19,8 @@ export interface LegacyClientCallAPIParams { } export interface DataSourceClientParams { - dataSourceId: string; + // id is optional when creating test client + dataSourceId?: string; // this saved objects client is used to fetch data source on behalf of users, caller should pass scoped saved objects client savedObjects: SavedObjectsClientContract; cryptography: CryptographyServiceSetup; diff --git a/src/plugins/data_source_management/public/components/utils.test.ts b/src/plugins/data_source_management/public/components/utils.test.ts index 99bb126933a5..ec30c7966971 100644 --- a/src/plugins/data_source_management/public/components/utils.test.ts +++ b/src/plugins/data_source_management/public/components/utils.test.ts @@ -156,7 +156,7 @@ describe('DataSourceManagement: Utils.ts', () => { Array [ "/internal/data-source-management/validate", Object { - "body": "{\\"id\\":\\"\\",\\"endpoint\\":\\"https://test.com\\",\\"auth\\":{\\"type\\":\\"no_auth\\",\\"credentials\\":null}}", + "body": "{\\"dataSourceAttr\\":{\\"endpoint\\":\\"https://test.com\\",\\"auth\\":{\\"type\\":\\"no_auth\\",\\"credentials\\":null}}}", }, ], ] @@ -170,7 +170,7 @@ describe('DataSourceManagement: Utils.ts', () => { Array [ "/internal/data-source-management/validate", Object { - "body": "{\\"id\\":\\"test1234\\",\\"endpoint\\":\\"https://test.com\\",\\"auth\\":{\\"type\\":\\"no_auth\\",\\"credentials\\":null}}", + "body": "{\\"id\\":\\"test1234\\",\\"dataSourceAttr\\":{\\"endpoint\\":\\"https://test.com\\",\\"auth\\":{\\"type\\":\\"no_auth\\",\\"credentials\\":null}}}", }, ], ] diff --git a/src/plugins/data_source_management/public/components/utils.ts b/src/plugins/data_source_management/public/components/utils.ts index dafc03777fca..3d9d6f51b413 100644 --- a/src/plugins/data_source_management/public/components/utils.ts +++ b/src/plugins/data_source_management/public/components/utils.ts @@ -85,11 +85,13 @@ export async function testConnection( dataSourceID?: string ) { const query: any = { - id: dataSourceID || '', - endpoint, - auth: { - type, - credentials: type === AuthType.NoAuth ? null : { ...credentials }, + id: dataSourceID, + dataSourceAttr: { + endpoint, + auth: { + type, + credentials: type === AuthType.NoAuth ? null : { ...credentials }, + }, }, };