From f84f2c35a61eced159ecb6b02be6137090f6936a Mon Sep 17 00:00:00 2001 From: Simon Walker Date: Thu, 7 Aug 2025 15:08:24 +0100 Subject: [PATCH 01/17] Handle stack already exists --- .../services/cloudformation/v2/provider.py | 23 ++++++++++++++++++- .../cloudformation/api/test_stacks.py | 2 +- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/localstack-core/localstack/services/cloudformation/v2/provider.py b/localstack-core/localstack/services/cloudformation/v2/provider.py index c7372de2ac5e3..24cf49484a858 100644 --- a/localstack-core/localstack/services/cloudformation/v2/provider.py +++ b/localstack-core/localstack/services/cloudformation/v2/provider.py @@ -7,6 +7,7 @@ from localstack.aws.api import RequestContext, handler from localstack.aws.api.cloudformation import ( + AlreadyExistsException, CallAs, Changes, ChangeSetNameOrId, @@ -638,6 +639,26 @@ def create_stack(self, context: RequestContext, request: CreateStackInput) -> Cr raise ValidationError("StackName must be specified") state = get_cloudformation_store(context.account_id, context.region) + + active_stack_candidates = [ + stack + for stack in state.stacks_v2.values() + if stack.stack_name == stack_name and stack.status not in [StackStatus.DELETE_COMPLETE] + ] + + # TODO: fix/implement this code path + # this needs more investigation how Cloudformation handles it (e.g. normal stack create or does it create a separate changeset?) + # REVIEW_IN_PROGRESS is another special status + # in this case existing changesets are set to obsolete and the stack is created + # review_stack_candidates = [s for s in stack_candidates if s.status == StackStatus.REVIEW_IN_PROGRESS] + # if review_stack_candidates: + # set changesets to obsolete + # for cs in review_stack_candidates[0].change_sets: + # cs.execution_status = ExecutionStatus.OBSOLETE + + if active_stack_candidates: + raise AlreadyExistsException(f"Stack [{stack_name}] already exists") + # TODO: copied from create_change_set, consider unifying template_body = request.get("TemplateBody") # s3 or secretsmanager url @@ -747,7 +768,7 @@ def describe_stacks( if stack_name: stack = find_stack_v2(state, stack_name) if not stack: - raise StackNotFoundError(stack_name) + raise ValidationError(f"Stack with id {stack_name} does not exist") stacks = [stack] else: stacks = state.stacks_v2.values() diff --git a/tests/aws/services/cloudformation/api/test_stacks.py b/tests/aws/services/cloudformation/api/test_stacks.py index d38e17d42ce06..46ee8adeb99ee 100644 --- a/tests/aws/services/cloudformation/api/test_stacks.py +++ b/tests/aws/services/cloudformation/api/test_stacks.py @@ -702,9 +702,9 @@ def test_blocked_stack_deletion(aws_client, cleanups, snapshot): """ -@skip_if_v2_provider(reason="CFNV2:Validation") @markers.snapshot.skip_snapshot_verify( paths=["$..EnableTerminationProtection", "$..LastUpdatedTime"] + + skipped_v2_items("$..Capabilities") ) @markers.aws.validated def test_name_conflicts(aws_client, snapshot, cleanups): From 8ace993cf489fa9fc1a0b3004ff6d91627c3e6b9 Mon Sep 17 00:00:00 2001 From: Simon Walker Date: Thu, 7 Aug 2025 15:08:37 +0100 Subject: [PATCH 02/17] Handle update stack with same template --- tests/aws/services/cloudformation/api/test_stacks.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/aws/services/cloudformation/api/test_stacks.py b/tests/aws/services/cloudformation/api/test_stacks.py index 46ee8adeb99ee..848bd241299e0 100644 --- a/tests/aws/services/cloudformation/api/test_stacks.py +++ b/tests/aws/services/cloudformation/api/test_stacks.py @@ -219,7 +219,6 @@ def test_stack_update_resources( resources = aws_client.cloudformation.describe_stack_resources(StackName=stack_name) snapshot.match("stack_resources", resources) - @skip_if_v2_provider(reason="CFNV2:Validation") @markers.aws.validated def test_update_stack_with_same_template_withoutchange( self, deploy_cfn_template, aws_client, snapshot @@ -239,7 +238,7 @@ def test_update_stack_with_same_template_withoutchange( snapshot.match("no_change_exception", ctx.value.response) - @skip_if_v2_provider(reason="CFNV2:Validation") + @skip_if_v2_provider(reason="CFNV2:Transform") @markers.aws.validated def test_update_stack_with_same_template_withoutchange_transformation( self, deploy_cfn_template, aws_client From 81f143e495ab0bf492aae1aefe91515640c18731 Mon Sep 17 00:00:00 2001 From: Simon Walker Date: Thu, 7 Aug 2025 15:48:16 +0100 Subject: [PATCH 03/17] Preprocessor can emit validation errors --- .../cloudformation/engine/v2/change_set_model_preproc.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_preproc.py b/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_preproc.py index 729716e0da913..3c26546d9a6c6 100644 --- a/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_preproc.py +++ b/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_preproc.py @@ -47,6 +47,7 @@ extract_dynamic_reference, perform_dynamic_reference_lookup, ) +from localstack.services.cloudformation.engine.validations import ValidationError from localstack.services.cloudformation.stores import ( exports_map, ) @@ -372,7 +373,8 @@ def _resolve_mapping( node_mapping: NodeMapping = self._get_node_mapping(map_name=map_name) top_level_value = node_mapping.bindings.bindings.get(top_level_key) if not isinstance(top_level_value, NodeObject): - raise RuntimeError() + error_key = "::".join([map_name, top_level_key, second_level_key]) + raise ValidationError(f"Template error: Unable to get mapping for {error_key}") second_level_value = top_level_value.bindings.get(second_level_key) mapping_value_delta = self.visit(second_level_value) return mapping_value_delta From ecfae265c5b08e3fffe09d0140b41394f603c376 Mon Sep 17 00:00:00 2001 From: Simon Walker Date: Thu, 7 Aug 2025 16:03:37 +0100 Subject: [PATCH 04/17] Move some validations to the preprocessor --- .../engine/v2/change_set_model_preproc.py | 6 ++++ .../engine/v2/change_set_model_validator.py | 31 ++++--------------- .../cloudformation/engine/test_mappings.py | 28 ++++++++++++++++- .../engine/test_mappings.snapshot.json | 6 ++++ .../engine/test_mappings.validation.json | 9 ++++++ 5 files changed, 54 insertions(+), 26 deletions(-) diff --git a/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_preproc.py b/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_preproc.py index 3c26546d9a6c6..f4a8563c2093a 100644 --- a/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_preproc.py +++ b/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_preproc.py @@ -76,6 +76,8 @@ MOCKED_REFERENCE = "unknown" +VALID_LOGICAL_RESOURCE_ID_RE = re.compile(r"^[A-Za-z0-9]+$") + class PreprocEntityDelta(Generic[TBefore, TAfter]): before: Maybe[TBefore] @@ -1055,6 +1057,10 @@ def _resolve_resource_condition_reference(self, reference: TerminalValue) -> Pre def visit_node_resource( self, node_resource: NodeResource ) -> PreprocEntityDelta[PreprocResource, PreprocResource]: + if not VALID_LOGICAL_RESOURCE_ID_RE.match(node_resource.name): + raise ValidationError( + f"Template format error: Resource name {node_resource.name} is non alphanumeric." + ) change_type = node_resource.change_type condition_before = Nothing condition_after = Nothing diff --git a/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_validator.py b/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_validator.py index 8176733b44667..eeffa7405a1aa 100644 --- a/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_validator.py +++ b/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_validator.py @@ -1,42 +1,30 @@ -import re - from localstack.services.cloudformation.engine.v2.change_set_model import ( NodeParameters, - NodeResource, NodeTemplate, is_nothing, ) from localstack.services.cloudformation.engine.v2.change_set_model_preproc import ( + ChangeSetModelPreproc, PreprocEntityDelta, ) -from localstack.services.cloudformation.engine.v2.change_set_model_visitor import ( - ChangeSetModelVisitor, -) from localstack.services.cloudformation.engine.validations import ValidationError -from localstack.services.cloudformation.v2.entities import ChangeSet - -VALID_LOGICAL_RESOURCE_ID_RE = re.compile(r"^[A-Za-z0-9]+$") -class ChangeSetModelValidator(ChangeSetModelVisitor): - def __init__(self, change_set: ChangeSet): - self._change_set = change_set - +class ChangeSetModelValidator(ChangeSetModelPreproc): def validate(self): - self.visit(self._change_set.update_model.node_template) + self.process() def visit_node_template(self, node_template: NodeTemplate): self.visit(node_template.parameters) + self.visit(node_template.mappings) self.visit(node_template.resources) def visit_node_parameters(self, node_parameters: NodeParameters) -> PreprocEntityDelta: # check that all parameters have values invalid_parameters = [] for node_parameter in node_parameters.parameters: - self.visit(node_parameter) - if is_nothing(node_parameter.default_value.value) and is_nothing( - node_parameter.dynamic_value.value - ): + parameter_value = self.visit(node_parameter) + if is_nothing(parameter_value.before) and is_nothing(parameter_value.after): invalid_parameters.append(node_parameter.name) if invalid_parameters: @@ -44,10 +32,3 @@ def visit_node_parameters(self, node_parameters: NodeParameters) -> PreprocEntit # continue visiting return super().visit_node_parameters(node_parameters) - - def visit_node_resource(self, node_resource: NodeResource) -> PreprocEntityDelta: - if not VALID_LOGICAL_RESOURCE_ID_RE.match(node_resource.name): - raise ValidationError( - f"Template format error: Resource name {node_resource.name} is non alphanumeric." - ) - return super().visit_node_resource(node_resource) diff --git a/tests/aws/services/cloudformation/engine/test_mappings.py b/tests/aws/services/cloudformation/engine/test_mappings.py index b16fccdfa02df..a0fa11e104f9b 100644 --- a/tests/aws/services/cloudformation/engine/test_mappings.py +++ b/tests/aws/services/cloudformation/engine/test_mappings.py @@ -1,6 +1,7 @@ import os import pytest +from botocore.exceptions import ClientError from tests.aws.services.cloudformation.conftest import skip_if_v2_provider from localstack.testing.pytest import markers @@ -11,7 +12,6 @@ THIS_DIR = os.path.dirname(__file__) -@markers.snapshot.skip_snapshot_verify class TestCloudFormationMappings: @markers.aws.validated def test_simple_mapping_working(self, aws_client, deploy_cfn_template): @@ -93,6 +93,32 @@ def test_async_mapping_error_first_level(self, deploy_cfn_template): assert "Cannot find map key 'C' in mapping 'TopicSuffixMap'" in str(exc_info.value) + @markers.aws.validated + def test_async_mapping_error_first_level_v2(self, aws_client, snapshot): + snapshot.add_transformer(snapshot.transform.cloudformation_api()) + topic_name = f"test-topic-{short_uid()}" + template_path = os.path.join( + THIS_DIR, + "../../../templates/mappings/simple-mapping.yaml", + ) + parameters = [ + {"ParameterKey": "TopicName", "ParameterValue": topic_name}, + {"ParameterKey": "TopicNameSuffixSelector", "ParameterValue": "C"}, + ] + + stack_name = f"stack-{short_uid()}" + change_set_name = f"cs-{short_uid()}" + with pytest.raises(ClientError) as exc_info: + aws_client.cloudformation.create_change_set( + ChangeSetName=change_set_name, + StackName=stack_name, + ChangeSetType="CREATE", + Parameters=parameters, + TemplateBody=open(template_path).read(), + ) + + snapshot.match("error", exc_info.value) + @skip_if_v2_provider(reason="CFNV2:Validation") @markers.aws.only_localstack def test_async_mapping_error_second_level(self, deploy_cfn_template): diff --git a/tests/aws/services/cloudformation/engine/test_mappings.snapshot.json b/tests/aws/services/cloudformation/engine/test_mappings.snapshot.json index c0287ad3e85fe..c97612f2e7e9b 100644 --- a/tests/aws/services/cloudformation/engine/test_mappings.snapshot.json +++ b/tests/aws/services/cloudformation/engine/test_mappings.snapshot.json @@ -62,5 +62,11 @@ } } } + }, + "tests/aws/services/cloudformation/engine/test_mappings.py::TestCloudFormationMappings::test_async_mapping_error_first_level_v2": { + "recorded-date": "07-08-2025, 14:34:05", + "recorded-content": { + "error": "An error occurred (ValidationError) when calling the CreateChangeSet operation: Template error: Unable to get mapping for TopicSuffixMap::C::Suffix" + } } } diff --git a/tests/aws/services/cloudformation/engine/test_mappings.validation.json b/tests/aws/services/cloudformation/engine/test_mappings.validation.json index d59232a7b10f5..6c1df79df241b 100644 --- a/tests/aws/services/cloudformation/engine/test_mappings.validation.json +++ b/tests/aws/services/cloudformation/engine/test_mappings.validation.json @@ -1,4 +1,13 @@ { + "tests/aws/services/cloudformation/engine/test_mappings.py::TestCloudFormationMappings::test_async_mapping_error_first_level_v2": { + "last_validated_date": "2025-08-07T14:34:05+00:00", + "durations_in_seconds": { + "setup": 0.87, + "call": 0.28, + "teardown": 0.0, + "total": 1.15 + } + }, "tests/aws/services/cloudformation/engine/test_mappings.py::TestCloudFormationMappings::test_aws_refs_in_mappings": { "last_validated_date": "2024-10-15T17:22:43+00:00" }, From f3f8f60c2625d131f4f5f4dbcb734701f110107d Mon Sep 17 00:00:00 2001 From: Simon Walker Date: Thu, 7 Aug 2025 16:06:49 +0100 Subject: [PATCH 05/17] Handle second level mapping keys --- .../engine/v2/change_set_model_preproc.py | 3 ++ .../cloudformation/engine/test_mappings.py | 31 +++++++++++++++++++ .../engine/test_mappings.snapshot.json | 6 ++++ .../engine/test_mappings.validation.json | 9 ++++++ 4 files changed, 49 insertions(+) diff --git a/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_preproc.py b/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_preproc.py index f4a8563c2093a..3a0c2e1ce9123 100644 --- a/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_preproc.py +++ b/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_preproc.py @@ -378,6 +378,9 @@ def _resolve_mapping( error_key = "::".join([map_name, top_level_key, second_level_key]) raise ValidationError(f"Template error: Unable to get mapping for {error_key}") second_level_value = top_level_value.bindings.get(second_level_key) + if not isinstance(second_level_value, NodeObject): + error_key = "::".join([map_name, top_level_key, second_level_key]) + raise ValidationError(f"Template error: Unable to get mapping for {error_key}") mapping_value_delta = self.visit(second_level_value) return mapping_value_delta diff --git a/tests/aws/services/cloudformation/engine/test_mappings.py b/tests/aws/services/cloudformation/engine/test_mappings.py index a0fa11e104f9b..0e5f13f2a3288 100644 --- a/tests/aws/services/cloudformation/engine/test_mappings.py +++ b/tests/aws/services/cloudformation/engine/test_mappings.py @@ -144,6 +144,37 @@ def test_async_mapping_error_second_level(self, deploy_cfn_template): exc_info.value ) + @markers.aws.validated + def test_async_mapping_error_second_level_v2(self, aws_client, snapshot): + """ + Similar to the `test_async_mapping_error_first_level` test above, but + checking the second level of mapping lookup + """ + snapshot.add_transformer(snapshot.transform.cloudformation_api()) + topic_name = f"test-topic-{short_uid()}" + template_path = os.path.join( + THIS_DIR, + "../../../templates/mappings/simple-mapping.yaml", + ) + parameters = [ + {"ParameterKey": "TopicName", "ParameterValue": topic_name}, + {"ParameterKey": "TopicNameSuffixSelector", "ParameterValue": "A"}, + {"ParameterKey": "TopicAttributeSelector", "ParameterValue": "NotValid"}, + ] + + stack_name = f"stack-{short_uid()}" + change_set_name = f"cs-{short_uid()}" + with pytest.raises(ClientError) as exc_info: + aws_client.cloudformation.create_change_set( + ChangeSetName=change_set_name, + StackName=stack_name, + ChangeSetType="CREATE", + Parameters=parameters, + TemplateBody=open(template_path).read(), + ) + + snapshot.match("error", exc_info.value) + @markers.aws.validated @pytest.mark.skip(reason="not implemented") def test_mapping_with_invalid_refs(self, aws_client, deploy_cfn_template, cleanups, snapshot): diff --git a/tests/aws/services/cloudformation/engine/test_mappings.snapshot.json b/tests/aws/services/cloudformation/engine/test_mappings.snapshot.json index c97612f2e7e9b..b40e5054b585d 100644 --- a/tests/aws/services/cloudformation/engine/test_mappings.snapshot.json +++ b/tests/aws/services/cloudformation/engine/test_mappings.snapshot.json @@ -68,5 +68,11 @@ "recorded-content": { "error": "An error occurred (ValidationError) when calling the CreateChangeSet operation: Template error: Unable to get mapping for TopicSuffixMap::C::Suffix" } + }, + "tests/aws/services/cloudformation/engine/test_mappings.py::TestCloudFormationMappings::test_async_mapping_error_second_level_v2": { + "recorded-date": "07-08-2025, 15:05:47", + "recorded-content": { + "error": "An error occurred (ValidationError) when calling the CreateChangeSet operation: Template error: Unable to get mapping for TopicSuffixMap::A::NotValid" + } } } diff --git a/tests/aws/services/cloudformation/engine/test_mappings.validation.json b/tests/aws/services/cloudformation/engine/test_mappings.validation.json index 6c1df79df241b..8a5c3011aa1e8 100644 --- a/tests/aws/services/cloudformation/engine/test_mappings.validation.json +++ b/tests/aws/services/cloudformation/engine/test_mappings.validation.json @@ -8,6 +8,15 @@ "total": 1.15 } }, + "tests/aws/services/cloudformation/engine/test_mappings.py::TestCloudFormationMappings::test_async_mapping_error_second_level_v2": { + "last_validated_date": "2025-08-07T15:05:47+00:00", + "durations_in_seconds": { + "setup": 1.01, + "call": 0.36, + "teardown": 0.0, + "total": 1.37 + } + }, "tests/aws/services/cloudformation/engine/test_mappings.py::TestCloudFormationMappings::test_aws_refs_in_mappings": { "last_validated_date": "2024-10-15T17:22:43+00:00" }, From 8356f3efc46bf16d58caf79bc938a5fa7e2e179e Mon Sep 17 00:00:00 2001 From: Simon Walker Date: Thu, 7 Aug 2025 16:08:20 +0100 Subject: [PATCH 06/17] Fix wording of missing resources --- .../cloudformation/engine/v2/change_set_model_preproc.py | 4 +++- tests/aws/services/cloudformation/engine/test_references.py | 2 -- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_preproc.py b/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_preproc.py index 3a0c2e1ce9123..f45d8ec53da46 100644 --- a/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_preproc.py +++ b/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_preproc.py @@ -226,7 +226,9 @@ def _get_node_resource_for( if node_resource.name == resource_name: self.visit(node_resource) return node_resource - raise RuntimeError(f"No resource '{resource_name}' was found") + raise ValidationError( + f"Template format error: Unresolved resource dependencies [{resource_name}] in the Resources block of the template" + ) def _get_node_property_for( self, property_name: str, node_resource: NodeResource diff --git a/tests/aws/services/cloudformation/engine/test_references.py b/tests/aws/services/cloudformation/engine/test_references.py index 2d6b15e373b13..ced32e1e92a27 100644 --- a/tests/aws/services/cloudformation/engine/test_references.py +++ b/tests/aws/services/cloudformation/engine/test_references.py @@ -3,7 +3,6 @@ import pytest from botocore.exceptions import ClientError -from tests.aws.services.cloudformation.conftest import skip_if_v2_provider from localstack.testing.aws.util import is_aws_cloud from localstack.testing.pytest import markers @@ -75,7 +74,6 @@ def test_non_string_parameter_in_sub(self, deploy_cfn_template, aws_client, snap snapshot.match("get-parameter-result", get_param_res) -@skip_if_v2_provider(reason="CFNV2:Validation") @markers.aws.validated def test_useful_error_when_invalid_ref(deploy_cfn_template, snapshot): """ From 36596e99627a7bd5f92821b34ba98ac0b07875e1 Mon Sep 17 00:00:00 2001 From: Simon Walker Date: Thu, 7 Aug 2025 16:30:44 +0100 Subject: [PATCH 07/17] Rescope or remove validation tags --- .../engine/v2/change_set_model_executor.py | 2 +- .../localstack/services/cloudformation/v2/provider.py | 5 ++++- .../aws/services/cloudformation/engine/test_mappings.py | 4 ++-- .../aws/services/cloudformation/test_template_engine.py | 9 ++++----- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_executor.py b/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_executor.py index 63042f333eadb..e26f5a988abec 100644 --- a/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_executor.py +++ b/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_executor.py @@ -441,7 +441,7 @@ def _execute_resource_action( LOG.warning( "Resource provider operation failed: '%s'", reason, - exc_info=LOG.isEnabledFor(logging.DEBUG), + exc_info=LOG.isEnabledFor(logging.DEBUG) and config.CFN_VERBOSE_ERRORS, ) event = ProgressEvent( OperationStatus.FAILED, diff --git a/localstack-core/localstack/services/cloudformation/v2/provider.py b/localstack-core/localstack/services/cloudformation/v2/provider.py index 24cf49484a858..5d7e407a31013 100644 --- a/localstack-core/localstack/services/cloudformation/v2/provider.py +++ b/localstack-core/localstack/services/cloudformation/v2/provider.py @@ -78,6 +78,7 @@ Stack as ApiStack, ) from localstack.aws.connect import connect_to +from localstack.config import CFN_VERBOSE_ERRORS from localstack.services.cloudformation import api_utils from localstack.services.cloudformation.engine import template_preparer from localstack.services.cloudformation.engine.parameters import resolve_ssm_parameter @@ -533,7 +534,9 @@ def _run(*args): change_set.stack.template_body = change_set.template_body except Exception as e: LOG.error( - "Execute change set failed: %s", e, exc_info=LOG.isEnabledFor(logging.WARNING) + "Execute change set failed: %s", + e, + exc_info=LOG.isEnabledFor(logging.DEBUG) and CFN_VERBOSE_ERRORS, ) new_stack_status = StackStatus.UPDATE_FAILED if change_set.change_set_type == ChangeSetType.CREATE: diff --git a/tests/aws/services/cloudformation/engine/test_mappings.py b/tests/aws/services/cloudformation/engine/test_mappings.py index 0e5f13f2a3288..2b6a42dc4e5c6 100644 --- a/tests/aws/services/cloudformation/engine/test_mappings.py +++ b/tests/aws/services/cloudformation/engine/test_mappings.py @@ -70,7 +70,7 @@ def test_mapping_with_nonexisting_key(self, aws_client, cleanups, snapshot): ) snapshot.match("mapping_nonexisting_key_exc", e.value.response) - @skip_if_v2_provider(reason="CFNV2:Validation") + @skip_if_v2_provider(reason="CFNV2:Validation replaced with v2 test below") @markers.aws.only_localstack def test_async_mapping_error_first_level(self, deploy_cfn_template): """ @@ -119,7 +119,7 @@ def test_async_mapping_error_first_level_v2(self, aws_client, snapshot): snapshot.match("error", exc_info.value) - @skip_if_v2_provider(reason="CFNV2:Validation") + @skip_if_v2_provider(reason="CFNV2:Validation replaced with v2 test below") @markers.aws.only_localstack def test_async_mapping_error_second_level(self, deploy_cfn_template): """ diff --git a/tests/aws/services/cloudformation/test_template_engine.py b/tests/aws/services/cloudformation/test_template_engine.py index fd8b374f13042..28b46290b459b 100644 --- a/tests/aws/services/cloudformation/test_template_engine.py +++ b/tests/aws/services/cloudformation/test_template_engine.py @@ -862,7 +862,7 @@ def test_scope_order_and_parameters( ) snapshot.match("processed_template", processed_template) - @skip_if_v2_provider(reason="CFNV2:Validation") + @skip_if_v2_provider(reason="CFNV2:Transform") @markers.aws.validated @markers.snapshot.skip_snapshot_verify( paths=[ @@ -994,7 +994,7 @@ def test_validate_lambda_internals( processed_template["TemplateBody"]["Resources"]["Parameter"]["Properties"]["Value"], ) - @skip_if_v2_provider(reason="CFNV2:Validation") + @skip_if_v2_provider(reason="CFNV2:Transform") @markers.aws.validated def test_to_validate_template_limit_for_macro( self, deploy_cfn_template, create_lambda_function, snapshot, aws_client @@ -1047,7 +1047,7 @@ def test_to_validate_template_limit_for_macro( ) snapshot.match("error_response", response) - @skip_if_v2_provider(reason="CFNV2:Validation") + @skip_if_v2_provider(reason="CFNV2:Transform") @markers.aws.validated def test_error_pass_macro_as_reference(self, snapshot, aws_client): """ @@ -1120,7 +1120,7 @@ def test_functions_and_references_during_transformation( processed_template["TemplateBody"]["Resources"]["Parameter"]["Properties"]["Value"], ) - @skip_if_v2_provider(reason="CFNV2:Validation") + @skip_if_v2_provider(reason="CFNV2:Transform") @pytest.mark.parametrize( "macro_function", [ @@ -1227,7 +1227,6 @@ def test_pyplate_param_type_list(self, deploy_cfn_template, aws_client, snapshot class TestStackEvents: - @skip_if_v2_provider(reason="CFNV2:Validation") @markers.aws.validated @markers.snapshot.skip_snapshot_verify( paths=[ From aaacd07cf2a6a7a7ac93e9fd0d025cc0c4ba01a3 Mon Sep 17 00:00:00 2001 From: Simon Walker Date: Thu, 7 Aug 2025 16:38:00 +0100 Subject: [PATCH 08/17] Hide exceptions unless CFN_VERBOSE_ERRORS is passed --- .../services/cloudformation/resource_provider.py | 6 +++--- .../services/cloudformation/v2/provider.py | 16 +++++++++++----- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/localstack-core/localstack/services/cloudformation/resource_provider.py b/localstack-core/localstack/services/cloudformation/resource_provider.py index 54b6b64f4597a..169b7c33a30da 100644 --- a/localstack-core/localstack/services/cloudformation/resource_provider.py +++ b/localstack-core/localstack/services/cloudformation/resource_provider.py @@ -236,7 +236,7 @@ def get_resource_type(resource: dict) -> str: LOG.warning( "Failed to retrieve resource type %s", resource.get("Type"), - exc_info=LOG.isEnabledFor(logging.DEBUG), + exc_info=LOG.isEnabledFor(logging.DEBUG) and config.CFN_VERBOSE_ERRORS, ) @@ -577,7 +577,7 @@ def try_load_resource_provider(resource_type: str) -> ResourceProvider | None: LOG.warning( "Failed to load PRO resource type %s as a ResourceProvider.", resource_type, - exc_info=LOG.isEnabledFor(logging.DEBUG), + exc_info=LOG.isEnabledFor(logging.DEBUG) and config.CFN_VERBOSE_ERRORS, ) # 2. try to load community resource provider @@ -592,7 +592,7 @@ def try_load_resource_provider(resource_type: str) -> ResourceProvider | None: LOG.warning( "Failed to load community resource type %s as a ResourceProvider.", resource_type, - exc_info=LOG.isEnabledFor(logging.DEBUG), + exc_info=LOG.isEnabledFor(logging.DEBUG) and config.CFN_VERBOSE_ERRORS, ) # we could not find the resource provider diff --git a/localstack-core/localstack/services/cloudformation/v2/provider.py b/localstack-core/localstack/services/cloudformation/v2/provider.py index 5d7e407a31013..0ef9bdeedc5a4 100644 --- a/localstack-core/localstack/services/cloudformation/v2/provider.py +++ b/localstack-core/localstack/services/cloudformation/v2/provider.py @@ -5,6 +5,7 @@ from datetime import UTC, datetime from typing import Any +from localstack import config from localstack.aws.api import RequestContext, handler from localstack.aws.api.cloudformation import ( AlreadyExistsException, @@ -78,7 +79,6 @@ Stack as ApiStack, ) from localstack.aws.connect import connect_to -from localstack.config import CFN_VERBOSE_ERRORS from localstack.services.cloudformation import api_utils from localstack.services.cloudformation.engine import template_preparer from localstack.services.cloudformation.engine.parameters import resolve_ssm_parameter @@ -536,7 +536,7 @@ def _run(*args): LOG.error( "Execute change set failed: %s", e, - exc_info=LOG.isEnabledFor(logging.DEBUG) and CFN_VERBOSE_ERRORS, + exc_info=LOG.isEnabledFor(logging.DEBUG) and config.CFN_VERBOSE_ERRORS, ) new_stack_status = StackStatus.UPDATE_FAILED if change_set.change_set_type == ChangeSetType.CREATE: @@ -741,7 +741,9 @@ def _run(*args): stack.resolved_parameters = change_set.resolved_parameters except Exception as e: LOG.error( - "Create Stack set failed: %s", e, exc_info=LOG.isEnabledFor(logging.WARNING) + "Create Stack set failed: %s", + e, + exc_info=LOG.isEnabledFor(logging.WARNING) and config.CFN_VERBOSE_ERRORS, ) stack.set_stack_status(StackStatus.CREATE_FAILED) @@ -1371,7 +1373,11 @@ def _run(*args): stack.template_body = change_set.template_body stack.resolved_parameters = change_set.resolved_parameters except Exception as e: - LOG.error("Update Stack failed: %s", e, exc_info=LOG.isEnabledFor(logging.WARNING)) + LOG.error( + "Update Stack failed: %s", + e, + exc_info=LOG.isEnabledFor(logging.WARNING) and config.CFN_VERBOSE_ERRORS, + ) stack.set_stack_status(StackStatus.UPDATE_FAILED) start_worker_thread(_run) @@ -1434,7 +1440,7 @@ def _run(*args): "Failed to delete stack '%s': %s", stack.stack_name, e, - exc_info=LOG.isEnabledFor(logging.DEBUG), + exc_info=LOG.isEnabledFor(logging.DEBUG) and config.CFN_VERBOSE_ERRORS, ) stack.set_stack_status(StackStatus.DELETE_FAILED) From 0a2ff85702b16dd4e4b2f7139f0ca765d3c3ab1d Mon Sep 17 00:00:00 2001 From: Simon Walker Date: Thu, 7 Aug 2025 16:56:01 +0100 Subject: [PATCH 09/17] Fix vpc endpoint test --- .../cloudformation/engine/v2/change_set_model_preproc.py | 3 ++- .../resource_providers/ec2/test_ec2_resource_provider.py | 2 -- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_preproc.py b/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_preproc.py index 729716e0da913..f925fd75cda39 100644 --- a/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_preproc.py +++ b/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_preproc.py @@ -4,6 +4,7 @@ import copy import re from collections.abc import Callable +from datetime import datetime from typing import Any, Final, Generic, TypeVar from botocore.exceptions import ClientError @@ -258,7 +259,7 @@ def _deployed_property_value_of( property_value: Any | None = get_value_from_path(properties, property_name) if property_value: - if not isinstance(property_value, str): + if not isinstance(property_value, (datetime, str)): # TODO: is this correct? If there is a bug in the logic here, it's probably # better to know about it with a clear error message than to receive some form # of message about trying to use a dictionary in place of a string diff --git a/tests/aws/services/cloudformation/resource_providers/ec2/test_ec2_resource_provider.py b/tests/aws/services/cloudformation/resource_providers/ec2/test_ec2_resource_provider.py index 4c7d235c2fd15..27ab4343845d0 100644 --- a/tests/aws/services/cloudformation/resource_providers/ec2/test_ec2_resource_provider.py +++ b/tests/aws/services/cloudformation/resource_providers/ec2/test_ec2_resource_provider.py @@ -3,7 +3,6 @@ import pytest from botocore.exceptions import ClientError from localstack_snapshot.snapshots.transformer import SortingTransformer -from tests.aws.services.cloudformation.conftest import skip_if_v2_provider from localstack.testing.pytest import markers @@ -78,7 +77,6 @@ def test_deploy_security_group_with_tags(deploy_cfn_template, aws_client, snapsh snapshot.match("security-group", security_group) -@skip_if_v2_provider(reason="CFNV2:ResourceProvider(AWS::EC2::VPCEndpoint)") @markers.aws.validated @markers.snapshot.skip_snapshot_verify( paths=[ From 556951903ab9568b5d0b021fee86ad2f973f94f3 Mon Sep 17 00:00:00 2001 From: Simon Walker Date: Thu, 7 Aug 2025 18:12:32 +0100 Subject: [PATCH 10/17] Add v1 skips --- tests/aws/services/cloudformation/engine/test_mappings.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/aws/services/cloudformation/engine/test_mappings.py b/tests/aws/services/cloudformation/engine/test_mappings.py index 2b6a42dc4e5c6..9a2ca50ef5fa4 100644 --- a/tests/aws/services/cloudformation/engine/test_mappings.py +++ b/tests/aws/services/cloudformation/engine/test_mappings.py @@ -2,7 +2,7 @@ import pytest from botocore.exceptions import ClientError -from tests.aws.services.cloudformation.conftest import skip_if_v2_provider +from tests.aws.services.cloudformation.conftest import skip_if_v1_provider, skip_if_v2_provider from localstack.testing.pytest import markers from localstack.testing.pytest.fixtures import StackDeployError @@ -94,6 +94,7 @@ def test_async_mapping_error_first_level(self, deploy_cfn_template): assert "Cannot find map key 'C' in mapping 'TopicSuffixMap'" in str(exc_info.value) @markers.aws.validated + @skip_if_v1_provider(reason="V1 provider is not in parity with AWS") def test_async_mapping_error_first_level_v2(self, aws_client, snapshot): snapshot.add_transformer(snapshot.transform.cloudformation_api()) topic_name = f"test-topic-{short_uid()}" @@ -145,6 +146,7 @@ def test_async_mapping_error_second_level(self, deploy_cfn_template): ) @markers.aws.validated + @skip_if_v1_provider(reason="V1 provider is not in parity with AWS") def test_async_mapping_error_second_level_v2(self, aws_client, snapshot): """ Similar to the `test_async_mapping_error_first_level` test above, but From 56fedb67ce6ee1366df94fe3aa5384966718545b Mon Sep 17 00:00:00 2001 From: Simon Walker Date: Thu, 7 Aug 2025 22:08:30 +0100 Subject: [PATCH 11/17] Fix visiting in validator --- .../engine/v2/change_set_model_preproc.py | 3 +- .../engine/v2/change_set_model_validator.py | 31 ++++++++++--------- .../services/cloudformation/v2/provider.py | 7 ++++- 3 files changed, 25 insertions(+), 16 deletions(-) diff --git a/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_preproc.py b/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_preproc.py index f45d8ec53da46..7c6ef16f99a6a 100644 --- a/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_preproc.py +++ b/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_preproc.py @@ -243,7 +243,8 @@ def _get_node_property_for( def _deployed_property_value_of( self, resource_logical_id: str, property_name: str, resolved_resources: dict ) -> Any: - # TODO: typing around resolved resources is needed and should be reflected here. + # We have to override this function to make sure it does not try to access the + # resolved resource # Before we can obtain deployed value for a resource, we need to first ensure to # process the resource if this wasn't processed already. Ideally, values should only diff --git a/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_validator.py b/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_validator.py index eeffa7405a1aa..ab8e44948a2bf 100644 --- a/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_validator.py +++ b/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_validator.py @@ -1,13 +1,14 @@ from localstack.services.cloudformation.engine.v2.change_set_model import ( - NodeParameters, + Maybe, + NodeIntrinsicFunction, NodeTemplate, + Nothing, is_nothing, ) from localstack.services.cloudformation.engine.v2.change_set_model_preproc import ( ChangeSetModelPreproc, PreprocEntityDelta, ) -from localstack.services.cloudformation.engine.validations import ValidationError class ChangeSetModelValidator(ChangeSetModelPreproc): @@ -15,20 +16,22 @@ def validate(self): self.process() def visit_node_template(self, node_template: NodeTemplate): - self.visit(node_template.parameters) self.visit(node_template.mappings) self.visit(node_template.resources) - def visit_node_parameters(self, node_parameters: NodeParameters) -> PreprocEntityDelta: - # check that all parameters have values - invalid_parameters = [] - for node_parameter in node_parameters.parameters: - parameter_value = self.visit(node_parameter) - if is_nothing(parameter_value.before) and is_nothing(parameter_value.after): - invalid_parameters.append(node_parameter.name) + def visit_node_intrinsic_function_fn_get_att( + self, node_intrinsic_function: NodeIntrinsicFunction + ) -> PreprocEntityDelta: + arguments_delta = self.visit(node_intrinsic_function.arguments) + before_arguments: Maybe[str | list[str]] = arguments_delta.before + after_arguments: Maybe[str | list[str]] = arguments_delta.after - if invalid_parameters: - raise ValidationError(f"Parameters: [{','.join(invalid_parameters)}] must have values") + before = self._before_cache.get(node_intrinsic_function.scope, Nothing) + if is_nothing(before) and not is_nothing(before_arguments): + before = ".".join(before_arguments) - # continue visiting - return super().visit_node_parameters(node_parameters) + after = self._after_cache.get(node_intrinsic_function.scope, Nothing) + if is_nothing(after) and not is_nothing(after_arguments): + after = ".".join(after_arguments) + + return PreprocEntityDelta(before=before, after=after) diff --git a/localstack-core/localstack/services/cloudformation/v2/provider.py b/localstack-core/localstack/services/cloudformation/v2/provider.py index 0ef9bdeedc5a4..1299c73e4c284 100644 --- a/localstack-core/localstack/services/cloudformation/v2/provider.py +++ b/localstack-core/localstack/services/cloudformation/v2/provider.py @@ -215,6 +215,7 @@ def _resolve_parameters( ) -> dict[str, EngineParameter]: template_parameters = template.get("Parameters", {}) resolved_parameters = {} + invalid_parameters = [] for name, parameter in template_parameters.items(): given_value = parameters.get(name) default_value = parameter.get("Default") @@ -233,10 +234,14 @@ def _resolve_parameters( f"Parameter {name} should either have input value or default value" ) elif given_value is None and default_value is None: - raise ValidationError(f"Parameters: [{name}] must have values") + invalid_parameters.append(name) + continue resolved_parameters[name] = resolved_parameter + if invalid_parameters: + raise ValidationError(f"Parameters: [{','.join(invalid_parameters)}] must have values") + for name, parameter in resolved_parameters.items(): if ( parameter.get("resolved_value") is None From 9101dacdaa63981a63823867526a597c102607f1 Mon Sep 17 00:00:00 2001 From: Simon Walker Date: Thu, 7 Aug 2025 22:52:26 +0100 Subject: [PATCH 12/17] Fix top level key lookup --- .../cloudformation/engine/v2/change_set_model_preproc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_preproc.py b/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_preproc.py index 7c6ef16f99a6a..7d53269a19dc0 100644 --- a/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_preproc.py +++ b/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_preproc.py @@ -381,7 +381,7 @@ def _resolve_mapping( error_key = "::".join([map_name, top_level_key, second_level_key]) raise ValidationError(f"Template error: Unable to get mapping for {error_key}") second_level_value = top_level_value.bindings.get(second_level_key) - if not isinstance(second_level_value, NodeObject): + if not isinstance(second_level_value, TerminalValue): error_key = "::".join([map_name, top_level_key, second_level_key]) raise ValidationError(f"Template error: Unable to get mapping for {error_key}") mapping_value_delta = self.visit(second_level_value) From 6c03e4c55085e137698b2822197334fb8c4b6da0 Mon Sep 17 00:00:00 2001 From: Simon Walker Date: Thu, 7 Aug 2025 23:11:52 +0100 Subject: [PATCH 13/17] Fix validator to not evaluate deployed resource --- .../engine/v2/change_set_model_validator.py | 104 ++++++++++++++++++ 1 file changed, 104 insertions(+) diff --git a/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_validator.py b/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_validator.py index ab8e44948a2bf..e8248d05e39f6 100644 --- a/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_validator.py +++ b/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_validator.py @@ -1,3 +1,6 @@ +import re +from typing import Any + from localstack.services.cloudformation.engine.v2.change_set_model import ( Maybe, NodeIntrinsicFunction, @@ -6,8 +9,10 @@ is_nothing, ) from localstack.services.cloudformation.engine.v2.change_set_model_preproc import ( + _PSEUDO_PARAMETERS, ChangeSetModelPreproc, PreprocEntityDelta, + PreprocResource, ) @@ -35,3 +40,102 @@ def visit_node_intrinsic_function_fn_get_att( after = ".".join(after_arguments) return PreprocEntityDelta(before=before, after=after) + + def visit_node_intrinsic_function_fn_sub( + self, node_intrinsic_function: NodeIntrinsicFunction + ) -> PreprocEntityDelta: + def _compute_sub(args: str | list[Any], select_before: bool) -> str: + # TODO: add further schema validation. + string_template: str + sub_parameters: dict + if isinstance(args, str): + string_template = args + sub_parameters = dict() + elif ( + isinstance(args, list) + and len(args) == 2 + and isinstance(args[0], str) + and isinstance(args[1], dict) + ): + string_template = args[0] + sub_parameters = args[1] + else: + raise RuntimeError( + "Invalid arguments shape for Fn::Sub, expected a String " + f"or a Tuple of String and Map but got '{args}'" + ) + sub_string = string_template + template_variable_names = re.findall("\\${([^}]+)}", string_template) + for template_variable_name in template_variable_names: + template_variable_value = Nothing + + # Try to resolve the variable name as pseudo parameter. + if template_variable_name in _PSEUDO_PARAMETERS: + template_variable_value = self._resolve_pseudo_parameter( + pseudo_parameter_name=template_variable_name + ) + + # Try to resolve the variable name as an entry to the defined parameters. + elif template_variable_name in sub_parameters: + template_variable_value = sub_parameters[template_variable_name] + + # Try to resolve the variable name as GetAtt. + elif "." in template_variable_name: + try: + template_variable_value = self._resolve_attribute( + arguments=template_variable_name, select_before=select_before + ) + except RuntimeError: + pass + + # Try to resolve the variable name as Ref. + else: + try: + resource_delta = self._resolve_reference(logical_id=template_variable_name) + template_variable_value = ( + resource_delta.before if select_before else resource_delta.after + ) + if isinstance(template_variable_value, PreprocResource): + template_variable_value = template_variable_value.physical_resource_id + except RuntimeError: + pass + + if is_nothing(template_variable_value): + # override the base method just for this line to prevent accessing the + # resource properties since we are not deploying any resources + template_variable_value = "" + + if not isinstance(template_variable_value, str): + template_variable_value = str(template_variable_value) + + sub_string = sub_string.replace( + f"${{{template_variable_name}}}", template_variable_value + ) + + # FIXME: the following type reduction is ported from v1; however it appears as though such + # reduction is not performed by the engine, and certainly not at this depth given the + # lack of context. This section should be removed with Fn::Sub always retuning a string + # and the resource providers reviewed. + account_id = self._change_set.account_id + is_another_account_id = sub_string.isdigit() and len(sub_string) == len(account_id) + if sub_string == account_id or is_another_account_id: + result = sub_string + elif sub_string.isdigit(): + result = int(sub_string) + else: + try: + result = float(sub_string) + except ValueError: + result = sub_string + return result + + arguments_delta = self.visit(node_intrinsic_function.arguments) + arguments_before = arguments_delta.before + arguments_after = arguments_delta.after + before = self._before_cache.get(node_intrinsic_function.scope, Nothing) + if is_nothing(before) and not is_nothing(arguments_before): + before = _compute_sub(args=arguments_before, select_before=True) + after = self._after_cache.get(node_intrinsic_function.scope, Nothing) + if is_nothing(after) and not is_nothing(arguments_after): + after = _compute_sub(args=arguments_after, select_before=False) + return PreprocEntityDelta(before=before, after=after) From 34a28a27151db3d097c357fba5d5a81148c710e7 Mon Sep 17 00:00:00 2001 From: Simon Walker Date: Fri, 8 Aug 2025 11:48:59 +0100 Subject: [PATCH 14/17] CFNV2: fix delete change set --- .../services/cloudformation/v2/provider.py | 17 +++++------------ .../cloudformation/api/test_changesets.py | 1 - 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/localstack-core/localstack/services/cloudformation/v2/provider.py b/localstack-core/localstack/services/cloudformation/v2/provider.py index 133defda8171b..1571a13cdbe47 100644 --- a/localstack-core/localstack/services/cloudformation/v2/provider.py +++ b/localstack-core/localstack/services/cloudformation/v2/provider.py @@ -173,7 +173,7 @@ def find_change_set_v2( state: CloudFormationStore, change_set_name: str, stack_name: str | None = None ) -> ChangeSet | None: if is_changeset_arn(change_set_name): - return state.change_sets[change_set_name] + return state.change_sets.get(change_set_name) else: if stack_name is not None: stack = find_stack_v2(state, stack_name) @@ -185,7 +185,9 @@ def find_change_set_v2( if change_set_candidate.change_set_name == change_set_name: return change_set_candidate else: - raise ValueError("No stack name specified when finding change set") + raise ValidationError( + "StackName must be specified if ChangeSetName is not specified as an ARN." + ) def find_stack_set_v2(state: CloudFormationStore, stack_set_name: str) -> StackSet | None: @@ -618,16 +620,7 @@ def delete_change_set( **kwargs, ) -> DeleteChangeSetOutput: state = get_cloudformation_store(context.account_id, context.region) - - if is_changeset_arn(change_set_name): - change_set = state.change_sets.get(change_set_name) - elif not is_changeset_arn(change_set_name) and stack_name: - change_set = find_change_set_v2(state, change_set_name, stack_name) - else: - raise ValidationError( - "StackName must be specified if ChangeSetName is not specified as an ARN." - ) - + change_set = find_change_set_v2(state, change_set_name, stack_name) if not change_set: return DeleteChangeSetOutput() diff --git a/tests/aws/services/cloudformation/api/test_changesets.py b/tests/aws/services/cloudformation/api/test_changesets.py index 2524d44d64dd7..f1ae38b8a8f04 100644 --- a/tests/aws/services/cloudformation/api/test_changesets.py +++ b/tests/aws/services/cloudformation/api/test_changesets.py @@ -848,7 +848,6 @@ def _check_changeset_success(): snapshot.match("error_execute_failed", e.value) -@skip_if_v2_provider(reason="CFNV2:DeleteChangeSet") @markers.aws.validated def test_deleted_changeset(snapshot, cleanups, aws_client): """simple case verifying that proper exception is thrown when trying to get a deleted changeset""" From 48ab7205eae6dc533349842ab6e42a9701548fa0 Mon Sep 17 00:00:00 2001 From: Simon Walker Date: Fri, 8 Aug 2025 12:06:49 +0100 Subject: [PATCH 15/17] Move `ChangeSetModelExecutorResult` to entities.py --- .../engine/v2/change_set_model_executor.py | 14 ++++------ .../services/cloudformation/v2/entities.py | 26 ++++++++++++------- 2 files changed, 22 insertions(+), 18 deletions(-) diff --git a/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_executor.py b/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_executor.py index e26f5a988abec..e32fbd3755402 100644 --- a/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_executor.py +++ b/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_executor.py @@ -1,7 +1,6 @@ import copy import logging import uuid -from dataclasses import dataclass from datetime import UTC, datetime from typing import Final, Protocol @@ -39,7 +38,11 @@ ResourceProviderExecutor, ResourceProviderPayload, ) -from localstack.services.cloudformation.v2.entities import ChangeSet, ResolvedResource +from localstack.services.cloudformation.v2.entities import ( + ChangeSet, + ChangeSetModelExecutorResult, + ResolvedResource, +) from localstack.utils.urls import localstack_host LOG = logging.getLogger(__name__) @@ -47,13 +50,6 @@ EventOperationFromAction = {"Add": "CREATE", "Modify": "UPDATE", "Remove": "DELETE"} -@dataclass -class ChangeSetModelExecutorResult: - resources: dict[str, ResolvedResource] - outputs: dict - exports: dict - - class DeferredAction(Protocol): def __call__(self) -> None: ... diff --git a/localstack-core/localstack/services/cloudformation/v2/entities.py b/localstack-core/localstack/services/cloudformation/v2/entities.py index 88517eb1276c8..e210c0512f88f 100644 --- a/localstack-core/localstack/services/cloudformation/v2/entities.py +++ b/localstack-core/localstack/services/cloudformation/v2/entities.py @@ -1,3 +1,4 @@ +from dataclasses import dataclass from datetime import UTC, datetime from typing import NotRequired, TypedDict @@ -39,6 +40,22 @@ from localstack.utils.strings import long_uid, short_uid +class ResolvedResource(TypedDict): + LogicalResourceId: str + Type: str + Properties: dict + ResourceStatus: ResourceStatus + PhysicalResourceId: str | None + LastUpdatedTimestamp: datetime | None + + +@dataclass +class ChangeSetModelExecutorResult: + resources: dict[str, ResolvedResource] + outputs: dict + exports: dict + + # TODO: turn into class/dataclass class EngineParameter(TypedDict): """ @@ -51,15 +68,6 @@ class EngineParameter(TypedDict): default_value: NotRequired[str | None] -class ResolvedResource(TypedDict): - LogicalResourceId: str - Type: str - Properties: dict - ResourceStatus: ResourceStatus - PhysicalResourceId: str | None - LastUpdatedTimestamp: datetime | None - - class Stack: stack_name: str parameters: list[ApiParameter] From 0ecd24bdecddf0accb81f0f69920180478087693 Mon Sep 17 00:00:00 2001 From: Simon Walker Date: Fri, 8 Aug 2025 12:06:49 +0100 Subject: [PATCH 16/17] Add changeset helper for updating stack --- .../services/cloudformation/v2/entities.py | 19 ++++++++++++ .../services/cloudformation/v2/provider.py | 31 ++----------------- 2 files changed, 22 insertions(+), 28 deletions(-) diff --git a/localstack-core/localstack/services/cloudformation/v2/entities.py b/localstack-core/localstack/services/cloudformation/v2/entities.py index e210c0512f88f..26c45e7b2b942 100644 --- a/localstack-core/localstack/services/cloudformation/v2/entities.py +++ b/localstack-core/localstack/services/cloudformation/v2/entities.py @@ -82,6 +82,7 @@ class Stack: enable_termination_protection: bool processed_template: dict | None template_body: str | None + template: dict | None # state after deploy resolved_parameters: dict[str, str] @@ -96,6 +97,7 @@ def __init__( region_name: str, request_payload: CreateChangeSetInput | CreateStackInput, initial_status: StackStatus = StackStatus.CREATE_IN_PROGRESS, + template: dict | None = None, ): self.account_id = account_id self.region_name = region_name @@ -108,6 +110,7 @@ def __init__( self.enable_termination_protection = False self.processed_template = None self.template_body = None + self.template = template self.stack_name = request_payload["StackName"] self.parameters = request_payload.get("Parameters", []) @@ -291,6 +294,22 @@ def __init__( ) self.processed_template = None + def propagate_state_to_stack( + self, result: ChangeSetModelExecutorResult, new_stack_status: StackStatus + ): + self.stack.set_stack_status(new_stack_status) + self.set_execution_status(ExecutionStatus.EXECUTE_COMPLETE) + self.stack.resolved_resources = result.resources + self.stack.resolved_parameters = self.resolved_parameters + self.stack.resolved_outputs = result.outputs + self.stack.resolved_exports = result.exports + + # if the deployment succeeded, update the stack's template representation to that + # which was just deployed + self.stack.template = self.template + self.stack.processed_template = self.processed_template + self.stack.template_body = self.template_body + def set_update_model(self, update_model: UpdateModel) -> None: self.update_model = update_model diff --git a/localstack-core/localstack/services/cloudformation/v2/provider.py b/localstack-core/localstack/services/cloudformation/v2/provider.py index 79df1be18597e..34bf992e76b81 100644 --- a/localstack-core/localstack/services/cloudformation/v2/provider.py +++ b/localstack-core/localstack/services/cloudformation/v2/provider.py @@ -534,18 +534,7 @@ def _run(*args): new_stack_status = StackStatus.UPDATE_COMPLETE if change_set.change_set_type == ChangeSetType.CREATE: new_stack_status = StackStatus.CREATE_COMPLETE - change_set.stack.set_stack_status(new_stack_status) - change_set.set_execution_status(ExecutionStatus.EXECUTE_COMPLETE) - change_set.stack.resolved_resources = result.resources - change_set.stack.resolved_parameters = change_set.resolved_parameters - change_set.stack.resolved_outputs = result.outputs - change_set.stack.resolved_exports = result.exports - - # if the deployment succeeded, update the stack's template representation to that - # which was just deployed - change_set.stack.template = change_set.template - change_set.stack.processed_template = change_set.processed_template - change_set.stack.template_body = change_set.template_body + change_set.propagate_state_to_stack(result, new_stack_status) except Exception as e: LOG.error( "Execute change set failed: %s", @@ -736,14 +725,7 @@ def create_stack(self, context: RequestContext, request: CreateStackInput) -> Cr def _run(*args): try: result = change_set_executor.execute() - stack.set_stack_status(StackStatus.CREATE_COMPLETE) - stack.resolved_resources = result.resources - stack.resolved_outputs = result.outputs - # if the deployment succeeded, update the stack's template representation to that - # which was just deployed - stack.template = change_set.template - stack.template_body = change_set.template_body - stack.resolved_parameters = change_set.resolved_parameters + change_set.propagate_state_to_stack(result, StackStatus.CREATE_COMPLETE) except Exception as e: LOG.error( "Create Stack set failed: %s", @@ -1371,14 +1353,7 @@ def update_stack( def _run(*args): try: result = change_set_executor.execute() - stack.set_stack_status(StackStatus.UPDATE_COMPLETE) - stack.resolved_resources = result.resources - stack.resolved_outputs = result.outputs - # if the deployment succeeded, update the stack's template representation to that - # which was just deployed - stack.template = change_set.template - stack.template_body = change_set.template_body - stack.resolved_parameters = change_set.resolved_parameters + change_set.propagate_state_to_stack(result, StackStatus.UPDATE_COMPLETE) except Exception as e: LOG.error( "Update Stack failed: %s", From df28764fdbad35a621a1a85fc2afe71c13ce44c3 Mon Sep 17 00:00:00 2001 From: Simon Walker Date: Fri, 8 Aug 2025 12:06:49 +0100 Subject: [PATCH 17/17] WIP --- .../engine/v2/change_set_model_executor.py | 38 ++++++++++++++++++- .../engine/v2/change_set_model_validator.py | 14 +++++++ .../cloudformation/resource_provider.py | 4 +- .../services/cloudformation/v2/provider.py | 7 +++- .../cloudformation/api/test_nested_stacks.py | 8 ++-- .../api/test_nested_stacks.snapshot.json | 2 +- .../api/test_nested_stacks.validation.json | 8 +++- 7 files changed, 70 insertions(+), 11 deletions(-) diff --git a/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_executor.py b/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_executor.py index e32fbd3755402..e39df71f146b7 100644 --- a/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_executor.py +++ b/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_executor.py @@ -22,6 +22,7 @@ TerminalValueModified, TerminalValueUnchanged, is_nothing, + ChangeType, ) from localstack.services.cloudformation.engine.v2.change_set_model_preproc import ( MOCKED_REFERENCE, @@ -50,6 +51,21 @@ EventOperationFromAction = {"Add": "CREATE", "Modify": "UPDATE", "Remove": "DELETE"} +class ResourceDeployError(Exception): + def __init__( + self, + resource_name: str, + before: PreprocResource | None, + after: PreprocResource | None, + event: ProgressEvent, + ): + self.resource_name = resource_name + self.before = before + self.after = after + self.event = event + super().__init__(f"Failed to deploy resource {resource_name}: {event.message}") + + class DeferredAction(Protocol): def __call__(self) -> None: ... @@ -233,9 +249,10 @@ def visit_node_output( def _execute_resource_change( self, name: str, before: PreprocResource | None, after: PreprocResource | None - ) -> None: + ) -> ProgressEvent: # Changes are to be made about this resource. # TODO: this logic is a POC and should be revised. + event: ProgressEvent | None = None if not is_nothing(before) and not is_nothing(after): # Case: change on same type. if before.resource_type == after.resource_type: @@ -507,6 +524,25 @@ def _execute_resource_action( "Resource provider operation failed: '%s'", reason, ) + if action != ChangeAction.Remove: + status_from_action = EventOperationFromAction[action.value] + physical_resource_id = ( + extra_resource_properties["PhysicalResourceId"] + if resource_provider + else MOCKED_REFERENCE + ) + resolved_resource = ResolvedResource( + Properties=event.resource_model, + LogicalResourceId=logical_resource_id, + Type=resource_type, + LastUpdatedTimestamp=datetime.now(UTC), + ResourceStatus=ResourceStatus(f"{status_from_action}_FAILED"), + PhysicalResourceId=physical_resource_id, + ) + # TODO: do we actually need this line? + resolved_resource.update(extra_resource_properties) + + self.resources[logical_resource_id] = resolved_resource case other: raise NotImplementedError(f"Event status '{other}' not handled") return event diff --git a/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_validator.py b/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_validator.py index e8248d05e39f6..470602acddf87 100644 --- a/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_validator.py +++ b/localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_validator.py @@ -1,4 +1,5 @@ import re +import logging from typing import Any from localstack.services.cloudformation.engine.v2.change_set_model import ( @@ -7,6 +8,7 @@ NodeTemplate, Nothing, is_nothing, + NodeResources, ) from localstack.services.cloudformation.engine.v2.change_set_model_preproc import ( _PSEUDO_PARAMETERS, @@ -14,6 +16,9 @@ PreprocEntityDelta, PreprocResource, ) +from localstack.services.cloudformation.engine.validations import ValidationError + +LOG = logging.getLogger(__name__) class ChangeSetModelValidator(ChangeSetModelPreproc): @@ -24,6 +29,15 @@ def visit_node_template(self, node_template: NodeTemplate): self.visit(node_template.mappings) self.visit(node_template.resources) + def visit_node_resources(self, node_resources: NodeResources): + for node_resource in node_resources.resources: + try: + self.visit(node_resource) + except ValidationError: + raise + except Exception as e: + LOG.warning("Error validating resource '%s': %s", node_resource.name, e) + def visit_node_intrinsic_function_fn_get_att( self, node_intrinsic_function: NodeIntrinsicFunction ) -> PreprocEntityDelta: diff --git a/localstack-core/localstack/services/cloudformation/resource_provider.py b/localstack-core/localstack/services/cloudformation/resource_provider.py index 169b7c33a30da..ac4cdd0fd2809 100644 --- a/localstack-core/localstack/services/cloudformation/resource_provider.py +++ b/localstack-core/localstack/services/cloudformation/resource_provider.py @@ -458,9 +458,7 @@ def deploy_loop( raise match event.status: - case OperationStatus.FAILED: - return event - case OperationStatus.SUCCESS: + case OperationStatus.SUCCESS | OperationStatus.FAILED: if not hasattr(resource_provider, "SCHEMA"): raise Exception( "A ResourceProvider should always have a SCHEMA property defined." diff --git a/localstack-core/localstack/services/cloudformation/v2/provider.py b/localstack-core/localstack/services/cloudformation/v2/provider.py index 34bf992e76b81..b149d85fa95c3 100644 --- a/localstack-core/localstack/services/cloudformation/v2/provider.py +++ b/localstack-core/localstack/services/cloudformation/v2/provider.py @@ -536,7 +536,7 @@ def _run(*args): new_stack_status = StackStatus.CREATE_COMPLETE change_set.propagate_state_to_stack(result, new_stack_status) except Exception as e: - LOG.error( + LOG.warning( "Execute change set failed: %s", e, exc_info=LOG.isEnabledFor(logging.DEBUG) and config.CFN_VERBOSE_ERRORS, @@ -686,6 +686,8 @@ def create_stack(self, context: RequestContext, request: CreateStackInput) -> Cr account_id=context.account_id, region_name=context.region, request_payload=request, + # because this change does not externally involve a change set, we should set the template on the stack + template=structured_template, ) # TODO: what is the correct initial status? state.stacks_v2[stack.stack_id] = stack @@ -718,6 +720,9 @@ def create_stack(self, context: RequestContext, request: CreateStackInput) -> Cr previous_update_model=None, ) + # propagate the processed template to the stack + stack.processed_template = change_set.processed_template + # deployment process stack.set_stack_status(StackStatus.CREATE_IN_PROGRESS) change_set_executor = ChangeSetModelExecutor(change_set) diff --git a/tests/aws/services/cloudformation/api/test_nested_stacks.py b/tests/aws/services/cloudformation/api/test_nested_stacks.py index e09745f9ced23..54a90c5ed4874 100644 --- a/tests/aws/services/cloudformation/api/test_nested_stacks.py +++ b/tests/aws/services/cloudformation/api/test_nested_stacks.py @@ -295,7 +295,6 @@ def test_nested_stacks_conditions(deploy_cfn_template, s3_create_bucket, aws_cli assert ":" not in nested_stack["Stacks"][0]["StackName"] -@skip_if_v2_provider(reason="CFNV2:Deletion") @markers.aws.validated def test_deletion_of_failed_nested_stack(s3_create_bucket, aws_client, region_name, snapshot): """ @@ -341,9 +340,10 @@ def test_deletion_of_failed_nested_stack(s3_create_bucket, aws_client, region_na 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"] + name_ = [stack for stack in stacks if f"{stack_name}-ChildStack-" in stack["StackName"]][0][ + "StackName" + ] + nested_stack_name = name_ aws_client.cloudformation.delete_stack(StackName=stack_name) aws_client.cloudformation.get_waiter("stack_delete_complete").wait(StackName=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 fbd1c318a283b..ddfaa950f873c 100644 --- a/tests/aws/services/cloudformation/api/test_nested_stacks.snapshot.json +++ b/tests/aws/services/cloudformation/api/test_nested_stacks.snapshot.json @@ -65,7 +65,7 @@ } }, "tests/aws/services/cloudformation/api/test_nested_stacks.py::test_deletion_of_failed_nested_stack": { - "recorded-date": "17-09-2024, 20:09:36", + "recorded-date": "08-08-2025, 13:12:13", "recorded-content": { "error": { "Error": { 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 f5936f2e379e7..441dbacef4d9d 100644 --- a/tests/aws/services/cloudformation/api/test_nested_stacks.validation.json +++ b/tests/aws/services/cloudformation/api/test_nested_stacks.validation.json @@ -1,6 +1,12 @@ { "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" + "last_validated_date": "2025-08-08T13:12:13+00:00", + "durations_in_seconds": { + "setup": 1.0, + "call": 63.28, + "teardown": 0.72, + "total": 65.0 + } }, "tests/aws/services/cloudformation/api/test_nested_stacks.py::test_nested_output_in_params": { "last_validated_date": "2023-02-07T09:57:47+00:00"