Skip to content

CFn: implement language extensions transform #12813

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 12 commits into from
Jul 8, 2025
Merged

CFn: implement language extensions transform #12813

merged 12 commits into from
Jul 8, 2025

Conversation

simonrw
Copy link
Contributor

@simonrw simonrw commented Jun 27, 2025 •
edited
Loading

Motivation

We have had many requests for the Language Extensions Transform. These provide additional useful functionality to CloudFormation.

In particular, the Fn::ForEach function is useful for generating many templated resources.

Changes

Unfortunately the implementation is quite dense, relying on recurse_object which creates some complex situations. Though I found it quite straightforward to write the implementation, I feel it might be confusing to read.

  • (minor) refactor resolve_refs_recursively to use a ResolveRefsRecursivelyContext dataclass with resolve(thing) method
    • only use it partially since we don't plan on keeping this implementation
  • Add fixture for deploying a template with transform, then calling get_template and get_template_resources
  • Implement Fn::Length
  • Implement Fn::ForEach for a simple case (taken from AWS docs)
  • Implement Fn::ForEach for a specific customer use case
  • Implement Fn::ForEach for a nested use case (taken from AWS docs)
  • Implement Fn::ToJsonString for use case from AWS docs
  • Add unit test for expand_fn_foreach so we can quicker iterate cases that don't require deployment of CFn templates

@simonrw simonrw added the semver: patch Non-breaking changes which can be included in patch releases label Jun 27, 2025
@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 27, 2025 •
edited
Loading

Test Results - Preflight, Unit

21 796 tests  +1   20 139 ✅ +1   6m 15s ⏱️ -2s
     1 suites ±0    1 657 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit 4dc46ee. ± Comparison against base commit 1fcf3e9.

♻️ This comment has been updated with latest results.

@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 27, 2025 •
edited
Loading

Test Results (amd64) - Acceptance

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

Results for commit 4dc46ee. ± Comparison against base commit 1fcf3e9.

♻️ This comment has been updated with latest results.

@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 28, 2025 •
edited
Loading

Test Results (amd64) - Integration, Bootstrap

    5 files      5 suites   2h 18m 2s ⏱️
5 276 tests 4 348 ✅ 928 💤 0 ❌
5 282 runs  4 348 ✅ 934 💤 0 ❌

Results for commit 4dc46ee.

♻️ This comment has been updated with latest results.

@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 28, 2025 •
edited
Loading

LocalStack Community integration with Pro

    2 files  ±0      2 suites  ±0   1h 43m 44s ⏱️ - 2m 35s
4 919 tests +5  4 143 ✅ +5  776 💤 ±0  0 ❌ ±0 
4 921 runs  +5  4 143 ✅ +5  778 💤 ±0  0 ❌ ±0 

Results for commit 4dc46ee. ± Comparison against base commit 1fcf3e9.

♻️ This comment has been updated with latest results.

@simonrw simonrw force-pushed the cfn/language-extensions branch 2 times, most recently from 69e9fb6 to 5991ea7 Compare July 3, 2025 15:21
@simonrw simonrw added semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases and removed semver: patch Non-breaking changes which can be included in patch releases labels Jul 3, 2025
@simonrw simonrw force-pushed the cfn/language-extensions branch from 5991ea7 to 58b5e60 Compare July 4, 2025 14:14
@simonrw simonrw marked this pull request as ready for review July 4, 2025 15:52
simonrw
simonrw commented Jul 4, 2025
View reviewed changes
localstack-core/localstack/services/cloudformation/engine/transformers.py
@@ -26,6 +30,29 @@
TransformResult = Union[dict, str]


@dataclass
class ResolveRefsRecursivelyContext:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Slight improvement to the resolve_refs_recursively API however I don't plan on continuing this refactoring

localstack-core/localstack/services/cloudformation/engine/v2/change_set_model_executor.py
)
if resource_provider is None and not config.CFN_IGNORE_UNSUPPORTED_RESOURCE_TYPES:
raise NoResourceProvider
if resource_provider is None:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a fix for a previous PR where I incorrectly logged the not available message regardless of whether we could load the resource provider or not.

localstack-core/localstack/services/cloudformation/provider.py
@@ -269,12 +268,15 @@ def create_stack(self, context: RequestContext, request: CreateStackInput) -> Cr
state.stacks[stack.stack_id] = stack
return CreateStackOutput(StackId=stack.stack_id)

# HACK: recreate the stack (including all of its confusing processes in the __init__ method
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This comment is to add clarity more than anything else. The movement of the construction of the stack is not relevant here

localstack-core/localstack/services/cloudformation/provider.py
@@ -512,8 +514,18 @@ def get_template(

if template_stage == TemplateStage.Processed and "Transform" in stack.template_body:
copy_template = clone(stack.template_original)
copy_template.pop("ChangeSetName", None)
copy_template.pop("StackName", None)
for key in [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are other keys that are not present in the response, so clean up here

localstack-core/localstack/testing/pytest/fixtures.py
@@ -1106,6 +1106,8 @@ def _deploy(

if template_path is not None:
template = load_template_file(template_path)
if template is None:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This has annoyed me SO much!

simonrw
simonrw commented Jul 4, 2025
View reviewed changes
localstack-core/localstack/services/cloudformation/engine/transformers.py

def _visit(obj, path, **_):
# Fn::ForEach
# TODO: can this be used in non-resource positions?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, yes it can... 🤦‍♂️

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should delete the TODO

pinzon
pinzon approved these changes Jul 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.

Thank you for implementing this. The changes look good. Hopefully this will be enough for the use case.

@simonrw simonrw merged commit 752e01d into master Jul 8, 2025
39 checks passed
@simonrw simonrw deleted the cfn/language-extensions branch July 8, 2025 06:30
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
semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases
Projects
None yet
Milestone
No milestone
Development

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