[pdm-devel] help

[pdm-devel] help

[David Kinyua]

unsubscribe
        

-----Original Message-----
From: pdm-devel <pdm-devel-bounces@lists.proxmox.com> On Behalf Of pdm-devel-request@lists.proxmox.com
Sent: Friday, 31 October 2025 1:00 PM
To: pdm-devel@lists.proxmox.com
Subject: pdm-devel Digest, Vol 11, Issue 124

Send pdm-devel mailing list submissions to
	pdm-devel@lists.proxmox.com

To subscribe or unsubscribe via the World Wide Web, visit
	https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
or, via email, send a message with subject or body 'help' to
	pdm-devel-request@lists.proxmox.com

You can reach the person managing the list at
	pdm-devel-owner@lists.proxmox.com

When replying, please edit your Subject line so it is more specific than "Re: Contents of pdm-devel digest..."


Today's Topics:

   1. Re: [PATCH datacenter-manager 10/13] api: subscription
      status: add support for view-filter parameter (Lukas Wagner)


----------------------------------------------------------------------

Message: 1
Date: Fri, 31 Oct 2025 11:38:54 +0100
From: "Lukas Wagner" <l.wagner@proxmox.com>
To: "Shannon Sterz" <s.sterz@proxmox.com>, "Lukas Wagner"
	<l.wagner@proxmox.com>
Cc: "Proxmox Datacenter Manager development discussion"
	<pdm-devel@lists.proxmox.com>
Subject: Re: [pdm-devel] [PATCH datacenter-manager 10/13] api:
	subscription status: add support for view-filter parameter
Message-ID: <DDWFYBDGMXG5.1L3RHJC7YWT9F@proxmox.com>
Content-Type: text/plain; charset=UTF-8

On Thu Oct 30, 2025 at 12:44 PM CET, Shannon Sterz wrote:
>> @@ -560,6 +564,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()?; @@ 
>> -572,6 +577,14 @@ pub async fn get_subscription_status(
>>          .check_privs(&auth_id, &["resources"], PRIV_RESOURCE_AUDIT, false)
>>          .is_ok();
>>
>> +    if let Some(view_filter) = &view_filter {
>> +        user_info.check_privs(&auth_id, &["view", view_filter], PRIV_RESOURCE_AUDIT, false)?;
>> +    }
>
> this is minor, but maybe consider moving the view check before the 
> `allow_all` check and skipping the `allow_all` check (by setting it to
> false) if a view was found. that should safe us an unecessary 
> traversal of the acl tree here.
>
> i think wolfgang already noted that tho.
>

ack, will be fixed for the next iteration.

>> +
>> +    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
>>              .check_privs(
>> @@ -584,35 +597,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();
>> +
>>          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() {
>> +                    // Don't leak the existence of failed remotes, since we cannot apply
>> +                    // view-filters here.
>> +                    return None;
>
> shouldn't this be gated by checking if view_filters was defined?
> otherwise we now return nothing every time we get an error?
>
> also correct me if i'm wrong, but isn't this `return None;` unreachable?
> error is assigned `Some` iff the `get_subscription_status` call above 
> returns an `Err`, but in that case `node_status` is set to `None`, so 
> the `if let Some(node_status) = &node_status` above should be false?
>

Excellent catch! You are absolutely right, I think I can just drop the if-clause completely. Thanks a lot!

>> +                }
>>
>> -            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?





------------------------------

Subject: Digest Footer

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


------------------------------

End of pdm-devel Digest, Vol 11, Issue 124
******************************************


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