Skip to content

CFNv2: support dynamic parameter resolution #12797

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.

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

Open
wants to merge 1 commit into
base: cfn/v2/describe-parameters
Choose a base branch
from

Conversation

simonrw
Copy link
Contributor

@simonrw simonrw commented Jun 24, 2025

Motivation

The V2 engine does not currently support resolving dynamic parameter values, e.g.

{{resolve:secretsmanager:secret-id:secret-string:json-key:version-stage:version-id}}

Changes

  • Extract relevant functions out of the template deployer into their own module
  • Replace/resolve dynamic references when visiting node properties in the preprocessor
  • Unskip ssm resolving test in template engine tests

@simonrw simonrw added the semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases label Jun 24, 2025
Copy link

github-actions bot commented Jun 24, 2025

Test Results - Preflight, Unit

21 727 tests  ±0   20 070 ✅ ±0   6m 23s ⏱️ -50s
     1 suites ±0    1 657 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit 6574897. ± Comparison against base commit 6c4cd17.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Jun 24, 2025

Test Results (amd64) - Acceptance

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

Results for commit 6574897. ± Comparison against base commit 6c4cd17.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Jun 24, 2025

LocalStack Community integration with Pro

  2 files  ±0    2 suites  ±0   21m 27s ⏱️ -2s
887 tests ±0  323 ✅ ±0  564 💤 ±0  0 ❌ ±0 
889 runs  ±0  323 ✅ ±0  566 💤 ±0  0 ❌ ±0 

Results for commit 6574897. ± Comparison against base commit 6c4cd17.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Jun 24, 2025

Test Results (amd64) - Integration, Bootstrap

  5 files  ±0    5 suites  ±0   34m 14s ⏱️ -20s
911 tests ±0  348 ✅ ±0  563 💤 ±0  0 ❌ ±0 
917 runs  ±0  348 ✅ ±0  569 💤 ±0  0 ❌ ±0 

Results for commit 6574897. ± Comparison against base commit 6c4cd17.

♻️ This comment has been updated with latest results.

@simonrw simonrw force-pushed the cfn/v2/dynamic-ssm-lookup branch from acc5133 to 6574897 Compare June 24, 2025 22:06
@simonrw simonrw marked this pull request as ready for review June 24, 2025 22:58
@simonrw simonrw requested a review from MEPalma June 24, 2025 22:58
@@ -1040,7 +1045,23 @@ def visit_node_array(self, node_array: NodeArray) -> PreprocEntityDelta:
return PreprocEntityDelta(before=before, after=after)

def visit_node_property(self, node_property: NodeProperty) -> PreprocEntityDelta:
return self.visit(node_property.value)
# TODO: is this the right place?
Copy link
Contributor

Choose a reason for hiding this comment

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

Are dynamic parameters only resolved at a property level? For example, these are never resolved as input for another intrinsic function?

@simonrw simonrw added this to the Playground milestone Jun 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants