Skip to content

handle unsupported resources in CFn v2 #12905

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
merged 5 commits into from
Jul 24, 2025
Merged

handle unsupported resources in CFn v2 #12905

merged 5 commits into from
Jul 24, 2025

Conversation

pinzon
Copy link
Member

@pinzon pinzon commented Jul 24, 2025

Motivation

  • This PR adds to the CFn v2 the capability to handle unsupported resources in a better way.

Changes

  • if CFN_IGNORE_UNSUPPORTED_RESOURCE_TYPES is set to true the engine will ignore nonexistent resource providers and return unknown when a property is requested.
  • Removal of duplicated code.

Testing

  • Unskipped test.

pinzon
pinzon commented Jul 24, 2025
View reviewed changes
localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_executor.py
Comment on lines -392 to -402
stack = self._change_set.stack
stack.set_resource_status(
logical_resource_id=logical_resource_id,
# TODO,
physical_resource_id="",
resource_type=resource_type,
status=ResourceStatus.CREATE_FAILED
if action == ChangeAction.Add
else ResourceStatus.UPDATE_FAILED,
resource_status_reason=reason,
)
Copy link
Member Author

@pinzon pinzon Jul 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

The processing of a failed event is handled at a level above. So these lines just add repetition to that

pinzon
pinzon commented Jul 24, 2025
View reviewed changes
localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_executor.py
Comment on lines -369 to -375
if resource_provider is None:
log_not_available_message(
resource_type,
f'No resource provider found for "{resource_type}"',
)
if not config.CFN_IGNORE_UNSUPPORTED_RESOURCE_TYPES:
raise NoResourceProvider
Copy link
Member Author

@pinzon pinzon Jul 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@simonrw I believe that raising an error would confuse and alert users. Having the deployment fail and explaining why seems to me like a better experience.

@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 24, 2025 •
edited
Loading

Test Results - Preflight, Unit

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

Results for commit f7d05e1. ± Comparison against base commit ba66ba1.

♻️ This comment has been updated with latest results.

@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 24, 2025 •
edited
Loading

LocalStack Community integration with Pro

  2 files  ±    0    2 suites  ±0   22m 18s ⏱️ - 1h 21m 52s
898 tests  - 4 040  334 ✅  - 3 827  564 💤  - 213  0 ❌ ±0 
900 runs   - 4 040  334 ✅  - 3 827  566 💤  - 213  0 ❌ ±0 

Results for commit f7d05e1. ± Comparison against base commit ba66ba1.

This pull request removes 4040 tests. 
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_lambda_dynamodb
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_opensearch_crud
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_search_books
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_setup
tests.aws.scenario.kinesis_firehose.test_kinesis_firehose.TestKinesisFirehoseScenario ‑ test_kinesis_firehose_s3
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_destination_sns
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_infra
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_prefill_dynamodb_table
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input0-SUCCEEDED]
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input1-SUCCEEDED]
…

♻️ This comment has been updated with latest results.

@pinzon pinzon added the semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases label Jul 24, 2025
@pinzon pinzon marked this pull request as ready for review July 24, 2025 18:57
@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 24, 2025 •
edited
Loading

Test Results (amd64) - Acceptance

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

Results for commit f7d05e1. ± Comparison against base commit ba66ba1.

♻️ This comment has been updated with latest results.

@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 24, 2025 •
edited
Loading

Test Results (amd64) - Integration, Bootstrap

  5 files    5 suites   34m 51s ⏱️
922 tests 359 ✅ 563 💤 0 ❌
928 runs  359 ✅ 569 💤 0 ❌

Results for commit f7d05e1.

♻️ This comment has been updated with latest results.

@pinzon pinzon force-pushed the cp/cfn/v2/handle-unsupported-resources branch from 9829c9d to f7d05e1 Compare July 24, 2025 19:28
simonrw
simonrw approved these changes Jul 24, 2025
View reviewed changes
Copy link
Contributor

@simonrw simonrw left a comment

Choose a reason for hiding this comment

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

I think you are right this behaviour is better, but this whole feature is going to be revisited in an upcoming initiative so I'm not too worried at the moment about the specifics. Glad we have a passing test though

@pinzon pinzon merged commit d0383c5 into main Jul 24, 2025
39 checks passed
@pinzon pinzon deleted the cp/cfn/v2/handle-unsupported-resources branch July 24, 2025 22:15
@pinzon pinzon changed the title handle unsupported resource in CFn v2 handle unsupported resources in CFn v2 Jul 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simonrw simonrw simonrw approved these changes

@dominikschubert dominikschubert Awaiting requested review from dominikschubert dominikschubert is a code owner

Assignees
No one assigned
Labels
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.
2 participants
@pinzon @simonrw