Re: [PATCH datacenter-manager/proxmox v2 00/20] inject application context via API macro for easier integration testing

[Dominik Csapak <d.csapak@proxmox.com>]

looked at the general idea (and at the code in some patches, but not
too deeply)

the direction looks good to me, it's basically an expansion on our
product-config + injection into the api calls, which is nice.

for some pointers/questions see the answer to the individual patches

On 8/20/26 4:52 PM, Lukas Wagner wrote:
> 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 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
> reasonably 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 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 RpcEnvironment,
> 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 frameworks, 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
> 
> 
> ## Bumps needed
> 
>   - proxmox-router
>   - proxmox-rest-server
>   - proxmox-api-macro
> 
> ## Changes since v1:
>    - fix a couple typos, grammar mistakes
>    - extend the commit message for the commit that adds the type-keyed map
>      to RpcEnvironment
>    - fix test case for API macro
>    - consistently use app handle in node subscpription api handler
>    - in read_captured_response, don't unwrap but return an Error
>    - add missing doc comments
> 
> 
> 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          |  25 +-
>   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            |  28 ++
>   server/tests/common/mod.rs                    | 109 ++++++
>   server/tests/common/test_application.rs       | 337 ++++++++++++++++++
>   server/tests/test_sdn.rs                      |  88 +++++
>   server/tests/test_subscriptions.rs            | 130 +++++++
>   40 files changed, 1606 insertions(+), 405 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, 1908 insertions(+), 409 deletions(-)
>