Use catalog in CloudFormation CreateStack and save errors for DescribeStack #13321

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Merged

silv-io merged 10 commits into main from flc-62-use-catalog-in-cfn-createstack-and-save-errors-for-describe

Dec 5, 2025

Member

silv-io commented Oct 30, 2025 •

edited

Loading

Motivation

This PR is part of the IaC usability initiative. In that initiative we want to make error messages for Cfn clearer. Adding which resources can't be added an why in the DescribeStack is a first step for this.

Changes

Add a visitor to the cloudformation engine which determines if a resource is available in the license catalog
Use that visitor when ignoring of unsupported resources is disabled to set the status to failed and adding the reason to the describe output
Add a test to test this behavior for every kind of error message that we've prepared

Tests

Added a location for catalog integration tests in tests/integration/test_catalog.py and also a parametrized test for all error cases in test_catalog_reports_unsupported_resources_in_stack_status

Related

FLC-62

silv-io added notes: skip notes: needed semver: minor docs: needed and removed notes: skip labels

Contributor

localstack-bot commented Oct 30, 2025

Currently, only patch changes are allowed on main. Your PR labels (semver: minor, docs: needed, notes: needed) indicate that it cannot be merged into the main at this time.

github-actions bot commented Oct 30, 2025 •

edited

Loading

Test Results - Preflight, Unit

22 889 tests ±0 21 075 ✅ ±0 6m 25s ⏱️ -7s
1 suites ±0 1 814 💤 ±0
1 files ±0 0 ❌ ±0

Results for commit 1b70e37. ± Comparison against base commit 53d7108.

♻️ This comment has been updated with latest results.

github-actions bot commented Oct 30, 2025 •

edited

Loading

Test Results (amd64) - Acceptance

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

Results for commit 1b70e37. ± Comparison against base commit 53d7108.

♻️ This comment has been updated with latest results.

github-actions bot commented Oct 30, 2025 •

edited

Loading

Test Results - Alternative Providers

587 tests 331 ✅ 18m 49s ⏱️
1 suites 256 💤
1 files 0 ❌

Results for commit 1b70e37.

♻️ This comment has been updated with latest results.

github-actions bot commented Oct 30, 2025 •

edited

Loading

Test Results (amd64) - Integration, Bootstrap

5 files 5 suites 2h 32m 55s ⏱️
5 501 tests 4 949 ✅ 552 💤 0 ❌
5 507 runs 4 949 ✅ 558 💤 0 ❌

Results for commit 1b70e37.

♻️ This comment has been updated with latest results.

github-actions bot commented Oct 30, 2025 •

edited

Loading

LocalStack Community integration with Pro

2 files ±0 2 suites ±0 1h 55m 47s ⏱️ -46s
5 121 tests ±0 4 729 ✅ ±0 392 💤 ±0 0 ❌ ±0
5 123 runs ±0 4 729 ✅ ±0 394 💤 ±0 0 ❌ ±0

Results for commit 1b70e37. ± Comparison against base commit 53d7108.

♻️ This comment has been updated with latest results.

silv-io added this to the 4.12 milestone

silv-io force-pushed the flc-62-use-catalog-in-cfn-createstack-and-save-errors-for-describe branch from d56f88f to d0fd050 Compare

November 26, 2025 15:01

github-actions bot commented Nov 26, 2025 •

edited

Loading

S3 Image Test Results (AMD64 / ARM64)

2 files 2 suites 8m 24s ⏱️
552 tests 500 ✅ 52 💤 0 ❌
1 104 runs 1 000 ✅ 104 💤 0 ❌

Results for commit 1b70e37.

♻️ This comment has been updated with latest results.

silv-io marked this pull request as ready for review

November 28, 2025 12:32

silv-io requested review from dominikschubert, pinzon, simonrw and thrau as code owners

November 28, 2025 12:32

silv-io requested a review from k-a-il

November 28, 2025 12:32

simonrw requested changes

View reviewed changes

Contributor

simonrw left a comment

The blocking issue is moving the resource support checking to after the transformation

localstack-core/localstack/config.py Outdated

    
              CFN_IGNORE_UNSUPPORTED_RESOURCE_TYPES = is_env_not_false("CFN_IGNORE_UNSUPPORTED_RESOURCE_TYPES")

              # Comma-separated list of resource type names that CLoudFormation will ignore on stack create/update

              CFN_IGNORE_UNSUPPORTED_TYPE_CREATE: list[str] = parse_comma_separated_list(

Contributor

simonrw Dec 1, 2025

question: do we need to include DELETE operations as well?

Member Author

silv-io Dec 1, 2025

It's not part of the spec so that's why I didn't add that. Is there a case where we'd need to specifically ignore deletion of a resource that we didn't handle in the create/update?

Also, I think I'll delete the addition of these env vars here because they'd be part of a next iteration PR. So we can discuss that in that follow-up

tests/integration/test_catalog.py Outdated

Comment on lines 149 to 152

    
                  try:

                      aws_client.cloudformation.delete_stack(StackName=stack_id)

                  except Exception:

                      pass

Contributor

simonrw Dec 1, 2025

suggestion(minor): we should probably use cleanups here but of course we know the stack deployment should fail so it's not a huge concern.

Member Author

silv-io Dec 1, 2025

I had that in there while working on it in case something went wrong. I can switch it over to using cleanups instead

localstack-core/localstack/services/cloudformation/v2/provider.py

    
                      )

                      validator.validate()

                      if not config.CFN_IGNORE_UNSUPPORTED_RESOURCE_TYPES:

Contributor

simonrw Dec 1, 2025

issue: we should apply this step after transformation, because the transformation process may add resources to the template.

Member Author

silv-io Dec 1, 2025

In our initial discussion I think it was mentioned to be "after the validations" and I wasn't sure if the transformations are part of that or not. Thank you for spotting that :)

...ack-core/localstack/services/cloudformation/engine/v2/change_set_resource_support_checker.py

Comment on lines 82 to 99

    
                          support_status = self._resource_support_status(resource_type)

                          if support_status == CloudFormationResourcesSupportAtRuntime.AVAILABLE:

                              pass

                          else:

                              failure_message = self._build_resource_failure_message(

                                  resource_type, support_status

                              )

                              self._resource_failure_messages[resource_type] = failure_message

Contributor

simonrw Dec 1, 2025

suggestion(style): maybe a match statement?

Member Author

silv-io Dec 1, 2025

I initially thought I'll use it in multiple locations, that's why I had the dict. match statement makes more sense now though, I agree :)

...ack-core/localstack/services/cloudformation/engine/v2/change_set_resource_support_checker.py

    
                      self, resource_type: str

                  ) -> AwsServicesSupportStatus | CfnResourceSupportStatus:

                      service_name = _get_service_name(resource_type)

                      return self.catalog.get_cloudformation_resource_status(resource_type, service_name, True)

Contributor

simonrw Dec 1, 2025

question: how do we deal with CFn custom resources? I see this was part of #13027 so I don't have the context here. Do we only look at resources with the AWS:: prefix?

Member Author

silv-io Dec 1, 2025

Just skimmed through the code and I don't think we do. I can add in a cross check with what we've added in #13027 here to make it safe 👍🏼

silv-io force-pushed the flc-62-use-catalog-in-cfn-createstack-and-save-errors-for-describe branch from cc62478 to 06cb0e8 Compare

December 4, 2025 12:56

silv-io added 8 commits

December 4, 2025 16:34


          Fix TypeAlias detection in catalog responses

e9efa02


          Add ChangeSetResourceSupportChecker

220f99f


          Use ChangeSetResourceSupportChecker in Cfn Provider

e99073e


          Add test for unsupported Cfn resources

89cd1e0


          Only fail if not ignoring supported resources types

ddd2025

# Conflicts:
#	localstack-core/localstack/config.py


          Also do not set a status if ignored

1151a07


          Add monkeypatch to not ignore unsupported resource types in new tests

e474b94


          Use cleanups fixture in test

9aac0fe


          Work in review suggestions

ca9cf02

silv-io force-pushed the flc-62-use-catalog-in-cfn-createstack-and-save-errors-for-describe branch from 63314cb to ca9cf02 Compare

December 4, 2025 15:35


          Ignore non-AWS resources for setting the status

1b70e37

silv-io requested a review from simonrw

December 4, 2025 15:57

simonrw approved these changes

View reviewed changes

Contributor

simonrw left a comment

Thanks for addressing my feedback. Looks great 👍

silv-io merged commit 0a759a9 into main

50 checks passed

silv-io deleted the flc-62-use-catalog-in-cfn-createstack-and-save-errors-for-describe branch

December 5, 2025 09:38

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

simonrw simonrw approved these changes

pinzon Awaiting requested review from pinzon pinzon is a code owner

dominikschubert Awaiting requested review from dominikschubert dominikschubert is a code owner

thrau Awaiting requested review from thrau thrau is a code owner

k-a-il Awaiting requested review from k-a-il

Labels

docs: needed notes: needed semver: minor