From 794c666635c5d5137225c6b8fd1f7e4a6414e2da Mon Sep 17 00:00:00 2001 From: Easton Crupper <65553218+ecrupper@users.noreply.github.com> Date: Mon, 30 Dec 2024 13:29:03 -0600 Subject: [PATCH] enhance(yaml): silent fix anchor merges in YAML maps (#1231) --- compiler/native/compile_test.go | 14 ++-- .../native/testdata/steps_merge_anchor_1.yml | 46 ++++++++++ internal/testdata/buildkite_new_version.yml | 18 ++++ internal/yaml.go | 84 ++++++++++++++++++- internal/yaml_test.go | 4 + 5 files changed, 159 insertions(+), 7 deletions(-) create mode 100644 compiler/native/testdata/steps_merge_anchor_1.yml create mode 100644 internal/testdata/buildkite_new_version.yml diff --git a/compiler/native/compile_test.go b/compiler/native/compile_test.go index 3767760c2..09a1ff5dd 100644 --- a/compiler/native/compile_test.go +++ b/compiler/native/compile_test.go @@ -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) } } diff --git a/compiler/native/testdata/steps_merge_anchor_1.yml b/compiler/native/testdata/steps_merge_anchor_1.yml new file mode 100644 index 000000000..3de1e64b4 --- /dev/null +++ b/compiler/native/testdata/steps_merge_anchor_1.yml @@ -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 \ No newline at end of file diff --git a/internal/testdata/buildkite_new_version.yml b/internal/testdata/buildkite_new_version.yml new file mode 100644 index 000000000..50e3ba513 --- /dev/null +++ b/internal/testdata/buildkite_new_version.yml @@ -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 \ No newline at end of file diff --git a/internal/yaml.go b/internal/yaml.go index 8267486e8..363e9c9c3 100644 --- a/internal/yaml.go +++ b/internal/yaml.go @@ -4,6 +4,7 @@ package internal import ( "fmt" + "strings" bkYaml "github.com/buildkite/yaml" yaml "gopkg.in/yaml.v3" @@ -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) + } + } +} diff --git a/internal/yaml_test.go b/internal/yaml_test.go index 627139229..0a12c6e44 100644 --- a/internal/yaml_test.go +++ b/internal/yaml_test.go @@ -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,