From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 45AFD1FF183 for ; Wed, 5 Nov 2025 15:10:43 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 96B6B25DF7; Wed, 5 Nov 2025 15:11:22 +0100 (CET) Mime-Version: 1.0 Date: Wed, 05 Nov 2025 15:11:18 +0100 Message-Id: From: "Lukas Wagner" To: "Dominik Csapak" , "Proxmox Datacenter Manager development discussion" , "Lukas Wagner" X-Mailer: aerc 0.21.0-0-g5549850facc2-dirty References: <20251103123521.266258-1-l.wagner@proxmox.com> <20251103123521.266258-10-l.wagner@proxmox.com> In-Reply-To: X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1762351860069 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.027 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pdm-devel] [PATCH datacenter-manager v2 09/12] api: subscription status: add support for view-filter parameter X-BeenThere: pdm-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Datacenter Manager development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Datacenter Manager development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pdm-devel-bounces@lists.proxmox.com Sender: "pdm-devel" On Wed Nov 5, 2025 at 11:08 AM CET, Dominik Csapak wrote: >> @@ -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' Ack - will do for v3. > >> + >> 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? > > The problem is that we cannot always reliably tell if a remote is included or not without checking the remote's resources. There is ViewFilter::can_skip_remote - but since that one only checks `include/exclude remote:...` rules, it might return false (meaning, the remote must be considered) even if at the end the data from this remote might be filtered out due to the other rules (and if no resources remain, the remote as a whole would then be filtered out from the response - similar to what I did in patch 6 to which you replied). Example: Consider only having `include tag:sometag` in the config. From this alone, we cannot exclude any remotes from being queried a priori and must do *all* filtering when we already have the data. Similar thing here. If we cannot reach the remote, then we don't have a list of nodes for this remote. Without the list of nodes, we cannot do any checks with the node resource. This means if we do not filter out the failed remote, we would leak its existence in a view which potentially would not match on any of the node's resources. I hope you could follow this explanation. Do you maybe have an idea on how to fix this? Only thing that comes to mind is making the include/exclude remote:{remote} rules required, so that a view *always* needs to specify which remotes to (not) consider. If we make these required, we probably should make these a separate key in the config file again. >> + 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