-
Notifications
You must be signed in to change notification settings - Fork 63
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
feat: Support AWS SDK v3 #1043
Conversation
This is built on #1041 and should be rebase after that has been merged |
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.
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... |
There was a problem hiding this 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?
Our Release Build failed on
Fudge... Maybe the CodeBuild Image is too old... Update @ 12:53 PST: But surely It did. But the CI uses Image I could either downgrade the Node version used in CodeBuild to 16, I am going to update the Release Image to |
@brianfrantz |
@texastony any update on when that major release is supposed to happen? |
@luc2302, this work is on our roadmap. |
Bumping - Hopeful AWS SDK v3 major release soon! This would greatly improve our lambdas performance and cost. |
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 copythat needs to be shared between
kms-keyring
,kms-keyring-node
, andkms-keyring-browser
.Adding
@aws-sdk/client-kms
as a devDependenciestests have been added for v2 and v3
for every AWS KMS keyring.
Two additional changes were required.
First, getting the
region
valuefrom 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: