Skip to content

feat: Support AWS SDK v3 #1043

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

Merged
merged 5 commits into from
Feb 23, 2023
Merged

feat: Support AWS SDK v3 #1043

merged 5 commits into from
Feb 23, 2023

Conversation

seebees
Copy link
Contributor

@seebees seebees commented Oct 30, 2022

To support AWS SDK v3,
there needs to be a single interface
between AWS SDK v2 and v3
for the AWS KMS Client.
The AwsEsdkKMSInterface is a close copy
that needs to be shared between kms-keyring,
kms-keyring-node, and kms-keyring-browser.

Adding @aws-sdk/client-kms as a devDependencies
tests have been added for v2 and v3
for every AWS KMS keyring.

Two additional changes were required.

First, getting the region value
from an AWS SDK v3 client is an async function,
rather than a string property.
This means that the AwsKmsMrkAwareSymmetricDiscoveryKeyring
needs to be able to handle this.
The constructor, a synchronous function
needs to store enough state
that the _OnDecrypt function
can await the setting of the region value.

Second in setting this value readOnlyProperty
was updated to explicitly set writable: false.
This allows this helper to update a property
and set this updated value to read only.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Check any applicable:

  • Were any files moved? Moving files changes their URL, which breaks all hyperlinks to the files.

@seebees seebees requested a review from a team as a code owner October 30, 2022 19:02
@seebees
Copy link
Contributor Author

seebees commented Oct 30, 2022

This is built on #1041 and should be rebase after that has been merged

@seebees seebees marked this pull request as draft October 30, 2022 19:03
To support AWS SDK v3,
there needs to be a single interface
between AWS SDK v2 and v3
for the AWS KMS Client.
The `AwsEsdkKMSInterface` is a close copy
that needs to be shared between `kms-keyring`,
`kms-keyring-node, and `kms-keyring-browser`.

Adding `@aws-sdk/client-kms` as a devDependencies
tests have been added for v2 and v3
for every AWS KMS keyring.

Two additional changes were required.

First, getting the `region` value
from an AWS SDK v3 client is an `async` function,
rather than a string property.
This means that the `AwsKmsMrkAwareSymmetricDiscoveryKeyring`
needs to be able to handle this.
The constructor, a syncronus function
needs to store enugh state
that the _OnDecrypt function
can await the setting of the region value.

Second in setting this value `readOnlyProperty`
was updated to explictly set `writable: false`.
This allows this helper to update a property
and set this updated value to read only.
@seebees seebees marked this pull request as ready for review October 31, 2022 19:51
@brianfrantz
Copy link

Is there an ETA on pushing this out? Guessing this will unblock me from using AWS SSO, which isn't working in the current version...

Copy link
Contributor

@texastony texastony left a comment

Choose a reason for hiding this comment

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

I am not Java Script savy enough to know if we can mark a dependency as a test or dev dependency inside a module.

If we cannot, does this mean that our customers will pull in both AWS SDKs?

@seebees seebees merged commit 33a9e43 into aws:master Feb 23, 2023
@seebees seebees deleted the aws-sdk-v3 branch February 23, 2023 18:28
@texastony
Copy link
Contributor

texastony commented Feb 23, 2023

Our Release Build failed on testBrowser:

[Container] 2023/02/23 20:29:03 Running command n 18
  installing : node-v18.14.2
       mkdir : /usr/local/n/versions/node/18.14.2
       fetch : https://nodejs.org/dist/v18.14.2/node-v18.14.2-linux-x64.tar.xz
     copying : node/18.14.2
node: /lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.28' not found (required by node)
/usr/local/bin/node: /lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.28' not found (required by /usr/local/bin/node)
   installed :  (with npm )

[Container] 2023/02/23 20:29:18 Running command npm ci --unsafe-perm
node: /lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.28' not found (required by node)

[Container] 2023/02/23 20:29:18 Command did not exit successfully npm ci --unsafe-perm exit status 1
[Container] 2023/02/23 20:29:18 Phase complete: INSTALL State: FAILED
[Container] 2023/02/23 20:29:18 Phase context status code: COMMAND_EXECUTION_ERROR Message: Error while executing command: npm ci --unsafe-perm. Reason: exit status 1

Fudge... Maybe the CodeBuild Image is too old...

Update @ 12:53 PST:
CodeBuild only supports up to Node 16:
https://docs.aws.amazon.com/codebuild/latest/userguide/available-runtimes.html

But surely testBrowser just passed earlier today?

It did. But the CI uses Image aws/codebuild/standard:5.0,
while release is using aws/codebuild/standard:4.0.

I could either downgrade the Node version used in CodeBuild to 16,
or update the CodeBuild project Image to aws/codebuild/standard:5.0in release.

I am going to update the Release Image to aws/codebuild/standard:5.0;
it's best to have Release and CI be on the same page.

@texastony
Copy link
Contributor

Is there an ETA on pushing this out? Guessing this will unblock me from using AWS SSO, which isn't working in the current version...

@brianfrantz
We have released AWS SKD V3 support in ESDK-JS 3.2.0.
Note: The ESDK-JS still lists the AWS SDK V2 as a dependency.
We will release a new major version that removes this V2 dependency
as a breaking change.

@luk2302
Copy link

luk2302 commented Apr 24, 2023

@texastony any update on when that major release is supposed to happen?

@texastony
Copy link
Contributor

@luc2302, this work is on our roadmap.
However, we cannot commit to a date.

@dmyerspgr
Copy link

Bumping - Hopeful AWS SDK v3 major release soon! This would greatly improve our lambdas performance and cost.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants