-
Notifications
You must be signed in to change notification settings - Fork 8
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
Invalid regions us-gov-west-1, us-gov-east-1 fail Control Tower deployment #58
Comments
Hi @iainelder , thanks for reporting, i noticed the initial report. Not sure what you have in mind as a solution though. The deploy method already accepts an argument of regions that respects. So if gov regions should not be used one should construct the argument before calling the deploy method. What else did you have in mind? |
I think this code needs to change. If I've misunderstood anything please let me know. awsapilib/awsapilib/controltower/controltower.py Lines 1236 to 1238 in 0c8432a
The awsapilib takes the regions from the "AWS Services by Region" API and writes all of them into the payload. The The call in Superwerker issue 417 happened in us-west-2. It didn't pass a region list to the So So [
{"Region": "us-west-2", "RegionConfigurationStatus": "ENABLED"},
{"Region": "us-east-1", "RegionConfigurationStatus": "DISABLED"},
...
{"Region": "us-gov-west-1", "RegionConfigurationStatus": "DISABLED"},
{"Region": "us-gov-east-1", "RegionConfigurationStatus": "DISABLED"},
] Control Tower rejects those GovCloud regions I think there's nothing the client can do to stop awsapilib writing them into the payload. I think you can fix it by leaving the GovCloud regions out of the payload. For now that would mean awsapilib doesn't support GovCloud, but that changes nothing since it didn't support it before either. If you want to support GovCloud in the future, you can maybe change the region-handling code to select regions based on on the home partition. |
That makes perfect sense. So what you are saying is that deploying control tower fails even if it has the gov regions set as disabled in the call? That is the part that I missed then, i thought that since they are now supported it should work fine. I think that providing the capability to filter out the gov ones would be the way to go for now, what do you think? |
How does https://github.com/schubergphilis/awsapilib/pull/59/files look like to you @iainelder ? |
For improved readability @costastf I would change the name of the new argument from |
🤣 The negation broke my brain to be honest, i am just doing 120 things at the same time. Thanks of course it makes perfect sense! 🙇 |
@costastf merged #59 and released version 3.1.4. I say we keep this issue open until we confirm that it solves the problem downstream. I think it will be easier to test this as part of superwerker. |
Sure, makes perfect sense. |
See superwerker issue 417 for the original report and analysis.
I cross post it here because I think the solution depends on a change to awsapilib.
If you prefer to discuss it here, I'll copy the relevant details.
The text was updated successfully, but these errors were encountered: