Approval gates: a job waits for a person with the role 417184c73f4e
Pinned at oxygenflow c3547908. A job whose pipeline asks for approval
waits (flow_jobs.approval, migration 0070): no runner gets it, placement
and the queue leave it out, until someone with the role it names (and,
with others-only, not the pusher) approves it once the jobs it depends on
passed. The run page shows an Approvals panel with Approve and Reject;
POST /api/v1/repos/{o}/{n}/flow/runs/{n}/jobs/{job}/approve|reject does
the same. Decisions go on the job (who, when), its log and the audit log;
a rejected or expired job (the reaper, from when its dependencies
finished) ends cancelled and the jobs after it are skipped; a rerun asks
again. Test covers waiting, the role, others-only, approve, reject, expiry.
Refs andymccall/oxygenflow#57
- Author
- Andy McCall @andymccall <[email protected]>
- via
- agent
Claude Code <[email protected]> · delegate for changes, until 2026-11-13 19:00Z · key ea5f78160088
- When
- · 2026-10-16 04:04 UTC
- Signature
- ✔ signed by Claude Code
- Hash
417184c73f4e7d597fca5f1efe2c8d662ba7ba2a561a52e6008f6f4e88fd07bb- Depends on
- 070a7b64b368 10708ab8f54d 380996db84e5 42fa95540468 44a3d2ebaddd 4d2e44d79b03 501d8b268dd4 745d225de344 74be97962ed8 961c10bee174 b64e2537d702 bf3d07541368 c4c87f481d05 c889911bec12 cd437c4f5f8d d23464c0ac60 e8fa53694f68
- Depended on by
- b1860d6090ec 0468394734e9 ab4df6aae179 a9f1c0bf403a e97500b70f9a 05f1a85a2842 107eaba52c9d 55673c650986 3b1bc27fe805 5d90e0e045e2 47e374020b59 67be8ac00c24 7e01a7665aa5 b644e1757711 dc59d1b56c3d
- In bookmarks
- api-unknown-fields cancel-landed-runs diffusion-pin-bookmark draft-ready main oh-comment-body-file proposal-list-fast
- In tags
- v0.6.0 v0.6.1 v0.7.0 v0.8.0 v0.9.0 v0.9.1 v0.9.11 v0.9.15 v0.9.16 v0.9.19 v0.9.2 v0.9.20 v0.9.21 v0.9.22 v0.9.23 v0.9.24 v0.9.3 v0.9.4 v0.9.5 v0.9.6 v0.9.7 v0.9.8 v0.9.9
- Edits
- 25 atoms, 1 tree operation, 9 files
Browse the tree at this change
added crates/hub/migrations/0070_flow_approvals.sql +6 −0
| 1 | + | -- Approval gates (oxygenflow#57): a job whose pipeline asks for approval |
|
| 2 | + | -- waits ('waiting') until someone with the role approves it ('approved'); |
|
| 3 | + | -- '' for a job that needs none. Who and when are kept on the job. |
|
| 4 | + | ALTER TABLE flow_jobs ADD COLUMN approval TEXT NOT NULL DEFAULT ''; |
|
| 5 | + | ALTER TABLE flow_jobs ADD COLUMN approved_by TEXT NOT NULL DEFAULT ''; |
|
| 6 | + | ALTER TABLE flow_jobs ADD COLUMN approved_ms INTEGER; |
modified crates/hub/src/api/mod.rs +37 −0
| … | 141 unchanged lines |
||
| 142 | 142 | .route("/repos/{owner}/{name}/flow/runs/{number}/rerun", post(rerun_flow_run)) |
|
| 143 | 143 | .route("/repos/{owner}/{name}/flow/runs/{number}/jobs/{job}/rerun", post(rerun_flow_job)) |
|
| 144 | 144 | .route("/repos/{owner}/{name}/flow/runs/{number}/cancel", post(cancel_flow_run)) |
|
| 145 | + | .route("/repos/{owner}/{name}/flow/runs/{number}/jobs/{job}/approve", post(approve_flow_job)) |
|
| 146 | + | .route("/repos/{owner}/{name}/flow/runs/{number}/jobs/{job}/reject", post(reject_flow_job)) |
|
| 145 | 147 | .route("/repos/{owner}/{name}/checks/required", get(required_checks).put(set_required_checks)) |
|
| 146 | 148 | .route("/repos/{owner}/{name}/checks/{state}", get(checks_for_state)) |
|
| 147 | 149 | .route("/repos/{owner}/{name}/releases", get(releases).post(create_release)) |
|
| … | 2006 unchanged lines |
||
| 2154 | 2156 | Ok(Json(serde_json::json!({ "number": number, "status": "cancelled" }))) |
|
| 2155 | 2157 | } |
|
| 2156 | 2158 | ||
| 2159 | + | /// `POST /repos/{o}/{n}/flow/runs/{number}/jobs/{job}/approve` and |
|
| 2160 | + | /// `.../reject` (oxygenflow#57): the job's approval rules decide who may. |
|
| 2161 | + | async fn approve_flow_job( |
|
| 2162 | + | State(app): State<Shared>, |
|
| 2163 | + | headers: HeaderMap, |
|
| 2164 | + | Path((owner, name, number, job)): Path<(String, String, i64, String)>, |
|
| 2165 | + | ) -> Result<Json<serde_json::Value>, ApiError> { |
|
| 2166 | + | decide_flow_job(&app, &headers, &owner, &name, number, &job, true).await |
|
| 2167 | + | } |
|
| 2168 | + | ||
| 2169 | + | async fn reject_flow_job( |
|
| 2170 | + | State(app): State<Shared>, |
|
| 2171 | + | headers: HeaderMap, |
|
| 2172 | + | Path((owner, name, number, job)): Path<(String, String, i64, String)>, |
|
| 2173 | + | ) -> Result<Json<serde_json::Value>, ApiError> { |
|
| 2174 | + | decide_flow_job(&app, &headers, &owner, &name, number, &job, false).await |
|
| 2175 | + | } |
|
| 2176 | + | ||
| 2177 | + | async fn decide_flow_job( |
|
| 2178 | + | app: &Shared, |
|
| 2179 | + | headers: &HeaderMap, |
|
| 2180 | + | owner: &str, |
|
| 2181 | + | name: &str, |
|
| 2182 | + | number: i64, |
|
| 2183 | + | job: &str, |
|
| 2184 | + | approve: bool, |
|
| 2185 | + | ) -> Result<Json<serde_json::Value>, ApiError> { |
|
| 2186 | + | let c = caller(app, headers).await?; |
|
| 2187 | + | let me = c.require("repo:write")?.clone(); |
|
| 2188 | + | let (_, row, _) = reach(app, &c, owner, name, Access::Read).await?; |
|
| 2189 | + | let run = db::flow::run(&app.db, row.id, number).await?.ok_or_else(|| ApiError(StatusCode::NOT_FOUND, "no such run".into()))?; |
|
| 2190 | + | let said = crate::flow::decide(app, &row, &run, job, &me, approve).await.map_err(|m| ApiError(StatusCode::CONFLICT, m))?; |
|
| 2191 | + | Ok(Json(serde_json::json!({ "number": number, "job": job, "decision": if approve { "approved" } else { "rejected" }, "said": said }))) |
|
| 2192 | + | } |
|
| 2193 | + | ||
| 2157 | 2194 | /// `POST /repos/{o}/{n}/checks`: any `checks:write` (or write) token may |
|
| 2158 | 2195 | /// report; the state hash names what was checked. |
|
| 2159 | 2196 | async fn post_check( |
|
| … | 381 unchanged lines |
modified crates/hub/src/db/flow.rs +54 −6
| … | 96 unchanged lines |
||
| 97 | 97 | pub need_disk_mib: Option<i64>, |
|
| 98 | 98 | /// Why it ran on its runner (oxygenflow#42). |
|
| 99 | 99 | pub placed: String, |
|
| 100 | + | /// `waiting` for a person, `approved`, or empty (oxygenflow#57). |
|
| 101 | + | pub approval: String, |
|
| 102 | + | /// Who approved or rejected it, and when. |
|
| 103 | + | pub approved_by: String, |
|
| 104 | + | pub approved_ms: Option<i64>, |
|
| 100 | 105 | } |
|
| 101 | 106 | ||
| 102 | 107 | impl Job { |
|
| … | 99 unchanged lines |
||
| 202 | 207 | .await?; |
|
| 203 | 208 | for j in jobs { |
|
| 204 | 209 | sqlx::query( |
|
| 205 | - | "INSERT INTO flow_jobs (run_id, name, depth, depends, spec, sandbox, runtime, status, need_disk_mib) VALUES (?, ?, ?, ?, ?, ?, ?, 'queued', ?)", |
|
| 210 | + | "INSERT INTO flow_jobs (run_id, name, depth, depends, spec, sandbox, runtime, status, need_disk_mib, approval) \ |
|
| 211 | + | VALUES (?, ?, ?, ?, ?, ?, ?, 'queued', ?, ?)", |
|
| 206 | 212 | ) |
|
| 207 | 213 | .bind(id) |
|
| 208 | 214 | .bind(&j.name) |
|
| … | 3 unchanged lines |
||
| 212 | 218 | .bind(&j.sandbox) |
|
| 213 | 219 | .bind(&j.runtime) |
|
| 214 | 220 | .bind(j.need_disk_mib) |
|
| 221 | + | .bind(if needs_approval(&j.spec) { "waiting" } else { "" }) |
|
| 215 | 222 | .execute(&mut *tx) |
|
| 216 | 223 | .await?; |
|
| 217 | 224 | } |
|
| … | 1 unchanged lines |
||
| 219 | 226 | Ok(id) |
|
| 220 | 227 | } |
|
| 221 | 228 | ||
| 229 | + | /// Whether a compiled job asks for approval (oxygenflow#57). |
|
| 230 | + | fn needs_approval(spec: &str) -> bool { |
|
| 231 | + | serde_json::from_str::<serde_json::Value>(spec).is_ok_and(|v| !v["approval"].is_null()) |
|
| 232 | + | } |
|
| 233 | + | ||
| 222 | 234 | /// The labels a compiled job and its sandbox ask a runner for (`runs-on`). |
|
| 223 | 235 | fn labels_in(spec: &str, sandbox: &str) -> Vec<String> { |
|
| 224 | 236 | let mut out: Vec<String> = [spec, sandbox] |
|
| … | 60 unchanged lines |
||
| 285 | 297 | sqlx::query_as( |
|
| 286 | 298 | "SELECT j.id AS job_id, j.run_id, r.repo_id, p.owner, p.name, p.visibility, r.number AS run_number, j.name AS job, j.status, j.runner, j.spec, j.sandbox \ |
|
| 287 | 299 | FROM flow_jobs j JOIN flow_runs r ON r.id = j.run_id JOIN repositories p ON p.id = r.repo_id \ |
|
| 288 | - | WHERE j.status IN ('queued', 'running') AND r.status IN ('queued', 'running') \ |
|
| 300 | + | WHERE j.status IN ('queued', 'running') AND j.approval != 'waiting' AND r.status IN ('queued', 'running') \ |
|
| 289 | 301 | ORDER BY CASE j.status WHEN 'running' THEN 0 ELSE 1 END, j.run_id, j.depth, j.id", |
|
| 290 | 302 | ) |
|
| 291 | 303 | .fetch_all(pool) |
|
| … | 201 unchanged lines |
||
| 493 | 505 | let mut tx = super::begin_write(pool).await?; |
|
| 494 | 506 | let queued: Vec<Job> = sqlx::query_as( |
|
| 495 | 507 | "SELECT j.* FROM flow_jobs j JOIN flow_runs r ON r.id = j.run_id JOIN repositories rep ON rep.id = r.repo_id \ |
|
| 496 | - | WHERE j.status = 'queued' AND r.status IN ('queued', 'running') \ |
|
| 508 | + | WHERE j.status = 'queued' AND j.approval != 'waiting' AND r.status IN ('queued', 'running') \ |
|
| 497 | 509 | AND (? = '' OR rep.owner = ?) AND (? IS NULL OR r.repo_id = ?) \ |
|
| 498 | 510 | AND NOT (? = '' AND ? IS NULL AND rep.flow = 'own') \ |
|
| 499 | 511 | ORDER BY j.run_id, j.depth, j.id LIMIT 200", |
|
| … | 170 unchanged lines |
||
| 670 | 682 | pub async fn ready_queued(pool: &Pool) -> sqlx::Result<Vec<(Job, String, i64)>> { |
|
| 671 | 683 | let queued: Vec<Job> = sqlx::query_as( |
|
| 672 | 684 | "SELECT j.* FROM flow_jobs j JOIN flow_runs r ON r.id = j.run_id \ |
|
| 673 | - | WHERE j.status = 'queued' AND r.status IN ('queued', 'running') ORDER BY j.run_id, j.depth, j.id LIMIT 500", |
|
| 685 | + | WHERE j.status = 'queued' AND j.approval != 'waiting' AND r.status IN ('queued', 'running') ORDER BY j.run_id, j.depth, j.id LIMIT 500", |
|
| 674 | 686 | ) |
|
| 675 | 687 | .fetch_all(pool) |
|
| 676 | 688 | .await?; |
|
| … | 156 unchanged lines |
||
| 833 | 845 | .filter(|x| { |
|
| 834 | 846 | x.depends_on() |
|
| 835 | 847 | .iter() |
|
| 836 | - | .any(|d| all.iter().any(|y| &y.name == d && matches!(y.status.as_str(), "failed" | "skipped" | "error"))) |
|
| 848 | + | .any(|d| all.iter().any(|y| &y.name == d && matches!(y.status.as_str(), "failed" | "skipped" | "error" | "cancelled"))) |
|
| 837 | 849 | }) |
|
| 838 | 850 | .map(|x| x.id) |
|
| 839 | 851 | .collect(); |
|
| … | 95 unchanged lines |
||
| 935 | 947 | "UPDATE flow_jobs SET status = 'queued', runner = '', token_hash = '', started_ms = NULL, finished_ms = NULL, |
|
| 936 | 948 | exit_code = NULL, error = '', receipt = '', receipt_hash = '', log_count = 0, also_read = '', receipt_attest = '', |
|
| 937 | 949 | receipt_signer = '', receipt_note = '', runner_id = NULL, waiting = '', placed = '', peak_disk_mib = NULL, |
|
| 938 | - | peak_memory_mib = NULL WHERE id = ?", |
|
| 950 | + | peak_memory_mib = NULL, approval = CASE WHEN approval = '' THEN '' ELSE 'waiting' END, approved_by = '', |
|
| 951 | + | approved_ms = NULL WHERE id = ?", |
|
| 939 | 952 | ) |
|
| 940 | 953 | .bind(id) |
|
| 941 | 954 | .execute(&mut *tx) |
|
| … | 111 unchanged lines |
||
| 1053 | 1066 | .bind(repo_id) |
|
| 1054 | 1067 | .bind(content) |
|
| 1055 | 1068 | .fetch_optional(pool) |
|
| 1069 | + | .await |
|
| 1070 | + | } |
|
| 1071 | + | ||
| 1072 | + | /// Approve job `id` if it is still waiting for a person (oxygenflow#57); |
|
| 1073 | + | /// whether it was. |
|
| 1074 | + | pub async fn approve_job(pool: &Pool, id: i64, who: &str) -> sqlx::Result<bool> { |
|
| 1075 | + | let r = sqlx::query( |
|
| 1076 | + | "UPDATE flow_jobs SET approval = 'approved', approved_by = ?, approved_ms = ? WHERE id = ? AND status = 'queued' AND approval = 'waiting'", |
|
| 1077 | + | ) |
|
| 1078 | + | .bind(who) |
|
| 1079 | + | .bind(now_ms()) |
|
| 1080 | + | .bind(id) |
|
| 1081 | + | .execute(pool) |
|
| 1082 | + | .await?; |
|
| 1083 | + | Ok(r.rows_affected() > 0) |
|
| 1084 | + | } |
|
| 1085 | + | ||
| 1086 | + | /// Note who rejected job `id` (it is then finished as cancelled). |
|
| 1087 | + | pub async fn note_rejection(pool: &Pool, id: i64, who: &str) -> sqlx::Result<()> { |
|
| 1088 | + | sqlx::query("UPDATE flow_jobs SET approved_by = ?, approved_ms = ? WHERE id = ?") |
|
| 1089 | + | .bind(who) |
|
| 1090 | + | .bind(now_ms()) |
|
| 1091 | + | .bind(id) |
|
| 1092 | + | .execute(pool) |
|
| 1093 | + | .await?; |
|
| 1094 | + | Ok(()) |
|
| 1095 | + | } |
|
| 1096 | + | ||
| 1097 | + | /// Jobs waiting for a person, in runs still going. |
|
| 1098 | + | pub async fn waiting_for_approval(pool: &Pool) -> sqlx::Result<Vec<Job>> { |
|
| 1099 | + | sqlx::query_as( |
|
| 1100 | + | "SELECT j.* FROM flow_jobs j JOIN flow_runs r ON r.id = j.run_id \ |
|
| 1101 | + | WHERE j.status = 'queued' AND j.approval = 'waiting' AND r.status IN ('queued', 'running')", |
|
| 1102 | + | ) |
|
| 1103 | + | .fetch_all(pool) |
|
| 1056 | 1104 | .await |
|
| 1057 | 1105 | } |
modified crates/hub/src/flow.rs +103 −0
| … | 157 unchanged lines |
||
| 158 | 158 | report_changed(app, run.id, changed).await |
|
| 159 | 159 | } |
|
| 160 | 160 | ||
| 161 | + | /// Why job `job` of `run` cannot be decided on by `who` (oxygenflow#57), |
|
| 162 | + | /// or its approval: it must be waiting, the jobs it depends on done and |
|
| 163 | + | /// passed, `who` must have the role it names on the repository, and with |
|
| 164 | + | /// others-only must not be the person whose push started the run. |
|
| 165 | + | pub async fn may_decide( |
|
| 166 | + | app: &Shared, |
|
| 167 | + | row: &db::repos::RepoRow, |
|
| 168 | + | run: &fdb::Run, |
|
| 169 | + | job: &fdb::Job, |
|
| 170 | + | who: &db::accounts::Account, |
|
| 171 | + | ) -> Result<flow_core::Approval, String> { |
|
| 172 | + | let spec: flow_core::Job = serde_json::from_str(&job.spec).map_err(|e| format!("stored job: {e}"))?; |
|
| 173 | + | let Some(approval) = spec.approval else { return Err(format!("{} does not wait for approval", job.name)) }; |
|
| 174 | + | if job.status != "queued" || job.approval != "waiting" { |
|
| 175 | + | return Err(format!("{} is not waiting for approval", job.name)); |
|
| 176 | + | } |
|
| 177 | + | let jobs = fdb::jobs(&app.db, run.id).await.map_err(|e| e.to_string())?; |
|
| 178 | + | if let Some(d) = job.depends_on().iter().find(|d| !jobs.iter().any(|j| &j.name == *d && j.status == "passed")) { |
|
| 179 | + | return Err(format!("{} waits for {d} first", job.name)); |
|
| 180 | + | } |
|
| 181 | + | let need = crate::auth::roles::Role::parse(&approval.by).unwrap_or(crate::auth::roles::Role::Maintain); |
|
| 182 | + | let role = crate::auth::roles::role_for(&app.db, Some(who), row).await.map_err(|e| e.to_string())?; |
|
| 183 | + | if role.is_none_or(|r| r < need) { |
|
| 184 | + | return Err(format!("approving {} takes the {} role on {}/{}", job.name, approval.by, row.owner, row.name)); |
|
| 185 | + | } |
|
| 186 | + | if approval.others_only && who.username == run.actor { |
|
| 187 | + | return Err(format!("{} must be approved by someone other than {}, whose push started the run", job.name, run.actor)); |
|
| 188 | + | } |
|
| 189 | + | Ok(approval) |
|
| 190 | + | } |
|
| 191 | + | ||
| 192 | + | /// Approve or reject job `name` of `run` as `who` (oxygenflow#57): the |
|
| 193 | + | /// decision goes on the job, its log and the audit log; a rejected job ends |
|
| 194 | + | /// as cancelled and the jobs after it are skipped. What happened, or why not. |
|
| 195 | + | pub async fn decide( |
|
| 196 | + | app: &Shared, |
|
| 197 | + | row: &db::repos::RepoRow, |
|
| 198 | + | run: &fdb::Run, |
|
| 199 | + | name: &str, |
|
| 200 | + | who: &db::accounts::Account, |
|
| 201 | + | approve: bool, |
|
| 202 | + | ) -> Result<String, String> { |
|
| 203 | + | let jobs = fdb::jobs(&app.db, run.id).await.map_err(|e| e.to_string())?; |
|
| 204 | + | let Some(job) = jobs.into_iter().find(|j| j.name == name) else { return Err(format!("run #{} has no job {name}", run.number)) }; |
|
| 205 | + | may_decide(app, row, run, &job, who).await?; |
|
| 206 | + | let target = format!("{}/{}#{} {name}", row.owner, row.name, run.number); |
|
| 207 | + | let said = if approve { |
|
| 208 | + | if !fdb::approve_job(&app.db, job.id, &who.username).await.map_err(|e| e.to_string())? { |
|
| 209 | + | return Err(format!("{name} was decided on meanwhile")); |
|
| 210 | + | } |
|
| 211 | + | db::accounts::audit(&app.db, Some(who), "flow.approved", &target, "").await.ok(); |
|
| 212 | + | format!("approved by {}", who.username) |
|
| 213 | + | } else { |
|
| 214 | + | fdb::note_rejection(&app.db, job.id, &who.username).await.map_err(|e| e.to_string())?; |
|
| 215 | + | let why = format!("rejected by {}", who.username); |
|
| 216 | + | let changed = |
|
| 217 | + | fdb::finish(&app.db, job.id, fdb::Outcome { status: "cancelled", exit_code: None, error: &why, receipt: "", receipt_hash: "" }) |
|
| 218 | + | .await |
|
| 219 | + | .map_err(|e| e.to_string())?; |
|
| 220 | + | report_changed(app, run.id, changed).await.ok(); |
|
| 221 | + | db::accounts::audit(&app.db, Some(who), "flow.rejected", &target, "").await.ok(); |
|
| 222 | + | why |
|
| 223 | + | }; |
|
| 224 | + | let record = flow_core::api::Record { t: 0, stream: "flow".into(), text: said.clone() }; |
|
| 225 | + | fdb::append_log(&app.db, job.id, &[record], MAX_RECORDS as i64).await.ok(); |
|
| 226 | + | Ok(format!("{name}: {said}")) |
|
| 227 | + | } |
|
| 228 | + | ||
| 229 | + | /// End the approvals that waited too long (oxygenflow#57): from when the |
|
| 230 | + | /// jobs a waiting job depends on finished (or its run began). |
|
| 231 | + | pub async fn expire_approvals(app: &Shared, now: i64) -> sqlx::Result<usize> { |
|
| 232 | + | let mut ended = 0; |
|
| 233 | + | for job in fdb::waiting_for_approval(&app.db).await? { |
|
| 234 | + | let Ok(spec) = serde_json::from_str::<flow_core::Job>(&job.spec) else { continue }; |
|
| 235 | + | let Some(approval) = spec.approval else { continue }; |
|
| 236 | + | let Some(run) = fdb::run_by_id(&app.db, job.run_id).await? else { continue }; |
|
| 237 | + | let jobs = fdb::jobs(&app.db, run.id).await?; |
|
| 238 | + | let wanted = job.depends_on(); |
|
| 239 | + | let deps: Vec<&fdb::Job> = jobs.iter().filter(|j| wanted.contains(&j.name)).collect(); |
|
| 240 | + | if deps.iter().any(|d| d.status != "passed") { |
|
| 241 | + | continue; |
|
| 242 | + | } |
|
| 243 | + | let since = deps.iter().filter_map(|d| d.finished_ms).max().unwrap_or(run.created_ms); |
|
| 244 | + | if now - since < approval.expires_s as i64 * 1000 { |
|
| 245 | + | continue; |
|
| 246 | + | } |
|
| 247 | + | let hours = approval.expires_s as f64 / 3600.0; |
|
| 248 | + | let why = |
|
| 249 | + | format!("no approval within {}", if hours >= 1.0 { format!("{hours:.0}h") } else { format!("{}m", approval.expires_s / 60) }); |
|
| 250 | + | let changed = |
|
| 251 | + | fdb::finish(&app.db, job.id, fdb::Outcome { status: "cancelled", exit_code: None, error: &why, receipt: "", receipt_hash: "" }) |
|
| 252 | + | .await?; |
|
| 253 | + | report_changed(app, run.id, changed).await?; |
|
| 254 | + | ended += 1; |
|
| 255 | + | } |
|
| 256 | + | Ok(ended) |
|
| 257 | + | } |
|
| 258 | + | ||
| 161 | 259 | /// A push makes the push runs still going on its bookmark moot: they are |
|
| 162 | 260 | /// cancelled, and the new run's scope reaches back past them. |
|
| 163 | 261 | async fn supersede(app: &Shared, row: &db::repos::RepoRow, bookmark: &str, number: i64) { |
|
| … | 1801 unchanged lines |
||
| 1965 | 2063 | tokio::time::sleep(std::time::Duration::from_secs(60)).await; |
|
| 1966 | 2064 | if let Err(e) = reap_lost_jobs(&app).await { |
|
| 1967 | 2065 | tracing::warn!("flow: looking for lost jobs: {e}"); |
|
| 2066 | + | } |
|
| 2067 | + | match expire_approvals(&app, db::now_ms()).await { |
|
| 2068 | + | Ok(n) if n > 0 => tracing::info!("flow: {n} approvals expired"), |
|
| 2069 | + | Ok(_) => {} |
|
| 2070 | + | Err(e) => tracing::warn!("flow: expiring approvals: {e}"), |
|
| 1968 | 2071 | } |
|
| 1969 | 2072 | } |
|
| 1970 | 2073 | } |
|
| … | 236 unchanged lines |
modified crates/hub/src/web/flow.rs +86 −0
| … | 340 unchanged lines |
||
| 341 | 341 | queue: Option<crate::flow::QueueInfo>, |
|
| 342 | 342 | /// Releases a writer may attach this run's artifacts to. |
|
| 343 | 343 | releases: Vec<db::releases::Release>, |
|
| 344 | + | /// Jobs that wait, or waited, for a person (oxygenflow#57). |
|
| 345 | + | approvals: Vec<ApprovalView>, |
|
| 344 | 346 | } |
|
| 345 | 347 | ||
| 348 | + | /// A job's approval as the run page shows it. |
|
| 349 | + | pub struct ApprovalView { |
|
| 350 | + | pub job: String, |
|
| 351 | + | pub by: String, |
|
| 352 | + | pub others_only: bool, |
|
| 353 | + | /// `waiting`, `approved`, `rejected` or `expired`. |
|
| 354 | + | pub state: &'static str, |
|
| 355 | + | pub who: String, |
|
| 356 | + | pub when_ms: i64, |
|
| 357 | + | /// Whether the viewer may approve or reject it now, else why not. |
|
| 358 | + | pub may: Result<(), String>, |
|
| 359 | + | } |
|
| 360 | + | ||
| 361 | + | impl ApprovalView { |
|
| 362 | + | pub fn may_decide(&self) -> bool { |
|
| 363 | + | self.may.is_ok() |
|
| 364 | + | } |
|
| 365 | + | ||
| 366 | + | pub fn why_not(&self) -> String { |
|
| 367 | + | self.may.clone().err().unwrap_or_default() |
|
| 368 | + | } |
|
| 369 | + | } |
|
| 370 | + | ||
| 346 | 371 | /// Lines of a job's log a page shows before "the rest is loaded as it comes". |
|
| 347 | 372 | const PAGE_LINES: i64 = 5000; |
|
| 348 | 373 | ||
| … | 31 unchanged lines |
||
| 380 | 405 | Some(f) => crate::flow::human_ms(f - run.created_ms), |
|
| 381 | 406 | None => crate::flow::human_ms(db::now_ms() - run.created_ms), |
|
| 382 | 407 | }; |
|
| 408 | + | let mut approvals = Vec::new(); |
|
| 409 | + | for j in jobs.iter().filter(|j| !j.approval.is_empty()) { |
|
| 410 | + | let Some(a) = serde_json::from_str::<flow_core::Job>(&j.spec).ok().and_then(|s| s.approval) else { continue }; |
|
| 411 | + | let state = match (j.approval.as_str(), j.status.as_str()) { |
|
| 412 | + | ("waiting", "queued") => "waiting", |
|
| 413 | + | ("approved", _) => "approved", |
|
| 414 | + | (_, "cancelled") if !j.approved_by.is_empty() => "rejected", |
|
| 415 | + | (_, "cancelled") if j.error.starts_with("no approval within") => "expired", |
|
| 416 | + | _ => continue, |
|
| 417 | + | }; |
|
| 418 | + | let may = match (state, viewer.account()) { |
|
| 419 | + | ("waiting", Some(me)) => crate::flow::may_decide(&app, &row, &run, j, me).await.map(|_| ()), |
|
| 420 | + | ("waiting", None) => Err("sign in to approve".into()), |
|
| 421 | + | _ => Err(String::new()), |
|
| 422 | + | }; |
|
| 423 | + | approvals.push(ApprovalView { |
|
| 424 | + | job: j.name.clone(), |
|
| 425 | + | by: a.by, |
|
| 426 | + | others_only: a.others_only, |
|
| 427 | + | state, |
|
| 428 | + | who: j.approved_by.clone(), |
|
| 429 | + | when_ms: j.approved_ms.unwrap_or(0), |
|
| 430 | + | may, |
|
| 431 | + | }); |
|
| 432 | + | } |
|
| 383 | 433 | let graph = layout(jobs); |
|
| 384 | 434 | let can_write = viewer.account().is_some() && role >= Role::Write; |
|
| 385 | 435 | let can_rerun = !run.live() && can_write && crate::flow::enabled(&app, &row); |
|
| … | 20 unchanged lines |
||
| 406 | 456 | can_cancel, |
|
| 407 | 457 | queue, |
|
| 408 | 458 | releases, |
|
| 459 | + | approvals, |
|
| 409 | 460 | })) |
|
| 461 | + | } |
|
| 462 | + | ||
| 463 | + | #[derive(Deserialize)] |
|
| 464 | + | pub struct DecideForm { |
|
| 465 | + | #[serde(rename = "_csrf")] |
|
| 466 | + | csrf_token: String, |
|
| 467 | + | /// `approve` or `reject`. |
|
| 468 | + | action: String, |
|
| 469 | + | } |
|
| 470 | + | ||
| 471 | + | /// `POST /{owner}/{name}/flow/{id}/jobs/{job}/decide`: approve or reject |
|
| 472 | + | /// a job that waits for a person (oxygenflow#57). |
|
| 473 | + | pub async fn decide( |
|
| 474 | + | State(app): State<Shared>, |
|
| 475 | + | viewer: Viewer, |
|
| 476 | + | Path((owner, name, number, job)): Path<(String, String, i64, String)>, |
|
| 477 | + | Form(f): Form<DecideForm>, |
|
| 478 | + | ) -> Result<Response, Error> { |
|
| 479 | + | let shell = Shell::new(&app, &format!("/{owner}/{name}/flow/{number}"), &viewer); |
|
| 480 | + | let me = require(&viewer, &shell)?; |
|
| 481 | + | let (_, row, _) = lookup(&app, &shell, &viewer, &owner, &name).await?; |
|
| 482 | + | csrf(&viewer, &f.csrf_token, &shell)?; |
|
| 483 | + | let run = fdb::run(&app.db, row.id, number) |
|
| 484 | + | .await |
|
| 485 | + | .map_err(|e| Error::from_db(shell.clone(), e))? |
|
| 486 | + | .ok_or_else(|| Error::not_found(shell.clone()))?; |
|
| 487 | + | let approve = match f.action.as_str() { |
|
| 488 | + | "approve" => true, |
|
| 489 | + | "reject" => false, |
|
| 490 | + | _ => return Err(Error::bad(shell, "approve or reject")), |
|
| 491 | + | }; |
|
| 492 | + | match crate::flow::decide(&app, &row, &run, &job, &me.account, approve).await { |
|
| 493 | + | Ok(_) => Ok(Redirect::to(&format!("/{owner}/{name}/flow/{number}")).into_response()), |
|
| 494 | + | Err(why) => Err(Error::bad(shell, why)), |
|
| 495 | + | } |
|
| 410 | 496 | } |
|
| 411 | 497 | ||
| 412 | 498 | #[derive(Deserialize)] |
|
| … | 361 unchanged lines |
modified crates/hub/src/web/mod.rs +1 −0
| … | 253 unchanged lines |
||
| 254 | 254 | .route("/{owner}/{name}/flow/start", post(flow::start)) |
|
| 255 | 255 | .route("/{owner}/{name}/flow/{id}/rerun", post(flow::rerun)) |
|
| 256 | 256 | .route("/{owner}/{name}/flow/{id}/jobs/{job}/rerun", post(flow::rerun_job)) |
|
| 257 | + | .route("/{owner}/{name}/flow/{id}/jobs/{job}/decide", post(flow::decide)) |
|
| 257 | 258 | .route("/{owner}/{name}/flow/{id}/cancel", post(flow::cancel)) |
|
| 258 | 259 | .route("/{owner}/{name}/flow/stamp/{r}", get(flow::stamp)) |
|
| 259 | 260 | .route("/{owner}/{name}/flow/at/{r}", get(flow::at)) |
|
| … | 506 unchanged lines |
modified crates/hub/tests/flow.rs +91 −0
| … | 4252 unchanged lines |
||
| 4253 | 4253 | let (st, _) = call(&h.app, "GET", &format!("/api/v1/flow/jobs/{plan_id}/inputs/0"), Some("runner-secret"), None).await; |
|
| 4254 | 4254 | assert_eq!(st, StatusCode::CONFLICT, "only a running job's"); |
|
| 4255 | 4255 | } |
|
| 4256 | + | ||
| 4257 | + | #[tokio::test(flavor = "multi_thread")] |
|
| 4258 | + | async fn a_job_waits_for_approval_and_is_approved_rejected_or_expires() { |
|
| 4259 | + | let h = hub().await; |
|
| 4260 | + | let andy = db::accounts::create_account(&h.app.db, "andy", "[email protected]", "x", "salt", true).await.unwrap(); |
|
| 4261 | + | let bob = db::accounts::create_account(&h.app.db, "bob", "[email protected]", "x", "salt", false).await.unwrap(); |
|
| 4262 | + | let carol = db::accounts::create_account(&h.app.db, "carol", "[email protected]", "x", "salt", true).await.unwrap(); |
|
| 4263 | + | create(&h.app, "andy", "gated").await; |
|
| 4264 | + | let row = db::repos::get(&h.app.db, "andy", "gated").await.unwrap().unwrap(); |
|
| 4265 | + | let work = tempfile::tempdir().unwrap(); |
|
| 4266 | + | let c = client(&format!("{}/andy/gated", h.base), work.path()); |
|
| 4267 | + | write( |
|
| 4268 | + | work.path(), |
|
| 4269 | + | ".flow/pipeline.kdl", |
|
| 4270 | + | &format!( |
|
| 4271 | + | "schema-version 1\nsandbox \"alpine\" {{\n rootfs \"cas://blob/{ROOTFS}\"\n}}\njob \"plan\" {{\n sandbox \"alpine\"\n exec \"plan\"\n}}\njob \"apply\" {{\n sandbox \"alpine\"\n depends-on \"plan\"\n approval {{\n by \"maintain\"\n others-only #true\n }}\n exec \"apply\"\n}}\n" |
|
| 4272 | + | ), |
|
| 4273 | + | ); |
|
| 4274 | + | let mut n = 0; |
|
| 4275 | + | let mut push = || { |
|
| 4276 | + | n += 1; |
|
| 4277 | + | write(work.path(), "n.txt", &format!("{n}\n")); |
|
| 4278 | + | c.snapshot("test").unwrap(); |
|
| 4279 | + | c.commit(&format!("Push {n}"), &[]).unwrap(); |
|
| 4280 | + | assert!(c.push("origin", false).unwrap().iter().all(|r| r.ok)); |
|
| 4281 | + | }; |
|
| 4282 | + | // Runs `plan` to a pass; returns the run, its actor set to andy. |
|
| 4283 | + | let plan_passes = |number: i64| { |
|
| 4284 | + | let app = h.app.clone(); |
|
| 4285 | + | let row = row.clone(); |
|
| 4286 | + | async move { |
|
| 4287 | + | let mut run = None; |
|
| 4288 | + | for _ in 0..100 { |
|
| 4289 | + | if let Some(r) = db::flow::run(&app.db, row.id, number).await.unwrap() { |
|
| 4290 | + | run = Some(r); |
|
| 4291 | + | break; |
|
| 4292 | + | } |
|
| 4293 | + | tokio::time::sleep(std::time::Duration::from_millis(30)).await; |
|
| 4294 | + | } |
|
| 4295 | + | let run = run.expect("the run"); |
|
| 4296 | + | sqlx::query("UPDATE flow_runs SET actor = 'andy' WHERE id = ?").bind(run.id).execute(&app.db).await.unwrap(); |
|
| 4297 | + | let j = claim(&app).await.expect("plan is claimed"); |
|
| 4298 | + | assert_eq!(j["job"]["name"], "plan"); |
|
| 4299 | + | let (id, ph) = (j["job_id"].as_i64().unwrap(), j["pipeline_hash"].as_str().unwrap().to_string()); |
|
| 4300 | + | finish(&app, id, serde_json::json!({ "exit_code": 0, "duration_ms": 10, "receipt": receipt("plan", &ph, 0) })).await; |
|
| 4301 | + | db::flow::run(&app.db, row.id, number).await.unwrap().unwrap() |
|
| 4302 | + | } |
|
| 4303 | + | }; |
|
| 4304 | + | ||
| 4305 | + | // Run 1: apply waits; only someone with the role, other than the pusher, approves it. |
|
| 4306 | + | push(); |
|
| 4307 | + | let run = runs_of(&h.app, "andy", "gated").await[0].clone(); |
|
| 4308 | + | sqlx::query("UPDATE flow_runs SET actor = 'andy' WHERE id = ?").bind(run.id).execute(&h.app.db).await.unwrap(); |
|
| 4309 | + | let early = oxygenhub::flow::decide(&h.app, &row, &run, "apply", &carol, true).await.unwrap_err(); |
|
| 4310 | + | assert!(early.contains("waits for plan first"), "{early}"); |
|
| 4311 | + | let run = plan_passes(run.number).await; |
|
| 4312 | + | assert!(claim(&h.app).await.is_none(), "apply waits for a person, so no runner gets it"); |
|
| 4313 | + | let (_, page) = call(&h.app, "GET", &format!("/andy/gated/flow/{}", run.number), None, None).await; |
|
| 4314 | + | assert!(page.contains("waits for approval by someone with the <em>maintain</em> role, other than andy"), "the run page says so"); |
|
| 4315 | + | let own = oxygenhub::flow::decide(&h.app, &row, &run, "apply", &andy, true).await.unwrap_err(); |
|
| 4316 | + | assert!(own.contains("someone other than andy"), "{own}"); |
|
| 4317 | + | let no_role = oxygenhub::flow::decide(&h.app, &row, &run, "apply", &bob, true).await.unwrap_err(); |
|
| 4318 | + | assert!(no_role.contains("takes the maintain role"), "{no_role}"); |
|
| 4319 | + | assert_eq!(oxygenhub::flow::decide(&h.app, &row, &run, "apply", &carol, true).await.unwrap(), "apply: approved by carol"); |
|
| 4320 | + | let j = claim(&h.app).await.expect("approved, it runs"); |
|
| 4321 | + | assert_eq!(j["job"]["name"], "apply"); |
|
| 4322 | + | let (id, ph) = (j["job_id"].as_i64().unwrap(), j["pipeline_hash"].as_str().unwrap().to_string()); |
|
| 4323 | + | finish(&h.app, id, serde_json::json!({ "exit_code": 0, "duration_ms": 10, "receipt": receipt("apply", &ph, 0) })).await; |
|
| 4324 | + | let audit = db::accounts::audit_rows(&h.app.db, 50).await.unwrap(); |
|
| 4325 | + | assert!(audit.iter().any(|r| r.action == "flow.approved" && r.actor == "carol" && r.target.ends_with("#1 apply")), "audited"); |
|
| 4326 | + | let (_, page) = call(&h.app, "GET", &format!("/andy/gated/flow/{}", run.number), None, None).await; |
|
| 4327 | + | assert!(page.contains("approved by carol"), "the run says who"); |
|
| 4328 | + | ||
| 4329 | + | // Run 2: rejected through the API's rules: the job ends cancelled, the run ends. |
|
| 4330 | + | push(); |
|
| 4331 | + | let run = plan_passes(2).await; |
|
| 4332 | + | assert_eq!(oxygenhub::flow::decide(&h.app, &row, &run, "apply", &carol, false).await.unwrap(), "apply: rejected by carol"); |
|
| 4333 | + | let jobs = db::flow::jobs(&h.app.db, run.id).await.unwrap(); |
|
| 4334 | + | let apply = jobs.iter().find(|j| j.name == "apply").unwrap(); |
|
| 4335 | + | assert_eq!((apply.status.as_str(), apply.error.as_str()), ("cancelled", "rejected by carol")); |
|
| 4336 | + | assert!(claim(&h.app).await.is_none()); |
|
| 4337 | + | ||
| 4338 | + | // Run 3: nobody decides in time. |
|
| 4339 | + | push(); |
|
| 4340 | + | let run = plan_passes(3).await; |
|
| 4341 | + | assert_eq!(oxygenhub::flow::expire_approvals(&h.app, db::now_ms() + 3_600_000).await.unwrap(), 0, "not yet"); |
|
| 4342 | + | assert_eq!(oxygenhub::flow::expire_approvals(&h.app, db::now_ms() + 25 * 3_600_000).await.unwrap(), 1); |
|
| 4343 | + | let jobs = db::flow::jobs(&h.app.db, run.id).await.unwrap(); |
|
| 4344 | + | let apply = jobs.iter().find(|j| j.name == "apply").unwrap(); |
|
| 4345 | + | assert_eq!((apply.status.as_str(), apply.error.as_str()), ("cancelled", "no approval within 24h")); |
|
| 4346 | + | } |
replaced deps/oxygenflow +0 −0
No line changes.
modified templates/flow_run.html +10 −0
| … | 16 unchanged lines |
||
| 17 | 17 | {% if run.status == "error" %} |
|
| 18 | 18 | <section class="panel"><div class="strip bad">The pipeline is not valid, so nothing ran.</div><pre class="snippet">{{ run.error }}</pre><p class="small mute">Run <code>flow check</code> locally to see the same problems.</p></section> |
|
| 19 | 19 | {% else %} |
|
| 20 | + | {% if !approvals.is_empty() %}<section class="panel approvals" aria-label="Approvals"> |
|
| 21 | + | <h2>Approvals</h2> |
|
| 22 | + | <ul class="plain">{% for a in approvals %}<li class="approval st-{{ a.state }}"><strong>{{ a.job }}</strong> |
|
| 23 | + | {% if a.state == "waiting" %}waits for approval by someone with the <em>{{ a.by }}</em> role{% if a.others_only %}, other than {{ run.actor }}{% endif %}. |
|
| 24 | + | {% if a.may_decide() %}<form class="inline" method="post" action="{{ repo.url }}/flow/{{ run.number }}/jobs/{{ a.job }}/decide"><input type="hidden" name="_csrf" value="{{ shell.csrf }}"><button type="submit" name="action" value="approve" class="small">APPROVE</button> <button type="submit" name="action" value="reject" class="small danger-btn">REJECT</button></form> |
|
| 25 | + | {% else %}<span class="small mute">{{ a.why_not() }}</span>{% endif %} |
|
| 26 | + | {% else if a.state == "approved" %}approved by {{ a.who }} <time title="{{ a.when_ms|when }}">{{ a.when_ms|ago }}</time>. |
|
| 27 | + | {% else if a.state == "rejected" %}rejected by {{ a.who }} <time title="{{ a.when_ms|when }}">{{ a.when_ms|ago }}</time>. |
|
| 28 | + | {% else %}not approved in time.{% endif %}</li>{% endfor %}</ul> |
|
| 29 | + | </section>{% endif %} |
|
| 20 | 30 | <section class="panel flow-graph-box"> |
|
| 21 | 31 | <svg class="flow-graph" viewBox="0 0 {{ graph.width }} {{ graph.height }}" width="{{ graph.width }}" height="{{ graph.height }}" role="img" aria-label="Job graph of run {{ run.number }}"> |
|
| 22 | 32 | {% for e in graph.edges %}<path class="edge st-{{ e.status }}" d="{{ e.d }}"/>{% endfor %} |
|
| … | 33 unchanged lines |