Skip to content

Cloud Formation v2 Engine: Support for Default fields in Parameters #12537

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 8 commits into from
Apr 24, 2025
Merged

Cloud Formation v2 Engine: Support for Default fields in Parameters #12537

merged 8 commits into from
Apr 24, 2025

Conversation

MEPalma
Copy link
Contributor

@MEPalma MEPalma commented Apr 17, 2025 •
edited
Loading

Motivation

Currently, the CFn v2 engine only supports CFn parameters if set through dynamic assignment as api action arguments. Moreover, the internal representation of Parameter definition is limited to a generic set of bindings.

Changes

  • Rework the internal representation of Parameters to include the supported types of Dynamic, Default, and Type
  • Add support for parameter resolution through Default fields
  • Add base boundary tests for the update of templates about Default and Dynamic values of Parameters

@MEPalma MEPalma added the semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases label Apr 17, 2025
@MEPalma MEPalma added this to the 4.4 milestone Apr 17, 2025
@MEPalma MEPalma self-assigned this Apr 17, 2025
@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 17, 2025 •
edited
Loading

LocalStack Community integration with Pro

  2 files  ±    0    2 suites  ±0   21m 34s ⏱️ - 1h 27m 48s
467 tests  - 3 913  314 ✅  - 3 708  153 💤  - 204  0 ❌  - 1 
469 runs   - 3 913  314 ✅  - 3 708  155 💤  - 204  0 ❌  - 1 

Results for commit bf56474. ± Comparison against base commit 9a40181.

This pull request removes 3917 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.v2.test_change_set_parameters.TestChangeSetParameters ‑ test_update_parameter_default_value
tests.aws.services.cloudformation.v2.test_change_set_parameters.TestChangeSetParameters ‑ test_update_parameter_default_value_with_dynamic_overrides
tests.aws.services.cloudformation.v2.test_change_set_parameters.TestChangeSetParameters ‑ test_update_parameter_with_added_default_value
tests.aws.services.cloudformation.v2.test_change_set_parameters.TestChangeSetParameters ‑ test_update_parameter_with_removed_default_value
This pull request removes 208 skipped tests and adds 4 skipped tests. Note that renamed tests count towards both. 
tests.aws.scenario.kinesis_firehose.test_kinesis_firehose.TestKinesisFirehoseScenario ‑ test_kinesis_firehose_s3
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input4-FAILED]
tests.aws.scenario.mythical_mysfits.test_mythical_misfits.TestMythicalMisfitsScenario ‑ test_deployed_infra_state
tests.aws.scenario.mythical_mysfits.test_mythical_misfits.TestMythicalMisfitsScenario ‑ test_populate_data
tests.aws.scenario.mythical_mysfits.test_mythical_misfits.TestMythicalMisfitsScenario ‑ test_user_clicks_are_stored
tests.aws.services.apigateway.test_apigateway_api.TestApiGatewayApiRestApi ‑ test_get_api_case_insensitive
tests.aws.services.apigateway.test_apigateway_api.TestApigatewayIntegration ‑ test_put_integration_request_parameter_bool_type
tests.aws.services.apigateway.test_apigateway_basic.TestAPIGateway ‑ test_api_gateway_authorizer_crud
tests.aws.services.apigateway.test_apigateway_basic.TestAPIGateway ‑ test_api_gateway_http_integration_with_path_request_parameter
tests.aws.services.apigateway.test_apigateway_basic.TestAPIGateway ‑ test_api_gateway_lambda_proxy_integration[/lambda/foo1]
…

tests.aws.services.cloudformation.v2.test_change_set_parameters.TestChangeSetParameters ‑ test_update_parameter_default_value
tests.aws.services.cloudformation.v2.test_change_set_parameters.TestChangeSetParameters ‑ test_update_parameter_default_value_with_dynamic_overrides
tests.aws.services.cloudformation.v2.test_change_set_parameters.TestChangeSetParameters ‑ test_update_parameter_with_added_default_value
tests.aws.services.cloudformation.v2.test_change_set_parameters.TestChangeSetParameters ‑ test_update_parameter_with_removed_default_value

♻️ This comment has been updated with latest results.

simonrw
simonrw approved these changes Apr 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.

A nice comprehensive set of tests, and a clean implementation, thank you!

tests/aws/services/cloudformation/v2/test_change_set_parameters.py Outdated
from localstack.utils.strings import long_uid


@pytest.mark.skipif(condition=not is_v2_engine(), reason="Requires the V2 engine")
Copy link
Contributor

Choose a reason for hiding this comment

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

A minor ease of use feature

Suggested change
@pytest.mark.skipif(condition=not is_v2_engine(), reason="Requires the V2 engine")
@pytest.mark.skipif(condition=not is_v2_engine() and not is_aws_cloud(), reason="Requires the V2 engine")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed this in other test files too

@MEPalma MEPalma merged commit 98c3e96 into master Apr 24, 2025
31 checks passed
@MEPalma MEPalma deleted the MEP-CFN-parameters_defaults branch April 24, 2025 15:45
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

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

Assignees

@MEPalma MEPalma

Labels
semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases
Projects
None yet
Milestone
4.4
Development

Successfully merging this pull request may close these issues.
2 participants
@MEPalma @simonrw