From 377e50ba65bfa6278683c001c144fb652a1c3e7e Mon Sep 17 00:00:00 2001 From: Albert Wu Date: Fri, 5 Jun 2026 13:43:35 -0700 Subject: [PATCH 1/2] feat(buildrunner): add GitHub Actions backend --- submitqueue/extension/buildrunner/README.md | 8 + .../buildrunner/githubactions/BUILD.bazel | 30 ++ .../buildrunner/githubactions/README.md | 91 +++++ .../buildrunner/githubactions/client.go | 178 ++++++++++ .../githubactions/githubactions.go | 332 ++++++++++++++++++ .../githubactions/githubactions_test.go | 237 +++++++++++++ 6 files changed, 876 insertions(+) create mode 100644 submitqueue/extension/buildrunner/githubactions/BUILD.bazel create mode 100644 submitqueue/extension/buildrunner/githubactions/README.md create mode 100644 submitqueue/extension/buildrunner/githubactions/client.go create mode 100644 submitqueue/extension/buildrunner/githubactions/githubactions.go create mode 100644 submitqueue/extension/buildrunner/githubactions/githubactions_test.go diff --git a/submitqueue/extension/buildrunner/README.md b/submitqueue/extension/buildrunner/README.md index 01e716f3..d6ea2f80 100644 --- a/submitqueue/extension/buildrunner/README.md +++ b/submitqueue/extension/buildrunner/README.md @@ -10,3 +10,11 @@ See [`doc/rfc/submitqueue/build-runner.md`](../../../doc/rfc/submitqueue/build-r 2. Map the `base` and `head` change slices onto the backend's build primitives (apply `base`, apply `head`, validate the result). 3. Map the runner's lifecycle states down to the `BuildStatus` values: `Accepted` (accepted for execution), `Running` (executing), and the terminal `Succeeded` / `Failed` / `Cancelled`. 4. Implement internal reconnect / retry so transient failures surface as plain errors without blocking the caller. + +## Backends + +- `noop`: local-development backend that immediately succeeds every build. +- `githubactions`: proof-of-architecture backend that dispatches a GitHub + Actions workflow. See [`githubactions/README.md`](githubactions/README.md) + for the workflow inputs and example orchestrator environment variables. +- `buildkite`: Buildkite-backed backend. diff --git a/submitqueue/extension/buildrunner/githubactions/BUILD.bazel b/submitqueue/extension/buildrunner/githubactions/BUILD.bazel new file mode 100644 index 00000000..8ff1abb7 --- /dev/null +++ b/submitqueue/extension/buildrunner/githubactions/BUILD.bazel @@ -0,0 +1,30 @@ +load("@rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "githubactions", + srcs = [ + "client.go", + "githubactions.go", + ], + importpath = "github.com/uber/submitqueue/submitqueue/extension/buildrunner/githubactions", + visibility = ["//visibility:public"], + deps = [ + "//submitqueue/entity", + "//submitqueue/extension/buildrunner", + "@org_uber_go_zap//:zap", + ], +) + +go_test( + name = "githubactions_test", + srcs = ["githubactions_test.go"], + embed = [":githubactions"], + deps = [ + "//core/httpclient", + "//submitqueue/entity", + "//submitqueue/extension/buildrunner", + "@com_github_stretchr_testify//assert", + "@com_github_stretchr_testify//require", + "@org_uber_go_zap//:zap", + ], +) diff --git a/submitqueue/extension/buildrunner/githubactions/README.md b/submitqueue/extension/buildrunner/githubactions/README.md new file mode 100644 index 00000000..e69d1a2b --- /dev/null +++ b/submitqueue/extension/buildrunner/githubactions/README.md @@ -0,0 +1,91 @@ +# GitHub Actions BuildRunner + +`githubactions` implements `buildrunner.BuildRunner` with GitHub Actions +`workflow_dispatch`. It is intended to prove the SubmitQueue BuildRunner +architecture against a common CI system without adding local state. + +## How it works + +1. `Trigger` dispatches the configured workflow on a trusted ref, usually + `main`, and returns GitHub's workflow run ID as the SubmitQueue build ID. +2. SubmitQueue passes these workflow inputs: + - `sq_base_uris` + - `sq_head_uris` + - `sq_queue` + - `sq_metadata` +3. `Status` calls GitHub's get-workflow-run endpoint with that run ID. +4. `Cancel` calls GitHub's cancel-workflow-run endpoint with that run ID. + +## Minimal workflow + +Create a workflow on the target repository's default branch: + +```yaml +name: SubmitQueue CI +run-name: SubmitQueue ${{ inputs.sq_queue }} + +on: + workflow_dispatch: + inputs: + sq_base_uris: + required: true + type: string + sq_head_uris: + required: true + type: string + sq_queue: + required: true + type: string + sq_metadata: + required: false + type: string + +permissions: + contents: read + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Inspect SubmitQueue payload + run: | + echo '${{ inputs.sq_base_uris }}' + echo '${{ inputs.sq_head_uris }}' + echo '${{ inputs.sq_queue }}' + # Prototype: add a script here that applies sq_base_uris, then + # sq_head_uris, then runs the repository's real CI command. +``` + +The workflow definition should live on a trusted ref. The untrusted changes +should be represented by `sq_base_uris` and `sq_head_uris` and applied inside +the job. + +## Integrator configuration + +A server wiring this backend should provide the GitHub API client and runner +configuration equivalent to: + +```sh +BUILD_RUNNER=githubactions +GITHUB_BASE_URL=https://api.github.com +GITHUB_TOKEN= +GITHUB_ACTIONS_OWNER=uber +GITHUB_ACTIONS_REPO=submitqueue +GITHUB_ACTIONS_WORKFLOW=submitqueue-ci.yml +GITHUB_ACTIONS_REF=main +GITHUB_ACTIONS_EXTRA_INPUTS=runner=ubuntu-latest,test_command=make test +``` + +`GITHUB_ACTIONS_REF` defaults to `main`. `GITHUB_ACTIONS_EXTRA_INPUTS` is +optional comma-separated `key=value` data copied into every dispatch request; +use it for workflow-specific knobs like runner labels or test commands. + +## Practical setup notes + +- Use a trusted workflow definition from the target repository's default branch. +- Keep workflow permissions minimal. The job may apply untrusted changes before + running tests, so avoid broad secrets in this workflow. +- The backend proves the BuildRunner architecture. It does not prescribe how + your workflow materializes `sq_base_uris` and `sq_head_uris`; wire that to the + repository's existing patch/PR application logic. diff --git a/submitqueue/extension/buildrunner/githubactions/client.go b/submitqueue/extension/buildrunner/githubactions/client.go new file mode 100644 index 00000000..535cc105 --- /dev/null +++ b/submitqueue/extension/buildrunner/githubactions/client.go @@ -0,0 +1,178 @@ +// 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 githubactions + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "net/url" + "strconv" +) + +const githubAPIVersion = "2026-03-10" + +// client is a thin wrapper around the GitHub Actions REST endpoints that +// BuildRunner needs: workflow dispatch, get workflow run, and cancel run. +type client struct { + httpClient *http.Client + owner string + repo string + workflowID string +} + +type dispatchWorkflowRequest struct { + Ref string `json:"ref"` + ReturnRunDetails bool `json:"return_run_details,omitempty"` + Inputs map[string]string `json:"inputs,omitempty"` +} + +type dispatchWorkflowResponse struct { + WorkflowRunID int64 `json:"workflow_run_id"` + RunURL string `json:"run_url"` + HTMLURL string `json:"html_url"` +} + +// workflowRun is the subset of a GitHub Actions workflow run object the +// runner needs. +type workflowRun struct { + ID int64 `json:"id"` + Name string `json:"name"` + DisplayTitle string `json:"display_title"` + Status string `json:"status"` + Conclusion string `json:"conclusion"` + HTMLURL string `json:"html_url"` + RunAttempt int `json:"run_attempt"` + Event string `json:"event"` + HeadBranch string `json:"head_branch"` + CreatedAt string `json:"created_at"` +} + +func (c *client) dispatchWorkflow(ctx context.Context, req dispatchWorkflowRequest) (dispatchWorkflowResponse, error) { + body, err := json.Marshal(req) + if err != nil { + return dispatchWorkflowResponse{}, fmt.Errorf("marshal request: %w", err) + } + var resp dispatchWorkflowResponse + if err := c.do(ctx, http.MethodPost, c.workflowPath("dispatches"), body, &resp); err != nil { + return dispatchWorkflowResponse{}, err + } + return resp, nil +} + +func (c *client) getRun(ctx context.Context, runID int64) (workflowRun, error) { + var run workflowRun + if err := c.do(ctx, http.MethodGet, c.runPath(runID), nil, &run); err != nil { + return workflowRun{}, err + } + return run, nil +} + +func (c *client) cancelRun(ctx context.Context, runID int64) error { + req, err := http.NewRequestWithContext(ctx, http.MethodPost, c.runPath(runID)+"/cancel", 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.StatusAccepted, http.StatusOK, http.StatusCreated, http.StatusNoContent: + return nil + case http.StatusConflict, http.StatusUnprocessableEntity: + // Already terminal or otherwise not cancellable: no-op per + // BuildRunner.Cancel contract. + return nil + case http.StatusNotFound: + return fmt.Errorf("workflow run not found") + default: + return fmt.Errorf("unexpected status %d from cancel", resp.StatusCode) + } +} + +func (c *client) workflowPath(suffix string) string { + path := fmt.Sprintf( + "/repos/%s/%s/actions/workflows/%s", + url.PathEscape(c.owner), + url.PathEscape(c.repo), + url.PathEscape(c.workflowID), + ) + if suffix != "" { + path += "/" + suffix + } + return path +} + +func (c *client) runPath(runID int64) string { + return fmt.Sprintf( + "/repos/%s/%s/actions/runs/%s", + url.PathEscape(c.owner), + url.PathEscape(c.repo), + url.PathEscape(strconv.FormatInt(runID, 10)), + ) +} + +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) + if err != nil { + return fmt.Errorf("read response: %w", err) + } + + if resp.StatusCode == http.StatusNotFound { + return fmt.Errorf("GitHub API returned 404 for %s %s", method, rawURL) + } + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + return fmt.Errorf("API returned status %d: %s", resp.StatusCode, respBody) + } + + if out != nil && len(respBody) > 0 { + 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("Accept", "application/vnd.github+json") + req.Header.Set("Content-Type", "application/json") + req.Header.Set("X-GitHub-Api-Version", githubAPIVersion) +} diff --git a/submitqueue/extension/buildrunner/githubactions/githubactions.go b/submitqueue/extension/buildrunner/githubactions/githubactions.go new file mode 100644 index 00000000..242a6752 --- /dev/null +++ b/submitqueue/extension/buildrunner/githubactions/githubactions.go @@ -0,0 +1,332 @@ +// 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 githubactions implements buildrunner.BuildRunner backed by GitHub +// Actions workflow_dispatch. +// +// The runner dispatches a trusted workflow and passes SubmitQueue's base/head +// change URIs as workflow inputs. Trigger returns the GitHub workflow run ID; +// Status and Cancel use that ID to call GitHub's workflow-run endpoints. +package githubactions + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "strconv" + + "go.uber.org/zap" + + "github.com/uber/submitqueue/submitqueue/entity" + "github.com/uber/submitqueue/submitqueue/extension/buildrunner" +) + +const ( + // InputKeyBaseURIs carries the JSON-encoded ordered list of change URIs + // from dependency batches. + InputKeyBaseURIs = "sq_base_uris" + // InputKeyHeadURIs carries the JSON-encoded ordered list of change URIs + // from the batch under test. + InputKeyHeadURIs = "sq_head_uris" + // InputKeyQueue carries the SubmitQueue queue name. + InputKeyQueue = "sq_queue" + // InputKeyMetadata carries caller-supplied BuildMetadata as JSON. + InputKeyMetadata = "sq_metadata" + + defaultRef = "main" +) + +// Params holds the dependencies and GitHub workflow identity for a GitHub +// Actions BuildRunner. +type Params struct { + // Config holds the per-queue identity for this BuildRunner. + Config buildrunner.Config + // HTTPClient is a pre-configured GitHub API client. The caller is responsible + // for base URL resolution (e.g. via httpclient.BaseURLTransport) and auth. + // The token needs actions:write to dispatch/cancel workflows and actions:read + // to poll status. + HTTPClient *http.Client + // Logger is the structured logger. + Logger *zap.SugaredLogger + // Owner is the repository owner or organization, for example "uber". + Owner string + // Repo is the repository name, for example "submitqueue". + Repo string + // WorkflowID is the workflow file name or numeric workflow ID accepted by + // the GitHub Actions API, for example "submitqueue-ci.yml". + WorkflowID string + // Ref is the branch, tag, or SHA where the trusted workflow is read from. + // Defaults to "main". + Ref string + // ExtraInputs are copied into every workflow_dispatch request. Reserved + // sq_* inputs managed by this package override conflicting keys. + ExtraInputs map[string]string +} + +// FactoryParams holds shared dependencies for a GitHub Actions BuildRunner +// factory. For each queue, For returns a runner with Config.QueueName set to +// that queue. +type FactoryParams struct { + HTTPClient *http.Client + Logger *zap.SugaredLogger + Owner string + Repo string + WorkflowID string + Ref string + ExtraInputs map[string]string +} + +type factory struct { + params FactoryParams +} + +var _ buildrunner.Factory = (*factory)(nil) + +// NewFactory constructs a queue-aware GitHub Actions BuildRunner factory. +func NewFactory(params FactoryParams) (buildrunner.Factory, error) { + if err := validateConfig(params.HTTPClient, params.Logger, params.Owner, params.Repo, params.WorkflowID); err != nil { + return nil, err + } + if params.Ref == "" { + params.Ref = defaultRef + } + return &factory{params: params}, nil +} + +func (f *factory) For(cfg buildrunner.Config) (buildrunner.BuildRunner, error) { + return NewBuildRunner(Params{ + Config: cfg, + HTTPClient: f.params.HTTPClient, + Logger: f.params.Logger, + Owner: f.params.Owner, + Repo: f.params.Repo, + WorkflowID: f.params.WorkflowID, + Ref: f.params.Ref, + ExtraInputs: f.params.ExtraInputs, + }) +} + +type runner struct { + cfg buildrunner.Config + ref string + extraInputs map[string]string + client *client + logger *zap.SugaredLogger +} + +var _ buildrunner.BuildRunner = (*runner)(nil) + +// NewBuildRunner constructs a GitHub Actions-backed BuildRunner bound to one +// repository/workflow and one queue config. +func NewBuildRunner(params Params) (buildrunner.BuildRunner, error) { + if err := validateConfig(params.HTTPClient, params.Logger, params.Owner, params.Repo, params.WorkflowID); err != nil { + return nil, err + } + if params.Ref == "" { + params.Ref = defaultRef + } + + return newRunner( + params.Config, + params.Ref, + params.ExtraInputs, + &client{ + httpClient: params.HTTPClient, + owner: params.Owner, + repo: params.Repo, + workflowID: params.WorkflowID, + }, + params.Logger.Named("githubactions_buildrunner"), + ), nil +} + +func newRunner(cfg buildrunner.Config, ref string, extraInputs map[string]string, c *client, logger *zap.SugaredLogger) *runner { + copied := make(map[string]string, len(extraInputs)) + for k, v := range extraInputs { + copied[k] = v + } + return &runner{ + cfg: cfg, + ref: ref, + extraInputs: copied, + client: c, + logger: logger, + } +} + +func validateConfig(httpClient *http.Client, logger *zap.SugaredLogger, owner, repo, workflowID string) error { + if httpClient == nil { + return fmt.Errorf("http client is required") + } + if logger == nil { + return fmt.Errorf("logger is required") + } + if owner == "" { + return fmt.Errorf("owner is required") + } + if repo == "" { + return fmt.Errorf("repo is required") + } + if workflowID == "" { + return fmt.Errorf("workflow ID is required") + } + return nil +} + +// Trigger dispatches the configured GitHub Actions workflow and returns the +// GitHub workflow run ID as the SubmitQueue 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, metadata entity.BuildMetadata) (entity.BuildID, error) { + inputs, err := r.dispatchInputs(base, head, metadata) + if err != nil { + return entity.BuildID{}, err + } + + resp, err := r.client.dispatchWorkflow(ctx, dispatchWorkflowRequest{ + Ref: r.ref, + ReturnRunDetails: true, + Inputs: inputs, + }) + if err != nil { + return entity.BuildID{}, fmt.Errorf("github actions: dispatch workflow: %w", err) + } + if resp.WorkflowRunID <= 0 { + return entity.BuildID{}, fmt.Errorf("github actions: dispatch workflow: response missing workflow_run_id (requires X-GitHub-Api-Version %s and return_run_details support)", githubAPIVersion) + } + + r.logger.Debugw("dispatched GitHub Actions workflow", + "owner", r.client.owner, + "repo", r.client.repo, + "workflow_id", r.client.workflowID, + "ref", r.ref, + "github_run_id", resp.WorkflowRunID, + ) + return entity.BuildID{ID: strconv.FormatInt(resp.WorkflowRunID, 10)}, nil +} + +func (r *runner) dispatchInputs(base, head []entity.Change, metadata entity.BuildMetadata) (map[string]string, error) { + baseJSON, err := json.Marshal(flattenURIs(base)) + if err != nil { + return nil, fmt.Errorf("marshal base URIs: %w", err) + } + headJSON, err := json.Marshal(flattenURIs(head)) + if err != nil { + return nil, fmt.Errorf("marshal head URIs: %w", err) + } + + inputs := make(map[string]string, len(r.extraInputs)+4) + for k, v := range r.extraInputs { + inputs[k] = v + } + inputs[InputKeyBaseURIs] = string(baseJSON) + inputs[InputKeyHeadURIs] = string(headJSON) + inputs[InputKeyQueue] = r.cfg.QueueName + if len(metadata) > 0 { + metadataJSON, err := json.Marshal(metadata) + if err != nil { + return nil, fmt.Errorf("marshal metadata: %w", err) + } + inputs[InputKeyMetadata] = string(metadataJSON) + } + return inputs, nil +} + +// Status fetches the workflow run by GitHub run ID and maps the run's +// status/conclusion onto BuildStatus. +func (r *runner) Status(ctx context.Context, buildID entity.BuildID) (entity.BuildStatus, entity.BuildMetadata, error) { + runID, err := parseRunID(buildID.ID) + if err != nil { + return entity.BuildStatusUnknown, nil, fmt.Errorf("github actions: malformed build ID: %w", err) + } + + run, err := r.client.getRun(ctx, runID) + if err != nil { + return entity.BuildStatusUnknown, nil, fmt.Errorf("github actions: get run: %w", err) + } + return mapRunStatus(run.Status, run.Conclusion), metadataFromRun(run), nil +} + +// Cancel requests cancellation of the workflow run identified by buildID. +func (r *runner) Cancel(ctx context.Context, buildID entity.BuildID) error { + runID, err := parseRunID(buildID.ID) + if err != nil { + return fmt.Errorf("github actions: malformed build ID: %w", err) + } + if err := r.client.cancelRun(ctx, runID); err != nil { + return fmt.Errorf("github actions: cancel run: %w", err) + } + r.logger.Debugw("cancelled GitHub Actions workflow run", + "github_run_id", runID, + ) + return nil +} + +func metadataFromRun(run workflowRun) entity.BuildMetadata { + meta := entity.BuildMetadata{ + "github_run_id": strconv.FormatInt(run.ID, 10), + "github_run_attempt": strconv.Itoa(run.RunAttempt), + "github_status": run.Status, + "github_conclusion": run.Conclusion, + "github_display_title": run.DisplayTitle, + "url": run.HTMLURL, + } + if run.HeadBranch != "" { + meta["github_head_branch"] = run.HeadBranch + } + if run.CreatedAt != "" { + meta["github_created_at"] = run.CreatedAt + } + return meta +} + +func mapRunStatus(status, conclusion string) entity.BuildStatus { + switch status { + case "queued", "requested", "waiting", "pending": + return entity.BuildStatusAccepted + case "in_progress": + return entity.BuildStatusRunning + case "completed": + switch conclusion { + case "success": + return entity.BuildStatusSucceeded + case "cancelled": + return entity.BuildStatusCancelled + case "": + return entity.BuildStatusUnknown + default: + // Any completed non-success/non-cancelled conclusion is a terminal + // failure for SubmitQueue purposes. + return entity.BuildStatusFailed + } + default: + return entity.BuildStatusUnknown + } +} + +func flattenURIs(changes []entity.Change) []string { + uris := make([]string, 0, len(changes)) + for _, c := range changes { + uris = append(uris, c.URIs...) + } + return uris +} + +func parseRunID(id string) (int64, error) { + runID, err := strconv.ParseInt(id, 10, 64) + if err != nil || runID <= 0 { + return 0, fmt.Errorf("invalid build ID %q", id) + } + return runID, nil +} diff --git a/submitqueue/extension/buildrunner/githubactions/githubactions_test.go b/submitqueue/extension/buildrunner/githubactions/githubactions_test.go new file mode 100644 index 00000000..0d3217e6 --- /dev/null +++ b/submitqueue/extension/buildrunner/githubactions/githubactions_test.go @@ -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 githubactions + +import ( + "context" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "strconv" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/zap" + + "github.com/uber/submitqueue/core/httpclient" + "github.com/uber/submitqueue/submitqueue/entity" + "github.com/uber/submitqueue/submitqueue/extension/buildrunner" +) + +func newTestRunner(t *testing.T, handler http.Handler) *runner { + t.Helper() + srv := httptest.NewServer(handler) + t.Cleanup(srv.Close) + + c, err := httpclient.NewClient(srv.URL) + require.NoError(t, err) + + return newRunner( + buildrunner.Config{QueueName: "my-queue"}, + "main", + map[string]string{"custom": "value"}, + &client{ + httpClient: c, + owner: "uber", + repo: "submitqueue", + workflowID: "submitqueue-ci.yml", + }, + zap.NewNop().Sugar(), + ) +} + +func TestNewBuildRunner_ValidatesConfig(t *testing.T) { + _, err := NewBuildRunner(Params{Logger: zap.NewNop().Sugar()}) + require.Error(t, err) + assert.Contains(t, err.Error(), "http client is required") +} + +func TestNewFactory_ForBuildsQueueBoundRunner(t *testing.T) { + f, err := NewFactory(FactoryParams{ + HTTPClient: http.DefaultClient, + Logger: zap.NewNop().Sugar(), + Owner: "uber", + Repo: "submitqueue", + WorkflowID: "submitqueue-ci.yml", + Ref: "main", + ExtraInputs: map[string]string{"runner": "ubuntu-latest"}, + }) + require.NoError(t, err) + + br, err := f.For(buildrunner.Config{QueueName: "queue-a"}) + require.NoError(t, err) + r := br.(*runner) + assert.Equal(t, "queue-a", r.cfg.QueueName) + assert.Equal(t, "ubuntu-latest", r.extraInputs["runner"]) +} + +func TestTrigger_DispatchesWorkflowAndReturnsRunID(t *testing.T) { + var capturedMethod string + var capturedPath string + var capturedBody []byte + + r := newTestRunner(t, http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + capturedMethod = req.Method + capturedPath = req.URL.String() + capturedBody, _ = io.ReadAll(req.Body) + _ = json.NewEncoder(w).Encode(dispatchWorkflowResponse{ + WorkflowRunID: 42, + RunURL: "https://api.github.com/repos/uber/submitqueue/actions/runs/42", + HTMLURL: "https://github.com/uber/submitqueue/actions/runs/42", + }) + })) + + base := []entity.Change{{URIs: []string{"github://org/repo/pull/1/aaa111"}}} + head := []entity.Change{{URIs: []string{"github://org/repo/pull/2/bbb222"}}} + metadata := entity.BuildMetadata{"requester": "alice"} + + id, err := r.Trigger(context.Background(), base, head, metadata) + require.NoError(t, err) + assert.Equal(t, "42", id.ID) + + assert.Equal(t, http.MethodPost, capturedMethod) + assert.Equal(t, "/repos/uber/submitqueue/actions/workflows/submitqueue-ci.yml/dispatches", capturedPath) + + var req dispatchWorkflowRequest + require.NoError(t, json.Unmarshal(capturedBody, &req)) + assert.Equal(t, "main", req.Ref) + assert.True(t, req.ReturnRunDetails) + assert.NotContains(t, req.Inputs, "sq_build_id") + assert.Equal(t, `["github://org/repo/pull/1/aaa111"]`, req.Inputs[InputKeyBaseURIs]) + assert.Equal(t, `["github://org/repo/pull/2/bbb222"]`, req.Inputs[InputKeyHeadURIs]) + assert.Equal(t, "my-queue", req.Inputs[InputKeyQueue]) + assert.Equal(t, "value", req.Inputs["custom"]) + + var gotMetadata entity.BuildMetadata + require.NoError(t, json.Unmarshal([]byte(req.Inputs[InputKeyMetadata]), &gotMetadata)) + assert.Equal(t, metadata, gotMetadata) +} + +func TestTrigger_EmptyBaseProducesJSONArray(t *testing.T) { + var capturedBody []byte + r := newTestRunner(t, http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + capturedBody, _ = io.ReadAll(req.Body) + _ = json.NewEncoder(w).Encode(dispatchWorkflowResponse{WorkflowRunID: 7}) + })) + + _, err := r.Trigger(context.Background(), nil, []entity.Change{{URIs: []string{"u"}}}, nil) + require.NoError(t, err) + + var req dispatchWorkflowRequest + require.NoError(t, json.Unmarshal(capturedBody, &req)) + assert.Equal(t, "[]", req.Inputs[InputKeyBaseURIs]) + assert.NotContains(t, req.Inputs, InputKeyMetadata) +} + +func TestTrigger_ErrorsWhenDispatchResponseHasNoRunID(t *testing.T) { + r := newTestRunner(t, http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + _ = json.NewEncoder(w).Encode(dispatchWorkflowResponse{}) + })) + + _, err := r.Trigger(context.Background(), nil, nil, nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "response missing workflow_run_id") +} + +func TestStatus_GetsRunAndMapsState(t *testing.T) { + r := newTestRunner(t, http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + assert.Equal(t, http.MethodGet, req.Method) + assert.Equal(t, "/repos/uber/submitqueue/actions/runs/42", req.URL.Path) + _ = json.NewEncoder(w).Encode(workflowRun{ + ID: 42, + DisplayTitle: "SubmitQueue gha-trace", + Status: "in_progress", + HTMLURL: "https://github.com/uber/submitqueue/actions/runs/42", + RunAttempt: 2, + Event: "workflow_dispatch", + HeadBranch: "main", + CreatedAt: "2026-06-05T00:00:00Z", + }) + })) + + status, meta, err := r.Status(context.Background(), entity.BuildID{ID: "42"}) + require.NoError(t, err) + assert.Equal(t, entity.BuildStatusRunning, status) + assert.Equal(t, "42", meta["github_run_id"]) + assert.Equal(t, "2", meta["github_run_attempt"]) + assert.Equal(t, "https://github.com/uber/submitqueue/actions/runs/42", meta["url"]) + assert.Equal(t, "main", meta["github_head_branch"]) +} + +func TestStatus_MalformedBuildIDErrors(t *testing.T) { + r := newTestRunner(t, http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + t.Fatalf("unexpected request: %s %s", req.Method, req.URL.Path) + })) + + _, _, err := r.Status(context.Background(), entity.BuildID{ID: "gha-old-id"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "malformed build ID") +} + +func TestCancel_CancelsRun(t *testing.T) { + var cancelledRunID int64 + r := newTestRunner(t, http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + if req.Method != http.MethodPost || !strings.HasSuffix(req.URL.Path, "/cancel") { + t.Fatalf("unexpected request: %s %s", req.Method, req.URL.Path) + } + parts := strings.Split(req.URL.Path, "/") + id, err := strconv.ParseInt(parts[len(parts)-2], 10, 64) + require.NoError(t, err) + cancelledRunID = id + w.WriteHeader(http.StatusAccepted) + })) + + require.NoError(t, r.Cancel(context.Background(), entity.BuildID{ID: "99"})) + assert.Equal(t, int64(99), cancelledRunID) +} + +func TestCancel_MalformedBuildIDErrors(t *testing.T) { + r := newTestRunner(t, http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + t.Fatalf("unexpected request: %s %s", req.Method, req.URL.Path) + })) + + err := r.Cancel(context.Background(), entity.BuildID{ID: "gha-old-id"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "malformed build ID") +} + +func TestMapRunStatus(t *testing.T) { + tests := []struct { + name string + status string + conclusion string + want entity.BuildStatus + }{ + {name: "queued", status: "queued", want: entity.BuildStatusAccepted}, + {name: "pending", status: "pending", want: entity.BuildStatusAccepted}, + {name: "requested", status: "requested", want: entity.BuildStatusAccepted}, + {name: "waiting", status: "waiting", want: entity.BuildStatusAccepted}, + {name: "running", status: "in_progress", want: entity.BuildStatusRunning}, + {name: "success", status: "completed", conclusion: "success", want: entity.BuildStatusSucceeded}, + {name: "failure", status: "completed", conclusion: "failure", want: entity.BuildStatusFailed}, + {name: "timed out", status: "completed", conclusion: "timed_out", want: entity.BuildStatusFailed}, + {name: "cancelled", status: "completed", conclusion: "cancelled", want: entity.BuildStatusCancelled}, + {name: "completed missing conclusion", status: "completed", want: entity.BuildStatusUnknown}, + {name: "unknown", status: "some_future_state", want: entity.BuildStatusUnknown}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, mapRunStatus(tt.status, tt.conclusion)) + }) + } +} From 453829cf68b876570393d6441f688fec7358c305 Mon Sep 17 00:00:00 2001 From: Albert Wu Date: Fri, 5 Jun 2026 15:53:10 -0700 Subject: [PATCH 2/2] refactor(buildrunner): drop githubactions factory; fix README backend list Address review feedback on #208: - Remove NewFactory/FactoryParams from the githubactions package. Per-queue factory wiring belongs in the integrator/wiring layer (matching the buildkite backend); the extension package keeps only the impl + Params. Reworked the corresponding test to construct the runner via NewBuildRunner. - README: replace the retired `noop` backend bullet with `fake`. Co-Authored-By: Claude Opus 4.8 (1M context) --- submitqueue/extension/buildrunner/README.md | 2 +- .../githubactions/githubactions.go | 43 ------------------- .../githubactions/githubactions_test.go | 7 ++- 3 files changed, 4 insertions(+), 48 deletions(-) diff --git a/submitqueue/extension/buildrunner/README.md b/submitqueue/extension/buildrunner/README.md index d6ea2f80..29d8f512 100644 --- a/submitqueue/extension/buildrunner/README.md +++ b/submitqueue/extension/buildrunner/README.md @@ -13,7 +13,7 @@ See [`doc/rfc/submitqueue/build-runner.md`](../../../doc/rfc/submitqueue/build-r ## Backends -- `noop`: local-development backend that immediately succeeds every build. +- `fake`: local-development backend; every build succeeds unless a head change URI carries a failure marker (see `fake` package doc). - `githubactions`: proof-of-architecture backend that dispatches a GitHub Actions workflow. See [`githubactions/README.md`](githubactions/README.md) for the workflow inputs and example orchestrator environment variables. diff --git a/submitqueue/extension/buildrunner/githubactions/githubactions.go b/submitqueue/extension/buildrunner/githubactions/githubactions.go index 242a6752..d7dff659 100644 --- a/submitqueue/extension/buildrunner/githubactions/githubactions.go +++ b/submitqueue/extension/buildrunner/githubactions/githubactions.go @@ -75,49 +75,6 @@ type Params struct { ExtraInputs map[string]string } -// FactoryParams holds shared dependencies for a GitHub Actions BuildRunner -// factory. For each queue, For returns a runner with Config.QueueName set to -// that queue. -type FactoryParams struct { - HTTPClient *http.Client - Logger *zap.SugaredLogger - Owner string - Repo string - WorkflowID string - Ref string - ExtraInputs map[string]string -} - -type factory struct { - params FactoryParams -} - -var _ buildrunner.Factory = (*factory)(nil) - -// NewFactory constructs a queue-aware GitHub Actions BuildRunner factory. -func NewFactory(params FactoryParams) (buildrunner.Factory, error) { - if err := validateConfig(params.HTTPClient, params.Logger, params.Owner, params.Repo, params.WorkflowID); err != nil { - return nil, err - } - if params.Ref == "" { - params.Ref = defaultRef - } - return &factory{params: params}, nil -} - -func (f *factory) For(cfg buildrunner.Config) (buildrunner.BuildRunner, error) { - return NewBuildRunner(Params{ - Config: cfg, - HTTPClient: f.params.HTTPClient, - Logger: f.params.Logger, - Owner: f.params.Owner, - Repo: f.params.Repo, - WorkflowID: f.params.WorkflowID, - Ref: f.params.Ref, - ExtraInputs: f.params.ExtraInputs, - }) -} - type runner struct { cfg buildrunner.Config ref string diff --git a/submitqueue/extension/buildrunner/githubactions/githubactions_test.go b/submitqueue/extension/buildrunner/githubactions/githubactions_test.go index 0d3217e6..80502f39 100644 --- a/submitqueue/extension/buildrunner/githubactions/githubactions_test.go +++ b/submitqueue/extension/buildrunner/githubactions/githubactions_test.go @@ -61,8 +61,9 @@ func TestNewBuildRunner_ValidatesConfig(t *testing.T) { assert.Contains(t, err.Error(), "http client is required") } -func TestNewFactory_ForBuildsQueueBoundRunner(t *testing.T) { - f, err := NewFactory(FactoryParams{ +func TestNewBuildRunner_BindsQueueConfigAndExtraInputs(t *testing.T) { + br, err := NewBuildRunner(Params{ + Config: buildrunner.Config{QueueName: "queue-a"}, HTTPClient: http.DefaultClient, Logger: zap.NewNop().Sugar(), Owner: "uber", @@ -73,8 +74,6 @@ func TestNewFactory_ForBuildsQueueBoundRunner(t *testing.T) { }) require.NoError(t, err) - br, err := f.For(buildrunner.Config{QueueName: "queue-a"}) - require.NoError(t, err) r := br.(*runner) assert.Equal(t, "queue-a", r.cfg.QueueName) assert.Equal(t, "ubuntu-latest", r.extraInputs["runner"])