From d01d9dce6d6a4a0554ab36e98eb38a1c1fccbf13 Mon Sep 17 00:00:00 2001 From: Abhinav Chordia Date: Fri, 30 Oct 2026 21:43:20 +0000 Subject: [PATCH 1/4] api: send the untracked wake to WorkerApiInternal.WakeSandbox An explicit or implicit wake for a sandbox with no loaded row called the public WorkerApi.WakeSandbox, the only caller of that RPC. Send it to WorkerApiInternal.WakeSandbox instead, which tracked Direct and Kubernetes wakes already use, so the public twin can be removed. The request is unchanged: no start context, no runtime upgrade, blocking, and the implicit wake source for an implicit wake. The internal handler requires control-plane policy authority only for a runtime upgrade or an activation auto-lifecycle policy, so this request needs none, and the api already holds the internal-surface credential its tracked wakes use. A characterization test serves only WorkerApiInternal on the placed worker and checks that both wakes arrive there under the internal bearer with the same fields. It failed with HTTP 501 (UNIMPLEMENTED) before the change. --- .../api/src/controllers/sandbox/lifecycle.rs | 9 +- .../lifecycle/characterization_tests.rs | 132 +++++++++++++++++- .../controllers/substrate/node_api_client.rs | 2 +- 3 files changed, 136 insertions(+), 7 deletions(-) diff --git a/crates/libs/api/src/controllers/sandbox/lifecycle.rs b/crates/libs/api/src/controllers/sandbox/lifecycle.rs index 68caf9fb6b..d79f6db1ce 100644 --- a/crates/libs/api/src/controllers/sandbox/lifecycle.rs +++ b/crates/libs/api/src/controllers/sandbox/lifecycle.rs @@ -2208,14 +2208,15 @@ impl SandboxController { SandboxLifecycleStatus::Started } SandboxAction::Wake | SandboxAction::ImplicitWake(_) => { - let mut client = self - .substrate + // The worker serves Wake only on its internal surface. With no + // row there is no start context to send. + self.substrate .runtime_client( typed_id, crate::controllers::substrate::RecoveryCharge::Exempt, ) - .await?; - client + .await? + .internal_client() .wake_sandbox(WakeRequest { implicit_wake_source: action .wake_activity() diff --git a/crates/libs/api/src/controllers/sandbox/lifecycle/characterization_tests.rs b/crates/libs/api/src/controllers/sandbox/lifecycle/characterization_tests.rs index 4fa1a63b4e..d6ab7fcc94 100644 --- a/crates/libs/api/src/controllers/sandbox/lifecycle/characterization_tests.rs +++ b/crates/libs/api/src/controllers/sandbox/lifecycle/characterization_tests.rs @@ -179,6 +179,11 @@ async fn start_sim_worker(pool: &PgPool) -> TestResult<(SimWorkerHarness, NodeId .with_internal_api() .start() .await?; + register_direct_worker(pool, node_id, harness.endpoint()).await?; + Ok((harness, node_id)) +} + +async fn register_direct_worker(pool: &PgPool, node_id: NodeId, endpoint: &str) -> TestResult { sqlx::query(sql!( "INSERT INTO nodes (id, address, role, managed_by, state) VALUES ($1, '127.0.0.1', 'worker', 'space', 'ready')" @@ -190,10 +195,10 @@ async fn start_sim_worker(pool: &PgPool) -> TestResult<(SimWorkerHarness, NodeId "INSERT INTO node_direct (node_id, node_api_endpoint) VALUES ($1, $2)" )) .bind(node_id) - .bind(harness.endpoint()) + .bind(endpoint) .execute(pool) .await?; - Ok((harness, node_id)) + Ok(()) } async fn insert_resident(state: &SimState, sandbox_id: SandboxId, status: &str) -> TestResult { @@ -1300,6 +1305,129 @@ async fn untracked_actions_report_status_and_leave_the_row() -> TestResult { Ok(()) } +/// Records every Wake and the bearer it arrived with. It serves only the +/// internal worker service, so a call that reached any other one fails. +#[derive(Clone, Default)] +struct RecordingInternalWorker { + wakes: std::sync::Arc>>, +} + +type RecordedWake = ( + WakeRequest, + Option>, +); + +#[tonic::async_trait] +impl pplx_space_node_api_client::proto::worker_api_internal_server::WorkerApiInternal + for RecordingInternalWorker +{ + async fn adopt_pool_sandbox( + &self, + _request: tonic::Request, + ) -> Result< + tonic::Response, + tonic::Status, + > { + Err(tonic::Status::unimplemented("not used by this test")) + } + + async fn wake_sandbox( + &self, + request: tonic::Request, + ) -> Result, tonic::Status> + { + let bearer = request.metadata().get("authorization").cloned(); + let request = request.into_inner(); + self.wakes.lock().unwrap().push((request.clone(), bearer)); + Ok(tonic::Response::new( + pplx_space_sbx_mgr_client::proto::WakeResponse { + sandbox_id: request.sandbox_id, + transition: pplx_space_sbx_mgr_client::proto::WakeTransition::Resume as i32, + route_identity: None, + }, + )) + } +} + +/// With no row to build a start context from, an untracked wake sends the +/// worker's internal Wake a bare request under the control-plane bearer, +/// the call a tracked wake makes with its context. +#[tokio::test(flavor = "current_thread")] +async fn untracked_wake_uses_the_internal_worker_wake() -> TestResult { + use pplx_space_common_svc::bearer_token::{SpaceBearerToken, space_bearer_metadata}; + use pplx_space_node_api_client::proto::worker_api_internal_server::WorkerApiInternalServer; + + if !crate::test_support::enter_sim_worker_process() { + return Ok(()); + } + let db = pg_test_db_with_max_connections(3).await?; + let pool = &db.pool; + let controller = direct_controller(pool); + let worker = RecordingInternalWorker::default(); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await?; + let endpoint = listener.local_addr()?.to_string(); + let shutdown = tokio_util::sync::CancellationToken::new(); + let server = tokio::spawn( + tonic::transport::Server::builder() + .add_service(WorkerApiInternalServer::new(worker.clone())) + .serve_with_incoming_shutdown( + tonic::transport::server::TcpIncoming::from(listener), + shutdown.clone().cancelled_owned(), + ), + ); + let node_id = NodeId::new(); + register_direct_worker(pool, node_id, &endpoint).await?; + + let explicit = SandboxId::new(); + let implicit = SandboxId::new(); + let activity = ExecutionActivityKind::Command; + for (sandbox_id, action) in [ + (explicit, SandboxAction::Wake), + (implicit, SandboxAction::ImplicitWake(activity)), + ] { + insert_sandbox(pool, sandbox_id, "created", "space").await?; + place_on_node(pool, sandbox_id, node_id).await?; + let body = Box::pin(run_action( + &controller, + pool, + sandbox_id, + None, + action, + None, + )) + .await?; + assert_eq!(body["status"], "started", "{action:?}"); + } + shutdown.cancel(); + server.await??; + + let control_bearer = space_bearer_metadata(SpaceBearerToken::NodeApiInternal)?; + let wakes = worker.wakes.lock().unwrap().clone(); + assert_eq!( + wakes, + [ + ( + WakeRequest { + sandbox_id: explicit.to_string(), + blocking: true, + ..Default::default() + }, + Some(control_bearer.clone()), + ), + ( + WakeRequest { + sandbox_id: implicit.to_string(), + blocking: true, + implicit_wake_source: activity.wake_source(), + ..Default::default() + }, + Some(control_bearer), + ), + ] + ); + Ok(()) +} + // --------------------------------------------------------------------------- // `delete` of an observed-only sandbox (`delete_observed_sandbox`). // --------------------------------------------------------------------------- diff --git a/crates/libs/api/src/controllers/substrate/node_api_client.rs b/crates/libs/api/src/controllers/substrate/node_api_client.rs index 82ee1b76da..c51be3beff 100644 --- a/crates/libs/api/src/controllers/substrate/node_api_client.rs +++ b/crates/libs/api/src/controllers/substrate/node_api_client.rs @@ -150,7 +150,7 @@ impl RuntimeClient { .await } - pub(super) fn internal_client(&self) -> RuntimeInternalClient { + pub(in crate::controllers) fn internal_client(&self) -> RuntimeInternalClient { WorkerApiInternalClient::new(node_api_channel( self.channel.clone(), SpaceBearerToken::NodeApiInternal, From 725f217eb2249d279104685a6d2ad9663a17f259 Mon Sep 17 00:00:00 2001 From: Abhinav Chordia Date: Fri, 30 Oct 2026 21:43:36 +0000 Subject: [PATCH 2/4] node-api: remove WorkerApi RPCs that have no caller WorkerApi becomes a 1.x-stable contract at 1.0, and 0.9 is where breaking changes consolidate. Remove 17 relays nothing calls: - GetSandboxHealth, ListSandboxes, SpacedPingGuest - WakeSandbox (the public twin; WorkerApiInternal.WakeSandbox stays) - EnableSnapshots, DisableSnapshots, DeleteSnapshots - ListTemplates, GetTemplate, UpdateTemplateSettings, UpdateTemplateSnapshotPolicy, CreatePool, DeletePool, PoolStatus - UnexposePort, DenyPort - RefreshFacetState Their functions are served elsewhere: template, snapshot, port and facet state arrive through node-ctl facets that call the node services over local sockets, the node-local CLIs cover operator use, and control-plane wakes use WorkerApiInternal.WakeSandbox. Protobuf services cannot reserve names, so worker.proto lists the retired names in a comment. Every message stays: each is imported from an internal package or shared with another service. Delete the node-api handlers, the simulator's handlers, the client-crate wrappers, SpacedPingGuest from the guest-bridge allowlist, and the public wake guards only the removed handler used. Tests that named a removed method now use one that remains, the runtime-upgrade wake test covers the internal service only, and the template and policy authorization tests cover the pool and settings RPCs on the builder only. Docs and the Kind chart comment no longer list worker pool mutation or a public Wake. A caller outside this repository that calls a removed RPC now gets UNIMPLEMENTED. --- crates/libs/common/svc/src/auth/k8s.rs | 3 +- crates/libs/common/svc/src/auth/k8s/tests.rs | 4 +- crates/libs/node/api/client/src/lib.rs | 17 -- .../server/src/clients/policy_auth_tests.rs | 23 +-- .../server/src/clients/template_auth_tests.rs | 14 +- crates/libs/node/api/server/src/worker.rs | 175 +----------------- .../libs/node/api/server/src/worker/tests.rs | 42 +---- crates/libs/node/api/sim/src/worker.rs | 172 +---------------- crates/libs/node/api/sim/src/worker/tests.rs | 30 +-- docs/configuration.md | 15 +- docs/execution-environments.md | 6 +- infra/space/kind/README.md | 2 +- .../charts/space-node-api-local/values.yaml | 4 +- proto/node/api/v1/worker.proto | 24 +-- 14 files changed, 55 insertions(+), 476 deletions(-) diff --git a/crates/libs/common/svc/src/auth/k8s.rs b/crates/libs/common/svc/src/auth/k8s.rs index 95e46bc9c4..e516b983b6 100644 --- a/crates/libs/common/svc/src/auth/k8s.rs +++ b/crates/libs/common/svc/src/auth/k8s.rs @@ -747,8 +747,7 @@ fn guest_bridge_method(role: NodeRole, path: &str) -> bool { }; matches!( method, - "SpacedPingGuest" - | "SpacedGetStatus" + "SpacedGetStatus" | "SpacedWaitReady" | "SpacedStat" | "SpacedList" diff --git a/crates/libs/common/svc/src/auth/k8s/tests.rs b/crates/libs/common/svc/src/auth/k8s/tests.rs index eb20cd20c6..99cda6f6b8 100644 --- a/crates/libs/common/svc/src/auth/k8s/tests.rs +++ b/crates/libs/common/svc/src/auth/k8s/tests.rs @@ -77,7 +77,7 @@ async fn data_plane_identity_cannot_reuse_guest_authority_for_control_or_transfe ("/space.node.api.v1.WorkerApi/ServicePortIngress", true), ("/space.node.api.v1.NodeInfo/GetNodeInfo", true), ("/space.node.api.v1.WorkerApi/CreateSandboxSnapshot", false), - ("/space.node.api.v1.WorkerApi/ListSandboxes", false), + ("/space.node.api.v1.WorkerApi/KillSandbox", false), ( "/space.node.api.v1.WorkerApi/SpacedFuturePrivilegedMethod", false, @@ -311,7 +311,7 @@ async fn peer_node_identity_is_confined_to_template_export() { reviewer_returning_status(config, token_review_status("space-node-api", username)); let token = projected_token(); for (method, allowed) in [ - ("/space.node.api.v1.WorkerApi/ListSandboxes", false), + ("/space.node.api.v1.WorkerApi/KillSandbox", false), ("/space.node.api.v1.WorkerApi/NodeExportTemplate", true), ("/space.node.api.v1.WorkerApi/SandboxState", false), ("/space.node.api.v1.WorkerApi/SandboxStats", false), diff --git a/crates/libs/node/api/client/src/lib.rs b/crates/libs/node/api/client/src/lib.rs index e2a4ffee41..1b59281ef4 100644 --- a/crates/libs/node/api/client/src/lib.rs +++ b/crates/libs/node/api/client/src/lib.rs @@ -464,11 +464,8 @@ impl<'a> WorkerApiClient<'a> { kill_sandbox, proto::worker_api_client::WorkerApiClient::new, pplx_space_sbx_mgr_client::proto::KillRequest, pplx_space_sbx_mgr_client::proto::KillResponse; delete_sandbox, proto::worker_api_client::WorkerApiClient::new, pplx_space_sbx_mgr_client::proto::DeleteRequest, pplx_space_sbx_mgr_client::proto::DeleteResponse; sandbox_state, proto::worker_api_client::WorkerApiClient::new, pplx_space_sbx_mgr_client::proto::StateRequest, pplx_space_sbx_mgr_client::proto::StateResponse; - get_sandbox_health, proto::worker_api_client::WorkerApiClient::new, pplx_space_sbx_mgr_client::proto::GetSandboxHealthRequest, pplx_space_sbx_mgr_client::proto::GetSandboxHealthResponse; - list_sandboxes, proto::worker_api_client::WorkerApiClient::new, pplx_space_sbx_mgr_client::proto::ListRequest, pplx_space_sbx_mgr_client::proto::ListResponse; pause_sandbox, proto::worker_api_client::WorkerApiClient::new, pplx_space_sbx_mgr_client::proto::PauseRequest, pplx_space_sbx_mgr_client::proto::PauseResponse; resume_sandbox, proto::worker_api_client::WorkerApiClient::new, pplx_space_sbx_mgr_client::proto::ResumeRequest, pplx_space_sbx_mgr_client::proto::ResumeResponse; - wake_sandbox, proto::worker_api_client::WorkerApiClient::new, pplx_space_sbx_mgr_client::proto::WakeRequest, pplx_space_sbx_mgr_client::proto::WakeResponse; create_sandbox_snapshot, proto::worker_api_client::WorkerApiClient::new, pplx_space_sbx_mgr_client::proto::CreateSnapshotRequest, pplx_space_sbx_mgr_client::proto::CreateSnapshotResponse; suspend_sandbox, proto::worker_api_client::WorkerApiClient::new, pplx_space_sbx_mgr_client::proto::SuspendRequest, pplx_space_sbx_mgr_client::proto::SuspendResponse; prepare_teardown_suspend, proto::worker_api_client::WorkerApiClient::new, pplx_space_sbx_mgr_client::proto::PrepareTeardownSuspendRequest, pplx_space_sbx_mgr_client::proto::PrepareTeardownSuspendResponse; @@ -478,7 +475,6 @@ impl<'a> WorkerApiClient<'a> { set_sandbox_auto_lifecycle_policy, proto::worker_api_client::WorkerApiClient::new, pplx_space_sbx_mgr_client::proto::SetAutoLifecyclePolicyRequest, pplx_space_sbx_mgr_client::proto::SetAutoLifecyclePolicyResponse; touch_sandbox_auto_lifecycle, proto::worker_api_client::WorkerApiClient::new, pplx_space_sbx_mgr_client::proto::TouchAutoLifecycleRequest, pplx_space_sbx_mgr_client::proto::TouchAutoLifecycleResponse; set_sandbox_network_policy, proto::worker_api_client::WorkerApiClient::new, pplx_space_sbx_mgr_client::proto::SetNetworkPolicyRequest, pplx_space_sbx_mgr_client::proto::SetNetworkPolicyResponse; - spaced_ping_guest, proto::worker_api_client::WorkerApiClient::new, pplx_space_sbx_mgr_client::proto::SpacedPingGuestRequest, pplx_space_sbx_mgr_client::proto::SpacedPingGuestResponse; spaced_get_status, proto::worker_api_client::WorkerApiClient::new, pplx_space_sbx_mgr_client::proto::SpacedGetStatusRequest, pplx_space_sbx_mgr_client::proto::SpacedGetStatusResponse; spaced_wait_ready, proto::worker_api_client::WorkerApiClient::new, pplx_space_sbx_mgr_client::proto::SpacedWaitReadyRequest, pplx_space_sbx_mgr_client::proto::SpacedWaitReadyResponse; spaced_stat, proto::worker_api_client::WorkerApiClient::new, pplx_space_sbx_mgr_client::proto::SpacedStatRequest, pplx_space_sbx_mgr_client::operations::StatResponse; @@ -501,27 +497,14 @@ impl<'a> WorkerApiClient<'a> { spaced_kill_process, proto::worker_api_client::WorkerApiClient::new, pplx_space_sbx_mgr_client::proto::SpacedKillProcessRequest, pplx_space_sbx_mgr_client::operations::KillResponse; spaced_open_loopback_bridge, proto::worker_api_client::WorkerApiClient::new, pplx_space_sbx_mgr_client::proto::SpacedOpenLoopbackBridgeRequest, pplx_space_sbx_mgr_client::operations::OpenLoopbackBridgeResponse; spaced_close_loopback_bridge, proto::worker_api_client::WorkerApiClient::new, pplx_space_sbx_mgr_client::proto::SpacedCloseLoopbackBridgeRequest, pplx_space_sbx_mgr_client::operations::CloseLoopbackBridgeResponse; - enable_snapshots, proto::worker_api_client::WorkerApiClient::new, pplx_space_sbx_mgr_client::proto::EnableSnapshotsRequest, pplx_space_sbx_mgr_client::proto::EnableSnapshotsResponse; - disable_snapshots, proto::worker_api_client::WorkerApiClient::new, pplx_space_sbx_mgr_client::proto::DisableSnapshotsRequest, pplx_space_sbx_mgr_client::proto::DisableSnapshotsResponse; list_snapshots, proto::worker_api_client::WorkerApiClient::new, pplx_space_sbx_mgr_client::proto::ListSnapshotsRequest, pplx_space_sbx_mgr_client::proto::ListSnapshotsResponse; - delete_snapshots, proto::worker_api_client::WorkerApiClient::new, pplx_space_sbx_mgr_client::proto::DeleteSnapshotsRequest, pplx_space_sbx_mgr_client::proto::DeleteSnapshotsResponse; rollback_sandbox, proto::worker_api_client::WorkerApiClient::new, pplx_space_sbx_mgr_client::proto::RollbackRequest, pplx_space_sbx_mgr_client::proto::RollbackResponse; delete_template, proto::worker_api_client::WorkerApiClient::new, pplx_space_template_mgr_client::proto::DeleteTemplateRequest, pplx_space_template_mgr_client::proto::DeleteTemplateResponse; delete_template_build, proto::worker_api_client::WorkerApiClient::new, pplx_space_template_mgr_client::proto::DeleteTemplateBuildRequest, pplx_space_template_mgr_client::proto::DeleteTemplateBuildResponse; - list_templates, proto::worker_api_client::WorkerApiClient::new, pplx_space_template_mgr_client::proto::ListTemplatesRequest, pplx_space_template_mgr_client::proto::ListTemplatesResponse; - get_template, proto::worker_api_client::WorkerApiClient::new, pplx_space_template_mgr_client::proto::GetTemplateRequest, pplx_space_template_mgr_client::proto::GetTemplateResponse; - update_template_settings, proto::worker_api_client::WorkerApiClient::new, pplx_space_template_mgr_client::proto::UpdateTemplateSettingsRequest, pplx_space_template_mgr_client::proto::UpdateTemplateSettingsResponse; - update_template_snapshot_policy, proto::worker_api_client::WorkerApiClient::new, pplx_space_template_mgr_client::proto::UpdateTemplateSnapshotPolicyRequest, pplx_space_template_mgr_client::proto::UpdateTemplateSnapshotPolicyResponse; - create_pool, proto::worker_api_client::WorkerApiClient::new, pplx_space_template_mgr_client::proto::CreatePoolRequest, pplx_space_template_mgr_client::proto::CreatePoolResponse; - delete_pool, proto::worker_api_client::WorkerApiClient::new, pplx_space_template_mgr_client::proto::DeletePoolRequest, pplx_space_template_mgr_client::proto::DeletePoolResponse; - pool_status, proto::worker_api_client::WorkerApiClient::new, pplx_space_template_mgr_client::proto::PoolStatusRequest, pplx_space_template_mgr_client::proto::PoolStatusResponse; expose_port, proto::worker_api_client::WorkerApiClient::new, pplx_space_node_mgr_client::proto::ExposePortRequest, pplx_space_node_mgr_client::proto::ExposePortResponse; - unexpose_port, proto::worker_api_client::WorkerApiClient::new, pplx_space_node_mgr_client::proto::UnexposePortRequest, pplx_space_node_mgr_client::proto::UnexposePortResponse; allow_port, proto::worker_api_client::WorkerApiClient::new, pplx_space_node_mgr_client::proto::AllowPortRequest, pplx_space_node_mgr_client::proto::AllowPortResponse; - deny_port, proto::worker_api_client::WorkerApiClient::new, pplx_space_node_mgr_client::proto::DenyPortRequest, pplx_space_node_mgr_client::proto::DenyPortResponse; pull_template, proto::worker_api_client::WorkerApiClient::new, proto::PullTemplateRequest, proto::PullTemplateResponse; list_node_templates, proto::worker_api_client::WorkerApiClient::new, proto::ListNodeTemplatesRequest, proto::ListNodeTemplatesResponse; - refresh_facet_state, proto::worker_api_client::WorkerApiClient::new, proto::RefreshFacetStateRequest, proto::RefreshFacetStateResponse; trigger_state_reconciliation, proto::worker_api_client::WorkerApiClient::new, proto::TriggerStateReconciliationRequest, proto::TriggerStateReconciliationResponse; } } diff --git a/crates/libs/node/api/server/src/clients/policy_auth_tests.rs b/crates/libs/node/api/server/src/clients/policy_auth_tests.rs index c0555ac8af..a454105615 100644 --- a/crates/libs/node/api/server/src/clients/policy_auth_tests.rs +++ b/crates/libs/node/api/server/src/clients/policy_auth_tests.rs @@ -137,7 +137,6 @@ pub(super) fn bearer(body: T, token: &str) -> Request { enum Rpc { Create, Adopt, - WorkerTemplate, BuilderTemplate, Network, Lifecycle, @@ -157,7 +156,6 @@ async fn policy_rpc_authentication_reaches_handlers() { for rpc in [ Rpc::Create, Rpc::Adopt, - Rpc::WorkerTemplate, Rpc::BuilderTemplate, Rpc::Network, Rpc::Lifecycle, @@ -220,8 +218,8 @@ async fn policy_rpc_authentication_reaches_handlers() { )) .await .map(|_| ()), - Rpc::WorkerTemplate | Rpc::BuilderTemplate => { - let request = bearer( + Rpc::BuilderTemplate => BuilderApiClient::new(service.clone()) + .update_template_settings(bearer( tmpl::UpdateTemplateSettingsRequest { network_policy, default_auto_lifecycle: auto_lifecycle, @@ -231,18 +229,9 @@ async fn policy_rpc_authentication_reaches_handlers() { ..Default::default() }, token, - ); - match rpc { - Rpc::WorkerTemplate => WorkerApiClient::new(service.clone()) - .update_template_settings(request) - .await - .map(|_| ()), - _ => BuilderApiClient::new(service.clone()) - .update_template_settings(request) - .await - .map(|_| ()), - } - } + )) + .await + .map(|_| ()), Rpc::Network => WorkerApiClient::new(service.clone()) .set_sandbox_network_policy(bearer( sbx::SetNetworkPolicyRequest { @@ -281,7 +270,7 @@ async fn policy_rpc_authentication_reaches_handlers() { let requires_authority = match rpc { Rpc::Network | Rpc::Lifecycle => true, Rpc::Create | Rpc::Adopt => network || lifecycle || generation != 0, - Rpc::WorkerTemplate | Rpc::BuilderTemplate => network || lifecycle, + Rpc::BuilderTemplate => network || lifecycle, }; let allowed = trusted || !requires_authority; assert_eq!( diff --git a/crates/libs/node/api/server/src/clients/template_auth_tests.rs b/crates/libs/node/api/server/src/clients/template_auth_tests.rs index 266dea55c3..37ab14e8a5 100644 --- a/crates/libs/node/api/server/src/clients/template_auth_tests.rs +++ b/crates/libs/node/api/server/src/clients/template_auth_tests.rs @@ -50,6 +50,11 @@ async fn template_mutation_authority_survives_the_wire( )] rpc: TemplateRpc, ) { + if role == NodeRole::Worker && matches!(rpc, TemplateRpc::CreatePool | TemplateRpc::DeletePool) + { + // Only the builder serves pool RPCs. + return; + } let (clients, calls, shutdown, mut servers) = backend().await; let _cancel_on_drop = shutdown.clone().drop_guard(); let auth = auth_layer(role, k8s, username, Arc::new(AtomicUsize::new(0))); @@ -92,6 +97,11 @@ async fn template_mutation_authority_survives_the_wire( } }}; } + macro_rules! builder_call { + ($method:ident, $body:expr) => { + builder.$method(bearer($body, token)).await.map(|_| ()) + }; + } let result = match rpc { TemplateRpc::DeleteTemplate => { call!(delete_template, tmpl::DeleteTemplateRequest::default()) @@ -100,8 +110,8 @@ async fn template_mutation_authority_survives_the_wire( delete_template_build, tmpl::DeleteTemplateBuildRequest::default() ), - TemplateRpc::CreatePool => call!(create_pool, tmpl::CreatePoolRequest::default()), - TemplateRpc::DeletePool => call!(delete_pool, tmpl::DeletePoolRequest::default()), + TemplateRpc::CreatePool => builder_call!(create_pool, tmpl::CreatePoolRequest::default()), + TemplateRpc::DeletePool => builder_call!(delete_pool, tmpl::DeletePoolRequest::default()), }; assert_eq!( diff --git a/crates/libs/node/api/server/src/worker.rs b/crates/libs/node/api/server/src/worker.rs index 619d78531d..7f7f128048 100644 --- a/crates/libs/node/api/server/src/worker.rs +++ b/crates/libs/node/api/server/src/worker.rs @@ -2,10 +2,7 @@ use crate::clients::BackendClients; use crate::node_ctl; use crate::proto::worker_api_internal_server::WorkerApiInternal; use crate::proto::worker_api_server::WorkerApi; -use crate::proto::{ - RefreshFacetStateRequest, RefreshFacetStateResponse, TriggerStateReconciliationRequest, - TriggerStateReconciliationResponse, -}; +use crate::proto::{TriggerStateReconciliationRequest, TriggerStateReconciliationResponse}; use crate::restart_handoff::RpcReplaySafety; use crate::uds_per_sandbox_limiter::{SandboxBudget, UdsPerSandboxLimiter, UdsPerSandboxPermit}; use std::future::Future; @@ -408,7 +405,6 @@ sandbox_scoped_requests!( pplx_space_sbx_mgr_client::proto::SetAutoLifecyclePolicyRequest, pplx_space_sbx_mgr_client::proto::TouchAutoLifecycleRequest, pplx_space_sbx_mgr_client::proto::SetNetworkPolicyRequest, - pplx_space_sbx_mgr_client::proto::SpacedPingGuestRequest, pplx_space_sbx_mgr_client::proto::SpacedGetStatusRequest, pplx_space_sbx_mgr_client::proto::SpacedWaitReadyRequest, pplx_space_sbx_mgr_client::proto::SpacedStatRequest, @@ -437,9 +433,6 @@ sandbox_scoped_requests!( pplx_space_sbx_mgr_client::proto::SpacedKillProcessRequest, pplx_space_sbx_mgr_client::proto::SpacedOpenLoopbackBridgeRequest, pplx_space_sbx_mgr_client::proto::SpacedCloseLoopbackBridgeRequest, - pplx_space_sbx_mgr_client::proto::EnableSnapshotsRequest, - pplx_space_sbx_mgr_client::proto::DisableSnapshotsRequest, - pplx_space_sbx_mgr_client::proto::DeleteSnapshotsRequest, pplx_space_sbx_mgr_client::proto::RollbackRequest, ); @@ -559,32 +552,6 @@ fn reject_public_suspend_restore_context( Ok(()) } -fn reject_public_wake_start_context( - rpc: &'static str, - start_context: &Option, -) -> Result<(), Status> { - if start_context.is_some() { - return Err(Status::invalid_argument(format!( - "{rpc}.start_context is internal-only; use WorkerApiInternal" - ))); - } - Ok(()) -} - -// Sandbox-manager authorizes an upgrade by its caller, which is node-api on -// both services, so the public service must refuse it itself. -fn reject_public_wake_runtime_upgrade( - rpc: &'static str, - runtime_upgrade: &Option, -) -> Result<(), Status> { - if runtime_upgrade.is_some() { - return Err(Status::invalid_argument(format!( - "{rpc}.runtime_upgrade is internal-only; use WorkerApiInternal" - ))); - } - Ok(()) -} - fn reject_public_operation_activation_attempt(operation: &Option) -> Result<(), Status> { if operation.is_some() { return Err(Status::invalid_argument( @@ -691,21 +658,6 @@ impl WorkerApi for WorkerApiService { fwd_sbx_read!(self, "SandboxState", state, r) } - async fn get_sandbox_health( - &self, - r: Request, - ) -> Result, Status> { - record_sandbox_id(&tracing::Span::current(), &r.get_ref().sandbox_id); - fwd_sbx_read!(self, "GetSandboxHealth", get_sandbox_health, r) - } - - async fn list_sandboxes( - &self, - r: Request, - ) -> Result, Status> { - fwd_sbx_read!(self, "ListSandboxes", list, r) - } - // ── Sandbox state management (sandbox-manager) ─────────────────────────── async fn pause_sandbox( @@ -727,16 +679,6 @@ impl WorkerApi for WorkerApiService { fwd_sbx!(self, "ResumeSandbox", resume, r) } - async fn wake_sandbox( - &self, - r: Request, - ) -> Result, Status> { - record_sandbox_id(&tracing::Span::current(), &r.get_ref().sandbox_id); - reject_public_wake_start_context("WakeSandbox", &r.get_ref().start_context)?; - reject_public_wake_runtime_upgrade("WakeSandbox", &r.get_ref().runtime_upgrade)?; - fwd_sbx!(self, "WakeSandbox", wake, r) - } - async fn create_sandbox_snapshot( &self, r: Request, @@ -819,13 +761,6 @@ impl WorkerApi for WorkerApiService { fwd_sbx!(self, "SetSandboxNetworkPolicy", set_network_policy, r) } - async fn spaced_ping_guest( - &self, - r: Request, - ) -> Result, Status> { - fwd_sbx!(self, "SpacedPingGuest", spaced_ping_guest, r) - } - async fn spaced_get_status( &self, r: Request, @@ -1187,20 +1122,6 @@ impl WorkerApi for WorkerApiService { // ── Sandbox snapshots (sandbox-manager) ────────────────────────────────── - async fn enable_snapshots( - &self, - r: Request, - ) -> Result, Status> { - fwd_sbx!(self, "EnableSnapshots", enable_snapshots, r) - } - - async fn disable_snapshots( - &self, - r: Request, - ) -> Result, Status> { - fwd_sbx!(self, "DisableSnapshots", disable_snapshots, r) - } - async fn list_snapshots( &self, r: Request, @@ -1208,13 +1129,6 @@ impl WorkerApi for WorkerApiService { fwd_sbx_read!(self, "ListSandboxSnapshots", list_snapshots, r) } - async fn delete_snapshots( - &self, - r: Request, - ) -> Result, Status> { - fwd_sbx!(self, "DeleteSnapshots", delete_snapshots, r) - } - async fn rollback_sandbox( &self, r: Request, @@ -1243,73 +1157,8 @@ impl WorkerApi for WorkerApiService { fwd!(self.clients.tmpl(), delete_template_build, r) } - async fn list_templates( - &self, - r: Request, - ) -> Result, Status> - { - fwd_retry!("template-manager", self.clients.tmpl(), list_templates, r) - } - - async fn get_template( - &self, - mut r: Request, - ) -> Result, Status> { - let scope = crate::template_lookup::bind_caller_scope(&mut r)?; - let response = fwd_retry!("template-manager", self.clients.tmpl(), get_template, r)?; - scope.observe_response(&response)?; - Ok(response) - } - - async fn update_template_settings( - &self, - r: Request, - ) -> Result< - Response, - Status, - > { - let req = r.get_ref(); - if req.network_policy.is_some() || req.default_auto_lifecycle.is_some() { - require_policy_mutation_authority(&r)?; - } - fwd!(self.clients.tmpl(), update_template_settings, r) - } - - async fn update_template_snapshot_policy( - &self, - r: Request, - ) -> Result< - Response, - Status, - > { - fwd!(self.clients.tmpl(), update_template_snapshot_policy, r) - } - // ── Template pools (template-manager) ──────────────────────────────────── - async fn create_pool( - &self, - r: Request, - ) -> Result, Status> { - pplx_space_common_svc::auth::template::require_template_mutation_authority(&r)?; - fwd!(self.clients.tmpl(), create_pool, r) - } - - async fn delete_pool( - &self, - r: Request, - ) -> Result, Status> { - pplx_space_common_svc::auth::template::require_template_mutation_authority(&r)?; - fwd!(self.clients.tmpl(), delete_pool, r) - } - - async fn pool_status( - &self, - r: Request, - ) -> Result, Status> { - fwd_retry!("template-manager", self.clients.tmpl(), pool_status, r) - } - // ── Network (node-manager) ─────────────────────────────────────────────── async fn expose_port( @@ -1319,13 +1168,6 @@ impl WorkerApi for WorkerApiService { fwd!(self.clients.node(), expose_port, r) } - async fn unexpose_port( - &self, - r: Request, - ) -> Result, Status> { - fwd!(self.clients.node(), unexpose_port, r) - } - async fn allow_port( &self, r: Request, @@ -1339,23 +1181,8 @@ impl WorkerApi for WorkerApiService { result } - async fn deny_port( - &self, - r: Request, - ) -> Result, Status> { - fwd!(self.clients.node(), deny_port, r) - } - // ── Custom orchestration ───────────────────────────────────────────────── - async fn refresh_facet_state( - &self, - r: Request, - ) -> Result, Status> { - node_ctl::refresh_facet_state(r.into_inner()).await?; - Ok(Response::new(RefreshFacetStateResponse {})) - } - async fn trigger_state_reconciliation( &self, r: Request, diff --git a/crates/libs/node/api/server/src/worker/tests.rs b/crates/libs/node/api/server/src/worker/tests.rs index e8ca79f964..ab91f9f4d2 100644 --- a/crates/libs/node/api/server/src/worker/tests.rs +++ b/crates/libs/node/api/server/src/worker/tests.rs @@ -58,15 +58,6 @@ fn public_worker_api_rejects_explicit_netns_path() { assert!(error.message().contains("internal-only")); } -#[test] -fn public_worker_api_rejects_wake_start_context() { - let start_context = Some(pplx_space_sbx_mgr_client::proto::WakeStartContext::default()); - let error = reject_public_wake_start_context("WakeSandbox", &start_context).unwrap_err(); - - assert_eq!(error.code(), tonic::Code::InvalidArgument); - assert!(error.message().contains("internal-only")); -} - #[test] fn public_worker_api_rejects_explicit_suspend_restore_storage_uri() { let storage_uri = Some("s3://space/snapshot?region=us-east-1".to_string()); @@ -288,33 +279,23 @@ async fn sandbox_kill_is_admitted_while_every_sandbox_permit_is_held() { assert_eq!(sandbox_kill.unwrap_err().code(), tonic::Code::Unimplemented); } -/// Sandbox-manager authorizes an upgrade by its caller, which is node-api on -/// either service, so the public service refuses one before forwarding and -/// only the internal one forwards it, for the control plane. +/// Sandbox-manager authorizes an upgrade by its caller, which is node-api, so +/// node-api's internal Wake forwards one only for the control plane. #[tokio::test] -async fn only_the_internal_wake_forwards_a_runtime_upgrade() { +async fn the_internal_wake_forwards_a_runtime_upgrade_only_for_the_control_plane() { use pplx_space_node_api_client::proto::worker_api_internal_server::WorkerApiInternal; - use pplx_space_node_api_client::proto::worker_api_server::WorkerApi; let service = service_with_limiter(UdsPerSandboxLimiter::for_tests_with_reserved(1, 1, 8)).await; - let wake = |runtime_upgrade| { + let wake = || { Request::new(proto::WakeRequest { sandbox_id: SandboxId::new().to_string(), blocking: true, - runtime_upgrade, + runtime_upgrade: Some(proto::WakeRuntimeUpgrade::default()), ..Default::default() }) }; - let upgrade = || Some(proto::WakeRuntimeUpgrade::default()); - let refused = WorkerApi::wake_sandbox(&service, wake(upgrade())) - .await - .unwrap_err(); - assert_eq!(refused.code(), tonic::Code::InvalidArgument, "{refused}"); - assert!(refused.message().contains("internal-only"), "{refused}"); - - // A data-plane caller may not run one even on the internal service. - let mut unprivileged = wake(upgrade()); + let mut unprivileged = wake(); unprivileged.extensions_mut().insert( pplx_space_common_svc::auth::NodeApiAuthenticatedCaller::unprivileged_workload_for_test(), ); @@ -324,11 +305,8 @@ async fn only_the_internal_wake_forwards_a_runtime_upgrade() { assert_eq!(denied.code(), tonic::Code::PermissionDenied, "{denied}"); // The fake sandbox-manager answers whatever reaches it with Unimplemented. - for forwarded in [ - WorkerApi::wake_sandbox(&service, wake(None)).await, - WorkerApiInternal::wake_sandbox(&service, wake(upgrade())).await, - ] { - let forwarded = forwarded.unwrap_err(); - assert_eq!(forwarded.code(), tonic::Code::Unimplemented, "{forwarded}"); - } + let forwarded = WorkerApiInternal::wake_sandbox(&service, wake()) + .await + .unwrap_err(); + assert_eq!(forwarded.code(), tonic::Code::Unimplemented, "{forwarded}"); } diff --git a/crates/libs/node/api/sim/src/worker.rs b/crates/libs/node/api/sim/src/worker.rs index b59f243440..a4ce4fc0da 100644 --- a/crates/libs/node/api/sim/src/worker.rs +++ b/crates/libs/node/api/sim/src/worker.rs @@ -22,8 +22,7 @@ use pplx_space_node_api_client::proto::worker_api_internal_server::WorkerApiInte use pplx_space_node_api_client::proto::worker_api_server::WorkerApi; use pplx_space_node_api_client::proto::{ ListNodeTemplatesRequest, ListNodeTemplatesResponse, PullTemplateRequest, PullTemplateResponse, - RefreshFacetStateRequest, RefreshFacetStateResponse, TriggerStateReconciliationRequest, - TriggerStateReconciliationResponse, + TriggerStateReconciliationRequest, TriggerStateReconciliationResponse, }; use pplx_space_sbx_mgr_client::proto as sbx; use pplx_space_template_mgr_client::proto as tmpl; @@ -326,19 +325,6 @@ fn sandbox_info_from_record(rec: &crate::state::SandboxRecord) -> sbx::SandboxIn } } -fn list_element_from_record(rec: &crate::state::SandboxRecord) -> sbx::SandboxListElement { - sbx::SandboxListElement { - lifecycle: Some(sim_lifecycle(&rec.status) as i32), - sandbox_id: rec.sandbox_id.clone(), - template: Some(rec.template.clone()), - image_hash: rec.content_hash.clone(), - pool_template: rec.pool_template.clone(), - runtime: Some(rec.runtime.clone()), - created_at: Some(rec.created_at.clone()), - owner: Some(rec.user.clone()), - } -} - /// Implementation of SpacedWrite shared by the trait method and unit tests. /// Tonic's trait signature pins the stream type to `tonic::Streaming<_>`, /// which tests can't construct from a plain channel, so we extract the logic @@ -649,34 +635,6 @@ impl WorkerApi for SimWorkerApiService { })) } - async fn get_sandbox_health( - &self, - request: Request, - ) -> Result, Status> { - let sandbox_id = request.into_inner().sandbox_id; - self.state.get(&sandbox_id).await.map_err(map_state_error)?; - Ok(Response::new(sbx::GetSandboxHealthResponse { - sandbox_id, - status: sbx::SandboxHealthStatus::Healthy as i32, - evaluated_at: None, - degraded_components: Vec::new(), - components: Vec::new(), - })) - } - - async fn list_sandboxes( - &self, - _request: Request, - ) -> Result, Status> { - let recs = self.state.list().await; - // The simulated inventory is small: answer every `page_size` with one - // final page, which paging clients accept as the end of the scan. - Ok(Response::new(sbx::ListResponse { - sandboxes: recs.iter().map(list_element_from_record).collect(), - next_page_token: String::new(), - })) - } - // ── Sandbox state management ───────────────────────────────────────────── async fn pause_sandbox( @@ -729,27 +687,6 @@ impl WorkerApi for SimWorkerApiService { })) } - async fn wake_sandbox( - &self, - request: Request, - ) -> Result, Status> { - let req = request.into_inner(); - if req.start_context.is_some() { - return Err(Status::invalid_argument( - "WakeSandbox.start_context is internal-only; use WorkerApiInternal", - )); - } - sim_wake( - &self.state, - &req.sandbox_id, - None, - req.implicit_wake_source, - None, - None, - ) - .await - } - async fn create_sandbox_snapshot( &self, request: Request, @@ -1051,18 +988,6 @@ impl WorkerApi for SimWorkerApiService { // ── Spaced guest operations: ping/status/wait_ready stub healthy ───────── - async fn spaced_ping_guest( - &self, - request: Request, - ) -> Result, Status> { - let req = request.into_inner(); - self.state - .ensure_default_running(&req.sandbox_id) - .await - .map_err(map_state_error)?; - Ok(Response::new(sbx::SpacedPingGuestResponse {})) - } - async fn spaced_get_status( &self, request: Request, @@ -1583,20 +1508,6 @@ impl WorkerApi for SimWorkerApiService { // ── Sandbox snapshots (templates have their own pull/export) ───────────── - async fn enable_snapshots( - &self, - _request: Request, - ) -> Result, Status> { - unimplemented() - } - - async fn disable_snapshots( - &self, - _request: Request, - ) -> Result, Status> { - unimplemented() - } - async fn list_snapshots( &self, _request: Request, @@ -1604,13 +1515,6 @@ impl WorkerApi for SimWorkerApiService { unimplemented() } - async fn delete_snapshots( - &self, - _request: Request, - ) -> Result, Status> { - unimplemented() - } - async fn rollback_sandbox( &self, _request: Request, @@ -1634,55 +1538,6 @@ impl WorkerApi for SimWorkerApiService { unimplemented() } - async fn list_templates( - &self, - _request: Request, - ) -> Result, Status> { - unimplemented() - } - - async fn get_template( - &self, - _request: Request, - ) -> Result, Status> { - unimplemented() - } - - async fn update_template_settings( - &self, - _request: Request, - ) -> Result, Status> { - unimplemented() - } - - async fn update_template_snapshot_policy( - &self, - _request: Request, - ) -> Result, Status> { - unimplemented() - } - - async fn create_pool( - &self, - _request: Request, - ) -> Result, Status> { - unimplemented() - } - - async fn delete_pool( - &self, - _request: Request, - ) -> Result, Status> { - unimplemented() - } - - async fn pool_status( - &self, - _request: Request, - ) -> Result, Status> { - unimplemented() - } - // ── Network (no-op; preserve transitions reconcilers expect) ───────────── async fn expose_port( @@ -1694,15 +1549,6 @@ impl WorkerApi for SimWorkerApiService { )) } - async fn unexpose_port( - &self, - _request: Request, - ) -> Result, Status> { - Ok(Response::new( - pplx_space_node_mgr_client::proto::UnexposePortResponse {}, - )) - } - async fn allow_port( &self, _request: Request, @@ -1712,24 +1558,8 @@ impl WorkerApi for SimWorkerApiService { )) } - async fn deny_port( - &self, - _request: Request, - ) -> Result, Status> { - Ok(Response::new( - pplx_space_node_mgr_client::proto::DenyPortResponse {}, - )) - } - // ── Custom orchestration ───────────────────────────────────────────────── - async fn refresh_facet_state( - &self, - _request: Request, - ) -> Result, Status> { - Ok(Response::new(RefreshFacetStateResponse {})) - } - async fn trigger_state_reconciliation( &self, _request: Request, diff --git a/crates/libs/node/api/sim/src/worker/tests.rs b/crates/libs/node/api/sim/src/worker/tests.rs index acc77e0989..6dba14dd57 100644 --- a/crates/libs/node/api/sim/src/worker/tests.rs +++ b/crates/libs/node/api/sim/src/worker/tests.rs @@ -91,7 +91,7 @@ fn simulated_kubernetes_identity_fills_a_canonical_node_id() { } #[tokio::test] -async fn create_then_state_then_start_then_list() { +async fn create_then_state_then_start() { let (_dir, svc, _internal, _state) = fresh_pair("node-a"); let create = svc @@ -135,17 +135,6 @@ async fn create_then_state_then_start_then_list() { })) .await .unwrap(); - - let list = svc - .list_sandboxes(Request::new(sbx::ListRequest::default())) - .await - .unwrap() - .into_inner(); - assert_eq!(list.sandboxes.len(), 1); - assert_eq!( - list.sandboxes[0].observed_lifecycle().unwrap(), - sbx::SandboxLifecycleState::Started - ); } #[tokio::test] @@ -493,23 +482,6 @@ async fn spaced_write_rejects_path_traversal() { assert_eq!(err.code(), tonic::Code::InvalidArgument); } -#[tokio::test] -async fn public_wake_rejects_start_context() { - let (_dir, svc, _internal, _state) = fresh_pair("node-a"); - let err = svc - .wake_sandbox(Request::new(sbx::WakeRequest { - implicit_wake_source: 0, - sandbox_id: "sbx-1".into(), - blocking: true, - start_context: Some(sbx::WakeStartContext::default()), - target_state: 0, - runtime_upgrade: None, - })) - .await - .unwrap_err(); - assert_eq!(err.code(), tonic::Code::InvalidArgument); -} - #[tokio::test] async fn generation_suspend_returns_exact_artifact_and_fences_older_requests() { let (_dir, svc, _internal, state) = fresh_pair("node-a"); diff --git a/docs/configuration.md b/docs/configuration.md index 8f1bfde62a..850e6c1dac 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -520,11 +520,12 @@ password sessions that have not expired become valid again. ### Template mutation authority -On worker and builder node-api, `DeleteTemplate`, `DeleteTemplateBuild`, -`CreatePool`, and `DeletePool` require the trusted policy service-account -capability. Template-manager's local roles and deletion dependency guards still -apply. Pool target counts have no capacity-based upper bound yet; operators -must budget pool sizes even after caller authorization succeeds. +On worker and builder node-api, `DeleteTemplate` and `DeleteTemplateBuild` +require the trusted policy service-account capability, as do builder +node-api's `CreatePool` and `DeletePool`. Template-manager's local roles and +deletion dependency guards still apply. Pool target counts have no +capacity-based upper bound yet; operators must budget pool sizes even after +caller authorization succeeds. With Kubernetes authentication disabled, the shared internal token remains the operator trust boundary and retains mutation authority. When Kubernetes @@ -579,8 +580,8 @@ uses the internal surface, so its caller needs an internal-audience token. Shared static-token fallback cannot mutate policy when Kubernetes authentication is enabled; static-only deployments retain that authority. -Policy-bearing cold creation, internal pool adoption, and worker and builder -template updates require a trusted caller. Cold creation and pool adoption also +Policy-bearing cold creation, internal pool adoption, and builder template +updates require a trusted caller. Cold creation and pool adoption also require policy authority for a nonzero sandbox-policy generation, even without an override. Requests with no overrides and generation zero remain available to otherwise admitted callers and inherit the template's policy. Guest-bridge diff --git a/docs/execution-environments.md b/docs/execution-environments.md index d31f194471..56036f0add 100644 --- a/docs/execution-environments.md +++ b/docs/execution-environments.md @@ -1340,9 +1340,9 @@ populate it, every move off an environment that may not execute is stranded. upgraded: that's the cluster-side materialization for a sandbox with no pod. A sandbox-manager that predates the field ignores it and wakes normally, where its execution policy refuses a runtime that may not run. - Node-api forwards `runtime_upgrade` only on `WorkerApiInternal`, and only - for a caller with the control-plane authority a policy write needs; its - public `WakeSandbox` refuses the field. + Node-api serves Wake only on `WorkerApiInternal`, and forwards + `runtime_upgrade` only for a caller with the control-plane authority a + policy write needs. A suspended sandbox's checkpoint on the node is booted only if it's the one the upgrade was decided from: the Wake names that checkpoint's local artifact, and the row must pin it, as it does on the checkpoint's source diff --git a/infra/space/kind/README.md b/infra/space/kind/README.md index 612d4087d7..f2ea7aa8ce 100644 --- a/infra/space/kind/README.md +++ b/infra/space/kind/README.md @@ -1913,7 +1913,7 @@ and the target node-api's export request to the source: |---|---| | worker WorkerApi, NodeInfo | `space-api` (shared and admin api), `space-cluster-controller`, `space-data-plane` (guest bridge, guest calls only), `space-internal-api`, both node-api accounts (template transfers) | | worker WorkerApiInternal | `space-api`, `space-cluster-controller` | -| worker lifecycle-probe `SpacedWaitReady`, caller-supplied policy, template and pool mutation | `space-api`, `space-cluster-controller` | +| worker lifecycle-probe `SpacedWaitReady`, caller-supplied policy, template deletion | `space-api`, `space-cluster-controller` | | builder BuilderApi, NodeInfo | `space-api`, `space-cluster-controller`, both node-api accounts | | builder `BuildTemplate` (system and tenant), policy, template deletion | `space-api` | | peer template transfers (`PullTemplate`, `NodeExportTemplate`), both roles | `space-api` (starts a pull), both node-api accounts (the target asks the source for the export) | diff --git a/infra/space/kind/charts/space-node-api-local/values.yaml b/infra/space/kind/charts/space-node-api-local/values.yaml index 78d99a875c..967cac5d06 100644 --- a/infra/space/kind/charts/space-node-api-local/values.yaml +++ b/infra/space/kind/charts/space-node-api-local/values.yaml @@ -55,8 +55,8 @@ auth: SPACE_NODE_API_AUTH_TRUSTED_WAIT_READY_SERVICE_ACCOUNTS: - space-api - space-cluster-controller - # Caller-supplied network and auto-lifecycle policy, template and pool - # mutation (worker.rs, auth/template.rs): the substrate's creates, starts, + # Caller-supplied network and auto-lifecycle policy and template deletion + # (worker.rs, auth/template.rs): the substrate's creates, starts, # wakes and policy updates. Required on both roles (validate_for_role). SPACE_NODE_API_AUTH_TRUSTED_POLICY_SERVICE_ACCOUNTS: - space-api diff --git a/proto/node/api/v1/worker.proto b/proto/node/api/v1/worker.proto index c9745fbe63..0e95574be1 100644 --- a/proto/node/api/v1/worker.proto +++ b/proto/node/api/v1/worker.proto @@ -10,6 +10,13 @@ import "node/api/v1/common.proto"; // WorkerApi is the external API for a SPACE worker node. // Exposes full sandbox lifecycle, template management, and network operations. +// +// Retired in 0.9.0; services cannot reserve names, so do not reuse these: +// GetSandboxHealth, ListSandboxes, WakeSandbox (WorkerApiInternal keeps its +// own), SpacedPingGuest, EnableSnapshots, DisableSnapshots, DeleteSnapshots, +// ListTemplates, GetTemplate, UpdateTemplateSettings, +// UpdateTemplateSnapshotPolicy, CreatePool, DeletePool, PoolStatus, +// UnexposePort, DenyPort, RefreshFacetState. service WorkerApi { // ── Sandbox lifecycle ────────────────────────────────────────────────────── @@ -19,14 +26,11 @@ service WorkerApi { rpc KillSandbox(space.sandbox.manager.v1.KillRequest) returns (space.sandbox.manager.v1.KillResponse); rpc DeleteSandbox(space.sandbox.manager.v1.DeleteRequest) returns (space.sandbox.manager.v1.DeleteResponse); rpc SandboxState(space.sandbox.manager.v1.StateRequest) returns (space.sandbox.manager.v1.StateResponse); - rpc GetSandboxHealth(space.sandbox.manager.v1.GetSandboxHealthRequest) returns (space.sandbox.manager.v1.GetSandboxHealthResponse); - rpc ListSandboxes(space.sandbox.manager.v1.ListRequest) returns (space.sandbox.manager.v1.ListResponse); // ── Sandbox state management ─────────────────────────────────────────────── rpc PauseSandbox(space.sandbox.manager.v1.PauseRequest) returns (space.sandbox.manager.v1.PauseResponse); rpc ResumeSandbox(space.sandbox.manager.v1.ResumeRequest) returns (space.sandbox.manager.v1.ResumeResponse); - rpc WakeSandbox(space.sandbox.manager.v1.WakeRequest) returns (space.sandbox.manager.v1.WakeResponse); rpc CreateSandboxSnapshot(space.sandbox.manager.v1.CreateSnapshotRequest) returns (space.sandbox.manager.v1.CreateSnapshotResponse); rpc SuspendSandbox(space.sandbox.manager.v1.SuspendRequest) returns (space.sandbox.manager.v1.SuspendResponse); rpc PrepareTeardownSuspend(space.sandbox.manager.v1.PrepareTeardownSuspendRequest) returns (space.sandbox.manager.v1.PrepareTeardownSuspendResponse); @@ -42,7 +46,6 @@ service WorkerApi { // ── Spaced guest operations (via sandbox-manager) ───────────────────────── - rpc SpacedPingGuest(space.sandbox.manager.v1.SpacedPingGuestRequest) returns (space.sandbox.manager.v1.SpacedPingGuestResponse); rpc SpacedGetStatus(space.sandbox.manager.v1.SpacedGetStatusRequest) returns (space.sandbox.manager.v1.SpacedGetStatusResponse); rpc SpacedWaitReady(space.sandbox.manager.v1.SpacedWaitReadyRequest) returns (space.sandbox.manager.v1.SpacedWaitReadyResponse); rpc SpacedStat(space.sandbox.manager.v1.SpacedStatRequest) returns (space.spaced.operations.v1.StatResponse); @@ -79,30 +82,18 @@ service WorkerApi { // ── Sandbox snapshots ────────────────────────────────────────────────────── - rpc EnableSnapshots(space.sandbox.manager.v1.EnableSnapshotsRequest) returns (space.sandbox.manager.v1.EnableSnapshotsResponse); - rpc DisableSnapshots(space.sandbox.manager.v1.DisableSnapshotsRequest) returns (space.sandbox.manager.v1.DisableSnapshotsResponse); rpc ListSnapshots(space.sandbox.manager.v1.ListSnapshotsRequest) returns (space.sandbox.manager.v1.ListSnapshotsResponse); - rpc DeleteSnapshots(space.sandbox.manager.v1.DeleteSnapshotsRequest) returns (space.sandbox.manager.v1.DeleteSnapshotsResponse); rpc RollbackSandbox(space.sandbox.manager.v1.RollbackRequest) returns (space.sandbox.manager.v1.RollbackResponse); // ── Template management (via template-manager) ───────────────────────────── rpc DeleteTemplate(space.template.manager.v1.DeleteTemplateRequest) returns (space.template.manager.v1.DeleteTemplateResponse); rpc DeleteTemplateBuild(space.template.manager.v1.DeleteTemplateBuildRequest) returns (space.template.manager.v1.DeleteTemplateBuildResponse); - rpc ListTemplates(space.template.manager.v1.ListTemplatesRequest) returns (space.template.manager.v1.ListTemplatesResponse); - rpc GetTemplate(space.template.manager.v1.GetTemplateRequest) returns (space.template.manager.v1.GetTemplateResponse); - rpc UpdateTemplateSettings(space.template.manager.v1.UpdateTemplateSettingsRequest) returns (space.template.manager.v1.UpdateTemplateSettingsResponse); - rpc UpdateTemplateSnapshotPolicy(space.template.manager.v1.UpdateTemplateSnapshotPolicyRequest) returns (space.template.manager.v1.UpdateTemplateSnapshotPolicyResponse); - rpc CreatePool(space.template.manager.v1.CreatePoolRequest) returns (space.template.manager.v1.CreatePoolResponse); - rpc DeletePool(space.template.manager.v1.DeletePoolRequest) returns (space.template.manager.v1.DeletePoolResponse); - rpc PoolStatus(space.template.manager.v1.PoolStatusRequest) returns (space.template.manager.v1.PoolStatusResponse); // ── Network (via node-manager) ───────────────────────────────────────────── rpc ExposePort(space.node.v1.ExposePortRequest) returns (space.node.v1.ExposePortResponse); - rpc UnexposePort(space.node.v1.UnexposePortRequest) returns (space.node.v1.UnexposePortResponse); rpc AllowPort(space.node.v1.AllowPortRequest) returns (space.node.v1.AllowPortResponse); - rpc DenyPort(space.node.v1.DenyPortRequest) returns (space.node.v1.DenyPortResponse); // ── Template transfer ─────────────────────────────────────────────────────── @@ -119,7 +110,6 @@ service WorkerApi { // ── Node-level orchestration ─────────────────────────────────────────────── - rpc RefreshFacetState(RefreshFacetStateRequest) returns (RefreshFacetStateResponse); rpc TriggerStateReconciliation(TriggerStateReconciliationRequest) returns (TriggerStateReconciliationResponse); } From 746b5f69ae24ea74b74a59019a891d6dda04d630 Mon Sep 17 00:00:00 2001 From: Abhinav Chordia Date: Fri, 30 Oct 2026 21:48:07 +0000 Subject: [PATCH 3/4] node-api: remove BuilderApi RPCs that have no caller Remove 12 BuilderApi relays nothing calls: - SandboxState, ListSandboxes, StopSandbox, DeleteSandbox - ListTemplates, GetTemplate, UpdateTemplateSettings, UpdateTemplateSnapshotPolicy, CreatePool, DeletePool, PoolStatus - RefreshFacetState The api's sandbox calls use the worker runtime client, and the template calls were reached only through the TemplateController wrappers the previous change deleted. On a builder node, template state arrives through node-ctl's template facet, which calls template-manager over its local socket, the node-local CLIs cover operator use, and node-ctl serves RefreshFacetState itself. BuilderApi keeps BuildTemplate, GetBuildTemplateStatus, UploadBuildBlob, TriggerStateReconciliation, DeleteTemplate, DeleteTemplateBuild, PullTemplate, NodeExportTemplate and ListNodeTemplates, and stays a separate service. builder.proto lists the retired names in a comment, since services cannot reserve them, and drops the sandbox-manager import it no longer uses along with the node/v1 import nothing in it used. Delete the node-api handlers, their restart-handoff forwarding macro, the client-crate wrappers and node-api's now-unused node-ctl refresh_facet_state helper. The policy authorization test loses its builder settings case, the template authorization test its pool cases, and the peer-node test names a builder method that remains. Docs no longer list builder pool or settings authority. A caller outside this repository that calls a removed RPC now gets UNIMPLEMENTED. --- crates/libs/common/svc/src/auth/k8s/tests.rs | 2 +- crates/libs/node/api/client/src/lib.rs | 12 -- crates/libs/node/api/server/src/builder.rs | 135 +----------------- .../server/src/clients/policy_auth_tests.rs | 41 +----- .../server/src/clients/template_auth_tests.rs | 22 +-- crates/libs/node/api/server/src/node_ctl.rs | 8 +- docs/configuration.md | 20 ++- infra/space/kind/README.md | 2 +- proto/node/api/v1/builder.proto | 29 +--- 9 files changed, 25 insertions(+), 246 deletions(-) diff --git a/crates/libs/common/svc/src/auth/k8s/tests.rs b/crates/libs/common/svc/src/auth/k8s/tests.rs index 99cda6f6b8..ea64bf5022 100644 --- a/crates/libs/common/svc/src/auth/k8s/tests.rs +++ b/crates/libs/common/svc/src/auth/k8s/tests.rs @@ -351,7 +351,7 @@ async fn peer_node_identity_is_confined_to_template_export() { .unwrap(); for (method, allowed) in [ ("/space.node.api.v1.BuilderApi/NodeExportTemplate", true), - ("/space.node.api.v1.BuilderApi/SandboxState", false), + ("/space.node.api.v1.BuilderApi/DeleteTemplate", false), ("/space.node.api.v1.BuilderApi/BuildTemplate", false), ("/space.node.api.v1.WorkerApi/NodeExportTemplate", false), ] { diff --git a/crates/libs/node/api/client/src/lib.rs b/crates/libs/node/api/client/src/lib.rs index 1b59281ef4..306753b374 100644 --- a/crates/libs/node/api/client/src/lib.rs +++ b/crates/libs/node/api/client/src/lib.rs @@ -527,22 +527,10 @@ impl<'a> BuilderApiClient<'a> { } node_api_unary_methods! { - sandbox_state, proto::builder_api_client::BuilderApiClient::new, pplx_space_sbx_mgr_client::proto::StateRequest, pplx_space_sbx_mgr_client::proto::StateResponse; - list_sandboxes, proto::builder_api_client::BuilderApiClient::new, pplx_space_sbx_mgr_client::proto::ListRequest, pplx_space_sbx_mgr_client::proto::ListResponse; - stop_sandbox, proto::builder_api_client::BuilderApiClient::new, pplx_space_sbx_mgr_client::proto::StopRequest, pplx_space_sbx_mgr_client::proto::StopResponse; - delete_sandbox, proto::builder_api_client::BuilderApiClient::new, pplx_space_sbx_mgr_client::proto::DeleteRequest, pplx_space_sbx_mgr_client::proto::DeleteResponse; delete_template, proto::builder_api_client::BuilderApiClient::new, pplx_space_template_mgr_client::proto::DeleteTemplateRequest, pplx_space_template_mgr_client::proto::DeleteTemplateResponse; delete_template_build, proto::builder_api_client::BuilderApiClient::new, pplx_space_template_mgr_client::proto::DeleteTemplateBuildRequest, pplx_space_template_mgr_client::proto::DeleteTemplateBuildResponse; - list_templates, proto::builder_api_client::BuilderApiClient::new, pplx_space_template_mgr_client::proto::ListTemplatesRequest, pplx_space_template_mgr_client::proto::ListTemplatesResponse; - get_template, proto::builder_api_client::BuilderApiClient::new, pplx_space_template_mgr_client::proto::GetTemplateRequest, pplx_space_template_mgr_client::proto::GetTemplateResponse; - update_template_settings, proto::builder_api_client::BuilderApiClient::new, pplx_space_template_mgr_client::proto::UpdateTemplateSettingsRequest, pplx_space_template_mgr_client::proto::UpdateTemplateSettingsResponse; - update_template_snapshot_policy, proto::builder_api_client::BuilderApiClient::new, pplx_space_template_mgr_client::proto::UpdateTemplateSnapshotPolicyRequest, pplx_space_template_mgr_client::proto::UpdateTemplateSnapshotPolicyResponse; - create_pool, proto::builder_api_client::BuilderApiClient::new, pplx_space_template_mgr_client::proto::CreatePoolRequest, pplx_space_template_mgr_client::proto::CreatePoolResponse; - delete_pool, proto::builder_api_client::BuilderApiClient::new, pplx_space_template_mgr_client::proto::DeletePoolRequest, pplx_space_template_mgr_client::proto::DeletePoolResponse; - pool_status, proto::builder_api_client::BuilderApiClient::new, pplx_space_template_mgr_client::proto::PoolStatusRequest, pplx_space_template_mgr_client::proto::PoolStatusResponse; pull_template, proto::builder_api_client::BuilderApiClient::new, proto::PullTemplateRequest, proto::PullTemplateResponse; list_node_templates, proto::builder_api_client::BuilderApiClient::new, proto::ListNodeTemplatesRequest, proto::ListNodeTemplatesResponse; - refresh_facet_state, proto::builder_api_client::BuilderApiClient::new, proto::RefreshFacetStateRequest, proto::RefreshFacetStateResponse; trigger_state_reconciliation, proto::builder_api_client::BuilderApiClient::new, proto::TriggerStateReconciliationRequest, proto::TriggerStateReconciliationResponse; build_template, proto::builder_api_client::BuilderApiClient::new, pplx_space_build_mgr_client::proto::StartBuildRequest, proto::BuildTemplateResponse; get_build_template_status, proto::builder_api_client::BuilderApiClient::new, pplx_space_build_mgr_client::proto::GetBuildStatusRequest, pplx_space_build_mgr_client::proto::GetBuildStatusResponse; diff --git a/crates/libs/node/api/server/src/builder.rs b/crates/libs/node/api/server/src/builder.rs index f74b3046b8..6f4c68f018 100644 --- a/crates/libs/node/api/server/src/builder.rs +++ b/crates/libs/node/api/server/src/builder.rs @@ -2,10 +2,8 @@ use crate::clients::BackendClients; use crate::node_ctl; use crate::proto::builder_api_server::BuilderApi; use crate::proto::{ - BuildTemplateResponse, RefreshFacetStateRequest, RefreshFacetStateResponse, - TriggerStateReconciliationRequest, TriggerStateReconciliationResponse, + BuildTemplateResponse, TriggerStateReconciliationRequest, TriggerStateReconciliationResponse, }; -use crate::restart_handoff::RpcReplaySafety; use pplx_space_build_mgr_client::proto as build_proto; use pplx_space_common_core::types::{NodeId, SystemTemplate}; use pplx_space_common_grpc::{RequestAdmission, fwd_retry}; @@ -68,60 +66,8 @@ macro_rules! fwd { }; } -// Sandbox-manager forwards must survive a restart handoff: the drain -// interceptor can mark any RPC, so writers replay on proof and reads replay -// broadly, exactly as on the worker surface. -macro_rules! fwd_sbx_handoff { - ($service:expr, $method:literal, $safety:expr, $rpc:ident, $req:expr) => { - crate::restart_handoff::call_with_restart_handoff( - $method, - $safety, - $req, - $service.clients.sbx(), - |mut client, request| async move { client.$rpc(request).await }, - ) - .await - }; -} - #[tonic::async_trait] impl BuilderApi for BuilderApiService { - // ── Sandbox query + cleanup (sandbox-manager) ──────────────────────────── - - async fn sandbox_state( - &self, - r: Request, - ) -> Result, Status> { - fwd_sbx_handoff!(self, "SandboxState", RpcReplaySafety::ReadOnly, state, r) - } - - async fn list_sandboxes( - &self, - r: Request, - ) -> Result, Status> { - fwd_sbx_handoff!(self, "ListSandboxes", RpcReplaySafety::ReadOnly, list, r) - } - - async fn stop_sandbox( - &self, - r: Request, - ) -> Result, Status> { - fwd_sbx_handoff!(self, "StopSandbox", RpcReplaySafety::RequiresProof, stop, r) - } - - async fn delete_sandbox( - &self, - r: Request, - ) -> Result, Status> { - fwd_sbx_handoff!( - self, - "DeleteSandbox", - RpcReplaySafety::RequiresProof, - delete, - r - ) - } - // ── Template management (template-manager) ────────────────────────────── async fn delete_template( @@ -142,87 +88,8 @@ impl BuilderApi for BuilderApiService { fwd!(self.clients.tmpl(), delete_template_build, r) } - async fn list_templates( - &self, - r: Request, - ) -> Result, Status> - { - fwd_retry!("template-manager", self.clients.tmpl(), list_templates, r) - } - - async fn get_template( - &self, - mut r: Request, - ) -> Result, Status> { - let scope = crate::template_lookup::bind_caller_scope(&mut r)?; - let response = fwd_retry!("template-manager", self.clients.tmpl(), get_template, r)?; - scope.observe_response(&response)?; - Ok(response) - } - - async fn update_template_settings( - &self, - r: Request, - ) -> Result< - Response, - Status, - > { - // A policy-bearing template update sets the default network or - // auto-lifecycle policy inherited by every sandbox of this template, so - // it must be gated on the same control-plane capability as the sandbox - // policy RPCs. - let req = r.get_ref(); - if req.network_policy.is_some() || req.default_auto_lifecycle.is_some() { - crate::worker::require_policy_mutation_authority(&r)?; - } - fwd!(self.clients.tmpl(), update_template_settings, r) - } - - async fn update_template_snapshot_policy( - &self, - r: Request, - ) -> Result< - Response, - Status, - > { - fwd!(self.clients.tmpl(), update_template_snapshot_policy, r) - } - - // ── Template pools (template-manager) ──────────────────────────────────── - - async fn create_pool( - &self, - r: Request, - ) -> Result, Status> { - pplx_space_common_svc::auth::template::require_template_mutation_authority(&r)?; - fwd!(self.clients.tmpl(), create_pool, r) - } - - async fn delete_pool( - &self, - r: Request, - ) -> Result, Status> { - pplx_space_common_svc::auth::template::require_template_mutation_authority(&r)?; - fwd!(self.clients.tmpl(), delete_pool, r) - } - - async fn pool_status( - &self, - r: Request, - ) -> Result, Status> { - fwd_retry!("template-manager", self.clients.tmpl(), pool_status, r) - } - // ── Custom orchestration ───────────────────────────────────────────────── - async fn refresh_facet_state( - &self, - r: Request, - ) -> Result, Status> { - node_ctl::refresh_facet_state(r.into_inner()).await?; - Ok(Response::new(RefreshFacetStateResponse {})) - } - async fn trigger_state_reconciliation( &self, r: Request, diff --git a/crates/libs/node/api/server/src/clients/policy_auth_tests.rs b/crates/libs/node/api/server/src/clients/policy_auth_tests.rs index a454105615..90d12756e8 100644 --- a/crates/libs/node/api/server/src/clients/policy_auth_tests.rs +++ b/crates/libs/node/api/server/src/clients/policy_auth_tests.rs @@ -7,16 +7,13 @@ use std::sync::{ }; use crate::{ - builder::BuilderApiService, proto::{ - builder_api_client::BuilderApiClient, worker_api_client::WorkerApiClient, - worker_api_internal_client::WorkerApiInternalClient, + worker_api_client::WorkerApiClient, worker_api_internal_client::WorkerApiInternalClient, worker_api_internal_server::WorkerApiInternalServer, worker_api_server::WorkerApiServer, }, worker::WorkerApiService, }; use axum::{Router, body::Body, routing::any}; -use pplx_space_common_core::types::NodeId; use pplx_space_common_svc::{ auth::{NodeApiAuthConfig, NodeApiAuthLayer, NodeRole}, bootstrap::wrap_grpc_client_channel, @@ -137,7 +134,6 @@ pub(super) fn bearer(body: T, token: &str) -> Request { enum Rpc { Create, Adopt, - BuilderTemplate, Network, Lifecycle, } @@ -153,29 +149,15 @@ async fn policy_rpc_authentication_reaches_handlers() { (true, "space-api", STATIC_TOKEN, false), (false, "space-api", STATIC_TOKEN, true), ] { - for rpc in [ - Rpc::Create, - Rpc::Adopt, - Rpc::BuilderTemplate, - Rpc::Network, - Rpc::Lifecycle, - ] { - let role = match rpc { - Rpc::BuilderTemplate => NodeRole::Builder, - _ => NodeRole::Worker, - }; + for rpc in [Rpc::Create, Rpc::Adopt, Rpc::Network, Rpc::Lifecycle] { let reviews = Arc::new(AtomicUsize::new(0)); - let auth = auth_layer(role, k8s, username, reviews.clone()); + let auth = auth_layer(NodeRole::Worker, k8s, username, reviews.clone()); let routes = tonic::service::Routes::new(WorkerApiServer::new(WorkerApiService::new( clients.clone(), ))) .add_service(WorkerApiInternalServer::new(WorkerApiService::new( clients.clone(), - ))) - .add_service( - BuilderApiService::new(clients.clone(), NodeId::new()) - .into_grpc_service(Arc::new(tokio::sync::Semaphore::new(32))), - ); + ))); let service = auth.layer(routes); for (network, lifecycle, generation) in [ (false, false, 0), @@ -218,20 +200,6 @@ async fn policy_rpc_authentication_reaches_handlers() { )) .await .map(|_| ()), - Rpc::BuilderTemplate => BuilderApiClient::new(service.clone()) - .update_template_settings(bearer( - tmpl::UpdateTemplateSettingsRequest { - network_policy, - default_auto_lifecycle: auto_lifecycle, - network_policy_generation: generation, - network_policy_authority: authority, - auto_lifecycle_authority: authority, - ..Default::default() - }, - token, - )) - .await - .map(|_| ()), Rpc::Network => WorkerApiClient::new(service.clone()) .set_sandbox_network_policy(bearer( sbx::SetNetworkPolicyRequest { @@ -270,7 +238,6 @@ async fn policy_rpc_authentication_reaches_handlers() { let requires_authority = match rpc { Rpc::Network | Rpc::Lifecycle => true, Rpc::Create | Rpc::Adopt => network || lifecycle || generation != 0, - Rpc::BuilderTemplate => network || lifecycle, }; let allowed = trusted || !requires_authority; assert_eq!( diff --git a/crates/libs/node/api/server/src/clients/template_auth_tests.rs b/crates/libs/node/api/server/src/clients/template_auth_tests.rs index 37ab14e8a5..8167bc6420 100644 --- a/crates/libs/node/api/server/src/clients/template_auth_tests.rs +++ b/crates/libs/node/api/server/src/clients/template_auth_tests.rs @@ -24,8 +24,6 @@ use crate::{ enum TemplateRpc { DeleteTemplate, DeleteTemplateBuild, - CreatePool, - DeletePool, } #[rstest] @@ -42,19 +40,8 @@ async fn template_mutation_authority_survives_the_wire( #[case] token: &'static str, #[case] mutation: bool, #[values(NodeRole::Worker, NodeRole::Builder)] role: NodeRole, - #[values( - TemplateRpc::DeleteTemplate, - TemplateRpc::DeleteTemplateBuild, - TemplateRpc::CreatePool, - TemplateRpc::DeletePool - )] - rpc: TemplateRpc, + #[values(TemplateRpc::DeleteTemplate, TemplateRpc::DeleteTemplateBuild)] rpc: TemplateRpc, ) { - if role == NodeRole::Worker && matches!(rpc, TemplateRpc::CreatePool | TemplateRpc::DeletePool) - { - // Only the builder serves pool RPCs. - return; - } let (clients, calls, shutdown, mut servers) = backend().await; let _cancel_on_drop = shutdown.clone().drop_guard(); let auth = auth_layer(role, k8s, username, Arc::new(AtomicUsize::new(0))); @@ -97,11 +84,6 @@ async fn template_mutation_authority_survives_the_wire( } }}; } - macro_rules! builder_call { - ($method:ident, $body:expr) => { - builder.$method(bearer($body, token)).await.map(|_| ()) - }; - } let result = match rpc { TemplateRpc::DeleteTemplate => { call!(delete_template, tmpl::DeleteTemplateRequest::default()) @@ -110,8 +92,6 @@ async fn template_mutation_authority_survives_the_wire( delete_template_build, tmpl::DeleteTemplateBuildRequest::default() ), - TemplateRpc::CreatePool => builder_call!(create_pool, tmpl::CreatePoolRequest::default()), - TemplateRpc::DeletePool => builder_call!(delete_pool, tmpl::DeletePoolRequest::default()), }; assert_eq!( diff --git a/crates/libs/node/api/server/src/node_ctl.rs b/crates/libs/node/api/server/src/node_ctl.rs index 37037249dd..e0648c05f9 100644 --- a/crates/libs/node/api/server/src/node_ctl.rs +++ b/crates/libs/node/api/server/src/node_ctl.rs @@ -1,6 +1,6 @@ use pplx_space_node_api_client::proto::{ FacetSyncOverride, GetReconciliationStatusRequest, NodeCtlStatus, ReconciliationStatus, - RefreshFacetStateRequest, TriggerStateReconciliationRequest, + TriggerStateReconciliationRequest, }; use tonic::{Request, Status}; @@ -42,12 +42,6 @@ pub async fn node_ctl_status_if_present() -> Option { Some(node_ctl_status().await) } -pub async fn refresh_facet_state(request: RefreshFacetStateRequest) -> Result<(), Status> { - let mut client = client().await?; - client.refresh_facet_state(Request::new(request)).await?; - Ok(()) -} - pub async fn trigger_state_reconciliation( request: TriggerStateReconciliationRequest, ) -> Result<(), Status> { diff --git a/docs/configuration.md b/docs/configuration.md index 850e6c1dac..7ca7d7fb2c 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -521,11 +521,9 @@ password sessions that have not expired become valid again. ### Template mutation authority On worker and builder node-api, `DeleteTemplate` and `DeleteTemplateBuild` -require the trusted policy service-account capability, as do builder -node-api's `CreatePool` and `DeletePool`. Template-manager's local roles and -deletion dependency guards still apply. Pool target counts have no -capacity-based upper bound yet; operators must budget pool sizes even after -caller authorization succeeds. +require the trusted policy service-account capability. Template-manager's local +roles and deletion dependency guards still apply. Pool target counts have no +capacity-based upper bound yet; operators must budget pool sizes. With Kubernetes authentication disabled, the shared internal token remains the operator trust boundary and retains mutation authority. When Kubernetes @@ -580,12 +578,12 @@ uses the internal surface, so its caller needs an internal-audience token. Shared static-token fallback cannot mutate policy when Kubernetes authentication is enabled; static-only deployments retain that authority. -Policy-bearing cold creation, internal pool adoption, and builder template -updates require a trusted caller. Cold creation and pool adoption also -require policy authority for a nonzero sandbox-policy generation, even without -an override. Requests with no overrides and generation zero remain available to -otherwise admitted callers and inherit the template's policy. Guest-bridge -accounts already have separate method restrictions. +Policy-bearing cold creation and internal pool adoption require a trusted +caller. Cold creation and pool adoption also require policy authority for a +nonzero sandbox-policy generation, even without an override. Requests with no +overrides and generation zero remain available to otherwise admitted callers +and inherit the template's policy. Guest-bridge accounts already have separate +method restrictions. `Local` policy is node-administrator-owned, including normal standalone use and operator overrides on cluster-managed nodes. It requires a root or Operator Unix diff --git a/infra/space/kind/README.md b/infra/space/kind/README.md index f2ea7aa8ce..70141f4941 100644 --- a/infra/space/kind/README.md +++ b/infra/space/kind/README.md @@ -1915,7 +1915,7 @@ and the target node-api's export request to the source: | worker WorkerApiInternal | `space-api`, `space-cluster-controller` | | worker lifecycle-probe `SpacedWaitReady`, caller-supplied policy, template deletion | `space-api`, `space-cluster-controller` | | builder BuilderApi, NodeInfo | `space-api`, `space-cluster-controller`, both node-api accounts | -| builder `BuildTemplate` (system and tenant), policy, template deletion | `space-api` | +| builder `BuildTemplate` (system and tenant), template deletion | `space-api` | | peer template transfers (`PullTemplate`, `NodeExportTemplate`), both roles | `space-api` (starts a pull), both node-api accounts (the target asks the source for the export) | | build-manager (system and tenant builds) | `space-builder-node-api` | diff --git a/proto/node/api/v1/builder.proto b/proto/node/api/v1/builder.proto index 8bd84aa210..5047b6dfbb 100644 --- a/proto/node/api/v1/builder.proto +++ b/proto/node/api/v1/builder.proto @@ -2,36 +2,22 @@ syntax = "proto3"; package space.node.api.v1; import "build/manager/v1/manager.proto"; -import "sandbox/manager/v1/manager.proto"; import "template/manager/v1/manager.proto"; -import "node/v1/node.proto"; import "node/api/v1/common.proto"; // BuilderApi is the external API for a SPACE builder node. -// Exposes template management and limited sandbox operations (query + cleanup). -// Sandbox creation/start is internal to template-manager and builder. +// Exposes template builds, deletion and transfer. Sandbox creation/start is +// internal to template-manager and builder. +// +// Retired in 0.9.0; services cannot reserve names, so do not reuse these: +// SandboxState, ListSandboxes, StopSandbox, DeleteSandbox, ListTemplates, +// GetTemplate, UpdateTemplateSettings, UpdateTemplateSnapshotPolicy, +// CreatePool, DeletePool, PoolStatus, RefreshFacetState. service BuilderApi { - // ── Sandbox query + cleanup (NO Create/Start/Kill) ───────────────────────── - - rpc SandboxState(space.sandbox.manager.v1.StateRequest) returns (space.sandbox.manager.v1.StateResponse); - rpc ListSandboxes(space.sandbox.manager.v1.ListRequest) returns (space.sandbox.manager.v1.ListResponse); - rpc StopSandbox(space.sandbox.manager.v1.StopRequest) returns (space.sandbox.manager.v1.StopResponse); - rpc DeleteSandbox(space.sandbox.manager.v1.DeleteRequest) returns (space.sandbox.manager.v1.DeleteResponse); - // ── Template management (via template-manager) ───────────────────────────── rpc DeleteTemplate(space.template.manager.v1.DeleteTemplateRequest) returns (space.template.manager.v1.DeleteTemplateResponse); rpc DeleteTemplateBuild(space.template.manager.v1.DeleteTemplateBuildRequest) returns (space.template.manager.v1.DeleteTemplateBuildResponse); - rpc ListTemplates(space.template.manager.v1.ListTemplatesRequest) returns (space.template.manager.v1.ListTemplatesResponse); - rpc GetTemplate(space.template.manager.v1.GetTemplateRequest) returns (space.template.manager.v1.GetTemplateResponse); - rpc UpdateTemplateSettings(space.template.manager.v1.UpdateTemplateSettingsRequest) returns (space.template.manager.v1.UpdateTemplateSettingsResponse); - rpc UpdateTemplateSnapshotPolicy(space.template.manager.v1.UpdateTemplateSnapshotPolicyRequest) returns (space.template.manager.v1.UpdateTemplateSnapshotPolicyResponse); - rpc CreatePool(space.template.manager.v1.CreatePoolRequest) returns (space.template.manager.v1.CreatePoolResponse); - rpc DeletePool(space.template.manager.v1.DeletePoolRequest) returns (space.template.manager.v1.DeletePoolResponse); - rpc PoolStatus(space.template.manager.v1.PoolStatusRequest) returns (space.template.manager.v1.PoolStatusResponse); - - // ── Network (via node-manager) ───────────────────────────────────────────── - // ── Template transfer ─────────────────────────────────────────────────────── @@ -46,7 +32,6 @@ service BuilderApi { // ── Node-level orchestration ─────────────────────────────────────────────── - rpc RefreshFacetState(RefreshFacetStateRequest) returns (RefreshFacetStateResponse); rpc TriggerStateReconciliation(TriggerStateReconciliationRequest) returns (TriggerStateReconciliationResponse); // ── Template build (proxy to space-buildmgr-svc on this node) ───────────── From 3451e73a4a09aa552becc15ca8012ba6969a1720 Mon Sep 17 00:00:00 2001 From: Abhinav Chordia Date: Sat, 31 Oct 2026 01:18:05 +0000 Subject: [PATCH 4/4] node-api: correct comments and docs the RPC removal left stale Removing the unused WorkerApi and BuilderApi RPCs left text that still described them. This updates comments, one error message, proto comments and docs. Behaviour does not change. - worker.rs: the require_policy_mutation_authority doc now lists the requests it gates: the two policy setters; CreateSandbox and AdoptPoolSandbox with a policy override or nonzero generation; an activation auto-lifecycle policy on StartSandbox, ResumeSandbox or the internal Wake; and the internal Wake's runtime upgrade. The function is now private, since only worker.rs calls it. The empty template-pools section header is gone. - auth: the k8s.rs comments say a worker serves the policy RPCs and both roles gate template deletion on the trusted policy allowlist. The TrustedPolicyServiceAccountsMissing message no longer calls the builder policy-serving. - worker.proto: the WorkerApi and WorkerApiInternal descriptions match what each service now serves. Rust stubs are generated at build time, so no generated file changes. - simulator: the sim_wake doc names WorkerApiInternal only, "ping" is gone from two comments, and SimState::list is test-only. - docs: the pool-target capacity note moves from the template mutation authority section of configuration.md to warm-pools.md, where targets are set. - untracked_wake_uses_the_internal_worker_wake writes its own token files, so its bearer check can tell the two credentials apart even on a host that has the default token files. --- .../lifecycle/characterization_tests.rs | 2 +- crates/libs/common/svc/src/auth/k8s.rs | 12 +++++----- crates/libs/common/svc/src/auth/mod.rs | 2 +- crates/libs/node/api/server/src/worker.rs | 22 +++++++++---------- crates/libs/node/api/sim/src/state.rs | 3 ++- crates/libs/node/api/sim/src/worker.rs | 7 +++--- docs/configuration.md | 3 +-- docs/warm-pools.md | 4 +++- proto/node/api/v1/worker.proto | 11 ++++++---- 9 files changed, 36 insertions(+), 30 deletions(-) diff --git a/crates/libs/api/src/controllers/sandbox/lifecycle/characterization_tests.rs b/crates/libs/api/src/controllers/sandbox/lifecycle/characterization_tests.rs index d6ab7fcc94..9b9dc0352b 100644 --- a/crates/libs/api/src/controllers/sandbox/lifecycle/characterization_tests.rs +++ b/crates/libs/api/src/controllers/sandbox/lifecycle/characterization_tests.rs @@ -1357,7 +1357,7 @@ async fn untracked_wake_uses_the_internal_worker_wake() -> TestResult { use pplx_space_common_svc::bearer_token::{SpaceBearerToken, space_bearer_metadata}; use pplx_space_node_api_client::proto::worker_api_internal_server::WorkerApiInternalServer; - if !crate::test_support::enter_sim_worker_process() { + if !crate::test_support::enter_sim_worker_process_with_owned_token_files() { return Ok(()); } let db = pg_test_db_with_max_connections(3).await?; diff --git a/crates/libs/common/svc/src/auth/k8s.rs b/crates/libs/common/svc/src/auth/k8s.rs index e516b983b6..87ed943c6e 100644 --- a/crates/libs/common/svc/src/auth/k8s.rs +++ b/crates/libs/common/svc/src/auth/k8s.rs @@ -611,9 +611,10 @@ impl K8sTokenAuthConfig { if role == NodeRole::Worker && self.node_api_trusted_wait_ready_usernames.is_empty() { return Err(AuthConfigError::TrustedWaitReadyServiceAccountsMissing); } - // Worker and builder both serve the policy-mutation RPCs, so both must - // ship a non-empty trusted policy allowlist. Fail closed at startup - // rather than silently admitting every allow-listed caller. + // A worker serves the policy-mutation RPCs, and both roles gate template + // deletion on this allowlist, so both must ship a non-empty one. Fail + // closed at startup rather than silently admitting every allow-listed + // caller. if matches!(role, NodeRole::Worker | NodeRole::Builder) && self.node_api_trusted_policy_usernames.is_empty() { @@ -656,8 +657,9 @@ impl K8sTokenAuthConfig { // Policy-mutation trust is an explicit, independently-configured // allowlist (validated non-empty at startup for worker/builder). It is - // `Some` only on the surfaces that serve the policy RPCs; every other - // surface reports `None`, which the authority check treats as "deny". + // `Some` only on the surfaces that serve policy or template-mutation + // RPCs; every other surface reports `None`, which the authority check + // treats as "deny". let trusted_policy_usernames = match (role, surface) { // Internal pool adoption also accepts caller-supplied policy. Its // caller must pass the internal allowlist as well as policy trust. diff --git a/crates/libs/common/svc/src/auth/mod.rs b/crates/libs/common/svc/src/auth/mod.rs index 29d760d82e..9da004aa01 100644 --- a/crates/libs/common/svc/src/auth/mod.rs +++ b/crates/libs/common/svc/src/auth/mod.rs @@ -260,7 +260,7 @@ pub enum AuthConfigError { )] TrustedWaitReadyServiceAccountsMissing, #[error( - "Kubernetes token auth is enabled for a policy-serving node (worker or builder) but no trusted policy service accounts are configured" + "Kubernetes token auth is enabled for a worker or builder node-api but no trusted policy service accounts are configured" )] TrustedPolicyServiceAccountsMissing, #[error("failed to initialize Kubernetes TokenReview client: {0}")] diff --git a/crates/libs/node/api/server/src/worker.rs b/crates/libs/node/api/server/src/worker.rs index 7f7f128048..bda0a99275 100644 --- a/crates/libs/node/api/server/src/worker.rs +++ b/crates/libs/node/api/server/src/worker.rs @@ -33,15 +33,17 @@ pub struct WorkerApiService { shutdown_token: CancellationToken, } -/// Reject policy-mutation RPCs from callers node-api did not classify as the -/// trusted control plane. `SetSandboxNetworkPolicy`, `SetSandboxAutoLifecyclePolicy`, -/// and a policy-bearing `UpdateTemplateSettings` carry an attacker-controllable -/// `authority` and `generation`; a data-plane service-account token that reached -/// the backend with one could pin a node-local or stale-generation policy that -/// cluster reconciliation can never override (a template policy also propagates to -/// every running sandbox inheriting it). Absent an authenticated caller (auth -/// layer disabled, e.g. local dev/tests) the request forwards as before. -pub(crate) fn require_policy_mutation_authority(request: &Request) -> Result<(), Status> { +/// Reject policy-bearing requests from callers node-api did not classify as the +/// trusted control plane: `SetSandboxNetworkPolicy` and +/// `SetSandboxAutoLifecyclePolicy`, a `CreateSandbox` or `AdoptPoolSandbox` with a +/// policy override or a nonzero generation, an activation auto-lifecycle policy on +/// `StartSandbox`, `ResumeSandbox` or the internal Wake, and the internal Wake's +/// runtime upgrade. Policy requests carry an attacker-controllable `authority` and +/// `generation`; a data-plane service-account token that reached the backend with +/// one could pin a node-local or stale-generation policy that cluster +/// reconciliation can never override. Absent an authenticated caller (auth layer +/// disabled, e.g. local dev/tests) the request forwards as before. +fn require_policy_mutation_authority(request: &Request) -> Result<(), Status> { check_policy_mutation_authority( request .extensions() @@ -1157,8 +1159,6 @@ impl WorkerApi for WorkerApiService { fwd!(self.clients.tmpl(), delete_template_build, r) } - // ── Template pools (template-manager) ──────────────────────────────────── - // ── Network (node-manager) ─────────────────────────────────────────────── async fn expose_port( diff --git a/crates/libs/node/api/sim/src/state.rs b/crates/libs/node/api/sim/src/state.rs index 03b4cc6dc6..65cf6d56c6 100644 --- a/crates/libs/node/api/sim/src/state.rs +++ b/crates/libs/node/api/sim/src/state.rs @@ -230,13 +230,14 @@ impl SimState { .ok_or_else(|| SimStateError::NotFound(sandbox_id.to_string())) } + #[cfg(test)] pub async fn list(&self) -> Vec { self.inner.lock().await.values().cloned().collect() } /// Get the record for `sandbox_id`, inserting a default-running record if /// the simulator has never seen this id before. Used by the read-only - /// surface (Spaced ping/status/wait_ready/files) when the simulator runs + /// surface (Spaced status/wait_ready/files) when the simulator runs /// as the per-sandbox pod under K8s substrate, where the api-server /// never calls `CreateSandbox` — the pod's mere existence on a pod IP is /// the substrate's "creation" event. diff --git a/crates/libs/node/api/sim/src/worker.rs b/crates/libs/node/api/sim/src/worker.rs index a4ce4fc0da..e151582985 100644 --- a/crates/libs/node/api/sim/src/worker.rs +++ b/crates/libs/node/api/sim/src/worker.rs @@ -986,7 +986,7 @@ impl WorkerApi for SimWorkerApiService { unimplemented() } - // ── Spaced guest operations: ping/status/wait_ready stub healthy ───────── + // ── Spaced guest operations: status/wait_ready stub healthy ────────────── async fn spaced_get_status( &self, @@ -1616,9 +1616,8 @@ async fn sim_install_activation_policy( Ok(()) } -/// Shared implementation of WakeSandbox used by both `WorkerApi` (public) and -/// `WorkerApiInternal`. Picks the appropriate WakeTransition based on the -/// current in-memory state. +/// Implementation of `WorkerApiInternal.WakeSandbox`. Picks the appropriate +/// WakeTransition based on the current in-memory state. /// `requested_identity` is the reserved identity the control plane passes in /// `WakeStartContext.route_identity` when a direct wake hydrates a new /// execution. A start or restore prefers it over the record's last identity diff --git a/docs/configuration.md b/docs/configuration.md index 7ca7d7fb2c..a8095917f0 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -522,8 +522,7 @@ password sessions that have not expired become valid again. On worker and builder node-api, `DeleteTemplate` and `DeleteTemplateBuild` require the trusted policy service-account capability. Template-manager's local -roles and deletion dependency guards still apply. Pool target counts have no -capacity-based upper bound yet; operators must budget pool sizes. +roles and deletion dependency guards still apply. With Kubernetes authentication disabled, the shared internal token remains the operator trust boundary and retains mutation authority. When Kubernetes diff --git a/docs/warm-pools.md b/docs/warm-pools.md index 6712c6d2e1..675fb11579 100644 --- a/docs/warm-pools.md +++ b/docs/warm-pools.md @@ -46,7 +46,9 @@ space-admin-cli warm-pool create \ This pins the pool to the template's current `latest` release. Use `--content-hash sha256:...` to pick a different release. `--refill-per-tick` -and `--max-warming-slots` limit how fast the pool fills. +and `--max-warming-slots` limit how fast the pool fills. SPACE does not check +`--target-ready-slots` against cluster capacity, so budget pool sizes to fit +your nodes. ## Check that it works diff --git a/proto/node/api/v1/worker.proto b/proto/node/api/v1/worker.proto index 0e95574be1..3ee738935c 100644 --- a/proto/node/api/v1/worker.proto +++ b/proto/node/api/v1/worker.proto @@ -8,8 +8,9 @@ import "template/manager/v1/manager.proto"; import "node/v1/node.proto"; import "node/api/v1/common.proto"; -// WorkerApi is the external API for a SPACE worker node. -// Exposes full sandbox lifecycle, template management, and network operations. +// WorkerApi is the external API for a SPACE worker node: sandbox lifecycle +// (Wake is on WorkerApiInternal), guest operations, sandbox snapshots, template +// deletion and transfer, and port exposure. // // Retired in 0.9.0; services cannot reserve names, so do not reuse these: // GetSandboxHealth, ListSandboxes, WakeSandbox (WorkerApiInternal keeps its @@ -113,8 +114,10 @@ service WorkerApi { rpc TriggerStateReconciliation(TriggerStateReconciliationRequest) returns (TriggerStateReconciliationResponse); } -// WorkerApiInternal is the node-local worker surface for recovery/orchestration -// RPCs that can carry privileged WakeRequest.start_context. +// WorkerApiInternal is the worker's control-plane surface, served on the same +// listener as WorkerApi with its own credential: pool adoption, and the only Wake +// node-api serves, whose request can carry a privileged start_context or +// runtime_upgrade. service WorkerApiInternal { rpc AdoptPoolSandbox(space.sandbox.manager.v1.AdoptPoolSandboxRequest) returns (space.sandbox.manager.v1.AdoptPoolSandboxResponse); rpc WakeSandbox(space.sandbox.manager.v1.WakeRequest) returns (space.sandbox.manager.v1.WakeResponse);