CFNv2: implement describe-stack-resource #12912

simonrw · 2025-07-25T22:29:55Z

Motivation

The current V2 CloudFormation provider does not support the describe_stack_resource operation. In order to reach parity with the v1 provider it must do.

Changes

Provide typing for the NodeResource in various places
Rework the way the resolved resources are propagated to provide more type safety and structure
Add ChangeSetModelValidator for validations we know happen before create_change_set returns (i.e. during the modeling phase). This can be a place to hook other validations as well
Validate logical resource ids are valid
Update the StackNotFoundError to handle yet another case, this time through an explicit message override
Implement DescribeStackResource
Add tests for describing non-existent things
Add test validating that stack parameters are given
Implement list-stack-resources to unskip another test
Unskip 9 tests

github-actions · 2025-07-25T22:40:39Z

Test Results - Preflight, Unit

21 991 tests ±0 20 257 ✅ ±0 7m 1s ⏱️ +34s
1 suites ±0 1 734 💤 ±0
1 files ±0 0 ❌ ±0

Results for commit 5cb7f8d. ± Comparison against base commit fb42a08.

♻️ This comment has been updated with latest results.

github-actions · 2025-07-25T22:52:55Z

Test Results (amd64) - Acceptance

7 tests ±0 5 ✅ ±0 3m 12s ⏱️ +3s
1 suites ±0 2 💤 ±0
1 files ±0 0 ❌ ±0

Results for commit 5cb7f8d. ± Comparison against base commit fb42a08.

♻️ This comment has been updated with latest results.

github-actions · 2025-07-25T22:53:31Z

LocalStack Community integration with Pro

2 files ± 0 2 suites ±0 1h 43m 18s ⏱️ - 5m 8s
4 482 tests - 460 4 155 ✅ - 14 327 💤 - 446 0 ❌ ±0
4 484 runs - 460 4 155 ✅ - 14 329 💤 - 446 0 ❌ ±0

Results for commit 5cb7f8d. ± Comparison against base commit fb42a08.

This pull request removes 460 tests.

tests.aws.services.cloudformation.v2.ported_from_v1.api.test_changesets ‑ test_autoexpand_capability_requirement
tests.aws.services.cloudformation.v2.ported_from_v1.api.test_changesets ‑ test_create_and_then_remove_non_supported_resource_change_set
tests.aws.services.cloudformation.v2.ported_from_v1.api.test_changesets ‑ test_create_and_then_remove_supported_resource_change_set
tests.aws.services.cloudformation.v2.ported_from_v1.api.test_changesets ‑ test_create_and_then_update_refreshes_template_metadata
tests.aws.services.cloudformation.v2.ported_from_v1.api.test_changesets ‑ test_create_change_set_create_existing
tests.aws.services.cloudformation.v2.ported_from_v1.api.test_changesets ‑ test_create_change_set_invalid_params
tests.aws.services.cloudformation.v2.ported_from_v1.api.test_changesets ‑ test_create_change_set_missing_stackname
tests.aws.services.cloudformation.v2.ported_from_v1.api.test_changesets ‑ test_create_change_set_update_nonexisting
tests.aws.services.cloudformation.v2.ported_from_v1.api.test_changesets ‑ test_create_change_set_update_without_parameters
tests.aws.services.cloudformation.v2.ported_from_v1.api.test_changesets ‑ test_create_change_set_with_ssm_parameter
…

♻️ This comment has been updated with latest results.

github-actions · 2025-07-25T22:58:10Z

Test Results (amd64) - Integration, Bootstrap

5 files 5 suites 2h 19m 51s ⏱️
4 841 tests 4 361 ✅ 480 💤 0 ❌
4 847 runs 4 361 ✅ 486 💤 0 ❌

Results for commit 5cb7f8d.

♻️ This comment has been updated with latest results.

github-actions · 2025-07-26T04:43:12Z

Test Results (MA/MR) - Preflight, Unit

21 983 tests 20 249 ✅ 6m 24s ⏱️
1 suites 1 734 💤
1 files 0 ❌

Results for commit cea3566.

github-actions · 2025-07-26T04:51:44Z

Test Results (amd64, MA/MR) - Acceptance

7 tests 5 ✅ 3m 8s ⏱️
1 suites 2 💤
1 files 0 ❌

Results for commit cea3566.

github-actions · 2025-07-26T05:23:42Z

Test Results (amd64, MA/MR) - Integration, Bootstrap

5 files 5 suites 2h 21m 53s ⏱️
4 839 tests 4 359 ✅ 480 💤 0 ❌
4 845 runs 4 359 ✅ 486 💤 0 ❌

Results for commit cea3566.

We don't need preprocessing in this implmenentation

This reverts commit cea3566.

…rce id

alexrashed

Nice to see the tests being unskipped! The implementation, the snapshots, and the tiny pipeline change are looking good! 💯

alexrashed · 2025-07-28T06:56:56Z

.github/workflows/tests-pro-integration.yml

@@ -339,7 +339,7 @@ jobs:
          AWS_DEFAULT_REGION: "us-east-1"
          JUNIT_REPORTS_FILE: "pytest-junit-community-${{ matrix.group }}.xml"
          TEST_PATH: "../../localstack/tests/aws/"  # TODO: run tests in tests/integration
-          PYTEST_ARGS: "${{ env.TINYBIRD_PYTEST_ARGS }}${{ env.TESTSELECTION_PYTEST_ARGS }}--splits ${{ strategy.job-total }} --group ${{ matrix.group }} --durations-path ../../localstack/.test_durations --store-durations"
+          PYTEST_ARGS: "${{ env.TINYBIRD_PYTEST_ARGS }}${{ env.TESTSELECTION_PYTEST_ARGS }}--splits ${{ strategy.job-total }} --group ${{ matrix.group }} --durations-path ../../localstack/.test_durations --store-durations --ignore ../../localstack/tests/aws/services/cloudformation/v2"


FYI: As far as I can tell, this change really just migrates the ignore introduced in #12871 to the tests executed against Pro.

baermat

SQS changes are looking good 👍

pinzon

Looks very good 👍 . The only blocker is the location of the new tests.

pinzon · 2025-07-28T17:10:06Z

tests/aws/services/cloudformation/v2/ported_from_v1/api/test_stacks.py

@@ -1061,3 +1060,13 @@ def test_stack_resource_not_found(deploy_cfn_template, aws_client, snapshot):

    snapshot.add_transformer(snapshot.transform.regex(stack.stack_name, "<stack-name>"))
    snapshot.match("Error", ex.value.response)
+
+
+@markers.aws.validated


comment: can't believe we didn't have this validation tested. 😅
Edit: This test should be moved too.

pinzon · 2025-07-28T17:14:48Z

tests/aws/services/cloudformation/v2/ported_from_v1/api/test_resources.py

Changes: The tests look great but I'd prefer if we move them to the original CFn test folder and conditionally skip them if the old engine doesn't support them.

At some point in the near future I want to re-merge the tests again so they we don't have divergence. And yes these are tests that don't work with the old engine so would require explicit skipping. Is it ok to move the tests at that point? I might work on this tomorrow if this PR is approved

pinzon · 2025-07-28T17:16:42Z

localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_validator.py

comment: Nice 🎉

pinzon · 2025-07-28T17:29:08Z

localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_executor.py

-                self.resources[logical_resource_id].update(extra_resource_properties)
-                # XXX for legacy delete_stack compatibility
-                self.resources[logical_resource_id]["LogicalResourceId"] = logical_resource_id
-                self.resources[logical_resource_id]["Type"] = resource_type
-
+                status_from_action = EventOperationFromAction[action.value]
                physical_resource_id = (
-                    self._get_physical_id(logical_resource_id)
+                    extra_resource_properties["PhysicalResourceId"]
                    if resource_provider
                    else MOCKED_REFERENCE
                )
-                self.resources[logical_resource_id]["PhysicalResourceId"] = physical_resource_id
+                resolved_resource = ResolvedResource(
+                    Properties=event.resource_model,
+                    LogicalResourceId=logical_resource_id,
+                    Type=resource_type,
+                    LastUpdatedTimestamp=datetime.now(timezone.utc),
+                    ResourceStatus=ResourceStatus(f"{status_from_action}_COMPLETE"),
+                    PhysicalResourceId=physical_resource_id,
+                )
+                # TODO: do we actually need this line?
+                resolved_resource.update(extra_resource_properties)
+
+                self.resources[logical_resource_id] = resolved_resource



question/comment: Does this mean that we only store the resource description when the event is successful? Is that correct? If not, we could move the storage one level above,

You make an excellent point. However we don't have much test coverage of this situation. It should be fairly easy to trigger: set up a resource definition with deliberately invalid data. Then the stack will be left in a bad state, HOWEVER we don't yet model that state and behaviour. I think that's a pre-requisite. I would feel nervous to implement something as fundamental as that without test coverage. I would love it if at the end of this initiative we have correct rollback behaviour but I'm not sure we will yet.

Perhaps I can work on this in a follow up?

…-resources

coveralls · 2025-07-29T14:45:10Z

coverage: 85.982% (+34.6%) from 51.377%
when pulling 5cb7f8d on cfn/v2/describe-stack-resources
into fb42a08 on main.

simonrw added 10 commits July 25, 2025 18:24

WIP

2e99441

Type resolved resources

a6717dc

Restructure the way the ResolvedResource works

96f13be

Handle validation of parameter values

892e1b8

Implement describe-stack-resource

d3a1aa9

Add test for describing non-existent resource

18a24b6

Handle error cases for describing stack resources

d1907cd

Unskip tests

beff6ee

Remove unused method override

ce1a108

Add validation of logical resource ids

f337492

simonrw added the semver: patch Non-breaking changes which can be included in patch releases label Jul 25, 2025

Increased test timeout

cea3566

simonrw added 7 commits July 26, 2025 22:23

Validator inherits directly from visitor

be599f7

We don't need preprocessing in this implmenentation

Revert "Increased test timeout"

756b0a0

This reverts commit cea3566.

Ignore cloudformation v2 tests for community tests against pro

948c8d5

Remove accidental unskip

6135c93

Implement list_stack_resources

d5cc6a6

Make sure update method of queue policy correctly sets physical resou…

aabdeac

…rce id

Correct skip path

481143c

simonrw marked this pull request as ready for review July 28, 2025 04:27

simonrw requested review from thrau, baermat and gregfurman as code owners July 28, 2025 04:27

simonrw requested review from dominikschubert, pinzon, alexrashed, dfangl, silv-io and k-a-il as code owners July 28, 2025 04:27

alexrashed approved these changes Jul 28, 2025

View reviewed changes

baermat approved these changes Jul 28, 2025

View reviewed changes

simonrw removed request for thrau, silv-io, dfangl, gregfurman and k-a-il July 28, 2025 07:58

pinzon requested changes Jul 28, 2025

View reviewed changes

pinzon self-requested a review July 28, 2025 21:22

pinzon approved these changes Jul 28, 2025

View reviewed changes

alexrashed added this to the 4.8 milestone Jul 29, 2025

Merge remote-tracking branch 'origin/main' into cfn/v2/describe-stack…

5cb7f8d

…-resources

simonrw mentioned this pull request Jul 29, 2025

CFNv2: add validation #12780

Closed

simonrw merged commit 5535768 into main Jul 29, 2025
40 checks passed

simonrw deleted the cfn/v2/describe-stack-resources branch July 29, 2025 16:20

Uh oh!

CFNv2: implement describe-stack-resource #12912

CFNv2: implement describe-stack-resource #12912

Uh oh!

Conversation

simonrw commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Changes

Uh oh!

github-actions bot commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results - Preflight, Unit

Uh oh!

github-actions bot commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results (amd64) - Acceptance

Uh oh!

github-actions bot commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

LocalStack Community integration with Pro

Uh oh!

github-actions bot commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results (amd64) - Integration, Bootstrap

Uh oh!

github-actions bot commented Jul 26, 2025

Test Results (MA/MR) - Preflight, Unit

Uh oh!

github-actions bot commented Jul 26, 2025

Test Results (amd64, MA/MR) - Acceptance

Uh oh!

github-actions bot commented Jul 26, 2025

Test Results (amd64, MA/MR) - Integration, Bootstrap

Uh oh!

alexrashed left a comment

Choose a reason for hiding this comment

Uh oh!

alexrashed Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

baermat left a comment

Choose a reason for hiding this comment

Uh oh!

pinzon left a comment

Choose a reason for hiding this comment

Uh oh!

pinzon Jul 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pinzon Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

simonrw Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

pinzon Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

pinzon Jul 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

simonrw Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

coveralls commented Jul 29, 2025

Uh oh!

Uh oh!

Uh oh!

simonrw commented Jul 25, 2025 •

edited

Loading

github-actions bot commented Jul 25, 2025 •

edited

Loading

github-actions bot commented Jul 25, 2025 •

edited

Loading

github-actions bot commented Jul 25, 2025 •

edited

Loading

github-actions bot commented Jul 25, 2025 •

edited

Loading

pinzon Jul 28, 2025 •

edited

Loading

pinzon Jul 28, 2025 •

edited

Loading