Inconsistent validation of CT enrolled accounts between Deployment Target approaches

[dhutchison]

Describe the bug In a stack_set deployment there are two ways that accounts can be included - by account and by organisational unit.

When an account number is specified which is not enrolled in control tower, it is filtered out during the manifest parsing and no attempt is made to deploy to the account.

When an organisational unit is specified which is part of the AWS organisation but is not enrolled in control tower then an attempt is made to deploy to the non-CT accounts in the OU. This fails as the required trust relationships do not exist.

I think the validation of accounts against control tower enrolment should be consistent between if the account was specified by number or by OU.

To Reproduce

Include in the deployment targets in the manifest an organisational unit which is valid for the AWS Organisation, but includes accounts which have not been enrolled in control tower. (In the example we saw, this was a top level OU, not a nested OU).

The StackSet will be deployed and attempt to include Stack Instances for non-control tower accounts. These instances will fail to deploy.

Expected behavior Accounts that are not enrolled in control tower will not have a deployment attempted to them.

Please complete the following information about the solution:

• [ ] Version: 2.4.0

To get the version of the solution, you can look at the description of the created CloudFormation stack. For example, "(SO0089) - customizations-for-aws-control-tower Solution. Version: v1.0.0". You can also find the version from releases

• [ ] Region: us-east-1, eu-west-1, eu-west-2
• [ ] Was the solution modified from the version published on this repository? No
• [ ] If the answer to the previous question was yes, are the changes available on GitHub?
• [ ] Have you checked your service quotas for the sevices this solution uses?
• [ ] Were there any errors in the CloudWatch Logs?

Screenshots If applicable, add screenshots to help explain your problem (please DO NOT include sensitive information).

Additional context

I think the issue lies in the result of https://github.com/aws-solutions/aws-control-tower-customizations/blob/f9e2921c78a053eaec840c487eeac3bcca459f21/source/src/cfct/manifest/manifest_parser.py#L247

This get_final_account_list method contains this which removes any non-CT accounts from the new_account_list, which contains the accounts which per specified by name/number in the accounts deployment target.

        # Remove account ids from the manifest that is not
        # in the organization or not active
        sanitized_account_list = list(
            set(new_account_list).intersection(set(accounts_in_all_ous))
        )

Where accounts_in_all_ous ultimately comes from get_all_accounts_in_all_nested_ous which uses get_accounts_in_ct_baseline_config_stack_set as the source of the account list.

However, after this the account numbers from the specified OU deployment targets are added.

        # merge account lists manifest account list and
        # accounts under OUs in the manifest
        sanitized_account_list.extend(accounts_in_ou)

As far as I can tell from reading through the code, no validation is performed to restrict accounts_in_ou to only control tower enrolled accounts.

[snebhu3]

@dhutchison , thank you for bringing up the issue. I have gone ahead a recorded a backlog item to discuss with the team.

[meskander-ss]

@snebhu3 it's been almost 2 years since this has been recorded as a backlog item. has it been prioritized?