aws-ia / terraform-aws-control_tower_account_factory

AWS Control Tower Account Factory
Apache License 2.0
605 stars 386 forks source link

aft-invoke-customizations with nested ou 1.9.1 #332

Closed oscarDovao closed 1 year ago

oscarDovao commented 1 year ago

Terraform Version & Prov:

AFT Version: 1.9.1

Terraform Version & Provider Versions Please provide the outputs of terraform version and terraform providers from within your AFT environment

terraform version

1.3.6

terraform providers

Initializing provider plugins...
- Finding hashicorp/aws versions matching ">= 4.30.0"...
- Finding latest version of hashicorp/time...
- Installing hashicorp/aws v4.60.0...
- Installed hashicorp/aws v4.60.0 (signed by HashiCorp)
- Installing hashicorp/time v0.9.1...
- Installed hashicorp/time v0.9.1 (signed by HashiCorp)

Bug Description The aft-invoke-customizations state machine doesn't trigger account customisation pipelines within nested OUs

To Reproduce Steps to reproduce the behavior:

  1. Provisioned new account with AFT
  2. Execute aft-invoke-customizations with input as such
    {
    "include": [
    {
      "type": "ous",
      "target_value": [ "OU Name (ou-id-1234)"]
    }
    ]
    }

Expected behavior Customisation pipeline/s for the Account/s under the nested OU specified are triggered

Related Logs No errors observed

Additional context This seemed to be working on previous versions https://github.com/aws-ia/terraform-aws-control_tower_account_factory/issues/280

balltrev commented 1 year ago

Hey @oscarDovao thanks for reaching out! We're unable to reproduce the issue in our testing environments, I would recommend that you reach out to AWS Premium Support to help further deep dive the issue in your environment.

stumins commented 1 year ago

Closing this issue as further investigation by AWS Support is needed. Please feel free to open a new issue if you identify an issue with AFT while working with Support.

electrofelix commented 1 year ago

Recently bumped into this and it looks like the following potentially causes an issue where the same name is used for multiple OUs under different tree structures https://github.com/aws-ia/terraform-aws-control_tower_account_factory/blob/60ee77b6a90e4bc6a96a60c8d8105d71ba5d3543/sources/aft-lambda-layer/aft_common/organizations.py#L170

I've logged a support case as well, but it seems to me that if you have multiple OUs under different branches, constructing the map using the name as the key with a single value rather means the following will often fail to match because the id stored in the map could have been overwritten multiple times https://github.com/aws-ia/terraform-aws-control_tower_account_factory/blob/60ee77b6a90e4bc6a96a60c8d8105d71ba5d3543/sources/aft-lambda-layer/aft_common/organizations.py#L181-L182

I'd suggest something like:

        # Convert list of OUs to name->id map for constant time lookups
        for ou in ous:
            try:
                ou_map[ou["Name"]].add(ou["Id"])
            except KeyError:
                ou_map[ou["Name"]] = {ou["Id"]}

And then update the if check to something like:

            if nested_parsed is not None:  # Nested OU pattern matched!
                target_name, target_id = nested_parsed
                if target_id in ou_map[target_name]:
                    matched_ou_ids.append(target_id)
            else:
                if target_name in ou_map:
                    matched_ou_ids.append(ou_map[target_name][0])