Merge pull request #298 from ericzbeard/fix-nmodules

Fix a bug in modules that caused some nodes to be skipped

cft/pkg/module.go

@@ -133,8 +133,8 @@ type refctx struct {
 
 func replaceProp(prop *yaml.Node, parentName string, v *yaml.Node, outNode *yaml.Node, sidx int) error {
 
-	config.Debugf("replaceProp prop: %s, parentName: %s, v: %s, sidx: %d",
-		node.ToSJson(prop), parentName, node.ToSJson(v), sidx)
+	// config.Debugf("replaceProp prop: %s, parentName: %s, v: %s, sidx: %d",
+	//  	node.ToSJson(prop), parentName, node.ToSJson(v), sidx)
 
 	if sidx > -1 {
 		// The node is a sequence element
@@ -146,7 +146,7 @@ func replaceProp(prop *yaml.Node, parentName string, v *yaml.Node, outNode *yaml
 
 		if v.Kind == yaml.MappingNode {
 			parentNode := node.GetParent(prop, outNode, nil)
-			config.Debugf("replaceProp parentNode: %s", parentNode.String())
+			//config.Debugf("replaceProp parentNode: %s", parentNode.String())
 			*parentNode.Value = *newVal
 		} else {
 			*prop = *newVal
@@ -227,7 +227,7 @@ func resolveModuleRef(parentName string, prop *yaml.Node, sidx int, ctx *refctx)
 					prop.Value)
 			}
 
-			config.Debugf("resolveModuleRef about to call replaceProp")
+			//config.Debugf("resolveModuleRef about to call replaceProp")
 
 			replaceProp(prop, parentName, parentVal, outNode, sidx)
 
@@ -380,8 +380,8 @@ func resolveModuleSub(parentName string, prop *yaml.Node, sidx int, ctx *refctx)
 // If sidx is > -1, this prop is in a sequence
 func renamePropRefs(parentName string, propName string, prop *yaml.Node, sidx int, ctx *refctx) error {
 
-	config.Debugf("renamePropRefs parentName: %s, propName: %s, prop: %s, sidx: %d",
-		parentName, propName, node.ToSJson(prop), sidx)
+	//config.Debugf("renamePropRefs parentName: %s, propName: %s, prop: %s, sidx: %d",
+	//	parentName, propName, node.ToSJson(prop), sidx)
 
 	logicalId := ctx.logicalId
 
@@ -419,7 +419,7 @@ func renamePropRefs(parentName string, propName string, prop *yaml.Node, sidx in
 			for i, p := range prop.Content {
 				// propName is blank so the next parentName is blank
 
-				config.Debugf("About to call renamePropRefs, propName is %s", propName)
+				//config.Debugf("About to call renamePropRefs, propName is %s", propName)
 
 				//result := renamePropRefs(parentName, p.Value, prop.Content[i], i, ctx)
 				result := renamePropRefs(propName, p.Value, prop.Content[i], i, ctx)

cft/pkg/module_test.go

@@ -26,6 +26,10 @@ func TestSub(t *testing.T) {
 	runTest("sub", t)
 }
 
+func TestMany(t *testing.T) {
+	runTest("many", t)
+}
+
 // TODO: This was broken in the refactor, come back to it later
 //func TestForeach(t *testing.T) {
 //	runTest("foreach", t)

cft/pkg/pkg.go

@@ -26,6 +26,7 @@ package pkg
 
 import (
 	"embed"
+	"errors"
 	"fmt"
 	"path/filepath"
 
@@ -54,12 +55,11 @@ func transform(ctx *transformContext) (bool, error) {
 	// registry is a map of functions defined in rain.go
 	for path, fn := range registry {
 		for found := range s11n.MatchAll(ctx.nodeToTransform, path) {
-			//config.Debugf("pkg transform path: %v, nodeToTransform:\n%v", path,
-			//	node.ToSJson(ctx.nodeToTransform))
-			//config.Debugf("pkg transform found: %v", node.ToSJson(found))
 			nodeParent := node.GetParent(found, ctx.nodeToTransform, nil)
 			nodeParent.Parent = ctx.parent
-			//config.Debugf("pkg transform nodeParent: %s", nodeParent.String())
+			if path == "**/*|Rain::Module" {
+				config.Debugf("found a Module at line %v", nodeParent.Value.Line)
+			}
 			c, err := fn(&directiveContext{found, ctx.rootDir, ctx.t, nodeParent, ctx.fs})
 			if err != nil {
 				config.Debugf("Error packaging template: %s\n", err)
@@ -89,14 +89,34 @@ func Template(t cft.Template, rootDir string, fs *embed.FS) (cft.Template, error
 		parent:          &node.NodePair{Key: t.Node, Value: t.Node},
 		fs:              fs,
 	}
-	changed, err := transform(ctx)
-	if err != nil {
-		return t, err
+	var changed bool = false
+	passes := 0
+	maxPasses := 100
+	for {
+		passes += 1
+		// Modules can add new nodes to the template, which
+		// breaks s11n.MatchAll, since it expects the length to stay the same.
+		// Just start over a transform the whole template again.
+		changedThisPass, err := transform(ctx)
+		if err != nil {
+			return t, err
+		}
+		if changedThisPass {
+			config.Debugf("Need another pass: %d", passes)
+			changed = true
+		}
+		if !changedThisPass {
+			config.Debugf("No changes this pass: %d", passes)
+			break
+		}
+		if passes > maxPasses {
+			return t, errors.New("reached maxPasses while transforming")
+		}
 	}
 
 	// j, _ = json.MarshalIndent(templateNode, "", "  ")
 	// config.Debugf("Transformed template: %v", string(j))
-
+	var err error
 	if changed {
 		t, err = parse.Node(templateNode)
 		if err != nil {

cft/pkg/tmpl/many-expect.yaml

@@ -0,0 +1,20 @@
+Resources:
+  My1Bucket1:
+    Type: AWS::S3::Bucket
+    Properties:
+      BucketName: foo1
+
+  My2Bucket1:
+    Type: AWS::S3::Bucket
+    Properties:
+      BucketName: foo2
+
+  My3Bucket1:
+    Type: AWS::S3::Bucket
+    Properties:
+      BucketName: foo3
+
+  My4Bucket1:
+    Type: AWS::S3::Bucket
+    Properties:
+      BucketName: foo4

cft/pkg/tmpl/many-module.yaml

@@ -0,0 +1,9 @@
+Parameters:
+  Name:
+    Type: String
+Resources:
+  Bucket1:
+    Type: AWS::S3::Bucket
+    Properties:
+      BucketName: !Ref Name
+

cft/pkg/tmpl/many-template.yaml

@@ -0,0 +1,18 @@
+Resources:
+  My1:
+    Type: !Rain::Module "./many-module.yaml"
+    Properties:
+      Name: foo1
+  My2:
+    Type: !Rain::Module "./many-module.yaml"
+    Properties:
+      Name: foo2
+  My3:
+    Type: !Rain::Module "./many-module.yaml"
+    Properties:
+      Name: foo3
+  My4:
+    Type: !Rain::Module "./many-module.yaml"
+    Properties:
+      Name: foo4
+

cft/pkg/util.go

@@ -184,13 +184,13 @@ func expectFile(n *yaml.Node, root string) ([]byte, string, error) {
 		return nil, "", err
 	}
 
-	config.Debugf("root: %v, path: %v", root, path)
+	//config.Debugf("root: %v, path: %v", root, path)
 
 	if !filepath.IsAbs(path) {
 		path = filepath.Join(root, path)
 	}
 
-	config.Debugf("path: %v", path)
+	//config.Debugf("path: %v", path)
 
 	info, err := os.Stat(path)
 	if err != nil {

internal/config/version.go

@@ -5,5 +5,5 @@ const (
 	NAME = "Rain"
 
 	// VERSION is the application's version string
-	VERSION = "v1.8.0"
+	VERSION = "v1.8.1"
 )