diff --git a/.agents/skills/create-github-pr/SKILL.md b/.agents/skills/create-github-pr/SKILL.md index 8e14bc5f9a..5eafc91f9b 100644 --- a/.agents/skills/create-github-pr/SKILL.md +++ b/.agents/skills/create-github-pr/SKILL.md @@ -187,7 +187,7 @@ gh pr create \ --body "$(cat <<'EOF' ## Summary -Add `--limit` and `--offset` flags to `openshell sandbox list` for pagination. +Add `--page-size` and `--page-token` flags to `openshell sandbox list` for continuation-token pagination. ## Related Issue @@ -195,9 +195,9 @@ Closes #456 ## Changes -- Added `offset` and `limit` query parameters to the sandbox list API call -- Default limit is 20, max is 100 -- Response includes `total_count` field +- Added `page_size` and `page_token` fields to the sandbox list API call +- Default page size is 100, max is 1,000 +- Structured responses include `next_page_token` ## Testing diff --git a/.agents/skills/tui-development/SKILL.md b/.agents/skills/tui-development/SKILL.md index edd1397d56..ceb6d45405 100644 --- a/.agents/skills/tui-development/SKILL.md +++ b/.agents/skills/tui-development/SKILL.md @@ -170,13 +170,15 @@ Phase 1: GetSandboxLogs → 500 initial lines → send via Event::LogLines Phase 2: WatchSandbox(follow_logs: true) → live tail → send via Event::LogLines ``` -**Sandboxes**: Fetched via `ListSandboxes` on a 2-second tick, scoped to the current workspace (or all workspaces). +**Sandboxes**: Fetched via `ListSandboxes` in a background collection-refresh task scheduled from the 2-second tick, scoped to the current workspace (or all workspaces). Follow `next_page_token` until empty so the dashboard reflects the complete collection. -**Providers**: Fetched via `ListProviders` on each tick. Provider profiles are fetched per-workspace via `ListProviderProfiles` and cached in a `ProviderProfileCache` keyed by `(workspace, profile_id)`. +**Providers**: Fetched via `ListProviders` in the background collection-refresh task. Provider profiles are fetched per-workspace via `ListProviderProfiles` and cached in a `ProviderProfileCache` keyed by `(workspace, profile_id)`. Follow each list RPC's `next_page_token` until empty. **Settings**: Global settings are fetched via `GetGatewayConfig` on each tick. Sandbox settings are fetched alongside the sandbox policy via `GetSandboxConfig` and refreshed on each tick when viewing a sandbox. -**Workspaces**: The workspace list is fetched via `ListWorkspaces` on each tick. +**Workspaces**: The workspace list is fetched via `ListWorkspaces` in the background collection-refresh task, following `next_page_token` until empty. + +Only one collection-refresh task may run at a time. Workspace and gateway changes abort the active task, and refresh results carry their gateway/workspace context so stale results are discarded. ### Never block the event loop @@ -411,9 +413,9 @@ All actions are accessible via keyboard shortcuts displayed in the nav bar. The | File | Purpose | | --- | --- | | `crates/openshell-tui/Cargo.toml` | Crate manifest — dependencies on `openshell-core`, `openshell-bootstrap`, `ratatui`, `crossterm`, `tonic`, `tokio` | -| `crates/openshell-tui/src/lib.rs` | Entry point. Event loop, gRPC calls (`refresh_data`, `refresh_providers`, `refresh_global_settings`, `refresh_workspaces`, `refresh_sandboxes`, `spawn_log_stream`, `handle_sandbox_delete`), gateway switching, mTLS channel building, provider CRUD spawners, settings CRUD spawners, draft approval spawners | +| `crates/openshell-tui/src/lib.rs` | Entry point. Event loop, background collection refresh (`spawn_list_refresh`), gRPC calls (`refresh_global_settings`, `spawn_log_stream`, `handle_sandbox_delete`), gateway switching, mTLS channel building, provider CRUD spawners, settings CRUD spawners, draft approval spawners | | `crates/openshell-tui/src/app.rs` | `App` state struct, `Screen`/`Focus`/`InputMode`/`LogSourceFilter`/`MiddlePaneTab`/`SandboxPolicyTab` enums, `LogLine`/`GatewayEntry`/`GlobalSettingEntry`/`SandboxSettingEntry`/`ProviderListEntry`/`ProviderDetailView` structs, create sandbox/provider form state, all key handling logic | -| `crates/openshell-tui/src/event.rs` | `Event` enum (`Key`, `Mouse`, `Tick`, `Redraw`, `Resize`, `LogLines`, `CreateResult`, `ProviderCreateResult`, `ProviderDetailFetched`, `ProviderUpdateResult`, `ProviderDeleteResult`, `DraftActionResult`, `GlobalSettingsFetched`, `GlobalSettingSetResult`, `GlobalSettingDeleteResult`, `SandboxSettingSetResult`, `SandboxSettingDeleteResult`, `ForwardWarnings`), `EventHandler` with mpsc channels and crossterm polling | +| `crates/openshell-tui/src/event.rs` | `Event` enum (`Key`, `Mouse`, `Tick`, `Redraw`, `Resize`, `LogLines`, `ListRefreshCompleted`, `CreateResult`, `ProviderCreateResult`, `ProviderDetailFetched`, `ProviderUpdateResult`, `ProviderDeleteResult`, `DraftActionResult`, `GlobalSettingsFetched`, `GlobalSettingSetResult`, `GlobalSettingDeleteResult`, `SandboxSettingSetResult`, `SandboxSettingDeleteResult`, `ForwardWarnings`), `EventHandler` with mpsc channels and crossterm polling | | `crates/openshell-tui/src/theme.rs` | `colors` module (NVIDIA_GREEN, EVERGLADE, BG, FG) and `styles` module (all `Style` constants) | | `crates/openshell-tui/src/clipboard.rs` | Clipboard copy support for log lines | | `crates/openshell-tui/src/ui/mod.rs` | Top-level `draw()` dispatcher, `draw_title_bar` (with workspace display), `draw_nav_bar`, `draw_command_bar`, screen routing, shared setting-edit overlay, modal helpers | @@ -502,12 +504,15 @@ use openshell_core::proto::{ `Some(all_workspaces_selector())`; do not use that marker on other requests. - `GetSandboxLogsRequest` fields: `sandbox_id`, `lines` (u32), `since_ms` (i64), `sources` (Vec), `min_level` (String), `workspace_scope`. -- `ListSandboxesRequest` fields: `limit` (u32), `offset` (u32), +- `ListSandboxesRequest` fields: `page_size` (i32), `page_token` (String), `label_selector` (String), `workspace_scope`. -- `ListProvidersRequest` fields: `limit` (u32), `offset` (u32), +- `ListProvidersRequest` fields: `page_size` (i32), `page_token` (String), `workspace_scope`. -- `ListWorkspacesRequest` fields: `limit` (u32), `offset` (u32), +- `ListWorkspacesRequest` fields: `page_size` (i32), `page_token` (String), `label_selector` (String). +- Paginated list responses return `next_page_token`. Continue with the same + request parameters and that token until it is empty; changing filters or + scope invalidates the token. - `UpdateConfigRequest` fields include `name` (String, sandbox name or empty for global), `setting_key`, `setting_value`, `delete_setting` (bool), `global` (bool), and `workspace_scope`. Sandbox-scoped updates require a named selector; @@ -543,7 +548,7 @@ The connect timeout for gateway switching is 10 seconds with HTTP/2 keepalive at 4. On success: - `app.client` is replaced with a new intercepted client - `reset_sandbox_state()` clears all sandbox/log/draft/policy data - - `refresh_data()` runs the full capability refresh sequence: `refresh_health` → `refresh_global_settings` → `refresh_workspaces` → `refresh_providers` → `refresh_sandboxes` + - health and global settings are refreshed, then `spawn_list_refresh()` starts the cancellable workspace/provider/sandbox refresh task 5. On failure: `status_text` shows the error ### Initial startup lifecycle @@ -551,13 +556,13 @@ The connect timeout for gateway switching is 10 seconds with HTTP/2 keepalive at On launch, before the event loop starts: 1. `refresh_gateway_list()` — discover gateways from disk -2. `refresh_data()` — full refresh (health, global settings, workspaces, providers, sandboxes) +2. Refresh health and global settings, then start `spawn_list_refresh()` for workspaces, providers, and sandboxes ### Workspace switching lifecycle 1. User presses `[w]` on the providers or sandboxes panel → `cycle_workspace()` advances through discovered workspace names, then "all" 2. `pending_workspace_refresh = true` is set, cursor indices are reset -3. Event loop calls `refresh_providers()` and `refresh_sandboxes()` with the new workspace scope +3. Event loop cancels any in-flight collection refresh and starts `spawn_list_refresh()` with the new workspace scope ### Settings CRUD lifecycle (global and sandbox) diff --git a/Cargo.lock b/Cargo.lock index 6a7436cb90..806e967182 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4668,6 +4668,7 @@ version = "0.0.0" dependencies = [ "base64", "crossterm 0.28.1", + "futures", "indexmap", "miette", "openshell-bootstrap", diff --git a/architecture/gateway.md b/architecture/gateway.md index 59b2bd52ed..588b9f56a4 100644 --- a/architecture/gateway.md +++ b/architecture/gateway.md @@ -329,7 +329,7 @@ Compute-driver, credential-driver, gateway-interceptor, and supervisor-middleware services are compiled contracts for internal extension boundaries, not public gateway RPCs. The current public inventory has 74 methods, 278 messages, and 12 enums -(`c95ae90962c10fb28747db2b645adf4044562a3d208d84dfe4699d677e4364ee`). +(`0f14943574349d02bdc61076c8c5a59a98b627325564ef1a6d21d7941825dc46`). Storage-only messages live in the private, versioned `openshell.storage.v1` package under `crates/openshell-server/proto`. The server @@ -529,11 +529,33 @@ modes: `UpdateProvider`, `UpdateProviderProfiles`, and `UpdateConfig` (policy backfill and sandbox annotation updates). -**Lists.** The `list_messages` and `list_messages_with_selector` helpers decode -protobuf payloads from list results and hydrate `resource_version` from the -authoritative database column into each decoded message, mirroring the -`get_message` pattern. This ensures list responses carry correct versions -without requiring callers to manually hydrate each record. +**Lists.** Public list RPCs follow AIP-158: requests carry direct `page_size` +and `page_token` fields, and responses carry `next_page_token`. The gateway +clamps page sizes to 1,000 and returns opaque base64url continuation tokens. +Tokens bind the RPC and every request parameter except `page_size`, contain no +authorization grant, and use immutable keyset cursors rather than database +offsets. Each page repeats normal authentication and authorization. Pagination +is weakly consistent under concurrent writes and deletes; it does not provide a +historical snapshot. + +The token wire format is a private shared protobuf used only by the gateway. +Public request and response messages repeat the standard AIP fields directly +instead of wrapping them in a shared pagination message. + +The CLI returns paginated JSON and YAML as response-shaped envelopes containing +the resource collection and `next_page_token`; table output reports a non-empty +token on stderr. The TUI traverses complete workspace, provider, profile, and +sandbox collections in one cancellable background refresh task, never overlaps +periodic list refreshes, and discards results after a gateway or workspace +change. + +Persistence distinguishes one-page operations from exhaustive scans. +`list_object_page` and `list_message_page` return one keyset page and its next +cursor. `collect_records` and `collect_messages` exhaust those pages, fail on +database or protobuf decode errors, and hydrate `resource_version` from the +authoritative database column. Internal callers that require every matching +record use the exhaustive helpers; bounded lookups continue to use page-level +methods. **Deletes.** Delete operations are not yet CAS-protected -- the delete request protos do not carry `expected_resource_version`. A `delete_if` primitive exists diff --git a/crates/openshell-cli/src/commands/provider.rs b/crates/openshell-cli/src/commands/provider.rs index e966eb690e..b85fef0013 100644 --- a/crates/openshell-cli/src/commands/provider.rs +++ b/crates/openshell-cli/src/commands/provider.rs @@ -298,18 +298,18 @@ pub async fn ensure_required_providers( let mut known_names: HashSet = HashSet::new(); let mut type_to_name: HashMap = HashMap::new(); { - let mut offset = 0_u32; - let limit = 100_u32; + let mut page_token = String::new(); loop { let response = client .list_providers(ListProvidersRequest { - limit, - offset, + page_size: 100, + page_token, workspace_scope: Some(openshell_core::proto::workspace_selector(workspace)), }) .await .into_diagnostic()?; - let providers = response.into_inner().providers; + let response = response.into_inner(); + let providers = response.providers; for provider in &providers { known_names.insert(provider.object_name().to_string()); if !provider.r#type.is_empty() { @@ -319,10 +319,10 @@ pub async fn ensure_required_providers( .or_insert_with(|| provider.object_name().to_string()); } } - if providers.len() < limit as usize { + if response.next_page_token.is_empty() { break; } - offset = offset.saturating_add(limit); + page_token = response.next_page_token; } } @@ -1325,8 +1325,8 @@ fn provider_credential_keys(provider: &Provider) -> Vec { #[allow(clippy::too_many_arguments)] pub async fn provider_list( server: &str, - limit: u32, - offset: u32, + page_size: i32, + page_token: &str, names_only: bool, output: &str, workspace: &str, @@ -1336,8 +1336,8 @@ pub async fn provider_list( let mut client = grpc_client(server, tls).await?; let response = client .list_providers(ListProvidersRequest { - limit, - offset, + page_size, + page_token: page_token.to_string(), workspace_scope: Some(if all_workspaces { openshell_core::proto::all_workspaces_selector() } else { @@ -1346,12 +1346,21 @@ pub async fn provider_list( }) .await .into_diagnostic()?; - let providers = response.into_inner().providers; + let response = response.into_inner(); + let next_page_token = response.next_page_token; + let providers = response.providers; // Handle structured output formats (json, yaml) - if crate::output::print_output_collection(output, &providers, provider_to_json)? { + if crate::output::print_paginated_output_collection( + output, + "providers", + &providers, + &next_page_token, + provider_to_json, + )? { return Ok(()); } + crate::output::print_next_page_token(&next_page_token); if providers.is_empty() { if !names_only { @@ -1444,15 +1453,24 @@ pub async fn provider_list_profiles( tls: &TlsOptions, ) -> Result<()> { let mut client = grpc_client(server, tls).await?; - let response = client - .list_provider_profiles(ListProviderProfilesRequest { - limit: 100, - offset: 0, - workspace: workspace.to_string(), - }) - .await - .into_diagnostic()?; - let mut profiles = response.into_inner().profiles; + let mut page_token = String::new(); + let mut profiles = Vec::new(); + loop { + let response = client + .list_provider_profiles(ListProviderProfilesRequest { + page_size: 100, + page_token, + workspace: workspace.to_string(), + }) + .await + .into_diagnostic()? + .into_inner(); + profiles.extend(response.profiles); + if response.next_page_token.is_empty() { + break; + } + page_token = response.next_page_token; + } profiles.sort_by(|left, right| { left.category .cmp(&right.category) diff --git a/crates/openshell-cli/src/completers.rs b/crates/openshell-cli/src/completers.rs index 0778babbf1..c5fec20634 100644 --- a/crates/openshell-cli/src/completers.rs +++ b/crates/openshell-cli/src/completers.rs @@ -36,8 +36,8 @@ pub fn complete_sandbox_names(_prefix: &OsStr) -> Vec { let mut client = completion_grpc_client(&endpoint, &gateway_name).await?; let response = client .list_sandboxes(ListSandboxesRequest { - limit: 200, - offset: 0, + page_size: 200, + page_token: String::new(), label_selector: String::new(), workspace_scope: Some(openshell_core::proto::workspace_selector( workspace_from_args(), @@ -63,8 +63,8 @@ pub fn complete_provider_names(_prefix: &OsStr) -> Vec { let mut client = completion_grpc_client(&endpoint, &gateway_name).await?; let response = client .list_providers(ListProvidersRequest { - limit: 200, - offset: 0, + page_size: 200, + page_token: String::new(), workspace_scope: Some(openshell_core::proto::workspace_selector( workspace_from_args(), )), @@ -89,8 +89,8 @@ pub fn complete_workspace_names(_prefix: &OsStr) -> Vec { let mut client = completion_grpc_client(&endpoint, &gateway_name).await?; let response = client .list_workspaces(ListWorkspacesRequest { - limit: 200, - offset: 0, + page_size: 200, + page_token: String::new(), label_selector: String::new(), }) .await diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs index db18795b68..bffd137c09 100644 --- a/crates/openshell-cli/src/main.rs +++ b/crates/openshell-cli/src/main.rs @@ -877,13 +877,13 @@ enum ProviderCommands { /// List providers. #[command(help_template = LEAF_HELP_TEMPLATE, next_help_heading = "FLAGS")] List { - /// Maximum number of providers to return. + /// Maximum number of providers to return in this page. #[arg(long, default_value_t = 100)] - limit: u32, + page_size: i32, - /// Offset into the provider list. - #[arg(long, default_value_t = 0)] - offset: u32, + /// Opaque continuation token from a previous page. + #[arg(long, default_value = "")] + page_token: String, /// Print only provider names, one per line. #[arg(long, conflicts_with = "output")] @@ -1440,13 +1440,13 @@ enum SandboxCommands { /// List sandboxes. #[command(help_template = LEAF_HELP_TEMPLATE, next_help_heading = "FLAGS")] List { - /// Maximum number of sandboxes to return. + /// Maximum number of sandboxes to return in this page. #[arg(long, default_value_t = 100)] - limit: u32, + page_size: i32, - /// Offset into the sandbox list. - #[arg(long, default_value_t = 0)] - offset: u32, + /// Opaque continuation token from a previous page. + #[arg(long, default_value = "")] + page_token: String, /// Print only sandbox ids (one per line). #[arg(long, conflicts_with_all = ["names", "output"])] @@ -1744,13 +1744,13 @@ enum SandboxTemplateCommands { /// List sandbox workload templates. #[command(help_template = LEAF_HELP_TEMPLATE, next_help_heading = "FLAGS")] List { - /// Maximum number of templates to return. + /// Maximum number of templates to return in this page. #[arg(long, default_value_t = 100)] - limit: u32, + page_size: i32, - /// Offset into the template list. - #[arg(long, default_value_t = 0)] - offset: u32, + /// Opaque continuation token from a previous page. + #[arg(long, default_value = "")] + page_token: String, /// Filter templates by labels, e.g. env=prod,team=runtime. #[arg(long)] @@ -1958,9 +1958,13 @@ enum PolicyCommands { #[arg(add = ArgValueCompleter::new(completers::complete_sandbox_names))] name: Option, - /// Maximum number of revisions to return. + /// Maximum number of revisions to return in this page. #[arg(long, default_value_t = 20)] - limit: u32, + page_size: i32, + + /// Opaque continuation token from a previous page. + #[arg(long, default_value = "")] + page_token: String, /// List global policy revisions. #[arg(long)] @@ -2128,13 +2132,13 @@ enum ServiceCommands { #[arg(add = ArgValueCompleter::new(completers::complete_sandbox_names))] sandbox: Option, - /// Maximum number of endpoints to return. + /// Maximum number of endpoints to return in this page. #[arg(long, default_value_t = 100)] - limit: u32, + page_size: i32, - /// Number of endpoints to skip. - #[arg(long, default_value_t = 0)] - offset: u32, + /// Opaque continuation token from a previous page. + #[arg(long, default_value = "")] + page_token: String, /// List services across all workspaces (overrides --workspace). #[arg(long)] @@ -2193,13 +2197,13 @@ enum WorkspaceCommands { /// List workspaces. #[command(help_template = LEAF_HELP_TEMPLATE, next_help_heading = "FLAGS")] List { - /// Maximum number of workspaces to return. + /// Maximum number of workspaces to return in this page. #[arg(long, default_value_t = 100)] - limit: u32, + page_size: i32, - /// Offset into the workspace list. - #[arg(long, default_value_t = 0)] - offset: u32, + /// Opaque continuation token from a previous page. + #[arg(long, default_value = "")] + page_token: String, /// Filter by label selector (e.g. "env=staging"). #[arg(long)] @@ -2260,13 +2264,13 @@ enum WorkspaceMemberCommands { #[arg(long, add = ArgValueCompleter::new(completers::complete_workspace_names))] workspace: String, - /// Maximum number of members to return. + /// Maximum number of members to return in this page. #[arg(long, default_value_t = 100)] - limit: u32, + page_size: i32, - /// Offset into the member list. - #[arg(long, default_value_t = 0)] - offset: u32, + /// Opaque continuation token from a previous page. + #[arg(long, default_value = "")] + page_token: String, /// Output format. #[arg(short = 'o', long = "output", value_enum, default_value_t = OutputFormat::Table)] @@ -2665,16 +2669,16 @@ async fn run_async() -> Result<()> { } ServiceCommands::List { sandbox, - limit, - offset, + page_size, + page_token, all_workspaces, output, } => { run::service_list( &ctx.endpoint, sandbox.as_deref(), - limit, - offset, + page_size, + &page_token, &cli.workspace, all_workspaces, output.as_str(), @@ -2839,14 +2843,16 @@ async fn run_async() -> Result<()> { } PolicyCommands::List { name, - limit, + page_size, + page_token, global, output, } => { if global { run::sandbox_policy_list_global( &ctx.endpoint, - limit, + page_size, + &page_token, output.as_str(), &cli.workspace, &tls, @@ -2857,7 +2863,8 @@ async fn run_async() -> Result<()> { run::sandbox_policy_list( &ctx.endpoint, &name, - limit, + page_size, + &page_token, output.as_str(), &cli.workspace, &tls, @@ -3232,8 +3239,8 @@ async fn run_async() -> Result<()> { .await?; } SandboxCommands::List { - limit, - offset, + page_size, + page_token, ids, names, selector, @@ -3242,8 +3249,8 @@ async fn run_async() -> Result<()> { } => { run::sandbox_list( endpoint, - limit, - offset, + page_size, + &page_token, ids, names, selector.as_deref(), @@ -3423,8 +3430,8 @@ async fn run_async() -> Result<()> { .await?; } SandboxTemplateCommands::List { - limit, - offset, + page_size, + page_token, label_selector, names, output, @@ -3432,8 +3439,8 @@ async fn run_async() -> Result<()> { } => { run::sandbox_template_list( endpoint, - limit, - offset, + page_size, + &page_token, label_selector.as_deref(), names, output.as_str(), @@ -3477,15 +3484,15 @@ async fn run_async() -> Result<()> { run::workspace_get(endpoint, &name, &tls).await?; } WorkspaceCommands::List { - limit, - offset, + page_size, + page_token, label_selector, output, } => { run::workspace_list( endpoint, - limit, - offset, + page_size, + &page_token, label_selector.as_deref().unwrap_or(""), output.as_str(), &tls, @@ -3509,15 +3516,15 @@ async fn run_async() -> Result<()> { } WorkspaceMemberCommands::List { workspace, - limit, - offset, + page_size, + page_token, output, } => { run::workspace_member_list( endpoint, &workspace, - limit, - offset, + page_size, + &page_token, output.as_str(), &tls, ) @@ -3642,16 +3649,16 @@ async fn run_async() -> Result<()> { run::provider_get(endpoint, &name, &cli.workspace, &tls).await?; } ProviderCommands::List { - limit, - offset, + page_size, + page_token, names, output, all_workspaces, } => { run::provider_list( endpoint, - limit, - offset, + page_size, + &page_token, names, output.as_str(), &cli.workspace, @@ -5742,10 +5749,10 @@ mod tests { "--all-workspaces", "--label-selector", "team=runtime", - "--limit", + "--page-size", "25", - "--offset", - "5", + "--page-token", + "next-template-page", ]) .expect("sandbox template list should parse"); @@ -5753,8 +5760,8 @@ mod tests { Some(Commands::Sandbox { command: Some(SandboxCommands::Template(SandboxTemplateCommands::List { - limit, - offset, + page_size, + page_token, label_selector, names, all_workspaces, @@ -5762,8 +5769,8 @@ mod tests { })), .. }) => { - assert_eq!(limit, 25); - assert_eq!(offset, 5); + assert_eq!(page_size, 25); + assert_eq!(page_token, "next-template-page"); assert_eq!(label_selector.as_deref(), Some("team=runtime")); assert!(names); assert!(all_workspaces); @@ -6014,10 +6021,10 @@ mod tests { "service", "list", "my-sandbox", - "--limit", + "--page-size", "10", - "--offset", - "2", + "--page-token", + "next-service-page", ]) .expect("service list should parse optional sandbox and paging"); @@ -6026,14 +6033,14 @@ mod tests { command: Some(ServiceCommands::List { sandbox, - limit, - offset, + page_size, + page_token, .. }), }) => { assert_eq!(sandbox.as_deref(), Some("my-sandbox")); - assert_eq!(limit, 10); - assert_eq!(offset, 2); + assert_eq!(page_size, 10); + assert_eq!(page_token, "next-service-page"); } other => panic!("expected service list command, got: {other:?}"), } @@ -6046,14 +6053,14 @@ mod tests { command: Some(ServiceCommands::List { sandbox, - limit, - offset, + page_size, + page_token, .. }), }) => { assert_eq!(sandbox, None); - assert_eq!(limit, 100); - assert_eq!(offset, 0); + assert_eq!(page_size, 100); + assert!(page_token.is_empty()); } other => panic!("expected service list command, got: {other:?}"), } diff --git a/crates/openshell-cli/src/output.rs b/crates/openshell-cli/src/output.rs index 55c548787e..fee304870a 100644 --- a/crates/openshell-cli/src/output.rs +++ b/crates/openshell-cli/src/output.rs @@ -34,6 +34,74 @@ use miette::{IntoDiagnostic, Result}; use std::io::Write; +/// Report a continuation token for table output. +pub fn print_next_page_token(next_page_token: &str) { + if !next_page_token.is_empty() { + eprintln!("Next page token: {next_page_token}"); + } +} + +fn paginated_collection_value( + collection_name: &str, + items: &[T], + next_page_token: &str, + to_json: F, +) -> serde_json::Value +where + F: Fn(&T) -> serde_json::Value, +{ + let mut envelope = serde_json::Map::new(); + envelope.insert( + collection_name.to_string(), + serde_json::Value::Array(items.iter().map(to_json).collect()), + ); + envelope.insert( + "next_page_token".to_string(), + serde_json::Value::String(next_page_token.to_string()), + ); + serde_json::Value::Object(envelope) +} + +/// Print a single page as a structured response envelope. +/// +/// JSON and YAML output include both the named collection and +/// `next_page_token`, so callers can continue without scraping stderr. Table +/// output is not handled; callers should render the table and may report the +/// token with [`print_next_page_token`]. +pub fn print_paginated_output_collection( + format: impl AsRef, + collection_name: &str, + items: &[T], + next_page_token: &str, + to_json: F, +) -> Result +where + F: Fn(&T) -> serde_json::Value, +{ + match format.as_ref() { + "json" => { + let envelope = + paginated_collection_value(collection_name, items, next_page_token, to_json); + println!( + "{}", + serde_json::to_string_pretty(&envelope).into_diagnostic()? + ); + Ok(true) + } + "yaml" => { + let envelope = + paginated_collection_value(collection_name, items, next_page_token, to_json); + print!("{}", serde_yml::to_string(&envelope).into_diagnostic()?); + Ok(true) + } + "table" => Ok(false), + _ => Err(miette::miette!( + "unsupported output format: {}", + format.as_ref() + )), + } +} + /// Print collection output in specified format (json/yaml/table). /// /// # Returns @@ -210,6 +278,50 @@ where } } +/// Writer variant of [`print_paginated_output_collection`]. +pub fn print_paginated_output_collection_to_writer( + format: impl AsRef, + writer: &mut W, + collection_name: &str, + items: &[T], + next_page_token: &str, + to_json: F, +) -> Result +where + W: Write, + F: Fn(&T) -> serde_json::Value, +{ + match format.as_ref() { + "json" => { + let envelope = + paginated_collection_value(collection_name, items, next_page_token, to_json); + writeln!( + writer, + "{}", + serde_json::to_string_pretty(&envelope).into_diagnostic()? + ) + .into_diagnostic()?; + Ok(true) + } + "yaml" => { + let envelope = + paginated_collection_value(collection_name, items, next_page_token, to_json); + write!( + writer, + "{}", + serde_yml::to_string(&envelope).into_diagnostic()? + ) + .into_diagnostic()?; + Ok(true) + } + "table" => Ok(false), + _ => Err(miette::miette!( + "unsupported output format: {}", + format.as_ref() + )), + } +} + /// Print single item output to a custom writer in specified format (json/yaml/table). /// /// Writer variant for commands that need custom output destinations. @@ -472,6 +584,73 @@ mod tests { assert!(output.contains("name: test")); } + #[test] + fn test_print_paginated_output_collection_to_writer_json() { + let items = vec![TestItem { + id: 1, + name: "test".to_string(), + }]; + let mut buffer = Vec::new(); + + let handled = print_paginated_output_collection_to_writer( + "json", + &mut buffer, + "items", + &items, + "next-token", + test_item_to_json, + ) + .unwrap(); + + assert!(handled); + let output: serde_json::Value = serde_json::from_slice(&buffer).unwrap(); + assert_eq!(output["items"][0]["id"], 1); + assert_eq!(output["next_page_token"], "next-token"); + } + + #[test] + fn test_print_paginated_output_collection_to_writer_yaml() { + let items = vec![TestItem { + id: 1, + name: "test".to_string(), + }]; + let mut buffer = Vec::new(); + + let handled = print_paginated_output_collection_to_writer( + "yaml", + &mut buffer, + "items", + &items, + "", + test_item_to_json, + ) + .unwrap(); + + assert!(handled); + let output: serde_json::Value = serde_yml::from_slice(&buffer).unwrap(); + assert_eq!(output["items"][0]["name"], "test"); + assert_eq!(output["next_page_token"], ""); + } + + #[test] + fn test_print_paginated_output_collection_table_is_not_handled() { + let items = Vec::::new(); + let mut buffer = Vec::new(); + + let handled = print_paginated_output_collection_to_writer( + "table", + &mut buffer, + "items", + &items, + "next-token", + test_item_to_json, + ) + .unwrap(); + + assert!(!handled); + assert!(buffer.is_empty()); + } + #[test] fn test_print_output_single_to_writer_json() { let item = TestItem { diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 3421f0f871..a89e3c30b3 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -2232,8 +2232,8 @@ async fn sandbox_exec_interactive_grpc( #[allow(clippy::too_many_arguments)] pub async fn sandbox_list( server: &str, - limit: u32, - offset: u32, + page_size: i32, + page_token: &str, ids_only: bool, names_only: bool, label_selector: Option<&str>, @@ -2246,8 +2246,8 @@ pub async fn sandbox_list( let response = client .list_sandboxes(ListSandboxesRequest { - limit, - offset, + page_size, + page_token: page_token.to_string(), label_selector: label_selector.unwrap_or("").to_string(), workspace_scope: Some(if all_workspaces { openshell_core::proto::all_workspaces_selector() @@ -2258,11 +2258,20 @@ pub async fn sandbox_list( .await .into_diagnostic()?; - let sandboxes = response.into_inner().sandboxes; + let response = response.into_inner(); + let next_page_token = response.next_page_token; + let sandboxes = response.sandboxes; - if crate::output::print_output_collection(output, &sandboxes, sandbox_to_json)? { + if crate::output::print_paginated_output_collection( + output, + "sandboxes", + &sandboxes, + &next_page_token, + sandbox_to_json, + )? { return Ok(()); } + crate::output::print_next_page_token(&next_page_token); if sandboxes.is_empty() { if !ids_only && !names_only { @@ -2587,8 +2596,8 @@ pub async fn sandbox_template_get( #[allow(clippy::too_many_arguments)] pub async fn sandbox_template_list( server: &str, - limit: u32, - offset: u32, + page_size: i32, + page_token: &str, label_selector: Option<&str>, names_only: bool, output: &str, @@ -2599,8 +2608,8 @@ pub async fn sandbox_template_list( let mut client = grpc_client(server, tls).await?; let response = client .list_sandbox_templates(ListSandboxTemplatesRequest { - limit, - offset, + page_size, + page_token: page_token.to_string(), workspace_scope: Some(if all_workspaces { openshell_core::proto::all_workspaces_selector() } else { @@ -2610,11 +2619,20 @@ pub async fn sandbox_template_list( }) .await .into_diagnostic()?; - let templates = response.into_inner().templates; + let response = response.into_inner(); + let next_page_token = response.next_page_token; + let templates = response.templates; - if crate::output::print_output_collection(output, &templates, sandbox_template_to_json)? { + if crate::output::print_paginated_output_collection( + output, + "templates", + &templates, + &next_page_token, + sandbox_template_to_json, + )? { return Ok(()); } + crate::output::print_next_page_token(&next_page_token); if templates.is_empty() { if !names_only { @@ -3028,17 +3046,25 @@ pub async fn sandbox_delete( let mut client = grpc_client(server, tls).await?; let names_to_delete: Vec = if all { - // Fetch all sandboxes (use a large page size). - let response = client - .list_sandboxes(ListSandboxesRequest { - limit: 1000, - offset: 0, - label_selector: String::new(), - workspace_scope: Some(openshell_core::proto::workspace_selector(workspace)), - }) - .await - .into_diagnostic()?; - let sandboxes = response.into_inner().sandboxes; + let mut page_token = String::new(); + let mut sandboxes = Vec::new(); + loop { + let response = client + .list_sandboxes(ListSandboxesRequest { + page_size: 1000, + page_token, + label_selector: String::new(), + workspace_scope: Some(openshell_core::proto::workspace_selector(workspace)), + }) + .await + .into_diagnostic()? + .into_inner(); + sandboxes.extend(response.sandboxes); + if response.next_page_token.is_empty() { + break; + } + page_token = response.next_page_token; + } if sandboxes.is_empty() { println!("No sandboxes to delete."); return Ok(()); @@ -3279,8 +3305,8 @@ fn service_expose_status_error(status: Status) -> miette::Report { pub async fn service_list( server: &str, sandbox: Option<&str>, - limit: u32, - offset: u32, + page_size: i32, + page_token: &str, workspace: &str, all_workspaces: bool, output: &str, @@ -3290,8 +3316,8 @@ pub async fn service_list( let response = client .list_services(ListServicesRequest { sandbox: sandbox.unwrap_or_default().to_string(), - limit, - offset, + page_size, + page_token: page_token.to_string(), workspace_scope: Some(if all_workspaces { openshell_core::proto::all_workspaces_selector() } else { @@ -3302,14 +3328,22 @@ pub async fn service_list( .map_err(|status| service_status_error("list services", "sandbox:read", status))? .into_inner(); + let next_page_token = response.next_page_token.clone(); let services = response .services .iter() .filter_map(|response| service_endpoint_to_json(response, server)) .collect::>(); - if crate::output::print_output_collection(output, &services, Clone::clone)? { + if crate::output::print_paginated_output_collection( + output, + "services", + &services, + &next_page_token, + Clone::clone, + )? { return Ok(()); } + crate::output::print_next_page_token(&next_page_token); if response.services.is_empty() { if let Some(sandbox) = sandbox { @@ -3646,8 +3680,8 @@ pub async fn workspace_get(server: &str, name: &str, tls: &TlsOptions) -> Result pub async fn workspace_list( server: &str, - limit: u32, - offset: u32, + page_size: i32, + page_token: &str, label_selector: &str, output: &str, tls: &TlsOptions, @@ -3657,17 +3691,26 @@ pub async fn workspace_list( let mut client = grpc_client(server, tls).await?; let response = client .list_workspaces(ListWorkspacesRequest { - limit, - offset, + page_size, + page_token: page_token.to_string(), label_selector: label_selector.to_string(), }) .await .into_diagnostic()?; - let workspaces = response.into_inner().workspaces; + let response = response.into_inner(); + let next_page_token = response.next_page_token; + let workspaces = response.workspaces; - if crate::output::print_output_collection(output, &workspaces, workspace_to_json)? { + if crate::output::print_paginated_output_collection( + output, + "workspaces", + &workspaces, + &next_page_token, + workspace_to_json, + )? { return Ok(()); } + crate::output::print_next_page_token(&next_page_token); if workspaces.is_empty() { println!("No workspaces found."); @@ -3817,8 +3860,8 @@ pub async fn workspace_member_remove( pub async fn workspace_member_list( server: &str, workspace: &str, - limit: u32, - offset: u32, + page_size: i32, + page_token: &str, output: &str, tls: &TlsOptions, ) -> Result<()> { @@ -3828,16 +3871,25 @@ pub async fn workspace_member_list( let response = client .list_workspace_members(ListWorkspaceMembersRequest { workspace: workspace.to_string(), - limit, - offset, + page_size, + page_token: page_token.to_string(), }) .await .into_diagnostic()?; - let members = response.into_inner().members; + let response = response.into_inner(); + let next_page_token = response.next_page_token; + let members = response.members; - if crate::output::print_output_collection(output, &members, workspace_member_to_json)? { + if crate::output::print_paginated_output_collection( + output, + "members", + &members, + &next_page_token, + workspace_member_to_json, + )? { return Ok(()); } + crate::output::print_next_page_token(&next_page_token); if members.is_empty() { println!("No members found in workspace {workspace}."); @@ -5163,7 +5215,8 @@ fn policy_for_view(policy: &SandboxPolicy, view: PolicyGetView) -> Cow<'_, Sandb pub async fn sandbox_policy_list( server: &str, name: &str, - limit: u32, + page_size: i32, + page_token: &str, output: &str, workspace: &str, tls: &TlsOptions, @@ -5173,19 +5226,28 @@ pub async fn sandbox_policy_list( let resp = client .list_sandbox_policies(ListSandboxPoliciesRequest { name: name.to_string(), - limit, - offset: 0, + page_size, + page_token: page_token.to_string(), global: false, workspace_scope: Some(openshell_core::proto::workspace_selector(workspace)), }) .await .into_diagnostic()?; - let revisions = resp.into_inner().revisions; + let resp = resp.into_inner(); + let next_page_token = resp.next_page_token; + let revisions = resp.revisions; let structured = policy_revision_list_json("sandbox", Some(name), &revisions)?; - if crate::output::print_output_collection(output, &structured, Clone::clone)? { + if crate::output::print_paginated_output_collection( + output, + "revisions", + &structured, + &next_page_token, + Clone::clone, + )? { return Ok(()); } + crate::output::print_next_page_token(&next_page_token); if revisions.is_empty() { eprintln!("No policy history found for sandbox '{name}'"); @@ -5198,7 +5260,8 @@ pub async fn sandbox_policy_list( pub async fn sandbox_policy_list_global( server: &str, - limit: u32, + page_size: i32, + page_token: &str, output: &str, _workspace: &str, tls: &TlsOptions, @@ -5208,19 +5271,28 @@ pub async fn sandbox_policy_list_global( let resp = client .list_sandbox_policies(ListSandboxPoliciesRequest { name: String::new(), - limit, - offset: 0, + page_size, + page_token: page_token.to_string(), global: true, workspace_scope: None, }) .await .into_diagnostic()?; - let revisions = resp.into_inner().revisions; + let resp = resp.into_inner(); + let next_page_token = resp.next_page_token; + let revisions = resp.revisions; let structured = policy_revision_list_json("global", None, &revisions)?; - if crate::output::print_output_collection(output, &structured, Clone::clone)? { + if crate::output::print_paginated_output_collection( + output, + "revisions", + &structured, + &next_page_token, + Clone::clone, + )? { return Ok(()); } + crate::output::print_next_page_token(&next_page_token); if revisions.is_empty() { eprintln!("No global policy history found"); diff --git a/crates/openshell-cli/tests/ensure_providers_integration.rs b/crates/openshell-cli/tests/ensure_providers_integration.rs index 7545786a12..7b17ee7a35 100644 --- a/crates/openshell-cli/tests/ensure_providers_integration.rs +++ b/crates/openshell-cli/tests/ensure_providers_integration.rs @@ -314,7 +314,10 @@ impl OpenShell for TestOpenShell { .values() .cloned() .collect::>(); - Ok(Response::new(ListProvidersResponse { providers })) + Ok(Response::new(ListProvidersResponse { + providers, + next_page_token: String::new(), + })) } async fn list_provider_profiles( @@ -326,7 +329,10 @@ impl OpenShell for TestOpenShell { .map(openshell_providers::ProviderTypeProfile::to_proto) .collect(); Ok(Response::new( - openshell_core::proto::ListProviderProfilesResponse { profiles }, + openshell_core::proto::ListProviderProfilesResponse { + profiles, + next_page_token: String::new(), + }, )) } diff --git a/crates/openshell-cli/tests/provider_commands_integration.rs b/crates/openshell-cli/tests/provider_commands_integration.rs index 912835faa3..5b8ae54333 100644 --- a/crates/openshell-cli/tests/provider_commands_integration.rs +++ b/crates/openshell-cli/tests/provider_commands_integration.rs @@ -480,7 +480,10 @@ impl OpenShell for TestOpenShell { .values() .cloned() .collect::>(); - Ok(Response::new(ListProvidersResponse { providers })) + Ok(Response::new(ListProvidersResponse { + providers, + next_page_token: String::new(), + })) } async fn list_provider_profiles( @@ -493,7 +496,10 @@ impl OpenShell for TestOpenShell { .collect::>(); profiles.extend(self.state.profiles.lock().await.values().cloned()); Ok(Response::new( - openshell_core::proto::ListProviderProfilesResponse { profiles }, + openshell_core::proto::ListProviderProfilesResponse { + profiles, + next_page_token: String::new(), + }, )) } @@ -1357,7 +1363,7 @@ async fn provider_cli_run_functions_support_full_crud_flow() { run::provider_list( &ts.endpoint, 100, - 0, + "", false, "table", "default", @@ -1428,7 +1434,7 @@ async fn provider_list_json_output() { run::provider_list( &ts.endpoint, 100, - 0, + "", false, "json", "default", @@ -1471,7 +1477,7 @@ async fn provider_list_yaml_output() { run::provider_list( &ts.endpoint, 100, - 0, + "", false, "yaml", "default", @@ -1499,7 +1505,7 @@ async fn provider_list_json_empty() { run::provider_list( &ts.endpoint, 100, - 0, + "", false, "json", "default", diff --git a/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs b/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs index 6ed9a74339..5c24fc6695 100644 --- a/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs +++ b/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs @@ -285,6 +285,7 @@ impl OpenShell for TestOpenShell { .push(request.into_inner()); Ok(Response::new(ListSandboxTemplatesResponse { templates: Vec::new(), + next_page_token: String::new(), })) } @@ -461,6 +462,7 @@ impl OpenShell for TestOpenShell { ) -> Result, Status> { Ok(Response::new(ListProvidersResponse { providers: self.state.providers.lock().await.clone(), + next_page_token: String::new(), })) } @@ -473,7 +475,10 @@ impl OpenShell for TestOpenShell { .map(openshell_providers::ProviderTypeProfile::to_proto) .collect(); Ok(Response::new( - openshell_core::proto::ListProviderProfilesResponse { profiles }, + openshell_core::proto::ListProviderProfilesResponse { + profiles, + next_page_token: String::new(), + }, )) } @@ -1874,7 +1879,7 @@ async fn sandbox_template_list_and_delete_send_workspace_requests() { run::sandbox_template_list( &server.endpoint, 25, - 5, + "next-template-page", Some("team=runtime"), false, "table", @@ -1892,8 +1897,8 @@ async fn sandbox_template_list_and_delete_send_workspace_requests() { let list_request = list_requests .first() .expect("template list request should be recorded"); - assert_eq!(list_request.limit, 25); - assert_eq!(list_request.offset, 5); + assert_eq!(list_request.page_size, 25); + assert_eq!(list_request.page_token, "next-template-page"); assert_eq!(list_request.label_selector, "team=runtime"); assert_eq!( selected_workspace(&list_request.workspace_scope), diff --git a/crates/openshell-conformance/src/scenarios/smoke.rs b/crates/openshell-conformance/src/scenarios/smoke.rs index 7c5fa0068c..59f2d2b38f 100644 --- a/crates/openshell-conformance/src/scenarios/smoke.rs +++ b/crates/openshell-conformance/src/scenarios/smoke.rs @@ -22,6 +22,12 @@ struct SandboxListEntry { phase: String, } +#[derive(Debug, Deserialize)] +struct SandboxListPage { + sandboxes: Vec, + next_page_token: String, +} + /// Certify status -> create -> list Ready -> exec -> delete -> list empty. pub const SMOKE_SCENARIO: Scenario = Scenario { name: "smoke", @@ -162,22 +168,22 @@ async fn find_sandbox( sandbox_name: &str, step: &str, ) -> Result, String> { - let mut offset = 0u32; + let mut page_token = String::new(); + let mut page = 0u32; loop { - let limit = LIST_PAGE_SIZE.to_string(); - let page_offset = offset.to_string(); + let page_size = LIST_PAGE_SIZE.to_string(); let result = runner - .step(format!("{step}/{offset}")) - .description(format!("sandbox list page at offset {offset} succeeds")) + .step(format!("{step}/{page}")) + .description(format!("sandbox list page {page} succeeds")) .with_timeout(LIST_ATTEMPT_TIMEOUT) .run(&[ "sandbox", "list", - "--limit", - &limit, - "--offset", - &page_offset, + "--page-size", + &page_size, + "--page-token", + &page_token, "--output", "json", ]) @@ -185,10 +191,11 @@ async fn find_sandbox( .map_err(|error| error.to_string())?; result.require_success()?; - let sandboxes = result - .json::>() + let response = result + .json::() .map_err(|error| error.to_string())?; - if let Some(sandbox) = sandboxes + if let Some(sandbox) = response + .sandboxes .iter() .find(|sandbox| sandbox.name == sandbox_name) { @@ -197,12 +204,13 @@ async fn find_sandbox( phase: sandbox.phase.clone(), })); } - if sandboxes.len() < LIST_PAGE_SIZE as usize { + if response.next_page_token.is_empty() { return Ok(None); } - offset = offset - .checked_add(LIST_PAGE_SIZE) - .ok_or_else(|| "sandbox list pagination offset overflowed".to_string())?; + page_token = response.next_page_token; + page = page + .checked_add(1) + .ok_or_else(|| "sandbox list page counter overflowed".to_string())?; } } diff --git a/crates/openshell-core/src/proto/mod.rs b/crates/openshell-core/src/proto/mod.rs index d0b976ff24..43d2bce267 100644 --- a/crates/openshell-core/src/proto/mod.rs +++ b/crates/openshell-core/src/proto/mod.rs @@ -63,6 +63,11 @@ pub mod middleware { pub use super::generated::openshell::middleware::v1; } +#[doc(hidden)] +pub mod pagination { + pub use super::generated::openshell::internal::pagination::v1; +} + pub mod gateway_interceptor { pub use super::generated::openshell::gateway_interceptor::v1; } diff --git a/crates/openshell-sdk/README.md b/crates/openshell-sdk/README.md index dc57d5ec11..4ff8aca91c 100644 --- a/crates/openshell-sdk/README.md +++ b/crates/openshell-sdk/README.md @@ -59,6 +59,12 @@ Curated calls without a workspace argument explicitly select the `default` workspace. Cross-workspace listing uses the separate `*_all_workspaces` methods and requires Platform Admin access. +Curated list methods follow continuation tokens until the collection is +exhausted. `ListOptions::page_size` and +`SandboxTemplateListOptions::page_size` control the size of each gateway +request; callers that need explicit page boundaries can use the raw protobuf +client. + ```rust use openshell_sdk::{ ClientConfig, OpenShellClient, SandboxTemplateCreateSpec, diff --git a/crates/openshell-sdk/src/client.rs b/crates/openshell-sdk/src/client.rs index 4713f0ed1c..d8f1b26205 100644 --- a/crates/openshell-sdk/src/client.rs +++ b/crates/openshell-sdk/src/client.rs @@ -196,18 +196,27 @@ impl OpenShellClient { &self, opts: SandboxTemplateListOptions, ) -> Result> { - let response = self - .unary(|mut grpc| { - let request = proto::ListSandboxTemplatesRequest { - limit: opts.limit, - offset: opts.offset, - label_selector: opts.label_selector.clone(), - workspace_scope: Some(proto::workspace_selector("default")), - }; - async move { grpc.list_sandbox_templates(request).await } - }) - .await?; - Ok(response.templates) + let mut templates = Vec::new(); + let mut page_token = String::new(); + loop { + let response = self + .unary(|mut grpc| { + let page_token = page_token.clone(); + let request = proto::ListSandboxTemplatesRequest { + page_size: opts.page_size, + page_token, + label_selector: opts.label_selector.clone(), + workspace_scope: Some(proto::workspace_selector("default")), + }; + async move { grpc.list_sandbox_templates(request).await } + }) + .await?; + templates.extend(response.templates); + if response.next_page_token.is_empty() { + return Ok(templates); + } + page_token = response.next_page_token; + } } /// List reusable sandbox templates across all workspaces. @@ -215,18 +224,27 @@ impl OpenShellClient { &self, opts: SandboxTemplateListOptions, ) -> Result> { - let response = self - .unary(|mut grpc| { - let request = proto::ListSandboxTemplatesRequest { - limit: opts.limit, - offset: opts.offset, - label_selector: opts.label_selector.clone(), - workspace_scope: Some(proto::all_workspaces_selector()), - }; - async move { grpc.list_sandbox_templates(request).await } - }) - .await?; - Ok(response.templates) + let mut templates = Vec::new(); + let mut page_token = String::new(); + loop { + let response = self + .unary(|mut grpc| { + let page_token = page_token.clone(); + let request = proto::ListSandboxTemplatesRequest { + page_size: opts.page_size, + page_token, + label_selector: opts.label_selector.clone(), + workspace_scope: Some(proto::all_workspaces_selector()), + }; + async move { grpc.list_sandbox_templates(request).await } + }) + .await?; + templates.extend(response.templates); + if response.next_page_token.is_empty() { + return Ok(templates); + } + page_token = response.next_page_token; + } } /// Delete a reusable sandbox template by name from the default workspace. @@ -259,22 +277,27 @@ impl OpenShellClient { /// List sandboxes. pub async fn list_sandboxes(&self, opts: ListOptions) -> Result> { - let response = self - .unary(|mut grpc| { - let request = proto::ListSandboxesRequest { - limit: opts.limit, - offset: opts.offset, - label_selector: opts.label_selector.clone().unwrap_or_default(), - workspace_scope: Some(proto::workspace_selector("default")), - }; - async move { grpc.list_sandboxes(request).await } - }) - .await?; - Ok(response - .sandboxes - .into_iter() - .map(SandboxRef::from_proto) - .collect()) + let mut sandboxes = Vec::new(); + let mut page_token = String::new(); + loop { + let response = self + .unary(|mut grpc| { + let page_token = page_token.clone(); + let request = proto::ListSandboxesRequest { + page_size: opts.page_size, + page_token, + label_selector: opts.label_selector.clone().unwrap_or_default(), + workspace_scope: Some(proto::workspace_selector("default")), + }; + async move { grpc.list_sandboxes(request).await } + }) + .await?; + sandboxes.extend(response.sandboxes.into_iter().map(SandboxRef::from_proto)); + if response.next_page_token.is_empty() { + return Ok(sandboxes); + } + page_token = response.next_page_token; + } } /// Delete a sandbox by name. @@ -384,22 +407,27 @@ impl OpenShellClient { &self, opts: ListOptions, ) -> Result> { - let response = self - .unary(|mut grpc| { - let request = proto::ListSandboxesRequest { - limit: opts.limit, - offset: opts.offset, - label_selector: opts.label_selector.clone().unwrap_or_default(), - workspace_scope: Some(proto::all_workspaces_selector()), - }; - async move { grpc.list_sandboxes(request).await } - }) - .await?; - Ok(response - .sandboxes - .into_iter() - .map(SandboxRef::from_proto) - .collect()) + let mut sandboxes = Vec::new(); + let mut page_token = String::new(); + loop { + let response = self + .unary(|mut grpc| { + let page_token = page_token.clone(); + let request = proto::ListSandboxesRequest { + page_size: opts.page_size, + page_token, + label_selector: opts.label_selector.clone().unwrap_or_default(), + workspace_scope: Some(proto::all_workspaces_selector()), + }; + async move { grpc.list_sandboxes(request).await } + }) + .await?; + sandboxes.extend(response.sandboxes.into_iter().map(SandboxRef::from_proto)); + if response.next_page_token.is_empty() { + return Ok(sandboxes); + } + page_token = response.next_page_token; + } } /// Create a new workspace. @@ -441,21 +469,31 @@ impl OpenShellClient { /// List workspaces. pub async fn list_workspaces(&self, opts: ListOptions) -> Result> { - let response = self - .unary(|mut grpc| { - let request = proto::ListWorkspacesRequest { - limit: opts.limit, - offset: opts.offset, - label_selector: opts.label_selector.clone().unwrap_or_default(), - }; - async move { grpc.list_workspaces(request).await } - }) - .await?; - Ok(response - .workspaces - .into_iter() - .map(WorkspaceRef::from_proto) - .collect()) + let mut workspaces = Vec::new(); + let mut page_token = String::new(); + loop { + let response = self + .unary(|mut grpc| { + let page_token = page_token.clone(); + let request = proto::ListWorkspacesRequest { + page_size: opts.page_size, + page_token, + label_selector: opts.label_selector.clone().unwrap_or_default(), + }; + async move { grpc.list_workspaces(request).await } + }) + .await?; + workspaces.extend( + response + .workspaces + .into_iter() + .map(WorkspaceRef::from_proto), + ); + if response.next_page_token.is_empty() { + return Ok(workspaces); + } + page_token = response.next_page_token; + } } /// Delete a workspace by name. @@ -700,19 +738,28 @@ impl WorkspaceScopedClient { &self, opts: SandboxTemplateListOptions, ) -> Result> { - let response = self - .client - .unary(|mut grpc| { - let request = proto::ListSandboxTemplatesRequest { - limit: opts.limit, - offset: opts.offset, - label_selector: opts.label_selector.clone(), - workspace_scope: Some(proto::workspace_selector(&self.workspace)), - }; - async move { grpc.list_sandbox_templates(request).await } - }) - .await?; - Ok(response.templates) + let mut templates = Vec::new(); + let mut page_token = String::new(); + loop { + let response = self + .client + .unary(|mut grpc| { + let page_token = page_token.clone(); + let request = proto::ListSandboxTemplatesRequest { + page_size: opts.page_size, + page_token, + label_selector: opts.label_selector.clone(), + workspace_scope: Some(proto::workspace_selector(&self.workspace)), + }; + async move { grpc.list_sandbox_templates(request).await } + }) + .await?; + templates.extend(response.templates); + if response.next_page_token.is_empty() { + return Ok(templates); + } + page_token = response.next_page_token; + } } /// Delete a reusable sandbox template by name in this workspace. @@ -747,23 +794,28 @@ impl WorkspaceScopedClient { /// List sandboxes in this workspace. pub async fn list_sandboxes(&self, opts: ListOptions) -> Result> { - let response = self - .client - .unary(|mut grpc| { - let request = proto::ListSandboxesRequest { - limit: opts.limit, - offset: opts.offset, - label_selector: opts.label_selector.clone().unwrap_or_default(), - workspace_scope: Some(proto::workspace_selector(&self.workspace)), - }; - async move { grpc.list_sandboxes(request).await } - }) - .await?; - Ok(response - .sandboxes - .into_iter() - .map(SandboxRef::from_proto) - .collect()) + let mut sandboxes = Vec::new(); + let mut page_token = String::new(); + loop { + let response = self + .client + .unary(|mut grpc| { + let page_token = page_token.clone(); + let request = proto::ListSandboxesRequest { + page_size: opts.page_size, + page_token, + label_selector: opts.label_selector.clone().unwrap_or_default(), + workspace_scope: Some(proto::workspace_selector(&self.workspace)), + }; + async move { grpc.list_sandboxes(request).await } + }) + .await?; + sandboxes.extend(response.sandboxes.into_iter().map(SandboxRef::from_proto)); + if response.next_page_token.is_empty() { + return Ok(sandboxes); + } + page_token = response.next_page_token; + } } /// Delete a sandbox by name in this workspace. diff --git a/crates/openshell-sdk/src/types.rs b/crates/openshell-sdk/src/types.rs index 12b5a72cb9..1c3fa7014d 100644 --- a/crates/openshell-sdk/src/types.rs +++ b/crates/openshell-sdk/src/types.rs @@ -161,10 +161,8 @@ pub type SandboxStartup = proto::SandboxStartup; /// Options for listing reusable sandbox templates. #[derive(Clone, Debug, Default)] pub struct SandboxTemplateListOptions { - /// Maximum templates to return. `0` defers to the server default. - pub limit: u32, - /// Offset into the result list. - pub offset: u32, + /// Page size requested while collecting templates. `0` uses the server default. + pub page_size: i32, /// Optional label selector in `key=value,key2=value2` form. pub label_selector: String, } @@ -247,10 +245,8 @@ impl WorkspaceRef { /// Options for listing sandboxes. #[derive(Clone, Debug, Default)] pub struct ListOptions { - /// Maximum sandboxes to return. `0` defers to the server default. - pub limit: u32, - /// Offset into the result list. - pub offset: u32, + /// Page size requested while collecting results. `0` uses the server default. + pub page_size: i32, /// Optional Kubernetes-style label selector (e.g. `env=prod,team=core`). pub label_selector: Option, } diff --git a/crates/openshell-sdk/tests/client_mock.rs b/crates/openshell-sdk/tests/client_mock.rs index 5fd2133b5c..7acce33e0e 100644 --- a/crates/openshell-sdk/tests/client_mock.rs +++ b/crates/openshell-sdk/tests/client_mock.rs @@ -55,12 +55,14 @@ struct MockState { last_stop: Mutex>, last_start: Mutex>, last_list_request: Mutex>, + list_requests: Mutex>, last_exec_request: Mutex>, last_workspace_request: Mutex>, get_calls: AtomicU32, phase_sequence: Vec, get_returns_not_found: bool, not_found_after: Option, + paginate_list: bool, /// When set, `health` rejects any request whose `authorization` header /// does not match this exact value (e.g. `"Bearer fresh-token"`). require_bearer: Option, @@ -278,6 +280,7 @@ impl OpenShell for TestOpenShell { workload_template_proto("python", "default"), workload_template_proto("cuda", "gpu"), ], + next_page_token: String::new(), })) } @@ -360,12 +363,36 @@ impl OpenShell for TestOpenShell { &self, request: tonic::Request, ) -> Result, Status> { - *self.state.last_list_request.lock().await = Some(request.into_inner()); + let request = request.into_inner(); + *self.state.last_list_request.lock().await = Some(request.clone()); + self.state.list_requests.lock().await.push(request.clone()); + if self.state.paginate_list { + let (sandboxes, next_page_token) = if request.page_token.is_empty() { + ( + vec![sandbox_with_phase("alpha", proto::SandboxPhase::Ready)], + "page-2".to_string(), + ) + } else { + assert_eq!(request.page_token, "page-2"); + ( + vec![sandbox_with_phase( + "beta", + proto::SandboxPhase::Provisioning, + )], + String::new(), + ) + }; + return Ok(Response::new(proto::ListSandboxesResponse { + sandboxes, + next_page_token, + })); + } Ok(Response::new(proto::ListSandboxesResponse { sandboxes: vec![ sandbox_with_phase("alpha", proto::SandboxPhase::Ready), sandbox_with_phase("beta", proto::SandboxPhase::Provisioning), ], + next_page_token: String::new(), })) } @@ -826,6 +853,7 @@ impl OpenShell for TestOpenShell { workspace_proto("default", proto::datamodel::v1::WorkspacePhase::Active), workspace_proto("staging", proto::datamodel::v1::WorkspacePhase::Active), ], + next_page_token: String::new(), })) } @@ -1004,16 +1032,15 @@ async fn sandbox_template_crud_uses_default_workspace() { let listed = client .list_sandbox_templates_all_workspaces(SandboxTemplateListOptions { - limit: 10, - offset: 2, + page_size: 10, label_selector: String::new(), }) .await .unwrap(); assert_eq!(listed.len(), 2); let observed_list = state.last_template_list.lock().await.clone().unwrap(); - assert_eq!(observed_list.limit, 10); - assert_eq!(observed_list.offset, 2); + assert_eq!(observed_list.page_size, 10); + assert!(observed_list.page_token.is_empty()); assert!(selects_all_workspaces(&observed_list.workspace_scope)); let deleted = client.delete_sandbox_template("python").await.unwrap(); @@ -1066,8 +1093,7 @@ async fn list_sandboxes_propagates_filters() { let client = connect(&endpoint).await; let opts = ListOptions { - limit: 25, - offset: 5, + page_size: 25, label_selector: Some("team=core".to_string()), }; let items = client.list_sandboxes(opts).await.unwrap(); @@ -1077,11 +1103,38 @@ async fn list_sandboxes_propagates_filters() { assert_eq!(items[1].phase, SandboxPhase::Provisioning); let observed = state.last_list_request.lock().await.clone().unwrap(); - assert_eq!(observed.limit, 25); - assert_eq!(observed.offset, 5); + assert_eq!(observed.page_size, 25); + assert!(observed.page_token.is_empty()); assert_eq!(observed.label_selector, "team=core"); } +#[tokio::test] +async fn list_sandboxes_follows_continuation_tokens() { + let state = Arc::new(MockState { + paginate_list: true, + ..Default::default() + }); + let endpoint = start_mock(state.clone()).await; + let client = connect(&endpoint).await; + + let items = client + .list_sandboxes(ListOptions { + page_size: 1, + label_selector: Some("team=core".to_string()), + }) + .await + .unwrap(); + + assert_eq!(items.len(), 2); + assert_eq!(items[0].name, "alpha"); + assert_eq!(items[1].name, "beta"); + let requests = state.list_requests.lock().await; + assert_eq!(requests.len(), 2); + assert!(requests[0].page_token.is_empty()); + assert_eq!(requests[1].page_token, "page-2"); + assert_eq!(requests[1].label_selector, "team=core"); +} + #[tokio::test] async fn delete_sandbox_returns_server_ack() { let state = Arc::new(MockState::default()); diff --git a/crates/openshell-server/migrations/postgres/008_add_pagination_indexes.sql b/crates/openshell-server/migrations/postgres/008_add_pagination_indexes.sql new file mode 100644 index 0000000000..b6c9934e78 --- /dev/null +++ b/crates/openshell-server/migrations/postgres/008_add_pagination_indexes.sql @@ -0,0 +1,6 @@ +-- Support workspace-scoped and platform-wide keyset pagination orderings. +CREATE INDEX IF NOT EXISTS objects_workspace_page_idx + ON objects (object_type, workspace, created_at_ms, COALESCE(name, ''), id); + +CREATE INDEX IF NOT EXISTS objects_all_workspaces_page_idx + ON objects (object_type, created_at_ms, COALESCE(name, ''), workspace, id); diff --git a/crates/openshell-server/migrations/sqlite/008_add_pagination_indexes.sql b/crates/openshell-server/migrations/sqlite/008_add_pagination_indexes.sql new file mode 100644 index 0000000000..b6c9934e78 --- /dev/null +++ b/crates/openshell-server/migrations/sqlite/008_add_pagination_indexes.sql @@ -0,0 +1,6 @@ +-- Support workspace-scoped and platform-wide keyset pagination orderings. +CREATE INDEX IF NOT EXISTS objects_workspace_page_idx + ON objects (object_type, workspace, created_at_ms, COALESCE(name, ''), id); + +CREATE INDEX IF NOT EXISTS objects_all_workspaces_page_idx + ON objects (object_type, created_at_ms, COALESCE(name, ''), workspace, id); diff --git a/crates/openshell-server/src/compute/mod.rs b/crates/openshell-server/src/compute/mod.rs index 70356e1add..70ffb5fe5d 100644 --- a/crates/openshell-server/src/compute/mod.rs +++ b/crates/openshell-server/src/compute/mod.rs @@ -10,8 +10,8 @@ pub mod rootfs_tar; use crate::grpc::policy::SANDBOX_SETTINGS_OBJECT_TYPE; use crate::otel_tracing::TraceContextInterceptor; use crate::persistence::{ - DRAFT_CHUNK_OBJECT_TYPE, ObjectCursor, ObjectId, ObjectName, ObjectRecord, ObjectType, - POLICY_OBJECT_TYPE, Store, WriteCondition, + DRAFT_CHUNK_OBJECT_TYPE, ObjectCursor, ObjectId, ObjectListQuery, ObjectName, ObjectRecord, + ObjectType, POLICY_OBJECT_TYPE, Store, WriteCondition, }; use crate::sandbox_index::SandboxIndex; use crate::sandbox_watch::SandboxWatchBus; @@ -2491,25 +2491,11 @@ impl ComputeRuntime { } async fn list_persisted_sandbox_ids(&self, operation: &str) -> Result, String> { - let mut sandbox_ids = Vec::new(); - let mut offset = 0u32; - loop { - let records = self - .store - .list_by_type(Sandbox::object_type(), LIFECYCLE_SWEEP_PAGE_SIZE, offset) - .await - .map_err(|err| format!("failed to list sandboxes for {operation}: {err}"))?; - let page_len = u32::try_from(records.len()) - .map_err(|_| format!("sandbox page size overflow during {operation}"))?; - sandbox_ids.extend(records.into_iter().map(|record| record.id)); - if page_len < LIFECYCLE_SWEEP_PAGE_SIZE { - break; - } - offset = offset - .checked_add(page_len) - .ok_or_else(|| format!("sandbox pagination offset overflow during {operation}"))?; - } - Ok(sandbox_ids) + self.store + .collect_records(Sandbox::object_type(), ObjectListQuery::AllWorkspaces) + .await + .map(|records| records.into_iter().map(|record| record.id).collect()) + .map_err(|err| format!("failed to list sandboxes for {operation}: {err}")) } async fn mark_sandbox_error(&self, sandbox: &Sandbox, reason: &str, message: &str) { @@ -2791,7 +2777,7 @@ impl ComputeRuntime { let records = self .store - .list_by_type(Sandbox::object_type(), 500, 0) + .collect_records(Sandbox::object_type(), ObjectListQuery::AllWorkspaces) .await .map_err(|e| e.to_string()) .inspect_err(|_| crate::otel_tracing::mark_error(&tracing::Span::current()))?; @@ -3501,10 +3487,6 @@ impl ComputeRuntime { .await } - // TODO: introduce a per-sandbox cap on service endpoints and paginate - // this cleanup loop, or query by sandbox label instead of scanning the - // full workspace. Without a cap the flat 1,000-record page could miss - // endpoints in large workspaces. async fn cleanup_sandbox_service_endpoints( &self, sandbox_id: &str, @@ -3512,7 +3494,10 @@ impl ComputeRuntime { ) -> Result<(), String> { let records = self .store - .list(ServiceEndpoint::object_type(), workspace, 1000, 0) + .collect_records( + ServiceEndpoint::object_type(), + ObjectListQuery::Workspace(workspace), + ) .await .map_err(|e| format!("list service endpoints: {e}"))?; diff --git a/crates/openshell-server/src/grpc/mod.rs b/crates/openshell-server/src/grpc/mod.rs index c5a12a15b5..95bd6eaa3b 100644 --- a/crates/openshell-server/src/grpc/mod.rs +++ b/crates/openshell-server/src/grpc/mod.rs @@ -68,24 +68,6 @@ use tonic::{Request, Response, Status}; use crate::ServerState; -// --------------------------------------------------------------------------- -// Public re-exports -// --------------------------------------------------------------------------- - -/// Maximum number of records a single list RPC may return. -/// -/// Client-provided `limit` values are clamped to this ceiling to prevent -/// unbounded memory allocation from an excessively large page request. -pub const MAX_PAGE_SIZE: u32 = 1000; - -/// Clamp a client-provided page `limit`. -/// -/// Returns `default` when `raw` is 0 (the protobuf zero-value convention), -/// otherwise returns the smaller of `raw` and `max`. -pub fn clamp_limit(raw: u32, default: u32, max: u32) -> u32 { - if raw == 0 { default } else { raw.min(max) } -} - /// Map a `PersistenceError` to an appropriate gRPC `Status`. /// /// CAS conflicts (optimistic concurrency failures) are mapped to `ABORTED` @@ -932,31 +914,6 @@ mod tests { ResourceCapabilities as DriverResourceCapabilities, }; - #[test] - fn clamp_limit_zero_returns_default() { - assert_eq!(clamp_limit(0, 100, MAX_PAGE_SIZE), 100); - assert_eq!(clamp_limit(0, 50, MAX_PAGE_SIZE), 50); - } - - #[test] - fn clamp_limit_within_range_passes_through() { - assert_eq!(clamp_limit(1, 100, MAX_PAGE_SIZE), 1); - assert_eq!(clamp_limit(500, 100, MAX_PAGE_SIZE), 500); - assert_eq!( - clamp_limit(MAX_PAGE_SIZE, 100, MAX_PAGE_SIZE), - MAX_PAGE_SIZE - ); - } - - #[test] - fn clamp_limit_exceeding_max_is_capped() { - assert_eq!( - clamp_limit(MAX_PAGE_SIZE + 1, 100, MAX_PAGE_SIZE), - MAX_PAGE_SIZE - ); - assert_eq!(clamp_limit(u32::MAX, 100, MAX_PAGE_SIZE), MAX_PAGE_SIZE); - } - #[test] fn public_resource_capabilities_preserves_reported_fields() { let driver_capabilities = DriverResourceCapabilities { diff --git a/crates/openshell-server/src/grpc/policy.rs b/crates/openshell-server/src/grpc/policy.rs index 62bc0f8575..814ee1b567 100644 --- a/crates/openshell-server/src/grpc/policy.rs +++ b/crates/openshell-server/src/grpc/policy.rs @@ -16,8 +16,10 @@ use crate::auth::workspace_authz::{ MinWorkspaceRole, authorize_sandbox_workspace, authorize_workspace_selector, require_platform_admin, selected_workspace_name, }; +use crate::pagination::Pagination; use crate::persistence::{ - DraftChunkRecord, ObjectId, ObjectName, ObjectType, ObjectWorkspace, PolicyRecord, Store, + DraftChunkRecord, ObjectId, ObjectListQuery, ObjectName, ObjectType, ObjectWorkspace, + PolicyRecord, Store, }; use crate::policy_store::{AtomicPolicyRevisionWrite, PolicyStoreExt}; use crate::provider_profile_sources::EffectiveProviderProfileCatalog; @@ -88,7 +90,7 @@ use super::validation::{ validate_no_reserved_provider_policy_keys, validate_policy_safety, validate_static_fields_unchanged, }; -use super::{MAX_PAGE_SIZE, StoredSettingValue, StoredSettings, clamp_limit}; +use super::{StoredSettingValue, StoredSettings}; use crate::persistence::current_time_ms; // --------------------------------------------------------------------------- @@ -100,6 +102,7 @@ const GLOBAL_SETTINGS_OBJECT_TYPE: &str = "gateway_settings"; const GLOBAL_SETTINGS_NAME: &str = "global"; /// Internal object type for durable sandbox-scoped settings. pub const SANDBOX_SETTINGS_OBJECT_TYPE: &str = "sandbox_settings"; +const PROVIDER_COMPOSITION_VALIDATION_PAGE_SIZE: u32 = 1000; /// Reserved settings key used to store global policy payload. const POLICY_SETTING_KEY: &str = "policy"; /// Sentinel `sandbox_id` used to store global policy revisions. @@ -2094,18 +2097,20 @@ fn provider_policy_composition_enabled_in(settings: &StoredSettings) -> Result Result<(), Status> { - let mut offset = 0; let mut catalogs = HashMap::::new(); - + let mut cursor = None; loop { - let sandboxes = state + let page = state .store - .list_all_messages::(MAX_PAGE_SIZE, offset) + .list_message_page::( + ObjectListQuery::AllWorkspaces, + cursor.as_ref(), + PROVIDER_COMPOSITION_VALIDATION_PAGE_SIZE, + ) .await .map_err(|e| Status::internal(format!("list sandboxes failed: {e}")))?; - let page_len = sandboxes.len(); - for sandbox in sandboxes { + for sandbox in page.messages { let provider_names = sandbox .spec .as_ref() @@ -2146,13 +2151,11 @@ async fn validate_provider_composition_for_existing_sandboxes( })?; } - if page_len < MAX_PAGE_SIZE as usize { - break; - } - offset = offset.saturating_add(MAX_PAGE_SIZE); + let Some(next_cursor) = page.next_cursor else { + return Ok(()); + }; + cursor = Some(next_cursor); } - - Ok(()) } pub async fn validate_provider_composition_startup_preflight( @@ -4038,19 +4041,44 @@ pub(super) async fn handle_list_sandbox_policies( sandbox.object_id().to_string() }; - let limit = clamp_limit(req.limit, 50, MAX_PAGE_SIZE); - let records = state + let pagination = Pagination::new( + req.page_size, + &req.page_token, + "ListSandboxPolicies", + &[ + &req.name, + if req.global { "true" } else { "false" }, + &workspace, + ], + )?; + let mut records = state .store - .list_policies(&policy_id, limit, req.offset) + .list_policies_before( + &policy_id, + pagination.page_size() + 1, + pagination.policy_cursor()?, + ) .await .map_err(|e| Status::internal(format!("list policies failed: {e}")))?; + let page_size = usize::try_from(pagination.page_size()) + .map_err(|_| Status::internal("page size does not fit usize"))?; + let has_more = records.len() > page_size; + records.truncate(page_size); + let next_page_token = pagination.next_policy_token( + has_more + .then(|| records.last().map(|record| record.version)) + .flatten(), + ); let revisions = records .iter() .map(|r| policy_record_to_revision(r, false)) .collect::, Status>>()?; - Ok(Response::new(ListSandboxPoliciesResponse { revisions })) + Ok(Response::new(ListSandboxPoliciesResponse { + revisions, + next_page_token, + })) } pub(super) async fn handle_report_policy_status( @@ -7261,6 +7289,101 @@ mod tests { .expect("test global policy must be present") } + #[tokio::test] + async fn list_sandbox_policies_traverses_multiple_pages_exactly_once() { + let state = test_server_state().await; + let policy = ProtoSandboxPolicy::default(); + let payload = policy.encode_to_vec(); + for version in 1..=7 { + state + .store + .put_policy_revision( + &format!("global-policy-{version}"), + GLOBAL_POLICY_SANDBOX_ID, + "", + version, + &payload, + &format!("hash-{version}"), + ) + .await + .expect("store policy revision"); + } + + let mut versions = Vec::new(); + let mut page_token = String::new(); + let mut page_size = 2; + loop { + let page = handle_list_sandbox_policies( + &state, + authed_request(ListSandboxPoliciesRequest { + page_size, + page_token, + global: true, + ..Default::default() + }), + ) + .await + .unwrap() + .into_inner(); + versions.extend(page.revisions.into_iter().map(|revision| revision.version)); + if page.next_page_token.is_empty() { + break; + } + page_token = page.next_page_token; + page_size = 3; + } + + assert_eq!(versions, vec![7, 6, 5, 4, 3, 2, 1]); + } + + #[tokio::test] + async fn list_sandbox_policies_rejects_token_from_different_filter() { + let state = test_server_state().await; + let policy = ProtoSandboxPolicy::default(); + let payload = policy.encode_to_vec(); + for version in 1..=2 { + state + .store + .put_policy_revision( + &format!("global-policy-{version}"), + GLOBAL_POLICY_SANDBOX_ID, + "", + version, + &payload, + &format!("hash-{version}"), + ) + .await + .expect("store policy revision"); + } + let first = handle_list_sandbox_policies( + &state, + authed_request(ListSandboxPoliciesRequest { + page_size: 1, + global: true, + ..Default::default() + }), + ) + .await + .unwrap() + .into_inner(); + assert!(!first.next_page_token.is_empty()); + + let error = handle_list_sandbox_policies( + &state, + authed_request(ListSandboxPoliciesRequest { + page_size: 1, + page_token: first.next_page_token, + global: true, + name: "different".to_string(), + ..Default::default() + }), + ) + .await + .unwrap_err(); + + assert_eq!(error.code(), Code::InvalidArgument); + } + #[tokio::test] async fn get_sandbox_config_rejects_invalid_spec_policy_before_history_backfill() { let state = test_server_state().await; @@ -7587,7 +7710,7 @@ mod tests { &state, with_user(Request::new(ListSandboxPoliciesRequest { name: "stored-invalid-history".to_string(), - limit: 10, + page_size: 10, workspace_scope: Some(openshell_core::proto::workspace_selector("default")), ..Default::default() })), @@ -13568,8 +13691,8 @@ mod tests { &state, authed_request(ListSandboxPoliciesRequest { name: sandbox_name.clone(), - limit: 10, - offset: 0, + page_size: 10, + page_token: String::new(), global: false, workspace_scope: Some(openshell_core::proto::workspace_selector( "default".to_string(), @@ -13633,8 +13756,8 @@ mod tests { &state, authed_request(ListSandboxPoliciesRequest { name: sandbox_name.clone(), - limit: 10, - offset: 0, + page_size: 10, + page_token: String::new(), global: false, workspace_scope: Some(openshell_core::proto::workspace_selector( "default".to_string(), diff --git a/crates/openshell-server/src/grpc/provider.rs b/crates/openshell-server/src/grpc/provider.rs index 46eb9ce9be..0500b668e8 100644 --- a/crates/openshell-server/src/grpc/provider.rs +++ b/crates/openshell-server/src/grpc/provider.rs @@ -7,12 +7,14 @@ #[cfg(test)] use crate::credentials::RefreshMaterialScope; +use crate::pagination::Pagination; use crate::persistence::{ - ObjectId, ObjectLabels, ObjectName, ObjectType, Store, WriteCondition, generate_name, + ObjectId, ObjectLabels, ObjectListQuery, ObjectName, ObjectType, Store, WriteCondition, + generate_name, }; use crate::provider_profile_sources::{ - EffectiveProviderProfileCatalog, ProviderProfileSources, profile_response_payload, - profile_storage_payload, stored_profile_resource_version, + EffectiveProviderProfileCatalog, ProfileScope, ProviderProfileSources, + profile_response_payload, profile_storage_payload, stored_profile_resource_version, }; use crate::storage_proto::{StoredProviderCredentialRefreshState, StoredProviderProfile}; use openshell_core::metadata::ObjectWorkspace; @@ -33,9 +35,7 @@ use tonic::Status; use tracing::warn; use super::validation::{validate_provider_fields, validate_provider_mutable_fields}; -use super::{ - MAX_MAP_KEY_LEN, MAX_MAP_VALUE_LEN, MAX_PAGE_SIZE, MAX_PROVIDER_CONFIG_ENTRIES, clamp_limit, -}; +use super::{MAX_MAP_KEY_LEN, MAX_MAP_VALUE_LEN, MAX_PROVIDER_CONFIG_ENTRIES}; const GATEWAY_SPIFFE_WORKLOAD_API_SOCKET: &str = "OPENSHELL_GATEWAY_SPIFFE_WORKLOAD_API_SOCKET"; @@ -277,6 +277,7 @@ pub(super) async fn get_provider_record( .map(redact_provider_credentials) } +#[cfg(test)] pub(super) async fn list_provider_records( store: &Store, workspace: &str, @@ -627,32 +628,15 @@ async fn scan_sandboxes_inner( where F: FnMut(Sandbox) -> Option, { - let mut out = Vec::new(); - let mut offset = 0u32; - loop { - let records = if let Some(ws) = workspace { - store.list(Sandbox::object_type(), ws, 1000, offset).await - } else { - store - .list_by_type(Sandbox::object_type(), 1000, offset) - .await - } + let query = workspace.map_or(ObjectListQuery::AllWorkspaces, ObjectListQuery::Workspace); + let sandboxes: Vec = store + .collect_messages(query) + .await .map_err(|e| Status::internal(format!("list sandboxes failed: {e}")))?; - if records.is_empty() { - break; - } - offset = offset - .checked_add( - u32::try_from(records.len()) - .map_err(|_| Status::internal("sandbox page size exceeded u32"))?, - ) - .ok_or_else(|| Status::internal("sandbox pagination offset overflow"))?; - for record in records { - let sandbox = Sandbox::decode(record.payload.as_slice()) - .map_err(|e| Status::internal(format!("decode sandbox failed: {e}")))?; - if let Some(item) = f(sandbox) { - out.push(item); - } + let mut out = Vec::new(); + for sandbox in sandboxes { + if let Some(item) = f(sandbox) { + out.push(item); } } Ok(out) @@ -664,43 +648,28 @@ async fn providers_using_profile( profile_id: &str, ) -> Result, Status> { let is_platform_scope = workspace.is_empty(); - let mut offset = 0u32; let mut blocking = Vec::new(); - loop { - let records = if is_platform_scope { - store - .list_by_type(Provider::object_type(), 1000, offset) - .await - } else { - store - .list(Provider::object_type(), workspace, 1000, offset) - .await - } + let query = if is_platform_scope { + ObjectListQuery::AllWorkspaces + } else { + ObjectListQuery::Workspace(workspace) + }; + let providers: Vec = store + .collect_messages(query) + .await .map_err(|e| Status::internal(format!("list providers failed: {e}")))?; - if records.is_empty() { - break; - } - offset = offset - .checked_add( - u32::try_from(records.len()) - .map_err(|_| Status::internal("provider page size exceeded u32"))?, - ) - .ok_or_else(|| Status::internal("provider pagination offset overflow"))?; - for record in records { - let provider = Provider::decode(record.payload.as_slice()) - .map_err(|e| Status::internal(format!("decode provider failed: {e}")))?; - if provider.profile_workspace != workspace - || normalize_profile_id(&provider.r#type).as_deref() != Some(profile_id) - { - continue; - } - let label = if is_platform_scope { - format!("{}/{}", provider.object_workspace(), provider.object_name()) - } else { - provider.object_name().to_string() - }; - blocking.push(label); + for provider in providers { + if provider.profile_workspace != workspace + || normalize_profile_id(&provider.r#type).as_deref() != Some(profile_id) + { + continue; } + let label = if is_platform_scope { + format!("{}/{}", provider.object_workspace(), provider.object_name()) + } else { + provider.object_name().to_string() + }; + blocking.push(label); } blocking.sort(); blocking.dedup(); @@ -2569,8 +2538,6 @@ pub(super) async fn handle_list_providers( ) -> Result, Status> { let principal = super::extract_principal(&request)?; let request = request.into_inner(); - let limit = clamp_limit(request.limit, 100, MAX_PAGE_SIZE); - let scope = authorize_list_workspace_selector( &state.store, &state.admin_role, @@ -2579,13 +2546,8 @@ pub(super) async fn handle_list_providers( MinWorkspaceRole::User, ) .await?; - let providers = if matches!(scope, AuthorizedWorkspaceScope::AllWorkspaces) { - let all: Vec = state - .store - .list_all_messages(limit, request.offset) - .await - .map_err(|e| Status::internal(format!("list providers failed: {e}")))?; - all.into_iter().map(redact_provider_credentials).collect() + let workspace = if matches!(scope, AuthorizedWorkspaceScope::AllWorkspaces) { + None } else { let AuthorizedWorkspaceScope::Workspace(authz) = scope else { unreachable!("all-workspaces scope handled above") @@ -2593,10 +2555,34 @@ pub(super) async fn handle_list_providers( let workspace = super::workspace::resolve_workspace(state.store.as_ref(), &authz.workspace) .await? .name; - list_provider_records(state.store.as_ref(), &workspace, limit, request.offset).await? + Some(workspace) }; - - Ok(Response::new(ListProvidersResponse { providers })) + let scope_fingerprint = workspace.as_deref().unwrap_or("*"); + let pagination = Pagination::new( + request.page_size, + &request.page_token, + "ListProviders", + &[scope_fingerprint], + )?; + let after = pagination.object_cursor()?; + let query = workspace + .as_deref() + .map_or(ObjectListQuery::AllWorkspaces, ObjectListQuery::Workspace); + let page = state + .store + .list_message_page::(query, after.as_ref(), pagination.page_size()) + .await + .map_err(|e| Status::internal(format!("list providers failed: {e}")))?; + let providers = page + .messages + .into_iter() + .map(redact_provider_credentials) + .collect(); + let next_page_token = pagination.next_object_token(page.next_cursor.as_ref()); + Ok(Response::new(ListProvidersResponse { + providers, + next_page_token, + })) } /// Return provider profiles visible in the given workspace scope. @@ -2618,21 +2604,47 @@ pub(super) async fn handle_list_provider_profiles( ) .await? .name; - let limit = clamp_limit(request.limit, 100, MAX_PAGE_SIZE) as usize; - let offset = request.offset as usize; + let pagination = Pagination::new( + request.page_size, + &request.page_token, + "ListProviderProfiles", + &[&request.workspace], + )?; + let after = pagination.profile_cursor()?; let catalog = state .provider_profile_sources .snapshot_catalog(state.store.as_ref(), &workspace) .await?; - let profiles = catalog + let mut profiles = catalog .list_all_scoped_profiles() .into_iter() - .map(|(_, profile)| profile) - .skip(offset) - .take(limit) - .collect(); - - Ok(Response::new(ListProviderProfilesResponse { profiles })) + .map(|(scope, profile)| { + let scope = match scope { + ProfileScope::Static => "static", + ProfileScope::Platform => "platform", + ProfileScope::Workspace => "workspace", + }; + (format!("{}\0{scope}", profile.id), profile) + }) + .collect::>(); + profiles.sort_unstable_by(|left, right| left.0.cmp(&right.0)); + if let Some(after) = after { + profiles.retain(|(key, _)| key.as_str() > after); + } + let page_size = usize::try_from(pagination.page_size()) + .map_err(|_| Status::internal("page_size does not fit usize"))?; + let has_more = profiles.len() > page_size; + profiles.truncate(page_size); + let next_key = if has_more { + profiles.last().map(|(key, _)| key.as_str()) + } else { + None + }; + let next_page_token = pagination.next_profile_token(next_key); + Ok(Response::new(ListProviderProfilesResponse { + profiles: profiles.into_iter().map(|(_, profile)| profile).collect(), + next_page_token, + })) } pub(super) async fn handle_get_provider_profile( @@ -3012,7 +3024,7 @@ pub(super) async fn get_provider_type_profile( ) -> Result, Status> { // Query stored profiles scoped to the requested workspace. let stored: Vec = store - .list_messages(workspace, 10_000, 0) + .collect_messages(ObjectListQuery::Workspace(workspace)) .await .map_err(|e| Status::internal(format!("list provider profiles failed: {e}")))?; let id_norm = normalize_profile_id(id); @@ -5852,8 +5864,8 @@ mod tests { let response = handle_list_provider_profiles( &state, authed_request(ListProviderProfilesRequest { - limit: 100, - offset: 0, + page_size: 100, + page_token: String::new(), workspace: "default".to_string(), }), ) @@ -5898,6 +5910,81 @@ mod tests { ); } + #[tokio::test] + async fn list_provider_profiles_traverses_multiple_pages_exactly_once() { + let state = test_server_state().await; + let all = handle_list_provider_profiles( + &state, + authed_request(ListProviderProfilesRequest { + page_size: 100, + page_token: String::new(), + workspace: "default".to_string(), + }), + ) + .await + .unwrap() + .into_inner() + .profiles; + + let mut listed = Vec::new(); + let mut page_token = String::new(); + let mut page_size = 2; + loop { + let page = handle_list_provider_profiles( + &state, + authed_request(ListProviderProfilesRequest { + page_size, + page_token, + workspace: "default".to_string(), + }), + ) + .await + .unwrap() + .into_inner(); + listed.extend(page.profiles); + if page.next_page_token.is_empty() { + break; + } + page_token = page.next_page_token; + page_size = 3; + } + + assert_eq!( + listed.iter().map(|profile| &profile.id).collect::>(), + all.iter().map(|profile| &profile.id).collect::>() + ); + } + + #[tokio::test] + async fn list_provider_profiles_rejects_token_from_different_workspace_filter() { + let state = test_server_state().await; + let first = handle_list_provider_profiles( + &state, + authed_request(ListProviderProfilesRequest { + page_size: 1, + page_token: String::new(), + workspace: "default".to_string(), + }), + ) + .await + .unwrap() + .into_inner(); + assert!(!first.next_page_token.is_empty()); + + let error = handle_list_provider_profiles( + &state, + authed_request(ListProviderProfilesRequest { + page_size: 1, + page_token: first.next_page_token, + workspace: String::new(), + }), + ) + .await + .unwrap_err(); + + assert_eq!(error.code(), Code::InvalidArgument); + } + #[tokio::test] async fn get_provider_profile_returns_profile_or_not_found() { let state = test_server_state().await; @@ -5954,8 +6041,8 @@ mod tests { let listed = handle_list_provider_profiles( &state, authed_request(ListProviderProfilesRequest { - limit: 100, - offset: 0, + page_size: 100, + page_token: String::new(), workspace: "default".to_string(), }), ) @@ -13127,8 +13214,8 @@ mod tests { let listed = handle_list_providers( &state, authed_request(ListProvidersRequest { - limit: 100, - offset: 0, + page_size: 100, + page_token: String::new(), workspace_scope: Some(openshell_core::proto::workspace_selector( "default".to_string(), )), @@ -13143,8 +13230,8 @@ mod tests { let listed = handle_list_providers( &state, authed_request(ListProvidersRequest { - limit: 100, - offset: 0, + page_size: 100, + page_token: String::new(), workspace_scope: Some(openshell_core::proto::workspace_selector( "beta".to_string(), )), @@ -13174,8 +13261,8 @@ mod tests { let listed = handle_list_providers( &state, authed_request(ListProvidersRequest { - limit: 100, - offset: 0, + page_size: 100, + page_token: String::new(), workspace_scope: Some(openshell_core::proto::workspace_selector( "default".to_string(), )), @@ -13230,8 +13317,8 @@ mod tests { let listed = handle_list_providers( &state, authed_request(ListProvidersRequest { - limit: 100, - offset: 0, + page_size: 100, + page_token: String::new(), workspace_scope: Some(openshell_core::proto::all_workspaces_selector()), }), ) @@ -13596,8 +13683,8 @@ mod tests { handle_list_provider_profiles( &state, authed_request(ListProviderProfilesRequest { - limit: 200, - offset: 0, + page_size: 200, + page_token: String::new(), workspace, }), ) @@ -13720,8 +13807,8 @@ mod tests { let resp = handle_list_provider_profiles( &state, authed_request(ListProviderProfilesRequest { - limit: 200, - offset: 0, + page_size: 200, + page_token: String::new(), workspace: "default".to_string(), }), ) @@ -13765,8 +13852,8 @@ mod tests { let resp = handle_list_provider_profiles( &state, authed_request(ListProviderProfilesRequest { - limit: 200, - offset: 0, + page_size: 200, + page_token: String::new(), workspace: "default".to_string(), }), ) @@ -13905,8 +13992,8 @@ mod tests { let resp = handle_list_provider_profiles( &state, authed_request(ListProviderProfilesRequest { - limit: 200, - offset: 0, + page_size: 200, + page_token: String::new(), workspace: String::new(), }), ) diff --git a/crates/openshell-server/src/grpc/sandbox.rs b/crates/openshell-server/src/grpc/sandbox.rs index 17ae70d2fb..d428e64569 100644 --- a/crates/openshell-server/src/grpc/sandbox.rs +++ b/crates/openshell-server/src/grpc/sandbox.rs @@ -14,7 +14,10 @@ use crate::auth::workspace_authz::{ AuthorizedWorkspaceScope, MinWorkspaceRole, authorize_list_workspace_selector, authorize_sandbox_workspace, authorize_workspace_selector, }; -use crate::persistence::{ObjectLabels, ObjectType, WriteCondition, generate_name}; +use crate::pagination::Pagination; +use crate::persistence::{ + ObjectLabels, ObjectListQuery, ObjectType, WriteCondition, generate_name, +}; use futures::future; use openshell_core::net::set_tcp_nodelay_best_effort; use openshell_core::proto::datamodel::v1::ObjectMeta; @@ -66,7 +69,7 @@ use super::validation::{ validate_exec_request_fields, validate_no_reserved_provider_policy_keys, validate_policy_safety, validate_sandbox_governance_spec, validate_sandbox_spec, }; -use super::{MAX_PAGE_SIZE, MAX_PROVIDERS, MAX_ROUTABLE_NAME_LEN, clamp_limit}; +use super::{MAX_PROVIDERS, MAX_ROUTABLE_NAME_LEN}; use crate::persistence::current_time_ms; const TCP_FORWARD_CHUNK_SIZE: usize = 64 * 1024; @@ -696,8 +699,6 @@ pub(super) async fn handle_list_sandboxes( ) -> Result, Status> { let principal = super::extract_principal(&request)?; let request = request.into_inner(); - let limit = clamp_limit(request.limit, 100, MAX_PAGE_SIZE); - let scope = authorize_list_workspace_selector( &state.store, &state.admin_role, @@ -706,21 +707,11 @@ pub(super) async fn handle_list_sandboxes( MinWorkspaceRole::User, ) .await?; - let sandboxes: Vec = if matches!(scope, AuthorizedWorkspaceScope::AllWorkspaces) { - if request.label_selector.is_empty() { - state - .store - .list_all_messages(limit, request.offset) - .await - .map_err(|e| Status::internal(format!("list sandboxes failed: {e}")))? - } else { - crate::grpc::validation::validate_label_selector(&request.label_selector)?; - state - .store - .list_all_messages_with_selector(&request.label_selector, limit, request.offset) - .await - .map_err(|e| Status::internal(format!("list sandboxes failed: {e}")))? - } + if !request.label_selector.is_empty() { + crate::grpc::validation::validate_label_selector(&request.label_selector)?; + } + let workspace = if matches!(scope, AuthorizedWorkspaceScope::AllWorkspaces) { + None } else { let AuthorizedWorkspaceScope::Workspace(authz) = scope else { unreachable!("all-workspaces scope handled above") @@ -728,30 +719,35 @@ pub(super) async fn handle_list_sandboxes( let workspace = super::workspace::resolve_workspace(state.store.as_ref(), &authz.workspace) .await? .name; - if request.label_selector.is_empty() { - state - .store - .list_messages(&workspace, limit, request.offset) - .await - .map_err(|e| Status::internal(format!("list sandboxes failed: {e}")))? - } else { - crate::grpc::validation::validate_label_selector(&request.label_selector)?; - state - .store - .list_messages_with_selector( - &workspace, - &request.label_selector, - limit, - request.offset, - ) - .await - .map_err(|e| { - Status::internal(format!("list sandboxes with selector failed: {e}")) - })? - } + Some(workspace) }; - - Ok(Response::new(ListSandboxesResponse { sandboxes })) + let scope_fingerprint = workspace.as_deref().unwrap_or("*"); + let pagination = Pagination::new( + request.page_size, + &request.page_token, + "ListSandboxes", + &[scope_fingerprint, &request.label_selector], + )?; + let after = pagination.object_cursor()?; + let query = match (workspace.as_deref(), request.label_selector.as_str()) { + (None, "") => ObjectListQuery::AllWorkspaces, + (None, selector) => ObjectListQuery::AllWorkspacesSelector(selector), + (Some(workspace), "") => ObjectListQuery::Workspace(workspace), + (Some(workspace), selector) => ObjectListQuery::WorkspaceSelector { + workspace, + label_selector: selector, + }, + }; + let page = state + .store + .list_message_page::(query, after.as_ref(), pagination.page_size()) + .await + .map_err(|e| Status::internal(format!("list sandboxes failed: {e}")))?; + let next_page_token = pagination.next_object_token(page.next_cursor.as_ref()); + Ok(Response::new(ListSandboxesResponse { + sandboxes: page.messages, + next_page_token, + })) } pub(super) async fn handle_create_sandbox_template( @@ -882,7 +878,6 @@ pub(super) async fn handle_list_sandbox_templates( ) -> Result, Status> { let principal = super::extract_principal(&request)?; let request = request.into_inner(); - let limit = clamp_limit(request.limit, 100, MAX_PAGE_SIZE); let scope = authorize_list_workspace_selector( &state.store, &state.admin_role, @@ -891,25 +886,11 @@ pub(super) async fn handle_list_sandbox_templates( MinWorkspaceRole::User, ) .await?; - let templates = if matches!(scope, AuthorizedWorkspaceScope::AllWorkspaces) { - if request.label_selector.is_empty() { - state - .store - .list_all_messages::(limit, request.offset) - .await - .map_err(|e| Status::internal(format!("list sandbox templates failed: {e}")))? - } else { - crate::grpc::validation::validate_label_selector(&request.label_selector)?; - state - .store - .list_all_messages_with_selector::( - &request.label_selector, - limit, - request.offset, - ) - .await - .map_err(|e| Status::internal(format!("list sandbox templates failed: {e}")))? - } + if !request.label_selector.is_empty() { + crate::grpc::validation::validate_label_selector(&request.label_selector)?; + } + let workspace = if matches!(scope, AuthorizedWorkspaceScope::AllWorkspaces) { + None } else { let AuthorizedWorkspaceScope::Workspace(authz) = scope else { unreachable!("all-workspaces scope handled above") @@ -917,29 +898,35 @@ pub(super) async fn handle_list_sandbox_templates( let workspace = super::workspace::resolve_workspace(state.store.as_ref(), &authz.workspace) .await? .name; - if request.label_selector.is_empty() { - state - .store - .list_messages::(&workspace, limit, request.offset) - .await - .map_err(|e| Status::internal(format!("list sandbox templates failed: {e}")))? - } else { - crate::grpc::validation::validate_label_selector(&request.label_selector)?; - state - .store - .list_messages_with_selector::( - &workspace, - &request.label_selector, - limit, - request.offset, - ) - .await - .map_err(|e| { - Status::internal(format!("list sandbox templates with selector failed: {e}")) - })? - } + Some(workspace) + }; + let scope_fingerprint = workspace.as_deref().unwrap_or("*"); + let pagination = Pagination::new( + request.page_size, + &request.page_token, + "ListSandboxTemplates", + &[scope_fingerprint, &request.label_selector], + )?; + let after = pagination.object_cursor()?; + let query = match (workspace.as_deref(), request.label_selector.as_str()) { + (None, "") => ObjectListQuery::AllWorkspaces, + (None, selector) => ObjectListQuery::AllWorkspacesSelector(selector), + (Some(workspace), "") => ObjectListQuery::Workspace(workspace), + (Some(workspace), selector) => ObjectListQuery::WorkspaceSelector { + workspace, + label_selector: selector, + }, }; - Ok(Response::new(ListSandboxTemplatesResponse { templates })) + let page = state + .store + .list_message_page::(query, after.as_ref(), pagination.page_size()) + .await + .map_err(|e| Status::internal(format!("list sandbox templates failed: {e}")))?; + let next_page_token = pagination.next_object_token(page.next_cursor.as_ref()); + Ok(Response::new(ListSandboxTemplatesResponse { + templates: page.messages, + next_page_token, + })) } pub(super) async fn handle_delete_sandbox_template( @@ -4826,8 +4813,8 @@ mod tests { let listed = handle_list_sandbox_templates( &state, authed_request(ListSandboxTemplatesRequest { - limit: 100, - offset: 0, + page_size: 100, + page_token: String::new(), workspace_scope: Some(openshell_core::proto::workspace_selector( "default".to_string(), )), @@ -4912,8 +4899,8 @@ mod tests { let listed = handle_list_sandbox_templates( &state, authed_request(ListSandboxTemplatesRequest { - limit: 100, - offset: 0, + page_size: 100, + page_token: String::new(), workspace_scope: Some(openshell_core::proto::workspace_selector( "default".to_string(), )), @@ -4951,8 +4938,8 @@ mod tests { let listed = handle_list_sandbox_templates( &state, authed_request(ListSandboxTemplatesRequest { - limit: 100, - offset: 0, + page_size: 100, + page_token: String::new(), workspace_scope: Some(openshell_core::proto::workspace_selector( "default".to_string(), )), @@ -5000,8 +4987,8 @@ mod tests { let listed = handle_list_sandbox_templates( &state, authed_request(ListSandboxTemplatesRequest { - limit: 100, - offset: 0, + page_size: 100, + page_token: String::new(), workspace_scope: Some(openshell_core::proto::workspace_selector( "beta".to_string(), )), @@ -6399,8 +6386,8 @@ mod tests { let listed = handle_list_sandboxes( &state, authed_request(ListSandboxesRequest { - limit: 100, - offset: 0, + page_size: 100, + page_token: String::new(), label_selector: String::new(), workspace_scope: Some(openshell_core::proto::workspace_selector( "default".to_string(), @@ -6417,8 +6404,8 @@ mod tests { let listed = handle_list_sandboxes( &state, authed_request(ListSandboxesRequest { - limit: 100, - offset: 0, + page_size: 100, + page_token: String::new(), label_selector: String::new(), workspace_scope: Some(openshell_core::proto::workspace_selector( "beta".to_string(), @@ -6442,8 +6429,8 @@ mod tests { let listed = handle_list_sandboxes( &state, authed_request(ListSandboxesRequest { - limit: 100, - offset: 0, + page_size: 100, + page_token: String::new(), label_selector: String::new(), workspace_scope: Some(openshell_core::proto::workspace_selector( "default".to_string(), @@ -6487,8 +6474,8 @@ mod tests { let listed = handle_list_sandboxes( &state, authed_request(ListSandboxesRequest { - limit: 100, - offset: 0, + page_size: 100, + page_token: String::new(), label_selector: String::new(), workspace_scope: Some(openshell_core::proto::all_workspaces_selector()), }), @@ -6497,6 +6484,38 @@ mod tests { .unwrap() .into_inner(); assert_eq!(listed.sandboxes.len(), 2); + let first_page = handle_list_sandboxes( + &state, + authed_request(ListSandboxesRequest { + page_size: 1, + page_token: String::new(), + label_selector: String::new(), + workspace_scope: Some(openshell_core::proto::all_workspaces_selector()), + }), + ) + .await + .unwrap() + .into_inner(); + assert_eq!(first_page.sandboxes.len(), 1); + assert!(!first_page.next_page_token.is_empty()); + let second_page = handle_list_sandboxes( + &state, + authed_request(ListSandboxesRequest { + page_size: 100, + page_token: first_page.next_page_token, + label_selector: String::new(), + workspace_scope: Some(openshell_core::proto::all_workspaces_selector()), + }), + ) + .await + .unwrap() + .into_inner(); + assert_eq!(second_page.sandboxes.len(), 1); + assert!(second_page.next_page_token.is_empty()); + assert_ne!( + first_page.sandboxes[0].object_id(), + second_page.sandboxes[0].object_id() + ); } /// Non-members must receive `PERMISSION_DENIED` — never `NOT_FOUND` — when diff --git a/crates/openshell-server/src/grpc/service.rs b/crates/openshell-server/src/grpc/service.rs index 34c7d62d3f..a691ec0ecf 100644 --- a/crates/openshell-server/src/grpc/service.rs +++ b/crates/openshell-server/src/grpc/service.rs @@ -19,7 +19,8 @@ use crate::auth::workspace_authz::{ AuthorizedWorkspaceScope, MinWorkspaceRole, authorize_list_workspace_selector, authorize_workspace_selector, }; -use crate::persistence::{ObjectType, WriteCondition}; +use crate::pagination::Pagination; +use crate::persistence::{ObjectListQuery, ObjectType, WriteCondition}; use crate::service_routing; const MAX_SERVICE_NAME_LEN: usize = super::MAX_ROUTABLE_NAME_LEN; @@ -181,7 +182,6 @@ pub(super) async fn handle_list_services( validate_endpoint_name("sandbox", &req.sandbox, MAX_SANDBOX_NAME_LEN)?; } - let limit = super::clamp_limit(req.limit, 100, super::MAX_PAGE_SIZE); let scope = authorize_list_workspace_selector( &state.store, &state.admin_role, @@ -190,47 +190,59 @@ pub(super) async fn handle_list_services( MinWorkspaceRole::User, ) .await?; - let endpoints: Vec = - if matches!(scope, AuthorizedWorkspaceScope::AllWorkspaces) { - if !req.sandbox.is_empty() { - return Err(Status::invalid_argument( - "sandbox filter is not supported with all_workspaces", - )); - } - state.store.list_all_messages(limit, req.offset).await - } else { - let AuthorizedWorkspaceScope::Workspace(authz) = scope else { - unreachable!("all-workspaces scope handled above") - }; - let workspace = - super::workspace::resolve_workspace(state.store.as_ref(), &authz.workspace) - .await? - .name; - if req.sandbox.is_empty() { - state - .store - .list_messages(&workspace, limit, req.offset) - .await - } else { - state - .store - .list_messages_with_selector( - &workspace, - &format!("sandbox={}", req.sandbox), - limit, - req.offset, - ) - .await - } + let workspace = if matches!(scope, AuthorizedWorkspaceScope::AllWorkspaces) { + if !req.sandbox.is_empty() { + return Err(Status::invalid_argument( + "sandbox filter is not supported with all_workspaces", + )); } + None + } else { + let AuthorizedWorkspaceScope::Workspace(authz) = scope else { + unreachable!("all-workspaces scope handled above") + }; + let workspace = super::workspace::resolve_workspace(state.store.as_ref(), &authz.workspace) + .await? + .name; + Some(workspace) + }; + let scope_fingerprint = workspace.as_deref().unwrap_or("*"); + let pagination = Pagination::new( + req.page_size, + &req.page_token, + "ListServices", + &[&req.sandbox, scope_fingerprint], + )?; + let after = pagination.object_cursor()?; + let selector = (!req.sandbox.is_empty()).then(|| format!("sandbox={}", req.sandbox)); + let query = match (workspace.as_deref(), selector.as_deref()) { + (None, None) => ObjectListQuery::AllWorkspaces, + (Some(workspace), None) => ObjectListQuery::Workspace(workspace), + (Some(workspace), Some(selector)) => ObjectListQuery::WorkspaceSelector { + workspace, + label_selector: selector, + }, + (None, Some(_)) => { + return Err(Status::invalid_argument( + "sandbox filter cannot be combined with all_workspaces", + )); + } + }; + let page = state + .store + .list_message_page::(query, after.as_ref(), pagination.page_size()) + .await .map_err(|e| Status::internal(format!("list endpoints failed: {e}")))?; - - let services = endpoints + let services = page + .messages .into_iter() .map(|ep| service_endpoint_response(state, ep)) .collect(); - - Ok(Response::new(ListServicesResponse { services })) + let next_page_token = pagination.next_object_token(page.next_cursor.as_ref()); + Ok(Response::new(ListServicesResponse { + services, + next_page_token, + })) } pub(super) async fn handle_delete_service( @@ -429,8 +441,8 @@ mod tests { &state, authed_request(ListServicesRequest { sandbox: "my-sandbox".to_string(), - limit: 0, - offset: 0, + page_size: 0, + page_token: String::new(), workspace_scope: Some(openshell_core::proto::workspace_selector( "default".to_string(), )), @@ -493,8 +505,8 @@ mod tests { &state, authed_request(ListServicesRequest { sandbox: "my-sandbox".to_string(), - limit: 0, - offset: 0, + page_size: 0, + page_token: String::new(), workspace_scope: Some(openshell_core::proto::workspace_selector( "default".to_string(), )), @@ -563,8 +575,8 @@ mod tests { &state, authed_request(ListServicesRequest { sandbox: "my-sandbox".to_string(), - limit: 0, - offset: 0, + page_size: 0, + page_token: String::new(), workspace_scope: Some(openshell_core::proto::workspace_selector( "default".to_string(), )), @@ -772,8 +784,8 @@ mod tests { &state, authed_request(ListServicesRequest { sandbox: "my-sandbox".to_string(), - limit: 100, - offset: 0, + page_size: 100, + page_token: String::new(), workspace_scope: Some(openshell_core::proto::workspace_selector( "default".to_string(), )), @@ -792,8 +804,8 @@ mod tests { &state, authed_request(ListServicesRequest { sandbox: "my-sandbox".to_string(), - limit: 100, - offset: 0, + page_size: 100, + page_token: String::new(), workspace_scope: Some(openshell_core::proto::workspace_selector( "beta".to_string(), )), @@ -828,8 +840,8 @@ mod tests { &state, authed_request(ListServicesRequest { sandbox: "my-sandbox".to_string(), - limit: 100, - offset: 0, + page_size: 100, + page_token: String::new(), workspace_scope: Some(openshell_core::proto::workspace_selector( "default".to_string(), )), @@ -876,8 +888,8 @@ mod tests { &state, authed_request(ListServicesRequest { sandbox: String::new(), - limit: 100, - offset: 0, + page_size: 100, + page_token: String::new(), workspace_scope: Some(openshell_core::proto::all_workspaces_selector()), }), ) diff --git a/crates/openshell-server/src/grpc/workspace.rs b/crates/openshell-server/src/grpc/workspace.rs index 546934a6ed..c18819be52 100644 --- a/crates/openshell-server/src/grpc/workspace.rs +++ b/crates/openshell-server/src/grpc/workspace.rs @@ -23,15 +23,14 @@ use tonic::{Request, Response, Status}; use crate::ServerState; use crate::auth::principal::Principal; use crate::auth::workspace_authz::{AuthGrant, MinWorkspaceRole, authorize_workspace}; +use crate::pagination::Pagination; use crate::persistence::{ - DRAFT_CHUNK_OBJECT_TYPE, ObjectLabels, ObjectType, POLICY_OBJECT_TYPE, WriteCondition, - current_time_ms, + DRAFT_CHUNK_OBJECT_TYPE, ObjectLabels, ObjectListQuery, ObjectType, POLICY_OBJECT_TYPE, + WriteCondition, current_time_ms, }; use crate::storage_proto::{StoredProviderCredentialRefreshState, StoredProviderProfile}; use std::collections::HashMap; -use super::{MAX_PAGE_SIZE, clamp_limit}; - pub const WORKSPACE_OBJECT_TYPE: &str = "workspace"; pub const DEFAULT_WORKSPACE_NAME: &str = "default"; const MAX_WORKSPACE_MEMBERS: u32 = 1000; @@ -248,40 +247,42 @@ pub(super) async fn handle_list_workspaces( let principal = super::extract_principal(&request)?; let req = request.into_inner(); super::validation::validate_label_selector(&req.label_selector)?; - let limit = clamp_limit(req.limit, 100, MAX_PAGE_SIZE); let subject = membership_filter_subject(state, &principal)?; - let member_type = WorkspaceMember::object_type(); - let workspaces = match subject { - Some(subject) if req.label_selector.is_empty() => state - .store - .list_messages_with_membership::(member_type, subject, limit, req.offset) - .await - .map_err(|e| Status::internal(format!("list workspaces failed: {e}")))?, - Some(subject) => state - .store - .list_messages_with_membership_and_selector::( - member_type, - subject, - &req.label_selector, - limit, - req.offset, - ) - .await - .map_err(|e| Status::internal(format!("list workspaces failed: {e}")))?, - None if req.label_selector.is_empty() => state - .store - .list_messages("", limit, req.offset) - .await - .map_err(|e| Status::internal(format!("list workspaces failed: {e}")))?, - None => state - .store - .list_messages_with_selector("", &req.label_selector, limit, req.offset) - .await - .map_err(|e| Status::internal(format!("list workspaces failed: {e}")))?, + let pagination = Pagination::new( + req.page_size, + &req.page_token, + "ListWorkspaces", + &[&req.label_selector, subject.unwrap_or("")], + )?; + let after = pagination.object_cursor()?; + let query = match (subject, req.label_selector.as_str()) { + (Some(subject), "") => ObjectListQuery::Membership { + member_type, + member_name: subject, + }, + (Some(subject), selector) => ObjectListQuery::MembershipSelector { + member_type, + member_name: subject, + label_selector: selector, + }, + (None, "") => ObjectListQuery::Workspace(""), + (None, selector) => ObjectListQuery::WorkspaceSelector { + workspace: "", + label_selector: selector, + }, }; + let page = state + .store + .list_message_page::(query, after.as_ref(), pagination.page_size()) + .await + .map_err(|e| Status::internal(format!("list workspaces failed: {e}")))?; + let next_page_token = pagination.next_object_token(page.next_cursor.as_ref()); - Ok(Response::new(ListWorkspacesResponse { workspaces })) + Ok(Response::new(ListWorkspacesResponse { + workspaces: page.messages, + next_page_token, + })) } pub(super) async fn handle_delete_workspace( @@ -603,15 +604,28 @@ pub(super) async fn handle_list_workspace_members( .await? .name; - let limit = clamp_limit(req.limit, 100, MAX_PAGE_SIZE); - - let members: Vec = state + let pagination = Pagination::new( + req.page_size, + &req.page_token, + "ListWorkspaceMembers", + &[&req.workspace], + )?; + let after = pagination.object_cursor()?; + let page = state .store - .list_messages(&workspace, limit, req.offset) + .list_message_page::( + ObjectListQuery::Workspace(&workspace), + after.as_ref(), + pagination.page_size(), + ) .await .map_err(|e| Status::internal(format!("list workspace members failed: {e}")))?; + let next_page_token = pagination.next_object_token(page.next_cursor.as_ref()); - Ok(Response::new(ListWorkspaceMembersResponse { members })) + Ok(Response::new(ListWorkspaceMembersResponse { + members: page.messages, + next_page_token, + })) } #[cfg(test)] @@ -1044,8 +1058,8 @@ mod tests { &state, authed_request(ListWorkspaceMembersRequest { workspace: "default".to_string(), - limit: 100, - offset: 0, + page_size: 100, + page_token: String::new(), }), ) .await @@ -1086,8 +1100,8 @@ mod tests { &state, authed_request(ListWorkspaceMembersRequest { workspace: "default".to_string(), - limit: 100, - offset: 0, + page_size: 100, + page_token: String::new(), }), ) .await @@ -1166,8 +1180,8 @@ mod tests { &state, authed_request(ListWorkspaceMembersRequest { workspace: "cleanup-test".to_string(), - limit: 100, - offset: 0, + page_size: 100, + page_token: String::new(), }), ) .await diff --git a/crates/openshell-server/src/lib.rs b/crates/openshell-server/src/lib.rs index 50c4f98696..6bea00e6a0 100644 --- a/crates/openshell-server/src/lib.rs +++ b/crates/openshell-server/src/lib.rs @@ -26,6 +26,7 @@ mod http; mod middleware; mod multiplex; mod otel_tracing; +mod pagination; mod persistence; pub(crate) mod policy_store; mod provider_profile_sources; diff --git a/crates/openshell-server/src/pagination.rs b/crates/openshell-server/src/pagination.rs new file mode 100644 index 0000000000..66dd1d092e --- /dev/null +++ b/crates/openshell-server/src/pagination.rs @@ -0,0 +1,245 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Shared AIP-158 pagination validation and opaque continuation-token codec. + +use base64::{Engine as _, engine::general_purpose::URL_SAFE_NO_PAD}; +use openshell_core::proto::pagination::v1::{ + ObjectCursor as ProtoObjectCursor, PageToken, PolicyCursor, ProfileCursor, page_token::Cursor, +}; +use prost::Message; +use sha2::{Digest, Sha256}; +use tonic::Status; + +use crate::persistence::ObjectCursor; + +const TOKEN_VERSION: u32 = 1; +pub const DEFAULT_PAGE_SIZE: u32 = 100; +pub const MAX_PAGE_SIZE: u32 = 1000; + +/// Validated pagination state for one list request. +#[derive(Debug)] +pub struct Pagination { + page_size: u32, + method: &'static str, + request_fingerprint: Vec, + cursor: Option, +} + +impl Pagination { + pub fn new( + page_size: i32, + page_token: &str, + method: &'static str, + request_parameters: &[&str], + ) -> Result { + if page_size < 0 { + return Err(Status::invalid_argument("page_size must not be negative")); + } + let page_size = if page_size == 0 { + DEFAULT_PAGE_SIZE + } else { + u32::try_from(page_size) + .expect("a positive i32 always fits in u32") + .min(MAX_PAGE_SIZE) + }; + let request_fingerprint = fingerprint(request_parameters); + let cursor = if page_token.is_empty() { + None + } else { + let bytes = URL_SAFE_NO_PAD + .decode(page_token) + .map_err(|_| Status::invalid_argument("page_token is malformed"))?; + let token = PageToken::decode(bytes.as_slice()) + .map_err(|_| Status::invalid_argument("page_token is malformed"))?; + if token.version != TOKEN_VERSION + || token.method != method + || token.request_fingerprint != request_fingerprint + { + return Err(Status::invalid_argument( + "page_token does not match this list request", + )); + } + token.cursor + }; + + Ok(Self { + page_size, + method, + request_fingerprint, + cursor, + }) + } + + pub fn page_size(&self) -> u32 { + self.page_size + } + + pub fn object_cursor(&self) -> Result, Status> { + match &self.cursor { + None => Ok(None), + Some(Cursor::Object(cursor)) => Ok(Some(ObjectCursor { + created_at_ms: cursor.created_at_ms, + name: cursor.name.clone(), + workspace: cursor.workspace.clone(), + id: cursor.id.clone(), + })), + Some(_) => Err(Status::invalid_argument( + "page_token has the wrong cursor type", + )), + } + } + + pub fn policy_cursor(&self) -> Result, Status> { + match &self.cursor { + None => Ok(None), + Some(Cursor::Policy(cursor)) => Ok(Some(cursor.version)), + Some(_) => Err(Status::invalid_argument( + "page_token has the wrong cursor type", + )), + } + } + + pub fn profile_cursor(&self) -> Result, Status> { + match &self.cursor { + None => Ok(None), + Some(Cursor::Profile(cursor)) => Ok(Some(&cursor.key)), + Some(_) => Err(Status::invalid_argument( + "page_token has the wrong cursor type", + )), + } + } + + pub fn next_object_token(&self, cursor: Option<&ObjectCursor>) -> String { + cursor.map_or_else(String::new, |cursor| { + self.encode(Cursor::Object(ProtoObjectCursor { + created_at_ms: cursor.created_at_ms, + name: cursor.name.clone(), + workspace: cursor.workspace.clone(), + id: cursor.id.clone(), + })) + }) + } + + pub fn next_policy_token(&self, version: Option) -> String { + version.map_or_else(String::new, |version| { + self.encode(Cursor::Policy(PolicyCursor { version })) + }) + } + + pub fn next_profile_token(&self, key: Option<&str>) -> String { + key.map_or_else(String::new, |key| { + self.encode(Cursor::Profile(ProfileCursor { + key: key.to_string(), + })) + }) + } + + fn encode(&self, cursor: Cursor) -> String { + URL_SAFE_NO_PAD.encode( + PageToken { + version: TOKEN_VERSION, + method: self.method.to_string(), + request_fingerprint: self.request_fingerprint.clone(), + cursor: Some(cursor), + } + .encode_to_vec(), + ) + } +} + +fn fingerprint(parameters: &[&str]) -> Vec { + let mut hasher = Sha256::new(); + for parameter in parameters { + hasher.update( + u64::try_from(parameter.len()) + .unwrap_or(u64::MAX) + .to_le_bytes(), + ); + hasher.update(parameter.as_bytes()); + } + hasher.finalize().to_vec() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn page_size_defaults_clamps_and_rejects_negative_values() { + assert_eq!( + Pagination::new(0, "", "list", &[]).unwrap().page_size(), + 100 + ); + assert_eq!( + Pagination::new(1500, "", "list", &[]).unwrap().page_size(), + 1000 + ); + assert_eq!( + Pagination::new(-1, "", "list", &[]).unwrap_err().code(), + tonic::Code::InvalidArgument + ); + } + + #[test] + fn object_token_round_trips_and_allows_page_size_changes() { + let first = Pagination::new(10, "", "sandboxes", &["default", "env=prod"]).unwrap(); + let cursor = ObjectCursor { + created_at_ms: 42, + name: "sandbox".into(), + workspace: "default".into(), + id: "id".into(), + }; + let token = first.next_object_token(Some(&cursor)); + assert!( + token + .bytes() + .all(|byte| byte.is_ascii_alphanumeric() || matches!(byte, b'-' | b'_')) + ); + + let next = Pagination::new(20, &token, "sandboxes", &["default", "env=prod"]) + .unwrap() + .object_cursor() + .unwrap() + .unwrap(); + assert_eq!(next.id, cursor.id); + assert_eq!(next.created_at_ms, cursor.created_at_ms); + } + + #[test] + fn token_rejects_method_filter_and_cursor_mismatches() { + let page = Pagination::new(10, "", "sandboxes", &["default"]).unwrap(); + let token = page.next_profile_token(Some("profile")); + + assert!(Pagination::new(10, &token, "providers", &["default"]).is_err()); + assert!(Pagination::new(10, &token, "sandboxes", &["other"]).is_err()); + assert!( + Pagination::new(10, &token, "sandboxes", &["default"]) + .unwrap() + .object_cursor() + .is_err() + ); + } + + #[test] + fn token_rejects_malformed_and_unsupported_versions() { + let malformed = Pagination::new(10, "not+base64", "sandboxes", &["default"]) + .expect_err("malformed tokens must be rejected"); + assert_eq!(malformed.code(), tonic::Code::InvalidArgument); + + let unsupported = URL_SAFE_NO_PAD.encode( + PageToken { + version: TOKEN_VERSION + 1, + method: "sandboxes".to_string(), + request_fingerprint: fingerprint(&["default"]), + cursor: Some(Cursor::Profile(ProfileCursor { + key: "profile".to_string(), + })), + } + .encode_to_vec(), + ); + let error = Pagination::new(10, &unsupported, "sandboxes", &["default"]) + .expect_err("unsupported token versions must be rejected"); + assert_eq!(error.code(), tonic::Code::InvalidArgument); + } +} diff --git a/crates/openshell-server/src/persistence/mod.rs b/crates/openshell-server/src/persistence/mod.rs index 066ce6cc3a..e22f5f99b9 100644 --- a/crates/openshell-server/src/persistence/mod.rs +++ b/crates/openshell-server/src/persistence/mod.rs @@ -45,6 +45,8 @@ pub enum PersistenceError { Decode(String), #[error("encode error: {0}")] Encode(String), + #[error("pagination error: {0}")] + Pagination(String), #[error("unique violation{constraint_msg}")] UniqueViolation { constraint: Option, @@ -109,9 +111,8 @@ pub struct ObjectRecord { /// Stable position in the global object-listing order. /// -/// Keyset consumers must use the matching store method for the order encoded -/// here: workspace-scoped lists use `created_at_ms`, `name`, and `id`; global -/// lists additionally include `workspace`. +/// Keyset consumers must use the matching store method for the total +/// `(created_at_ms, name, workspace, id)` order encoded here. #[derive(Debug, Clone)] pub struct ObjectCursor { pub created_at_ms: i64, @@ -131,6 +132,44 @@ impl From<&ObjectRecord> for ObjectCursor { } } +/// Filters supported by the shared object-store keyset pager. +#[derive(Debug, Clone, Copy)] +pub enum ObjectListQuery<'a> { + Workspace(&'a str), + AllWorkspaces, + Scope(&'a str), + WorkspaceSelector { + workspace: &'a str, + label_selector: &'a str, + }, + AllWorkspacesSelector(&'a str), + Membership { + member_type: &'a str, + member_name: &'a str, + }, + MembershipSelector { + member_type: &'a str, + member_name: &'a str, + label_selector: &'a str, + }, +} + +/// One keyset page of raw object records. +#[derive(Debug)] +pub struct ObjectPage { + pub records: Vec, + pub next_cursor: Option, +} + +/// One keyset page of decoded protobuf messages. +#[derive(Debug)] +pub struct MessagePage { + pub messages: Vec, + pub next_cursor: Option, +} + +const FULL_SCAN_PAGE_SIZE: u32 = 1000; + /// Write condition for compare-and-swap operations. #[derive(Debug, Clone, Copy)] pub enum WriteCondition { @@ -619,6 +658,109 @@ impl Store { store_dispatch_traced!(self.list_by_type_after(object_type, after, limit)) } + /// Return one keyset page for an explicit query shape. + /// + /// The page is ordered by the immutable total key + /// `(created_at_ms, name, workspace, id)`. `next_cursor` is present only + /// when another page exists. + pub async fn list_object_page( + &self, + object_type: &str, + query: ObjectListQuery<'_>, + after: Option<&ObjectCursor>, + page_size: u32, + ) -> PersistenceResult { + if page_size == 0 { + return Err(PersistenceError::Pagination( + "page size must be greater than zero".into(), + )); + } + let fetch_size = page_size.checked_add(1).ok_or_else(|| { + PersistenceError::Pagination("page size overflow while probing next page".into()) + })?; + let mut records = match self { + Self::Postgres(store) => { + store + .list_object_page(object_type, query, after, fetch_size) + .await + } + Self::Sqlite(store) => { + store + .list_object_page(object_type, query, after, fetch_size) + .await + } + }?; + let has_more = records.len() + > usize::try_from(page_size) + .map_err(|_| PersistenceError::Pagination("page size does not fit usize".into()))?; + if has_more { + records.truncate(usize::try_from(page_size).map_err(|_| { + PersistenceError::Pagination("page size does not fit usize".into()) + })?); + } + let next_cursor = if has_more { + records.last().map(ObjectCursor::from) + } else { + None + }; + Ok(ObjectPage { + records, + next_cursor, + }) + } + + /// Return one decoded keyset page and hydrate every resource version. + pub async fn list_message_page( + &self, + query: ObjectListQuery<'_>, + after: Option<&ObjectCursor>, + page_size: u32, + ) -> PersistenceResult> { + let page = self + .list_object_page(T::object_type(), query, after, page_size) + .await?; + Ok(MessagePage { + messages: page + .records + .into_iter() + .map(decode_record) + .collect::>>()?, + next_cursor: page.next_cursor, + }) + } + + /// Exhaust every keyset page and return all matching raw records. + pub async fn collect_records( + &self, + object_type: &str, + query: ObjectListQuery<'_>, + ) -> PersistenceResult> { + let mut records = Vec::new(); + let mut cursor = None; + loop { + let page = self + .list_object_page(object_type, query, cursor.as_ref(), FULL_SCAN_PAGE_SIZE) + .await?; + records.extend(page.records); + let Some(next_cursor) = page.next_cursor else { + return Ok(records); + }; + cursor = Some(next_cursor); + } + } + + /// Exhaust every keyset page and return all matching decoded messages. + /// + /// Database and protobuf decode failures abort the operation; no partial + /// result is returned. + pub async fn collect_messages( + &self, + query: ObjectListQuery<'_>, + ) -> PersistenceResult> { + let records = self.collect_records(T::object_type(), query).await?; + records.into_iter().map(decode_record).collect() + } + /// List objects by type and application-owned scope. /// /// Workspace filtering is intentionally omitted: scope values are sandbox diff --git a/crates/openshell-server/src/persistence/postgres.rs b/crates/openshell-server/src/persistence/postgres.rs index 449f8f6df3..3fa54151b5 100644 --- a/crates/openshell-server/src/persistence/postgres.rs +++ b/crates/openshell-server/src/persistence/postgres.rs @@ -2,8 +2,9 @@ // SPDX-License-Identifier: Apache-2.0 use super::{ - DraftChunkRecord, ObjectCursor, ObjectRecord, PersistenceError, PersistenceResult, - PolicyRecord, WriteCondition, WriteResult, current_time_ms, map_db_error, map_migrate_error, + DraftChunkRecord, ObjectCursor, ObjectListQuery, ObjectRecord, PersistenceError, + PersistenceResult, PolicyRecord, WriteCondition, WriteResult, current_time_ms, map_db_error, + map_migrate_error, }; use crate::policy_store::{ AtomicPolicyRevisionWrite, draft_chunk_payload_from_record, draft_chunk_record_from_parts, @@ -625,6 +626,103 @@ LIMIT $2 OFFSET $3 Ok(rows.into_iter().map(row_to_object_record).collect()) } + pub async fn list_object_page( + &self, + object_type: &str, + query: ObjectListQuery<'_>, + after: Option<&ObjectCursor>, + limit: u32, + ) -> PersistenceResult> { + use super::parse_label_selector; + + let mut sql = QueryBuilder::::new( + "SELECT o.object_type, o.id, o.name, o.workspace, o.payload, \ + o.created_at_ms, o.updated_at_ms, o.labels, o.resource_version \ + FROM objects o WHERE o.object_type = ", + ); + sql.push_bind(object_type); + + match query { + ObjectListQuery::Workspace(workspace) => { + sql.push(" AND o.workspace = ").push_bind(workspace); + } + ObjectListQuery::AllWorkspaces => {} + ObjectListQuery::Scope(scope) => { + sql.push(" AND o.scope = ").push_bind(scope); + } + ObjectListQuery::WorkspaceSelector { + workspace, + label_selector, + } => { + let labels = serde_json::to_value(parse_label_selector(label_selector)?) + .map_err(|e| PersistenceError::Encode(e.to_string()))?; + sql.push(" AND o.workspace = ") + .push_bind(workspace) + .push(" AND o.labels @> ") + .push_bind(labels); + } + ObjectListQuery::AllWorkspacesSelector(label_selector) => { + let labels = serde_json::to_value(parse_label_selector(label_selector)?) + .map_err(|e| PersistenceError::Encode(e.to_string()))?; + sql.push(" AND o.labels @> ").push_bind(labels); + } + ObjectListQuery::Membership { + member_type, + member_name, + } => { + sql.push( + " AND o.workspace = '' AND EXISTS (SELECT 1 FROM objects m \ + WHERE m.object_type = ", + ) + .push_bind(member_type) + .push(" AND m.workspace = o.name AND m.name = ") + .push_bind(member_name) + .push(")"); + } + ObjectListQuery::MembershipSelector { + member_type, + member_name, + label_selector, + } => { + let labels = serde_json::to_value(parse_label_selector(label_selector)?) + .map_err(|e| PersistenceError::Encode(e.to_string()))?; + sql.push( + " AND o.workspace = '' AND EXISTS (SELECT 1 FROM objects m \ + WHERE m.object_type = ", + ) + .push_bind(member_type) + .push(" AND m.workspace = o.name AND m.name = ") + .push_bind(member_name) + .push(") AND o.labels @> ") + .push_bind(labels); + } + } + + if let Some(cursor) = after { + sql.push(" AND (o.created_at_ms, COALESCE(o.name, ''), o.workspace, o.id) > (") + .push_bind(cursor.created_at_ms) + .push(", ") + .push_bind(&cursor.name) + .push(", ") + .push_bind(&cursor.workspace) + .push(", ") + .push_bind(&cursor.id) + .push(")"); + } + sql.push( + " ORDER BY o.created_at_ms ASC, COALESCE(o.name, '') ASC, \ + o.workspace ASC, o.id ASC LIMIT ", + ) + .push_bind(i64::from(limit)); + + let rows = sql + .build() + .fetch_all(&self.pool) + .await + .map_err(|e| map_db_error(&e))?; + Ok(rows.into_iter().map(row_to_object_record).collect()) + } + pub async fn list_with_membership( &self, object_type: &str, @@ -1038,6 +1136,32 @@ LIMIT $3 OFFSET $4 rows.into_iter().map(row_to_policy_record).collect() } + pub async fn list_policies_before( + &self, + sandbox_id: &str, + limit: u32, + before_version: Option, + ) -> PersistenceResult> { + let rows = sqlx::query( + r" +SELECT id, scope, version, status, payload, created_at_ms +FROM objects +WHERE object_type = $1 AND scope = $2 AND ($3::BIGINT IS NULL OR version < $3) +ORDER BY version DESC +LIMIT $4 +", + ) + .bind(POLICY_OBJECT_TYPE) + .bind(sandbox_id) + .bind(before_version) + .bind(i64::from(limit)) + .fetch_all(&self.pool) + .await + .map_err(|e| map_db_error(&e))?; + + rows.into_iter().map(row_to_policy_record).collect() + } + pub async fn update_policy_status( &self, sandbox_id: &str, diff --git a/crates/openshell-server/src/persistence/sqlite.rs b/crates/openshell-server/src/persistence/sqlite.rs index 5497b96c22..4c274386ca 100644 --- a/crates/openshell-server/src/persistence/sqlite.rs +++ b/crates/openshell-server/src/persistence/sqlite.rs @@ -2,8 +2,9 @@ // SPDX-License-Identifier: Apache-2.0 use super::{ - DraftChunkRecord, ObjectCursor, ObjectRecord, PersistenceError, PersistenceResult, - PolicyRecord, WriteCondition, WriteResult, current_time_ms, map_db_error, map_migrate_error, + DraftChunkRecord, ObjectCursor, ObjectListQuery, ObjectRecord, PersistenceError, + PersistenceResult, PolicyRecord, WriteCondition, WriteResult, current_time_ms, map_db_error, + map_migrate_error, }; use crate::policy_store::{ AtomicPolicyRevisionWrite, draft_chunk_payload_from_record, draft_chunk_record_from_parts, @@ -42,6 +43,27 @@ pub struct SqliteStore { in_memory_keepalive: Option>>>, } +fn push_label_selector( + sql: &mut QueryBuilder, + label_selector: &str, +) -> PersistenceResult<()> { + let mut labels: Vec<_> = super::parse_label_selector(label_selector)? + .into_iter() + .collect(); + labels.sort_unstable_by(|left, right| left.0.cmp(&right.0)); + for (key, value) in labels { + let escaped_key = key + .replace('\\', "\\\\") + .replace('"', "\\\"") + .replace('\'', "''"); + sql.push(format!( + " AND json_extract(o.labels, '$.\"{escaped_key}\"') = " + )) + .push_bind(value); + } + Ok(()) +} + #[cfg(test)] pub(super) async fn replace_pool_connection(store: &SqliteStore) -> PersistenceResult<()> { let connection = store.pool.acquire().await.map_err(|e| map_db_error(&e))?; @@ -743,6 +765,93 @@ LIMIT ?2 Ok(rows.into_iter().map(row_to_object_record).collect()) } + pub async fn list_object_page( + &self, + object_type: &str, + query: ObjectListQuery<'_>, + after: Option<&ObjectCursor>, + limit: u32, + ) -> PersistenceResult> { + let mut sql = QueryBuilder::::new( + "SELECT o.object_type, o.id, o.name, o.workspace, o.payload, \ + o.created_at_ms, o.updated_at_ms, o.labels, o.resource_version \ + FROM objects o WHERE o.object_type = ", + ); + sql.push_bind(object_type); + + match query { + ObjectListQuery::Workspace(workspace) => { + sql.push(" AND o.workspace = ").push_bind(workspace); + } + ObjectListQuery::AllWorkspaces => {} + ObjectListQuery::Scope(scope) => { + sql.push(" AND o.scope = ").push_bind(scope); + } + ObjectListQuery::WorkspaceSelector { + workspace, + label_selector, + } => { + sql.push(" AND o.workspace = ").push_bind(workspace); + push_label_selector(&mut sql, label_selector)?; + } + ObjectListQuery::AllWorkspacesSelector(label_selector) => { + push_label_selector(&mut sql, label_selector)?; + } + ObjectListQuery::Membership { + member_type, + member_name, + } => { + sql.push( + " AND o.workspace = '' AND EXISTS (SELECT 1 FROM objects m \ + WHERE m.object_type = ", + ) + .push_bind(member_type) + .push(" AND m.workspace = o.name AND m.name = ") + .push_bind(member_name) + .push(")"); + } + ObjectListQuery::MembershipSelector { + member_type, + member_name, + label_selector, + } => { + sql.push( + " AND o.workspace = '' AND EXISTS (SELECT 1 FROM objects m \ + WHERE m.object_type = ", + ) + .push_bind(member_type) + .push(" AND m.workspace = o.name AND m.name = ") + .push_bind(member_name) + .push(")"); + push_label_selector(&mut sql, label_selector)?; + } + } + + if let Some(cursor) = after { + sql.push(" AND (o.created_at_ms, COALESCE(o.name, ''), o.workspace, o.id) > (") + .push_bind(cursor.created_at_ms) + .push(", ") + .push_bind(&cursor.name) + .push(", ") + .push_bind(&cursor.workspace) + .push(", ") + .push_bind(&cursor.id) + .push(")"); + } + sql.push( + " ORDER BY o.created_at_ms ASC, COALESCE(o.name, '') ASC, \ + o.workspace ASC, o.id ASC LIMIT ", + ) + .push_bind(i64::from(limit)); + + let rows = sql + .build() + .fetch_all(&self.pool) + .await + .map_err(|e| map_db_error(&e))?; + Ok(rows.into_iter().map(row_to_object_record).collect()) + } + pub async fn list_with_membership( &self, object_type: &str, @@ -884,27 +993,24 @@ LIMIT ?3 OFFSET ?4 limit: u32, offset: u32, ) -> PersistenceResult> { - use super::parse_label_selector; - - let required_labels = parse_label_selector(label_selector)?; - let all_records = self.list(object_type, workspace, u32::MAX, 0).await?; - - let filtered: Vec = all_records - .into_iter() - .filter(|record| { - let labels_json = record.labels.as_deref().unwrap_or("{}"); - let labels: std::collections::HashMap = - serde_json::from_str(labels_json).unwrap_or_default(); - - required_labels - .iter() - .all(|(key, value)| labels.get(key).is_some_and(|v| v == value)) - }) - .skip(offset as usize) - .take(limit as usize) - .collect(); - - Ok(filtered) + let mut sql = QueryBuilder::::new( + r#"SELECT o."object_type", o."id", o."name", o."workspace", o."payload", o."created_at_ms", o."updated_at_ms", o."labels", o."resource_version" +FROM "objects" o +WHERE o."object_type" = "#, + ); + sql.push_bind(object_type).push(" AND o.\"workspace\" = "); + sql.push_bind(workspace); + push_label_selector(&mut sql, label_selector)?; + sql.push(" ORDER BY o.\"created_at_ms\" ASC, o.\"name\" ASC LIMIT ") + .push_bind(i64::from(limit)) + .push(" OFFSET ") + .push_bind(i64::from(offset)); + let rows = sql + .build() + .fetch_all(&self.pool) + .await + .map_err(|e| map_db_error(&e))?; + Ok(rows.into_iter().map(row_to_object_record).collect()) } pub async fn list_all_with_selector( @@ -914,27 +1020,23 @@ LIMIT ?3 OFFSET ?4 limit: u32, offset: u32, ) -> PersistenceResult> { - use super::parse_label_selector; - - let required_labels = parse_label_selector(label_selector)?; - let all_records = self.list_by_type(object_type, u32::MAX, 0).await?; - - let filtered: Vec = all_records - .into_iter() - .filter(|record| { - let labels_json = record.labels.as_deref().unwrap_or("{}"); - let labels: std::collections::HashMap = - serde_json::from_str(labels_json).unwrap_or_default(); - - required_labels - .iter() - .all(|(key, value)| labels.get(key).is_some_and(|v| v == value)) - }) - .skip(offset as usize) - .take(limit as usize) - .collect(); - - Ok(filtered) + let mut sql = QueryBuilder::::new( + r#"SELECT o."object_type", o."id", o."name", o."workspace", o."payload", o."created_at_ms", o."updated_at_ms", o."labels", o."resource_version" +FROM "objects" o +WHERE o."object_type" = "#, + ); + sql.push_bind(object_type); + push_label_selector(&mut sql, label_selector)?; + sql.push(" ORDER BY o.\"created_at_ms\" ASC, o.\"name\" ASC LIMIT ") + .push_bind(i64::from(limit)) + .push(" OFFSET ") + .push_bind(i64::from(offset)); + let rows = sql + .build() + .fetch_all(&self.pool) + .await + .map_err(|e| map_db_error(&e))?; + Ok(rows.into_iter().map(row_to_object_record).collect()) } pub async fn put_policy_revision( &self, @@ -1176,6 +1278,32 @@ LIMIT ?3 OFFSET ?4 rows.into_iter().map(row_to_policy_record).collect() } + pub async fn list_policies_before( + &self, + sandbox_id: &str, + limit: u32, + before_version: Option, + ) -> PersistenceResult> { + let rows = sqlx::query( + r#" +SELECT "id", "scope", "version", "status", "payload", "created_at_ms" +FROM "objects" +WHERE "object_type" = ?1 AND "scope" = ?2 AND (?3 IS NULL OR "version" < ?3) +ORDER BY "version" DESC +LIMIT ?4 +"#, + ) + .bind(POLICY_OBJECT_TYPE) + .bind(sandbox_id) + .bind(before_version) + .bind(i64::from(limit)) + .fetch_all(&self.pool) + .await + .map_err(|e| map_db_error(&e))?; + + rows.into_iter().map(row_to_policy_record).collect() + } + pub async fn update_policy_status( &self, sandbox_id: &str, diff --git a/crates/openshell-server/src/persistence/tests.rs b/crates/openshell-server/src/persistence/tests.rs index 169fa81538..a292926270 100644 --- a/crates/openshell-server/src/persistence/tests.rs +++ b/crates/openshell-server/src/persistence/tests.rs @@ -1,7 +1,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -use super::{ObjectType, PersistenceError, Store, generate_name, test_store}; +use super::{ObjectListQuery, ObjectType, PersistenceError, Store, generate_name, test_store}; use crate::policy_store::{AtomicPolicyRevisionWrite, PolicyStoreExt}; use openshell_core::proto::datamodel::v1::ObjectMeta as ProtoObjectMeta; use openshell_core::proto::{ObjectForTest, Sandbox, SandboxPolicy, SandboxSpec}; @@ -124,6 +124,32 @@ async fn sqlite_put_get_round_trip() { assert_eq!(record.payload, b"payload"); } +#[tokio::test] +async fn collect_records_exhausts_multiple_keyset_pages_exactly_once() { + let store = test_store().await; + let expected = 1005_usize; + for index in 0..expected { + let id = format!("collect-{index:04}"); + let name = format!("sandbox-{index:04}"); + store + .put("sandbox", &id, &name, "collect-test", b"payload", None) + .await + .unwrap(); + } + + let records = store + .collect_records("sandbox", ObjectListQuery::Workspace("collect-test")) + .await + .unwrap(); + let ids = records + .iter() + .map(|record| record.id.as_str()) + .collect::>(); + + assert_eq!(records.len(), expected); + assert_eq!(ids.len(), expected, "every record is visited exactly once"); +} + #[tokio::test] async fn sqlite_connect_runs_embedded_migrations() { let store = test_store().await; @@ -182,6 +208,22 @@ fn embedded_migrators_include_inference_route_removal() { } } +#[test] +fn embedded_migrators_include_pagination_indexes() { + for (backend, migration) in [ + ("sqlite", super::sqlite::embedded_migration_sql(8)), + ("postgres", super::postgres::embedded_migration_sql(8)), + ] { + let sql = + migration.unwrap_or_else(|| panic!("{backend} migrator is missing migration 008")); + assert!( + sql.contains("objects_workspace_page_idx") + && sql.contains("objects_all_workspaces_page_idx"), + "{backend} migration 008 must add both keyset pagination indexes" + ); + } +} + #[tokio::test] async fn sqlite_in_memory_store_survives_pool_connection_replacement() { for url in ["sqlite::memory:", "sqlite://?mode=memory"] { diff --git a/crates/openshell-server/src/policy_store.rs b/crates/openshell-server/src/policy_store.rs index 8b1b6df335..8538e72205 100644 --- a/crates/openshell-server/src/policy_store.rs +++ b/crates/openshell-server/src/policy_store.rs @@ -125,6 +125,7 @@ pub trait PolicyStoreExt { version: i64, ) -> PersistenceResult>; + #[allow(dead_code)] async fn list_policies( &self, sandbox_id: &str, @@ -132,6 +133,13 @@ pub trait PolicyStoreExt { offset: u32, ) -> PersistenceResult>; + async fn list_policies_before( + &self, + sandbox_id: &str, + limit: u32, + before_version: Option, + ) -> PersistenceResult>; + async fn update_policy_status( &self, sandbox_id: &str, @@ -282,6 +290,26 @@ impl PolicyStoreExt for Store { } } + async fn list_policies_before( + &self, + sandbox_id: &str, + limit: u32, + before_version: Option, + ) -> PersistenceResult> { + match self { + Self::Postgres(store) => { + store + .list_policies_before(sandbox_id, limit, before_version) + .await + } + Self::Sqlite(store) => { + store + .list_policies_before(sandbox_id, limit, before_version) + .await + } + } + } + async fn update_policy_status( &self, sandbox_id: &str, diff --git a/crates/openshell-server/src/provider_profile_sources.rs b/crates/openshell-server/src/provider_profile_sources.rs index cea7faf31b..a62d9dc201 100644 --- a/crates/openshell-server/src/provider_profile_sources.rs +++ b/crates/openshell-server/src/provider_profile_sources.rs @@ -23,7 +23,7 @@ use sha2::{Digest, Sha256}; use tonic::Status; use tracing::debug; -use crate::persistence::{ObjectType, Store}; +use crate::persistence::{ObjectListQuery, ObjectType, Store}; use crate::storage_proto::StoredProviderProfile; const BUILTIN_SOURCE_ID: &str = "builtin"; @@ -130,8 +130,10 @@ impl ProviderProfileSource for UserProviderProfileSource { let mut hasher = Sha256::new(); hasher.update(b"openshell-user-provider-profile-source-v1"); - let platform_stored: Vec = - store.list_messages("", 10_000, 0).await.map_err(|e| { + let platform_stored: Vec = store + .collect_messages(ObjectListQuery::Workspace("")) + .await + .map_err(|e| { Status::internal(format!("list platform provider profiles failed: {e}")) })?; for stored in platform_stored { @@ -150,7 +152,7 @@ impl ProviderProfileSource for UserProviderProfileSource { if !workspace.is_empty() { let ws_stored: Vec = store - .list_messages(workspace, 10_000, 0) + .collect_messages(ObjectListQuery::Workspace(workspace)) .await .map_err(|e| { Status::internal(format!("list workspace provider profiles failed: {e}")) diff --git a/crates/openshell-server/src/provider_refresh.rs b/crates/openshell-server/src/provider_refresh.rs index e8386abcb8..f322be0dd0 100644 --- a/crates/openshell-server/src/provider_refresh.rs +++ b/crates/openshell-server/src/provider_refresh.rs @@ -6,13 +6,15 @@ #![allow(clippy::result_large_err)] use crate::credentials::RefreshMaterialScope; -use crate::persistence::{ObjectType, PersistenceError, Store, WriteCondition, current_time_ms}; +use crate::persistence::{ + ObjectListQuery, ObjectType, PersistenceError, Store, WriteCondition, current_time_ms, +}; use openshell_core::ObjectWorkspace; use openshell_core::proto::{ CredentialHandle, Provider, ProviderCredentialRefreshRecoveryAction, ProviderCredentialRefreshStatus, ProviderCredentialRefreshStrategy, }; -use openshell_core::{ObjectId, ObjectName, SetResourceVersion}; +use openshell_core::{ObjectId, ObjectName}; use prost::Message; use serde::{Deserialize, Serialize}; use std::collections::HashMap; @@ -26,7 +28,6 @@ const DEFAULT_REFRESH_BEFORE_SECONDS: i64 = 300; const DEFAULT_MAX_LIFETIME_SECONDS: i64 = 3600; const REFRESH_ERROR_RETRY_SECONDS: i64 = 60; const REFRESH_CONFIGURATION_RETRY_SECONDS: i64 = 60 * 60; -const REFRESH_WORKER_PAGE_SIZE: u32 = 1000; const MAX_OAUTH_ERROR_RESPONSE_BYTES: usize = 8 * 1024; pub fn refresh_material_scope( @@ -171,51 +172,19 @@ pub async fn list_refresh_states_for_provider( store: &Store, provider_id: &str, ) -> Result, Status> { - let records = store - .list_by_scope( - StoredProviderCredentialRefreshState::object_type(), - provider_id, - 1000, - 0, - ) + store + .collect_messages(ObjectListQuery::Scope(provider_id)) .await - .map_err(|e| Status::internal(format!("list provider refresh states failed: {e}")))?; - - let mut states = Vec::with_capacity(records.len()); - for record in records { - let mut state = StoredProviderCredentialRefreshState::decode(record.payload.as_slice()) - .map_err(|e| Status::internal(format!("decode provider refresh state failed: {e}")))?; - state.set_resource_version(record.resource_version); - states.push(state); - } - Ok(states) + .map_err(|e| Status::internal(format!("list provider refresh states failed: {e}"))) } pub async fn list_all_refresh_states( store: &Store, ) -> Result, Status> { - let mut states = Vec::new(); - let mut offset = 0; - loop { - let page = store - .list_all_messages::( - REFRESH_WORKER_PAGE_SIZE, - offset, - ) - .await - .map_err(|e| Status::internal(format!("list provider refresh states failed: {e}")))?; - if page.is_empty() { - break; - } - offset = offset - .checked_add( - u32::try_from(page.len()) - .map_err(|_| Status::internal("provider refresh page size exceeded u32"))?, - ) - .ok_or_else(|| Status::internal("provider refresh pagination offset overflow"))?; - states.extend(page); - } - Ok(states) + store + .collect_messages(ObjectListQuery::AllWorkspaces) + .await + .map_err(|e| Status::internal(format!("list provider refresh states failed: {e}"))) } pub async fn get_refresh_state( diff --git a/crates/openshell-server/src/storage_proto.rs b/crates/openshell-server/src/storage_proto.rs index cd5e065dee..8f7c55914f 100644 --- a/crates/openshell-server/src/storage_proto.rs +++ b/crates/openshell-server/src/storage_proto.rs @@ -119,7 +119,7 @@ mod tests { const STORAGE_V1_SCHEMA_SHA256: &str = "79c72615d957fc0653c672f61998bf7d8d21b757bc05d07b3fff92bd70fc8f52"; const PUBLIC_RPC_SCHEMA_SHA256: &str = - "c95ae90962c10fb28747db2b645adf4044562a3d208d84dfe4699d677e4364ee"; + "0f14943574349d02bdc61076c8c5a59a98b627325564ef1a6d21d7941825dc46"; const DURABLE_SCHEMA_SHA256: &str = "920a5243dfb37ce709f0f562a47d17791a5ede90fd7f662ed01542abd60a0dfb"; const PUBLIC_DURABLE_OVERLAP_SHA256: &str = diff --git a/crates/openshell-tui/Cargo.toml b/crates/openshell-tui/Cargo.toml index c1c64d83cb..bc79d114b2 100644 --- a/crates/openshell-tui/Cargo.toml +++ b/crates/openshell-tui/Cargo.toml @@ -17,6 +17,7 @@ openshell-policy = { path = "../openshell-policy" } openshell-providers = { path = "../openshell-providers" } base64 = { workspace = true } +futures = { workspace = true } ratatui = { workspace = true } crossterm = { workspace = true } terminal-colorsaurus = { workspace = true } diff --git a/crates/openshell-tui/src/app.rs b/crates/openshell-tui/src/app.rs index cf7464fcd8..64d9bd5806 100644 --- a/crates/openshell-tui/src/app.rs +++ b/crates/openshell-tui/src/app.rs @@ -599,6 +599,14 @@ pub struct App { pub all_workspaces: bool, pub workspace_names: Vec, pub pending_workspace_refresh: bool, + /// Monotonic identity for the active background list refresh. + pub list_refresh_generation: u64, + /// Active list refresh task. New ticks do not overlap this task. + pub list_refresh_handle: Option>, + /// Monotonic identity for the active draft-count refresh. + pub draft_counts_refresh_generation: u64, + /// Active draft-count refresh task. New ticks do not overlap this task. + pub draft_counts_refresh_handle: Option>, // Provider list pub provider_profiles: Vec, @@ -981,6 +989,10 @@ impl App { all_workspaces: false, workspace_names: Vec::new(), pending_workspace_refresh: false, + list_refresh_generation: 0, + list_refresh_handle: None, + draft_counts_refresh_generation: 0, + draft_counts_refresh_handle: None, provider_profiles: Vec::new(), provider_entries: Vec::new(), provider_names: Vec::new(), @@ -3479,6 +3491,23 @@ impl App { } } + /// Cancel any in-flight collection refresh and invalidate queued results. + pub fn cancel_list_refresh(&mut self) { + if let Some(handle) = self.list_refresh_handle.take() { + handle.abort(); + } + self.list_refresh_generation = self.list_refresh_generation.wrapping_add(1); + self.cancel_draft_counts_refresh(); + } + + /// Cancel any in-flight draft-count refresh and invalidate queued results. + pub fn cancel_draft_counts_refresh(&mut self) { + if let Some(handle) = self.draft_counts_refresh_handle.take() { + handle.abort(); + } + self.draft_counts_refresh_generation = self.draft_counts_refresh_generation.wrapping_add(1); + } + /// Stop the animation ticker if running. pub fn stop_anim(&mut self) { if let Some(h) = self.anim_handle.take() { @@ -3490,6 +3519,7 @@ impl App { pub fn reset_sandbox_state(&mut self) { self.stop_anim(); self.cancel_log_stream(); + self.cancel_list_refresh(); self.sandbox_ids.clear(); self.sandbox_names.clear(); self.sandbox_phases.clear(); diff --git a/crates/openshell-tui/src/event.rs b/crates/openshell-tui/src/event.rs index 151589e8c5..f24ab54ed7 100644 --- a/crates/openshell-tui/src/event.rs +++ b/crates/openshell-tui/src/event.rs @@ -20,6 +20,10 @@ pub enum Event { Resize(u16, u16), /// A batch of log lines from the streaming log task. LogLines(Vec), + /// Completed workspace/provider/sandbox list refresh from a background task. + ListRefreshCompleted(crate::ListRefreshResult), + /// Completed per-sandbox draft-count refresh from a background task. + DraftCountsRefreshCompleted(crate::DraftCountsRefreshResult), /// Result of a create sandbox request: `Ok((name, workspace))` or `Err(message)`. CreateResult(Result<(String, String), String>), /// Result of creating a provider on the gateway: `Ok(name)` or `Err(message)`. diff --git a/crates/openshell-tui/src/lib.rs b/crates/openshell-tui/src/lib.rs index 2ab19d89d7..55fbcaff87 100644 --- a/crates/openshell-tui/src/lib.rs +++ b/crates/openshell-tui/src/lib.rs @@ -18,6 +18,7 @@ use crossterm::execute; use crossterm::terminal::{ EnterAlternateScreen, LeaveAlternateScreen, disable_raw_mode, enable_raw_mode, }; +use futures::{StreamExt, stream}; use miette::{IntoDiagnostic, Result}; use openshell_bootstrap::list_gateways_with_source; use openshell_core::auth::EdgeAuthInterceptor; @@ -28,6 +29,7 @@ use ratatui::Terminal; use ratatui::backend::CrosstermBackend; use tokio::sync::mpsc; use tonic::Code; +use tonic::service::interceptor::InterceptedService; use tonic::transport::{Certificate, Channel, ClientTlsConfig, Endpoint, Identity}; use app::{App, Focus, GatewayEntry, LogLine, Screen}; @@ -36,9 +38,39 @@ use event::{Event, EventHandler}; /// Duration to show the splash screen before auto-dismissing. const SPLASH_DURATION: Duration = Duration::from_secs(3); const PROVIDER_PROFILE_SCOPE_WORKSPACE: &str = "workspace"; -const PROVIDER_PROFILE_PAGE_SIZE: u32 = 100; +const PROVIDER_PROFILE_PAGE_SIZE: i32 = 100; +const DRAFT_COUNT_REFRESH_CONCURRENCY: usize = 16; type ProviderProfileCache = HashMap<(String, String), openshell_core::proto::ProviderProfile>; +type TuiClient = OpenShellClient>; + +#[derive(Debug)] +pub(crate) struct ListRefreshResult { + generation: u64, + gateway_name: String, + workspace: String, + all_workspaces: bool, + workspaces: Result, String>, + providers: Result, + sandboxes: Result, String>, +} + +#[derive(Debug)] +pub(crate) struct DraftCountsRefreshResult { + generation: u64, + gateway_name: String, + workspace: String, + all_workspaces: bool, + sandboxes: Vec<(String, String)>, + counts: Vec, +} + +#[derive(Debug)] +struct ProviderListRefresh { + providers: Vec, + profiles: ProviderProfileCache, + workspace_profiles: Vec, +} fn named_workspace_scope(workspace: impl Into) -> openshell_core::proto::WorkspaceSelector { openshell_core::proto::workspace_selector(workspace) @@ -96,7 +128,9 @@ pub async fn run( let mut events = EventHandler::new(Duration::from_secs(2)); refresh_gateway_list(&mut app); - refresh_data(&mut app).await; + refresh_health(&mut app).await; + refresh_global_settings(&mut app).await; + spawn_list_refresh(&mut app, events.sender()); while app.running { terminal @@ -108,7 +142,7 @@ pub async fn run( app.handle_key(key); // Handle async actions triggered by key presses. if app.pending_gateway_switch.is_some() { - handle_gateway_switch(&mut app).await; + handle_gateway_switch(&mut app, events.sender()).await; } if app.pending_log_fetch { app.pending_log_fetch = false; @@ -116,7 +150,7 @@ pub async fn run( } if app.pending_sandbox_delete { app.pending_sandbox_delete = false; - handle_sandbox_delete(&mut app).await; + handle_sandbox_delete(&mut app, events.sender()).await; } if app.pending_create_sandbox { app.pending_create_sandbox = false; @@ -183,10 +217,17 @@ pub async fn run( } if app.pending_workspace_refresh { app.pending_workspace_refresh = false; - refresh_providers(&mut app).await; - refresh_sandboxes(&mut app).await; + app.cancel_list_refresh(); + spawn_list_refresh(&mut app, events.sender()); } } + Some(Event::ListRefreshCompleted(result)) => { + apply_list_refresh(&mut app, result); + spawn_sandbox_draft_counts_refresh(&mut app, events.sender()); + } + Some(Event::DraftCountsRefreshCompleted(result)) => { + apply_sandbox_draft_counts_refresh(&mut app, result); + } Some(Event::LogLines(lines)) => { app.sandbox_log_lines.extend(lines); if app.log_autoscroll { @@ -224,7 +265,8 @@ pub async fn run( Ok(name) => { app.update_provider_form = None; app.status_text = format!("Updated provider: {name}"); - refresh_providers(&mut app).await; + app.cancel_list_refresh(); + spawn_list_refresh(&mut app, events.sender()); } Err(msg) => { if let Some(form) = app.update_provider_form.as_mut() { @@ -235,7 +277,8 @@ pub async fn run( Some(Event::ProviderDeleteResult(result)) => match result { Ok(true) => { app.status_text = "Provider deleted.".to_string(); - refresh_providers(&mut app).await; + app.cancel_list_refresh(); + spawn_list_refresh(&mut app, events.sender()); } Ok(false) => { app.status_text = "Provider not found.".to_string(); @@ -255,7 +298,8 @@ pub async fn run( } // Refresh draft chunks + counts immediately after any action. refresh_draft_chunks(&mut app).await; - refresh_sandbox_draft_counts(&mut app).await; + app.cancel_draft_counts_refresh(); + spawn_sandbox_draft_counts_refresh(&mut app, events.sender()); } Some(Event::GlobalSettingsFetched(result)) => match result { Ok((settings, revision)) => { @@ -340,10 +384,12 @@ pub async fn run( } refresh_gateway_list(&mut app); - refresh_data(&mut app).await; + refresh_health(&mut app).await; + refresh_global_settings(&mut app).await; + spawn_list_refresh(&mut app, events.sender()); // Refresh per-sandbox draft counts for badges (dashboard + detail). - refresh_sandbox_draft_counts(&mut app).await; + spawn_sandbox_draft_counts_refresh(&mut app, events.sender()); // Auto-refresh sandbox detail (policy, settings, drafts) on // every tick when viewing a sandbox. The gRPC call is @@ -386,7 +432,6 @@ pub async fn run( format!(" (forwarding port(s) {list})") }; app.status_text = format!("Created sandbox: {name}{port_info}"); - refresh_sandboxes(&mut app).await; // If a command was specified, suspend TUI and exec it. if !command.is_empty() { @@ -400,6 +445,8 @@ pub async fn run( ) .await?; } + app.cancel_list_refresh(); + spawn_list_refresh(&mut app, events.sender()); } Some(Err(msg)) => { if let Some(form) = app.create_form.as_mut() { @@ -431,7 +478,8 @@ pub async fn run( Some(Ok(name)) => { app.create_provider_form = None; app.status_text = format!("Created provider: {name}"); - refresh_providers(&mut app).await; + app.cancel_list_refresh(); + spawn_list_refresh(&mut app, events.sender()); } Some(Err(msg)) => { if let Some(form) = app.create_provider_form.as_mut() { @@ -499,7 +547,7 @@ fn refresh_gateway_list(app: &mut App) { } /// Handle a pending gateway switch requested by the user. -async fn handle_gateway_switch(app: &mut App) { +async fn handle_gateway_switch(app: &mut App, tx: mpsc::UnboundedSender) { let Some(name) = app.pending_gateway_switch.take() else { return; }; @@ -516,7 +564,9 @@ async fn handle_gateway_switch(app: &mut App) { app.gateway_name = name; app.endpoint = endpoint; app.reset_sandbox_state(); - refresh_data(app).await; + refresh_health(app).await; + refresh_global_settings(app).await; + spawn_list_refresh(app, tx); } Err(e) => { app.status_text = format!("switch failed: {e}"); @@ -747,7 +797,7 @@ fn proto_to_log_line(log: openshell_core::proto::SandboxLogLine) -> LogLine { } /// Delete the currently selected sandbox. -async fn handle_sandbox_delete(app: &mut App) { +async fn handle_sandbox_delete(app: &mut App, tx: mpsc::UnboundedSender) { let sandbox_name = match app.selected_sandbox_name() { Some(n) => n.to_string(), None => return, @@ -773,7 +823,8 @@ async fn handle_sandbox_delete(app: &mut App) { app.cancel_log_stream(); app.screen = Screen::Dashboard; app.focus = Focus::Sandboxes; - refresh_sandboxes(app).await; + app.cancel_list_refresh(); + spawn_list_refresh(app, tx); } Err(e) => { app.status_text = format!("delete failed: {}", e.message()); @@ -977,6 +1028,7 @@ async fn handle_shell_connect( // Step 6: Cancel log stream and pause event handler before suspending. app.cancel_log_stream(); + app.cancel_list_refresh(); events.pause(); // Wait for the reader task to finish its current poll cycle (tick_rate = 2s max). tokio::time::sleep(Duration::from_millis(100)).await; @@ -1021,6 +1073,7 @@ async fn handle_shell_connect( .into_diagnostic()?; events.discard_pending(); events.resume(); + spawn_list_refresh(app, events.sender()); Ok(()) } @@ -1140,6 +1193,7 @@ async fn handle_exec_command( // Step 4: Suspend TUI. app.cancel_log_stream(); + app.cancel_list_refresh(); events.pause(); tokio::time::sleep(Duration::from_millis(100)).await; @@ -1179,6 +1233,7 @@ async fn handle_exec_command( terminal.clear().into_diagnostic()?; events.discard_pending(); events.resume(); + spawn_list_refresh(app, events.sender()); Ok(()) } @@ -1505,9 +1560,7 @@ fn spawn_create_sandbox(app: &mut App, tx: mpsc::UnboundedSender) { /// This is called from within the create-sandbox task so the pacman animation /// keeps running while forwards are being established. async fn start_port_forwards( - client: &mut OpenShellClient< - tonic::service::interceptor::InterceptedService, - >, + client: &mut TuiClient, endpoint: &str, gateway_name: &str, sandbox_name: &str, @@ -2037,34 +2090,98 @@ fn format_draft_approve_all_result( // Data refresh // --------------------------------------------------------------------------- -async fn refresh_data(app: &mut App) { - refresh_health(app).await; - refresh_global_settings(app).await; - refresh_workspaces(app).await; - refresh_providers(app).await; - refresh_sandboxes(app).await; +fn spawn_list_refresh(app: &mut App, tx: mpsc::UnboundedSender) { + if let Some(handle) = app.list_refresh_handle.as_ref() { + if !handle.is_finished() { + return; + } + app.list_refresh_handle.take(); + } + + app.list_refresh_generation = app.list_refresh_generation.wrapping_add(1); + let generation = app.list_refresh_generation; + let gateway_name = app.gateway_name.clone(); + let refresh_workspace = app.current_workspace.clone(); + let all_workspaces = app.all_workspaces; + let client = app.client.clone(); + let handle = tokio::spawn(async move { + let (workspaces, providers, sandboxes) = tokio::join!( + fetch_workspaces(client.clone()), + fetch_providers(client.clone(), refresh_workspace.clone(), all_workspaces), + fetch_sandboxes(client, refresh_workspace.clone(), all_workspaces), + ); + let _ = tx.send(Event::ListRefreshCompleted(ListRefreshResult { + generation, + gateway_name, + workspace: refresh_workspace, + all_workspaces, + workspaces, + providers, + sandboxes, + })); + }); + app.list_refresh_handle = Some(handle); } -async fn refresh_workspaces(app: &mut App) { - let req = openshell_core::proto::ListWorkspacesRequest { - limit: 100, - offset: 0, - label_selector: String::new(), - }; - match tokio::time::timeout(Duration::from_secs(5), app.client.list_workspaces(req)).await { - Ok(Ok(resp)) => { - app.workspace_names = resp - .into_inner() - .workspaces - .into_iter() - .filter_map(|w| w.metadata.map(|m| m.name)) - .collect(); - } - Ok(Err(e)) => { - app.status_text = format!("failed to list workspaces: {}", e.message()); - } - Err(_) => { - app.status_text = "list workspaces timed out".to_string(); +fn apply_list_refresh(app: &mut App, result: ListRefreshResult) { + if result.generation != app.list_refresh_generation { + return; + } + app.list_refresh_handle.take(); + if result.gateway_name != app.gateway_name { + return; + } + if result.workspace != app.current_workspace { + return; + } + if result.all_workspaces != app.all_workspaces { + return; + } + app.cancel_draft_counts_refresh(); + + match result.workspaces { + Ok(workspaces) => app.workspace_names = workspaces, + Err(message) => app.status_text = message, + } + match result.providers { + Ok(providers) => apply_provider_refresh(app, providers), + Err(message) => app.status_text = message, + } + match result.sandboxes { + Ok(sandboxes) => apply_sandbox_refresh(app, sandboxes), + Err(message) => app.status_text = message, + } +} + +async fn fetch_workspaces(mut client: TuiClient) -> std::result::Result, String> { + let mut workspace_names = Vec::new(); + let mut page_token = String::new(); + loop { + let req = openshell_core::proto::ListWorkspacesRequest { + page_size: 100, + page_token, + label_selector: String::new(), + }; + match tokio::time::timeout(Duration::from_secs(5), client.list_workspaces(req)).await { + Ok(Ok(resp)) => { + let response = resp.into_inner(); + workspace_names.extend( + response + .workspaces + .into_iter() + .filter_map(|workspace| workspace.metadata.map(|metadata| metadata.name)), + ); + if response.next_page_token.is_empty() { + return Ok(workspace_names); + } + page_token = response.next_page_token; + } + Ok(Err(e)) => { + return Err(format!("failed to list workspaces: {}", e.message())); + } + Err(_) => { + return Err("list workspaces timed out".to_string()); + } } } } @@ -2111,28 +2228,36 @@ fn cached_provider_profile( .cloned() } -async fn refresh_providers(app: &mut App) { - let req = openshell_core::proto::ListProvidersRequest { - limit: 100, - offset: 0, - workspace_scope: Some(list_workspace_scope( - &app.current_workspace, - app.all_workspaces, - )), - }; - let response = - match tokio::time::timeout(Duration::from_secs(5), app.client.list_providers(req)).await { - Ok(Ok(resp)) => resp.into_inner(), +async fn fetch_providers( + mut client: TuiClient, + current_workspace: String, + all_workspaces: bool, +) -> std::result::Result { + let mut providers = Vec::new(); + let mut page_token = String::new(); + loop { + let req = openshell_core::proto::ListProvidersRequest { + page_size: 100, + page_token, + workspace_scope: Some(list_workspace_scope(¤t_workspace, all_workspaces)), + }; + match tokio::time::timeout(Duration::from_secs(5), client.list_providers(req)).await { + Ok(Ok(resp)) => { + let response = resp.into_inner(); + providers.extend(response.providers); + if response.next_page_token.is_empty() { + break; + } + page_token = response.next_page_token; + } Ok(Err(e)) => { - app.status_text = format!("failed to list providers: {}", e.message()); - return; + return Err(format!("failed to list providers: {}", e.message())); } Err(_) => { - app.status_text = "list providers timed out".to_string(); - return; + return Err("list providers timed out".to_string()); } - }; - let providers = response.providers; + } + } let mut workspaces: std::collections::HashSet = providers .iter() @@ -2141,21 +2266,21 @@ async fn refresh_providers(app: &mut App) { // turn that missing context into a platform-scoped profile request. .filter(|workspace| !workspace.is_empty()) .collect(); - if !app.all_workspaces { - workspaces.insert(app.current_workspace.clone()); + if !all_workspaces { + workspaces.insert(current_workspace.clone()); } let mut profiles = HashMap::new(); - app.provider_profiles.clear(); + let mut workspace_profiles = Vec::new(); for ws in &workspaces { - let client = app.client.clone(); + let client = client.clone(); let workspace = ws.clone(); - if let Some(listed) = collect_provider_profile_pages(move |offset| { + if let Some(listed) = collect_provider_profile_pages(move |page_token| { let mut client = client.clone(); let workspace = workspace.clone(); async move { let req = openshell_core::proto::ListProviderProfilesRequest { - limit: PROVIDER_PROFILE_PAGE_SIZE, - offset, + page_size: PROVIDER_PROFILE_PAGE_SIZE, + page_token, workspace, }; match tokio::time::timeout( @@ -2164,21 +2289,39 @@ async fn refresh_providers(app: &mut App) { ) .await { - Ok(Ok(response)) => Some(response.into_inner().profiles), + Ok(Ok(response)) => { + let response = response.into_inner(); + Some((response.profiles, response.next_page_token)) + } _ => None, } } }) .await { - if !app.all_workspaces && ws == &app.current_workspace { - app.provider_profiles.clone_from(&listed); + if !all_workspaces && ws == ¤t_workspace { + workspace_profiles.clone_from(&listed); } for profile in listed { cache_provider_profile(&mut profiles, ws, profile); } } } + + Ok(ProviderListRefresh { + providers, + profiles, + workspace_profiles, + }) +} + +fn apply_provider_refresh(app: &mut App, refresh: ProviderListRefresh) { + let ProviderListRefresh { + providers, + profiles, + workspace_profiles, + } = refresh; + app.provider_profiles = workspace_profiles; app.sync_create_provider_types(); app.provider_count = providers.len(); @@ -2218,19 +2361,18 @@ async fn collect_provider_profile_pages( mut fetch_page: F, ) -> Option> where - F: FnMut(u32) -> Fut, - Fut: Future>>, + F: FnMut(String) -> Fut, + Fut: Future, String)>>, { let mut profiles = Vec::new(); - let mut offset = 0; + let mut page_token = String::new(); loop { - let page = fetch_page(offset).await?; - let page_len = page.len(); + let (page, next_page_token) = fetch_page(page_token).await?; profiles.extend(page); - if page_len < PROVIDER_PROFILE_PAGE_SIZE as usize { + if next_page_token.is_empty() { return Some(profiles); } - offset = offset.saturating_add(PROVIDER_PROFILE_PAGE_SIZE); + page_token = next_page_token; } } @@ -2263,8 +2405,8 @@ async fn refresh_global_settings(app: &mut App) { // Check for an active global policy only while the caller can read it. let policy_req = openshell_core::proto::ListSandboxPoliciesRequest { name: String::new(), - limit: 1, - offset: 0, + page_size: 1, + page_token: String::new(), global: true, workspace_scope: None, }; @@ -2523,113 +2665,126 @@ async fn refresh_health(app: &mut App) { } } -async fn refresh_sandboxes(app: &mut App) { - let req = openshell_core::proto::ListSandboxesRequest { - limit: 100, - offset: 0, - label_selector: String::new(), - workspace_scope: Some(list_workspace_scope( - &app.current_workspace, - app.all_workspaces, - )), - }; - let result = tokio::time::timeout(Duration::from_secs(5), app.client.list_sandboxes(req)).await; - match result { - Ok(Err(e)) => { - app.status_text = format!("failed to list sandboxes: {}", e.message()); - } - Err(_) => { - app.status_text = "list sandboxes timed out".to_string(); - } - Ok(Ok(resp)) => { - let sandboxes = resp.into_inner().sandboxes; - app.sandbox_count = sandboxes.len(); - app.sandbox_ids = sandboxes - .iter() - .map(|s| s.object_id().to_string()) - .collect(); - app.sandbox_names = sandboxes - .iter() - .map(|s| s.object_name().to_string()) - .collect(); - app.sandbox_phases = sandboxes.iter().map(|s| phase_label(s.phase())).collect(); - app.sandbox_images = sandboxes - .iter() - .map(|s| { - s.spec - .as_ref() - .and_then(|spec| spec.template.as_ref()) - .map(|t| t.image.as_str()) - .filter(|img| !img.is_empty()) - .unwrap_or("-") - .to_string() - }) - .collect(); - app.sandbox_ages = sandboxes - .iter() - .map(|s| { - s.metadata - .as_ref() - .map_or_else(|| "?".to_string(), |m| format_age(m.created_at_ms)) - }) - .collect(); - app.sandbox_created = sandboxes - .iter() - .map(|s| { - s.metadata - .as_ref() - .map_or_else(|| "?".to_string(), |m| format_timestamp(m.created_at_ms)) - }) - .collect(); - - app.sandbox_policy_versions = sandboxes - .iter() - .map(openshell_core::proto::Sandbox::current_policy_version) - .collect(); - - // Build NOTES column from active port forwards. - let forwards = openshell_core::forward::list_forwards().unwrap_or_default(); - app.sandbox_notes = sandboxes - .iter() - .map(|s| { - let name = s.object_name(); - openshell_core::forward::build_sandbox_notes(name, &forwards) - }) - .collect(); - - // Build LABELS column from metadata. - app.sandbox_labels = sandboxes - .iter() - .map(|s| { - s.object_labels() - .as_ref() - .map(app::format_labels) - .unwrap_or_default() - }) - .collect(); - - app.sandbox_annotations = sandboxes - .iter() - .map(|s| { - s.metadata - .as_ref() - .map(|metadata| app::format_annotations(&metadata.annotations)) - .unwrap_or_default() - }) - .collect(); - - app.sandbox_workspaces = sandboxes - .iter() - .map(|s| s.object_workspace().to_string()) - .collect(); - - if app.sandbox_selected >= app.sandbox_count && app.sandbox_count > 0 { - app.sandbox_selected = app.sandbox_count - 1; +async fn fetch_sandboxes( + mut client: TuiClient, + current_workspace: String, + all_workspaces: bool, +) -> std::result::Result, String> { + let mut page_token = String::new(); + let mut sandboxes = Vec::new(); + loop { + let req = openshell_core::proto::ListSandboxesRequest { + page_size: 100, + page_token, + label_selector: String::new(), + workspace_scope: Some(list_workspace_scope(¤t_workspace, all_workspaces)), + }; + let result = tokio::time::timeout(Duration::from_secs(5), client.list_sandboxes(req)).await; + match result { + Ok(Err(e)) => { + return Err(format!("failed to list sandboxes: {}", e.message())); + } + Err(_) => { + return Err("list sandboxes timed out".to_string()); + } + Ok(Ok(resp)) => { + let response = resp.into_inner(); + sandboxes.extend(response.sandboxes); + if response.next_page_token.is_empty() { + return Ok(sandboxes); + } + page_token = response.next_page_token; } } } } +fn apply_sandbox_refresh(app: &mut App, sandboxes: Vec) { + app.sandbox_count = sandboxes.len(); + app.sandbox_ids = sandboxes + .iter() + .map(|s| s.object_id().to_string()) + .collect(); + app.sandbox_names = sandboxes + .iter() + .map(|s| s.object_name().to_string()) + .collect(); + app.sandbox_phases = sandboxes.iter().map(|s| phase_label(s.phase())).collect(); + app.sandbox_images = sandboxes + .iter() + .map(|s| { + s.spec + .as_ref() + .and_then(|spec| spec.template.as_ref()) + .map(|t| t.image.as_str()) + .filter(|img| !img.is_empty()) + .unwrap_or("-") + .to_string() + }) + .collect(); + app.sandbox_ages = sandboxes + .iter() + .map(|s| { + s.metadata + .as_ref() + .map_or_else(|| "?".to_string(), |m| format_age(m.created_at_ms)) + }) + .collect(); + app.sandbox_created = sandboxes + .iter() + .map(|s| { + s.metadata + .as_ref() + .map_or_else(|| "?".to_string(), |m| format_timestamp(m.created_at_ms)) + }) + .collect(); + + app.sandbox_policy_versions = sandboxes + .iter() + .map(openshell_core::proto::Sandbox::current_policy_version) + .collect(); + + // Build NOTES column from active port forwards. + let forwards = openshell_core::forward::list_forwards().unwrap_or_default(); + app.sandbox_notes = sandboxes + .iter() + .map(|s| { + let name = s.object_name(); + openshell_core::forward::build_sandbox_notes(name, &forwards) + }) + .collect(); + + // Build LABELS column from metadata. + app.sandbox_labels = sandboxes + .iter() + .map(|s| { + s.object_labels() + .as_ref() + .map(app::format_labels) + .unwrap_or_default() + }) + .collect(); + + app.sandbox_annotations = sandboxes + .iter() + .map(|s| { + s.metadata + .as_ref() + .map(|metadata| app::format_annotations(&metadata.annotations)) + .unwrap_or_default() + }) + .collect(); + + app.sandbox_workspaces = sandboxes + .iter() + .map(|s| s.object_workspace().to_string()) + .collect(); + + if app.sandbox_selected >= app.sandbox_count && app.sandbox_count > 0 { + app.sandbox_selected = app.sandbox_count - 1; + } +} + /// Re-fetch only the sandbox policy when a version change is detected. /// /// Unlike `fetch_sandbox_detail()`, this skips the `GetSandbox` metadata call @@ -2695,31 +2850,127 @@ async fn refresh_draft_chunks(app: &mut App) { } } -/// Fetch the count of pending draft recommendations for every sandbox. -/// -/// This runs on the Dashboard tick so the sandbox list can show notification -/// badges without entering the sandbox detail view. -async fn refresh_sandbox_draft_counts(app: &mut App) { - let names: Vec = app.sandbox_names.clone(); - let workspaces: Vec = app.sandbox_workspaces.clone(); - let mut counts = vec![0usize; names.len()]; - for (i, name) in names.iter().enumerate() { - let ws = workspaces - .get(i) - .cloned() - .unwrap_or_else(|| app.current_workspace.clone()); - let req = openshell_core::proto::GetDraftPolicyRequest { - name: name.clone(), - status_filter: "pending".to_string(), - workspace_scope: Some(named_workspace_scope(ws)), - }; - if let Ok(Ok(resp)) = - tokio::time::timeout(Duration::from_secs(2), app.client.get_draft_policy(req)).await - { - counts[i] = resp.into_inner().chunks.len(); +/// Start a bounded, non-overlapping background refresh of pending draft counts. +fn spawn_sandbox_draft_counts_refresh(app: &mut App, tx: mpsc::UnboundedSender) { + if let Some(handle) = app.draft_counts_refresh_handle.as_ref() { + if !handle.is_finished() { + return; } + app.draft_counts_refresh_handle.take(); + } + + let sandboxes: Vec<_> = app + .sandbox_names + .iter() + .cloned() + .enumerate() + .map(|(index, name)| { + let workspace = app + .sandbox_workspaces + .get(index) + .cloned() + .unwrap_or_else(|| app.current_workspace.clone()); + (name, workspace) + }) + .collect(); + if sandboxes.is_empty() { + app.sandbox_draft_counts.clear(); + return; + } + + app.draft_counts_refresh_generation = app.draft_counts_refresh_generation.wrapping_add(1); + let generation = app.draft_counts_refresh_generation; + let gateway_name = app.gateway_name.clone(); + let workspace = app.current_workspace.clone(); + let all_workspaces = app.all_workspaces; + let client = app.client.clone(); + let refresh_sandboxes = sandboxes.clone(); + let handle = tokio::spawn(async move { + let counts = fetch_sandbox_draft_counts(client, refresh_sandboxes).await; + let _ = tx.send(Event::DraftCountsRefreshCompleted( + DraftCountsRefreshResult { + generation, + gateway_name, + workspace, + all_workspaces, + sandboxes, + counts, + }, + )); + }); + app.draft_counts_refresh_handle = Some(handle); +} + +async fn fetch_sandbox_draft_counts( + client: TuiClient, + sandboxes: Vec<(String, String)>, +) -> Vec { + let count = sandboxes.len(); + let results = stream::iter(sandboxes.into_iter().enumerate().map( + |(index, (name, workspace))| { + let mut client = client.clone(); + async move { + let req = openshell_core::proto::GetDraftPolicyRequest { + name, + status_filter: "pending".to_string(), + workspace_scope: Some(named_workspace_scope(workspace)), + }; + let count = match tokio::time::timeout( + Duration::from_secs(2), + client.get_draft_policy(req), + ) + .await + { + Ok(Ok(resp)) => resp.into_inner().chunks.len(), + _ => 0, + }; + (index, count) + } + }, + )) + .buffer_unordered(DRAFT_COUNT_REFRESH_CONCURRENCY) + .collect::>() + .await; + let mut counts = vec![0; count]; + for (index, value) in results { + counts[index] = value; + } + counts +} + +fn apply_sandbox_draft_counts_refresh(app: &mut App, result: DraftCountsRefreshResult) { + if result.generation != app.draft_counts_refresh_generation { + return; + } + app.draft_counts_refresh_handle.take(); + if ( + result.gateway_name.as_str(), + result.workspace.as_str(), + result.all_workspaces, + ) != ( + app.gateway_name.as_str(), + app.current_workspace.as_str(), + app.all_workspaces, + ) { + return; + } + let current: Vec<_> = app + .sandbox_names + .iter() + .cloned() + .enumerate() + .map(|(index, name)| { + let workspace = app + .sandbox_workspaces + .get(index) + .cloned() + .unwrap_or_else(|| app.current_workspace.clone()); + (name, workspace) + }) + .collect(); + if result.sandboxes == current { + app.sandbox_draft_counts = result.counts; } - app.sandbox_draft_counts = counts; } fn phase_label(phase: i32) -> String { @@ -2896,29 +3147,31 @@ mod provider_profile_pagination_tests { #[tokio::test] async fn profile_fetch_continues_until_page_two_is_collected() { - let requested_offsets = Arc::new(Mutex::new(Vec::new())); - let offsets = Arc::clone(&requested_offsets); + let requested_tokens = Arc::new(Mutex::new(Vec::new())); + let tokens = Arc::clone(&requested_tokens); - let profiles = collect_provider_profile_pages(move |offset| { - let offsets = Arc::clone(&offsets); + let profiles = collect_provider_profile_pages(move |page_token| { + let tokens = Arc::clone(&tokens); async move { - offsets.lock().unwrap().push(offset); - match offset { - 0 => Some( + tokens.lock().unwrap().push(page_token.clone()); + match page_token.as_str() { + "" => Some(( (0..PROVIDER_PROFILE_PAGE_SIZE) .map(|index| openshell_core::proto::ProviderProfile { id: format!("profile-{index}"), ..Default::default() }) .collect(), - ), - PROVIDER_PROFILE_PAGE_SIZE => { - Some(vec![openshell_core::proto::ProviderProfile { + "next".to_string(), + )), + "next" => Some(( + vec![openshell_core::proto::ProviderProfile { id: "page-two-profile".to_string(), ..Default::default() - }]) - } - _ => panic!("unexpected profile page offset {offset}"), + }], + String::new(), + )), + _ => panic!("unexpected profile page token {page_token}"), } } }) @@ -2926,8 +3179,8 @@ mod provider_profile_pagination_tests { .expect("all pages should load"); assert_eq!( - *requested_offsets.lock().unwrap(), - vec![0, PROVIDER_PROFILE_PAGE_SIZE] + *requested_tokens.lock().unwrap(), + vec![String::new(), "next".to_string()] ); assert_eq!(profiles.len(), PROVIDER_PROFILE_PAGE_SIZE as usize + 1); assert_eq!(profiles.last().unwrap().id, "page-two-profile"); diff --git a/docs/sandboxes/manage-providers.mdx b/docs/sandboxes/manage-providers.mdx index 895d6761ca..93f5b004b6 100644 --- a/docs/sandboxes/manage-providers.mdx +++ b/docs/sandboxes/manage-providers.mdx @@ -157,7 +157,14 @@ openshell provider list -o json openshell provider list -o yaml ``` -Structured output includes provider metadata (`id`, `name`, `type`), credential key names, config key names, labels, creation timestamp, resource version, and credential expiration times. Only credential and config keys are exposed, never values, preventing accidental credential leakage in logs or output. For details on how credentials are injected into sandboxes, refer to [Credential Injection](#how-credential-injection-works). +Structured list output is an envelope with `providers` and +`next_page_token` fields. Each provider includes metadata (`id`, `name`, +`type`), credential key names, config key names, labels, creation timestamp, +resource version, and credential expiration times. Only credential and config +keys are exposed, never values, preventing accidental credential leakage in +logs or output. Pass the returned token to `--page-token` to continue. For +details on how credentials are injected into sandboxes, refer to +[Credential Injection](#how-credential-injection-works). Inspect a provider: diff --git a/docs/sandboxes/manage-sandboxes.mdx b/docs/sandboxes/manage-sandboxes.mdx index dd192d40fe..41fe7e9e81 100644 --- a/docs/sandboxes/manage-sandboxes.mdx +++ b/docs/sandboxes/manage-sandboxes.mdx @@ -262,6 +262,10 @@ Use `--all-workspaces` with `sandbox template list` when you need an admin view openshell sandbox template list --all-workspaces ``` +For JSON or YAML, template list output contains `templates` and +`next_page_token` fields. Pass the returned token to `--page-token` to +continue. + ## Base Sandbox Container The `base` sandbox container is the default runtime image for standard OpenShell sandboxes unless the gateway overrides its default sandbox image. It is published as `ghcr.io/nvidia/openshell-community/sandboxes/base:latest` and maintained in the [OpenShell Community](https://github.com/NVIDIA/OpenShell-Community/tree/main/sandboxes/base) repository. @@ -465,9 +469,10 @@ openshell service list --output json openshell service list my-sandbox --output yaml ``` -Each record contains `workspace`, `sandbox`, `service`, `target_port`, and -`url`. The unnamed service uses an empty `service` string. An empty result is -an empty collection. +Structured list output contains `services` and `next_page_token` fields. Each +record contains `workspace`, `sandbox`, `service`, `target_port`, and `url`. +The unnamed service uses an empty `service` string. Pass the returned token to +`--page-token` to continue. An empty result has an empty `services` collection. Show or delete one endpoint: @@ -508,6 +513,9 @@ openshell sandbox list -o json openshell sandbox list -o yaml ``` +Structured list output contains `sandboxes` and `next_page_token` fields. +Pass the returned token to `--page-token` to continue. + Get detailed information about a specific sandbox. The output lists **Policy source** (`sandbox` or `global`), **Revision** (the active policy’s row version for that source), and the formatted active policy YAML: ```shell diff --git a/docs/sandboxes/manage-workspaces.mdx b/docs/sandboxes/manage-workspaces.mdx index 003201ac21..635fa1e7e3 100644 --- a/docs/sandboxes/manage-workspaces.mdx +++ b/docs/sandboxes/manage-workspaces.mdx @@ -126,7 +126,9 @@ openshell workspace member remove \ Add `--output json` or `--output yaml` to list membership records for automation. Each record contains `subject` and a normalized `role` of `admin`, -`user`, or `unknown`. An empty result is an empty collection. +`user`, or `unknown`. Structured output is an envelope with `members` and +`next_page_token` fields; pass the returned token to `--page-token` to +continue. An empty result has an empty `members` collection. To change a member's role, remove the existing membership and add it again with the new role. A Platform Admin must perform any change to `admin`. diff --git a/docs/sandboxes/policies.mdx b/docs/sandboxes/policies.mdx index 6404eabcb9..da2cb16eb4 100644 --- a/docs/sandboxes/policies.mdx +++ b/docs/sandboxes/policies.mdx @@ -253,9 +253,11 @@ The following steps outline the hot-reload policy update workflow. ``` Add `--output json` or `--output yaml` for automation. Structured policy - history contains the scope, sandbox name when applicable, version, full + history is an envelope with `revisions` and `next_page_token` fields. Each + revision contains the scope, sandbox name when applicable, version, full hash, status, and available revision timestamps, load error, and provenance. - Use `openshell policy list --global --output json` for global history. + Pass the returned token to `--page-token` to continue. Use + `openshell policy list --global --output json` for global history. ### Validation failures diff --git a/e2e/python/conftest.py b/e2e/python/conftest.py index 502f2540e1..ed8775017e 100644 --- a/e2e/python/conftest.py +++ b/e2e/python/conftest.py @@ -64,7 +64,7 @@ def sandbox_client(cluster_name: str | None) -> Iterator[SandboxClient]: def ensure_sandbox_persistence_ready(sandbox_client: SandboxClient) -> None: for _ in range(60): try: - sandbox_client.list_ids(workspace="default", limit=1) + sandbox_client.list_ids(workspace="default", page_size=1) return except grpc.RpcError as exc: details = exc.details() or "" diff --git a/e2e/python/test_sandbox_api.py b/e2e/python/test_sandbox_api.py index bc7e4d5f8c..954eb228de 100644 --- a/e2e/python/test_sandbox_api.py +++ b/e2e/python/test_sandbox_api.py @@ -42,7 +42,7 @@ def read(self, path: str) -> str: fetched = sandbox_client.get(sb.sandbox.name, workspace="default") assert fetched.id == sb.id - ids = set(sandbox_client.list_ids(workspace="default", limit=100)) + ids = set(sandbox_client.list_ids(workspace="default", page_size=100)) assert sb.id in ids result = sb.exec(["python", "-c", "print('sandbox-ok')"]) diff --git a/e2e/python/test_sandbox_providers.py b/e2e/python/test_sandbox_providers.py index 0b3c9b0a83..85a1a354ba 100644 --- a/e2e/python/test_sandbox_providers.py +++ b/e2e/python/test_sandbox_providers.py @@ -1091,7 +1091,7 @@ def _cleanup() -> None: assert resp.imported, "workspace-scoped import should succeed" platform_list = stub.ListProviderProfiles( - openshell_pb2.ListProviderProfilesRequest(limit=200, workspace="") + openshell_pb2.ListProviderProfilesRequest(page_size=200, workspace="") ) platform_ids = [p.id for p in platform_list.profiles] assert platform_id in platform_ids, ( @@ -1102,7 +1102,7 @@ def _cleanup() -> None: ) workspace_list = stub.ListProviderProfiles( - openshell_pb2.ListProviderProfilesRequest(limit=200, workspace="default") + openshell_pb2.ListProviderProfilesRequest(page_size=200, workspace="default") ) workspace_ids = [p.id for p in workspace_list.profiles] assert workspace_id in workspace_ids, ( @@ -1158,14 +1158,14 @@ def _make_profile() -> openshell_pb2.ProviderProfileImportItem: assert resp_b.imported, "import into ws-b should succeed" list_a = stub.ListProviderProfiles( - openshell_pb2.ListProviderProfilesRequest(limit=200, workspace=ws_a) + openshell_pb2.ListProviderProfilesRequest(page_size=200, workspace=ws_a) ) assert any(p.id == profile_id for p in list_a.profiles), ( "profile should appear in ws-a" ) list_b = stub.ListProviderProfiles( - openshell_pb2.ListProviderProfilesRequest(limit=200, workspace=ws_b) + openshell_pb2.ListProviderProfilesRequest(page_size=200, workspace=ws_b) ) assert any(p.id == profile_id for p in list_b.profiles), ( "profile should appear in ws-b" diff --git a/examples/governance-interceptor/src/main.rs b/examples/governance-interceptor/src/main.rs index eefeb695fa..3636ec0570 100644 --- a/examples/governance-interceptor/src/main.rs +++ b/examples/governance-interceptor/src/main.rs @@ -1224,21 +1224,20 @@ async fn propagate_policy_to_running_sandboxes( .await .map_err(|err| format!("connect to gateway {gateway_endpoint} failed: {err}"))?; let mut client = OpenShellClient::new(channel); - let mut offset = 0_u32; - let limit = 100_u32; + let mut page_token = String::new(); let correlation_id = format!("{}:{}", RELOAD_CORRELATION_PREFIX, now_secs()); loop { let response = client .list_sandboxes(ListSandboxesRequest { - limit, - offset, + page_size: 100, + page_token, label_selector: String::new(), workspace_scope: Some(openshell_core::proto::all_workspaces_selector()), }) .await .map_err(|status| format!("list sandboxes failed: {status}"))? .into_inner(); - let count = response.sandboxes.len(); + let next_page_token = response.next_page_token; for sandbox in response.sandboxes { if !sandbox_accepts_policy_reload(&sandbox) { continue; @@ -1279,10 +1278,10 @@ async fn propagate_policy_to_running_sandboxes( } } } - if count < usize::try_from(limit).unwrap_or(usize::MAX) { + if next_page_token.is_empty() { break; } - offset = offset.saturating_add(limit); + page_token = next_page_token; } Ok(()) } diff --git a/examples/transparent-tcp-redis/demo.sh b/examples/transparent-tcp-redis/demo.sh index 687513f363..0d8b24c523 100755 --- a/examples/transparent-tcp-redis/demo.sh +++ b/examples/transparent-tcp-redis/demo.sh @@ -82,7 +82,7 @@ if ! docker network inspect "$DOCKER_NETWORK" >/dev/null 2>&1; then exit 1 fi -if ! openshell sandbox list --limit 1 >/dev/null 2>&1; then +if ! openshell sandbox list --page-size 1 >/dev/null 2>&1; then printf 'The configured OpenShell gateway is not reachable.\n' >&2 printf 'Start or select a Docker-backed gateway and try again.\n' >&2 exit 1 diff --git a/proto/openshell.proto b/proto/openshell.proto index 580cb2ce2c..def6d7c473 100644 --- a/proto/openshell.proto +++ b/proto/openshell.proto @@ -1155,8 +1155,12 @@ message GetSandboxTemplateRequest { message ListSandboxTemplatesRequest { reserved 3, 4; reserved "workspace", "all_workspaces"; - uint32 limit = 1; - uint32 offset = 2; + // The maximum number of templates to return. Zero uses 100. Values above + // 1000 are coerced to 1000; negative values are invalid. + int32 page_size = 1; + // Token from a previous ListSandboxTemplates response. All other request + // parameters except page_size must match the request that produced it. + string page_token = 2; // Optional label selector in key=value comma-separated form. string label_selector = 5; // Explicit named or all-workspaces scope. @@ -1177,6 +1181,8 @@ message SandboxTemplateResponse { message ListSandboxTemplatesResponse { repeated SandboxWorkloadTemplate templates = 1; + // Token for the next page. Empty when there are no subsequent pages. + string next_page_token = 2; } message DeleteSandboxTemplateResponse { @@ -1226,8 +1232,12 @@ message GetSandboxRequest { message ListSandboxesRequest { reserved 4, 5; reserved "workspace", "all_workspaces"; - uint32 limit = 1; - uint32 offset = 2; + // The maximum number of sandboxes to return. Zero uses 100. Values above + // 1000 are coerced to 1000; negative values are invalid. + int32 page_size = 1; + // Token from a previous ListSandboxes response. All other request parameters + // except page_size must match the request that produced it. + string page_token = 2; // Optional label selector for filtering (format: "key1=value1,key2=value2"). string label_selector = 3; // Explicit named or all-workspaces scope. @@ -1316,6 +1326,8 @@ message SandboxResponse { // List sandboxes response. message ListSandboxesResponse { repeated Sandbox sandboxes = 1; + // Token for the next page. Empty when there are no subsequent pages. + string next_page_token = 2; } // List providers attached to a sandbox response. @@ -1416,10 +1428,12 @@ message ListServicesRequest { reserved "workspace", "all_workspaces"; // Optional sandbox name. Empty lists endpoints for all sandboxes. string sandbox = 1; - // Page size. Zero uses the server default. - uint32 limit = 2; - // Page offset. - uint32 offset = 3; + // The maximum number of services to return. Zero uses 100. Values above + // 1000 are coerced to 1000; negative values are invalid. + int32 page_size = 2; + // Token from a previous ListServices response. All other request parameters + // except page_size must match the request that produced it. + string page_token = 3; // Explicit named or all-workspaces scope. openshell.datamodel.v1.WorkspaceSelector workspace_scope = 6; } @@ -1427,6 +1441,8 @@ message ListServicesRequest { // Response containing exposed sandbox service endpoints. message ListServicesResponse { repeated ServiceEndpointResponse services = 1; + // Token for the next page. Empty when there are no subsequent pages. + string next_page_token = 2; } // Request to delete an exposed sandbox service endpoint. @@ -1695,8 +1711,12 @@ message GetProviderRequest { message ListProvidersRequest { reserved 3, 4; reserved "workspace", "all_workspaces"; - uint32 limit = 1; - uint32 offset = 2; + // The maximum number of providers to return. Zero uses 100. Values above + // 1000 are coerced to 1000; negative values are invalid. + int32 page_size = 1; + // Token from a previous ListProviders response. All other request parameters + // except page_size must match the request that produced it. + string page_token = 2; // Explicit named or all-workspaces scope. openshell.datamodel.v1.WorkspaceSelector workspace_scope = 5; } @@ -1730,12 +1750,18 @@ message ProviderResponse { // List providers response. message ListProvidersResponse { repeated openshell.datamodel.v1.Provider providers = 1; + // Token for the next page. Empty when there are no subsequent pages. + string next_page_token = 2; } // List provider type profiles request. message ListProviderProfilesRequest { - uint32 limit = 1; - uint32 offset = 2; + // The maximum number of profiles to return. Zero uses 100. Values above + // 1000 are coerced to 1000; negative values are invalid. + int32 page_size = 1; + // Token from a previous ListProviderProfiles response. All other request + // parameters except page_size must match the request that produced it. + string page_token = 2; // Workspace scope. When set, returns workspace-scoped + built-in profiles. // When empty, returns platform-scoped + built-in only. string workspace = 3; @@ -2027,6 +2053,8 @@ message ProviderProfileResponse { // List provider profiles response. message ListProviderProfilesResponse { repeated ProviderProfile profiles = 1; + // Token for the next page. Empty when there are no subsequent pages. + string next_page_token = 2; } // Import custom provider profiles request. @@ -2308,8 +2336,12 @@ message ListSandboxPoliciesRequest { reserved "workspace"; // Sandbox name (canonical lookup key). Ignored when global is true. string name = 1; - uint32 limit = 2; - uint32 offset = 3; + // The maximum number of revisions to return. Zero uses 100. Values above + // 1000 are coerced to 1000; negative values are invalid. + int32 page_size = 2; + // Token from a previous ListSandboxPolicies response. All other request + // parameters except page_size must match the request that produced it. + string page_token = 3; // List global policy revisions instead of sandbox-scoped ones. bool global = 4; // Explicit workspace scope for sandbox-scoped queries. Omit only when @@ -2322,6 +2354,8 @@ message ListSandboxPoliciesResponse { // Invalid historical payloads remain visible as failed projections so one // legacy row cannot hide the rest of the policy history. repeated SandboxPolicyRevision revisions = 1; + // Token for the next page. Empty when there are no subsequent pages. + string next_page_token = 2; } // Report policy load status (called by sandbox runtime after reload attempt). @@ -2949,8 +2983,12 @@ message GetWorkspaceResponse { // List workspaces request. message ListWorkspacesRequest { - uint32 limit = 1; - uint32 offset = 2; + // The maximum number of workspaces to return. Zero uses 100. Values above + // 1000 are coerced to 1000; negative values are invalid. + int32 page_size = 1; + // Token from a previous ListWorkspaces response. All other request parameters + // except page_size must match the request that produced it. + string page_token = 2; // Optional label selector for filtering (format: "key1=value1,key2=value2"). string label_selector = 3; } @@ -2958,6 +2996,8 @@ message ListWorkspacesRequest { // List workspaces response. message ListWorkspacesResponse { repeated openshell.datamodel.v1.Workspace workspaces = 1; + // Token for the next page. Empty when there are no subsequent pages. + string next_page_token = 2; } // Delete workspace request. @@ -3035,13 +3075,19 @@ message RemoveWorkspaceMemberResponse { message ListWorkspaceMembersRequest { // Workspace name. string workspace = 1; - uint32 limit = 2; - uint32 offset = 3; + // The maximum number of members to return. Zero uses 100. Values above + // 1000 are coerced to 1000; negative values are invalid. + int32 page_size = 2; + // Token from a previous ListWorkspaceMembers response. All other request + // parameters except page_size must match the request that produced it. + string page_token = 3; } // List workspace members response. message ListWorkspaceMembersResponse { repeated WorkspaceMember members = 1; + // Token for the next page. Empty when there are no subsequent pages. + string next_page_token = 2; } // Short-lived credential for one policy-authorized extension service. diff --git a/proto/pagination.proto b/proto/pagination.proto new file mode 100644 index 0000000000..05fcaef17f --- /dev/null +++ b/proto/pagination.proto @@ -0,0 +1,40 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +syntax = "proto3"; + +package openshell.internal.pagination.v1; + +// Private, versioned state carried by an opaque public page token. +// +// This message is an implementation detail. Public list RPCs expose only the +// AIP-158 string page_token and next_page_token fields. +message PageToken { + uint32 version = 1; + string method = 2; + bytes request_fingerprint = 3; + + oneof cursor { + ObjectCursor object = 4; + PolicyCursor policy = 5; + ProfileCursor profile = 6; + } +} + +// Cursor for the total object-store order. +message ObjectCursor { + int64 created_at_ms = 1; + string name = 2; + string workspace = 3; + string id = 4; +} + +// Cursor for descending policy revision order. +message PolicyCursor { + int64 version = 1; +} + +// Cursor for a deterministically ordered provider-profile catalog. +message ProfileCursor { + string key = 1; +} diff --git a/python/openshell/sandbox.py b/python/openshell/sandbox.py index 343d75edcc..cfffdf6307 100644 --- a/python/openshell/sandbox.py +++ b/python/openshell/sandbox.py @@ -819,49 +819,61 @@ def list( self, *, workspace: str, - limit: int = 100, - offset: int = 0, + page_size: int = 100, label_selector: str | None = None, ) -> builtins.list[SandboxRef]: - request = openshell_pb2.ListSandboxesRequest( - workspace_scope=_workspace_scope(workspace), - limit=limit, - offset=offset, - label_selector=label_selector or "", - ) - response = self._stub.ListSandboxes(request, timeout=self._timeout) - return [_sandbox_ref(item) for item in response.sandboxes] + sandboxes: builtins.list[SandboxRef] = [] + page_token = "" + while True: + response = self._stub.ListSandboxes( + openshell_pb2.ListSandboxesRequest( + workspace_scope=_workspace_scope(workspace), + page_size=page_size, + page_token=page_token, + label_selector=label_selector or "", + ), + timeout=self._timeout, + ) + sandboxes.extend(_sandbox_ref(item) for item in response.sandboxes) + if not getattr(response, "next_page_token", ""): + return sandboxes + page_token = response.next_page_token def list_for_all_workspaces( self, *, - limit: int = 100, - offset: int = 0, + page_size: int = 100, label_selector: str | None = None, ) -> builtins.list[SandboxRef]: - request = openshell_pb2.ListSandboxesRequest( - workspace_scope=_all_workspaces_scope(), - limit=limit, - offset=offset, - label_selector=label_selector or "", - ) - response = self._stub.ListSandboxes(request, timeout=self._timeout) - return [_sandbox_ref(item) for item in response.sandboxes] + sandboxes: builtins.list[SandboxRef] = [] + page_token = "" + while True: + response = self._stub.ListSandboxes( + openshell_pb2.ListSandboxesRequest( + workspace_scope=_all_workspaces_scope(), + page_size=page_size, + page_token=page_token, + label_selector=label_selector or "", + ), + timeout=self._timeout, + ) + sandboxes.extend(_sandbox_ref(item) for item in response.sandboxes) + if not getattr(response, "next_page_token", ""): + return sandboxes + page_token = response.next_page_token def list_ids( self, *, workspace: str, - limit: int = 100, - offset: int = 0, + page_size: int = 100, label_selector: str | None = None, ) -> builtins.list[str]: return [ item.id for item in self.list( workspace=workspace, - limit=limit, - offset=offset, + page_size=page_size, label_selector=label_selector, ) ] @@ -869,15 +881,13 @@ def list_ids( def list_ids_for_all_workspaces( self, *, - limit: int = 100, - offset: int = 0, + page_size: int = 100, label_selector: str | None = None, ) -> builtins.list[str]: return [ item.id for item in self.list_for_all_workspaces( - limit=limit, - offset=offset, + page_size=page_size, label_selector=label_selector, ) ] @@ -1181,38 +1191,48 @@ def list( self, *, workspace: str, - limit: int = 100, - offset: int = 0, + page_size: int = 100, label_selector: str = "", ) -> builtins.list[openshell_pb2.SandboxWorkloadTemplate]: - response = self._stub.ListSandboxTemplates( - openshell_pb2.ListSandboxTemplatesRequest( - workspace_scope=_workspace_scope(workspace), - limit=limit, - offset=offset, - label_selector=label_selector, - ), - timeout=self._timeout, - ) - return list(response.templates) + templates: builtins.list[openshell_pb2.SandboxWorkloadTemplate] = [] + page_token = "" + while True: + response = self._stub.ListSandboxTemplates( + openshell_pb2.ListSandboxTemplatesRequest( + workspace_scope=_workspace_scope(workspace), + page_size=page_size, + page_token=page_token, + label_selector=label_selector, + ), + timeout=self._timeout, + ) + templates.extend(response.templates) + if not getattr(response, "next_page_token", ""): + return templates + page_token = response.next_page_token def list_for_all_workspaces( self, *, - limit: int = 100, - offset: int = 0, + page_size: int = 100, label_selector: str = "", ) -> builtins.list[openshell_pb2.SandboxWorkloadTemplate]: - response = self._stub.ListSandboxTemplates( - openshell_pb2.ListSandboxTemplatesRequest( - workspace_scope=_all_workspaces_scope(), - limit=limit, - offset=offset, - label_selector=label_selector, - ), - timeout=self._timeout, - ) - return list(response.templates) + templates: builtins.list[openshell_pb2.SandboxWorkloadTemplate] = [] + page_token = "" + while True: + response = self._stub.ListSandboxTemplates( + openshell_pb2.ListSandboxTemplatesRequest( + workspace_scope=_all_workspaces_scope(), + page_size=page_size, + page_token=page_token, + label_selector=label_selector, + ), + timeout=self._timeout, + ) + templates.extend(response.templates) + if not getattr(response, "next_page_token", ""): + return templates + page_token = response.next_page_token def delete(self, name: str, *, workspace: str) -> bool: response = self._stub.DeleteSandboxTemplate( @@ -1276,19 +1296,24 @@ def get(self, name: str) -> WorkspaceRef: def list( self, *, - limit: int = 100, - offset: int = 0, + page_size: int = 100, label_selector: str | None = None, ) -> builtins.list[WorkspaceRef]: - response = self._stub.ListWorkspaces( - openshell_pb2.ListWorkspacesRequest( - limit=limit, - offset=offset, - label_selector=label_selector or "", - ), - timeout=self._timeout, - ) - return [_workspace_ref(ws) for ws in response.workspaces] + workspaces: builtins.list[WorkspaceRef] = [] + page_token = "" + while True: + response = self._stub.ListWorkspaces( + openshell_pb2.ListWorkspacesRequest( + page_size=page_size, + page_token=page_token, + label_selector=label_selector or "", + ), + timeout=self._timeout, + ) + workspaces.extend(_workspace_ref(ws) for ws in response.workspaces) + if not getattr(response, "next_page_token", ""): + return workspaces + page_token = response.next_page_token def delete(self, name: str) -> bool: response = self._stub.DeleteWorkspace( diff --git a/python/openshell/sandbox_test.py b/python/openshell/sandbox_test.py index 772a327239..e436f714db 100644 --- a/python/openshell/sandbox_test.py +++ b/python/openshell/sandbox_test.py @@ -1963,7 +1963,11 @@ def _make_workload_template_proto( class _FakeSandboxStub: - def __init__(self, listed: list[openshell_pb2.Sandbox] | None = None) -> None: + def __init__( + self, + listed: list[openshell_pb2.Sandbox] | None = None, + listed_pages: list[list[openshell_pb2.Sandbox]] | None = None, + ) -> None: self.create_request: openshell_pb2.CreateSandboxRequest | None = None self.list_request: openshell_pb2.ListSandboxesRequest | None = None self.get_request: openshell_pb2.GetSandboxRequest | None = None @@ -1981,6 +1985,8 @@ def __init__(self, listed: list[openshell_pb2.Sandbox] | None = None) -> None: openshell_pb2.DeleteSandboxTemplateRequest | None ) = None self._listed = listed or [] + self._listed_pages = listed_pages + self.list_requests: list[openshell_pb2.ListSandboxesRequest] = [] self._templates: list[openshell_pb2.SandboxWorkloadTemplate] = [] def GetSandbox( @@ -2061,7 +2067,17 @@ def ListSandboxes( timeout: float | None = None, ) -> Any: self.list_request = request + self.list_requests.append(deepcopy(request)) _ = timeout + if self._listed_pages is not None: + page = int(request.page_token or "0") + next_page_token = ( + str(page + 1) if page + 1 < len(self._listed_pages) else "" + ) + return SimpleNamespace( + sandboxes=list(self._listed_pages[page]), + next_page_token=next_page_token, + ) return SimpleNamespace(sandboxes=list(self._listed)) def CreateSandboxTemplate( @@ -2393,13 +2409,13 @@ def test_sandbox_template_client_crud_forwards_requests() -> None: assert _request_workspace(stub.get_template_request) == "default" listed = client.list( - workspace="default", limit=50, offset=10, label_selector="team=runtime" + workspace="default", page_size=50, label_selector="team=runtime" ) assert len(listed) == 1 assert stub.list_template_request is not None assert _request_workspace(stub.list_template_request) == "default" - assert stub.list_template_request.limit == 50 - assert stub.list_template_request.offset == 10 + assert stub.list_template_request.page_size == 50 + assert stub.list_template_request.page_token == "" assert stub.list_template_request.label_selector == "team=runtime" assert not _request_selects_all_workspaces(stub.list_template_request) @@ -2413,13 +2429,13 @@ def test_sandbox_template_list_for_all_workspaces_selects_all() -> None: stub = _FakeSandboxStub() client = _template_client_with_fake_stub(stub) - client.list_for_all_workspaces(limit=100, offset=5, label_selector="team=runtime") + client.list_for_all_workspaces(page_size=100, label_selector="team=runtime") assert stub.list_template_request is not None assert _request_selects_all_workspaces(stub.list_template_request) assert _request_workspace(stub.list_template_request) is None - assert stub.list_template_request.limit == 100 - assert stub.list_template_request.offset == 5 + assert stub.list_template_request.page_size == 100 + assert stub.list_template_request.page_token == "" assert stub.list_template_request.label_selector == "team=runtime" @@ -2534,6 +2550,26 @@ def test_list_without_selector_sends_empty_string() -> None: assert stub.list_request.label_selector == "" +def test_list_follows_continuation_tokens() -> None: + stub = _FakeSandboxStub( + listed_pages=[ + [_make_sandbox_proto("sandbox-1", "job-1")], + [_make_sandbox_proto("sandbox-2", "job-2")], + ] + ) + client = _client_with_fake_stub(stub) + + sandboxes = client.list( + workspace="default", page_size=1, label_selector="team=core" + ) + + assert [sandbox.name for sandbox in sandboxes] == ["job-1", "job-2"] + assert len(stub.list_requests) == 2 + assert stub.list_requests[0].page_token == "" + assert stub.list_requests[1].page_token == "1" + assert stub.list_requests[1].label_selector == "team=core" + + def test_list_ids_forwards_label_selector() -> None: stub = _FakeSandboxStub(listed=[_make_sandbox_proto("sandbox-1", "job-1")]) client = _client_with_fake_stub(stub) diff --git a/sdk/go/docs/src/api/providers.md b/sdk/go/docs/src/api/providers.md index 87c82379a1..f9b1a8c967 100644 --- a/sdk/go/docs/src/api/providers.md +++ b/sdk/go/docs/src/api/providers.md @@ -42,7 +42,8 @@ fmt.Println("Provider type:", provider.Type) ## List -List all registered providers, with optional pagination. +List all registered providers. The SDK follows gateway continuation tokens +automatically; `PageSize` controls each request. ```go // List all providers @@ -54,10 +55,9 @@ for _, p := range providers { fmt.Println(p.Name, p.Type) } -// With pagination +// With a smaller page size providers, err = client.Providers().List(ctx, "default", v1.ListOptions{ - Limit: 10, - Offset: 0, + PageSize: 10, }) // Platform Admin only: list across all workspaces diff --git a/sdk/go/docs/src/api/sandbox-templates.md b/sdk/go/docs/src/api/sandbox-templates.md index 03e8ae43e8..e955dc1f29 100644 --- a/sdk/go/docs/src/api/sandbox-templates.md +++ b/sdk/go/docs/src/api/sandbox-templates.md @@ -89,16 +89,16 @@ fmt.Println(template.Spec.Workload.Image) ## List -Lists templates in one workspace or across all workspaces. +Lists every matching template in one workspace or across all workspaces. The +SDK follows gateway continuation tokens automatically. ```go templates, err := client.SandboxTemplates().List(ctx, "default", v1.ListOptions{ - Limit: 50, - Offset: 0, + PageSize: 50, }) allTemplates, err := client.SandboxTemplates().ListAll(ctx, v1.ListOptions{ - Limit: 50, + PageSize: 50, }) ``` diff --git a/sdk/go/docs/src/api/sandboxes.md b/sdk/go/docs/src/api/sandboxes.md index af026a17cb..e9ab530690 100644 --- a/sdk/go/docs/src/api/sandboxes.md +++ b/sdk/go/docs/src/api/sandboxes.md @@ -57,16 +57,16 @@ fmt.Println(sb.Status.Phase) // "Ready", "Provisioning", etc. ## List -Lists sandboxes with optional pagination and label filtering. +Lists every matching sandbox, following gateway continuation tokens +automatically. `PageSize` controls the size of each request. ```go // List all sandboxes sandboxes, err := client.Sandboxes().List(ctx, "default") -// With pagination and label filtering +// With a page size and label filtering sandboxes, err := client.Sandboxes().List(ctx, "default", v1.ListOptions{ - Limit: 10, - Offset: 0, + PageSize: 10, LabelSelector: "team=platform", }) diff --git a/sdk/go/openshell/v1/fake/policy.go b/sdk/go/openshell/v1/fake/policy.go index 7360af1b03..e69c7bad8e 100644 --- a/sdk/go/openshell/v1/fake/policy.go +++ b/sdk/go/openshell/v1/fake/policy.go @@ -179,6 +179,9 @@ func (c *fakePolicyClient) List(_ context.Context, workspace string, opts ...v1. return nil, &types.StatusError{Code: types.ErrorUnavailable, Message: "client is closed"} } cfg := types.ApplyListPolicyOptions(opts) + if cfg.PageSize() < 0 { + return nil, &types.StatusError{Code: types.ErrorInvalidArgument, Message: "page size must not be negative"} + } c.mu.RLock() defer c.mu.RUnlock() @@ -211,17 +214,6 @@ func (c *fakePolicyClient) List(_ context.Context, workspace string, opts ...v1. return 0 }) - // Apply pagination. - offset := int(cfg.Offset()) - if offset >= len(revisions) { - return nil, nil - } - revisions = revisions[offset:] - - if limit := int(cfg.Limit()); limit > 0 && limit < len(revisions) { - revisions = revisions[:limit] - } - result := make([]types.SandboxPolicyRevision, len(revisions)) for i, r := range revisions { result[i] = copySandboxPolicyRevision(r) diff --git a/sdk/go/openshell/v1/fake/policy_test.go b/sdk/go/openshell/v1/fake/policy_test.go index f48398a137..d2ff4e3b0e 100644 --- a/sdk/go/openshell/v1/fake/policy_test.go +++ b/sdk/go/openshell/v1/fake/policy_test.go @@ -132,26 +132,19 @@ func TestFakePolicy_List_NoIsolationCrossContamination(t *testing.T) { assert.Nil(t, revisions) } -func TestFakePolicy_List_GlobalWithPagination(t *testing.T) { +func TestFakePolicy_List_GlobalPageSizeStillReturnsAll(t *testing.T) { c := newFakePolicyClient(func() bool { return false }) c.AddGlobalRevision(types.SandboxPolicyRevision{Version: 1}) c.AddGlobalRevision(types.SandboxPolicyRevision{Version: 2}) c.AddGlobalRevision(types.SandboxPolicyRevision{Version: 3}) - // Limit to 2. - revisions, err := c.List(context.Background(), "", types.WithListGlobal(true), types.WithLimit(2)) + revisions, err := c.List(context.Background(), "", types.WithListGlobal(true), types.WithPageSize(2)) require.NoError(t, err) - require.Len(t, revisions, 2) + require.Len(t, revisions, 3) assert.Equal(t, uint32(1), revisions[0].Version) assert.Equal(t, uint32(2), revisions[1].Version) - - // Offset by 1, limit 2. - revisions, err = c.List(context.Background(), "", types.WithListGlobal(true), types.WithLimit(2), types.WithOffset(1)) - require.NoError(t, err) - require.Len(t, revisions, 2) - assert.Equal(t, uint32(2), revisions[0].Version) - assert.Equal(t, uint32(3), revisions[1].Version) + assert.Equal(t, uint32(3), revisions[2].Version) } func TestFakePolicy_GetStatus_Sandbox(t *testing.T) { diff --git a/sdk/go/openshell/v1/fake/sandbox_template.go b/sdk/go/openshell/v1/fake/sandbox_template.go index b105de7c13..f58f8f6eaf 100644 --- a/sdk/go/openshell/v1/fake/sandbox_template.go +++ b/sdk/go/openshell/v1/fake/sandbox_template.go @@ -120,11 +120,8 @@ func (c *fakeSandboxTemplateClient) list(workspace string, allWorkspaces bool, o var options v1.ListOptions if len(opts) > 0 { options = opts[0] - if options.Limit < 0 { - return nil, &types.StatusError{Code: types.ErrorInvalidArgument, Message: "limit must not be negative"} - } - if options.Offset < 0 { - return nil, &types.StatusError{Code: types.ErrorInvalidArgument, Message: "offset must not be negative"} + if options.PageSize < 0 { + return nil, &types.StatusError{Code: types.ErrorInvalidArgument, Message: "page size must not be negative"} } } var templates []*types.SandboxWorkloadTemplate @@ -138,7 +135,7 @@ func (c *fakeSandboxTemplateClient) list(workspace string, allWorkspaces bool, o if err != nil { return nil, err } - return paginateSandboxWorkloadTemplates(templates, options), nil + return templates, nil } func (c *fakeSandboxTemplateClient) Delete(_ context.Context, workspace, name string) (bool, error) { @@ -248,20 +245,6 @@ func compareSandboxWorkloadTemplatesForList(a, b *types.SandboxWorkloadTemplate) return 0 } -func paginateSandboxWorkloadTemplates( - templates []*types.SandboxWorkloadTemplate, - options v1.ListOptions, -) []*types.SandboxWorkloadTemplate { - if options.Offset >= len(templates) { - return templates[:0] - } - templates = templates[options.Offset:] - if options.Limit > 0 && options.Limit < len(templates) { - return templates[:options.Limit] - } - return templates -} - func isDNS1123Label(name string) bool { if len(name) == 0 || len(name) > 63 || name[0] == '-' || name[len(name)-1] == '-' { return false diff --git a/sdk/go/openshell/v1/fake/sandbox_template_test.go b/sdk/go/openshell/v1/fake/sandbox_template_test.go index 6caa0da684..1df35f8436 100644 --- a/sdk/go/openshell/v1/fake/sandbox_template_test.go +++ b/sdk/go/openshell/v1/fake/sandbox_template_test.go @@ -143,16 +143,12 @@ func TestSandboxTemplate_ListRejectsNegativePagination(t *testing.T) { tc := newTestSandboxTemplateClient() ctx := context.Background() - _, err := tc.List(ctx, "default", types.ListOptions{Limit: -1}) - require.Error(t, err) - assert.True(t, types.IsInvalidArgument(err)) - - _, err = tc.List(ctx, "default", types.ListOptions{Offset: -1}) + _, err := tc.List(ctx, "default", types.ListOptions{PageSize: -1}) require.Error(t, err) assert.True(t, types.IsInvalidArgument(err)) } -func TestSandboxTemplate_ListAppliesPaginationAfterFiltering(t *testing.T) { +func TestSandboxTemplate_ListReturnsAllFilteredResults(t *testing.T) { tc := newTestSandboxTemplateClient() ctx := context.Background() @@ -180,21 +176,13 @@ func TestSandboxTemplate_ListAppliesPaginationAfterFiltering(t *testing.T) { listed, err := tc.List(ctx, "default", types.ListOptions{ LabelSelector: "team=runtime", - Offset: 1, - Limit: 1, - }) - - require.NoError(t, err) - require.Len(t, listed, 1) - assert.Equal(t, "runtime-b", listed[0].Name) - - listed, err = tc.List(ctx, "default", types.ListOptions{ - LabelSelector: "team=runtime", - Offset: 2, + PageSize: 1, }) require.NoError(t, err) - assert.Empty(t, listed) + require.Len(t, listed, 2) + assert.Equal(t, "runtime-a", listed[0].Name) + assert.Equal(t, "runtime-b", listed[1].Name) } func TestSandboxTemplate_CreateSandboxFromTemplateRequiresExistingTemplate(t *testing.T) { diff --git a/sdk/go/openshell/v1/options.go b/sdk/go/openshell/v1/options.go index caac82c96b..78a6179a8f 100644 --- a/sdk/go/openshell/v1/options.go +++ b/sdk/go/openshell/v1/options.go @@ -4,6 +4,8 @@ package v1 import ( + "math" + "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/types" ) @@ -21,3 +23,16 @@ type WaitOptions = types.WaitOptions // ExecOptions configures command execution. type ExecOptions = types.ExecOptions + +func listPageSize(opts []ListOptions) (int32, error) { + if len(opts) == 0 { + return 0, nil + } + if opts[0].PageSize < 0 { + return 0, &StatusError{Code: ErrorInvalidArgument, Message: "page size must not be negative"} + } + if opts[0].PageSize > math.MaxInt32 { + return math.MaxInt32, nil + } + return int32(opts[0].PageSize), nil +} diff --git a/sdk/go/openshell/v1/policy.go b/sdk/go/openshell/v1/policy.go index cbe2e8621a..a6025074df 100644 --- a/sdk/go/openshell/v1/policy.go +++ b/sdk/go/openshell/v1/policy.go @@ -87,11 +87,8 @@ var WithVersion = types.WithVersion // ListPolicyOption configures a List call. type ListPolicyOption = types.ListPolicyOption -// WithLimit sets the maximum number of revisions to return. -var WithLimit = types.WithLimit - -// WithOffset sets the pagination offset. -var WithOffset = types.WithOffset +// WithPageSize sets the page size used while collecting every revision. +var WithPageSize = types.WithPageSize // WithListGlobal enables global policy mode on List. When true, the query // retrieves gateway-global policy revisions instead of sandbox-scoped ones. diff --git a/sdk/go/openshell/v1/policy_client.go b/sdk/go/openshell/v1/policy_client.go index 12801af02a..1dec3fad7e 100644 --- a/sdk/go/openshell/v1/policy_client.go +++ b/sdk/go/openshell/v1/policy_client.go @@ -131,29 +131,32 @@ func (p *policyClient) GetStatus(ctx context.Context, workspace, sandboxName str func (p *policyClient) List(ctx context.Context, workspace string, opts ...ListPolicyOption) ([]SandboxPolicyRevision, error) { cfg := types.ApplyListPolicyOptions(opts) + if cfg.PageSize() < 0 { + return nil, &StatusError{Code: ErrorInvalidArgument, Message: "page size must not be negative"} + } req := &pb.ListSandboxPoliciesRequest{ - Limit: cfg.Limit(), - Offset: cfg.Offset(), - Global: cfg.Global(), + PageSize: cfg.PageSize(), + Global: cfg.Global(), } if !cfg.Global() { req.WorkspaceScope = namedWorkspaceScope(workspace) } - resp, err := p.client.ListSandboxPolicies(ctx, req) - if err != nil { - return nil, converter.FromGRPCError(err) - } - revisions := resp.GetRevisions() - if len(revisions) == 0 { - return nil, nil - } - result := make([]SandboxPolicyRevision, 0, len(revisions)) - for _, r := range revisions { - if converted := converter.SandboxPolicyRevisionFromProto(r); converted != nil { - result = append(result, *converted) + var result []SandboxPolicyRevision + for { + resp, err := p.client.ListSandboxPolicies(ctx, req) + if err != nil { + return nil, converter.FromGRPCError(err) + } + for _, revision := range resp.GetRevisions() { + if converted := converter.SandboxPolicyRevisionFromProto(revision); converted != nil { + result = append(result, *converted) + } } + if resp.GetNextPageToken() == "" { + return result, nil + } + req.PageToken = resp.GetNextPageToken() } - return result, nil } func (p *policyClient) EditDraftChunk(ctx context.Context, workspace, sandboxName, chunkID string, proposedRule *NetworkPolicyRule) error { diff --git a/sdk/go/openshell/v1/policy_client_test.go b/sdk/go/openshell/v1/policy_client_test.go index a0e16f5a40..ebb2491ea3 100644 --- a/sdk/go/openshell/v1/policy_client_test.go +++ b/sdk/go/openshell/v1/policy_client_test.go @@ -781,8 +781,8 @@ func TestPolicyList(t *testing.T) { // Verify request was forwarded (no pagination options). mock.mu.Lock() assert.Equal(t, "default", mock.lastListReq.GetWorkspaceScope().GetWorkspace()) - assert.Equal(t, uint32(0), mock.lastListReq.GetLimit()) - assert.Equal(t, uint32(0), mock.lastListReq.GetOffset()) + assert.Equal(t, int32(0), mock.lastListReq.GetPageSize()) + assert.Empty(t, mock.lastListReq.GetPageToken()) mock.mu.Unlock() assert.Equal(t, uint32(1), revisions[0].Version) @@ -794,7 +794,7 @@ func TestPolicyList(t *testing.T) { assert.Equal(t, PolicyLoadStatusLoaded, revisions[1].Status) } -func TestPolicyList_WithPagination(t *testing.T) { +func TestPolicyList_WithPageSize(t *testing.T) { mock := newMockPolicyServer() mock.listResp = &pb.ListSandboxPoliciesResponse{ Revisions: []*pb.SandboxPolicyRevision{ @@ -806,17 +806,16 @@ func TestPolicyList_WithPagination(t *testing.T) { defer cleanup() revisions, err := client.List(context.Background(), "default", - types.WithLimit(10), - types.WithOffset(20), + types.WithPageSize(10), ) require.NoError(t, err) require.Len(t, revisions, 1) - // Verify pagination options were forwarded. + // Verify page size was forwarded and the initial token is empty. mock.mu.Lock() - assert.Equal(t, uint32(10), mock.lastListReq.GetLimit()) - assert.Equal(t, uint32(20), mock.lastListReq.GetOffset()) + assert.Equal(t, int32(10), mock.lastListReq.GetPageSize()) + assert.Empty(t, mock.lastListReq.GetPageToken()) mock.mu.Unlock() } @@ -892,11 +891,10 @@ func TestPolicyList_WithGlobalAndPagination(t *testing.T) { client, cleanup := setupPolicyTest(t, mock) defer cleanup() - // Global flag composes with pagination options. + // Global flag composes with page-size options. revisions, err := client.List(context.Background(), "", types.WithListGlobal(true), - types.WithLimit(10), - types.WithOffset(20), + types.WithPageSize(10), ) require.NoError(t, err) @@ -904,8 +902,8 @@ func TestPolicyList_WithGlobalAndPagination(t *testing.T) { mock.mu.Lock() assert.True(t, mock.lastListReq.GetGlobal()) - assert.Equal(t, uint32(10), mock.lastListReq.GetLimit()) - assert.Equal(t, uint32(20), mock.lastListReq.GetOffset()) + assert.Equal(t, int32(10), mock.lastListReq.GetPageSize()) + assert.Empty(t, mock.lastListReq.GetPageToken()) mock.mu.Unlock() } diff --git a/sdk/go/openshell/v1/profile_client.go b/sdk/go/openshell/v1/profile_client.go index 67f2d36a4e..11808c2faa 100644 --- a/sdk/go/openshell/v1/profile_client.go +++ b/sdk/go/openshell/v1/profile_client.go @@ -20,30 +20,28 @@ func newProfileClient(conn grpc.ClientConnInterface) *profileClient { } func (p *profileClient) List(ctx context.Context, workspace string, opts ...ListOptions) ([]*ProviderProfile, error) { + pageSize, err := listPageSize(opts) + if err != nil { + return nil, err + } req := &pb.ListProviderProfilesRequest{ Workspace: workspace, + PageSize: pageSize, } - if len(opts) > 0 { - if opts[0].Limit < 0 { - return nil, &StatusError{Code: ErrorInvalidArgument, Message: "limit must not be negative"} + profiles := make([]*ProviderProfile, 0) + for { + resp, err := p.client.ListProviderProfiles(ctx, req) + if err != nil { + return nil, converter.FromGRPCError(err) } - if opts[0].Offset < 0 { - return nil, &StatusError{Code: ErrorInvalidArgument, Message: "offset must not be negative"} + for _, profile := range resp.GetProfiles() { + profiles = append(profiles, converter.ProviderProfileFromProto(profile)) } - req.Limit = uint32(opts[0].Limit) - req.Offset = uint32(opts[0].Offset) - } - - resp, err := p.client.ListProviderProfiles(ctx, req) - if err != nil { - return nil, converter.FromGRPCError(err) - } - - profiles := make([]*ProviderProfile, 0, len(resp.GetProfiles())) - for _, pp := range resp.GetProfiles() { - profiles = append(profiles, converter.ProviderProfileFromProto(pp)) + if resp.GetNextPageToken() == "" { + return profiles, nil + } + req.PageToken = resp.GetNextPageToken() } - return profiles, nil } func (p *profileClient) Get(ctx context.Context, workspace, id string) (*ProviderProfile, error) { diff --git a/sdk/go/openshell/v1/profile_client_test.go b/sdk/go/openshell/v1/profile_client_test.go index b071038c54..979fe5bbad 100644 --- a/sdk/go/openshell/v1/profile_client_test.go +++ b/sdk/go/openshell/v1/profile_client_test.go @@ -233,6 +233,7 @@ func TestProfileList_Empty(t *testing.T) { profiles, err := client.List(context.Background(), "default") require.NoError(t, err) + assert.NotNil(t, profiles) assert.Empty(t, profiles) } @@ -242,13 +243,13 @@ func TestProfileList_WithOptions(t *testing.T) { client, cleanup := setupProfileTest(t, mock) defer cleanup() - profiles, err := client.List(context.Background(), "default", ListOptions{Limit: 10, Offset: 5}) + profiles, err := client.List(context.Background(), "default", ListOptions{PageSize: 10}) require.NoError(t, err) assert.Len(t, profiles, 1) require.NotNil(t, mock.lastListReq) - assert.Equal(t, uint32(10), mock.lastListReq.GetLimit()) - assert.Equal(t, uint32(5), mock.lastListReq.GetOffset()) + assert.Equal(t, int32(10), mock.lastListReq.GetPageSize()) + assert.Empty(t, mock.lastListReq.GetPageToken()) } func TestProfileList_Error(t *testing.T) { diff --git a/sdk/go/openshell/v1/provider_client.go b/sdk/go/openshell/v1/provider_client.go index 237325a1e3..fdfb4c0bc9 100644 --- a/sdk/go/openshell/v1/provider_client.go +++ b/sdk/go/openshell/v1/provider_client.go @@ -67,27 +67,26 @@ func (p *providerClient) ListAll(ctx context.Context, opts ...ListOptions) ([]*P } func (p *providerClient) list(ctx context.Context, req *pb.ListProvidersRequest, opts ...ListOptions) ([]*Provider, error) { - if len(opts) > 0 { - if opts[0].Limit < 0 { - return nil, &StatusError{Code: ErrorInvalidArgument, Message: "limit must not be negative"} - } - if opts[0].Offset < 0 { - return nil, &StatusError{Code: ErrorInvalidArgument, Message: "offset must not be negative"} - } - req.Limit = uint32(opts[0].Limit) - req.Offset = uint32(opts[0].Offset) - } - - resp, err := p.client.ListProviders(ctx, req) + pageSize, err := listPageSize(opts) if err != nil { - return nil, converter.FromGRPCError(err) + return nil, err } + req.PageSize = pageSize - providers := make([]*Provider, 0, len(resp.GetProviders())) - for _, proto := range resp.GetProviders() { - providers = append(providers, converter.ProviderFromProto(proto)) + providers := make([]*Provider, 0) + for { + resp, err := p.client.ListProviders(ctx, req) + if err != nil { + return nil, converter.FromGRPCError(err) + } + for _, proto := range resp.GetProviders() { + providers = append(providers, converter.ProviderFromProto(proto)) + } + if resp.GetNextPageToken() == "" { + return providers, nil + } + req.PageToken = resp.GetNextPageToken() } - return providers, nil } func (p *providerClient) Update(ctx context.Context, workspace string, provider *Provider) (*Provider, error) { diff --git a/sdk/go/openshell/v1/provider_client_test.go b/sdk/go/openshell/v1/provider_client_test.go index 55edddf16d..51830b7881 100644 --- a/sdk/go/openshell/v1/provider_client_test.go +++ b/sdk/go/openshell/v1/provider_client_test.go @@ -202,6 +202,7 @@ func TestProviderList_Empty(t *testing.T) { result, err := client.List(context.Background(), "default") require.NoError(t, err) + assert.NotNil(t, result) assert.Empty(t, result) } diff --git a/sdk/go/openshell/v1/sandbox_client.go b/sdk/go/openshell/v1/sandbox_client.go index 4028634e2d..187c826d8b 100644 --- a/sdk/go/openshell/v1/sandbox_client.go +++ b/sdk/go/openshell/v1/sandbox_client.go @@ -110,28 +110,29 @@ func (s *sandboxClient) ListAll(ctx context.Context, opts ...ListOptions) ([]*Sa } func (s *sandboxClient) list(ctx context.Context, req *pb.ListSandboxesRequest, opts ...ListOptions) ([]*Sandbox, error) { + pageSize, err := listPageSize(opts) + if err != nil { + return nil, err + } + req.PageSize = pageSize if len(opts) > 0 { - if opts[0].Limit < 0 { - return nil, &StatusError{Code: ErrorInvalidArgument, Message: "limit must not be negative"} - } - if opts[0].Offset < 0 { - return nil, &StatusError{Code: ErrorInvalidArgument, Message: "offset must not be negative"} - } - req.Limit = uint32(opts[0].Limit) - req.Offset = uint32(opts[0].Offset) req.LabelSelector = opts[0].LabelSelector } - resp, err := s.client.ListSandboxes(ctx, req) - if err != nil { - return nil, converter.FromGRPCError(err) - } - - sandboxes := make([]*Sandbox, 0, len(resp.GetSandboxes())) - for _, proto := range resp.GetSandboxes() { - sandboxes = append(sandboxes, converter.SandboxFromProto(proto)) + sandboxes := make([]*Sandbox, 0) + for { + resp, err := s.client.ListSandboxes(ctx, req) + if err != nil { + return nil, converter.FromGRPCError(err) + } + for _, proto := range resp.GetSandboxes() { + sandboxes = append(sandboxes, converter.SandboxFromProto(proto)) + } + if resp.GetNextPageToken() == "" { + return sandboxes, nil + } + req.PageToken = resp.GetNextPageToken() } - return sandboxes, nil } func (s *sandboxClient) Delete(ctx context.Context, workspace, name string) error { diff --git a/sdk/go/openshell/v1/sandbox_client_test.go b/sdk/go/openshell/v1/sandbox_client_test.go index 7926b28a16..53429c6d1b 100644 --- a/sdk/go/openshell/v1/sandbox_client_test.go +++ b/sdk/go/openshell/v1/sandbox_client_test.go @@ -30,6 +30,8 @@ type mockSandboxServer struct { createErr error getErr error listErr error + listPages [][]*pb.Sandbox + listRequests []*pb.ListSandboxesRequest deleteErr error attachErr error detachErr error @@ -98,12 +100,27 @@ func (s *mockSandboxServer) setPhase(name string, phase pb.SandboxPhase) { } } -func (s *mockSandboxServer) ListSandboxes(_ context.Context, _ *pb.ListSandboxesRequest) (*pb.ListSandboxesResponse, error) { +func (s *mockSandboxServer) ListSandboxes(_ context.Context, req *pb.ListSandboxesRequest) (*pb.ListSandboxesResponse, error) { s.mu.Lock() defer s.mu.Unlock() + s.listRequests = append(s.listRequests, proto.Clone(req).(*pb.ListSandboxesRequest)) if s.listErr != nil { return nil, s.listErr } + if s.listPages != nil { + page := 0 + if req.GetPageToken() == "page-2" { + page = 1 + } + nextPageToken := "" + if page+1 < len(s.listPages) { + nextPageToken = "page-2" + } + return &pb.ListSandboxesResponse{ + Sandboxes: s.listPages[page], + NextPageToken: nextPageToken, + }, nil + } var list []*pb.Sandbox for _, sb := range s.sandboxes { list = append(list, sb) @@ -416,6 +433,7 @@ func TestSandboxList_Empty(t *testing.T) { result, err := client.List(context.Background(), "default") require.NoError(t, err) + assert.NotNil(t, result) assert.Empty(t, result) } @@ -428,12 +446,37 @@ func TestSandboxList_WithOptions(t *testing.T) { client, cleanup := setupSandboxTest(t, mock) defer cleanup() - result, err := client.List(context.Background(), "default", ListOptions{Limit: 10, Offset: 0}) + result, err := client.List(context.Background(), "default", ListOptions{PageSize: 10}) require.NoError(t, err) assert.Len(t, result, 1) } +func TestSandboxList_FollowsContinuationTokens(t *testing.T) { + mock := newMockSandboxServer() + mock.listPages = [][]*pb.Sandbox{ + {{Metadata: &dm.ObjectMeta{Name: "first"}}}, + {{Metadata: &dm.ObjectMeta{Name: "second"}}}, + } + client, cleanup := setupSandboxTest(t, mock) + defer cleanup() + + result, err := client.List(context.Background(), "default", ListOptions{ + PageSize: 1, + LabelSelector: "team=core", + }) + + require.NoError(t, err) + require.Len(t, result, 2) + assert.Equal(t, "first", result[0].Name) + assert.Equal(t, "second", result[1].Name) + require.Len(t, mock.listRequests, 2) + assert.Empty(t, mock.listRequests[0].GetPageToken()) + assert.Equal(t, "page-2", mock.listRequests[1].GetPageToken()) + assert.Equal(t, int32(1), mock.listRequests[1].GetPageSize()) + assert.Equal(t, "team=core", mock.listRequests[1].GetLabelSelector()) +} + func TestSandboxDelete(t *testing.T) { mock := newMockSandboxServer() mock.sandboxes["deleteme"] = &pb.Sandbox{ diff --git a/sdk/go/openshell/v1/sandbox_template_client.go b/sdk/go/openshell/v1/sandbox_template_client.go index fd9ddb53e3..3b14e36e66 100644 --- a/sdk/go/openshell/v1/sandbox_template_client.go +++ b/sdk/go/openshell/v1/sandbox_template_client.go @@ -62,28 +62,29 @@ func (s *sandboxTemplateClient) ListAll(ctx context.Context, opts ...ListOptions } func (s *sandboxTemplateClient) list(ctx context.Context, req *pb.ListSandboxTemplatesRequest, opts ...ListOptions) ([]*SandboxWorkloadTemplate, error) { + pageSize, err := listPageSize(opts) + if err != nil { + return nil, err + } + req.PageSize = pageSize if len(opts) > 0 { - if opts[0].Limit < 0 { - return nil, &StatusError{Code: ErrorInvalidArgument, Message: "limit must not be negative"} - } - if opts[0].Offset < 0 { - return nil, &StatusError{Code: ErrorInvalidArgument, Message: "offset must not be negative"} - } - req.Limit = uint32(opts[0].Limit) - req.Offset = uint32(opts[0].Offset) req.LabelSelector = opts[0].LabelSelector } - resp, err := s.client.ListSandboxTemplates(ctx, req) - if err != nil { - return nil, converter.FromGRPCError(err) - } - - templates := make([]*SandboxWorkloadTemplate, 0, len(resp.GetTemplates())) - for _, protoTemplate := range resp.GetTemplates() { - templates = append(templates, converter.SandboxWorkloadTemplateFromProto(protoTemplate)) + templates := make([]*SandboxWorkloadTemplate, 0) + for { + resp, err := s.client.ListSandboxTemplates(ctx, req) + if err != nil { + return nil, converter.FromGRPCError(err) + } + for _, protoTemplate := range resp.GetTemplates() { + templates = append(templates, converter.SandboxWorkloadTemplateFromProto(protoTemplate)) + } + if resp.GetNextPageToken() == "" { + return templates, nil + } + req.PageToken = resp.GetNextPageToken() } - return templates, nil } func (s *sandboxTemplateClient) Delete(ctx context.Context, workspace, name string) (bool, error) { diff --git a/sdk/go/openshell/v1/sandbox_template_client_test.go b/sdk/go/openshell/v1/sandbox_template_client_test.go index 48045dbec8..2f63c6c042 100644 --- a/sdk/go/openshell/v1/sandbox_template_client_test.go +++ b/sdk/go/openshell/v1/sandbox_template_client_test.go @@ -221,8 +221,7 @@ func TestSandboxTemplateGetListDelete(t *testing.T) { assert.Equal(t, "img:v1", got.Spec.Workload.Image) list, err := client.ListAll(context.Background(), ListOptions{ - Limit: 10, - Offset: 2, + PageSize: 10, LabelSelector: "team=runtime", }) require.NoError(t, err) @@ -240,8 +239,8 @@ func TestSandboxTemplateGetListDelete(t *testing.T) { assert.Equal(t, "gpu-kata", mock.getRequest.Name) require.NotNil(t, mock.listRequest) assert.NotNil(t, mock.listRequest.GetWorkspaceScope().GetAllWorkspaces()) - assert.Equal(t, uint32(10), mock.listRequest.Limit) - assert.Equal(t, uint32(2), mock.listRequest.Offset) + assert.Equal(t, int32(10), mock.listRequest.PageSize) + assert.Empty(t, mock.listRequest.PageToken) assert.Equal(t, "team=runtime", mock.listRequest.LabelSelector) require.NotNil(t, mock.deleteRequest) assert.Equal(t, "default", mock.deleteRequest.GetWorkspaceScope().GetWorkspace()) @@ -253,11 +252,19 @@ func TestSandboxTemplateList_RejectsNegativePagination(t *testing.T) { client, cleanup := setupSandboxTemplateTest(t, mock) defer cleanup() - _, err := client.List(context.Background(), "default", ListOptions{Limit: -1}) + _, err := client.List(context.Background(), "default", ListOptions{PageSize: -1}) require.Error(t, err) assert.True(t, IsInvalidArgument(err)) +} - _, err = client.List(context.Background(), "default", ListOptions{Offset: -1}) - require.Error(t, err) - assert.True(t, IsInvalidArgument(err)) +func TestSandboxTemplateList_EmptyReturnsNonNilSlice(t *testing.T) { + mock := newMockSandboxTemplateServer() + client, cleanup := setupSandboxTemplateTest(t, mock) + defer cleanup() + + templates, err := client.List(context.Background(), "default") + + require.NoError(t, err) + assert.NotNil(t, templates) + assert.Empty(t, templates) } diff --git a/sdk/go/openshell/v1/service_client.go b/sdk/go/openshell/v1/service_client.go index 86013f8b30..3ab60d2da7 100644 --- a/sdk/go/openshell/v1/service_client.go +++ b/sdk/go/openshell/v1/service_client.go @@ -58,27 +58,26 @@ func (s *serviceClient) ListAll(ctx context.Context, opts ...ListOptions) ([]*Se } func (s *serviceClient) list(ctx context.Context, req *pb.ListServicesRequest, opts ...ListOptions) ([]*ServiceEndpoint, error) { - if len(opts) > 0 { - if opts[0].Limit < 0 { - return nil, &StatusError{Code: ErrorInvalidArgument, Message: "limit must not be negative"} - } - if opts[0].Offset < 0 { - return nil, &StatusError{Code: ErrorInvalidArgument, Message: "offset must not be negative"} - } - req.Limit = uint32(opts[0].Limit) - req.Offset = uint32(opts[0].Offset) - } - - resp, err := s.client.ListServices(ctx, req) + pageSize, err := listPageSize(opts) if err != nil { - return nil, converter.FromGRPCError(err) + return nil, err } + req.PageSize = pageSize - endpoints := make([]*ServiceEndpoint, 0, len(resp.GetServices())) - for _, svc := range resp.GetServices() { - endpoints = append(endpoints, converter.ServiceEndpointFromProto(svc)) + endpoints := make([]*ServiceEndpoint, 0) + for { + resp, err := s.client.ListServices(ctx, req) + if err != nil { + return nil, converter.FromGRPCError(err) + } + for _, svc := range resp.GetServices() { + endpoints = append(endpoints, converter.ServiceEndpointFromProto(svc)) + } + if resp.GetNextPageToken() == "" { + return endpoints, nil + } + req.PageToken = resp.GetNextPageToken() } - return endpoints, nil } func (s *serviceClient) Delete(ctx context.Context, workspace, sandboxName, serviceName string) error { diff --git a/sdk/go/openshell/v1/service_client_test.go b/sdk/go/openshell/v1/service_client_test.go index bf0b4eccb8..ace6a25099 100644 --- a/sdk/go/openshell/v1/service_client_test.go +++ b/sdk/go/openshell/v1/service_client_test.go @@ -251,6 +251,7 @@ func TestServiceList_Empty(t *testing.T) { endpoints, err := client.List(context.Background(), "default", "web-app") require.NoError(t, err) + assert.NotNil(t, endpoints) assert.Empty(t, endpoints) } @@ -262,7 +263,7 @@ func TestServiceList_WithOptions(t *testing.T) { _, err := client.Expose(context.Background(), "default", "web-app", "api", 8080, true) require.NoError(t, err) - endpoints, err := client.List(context.Background(), "default", "web-app", ListOptions{Limit: 10, Offset: 0}) + endpoints, err := client.List(context.Background(), "default", "web-app", ListOptions{PageSize: 10}) require.NoError(t, err) assert.Len(t, endpoints, 1) @@ -273,7 +274,7 @@ func TestServiceListAll_SelectsAllWorkspaces(t *testing.T) { client, cleanup := setupServiceTest(t, mock) defer cleanup() - endpoints, err := client.ListAll(context.Background(), ListOptions{Limit: 10}) + endpoints, err := client.ListAll(context.Background(), ListOptions{PageSize: 10}) require.NoError(t, err) assert.Empty(t, endpoints) diff --git a/sdk/go/openshell/v1/types/options.go b/sdk/go/openshell/v1/types/options.go index 0d9136a407..8f220248ef 100644 --- a/sdk/go/openshell/v1/types/options.go +++ b/sdk/go/openshell/v1/types/options.go @@ -12,8 +12,7 @@ type CreateOptions struct { // ListOptions configures resource listing with pagination and filtering. type ListOptions struct { - Limit int - Offset int + PageSize int LabelSelector string } diff --git a/sdk/go/openshell/v1/types/policy.go b/sdk/go/openshell/v1/types/policy.go index 4cd39eb081..858cc2c162 100644 --- a/sdk/go/openshell/v1/types/policy.go +++ b/sdk/go/openshell/v1/types/policy.go @@ -363,25 +363,17 @@ func (c *getStatusConfig) Global() bool { // listPolicyConfig holds configuration for List calls. type listPolicyConfig struct { - limit uint32 - offset uint32 - global bool + pageSize int32 + global bool } // ListPolicyOption configures a List call. type ListPolicyOption func(*listPolicyConfig) -// WithLimit sets the maximum number of revisions to return. -func WithLimit(limit uint32) ListPolicyOption { +// WithPageSize sets the page size used while collecting every revision. +func WithPageSize(pageSize int32) ListPolicyOption { return func(c *listPolicyConfig) { - c.limit = limit - } -} - -// WithOffset sets the pagination offset. -func WithOffset(offset uint32) ListPolicyOption { - return func(c *listPolicyConfig) { - c.offset = offset + c.pageSize = pageSize } } @@ -401,14 +393,9 @@ func ApplyListPolicyOptions(opts []ListPolicyOption) listPolicyConfig { //nolint return cfg } -// Limit returns the configured limit (0 means server default). -func (c *listPolicyConfig) Limit() uint32 { - return c.limit -} - -// Offset returns the configured offset. -func (c *listPolicyConfig) Offset() uint32 { - return c.offset +// PageSize returns the configured page size (0 means server default). +func (c *listPolicyConfig) PageSize() int32 { + return c.pageSize } // Global returns whether global policy mode is enabled. diff --git a/sdk/go/openshell/v1/workspace_client.go b/sdk/go/openshell/v1/workspace_client.go index b036217737..852c32969b 100644 --- a/sdk/go/openshell/v1/workspace_client.go +++ b/sdk/go/openshell/v1/workspace_client.go @@ -49,29 +49,29 @@ func (w *workspaceClient) Get(ctx context.Context, name string) (*Workspace, err } func (w *workspaceClient) List(ctx context.Context, opts ...ListOptions) ([]*Workspace, error) { - req := &pb.ListWorkspacesRequest{} + pageSize, err := listPageSize(opts) + if err != nil { + return nil, err + } + req := &pb.ListWorkspacesRequest{PageSize: pageSize} if len(opts) > 0 { - if opts[0].Limit < 0 { - return nil, &StatusError{Code: ErrorInvalidArgument, Message: "limit must not be negative"} - } - if opts[0].Offset < 0 { - return nil, &StatusError{Code: ErrorInvalidArgument, Message: "offset must not be negative"} - } - req.Limit = uint32(opts[0].Limit) - req.Offset = uint32(opts[0].Offset) req.LabelSelector = opts[0].LabelSelector } - resp, err := w.client.ListWorkspaces(ctx, req) - if err != nil { - return nil, converter.FromGRPCError(err) - } - - workspaces := make([]*Workspace, 0, len(resp.GetWorkspaces())) - for _, proto := range resp.GetWorkspaces() { - workspaces = append(workspaces, converter.WorkspaceFromProto(proto)) + workspaces := make([]*Workspace, 0) + for { + resp, err := w.client.ListWorkspaces(ctx, req) + if err != nil { + return nil, converter.FromGRPCError(err) + } + for _, proto := range resp.GetWorkspaces() { + workspaces = append(workspaces, converter.WorkspaceFromProto(proto)) + } + if resp.GetNextPageToken() == "" { + return workspaces, nil + } + req.PageToken = resp.GetNextPageToken() } - return workspaces, nil } func (w *workspaceClient) Delete(ctx context.Context, name string) error { @@ -135,28 +135,26 @@ func (w *workspaceClient) ListMembers(ctx context.Context, workspace string, opt return nil, &StatusError{Code: ErrorInvalidArgument, Message: "workspace name must not be empty"} } + pageSize, err := listPageSize(opts) + if err != nil { + return nil, err + } req := &pb.ListWorkspaceMembersRequest{ Workspace: workspace, + PageSize: pageSize, } - if len(opts) > 0 { - if opts[0].Limit < 0 { - return nil, &StatusError{Code: ErrorInvalidArgument, Message: "limit must not be negative"} + members := make([]*WorkspaceMember, 0) + for { + resp, err := w.client.ListWorkspaceMembers(ctx, req) + if err != nil { + return nil, converter.FromGRPCError(err) } - if opts[0].Offset < 0 { - return nil, &StatusError{Code: ErrorInvalidArgument, Message: "offset must not be negative"} + for _, proto := range resp.GetMembers() { + members = append(members, converter.WorkspaceMemberFromProto(proto)) } - req.Limit = uint32(opts[0].Limit) - req.Offset = uint32(opts[0].Offset) - } - - resp, err := w.client.ListWorkspaceMembers(ctx, req) - if err != nil { - return nil, converter.FromGRPCError(err) - } - - members := make([]*WorkspaceMember, 0, len(resp.GetMembers())) - for _, proto := range resp.GetMembers() { - members = append(members, converter.WorkspaceMemberFromProto(proto)) + if resp.GetNextPageToken() == "" { + return members, nil + } + req.PageToken = resp.GetNextPageToken() } - return members, nil } diff --git a/sdk/go/openshell/v1/workspace_test.go b/sdk/go/openshell/v1/workspace_test.go index f64a76e998..743a9be0eb 100644 --- a/sdk/go/openshell/v1/workspace_test.go +++ b/sdk/go/openshell/v1/workspace_test.go @@ -239,17 +239,28 @@ func TestWorkspaceList_WithOptions(t *testing.T) { wc := newWorkspaceClient(conn) _, err := wc.List(context.Background(), ListOptions{ - Limit: 10, - Offset: 5, + PageSize: 10, LabelSelector: "team=platform", }) require.NoError(t, err) - assert.Equal(t, uint32(10), mock.lastListReq.GetLimit()) - assert.Equal(t, uint32(5), mock.lastListReq.GetOffset()) + assert.Equal(t, int32(10), mock.lastListReq.GetPageSize()) + assert.Empty(t, mock.lastListReq.GetPageToken()) assert.Equal(t, "team=platform", mock.lastListReq.GetLabelSelector()) } +func TestWorkspaceList_EmptyReturnsNonNilSlice(t *testing.T) { + mock := &mockWorkspaceServer{listResp: &pb.ListWorkspacesResponse{}} + conn, cleanup := newMockWorkspaceServer(mock) + defer cleanup() + + workspaces, err := newWorkspaceClient(conn).List(context.Background()) + + require.NoError(t, err) + assert.NotNil(t, workspaces) + assert.Empty(t, workspaces) +} + func TestWorkspaceDelete_Success(t *testing.T) { mock := &mockWorkspaceServer{ deleteResp: &pb.DeleteWorkspaceResponse{Deleted: true}, @@ -452,6 +463,20 @@ func TestListMembers_EmptyWorkspace(t *testing.T) { assert.True(t, IsInvalidArgument(err)) } +func TestListMembers_EmptyResultReturnsNonNilSlice(t *testing.T) { + mock := &mockWorkspaceServer{ + listMembersResp: &pb.ListWorkspaceMembersResponse{}, + } + conn, cleanup := newMockWorkspaceServer(mock) + defer cleanup() + + members, err := newWorkspaceClient(conn).ListMembers(context.Background(), "test-ws") + + require.NoError(t, err) + assert.NotNil(t, members) + assert.Empty(t, members) +} + func TestListMembers_WithOptions(t *testing.T) { mock := &mockWorkspaceServer{ listMembersResp: &pb.ListWorkspaceMembersResponse{}, @@ -460,9 +485,9 @@ func TestListMembers_WithOptions(t *testing.T) { defer cleanup() wc := newWorkspaceClient(conn) - _, err := wc.ListMembers(context.Background(), "test-ws", ListOptions{Limit: 5, Offset: 2}) + _, err := wc.ListMembers(context.Background(), "test-ws", ListOptions{PageSize: 5}) require.NoError(t, err) - assert.Equal(t, uint32(5), mock.lastListMembersReq.GetLimit()) - assert.Equal(t, uint32(2), mock.lastListMembersReq.GetOffset()) + assert.Equal(t, int32(5), mock.lastListMembersReq.GetPageSize()) + assert.Empty(t, mock.lastListMembersReq.GetPageToken()) } diff --git a/sdk/go/proto/openshellv1/openshell.pb.go b/sdk/go/proto/openshellv1/openshell.pb.go index 6443c3a361..735c17bc88 100644 --- a/sdk/go/proto/openshellv1/openshell.pb.go +++ b/sdk/go/proto/openshellv1/openshell.pb.go @@ -2675,9 +2675,13 @@ func (x *GetSandboxTemplateRequest) GetWorkspaceScope() *datamodelv1.WorkspaceSe } type ListSandboxTemplatesRequest struct { - state protoimpl.MessageState `protogen:"open.v1"` - Limit uint32 `protobuf:"varint,1,opt,name=limit,proto3" json:"limit,omitempty"` - Offset uint32 `protobuf:"varint,2,opt,name=offset,proto3" json:"offset,omitempty"` + state protoimpl.MessageState `protogen:"open.v1"` + // The maximum number of templates to return. Zero uses 100. Values above + // 1000 are coerced to 1000; negative values are invalid. + PageSize int32 `protobuf:"varint,1,opt,name=page_size,json=pageSize,proto3" json:"page_size,omitempty"` + // Token from a previous ListSandboxTemplates response. All other request + // parameters except page_size must match the request that produced it. + PageToken string `protobuf:"bytes,2,opt,name=page_token,json=pageToken,proto3" json:"page_token,omitempty"` // Optional label selector in key=value comma-separated form. LabelSelector string `protobuf:"bytes,5,opt,name=label_selector,json=labelSelector,proto3" json:"label_selector,omitempty"` // Explicit named or all-workspaces scope. @@ -2716,18 +2720,18 @@ func (*ListSandboxTemplatesRequest) Descriptor() ([]byte, []int) { return file_openshell_proto_rawDescGZIP(), []int{34} } -func (x *ListSandboxTemplatesRequest) GetLimit() uint32 { +func (x *ListSandboxTemplatesRequest) GetPageSize() int32 { if x != nil { - return x.Limit + return x.PageSize } return 0 } -func (x *ListSandboxTemplatesRequest) GetOffset() uint32 { +func (x *ListSandboxTemplatesRequest) GetPageToken() string { if x != nil { - return x.Offset + return x.PageToken } - return 0 + return "" } func (x *ListSandboxTemplatesRequest) GetLabelSelector() string { @@ -2842,8 +2846,10 @@ func (x *SandboxTemplateResponse) GetTemplate() *SandboxWorkloadTemplate { } type ListSandboxTemplatesResponse struct { - state protoimpl.MessageState `protogen:"open.v1"` - Templates []*SandboxWorkloadTemplate `protobuf:"bytes,1,rep,name=templates,proto3" json:"templates,omitempty"` + state protoimpl.MessageState `protogen:"open.v1"` + Templates []*SandboxWorkloadTemplate `protobuf:"bytes,1,rep,name=templates,proto3" json:"templates,omitempty"` + // Token for the next page. Empty when there are no subsequent pages. + NextPageToken string `protobuf:"bytes,2,opt,name=next_page_token,json=nextPageToken,proto3" json:"next_page_token,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -2885,6 +2891,13 @@ func (x *ListSandboxTemplatesResponse) GetTemplates() []*SandboxWorkloadTemplate return nil } +func (x *ListSandboxTemplatesResponse) GetNextPageToken() string { + if x != nil { + return x.NextPageToken + } + return "" +} + type DeleteSandboxTemplateResponse struct { state protoimpl.MessageState `protogen:"open.v1"` Deleted bool `protobuf:"varint,1,opt,name=deleted,proto3" json:"deleted,omitempty"` @@ -3128,9 +3141,13 @@ func (x *GetSandboxRequest) GetWorkspaceScope() *datamodelv1.WorkspaceSelector { // List sandboxes request. type ListSandboxesRequest struct { - state protoimpl.MessageState `protogen:"open.v1"` - Limit uint32 `protobuf:"varint,1,opt,name=limit,proto3" json:"limit,omitempty"` - Offset uint32 `protobuf:"varint,2,opt,name=offset,proto3" json:"offset,omitempty"` + state protoimpl.MessageState `protogen:"open.v1"` + // The maximum number of sandboxes to return. Zero uses 100. Values above + // 1000 are coerced to 1000; negative values are invalid. + PageSize int32 `protobuf:"varint,1,opt,name=page_size,json=pageSize,proto3" json:"page_size,omitempty"` + // Token from a previous ListSandboxes response. All other request parameters + // except page_size must match the request that produced it. + PageToken string `protobuf:"bytes,2,opt,name=page_token,json=pageToken,proto3" json:"page_token,omitempty"` // Optional label selector for filtering (format: "key1=value1,key2=value2"). LabelSelector string `protobuf:"bytes,3,opt,name=label_selector,json=labelSelector,proto3" json:"label_selector,omitempty"` // Explicit named or all-workspaces scope. @@ -3169,18 +3186,18 @@ func (*ListSandboxesRequest) Descriptor() ([]byte, []int) { return file_openshell_proto_rawDescGZIP(), []int{42} } -func (x *ListSandboxesRequest) GetLimit() uint32 { +func (x *ListSandboxesRequest) GetPageSize() int32 { if x != nil { - return x.Limit + return x.PageSize } return 0 } -func (x *ListSandboxesRequest) GetOffset() uint32 { +func (x *ListSandboxesRequest) GetPageToken() string { if x != nil { - return x.Offset + return x.PageToken } - return 0 + return "" } func (x *ListSandboxesRequest) GetLabelSelector() string { @@ -3616,8 +3633,10 @@ func (x *SandboxResponse) GetSandbox() *Sandbox { // List sandboxes response. type ListSandboxesResponse struct { - state protoimpl.MessageState `protogen:"open.v1"` - Sandboxes []*Sandbox `protobuf:"bytes,1,rep,name=sandboxes,proto3" json:"sandboxes,omitempty"` + state protoimpl.MessageState `protogen:"open.v1"` + Sandboxes []*Sandbox `protobuf:"bytes,1,rep,name=sandboxes,proto3" json:"sandboxes,omitempty"` + // Token for the next page. Empty when there are no subsequent pages. + NextPageToken string `protobuf:"bytes,2,opt,name=next_page_token,json=nextPageToken,proto3" json:"next_page_token,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -3659,6 +3678,13 @@ func (x *ListSandboxesResponse) GetSandboxes() []*Sandbox { return nil } +func (x *ListSandboxesResponse) GetNextPageToken() string { + if x != nil { + return x.NextPageToken + } + return "" +} + // List providers attached to a sandbox response. type ListSandboxProvidersResponse struct { state protoimpl.MessageState `protogen:"open.v1"` @@ -4164,10 +4190,12 @@ type ListServicesRequest struct { state protoimpl.MessageState `protogen:"open.v1"` // Optional sandbox name. Empty lists endpoints for all sandboxes. Sandbox string `protobuf:"bytes,1,opt,name=sandbox,proto3" json:"sandbox,omitempty"` - // Page size. Zero uses the server default. - Limit uint32 `protobuf:"varint,2,opt,name=limit,proto3" json:"limit,omitempty"` - // Page offset. - Offset uint32 `protobuf:"varint,3,opt,name=offset,proto3" json:"offset,omitempty"` + // The maximum number of services to return. Zero uses 100. Values above + // 1000 are coerced to 1000; negative values are invalid. + PageSize int32 `protobuf:"varint,2,opt,name=page_size,json=pageSize,proto3" json:"page_size,omitempty"` + // Token from a previous ListServices response. All other request parameters + // except page_size must match the request that produced it. + PageToken string `protobuf:"bytes,3,opt,name=page_token,json=pageToken,proto3" json:"page_token,omitempty"` // Explicit named or all-workspaces scope. WorkspaceScope *datamodelv1.WorkspaceSelector `protobuf:"bytes,6,opt,name=workspace_scope,json=workspaceScope,proto3" json:"workspace_scope,omitempty"` unknownFields protoimpl.UnknownFields @@ -4211,18 +4239,18 @@ func (x *ListServicesRequest) GetSandbox() string { return "" } -func (x *ListServicesRequest) GetLimit() uint32 { +func (x *ListServicesRequest) GetPageSize() int32 { if x != nil { - return x.Limit + return x.PageSize } return 0 } -func (x *ListServicesRequest) GetOffset() uint32 { +func (x *ListServicesRequest) GetPageToken() string { if x != nil { - return x.Offset + return x.PageToken } - return 0 + return "" } func (x *ListServicesRequest) GetWorkspaceScope() *datamodelv1.WorkspaceSelector { @@ -4234,8 +4262,10 @@ func (x *ListServicesRequest) GetWorkspaceScope() *datamodelv1.WorkspaceSelector // Response containing exposed sandbox service endpoints. type ListServicesResponse struct { - state protoimpl.MessageState `protogen:"open.v1"` - Services []*ServiceEndpointResponse `protobuf:"bytes,1,rep,name=services,proto3" json:"services,omitempty"` + state protoimpl.MessageState `protogen:"open.v1"` + Services []*ServiceEndpointResponse `protobuf:"bytes,1,rep,name=services,proto3" json:"services,omitempty"` + // Token for the next page. Empty when there are no subsequent pages. + NextPageToken string `protobuf:"bytes,2,opt,name=next_page_token,json=nextPageToken,proto3" json:"next_page_token,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -4277,6 +4307,13 @@ func (x *ListServicesResponse) GetServices() []*ServiceEndpointResponse { return nil } +func (x *ListServicesResponse) GetNextPageToken() string { + if x != nil { + return x.NextPageToken + } + return "" +} + // Request to delete an exposed sandbox service endpoint. type DeleteServiceRequest struct { state protoimpl.MessageState `protogen:"open.v1"` @@ -5937,9 +5974,13 @@ func (x *GetProviderRequest) GetWorkspaceScope() *datamodelv1.WorkspaceSelector // List providers request. type ListProvidersRequest struct { - state protoimpl.MessageState `protogen:"open.v1"` - Limit uint32 `protobuf:"varint,1,opt,name=limit,proto3" json:"limit,omitempty"` - Offset uint32 `protobuf:"varint,2,opt,name=offset,proto3" json:"offset,omitempty"` + state protoimpl.MessageState `protogen:"open.v1"` + // The maximum number of providers to return. Zero uses 100. Values above + // 1000 are coerced to 1000; negative values are invalid. + PageSize int32 `protobuf:"varint,1,opt,name=page_size,json=pageSize,proto3" json:"page_size,omitempty"` + // Token from a previous ListProviders response. All other request parameters + // except page_size must match the request that produced it. + PageToken string `protobuf:"bytes,2,opt,name=page_token,json=pageToken,proto3" json:"page_token,omitempty"` // Explicit named or all-workspaces scope. WorkspaceScope *datamodelv1.WorkspaceSelector `protobuf:"bytes,5,opt,name=workspace_scope,json=workspaceScope,proto3" json:"workspace_scope,omitempty"` unknownFields protoimpl.UnknownFields @@ -5976,18 +6017,18 @@ func (*ListProvidersRequest) Descriptor() ([]byte, []int) { return file_openshell_proto_rawDescGZIP(), []int{83} } -func (x *ListProvidersRequest) GetLimit() uint32 { +func (x *ListProvidersRequest) GetPageSize() int32 { if x != nil { - return x.Limit + return x.PageSize } return 0 } -func (x *ListProvidersRequest) GetOffset() uint32 { +func (x *ListProvidersRequest) GetPageToken() string { if x != nil { - return x.Offset + return x.PageToken } - return 0 + return "" } func (x *ListProvidersRequest) GetWorkspaceScope() *datamodelv1.WorkspaceSelector { @@ -6162,8 +6203,10 @@ func (x *ProviderResponse) GetProvider() *datamodelv1.Provider { // List providers response. type ListProvidersResponse struct { - state protoimpl.MessageState `protogen:"open.v1"` - Providers []*datamodelv1.Provider `protobuf:"bytes,1,rep,name=providers,proto3" json:"providers,omitempty"` + state protoimpl.MessageState `protogen:"open.v1"` + Providers []*datamodelv1.Provider `protobuf:"bytes,1,rep,name=providers,proto3" json:"providers,omitempty"` + // Token for the next page. Empty when there are no subsequent pages. + NextPageToken string `protobuf:"bytes,2,opt,name=next_page_token,json=nextPageToken,proto3" json:"next_page_token,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -6205,11 +6248,22 @@ func (x *ListProvidersResponse) GetProviders() []*datamodelv1.Provider { return nil } +func (x *ListProvidersResponse) GetNextPageToken() string { + if x != nil { + return x.NextPageToken + } + return "" +} + // List provider type profiles request. type ListProviderProfilesRequest struct { - state protoimpl.MessageState `protogen:"open.v1"` - Limit uint32 `protobuf:"varint,1,opt,name=limit,proto3" json:"limit,omitempty"` - Offset uint32 `protobuf:"varint,2,opt,name=offset,proto3" json:"offset,omitempty"` + state protoimpl.MessageState `protogen:"open.v1"` + // The maximum number of profiles to return. Zero uses 100. Values above + // 1000 are coerced to 1000; negative values are invalid. + PageSize int32 `protobuf:"varint,1,opt,name=page_size,json=pageSize,proto3" json:"page_size,omitempty"` + // Token from a previous ListProviderProfiles response. All other request + // parameters except page_size must match the request that produced it. + PageToken string `protobuf:"bytes,2,opt,name=page_token,json=pageToken,proto3" json:"page_token,omitempty"` // Workspace scope. When set, returns workspace-scoped + built-in profiles. // When empty, returns platform-scoped + built-in only. Workspace string `protobuf:"bytes,3,opt,name=workspace,proto3" json:"workspace,omitempty"` @@ -6247,18 +6301,18 @@ func (*ListProviderProfilesRequest) Descriptor() ([]byte, []int) { return file_openshell_proto_rawDescGZIP(), []int{88} } -func (x *ListProviderProfilesRequest) GetLimit() uint32 { +func (x *ListProviderProfilesRequest) GetPageSize() int32 { if x != nil { - return x.Limit + return x.PageSize } return 0 } -func (x *ListProviderProfilesRequest) GetOffset() uint32 { +func (x *ListProviderProfilesRequest) GetPageToken() string { if x != nil { - return x.Offset + return x.PageToken } - return 0 + return "" } func (x *ListProviderProfilesRequest) GetWorkspace() string { @@ -7912,8 +7966,10 @@ func (x *ProviderProfileResponse) GetProfile() *ProviderProfile { // List provider profiles response. type ListProviderProfilesResponse struct { - state protoimpl.MessageState `protogen:"open.v1"` - Profiles []*ProviderProfile `protobuf:"bytes,1,rep,name=profiles,proto3" json:"profiles,omitempty"` + state protoimpl.MessageState `protogen:"open.v1"` + Profiles []*ProviderProfile `protobuf:"bytes,1,rep,name=profiles,proto3" json:"profiles,omitempty"` + // Token for the next page. Empty when there are no subsequent pages. + NextPageToken string `protobuf:"bytes,2,opt,name=next_page_token,json=nextPageToken,proto3" json:"next_page_token,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -7955,6 +8011,13 @@ func (x *ListProviderProfilesResponse) GetProfiles() []*ProviderProfile { return nil } +func (x *ListProviderProfilesResponse) GetNextPageToken() string { + if x != nil { + return x.NextPageToken + } + return "" +} + // Import custom provider profiles request. type ImportProviderProfilesRequest struct { state protoimpl.MessageState `protogen:"open.v1"` @@ -9713,9 +9776,13 @@ func (x *GetSandboxPolicyStatusResponse) GetActiveVersion() uint32 { type ListSandboxPoliciesRequest struct { state protoimpl.MessageState `protogen:"open.v1"` // Sandbox name (canonical lookup key). Ignored when global is true. - Name string `protobuf:"bytes,1,opt,name=name,proto3" json:"name,omitempty"` - Limit uint32 `protobuf:"varint,2,opt,name=limit,proto3" json:"limit,omitempty"` - Offset uint32 `protobuf:"varint,3,opt,name=offset,proto3" json:"offset,omitempty"` + Name string `protobuf:"bytes,1,opt,name=name,proto3" json:"name,omitempty"` + // The maximum number of revisions to return. Zero uses 100. Values above + // 1000 are coerced to 1000; negative values are invalid. + PageSize int32 `protobuf:"varint,2,opt,name=page_size,json=pageSize,proto3" json:"page_size,omitempty"` + // Token from a previous ListSandboxPolicies response. All other request + // parameters except page_size must match the request that produced it. + PageToken string `protobuf:"bytes,3,opt,name=page_token,json=pageToken,proto3" json:"page_token,omitempty"` // List global policy revisions instead of sandbox-scoped ones. Global bool `protobuf:"varint,4,opt,name=global,proto3" json:"global,omitempty"` // Explicit workspace scope for sandbox-scoped queries. Omit only when @@ -9762,18 +9829,18 @@ func (x *ListSandboxPoliciesRequest) GetName() string { return "" } -func (x *ListSandboxPoliciesRequest) GetLimit() uint32 { +func (x *ListSandboxPoliciesRequest) GetPageSize() int32 { if x != nil { - return x.Limit + return x.PageSize } return 0 } -func (x *ListSandboxPoliciesRequest) GetOffset() uint32 { +func (x *ListSandboxPoliciesRequest) GetPageToken() string { if x != nil { - return x.Offset + return x.PageToken } - return 0 + return "" } func (x *ListSandboxPoliciesRequest) GetGlobal() bool { @@ -9795,7 +9862,9 @@ type ListSandboxPoliciesResponse struct { state protoimpl.MessageState `protogen:"open.v1"` // Invalid historical payloads remain visible as failed projections so one // legacy row cannot hide the rest of the policy history. - Revisions []*SandboxPolicyRevision `protobuf:"bytes,1,rep,name=revisions,proto3" json:"revisions,omitempty"` + Revisions []*SandboxPolicyRevision `protobuf:"bytes,1,rep,name=revisions,proto3" json:"revisions,omitempty"` + // Token for the next page. Empty when there are no subsequent pages. + NextPageToken string `protobuf:"bytes,2,opt,name=next_page_token,json=nextPageToken,proto3" json:"next_page_token,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -9837,6 +9906,13 @@ func (x *ListSandboxPoliciesResponse) GetRevisions() []*SandboxPolicyRevision { return nil } +func (x *ListSandboxPoliciesResponse) GetNextPageToken() string { + if x != nil { + return x.NextPageToken + } + return "" +} + // Report policy load status (called by sandbox runtime after reload attempt). type ReportPolicyStatusRequest struct { state protoimpl.MessageState `protogen:"open.v1"` @@ -13568,9 +13644,13 @@ func (x *GetWorkspaceResponse) GetWorkspace() *datamodelv1.Workspace { // List workspaces request. type ListWorkspacesRequest struct { - state protoimpl.MessageState `protogen:"open.v1"` - Limit uint32 `protobuf:"varint,1,opt,name=limit,proto3" json:"limit,omitempty"` - Offset uint32 `protobuf:"varint,2,opt,name=offset,proto3" json:"offset,omitempty"` + state protoimpl.MessageState `protogen:"open.v1"` + // The maximum number of workspaces to return. Zero uses 100. Values above + // 1000 are coerced to 1000; negative values are invalid. + PageSize int32 `protobuf:"varint,1,opt,name=page_size,json=pageSize,proto3" json:"page_size,omitempty"` + // Token from a previous ListWorkspaces response. All other request parameters + // except page_size must match the request that produced it. + PageToken string `protobuf:"bytes,2,opt,name=page_token,json=pageToken,proto3" json:"page_token,omitempty"` // Optional label selector for filtering (format: "key1=value1,key2=value2"). LabelSelector string `protobuf:"bytes,3,opt,name=label_selector,json=labelSelector,proto3" json:"label_selector,omitempty"` unknownFields protoimpl.UnknownFields @@ -13607,18 +13687,18 @@ func (*ListWorkspacesRequest) Descriptor() ([]byte, []int) { return file_openshell_proto_rawDescGZIP(), []int{195} } -func (x *ListWorkspacesRequest) GetLimit() uint32 { +func (x *ListWorkspacesRequest) GetPageSize() int32 { if x != nil { - return x.Limit + return x.PageSize } return 0 } -func (x *ListWorkspacesRequest) GetOffset() uint32 { +func (x *ListWorkspacesRequest) GetPageToken() string { if x != nil { - return x.Offset + return x.PageToken } - return 0 + return "" } func (x *ListWorkspacesRequest) GetLabelSelector() string { @@ -13630,8 +13710,10 @@ func (x *ListWorkspacesRequest) GetLabelSelector() string { // List workspaces response. type ListWorkspacesResponse struct { - state protoimpl.MessageState `protogen:"open.v1"` - Workspaces []*datamodelv1.Workspace `protobuf:"bytes,1,rep,name=workspaces,proto3" json:"workspaces,omitempty"` + state protoimpl.MessageState `protogen:"open.v1"` + Workspaces []*datamodelv1.Workspace `protobuf:"bytes,1,rep,name=workspaces,proto3" json:"workspaces,omitempty"` + // Token for the next page. Empty when there are no subsequent pages. + NextPageToken string `protobuf:"bytes,2,opt,name=next_page_token,json=nextPageToken,proto3" json:"next_page_token,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -13673,6 +13755,13 @@ func (x *ListWorkspacesResponse) GetWorkspaces() []*datamodelv1.Workspace { return nil } +func (x *ListWorkspacesResponse) GetNextPageToken() string { + if x != nil { + return x.NextPageToken + } + return "" +} + // Delete workspace request. type DeleteWorkspaceRequest struct { state protoimpl.MessageState `protogen:"open.v1"` @@ -14040,9 +14129,13 @@ func (x *RemoveWorkspaceMemberResponse) GetRemoved() bool { type ListWorkspaceMembersRequest struct { state protoimpl.MessageState `protogen:"open.v1"` // Workspace name. - Workspace string `protobuf:"bytes,1,opt,name=workspace,proto3" json:"workspace,omitempty"` - Limit uint32 `protobuf:"varint,2,opt,name=limit,proto3" json:"limit,omitempty"` - Offset uint32 `protobuf:"varint,3,opt,name=offset,proto3" json:"offset,omitempty"` + Workspace string `protobuf:"bytes,1,opt,name=workspace,proto3" json:"workspace,omitempty"` + // The maximum number of members to return. Zero uses 100. Values above + // 1000 are coerced to 1000; negative values are invalid. + PageSize int32 `protobuf:"varint,2,opt,name=page_size,json=pageSize,proto3" json:"page_size,omitempty"` + // Token from a previous ListWorkspaceMembers response. All other request + // parameters except page_size must match the request that produced it. + PageToken string `protobuf:"bytes,3,opt,name=page_token,json=pageToken,proto3" json:"page_token,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -14084,24 +14177,26 @@ func (x *ListWorkspaceMembersRequest) GetWorkspace() string { return "" } -func (x *ListWorkspaceMembersRequest) GetLimit() uint32 { +func (x *ListWorkspaceMembersRequest) GetPageSize() int32 { if x != nil { - return x.Limit + return x.PageSize } return 0 } -func (x *ListWorkspaceMembersRequest) GetOffset() uint32 { +func (x *ListWorkspaceMembersRequest) GetPageToken() string { if x != nil { - return x.Offset + return x.PageToken } - return 0 + return "" } // List workspace members response. type ListWorkspaceMembersResponse struct { - state protoimpl.MessageState `protogen:"open.v1"` - Members []*WorkspaceMember `protobuf:"bytes,1,rep,name=members,proto3" json:"members,omitempty"` + state protoimpl.MessageState `protogen:"open.v1"` + Members []*WorkspaceMember `protobuf:"bytes,1,rep,name=members,proto3" json:"members,omitempty"` + // Token for the next page. Empty when there are no subsequent pages. + NextPageToken string `protobuf:"bytes,2,opt,name=next_page_token,json=nextPageToken,proto3" json:"next_page_token,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -14143,6 +14238,13 @@ func (x *ListWorkspaceMembersResponse) GetMembers() []*WorkspaceMember { return nil } +func (x *ListWorkspaceMembersResponse) GetNextPageToken() string { + if x != nil { + return x.NextPageToken + } + return "" +} + // Short-lived credential for one policy-authorized extension service. // Kept at the end of the file so adding it does not renumber existing // generated message descriptors. @@ -14382,19 +14484,21 @@ const file_openshell_proto_rawDesc = "" + "\x0fworkspace_scope\x18\x03 \x01(\v2).openshell.datamodel.v1.WorkspaceSelectorR\x0eworkspaceScopeJ\x04\b\x02\x10\x03R\tworkspace\"\x94\x01\n" + "\x19GetSandboxTemplateRequest\x12\x12\n" + "\x04name\x18\x01 \x01(\tR\x04name\x12R\n" + - "\x0fworkspace_scope\x18\x03 \x01(\v2).openshell.datamodel.v1.WorkspaceSelectorR\x0eworkspaceScopeJ\x04\b\x02\x10\x03R\tworkspace\"\xed\x01\n" + - "\x1bListSandboxTemplatesRequest\x12\x14\n" + - "\x05limit\x18\x01 \x01(\rR\x05limit\x12\x16\n" + - "\x06offset\x18\x02 \x01(\rR\x06offset\x12%\n" + + "\x0fworkspace_scope\x18\x03 \x01(\v2).openshell.datamodel.v1.WorkspaceSelectorR\x0eworkspaceScopeJ\x04\b\x02\x10\x03R\tworkspace\"\xfb\x01\n" + + "\x1bListSandboxTemplatesRequest\x12\x1b\n" + + "\tpage_size\x18\x01 \x01(\x05R\bpageSize\x12\x1d\n" + + "\n" + + "page_token\x18\x02 \x01(\tR\tpageToken\x12%\n" + "\x0elabel_selector\x18\x05 \x01(\tR\rlabelSelector\x12R\n" + "\x0fworkspace_scope\x18\x06 \x01(\v2).openshell.datamodel.v1.WorkspaceSelectorR\x0eworkspaceScopeJ\x04\b\x03\x10\x04J\x04\b\x04\x10\x05R\tworkspaceR\x0eall_workspaces\"\x97\x01\n" + "\x1cDeleteSandboxTemplateRequest\x12\x12\n" + "\x04name\x18\x01 \x01(\tR\x04name\x12R\n" + "\x0fworkspace_scope\x18\x03 \x01(\v2).openshell.datamodel.v1.WorkspaceSelectorR\x0eworkspaceScopeJ\x04\b\x02\x10\x03R\tworkspace\"\\\n" + "\x17SandboxTemplateResponse\x12A\n" + - "\btemplate\x18\x01 \x01(\v2%.openshell.v1.SandboxWorkloadTemplateR\btemplate\"c\n" + + "\btemplate\x18\x01 \x01(\v2%.openshell.v1.SandboxWorkloadTemplateR\btemplate\"\x8b\x01\n" + "\x1cListSandboxTemplatesResponse\x12C\n" + - "\ttemplates\x18\x01 \x03(\v2%.openshell.v1.SandboxWorkloadTemplateR\ttemplates\"9\n" + + "\ttemplates\x18\x01 \x03(\v2%.openshell.v1.SandboxWorkloadTemplateR\ttemplates\x12&\n" + + "\x0fnext_page_token\x18\x02 \x01(\tR\rnextPageToken\"9\n" + "\x1dDeleteSandboxTemplateResponse\x12\x18\n" + "\adeleted\x18\x01 \x01(\bR\adeleted\"\xbf\x01\n" + "\x1cBeginRootfsTarStagingRequest\x12\x1b\n" + @@ -14410,10 +14514,11 @@ const file_openshell_proto_rawDesc = "" + "\rexpires_at_ms\x18\x04 \x01(\x03R\vexpiresAtMs\"\x8c\x01\n" + "\x11GetSandboxRequest\x12\x12\n" + "\x04name\x18\x01 \x01(\tR\x04name\x12R\n" + - "\x0fworkspace_scope\x18\x03 \x01(\v2).openshell.datamodel.v1.WorkspaceSelectorR\x0eworkspaceScopeJ\x04\b\x02\x10\x03R\tworkspace\"\xe6\x01\n" + - "\x14ListSandboxesRequest\x12\x14\n" + - "\x05limit\x18\x01 \x01(\rR\x05limit\x12\x16\n" + - "\x06offset\x18\x02 \x01(\rR\x06offset\x12%\n" + + "\x0fworkspace_scope\x18\x03 \x01(\v2).openshell.datamodel.v1.WorkspaceSelectorR\x0eworkspaceScopeJ\x04\b\x02\x10\x03R\tworkspace\"\xf4\x01\n" + + "\x14ListSandboxesRequest\x12\x1b\n" + + "\tpage_size\x18\x01 \x01(\x05R\bpageSize\x12\x1d\n" + + "\n" + + "page_token\x18\x02 \x01(\tR\tpageToken\x12%\n" + "\x0elabel_selector\x18\x03 \x01(\tR\rlabelSelector\x12R\n" + "\x0fworkspace_scope\x18\x06 \x01(\v2).openshell.datamodel.v1.WorkspaceSelectorR\x0eworkspaceScopeJ\x04\b\x04\x10\x05J\x04\b\x05\x10\x06R\tworkspaceR\x0eall_workspaces\"\xa5\x01\n" + "\x1bListSandboxProvidersRequest\x12!\n" + @@ -14439,9 +14544,10 @@ const file_openshell_proto_rawDesc = "" + "\x04name\x18\x01 \x01(\tR\x04name\x12R\n" + "\x0fworkspace_scope\x18\x03 \x01(\v2).openshell.datamodel.v1.WorkspaceSelectorR\x0eworkspaceScopeJ\x04\b\x02\x10\x03R\tworkspace\"B\n" + "\x0fSandboxResponse\x12/\n" + - "\asandbox\x18\x01 \x01(\v2\x15.openshell.v1.SandboxR\asandbox\"L\n" + + "\asandbox\x18\x01 \x01(\v2\x15.openshell.v1.SandboxR\asandbox\"t\n" + "\x15ListSandboxesResponse\x123\n" + - "\tsandboxes\x18\x01 \x03(\v2\x15.openshell.v1.SandboxR\tsandboxes\"^\n" + + "\tsandboxes\x18\x01 \x03(\v2\x15.openshell.v1.SandboxR\tsandboxes\x12&\n" + + "\x0fnext_page_token\x18\x02 \x01(\tR\rnextPageToken\"^\n" + "\x1cListSandboxProvidersResponse\x12>\n" + "\tproviders\x18\x01 \x03(\v2 .openshell.datamodel.v1.ProviderR\tproviders\"l\n" + "\x1dAttachSandboxProviderResponse\x12/\n" + @@ -14474,14 +14580,16 @@ const file_openshell_proto_rawDesc = "" + "\x11GetServiceRequest\x12\x18\n" + "\asandbox\x18\x01 \x01(\tR\asandbox\x12\x18\n" + "\aservice\x18\x02 \x01(\tR\aservice\x12R\n" + - "\x0fworkspace_scope\x18\x04 \x01(\v2).openshell.datamodel.v1.WorkspaceSelectorR\x0eworkspaceScopeJ\x04\b\x03\x10\x04R\tworkspace\"\xd8\x01\n" + + "\x0fworkspace_scope\x18\x04 \x01(\v2).openshell.datamodel.v1.WorkspaceSelectorR\x0eworkspaceScopeJ\x04\b\x03\x10\x04R\tworkspace\"\xe6\x01\n" + "\x13ListServicesRequest\x12\x18\n" + - "\asandbox\x18\x01 \x01(\tR\asandbox\x12\x14\n" + - "\x05limit\x18\x02 \x01(\rR\x05limit\x12\x16\n" + - "\x06offset\x18\x03 \x01(\rR\x06offset\x12R\n" + - "\x0fworkspace_scope\x18\x06 \x01(\v2).openshell.datamodel.v1.WorkspaceSelectorR\x0eworkspaceScopeJ\x04\b\x04\x10\x05J\x04\b\x05\x10\x06R\tworkspaceR\x0eall_workspaces\"Y\n" + + "\asandbox\x18\x01 \x01(\tR\asandbox\x12\x1b\n" + + "\tpage_size\x18\x02 \x01(\x05R\bpageSize\x12\x1d\n" + + "\n" + + "page_token\x18\x03 \x01(\tR\tpageToken\x12R\n" + + "\x0fworkspace_scope\x18\x06 \x01(\v2).openshell.datamodel.v1.WorkspaceSelectorR\x0eworkspaceScopeJ\x04\b\x04\x10\x05J\x04\b\x05\x10\x06R\tworkspaceR\x0eall_workspaces\"\x81\x01\n" + "\x14ListServicesResponse\x12A\n" + - "\bservices\x18\x01 \x03(\v2%.openshell.v1.ServiceEndpointResponseR\bservices\"\xaf\x01\n" + + "\bservices\x18\x01 \x03(\v2%.openshell.v1.ServiceEndpointResponseR\bservices\x12&\n" + + "\x0fnext_page_token\x18\x02 \x01(\tR\rnextPageToken\"\xaf\x01\n" + "\x14DeleteServiceRequest\x12\x18\n" + "\asandbox\x18\x01 \x01(\tR\asandbox\x12\x18\n" + "\aservice\x18\x02 \x01(\tR\aservice\x12R\n" + @@ -14602,10 +14710,11 @@ const file_openshell_proto_rawDesc = "" + "\x0fworkspace_scope\x18\x03 \x01(\v2).openshell.datamodel.v1.WorkspaceSelectorR\x0eworkspaceScopeJ\x04\b\x02\x10\x03R\tworkspace\"\x8d\x01\n" + "\x12GetProviderRequest\x12\x12\n" + "\x04name\x18\x01 \x01(\tR\x04name\x12R\n" + - "\x0fworkspace_scope\x18\x03 \x01(\v2).openshell.datamodel.v1.WorkspaceSelectorR\x0eworkspaceScopeJ\x04\b\x02\x10\x03R\tworkspace\"\xbf\x01\n" + - "\x14ListProvidersRequest\x12\x14\n" + - "\x05limit\x18\x01 \x01(\rR\x05limit\x12\x16\n" + - "\x06offset\x18\x02 \x01(\rR\x06offset\x12R\n" + + "\x0fworkspace_scope\x18\x03 \x01(\v2).openshell.datamodel.v1.WorkspaceSelectorR\x0eworkspaceScopeJ\x04\b\x02\x10\x03R\tworkspace\"\xcd\x01\n" + + "\x14ListProvidersRequest\x12\x1b\n" + + "\tpage_size\x18\x01 \x01(\x05R\bpageSize\x12\x1d\n" + + "\n" + + "page_token\x18\x02 \x01(\tR\tpageToken\x12R\n" + "\x0fworkspace_scope\x18\x05 \x01(\v2).openshell.datamodel.v1.WorkspaceSelectorR\x0eworkspaceScopeJ\x04\b\x03\x10\x04J\x04\b\x04\x10\x05R\tworkspaceR\x0eall_workspaces\"\xfd\x02\n" + "\x15UpdateProviderRequest\x12<\n" + "\bprovider\x18\x01 \x01(\v2 .openshell.datamodel.v1.ProviderR\bprovider\x12w\n" + @@ -14618,12 +14727,14 @@ const file_openshell_proto_rawDesc = "" + "\x04name\x18\x01 \x01(\tR\x04name\x12R\n" + "\x0fworkspace_scope\x18\x03 \x01(\v2).openshell.datamodel.v1.WorkspaceSelectorR\x0eworkspaceScopeJ\x04\b\x02\x10\x03R\tworkspace\"P\n" + "\x10ProviderResponse\x12<\n" + - "\bprovider\x18\x01 \x01(\v2 .openshell.datamodel.v1.ProviderR\bprovider\"W\n" + + "\bprovider\x18\x01 \x01(\v2 .openshell.datamodel.v1.ProviderR\bprovider\"\x7f\n" + "\x15ListProvidersResponse\x12>\n" + - "\tproviders\x18\x01 \x03(\v2 .openshell.datamodel.v1.ProviderR\tproviders\"i\n" + - "\x1bListProviderProfilesRequest\x12\x14\n" + - "\x05limit\x18\x01 \x01(\rR\x05limit\x12\x16\n" + - "\x06offset\x18\x02 \x01(\rR\x06offset\x12\x1c\n" + + "\tproviders\x18\x01 \x03(\v2 .openshell.datamodel.v1.ProviderR\tproviders\x12&\n" + + "\x0fnext_page_token\x18\x02 \x01(\tR\rnextPageToken\"w\n" + + "\x1bListProviderProfilesRequest\x12\x1b\n" + + "\tpage_size\x18\x01 \x01(\x05R\bpageSize\x12\x1d\n" + + "\n" + + "page_token\x18\x02 \x01(\tR\tpageToken\x12\x1c\n" + "\tworkspace\x18\x03 \x01(\tR\tworkspace\"I\n" + "\x19GetProviderProfileRequest\x12\x0e\n" + "\x02id\x18\x01 \x01(\tR\x02id\x12\x1c\n" + @@ -14767,9 +14878,10 @@ const file_openshell_proto_rawDesc = "" + "\x03key\x18\x01 \x01(\tR\x03key\x12\x14\n" + "\x05value\x18\x02 \x01(\tR\x05value:\x028\x01\"R\n" + "\x17ProviderProfileResponse\x127\n" + - "\aprofile\x18\x01 \x01(\v2\x1d.openshell.v1.ProviderProfileR\aprofile\"Y\n" + + "\aprofile\x18\x01 \x01(\v2\x1d.openshell.v1.ProviderProfileR\aprofile\"\x81\x01\n" + "\x1cListProviderProfilesResponse\x129\n" + - "\bprofiles\x18\x01 \x03(\v2\x1d.openshell.v1.ProviderProfileR\bprofiles\"\x82\x01\n" + + "\bprofiles\x18\x01 \x03(\v2\x1d.openshell.v1.ProviderProfileR\bprofiles\x12&\n" + + "\x0fnext_page_token\x18\x02 \x01(\tR\rnextPageToken\"\x82\x01\n" + "\x1dImportProviderProfilesRequest\x12C\n" + "\bprofiles\x18\x01 \x03(\v2'.openshell.v1.ProviderProfileImportItemR\bprofiles\x12\x1c\n" + "\tworkspace\x18\x02 \x01(\tR\tworkspace\"\xc2\x01\n" + @@ -14906,15 +15018,17 @@ const file_openshell_proto_rawDesc = "" + "\x0fworkspace_scope\x18\x05 \x01(\v2).openshell.datamodel.v1.WorkspaceSelectorR\x0eworkspaceScopeJ\x04\b\x04\x10\x05R\tworkspace\"\x88\x01\n" + "\x1eGetSandboxPolicyStatusResponse\x12?\n" + "\brevision\x18\x01 \x01(\v2#.openshell.v1.SandboxPolicyRevisionR\brevision\x12%\n" + - "\x0eactive_version\x18\x02 \x01(\rR\ractiveVersion\"\xdb\x01\n" + + "\x0eactive_version\x18\x02 \x01(\rR\ractiveVersion\"\xe9\x01\n" + "\x1aListSandboxPoliciesRequest\x12\x12\n" + - "\x04name\x18\x01 \x01(\tR\x04name\x12\x14\n" + - "\x05limit\x18\x02 \x01(\rR\x05limit\x12\x16\n" + - "\x06offset\x18\x03 \x01(\rR\x06offset\x12\x16\n" + + "\x04name\x18\x01 \x01(\tR\x04name\x12\x1b\n" + + "\tpage_size\x18\x02 \x01(\x05R\bpageSize\x12\x1d\n" + + "\n" + + "page_token\x18\x03 \x01(\tR\tpageToken\x12\x16\n" + "\x06global\x18\x04 \x01(\bR\x06global\x12R\n" + - "\x0fworkspace_scope\x18\x06 \x01(\v2).openshell.datamodel.v1.WorkspaceSelectorR\x0eworkspaceScopeJ\x04\b\x05\x10\x06R\tworkspace\"`\n" + + "\x0fworkspace_scope\x18\x06 \x01(\v2).openshell.datamodel.v1.WorkspaceSelectorR\x0eworkspaceScopeJ\x04\b\x05\x10\x06R\tworkspace\"\x88\x01\n" + "\x1bListSandboxPoliciesResponse\x12A\n" + - "\trevisions\x18\x01 \x03(\v2#.openshell.v1.SandboxPolicyRevisionR\trevisions\"\xa7\x01\n" + + "\trevisions\x18\x01 \x03(\v2#.openshell.v1.SandboxPolicyRevisionR\trevisions\x12&\n" + + "\x0fnext_page_token\x18\x02 \x01(\tR\rnextPageToken\"\xa7\x01\n" + "\x19ReportPolicyStatusRequest\x12\x1d\n" + "\n" + "sandbox_id\x18\x01 \x01(\tR\tsandboxId\x12\x18\n" + @@ -15192,15 +15306,17 @@ const file_openshell_proto_rawDesc = "" + "\x13GetWorkspaceRequest\x12\x12\n" + "\x04name\x18\x01 \x01(\tR\x04name\"W\n" + "\x14GetWorkspaceResponse\x12?\n" + - "\tworkspace\x18\x01 \x01(\v2!.openshell.datamodel.v1.WorkspaceR\tworkspace\"l\n" + - "\x15ListWorkspacesRequest\x12\x14\n" + - "\x05limit\x18\x01 \x01(\rR\x05limit\x12\x16\n" + - "\x06offset\x18\x02 \x01(\rR\x06offset\x12%\n" + - "\x0elabel_selector\x18\x03 \x01(\tR\rlabelSelector\"[\n" + + "\tworkspace\x18\x01 \x01(\v2!.openshell.datamodel.v1.WorkspaceR\tworkspace\"z\n" + + "\x15ListWorkspacesRequest\x12\x1b\n" + + "\tpage_size\x18\x01 \x01(\x05R\bpageSize\x12\x1d\n" + + "\n" + + "page_token\x18\x02 \x01(\tR\tpageToken\x12%\n" + + "\x0elabel_selector\x18\x03 \x01(\tR\rlabelSelector\"\x83\x01\n" + "\x16ListWorkspacesResponse\x12A\n" + "\n" + "workspaces\x18\x01 \x03(\v2!.openshell.datamodel.v1.WorkspaceR\n" + - "workspaces\",\n" + + "workspaces\x12&\n" + + "\x0fnext_page_token\x18\x02 \x01(\tR\rnextPageToken\",\n" + "\x16DeleteWorkspaceRequest\x12\x12\n" + "\x04name\x18\x01 \x01(\tR\x04name\"3\n" + "\x17DeleteWorkspaceResponse\x12\x18\n" + @@ -15219,13 +15335,15 @@ const file_openshell_proto_rawDesc = "" + "\tworkspace\x18\x01 \x01(\tR\tworkspace\x12+\n" + "\x11principal_subject\x18\x02 \x01(\tR\x10principalSubject\"9\n" + "\x1dRemoveWorkspaceMemberResponse\x12\x18\n" + - "\aremoved\x18\x01 \x01(\bR\aremoved\"i\n" + + "\aremoved\x18\x01 \x01(\bR\aremoved\"w\n" + "\x1bListWorkspaceMembersRequest\x12\x1c\n" + - "\tworkspace\x18\x01 \x01(\tR\tworkspace\x12\x14\n" + - "\x05limit\x18\x02 \x01(\rR\x05limit\x12\x16\n" + - "\x06offset\x18\x03 \x01(\rR\x06offset\"W\n" + + "\tworkspace\x18\x01 \x01(\tR\tworkspace\x12\x1b\n" + + "\tpage_size\x18\x02 \x01(\x05R\bpageSize\x12\x1d\n" + + "\n" + + "page_token\x18\x03 \x01(\tR\tpageToken\"\x7f\n" + "\x1cListWorkspaceMembersResponse\x127\n" + - "\amembers\x18\x01 \x03(\v2\x1d.openshell.v1.WorkspaceMemberR\amembers\"\x7f\n" + + "\amembers\x18\x01 \x03(\v2\x1d.openshell.v1.WorkspaceMemberR\amembers\x12&\n" + + "\x0fnext_page_token\x18\x02 \x01(\tR\rnextPageToken\"\x7f\n" + "\x1aExtensionServiceCredential\x12!\n" + "\fservice_name\x18\x01 \x01(\tR\vserviceName\x12\x1a\n" + "\x05token\x18\x02 \x01(\tB\x04\x88\xb5\x18\x01R\x05token\x12\"\n" + diff --git a/sdk/typescript/README.md b/sdk/typescript/README.md index 112bfdc5fe..01689c61e8 100644 --- a/sdk/typescript/README.md +++ b/sdk/typescript/README.md @@ -186,13 +186,15 @@ const sandbox = await client.sandbox.createFromTemplate({ }) await client.sandboxTemplates.get('python', { workspace: 'default' }) -await client.sandboxTemplates.list({ workspace: 'default', limit: 100 }) +await client.sandboxTemplates.list({ workspace: 'default', pageSize: 100 }) await client.sandboxTemplates.delete('python', { workspace: 'default' }) ``` Use `allWorkspaces: true` on `list()` for a platform-admin view. The discriminated option type makes `workspace` and `allWorkspaces` mutually exclusive. Omitting both options explicitly selects the `default` workspace. +List methods follow continuation tokens automatically; `pageSize` controls +each gateway request. ## Surface and roadmap diff --git a/sdk/typescript/src/client.test.ts b/sdk/typescript/src/client.test.ts index 60cd5c3f0e..824ab4a115 100644 --- a/sdk/typescript/src/client.test.ts +++ b/sdk/typescript/src/client.test.ts @@ -438,7 +438,7 @@ describe('create', () => { const created = await sandbox.create({ name: 'direct', workspace: 'staging', image: 'img' }); const got = await sandbox.get('lookup', { workspace: 'staging' }); - const listed = await sandbox.list({ workspace: 'staging', limit: 10 }); + const listed = await sandbox.list({ workspace: 'staging', pageSize: 10 }); const deleted = await sandbox.delete('lookup', { workspace: 'staging' }); await expect(sandbox.waitReady('lookup', 1, { workspace: 'staging' })).resolves.toMatchObject({ workspace: 'staging', @@ -483,6 +483,32 @@ describe('create', () => { expect(selectedWorkspace(observed.updateSetting ?? {})).toBe('staging'); }); + it('follows sandbox list continuation tokens', async () => { + const requests: Array<{ pageToken?: string; pageSize?: number; labelSelector?: string }> = []; + const sandbox = client({ + listSandboxes: (req) => { + requests.push(req); + if (req.pageToken === '') { + return { + sandboxes: [readySandbox('first', 'first-id').sandbox ?? {}], + nextPageToken: 'page-2', + }; + } + return { + sandboxes: [readySandbox('second', 'second-id').sandbox ?? {}], + nextPageToken: '', + }; + }, + }); + + const listed = await sandbox.list({ pageSize: 1, labelSelector: 'team=core' }); + + expect(listed.map((item) => item.name)).toEqual(['first', 'second']); + expect(requests).toHaveLength(2); + expect(requests[0]).toMatchObject({ pageToken: '', pageSize: 1, labelSelector: 'team=core' }); + expect(requests[1]).toMatchObject({ pageToken: 'page-2', pageSize: 1, labelSelector: 'team=core' }); + }); + it('createFromTemplate rejects an empty template name locally', async () => { const sandbox = client({}); await expect(sandbox.createFromTemplate({ templateName: ' ' })).rejects.toMatchObject({ @@ -593,10 +619,10 @@ describe('sandbox templates', () => { expect(created.metadata?.resourceVersion).toBe(1n); }); - it('get list and delete forward workspace and pagination', async () => { + it('get list and delete forward workspace and page size', async () => { const observed: { get?: ScopedRequest & { name?: string }; - list?: ScopedRequest & { limit?: number; offset?: number; labelSelector?: string }; + list?: ScopedRequest & { pageSize?: number; pageToken?: string; labelSelector?: string }; delete?: ScopedRequest & { name?: string }; } = {}; const templates = templateClient({ @@ -627,7 +653,7 @@ describe('sandbox templates', () => { }); const got = await templates.get('gpu-kata', { workspace: 'staging' }); - const listed = await templates.list({ workspace: 'staging', limit: 10, offset: 2, labelSelector: 'team=runtime' }); + const listed = await templates.list({ workspace: 'staging', pageSize: 10, labelSelector: 'team=runtime' }); const deleted = await templates.delete('gpu-kata', { workspace: 'staging' }); expect(got.metadata?.name).toBe('gpu-kata'); @@ -636,8 +662,8 @@ describe('sandbox templates', () => { expect(observed.get).toMatchObject({ name: 'gpu-kata' }); expect(selectedWorkspace(observed.get ?? {})).toBe('staging'); expect(observed.list).toMatchObject({ - limit: 10, - offset: 2, + pageSize: 10, + pageToken: '', labelSelector: 'team=runtime', }); expect(selectedWorkspace(observed.list ?? {})).toBe('staging'); diff --git a/sdk/typescript/src/client.ts b/sdk/typescript/src/client.ts index 839a425e2b..989ce2f0a7 100644 --- a/sdk/typescript/src/client.ts +++ b/sdk/typescript/src/client.ts @@ -150,8 +150,8 @@ export interface SandboxWorkloadTemplateProvenance { } interface PaginationOptions { - limit?: number; - offset?: number; + /** Page size used while collecting every result. */ + pageSize?: number; labelSelector?: string; } @@ -175,8 +175,8 @@ export interface SandboxTemplateWorkspaceOptions { } export type SandboxTemplateListOptions = WorkspaceListScope & { - limit?: number; - offset?: number; + /** Page size used while collecting every result. */ + pageSize?: number; /** Optional label selector in key=value comma-separated form. */ labelSelector?: string; }; @@ -687,13 +687,19 @@ export class SandboxTemplateClient { async list(options?: SandboxTemplateListOptions | null): Promise { try { - const resp = await this.grpc.listSandboxTemplates({ - limit: options?.limit ?? 0, - offset: options?.offset ?? 0, - labelSelector: options?.labelSelector ?? '', - workspaceScope: listWorkspaceScope(options), - }); - return resp.templates; + const templates: SandboxWorkloadTemplate[] = []; + let pageToken = ''; + do { + const resp = await this.grpc.listSandboxTemplates({ + pageSize: options?.pageSize ?? 0, + pageToken, + labelSelector: options?.labelSelector ?? '', + workspaceScope: listWorkspaceScope(options), + }); + templates.push(...resp.templates); + pageToken = resp.nextPageToken; + } while (pageToken !== ''); + return templates; } catch (e) { throw fromConnect(e); } @@ -811,13 +817,19 @@ export class SandboxClient { async list(options?: ListOptions | null): Promise { try { - const resp = await this.grpc.listSandboxes({ - limit: options?.limit ?? 0, - offset: options?.offset ?? 0, - labelSelector: options?.labelSelector ?? '', - workspaceScope: listWorkspaceScope(options), - }); - return resp.sandboxes.map((s) => sandboxRef(s)); + const sandboxes: SandboxRef[] = []; + let pageToken = ''; + do { + const resp = await this.grpc.listSandboxes({ + pageSize: options?.pageSize ?? 0, + pageToken, + labelSelector: options?.labelSelector ?? '', + workspaceScope: listWorkspaceScope(options), + }); + sandboxes.push(...resp.sandboxes.map((sandbox) => sandboxRef(sandbox))); + pageToken = resp.nextPageToken; + } while (pageToken !== ''); + return sandboxes; } catch (e) { throw fromConnect(e); } diff --git a/skills/openshell-cli/SKILL.md b/skills/openshell-cli/SKILL.md index ed64496190..d986a45cc1 100644 --- a/skills/openshell-cli/SKILL.md +++ b/skills/openshell-cli/SKILL.md @@ -542,7 +542,7 @@ Return to Step 2. Continue monitoring logs and refining the policy until all req View all revisions to understand how the policy evolved: ```bash -openshell policy list dev --limit 50 +openshell policy list dev --page-size 50 openshell policy list dev --output json ```