From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 9D51B1FF0AA for ; Fri, 21 Aug 2026 15:59:34 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id F005E215D2; Fri, 21 Aug 2026 15:59:33 +0200 (CEST) MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Subject: Re: [PATCH proxmox 01/20] router: introduce shared state From: Robert Obkircher To: Lukas Wagner In-Reply-To: <20260817125727.454039-2-l.wagner@proxmox.com> References: <20260817125727.454039-1-l.wagner@proxmox.com> <20260817125727.454039-2-l.wagner@proxmox.com> Date: Fri, 21 Aug 2026 15:59:15 +0200 Message-Id: <178732075530.242959.4186982857535541130.b4-review@b4> X-Mailer: b4 0.16-dev X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1787320743091 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.667 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: 2NQMYDSXFWJYC5ZSHBMACNIXJRBK3OTM X-Message-ID-Hash: 2NQMYDSXFWJYC5ZSHBMACNIXJRBK3OTM X-MailFrom: r.obkircher@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: pdm-devel@lists.proxmox.com X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Datacenter Manager development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: > API handlers often need access to long-lived application data such as > configuration, caches or client handles. So far the only way to get > there is a global static, which hides the actual dependencies of a > handler and makes testing awkward. > > Add a type-keyed registry that a server fills once during startup, > together with an accessor on RpcEnvironment so that handlers can reach > it. The State newtype wraps values that come from the registry, > which allows telling them apart from regular API parameters. > > Signed-off-by: Lukas Wagner > > diff --git a/proxmox-router/src/cli/environment.rs b/proxmox-router/src/cli/environment.rs > index c85105a7..c9aefb75 100644 > --- a/proxmox-router/src/cli/environment.rs > +++ b/proxmox-router/src/cli/environment.rs > @@ -5,7 +5,7 @@ use serde_json::Value; > > use proxmox_schema::ApiType; > > -use crate::{RpcEnvironment, RpcEnvironmentType}; > +use crate::{RpcEnvironment, RpcEnvironmentType, SharedStateRegistry}; > > /// [`RpcEnvironment`] implementation for command line tools. > /// > @@ -15,6 +15,7 @@ use crate::{RpcEnvironment, RpcEnvironmentType}; > pub struct CliEnvironment { > result_attributes: Value, > auth_id: Option, > + shared_state_registry: Option, > pub(crate) global_options: HashMap>, > } > > @@ -23,6 +24,11 @@ impl CliEnvironment { > Default::default() > } > > + /// Set the shared state registry for this environment. > + pub fn set_shared_state_registry(&mut self, registry: SharedStateRegistry) { > + self.shared_state_registry = Some(registry); > + } > + > /// Borrow a global option by type. > /// > /// Returns `None` if the option type was not registered or no value was provided on the > @@ -98,4 +104,8 @@ impl RpcEnvironment for CliEnvironment { > fn get_auth_id(&self) -> Option { > self.auth_id.clone() > } > + > + fn shared_state(&self) -> Option<&SharedStateRegistry> { > + self.shared_state_registry.as_ref() > + } > } > diff --git a/proxmox-router/src/lib.rs b/proxmox-router/src/lib.rs > index da2f018f..df225d25 100644 > --- a/proxmox-router/src/lib.rs > +++ b/proxmox-router/src/lib.rs > @@ -16,6 +16,7 @@ mod permission; > mod router; > mod rpc_environment; > mod serializable_return; > +mod shared_state; > > #[doc(inline)] > #[cfg(feature = "server")] > @@ -25,6 +26,7 @@ pub use permission::*; > pub use router::*; > pub use rpc_environment::{RpcEnvironment, RpcEnvironmentType}; > pub use serializable_return::SerializableReturn; > +pub use shared_state::{SharedStateRegistry, State}; > > // make list_subdirs_api_method! work without an explicit proxmox-schema dependency: > #[doc(hidden)] > diff --git a/proxmox-router/src/rpc_environment.rs b/proxmox-router/src/rpc_environment.rs > index 8ce2d99d..e065504a 100644 > --- a/proxmox-router/src/rpc_environment.rs > +++ b/proxmox-router/src/rpc_environment.rs > @@ -2,6 +2,8 @@ use std::any::Any; > > use serde_json::Value; > > +use crate::SharedStateRegistry; > + > /// Helper to get around `RpcEnvironment: Sized` > pub trait AsAny { > fn as_any(&self) -> &(dyn Any + Send); > @@ -45,6 +47,11 @@ pub trait RpcEnvironment: Any + AsAny + Send { > fn get_client_ip(&self) -> Option { > None // dummy no-op implementation, as most environments don't need this > } > + > + /// Return a reference to the shared state registry. > + fn shared_state(&self) -> Option<&SharedStateRegistry> { > + None > + } > } > > /// Environment Type > diff --git a/proxmox-router/src/shared_state.rs b/proxmox-router/src/shared_state.rs > new file mode 100644 > index 00000000..14a539af > --- /dev/null > +++ b/proxmox-router/src/shared_state.rs > @@ -0,0 +1,46 @@ > +//! Type-keyed state that API handlers can request as a parameter. > + > +use std::any::{Any, TypeId}; > +use std::collections::HashMap; > + > +use anyhow::{Error, bail}; > + > +/// Registry of state values, keyed by their type. > +/// > +/// It holds at most one value per type. Values are registered with > +/// [`register`](SharedStateRegistry::register) before the registry is handed over to the API > +/// environment, from where handlers can access them through > +/// [`RpcEnvironment::shared_state`](crate::RpcEnvironment::shared_state). This allows passing > +/// application context to handlers without resorting to globals. > +#[derive(Default)] > +pub struct SharedStateRegistry { > + map: HashMap>, > +} We could get rid of some indirection by replacing the map with a Box<&dyn SharedState>. Probably not worth it, though. trait SharedState { fn lookup(&self, id: TypeId) -> &dyn Any; } struct PdmState { a: A, b: B } impl SharedState for PdmState { fn lookup(&self, id: TypeId) -> &dyn Any { if id == TypeId::of::() { &self.a } else if id == TypeId::of::() { &self.b } else { unreachable!() } } } > + > +impl SharedStateRegistry { > + /// Get a clone of the registered value of type `T`, if there is one. > + pub fn lookup(&self) -> Option { > + self.map > + .get(&TypeId::of::()) > + .and_then(|s| s.downcast_ref()) > + .cloned() This could .expect("value must have correct type for key"). -- Robert Obkircher