From 0973b192e57e94d237c827c991ec3005d325c432 Mon Sep 17 00:00:00 2001 From: Abhinav Chordia Date: Mon, 2 Nov 2026 07:01:42 +0000 Subject: [PATCH 1/5] api: page and filter the admin sandbox list, and accept every sandbox ID form GET /admin/api/v1/sandboxes takes an opaque keyset cursor (the response is then {sandboxes, next_cursor}) and filters by lifecycle, template, node, assignment, user, creation time and name or ID prefix. Without a cursor it answers the same bare array as before. The node-reported rows stay on the first page of the unfiltered list only; the org-scoped list's test moves with the handler into sandbox_list. Every admin sandbox route and the debug route resolve sbx_, a dashed or undashed UUID and the sandbox pod name through one parser, and answer 400 for any other form. GET /admin/api/v1/sandboxes/spaced-versions counts non-deleted sandboxes by template, reported spaced version and lifecycle. --- architecture-baseline.toml | 1 - crates/libs/api/Cargo.toml | 1 + crates/libs/api/src/routes/admin/docs.rs | 49 +- ...sts__admin_openapi_stub_is_admin_only.snap | 4 + crates/libs/api/src/routes/admin/mod.rs | 10 +- .../libs/api/src/routes/admin/sandbox_id.rs | 75 +++ .../api/src/routes/admin/sandbox_id/tests.rs | 83 +++ .../libs/api/src/routes/admin/sandbox_list.rs | 326 ++++++++++ .../src/routes/admin/sandbox_list/tests.rs | 368 +++++++++++ .../api/src/routes/admin/sandbox_store.rs | 228 +++++++ .../src/routes/admin/sandbox_store/tests.rs | 596 ++++++++++++++++++ crates/libs/api/src/routes/admin/sandboxes.rs | 145 +---- .../api/src/routes/admin/sandboxes/tests.rs | 99 --- crates/libs/api/src/routes/debug/sandbox.rs | 6 +- .../api/src/routes/debug/sandbox/tests.rs | 4 +- 15 files changed, 1764 insertions(+), 231 deletions(-) create mode 100644 crates/libs/api/src/routes/admin/sandbox_id.rs create mode 100644 crates/libs/api/src/routes/admin/sandbox_id/tests.rs create mode 100644 crates/libs/api/src/routes/admin/sandbox_list.rs create mode 100644 crates/libs/api/src/routes/admin/sandbox_list/tests.rs create mode 100644 crates/libs/api/src/routes/admin/sandbox_store.rs create mode 100644 crates/libs/api/src/routes/admin/sandbox_store/tests.rs diff --git a/architecture-baseline.toml b/architecture-baseline.toml index 57a988813e..a6c3b24de5 100644 --- a/architecture-baseline.toml +++ b/architecture-baseline.toml @@ -24,7 +24,6 @@ "crates/libs/api/src/public_operations.rs" = 1787 "crates/libs/api/src/public_sandbox_operations.rs" = 1023 "crates/libs/api/src/routes/admin/node_maintenance.rs" = 1783 -"crates/libs/api/src/routes/admin/sandboxes.rs" = 1011 "crates/libs/api/src/routes/admin/templates.rs" = 2262 "crates/libs/api/src/routes/e2b/lifecycle.rs" = 1236 "crates/libs/api/src/routes/public/sandboxes.rs" = 1245 diff --git a/crates/libs/api/Cargo.toml b/crates/libs/api/Cargo.toml index 6fc8619d3b..c79876cc52 100644 --- a/crates/libs/api/Cargo.toml +++ b/crates/libs/api/Cargo.toml @@ -89,6 +89,7 @@ pplx-space-events = { workspace = true } pplx-space-metrics = { workspace = true } pplx-space-observability = { workspace = true, default-features = false } pplx-space-public-api = { workspace = true, features = ["schema", "serde"] } +pplx-space-placement = { workspace = true } pplx-space-common-core = { workspace = true, features = ["serde", "sqlx"] } pplx-space-common-grpc = { workspace = true, features = ["internal-tls"] } pplx-space-common-svc = { workspace = true } diff --git a/crates/libs/api/src/routes/admin/docs.rs b/crates/libs/api/src/routes/admin/docs.rs index 6522a7ae92..d432ebb757 100644 --- a/crates/libs/api/src/routes/admin/docs.rs +++ b/crates/libs/api/src/routes/admin/docs.rs @@ -331,13 +331,13 @@ fn admin_openapi_value() -> Value { "post": { "summary": "Enqueue an audited break-glass sandbox lifecycle operation", "parameters": [ - { "name": "sandbox_id", "in": "path", "required": true, "schema": { "type": "string", "pattern": "^sbx_" } }, + { "name": "sandbox_id", "in": "path", "required": true, "description": "sbx_, a dashed or undashed UUID in either case, or the sandbox pod name (space-sbx-, optionally with its -g suffix)", "schema": { "type": "string" } }, { "name": "action", "in": "path", "required": true, "schema": { "type": "string", "enum": ["start", "stop", "pause", "resume", "suspend", "restore", "wake", "kill", "delete"] } }, { "name": "Idempotency-Key", "in": "header", "required": true, "schema": { "type": "string" } } ], "responses": { "202": { "description": "Operation enqueued" }, - "400": { "description": "Invalid action or idempotency key" }, + "400": { "description": "Invalid sandbox id, action or idempotency key" }, "404": { "description": "Sandbox not found" }, "409": { "description": "Idempotency conflict" } } @@ -790,12 +790,23 @@ fn insert_admin_sandbox_create_paths(paths: &mut Map) { json!({ "get": { "summary": "List sandboxes across every organization", + "description": "Newest first by created_at, then id. Without cursor the response is a bare array of at most limit sandboxes; with no filter, it is followed by the sandboxes nodes report that have no sandbox row. Pass cursor, empty for the first page, to page: the response is then {sandboxes, next_cursor}, next_cursor is null on the last page, and the node-reported rows come on the first page only. Rows created while paging sort before the cursor and do not appear on later pages. Filters, org_id included, combine and leave out the node-reported rows. Unknown parameters are ignored.", "parameters": [ - { "name": "limit", "in": "query", "required": false, "schema": { "type": "integer" } }, - { "name": "org_id", "in": "query", "required": false, "schema": { "type": "string" } } + { "name": "limit", "in": "query", "required": false, "description": "1 to 5000, default 1000", "schema": { "type": "integer" } }, + { "name": "cursor", "in": "query", "required": false, "description": "Opaque; empty for the first page, then the previous page's next_cursor", "schema": { "type": "string" } }, + { "name": "org_id", "in": "query", "required": false, "schema": { "type": "string" } }, + { "name": "lifecycle", "in": "query", "required": false, "description": "Comma-separated stored lifecycles, including deleted and cleanup_required. A row's reported lifecycle can differ while its node's observation lags.", "schema": { "type": "string" } }, + { "name": "template", "in": "query", "required": false, "description": "The sandbox row's template", "schema": { "type": "string" } }, + { "name": "node_id", "in": "query", "required": false, "description": "The node reported as active_node_id", "schema": { "type": "string" } }, + { "name": "assigned", "in": "query", "required": false, "description": "true for sandboxes with an active node, false for those without; false cannot be combined with node_id", "schema": { "type": "boolean" } }, + { "name": "user_id", "in": "query", "required": false, "schema": { "type": "string" } }, + { "name": "created_after", "in": "query", "required": false, "description": "RFC 3339; exclusive", "schema": { "type": "string" } }, + { "name": "created_before", "in": "query", "required": false, "description": "RFC 3339; exclusive, later than created_after", "schema": { "type": "string" } }, + { "name": "prefix", "in": "query", "required": false, "description": "Case-sensitive name prefix, or an ID prefix with optional sbx_ and dashes where a UUID has them", "schema": { "type": "string" } } ], "responses": { - "200": { "description": "Sandbox list" } + "200": { "description": "Sandbox array, or {sandboxes, next_cursor} when cursor is set" }, + "400": { "description": "Invalid cursor or filter" } } }, "post": { @@ -812,13 +823,39 @@ fn insert_admin_sandbox_create_paths(paths: &mut Map) { } }), ); + paths.insert( + "/sandboxes/spaced-versions".to_string(), + json!({ + "get": { + "summary": "Count sandboxes by template, spaced version and lifecycle", + "description": "Every sandbox that is not deleted, grouped by its template (else the one its node reports), the spaced version its node last reported (null before any report) and its stored lifecycle.", + "parameters": [ + { "name": "org_id", "in": "query", "required": false, "schema": { "type": "string" } } + ], + "responses": { + "200": { "description": "Object with counts (template, spaced_version, lifecycle, count)" } + } + } + }), + ); paths.insert( "/sandboxes/{sandbox_id}".to_string(), json!({ + "get": { + "summary": "Get a sandbox in any organization", + "parameters": [ + { "name": "sandbox_id", "in": "path", "required": true, "description": "sbx_, a dashed or undashed UUID in either case, or the sandbox pod name (space-sbx-, optionally with its -g suffix)", "schema": { "type": "string" } } + ], + "responses": { + "200": { "description": "Sandbox" }, + "400": { "description": "Invalid sandbox id" }, + "404": { "description": "Sandbox not found" } + } + }, "patch": { "summary": "Enqueue a sandbox update (network policy, auto-lifecycle, metadata) under the sandbox's owning organization", "parameters": [ - { "name": "sandbox_id", "in": "path", "required": true, "schema": { "type": "string", "pattern": "^sbx_" } }, + { "name": "sandbox_id", "in": "path", "required": true, "description": "sbx_, a dashed or undashed UUID in either case, or the sandbox pod name (space-sbx-, optionally with its -g suffix)", "schema": { "type": "string" } }, { "name": "Idempotency-Key", "in": "header", "required": true, "schema": { "type": "string" } } ], "responses": { diff --git a/crates/libs/api/src/routes/admin/docs/snapshots/pplx_space_api__routes__admin__docs__tests__admin_openapi_stub_is_admin_only.snap b/crates/libs/api/src/routes/admin/docs/snapshots/pplx_space_api__routes__admin__docs__tests__admin_openapi_stub_is_admin_only.snap index 92f6d60956..df0b85a526 100644 --- a/crates/libs/api/src/routes/admin/docs/snapshots/pplx_space_api__routes__admin__docs__tests__admin_openapi_stub_is_admin_only.snap +++ b/crates/libs/api/src/routes/admin/docs/snapshots/pplx_space_api__routes__admin__docs__tests__admin_openapi_stub_is_admin_only.snap @@ -109,7 +109,11 @@ expression: paths_by_method "get", "post" ], + "/sandboxes/spaced-versions": [ + "get" + ], "/sandboxes/{sandbox_id}": [ + "get", "patch" ], "/sandboxes/{sandbox_id}:force-{action}": [ diff --git a/crates/libs/api/src/routes/admin/mod.rs b/crates/libs/api/src/routes/admin/mod.rs index 039cf4664f..4f3a3db299 100644 --- a/crates/libs/api/src/routes/admin/mod.rs +++ b/crates/libs/api/src/routes/admin/mod.rs @@ -47,6 +47,10 @@ pub mod nodes; mod operations; mod runtime_environment_store; mod runtime_environments; +// `pub(in crate::routes)`: the debug route takes the same sandbox ID forms. +pub(in crate::routes) mod sandbox_id; +mod sandbox_list; +mod sandbox_store; mod sandboxes; // `pub(in crate::routes)` rather than private: `routes::debug` is mounted // inside this router (`debug::admin_v1_debug_router`, merged by `v1_router` @@ -277,7 +281,11 @@ pub fn v1_router(state: AppState) -> Router { ) .route( "/sandboxes", - get(sandboxes::list_sandboxes).post(sandboxes::create_sandbox), + get(sandbox_list::list_sandboxes).post(sandboxes::create_sandbox), + ) + .route( + "/sandboxes/spaced-versions", + get(sandbox_list::count_spaced_versions), ) .route( "/sandboxes/{spec}", diff --git a/crates/libs/api/src/routes/admin/sandbox_id.rs b/crates/libs/api/src/routes/admin/sandbox_id.rs new file mode 100644 index 0000000000..1fce1b7c5e --- /dev/null +++ b/crates/libs/api/src/routes/admin/sandbox_id.rs @@ -0,0 +1,75 @@ +//! The sandbox identifier forms every admin sandbox route accepts. +//! +//! An operator holds a sandbox's identity in whatever form the tool in front +//! of them printed: the public `sbx_` ID from the API, the bare UUID from the +//! debug view and node routes, the undashed UUID from logs, or the pod name +//! from `kubectl`. The admin sandbox routes and the debug route resolve all of +//! them through [`parse_admin_sandbox_id`], so a form works on every route or +//! on none. + +use pplx_space_common_core::types::SandboxId; +use pplx_space_public_api::PUBLIC_SANDBOX_ID_PREFIX; +use uuid::Uuid; + +/// The prefix of the names `pplx_space_common_core::kubernetes::sandbox_pod_name` +/// gives sandbox pods: `space-sbx--g`. +const SANDBOX_POD_NAME_PREFIX: &str = "space-sbx-"; +const SANDBOX_POD_GENERATION_PREFIX: &str = "-g"; +const DASHED_UUID_LEN: usize = 36; +const UNDASHED_UUID_LEN: usize = 32; + +/// The identifier matched none of the forms the admin sandbox routes accept. +#[derive(Clone, Copy, Debug, Eq, PartialEq, thiserror::Error)] +#[error( + "sandbox id must be sbx_, a dashed or undashed UUID, or a sandbox pod name \ + (space-sbx-, optionally with its -g suffix)" +)] +pub(in crate::routes) struct AdminSandboxIdError; + +/// Resolve an admin sandbox route's identifier to the sandbox it names. +/// +/// Accepts `sbx_`, a bare UUID, and either UUID with its dashes removed, +/// with hex digits in either case, and the sandbox's pod name with or without +/// its generation suffix. Nothing is trimmed, and other UUID spellings (braced, +/// `urn:uuid:`) are refused, so the route answers 400 rather than guessing. +pub(in crate::routes) fn parse_admin_sandbox_id( + value: &str, +) -> Result { + let uuid = if let Some(uuid) = value.strip_prefix(PUBLIC_SANDBOX_ID_PREFIX) { + parse_uuid(uuid)? + } else if let Some(name) = value.strip_prefix(SANDBOX_POD_NAME_PREFIX) { + parse_pod_name_uuid(name)? + } else { + parse_uuid(value)? + }; + Ok(SandboxId::from_uuid(uuid)) +} + +/// A dashed or undashed UUID and nothing else: `Uuid::try_parse` also takes +/// the braced and URN spellings, which no SPACE tool prints. +fn parse_uuid(value: &str) -> Result { + if !matches!(value.len(), DASHED_UUID_LEN | UNDASHED_UUID_LEN) { + return Err(AdminSandboxIdError); + } + Uuid::try_parse(value).map_err(|_| AdminSandboxIdError) +} + +/// The UUID of a pod name with its `space-sbx-` prefix removed. +fn parse_pod_name_uuid(name: &str) -> Result { + let (uuid, generation) = match name.split_at_checked(UNDASHED_UUID_LEN) { + Some(parts) => parts, + None => return Err(AdminSandboxIdError), + }; + if !generation.is_empty() { + let digits = generation + .strip_prefix(SANDBOX_POD_GENERATION_PREFIX) + .ok_or(AdminSandboxIdError)?; + if digits.is_empty() || !digits.bytes().all(|byte| byte.is_ascii_digit()) { + return Err(AdminSandboxIdError); + } + } + parse_uuid(uuid) +} + +#[cfg(test)] +mod tests; diff --git a/crates/libs/api/src/routes/admin/sandbox_id/tests.rs b/crates/libs/api/src/routes/admin/sandbox_id/tests.rs new file mode 100644 index 0000000000..7487bc425d --- /dev/null +++ b/crates/libs/api/src/routes/admin/sandbox_id/tests.rs @@ -0,0 +1,83 @@ +use pplx_space_common_core::types::SandboxId; +use pplx_space_public_api::PublicSandboxId; + +use super::*; + +const UUID: &str = "0190a1b2-c3d4-7e5f-8a9b-0c1d2e3f4a5b"; + +fn sandbox() -> SandboxId { + SandboxId::from_uuid(Uuid::parse_str(UUID).expect("fixture uuid")) +} + +/// Every form an operator may hold for one sandbox resolves to it. +#[test] +fn every_accepted_form_names_the_same_sandbox() { + let undashed = UUID.replace('-', ""); + let accepted = [ + PublicSandboxId::from(sandbox()).to_string(), + format!("sbx_{}", UUID.to_uppercase()), + format!("sbx_{undashed}"), + UUID.to_string(), + UUID.to_uppercase(), + undashed.clone(), + undashed.to_uppercase(), + format!("space-sbx-{undashed}-g0"), + format!("space-sbx-{undashed}-g17"), + format!("space-sbx-{undashed}"), + ]; + for form in accepted { + assert_eq!( + parse_admin_sandbox_id(&form), + Ok(sandbox()), + "{form} should resolve" + ); + } +} + +/// The pod-name form is the one the pod builder actually produces. +#[cfg(feature = "kubernetes")] +#[test] +fn the_pod_builder_names_resolve() { + use pplx_space_common_core::kubernetes::sandbox_pod_name; + + for generation in [0, 1, i64::MAX] { + let name = sandbox_pod_name(sandbox(), generation); + assert_eq!(parse_admin_sandbox_id(&name), Ok(sandbox()), "{name}"); + } +} + +#[test] +fn unknown_forms_are_refused() { + let undashed = UUID.replace('-', ""); + let refused = [ + String::new(), + "sbx_".to_string(), + format!("{{{UUID}}}"), + format!("urn:uuid:{UUID}"), + format!("sbx_{{{UUID}}}"), + format!(" {UUID}"), + format!("{UUID} "), + format!("sandbox:{UUID}"), + UUID.chars().take(35).collect(), + undashed.chars().take(31).collect(), + format!("space-sbx-{UUID}"), + format!("space-sbx-{undashed}-g"), + format!("space-sbx-{undashed}-gx"), + format!("space-sbx-{undashed}-g1-g2"), + format!("space-sbx-{undashed}g1"), + format!( + "space-sbx-{}", + undashed.chars().take(31).collect::() + ), + format!("sbx_space-sbx-{undashed}"), + format!("SBX_{UUID}"), + "my-sandbox".to_string(), + ]; + for form in refused { + assert_eq!( + parse_admin_sandbox_id(&form), + Err(AdminSandboxIdError), + "{form:?} should be refused" + ); + } +} diff --git a/crates/libs/api/src/routes/admin/sandbox_list.rs b/crates/libs/api/src/routes/admin/sandbox_list.rs new file mode 100644 index 0000000000..5c3a540a83 --- /dev/null +++ b/crates/libs/api/src/routes/admin/sandbox_list.rs @@ -0,0 +1,326 @@ +//! `GET /sandboxes` and `GET /sandboxes/spaced-versions` on the admin +//! surface: the cross-org sandbox list, its cursor and filters, and the count +//! of sandboxes by template and `spaced` version. + +use std::str::FromStr; + +use axum::{ + Extension, Json, + extract::{Query, State}, + http::StatusCode, + response::IntoResponse, +}; +use base64::{Engine as _, engine::general_purpose::URL_SAFE_NO_PAD}; +use pplx_space_common_core::types::{NodeId, OrgId, SandboxId, TemplateName, UserId}; +use pplx_space_placement::StoredSandboxLifecycle; +use serde::{Deserialize, Serialize}; +use time::OffsetDateTime; + +use super::sandbox_store::{ + self, AdminSandboxFilter, NodeAssignment, SandboxSearch, SpacedVersionCount, +}; +use super::support::{self, ContentionScope, api_error, require_managing_org_admin}; +use crate::controllers::sandbox::SandboxPageCursor; +use crate::middleware::auth::AuthUser; +use crate::models::{Sandbox, SandboxResponse}; +use crate::routes::AppState; + +const DEFAULT_ADMIN_SANDBOX_LIMIT: u32 = 1000; +const MAX_ADMIN_SANDBOX_LIMIT: u32 = 5000; +const CURSOR_VERSION: &str = "v1"; + +type QueryError = (StatusCode, Json); + +/// The list's query string. Unknown parameters are ignored, as they were +/// before the filters existed, so a console that sends a filter this server +/// predates gets the unfiltered list rather than an error. +#[derive(Debug, Default, Deserialize)] +pub struct AdminSandboxListQuery { + limit: Option, + org_id: Option, + /// Present (empty for the first page) to page through the list; the + /// response is then `{sandboxes, next_cursor}` instead of a bare array. + cursor: Option, + /// Comma-separated stored lifecycles. + lifecycle: Option, + template: Option, + node_id: Option, + assigned: Option, + user_id: Option, + created_after: Option, + created_before: Option, + prefix: Option, +} + +/// One page of the list, served when the request carries `cursor`. +#[derive(Debug, Serialize)] +struct AdminSandboxPage { + sandboxes: Vec, + /// Pass as `cursor` for the next page; `None` on the last page. + next_cursor: Option, +} + +/// How the request pages: not at all (the legacy bare array), or from the +/// start or a cursor. +#[derive(Debug, Clone, Copy)] +enum Paging { + Unpaged, + First, + After(SandboxPageCursor), +} + +impl Paging { + fn is_paged(self) -> bool { + !matches!(self, Self::Unpaged) + } + + fn after(self) -> Option { + match self { + Self::After(cursor) => Some(cursor), + Self::Unpaged | Self::First => None, + } + } +} + +#[derive(Debug)] +struct ListRequest { + limit: u32, + paging: Paging, + filter: AdminSandboxFilter, +} + +impl AdminSandboxListQuery { + fn into_request(self) -> Result { + let limit = self + .limit + .unwrap_or(DEFAULT_ADMIN_SANDBOX_LIMIT) + .clamp(1, MAX_ADMIN_SANDBOX_LIMIT); + let paging = match self.cursor.as_deref() { + None => Paging::Unpaged, + Some("") => Paging::First, + Some(cursor) => Paging::After(decode_cursor(cursor)?), + }; + let node = match (self.node_id, self.assigned) { + (Some(_), Some(false)) => { + return Err(bad_request( + "node_id cannot be combined with assigned=false", + )); + } + (Some(node_id), _) => Some(NodeAssignment::Node(node_id)), + (None, Some(true)) => Some(NodeAssignment::Assigned), + (None, Some(false)) => Some(NodeAssignment::Unassigned), + (None, None) => None, + }; + let created_after = self + .created_after + .as_deref() + .map(|value| parse_timestamp("created_after", value)) + .transpose()?; + let created_before = self + .created_before + .as_deref() + .map(|value| parse_timestamp("created_before", value)) + .transpose()?; + if let (Some(after), Some(before)) = (created_after, created_before) + && after >= before + { + return Err(bad_request( + "created_after must be earlier than created_before", + )); + } + let filter = AdminSandboxFilter { + org_id: self.org_id, + lifecycles: self + .lifecycle + .as_deref() + .map(parse_lifecycles) + .transpose()?, + template: self + .template + .map(|template| { + TemplateName::parse(template) + .map_err(|error| bad_request(format!("invalid template: {error}"))) + }) + .transpose()?, + node, + user_id: self.user_id, + created_after, + created_before, + search: self + .prefix + .filter(|prefix| !prefix.is_empty()) + .map(SandboxSearch::new), + }; + Ok(ListRequest { + limit, + paging, + filter, + }) + } +} + +fn bad_request(message: impl Into) -> QueryError { + api_error(StatusCode::BAD_REQUEST, message) +} + +fn parse_lifecycles(value: &str) -> Result, QueryError> { + let mut lifecycles = Vec::new(); + for part in value.split(',') { + let lifecycle = StoredSandboxLifecycle::from_str(part) + .map_err(|_| bad_request(format!("unknown lifecycle {part:?}")))?; + if !lifecycles.contains(&lifecycle) { + lifecycles.push(lifecycle); + } + } + Ok(lifecycles) +} + +fn parse_timestamp(name: &str, value: &str) -> Result { + let invalid = || bad_request(format!("{name} must be an RFC 3339 timestamp")); + let timestamp = jiff::Timestamp::from_str(value).map_err(|_| invalid())?; + OffsetDateTime::from_unix_timestamp_nanos(timestamp.as_nanosecond()).map_err(|_| invalid()) +} + +/// `v1::`, base64url without padding. Not +/// signed: it carries only a position in a list the caller can already read +/// in full. +fn encode_cursor(sandbox: &Sandbox) -> String { + URL_SAFE_NO_PAD.encode(format!( + "{CURSOR_VERSION}:{}:{}", + sandbox.created_at.unix_timestamp_nanos(), + sandbox.id + )) +} + +fn decode_cursor(cursor: &str) -> Result { + let invalid = || bad_request("invalid cursor"); + let raw = URL_SAFE_NO_PAD.decode(cursor).map_err(|_| invalid())?; + let raw = std::str::from_utf8(&raw).map_err(|_| invalid())?; + let mut parts = raw.splitn(3, ':'); + let (Some(CURSOR_VERSION), Some(nanos), Some(id)) = (parts.next(), parts.next(), parts.next()) + else { + return Err(invalid()); + }; + let created_at = nanos + .parse::() + .ok() + .and_then(|nanos| OffsetDateTime::from_unix_timestamp_nanos(nanos).ok()) + .ok_or_else(invalid)?; + let id = SandboxId::from_str(id).map_err(|_| invalid())?; + Ok(SandboxPageCursor { created_at, id }) +} + +pub async fn list_sandboxes( + State(state): State, + Extension(AuthUser(actor)): Extension, + Query(query): Query, +) -> axum::response::Response { + if let Err(err) = require_managing_org_admin(&state.pool, &actor).await { + return err.into_response(); + } + let request = match query.into_request() { + Ok(request) => request, + Err(err) => return err.into_response(), + }; + // One lookahead row tells a paged request whether another page follows. + let max_rows = i64::from(request.limit) + i64::from(request.paging.is_paged()); + let mut sandboxes = match sandbox_store::list_sandboxes( + &state.pool, + &request.filter, + request.paging.after(), + max_rows, + ) + .await + { + Ok(sandboxes) => sandboxes, + // `ReadOnly`: the busiest admin read on this surface, and a plain + // `SELECT` over `sandboxes`, so it blocks behind DDL on the hottest + // table in the schema. It wrote nothing, so every contention SQLSTATE + // answers a retryable 503 and none takes the write-specific 409 — see + // `support::contention_disposition`. + Err(error) => { + if let Some((status, message)) = + support::contention_disposition(ContentionScope::ReadOnly, &error) + { + return api_error(status, message).into_response(); + } + tracing::warn!(%error, "admin sandbox list query failed"); + return api_error(StatusCode::INTERNAL_SERVER_ERROR, "Internal server error") + .into_response(); + } + }; + let next_cursor = match usize::try_from(request.limit) { + Ok(limit) if sandboxes.len() > limit => { + sandboxes.truncate(limit); + sandboxes.last().map(encode_cursor) + } + _ => None, + }; + // The node-reported rows with no sandbox row are not in the keyset order + // and belong to no org, so they come once, on the first page of the + // unfiltered list. + let include_observed_only = + !matches!(request.paging, Paging::After(_)) && request.filter.is_unfiltered(); + match state + .sandbox_controller + .list_responses(&state.pool, &sandboxes, include_observed_only) + .await + { + Ok(sandboxes) if !request.paging.is_paged() => Json(sandboxes).into_response(), + Ok(sandboxes) => Json(AdminSandboxPage { + sandboxes, + next_cursor, + }) + .into_response(), + // A known remainder, in two halves, neither of which is useful + // without the other. + // + // `list_responses` reaches `responses::fetch_sandbox_observed_states` + // and `fetch_observed_only_sandboxes`, both of which erase the + // SQLSTATE into `ControllerError::internal` — so no contention + // identity exists to honour here yet. And this arm flattens the + // `ControllerError` to a 500 regardless of the status it carried, so + // classifying those reads alone would not surface it either. + // + // Adding an `is_lock_contention()` check here today would be a guard + // for a state nothing produces. Projection needs to preserve the + // database error identity, and this handler must also preserve the + // resulting status. These are independent fixes: classification + // recovers contention identity, while flattening discards every status, + // not just contention. Neither alone makes this path retryable. + // Same shape on `sandboxes::get_sandbox`. + Err(error) => support::controller_read_error(&error, || { + tracing::warn!(%error, "admin sandbox response projection failed"); + }), + } +} + +#[derive(Debug, Default, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct SpacedVersionCountQuery { + org_id: Option, +} + +#[derive(Debug, Serialize)] +struct SpacedVersionCounts { + counts: Vec, +} + +/// Every sandbox that is not deleted, counted by template, the `spaced` +/// version its node last reported, and stored lifecycle. +pub async fn count_spaced_versions( + State(state): State, + Extension(AuthUser(actor)): Extension, + Query(query): Query, +) -> axum::response::Response { + if let Err(err) = require_managing_org_admin(&state.pool, &actor).await { + return err.into_response(); + } + match sandbox_store::count_by_spaced_version(&state.pool, query.org_id).await { + Ok(counts) => Json(SpacedVersionCounts { counts }).into_response(), + Err(error) => support::read_db_error(error), + } +} + +#[cfg(test)] +mod tests; diff --git a/crates/libs/api/src/routes/admin/sandbox_list/tests.rs b/crates/libs/api/src/routes/admin/sandbox_list/tests.rs new file mode 100644 index 0000000000..017271d9b9 --- /dev/null +++ b/crates/libs/api/src/routes/admin/sandbox_list/tests.rs @@ -0,0 +1,368 @@ +use axum::Router; +use axum::body::Body; +use axum::http::{Method, Request}; +use pplx_space_common_core::sql; +use pplx_space_db_test_fixtures::{TestDbResult, insert_ready_worker_node, pg_test_db}; +use pplx_space_public_api::PublicSandboxId; +use serde_json::{Value, json}; +use sqlx::PgPool; +use tower::ServiceExt; +use uuid::Uuid; + +use super::*; +use crate::routes::admin::sandbox_store::tests::{ + SeedSandbox, seed_observation, seed_owner, seed_sandbox, +}; + +const SEED_ORG: Uuid = pplx_space_db_test_fixtures::SEED_ORG_ID; + +// --------------------------------------------------------------------------- +// Route tests: the real admin router, session auth included, against a +// migrated database. + +struct Harness { + pool: PgPool, + app: Router, + admin_token: String, + member_token: String, +} + +async fn insert_session_user(pool: &PgPool, role: &str) -> TestDbResult { + let user_id = UserId::new(); + sqlx::query(sql!( + "INSERT INTO users (id, email, password_hash, display_name, org_id, role) + VALUES ($1, $2, 'synthetic-password-hash', 'sandbox list tests', $3, $4)" + )) + .bind(user_id) + .bind(format!("{user_id}@example.invalid")) + .bind(OrgId::from_uuid(SEED_ORG)) + .bind(role) + .execute(pool) + .await?; + let (token, token_hash) = crate::auth::generate_session_token(); + sqlx::query(sql!( + "INSERT INTO sessions (user_id, token_hash, expires_at) + VALUES ($1, $2, NOW() + INTERVAL '1 hour')" + )) + .bind(user_id) + .bind(token_hash) + .execute(pool) + .await?; + Ok(token) +} + +async fn harness(pool: PgPool) -> TestDbResult { + let admin_token = insert_session_user(&pool, "organization_admin").await?; + let member_token = insert_session_user(&pool, "member").await?; + let state = AppState::from_pool(pool.clone()).await?; + let app = crate::routes::admin::shared_host_router(state.clone()).with_state(state); + Ok(Harness { + pool, + app, + admin_token, + member_token, + }) +} + +impl Harness { + async fn request( + &self, + token: &str, + method: Method, + path: &str, + body: Option, + ) -> (StatusCode, Value) { + let request = Request::builder() + .method(method) + .uri(format!("/admin/api/v1{path}")) + .header("authorization", format!("Bearer {token}")) + .header("content-type", "application/json") + .header("idempotency-key", Uuid::now_v7().to_string()) + .body(body.map_or_else(Body::empty, |body| Body::from(body.to_string()))) + .expect("request"); + let response = self.app.clone().oneshot(request).await.expect("response"); + let status = response.status(); + let bytes = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .expect("body"); + let value = serde_json::from_slice(&bytes) + .unwrap_or_else(|_| Value::String(String::from_utf8_lossy(&bytes).into_owned())); + (status, value) + } + + async fn get(&self, path: &str) -> (StatusCode, Value) { + self.request(&self.admin_token, Method::GET, path, None) + .await + } +} + +fn ids(rows: &Value) -> Vec { + rows.as_array() + .expect("sandbox array") + .iter() + .map(|row| row["id"].as_str().expect("id").to_string()) + .collect() +} + +/// Without `cursor` the list is the bare array it always was. With one, it +/// is a page with `next_cursor`, and following the cursors returns every row +/// once. The node-reported rows with no sandbox row belong to no org: they +/// come on the first page of the unfiltered list only, never under `org_id` +/// or another filter. +#[tokio::test] +async fn the_list_pages_by_cursor_and_keeps_the_unpaged_array() -> TestDbResult<()> { + let db = pg_test_db().await?; + let harness = harness(db.pool.clone()).await?; + let owner = seed_owner(&harness.pool).await?; + let mut seeded = Vec::new(); + for _ in 0..5 { + seeded.push(seed_sandbox(&harness.pool, owner, SeedSandbox::default()).await?); + } + seeded.reverse(); + let seeded = seeded.iter().map(ToString::to_string).collect::>(); + let node_id = NodeId::new(); + insert_ready_worker_node(&harness.pool, node_id).await?; + let node_only = SandboxId::new().to_string(); + seed_observation( + &harness.pool, + node_only.parse().expect("sandbox id"), + node_id, + Some("py"), + Some("1.0.0"), + ) + .await?; + let with_node_only = |rows: &[String]| { + let mut rows = rows.to_vec(); + rows.push(node_only.clone()); + rows + }; + let org = owner.org_id.as_uuid(); + + for (scope, first_page, walk) in [ + ( + format!("org_id={org}&"), + seeded[..2].to_vec(), + seeded.clone(), + ), + (String::new(), with_node_only(&seeded[..2]), { + let mut walk = with_node_only(&seeded[..2]); + walk.extend_from_slice(&seeded[2..]); + walk + }), + ] { + let (status, unpaged) = harness.get(&format!("/sandboxes?{scope}limit=2")).await; + assert_eq!(status, StatusCode::OK, "{scope}: {unpaged}"); + assert_eq!(ids(&unpaged), first_page, "{scope}"); + + let mut walked = Vec::new(); + let mut cursor = String::new(); + let mut pages = 0; + loop { + let (status, page) = harness + .get(&format!("/sandboxes?{scope}limit=2&cursor={cursor}")) + .await; + assert_eq!(status, StatusCode::OK, "{scope}: {page}"); + let rows = ids(&page["sandboxes"]); + if pages == 0 { + assert_eq!( + rows, first_page, + "{scope}: the first page is the unpaged list" + ); + } + walked.extend(rows); + pages += 1; + match page["next_cursor"].as_str() { + Some(next) => cursor = next.to_string(), + None => break, + } + } + assert_eq!(pages, 3, "{scope}"); + assert_eq!(walked, walk, "{scope}"); + } + + let (status, filtered) = harness.get("/sandboxes?lifecycle=started&cursor=").await; + assert_eq!(status, StatusCode::OK, "{filtered}"); + assert_eq!(ids(&filtered["sandboxes"]), seeded); + assert_eq!(filtered["next_cursor"], Value::Null); + db.pool.close().await; + Ok(()) +} + +#[tokio::test] +async fn malformed_list_parameters_answer_400() -> TestDbResult<()> { + let db = pg_test_db().await?; + let harness = harness(db.pool.clone()).await?; + let node = NodeId::new(); + for (query, needle) in [ + ("cursor=bm90LWEtY3Vyc29y", "invalid cursor"), + ("cursor=%%%", "invalid"), + ("lifecycle=started,napping", "unknown lifecycle"), + ("lifecycle=", "unknown lifecycle"), + ("template=..", "invalid template"), + (&format!("node_id={node}&assigned=false"), "assigned=false"), + ("created_after=yesterday", "created_after"), + ( + "created_after=2026-01-02T00:00:00Z&created_before=2026-01-01T00:00:00Z", + "earlier", + ), + ] { + let (status, body) = harness.get(&format!("/sandboxes?{query}")).await; + assert_eq!(status, StatusCode::BAD_REQUEST, "{query}: {body}"); + let message = body["error"] + .as_str() + .unwrap_or(body.as_str().unwrap_or_default()); + assert!(message.contains(needle), "{query}: {body}"); + } + db.pool.close().await; + Ok(()) +} + +#[tokio::test] +async fn the_list_and_the_count_require_a_managing_org_admin() -> TestDbResult<()> { + let db = pg_test_db().await?; + let harness = harness(db.pool.clone()).await?; + for path in [ + "/sandboxes", + "/sandboxes?cursor=", + "/sandboxes/spaced-versions", + ] { + let (status, body) = harness + .request(&harness.member_token, Method::GET, path, None) + .await; + assert_eq!(status, StatusCode::FORBIDDEN, "{path}: {body}"); + } + db.pool.close().await; + Ok(()) +} + +/// The count is its own route, not a sandbox ID, and answers the store's +/// rows for one org on request. +#[tokio::test] +async fn the_spaced_version_count_is_served() -> TestDbResult<()> { + let db = pg_test_db().await?; + let harness = harness(db.pool.clone()).await?; + let owner = seed_owner(&harness.pool).await?; + let node_id = NodeId::new(); + insert_ready_worker_node(&harness.pool, node_id).await?; + let sandbox_id = seed_sandbox( + &harness.pool, + owner, + SeedSandbox { + template: Some("py"), + ..SeedSandbox::default() + }, + ) + .await?; + seed_observation(&harness.pool, sandbox_id, node_id, None, Some("1.4.0")).await?; + let (status, body) = harness + .get(&format!( + "/sandboxes/spaced-versions?org_id={}", + owner.org_id.as_uuid() + )) + .await; + assert_eq!(status, StatusCode::OK, "{body}"); + assert_eq!( + body, + json!({"counts": [ + {"template": "py", "spaced_version": "1.4.0", "lifecycle": "started", "count": 1} + ]}) + ); + let (status, body) = harness.get("/sandboxes/spaced-versions?template=py").await; + assert_eq!(status, StatusCode::BAD_REQUEST, "{body}"); + db.pool.close().await; + Ok(()) +} + +/// Every admin sandbox route and the debug route resolve each ID form to the +/// same sandbox, and refuse an unknown form with 400. +#[tokio::test] +async fn every_admin_sandbox_route_takes_every_id_form() -> TestDbResult<()> { + let db = pg_test_db().await?; + let harness = harness(db.pool.clone()).await?; + let owner = seed_owner(&harness.pool).await?; + let sandbox_id = seed_sandbox(&harness.pool, owner, SeedSandbox::default()).await?; + let public_id = PublicSandboxId::from(sandbox_id).to_string(); + let undashed = sandbox_id.as_uuid().simple().to_string(); + let forms = [ + public_id.clone(), + sandbox_id.to_string(), + sandbox_id.to_string().to_uppercase(), + undashed.clone(), + undashed.to_uppercase(), + format!("space-sbx-{undashed}-g1"), + format!("space-sbx-{undashed}"), + ]; + for form in &forms { + let (status, body) = harness.get(&format!("/sandboxes/{form}")).await; + assert_eq!(status, StatusCode::OK, "GET {form}: {body}"); + assert_eq!(body["id"], sandbox_id.to_string(), "GET {form}"); + + let (status, body) = harness.get(&format!("/debug/sandbox/{form}")).await; + assert_eq!(status, StatusCode::OK, "debug {form}: {body}"); + + let (status, body) = harness + .request( + &harness.admin_token, + Method::POST, + &format!("/sandboxes/{form}:force-stop"), + None, + ) + .await; + assert_eq!(status, StatusCode::ACCEPTED, "force-stop {form}: {body}"); + assert_eq!(body["sandbox_id"], public_id, "force-stop {form}"); + + let (status, body) = harness + .request( + &harness.admin_token, + Method::PATCH, + &format!("/sandboxes/{form}"), + Some(json!({"metadata": {"listed": "yes"}})), + ) + .await; + assert_eq!(status, StatusCode::ACCEPTED, "PATCH {form}: {body}"); + assert_eq!(body["sandbox_id"], public_id, "PATCH {form}"); + + let (status, body) = harness + .request( + &harness.admin_token, + Method::POST, + &format!("/sandboxes/{form}/commands"), + Some(json!({"command": ["true"]})), + ) + .await; + assert_ne!(status, StatusCode::BAD_REQUEST, "commands {form}: {body}"); + assert_ne!(status, StatusCode::NOT_FOUND, "commands {form}: {body}"); + } + + let unknown = format!("{{{sandbox_id}}}"); + for (method, path, body) in [ + (Method::GET, format!("/sandboxes/{unknown}"), None), + (Method::GET, format!("/debug/sandbox/{unknown}"), None), + ( + Method::POST, + format!("/sandboxes/{unknown}:force-stop"), + None, + ), + ( + Method::PATCH, + format!("/sandboxes/{unknown}"), + Some(json!({"metadata": {}})), + ), + ( + Method::POST, + format!("/sandboxes/{unknown}/commands"), + Some(json!({"command": ["true"]})), + ), + ] { + let (status, response) = harness + .request(&harness.admin_token, method.clone(), &path, body) + .await; + assert_eq!( + status, + StatusCode::BAD_REQUEST, + "{method} {path}: {response}" + ); + } + db.pool.close().await; + Ok(()) +} diff --git a/crates/libs/api/src/routes/admin/sandbox_store.rs b/crates/libs/api/src/routes/admin/sandbox_store.rs new file mode 100644 index 0000000000..4fcbc29769 --- /dev/null +++ b/crates/libs/api/src/routes/admin/sandbox_store.rs @@ -0,0 +1,228 @@ +//! Postgres-side queries that back the admin sandbox list and the +//! spaced-version count. +//! +//! The handlers in [`super::sandbox_list`] stay focused on HTTP: parsing the +//! query string, auth, and the response shape. The statements live here. + +use std::sync::LazyLock; + +use pplx_space_common_core::sql; +use pplx_space_common_core::types::{NodeId, OrgId, SandboxId, TemplateName, UserId}; +use pplx_space_placement::StoredSandboxLifecycle; +use pplx_space_public_api::PUBLIC_SANDBOX_ID_PREFIX; +use serde::Serialize; +use sqlx::{FromRow, PgExecutor}; +use time::OffsetDateTime; +use uuid::Uuid; + +use crate::controllers::sandbox::{SandboxPageCursor, sandbox_query}; +use crate::models::Sandbox; + +/// What the admin sandbox list narrows to. Every field that is set must +/// match; an unset field matches every row. +#[derive(Debug, Clone, Default)] +pub(super) struct AdminSandboxFilter { + pub(super) org_id: Option, + /// The stored lifecycle (`sandboxes.lifecycle`), any of these. Never + /// empty. + pub(super) lifecycles: Option>, + /// The row's own template (`sandboxes.template_id`). + pub(super) template: Option, + pub(super) node: Option, + pub(super) user_id: Option, + /// Created strictly after this instant. + pub(super) created_after: Option, + /// Created strictly before this instant. + pub(super) created_before: Option, + pub(super) search: Option, +} + +impl AdminSandboxFilter { + /// Whether no filter is set. Only the unfiltered list carries the + /// node-reported rows with no sandbox row: they belong to no org, and + /// none of the other filters can be applied to them. + pub(super) fn is_unfiltered(&self) -> bool { + let Self { + org_id, + lifecycles, + template, + node, + user_id, + created_after, + created_before, + search, + } = self; + org_id.is_none() + && lifecycles.is_none() + && template.is_none() + && node.is_none() + && user_id.is_none() + && created_after.is_none() + && created_before.is_none() + && search.is_none() + } +} + +/// The node a sandbox is assigned to, as the list response reports it in +/// `active_node_id`. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(super) enum NodeAssignment { + Node(NodeId), + Assigned, + Unassigned, +} + +/// A name or ID prefix. A row matches when its name starts with the prefix +/// as given, or when the prefix, with an optional `sbx_` and dashes where a +/// UUID has them, is the start of its ID. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(super) struct SandboxSearch { + prefix: String, + /// The lowest and highest IDs that start with the prefix, when it can + /// start an ID. Postgres compares UUIDs byte by byte, so a hex prefix is + /// exactly this range, which the primary key serves. + id_range: Option<(SandboxId, SandboxId)>, +} + +impl SandboxSearch { + pub(super) fn new(prefix: String) -> Self { + let id_range = id_prefix_range(&prefix); + Self { prefix, id_range } + } +} + +const DASHED_UUID_DASH_POSITIONS: [usize; 4] = [8, 13, 18, 23]; +const UNDASHED_UUID_LEN: usize = 32; + +fn id_prefix_range(prefix: &str) -> Option<(SandboxId, SandboxId)> { + let id = prefix + .strip_prefix(PUBLIC_SANDBOX_ID_PREFIX) + .unwrap_or(prefix); + let dashed = id.contains('-'); + let mut hex = String::with_capacity(UNDASHED_UUID_LEN); + for (position, character) in id.chars().enumerate() { + if dashed && DASHED_UUID_DASH_POSITIONS.contains(&position) { + if character != '-' { + return None; + } + } else if character.is_ascii_hexdigit() { + hex.push(character.to_ascii_lowercase()); + } else { + return None; + } + } + if hex.len() > UNDASHED_UUID_LEN { + return None; + } + let bound = |fill: char| { + let mut digits = hex.clone(); + digits.extend(std::iter::repeat_n(fill, UNDASHED_UUID_LEN - hex.len())); + Uuid::try_parse(&digits).ok().map(SandboxId::from_uuid) + }; + Some((bound('0')?, bound('f')?)) +} + +/// The admin list: newest first by `(created_at, id)`, after `after` when +/// set. `active_node_id` is the response's: the direct route's current node +/// when the sandbox has one, else the row's. +static SANDBOX_PAGE_SQL: LazyLock = LazyLock::new(|| { + sandbox_query( + "SELECT {sandbox_columns} + FROM {sandbox_from} + WHERE ($2::uuid IS NULL OR s.org_id = $2) + AND (s.created_at, s.id) < (COALESCE($3::TIMESTAMPTZ, 'infinity'), + COALESCE($4::UUID, '00000000-0000-0000-0000-000000000000')) + AND ($5::TEXT[] IS NULL OR s.lifecycle = ANY($5::TEXT[])) + AND ($6::TEXT IS NULL OR s.template_id = $6) + AND ($7::UUID IS NULL OR COALESCE(direct_route.current_node_id, s.active_node_id) = $7) + AND ($8::BOOLEAN IS NULL + OR (COALESCE(direct_route.current_node_id, s.active_node_id) IS NOT NULL) = $8) + AND ($9::UUID IS NULL OR s.user_id = $9) + AND ($10::TIMESTAMPTZ IS NULL OR s.created_at > $10) + AND ($11::TIMESTAMPTZ IS NULL OR s.created_at < $11) + AND ($12::TEXT IS NULL + OR starts_with(s.name, $12) + OR s.id BETWEEN $13::UUID AND $14::UUID) + ORDER BY s.created_at DESC, s.id DESC + LIMIT $1", + ) +}); + +/// At most `max_rows` sandboxes matching `filter`, newest first, after the +/// cursor when one is set. +pub(super) async fn list_sandboxes( + executor: impl PgExecutor<'_>, + filter: &AdminSandboxFilter, + after: Option, + max_rows: i64, +) -> Result, sqlx::Error> { + let lifecycles = filter.lifecycles.as_ref().map(|lifecycles| { + lifecycles + .iter() + .map(|lifecycle| lifecycle.as_str()) + .collect::>() + }); + let (node_id, assigned) = match filter.node { + Some(NodeAssignment::Node(node_id)) => (Some(node_id), None), + Some(NodeAssignment::Assigned) => (None, Some(true)), + Some(NodeAssignment::Unassigned) => (None, Some(false)), + None => (None, None), + }; + let id_range = filter.search.as_ref().and_then(|search| search.id_range); + sqlx::query_as::<_, Sandbox>(sqlx::AssertSqlSafe(sql!(SANDBOX_PAGE_SQL.as_str()))) + .bind(max_rows) + .bind(filter.org_id) + .bind(after.map(|cursor| cursor.created_at)) + .bind(after.map(|cursor| cursor.id)) + .bind(lifecycles) + .bind(filter.template.as_ref().map(TemplateName::as_str)) + .bind(node_id) + .bind(assigned) + .bind(filter.user_id) + .bind(filter.created_after) + .bind(filter.created_before) + .bind(filter.search.as_ref().map(|search| search.prefix.as_str())) + .bind(id_range.map(|(low, _)| low)) + .bind(id_range.map(|(_, high)| high)) + .fetch_all(executor) + .await +} + +/// How many sandboxes run each template at each `spaced` version, by stored +/// lifecycle. +#[derive(Debug, Clone, PartialEq, Eq, FromRow, Serialize)] +pub(super) struct SpacedVersionCount { + /// The row's template, else the one its node reports. + pub(super) template: Option, + /// The version its node last reported; `None` before any report. + pub(super) spaced_version: Option, + pub(super) lifecycle: String, + pub(super) count: i64, +} + +/// [`SpacedVersionCount`]s over every sandbox not deleted, in `org_id` when +/// set. +pub(super) async fn count_by_spaced_version( + executor: impl PgExecutor<'_>, + org_id: Option, +) -> Result, sqlx::Error> { + sqlx::query_as::<_, SpacedVersionCount>(sql!( + "SELECT COALESCE(s.template_id, sos.template, sos.pool_template) AS template, + sos.spaced_version, + s.lifecycle, + COUNT(*)::BIGINT AS count + FROM sandboxes s + LEFT JOIN sandbox_observed_states sos ON sos.sandbox_id = s.id + WHERE s.lifecycle <> $1 + AND ($2::UUID IS NULL OR s.org_id = $2) + GROUP BY 1, 2, 3 + ORDER BY 1 NULLS LAST, 2 NULLS LAST, 3" + )) + .bind(StoredSandboxLifecycle::Deleted.as_str()) + .bind(org_id) + .fetch_all(executor) + .await +} + +#[cfg(test)] +pub(super) mod tests; diff --git a/crates/libs/api/src/routes/admin/sandbox_store/tests.rs b/crates/libs/api/src/routes/admin/sandbox_store/tests.rs new file mode 100644 index 0000000000..105c34f26a --- /dev/null +++ b/crates/libs/api/src/routes/admin/sandbox_store/tests.rs @@ -0,0 +1,596 @@ +use pplx_space_common_core::types::SubstrateKind; +use pplx_space_db_test_fixtures::{TestDbResult, insert_ready_worker_node, pg_test_db}; +use sqlx::PgPool; +use time::Duration; + +use super::*; + +/// An org and one of its users, for seeded sandboxes to belong to. +#[derive(Debug, Clone, Copy)] +pub(in crate::routes::admin) struct Owner { + pub(in crate::routes::admin) org_id: OrgId, + pub(in crate::routes::admin) user_id: UserId, +} + +pub(in crate::routes::admin) async fn seed_org(pool: &PgPool) -> TestDbResult { + let id = Uuid::now_v7(); + sqlx::query(sql!( + "INSERT INTO organizations (id, name, slug, byok_enabled) + VALUES ($1, $2, $3, false)" + )) + .bind(id) + .bind(format!("Org {id}")) + .bind(format!("org-{}", id.simple())) + .execute(pool) + .await?; + Ok(OrgId::from_uuid(id)) +} + +pub(in crate::routes::admin) async fn seed_user( + pool: &PgPool, + org_id: OrgId, +) -> TestDbResult { + let user_id = UserId::new(); + sqlx::query(sql!( + "INSERT INTO users (id, email, password_hash, display_name, org_id, role) + VALUES ($1, $2, 'synthetic-password-hash', 'Sandbox list tests', $3, 'member')" + )) + .bind(user_id) + .bind(format!("{user_id}@example.invalid")) + .bind(org_id) + .execute(pool) + .await?; + Ok(Owner { org_id, user_id }) +} + +pub(in crate::routes::admin) async fn seed_owner(pool: &PgPool) -> TestDbResult { + let org_id = seed_org(pool).await?; + seed_user(pool, org_id).await +} + +/// A sandbox row to seed; unset fields take the column defaults. +#[derive(Debug, Clone, Default)] +pub(in crate::routes::admin) struct SeedSandbox { + pub(in crate::routes::admin) id: Option, + pub(in crate::routes::admin) name: Option<&'static str>, + pub(in crate::routes::admin) lifecycle: Option<&'static str>, + pub(in crate::routes::admin) template: Option<&'static str>, + pub(in crate::routes::admin) active_node_id: Option, + pub(in crate::routes::admin) created_at: Option, +} + +pub(in crate::routes::admin) async fn seed_sandbox( + pool: &PgPool, + owner: Owner, + seed: SeedSandbox, +) -> TestDbResult { + let id = seed.id.unwrap_or_default(); + sqlx::query(sql!( + "INSERT INTO sandboxes ( + id, name, user_id, org_id, managed_by, lifecycle, placement, + network_policy_generation, template_id, active_node_id, created_at + ) + VALUES ($1, $2, $3, $4, $5, COALESCE($6, 'started'), 'placed', 1, $7, $8, + COALESCE($9, NOW()))" + )) + .bind(id) + .bind(seed.name) + .bind(owner.user_id) + .bind(owner.org_id) + .bind(SubstrateKind::Kubernetes) + .bind(seed.lifecycle) + .bind(seed.template) + .bind(seed.active_node_id) + .bind(seed.created_at) + .execute(pool) + .await?; + Ok(id) +} + +/// What a node reports for `sandbox_id`, which need not have a sandbox row. +pub(in crate::routes::admin) async fn seed_observation( + pool: &PgPool, + sandbox_id: SandboxId, + node_id: NodeId, + template: Option<&str>, + spaced_version: Option<&str>, +) -> TestDbResult<()> { + sqlx::query(sql!( + "INSERT INTO sandbox_observed_states (sandbox_id, node_id, status, template, spaced_version) + VALUES ($1, $2, 'started', $3, $4)" + )) + .bind(sandbox_id) + .bind(node_id) + .bind(template) + .bind(spaced_version) + .execute(pool) + .await?; + Ok(()) +} + +fn base_time() -> OffsetDateTime { + OffsetDateTime::from_unix_timestamp(1_790_000_000).expect("fixture timestamp") +} + +fn cursor_after(sandbox: &Sandbox) -> SandboxPageCursor { + SandboxPageCursor { + created_at: sandbox.created_at, + id: sandbox.id, + } +} + +/// Walking the list a page at a time returns every row that existed when the +/// walk began exactly once, in list order, although rows sharing a +/// `created_at` straddle page boundaries and new rows land between pages. +#[tokio::test] +async fn pages_return_every_row_once_while_rows_are_inserted() -> TestDbResult<()> { + const PAGE: usize = 2; + let db = pg_test_db().await?; + let owner = seed_owner(&db.pool).await?; + let mut existing = Vec::new(); + for index in 0..9 { + // Three rows per timestamp, so the order between them is the id's. + let created_at = base_time() - Duration::minutes(index / 3); + let id = seed_sandbox( + &db.pool, + owner, + SeedSandbox { + created_at: Some(created_at), + ..SeedSandbox::default() + }, + ) + .await?; + existing.push((created_at, id)); + } + existing.sort_unstable_by(|left, right| right.cmp(left)); + + let filter = AdminSandboxFilter { + org_id: Some(owner.org_id), + ..AdminSandboxFilter::default() + }; + let mut seen = Vec::new(); + let mut after = None; + loop { + let mut page = list_sandboxes(&db.pool, &filter, after, PAGE as i64 + 1).await?; + seed_sandbox(&db.pool, owner, SeedSandbox::default()).await?; + let more = page.len() > PAGE; + page.truncate(PAGE); + seen.extend(page.iter().map(|sandbox| sandbox.id)); + if !more { + break; + } + after = page.last().map(cursor_after); + } + let expected = existing.iter().map(|(_, id)| *id).collect::>(); + assert_eq!(seen, expected); + db.pool.close().await; + Ok(()) +} + +/// With no filter beyond `org_id` and no cursor, the list is the rows the +/// route's statement returned before paging and filters existed. +#[tokio::test] +async fn the_first_page_is_the_list_from_before_paging() -> TestDbResult<()> { + let db = pg_test_db().await?; + let first = seed_owner(&db.pool).await?; + let second = seed_owner(&db.pool).await?; + for (index, owner) in [first, second, first, second, first] + .into_iter() + .enumerate() + { + seed_sandbox( + &db.pool, + owner, + SeedSandbox { + created_at: Some(base_time() - Duration::seconds(index as i64 / 2)), + ..SeedSandbox::default() + }, + ) + .await?; + } + for org_id in [None, Some(first.org_id), Some(second.org_id)] { + for limit in [1, 3, 1000] { + let legacy: Vec = sqlx::query_scalar(sql!( + "SELECT s.id + FROM sandboxes s + WHERE ($2::uuid IS NULL OR s.org_id = $2) + ORDER BY s.created_at DESC, s.id DESC + LIMIT $1" + )) + .bind(limit) + .bind(org_id) + .fetch_all(&db.pool) + .await?; + let filter = AdminSandboxFilter { + org_id, + ..AdminSandboxFilter::default() + }; + let listed = list_sandboxes(&db.pool, &filter, None, limit).await?; + let listed = listed.iter().map(|sandbox| sandbox.id).collect::>(); + assert_eq!(listed, legacy, "org {org_id:?}, limit {limit}"); + } + } + db.pool.close().await; + Ok(()) +} + +fn fixed_id(value: &str) -> SandboxId { + SandboxId::from_uuid(Uuid::parse_str(value).expect("fixture uuid")) +} + +/// Each filter, alone and combined, returns exactly the rows it describes. +#[tokio::test] +async fn each_filter_returns_exactly_its_rows() -> TestDbResult<()> { + let db = pg_test_db().await?; + let first_org = seed_org(&db.pool).await?; + let first_user = seed_user(&db.pool, first_org).await?; + let second_user = seed_user(&db.pool, first_org).await?; + let other_org = seed_owner(&db.pool).await?; + let (node_one, node_two) = (NodeId::new(), NodeId::new()); + let at = |minutes: i64| base_time() + Duration::minutes(minutes); + let rows = [ + ( + first_user, + SeedSandbox { + id: Some(fixed_id("10000000-0000-7000-8000-000000000000")), + name: Some("alpha-one"), + lifecycle: Some("started"), + template: Some("py"), + active_node_id: Some(node_one), + created_at: Some(at(0)), + }, + ), + ( + first_user, + SeedSandbox { + id: Some(fixed_id("10000000-0000-7000-8000-000000000001")), + name: Some("alpha-two"), + lifecycle: Some("paused"), + template: Some("py"), + active_node_id: Some(node_two), + created_at: Some(at(1)), + }, + ), + ( + second_user, + SeedSandbox { + id: Some(fixed_id("2abc0000-0000-7000-8000-000000000000")), + name: Some("beta"), + lifecycle: Some("suspended"), + template: Some("node"), + active_node_id: None, + created_at: Some(at(2)), + }, + ), + ( + first_user, + SeedSandbox { + id: Some(fixed_id("30000000-0000-7000-8000-000000000000")), + name: None, + lifecycle: Some("deleted"), + template: Some("py"), + active_node_id: None, + created_at: Some(at(3)), + }, + ), + ( + other_org, + SeedSandbox { + id: Some(fixed_id("40000000-0000-7000-8000-000000000000")), + name: Some("gamma"), + lifecycle: Some("started"), + template: None, + active_node_id: Some(node_one), + created_at: Some(at(4)), + }, + ), + ( + second_user, + SeedSandbox { + id: Some(fixed_id("50000000-0000-7000-8000-000000000000")), + name: Some("Alpha-caps"), + lifecycle: Some("cleanup_required"), + template: Some("node"), + active_node_id: Some(node_two), + created_at: Some(at(5)), + }, + ), + ]; + let mut ids = Vec::new(); + for (owner, seed) in rows { + ids.push(seed_sandbox(&db.pool, owner, seed).await?); + } + let lifecycles = |values: &[&str]| { + Some( + values + .iter() + .map(|value| value.parse().expect("stored lifecycle")) + .collect(), + ) + }; + let search = |prefix: &str| Some(SandboxSearch::new(prefix.to_string())); + let template = |name: &str| Some(TemplateName::parse(name).expect("template name")); + let cases: Vec<(&str, AdminSandboxFilter, &[usize])> = vec![ + ( + "no filter", + AdminSandboxFilter::default(), + &[0, 1, 2, 3, 4, 5], + ), + ( + "org", + AdminSandboxFilter { + org_id: Some(other_org.org_id), + ..AdminSandboxFilter::default() + }, + &[4], + ), + ( + "one lifecycle", + AdminSandboxFilter { + lifecycles: lifecycles(&["started"]), + ..AdminSandboxFilter::default() + }, + &[0, 4], + ), + ( + "two lifecycles", + AdminSandboxFilter { + lifecycles: lifecycles(&["paused", "suspended"]), + ..AdminSandboxFilter::default() + }, + &[1, 2], + ), + ( + "tombstones", + AdminSandboxFilter { + lifecycles: lifecycles(&["deleted", "cleanup_required"]), + ..AdminSandboxFilter::default() + }, + &[3, 5], + ), + ( + "template", + AdminSandboxFilter { + template: template("py"), + ..AdminSandboxFilter::default() + }, + &[0, 1, 3], + ), + ( + "node", + AdminSandboxFilter { + node: Some(NodeAssignment::Node(node_one)), + ..AdminSandboxFilter::default() + }, + &[0, 4], + ), + ( + "assigned", + AdminSandboxFilter { + node: Some(NodeAssignment::Assigned), + ..AdminSandboxFilter::default() + }, + &[0, 1, 4, 5], + ), + ( + "unassigned", + AdminSandboxFilter { + node: Some(NodeAssignment::Unassigned), + ..AdminSandboxFilter::default() + }, + &[2, 3], + ), + ( + "user", + AdminSandboxFilter { + user_id: Some(first_user.user_id), + ..AdminSandboxFilter::default() + }, + &[0, 1, 3], + ), + ( + "created after, exclusive", + AdminSandboxFilter { + created_after: Some(at(2)), + ..AdminSandboxFilter::default() + }, + &[3, 4, 5], + ), + ( + "created before, exclusive", + AdminSandboxFilter { + created_before: Some(at(2)), + ..AdminSandboxFilter::default() + }, + &[0, 1], + ), + ( + "created between", + AdminSandboxFilter { + created_after: Some(at(0)), + created_before: Some(at(3)), + ..AdminSandboxFilter::default() + }, + &[1, 2], + ), + ( + "name prefix is case-sensitive", + AdminSandboxFilter { + search: search("alpha"), + ..AdminSandboxFilter::default() + }, + &[0, 1], + ), + ( + "public id prefix", + AdminSandboxFilter { + search: search("sbx_2abc"), + ..AdminSandboxFilter::default() + }, + &[2], + ), + ( + "dashed id prefix in upper case", + AdminSandboxFilter { + search: search("2ABC0000-00"), + ..AdminSandboxFilter::default() + }, + &[2], + ), + ( + "undashed id prefix", + AdminSandboxFilter { + search: search("100000000000700080000000000000"), + ..AdminSandboxFilter::default() + }, + &[0, 1], + ), + ( + "whole id", + AdminSandboxFilter { + search: search("10000000-0000-7000-8000-000000000001"), + ..AdminSandboxFilter::default() + }, + &[1], + ), + ( + "no match", + AdminSandboxFilter { + search: search("zeta"), + ..AdminSandboxFilter::default() + }, + &[], + ), + ( + "combined", + AdminSandboxFilter { + org_id: Some(first_org), + lifecycles: lifecycles(&["started", "deleted"]), + template: template("py"), + node: Some(NodeAssignment::Unassigned), + ..AdminSandboxFilter::default() + }, + &[3], + ), + ]; + for (case, filter, expected) in cases { + let listed = list_sandboxes(&db.pool, &filter, None, 100).await?; + let listed = listed.iter().map(|sandbox| sandbox.id).collect::>(); + // Newest first, so the expected rows in reverse seed order. + let expected = expected + .iter() + .rev() + .map(|index| ids[*index]) + .collect::>(); + assert_eq!(listed, expected, "{case}"); + } + db.pool.close().await; + Ok(()) +} + +#[test] +fn an_id_prefix_is_the_range_of_ids_it_starts() { + let range = |low: &str, high: &str| Some((fixed_id(low), fixed_id(high))); + let cases = [ + ( + "", + range( + "00000000-0000-0000-0000-000000000000", + "ffffffff-ffff-ffff-ffff-ffffffffffff", + ), + ), + ( + "sbx_AB", + range( + "ab000000-0000-0000-0000-000000000000", + "abffffff-ffff-ffff-ffff-ffffffffffff", + ), + ), + ( + "0123abcd-ef", + range( + "0123abcd-ef00-0000-0000-000000000000", + "0123abcd-efff-ffff-ffff-ffffffffffff", + ), + ), + ( + "0123abcdef", + range( + "0123abcd-ef00-0000-0000-000000000000", + "0123abcd-efff-ffff-ffff-ffffffffffff", + ), + ), + ("0123-abcd", None), + ("0123abcd-ef-", None), + ("sandbox", None), + ("0123abcd-ef01-2345-6789-abcdef0123456", None), + ("0123abcdef0123456789abcdef01234567", None), + ]; + for (prefix, expected) in cases { + assert_eq!(id_prefix_range(prefix), expected, "{prefix:?}"); + } +} + +/// A node report seeded with a row: its template and `spaced` version. +type SeedObservation = (Option<&'static str>, &'static str); + +/// The count groups every sandbox that is not deleted by template, reported +/// `spaced` version and stored lifecycle, and narrows to one org on request. +#[tokio::test] +async fn sandboxes_count_by_template_version_and_lifecycle() -> TestDbResult<()> { + let db = pg_test_db().await?; + let owner = seed_owner(&db.pool).await?; + let other = seed_owner(&db.pool).await?; + let node_id = NodeId::new(); + insert_ready_worker_node(&db.pool, node_id).await?; + let rows: [(Owner, Option<&str>, &str, Option); 6] = [ + (owner, Some("py"), "started", Some((None, "1.2.0"))), + ( + owner, + Some("py"), + "started", + Some((Some("ignored"), "1.2.0")), + ), + (owner, Some("py"), "suspended", None), + (owner, None, "started", Some((Some("observed"), "1.3.0"))), + (owner, Some("py"), "deleted", Some((None, "1.2.0"))), + (other, Some("py"), "started", Some((None, "1.2.0"))), + ]; + for (row_owner, template, lifecycle, observed) in rows { + let id = seed_sandbox( + &db.pool, + row_owner, + SeedSandbox { + template, + lifecycle: Some(lifecycle), + ..SeedSandbox::default() + }, + ) + .await?; + if let Some((observed_template, version)) = observed { + seed_observation(&db.pool, id, node_id, observed_template, Some(version)).await?; + } + } + let count = + |template: &str, version: Option<&str>, lifecycle: &str, count| SpacedVersionCount { + template: Some(template.to_string()), + spaced_version: version.map(str::to_string), + lifecycle: lifecycle.to_string(), + count, + }; + let everywhere = count_by_spaced_version(&db.pool, None).await?; + assert_eq!( + everywhere, + [ + count("observed", Some("1.3.0"), "started", 1), + count("py", Some("1.2.0"), "started", 3), + count("py", None, "suspended", 1), + ] + ); + let in_other = count_by_spaced_version(&db.pool, Some(other.org_id)).await?; + assert_eq!(in_other, [count("py", Some("1.2.0"), "started", 1)]); + let none_seen = count_by_spaced_version(&db.pool, Some(seed_org(&db.pool).await?)).await?; + assert!(none_seen.is_empty()); + db.pool.close().await; + Ok(()) +} diff --git a/crates/libs/api/src/routes/admin/sandboxes.rs b/crates/libs/api/src/routes/admin/sandboxes.rs index a96b601aef..f8d61d72a3 100644 --- a/crates/libs/api/src/routes/admin/sandboxes.rs +++ b/crates/libs/api/src/routes/admin/sandboxes.rs @@ -3,7 +3,7 @@ use std::str::FromStr; use axum::{ Extension, Json, - extract::{FromRequest, Path, Query, Request, State, rejection::JsonRejection}, + extract::{FromRequest, Path, Request, State, rejection::JsonRejection}, http::{HeaderMap, StatusCode}, response::IntoResponse, }; @@ -21,6 +21,7 @@ use serde::Deserialize; use sha2::{Digest, Sha256}; use super::audit::{self, AdminSandboxAuditEntry}; +use super::sandbox_id::{AdminSandboxIdError, parse_admin_sandbox_id}; use crate::middleware::auth::AuthUser; use crate::models::{Sandbox, SandboxResponse, User}; use crate::public_operations::{ @@ -41,8 +42,6 @@ use crate::routes::public::sessions::{ }; const ADMIN_SANDBOX_ACTION_PREFIX: &str = "force-"; -const DEFAULT_ADMIN_SANDBOX_LIMIT: u32 = 1000; -const MAX_ADMIN_SANDBOX_LIMIT: u32 = 5000; /// Distinct from the public `/v1/sandboxes` template so admin and public /// idempotency records never collide on the same key. const ADMIN_CREATE_SANDBOX_PATH_TEMPLATE: &str = "/v1/sandboxes:admin-create"; @@ -64,73 +63,11 @@ impl AdminSandboxTarget { domain_id: public_id.into(), } } -} - -#[derive(Debug, Deserialize)] -pub struct AdminSandboxListQuery { - limit: Option, - org_id: Option, -} -pub async fn list_sandboxes( - State(state): State, - Extension(AuthUser(actor)): Extension, - Query(query): Query, -) -> axum::response::Response { - if let Err(err) = require_managing_org_admin(&state.pool, &actor).await { - return err.into_response(); - } - let limit = query - .limit - .unwrap_or(DEFAULT_ADMIN_SANDBOX_LIMIT) - .clamp(1, MAX_ADMIN_SANDBOX_LIMIT); - let sandboxes = match fetch_admin_sandboxes(&state.pool, limit, query.org_id).await { - Ok(sandboxes) => sandboxes, - // `ReadOnly`: the busiest admin read on this surface, and a plain - // `SELECT` over `sandboxes`, so it blocks behind DDL on the hottest - // table in the schema. It wrote nothing, so every contention SQLSTATE - // answers a retryable 503 and none takes the write-specific 409 — see - // `support::contention_disposition`. - Err(error) => { - if let Some((status, message)) = - support::contention_disposition(ContentionScope::ReadOnly, &error) - { - return api_error(status, message).into_response(); - } - tracing::warn!(%error, "admin sandbox list query failed"); - return api_error(StatusCode::INTERNAL_SERVER_ERROR, "Internal server error") - .into_response(); - } - }; - // Rowless warm-pool slots and observed-local sandboxes have no org, so - // only the unscoped list carries them. - let include_observed_only = query.org_id.is_none(); - match state - .sandbox_controller - .list_responses(&state.pool, &sandboxes, include_observed_only) - .await - { - Ok(response) => Json(response).into_response(), - // A known remainder, in two halves, neither of which is useful - // without the other. - // - // `list_responses` reaches `responses::fetch_sandbox_observed_states` - // and `fetch_observed_only_sandboxes`, both of which erase the - // SQLSTATE into `ControllerError::internal` — so no contention - // identity exists to honour here yet. And this arm flattens the - // `ControllerError` to a 500 regardless of the status it carried, so - // classifying those reads alone would not surface it either. - // - // Adding an `is_lock_contention()` check here today would be a guard - // for a state nothing produces. Projection needs to preserve the - // database error identity, and this handler must also preserve the - // resulting status. These are independent fixes: classification - // recovers contention identity, while flattening discards every status, - // not just contention. Neither alone makes this path retryable. - // Same shape on `get_sandbox` below. - Err(error) => super::support::controller_read_error(&error, || { - tracing::warn!(%error, "admin sandbox response projection failed"); - }), + /// The sandbox an admin route's path names, in any form + /// [`parse_admin_sandbox_id`] accepts. + fn parse(spec: &str) -> Result { + parse_admin_sandbox_id(spec).map(|id| Self::from_public_id(id.into())) } } @@ -142,26 +79,19 @@ pub async fn get_sandbox( if let Err(err) = require_managing_org_admin(&state.pool, &actor).await { return err.into_response(); } - let public_id = match PublicSandboxId::from_str(&spec) { - Ok(value) => value, - Err(_) => { - return api_error( - StatusCode::BAD_REQUEST, - "sandbox id must be an sbx_ prefixed UUID", - ) - .into_response(); - } + let target = match AdminSandboxTarget::parse(&spec) { + Ok(target) => target, + Err(error) => return api_error(StatusCode::BAD_REQUEST, error.to_string()).into_response(), }; - let target = AdminSandboxTarget::from_public_id(public_id); let response = match fetch_admin_sandbox_response(&state.pool, &state, target).await { Ok(Some(response)) => response, Ok(None) => { return api_error(StatusCode::NOT_FOUND, "Sandbox not found").into_response(); } - // Not classified, for the reason spelled out on `list_sandboxes`'s - // projection arm: this flattens a `ControllerError` rather than - // honouring the status it carries, and the reads behind it do not - // classify yet either. + // Not classified, for the reason spelled out on + // `sandbox_list::list_sandboxes`'s projection arm: this flattens a + // `ControllerError` rather than honouring the status it carries, and + // the reads behind it do not classify yet either. Err(error) => { return super::support::controller_read_error(&error, || { tracing::warn!( @@ -552,9 +482,8 @@ pub async fn create_command( Path(spec): Path, Json(request): Json, ) -> Result<(StatusCode, Json), PublicApiError> { - let public_id = PublicSandboxId::from_str(&spec) - .map_err(|_| PublicApiError::bad_request("sandbox id must be an sbx_ prefixed UUID"))?; - let target = AdminSandboxTarget::from_public_id(public_id); + let target = AdminSandboxTarget::parse(&spec) + .map_err(|error| PublicApiError::bad_request(error.to_string()))?; let sandbox = fetch_admin_sandbox(&state.pool, target) .await // `From for PublicApiError` classifies contention (and @@ -571,7 +500,7 @@ pub async fn create_command( &state.public_session_store(), sandbox.org_id, actor.id, - public_id, + target.public_id, request, idempotency_key, || Ok(()), @@ -599,17 +528,13 @@ pub async fn update_sandbox( if let Err(err) = require_managing_org_admin(&state.pool, &actor).await { return err.into_response(); } - let public_id = match PublicSandboxId::from_str(&spec) { - Ok(value) => value, - Err(_) => { - return AdminSandboxActionError::new( - StatusCode::BAD_REQUEST, - "sandbox id must be an sbx_ prefixed UUID", - ) - .into_response(); + let target = match AdminSandboxTarget::parse(&spec) { + Ok(target) => target, + Err(error) => { + return AdminSandboxActionError::new(StatusCode::BAD_REQUEST, error.to_string()) + .into_response(); } }; - let target = AdminSandboxTarget::from_public_id(public_id); if request .resources .as_ref() @@ -900,25 +825,6 @@ async fn fetch_admin_sandbox( crate::controllers::sandbox::fetch_sandbox(pool, target.domain_id).await } -async fn fetch_admin_sandboxes( - pool: &sqlx::PgPool, - limit: u32, - org_id: Option, -) -> Result, sqlx::Error> { - let sql = crate::controllers::sandbox::sandbox_query( - "SELECT {sandbox_columns} - FROM {sandbox_from} - WHERE ($2::uuid IS NULL OR s.org_id = $2) - ORDER BY s.created_at DESC, s.id DESC - LIMIT $1", - ); - sqlx::query_as::<_, Sandbox>(sqlx::AssertSqlSafe(sql!(sql.as_str()))) - .bind(i64::from(limit)) - .bind(org_id.map(OrgId::as_uuid)) - .fetch_all(pool) - .await -} - async fn fetch_admin_sandbox_response( pool: &sqlx::PgPool, state: &AppState, @@ -956,11 +862,8 @@ fn parse_spec( "invalid sandbox action path", )); } - let sandbox_id = PublicSandboxId::from_str(sandbox_id).map_err(|_| { - AdminSandboxActionError::new( - StatusCode::BAD_REQUEST, - "sandbox id must be an sbx_ prefixed UUID", - ) + let target = AdminSandboxTarget::parse(sandbox_id).map_err(|error| { + AdminSandboxActionError::new(StatusCode::BAD_REQUEST, error.to_string()) })?; let Some(action) = action.strip_prefix(ADMIN_SANDBOX_ACTION_PREFIX) else { return Err(AdminSandboxActionError::new( @@ -971,7 +874,7 @@ fn parse_spec( let action = SandboxLifecycleAction::parse_known(action).map_err(|_| { AdminSandboxActionError::new(StatusCode::NOT_FOUND, "unknown admin sandbox action") })?; - Ok((AdminSandboxTarget::from_public_id(sandbox_id), action)) + Ok((target, action)) } fn idempotency_key(headers: &HeaderMap) -> Result { diff --git a/crates/libs/api/src/routes/admin/sandboxes/tests.rs b/crates/libs/api/src/routes/admin/sandboxes/tests.rs index 05f645a2af..7b9d0187a5 100644 --- a/crates/libs/api/src/routes/admin/sandboxes/tests.rs +++ b/crates/libs/api/src/routes/admin/sandboxes/tests.rs @@ -293,105 +293,6 @@ async fn admin_update_enqueues_under_sandbox_org_for_cross_org_caller() -> TestR Ok(()) } -/// An org-scoped list holds that org's sandboxes only. Rows nodes report -/// without a database row (rowless pool slots, observed-local sandboxes) -/// belong to no org, so only the unscoped list carries them. -#[tokio::test(flavor = "current_thread")] -async fn org_scoped_list_leaves_out_observed_only_rows() -> TestResult { - // `AppState::from_pool` picks its substrate from the environment; keep it - // Direct so construction never reaches a cluster. - if !pplx_space_test_env::enter_test_process(&[( - "SPACE_K8S_CONTROL_PLANE", - Some(std::ffi::OsStr::new("false")), - )]) { - return Ok(()); - } - let Some(db) = try_pg_test_db().await? else { - return Ok(()); - }; - let org = seed_org(&db.pool).await?; - let user = seed_user(&db.pool, OrgId::from_uuid(org)).await?; - let owned = seed_sandbox(&db.pool, OrgId::from_uuid(org), UserId::from_uuid(user)).await?; - - let node_id = Uuid::now_v7(); - sqlx::query(sql!( - "INSERT INTO nodes (id, address, role, managed_by, state) - VALUES ($1, '127.0.0.1', 'worker', 'space', 'ready')" - )) - .bind(node_id) - .execute(&db.pool) - .await?; - let observed_only = Uuid::now_v7(); - sqlx::query(sql!( - "INSERT INTO sandbox_observed_states (sandbox_id, node_id, status) - VALUES ($1, $2, 'running')" - )) - .bind(observed_only) - .bind(node_id) - .execute(&db.pool) - .await?; - - let actor = sqlx::query_as::<_, User>(sql!( - "SELECT u.* - FROM users u - JOIN clusters c ON c.managing_org_id = u.org_id - WHERE c.singleton - AND u.role = 'organization_admin' - ORDER BY u.id - LIMIT 1" - )) - .fetch_one(&db.pool) - .await?; - let state = AppState::from_pool(db.pool.clone()).await?; - - let list = |org_id: Option| { - let state = state.clone(); - let actor = actor.clone(); - async move { - let response = list_sandboxes( - State(state), - Extension(AuthUser(actor)), - Query(AdminSandboxListQuery { - limit: None, - org_id, - }), - ) - .await - .into_response(); - assert_eq!(response.status(), StatusCode::OK); - let body = axum::body::to_bytes(response.into_body(), usize::MAX).await?; - let rows: Vec = serde_json::from_slice(&body)?; - let ids = rows - .iter() - .filter_map(|row| row["id"].as_str().map(str::to_string)) - .collect::>(); - Ok::<_, Box>(ids) - } - }; - let contains = |ids: &[String], id: Uuid| ids.iter().any(|row| row.contains(&id.to_string())); - - let scoped = list(Some(OrgId::from_uuid(org))).await?; - assert!( - contains(&scoped, owned), - "org list holds the org's sandbox: {scoped:?}" - ); - assert!( - !contains(&scoped, observed_only), - "org list leaves out observed-only rows: {scoped:?}" - ); - - let unscoped = list(None).await?; - assert!( - contains(&unscoped, owned), - "unscoped list holds the sandbox: {unscoped:?}" - ); - assert!( - contains(&unscoped, observed_only), - "unscoped list keeps observed-only rows: {unscoped:?}" - ); - Ok(()) -} - async fn seed_org(pool: &sqlx::PgPool) -> Result { let id = Uuid::now_v7(); sqlx::query(sql!( diff --git a/crates/libs/api/src/routes/debug/sandbox.rs b/crates/libs/api/src/routes/debug/sandbox.rs index 2e1a3ed1bf..f4d3ae10a9 100644 --- a/crates/libs/api/src/routes/debug/sandbox.rs +++ b/crates/libs/api/src/routes/debug/sandbox.rs @@ -13,6 +13,7 @@ use crate::{ models::User, routes::{ AppState, + admin::sandbox_id::parse_admin_sandbox_id, admin::support::{self, ContentionScope}, authz::is_managing_org_admin, debug::{ @@ -67,11 +68,14 @@ async fn require_managing_org_admin(state: &AppState, actor: &User) -> Result<() pub async fn get_sandbox_debug( State(state): State, Extension(AuthUser(actor)): Extension, - Path(sandbox_id): Path, + Path(spec): Path, ) -> Response { if let Err(response) = require_managing_org_admin(&state, &actor).await { return *response; } + let Ok(sandbox_id) = parse_admin_sandbox_id(&spec) else { + return api_error(StatusCode::BAD_REQUEST, "invalid_sandbox_id"); + }; match fetch_postgres_debug_sections(&state, sandbox_id).await { Ok(Some(response)) => Json(response).into_response(), diff --git a/crates/libs/api/src/routes/debug/sandbox/tests.rs b/crates/libs/api/src/routes/debug/sandbox/tests.rs index e1e60b444b..2ab3d3f4b2 100644 --- a/crates/libs/api/src/routes/debug/sandbox/tests.rs +++ b/crates/libs/api/src/routes/debug/sandbox/tests.rs @@ -107,7 +107,7 @@ async fn the_debug_handler_answers_a_contended_read_with_a_503() let response = get_sandbox_debug( State(state.clone()), axum::Extension(crate::middleware::auth::AuthUser(actor.clone())), - Path(SandboxId::new()), + Path(SandboxId::new().to_string()), ) .await; @@ -235,7 +235,7 @@ async fn the_debug_read_reports_quarantine_and_snapshot_storage() let response = get_sandbox_debug( State(state), axum::Extension(crate::middleware::auth::AuthUser(actor)), - Path(sandbox_id), + Path(pplx_space_public_api::PublicSandboxId::from(sandbox_id).to_string()), ) .await; assert_eq!(response.status(), StatusCode::OK); From aef3be8090a651787b807f76100147e83c3b5c4e Mon Sep 17 00:00:00 2001 From: Abhinav Chordia Date: Mon, 2 Nov 2026 19:42:42 +0000 Subject: [PATCH 2/5] api: count what the admin list shows, and page it through the controller The spaced-version count now tallies the full list as the console does: every row whose status is not deleted, by the template, spaced version, status and resource type the list response shows, including the node-reported rows when no org_id is given. The admin filters, cursor and count move into the sandbox controller's admin_list module, which pages through the ECW list's page walk and full-row select; a page stops at the list byte budget, and the unpaged array keeps its limit with no budget. Malformed query parameters on the list and the count answer the admin JSON error, and with cursor an unknown parameter answers 400. A prefix with a NUL answers 400, created_before rounds up to the microsecond Postgres keeps, and the sandbox ID parser ignores surrounding whitespace. --- .../api/src/controllers/sandbox/admin_list.rs | 332 +++++++++++++++ .../sandbox/admin_list}/tests.rs | 391 ++++++++++++++---- .../libs/api/src/controllers/sandbox/mod.rs | 1 + .../libs/api/src/controllers/sandbox/org.rs | 15 +- crates/libs/api/src/routes/admin/docs.rs | 19 +- crates/libs/api/src/routes/admin/mod.rs | 1 - .../libs/api/src/routes/admin/sandbox_id.rs | 6 +- .../api/src/routes/admin/sandbox_id/tests.rs | 4 +- .../libs/api/src/routes/admin/sandbox_list.rs | 144 +++++-- .../src/routes/admin/sandbox_list/tests.rs | 146 ++++++- .../api/src/routes/admin/sandbox_store.rs | 228 ---------- 11 files changed, 910 insertions(+), 377 deletions(-) create mode 100644 crates/libs/api/src/controllers/sandbox/admin_list.rs rename crates/libs/api/src/{routes/admin/sandbox_store => controllers/sandbox/admin_list}/tests.rs (58%) delete mode 100644 crates/libs/api/src/routes/admin/sandbox_store.rs diff --git a/crates/libs/api/src/controllers/sandbox/admin_list.rs b/crates/libs/api/src/controllers/sandbox/admin_list.rs new file mode 100644 index 0000000000..a22e9db9e6 --- /dev/null +++ b/crates/libs/api/src/controllers/sandbox/admin_list.rs @@ -0,0 +1,332 @@ +//! The admin sandbox list's filters and pages, and its count of sandboxes by +//! template and `spaced` version. +//! +//! A page is the ECW list's page walk ([`super::list_page`]) over the admin +//! candidate statement below, with the same full-row select, so both lists +//! resolve rows and stop at the byte budget the same way. The admin route's +//! legacy unpaged array runs the same statement with no byte budget, since it +//! has no cursor to continue from. + +use std::sync::LazyLock; + +use pplx_space_common_core::sql; +use pplx_space_common_core::types::{NodeId, OrgId, SandboxId, SemVer, TemplateName, UserId}; +use pplx_space_placement::StoredSandboxLifecycle; +use pplx_space_public_api::PUBLIC_SANDBOX_ID_PREFIX; +use serde::Serialize; +use sqlx::PgPool; +use time::OffsetDateTime; +use uuid::Uuid; + +use super::list_page::{ + ListPage, MAX_SANDBOX_LIST_PAGE_BYTES, PageWalkParams, SANDBOX_LIST_ROW_OVERHEAD_BYTES, +}; +use super::org::{ScopedPageRow, scoped_page_sql}; +use super::{SandboxController, SandboxPageCursor}; +use crate::controllers::{ControllerError, db_contention_error}; +use crate::models::{Sandbox, SandboxResourceType}; + +/// What the admin sandbox list narrows to. Every field that is set must +/// match; an unset field matches every row. +#[derive(Debug, Clone, Default)] +pub struct AdminSandboxFilter { + pub org_id: Option, + /// The stored lifecycle (`sandboxes.lifecycle`), any of these. No + /// response field shows it: `lifecycle` and `status` report the node's + /// observed status when there is one. Never empty. + pub lifecycles: Option>, + /// The row's own template (`sandboxes.template_id`), not the fallbacks + /// the response's `template` resolves when the row has none. + pub template: Option, + pub node: Option, + pub user_id: Option, + /// Created strictly after this instant. + pub created_after: Option, + /// Created strictly before this instant. + pub created_before: Option, + pub search: Option, +} + +impl AdminSandboxFilter { + /// Whether no filter is set. Only the unfiltered list carries the + /// node-reported rows with no sandbox row: they belong to no org, and + /// none of the other filters can be applied to them. + pub fn is_unfiltered(&self) -> bool { + let Self { + org_id, + lifecycles, + template, + node, + user_id, + created_after, + created_before, + search, + } = self; + org_id.is_none() + && lifecycles.is_none() + && template.is_none() + && node.is_none() + && user_id.is_none() + && created_after.is_none() + && created_before.is_none() + && search.is_none() + } +} + +/// The node a sandbox is assigned to, as the list response reports it in +/// `active_node_id`: the direct route's current node when the sandbox has +/// one, else the row's `active_node_id`. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum NodeAssignment { + Node(NodeId), + Assigned, + Unassigned, +} + +/// A name or ID prefix. A row matches when its name starts with the prefix +/// as given, or when the prefix, with an optional `sbx_` and dashes where a +/// UUID has them, is the start of its ID. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SandboxSearch { + prefix: String, + /// The lowest and highest IDs that start with the prefix, when it can + /// start an ID. Postgres compares UUIDs byte by byte, so a hex prefix is + /// exactly this range. + id_range: Option<(SandboxId, SandboxId)>, +} + +impl SandboxSearch { + pub fn new(prefix: String) -> Self { + let id_range = id_prefix_range(&prefix); + Self { prefix, id_range } + } +} + +const DASHED_UUID_DASH_POSITIONS: [usize; 4] = [8, 13, 18, 23]; +const UNDASHED_UUID_LEN: usize = 32; + +fn id_prefix_range(prefix: &str) -> Option<(SandboxId, SandboxId)> { + let id = prefix + .strip_prefix(PUBLIC_SANDBOX_ID_PREFIX) + .unwrap_or(prefix); + let dashed = id.contains('-'); + let mut hex = String::with_capacity(UNDASHED_UUID_LEN); + for (position, character) in id.chars().enumerate() { + if dashed && DASHED_UUID_DASH_POSITIONS.contains(&position) { + if character != '-' { + return None; + } + } else if character.is_ascii_hexdigit() { + hex.push(character.to_ascii_lowercase()); + } else { + return None; + } + } + if hex.len() > UNDASHED_UUID_LEN { + return None; + } + let bound = |fill: char| { + let mut digits = hex.clone(); + digits.extend(std::iter::repeat_n(fill, UNDASHED_UUID_LEN - hex.len())); + Uuid::try_parse(&digits).ok().map(SandboxId::from_uuid) + }; + Some((bound('0')?, bound('f')?)) +} + +/// The admin list's sandboxes ($1-$11) after the cursor. `active_node_id` is +/// the response's (`SANDBOX_COLUMNS`): the direct route's current node, else +/// the row's. +const ADMIN_CANDIDATE_ROWS: &str = "SELECT s.id, s.created_at + FROM sandboxes s + LEFT JOIN sandbox_direct_route_identities direct_route ON direct_route.sandbox_id = s.id + WHERE ($1::UUID IS NULL OR s.org_id = $1) + AND ($2::TEXT[] IS NULL OR s.lifecycle = ANY($2::TEXT[])) + AND ($3::TEXT IS NULL OR s.template_id = $3) + AND ($4::UUID IS NULL OR COALESCE(direct_route.current_node_id, s.active_node_id) = $4) + AND ($5::BOOLEAN IS NULL + OR (COALESCE(direct_route.current_node_id, s.active_node_id) IS NOT NULL) = $5) + AND ($6::UUID IS NULL OR s.user_id = $6) + AND ($7::TIMESTAMPTZ IS NULL OR s.created_at > $7) + AND ($8::TIMESTAMPTZ IS NULL OR s.created_at < $8) + AND ($9::TEXT IS NULL + OR starts_with(s.name, $9) + OR s.id BETWEEN $10::UUID AND $11::UUID) + AND {after_cursor} + ORDER BY s.created_at DESC, s.id DESC + LIMIT {max_rows}"; + +static ADMIN_PAGE_SQL: LazyLock = LazyLock::new(|| { + scoped_page_sql( + ADMIN_CANDIDATE_ROWS, + &PageWalkParams { + cursor_created_at: "$12", + cursor_id: "$13", + max_rows: "$14", + max_bytes: "$15", + row_overhead_bytes: "$16", + // The admin response carries no `env`. + lists_env: false, + }, + ) +}); + +/// Whether a page stops at the list byte budget. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum PageBudget { + /// [`MAX_SANDBOX_LIST_PAGE_BYTES`], for a page a cursor continues. + Bounded, + /// No byte budget: the legacy unpaged array, which has no cursor to + /// continue from and answered up to its limit before paging existed. + Unbounded, +} + +/// How many sandboxes run each template at each `spaced` version, by the +/// status and resource type the admin list shows. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct SpacedVersionCount { + /// The list response's `template`. + pub template: Option, + /// The list response's `spaced_version`: what the node last reported. + pub spaced_version: Option, + /// The list response's `status` (and `lifecycle`): the node's observed + /// status, else the stored lifecycle. + pub status: Option, + pub resource_type: SandboxResourceType, + pub count: i64, +} + +#[derive(sqlx::FromRow)] +struct SpacedVersionCountRow { + template: Option, + spaced_version: Option, + status: Option, + has_row: bool, + pool_slot: bool, + count: i64, +} + +impl SandboxController { + /// One admin list page matching `filter`, newest first by `(created_at, + /// id)`, after `cursor`: at most `limit` rows, and whether more follow. + pub(crate) async fn admin_sandbox_page( + &self, + pool: &PgPool, + filter: &AdminSandboxFilter, + cursor: Option, + limit: u32, + budget: PageBudget, + ) -> Result, sqlx::Error> { + let lifecycles = filter.lifecycles.as_ref().map(|lifecycles| { + lifecycles + .iter() + .map(|lifecycle| lifecycle.as_str()) + .collect::>() + }); + let (node_id, assigned) = match filter.node { + Some(NodeAssignment::Node(node_id)) => (Some(node_id), None), + Some(NodeAssignment::Assigned) => (None, Some(true)), + Some(NodeAssignment::Unassigned) => (None, Some(false)), + None => (None, None), + }; + let id_range = filter.search.as_ref().and_then(|search| search.id_range); + let max_bytes = match budget { + PageBudget::Bounded => MAX_SANDBOX_LIST_PAGE_BYTES, + PageBudget::Unbounded => i64::MAX, + }; + let rows = + sqlx::query_as::<_, ScopedPageRow>(sqlx::AssertSqlSafe(sql!(ADMIN_PAGE_SQL.as_str()))) + .bind(filter.org_id) + .bind(lifecycles) + .bind(filter.template.as_ref().map(TemplateName::as_str)) + .bind(node_id) + .bind(assigned) + .bind(filter.user_id) + .bind(filter.created_after) + .bind(filter.created_before) + .bind(filter.search.as_ref().map(|search| search.prefix.as_str())) + .bind(id_range.map(|(low, _)| low)) + .bind(id_range.map(|(_, high)| high)) + .bind(cursor.map(|cursor| cursor.created_at)) + .bind(cursor.map(|cursor| cursor.id)) + .bind(i64::from(limit.saturating_add(1))) + .bind(max_bytes) + .bind(SANDBOX_LIST_ROW_OVERHEAD_BYTES) + .fetch_all(pool) + .await?; + let page = ListPage::from_rows(rows, limit as usize, |row| row.has_next); + Ok(ListPage { + rows: page.rows.into_iter().map(|row| row.sandbox).collect(), + has_more: page.has_more, + }) + } + + /// Every listed sandbox whose status is not `deleted`, counted by the + /// template, `spaced` version, status and resource type the admin list + /// shows for it. Without `org_id` that includes the rows nodes report + /// with no sandbox row, as the unscoped list does. + /// + /// The template and status follow the list's projection + /// (`sandbox_to_cached_response`, `observed_only_sandbox_response` and + /// `fetch_sandbox_observed_states`): the row's template, else the node's, + /// else the template the node's inventory holds for the image it runs, + /// else the pool's; the node's status, else the stored lifecycle. + pub async fn admin_spaced_version_counts( + &self, + pool: &PgPool, + org_id: Option, + ) -> Result, ControllerError> { + let rows = sqlx::query_as::<_, SpacedVersionCountRow>(sql!( + "SELECT counted.template, + counted.spaced_version, + counted.status, + counted.has_row, + counted.pool_slot, + COUNT(*)::BIGINT AS count + FROM ( + SELECT COALESCE(s.template_id, sos.template, observed_template.template_name, + sos.pool_template) AS template, + sos.spaced_version, + COALESCE(sos.status, s.lifecycle) AS status, + s.id IS NOT NULL AS has_row, + s.id IS NULL AND sos.pool_template IS NOT NULL AS pool_slot + FROM sandboxes s + FULL JOIN sandbox_observed_states sos ON sos.sandbox_id = s.id + LEFT JOIN LATERAL ( + SELECT nti.template_name + FROM node_template_inventory nti + WHERE nti.node_id = sos.node_id + AND sos.image_hash IS NOT NULL + AND nti.content_hash = sos.image_hash + ORDER BY nti.observed_at DESC, nti.template_name ASC + LIMIT 1 + ) observed_template ON TRUE + WHERE ($1::UUID IS NULL OR s.org_id = $1) + ) counted + WHERE counted.status IS DISTINCT FROM $2 + GROUP BY 1, 2, 3, 4, 5 + ORDER BY 1 NULLS LAST, 2 NULLS LAST, 3 NULLS LAST, 4 DESC, 5" + )) + .bind(org_id) + .bind(StoredSandboxLifecycle::Deleted.as_str()) + .fetch_all(pool) + .await + .map_err(|err| db_contention_error("count sandboxes by spaced version", &err))?; + Ok(rows + .into_iter() + .map(|row| SpacedVersionCount { + template: row.template, + spaced_version: row.spaced_version, + status: row.status, + resource_type: match (row.has_row, row.pool_slot) { + (true, _) => SandboxResourceType::Sandbox, + (false, true) => SandboxResourceType::PoolSlot, + (false, false) => SandboxResourceType::ObservedLocal, + }, + count: row.count, + }) + .collect()) + } +} + +#[cfg(test)] +pub(crate) mod tests; diff --git a/crates/libs/api/src/routes/admin/sandbox_store/tests.rs b/crates/libs/api/src/controllers/sandbox/admin_list/tests.rs similarity index 58% rename from crates/libs/api/src/routes/admin/sandbox_store/tests.rs rename to crates/libs/api/src/controllers/sandbox/admin_list/tests.rs index 105c34f26a..c6d9c87c8f 100644 --- a/crates/libs/api/src/routes/admin/sandbox_store/tests.rs +++ b/crates/libs/api/src/controllers/sandbox/admin_list/tests.rs @@ -1,18 +1,21 @@ +use std::collections::BTreeMap; + use pplx_space_common_core::types::SubstrateKind; use pplx_space_db_test_fixtures::{TestDbResult, insert_ready_worker_node, pg_test_db}; -use sqlx::PgPool; use time::Duration; use super::*; +use crate::controllers::sandbox::SandboxController; +use crate::models::SandboxResponse; /// An org and one of its users, for seeded sandboxes to belong to. #[derive(Debug, Clone, Copy)] -pub(in crate::routes::admin) struct Owner { - pub(in crate::routes::admin) org_id: OrgId, - pub(in crate::routes::admin) user_id: UserId, +pub(crate) struct Owner { + pub(crate) org_id: OrgId, + pub(crate) user_id: UserId, } -pub(in crate::routes::admin) async fn seed_org(pool: &PgPool) -> TestDbResult { +pub(crate) async fn seed_org(pool: &PgPool) -> TestDbResult { let id = Uuid::now_v7(); sqlx::query(sql!( "INSERT INTO organizations (id, name, slug, byok_enabled) @@ -26,10 +29,7 @@ pub(in crate::routes::admin) async fn seed_org(pool: &PgPool) -> TestDbResult TestDbResult { +pub(crate) async fn seed_user(pool: &PgPool, org_id: OrgId) -> TestDbResult { let user_id = UserId::new(); sqlx::query(sql!( "INSERT INTO users (id, email, password_hash, display_name, org_id, role) @@ -43,23 +43,23 @@ pub(in crate::routes::admin) async fn seed_user( Ok(Owner { org_id, user_id }) } -pub(in crate::routes::admin) async fn seed_owner(pool: &PgPool) -> TestDbResult { +pub(crate) async fn seed_owner(pool: &PgPool) -> TestDbResult { let org_id = seed_org(pool).await?; seed_user(pool, org_id).await } /// A sandbox row to seed; unset fields take the column defaults. #[derive(Debug, Clone, Default)] -pub(in crate::routes::admin) struct SeedSandbox { - pub(in crate::routes::admin) id: Option, - pub(in crate::routes::admin) name: Option<&'static str>, - pub(in crate::routes::admin) lifecycle: Option<&'static str>, - pub(in crate::routes::admin) template: Option<&'static str>, - pub(in crate::routes::admin) active_node_id: Option, - pub(in crate::routes::admin) created_at: Option, +pub(crate) struct SeedSandbox { + pub(crate) id: Option, + pub(crate) name: Option<&'static str>, + pub(crate) lifecycle: Option<&'static str>, + pub(crate) template: Option<&'static str>, + pub(crate) active_node_id: Option, + pub(crate) created_at: Option, } -pub(in crate::routes::admin) async fn seed_sandbox( +pub(crate) async fn seed_sandbox( pool: &PgPool, owner: Owner, seed: SeedSandbox, @@ -87,27 +87,61 @@ pub(in crate::routes::admin) async fn seed_sandbox( Ok(id) } -/// What a node reports for `sandbox_id`, which need not have a sandbox row. -pub(in crate::routes::admin) async fn seed_observation( +/// What a node reports for a sandbox, which need not have a sandbox row. +#[derive(Debug, Clone, Default)] +pub(crate) struct SeedObservation { + pub(crate) status: Option<&'static str>, + pub(crate) template: Option<&'static str>, + pub(crate) pool_template: Option<&'static str>, + pub(crate) image_hash: Option<&'static str>, + pub(crate) spaced_version: Option<&'static str>, +} + +pub(crate) async fn seed_observation( pool: &PgPool, sandbox_id: SandboxId, node_id: NodeId, - template: Option<&str>, - spaced_version: Option<&str>, + seed: SeedObservation, ) -> TestDbResult<()> { sqlx::query(sql!( - "INSERT INTO sandbox_observed_states (sandbox_id, node_id, status, template, spaced_version) - VALUES ($1, $2, 'started', $3, $4)" + "INSERT INTO sandbox_observed_states ( + sandbox_id, node_id, status, template, pool_template, image_hash, spaced_version + ) + VALUES ($1, $2, $3, $4, $5, $6, $7)" )) .bind(sandbox_id) .bind(node_id) - .bind(template) - .bind(spaced_version) + .bind(seed.status) + .bind(seed.template) + .bind(seed.pool_template) + .bind(seed.image_hash) + .bind(seed.spaced_version) .execute(pool) .await?; Ok(()) } +fn controller(pool: &PgPool) -> SandboxController { + SandboxController::new(crate::test_support::direct_substrate(pool)) +} + +/// One page of the list, with no byte budget unless `budget` says so. +async fn page( + pool: &PgPool, + filter: &AdminSandboxFilter, + after: Option, + limit: u32, +) -> Result, sqlx::Error> { + controller(pool) + .admin_sandbox_page(pool, filter, after, limit, PageBudget::Bounded) + .await +} + +async fn listed_ids(pool: &PgPool, filter: &AdminSandboxFilter) -> TestDbResult> { + let page = page(pool, filter, None, 100).await?; + Ok(page.rows.iter().map(|sandbox| sandbox.id).collect()) +} + fn base_time() -> OffsetDateTime { OffsetDateTime::from_unix_timestamp(1_790_000_000).expect("fixture timestamp") } @@ -121,10 +155,11 @@ fn cursor_after(sandbox: &Sandbox) -> SandboxPageCursor { /// Walking the list a page at a time returns every row that existed when the /// walk began exactly once, in list order, although rows sharing a -/// `created_at` straddle page boundaries and new rows land between pages. +/// `created_at` straddle page boundaries and rows created later land between +/// pages. #[tokio::test] async fn pages_return_every_row_once_while_rows_are_inserted() -> TestDbResult<()> { - const PAGE: usize = 2; + const PAGE: u32 = 2; let db = pg_test_db().await?; let owner = seed_owner(&db.pool).await?; let mut existing = Vec::new(); @@ -151,15 +186,14 @@ async fn pages_return_every_row_once_while_rows_are_inserted() -> TestDbResult<( let mut seen = Vec::new(); let mut after = None; loop { - let mut page = list_sandboxes(&db.pool, &filter, after, PAGE as i64 + 1).await?; + let page = page(&db.pool, &filter, after, PAGE).await?; seed_sandbox(&db.pool, owner, SeedSandbox::default()).await?; - let more = page.len() > PAGE; - page.truncate(PAGE); - seen.extend(page.iter().map(|sandbox| sandbox.id)); - if !more { + assert!(page.rows.len() <= PAGE as usize); + seen.extend(page.rows.iter().map(|sandbox| sandbox.id)); + if !page.has_more { break; } - after = page.last().map(cursor_after); + after = page.rows.last().map(cursor_after); } let expected = existing.iter().map(|(_, id)| *id).collect::>(); assert_eq!(seen, expected); @@ -197,7 +231,7 @@ async fn the_first_page_is_the_list_from_before_paging() -> TestDbResult<()> { ORDER BY s.created_at DESC, s.id DESC LIMIT $1" )) - .bind(limit) + .bind(i64::from(limit)) .bind(org_id) .fetch_all(&db.pool) .await?; @@ -205,8 +239,14 @@ async fn the_first_page_is_the_list_from_before_paging() -> TestDbResult<()> { org_id, ..AdminSandboxFilter::default() }; - let listed = list_sandboxes(&db.pool, &filter, None, limit).await?; - let listed = listed.iter().map(|sandbox| sandbox.id).collect::>(); + let listed = controller(&db.pool) + .admin_sandbox_page(&db.pool, &filter, None, limit, PageBudget::Unbounded) + .await?; + let listed = listed + .rows + .iter() + .map(|sandbox| sandbox.id) + .collect::>(); assert_eq!(listed, legacy, "org {org_id:?}, limit {limit}"); } } @@ -474,8 +514,7 @@ async fn each_filter_returns_exactly_its_rows() -> TestDbResult<()> { ), ]; for (case, filter, expected) in cases { - let listed = list_sandboxes(&db.pool, &filter, None, 100).await?; - let listed = listed.iter().map(|sandbox| sandbox.id).collect::>(); + let listed = listed_ids(&db.pool, &filter).await?; // Newest first, so the expected rows in reverse seed order. let expected = expected .iter() @@ -531,30 +570,208 @@ fn an_id_prefix_is_the_range_of_ids_it_starts() { } } -/// A node report seeded with a row: its template and `spaced` version. -type SeedObservation = (Option<&'static str>, &'static str); +/// The node-reported half of `node_id` and `assigned`: a sandbox whose +/// direct route names a node is on that node, whatever its row's +/// `active_node_id` says, exactly as the response reports it. +#[tokio::test] +async fn the_node_filters_follow_the_direct_route_the_response_reports() -> TestDbResult<()> { + let db = pg_test_db().await?; + let owner = seed_owner(&db.pool).await?; + let (row_node, route_node) = (NodeId::new(), NodeId::new()); + let moved = seed_sandbox( + &db.pool, + owner, + SeedSandbox { + active_node_id: Some(row_node), + ..SeedSandbox::default() + }, + ) + .await?; + let routed_only = seed_sandbox(&db.pool, owner, SeedSandbox::default()).await?; + let unassigned = seed_sandbox(&db.pool, owner, SeedSandbox::default()).await?; + for sandbox_id in [moved, routed_only] { + sqlx::query(sql!( + "INSERT INTO sandbox_direct_route_identities ( + sandbox_id, route_generation, current_node_id, substrate_kind, + execution_generation, activation_attempt_id + ) + VALUES ($1, 1, $2, $3, 1, $4)" + )) + .bind(sandbox_id) + .bind(route_node) + .bind(SubstrateKind::Space) + .bind(Uuid::now_v7()) + .execute(&db.pool) + .await?; + } + let filter = |node| AdminSandboxFilter { + org_id: Some(owner.org_id), + node: Some(node), + ..AdminSandboxFilter::default() + }; + let on_route_node = page( + &db.pool, + &filter(NodeAssignment::Node(route_node)), + None, + 100, + ) + .await?; + assert_eq!( + on_route_node + .rows + .iter() + .map(|sandbox| sandbox.id) + .collect::>(), + [routed_only, moved] + ); + assert!( + on_route_node + .rows + .iter() + .all(|sandbox| sandbox.active_node_id == Some(route_node)), + "the filter matches the node the response reports" + ); + assert!( + listed_ids(&db.pool, &filter(NodeAssignment::Node(row_node))) + .await? + .is_empty() + ); + assert_eq!( + listed_ids(&db.pool, &filter(NodeAssignment::Assigned)).await?, + [routed_only, moved] + ); + assert_eq!( + listed_ids(&db.pool, &filter(NodeAssignment::Unassigned)).await?, + [unassigned] + ); + db.pool.close().await; + Ok(()) +} + +type Bucket = ( + Option, + Option, + Option, + SandboxResourceType, +); -/// The count groups every sandbox that is not deleted by template, reported -/// `spaced` version and stored lifecycle, and narrows to one org on request. +/// The tally the console draws from the list: every row whose status (else +/// lifecycle) is not `deleted`, by the fields the list response shows. +fn tally(responses: &[SandboxResponse]) -> BTreeMap { + let mut buckets = BTreeMap::new(); + for response in responses { + if response.status.as_deref().or(response.lifecycle.as_deref()) == Some("deleted") { + continue; + } + let bucket: Bucket = ( + response.template.clone(), + response.spaced_version.clone(), + response.status.clone(), + response.resource_type, + ); + *buckets.entry(format!("{bucket:?}")).or_insert(0) += 1; + } + buckets +} + +fn counted(counts: &[SpacedVersionCount]) -> BTreeMap { + counts + .iter() + .map(|count| { + let bucket: Bucket = ( + count.template.clone(), + count.spaced_version.clone(), + count.status.clone(), + count.resource_type, + ); + (format!("{bucket:?}"), count.count) + }) + .collect() +} + +/// The count is the console's tally of the full list: the same template +/// (including the node inventory's), version, status and resource type the +/// list response shows, the node-reported rows with no sandbox row in the +/// unscoped count only, and nothing whose status is `deleted`. The oracle is +/// the list's own projection (`list_responses`) over every listed row. #[tokio::test] -async fn sandboxes_count_by_template_version_and_lifecycle() -> TestDbResult<()> { +async fn the_count_is_the_tally_of_the_full_list() -> TestDbResult<()> { let db = pg_test_db().await?; let owner = seed_owner(&db.pool).await?; let other = seed_owner(&db.pool).await?; let node_id = NodeId::new(); insert_ready_worker_node(&db.pool, node_id).await?; - let rows: [(Owner, Option<&str>, &str, Option); 6] = [ - (owner, Some("py"), "started", Some((None, "1.2.0"))), + sqlx::query(sql!( + "INSERT INTO templates (id, name) VALUES ($1, 'inventoried')" + )) + .bind(Uuid::now_v7()) + .execute(&db.pool) + .await?; + sqlx::query(sql!( + "INSERT INTO node_template_inventory (node_id, template_name, content_hash) + VALUES ($1, 'inventoried', 'sha256:inventoried')" + )) + .bind(node_id) + .execute(&db.pool) + .await?; + let started = |spaced_version| SeedObservation { + status: Some("started"), + spaced_version, + ..SeedObservation::default() + }; + let rows: Vec<( + Owner, + Option<&'static str>, + &'static str, + Option, + )> = vec![ + (owner, Some("py"), "started", Some(started(Some("1.2.0")))), ( owner, Some("py"), "started", - Some((Some("ignored"), "1.2.0")), + Some(SeedObservation { + template: Some("ignored"), + ..started(Some("1.2.0")) + }), ), (owner, Some("py"), "suspended", None), - (owner, None, "started", Some((Some("observed"), "1.3.0"))), - (owner, Some("py"), "deleted", Some((None, "1.2.0"))), - (other, Some("py"), "started", Some((None, "1.2.0"))), + ( + owner, + None, + "started", + Some(SeedObservation { + image_hash: Some("sha256:inventoried"), + ..started(Some("1.3.0")) + }), + ), + ( + owner, + None, + "started", + Some(SeedObservation { + template: Some("reported"), + ..started(None) + }), + ), + (owner, Some("py"), "deleted", None), + // The node's status wins over the stored lifecycle, as in the list. + ( + owner, + Some("py"), + "cleanup_required", + Some(started(Some("1.2.0"))), + ), + ( + owner, + Some("py"), + "deleted", + Some(SeedObservation { + status: Some("deleted"), + ..SeedObservation::default() + }), + ), + (other, Some("py"), "started", Some(started(Some("1.2.0")))), ]; for (row_owner, template, lifecycle, observed) in rows { let id = seed_sandbox( @@ -567,30 +784,64 @@ async fn sandboxes_count_by_template_version_and_lifecycle() -> TestDbResult<()> }, ) .await?; - if let Some((observed_template, version)) = observed { - seed_observation(&db.pool, id, node_id, observed_template, Some(version)).await?; + if let Some(observed) = observed { + seed_observation(&db.pool, id, node_id, observed).await?; } } - let count = - |template: &str, version: Option<&str>, lifecycle: &str, count| SpacedVersionCount { - template: Some(template.to_string()), - spaced_version: version.map(str::to_string), - lifecycle: lifecycle.to_string(), - count, + // Rowless: a warm-pool slot and an observed-local sandbox. + seed_observation( + &db.pool, + SandboxId::new(), + node_id, + SeedObservation { + pool_template: Some("pooled"), + ..started(Some("1.2.0")) + }, + ) + .await?; + seed_observation( + &db.pool, + SandboxId::new(), + node_id, + SeedObservation::default(), + ) + .await?; + + let controller = controller(&db.pool); + for org_id in [None, Some(owner.org_id), Some(other.org_id)] { + let filter = AdminSandboxFilter { + org_id, + ..AdminSandboxFilter::default() }; - let everywhere = count_by_spaced_version(&db.pool, None).await?; - assert_eq!( - everywhere, - [ - count("observed", Some("1.3.0"), "started", 1), - count("py", Some("1.2.0"), "started", 3), - count("py", None, "suspended", 1), - ] + let listed = controller + .admin_sandbox_page(&db.pool, &filter, None, 1000, PageBudget::Unbounded) + .await?; + let responses = controller + .list_responses(&db.pool, &listed.rows, filter.is_unfiltered()) + .await + .map_err(|error| format!("{error:?}"))?; + let counts = controller + .admin_spaced_version_counts(&db.pool, org_id) + .await + .map_err(|error| format!("{error:?}"))?; + assert_eq!(counted(&counts), tally(&responses), "org {org_id:?}"); + } + let everywhere = controller + .admin_spaced_version_counts(&db.pool, None) + .await + .map_err(|error| format!("{error:?}"))?; + let has = |template: &str, resource_type| { + everywhere.iter().any(|count| { + count.template.as_deref() == Some(template) && count.resource_type == resource_type + }) + }; + assert!(has("inventoried", SandboxResourceType::Sandbox)); + assert!(has("pooled", SandboxResourceType::PoolSlot)); + assert!( + everywhere + .iter() + .any(|count| count.resource_type == SandboxResourceType::ObservedLocal) ); - let in_other = count_by_spaced_version(&db.pool, Some(other.org_id)).await?; - assert_eq!(in_other, [count("py", Some("1.2.0"), "started", 1)]); - let none_seen = count_by_spaced_version(&db.pool, Some(seed_org(&db.pool).await?)).await?; - assert!(none_seen.is_empty()); db.pool.close().await; Ok(()) } diff --git a/crates/libs/api/src/controllers/sandbox/mod.rs b/crates/libs/api/src/controllers/sandbox/mod.rs index ca0bdbd21f..cc14b0c867 100644 --- a/crates/libs/api/src/controllers/sandbox/mod.rs +++ b/crates/libs/api/src/controllers/sandbox/mod.rs @@ -1358,6 +1358,7 @@ impl SandboxWakeSignal { #[cfg(test)] mod reconnect_metrics_tests; +pub(crate) mod admin_list; mod ingress; pub(crate) mod lifecycle; pub(crate) mod list_page; diff --git a/crates/libs/api/src/controllers/sandbox/org.rs b/crates/libs/api/src/controllers/sandbox/org.rs index 38d8c8e871..f88bfcd834 100644 --- a/crates/libs/api/src/controllers/sandbox/org.rs +++ b/crates/libs/api/src/controllers/sandbox/org.rs @@ -39,12 +39,13 @@ pub enum SandboxAccessScope { }, } -/// A sandbox on an ECW list page and whether another sandbox follows it. +/// A sandbox on an ECW or admin list page and whether another sandbox +/// follows it. #[derive(sqlx::FromRow)] -struct ScopedPageRow { +pub(super) struct ScopedPageRow { #[sqlx(flatten)] - sandbox: Sandbox, - has_next: bool, + pub(super) sandbox: Sandbox, + pub(super) has_next: bool, } /// The local org's sandboxes ($1) after the cursor, matching the metadata @@ -120,9 +121,9 @@ static FEDERATED_SUBJECT_PAGE_SQL: LazyLock = LazyLock::new(|| { ) }); -/// An ECW list page statement: the page walk over `candidate_rows`, then the -/// full sandbox row of each page row. -fn scoped_page_sql(candidate_rows: &str, params: &PageWalkParams) -> String { +/// A list page statement, ECW or admin: the page walk over `candidate_rows`, +/// then the full sandbox row of each page row. +pub(super) fn scoped_page_sql(candidate_rows: &str, params: &PageWalkParams) -> String { let page_rows = page_rows_cte(candidate_rows, params); sandbox_query(&format!( "{page_rows} diff --git a/crates/libs/api/src/routes/admin/docs.rs b/crates/libs/api/src/routes/admin/docs.rs index d432ebb757..067df1ceb5 100644 --- a/crates/libs/api/src/routes/admin/docs.rs +++ b/crates/libs/api/src/routes/admin/docs.rs @@ -331,7 +331,7 @@ fn admin_openapi_value() -> Value { "post": { "summary": "Enqueue an audited break-glass sandbox lifecycle operation", "parameters": [ - { "name": "sandbox_id", "in": "path", "required": true, "description": "sbx_, a dashed or undashed UUID in either case, or the sandbox pod name (space-sbx-, optionally with its -g suffix)", "schema": { "type": "string" } }, + { "name": "sandbox_id", "in": "path", "required": true, "description": "sbx_, a dashed or undashed UUID in either case, or the sandbox pod name (space-sbx-, optionally with its -g suffix); surrounding whitespace is ignored", "schema": { "type": "string" } }, { "name": "action", "in": "path", "required": true, "schema": { "type": "string", "enum": ["start", "stop", "pause", "resume", "suspend", "restore", "wake", "kill", "delete"] } }, { "name": "Idempotency-Key", "in": "header", "required": true, "schema": { "type": "string" } } ], @@ -790,13 +790,13 @@ fn insert_admin_sandbox_create_paths(paths: &mut Map) { json!({ "get": { "summary": "List sandboxes across every organization", - "description": "Newest first by created_at, then id. Without cursor the response is a bare array of at most limit sandboxes; with no filter, it is followed by the sandboxes nodes report that have no sandbox row. Pass cursor, empty for the first page, to page: the response is then {sandboxes, next_cursor}, next_cursor is null on the last page, and the node-reported rows come on the first page only. Rows created while paging sort before the cursor and do not appear on later pages. Filters, org_id included, combine and leave out the node-reported rows. Unknown parameters are ignored.", + "description": "Newest first by created_at, then id. Without cursor the response is a bare array of at most limit sandboxes; with no filter, it is followed by the sandboxes nodes report that have no sandbox row. Unknown parameters are then ignored, as before paging existed. Pass cursor, empty for the first page, to page: the response is then {sandboxes, next_cursor}, with next_cursor null on the last page. A page holds at most limit sandboxes and stops early at the list byte budget. The node-reported rows come on the first page only, and an unknown parameter answers 400. Each page reads the current matching rows strictly below the supplied (created_at, id) position. Paging is not a snapshot: concurrent changes can affect membership. Keep the filters unchanged between pages. Filters, org_id included, combine and leave out the node-reported rows. A malformed parameter answers 400 with an error message.", "parameters": [ { "name": "limit", "in": "query", "required": false, "description": "1 to 5000, default 1000", "schema": { "type": "integer" } }, { "name": "cursor", "in": "query", "required": false, "description": "Opaque; empty for the first page, then the previous page's next_cursor", "schema": { "type": "string" } }, { "name": "org_id", "in": "query", "required": false, "schema": { "type": "string" } }, - { "name": "lifecycle", "in": "query", "required": false, "description": "Comma-separated stored lifecycles, including deleted and cleanup_required. A row's reported lifecycle can differ while its node's observation lags.", "schema": { "type": "string" } }, - { "name": "template", "in": "query", "required": false, "description": "The sandbox row's template", "schema": { "type": "string" } }, + { "name": "lifecycle", "in": "query", "required": false, "description": "Comma-separated values of the stored lifecycle, including deleted and cleanup_required. No response field shows the stored lifecycle: lifecycle and status report the node's observed status when there is one, so a row can show another value.", "schema": { "type": "string" } }, + { "name": "template", "in": "query", "required": false, "description": "The sandbox row's own template. A row without one does not match, although the response's template, like the spaced-version count, falls back to the template its node reports.", "schema": { "type": "string" } }, { "name": "node_id", "in": "query", "required": false, "description": "The node reported as active_node_id", "schema": { "type": "string" } }, { "name": "assigned", "in": "query", "required": false, "description": "true for sandboxes with an active node, false for those without; false cannot be combined with node_id", "schema": { "type": "boolean" } }, { "name": "user_id", "in": "query", "required": false, "schema": { "type": "string" } }, @@ -827,13 +827,14 @@ fn insert_admin_sandbox_create_paths(paths: &mut Map) { "/sandboxes/spaced-versions".to_string(), json!({ "get": { - "summary": "Count sandboxes by template, spaced version and lifecycle", - "description": "Every sandbox that is not deleted, grouped by its template (else the one its node reports), the spaced version its node last reported (null before any report) and its stored lifecycle.", + "summary": "Count sandboxes by template, spaced version and status", + "description": "The tally of the full sandbox list. Every row whose status is not deleted is grouped by the fields the list response shows for it: template, spaced_version (null before its node reports one), status (the node's observed status, else the stored lifecycle) and resource_type. Without org_id this includes the rows nodes report with no sandbox row, such as warm-pool slots, as the unscoped list does. Unknown parameters answer 400.", "parameters": [ { "name": "org_id", "in": "query", "required": false, "schema": { "type": "string" } } ], "responses": { - "200": { "description": "Object with counts (template, spaced_version, lifecycle, count)" } + "200": { "description": "Object with counts (template, spaced_version, status, resource_type, count)" }, + "400": { "description": "Invalid org_id or unknown parameter" } } } }), @@ -844,7 +845,7 @@ fn insert_admin_sandbox_create_paths(paths: &mut Map) { "get": { "summary": "Get a sandbox in any organization", "parameters": [ - { "name": "sandbox_id", "in": "path", "required": true, "description": "sbx_, a dashed or undashed UUID in either case, or the sandbox pod name (space-sbx-, optionally with its -g suffix)", "schema": { "type": "string" } } + { "name": "sandbox_id", "in": "path", "required": true, "description": "sbx_, a dashed or undashed UUID in either case, or the sandbox pod name (space-sbx-, optionally with its -g suffix); surrounding whitespace is ignored", "schema": { "type": "string" } } ], "responses": { "200": { "description": "Sandbox" }, @@ -855,7 +856,7 @@ fn insert_admin_sandbox_create_paths(paths: &mut Map) { "patch": { "summary": "Enqueue a sandbox update (network policy, auto-lifecycle, metadata) under the sandbox's owning organization", "parameters": [ - { "name": "sandbox_id", "in": "path", "required": true, "description": "sbx_, a dashed or undashed UUID in either case, or the sandbox pod name (space-sbx-, optionally with its -g suffix)", "schema": { "type": "string" } }, + { "name": "sandbox_id", "in": "path", "required": true, "description": "sbx_, a dashed or undashed UUID in either case, or the sandbox pod name (space-sbx-, optionally with its -g suffix); surrounding whitespace is ignored", "schema": { "type": "string" } }, { "name": "Idempotency-Key", "in": "header", "required": true, "schema": { "type": "string" } } ], "responses": { diff --git a/crates/libs/api/src/routes/admin/mod.rs b/crates/libs/api/src/routes/admin/mod.rs index 4f3a3db299..e16c53805c 100644 --- a/crates/libs/api/src/routes/admin/mod.rs +++ b/crates/libs/api/src/routes/admin/mod.rs @@ -50,7 +50,6 @@ mod runtime_environments; // `pub(in crate::routes)`: the debug route takes the same sandbox ID forms. pub(in crate::routes) mod sandbox_id; mod sandbox_list; -mod sandbox_store; mod sandboxes; // `pub(in crate::routes)` rather than private: `routes::debug` is mounted // inside this router (`debug::admin_v1_debug_router`, merged by `v1_router` diff --git a/crates/libs/api/src/routes/admin/sandbox_id.rs b/crates/libs/api/src/routes/admin/sandbox_id.rs index 1fce1b7c5e..516170360f 100644 --- a/crates/libs/api/src/routes/admin/sandbox_id.rs +++ b/crates/libs/api/src/routes/admin/sandbox_id.rs @@ -30,11 +30,13 @@ pub(in crate::routes) struct AdminSandboxIdError; /// /// Accepts `sbx_`, a bare UUID, and either UUID with its dashes removed, /// with hex digits in either case, and the sandbox's pod name with or without -/// its generation suffix. Nothing is trimmed, and other UUID spellings (braced, -/// `urn:uuid:`) are refused, so the route answers 400 rather than guessing. +/// its generation suffix, ignoring surrounding whitespace. Other UUID +/// spellings (braced, `urn:uuid:`) and prefixes (`SBX_`, `sandbox:`) are +/// refused, so the route answers 400 rather than guessing. pub(in crate::routes) fn parse_admin_sandbox_id( value: &str, ) -> Result { + let value = value.trim(); let uuid = if let Some(uuid) = value.strip_prefix(PUBLIC_SANDBOX_ID_PREFIX) { parse_uuid(uuid)? } else if let Some(name) = value.strip_prefix(SANDBOX_POD_NAME_PREFIX) { diff --git a/crates/libs/api/src/routes/admin/sandbox_id/tests.rs b/crates/libs/api/src/routes/admin/sandbox_id/tests.rs index 7487bc425d..ef74658d79 100644 --- a/crates/libs/api/src/routes/admin/sandbox_id/tests.rs +++ b/crates/libs/api/src/routes/admin/sandbox_id/tests.rs @@ -24,6 +24,8 @@ fn every_accepted_form_names_the_same_sandbox() { format!("space-sbx-{undashed}-g0"), format!("space-sbx-{undashed}-g17"), format!("space-sbx-{undashed}"), + format!(" {UUID}\t"), + format!("\nsbx_{UUID} "), ]; for form in accepted { assert_eq!( @@ -55,8 +57,6 @@ fn unknown_forms_are_refused() { format!("{{{UUID}}}"), format!("urn:uuid:{UUID}"), format!("sbx_{{{UUID}}}"), - format!(" {UUID}"), - format!("{UUID} "), format!("sandbox:{UUID}"), UUID.chars().take(35).collect(), undashed.chars().take(31).collect(), diff --git a/crates/libs/api/src/routes/admin/sandbox_list.rs b/crates/libs/api/src/routes/admin/sandbox_list.rs index 5c3a540a83..6c09b9faa9 100644 --- a/crates/libs/api/src/routes/admin/sandbox_list.rs +++ b/crates/libs/api/src/routes/admin/sandbox_list.rs @@ -7,20 +7,21 @@ use std::str::FromStr; use axum::{ Extension, Json, extract::{Query, State}, - http::StatusCode, + http::{StatusCode, Uri}, response::IntoResponse, }; use base64::{Engine as _, engine::general_purpose::URL_SAFE_NO_PAD}; use pplx_space_common_core::types::{NodeId, OrgId, SandboxId, TemplateName, UserId}; use pplx_space_placement::StoredSandboxLifecycle; +use serde::de::DeserializeOwned; use serde::{Deserialize, Serialize}; use time::OffsetDateTime; -use super::sandbox_store::{ - self, AdminSandboxFilter, NodeAssignment, SandboxSearch, SpacedVersionCount, -}; use super::support::{self, ContentionScope, api_error, require_managing_org_admin}; use crate::controllers::sandbox::SandboxPageCursor; +use crate::controllers::sandbox::admin_list::{ + AdminSandboxFilter, NodeAssignment, PageBudget, SandboxSearch, SpacedVersionCount, +}; use crate::middleware::auth::AuthUser; use crate::models::{Sandbox, SandboxResponse}; use crate::routes::AppState; @@ -29,11 +30,27 @@ const DEFAULT_ADMIN_SANDBOX_LIMIT: u32 = 1000; const MAX_ADMIN_SANDBOX_LIMIT: u32 = 5000; const CURSOR_VERSION: &str = "v1"; +/// The list's query parameters. With `cursor` any other parameter is +/// refused; without it, other parameters are ignored, as they were before +/// paging existed, so callers written against that list keep working. +const LIST_PARAMETERS: [&str; 11] = [ + "limit", + "org_id", + "cursor", + "lifecycle", + "template", + "node_id", + "assigned", + "user_id", + "created_after", + "created_before", + "prefix", +]; +const NANOSECONDS_PER_MICROSECOND: i128 = 1_000; + type QueryError = (StatusCode, Json); -/// The list's query string. Unknown parameters are ignored, as they were -/// before the filters existed, so a console that sends a filter this server -/// predates gets the unfiltered list rather than an error. +/// The list's query string; see [`LIST_PARAMETERS`] for unknown parameters. #[derive(Debug, Default, Deserialize)] pub struct AdminSandboxListQuery { limit: Option, @@ -90,6 +107,22 @@ struct ListRequest { } impl AdminSandboxListQuery { + /// Parses the list's query string, answering a malformed one with the + /// admin JSON error rather than axum's plain-text rejection. + fn from_uri(uri: &Uri) -> Result { + let query: Self = parse_query(uri)?; + if query.cursor.is_some() { + let pairs: Vec<(String, String)> = parse_query(uri)?; + if let Some((name, _)) = pairs + .iter() + .find(|(name, _)| !LIST_PARAMETERS.contains(&name.as_str())) + { + return Err(bad_request(format!("unknown query parameter {name:?}"))); + } + } + Ok(query) + } + fn into_request(self) -> Result { let limit = self .limit @@ -116,10 +149,13 @@ impl AdminSandboxListQuery { .as_deref() .map(|value| parse_timestamp("created_after", value)) .transpose()?; + // Postgres keeps microseconds and the bound value is truncated to + // them, so a bound inside a microsecond rounds up: a row created + // earlier in that microsecond is still before it. let created_before = self .created_before .as_deref() - .map(|value| parse_timestamp("created_before", value)) + .map(|value| parse_timestamp("created_before", value).and_then(round_up_to_microsecond)) .transpose()?; if let (Some(after), Some(before)) = (created_after, created_before) && after >= before @@ -146,10 +182,13 @@ impl AdminSandboxListQuery { user_id: self.user_id, created_after, created_before, - search: self - .prefix - .filter(|prefix| !prefix.is_empty()) - .map(SandboxSearch::new), + search: match self.prefix { + Some(prefix) if prefix.contains('\0') => { + return Err(bad_request("prefix must not contain a NUL character")); + } + Some(prefix) if !prefix.is_empty() => Some(SandboxSearch::new(prefix)), + _ => None, + }, }; Ok(ListRequest { limit, @@ -163,6 +202,12 @@ fn bad_request(message: impl Into) -> QueryError { api_error(StatusCode::BAD_REQUEST, message) } +fn parse_query(uri: &Uri) -> Result { + Query::::try_from_uri(uri) + .map(|Query(query)| query) + .map_err(|rejection| bad_request(rejection.body_text())) +} + fn parse_lifecycles(value: &str) -> Result, QueryError> { let mut lifecycles = Vec::new(); for part in value.split(',') { @@ -181,6 +226,16 @@ fn parse_timestamp(name: &str, value: &str) -> Result Result { + let nanos = value.unix_timestamp_nanos(); + let rounded = match nanos.rem_euclid(NANOSECONDS_PER_MICROSECOND) { + 0 => nanos, + within => nanos + NANOSECONDS_PER_MICROSECOND - within, + }; + OffsetDateTime::from_unix_timestamp_nanos(rounded) + .map_err(|_| bad_request("created_before must be an RFC 3339 timestamp")) +} + /// `v1::`, base64url without padding. Not /// signed: it carries only a position in a list the caller can already read /// in full. @@ -213,26 +268,33 @@ fn decode_cursor(cursor: &str) -> Result { pub async fn list_sandboxes( State(state): State, Extension(AuthUser(actor)): Extension, - Query(query): Query, + uri: Uri, ) -> axum::response::Response { if let Err(err) = require_managing_org_admin(&state.pool, &actor).await { return err.into_response(); } - let request = match query.into_request() { + let request = match AdminSandboxListQuery::from_uri(&uri).and_then(|query| query.into_request()) + { Ok(request) => request, Err(err) => return err.into_response(), }; - // One lookahead row tells a paged request whether another page follows. - let max_rows = i64::from(request.limit) + i64::from(request.paging.is_paged()); - let mut sandboxes = match sandbox_store::list_sandboxes( - &state.pool, - &request.filter, - request.paging.after(), - max_rows, - ) - .await + let budget = if request.paging.is_paged() { + PageBudget::Bounded + } else { + PageBudget::Unbounded + }; + let page = match state + .sandbox_controller + .admin_sandbox_page( + &state.pool, + &request.filter, + request.paging.after(), + request.limit, + budget, + ) + .await { - Ok(sandboxes) => sandboxes, + Ok(page) => page, // `ReadOnly`: the busiest admin read on this surface, and a plain // `SELECT` over `sandboxes`, so it blocks behind DDL on the hottest // table in the schema. It wrote nothing, so every contention SQLSTATE @@ -249,12 +311,11 @@ pub async fn list_sandboxes( .into_response(); } }; - let next_cursor = match usize::try_from(request.limit) { - Ok(limit) if sandboxes.len() > limit => { - sandboxes.truncate(limit); - sandboxes.last().map(encode_cursor) - } - _ => None, + let sandboxes = page.rows; + let next_cursor = if page.has_more { + sandboxes.last().map(encode_cursor) + } else { + None }; // The node-reported rows with no sandbox row are not in the keyset order // and belong to no org, so they come once, on the first page of the @@ -297,7 +358,7 @@ pub async fn list_sandboxes( #[derive(Debug, Default, Deserialize)] #[serde(deny_unknown_fields)] -pub struct SpacedVersionCountQuery { +struct SpacedVersionCountQuery { org_id: Option, } @@ -306,19 +367,30 @@ struct SpacedVersionCounts { counts: Vec, } -/// Every sandbox that is not deleted, counted by template, the `spaced` -/// version its node last reported, and stored lifecycle. +/// The listed sandboxes whose status is not `deleted`, counted by the +/// template, `spaced` version, status and resource type the list shows; see +/// `SandboxController::admin_spaced_version_counts`. pub async fn count_spaced_versions( State(state): State, Extension(AuthUser(actor)): Extension, - Query(query): Query, + uri: Uri, ) -> axum::response::Response { if let Err(err) = require_managing_org_admin(&state.pool, &actor).await { return err.into_response(); } - match sandbox_store::count_by_spaced_version(&state.pool, query.org_id).await { + let query: SpacedVersionCountQuery = match parse_query(&uri) { + Ok(query) => query, + Err(err) => return err.into_response(), + }; + match state + .sandbox_controller + .admin_spaced_version_counts(&state.pool, query.org_id) + .await + { Ok(counts) => Json(SpacedVersionCounts { counts }).into_response(), - Err(error) => support::read_db_error(error), + Err(error) => support::controller_read_error(&error, || { + tracing::warn!(%error, "admin spaced version count failed"); + }), } } diff --git a/crates/libs/api/src/routes/admin/sandbox_list/tests.rs b/crates/libs/api/src/routes/admin/sandbox_list/tests.rs index 017271d9b9..f2cf16aeb2 100644 --- a/crates/libs/api/src/routes/admin/sandbox_list/tests.rs +++ b/crates/libs/api/src/routes/admin/sandbox_list/tests.rs @@ -10,8 +10,8 @@ use tower::ServiceExt; use uuid::Uuid; use super::*; -use crate::routes::admin::sandbox_store::tests::{ - SeedSandbox, seed_observation, seed_owner, seed_sandbox, +use crate::controllers::sandbox::admin_list::tests::{ + SeedObservation, SeedSandbox, seed_observation, seed_owner, seed_sandbox, }; const SEED_ORG: Uuid = pplx_space_db_test_fixtures::SEED_ORG_ID; @@ -51,17 +51,27 @@ async fn insert_session_user(pool: &PgPool, role: &str) -> TestDbResult Ok(token) } -async fn harness(pool: PgPool) -> TestDbResult { +/// `None` when this test process cannot pin the environment +/// `AppState::from_pool` reads. +async fn harness(pool: PgPool) -> TestDbResult> { + // `AppState::from_pool` picks its substrate from the environment; keep it + // Direct so construction never reaches a cluster. + if !pplx_space_test_env::enter_test_process(&[( + "SPACE_K8S_CONTROL_PLANE", + Some(std::ffi::OsStr::new("false")), + )]) { + return Ok(None); + } let admin_token = insert_session_user(&pool, "organization_admin").await?; let member_token = insert_session_user(&pool, "member").await?; let state = AppState::from_pool(pool.clone()).await?; let app = crate::routes::admin::shared_host_router(state.clone()).with_state(state); - Ok(Harness { + Ok(Some(Harness { pool, app, admin_token, member_token, - }) + })) } impl Harness { @@ -112,7 +122,9 @@ fn ids(rows: &Value) -> Vec { #[tokio::test] async fn the_list_pages_by_cursor_and_keeps_the_unpaged_array() -> TestDbResult<()> { let db = pg_test_db().await?; - let harness = harness(db.pool.clone()).await?; + let Some(harness) = harness(db.pool.clone()).await? else { + return Ok(()); + }; let owner = seed_owner(&harness.pool).await?; let mut seeded = Vec::new(); for _ in 0..5 { @@ -127,8 +139,12 @@ async fn the_list_pages_by_cursor_and_keeps_the_unpaged_array() -> TestDbResult< &harness.pool, node_only.parse().expect("sandbox id"), node_id, - Some("py"), - Some("1.0.0"), + SeedObservation { + status: Some("started"), + pool_template: Some("py"), + spaced_version: Some("1.0.0"), + ..SeedObservation::default() + }, ) .await?; let with_node_only = |rows: &[String]| { @@ -171,9 +187,11 @@ async fn the_list_pages_by_cursor_and_keeps_the_unpaged_array() -> TestDbResult< } walked.extend(rows); pages += 1; - match page["next_cursor"].as_str() { - Some(next) => cursor = next.to_string(), - None => break, + match page.get("next_cursor") { + Some(Value::String(next)) => cursor = next.clone(), + // `null`, never absent, on the last page. + Some(Value::Null) => break, + other => panic!("{scope}: next_cursor {other:?}"), } } assert_eq!(pages, 3, "{scope}"); @@ -188,12 +206,26 @@ async fn the_list_pages_by_cursor_and_keeps_the_unpaged_array() -> TestDbResult< Ok(()) } +/// Every malformed parameter, typed ones included, answers 400 with the +/// admin JSON `error`; with `cursor`, so does an unknown parameter. #[tokio::test] async fn malformed_list_parameters_answer_400() -> TestDbResult<()> { let db = pg_test_db().await?; - let harness = harness(db.pool.clone()).await?; + let Some(harness) = harness(db.pool.clone()).await? else { + return Ok(()); + }; let node = NodeId::new(); for (query, needle) in [ + ("limit=many", "limit"), + ("org_id=not-a-uuid", "org_id"), + ("node_id=not-a-uuid", "node_id"), + ("user_id=not-a-uuid", "user_id"), + ("assigned=maybe", "assigned"), + ( + "cursor=&lifecycel=started", + "unknown query parameter \"lifecycel\"", + ), + ("prefix=a%00b", "NUL"), ("cursor=bm90LWEtY3Vyc29y", "invalid cursor"), ("cursor=%%%", "invalid"), ("lifecycle=started,napping", "unknown lifecycle"), @@ -208,11 +240,58 @@ async fn malformed_list_parameters_answer_400() -> TestDbResult<()> { ] { let (status, body) = harness.get(&format!("/sandboxes?{query}")).await; assert_eq!(status, StatusCode::BAD_REQUEST, "{query}: {body}"); - let message = body["error"] - .as_str() - .unwrap_or(body.as_str().unwrap_or_default()); + let message = body["error"].as_str().unwrap_or_default(); assert!(message.contains(needle), "{query}: {body}"); } + for query in ["org_id=not-a-uuid", "template=py"] { + let (status, body) = harness + .get(&format!("/sandboxes/spaced-versions?{query}")) + .await; + assert_eq!(status, StatusCode::BAD_REQUEST, "{query}: {body}"); + assert!(body["error"].is_string(), "{query}: {body}"); + } + // Without `cursor`, an unknown parameter is ignored, as before paging. + let (status, body) = harness.get("/sandboxes?lifecycel=started").await; + assert_eq!(status, StatusCode::OK, "{body}"); + assert!(body.is_array(), "{body}"); + db.pool.close().await; + Ok(()) +} + +/// Postgres keeps microseconds, so a `created_before` inside a microsecond +/// still includes a row created at its start. +#[tokio::test] +async fn created_before_keeps_a_row_inside_its_microsecond() -> TestDbResult<()> { + let db = pg_test_db().await?; + let Some(harness) = harness(db.pool.clone()).await? else { + return Ok(()); + }; + let owner = seed_owner(&harness.pool).await?; + let created_at = OffsetDateTime::from_unix_timestamp(1_790_000_000).expect("timestamp"); + let sandbox_id = seed_sandbox( + &harness.pool, + owner, + SeedSandbox { + created_at: Some(created_at), + ..SeedSandbox::default() + }, + ) + .await?; + let org = owner.org_id.as_uuid(); + for (before, expected) in [ + ( + "2026-09-21T14:13:20.000000500Z", + vec![sandbox_id.to_string()], + ), + ("2026-09-21T14:13:20.000001Z", vec![sandbox_id.to_string()]), + ("2026-09-21T14:13:20Z", vec![]), + ] { + let (status, body) = harness + .get(&format!("/sandboxes?org_id={org}&created_before={before}")) + .await; + assert_eq!(status, StatusCode::OK, "{before}: {body}"); + assert_eq!(ids(&body), expected, "{before}"); + } db.pool.close().await; Ok(()) } @@ -220,7 +299,9 @@ async fn malformed_list_parameters_answer_400() -> TestDbResult<()> { #[tokio::test] async fn the_list_and_the_count_require_a_managing_org_admin() -> TestDbResult<()> { let db = pg_test_db().await?; - let harness = harness(db.pool.clone()).await?; + let Some(harness) = harness(db.pool.clone()).await? else { + return Ok(()); + }; for path in [ "/sandboxes", "/sandboxes?cursor=", @@ -235,12 +316,15 @@ async fn the_list_and_the_count_require_a_managing_org_admin() -> TestDbResult<( Ok(()) } -/// The count is its own route, not a sandbox ID, and answers the store's -/// rows for one org on request. +/// The count is its own route, not a sandbox ID, and answers the +/// controller's buckets for one org on request. Their contents are pinned +/// against the list in `admin_list::tests`. #[tokio::test] async fn the_spaced_version_count_is_served() -> TestDbResult<()> { let db = pg_test_db().await?; - let harness = harness(db.pool.clone()).await?; + let Some(harness) = harness(db.pool.clone()).await? else { + return Ok(()); + }; let owner = seed_owner(&harness.pool).await?; let node_id = NodeId::new(); insert_ready_worker_node(&harness.pool, node_id).await?; @@ -253,7 +337,17 @@ async fn the_spaced_version_count_is_served() -> TestDbResult<()> { }, ) .await?; - seed_observation(&harness.pool, sandbox_id, node_id, None, Some("1.4.0")).await?; + seed_observation( + &harness.pool, + sandbox_id, + node_id, + SeedObservation { + status: Some("started"), + spaced_version: Some("1.4.0"), + ..SeedObservation::default() + }, + ) + .await?; let (status, body) = harness .get(&format!( "/sandboxes/spaced-versions?org_id={}", @@ -264,7 +358,13 @@ async fn the_spaced_version_count_is_served() -> TestDbResult<()> { assert_eq!( body, json!({"counts": [ - {"template": "py", "spaced_version": "1.4.0", "lifecycle": "started", "count": 1} + { + "template": "py", + "spaced_version": "1.4.0", + "status": "started", + "resource_type": "sandbox", + "count": 1 + } ]}) ); let (status, body) = harness.get("/sandboxes/spaced-versions?template=py").await; @@ -278,7 +378,9 @@ async fn the_spaced_version_count_is_served() -> TestDbResult<()> { #[tokio::test] async fn every_admin_sandbox_route_takes_every_id_form() -> TestDbResult<()> { let db = pg_test_db().await?; - let harness = harness(db.pool.clone()).await?; + let Some(harness) = harness(db.pool.clone()).await? else { + return Ok(()); + }; let owner = seed_owner(&harness.pool).await?; let sandbox_id = seed_sandbox(&harness.pool, owner, SeedSandbox::default()).await?; let public_id = PublicSandboxId::from(sandbox_id).to_string(); diff --git a/crates/libs/api/src/routes/admin/sandbox_store.rs b/crates/libs/api/src/routes/admin/sandbox_store.rs deleted file mode 100644 index 4fcbc29769..0000000000 --- a/crates/libs/api/src/routes/admin/sandbox_store.rs +++ /dev/null @@ -1,228 +0,0 @@ -//! Postgres-side queries that back the admin sandbox list and the -//! spaced-version count. -//! -//! The handlers in [`super::sandbox_list`] stay focused on HTTP: parsing the -//! query string, auth, and the response shape. The statements live here. - -use std::sync::LazyLock; - -use pplx_space_common_core::sql; -use pplx_space_common_core::types::{NodeId, OrgId, SandboxId, TemplateName, UserId}; -use pplx_space_placement::StoredSandboxLifecycle; -use pplx_space_public_api::PUBLIC_SANDBOX_ID_PREFIX; -use serde::Serialize; -use sqlx::{FromRow, PgExecutor}; -use time::OffsetDateTime; -use uuid::Uuid; - -use crate::controllers::sandbox::{SandboxPageCursor, sandbox_query}; -use crate::models::Sandbox; - -/// What the admin sandbox list narrows to. Every field that is set must -/// match; an unset field matches every row. -#[derive(Debug, Clone, Default)] -pub(super) struct AdminSandboxFilter { - pub(super) org_id: Option, - /// The stored lifecycle (`sandboxes.lifecycle`), any of these. Never - /// empty. - pub(super) lifecycles: Option>, - /// The row's own template (`sandboxes.template_id`). - pub(super) template: Option, - pub(super) node: Option, - pub(super) user_id: Option, - /// Created strictly after this instant. - pub(super) created_after: Option, - /// Created strictly before this instant. - pub(super) created_before: Option, - pub(super) search: Option, -} - -impl AdminSandboxFilter { - /// Whether no filter is set. Only the unfiltered list carries the - /// node-reported rows with no sandbox row: they belong to no org, and - /// none of the other filters can be applied to them. - pub(super) fn is_unfiltered(&self) -> bool { - let Self { - org_id, - lifecycles, - template, - node, - user_id, - created_after, - created_before, - search, - } = self; - org_id.is_none() - && lifecycles.is_none() - && template.is_none() - && node.is_none() - && user_id.is_none() - && created_after.is_none() - && created_before.is_none() - && search.is_none() - } -} - -/// The node a sandbox is assigned to, as the list response reports it in -/// `active_node_id`. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub(super) enum NodeAssignment { - Node(NodeId), - Assigned, - Unassigned, -} - -/// A name or ID prefix. A row matches when its name starts with the prefix -/// as given, or when the prefix, with an optional `sbx_` and dashes where a -/// UUID has them, is the start of its ID. -#[derive(Debug, Clone, PartialEq, Eq)] -pub(super) struct SandboxSearch { - prefix: String, - /// The lowest and highest IDs that start with the prefix, when it can - /// start an ID. Postgres compares UUIDs byte by byte, so a hex prefix is - /// exactly this range, which the primary key serves. - id_range: Option<(SandboxId, SandboxId)>, -} - -impl SandboxSearch { - pub(super) fn new(prefix: String) -> Self { - let id_range = id_prefix_range(&prefix); - Self { prefix, id_range } - } -} - -const DASHED_UUID_DASH_POSITIONS: [usize; 4] = [8, 13, 18, 23]; -const UNDASHED_UUID_LEN: usize = 32; - -fn id_prefix_range(prefix: &str) -> Option<(SandboxId, SandboxId)> { - let id = prefix - .strip_prefix(PUBLIC_SANDBOX_ID_PREFIX) - .unwrap_or(prefix); - let dashed = id.contains('-'); - let mut hex = String::with_capacity(UNDASHED_UUID_LEN); - for (position, character) in id.chars().enumerate() { - if dashed && DASHED_UUID_DASH_POSITIONS.contains(&position) { - if character != '-' { - return None; - } - } else if character.is_ascii_hexdigit() { - hex.push(character.to_ascii_lowercase()); - } else { - return None; - } - } - if hex.len() > UNDASHED_UUID_LEN { - return None; - } - let bound = |fill: char| { - let mut digits = hex.clone(); - digits.extend(std::iter::repeat_n(fill, UNDASHED_UUID_LEN - hex.len())); - Uuid::try_parse(&digits).ok().map(SandboxId::from_uuid) - }; - Some((bound('0')?, bound('f')?)) -} - -/// The admin list: newest first by `(created_at, id)`, after `after` when -/// set. `active_node_id` is the response's: the direct route's current node -/// when the sandbox has one, else the row's. -static SANDBOX_PAGE_SQL: LazyLock = LazyLock::new(|| { - sandbox_query( - "SELECT {sandbox_columns} - FROM {sandbox_from} - WHERE ($2::uuid IS NULL OR s.org_id = $2) - AND (s.created_at, s.id) < (COALESCE($3::TIMESTAMPTZ, 'infinity'), - COALESCE($4::UUID, '00000000-0000-0000-0000-000000000000')) - AND ($5::TEXT[] IS NULL OR s.lifecycle = ANY($5::TEXT[])) - AND ($6::TEXT IS NULL OR s.template_id = $6) - AND ($7::UUID IS NULL OR COALESCE(direct_route.current_node_id, s.active_node_id) = $7) - AND ($8::BOOLEAN IS NULL - OR (COALESCE(direct_route.current_node_id, s.active_node_id) IS NOT NULL) = $8) - AND ($9::UUID IS NULL OR s.user_id = $9) - AND ($10::TIMESTAMPTZ IS NULL OR s.created_at > $10) - AND ($11::TIMESTAMPTZ IS NULL OR s.created_at < $11) - AND ($12::TEXT IS NULL - OR starts_with(s.name, $12) - OR s.id BETWEEN $13::UUID AND $14::UUID) - ORDER BY s.created_at DESC, s.id DESC - LIMIT $1", - ) -}); - -/// At most `max_rows` sandboxes matching `filter`, newest first, after the -/// cursor when one is set. -pub(super) async fn list_sandboxes( - executor: impl PgExecutor<'_>, - filter: &AdminSandboxFilter, - after: Option, - max_rows: i64, -) -> Result, sqlx::Error> { - let lifecycles = filter.lifecycles.as_ref().map(|lifecycles| { - lifecycles - .iter() - .map(|lifecycle| lifecycle.as_str()) - .collect::>() - }); - let (node_id, assigned) = match filter.node { - Some(NodeAssignment::Node(node_id)) => (Some(node_id), None), - Some(NodeAssignment::Assigned) => (None, Some(true)), - Some(NodeAssignment::Unassigned) => (None, Some(false)), - None => (None, None), - }; - let id_range = filter.search.as_ref().and_then(|search| search.id_range); - sqlx::query_as::<_, Sandbox>(sqlx::AssertSqlSafe(sql!(SANDBOX_PAGE_SQL.as_str()))) - .bind(max_rows) - .bind(filter.org_id) - .bind(after.map(|cursor| cursor.created_at)) - .bind(after.map(|cursor| cursor.id)) - .bind(lifecycles) - .bind(filter.template.as_ref().map(TemplateName::as_str)) - .bind(node_id) - .bind(assigned) - .bind(filter.user_id) - .bind(filter.created_after) - .bind(filter.created_before) - .bind(filter.search.as_ref().map(|search| search.prefix.as_str())) - .bind(id_range.map(|(low, _)| low)) - .bind(id_range.map(|(_, high)| high)) - .fetch_all(executor) - .await -} - -/// How many sandboxes run each template at each `spaced` version, by stored -/// lifecycle. -#[derive(Debug, Clone, PartialEq, Eq, FromRow, Serialize)] -pub(super) struct SpacedVersionCount { - /// The row's template, else the one its node reports. - pub(super) template: Option, - /// The version its node last reported; `None` before any report. - pub(super) spaced_version: Option, - pub(super) lifecycle: String, - pub(super) count: i64, -} - -/// [`SpacedVersionCount`]s over every sandbox not deleted, in `org_id` when -/// set. -pub(super) async fn count_by_spaced_version( - executor: impl PgExecutor<'_>, - org_id: Option, -) -> Result, sqlx::Error> { - sqlx::query_as::<_, SpacedVersionCount>(sql!( - "SELECT COALESCE(s.template_id, sos.template, sos.pool_template) AS template, - sos.spaced_version, - s.lifecycle, - COUNT(*)::BIGINT AS count - FROM sandboxes s - LEFT JOIN sandbox_observed_states sos ON sos.sandbox_id = s.id - WHERE s.lifecycle <> $1 - AND ($2::UUID IS NULL OR s.org_id = $2) - GROUP BY 1, 2, 3 - ORDER BY 1 NULLS LAST, 2 NULLS LAST, 3" - )) - .bind(StoredSandboxLifecycle::Deleted.as_str()) - .bind(org_id) - .fetch_all(executor) - .await -} - -#[cfg(test)] -pub(super) mod tests; From 77c635b21b8c4c42f3a92ddd2758b87c5ca94c5a Mon Sep 17 00:00:00 2001 From: Abhinav Chordia Date: Mon, 2 Nov 2026 21:01:31 +0000 Subject: [PATCH 3/5] api: drop the admin list's node filters and add no Direct table read The node_id and assigned filters read sandbox_direct_route_identities in a new join, which no owning read covers yet; they go until the Direct route has one. The console lists a node's sandboxes with GET /nodes/{id}/sandboxes. The unpaged array runs a plain list statement again, without the page walk's size estimate it never used, and the admin page module moves under list_page so the sandbox controller's module list does not grow. The count test gains a stored-deleted row its node still reports started. --- .../api/src/controllers/sandbox/list_page.rs | 2 + .../{admin_list.rs => list_page/admin.rs} | 133 ++++++++++-------- .../{admin_list => list_page/admin}/tests.rs | 110 +-------------- .../libs/api/src/controllers/sandbox/mod.rs | 1 - crates/libs/api/src/routes/admin/docs.rs | 4 +- .../libs/api/src/routes/admin/sandbox_list.rs | 24 +--- .../src/routes/admin/sandbox_list/tests.rs | 10 +- 7 files changed, 92 insertions(+), 192 deletions(-) rename crates/libs/api/src/controllers/sandbox/{admin_list.rs => list_page/admin.rs} (75%) rename crates/libs/api/src/controllers/sandbox/{admin_list => list_page/admin}/tests.rs (87%) diff --git a/crates/libs/api/src/controllers/sandbox/list_page.rs b/crates/libs/api/src/controllers/sandbox/list_page.rs index f38b597a89..689c3bfe74 100644 --- a/crates/libs/api/src/controllers/sandbox/list_page.rs +++ b/crates/libs/api/src/controllers/sandbox/list_page.rs @@ -16,6 +16,8 @@ pub(crate) const MAX_SANDBOX_LIST_PAGE_BYTES: i64 = 8 * 1024 * 1024; /// of native JSON, up to about 1.1 KB with the longest template reference. pub(crate) const SANDBOX_LIST_ROW_OVERHEAD_BYTES: i64 = 2 * 1024; +pub(crate) mod admin; + /// The placeholders of the bound values [`page_rows_cte`] reads, such as /// `"$11"`, and the columns it sizes. The caller binds the cursor's /// `created_at` and `id` (both NULL for the first page), the most rows to diff --git a/crates/libs/api/src/controllers/sandbox/admin_list.rs b/crates/libs/api/src/controllers/sandbox/list_page/admin.rs similarity index 75% rename from crates/libs/api/src/controllers/sandbox/admin_list.rs rename to crates/libs/api/src/controllers/sandbox/list_page/admin.rs index a22e9db9e6..b3e274248c 100644 --- a/crates/libs/api/src/controllers/sandbox/admin_list.rs +++ b/crates/libs/api/src/controllers/sandbox/list_page/admin.rs @@ -1,16 +1,17 @@ //! The admin sandbox list's filters and pages, and its count of sandboxes by //! template and `spaced` version. //! -//! A page is the ECW list's page walk ([`super::list_page`]) over the admin -//! candidate statement below, with the same full-row select, so both lists -//! resolve rows and stop at the byte budget the same way. The admin route's -//! legacy unpaged array runs the same statement with no byte budget, since it -//! has no cursor to continue from. +//! A page is the ECW list's page walk ([`super`]) over the admin +//! candidate statement below, composed with the same full-row select +//! (`org::scoped_page_sql`), so both lists resolve rows and stop at the byte +//! budget the same way. The admin route's legacy unpaged array has no cursor +//! to continue from, so it runs a plain list statement with the same filters +//! and no size walk. use std::sync::LazyLock; use pplx_space_common_core::sql; -use pplx_space_common_core::types::{NodeId, OrgId, SandboxId, SemVer, TemplateName, UserId}; +use pplx_space_common_core::types::{OrgId, SandboxId, SemVer, TemplateName, UserId}; use pplx_space_placement::StoredSandboxLifecycle; use pplx_space_public_api::PUBLIC_SANDBOX_ID_PREFIX; use serde::Serialize; @@ -18,11 +19,11 @@ use sqlx::PgPool; use time::OffsetDateTime; use uuid::Uuid; -use super::list_page::{ +use super::super::org::{ScopedPageRow, scoped_page_sql}; +use super::{ ListPage, MAX_SANDBOX_LIST_PAGE_BYTES, PageWalkParams, SANDBOX_LIST_ROW_OVERHEAD_BYTES, }; -use super::org::{ScopedPageRow, scoped_page_sql}; -use super::{SandboxController, SandboxPageCursor}; +use crate::controllers::sandbox::{SandboxController, SandboxPageCursor, sandbox_query}; use crate::controllers::{ControllerError, db_contention_error}; use crate::models::{Sandbox, SandboxResourceType}; @@ -38,7 +39,6 @@ pub struct AdminSandboxFilter { /// The row's own template (`sandboxes.template_id`), not the fallbacks /// the response's `template` resolves when the row has none. pub template: Option, - pub node: Option, pub user_id: Option, /// Created strictly after this instant. pub created_after: Option, @@ -56,7 +56,6 @@ impl AdminSandboxFilter { org_id, lifecycles, template, - node, user_id, created_after, created_before, @@ -65,7 +64,6 @@ impl AdminSandboxFilter { org_id.is_none() && lifecycles.is_none() && template.is_none() - && node.is_none() && user_id.is_none() && created_after.is_none() && created_before.is_none() @@ -73,16 +71,6 @@ impl AdminSandboxFilter { } } -/// The node a sandbox is assigned to, as the list response reports it in -/// `active_node_id`: the direct route's current node when the sandbox has -/// one, else the row's `active_node_id`. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum NodeAssignment { - Node(NodeId), - Assigned, - Unassigned, -} - /// A name or ID prefix. A row matches when its name starts with the prefix /// as given, or when the prefix, with an optional `sbx_` and dashes where a /// UUID has them, is the start of its ID. @@ -133,24 +121,19 @@ fn id_prefix_range(prefix: &str) -> Option<(SandboxId, SandboxId)> { Some((bound('0')?, bound('f')?)) } -/// The admin list's sandboxes ($1-$11) after the cursor. `active_node_id` is -/// the response's (`SANDBOX_COLUMNS`): the direct route's current node, else -/// the row's. +/// The admin list's sandboxes ($1-$9) after the cursor, for the page walk. +/// The filters are the same as [`ADMIN_LIST_SQL`]'s. const ADMIN_CANDIDATE_ROWS: &str = "SELECT s.id, s.created_at FROM sandboxes s - LEFT JOIN sandbox_direct_route_identities direct_route ON direct_route.sandbox_id = s.id WHERE ($1::UUID IS NULL OR s.org_id = $1) AND ($2::TEXT[] IS NULL OR s.lifecycle = ANY($2::TEXT[])) AND ($3::TEXT IS NULL OR s.template_id = $3) - AND ($4::UUID IS NULL OR COALESCE(direct_route.current_node_id, s.active_node_id) = $4) - AND ($5::BOOLEAN IS NULL - OR (COALESCE(direct_route.current_node_id, s.active_node_id) IS NOT NULL) = $5) - AND ($6::UUID IS NULL OR s.user_id = $6) - AND ($7::TIMESTAMPTZ IS NULL OR s.created_at > $7) - AND ($8::TIMESTAMPTZ IS NULL OR s.created_at < $8) - AND ($9::TEXT IS NULL - OR starts_with(s.name, $9) - OR s.id BETWEEN $10::UUID AND $11::UUID) + AND ($4::UUID IS NULL OR s.user_id = $4) + AND ($5::TIMESTAMPTZ IS NULL OR s.created_at > $5) + AND ($6::TIMESTAMPTZ IS NULL OR s.created_at < $6) + AND ($7::TEXT IS NULL + OR starts_with(s.name, $7) + OR s.id BETWEEN $8::UUID AND $9::UUID) AND {after_cursor} ORDER BY s.created_at DESC, s.id DESC LIMIT {max_rows}"; @@ -159,24 +142,46 @@ static ADMIN_PAGE_SQL: LazyLock = LazyLock::new(|| { scoped_page_sql( ADMIN_CANDIDATE_ROWS, &PageWalkParams { - cursor_created_at: "$12", - cursor_id: "$13", - max_rows: "$14", - max_bytes: "$15", - row_overhead_bytes: "$16", + cursor_created_at: "$10", + cursor_id: "$11", + max_rows: "$12", + max_bytes: "$13", + row_overhead_bytes: "$14", // The admin response carries no `env`. lists_env: false, }, ) }); +/// The legacy unpaged list: at most $10 sandboxes matching the filters +/// ($1-$9, as [`ADMIN_CANDIDATE_ROWS`]'s), newest first. +static ADMIN_LIST_SQL: LazyLock = LazyLock::new(|| { + sandbox_query( + "SELECT {sandbox_columns} + FROM {sandbox_from} + WHERE ($1::UUID IS NULL OR s.org_id = $1) + AND ($2::TEXT[] IS NULL OR s.lifecycle = ANY($2::TEXT[])) + AND ($3::TEXT IS NULL OR s.template_id = $3) + AND ($4::UUID IS NULL OR s.user_id = $4) + AND ($5::TIMESTAMPTZ IS NULL OR s.created_at > $5) + AND ($6::TIMESTAMPTZ IS NULL OR s.created_at < $6) + AND ($7::TEXT IS NULL + OR starts_with(s.name, $7) + OR s.id BETWEEN $8::UUID AND $9::UUID) + ORDER BY s.created_at DESC, s.id DESC + LIMIT $10", + ) +}); + /// Whether a page stops at the list byte budget. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum PageBudget { - /// [`MAX_SANDBOX_LIST_PAGE_BYTES`], for a page a cursor continues. + /// [`MAX_SANDBOX_LIST_PAGE_BYTES`], for a page a cursor continues: the + /// page walk. Bounded, - /// No byte budget: the legacy unpaged array, which has no cursor to - /// continue from and answered up to its limit before paging existed. + /// No byte budget and no size walk: the legacy unpaged array, which has + /// no cursor to continue from and answered up to its limit before paging + /// existed. Unbounded, } @@ -208,6 +213,8 @@ struct SpacedVersionCountRow { impl SandboxController { /// One admin list page matching `filter`, newest first by `(created_at, /// id)`, after `cursor`: at most `limit` rows, and whether more follow. + /// A [`PageBudget::Unbounded`] list ignores `cursor` and never says more + /// follow. pub(crate) async fn admin_sandbox_page( &self, pool: &PgPool, @@ -222,34 +229,44 @@ impl SandboxController { .map(|lifecycle| lifecycle.as_str()) .collect::>() }); - let (node_id, assigned) = match filter.node { - Some(NodeAssignment::Node(node_id)) => (Some(node_id), None), - Some(NodeAssignment::Assigned) => (None, Some(true)), - Some(NodeAssignment::Unassigned) => (None, Some(false)), - None => (None, None), - }; let id_range = filter.search.as_ref().and_then(|search| search.id_range); - let max_bytes = match budget { - PageBudget::Bounded => MAX_SANDBOX_LIST_PAGE_BYTES, - PageBudget::Unbounded => i64::MAX, - }; + let template = filter.template.as_ref().map(TemplateName::as_str); + let prefix = filter.search.as_ref().map(|search| search.prefix.as_str()); + if budget == PageBudget::Unbounded { + let rows = + sqlx::query_as::<_, Sandbox>(sqlx::AssertSqlSafe(sql!(ADMIN_LIST_SQL.as_str()))) + .bind(filter.org_id) + .bind(lifecycles) + .bind(template) + .bind(filter.user_id) + .bind(filter.created_after) + .bind(filter.created_before) + .bind(prefix) + .bind(id_range.map(|(low, _)| low)) + .bind(id_range.map(|(_, high)| high)) + .bind(i64::from(limit)) + .fetch_all(pool) + .await?; + return Ok(ListPage { + rows, + has_more: false, + }); + } let rows = sqlx::query_as::<_, ScopedPageRow>(sqlx::AssertSqlSafe(sql!(ADMIN_PAGE_SQL.as_str()))) .bind(filter.org_id) .bind(lifecycles) - .bind(filter.template.as_ref().map(TemplateName::as_str)) - .bind(node_id) - .bind(assigned) + .bind(template) .bind(filter.user_id) .bind(filter.created_after) .bind(filter.created_before) - .bind(filter.search.as_ref().map(|search| search.prefix.as_str())) + .bind(prefix) .bind(id_range.map(|(low, _)| low)) .bind(id_range.map(|(_, high)| high)) .bind(cursor.map(|cursor| cursor.created_at)) .bind(cursor.map(|cursor| cursor.id)) .bind(i64::from(limit.saturating_add(1))) - .bind(max_bytes) + .bind(MAX_SANDBOX_LIST_PAGE_BYTES) .bind(SANDBOX_LIST_ROW_OVERHEAD_BYTES) .fetch_all(pool) .await?; diff --git a/crates/libs/api/src/controllers/sandbox/admin_list/tests.rs b/crates/libs/api/src/controllers/sandbox/list_page/admin/tests.rs similarity index 87% rename from crates/libs/api/src/controllers/sandbox/admin_list/tests.rs rename to crates/libs/api/src/controllers/sandbox/list_page/admin/tests.rs index c6d9c87c8f..b2a5582e16 100644 --- a/crates/libs/api/src/controllers/sandbox/admin_list/tests.rs +++ b/crates/libs/api/src/controllers/sandbox/list_page/admin/tests.rs @@ -1,6 +1,6 @@ use std::collections::BTreeMap; -use pplx_space_common_core::types::SubstrateKind; +use pplx_space_common_core::types::{NodeId, SubstrateKind}; use pplx_space_db_test_fixtures::{TestDbResult, insert_ready_worker_node, pg_test_db}; use time::Duration; @@ -396,30 +396,6 @@ async fn each_filter_returns_exactly_its_rows() -> TestDbResult<()> { }, &[0, 1, 3], ), - ( - "node", - AdminSandboxFilter { - node: Some(NodeAssignment::Node(node_one)), - ..AdminSandboxFilter::default() - }, - &[0, 4], - ), - ( - "assigned", - AdminSandboxFilter { - node: Some(NodeAssignment::Assigned), - ..AdminSandboxFilter::default() - }, - &[0, 1, 4, 5], - ), - ( - "unassigned", - AdminSandboxFilter { - node: Some(NodeAssignment::Unassigned), - ..AdminSandboxFilter::default() - }, - &[2, 3], - ), ( "user", AdminSandboxFilter { @@ -507,7 +483,8 @@ async fn each_filter_returns_exactly_its_rows() -> TestDbResult<()> { org_id: Some(first_org), lifecycles: lifecycles(&["started", "deleted"]), template: template("py"), - node: Some(NodeAssignment::Unassigned), + user_id: Some(first_user.user_id), + created_after: Some(at(2)), ..AdminSandboxFilter::default() }, &[3], @@ -570,84 +547,6 @@ fn an_id_prefix_is_the_range_of_ids_it_starts() { } } -/// The node-reported half of `node_id` and `assigned`: a sandbox whose -/// direct route names a node is on that node, whatever its row's -/// `active_node_id` says, exactly as the response reports it. -#[tokio::test] -async fn the_node_filters_follow_the_direct_route_the_response_reports() -> TestDbResult<()> { - let db = pg_test_db().await?; - let owner = seed_owner(&db.pool).await?; - let (row_node, route_node) = (NodeId::new(), NodeId::new()); - let moved = seed_sandbox( - &db.pool, - owner, - SeedSandbox { - active_node_id: Some(row_node), - ..SeedSandbox::default() - }, - ) - .await?; - let routed_only = seed_sandbox(&db.pool, owner, SeedSandbox::default()).await?; - let unassigned = seed_sandbox(&db.pool, owner, SeedSandbox::default()).await?; - for sandbox_id in [moved, routed_only] { - sqlx::query(sql!( - "INSERT INTO sandbox_direct_route_identities ( - sandbox_id, route_generation, current_node_id, substrate_kind, - execution_generation, activation_attempt_id - ) - VALUES ($1, 1, $2, $3, 1, $4)" - )) - .bind(sandbox_id) - .bind(route_node) - .bind(SubstrateKind::Space) - .bind(Uuid::now_v7()) - .execute(&db.pool) - .await?; - } - let filter = |node| AdminSandboxFilter { - org_id: Some(owner.org_id), - node: Some(node), - ..AdminSandboxFilter::default() - }; - let on_route_node = page( - &db.pool, - &filter(NodeAssignment::Node(route_node)), - None, - 100, - ) - .await?; - assert_eq!( - on_route_node - .rows - .iter() - .map(|sandbox| sandbox.id) - .collect::>(), - [routed_only, moved] - ); - assert!( - on_route_node - .rows - .iter() - .all(|sandbox| sandbox.active_node_id == Some(route_node)), - "the filter matches the node the response reports" - ); - assert!( - listed_ids(&db.pool, &filter(NodeAssignment::Node(row_node))) - .await? - .is_empty() - ); - assert_eq!( - listed_ids(&db.pool, &filter(NodeAssignment::Assigned)).await?, - [routed_only, moved] - ); - assert_eq!( - listed_ids(&db.pool, &filter(NodeAssignment::Unassigned)).await?, - [unassigned] - ); - db.pool.close().await; - Ok(()) -} - type Bucket = ( Option, Option, @@ -755,6 +654,9 @@ async fn the_count_is_the_tally_of_the_full_list() -> TestDbResult<()> { }), ), (owner, Some("py"), "deleted", None), + // A tombstone whose VM its node still reports: the list shows it + // `started`, so the console's tally counts it, and so must the count. + (owner, Some("py"), "deleted", Some(started(Some("1.2.0")))), // The node's status wins over the stored lifecycle, as in the list. ( owner, diff --git a/crates/libs/api/src/controllers/sandbox/mod.rs b/crates/libs/api/src/controllers/sandbox/mod.rs index cc14b0c867..ca0bdbd21f 100644 --- a/crates/libs/api/src/controllers/sandbox/mod.rs +++ b/crates/libs/api/src/controllers/sandbox/mod.rs @@ -1358,7 +1358,6 @@ impl SandboxWakeSignal { #[cfg(test)] mod reconnect_metrics_tests; -pub(crate) mod admin_list; mod ingress; pub(crate) mod lifecycle; pub(crate) mod list_page; diff --git a/crates/libs/api/src/routes/admin/docs.rs b/crates/libs/api/src/routes/admin/docs.rs index 067df1ceb5..78b9da767a 100644 --- a/crates/libs/api/src/routes/admin/docs.rs +++ b/crates/libs/api/src/routes/admin/docs.rs @@ -790,15 +790,13 @@ fn insert_admin_sandbox_create_paths(paths: &mut Map) { json!({ "get": { "summary": "List sandboxes across every organization", - "description": "Newest first by created_at, then id. Without cursor the response is a bare array of at most limit sandboxes; with no filter, it is followed by the sandboxes nodes report that have no sandbox row. Unknown parameters are then ignored, as before paging existed. Pass cursor, empty for the first page, to page: the response is then {sandboxes, next_cursor}, with next_cursor null on the last page. A page holds at most limit sandboxes and stops early at the list byte budget. The node-reported rows come on the first page only, and an unknown parameter answers 400. Each page reads the current matching rows strictly below the supplied (created_at, id) position. Paging is not a snapshot: concurrent changes can affect membership. Keep the filters unchanged between pages. Filters, org_id included, combine and leave out the node-reported rows. A malformed parameter answers 400 with an error message.", + "description": "Newest first by created_at, then id. Without cursor the response is a bare array of at most limit sandboxes; with no filter, it is followed by the sandboxes nodes report that have no sandbox row. Unknown parameters are then ignored, as before paging existed. Pass cursor, empty for the first page, to page: the response is then {sandboxes, next_cursor}, with next_cursor null on the last page. The limit and the estimated byte budget apply to the rows from the database, and the first such row is kept even when it is larger than the budget. The rows nodes report without a database row are all appended to the first unfiltered page, outside those bounds. The estimate does not count each row's network policy. With cursor, an unknown parameter answers 400. Each page reads the current matching rows strictly below the supplied (created_at, id) position. Paging is not a snapshot: concurrent changes can affect membership. Keep the filters unchanged between pages. Filters, org_id included, combine and leave out the node-reported rows. A malformed parameter answers 400 with an error message.", "parameters": [ { "name": "limit", "in": "query", "required": false, "description": "1 to 5000, default 1000", "schema": { "type": "integer" } }, { "name": "cursor", "in": "query", "required": false, "description": "Opaque; empty for the first page, then the previous page's next_cursor", "schema": { "type": "string" } }, { "name": "org_id", "in": "query", "required": false, "schema": { "type": "string" } }, { "name": "lifecycle", "in": "query", "required": false, "description": "Comma-separated values of the stored lifecycle, including deleted and cleanup_required. No response field shows the stored lifecycle: lifecycle and status report the node's observed status when there is one, so a row can show another value.", "schema": { "type": "string" } }, { "name": "template", "in": "query", "required": false, "description": "The sandbox row's own template. A row without one does not match, although the response's template, like the spaced-version count, falls back to the template its node reports.", "schema": { "type": "string" } }, - { "name": "node_id", "in": "query", "required": false, "description": "The node reported as active_node_id", "schema": { "type": "string" } }, - { "name": "assigned", "in": "query", "required": false, "description": "true for sandboxes with an active node, false for those without; false cannot be combined with node_id", "schema": { "type": "boolean" } }, { "name": "user_id", "in": "query", "required": false, "schema": { "type": "string" } }, { "name": "created_after", "in": "query", "required": false, "description": "RFC 3339; exclusive", "schema": { "type": "string" } }, { "name": "created_before", "in": "query", "required": false, "description": "RFC 3339; exclusive, later than created_after", "schema": { "type": "string" } }, diff --git a/crates/libs/api/src/routes/admin/sandbox_list.rs b/crates/libs/api/src/routes/admin/sandbox_list.rs index 6c09b9faa9..37ae8cef4f 100644 --- a/crates/libs/api/src/routes/admin/sandbox_list.rs +++ b/crates/libs/api/src/routes/admin/sandbox_list.rs @@ -11,7 +11,7 @@ use axum::{ response::IntoResponse, }; use base64::{Engine as _, engine::general_purpose::URL_SAFE_NO_PAD}; -use pplx_space_common_core::types::{NodeId, OrgId, SandboxId, TemplateName, UserId}; +use pplx_space_common_core::types::{OrgId, SandboxId, TemplateName, UserId}; use pplx_space_placement::StoredSandboxLifecycle; use serde::de::DeserializeOwned; use serde::{Deserialize, Serialize}; @@ -19,8 +19,8 @@ use time::OffsetDateTime; use super::support::{self, ContentionScope, api_error, require_managing_org_admin}; use crate::controllers::sandbox::SandboxPageCursor; -use crate::controllers::sandbox::admin_list::{ - AdminSandboxFilter, NodeAssignment, PageBudget, SandboxSearch, SpacedVersionCount, +use crate::controllers::sandbox::list_page::admin::{ + AdminSandboxFilter, PageBudget, SandboxSearch, SpacedVersionCount, }; use crate::middleware::auth::AuthUser; use crate::models::{Sandbox, SandboxResponse}; @@ -33,14 +33,12 @@ const CURSOR_VERSION: &str = "v1"; /// The list's query parameters. With `cursor` any other parameter is /// refused; without it, other parameters are ignored, as they were before /// paging existed, so callers written against that list keep working. -const LIST_PARAMETERS: [&str; 11] = [ +const LIST_PARAMETERS: [&str; 9] = [ "limit", "org_id", "cursor", "lifecycle", "template", - "node_id", - "assigned", "user_id", "created_after", "created_before", @@ -61,8 +59,6 @@ pub struct AdminSandboxListQuery { /// Comma-separated stored lifecycles. lifecycle: Option, template: Option, - node_id: Option, - assigned: Option, user_id: Option, created_after: Option, created_before: Option, @@ -133,17 +129,6 @@ impl AdminSandboxListQuery { Some("") => Paging::First, Some(cursor) => Paging::After(decode_cursor(cursor)?), }; - let node = match (self.node_id, self.assigned) { - (Some(_), Some(false)) => { - return Err(bad_request( - "node_id cannot be combined with assigned=false", - )); - } - (Some(node_id), _) => Some(NodeAssignment::Node(node_id)), - (None, Some(true)) => Some(NodeAssignment::Assigned), - (None, Some(false)) => Some(NodeAssignment::Unassigned), - (None, None) => None, - }; let created_after = self .created_after .as_deref() @@ -178,7 +163,6 @@ impl AdminSandboxListQuery { .map_err(|error| bad_request(format!("invalid template: {error}"))) }) .transpose()?, - node, user_id: self.user_id, created_after, created_before, diff --git a/crates/libs/api/src/routes/admin/sandbox_list/tests.rs b/crates/libs/api/src/routes/admin/sandbox_list/tests.rs index f2cf16aeb2..f7249541b3 100644 --- a/crates/libs/api/src/routes/admin/sandbox_list/tests.rs +++ b/crates/libs/api/src/routes/admin/sandbox_list/tests.rs @@ -2,6 +2,7 @@ use axum::Router; use axum::body::Body; use axum::http::{Method, Request}; use pplx_space_common_core::sql; +use pplx_space_common_core::types::NodeId; use pplx_space_db_test_fixtures::{TestDbResult, insert_ready_worker_node, pg_test_db}; use pplx_space_public_api::PublicSandboxId; use serde_json::{Value, json}; @@ -10,7 +11,7 @@ use tower::ServiceExt; use uuid::Uuid; use super::*; -use crate::controllers::sandbox::admin_list::tests::{ +use crate::controllers::sandbox::list_page::admin::tests::{ SeedObservation, SeedSandbox, seed_observation, seed_owner, seed_sandbox, }; @@ -214,24 +215,21 @@ async fn malformed_list_parameters_answer_400() -> TestDbResult<()> { let Some(harness) = harness(db.pool.clone()).await? else { return Ok(()); }; - let node = NodeId::new(); for (query, needle) in [ ("limit=many", "limit"), ("org_id=not-a-uuid", "org_id"), - ("node_id=not-a-uuid", "node_id"), ("user_id=not-a-uuid", "user_id"), - ("assigned=maybe", "assigned"), ( "cursor=&lifecycel=started", "unknown query parameter \"lifecycel\"", ), + ("cursor=&node_id=x", "unknown query parameter \"node_id\""), ("prefix=a%00b", "NUL"), ("cursor=bm90LWEtY3Vyc29y", "invalid cursor"), ("cursor=%%%", "invalid"), ("lifecycle=started,napping", "unknown lifecycle"), ("lifecycle=", "unknown lifecycle"), ("template=..", "invalid template"), - (&format!("node_id={node}&assigned=false"), "assigned=false"), ("created_after=yesterday", "created_after"), ( "created_after=2026-01-02T00:00:00Z&created_before=2026-01-01T00:00:00Z", @@ -318,7 +316,7 @@ async fn the_list_and_the_count_require_a_managing_org_admin() -> TestDbResult<( /// The count is its own route, not a sandbox ID, and answers the /// controller's buckets for one org on request. Their contents are pinned -/// against the list in `admin_list::tests`. +/// against the list in `list_page::admin::tests`. #[tokio::test] async fn the_spaced_version_count_is_served() -> TestDbResult<()> { let db = pg_test_db().await?; From eb9be9fa6ecc5cd97104a11f6e6902b745620a3f Mon Sep 17 00:00:00 2001 From: Abhinav Chordia Date: Tue, 3 Nov 2026 04:56:18 +0000 Subject: [PATCH 4/5] api: parse the admin list's lifecycle filter without the placement store pplx-space-placement is a store, and pplx-space-api is not one of its store_users, so the normal dependency the lifecycle filter added fails the required layers.store_edges check. The filter and the count now use AdminStoredLifecycle, a SandboxLifecycle state or one of the two tombstones, parsed with the same case and whitespace normalization. A test pins it to StoredSandboxLifecycle::all through the existing test-support dependency. --- crates/libs/api/Cargo.toml | 1 - .../controllers/sandbox/list_page/admin.rs | 49 +++++++++++++++++-- .../sandbox/list_page/admin/tests.rs | 38 ++++++++++++++ .../libs/api/src/routes/admin/sandbox_list.rs | 7 ++- 4 files changed, 86 insertions(+), 9 deletions(-) diff --git a/crates/libs/api/Cargo.toml b/crates/libs/api/Cargo.toml index c79876cc52..6fc8619d3b 100644 --- a/crates/libs/api/Cargo.toml +++ b/crates/libs/api/Cargo.toml @@ -89,7 +89,6 @@ pplx-space-events = { workspace = true } pplx-space-metrics = { workspace = true } pplx-space-observability = { workspace = true, default-features = false } pplx-space-public-api = { workspace = true, features = ["schema", "serde"] } -pplx-space-placement = { workspace = true } pplx-space-common-core = { workspace = true, features = ["serde", "sqlx"] } pplx-space-common-grpc = { workspace = true, features = ["internal-tls"] } pplx-space-common-svc = { workspace = true } diff --git a/crates/libs/api/src/controllers/sandbox/list_page/admin.rs b/crates/libs/api/src/controllers/sandbox/list_page/admin.rs index b3e274248c..77f27930e2 100644 --- a/crates/libs/api/src/controllers/sandbox/list_page/admin.rs +++ b/crates/libs/api/src/controllers/sandbox/list_page/admin.rs @@ -8,11 +8,13 @@ //! to continue from, so it runs a plain list statement with the same filters //! and no size walk. +use std::str::FromStr; use std::sync::LazyLock; use pplx_space_common_core::sql; -use pplx_space_common_core::types::{OrgId, SandboxId, SemVer, TemplateName, UserId}; -use pplx_space_placement::StoredSandboxLifecycle; +use pplx_space_common_core::types::{ + OrgId, SandboxId, SandboxLifecycle, SandboxLifecycleParseError, SemVer, TemplateName, UserId, +}; use pplx_space_public_api::PUBLIC_SANDBOX_ID_PREFIX; use serde::Serialize; use sqlx::PgPool; @@ -27,6 +29,45 @@ use crate::controllers::sandbox::{SandboxController, SandboxPageCursor, sandbox_ use crate::controllers::{ControllerError, db_contention_error}; use crate::models::{Sandbox, SandboxResourceType}; +const DELETED_LIFECYCLE: &str = "deleted"; + +/// The `sandboxes.lifecycle` values outside [`SandboxLifecycle`]. +const TOMBSTONE_LIFECYCLES: [&str; 2] = [DELETED_LIFECYCLE, "cleanup_required"]; + +/// A stored lifecycle (`sandboxes.lifecycle`) the admin list filters on: a +/// [`SandboxLifecycle`] state or a tombstone, in the column's form. +/// +/// `pplx-space-placement`'s `StoredSandboxLifecycle` names the same values, +/// but this crate is not one of that store's users; the tests pin this parse +/// to it. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct AdminStoredLifecycle(&'static str); + +impl AdminStoredLifecycle { + pub const fn as_str(self) -> &'static str { + self.0 + } +} + +impl FromStr for AdminStoredLifecycle { + type Err = SandboxLifecycleParseError; + + /// Normalizes case and surrounding whitespace, as `StoredSandboxLifecycle` + /// does. + fn from_str(value: &str) -> Result { + let normalized = value.trim().to_ascii_lowercase(); + if let Some(tombstone) = TOMBSTONE_LIFECYCLES + .into_iter() + .find(|tombstone| *tombstone == normalized) + { + return Ok(Self(tombstone)); + } + normalized + .parse::() + .map(|lifecycle| Self(lifecycle.as_str())) + } +} + /// What the admin sandbox list narrows to. Every field that is set must /// match; an unset field matches every row. #[derive(Debug, Clone, Default)] @@ -35,7 +76,7 @@ pub struct AdminSandboxFilter { /// The stored lifecycle (`sandboxes.lifecycle`), any of these. No /// response field shows it: `lifecycle` and `status` report the node's /// observed status when there is one. Never empty. - pub lifecycles: Option>, + pub lifecycles: Option>, /// The row's own template (`sandboxes.template_id`), not the fallbacks /// the response's `template` resolves when the row has none. pub template: Option, @@ -324,7 +365,7 @@ impl SandboxController { ORDER BY 1 NULLS LAST, 2 NULLS LAST, 3 NULLS LAST, 4 DESC, 5" )) .bind(org_id) - .bind(StoredSandboxLifecycle::Deleted.as_str()) + .bind(DELETED_LIFECYCLE) .fetch_all(pool) .await .map_err(|err| db_contention_error("count sandboxes by spaced version", &err))?; diff --git a/crates/libs/api/src/controllers/sandbox/list_page/admin/tests.rs b/crates/libs/api/src/controllers/sandbox/list_page/admin/tests.rs index b2a5582e16..65d2cb4593 100644 --- a/crates/libs/api/src/controllers/sandbox/list_page/admin/tests.rs +++ b/crates/libs/api/src/controllers/sandbox/list_page/admin/tests.rs @@ -504,6 +504,44 @@ async fn each_filter_returns_exactly_its_rows() -> TestDbResult<()> { Ok(()) } +/// The filter takes every value `sandboxes.lifecycle` holds, as the stored +/// vocabulary normalizes it, and nothing else. +#[test] +fn the_lifecycle_filter_takes_exactly_the_stored_lifecycles() { + for stored in pplx_space_placement::StoredSandboxLifecycle::all() { + let value = stored.as_str(); + for spelling in [ + value.to_string(), + format!(" {} ", value.to_ascii_uppercase()), + ] { + assert_eq!( + spelling + .parse::() + .map(AdminStoredLifecycle::as_str), + Ok(value), + "{spelling:?}" + ); + assert_eq!( + spelling.parse::(), + Ok(*stored), + "{spelling:?}" + ); + } + } + for unknown in ["", "delete", "cleanup-required", "live"] { + assert!( + unknown.parse::().is_err(), + "{unknown:?}" + ); + assert!( + unknown + .parse::() + .is_err(), + "{unknown:?}" + ); + } +} + #[test] fn an_id_prefix_is_the_range_of_ids_it_starts() { let range = |low: &str, high: &str| Some((fixed_id(low), fixed_id(high))); diff --git a/crates/libs/api/src/routes/admin/sandbox_list.rs b/crates/libs/api/src/routes/admin/sandbox_list.rs index 37ae8cef4f..19761bcd19 100644 --- a/crates/libs/api/src/routes/admin/sandbox_list.rs +++ b/crates/libs/api/src/routes/admin/sandbox_list.rs @@ -12,7 +12,6 @@ use axum::{ }; use base64::{Engine as _, engine::general_purpose::URL_SAFE_NO_PAD}; use pplx_space_common_core::types::{OrgId, SandboxId, TemplateName, UserId}; -use pplx_space_placement::StoredSandboxLifecycle; use serde::de::DeserializeOwned; use serde::{Deserialize, Serialize}; use time::OffsetDateTime; @@ -20,7 +19,7 @@ use time::OffsetDateTime; use super::support::{self, ContentionScope, api_error, require_managing_org_admin}; use crate::controllers::sandbox::SandboxPageCursor; use crate::controllers::sandbox::list_page::admin::{ - AdminSandboxFilter, PageBudget, SandboxSearch, SpacedVersionCount, + AdminSandboxFilter, AdminStoredLifecycle, PageBudget, SandboxSearch, SpacedVersionCount, }; use crate::middleware::auth::AuthUser; use crate::models::{Sandbox, SandboxResponse}; @@ -192,10 +191,10 @@ fn parse_query(uri: &Uri) -> Result { .map_err(|rejection| bad_request(rejection.body_text())) } -fn parse_lifecycles(value: &str) -> Result, QueryError> { +fn parse_lifecycles(value: &str) -> Result, QueryError> { let mut lifecycles = Vec::new(); for part in value.split(',') { - let lifecycle = StoredSandboxLifecycle::from_str(part) + let lifecycle = AdminStoredLifecycle::from_str(part) .map_err(|_| bad_request(format!("unknown lifecycle {part:?}")))?; if !lifecycles.contains(&lifecycle) { lifecycles.push(lifecycle); From 411c536b4901c5cabfa2c30f40d27a5f1ff3745c Mon Sep 17 00:00:00 2001 From: Abhinav Chordia Date: Tue, 3 Nov 2026 05:32:58 +0000 Subject: [PATCH 5/5] api: keep IDs and bodies out of admin list test messages CodeQL's rust/cleartext-logging flags assertion messages that format sandbox IDs, ID forms and response bodies. Label each table case with a static name or index instead of printing the value. Every check is unchanged. --- .../api/src/routes/admin/sandbox_id/tests.rs | 6 +-- .../src/routes/admin/sandbox_list/tests.rs | 43 +++++++++++-------- 2 files changed, 27 insertions(+), 22 deletions(-) diff --git a/crates/libs/api/src/routes/admin/sandbox_id/tests.rs b/crates/libs/api/src/routes/admin/sandbox_id/tests.rs index ef74658d79..80ed937736 100644 --- a/crates/libs/api/src/routes/admin/sandbox_id/tests.rs +++ b/crates/libs/api/src/routes/admin/sandbox_id/tests.rs @@ -27,11 +27,11 @@ fn every_accepted_form_names_the_same_sandbox() { format!(" {UUID}\t"), format!("\nsbx_{UUID} "), ]; - for form in accepted { + for (case, form) in accepted.iter().enumerate() { assert_eq!( - parse_admin_sandbox_id(&form), + parse_admin_sandbox_id(form), Ok(sandbox()), - "{form} should resolve" + "accepted form #{case} should resolve" ); } } diff --git a/crates/libs/api/src/routes/admin/sandbox_list/tests.rs b/crates/libs/api/src/routes/admin/sandbox_list/tests.rs index f7249541b3..bc9f57fba1 100644 --- a/crates/libs/api/src/routes/admin/sandbox_list/tests.rs +++ b/crates/libs/api/src/routes/admin/sandbox_list/tests.rs @@ -167,9 +167,14 @@ async fn the_list_pages_by_cursor_and_keeps_the_unpaged_array() -> TestDbResult< walk }), ] { + let label = if scope.is_empty() { + "unscoped" + } else { + "org-scoped" + }; let (status, unpaged) = harness.get(&format!("/sandboxes?{scope}limit=2")).await; - assert_eq!(status, StatusCode::OK, "{scope}: {unpaged}"); - assert_eq!(ids(&unpaged), first_page, "{scope}"); + assert_eq!(status, StatusCode::OK, "{label}: unpaged list status"); + assert_eq!(ids(&unpaged), first_page, "{label}"); let mut walked = Vec::new(); let mut cursor = String::new(); @@ -178,12 +183,12 @@ async fn the_list_pages_by_cursor_and_keeps_the_unpaged_array() -> TestDbResult< let (status, page) = harness .get(&format!("/sandboxes?{scope}limit=2&cursor={cursor}")) .await; - assert_eq!(status, StatusCode::OK, "{scope}: {page}"); + assert_eq!(status, StatusCode::OK, "{label}: page status"); let rows = ids(&page["sandboxes"]); if pages == 0 { assert_eq!( rows, first_page, - "{scope}: the first page is the unpaged list" + "{label}: the first page is the unpaged list" ); } walked.extend(rows); @@ -192,11 +197,11 @@ async fn the_list_pages_by_cursor_and_keeps_the_unpaged_array() -> TestDbResult< Some(Value::String(next)) => cursor = next.clone(), // `null`, never absent, on the last page. Some(Value::Null) => break, - other => panic!("{scope}: next_cursor {other:?}"), + _ => panic!("{label}: unexpected next_cursor"), } } - assert_eq!(pages, 3, "{scope}"); - assert_eq!(walked, walk, "{scope}"); + assert_eq!(pages, 3, "{label}"); + assert_eq!(walked, walk, "{label}"); } let (status, filtered) = harness.get("/sandboxes?lifecycle=started&cursor=").await; @@ -392,13 +397,13 @@ async fn every_admin_sandbox_route_takes_every_id_form() -> TestDbResult<()> { format!("space-sbx-{undashed}-g1"), format!("space-sbx-{undashed}"), ]; - for form in &forms { + for (case, form) in forms.iter().enumerate() { let (status, body) = harness.get(&format!("/sandboxes/{form}")).await; - assert_eq!(status, StatusCode::OK, "GET {form}: {body}"); - assert_eq!(body["id"], sandbox_id.to_string(), "GET {form}"); + assert_eq!(status, StatusCode::OK, "GET form #{case}"); + assert_eq!(body["id"], sandbox_id.to_string(), "GET form #{case}"); - let (status, body) = harness.get(&format!("/debug/sandbox/{form}")).await; - assert_eq!(status, StatusCode::OK, "debug {form}: {body}"); + let (status, _) = harness.get(&format!("/debug/sandbox/{form}")).await; + assert_eq!(status, StatusCode::OK, "debug form #{case}"); let (status, body) = harness .request( @@ -408,8 +413,8 @@ async fn every_admin_sandbox_route_takes_every_id_form() -> TestDbResult<()> { None, ) .await; - assert_eq!(status, StatusCode::ACCEPTED, "force-stop {form}: {body}"); - assert_eq!(body["sandbox_id"], public_id, "force-stop {form}"); + assert_eq!(status, StatusCode::ACCEPTED, "force-stop form #{case}"); + assert_eq!(body["sandbox_id"], public_id, "force-stop form #{case}"); let (status, body) = harness .request( @@ -419,10 +424,10 @@ async fn every_admin_sandbox_route_takes_every_id_form() -> TestDbResult<()> { Some(json!({"metadata": {"listed": "yes"}})), ) .await; - assert_eq!(status, StatusCode::ACCEPTED, "PATCH {form}: {body}"); - assert_eq!(body["sandbox_id"], public_id, "PATCH {form}"); + assert_eq!(status, StatusCode::ACCEPTED, "PATCH form #{case}"); + assert_eq!(body["sandbox_id"], public_id, "PATCH form #{case}"); - let (status, body) = harness + let (status, _) = harness .request( &harness.admin_token, Method::POST, @@ -430,8 +435,8 @@ async fn every_admin_sandbox_route_takes_every_id_form() -> TestDbResult<()> { Some(json!({"command": ["true"]})), ) .await; - assert_ne!(status, StatusCode::BAD_REQUEST, "commands {form}: {body}"); - assert_ne!(status, StatusCode::NOT_FOUND, "commands {form}: {body}"); + assert_ne!(status, StatusCode::BAD_REQUEST, "commands form #{case}"); + assert_ne!(status, StatusCode::NOT_FOUND, "commands form #{case}"); } let unknown = format!("{{{sandbox_id}}}");