buildkite: persist build org so /logs honours workflow override #3

+16 -7

provider_buildkite.go

··· 259 259 ) 260 260 261 261 pipelineURI := pipelineATURI(knot, pipelineRkey) 262 + // Persist the org we actually used. cfg.Org (not the resolved 263 + // `org`) is intentional: empty means "fall back to defaultOrg 264 + // at read time", which keeps the row meaningful even if the 265 + // provider's defaultOrg changes later — and matches what older 266 + // rows scan as before this column existed. 262 267 if err := p.st.InsertBuildkiteBuild(ctx, BuildkiteBuildRef{ 263 268 BuildUUID: build.ID, 264 269 BuildNumber: build.Number, 265 270 PipelineSlug: cfg.Pipeline, 271 + Org: cfg.Org, 266 272 Knot: knot, 267 273 PipelineRkey: pipelineRkey, 268 274 Workflow: wf.Name, ··· 377 383 return nil, ErrLogsNotFound 378 384 } 379 385 380 - // Resolve the org against which we should pull jobs/logs. We 381 - // don't persist it on BuildkiteBuildRef today (the slug + token 382 - // have always been enough); fall back to the provider default, 383 - // which is correct for the common single-org install. A 384 - // future migration can add a column when multi-org installs 385 - // need it. 386 - org := p.defaultOrg 386 + // Resolve the org against which we should pull jobs/logs. The 387 + // workflow YAML can override the spindle's default at Spawn 388 + // time (tack.buildkite.org), so we honour whatever the row 389 + // recorded; an empty value means the workflow didn't override, 390 + // which is the same as "use defaultOrg" — including for rows 391 + // written before the org column was added. 392 + org := ref.Org 393 + if org == "" { 394 + org = p.defaultOrg 395 + } 387 396 388 397 build, err := p.client.GetBuild(ctx, org, ref.PipelineSlug, ref.BuildNumber) 389 398 if err != nil {

+29 -1

provider_buildkite_test.go

··· 171 171 w.WriteHeader(http.StatusCreated) 172 172 _ = json.NewEncoder(w).Encode(buildkite.Build{ID: "uuid-9", Number: 9}) 173 173 }) 174 - p, _, _, _ := newBuildkiteTestProvider(t, buildkite.WebhookModeToken, "s", bk) 174 + p, st, _, _ := newBuildkiteTestProvider(t, buildkite.WebhookModeToken, "s", bk) 175 175 176 176 raw := strings.Join([]string{ 177 177 "tack:", ··· 223 223 case <-time.After(2 * time.Second): 224 224 t.Fatal("CreateBuild not called") 225 225 } 226 + 227 + // The workflow's `org` override has to round-trip through 228 + // the store: /logs (and any future post-creation API call) 229 + // looks the row up by tuple and re-issues calls against the 230 + // org we recorded here. If we drop it, log retrieval falls 231 + // back to defaultOrg and 404s for cross-org workflows. 232 + deadline := time.Now().Add(2 * time.Second) 233 + var ref *BuildkiteBuildRef 234 + for time.Now().Before(deadline) { 235 + var err error 236 + ref, err = st.LookupBuildkiteBuildByTuple( 237 + context.Background(), 238 + "knot.example.com", "rkey-x", "ci.yml", 239 + ) 240 + if err != nil { 241 + t.Fatalf("lookup: %v", err) 242 + } 243 + if ref != nil { 244 + break 245 + } 246 + time.Sleep(20 * time.Millisecond) 247 + } 248 + if ref == nil { 249 + t.Fatal("buildkite build row not persisted within deadline") 250 + } 251 + if ref.Org != "workflow-org" { 252 + t.Fatalf("ref.Org = %q; want %q", ref.Org, "workflow-org") 253 + } 226 254 } 227 255 228 256 // TestBuildkiteSpawnInvalidYAML proves a workflow without the

+23 -14

store.go

··· 329 329 // and the /logs handler (by knot+rkey+workflow) when an appview 330 330 // client asks for output. 331 331 type BuildkiteBuildRef struct { 332 - BuildUUID string 333 - BuildNumber int64 334 - PipelineSlug string 335 - Knot string 336 - PipelineRkey string 337 - Workflow string 338 - PipelineURI string 332 + BuildUUID string 333 + BuildNumber int64 334 + PipelineSlug string 335 + // Org is the Buildkite organisation slug the build was created 336 + // against. Persisted at Spawn time so /logs and any other 337 + // post-creation API call can target the same org the workflow 338 + // originally chose — see the workflow YAML `tack.buildkite.org` 339 + // override. Empty means "use the provider's default org", which 340 + // is what every row written before the org column existed will 341 + // scan as. 342 + Org string 343 + Knot string 344 + PipelineRkey string 345 + Workflow string 346 + PipelineURI string 339 347 } 340 348 341 349 // InsertBuildkiteBuild records that a Buildkite build was created on ··· 346 354 func (s *store) InsertBuildkiteBuild(ctx context.Context, ref BuildkiteBuildRef) error { 347 355 _, err := s.db.ExecContext(ctx, 348 356 `INSERT INTO buildkite_builds ( 349 - build_uuid, build_number, pipeline_slug, 357 + build_uuid, build_number, pipeline_slug, org, 350 358 knot, pipeline_rkey, workflow, 351 359 pipeline_uri, created_at 352 - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?) 360 + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?) 353 361 ON CONFLICT(build_uuid) DO UPDATE SET 354 362 build_number = excluded.build_number, 355 363 pipeline_slug = excluded.pipeline_slug, 364 + org = excluded.org, 356 365 knot = excluded.knot, 357 366 pipeline_rkey = excluded.pipeline_rkey, 358 367 workflow = excluded.workflow, 359 368 pipeline_uri = excluded.pipeline_uri, 360 369 created_at = excluded.created_at`, 361 - ref.BuildUUID, ref.BuildNumber, ref.PipelineSlug, 370 + ref.BuildUUID, ref.BuildNumber, ref.PipelineSlug, ref.Org, 362 371 ref.Knot, ref.PipelineRkey, ref.Workflow, 363 372 ref.PipelineURI, time.Now().UTC().Format(time.RFC3339Nano), 364 373 ) ··· 376 385 func (s *store) LookupBuildkiteBuildByUUID(ctx context.Context, buildUUID string) (*BuildkiteBuildRef, error) { 377 386 var ref BuildkiteBuildRef 378 387 err := s.db.QueryRowContext(ctx, 379 - `SELECT build_uuid, build_number, pipeline_slug, 388 + `SELECT build_uuid, build_number, pipeline_slug, org, 380 389 knot, pipeline_rkey, workflow, pipeline_uri 381 390 FROM buildkite_builds WHERE build_uuid = ?`, 382 391 buildUUID, 383 392 ).Scan( 384 - &ref.BuildUUID, &ref.BuildNumber, &ref.PipelineSlug, 393 + &ref.BuildUUID, &ref.BuildNumber, &ref.PipelineSlug, &ref.Org, 385 394 &ref.Knot, &ref.PipelineRkey, &ref.Workflow, &ref.PipelineURI, 386 395 ) 387 396 if errors.Is(err, sql.ErrNoRows) { ··· 405 414 func (s *store) LookupBuildkiteBuildByTuple(ctx context.Context, knot, pipelineRkey, workflow string) (*BuildkiteBuildRef, error) { 406 415 var ref BuildkiteBuildRef 407 416 err := s.db.QueryRowContext(ctx, 408 - `SELECT build_uuid, build_number, pipeline_slug, 417 + `SELECT build_uuid, build_number, pipeline_slug, org, 409 418 knot, pipeline_rkey, workflow, pipeline_uri 410 419 FROM buildkite_builds 411 420 WHERE knot = ? AND pipeline_rkey = ? AND workflow = ? ··· 413 422 LIMIT 1`, 414 423 knot, pipelineRkey, workflow, 415 424 ).Scan( 416 - &ref.BuildUUID, &ref.BuildNumber, &ref.PipelineSlug, 425 + &ref.BuildUUID, &ref.BuildNumber, &ref.PipelineSlug, &ref.Org, 417 426 &ref.Knot, &ref.PipelineRkey, &ref.Workflow, &ref.PipelineURI, 418 427 ) 419 428 if errors.Is(err, sql.ErrNoRows) {

+30

store_migrate.go

··· 7 7 import ( 8 8 "context" 9 9 "fmt" 10 + "strings" 10 11 ) 11 12 12 13 // schema is the full set of CREATE statements applied at startup. It is ··· 92 93 -- 93 94 -- The (knot, pipeline_rkey, workflow) index supports the /logs 94 95 -- handler, which only knows that tuple at request time. 96 + -- org is the Buildkite organisation slug the build was created 97 + -- against. A workflow's YAML can override the spindle's default 98 + -- org via tack.buildkite.org, so we persist whatever was used at 99 + -- Spawn time and read it back when fetching jobs/logs. The empty 100 + -- string means "use the provider's defaultOrg" — that's both the 101 + -- usual single-org case and what every row written before this 102 + -- column existed will scan as. 95 103 CREATE TABLE IF NOT EXISTS buildkite_builds ( 96 104 build_uuid TEXT PRIMARY KEY, 97 105 build_number INTEGER NOT NULL, 98 106 pipeline_slug TEXT NOT NULL, 107 + org TEXT NOT NULL DEFAULT '', 99 108 knot TEXT NOT NULL, 100 109 pipeline_rkey TEXT NOT NULL, 101 110 workflow TEXT NOT NULL, ··· 107 116 ` 108 117 109 118 // migrate applies the schema. Safe to call repeatedly. 119 + // 120 + // CREATE TABLE IF NOT EXISTS is enough for fresh databases, but it 121 + // won't widen an already-existing table. Columns added after the 122 + // initial release therefore need a parallel ALTER TABLE step here; 123 + // SQLite has no `ADD COLUMN IF NOT EXISTS`, so we run the ALTER and 124 + // swallow the "duplicate column" error that fires on subsequent 125 + // startups. Anything else is fatal. 110 126 func (s *store) migrate(ctx context.Context) error { 111 127 if _, err := s.db.ExecContext(ctx, schema); err != nil { 112 128 return fmt.Errorf("apply schema: %w", err) 113 129 } 130 + for _, alter := range []string{ 131 + // Persist the Buildkite org each build was created 132 + // against so /logs can target the same org the workflow 133 + // chose. Pre-existing rows scan as empty string, which 134 + // the provider treats as "use defaultOrg". 135 + `ALTER TABLE buildkite_builds ADD COLUMN org TEXT NOT NULL DEFAULT ''`, 136 + } { 137 + if _, err := s.db.ExecContext(ctx, alter); err != nil { 138 + if strings.Contains(err.Error(), "duplicate column name") { 139 + continue 140 + } 141 + return fmt.Errorf("apply alter %q: %w", alter, err) 142 + } 143 + } 114 144 return nil 115 145 }

Configure Feed

Configure Feed

History