Parser deduplicator (#209)

Needed for Cirrus Cloud parser compatibility.

pkg/parser/node/finder.go

@@ -1,37 +1,43 @@
 package node
 
-func (node *Node) DeepFindChild(name string) *Node {
+func (node *Node) DeepFindCollectible(name string) *Node {
 	var fulfilledAtLeastOnce bool
 	var virtualNode Node
 
+	// Accumulate collectible nodes in descending order of priority
+	// (e.g. "env" field from the root YAML map comes first, then "env" field from the task's map, etc.)
+	var traverseChain []*Node
+
 	for current := node; current != nil; current = current.Parent {
 		for i := len(current.Children) - 1; i >= 0; i-- {
 			child := current.Children[i]
 
-			if child.Name != name {
-				continue
-			}
-
-			if !fulfilledAtLeastOnce {
-				virtualNode = *child
-				fulfilledAtLeastOnce = true
+			if child.Name == name {
+				traverseChain = append(traverseChain, child)
 			}
+		}
+	}
 
-			for i := len(child.Children) - 1; i >= 0; i-- {
-				subChild := child.Children[i]
+	// Starting from the lowest priority node,
+	// merge the rest of the nodes into it
+	for i := len(traverseChain) - 1; i >= 0; i-- {
+		child := traverseChain[i]
 
-				// Append fields from child that we don't have
-				if !virtualNode.HasChild(subChild.Name) {
-					virtualNode.Children = append(virtualNode.Children, subChild)
-				}
-			}
+		if !fulfilledAtLeastOnce {
+			virtualNode = *child
+			fulfilledAtLeastOnce = true
+		} else {
+			virtualNode.MergeFrom(child)
 		}
 	}
 
 	if !fulfilledAtLeastOnce {
 		return nil
 	}
 
+	// Simulate Cirrus Cloud parser behavior
+	virtualNode.Deduplicate()
+
 	return &virtualNode
 }
 

pkg/parser/node/finder_test.go

@@ -7,7 +7,7 @@ import (
 	"testing"
 )
 
-func TestDeepFindChildren(t *testing.T) {
+func TestDeepFindCollectible(t *testing.T) {
 	yamlSlice := yaml.MapSlice{
 		// First tree's children
 		{Key: "env", Value: yaml.MapSlice{
@@ -26,9 +26,9 @@ func TestDeepFindChildren(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	virtualNode := tree.Children[1].DeepFindChild("env")
-	assert.Equal(t, "KEY2", virtualNode.Children[0].Name)
-	assert.Equal(t, "KEY1", virtualNode.Children[1].Name)
+	virtualNode := tree.Children[1].DeepFindCollectible("env")
+	assert.Equal(t, "KEY1", virtualNode.Children[0].Name)
+	assert.Equal(t, "KEY2", virtualNode.Children[1].Name)
 }
 
 func TestDeepFindChildrenSameLevel(t *testing.T) {
@@ -48,10 +48,10 @@ func TestDeepFindChildrenSameLevel(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	virtualNode := tree.Children[0].DeepFindChild("env")
+	virtualNode := tree.Children[0].DeepFindCollectible("env")
 	assert.Len(t, virtualNode.Children, 2)
-	assert.Equal(t, "KEY1", virtualNode.Children[1].Name)
-	assert.Equal(t, "KEY2", virtualNode.Children[0].Name)
+	assert.Equal(t, "KEY1", virtualNode.Children[0].Name)
+	assert.Equal(t, "KEY2", virtualNode.Children[1].Name)
 }
 
 func TestHasChildren(t *testing.T) {

pkg/parser/node/node.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 	"gopkg.in/yaml.v2"
+	"reflect"
 )
 
 type Node struct {
@@ -21,6 +22,72 @@ type ScalarValue struct {
 
 var ErrNodeConversionFailed = errors.New("node conversion failed")
 
+func (node *Node) Deduplicate() {
+	// Split children into two groups
+	seen := map[string]*Node{}
+	var unique, duplicate []*Node
+
+	for _, child := range node.Children {
+		if _, ok := seen[child.Name]; ok {
+			duplicate = append(duplicate, child)
+		} else {
+			unique = append(unique, child)
+			seen[child.Name] = child
+		}
+	}
+
+	// Merge children from the duplicate group into their unique counterparts
+	// with recursive descent
+	for _, duplicateChild := range duplicate {
+		duplicateChild.Deduplicate()
+
+		uniqueChild := seen[duplicateChild.Name]
+		uniqueChild.MergeFrom(duplicateChild)
+	}
+
+	node.Children = unique
+}
+
+func (node *Node) MergeFrom(other *Node) {
+	node.Name = other.Name
+
+	if reflect.TypeOf(node.Value) != reflect.TypeOf(other.Value) {
+		node.Value = other.Value
+		node.Children = other.Children
+		return
+	}
+
+	switch other.Value.(type) {
+	case *MapValue:
+		existingChildren := map[string]*Node{}
+
+		for _, child := range node.Children {
+			existingChildren[child.Name] = child
+		}
+
+		for _, otherChild := range other.Children {
+			existingChild, ok := existingChildren[otherChild.Name]
+			if ok {
+				existingChild.MergeFrom(otherChild)
+			} else {
+				node.Children = append(node.Children, otherChild)
+			}
+		}
+	case *ListValue:
+		for i, otherChild := range other.Children {
+			if i < len(node.Children) {
+				// We have a counterpart child, do a merge
+				node.Children[i].MergeFrom(otherChild)
+			} else {
+				// They have more children that we do, simply append them one by one
+				node.Children = append(node.Children, otherChild)
+			}
+		}
+	case *ScalarValue:
+		node.Value = other.Value
+	}
+}
+
 func NewFromSlice(slice yaml.MapSlice) (*Node, error) {
 	return convert(nil, "root", slice)
 }

pkg/parser/parseable/parseable.go

@@ -111,7 +111,7 @@ func (parser *DefaultParser) CollectibleFields() []CollectibleField {
 }
 
 func evaluateCollectible(node *node.Node, field CollectibleField) error {
-	children := node.DeepFindChild(field.Name)
+	children := node.DeepFindCollectible(field.Name)
 
 	if children == nil {
 		return nil

pkg/parser/testdata/via-rpc/new-deduplicator-different-type.json

@@ -0,0 +1,28 @@
+[
+  {
+    "commands": [
+      {
+        "cloneInstruction": {},
+        "name": "clone"
+      }
+    ],
+    "environment": {
+      "CIRRUS_OS": "linux"
+    },
+    "instance": {
+      "@type": "type.googleapis.com/org.cirruslabs.ci.services.cirruscigrpc.ContainerInstance",
+      "cpu": 2,
+      "memory": 4096
+    },
+    "metadata": {
+      "properties": {
+        "allow_failures": "false",
+        "experimental": "false",
+        "indexWithinBuild": "0",
+        "timeout_in": "3600",
+        "trigger_type": "AUTOMATIC"
+      }
+    },
+    "name": "main"
+  }
+]

pkg/parser/testdata/via-rpc/new-deduplicator-different-type.yml

@@ -0,0 +1,4 @@
+task:
+  container:
+    image: debian:latest
+  container:

pkg/parser/testdata/via-rpc/new-deduplicator-same-type.json

@@ -0,0 +1,46 @@
+[
+  {
+    "commands": [
+      {
+        "cloneInstruction": {},
+        "name": "clone"
+      },
+      {
+        "name": "main",
+        "scriptInstruction": {
+          "scripts": [
+            "true"
+          ]
+        }
+      }
+    ],
+    "environment": {
+      "CIRRUS_OS": "linux"
+    },
+    "instance": {
+      "@type": "type.googleapis.com/org.cirruslabs.ci.services.cirruscigrpc.ContainerInstance",
+      "additionalContainers": [
+        {
+          "containerPort": 80,
+          "cpu": 0.5,
+          "image": "nginx:latest",
+          "memory": 20480,
+          "name": "nginx"
+        }
+      ],
+      "cpu": 2,
+      "image": "debian:latest",
+      "memory": 4096
+    },
+    "metadata": {
+      "properties": {
+        "allow_failures": "false",
+        "experimental": "false",
+        "indexWithinBuild": "0",
+        "timeout_in": "3600",
+        "trigger_type": "AUTOMATIC"
+      }
+    },
+    "name": "main"
+  }
+]

pkg/parser/testdata/via-rpc/new-deduplicator-same-type.yml

@@ -0,0 +1,13 @@
+task:
+  container:
+    image: debian:latest
+  container:
+    image: debian:latest
+    additional_containers:
+      - name: nginx
+        memory: 10Gb
+        image: nginx:latest
+        port: 80
+    additional_containers:
+      - memory: 20Gb
+  script: true