Repository: aws-cloudformation/rain
Pull request #28 (2020-07-09 17:07:05)

Rework parser
*Issue #, if available:*

*Description of changes:*


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.



Patch:
diff --git a/cfn/parse/parse.go b/cfn/parse/parse.go
index 461f6a2a..89db90a5 100644
--- a/cfn/parse/parse.go
+++ b/cfn/parse/parse.go
@@ -3,84 +3,17 @@
 package parse
 
 import (
-	"encoding/json"
 	"fmt"
 	"io"
 	"io/ioutil"
-	"reflect"
-	"strings"
 
 	"github.com/aws-cloudformation/rain/cfn"
 	"github.com/aws-cloudformation/rain/cfn/diff"
 	"github.com/aws-cloudformation/rain/cfn/format"
 
-	yaml "github.com/sanathkr/go-yaml"
-	yamlwrapper "github.com/sanathkr/yaml"
+	"gopkg.in/yaml.v3"
 )
 
-var tags = []string{
-	"And",
-	"Base64",
-	"Cidr",
-	"Equals",
-	"FindInMap",
-	"GetAZs",
-	"GetAtt",
-	"If",
-	"ImportValue",
-	"Join",
-	"Not",
-	"Or",
-	"Ref",
-	"Select",
-	"Split",
-	"Sub",
-	"Transform",
-}
-
-type tagUnmarshalerType struct {
-}
-
-var tagUnmarshaler = &tagUnmarshalerType{}
-
-func init() {
-	for _, tag := range tags {
-		yaml.RegisterTagUnmarshaler("!"+tag, tagUnmarshaler)
-	}
-}
-
-func (t *tagUnmarshalerType) UnmarshalYAMLTag(tag string, value reflect.Value) reflect.Value {
-	prefix := "Fn::"
-	if tag == "Ref" || tag == "Condition" {
-		prefix = ""
-	}
-	tag = prefix + tag
-
-	output := reflect.ValueOf(make(map[interface{}]interface{}))
-	key := reflect.ValueOf(tag)
-	output.SetMapIndex(key, value)
-
-	return output
-}
-
-func transform(in map[string]interface{}) map[string]interface{} {
-	for k, v := range in {
-		if k == "Fn::GetAtt" {
-			if s, ok := v.(string); ok {
-				value := make([]interface{}, 2)
-				for i, part := range strings.SplitN(s, ".", 2) {
-					value[i] = part
-				}
-				in[k] = value
-			}
-		} else if m, ok := v.(map[string]interface{}); ok {
-			in[k] = transform(m)
-		}
-	}
-
-	return in
-}
-
 // Reader returns a cfn.Template parsed from an io.Reader
 func Reader(r io.Reader) (cfn.Template, error) {
 	data, err := ioutil.ReadAll(r)
@@ -103,23 +36,21 @@ func File(fileName string) (cfn.Template, error) {
 
 // String returns a cfn.Template parsed from a string
 func String(input string) (cfn.Template, error) {
-	parsed, err := yamlwrapper.YAMLToJSON([]byte(input))
+	var node yaml.Node
+	err := yaml.Unmarshal([]byte(input), &node)
 	if err != nil {
 		return cfn.Template{}, fmt.Errorf("Invalid YAML: %s", err)
 	}
 
+	transform(&node)
+
 	var output map[string]interface{}
-	err = json.Unmarshal(parsed, &output)
+	err = node.Decode(&output)
 	if err != nil {
-		return cfn.Template{}, fmt.Errorf("Invalid YAML: %s", err)
+		return cfn.Template{}, fmt.Errorf("Invalid template: %s", err)
 	}
 
-	return Map(output)
-}
-
-// Map returns a cfn.Template parsed from a map[string]interface{} value
-func Map(input map[string]interface{}) (cfn.Template, error) {
-	return cfn.Template(transform(input)), nil
+	return cfn.Template(output), nil
 }
 
 // Verify confirms that there is no semantic difference between
diff --git a/cfn/parse/transform.go b/cfn/parse/transform.go
new file mode 100644
index 00000000..3baba3f0
--- /dev/null
+++ b/cfn/parse/transform.go
@@ -0,0 +1,96 @@
+package parse
+
+import (
+	"strings"
+
+	"gopkg.in/yaml.v3"
+)
+
+var tags = []string{
+	"And",
+	"Base64",
+	"Cidr",
+	"Equals",
+	"FindInMap",
+	"GetAZs",
+	"GetAtt",
+	"If",
+	"ImportValue",
+	"Join",
+	"Not",
+	"Or",
+	"Ref",
+	"Select",
+	"Split",
+	"Sub",
+	"Transform",
+}
+
+func transform(node *yaml.Node) {
+	// Fix badly-parsed numbers
+	if node.ShortTag() == "!!float" && node.Value[0] == '0' {
+		node.Tag = "!!str"
+	}
+
+	// See if we're dealing with a Fn:: tag
+	for _, tag := range tags {
+		if node.ShortTag() == "!"+tag {
+			key := tag
+			if tag != "Ref" && tag != "Condition" {
+				key = "Fn::" + key
+			}
+
+			body := yaml.Node{
+				Kind:        node.Kind,
+				Style:       0,
+				Value:       node.Value,
+				Content:     node.Content,
+				HeadComment: node.HeadComment,
+				LineComment: node.LineComment,
+				FootComment: node.FootComment,
+				Line:        node.Line,
+				Column:      node.Column,
+			}
+
+			body.Tag = body.ShortTag()
+
+			if tag == "GetAtt" && body.Tag == "!!str" {
+				body.Kind = yaml.SequenceNode
+
+				parts := strings.SplitN(node.Value, ".", 2)
+
+				body.Content = []*yaml.Node{
+					&yaml.Node{
+						Kind:  yaml.ScalarNode,
+						Style: 0,
+						Tag:   "!!str",
+						Value: parts[0],
+					},
+					&yaml.Node{
+						Kind:  yaml.ScalarNode,
+						Style: 0,
+						Tag:   "!!str",
+						Value: parts[1],
+					},
+				}
+			}
+
+			node.Kind = yaml.MappingNode
+			node.Style = 0
+			node.Tag = "!!map"
+			node.Content = []*yaml.Node{
+				&yaml.Node{
+					Kind:  yaml.ScalarNode,
+					Style: 0,
+					Tag:   "!!str",
+					Value: key,
+				},
+				&body,
+			}
+		}
+	}
+
+	for _, child := range node.Content {
+		transform(child)
+	}
+}
diff --git a/cfn/spec/generate/main.go b/cfn/spec/generate/main.go
index 2736d091..881db46c 100644
--- a/cfn/spec/generate/main.go
+++ b/cfn/spec/generate/main.go
@@ -12,7 +12,8 @@ import (
 	"strings"
 
 	"github.com/aws-cloudformation/rain/cfn/spec/models"
-	yamlwrapper "github.com/sanathkr/yaml"
+
+	"gopkg.in/yaml.v3"
 )
 
 const (
@@ -23,19 +24,21 @@ const (
 )
 
 func load(r io.Reader, s *models.Spec) {
-	inYAML, err := ioutil.ReadAll(r)
+	var intermediate map[string]interface{}
+	yamlDecoder := yaml.NewDecoder(r)
+	err := yamlDecoder.Decode(&intermediate)
 	if err != nil {
 		panic(err)
 	}
 
-	inJSON, err := yamlwrapper.YAMLToJSON(inYAML)
+	inJSON, err := json.Marshal(intermediate)
 	if err != nil {
 		panic(err)
 	}
 
-	yamlReader := bytes.NewReader(inJSON)
+	jsonReader := bytes.NewReader(inJSON)
 
-	decoder := json.NewDecoder(yamlReader)
+	decoder := json.NewDecoder(jsonReader)
 	decoder.DisallowUnknownFields()
 	err = decoder.Decode(&s)
 	if err != nil {
diff --git a/examples/account-number.template b/examples/account-number.template
new file mode 100644
index 00000000..5d1a12dd
--- /dev/null
+++ b/examples/account-number.template
@@ -0,0 +1,12 @@
+AWSTemplateFormatVersion: '2010-09-09'
+
+Parameters:
+  Account:
+    Default: 012345678901
+    Type: String
+
+Resources:
+  Bucket:
+    Type: AWS::S3::Bucket
+    Properties:
+      BucketName: !Sub "bucket-${Account}"
diff --git a/go.mod b/go.mod
index e349ef49..6eb681fa 100644
--- a/go.mod
+++ b/go.mod
@@ -8,14 +8,13 @@ require (
 	github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1 // indirect
 	github.com/google/go-cmp v0.3.0
 	github.com/kr/pretty v0.1.0 // indirect
-	github.com/sanathkr/go-yaml v0.0.0-20170819195128-ed9d249f429b
-	github.com/sanathkr/yaml v1.0.1-0.20170819201035-0056894fa522
 	github.com/spf13/cobra v0.0.4
 	github.com/spf13/pflag v1.0.3
 	github.com/spf13/viper v1.3.2
 	github.com/stretchr/testify v1.3.0 // indirect
 	golang.org/x/sys v0.0.0-20190602015325-4c4f7f33c9ed // indirect
 	gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 // indirect
+	gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776
 )
 
 go 1.13
diff --git a/go.sum b/go.sum
index 22fe5418..edc7fd46 100644
--- a/go.sum
+++ b/go.sum
@@ -83,3 +83,5 @@ gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33
 gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw=
 gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
+gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776 h1:tQIYjPdBoyREyB9XMu+nnTclpTYkz2zFM+lzLJFO4gQ=
+gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=


Test patch:
diff --git a/cfn/cfn_test.go b/cfn/cfn_test.go
index 30c6deb0..96bae909 100644
--- a/cfn/cfn_test.go
+++ b/cfn/cfn_test.go
@@ -5,10 +5,9 @@ import (
 	"testing"
 
 	"github.com/aws-cloudformation/rain/cfn"
-	"github.com/aws-cloudformation/rain/cfn/parse"
 )
 
-var testCase, _ = parse.Map(map[string]interface{}{
+var testCase = cfn.Template(map[string]interface{}{
 	"Parameters": map[string]interface{}{
 		"Name": map[string]interface{}{
 			"Type": "String",
diff --git a/cfn/parse/parse_test.go b/cfn/parse/parse_test.go
index b6a69068..a486ea0b 100644
--- a/cfn/parse/parse_test.go
+++ b/cfn/parse/parse_test.go
@@ -6,6 +6,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/aws-cloudformation/rain/cfn"
 	"github.com/aws-cloudformation/rain/cfn/parse"
 	"github.com/google/go-cmp/cmp"
 )
@@ -14,11 +15,19 @@ var testFile = "test.yaml"
 
 var testTemplate string
 
-var expected, _ = parse.Map(map[string]interface{}{
+var expected = cfn.Template(map[string]interface{}{
 	"Parameters": map[string]interface{}{
-		"Number": map[string]interface{}{
+		"Int": map[string]interface{}{
 			"Type":    "Number",
-			"Default": float64(500000000),
+			"Default": int(500000000),
+		},
+		"Float": map[string]interface{}{
+			"Type":    "Number",
+			"Default": float64(12345.6789),
+		},
+		"AccountID": map[string]interface{}{
+			"Type":    "String",
+			"Default": "0123456789",
 		},
 	},
 	"Resources": map[string]interface{}{
@@ -111,7 +120,7 @@ func TestReadString(t *testing.T) {
 }
 
 func TestVerifyOutput(t *testing.T) {
-	source, _ := parse.Map(map[string]interface{}{
+	source := cfn.Template(map[string]interface{}{
 		"foo": map[string]interface{}{
 			"bar": map[string]interface{}{
 				"Fn::GetAtt": []interface{}{
@@ -120,7 +129,10 @@ func TestVerifyOutput(t *testing.T) {
 				},
 			},
 			"baz": map[string]interface{}{
-				"Fn::GetAtt": "foo.bar",
+				"Fn::GetAtt": []interface{}{
+					"foo",
+					"bar",
+				},
 			},
 			"quux": []interface{}{
 				"mooz",
diff --git a/cfn/parse/test.yaml b/cfn/parse/test.yaml
index 01a41af5..cf9e215e 100644
--- a/cfn/parse/test.yaml
+++ b/cfn/parse/test.yaml
@@ -1,8 +1,16 @@
 Parameters:
-  Number:
+  Int:
     Type: Number
     Default: 500000000
 
+  Float:
+    Type: Number
+    Default: 12345.6789
+
+  AccountID:
+    Type: String
+    Default: 0123456789
+
 Resources:
   Bucket1:
     Type: AWS::S3::Bucket