Skip to content

Conversation

simonrw
Copy link
Contributor

@simonrw simonrw commented Sep 11, 2025

Motivation

While investigating issues from the telemetry, I found that an AWS::NoValue was being turned into a Nothing which itself was being looked up as a key in a dictionary (the conditions).

foo = Nothing
bar = {}
print(foo in bar)

This does not work because Nothing is not hashable, so cannot be used as a dictionary key.

The reason for this is that in #13000 I incorrectly set any NoValue to Nothing during the modelling phase, however this caused the above issue.

Changes

  • Remove incorrect setting of arguments.value to Nothing
  • Add test that exercises the bug found, specifically checking for AWS::NoValue in an update operation

@simonrw simonrw added this to the 4.9 milestone Sep 11, 2025
@simonrw simonrw added aws:cloudformation AWS CloudFormation semver: patch Non-breaking changes which can be included in patch releases docs: skip Pull request does not require documentation changes labels Sep 11, 2025
Copy link

github-actions bot commented Sep 11, 2025

Test Results - Preflight, Unit

22 125 tests  ±0   20 387 ✅ ±0   6m 24s ⏱️ +6s
     1 suites ±0    1 738 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit c21494b. ± Comparison against base commit 515c19b.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Sep 11, 2025

Test Results (amd64) - Acceptance

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

Results for commit c21494b. ± Comparison against base commit 515c19b.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Sep 11, 2025

Test Results (amd64) - Integration, Bootstrap

  5 files    5 suites   46m 48s ⏱️
589 tests 475 ✅ 114 💤 0 ❌
595 runs  475 ✅ 120 💤 0 ❌

Results for commit c21494b.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Sep 11, 2025

LocalStack Community integration with Pro

  2 files  ±    0    2 suites  ±0   34m 4s ⏱️ - 1h 21m 41s
565 tests  - 4 094  450 ✅  - 3 878  115 💤  - 216  0 ❌ ±0 
567 runs   - 4 094  450 ✅  - 3 878  117 💤  - 216  0 ❌ ±0 

Results for commit c21494b. ± Comparison against base commit 515c19b.

This pull request removes 4098 and adds 4 tests. Note that renamed tests count towards both.
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]
…
tests.aws.services.cloudformation.api.test_changesets ‑ test_update_change_set_with_aws_novalue_repro
tests.aws.services.cloudformation.api.test_changesets ‑ test_using_pseudoparameters_in_places[condition]
tests.aws.services.cloudformation.api.test_changesets ‑ test_using_pseudoparameters_in_places[parameter]
tests.aws.services.cloudformation.api.test_changesets ‑ test_using_pseudoparameters_in_places[resource]

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Sep 11, 2025

Test Results - Alternative Providers

564 tests   329 ✅  23m 45s ⏱️
  1 suites  235 💤
  1 files      0 ❌

Results for commit c21494b.

♻️ This comment has been updated with latest results.

@simonrw simonrw marked this pull request as ready for review September 15, 2025 21:29
Copy link
Member

@pinzon pinzon left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this!

@simonrw simonrw merged commit 3139f35 into main Sep 15, 2025
47 checks passed
@simonrw simonrw deleted the cfn/novalue-kept branch September 15, 2025 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aws:cloudformation AWS CloudFormation docs: skip Pull request does not require documentation changes semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants