Improves connection pooling support for AWSSigV4 clients in data sou…

…rces. (opensearch-project#6135) * Upgrade @opensearch/[email protected] which inherits AWSSigv4 to .child Signed-off-by: Bandini Bhopi <[email protected]> * Uses client.child for aws sigv4 connection Signed-off-by: Bandini Bhopi <[email protected]> * Import http-aws-es connector class Signed-off-by: Bandini Bhopi <[email protected]> * Refactor client caching mechanism Signed-off-by: Bandini Bhopi <[email protected]> * Fix UT Signed-off-by: Bandini Bhopi <[email protected]> * Added UT for client pool Signed-off-by: Bandini Bhopi <[email protected]> * Revert client pool changes from authentication method Signed-off-by: Bandini Bhopi <[email protected]> --------- Signed-off-by: Bandini Bhopi <[email protected]>
manasvinibs · 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:/opensearch-project/OpenSearch-Dashboards/pull/6014))
 - [Multiple Datasource] Export DataSourcePluginRequestContext at top level for plugins to use ([#6108](https:/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:/opensearch-project/OpenSearch-Dashboards/pull/6113))
+- [Multiple Datasource] Improves connection pooling support for AWSSigV4 clients in data sources ([#6135](https:/opensearch-project/OpenSearch-Dashboards/pull/6135))
 - [Workspace] Add delete saved objects by workspace functionality([#6013](https:/opensearch-project/OpenSearch-Dashboards/pull/6013))
 - [Workspace] Add a workspace client in workspace plugin ([#6094](https:/opensearch-project/OpenSearch-Dashboards/pull/6094))
 - [Multiple Datasource] Add component to show single selected data source in read only mode ([#6125](https:/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:/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:/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,
- };
-});