Improves connection pooling support for AWSSigV4 clients in data sou…

…rces. (#6135) * Upgrade @opensearch/opensearch@2.6.0 which inherits AWSSigv4 to .child Signed-off-by: Bandini Bhopi <bandinib@amazon.com> * Uses client.child for aws sigv4 connection Signed-off-by: Bandini Bhopi <bandinib@amazon.com> * Import http-aws-es connector class Signed-off-by: Bandini Bhopi <bandinib@amazon.com> * Refactor client caching mechanism Signed-off-by: Bandini Bhopi <bandinib@amazon.com> * Fix UT Signed-off-by: Bandini Bhopi <bandinib@amazon.com> * Added UT for client pool Signed-off-by: Bandini Bhopi <bandinib@amazon.com> * Revert client pool changes from authentication method Signed-off-by: Bandini Bhopi <bandinib@amazon.com> --------- Signed-off-by: Bandini Bhopi <bandinib@amazon.com>
opensearch-project · Mar 15, 2024 · 09f5563 · 09f5563
1 parent 8c4f49a
commit 09f5563
Show file tree

Hide file tree

Showing 19 changed files with 1,147 additions and 200 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -38,6 +38,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - [Workspace] Consume workspace id in saved object client ([#6014](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6014))
 - [Multiple Datasource] Export DataSourcePluginRequestContext at top level for plugins to use ([#6108](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6108))
 - [Multiple Datasource] Expose filterfn in datasource menu component to allow filter data sources before rendering in navigation bar ([#6113](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6113))
+- [Multiple Datasource] Improves connection pooling support for AWSSigV4 clients in data sources ([#6135](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6135))
 - [Workspace] Add delete saved objects by workspace functionality([#6013](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6013))
 - [Workspace] Add a workspace client in workspace plugin ([#6094](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6094))
 - [Multiple Datasource] Add component to show single selected data source in read only mode ([#6125](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6125))

diff --git a/package.json b/package.json
@@ -152,7 +152,7 @@
     "@hapi/podium": "^4.1.3",
     "@hapi/vision": "^6.1.0",
     "@hapi/wreck": "^17.1.0",
-    "@opensearch-project/opensearch": "^2.3.1",
+    "@opensearch-project/opensearch": "^2.6.0",
     "@osd/ace": "1.0.0",
     "@osd/analytics": "1.0.0",
     "@osd/apm-config-loader": "1.0.0",

diff --git a/packages/osd-opensearch-archiver/package.json b/packages/osd-opensearch-archiver/package.json
@@ -13,7 +13,7 @@
   "dependencies": {
     "@osd/dev-utils": "1.0.0",
     "@osd/std": "1.0.0",
-    "@opensearch-project/opensearch": "^2.3.1"
+    "@opensearch-project/opensearch": "^2.6.0"
   },
   "devDependencies": {}
 }
diff --git a/packages/osd-opensearch/package.json b/packages/osd-opensearch/package.json
@@ -12,7 +12,7 @@
     "osd:watch": "../../scripts/use_node scripts/build --watch"
   },
   "dependencies": {
-    "@opensearch-project/opensearch": "^2.3.1",
+    "@opensearch-project/opensearch": "^2.6.0",
     "@osd/dev-utils": "1.0.0",
     "abort-controller": "^3.0.0",
     "chalk": "^4.1.0",

diff --git a/src/core/server/opensearch/client/configure_client.ts b/src/core/server/opensearch/client/configure_client.ts
@@ -45,7 +45,6 @@ export const configureClient = (
   }: { logger: Logger; scoped?: boolean; withLongNumeralsSupport?: boolean }
 ): Client => {
   const clientOptions = parseClientOptions(config, scoped);
-  // @ts-expect-error - ToDo: Remove ignoring after https://github.com/opensearch-project/opensearch-js/pull/598 is included in a release
   if (withLongNumeralsSupport) clientOptions.enableLongNumeralSupport = true;
 
   const client = new Client(clientOptions);

diff --git a/src/core/server/opensearch/client/mocks.ts b/src/core/server/opensearch/client/mocks.ts
@@ -37,7 +37,6 @@ const createInternalClientMock = (withLongNumeralsSupport = false): DeeplyMocked
   // we mimic 'reflection' on a concrete instance of the client to generate the mocked functions.
   const client = new Client({
     node: 'http://localhost',
-    // @ts-expect-error - ToDo: Remove ignoring after https://github.com/opensearch-project/opensearch-js/pull/598 is included in a release
     enableLongNumeralSupport: withLongNumeralsSupport,
   }) as any;
 

diff --git a/src/plugins/data_source/server/auth_registry/authentication_methods_registry.test.ts b/src/plugins/data_source/server/auth_registry/authentication_methods_registry.test.ts
@@ -5,13 +5,11 @@
 
 import { AuthenticationMethodRegistery } from './authentication_methods_registry';
 import { AuthenticationMethod } from '../../server/types';
-import { AuthType } from '../../common/data_sources';
 
 const createAuthenticationMethod = (
   authMethod: Partial<AuthenticationMethod>
 ): AuthenticationMethod => ({
   name: 'unknown',
-  authType: AuthType.NoAuth,
   credentialProvider: jest.fn(),
   ...authMethod,
 });
@@ -61,14 +59,14 @@ describe('AuthenticationMethodRegistery', () => {
       registry.registerAuthenticationMethod(
         createAuthenticationMethod({
           name: 'typeA',
-          authType: AuthType.NoAuth,
+          credentialProvider: jest.fn(),
         })
       );
 
       const typeA = registry.getAuthenticationMethod('typeA')!;
 
       expect(() => {
-        typeA.authType = AuthType.SigV4;
+        typeA.credentialProvider = jest.fn();
       }).toThrow();
       expect(() => {
         typeA.name = 'foo';

diff --git a/src/plugins/data_source/server/auth_registry/authentication_methods_registry.ts b/src/plugins/data_source/server/auth_registry/authentication_methods_registry.ts
@@ -5,6 +5,7 @@
 
 import { deepFreeze } from '@osd/std';
 import { AuthenticationMethod } from '../../server/types';
+import { AuthType } from '../../common/data_sources';
 
 export type IAuthenticationMethodRegistery = Omit<
   AuthenticationMethodRegistery,
@@ -18,6 +19,15 @@ export class AuthenticationMethodRegistery {
    * Authentication Method can only be registered once. subsequent calls with the same method name will throw an error.
    */
   public registerAuthenticationMethod(method: AuthenticationMethod) {
+    if (
+      method.name === AuthType.NoAuth ||
+      method.name === AuthType.UsernamePasswordType ||
+      method.name === AuthType.SigV4
+    ) {
+      throw new Error(
+        `Must not be no_auth or username_password or sigv4 for registered auth types`
+      );
+    }
     if (this.authMethods.has(method.name)) {
       throw new Error(`Authentication method '${method.name}' is already registered`);
     }

diff --git a/src/plugins/data_source/server/client/configure_client.test.mocks.ts b/src/plugins/data_source/server/client/configure_client.test.mocks.ts
@@ -21,12 +21,3 @@ export const authRegistryCredentialProviderMock = jest.fn();
 jest.doMock('../util/credential_provider', () => ({
   authRegistryCredentialProvider: authRegistryCredentialProviderMock,
 }));
-
-export const CredentialsMock = jest.fn();
-jest.doMock('aws-sdk', () => {
-  const actual = jest.requireActual('aws-sdk');
-  return {
-    ...actual,
-    Credentials: CredentialsMock,
-  };
-});