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

Filter out gov regions by default. #59

Merged
merged 2 commits into from
Nov 28, 2023
Merged

Filter out gov regions by default. #59

merged 2 commits into from
Nov 28, 2023

Conversation

costastf
Copy link
Collaborator

No description provided.

Copy link

@aperov9 aperov9 left a comment

Choose a reason for hiding this comment

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

LGTM

@iainelder-smg
Copy link

iainelder-smg commented Nov 27, 2023

I would make it even simpler. No parameters. Always remove the GovCloud regions.

return [
    entry.get('id', '').split(':')[1]
    for entry in response.json().get('prices')
    if entry.get('id').startswith('controltower') and not "us-gov-" in entry.get('id')
]

Each AWS account is scoped to one partition.

You cannot use IAM credentials from one partition to interact with resources in a different partition.

Those features mean an AWS account created in the aws partition will never be able to use a region in the aws-us-gov partition.

Similarly an AWS account created in the aws-us-gov partition will never be able to use a region in the aws partition.

So enable_gov_regions=True will never work. Since it returns the regions across all partitions, it is going to fail in the aws partition and in the aws-us-gov partition.

So for now just make it work again in the aws partition.


Later if you want to make it work in either partition you could add a check for the current partition and then build a payload that contains just the regions that belong to that partition.

You can check the current partition using STS.GetCallerIdentity.

Session().client("sts").get_caller_identity()["Arn"].split(":")[1]

That will return one of aws, aws-us-gov, or aws-cn depending on the partition you are in.

That's how we detect the partition in botocove. (See connelldave/botocove#63.)

caller_id = self.sts_client.get_caller_identity()
self.host_account_id = caller_id["Account"]
self.host_account_partition = caller_id["Arn"].split(":")[1]

@iainelder
Copy link

I need to get better at using my personal account for personal stuff 😂

@costastf
Copy link
Collaborator Author

Assuming you agree with @iainelder @aperov9 I have made the change. If you guys are ok, i can release a patch version.

Copy link

@iainelder iainelder left a comment

Choose a reason for hiding this comment

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

Looks good!

@costastf costastf merged commit 262c699 into main Nov 28, 2023
4 checks passed
@iainelder
Copy link

@costastf , let us know when you publish a new patch version so that we can use it downstream in superwerker :-)

@costastf
Copy link
Collaborator Author

v3.1.4 is released. Thanks!

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 participants