Re: [pdm-devel] [PATCH datacenter-manager 05/13] views: add implementation for view filters

["Shannon Sterz" <s.sterz@proxmox.com>]

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 <l.wagner@proxmox.com>
> ---
>  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<ViewFilter, Error> {
> +    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<ViewId, HashMap<ResourceId, bool>> 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