Skip to content

CFNV2: support CDK bootstrap and deployment #12967

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
simonrw merged 8 commits into from
Aug 7, 2025
Merged

CFNV2: support CDK bootstrap and deployment #12967

simonrw merged 8 commits into from
Aug 7, 2025

Conversation

@simonrw
Copy link
Contributor

@simonrw simonrw commented Aug 6, 2025 •
edited
Loading

Motivation

During my testing, even a simple CDK bootstrap failed to work against the engine. If we want to open the new engine up for testing, we should at least support the first CDK command a user will run.

The reason for the failure was mostly the messaging around performing a DescribeStacks on a stack that didn't exist. Because we didn't have 100% parity, the CDK CLI treated the failure to describe the CDKToolkit stack as a fatal error and terminated the process.

Changes

Emulator changes

  • Create parity with stack not found error
  • Ensure the AWS::CDK::Metadata resource returns its physical resource id on update
  • Fix bug with determining the change type of an if condition
  • Fn::If is now short circuiting, meaning if it evaluates to True then we only visit the True case, and the same for the False case
    • this is important for the bootstrap template because there are quite a few conditionals, and performing !Sub ${Resource.Property} on a resource that has a falsey condition would lead to trying to look that resource up and failing (and therefore crashing)
  • Initial support for UsePreviousValue when supplying parameters (this is what the CDK does)
    • deploy_cfn_template now supports raw_parameters which allows for UsePreviousValue to be specified

Testing changes

  • Capture the latest (at time of writing) version of the bootstrap template, v28
  • Update the parameters given to the CDK deployment in test_cdk.py::test_cdk_bootstrap
  • Expand test_cdk.py::test_cdk_bootstrap_redeploy to
    • use snapshots
    • be parametrized over v20 and v28 of the bootstrap template
  • Correct incorrect formatting of test_dynamic_resolving.py as it got garbled in a previous PR

TODO (follow ups)

  • Should Fn::And and Fn::Or be short circuiting? What about other condition type intrinsic functions?

@github-actions
Copy link

github-actions bot commented Aug 6, 2025 •
edited
Loading

Test Results - Preflight, Unit

22 063 tests  ±0   20 329 ✅ ±0   6m 24s ⏱️ +4s
     1 suites ±0    1 734 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit 416e809. ± Comparison against base commit 2d08a27.

♻️ This comment has been updated with latest results.

@simonrw simonrw added semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases review: merge when ready Signals to the reviewer that a PR can be merged if accepted labels Aug 6, 2025
@simonrw simonrw added this to the 4.8 milestone Aug 6, 2025
@github-actions
Copy link

github-actions bot commented Aug 6, 2025 •
edited
Loading

Test Results (amd64) - Acceptance

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

Results for commit 416e809. ± Comparison against base commit 2d08a27.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Aug 6, 2025 •
edited
Loading

Test Results (amd64) - Integration, Bootstrap

    5 files      5 suites   2h 20m 11s ⏱️
4 982 tests 4 393 ✅ 589 💤 0 ❌
4 988 runs  4 393 ✅ 595 💤 0 ❌

Results for commit 416e809.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Aug 6, 2025 •
edited
Loading

LocalStack Community integration with Pro

    2 files  ±0      2 suites  ±0   1h 43m 43s ⏱️ -39s
4 623 tests +2  4 186 ✅ ±0  437 💤 +2  0 ❌ ±0 
4 625 runs  +2  4 186 ✅ ±0  439 💤 +2  0 ❌ ±0 

Results for commit 416e809. ± Comparison against base commit 2d08a27.

This pull request removes 1 and adds 3 tests. Note that renamed tests count towards both. 
tests.aws.services.cloudformation.resources.test_cdk.TestCdkInit ‑ test_cdk_bootstrap_redeploy

tests.aws.services.cloudformation.resources.test_cdk.TestCdkInit ‑ test_cdk_bootstrap[28]
tests.aws.services.cloudformation.resources.test_cdk.TestCdkInit ‑ test_cdk_bootstrap_redeploy[v20]
tests.aws.services.cloudformation.resources.test_cdk.TestCdkInit ‑ test_cdk_bootstrap_redeploy[v28]

♻️ This comment has been updated with latest results.

@simonrw simonrw force-pushed the cfn/v2/cdk-support branch from b339278 to fad779e Compare August 7, 2025 11:44
@simonrw simonrw marked this pull request as ready for review August 7, 2025 15:17
pinzon
pinzon approved these changes Aug 7, 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.

Thanks for tackling this! Now CDK should be usable with CFnV2

tests/aws/services/cloudformation/v2/test_dynamic_resolving.py
Copy link
Member

@pinzon pinzon Aug 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Still don't get what happened. I didn't see it when I reviewed it the other day.

tests/aws/services/cloudformation/resources/test_cdk.py
Copy link
Member

@pinzon pinzon Aug 7, 2025

Choose a reason for hiding this comment

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

comment: Nice. Now we have an updated version of the CDK template and still test for previous versions.

localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_preproc.py
Copy link
Member

@pinzon pinzon Aug 7, 2025

Choose a reason for hiding this comment

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

Thanks for fixing this!

@simonrw simonrw merged commit 4464cd2 into main Aug 7, 2025
42 checks passed
@simonrw simonrw deleted the cfn/v2/cdk-support branch August 7, 2025 17:03
@simonrw simonrw mentioned this pull request Sep 5, 2025
Merged
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

review: merge when ready Signals to the reviewer that a PR can be merged if accepted semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases

Projects

None yet

Milestone

4.8

Development

Successfully merging this pull request may close these issues.

3 participants

@simonrw @pinzon