From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 6FAA01FF17E for ; Thu, 30 Oct 2025 12:44:10 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 08B831F91D; Thu, 30 Oct 2025 12:44:44 +0100 (CET) Mime-Version: 1.0 Date: Thu, 30 Oct 2025 12:44:39 +0100 Message-Id: To: "Lukas Wagner" X-Mailer: aerc 0.20.0 References: <20251029144902.446852-1-l.wagner@proxmox.com> <20251029144902.446852-6-l.wagner@proxmox.com> In-Reply-To: <20251029144902.446852-6-l.wagner@proxmox.com> From: "Shannon Sterz" X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1761824665990 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.060 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_MSPIKE_H2 0.001 Average reputation (+2) 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 05/13] views: add implementation for view filters 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 Cc: 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 Oct 29, 2025 at 3:48 PM CET, Lukas Wagner wrote: > This commit adds the filter implementation for the previously defined > ViewFilterConfig type. > > There are include/exclude rules for the following properties: > - (global) resource-id > - resource pool > - resource type > - remote > - tags > > The rules are interpreted as follows: > - no rules: everything matches > - only includes: included resources match > - only excluded: everything *but* the excluded resources match > - include and exclude: excludes are applied *after* includes, meaning if > one has a `include-remote foo` and `exclude-remote foo` at the same > time, the remote `foo` will never match > > Signed-off-by: Lukas Wagner > --- > server/src/lib.rs | 1 + > server/src/views/mod.rs | 1 + > server/src/views/view_filter.rs | 225 ++++++++++++++++++++++++++++++++ > 3 files changed, 227 insertions(+) > create mode 100644 server/src/views/mod.rs > create mode 100644 server/src/views/view_filter.rs > > diff --git a/server/src/lib.rs b/server/src/lib.rs > index 964807eb..0f25aa71 100644 > --- a/server/src/lib.rs > +++ b/server/src/lib.rs > @@ -12,6 +12,7 @@ pub mod remote_tasks; > pub mod remote_updates; > pub mod resource_cache; > pub mod task_utils; > +pub mod views; > > pub mod connection; > pub mod pbs_client; > diff --git a/server/src/views/mod.rs b/server/src/views/mod.rs > new file mode 100644 > index 00000000..9a2856a4 > --- /dev/null > +++ b/server/src/views/mod.rs > @@ -0,0 +1 @@ > +pub mod view_filter; > diff --git a/server/src/views/view_filter.rs b/server/src/views/view_filter.rs > new file mode 100644 > index 00000000..656b5523 > --- /dev/null > +++ b/server/src/views/view_filter.rs > @@ -0,0 +1,225 @@ > +use anyhow::Error; > + > +use pdm_api_types::{ > + resource::{Resource, ResourceType}, > + views::ViewFilterConfig, > +}; > + > +/// Get view filter with a given ID. > +/// > +/// Returns an error if the view filter configuration file could not be read, or > +/// if the view filter with the provided ID does not exist. > +pub fn get_view_filter(filter_id: &str) -> Result { > + pdm_config::views::get_view_filter_config(filter_id).map(ViewFilter::new) > +} > + > +/// View filter implementation. > +/// > +/// Given a [`ViewFilterConfig`], this struct can be used to check if a resource/remote/node > +/// matches the filter rules. > +#[derive(Clone)] > +pub struct ViewFilter { > + config: ViewFilterConfig, > +} > + > +impl ViewFilter { > + /// Create a new [`ViewFiler`]. > + pub fn new(config: ViewFilterConfig) -> Self { > + Self { config } > + } > + > + /// Check if a [`Resource`] matches the filter rules. > + pub fn resource_matches(&self, remote: &str, resource: &Resource) -> bool { > + // NOTE: Establishing a cache here is not worth the effort at the moment, evaluation of > + // rules is *very* fast. > + // > + // Some experiments were performed with a cache that works roughly as following: > + // - HashMap> in a mutex > + // - Cache invalidated if view-filter config digest changed > + // - Cache invalidated if certain resource fields such as tags or resource pools change > + // from the last time (also with a digest-based implementation) > + // > + // Experimented with the `fake-remote` feature and and 15000 guests showed that > + // caching was only faster than direct evaluation if the number of rules in the > + // ViewFilterConfig is *huge* (e.g. >1000 `include-resource-id` entries). But even for those, > + // direct evaluation was always plenty fast, with evaluation times ~20ms for *all* resources. > + // > + // -> for any *realistic* filter config, we should be good with direct evaluation, as long > + // as we don't add any filter rules which are very expensive to evaluate. > + > + let resource_data = resource.into(); > + > + self.check_if_included(remote, &resource_data) > + && !self.check_if_excluded(remote, &resource_data) > + } > + > + /// Check if a remote can be safely skipped based on the filter rule definition. > + /// > + /// When there are `include-remote` or `exclude-remote` rules, we can use these to > + /// check if a remote needs to be considered at all. > + pub fn can_skip_remote(&self, remote: &str) -> bool { > + let no_includes = self.config.include_remote.is_empty(); > + let any_include = self.config.include_remote.iter().any(|r| r == remote); > + let any_exclude = self.config.exclude_remote.iter().any(|r| r == remote); > + > + (!no_includes && !any_include) || any_exclude > + } > + > + /// Check if a node is matched by the filter rules. > + /// > + /// This is equivalent to checking an actual node resource. > + pub fn is_node_included(&self, remote: &str, node: &str) -> bool { > + let resource_data = ResourceData { > + resource_type: ResourceType::Node, > + tags: None, > + resource_pool: None, > + resource_id: &format!("remote/{remote}/node/{node}"), > + }; > + > + self.check_if_included(remote, &resource_data) > + && !self.check_if_excluded(remote, &resource_data) > + } > + > + /// Returns the name of the view filter. > + pub fn name(&self) -> &str { > + &self.config.id > + } > + > + fn check_if_included(&self, remote: &str, resource: &ResourceData) -> bool { > + let rules = Rules { > + ruleset_type: RulesetType::Include, > + tags: &self.config.include_tag, > + resource_ids: &self.config.include_resource_id, > + resource_type: &self.config.include_resource_type, > + resource_pools: &self.config.include_resource_pool, > + remotes: &self.config.include_remote, > + }; > + > + check_rules(rules, remote, resource) > + } > + > + fn check_if_excluded(&self, remote: &str, resource: &ResourceData) -> bool { > + let rules = Rules { > + ruleset_type: RulesetType::Exclude, > + tags: &self.config.exclude_tag, > + resource_ids: &self.config.exclude_resource_id, > + resource_type: &self.config.exclude_resource_type, > + resource_pools: &self.config.exclude_resource_pool, > + remotes: &self.config.exclude_remote, > + }; > + > + check_rules(rules, remote, resource) > + } > +} > + > +enum RulesetType { > + Include, > + Exclude, > +} > + > +struct Rules<'a> { > + ruleset_type: RulesetType, > + tags: &'a [String], > + resource_ids: &'a [String], > + resource_pools: &'a [String], > + resource_type: &'a [ResourceType], > + remotes: &'a [String], > +} > + > +struct ResourceData<'a> { > + resource_type: ResourceType, > + tags: Option<&'a [String]>, > + resource_pool: Option<&'a String>, > + resource_id: &'a str, > +} > + > +impl<'a> From<&'a Resource> for ResourceData<'a> { > + fn from(value: &'a Resource) -> Self { > + match value { > + Resource::PveStorage(_) => ResourceData { > + resource_type: value.resource_type(), > + tags: None, > + resource_pool: None, > + resource_id: value.global_id(), > + }, > + Resource::PveQemu(resource) => ResourceData { > + resource_type: value.resource_type(), > + tags: Some(&resource.tags), > + resource_pool: Some(&resource.pool), > + resource_id: value.global_id(), > + }, > + Resource::PveLxc(resource) => ResourceData { > + resource_type: value.resource_type(), > + tags: Some(&resource.tags), > + resource_pool: Some(&resource.pool), > + resource_id: value.global_id(), > + }, > + Resource::PveNode(_) => ResourceData { > + resource_type: value.resource_type(), > + tags: None, > + resource_pool: None, > + resource_id: value.global_id(), > + }, > + Resource::PveSdn(_) => ResourceData { > + resource_type: value.resource_type(), > + tags: None, > + resource_pool: None, > + resource_id: value.global_id(), > + }, > + Resource::PbsNode(_) => ResourceData { > + resource_type: value.resource_type(), > + tags: None, > + resource_pool: None, > + resource_id: value.global_id(), > + }, > + Resource::PbsDatastore(_) => ResourceData { > + resource_type: value.resource_type(), > + tags: None, > + resource_pool: None, > + resource_id: value.global_id(), > + }, > + } > + } > +} nit: imo this would be fine to group this a match statement a bit. i.e. PveStorage, PveNode, PveSdn, PbsNode and PbsDatastore have essentially the same arm here. could be simply or-ed (|) > + > +fn check_rules(rules: Rules, remote: &str, resource: &ResourceData) -> bool { > + let has_any_rules = !rules.tags.is_empty() > + || !rules.remotes.is_empty() > + || !rules.resource_pools.is_empty() > + || !rules.resource_type.is_empty() > + || !rules.resource_ids.is_empty(); > + > + if !has_any_rules { > + return matches!(rules.ruleset_type, RulesetType::Include); > + } > + > + if let Some(tags) = resource.tags { > + if rules.tags.iter().any(|tag| tags.contains(tag)) { > + return true; > + } > + } > + > + if let Some(pool) = resource.resource_pool { > + if rules.resource_pools.iter().any(|p| p == pool) { > + return true; > + } > + } > + > + if rules.remotes.iter().any(|r| r == remote) { > + return true; > + } > + > + if rules.resource_ids.iter().any(|r| r == resource.resource_id) { > + return true; > + } > + > + if rules > + .resource_type > + .iter() > + .any(|ty| *ty == resource.resource_type) > + { > + return true; > + } > + > + false > +} _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel