Skip to content

Conversation

@simonrw
Copy link
Contributor

@simonrw simonrw commented Aug 6, 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

  • Duplicate the existing resolving functionality into its own module
  • Replace/resolve dynamic references when visiting node properties in the preprocessor
  • Unskip ssm and secretsmanager resolving tests in template engine tests

@simonrw simonrw added this to the 4.8 milestone Aug 6, 2025
@simonrw simonrw added the semver: patch Non-breaking changes which can be included in patch releases label Aug 6, 2025
@github-actions
Copy link

github-actions bot commented Aug 6, 2025

Test Results - Preflight, Unit

22 063 tests  ±0   20 329 ✅ ±0   6m 19s ⏱️ -11s
     1 suites ±0    1 734 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit 7b1baa1. ± Comparison against base commit 3cfb711.

@github-actions
Copy link

github-actions bot commented Aug 6, 2025

Test Results (amd64) - Acceptance

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

Results for commit 7b1baa1. ± Comparison against base commit 3cfb711.

@github-actions
Copy link

github-actions bot commented Aug 6, 2025

LocalStack Community integration with Pro

  2 files    2 suites   22m 17s ⏱️
544 tests 329 ✅ 215 💤 0 ❌
546 runs  329 ✅ 217 💤 0 ❌

Results for commit 7b1baa1.

@github-actions
Copy link

github-actions bot commented Aug 6, 2025

Test Results (amd64) - Integration, Bootstrap

  5 files    5 suites   33m 46s ⏱️
568 tests 354 ✅ 214 💤 0 ❌
574 runs  354 ✅ 220 💤 0 ❌

Results for commit 7b1baa1.

@simonrw simonrw marked this pull request as ready for review August 6, 2025 16:09
@simonrw simonrw added the review: merge when ready Signals to the reviewer that a PR can be merged if accepted label Aug 6, 2025
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. It would be nice to have a test that asserts the Not happy path of the resolving. But let's handle that in the future.

Copy link
Member

Choose a reason for hiding this comment

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

🎉

Comment on lines +70 to +74
kwargs = {} # optional args for get_secret_value
if version_id:
kwargs["VersionId"] = version_id
if version_stage:
kwargs["VersionStage"] = version_stage
Copy link
Member

Choose a reason for hiding this comment

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

python complain: there should be a more pythonic way to handle this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hehe this came from @dominikschubert in 2021!

if version_id:
kwargs["VersionId"] = version_id
if version_stage:
kwargs["VersionStage"] = version_stage

@simonrw simonrw merged commit aee6433 into main Aug 6, 2025
45 checks passed
@simonrw simonrw deleted the cfn/v2/dynamic-ssm-lookup-2 branch August 6, 2025 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Development

Successfully merging this pull request may close these issues.

3 participants