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

Update the AWS STS endpoint to be regional as the method is now regional #528

Merged

Conversation

snorlaX-sleeps
Copy link
Contributor

Required to support AWS GovCloud.
Currently the token method accepts a region to create the signature for logging into an AWS Cluster, which should make it "quicker" on a regional basis.
However the AWS GovCloud partition does not / is not supported by the URL https://sts.amazonaws.com as AWS treats it as a totally separate entity.

This change updates the STS endpoint to match the region provided with the method.

I would be happy to change the PR to update the token method (even the parameters) to pass a gov_cloud named parameter (similar to the region one), then the original https://sts.amazonaws.com could be kept for speed / efficiency for end-users that do not set region and use normal AWS regions - then only set the region in the URL when using the govCloud partition?

List of AWS STS endpoint URLs

Fixes #527
Follows on from #507

…nal. Required to support AWS GovCloud. Fixes ManageIQ#527

Signed-off-by: Adam Robinson <[email protected]>
@cben
Copy link
Collaborator

cben commented Jan 29, 2022

from the doc you linked:

AWS recommends using Regional STS endpoints to reduce latency, build in redundancy, and increase session token validity. Most Regional endpoints are active by default, but you must manually enable endpoints for some Regions, such as Asia Pacific (Hong Kong). You can deactivate STS endpoints for any Regions that are enabled by default if you do not intend to use those Regions.

So sounds like defaulting to per-region endpoint is best practice, not clear that a gov_cloud: false param should imply using https://sts.amazonaws.com. If you think this is useful to control, consider an explicit use_global_sts param?
(I never used this, deferring to you on what's actually best)

Meanwhile this LGTM as-is, merging. (and sorry for my non-responsiveness 🐌)

@cben
Copy link
Collaborator

cben commented Jan 29, 2022

close-cycling to rerun CI

@cben cben closed this Jan 29, 2022
@cben cben reopened this Jan 29, 2022
@cben cben merged commit 1c92f9d into ManageIQ:master Jan 29, 2022
@snorlaX-sleeps
Copy link
Contributor Author

Hey @cben - thanks for getting back to me (I had a local copy of the gem so this was not an issue)

not clear that a gov_cloud: false param should imply using https://sts.amazonaws.com. If you think this is useful to control, consider an explicit use_global_sts param?

The https://sts.amazonaws.com endpoint is regional, that is just a common endpoint for any commercial (non-gov_cloud or certain other regions) to hit without needing to be region specific.
I think they are equivalent, I just didn't want to change pre-written functionality too much if you preferred.
There is a hard partition between commercial AWS accounts and gov_cloud accounts, so they have to use different services, hence the issue

@snorlaX-sleeps snorlaX-sleeps deleted the update_aws_sts_endpoint_to_be_regional branch January 31, 2022 13:00
@agrare
Copy link
Member

agrare commented Jan 31, 2022

Great fix @snorlaX-sleeps thank you

agrare pushed a commit to agrare/kubeclient that referenced this pull request Jun 18, 2024
…dpoint_to_be_regional

Update the AWS STS endpoint to be regional as the method is now regional

(cherry picked from commit 1c92f9d)
agrare added a commit to agrare/kubeclient that referenced this pull request Jun 18, 2024
Added
- Add test coverage for Ruby 3.2 (ManageIQ#615)
- Allow a region when getting a signer for Aws::Sts (ManageIQ#507)
- Update the AWS STS endpoint to be regional as the method is now regional (ManageIQ#528)
- Assume role support for aws eks credentials (ManageIQ#630)

Fixed
- [v4.y] Regenerated expired test TLS certs by running `test/config/update_certs_k0s.rb`.
- [v4.y] Regenerated expired test TLS certs (ManageIQ#611)
- Regenerated expired test TLS certs (ManageIQ#632)

Changed
- Update actions/checkout (ManageIQ#590)
- chore(deps): update actions/checkout action to v4 (ManageIQ#619)
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.

4.9.2 and master do not work with AWS GovCloud due to regional hardcoding
3 participants