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.

Sign up for GitHub

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

Fix signature mismatches when steps have plugins #2319

Merged
merged 4 commits into from
Aug 24, 2023
Merged

Fix signature mismatches when steps have plugins #2319

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
30e41e8
Teach CommandStep to unmarshal JSON
DrJosh9000 Aug 23, 2023
b6d7ed5
Name -> Source; marshal with FullSource
DrJosh9000 Aug 23, 2023
2834ae0
Fix %w in Fatal log
DrJosh9000 Aug 24, 2023
948bba6
ordered.Unmarshal: support unpointered yaml.Node
DrJosh9000 Aug 24, 2023
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
8 changes: 4 additions & 4 deletions agent/integration/job_verification_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ var (
Step: pipeline.CommandStep{
Command: "echo hello world",
Plugins: pipeline.Plugins{{
Name: "some-plugin#v1.0.0",
Source: "some#v1.0.0",
Config: map[string]string{
"key": "value",
},
Expand All @@ -41,7 +41,7 @@ var (
},
Env: map[string]string{
"BUILDKITE_COMMAND": "echo hello world",
"BUILDKITE_PLUGINS": `[{"some-plugin#v1.0.0":{"key":"value"}}]`,
"BUILDKITE_PLUGINS": `[{"github.com/buildkite-plugins/some-buildkite-plugin#v1.0.0":{"key":"value"}}]`,
"DEPLOY": "0",
},
}
Expand All @@ -64,15 +64,15 @@ var (
Step: pipeline.CommandStep{
Command: "echo hello world",
Plugins: pipeline.Plugins{{
Name: "some-plugin#v1.0.0",
Source: "some#v1.0.0",
Config: map[string]string{
"key": "value",
},
}},
},
Env: map[string]string{
"BUILDKITE_COMMAND": "echo hello world",
"BUILDKITE_PLUGINS": `[{"crimes-plugin#v1.0.0":{"steal":"everything"}}]`,
"BUILDKITE_PLUGINS": `[{"github.com/buildkite-plugins/crimes-buildkite-plugin#v1.0.0":{"steal":"everything"}}]`,
"DEPLOY": "0",
},
}
Expand Down
4 changes: 2 additions & 2 deletions clicommand/agent_start.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -813,12 +813,12 @@ var AgentStartCommand = cli.Command{
if cfg.JobVerificationJWKSPath != "" {
jwksBytes, err := os.ReadFile(cfg.JobVerificationJWKSPath)
if err != nil {
l.Fatal("Failed to read job verification key: %w", err)
l.Fatal("Failed to read job verification key: %v", err)
}

jwks, err = jwk.Parse(jwksBytes)
if err != nil {
l.Fatal("Failed to parse job verification key set: %w", err)
l.Fatal("Failed to parse job verification key set: %v", err)
}

if jwks.Len() == 0 {
Expand Down
15 changes: 12 additions & 3 deletions internal/ordered/unmarshal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ type Unmarshaler interface {
// of types under the hood, but some combinations don't work. Good luck!
//
// - If dst is nil, then src must be nil.
// - If src is *yaml.Node, then DecodeYAML is called to translate the node
// into another type.
// - If src is yaml.Node or *yaml.Node, then DecodeYAML is called to translate
// the node into another type.
// - If dst is a pointer and src is nil, then the value dst points to is set
// to zero.
// - If dst is a pointer to a pointer, Unmarshal recursively calls Unmarshal
Expand Down Expand Up @@ -69,7 +69,16 @@ func Unmarshal(src, dst any) error {
return ErrIntoNil
}

if n, ok := src.(*yaml.Node); ok {
// Apply DecodeYAML to yaml.Node or *yaml.Node first.
switch n := src.(type) {
case yaml.Node:
o, err := DecodeYAML(&n)
if err != nil {
return err
}
src = o

case *yaml.Node:
o, err := DecodeYAML(n)
if err != nil {
return err
Expand Down
145 changes: 78 additions & 67 deletions internal/ordered/unmarshal_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,71 @@ import (
"gopkg.in/yaml.v3"
)

var (
bigYAMLNode = yaml.Node{
Kind: yaml.MappingNode,
Tag: "!!map",
Content: []*yaml.Node{
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "key"},
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "value"},
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "molehill"},
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "large"},
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "switch"},
{Kind: yaml.ScalarNode, Tag: "!!bool", Value: "true"},
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "count"},
{Kind: yaml.ScalarNode, Tag: "!!int", Value: "42"},
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "fader"},
{Kind: yaml.ScalarNode, Tag: "!!float", Value: "2.71828"},
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "slicey"},
{Kind: yaml.SequenceNode, Tag: "!!seq", Content: []*yaml.Node{
{Kind: yaml.ScalarNode, Tag: "!!int", Value: "5"},
{Kind: yaml.ScalarNode, Tag: "!!int", Value: "6"},
{Kind: yaml.ScalarNode, Tag: "!!int", Value: "7"},
{Kind: yaml.ScalarNode, Tag: "!!int", Value: "8"},
}},
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "next"},
{Kind: yaml.MappingNode, Tag: "!!map", Content: []*yaml.Node{
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "key"},
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "another value"},
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "molehill"},
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "extra large"},
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "switch"},
{Kind: yaml.ScalarNode, Tag: "!!bool", Value: "true"},
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "count"},
{Kind: yaml.ScalarNode, Tag: "!!int", Value: "42000"},
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "fader"},
{Kind: yaml.ScalarNode, Tag: "!!float", Value: "1.618"},
}},
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "inner"},
{Kind: yaml.MappingNode, Tag: "!!map", Content: []*yaml.Node{
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "llama"},
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "Kuzco"},
}},
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "notAField"},
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "super important"},
},
}
bigOrdinaryStruct = ordinaryStruct{
Key: "value",
Mountain: "large",
Switch: true,
Count: 42,
Fader: 2.71828,
Slicey: []int{5, 6, 7, 8},
Inner: struct{ Llama string }{"Kuzco"},
Next: &ordinaryStruct{
Key: "another value",
Mountain: "extra large",
Switch: true,
Count: 42000,
Fader: 1.618,
},
Remaining: map[string]any{
"notAField": "super important",
},
}
)

type ordinaryStruct struct {
Key string
Mountain string `yaml:"molehill"`
Expand Down Expand Up @@ -353,7 +418,7 @@ func TestUnmarshal(t *testing.T) {
))),
},
{
desc: "*MapSA to *testStruct without inline",
desc: "*MapSA to *ordinaryStruct without inline",
src: MapFromItems(
TupleSA{Key: "key", Value: "value"},
TupleSA{Key: "molehill", Value: "large"},
Expand All @@ -373,7 +438,7 @@ func TestUnmarshal(t *testing.T) {
},
},
{
desc: "*MapSA to *testStruct with inline",
desc: "*MapSA to *ordinaryStruct with inline",
src: MapFromItems(
TupleSA{Key: "key", Value: "value"},
TupleSA{Key: "molehill", Value: "large"},
Expand Down Expand Up @@ -401,7 +466,7 @@ func TestUnmarshal(t *testing.T) {
},
},
{
desc: "*MapSA to *testStruct with existing",
desc: "*MapSA to *ordinaryStruct with existing",
src: MapFromItems(
TupleSA{Key: "key", Value: "value"},
TupleSA{Key: "molehill", Value: "large"},
Expand Down Expand Up @@ -570,70 +635,16 @@ func TestUnmarshal(t *testing.T) {
},
},
{
desc: "*yaml.Node into testStruct",
src: &yaml.Node{
Kind: yaml.MappingNode,
Tag: "!!map",
Content: []*yaml.Node{
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "key"},
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "value"},
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "molehill"},
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "large"},
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "switch"},
{Kind: yaml.ScalarNode, Tag: "!!bool", Value: "true"},
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "count"},
{Kind: yaml.ScalarNode, Tag: "!!int", Value: "42"},
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "fader"},
{Kind: yaml.ScalarNode, Tag: "!!float", Value: "2.71828"},
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "slicey"},
{Kind: yaml.SequenceNode, Tag: "!!seq", Content: []*yaml.Node{
{Kind: yaml.ScalarNode, Tag: "!!int", Value: "5"},
{Kind: yaml.ScalarNode, Tag: "!!int", Value: "6"},
{Kind: yaml.ScalarNode, Tag: "!!int", Value: "7"},
{Kind: yaml.ScalarNode, Tag: "!!int", Value: "8"},
}},
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "next"},
{Kind: yaml.MappingNode, Tag: "!!map", Content: []*yaml.Node{
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "key"},
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "another value"},
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "molehill"},
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "extra large"},
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "switch"},
{Kind: yaml.ScalarNode, Tag: "!!bool", Value: "true"},
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "count"},
{Kind: yaml.ScalarNode, Tag: "!!int", Value: "42000"},
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "fader"},
{Kind: yaml.ScalarNode, Tag: "!!float", Value: "1.618"},
}},
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "inner"},
{Kind: yaml.MappingNode, Tag: "!!map", Content: []*yaml.Node{
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "llama"},
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "Kuzco"},
}},
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "notAField"},
{Kind: yaml.ScalarNode, Tag: "!!str", Value: "super important"},
},
},
dst: &ordinaryStruct{},
want: &ordinaryStruct{
Key: "value",
Mountain: "large",
Switch: true,
Count: 42,
Fader: 2.71828,
Slicey: []int{5, 6, 7, 8},
Inner: struct{ Llama string }{"Kuzco"},
Next: &ordinaryStruct{
Key: "another value",
Mountain: "extra large",
Switch: true,
Count: 42000,
Fader: 1.618,
},
Remaining: map[string]any{
"notAField": "super important",
},
},
desc: "yaml.Node into ordinaryStruct",
src: bigYAMLNode,
dst: &ordinaryStruct{},
want: &bigOrdinaryStruct,
},
{
desc: "*yaml.Node into ordinaryStruct",
src: &bigYAMLNode,
dst: &ordinaryStruct{},
want: &bigOrdinaryStruct,
},
}

Expand Down
26 changes: 13 additions & 13 deletions internal/pipeline/parser_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -929,7 +929,7 @@ steps:
Command: "echo foo",
Plugins: Plugins{
{
Name: "ecr#v2.7.0",
Source: "ecr#v2.7.0",
Config: ordered.MapFromItems(
ordered.TupleSA{Key: "login", Value: true},
ordered.TupleSA{Key: "account_ids", Value: "0123456789"},
Expand Down Expand Up @@ -1017,21 +1017,21 @@ steps:
Command: "script/buildkite/xxx.sh",
Plugins: Plugins{
{
Name: "xxx/aws-assume-role#v0.1.0",
Source: "xxx/aws-assume-role#v0.1.0",
Config: ordered.MapFromItems(
ordered.TupleSA{Key: "role", Value: "arn:aws:iam::xxx:role/xxx"},
),
},
{
Name: "ecr#v1.1.4",
Source: "ecr#v1.1.4",
Config: ordered.MapFromItems(
ordered.TupleSA{Key: "login", Value: true},
ordered.TupleSA{Key: "account_ids", Value: "xxx"},
ordered.TupleSA{Key: "registry_region", Value: "us-east-1"},
),
},
{
Name: "docker-compose#v2.5.1",
Source: "docker-compose#v2.5.1",
Config: ordered.MapFromItems(
ordered.TupleSA{Key: "run", Value: "xxx"},
ordered.TupleSA{Key: "config", Value: ".buildkite/docker/docker-compose.yml"},
Expand Down Expand Up @@ -1070,19 +1070,19 @@ steps:
"name": ":s3: xxx",
"plugins": [
{
"xxx/aws-assume-role#v0.1.0": {
"github.com/xxx/aws-assume-role-buildkite-plugin#v0.1.0": {
"role": "arn:aws:iam::xxx:role/xxx"
}
},
{
"ecr#v1.1.4": {
"github.com/buildkite-plugins/ecr-buildkite-plugin#v1.1.4": {
"login": true,
"account_ids": "xxx",
"registry_region": "us-east-1"
}
},
{
"docker-compose#v2.5.1": {
"github.com/buildkite-plugins/docker-compose-buildkite-plugin#v2.5.1": {
"run": "xxx",
"config": ".buildkite/docker/docker-compose.yml",
"env": [
Expand Down Expand Up @@ -1124,13 +1124,13 @@ func TestParserParsesScalarPlugins(t *testing.T) {
Command: "script/buildkite/xxx.sh",
Plugins: Plugins{
{
Name: "example-plugin#v1.0.0",
Source: "example-plugin#v1.0.0",
},
{
Name: "another-plugin#v0.0.1-beta43",
Source: "another-plugin#v0.0.1-beta43",
},
{
Name: "docker-compose#v2.5.1",
Source: "docker-compose#v2.5.1",
Config: ordered.MapFromItems(
ordered.TupleSA{Key: "config", Value: ".buildkite/docker/docker-compose.yml"},
),
Expand All @@ -1156,10 +1156,10 @@ func TestParserParsesScalarPlugins(t *testing.T) {
"command": "script/buildkite/xxx.sh",
"name": ":s3: xxx",
"plugins": [
"example-plugin#v1.0.0",
"another-plugin#v0.0.1-beta43",
"github.com/buildkite-plugins/example-plugin-buildkite-plugin#v1.0.0",
"github.com/buildkite-plugins/another-plugin-buildkite-plugin#v0.0.1-beta43",
{
"docker-compose#v2.5.1": {
"github.com/buildkite-plugins/docker-compose-buildkite-plugin#v2.5.1": {
"config": ".buildkite/docker/docker-compose.yml"
}
}
Expand Down
Loading