Repository: aws-cloudformation/rain
Pull request #125 (2023-04-19 23:54:59)

Module refs
Another bug fix for Refs in modules.


Patch:
diff --git a/cft/pkg/module.go b/cft/pkg/module.go
index 91382ffc..0743f558 100644
--- a/cft/pkg/module.go
+++ b/cft/pkg/module.go
@@ -99,34 +99,39 @@ func renamePropRefs(
 	//           C: !Ref D
 
 	if prop.Kind == yaml.ScalarNode {
-		config.Debugf("Scalar %v %v", propName, node.ToJson(prop))
 		if propName == "Ref" {
-
 			// Find the module parameter that matches the !Ref
 			_, param := s11n.GetMapValue(moduleParams, prop.Value)
 			if param != nil {
-				config.Debugf("Found param for %v", prop.Value)
 				// We need to get the parameter value from the parent template.
 				// Module params are set by the parent template resource properties.
+
+				// Look for this property name in the parent template
 				_, parentVal := s11n.GetMapValue(templateProps, prop.Value)
 				if parentVal == nil {
 					return fmt.Errorf("did not find %v in parent template Resource Properties", prop.Value)
 				}
+
+				config.Debugf("parentVal: %v", node.ToJson(parentVal))
+
 				// We can't just set prop.Value, since we would end up with Prop: !Ref Value instead of just Prop: Value
 				// Get the property's parent and set the entire map value for the property
-				config.Debugf("Calling GetParent for %v\nroot:\n%v", node.ToJson(prop), node.ToJson(ext))
+
+				// Get the map parent within the extension node we created
 				refMap := node.GetParent(prop, ext, nil)
 				if refMap.Value == nil {
 					return fmt.Errorf("could not find parent for %v", prop)
 				}
-				config.Debugf("refMap.Value: %v", node.ToJson(refMap.Value))
 				propParentPair := node.GetParent(refMap.Value, ext, nil)
+
 				config.Debugf("propParentPair.Value: %v", node.ToJson(propParentPair.Value))
-				newValue := &yaml.Node{Kind: yaml.ScalarNode, Value: parentVal.Value}
-				config.Debugf("About to set %v to newValue: %v", prop.Value, node.ToJson(newValue))
-				// TODO - Bug - look up the map value to replace, not [0]
+
+				// Create a new node to replace what's defined in the module
+				newValue := node.Clone(parentVal)
+
+				config.Debugf("Setting %v to:\n%v", parentName, node.ToJson(newValue))
+
 				node.SetMapValue(propParentPair.Value, parentName, newValue)
-				config.Debugf("propParentPair.Value after: %v", node.ToJson(propParentPair.Value))
 			} else {
 				config.Debugf("Did not find param for %v", prop.Value)
 				// Look for a resource in the module
@@ -202,10 +207,6 @@ func processModule(module *yaml.Node,
 	typeNode *yaml.Node, parent node.NodePair) (bool, error) {
 
 	// The parent arg is the map in the template resource's Content[1] that contains Type, Properties, etc
-	// p, _ := json.MarshalIndent(parent, "", "  ")
-	// config.Debugf("parent: %v", string(p))
-
-	// config.Debugf("module: %v", node.ToJson(module))
 
 	if parent.Key == nil {
 		return false, errors.New("expected parent.Key to not be nil. The !Rain::Module directive should come after Type: ")
@@ -213,7 +214,6 @@ func processModule(module *yaml.Node,
 
 	// Get the logical id of the resource we are transforming
 	logicalId := parent.Key.Value
-	//config.Debugf("logicalId: %v", logicalId)
 
 	// Make a new node that will hold our additions to the original template
 	outputNode.Content = make([]*yaml.Node, 0)
@@ -345,6 +345,32 @@ func processModule(module *yaml.Node,
 		if resource.Kind == yaml.MappingNode {
 			name := moduleResources.Content[i-1].Value
 			if name != "ModuleExtension" {
+
+				// Resolve Conditions. Rain handles this differently, since a rain
+				// module cannot have a Condition section. This value must be a module parameter
+				// name, and the value must be set in the parent template as the name of
+				// a Condition that is defined in the parent.
+				_, condition := s11n.GetMapValue(resource, "Condition")
+				if condition != nil {
+					conditionErr := errors.New("a Condition in a rain module must be the name of a Parameter that is set the name of a Condition in the parent template")
+					// The value must be present in the module's parameters
+					if condition.Kind != yaml.ScalarNode {
+						return false, conditionErr
+					}
+					_, param := s11n.GetMapValue(moduleParams, condition.Value)
+					if param == nil {
+						return false, conditionErr
+					}
+					_, conditionVal := s11n.GetMapValue(templateProps, condition.Value)
+					if conditionVal == nil {
+						return false, conditionErr
+					}
+					if conditionVal.Kind != yaml.ScalarNode {
+						return false, conditionErr
+					}
+					condition.Value = conditionVal.Value
+				}
+
 				// This is an additional resource to be added
 				nameNode := node.Clone(moduleResources.Content[i-1])
 				nameNode.Value = rename(logicalId, nameNode.Value)
diff --git a/docs/README.tmpl b/docs/README.tmpl
index 0ffe190c..5fe5f2e0 100644
--- a/docs/README.tmpl
+++ b/docs/README.tmpl
@@ -33,6 +33,8 @@ cfn-lint, Guard and more:
 
 * **Predict deployment failures** (EXPERIMENTAL): `rain forecast` analyzes a template and the target deployment account to predict things that might go wrong when you attempt to create, update, or delete a stack. This command speeds up development by giving you advanced notice for issues like missing permissions, resources that already exist, and a variety of other common resource-specific deployment blockers.
 
+* **Modules** (EXPERIMENTAL): `rain pkg` supports client-side module development with the `!Rain::Module` directive. Rain modules are partial templates that are inserted into the parent template, with some extra functionality added to enable extending existing resource types.
+
 _Note that in order to use experimental commands, you have to add `--experimental` or `-x` as an argument._
 
 ## Getting started


Test patch:
diff --git a/test/modules/bucket.yaml b/test/modules/bucket.yaml
index ef6971b9..52db3ec1 100644
--- a/test/modules/bucket.yaml
+++ b/test/modules/bucket.yaml
@@ -7,9 +7,10 @@ Description: |
 Parameters:
   LogBucketName:
     Type: String
-    Description: The name of the log bucket
   RetentionPolicy:
     Type: String
+  ConditionName:
+    Type: String
 
 Resources:
   ModuleExtension:
@@ -82,5 +83,8 @@ Resources:
     Type: AWS::S3::Bucket
     DependsOn: ModuleExtension
 
+  ConditionalResource:
+    Condition: ConditionName
+    Type: AWS::S3::Bucket
 
 
diff --git a/test/modules/expect.yaml b/test/modules/expect.yaml
index 248b28fe..0ad14021 100644
--- a/test/modules/expect.yaml
+++ b/test/modules/expect.yaml
@@ -1,9 +1,22 @@
+Parameters:
+  BucketRetentionPolicy:
+    Type: String
+    AllowedValues:
+      - Delete
+      - Retain
+
+Conditions:
+  ConditionA:
+    !Equals:
+      - true
+      - true
+
 Resources:
   SecondResourceInOriginal:
     Type: AWS::SQS::Queue
 
   ModuleExample:
-    DeletionPolicy: Retain
+    DeletionPolicy: !Ref BucketRetentionPolicy
     UpdateReplacePolicy: Delete
     Type: AWS::S3::Bucket
     DependsOn:
@@ -36,7 +49,7 @@ Resources:
 
   ModuleExampleLogBucket:
     DeletionPolicy: Delete
-    UpdateReplacePolicy: Retain
+    UpdateReplacePolicy: !Ref BucketRetentionPolicy
     Type: AWS::S3::Bucket
     DependsOn:
       - ModuleExampleAdditionalResource1
@@ -74,6 +87,10 @@ Resources:
     DependsOn:
       - ModuleExample
 
+  ModuleExampleConditionalResource:
+    Type: AWS::S3::Bucket
+    Condition: ConditionA
+
   WithoutExtensionBucketA:
     Type: AWS::S3::Bucket
     Properties:
diff --git a/test/templates/ext-ref-module.yaml b/test/templates/ext-ref-module.yaml
index 5a184454..9bbd91ae 100644
--- a/test/templates/ext-ref-module.yaml
+++ b/test/templates/ext-ref-module.yaml
@@ -1,9 +1,13 @@
+Parameters:
+  ThePolicy:
+    Type: String
+
 Resources:
 
   ModuleExample:
     Type: !Rain::Module "file://../../modules/ext-ref.yaml"
     Properties:
       BucketName: ezbeard-cep-test-module-bucket
-      RetentionPolicy: Retain
+      RetentionPolicy: !Ref ThePolicy
 
 
diff --git a/test/templates/module.yaml b/test/templates/module.yaml
index 33571c66..944a3b6b 100644
--- a/test/templates/module.yaml
+++ b/test/templates/module.yaml
@@ -1,3 +1,18 @@
+Parameters:
+
+  BucketRetentionPolicy:
+    Type: String
+    AllowedValues:
+      - Delete
+      - Retain
+
+Conditions:
+
+  ConditionA:
+    !Equals:
+      - true
+      - true
+
 Resources:
 
   ModuleExample:
@@ -8,7 +23,8 @@ Resources:
     Properties:
       LogBucketName: ezbeard-cep-test-module-log-bucket
       BucketName: ezbeard-cep-test-module-bucket
-      RetentionPolicy: Retain
+      RetentionPolicy: !Ref BucketRetentionPolicy
+      ConditionName: ConditionA
       Tags:
         - Key: test-tag
           Value: test-value2