Re: [pdm-devel] [PATCH datacenter-manager v2 09/12] api: subscription status: add support for view-filter parameter

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

see comments inline

On 11/3/25 1:36 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 | 66 ++++++++++++++++++++++++++++++-------
>   1 file changed, 54 insertions(+), 12 deletions(-)
> 
> diff --git a/server/src/api/resources.rs b/server/src/api/resources.rs
> index f718ce3e..db1e2c7c 100644
> --- a/server/src/api/resources.rs
> +++ b/server/src/api/resources.rs
> @@ -552,6 +552,10 @@ pub async fn get_status(
>                   default: false,
>                   description: "If true, includes subscription information per node (with enough privileges)",
>               },
> +            "view-filter": {
> +                schema: VIEW_FILTER_ID_SCHEMA,
> +                optional: true,
> +            },
>           },
>       },
>       returns: {
> @@ -566,6 +570,7 @@ pub async fn get_status(
>   pub async fn get_subscription_status(
>       max_age: u64,
>       verbose: bool,
> +    view_filter: Option<String>,
>       rpcenv: &mut dyn RpcEnvironment,
>   ) -> Result<Vec<RemoteSubscriptions>, Error> {
>       let (remotes_config, _) = pdm_config::remotes::config()?;
> @@ -574,9 +579,19 @@ pub async fn get_subscription_status(
>   
>       let auth_id = rpcenv.get_auth_id().unwrap().parse()?;
>       let user_info = CachedUserInfo::new()?;
> -    let allow_all = user_info
> -        .check_privs(&auth_id, &["resources"], PRIV_RESOURCE_AUDIT, false)
> -        .is_ok();
> +
> +    let allow_all = if let Some(view_filter) = &view_filter {
> +        user_info.check_privs(&auth_id, &["view", view_filter], PRIV_RESOURCE_AUDIT, false)?;
> +        false
> +    } else {
> +        user_info
> +            .check_privs(&auth_id, &["resources"], PRIV_RESOURCE_AUDIT, false)
> +            .is_ok()
> +    };
> +
> +    let view_filter = view_filter
> +        .map(|filter_name| views::view_filter::get_view_filter(&filter_name))
> +        .transpose()?;
>   
>       let check_priv = |remote_name: &str| -> bool {
>           user_info
> @@ -590,35 +605,62 @@ pub async fn get_subscription_status(
>       };
>   
>       for (remote_name, remote) in remotes_config {
> -        if !allow_all && !check_priv(&remote_name) {
> +        if let Some(filter) = &view_filter {
> +            if filter.can_skip_remote(&remote_name) {
> +                continue;
> +            }
> +        } else if !allow_all && !check_priv(&remote_name) {
>               continue;
>           }
>   
> +        let view_filter_clone = view_filter.clone();

this could just be named 'view_filter' too, no need to postfix it with 
'_clone'

> +
>           let future = async move {
>               let (node_status, error) =
>                   match get_subscription_info_for_remote(&remote, max_age).await {
> -                    Ok(node_status) => (Some(node_status), None),
> +                    Ok(mut node_status) => {
> +                        node_status.retain(|node, _| {
> +                            if let Some(filter) = &view_filter_clone {
> +                                filter.is_node_included(&remote.id, node)
> +                            } else {
> +                                true
> +                            }
> +                        });
> +                        (Some(node_status), None)
> +                    }
>                       Err(error) => (None, Some(error.to_string())),
>                   };
>   
> -            let mut state = RemoteSubscriptionState::Unknown;
> +            let state = if let Some(node_status) = &node_status {
> +                if error.is_some() && view_filter_clone.is_some() {
> +                    // Don't leak the existence of failed remotes, since we cannot apply
> +                    // view-filters here.

why not? we can check if the remote should be included, that does not 
requires any more info?


> +                    return None;
> +                }
>   
> -            if let Some(node_status) = &node_status {
> -                state = map_node_subscription_list_to_state(node_status);
> -            }
> +                if node_status.is_empty() {
> +                    return None;
> +                }
>   
> -            RemoteSubscriptions {
> +                map_node_subscription_list_to_state(node_status)
> +            } else {
> +                RemoteSubscriptionState::Unknown
> +            };
> +
> +            Some(RemoteSubscriptions {
>                   remote: remote_name,
>                   error,
>                   state,
>                   node_status: if verbose { node_status } else { None },
> -            }
> +            })
>           };
>   
>           futures.push(future);
>       }
>   
> -    Ok(join_all(futures).await)
> +    let status = join_all(futures).await.into_iter().flatten().collect();
> +
> +    Ok(status)
>   }
>   
>   // FIXME: make timeframe and count parameters?



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