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 STS Credentials Provider to use regional endpoint if possible #237

Merged
merged 21 commits into from
May 9, 2024

Conversation

waahm7
Copy link
Contributor

@waahm7 waahm7 commented May 6, 2024

Issue #, if available:
awslabs/mountpoint-s3#861

Description of changes:

  • If we can parse region from environment variable or config file, use a regional endpoint instead of global sts endpoint.
  • Update STSWebIdentity to look for the AWS_REGION variable first as well.

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

@codecov-commenter
Copy link

codecov-commenter commented May 6, 2024

Codecov Report

Attention: Patch coverage is 82.47423% with 17 lines in your changes are missing coverage. Please review.

Project coverage is 80.22%. Comparing base (140652a) to head (c29497d).

Files Patch % Lines
source/credentials_provider_sts.c 81.48% 10 Missing ⚠️
source/credentials_utils.c 78.57% 6 Missing ⚠️
source/credentials_provider_sts_web_identity.c 92.30% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #237      +/-   ##
==========================================
+ Coverage   79.91%   80.22%   +0.30%     
==========================================
  Files          33       33              
  Lines        5921     5958      +37     
==========================================
+ Hits         4732     4780      +48     
+ Misses       1189     1178      -11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@waahm7 waahm7 changed the title WIP | Fix STS Endpoint Fix STS Endpoint to use regional endpoint if available May 6, 2024
@waahm7 waahm7 marked this pull request as ready for review May 8, 2024 18:51
@waahm7 waahm7 changed the title Fix STS Endpoint to use regional endpoint if available Fix STS Credentials Provider to use regional endpoint if possible May 8, 2024
source/credentials_provider_profile.c Show resolved Hide resolved
source/credentials_provider_sts.c Show resolved Hide resolved

int aws_credentials_provider_construct_regional_endpoint(
struct aws_allocator *allocator,
struct aws_byte_buf *out_endpoint,
Copy link
Contributor

Choose a reason for hiding this comment

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

debatable: is it better to hide the buf as impl detail and return string instead? dont have to deal with cleaning buf, worrying if they pass an existing buf, etc...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, updated. That's much better.

@waahm7 waahm7 merged commit ef9cfa1 into main May 9, 2024
29 checks passed
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.

None yet

3 participants