Fix unresolved conditions bug with sub modules

Author: ericzbeard

cft/pkg/conditions.go

@@ -5,7 +5,6 @@ import (
 	"slices"
 
 	"github.com/aws-cloudformation/rain/cft"
-	"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"
@@ -102,21 +101,15 @@ func (module *Module) ProcessConditions() error {
 				condName := conditionNode.Value
 				condResult, ok := module.ConditionValues[condName]
 				if !ok {
-					msg := "%s Condition not found in %s"
-
 					// Is this a condition that we could not
 					// fully resolve, or a condition that doesn't exist?
 
-					config.Debugf(msg, condName, module.Config.Source)
+					if slices.Contains(unResolved, condName) {
+						// Prepend the module name to the condition
+						newName := module.Config.Name + conditionNode.Value
+						conditionNode.Value = newName
 
-					if !slices.Contains(unResolved, condName) {
-						// If it's not in that list, it's likely a mistake
-						return fmt.Errorf(msg, condName, module.Config.Source)
 					}
-
-					// Prepend the module name to the condition
-					newName := module.Config.Name + conditionNode.Value
-					conditionNode.Value = newName
 				} else {
 					if !condResult {
 						itemsToRemove = append(itemsToRemove, itemName)
@@ -144,23 +137,27 @@ func (module *Module) ProcessConditions() error {
 	}
 
 	if len(unResolved) == 0 {
-		config.Debugf("No unresolved conditions in %s", module.Config.Name)
 		node.RemoveFromMap(module.Node, string(cft.Conditions))
 	} else {
 		// Only remove those that we fully resolved.
 		// Emit the rest into the parent template.
 		for _, name := range resolved {
 			node.RemoveFromMap(module.ConditionsNode, name)
 		}
+
+		// Ensure that the parent template has a Conditions section
 		t := module.ParentTemplate
 		tc, err := t.GetSection(cft.Conditions)
 		if err != nil {
 			tc, _ = t.AddMapSection(cft.Conditions)
 		}
+
+		// Record the existing conditions in the parent
 		tcNames := make(map[string]*yaml.Node)
 		for i := 0; i < len(tc.Content); i += 2 {
 			tcNames[tc.Content[i].Value] = tc.Content[i+1]
 		}
+
 		for i, condNode := range module.ConditionsNode.Content {
 			if i%2 == 0 {
 				// Prepend the module name to the condition
@@ -172,12 +169,12 @@ func (module *Module) ProcessConditions() error {
 							"%s that conflicts with a Condition in the parent"
 						return fmt.Errorf(msg, module.Config.Source, name)
 					}
+				} else {
+					nameNode := node.MakeScalar(name)
+					node.Append(tc, nameNode)
+					cloned := node.Clone(val)
+					node.Append(tc, cloned)
 				}
-				nameNode := node.MakeScalar(name)
-				node.Append(tc, nameNode)
-				cloned := node.Clone(module.ConditionsNode.Content[i+1])
-				node.Append(tc, cloned)
-				break
 			}
 		}
 
@@ -190,9 +187,6 @@ func (module *Module) ProcessConditions() error {
 func (module *Module) EvalCond(
 	name string, val *yaml.Node) (EvalResult, error) {
 
-	config.Debugf("module.EvalCond %s %s:\n%s",
-		module.Config.Name, name, node.YamlStr(val))
-
 	// Handle mapping node (most condition functions)
 	if val.Kind == yaml.MappingNode && len(val.Content) >= 2 {
 		key := val.Content[0].Value
@@ -343,10 +337,6 @@ func (module *Module) EvalEquals(n *yaml.Node) (EvalResult, error) {
 		}
 	}
 
-	// This looks like an unresolved reference
-	config.Debugf("EvalEquals unresolved val1: %s, val2: %s",
-		node.YamlStr(val1), node.YamlStr(val2))
-
 	return UnResolved, nil
 }
 

cft/pkg/module.go

@@ -242,12 +242,20 @@ func processModule(
 		return err
 	}
 
-	err = processAddedSections(moduleAsTemplate, moduleAsTemplate.Node.Content[0],
+	// processAddedSections is where we recurse on sub-modules
+	err = processAddedSections(moduleAsTemplate,
+		moduleAsTemplate.Node.Content[0],
 		parsedModule.RootDir, parsedModule.FS, m)
 	if err != nil {
 		return err
 	}
 
+	// Process conditions again to emit sub-module conditions into the parent
+	err = m.ProcessConditions()
+	if err != nil {
+		return err
+	}
+
 	err = m.ValidateOverrides()
 	if err != nil {
 		return err

cft/pkg/tmpl/awscli-modules/cond-unres-expect.yaml

@@ -1,8 +1,19 @@
+Parameters:
+  P:
+    Type: String
 Conditions:
   FooIsRegionUsEast1:
     Fn::Equals:
     - Ref: AWS::Region
     - us-east-1
+  FooIsPTrue:
+    Fn::Equals:
+      - !Ref P
+      - true
+  FooSubIsParam:
+    Fn::Equals:
+      - !Ref P
+      - true
 
 Resources:
   FooS3Bucket:
@@ -16,3 +27,9 @@ Resources:
           - FooIsRegionUsEast1
           - a-east
           - a-not-east
+  FooSubB:
+    Type: A::B::C
+    Condition: FooSubIsParam
+  FooC:
+    Type: D::E::F
+    Condition: FooIsPTrue

cft/pkg/tmpl/awscli-modules/cond-unres-module.yaml

@@ -1,8 +1,22 @@
+Parameters:
+  P:
+    Type: String
+
 Conditions:
   IsRegionUsEast1:
     Fn::Equals:
     - Ref: AWS::Region
     - us-east-1
+  IsPTrue:
+    Fn::Equals:
+      - !Ref P
+      - true
+
+Modules:
+  Sub:
+    Source: cond-unres-submodule.yaml
+    Properties:
+      P: !Ref P
 
 Resources:
   S3Bucket:
@@ -16,3 +30,6 @@ Resources:
           - IsRegionUsEast1
           - a-east
           - a-not-east
+  C:
+    Type: D::E::F
+    Condition: IsPTrue

cft/pkg/tmpl/awscli-modules/cond-unres-submodule.yaml

@@ -0,0 +1,14 @@
+Parameters:
+  P:
+    Type: String
+
+Conditions:
+  IsParam:
+    Fn::Equals:
+      - !Ref P
+      - true
+
+Resources:
+  B:
+    Condition: IsParam
+    Type: A::B::C

cft/pkg/tmpl/awscli-modules/cond-unres-template.yaml

@@ -1,3 +1,9 @@
+Parameters:
+  P:
+    Type: String
+
 Modules:
   Foo:
-    Source: cond-unres-module.yaml
+    Source: cond-unres-module.yaml
+    Properties:
+      P: !Ref P

cft/pkg/tmpl/awscli-modules/conditional-module.yaml

@@ -5,15 +5,17 @@ Parameters:
 
   LambdaRoleName:
     Type: String
-    Description: If set, allow the lambda function to access this table
     Default: ""
 
+  HasLambda:
+    Type: Boolean
+    Default: false
+
 Conditions:
   IfLambdaRoleIsSet:
-    Fn::Not: 
-      - Fn::Equals:
-        - !Ref LambdaRoleName
-        - ""
+    Fn::Equals:
+      - !Ref HasLambda
+      - true 
 
   Nested:
     Fn::And:

cft/pkg/tmpl/awscli-modules/conditional-template.yaml

@@ -4,6 +4,7 @@ Modules:
     Properties:
       TableName: table-a
       LambdaRoleName: my-role
+      HasLambda: true
   B:
     Source: ./conditional-module.yaml
     Properties: