Re: [pdm-devel] [PATCH datacenter-manager v3 08/11] api: subscription status: add support for view parameter

["Michael Köppl" <m.koeppl@proxmox.com>]

1 comment inline

On Thu Nov 6, 2025 at 2:43 PM CET, Lukas Wagner wrote:
> A view allows one to get filtered subset of all resources, based on
> filter rules defined in a config file. Views integrate with the
> permission system - if a user has permissions on /view/{view-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.
>
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
>
> Notes:
>     Changes since v2:
>       - make sure to not filter out a remote if it has been explicitly
>         included
>
>  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 143335fe..2930da8b 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: {
> +                schema: VIEW_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: Option<String>,
>      rpcenv: &mut dyn RpcEnvironment,
>  ) -> Result<Vec<RemoteSubscriptions>, Error> {
>      let (remotes_config, _) = pdm_config::remotes::config()?;
> @@ -574,9 +579,17 @@ 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, &["resource"], PRIV_RESOURCE_AUDIT, false)
> -        .is_ok();
> +
> +    let allow_all = if let Some(view) = &view {
> +        user_info.check_privs(&auth_id, &["view", view], PRIV_RESOURCE_AUDIT, false)?;
> +        false
> +    } else {
> +        user_info
> +            .check_privs(&auth_id, &["resource"], PRIV_RESOURCE_AUDIT, false)
> +            .is_ok()
> +    };
> +
> +    let view = views::get_optional_view(view.as_deref())?;
>  
>      let check_priv = |remote_name: &str| -> bool {
>          user_info
> @@ -590,35 +603,64 @@ pub async fn get_subscription_status(
>      };
>  
>      for (remote_name, remote) in remotes_config {
> -        if !allow_all && !check_priv(&remote_name) {
> +        if let Some(view) = &view {
> +            if view.can_skip_remote(&remote_name) {
> +                continue;
> +            }
> +        } else if !allow_all && !check_priv(&remote_name) {
>              continue;
>          }
>  
> +        let view = view.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(view) = &view {
> +                                view.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 let Some(view) = view {
> +                    if error.is_some() && !view.is_remote_explicitly_included(&remote.id) {

Can this ever be be true? From the code above, you can never have the
case that node_status and error are both Some(...) at the same time, so
error.is_some() would always evaluate to false here, unless I'm missing
something.

I think you'd have to put the check outside of this block.

    if let Some(view) = &view {
        if error.is_some() && !view.is_remote_explicitly_included(&remote.id) {
            return None;
        }
    }

    let state = if let Some(node_status) = &node_status {
        ...
    }

> +                        // Don't leak the existence of failed remotes unless they were explicitly
> +                        // pulled in by a `include remote:<id>` rule.
> +                        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