Skip to content

CFn: Improve parameter value validation #13124

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 1 commit into from
Sep 11, 2025
Merged

CFn: Improve parameter value validation #13124

merged 1 commit into from
Sep 11, 2025

Conversation

@simonrw
Copy link
Contributor

@simonrw simonrw commented Sep 10, 2025 •
edited
Loading

Motivation

While investigating user errors with the new CloudFormation engine, I saw some examples of

File "/opt/code/localstack/.venv/lib/python3.11/site-packages/localstack/services/cloudformation/v2/types.py", line 22, in engine_parameter_value
    raise RuntimeError("Parameter value is None")

This is raised because of our parameter value detection. When the user provides a falsy value (like the empty string) to a parameter, we incorreclty skip over this "given value" (since it's falsey) and try to load the default value. If this is also not present then we raise this exception.

Changes

  • Handle falsey values better with the parameter value determination (i.e. separate None from everything else)
  • Add test that exercises this functionality

@simonrw simonrw added this to the 4.9 milestone Sep 10, 2025
@simonrw simonrw added aws:cloudformation AWS CloudFormation semver: patch Non-breaking changes which can be included in patch releases review: merge when ready Signals to the reviewer that a PR can be merged if accepted docs: skip Pull request does not require documentation changes labels Sep 10, 2025
@simonrw simonrw changed the title CFn: Improve parameter value validation" CFn: Improve parameter value validation Sep 10, 2025
@github-actions
Copy link

github-actions bot commented Sep 10, 2025 •
edited
Loading

Test Results - Preflight, Unit

22 124 tests  ±0   20 386 ✅ ±0   6m 45s ⏱️ +13s
     1 suites ±0    1 738 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit 1df46cc3. ± Comparison against base commit aa8c1ba.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Sep 10, 2025 •
edited
Loading

Test Results (amd64) - Acceptance

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

Results for commit 1df46cc3. ± Comparison against base commit aa8c1ba.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Sep 10, 2025 •
edited
Loading

LocalStack Community integration with Pro

  2 files  ±    0    2 suites  ±0   34m 49s ⏱️ - 1h 23m 41s
559 tests  - 4 096  444 ✅  - 3 880  115 💤  - 216  0 ❌ ±0 
561 runs   - 4 096  444 ✅  - 3 880  117 💤  - 216  0 ❌ ±0 

Results for commit 1df46cc3. ± Comparison against base commit aa8c1ba.

This pull request removes 4097 and adds 1 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_stacks ‑ test_blank_parameter_value

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Sep 10, 2025 •
edited
Loading

Test Results (amd64) - Integration, Bootstrap

  5 files    5 suites   47m 19s ⏱️
583 tests 469 ✅ 114 💤 0 ❌
589 runs  469 ✅ 120 💤 0 ❌

Results for commit 1df46cc3.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Sep 10, 2025 •
edited
Loading

Test Results - Alternative Providers

558 tests   329 ✅  23m 33s ⏱️
  1 suites  229 💤
  1 files      0 ❌

Results for commit 1df46cc3.

♻️ This comment has been updated with latest results.

@simonrw simonrw force-pushed the cfn/extend-parameter-value-validation branch from 650d49f to 1df46cc Compare September 10, 2025 21:38
@simonrw simonrw marked this pull request as ready for review September 10, 2025 23:31
pinzon
pinzon approved these changes Sep 11, 2025
View reviewed changes
tests/aws/services/cloudformation/api/test_stacks.py
Comment on lines +1261 to +1265
res = aws_client.cloudformation.create_stack(
TemplateBody=json.dumps(template),
StackName=f"stack-{short_uid()}",
Parameters=[{"ParameterKey": "MyFoo", "ParameterValue": ""}],
)
Copy link
Member

@pinzon pinzon Sep 11, 2025

Choose a reason for hiding this comment

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

TIL: This is valid

Copy link
Contributor Author

@simonrw simonrw Sep 11, 2025

Choose a reason for hiding this comment

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

Yup on AWS it's only rejected by the AWS resource provider, not the CFn engine. I think it's quite often used as a condition for sort of the False value (i.e.

Conditions:
  Foo:
   Fn::Equals:
     - !Ref: MyParameter
     - ""

@simonrw simonrw merged commit a2411d3 into main Sep 11, 2025
47 checks passed
@simonrw simonrw deleted the cfn/extend-parameter-value-validation branch September 11, 2025 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pinzon pinzon pinzon approved these changes

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

Assignees

No one assigned

Labels

aws:cloudformation AWS CloudFormation docs: skip Pull request does not require documentation changes review: merge when ready Signals to the reviewer that a PR can be merged if accepted semver: patch Non-breaking changes which can be included in patch releases

Projects

None yet

Milestone

4.9

Development

Successfully merging this pull request may close these issues.

3 participants

@simonrw @pinzon