Re: [pdm-devel] [PATCH datacenter-manager v2 06/12] api: resources: list: add support for view-filter parameter

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

see comment inline

On 11/3/25 1:35 PM, Lukas Wagner wrote:
> A view filter allows one to get filtered subset of all resources, based
> on filter rules defined in a config file. View filters integrate with
> the permission system - if a user has permissions on
> /view/{view-filter-id}, then these privileges are transitively applied
> to all resources which are matched by the rules. All other permission
> checks are replaced if requesting data through a view filter.
> 
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
>   server/src/api/resources.rs  | 56 ++++++++++++++++++++++++++++++------
>   server/src/resource_cache.rs |  3 +-
>   2 files changed, 50 insertions(+), 9 deletions(-)
> 
> diff --git a/server/src/api/resources.rs b/server/src/api/resources.rs
> index 81c9d9ae..6feda45b 100644
> --- a/server/src/api/resources.rs
> +++ b/server/src/api/resources.rs
> @@ -18,7 +18,7 @@ use pdm_api_types::resource::{
>   use pdm_api_types::subscription::{
>       NodeSubscriptionInfo, RemoteSubscriptionState, RemoteSubscriptions, SubscriptionLevel,
>   };
> -use pdm_api_types::{Authid, PRIV_RESOURCE_AUDIT};
> +use pdm_api_types::{Authid, PRIV_RESOURCE_AUDIT, VIEW_FILTER_ID_SCHEMA};
>   use pdm_search::{Search, SearchTerm};
>   use proxmox_access_control::CachedUserInfo;
>   use proxmox_router::{
> @@ -30,8 +30,8 @@ use proxmox_sortable_macro::sortable;
>   use proxmox_subscription::SubscriptionStatus;
>   use pve_api_types::{ClusterResource, ClusterResourceType};
>   
> -use crate::connection;
>   use crate::metric_collection::top_entities;
> +use crate::{connection, views};
>   
>   pub const ROUTER: Router = Router::new()
>       .get(&list_subdirs_api_method!(SUBDIRS))
> @@ -221,6 +221,10 @@ impl From<RemoteWithResources> for RemoteResources {
>                   type: ResourceType,
>                   optional: true,
>               },
> +            "view-filter": {
> +                schema: VIEW_FILTER_ID_SCHEMA,
> +                optional: true,
> +            },
>           }
>       },
>       returns: {
> @@ -236,10 +240,17 @@ pub async fn get_resources(
>       max_age: u64,
>       resource_type: Option<ResourceType>,
>       search: Option<String>,
> +    view_filter: Option<String>,
>       rpcenv: &mut dyn RpcEnvironment,
>   ) -> Result<Vec<RemoteResources>, Error> {
> -    let remotes_with_resources =
> -        get_resources_impl(max_age, search, resource_type, Some(rpcenv)).await?;
> +    let remotes_with_resources = get_resources_impl(
> +        max_age,
> +        search,
> +        resource_type,
> +        view_filter.as_deref(),
> +        Some(rpcenv),
> +    )
> +    .await?;
>       let resources = remotes_with_resources.into_iter().map(Into::into).collect();
>       Ok(resources)
>   }
> @@ -276,6 +287,7 @@ pub(crate) async fn get_resources_impl(
>       max_age: u64,
>       search: Option<String>,
>       resource_type: Option<ResourceType>,
> +    view_filter: Option<&str>,
>       rpcenv: Option<&mut dyn RpcEnvironment>,
>   ) -> Result<Vec<RemoteWithResources>, Error> {
>       let user_info = CachedUserInfo::new()?;
> @@ -285,9 +297,15 @@ pub(crate) async fn get_resources_impl(
>               .get_auth_id()
>               .ok_or_else(|| format_err!("no authid available"))?
>               .parse()?;
> -        if !user_info.any_privs_below(&auth_id, &["resource"], PRIV_RESOURCE_AUDIT)? {
> +
> +        // NOTE: Assumption is that the regular permission check is completely replaced by a check
> +        // on the view ACL object *if* a view parameter is passed.
> +        if let Some(view_filter) = &view_filter {
> +            user_info.check_privs(&auth_id, &["view", view_filter], PRIV_RESOURCE_AUDIT, false)?;
> +        } else if !user_info.any_privs_below(&auth_id, &["resource"], PRIV_RESOURCE_AUDIT)? {
>               http_bail!(UNAUTHORIZED, "user has no access to resources");
>           }
> +
>           opt_auth_id = Some(auth_id);
>       }
>   
> @@ -296,12 +314,24 @@ pub(crate) async fn get_resources_impl(
>   
>       let filters = search.map(Search::from).unwrap_or_default();
>   
> +    let view_filter = view_filter
> +        .map(|filter_name| views::view_filter::get_view_filter(&filter_name))
> +        .transpose()?;
> +
>       let remotes_only = is_remotes_only(&filters);
>   
>       for (remote_name, remote) in remotes_config {
>           if let Some(ref auth_id) = opt_auth_id {
> -            let remote_privs = user_info.lookup_privs(auth_id, &["resource", &remote_name]);
> -            if remote_privs & PRIV_RESOURCE_AUDIT == 0 {
> +            if view_filter.is_none() {
> +                let remote_privs = user_info.lookup_privs(auth_id, &["resource", &remote_name]);
> +                if remote_privs & PRIV_RESOURCE_AUDIT == 0 {
> +                    continue;
> +                }
> +            }
> +        }
> +
> +        if let Some(view_filter) = &view_filter {
> +            if view_filter.can_skip_remote(&remote_name) {
>                   continue;
>               }
>           }
> @@ -374,6 +404,15 @@ pub(crate) async fn get_resources_impl(
>           }
>       }
>   
> +    if let Some(filter) = &view_filter {
> +        remote_resources.retain_mut(|r| {
> +            r.resources
> +                .retain(|resource| filter.resource_matches(&r.remote_name, resource));
> +
> +            !r.resources.is_empty()

this leads to remotes that have an error are not being returned.

this should probably only return false when either
the list was not empty before but is after
or if r.error.is_none()

> +        });
> +    }
> +
>       Ok(remote_resources)
>   }
>   
> @@ -405,7 +444,8 @@ pub async fn get_status(
>       max_age: u64,
>       rpcenv: &mut dyn RpcEnvironment,
>   ) -> Result<ResourcesStatus, Error> {
> -    let remotes_with_resources = get_resources_impl(max_age, None, None, Some(rpcenv)).await?;
> +    let remotes_with_resources =
> +        get_resources_impl(max_age, None, None, None, Some(rpcenv)).await?;
>       let mut counts = ResourcesStatus::default();
>       for remote_with_resources in remotes_with_resources {
>           if let Some(err) = remote_with_resources.error {
> diff --git a/server/src/resource_cache.rs b/server/src/resource_cache.rs
> index aa20c54e..dc3cbeaf 100644
> --- a/server/src/resource_cache.rs
> +++ b/server/src/resource_cache.rs
> @@ -21,7 +21,8 @@ pub fn start_task() {
>   async fn resource_caching_task() -> Result<(), Error> {
>       loop {
>           if let Err(err) =
> -            crate::api::resources::get_resources_impl(METRIC_POLL_INTERVALL, None, None, None).await
> +            crate::api::resources::get_resources_impl(METRIC_POLL_INTERVALL, None, None, None, None)
> +                .await
>           {
>               log::error!("could not update resource cache: {err}");
>           }



_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel