fix: Fixes STS region resolution when using cross-region authentication #3718

maastha · 2025-09-26T14:01:09Z

Description

Fixes STS region resolution when using cross-region authentication in the MongoDB Atlas Terraform provider. The fix addresses an issue where the provider wasn't correctly handling STS endpoint configuration when AWS credentials are in one region but the STS endpoint is in another region.

Link to any related issue(s): CLOUDP-347906

Type of change:

Bug fix (non-breaking change which fixes an issue). Please, add the "bug" label to the PR.
New feature (non-breaking change which adds functionality). Please, add the "enhancement" label to the PR. A migration guide must be created or updated if the new feature will go in a major version.
Breaking change (fix or feature that would cause existing functionality to not work as expected). Please, add the "breaking change" label to the PR. A migration guide must be created or updated.
This change requires a documentation update
Documentation fix/enhancement

Required Checklist:

I have signed the MongoDB CLA
I have read the contributing guides
I have checked that this change does not generate any credentials and that they are NOT accidentally logged anywhere.
I have added tests that prove my fix is effective or that my feature works per HashiCorp requirements
I have added any necessary documentation (if appropriate)
I have run make fmt and formatted my code
If changes include deprecations or removals I have added appropriate changelog entries.
If changes include removal or addition of 3rd party GitHub actions, I updated our internal document. Reach out to the APIx Integration slack channel to get access to the internal document.

Further comments

github-actions · 2025-09-26T20:59:24Z

APIx bot: a message has been sent to Docs Slack channel

Copilot

Pull Request Overview

This PR fixes STS region resolution when using cross-region authentication in the MongoDB Atlas Terraform provider. The fix addresses an issue where the provider wasn't correctly handling STS endpoint configuration when AWS credentials are in one region but the STS endpoint is in another region.

Refactors STS endpoint resolution logic to properly derive signing regions from endpoint URLs
Adds utility functions for parsing STS endpoints and resolving regional configurations
Introduces comprehensive test coverage for cross-region authentication scenarios

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
internal/provider/credentials.go	Implements new STS endpoint resolution logic with proper region parsing
internal/provider/credentials_test.go	Adds comprehensive test coverage for STS endpoint resolution functions
.github/workflows/acceptance-tests-runner.yml	Updates CI to test cross-region authentication scenarios with matrix strategy
.changelog/3718.txt	Adds changelog entry documenting the bug fix

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

internal/provider/credentials.go

maastha · 2025-09-26T20:59:39Z

will hold off on merging this until we hear back from the customer

lantoli · 2025-09-29T06:07:46Z

.github/workflows/acceptance-tests-runner.yml

+      fail-fast: false
+      matrix:
+        include:
+          # Same region


nit: the name makes it clear, no need for the comments

made the comment more clear and specific in 06a0b97. I think it's useful

lantoli · 2025-09-29T06:17:52Z

.github/workflows/acceptance-tests-runner.yml

+          # Cross-region
+          - name: cross-sts-us-east-1-secret-eu-north-1
+            aws_region: EU_NORTH_1
+            sts_endpoint: https://sts.us-east-1.amazonaws.com/


although the test group list is "polluted" instead of having only one entry for assume_role, i think it's good use case for matrix.

also we'll need to merge with changes in SA dev branch, we might need to remove the matrix approach if we want to keep all authentication tests together, but that's ok. cc @oarbusi

no need for action about this comment.

lantoli · 2025-09-29T06:21:26Z

internal/provider/credentials.go

+
+	return endpoints.ResolvedEndpoint{
+		URL:           ep,
+		SigningRegion: signingRegion,


do you mind to check if SigningRegion is really needed or URL is enough so we don't need to calculate the sts region?

+1, can we actually check how AWS does it?

Yes, SigningRegion is needed. URL alone isn’t sufficient.

The AWS signer uses the client’s region when SigningRegion is not set. In this case, the client region is the Secrets Manager region, which may not match the STS endpoint’s region.

For the global endpoint sts.amazonaws.com, requests must be signed with us-east-1; without setting SigningRegion, signatures will be computed with the client region and can fail.

For regional STS endpoints, the signature must match that region as well.

lantoli · 2025-09-29T06:22:03Z

.github/workflows/acceptance-tests-runner.yml

    runs-on: ubuntu-latest
+    strategy:
+      fail-fast: false
+      matrix:


nice test coverage!

internal/provider/credentials_test.go

manupedrozo · 2025-09-29T07:54:08Z

internal/provider/credentials.go

+	if err != nil {
+		return DefaultRegionSTS
+	}
+	host := u.Hostname() // valid values: sts.us-west-2.amazonaws.com or sts.amazonaws.com


This removes the port if provided, correct? Not sure if needed.

exactly, "Hostname returns u.Host, stripping any valid port number if present.". It's not strictly necessary but with it we make sure at this point we have either sts.us-west-2.amazonaws.com or sts.amazonaws.com

manupedrozo · 2025-09-29T07:54:36Z

internal/provider/credentials.go

+		ep = fmt.Sprintf("https://sts.%s.amazonaws.com/", r)
+	}
+
+	signingRegion := DeriveSTSRegionFromEndpoint(ep)


If the endpoint is "", no need to derive the region since we know it already.

if endpoint is "" the region can be secretsRegion or default region. I think we can keep this as is to avoid having repeated logic(e.g. if r == ""). WDYT?

I was thinking something along the lines of:

ep := stsEndpoint var signingRegion string if ep == "" { signingRegion = secretsRegion if signingRegion == "" { signingRegion = DefaultRegionSTS } ep = fmt.Sprintf("https://sts.%s.amazonaws.com/", signingRegion) } else { signingRegion = DeriveSTSRegionFromEndpoint(ep) }

But ok either way

lantoli · 2025-09-29T08:39:09Z

.changelog/3718.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+provider: Fixes STS region resolution when using cross-region authentication


I think that it'd have been better to create an associated cloudp ticket

I agree, thank you Leo

Created CLOUDP-347906

marcosuma

nice one

marcosuma · 2025-09-29T10:17:48Z

.changelog/3718.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+provider: Fixes STS region resolution when using cross-region authentication


I agree, thank you Leo

marcosuma · 2025-09-29T10:23:55Z

.github/workflows/acceptance-tests-runner.yml

+        include:
+          # Same region
+          - name: same-region-us-east-1
+            aws_region: US_EAST_1


nit: could we lease a comment to explain what this aws_region is used for?

marcosuma · 2025-09-29T10:24:37Z

.github/workflows/acceptance-tests-runner.yml

    runs-on: ubuntu-latest
+    strategy:
+      fail-fast: false
+      matrix:


marcosuma · 2025-09-29T10:25:19Z

internal/provider/credentials.go

+
+	return endpoints.ResolvedEndpoint{
+		URL:           ep,
+		SigningRegion: signingRegion,


+1, can we actually check how AWS does it?

marcosuma · 2025-09-29T10:29:42Z

internal/provider/credentials.go

 	sess := session.Must(session.NewSession(&aws.Config{
-		Region:              aws.String(region),
-		Credentials:         credentials.NewStaticCredentials(awsAccessKeyID, awsSecretAccessKey, awsSessionToken),
-		STSRegionalEndpoint: ep,


curious why we don't need this/

marcosuma · 2025-09-29T10:30:16Z

internal/provider/credentials.go

 	return *cfg, nil
 }

+func DeriveSTSRegionFromEndpoint(ep string) string {


any chance we could easily unit test this?

sweet, you've done it- ignore me

carriecwk

lgtm

…on (#3718) * update provider to infer region from sts_endpoint * changelog * fmt * nit * nit * fmt * unit tests * test * nit * nit * nit * improve clarity of comment * use asserts in unit test * use const instead of magic number --------- Co-authored-by: Oriol Arbusi Abadal <oriol.abadal@mongodb.com>

… authentication (#3718)" (#3731) * fix: Fixes STS region resolution when using cross-region authentication (#3718) * update provider to infer region from sts_endpoint * changelog * fmt * nit * nit * fmt * unit tests * test * nit * nit * nit * improve clarity of comment * use asserts in unit test * use const instead of magic number --------- Co-authored-by: Oriol Arbusi Abadal <oriol.abadal@mongodb.com> * rename changelog file --------- Co-authored-by: maastha <122359335+maastha@users.noreply.github.com>

maastha added 3 commits September 26, 2025 16:00

update provider to infer region from sts_endpoint

e05a8ee

changelog

2954903

fmt

ad7c51c

maastha changed the title ~~chore: Updates provider to infer AWS region from sts_endpoint~~ fix: Fixes STS region resolution when using cross-region authentication Sep 26, 2025

github-actions bot added the bug label Sep 26, 2025

maastha added 8 commits September 26, 2025 16:11

nit

c961c9d

nit

c8e32ae

fmt

5ae1c96

unit tests

7e80380

test

915be05

nit

a8254a9

nit

455013a

nit

095e54b

maastha marked this pull request as ready for review September 26, 2025 20:59

maastha requested review from a team as code owners September 26, 2025 20:59

Copilot AI review requested due to automatic review settings September 26, 2025 20:59

Copilot AI reviewed Sep 26, 2025

View reviewed changes

internal/provider/credentials.go Show resolved Hide resolved

lantoli reviewed Sep 29, 2025

View reviewed changes

internal/provider/credentials_test.go Outdated Show resolved Hide resolved

lantoli reviewed Sep 29, 2025

View reviewed changes

internal/provider/credentials_test.go Outdated Show resolved Hide resolved

lantoli approved these changes Sep 29, 2025

View reviewed changes

oarbusi approved these changes Sep 29, 2025

View reviewed changes

manupedrozo approved these changes Sep 29, 2025

View reviewed changes

lantoli reviewed Sep 29, 2025

View reviewed changes

marcosuma approved these changes Sep 29, 2025

View reviewed changes

oarbusi added 3 commits September 29, 2025 14:37

improve clarity of comment

06a0b97

use asserts in unit test

f023a42

use const instead of magic number

537269c

carriecwk approved these changes Sep 29, 2025

View reviewed changes

oarbusi merged commit bcc21de into master Sep 30, 2025
45 checks passed

oarbusi deleted the update-sts-region branch September 30, 2025 07:26

svc-apix-Bot added a commit that referenced this pull request Sep 30, 2025

chore: Updates CHANGELOG.md for #3718

05ed805

oarbusi mentioned this pull request Oct 1, 2025

fix: Backport of "Fixes STS region resolution when using cross-region authentication (#3718)" #3731

Merged

13 tasks

		@@ -0,0 +1,3 @@
		```release-note:bug
		provider: Fixes STS region resolution when using cross-region authentication

fix: Fixes STS region resolution when using cross-region authentication #3718

fix: Fixes STS region resolution when using cross-region authentication #3718

Uh oh!

Conversation

maastha commented Sep 26, 2025 • edited by oarbusi Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change:

Required Checklist:

Further comments

Uh oh!

github-actions bot commented Sep 26, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

maastha commented Sep 26, 2025

Uh oh!

lantoli Sep 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

manupedrozo Sep 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marcosuma left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carriecwk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

maastha commented Sep 26, 2025 •

edited by oarbusi

Loading

lantoli Sep 29, 2025 •

edited

Loading

manupedrozo Sep 29, 2025 •

edited

Loading