Replies: 1 comment
-
Below are the other differences that we've documented. Major shoutout to @plyr4 for researching and compiling this. Collapsing Multiple AnchorsConsider the YAML !!merge spec and the following example env-foo: &env-foo
environment:
foo: "1"
env-bar: &env-bar
environment:
bar: "2"
steps:
- name: test
image: alpine
<<: *env-foo
<<: *env-bar
commands:
- echo $FOO
- echo $BAR BuildkiteIn buildkite the result steps:
- name: test
image: alpine
+ environment:
+ bar: "2"
commands:
- echo $foo
- echo $bar go-yaml v3In go-yaml the result steps:
- name: test
image: alpine
+ environment:
+ foo: "1"
- bar: "2"
commands:
- echo $foo
- echo $bar $ echo $foo
1
$ echo $bar
Merging Keys via TemplatesConsider the YAML !!merge spec and the following example steps:
- name: merge-keys-true
template:
name: merge-keys
vars:
+ foo: true
- name: merge-keys-false
template:
name: merge-keys
vars:
+ foo: false with the template metadata:
template: true
steps:
- name: test
image: alpine:latest
environment:
bar: "1"
+ {{if .foo}}
+ bar: "2"
+ {{end}}
commands:
- echo hello BuildkiteIn buildkite the result
steps:
- commands:
- echo hello
image: alpine:latest
name: merge-keys-true_test
pull: not_present
environment:
+ bar: "2"
- commands:
- echo hello
image: alpine:latest
name: merge-keys-false_test
pull: not_present
environment:
+ bar: "1" go-yaml v3In go-yaml the result is a merge error for trying to merge keys:
Overriding Nested Keys via AnchorsConsider the YAML !!merge spec and the following example echo-world: &echo-world
commands: echo world
steps:
- name: test
image: alpine
commands:
- echo hello
+ <<: *echo-world
BuildkiteIn buildkite the result steps:
- name: test
image: alpine
commands:
- - echo hello
+ - echo world go-yaml v3In go-yaml the result steps:
- name: test
image: alpine
commands:
+ - echo hello |
Beta Was this translation helpful? Give feedback.
-
What is changing?
In
v0.26.0
, Vela should use the officialgopkg.in/yaml.v3
instead ofgithub.com/buildkite/yaml
Why the change?
Vela uses github.com/buildkite/yaml as the YAML parsing library for pipeline and template files.
The buildkite project has beenarchived and has lingering security vulnerabilities.
Why did we choose that library in the first place?
The official
go-yaml
library did not support anchors withstages
. They had an open issue for it for years that was never addressed until the introduction ofv3
.Several forks of the project were made, and we chose buildkite.
Which library are we going to use going forward?
The
go-yaml
library has av3
that we are planning on jumping over to in the next release.What is the change that users should expect?
According to the official YAML spec, anchors can not be declared as keys multiple times in a map. For example:
These should instead be declared as
The current buildkite library merges these keys for us, whereas the new go-yaml v3 is more strict and will throw an error.
This is a frequently used config pattern. Impact could be very high.
There are other things the Vela team has noticed that will change behavior in more subtle ways. I will follow up this initial post with more examples of these changes.
Path Forward
Below are some considerations for the Vela team as they execute this library change.
Community Outreach
We will pre-release the CLI to allow for teams to run
vela validate
on their pipelines to confirm things are compiling as expected.Partial Cutover
Vela will have the ability to compile every pipeline file (templates included) using the buildkite library for one release cycle. This will be determined by
version: "legacy"
at the top level of the pipeline config.This quick band-aid will give users the ability to debug this change on their own timeline.
Using
gopkg.in/yaml.v3
custom node handlingIf far more pipelines break than expected, potentially investigate handling YAML map key merging ourselves, which is technically possible in v3, as they give a lot more freedom to custom unmarshal.
There is a draft PR which serves as a proof of concept that Vela can support its own sort of map merging. However, its performance is untested and it is a deliberate step away from the official spec. Further, the solution will only address the anchor merging at the step level and not any of the other disparities. At some point, it's a game of whack-a-mole.
Call to Action
At the end of the month (November 30th, 2024), this discussion post will be converted to a proposal, which will be merged into this repo for permanent record.
Until then, if you have anything you'd like to bring up related to this change prior to that proposal, please comment below.
Beta Was this translation helpful? Give feedback.
All reactions