Skip to content

Commit

Permalink
enhance(yaml): silent fix anchor merges in YAML maps (#1231)
Browse files Browse the repository at this point in the history
  • Loading branch information
ecrupper authored Dec 30, 2024
1 parent 70ca430 commit 794c666
Show file tree
Hide file tree
Showing 5 changed files with 159 additions and 7 deletions.
14 changes: 8 additions & 6 deletions compiler/native/compile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2162,19 +2162,21 @@ func TestNative_Compile_LegacyMergeAnchor(t *testing.T) {
t.Errorf("Compile() mismatch (-want +got):\n%s", diff)
}

// run test on current version (should fail)
yaml, err = os.ReadFile("../types/yaml/buildkite/testdata/merge_anchor.yml") // has `version: "1"` instead of `version: "legacy"`
// run test on current version
yaml, err = os.ReadFile("testdata/steps_merge_anchor_1.yml") // has `version: "1"` instead of `version: "legacy"`
if err != nil {
t.Errorf("Reading yaml file return err: %v", err)
}

got, _, err = compiler.Compile(context.Background(), yaml)
if err == nil {
t.Errorf("Compile should have returned err")
if err != nil {
t.Errorf("Compile returned err: %v", err)
}

if got != nil {
t.Errorf("Compile is %v, want %v", got, nil)
want.Version = "1"

if diff := cmp.Diff(want, got); diff != "" {
t.Errorf("Compile() mismatch (-want +got):\n%s", diff)
}
}

Expand Down
46 changes: 46 additions & 0 deletions compiler/native/testdata/steps_merge_anchor_1.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# test file that uses the non-standard multiple anchor keys in one step to test custom step unmarshaler

version: "1"

aliases:
images:
alpine: &alpine-image
image: alpine:latest
postgres: &pg-image
image: postgres

events:
push: &event-push
ruleset:
event:
- push
env:
dev-env: &dev-environment
environment:
REGION: dev

services:
- name: service-a
<<: *pg-image
<<: *dev-environment
ports:
- "5432:5432"

steps:
- name: alpha
<<: *alpine-image
<<: *event-push
commands:
- echo alpha

- name: beta
<<: [ *alpine-image, *event-push ]
commands:
- echo beta

- name: gamma
<<: *alpine-image
<<: *event-push
<<: *dev-environment
commands:
- echo gamma
18 changes: 18 additions & 0 deletions internal/testdata/buildkite_new_version.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
version: "1"

aliases:
images:
alpine: &alpine-image
image: alpine:latest

env:
dev-env: &dev-environment
environment:
REGION: dev

steps:
- name: example
<<: *alpine-image
<<: *dev-environment
commands:
- echo $REGION
84 changes: 83 additions & 1 deletion internal/yaml.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package internal

import (
"fmt"
"strings"

bkYaml "github.com/buildkite/yaml"
yaml "gopkg.in/yaml.v3"
Expand Down Expand Up @@ -55,9 +56,90 @@ func ParseYAML(data []byte) (*types.Build, error) {
// unmarshal the bytes into the yaml configuration
err := yaml.Unmarshal(data, config)
if err != nil {
return nil, fmt.Errorf("unable to unmarshal yaml: %w", err)
// if error is related to duplicate `<<` keys, attempt to fix
if strings.Contains(err.Error(), "mapping key \"<<\" already defined") {
root := new(yaml.Node)

if err := yaml.Unmarshal(data, root); err != nil {
fmt.Println("error unmarshalling YAML:", err)

return nil, err
}

collapseMergeAnchors(root.Content[0])

newData, err := yaml.Marshal(root)
if err != nil {
return nil, err
}

err = yaml.Unmarshal(newData, config)
if err != nil {
return nil, fmt.Errorf("unable to unmarshal yaml: %w", err)
}
} else {
return nil, fmt.Errorf("unable to unmarshal yaml: %w", err)
}
}
}

return config, nil
}

// collapseMergeAnchors traverses the entire pipeline and replaces duplicate `<<` keys with a single key->sequence.
func collapseMergeAnchors(node *yaml.Node) {
// only replace on maps
if node.Kind == yaml.MappingNode {
var (
anchors []*yaml.Node
keysToRemove []int
firstIndex int
firstFound bool
)

// traverse mapping node content
for i := 0; i < len(node.Content); i += 2 {
keyNode := node.Content[i]

// anchor found
if keyNode.Value == "<<" {
if (i+1) < len(node.Content) && node.Content[i+1].Kind == yaml.AliasNode {
anchors = append(anchors, node.Content[i+1])
}

if !firstFound {
firstIndex = i
firstFound = true
} else {
keysToRemove = append(keysToRemove, i)
}
}
}

// only replace if there were duplicates
if len(anchors) > 1 && firstFound {
seqNode := &yaml.Node{
Kind: yaml.SequenceNode,
Content: anchors,
}

node.Content[firstIndex] = &yaml.Node{Kind: yaml.ScalarNode, Value: "<<"}
node.Content[firstIndex+1] = seqNode

for i := len(keysToRemove) - 1; i >= 0; i-- {
index := keysToRemove[i]

node.Content = append(node.Content[:index], node.Content[index+2:]...)
}
}

// go to next level
for _, content := range node.Content {
collapseMergeAnchors(content)
}
} else if node.Kind == yaml.SequenceNode {
for _, item := range node.Content {
collapseMergeAnchors(item)
}
}
}
4 changes: 4 additions & 0 deletions internal/yaml_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ func TestInternal_ParseYAML(t *testing.T) {
file: "testdata/buildkite.yml",
want: wantBuild,
},
{
file: "testdata/buildkite_new_version.yml",
want: wantBuild,
},
{
file: "testdata/no_version.yml",
want: wantBuild,
Expand Down

0 comments on commit 794c666

Please sign in to comment.