diff --git a/submitqueue/extension/buildrunner/buildkite/buildkite.go b/submitqueue/extension/buildrunner/buildkite/buildkite.go index eccd046..b38fefe 100644 --- a/submitqueue/extension/buildrunner/buildkite/buildkite.go +++ b/submitqueue/extension/buildrunner/buildkite/buildkite.go @@ -23,6 +23,10 @@ // environment variables (SQ_BASE_URIS, SQ_HEAD_URIS, SQ_QUEUE). The pipeline // script fetches each PR's diff with the GitHub API, applies them with // `git apply -3`, produces one commit per layer (base, head), then runs CI. +// +// Caller-supplied BuildMetadata is forwarded to the build as SQ_METADATA +// (JSON-encoded). Buildkite echoes env vars back on the build object, so +// Status recovers and returns the original metadata without any local state. package buildkite import ( @@ -53,6 +57,11 @@ const ( // EnvKeyQueue carries the SQ queue name so the pipeline script can select // queue-specific test targets. EnvKeyQueue = "SQ_QUEUE" + + // EnvKeyMetadata carries the JSON-encoded BuildMetadata provided by the + // caller to Trigger. Buildkite echoes env vars on the build object, so + // Status can recover and return the original metadata without local state. + EnvKeyMetadata = "SQ_METADATA" ) // runner implements buildrunner.BuildRunner. @@ -106,17 +115,23 @@ func newRunner(cfg buildrunner.Config, c *client, logger *zap.SugaredLogger) *ru // Trigger calls the Buildkite API to create the build and returns the Buildkite // build number as the build ID. Errors are propagated to the caller so the // queue consumer can nack and retry. -func (r *runner) Trigger(ctx context.Context, base, head []entity.Change, _ entity.BuildMetadata) (entity.BuildID, error) { +func (r *runner) Trigger(ctx context.Context, base, head []entity.Change, metadata entity.BuildMetadata) (entity.BuildID, error) { baseJSON, _ := json.Marshal(flattenURIs(base)) headJSON, _ := json.Marshal(flattenURIs(head)) + env := map[string]string{ + EnvKeyBaseURIs: string(baseJSON), + EnvKeyHeadURIs: string(headJSON), + EnvKeyQueue: r.cfg.QueueName, + } + if len(metadata) > 0 { + metaJSON, _ := json.Marshal(metadata) + env[EnvKeyMetadata] = string(metaJSON) + } + req := createBuildRequest{ Message: "submitqueue speculative build", - Env: map[string]string{ - EnvKeyBaseURIs: string(baseJSON), - EnvKeyHeadURIs: string(headJSON), - EnvKeyQueue: r.cfg.QueueName, - }, + Env: env, } resp, err := r.client.createBuild(ctx, req) @@ -131,7 +146,7 @@ func (r *runner) Trigger(ctx context.Context, base, head []entity.Change, _ enti } // Status fetches the current state of the build from Buildkite and returns it -// with the build URL in BuildMetadata["url"]. +// with the build URL and any caller-supplied metadata in BuildMetadata. func (r *runner) Status(ctx context.Context, buildID entity.BuildID) (entity.BuildStatus, entity.BuildMetadata, error) { number, err := parseBuildNumber(buildID.ID) if err != nil { @@ -143,7 +158,9 @@ func (r *runner) Status(ctx context.Context, buildID entity.BuildID) (entity.Bui return entity.BuildStatusUnknown, nil, fmt.Errorf("buildkite: get build: %w", err) } - return mapState(resp.State), entity.BuildMetadata{"url": resp.WebURL}, nil + meta := decodeMetadata(resp.Env) + meta["url"] = resp.WebURL + return mapState(resp.State), meta, nil } // Cancel calls the Buildkite API to cancel the build. A no-op on already-terminal @@ -186,6 +203,20 @@ func parseBuildNumber(id string) (int, error) { return n, nil } +// decodeMetadata recovers the caller-supplied BuildMetadata from the env vars +// Buildkite echoes back on the build object. Returns an empty non-nil map when +// SQ_METADATA is absent or cannot be decoded — a corrupt env var must not fail +// a Status call. +func decodeMetadata(env map[string]string) entity.BuildMetadata { + meta := make(entity.BuildMetadata) + raw, ok := env[EnvKeyMetadata] + if !ok || raw == "" { + return meta + } + _ = json.Unmarshal([]byte(raw), &meta) + return meta +} + // mapState maps a Buildkite build state string to a BuildStatus. // // Buildkite states: creating, scheduled, running, blocked, passed, failed, diff --git a/submitqueue/extension/buildrunner/buildkite/buildkite_test.go b/submitqueue/extension/buildrunner/buildkite/buildkite_test.go index 5f8c83c..cea6a1a 100644 --- a/submitqueue/extension/buildrunner/buildkite/buildkite_test.go +++ b/submitqueue/extension/buildrunner/buildkite/buildkite_test.go @@ -47,7 +47,12 @@ func newTestRunner(t *testing.T, handler http.Handler) *runner { // buildJSON encodes fields into a minimal Buildkite build JSON response. func buildJSON(number int, state, webURL string) []byte { - b, _ := json.Marshal(buildResponse{Number: number, State: state, WebURL: webURL}) + return buildJSONWithEnv(number, state, webURL, nil) +} + +// buildJSONWithEnv encodes fields into a Buildkite build JSON response including env vars. +func buildJSONWithEnv(number int, state, webURL string, env map[string]string) []byte { + b, _ := json.Marshal(buildResponse{Number: number, State: state, WebURL: webURL, Env: env}) return b } @@ -137,6 +142,43 @@ func TestTrigger_BuildkiteError_ReturnsError(t *testing.T) { require.Error(t, err) } +func TestTrigger_WithMetadata_SetsEnvVar(t *testing.T) { + var capturedBody []byte + r := newTestRunner(t, http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + capturedBody, _ = io.ReadAll(req.Body) + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write(buildJSON(10, "scheduled", "")) + })) + + metadata := entity.BuildMetadata{"requester": "alice", "ticket": "SQ-42"} + _, err := r.Trigger(context.Background(), nil, []entity.Change{{URIs: []string{"u"}}}, metadata) + require.NoError(t, err) + + var req createBuildRequest + require.NoError(t, json.Unmarshal(capturedBody, &req)) + require.Contains(t, req.Env, EnvKeyMetadata) + + var got entity.BuildMetadata + require.NoError(t, json.Unmarshal([]byte(req.Env[EnvKeyMetadata]), &got)) + assert.Equal(t, metadata, got) +} + +func TestTrigger_NilMetadata_NoMetadataEnvVar(t *testing.T) { + var capturedBody []byte + r := newTestRunner(t, http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + capturedBody, _ = io.ReadAll(req.Body) + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write(buildJSON(11, "scheduled", "")) + })) + + _, err := r.Trigger(context.Background(), nil, []entity.Change{{URIs: []string{"u"}}}, nil) + require.NoError(t, err) + + var req createBuildRequest + require.NoError(t, json.Unmarshal(capturedBody, &req)) + assert.NotContains(t, req.Env, EnvKeyMetadata) +} + // --- Status --- func TestStatus_StateMapping(t *testing.T) { @@ -187,6 +229,35 @@ func TestStatus_BuildkiteNotFound(t *testing.T) { require.Error(t, err) } +func TestStatus_EchosCallerMetadata(t *testing.T) { + metadata := entity.BuildMetadata{"requester": "alice", "ticket": "SQ-42"} + metaJSON, _ := json.Marshal(metadata) + + r := newTestRunner(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write(buildJSONWithEnv(7, "passed", "https://buildkite.com/test-org/my-pipeline/builds/7", + map[string]string{EnvKeyMetadata: string(metaJSON)}, + )) + })) + + _, meta, err := r.Status(context.Background(), entity.BuildID{ID: encodeBuildNumber(7)}) + require.NoError(t, err) + assert.Equal(t, "alice", meta["requester"]) + assert.Equal(t, "SQ-42", meta["ticket"]) + assert.Equal(t, "https://buildkite.com/test-org/my-pipeline/builds/7", meta["url"]) +} + +func TestStatus_NoMetadata_ReturnsOnlyURL(t *testing.T) { + r := newTestRunner(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write(buildJSON(8, "running", "https://buildkite.com/test-org/my-pipeline/builds/8")) + })) + + _, meta, err := r.Status(context.Background(), entity.BuildID{ID: encodeBuildNumber(8)}) + require.NoError(t, err) + assert.Equal(t, entity.BuildMetadata{"url": "https://buildkite.com/test-org/my-pipeline/builds/8"}, meta) +} + // --- Cancel --- func TestCancel_CallsBuildkite(t *testing.T) { diff --git a/submitqueue/extension/buildrunner/buildkite/client.go b/submitqueue/extension/buildrunner/buildkite/client.go index 38a217f..dcf09d9 100644 --- a/submitqueue/extension/buildrunner/buildkite/client.go +++ b/submitqueue/extension/buildrunner/buildkite/client.go @@ -37,9 +37,10 @@ type createBuildRequest struct { // buildResponse is the subset of fields the runner needs from a Buildkite // build object. type buildResponse struct { - Number int `json:"number"` - State string `json:"state"` - WebURL string `json:"web_url"` + Number int `json:"number"` + State string `json:"state"` + WebURL string `json:"web_url"` + Env map[string]string `json:"env"` } func (c *client) createBuild(ctx context.Context, req createBuildRequest) (buildResponse, error) {