Skip to content
This repository was archived by the owner on Mar 23, 2026. It is now read-only.

Commit eae4b84

Browse files
sodreclaude
sodre
and
claude
committed
Address review feedback: trim comments, consolidate test, add AWS validation
- Remove test_delete_stack_with_conditional_getatt (passed without fix) - Rename and repurpose test to verify template acceptance with conditions - Use snapshot matching instead of manual assertions - Delete unused conditional-getatt-delete.yaml template - Add AWS-validated snapshot and validation data Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 10145b3 commit eae4b84

5 files changed

Lines changed: 50 additions & 104 deletions

File tree

‎localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_preproc.py‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1264,20 +1264,15 @@ def visit_node_resource(
12641264

12651265
type_delta = self.visit(node_resource.type_)
12661266

1267-
# Determine if this resource should be processed for before/after states.
1268-
# A resource is "active" if it has no condition or its condition evaluates to true.
1269-
# We must check this BEFORE visiting properties to avoid resolving references
1270-
# (like GetAtt) to other conditional resources that may not exist.
1267+
# Check conditions before visiting properties to avoid resolving references
1268+
# (e.g. GetAtt) to conditional resources that were never created.
12711269
should_process_before = change_type != ChangeType.CREATED and (
12721270
is_nothing(condition_before) or condition_before
12731271
)
12741272
should_process_after = change_type != ChangeType.REMOVED and (
12751273
is_nothing(condition_after) or condition_after
12761274
)
12771275

1278-
# Only visit properties if at least one state needs to be processed.
1279-
# This prevents evaluating GetAtt/Ref inside conditional resources whose
1280-
# conditions are false (and thus were never created).
12811276
properties_delta: PreprocEntityDelta[PreprocProperties, PreprocProperties]
12821277
if should_process_before or should_process_after:
12831278
properties_delta = self.visit(node_resource.properties)

‎tests/aws/services/cloudformation/engine/test_conditions.py‎

Lines changed: 14 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -555,55 +555,25 @@ def test_references_to_disabled_resources(
555555

556556
@markers.aws.validated
557557
@skip_if_legacy_engine()
558-
def test_delete_stack_with_conditional_getatt(self, deploy_cfn_template, aws_client):
559-
"""
560-
Test that stack deletion works when a resource references a conditional resource
561-
via Fn::If that was never created (condition was false).
562-
563-
Reproduces: https://github.com/localstack/localstack/issues/13609
564-
"""
565-
stack = deploy_cfn_template(
566-
template_path=os.path.join(
567-
os.path.dirname(__file__),
568-
"../../../templates/conditions/conditional-getatt-delete.yaml",
569-
),
570-
parameters={"DeployQueue": "false"},
571-
)
572-
573-
# Verify the stack was created successfully with only the always-created resource
574-
resources = aws_client.cloudformation.describe_stack_resources(
575-
StackName=stack.stack_id
576-
)
577-
resource_types = [r["ResourceType"] for r in resources["StackResources"]]
578-
# Only one SQS queue should be created (AlwaysCreatedResource)
579-
assert resource_types.count("AWS::SQS::Queue") == 1
580-
581-
# The conditional output should not exist
582-
assert "ConditionalQueueArn" not in stack.outputs
583-
584-
# Now delete the stack - this is where the bug manifests
585-
# The v2 engine incorrectly tries to resolve !GetAtt MyQueue.Arn during deletion
586-
# even though MyQueue was never created (condition was false)
587-
aws_client.cloudformation.delete_stack(StackName=stack.stack_id)
588-
aws_client.cloudformation.get_waiter("stack_delete_complete").wait(
589-
StackName=stack.stack_id
590-
)
591-
592-
@markers.aws.validated
593-
@skip_if_legacy_engine()
594-
def test_delete_stack_with_conditional_resource_referencing_conditional(
595-
self, deploy_cfn_template, aws_client
558+
def test_conditional_resource_referencing_conditional(
559+
self, deploy_cfn_template, aws_client, snapshot
596560
):
597561
"""
598-
Test that stack deletion works when a conditional resource references
599-
another conditional resource via !GetAtt, and both conditions were false.
562+
Test that a template with a conditional resource referencing another conditional
563+
resource via !GetAtt is accepted and deploys correctly when conditions are false.
600564
601-
This is different from test_delete_stack_with_conditional_getatt which
602-
uses Fn::If to wrap the GetAtt. Here the GetAtt is directly inside
603-
a conditional resource's properties.
565+
Without the fix, the v2 engine incorrectly visits properties of conditional
566+
resources even when their condition is false, causing failures when those
567+
properties contain !GetAtt references to other undeployed conditional resources.
604568
605569
Reproduces: https://github.com/localstack/localstack/issues/13609
606570
"""
571+
snapshot.add_transformer(
572+
SortingTransformer("StackResources", lambda e: e["LogicalResourceId"])
573+
)
574+
snapshot.add_transformer(snapshot.transform.key_value("PhysicalResourceId"))
575+
snapshot.add_transformer(snapshot.transform.cloudformation_api())
576+
607577
stack = deploy_cfn_template(
608578
template_path=os.path.join(
609579
os.path.dirname(__file__),
@@ -612,23 +582,10 @@ def test_delete_stack_with_conditional_resource_referencing_conditional(
612582
parameters={"EnableQueue": "false"},
613583
)
614584

615-
# Verify only the DynamoDB table was created (conditional resources skipped)
616585
resources = aws_client.cloudformation.describe_stack_resources(
617586
StackName=stack.stack_id
618587
)
619-
resource_types = [r["ResourceType"] for r in resources["StackResources"]]
620-
assert "AWS::DynamoDB::Table" in resource_types
621-
assert "AWS::SQS::Queue" not in resource_types
622-
assert "AWS::CloudWatch::Alarm" not in resource_types
623-
624-
# Delete the stack - this is where the bug manifests
625-
# The v2 engine incorrectly tries to resolve !GetAtt MyQueue.QueueName
626-
# inside MyAlarm's properties during deletion, even though MyAlarm
627-
# was never created (its condition was false)
628-
aws_client.cloudformation.delete_stack(StackName=stack.stack_id)
629-
aws_client.cloudformation.get_waiter("stack_delete_complete").wait(
630-
StackName=stack.stack_id
631-
)
588+
snapshot.match("resources-description", resources)
632589

633590

634591
class TestValidateConditions:

‎tests/aws/services/cloudformation/engine/test_conditions.snapshot.json‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -949,5 +949,30 @@
949949
}
950950
}
951951
}
952+
},
953+
"tests/aws/services/cloudformation/engine/test_conditions.py::TestCloudFormationConditions::test_conditional_resource_referencing_conditional": {
954+
"recorded-date": "04-02-2026, 01:53:15",
955+
"recorded-content": {
956+
"resources-description": {
957+
"StackResources": [
958+
{
959+
"DriftInformation": {
960+
"StackResourceDriftStatus": "NOT_CHECKED"
961+
},
962+
"LogicalResourceId": "MyTable",
963+
"PhysicalResourceId": "<physical-resource-id:1>",
964+
"ResourceStatus": "CREATE_COMPLETE",
965+
"ResourceType": "AWS::DynamoDB::Table",
966+
"StackId": "arn:<partition>:cloudformation:<region>:111111111111:stack/<stack-name:1>/<resource:1>",
967+
"StackName": "<stack-name:1>",
968+
"Timestamp": "timestamp"
969+
}
970+
],
971+
"ResponseMetadata": {
972+
"HTTPHeaders": {},
973+
"HTTPStatusCode": 200
974+
}
975+
}
976+
}
952977
}
953978
}

‎tests/aws/services/cloudformation/engine/test_conditions.validation.json‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,13 @@
11
{
2+
"tests/aws/services/cloudformation/engine/test_conditions.py::TestCloudFormationConditions::test_conditional_resource_referencing_conditional": {
3+
"last_validated_date": "2026-02-04T01:53:29+00:00",
4+
"durations_in_seconds": {
5+
"setup": 0.7,
6+
"call": 23.45,
7+
"teardown": 14.46,
8+
"total": 38.61
9+
}
10+
},
211
"tests/aws/services/cloudformation/engine/test_conditions.py::TestCloudFormationConditions::test_dependent_get_att": {
312
"last_validated_date": "2025-09-30T19:25:10+00:00",
413
"durations_in_seconds": {

‎tests/aws/templates/conditions/conditional-getatt-delete.yaml‎

Lines changed: 0 additions & 40 deletions
This file was deleted.

0 commit comments

Comments
 (0)