From cd40abe0a94a4461f1e3615068a92f498a38b616 Mon Sep 17 00:00:00 2001 From: Bert Blommers Date: Fri, 1 Mar 2024 22:04:10 +0000 Subject: [PATCH] CloudFormation: Allow deletion of deeply nested structures (#7413) --- moto/cloudformation/parsing.py | 15 +++++-- moto/iam/models.py | 2 +- .../test_cloudformation_stack_crud_boto3.py | 43 +++++++++++++++++++ 3 files changed, 55 insertions(+), 5 deletions(-) diff --git a/moto/cloudformation/parsing.py b/moto/cloudformation/parsing.py index 239cc886d..5a1216be7 100644 --- a/moto/cloudformation/parsing.py +++ b/moto/cloudformation/parsing.py @@ -835,8 +835,15 @@ class ResourceMap(collections_abc.Mapping): # type: ignore[type-arg] for key, value in self._resource_json_map.items() if not value.get("DeletionPolicy") == "Retain" ) - tries = 1 - while remaining_resources and tries < 5: + # Keep track of how many resources we deleted before + # (set to current + 1 to pretend the number of resources is already going down) + previous_remaining_resources = len(remaining_resources) + 1 + # Delete while we have resources, and while the number of remaining resources is going down + while ( + remaining_resources + and len(remaining_resources) < previous_remaining_resources + ): + previous_remaining_resources = len(remaining_resources) for resource in remaining_resources.copy(): parsed_resource = self._parsed_resources.get(resource) try: @@ -858,8 +865,8 @@ class ResourceMap(collections_abc.Mapping): # type: ignore[type-arg] last_exception = e else: remaining_resources.remove(resource) - tries += 1 - if tries == 5: + + if remaining_resources: raise last_exception def _delete_resource( diff --git a/moto/iam/models.py b/moto/iam/models.py index 485c7ba5d..6a21e0781 100644 --- a/moto/iam/models.py +++ b/moto/iam/models.py @@ -743,7 +743,7 @@ class Role(CloudFormationModel): for role in backend.roles.values(): if role.name == resource_name: - for arn in role.policies.keys(): + for arn in list(role.policies.keys()): role.delete_policy(arn) backend.delete_role(resource_name) diff --git a/tests/test_cloudformation/test_cloudformation_stack_crud_boto3.py b/tests/test_cloudformation/test_cloudformation_stack_crud_boto3.py index 7706a0286..258369837 100644 --- a/tests/test_cloudformation/test_cloudformation_stack_crud_boto3.py +++ b/tests/test_cloudformation/test_cloudformation_stack_crud_boto3.py @@ -1723,6 +1723,49 @@ def test_delete_stack(): assert ec2.describe_instances()["Reservations"][0]["Instances"][0]["State"] != state +@mock_aws +@pytest.mark.parametrize("nr_of_resources", [1, 3, 5]) +def test_delete_stack_with_nested_resources(nr_of_resources): + """ + Resources can be dependent on eachother + Verify that we'll keep deleting resources from a stack until there are none left + """ + nested_template = {"Resources": {}} + for idx in range(nr_of_resources): + role_refs = [] + for idy in range(nr_of_resources): + role = { + "Type": "AWS::IAM::Role", + "Properties": { + "AssumeRolePolicyDocument": { + "Statement": [ + { + "Action": ["sts:AssumeRole"], + "Effect": "Allow", + "Principal": {"Service": ["ec2.amazonaws.com"]}, + } + ] + } + }, + } + nested_template["Resources"].update({f"role{idx}_{idy}": role}) + role_refs.append({"Ref": f"role{idx}_{idy}"}) + + instance_profile = { + "Type": "AWS::IAM::InstanceProfile", + "Properties": {"Path": "/", "Roles": role_refs}, + } + nested_template["Resources"].update({f"ip{idx}": instance_profile}) + cf = boto3.client("cloudformation", region_name=REGION_NAME) + + cf.create_stack(StackName=TEST_STACK_NAME, TemplateBody=json.dumps(nested_template)) + cf.delete_stack(StackName=TEST_STACK_NAME) + + iam = boto3.client("iam", REGION_NAME) + assert not iam.list_roles()["Roles"] + assert not iam.list_instance_profiles()["InstanceProfiles"] + + @mock_aws @pytest.mark.skipif( settings.TEST_SERVER_MODE,