Fixed dependency order issue with imported-portfolios and fixed issue…

… with associations not being deleted.
awslabs · Jun 30, 2023 · 0463001 · 0463001
1 parent fbce132
commit 0463001
Show file tree

Hide file tree

Showing 7 changed files with 107 additions and 72 deletions.
diff --git a/pyproject.toml b/pyproject.toml
@@ -3,7 +3,7 @@
 
 [tool.poetry]
 name = "aws-service-catalog-puppet"
-version = "0.232.0"
+version = "0.232.1"
 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/"

diff --git a/servicecatalog_puppet/commands/task_reference_helpers/generators/imported_portfolios.py b/servicecatalog_puppet/commands/task_reference_helpers/generators/imported_portfolios.py
@@ -20,10 +20,14 @@ def get_imported_portfolio_common_args(
 
 
 def add_puppet_associations_for_when_not_sharing_with_puppet_account(
-    all_tasks, all_tasks_task_reference, puppet_account_id, task_to_add
+    all_tasks,
+    all_tasks_task_reference,
+    puppet_account_id,
+    task_to_add,
+    hub_portfolio_ref,
+    dependencies_for_import_portfolios_sub_tasks,
 ):
     # GET THE HUB PORTFOLIO
-    hub_portfolio_ref = f"{constants.PORTFOLIO_LOCAL}-{puppet_account_id}-{task_to_add.get('region')}-{task_to_add.get('portfolio')}"
     if not all_tasks.get(hub_portfolio_ref):
         all_tasks[hub_portfolio_ref] = dict(
             puppet_account_id=puppet_account_id,
@@ -81,6 +85,47 @@ def add_puppet_associations_for_when_not_sharing_with_puppet_account(
         task_to_add.get("manifest_account_ids")
     )
 
+    # CREATE SPOKE ASSOCIATIONS
+    spoke_portfolio_puppet_association_ref = f"{constants.PORTFOLIO_PUPPET_ROLE_ASSOCIATION}-{task_to_add.get('account_id')}-{task_to_add.get('region')}-{task_to_add.get('portfolio')}"
+    if not all_tasks.get(spoke_portfolio_puppet_association_ref):
+        all_tasks[spoke_portfolio_puppet_association_ref] = dict(
+            puppet_account_id=puppet_account_id,
+            task_reference=spoke_portfolio_puppet_association_ref,
+            portfolio_task_reference=hub_portfolio_ref,
+            dependencies_by_reference=[hub_portfolio_ref, constants.CREATE_POLICIES,]
+            + dependencies_for_import_portfolios_sub_tasks,
+            account_id=task_to_add.get("account_id"),
+            region=task_to_add.get("region"),
+            portfolio=task_to_add.get("portfolio"),
+            execution=task_to_add.get("execution"),
+            section_name=constants.PORTFOLIO_PUPPET_ROLE_ASSOCIATION,
+            manifest_section_names=dict(),
+            manifest_item_names=dict(),
+            manifest_account_ids=dict(),
+        )
+    all_tasks[spoke_portfolio_puppet_association_ref]["manifest_section_names"].update(
+        task_to_add.get("manifest_section_names")
+    )
+    all_tasks[spoke_portfolio_puppet_association_ref]["manifest_item_names"].update(
+        task_to_add.get("manifest_item_names")
+    )
+    all_tasks[spoke_portfolio_puppet_association_ref]["manifest_account_ids"].update(
+        task_to_add.get("manifest_account_ids")
+    )
+
+    return hub_portfolio_ref
+
+
+def setup_sharing_for_portfolio(
+    all_tasks,
+    all_tasks_task_reference,
+    item_name,
+    puppet_account_id,
+    section_name,
+    task_reference,
+    task_to_add,
+    hub_portfolio_ref,
+):
     # SHARE THE PORTFOLIO
     sharing_mode = task_to_add.get(
         "sharing_mode", config.get_global_sharing_mode_default()
@@ -134,38 +179,7 @@ def add_puppet_associations_for_when_not_sharing_with_puppet_account(
     all_tasks[share_and_accept_ref]["manifest_account_ids"].update(
         task_to_add.get("manifest_account_ids")
     )
-
-    # CREATE SPOKE ASSOCIATIONS
-    spoke_portfolio_puppet_association_ref = f"{constants.PORTFOLIO_PUPPET_ROLE_ASSOCIATION}-{task_to_add.get('account_id')}-{task_to_add.get('region')}-{task_to_add.get('portfolio')}"
-    if not all_tasks.get(spoke_portfolio_puppet_association_ref):
-        all_tasks[spoke_portfolio_puppet_association_ref] = dict(
-            puppet_account_id=puppet_account_id,
-            task_reference=spoke_portfolio_puppet_association_ref,
-            portfolio_task_reference=hub_portfolio_ref,
-            dependencies_by_reference=[hub_portfolio_ref, constants.CREATE_POLICIES,],
-            account_id=task_to_add.get("account_id"),
-            region=task_to_add.get("region"),
-            portfolio=task_to_add.get("portfolio"),
-            execution=task_to_add.get("execution"),
-            section_name=constants.PORTFOLIO_PUPPET_ROLE_ASSOCIATION,
-            manifest_section_names=dict(),
-            manifest_item_names=dict(),
-            manifest_account_ids=dict(),
-        )
-    all_tasks[spoke_portfolio_puppet_association_ref]["manifest_section_names"].update(
-        task_to_add.get("manifest_section_names")
-    )
-    all_tasks[spoke_portfolio_puppet_association_ref]["manifest_item_names"].update(
-        task_to_add.get("manifest_item_names")
-    )
-    all_tasks[spoke_portfolio_puppet_association_ref]["manifest_account_ids"].update(
-        task_to_add.get("manifest_account_ids")
-    )
-
-    return hub_portfolio_ref
-
-
-#             imported_portfolio_name=item_name,
+    return share_and_accept_ref
 
 
 def handle_imported_portfolios(
@@ -180,12 +194,10 @@ def handle_imported_portfolios(
     is_sharing_with_puppet_account = task_to_add.get("account_id") == puppet_account_id
     #
     if task_to_add.get("status") == constants.TERMINATED:
-        deps = list()
         # DELETE THE ASSOCIATION IF IT EXISTS
         if task_to_add.get("associations"):
             shared_ref = f"{section_name}-{item_name}-{task_to_add.get('account_id')}-{task_to_add.get('region')}"
             ref = f"portfolio_associations-{shared_ref}"
-            deps.append(ref)
             if not all_tasks.get(ref):
                 all_tasks[ref] = dict(
                     status=task_to_add.get("status"),
@@ -215,23 +227,44 @@ def handle_imported_portfolios(
                 all_tasks_task_reference
             ]  # DO NOT NEED THIS TASK TO DO ANYTHING AT THE MOMENT
     else:
-        dependencies_for_constraints = list()
+        dependencies_for_import_portfolios_sub_tasks = list()
         if is_sharing_with_puppet_account:
             target_portfolio_ref = add_puppet_associations_for_when_sharing_with_puppet_account(
                 all_tasks, all_tasks_task_reference, puppet_account_id, task_to_add
             )
 
         else:
+            hub_portfolio_ref = f"{constants.PORTFOLIO_LOCAL}-{puppet_account_id}-{task_to_add.get('region')}-{task_to_add.get('portfolio')}"
+            share_and_accept_ref = setup_sharing_for_portfolio(
+                all_tasks,
+                all_tasks_task_reference,
+                item_name,
+                puppet_account_id,
+                section_name,
+                task_reference,
+                task_to_add,
+                hub_portfolio_ref,
+            )
+            dependencies_for_import_portfolios_sub_tasks.append(share_and_accept_ref)
             target_portfolio_ref = add_puppet_associations_for_when_not_sharing_with_puppet_account(
-                all_tasks, all_tasks_task_reference, puppet_account_id, task_to_add
+                all_tasks,
+                all_tasks_task_reference,
+                puppet_account_id,
+                task_to_add,
+                hub_portfolio_ref,
+                dependencies_for_import_portfolios_sub_tasks,
             )
 
+        # need to add the sharing tasks to the dependencies
         if task_to_add.get("associations"):
             shared_ref = f"{section_name}-{item_name}-{task_to_add.get('account_id')}-{task_to_add.get('region')}"
             ref = f"portfolio_associations-{shared_ref}"
             all_tasks[ref] = dict(
                 **get_imported_portfolio_common_args(
-                    task_to_add, target_portfolio_ref, [constants.CREATE_POLICIES],
+                    task_to_add,
+                    target_portfolio_ref,
+                    [constants.CREATE_POLICIES]
+                    + dependencies_for_import_portfolios_sub_tasks,
                 ),
                 task_reference=ref,
                 spoke_local_portfolio_name=item_name,
@@ -255,7 +288,8 @@ def handle_imported_portfolios(
                 region=task_to_add.get("region"),
                 portfolio=task_to_add.get("portfolio"),
                 execution=task_to_add.get("execution"),
-                dependencies_by_reference=[constants.CREATE_POLICIES],
+                dependencies_by_reference=[constants.CREATE_POLICIES]
+                + dependencies_for_import_portfolios_sub_tasks,
                 task_reference=ref,
                 section_name=constants.PORTFOLIO_CONSTRAINTS_LAUNCH,
                 spoke_local_portfolio_name=item_name,
@@ -283,7 +317,8 @@ def handle_imported_portfolios(
                 region=task_to_add.get("region"),
                 portfolio=task_to_add.get("portfolio"),
                 execution=task_to_add.get("execution"),
-                dependencies_by_reference=[constants.CREATE_POLICIES],
+                dependencies_by_reference=[constants.CREATE_POLICIES]
+                + dependencies_for_import_portfolios_sub_tasks,
                 task_reference=ref,
                 section_name=constants.PORTFOLIO_CONSTRAINTS_RESOURCE_UPDATE,
                 resource_update_constraints=task_to_add.get(
@@ -338,27 +373,3 @@ def add_puppet_associations_for_when_sharing_with_puppet_account(
         task_to_add.get("manifest_account_ids")
     )
     return spoke_portfolio_puppet_association_ref
-
-
-#
-# if task_to_add.get("resource_update_constraints"):
-#     shared_ref = f"{section_name}-{item_name}-{task_to_add.get('account_id')}-{task_to_add.get('region')}"
-#     ref = f"resource_update_constraints-{shared_ref}"
-#     all_tasks[ref] = dict(
-#         **get_imported_portfolio_common_args(
-#             task_to_add, all_tasks_task_reference, dependencies_for_constraints,
-#         ),
-#         task_reference=ref,
-#         section_name=constants.PORTFOLIO_CONSTRAINTS_RESOURCE_UPDATE,
-#         imported_portfolio_name=item_name,
-#         resource_update_constraints=task_to_add["resource_update_constraints"],
-#         portfolio_get_all_products_and_their_versions_ref=spoke_portfolio_all_products_and_versions_after_ref,
-#         manifest_section_names=dict(
-#             **task_to_add.get("manifest_section_names")
-#         ),
-#         manifest_item_names=dict(**task_to_add.get("manifest_item_names")),
-#         manifest_account_ids=dict(**task_to_add.get("manifest_account_ids")),
-#     )
-
-
-#
diff --git a/servicecatalog_puppet/commands/task_reference_helpers/generators/imported_portfolios_test.py b/servicecatalog_puppet/commands/task_reference_helpers/generators/imported_portfolios_test.py
@@ -164,6 +164,7 @@ def test_for_ous(self):
                 "dependencies_by_reference": [
                     f"portfolio-local-{puppet_account_id}-{region}-{portfolio}",
                     "create-policies",
+                    f"portfolio_share_and_accept-ou-do8d-me7f39on-{region}-{portfolio}",
                 ],
                 "execution": "hub",
                 "manifest_account_ids": {account_id: True},
@@ -188,6 +189,7 @@ def test_for_ous(self):
                 "dependencies_by_reference": [
                     f"portfolio-local-{puppet_account_id}-{region}-{portfolio}",
                     "create-policies",
+                    f"portfolio_share_and_accept-ou-do8d-me7f39on-{region}-{portfolio}",
                 ],
                 "execution": "hub",
                 "spoke_local_portfolio_name": item_name,
@@ -348,7 +350,10 @@ def test_for_was_spoke_local_portfolio(self):
             {
                 "spoke_local_portfolio_name": item_name,
                 "account_id": account_id,
-                "dependencies_by_reference": ["create-policies"],
+                "dependencies_by_reference": [
+                    "create-policies",
+                    f"portfolio_share_and_accept-ou-do8d-me7f39on-{region}-{portfolio}",
+                ],
                 "execution": "hub",
                 "launch_constraints": "thisisignored",
                 "manifest_account_ids": {account_id: True},
@@ -367,7 +372,10 @@ def test_for_was_spoke_local_portfolio(self):
         self.assertEqual(
             {
                 "account_id": account_id,
-                "dependencies_by_reference": ["create-policies",],
+                "dependencies_by_reference": [
+                    "create-policies",
+                    f"portfolio_share_and_accept-ou-do8d-me7f39on-{region}-{portfolio}",
+                ],
                 "resource_update_constraints": "thisisignored",
                 "execution": "hub",
                 "manifest_account_ids": {account_id: True},

diff --git a/servicecatalog_puppet/workflow/portfolio/associations/association_utils.py b/servicecatalog_puppet/workflow/portfolio/associations/association_utils.py
@@ -0,0 +1,9 @@
+#   Copyright 2023 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+#   SPDX-License-Identifier: Apache-2.0
+from servicecatalog_puppet import utils
+
+
+def generate_stack_name_for_associations_by_item_name(item_name):
+    return (
+        f"associations-v2-for-{utils.slugify_for_cloudformation_stack_name(item_name)}"
+    )
diff --git a/...pet/workflow/portfolio/associations/create_associations_for_spoke_local_portfolio_task.py b/...pet/workflow/portfolio/associations/create_associations_for_spoke_local_portfolio_task.py
@@ -9,6 +9,7 @@
 
 from servicecatalog_puppet import config, constants, utils
 from servicecatalog_puppet.workflow.dependencies import tasks
+from servicecatalog_puppet.workflow.portfolio.associations import association_utils
 
 
 class CreateAssociationsForSpokeLocalPortfolioTask(tasks.TaskWithReference):
@@ -90,7 +91,10 @@ def run(self):
             v1_stack_name = f"associations-for-{utils.slugify_for_cloudformation_stack_name(self.spoke_local_portfolio_name)}"
             cloudformation.ensure_deleted(StackName=v1_stack_name)
 
-            v2_stack_name = f"associations-v2-for-{utils.slugify_for_cloudformation_stack_name(self.spoke_local_portfolio_name)}"
+            v2_stack_name = association_utils.generate_stack_name_for_associations_by_item_name(
+                self.spoke_local_portfolio_name
+            )
+
             if associations_to_use:
                 tpl = t.Template()
                 tpl.description = f"Associations for {self.portfolio}"

diff --git a/.../workflow/portfolio/associations/terminate_associations_for_spoke_local_portfolio_task.py b/.../workflow/portfolio/associations/terminate_associations_for_spoke_local_portfolio_task.py
@@ -2,8 +2,9 @@
 #  SPDX-License-Identifier: Apache-2.0
 import luigi
 
-from servicecatalog_puppet import constants, utils
+from servicecatalog_puppet import constants
 from servicecatalog_puppet.workflow.dependencies import tasks
+from servicecatalog_puppet.workflow.portfolio.associations import association_utils
 
 
 class TerminateAssociationsForSpokeLocalPortfolioTask(tasks.TaskWithReference):
@@ -23,7 +24,9 @@ def params_for_results_display(self):
         }
 
     def run(self):
-        stack_name = f"associations-for-{utils.slugify_for_cloudformation_stack_name(self.spoke_local_portfolio_name)}"
+        stack_name = association_utils.generate_stack_name_for_associations_by_item_name(
+            self.spoke_local_portfolio_name
+        )
 
         with self.spoke_regional_client("cloudformation") as cloudformation:
             self.info(f"About to delete the stack: {stack_name}")

diff --git a/setup.py b/setup.py
@@ -74,7 +74,7 @@
 
 setup_kwargs = {
     'name': 'aws-service-catalog-puppet',
-    'version': '0.232.0',
+    'version': '0.232.1',
     'description': 'Making it easier to deploy ServiceCatalog products',
     'long_description': '# aws-service-catalog-puppet\n\n![logo](./docs/logo.png) \n\n## Badges\n\n[![codecov](https://codecov.io/gh/awslabs/aws-service-catalog-puppet/branch/master/graph/badge.svg?token=e8M7mdsmy0)](https://codecov.io/gh/awslabs/aws-service-catalog-puppet)\n\n\n## What is it?\nThis is a python3 framework that makes it easier to share multi region AWS Service Catalog portfolios and makes it \npossible to provision products into accounts declaratively using a metadata based rules engine.\n\nWith this framework you define your accounts in a YAML file.  You give each account a set of tags, a default region and \na set of enabled regions.\n\nOnce you have done this you can define portfolios should be shared with each set of accounts using the tags and you \ncan specify which regions the shares occur in.\n\nIn addition to this, you can also define products that should be provisioned into accounts using the same tag based \napproach.  The framework will assume role into the target account and provision the product on your behalf.\n\n\n## Getting started\n\nYou can read the [installation how to](https://service-catalog-tools-workshop.com/30-how-tos/10-installation/30-service-catalog-puppet.html)\nor you can read through the [every day use](https://service-catalog-tools-workshop.com/30-how-tos/50-every-day-use.html)\nguides.\n\nYou can read the [documentation](https://aws-service-catalog-puppet.readthedocs.io/en/latest/) to understand the inner \nworkings. \n\n\n## Going further\n\nThe framework is one of a pair.  The other is [aws-service-catalog-factory](https://github.com/awslabs/aws-service-catalog-factory).\nWith Service Catalog Factory you can create pipelines that deploy multi region portfolios very easily. \n\n## License\n\nThis library is licensed under the Apache 2.0 License. \n \n',
     'author': 'Eamonn Faherty',