* [PATCH datacenter-manager/proxmox 00/20] inject application context via API macro for easier integration testing
@ 2026-08-17 12:57 Lukas Wagner
2026-08-17 12:57 ` [PATCH proxmox 01/20] router: introduce shared state Lukas Wagner
` (20 more replies)
0 siblings, 21 replies; 28+ messages in thread
From: Lukas Wagner @ 2026-08-17 12:57 UTC (permalink / raw)
To: pdm-devel
TL;DR: Inject essential runtime config, client factory, etc. as an application
context object and allow to retrieve this object easily in an API handler via
the API macro. This allows us directly call API handler implementations from
integration tests. The aim is to make it easier to write good tests on a larger
level.
## Rationale
Considering the different stages of automated software testing, unit testing
(test small software components in isolation) integration testing (test
multiple components to together, specifically their interactions) and
end-to-end testing (test the entire application in a context as close to
production as possible), I've found that the middle one, integration testing is
a very challenging one in our stack.
I've found that the challenges with integration testing mostly stem from the
following:
- "hardcoded" (as in, determined by some constant or literally
hard-coded) assumptions about storage paths (config, state, caches) and
users/permissions. This makes it challenging to call into the component
from a test running as normal user, e.g. from a regular `cargo test`.
- use of global/static instances (examples: Worker task context,
proxmox-product-config, client factory in PDM...) in application code and
shared crates. While this can be okay for things that are truly global (e.g.
logging), it often hinders testing and especially test isolation due to hidden
dependencies between test cases and some potential internal state of the global
instance. This is one of the common causes of flaky tests. Also, the setup of
these global instances in test cases is always a bit awkward, since the order
of test execution is not defined (and might actually run in parallel), so some
kind of synchronization between the test cases is necessary. Furthermore,
certain test cases might require a *different* setup for these global instances
(example: client factory that should produce a different kind of mocked PVE
client); but since we usually put these in OnceLocks, one has to put these
tests into a separate test binary.
- tight coupling between major subsystems (e.g. one part calling into the
other without any clear boundary or abstraction via callbacks or traits)
With regards to PDM, there are a couple of things that need considering when testing:
- Filesystem access to config, state and caches
- Interactions with remotes via their API
I feel like if we find a sensible way to abstract these for tests, we can
resonably cover a huge amount of the code in the backend.
## Implementation
The core idea is to provide a context/application object ('struct
PdmApplication') and "injecting" it into out API handlers via special `State`
parameters, automatically handled by the API macro.
This new context object would give access to:
- base paths for caches, config, state
- file permissions, user/group
- client factory
- api cache
The PdmApplication object is set up during daemon startup, stored in the RpcEnvironement,
and then cloned for each API request, assuming that the handle requested access via
an State<PdmApplication> parameter.
PdmApplication is a thin wrapper around Arc<PdmApplicationInner>, which stores the actual
configuration and trait objects.
This general pattern of injecting application state/context via a parameter
to the handler is something that is also common in other Rust-based
web framworks, e.g. [actix-web] and [axum].
## References:
[actix-web]: https://actix.rs/docs/application/#state
[axum]: https://docs.rs/axum/latest/axum/#sharing-state-with-handlers
proxmox:
Lukas Wagner (3):
router: introduce shared state
rest-server: allow to inject shared state
api-macro: support shared state extraction type
proxmox-api-macro/src/api/method.rs | 88 ++++++++++++++++++++-
proxmox-api-macro/tests/state.rs | 101 +++++++++++++++++++++++++
proxmox-rest-server/src/api_config.rs | 14 +++-
proxmox-rest-server/src/environment.rs | 6 +-
proxmox-router/src/cli/environment.rs | 12 ++-
proxmox-router/src/lib.rs | 2 +
proxmox-router/src/rpc_environment.rs | 7 ++
proxmox-router/src/shared_state.rs | 76 +++++++++++++++++++
8 files changed, 302 insertions(+), 4 deletions(-)
create mode 100644 proxmox-api-macro/tests/state.rs
create mode 100644 proxmox-router/src/shared_state.rs
proxmox-datacenter-manager:
Lukas Wagner (17):
context: promote context to a dir-style module
pdm-config: remotes: rename trait methods to read/write/lock
pdm-config: subscriptions: rename trait methods to read/write/lock
remote iterator: pass remote config reader explicitly
context: introduce a ContextFactory to build application context
context: establish PdmApplication object
context: register PdmApplication in router
parallel fetcher: pass arguments to closure in a single type
parallel fetcher: support a custom client factory
api: sdn: use PdmApplication handle for accessing remotes
tests: add helpers for building API-handler-level integration tests
tests: add example tests for SDN API routes
api-cache: add wrapper type
context: provide api-cache on the app object
api: subscriptions: use PdmApplication instead of globals
pdm-config: subscriptions: drop unused accessor functions
tests: add example tests for remote subscription management
cli/admin/src/main.rs | 9 +-
lib/pdm-config/src/remotes.rs | 26 +-
lib/pdm-config/src/subscriptions.rs | 59 +---
server/src/api/nodes/subscription.rs | 21 +-
server/src/api/nodes/tasks.rs | 2 +-
server/src/api/pbs/mod.rs | 10 +-
server/src/api/pve/firewall.rs | 34 +-
server/src/api/pve/mod.rs | 3 +-
server/src/api/remotes/mod.rs | 5 +-
server/src/api/remotes/updates.rs | 3 +-
server/src/api/resources.rs | 39 ++-
server/src/api/sdn/controllers.rs | 20 +-
server/src/api/sdn/vnets.rs | 23 +-
server/src/api/sdn/zones.rs | 20 +-
server/src/api/subscriptions/mod.rs | 226 +++++++-----
server/src/api_cache.rs | 117 +++++--
server/src/bin/proxmox-datacenter-api/main.rs | 17 +-
...proxmox-datacenter-manager-daily-update.rs | 14 +-
.../bin/proxmox-datacenter-privileged-api.rs | 22 +-
server/src/connection.rs | 54 ++-
server/src/context.rs | 49 ---
server/src/context/default.rs | 5 +
server/src/context/faked_remotes.rs | 42 +++
server/src/context/mod.rs | 167 +++++++++
server/src/context/product_config.rs | 132 +++++++
server/src/metric_collection/mod.rs | 6 +-
.../remote_collection_task.rs | 47 +--
server/src/parallel_fetcher.rs | 73 +++-
server/src/remote_tasks/refresh_task.rs | 23 +-
server/src/remote_updates.rs | 7 +-
server/src/test_support/fake_remote.rs | 8 +-
.../pve/remote-a/list_vnets.json | 8 +
.../pve/remote-a/list_zones.json | 8 +
.../pve/remote-b/list_vnets.json | 8 +
.../pve/remote-b/list_zones.json | 8 +
server/tests/common/environment.rs | 27 ++
server/tests/common/mod.rs | 102 ++++++
server/tests/common/test_application.rs | 323 ++++++++++++++++++
server/tests/test_sdn.rs | 88 +++++
server/tests/test_subscriptions.rs | 130 +++++++
40 files changed, 1582 insertions(+), 403 deletions(-)
delete mode 100644 server/src/context.rs
create mode 100644 server/src/context/default.rs
create mode 100644 server/src/context/faked_remotes.rs
create mode 100644 server/src/context/mod.rs
create mode 100644 server/src/context/product_config.rs
create mode 100644 server/tests/api_responses/pve/remote-a/list_vnets.json
create mode 100644 server/tests/api_responses/pve/remote-a/list_zones.json
create mode 100644 server/tests/api_responses/pve/remote-b/list_vnets.json
create mode 100644 server/tests/api_responses/pve/remote-b/list_zones.json
create mode 100644 server/tests/common/environment.rs
create mode 100644 server/tests/common/mod.rs
create mode 100644 server/tests/common/test_application.rs
create mode 100644 server/tests/test_sdn.rs
create mode 100644 server/tests/test_subscriptions.rs
Summary over all repositories:
48 files changed, 1884 insertions(+), 407 deletions(-)
--
Generated by murpp 0.12.1
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH proxmox 01/20] router: introduce shared state
2026-08-17 12:57 [PATCH datacenter-manager/proxmox 00/20] inject application context via API macro for easier integration testing Lukas Wagner
@ 2026-08-17 12:57 ` Lukas Wagner
2026-08-17 13:26 ` Lukas Wagner
` (2 more replies)
2026-08-17 12:57 ` [PATCH proxmox 02/20] rest-server: allow to inject " Lukas Wagner
` (19 subsequent siblings)
20 siblings, 3 replies; 28+ messages in thread
From: Lukas Wagner @ 2026-08-17 12:57 UTC (permalink / raw)
To: pdm-devel
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<T> newtype wraps values that come from the registry,
which allows telling them apart from regular API parameters.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
proxmox-router/src/cli/environment.rs | 12 ++++++-
proxmox-router/src/lib.rs | 2 ++
proxmox-router/src/rpc_environment.rs | 7 ++++
proxmox-router/src/shared_state.rs | 46 +++++++++++++++++++++++++++
4 files changed, 66 insertions(+), 1 deletion(-)
create mode 100644 proxmox-router/src/shared_state.rs
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<String>,
+ shared_state_registry: Option<SharedStateRegistry>,
pub(crate) global_options: HashMap<TypeId, Box<dyn Any + Send + Sync + 'static>>,
}
@@ -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<String> {
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<std::net::SocketAddr> {
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<TypeId, Box<dyn Any + Send + Sync>>,
+}
+
+impl SharedStateRegistry {
+ /// Get a clone of the registered value of type `T`, if there is one.
+ pub fn lookup<T: 'static + Send + Sync + Clone>(&self) -> Option<T> {
+ self.map
+ .get(&TypeId::of::<T>())
+ .and_then(|s| s.downcast_ref())
+ .cloned()
+ }
+
+ /// Register a value so that handlers can request it as a `State<T>` parameter.
+ ///
+ /// Since the type is the key, wrap values in a newtype if their type alone is not specific
+ /// enough to identify them.
+ ///
+ /// Fails if a value of type `T` was already registered.
+ pub fn register<T: 'static + Send + Sync + Clone>(&mut self, data: T) -> Result<(), Error> {
+ let type_id = TypeId::of::<T>();
+
+ if self.map.contains_key(&type_id) {
+ bail!("type already registered");
+ }
+
+ self.map.insert(type_id, Box::new(data));
+
+ Ok(())
+ }
+}
--
2.47.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH proxmox 02/20] rest-server: allow to inject shared state
2026-08-17 12:57 [PATCH datacenter-manager/proxmox 00/20] inject application context via API macro for easier integration testing Lukas Wagner
2026-08-17 12:57 ` [PATCH proxmox 01/20] router: introduce shared state Lukas Wagner
@ 2026-08-17 12:57 ` Lukas Wagner
2026-08-17 12:57 ` [PATCH proxmox 03/20] api-macro: support shared state extraction type Lukas Wagner
` (18 subsequent siblings)
20 siblings, 0 replies; 28+ messages in thread
From: Lukas Wagner @ 2026-08-17 12:57 UTC (permalink / raw)
To: pdm-devel
Let users of ApiConfig register a shared state registry while setting
up the server and pass it on to API handlers via RestEnvironment.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
proxmox-rest-server/src/api_config.rs | 14 +++++++++++++-
proxmox-rest-server/src/environment.rs | 6 +++++-
2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/proxmox-rest-server/src/api_config.rs b/proxmox-rest-server/src/api_config.rs
index 3543f8c4..53b1ca97 100644
--- a/proxmox-rest-server/src/api_config.rs
+++ b/proxmox-rest-server/src/api_config.rs
@@ -17,7 +17,7 @@ use proxmox_daemon::command_socket::CommandSocket;
use proxmox_http::Body;
use proxmox_log::{FileLogOptions, FileLogger};
use proxmox_network_types::Cidr;
-use proxmox_router::{Router, RpcEnvironmentType, UserInformation};
+use proxmox_router::{Router, RpcEnvironmentType, SharedStateRegistry, UserInformation};
use proxmox_sys::fs::{CreateOptions, create_path};
use crate::RestEnvironment;
@@ -33,6 +33,8 @@ pub struct ApiConfig {
handlers: Vec<Handler>,
auth_handler: Option<AuthHandler>,
index_handler: Option<IndexHandler>,
+ /// State that API handlers can request via a `State<T>` parameter.
+ pub(crate) shared_state: Option<SharedStateRegistry>,
pub(crate) privileged_addr: Option<PrivilegedAddr>,
// Name of the auth cookie that should be unset on 401 request. If `None` no cookie will be
// removed.
@@ -92,6 +94,7 @@ impl ApiConfig {
index_handler: None,
privileged_addr: None,
auth_cookie_name: None,
+ shared_state: None,
real_ip_header,
real_ip_allow_from: None,
@@ -366,6 +369,15 @@ impl ApiConfig {
.push(Handler::unformatted_router(prefix, router));
self
}
+
+ /// Set the [`SharedStateRegistry`] from which API handlers get their `State<T>` parameters.
+ ///
+ /// All types a handler requests must be registered before using
+ /// [`SharedStateRegistry::register`] before, otherwise a call to the API handler will fail.
+ pub fn with_shared_state(mut self, shared_state: SharedStateRegistry) -> Self {
+ self.shared_state = Some(shared_state);
+ self
+ }
}
#[cfg(feature = "templates")]
diff --git a/proxmox-rest-server/src/environment.rs b/proxmox-rest-server/src/environment.rs
index 47ff5a4f..9133999f 100644
--- a/proxmox-rest-server/src/environment.rs
+++ b/proxmox-rest-server/src/environment.rs
@@ -3,7 +3,7 @@ use std::sync::Arc;
use serde_json::{Value, json};
-use proxmox_router::{RpcEnvironment, RpcEnvironmentType};
+use proxmox_router::{RpcEnvironment, RpcEnvironmentType, SharedStateRegistry};
use crate::ApiConfig;
@@ -89,4 +89,8 @@ impl RpcEnvironment for RestEnvironment {
fn get_client_ip(&self) -> Option<SocketAddr> {
self.client_ip
}
+
+ fn shared_state(&self) -> Option<&SharedStateRegistry> {
+ self.api.shared_state.as_ref()
+ }
}
--
2.47.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH proxmox 03/20] api-macro: support shared state extraction type
2026-08-17 12:57 [PATCH datacenter-manager/proxmox 00/20] inject application context via API macro for easier integration testing Lukas Wagner
2026-08-17 12:57 ` [PATCH proxmox 01/20] router: introduce shared state Lukas Wagner
2026-08-17 12:57 ` [PATCH proxmox 02/20] rest-server: allow to inject " Lukas Wagner
@ 2026-08-17 12:57 ` Lukas Wagner
2026-08-21 13:59 ` Robert Obkircher
2026-08-17 12:57 ` [PATCH datacenter-manager 04/20] context: promote context to a dir-style module Lukas Wagner
` (17 subsequent siblings)
20 siblings, 1 reply; 28+ messages in thread
From: Lukas Wagner @ 2026-08-17 12:57 UTC (permalink / raw)
To: pdm-devel
The State<T> newtype wraps values that come from the registry, which
allows telling them apart from regular API parameters. The API macro
recognizes them in API handler signatures and looks them up in the
shared state registry of the RpcEnvironment, instead of deserializing
them from the request parameters. That way a handler declares its
dependencies on application state in its signature and does not have to
access the environment by hand.
Extraction fails at runtime if the router has no registry set up or if
the requested type was never registered.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
proxmox-api-macro/src/api/method.rs | 88 +++++++++++++++++++++++-
proxmox-api-macro/tests/state.rs | 101 ++++++++++++++++++++++++++++
proxmox-router/src/shared_state.rs | 30 +++++++++
3 files changed, 218 insertions(+), 1 deletion(-)
create mode 100644 proxmox-api-macro/tests/state.rs
diff --git a/proxmox-api-macro/src/api/method.rs b/proxmox-api-macro/src/api/method.rs
index 5ef69e72..20acd50c 100644
--- a/proxmox-api-macro/src/api/method.rs
+++ b/proxmox-api-macro/src/api/method.rs
@@ -3,7 +3,8 @@
//! This has to perform quite a few things: infer types from parameters, deal with optional types
//! and defaults, expose parameter and return value schema to the public, and finally create the
//! wrapper function converting from a json Value hash to the parameters listed in the function
-//! signature, while recognizing specially handling `RPCEnvironment` and `ApiMethod` parameters.
+//! signature, while recognizing specially handling `RPCEnvironment`, `ApiMethod`, and `State<T>`
+//! parameters.
use std::convert::{TryFrom, TryInto};
use std::mem;
@@ -349,6 +350,7 @@ enum ParameterType {
Value,
ApiMethod,
RpcEnv,
+ State(StateParameter),
Normal(NormalParameter),
}
@@ -357,6 +359,11 @@ struct NormalParameter {
entry: ObjectEntry,
}
+#[derive(Debug)]
+struct StateParameter {
+ ty: syn::Type,
+}
+
fn check_input_type(input: &syn::FnArg) -> Result<(&syn::PatType, &syn::PatIdent), syn::Error> {
// `self` types are not supported:
let pat_type = match input {
@@ -463,6 +470,8 @@ fn handle_function_signature(method_info: &mut MethodInfo) -> Result<Ident, Erro
}
api_method_param = Some(param_list.len());
ParameterType::ApiMethod
+ } else if let Some(s) = as_state_type(&pat_type.ty) {
+ ParameterType::State(s)
} else if is_rpc_env_type(&pat_type.ty) {
if rpc_env_param.is_some() {
error!(pat_type => "multiple RpcEnvironment parameters found");
@@ -502,6 +511,38 @@ fn is_api_method_type(ty: &syn::Type) -> bool {
false
}
+fn as_state_type(ty: &syn::Type) -> Option<StateParameter> {
+ if let syn::Type::Path(p) = ty {
+ if p.qself.is_some() {
+ return None;
+ }
+
+ if let Some(ps) = p.path.segments.last() {
+ if ps.ident != "State" {
+ return None;
+ }
+
+ match &ps.arguments {
+ syn::PathArguments::AngleBracketed(angle_bracketed_generic_arguments) => {
+ if angle_bracketed_generic_arguments.args.len() != 1 {
+ return None;
+ }
+
+ let generic_argument = angle_bracketed_generic_arguments.args[0].clone();
+
+ match generic_argument {
+ syn::GenericArgument::Type(ty) => return Some(StateParameter { ty }),
+ _ => return None,
+ }
+ }
+ _ => return None,
+ }
+ }
+ }
+
+ None
+}
+
fn is_rpc_env_type(ty: &syn::Type) -> bool {
if let syn::Type::Reference(r) = ty
&& let syn::Type::TraitObject(t) = &*r.elem
@@ -563,6 +604,9 @@ fn create_wrapper_function(
&mut method_info.default_consts,
)?;
}
+ ParameterType::State(param) => {
+ extract_state_parameter(param, &mut body, &mut args, name, span)?;
+ }
}
}
@@ -824,6 +868,48 @@ fn extract_normal_parameter(
Ok(())
}
+fn extract_state_parameter(
+ param: StateParameter,
+ body: &mut TokenStream,
+ args: &mut TokenStream,
+ name: FieldName,
+ span: Span,
+) -> Result<(), Error> {
+ let name_str = syn::LitStr::new(name.as_str(), span);
+ let arg_name = Ident::new(&format!("state_arg_{}", name.as_ident()), span);
+
+ let type_name = match param.ty {
+ syn::Type::Path(type_path) => {
+ let last_segment = type_path.path.segments.last().unwrap();
+ last_segment.ident.to_string()
+ }
+ _ => {
+ error!(span, "only simple types allowed");
+ "".into()
+ }
+ };
+
+ body.extend(quote_spanned! { span =>
+ #[allow(non_snake_case)]
+ let #arg_name = rpc_env_param
+ .shared_state()
+ .ok_or_else(|| ::anyhow::format_err!(
+ "router has no shared state set up",
+ ))?
+ .lookup()
+ .map(::proxmox_router::State)
+ .ok_or_else(|| ::anyhow::format_err!(
+ "shared context type '{}' for parameter '{}' not registered",
+ #type_name,
+ #name_str,
+ ))?;
+ });
+
+ args.extend(quote_spanned! { span => #arg_name, });
+
+ Ok(())
+}
+
/// Returns a tuple containing the schema code first and the `ParameterSchema` parameter for the
/// `ApiMethod` second.
fn serialize_input_schema(
diff --git a/proxmox-api-macro/tests/state.rs b/proxmox-api-macro/tests/state.rs
new file mode 100644
index 00000000..732c3e2e
--- /dev/null
+++ b/proxmox-api-macro/tests/state.rs
@@ -0,0 +1,101 @@
+use proxmox_api_macro::api;
+use proxmox_router::{SharedStateRegistry, State};
+
+use anyhow::Error;
+use serde_json::{Value, json};
+
+#[derive(Clone)]
+struct Foo {
+ val: i32,
+}
+
+#[api(
+ input: {
+ properties: {
+ value: {
+ description: "Something",
+ }
+ }
+ }
+)]
+/// Test multiple state args.
+fn multiple_args(
+ state_arg1: State<i32>,
+ value: isize,
+ state_arg2: State<Foo>,
+) -> Result<(), Error> {
+ assert_eq!(*state_arg1, 10);
+ assert_eq!(state_arg2.val, 20);
+
+ assert_eq!(value, 50);
+
+ Ok(())
+}
+
+#[api]
+/// Test not registered state arg.
+fn not_registered(_state: State<Foo>) -> Result<(), Error> {
+ panic!("should not reach this");
+}
+
+struct RpcEnv {
+ shared_state: SharedStateRegistry,
+}
+impl proxmox_router::RpcEnvironment for RpcEnv {
+ fn result_attrib_mut(&mut self) -> &mut Value {
+ panic!("result_attrib_mut called");
+ }
+
+ fn result_attrib(&self) -> &Value {
+ panic!("result_attrib called");
+ }
+
+ /// The environment type
+ fn env_type(&self) -> proxmox_router::RpcEnvironmentType {
+ panic!("env_type called");
+ }
+
+ /// Set authentication id
+ fn set_auth_id(&mut self, user: Option<String>) {
+ let _ = user;
+ panic!("set_auth_id called");
+ }
+
+ /// Get authentication id
+ fn get_auth_id(&self) -> Option<String> {
+ panic!("get_auth_id called");
+ }
+
+ fn shared_state(&self) -> Option<&SharedStateRegistry> {
+ Some(&self.shared_state)
+ }
+}
+
+#[test]
+fn test_invocations() {
+ let mut shared_state = SharedStateRegistry::default();
+
+ shared_state.register::<i32>(10).unwrap();
+ shared_state.register::<Foo>(Foo { val: 20 }).unwrap();
+
+ let mut env = RpcEnv { shared_state };
+
+ api_function_multiple_args(
+ json!({
+ "value": 50,
+ }),
+ &API_METHOD_MULTIPLE_ARGS,
+ &mut env,
+ )
+ .expect("func with multiple injected works");
+}
+
+#[test]
+fn test_not_registered() {
+ let shared_state = SharedStateRegistry::default();
+
+ let mut env = RpcEnv { shared_state };
+
+ api_function_multiple_args(json!({}), &API_METHOD_NOT_REGISTERED, &mut env)
+ .expect_err("func did not fail");
+}
diff --git a/proxmox-router/src/shared_state.rs b/proxmox-router/src/shared_state.rs
index 14a539af..9bd6de22 100644
--- a/proxmox-router/src/shared_state.rs
+++ b/proxmox-router/src/shared_state.rs
@@ -2,6 +2,7 @@
use std::any::{Any, TypeId};
use std::collections::HashMap;
+use std::ops::Deref;
use anyhow::{Error, bail};
@@ -44,3 +45,32 @@ impl SharedStateRegistry {
Ok(())
}
}
+
+/// Wrapper for a value taken from a [`SharedStateRegistry`].
+///
+/// An API handler declares a parameter of this type to get a clone of the registered value of
+/// type `T`. Such parameters are not part of the API schema, they are filled in from the
+/// environment's registry when the handler is called.
+#[derive(Clone)]
+pub struct State<T>(pub T);
+
+impl<T> Deref for State<T> {
+ type Target = T;
+
+ fn deref(&self) -> &Self::Target {
+ &self.0
+ }
+}
+
+impl<T> From<T> for State<T> {
+ fn from(value: T) -> Self {
+ State(value)
+ }
+}
+
+impl<T> State<T> {
+ /// Take out the wrapped value.
+ pub fn into_inner(self) -> T {
+ self.0
+ }
+}
--
2.47.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH datacenter-manager 04/20] context: promote context to a dir-style module
2026-08-17 12:57 [PATCH datacenter-manager/proxmox 00/20] inject application context via API macro for easier integration testing Lukas Wagner
` (2 preceding siblings ...)
2026-08-17 12:57 ` [PATCH proxmox 03/20] api-macro: support shared state extraction type Lukas Wagner
@ 2026-08-17 12:57 ` Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 05/20] pdm-config: remotes: rename trait methods to read/write/lock Lukas Wagner
` (16 subsequent siblings)
20 siblings, 0 replies; 28+ messages in thread
From: Lukas Wagner @ 2026-08-17 12:57 UTC (permalink / raw)
To: pdm-devel
No functional changes.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
server/src/{context.rs => context/mod.rs} | 0
1 file changed, 0 insertions(+), 0 deletions(-)
rename server/src/{context.rs => context/mod.rs} (100%)
diff --git a/server/src/context.rs b/server/src/context/mod.rs
similarity index 100%
rename from server/src/context.rs
rename to server/src/context/mod.rs
--
2.47.3
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH datacenter-manager 05/20] pdm-config: remotes: rename trait methods to read/write/lock
2026-08-17 12:57 [PATCH datacenter-manager/proxmox 00/20] inject application context via API macro for easier integration testing Lukas Wagner
` (3 preceding siblings ...)
2026-08-17 12:57 ` [PATCH datacenter-manager 04/20] context: promote context to a dir-style module Lukas Wagner
@ 2026-08-17 12:57 ` Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 06/20] pdm-config: subscriptions: " Lukas Wagner
` (15 subsequent siblings)
20 siblings, 0 replies; 28+ messages in thread
From: Lukas Wagner @ 2026-08-17 12:57 UTC (permalink / raw)
To: pdm-devel
This makes the use of these less awkward when being called via the trait
object itself, e.g. on a dependency injected application context object:
app.remote_config().read()
instead of
app.remote_config().config()
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
lib/pdm-config/src/remotes.rs | 24 ++++++++++++------------
server/src/test_support/fake_remote.rs | 8 ++++----
2 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/lib/pdm-config/src/remotes.rs b/lib/pdm-config/src/remotes.rs
index a1004cfe..47754e9b 100644
--- a/lib/pdm-config/src/remotes.rs
+++ b/lib/pdm-config/src/remotes.rs
@@ -35,36 +35,36 @@ fn instance() -> &'static (dyn RemoteConfig + Send + Sync) {
///
/// Will panic if the the remote config instance has not been set before.
pub fn lock_config() -> Result<ApiLockGuard, Error> {
- instance().lock_config()
+ instance().lock()
}
/// Return contents of the remotes config
///
/// Will panic if the the remote config instance has not been set before.
pub fn config() -> Result<(SectionConfigData<Remote>, ConfigDigest), Error> {
- instance().config()
+ instance().read()
}
pub fn get_secret_token(remote: &Remote) -> Result<String, Error> {
- instance().get_secret_token(remote)
+ instance().read_secret_token(remote)
}
/// Replace the currently persisted remotes config
///
/// Will panic if the the remote config instance has not been set before.
pub fn save_config(config: SectionConfigData<Remote>) -> Result<(), Error> {
- instance().save_config(config)
+ instance().write(config)
}
pub trait RemoteConfig {
/// Return contents of the remotes config
- fn config(&self) -> Result<(SectionConfigData<Remote>, ConfigDigest), Error>;
+ fn read(&self) -> Result<(SectionConfigData<Remote>, ConfigDigest), Error>;
/// Return contents of the remotes shadow config
- fn get_secret_token(&self, remote: &Remote) -> Result<String, Error>;
+ fn read_secret_token(&self, remote: &Remote) -> Result<String, Error>;
/// Lock the remotes config
- fn lock_config(&self) -> Result<ApiLockGuard, Error>;
+ fn lock(&self) -> Result<ApiLockGuard, Error>;
/// Replace the currently persisted remotes config
- fn save_config(&self, remotes: SectionConfigData<Remote>) -> Result<(), Error>;
+ fn write(&self, remotes: SectionConfigData<Remote>) -> Result<(), Error>;
}
/// Default, production implementation for reading/writing the `remotes.cfg`
@@ -72,11 +72,11 @@ pub trait RemoteConfig {
pub struct DefaultRemoteConfig;
impl RemoteConfig for DefaultRemoteConfig {
- fn lock_config(&self) -> Result<ApiLockGuard, Error> {
+ fn lock(&self) -> Result<ApiLockGuard, Error> {
open_api_lockfile(REMOTES_CFG_LOCKFILE, None, true)
}
- fn config(&self) -> Result<(SectionConfigData<Remote>, ConfigDigest), Error> {
+ fn read(&self) -> Result<(SectionConfigData<Remote>, ConfigDigest), Error> {
let content =
proxmox_sys::fs::file_read_optional_string(REMOTES_CFG_FILENAME)?.unwrap_or_default();
@@ -86,7 +86,7 @@ impl RemoteConfig for DefaultRemoteConfig {
Ok((data, digest.into()))
}
- fn save_config(&self, mut config: SectionConfigData<Remote>) -> Result<(), Error> {
+ fn write(&self, mut config: SectionConfigData<Remote>) -> Result<(), Error> {
let shadow_content = proxmox_sys::fs::file_read_optional_string(REMOTES_SHADOW_FILENAME)?
.unwrap_or_default();
@@ -135,7 +135,7 @@ impl RemoteConfig for DefaultRemoteConfig {
replace_config(REMOTES_CFG_FILENAME, raw.as_bytes())
}
- fn get_secret_token(&self, remote: &Remote) -> Result<String, Error> {
+ fn read_secret_token(&self, remote: &Remote) -> Result<String, Error> {
// not yet rewritten into shadow config
if remote.token != "-" {
return Ok(remote.token.clone());
diff --git a/server/src/test_support/fake_remote.rs b/server/src/test_support/fake_remote.rs
index e13ffa43..f387029f 100644
--- a/server/src/test_support/fake_remote.rs
+++ b/server/src/test_support/fake_remote.rs
@@ -35,7 +35,7 @@ pub struct FakeRemoteConfig {
}
impl RemoteConfig for FakeRemoteConfig {
- fn config(&self) -> Result<(SectionConfigData<Remote>, ConfigDigest), Error> {
+ fn read(&self) -> Result<(SectionConfigData<Remote>, ConfigDigest), Error> {
let mut section_config = SectionConfigData::default();
for i in 0..self.nr_of_pve_remotes {
@@ -59,15 +59,15 @@ impl RemoteConfig for FakeRemoteConfig {
Ok((section_config, digest))
}
- fn lock_config(&self) -> Result<ApiLockGuard, Error> {
+ fn lock(&self) -> Result<ApiLockGuard, Error> {
unsafe { Ok(proxmox_product_config::create_mocked_lock()) }
}
- fn save_config(&self, _remotes: SectionConfigData<Remote>) -> Result<(), Error> {
+ fn write(&self, _remotes: SectionConfigData<Remote>) -> Result<(), Error> {
Ok(())
}
- fn get_secret_token(&self, _remote: &Remote) -> Result<String, Error> {
+ fn read_secret_token(&self, _remote: &Remote) -> Result<String, Error> {
Ok(String::new())
}
}
--
2.47.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH datacenter-manager 06/20] pdm-config: subscriptions: rename trait methods to read/write/lock
2026-08-17 12:57 [PATCH datacenter-manager/proxmox 00/20] inject application context via API macro for easier integration testing Lukas Wagner
` (4 preceding siblings ...)
2026-08-17 12:57 ` [PATCH datacenter-manager 05/20] pdm-config: remotes: rename trait methods to read/write/lock Lukas Wagner
@ 2026-08-17 12:57 ` Lukas Wagner
2026-08-21 14:00 ` Robert Obkircher
2026-08-17 12:57 ` [PATCH datacenter-manager 07/20] remote iterator: pass remote config reader explicitly Lukas Wagner
` (14 subsequent siblings)
20 siblings, 1 reply; 28+ messages in thread
From: Lukas Wagner @ 2026-08-17 12:57 UTC (permalink / raw)
To: pdm-devel
This makes the use of these less awkward when being called via the trait
object itself, e.g. on a dependency injected application context object:
app.subscription_key_config().read()
instead of
app.subscription_key_config().config()
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
lib/pdm-config/src/subscriptions.rs | 30 ++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/lib/pdm-config/src/subscriptions.rs b/lib/pdm-config/src/subscriptions.rs
index a2954418..5be88a13 100644
--- a/lib/pdm-config/src/subscriptions.rs
+++ b/lib/pdm-config/src/subscriptions.rs
@@ -35,46 +35,46 @@ fn instance() -> &'static (dyn SubscriptionKeyConfig + Send + Sync) {
}
pub fn lock_config() -> Result<ApiLockGuard, Error> {
- instance().lock_config()
+ instance().lock()
}
pub fn config() -> Result<(SectionConfigData<SubscriptionKeyEntry>, ConfigDigest), Error> {
- instance().config()
+ instance().read()
}
pub fn shadow_config() -> Result<SectionConfigData<SubscriptionKeyShadow>, Error> {
- instance().shadow_config()
+ instance().read_shadow()
}
pub fn save_config(
config: &SectionConfigData<SubscriptionKeyEntry>,
) -> Result<ConfigDigest, Error> {
- instance().save_config(config)
+ instance().write(config)
}
pub fn save_shadow(shadow: &SectionConfigData<SubscriptionKeyShadow>) -> Result<(), Error> {
- instance().save_shadow(shadow)
+ instance().write_shadow(shadow)
}
pub trait SubscriptionKeyConfig {
- fn config(&self) -> Result<(SectionConfigData<SubscriptionKeyEntry>, ConfigDigest), Error>;
- fn shadow_config(&self) -> Result<SectionConfigData<SubscriptionKeyShadow>, Error>;
- fn lock_config(&self) -> Result<ApiLockGuard, Error>;
- fn save_config(
+ fn read(&self) -> Result<(SectionConfigData<SubscriptionKeyEntry>, ConfigDigest), Error>;
+ fn read_shadow(&self) -> Result<SectionConfigData<SubscriptionKeyShadow>, Error>;
+ fn lock(&self) -> Result<ApiLockGuard, Error>;
+ fn write(
&self,
config: &SectionConfigData<SubscriptionKeyEntry>,
) -> Result<ConfigDigest, Error>;
- fn save_shadow(&self, shadow: &SectionConfigData<SubscriptionKeyShadow>) -> Result<(), Error>;
+ fn write_shadow(&self, shadow: &SectionConfigData<SubscriptionKeyShadow>) -> Result<(), Error>;
}
pub struct DefaultSubscriptionKeyConfig;
impl SubscriptionKeyConfig for DefaultSubscriptionKeyConfig {
- fn lock_config(&self) -> Result<ApiLockGuard, Error> {
+ fn lock(&self) -> Result<ApiLockGuard, Error> {
open_api_lockfile(SUBSCRIPTIONS_CFG_LOCKFILE, None, true)
}
- fn config(&self) -> Result<(SectionConfigData<SubscriptionKeyEntry>, ConfigDigest), Error> {
+ fn read(&self) -> Result<(SectionConfigData<SubscriptionKeyEntry>, ConfigDigest), Error> {
let content = proxmox_sys::fs::file_read_optional_string(SUBSCRIPTIONS_CFG_FILENAME)?
.unwrap_or_default();
@@ -85,13 +85,13 @@ impl SubscriptionKeyConfig for DefaultSubscriptionKeyConfig {
Ok((data, digest.into()))
}
- fn shadow_config(&self) -> Result<SectionConfigData<SubscriptionKeyShadow>, Error> {
+ fn read_shadow(&self) -> Result<SectionConfigData<SubscriptionKeyShadow>, Error> {
let content = proxmox_sys::fs::file_read_optional_string(SUBSCRIPTIONS_SHADOW_FILENAME)?
.unwrap_or_default();
SubscriptionKeyShadow::parse_section_config(SUBSCRIPTIONS_SHADOW_FILENAME, &content)
}
- fn save_config(
+ fn write(
&self,
config: &SectionConfigData<SubscriptionKeyEntry>,
) -> Result<ConfigDigest, Error> {
@@ -101,7 +101,7 @@ impl SubscriptionKeyConfig for DefaultSubscriptionKeyConfig {
Ok(digest)
}
- fn save_shadow(&self, shadow: &SectionConfigData<SubscriptionKeyShadow>) -> Result<(), Error> {
+ fn write_shadow(&self, shadow: &SectionConfigData<SubscriptionKeyShadow>) -> Result<(), Error> {
let raw =
SubscriptionKeyShadow::write_section_config(SUBSCRIPTIONS_SHADOW_FILENAME, shadow)?;
// Signed `SubscriptionInfo` blobs are secrets - mode 0600, priv:priv, so the
--
2.47.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH datacenter-manager 07/20] remote iterator: pass remote config reader explicitly
2026-08-17 12:57 [PATCH datacenter-manager/proxmox 00/20] inject application context via API macro for easier integration testing Lukas Wagner
` (5 preceding siblings ...)
2026-08-17 12:57 ` [PATCH datacenter-manager 06/20] pdm-config: subscriptions: " Lukas Wagner
@ 2026-08-17 12:57 ` Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 08/20] context: introduce a ContextFactory to build application context Lukas Wagner
` (13 subsequent siblings)
20 siblings, 0 replies; 28+ messages in thread
From: Lukas Wagner @ 2026-08-17 12:57 UTC (permalink / raw)
To: pdm-devel
This will be useful later once we get a reference to the remote config
implementation via the application context object. Also, it gently
nudges developers to think about the previously hidden dependency.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
lib/pdm-config/src/remotes.rs | 2 +-
server/src/api/pbs/mod.rs | 10 ++++------
server/src/api/pve/firewall.rs | 3 ++-
server/src/api/pve/mod.rs | 3 ++-
server/src/api/remotes/mod.rs | 5 +++--
server/src/api/remotes/updates.rs | 3 ++-
server/src/api/sdn/controllers.rs | 2 +-
server/src/api/sdn/vnets.rs | 2 +-
server/src/api/sdn/zones.rs | 2 +-
server/src/api/subscriptions/mod.rs | 10 ++++++----
10 files changed, 23 insertions(+), 19 deletions(-)
diff --git a/lib/pdm-config/src/remotes.rs b/lib/pdm-config/src/remotes.rs
index 47754e9b..4db6be20 100644
--- a/lib/pdm-config/src/remotes.rs
+++ b/lib/pdm-config/src/remotes.rs
@@ -21,7 +21,7 @@ pub const REMOTES_CFG_LOCKFILE: &str = configdir!("/.remotes.lock");
static INSTANCE: OnceLock<Box<dyn RemoteConfig + Send + Sync>> = OnceLock::new();
-fn instance() -> &'static (dyn RemoteConfig + Send + Sync) {
+pub fn instance() -> &'static (dyn RemoteConfig + Send + Sync) {
// Not initializing the remote config instance is
// entirely in our responsibility and not something we can recover from,
// so it should be okay to panic in this case.
diff --git a/server/src/api/pbs/mod.rs b/server/src/api/pbs/mod.rs
index 1fc75c34..41ee2c08 100644
--- a/server/src/api/pbs/mod.rs
+++ b/server/src/api/pbs/mod.rs
@@ -13,11 +13,9 @@ use pdm_api_types::{
Authid, HOST_OPTIONAL_PORT_FORMAT, PRIV_RESOURCE_AUDIT, PRIV_SYS_MODIFY, RemoteUpid,
};
-use crate::{
- connection::{self, probe_tls_connection},
- pbs_client::{self, PbsClient, get_remote},
-};
-
+use crate::api::remotes::RemoteIterator;
+use crate::connection::{self, probe_tls_connection};
+use crate::pbs_client::{self, PbsClient, get_remote};
use crate::remote_tasks;
mod node;
@@ -96,7 +94,7 @@ pub async fn new_remote_upid(
)]
/// Return the list of PBS remotes
fn list_remotes() -> Result<Vec<RemoteListEntry>, Error> {
- Ok(super::remotes::RemoteIterator::new()?
+ Ok(RemoteIterator::new(pdm_config::remotes::instance())?
.remote_type(RemoteType::Pbs)
.into_names()
.map(|name| RemoteListEntry { remote: name })
diff --git a/server/src/api/pve/firewall.rs b/server/src/api/pve/firewall.rs
index 5a6a209e..b381a14e 100644
--- a/server/src/api/pve/firewall.rs
+++ b/server/src/api/pve/firewall.rs
@@ -17,6 +17,7 @@ use pdm_api_types::{NODE_SCHEMA, VMID_SCHEMA};
use pdm_api_types::{PRIV_RESOURCE_AUDIT, PRIV_RESOURCE_MODIFY, PRIV_SYS_MODIFY};
use super::{connect_to_remote_by_id, find_node_for_vm};
+use crate::api::remotes::RemoteIterator;
use crate::connection::PveClient;
use crate::parallel_fetcher::ParallelFetcher;
@@ -249,7 +250,7 @@ async fn fetch_node_firewall_status(
pub async fn pve_firewall_status(
_rpcenv: &mut dyn RpcEnvironment,
) -> Result<Vec<RemoteFirewallStatus>, Error> {
- let pve_remotes: Vec<Remote> = crate::api::remotes::RemoteIterator::new()?
+ let pve_remotes: Vec<Remote> = RemoteIterator::new(pdm_config::remotes::instance())?
.remote_type(pdm_api_types::remotes::RemoteType::Pve)
.into_remotes()
.collect();
diff --git a/server/src/api/pve/mod.rs b/server/src/api/pve/mod.rs
index 0970f2ff..4f6f7215 100644
--- a/server/src/api/pve/mod.rs
+++ b/server/src/api/pve/mod.rs
@@ -30,6 +30,7 @@ use pve_api_types::{ClusterResourceKind, ClusterResourceType};
use super::resources::{map_pve_lxc, map_pve_node, map_pve_qemu, map_pve_storage};
+use crate::api::remotes::RemoteIterator;
use crate::connection::PveClient;
use crate::connection::{self, probe_tls_connection};
use crate::remote_tasks;
@@ -142,7 +143,7 @@ pub fn connect_to_remote_by_id(id: &str) -> Result<Arc<PveClient>, Error> {
)]
/// Return the list of PVE remotes
fn list_remotes() -> Result<Vec<RemoteListEntry>, Error> {
- Ok(super::remotes::RemoteIterator::new()?
+ Ok(RemoteIterator::new(pdm_config::remotes::instance())?
.remote_type(RemoteType::Pve)
.into_names()
.map(|name| RemoteListEntry { remote: name })
diff --git a/server/src/api/remotes/mod.rs b/server/src/api/remotes/mod.rs
index 5c9fdccf..d039e6da 100644
--- a/server/src/api/remotes/mod.rs
+++ b/server/src/api/remotes/mod.rs
@@ -4,6 +4,7 @@ use std::collections::HashSet;
use std::error::Error as _;
use anyhow::{Context, Error, bail, format_err};
+use pdm_config::remotes::RemoteConfig;
use serde::{Deserialize, Serialize};
use proxmox_access_control::CachedUserInfo;
@@ -129,8 +130,8 @@ pub struct RemoteIterator {
impl RemoteIterator {
/// Load the remote config and create an unfiltered iterator.
- pub fn new() -> Result<Self, Error> {
- let (config, _) = pdm_config::remotes::config()?;
+ pub fn new(config_impl: &dyn RemoteConfig) -> Result<Self, Error> {
+ let (config, _) = config_impl.read()?;
Ok(Self {
remotes: config.into_iter().collect(),
})
diff --git a/server/src/api/remotes/updates.rs b/server/src/api/remotes/updates.rs
index d5b6d2dd..0945478b 100644
--- a/server/src/api/remotes/updates.rs
+++ b/server/src/api/remotes/updates.rs
@@ -17,6 +17,7 @@ use proxmox_router::{
use proxmox_schema::api;
use proxmox_sortable_macro::sortable;
+use crate::api::remotes::RemoteIterator;
use crate::{connection, remote_updates};
use super::get_remote;
@@ -88,7 +89,7 @@ pub fn refresh_remote_update_summaries(rpcenv: &mut dyn RpcEnvironment) -> Resul
http_bail!(FORBIDDEN, "user has no access to resources");
}
- let remotes: Vec<Remote> = super::RemoteIterator::new()?
+ let remotes: Vec<Remote> = RemoteIterator::new(pdm_config::remotes::instance())?
.all_privs(&user_info, &auth_id, PRIV_RESOURCE_MODIFY)
.into_remotes()
.collect();
diff --git a/server/src/api/sdn/controllers.rs b/server/src/api/sdn/controllers.rs
index c90e44c9..060fef72 100644
--- a/server/src/api/sdn/controllers.rs
+++ b/server/src/api/sdn/controllers.rs
@@ -75,7 +75,7 @@ pub async fn list_controllers(
http_bail!(FORBIDDEN, "user has no access to resources");
}
- let mut iter = RemoteIterator::new()?
+ let mut iter = RemoteIterator::new(pdm_config::remotes::instance())?
.remote_type(RemoteType::Pve)
.any_privs(&user_info, &auth_id, PRIV_RESOURCE_AUDIT);
if let Some(ref filter) = remotes {
diff --git a/server/src/api/sdn/vnets.rs b/server/src/api/sdn/vnets.rs
index ede7b539..30d431bc 100644
--- a/server/src/api/sdn/vnets.rs
+++ b/server/src/api/sdn/vnets.rs
@@ -76,7 +76,7 @@ async fn list_vnets(
http_bail!(FORBIDDEN, "user has no access to resources");
}
- let mut iter = RemoteIterator::new()?
+ let mut iter = RemoteIterator::new(pdm_config::remotes::instance())?
.remote_type(RemoteType::Pve)
.any_privs(&user_info, &auth_id, PRIV_RESOURCE_AUDIT);
if let Some(ref filter) = remotes {
diff --git a/server/src/api/sdn/zones.rs b/server/src/api/sdn/zones.rs
index da471ba9..d37dc46c 100644
--- a/server/src/api/sdn/zones.rs
+++ b/server/src/api/sdn/zones.rs
@@ -82,7 +82,7 @@ pub async fn list_zones(
http_bail!(FORBIDDEN, "user has no access to resources");
}
- let mut iter = RemoteIterator::new()?
+ let mut iter = RemoteIterator::new(pdm_config::remotes::instance())?
.remote_type(RemoteType::Pve)
.any_privs(&user_info, &auth_id, PRIV_RESOURCE_AUDIT);
if let Some(ref filter) = remotes {
diff --git a/server/src/api/subscriptions/mod.rs b/server/src/api/subscriptions/mod.rs
index 48b15b98..81582945 100644
--- a/server/src/api/subscriptions/mod.rs
+++ b/server/src/api/subscriptions/mod.rs
@@ -29,6 +29,7 @@ use pdm_api_types::{
Authid, NODE_SCHEMA, PRIV_RESOURCE_AUDIT, PRIV_RESOURCE_MODIFY, PRIV_SYS_AUDIT, PRIV_SYS_MODIFY,
};
+use crate::api::remotes::RemoteIterator;
use crate::api::resources::{
get_subscription_info_for_remote, invalidate_subscription_info_for_remote,
};
@@ -1431,10 +1432,11 @@ async fn collect_node_status(
.parse()?;
let user_info = CachedUserInfo::new()?;
- let visible_remotes: Vec<(String, Remote)> = crate::api::remotes::RemoteIterator::new()?
- .any_privs(&user_info, &auth_id, PRIV_RESOURCE_AUDIT)
- .into_iter()
- .collect();
+ let visible_remotes: Vec<(String, Remote)> =
+ RemoteIterator::new(pdm_config::remotes::instance())?
+ .any_privs(&user_info, &auth_id, PRIV_RESOURCE_AUDIT)
+ .into_iter()
+ .collect();
let (keys_config, _) = pdm_config::subscriptions::config()?;
--
2.47.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH datacenter-manager 08/20] context: introduce a ContextFactory to build application context
2026-08-17 12:57 [PATCH datacenter-manager/proxmox 00/20] inject application context via API macro for easier integration testing Lukas Wagner
` (6 preceding siblings ...)
2026-08-17 12:57 ` [PATCH datacenter-manager 07/20] remote iterator: pass remote config reader explicitly Lukas Wagner
@ 2026-08-17 12:57 ` Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 09/20] context: establish PdmApplication object Lukas Wagner
` (12 subsequent siblings)
20 siblings, 0 replies; 28+ messages in thread
From: Lukas Wagner @ 2026-08-17 12:57 UTC (permalink / raw)
To: pdm-devel
This makes it easier to selectively override behavior for the fake
remote feature, as well as integration tests.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
server/src/context/default.rs | 5 ++
server/src/context/faked_remotes.rs | 42 ++++++++++++++++
server/src/context/mod.rs | 78 ++++++++++++++++-------------
3 files changed, 90 insertions(+), 35 deletions(-)
create mode 100644 server/src/context/default.rs
create mode 100644 server/src/context/faked_remotes.rs
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<Self, Error> {
+ 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<Arc<dyn ClientFactory + Send + Sync>, Error> {
+ Ok(Arc::new(FakeClientFactory {
+ config: self.0.clone(),
+ }))
+ }
+
+ fn make_remote_config(&self) -> Result<Box<dyn RemoteConfig + Send + Sync>, 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..24d653c3 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(Box::new(connection::DefaultClientFactory));
Ok(())
}
+
+pub trait ContextFactory {
+ fn make_client_factory(&self) -> Result<Arc<dyn ClientFactory + Send + Sync>, Error> {
+ Ok(Arc::new(connection::DefaultClientFactory))
+ }
+
+ fn make_remote_config(&self) -> Result<Box<dyn RemoteConfig + Send + Sync>, Error> {
+ Ok(Box::new(pdm_config::remotes::DefaultRemoteConfig))
+ }
+
+ fn make_subscription_key_config(
+ &self,
+ ) -> Result<Box<dyn SubscriptionKeyConfig + Send + Sync>, Error> {
+ Ok(Box::new(
+ pdm_config::subscriptions::DefaultSubscriptionKeyConfig,
+ ))
+ }
+}
+
+fn context_factory() -> Result<impl ContextFactory, Error> {
+ #[cfg(remote_config = "faked")]
+ {
+ faked_remotes::FakedRemoteContextFactory::new()
+ }
+ #[cfg(not(remote_config = "faked"))]
+ {
+ Ok(default::DefaultContextFactory)
+ }
+}
--
2.47.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH datacenter-manager 09/20] context: establish PdmApplication object
2026-08-17 12:57 [PATCH datacenter-manager/proxmox 00/20] inject application context via API macro for easier integration testing Lukas Wagner
` (7 preceding siblings ...)
2026-08-17 12:57 ` [PATCH datacenter-manager 08/20] context: introduce a ContextFactory to build application context Lukas Wagner
@ 2026-08-17 12:57 ` Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 10/20] context: register PdmApplication in router Lukas Wagner
` (11 subsequent siblings)
20 siblings, 0 replies; 28+ messages in thread
From: Lukas Wagner @ 2026-08-17 12:57 UTC (permalink / raw)
To: pdm-devel
PdmApplication bundles the client factory, remote config, subscription
key config, and product config behind a single cloneable handle. This
replaces the growing set of independent global statics with one object
that can be assembled differently for production, the fake-remote
feature, and integration tests.
It is reachable through a new context::pdm_application() accessor,
built by the same context::init() call that already seeds the
existing remote/subscription config globals. Later commits in this
series thread it through the API handlers via the router's shared
state instead of this accessor.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
server/src/bin/proxmox-datacenter-api/main.rs | 9 +-
.../bin/proxmox-datacenter-privileged-api.rs | 7 +-
server/src/connection.rs | 54 ++++-----
server/src/context/mod.rs | 107 +++++++++++++++--
server/src/context/product_config.rs | 111 ++++++++++++++++++
server/src/metric_collection/mod.rs | 6 +-
.../remote_collection_task.rs | 47 ++++----
7 files changed, 274 insertions(+), 67 deletions(-)
create mode 100644 server/src/context/product_config.rs
diff --git a/server/src/bin/proxmox-datacenter-api/main.rs b/server/src/bin/proxmox-datacenter-api/main.rs
index 57aa6e22..8ae7192c 100644
--- a/server/src/bin/proxmox-datacenter-api/main.rs
+++ b/server/src/bin/proxmox-datacenter-api/main.rs
@@ -28,6 +28,7 @@ use proxmox_auth_api::api::assemble_csrf_prevention_token;
use server::auth;
use server::auth::csrf::csrf_secret;
+use server::context::PdmApplication;
use server::metric_collection;
use server::resource_cache;
use server::task_utils;
@@ -67,9 +68,9 @@ fn main() -> Result<(), Error> {
}
proxmox_product_config::init(pdm_config::api_user()?, pdm_config::priv_user()?);
- server::context::init()?;
+ let app = server::context::init()?;
- proxmox_async::runtime::main(run(debug))
+ proxmox_async::runtime::main(run(app, debug))
}
async fn get_index_future(env: RestEnvironment, parts: Parts) -> Response<proxmox_http::Body> {
@@ -144,7 +145,7 @@ async fn get_index_future(env: RestEnvironment, parts: Parts) -> Response<proxmo
resp
}
-async fn run(debug: bool) -> Result<(), Error> {
+async fn run(app: PdmApplication, debug: bool) -> Result<(), Error> {
auth::init(false);
proxmox_acme_api::init(configdir!("/acme"), false)?;
@@ -339,7 +340,7 @@ async fn run(debug: bool) -> Result<(), Error> {
});
start_task_scheduler();
- metric_collection::start_task()?;
+ metric_collection::start_task(app)?;
tasks::remote_node_mapping::start_task();
resource_cache::start_task();
tasks::remote_tasks::start_task()?;
diff --git a/server/src/bin/proxmox-datacenter-privileged-api.rs b/server/src/bin/proxmox-datacenter-privileged-api.rs
index 59d30513..e1de53c3 100644
--- a/server/src/bin/proxmox-datacenter-privileged-api.rs
+++ b/server/src/bin/proxmox-datacenter-privileged-api.rs
@@ -14,6 +14,7 @@ use proxmox_rest_server::{ApiConfig, RestServer};
use proxmox_router::RpcEnvironmentType;
use proxmox_sys::fs::CreateOptions;
+use server::context::PdmApplication;
use server::{api_cache, auth};
use pdm_buildcfg::configdir;
@@ -54,9 +55,9 @@ fn main() -> Result<(), Error> {
}
}
- server::context::init()?;
+ let app = server::context::init()?;
- proxmox_async::runtime::main(run())
+ proxmox_async::runtime::main(run(app))
}
fn create_directories() -> Result<(), Error> {
@@ -114,7 +115,7 @@ fn create_directories() -> Result<(), Error> {
Ok(())
}
-async fn run() -> Result<(), Error> {
+async fn run(app: PdmApplication) -> Result<(), Error> {
auth::init(true);
proxmox_acme_api::init(configdir!("/acme"), true)?;
diff --git a/server/src/connection.rs b/server/src/connection.rs
index a63ea7da..df122836 100644
--- a/server/src/connection.rs
+++ b/server/src/connection.rs
@@ -7,9 +7,9 @@ use std::collections::HashMap;
use std::future::Future;
use std::pin::{Pin, pin};
use std::sync::Arc;
+use std::sync::LazyLock;
use std::sync::Mutex as StdMutex;
use std::sync::Once;
-use std::sync::{LazyLock, OnceLock};
use std::time::{Duration, SystemTime};
use anyhow::{Error, bail, format_err};
@@ -25,11 +25,10 @@ use proxmox_time::epoch_i64;
use pdm_api_types::remotes::{NodeUrl, Remote, RemoteType, TlsProbeOutcome};
use pve_api_types::client::PveClientImpl;
+use crate::context;
use crate::pbs_client::PbsClient;
use crate::remote_cache::ConnectionState;
-static INSTANCE: OnceLock<Box<dyn ClientFactory + Send + Sync>> = OnceLock::new();
-
/// Connection Info returned from [`prepare_connect_client`]
struct ConnectInfo {
prefix: String,
@@ -404,19 +403,11 @@ impl ClientFactory for DefaultClientFactory {
}
}
-fn instance() -> &'static (dyn ClientFactory + Send + Sync) {
- // Not initializing the connection factory instance is
- // entirely in our responsibility and not something we can recover from,
- // so it should be okay to panic in this case.
- INSTANCE
- .get()
- .expect("client factory instance not set")
- .as_ref()
-}
-
/// Create a new API client for PVE remotes
pub fn make_pve_client(remote: &Remote) -> Result<Arc<PveClient>, Error> {
- instance().make_pve_client(remote)
+ context::pdm_application()
+ .client_factory()
+ .make_pve_client(remote)
}
/// Create a new API client for PVE remotes, but for a specific endpoint
@@ -424,21 +415,29 @@ pub fn make_pve_client_with_endpoint(
remote: &Remote,
target_endpoint: Option<&str>,
) -> Result<Arc<PveClient>, Error> {
- instance().make_pve_client_with_endpoint(remote, target_endpoint)
+ context::pdm_application()
+ .client_factory()
+ .make_pve_client_with_endpoint(remote, target_endpoint)
}
/// Create a new API client for PVE remotes and try to make it connect to a specific *node*.
pub fn make_pve_client_with_node(remote: &Remote, node: &str) -> Result<Arc<PveClient>, Error> {
- instance().make_pve_client_with_node(remote, node)
+ context::pdm_application()
+ .client_factory()
+ .make_pve_client_with_node(remote, node)
}
/// Create a new API client for PBS remotes
pub fn make_pbs_client(remote: &Remote) -> Result<Box<PbsClient>, Error> {
- instance().make_pbs_client(remote)
+ context::pdm_application()
+ .client_factory()
+ .make_pbs_client(remote)
}
pub fn make_raw_client(remote: &Remote) -> Result<Box<Client>, Error> {
- instance().make_raw_client(remote)
+ context::pdm_application()
+ .client_factory()
+ .make_raw_client(remote)
}
/// Create a new API client for PVE remotes.
@@ -451,7 +450,10 @@ pub fn make_raw_client(remote: &Remote) -> Result<Box<Client>, Error> {
///
/// Note: currently does not support two factor authentication.
pub async fn make_pve_client_and_login(remote: &Remote) -> Result<Arc<PveClient>, Error> {
- instance().make_pve_client_and_login(remote).await
+ context::pdm_application()
+ .client_factory()
+ .make_pve_client_and_login(remote)
+ .await
}
/// Create a new API client for PBS remotes.
@@ -464,16 +466,10 @@ pub async fn make_pve_client_and_login(remote: &Remote) -> Result<Arc<PveClient>
///
/// Note: currently does not support two factor authentication.
pub async fn make_pbs_client_and_login(remote: &Remote) -> Result<Box<PbsClient<Client>>, Error> {
- instance().make_pbs_client_and_login(remote).await
-}
-
-/// Initialize the [`ClientFactory`] instance.
-///
-/// Will panic if the instance has already been set.
-pub fn init(instance: Box<dyn ClientFactory + Send + Sync>) {
- if INSTANCE.set(instance).is_err() {
- panic!("connection factory instance already set");
- }
+ context::pdm_application()
+ .client_factory()
+ .make_pbs_client_and_login(remote)
+ .await
}
/// In order to allow the [`MultiClient`] to check the cached reachability state of a client, we
diff --git a/server/src/context/mod.rs b/server/src/context/mod.rs
index 24d653c3..bf9c6c2e 100644
--- a/server/src/context/mod.rs
+++ b/server/src/context/mod.rs
@@ -2,9 +2,10 @@
//!
//! Make sure to call `init` *once* when starting up the API server.
-use std::sync::Arc;
+use std::sync::{Arc, OnceLock};
use anyhow::Error;
+
use pdm_config::{remotes::RemoteConfig, subscriptions::SubscriptionKeyConfig};
use crate::connection::{self, ClientFactory};
@@ -15,16 +16,37 @@ mod faked_remotes;
#[cfg(not(remote_config = "faked"))]
mod default;
+pub mod product_config;
+
+use product_config::ProductConfig;
+
+static APP: OnceLock<PdmApplication> = OnceLock::new();
+
/// Dependency-inject concrete implementations needed at runtime.
-pub fn init() -> Result<(), Error> {
+pub fn init() -> Result<PdmApplication, Error> {
let factory = context_factory()?;
- 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(Box::new(connection::DefaultClientFactory));
+ let app = factory.make_pdm_application()?;
+ APP.set(app.clone())
+ .map_err(|_| anyhow::format_err!("context::init was already called"))?;
- Ok(())
+ // NOTE: This is technically a second, independent instance of the remote config.
+ // Long-term, we'd like to get rid of the global instance handle in pdm_config::remotes
+ // anyway, and the implementation is stateless, so having this second
+ // instance is not an issue *currently*.
+ pdm_config::remotes::init(factory.make_remote_config()?);
+ pdm_config::subscriptions::init(factory.make_subscription_key_config()?);
+
+ Ok(app)
+}
+
+/// Retrieve a handle to [`PdmApplication`] for this server.
+///
+/// Prefer to retrieve this via the API handler using [`proxmox_router::State`].
+pub fn pdm_application() -> PdmApplication {
+ APP.get()
+ .expect("context::init was not called to set up the application context object")
+ .clone()
}
pub trait ContextFactory {
@@ -43,6 +65,30 @@ pub trait ContextFactory {
pdm_config::subscriptions::DefaultSubscriptionKeyConfig,
))
}
+
+ fn make_product_config(&self) -> Result<ProductConfig, Error> {
+ let product_config = ProductConfig::builder()
+ .api_user(pdm_config::api_user()?)
+ .priv_user(pdm_config::priv_user()?)
+ .config_dir(pdm_buildcfg::configdir!("/"))
+ .state_dir(pdm_buildcfg::statedir!("/"))
+ .run_dir(pdm_buildcfg::rundir!("/"))
+ .cache_dir(pdm_buildcfg::PDM_CACHE_DIR)
+ .build()?;
+
+ Ok(product_config)
+ }
+
+ fn make_pdm_application(&self) -> Result<PdmApplication, Error> {
+ Ok(PdmApplication {
+ inner: Arc::new(PdmApplicationInner {
+ client_factory: self.make_client_factory()?,
+ remote_config: self.make_remote_config()?,
+ subscription_key_config: self.make_subscription_key_config()?,
+ product_config: self.make_product_config()?,
+ }),
+ })
+ }
}
fn context_factory() -> Result<impl ContextFactory, Error> {
@@ -55,3 +101,50 @@ fn context_factory() -> Result<impl ContextFactory, Error> {
Ok(default::DefaultContextFactory)
}
}
+
+/// Application context handle.
+///
+/// This type gives access to dependency-injected implementations and general product
+/// configuration.
+///
+/// This implements [`Clone`] and cheaply copied (it contains a single [`Arc<PdmApplicationInner>`]).
+#[derive(Clone)]
+pub struct PdmApplication {
+ inner: Arc<PdmApplicationInner>,
+}
+
+impl PdmApplication {
+ /// Get a handle to the [`ClientFactory`] as an [`Arc`].
+ ///
+ /// Prefer to use [`Self::client_factory`] if possible.
+ pub fn client_factory_shared(&self) -> Arc<dyn ClientFactory + Send + Sync> {
+ Arc::clone(&self.inner.client_factory)
+ }
+
+ /// Get a reference to the [`ClientFactory`].
+ pub fn client_factory(&self) -> &(dyn ClientFactory + Send + Sync) {
+ self.inner.client_factory.as_ref()
+ }
+
+ /// Get a reference to the [`RemoteConfig`].
+ pub fn remote_config(&self) -> &(dyn RemoteConfig + Send + Sync) {
+ self.inner.remote_config.as_ref()
+ }
+
+ /// Get a reference to the [`SubscriptionKeyConfig`].
+ pub fn subscription_key_config(&self) -> &(dyn SubscriptionKeyConfig + Send + Sync) {
+ self.inner.subscription_key_config.as_ref()
+ }
+
+ /// Get a reference to the [`ProductConfig`].
+ pub fn product_config(&self) -> &ProductConfig {
+ &self.inner.product_config
+ }
+}
+
+struct PdmApplicationInner {
+ client_factory: Arc<dyn ClientFactory + Send + Sync>,
+ remote_config: Box<dyn RemoteConfig + Send + Sync>,
+ subscription_key_config: Box<dyn SubscriptionKeyConfig + Send + Sync>,
+ product_config: ProductConfig,
+}
diff --git a/server/src/context/product_config.rs b/server/src/context/product_config.rs
new file mode 100644
index 00000000..3c1f44ab
--- /dev/null
+++ b/server/src/context/product_config.rs
@@ -0,0 +1,111 @@
+// NOTE: This is probably generic enough to be moved to proxmox-product-config.
+
+use std::path::{Path, PathBuf};
+
+use anyhow::Error;
+use nix::unistd::User;
+
+#[derive(Clone, Debug)]
+pub struct ProductConfig {
+ api_user: User,
+ priv_user: User,
+ config_dir: PathBuf,
+ state_dir: PathBuf,
+ run_dir: PathBuf,
+ cache_dir: PathBuf,
+}
+
+impl ProductConfig {
+ pub fn builder() -> ProductConfigBuilder {
+ ProductConfigBuilder::default()
+ }
+
+ pub fn api_user(&self) -> &User {
+ &self.api_user
+ }
+
+ pub fn priv_user(&self) -> &User {
+ &self.priv_user
+ }
+
+ pub fn config_dir(&self) -> &Path {
+ &self.config_dir
+ }
+
+ pub fn state_dir(&self) -> &Path {
+ &self.state_dir
+ }
+
+ pub fn run_dir(&self) -> &Path {
+ &self.run_dir
+ }
+
+ pub fn cache_dir(&self) -> &Path {
+ &self.cache_dir
+ }
+}
+
+#[derive(Default)]
+pub struct ProductConfigBuilder {
+ api_user: Option<User>,
+ priv_user: Option<User>,
+ config_dir: Option<PathBuf>,
+ state_dir: Option<PathBuf>,
+ run_dir: Option<PathBuf>,
+ cache_dir: Option<PathBuf>,
+}
+
+impl ProductConfigBuilder {
+ pub fn api_user(mut self, api_user: User) -> Self {
+ self.api_user = Some(api_user);
+ self
+ }
+
+ pub fn priv_user(mut self, priv_user: User) -> Self {
+ self.priv_user = Some(priv_user);
+ self
+ }
+
+ pub fn config_dir(mut self, config_dir: impl Into<PathBuf>) -> Self {
+ self.config_dir = Some(config_dir.into());
+ self
+ }
+
+ pub fn state_dir(mut self, state_dir: impl Into<PathBuf>) -> Self {
+ self.state_dir = Some(state_dir.into());
+ self
+ }
+
+ pub fn run_dir(mut self, run_dir: impl Into<PathBuf>) -> Self {
+ self.run_dir = Some(run_dir.into());
+ self
+ }
+
+ pub fn cache_dir(mut self, cache_dir: impl Into<PathBuf>) -> Self {
+ self.cache_dir = Some(cache_dir.into());
+ self
+ }
+
+ pub fn build(self) -> Result<ProductConfig, Error> {
+ Ok(ProductConfig {
+ api_user: self
+ .api_user
+ .ok_or_else(|| anyhow::format_err!("missing api_user"))?,
+ priv_user: self
+ .priv_user
+ .ok_or_else(|| anyhow::format_err!("missing priv_user"))?,
+ config_dir: self
+ .config_dir
+ .ok_or_else(|| anyhow::format_err!("missing config_dir"))?,
+ state_dir: self
+ .state_dir
+ .ok_or_else(|| anyhow::format_err!("missing state_dir"))?,
+ run_dir: self
+ .run_dir
+ .ok_or_else(|| anyhow::format_err!("missing run_dir"))?,
+ cache_dir: self
+ .cache_dir
+ .ok_or_else(|| anyhow::format_err!("missing cache_dir"))?,
+ })
+ }
+}
diff --git a/server/src/metric_collection/mod.rs b/server/src/metric_collection/mod.rs
index 3d7a477d..15b01b9a 100644
--- a/server/src/metric_collection/mod.rs
+++ b/server/src/metric_collection/mod.rs
@@ -20,6 +20,7 @@ pub mod top_entities;
use remote_collection_task::{ControlMsg, RemoteMetricCollectionTask};
use rrd_cache::RrdCache;
+use crate::context::PdmApplication;
use crate::metric_collection::local_collection_task::LocalMetricCollectionTask;
const RRD_CACHE_BASEDIR: &str = concat!(PDM_STATE_DIR_M!(), "/rrdb");
@@ -39,7 +40,7 @@ pub fn init() -> Result<(), Error> {
}
/// Start the metric collection task.
-pub fn start_task() -> Result<(), Error> {
+pub fn start_task(app: PdmApplication) -> Result<(), Error> {
let (metric_data_tx, metric_data_rx) = mpsc::channel(128);
let cache = rrd_cache::get_cache();
@@ -57,7 +58,8 @@ pub fn start_task() -> Result<(), Error> {
let metric_data_tx_clone = metric_data_tx.clone();
tokio::spawn(async move {
let metric_collection_task_future = pin!(async move {
- match RemoteMetricCollectionTask::new(metric_data_tx_clone, trigger_collection_rx) {
+ match RemoteMetricCollectionTask::new(app, metric_data_tx_clone, trigger_collection_rx)
+ {
Ok(mut task) => task.run().await,
Err(err) => log::error!("could not start metric collection task: {err}"),
}
diff --git a/server/src/metric_collection/remote_collection_task.rs b/server/src/metric_collection/remote_collection_task.rs
index d243dcf1..57910d46 100644
--- a/server/src/metric_collection/remote_collection_task.rs
+++ b/server/src/metric_collection/remote_collection_task.rs
@@ -19,8 +19,9 @@ use proxmox_sys::fs::CreateOptions;
use pdm_api_types::remotes::{Remote, RemoteType};
+use crate::context::PdmApplication;
use crate::metric_collection::rrd_task::CollectionStats;
-use crate::{connection, task_utils};
+use crate::task_utils;
use super::{
rrd_task::{RrdStoreRequest, RrdStoreResult},
@@ -48,6 +49,7 @@ pub(super) enum ControlMsg {
/// Task which periodically collects metrics from all remotes and stores
/// them in the local metrics database.
pub(super) struct RemoteMetricCollectionTask {
+ app: PdmApplication,
state: MetricCollectionState,
metric_data_tx: Sender<RrdStoreRequest>,
control_message_rx: Receiver<ControlMsg>,
@@ -56,12 +58,14 @@ pub(super) struct RemoteMetricCollectionTask {
impl RemoteMetricCollectionTask {
/// Create a new metric collection task.
pub(super) fn new(
+ app: PdmApplication,
metric_data_tx: Sender<RrdStoreRequest>,
control_message_rx: Receiver<ControlMsg>,
) -> Result<Self, Error> {
let state = load_state()?;
Ok(Self {
+ app,
state,
metric_data_tx,
control_message_rx,
@@ -234,9 +238,12 @@ impl RemoteMetricCollectionTask {
// called on the semaphore.
let permit = Arc::clone(&semaphore).acquire_owned().await.unwrap();
+ let app_clone = self.app.clone();
+
if let Some(remote) = remote_config.get(remote_name).cloned() {
log::debug!("fetching remote '{}'", remote.id);
handles.spawn(Self::fetch_single_remote(
+ app_clone,
remote,
status,
self.metric_data_tx.clone(),
@@ -294,6 +301,7 @@ impl RemoteMetricCollectionTask {
/// Fetch a single remote.
#[tracing::instrument(skip_all, fields(remote = remote.id), name = "metric_collection_task")]
async fn fetch_single_remote(
+ app: PdmApplication,
remote: Remote,
mut status: RemoteStatus,
sender: Sender<RrdStoreRequest>,
@@ -307,7 +315,7 @@ impl RemoteMetricCollectionTask {
let res: Result<RrdStoreResult, Error> = async {
match remote.ty {
RemoteType::Pve => {
- let client = connection::make_pve_client(&remote)?;
+ let client = app.client_factory().make_pve_client(&remote)?;
let metrics = client
.cluster_metrics_export(
Some(true),
@@ -331,7 +339,7 @@ impl RemoteMetricCollectionTask {
.await?;
}
RemoteType::Pbs => {
- let client = connection::make_pbs_client(&remote)?;
+ let client = app.client_factory().make_pbs_client(&remote)?;
let metrics = client
.metrics(Some(true), Some(status.most_recent_datapoint))
.await?;
@@ -386,8 +394,6 @@ pub(super) fn load_state() -> Result<MetricCollectionState, Error> {
#[cfg(test)]
pub(super) mod tests {
- use std::sync::Once;
-
use anyhow::bail;
use http::StatusCode;
@@ -397,6 +403,7 @@ pub(super) mod tests {
use crate::{
connection::{ClientFactory, PveClient},
+ context::ContextFactory,
metric_collection::rrd_task::RrdStoreResult,
pbs_client::PbsClient,
test_support::temp::NamedTempFile,
@@ -550,25 +557,18 @@ pub(super) mod tests {
number_of_requests
}
- static START: Once = Once::new();
+ const NOW: i64 = 1000;
- fn test_init() -> i64 {
- let now = 10000;
- START.call_once(|| {
- // 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 }));
- });
+ struct TestContextFactory();
- now
+ impl ContextFactory for TestContextFactory {
+ fn make_client_factory(&self) -> Result<Arc<dyn ClientFactory + Send + Sync>, Error> {
+ Ok(Arc::new(TestClientFactory { now: NOW }))
+ }
}
#[tokio::test]
async fn test_fetch_remotes_updates_state() {
- // Arrange
- let now = test_init();
-
let (tx, rx) = tokio::sync::mpsc::channel(10);
let handle = tokio::task::spawn(fake_rrd_task(rx));
@@ -579,7 +579,10 @@ pub(super) mod tests {
let (_control_tx, control_rx) = tokio::sync::mpsc::channel(10);
+ let app = TestContextFactory().make_pdm_application().unwrap();
+
let mut task = RemoteMetricCollectionTask {
+ app,
state,
metric_data_tx: tx,
control_message_rx: control_rx,
@@ -608,7 +611,7 @@ pub(super) mod tests {
);
assert_eq!(status.last_collection, None);
} else {
- assert!(now - status.most_recent_datapoint <= 10);
+ assert!(NOW - status.most_recent_datapoint <= 10);
assert!(status.error.is_none());
}
}
@@ -619,9 +622,6 @@ pub(super) mod tests {
#[tokio::test]
async fn test_fetch_overdue() {
- // Arrange
- test_init();
-
let (tx, rx) = tokio::sync::mpsc::channel(10);
let handle = tokio::task::spawn(fake_rrd_task(rx));
@@ -630,6 +630,8 @@ pub(super) mod tests {
let state_file = NamedTempFile::new(get_create_options()).unwrap();
let mut state = MetricCollectionState::new(state_file.path().into(), get_create_options());
+ let app = TestContextFactory().make_pdm_application().unwrap();
+
let now = proxmox_time::epoch_i64();
// This one should be fetched
@@ -652,6 +654,7 @@ pub(super) mod tests {
let (_control_tx, control_rx) = tokio::sync::mpsc::channel(10);
let mut task = RemoteMetricCollectionTask {
+ app,
state,
metric_data_tx: tx,
control_message_rx: control_rx,
--
2.47.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH datacenter-manager 10/20] context: register PdmApplication in router
2026-08-17 12:57 [PATCH datacenter-manager/proxmox 00/20] inject application context via API macro for easier integration testing Lukas Wagner
` (8 preceding siblings ...)
2026-08-17 12:57 ` [PATCH datacenter-manager 09/20] context: establish PdmApplication object Lukas Wagner
@ 2026-08-17 12:57 ` Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 11/20] parallel fetcher: pass arguments to closure in a single type Lukas Wagner
` (10 subsequent siblings)
20 siblings, 0 replies; 28+ messages in thread
From: Lukas Wagner @ 2026-08-17 12:57 UTC (permalink / raw)
To: pdm-devel
Register the PdmApplication handle in a SharedStateRegistry and pass
it to the REST server config via with_shared_state(), so API handlers
can request it through the State<PdmApplication> extractor instead of
reaching for context::pdm_application() or other globals directly.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
cli/admin/src/main.rs | 9 +++++++--
server/src/bin/proxmox-datacenter-api/main.rs | 8 ++++++--
server/src/bin/proxmox-datacenter-privileged-api.rs | 8 ++++++--
3 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/cli/admin/src/main.rs b/cli/admin/src/main.rs
index a2c5c623..2275eea3 100644
--- a/cli/admin/src/main.rs
+++ b/cli/admin/src/main.rs
@@ -3,14 +3,15 @@ use core::matches;
use anyhow::{Context, Error};
use serde_json::{Value, json};
-use proxmox_router::RpcEnvironment;
use proxmox_router::cli::{
CliCommand, CliCommandMap, CliEnvironment, ColumnConfig, OUTPUT_FORMAT,
default_table_format_options, format_and_print_result_full, get_output_format,
run_async_cli_command,
};
+use proxmox_router::{RpcEnvironment, SharedStateRegistry};
use proxmox_schema::api;
use proxmox_sys::fs::CreateOptions;
+use server::context::PdmApplication;
mod acme;
mod cert;
@@ -36,7 +37,7 @@ async fn run() -> Result<(), Error> {
.init()
.context("failed to set up logger")?;
- server::context::init().context("could not set up server context")?;
+ let app = server::context::init().context("could not set up server context")?;
let cmd_def = CliCommandMap::new()
.insert("acme", acme::acme_mgmt_cli())
@@ -68,7 +69,11 @@ async fn run() -> Result<(), Error> {
.context("failed to activate the socket")?;
}
+ let mut shared_state = SharedStateRegistry::default();
+ shared_state.register::<PdmApplication>(app)?;
+
let mut rpcenv = CliEnvironment::new();
+ rpcenv.set_shared_state_registry(shared_state);
rpcenv.set_auth_id(Some("root@pam".into()));
run_async_cli_command(cmd_def, rpcenv).await;
diff --git a/server/src/bin/proxmox-datacenter-api/main.rs b/server/src/bin/proxmox-datacenter-api/main.rs
index 8ae7192c..eac1585b 100644
--- a/server/src/bin/proxmox-datacenter-api/main.rs
+++ b/server/src/bin/proxmox-datacenter-api/main.rs
@@ -18,7 +18,7 @@ use url::form_urlencoded;
use proxmox_lang::try_block;
use proxmox_rest_server::{ApiConfig, RestEnvironment, RestServer};
-use proxmox_router::{RpcEnvironment, RpcEnvironmentType};
+use proxmox_router::{RpcEnvironment, RpcEnvironmentType, SharedStateRegistry};
use proxmox_sys::fs::CreateOptions;
use pdm_buildcfg::configdir;
@@ -160,6 +160,9 @@ async fn run(app: PdmApplication, debug: bool) -> Result<(), Error> {
let indexpath = Path::new(pdm_buildcfg::JS_DIR).join("index.hbs");
+ let mut shared_state = SharedStateRegistry::default();
+ shared_state.register::<PdmApplication>(app.clone())?;
+
let config = ApiConfig::new(pdm_buildcfg::JS_DIR, RpcEnvironmentType::PUBLIC)
.privileged_addr(
std::os::unix::net::SocketAddr::from_pathname(
@@ -195,7 +198,8 @@ async fn run(app: PdmApplication, debug: bool) -> Result<(), Error> {
Some(dir_opts),
Some(file_opts),
&mut command_sock,
- )?;
+ )?
+ .with_shared_state(shared_state);
let rest_server = RestServer::new(config);
let redirector = proxmox_rest_server::Redirector::new();
diff --git a/server/src/bin/proxmox-datacenter-privileged-api.rs b/server/src/bin/proxmox-datacenter-privileged-api.rs
index e1de53c3..3815ab03 100644
--- a/server/src/bin/proxmox-datacenter-privileged-api.rs
+++ b/server/src/bin/proxmox-datacenter-privileged-api.rs
@@ -11,7 +11,7 @@ use tracing::level_filters::LevelFilter;
use proxmox_lang::try_block;
use proxmox_rest_server::{ApiConfig, RestServer};
-use proxmox_router::RpcEnvironmentType;
+use proxmox_router::{RpcEnvironmentType, SharedStateRegistry};
use proxmox_sys::fs::CreateOptions;
use server::context::PdmApplication;
@@ -140,6 +140,9 @@ async fn run(app: PdmApplication) -> Result<(), Error> {
let dir_opts = CreateOptions::new().owner(api_user.uid).group(api_user.gid);
let file_opts = CreateOptions::new().owner(api_user.uid).group(api_user.gid);
+ let mut shared_state = SharedStateRegistry::default();
+ shared_state.register::<PdmApplication>(app)?;
+
let config = ApiConfig::new(pdm_buildcfg::JS_DIR, RpcEnvironmentType::PRIVILEGED)
.auth_handler_func(|h, m| Box::pin(auth::check_auth(h, m)))
.formatted_router(&["api2"], &server::api::ROUTER)
@@ -154,7 +157,8 @@ async fn run(app: PdmApplication) -> Result<(), Error> {
Some(dir_opts),
Some(file_opts),
&mut command_sock,
- )?;
+ )?
+ .with_shared_state(shared_state);
let rest_server = RestServer::new(config);
proxmox_rest_server::init_worker_tasks(pdm_buildcfg::PDM_LOG_DIR_M!().into(), file_opts)?;
--
2.47.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH datacenter-manager 11/20] parallel fetcher: pass arguments to closure in a single type
2026-08-17 12:57 [PATCH datacenter-manager/proxmox 00/20] inject application context via API macro for easier integration testing Lukas Wagner
` (9 preceding siblings ...)
2026-08-17 12:57 ` [PATCH datacenter-manager 10/20] context: register PdmApplication in router Lukas Wagner
@ 2026-08-17 12:57 ` Lukas Wagner
2026-08-21 14:00 ` Robert Obkircher
2026-08-17 12:57 ` [PATCH datacenter-manager 12/20] parallel fetcher: support a custom client factory Lukas Wagner
` (9 subsequent siblings)
20 siblings, 1 reply; 28+ messages in thread
From: Lukas Wagner @ 2026-08-17 12:57 UTC (permalink / raw)
To: pdm-devel
This makes it much easier to pass more data later, e.g. a client
factory.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
server/src/api/pve/firewall.rs | 31 +++++++++-----------
server/src/api/sdn/controllers.rs | 6 ++--
server/src/api/sdn/vnets.rs | 6 ++--
server/src/api/sdn/zones.rs | 6 ++--
server/src/parallel_fetcher.rs | 39 ++++++++++++++++++-------
server/src/remote_tasks/refresh_task.rs | 23 +++++++--------
server/src/remote_updates.rs | 7 +++--
7 files changed, 69 insertions(+), 49 deletions(-)
diff --git a/server/src/api/pve/firewall.rs b/server/src/api/pve/firewall.rs
index b381a14e..dc6c97b5 100644
--- a/server/src/api/pve/firewall.rs
+++ b/server/src/api/pve/firewall.rs
@@ -19,7 +19,7 @@ use pdm_api_types::{PRIV_RESOURCE_AUDIT, PRIV_RESOURCE_MODIFY, PRIV_SYS_MODIFY};
use super::{connect_to_remote_by_id, find_node_for_vm};
use crate::api::remotes::RemoteIterator;
use crate::connection::PveClient;
-use crate::parallel_fetcher::ParallelFetcher;
+use crate::parallel_fetcher::{ParallelFetcher, ParallelFetcherArgs};
// top-level firewall routers
pub const PVE_FW_ROUTER: Router = Router::new()
@@ -112,11 +112,9 @@ struct ClusterFirewallData {
}
async fn fetch_cluster_firewall_data(
- _context: (),
- remote: Remote,
- _node: String, // unused for cluster-level data
+ args: ParallelFetcherArgs<()>,
) -> Result<ClusterFirewallData, Error> {
- let pve = crate::connection::make_pve_client(&remote)?;
+ let pve = crate::connection::make_pve_client(&args.remote)?;
let guests = match pve.cluster_resources(Some(ClusterResourceKind::Vm)).await {
Ok(guests) => guests,
@@ -204,14 +202,12 @@ async fn load_guests_firewall_status(
}
async fn fetch_node_firewall_status(
- context: FirewallFetchContext,
- remote: Remote,
- node: String,
+ args: ParallelFetcherArgs<FirewallFetchContext>,
) -> Result<NodeFirewallStatus, Error> {
- let pve = crate::connection::make_pve_client(&remote)?;
+ let pve = crate::connection::make_pve_client(&args.remote)?;
- let options_response = pve.node_firewall_options(&node);
- let rules_response = pve.list_node_firewall_rules(&node);
+ let options_response = pve.node_firewall_options(&args.node);
+ let rules_response = pve.list_node_firewall_rules(&args.node);
let enabled = options_response
.await
@@ -227,10 +223,11 @@ async fn fetch_node_firewall_status(
_ => None,
};
- let guests_status = load_guests_firewall_status(pve, node.clone(), &context.guests).await;
+ let guests_status =
+ load_guests_firewall_status(pve, args.node.clone(), &args.context.guests).await;
Ok(NodeFirewallStatus {
- node,
+ node: args.node,
status,
guests: guests_status,
})
@@ -280,11 +277,11 @@ pub async fn pve_firewall_status(
let node_fetcher = ParallelFetcher::new(context);
let node_results = node_fetcher
- .do_for_all_remote_nodes(pve_remotes.iter().cloned(), move |mut ctx, remote, node| {
- if let Some(guests) = guests_per_remote.get(&remote.id) {
- ctx.guests = guests.clone();
+ .do_for_all_remote_nodes(pve_remotes.iter().cloned(), move |mut args| {
+ if let Some(guests) = guests_per_remote.get(&args.remote.id) {
+ args.context.guests = guests.clone();
}
- fetch_node_firewall_status(ctx, remote, node)
+ fetch_node_firewall_status(args)
})
.await;
diff --git a/server/src/api/sdn/controllers.rs b/server/src/api/sdn/controllers.rs
index 060fef72..049358cd 100644
--- a/server/src/api/sdn/controllers.rs
+++ b/server/src/api/sdn/controllers.rs
@@ -86,9 +86,9 @@ pub async fn list_controllers(
let fetcher = ParallelFetcher::new((pending, running, ty));
let results = fetcher
- .do_for_all_remotes(iter.into_remotes(), async |ctx, r, _| {
- Ok(pve::connect(&r)?
- .list_controllers(ctx.0, ctx.1, ctx.2)
+ .do_for_all_remotes(iter.into_remotes(), async |args| {
+ Ok(pve::connect(&args.remote)?
+ .list_controllers(args.context.0, args.context.1, args.context.2)
.await?)
})
.await;
diff --git a/server/src/api/sdn/vnets.rs b/server/src/api/sdn/vnets.rs
index 30d431bc..8e7ae52b 100644
--- a/server/src/api/sdn/vnets.rs
+++ b/server/src/api/sdn/vnets.rs
@@ -87,8 +87,10 @@ async fn list_vnets(
let fetcher = ParallelFetcher::new((pending, running));
let results = fetcher
- .do_for_all_remotes(iter.into_remotes(), async |ctx, r, _| {
- Ok(pve::connect(&r)?.list_vnets(ctx.0, ctx.1).await?)
+ .do_for_all_remotes(iter.into_remotes(), async |args| {
+ Ok(pve::connect(&args.remote)?
+ .list_vnets(args.context.0, args.context.1)
+ .await?)
})
.await;
diff --git a/server/src/api/sdn/zones.rs b/server/src/api/sdn/zones.rs
index d37dc46c..bb7a3822 100644
--- a/server/src/api/sdn/zones.rs
+++ b/server/src/api/sdn/zones.rs
@@ -93,8 +93,10 @@ pub async fn list_zones(
let fetcher = ParallelFetcher::new((pending, running, ty));
let results = fetcher
- .do_for_all_remotes(iter.into_remotes(), async |ctx, r, _| {
- Ok(pve::connect(&r)?.list_zones(ctx.0, ctx.1, ctx.2).await?)
+ .do_for_all_remotes(iter.into_remotes(), async |args| {
+ Ok(pve::connect(&args.remote)?
+ .list_zones(args.context.0, args.context.1, args.context.2)
+ .await?)
})
.await;
diff --git a/server/src/parallel_fetcher.rs b/server/src/parallel_fetcher.rs
index 8153f1fe..0819eb86 100644
--- a/server/src/parallel_fetcher.rs
+++ b/server/src/parallel_fetcher.rs
@@ -4,18 +4,16 @@
//! # use anyhow::Error;
//! #
//! # use pdm_api_types::remotes::{RemoteType, Remote};
-//! # use server::parallel_fetcher::ParallelFetcher;
+//! # use server::parallel_fetcher::{ParallelFetcher, ParallelFetcherArgs};
//! #
//! # #[tokio::main]
//! # async fn main() -> Result<(), Error> {
//! # let remotes: Vec<Remote> = Vec::new();
//! #
//! async fn fetch_meaning(
-//! _context: (),
-//! remote: Remote,
-//! node: String,
+//! args: ParallelFetcherArgs<()>,
//! ) -> Result<i32, Error> {
-//! match remote.ty {
+//! match args.remote.ty {
//! RemoteType::Pve => {
//! // Perform the API request here and return some result.
//! Ok(42)
@@ -269,6 +267,20 @@ impl<C> ParallelFetcherBuilder<C> {
}
}
+#[non_exhaustive]
+/// The argument type that is passed when calling the closure passed to
+/// [`ParallelFetcher::do_for_all_remote_nodes`] or [`ParallelFetcher::do_for_all_remotes`].
+pub struct ParallelFetcherArgs<C> {
+ /// The context provided in [`ParallelFetcherBuilder::new`] or
+ /// [`ParallelFetcher::new`].
+ pub context: C,
+ /// The remote.
+ pub remote: Remote,
+ /// The node. This may be 'localhost' for PBS remotes or if using
+ /// [`ParallelFetcher::do_for_all_remotes`].
+ pub node: String,
+}
+
/// Helper for parallelizing API requests to multiple remotes/nodes.
pub struct ParallelFetcher<C> {
max_connections: usize,
@@ -295,7 +307,7 @@ impl<C: Clone + Send + 'static> ParallelFetcher<C> {
) -> FetcherResponse<MultipleNodesResponse<T>>
where
A: Iterator<Item = Remote>,
- F: Fn(C, Remote, String) -> Ft + Clone + Send + 'static,
+ F: Fn(ParallelFetcherArgs<C>) -> Ft + Clone + Send + 'static,
Ft: Future<Output = Result<T, Error>> + Send + 'static,
T: Send + Debug + 'static,
{
@@ -346,7 +358,7 @@ impl<C: Clone + Send + 'static> ParallelFetcher<C> {
max_connections_per_remote: usize,
) -> RemoteResponse<MultipleNodesResponse<T>>
where
- F: Fn(C, Remote, String) -> Ft + Clone + Send + 'static,
+ F: Fn(ParallelFetcherArgs<C>) -> Ft + Clone + Send + 'static,
Ft: Future<Output = Result<T, Error>> + Send + 'static,
T: Send + Debug + 'static,
{
@@ -456,12 +468,19 @@ impl<C: Clone + Send + 'static> ParallelFetcher<C> {
_per_remote_connections_permit: Option<OwnedSemaphorePermit>,
) -> NodeResponse<T>
where
- F: Fn(C, Remote, String) -> Ft + Clone + Send + 'static,
+ F: Fn(ParallelFetcherArgs<C>) -> Ft + Clone + Send + 'static,
Ft: Future<Output = Result<T, Error>> + Send + 'static,
T: Send + Debug + 'static,
{
let now = Instant::now();
- let result = func(context, remote.clone(), node.clone()).await;
+
+ let parallel_fetcher_context = ParallelFetcherArgs {
+ context,
+ remote,
+ node: node.clone(),
+ };
+
+ let result = func(parallel_fetcher_context).await;
let api_response_time = now.elapsed();
NodeResponse {
@@ -479,7 +498,7 @@ impl<C: Clone + Send + 'static> ParallelFetcher<C> {
) -> FetcherResponse<NodeResponse<T>>
where
A: Iterator<Item = Remote>,
- F: Fn(C, Remote, String) -> Ft + Clone + Send + 'static,
+ F: Fn(ParallelFetcherArgs<C>) -> Ft + Clone + Send + 'static,
Ft: Future<Output = Result<T, Error>> + Send + 'static,
T: Send + Debug + 'static,
{
diff --git a/server/src/remote_tasks/refresh_task.rs b/server/src/remote_tasks/refresh_task.rs
index 668b63e4..c5d25284 100644
--- a/server/src/remote_tasks/refresh_task.rs
+++ b/server/src/remote_tasks/refresh_task.rs
@@ -11,7 +11,7 @@ use proxmox_section_config::typed::SectionConfigData;
use crate::api;
use crate::connection;
-use crate::parallel_fetcher::ParallelFetcher;
+use crate::parallel_fetcher::{ParallelFetcher, ParallelFetcherArgs};
use crate::pbs_client;
use crate::remote_tasks::{
KEEP_OLD_FILES, ROTATE_AFTER,
@@ -277,17 +277,16 @@ async fn fetch_remotes(
}
async fn fetch_tasks_from_single_node(
- context: Arc<State>,
- remote: Remote,
- node: String,
+ args: ParallelFetcherArgs<Arc<State>>,
) -> Result<Vec<TaskCacheItem>, Error> {
- let since = context
- .cutoff_timestamp(&remote.id, &node)
+ let since = args
+ .context
+ .cutoff_timestamp(&args.remote.id, &args.node)
.unwrap_or_else(|| {
proxmox_time::epoch_i64() - (KEEP_OLD_FILES as u64 * ROTATE_AFTER) as i64
});
- match remote.ty {
+ match args.remote.ty {
RemoteType::Pve => {
let params = pve_api_types::ListTasks {
source: Some(pve_api_types::ListTasksSource::All),
@@ -297,13 +296,13 @@ async fn fetch_tasks_from_single_node(
..Default::default()
};
- let client = connection::make_pve_client(&remote)?;
+ let client = connection::make_pve_client(&args.remote)?;
let task_list = client
- .get_task_list(&node, params)
+ .get_task_list(&args.node, params)
.await?
.into_iter()
- .map(|task| map_pve_task(task, remote.id.clone()))
+ .map(|task| map_pve_task(task, args.remote.id.clone()))
.collect();
Ok(task_list)
@@ -315,13 +314,13 @@ async fn fetch_tasks_from_single_node(
limit: Some(MAX_TASKS_TO_FETCH),
};
- let client = connection::make_pbs_client(&remote)?;
+ let client = connection::make_pbs_client(&args.remote)?;
let task_list = client
.get_task_list(params)
.await?
.into_iter()
- .map(|task| map_pbs_task(task, remote.id.clone()))
+ .map(|task| map_pbs_task(task, args.remote.id.clone()))
.collect();
Ok(task_list)
diff --git a/server/src/remote_updates.rs b/server/src/remote_updates.rs
index 855d9507..83b5a68f 100644
--- a/server/src/remote_updates.rs
+++ b/server/src/remote_updates.rs
@@ -232,14 +232,15 @@ pub async fn refresh_update_summary_cache(remotes: Vec<Remote>) -> Result<(), Er
let fetcher = ParallelFetcher::new(());
let fetch_response = fetcher
- .do_for_all_remote_nodes(remotes.into_iter(), |context, remote, node| async move {
- let result = fetch_available_updates(context, remote.clone(), node.clone()).await;
+ .do_for_all_remote_nodes(remotes.into_iter(), |args| async move {
+ let result = fetch_available_updates((), args.remote.clone(), args.node.clone()).await;
let summary = match &result {
Ok(update_info) => update_info.into(),
Err(err) => node_error_summary(err),
};
- if let Err(err) = update_cached_summary_for_node(remote, node, summary).await {
+ if let Err(err) = update_cached_summary_for_node(args.remote, args.node, summary).await
+ {
log::error!("could not update 'remote-updates' API cache entry: {err}");
}
--
2.47.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH datacenter-manager 12/20] parallel fetcher: support a custom client factory
2026-08-17 12:57 [PATCH datacenter-manager/proxmox 00/20] inject application context via API macro for easier integration testing Lukas Wagner
` (10 preceding siblings ...)
2026-08-17 12:57 ` [PATCH datacenter-manager 11/20] parallel fetcher: pass arguments to closure in a single type Lukas Wagner
@ 2026-08-17 12:57 ` Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 13/20] api: sdn: use PdmApplication handle for accessing remotes Lukas Wagner
` (8 subsequent siblings)
20 siblings, 0 replies; 28+ messages in thread
From: Lukas Wagner @ 2026-08-17 12:57 UTC (permalink / raw)
To: pdm-devel
Add a client_factory() builder method and thread the resulting handle
through fetch_remote/fetch_node, defaulting to the client factory of
the current PdmApplication when none is set explicitly. This lets
callers, and later tests, supply a fake client factory instead of
always going through the real one.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
server/src/parallel_fetcher.rs | 34 ++++++++++++++++++++++++++++++++--
1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/server/src/parallel_fetcher.rs b/server/src/parallel_fetcher.rs
index 0819eb86..74ba91b3 100644
--- a/server/src/parallel_fetcher.rs
+++ b/server/src/parallel_fetcher.rs
@@ -73,7 +73,8 @@ use pve_api_types::ClusterNodeIndexResponse;
use pdm_api_types::remotes::{Remote, RemoteType};
-use crate::connection;
+use crate::connection::ClientFactory;
+use crate::context;
/// Maximum number of parallel outgoing API requests.
pub const DEFAULT_MAX_CONNECTIONS: usize = 20;
@@ -229,6 +230,7 @@ impl<T> NodeResponse<T> {
pub struct ParallelFetcherBuilder<C> {
max_connections: Option<usize>,
max_connections_per_remote: Option<usize>,
+ client_factory: Option<Arc<dyn ClientFactory + Send + Sync>>,
context: C,
}
@@ -238,6 +240,7 @@ impl<C> ParallelFetcherBuilder<C> {
context,
max_connections: None,
max_connections_per_remote: None,
+ client_factory: None,
}
}
@@ -255,6 +258,12 @@ impl<C> ParallelFetcherBuilder<C> {
self
}
+ /// Set the client factory that should be used.
+ pub fn client_factory(mut self, client_factory: Arc<dyn ClientFactory + Send + Sync>) -> Self {
+ self.client_factory = Some(client_factory);
+ self
+ }
+
/// Build the [`ParallelFetcher`] instance.
pub fn build(self) -> ParallelFetcher<C> {
ParallelFetcher {
@@ -262,6 +271,9 @@ impl<C> ParallelFetcherBuilder<C> {
max_connections_per_remote: self
.max_connections_per_remote
.unwrap_or(DEFAULT_MAX_CONNECTIONS_PER_REMOTE),
+ client_factory: self
+ .client_factory
+ .unwrap_or_else(|| context::pdm_application().client_factory_shared()),
context: self.context,
}
}
@@ -279,12 +291,15 @@ pub struct ParallelFetcherArgs<C> {
/// The node. This may be 'localhost' for PBS remotes or if using
/// [`ParallelFetcher::do_for_all_remotes`].
pub node: String,
+ /// A handle to the client factory.
+ pub client_factory: Arc<dyn ClientFactory + Send + Sync>,
}
/// Helper for parallelizing API requests to multiple remotes/nodes.
pub struct ParallelFetcher<C> {
max_connections: usize,
max_connections_per_remote: usize,
+ client_factory: Arc<dyn ClientFactory + Send + Sync>,
context: C,
}
@@ -318,13 +333,16 @@ impl<C: Clone + Send + 'static> ParallelFetcher<C> {
for remote in remotes {
let semaphore = Arc::clone(&total_connections_semaphore);
+ let client_factory = Arc::clone(&self.client_factory);
let f = func.clone();
+
let future = Self::fetch_remote(
remote,
self.context.clone(),
semaphore,
f,
self.max_connections_per_remote,
+ client_factory,
);
if let Some(log_context) = LogContext::current() {
@@ -356,6 +374,7 @@ impl<C: Clone + Send + 'static> ParallelFetcher<C> {
semaphore: Arc<Semaphore>,
func: F,
max_connections_per_remote: usize,
+ client_factory: Arc<dyn ClientFactory + Send + Sync>,
) -> RemoteResponse<MultipleNodesResponse<T>>
where
F: Fn(ParallelFetcherArgs<C>) -> Ft + Clone + Send + 'static,
@@ -371,8 +390,10 @@ impl<C: Clone + Send + 'static> ParallelFetcher<C> {
RemoteType::Pve => {
let remote_clone = remote.clone();
+ let cf = Arc::clone(&client_factory);
+
let nodes = match async move {
- let client = connection::make_pve_client(&remote_clone)?;
+ let client = cf.make_pve_client(&remote_clone)?;
let nodes = client.list_nodes().await?;
Ok::<Vec<ClusterNodeIndexResponse>, Error>(nodes)
@@ -407,12 +428,14 @@ impl<C: Clone + Send + 'static> ParallelFetcher<C> {
let remote_clone = remote.clone();
let node_name = node.node.clone();
let context_clone = context.clone();
+ let client_factory = Arc::clone(&client_factory);
let future = Self::fetch_node(
func_clone,
context_clone,
remote_clone,
node_name,
+ client_factory,
permit,
Some(per_remote_connections_permit),
);
@@ -441,6 +464,7 @@ impl<C: Clone + Send + 'static> ParallelFetcher<C> {
context,
remote.clone(),
"localhost".into(),
+ client_factory,
permit.unwrap(), // Always set to `Some` at this point
None,
)
@@ -464,6 +488,7 @@ impl<C: Clone + Send + 'static> ParallelFetcher<C> {
context: C,
remote: Remote,
node: String,
+ client_factory: Arc<dyn ClientFactory + Send + Sync>,
_permit: OwnedSemaphorePermit,
_per_remote_connections_permit: Option<OwnedSemaphorePermit>,
) -> NodeResponse<T>
@@ -478,6 +503,7 @@ impl<C: Clone + Send + 'static> ParallelFetcher<C> {
context,
remote,
node: node.clone(),
+ client_factory,
};
let result = func(parallel_fetcher_context).await;
@@ -514,6 +540,9 @@ impl<C: Clone + Send + 'static> ParallelFetcher<C> {
let context = self.context.clone();
let func = func.clone();
+
+ let client_factory = Arc::clone(&self.client_factory);
+
let future = async move {
let permit = total_connections_semaphore.acquire_owned().await.unwrap();
@@ -525,6 +554,7 @@ impl<C: Clone + Send + 'static> ParallelFetcher<C> {
context,
remote,
"localhost".into(),
+ client_factory,
permit,
None,
)
--
2.47.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH datacenter-manager 13/20] api: sdn: use PdmApplication handle for accessing remotes
2026-08-17 12:57 [PATCH datacenter-manager/proxmox 00/20] inject application context via API macro for easier integration testing Lukas Wagner
` (11 preceding siblings ...)
2026-08-17 12:57 ` [PATCH datacenter-manager 12/20] parallel fetcher: support a custom client factory Lukas Wagner
@ 2026-08-17 12:57 ` Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 14/20] tests: add helpers for building API-handler-level integration tests Lukas Wagner
` (7 subsequent siblings)
20 siblings, 0 replies; 28+ messages in thread
From: Lukas Wagner @ 2026-08-17 12:57 UTC (permalink / raw)
To: pdm-devel
By using the State<T> extractor in the API handler, we can get a handle
to the PdmApplication injected at server startup.
SDN routes only need to read the remote configuration and use
ParallelFetcher to make requests to remotes, all of which are easily
possible via PdmApplication already.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
server/src/api/sdn/controllers.rs | 16 +++++++++++-----
server/src/api/sdn/vnets.rs | 17 +++++++++++------
server/src/api/sdn/zones.rs | 16 ++++++++++------
3 files changed, 32 insertions(+), 17 deletions(-)
diff --git a/server/src/api/sdn/controllers.rs b/server/src/api/sdn/controllers.rs
index 049358cd..2375b043 100644
--- a/server/src/api/sdn/controllers.rs
+++ b/server/src/api/sdn/controllers.rs
@@ -5,12 +5,12 @@ use anyhow::{Context, Error};
use pbs_api_types::REMOTE_ID_SCHEMA;
use pdm_api_types::{Authid, PRIV_RESOURCE_AUDIT, remotes::RemoteType, sdn::ListController};
use proxmox_access_control::CachedUserInfo;
-use proxmox_router::{Permission, Router, RpcEnvironment, http_bail};
+use proxmox_router::{Permission, Router, RpcEnvironment, State, http_bail};
use proxmox_schema::api;
use pve_api_types::ListControllersType;
-use crate::api::pve;
use crate::api::remotes::RemoteIterator;
+use crate::context::PdmApplication;
use crate::parallel_fetcher::ParallelFetcher;
pub const ROUTER: Router = Router::new().get(&API_METHOD_LIST_CONTROLLERS);
@@ -63,6 +63,7 @@ pub async fn list_controllers(
ty: Option<ListControllersType>,
remotes: Option<HashSet<String>>,
rpcenv: &mut dyn RpcEnvironment,
+ app: State<PdmApplication>,
) -> Result<Vec<ListController>, Error> {
let user_info = CachedUserInfo::new()?;
@@ -75,7 +76,7 @@ pub async fn list_controllers(
http_bail!(FORBIDDEN, "user has no access to resources");
}
- let mut iter = RemoteIterator::new(pdm_config::remotes::instance())?
+ let mut iter = RemoteIterator::new(app.remote_config())?
.remote_type(RemoteType::Pve)
.any_privs(&user_info, &auth_id, PRIV_RESOURCE_AUDIT);
if let Some(ref filter) = remotes {
@@ -83,11 +84,16 @@ pub async fn list_controllers(
}
let mut vnets = Vec::new();
- let fetcher = ParallelFetcher::new((pending, running, ty));
+
+ let fetcher = ParallelFetcher::builder((pending, running, ty))
+ .client_factory(app.client_factory_shared())
+ .build();
let results = fetcher
.do_for_all_remotes(iter.into_remotes(), async |args| {
- Ok(pve::connect(&args.remote)?
+ Ok(args
+ .client_factory
+ .make_pve_client(&args.remote)?
.list_controllers(args.context.0, args.context.1, args.context.2)
.await?)
})
diff --git a/server/src/api/sdn/vnets.rs b/server/src/api/sdn/vnets.rs
index 8e7ae52b..1d72114e 100644
--- a/server/src/api/sdn/vnets.rs
+++ b/server/src/api/sdn/vnets.rs
@@ -1,6 +1,7 @@
use std::collections::HashSet;
use anyhow::{Context, Error};
+
use pbs_api_types::REMOTE_ID_SCHEMA;
use pdm_api_types::{
Authid, PRIV_RESOURCE_AUDIT,
@@ -9,12 +10,11 @@ use pdm_api_types::{
};
use proxmox_access_control::CachedUserInfo;
use proxmox_rest_server::WorkerTask;
-use proxmox_router::{Permission, Router, RpcEnvironment, http_bail};
+use proxmox_router::{Permission, Router, RpcEnvironment, State, http_bail};
use proxmox_schema::api;
use pve_api_types::{CreateVnet, SdnVnetType};
-use crate::api::pve;
-use crate::api::remotes::RemoteIterator;
+use crate::{api::remotes::RemoteIterator, context::PdmApplication};
use crate::{parallel_fetcher::ParallelFetcher, sdn_client::LockedSdnClients};
pub const ROUTER: Router = Router::new()
@@ -64,6 +64,7 @@ async fn list_vnets(
running: Option<bool>,
remotes: Option<HashSet<String>>,
rpcenv: &mut dyn RpcEnvironment,
+ app: State<PdmApplication>,
) -> Result<Vec<ListVnet>, Error> {
let user_info = CachedUserInfo::new()?;
@@ -76,7 +77,7 @@ async fn list_vnets(
http_bail!(FORBIDDEN, "user has no access to resources");
}
- let mut iter = RemoteIterator::new(pdm_config::remotes::instance())?
+ let mut iter = RemoteIterator::new(app.remote_config())?
.remote_type(RemoteType::Pve)
.any_privs(&user_info, &auth_id, PRIV_RESOURCE_AUDIT);
if let Some(ref filter) = remotes {
@@ -84,11 +85,15 @@ async fn list_vnets(
}
let mut vnets = Vec::new();
- let fetcher = ParallelFetcher::new((pending, running));
+ let fetcher = ParallelFetcher::builder((pending, running))
+ .client_factory(app.client_factory_shared())
+ .build();
let results = fetcher
.do_for_all_remotes(iter.into_remotes(), async |args| {
- Ok(pve::connect(&args.remote)?
+ Ok(args
+ .client_factory
+ .make_pve_client(&args.remote)?
.list_vnets(args.context.0, args.context.1)
.await?)
})
diff --git a/server/src/api/sdn/zones.rs b/server/src/api/sdn/zones.rs
index bb7a3822..58b69414 100644
--- a/server/src/api/sdn/zones.rs
+++ b/server/src/api/sdn/zones.rs
@@ -10,12 +10,11 @@ use pdm_api_types::{
};
use proxmox_access_control::CachedUserInfo;
use proxmox_rest_server::WorkerTask;
-use proxmox_router::{Permission, Router, RpcEnvironment, http_bail};
+use proxmox_router::{Permission, Router, RpcEnvironment, State, http_bail};
use proxmox_schema::api;
use pve_api_types::{CreateZone, ListZonesType};
-use crate::api::pve;
-use crate::api::remotes::RemoteIterator;
+use crate::{api::remotes::RemoteIterator, context::PdmApplication};
use crate::{parallel_fetcher::ParallelFetcher, sdn_client::LockedSdnClients};
pub const ROUTER: Router = Router::new()
@@ -70,6 +69,7 @@ pub async fn list_zones(
ty: Option<ListZonesType>,
remotes: Option<HashSet<String>>,
rpcenv: &mut dyn RpcEnvironment,
+ app: State<PdmApplication>,
) -> Result<Vec<ListZone>, Error> {
let user_info = CachedUserInfo::new()?;
@@ -82,7 +82,7 @@ pub async fn list_zones(
http_bail!(FORBIDDEN, "user has no access to resources");
}
- let mut iter = RemoteIterator::new(pdm_config::remotes::instance())?
+ let mut iter = RemoteIterator::new(app.remote_config())?
.remote_type(RemoteType::Pve)
.any_privs(&user_info, &auth_id, PRIV_RESOURCE_AUDIT);
if let Some(ref filter) = remotes {
@@ -90,11 +90,15 @@ pub async fn list_zones(
}
let mut vnets = Vec::new();
- let fetcher = ParallelFetcher::new((pending, running, ty));
+ let fetcher = ParallelFetcher::builder((pending, running, ty))
+ .client_factory(app.client_factory_shared())
+ .build();
let results = fetcher
.do_for_all_remotes(iter.into_remotes(), async |args| {
- Ok(pve::connect(&args.remote)?
+ Ok(args
+ .client_factory
+ .make_pve_client(&args.remote)?
.list_zones(args.context.0, args.context.1, args.context.2)
.await?)
})
--
2.47.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH datacenter-manager 14/20] tests: add helpers for building API-handler-level integration tests
2026-08-17 12:57 [PATCH datacenter-manager/proxmox 00/20] inject application context via API macro for easier integration testing Lukas Wagner
` (12 preceding siblings ...)
2026-08-17 12:57 ` [PATCH datacenter-manager 13/20] api: sdn: use PdmApplication handle for accessing remotes Lukas Wagner
@ 2026-08-17 12:57 ` Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 15/20] tests: add example tests for SDN API routes Lukas Wagner
` (6 subsequent siblings)
20 siblings, 0 replies; 28+ messages in thread
From: Lukas Wagner @ 2026-08-17 12:57 UTC (permalink / raw)
To: pdm-devel
Unfortunately, some subsystems still require static initialization,
namely access control and worker tasks. For those we need a setup
function that is ensure to be only called once in each test binary.
Worker tasks especially require a directory in which it can place task
logs. For this we create a temporary directory, which we clean up using
libc::atexit. Unfortunately, tempfile::TempDir does not work for this
case, since we'd need to put it in a static variable, and `drop` is
never called for those.
TestApplication is essentially a builder that can be used to produce a
PdmApplication suitable for tests.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
server/tests/common/environment.rs | 25 +++
server/tests/common/mod.rs | 56 +++++
server/tests/common/test_application.rs | 273 ++++++++++++++++++++++++
3 files changed, 354 insertions(+)
create mode 100644 server/tests/common/environment.rs
create mode 100644 server/tests/common/mod.rs
create mode 100644 server/tests/common/test_application.rs
diff --git a/server/tests/common/environment.rs b/server/tests/common/environment.rs
new file mode 100644
index 00000000..5dad7b21
--- /dev/null
+++ b/server/tests/common/environment.rs
@@ -0,0 +1,25 @@
+use proxmox_router::RpcEnvironment;
+
+pub struct TestRpcEnvironment;
+
+impl RpcEnvironment for TestRpcEnvironment {
+ fn result_attrib_mut(&mut self) -> &mut serde_json::Value {
+ unimplemented!()
+ }
+
+ fn result_attrib(&self) -> &serde_json::Value {
+ unimplemented!()
+ }
+
+ fn env_type(&self) -> proxmox_router::RpcEnvironmentType {
+ unimplemented!()
+ }
+
+ fn set_auth_id(&mut self, _user: Option<String>) {
+ unimplemented!()
+ }
+
+ fn get_auth_id(&self) -> Option<String> {
+ Some("root@pam".to_string())
+ }
+}
diff --git a/server/tests/common/mod.rs b/server/tests/common/mod.rs
new file mode 100644
index 00000000..49646706
--- /dev/null
+++ b/server/tests/common/mod.rs
@@ -0,0 +1,56 @@
+use std::{
+ path::{Path, PathBuf},
+ sync::{Once, OnceLock},
+};
+
+use anyhow::Context;
+use serde::de::DeserializeOwned;
+
+use proxmox_sys::fs::CreateOptions;
+
+mod environment;
+mod test_application;
+
+pub use environment::TestRpcEnvironment;
+pub use test_application::*;
+
+pub async fn read_captured_response<T: DeserializeOwned, P: AsRef<Path>>(
+ path: P,
+) -> Result<T, proxmox_client::Error> {
+ let s = tokio::fs::read_to_string(path.as_ref())
+ .await
+ .with_context(|| format!("could not read from {path}", path = path.as_ref().display()))
+ .unwrap();
+ Ok(serde_json::from_str(&s).unwrap())
+}
+
+static STATIC_TEMP_DIR: OnceLock<PathBuf> = OnceLock::new();
+
+extern "C" fn cleanup() {
+ if let Some(dir) = STATIC_TEMP_DIR.get() {
+ let _ = std::fs::remove_dir_all(dir);
+ }
+}
+
+pub fn test_setup() {
+ static INIT: Once = Once::new();
+
+ INIT.call_once(|| {
+ let file_opts = CreateOptions::new();
+
+ let dir = proxmox_sys::fs::make_tmp_dir("/tmp", None).unwrap();
+ STATIC_TEMP_DIR.set(dir.clone()).unwrap();
+
+ proxmox_rest_server::init_worker_tasks(dir.clone(), file_opts).unwrap();
+ proxmox_access_control::init::init(&pdm_api_types::AccessControlConfig, dir)
+ .expect("failed to setup access control config");
+
+ unsafe {
+ libc::atexit(cleanup);
+ }
+ });
+}
+
+pub fn rpcenv() -> TestRpcEnvironment {
+ TestRpcEnvironment
+}
diff --git a/server/tests/common/test_application.rs b/server/tests/common/test_application.rs
new file mode 100644
index 00000000..cd8409e3
--- /dev/null
+++ b/server/tests/common/test_application.rs
@@ -0,0 +1,273 @@
+use std::collections::HashMap;
+use std::sync::Arc;
+use std::sync::Mutex;
+
+use anyhow::{Context, Error, bail};
+use serde::{Serialize, de::DeserializeOwned};
+
+use nix::unistd::User;
+use proxmox_client::Client;
+use proxmox_section_config::typed::SectionConfigData;
+
+use pbs_api_types::Authid;
+use pdm_api_types::{
+ ConfigDigest,
+ remotes::{Remote, RemoteType},
+};
+
+use pdm_config::remotes::RemoteConfig;
+
+use server::context::product_config::ProductConfig;
+use server::{
+ connection::{ClientFactory, PveClient},
+ context::ContextFactory,
+ pbs_client::PbsClient,
+};
+
+/// Shared, clonable storage for arbitrary data recorded by a [`PveTestRemote`]'s client
+/// implementation.
+#[derive(Clone, Default)]
+pub struct TestRemoteState(Arc<Mutex<HashMap<String, serde_json::Value>>>);
+
+impl TestRemoteState {
+ /// Record `value` under `key`, overwriting any previous value stored there.
+ pub fn set<T: Serialize>(&self, key: impl Into<String>, value: &T) {
+ let value = serde_json::to_value(value).expect("failed to serialize test state value");
+ self.0.lock().unwrap().insert(key.into(), value);
+ }
+
+ /// Retrieve the value previously stored under `key`, if any.
+ pub fn get<T: DeserializeOwned>(&self, key: &str) -> Option<T> {
+ self.0.lock().unwrap().get(key).cloned().map(|value| {
+ serde_json::from_value(value).expect("failed to deserialize test state value")
+ })
+ }
+}
+
+#[derive(Clone)]
+pub struct PveTestRemote {
+ pub name: String,
+ pub nodes: u32,
+ pub state: TestRemoteState,
+}
+
+#[derive(Clone)]
+struct PveTestRemoteWithClientMaker {
+ remote: PveTestRemote,
+ make_client: Arc<dyn Fn(PveTestRemote) -> Result<Arc<PveClient>, Error> + Send + Sync>,
+}
+
+impl PveTestRemote {
+ pub fn to_remote(&self) -> Remote {
+ Remote {
+ ty: RemoteType::Pve,
+ id: self.name.clone(),
+ nodes: Vec::new(),
+ authid: Authid::root_auth_id().clone(),
+ token: "".into(),
+ web_url: None,
+ }
+ }
+}
+
+#[derive(Clone)]
+pub struct TestApplication {
+ pve_remotes: HashMap<String, PveTestRemoteWithClientMaker>,
+ base_dir: Arc<tempfile::TempDir>,
+}
+
+impl Default for TestApplication {
+ fn default() -> Self {
+ Self::new()
+ }
+}
+
+impl TestApplication {
+ pub fn new() -> Self {
+ Self {
+ pve_remotes: HashMap::new(),
+ base_dir: Arc::new(tempfile::tempdir().expect("could not create temporary directory")),
+ }
+ }
+
+ pub fn with_pve_remote<S, C, F>(mut self, name: S, state: TestRemoteState, f: F) -> Self
+ where
+ S: Into<String>,
+ C: pve_api_types::client::PveClient + Send + Sync + 'static,
+ F: Fn(PveTestRemote) -> C + Send + Sync + 'static,
+ {
+ let name = name.into();
+ let make_client = Arc::new(move |remote| Ok(Arc::new(f(remote)) as Arc<PveClient>));
+
+ self.pve_remotes.insert(
+ name.clone(),
+ PveTestRemoteWithClientMaker {
+ make_client,
+ remote: PveTestRemote {
+ name,
+ nodes: 1,
+ state,
+ },
+ },
+ );
+
+ self
+ }
+}
+
+impl ContextFactory for TestApplication {
+ fn make_client_factory(&self) -> Result<Arc<dyn ClientFactory + Send + Sync>, Error> {
+ Ok(Arc::new(self.clone()))
+ }
+
+ fn make_remote_config(&self) -> Result<Box<dyn RemoteConfig + Send + Sync>, Error> {
+ Ok(Box::new(self.clone()))
+ }
+
+ fn make_product_config(&self) -> Result<ProductConfig, Error> {
+ let user = User::from_uid(nix::unistd::getuid())
+ .ok()
+ .flatten()
+ .context("could not look up user")?;
+
+ let base = self.base_dir.path();
+
+ let config_dir = base.join("config");
+ let state_dir = base.join("state");
+ let run_dir = base.join("run");
+ let cache_dir = base.join("cache");
+
+ std::fs::create_dir(&config_dir)?;
+ std::fs::create_dir(&state_dir)?;
+ std::fs::create_dir(&run_dir)?;
+ std::fs::create_dir(&cache_dir)?;
+
+ ProductConfig::builder()
+ .api_user(user.clone())
+ .priv_user(user)
+ .config_dir(config_dir)
+ .state_dir(state_dir)
+ .run_dir(run_dir)
+ .cache_dir(cache_dir)
+ .build()
+ }
+
+ // Override any other methods here if needed.
+}
+
+impl RemoteConfig for TestApplication {
+ fn read(&self) -> Result<(SectionConfigData<Remote>, ConfigDigest), anyhow::Error> {
+ let mut sections = SectionConfigData::default();
+
+ for pve_remote in self.pve_remotes.values() {
+ sections.insert(
+ pve_remote.remote.name.clone(),
+ pve_remote.remote.to_remote(),
+ );
+ }
+
+ Ok((sections, ConfigDigest::from_slice([])))
+ }
+
+ fn read_secret_token(
+ &self,
+ _remote: &pdm_api_types::remotes::Remote,
+ ) -> Result<String, anyhow::Error> {
+ unimplemented!()
+ }
+
+ fn lock(&self) -> Result<proxmox_product_config::ApiLockGuard, anyhow::Error> {
+ unimplemented!()
+ }
+
+ fn write(
+ &self,
+ _remotes: proxmox_section_config::typed::SectionConfigData<pdm_api_types::remotes::Remote>,
+ ) -> Result<(), anyhow::Error> {
+ unimplemented!()
+ }
+}
+
+#[async_trait::async_trait]
+impl ClientFactory for TestApplication {
+ fn make_pve_client(&self, remote: &Remote) -> Result<Arc<PveClient>, Error> {
+ if let Some(a) = self.pve_remotes.get(&remote.id) {
+ (a.make_client)(a.remote.clone())
+ } else {
+ bail!("remote not registered")
+ }
+ }
+
+ fn make_pve_client_with_endpoint(
+ &self,
+ _remote: &Remote,
+ _target_endpoint: Option<&str>,
+ ) -> Result<Arc<PveClient>, Error> {
+ // (or_bail(&self.make_pve_client, "make_pve_client")?)(remote)
+ bail!("not implemented")
+ }
+
+ fn make_pbs_client(&self, _remote: &Remote) -> Result<Box<PbsClient>, Error> {
+ bail!("not implemented")
+ }
+
+ fn make_raw_client(&self, _remote: &Remote) -> Result<Box<proxmox_client::Client>, Error> {
+ bail!("not implemented")
+ }
+
+ async fn make_pve_client_and_login(&self, _remote: &Remote) -> Result<Arc<PveClient>, Error> {
+ bail!("not implemented")
+ }
+
+ async fn make_pbs_client_and_login(
+ &self,
+ _remote: &Remote,
+ ) -> Result<Box<PbsClient<Client>>, Error> {
+ bail!("not implemented")
+ }
+}
+
+macro_rules! test_pve_client {
+ ($ty:ident { $($overrides:tt)* }) => {
+
+ pub struct $ty(crate::common::PveTestRemote);
+
+ #[async_trait::async_trait]
+ impl pve_api_types::client::PveClient for $ty {
+ async fn list_nodes(
+ &self,
+ ) -> Result<Vec<pve_api_types::ClusterNodeIndexResponse>, proxmox_client::Error> {
+ let mut nodes = Vec::new();
+
+ for i in 0..self.0.nodes {
+ nodes.push(ClusterNodeIndexResponse {
+ cpu: Some(0.0),
+ level: None,
+ maxcpu: Some(4),
+ maxmem: Some(4096),
+ mem: Some(1000),
+ node: format!("{}-node-{i}", self.0.name),
+ ssl_fingerprint: None,
+ status: pve_api_types::ClusterNodeIndexResponseStatus::Online,
+ uptime: Some(1000),
+ });
+ }
+
+ Ok(nodes)
+ }
+ $($overrides)*
+ }
+
+ impl $ty {
+ pub fn remote(&self) -> &str {
+ &self.0.name
+ }
+
+ pub fn state(&self) -> crate::common::TestRemoteState {
+ self.0.state.clone()
+ }
+ }
+ };
+}
+
+pub(crate) use test_pve_client;
--
2.47.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH datacenter-manager 15/20] tests: add example tests for SDN API routes
2026-08-17 12:57 [PATCH datacenter-manager/proxmox 00/20] inject application context via API macro for easier integration testing Lukas Wagner
` (13 preceding siblings ...)
2026-08-17 12:57 ` [PATCH datacenter-manager 14/20] tests: add helpers for building API-handler-level integration tests Lukas Wagner
@ 2026-08-17 12:57 ` Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 16/20] api-cache: add wrapper type Lukas Wagner
` (5 subsequent siblings)
20 siblings, 0 replies; 28+ messages in thread
From: Lukas Wagner @ 2026-08-17 12:57 UTC (permalink / raw)
To: pdm-devel
Using the previously added dependency-injected abstractions for
accessing remotes and clients, and by using the test helpers, we can now
fairly trivially create test cases that call the API handler directly.
These tests also demonstrate how previously captured API responses from
real PVE nodes can be used to build test cases.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
server/src/api/sdn/vnets.rs | 2 +-
.../pve/remote-a/list_vnets.json | 8 ++
.../pve/remote-a/list_zones.json | 8 ++
.../pve/remote-b/list_vnets.json | 8 ++
.../pve/remote-b/list_zones.json | 8 ++
server/tests/test_sdn.rs | 88 +++++++++++++++++++
6 files changed, 121 insertions(+), 1 deletion(-)
create mode 100644 server/tests/api_responses/pve/remote-a/list_vnets.json
create mode 100644 server/tests/api_responses/pve/remote-a/list_zones.json
create mode 100644 server/tests/api_responses/pve/remote-b/list_vnets.json
create mode 100644 server/tests/api_responses/pve/remote-b/list_zones.json
create mode 100644 server/tests/test_sdn.rs
diff --git a/server/src/api/sdn/vnets.rs b/server/src/api/sdn/vnets.rs
index 1d72114e..550d75f7 100644
--- a/server/src/api/sdn/vnets.rs
+++ b/server/src/api/sdn/vnets.rs
@@ -59,7 +59,7 @@ pub const ROUTER: Router = Router::new()
}
)]
/// Query VNets of PVE remotes with optional filtering options
-async fn list_vnets(
+pub async fn list_vnets(
pending: Option<bool>,
running: Option<bool>,
remotes: Option<HashSet<String>>,
diff --git a/server/tests/api_responses/pve/remote-a/list_vnets.json b/server/tests/api_responses/pve/remote-a/list_vnets.json
new file mode 100644
index 00000000..a105de13
--- /dev/null
+++ b/server/tests/api_responses/pve/remote-a/list_vnets.json
@@ -0,0 +1,8 @@
+[
+ {
+ "digest" : "3458032372eb62efb730fec12b34d36627a9e6e8",
+ "type" : "vnet",
+ "vnet" : "aaaavnet",
+ "zone" : "aaaa"
+ }
+]
diff --git a/server/tests/api_responses/pve/remote-a/list_zones.json b/server/tests/api_responses/pve/remote-a/list_zones.json
new file mode 100644
index 00000000..f7932bd9
--- /dev/null
+++ b/server/tests/api_responses/pve/remote-a/list_zones.json
@@ -0,0 +1,8 @@
+[
+ {
+ "digest" : "142f8be078a0fb61fa28a89cffdb17f64b05e3f9",
+ "ipam" : "pve",
+ "type" : "simple",
+ "zone" : "aaaa"
+ }
+]
diff --git a/server/tests/api_responses/pve/remote-b/list_vnets.json b/server/tests/api_responses/pve/remote-b/list_vnets.json
new file mode 100644
index 00000000..4a59edc7
--- /dev/null
+++ b/server/tests/api_responses/pve/remote-b/list_vnets.json
@@ -0,0 +1,8 @@
+[
+ {
+ "digest" : "56e0d24671dca1e6cc30a2cfdfab79a037e66a44",
+ "type" : "vnet",
+ "vnet" : "bbbbvnet",
+ "zone" : "bbbb"
+ }
+]
diff --git a/server/tests/api_responses/pve/remote-b/list_zones.json b/server/tests/api_responses/pve/remote-b/list_zones.json
new file mode 100644
index 00000000..75fd763a
--- /dev/null
+++ b/server/tests/api_responses/pve/remote-b/list_zones.json
@@ -0,0 +1,8 @@
+[
+ {
+ "digest" : "bd994284b5d766ab58b5cb69f7363153d42816f7",
+ "ipam" : "pve",
+ "type" : "simple",
+ "zone" : "bbbb"
+ }
+]
diff --git a/server/tests/test_sdn.rs b/server/tests/test_sdn.rs
new file mode 100644
index 00000000..6eb03101
--- /dev/null
+++ b/server/tests/test_sdn.rs
@@ -0,0 +1,88 @@
+use proxmox_router::State;
+use pve_api_types::{ClusterNodeIndexResponse, ListZonesType, SdnVnet, SdnVnetType, SdnZone};
+use server::context::ContextFactory;
+
+use crate::common::{TestApplication, rpcenv};
+
+pub mod common;
+
+common::test_pve_client!(SdnPveClient {
+ async fn list_zones(
+ &self,
+ _pending: Option<bool>,
+ _running: Option<bool>,
+ _ty: Option<ListZonesType>,
+ ) -> Result<Vec<SdnZone>, proxmox_client::Error> {
+ common::read_captured_response(&format!(
+ "tests/api_responses/pve/{}/list_zones.json",
+ self.remote(),
+ )).await
+ }
+
+ async fn list_vnets(
+ &self,
+ _pending: Option<bool>,
+ _running: Option<bool>,
+ ) -> Result<Vec<SdnVnet>, proxmox_client::Error> {
+ common::read_captured_response(&format!(
+ "tests/api_responses/pve/{}/list_vnets.json",
+ self.remote(),
+ )).await
+ }
+});
+
+#[tokio::test]
+async fn test_lists_zones() {
+ common::test_setup();
+
+ let test_app = TestApplication::new()
+ .with_pve_remote("remote-a", Default::default(), SdnPveClient)
+ .with_pve_remote("remote-b", Default::default(), SdnPveClient);
+
+ let app = test_app.make_pdm_application().unwrap();
+
+ let mut result =
+ server::api::sdn::zones::list_zones(None, None, None, None, &mut rpcenv(), State(app))
+ .await
+ .unwrap();
+
+ // We don't guarantee any ordering
+ result.sort_by(|a, b| a.remote.cmp(&b.remote));
+
+ assert_eq!(result.len(), 2);
+
+ assert_eq!(result[0].remote, "remote-a");
+ assert_eq!(result[0].zone.zone, "aaaa");
+ assert_eq!(result[0].zone.ty, ListZonesType::Simple);
+ assert_eq!(result[1].remote, "remote-b");
+ assert_eq!(result[1].zone.zone, "bbbb");
+ assert_eq!(result[1].zone.ty, ListZonesType::Simple);
+}
+
+#[tokio::test]
+async fn test_lists_vnets() {
+ common::test_setup();
+
+ let test_app = TestApplication::new()
+ .with_pve_remote("remote-a", Default::default(), SdnPveClient)
+ .with_pve_remote("remote-b", Default::default(), SdnPveClient);
+
+ let app = test_app.make_pdm_application().unwrap();
+
+ let mut result =
+ server::api::sdn::vnets::list_vnets(None, None, None, &mut rpcenv(), State(app))
+ .await
+ .unwrap();
+
+ // We don't guarantee any ordering
+ result.sort_by(|a, b| a.remote.cmp(&b.remote));
+
+ assert_eq!(result.len(), 2);
+
+ assert_eq!(result[0].remote, "remote-a");
+ assert_eq!(result[0].vnet.vnet, "aaaavnet");
+ assert_eq!(result[0].vnet.ty, SdnVnetType::Vnet);
+ assert_eq!(result[1].remote, "remote-b");
+ assert_eq!(result[1].vnet.vnet, "bbbbvnet");
+ assert_eq!(result[1].vnet.ty, SdnVnetType::Vnet);
+}
--
2.47.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH datacenter-manager 16/20] api-cache: add wrapper type
2026-08-17 12:57 [PATCH datacenter-manager/proxmox 00/20] inject application context via API macro for easier integration testing Lukas Wagner
` (14 preceding siblings ...)
2026-08-17 12:57 ` [PATCH datacenter-manager 15/20] tests: add example tests for SDN API routes Lukas Wagner
@ 2026-08-17 12:57 ` Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 17/20] context: provide api-cache on the app object Lukas Wagner
` (4 subsequent siblings)
20 siblings, 0 replies; 28+ messages in thread
From: Lukas Wagner @ 2026-08-17 12:57 UTC (permalink / raw)
To: pdm-devel
Wrap the underlying NamespacedCache in a new ApiCache type that
exposes the same read/write helpers as methods. The global CACHE
static and its free functions remain, just implemented as thin
wrappers around the new type, so this only prepares ApiCache to be
owned by PdmApplication in the next commit.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
server/src/api_cache.rs | 98 +++++++++++++++++++++++++++++++++++------
1 file changed, 84 insertions(+), 14 deletions(-)
diff --git a/server/src/api_cache.rs b/server/src/api_cache.rs
index b20f0535..6dc8a043 100644
--- a/server/src/api_cache.rs
+++ b/server/src/api_cache.rs
@@ -59,6 +59,8 @@ use std::time::Duration;
use nix::sys::stat::Mode;
+use proxmox_sys::fs::CreateOptions;
+
use crate::namespaced_cache::{
BlockingReadableCacheNamespace, BlockingWritableCacheNamespace, CacheError, NamespacedCache,
ReadableCacheNamespace, WritableCacheNamespace,
@@ -70,57 +72,125 @@ pub const PDM_API_CACHE_PATH: &str = concat!(pdm_buildcfg::PDM_RUN_DIR_M!(), "/a
const GLOBAL_NAMESPACE: &str = "global";
const LOCK_TIMEOUT: Duration = Duration::from_secs(10);
-static CACHE: LazyLock<NamespacedCache> = LazyLock::new(|| {
+static CACHE: LazyLock<ApiCache> = LazyLock::new(|| {
let file_options = proxmox_product_config::default_create_options();
let dir_options = file_options.perm(Mode::from_bits_truncate(0o750));
- NamespacedCache::new(PathBuf::from(PDM_API_CACHE_PATH), dir_options, file_options)
+ ApiCache::new(PathBuf::from(PDM_API_CACHE_PATH), dir_options, file_options)
});
fn format_remote_namespace(remote: &str) -> String {
format!("remote-{remote}")
}
+/// Cache for API responses from remotes.
+///
+/// Thin wrapper around a [`NamespacedCache`], providing remote-specific and
+/// global namespaces as described in the module documentation.
+pub struct ApiCache {
+ cache: NamespacedCache,
+}
+
+impl ApiCache {
+ pub fn new<P: Into<PathBuf>>(
+ base_directory: P,
+ dir_options: CreateOptions,
+ file_options: CreateOptions,
+ ) -> Self {
+ Self {
+ cache: NamespacedCache::new(base_directory, dir_options, file_options),
+ }
+ }
+
+ /// Lock the cache for reading remote-specific data (blocking interface).
+ pub fn read_remote_blocking(
+ &self,
+ remote: &str,
+ ) -> Result<BlockingReadableCacheNamespace, CacheError> {
+ self.cache
+ .read_blocking(&format_remote_namespace(remote), LOCK_TIMEOUT)
+ }
+
+ /// Lock the cache for writing remote-specific data (blocking interface).
+ pub fn write_remote_blocking(
+ &self,
+ remote: &str,
+ ) -> Result<BlockingWritableCacheNamespace, CacheError> {
+ self.cache
+ .write_blocking(&format_remote_namespace(remote), LOCK_TIMEOUT)
+ }
+
+ /// Lock the cache for reading global data (blocking interface).
+ pub fn read_global_blocking(&self) -> Result<BlockingReadableCacheNamespace, CacheError> {
+ self.cache.read_blocking(GLOBAL_NAMESPACE, LOCK_TIMEOUT)
+ }
+
+ /// Lock the cache for writing global data (blocking interface).
+ pub fn write_global_blocking(&self) -> Result<BlockingWritableCacheNamespace, CacheError> {
+ self.cache.write_blocking(GLOBAL_NAMESPACE, LOCK_TIMEOUT)
+ }
+
+ /// Lock the cache for reading remote-specific data (async interface).
+ pub async fn read_remote(&self, remote: &str) -> Result<ReadableCacheNamespace, CacheError> {
+ self.cache
+ .read(&format_remote_namespace(remote), LOCK_TIMEOUT)
+ .await
+ }
+
+ /// Lock the cache for writing remote-specific data (async interface).
+ pub async fn write_remote(&self, remote: &str) -> Result<WritableCacheNamespace, CacheError> {
+ self.cache
+ .write(&format_remote_namespace(remote), LOCK_TIMEOUT)
+ .await
+ }
+
+ /// Lock the cache for reading global data (async interface).
+ pub async fn read_global(&self) -> Result<ReadableCacheNamespace, CacheError> {
+ self.cache.read(GLOBAL_NAMESPACE, LOCK_TIMEOUT).await
+ }
+
+ /// Lock the cache for writing global data (async interface).
+ pub async fn write_global(&self) -> Result<WritableCacheNamespace, CacheError> {
+ self.cache.write(GLOBAL_NAMESPACE, LOCK_TIMEOUT).await
+ }
+}
+
/// Lock the cache for reading remote-specific data (blocking interface).
pub fn read_remote_blocking(remote: &str) -> Result<BlockingReadableCacheNamespace, CacheError> {
- CACHE.read_blocking(&format_remote_namespace(remote), LOCK_TIMEOUT)
+ CACHE.read_remote_blocking(remote)
}
/// Lock the cache for writing remote-specific data (blocking interface).
pub fn write_remote_blocking(remote: &str) -> Result<BlockingWritableCacheNamespace, CacheError> {
- CACHE.write_blocking(&format_remote_namespace(remote), LOCK_TIMEOUT)
+ CACHE.write_remote_blocking(remote)
}
/// Lock the cache for reading global data (blocking interface).
pub fn read_global_blocking() -> Result<BlockingReadableCacheNamespace, CacheError> {
- CACHE.read_blocking(GLOBAL_NAMESPACE, LOCK_TIMEOUT)
+ CACHE.read_global_blocking()
}
/// Lock the cache for writing global data (blocking interface).
pub fn write_global_blocking() -> Result<BlockingWritableCacheNamespace, CacheError> {
- CACHE.write_blocking(GLOBAL_NAMESPACE, LOCK_TIMEOUT)
+ CACHE.write_global_blocking()
}
/// Lock the cache for reading remote-specific data (async interface).
pub async fn read_remote(remote: &str) -> Result<ReadableCacheNamespace, CacheError> {
- CACHE
- .read(&format_remote_namespace(remote), LOCK_TIMEOUT)
- .await
+ CACHE.read_remote(remote).await
}
/// Lock the cache for writing remote-specific data (async interface).
pub async fn write_remote(remote: &str) -> Result<WritableCacheNamespace, CacheError> {
- CACHE
- .write(&format_remote_namespace(remote), LOCK_TIMEOUT)
- .await
+ CACHE.write_remote(remote).await
}
/// Lock the cache for reading global data (async interface).
pub async fn read_global() -> Result<ReadableCacheNamespace, CacheError> {
- CACHE.read(GLOBAL_NAMESPACE, LOCK_TIMEOUT).await
+ CACHE.read_global().await
}
/// Lock the cache for writing global data (async interface).
pub async fn write_global() -> Result<WritableCacheNamespace, CacheError> {
- CACHE.write(GLOBAL_NAMESPACE, LOCK_TIMEOUT).await
+ CACHE.write_global().await
}
--
2.47.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH datacenter-manager 17/20] context: provide api-cache on the app object
2026-08-17 12:57 [PATCH datacenter-manager/proxmox 00/20] inject application context via API macro for easier integration testing Lukas Wagner
` (15 preceding siblings ...)
2026-08-17 12:57 ` [PATCH datacenter-manager 16/20] api-cache: add wrapper type Lukas Wagner
@ 2026-08-17 12:57 ` Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 18/20] api: subscriptions: use PdmApplication instead of globals Lukas Wagner
` (3 subsequent siblings)
20 siblings, 0 replies; 28+ messages in thread
From: Lukas Wagner @ 2026-08-17 12:57 UTC (permalink / raw)
To: pdm-devel
Move the ApiCache instance from the global CACHE static onto
PdmApplication, built from the product config's cache directory
instead of the hardcoded PDM_API_CACHE_PATH. The free functions in
api_cache now fetch it via context::pdm_application() instead of the
static.
Add default_file_create_options()/default_dir_create_options() to
ProductConfig so the cache's directory and file permissions can be
derived the same way in both the real application and tests.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
server/src/api_cache.rs | 45 ++++++++++---------
.../bin/proxmox-datacenter-privileged-api.rs | 7 ++-
server/src/context/mod.rs | 20 ++++++++-
server/src/context/product_config.rs | 23 +++++++++-
server/tests/common/test_application.rs | 3 ++
5 files changed, 75 insertions(+), 23 deletions(-)
diff --git a/server/src/api_cache.rs b/server/src/api_cache.rs
index 6dc8a043..fed66483 100644
--- a/server/src/api_cache.rs
+++ b/server/src/api_cache.rs
@@ -54,31 +54,22 @@
//! ```
use std::path::PathBuf;
-use std::sync::LazyLock;
use std::time::Duration;
-use nix::sys::stat::Mode;
-
use proxmox_sys::fs::CreateOptions;
+use crate::context;
use crate::namespaced_cache::{
BlockingReadableCacheNamespace, BlockingWritableCacheNamespace, CacheError, NamespacedCache,
ReadableCacheNamespace, WritableCacheNamespace,
};
-/// Path at which API responses are cached.
-pub const PDM_API_CACHE_PATH: &str = concat!(pdm_buildcfg::PDM_RUN_DIR_M!(), "/api-cache");
+/// Subdirectory at which API responses are cached.
+pub const PDM_API_CACHE_SUBDIR: &str = "api-cache";
const GLOBAL_NAMESPACE: &str = "global";
const LOCK_TIMEOUT: Duration = Duration::from_secs(10);
-static CACHE: LazyLock<ApiCache> = LazyLock::new(|| {
- let file_options = proxmox_product_config::default_create_options();
- let dir_options = file_options.perm(Mode::from_bits_truncate(0o750));
-
- ApiCache::new(PathBuf::from(PDM_API_CACHE_PATH), dir_options, file_options)
-});
-
fn format_remote_namespace(remote: &str) -> String {
format!("remote-{remote}")
}
@@ -157,40 +148,54 @@ impl ApiCache {
/// Lock the cache for reading remote-specific data (blocking interface).
pub fn read_remote_blocking(remote: &str) -> Result<BlockingReadableCacheNamespace, CacheError> {
- CACHE.read_remote_blocking(remote)
+ context::pdm_application()
+ .api_cache()
+ .read_remote_blocking(remote)
}
/// Lock the cache for writing remote-specific data (blocking interface).
pub fn write_remote_blocking(remote: &str) -> Result<BlockingWritableCacheNamespace, CacheError> {
- CACHE.write_remote_blocking(remote)
+ context::pdm_application()
+ .api_cache()
+ .write_remote_blocking(remote)
}
/// Lock the cache for reading global data (blocking interface).
pub fn read_global_blocking() -> Result<BlockingReadableCacheNamespace, CacheError> {
- CACHE.read_global_blocking()
+ context::pdm_application()
+ .api_cache()
+ .read_global_blocking()
}
/// Lock the cache for writing global data (blocking interface).
pub fn write_global_blocking() -> Result<BlockingWritableCacheNamespace, CacheError> {
- CACHE.write_global_blocking()
+ context::pdm_application()
+ .api_cache()
+ .write_global_blocking()
}
/// Lock the cache for reading remote-specific data (async interface).
pub async fn read_remote(remote: &str) -> Result<ReadableCacheNamespace, CacheError> {
- CACHE.read_remote(remote).await
+ context::pdm_application()
+ .api_cache()
+ .read_remote(remote)
+ .await
}
/// Lock the cache for writing remote-specific data (async interface).
pub async fn write_remote(remote: &str) -> Result<WritableCacheNamespace, CacheError> {
- CACHE.write_remote(remote).await
+ context::pdm_application()
+ .api_cache()
+ .write_remote(remote)
+ .await
}
/// Lock the cache for reading global data (async interface).
pub async fn read_global() -> Result<ReadableCacheNamespace, CacheError> {
- CACHE.read_global().await
+ context::pdm_application().api_cache().read_global().await
}
/// Lock the cache for writing global data (async interface).
pub async fn write_global() -> Result<WritableCacheNamespace, CacheError> {
- CACHE.write_global().await
+ context::pdm_application().api_cache().write_global().await
}
diff --git a/server/src/bin/proxmox-datacenter-privileged-api.rs b/server/src/bin/proxmox-datacenter-privileged-api.rs
index 3815ab03..d32e10c5 100644
--- a/server/src/bin/proxmox-datacenter-privileged-api.rs
+++ b/server/src/bin/proxmox-datacenter-privileged-api.rs
@@ -2,6 +2,7 @@ use std::path::Path;
use std::pin::pin;
use anyhow::{Context as _, Error, bail, format_err};
+use const_format::concatcp;
use futures::*;
use hyper_util::server::graceful::GracefulShutdown;
use nix::fcntl::AtFlags;
@@ -104,7 +105,11 @@ fn create_directories() -> Result<(), Error> {
)?;
pdm_config::setup::mkdir_perms(
- api_cache::PDM_API_CACHE_PATH,
+ concatcp!(
+ pdm_buildcfg::PDM_RUN_DIR_M!(),
+ "/",
+ api_cache::PDM_API_CACHE_SUBDIR,
+ ),
api_user.uid,
api_user.gid,
0o750,
diff --git a/server/src/context/mod.rs b/server/src/context/mod.rs
index bf9c6c2e..5f95d358 100644
--- a/server/src/context/mod.rs
+++ b/server/src/context/mod.rs
@@ -8,6 +8,7 @@ use anyhow::Error;
use pdm_config::{remotes::RemoteConfig, subscriptions::SubscriptionKeyConfig};
+use crate::api_cache::ApiCache;
use crate::connection::{self, ClientFactory};
#[cfg(remote_config = "faked")]
@@ -80,12 +81,22 @@ pub trait ContextFactory {
}
fn make_pdm_application(&self) -> Result<PdmApplication, Error> {
+ let product_config = self.make_product_config()?;
+
+ let api_cache = ApiCache::new(
+ product_config.run_dir().join("api-cache"),
+ product_config.default_dir_create_options(),
+ product_config.default_file_create_options(),
+ );
+
Ok(PdmApplication {
inner: Arc::new(PdmApplicationInner {
client_factory: self.make_client_factory()?,
remote_config: self.make_remote_config()?,
subscription_key_config: self.make_subscription_key_config()?,
- product_config: self.make_product_config()?,
+ product_config,
+
+ api_cache,
}),
})
}
@@ -140,6 +151,11 @@ impl PdmApplication {
pub fn product_config(&self) -> &ProductConfig {
&self.inner.product_config
}
+
+ /// Get a reference to the [`ApiCache`].
+ pub fn api_cache(&self) -> &ApiCache {
+ &self.inner.api_cache
+ }
}
struct PdmApplicationInner {
@@ -147,4 +163,6 @@ struct PdmApplicationInner {
remote_config: Box<dyn RemoteConfig + Send + Sync>,
subscription_key_config: Box<dyn SubscriptionKeyConfig + Send + Sync>,
product_config: ProductConfig,
+
+ api_cache: ApiCache,
}
diff --git a/server/src/context/product_config.rs b/server/src/context/product_config.rs
index 3c1f44ab..a11c5db1 100644
--- a/server/src/context/product_config.rs
+++ b/server/src/context/product_config.rs
@@ -3,7 +3,8 @@
use std::path::{Path, PathBuf};
use anyhow::Error;
-use nix::unistd::User;
+use nix::{sys::stat::Mode, unistd::User};
+use proxmox_sys::fs::CreateOptions;
#[derive(Clone, Debug)]
pub struct ProductConfig {
@@ -43,6 +44,26 @@ impl ProductConfig {
pub fn cache_dir(&self) -> &Path {
&self.cache_dir
}
+
+ pub fn default_file_create_options(&self) -> CreateOptions {
+ let api_user = self.api_user();
+ let mode = Mode::from_bits_truncate(0o0640);
+
+ CreateOptions::new()
+ .perm(mode)
+ .owner(api_user.uid)
+ .group(api_user.gid)
+ }
+
+ pub fn default_dir_create_options(&self) -> CreateOptions {
+ let api_user = self.api_user();
+ let mode = Mode::from_bits_truncate(0o0750);
+
+ CreateOptions::new()
+ .perm(mode)
+ .owner(api_user.uid)
+ .group(api_user.gid)
+ }
}
#[derive(Default)]
diff --git a/server/tests/common/test_application.rs b/server/tests/common/test_application.rs
index cd8409e3..436513f0 100644
--- a/server/tests/common/test_application.rs
+++ b/server/tests/common/test_application.rs
@@ -142,6 +142,9 @@ impl ContextFactory for TestApplication {
std::fs::create_dir(&run_dir)?;
std::fs::create_dir(&cache_dir)?;
+ // FIXME: Maybe ApiCache::new should do this.
+ std::fs::create_dir(run_dir.join("api-cache"))?;
+
ProductConfig::builder()
.api_user(user.clone())
.priv_user(user)
--
2.47.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH datacenter-manager 18/20] api: subscriptions: use PdmApplication instead of globals
2026-08-17 12:57 [PATCH datacenter-manager/proxmox 00/20] inject application context via API macro for easier integration testing Lukas Wagner
` (16 preceding siblings ...)
2026-08-17 12:57 ` [PATCH datacenter-manager 17/20] context: provide api-cache on the app object Lukas Wagner
@ 2026-08-17 12:57 ` Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 19/20] pdm-config: subscriptions: drop unused accessor functions Lukas Wagner
` (2 subsequent siblings)
20 siblings, 0 replies; 28+ messages in thread
From: Lukas Wagner @ 2026-08-17 12:57 UTC (permalink / raw)
To: pdm-devel
Thread State<PdmApplication> through the subscription API handlers and
the daily-update binary, replacing direct calls to
pdm_config::subscriptions/remotes, crate::connection::make_*_client,
and the api_cache free functions with the equivalent methods on the
injected app handle.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
server/src/api/nodes/subscription.rs | 21 +-
server/src/api/resources.rs | 39 ++-
server/src/api/subscriptions/mod.rs | 224 ++++++++++--------
...proxmox-datacenter-manager-daily-update.rs | 14 +-
4 files changed, 176 insertions(+), 122 deletions(-)
diff --git a/server/src/api/nodes/subscription.rs b/server/src/api/nodes/subscription.rs
index 04e4141e..415f77bc 100644
--- a/server/src/api/nodes/subscription.rs
+++ b/server/src/api/nodes/subscription.rs
@@ -2,7 +2,7 @@ use std::collections::{HashMap, HashSet};
use anyhow::{Error, bail};
-use proxmox_router::{Permission, Router};
+use proxmox_router::{Permission, Router, State};
use proxmox_schema::api;
use proxmox_schema::api_types::NODE_SCHEMA;
use proxmox_subscription::files::update_apt_auth;
@@ -18,6 +18,7 @@ use pdm_api_types::subscription::{
use crate::api::resources::{
fetch_complete_subscription_info_for_remote, get_subscription_info_for_remote,
};
+use crate::context::PdmApplication;
const PRODUCT_URL: &str = "https://pdm.proxmox.com/faq.html";
const APT_AUTH_FN: &str = "/etc/apt/auth.conf.d/pdm.conf";
@@ -31,13 +32,14 @@ fn apt_auth_file_opts() -> CreateOptions {
CreateOptions::new().perm(mode).owner(nix::unistd::ROOT)
}
-async fn get_all_subscription_infos()
--> Result<HashMap<String, (RemoteType, HashMap<String, Option<NodeSubscriptionInfo>>)>, Error> {
+async fn get_all_subscription_infos(
+ app: &PdmApplication,
+) -> Result<HashMap<String, (RemoteType, HashMap<String, Option<NodeSubscriptionInfo>>)>, Error> {
let (remotes_config, _digest) = pdm_config::remotes::config()?;
let mut subscription_info = HashMap::new();
for (remote_name, remote) in remotes_config.iter() {
- match get_subscription_info_for_remote(remote, 24 * 60 * 60).await {
+ match get_subscription_info_for_remote(app, remote, 24 * 60 * 60).await {
Ok(info) => {
subscription_info.insert(remote_name.to_string(), (remote.ty, info));
}
@@ -118,8 +120,8 @@ fn check_counts(stats: &SubscriptionStatistics) -> Result<(), Error> {
}
)]
/// Return subscription status
-pub async fn get_subscription() -> Result<PdmSubscriptionInfo, Error> {
- let infos = get_all_subscription_infos().await?;
+pub async fn get_subscription(app: State<PdmApplication>) -> Result<PdmSubscriptionInfo, Error> {
+ let infos = get_all_subscription_infos(&app).await?;
let statistics = count_subscriptions(&infos);
@@ -156,8 +158,8 @@ pub async fn get_subscription() -> Result<PdmSubscriptionInfo, Error> {
},
)]
/// Update subscription information
-pub async fn check_subscription() -> Result<(), Error> {
- let infos = get_all_subscription_infos().await?;
+pub async fn check_subscription(app: State<PdmApplication>) -> Result<(), Error> {
+ let infos = get_all_subscription_infos(&app).await?;
let stats = count_subscriptions(&infos);
if let Err(err) = check_counts(&stats) {
@@ -189,7 +191,8 @@ pub async fn check_subscription() -> Result<(), Error> {
);
continue 'outer;
};
- let node_info = fetch_complete_subscription_info_for_remote(remote).await?;
+ let node_info =
+ fetch_complete_subscription_info_for_remote(&app, remote).await?;
let Some(info) = node_info.iter().find_map(|(node, val)| {
if let Some(info) = val.as_ref() {
if info.status == SubscriptionStatus::Active
diff --git a/server/src/api/resources.rs b/server/src/api/resources.rs
index 09d2b88d..168f6b68 100644
--- a/server/src/api/resources.rs
+++ b/server/src/api/resources.rs
@@ -22,7 +22,7 @@ use pdm_api_types::{Authid, CachedLocationInfo, PRIV_RESOURCE_AUDIT, VIEW_ID_SCH
use pdm_search::{Search, SearchTerm};
use proxmox_access_control::CachedUserInfo;
use proxmox_router::{
- Permission, Router, RpcEnvironment, SubdirMap, http_bail, list_subdirs_api_method,
+ Permission, Router, RpcEnvironment, State, SubdirMap, http_bail, list_subdirs_api_method,
};
use proxmox_rrd_api_types::RrdTimeframe;
use proxmox_schema::{api, parse_boolean};
@@ -31,6 +31,8 @@ use proxmox_subscription::SubscriptionStatus;
use pve_api_types::{ClusterResource, ClusterResourceNetworkType, ClusterResourceType};
use serde::{Deserialize, Serialize};
+use crate::api_cache::ApiCache;
+use crate::context::PdmApplication;
use crate::metric_collection::top_entities;
use crate::{api_cache, connection, views};
@@ -686,6 +688,7 @@ pub async fn get_subscription_status(
verbose: bool,
view: Option<String>,
rpcenv: &mut dyn RpcEnvironment,
+ app: State<PdmApplication>,
) -> Result<Vec<RemoteSubscriptions>, Error> {
let (remotes_config, _) = pdm_config::remotes::config()?;
@@ -711,10 +714,11 @@ pub async fn get_subscription_status(
}
let view = view.clone();
+ let app_clone = app.clone();
let future = async move {
let (node_status, error) =
- match get_subscription_info_for_remote(&remote, max_age).await {
+ match get_subscription_info_for_remote(&app_clone, &remote, max_age).await {
Ok(mut node_status) => {
node_status.retain(|node, _| {
if let Some(view) = &view {
@@ -846,17 +850,21 @@ struct CachedSubscriptionState {
/// If recent enough cached data is available, it is returned
/// instead of calling out to the remote.
pub async fn get_subscription_info_for_remote(
+ app: &PdmApplication,
remote: &Remote,
max_age: u64,
) -> Result<HashMap<String, Option<NodeSubscriptionInfo>>, Error> {
- if let Some(cached_subscription) = get_cached_subscription_info(&remote.id, max_age).await? {
+ if let Some(cached_subscription) =
+ get_cached_subscription_info(app.api_cache(), &remote.id, max_age).await?
+ {
Ok(cached_subscription.node_info)
} else {
- let node_info = fetch_remote_subscription_info(remote).await?;
+ let node_info = fetch_remote_subscription_info(app, remote).await?;
let now = proxmox_time::epoch_i64();
if let Some(existing_state) =
- update_cached_subscription_info(&remote.id, node_info.clone(), now).await?
+ update_cached_subscription_info(app.api_cache(), &remote.id, node_info.clone(), now)
+ .await?
{
// Somebody else updated the cache while we performed the API request,
// return the more recent data instead of the data we just fetched.
@@ -871,21 +879,24 @@ pub async fn get_subscription_info_for_remote(
/// The cache will be updated, but never read from. This guarantees that the `serverid` is set, as
/// it cannot be stored in the cache.
pub async fn fetch_complete_subscription_info_for_remote(
+ app: &PdmApplication,
remote: &Remote,
) -> Result<HashMap<String, Option<NodeSubscriptionInfo>>, Error> {
- let node_info = fetch_remote_subscription_info(remote).await?;
+ let node_info = fetch_remote_subscription_info(app, remote).await?;
let now = proxmox_time::epoch_i64();
- let _ = update_cached_subscription_info(&remote.id, node_info.clone(), now).await?;
+ let _ = update_cached_subscription_info(app.api_cache(), &remote.id, node_info.clone(), now)
+ .await?;
Ok(node_info)
}
const SUBSCRIPTION_STATE_CACHE_KEY: &str = "subscription-state";
async fn get_cached_subscription_info(
+ api_cache: &ApiCache,
remote: &str,
max_age: u64,
) -> Result<Option<CachedSubscriptionState>, Error> {
- let cache = api_cache::read_remote(remote).await?;
+ let cache = api_cache.read_remote(remote).await?;
let subscription_state = cache
.get_with_max_age(SUBSCRIPTION_STATE_CACHE_KEY, max_age as i64)
.await
@@ -897,8 +908,8 @@ async fn get_cached_subscription_info(
}
/// Drop the cached subscription state for a remote, forcing the next read to refetch.
-pub async fn invalidate_subscription_info_for_remote(remote_id: &str) {
- let cache = match api_cache::write_remote(remote_id).await {
+pub async fn invalidate_subscription_info_for_remote(api_cache: &ApiCache, remote_id: &str) {
+ let cache = match api_cache.write_remote(remote_id).await {
Ok(cache) => cache,
Err(err) => {
log::error!("could not open API cache for {remote_id}: {err}");
@@ -916,11 +927,12 @@ pub async fn invalidate_subscription_info_for_remote(remote_id: &str) {
/// stored state as `Ok(Some(state))`. If the data that was passed in replaced the cache
/// entry, `Ok(None)` is returned.
async fn update_cached_subscription_info(
+ api_cache: &ApiCache,
remote: &str,
node_info: HashMap<String, Option<NodeSubscriptionInfo>>,
now: i64,
) -> Result<Option<CachedSubscriptionState>, Error> {
- let cache = api_cache::write_remote(remote).await?;
+ let cache = api_cache.write_remote(remote).await?;
Ok(cache
.set_if_newer_with_timestamp(
@@ -967,12 +979,13 @@ fn map_node_subscription_list_to_state(
/// Fetch remote resources and map to pdm-native data types.
async fn fetch_remote_subscription_info(
+ app: &PdmApplication,
remote: &Remote,
) -> Result<HashMap<String, Option<NodeSubscriptionInfo>>, Error> {
let mut list = HashMap::new();
match remote.ty {
RemoteType::Pve => {
- let client = connection::make_pve_client(remote)?;
+ let client = app.client_factory().make_pve_client(remote)?;
let nodes = client.list_nodes().await?;
let mut futures = Vec::with_capacity(nodes.len());
@@ -1013,7 +1026,7 @@ async fn fetch_remote_subscription_info(
}
}
RemoteType::Pbs => {
- let client = connection::make_pbs_client(remote)?;
+ let client = app.client_factory().make_pbs_client(remote)?;
let info = client.get_subscription().await.ok().map(|info| {
let level = SubscriptionLevel::from_key(info.key.as_deref());
diff --git a/server/src/api/subscriptions/mod.rs b/server/src/api/subscriptions/mod.rs
index 81582945..922b43f7 100644
--- a/server/src/api/subscriptions/mod.rs
+++ b/server/src/api/subscriptions/mod.rs
@@ -13,7 +13,8 @@ use proxmox_access_control::CachedUserInfo;
use proxmox_config_digest::ConfigDigest;
use proxmox_log::{info, warn};
use proxmox_router::{
- Permission, Router, RpcEnvironment, SubdirMap, http_bail, http_err, list_subdirs_api_method,
+ Permission, Router, RpcEnvironment, State, SubdirMap, http_bail, http_err,
+ list_subdirs_api_method,
};
use proxmox_schema::api;
use proxmox_section_config::typed::SectionConfigData;
@@ -33,6 +34,7 @@ use crate::api::remotes::RemoteIterator;
use crate::api::resources::{
get_subscription_info_for_remote, invalidate_subscription_info_for_remote,
};
+use crate::context::PdmApplication;
pub const ROUTER: Router = Router::new()
.get(&list_subdirs_api_method!(SUBDIRS))
@@ -131,14 +133,18 @@ fn key_not_found(key: &str) -> Error {
/// additionally gated on per-remote `PRIV_RESOURCE_AUDIT` so that an operator who can audit the
/// pool but not a specific remote does not learn which keys are pinned to it (and through that,
/// the existence and rough size of that remote's deployment).
-fn list_keys(rpcenv: &mut dyn RpcEnvironment) -> Result<Vec<SubscriptionKeyEntry>, Error> {
+fn list_keys(
+ rpcenv: &mut dyn RpcEnvironment,
+ app: State<PdmApplication>,
+) -> Result<Vec<SubscriptionKeyEntry>, Error> {
let auth_id: Authid = rpcenv
.get_auth_id()
.context("no authid available")?
.parse()?;
let user_info = CachedUserInfo::new()?;
- let (config, digest) = pdm_config::subscriptions::config()?;
+ let (config, digest) = app.subscription_key_config().read()?;
+
rpcenv["digest"] = digest.to_hex().into();
Ok(config
.into_iter()
@@ -193,6 +199,7 @@ async fn add_keys(
keys: Vec<String>,
digest: Option<ConfigDigest>,
rpcenv: &mut dyn RpcEnvironment,
+ app: State<PdmApplication>,
) -> Result<AddKeysResult, Error> {
if keys.is_empty() {
http_bail!(BAD_REQUEST, "no keys provided");
@@ -229,8 +236,8 @@ async fn add_keys(
let added = entries.len() as u32;
let new_digest = tokio::task::spawn_blocking(move || -> Result<ConfigDigest, Error> {
- let _lock = pdm_config::subscriptions::lock_config()?;
- let (mut config, config_digest) = pdm_config::subscriptions::config()?;
+ let _lock = app.subscription_key_config().lock()?;
+ let (mut config, config_digest) = app.subscription_key_config().read()?;
config_digest.detect_modification(digest.as_ref())?;
// `insert` returns the previous entry when one existed; treat that as the duplicate
@@ -244,7 +251,7 @@ async fn add_keys(
}
}
- pdm_config::subscriptions::save_config(&config)
+ app.subscription_key_config().write(&config)
})
.await??;
rpcenv["digest"] = new_digest.to_hex().into();
@@ -270,14 +277,18 @@ async fn add_keys(
/// Bound entries are hidden from operators who cannot audit the bound remote (mirrors the
/// `list_keys` filter); the response is the same 404 either way so a probe cannot distinguish
/// "key exists but you cannot see it" from "key not in pool".
-fn get_key(key: String, rpcenv: &mut dyn RpcEnvironment) -> Result<SubscriptionKeyEntry, Error> {
+fn get_key(
+ key: String,
+ rpcenv: &mut dyn RpcEnvironment,
+ app: State<PdmApplication>,
+) -> Result<SubscriptionKeyEntry, Error> {
let auth_id: Authid = rpcenv
.get_auth_id()
.context("no authid available")?
.parse()?;
let user_info = CachedUserInfo::new()?;
- let (config, digest) = pdm_config::subscriptions::config()?;
+ let (config, digest) = app.subscription_key_config().read()?;
rpcenv["digest"] = digest.to_hex().into();
let mut entry = config
.get(&key)
@@ -322,6 +333,7 @@ async fn delete_key(
key: String,
digest: Option<ConfigDigest>,
rpcenv: &mut dyn RpcEnvironment,
+ app: State<PdmApplication>,
) -> Result<(), Error> {
let auth_id: Authid = rpcenv
.get_auth_id()
@@ -333,7 +345,7 @@ async fn delete_key(
// operator with only PRIV_SYS_MODIFY should not be able to probe live subscription state on
// a remote they cannot audit. Read the entry once without the lock for this gate; the
// authoritative read happens under the spawn_blocking section below.
- let (pre_config, pre_digest) = pdm_config::subscriptions::config()?;
+ let (pre_config, pre_digest) = app.subscription_key_config().read()?;
let Some(pre_entry) = pre_config.get(&key) else {
return Err(key_not_found(&key));
};
@@ -352,7 +364,7 @@ async fn delete_key(
let pre_binding = pre_entry.remote.as_deref().zip(pre_entry.node.as_deref());
// Owned bool so the orphan guard inside spawn_blocking does not borrow `pre_config`.
let pre_had_binding = pre_binding.is_some();
- let synced_block = check_synced_assignment_for_unassign(&key, pre_binding).await?;
+ let synced_block = check_synced_assignment_for_unassign(&app, &key, pre_binding).await?;
drop(pre_config);
// The lock + sync IO runs on a blocking thread so the async runtime is free for other work
@@ -361,8 +373,8 @@ async fn delete_key(
// cross the boundary; reconstructing it is cheap (it just reads the shared ACL cache).
let new_digest = tokio::task::spawn_blocking(move || -> Result<ConfigDigest, Error> {
let user_info = CachedUserInfo::new()?;
- let _lock = pdm_config::subscriptions::lock_config()?;
- let (mut config, config_digest) = pdm_config::subscriptions::config()?;
+ let _lock = app.subscription_key_config().lock()?;
+ let (mut config, config_digest) = app.subscription_key_config().read()?;
config_digest.detect_modification(digest.as_ref())?;
let Some(entry) = config.get(&key) else {
@@ -404,14 +416,14 @@ async fn delete_key(
// Save the authoritative pool config first: an interrupted remove must not leave a `key`
// entry whose signed blob is gone. A stale shadow blob with no main entry is benign, as
// readers do not consult it.
- let new_digest = pdm_config::subscriptions::save_config(&config)?;
+ let new_digest = app.subscription_key_config().write(&config)?;
// Best-effort shadow cleanup. The shadow only caches signed info, so a corrupt or
// otherwise unparseable shadow must not block removing the key from the pool: drop the
// cached blob when the shadow loads, and leave the orphan entry behind otherwise.
- match pdm_config::subscriptions::shadow_config() {
+ match app.subscription_key_config().read_shadow() {
Ok(mut shadow) => {
shadow.remove(&key);
- if let Err(err) = pdm_config::subscriptions::save_shadow(&shadow) {
+ if let Err(err) = app.subscription_key_config().write_shadow(&shadow) {
warn!("key '{key}' removed from pool, but updating its shadow failed: {err}");
}
}
@@ -457,6 +469,7 @@ async fn set_assignment(
node: String,
digest: Option<ConfigDigest>,
rpcenv: &mut dyn RpcEnvironment,
+ app: State<PdmApplication>,
) -> Result<(), Error> {
let auth_id: Authid = rpcenv
.get_auth_id()
@@ -475,7 +488,7 @@ async fn set_assignment(
// orphans whatever live subscription the old remote still ran. Same shape and same guard
// as delete_key / clear_assignment; only fires when the binding actually moves (re-set to
// the same target leaves the OLD binding intact and carries no orphan risk).
- let (pre_config, pre_digest) = pdm_config::subscriptions::config()?;
+ let (pre_config, pre_digest) = app.subscription_key_config().read()?;
let pre_entry = pre_config.get(&key);
let pre_binding = pre_entry.and_then(|e| e.remote.as_deref().zip(e.node.as_deref()));
let rebind_moves_binding = match pre_binding {
@@ -498,7 +511,7 @@ async fn set_assignment(
}
let pre_had_binding = pre_binding.is_some();
let synced_block = if rebind_moves_binding {
- check_synced_assignment_for_unassign(&key, pre_binding).await?
+ check_synced_assignment_for_unassign(&app, &key, pre_binding).await?
} else {
None
};
@@ -509,8 +522,8 @@ async fn set_assignment(
// under the lock.
let new_digest = tokio::task::spawn_blocking(move || -> Result<ConfigDigest, Error> {
let user_info = CachedUserInfo::new()?;
- let _lock = pdm_config::subscriptions::lock_config()?;
- let (mut config, config_digest) = pdm_config::subscriptions::config()?;
+ let _lock = app.subscription_key_config().lock()?;
+ let (mut config, config_digest) = app.subscription_key_config().read()?;
config_digest.detect_modification(digest.as_ref())?;
let Some(stored_entry) = config.get(&key).cloned() else {
@@ -559,7 +572,7 @@ async fn set_assignment(
);
}
- let (remotes_config, _) = pdm_config::remotes::config()?;
+ let (remotes_config, _) = app.remote_config().read()?;
let remote_entry = remotes_config
.get(&remote)
.ok_or_else(|| http_err!(NOT_FOUND, "remote '{remote}' not found"))?;
@@ -600,7 +613,7 @@ async fn set_assignment(
entry.pending_clear = false;
}
- pdm_config::subscriptions::save_config(&config)
+ app.subscription_key_config().write(&config)
})
.await??;
rpcenv["digest"] = new_digest.to_hex().into();
@@ -631,6 +644,7 @@ async fn clear_assignment(
key: String,
digest: Option<ConfigDigest>,
rpcenv: &mut dyn RpcEnvironment,
+ app: State<PdmApplication>,
) -> Result<(), Error> {
let auth_id: Authid = rpcenv
.get_auth_id()
@@ -641,7 +655,7 @@ async fn clear_assignment(
// Authorise against the entry's bound remote BEFORE hitting the network. An operator with
// only PRIV_SYS_MODIFY should not be able to probe live subscription state on a remote
// they cannot audit. The authoritative re-check happens after the lock below.
- let (pre_config, pre_digest) = pdm_config::subscriptions::config()?;
+ let (pre_config, pre_digest) = app.subscription_key_config().read()?;
let pre_entry = pre_config.get(&key);
if let Some(pre_entry) = pre_entry {
if let Some(assigned_remote) = pre_entry.remote.as_deref() {
@@ -662,7 +676,7 @@ async fn clear_assignment(
let pre_binding = pre_entry.and_then(|e| e.remote.as_deref().zip(e.node.as_deref()));
// Owned bool so the orphan guard inside spawn_blocking does not borrow `pre_config`.
let pre_had_binding = pre_binding.is_some();
- let synced_block = check_synced_assignment_for_unassign(&key, pre_binding).await?;
+ let synced_block = check_synced_assignment_for_unassign(&app, &key, pre_binding).await?;
drop(pre_config);
// The lock + sync IO runs on a blocking thread so the async runtime is free for other work
@@ -671,8 +685,8 @@ async fn clear_assignment(
// boundary; reconstructing it is cheap (it just reads the shared ACL cache).
let new_digest = tokio::task::spawn_blocking(move || -> Result<ConfigDigest, Error> {
let user_info = CachedUserInfo::new()?;
- let _lock = pdm_config::subscriptions::lock_config()?;
- let (mut config, config_digest) = pdm_config::subscriptions::config()?;
+ let _lock = app.subscription_key_config().lock()?;
+ let (mut config, config_digest) = app.subscription_key_config().read()?;
config_digest.detect_modification(digest.as_ref())?;
let Some(stored_entry) = config.get(&key).cloned() else {
@@ -720,7 +734,7 @@ async fn clear_assignment(
// not re-trigger a stale teardown.
entry.pending_clear = false;
- pdm_config::subscriptions::save_config(&config)
+ app.subscription_key_config().write(&config)
})
.await??;
rpcenv["digest"] = new_digest.to_hex().into();
@@ -739,17 +753,19 @@ async fn clear_assignment(
/// parallel rebind between pre-read and here cannot redirect us at a remote the caller has no
/// AUDIT on.
async fn check_synced_assignment_for_unassign(
+ app: &PdmApplication,
key: &str,
binding: Option<(&str, &str)>,
) -> Result<Option<(String, String)>, Error> {
let Some((prev_remote, prev_node)) = binding else {
return Ok(None);
};
- let (remotes_config, _) = pdm_config::remotes::config()?;
+ let (remotes_config, _) = app.remote_config().read()?;
let Some(remote_entry) = remotes_config.get(prev_remote) else {
return Ok(None);
};
- let live = match get_subscription_info_for_remote(remote_entry, FRESH_NODE_STATUS_MAX_AGE).await
+ let live = match get_subscription_info_for_remote(app, remote_entry, FRESH_NODE_STATUS_MAX_AGE)
+ .await
{
Ok(v) => v,
Err(_) => return Ok(None),
@@ -767,13 +783,18 @@ async fn check_synced_assignment_for_unassign(
/// Push a single key to its assigned remote node. Operates on a borrowed `Remote` so the
/// caller can fetch the remotes-config once and reuse it.
-async fn push_key_to_remote(remote: &Remote, key: &str, node_name: &str) -> Result<(), Error> {
+async fn push_key_to_remote(
+ app: &PdmApplication,
+ remote: &Remote,
+ key: &str,
+ node_name: &str,
+) -> Result<(), Error> {
let product_type =
ProductType::from_key(key).ok_or_else(|| format_err!("unrecognised key format: {key}"))?;
match product_type {
ProductType::Pve => {
- let client = crate::connection::make_pve_client(remote)?;
+ let client = app.client_factory().make_pve_client(remote)?;
client
.set_subscription(
node_name,
@@ -784,7 +805,7 @@ async fn push_key_to_remote(remote: &Remote, key: &str, node_name: &str) -> Resu
.await?;
}
ProductType::Pbs => {
- let client = crate::connection::make_pbs_client(remote)?;
+ let client = app.client_factory().make_pbs_client(remote)?;
client
.set_subscription(proxmox_subscription::SetSubscription {
key: key.to_string(),
@@ -806,17 +827,18 @@ async fn push_key_to_remote(remote: &Remote, key: &str, node_name: &str) -> Resu
/// Tear down a node's subscription via the remote's `/nodes/{node}/subscription` endpoint.
async fn delete_subscription_on_remote(
+ app: &PdmApplication,
remote: &Remote,
product_type: ProductType,
node_name: &str,
) -> Result<(), Error> {
match product_type {
ProductType::Pve => {
- let client = crate::connection::make_pve_client(remote)?;
+ let client = app.client_factory().make_pve_client(remote)?;
client.delete_subscription(node_name).await?;
}
ProductType::Pbs => {
- let client = crate::connection::make_pbs_client(remote)?;
+ let client = app.client_factory().make_pbs_client(remote)?;
client.delete_subscription().await?;
}
ProductType::Pmg | ProductType::Pom => {
@@ -831,13 +853,14 @@ async fn delete_subscription_on_remote(
/// Trigger a fresh shop-side subscription check on `remote`/`node` and return once the remote
/// has stored the result. Equivalent to the per-product "Check" button, just driven through PDM.
async fn check_subscription_on_remote(
+ app: &PdmApplication,
remote: &Remote,
product_type: ProductType,
node_name: &str,
) -> Result<(), Error> {
match product_type {
ProductType::Pve => {
- let client = crate::connection::make_pve_client(remote)?;
+ let client = app.client_factory().make_pve_client(remote)?;
client
.update_subscription(
node_name,
@@ -846,7 +869,7 @@ async fn check_subscription_on_remote(
.await?;
}
ProductType::Pbs => {
- let client = crate::connection::make_pbs_client(remote)?;
+ let client = app.client_factory().make_pbs_client(remote)?;
client
.check_subscription(proxmox_subscription::UpdateSubscription { force: Some(true) })
.await?;
@@ -896,6 +919,7 @@ async fn queue_clear(
node: String,
digest: Option<ConfigDigest>,
rpcenv: &mut dyn RpcEnvironment,
+ app: State<PdmApplication>,
) -> Result<(), Error> {
let auth_id: Authid = rpcenv
.get_auth_id()
@@ -916,8 +940,8 @@ async fn queue_clear(
// The lock + sync IO runs on a blocking thread so the async runtime stays free for other
// work even when /etc/proxmox-datacenter-manager/subscriptions is on slow storage.
let new_digest = tokio::task::spawn_blocking(move || -> Result<ConfigDigest, Error> {
- let _lock = pdm_config::subscriptions::lock_config()?;
- let (mut config, config_digest) = pdm_config::subscriptions::config()?;
+ let _lock = app.subscription_key_config().lock()?;
+ let (mut config, config_digest) = app.subscription_key_config().read()?;
config_digest.detect_modification(digest.as_ref())?;
let bound_id = config
@@ -943,7 +967,7 @@ async fn queue_clear(
}
entry.pending_clear = true;
- pdm_config::subscriptions::save_config(&config)
+ app.subscription_key_config().write(&config)
})
.await??;
rpcenv["digest"] = new_digest.to_hex().into();
@@ -975,6 +999,7 @@ async fn revert_pending_clear(
node: String,
digest: Option<ConfigDigest>,
rpcenv: &mut dyn RpcEnvironment,
+ app: State<PdmApplication>,
) -> Result<(), Error> {
let auth_id: Authid = rpcenv
.get_auth_id()
@@ -989,8 +1014,8 @@ async fn revert_pending_clear(
)?;
let new_digest = tokio::task::spawn_blocking(move || -> Result<ConfigDigest, Error> {
- let _lock = pdm_config::subscriptions::lock_config()?;
- let (mut config, config_digest) = pdm_config::subscriptions::config()?;
+ let _lock = app.subscription_key_config().lock()?;
+ let (mut config, config_digest) = app.subscription_key_config().read()?;
config_digest.detect_modification(digest.as_ref())?;
let bound_id = config
@@ -1012,7 +1037,7 @@ async fn revert_pending_clear(
}
entry.pending_clear = false;
- pdm_config::subscriptions::save_config(&config)
+ app.subscription_key_config().write(&config)
})
.await??;
rpcenv["digest"] = new_digest.to_hex().into();
@@ -1046,6 +1071,7 @@ async fn check_subscription(
remote: String,
node: String,
rpcenv: &mut dyn RpcEnvironment,
+ app: State<PdmApplication>,
) -> Result<(), Error> {
let auth_id: Authid = rpcenv
.get_auth_id()
@@ -1059,7 +1085,7 @@ async fn check_subscription(
false,
)?;
- let (remotes_config, _) = pdm_config::remotes::config()?;
+ let (remotes_config, _) = app.remote_config().read()?;
let remote_entry = remotes_config
.get(&remote)
.ok_or_else(|| http_err!(NOT_FOUND, "remote '{remote}' not found"))?;
@@ -1069,10 +1095,10 @@ async fn check_subscription(
pdm_api_types::remotes::RemoteType::Pbs => ProductType::Pbs,
};
- check_subscription_on_remote(remote_entry, product_type, &node)
+ check_subscription_on_remote(&app, remote_entry, product_type, &node)
.await
.map_err(|err| http_err!(BAD_REQUEST, "check failed on {remote}/{node}: {err}"))?;
- invalidate_subscription_info_for_remote(&remote).await;
+ invalidate_subscription_info_for_remote(app.api_cache(), &remote).await;
Ok(())
}
@@ -1115,6 +1141,7 @@ async fn adopt_key(
node: String,
digest: Option<ConfigDigest>,
rpcenv: &mut dyn RpcEnvironment,
+ app: State<PdmApplication>,
) -> Result<(), Error> {
let auth_id: Authid = rpcenv
.get_auth_id()
@@ -1129,15 +1156,15 @@ async fn adopt_key(
)?;
// Pre-fetch digest to catch a parallel set_assignment during the live read below.
- let (_pre_config, pre_digest) = pdm_config::subscriptions::config()?;
+ let (_pre_config, pre_digest) = app.subscription_key_config().read()?;
// Fetch live state before grabbing the config lock so the network call does not pin the
// lock for the duration of a remote query.
- let (remotes_config, _) = pdm_config::remotes::config()?;
+ let (remotes_config, _) = app.remote_config().read()?;
let remote_entry = remotes_config
.get(&remote)
.ok_or_else(|| http_err!(NOT_FOUND, "remote '{remote}' not found"))?;
- let live = get_subscription_info_for_remote(remote_entry, FRESH_NODE_STATUS_MAX_AGE)
+ let live = get_subscription_info_for_remote(&app, remote_entry, FRESH_NODE_STATUS_MAX_AGE)
.await
.map_err(|err| {
http_err!(
@@ -1159,8 +1186,8 @@ async fn adopt_key(
// The lock + sync IO runs on a blocking thread so the async runtime stays free for other
// work even when /etc/proxmox-datacenter-manager/subscriptions is on slow storage.
let new_digest = tokio::task::spawn_blocking(move || -> Result<ConfigDigest, Error> {
- let _lock = pdm_config::subscriptions::lock_config()?;
- let (mut config, config_digest) = pdm_config::subscriptions::config()?;
+ let _lock = app.subscription_key_config().lock()?;
+ let (mut config, config_digest) = app.subscription_key_config().read()?;
config_digest.detect_modification(digest.as_ref())?;
if config_digest != pre_digest {
http_bail!(
@@ -1225,7 +1252,7 @@ async fn adopt_key(
config.insert(live_current_key, entry);
}
- pdm_config::subscriptions::save_config(&config)
+ app.subscription_key_config().write(&config)
})
.await??;
rpcenv["digest"] = new_digest.to_hex().into();
@@ -1269,6 +1296,7 @@ async fn adopt_key(
async fn adopt_all(
digest: Option<ConfigDigest>,
rpcenv: &mut dyn RpcEnvironment,
+ app: State<PdmApplication>,
) -> Result<Vec<AdoptedEntry>, Error> {
let auth_id: Authid = rpcenv
.get_auth_id()
@@ -1278,7 +1306,7 @@ async fn adopt_all(
// Use a fresh node-status snapshot: a cached entry from minutes ago could miss a live
// subscription that was just installed on a remote, or vice-versa, claim a subscription
// that has since been removed. Adopting bogus or already-cleared keys would be a footgun.
- let node_statuses = collect_node_status(FRESH_NODE_STATUS_MAX_AGE, rpcenv).await?;
+ let node_statuses = collect_node_status(&app, FRESH_NODE_STATUS_MAX_AGE, rpcenv).await?;
// Lock + sync IO under spawn_blocking. The closure re-resolves the candidate set under the
// lock: a parallel admin's Assign / Adopt between the network read above and the lock
@@ -1287,8 +1315,8 @@ async fn adopt_all(
let (adopted, new_digest_opt) = tokio::task::spawn_blocking(
move || -> Result<(Vec<AdoptedEntry>, Option<ConfigDigest>), Error> {
let user_info = CachedUserInfo::new()?;
- let _lock = pdm_config::subscriptions::lock_config()?;
- let (mut config, config_digest) = pdm_config::subscriptions::config()?;
+ let _lock = app.subscription_key_config().lock()?;
+ let (mut config, config_digest) = app.subscription_key_config().read()?;
config_digest.detect_modification(digest.as_ref())?;
let mut adopted: Vec<AdoptedEntry> = Vec::new();
@@ -1374,7 +1402,7 @@ async fn adopt_all(
let new_digest = if adopted.is_empty() {
None
} else {
- Some(pdm_config::subscriptions::save_config(&config)?)
+ Some(app.subscription_key_config().write(&config)?)
};
Ok((adopted, new_digest))
},
@@ -1415,14 +1443,16 @@ async fn adopt_all(
async fn node_status(
max_age: Option<u64>,
rpcenv: &mut dyn RpcEnvironment,
+ app: State<PdmApplication>,
) -> Result<Vec<RemoteNodeStatus>, Error> {
- collect_node_status(max_age.unwrap_or(PANEL_NODE_STATUS_MAX_AGE), rpcenv).await
+ collect_node_status(&app, max_age.unwrap_or(PANEL_NODE_STATUS_MAX_AGE), rpcenv).await
}
/// Shared helper: fan out subscription queries to all remotes the caller has audit privilege on,
/// in parallel, reusing the per-remote API cache via `get_subscription_info_for_remote`.
/// Joins the results with the key-pool assignment table.
async fn collect_node_status(
+ app: &PdmApplication,
max_age: u64,
rpcenv: &mut dyn RpcEnvironment,
) -> Result<Vec<RemoteNodeStatus>, Error> {
@@ -1432,18 +1462,17 @@ async fn collect_node_status(
.parse()?;
let user_info = CachedUserInfo::new()?;
- let visible_remotes: Vec<(String, Remote)> =
- RemoteIterator::new(pdm_config::remotes::instance())?
- .any_privs(&user_info, &auth_id, PRIV_RESOURCE_AUDIT)
- .into_iter()
- .collect();
+ let visible_remotes: Vec<(String, Remote)> = RemoteIterator::new(app.remote_config())?
+ .any_privs(&user_info, &auth_id, PRIV_RESOURCE_AUDIT)
+ .into_iter()
+ .collect();
- let (keys_config, _) = pdm_config::subscriptions::config()?;
+ let (keys_config, _) = app.subscription_key_config().read()?;
// `get_subscription_info_for_remote` re-uses the per-remote API cache so this
// fan-out is safe to run concurrently.
let fetch = visible_remotes.iter().map(|(name, remote)| async move {
- let res = get_subscription_info_for_remote(remote, max_age).await;
+ let res = get_subscription_info_for_remote(app, remote, max_age).await;
(name.clone(), remote.ty, res)
});
let results = join_all(fetch).await;
@@ -1528,9 +1557,12 @@ async fn collect_node_status(
/// The response carries nested `AutoAssignProposal` data; clients must submit follow-up
/// `bulk_assign` calls with an `application/json` body, the form-urlencoded path cannot encode
/// the nested structure.
-async fn auto_assign(rpcenv: &mut dyn RpcEnvironment) -> Result<AutoAssignProposal, Error> {
- let node_statuses = collect_node_status(FRESH_NODE_STATUS_MAX_AGE, rpcenv).await?;
- let (config, keys_digest) = pdm_config::subscriptions::config()?;
+async fn auto_assign(
+ rpcenv: &mut dyn RpcEnvironment,
+ app: State<PdmApplication>,
+) -> Result<AutoAssignProposal, Error> {
+ let node_statuses = collect_node_status(&app, FRESH_NODE_STATUS_MAX_AGE, rpcenv).await?;
+ let (config, keys_digest) = app.subscription_key_config().read()?;
let assignments = compute_proposals(&config, &node_statuses);
Ok(AutoAssignProposal {
assignments,
@@ -1566,13 +1598,14 @@ async fn auto_assign(rpcenv: &mut dyn RpcEnvironment) -> Result<AutoAssignPropos
async fn bulk_assign(
proposal: AutoAssignProposal,
rpcenv: &mut dyn RpcEnvironment,
+ app: State<PdmApplication>,
) -> Result<Vec<ProposedAssignment>, Error> {
let auth_id: Authid = rpcenv
.get_auth_id()
.context("no authid available")?
.parse()?;
- let node_statuses = collect_node_status(FRESH_NODE_STATUS_MAX_AGE, rpcenv).await?;
+ let node_statuses = collect_node_status(&app, FRESH_NODE_STATUS_MAX_AGE, rpcenv).await?;
let live_digest = hash_node_status(&node_statuses);
if live_digest != proposal.node_status_digest {
http_bail!(
@@ -1587,10 +1620,10 @@ async fn bulk_assign(
let (applied, new_digest_opt) = tokio::task::spawn_blocking(
move || -> Result<(Vec<ProposedAssignment>, Option<ConfigDigest>), Error> {
let user_info = CachedUserInfo::new()?;
- let _lock = pdm_config::subscriptions::lock_config()?;
- let (mut config, config_digest) = pdm_config::subscriptions::config()?;
+ let _lock = app.subscription_key_config().lock()?;
+ let (mut config, config_digest) = app.subscription_key_config().read()?;
config_digest.detect_modification(Some(&proposal.keys_digest))?;
- let (remotes_config, _) = pdm_config::remotes::config()?;
+ let (remotes_config, _) = app.remote_config().read()?;
let mut applied = Vec::with_capacity(proposal.assignments.len());
for p in &proposal.assignments {
@@ -1652,7 +1685,7 @@ async fn bulk_assign(
let new_digest = if applied.is_empty() {
None
} else {
- Some(pdm_config::subscriptions::save_config(&config)?)
+ Some(app.subscription_key_config().write(&config)?)
};
Ok((applied, new_digest))
},
@@ -1788,6 +1821,7 @@ fn compute_proposals(
async fn apply_pending(
digest: Option<ConfigDigest>,
rpcenv: &mut dyn RpcEnvironment,
+ app: State<PdmApplication>,
) -> Result<Option<String>, Error> {
let auth_id: Authid = rpcenv
.get_auth_id()
@@ -1795,11 +1829,11 @@ async fn apply_pending(
.parse()?;
let user_info = CachedUserInfo::new()?;
- let (_, config_digest) = pdm_config::subscriptions::config()?;
+ let (_, config_digest) = app.subscription_key_config().read()?;
config_digest.detect_modification(digest.as_ref())?;
- let node_statuses = collect_node_status(FRESH_NODE_STATUS_MAX_AGE, rpcenv).await?;
- let pending = compute_pending(&user_info, &auth_id, &node_statuses)?;
+ let node_statuses = collect_node_status(&app, FRESH_NODE_STATUS_MAX_AGE, rpcenv).await?;
+ let pending = compute_pending(&app, &user_info, &auth_id, &node_statuses)?;
if pending.is_empty() {
return Ok(None);
@@ -1811,7 +1845,7 @@ async fn apply_pending(
None,
auth_id.to_string(),
true,
- move |_worker| async move { run_apply_pending(worker_auth).await },
+ move |_worker| async move { run_apply_pending(&app, worker_auth).await },
)?;
Ok(Some(upid))
@@ -1822,12 +1856,12 @@ async fn apply_pending(
/// The worker re-reads remotes and the pool config so a reassign or removal between the API call
/// returning a UPID and the worker firing is honoured (pushing the old key to a node after the
/// operator retracted the assignment was a real footgun).
-async fn run_apply_pending(auth_id: Authid) -> Result<(), Error> {
+async fn run_apply_pending(app: &PdmApplication, auth_id: Authid) -> Result<(), Error> {
let user_info = CachedUserInfo::new()?;
- let (remotes_config, _) = pdm_config::remotes::config()?;
+ let (remotes_config, _) = app.remote_config().read()?;
- let node_statuses = collect_status_uncached(&remotes_config).await;
- let pending = compute_pending(&user_info, &auth_id, &node_statuses)?;
+ let node_statuses = collect_status_uncached(app, &remotes_config).await;
+ let pending = compute_pending(app, &user_info, &auth_id, &node_statuses)?;
if pending.is_empty() {
info!("apply-pending: nothing to do (state changed since the API call)");
@@ -1845,7 +1879,7 @@ async fn run_apply_pending(auth_id: Authid) -> Result<(), Error> {
// branch) makes the at-start snapshot stale, and a parallel admin's Discard Pending
// between worker start and this iteration must cancel a planned op rather than have us
// execute it against a flag the operator just retracted.
- let (config, _) = pdm_config::subscriptions::config()?;
+ let (config, _) = app.subscription_key_config().read()?;
if !pool_assignment_still_valid(&config, &entry) {
info!(
"skipping {}/{}: pool entry changed before worker ran",
@@ -1885,7 +1919,7 @@ async fn run_apply_pending(auth_id: Authid) -> Result<(), Error> {
continue;
};
info!("pushing {redacted} to {}/{}...", entry.remote, entry.node);
- if let Err(err) = push_key_to_remote(remote, &entry.key, &entry.node).await {
+ if let Err(err) = push_key_to_remote(app, remote, &entry.key, &entry.node).await {
warn!(
"push of {redacted} to {}/{} failed: {err}",
entry.remote, entry.node
@@ -1914,7 +1948,7 @@ async fn run_apply_pending(auth_id: Authid) -> Result<(), Error> {
entry.remote, entry.node
);
if let Err(err) =
- delete_subscription_on_remote(remote, product_type, &entry.node).await
+ delete_subscription_on_remote(app, remote, product_type, &entry.node).await
{
warn!(
"clear of {redacted} on {}/{} failed: {err}",
@@ -1937,9 +1971,10 @@ async fn run_apply_pending(auth_id: Authid) -> Result<(), Error> {
let entry_key = entry.key.clone();
let entry_remote = entry.remote.clone();
let entry_node = entry.node.clone();
+ let app = app.clone();
let pool_update = tokio::task::spawn_blocking(move || -> Result<(), Error> {
- let _lock = pdm_config::subscriptions::lock_config()?;
- let (mut updated, _) = pdm_config::subscriptions::config()?;
+ let _lock = app.subscription_key_config().lock()?;
+ let (mut updated, _) = app.subscription_key_config().read()?;
if let Some(stored) = updated.get_mut(&entry_key) {
if stored.remote.as_deref() == Some(entry_remote.as_str())
&& stored.node.as_deref() == Some(entry_node.as_str())
@@ -1950,7 +1985,7 @@ async fn run_apply_pending(auth_id: Authid) -> Result<(), Error> {
}
}
// Worker context: no `rpcenv` to set, post-save digest is unused here.
- let _ = pdm_config::subscriptions::save_config(&updated)?;
+ let _ = app.subscription_key_config().write(&updated)?;
Ok(())
})
.await
@@ -1970,7 +2005,7 @@ async fn run_apply_pending(auth_id: Authid) -> Result<(), Error> {
}
}
info!(" success");
- invalidate_subscription_info_for_remote(&entry.remote).await;
+ invalidate_subscription_info_for_remote(app.api_cache(), &entry.remote).await;
ok += 1;
}
@@ -2031,6 +2066,7 @@ async fn run_apply_pending(auth_id: Authid) -> Result<(), Error> {
async fn clear_pending(
digest: Option<ConfigDigest>,
rpcenv: &mut dyn RpcEnvironment,
+ app: State<PdmApplication>,
) -> Result<ClearPendingResult, Error> {
let auth_id: Authid = rpcenv
.get_auth_id()
@@ -2038,11 +2074,11 @@ async fn clear_pending(
.parse()?;
let user_info = CachedUserInfo::new()?;
- let (_, pre_digest) = pdm_config::subscriptions::config()?;
+ let (_, pre_digest) = app.subscription_key_config().read()?;
pre_digest.detect_modification(digest.as_ref())?;
- let node_statuses = collect_node_status(FRESH_NODE_STATUS_MAX_AGE, rpcenv).await?;
- let pending = compute_pending(&user_info, &auth_id, &node_statuses)?;
+ let node_statuses = collect_node_status(&app, FRESH_NODE_STATUS_MAX_AGE, rpcenv).await?;
+ let pending = compute_pending(&app, &user_info, &auth_id, &node_statuses)?;
if pending.is_empty() {
return Ok(ClearPendingResult { cleared: 0 });
@@ -2052,8 +2088,8 @@ async fn clear_pending(
// operations.
let (cleared, new_digest_opt) =
tokio::task::spawn_blocking(move || -> Result<(u32, Option<ConfigDigest>), Error> {
- let _lock = pdm_config::subscriptions::lock_config()?;
- let (mut config, locked_digest) = pdm_config::subscriptions::config()?;
+ let _lock = app.subscription_key_config().lock()?;
+ let (mut config, locked_digest) = app.subscription_key_config().read()?;
locked_digest.detect_modification(digest.as_ref())?;
let mut cleared: u32 = 0;
@@ -2087,7 +2123,7 @@ async fn clear_pending(
}
let new_digest = if cleared > 0 {
- Some(pdm_config::subscriptions::save_config(&config)?)
+ Some(app.subscription_key_config().write(&config)?)
} else {
None
};
@@ -2120,11 +2156,12 @@ enum PendingOp {
}
fn compute_pending(
+ app: &PdmApplication,
user_info: &CachedUserInfo,
auth_id: &Authid,
node_statuses: &[RemoteNodeStatus],
) -> Result<Vec<PendingEntry>, Error> {
- let (config, _) = pdm_config::subscriptions::config()?;
+ let (config, _) = app.subscription_key_config().read()?;
Ok(config
.iter()
@@ -2182,10 +2219,11 @@ fn pool_assignment_still_valid(
/// Like [`collect_node_status`] but bypasses the auth filter, for the apply-pending worker
/// which gates each entry through its own per-remote priv check based on the persisted pool plan.
async fn collect_status_uncached(
+ app: &PdmApplication,
remotes_config: &SectionConfigData<Remote>,
) -> Vec<RemoteNodeStatus> {
let fetch = remotes_config.iter().map(|(name, remote)| async move {
- let res = get_subscription_info_for_remote(remote, FRESH_NODE_STATUS_MAX_AGE).await;
+ let res = get_subscription_info_for_remote(app, remote, FRESH_NODE_STATUS_MAX_AGE).await;
(name.to_string(), remote.ty, res)
});
let results = join_all(fetch).await;
diff --git a/server/src/bin/proxmox-datacenter-manager-daily-update.rs b/server/src/bin/proxmox-datacenter-manager-daily-update.rs
index 314b3399..b5989109 100644
--- a/server/src/bin/proxmox-datacenter-manager-daily-update.rs
+++ b/server/src/bin/proxmox-datacenter-manager-daily-update.rs
@@ -2,11 +2,11 @@ use anyhow::Error;
use serde_json::json;
//use proxmox_notify::context::pbs::PBS_CONTEXT;
-use proxmox_router::{ApiHandler, RpcEnvironment, cli::*};
+use proxmox_router::{ApiHandler, RpcEnvironment, State, cli::*};
use proxmox_subscription::SubscriptionStatus;
use proxmox_sys::fs::CreateOptions;
-use server::api;
+use server::{api, context::PdmApplication};
async fn wait_for_local_worker(upid_str: &str) -> Result<(), Error> {
let upid: pbs_api_types::UPID = upid_str.parse()?;
@@ -22,11 +22,11 @@ async fn wait_for_local_worker(upid_str: &str) -> Result<(), Error> {
}
/// Daily update
-async fn do_update(rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> {
- if let Err(err) = &api::nodes::subscription::check_subscription().await {
+async fn do_update(rpcenv: &mut dyn RpcEnvironment, app: PdmApplication) -> Result<(), Error> {
+ if let Err(err) = &api::nodes::subscription::check_subscription(State(app.clone())).await {
log::error!("Error checking subscription - {err}");
}
- match api::nodes::subscription::get_subscription().await {
+ match api::nodes::subscription::get_subscription(State(app)).await {
Ok(info) if info.info.status == SubscriptionStatus::Active => {}
Ok(info) => {
log::warn!(
@@ -101,9 +101,9 @@ async fn run(rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> {
proxmox_product_config::init(pdm_config::api_user()?, pdm_config::priv_user()?);
proxmox_acme_api::init(pdm_buildcfg::configdir!("/acme"), false)?;
- server::context::init()?;
+ let app = server::context::init()?;
- do_update(rpcenv).await
+ do_update(rpcenv, app).await
}
fn main() {
--
2.47.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH datacenter-manager 19/20] pdm-config: subscriptions: drop unused accessor functions
2026-08-17 12:57 [PATCH datacenter-manager/proxmox 00/20] inject application context via API macro for easier integration testing Lukas Wagner
` (17 preceding siblings ...)
2026-08-17 12:57 ` [PATCH datacenter-manager 18/20] api: subscriptions: use PdmApplication instead of globals Lukas Wagner
@ 2026-08-17 12:57 ` Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 20/20] tests: add example tests for remote subscription management Lukas Wagner
2026-08-20 14:54 ` superseded: [PATCH datacenter-manager/proxmox 00/20] inject application context via API macro for easier integration testing Lukas Wagner
20 siblings, 0 replies; 28+ messages in thread
From: Lukas Wagner @ 2026-08-17 12:57 UTC (permalink / raw)
To: pdm-devel
Now that the subscription API handlers reach the config through
PdmApplication instead, the free functions built on top of the global
INSTANCE static are unused. Drop them along with the init() call that
used to populate it.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
lib/pdm-config/src/subscriptions.rs | 39 -----------------------------
server/src/context/mod.rs | 1 -
2 files changed, 40 deletions(-)
diff --git a/lib/pdm-config/src/subscriptions.rs b/lib/pdm-config/src/subscriptions.rs
index 5be88a13..c4b99d25 100644
--- a/lib/pdm-config/src/subscriptions.rs
+++ b/lib/pdm-config/src/subscriptions.rs
@@ -7,8 +7,6 @@
//! entries, which is intended as future proofing for a more automated (shop) import without having
//! to adapt the data layer.
-use std::sync::OnceLock;
-
use anyhow::Error;
use proxmox_config_digest::ConfigDigest;
@@ -25,37 +23,6 @@ pub const SUBSCRIPTIONS_CFG_FILENAME: &str = configdir!("/subscriptions/keys.cfg
const SUBSCRIPTIONS_SHADOW_FILENAME: &str = configdir!("/subscriptions/keys.shadow");
pub const SUBSCRIPTIONS_CFG_LOCKFILE: &str = configdir!("/subscriptions/.keys.lock");
-static INSTANCE: OnceLock<Box<dyn SubscriptionKeyConfig + Send + Sync>> = OnceLock::new();
-
-fn instance() -> &'static (dyn SubscriptionKeyConfig + Send + Sync) {
- INSTANCE
- .get()
- .expect("subscription key config not initialized")
- .as_ref()
-}
-
-pub fn lock_config() -> Result<ApiLockGuard, Error> {
- instance().lock()
-}
-
-pub fn config() -> Result<(SectionConfigData<SubscriptionKeyEntry>, ConfigDigest), Error> {
- instance().read()
-}
-
-pub fn shadow_config() -> Result<SectionConfigData<SubscriptionKeyShadow>, Error> {
- instance().read_shadow()
-}
-
-pub fn save_config(
- config: &SectionConfigData<SubscriptionKeyEntry>,
-) -> Result<ConfigDigest, Error> {
- instance().write(config)
-}
-
-pub fn save_shadow(shadow: &SectionConfigData<SubscriptionKeyShadow>) -> Result<(), Error> {
- instance().write_shadow(shadow)
-}
-
pub trait SubscriptionKeyConfig {
fn read(&self) -> Result<(SectionConfigData<SubscriptionKeyEntry>, ConfigDigest), Error>;
fn read_shadow(&self) -> Result<SectionConfigData<SubscriptionKeyShadow>, Error>;
@@ -110,9 +77,3 @@ impl SubscriptionKeyConfig for DefaultSubscriptionKeyConfig {
replace_secret_config(SUBSCRIPTIONS_SHADOW_FILENAME, raw.as_bytes())
}
}
-
-pub fn init(instance: Box<dyn SubscriptionKeyConfig + Send + Sync>) {
- if INSTANCE.set(instance).is_err() {
- panic!("subscription key config instance already set");
- }
-}
diff --git a/server/src/context/mod.rs b/server/src/context/mod.rs
index 5f95d358..b64275c8 100644
--- a/server/src/context/mod.rs
+++ b/server/src/context/mod.rs
@@ -36,7 +36,6 @@ pub fn init() -> Result<PdmApplication, Error> {
// anyway, and the implementation is stateless, so having this second
// instance is not an issue *currently*.
pdm_config::remotes::init(factory.make_remote_config()?);
- pdm_config::subscriptions::init(factory.make_subscription_key_config()?);
Ok(app)
}
--
2.47.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH datacenter-manager 20/20] tests: add example tests for remote subscription management
2026-08-17 12:57 [PATCH datacenter-manager/proxmox 00/20] inject application context via API macro for easier integration testing Lukas Wagner
` (18 preceding siblings ...)
2026-08-17 12:57 ` [PATCH datacenter-manager 19/20] pdm-config: subscriptions: drop unused accessor functions Lukas Wagner
@ 2026-08-17 12:57 ` Lukas Wagner
2026-08-20 14:54 ` superseded: [PATCH datacenter-manager/proxmox 00/20] inject application context via API macro for easier integration testing Lukas Wagner
20 siblings, 0 replies; 28+ messages in thread
From: Lukas Wagner @ 2026-08-17 12:57 UTC (permalink / raw)
To: pdm-devel
This is obviously pretty incomplete, but it should demonstrate how easy
it is to write tests now, given the injected context.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
server/src/api/nodes/tasks.rs | 2 +-
server/src/api/subscriptions/mod.rs | 12 +--
server/tests/common/environment.rs | 8 +-
server/tests/common/mod.rs | 50 ++++++++-
server/tests/common/test_application.rs | 53 +++++++++-
server/tests/test_subscriptions.rs | 130 ++++++++++++++++++++++++
6 files changed, 240 insertions(+), 15 deletions(-)
create mode 100644 server/tests/test_subscriptions.rs
diff --git a/server/src/api/nodes/tasks.rs b/server/src/api/nodes/tasks.rs
index 31ddb8f1..4c28880f 100644
--- a/server/src/api/nodes/tasks.rs
+++ b/server/src/api/nodes/tasks.rs
@@ -277,7 +277,7 @@ fn stop_task(upid: UPID, rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> {
},
)]
/// Get task status.
-async fn get_task_status(upid: UPID, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
+pub async fn get_task_status(upid: UPID, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
let auth_id: Authid = rpcenv
.get_auth_id()
.context("no authid available")?
diff --git a/server/src/api/subscriptions/mod.rs b/server/src/api/subscriptions/mod.rs
index 922b43f7..087dbb0c 100644
--- a/server/src/api/subscriptions/mod.rs
+++ b/server/src/api/subscriptions/mod.rs
@@ -133,7 +133,7 @@ fn key_not_found(key: &str) -> Error {
/// additionally gated on per-remote `PRIV_RESOURCE_AUDIT` so that an operator who can audit the
/// pool but not a specific remote does not learn which keys are pinned to it (and through that,
/// the existence and rough size of that remote's deployment).
-fn list_keys(
+pub fn list_keys(
rpcenv: &mut dyn RpcEnvironment,
app: State<PdmApplication>,
) -> Result<Vec<SubscriptionKeyEntry>, Error> {
@@ -195,7 +195,7 @@ fn list_keys(
///
/// The post-save digest is set on the response so clients can chain a follow-up mutation without
/// a refetch round-trip.
-async fn add_keys(
+pub async fn add_keys(
keys: Vec<String>,
digest: Option<ConfigDigest>,
rpcenv: &mut dyn RpcEnvironment,
@@ -277,7 +277,7 @@ async fn add_keys(
/// Bound entries are hidden from operators who cannot audit the bound remote (mirrors the
/// `list_keys` filter); the response is the same 404 either way so a probe cannot distinguish
/// "key exists but you cannot see it" from "key not in pool".
-fn get_key(
+pub fn get_key(
key: String,
rpcenv: &mut dyn RpcEnvironment,
app: State<PdmApplication>,
@@ -329,7 +329,7 @@ fn get_key(
/// another admin had pinned. Refuses if the key is currently the live active key on its bound
/// node, since dropping the pool entry would orphan that subscription on the remote: the
/// operator must run Clear Key on the Node Subscription Status panel first.
-async fn delete_key(
+pub async fn delete_key(
key: String,
digest: Option<ConfigDigest>,
rpcenv: &mut dyn RpcEnvironment,
@@ -463,7 +463,7 @@ async fn delete_key(
/// `PRIV_SYS_MODIFY` lets the caller touch the pool config; per-remote `PRIV_RESOURCE_MODIFY`
/// is enforced inside this handler so an operator cannot push a key to a remote they have no
/// other authority on.
-async fn set_assignment(
+pub async fn set_assignment(
key: String,
remote: String,
node: String,
@@ -1818,7 +1818,7 @@ fn compute_proposals(
/// no longer sees. The worker itself deliberately re-reads the pool when it fires (a worker can
/// be scheduled with delay), so a parallel admin edit between API return and worker firing is
/// still honoured - the digest only pins the at-API-call-time plan, not the executed plan.
-async fn apply_pending(
+pub async fn apply_pending(
digest: Option<ConfigDigest>,
rpcenv: &mut dyn RpcEnvironment,
app: State<PdmApplication>,
diff --git a/server/tests/common/environment.rs b/server/tests/common/environment.rs
index 5dad7b21..0ac841c3 100644
--- a/server/tests/common/environment.rs
+++ b/server/tests/common/environment.rs
@@ -1,14 +1,16 @@
use proxmox_router::RpcEnvironment;
-pub struct TestRpcEnvironment;
+pub struct TestRpcEnvironment {
+ pub attribs: serde_json::Value,
+}
impl RpcEnvironment for TestRpcEnvironment {
fn result_attrib_mut(&mut self) -> &mut serde_json::Value {
- unimplemented!()
+ &mut self.attribs
}
fn result_attrib(&self) -> &serde_json::Value {
- unimplemented!()
+ &self.attribs
}
fn env_type(&self) -> proxmox_router::RpcEnvironmentType {
diff --git a/server/tests/common/mod.rs b/server/tests/common/mod.rs
index 49646706..d65918d3 100644
--- a/server/tests/common/mod.rs
+++ b/server/tests/common/mod.rs
@@ -1,9 +1,10 @@
use std::{
path::{Path, PathBuf},
sync::{Once, OnceLock},
+ time::Duration,
};
-use anyhow::Context;
+use anyhow::{Context, Error, bail};
use serde::de::DeserializeOwned;
use proxmox_sys::fs::CreateOptions;
@@ -52,5 +53,50 @@ pub fn test_setup() {
}
pub fn rpcenv() -> TestRpcEnvironment {
- TestRpcEnvironment
+ TestRpcEnvironment {
+ attribs: serde_json::Value::Null,
+ }
+}
+
+#[allow(unused)]
+macro_rules! assert_http_error {
+ ($result:expr, $status:expr $(, $expected:expr)? $(,)?) => {{
+ let err = $result.unwrap_err();
+ let http_err = err.downcast_ref::<proxmox_router::HttpError>().unwrap();
+
+ assert_eq!(http_err.code, $status);
+
+ $(
+ let err_text = err.to_string();
+ assert!(
+ err_text.contains($expected),
+ "'{}' not contained in '{}'",
+ $expected,
+ err_text,
+ );
+ )?
+ }};
+}
+
+#[allow(unused)]
+pub(crate) use assert_http_error;
+
+pub async fn wait_for_task(upid: &str) -> Result<String, Error> {
+ let upid = upid.parse::<pdm_api_types::UPID>()?;
+
+ for _ in 0..50 {
+ let response =
+ server::api::nodes::tasks::get_task_status(upid.clone(), &mut rpcenv()).await?;
+
+ if response["status"].as_str().unwrap() != "running" {
+ return Ok(response["exitstatus"]
+ .as_str()
+ .context("expected exitstatus to be a string")?
+ .into());
+ }
+
+ tokio::time::sleep(Duration::from_millis(100)).await;
+ }
+
+ bail!("worker did not finish after timeout");
}
diff --git a/server/tests/common/test_application.rs b/server/tests/common/test_application.rs
index 436513f0..f81a9681 100644
--- a/server/tests/common/test_application.rs
+++ b/server/tests/common/test_application.rs
@@ -1,21 +1,22 @@
use std::collections::HashMap;
-use std::sync::Arc;
-use std::sync::Mutex;
+use std::sync::{Arc, Mutex};
use anyhow::{Context, Error, bail};
use serde::{Serialize, de::DeserializeOwned};
use nix::unistd::User;
use proxmox_client::Client;
+use proxmox_product_config::create_mocked_lock;
use proxmox_section_config::typed::SectionConfigData;
use pbs_api_types::Authid;
use pdm_api_types::{
ConfigDigest,
remotes::{Remote, RemoteType},
+ subscription::{SubscriptionKeyEntry, SubscriptionKeyShadow},
};
-use pdm_config::remotes::RemoteConfig;
+use pdm_config::{remotes::RemoteConfig, subscriptions::SubscriptionKeyConfig};
use server::context::product_config::ProductConfig;
use server::{
@@ -124,6 +125,12 @@ impl ContextFactory for TestApplication {
Ok(Box::new(self.clone()))
}
+ fn make_subscription_key_config(
+ &self,
+ ) -> Result<Box<dyn SubscriptionKeyConfig + Send + Sync>, Error> {
+ Ok(Box::new(TestSubscritionKeyConfig::default()))
+ }
+
fn make_product_config(&self) -> Result<ProductConfig, Error> {
let user = User::from_uid(nix::unistd::getuid())
.ok()
@@ -230,6 +237,46 @@ impl ClientFactory for TestApplication {
}
}
+#[derive(Default)]
+struct TestSubscritionKeyConfig {
+ shadow: Mutex<SectionConfigData<SubscriptionKeyShadow>>,
+ config: Mutex<SectionConfigData<SubscriptionKeyEntry>>,
+}
+
+impl SubscriptionKeyConfig for TestSubscritionKeyConfig {
+ fn read(&self) -> Result<(SectionConfigData<SubscriptionKeyEntry>, ConfigDigest), Error> {
+ Ok((
+ self.config.lock().unwrap().clone(),
+ ConfigDigest::from_slice([]),
+ ))
+ }
+
+ fn read_shadow(&self) -> Result<SectionConfigData<SubscriptionKeyShadow>, Error> {
+ Ok(self.shadow.lock().unwrap().clone())
+ }
+
+ fn lock(&self) -> Result<proxmox_product_config::ApiLockGuard, Error> {
+ Ok(unsafe { create_mocked_lock() })
+ }
+
+ fn write(
+ &self,
+ config: &SectionConfigData<SubscriptionKeyEntry>,
+ ) -> Result<ConfigDigest, Error> {
+ let mut guard = self.config.lock().unwrap();
+ *guard = config.clone();
+
+ Ok(ConfigDigest::from_slice([]))
+ }
+
+ fn write_shadow(&self, shadow: &SectionConfigData<SubscriptionKeyShadow>) -> Result<(), Error> {
+ let mut guard = self.shadow.lock().unwrap();
+ *guard = shadow.clone();
+
+ Ok(())
+ }
+}
+
macro_rules! test_pve_client {
($ty:ident { $($overrides:tt)* }) => {
diff --git a/server/tests/test_subscriptions.rs b/server/tests/test_subscriptions.rs
new file mode 100644
index 00000000..122e0517
--- /dev/null
+++ b/server/tests/test_subscriptions.rs
@@ -0,0 +1,130 @@
+use http::StatusCode;
+use pdm_api_types::subscription::{ProductType, SubscriptionLevel};
+use proxmox_router::State;
+use pve_api_types::{ClusterNodeIndexResponse, SetSubscription};
+
+use crate::common::{TestApplication, TestRemoteState, rpcenv};
+
+use server::{api, context::ContextFactory};
+
+pub mod common;
+
+#[tokio::test]
+async fn test_manage_inventory() {
+ common::test_setup();
+
+ let test_app = TestApplication::new();
+ let app = test_app.make_pdm_application().unwrap();
+
+ let existing_keys = api::subscriptions::list_keys(&mut rpcenv(), State(app.clone())).unwrap();
+ assert!(existing_keys.is_empty());
+
+ let keys = vec![
+ "pve4c-aaaaaaaaaa".into(),
+ "pve4c-bbbbbbbbbb".into(),
+ "pve4c-bbbbbbbbbb".into(),
+ ];
+
+ let add_result = api::subscriptions::add_keys(keys, None, &mut rpcenv(), State(app.clone()))
+ .await
+ .unwrap();
+
+ assert_eq!(add_result.added, 2);
+ assert_eq!(add_result.deduplicated, 1);
+
+ let existing_keys = api::subscriptions::list_keys(&mut rpcenv(), State(app.clone())).unwrap();
+ assert_eq!(existing_keys.len(), 2);
+
+ let key =
+ api::subscriptions::get_key("pve4c-aaaaaaaaaa".into(), &mut rpcenv(), State(app.clone()))
+ .unwrap();
+
+ assert_eq!(key.key, "pve4c-aaaaaaaaaa");
+ assert_eq!(key.product_type, ProductType::Pve);
+ assert_eq!(key.level, SubscriptionLevel::Community);
+ assert_eq!(key.remote, None);
+
+ api::subscriptions::delete_key(
+ "pve4c-aaaaaaaaaa".into(),
+ None,
+ &mut rpcenv(),
+ State(app.clone()),
+ )
+ .await
+ .unwrap();
+
+ let result =
+ api::subscriptions::get_key("pve4c-aaaaaaaaaa".into(), &mut rpcenv(), State(app.clone()));
+
+ common::assert_http_error!(result, StatusCode::NOT_FOUND, "not found in pool");
+}
+
+#[tokio::test]
+async fn test_invalid_key_format() {
+ common::test_setup();
+
+ let test_app = TestApplication::new();
+ let app = test_app.make_pdm_application().unwrap();
+
+ let keys = vec!["aaaaa".into()];
+
+ let result = api::subscriptions::add_keys(keys, None, &mut rpcenv(), State(app.clone())).await;
+
+ common::assert_http_error!(result, StatusCode::BAD_REQUEST, "unrecognised key format");
+}
+
+common::test_pve_client!(SubscriptionClient {
+ async fn set_subscription(&self, node: &str, params: SetSubscription) -> Result<(), proxmox_client::Error> {
+ self.state().set(format!("set_subscription-{node}"), ¶ms);
+
+ Ok(())
+ }
+});
+
+#[tokio::test]
+async fn test_apply_key() {
+ common::test_setup();
+
+ let remote_state = TestRemoteState::default();
+
+ let test_app = TestApplication::new().with_pve_remote(
+ "remote-a",
+ remote_state.clone(),
+ SubscriptionClient,
+ );
+
+ let app = test_app.make_pdm_application().unwrap();
+
+ let keys = vec![
+ "pve4c-aaaaaaaaaa".into(),
+ "pve4c-bbbbbbbbbb".into(),
+ "pve4c-bbbbbbbbbb".into(),
+ ];
+ let _ = api::subscriptions::add_keys(keys.clone(), None, &mut rpcenv(), State(app.clone()))
+ .await
+ .unwrap();
+
+ api::subscriptions::set_assignment(
+ keys[0].clone(),
+ "remote-a".into(),
+ "remote-a-node-0".into(),
+ None,
+ &mut rpcenv(),
+ State(app.clone()),
+ )
+ .await
+ .unwrap();
+
+ let upid = api::subscriptions::apply_pending(None, &mut rpcenv(), State(app.clone()))
+ .await
+ .unwrap()
+ .unwrap();
+
+ assert_eq!(common::wait_for_task(&upid).await.unwrap(), "OK");
+
+ let set_subscription: SetSubscription = remote_state
+ .get("set_subscription-remote-a-node-0")
+ .expect("subscription was set on remote-a-node-0");
+
+ assert_eq!(set_subscription.key, keys[0]);
+}
--
2.47.3
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH proxmox 01/20] router: introduce shared state
2026-08-17 12:57 ` [PATCH proxmox 01/20] router: introduce shared state Lukas Wagner
@ 2026-08-17 13:26 ` Lukas Wagner
2026-08-20 11:23 ` Lukas Wagner
2026-08-21 13:59 ` Robert Obkircher
2 siblings, 0 replies; 28+ messages in thread
From: Lukas Wagner @ 2026-08-17 13:26 UTC (permalink / raw)
To: Lukas Wagner, pdm-devel
On Mon Aug 17, 2026 at 2:57 PM CEST, Lukas Wagner wrote:
> 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<T> newtype wraps values that come from the registry,
> which allows telling them apart from regular API parameters.
Last sentence is left-over from cleaning up the git history, State<T>
is actually introduced two commits later...
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH proxmox 01/20] router: introduce shared state
2026-08-17 12:57 ` [PATCH proxmox 01/20] router: introduce shared state Lukas Wagner
2026-08-17 13:26 ` Lukas Wagner
@ 2026-08-20 11:23 ` Lukas Wagner
2026-08-21 13:59 ` Robert Obkircher
2 siblings, 0 replies; 28+ messages in thread
From: Lukas Wagner @ 2026-08-20 11:23 UTC (permalink / raw)
To: Lukas Wagner, pdm-devel
On Mon Aug 17, 2026 at 2:57 PM CEST, Lukas Wagner wrote:
> 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<T> newtype wraps values that come from the registry,
> which allows telling them apart from regular API parameters.
>
Since the question came up off-list:
The reason why this is a type-keyed map is that we can inject multiple
types that can later be retrieved by using the State<T> macro. This
allows, for example, having API handlers in crates that require their
own context type to be set up. For instance, in proxmox-notify, one
requires proxmox_notify::Context to be implemented and set up.
For passing that along in API handlers, there could be a
struct NotifyContext {
inner: Box<dyn proxmox_notify::Context>
}
and then in the API handlers defined in proxmox_notify, the context
could then be retrieved as
fn add_sendmail_endpoint(context: State<NotifyContext>, ...) {}
This the outcome of a suggestion from Robert from the RFC of this
series:
https://lore.proxmox.com/pdm-devel/20260129134418.307552-1-l.wagner@proxmox.com/T/#t
In general, this is pretty much identical to how actix-web handles it:
https://actix.rs/docs/application/#state
Under the hood they also use a type-keyed map with `dyn Any` as a value
type.
^ permalink raw reply [flat|nested] 28+ messages in thread
* superseded: [PATCH datacenter-manager/proxmox 00/20] inject application context via API macro for easier integration testing
2026-08-17 12:57 [PATCH datacenter-manager/proxmox 00/20] inject application context via API macro for easier integration testing Lukas Wagner
` (19 preceding siblings ...)
2026-08-17 12:57 ` [PATCH datacenter-manager 20/20] tests: add example tests for remote subscription management Lukas Wagner
@ 2026-08-20 14:54 ` Lukas Wagner
20 siblings, 0 replies; 28+ messages in thread
From: Lukas Wagner @ 2026-08-20 14:54 UTC (permalink / raw)
To: Lukas Wagner, pdm-devel
https://lore.proxmox.com/pdm-devel/20260820145220.418032-1-l.wagner@proxmox.com/T/#t
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH proxmox 03/20] api-macro: support shared state extraction type
2026-08-17 12:57 ` [PATCH proxmox 03/20] api-macro: support shared state extraction type Lukas Wagner
@ 2026-08-21 13:59 ` Robert Obkircher
0 siblings, 0 replies; 28+ messages in thread
From: Robert Obkircher @ 2026-08-21 13:59 UTC (permalink / raw)
To: Lukas Wagner; +Cc: pdm-devel
> The State<T> newtype wraps values that come from the registry, which
> allows telling them apart from regular API parameters. The API macro
> recognizes them in API handler signatures and looks them up in the
> shared state registry of the RpcEnvironment, instead of deserializing
> them from the request parameters. That way a handler declares its
> dependencies on application state in its signature and does not have to
> access the environment by hand.
Another way of telling them appart would be to use an attribute. It's
slightly more verbose by itself, but it would make it easier to pass
the type by value:
e.g.
fn a(#[state] foo: Foo) { f(foo) }
fn b(foo: State<Foo>) { f(*foo) }
Or simply removing the wrapper from the proc macro output could work as
well, but that might be confusing.
The wrapper is totally fine though, I just wanted to mention this for
completeness.
--
Robert Obkircher <r.obkircher@proxmox.com>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH proxmox 01/20] router: introduce shared state
2026-08-17 12:57 ` [PATCH proxmox 01/20] router: introduce shared state Lukas Wagner
2026-08-17 13:26 ` Lukas Wagner
2026-08-20 11:23 ` Lukas Wagner
@ 2026-08-21 13:59 ` Robert Obkircher
2 siblings, 0 replies; 28+ messages in thread
From: Robert Obkircher @ 2026-08-21 13:59 UTC (permalink / raw)
To: Lukas Wagner; +Cc: pdm-devel
> 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<T> newtype wraps values that come from the registry,
> which allows telling them apart from regular API parameters.
>
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
>
> 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<String>,
> + shared_state_registry: Option<SharedStateRegistry>,
> pub(crate) global_options: HashMap<TypeId, Box<dyn Any + Send + Sync + 'static>>,
> }
>
> @@ -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<String> {
> 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<std::net::SocketAddr> {
> 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<TypeId, Box<dyn Any + Send + Sync>>,
> +}
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::<A>() { &self.a }
else if id == TypeId::of::<B>() { &self.b }
else { unreachable!() }
}
}
> +
> +impl SharedStateRegistry {
> + /// Get a clone of the registered value of type `T`, if there is one.
> + pub fn lookup<T: 'static + Send + Sync + Clone>(&self) -> Option<T> {
> + self.map
> + .get(&TypeId::of::<T>())
> + .and_then(|s| s.downcast_ref())
> + .cloned()
This could .expect("value must have correct type for key").
--
Robert Obkircher <r.obkircher@proxmox.com>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH datacenter-manager 06/20] pdm-config: subscriptions: rename trait methods to read/write/lock
2026-08-17 12:57 ` [PATCH datacenter-manager 06/20] pdm-config: subscriptions: " Lukas Wagner
@ 2026-08-21 14:00 ` Robert Obkircher
0 siblings, 0 replies; 28+ messages in thread
From: Robert Obkircher @ 2026-08-21 14:00 UTC (permalink / raw)
To: Lukas Wagner; +Cc: pdm-devel
> PdmApplication bundles the client factory, remote config, subscription
> key config, and product config behind a single cloneable handle. This
> replaces the growing set of independent global statics with one object
> that can be assembled differently for production, the fake-remote
> feature, and integration tests.
Bundling them is convenient, but the components could additionally be
registered individually, to allow for application independent methods.
e.g. State<ProductConfig> could work in PBS as well. That might be a
bad example though, becasue api_user and priv_user are used all over
the place and should maybe remain static [1].
I wonder if conditionally compiling to static variables for release
builds and tokio::task_local in tests would work there.
[1] https://lore.proxmox.com/pbs-devel/20260723090815.206114-1-c.ebner@proxmox.com/
--
Robert Obkircher <r.obkircher@proxmox.com>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH datacenter-manager 11/20] parallel fetcher: pass arguments to closure in a single type
2026-08-17 12:57 ` [PATCH datacenter-manager 11/20] parallel fetcher: pass arguments to closure in a single type Lukas Wagner
@ 2026-08-21 14:00 ` Robert Obkircher
0 siblings, 0 replies; 28+ messages in thread
From: Robert Obkircher @ 2026-08-21 14:00 UTC (permalink / raw)
To: Lukas Wagner; +Cc: pdm-devel
> Unfortunately, some subsystems still require static initialization,
> namely access control and worker tasks. For those we need a setup
> function that is ensure to be only called once in each test binary.
>
> Worker tasks especially require a directory in which it can place task
> logs. For this we create a temporary directory, which we clean up using
> libc::atexit. Unfortunately, tempfile::TempDir does not work for this
> case, since we'd need to put it in a static variable, and `drop` is
> never called for those.
>
> TestApplication is essentially a builder that can be used to produce a
> PdmApplication suitable for tests.
>
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
>
> diff --git a/server/tests/common/environment.rs b/server/tests/common/environment.rs
> new file mode 100644
> index 00000000..5dad7b21
> --- /dev/null
> +++ b/server/tests/common/environment.rs
> @@ -0,0 +1,25 @@
> +use proxmox_router::RpcEnvironment;
> +
> +pub struct TestRpcEnvironment;
> +
> +impl RpcEnvironment for TestRpcEnvironment {
> + fn result_attrib_mut(&mut self) -> &mut serde_json::Value {
> + unimplemented!()
> + }
> +
> + fn result_attrib(&self) -> &serde_json::Value {
> + unimplemented!()
> + }
> +
> + fn env_type(&self) -> proxmox_router::RpcEnvironmentType {
> + unimplemented!()
> + }
> +
> + fn set_auth_id(&mut self, _user: Option<String>) {
> + unimplemented!()
> + }
> +
> + fn get_auth_id(&self) -> Option<String> {
> + Some("root@pam".to_string())
> + }
> +}
> diff --git a/server/tests/common/mod.rs b/server/tests/common/mod.rs
> new file mode 100644
> index 00000000..49646706
> --- /dev/null
> +++ b/server/tests/common/mod.rs
> @@ -0,0 +1,56 @@
> +use std::{
> + path::{Path, PathBuf},
> + sync::{Once, OnceLock},
> +};
> +
> +use anyhow::Context;
> +use serde::de::DeserializeOwned;
> +
> +use proxmox_sys::fs::CreateOptions;
> +
> +mod environment;
> +mod test_application;
> +
> +pub use environment::TestRpcEnvironment;
> +pub use test_application::*;
> +
> +pub async fn read_captured_response<T: DeserializeOwned, P: AsRef<Path>>(
> + path: P,
> +) -> Result<T, proxmox_client::Error> {
> + let s = tokio::fs::read_to_string(path.as_ref())
> + .await
> + .with_context(|| format!("could not read from {path}", path = path.as_ref().display()))
> + .unwrap();
> + Ok(serde_json::from_str(&s).unwrap())
> +}
> +
> +static STATIC_TEMP_DIR: OnceLock<PathBuf> = OnceLock::new();
> +
> +extern "C" fn cleanup() {
> + if let Some(dir) = STATIC_TEMP_DIR.get() {
> + let _ = std::fs::remove_dir_all(dir);
> + }
> +}
> +
> +pub fn test_setup() {
> + static INIT: Once = Once::new();
> +
> + INIT.call_once(|| {
> + let file_opts = CreateOptions::new();
> +
> + let dir = proxmox_sys::fs::make_tmp_dir("/tmp", None).unwrap();
A while ago I had a brief discussion with Fabian about how file I/O
should be tested. He argued that tests should not make any assumptions
about the file system outside of CARGO_TARGET_TMPDIR, not even about
/tmp.
But that is only available for real integration tests, not for
unittests.
> + STATIC_TEMP_DIR.set(dir.clone()).unwrap();
> +
> + proxmox_rest_server::init_worker_tasks(dir.clone(), file_opts).unwrap();
> + proxmox_access_control::init::init(&pdm_api_types::AccessControlConfig, dir)
> + .expect("failed to setup access control config");
> +
> + unsafe {
> + libc::atexit(cleanup);
Doesn't this potentially delete files while other threads are stil
accessing them?
--
Robert Obkircher <r.obkircher@proxmox.com>
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2026-08-21 14:00 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-17 12:57 [PATCH datacenter-manager/proxmox 00/20] inject application context via API macro for easier integration testing Lukas Wagner
2026-08-17 12:57 ` [PATCH proxmox 01/20] router: introduce shared state Lukas Wagner
2026-08-17 13:26 ` Lukas Wagner
2026-08-20 11:23 ` Lukas Wagner
2026-08-21 13:59 ` Robert Obkircher
2026-08-17 12:57 ` [PATCH proxmox 02/20] rest-server: allow to inject " Lukas Wagner
2026-08-17 12:57 ` [PATCH proxmox 03/20] api-macro: support shared state extraction type Lukas Wagner
2026-08-21 13:59 ` Robert Obkircher
2026-08-17 12:57 ` [PATCH datacenter-manager 04/20] context: promote context to a dir-style module Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 05/20] pdm-config: remotes: rename trait methods to read/write/lock Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 06/20] pdm-config: subscriptions: " Lukas Wagner
2026-08-21 14:00 ` Robert Obkircher
2026-08-17 12:57 ` [PATCH datacenter-manager 07/20] remote iterator: pass remote config reader explicitly Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 08/20] context: introduce a ContextFactory to build application context Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 09/20] context: establish PdmApplication object Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 10/20] context: register PdmApplication in router Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 11/20] parallel fetcher: pass arguments to closure in a single type Lukas Wagner
2026-08-21 14:00 ` Robert Obkircher
2026-08-17 12:57 ` [PATCH datacenter-manager 12/20] parallel fetcher: support a custom client factory Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 13/20] api: sdn: use PdmApplication handle for accessing remotes Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 14/20] tests: add helpers for building API-handler-level integration tests Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 15/20] tests: add example tests for SDN API routes Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 16/20] api-cache: add wrapper type Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 17/20] context: provide api-cache on the app object Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 18/20] api: subscriptions: use PdmApplication instead of globals Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 19/20] pdm-config: subscriptions: drop unused accessor functions Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 20/20] tests: add example tests for remote subscription management Lukas Wagner
2026-08-20 14:54 ` superseded: [PATCH datacenter-manager/proxmox 00/20] inject application context via API macro for easier integration testing Lukas Wagner
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.