From 4b7c8f6a9886082cb61e5e4d46eee13fe7b0daa6 Mon Sep 17 00:00:00 2001
From: Benjamin Morris <sirromb@amazon.com>
Date: Wed, 9 Aug 2023 16:53:06 -0700
Subject: [PATCH] troubleshooting fixes for ALL resource and account exclusion,
 skip config bucket check in CT envs

---
 rdk/rdk.py                                    | 22 ++++++++++-----
 .../configManagedRuleOrganization.json        | 12 +++------
 rdk/template/configRuleOrganization.json      | 27 ++++++++++++-------
 3 files changed, 36 insertions(+), 25 deletions(-)

diff --git a/rdk/rdk.py b/rdk/rdk.py
index 8bb8233..3185840 100644
--- a/rdk/rdk.py
+++ b/rdk/rdk.py
@@ -948,7 +948,11 @@ def init(self):
 
         my_s3 = my_session.client("s3")
 
-        if not config_bucket_exists:
+        if control_tower and not config_bucket_exists:
+            print(
+                "Skipping Config Bucket check since this is part of a Control Tower, which automatically creates a Config bucket."
+            )
+        if not control_tower and not config_bucket_exists:
             # check whether bucket exists if not create config bucket
             response = my_s3.list_buckets()
             bucket_exists = False
@@ -2018,7 +2022,6 @@ def deploy(self):
                     "ParameterKey": "Timeout",
                     "ParameterValue": str(self.args.lambda_timeout),
                 },
-                {"ParameterKey": "ScopeIsAllResources", "ParameterValue": str(bool(source_events == "ALL")).lower()},
             ]
             layers = self.__get_lambda_layers(my_session, self.args, rule_params)
 
@@ -2228,7 +2231,12 @@ def deploy_organization(self):
                 combined_input_parameters.update(optional_parameters_json)
 
             if self.args.excluded_accounts or "ExcludedAccounts" in rule_params:
-                combined_excluded_accounts = set(rule_params.get("ExcludedAccounts", []), self.args.excluded_accounts)
+                combined_excluded_accounts_set = set(
+                    rule_params.get("ExcludedAccounts", []).split(",") + self.args.excluded_accounts
+                )
+                combined_excluded_accounts_str = ",".join(combined_excluded_accounts_set)
+            else:
+                combined_excluded_accounts_str = ""
 
             if "SourceIdentifier" in rule_params:
                 print("Found Managed Rule.")
@@ -2265,7 +2273,7 @@ def deploy_organization(self):
                     },
                     {
                         "ParameterKey": "ExcludedAccounts",
-                        "ParameterValue": combined_excluded_accounts,
+                        "ParameterValue": combined_excluded_accounts_str,
                     },
                 ]
                 my_cfn = my_session.client("cloudformation")
@@ -2411,7 +2419,7 @@ def deploy_organization(self):
                 },
                 {
                     "ParameterKey": "ExcludedAccounts",
-                    "ParameterValue": combined_excluded_accounts,
+                    "ParameterValue": combined_excluded_accounts_str,
                 },
             ]
             layers = self.__get_lambda_layers(my_session, self.args, rule_params)
@@ -3429,7 +3437,7 @@ def __parse_rule_args(self, is_required):
         self.args = get_rule_parser(is_required, self.args.command).parse_args(self.args.command_args, self.args)
 
         max_resource_types = 100
-        if self.args.resource_types and len(self.args.resource_types.split(",") > max_resource_types):
+        if self.args.resource_types and (len(self.args.resource_types.split(",")) > max_resource_types):
             print(f"Number of specified resource types exceeds Config service maximum of {max_resource_types}.")
             sys.exit(1)
 
@@ -3467,7 +3475,7 @@ def __parse_rule_args(self, is_required):
                 print(self.args.input_parameters)
                 input_params_dict = json.loads(self.args.input_parameters, strict=False)
             except Exception as e:
-                print("Failed to parse input parameters.")
+                print("Failed to parse input parameters. Remember to escape double-quotes if using Windows.")
                 sys.exit(1)
 
         if self.args.optional_parameters:
diff --git a/rdk/template/configManagedRuleOrganization.json b/rdk/template/configManagedRuleOrganization.json
index 5d05909..73530fb 100644
--- a/rdk/template/configManagedRuleOrganization.json
+++ b/rdk/template/configManagedRuleOrganization.json
@@ -76,14 +76,12 @@
         }
       ]
     },
-    "ExludedAccountsPresent": {
+    "ExcludedAccountsPresent": {
       "Fn::Not": [
         {
           "Fn::Equals": [
-            {
-              "Ref": "ExcludedAccounts"
-            },
-            "NONE"
+            { "Fn::Join": [",", { "Ref": "ExcludedAccounts" }] },
+            ""
           ]
         }
       ]
@@ -110,9 +108,7 @@
             "Fn::If": [
               "AllResources",
               {
-                "ComplianceResourceTypes": {
-                  "Ref": "SourceEvents"
-                }
+                "Ref": "SourceEvents"
               },
               {
                 "Ref": "AWS::NoValue"
diff --git a/rdk/template/configRuleOrganization.json b/rdk/template/configRuleOrganization.json
index a3772cc..b802c26 100644
--- a/rdk/template/configRuleOrganization.json
+++ b/rdk/template/configRuleOrganization.json
@@ -92,7 +92,7 @@
     },
     "ExcludedAccounts": {
       "Description": "Comma-separated list of account IDs where the Org Config rule should not be deployed to",
-      "Type": "String",
+      "Type": "CommaDelimitedList",
       "Default": ""
     }
   },
@@ -135,7 +135,14 @@
       ]
     },
     "ExcludedAccountsPresent": {
-      "Fn::Not": [{ "Fn::Equals": [{ "Ref": "ExcludedAccounts" }, ""] }]
+      "Fn::Not": [
+        {
+          "Fn::Equals": [
+            { "Fn::Join": [",", { "Ref": "ExcludedAccounts" }] },
+            ""
+          ]
+        }
+      ]
     }
   },
   "Resources": {
@@ -206,7 +213,7 @@
           "ResourceTypesScope": {
             "Fn::If": [
               "AllResources",
-              { "ComplianceResourceTypes": { "Ref": "SourceEvents" } },
+              { "Ref": "SourceEvents" },
               { "Ref": "AWS::NoValue" }
             ]
           },
@@ -227,14 +234,14 @@
               { "Ref": "AWS::NoValue" }
             ]
           }
+        },
+        "ExcludedAccounts": {
+          "Fn::If": [
+            "ExcludedAccountsPresent",
+            { "Ref": "ExcludedAccounts" },
+            { "Ref": "AWS::NoValue" }
+          ]
         }
-      },
-      "ExcludedAccounts": {
-        "Fn::If": [
-          "ExcludedAccountsPresent",
-          { "Ref": "ExcludedAccounts" },
-          { "Ref": "AWS::NoValue" }
-        ]
       }
     },
     "rdkLambdaRole": {