Re: [RFC PATCH datacenter-manager 3/3] ui: acl: list granular level permission paths for resources

Lukas Wagner
2026-03-27T10:21:05Z

On Wed Mar 25, 2026 at 4:35 PM CET, Shan Shaji wrote:
> previously, users were not able to assign permissions to specific
> resources. Resolved this by listing the resources Ids and extending the
> permission paths to include specific resource paths.
>
> Signed-off-by: Shan Shaji <s.shaji@proxmox.com>
> ---
> lib/pdm-api-types/src/acl.rs | 3 +-
> .../configuration/permission_path_selector.rs | 47 ++++++++++++++++++-
> 2 files changed, 48 insertions(+), 2 deletions(-)
>
> diff --git a/lib/pdm-api-types/src/acl.rs b/lib/pdm-api-types/src/acl.rs
> index 405982a..825fd6b 100644
> --- a/lib/pdm-api-types/src/acl.rs
> +++ b/lib/pdm-api-types/src/acl.rs
> @@ -280,9 +280,10 @@ impl proxmox_access_control::init::AccessControlConfig for AccessControlConfig {
> if components_len <= 2 {
> return Ok(());
> }
> +
> // `/resource/{remote-id}/{resource-type=guest,storage}/{resource-id}`
> match components[2] {
> - "guest" | "storage" => {
> + "guest" | "storage" | "datastore" | "node" => {
> // /resource/{remote-id}/{resource-type}
> // /resource/{remote-id}/{resource-type}/{resource-id}
> if components_len <= 4 {

Looks correct to me (quickly double-checked the code base), but as
Shannon already mentioned, this should be its own patch.

> diff --git a/ui/src/configuration/permission_path_selector.rs b/ui/src/configuration/permission_path_selector.rs
> index a0fec86..899fa05 100644
> --- a/ui/src/configuration/permission_path_selector.rs
> +++ b/ui/src/configuration/permission_path_selector.rs
> @@ -9,6 +9,7 @@ use pwt::{prelude::*, AsyncPool};
> use pwt_macros::{builder, widget};
>
> use crate::pdm_client;
> +use pdm_api_types::resource::ResourceType;
>
> static PREDEFINED_PATHS: &[&str] = &[
> "/",
> @@ -67,8 +68,52 @@ impl PdmPermissionPathSelector {
> Ok(paths)
> }
>
> + async fn get_resource_paths() -> Result<Vec<String>, Error> {
> + let resources = pdm_client().resources(None, None).await?;
> + let resource_paths: Vec<String> = resources
> + .into_iter()
> + .flat_map(|remote| {
> + let remote_name = remote.remote;
> +
> + let mut base_paths = vec![remote_name.clone(), format!("{remote_name}/node")];
> + let is_pve = remote.resources.iter().any(|r| {
> + matches!(
> + r.resource_type(),
> + ResourceType::PveQemu | ResourceType::PveLxc
> + )
> + });
> +
> + if is_pve {
> + base_paths.push(format!("{remote_name}/guest"));
> + }
> +
> + let resource_ids =
> + remote.resources.into_iter().filter_map(|resource| {
> + match resource.resource_type() {
> + ResourceType::PveLxc
> + | ResourceType::PveQemu
> + | ResourceType::Node
> + | ResourceType::PbsDatastore => resource
> + .global_id()
> + .strip_prefix("remote/")
> + .map(str::to_owned),
> + _ => None,
> + }
> + });
> +
> + base_paths.into_iter().chain(resource_ids)
> + })
> + .map(|v| format!("{}{v}", "/resource/"))
> + .collect();
> + Ok(resource_paths)
> + }
> +

I don't think we should pre-populate the ACL path selector with *all*
possible resource ACL paths. If you have a big setup with a large number
of remotes/resources, the list can quickly have thousands of entries.
While the performance seemed acceptable in my first test with roughly
6000 resources (which still is not *that* of a large setup), I don't
think having such a huge list of options is a good experience for the
user.

I think it might be best to just add entries for each remote (so
/resource/<remote>) for now.

If we ever want to provide more granularity, I think we need a smarter
approach for selecting/narrowing down on the ACL object in this dialog.

> async fn get_paths() -> Result<Vec<String>, Error> {
> - let paths = Self::get_view_paths().await?;
> + let mut paths = Self::get_view_paths().await?;
> + let mut resource_paths = Self::get_resource_paths().await?;
> +
> + paths.append(&mut resource_paths);
> +
> Ok(paths)
> }
> }