Add local module support to the cloudformation package command

[ericzbeard]

This PR adds the ability to write local CloudFormation modules, which are YAML files that contain Parameters and Resources. These modules can be included in a parent template in a new Modules section. Modules are configured by a combination of formal Parameters (which coincide with the Properties defined in the parent) and an Overrides attribute, which allows users full control over the rendered output. This is a port from the functionality that exists in the Rain CLI. See the snippets below for a basic example.

Parent template

Modules:
  Content:
    Source: ./basic-module.yaml
    Properties:
      Name: foo
    Overrides:
      Bucket:
        Properties:
          OverrideMe: def

basic-module.yaml

Parameters:
  Name:
    Type: String
Resources:
  Bucket:
    Type: AWS::S3::Bucket
    Properties:
      BucketName: !Ref Name
      OverrideMe: abc

Output from the package command

Resources:
  ContentBucket:
    Type: AWS::S3::Bucket
    Properties:
      BucketName: foo
      OverrideMe: def

Design Decisions

1. Modules can be imported into a template (or into a module itself) from a new Modules section. In Rain and in registry modules, this section does not exist. Instead, the imported content is configured in a resource and the source of the module is embedded in the Type attribute. This PR implements both ways of configuring an imported module, giving customers the option of whichever feels more intuitive. The benefit of using a Modules section is that it will be more intuitive if we decide to allow injecting other sections other than Resources, like Outputs.

2. Logical IDs are a concatenation of the module name and the logical id defined in the module. Unlike registry modules, users do not have the option of referring to the resources with a dot between the names.

3. The Overrides section is not considered an escape hatch of last resort. It is an encouraged way to configure modules without requiring the module author to anticipate every possible need. IaC abstractions are leaky by nature and we expect users to understand the internals of any module they import. Deciding how many actual Parameters to include in a module is highly opinionated and left up to authors. One caveat here is that Overrides are fragile, since they will break if the module author changes any of the logical IDs in the module. See item 5, Outputs, as a way to mitigate this.

4. This functionality has been added to the aws cloudformation package command. We could have created a new dedicated command for module packaging, like synth. This is an easily reversible decision. The code is ported from the Rain pkg command, which was originally written to mimic the behavior of aws cloudformation package, so it seems like the right place to put it. We removed the --s3-bucket requirement from the command, waiting to check for that parameter when we see that you need to upload something to S3.

5. Modules can define Outputs, which are key-value pairs that can be referenced by the parent. This gives module authors a formal way to pass values to the consumer without them needing to rely on knowing the internal structure of the module, which will be fragile if the module author changes any of the logical IDs.

6. Instead of using Fn::ForEach as a way of implementing loops, we added a Map attribute to a local module resource. The module output is repeated for each element in the map, substituting $MapValue and $MapIndex in module Properties. This can only be used with lists that can be resolved at build time.

7. The PR also adds a new Constants section to templates and modules. This feature is not absolutely necessary for an implementation of modules, but it serves a similar purpose of significantly reducing copy-paste when authoring CloudFormation.

Additional features to add on this PR or in the future

[ ] - Download modules from S3
[ ] - ForEach support within a module and in the parent template
[ ] - Module package support like in Rain, with an alias to a zip file

How to test this PR

Check out my branch and install the aws cli into a local Python environment.

git clone [email protected]:ericzbeard/aws-cli.git
git fetch origin cfn-modules
git checkout cfn-modules
python3 -m venv .env # Or whatever you use for python environments
source .env/bin/activate
pip install -e .
aws cloudformation package \
    --template-file tests/unit/customizations/cloudformation/modules/type-template.yaml

Examples

See the test templates in this PR at tests/unit/customizations/cloudformation/modules/ and also a draft PR with a full serverless webapp here: aws-cloudformation/aws-cloudformation-templates#457

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 0% with 683 lines in your changes missing coverage. Please review.

Project coverage is 0.07%. Comparing base (cdea504) to head (32fa228).
Report is 223 commits behind head on develop.

Files with missing lines	Patch %	Lines
awscli/customizations/cloudformation/modules.py	0.00%	416 Missing ⚠️
awscli/customizations/cloudformation/parse_sub.py	0.00%	79 Missing ⚠️
...customizations/cloudformation/module_conditions.py	0.00%	72 Missing ⚠️
.../customizations/cloudformation/module_constants.py	0.00%	56 Missing ⚠️
...customizations/cloudformation/artifact_exporter.py	0.00%	30 Missing ⚠️
...li/customizations/cloudformation/module_visitor.py	0.00%	19 Missing ⚠️
awscli/customizations/cloudformation/exceptions.py	0.00%	6 Missing ⚠️
awscli/customizations/cloudformation/package.py	0.00%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           develop   #9124      +/-   ##
==========================================
- Coverage     0.08%   0.07%   -0.01%     
==========================================
  Files          210     215       +5     
  Lines        16955   17662     +707     
==========================================
  Hits            14      14              
- Misses       16941   17648     +707     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[kddejong]

Solid start. I have some corrections and some questions.

[kddejong]

I feel like there are better flows here. You pass in k,v but also the dict and parent k that holds k, v? From what I can tell the big reason for that is to switch an object using an intrinsic function back into a value. You are taking advantage of the fact that dict/list are sent by reference. It feels like an approach that does a return instead of relying on a reference would read better.

[ericzbeard]

You mean something like d[n] = self.resolve(k, v)? Maybe that is better. But I am hesitant to change it, this was the trickiest piece of code to get right.

[benbridts]

I was thinking about how to refer to values from a template and noticed that that is a place where the leaky abstraction might be a downside (most of it is true for the overrides too, but it feels more expected there):

If you want to reference a resource in a module you would use e.g. !GetAtt ContentBucket.Arn, which means:

• the module author can't change any resource name in the template
• (not a problem now, but possibly when extending to allow conditions) there is no way to have to different resources cover the same generic module. Imagine e.g a ApiGateway module that creates an HTTP or REST api, depending on a parameter/condition.
• Even if users are not using any overrides, they would need to know the resources (and names) created by the template - I thing the same argument as why this is not a problem with overrides can be applied here, but if you see overrides as a more advanced use case, it might not.

Having an "outputs" or "outputs" equivalent could solve this.

e.g

Parent template

Modules:
  Content:
    Source: ./basic-module.yaml
    Properties:
      Name: foo
Outputs:
  ExampleOutput:
    Value: !GetAtt Content.BucketArn

basic-module.yaml

Parameters:
  Name:
    Type: String
Resources:
  Bucket:
    Type: AWS::S3::Bucket
    Properties:
      BucketName: !Ref Name
Outputs:
  BucketArn: !GetAtt Bucket.Arn

Output from the package command

Resources:
  ContentBucket:
    Type: AWS::S3::Bucket
    Properties:
      BucketName: foo
Outputs:
  ExampleOutput:
    Value: !GetAtt ContentBucket.Arn

[kddejong]

Agreed with Ben on this one. You could still allow for an override capability that allows for a missing output that may be needed but having to understand the module resource names can get frustrating and prevents flexible in changing them. There are going to be limitations in this approach that are similar to modules. Adding mappings/conditions into a module will limit its usage because it cannot rely on another resource from the parent template or other module.

[ericzbeard]

Having an "outputs" or "outputs" equivalent could solve this.

I don't see any reason not to add Outputs. It's definitely a more predictable way to reference resources in the module. I still want the Overrides section and the ability to reference things in the module directly if you know the name, but that can be considered a more advanced use case. If you're consuming a 3rd party module, the risk is higher than if you're referencing your own local modules.

[ericzbeard]

Having an "outputs" or "outputs" equivalent could solve this.

Implemented here: 160be7c

[kddejong]

Today CloudFormation doesn't support this. The only concern I have with this is it could cause infinite loops.

Conditions:
  One:
    Fn::Equals:
    - !If [Two, "A", "B"]
    - "A"
  Two:
    Fn::Equals:
    - !If [One, "A", "B"]
    - "B"

Later on we call out that Fn::If isn't supported so not sure why it is here.

[kddejong]

If I'm reading this correct its possible this becomes val0 = "foo", val1 = {"Ref": "SomeResource"}. This would evaluate to false but could be true if this resource returns the value foo

[benkehoe]

I'm for this kind of functionality, but why is the module interface template-like (parameters and output, CloudFormation typing), and not resource-like? To my mind, the parameters-and-outputs part should look like a resource definition schema, and the use of a module should look like a resource.

The other thing I'd like to see is something like, the CLI can accept as input not (just) the template file with implicit references to other files, but a zip file containing a template (with a well-known name) along with the associated modules as a single input. Maybe aligning with CDK's cloud assembly format. Because the direction should be, that zip file can be passed directly into CreateStack.

[ericzbeard]

Thanks for the feedback!

I'm for this kind of functionality, but why is the module interface template-like (parameters and output, CloudFormation typing), and not resource-like? To my mind, the parameters-and-outputs part should look like a resource definition schema, and the use of a module should look like a resource.

I worry that the added complexity might affect adoption. Can you give an example of why defining the module with a schema would be beneficial? Much of the schema can be implied from the current format, can't it?

The other thing I'd like to see is something like, the CLI can accept as input not (just) the template file with implicit references to other files, but a zip file containing a template (with a well-known name) along with the associated modules as a single input. Maybe aligning with CDK's cloud assembly format. Because the direction should be, that zip file can be passed directly into CreateStack.

Yes, we want to support zip files as input to CreateStack. That might come later, after we've validated the module format client-side. We could start with passing a directory name to the package command, with the expected parent template file name in the directory. Any preference on the name? main.yaml, parent.yaml, template.yaml?

[benbridts]

Any preference on the name? main.yaml, parent.yaml, template.yaml?

My personal preference is template.yaml, because it's what SAM creates by default. However, template.yml should probably also be supported (and so there would have to be a defined search order anyway - or an error if both exists)