RFC 0009: Add looping functionality in CFN Templates #75

MalikAtalla-AWS · 2022-05-13T17:54:59Z

Language Enhancement Request For Comment

This is a request for comments about a new intrinsic function called Fn::ForEach to support a looping functionality in CloudFormation templates. See the tracking issue (#9) for
additional details.

Licensing

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

benkehoe · 2022-05-13T21:14:25Z

I have a number of different pieces of feedback.

Naming

In the issue I originally opened (#41), I proposed Fn::Map; I believe it's important for users to understand this as a functional concept, rather than an imperative one. They aren't writing a for loop, they are mapping a JSON template over a set of values. I'd settle for Fn::Foreach, though. I believe there's also value in a Fn::Reduce (for example, to create a list of ARNs from a collection of resources created with this function), which supports Fn::Map as a name.

Syntax

Index/value parameter names

I don't like the need to define the index and value parameter names. They're probably always going to be the same, it's just one more thing to type out and one more thing to go wrong. Could they be made optional, defaulting to some useful names? This brings me to my next point...

Function parameter ordering

There are so many parameters to this function. I understand that all other intrinsic functions use positional parameters, but maybe this is a place to use named parameters. For example:

!Foreach
  Input: !Ref AmiIds
  Value:
    Type: AWS::EC2::Instance
    Properties:
      InstanceType: "m1.small"
      ImageId: !Ref x
  LogicalId: !Sub Instance${i}
  ParameterNames: ["i", "x"]

This would allow for multiple independently-optional parameters, like LogicalId and ParameterNames. Speaking of LogicalId...

Use cases

I think there's actually four use cases here.

Inserting a list as a value
Inserting an object as a value
Merging an object into a parent
Creating a set of CloudFormation resources

Use cases 3 and 4 are both accomplished by the LogicalId parameter. Use case 2 is not supported.

Here's I'd propose:

Instead of LogicalId and its current semantics, allow a Key parameter that, if set, means the function produces an object rather than a list. By default, this object is the value where the function is defined.
Allow a boolean Merge parameter that requires the Key parameter also be set, that when true, merges the object defined by the function with its parent.
Define a separate syntax for resources. Here's why...

Reifying resource mapping

If this intrinsic function simply allows users to create multiple resources with an intrinsic function that shuffles YAML around, we're missing a giant opportunity.
We should be enabling CloudFormation to represent a collection of resources defined by mapping over some values as a first-class concept.
Say that I want subnets in every AZ. That's an architectural intention that should be fully represented in the cloud. It should get a name, it should be able to inspected, etc.

This proposal doesn't serve that purpose. I think it can be a separate RFC, but imagine something like

Resources:
  Subnets:
    Type: CloudFormation::Map
    Input: [...]
    LogicalId: ... # actual logical id for the resource
    ResourceValue: ...
    # etc, similar syntax to the function

If we agree that such a thing should exist, it will help focus the syntax of this intrinsic function on use cases 1-3 (where use case 3 serves to allow users to create a non-reified set of resources).

michaelbrewer · 2022-05-14T03:53:20Z

Fn::Map or at worst Fn::Foreach make a-lot more sense then just Fn::For.

cloudyparts · 2022-05-14T17:10:56Z

Naming

Issue #9 describes Fn::For as;

Fn::For traverses a single given List of Strings from first to last, and over each iteration, it references each item under a given template snippet to be replicated.

This definition describes an iterator which implies functions like Map, Reduce, Filter, and Zip.

The term For may confuse developers since this term has a long history as A control-flow-statement.

I agree with @benkehow Fn::Map communicates the intent better than Fn::For.

jlhood · 2022-05-17T23:28:46Z

Really appreciate all the feedback and engagement on this RFC! After some internal discussions, we've determined it needs significant rework. We're going to iterate a bit internally and then post a new revision for more feedback. Stay tuned!

MalikAtalla-AWS · 2022-06-21T21:51:29Z

Hey all, I have posted a new revision addressing some of the feedback. This is a summary of the changes:

We are changing the name of the function to Fn::Map
We are making the parameters named, instead of positional. We figured that is warranted given the number of parameters, some of them being optional and how that impacts readability.
The parameters for the index and value variables are now optional. We think customizing them is an important use case, but in most cases, users will be fine with the defaults.
The LogicalId parameter is now simply called "key"
We're introducing a new intrinsic function called Fn::Merge, which can be used to merge the result of Fn::Map into its parent object.

RFCs/0009-Fn::For.md

benbridts · 2022-06-22T10:24:14Z

RFCs/0009-Fn::For.md

+{
+  "Fn::Map": {
+    "index": "Index",
+    "value": "Value",


Things I'm wondering:

Would it be clearer to use IndexParameter and ValueParameter for the keys (to make it clear that you're configuring something you will !Ref later)

Would it be clearer to use Item instead of Value? This might be related to the languages I've been exposed to, but I'd use Key-Value or Index-Item, not using Value prevents confusion with the Key parameter

I think "key" and "value" if they are both used should be outputs, currently "fragment" is used for the output value. We've got four parameters here:

input value from the input collection (currently "value")

index of the input value (currently "index")

output value (currently "fragment")

optional output key (currently "key")
Maybe making them more explicit makes sense? InputIndex, InputValue, OutputValue, OutputKey? It would leave open the possibility that in the future the collection could be an object, which would would then involve an InputKey parameter as well.

RFCs/0009-Fn::For.md

benbridts · 2022-06-22T10:29:03Z

RFCs/0009-Fn::For.md

+* Caveat: Make sure Logical Ids are alphanumeric characters
+
+#### SSM/AWS List Type Parameters
+* SSM/AWS List type parameters can be referenced in `Fn::Map`. Only List type parameter is allowed.


Should we add an example of using !Split to convert a string to a list?

RFCs/0009-Fn::For.md

benbridts · 2022-06-22T10:43:37Z

RFCs/0009-Fn::For.md

+     Properties:
+        SecurityGroups:
+          - Fn::Map:
+              collection: [!Ref SecurityGroup]


!GetAtt x.DnsEntries of a AWS::EC2::VPCEndpoint is also an interesting example

RFCs/0009-Fn::For.md

MalikAtalla-AWS · 2022-06-22T16:26:17Z

Really appreciate your feedback @benbridts and @benkehoe

benkehoe · 2022-06-22T17:14:55Z

re-posting this as a general comment so it doesn't get lost on an old revision:

from @benbridts

Things I'm wondering:

Would it be clearer to use IndexParameter and ValueParameter for the keys (to make it clear that you're configuring something you will !Ref later)

Would it be clearer to use Item instead of Value? This might be related to the languages I've been exposed to, but I'd use Key-Value or Index-Item, not using Value prevents confusion with the Key parameter

I think "key" and "value" if they are both used should be outputs, currently "fragment" is used for the output value. We've got four parameters here:

input value from the input collection (currently "value")
index of the input value (currently "index")
output value (currently "fragment")
optional output key (currently "key")

Maybe making them more explicit makes sense? InputIndex, InputValue, OutputValue, OutputKey? It would leave open the possibility that in the future the collection could be an object, which would would then involve an InputKey parameter as well.

benkehoe · 2022-06-22T17:16:46Z

Re-posting this as a general comment so it doesn't get lost on an old revision:

Both @benbridts and I commented that we think the parameters should be CamelCase to be consistent with CloudFormation conventions

benkehoe · 2022-06-22T17:17:55Z

@MalikAtalla-AWS FYI amending the commit and pushing means comments on previous commits are no longer visible in the diff, I'd recommend creating new commits and only squashing when it's ready for merging

RFCs/0009-Fn::Map.md

benbridts · 2022-06-28T14:49:25Z

I think "key" and "value" if they are both used should be outputs, currently "fragment" is used for the output value. We've got four parameters here:

input value from the input collection (currently "value")

index of the input value (currently "index")

output value (currently "fragment")

optional output key (currently "key")

(and collection, but that one seems to be clear)

Maybe making them more explicit makes sense? InputIndex, InputValue, OutputValue, OutputKey? It would leave open the possibility that in the future the collection could be an object, which would would then involve an InputKey parameter as well.

the current value and index do not feel like inputs to me. They are more meta-inputs (used within the fragment) than inputs (used by the map function).

Since the current default for Value is X, I would propose something like element, item or variable.

I'm not the biggest fan of using "Output" in the names for fragment and key, but if you translates what happens to python, it makes sense to use Key and Value in the names. And even though output is a fitting name, they are also templates. (template already has a specific meaning in CloudFormation, but I don't think this has to be a problem)

def cfn_map(collection: List, key: str, fragment: str):
    return {key.format(i=i, x=x): fragment.format(i=i, x=x) for i, x in enumerate(collection)}
    
 # or with different names
 
 def cfn_map(collection: List, key: str, value: str):
    return {key.format(i=i, x=x): value.format(i=i, x=x) for i, x in enumerate(collection)}

This leads me to thinking something like

IndexName (how you will refer to the index)
VariableName, ItemName or ElementName (how you will refer to the variable)
Collection (the actual collection you are mapping over)
KeyTemplate or OutputKey (the key you will output, that can also reference variables)
ValueTemplate or OutputValue (the value/fragment you will output, that can also reference variables)

arthurboghossian · 2023-03-08T20:56:44Z

Update: The AWS CloudFormation team really appreciates the interest and feedback we've received on this RFC! In the past months, our team did a deep dive to better understand specific customer pain points and brainstormed ideas with customers and community representatives. Based on this deep dive, we are updating this RFC with a proposal to add a single Fn::ForEach intrinsic function. Let us know your feedback and thoughts!

…ould be provided in the OutputKey

…including comments

RFCs/0009-Fn::ForEach.md

Co-authored-by: Malik <60721392+MalikAtalla-AWS@users.noreply.github.com>

benkehoe · 2023-03-21T17:36:17Z

RFCs/0009-Fn::ForEach.md

+        "Identifier",
+        ["Value1", "Value2"], ## Collection
+        {
+            "OutputKey": "OutputValue" ## Ex: {"OutputKeyName${Identifier}": "OutputValue"}


Using only a single item in the object could be read as only permitting one item; it might be useful to include two items, or a comment indicating more items are possible

Makes sense! I’ve updated the syntax to include a comment to indicate there can be multiple key-value pairs within the 3rd positional parameter

benkehoe · 2023-03-21T17:42:29Z

RFCs/0009-Fn::ForEach.md

+    "Resources": {
+        "Fn::ForEach::Buckets": [
+            "Identifier",
+            ["A", "B", "C"],


Perhaps this example might be better with the collection from a Mapping, so that it's not duplicated between the two?

Yeah that would be better, so I’ve updated the example accordingly

benkehoe · 2023-03-21T17:43:34Z

RFCs/0009-Fn::ForEach.md

+    "Outputs": {
+        "SecondInstanceId": {
+            "Description": "Instance Id for InstanceB",
+            "Value": {"Ref": "InstanceB"}


This feels gross, having to reference a generated identifier, relative to something that is more like !Select [1, !Ref Instances] (the ForEach name has to be a unique identifier in the template namespace, right?)

Yeah the ForEach name has to be a unique identifier in the template namespace (see Constraint 3 in RFC), where based on the number of asks for the feature, !Ref on the ForEach name, which would return a list of logicalId’s generated by ForEach, can be implemented in the future (see GitHub issue here with details on Additional Enhancements to ForEach)

benkehoe · 2023-03-21T17:46:53Z

RFCs/0009-Fn::ForEach.md

+
+## Potential Follow-up Features
+
+Features like supporting iterating over a key-valur pair or allowing Ref/Fn::GetAtt on the UniqueLoopName are out of scope for this RFC.


Typo on "key-valur pair"

benkehoe · 2023-03-21T17:48:13Z

Needed in the FAQ, because people will try this immediately:

Can I use an object for the collection instead of a list?
Can I output a list instead of merge into an object?

benkehoe · 2023-03-21T17:50:03Z

RFCs/0009-Fn::ForEach.md

+
+|FunctionName	|Pros	|Cons	|
+|---	|---	|---	|
+|Fn::ForEach	|Addresses valid feedback in GitHub pull requestIntrinsic function name indicates it’s a functional concept and not an imperative oneForEach Illustrates that we’re operating on a template fragment over a set of valuesGives customers familiar with coding concepts the idea that some form of “looping” (or replication) is done	|Intrinsic function name is potentially not unclear to customers who aren’t familiar with coding concepts; however, the syntax of Fn::ForEach::<ResourceLogicalId> would give users an idea that within the function, we define the template fragment "for each" resource	|


The formatting on these table entries is a mess

Nice catch! Looks like the resolved markdown looks ok, but I've updated the raw markdown to be clean as well.

benkehoe · 2023-03-21T17:51:41Z

RFCs/0009-Fn::ForEach.md

@@ -0,0 +1,1461 @@
+# Looping functionality for CFN Templates


I'm happy with Fn::ForEach as a name, but I'd prefer to stay away from "loop" as much as possible, if there's a way to do that. At least use it as an opportunity to help users understand that they're using it in a functional programming way, rather than an imperative one.

That makes sense! I've updated the title to not mention "looping", to ensure it isn’t misunderstood as being an imperative solution.

It's probably impossible to avoid completely, but as a general direction I think it's wise

rhbecker · 2023-06-17T04:47:45Z

Are there outstanding concerns preventing the assigned reviewers from approving this?

I'm looking forward to taking advantage of this capability, and am hoping it can soon move towards implementation.

arthurboghossian · 2023-06-19T17:42:58Z

Thanks for the callout @rhbecker!

Believe there aren't any outstanding concerns, so I'll re-request reviews and will merge the pull request if I receive the necessary approvals.

rhbecker · 2023-06-23T17:16:45Z

Sorry to be a pest, but if I'm reading the tracking issue #9 correctly, in order to keep things moving, I think someone needs to add an approved label to this PR, and a status/final-comments-period label to the tracking issue?

MalikAtalla-AWS · 2023-06-23T18:12:16Z

You're right @rhbecker. Appreciate the reminder. We'll take care of that.

rhbecker · 2023-07-07T21:23:35Z

🤔 Maybe instead of pestering you about labels (which still don't seem quite right on this or the tracking issue), I should instead ask whether implementation is fully unblocked.

My fear is that this issue is stuck in some queue, due to a "paperwork" blockage, when it could otherwise be ready for some hungry CFN engineer to start working on.

If it's already unblocked, or perhaps even already in progress, I can sit back, relax, and patiently (🤞) await completion.

MalikAtalla-AWS · 2023-07-10T10:33:21Z

Hey @rhbecker, thanks for following up. This issue is and has been unblocked. We have set the status of the RFC to "implementing" in the main readme of this repo and I (just now) added the approved-label to this PR. (Maybe we should consider removing that line about the approved label from our readme. The info in the main readme and the fact that the PR is merged is probably indication enough about the status.)

MalikAtalla-AWS force-pushed the loops branch from 1862b64 to db4aadd Compare May 13, 2022 18:20

MalikAtalla-AWS mentioned this pull request May 13, 2022

RFC 0009: Adding looping functionality in CFN Template (Fn::For) #19

Closed

MalikAtalla-AWS force-pushed the loops branch from db4aadd to 361d344 Compare May 13, 2022 18:36

MalikAtalla-AWS force-pushed the loops branch from 361d344 to ebe7834 Compare June 21, 2022 21:47

MalikAtalla-AWS changed the title ~~RFC 0009: Add looping functionality in CFN Templates (Fn::For)~~ RFC 0009: Add looping functionality in CFN Templates Jun 21, 2022

MalikAtalla-AWS force-pushed the loops branch 2 times, most recently from 8b83aa0 to a4a9628 Compare June 21, 2022 22:21

benbridts reviewed Jun 22, 2022

View reviewed changes

RFCs/0009-Fn::For.md Outdated Show resolved Hide resolved

RFCs/0009-Fn::For.md Outdated Show resolved Hide resolved

benbridts reviewed Jun 22, 2022

View reviewed changes

benkehoe reviewed Jun 22, 2022

View reviewed changes

RFCs/0009-Fn::For.md Outdated Show resolved Hide resolved

benkehoe reviewed Jun 22, 2022

View reviewed changes

RFCs/0009-Fn::For.md Outdated Show resolved Hide resolved

Add RFC for looping functionality in CloudFormation templates

124b5fb

MalikAtalla-AWS force-pushed the loops branch from a4a9628 to 124b5fb Compare June 22, 2022 17:00

Make parameters upper case to be consistent with CFN conventions

71a33dc

mluk-aws reviewed Jun 22, 2022

View reviewed changes

Address review comments from mluk-aws

90aebbf

nhienton reviewed Jun 22, 2022

View reviewed changes

RFCs/0009-Fn::Map.md Outdated Show resolved Hide resolved

RFCs/0009-Fn::Map.md Outdated Show resolved Hide resolved

RFCs/0009-Fn::Map.md Outdated Show resolved Hide resolved

RFCs/0009-Fn::Map.md Outdated Show resolved Hide resolved

Address review comments from nhienton

3a6f4dc

arthurboghossian mentioned this pull request Mar 8, 2023

Fn::ForEach intrinsic function - Additional Enhancements #118

Open

Modify RFC based on further research and community feedback

8215118

arthurboghossian added 2 commits March 9, 2023 13:18

Include example of 3rd parameter in Syntax to show that Identifier sh…

d822bb5

…ould be provided in the OutputKey

Modify markdown for JSON syntax to be yaml to prevent red error when …

2743e1b

…including comments

MalikAtalla-AWS commented Mar 13, 2023

View reviewed changes

RFCs/0009-Fn::ForEach.md Outdated Show resolved Hide resolved

MalikAtalla-AWS commented Mar 13, 2023

View reviewed changes

RFCs/0009-Fn::ForEach.md Outdated Show resolved Hide resolved

MalikAtalla-AWS commented Mar 13, 2023

View reviewed changes

RFCs/0009-Fn::ForEach.md Show resolved Hide resolved

RFCs/0009-Fn::ForEach.md Show resolved Hide resolved

Address review comments from MalikAtalla-AWS

6685ef5

Co-authored-by: Malik <60721392+MalikAtalla-AWS@users.noreply.github.com>

benkehoe reviewed Mar 21, 2023

View reviewed changes

Address review comments from benkehoe

603922c

lejiati approved these changes Jun 19, 2023

View reviewed changes

cdavidxu-hub approved these changes Jun 19, 2023

View reviewed changes

arthurboghossian self-assigned this Jun 19, 2023

arthurboghossian merged commit 3e50091 into aws-cloudformation:main Jun 19, 2023

MalikAtalla-AWS added the approved RFC PR approved by CFN engineers label Jul 10, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RFC 0009: Add looping functionality in CFN Templates #75

RFC 0009: Add looping functionality in CFN Templates #75

MalikAtalla-AWS commented May 13, 2022 •

edited by arthurboghossian

Loading

benkehoe commented May 13, 2022 •

edited

Loading

michaelbrewer commented May 14, 2022

cloudyparts commented May 14, 2022 •

edited

Loading

jlhood commented May 17, 2022

MalikAtalla-AWS commented Jun 21, 2022

benbridts Jun 22, 2022

benkehoe Jun 22, 2022

benbridts Jun 22, 2022

benbridts Jun 22, 2022

MalikAtalla-AWS commented Jun 22, 2022

benkehoe commented Jun 22, 2022

benkehoe commented Jun 22, 2022

benkehoe commented Jun 22, 2022

benbridts commented Jun 28, 2022

arthurboghossian commented Mar 8, 2023

benkehoe Mar 21, 2023

arthurboghossian Mar 22, 2023

benkehoe Mar 21, 2023

arthurboghossian Mar 22, 2023

benkehoe Mar 21, 2023 •

edited

Loading

arthurboghossian Mar 22, 2023

benkehoe Mar 21, 2023

benkehoe commented Mar 21, 2023

benkehoe Mar 21, 2023

arthurboghossian Mar 22, 2023

benkehoe Mar 21, 2023

arthurboghossian Mar 22, 2023

benkehoe Mar 23, 2023

rhbecker commented Jun 17, 2023

arthurboghossian commented Jun 19, 2023

rhbecker commented Jun 23, 2023

MalikAtalla-AWS commented Jun 23, 2023

rhbecker commented Jul 7, 2023

MalikAtalla-AWS commented Jul 10, 2023


		## Potential Follow-up Features

		Features like supporting iterating over a key-valur pair or allowing Ref/Fn::GetAtt on the UniqueLoopName are out of scope for this RFC.

RFC 0009: Add looping functionality in CFN Templates #75

RFC 0009: Add looping functionality in CFN Templates #75

Conversation

MalikAtalla-AWS commented May 13, 2022 • edited by arthurboghossian Loading

Language Enhancement Request For Comment

Licensing

benkehoe commented May 13, 2022 • edited Loading

Naming

Syntax

Index/value parameter names

Function parameter ordering

Use cases

Reifying resource mapping

michaelbrewer commented May 14, 2022

cloudyparts commented May 14, 2022 • edited Loading

Naming

jlhood commented May 17, 2022

MalikAtalla-AWS commented Jun 21, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MalikAtalla-AWS commented Jun 22, 2022

benkehoe commented Jun 22, 2022

benkehoe commented Jun 22, 2022

benkehoe commented Jun 22, 2022

benbridts commented Jun 28, 2022

arthurboghossian commented Mar 8, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benkehoe Mar 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benkehoe commented Mar 21, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rhbecker commented Jun 17, 2023

arthurboghossian commented Jun 19, 2023

rhbecker commented Jun 23, 2023

MalikAtalla-AWS commented Jun 23, 2023

rhbecker commented Jul 7, 2023

MalikAtalla-AWS commented Jul 10, 2023

MalikAtalla-AWS commented May 13, 2022 •

edited by arthurboghossian

Loading

benkehoe commented May 13, 2022 •

edited

Loading

cloudyparts commented May 14, 2022 •

edited

Loading

benkehoe Mar 21, 2023 •

edited

Loading