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

Improves connection pooling support for AWSSigV4 clients in data sources. #6135

Merged
merged 7 commits into from
Mar 15, 2024

Conversation

bandinib-amzn
Copy link
Member

@bandinib-amzn bandinib-amzn commented Mar 13, 2024

Description

As of today, both opensearch-js client and legacy client does not allow updating credentials for existing client. As a result, every HTTP request to AWS services entails the costly process of creating new client.

This PR solves exactly that problem. We have recently released opensearch-js 2.6.0 client to npm. 2.6.0 client has inherited AwsSigV4 in .child. This PR upgrades the opensearch-js 2.6.0 and refactor data source plugin to create child client if endpoint/node is same. Also this PR imports the HttpAmazonESConnector class and it's UT in OpenSearch-Dashboards repo. This will help us to maintain HttpAmazonESConnector proactively.

This PR

  • Upgrade @opensearch/[email protected] which supports AwsSigV4 in .child
  • Add support to create child client for AWS sigv4 auth type, similar to basic auth
  • Modify client caching mechanism
  • Copy HttpAmazonESConnector
  • Add child support in HttpAmazonESConnector
  • Add support to create child client for AWS sigv4 auth type, similar to basic auth
  • Does not allow authentication to be registered with the same name as no_auth, username_password, sigv4.

Issues Resolved

#6114, #5932, partially resolves #5838

Screenshot

Testing the changes

Check List

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

Copy link

codecov bot commented Mar 13, 2024

Codecov Report

Attention: Patch coverage is 77.68595% with 27 lines in your changes are missing coverage. Please review.

Project coverage is 67.26%. Comparing base (8c4f49a) to head (18af6c9).

Files Patch % Lines
...data_source/server/legacy/http_aws_es/connector.js 64.40% 21 Missing ⚠️
...r/auth_registry/authentication_methods_registry.ts 33.33% 1 Missing and 1 partial ⚠️
...ins/data_source/server/util/credential_provider.ts 0.00% 2 Missing ⚠️
...gins/data_source/server/client/configure_client.ts 95.23% 0 Missing and 1 partial ⚠️
...ta_source/server/legacy/configure_legacy_client.ts 95.83% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6135      +/-   ##
==========================================
+ Coverage   67.22%   67.26%   +0.03%     
==========================================
  Files        3333     3334       +1     
  Lines       64565    64639      +74     
  Branches    10391    10399       +8     
==========================================
+ Hits        43404    43478      +74     
- Misses      18628    18632       +4     
+ Partials     2533     2529       -4     
Flag Coverage Δ
Linux_1 31.69% <ø> (ø)
Linux_2 55.45% <ø> (ø)
Linux_3 44.76% <77.68%> (+0.10%) ⬆️
Linux_4 34.99% <ø> (ø)
Windows_1 31.74% <ø> (+0.02%) ⬆️
Windows_2 55.41% <ø> (ø)
Windows_3 44.76% <77.68%> (+0.08%) ⬆️
Windows_4 34.99% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bandinib-amzn bandinib-amzn changed the title Improves connection pooling support for AWSSigV4 clients/connection in data source Improves connection pooling support for AWSSigV4 clients in data sources. Mar 13, 2024
@bandinib-amzn bandinib-amzn force-pushed the awssigv4-child branch 3 times, most recently from 3e91572 to 9933938 Compare March 14, 2024 09:43
* @param [config.protocol=http:] {String} - The HTTP protocol that this connection will use, can be set to https:
* @class HttpConnector
*/
import { Config, Credentials } from 'aws-sdk';
Copy link
Member

Choose a reason for hiding this comment

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

It seem like that Config and Credentials were not used directly in the code. We may remove this. it is not block, we can do it later.

@bandinib-amzn bandinib-amzn merged commit 09f5563 into opensearch-project:main Mar 15, 2024
133 checks passed
@bandinib-amzn bandinib-amzn deleted the awssigv4-child branch March 15, 2024 21:49
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch-Dashboards/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch-Dashboards/backport-2.x
# Create a new branch
git switch --create backport/backport-6135-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 09f55636e4370b003c42d5bcd8ae93535c230c00
# Push it to GitHub
git push --set-upstream origin backport/backport-6135-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch-Dashboards/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-6135-to-2.x.

ananzh pushed a commit to ananzh/OpenSearch-Dashboards that referenced this pull request Mar 15, 2024
…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]>
bandinib-amzn added a commit to bandinib-amzn/OpenSearch-Dashboards that referenced this pull request Mar 18, 2024
…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]>
(cherry picked from commit 09f5563)
bandinib-amzn added a commit to bandinib-amzn/OpenSearch-Dashboards that referenced this pull request Mar 18, 2024
…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]>
(cherry picked from commit 09f5563)
bandinib-amzn added a commit to bandinib-amzn/OpenSearch-Dashboards that referenced this pull request Mar 18, 2024
…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]>
(cherry picked from commit 09f5563)
bandinib-amzn added a commit that referenced this pull request Mar 19, 2024
…rces. (#6135) (#6176)

* 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]>
(cherry picked from commit 09f5563)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Meta] Refactoring data source plugin to support add-on authentication method with plug-in module
4 participants