aws-samples / aws-security-reference-architecture-examples

Example solutions demonstrating how to implement patterns within the AWS Security Reference Architecture guide using CloudFormation (including Customizations for AWS Control Tower) and Terraform.
Other
977 stars 245 forks source link

[BUG] Incomplete set of regions collected in get_customer_control_tower_regions function. #142

Closed kkvinjam closed 2 months ago

kkvinjam commented 1 year ago

Community Note

Describe the bug

The current logic do not consistently pick all control tower regions. It appears the first account in the list is picked and collect all regions for that account. IHAC, with few accounts in EnrollmentFailed state, resulting in picking partial regions. This resulted in below failure.

"Traceback (most recent call last):\n File \"/var/task/crhelper/resource_helper.py\", line 204, in _wrap_function\n self.PhysicalResourceId = func(self._event, self._context) if func else ''\n File \"/var/task/app.py\", line 399, in create_update_event\n ssm_data3 = get_customer_control_tower_regions_ssm_parameter_info(ssm_data2[\"helper\"][\"HomeRegion\"], path=SRA_REGIONS_SSM_PATH)\n File \"/var/task/app.py\", line 281, in get_customer_control_tower_regions_ssm_parameter_info\n customer_regions_without_home_region.remove(home_region)\nValueError: list.remove(x): x not in list"

To Reproduce

This is environment-specific and hard to reproduce. While this is not frequent, possible use case for customers operating in Control Tower environments.

Expected behavior

A clear and concise description of what you expected to happen.

Screenshots

If applicable, add screenshots to help explain your problem.

Deployment Environment (please complete the following information)

Additional context

Add any other context about the problem here.

TaurusAlpha commented 11 months ago

@liamschn Forked solution: https://github.com/aws-samples/aws-security-reference-architecture-examples/compare/main...TaurusAlpha:aws-security-reference-architecture-examples:update-region-common-prerequisite Fork also includes some minor logic improvement in common-prerequisites.

mlfulleraws commented 2 months ago

issue reported has been resolved, thank you for reporting!