aws-solutions / aws-control-tower-customizations

The Customizations for AWS Control Tower solution combines AWS Control Tower and other highly-available, trusted AWS services to help customers more quickly set up a secure, multi-account AWS environment using AWS best practices.
https://docs.aws.amazon.com/controltower/latest/userguide/cfct-overview.html
Apache License 2.0
360 stars 205 forks source link

manifest_parser.py account list logic error #45

Closed drew-marumoto closed 3 years ago

drew-marumoto commented 3 years ago

In manifest_parser.py, the get_final_account_list() method has a logic error. the statement below is checking for name.lower() in the string key.lower(). the problem arises when you have an account name that is a subset of another account name. I have one account named "aws-ct" and one account named "aws-ct-master". when I specify "aws-ct" as a deploy_to_account, the logic in the statement below matches both "aws-ct" and "aws-ct-master" which is deploying the resource to both accounts, even though only the "aws-ct" account is listed in manifest.yaml. see the snipped below from my cloudtrail logs that show the name_to_account_map object.

    if name_list:
        # convert OU Name to OU IDs
        for name in name_list:
            name_account = [value for key, value in
                            name_to_account_map.items()
                            if name.lower() in key.lower()]
            self.logger.info("%%%%%%% Name {} -  Account {}"
                             .format(name, name_account))
            new_account_list.extend(name_account)

2020-12-10T09:31:29.382-08:00 | {"time_stamp": "2020-12-10 17:31:28,252","log_level": "INFO","log_message": Print Account Name > Account Mapping}
-- | --
  | 2020-12-10T09:31:29.382-08:00 |  
  | 2020-12-10T09:31:29.382-08:00 | {"time_stamp": "2020-12-10 17:31:28,253","log_level": "INFO","log_message": {
  | 2020-12-10T09:31:29.382-08:00 | "aftest3": "",
  | 2020-12-10T09:31:29.382-08:00 | "Audit": "",
  | 2020-12-10T09:31:29.382-08:00 | "aftest2": "",
  | 2020-12-10T09:31:29.382-08:00 | "Log archive": "",
  | 2020-12-10T09:31:29.382-08:00 | "account-factory-new-acct-lab": "",
  | 2020-12-10T09:31:29.382-08:00 | "route53": "",
  | 2020-12-10T09:31:29.382-08:00 | "aws-ct": "",
  | 2020-12-10T09:31:29.382-08:00 | "aws-ct-master": ""
  | 2020-12-10T09:31:29.382-08:00 | }}```
groverlalit commented 3 years ago

@budgreen619 Thanks for bringing this to our attention. We have added this to our backlog and plan to fix it in the next release.

groverlalit commented 3 years ago

This change was released in v2.1.0.