From 7f616af5df6089741095f00f9c4e023e7240b850 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cristopher=20Pinz=C3=B3n?= <18080804+pinzon@users.noreply.github.com> Date: Wed, 18 Sep 2024 19:33:07 -0500 Subject: [PATCH 1/2] fix deletion of failed nested stack in CFn (#11489) --- .../cloudformation/resource_provider.py | 1 + .../cloudformation/api/test_nested_stacks.py | 63 ++++++++++++++++++- .../api/test_nested_stacks.snapshot.json | 16 +++++ .../api/test_nested_stacks.validation.json | 3 + .../cfn_failed_nested_stack_child.yml | 34 ++++++++++ .../cfn_failed_nested_stack_parent.yml | 14 +++++ 6 files changed, 129 insertions(+), 2 deletions(-) create mode 100644 tests/aws/templates/cfn_failed_nested_stack_child.yml create mode 100644 tests/aws/templates/cfn_failed_nested_stack_parent.yml diff --git a/localstack-core/localstack/services/cloudformation/resource_provider.py b/localstack-core/localstack/services/cloudformation/resource_provider.py index c6b6be53c5591..92d7e707b6237 100644 --- a/localstack-core/localstack/services/cloudformation/resource_provider.py +++ b/localstack-core/localstack/services/cloudformation/resource_provider.py @@ -477,6 +477,7 @@ def deploy_loop( context = {**payload["callbackContext"], **event.custom_context} payload["callbackContext"] = context payload["requestData"]["resourceProperties"] = event.resource_model + resource["Properties"] = event.resource_model if current_iteration == 0: time.sleep(0) diff --git a/tests/aws/services/cloudformation/api/test_nested_stacks.py b/tests/aws/services/cloudformation/api/test_nested_stacks.py index 788e933209c59..c52b173a1ec80 100644 --- a/tests/aws/services/cloudformation/api/test_nested_stacks.py +++ b/tests/aws/services/cloudformation/api/test_nested_stacks.py @@ -1,8 +1,9 @@ import os import pytest -from botocore.exceptions import ClientError +from botocore.exceptions import ClientError, WaiterError +from localstack import config from localstack.testing.aws.util import is_aws_cloud from localstack.testing.pytest import markers from localstack.utils.files import load_file @@ -81,7 +82,6 @@ def test_nested_stack_output_refs(deploy_cfn_template, s3_create_bucket, aws_cli assert f"{nested_bucket_name}-suffix" == result.outputs["CustomOutput"] -@pytest.mark.skip(reason="Nested stacks don't work properly") @markers.aws.validated def test_nested_with_nested_stack(deploy_cfn_template, s3_create_bucket, aws_client): bucket_name = s3_create_bucket() @@ -291,3 +291,62 @@ def test_nested_stacks_conditions(deploy_cfn_template, s3_create_bucket, aws_cli StackName=stack.outputs["NestedStackArn"] ) assert ":" not in nested_stack["Stacks"][0]["StackName"] + + +@markers.aws.validated +def test_deletion_of_failed_nested_stack(s3_create_bucket, aws_client, region_name, snapshot): + """ + This test confirms that after deleting a stack parent with a failed nested stack. The nested stack is also deleted + """ + + bucket_name = s3_create_bucket() + aws_client.s3.upload_file( + os.path.join( + os.path.dirname(__file__), "../../../templates/cfn_failed_nested_stack_child.yml" + ), + Bucket=bucket_name, + Key="child.yml", + ) + + stack_name = f"stack-{short_uid()}" + child_template_url = ( + f"https://{bucket_name}.s3.{config.LOCALSTACK_HOST.host_and_port()}/child.yml" + ) + if is_aws_cloud(): + child_template_url = f"https://{bucket_name}.s3.{region_name}.amazonaws.com/child.yml" + + aws_client.cloudformation.create_stack( + StackName=stack_name, + TemplateBody=load_file( + os.path.join( + os.path.dirname(__file__), "../../../templates/cfn_failed_nested_stack_parent.yml" + ), + ), + Parameters=[ + {"ParameterKey": "TemplateUri", "ParameterValue": child_template_url}, + ], + OnFailure="DO_NOTHING", + Capabilities=["CAPABILITY_NAMED_IAM"], + ) + + with pytest.raises(WaiterError): + aws_client.cloudformation.get_waiter("stack_create_complete").wait(StackName=stack_name) + + stack_status = aws_client.cloudformation.describe_stacks(StackName=stack_name)["Stacks"][0][ + "StackStatus" + ] + assert stack_status == "CREATE_FAILED" + + stacks = aws_client.cloudformation.describe_stacks()["Stacks"] + nested_stack_name = [ + stack for stack in stacks if f"{stack_name}-ChildStack-" in stack["StackName"] + ][0]["StackName"] + + aws_client.cloudformation.delete_stack(StackName=stack_name) + aws_client.cloudformation.get_waiter("stack_delete_complete").wait(StackName=stack_name) + + with pytest.raises(ClientError) as ex: + aws_client.cloudformation.describe_stacks(StackName=nested_stack_name) + + snapshot.match("error", ex.value.response) + snapshot.add_transformer(snapshot.transform.regex(nested_stack_name, "")) diff --git a/tests/aws/services/cloudformation/api/test_nested_stacks.snapshot.json b/tests/aws/services/cloudformation/api/test_nested_stacks.snapshot.json index 9582684874be0..fbd1c318a283b 100644 --- a/tests/aws/services/cloudformation/api/test_nested_stacks.snapshot.json +++ b/tests/aws/services/cloudformation/api/test_nested_stacks.snapshot.json @@ -63,5 +63,21 @@ } } } + }, + "tests/aws/services/cloudformation/api/test_nested_stacks.py::test_deletion_of_failed_nested_stack": { + "recorded-date": "17-09-2024, 20:09:36", + "recorded-content": { + "error": { + "Error": { + "Code": "ValidationError", + "Message": "Stack with id does not exist", + "Type": "Sender" + }, + "ResponseMetadata": { + "HTTPHeaders": {}, + "HTTPStatusCode": 400 + } + } + } } } diff --git a/tests/aws/services/cloudformation/api/test_nested_stacks.validation.json b/tests/aws/services/cloudformation/api/test_nested_stacks.validation.json index af6f4d050affc..f5936f2e379e7 100644 --- a/tests/aws/services/cloudformation/api/test_nested_stacks.validation.json +++ b/tests/aws/services/cloudformation/api/test_nested_stacks.validation.json @@ -1,4 +1,7 @@ { + "tests/aws/services/cloudformation/api/test_nested_stacks.py::test_deletion_of_failed_nested_stack": { + "last_validated_date": "2024-09-17T20:09:36+00:00" + }, "tests/aws/services/cloudformation/api/test_nested_stacks.py::test_nested_output_in_params": { "last_validated_date": "2023-02-07T09:57:47+00:00" } diff --git a/tests/aws/templates/cfn_failed_nested_stack_child.yml b/tests/aws/templates/cfn_failed_nested_stack_child.yml new file mode 100644 index 0000000000000..7b638c5775b9f --- /dev/null +++ b/tests/aws/templates/cfn_failed_nested_stack_child.yml @@ -0,0 +1,34 @@ +Resources: + MyLambdaFunction: + Type: AWS::Lambda::Function + Properties: + FunctionName: testFunction + Handler: index.handler + Runtime: python3.8 + Role: !GetAtt LambdaExecutionRole.Arn + Code: + S3Bucket: non-existent-bucket-name # Invalid S3 bucket that does not exist + S3Key: non-existent-key.zip + + LambdaExecutionRole: + Type: AWS::IAM::Role + Properties: + RoleName: LambdaExecutionRole + AssumeRolePolicyDocument: + Version: '2012-10-17' + Statement: + - Effect: Allow + Principal: + Service: lambda.amazonaws.com + Action: sts:AssumeRole + Policies: + - PolicyName: LambdaBasicExecution + PolicyDocument: + Version: '2012-10-17' + Statement: + - Effect: Allow + Action: + - logs:CreateLogGroup + - logs:CreateLogStream + - logs:PutLogEvents + Resource: arn:aws:logs:*:*:* diff --git a/tests/aws/templates/cfn_failed_nested_stack_parent.yml b/tests/aws/templates/cfn_failed_nested_stack_parent.yml new file mode 100644 index 0000000000000..f685b2566f37c --- /dev/null +++ b/tests/aws/templates/cfn_failed_nested_stack_parent.yml @@ -0,0 +1,14 @@ +AWSTemplateFormatVersion: '2010-09-09' +Parameters: + TemplateUri: + Type: String + +Resources: + ChildStack: + Type: AWS::CloudFormation::Stack + Properties: + TemplateURL: !Ref TemplateUri + +Outputs: + BucketStackId: + Value: !Ref ChildStack From 52d90ec5983df12b0544078d009ce3c5715e177f Mon Sep 17 00:00:00 2001 From: Simon Walker Date: Wed, 18 Sep 2024 22:52:35 +0100 Subject: [PATCH 2/2] Implement fix and better errors --- .../services/cloudformation/engine/errors.py | 4 ++ .../cloudformation/engine/template_utils.py | 48 +++++++++++++- .../cloudformation/engine/test_mappings.py | 20 ++++++ .../engine/test_mappings.snapshot.json | 65 +++++++++++++++++++ .../engine/test_mappings.validation.json | 3 + .../mappings/mapping-ref-map-key.yaml | 36 ++++++++++ 6 files changed, 175 insertions(+), 1 deletion(-) create mode 100644 localstack-core/localstack/services/cloudformation/engine/errors.py create mode 100644 tests/aws/templates/mappings/mapping-ref-map-key.yaml diff --git a/localstack-core/localstack/services/cloudformation/engine/errors.py b/localstack-core/localstack/services/cloudformation/engine/errors.py new file mode 100644 index 0000000000000..0ee44f3530e58 --- /dev/null +++ b/localstack-core/localstack/services/cloudformation/engine/errors.py @@ -0,0 +1,4 @@ +class TemplateError(RuntimeError): + """ + Error thrown on a programming error from the user + """ diff --git a/localstack-core/localstack/services/cloudformation/engine/template_utils.py b/localstack-core/localstack/services/cloudformation/engine/template_utils.py index 3d5ca17e7aab3..5bbcfa2367e2f 100644 --- a/localstack-core/localstack/services/cloudformation/engine/template_utils.py +++ b/localstack-core/localstack/services/cloudformation/engine/template_utils.py @@ -2,6 +2,7 @@ from typing import Any from localstack.services.cloudformation.deployment_utils import PLACEHOLDER_AWS_NO_VALUE +from localstack.services.cloudformation.engine.errors import TemplateError from localstack.utils.urls import localstack_host AWS_URL_SUFFIX = localstack_host().host # value is "amazonaws.com" in real AWS @@ -181,7 +182,52 @@ def resolve_condition( ) case "Fn::FindInMap": map_name, top_level_key, second_level_key = v - return mappings[map_name][top_level_key][second_level_key] + if isinstance(map_name, dict) and "Ref" in map_name: + ref_name = map_name["Ref"] + param = parameters.get(ref_name) + if not param: + raise TemplateError( + f"Invalid reference: '{ref_name}' does not exist in parameters: '{parameters}'" + ) + map_name = param.get("ResolvedValue") or param.get("ParameterValue") + + if isinstance(top_level_key, dict) and "Ref" in top_level_key: + ref_name = top_level_key["Ref"] + param = parameters.get(ref_name) + if not param: + raise TemplateError( + f"Invalid reference: '{ref_name}' does not exist in parameters: '{parameters}'" + ) + top_level_key = param.get("ResolvedValue") or param.get("ParameterValue") + + if isinstance(second_level_key, dict) and "Ref" in second_level_key: + ref_name = second_level_key["Ref"] + param = parameters.get(ref_name) + if not param: + raise TemplateError( + f"Invalid reference: '{ref_name}' does not exist in parameters: '{parameters}'" + ) + second_level_key = param.get("ResolvedValue") or param.get("ParameterValue") + + mapping = mappings.get(map_name) + if not mapping: + raise TemplateError( + f"Invalid reference: '{map_name}' could not be found in the template mappings: '{list(mappings.keys())}'" + ) + + top_level_map = mapping.get(top_level_key) + if not top_level_map: + raise TemplateError( + f"Invalid reference: '{top_level_key}' could not be found in the '{map_name}' mapping: '{list(mapping.keys())}'" + ) + + value = top_level_map.get(second_level_key) + if not value: + raise TemplateError( + f"Invalid reference: '{second_level_key}' could not be found in the '{top_level_key}' mapping: '{top_level_map}'" + ) + + return value case "Fn::If": if_condition_name, true_branch, false_branch = v if resolve_condition( diff --git a/tests/aws/services/cloudformation/engine/test_mappings.py b/tests/aws/services/cloudformation/engine/test_mappings.py index d8a60bb9ebbba..f47b46d1524dc 100644 --- a/tests/aws/services/cloudformation/engine/test_mappings.py +++ b/tests/aws/services/cloudformation/engine/test_mappings.py @@ -153,3 +153,23 @@ def test_mapping_minimum_nesting_depth(self, aws_client, cleanups, snapshot): ], ) snapshot.match("mapping_minimum_level_exc", e.value.response) + + @markers.aws.validated + def test_mapping_ref_map_key(self, deploy_cfn_template, aws_client, snapshot): + topic_name = f"topic-{short_uid()}" + stack = deploy_cfn_template( + template_path=os.path.join( + THIS_DIR, "../../../templates/mappings/mapping-ref-map-key.yaml" + ), + parameters={ + "MapName": "MyMap", + "MapKey": "A", + "TopicName": topic_name, + }, + ) + + topic_arn = stack.outputs.get("TopicArn") + assert topic_arn is not None + + describe_res = aws_client.sns.get_topic_attributes(TopicArn=topic_arn) + snapshot.match("topic-attributes", describe_res) diff --git a/tests/aws/services/cloudformation/engine/test_mappings.snapshot.json b/tests/aws/services/cloudformation/engine/test_mappings.snapshot.json index c0287ad3e85fe..415a5fea1756e 100644 --- a/tests/aws/services/cloudformation/engine/test_mappings.snapshot.json +++ b/tests/aws/services/cloudformation/engine/test_mappings.snapshot.json @@ -62,5 +62,70 @@ } } } + }, + "tests/aws/services/cloudformation/engine/test_mappings.py::TestCloudFormationMappings::test_mapping_ref_map_key": { + "recorded-date": "18-09-2024, 21:26:20", + "recorded-content": { + "topic-attributes": { + "Attributes": { + "DisplayName": "", + "EffectiveDeliveryPolicy": { + "http": { + "defaultHealthyRetryPolicy": { + "minDelayTarget": 20, + "maxDelayTarget": 20, + "numRetries": 3, + "numMaxDelayRetries": 0, + "numNoDelayRetries": 0, + "numMinDelayRetries": 0, + "backoffFunction": "linear" + }, + "disableSubscriptionOverrides": false, + "defaultRequestPolicy": { + "headerContentType": "text/plain; charset=UTF-8" + } + } + }, + "Owner": "111111111111", + "Policy": { + "Version": "2008-10-17", + "Id": "__default_policy_ID", + "Statement": [ + { + "Sid": "__default_statement_ID", + "Effect": "Allow", + "Principal": { + "AWS": "*" + }, + "Action": [ + "SNS:GetTopicAttributes", + "SNS:SetTopicAttributes", + "SNS:AddPermission", + "SNS:RemovePermission", + "SNS:DeleteTopic", + "SNS:Subscribe", + "SNS:ListSubscriptionsByTopic", + "SNS:Publish" + ], + "Resource": "arn::sns::111111111111:topic-f1301a86", + "Condition": { + "StringEquals": { + "AWS:SourceOwner": "111111111111" + } + } + } + ] + }, + "SubscriptionsConfirmed": "0", + "SubscriptionsDeleted": "0", + "SubscriptionsPending": "0", + "TopicArn": "arn::sns::111111111111:topic-f1301a86" + }, + "ResponseMetadata": { + "HTTPHeaders": {}, + "HTTPStatusCode": 200 + } + } + } } } diff --git a/tests/aws/services/cloudformation/engine/test_mappings.validation.json b/tests/aws/services/cloudformation/engine/test_mappings.validation.json index 7b2ac5b1dcfb7..52374c4ff0380 100644 --- a/tests/aws/services/cloudformation/engine/test_mappings.validation.json +++ b/tests/aws/services/cloudformation/engine/test_mappings.validation.json @@ -5,6 +5,9 @@ "tests/aws/services/cloudformation/engine/test_mappings.py::TestCloudFormationMappings::test_mapping_minimum_nesting_depth": { "last_validated_date": "2023-06-12T14:47:25+00:00" }, + "tests/aws/services/cloudformation/engine/test_mappings.py::TestCloudFormationMappings::test_mapping_ref_map_key": { + "last_validated_date": "2024-09-18T21:26:20+00:00" + }, "tests/aws/services/cloudformation/engine/test_mappings.py::TestCloudFormationMappings::test_mapping_with_invalid_refs": { "last_validated_date": "2023-06-12T14:47:24+00:00" }, diff --git a/tests/aws/templates/mappings/mapping-ref-map-key.yaml b/tests/aws/templates/mappings/mapping-ref-map-key.yaml new file mode 100644 index 0000000000000..bff037f6fab30 --- /dev/null +++ b/tests/aws/templates/mappings/mapping-ref-map-key.yaml @@ -0,0 +1,36 @@ +Mappings: + MyMap: + A: + value: "true" + C: + value: "false" + +Conditions: + MyCondition: !Equals + - !FindInMap [ !Ref MapName, !Ref MapKey, value ] + - "true" + +Parameters: + MapName: + Type: String + + MapKey: + Type: String + + TopicName: + Type: String + +Resources: + MyTopic: + Type: AWS::SNS::Topic + Properties: + TopicName: !Ref TopicName + Condition: MyCondition + + Dummy: + Type: AWS::SNS::Topic + +Outputs: + TopicArn: + Value: !Ref MyTopic +