[pdm-devel] [PATCH datacenter-manager v5 00/11] backend implementation for view filters

[pdm-devel] [PATCH datacenter-manager v5 00/11] backend implementation for view filters

[Lukas Wagner]

Key aspects:
  - new config file at /etc/proxmox-datacenter-manager/views.cfg
  - ViewConfig as a definition type, has
     - {include,exclude} {remote,resource-id,resource-type,resource-pool,tag}:{value}

  - View resource filter implementation & big suite of unit tests
    - excludes are processed after includes
    - if no include rules are defined, all resources but those which were
      excluded are matched
    - if no rules are defined in a filter, everything is matched

  - Added the 'view' parameter to a couple of API endpoints
    - /resources/list
    - /resources/status
    - /resources/subscription
    - /resources/top-entities
    - /remote-tasks/list
    - /remote-tasks/statistis

  - ACL checks are done on /view/{view-filter-id} for now, replace
    any other permission check in the handler iff the view-filter paramter
    is set

Left to do:
  - CRUD for filter definition
  - UI for filter rules

Changes since v4:
  - Rebased onto master
  - Added regex to verify resource IDs

Changes since v3:
  - Incorporate minor review feedback from @Shannon and @Michael - thanks!

Changes since v2 (thx for the review @Dominik):

- Renamed:
  - ViewFilter -> View
  - ViewFilterConfig -> ViewConfig
  - 'view-filter' parameters to 'view'

- Use ApiSectionDataEntry trait for ViewConfig
- Use SAFE_ID_FORMAT to validate filter values
- Changed path for config file from `views/filter.cfg` to just `views.cfg`

- Include failed remotes in API responses iff they have been explicitly included
via `include remote:<...>`. Previously, they had been filtered out to avoid
leaking the existence of remotes, but if they have been explicitly included,
this is fine.

- Simplify `check_rules` function by using `.any()` instead of looping manually

- Merged the rule implementation and test commits

- Added views::get_optional_view as a convenience helper. This one is similar to
views::get_view, but accepts Option<&str> and returns Option<View>.

Changes since v1 (RFC):
  - Change config key structure, moving the type into the value
    e.g. 
      include-remote foo
    became
      include remote:foo

  - Minor fixes from the review (thanks Wolfgang & Shannon)


proxmox-datacenter-manager:

Lukas Wagner (11):
  pdm-api-types: views: add ViewConfig type
  pdm-config: views: add support for views
  acl: add '/view' and '/view/{view-id}' as allowed ACL paths
  views: add implementation for view resource filtering
  api: resources: list: add support for view parameter
  api: resources: top entities: add support for view parameter
  api: resources: status: add support for view parameter
  api: subscription status: add support for view parameter
  api: remote-tasks: add support for view parameter
  pdm-client: resource list: add view parameter
  pdm-client: top entities: add view parameter

 cli/client/src/resources.rs                  |   2 +-
 lib/pdm-api-types/src/lib.rs                 |   8 +
 lib/pdm-api-types/src/views.rs               | 210 +++++++
 lib/pdm-client/src/lib.rs                    |  19 +-
 lib/pdm-config/src/lib.rs                    |   2 +-
 lib/pdm-config/src/views.rs                  |  17 +
 server/src/acl.rs                            |   6 +
 server/src/api/remote_tasks.rs               |  36 +-
 server/src/api/resources.rs                  | 160 ++++-
 server/src/lib.rs                            |   1 +
 server/src/metric_collection/top_entities.rs |   5 +
 server/src/remote_tasks/mod.rs               |  67 +-
 server/src/resource_cache.rs                 |   3 +-
 server/src/views/mod.rs                      | 205 ++++++
 server/src/views/tests.rs                    | 619 +++++++++++++++++++
 ui/src/dashboard/view.rs                     |   2 +-
 ui/src/sdn/zone_tree.rs                      |   2 +-
 17 files changed, 1304 insertions(+), 60 deletions(-)
 create mode 100644 lib/pdm-api-types/src/views.rs
 create mode 100644 lib/pdm-config/src/views.rs
 create mode 100644 server/src/views/mod.rs
 create mode 100644 server/src/views/tests.rs


Summary over all repositories:
  17 files changed, 1304 insertions(+), 60 deletions(-)

-- 
Generated by murpp 0.9.0


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

[pdm-devel] [PATCH datacenter-manager v5 01/11] pdm-api-types: views: add ViewConfig type

[Lukas Wagner]

This type will be used to define views.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>
---

Notes:
    Changes since v2:
      - use ApiSectionDataEntry, as suggested by Dominik
      - Renamed ViewFilter* -> View*
      - Use SAFE_ID_FORMAT to check filter values where possible
    
    Changes since RFC:
      - Change config structure, instead of 'include-{x} value' we now do
        'include {x}:value'

 lib/pdm-api-types/src/lib.rs   |   8 ++
 lib/pdm-api-types/src/views.rs | 210 +++++++++++++++++++++++++++++++++
 2 files changed, 218 insertions(+)
 create mode 100644 lib/pdm-api-types/src/views.rs

diff --git a/lib/pdm-api-types/src/lib.rs b/lib/pdm-api-types/src/lib.rs
index 5ac99aa1..67a1a55f 100644
--- a/lib/pdm-api-types/src/lib.rs
+++ b/lib/pdm-api-types/src/lib.rs
@@ -107,6 +107,8 @@ pub mod subscription;
 
 pub mod sdn;
 
+pub mod views;
+
 const_regex! {
     // just a rough check - dummy acceptor is used before persisting
     pub OPENSSL_CIPHERS_REGEX = r"^[0-9A-Za-z_:, +!\-@=.]+$";
@@ -154,6 +156,12 @@ pub const REALM_ID_SCHEMA: Schema = StringSchema::new("Realm name.")
     .max_length(32)
     .schema();
 
+pub const VIEW_ID_SCHEMA: Schema = StringSchema::new("View name.")
+    .format(&PROXMOX_SAFE_ID_FORMAT)
+    .min_length(2)
+    .max_length(32)
+    .schema();
+
 pub const VMID_SCHEMA: Schema = IntegerSchema::new("A guest ID").minimum(1).schema();
 pub const SNAPSHOT_NAME_SCHEMA: Schema = StringSchema::new("The name of the snapshot")
     .format(&PROXMOX_SAFE_ID_FORMAT)
diff --git a/lib/pdm-api-types/src/views.rs b/lib/pdm-api-types/src/views.rs
new file mode 100644
index 00000000..ef39cc62
--- /dev/null
+++ b/lib/pdm-api-types/src/views.rs
@@ -0,0 +1,210 @@
+use std::{fmt::Display, str::FromStr, sync::OnceLock};
+
+use anyhow::bail;
+use const_format::concatcp;
+use serde::{Deserialize, Serialize};
+
+use proxmox_schema::{
+    api, api_types::SAFE_ID_REGEX_STR, const_regex, ApiStringFormat, ApiType, ArraySchema, Schema,
+    StringSchema, Updater,
+};
+use proxmox_section_config::{typed::ApiSectionDataEntry, SectionConfig, SectionConfigPlugin};
+
+use crate::{
+    remotes::REMOTE_ID_SCHEMA, resource::ResourceType, PROXMOX_SAFE_ID_REGEX, VIEW_ID_SCHEMA,
+};
+
+const_regex! {
+    /// Regex for matching global resource IDs
+    pub GLOBAL_RESOURCE_ID_REGEX = concatcp!(r"^", SAFE_ID_REGEX_STR, r"(\/", SAFE_ID_REGEX_STR, r")+$");
+}
+
+/// Schema for filter rules.
+pub const FILTER_RULE_SCHEMA: Schema = StringSchema::new("Filter rule for resources.")
+    .format(&ApiStringFormat::VerifyFn(verify_filter_rule))
+    .type_text(
+        "resource-type:<storage|qemu|lxc|sdn-zone|datastore|node>\
+            |resource-pool:<pool-name>\
+            |tag:<tag-name>\
+            |remote:<remote-name>\
+            |resource-id:<resource-id>",
+    )
+    .schema();
+
+/// Schema for list of filter rules.
+pub const FILTER_RULE_LIST_SCHEMA: Schema =
+    ArraySchema::new("List of filter rules.", &FILTER_RULE_SCHEMA).schema();
+
+#[api(
+    properties: {
+        "id": {
+            schema: VIEW_ID_SCHEMA,
+        },
+        "include": {
+            schema: FILTER_RULE_LIST_SCHEMA,
+            optional: true,
+        },
+        "exclude": {
+            schema: FILTER_RULE_LIST_SCHEMA,
+            optional: true,
+        }
+    }
+)]
+#[derive(Clone, Debug, Default, Deserialize, Serialize, Updater, PartialEq)]
+#[serde(rename_all = "kebab-case")]
+/// View definition
+pub struct ViewConfig {
+    /// View name.
+    #[updater(skip)]
+    pub id: String,
+
+    /// List of includes.
+    #[serde(default, skip_serializing_if = "Vec::is_empty")]
+    #[updater(serde(skip_serializing_if = "Option::is_none"))]
+    pub include: Vec<FilterRule>,
+
+    /// List of excludes.
+    #[serde(default, skip_serializing_if = "Vec::is_empty")]
+    #[updater(serde(skip_serializing_if = "Option::is_none"))]
+    pub exclude: Vec<FilterRule>,
+}
+
+#[derive(Clone, Debug, Deserialize, Serialize, PartialEq)]
+#[serde(rename_all = "kebab-case")]
+/// Enum for the different sections in the 'views.cfg' file.
+pub enum ViewConfigEntry {
+    /// 'view' section
+    View(ViewConfig),
+}
+
+const VIEW_SECTION_NAME: &str = "view";
+
+impl ApiSectionDataEntry for ViewConfigEntry {
+    fn section_config() -> &'static SectionConfig {
+        static CONFIG: OnceLock<SectionConfig> = OnceLock::new();
+
+        CONFIG.get_or_init(|| {
+            let mut this = SectionConfig::new(&VIEW_ID_SCHEMA);
+
+            this.register_plugin(SectionConfigPlugin::new(
+                VIEW_SECTION_NAME.into(),
+                Some("id".to_string()),
+                ViewConfig::API_SCHEMA.unwrap_object_schema(),
+            ));
+            this
+        })
+    }
+
+    fn section_type(&self) -> &'static str {
+        match self {
+            ViewConfigEntry::View(_) => VIEW_SECTION_NAME,
+        }
+    }
+}
+
+#[derive(Clone, Debug, PartialEq)]
+/// Filter rule for includes/excludes.
+pub enum FilterRule {
+    /// Match a resource type.
+    ResourceType(ResourceType),
+    /// Match a resource pools (for PVE guests).
+    ResourcePool(String),
+    /// Match a (global) resource ID, e.g. 'remote/<remote>/guest/<vmid>'.
+    ResourceId(String),
+    /// Match a tag (for PVE guests).
+    Tag(String),
+    /// Match a remote.
+    Remote(String),
+}
+
+impl FromStr for FilterRule {
+    type Err = anyhow::Error;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        Ok(match s.split_once(':') {
+            Some(("resource-type", value)) => FilterRule::ResourceType(value.parse()?),
+            Some(("resource-pool", value)) => {
+                if !PROXMOX_SAFE_ID_REGEX.is_match(value) {
+                    bail!("invalid resource-pool value: {value}");
+                }
+                FilterRule::ResourcePool(value.to_string())
+            }
+            Some(("resource-id", value)) => {
+                if !GLOBAL_RESOURCE_ID_REGEX.is_match(value) {
+                    bail!("invalid resource-id value: {value}");
+                }
+
+                FilterRule::ResourceId(value.to_string())
+            }
+            Some(("tag", value)) => {
+                if !PROXMOX_SAFE_ID_REGEX.is_match(value) {
+                    bail!("invalid tag value: {value}");
+                }
+                FilterRule::Tag(value.to_string())
+            }
+            Some(("remote", value)) => {
+                let _ = REMOTE_ID_SCHEMA.parse_simple_value(value)?;
+                FilterRule::Remote(value.to_string())
+            }
+            Some((ty, _)) => bail!("invalid type: {ty}"),
+            None => bail!("invalid filter rule: {s}"),
+        })
+    }
+}
+
+// used for serializing below, caution!
+impl Display for FilterRule {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            FilterRule::ResourceType(resource_type) => write!(f, "resource-type:{resource_type}"),
+            FilterRule::ResourcePool(pool) => write!(f, "resource-pool:{pool}"),
+            FilterRule::ResourceId(id) => write!(f, "resource-id:{id}"),
+            FilterRule::Tag(tag) => write!(f, "tag:{tag}"),
+            FilterRule::Remote(remote) => write!(f, "remote:{remote}"),
+        }
+    }
+}
+
+proxmox_serde::forward_deserialize_to_from_str!(FilterRule);
+proxmox_serde::forward_serialize_to_display!(FilterRule);
+
+fn verify_filter_rule(input: &str) -> Result<(), anyhow::Error> {
+    FilterRule::from_str(input).map(|_| ())
+}
+
+#[cfg(test)]
+mod test {
+    use anyhow::Error;
+
+    use crate::views::FilterRule;
+
+    fn parse_and_check_display(input: &str) -> Result<bool, Error> {
+        let rule: FilterRule = input.parse()?;
+
+        Ok(input == format!("{rule}"))
+    }
+
+    #[test]
+    fn test_filter_rule() {
+        assert!(parse_and_check_display("abc").is_err());
+        assert!(parse_and_check_display("abc:").is_err());
+
+        assert!(parse_and_check_display("resource-type:").is_err());
+        assert!(parse_and_check_display("resource-type:lxc").unwrap());
+        assert!(parse_and_check_display("resource-type:qemu").unwrap());
+        assert!(parse_and_check_display("resource-type:abc").is_err());
+
+        assert!(parse_and_check_display("resource-pool:").is_err());
+        assert!(parse_and_check_display("resource-pool:somepool").unwrap());
+
+        assert!(parse_and_check_display("resource-id:").is_err());
+        assert!(parse_and_check_display("resource-id:remote/someremote/guest/100").unwrap());
+        assert!(parse_and_check_display("resource-id:remote").is_err());
+
+        assert!(parse_and_check_display("tag:").is_err());
+        assert!(parse_and_check_display("tag:sometag").unwrap());
+
+        assert!(parse_and_check_display("remote:someremote").unwrap());
+        assert!(parse_and_check_display("remote:a").is_err());
+    }
+}
-- 
2.47.3



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

[pdm-devel] [PATCH datacenter-manager v5 02/11] pdm-config: views: add support for views

[Lukas Wagner]

This allows to read ViewConfig entries from a new config file at
/etc/proxmox-datacenter-manager/views.cfg

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>
---

Notes:
    Changes since v2:
      - Change config path to 'views.cfg' instead of 'views/filters.cfg'

 lib/pdm-config/src/lib.rs   |  2 +-
 lib/pdm-config/src/views.rs | 17 +++++++++++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)
 create mode 100644 lib/pdm-config/src/views.rs

diff --git a/lib/pdm-config/src/lib.rs b/lib/pdm-config/src/lib.rs
index ac398cab..4c490541 100644
--- a/lib/pdm-config/src/lib.rs
+++ b/lib/pdm-config/src/lib.rs
@@ -1,6 +1,5 @@
 use anyhow::{format_err, Error};
 use nix::unistd::{Gid, Group, Uid, User};
-
 pub use pdm_buildcfg::{BACKUP_GROUP_NAME, BACKUP_USER_NAME};
 
 pub mod certificate_config;
@@ -8,6 +7,7 @@ pub mod domains;
 pub mod node;
 pub mod remotes;
 pub mod setup;
+pub mod views;
 
 mod config_version_cache;
 pub use config_version_cache::ConfigVersionCache;
diff --git a/lib/pdm-config/src/views.rs b/lib/pdm-config/src/views.rs
new file mode 100644
index 00000000..59c02a66
--- /dev/null
+++ b/lib/pdm-config/src/views.rs
@@ -0,0 +1,17 @@
+use anyhow::Error;
+
+use proxmox_section_config::typed::{ApiSectionDataEntry, SectionConfigData};
+
+use pdm_api_types::views::ViewConfigEntry;
+
+use pdm_buildcfg::configdir;
+
+const VIEW_CFG_FILENAME: &str = configdir!("/views.cfg");
+
+/// Get the `views.cfg` config file contents.
+pub fn config() -> Result<SectionConfigData<ViewConfigEntry>, Error> {
+    let content =
+        proxmox_sys::fs::file_read_optional_string(VIEW_CFG_FILENAME)?.unwrap_or_default();
+
+    ViewConfigEntry::parse_section_config(VIEW_CFG_FILENAME, &content)
+}
-- 
2.47.3



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

[pdm-devel] [PATCH datacenter-manager v5 03/11] acl: add '/view' and '/view/{view-id}' as allowed ACL paths

[Lukas Wagner]

These paths will be used for ACL objects for views. A view has filter
rules that specify which resources/remotes are included in the view. If
a user has permissions on the corresponding ACL object for the view,
then the privileges are transitively applied to the included resources
as well.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>
---
 server/src/acl.rs | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/server/src/acl.rs b/server/src/acl.rs
index 52a1f972..f5f57c03 100644
--- a/server/src/acl.rs
+++ b/server/src/acl.rs
@@ -150,6 +150,12 @@ impl proxmox_access_control::init::AccessControlConfig for AccessControlConfig {
                     _ => {}
                 }
             }
+            "view" => {
+                // `/view` and `/view/{view-id}`
+                if components_len <= 2 {
+                    return Ok(());
+                }
+            }
             _ => {}
         }
 
-- 
2.47.3



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

[pdm-devel] [PATCH datacenter-manager v5 04/11] views: add implementation for view resource filtering

[Lukas Wagner]

This commit adds the resource filter implementation for the previously
defined ViewConfig 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

Some experiments were performed with a cache that works roughly as following:
  - HashMap<ViewId, HashMap<ResourceId, bool>> in a mutex
  - Cache invalidated if view 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 ViewConfig 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* view config, we should be good with direct
evaluation, as long as we don't add any filter rules which are very
expensive to evaluate.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>
---

Notes:
    Changes since v2:
      - add is_remote_explicitly_included helper
      - merge commit that added tests into this one
      - simply `check_rules` using .any on the rules vec
      - add get_optional_view helper
      - rename ViewFilter* -> View*

 server/src/lib.rs         |   1 +
 server/src/views/mod.rs   | 205 +++++++++++++
 server/src/views/tests.rs | 619 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 825 insertions(+)
 create mode 100644 server/src/views/mod.rs
 create mode 100644 server/src/views/tests.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..b5d79e0e
--- /dev/null
+++ b/server/src/views/mod.rs
@@ -0,0 +1,205 @@
+use anyhow::{format_err, Error};
+
+use pdm_api_types::{
+    resource::{Resource, ResourceType},
+    views::{FilterRule, ViewConfig, ViewConfigEntry},
+};
+
+#[cfg(test)]
+mod tests;
+
+/// Get view with a given ID.
+///
+/// Returns an error if the view configuration file could not be read, or
+/// if the view with the provided ID does not exist.
+pub fn get_view(view_id: &str) -> Result<View, Error> {
+    let config = pdm_config::views::config()?;
+
+    let entry = config
+        .get(view_id)
+        .cloned()
+        .ok_or_else(|| format_err!("unknown view: {view_id}"))?;
+
+    match entry {
+        ViewConfigEntry::View(view_config) => Ok(View::new(view_config)),
+    }
+}
+
+/// Get (optional) view with a given ID.
+///
+/// Returns an error if the view configuration file could not be read, or
+/// if the view with the provided ID does not exist.
+pub fn get_optional_view(view_id: Option<&str>) -> Result<Option<View>, Error> {
+    view_id.map(get_view).transpose()
+}
+
+/// View implementation.
+///
+/// Given a [`ViewConfig`], this struct can be used to check if a resource/remote/node
+/// matches the include/exclude rules.
+#[derive(Clone)]
+pub struct View {
+    config: ViewConfig,
+}
+
+impl View {
+    /// Create a new [`View`].
+    pub fn new(config: ViewConfig) -> 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.
+
+        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 mut has_any_include_remote = false;
+        let mut matches_any_include_remote = false;
+
+        let mut any_other = false;
+
+        for include in &self.config.include {
+            if let FilterRule::Remote(r) = include {
+                has_any_include_remote = true;
+                if r == remote {
+                    matches_any_include_remote = true;
+                    break;
+                }
+            } else {
+                any_other = true;
+            }
+        }
+
+        let matches_any_exclude_remote = self
+            .config
+            .exclude
+            .iter()
+            .any(|rule| Self::matches_remote_rule(remote, rule));
+
+        (has_any_include_remote && !matches_any_include_remote && !any_other)
+            || matches_any_exclude_remote
+    }
+
+    /// Check if a remote is *explicitly* included (and not excluded).
+    ///
+    /// A subset of the resources of a remote might still be pulled in by other rules,
+    /// but this function check if the remote as a whole is matched.
+    pub fn is_remote_explicitly_included(&self, remote: &str) -> bool {
+        let matches_include_remote = self
+            .config
+            .include
+            .iter()
+            .any(|rule| Self::matches_remote_rule(remote, rule));
+        let matches_exclude_remote = self
+            .config
+            .exclude
+            .iter()
+            .any(|rule| Self::matches_remote_rule(remote, rule));
+
+        matches_include_remote && !matches_exclude_remote
+    }
+
+    /// 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.
+    pub fn name(&self) -> &str {
+        &self.config.id
+    }
+
+    fn check_if_included(&self, remote: &str, resource: &ResourceData) -> bool {
+        if self.config.include.is_empty() {
+            // If there are no include rules, any resource is included (unless excluded)
+            return true;
+        }
+
+        check_rules(&self.config.include, remote, resource)
+    }
+
+    fn check_if_excluded(&self, remote: &str, resource: &ResourceData) -> bool {
+        check_rules(&self.config.exclude, remote, resource)
+    }
+
+    fn matches_remote_rule(remote: &str, rule: &FilterRule) -> bool {
+        if let FilterRule::Remote(r) = rule {
+            r == remote
+        } else {
+            false
+        }
+    }
+}
+
+fn check_rules(rules: &[FilterRule], remote: &str, resource: &ResourceData) -> bool {
+    rules.iter().any(|rule| match rule {
+        FilterRule::ResourceType(resource_type) => resource.resource_type == *resource_type,
+        FilterRule::ResourcePool(pool) => resource.resource_pool == Some(pool),
+        FilterRule::ResourceId(resource_id) => resource.resource_id == resource_id,
+        FilterRule::Tag(tag) => {
+            if let Some(resource_tags) = resource.tags {
+                resource_tags.contains(tag)
+            } else {
+                false
+            }
+        }
+        FilterRule::Remote(included_remote) => included_remote == remote,
+    })
+}
+
+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::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(_)
+            | Resource::PveSdn(_)
+            | Resource::PbsNode(_)
+            | Resource::PbsDatastore(_)
+            | Resource::PveStorage(_) => ResourceData {
+                resource_type: value.resource_type(),
+                tags: None,
+                resource_pool: None,
+                resource_id: value.global_id(),
+            },
+        }
+    }
+}
diff --git a/server/src/views/tests.rs b/server/src/views/tests.rs
new file mode 100644
index 00000000..030b7994
--- /dev/null
+++ b/server/src/views/tests.rs
@@ -0,0 +1,619 @@
+use pdm_api_types::{
+    resource::{PveLxcResource, PveQemuResource, PveStorageResource, Resource, ResourceType},
+    views::{FilterRule, ViewConfig},
+};
+
+use super::View;
+
+fn make_storage_resource(remote: &str, node: &str, storage_name: &str) -> Resource {
+    Resource::PveStorage(PveStorageResource {
+        disk: 1000,
+        maxdisk: 2000,
+        id: format!("remote/{remote}/storage/{node}/{storage_name}"),
+        storage: storage_name.into(),
+        node: node.into(),
+        status: "available".into(),
+    })
+}
+
+fn make_qemu_resource(
+    remote: &str,
+    node: &str,
+    vmid: u32,
+    pool: Option<&str>,
+    tags: &[&str],
+) -> Resource {
+    Resource::PveQemu(PveQemuResource {
+        disk: 1000,
+        maxdisk: 2000,
+        id: format!("remote/{remote}/guest/{vmid}"),
+        node: node.into(),
+        status: "available".into(),
+        cpu: 0.0,
+        maxcpu: 0.0,
+        maxmem: 1024,
+        mem: 512,
+        name: format!("vm-{vmid}"),
+        // TODO: Check the API type - i guess it should be an option?
+        pool: pool.map_or_else(String::new, |a| a.into()),
+        tags: tags.iter().map(|tag| String::from(*tag)).collect(),
+        template: false,
+        uptime: 1337,
+        vmid,
+    })
+}
+
+fn make_lxc_resource(
+    remote: &str,
+    node: &str,
+    vmid: u32,
+    pool: Option<&str>,
+    tags: &[&str],
+) -> Resource {
+    Resource::PveLxc(PveLxcResource {
+        disk: 1000,
+        maxdisk: 2000,
+        id: format!("remote/{remote}/guest/{vmid}"),
+        node: node.into(),
+        status: "available".into(),
+        cpu: 0.0,
+        maxcpu: 0.0,
+        maxmem: 1024,
+        mem: 512,
+        name: format!("vm-{vmid}"),
+        // TODO: Check the API type - i guess it should be an option?
+        pool: pool.map_or_else(String::new, |a| a.into()),
+        tags: tags.iter().map(|tag| String::from(*tag)).collect(),
+        template: false,
+        uptime: 1337,
+        vmid,
+    })
+}
+
+fn run_test(config: ViewConfig, tests: &[((&str, &Resource), bool)]) {
+    let filter = View::new(config);
+
+    for ((remote_name, resource), expected) in tests {
+        eprintln!("remote: {remote_name}, resource: {resource:?}");
+        assert_eq!(filter.resource_matches(remote_name, resource), *expected);
+    }
+}
+
+const NODE: &str = "somenode";
+const STORAGE: &str = "somestorage";
+const REMOTE: &str = "someremote";
+
+#[test]
+fn include_remotes() {
+    let config = ViewConfig {
+        id: "only-includes".into(),
+        include: vec![
+            FilterRule::Remote("remote-a".into()),
+            FilterRule::Remote("remote-b".into()),
+        ],
+        ..Default::default()
+    };
+    run_test(
+        config.clone(),
+        &[
+            (
+                (
+                    "remote-a",
+                    &make_storage_resource("remote-a", NODE, STORAGE),
+                ),
+                true,
+            ),
+            (
+                (
+                    "remote-b",
+                    &make_storage_resource("remote-b", NODE, STORAGE),
+                ),
+                true,
+            ),
+            (
+                (
+                    "remote-c",
+                    &make_storage_resource("remote-c", NODE, STORAGE),
+                ),
+                false,
+            ),
+        ],
+    );
+
+    let view = View::new(config);
+
+    assert!(!view.can_skip_remote("remote-a"));
+    assert!(!view.can_skip_remote("remote-b"));
+    assert!(view.can_skip_remote("remote-c"));
+}
+
+#[test]
+fn exclude_remotes() {
+    let config = ViewConfig {
+        id: "only-excludes".into(),
+        exclude: vec![
+            FilterRule::Remote("remote-a".into()),
+            FilterRule::Remote("remote-b".into()),
+        ],
+        ..Default::default()
+    };
+
+    run_test(
+        config.clone(),
+        &[
+            (
+                (
+                    "remote-a",
+                    &make_storage_resource("remote-a", NODE, STORAGE),
+                ),
+                false,
+            ),
+            (
+                (
+                    "remote-b",
+                    &make_storage_resource("remote-b", NODE, STORAGE),
+                ),
+                false,
+            ),
+            (
+                (
+                    "remote-c",
+                    &make_storage_resource("remote-c", NODE, STORAGE),
+                ),
+                true,
+            ),
+        ],
+    );
+
+    let view = View::new(config);
+
+    assert!(view.can_skip_remote("remote-a"));
+    assert!(view.can_skip_remote("remote-b"));
+    assert!(!view.can_skip_remote("remote-c"));
+}
+
+#[test]
+fn include_exclude_remotes() {
+    let config = ViewConfig {
+        id: "both".into(),
+        include: vec![
+            FilterRule::Remote("remote-a".into()),
+            FilterRule::Remote("remote-b".into()),
+        ],
+        exclude: vec![
+            FilterRule::Remote("remote-b".into()),
+            FilterRule::Remote("remote-c".into()),
+        ],
+    };
+    run_test(
+        config.clone(),
+        &[
+            (
+                (
+                    "remote-a",
+                    &make_storage_resource("remote-a", NODE, STORAGE),
+                ),
+                true,
+            ),
+            (
+                (
+                    "remote-b",
+                    &make_storage_resource("remote-b", NODE, STORAGE),
+                ),
+                false,
+            ),
+            (
+                (
+                    "remote-c",
+                    &make_storage_resource("remote-c", NODE, STORAGE),
+                ),
+                false,
+            ),
+        ],
+    );
+
+    let view = View::new(config);
+
+    assert!(!view.can_skip_remote("remote-a"));
+    assert!(view.can_skip_remote("remote-b"));
+    assert!(view.can_skip_remote("remote-c"));
+    assert!(view.can_skip_remote("remote-d"));
+}
+
+#[test]
+fn empty_config() {
+    let config = ViewConfig {
+        id: "empty".into(),
+        ..Default::default()
+    };
+    run_test(
+        config.clone(),
+        &[
+            (
+                (
+                    "remote-a",
+                    &make_storage_resource("remote-a", NODE, STORAGE),
+                ),
+                true,
+            ),
+            (
+                (
+                    "remote-b",
+                    &make_storage_resource("remote-b", NODE, STORAGE),
+                ),
+                true,
+            ),
+            (
+                (
+                    "remote-c",
+                    &make_storage_resource("remote-c", NODE, STORAGE),
+                ),
+                true,
+            ),
+            (
+                (REMOTE, &make_qemu_resource(REMOTE, NODE, 100, None, &[])),
+                true,
+            ),
+        ],
+    );
+
+    let view = View::new(config);
+
+    assert!(!view.can_skip_remote("remote-a"));
+    assert!(!view.can_skip_remote("remote-b"));
+    assert!(!view.can_skip_remote("remote-c"));
+}
+
+#[test]
+fn include_type() {
+    run_test(
+        ViewConfig {
+            id: "include-resource-type".into(),
+            include: vec![
+                FilterRule::ResourceType(ResourceType::PveStorage),
+                FilterRule::ResourceType(ResourceType::PveQemu),
+            ],
+            ..Default::default()
+        },
+        &[
+            (
+                (REMOTE, &make_storage_resource(REMOTE, NODE, STORAGE)),
+                true,
+            ),
+            (
+                (REMOTE, &make_qemu_resource(REMOTE, NODE, 100, None, &[])),
+                true,
+            ),
+            (
+                (REMOTE, &make_lxc_resource(REMOTE, NODE, 101, None, &[])),
+                false,
+            ),
+        ],
+    );
+}
+
+#[test]
+fn exclude_type() {
+    run_test(
+        ViewConfig {
+            id: "exclude-resource-type".into(),
+            exclude: vec![
+                FilterRule::ResourceType(ResourceType::PveStorage),
+                FilterRule::ResourceType(ResourceType::PveQemu),
+            ],
+            ..Default::default()
+        },
+        &[
+            (
+                (REMOTE, &make_storage_resource(REMOTE, NODE, STORAGE)),
+                false,
+            ),
+            (
+                (REMOTE, &make_qemu_resource(REMOTE, NODE, 100, None, &[])),
+                false,
+            ),
+            (
+                (REMOTE, &make_lxc_resource(REMOTE, NODE, 101, None, &[])),
+                true,
+            ),
+        ],
+    );
+}
+
+#[test]
+fn include_exclude_type() {
+    run_test(
+        ViewConfig {
+            id: "exclude-resource-type".into(),
+            include: vec![FilterRule::ResourceType(ResourceType::PveQemu)],
+            exclude: vec![FilterRule::ResourceType(ResourceType::PveStorage)],
+        },
+        &[
+            (
+                (REMOTE, &make_storage_resource(REMOTE, NODE, STORAGE)),
+                false,
+            ),
+            (
+                (REMOTE, &make_qemu_resource(REMOTE, NODE, 100, None, &[])),
+                true,
+            ),
+            (
+                (REMOTE, &make_lxc_resource(REMOTE, NODE, 101, None, &[])),
+                false,
+            ),
+        ],
+    );
+}
+
+#[test]
+fn include_exclude_tags() {
+    run_test(
+        ViewConfig {
+            id: "include-tags".into(),
+            include: vec![
+                FilterRule::Tag("tag1".to_string()),
+                FilterRule::Tag("tag2".to_string()),
+            ],
+            exclude: vec![FilterRule::Tag("tag3".to_string())],
+        },
+        &[
+            (
+                (REMOTE, &make_storage_resource(REMOTE, NODE, STORAGE)),
+                // only qemu/lxc can match tags for now
+                false,
+            ),
+            (
+                (
+                    REMOTE,
+                    &make_qemu_resource(REMOTE, NODE, 100, None, &["tag1", "tag3"]),
+                ),
+                // because tag3 is excluded
+                false,
+            ),
+            (
+                (
+                    REMOTE,
+                    &make_lxc_resource(REMOTE, NODE, 101, None, &["tag1"]),
+                ),
+                // matches since it's in the includes
+                true,
+            ),
+            (
+                (
+                    REMOTE,
+                    &make_lxc_resource(REMOTE, NODE, 102, None, &["tag4"]),
+                ),
+                // Not in includes, can never match
+                false,
+            ),
+        ],
+    );
+}
+
+#[test]
+fn include_exclude_resource_pool() {
+    run_test(
+        ViewConfig {
+            id: "pools".into(),
+            include: vec![
+                FilterRule::ResourcePool("pool1".to_string()),
+                FilterRule::ResourcePool("pool2".to_string()),
+            ],
+            exclude: vec![FilterRule::ResourcePool("pool2".to_string())],
+        },
+        &[
+            (
+                (REMOTE, &make_storage_resource(REMOTE, NODE, STORAGE)),
+                // only qemu/lxc can match pools for now
+                false,
+            ),
+            (
+                (
+                    REMOTE,
+                    &make_qemu_resource(REMOTE, NODE, 100, Some("pool2"), &[]),
+                ),
+                // because pool2 is excluded (takes precedence over includes)
+                false,
+            ),
+            (
+                (
+                    REMOTE,
+                    &make_lxc_resource(REMOTE, NODE, 101, Some("pool1"), &[]),
+                ),
+                // matches since it's in the includes
+                true,
+            ),
+            (
+                (
+                    REMOTE,
+                    &make_lxc_resource(REMOTE, NODE, 102, Some("pool4"), &[]),
+                ),
+                // Not in includes, can never match
+                false,
+            ),
+        ],
+    );
+}
+
+#[test]
+fn include_exclude_resource_id() {
+    run_test(
+        ViewConfig {
+            id: "resource-id".into(),
+            include: vec![
+                FilterRule::ResourceId(format!("remote/{REMOTE}/guest/100")),
+                FilterRule::ResourceId(format!("remote/{REMOTE}/storage/{NODE}/{STORAGE}")),
+            ],
+            exclude: vec![
+                FilterRule::ResourceId(format!("remote/{REMOTE}/guest/101")),
+                FilterRule::ResourceId("remote/otherremote/guest/101".to_string()),
+                FilterRule::ResourceId(format!("remote/{REMOTE}/storage/{NODE}/otherstorage")),
+            ],
+        },
+        &[
+            (
+                (REMOTE, &make_storage_resource(REMOTE, NODE, STORAGE)),
+                true,
+            ),
+            (
+                (REMOTE, &make_qemu_resource(REMOTE, NODE, 100, None, &[])),
+                true,
+            ),
+            (
+                (REMOTE, &make_lxc_resource(REMOTE, NODE, 101, None, &[])),
+                false,
+            ),
+            (
+                (REMOTE, &make_lxc_resource(REMOTE, NODE, 102, None, &[])),
+                false,
+            ),
+            (
+                (
+                    "otherremote",
+                    &make_lxc_resource("otherremote", NODE, 101, None, &[]),
+                ),
+                false,
+            ),
+            (
+                (
+                    "yetanoterremote",
+                    &make_lxc_resource("yetanotherremote", NODE, 104, None, &[]),
+                ),
+                false,
+            ),
+        ],
+    );
+}
+
+#[test]
+fn node_included() {
+    let view = View::new(ViewConfig {
+        id: "both".into(),
+
+        include: vec![
+            FilterRule::Remote("remote-a".to_string()),
+            FilterRule::ResourceId("remote/someremote/node/test".to_string()),
+        ],
+        exclude: vec![FilterRule::Remote("remote-b".to_string())],
+    });
+
+    assert!(view.is_node_included("remote-a", "somenode"));
+    assert!(view.is_node_included("remote-a", "somenode2"));
+    assert!(!view.is_node_included("remote-b", "somenode"));
+    assert!(!view.is_node_included("remote-b", "somenode2"));
+    assert!(view.is_node_included("someremote", "test"));
+
+    assert_eq!(view.name(), "both");
+}
+
+#[test]
+fn can_skip_remote_if_excluded() {
+    let view = View::new(ViewConfig {
+        id: "abc".into(),
+        include: vec![],
+        exclude: vec![FilterRule::Remote("remote-b".to_string())],
+    });
+
+    assert!(!view.can_skip_remote("remote-a"));
+    assert!(view.can_skip_remote("remote-b"));
+}
+
+#[test]
+fn can_skip_remote_if_included() {
+    let view = View::new(ViewConfig {
+        id: "abc".into(),
+        include: vec![FilterRule::Remote("remote-b".to_string())],
+        exclude: vec![],
+    });
+
+    assert!(!view.can_skip_remote("remote-b"));
+    assert!(view.can_skip_remote("remote-a"));
+}
+
+#[test]
+fn can_skip_remote_cannot_skip_if_any_other_include() {
+    let view = View::new(ViewConfig {
+        id: "abc".into(),
+        include: vec![
+            FilterRule::Remote("remote-b".to_string()),
+            FilterRule::ResourceId("resource/remote-a/guest/100".to_string()),
+        ],
+        exclude: vec![],
+    });
+
+    assert!(!view.can_skip_remote("remote-b"));
+    assert!(!view.can_skip_remote("remote-a"));
+}
+
+#[test]
+fn can_skip_remote_explicit_remote_exclude() {
+    let view = View::new(ViewConfig {
+        id: "abc".into(),
+        include: vec![FilterRule::ResourceId(
+            "resource/remote-a/guest/100".to_string(),
+        )],
+        exclude: vec![FilterRule::Remote("remote-a".to_string())],
+    });
+
+    assert!(view.can_skip_remote("remote-a"));
+}
+
+#[test]
+fn can_skip_remote_with_empty_config() {
+    let view = View::new(ViewConfig {
+        id: "abc".into(),
+        include: vec![],
+        exclude: vec![],
+    });
+
+    assert!(!view.can_skip_remote("remote-a"));
+    assert!(!view.can_skip_remote("remote-b"));
+}
+
+#[test]
+fn can_skip_remote_with_no_remote_includes() {
+    let view = View::new(ViewConfig {
+        id: "abc".into(),
+        include: vec![FilterRule::ResourceId(
+            "resource/remote-a/guest/100".to_string(),
+        )],
+        exclude: vec![],
+    });
+
+    assert!(!view.can_skip_remote("remote-a"));
+    assert!(!view.can_skip_remote("remote-b"));
+}
+
+#[test]
+fn explicitly_included_remote() {
+    let view = View::new(ViewConfig {
+        id: "abc".into(),
+        include: vec![FilterRule::Remote("remote-b".to_string())],
+        exclude: vec![],
+    });
+
+    assert!(view.is_remote_explicitly_included("remote-b"));
+}
+
+#[test]
+fn included_and_excluded_same_remote() {
+    let view = View::new(ViewConfig {
+        id: "abc".into(),
+        include: vec![FilterRule::Remote("remote-b".to_string())],
+        exclude: vec![FilterRule::Remote("remote-b".to_string())],
+    });
+
+    assert!(!view.is_remote_explicitly_included("remote-b"));
+}
+
+#[test]
+fn not_explicitly_included_remote() {
+    let view = View::new(ViewConfig {
+        id: "abc".into(),
+        include: vec![],
+        exclude: vec![],
+    });
+
+    // Assert that is not *explicitly* included
+    assert!(!view.is_remote_explicitly_included("remote-b"));
+}
-- 
2.47.3



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

Re: [pdm-devel] [PATCH datacenter-manager v5 04/11] views: add implementation for view resource filtering

[Thomas Lamprecht]

Am 13.11.25 um 13:16 schrieb Lukas Wagner:
> This commit adds the resource filter implementation for the previously
> defined ViewConfig type.
> 

a bit late given that this was already applied, don't get me wrong, I'm
thankful @Dominik for doing so, just a quick heads-up for coordination would
have been welcome by my side. Only few higher level comments for potential
future improvement below.

> There are include/exclude rules for the following properties:
>   - (global) resource-id

(host)name would be good to have too from the start.

>   - resource pool
>   - resource type
>   - remote
>   - tags
> 
> The rules are interpreted as follows:
> - no rules: everything matches

No hard feelings and this definitively works, but it might be a bit clearer
(and safer!) to require an explicit include "all" rule for that.

Deleting all rules by accident is easier than setting some explicit value,
and giving out access to all resources can have implications.
And sure, that's not something frequent or the like, but if there's no
disadvantage it can be still worth to go for the safer route.

It could be also a specific "include-all" flag, to make matching for it
easier and avoid that one has to iterate over all loops.
FWIW, we could still introduce this later if users actually run into it,
so mostly interested in what you think?

> - 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
> 
> Some experiments were performed with a cache that works roughly as following:
>   - HashMap<ViewId, HashMap<ResourceId, bool>> in a mutex
>   - Cache invalidated if view 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 ViewConfig 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* view config, we should be good with direct
> evaluation, as long as we don't add any filter rules which are very
> expensive to evaluate.

As already mentioned offlist a few weeks ago: I really would like to
see some flexible matching support (glob, regex, ...?)

I'm fine with that not being done here already, getting the core plumbing
in place is definitively more important, but I'd like to know how this
would look like, so doing at least a tiny/simple POC would be IMO good to
try before bumping 1.0.

> diff --git a/server/src/views/tests.rs b/server/src/views/tests.rs
> new file mode 100644
> index 00000000..030b7994
> --- /dev/null
> +++ b/server/src/views/tests.rs
> @@ -0,0 +1,619 @@

> +fn run_test(config: ViewConfig, tests: &[((&str, &Resource), bool)]) {
> +    let filter = View::new(config);
> +
> +    for ((remote_name, resource), expected) in tests {
> +        eprintln!("remote: {remote_name}, resource: {resource:?}");
> +        assert_eq!(filter.resource_matches(remote_name, resource), *expected);
> +    }
> +}
> +
> +const NODE: &str = "somenode";
> +const STORAGE: &str = "somestorage";
> +const REMOTE: &str = "someremote";
> +
> +#[test]
> +fn include_remotes() {
> +    let config = ViewConfig {
> +        id: "only-includes".into(),
> +        include: vec![
> +            FilterRule::Remote("remote-a".into()),
> +            FilterRule::Remote("remote-b".into()),
> +        ],
> +        ..Default::default()
> +    };

might be nice to have some of the tests also deserialize the config from
a raw string, as while one can argue that it's section config's job to get
us from that to the deserialized type, one can still introduce regressions
outside of the format it self (e.g. typos) and it's IMO also just nice to
see how the configs look in their serialized form, especially when planning
to extend this feature-wise.




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

Re: [pdm-devel] [PATCH datacenter-manager v5 04/11] views: add implementation for view resource filtering

[Lukas Wagner]

On Thu Nov 13, 2025 at 8:54 PM CET, Thomas Lamprecht wrote:
> Am 13.11.25 um 13:16 schrieb Lukas Wagner:
>> This commit adds the resource filter implementation for the previously
>> defined ViewConfig type.
>> 
>
> a bit late given that this was already applied, don't get me wrong, I'm
> thankful @Dominik for doing so, just a quick heads-up for coordination would
> have been welcome by my side. Only few higher level comments for potential
> future improvement below.
>

Just to be sure, what would be your preferred way to handle situations
similar to this one these? A quick "Hey, we are about to apply XYZ, any
objections?" ?

I'm still trying to find the right balance for "can *I*
apply this?" and "should this go through the project lead for final
signoff?". In the end, being to liberal when applying patches can lead
to more work for you (and all of us), which is of course something I
want to avoid. Any advice?

>> There are include/exclude rules for the following properties:
>>   - (global) resource-id
>
> (host)name would be good to have too from the start.
>

Good idea. Should be fairly trivial to add.

>>   - resource pool
>>   - resource type
>>   - remote
>>   - tags
>> 
>> The rules are interpreted as follows:
>> - no rules: everything matches
>
> No hard feelings and this definitively works, but it might be a bit clearer
> (and safer!) to require an explicit include "all" rule for that.
>
> Deleting all rules by accident is easier than setting some explicit value,
> and giving out access to all resources can have implications.
> And sure, that's not something frequent or the like, but if there's no
> disadvantage it can be still worth to go for the safer route.
>
> It could be also a specific "include-all" flag, to make matching for it
> easier and avoid that one has to iterate over all loops.
> FWIW, we could still introduce this later if users actually run into it,
> so mostly interested in what you think?
>

Yeah, I understand your point. Given that the matching also impacts
permissions, being a bit more cautious might be appropriate. The
dedicated 'include-all' flag sounds like a good idea. If we want to go
that route, we should probably add it right away.

I'll start a quick follow-up series with such a flag, then we can
continue the discussion there. Should not be much work, as far as I can
tell.

>> - 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
>> 
>> Some experiments were performed with a cache that works roughly as following:
>>   - HashMap<ViewId, HashMap<ResourceId, bool>> in a mutex
>>   - Cache invalidated if view 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 ViewConfig 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* view config, we should be good with direct
>> evaluation, as long as we don't add any filter rules which are very
>> expensive to evaluate.
>
> As already mentioned offlist a few weeks ago: I really would like to
> see some flexible matching support (glob, regex, ...?)
>
> I'm fine with that not being done here already, getting the core plumbing
> in place is definitively more important, but I'd like to know how this
> would look like, so doing at least a tiny/simple POC would be IMO good to
> try before bumping 1.0.
>

The off-list discussion didn't explicitly mention regex/globs, but
mentioned reusing the matching from the notification stack, which *does*
support regexes. Since I didn't end up reusing that part, I kind of lost
track of that aspect. Should be added now rather than later, otherwise
adding support for this in the config could become awkward.

Off-list we briefly discussed the issue; you mentioned the option to use
property strings to support the regex mode later on, e.g.

	include op=regex,value=tag:a[a-z]+

FWIW, we could just use the same convention that we already use for
'match-field' for notification matchers:

  include exact:tag=aaaz (where the "exact:" part is optional) [1]
  include regex:tag=a[a-z]+

Could make sense to just use the same syntax to avoid any confusion for
our users. This would require a minor change for the config format, so
we should do that *now*.

If we want to use this syntax, I'd quickly send a follow-up patch for
the config changes so that at least [1] is supported; we can then add
regex support at our convenience.

>> diff --git a/server/src/views/tests.rs b/server/src/views/tests.rs
>> new file mode 100644
>> index 00000000..030b7994
>> --- /dev/null
>> +++ b/server/src/views/tests.rs
>> @@ -0,0 +1,619 @@
>
>> +fn run_test(config: ViewConfig, tests: &[((&str, &Resource), bool)]) {
>> +    let filter = View::new(config);
>> +
>> +    for ((remote_name, resource), expected) in tests {
>> +        eprintln!("remote: {remote_name}, resource: {resource:?}");
>> +        assert_eq!(filter.resource_matches(remote_name, resource), *expected);
>> +    }
>> +}
>> +
>> +const NODE: &str = "somenode";
>> +const STORAGE: &str = "somestorage";
>> +const REMOTE: &str = "someremote";
>> +
>> +#[test]
>> +fn include_remotes() {
>> +    let config = ViewConfig {
>> +        id: "only-includes".into(),
>> +        include: vec![
>> +            FilterRule::Remote("remote-a".into()),
>> +            FilterRule::Remote("remote-b".into()),
>> +        ],
>> +        ..Default::default()
>> +    };
>
> might be nice to have some of the tests also deserialize the config from
> a raw string, as while one can argue that it's section config's job to get
> us from that to the deserialized type, one can still introduce regressions
> outside of the format it self (e.g. typos) and it's IMO also just nice to
> see how the configs look in their serialized form, especially when planning
> to extend this feature-wise.

Agreed.


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

[pdm-devel] [PATCH datacenter-manager v5 05/11] api: resources: list: add support for view parameter

[Lukas Wagner]

A view allows one to get filtered subset of all resources, based on
filter rules defined in a config file. View integrate with the
permission system - if a user has permissions on /view/{view-id}, then
these privileges are transitively applied to all resources which are
matched by the rules. All other permission checks are replaced if
requesting data through a view.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>
---
 server/src/api/resources.rs  | 50 +++++++++++++++++++++++++++++++-----
 server/src/resource_cache.rs |  3 ++-
 2 files changed, 45 insertions(+), 8 deletions(-)

diff --git a/server/src/api/resources.rs b/server/src/api/resources.rs
index dad3e6b6..1068fc49 100644
--- a/server/src/api/resources.rs
+++ b/server/src/api/resources.rs
@@ -18,7 +18,7 @@ use pdm_api_types::resource::{
 use pdm_api_types::subscription::{
     NodeSubscriptionInfo, RemoteSubscriptionState, RemoteSubscriptions, SubscriptionLevel,
 };
-use pdm_api_types::{Authid, PRIV_RESOURCE_AUDIT};
+use pdm_api_types::{Authid, PRIV_RESOURCE_AUDIT, VIEW_ID_SCHEMA};
 use pdm_search::{Search, SearchTerm};
 use proxmox_access_control::CachedUserInfo;
 use proxmox_router::{
@@ -30,8 +30,8 @@ use proxmox_sortable_macro::sortable;
 use proxmox_subscription::SubscriptionStatus;
 use pve_api_types::{ClusterResource, ClusterResourceType};
 
-use crate::connection;
 use crate::metric_collection::top_entities;
+use crate::{connection, views};
 
 pub const ROUTER: Router = Router::new()
     .get(&list_subdirs_api_method!(SUBDIRS))
@@ -221,6 +221,10 @@ impl From<RemoteWithResources> for RemoteResources {
                 type: ResourceType,
                 optional: true,
             },
+            view: {
+                schema: VIEW_ID_SCHEMA,
+                optional: true,
+            },
         }
     },
     returns: {
@@ -236,10 +240,17 @@ pub async fn get_resources(
     max_age: u64,
     resource_type: Option<ResourceType>,
     search: Option<String>,
+    view: Option<String>,
     rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<Vec<RemoteResources>, Error> {
-    let remotes_with_resources =
-        get_resources_impl(max_age, search, resource_type, Some(rpcenv)).await?;
+    let remotes_with_resources = get_resources_impl(
+        max_age,
+        search,
+        resource_type,
+        view.as_deref(),
+        Some(rpcenv),
+    )
+    .await?;
     let resources = remotes_with_resources.into_iter().map(Into::into).collect();
     Ok(resources)
 }
@@ -276,6 +287,7 @@ pub(crate) async fn get_resources_impl(
     max_age: u64,
     search: Option<String>,
     resource_type: Option<ResourceType>,
+    view: Option<&str>,
     rpcenv: Option<&mut dyn RpcEnvironment>,
 ) -> Result<Vec<RemoteWithResources>, Error> {
     let user_info = CachedUserInfo::new()?;
@@ -285,9 +297,15 @@ pub(crate) async fn get_resources_impl(
             .get_auth_id()
             .ok_or_else(|| format_err!("no authid available"))?
             .parse()?;
-        if !user_info.any_privs_below(&auth_id, &["resource"], PRIV_RESOURCE_AUDIT)? {
+
+        // NOTE: Assumption is that the regular permission check is completely replaced by a check
+        // on the view ACL object *if* a view parameter is passed.
+        if let Some(view) = &view {
+            user_info.check_privs(&auth_id, &["view", view], PRIV_RESOURCE_AUDIT, false)?;
+        } else if !user_info.any_privs_below(&auth_id, &["resource"], PRIV_RESOURCE_AUDIT)? {
             http_bail!(FORBIDDEN, "user has no access to resources");
         }
+
         opt_auth_id = Some(auth_id);
     }
 
@@ -296,10 +314,16 @@ pub(crate) async fn get_resources_impl(
 
     let filters = search.map(Search::from).unwrap_or_default();
 
+    let view = views::get_optional_view(view)?;
+
     let remotes_only = is_remotes_only(&filters);
 
     for (remote_name, remote) in remotes_config {
-        if let Some(ref auth_id) = opt_auth_id {
+        if let Some(view) = &view {
+            if view.can_skip_remote(&remote_name) {
+                continue;
+            }
+        } else if let Some(ref auth_id) = opt_auth_id {
             let remote_privs = user_info.lookup_privs(auth_id, &["resource", &remote_name]);
             if remote_privs & PRIV_RESOURCE_AUDIT == 0 {
                 continue;
@@ -374,6 +398,17 @@ pub(crate) async fn get_resources_impl(
         }
     }
 
+    if let Some(view) = &view {
+        remote_resources.retain_mut(|r| {
+            r.resources
+                .retain(|resource| view.resource_matches(&r.remote_name, resource));
+
+            let has_any_matched_resources = !r.resources.is_empty();
+            has_any_matched_resources
+                || (r.error.is_some() && view.is_remote_explicitly_included(&r.remote_name))
+        });
+    }
+
     Ok(remote_resources)
 }
 
@@ -405,7 +440,8 @@ pub async fn get_status(
     max_age: u64,
     rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<ResourcesStatus, Error> {
-    let remotes_with_resources = get_resources_impl(max_age, None, None, Some(rpcenv)).await?;
+    let remotes_with_resources =
+        get_resources_impl(max_age, None, None, None, Some(rpcenv)).await?;
     let mut counts = ResourcesStatus::default();
     for remote_with_resources in remotes_with_resources {
         if let Some(err) = remote_with_resources.error {
diff --git a/server/src/resource_cache.rs b/server/src/resource_cache.rs
index aa20c54e..dc3cbeaf 100644
--- a/server/src/resource_cache.rs
+++ b/server/src/resource_cache.rs
@@ -21,7 +21,8 @@ pub fn start_task() {
 async fn resource_caching_task() -> Result<(), Error> {
     loop {
         if let Err(err) =
-            crate::api::resources::get_resources_impl(METRIC_POLL_INTERVALL, None, None, None).await
+            crate::api::resources::get_resources_impl(METRIC_POLL_INTERVALL, None, None, None, None)
+                .await
         {
             log::error!("could not update resource cache: {err}");
         }
-- 
2.47.3



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

[pdm-devel] [PATCH datacenter-manager v5 06/11] api: resources: top entities: add support for view parameter

[Lukas Wagner]

A view allows one to get filtered subset of all resources, based on
filter rules defined in a config file. Views integrate with the
permission system - if a user has permissions on /view/{view-id}, then
these privileges are transitively applied to all resources which are
matched by the rules. All other permission checks are replaced if
requesting data through a view.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>
---
 server/src/api/resources.rs                  | 39 ++++++++++++++++++--
 server/src/metric_collection/top_entities.rs |  5 +++
 2 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/server/src/api/resources.rs b/server/src/api/resources.rs
index 1068fc49..abb75503 100644
--- a/server/src/api/resources.rs
+++ b/server/src/api/resources.rs
@@ -619,7 +619,11 @@ pub async fn get_subscription_status(
             "timeframe": {
                 type: RrdTimeframe,
                 optional: true,
-            }
+            },
+            view: {
+                schema: VIEW_ID_SCHEMA,
+                optional: true,
+            },
         }
     },
     access: {
@@ -632,6 +636,7 @@ pub async fn get_subscription_status(
 /// Returns the top X entities regarding the chosen type
 async fn get_top_entities(
     timeframe: Option<RrdTimeframe>,
+    view: Option<String>,
     rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<TopEntities, Error> {
     let user_info = CachedUserInfo::new()?;
@@ -640,17 +645,43 @@ async fn get_top_entities(
         .ok_or_else(|| format_err!("no authid available"))?
         .parse()?;
 
-    if !user_info.any_privs_below(&auth_id, &["resource"], PRIV_RESOURCE_AUDIT)? {
+    if let Some(view) = &view {
+        user_info.check_privs(&auth_id, &["view", view], PRIV_RESOURCE_AUDIT, false)?;
+    } else if !user_info.any_privs_below(&auth_id, &["resource"], PRIV_RESOURCE_AUDIT)? {
         http_bail!(FORBIDDEN, "user has no access to resources");
     }
 
+    let view = views::get_optional_view(view.as_deref())?;
+
     let (remotes_config, _) = pdm_config::remotes::config()?;
+
     let check_remote_privs = |remote_name: &str| {
-        user_info.lookup_privs(&auth_id, &["resource", remote_name]) & PRIV_RESOURCE_AUDIT != 0
+        if let Some(view) = &view {
+            // if `include-remote` or `exclude-remote` are used we can limit the
+            // number of remotes to check.
+            !view.can_skip_remote(remote_name)
+        } else {
+            user_info.lookup_privs(&auth_id, &["resource", remote_name]) & PRIV_RESOURCE_AUDIT != 0
+        }
+    };
+
+    let is_resource_included = |remote: &str, resource: &Resource| {
+        if let Some(view) = &view {
+            view.resource_matches(remote, resource)
+        } else {
+            true
+        }
     };
 
     let timeframe = timeframe.unwrap_or(RrdTimeframe::Day);
-    let res = top_entities::calculate_top(&remotes_config, timeframe, 10, check_remote_privs);
+    let res = top_entities::calculate_top(
+        &remotes_config,
+        timeframe,
+        10,
+        check_remote_privs,
+        is_resource_included,
+    );
+
     Ok(res)
 }
 
diff --git a/server/src/metric_collection/top_entities.rs b/server/src/metric_collection/top_entities.rs
index 73a3e63f..a91d586c 100644
--- a/server/src/metric_collection/top_entities.rs
+++ b/server/src/metric_collection/top_entities.rs
@@ -37,6 +37,7 @@ pub fn calculate_top(
     timeframe: proxmox_rrd_api_types::RrdTimeframe,
     num: usize,
     check_remote_privs: impl Fn(&str) -> bool,
+    is_resource_included: impl Fn(&str, &Resource) -> bool,
 ) -> TopEntities {
     let mut guest_cpu = Vec::new();
     let mut node_cpu = Vec::new();
@@ -51,6 +52,10 @@ pub fn calculate_top(
             crate::api::resources::get_cached_resources(remote_name, i64::MAX as u64)
         {
             for res in data.resources {
+                if !is_resource_included(remote_name, &res) {
+                    continue;
+                }
+
                 let id = res.id().to_string();
                 let name = format!("pve/{remote_name}/{id}");
                 match &res {
-- 
2.47.3



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

[pdm-devel] [PATCH datacenter-manager v5 07/11] api: resources: status: add support for view parameter

[Lukas Wagner]

A view allows one to get filtered subset of all resources, based on
filter rules defined in a config file. View integrate with the
permission system - if a user has permissions on /view/{view-id}, then
these privileges are transitively applied to all resources which are
matched by the rules. All other permission checks are replaced if
requesting data through a view.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>
---
 server/src/api/resources.rs | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/server/src/api/resources.rs b/server/src/api/resources.rs
index abb75503..7ad3e168 100644
--- a/server/src/api/resources.rs
+++ b/server/src/api/resources.rs
@@ -425,6 +425,10 @@ pub(crate) async fn get_resources_impl(
                 default: 30,
                 optional: true,
             },
+            view: {
+                schema: VIEW_ID_SCHEMA,
+                optional: true,
+            },
         }
     },
     returns: {
@@ -438,10 +442,11 @@ pub(crate) async fn get_resources_impl(
 /// Return the amount of configured/seen resources by type
 pub async fn get_status(
     max_age: u64,
+    view: Option<String>,
     rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<ResourcesStatus, Error> {
     let remotes_with_resources =
-        get_resources_impl(max_age, None, None, None, Some(rpcenv)).await?;
+        get_resources_impl(max_age, None, None, view.as_deref(), Some(rpcenv)).await?;
     let mut counts = ResourcesStatus::default();
     for remote_with_resources in remotes_with_resources {
         if let Some(err) = remote_with_resources.error {
-- 
2.47.3



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

[pdm-devel] [PATCH datacenter-manager v5 08/11] api: subscription status: add support for view parameter

[Lukas Wagner]

A view allows one to get filtered subset of all resources, based on
filter rules defined in a config file. Views integrate with the
permission system - if a user has permissions on /view/{view-id}, then
these privileges are transitively applied to all resources which are
matched by the rules. All other permission checks are replaced if
requesting data through a view.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>
---

Notes:
    Changes since v3:
      - Move check that avoids leaking the existence of remotes so that it
        is actually reachable (thx @Shannon & @Michael)
    
    Changes since v2:
      - make sure to not filter out a remote if it has been explicitly
        included

 server/src/api/resources.rs | 66 ++++++++++++++++++++++++++++++-------
 1 file changed, 54 insertions(+), 12 deletions(-)

diff --git a/server/src/api/resources.rs b/server/src/api/resources.rs
index 7ad3e168..cdbd90cf 100644
--- a/server/src/api/resources.rs
+++ b/server/src/api/resources.rs
@@ -548,6 +548,10 @@ pub async fn get_status(
                 default: false,
                 description: "If true, includes subscription information per node (with enough privileges)",
             },
+            view: {
+                schema: VIEW_ID_SCHEMA,
+                optional: true,
+            },
         },
     },
     returns: {
@@ -562,6 +566,7 @@ pub async fn get_status(
 pub async fn get_subscription_status(
     max_age: u64,
     verbose: bool,
+    view: Option<String>,
     rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<Vec<RemoteSubscriptions>, Error> {
     let (remotes_config, _) = pdm_config::remotes::config()?;
@@ -570,9 +575,17 @@ pub async fn get_subscription_status(
 
     let auth_id = rpcenv.get_auth_id().unwrap().parse()?;
     let user_info = CachedUserInfo::new()?;
-    let allow_all = user_info
-        .check_privs(&auth_id, &["resource"], PRIV_RESOURCE_AUDIT, false)
-        .is_ok();
+
+    let allow_all = if let Some(view) = &view {
+        user_info.check_privs(&auth_id, &["view", view], PRIV_RESOURCE_AUDIT, false)?;
+        false
+    } else {
+        user_info
+            .check_privs(&auth_id, &["resource"], PRIV_RESOURCE_AUDIT, false)
+            .is_ok()
+    };
+
+    let view = views::get_optional_view(view.as_deref())?;
 
     let check_priv = |remote_name: &str| -> bool {
         user_info
@@ -586,35 +599,64 @@ pub async fn get_subscription_status(
     };
 
     for (remote_name, remote) in remotes_config {
-        if !allow_all && !check_priv(&remote_name) {
+        if let Some(view) = &view {
+            if view.can_skip_remote(&remote_name) {
+                continue;
+            }
+        } else if !allow_all && !check_priv(&remote_name) {
             continue;
         }
 
+        let view = view.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(view) = &view {
+                                view.is_node_included(&remote.id, node)
+                            } else {
+                                true
+                            }
+                        });
+                        (Some(node_status), None)
+                    }
                     Err(error) => (None, Some(error.to_string())),
                 };
 
-            let mut state = RemoteSubscriptionState::Unknown;
-
-            if let Some(node_status) = &node_status {
-                state = map_node_subscription_list_to_state(node_status);
+            if let Some(view) = view {
+                if error.is_some() && !view.is_remote_explicitly_included(&remote.id) {
+                    // Don't leak the existence of failed remotes unless they were explicitly
+                    // pulled in by a `include remote:<id>` rule.
+                    return None;
+                }
             }
 
-            RemoteSubscriptions {
+            let state = if let Some(node_status) = &node_status {
+                if node_status.is_empty() {
+                    return None;
+                }
+
+                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?
-- 
2.47.3



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

[pdm-devel] [PATCH datacenter-manager v5 09/11] api: remote-tasks: add support for view parameter

[Lukas Wagner]

A view allows one to get filtered subset of all resources, based on
filter rules defined in a config file. View filters integrate with the
permission system - if a user has permissions on /view/{view-id}, then
these privileges are transitively applied to all resources which are
matched by the rules. All other permission checks are replaced if
requesting data through a view.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>
---
 server/src/api/remote_tasks.rs | 36 ++++++++++++++++--
 server/src/remote_tasks/mod.rs | 67 ++++++++++++++++++++++------------
 2 files changed, 75 insertions(+), 28 deletions(-)

diff --git a/server/src/api/remote_tasks.rs b/server/src/api/remote_tasks.rs
index 7b97b9cd..02b6383b 100644
--- a/server/src/api/remote_tasks.rs
+++ b/server/src/api/remote_tasks.rs
@@ -4,9 +4,10 @@ use anyhow::Error;
 
 use pdm_api_types::{
     remotes::REMOTE_ID_SCHEMA, RemoteUpid, TaskCount, TaskFilters, TaskListItem, TaskStateType,
-    TaskStatistics,
+    TaskStatistics, PRIV_RESOURCE_AUDIT, VIEW_ID_SCHEMA,
 };
-use proxmox_router::{list_subdirs_api_method, Permission, Router, SubdirMap};
+use proxmox_access_control::CachedUserInfo;
+use proxmox_router::{list_subdirs_api_method, Permission, Router, RpcEnvironment, SubdirMap};
 use proxmox_schema::api;
 use proxmox_sortable_macro::sortable;
 
@@ -41,6 +42,11 @@ const SUBDIRS: SubdirMap = &sorted!([
                 schema: REMOTE_ID_SCHEMA,
                 optional: true,
             },
+            view: {
+                schema: VIEW_ID_SCHEMA,
+                optional: true,
+            },
+
         },
     },
 )]
@@ -48,8 +54,17 @@ const SUBDIRS: SubdirMap = &sorted!([
 async fn list_tasks(
     filters: TaskFilters,
     remote: Option<String>,
+    view: Option<String>,
+    rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<Vec<TaskListItem>, Error> {
-    let tasks = remote_tasks::get_tasks(filters, remote).await?;
+    let auth_id = rpcenv.get_auth_id().unwrap().parse()?;
+    let user_info = CachedUserInfo::new()?;
+
+    if let Some(view) = &view {
+        user_info.check_privs(&auth_id, &["view", view], PRIV_RESOURCE_AUDIT, false)?;
+    }
+
+    let tasks = remote_tasks::get_tasks(filters, remote, view).await?;
 
     Ok(tasks)
 }
@@ -70,6 +85,10 @@ async fn list_tasks(
                 schema: REMOTE_ID_SCHEMA,
                 optional: true,
             },
+            view: {
+                schema: VIEW_ID_SCHEMA,
+                optional: true,
+            },
         },
     },
 )]
@@ -77,8 +96,17 @@ async fn list_tasks(
 async fn task_statistics(
     filters: TaskFilters,
     remote: Option<String>,
+    view: Option<String>,
+    rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<TaskStatistics, Error> {
-    let tasks = remote_tasks::get_tasks(filters, remote).await?;
+    let auth_id = rpcenv.get_auth_id().unwrap().parse()?;
+    let user_info = CachedUserInfo::new()?;
+
+    if let Some(view) = &view {
+        user_info.check_privs(&auth_id, &["view", view], PRIV_RESOURCE_AUDIT, false)?;
+    }
+
+    let tasks = remote_tasks::get_tasks(filters, remote, view).await?;
 
     let mut by_type: HashMap<String, TaskCount> = HashMap::new();
     let mut by_remote: HashMap<String, TaskCount> = HashMap::new();
diff --git a/server/src/remote_tasks/mod.rs b/server/src/remote_tasks/mod.rs
index f39d0b77..ea3c5539 100644
--- a/server/src/remote_tasks/mod.rs
+++ b/server/src/remote_tasks/mod.rs
@@ -9,6 +9,8 @@ pub mod task_cache;
 
 use task_cache::{GetTasks, TaskCache, TaskCacheItem};
 
+use crate::views;
+
 /// Base directory for the remote task cache.
 pub const REMOTE_TASKS_DIR: &str = concat!(pdm_buildcfg::PDM_CACHE_DIR_M!(), "/remote-tasks");
 
@@ -29,7 +31,10 @@ const NUMBER_OF_UNCOMPRESSED_FILES: u32 = 2;
 pub async fn get_tasks(
     filters: TaskFilters,
     remote_filter: Option<String>,
+    view: Option<String>,
 ) -> Result<Vec<TaskListItem>, Error> {
+    let view = views::get_optional_view(view.as_deref())?;
+
     tokio::task::spawn_blocking(move || {
         let cache = get_cache()?.read()?;
 
@@ -54,30 +59,44 @@ pub async fn get_tasks(
                 }
 
                 match task.upid.native_upid() {
-                    Ok(NativeUpid::PveUpid(pve_upid)) => Some(TaskListItem {
-                        upid: task.upid.to_string(),
-                        node: pve_upid.node,
-                        pid: pve_upid.pid as i64,
-                        pstart: pve_upid.pstart,
-                        starttime: pve_upid.starttime,
-                        worker_type: pve_upid.worker_type,
-                        worker_id: None,
-                        user: pve_upid.auth_id,
-                        endtime: task.endtime,
-                        status: task.status,
-                    }),
-                    Ok(NativeUpid::PbsUpid(pbs_upid)) => Some(TaskListItem {
-                        upid: task.upid.to_string(),
-                        node: pbs_upid.node,
-                        pid: pbs_upid.pid as i64,
-                        pstart: pbs_upid.pstart,
-                        starttime: pbs_upid.starttime,
-                        worker_type: pbs_upid.worker_type,
-                        worker_id: pbs_upid.worker_id,
-                        user: pbs_upid.auth_id,
-                        endtime: task.endtime,
-                        status: task.status,
-                    }),
+                    Ok(NativeUpid::PveUpid(pve_upid)) => {
+                        if let Some(view) = &view {
+                            if !view.is_node_included(task.upid.remote(), &pve_upid.node) {
+                                return None;
+                            }
+                        }
+                        Some(TaskListItem {
+                            upid: task.upid.to_string(),
+                            node: pve_upid.node,
+                            pid: pve_upid.pid as i64,
+                            pstart: pve_upid.pstart,
+                            starttime: pve_upid.starttime,
+                            worker_type: pve_upid.worker_type,
+                            worker_id: None,
+                            user: pve_upid.auth_id,
+                            endtime: task.endtime,
+                            status: task.status,
+                        })
+                    }
+                    Ok(NativeUpid::PbsUpid(pbs_upid)) => {
+                        if let Some(view) = &view {
+                            if !view.is_node_included(task.upid.remote(), &pbs_upid.node) {
+                                return None;
+                            }
+                        }
+                        Some(TaskListItem {
+                            upid: task.upid.to_string(),
+                            node: pbs_upid.node,
+                            pid: pbs_upid.pid as i64,
+                            pstart: pbs_upid.pstart,
+                            starttime: pbs_upid.starttime,
+                            worker_type: pbs_upid.worker_type,
+                            worker_id: pbs_upid.worker_id,
+                            user: pbs_upid.auth_id,
+                            endtime: task.endtime,
+                            status: task.status,
+                        })
+                    }
                     Err(err) => {
                         log::error!("could not parse UPID: {err:#}");
                         None
-- 
2.47.3



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

[pdm-devel] [PATCH datacenter-manager v5 10/11] pdm-client: resource list: add view parameter

[Lukas Wagner]

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>
---
 cli/client/src/resources.rs | 2 +-
 lib/pdm-client/src/lib.rs   | 9 ++++++++-
 ui/src/sdn/zone_tree.rs     | 2 +-
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/cli/client/src/resources.rs b/cli/client/src/resources.rs
index dbf9f265..bc3a98ba 100644
--- a/cli/client/src/resources.rs
+++ b/cli/client/src/resources.rs
@@ -31,7 +31,7 @@ pub fn cli() -> CommandLineInterface {
 )]
 /// List all the remotes this instance is managing.
 async fn get_resources(max_age: Option<u64>) -> Result<(), Error> {
-    let mut resources = client()?.resources(max_age).await?;
+    let mut resources = client()?.resources(max_age, None).await?;
     let output_format = env().format_args.output_format;
     if output_format == OutputFormat::Text {
         if resources.is_empty() {
diff --git a/lib/pdm-client/src/lib.rs b/lib/pdm-client/src/lib.rs
index 0cab7691..46707e31 100644
--- a/lib/pdm-client/src/lib.rs
+++ b/lib/pdm-client/src/lib.rs
@@ -863,9 +863,14 @@ impl<T: HttpApiClient> PdmClient<T> {
         Ok(self.0.get(&path).await?.expect_json()?.data)
     }
 
-    pub async fn resources(&self, max_age: Option<u64>) -> Result<Vec<RemoteResources>, Error> {
+    pub async fn resources(
+        &self,
+        max_age: Option<u64>,
+        view: Option<&str>,
+    ) -> Result<Vec<RemoteResources>, Error> {
         let path = ApiPathBuilder::new("/api2/extjs/resources/list")
             .maybe_arg("max-age", &max_age)
+            .maybe_arg("view", &view)
             .build();
         Ok(self.0.get(&path).await?.expect_json()?.data)
     }
@@ -874,10 +879,12 @@ impl<T: HttpApiClient> PdmClient<T> {
         &self,
         max_age: Option<u64>,
         resource_type: ResourceType,
+        view: Option<&str>,
     ) -> Result<Vec<RemoteResources>, Error> {
         let path = ApiPathBuilder::new("/api2/extjs/resources/list")
             .maybe_arg("max-age", &max_age)
             .arg("resource-type", resource_type)
+            .maybe_arg("view", &view)
             .build();
 
         Ok(self.0.get(&path).await?.expect_json()?.data)
diff --git a/ui/src/sdn/zone_tree.rs b/ui/src/sdn/zone_tree.rs
index 1f534388..7475483d 100644
--- a/ui/src/sdn/zone_tree.rs
+++ b/ui/src/sdn/zone_tree.rs
@@ -281,7 +281,7 @@ impl LoadableComponent for ZoneTreeComponent {
         Box::pin(async move {
             let client = pdm_client();
             let remote_resources = client
-                .resources_by_type(None, ResourceType::PveSdnZone)
+                .resources_by_type(None, ResourceType::PveSdnZone, None)
                 .await?;
             link.send_message(Self::Message::LoadFinished(remote_resources));
 
-- 
2.47.3



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

[pdm-devel] [PATCH datacenter-manager v5 11/11] pdm-client: top entities: add view parameter

[Lukas Wagner]

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>
---
 lib/pdm-client/src/lib.rs | 10 +++++++---
 ui/src/dashboard/view.rs  |  2 +-
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/lib/pdm-client/src/lib.rs b/lib/pdm-client/src/lib.rs
index 46707e31..9ea0be9b 100644
--- a/lib/pdm-client/src/lib.rs
+++ b/lib/pdm-client/src/lib.rs
@@ -956,9 +956,13 @@ impl<T: HttpApiClient> PdmClient<T> {
         Ok(self.0.get(&path).await?.expect_json()?.data)
     }
 
-    pub async fn get_top_entities(&self) -> Result<TopEntities, Error> {
-        let path = "/api2/extjs/resources/top-entities";
-        Ok(self.0.get(path).await?.expect_json()?.data)
+    pub async fn get_top_entities(&self, view: Option<&str>) -> Result<TopEntities, Error> {
+        let builder = ApiPathBuilder::new("/api2/extjs/resources/top-entities".to_string())
+            .maybe_arg("view", &view);
+
+        let path = builder.build();
+
+        Ok(self.0.get(&path).await?.expect_json()?.data)
     }
 
     pub async fn pve_node_status(&self, remote: &str, node: &str) -> Result<NodeStatus, Error> {
diff --git a/ui/src/dashboard/view.rs b/ui/src/dashboard/view.rs
index c781d991..c973d321 100644
--- a/ui/src/dashboard/view.rs
+++ b/ui/src/dashboard/view.rs
@@ -162,7 +162,7 @@ impl ViewComp {
                     if top_entities {
                         let client: pdm_client::PdmClient<Rc<proxmox_yew_comp::HttpClientWasm>> =
                             pdm_client();
-                        let res = client.get_top_entities().await;
+                        let res = client.get_top_entities(None).await;
                         link.send_message(Msg::LoadingResult(LoadingResult::TopEntities(res)));
                     }
                 };
-- 
2.47.3



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

[pdm-devel] applied: [PATCH datacenter-manager v5 00/11] backend implementation for view filters

[Dominik Csapak]

On Thu, 13 Nov 2025 13:16:33 +0100, Lukas Wagner wrote:
> Key aspects:
>   - new config file at /etc/proxmox-datacenter-manager/views.cfg
>   - ViewConfig as a definition type, has
>      - {include,exclude} {remote,resource-id,resource-type,resource-pool,tag}:{value}
> 
>   - View resource filter implementation & big suite of unit tests
>     - excludes are processed after includes
>     - if no include rules are defined, all resources but those which were
>       excluded are matched
>     - if no rules are defined in a filter, everything is matched
> 
> [...]

Applied, thanks!

[01/11] pdm-api-types: views: add ViewConfig type
        commit: 621d12f398f0113bac470f5bd421f53bf9cb08ad
[02/11] pdm-config: views: add support for views
        commit: 23762dc0330bc3c70cc0a74f8a5a2005a8ef09e5
[03/11] acl: add '/view' and '/view/{view-id}' as allowed ACL paths
        commit: eccac1ae7c4c60403c579755381d47e0717d3d2d
[04/11] views: add implementation for view resource filtering
        commit: 2b06afe898e05329530c26e2df222bd0994abd83
[05/11] api: resources: list: add support for view parameter
        commit: bd9a836a77c6b15d3369c727008e776feb74639e
[06/11] api: resources: top entities: add support for view parameter
        commit: 96732d74ad021feedc5a2988accb534fcad06cd4
[07/11] api: resources: status: add support for view parameter
        commit: 812c3e645fac222eb31c2a9b3968ad65055c786a
[08/11] api: subscription status: add support for view parameter
        commit: d9788d2c0794e2a4ce5f8de2f7a5a342b4c0af1c
[09/11] api: remote-tasks: add support for view parameter
        commit: 05d74958656082be91edd57807e0d4066ec297d7
[10/11] pdm-client: resource list: add view parameter
        commit: 51c259d68bf82e37a7c9994888c1f0b9a6b880e4
[11/11] pdm-client: top entities: add view parameter
        commit: bb2a2a89a707adb15d78afa22727240ecbd09914


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