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 09CC41FF0E1 for ; Thu, 27 Aug 2026 13:43:18 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 61B3521583; Thu, 27 Aug 2026 13:43:12 +0200 (CEST) From: Lukas Wagner To: pdm-devel@lists.proxmox.com Subject: [PATCH datacenter-manager v3 09/21] context: introduce a ContextFactory to build application context Date: Thu, 27 Aug 2026 13:42:32 +0200 Message-ID: <20260827114244.424784-10-l.wagner@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260827114244.424784-1-l.wagner@proxmox.com> References: <20260827114244.424784-1-l.wagner@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1787830960543 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.587 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: FJGYLVLGK4D5BEF4FKK7Z4DZNDGXBXZ4 X-Message-ID-Hash: FJGYLVLGK4D5BEF4FKK7Z4DZNDGXBXZ4 X-MailFrom: l.wagner@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 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: This makes it easier to selectively override behavior for the fake remote feature, as well as integration tests. The change from Box to Arc in the stored client factory is merely to maintain bisectability, a future commit needs make_client_factory to return an Arc. Signed-off-by: Lukas Wagner --- Notes: Changes since v2: Use the context factory to build the client factory. This required an intermittent change from Box<...> to Arc<...> in the stored client factory, but this is changed anyways in the following commit. server/src/connection.rs | 4 +- server/src/context/default.rs | 5 ++ server/src/context/faked_remotes.rs | 42 ++++++++++ server/src/context/mod.rs | 78 ++++++++++--------- .../remote_collection_task.rs | 2 +- 5 files changed, 93 insertions(+), 38 deletions(-) create mode 100644 server/src/context/default.rs create mode 100644 server/src/context/faked_remotes.rs diff --git a/server/src/connection.rs b/server/src/connection.rs index a63ea7da..140a1902 100644 --- a/server/src/connection.rs +++ b/server/src/connection.rs @@ -28,7 +28,7 @@ use pve_api_types::client::PveClientImpl; use crate::pbs_client::PbsClient; use crate::remote_cache::ConnectionState; -static INSTANCE: OnceLock> = OnceLock::new(); +static INSTANCE: OnceLock> = OnceLock::new(); /// Connection Info returned from [`prepare_connect_client`] struct ConnectInfo { @@ -470,7 +470,7 @@ pub async fn make_pbs_client_and_login(remote: &Remote) -> Result) { +pub fn init(instance: Arc) { if INSTANCE.set(instance).is_err() { panic!("connection factory instance already set"); } diff --git a/server/src/context/default.rs b/server/src/context/default.rs new file mode 100644 index 00000000..e9c29e53 --- /dev/null +++ b/server/src/context/default.rs @@ -0,0 +1,5 @@ +use crate::context::ContextFactory; + +pub struct DefaultContextFactory; + +impl ContextFactory for DefaultContextFactory {} diff --git a/server/src/context/faked_remotes.rs b/server/src/context/faked_remotes.rs new file mode 100644 index 00000000..b2ae1c3f --- /dev/null +++ b/server/src/context/faked_remotes.rs @@ -0,0 +1,42 @@ +use std::sync::Arc; + +use anyhow::{Context, Error}; +use pdm_config::remotes::RemoteConfig; + +use crate::connection::ClientFactory; +use crate::context::ContextFactory; +use crate::test_support::fake_remote::{FakeClientFactory, FakeRemoteConfig}; + +pub struct FakedRemoteContextFactory(FakeRemoteConfig); + +impl FakedRemoteContextFactory { + pub fn new() -> Result { + let path = std::env::var("PDM_FAKED_REMOTE_CONFIG").context( + "compiled with remote_config = 'faked', but PDM_FAKED_REMOTE_CONFIG not set", + )?; + + log::info!("using fake remotes from {path:?}"); + let config = FakeRemoteConfig::from_json_config(&path) + .context("could not deserialize fake remote config")?; + + Ok(Self(config)) + } +} + +impl ContextFactory for FakedRemoteContextFactory { + fn make_client_factory(&self) -> Result, Error> { + Ok(Arc::new(FakeClientFactory { + config: self.0.clone(), + })) + } + + fn make_remote_config(&self) -> Result, Error> { + Ok(Box::new(self.0.clone())) + } + + // No need to override subscription_key_config_impl here. + // + // The subscription key pool is product-only (PDM stores its own pool of + // keys regardless of how remotes are mocked or not), so initialise it on + // both paths. +} diff --git a/server/src/context/mod.rs b/server/src/context/mod.rs index a4afcddd..77c84f98 100644 --- a/server/src/context/mod.rs +++ b/server/src/context/mod.rs @@ -2,48 +2,56 @@ //! //! Make sure to call `init` *once* when starting up the API server. +use std::sync::Arc; + use anyhow::Error; +use pdm_config::{remotes::RemoteConfig, subscriptions::SubscriptionKeyConfig}; -use crate::connection; +use crate::connection::{self, ClientFactory}; -/// Dependency-inject production remote-config implementation and remote client factory -#[allow(dead_code)] -fn default_remote_setup() { - pdm_config::remotes::init(Box::new(pdm_config::remotes::DefaultRemoteConfig)); - connection::init(Box::new(connection::DefaultClientFactory)); -} +#[cfg(remote_config = "faked")] +mod faked_remotes; + +#[cfg(not(remote_config = "faked"))] +mod default; /// Dependency-inject concrete implementations needed at runtime. pub fn init() -> Result<(), Error> { - // The subscription key pool is product-only (PDM stores its own pool of - // keys regardless of how remotes are mocked or not), so initialise it on - // both paths. - pdm_config::subscriptions::init(Box::new( - pdm_config::subscriptions::DefaultSubscriptionKeyConfig, - )); + let factory = context_factory()?; - #[cfg(remote_config = "faked")] - { - use anyhow::bail; - - use crate::test_support::fake_remote; - - match std::env::var("PDM_FAKED_REMOTE_CONFIG") { - Ok(path) => { - log::info!("using fake remotes from {path:?}"); - let config = fake_remote::FakeRemoteConfig::from_json_config(&path)?; - pdm_config::remotes::init(Box::new(config.clone())); - connection::init(Box::new(fake_remote::FakeClientFactory { config })); - } - Err(_) => { - bail!("compiled with remote_config = 'faked', but PDM_FAKED_REMOTE_CONFIG not set") - } - } - } - #[cfg(not(remote_config = "faked"))] - { - default_remote_setup(); - } + pdm_config::subscriptions::init(factory.make_subscription_key_config()?); + pdm_config::remotes::init(factory.make_remote_config()?); + // FIXME: Rather let connection use an Application context object from here + connection::init(factory.make_client_factory()?); Ok(()) } + +pub trait ContextFactory { + fn make_client_factory(&self) -> Result, Error> { + Ok(Arc::new(connection::DefaultClientFactory)) + } + + fn make_remote_config(&self) -> Result, Error> { + Ok(Box::new(pdm_config::remotes::DefaultRemoteConfig)) + } + + fn make_subscription_key_config( + &self, + ) -> Result, Error> { + Ok(Box::new( + pdm_config::subscriptions::DefaultSubscriptionKeyConfig, + )) + } +} + +fn context_factory() -> Result { + #[cfg(remote_config = "faked")] + { + faked_remotes::FakedRemoteContextFactory::new() + } + #[cfg(not(remote_config = "faked"))] + { + Ok(default::DefaultContextFactory) + } +} diff --git a/server/src/metric_collection/remote_collection_task.rs b/server/src/metric_collection/remote_collection_task.rs index d243dcf1..9fe67371 100644 --- a/server/src/metric_collection/remote_collection_task.rs +++ b/server/src/metric_collection/remote_collection_task.rs @@ -558,7 +558,7 @@ pub(super) mod tests { // TODO: the client factory is currently stored in a OnceLock - // we can only set it from one test... Ideally we'd like to have the // option to set it in every single test if needed - task/thread local? - connection::init(Box::new(TestClientFactory { now })); + connection::init(Arc::new(TestClientFactory { now })); }); now -- 2.47.3