Skip to content

CFn: handle resolving parameter names constructed in intrinsics#13192

Merged
pinzon merged 8 commits intomainfrom
cfn/dynamic-parameter-resolving
Sep 25, 2025
Merged

CFn: handle resolving parameter names constructed in intrinsics#13192
pinzon merged 8 commits intomainfrom
cfn/dynamic-parameter-resolving

Conversation

@simonrw
Copy link
Contributor

@simonrw simonrw commented Sep 24, 2025 •
edited
Loading

Motivation

From #13169 we have an example of dynamic resolving where the name of the SSM parameter is built up from an Fn::Sub call, e.g.

Value: !Sub "abc:{{resolve:ssm:${ParameterName}}}"

This means the !Sub intrinsic should be invoked before the SSM parameter is resolved.

Changes

We now

  • perform better matching of whether a parameter is present
  • handle failure to resolve the parameter and assume that we will come back to it later

Also:

  • add a unit test for the regex matching
  • update an exisitng test to include this use case

Closes #13169

@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 notes: skip Pull request does not have to be mentioned in the release notes labels Sep 24, 2025
@github-actions
Copy link

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

Test Results - Preflight, Unit

22 277 tests  +7   20 536 ✅ +7   15m 46s ⏱️ +17s
     1 suites ±0    1 741 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit 72f71f2. ± Comparison against base commit 15fc427.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

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

LocalStack Community integration with Pro

    2 files  ±0      2 suites  ±0   2h 0m 37s ⏱️ -24s
4 781 tests ±0  4 445 ✅ ±0  336 💤 ±0  0 ❌ ±0 
4 783 runs  ±0  4 445 ✅ ±0  338 💤 ±0  0 ❌ ±0 

Results for commit 72f71f2. ± Comparison against base commit 15fc427.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

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

Test Results (amd64) - Acceptance

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

Results for commit 72f71f2. ± Comparison against base commit 15fc427.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

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

Test Results - Alternative Providers

572 tests   - 775   329 ✅  - 372   25m 51s ⏱️ - 13m 29s
  1 suites  -   4   243 💤  - 403 
  1 files    -   4     0 ❌ ±  0 

Results for commit 72f71f2. ± Comparison against base commit 15fc427.

This pull request removes 775 tests. 
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudWatchMultiProtocol ‑ test_basic_operations_multiple_protocols[json]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudWatchMultiProtocol ‑ test_basic_operations_multiple_protocols[query]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudWatchMultiProtocol ‑ test_basic_operations_multiple_protocols[smithy-rpc-v2-cbor]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudWatchMultiProtocol ‑ test_exception_serializing_with_no_shape_in_spec[json]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudWatchMultiProtocol ‑ test_exception_serializing_with_no_shape_in_spec[query]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudWatchMultiProtocol ‑ test_exception_serializing_with_no_shape_in_spec[smithy-rpc-v2-cbor]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudWatchMultiProtocol ‑ test_multi_protocol_client_fixture[json]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudWatchMultiProtocol ‑ test_multi_protocol_client_fixture[query]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudWatchMultiProtocol ‑ test_multi_protocol_client_fixture[smithy-rpc-v2-cbor]
tests.aws.services.cloudwatch.test_cloudwatch.TestCloudwatch ‑ test_alarm_lambda_target
…
This pull request skips 1 test. 
tests.aws.services.cloudformation.test_template_engine.TestSsmParameters ‑ test_resolve_ssm

♻️ This comment has been updated with latest results.

@github-actions
Copy link

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

Test Results (amd64) - Integration, Bootstrap

    5 files  ±0      5 suites  ±0   2h 38m 38s ⏱️ - 1m 2s
5 155 tests ±0  4 659 ✅ ±0  496 💤 ±0  0 ❌ ±0 
5 161 runs  ±0  4 659 ✅ ±0  502 💤 ±0  0 ❌ ±0 

Results for commit 72f71f2. ± Comparison against base commit 15fc427.

♻️ This comment has been updated with latest results.

@simonrw simonrw added the review: merge when ready Signals to the reviewer that a PR can be merged if accepted label Sep 25, 2025
@simonrw simonrw marked this pull request as ready for review September 25, 2025 11:42
pinzon
pinzon approved these changes Sep 25, 2025
View reviewed changes
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.

LGTM. Thanks for fixing this.

tests/aws/services/cloudformation/test_template_engine.py
Name=parameter_key, Overwrite=True, Type="String", Value=parameter_value_v2
)

versioned_parameter_reference = f"{parameter_key}:{v1['Version']}"
Copy link
Member

@pinzon pinzon Sep 25, 2025

Choose a reason for hiding this comment

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

Nit: You could have added the snapshot util here too.

@pinzon pinzon merged commit 339ad5c into main Sep 25, 2025
42 checks passed
@pinzon pinzon deleted the cfn/dynamic-parameter-resolving branch September 25, 2025 17:15
pinzon added a commit that referenced this pull request Sep 25, 2025
@pinzon
Revert "CFn: handle resolving parameter names constructed in intrinsi…
af82a02 
…cs (#13192)"

This reverts commit 339ad5c.
@alexrashed alexrashed added this to the 4.9 milestone Sep 30, 2025
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 notes: skip Pull request does not have to be mentioned in the release notes 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.

bug: CloudFormation dynamic reference regression 4.7 to 4.8

3 participants

@simonrw @pinzon @alexrashed