[MD] Improve test connection (opensearch-project#3110)

Signed-off-by: Su <[email protected]> Signed-off-by: Su <[email protected]> Signed-off-by: Arpit Bandejiya <[email protected]>
Arpit-Bandejiya · Mar 8, 2023 · e92a400 · e92a400
1 parent 7ebb647
commit e92a400
Show file tree

Hide file tree

Showing 12 changed files with 65 additions and 60 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -51,6 +51,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - [Doc] Add current plugin persistence implementation readme ([#3081](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3081))
 - [Table Visualization] Refactor table visualization using React and DataGrid component ([#2863](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2863))
 - [Vis Builder] Add redux store persistence ([#3088](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3088))
+- [Multi DataSource] Improve test connection ([#3110](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3110))
 
 ### 🐛 Bug Fixes
 

diff --git 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
@@ -25,7 +25,7 @@ export const configureClient = async (
   logger: Logger
 ): Promise<Client> => {
   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,36 +38,29 @@ export const configureClient = async (
 };
 
 export const configureTestClient = async (
-  { savedObjects, cryptography }: DataSourceClientParams,
+  { savedObjects, cryptography, dataSourceId }: DataSourceClientParams,
   dataSource: DataSourceAttributes,
   openSearchClientPoolSetup: OpenSearchClientPoolSetup,
   config: DataSourcePluginConfigType,
   logger: Logger
 ): Promise<Client> => {
   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
@@ -40,8 +40,8 @@ export class DataSourceService {
   }
 
   async setup(config: DataSourcePluginConfigType): Promise<DataSourceServiceSetup> {
-    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
@@ -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<typeof loggingSystemMock.createLogger>;
   let config: DataSourcePluginConfigType;
   let savedObjectsMock: jest.Mocked<SavedObjectsClientContract>;

diff --git 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<DataSourceAttributes>,
+  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
@@ -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
@@ -13,11 +13,7 @@ export class DataSourceConnectionValidator {
     try {
       return await this.callDataCluster.info<OpenSearchClient>();
     } 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
@@ -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
@@ -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
@@ -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
@@ -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 },
+      },
     },
   };