GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Add possibility to ignore specific unsupported resource types in CloudFormation by silv-io · Pull Request #13496 · localstack/localstack

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

Conversation

@silv-io

@silv-io silv-io commented Dec 10, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Motivation

As part of the IaC initiative we want to add the possibility to ignore only specific resource types when checking for their support.

Changes

  • Add new utility function should_ignore_unsupported_resource_type which is used in place of the general ignore of all unsupported resources in all locations.

Tests

  • Add new test to check end-to-end functionality with catalog of that toggle

Related

Fixes FLC-84
Fixes FLC-198

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
silv-io added 2 commits December 10, 2025 18:03
@silv-io
Add per-resource ignore for create and update in Cloudformation
f7baad1
@silv-io
Add test for ignore list
d6e9184
@github-actions

github-actions Bot commented Dec 10, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

S3 Image Test Results (AMD64 / ARM64)

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

Results for commit 977e8dd.

♻️ This comment has been updated with latest results.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions

github-actions Bot commented Dec 10, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

Test Results - Preflight, Unit

23 000 tests  ±0   21 157 ✅ ±0   6m 36s ⏱️ +6s
     1 suites ±0    1 843 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit 977e8dd. ± Comparison against base commit bad8808.

♻️ This comment has been updated with latest results.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions

github-actions Bot commented Dec 10, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

Test Results (amd64) - Acceptance

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

Results for commit 977e8dd. ± Comparison against base commit bad8808.

♻️ This comment has been updated with latest results.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions

github-actions Bot commented Dec 10, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

Test Results - Alternative Providers

583 tests   325 ✅  17m 49s ⏱️
  1 suites  258 💤
  1 files      0 ❌

Results for commit 977e8dd.

♻️ This comment has been updated with latest results.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions

github-actions Bot commented Dec 10, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

Test Results (amd64) - Integration, Bootstrap

    5 files      5 suites   2h 33m 45s ⏱️
5 527 tests 4 971 ✅ 556 💤 0 ❌
5 533 runs  4 971 ✅ 562 💤 0 ❌

Results for commit 977e8dd.

♻️ This comment has been updated with latest results.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions

github-actions Bot commented Dec 10, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

LocalStack Community integration with Pro

    2 files  ±0      2 suites  ±0   1h 55m 47s ⏱️ - 1m 22s
5 146 tests ±0  4 750 ✅ ±0  396 💤 ±0  0 ❌ ±0 
5 148 runs  ±0  4 750 ✅ ±0  398 💤 ±0  0 ❌ ±0 

Results for commit 977e8dd. ± Comparison against base commit bad8808.

♻️ This comment has been updated with latest results.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@silv-io silv-io added the semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases label Dec 11, 2025
@localstack-bot

localstack-bot commented Dec 11, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@silv-io silv-io added docs: needed Pull request requires documentation updates notes: needed Pull request should be mentioned in the release notes labels Dec 11, 2025
@silv-io
silv-io requested a review from k-a-il December 11, 2025 13:41
@silv-io
silv-io marked this pull request as ready for review December 11, 2025 13:41
@silv-io
silv-io requested review from dominikschubert, pinzon, simonrw and thrau as code owners December 11, 2025 13:41
simonrw added a commit that referenced this pull request Dec 15, 2025
@simonrw
CFn: validate Type is provided
c235eda 
# Motivation

While discusing #13496 I asserted that we can safely access `resource["Type"]` in the visitors since we validate that the `Type` key is always present. However this _was not true_! This PR resolves this

# Changes

* Add test deploying resource without a `Type` key
* Validate that the type is present during the modelling phase
@simonrw simonrw mentioned this pull request Dec 15, 2025
CFn: validate Type is provided #13524
Merged
simonrw
simonrw suggested changes Dec 15, 2025
View reviewed changes

@simonrw simonrw left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a few minor suggestions/nits but I would like the test to be updated to use direct boto calls where possible. This is for clarity and ease of understanding the test going forward.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_preproc.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread ...ack-core/localstack/services/cloudformation/engine/v2/change_set_resource_support_checker.py Outdated
TITLE_MESSAGE = "Unsupported resources detected:"

def __init__(self):
def __init__(self, change_set_type: ChangeSetType | str | None = None):

@simonrw simonrw Dec 15, 2025

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: it would be nice to not need the str type here, and surely it's always known so surely None is not a valid type?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@silv-io silv-io Dec 15, 2025

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah you're right. I think in some place I defined it as a string once and then had curly lines below, which I then auto-fixed. Just setting ChangeSetType should work fine.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread localstack-core/localstack/services/cloudformation/engine/v2/unsupported_resource.py Outdated


def should_ignore_unsupported_resource_type(
resource_type: str, change_set_type: ChangeSetType | str | None

@simonrw simonrw Dec 15, 2025

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: as stated above, under what circumstances is change_set_type either str or None?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread tests/integration/test_catalog.py Outdated

# template with one supported and one unsupported resource
bucket_name = f"cfn-toggle-{short_uid()}"
template_body = textwrap.dedent(

@simonrw simonrw Dec 15, 2025

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: for inline templates like this, using a dictionary and rendering to a JSON string might be nicer

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
silv-io reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread tests/integration/test_catalog.py Outdated
Comment on lines +115 to +125
def _wait_for_complete_change_set(change_set_id: str):
def _describe():
result = aws_client.cloudformation.describe_change_set(ChangeSetName=change_set_id)
status = result["Status"]
if status == "CREATE_COMPLETE":
return result
if status == "FAILED":
pytest.fail(f"change set unexpectedly failed: {result.get('StatusReason')}")
raise Exception("still waiting for change set")

return retry(_describe, retries=15, sleep=2)

@simonrw simonrw Dec 15, 2025

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: this function can be replaced by aws_client.cloudformation.get_waiter("change_set_create_complete").wait(...)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
silv-io reacted with hooray emoji
All reactions
  • 🎉 1 reaction
Comment thread tests/integration/test_catalog.py Outdated
def test_ignore_unsupported_resources_toggle(testing_catalog, aws_client, monkeypatch, cleanups):
unsupported_resource = "AWS::LatestService::NotSupported"

def _create_change_set(stack_name: str, template_body: str):

@simonrw simonrw Dec 15, 2025

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit I personally don't like this helper and would prefer the actual cfn function calls in the test, however I can see it's value as we perform this operation twice.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread tests/integration/test_catalog.py Outdated
cs_id_ok, stack_id_ok = _create_change_set(stack_name_ok, template_body)
_wait_for_complete_change_set(cs_id_ok)
aws_client.cloudformation.execute_change_set(ChangeSetName=cs_id_ok)
_wait_for_stack_status(stack_name_ok, "CREATE_COMPLETE")

@simonrw simonrw Dec 15, 2025

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit can we use the waiters here?

Suggested change
_wait_for_stack_status(stack_name_ok, "CREATE_COMPLETE")
aws_client.cloudformation.get_waiter("stack_create_complete").wait(StackName=stack_name_ok)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread tests/integration/test_catalog.py Outdated
monkeypatch.setattr(config, "CFN_IGNORE_UNSUPPORTED_TYPE_CREATE", [unsupported_resource])
stack_name_ok = f"stack-ok-{short_uid()}"
cs_id_ok, stack_id_ok = _create_change_set(stack_name_ok, template_body)
_wait_for_complete_change_set(cs_id_ok)

@simonrw simonrw Dec 15, 2025

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
_wait_for_complete_change_set(cs_id_ok)
aws_client.cloudformation.get_waiter("change_set_create_complete").wait(ChangeSetName=cs_id_ok)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread tests/integration/test_catalog.py Outdated
Comment on lines +159 to +162
failed_cs = _wait_for_failed_change_set(cs_id_fail)
status_reason = failed_cs.get("StatusReason", "")
assert ChangeSetResourceSupportChecker.TITLE_MESSAGE in status_reason
assert unsupported_resource in status_reason

@simonrw simonrw Dec 15, 2025

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: then we don't need the inner function

Suggested change
failed_cs = _wait_for_failed_change_set(cs_id_fail)
status_reason = failed_cs.get("StatusReason", "")
assert ChangeSetResourceSupportChecker.TITLE_MESSAGE in status_reason
assert unsupported_resource in status_reason
with pytest.raises(WaiterError) as exc_info:
aws_client.cloudformation.get_waiter("change_set_create_complete").wait(
ChangeSetName=cs_id_fail
)
assert exc_info.value.last_response["Status"] == "FAILED"
status_reason = exc_info.value.last_response["StatusReason"]
assert ChangeSetResourceSupportChecker.TITLE_MESSAGE in status_reason
assert unsupported_resource in status_reason

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread tests/integration/test_catalog.py


@markers.aws.only_localstack
def test_ignore_unsupported_resources_toggle(testing_catalog, aws_client, monkeypatch, cleanups):

@simonrw simonrw Dec 15, 2025

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue: in general I would prefer not defining inner functions (especially if they only have one use), and to use the waiters where possible. I don't find them easy to use, and the waiters code is more straightforward to read in the test itself. For the successful cases we swap a function definition and call with a single line, and for the failure cases using the WaiterError is clearer IMO.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@silv-io silv-io Dec 15, 2025

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks I didn't like it too much either but also didn't know about the waiters. Will switch it over :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@silv-io silv-io Dec 15, 2025

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've fixed it and also went ahead and fixed the old test to use the waiters. It's much cleaner now :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@silv-io
Work in review suggestions
977e8dd
@silv-io
silv-io force-pushed the flc-84 branch from 6c19db1 to 977e8dd Compare December 15, 2025 12:49
@silv-io
silv-io requested a review from simonrw December 15, 2025 12:53
@thrau
thrau removed their request for review December 18, 2025 14:04
simonrw
simonrw approved these changes Dec 22, 2025
View reviewed changes

@simonrw simonrw left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing my feedback

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
simonrw added a commit that referenced this pull request Dec 22, 2025
@simonrw
CFn: validate Type is provided
b6af98b 
# Motivation

While discusing #13496 I asserted that we can safely access `resource["Type"]` in the visitors since we validate that the `Type` key is always present. However this _was not true_! This PR resolves this

# Changes

* Add test deploying resource without a `Type` key
* Validate that the type is present during the modelling phase
@silv-io
silv-io merged commit 237a9e1 into main Dec 23, 2025
50 checks passed

Uh oh!

There was an error while loading. Please reload this page.
@silv-io
silv-io deleted the flc-84 branch December 23, 2025 09:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@k-a-il k-a-il Awaiting requested review from k-a-il
@pinzon pinzon Awaiting requested review from pinzon
@dominikschubert dominikschubert Awaiting requested review from dominikschubert dominikschubert is a code owner
1 more reviewer
@simonrw simonrw simonrw approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

docs: needed Pull request requires documentation updates notes: needed Pull request should be mentioned in the release notes semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@silv-io @localstack-bot @simonrw
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.