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

fix(NODE-5551): set AWS region from environment variable for STSClient #3831

Merged
merged 5 commits into from Aug 29, 2023

Conversation

nbbeeken
Copy link
Contributor

@nbbeeken nbbeeken commented Aug 21, 2023

Description

What is changing?

  • Adding detection for AWS_STS_REGIONAL_ENDPOINTS and AWS_REGION environment variables and setting the corresponding fromNodeProviderChain options.
  • LEGACY_REGIONS is created from this: https://docs.aws.amazon.com/sdkref/latest/guide/feature-sts-regionalized-endpoints.html documentation
  • The driver will only set region options if BOTH environment variables are present, this is so that the driver does not encode a default setting for AWS_STS_REGIONAL_ENDPOINTS.
Is there new documentation needed for these changes?

Yes. Will file a docs ticket.

What is the motivation for this change?

AWS recommends setting your STS region to be as close to your app as possible to reduce latency but currently, the JS SDK does not use the common AWS region environment variables, and the setting must be controlled programmatically.

Tracking issue here: aws/aws-sdk-js-v3#5105

Release Highlight

Use region settings for STS AWS credentials request

When using IAM AssumeRoleWithWebIdentity AWS authentication the driver uses the @aws-sdk/credential-providers package to contact the Security Token Service API for temporary credentials. AWS recommends using Regional AWS STS endpoints instead of the global endpoint to reduce latency, build-in redundancy, and increase session token validity. Unfortunately, environment variables AWS_STS_REGIONAL_ENDPOINTS and AWS_REGION do not directly control the region the SDK's STS client contacts for credentials.

The driver now has added support for detecting these variables and setting the appropriate options when calling the SDK's API: fromNodeProviderChain().

Important

The driver will only set region options if BOTH environment variables are present. AWS_STS_REGIONAL_ENDPOINTS MUST be set to either 'legacy' or 'regional', and AWS_REGION must be set.

Double check the following

  • Ran npm run check:lint script
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: type(NODE-xxxx)[!]: description
    • Example: feat(NODE-1234)!: rewriting everything in coffeescript
  • Release notes are filled out
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

test/integration/auth/mongodb_aws.test.ts Show resolved Hide resolved
src/cmap/auth/mongodb_aws.ts Show resolved Hide resolved
src/cmap/auth/mongodb_aws.ts Show resolved Hide resolved
@nbbeeken
Copy link
Contributor Author

@nbbeeken nbbeeken marked this pull request as ready for review August 21, 2023 18:55
@nbbeeken nbbeeken changed the title fix(NODE-5477): set AWS region from environment variable for STSClient fix(NODE-5551): set AWS region from environment variable for STSClient Aug 21, 2023
@W-A-James W-A-James added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Aug 22, 2023
@W-A-James W-A-James self-requested a review August 22, 2023 18:35
@W-A-James W-A-James self-assigned this Aug 22, 2023
Copy link
Contributor

@W-A-James W-A-James left a comment

Choose a reason for hiding this comment

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

Apart from the one question on the test file (which I won't block on), LGTM

test/integration/auth/mongodb_aws.test.ts Outdated Show resolved Hide resolved
@W-A-James W-A-James added Team Review Needs review from team and removed Primary Review In Review with primary reviewer, not yet ready for team's eyes labels Aug 22, 2023
test/integration/auth/mongodb_aws.test.ts Outdated Show resolved Hide resolved
@nbbeeken nbbeeken added the Blocked Blocked on other work label Aug 24, 2023
@nbbeeken nbbeeken removed the Blocked Blocked on other work label Aug 29, 2023
@W-A-James W-A-James merged commit e9a5079 into main Aug 29, 2023
1 of 2 checks passed
@W-A-James W-A-James deleted the NODE-5551-aws-sts-region branch August 29, 2023 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team Review Needs review from team
Projects
None yet
3 participants