Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

enhance(yaml): silent fix anchor merges in YAML maps #1231

Merged
merged 2 commits into from
Dec 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading