fixing #596 and #597 and adding #600

* can now create organizational units from the manifest file
* fixing #596 and #597

pyproject.toml

@@ -3,7 +3,7 @@
 
 [tool.poetry]
 name = "aws-service-catalog-puppet"
-version = "0.206.0"
+version = "0.207.0"
 description = "Making it easier to deploy ServiceCatalog products"
 classifiers = ["Development Status :: 5 - Production/Stable", "Intended Audience :: Developers", "Programming Language :: Python :: 3", "License :: OSI Approved :: Apache Software License", "Operating System :: OS Independent", "Natural Language :: English"]
 homepage = "https://service-catalog-tools-workshop.com/"

servicecatalog_puppet/commands/manifest.py

@@ -69,7 +69,10 @@ def expand(f, puppet_account_id, regions, single_account, subset=None):
         with betterboto_client.CrossAccountClientContextManager(
             "organizations", org_iam_role_arn, "org-iam-role"
         ) as client:
-            new_manifest = manifest_utils.expand_manifest(manifest, client)
+            conversions, new_manifest = manifest_utils.expand_manifest(manifest, client)
+            new_manifest = manifest_utils.rewrite_organizational_units(
+                new_manifest, conversions, client
+            )
     click.echo("Expanded")
 
     new_manifest = manifest_utils.rewrite_deploy_as_share_to_for_spoke_local_portfolios(

servicecatalog_puppet/commands/task_reference.py

@@ -290,6 +290,17 @@ def generate_complete_task_reference(puppet_account_id, manifest, output_file_pa
                         task_to_add,
                     )
 
+                if section_name == constants.ORGANIZATIONAL_UNITS:
+                    handle_organizational_units(
+                        all_tasks,
+                        all_tasks_task_reference,
+                        item_name,
+                        puppet_account_id,
+                        section_name,
+                        task_reference,
+                        task_to_add,
+                    )
+
     #
     # Second pass - adding get parameters
     #
@@ -565,6 +576,42 @@ def handle_service_control_policies(
         )
 
 
+def handle_organizational_units(
+    all_tasks,
+    all_tasks_task_reference,
+    item_name,
+    puppet_account_id,
+    section_name,
+    task_reference,
+    task_to_add,
+):
+    if not task_to_add.get("parent_ou_id"):
+        # parent may not exist as it wasn't specified
+        parent_path = os.path.dirname(task_to_add.get("path"))
+        if parent_path != task_to_add.get("path"):
+            parent_task_reference = f"{constants.ORGANIZATIONAL_UNITS}_{parent_path.replace('/', '%2F')}_{task_to_add.get('account_id')}_{task_to_add.get('region')}"
+            if not all_tasks.get(parent_task_reference):
+                parent_task_to_add = copy.deepcopy(task_to_add)
+                parent_task_to_add["task_reference"] = parent_task_reference
+                parent_task_to_add["path"] = parent_path
+                if parent_path == "/":
+                    parent_task_to_add["name"] = parent_path
+                else:
+                    parent_task_to_add["name"] = os.path.basename(parent_path)
+                all_tasks[parent_task_reference] = parent_task_to_add
+                handle_organizational_units(
+                    all_tasks,
+                    all_tasks_task_reference,
+                    item_name,
+                    puppet_account_id,
+                    section_name,
+                    parent_task_reference,
+                    parent_task_to_add,
+                )
+            task_to_add["parent_ou_task_ref"] = parent_task_reference
+            task_to_add["dependencies_by_reference"].append(parent_task_reference)
+
+
 def handle_tag_policies(
     all_tasks,
     all_tasks_task_reference,

servicecatalog_puppet/config.py

@@ -100,7 +100,13 @@ def get_puppet_role_path():
 @functools.lru_cache()
 def get_puppet_role_arn(puppet_account_id):
     logger.info("getting puppet_role_arn")
-    return f"arn:{get_partition()}:iam::{puppet_account_id}:role{get_puppet_role_path()}{get_puppet_role_name()}"
+    return get_role_arn(puppet_account_id, get_puppet_role_name())
+
+
+@functools.lru_cache()
+def get_role_arn(puppet_account_id, role_name):
+    logger.info("getting puppet_role_arn")
+    return f"arn:{get_partition()}:iam::{puppet_account_id}:role{get_puppet_role_path()}{role_name}"
 
 
 @functools.lru_cache()
@@ -256,3 +262,7 @@ def get_scheduler_threads_or_processes():
         environmental_variables.SCHEDULER_THREADS_OR_PROCESSES,
         constants.SCHEDULER_THREADS_OR_PROCESSES_DEFAULT,
     )
+
+
+def get_reporting_role_arn(puppet_account_id):
+    return get_role_arn(puppet_account_id, constants.REPORTING_ROLE_NAME)

servicecatalog_puppet/constants.py

@@ -44,6 +44,9 @@
 TAG_POLICIES = "tag-policies"
 TAG_POLICY = "tag-policy"
 
+ORGANIZATIONAL_UNIT = "organizational-unit"
+ORGANIZATIONAL_UNITS = "organizational-units"
+
 SIMULATE_POLICIES = "simulate-policies"
 SIMULATE_POLICY = "simulate-policy"
 
@@ -174,6 +177,7 @@
     (SERVICE_CONTROL_POLICY, SERVICE_CONTROL_POLICIES),
     (SIMULATE_POLICY, SIMULATE_POLICIES),
     (TAG_POLICY, TAG_POLICIES),
+    (ORGANIZATIONAL_UNIT, ORGANIZATIONAL_UNITS),
 ]
 
 SECTION_NAME_SINGULAR_AND_PLURAL_LIST_THAT_SUPPORTS_PARAMETERS = [
@@ -374,3 +378,6 @@
 """
 
 SCHEDULER_THREADS_OR_PROCESSES_DEFAULT = "threads"
+
+
+REPORTING_ROLE_NAME = "PuppetRoleForReporting"

servicecatalog_puppet/constants_unit_test.py

@@ -140,6 +140,7 @@ def test_constants_values():
         (constants.SERVICE_CONTROL_POLICY, constants.SERVICE_CONTROL_POLICIES),
         (constants.SIMULATE_POLICY, constants.SIMULATE_POLICIES),
         (constants.TAG_POLICY, constants.TAG_POLICIES),
+        (constants.ORGANIZATIONAL_UNIT, constants.ORGANIZATIONAL_UNITS),
     ]
     assert constants.SECTION_NAME_SINGULAR_AND_PLURAL_LIST_THAT_SUPPORTS_PARAMETERS == [
         (constants.LAUNCH, constants.LAUNCHES),

servicecatalog_puppet/manifest_utils.py

@@ -166,6 +166,7 @@ def load(f, puppet_account_id):
 def expand_manifest(manifest, client):
     new_manifest = deepcopy(manifest)
     temp_accounts = []
+    conversions = dict()
 
     logger.info("Starting the expand")
 
@@ -183,9 +184,11 @@ def expand_manifest(manifest, client):
             ou = account.get("ou")
             logger.info("Found an ou: {}".format(ou))
             if ou.startswith("/"):
-                temp_accounts += expand_path(account, client, manifest)
-            else:
-                temp_accounts += expand_ou(account, client, manifest)
+                ou = client.convert_path_to_ou(account.get("ou"))
+                conversions[account.get("ou")] = ou
+                account["ou_name"] = account["ou"]
+                account["ou"] = ou
+            temp_accounts += expand_ou(account, client, manifest)
 
     for parameter_name, parameter_details in new_manifest.get("parameters", {}).items():
         if parameter_details.get("macro"):
@@ -281,7 +284,7 @@ def expand_manifest(manifest, client):
                     parameter_details["default"] = result
                     del parameter_details["macro"]
 
-    return new_manifest
+    return conversions, new_manifest
 
 
 def rewrite_deploy_as_share_to_for_spoke_local_portfolios(manifest):
@@ -755,6 +758,7 @@ def get_tasks_for(
             "service-control-policies": "apply_to",
             "tag-policies": "apply_to",
             "simulate-policies": "simulate_for",
+            constants.ORGANIZATIONAL_UNITS: "create_in",
         }.get(section_name)
 
         if (
@@ -884,6 +888,13 @@ def get_tasks_for(
                 context_entries=item.get("context_entries", []),
                 resource_handling_option=item.get("resource_handling_option", ""),
             ),
+            constants.ORGANIZATIONAL_UNITS: dict(
+                organizational_unit_name=item_name,
+                path=item.get("path"),
+                parent_ou_id=item.get("parent_ou_id"),
+                name=item.get("name"),
+                tags=item.get("tags"),
+            ),
         }.get(section_name)
 
         common_parameters.update(
@@ -1014,6 +1025,7 @@ def get_tasks_for(
                 "service-control-policies": dict(account_id=account_id, ou_name="",),
                 "tag-policies": dict(account_id=account_id, ou_name="",),
                 constants.SIMULATE_POLICIES: dict(account_id=account_id,),
+                constants.ORGANIZATIONAL_UNITS: dict(account_id=account_id,),
             }.get(section_name)
 
             if isinstance(regions, str):
@@ -1513,3 +1525,16 @@ def parse_conditions(manifest):
                             f"Removed {item_name} from {section_name} because condition ({item.get('condition')}) evaluated to false"
                         )
     return manifest
+
+
+def rewrite_organizational_units(manifest, conversions, client):
+    for item_name, item in manifest.get(constants.ORGANIZATIONAL_UNITS, {}).items():
+        path = item.get("path")
+        if not item.get("parent_ou_id"):
+            parent_path = os.path.dirname(path)
+            if conversions.get(parent_path):
+                item["parent_ou_id"] = conversions.get(parent_path)
+        if not item.get("name"):
+            item["name"] = os.path.basename(path)
+
+    return manifest

servicecatalog_puppet/manifest_utils_for_expand_unit_test.py

@@ -111,7 +111,7 @@ def describe_account_side_effect(AccountId):
         )
 
         # exercise
-        actual_results = self.sut(expanded_manifest, self.client_mock)
+        _, actual_results = self.sut(expanded_manifest, self.client_mock)
 
         # verify
         self.assertDictEqual(expected_results, actual_results)
@@ -182,7 +182,7 @@ def list_children_nested_side_effect(ParentId, ChildType):
         self.client_mock.convert_path_to_ou = MagicMock(return_value="ou-aaaa-bbbbbbbb")
 
         # exercise
-        actual_results = self.sut(expanded_manifest, self.client_mock)
+        _, actual_results = self.sut(expanded_manifest, self.client_mock)
 
         # verify
         self.assertDictEqual(expected_results, actual_results)

servicecatalog_puppet/servicecatalog-puppet-scp-master.template.yaml

@@ -50,6 +50,9 @@ Resources:
                   - organizations:UpdatePolicy
 
                   - organizations:TagResource
+
+                  - organizations:DescribeOrganizationalUnit
+                  - organizations:CreateOrganizationalUnit
                 Resource: "*"
 
       AssumeRolePolicyDocument:

servicecatalog_puppet/template_builder/hub/bootstrap.py

@@ -161,7 +161,7 @@ def get_template(
         )
     )
 
-    template.add_resource(
+    log_bucket = template.add_resource(
         s3.Bucket(
             "LogStore",
             BucketName=t.Sub("sc-puppet-log-store-${AWS::AccountId}"),
@@ -233,7 +233,7 @@ def get_template(
             Value=t.Ref(puppet_role_path_template_parameter),
         )
     )
-    share_accept_function_role = template.add_resource(
+    template.add_resource(
         iam.Role(
             "ShareAcceptFunctionRole",
             RoleName="ShareAcceptFunctionRole",
@@ -273,7 +273,7 @@ def get_template(
         )
     )
 
-    provisioning_role = template.add_resource(
+    template.add_resource(
         iam.Role(
             "ProvisioningRole",
             RoleName="PuppetProvisioningRole",
@@ -302,7 +302,7 @@ def get_template(
         )
     )
 
-    cloud_formation_deploy_role = template.add_resource(
+    template.add_resource(
         iam.Role(
             "CloudFormationDeployRole",
             RoleName="CloudFormationDeployRole",
@@ -331,7 +331,7 @@ def get_template(
         )
     )
 
-    pipeline_role = template.add_resource(
+    template.add_resource(
         iam.Role(
             "PipelineRole",
             RoleName="PuppetCodePipelineRole",
@@ -355,7 +355,7 @@ def get_template(
         )
     )
 
-    source_role = template.add_resource(
+    template.add_resource(
         iam.Role(
             "SourceRole",
             RoleName="PuppetSourceRole",
@@ -386,7 +386,50 @@ def get_template(
         )
     )
 
-    dry_run_notification_topic = template.add_resource(
+    template.add_resource(
+        iam.Role(
+            constants.REPORTING_ROLE_NAME,
+            RoleName=constants.REPORTING_ROLE_NAME,
+            MaxSessionDuration=43200,
+            AssumeRolePolicyDocument={
+                "Version": "2012-10-17",
+                "Statement": [
+                    {
+                        "Action": ["sts:AssumeRole"],
+                        "Effect": "Allow",
+                        "Principal": {
+                            "AWS": {
+                                "Fn::Sub": "arn:${AWS::Partition}:iam::${AWS::AccountId}:root"
+                            }
+                        },
+                    },
+                ],
+            },
+            Policies=[
+                iam.Policy(
+                    PolicyName="ReportingActions",
+                    PolicyDocument={
+                        "Version": "2012-10-17",
+                        "Statement": [
+                            {
+                                "Action": ["s3:PutObject"],
+                                "Resource": t.Sub("${LogStore.Arn}/*"),
+                                "Effect": "Allow",
+                            },
+                            {
+                                "Action": ["cloudwatch:PutMetricData"],
+                                "Resource": "*",
+                                "Effect": "Allow",
+                            },
+                        ],
+                    },
+                )
+            ],
+            Path=t.Ref(puppet_role_path_template_parameter),
+        )
+    )
+
+    template.add_resource(
         sns.Topic(
             "DryRunNotificationTopic",
             DisplayName="service-catalog-puppet-dry-run-approvals",

servicecatalog_puppet/waluigi/processes/topological_generations.py

@@ -294,7 +294,7 @@ def scheduler_task(
 def on_task_processing_time(task_processing_time_queue, complete_event):
     with betterboto_client.CrossAccountClientContextManager(
         "cloudwatch",
-        config.get_puppet_role_arn(config.get_executor_account_id()),
+        config.get_reporting_role_arn(config.get_executor_account_id()),
         "cloudwatch-puppethub",
     ) as cloudwatch:
         while not complete_event.is_set():
@@ -349,7 +349,7 @@ def on_task_trace(task_trace_queue, complete_event, puppet_account_id, execution
     bucket = f"sc-puppet-log-store-{puppet_account_id}"
     key_prefix = f"{os.getenv('CODEBUILD_BUILD_ID', f'local/{os.getenv(environmental_variables.CACHE_INVALIDATOR)}')}/traces"
     with betterboto_client.CrossAccountClientContextManager(
-        "s3", config.get_puppet_role_arn(config.get_executor_account_id()), "s3",
+        "s3", config.get_reporting_role_arn(config.get_executor_account_id()), "s3",
     ) as s3:
         while not complete_event.is_set():
             # while True:

servicecatalog_puppet/waluigi/threads/topological_generations.py

@@ -297,7 +297,7 @@ def scheduler_task(
 def on_task_processing_time(task_processing_time_queue, complete_event):
     with betterboto_client.CrossAccountClientContextManager(
         "cloudwatch",
-        config.get_puppet_role_arn(config.get_executor_account_id()),
+        config.get_reporting_role_arn(config.get_executor_account_id()),
         "cloudwatch-puppethub",
     ) as cloudwatch:
         while not complete_event.is_set():
@@ -352,7 +352,7 @@ def on_task_trace(task_trace_queue, complete_event, puppet_account_id, execution
     bucket = f"sc-puppet-log-store-{puppet_account_id}"
     key_prefix = f"{os.getenv('CODEBUILD_BUILD_ID', f'local/{os.getenv(environmental_variables.CACHE_INVALIDATOR)}')}/traces"
     with betterboto_client.CrossAccountClientContextManager(
-        "s3", config.get_puppet_role_arn(config.get_executor_account_id()), "s3",
+        "s3", config.get_reporting_role_arn(config.get_executor_account_id()), "s3",
     ) as s3:
         while not complete_event.is_set():
             time.sleep(0.1)