From f0d1f084fd8a92c6da9ba1b1b1dd29b9f62630b7 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 1 May 2026 15:05:53 -0700 Subject: [PATCH] better buildkite schema --- README.md | 191 +++++++++++++++++++- go.mod | 2 +- internal/buildkite/buildkite.go | 33 ++-- internal/buildkite/buildkite_test.go | 16 +- main.go | 17 +- provider_buildkite.go | 256 +++++++++++++++++++-------- provider_buildkite_test.go | 128 +++++++++++++- 7 files changed, 531 insertions(+), 112 deletions(-) diff --git a/README.md b/README.md index 63eb5d6..9014fff 100644 --- a/README.md +++ b/README.md @@ -37,6 +37,9 @@ go run . -addr :8080 ## Configuration +Core configuration controls how tack talks to Tangled. Provider-specific +configuration (e.g. Buildkite) lives in its own section below. + ### Required | Env var | Description | @@ -53,17 +56,191 @@ go run . -addr :8080 | `TACK_JETSTREAM_URL` | Tangled Jetstream WebSocket URL | | `TACK_DEV` | Use `ws://` for knot event-streams (any non-empty value) | -### Buildkite +When no provider is configured, tack runs an in-process fake provider +that's useful for exercising the jetstream → knot → `/events` flow +locally without a real CI account. + +## Buildkite + +[Buildkite](https://buildkite.com) is the primary provider tack +supports today. In Buildkite mode, every Tangled pipeline trigger +fans out into one Buildkite build per workflow on the pipeline that +workflow names; build state flows back to Tangled via Buildkite's +notification webhooks. + +### How it fits together + +``` + sh.tangled.pipeline Buildkite + trigger record ──▶ tack ──▶ Create Build ─┐ + │ + /webhooks/buildkite ◀──── notification ◀─────┘ + │ + ▼ + sh.tangled.pipeline.status (broadcast on /events) +``` + +* **Spawn:** for each workflow on a pipeline trigger, tack POSTs to + `/v2/organizations//pipelines//builds`. Both `` and + `` come from the workflow's YAML body (see + [Configuring your workflows](#configuring-your-workflows)). +* **Track:** tack persists the resulting `(build_uuid → knot, rkey, + workflow)` mapping in its local SQLite store so it can later + resolve incoming webhooks back to the originating Tangled + pipeline. +* **Report:** Buildkite delivers `build.*` events to + `POST /webhooks/buildkite`. tack authenticates each request, + translates the Buildkite state into a Tangled status, and + broadcasts a `sh.tangled.pipeline.status` record on `/events`. + +### Setting up Buildkite + +These steps happen once on the Buildkite side, before tack can talk +to it. + +#### 1. Create one or more pipelines + +Each Tangled workflow targets exactly one Buildkite pipeline by +slug. There's no requirement that pipelines map 1:1 to workflows — +many users point every workflow at a single pipeline whose +`pipeline.yml` does `pipeline upload some-file-${TACK_WORKFLOW}.yml`, +keeping all the per-workflow logic in the repo rather than in +Buildkite's UI. + +In your Buildkite org, **Pipelines → New pipeline**: + +* Repository: any URL (the agent only needs to be able to clone it). +* Steps: a minimal `pipeline upload` is usually enough — tack passes + the workflow name through `$TACK_WORKFLOW` so you can branch on + it. + +Note the pipeline slug from the URL +(`https://buildkite.com//`); your workflow YAML +will reference it. + +#### 2. Create an API access token + +Tack uses a single API token to create builds, list jobs, and fetch +logs. Generate one at + with these scopes: + +| Scope | Used for | +| ------------------- | ------------------------------------------------- | +| `read_organizations`| Sanity-checking the configured org slug | +| `write_builds` | `POST .../builds` when a Tangled trigger arrives | +| `read_builds` | Resolving build → jobs for the `/logs` endpoint | +| `read_build_logs` | Streaming job logs back to the Tangled appview | + +Restrict the token to the specific organization(s) tack will spawn +into. + +#### 3. Configure a notification webhook + +Builds report their state back to tack through Buildkite's +notification service. + +In your Buildkite org, **Settings → Notification Services → Add → +Webhook**: -Setting `TACK_BUILDKITE_TOKEN` enables Buildkite mode; when unset, tack -runs the in-process fake provider for local development. When -Buildkite mode is enabled, every other variable in this section is +* **Webhook URL:** `https:///webhooks/buildkite` +* **Token / Secret:** any high-entropy string. You'll set the same + value in `TACK_BUILDKITE_WEBHOOK_SECRET`. +* **Events:** `build.scheduled`, `build.running`, `build.finished` + (job-level events are ignored). +* **Pipelines:** the pipelines tack will fire builds on. + +Buildkite supports two header schemes for authenticating webhooks; +tack supports both: + +| Header scheme | `TACK_BUILDKITE_WEBHOOK_MODE` | Notes | +| ----------------------- | ----------------------------- | -------------------------------------------- | +| `X-Buildkite-Token` | `token` (default) | Secret is sent verbatim in the header | +| `X-Buildkite-Signature` | `signature` | HMAC-SHA256 of `.`; safer | + +Pick `signature` if the notification setting offers it — it doesn't +expose the secret on the wire. + +### Configuring tack + +Setting `TACK_BUILDKITE_TOKEN` is the master switch that puts tack +into Buildkite mode. The other variables in this section are then required. | Env var | Description | | ------------------------------- | ------------------------------------------------------------------------------ | | `TACK_BUILDKITE_TOKEN` | Buildkite API token (enables Buildkite mode) | -| `TACK_BUILDKITE_ORG` | Buildkite organization slug | -| `TACK_BUILDKITE_PIPELINE` | Buildkite pipeline slug to fire builds on | +| `TACK_BUILDKITE_ORG` | Default Buildkite organization slug (workflows may override via YAML) | | `TACK_BUILDKITE_WEBHOOK_SECRET` | Shared secret for `/webhooks/buildkite` auth | -| `TACK_BUILDKITE_WEBHOOK_MODE` | `token` (default) or `signature` — must match Buildkite's notification setting | +| `TACK_BUILDKITE_WEBHOOK_MODE` | `token` (default) or `signature` — must match the notification service | + +The pipeline a workflow runs against is **not** an environment +variable. It lives inside the workflow YAML so each repo can target +its own pipeline without an operator round-trip. + +### Configuring your workflows + +A Tangled workflow's `raw` body is parsed by tack as YAML. Only +`pipeline` is required — every other field is an optional override +or extension of what the trigger metadata already provides: + +```yaml +# Required: which Buildkite pipeline this workflow fires. +pipeline: my-pipeline-slug + +# Optional: org override. Defaults to TACK_BUILDKITE_ORG. The API +# token must have access to whichever org you target. +org: another-org + +# Optional: human-readable build message (default: "tangled: "). +message: "Custom build message" + +# Optional: pin the commit/branch tack would otherwise derive from +# the trigger. Useful for manual triggers (which carry no commit). +commit: abcdef0123 +branch: main + +# Optional: extra env + meta_data merged on top of tack's defaults +# (see "What tack injects into every build" below). +env: + CUSTOM_VAR: value +meta_data: + custom-key: value + +# Optional: forwarded verbatim to the Buildkite create-build API. +clean_checkout: true +ignore_pipeline_branch_filters: true # default: true +author: + name: "Author Name" + email: "author@example.com" +``` + +When the trigger is a pull request, tack auto-populates Buildkite's +`pull_request_base_branch` from the PR target so step-level branch +filters work without extra config. + +#### What tack injects into every build + +Regardless of what the workflow YAML adds on top, tack always +provides the following so your Buildkite pipeline can recover the +Tangled identity of the build: + +| Channel | Key | Value | +| ----------- | -------------------- | ---------------------------------------- | +| `env` | `TACK_KNOT` | knot hostname the pipeline came from | +| `env` | `TACK_PIPELINE_RKEY` | rkey of the originating pipeline record | +| `env` | `TACK_WORKFLOW` | workflow name (typically a YAML filename) | +| `env` | `TACK_WORKFLOW_RAW` | the workflow's raw YAML body | +| `meta_data` | `tack:knot` | same as `TACK_KNOT` | +| `meta_data` | `tack:pipeline_rkey` | same as `TACK_PIPELINE_RKEY` | +| `meta_data` | `tack:workflow` | same as `TACK_WORKFLOW` | + +A common pattern is for the Buildkite pipeline's root step to do a +`pipeline upload` against a workflow-specific YAML file based on +`$TACK_WORKFLOW`, e.g.: + +```yaml +# Buildkite pipeline.yml +steps: + - label: ":pipeline: dispatch ${TACK_WORKFLOW}" + command: "buildkite-agent pipeline upload .buildkite/${TACK_WORKFLOW}" +``` diff --git a/go.mod b/go.mod index a3e757d..09ddd87 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( github.com/charmbracelet/log v1.0.0 github.com/gorilla/websocket v1.5.4-0.20250319132907-e064f32e3674 github.com/mattn/go-sqlite3 v1.14.44 + go.yaml.in/yaml/v2 v2.4.3 tangled.org/core v1.13.0-alpha ) @@ -50,7 +51,6 @@ require ( github.com/whyrusleeping/cbor-gen v0.3.1 // indirect github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect go.uber.org/atomic v1.11.0 // indirect - go.yaml.in/yaml/v2 v2.4.3 // indirect golang.org/x/crypto v0.48.0 // indirect golang.org/x/exp v0.0.0-20260112195511-716be5621a96 // indirect golang.org/x/net v0.50.0 // indirect diff --git a/internal/buildkite/buildkite.go b/internal/buildkite/buildkite.go index 406bf98..e4c2975 100644 --- a/internal/buildkite/buildkite.go +++ b/internal/buildkite/buildkite.go @@ -39,24 +39,24 @@ var APIBase = "https://api.buildkite.com" // maps it onto its own ErrLogsNotFound for the /logs handler). var ErrNotFound = errors.New("buildkite: not found") -// Client is a thin wrapper around net/http carrying API credentials -// + organization context so call sites don't repeat them. Safe for -// concurrent use; the embedded http.Client is goroutine-safe. +// Client is a thin wrapper around net/http carrying API credentials. +// Organisation slug is *not* held on the client — every call takes +// its target org explicitly so a single client can address multiple +// orgs the token has access to. Safe for concurrent use; the +// embedded http.Client is goroutine-safe. type Client struct { http *http.Client token string - org string } // NewClient builds a Client with sensible defaults. The 30s timeout // covers individual requests, not the whole client lifetime — // long-poll-style endpoints aren't used here, so a generous-but-bounded // per-request timeout is the right default. -func NewClient(token, org string) *Client { +func NewClient(token string) *Client { return &Client{ http: &http.Client{Timeout: 30 * time.Second}, token: token, - org: org, } } @@ -95,16 +95,22 @@ type Build struct { // fields callers actively use are exposed — the upstream API accepts // many more, but we'd just be passing through dead options. // -// IgnorePipelineBranchFilters defaults to false on the wire (omitempty -// elides the zero value); callers that want pipeline-level branch -// filters bypassed should set it to true. +// IgnorePipelineBranchFilters / CleanCheckout default to false on the +// wire (omitempty elides the zero value); callers that want them +// turned on should set the field explicitly. +// +// PullRequestBaseBranch surfaces a Tangled PR trigger's target +// branch so Buildkite step filters keyed on the PR base behave the +// way users expect. type CreateBuildRequest struct { Commit string `json:"commit"` Branch string `json:"branch"` Message string `json:"message,omitempty"` Env map[string]string `json:"env,omitempty"` MetaData map[string]string `json:"meta_data,omitempty"` + CleanCheckout bool `json:"clean_checkout,omitempty"` IgnorePipelineBranchFilters bool `json:"ignore_pipeline_branch_filters,omitempty"` + PullRequestBaseBranch string `json:"pull_request_base_branch,omitempty"` } // CreateBuild fires a build on the named pipeline. Returns the @@ -117,6 +123,7 @@ type CreateBuildRequest struct { // the caller's log. func (c *Client) CreateBuild( ctx context.Context, + org string, pipelineSlug string, req CreateBuildRequest, ) (*Build, error) { @@ -126,7 +133,7 @@ func (c *Client) CreateBuild( } url := fmt.Sprintf("%s/v2/organizations/%s/pipelines/%s/builds", - APIBase, c.org, pipelineSlug, + APIBase, org, pipelineSlug, ) httpReq, err := http.NewRequestWithContext(ctx, http.MethodPost, url, bytes.NewReader(body)) if err != nil { @@ -162,11 +169,12 @@ func (c *Client) CreateBuild( // Returns ErrNotFound when Buildkite responds 404. func (c *Client) GetBuild( ctx context.Context, + org string, pipelineSlug string, buildNumber int64, ) (*Build, error) { url := fmt.Sprintf("%s/v2/organizations/%s/pipelines/%s/builds/%d", - APIBase, c.org, pipelineSlug, buildNumber, + APIBase, org, pipelineSlug, buildNumber, ) httpReq, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) if err != nil { @@ -205,12 +213,13 @@ func (c *Client) GetBuild( // job that hasn't started yet — it has no log to serve). func (c *Client) GetJobLog( ctx context.Context, + org string, pipelineSlug string, buildNumber int64, jobID string, ) (string, error) { url := fmt.Sprintf("%s/v2/organizations/%s/pipelines/%s/builds/%d/jobs/%s/log", - APIBase, c.org, pipelineSlug, buildNumber, jobID, + APIBase, org, pipelineSlug, buildNumber, jobID, ) httpReq, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) if err != nil { diff --git a/internal/buildkite/buildkite_test.go b/internal/buildkite/buildkite_test.go index 5322e34..f92c1b5 100644 --- a/internal/buildkite/buildkite_test.go +++ b/internal/buildkite/buildkite_test.go @@ -121,8 +121,8 @@ func TestClientCreateBuild(t *testing.T) { APIBase = srv.URL defer func() { APIBase = prev }() - c := NewClient("tok", "myorg") - build, err := c.CreateBuild(context.Background(), "mypipe", CreateBuildRequest{ + c := NewClient("tok") + build, err := c.CreateBuild(context.Background(), "myorg", "mypipe", CreateBuildRequest{ Commit: "abc", Branch: "main", MetaData: map[string]string{"k": "v"}, @@ -150,8 +150,8 @@ func TestClientCreateBuildError(t *testing.T) { APIBase = srv.URL defer func() { APIBase = prev }() - c := NewClient("tok", "myorg") - _, err := c.CreateBuild(context.Background(), "mypipe", CreateBuildRequest{}) + c := NewClient("tok") + _, err := c.CreateBuild(context.Background(), "myorg", "mypipe", CreateBuildRequest{}) if err == nil { t.Fatal("expected error") } @@ -180,8 +180,8 @@ func TestClientGetJobLog(t *testing.T) { APIBase = srv.URL defer func() { APIBase = prev }() - body, err := NewClient("tok", "myorg"). - GetJobLog(context.Background(), "mypipe", 7, "job-1") + body, err := NewClient("tok"). + GetJobLog(context.Background(), "myorg", "mypipe", 7, "job-1") if err != nil { t.Fatalf("GetJobLog: %v", err) } @@ -198,8 +198,8 @@ func TestClientGetJobLog(t *testing.T) { APIBase = srv.URL defer func() { APIBase = prev }() - _, err := NewClient("tok", "myorg"). - GetJobLog(context.Background(), "mypipe", 7, "job-1") + _, err := NewClient("tok"). + GetJobLog(context.Background(), "myorg", "mypipe", 7, "job-1") if err != ErrNotFound { t.Fatalf("err = %v; want ErrNotFound", err) } diff --git a/main.go b/main.go index c9e88e3..5d6adb3 100644 --- a/main.go +++ b/main.go @@ -37,9 +37,13 @@ type config struct { // (useful for local development against a real Tangled // jetstream); when set, the other Buildkite fields are // required and tack will refuse to start without them. + // + // BuildkiteOrg is the *default* org used when a workflow YAML + // doesn't specify one of its own. The pipeline a workflow runs + // against is no longer global — it's pulled from the workflow + // body itself (see workflowConfig in provider_buildkite.go). BuildkiteToken string BuildkiteOrg string - BuildkitePipeline string BuildkiteWebhookSecret string BuildkiteWebhookMode buildkite.WebhookMode } @@ -54,7 +58,6 @@ func loadConfig() (config, error) { Dev: os.Getenv("TACK_DEV") != "", BuildkiteToken: os.Getenv("TACK_BUILDKITE_TOKEN"), BuildkiteOrg: os.Getenv("TACK_BUILDKITE_ORG"), - BuildkitePipeline: os.Getenv("TACK_BUILDKITE_PIPELINE"), BuildkiteWebhookSecret: os.Getenv("TACK_BUILDKITE_WEBHOOK_SECRET"), BuildkiteWebhookMode: buildkite.WebhookMode( envOr("TACK_BUILDKITE_WEBHOOK_MODE", string(buildkite.WebhookModeToken)), @@ -85,9 +88,6 @@ func loadConfig() (config, error) { if cfg.BuildkiteOrg == "" { return cfg, errors.New("TACK_BUILDKITE_ORG is required when TACK_BUILDKITE_TOKEN is set") } - if cfg.BuildkitePipeline == "" { - return cfg, errors.New("TACK_BUILDKITE_PIPELINE is required when TACK_BUILDKITE_TOKEN is set") - } if cfg.BuildkiteWebhookSecret == "" { return cfg, errors.New("TACK_BUILDKITE_WEBHOOK_SECRET is required when TACK_BUILDKITE_TOKEN is set") } @@ -176,16 +176,15 @@ func main() { if cfg.BuildkiteToken != "" { bkProvider = newBuildkiteProvider( br, st, - buildkite.NewClient(cfg.BuildkiteToken, cfg.BuildkiteOrg), - cfg.BuildkitePipeline, + buildkite.NewClient(cfg.BuildkiteToken), + cfg.BuildkiteOrg, cfg.BuildkiteWebhookSecret, cfg.BuildkiteWebhookMode, logger, ) provider = bkProvider logger.Info("buildkite provider enabled", - "org", cfg.BuildkiteOrg, - "pipeline", cfg.BuildkitePipeline, + "default_org", cfg.BuildkiteOrg, "webhook_mode", cfg.BuildkiteWebhookMode, ) } else { diff --git a/provider_buildkite.go b/provider_buildkite.go index 3602608..8dd0c9d 100644 --- a/provider_buildkite.go +++ b/provider_buildkite.go @@ -9,14 +9,12 @@ package main // Spawn time and publishes a sh.tangled.pipeline.status record on // the in-process broker. // -// Only one Buildkite pipeline is used per spindle (TACK_BUILDKITE_PIPELINE). -// Every Tangled workflow runs as a build on that single pipeline, with -// the workflow identity plumbed through env + meta_data. The operator -// configures their Buildkite pipeline to read those env vars and -// dispatch accordingly (e.g. via `pipeline upload`). Mapping every -// Tangled workflow to its own Buildkite pipeline would force operators -// to provision Buildkite resources for each workflow file in every -// repo that points at the spindle — friction we don't want to impose. +// The Buildkite *pipeline slug* a workflow targets is carried inside +// the workflow's YAML body (Pipeline_Workflow.Raw), not configured +// globally on the spindle. That keeps tack a thin translator: the +// repo author decides which Buildkite pipeline runs each Tangled +// workflow without an operator round-trip. See workflowConfig below +// for the supported YAML schema. import ( "context" @@ -28,6 +26,7 @@ import ( "strings" "time" + "go.yaml.in/yaml/v2" "tangled.org/core/api/tangled" "github.com/mitchellh/tack/internal/buildkite" @@ -44,6 +43,67 @@ const ( bkMetaWorkflow = "tack:workflow" ) +// workflowConfig is the tack-flavoured schema we expect inside each +// Tangled workflow's Raw YAML body. Only the Buildkite `pipeline` +// slug is required; everything else is optional. Fields nest under +// `tack: { buildkite: ... }` so the workflow YAML can grow other +// top-level keys (Tangled's own scheduling fields, future provider +// blocks) without colliding with our namespace. +// +// Fields map onto the Buildkite REST "Create a build" request +// properties documented at +// https://buildkite.com/docs/apis/rest-api/builds#create-a-build +// (see also the comment block on buildkite.CreateBuildRequest). We +// expose only the small subset users genuinely need to override — +// trigger metadata supplies commit/branch, and tack supplies the +// identity env+meta the webhook handler relies on, so there's no +// reason to let users re-specify those. +type workflowConfig struct { + Tack tackConfig `yaml:"tack"` +} + +// tackConfig is the per-provider block under the top-level `tack:` +// key. Right now the only nested provider is Buildkite. +type tackConfig struct { + Buildkite buildkiteConfig `yaml:"buildkite"` +} + +// buildkiteConfig is the Buildkite-specific subset of workflowConfig. +// +// `org` lets a workflow target a Buildkite organisation other than +// the spindle's default — useful when one tack instance fronts +// multiple orgs. The configured API token must have access to that +// org or the build creation request will 401/403; we surface that +// error verbatim rather than guessing. +// +// `clean_checkout` is forwarded verbatim to Buildkite. CleanCheckout +// is a *bool so omitting it leaves Buildkite's own default in place +// instead of always shipping `false`. +type buildkiteConfig struct { + Pipeline string `yaml:"pipeline"` + Org string `yaml:"org"` + CleanCheckout *bool `yaml:"clean_checkout"` +} + +// parseWorkflowConfig decodes a workflow YAML body into workflowConfig. +// An empty body is treated as a structural error so spawnWorkflow can +// short-circuit cleanly: a workflow with no body has nothing for tack +// to do anyway. +func parseWorkflowConfig(raw string) (*buildkiteConfig, error) { + if strings.TrimSpace(raw) == "" { + return nil, errors.New("workflow body is empty") + } + var cfg workflowConfig + if err := yaml.Unmarshal([]byte(raw), &cfg); err != nil { + return nil, fmt.Errorf("parse workflow yaml: %w", err) + } + bk := cfg.Tack.Buildkite + if bk.Pipeline == "" { + return nil, errors.New("workflow yaml: `tack.buildkite.pipeline` is required") + } + return &bk, nil +} + // buildkiteProvider implements Provider. // // webhookSecret + webhookMode live on the provider rather than on @@ -51,12 +111,16 @@ const ( // "everything Buildkite-y": colocating the auth knob with the API // client and the state translator keeps configuration drift to one // place and makes the http.go side pure transport. +// +// defaultOrg is the Buildkite organisation the configured API token +// belongs to. Workflows may opt into a different org via their YAML +// `org` field; the API token then needs to be authorised against it. type buildkiteProvider struct { br *broker st *store log *slog.Logger client *buildkite.Client - pipelineSlug string + defaultOrg string webhookSecret string webhookMode buildkite.WebhookMode } @@ -65,15 +129,15 @@ type buildkiteProvider struct { var _ Provider = (*buildkiteProvider)(nil) // newBuildkiteProvider wires a provider to its Buildkite client and -// to the broker it publishes pipeline.status records on. pipelineSlug -// is the Buildkite pipeline that all builds get fired on (see file -// header for why there's only one). webhookSecret/webhookMode govern -// inbound /webhooks/buildkite request authentication. +// to the broker it publishes pipeline.status records on. defaultOrg +// is the org the API token authenticates against and the org used +// when a workflow doesn't specify its own. webhookSecret/webhookMode +// govern inbound /webhooks/buildkite request authentication. func newBuildkiteProvider( br *broker, st *store, client *buildkite.Client, - pipelineSlug string, + defaultOrg string, webhookSecret string, webhookMode buildkite.WebhookMode, log *slog.Logger, @@ -83,7 +147,7 @@ func newBuildkiteProvider( st: st, log: log.With("component", "provider", "kind", "buildkite"), client: client, - pipelineSlug: pipelineSlug, + defaultOrg: defaultOrg, webhookSecret: webhookSecret, webhookMode: webhookMode, } @@ -111,10 +175,10 @@ func (p *buildkiteProvider) VerifyWebhook(headers http.Header, body []byte) erro } // Spawn satisfies Provider. For each workflow it fires a separate -// Buildkite build off the configured pipeline so each workflow gets -// its own status timeline. The actual API call runs on a goroutine — -// CreateBuild is one HTTP round-trip, but we still want Spawn to be -// non-blocking per the interface contract. +// Buildkite build off the pipeline named in that workflow's YAML so +// each workflow gets its own status timeline. The actual API call +// runs on a goroutine — CreateBuild is one HTTP round-trip, but we +// still want Spawn to be non-blocking per the interface contract. // // On a successful create we persist the build UUID → (knot, rkey, // workflow) mapping and publish a "pending" pipeline.status so the @@ -134,26 +198,12 @@ func (p *buildkiteProvider) Spawn( return } - // Derive build inputs once. Every workflow on this trigger - // targets the same commit/branch — only the workflow name - // varies between the per-workflow goroutines below. - commit, branch := triggerCommitAndBranch(trigger) - if commit == "" { - // Buildkite's create-build API requires a commit; we'd - // rather log loudly and skip than fire builds on "HEAD" - // and silently get whatever main happens to look like. - p.log.Error("trigger has no commit; refusing to spawn", - "knot", knot, "rkey", pipelineRkey, - ) - return - } - for _, wf := range workflows { if wf == nil || wf.Name == "" { continue } wf := wf - go p.spawnWorkflow(ctx, knot, pipelineRkey, commit, branch, wf) + go p.spawnWorkflow(ctx, knot, pipelineRkey, trigger, wf) } } @@ -166,8 +216,7 @@ func (p *buildkiteProvider) spawnWorkflow( ctx context.Context, knot string, pipelineRkey string, - commit string, - branch string, + trigger *tangled.Pipeline_TriggerMetadata, wf *tangled.Pipeline_Workflow, ) { logger := p.log.With( @@ -176,38 +225,44 @@ func (p *buildkiteProvider) spawnWorkflow( "workflow", wf.Name, ) - pipelineURI := pipelineATURI(knot, pipelineRkey) - meta := map[string]string{ - bkMetaKnot: knot, - bkMetaPipelineRkey: pipelineRkey, - bkMetaWorkflow: wf.Name, + cfg, err := parseWorkflowConfig(wf.Raw) + if err != nil { + // Bad workflow YAML is a user-facing config error: log it + // loudly and skip. Firing a build off some default would + // be more confusing than doing nothing. + logger.Error("invalid workflow config; refusing to spawn", "err", err) + return } - env := envFromTuple(knot, pipelineRkey, wf) + logger = logger.With("pipeline", cfg.Pipeline) - req := buildkite.CreateBuildRequest{ - Commit: commit, - Branch: branch, - Message: fmt.Sprintf("tangled: %s", wf.Name), - Env: env, - MetaData: meta, - IgnorePipelineBranchFilters: true, + req, err := p.buildCreateRequest(cfg, trigger, knot, pipelineRkey, wf) + if err != nil { + logger.Error("build create request", "err", err) + return } - build, err := p.client.CreateBuild(ctx, p.pipelineSlug, req) + org := cfg.Org + if org == "" { + org = p.defaultOrg + } + + build, err := p.client.CreateBuild(ctx, org, cfg.Pipeline, req) if err != nil { - logger.Error("create buildkite build", "err", err) + logger.Error("create buildkite build", "err", err, "org", org) return } logger.Info("buildkite build created", "build_uuid", build.ID, "build_number", build.Number, "web_url", build.WebURL, + "org", org, ) + pipelineURI := pipelineATURI(knot, pipelineRkey) if err := p.st.InsertBuildkiteBuild(ctx, BuildkiteBuildRef{ BuildUUID: build.ID, BuildNumber: build.Number, - PipelineSlug: p.pipelineSlug, + PipelineSlug: cfg.Pipeline, Knot: knot, PipelineRkey: pipelineRkey, Workflow: wf.Name, @@ -234,6 +289,66 @@ func (p *buildkiteProvider) spawnWorkflow( } } +// buildCreateRequest folds the parsed workflow config and the +// Tangled trigger metadata into a single Buildkite create-build +// payload. Trigger metadata supplies commit/branch; the workflow +// YAML supplies the Buildkite routing knobs (pipeline/org) and the +// small handful of build options we expose. +// +// `ignore_pipeline_branch_filters` is hard-coded to true: Tangled +// refs frequently don't match arbitrary Buildkite pipeline branch +// filters, and a build silently dropped at create time is a worse +// failure mode than running one we shouldn't have. Users wanting +// the filter back are expected to drop the filter on the Buildkite +// pipeline itself. +// +// Returns an error when the trigger lacks a commit — Buildkite's +// API requires one and we'd rather log+skip than fire a build that +// resolves to "whatever main happens to be". +func (p *buildkiteProvider) buildCreateRequest( + cfg *buildkiteConfig, + trigger *tangled.Pipeline_TriggerMetadata, + knot, pipelineRkey string, + wf *tangled.Pipeline_Workflow, +) (buildkite.CreateBuildRequest, error) { + commit, branch := triggerCommitAndBranch(trigger) + if commit == "" { + return buildkite.CreateBuildRequest{}, errors.New( + "trigger has no commit", + ) + } + + cleanCheckout := false + if cfg.CleanCheckout != nil { + cleanCheckout = *cfg.CleanCheckout + } + + req := buildkite.CreateBuildRequest{ + Commit: commit, + Branch: branch, + Message: fmt.Sprintf("tangled: %s", wf.Name), + Env: envFromTuple(knot, pipelineRkey, wf), + MetaData: map[string]string{ + bkMetaKnot: knot, + bkMetaPipelineRkey: pipelineRkey, + bkMetaWorkflow: wf.Name, + }, + CleanCheckout: cleanCheckout, + IgnorePipelineBranchFilters: true, + } + + // Auto-populate Buildkite's PR fields from the Tangled PR + // trigger when present. Buildkite doesn't get a PR number from + // us (Tangled doesn't surface one through the trigger), but + // the base branch alone is enough for `pull_request_base_branch`- + // gated step filters to work. + if trigger != nil && trigger.PullRequest != nil { + req.PullRequestBaseBranch = trigger.PullRequest.TargetBranch + } + + return req, nil +} + // Logs satisfies Provider. We resolve the (knot, rkey, workflow) // tuple to a Buildkite build via the store, fetch the current jobs // list, then drain each job's plain-text log into the channel as one @@ -256,44 +371,45 @@ func (p *buildkiteProvider) Logs( ) (<-chan LogLine, error) { ref, err := p.st.LookupBuildkiteBuildByTuple(ctx, knot, pipelineRkey, workflow) if err != nil { - return nil, fmt.Errorf("lookup build for logs: %w", err) + return nil, fmt.Errorf("lookup build mapping: %w", err) } if ref == nil { return nil, ErrLogsNotFound } - // Fresh fetch so we get the current job set, not whatever was - // returned at create time (when most jobs are still nil). The - // upstream's not-found is mapped to the Provider-shaped one - // here because the /logs handler only knows about ErrLogsNotFound. - build, err := p.client.GetBuild(ctx, ref.PipelineSlug, ref.BuildNumber) + // Resolve the org against which we should pull jobs/logs. We + // don't persist it on BuildkiteBuildRef today (the slug + token + // have always been enough); fall back to the provider default, + // which is correct for the common single-org install. A + // future migration can add a column when multi-org installs + // need it. + org := p.defaultOrg + + build, err := p.client.GetBuild(ctx, org, ref.PipelineSlug, ref.BuildNumber) if err != nil { if errors.Is(err, buildkite.ErrNotFound) { return nil, ErrLogsNotFound } - return nil, fmt.Errorf("get build for logs: %w", err) + return nil, fmt.Errorf("get build: %w", err) } - out := make(chan LogLine, 64) + out := make(chan LogLine, 32) go func() { defer close(out) stepID := 0 for _, job := range build.Jobs { - // Only "script" jobs have agent-produced logs. - // Waiter / manual / trigger jobs have no body to - // fetch; skip them so we don't hit Buildkite with - // 404-bound requests. if job.Type != "" && job.Type != "script" { + // Skip non-script jobs (waiter, manual, + // trigger). They have no log to fetch and + // surfacing empty steps just clutters the + // appview. continue } - name := job.Name if name == "" { - name = job.ID + name = fmt.Sprintf("job %s", job.ID) } - // Job-level start frame so the appview can bound - // timing per job. if !sendLine(ctx, out, LogLine{ Kind: LogKindControl, Time: time.Now(), @@ -304,7 +420,7 @@ func (p *buildkiteProvider) Logs( return } - body, err := p.client.GetJobLog(ctx, ref.PipelineSlug, ref.BuildNumber, job.ID) + body, err := p.client.GetJobLog(ctx, org, ref.PipelineSlug, ref.BuildNumber, job.ID) if err != nil { p.log.Debug("fetch job log", "err", err, @@ -512,8 +628,6 @@ func triggerCommitAndBranch(trigger *tangled.Pipeline_TriggerMetadata) (string, return trigger.Push.NewSha, refToBranch(trigger.Push.Ref) case trigger.PullRequest != nil: // PRs build the source commit on the source branch. - // Buildkite's pipeline can opt into PR-aware behaviour - // via pull_request_id (not currently plumbed through). return trigger.PullRequest.SourceSha, trigger.PullRequest.SourceBranch default: // Manual triggers and any future kinds: fall back to the diff --git a/provider_buildkite_test.go b/provider_buildkite_test.go index 3acdaa5..d46607a 100644 --- a/provider_buildkite_test.go +++ b/provider_buildkite_test.go @@ -47,8 +47,8 @@ func newBuildkiteTestProvider( logger := slog.Default() p := newBuildkiteProvider( br, st, - buildkite.NewClient("tok", "myorg"), - "mypipe", + buildkite.NewClient("tok"), + "myorg", secret, mode, logger, ) @@ -74,7 +74,7 @@ func TestBuildkiteSpawn(t *testing.T) { }, } workflows := []*tangled.Pipeline_Workflow{ - {Name: "test.yml", Raw: "steps:\n - run: true\n"}, + {Name: "test.yml", Raw: "tack:\n buildkite:\n pipeline: mypipe\n"}, } p.Spawn(context.Background(), "knot.example.com", "rkey-1", trigger, workflows) @@ -139,7 +139,8 @@ func TestBuildkiteSpawnNoCommit(t *testing.T) { p.Spawn(context.Background(), "knot.example.com", "rkey-1", &tangled.Pipeline_TriggerMetadata{Manual: &tangled.Pipeline_ManualTriggerData{}}, - []*tangled.Pipeline_Workflow{{Name: "test.yml"}}, + []*tangled.Pipeline_Workflow{{Name: "test.yml", + Raw: "tack:\n buildkite:\n pipeline: mypipe\n"}}, ) // Give any rogue goroutine a moment. @@ -153,6 +154,125 @@ func TestBuildkiteSpawnNoCommit(t *testing.T) { } } +// TestBuildkiteSpawnWorkflowConfig pins the YAML → create-build +// translation: pipeline + org from YAML pick the request URL, +// message/env/meta_data come through, and the trigger's PR target +// branch lands as `pull_request_base_branch`. Together these cover +// the "smuggle Buildkite parameters through workflow YAML" path. +func TestBuildkiteSpawnWorkflowConfig(t *testing.T) { + type captured struct { + path string + body buildkite.CreateBuildRequest + } + gotCh := make(chan captured, 1) + bk := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var body buildkite.CreateBuildRequest + _ = json.NewDecoder(r.Body).Decode(&body) + gotCh <- captured{path: r.URL.Path, body: body} + w.WriteHeader(http.StatusCreated) + _ = json.NewEncoder(w).Encode(buildkite.Build{ID: "uuid-9", Number: 9}) + }) + p, _, _, _ := newBuildkiteTestProvider(t, buildkite.WebhookModeToken, "s", bk) + + raw := strings.Join([]string{ + "pipeline: workflow-pipe", + "org: workflow-org", + "message: smuggled message", + "env:", + " CUSTOM: value", + "meta_data:", + " custom: meta", + "clean_checkout: true", + "author:", + " name: Author", + " email: a@example.com", + }, "\n") + "\n" + + trigger := &tangled.Pipeline_TriggerMetadata{ + PullRequest: &tangled.Pipeline_PullRequestTriggerData{ + SourceSha: "deadbeef", + SourceBranch: "feature", + TargetBranch: "main", + }, + } + + p.Spawn(context.Background(), "knot.example.com", "rkey-x", trigger, + []*tangled.Pipeline_Workflow{{Name: "ci.yml", Raw: raw}}) + + select { + case got := <-gotCh: + // URL must reflect YAML org + pipeline. + if !strings.Contains(got.path, "/organizations/workflow-org/pipelines/workflow-pipe/") { + t.Fatalf("path = %q", got.path) + } + if got.body.Commit != "deadbeef" || got.body.Branch != "feature" { + t.Fatalf("commit/branch = %q/%q", got.body.Commit, got.body.Branch) + } + if got.body.Message != "smuggled message" { + t.Fatalf("message = %q", got.body.Message) + } + if got.body.Env["CUSTOM"] != "value" { + t.Fatalf("env[CUSTOM] missing: %+v", got.body.Env) + } + // tack defaults must still be present (user keys merge, + // don't replace). + if got.body.Env["TACK_WORKFLOW"] != "ci.yml" { + t.Fatalf("env[TACK_WORKFLOW] missing: %+v", got.body.Env) + } + if got.body.MetaData["custom"] != "meta" || + got.body.MetaData[bkMetaWorkflow] != "ci.yml" { + t.Fatalf("meta_data wrong: %+v", got.body.MetaData) + } + if !got.body.CleanCheckout { + t.Fatalf("clean_checkout not set") + } + // IgnorePipelineBranchFilters defaults to true (see + // workflowConfig comment). + if !got.body.IgnorePipelineBranchFilters { + t.Fatalf("ignore_pipeline_branch_filters not defaulted to true") + } + if got.body.PullRequestBaseBranch != "main" { + t.Fatalf("pr base branch = %q; want main", + got.body.PullRequestBaseBranch) + } + if got.body.Author == nil || got.body.Author.Email != "a@example.com" { + t.Fatalf("author = %+v", got.body.Author) + } + case <-time.After(2 * time.Second): + t.Fatal("CreateBuild not called") + } +} + +// TestBuildkiteSpawnInvalidYAML proves a workflow without the +// required `pipeline` field is skipped — no API call, no DB row, no +// status. A misconfigured workflow shouldn't be silently swept onto +// some default pipeline. +func TestBuildkiteSpawnInvalidYAML(t *testing.T) { + called := false + bk := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + called = true + }) + p, st, _, _ := newBuildkiteTestProvider(t, buildkite.WebhookModeToken, "s", bk) + + p.Spawn(context.Background(), "knot.example.com", "rkey-z", + &tangled.Pipeline_TriggerMetadata{ + Push: &tangled.Pipeline_PushTriggerData{NewSha: "abc", Ref: "refs/heads/main"}, + }, + []*tangled.Pipeline_Workflow{ + {Name: "broken.yml", Raw: "steps:\n - run: true\n"}, + }, + ) + + time.Sleep(50 * time.Millisecond) + if called { + t.Fatal("CreateBuild called for workflow missing pipeline") + } + rows, _ := st.EventsAfter(context.Background(), 0) + if len(rows) != 0 { + t.Fatalf("got %d events, want 0", len(rows)) + } +} + // TestBuildkiteHandleWebhook checks the translation pipeline: // recorded build + matching webhook → success status published. func TestBuildkiteHandleWebhook(t *testing.T) { -- 2.51.2