Skip to content

Commit

Permalink
Implement module outputs
Browse files Browse the repository at this point in the history
  • Loading branch information
ericzbeard committed Dec 13, 2024
1 parent ec32ecf commit 160be7c
Show file tree
Hide file tree
Showing 5 changed files with 173 additions and 7 deletions.
137 changes: 131 additions & 6 deletions awscli/customizations/cloudformation/modules.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

"This file implements local module support for the package command"

# pylint: disable=fixme
# pylint: disable=fixme,too-many-instance-attributes

import os
import traceback
Expand Down Expand Up @@ -45,6 +45,7 @@
MODULES = "Modules"
TYPE = "Type"
LOCAL_MODULE = "LocalModule"
OUTPUTS = "Outputs"


def process_module_section(template, base_path):
Expand Down Expand Up @@ -227,8 +228,8 @@ def __init__(self, template, module_config):
# Parameters defined in the module
self.params = {}

# TODO: What about Conditions, Mappings, Outputs?
# Is there a use case for importing those into the parent?
# Outputs defined in the module
self.outputs = {}

def __str__(self):
"Print out a string with module details for logs"
Expand Down Expand Up @@ -256,6 +257,9 @@ def process(self):
if PARAMETERS in module_dict:
self.params = module_dict[PARAMETERS]

if OUTPUTS in module_dict:
self.outputs = module_dict[OUTPUTS]

# Recurse on nested modules
base_path = os.path.dirname(self.source)
section = ""
Expand All @@ -274,8 +278,126 @@ def process(self):
for logical_id, resource in self.resources.items():
self.process_resource(logical_id, resource)

self.process_outputs()

return self.template

def process_outputs(self):
"""
Fix parent template output references.
In the parent you can !GetAtt ModuleName.OutputName
This will be converted to !GetAtt ModuleName + OutputValue
Recurse over all sections in the parent template looking for
GetAtts and Subs that reference a module output value.
"""
sections = [RESOURCES, OUTPUTS] # TODO: Any others?
for section in sections:
if section not in self.template:
continue
for k, v in self.template[section].items():
self.resolve_outputs(k, v, self.template, section)

def resolve_outputs(self, k, v, d, n):
"""
Recursively resolve GetAtts and Subs that reference module outputs.
:param name The name of the output
:param output The output dict
:param k The name of the node
:param v The value of the node
:param d The dict that holds the parent of k
:param n The name of the node that holds k
If a reference is found, this function sets the value of d[n]
"""
if k == SUB:
self.resolve_output_sub(v, d, n)
elif k == GETATT:
self.resolve_output_getatt(v, d, n)
else:
if isdict(v):
for k2, v2 in v.copy().items():
self.resolve_outputs(k2, v2, d[n], k)
elif isinstance(v, list):
idx = -1
for v2 in v:
idx = idx + 1
if isdict(v2):
for k3, v3 in v2.copy().items():
self.resolve_outputs(k3, v3, v, idx)

def resolve_output_sub(self, v, d, n):
"Resolve a Sub that refers to a module output"
words = parse_sub(v, True)
sub = ""
for word in words:
if word.t == WordType.STR:
sub += word.w
elif word.t == WordType.AWS:
sub += "${AWS::" + word.w + "}"
elif word.t == WordType.REF:
# A reference to an output has to be a getatt
resolved = "${" + word.w + "}"
sub += resolved
elif word.t == WordType.GETATT:
resolved = "${" + word.w + "}"
tokens = word.w.split(".")
if len(tokens) != 2:
msg = f"GetAtt {word.w} has unexpected number of tokens"
raise exceptions.InvalidModuleError(msg=msg)
# !Sub ${Content.BucketArn} -> !Sub ${ContentBucket.Arn}
if tokens[0] == self.name and tokens[1] in self.outputs:
output = self.outputs[tokens[1]]
if GETATT in output:
getatt = output[GETATT]
resolved = "${" + self.name + ".".join(getatt) + "}"
elif SUB in output:
resolved = "${" + self.name + output[SUB] + "}"
sub += resolved

d[n] = {SUB: sub}

def resolve_output_getatt(self, v, d, n):
"Resolve a GetAtt that refers to a module output"
if not isinstance(v, list) or len(v) < 2:
msg = f"GetAtt {v} invalid"
raise exceptions.InvalidModuleError(msg=msg)
if v[0] == self.name and v[1] in self.outputs:
output = self.outputs[v[1]]
if GETATT in output:
getatt = output[GETATT]
d[n] = {GETATT: [self.name + getatt[0], getatt[1]]}
elif SUB in output:
# Parse the Sub in the module output
words = parse_sub(output[SUB], True)
sub = ""
for word in words:
if word.t == WordType.STR:
sub += word.w
elif word.t == WordType.AWS:
sub += "${AWS::" + word.w + "}"
elif word.t == WordType.REF:
# This is a ref to a param or resource
# TODO: If it's a ref to a param...? is this allowed?

This comment has been minimized.

Copy link
@benbridts

benbridts Dec 13, 2024

I think their are three options here? This is how I think they should work (but I can see arguments for some other ways too)

  • Ref to a pseudo-parameter: should probably be allowed
  • Ref to a Parameter (or Resource for that matter) outside of the template: should not be allowed, except in an override (do we know the difference here? - not sure how that would work)
  • Ref to a Parameter in the module: should be replace with the value of the parameter
# If it's a resource, concatenante the name
resolved = "${" + word.w + "}"
if word.w in self.resources:
resolved = "${" + self.name + word.w + "}"
sub += resolved
elif word.t == WordType.GETATT:
resolved = "${" + word.w + "}"
tokens = word.w.split(".")
if len(tokens) != 2:
msg = f"GetAtt {word.w} unexpected length"
raise exceptions.InvalidModuleError(msg=msg)
if tokens[0] in self.resources:
resolved = "${" + self.name + word.w + "}"
sub += resolved

d[n] = {SUB: sub}

def validate_overrides(self):
"Make sure resources referenced by overrides actually exist"
for logical_id in self.overrides:
Expand Down Expand Up @@ -500,12 +622,15 @@ def resolve_sub(self, v, d, n):
sub += resolved
elif word.t == WordType.GETATT:
need_sub = True
tokens = word.w.split()
if len(tokens) != 2:
msg = "GetAtt {word.w} has unexpected number of tokens"
resolved = "${" + word.w + "}"
tokens = word.w.split(".")
if len(tokens) < 2:
msg = f"GetAtt {word.w} has unexpected number of tokens"
raise exceptions.InvalidModuleError(msg=msg)
if tokens[0] in self.resources:
tokens[0] = self.name + tokens[0]
resolved = "${" + tokens[0] + "." + tokens[1] + "}"
sub += resolved

if need_sub:
d[n] = {SUB: sub}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
Outputs:
ExampleOutput:
Value:
Fn::GetAtt:
- ContentBucket
- Arn
ExampleSub:
Value:
Fn::Sub: ${ContentBucket.Arn}
ExampleGetSub:
Value:
Fn::Sub: ${ContentBucket.Arn}
Resources:
ContentBucket:
Type: AWS::S3::Bucket
Properties:
BucketName: foo
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
Parameters:
Name:
Type: String
Resources:
Bucket:
Type: AWS::S3::Bucket
Properties:
BucketName: !Ref Name
Outputs:
BucketArn: !GetAtt Bucket.Arn
BucketArnSub: !Sub ${Bucket.Arn}

This comment has been minimized.

Copy link
@benbridts

benbridts Dec 13, 2024

It might be worth it to add a !Ref test here as well (and in the intial version I think it is probably fine to only support Outputs that are derived from the resources - in future version I can see uses for Outputting constants (e.g a version number of the module) or something directly derived from the module parameters (e.g constructing an URL based on a domain name)

output-module.yaml

Outputs:
  BucketRef: !Ref Bucket

output-template.yaml

Outputs:
  ExampleGetRef:
    Value: !GetAtt Content.BucketRef

output-expect.yaml

Outputs:
  ExampleGetRef:
    Value:
      Ref: ContentBucket

Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
Modules:
Content:
Source: ./output-module.yaml
Properties:
Name: foo
Outputs:
ExampleOutput:
Value: !GetAtt Content.BucketArn
ExampleSub:
Value: !Sub ${Content.BucketArn}
ExampleGetSub:
Value: !GetAtt Content.BucketArnSub
2 changes: 1 addition & 1 deletion tests/unit/customizations/cloudformation/test_modules.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def test_main(self):
# The tests are in the modules directory.
# Each test has 3 files:
# test-template.yaml, test-module.yaml, and test-expect.yaml
tests = ["basic", "type", "sub", "modinmod"]
tests = ["basic", "type", "sub", "modinmod", "output"]
for test in tests:
base = "unit/customizations/cloudformation/modules"
t = modules.read_source(f"{base}/{test}-template.yaml")
Expand Down

0 comments on commit 160be7c

Please sign in to comment.