* [pdm-devel] help
[not found] <mailman.3.1761908401.29724.pdm-devel@lists.proxmox.com>
@ 2025-10-31 15:19 ` David Kinyua
0 siblings, 0 replies; only message in thread
From: David Kinyua @ 2025-10-31 15:19 UTC (permalink / raw)
To: pdm-devel
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
^ permalink raw reply [flat|nested] only message in thread