-
Notifications
You must be signed in to change notification settings - Fork 4
feat: add shared platform buildkite/githubactions BuildRunner clients #423
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
base: main
Are you sure you want to change the base?
Changes from all commits
52e64ce
7826743
4f58d41
a31f44d
fa7875f
85fc006
3c10122
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| load("@rules_go//go:def.bzl", "go_library", "go_test") | ||
|
|
||
| go_library( | ||
| name = "go_default_library", | ||
| srcs = ["client.go"], | ||
| importpath = "github.com/uber/submitqueue/platform/extension/buildrunner/buildkite", | ||
| visibility = ["//visibility:public"], | ||
| ) | ||
|
|
||
| go_test( | ||
| name = "go_default_test", | ||
| srcs = ["client_test.go"], | ||
| embed = [":go_default_library"], | ||
| deps = [ | ||
| "//platform/http:go_default_library", | ||
| "@com_github_stretchr_testify//assert:go_default_library", | ||
| "@com_github_stretchr_testify//require:go_default_library", | ||
| ], | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| # Buildkite client | ||
|
|
||
| Shared HTTP client and Buildkite-specific facts for every domain's Buildkite-backed `BuildRunner`. There is no `BuildRunner` interface here by design — each domain (`submitqueue`, `stovepipe`, ...) defines its own `BuildRunner` and its own `BuildStatus`, and adapts this package's `State` to it. See [`doc/rfc/stovepipe/steps/build.md`](../../../../doc/rfc/stovepipe/steps/build.md#alternatives-considered-for-sharing-the-contract) for why the contract stays per-domain while the backend is shared. | ||
|
|
||
| ## What lives here | ||
|
|
||
| - `Client`: `CreateBuild` / `GetBuild` / `CancelBuild` against the Buildkite REST API, plus `EncodeBuildNumber` / `ParseBuildNumber` for the build-number-as-id convention. Construct one with `NewClient(httpClient)`, where `httpClient` already has the pipeline's base URL (via `platform/http.BaseURLTransport`) and auth configured. | ||
| - `State` / `ParseState`: Buildkite's own build-state vocabulary (`creating`, `scheduled`, `running`, `blocked`, `passed`, `failed`, `canceling`, `canceled`, `skipped`, `not_run`), collapsed into the five states every domain's `BuildStatus` already distinguishes. Each domain still does its own trivial `State` → its own `BuildStatus` switch — this package does not know either domain's entity types. | ||
| - `DecodeMetadataEnv`: recovers a JSON-encoded `map[string]string` from a build's echoed env vars, given the env key the caller used at trigger time. | ||
|
|
||
| ## Who consumes it | ||
|
|
||
| - `submitqueue/extension/buildrunner/buildkite` — batch-identity `BuildRunner`, resolves changes via `changeset.Resolver` before triggering. | ||
| - `stovepipe/extension/buildrunner/buildkite` — URI-identity `BuildRunner`, triggers directly from `headURI`/`baseURI`. | ||
|
|
||
| Both wrap a `*Client` built at the wiring layer; this package never constructs one from raw config (no credentials, no pipeline slug) itself. | ||
|
|
||
| ## How the same `Client` stays safe to share across two different checkout strategies | ||
|
|
||
| SubmitQueue's build materializes state that doesn't exist yet — the runner resolves a batch DAG into composite base/head commits by applying patches. Stovepipe's build checks out a commit that already exists on trunk and, for an incremental build, diffs it against a baseline. These are different problems at the CI-pipeline level (see [build.md's "Why separate contracts"](../../../../doc/rfc/stovepipe/steps/build.md#why-separate-contracts)), yet both go through the same `Client.CreateBuild` call — the `Client` never inspects `CreateBuildRequest.Env` or picks a strategy, so there is nothing here that needs to "know" which pattern applies. | ||
|
|
||
| The split happens entirely outside this package, at two layers below it: | ||
|
|
||
| 1. **Each domain's own adapter shapes a different `Env` payload.** SubmitQueue's runner sends `SQ_BASE_URIS`/`SQ_HEAD_URIS` as JSON-encoded arrays of resolved change URIs; stovepipe's sends `STOVEPIPE_HEAD_URI`/`STOVEPIPE_BASE_URI` as single plain strings. Both just call `Client.CreateBuild` with their own `Env` map — this package treats it as an opaque `map[string]string`. | ||
| 2. **Each domain's `Client` targets a different Buildkite pipeline**, bound once at wiring time via the `httpClient`'s `BaseURLTransport.BaseURL` (e.g. `.../pipelines/submitqueue-go-code` vs `.../pipelines/stovepipe-go-code`). Each pipeline has its own Buildkite pipeline script (owned by Buildkite config, outside this repo) written against its domain's env-var contract — one applies patches into composite commits, the other checks out `STOVEPIPE_HEAD_URI` and diffs against `STOVEPIPE_BASE_URI`. | ||
|
|
||
| So the checkout strategy is a static, wiring-time binding — one `Client` instance ↔ one pipeline ↔ one pipeline script ↔ one env-var contract ↔ one domain's adapter — never a runtime decision made anywhere in Go code. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,237 @@ | ||
| // Copyright (c) 2025 Uber Technologies, Inc. | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| // Package buildkite provides the HTTP client and Buildkite-specific facts | ||
| // (build state vocabulary, the env-var metadata round-trip convention, and | ||
| // build number id encoding) shared by every domain's Buildkite-backed | ||
| // BuildRunner. It intentionally holds no BuildRunner interface or domain | ||
| // entity types — each domain (submitqueue, stovepipe, ...) defines its own | ||
| // BuildRunner and its own BuildStatus, and adapts this package's State to it. | ||
| // See doc/rfc/stovepipe/steps/build.md's "Alternatives considered for | ||
| // sharing the contract" for the rationale. | ||
| package buildkite | ||
|
|
||
| import ( | ||
| "bytes" | ||
| "context" | ||
| "encoding/json" | ||
| "fmt" | ||
| "io" | ||
| "net/http" | ||
| "strconv" | ||
| ) | ||
|
|
||
| // Client is a thin wrapper around the Buildkite REST endpoints a BuildRunner | ||
| // needs: create, get, and cancel a build. | ||
| type Client struct { | ||
| httpClient *http.Client | ||
| } | ||
|
|
||
| // NewClient wraps a pre-configured *http.Client as a Buildkite Client. The | ||
| // caller is responsible for the base URL (via platform/http.BaseURLTransport) | ||
| // and auth (via an Authorization-header transport). | ||
| func NewClient(httpClient *http.Client) *Client { | ||
| return &Client{httpClient: httpClient} | ||
| } | ||
|
|
||
| // CreateBuildRequest is the payload for POST /builds. | ||
| type CreateBuildRequest struct { | ||
| Message string `json:"message"` | ||
| Env map[string]string `json:"env"` | ||
| } | ||
|
|
||
| // BuildResponse is the subset of fields callers need from a Buildkite build | ||
| // object. | ||
| type BuildResponse struct { | ||
| Number int `json:"number"` | ||
| State string `json:"state"` | ||
| WebURL string `json:"web_url"` | ||
| Env map[string]string `json:"env"` | ||
| } | ||
|
|
||
| // CreateBuild creates a new Buildkite build. | ||
| func (c *Client) CreateBuild(ctx context.Context, req CreateBuildRequest) (BuildResponse, error) { | ||
| body, err := json.Marshal(req) | ||
| if err != nil { | ||
| return BuildResponse{}, fmt.Errorf("marshal request: %w", err) | ||
| } | ||
| var build BuildResponse | ||
| if err := c.do(ctx, http.MethodPost, "/builds", body, &build); err != nil { | ||
| return BuildResponse{}, err | ||
| } | ||
| return build, nil | ||
| } | ||
|
|
||
| // GetBuild fetches a build by its Buildkite build number. | ||
| func (c *Client) GetBuild(ctx context.Context, number int) (BuildResponse, error) { | ||
| u := fmt.Sprintf("/builds/%d", number) | ||
| var build BuildResponse | ||
| if err := c.do(ctx, http.MethodGet, u, nil, &build); err != nil { | ||
| return BuildResponse{}, err | ||
| } | ||
| return build, nil | ||
| } | ||
|
|
||
| // CancelBuild requests cancellation. Returns nil when the build is already | ||
| // terminal (HTTP 422) — the Buildkite API uses that status to indicate a | ||
| // non-cancellable build, which the BuildRunner contract treats as a no-op. | ||
| func (c *Client) CancelBuild(ctx context.Context, number int) error { | ||
| u := fmt.Sprintf("/builds/%d/cancel", number) | ||
| req, err := http.NewRequestWithContext(ctx, http.MethodPut, u, nil) | ||
| if err != nil { | ||
| return fmt.Errorf("create request: %w", err) | ||
| } | ||
| c.setHeaders(req) | ||
|
|
||
| resp, err := c.httpClient.Do(req) | ||
| if err != nil { | ||
| return fmt.Errorf("send request: %w", err) | ||
| } | ||
| defer resp.Body.Close() | ||
| _, _ = io.Copy(io.Discard, resp.Body) | ||
|
|
||
| switch resp.StatusCode { | ||
| case http.StatusOK: | ||
| return nil | ||
| case http.StatusUnprocessableEntity: | ||
| // Already terminal — no-op per BuildRunner.Cancel contract. | ||
| return nil | ||
| default: | ||
| return fmt.Errorf("unexpected status %d from cancel", resp.StatusCode) | ||
| } | ||
| } | ||
|
|
||
| // do sends an HTTP request with the standard Buildkite headers and, on a 2xx | ||
| // response, decodes the body into out (when non-nil). A 404 is reported as a | ||
| // "build not found" error per the BuildRunner contract. | ||
| func (c *Client) do(ctx context.Context, method, rawURL string, body []byte, out any) error { | ||
| var bodyReader io.Reader | ||
| if body != nil { | ||
| bodyReader = bytes.NewReader(body) | ||
| } | ||
|
|
||
| req, err := http.NewRequestWithContext(ctx, method, rawURL, bodyReader) | ||
| if err != nil { | ||
| return fmt.Errorf("create request: %w", err) | ||
| } | ||
| c.setHeaders(req) | ||
|
|
||
| resp, err := c.httpClient.Do(req) | ||
| if err != nil { | ||
| return fmt.Errorf("send request: %w", err) | ||
| } | ||
| defer resp.Body.Close() | ||
|
|
||
| respBody, err := io.ReadAll(resp.Body) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this may not properly respond to context cancellation. AI analysis:
The poor man's alternative is buffered read with cancellation checks: |
||
| if err != nil { | ||
| return fmt.Errorf("read response: %w", err) | ||
| } | ||
|
|
||
| if resp.StatusCode == http.StatusNotFound { | ||
| return fmt.Errorf("build not found") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. http helper function do() is not necessarily about build. You may want to define sentinel errors to transform http codes into. |
||
| } | ||
| if resp.StatusCode < 200 || resp.StatusCode >= 300 { | ||
| return fmt.Errorf("API returned status %d: %s", resp.StatusCode, respBody) | ||
| } | ||
|
|
||
| if out != nil { | ||
| if err := json.Unmarshal(respBody, out); err != nil { | ||
| return fmt.Errorf("unmarshal response: %w", err) | ||
| } | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func (c *Client) setHeaders(req *http.Request) { | ||
| req.Header.Set("Content-Type", "application/json") | ||
| } | ||
|
|
||
| // EncodeBuildNumber encodes a Buildkite build number as an opaque build id | ||
| // string. | ||
| func EncodeBuildNumber(number int) string { | ||
| return strconv.Itoa(number) | ||
| } | ||
|
|
||
| // ParseBuildNumber is the inverse of EncodeBuildNumber. | ||
| func ParseBuildNumber(id string) (int, error) { | ||
| n, err := strconv.Atoi(id) | ||
| if err != nil { | ||
| return 0, fmt.Errorf("invalid build ID %q", id) | ||
| } | ||
| return n, nil | ||
| } | ||
|
|
||
| // State is Buildkite's own build-state vocabulary, interpreted from the raw | ||
| // state string every domain's Buildkite adapter receives on a build object. | ||
| // Buildkite's raw states are: creating, scheduled, running, blocked, passed, | ||
| // failed, canceling, canceled, skipped, not_run. | ||
| type State string | ||
|
|
||
| const ( | ||
| // StateUnknown is returned for a raw state this package does not | ||
| // recognize. Not terminal — callers should keep polling rather than | ||
| // treat it as a final outcome. | ||
| StateUnknown State = "" | ||
| // StateAccepted means the build has been accepted for execution but has | ||
| // not started running yet (Buildkite: creating, scheduled). | ||
| StateAccepted State = "accepted" | ||
| // StateRunning means the build is currently executing, including while | ||
| // blocked on a manual block step (Buildkite: running, blocked). | ||
| StateRunning State = "running" | ||
| // StateSucceeded means the build completed successfully (Buildkite: | ||
| // passed). | ||
| StateSucceeded State = "succeeded" | ||
| // StateFailed means the build did not produce a passing result | ||
| // (Buildkite: failed, not_run, skipped). | ||
| StateFailed State = "failed" | ||
| // StateCancelled means the build was cancelled, or is in the process of | ||
| // being cancelled (Buildkite: canceling, canceled). | ||
| StateCancelled State = "cancelled" | ||
| ) | ||
|
|
||
| // ParseState maps a raw Buildkite build-state string to a State. An | ||
| // unrecognized raw string maps to StateUnknown rather than being assumed | ||
| // terminal. | ||
| func ParseState(raw string) State { | ||
| switch raw { | ||
| case "creating", "scheduled": | ||
| return StateAccepted | ||
| case "running", "blocked": | ||
| return StateRunning | ||
| case "passed": | ||
| return StateSucceeded | ||
| case "failed", "not_run", "skipped": | ||
| return StateFailed | ||
| case "canceling", "canceled": | ||
| return StateCancelled | ||
| default: | ||
| return StateUnknown | ||
| } | ||
| } | ||
|
|
||
| // DecodeMetadataEnv recovers a JSON-encoded map[string]string from env[key]. | ||
| // Buildkite echoes env vars back on the build object, so a caller that seeded | ||
| // a JSON-encoded map at Trigger time can recover it here at Status time | ||
| // without any local state. Returns an empty non-nil map when the key is | ||
| // absent or its value cannot be decoded — a corrupt env var must not fail the | ||
| // caller. | ||
| func DecodeMetadataEnv(env map[string]string, key string) map[string]string { | ||
| meta := make(map[string]string) | ||
| raw, ok := env[key] | ||
| if !ok || raw == "" { | ||
| return meta | ||
| } | ||
| _ = json.Unmarshal([]byte(raw), &meta) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. let's not swallow errors without proper visibility |
||
| return meta | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would be nice to also include the message into the error, typically in a body of a response; can be truncated if needed.