diff --git a/pkg/parser/node/finder.go b/pkg/parser/node/finder.go index 7f56debc..073415c1 100644 --- a/pkg/parser/node/finder.go +++ b/pkg/parser/node/finder.go @@ -1,30 +1,33 @@ 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) } } @@ -32,6 +35,9 @@ func (node *Node) DeepFindChild(name string) *Node { return nil } + // Simulate Cirrus Cloud parser behavior + virtualNode.Deduplicate() + return &virtualNode } diff --git a/pkg/parser/node/finder_test.go b/pkg/parser/node/finder_test.go index eb0cc85f..9cadbfff 100644 --- a/pkg/parser/node/finder_test.go +++ b/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) { diff --git a/pkg/parser/node/node.go b/pkg/parser/node/node.go index df5335fe..32fcd020 100644 --- a/pkg/parser/node/node.go +++ b/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) } diff --git a/pkg/parser/parseable/parseable.go b/pkg/parser/parseable/parseable.go index 0f5248b3..2822725c 100644 --- a/pkg/parser/parseable/parseable.go +++ b/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 diff --git a/pkg/parser/testdata/via-rpc/new-deduplicator-different-type.json b/pkg/parser/testdata/via-rpc/new-deduplicator-different-type.json new file mode 100644 index 00000000..382c57b6 --- /dev/null +++ b/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" + } +] diff --git a/pkg/parser/testdata/via-rpc/new-deduplicator-different-type.yml b/pkg/parser/testdata/via-rpc/new-deduplicator-different-type.yml new file mode 100644 index 00000000..38541968 --- /dev/null +++ b/pkg/parser/testdata/via-rpc/new-deduplicator-different-type.yml @@ -0,0 +1,4 @@ +task: + container: + image: debian:latest + container: diff --git a/pkg/parser/testdata/via-rpc/new-deduplicator-same-type.json b/pkg/parser/testdata/via-rpc/new-deduplicator-same-type.json new file mode 100644 index 00000000..98aafe5d --- /dev/null +++ b/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" + } +] diff --git a/pkg/parser/testdata/via-rpc/new-deduplicator-same-type.yml b/pkg/parser/testdata/via-rpc/new-deduplicator-same-type.yml new file mode 100644 index 00000000..8ec88ed4 --- /dev/null +++ b/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