Merge pull request #676 from ericzbeard/modules

Add features to Modules

Author: ericzbeard

README.md

@@ -457,13 +457,14 @@ Modules are not allowed to refer to themselves directly or in cycles. Module A
 can’t import Module A, and it can’t import Module B if that module imports
 Module A.
 
-Modules support a basic form of looping/foreach by adding a `Map` attribute to
-the module configuration. Special variables `$MapIndex` and `$MapValue` can be
-used to refer to the index and list value. Logical Ids are auto-incremented by
+Modules support a basic form of looping/foreach by either using the familiar
+`Fn::ForEach` syntax, or with a shorthand by adding a `ForEach` attribute to
+the module configuration. Special variables `$Identifier` and `$Index` can be
+used to refer to the value and list index. With the shorthand, or if you don't
+put the Identifier in the logical id, logical ids are auto-incremented by
 adding an integer starting at zero. Since this is a client-side-only feature,
 list values must be fully resolved scalars, not values that must be resolved at
-deploy time. When deploy-time values are needed, `Fn::ForEach` is a better
-design option. Local modules do not process `Fn::ForEach`.
+deploy time. 
 
 ```
 Parameters:
@@ -474,7 +475,7 @@ Parameters:
 Modules:
   Content:
     Source: ./map-module.yaml
-    Map: !Ref List
+    ForEach: !Ref List
     Properties:
       Name: !Sub my-bucket-$MapValue
 ```
@@ -506,15 +507,12 @@ It’s also possible to refer to elements within a `Map` using something like
 which resolves to a list of all of the `Arn` outputs from that module.
 
 When a module is processed, the first thing that happens is parsing of the
-`Conditions` within the module. These conditions must be fully resolvable
-client-side, since the package command does not have access to Parameters or
-deploy-time values. These conditions are converted to a dictionary of boolean
-values and the `Conditions` section is not emitted into the parent template. It
-is not merged into the parent. Any resources marked with a false condition are
-removed, and any property nodes with conditions are processed. Any values of
-`!Ref AWS::NoValue` are removed. No evidence of conditions will remain in the
-markup that is merged into the parent template, unless the condition is not
-found in the module. 
+Conditions within the module. Any Resources, Modules, or Outputs marked with a
+false condition are removed, and any property nodes with conditions are
+processed. Any values of !Ref AWS::NoValue are removed. Any unresolved
+conditions (for example, a condition that references a paramter in the parent
+template, or something like AWS::Region) are emitted into the parent template,
+prefixed with the module name.
 
 Much of the value of module output is in the smart handling of `Ref`,
 `Fn::GetAtt`, and `Fn::Sub`. For the most part, we want these to “just work” in
@@ -612,11 +610,6 @@ Modules:
     Source: $def/a/b/bar.yaml
 ```
 
-
-
-
-
-
 ### Publish modules to CodeArtifact 
 
 Rain integrates with AWS CodeArtifact to enable an experience similar to npm

cft/cft.go

@@ -257,6 +257,10 @@ func (t *Template) AddMappedModule(copiedConfig *ModuleConfig) {
 		t.ModuleMaps = make(map[string]*ModuleConfig)
 	}
 	t.ModuleMaps[copiedConfig.Name] = copiedConfig
+	keyName := copiedConfig.OriginalName + copiedConfig.MapKey
+	// Also add the name if referenced by key
+	t.ModuleMaps[keyName] = copiedConfig
+
 	if t.ModuleMapNames == nil {
 		t.ModuleMapNames = make(map[string][]string)
 	}
@@ -282,3 +286,9 @@ func (t *Template) AddResolvedModuleNode(n *yaml.Node) {
 func (t *Template) ModuleAlreadyResolved(n *yaml.Node) bool {
 	return slices.Contains(t.ModuleResolved, n)
 }
+
+// IsRef returns true if the node is a 2-length Mapping node
+// that starts with "Ref"
+func IsRef(n *yaml.Node) bool {
+	return len(n.Content) == 2 && n.Content[0].Value == string(Ref)
+}

cft/module_config.go

@@ -2,7 +2,10 @@ package cft
 
 import (
 	"errors"
+	"fmt"
+	"strings"
 
+	"github.com/aws-cloudformation/rain/internal/config"
 	"github.com/aws-cloudformation/rain/internal/node"
 	"github.com/aws-cloudformation/rain/internal/s11n"
 	"gopkg.in/yaml.v3"
@@ -43,6 +46,9 @@ type ModuleConfig struct {
 
 	// The root directory of the template that configures this module
 	ParentRootDir string
+
+	// If this module is wrapped in Fn::ForEach, this will be populated
+	FnForEach *FnForEach
 }
 
 func (c *ModuleConfig) Properties() map[string]any {
@@ -53,7 +59,8 @@ func (c *ModuleConfig) Overrides() map[string]any {
 	return node.DecodeMap(c.OverridesNode)
 }
 
-// ResourceOverridesNode returns the Overrides node for the given resource if it exists
+// ResourceOverridesNode returns the Overrides node for the
+// given resource if it exists
 func (c *ModuleConfig) ResourceOverridesNode(name string) *yaml.Node {
 	if c.OverridesNode == nil {
 		return nil
@@ -66,19 +73,59 @@ const (
 	Source     string = "Source"
 	Properties string = "Properties"
 	Overrides  string = "Overrides"
-	Map        string = "Map"
+	ForEach    string = "Fn::ForEach"
 )
 
 // parseModuleConfig parses a single module configuration
 // from the Modules section in the template
-func (t *Template) ParseModuleConfig(name string, n *yaml.Node) (*ModuleConfig, error) {
-	if n.Kind != yaml.MappingNode {
-		return nil, errors.New("not a mapping node")
-	}
+func (t *Template) ParseModuleConfig(
+	name string, n *yaml.Node) (*ModuleConfig, error) {
+
 	m := &ModuleConfig{}
 	m.Name = name
 	m.Node = n
 
+	// Handle Fn::ForEach modules
+	if strings.HasPrefix(name, ForEach) && n.Kind == yaml.SequenceNode {
+		if len(n.Content) != 3 {
+			msg := "expected %s len 3, got %d"
+			return nil, fmt.Errorf(msg, name, len(n.Content))
+		}
+
+		m.FnForEach = &FnForEach{}
+
+		loopName := strings.Replace(name, ForEach, "", 1)
+		loopName = strings.Replace(loopName, ":", "", -1)
+		m.FnForEach.LoopName = loopName
+
+		m.Name = loopName //  TODO: ?
+
+		m.FnForEach.Identifier = n.Content[0].Value
+		m.FnForEach.Collection = n.Content[1]
+		outputKeyValue := n.Content[2]
+
+		if outputKeyValue.Kind != yaml.MappingNode ||
+			len(outputKeyValue.Content) != 2 ||
+			outputKeyValue.Content[1].Kind != yaml.MappingNode {
+			msg := "invalid %s, expected OutputKey: OutputValue mapping"
+			return nil, fmt.Errorf(msg, name)
+		}
+
+		m.FnForEach.OutputKey = outputKeyValue.Content[0].Value
+		m.Node = outputKeyValue.Content[1]
+		m.FnForEach.OutputValue = m.Node
+		n = m.Node
+		m.Map = m.FnForEach.Collection
+
+		config.Debugf("ModuleConfig.FnForEach: %+v", m.FnForEach)
+
+	}
+
+	if n.Kind != yaml.MappingNode {
+		config.Debugf("ParseModuleConfig %s: %s", name, node.ToSJson(n))
+		return nil, errors.New("not a mapping node")
+	}
+
 	content := n.Content
 	for i := 0; i < len(content); i += 2 {
 		attr := content[i].Value
@@ -90,30 +137,41 @@ func (t *Template) ParseModuleConfig(name string, n *yaml.Node) (*ModuleConfig,
 			m.PropertiesNode = val
 		case Overrides:
 			m.OverridesNode = val
-		case Map:
+		case "ForEach":
 			m.Map = val
 		}
 	}
 
-	//err := t.ValidateModuleConfig(m)
-	//if err != nil {
-	//	return nil, err
-	//}
-
 	return m, nil
 }
 
-// ValidateModuleConfig makes sure the configuration does not
-// break any rules, such as not having a Property with the
-// same name as a Parameter.
-//func (t *Template) ValidateModuleConfig(moduleConfig *ModuleConfig) error {
-//	props := moduleConfig.Properties()
-//	for key := range props {
-//		_, err := t.GetParameter(key)
-//		if err == nil {
-//			return fmt.Errorf("module %s in %s has Property %s with the same name as a template Parameter",
-//				moduleConfig.Name, moduleConfig.ParentRootDir, key)
-//		}
-//	}
-//	return nil
-//}
+type FnForEach struct {
+	LoopName    string
+	Identifier  string
+	Collection  *yaml.Node
+	OutputKey   string
+	OutputValue *yaml.Node
+}
+
+// OutputKeyHasIdentifier returns true if the key uses the identifier
+func (ff *FnForEach) OutputKeyHasIdentifier() bool {
+	dollar := "${" + ff.Identifier + "}"
+	amper := "&{" + ff.Identifier + "}"
+	if strings.Contains(ff.OutputKey, dollar) {
+		return true
+	}
+	if strings.Contains(ff.OutputKey, amper) {
+		return true
+	}
+	return false
+}
+
+// ReplaceIdentifier replaces instance of the identifier in s for collection
+// key k
+func ReplaceIdentifier(s, k, identifier string) string {
+	dollar := "${" + identifier + "}"
+	amper := "&{" + identifier + "}"
+	s = strings.Replace(s, dollar, k, -1)
+	s = strings.Replace(s, amper, k, -1)
+	return s
+}