adds a new export from the package for aws-sdk v3

[tusharf5]

Description

Based on the conversation in this issue #757 , this PR adds a new way to import the AwsSigv4Signer as shown below.

const { AwsSigv4Signer } = require('@opensearch-project/opensearch/aws-v3');

By using a new import path, the sdk remains backwards compatible while also adding a way for clients to avoid getting the entire aws-sdk included in the final bundle which results in an additional 22MB.

I tested the code and it does reduce the bundle size with the new import path

The bundle size is still not good though because '@aws-sdk/credential-provider-node' dynamically imports all types of credential providers ini, ec2, environment, node, process, http etc which is about 1MB in size.

Would it make sense to have the client provide the provider based on the environment instead of relying on the default provider chain? This would be a breaking change but it would delegate the credential responsibility on the client eliminating the dependency on aws-sdk & @aws-sdk/credential-provider-node?

https://github.com/opensearch-project/opensearch-js/blob/main/lib/aws/AwsSigv4Signer.js#L74

We will have to throw an error on this (making it required) line so the client always provides a getCredential function and we can remove the dependency on a default provider (both v2 and v3) from our end.

The doc examples already use the getCredential function so I am guessing most of the clients would already be providing this function and not relying on the default getCredential.

https://opensearch.org/docs/latest/clients/javascript/index/#authenticating-with-amazon-opensearch-service-aws-signature-version-4

Issues Resolved

Closes #757

Check List

• New functionality includes testing.
  • All tests pass
• Linter check was successfull - yarn run lint doesn't show any errors
• Commits are signed per the DCO using --signoff
• Changelog was updated.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[dblock]

Good stuff. Document new functionality/changes in README, USERS_GUIDE, update samples. Add tests.

[dblock]

Would it make sense to have the client provide the provider based on the environment instead of relying on the default provider chain? This would be a breaking change but it would delegate the credential responsibility on the client eliminating the dependency on aws-sdk & @aws-sdk/credential-provider-node?

It's a good idea, but I would explore this in a future/separate PR.

[tusharf5]

@dblock updated the PR with the requested changes

[dblock]

👏 This looks great to me!

Let's get another pair of eyes, maybe @nhtruong and merge?

[dblock]

@tusharf5 fix CI pls, there's some license headers missing, and a complaint about a new license, 0BSD. It's just a BSD variant so I think we can add it to the list, but I'll go double check and we already allow it in OpenSearch-Dasbhoards.

[dblock]

The license workflow needs to have 0BSD added to it, https://github.com/opensearch-project/opensearch-js/actions/runs/8739773216/job/23986128639?pr=758.

[tusharf5]

@dblock fixed

[tusharf5]

checking the test error

[tusharf5]

1 existing test was failing since aws-sdk and v3 ask were not installed so the error thrown was different and now they are part of dev dependency so the error has changed. I've updated the test. hopefully no more test failures. @dblock

Edit - For some reason I can't run tests locally, I get "ERR_REQUIRE_ESM" errors otherwise I would have caught & fixed these errors locally.

[tusharf5]

Had to remove the peerDependency change since npm automatically installs peerDepenencies (yarn does not) and that changes the import result which fails the test. It should be working now.

[nhtruong]

Are these tests only testing if they throw the correct errors, and not for the code paths where no errors are thrown? I think that's why Codecov is not happy.

[tusharf5]

This would require mocking the import as the dependencies are not installed. While tap supports mocking imports, the project does not use the latest tap version which has breaking changes.

Doesn't seem like there is any other way to test dependency missing/present scenarios without mocking.

[dblock]

We should solve this with integration tests IMO, which is a separate affair.

[nhtruong]

I'm thinking of setting up different package.json for each test case like you have in test/bundlers/esbuild-test/package.json

[dblock]

It's a good idea. Do we want to hold this PR until then? Open an issue and do it later?

[nhtruong]

Let's do it in a different PR so we can get this merged. I'll make an issue.