* [pdm-devel] [PATCH datacenter-manager v2 00/12] backend implementation for view filters
@ 2025-11-03 12:35 Lukas Wagner
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 01/12] pdm-api-types: views: add ViewFilterConfig type Lukas Wagner
` (11 more replies)
0 siblings, 12 replies; 32+ messages in thread
From: Lukas Wagner @ 2025-11-03 12:35 UTC (permalink / raw)
To: pdm-devel
Key aspects:
- new config file at /etc/proxmox-datacenter-manager/views/filters.cfg
(subject to change, depending on where the view templates are stored)
- ViewFilterConfig as a filter defintion type,has
- {include,exclude} {remote,resource-id,resource-type,resource-pool,tag}:{value}
- 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-filter' 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
- Decide on how exactly they will work together with the view templates
-> most likely a new config type
View {
filter: String,
template: String,
}
- UI for filter rules
- Depending on how the view template is integrated, maybe rename
ViewFilter to View (also change the API parameter name)
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 (12):
pdm-api-types: views: add ViewFilterConfig type
pdm-config: views: add support for view-filters
acl: add '/view' and '/view/{view-id}' as allowed ACL paths
views: add implementation for view filters
views: add tests for view filter implementation
api: resources: list: add support for view-filter parameter
api: resources: top entities: add support for view-filter parameter
api: resources: status: add support for view-filter parameter
api: subscription status: add support for view-filter parameter
api: remote-tasks: add support for view-filter parameter
pdm-client: resource list: add view-filter parameter
pdm-client: top entities: add view-filter parameter
cli/client/src/resources.rs | 2 +-
lib/pdm-api-types/src/lib.rs | 8 +
lib/pdm-api-types/src/views.rs | 165 ++++++
lib/pdm-client/src/lib.rs | 19 +-
lib/pdm-config/src/lib.rs | 2 +-
lib/pdm-config/src/views.rs | 62 ++
server/src/acl.rs | 6 +
server/src/api/remote_tasks.rs | 36 +-
server/src/api/resources.rs | 168 +++++-
server/src/lib.rs | 1 +
server/src/metric_collection/top_entities.rs | 5 +
server/src/remote_tasks/mod.rs | 39 +-
server/src/resource_cache.rs | 3 +-
server/src/views/mod.rs | 4 +
server/src/views/tests.rs | 585 +++++++++++++++++++
server/src/views/view_filter.rs | 182 ++++++
ui/src/dashboard/mod.rs | 2 +-
ui/src/sdn/zone_tree.rs | 2 +-
18 files changed, 1242 insertions(+), 49 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
create mode 100644 server/src/views/view_filter.rs
Summary over all repositories:
18 files changed, 1242 insertions(+), 49 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
^ permalink raw reply [flat|nested] 32+ messages in thread
* [pdm-devel] [PATCH datacenter-manager v2 01/12] pdm-api-types: views: add ViewFilterConfig type
2025-11-03 12:35 [pdm-devel] [PATCH datacenter-manager v2 00/12] backend implementation for view filters Lukas Wagner
@ 2025-11-03 12:35 ` Lukas Wagner
2025-11-05 10:07 ` Dominik Csapak
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 02/12] pdm-config: views: add support for view-filters Lukas Wagner
` (10 subsequent siblings)
11 siblings, 1 reply; 32+ messages in thread
From: Lukas Wagner @ 2025-11-03 12:35 UTC (permalink / raw)
To: pdm-devel
This type will be used to define view filters.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
Notes:
Changes since RFC:
- Change config structure, instead of 'include-{x} value' we now do
'include {x}:value'
lib/pdm-api-types/src/lib.rs | 2 +
lib/pdm-api-types/src/views.rs | 165 +++++++++++++++++++++++++++++++++
2 files changed, 167 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 2fb61ef7..a7ba6d89 100644
--- a/lib/pdm-api-types/src/lib.rs
+++ b/lib/pdm-api-types/src/lib.rs
@@ -106,6 +106,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_:, +!\-@=.]+$";
diff --git a/lib/pdm-api-types/src/views.rs b/lib/pdm-api-types/src/views.rs
new file mode 100644
index 00000000..b262cc05
--- /dev/null
+++ b/lib/pdm-api-types/src/views.rs
@@ -0,0 +1,165 @@
+use std::{fmt::Display, str::FromStr};
+
+use anyhow::bail;
+use serde::{Deserialize, Serialize};
+
+use proxmox_schema::{api, ApiStringFormat, ArraySchema, Schema, StringSchema, Updater};
+
+use crate::{remotes::REMOTE_ID_SCHEMA, resource::ResourceType, VIEW_FILTER_ID_SCHEMA};
+
+#[api(
+ properties: {
+ "id": {
+ schema: VIEW_FILTER_ID_SCHEMA,
+ },
+ "include": {
+ schema: FILTER_RULE_LIST_SCHEMA,
+ optional: true,
+ },
+ "exclude": {
+ schema: FILTER_RULE_LIST_SCHEMA,
+ optional: true,
+ }
+ }
+)]
+#[derive(Clone, Default, Deserialize, Serialize, Updater)]
+#[serde(rename_all = "kebab-case")]
+/// View definition
+pub struct ViewFilterConfig {
+ /// View filter name.
+ 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, 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)) => {
+ // TODO: Define schema and use it to validate.
+ if value.is_empty() {
+ bail!("empty resource-pool rule not allowed");
+ }
+ FilterRule::ResourcePool(value.to_string())
+ }
+ Some(("resource-id", value)) => {
+ // TODO: Define schema and use it to validate.
+ if value.is_empty() {
+ bail!("empty resource-id rule not allowed");
+ }
+ FilterRule::ResourceId(value.to_string())
+ }
+ Some(("tag", value)) => {
+ // TODO: Define schema and use it to validate.
+ if value.is_empty() {
+ bail!("empty tag rule not allowed");
+ }
+ 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_group_filter(input: &str) -> Result<(), anyhow::Error> {
+ FilterRule::from_str(input).map(|_| ())
+}
+
+/// Schema for filter rules.
+pub const FILTER_RULE_SCHEMA: Schema = StringSchema::new("Filter rule for resources.")
+ .format(&ApiStringFormat::VerifyFn(verify_group_filter))
+ .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();
+
+#[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("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
^ permalink raw reply [flat|nested] 32+ messages in thread
* [pdm-devel] [PATCH datacenter-manager v2 02/12] pdm-config: views: add support for view-filters
2025-11-03 12:35 [pdm-devel] [PATCH datacenter-manager v2 00/12] backend implementation for view filters Lukas Wagner
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 01/12] pdm-api-types: views: add ViewFilterConfig type Lukas Wagner
@ 2025-11-03 12:35 ` Lukas Wagner
2025-11-05 10:07 ` Dominik Csapak
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 03/12] acl: add '/view' and '/view/{view-id}' as allowed ACL paths Lukas Wagner
` (9 subsequent siblings)
11 siblings, 1 reply; 32+ messages in thread
From: Lukas Wagner @ 2025-11-03 12:35 UTC (permalink / raw)
To: pdm-devel
This allows to read ViewFilterConfig entries from a new config file at
/etc/proxmox-datacenter-manager/views/filters.cfg.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
lib/pdm-api-types/src/lib.rs | 6 ++++
lib/pdm-config/src/lib.rs | 2 +-
lib/pdm-config/src/views.rs | 62 ++++++++++++++++++++++++++++++++++++
3 files changed, 69 insertions(+), 1 deletion(-)
create mode 100644 lib/pdm-config/src/views.rs
diff --git a/lib/pdm-api-types/src/lib.rs b/lib/pdm-api-types/src/lib.rs
index a7ba6d89..ed284ab2 100644
--- a/lib/pdm-api-types/src/lib.rs
+++ b/lib/pdm-api-types/src/lib.rs
@@ -159,6 +159,12 @@ pub const REALM_ID_SCHEMA: Schema = StringSchema::new("Realm name.")
.max_length(32)
.schema();
+pub const VIEW_FILTER_ID_SCHEMA: Schema = StringSchema::new("View filter 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-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..adeb67b1
--- /dev/null
+++ b/lib/pdm-config/src/views.rs
@@ -0,0 +1,62 @@
+use std::sync::LazyLock;
+
+use anyhow::Error;
+
+use proxmox_schema::ApiType;
+use proxmox_section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin};
+
+use pdm_api_types::{views::ViewFilterConfig, ConfigDigest, VIEW_FILTER_ID_SCHEMA};
+
+use pdm_buildcfg::configdir;
+
+static VIEW_FILTER_SECTION_NAME: &str = "view-filter";
+
+static CONFIG: LazyLock<SectionConfig> = LazyLock::new(init);
+
+fn init() -> SectionConfig {
+ let mut config = SectionConfig::new(&VIEW_FILTER_ID_SCHEMA);
+
+ let view_plugin = SectionConfigPlugin::new(
+ VIEW_FILTER_SECTION_NAME.to_string(),
+ Some("id".to_string()),
+ ViewFilterConfig::API_SCHEMA.unwrap_object_schema(),
+ );
+ config.register_plugin(view_plugin);
+
+ config
+}
+
+const VIEW_FILTER_CFG_FILENAME: &str = configdir!("/views/filters.cfg");
+
+// TODO: Will be needed once the CRUD API for view filters is implemented.
+// const VIEW_FILTER_CFG_LOCKFILE: &str = configdir!("/views/.filters.lock");
+
+// TODO: Will be needed once the CRUD API for view filters is implemented.
+/// Get exclusive lock
+// fn lock_config() -> Result<ApiLockGuard, Error> {
+// open_api_lockfile(VIEW_FILTER_CFG_LOCKFILE, None, true)
+// }
+
+fn config() -> Result<(SectionConfigData, ConfigDigest), Error> {
+ let content =
+ proxmox_sys::fs::file_read_optional_string(VIEW_FILTER_CFG_FILENAME)?.unwrap_or_default();
+ let digest = ConfigDigest::from_slice(content.as_bytes());
+ let data = CONFIG.parse(VIEW_FILTER_CFG_FILENAME, &content)?;
+ Ok((data, digest))
+}
+
+// TODO: Will be needed once the CRUD API for view filters is implemented.
+// fn save_config(config: &SectionConfigData) -> Result<(), Error> {
+// let raw = CONFIG.write(VIEW_FILTER_CFG_FILENAME, config)?;
+// replace_privileged_config(VIEW_FILTER_CFG_FILENAME, raw.as_bytes())
+// }
+//
+
+/// Get the [`ViewFilterConfig`] entry for a view filter with a given ID.
+///
+/// This will fail if the config file does not exist or if the view filter does not exist.
+pub fn get_view_filter_config(filter_name: &str) -> Result<ViewFilterConfig, Error> {
+ let (cfg, _) = config()?;
+
+ cfg.lookup(VIEW_FILTER_SECTION_NAME, filter_name)
+}
--
2.47.3
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 32+ messages in thread
* [pdm-devel] [PATCH datacenter-manager v2 03/12] acl: add '/view' and '/view/{view-id}' as allowed ACL paths
2025-11-03 12:35 [pdm-devel] [PATCH datacenter-manager v2 00/12] backend implementation for view filters Lukas Wagner
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 01/12] pdm-api-types: views: add ViewFilterConfig type Lukas Wagner
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 02/12] pdm-config: views: add support for view-filters Lukas Wagner
@ 2025-11-03 12:35 ` Lukas Wagner
2025-11-05 10:07 ` Dominik Csapak
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 04/12] views: add implementation for view filters Lukas Wagner
` (8 subsequent siblings)
11 siblings, 1 reply; 32+ messages in thread
From: Lukas Wagner @ 2025-11-03 12:35 UTC (permalink / raw)
To: pdm-devel
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>
---
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
^ permalink raw reply [flat|nested] 32+ messages in thread
* [pdm-devel] [PATCH datacenter-manager v2 04/12] views: add implementation for view filters
2025-11-03 12:35 [pdm-devel] [PATCH datacenter-manager v2 00/12] backend implementation for view filters Lukas Wagner
` (2 preceding siblings ...)
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 03/12] acl: add '/view' and '/view/{view-id}' as allowed ACL paths Lukas Wagner
@ 2025-11-03 12:35 ` Lukas Wagner
2025-11-05 10:08 ` Dominik Csapak
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 05/12] views: add tests for view filter implementation Lukas Wagner
` (7 subsequent siblings)
11 siblings, 1 reply; 32+ messages in thread
From: Lukas Wagner @ 2025-11-03 12:35 UTC (permalink / raw)
To: pdm-devel
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 | 182 ++++++++++++++++++++++++++++++++
3 files changed, 184 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..4f77e7bf
--- /dev/null
+++ b/server/src/views/view_filter.rs
@@ -0,0 +1,182 @@
+use anyhow::Error;
+
+use pdm_api_types::{
+ resource::{Resource, ResourceType},
+ views::{FilterRule, 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 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(|e| {
+ if let FilterRule::Remote(r) = e {
+ r == remote
+ } else {
+ false
+ }
+ });
+
+ (has_any_include_remote && !matches_any_include_remote && !any_other)
+ || matches_any_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 filter.
+ 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 check_rules(rules: &[FilterRule], remote: &str, resource: &ResourceData) -> bool {
+ for rule in rules {
+ let verdict = 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,
+ };
+
+ if verdict {
+ return true;
+ }
+ }
+
+ false
+}
+
+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(),
+ },
+ }
+ }
+}
--
2.47.3
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 32+ messages in thread
* [pdm-devel] [PATCH datacenter-manager v2 05/12] views: add tests for view filter implementation
2025-11-03 12:35 [pdm-devel] [PATCH datacenter-manager v2 00/12] backend implementation for view filters Lukas Wagner
` (3 preceding siblings ...)
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 04/12] views: add implementation for view filters Lukas Wagner
@ 2025-11-03 12:35 ` Lukas Wagner
2025-11-05 10:08 ` Dominik Csapak
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 06/12] api: resources: list: add support for view-filter parameter Lukas Wagner
` (6 subsequent siblings)
11 siblings, 1 reply; 32+ messages in thread
From: Lukas Wagner @ 2025-11-03 12:35 UTC (permalink / raw)
To: pdm-devel
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
server/src/views/mod.rs | 3 +
server/src/views/tests.rs | 585 ++++++++++++++++++++++++++++++++++++++
2 files changed, 588 insertions(+)
create mode 100644 server/src/views/tests.rs
diff --git a/server/src/views/mod.rs b/server/src/views/mod.rs
index 9a2856a4..ea9e6de7 100644
--- a/server/src/views/mod.rs
+++ b/server/src/views/mod.rs
@@ -1 +1,4 @@
pub mod view_filter;
+
+#[cfg(test)]
+mod tests;
diff --git a/server/src/views/tests.rs b/server/src/views/tests.rs
new file mode 100644
index 00000000..013b301f
--- /dev/null
+++ b/server/src/views/tests.rs
@@ -0,0 +1,585 @@
+use pdm_api_types::{
+ resource::{PveLxcResource, PveQemuResource, PveStorageResource, Resource, ResourceType},
+ views::{FilterRule, ViewFilterConfig},
+};
+
+use super::view_filter::ViewFilter;
+
+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: ViewFilterConfig, tests: &[((&str, &Resource), bool)]) {
+ let filter = ViewFilter::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 = ViewFilterConfig {
+ 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 filter = ViewFilter::new(config);
+
+ assert!(!filter.can_skip_remote("remote-a"));
+ assert!(!filter.can_skip_remote("remote-b"));
+ assert!(filter.can_skip_remote("remote-c"));
+}
+
+#[test]
+fn exclude_remotes() {
+ let config = ViewFilterConfig {
+ 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 filter = ViewFilter::new(config);
+
+ assert!(filter.can_skip_remote("remote-a"));
+ assert!(filter.can_skip_remote("remote-b"));
+ assert!(!filter.can_skip_remote("remote-c"));
+}
+
+#[test]
+fn include_exclude_remotes() {
+ let config = ViewFilterConfig {
+ 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 filter = ViewFilter::new(config);
+
+ assert!(!filter.can_skip_remote("remote-a"));
+ assert!(filter.can_skip_remote("remote-b"));
+ assert!(filter.can_skip_remote("remote-c"));
+ assert!(filter.can_skip_remote("remote-d"));
+}
+
+#[test]
+fn empty_config() {
+ let config = ViewFilterConfig {
+ 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 filter = ViewFilter::new(config);
+
+ assert!(!filter.can_skip_remote("remote-a"));
+ assert!(!filter.can_skip_remote("remote-b"));
+ assert!(!filter.can_skip_remote("remote-c"));
+}
+
+#[test]
+fn include_type() {
+ run_test(
+ ViewFilterConfig {
+ 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(
+ ViewFilterConfig {
+ 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(
+ ViewFilterConfig {
+ 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(
+ ViewFilterConfig {
+ 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(
+ ViewFilterConfig {
+ 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(
+ ViewFilterConfig {
+ 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 filter = ViewFilter::new(ViewFilterConfig {
+ 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!(filter.is_node_included("remote-a", "somenode"));
+ assert!(filter.is_node_included("remote-a", "somenode2"));
+ assert!(!filter.is_node_included("remote-b", "somenode"));
+ assert!(!filter.is_node_included("remote-b", "somenode2"));
+ assert!(filter.is_node_included("someremote", "test"));
+
+ assert_eq!(filter.name(), "both");
+}
+
+#[test]
+fn can_skip_remote_if_excluded() {
+ let filter = ViewFilter::new(ViewFilterConfig {
+ id: "abc".into(),
+ include: vec![],
+ exclude: vec![FilterRule::Remote("remote-b".to_string())],
+ });
+
+ assert!(!filter.can_skip_remote("remote-a"));
+ assert!(filter.can_skip_remote("remote-b"));
+}
+
+#[test]
+fn can_skip_remote_if_included() {
+ let filter = ViewFilter::new(ViewFilterConfig {
+ id: "abc".into(),
+ include: vec![FilterRule::Remote("remote-b".to_string())],
+ exclude: vec![],
+ });
+
+ assert!(!filter.can_skip_remote("remote-b"));
+ assert!(filter.can_skip_remote("remote-a"));
+}
+
+#[test]
+fn can_skip_remote_cannot_skip_if_any_other_include() {
+ let filter = ViewFilter::new(ViewFilterConfig {
+ id: "abc".into(),
+ include: vec![
+ FilterRule::Remote("remote-b".to_string()),
+ FilterRule::ResourceId("resource/remote-a/guest/100".to_string()),
+ ],
+ exclude: vec![],
+ });
+
+ assert!(!filter.can_skip_remote("remote-b"));
+ assert!(!filter.can_skip_remote("remote-a"));
+}
+
+#[test]
+fn can_skip_remote_explicit_remote_exclude() {
+ let filter = ViewFilter::new(ViewFilterConfig {
+ id: "abc".into(),
+ include: vec![FilterRule::ResourceId(
+ "resource/remote-a/guest/100".to_string(),
+ )],
+ exclude: vec![FilterRule::Remote("remote-a".to_string())],
+ });
+
+ assert!(filter.can_skip_remote("remote-a"));
+}
+
+#[test]
+fn can_skip_remote_with_empty_config() {
+ let filter = ViewFilter::new(ViewFilterConfig {
+ id: "abc".into(),
+ include: vec![],
+ exclude: vec![],
+ });
+
+ assert!(!filter.can_skip_remote("remote-a"));
+ assert!(!filter.can_skip_remote("remote-b"));
+}
+
+#[test]
+fn can_skip_remote_with_no_remote_includes() {
+ let filter = ViewFilter::new(ViewFilterConfig {
+ id: "abc".into(),
+ include: vec![FilterRule::ResourceId(
+ "resource/remote-a/guest/100".to_string(),
+ )],
+ exclude: vec![],
+ });
+
+ assert!(!filter.can_skip_remote("remote-a"));
+ assert!(!filter.can_skip_remote("remote-b"));
+}
--
2.47.3
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 32+ messages in thread
* [pdm-devel] [PATCH datacenter-manager v2 06/12] api: resources: list: add support for view-filter parameter
2025-11-03 12:35 [pdm-devel] [PATCH datacenter-manager v2 00/12] backend implementation for view filters Lukas Wagner
` (4 preceding siblings ...)
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 05/12] views: add tests for view filter implementation Lukas Wagner
@ 2025-11-03 12:35 ` Lukas Wagner
2025-11-05 10:08 ` Dominik Csapak
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 07/12] api: resources: top entities: " Lukas Wagner
` (5 subsequent siblings)
11 siblings, 1 reply; 32+ messages in thread
From: Lukas Wagner @ 2025-11-03 12:35 UTC (permalink / raw)
To: pdm-devel
A view filter 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-filter-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 filter.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
server/src/api/resources.rs | 56 ++++++++++++++++++++++++++++++------
server/src/resource_cache.rs | 3 +-
2 files changed, 50 insertions(+), 9 deletions(-)
diff --git a/server/src/api/resources.rs b/server/src/api/resources.rs
index 81c9d9ae..6feda45b 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_FILTER_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-filter": {
+ schema: VIEW_FILTER_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_filter: 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_filter.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_filter: 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_filter) = &view_filter {
+ user_info.check_privs(&auth_id, &["view", view_filter], PRIV_RESOURCE_AUDIT, false)?;
+ } else if !user_info.any_privs_below(&auth_id, &["resource"], PRIV_RESOURCE_AUDIT)? {
http_bail!(UNAUTHORIZED, "user has no access to resources");
}
+
opt_auth_id = Some(auth_id);
}
@@ -296,12 +314,24 @@ pub(crate) async fn get_resources_impl(
let filters = search.map(Search::from).unwrap_or_default();
+ let view_filter = view_filter
+ .map(|filter_name| views::view_filter::get_view_filter(&filter_name))
+ .transpose()?;
+
let remotes_only = is_remotes_only(&filters);
for (remote_name, remote) in remotes_config {
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 {
+ if view_filter.is_none() {
+ let remote_privs = user_info.lookup_privs(auth_id, &["resource", &remote_name]);
+ if remote_privs & PRIV_RESOURCE_AUDIT == 0 {
+ continue;
+ }
+ }
+ }
+
+ if let Some(view_filter) = &view_filter {
+ if view_filter.can_skip_remote(&remote_name) {
continue;
}
}
@@ -374,6 +404,15 @@ pub(crate) async fn get_resources_impl(
}
}
+ if let Some(filter) = &view_filter {
+ remote_resources.retain_mut(|r| {
+ r.resources
+ .retain(|resource| filter.resource_matches(&r.remote_name, resource));
+
+ !r.resources.is_empty()
+ });
+ }
+
Ok(remote_resources)
}
@@ -405,7 +444,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
^ permalink raw reply [flat|nested] 32+ messages in thread
* [pdm-devel] [PATCH datacenter-manager v2 07/12] api: resources: top entities: add support for view-filter parameter
2025-11-03 12:35 [pdm-devel] [PATCH datacenter-manager v2 00/12] backend implementation for view filters Lukas Wagner
` (5 preceding siblings ...)
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 06/12] api: resources: list: add support for view-filter parameter Lukas Wagner
@ 2025-11-03 12:35 ` Lukas Wagner
2025-11-05 10:08 ` Dominik Csapak
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 08/12] api: resources: status: " Lukas Wagner
` (4 subsequent siblings)
11 siblings, 1 reply; 32+ messages in thread
From: Lukas Wagner @ 2025-11-03 12:35 UTC (permalink / raw)
To: pdm-devel
A view filter 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-filter-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 filter.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
server/src/api/resources.rs | 41 ++++++++++++++++++--
server/src/metric_collection/top_entities.rs | 5 +++
2 files changed, 42 insertions(+), 4 deletions(-)
diff --git a/server/src/api/resources.rs b/server/src/api/resources.rs
index 6feda45b..ea76d81b 100644
--- a/server/src/api/resources.rs
+++ b/server/src/api/resources.rs
@@ -623,7 +623,11 @@ pub async fn get_subscription_status(
"timeframe": {
type: RrdTimeframe,
optional: true,
- }
+ },
+ "view-filter": {
+ schema: VIEW_FILTER_ID_SCHEMA,
+ optional: true,
+ },
}
},
access: {
@@ -636,6 +640,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_filter: Option<String>,
rpcenv: &mut dyn RpcEnvironment,
) -> Result<TopEntities, Error> {
let user_info = CachedUserInfo::new()?;
@@ -644,17 +649,45 @@ 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_filter) = &view_filter {
+ user_info.check_privs(&auth_id, &["view", view_filter], 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_filter = view_filter
+ .map(|a| views::view_filter::get_view_filter(&a))
+ .transpose()?;
+
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_filter) = &view_filter {
+ // if `include-remote` or `exclude-remote` are used we can limit the
+ // number of remotes to check.
+ !view_filter.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_filter) = &view_filter {
+ view_filter.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
^ permalink raw reply [flat|nested] 32+ messages in thread
* [pdm-devel] [PATCH datacenter-manager v2 08/12] api: resources: status: add support for view-filter parameter
2025-11-03 12:35 [pdm-devel] [PATCH datacenter-manager v2 00/12] backend implementation for view filters Lukas Wagner
` (6 preceding siblings ...)
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 07/12] api: resources: top entities: " Lukas Wagner
@ 2025-11-03 12:35 ` Lukas Wagner
2025-11-05 10:08 ` Dominik Csapak
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 09/12] api: subscription " Lukas Wagner
` (3 subsequent siblings)
11 siblings, 1 reply; 32+ messages in thread
From: Lukas Wagner @ 2025-11-03 12:35 UTC (permalink / raw)
To: pdm-devel
A view filter 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-filter-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 filter.
Signed-off-by: Lukas Wagner <l.wagner@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 ea76d81b..f718ce3e 100644
--- a/server/src/api/resources.rs
+++ b/server/src/api/resources.rs
@@ -429,6 +429,10 @@ pub(crate) async fn get_resources_impl(
default: 30,
optional: true,
},
+ "view-filter": {
+ schema: VIEW_FILTER_ID_SCHEMA,
+ optional: true,
+ },
}
},
returns: {
@@ -442,10 +446,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_filter: 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_filter.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
^ permalink raw reply [flat|nested] 32+ messages in thread
* [pdm-devel] [PATCH datacenter-manager v2 09/12] api: subscription status: add support for view-filter parameter
2025-11-03 12:35 [pdm-devel] [PATCH datacenter-manager v2 00/12] backend implementation for view filters Lukas Wagner
` (7 preceding siblings ...)
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 08/12] api: resources: status: " Lukas Wagner
@ 2025-11-03 12:35 ` Lukas Wagner
2025-11-05 10:08 ` Dominik Csapak
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 10/12] api: remote-tasks: " Lukas Wagner
` (2 subsequent siblings)
11 siblings, 1 reply; 32+ messages in thread
From: Lukas Wagner @ 2025-11-03 12:35 UTC (permalink / raw)
To: pdm-devel
A view filter 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-filter-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 filter.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
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 f718ce3e..db1e2c7c 100644
--- a/server/src/api/resources.rs
+++ b/server/src/api/resources.rs
@@ -552,6 +552,10 @@ pub async fn get_status(
default: false,
description: "If true, includes subscription information per node (with enough privileges)",
},
+ "view-filter": {
+ schema: VIEW_FILTER_ID_SCHEMA,
+ optional: true,
+ },
},
},
returns: {
@@ -566,6 +570,7 @@ pub async fn get_status(
pub async fn get_subscription_status(
max_age: u64,
verbose: bool,
+ view_filter: Option<String>,
rpcenv: &mut dyn RpcEnvironment,
) -> Result<Vec<RemoteSubscriptions>, Error> {
let (remotes_config, _) = pdm_config::remotes::config()?;
@@ -574,9 +579,19 @@ 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, &["resources"], PRIV_RESOURCE_AUDIT, false)
- .is_ok();
+
+ let allow_all = if let Some(view_filter) = &view_filter {
+ user_info.check_privs(&auth_id, &["view", view_filter], PRIV_RESOURCE_AUDIT, false)?;
+ false
+ } else {
+ user_info
+ .check_privs(&auth_id, &["resources"], PRIV_RESOURCE_AUDIT, false)
+ .is_ok()
+ };
+
+ let view_filter = view_filter
+ .map(|filter_name| views::view_filter::get_view_filter(&filter_name))
+ .transpose()?;
let check_priv = |remote_name: &str| -> bool {
user_info
@@ -590,35 +605,62 @@ pub async fn get_subscription_status(
};
for (remote_name, remote) in remotes_config {
- if !allow_all && !check_priv(&remote_name) {
+ if let Some(filter) = &view_filter {
+ if filter.can_skip_remote(&remote_name) {
+ continue;
+ }
+ } else if !allow_all && !check_priv(&remote_name) {
continue;
}
+ let view_filter_clone = view_filter.clone();
+
let future = async move {
let (node_status, error) =
match get_subscription_info_for_remote(&remote, max_age).await {
- Ok(node_status) => (Some(node_status), None),
+ Ok(mut node_status) => {
+ node_status.retain(|node, _| {
+ if let Some(filter) = &view_filter_clone {
+ filter.is_node_included(&remote.id, node)
+ } else {
+ true
+ }
+ });
+ (Some(node_status), None)
+ }
Err(error) => (None, Some(error.to_string())),
};
- let mut state = RemoteSubscriptionState::Unknown;
+ let state = if let Some(node_status) = &node_status {
+ if error.is_some() && view_filter_clone.is_some() {
+ // Don't leak the existence of failed remotes, since we cannot apply
+ // view-filters here.
+ return None;
+ }
- if let Some(node_status) = &node_status {
- state = map_node_subscription_list_to_state(node_status);
- }
+ if node_status.is_empty() {
+ return None;
+ }
- RemoteSubscriptions {
+ map_node_subscription_list_to_state(node_status)
+ } else {
+ RemoteSubscriptionState::Unknown
+ };
+
+ Some(RemoteSubscriptions {
remote: remote_name,
error,
state,
node_status: if verbose { node_status } else { None },
- }
+ })
};
futures.push(future);
}
- Ok(join_all(futures).await)
+ let status = join_all(futures).await.into_iter().flatten().collect();
+
+ Ok(status)
}
// FIXME: make timeframe and count parameters?
--
2.47.3
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 32+ messages in thread
* [pdm-devel] [PATCH datacenter-manager v2 10/12] api: remote-tasks: add support for view-filter parameter
2025-11-03 12:35 [pdm-devel] [PATCH datacenter-manager v2 00/12] backend implementation for view filters Lukas Wagner
` (8 preceding siblings ...)
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 09/12] api: subscription " Lukas Wagner
@ 2025-11-03 12:35 ` Lukas Wagner
2025-11-05 10:09 ` Dominik Csapak
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 11/12] pdm-client: resource list: add " Lukas Wagner
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 12/12] pdm-client: top entities: " Lukas Wagner
11 siblings, 1 reply; 32+ messages in thread
From: Lukas Wagner @ 2025-11-03 12:35 UTC (permalink / raw)
To: pdm-devel
A view filter 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-filter-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 filter.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
server/src/api/remote_tasks.rs | 36 +++++++++++++++++++++++++++----
server/src/remote_tasks/mod.rs | 39 +++++++++++++++++++++++-----------
2 files changed, 59 insertions(+), 16 deletions(-)
diff --git a/server/src/api/remote_tasks.rs b/server/src/api/remote_tasks.rs
index 7b97b9cd..4a68c5d9 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_FILTER_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-filter": {
+ schema: VIEW_FILTER_ID_SCHEMA,
+ optional: true,
+ },
+
},
},
)]
@@ -48,8 +54,17 @@ const SUBDIRS: SubdirMap = &sorted!([
async fn list_tasks(
filters: TaskFilters,
remote: Option<String>,
+ view_filter: 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_filter) = &view_filter {
+ user_info.check_privs(&auth_id, &["view", view_filter], PRIV_RESOURCE_AUDIT, false)?;
+ }
+
+ let tasks = remote_tasks::get_tasks(filters, remote, view_filter).await?;
Ok(tasks)
}
@@ -70,6 +85,10 @@ async fn list_tasks(
schema: REMOTE_ID_SCHEMA,
optional: true,
},
+ "view-filter": {
+ schema: VIEW_FILTER_ID_SCHEMA,
+ optional: true,
+ },
},
},
)]
@@ -77,8 +96,17 @@ async fn list_tasks(
async fn task_statistics(
filters: TaskFilters,
remote: Option<String>,
+ view_filter: 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_filter) = &view_filter {
+ user_info.check_privs(&auth_id, &["view", view_filter], PRIV_RESOURCE_AUDIT, false)?;
+ }
+
+ let tasks = remote_tasks::get_tasks(filters, remote, view_filter).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 c4939742..4d93d9b8 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,12 @@ const NUMBER_OF_UNCOMPRESSED_FILES: u32 = 2;
pub async fn get_tasks(
filters: TaskFilters,
remote_filter: Option<String>,
+ view_filter: Option<String>,
) -> Result<Vec<TaskListItem>, Error> {
+ let view_filter = view_filter
+ .map(|filter_name| views::view_filter::get_view_filter(&filter_name))
+ .transpose()?;
+
tokio::task::spawn_blocking(move || {
let cache = get_cache()?.read()?;
@@ -52,21 +59,29 @@ pub async fn get_tasks(
return None;
}
}
+
// TODO: Handle PBS tasks
let pve_upid: Result<PveUpid, Error> = task.upid.upid.parse();
match pve_upid {
- Ok(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(pve_upid) => {
+ if let Some(view_filter) = &view_filter {
+ if !view_filter.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,
+ })
+ }
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
^ permalink raw reply [flat|nested] 32+ messages in thread
* [pdm-devel] [PATCH datacenter-manager v2 11/12] pdm-client: resource list: add view-filter parameter
2025-11-03 12:35 [pdm-devel] [PATCH datacenter-manager v2 00/12] backend implementation for view filters Lukas Wagner
` (9 preceding siblings ...)
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 10/12] api: remote-tasks: " Lukas Wagner
@ 2025-11-03 12:35 ` Lukas Wagner
2025-11-05 10:11 ` Dominik Csapak
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 12/12] pdm-client: top entities: " Lukas Wagner
11 siblings, 1 reply; 32+ messages in thread
From: Lukas Wagner @ 2025-11-03 12:35 UTC (permalink / raw)
To: pdm-devel
Signed-off-by: Lukas Wagner <l.wagner@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..7102ee05 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_filter: Option<&str>,
+ ) -> Result<Vec<RemoteResources>, Error> {
let path = ApiPathBuilder::new("/api2/extjs/resources/list")
.maybe_arg("max-age", &max_age)
+ .maybe_arg("view-filter", &view_filter)
.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_filter: 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-filter", &view_filter)
.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
^ permalink raw reply [flat|nested] 32+ messages in thread
* [pdm-devel] [PATCH datacenter-manager v2 12/12] pdm-client: top entities: add view-filter parameter
2025-11-03 12:35 [pdm-devel] [PATCH datacenter-manager v2 00/12] backend implementation for view filters Lukas Wagner
` (10 preceding siblings ...)
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 11/12] pdm-client: resource list: add " Lukas Wagner
@ 2025-11-03 12:35 ` Lukas Wagner
2025-11-05 10:12 ` Dominik Csapak
11 siblings, 1 reply; 32+ messages in thread
From: Lukas Wagner @ 2025-11-03 12:35 UTC (permalink / raw)
To: pdm-devel
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
lib/pdm-client/src/lib.rs | 10 +++++++---
ui/src/dashboard/mod.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 7102ee05..c9ed18cc 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_filter: Option<&str>) -> Result<TopEntities, Error> {
+ let builder = ApiPathBuilder::new("/api2/extjs/resources/top-entities".to_string())
+ .maybe_arg("view-filter", &view_filter);
+
+ 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/mod.rs b/ui/src/dashboard/mod.rs
index 07d5cd99..ce64d847 100644
--- a/ui/src/dashboard/mod.rs
+++ b/ui/src/dashboard/mod.rs
@@ -319,7 +319,7 @@ impl PdmDashboard {
let top_entities_future = {
let link = link.clone();
async move {
- let res = client.get_top_entities().await;
+ let res = client.get_top_entities(None).await;
link.send_message(Msg::LoadingFinished(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
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager v2 01/12] pdm-api-types: views: add ViewFilterConfig type
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 01/12] pdm-api-types: views: add ViewFilterConfig type Lukas Wagner
@ 2025-11-05 10:07 ` Dominik Csapak
0 siblings, 0 replies; 32+ messages in thread
From: Dominik Csapak @ 2025-11-05 10:07 UTC (permalink / raw)
To: Proxmox Datacenter Manager development discussion, Lukas Wagner
some comments inline
On 11/3/25 1:35 PM, Lukas Wagner wrote:
> This type will be used to define view filters.
>
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
>
> Notes:
> Changes since RFC:
> - Change config structure, instead of 'include-{x} value' we now do
> 'include {x}:value'
>
> lib/pdm-api-types/src/lib.rs | 2 +
> lib/pdm-api-types/src/views.rs | 165 +++++++++++++++++++++++++++++++++
> 2 files changed, 167 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 2fb61ef7..a7ba6d89 100644
> --- a/lib/pdm-api-types/src/lib.rs
> +++ b/lib/pdm-api-types/src/lib.rs
> @@ -106,6 +106,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_:, +!\-@=.]+$";
> diff --git a/lib/pdm-api-types/src/views.rs b/lib/pdm-api-types/src/views.rs
> new file mode 100644
> index 00000000..b262cc05
> --- /dev/null
> +++ b/lib/pdm-api-types/src/views.rs
> @@ -0,0 +1,165 @@
> +use std::{fmt::Display, str::FromStr};
> +
> +use anyhow::bail;
> +use serde::{Deserialize, Serialize};
> +
> +use proxmox_schema::{api, ApiStringFormat, ArraySchema, Schema, StringSchema, Updater};
> +
> +use crate::{remotes::REMOTE_ID_SCHEMA, resource::ResourceType, VIEW_FILTER_ID_SCHEMA};
you use the VIEW_FILTER_ID_SCHEMA here but it's only introduced in the
next patch
> +
> +#[api(
> + properties: {
> + "id": {
> + schema: VIEW_FILTER_ID_SCHEMA,
> + },
> + "include": {
> + schema: FILTER_RULE_LIST_SCHEMA,
> + optional: true,
> + },
> + "exclude": {
> + schema: FILTER_RULE_LIST_SCHEMA,
> + optional: true,
> + }
> + }
> +)]
> +#[derive(Clone, Default, Deserialize, Serialize, Updater)]
> +#[serde(rename_all = "kebab-case")]
> +/// View definition
> +pub struct ViewFilterConfig {
> + /// View filter name.
> + 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, 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)) => {
> + // TODO: Define schema and use it to validate.
couldn't we reuse a schema/regex here ? e.g. PROXMOX_SAFE_ID_REGEX ?
> + if value.is_empty() {
> + bail!("empty resource-pool rule not allowed");
> + }
> + FilterRule::ResourcePool(value.to_string())
> + }
> + Some(("resource-id", value)) => {
> + // TODO: Define schema and use it to validate.
same here
> + if value.is_empty() {
> + bail!("empty resource-id rule not allowed");
> + }
> + FilterRule::ResourceId(value.to_string())
> + }
> + Some(("tag", value)) => {
> + // TODO: Define schema and use it to validate.
and here
> + if value.is_empty() {
> + bail!("empty tag rule not allowed");
> + }
> + 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_group_filter(input: &str) -> Result<(), anyhow::Error> {
> + FilterRule::from_str(input).map(|_| ())
> +}
> +
> +/// Schema for filter rules.
> +pub const FILTER_RULE_SCHEMA: Schema = StringSchema::new("Filter rule for resources.")
> + .format(&ApiStringFormat::VerifyFn(verify_group_filter))
> + .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();
nit: i would find it nicer if theses definitions are at the top of the
file, before they are used
> +
> +#[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("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());
> + }
> +}
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager v2 02/12] pdm-config: views: add support for view-filters
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 02/12] pdm-config: views: add support for view-filters Lukas Wagner
@ 2025-11-05 10:07 ` Dominik Csapak
2025-11-05 10:37 ` Lukas Wagner
0 siblings, 1 reply; 32+ messages in thread
From: Dominik Csapak @ 2025-11-05 10:07 UTC (permalink / raw)
To: Proxmox Datacenter Manager development discussion, Lukas Wagner
high level comments:
would it be nicer if we use the ApiSectionDataEntry trait like we do for
Remotes ?
that way we can use the typed section config helpers
(which are a bit more ergonomic to use later in the CRUD api calls)
Also if we just use one file for the views, is it necessary
to put it in a views folder?
I'm currently thinking about how to save the layouts, and (as we
recently discussed off-list) I'm leaning towards simply saving the
layout as a json string into the config here, as that makes the
loading/saving/updating/deleting much easier for both view filters
and layouts (no need to track/lock both configs, etc.)
We can of course leave it , but then we have to make sure to create
the directory correctly :)
On 11/3/25 1:35 PM, Lukas Wagner wrote:
> This allows to read ViewFilterConfig entries from a new config file at
> /etc/proxmox-datacenter-manager/views/filters.cfg.
>
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
> lib/pdm-api-types/src/lib.rs | 6 ++++
> lib/pdm-config/src/lib.rs | 2 +-
> lib/pdm-config/src/views.rs | 62 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 69 insertions(+), 1 deletion(-)
> create mode 100644 lib/pdm-config/src/views.rs
>
> diff --git a/lib/pdm-api-types/src/lib.rs b/lib/pdm-api-types/src/lib.rs
> index a7ba6d89..ed284ab2 100644
> --- a/lib/pdm-api-types/src/lib.rs
> +++ b/lib/pdm-api-types/src/lib.rs
> @@ -159,6 +159,12 @@ pub const REALM_ID_SCHEMA: Schema = StringSchema::new("Realm name.")
> .max_length(32)
> .schema();
>
> +pub const VIEW_FILTER_ID_SCHEMA: Schema = StringSchema::new("View filter 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-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..adeb67b1
> --- /dev/null
> +++ b/lib/pdm-config/src/views.rs
> @@ -0,0 +1,62 @@
> +use std::sync::LazyLock;
> +
> +use anyhow::Error;
> +
> +use proxmox_schema::ApiType;
> +use proxmox_section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin};
> +
> +use pdm_api_types::{views::ViewFilterConfig, ConfigDigest, VIEW_FILTER_ID_SCHEMA};
> +
> +use pdm_buildcfg::configdir;
> +
> +static VIEW_FILTER_SECTION_NAME: &str = "view-filter";
> +
> +static CONFIG: LazyLock<SectionConfig> = LazyLock::new(init);
> +
> +fn init() -> SectionConfig {
> + let mut config = SectionConfig::new(&VIEW_FILTER_ID_SCHEMA);
> +
> + let view_plugin = SectionConfigPlugin::new(
> + VIEW_FILTER_SECTION_NAME.to_string(),
> + Some("id".to_string()),
> + ViewFilterConfig::API_SCHEMA.unwrap_object_schema(),
> + );
> + config.register_plugin(view_plugin);
> +
> + config
> +}
> +
> +const VIEW_FILTER_CFG_FILENAME: &str = configdir!("/views/filters.cfg");
> +
> +// TODO: Will be needed once the CRUD API for view filters is implemented.
> +// const VIEW_FILTER_CFG_LOCKFILE: &str = configdir!("/views/.filters.lock");
> +
> +// TODO: Will be needed once the CRUD API for view filters is implemented.
> +/// Get exclusive lock
> +// fn lock_config() -> Result<ApiLockGuard, Error> {
> +// open_api_lockfile(VIEW_FILTER_CFG_LOCKFILE, None, true)
> +// }
> +
> +fn config() -> Result<(SectionConfigData, ConfigDigest), Error> {
> + let content =
> + proxmox_sys::fs::file_read_optional_string(VIEW_FILTER_CFG_FILENAME)?.unwrap_or_default();
> + let digest = ConfigDigest::from_slice(content.as_bytes());
> + let data = CONFIG.parse(VIEW_FILTER_CFG_FILENAME, &content)?;
> + Ok((data, digest))
> +}
> +
> +// TODO: Will be needed once the CRUD API for view filters is implemented.
> +// fn save_config(config: &SectionConfigData) -> Result<(), Error> {
> +// let raw = CONFIG.write(VIEW_FILTER_CFG_FILENAME, config)?;
> +// replace_privileged_config(VIEW_FILTER_CFG_FILENAME, raw.as_bytes())
> +// }
> +//
> +
> +/// Get the [`ViewFilterConfig`] entry for a view filter with a given ID.
> +///
> +/// This will fail if the config file does not exist or if the view filter does not exist.
> +pub fn get_view_filter_config(filter_name: &str) -> Result<ViewFilterConfig, Error> {
> + let (cfg, _) = config()?;
> +
> + cfg.lookup(VIEW_FILTER_SECTION_NAME, filter_name)
> +}
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager v2 03/12] acl: add '/view' and '/view/{view-id}' as allowed ACL paths
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 03/12] acl: add '/view' and '/view/{view-id}' as allowed ACL paths Lukas Wagner
@ 2025-11-05 10:07 ` Dominik Csapak
0 siblings, 0 replies; 32+ messages in thread
From: Dominik Csapak @ 2025-11-05 10:07 UTC (permalink / raw)
To: Proxmox Datacenter Manager development discussion, Lukas Wagner
Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
On 11/3/25 1:35 PM, Lukas Wagner wrote:
> 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>
> ---
> 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(());
> + }
> + }
> _ => {}
> }
>
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager v2 04/12] views: add implementation for view filters
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 04/12] views: add implementation for view filters Lukas Wagner
@ 2025-11-05 10:08 ` Dominik Csapak
2025-11-05 10:58 ` Lukas Wagner
0 siblings, 1 reply; 32+ messages in thread
From: Dominik Csapak @ 2025-11-05 10:08 UTC (permalink / raw)
To: Proxmox Datacenter Manager development discussion, Lukas Wagner
some comments inline
On 11/3/25 1:36 PM, 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 | 182 ++++++++++++++++++++++++++++++++
> 3 files changed, 184 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..4f77e7bf
> --- /dev/null
> +++ b/server/src/views/view_filter.rs
> @@ -0,0 +1,182 @@
> +use anyhow::Error;
> +
> +use pdm_api_types::{
> + resource::{Resource, ResourceType},
> + views::{FilterRule, 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,
> +}
wouldn't a newtype suffice here too?
pub struct ViewFilter(ViewFilterConfig)
? alternatively, what about having freestanding functions that
take a `&ViewFilterConfig` as parameter ?
If we're doing it this way though, I'd rather implement a
From<ViewFilterConfig> for ViewFilter than a `new` method
(or maybe both)
> +
> +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.
isn't that (full) info more suited for the commit message than a comment?
e.g. a single line comment with 'caching here is currently not worth it'
and the full text in the commit message should also be ok?
(no hard feelings though)
> +
> + 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(|e| {
> + if let FilterRule::Remote(r) = e {
> + r == remote
> + } else {
> + false
> + }
> + });
> +
> + (has_any_include_remote && !matches_any_include_remote && !any_other)
> + || matches_any_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 filter.
> + 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 check_rules(rules: &[FilterRule], remote: &str, resource: &ResourceData) -> bool {
> + for rule in rules {
> + let verdict = 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,
> + };
> +
> + if verdict {
> + return true;
> + }
> + }
> +
> + false
wouldn't this boil down to:
return rules.any(|rule| match rule { ... } ) ?
instead of looping and doing an early return manually?
> +}
> +
> +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(),
> + },
> + }
> + }
> +}
Is it really worht it, to define a seperate type that you only use
internally?
couldn't you simple use the &Resource type directly?
or maybe just having 2 helper methods to extract the relevant info?
(the type and global_id is already abstracted, so it's only relevant
for the tags and the resource_pool ?)
but i guess i'd have to see it to determine what is better...
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager v2 05/12] views: add tests for view filter implementation
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 05/12] views: add tests for view filter implementation Lukas Wagner
@ 2025-11-05 10:08 ` Dominik Csapak
2025-11-05 10:58 ` Lukas Wagner
0 siblings, 1 reply; 32+ messages in thread
From: Dominik Csapak @ 2025-11-05 10:08 UTC (permalink / raw)
To: Proxmox Datacenter Manager development discussion, Lukas Wagner
IMHO this should be melded into the last commit
On 11/3/25 1:35 PM, Lukas Wagner wrote:
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
> server/src/views/mod.rs | 3 +
> server/src/views/tests.rs | 585 ++++++++++++++++++++++++++++++++++++++
> 2 files changed, 588 insertions(+)
> create mode 100644 server/src/views/tests.rs
>
> diff --git a/server/src/views/mod.rs b/server/src/views/mod.rs
> index 9a2856a4..ea9e6de7 100644
> --- a/server/src/views/mod.rs
> +++ b/server/src/views/mod.rs
> @@ -1 +1,4 @@
> pub mod view_filter;
> +
> +#[cfg(test)]
> +mod tests;
> diff --git a/server/src/views/tests.rs b/server/src/views/tests.rs
> new file mode 100644
> index 00000000..013b301f
> --- /dev/null
> +++ b/server/src/views/tests.rs
> @@ -0,0 +1,585 @@
> +use pdm_api_types::{
> + resource::{PveLxcResource, PveQemuResource, PveStorageResource, Resource, ResourceType},
> + views::{FilterRule, ViewFilterConfig},
> +};
> +
> +use super::view_filter::ViewFilter;
> +
> +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: ViewFilterConfig, tests: &[((&str, &Resource), bool)]) {
> + let filter = ViewFilter::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 = ViewFilterConfig {
> + 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 filter = ViewFilter::new(config);
> +
> + assert!(!filter.can_skip_remote("remote-a"));
> + assert!(!filter.can_skip_remote("remote-b"));
> + assert!(filter.can_skip_remote("remote-c"));
> +}
> +
> +#[test]
> +fn exclude_remotes() {
> + let config = ViewFilterConfig {
> + 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 filter = ViewFilter::new(config);
> +
> + assert!(filter.can_skip_remote("remote-a"));
> + assert!(filter.can_skip_remote("remote-b"));
> + assert!(!filter.can_skip_remote("remote-c"));
> +}
> +
> +#[test]
> +fn include_exclude_remotes() {
> + let config = ViewFilterConfig {
> + 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 filter = ViewFilter::new(config);
> +
> + assert!(!filter.can_skip_remote("remote-a"));
> + assert!(filter.can_skip_remote("remote-b"));
> + assert!(filter.can_skip_remote("remote-c"));
> + assert!(filter.can_skip_remote("remote-d"));
> +}
> +
> +#[test]
> +fn empty_config() {
> + let config = ViewFilterConfig {
> + 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 filter = ViewFilter::new(config);
> +
> + assert!(!filter.can_skip_remote("remote-a"));
> + assert!(!filter.can_skip_remote("remote-b"));
> + assert!(!filter.can_skip_remote("remote-c"));
> +}
> +
> +#[test]
> +fn include_type() {
> + run_test(
> + ViewFilterConfig {
> + 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(
> + ViewFilterConfig {
> + 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(
> + ViewFilterConfig {
> + 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(
> + ViewFilterConfig {
> + 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(
> + ViewFilterConfig {
> + 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(
> + ViewFilterConfig {
> + 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 filter = ViewFilter::new(ViewFilterConfig {
> + 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!(filter.is_node_included("remote-a", "somenode"));
> + assert!(filter.is_node_included("remote-a", "somenode2"));
> + assert!(!filter.is_node_included("remote-b", "somenode"));
> + assert!(!filter.is_node_included("remote-b", "somenode2"));
> + assert!(filter.is_node_included("someremote", "test"));
> +
> + assert_eq!(filter.name(), "both");
> +}
> +
> +#[test]
> +fn can_skip_remote_if_excluded() {
> + let filter = ViewFilter::new(ViewFilterConfig {
> + id: "abc".into(),
> + include: vec![],
> + exclude: vec![FilterRule::Remote("remote-b".to_string())],
> + });
> +
> + assert!(!filter.can_skip_remote("remote-a"));
> + assert!(filter.can_skip_remote("remote-b"));
> +}
> +
> +#[test]
> +fn can_skip_remote_if_included() {
> + let filter = ViewFilter::new(ViewFilterConfig {
> + id: "abc".into(),
> + include: vec![FilterRule::Remote("remote-b".to_string())],
> + exclude: vec![],
> + });
> +
> + assert!(!filter.can_skip_remote("remote-b"));
> + assert!(filter.can_skip_remote("remote-a"));
> +}
> +
> +#[test]
> +fn can_skip_remote_cannot_skip_if_any_other_include() {
> + let filter = ViewFilter::new(ViewFilterConfig {
> + id: "abc".into(),
> + include: vec![
> + FilterRule::Remote("remote-b".to_string()),
> + FilterRule::ResourceId("resource/remote-a/guest/100".to_string()),
> + ],
> + exclude: vec![],
> + });
> +
> + assert!(!filter.can_skip_remote("remote-b"));
> + assert!(!filter.can_skip_remote("remote-a"));
> +}
> +
> +#[test]
> +fn can_skip_remote_explicit_remote_exclude() {
> + let filter = ViewFilter::new(ViewFilterConfig {
> + id: "abc".into(),
> + include: vec![FilterRule::ResourceId(
> + "resource/remote-a/guest/100".to_string(),
> + )],
> + exclude: vec![FilterRule::Remote("remote-a".to_string())],
> + });
> +
> + assert!(filter.can_skip_remote("remote-a"));
> +}
> +
> +#[test]
> +fn can_skip_remote_with_empty_config() {
> + let filter = ViewFilter::new(ViewFilterConfig {
> + id: "abc".into(),
> + include: vec![],
> + exclude: vec![],
> + });
> +
> + assert!(!filter.can_skip_remote("remote-a"));
> + assert!(!filter.can_skip_remote("remote-b"));
> +}
> +
> +#[test]
> +fn can_skip_remote_with_no_remote_includes() {
> + let filter = ViewFilter::new(ViewFilterConfig {
> + id: "abc".into(),
> + include: vec![FilterRule::ResourceId(
> + "resource/remote-a/guest/100".to_string(),
> + )],
> + exclude: vec![],
> + });
> +
> + assert!(!filter.can_skip_remote("remote-a"));
> + assert!(!filter.can_skip_remote("remote-b"));
> +}
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager v2 06/12] api: resources: list: add support for view-filter parameter
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 06/12] api: resources: list: add support for view-filter parameter Lukas Wagner
@ 2025-11-05 10:08 ` Dominik Csapak
0 siblings, 0 replies; 32+ messages in thread
From: Dominik Csapak @ 2025-11-05 10:08 UTC (permalink / raw)
To: Proxmox Datacenter Manager development discussion, Lukas Wagner
see comment inline
On 11/3/25 1:35 PM, Lukas Wagner wrote:
> A view filter 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-filter-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 filter.
>
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
> server/src/api/resources.rs | 56 ++++++++++++++++++++++++++++++------
> server/src/resource_cache.rs | 3 +-
> 2 files changed, 50 insertions(+), 9 deletions(-)
>
> diff --git a/server/src/api/resources.rs b/server/src/api/resources.rs
> index 81c9d9ae..6feda45b 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_FILTER_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-filter": {
> + schema: VIEW_FILTER_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_filter: 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_filter.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_filter: 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_filter) = &view_filter {
> + user_info.check_privs(&auth_id, &["view", view_filter], PRIV_RESOURCE_AUDIT, false)?;
> + } else if !user_info.any_privs_below(&auth_id, &["resource"], PRIV_RESOURCE_AUDIT)? {
> http_bail!(UNAUTHORIZED, "user has no access to resources");
> }
> +
> opt_auth_id = Some(auth_id);
> }
>
> @@ -296,12 +314,24 @@ pub(crate) async fn get_resources_impl(
>
> let filters = search.map(Search::from).unwrap_or_default();
>
> + let view_filter = view_filter
> + .map(|filter_name| views::view_filter::get_view_filter(&filter_name))
> + .transpose()?;
> +
> let remotes_only = is_remotes_only(&filters);
>
> for (remote_name, remote) in remotes_config {
> 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 {
> + if view_filter.is_none() {
> + let remote_privs = user_info.lookup_privs(auth_id, &["resource", &remote_name]);
> + if remote_privs & PRIV_RESOURCE_AUDIT == 0 {
> + continue;
> + }
> + }
> + }
> +
> + if let Some(view_filter) = &view_filter {
> + if view_filter.can_skip_remote(&remote_name) {
> continue;
> }
> }
> @@ -374,6 +404,15 @@ pub(crate) async fn get_resources_impl(
> }
> }
>
> + if let Some(filter) = &view_filter {
> + remote_resources.retain_mut(|r| {
> + r.resources
> + .retain(|resource| filter.resource_matches(&r.remote_name, resource));
> +
> + !r.resources.is_empty()
this leads to remotes that have an error are not being returned.
this should probably only return false when either
the list was not empty before but is after
or if r.error.is_none()
> + });
> + }
> +
> Ok(remote_resources)
> }
>
> @@ -405,7 +444,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}");
> }
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager v2 07/12] api: resources: top entities: add support for view-filter parameter
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 07/12] api: resources: top entities: " Lukas Wagner
@ 2025-11-05 10:08 ` Dominik Csapak
0 siblings, 0 replies; 32+ messages in thread
From: Dominik Csapak @ 2025-11-05 10:08 UTC (permalink / raw)
To: Proxmox Datacenter Manager development discussion, Lukas Wagner
Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
On 11/3/25 1:35 PM, Lukas Wagner wrote:
> A view filter 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-filter-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 filter.
>
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
> server/src/api/resources.rs | 41 ++++++++++++++++++--
> server/src/metric_collection/top_entities.rs | 5 +++
> 2 files changed, 42 insertions(+), 4 deletions(-)
>
> diff --git a/server/src/api/resources.rs b/server/src/api/resources.rs
> index 6feda45b..ea76d81b 100644
> --- a/server/src/api/resources.rs
> +++ b/server/src/api/resources.rs
> @@ -623,7 +623,11 @@ pub async fn get_subscription_status(
> "timeframe": {
> type: RrdTimeframe,
> optional: true,
> - }
> + },
> + "view-filter": {
> + schema: VIEW_FILTER_ID_SCHEMA,
> + optional: true,
> + },
> }
> },
> access: {
> @@ -636,6 +640,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_filter: Option<String>,
> rpcenv: &mut dyn RpcEnvironment,
> ) -> Result<TopEntities, Error> {
> let user_info = CachedUserInfo::new()?;
> @@ -644,17 +649,45 @@ 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_filter) = &view_filter {
> + user_info.check_privs(&auth_id, &["view", view_filter], 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_filter = view_filter
> + .map(|a| views::view_filter::get_view_filter(&a))
> + .transpose()?;
> +
> 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_filter) = &view_filter {
> + // if `include-remote` or `exclude-remote` are used we can limit the
> + // number of remotes to check.
> + !view_filter.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_filter) = &view_filter {
> + view_filter.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 {
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager v2 08/12] api: resources: status: add support for view-filter parameter
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 08/12] api: resources: status: " Lukas Wagner
@ 2025-11-05 10:08 ` Dominik Csapak
0 siblings, 0 replies; 32+ messages in thread
From: Dominik Csapak @ 2025-11-05 10:08 UTC (permalink / raw)
To: Proxmox Datacenter Manager development discussion, Lukas Wagner
Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
On 11/3/25 1:35 PM, Lukas Wagner wrote:
> A view filter 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-filter-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 filter.
>
> Signed-off-by: Lukas Wagner <l.wagner@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 ea76d81b..f718ce3e 100644
> --- a/server/src/api/resources.rs
> +++ b/server/src/api/resources.rs
> @@ -429,6 +429,10 @@ pub(crate) async fn get_resources_impl(
> default: 30,
> optional: true,
> },
> + "view-filter": {
> + schema: VIEW_FILTER_ID_SCHEMA,
> + optional: true,
> + },
> }
> },
> returns: {
> @@ -442,10 +446,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_filter: 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_filter.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 {
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager v2 09/12] api: subscription status: add support for view-filter parameter
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 09/12] api: subscription " Lukas Wagner
@ 2025-11-05 10:08 ` Dominik Csapak
2025-11-05 14:11 ` Lukas Wagner
0 siblings, 1 reply; 32+ messages in thread
From: Dominik Csapak @ 2025-11-05 10:08 UTC (permalink / raw)
To: Proxmox Datacenter Manager development discussion, Lukas Wagner
see comments inline
On 11/3/25 1:36 PM, Lukas Wagner wrote:
> A view filter 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-filter-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 filter.
>
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
> 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 f718ce3e..db1e2c7c 100644
> --- a/server/src/api/resources.rs
> +++ b/server/src/api/resources.rs
> @@ -552,6 +552,10 @@ pub async fn get_status(
> default: false,
> description: "If true, includes subscription information per node (with enough privileges)",
> },
> + "view-filter": {
> + schema: VIEW_FILTER_ID_SCHEMA,
> + optional: true,
> + },
> },
> },
> returns: {
> @@ -566,6 +570,7 @@ pub async fn get_status(
> pub async fn get_subscription_status(
> max_age: u64,
> verbose: bool,
> + view_filter: Option<String>,
> rpcenv: &mut dyn RpcEnvironment,
> ) -> Result<Vec<RemoteSubscriptions>, Error> {
> let (remotes_config, _) = pdm_config::remotes::config()?;
> @@ -574,9 +579,19 @@ 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, &["resources"], PRIV_RESOURCE_AUDIT, false)
> - .is_ok();
> +
> + let allow_all = if let Some(view_filter) = &view_filter {
> + user_info.check_privs(&auth_id, &["view", view_filter], PRIV_RESOURCE_AUDIT, false)?;
> + false
> + } else {
> + user_info
> + .check_privs(&auth_id, &["resources"], PRIV_RESOURCE_AUDIT, false)
> + .is_ok()
> + };
> +
> + let view_filter = view_filter
> + .map(|filter_name| views::view_filter::get_view_filter(&filter_name))
> + .transpose()?;
>
> let check_priv = |remote_name: &str| -> bool {
> user_info
> @@ -590,35 +605,62 @@ pub async fn get_subscription_status(
> };
>
> for (remote_name, remote) in remotes_config {
> - if !allow_all && !check_priv(&remote_name) {
> + if let Some(filter) = &view_filter {
> + if filter.can_skip_remote(&remote_name) {
> + continue;
> + }
> + } else if !allow_all && !check_priv(&remote_name) {
> continue;
> }
>
> + let view_filter_clone = view_filter.clone();
this could just be named 'view_filter' too, no need to postfix it with
'_clone'
> +
> let future = async move {
> let (node_status, error) =
> match get_subscription_info_for_remote(&remote, max_age).await {
> - Ok(node_status) => (Some(node_status), None),
> + Ok(mut node_status) => {
> + node_status.retain(|node, _| {
> + if let Some(filter) = &view_filter_clone {
> + filter.is_node_included(&remote.id, node)
> + } else {
> + true
> + }
> + });
> + (Some(node_status), None)
> + }
> Err(error) => (None, Some(error.to_string())),
> };
>
> - let mut state = RemoteSubscriptionState::Unknown;
> + let state = if let Some(node_status) = &node_status {
> + if error.is_some() && view_filter_clone.is_some() {
> + // Don't leak the existence of failed remotes, since we cannot apply
> + // view-filters here.
why not? we can check if the remote should be included, that does not
requires any more info?
> + return None;
> + }
>
> - if let Some(node_status) = &node_status {
> - state = map_node_subscription_list_to_state(node_status);
> - }
> + if node_status.is_empty() {
> + return None;
> + }
>
> - RemoteSubscriptions {
> + map_node_subscription_list_to_state(node_status)
> + } else {
> + RemoteSubscriptionState::Unknown
> + };
> +
> + Some(RemoteSubscriptions {
> remote: remote_name,
> error,
> state,
> node_status: if verbose { node_status } else { None },
> - }
> + })
> };
>
> futures.push(future);
> }
>
> - Ok(join_all(futures).await)
> + let status = join_all(futures).await.into_iter().flatten().collect();
> +
> + Ok(status)
> }
>
> // FIXME: make timeframe and count parameters?
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager v2 10/12] api: remote-tasks: add support for view-filter parameter
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 10/12] api: remote-tasks: " Lukas Wagner
@ 2025-11-05 10:09 ` Dominik Csapak
0 siblings, 0 replies; 32+ messages in thread
From: Dominik Csapak @ 2025-11-05 10:09 UTC (permalink / raw)
To: Proxmox Datacenter Manager development discussion, Lukas Wagner
Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
On 11/3/25 1:36 PM, Lukas Wagner wrote:
> A view filter 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-filter-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 filter.
>
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
> server/src/api/remote_tasks.rs | 36 +++++++++++++++++++++++++++----
> server/src/remote_tasks/mod.rs | 39 +++++++++++++++++++++++-----------
> 2 files changed, 59 insertions(+), 16 deletions(-)
>
> diff --git a/server/src/api/remote_tasks.rs b/server/src/api/remote_tasks.rs
> index 7b97b9cd..4a68c5d9 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_FILTER_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-filter": {
> + schema: VIEW_FILTER_ID_SCHEMA,
> + optional: true,
> + },
> +
> },
> },
> )]
> @@ -48,8 +54,17 @@ const SUBDIRS: SubdirMap = &sorted!([
> async fn list_tasks(
> filters: TaskFilters,
> remote: Option<String>,
> + view_filter: 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_filter) = &view_filter {
> + user_info.check_privs(&auth_id, &["view", view_filter], PRIV_RESOURCE_AUDIT, false)?;
> + }
> +
> + let tasks = remote_tasks::get_tasks(filters, remote, view_filter).await?;
>
> Ok(tasks)
> }
> @@ -70,6 +85,10 @@ async fn list_tasks(
> schema: REMOTE_ID_SCHEMA,
> optional: true,
> },
> + "view-filter": {
> + schema: VIEW_FILTER_ID_SCHEMA,
> + optional: true,
> + },
> },
> },
> )]
> @@ -77,8 +96,17 @@ async fn list_tasks(
> async fn task_statistics(
> filters: TaskFilters,
> remote: Option<String>,
> + view_filter: 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_filter) = &view_filter {
> + user_info.check_privs(&auth_id, &["view", view_filter], PRIV_RESOURCE_AUDIT, false)?;
> + }
> +
> + let tasks = remote_tasks::get_tasks(filters, remote, view_filter).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 c4939742..4d93d9b8 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,12 @@ const NUMBER_OF_UNCOMPRESSED_FILES: u32 = 2;
> pub async fn get_tasks(
> filters: TaskFilters,
> remote_filter: Option<String>,
> + view_filter: Option<String>,
> ) -> Result<Vec<TaskListItem>, Error> {
> + let view_filter = view_filter
> + .map(|filter_name| views::view_filter::get_view_filter(&filter_name))
> + .transpose()?;
> +
> tokio::task::spawn_blocking(move || {
> let cache = get_cache()?.read()?;
>
> @@ -52,21 +59,29 @@ pub async fn get_tasks(
> return None;
> }
> }
> +
> // TODO: Handle PBS tasks
> let pve_upid: Result<PveUpid, Error> = task.upid.upid.parse();
> match pve_upid {
> - Ok(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(pve_upid) => {
> + if let Some(view_filter) = &view_filter {
> + if !view_filter.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,
> + })
> + }
> Err(err) => {
> log::error!("could not parse UPID: {err:#}");
> None
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager v2 11/12] pdm-client: resource list: add view-filter parameter
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 11/12] pdm-client: resource list: add " Lukas Wagner
@ 2025-11-05 10:11 ` Dominik Csapak
0 siblings, 0 replies; 32+ messages in thread
From: Dominik Csapak @ 2025-11-05 10:11 UTC (permalink / raw)
To: Proxmox Datacenter Manager development discussion, Lukas Wagner
Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
On 11/3/25 1:36 PM, Lukas Wagner wrote:
> Signed-off-by: Lukas Wagner <l.wagner@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..7102ee05 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_filter: Option<&str>,
> + ) -> Result<Vec<RemoteResources>, Error> {
> let path = ApiPathBuilder::new("/api2/extjs/resources/list")
> .maybe_arg("max-age", &max_age)
> + .maybe_arg("view-filter", &view_filter)
> .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_filter: 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-filter", &view_filter)
> .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));
>
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager v2 12/12] pdm-client: top entities: add view-filter parameter
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 12/12] pdm-client: top entities: " Lukas Wagner
@ 2025-11-05 10:12 ` Dominik Csapak
0 siblings, 0 replies; 32+ messages in thread
From: Dominik Csapak @ 2025-11-05 10:12 UTC (permalink / raw)
To: Proxmox Datacenter Manager development discussion, Lukas Wagner
This'll need some changes in the gui since this moved to
ui/src/dashboard/view.rs
otherwise LGTM
On 11/3/25 1:35 PM, Lukas Wagner wrote:
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
> lib/pdm-client/src/lib.rs | 10 +++++++---
> ui/src/dashboard/mod.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 7102ee05..c9ed18cc 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_filter: Option<&str>) -> Result<TopEntities, Error> {
> + let builder = ApiPathBuilder::new("/api2/extjs/resources/top-entities".to_string())
> + .maybe_arg("view-filter", &view_filter);
> +
> + 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/mod.rs b/ui/src/dashboard/mod.rs
> index 07d5cd99..ce64d847 100644
> --- a/ui/src/dashboard/mod.rs
> +++ b/ui/src/dashboard/mod.rs
> @@ -319,7 +319,7 @@ impl PdmDashboard {
> let top_entities_future = {
> let link = link.clone();
> async move {
> - let res = client.get_top_entities().await;
> + let res = client.get_top_entities(None).await;
> link.send_message(Msg::LoadingFinished(LoadingResult::TopEntities(res)));
> }
> };
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager v2 02/12] pdm-config: views: add support for view-filters
2025-11-05 10:07 ` Dominik Csapak
@ 2025-11-05 10:37 ` Lukas Wagner
0 siblings, 0 replies; 32+ messages in thread
From: Lukas Wagner @ 2025-11-05 10:37 UTC (permalink / raw)
To: Dominik Csapak,
Proxmox Datacenter Manager development discussion, Lukas Wagner
On Wed Nov 5, 2025 at 11:07 AM CET, Dominik Csapak wrote:
> high level comments:
>
> would it be nicer if we use the ApiSectionDataEntry trait like we do for
> Remotes ?
Wasn't aware of this trait - will change it accordingly. Thanks!
> that way we can use the typed section config helpers
> (which are a bit more ergonomic to use later in the CRUD api calls)
>
> Also if we just use one file for the views, is it necessary
> to put it in a views folder?
Yeah, the structure was chosen due to the initial idea of separating the
filter definition and the view template.
>
> I'm currently thinking about how to save the layouts, and (as we
> recently discussed off-list) I'm leaning towards simply saving the
> layout as a json string into the config here, as that makes the
> loading/saving/updating/deleting much easier for both view filters
> and layouts (no need to track/lock both configs, etc.)
>
> We can of course leave it , but then we have to make sure to create
> the directory correctly :)
>
I have no hard feelings about this. Generally I'm fine with saving the
template-JSON data directly into this struct (we can still extend it
with 'externally' defined templates later and introduce a new key that
just contains the ID of that).
If we pack everthing into one struct, I'd probably rename it from
ViewFilter to just View (same for the API parameters).
So, for v3 I'd just do a plain `views.cfg` in the top-level config
dir.
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager v2 04/12] views: add implementation for view filters
2025-11-05 10:08 ` Dominik Csapak
@ 2025-11-05 10:58 ` Lukas Wagner
2025-11-05 11:48 ` Dominik Csapak
0 siblings, 1 reply; 32+ messages in thread
From: Lukas Wagner @ 2025-11-05 10:58 UTC (permalink / raw)
To: Dominik Csapak,
Proxmox Datacenter Manager development discussion, Lukas Wagner
On Wed Nov 5, 2025 at 11:08 AM CET, Dominik Csapak wrote:
>> +
>> +/// 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,
>> +}
>
> wouldn't a newtype suffice here too?
>
> pub struct ViewFilter(ViewFilterConfig)
Personally I'm not the biggest fan of newtypes unless I'm 100%
convinced that the type is only ever going to contain this single
member (simple example: pub struct UnixTimestamp(i64)), just to avoid
having to change any code when I have to introduce another member (e.g.
changing self.0 to self.config). Since it's only a very minor syntactic
thing, I'd be tempted to leave it as is - unless you insist I change it.
>
> ? alternatively, what about having freestanding functions that
> take a `&ViewFilterConfig` as parameter ?
>
I want to shield callers from the ViewFilterConfig itself (also the
reason why the `get_view_filter` fn exist, instead of having the caller
call directly into pdm_config) - so I'd prefer to keep this struct.
> If we're doing it this way though, I'd rather implement a
> From<ViewFilterConfig> for ViewFilter than a `new` method
> (or maybe both)
>
>> +
>> +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.
>
> isn't that (full) info more suited for the commit message than a comment?
>
> e.g. a single line comment with 'caching here is currently not worth it'
> and the full text in the commit message should also be ok?
>
> (no hard feelings though)
>
I'll move it to the commit message and leave a short summary in the
comment - thanks!
>> +
>> + 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(|e| {
>> + if let FilterRule::Remote(r) = e {
>> + r == remote
>> + } else {
>> + false
>> + }
>> + });
>> +
>> + (has_any_include_remote && !matches_any_include_remote && !any_other)
>> + || matches_any_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 filter.
>> + 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 check_rules(rules: &[FilterRule], remote: &str, resource: &ResourceData) -> bool {
>> + for rule in rules {
>> + let verdict = 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,
>> + };
>> +
>> + if verdict {
>> + return true;
>> + }
>> + }
>> +
>> + false
>
> wouldn't this boil down to:
>
> return rules.any(|rule| match rule { ... } ) ?
>
> instead of looping and doing an early return manually?
>
You are absolutely right - I'll change it as suggested.
>> +}
>> +
>> +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(),
>> + },
>> + }
>> + }
>> +}
>
> Is it really worht it, to define a seperate type that you only use
> internally?
> couldn't you simple use the &Resource type directly?
> or maybe just having 2 helper methods to extract the relevant info?
> (the type and global_id is already abstracted, so it's only relevant
> for the tags and the resource_pool ?)
>
> but i guess i'd have to see it to determine what is better...
The sole reason why there is this 'itermediary' type is the
'is_node_included' function. It is used when we don't handle resources,
but only really care about the node (e.g. task API). I didn't want to
'synthesize' some fake PveNode resource in the 'is_node_included'
function; it felt kind of wrong.
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager v2 05/12] views: add tests for view filter implementation
2025-11-05 10:08 ` Dominik Csapak
@ 2025-11-05 10:58 ` Lukas Wagner
0 siblings, 0 replies; 32+ messages in thread
From: Lukas Wagner @ 2025-11-05 10:58 UTC (permalink / raw)
To: Dominik Csapak,
Proxmox Datacenter Manager development discussion, Lukas Wagner
On Wed Nov 5, 2025 at 11:08 AM CET, Dominik Csapak wrote:
> IMHO this should be melded into the last commit
>
Ack, will do.
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager v2 04/12] views: add implementation for view filters
2025-11-05 10:58 ` Lukas Wagner
@ 2025-11-05 11:48 ` Dominik Csapak
0 siblings, 0 replies; 32+ messages in thread
From: Dominik Csapak @ 2025-11-05 11:48 UTC (permalink / raw)
To: Lukas Wagner, Proxmox Datacenter Manager development discussion
On 11/5/25 11:57 AM, Lukas Wagner wrote:
> On Wed Nov 5, 2025 at 11:08 AM CET, Dominik Csapak wrote:
>>> +
>>> +/// 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,
>>> +}
>>
>> wouldn't a newtype suffice here too?
>>
>> pub struct ViewFilter(ViewFilterConfig)
>
> Personally I'm not the biggest fan of newtypes unless I'm 100%
> convinced that the type is only ever going to contain this single
> member (simple example: pub struct UnixTimestamp(i64)), just to avoid
> having to change any code when I have to introduce another member (e.g.
> changing self.0 to self.config). Since it's only a very minor syntactic
> thing, I'd be tempted to leave it as is - unless you insist I change it.
>
>>
>> ? alternatively, what about having freestanding functions that
>> take a `&ViewFilterConfig` as parameter ?
>>
>
> I want to shield callers from the ViewFilterConfig itself (also the
> reason why the `get_view_filter` fn exist, instead of having the caller
> call directly into pdm_config) - so I'd prefer to keep this struct.
make sense, so just keep it as is
>
>> If we're doing it this way though, I'd rather implement a
>> From<ViewFilterConfig> for ViewFilter than a `new` method
>> (or maybe both)
>>
>>> +
>>> +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.
>>
>> isn't that (full) info more suited for the commit message than a comment?
>>
>> e.g. a single line comment with 'caching here is currently not worth it'
>> and the full text in the commit message should also be ok?
>>
>> (no hard feelings though)
>>
>
> I'll move it to the commit message and leave a short summary in the
> comment - thanks!
>
>>> +
>>> + 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(|e| {
>>> + if let FilterRule::Remote(r) = e {
>>> + r == remote
>>> + } else {
>>> + false
>>> + }
>>> + });
>>> +
>>> + (has_any_include_remote && !matches_any_include_remote && !any_other)
>>> + || matches_any_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 filter.
>>> + 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 check_rules(rules: &[FilterRule], remote: &str, resource: &ResourceData) -> bool {
>>> + for rule in rules {
>>> + let verdict = 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,
>>> + };
>>> +
>>> + if verdict {
>>> + return true;
>>> + }
>>> + }
>>> +
>>> + false
>>
>> wouldn't this boil down to:
>>
>> return rules.any(|rule| match rule { ... } ) ?
>>
>> instead of looping and doing an early return manually?
>>
>
> You are absolutely right - I'll change it as suggested.
>
>>> +}
>>> +
>>> +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(),
>>> + },
>>> + }
>>> + }
>>> +}
>>
>> Is it really worht it, to define a seperate type that you only use
>> internally?
>> couldn't you simple use the &Resource type directly?
>> or maybe just having 2 helper methods to extract the relevant info?
>> (the type and global_id is already abstracted, so it's only relevant
>> for the tags and the resource_pool ?)
>>
>> but i guess i'd have to see it to determine what is better...
>
> The sole reason why there is this 'itermediary' type is the
> 'is_node_included' function. It is used when we don't handle resources,
> but only really care about the node (e.g. task API). I didn't want to
> 'synthesize' some fake PveNode resource in the 'is_node_included'
> function; it felt kind of wrong.
mhmm i don't think creating a fake `PveNode` resource is any more wrong
that creating a 'fake' `ResourceData` in place, but again no hard
feelings. I probably would have to see both approaches side-by-side
to see which is better, but it's not super important to change for now
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager v2 09/12] api: subscription status: add support for view-filter parameter
2025-11-05 10:08 ` Dominik Csapak
@ 2025-11-05 14:11 ` Lukas Wagner
2025-11-05 14:28 ` Dominik Csapak
0 siblings, 1 reply; 32+ messages in thread
From: Lukas Wagner @ 2025-11-05 14:11 UTC (permalink / raw)
To: Dominik Csapak,
Proxmox Datacenter Manager development discussion, Lukas Wagner
On Wed Nov 5, 2025 at 11:08 AM CET, Dominik Csapak wrote:
>> @@ -590,35 +605,62 @@ pub async fn get_subscription_status(
>> };
>>
>> for (remote_name, remote) in remotes_config {
>> - if !allow_all && !check_priv(&remote_name) {
>> + if let Some(filter) = &view_filter {
>> + if filter.can_skip_remote(&remote_name) {
>> + continue;
>> + }
>> + } else if !allow_all && !check_priv(&remote_name) {
>> continue;
>> }
>>
>> + let view_filter_clone = view_filter.clone();
>
> this could just be named 'view_filter' too, no need to postfix it with
> '_clone'
Ack - will do for v3.
>
>> +
>> let future = async move {
>> let (node_status, error) =
>> match get_subscription_info_for_remote(&remote, max_age).await {
>> - Ok(node_status) => (Some(node_status), None),
>> + Ok(mut node_status) => {
>> + node_status.retain(|node, _| {
>> + if let Some(filter) = &view_filter_clone {
>> + filter.is_node_included(&remote.id, node)
>> + } else {
>> + true
>> + }
>> + });
>> + (Some(node_status), None)
>> + }
>> Err(error) => (None, Some(error.to_string())),
>> };
>>
>> - let mut state = RemoteSubscriptionState::Unknown;
>> + let state = if let Some(node_status) = &node_status {
>> + if error.is_some() && view_filter_clone.is_some() {
>> + // Don't leak the existence of failed remotes, since we cannot apply
>> + // view-filters here.
>
> why not? we can check if the remote should be included, that does not
> requires any more info?
>
>
The problem is that we cannot always reliably tell if a remote is
included or not without checking the remote's resources. There is
ViewFilter::can_skip_remote - but since that one only checks
`include/exclude remote:...` rules, it might return false (meaning,
the remote must be considered) even if at the end the data from this
remote might be filtered out due to the other rules (and if no resources
remain, the remote as a whole would then be filtered out from the
response - similar to what I did in patch 6 to which you replied).
Example:
Consider only having `include tag:sometag` in the config. From this
alone, we cannot exclude any remotes from being queried a priori and
must do *all* filtering when we already have the data.
Similar thing here. If we cannot reach the remote, then we don't have a
list of nodes for this remote. Without the list of nodes, we cannot do
any checks with the node resource. This means if we do not filter out
the failed remote, we would leak its existence in a view which
potentially would not match on any of the node's resources.
I hope you could follow this explanation. Do you maybe have an idea on
how to fix this?
Only thing that comes to mind is making the include/exclude
remote:{remote} rules required, so that a view *always* needs to specify
which remotes to (not) consider. If we make these required, we probably
should make these a separate key in the config file again.
>> + return None;
>> + }
>>
>> - if let Some(node_status) = &node_status {
>> - state = map_node_subscription_list_to_state(node_status);
>> - }
>> + if node_status.is_empty() {
>> + return None;
>> + }
>>
>> - RemoteSubscriptions {
>> + map_node_subscription_list_to_state(node_status)
>> + } else {
>> + RemoteSubscriptionState::Unknown
>> + };
>> +
>> + Some(RemoteSubscriptions {
>> remote: remote_name,
>> error,
>> state,
>> node_status: if verbose { node_status } else { None },
>> - }
>> + })
>> };
>>
>> futures.push(future);
>> }
>>
>> - Ok(join_all(futures).await)
>> + let status = join_all(futures).await.into_iter().flatten().collect();
>> +
>> + Ok(status)
>> }
>>
>> // FIXME: make timeframe and count parameters?
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager v2 09/12] api: subscription status: add support for view-filter parameter
2025-11-05 14:11 ` Lukas Wagner
@ 2025-11-05 14:28 ` Dominik Csapak
2025-11-05 14:35 ` Lukas Wagner
0 siblings, 1 reply; 32+ messages in thread
From: Dominik Csapak @ 2025-11-05 14:28 UTC (permalink / raw)
To: Lukas Wagner, Proxmox Datacenter Manager development discussion
On 11/5/25 3:11 PM, Lukas Wagner wrote:
> On Wed Nov 5, 2025 at 11:08 AM CET, Dominik Csapak wrote:
>>> @@ -590,35 +605,62 @@ pub async fn get_subscription_status(
>>> };
>>>
>>> for (remote_name, remote) in remotes_config {
>>> - if !allow_all && !check_priv(&remote_name) {
>>> + if let Some(filter) = &view_filter {
>>> + if filter.can_skip_remote(&remote_name) {
>>> + continue;
>>> + }
>>> + } else if !allow_all && !check_priv(&remote_name) {
>>> continue;
>>> }
>>>
>>> + let view_filter_clone = view_filter.clone();
>>
>> this could just be named 'view_filter' too, no need to postfix it with
>> '_clone'
>
> Ack - will do for v3.
>
>>
>>> +
>>> let future = async move {
>>> let (node_status, error) =
>>> match get_subscription_info_for_remote(&remote, max_age).await {
>>> - Ok(node_status) => (Some(node_status), None),
>>> + Ok(mut node_status) => {
>>> + node_status.retain(|node, _| {
>>> + if let Some(filter) = &view_filter_clone {
>>> + filter.is_node_included(&remote.id, node)
>>> + } else {
>>> + true
>>> + }
>>> + });
>>> + (Some(node_status), None)
>>> + }
>>> Err(error) => (None, Some(error.to_string())),
>>> };
>>>
>>> - let mut state = RemoteSubscriptionState::Unknown;
>>> + let state = if let Some(node_status) = &node_status {
>>> + if error.is_some() && view_filter_clone.is_some() {
>>> + // Don't leak the existence of failed remotes, since we cannot apply
>>> + // view-filters here.
>>
>> why not? we can check if the remote should be included, that does not
>> requires any more info?
>>
>>
>
> The problem is that we cannot always reliably tell if a remote is
> included or not without checking the remote's resources. There is
> ViewFilter::can_skip_remote - but since that one only checks
> `include/exclude remote:...` rules, it might return false (meaning,
> the remote must be considered) even if at the end the data from this
> remote might be filtered out due to the other rules (and if no resources
> remain, the remote as a whole would then be filtered out from the
> response - similar to what I did in patch 6 to which you replied).
>
> Example:
>
> Consider only having `include tag:sometag` in the config. From this
> alone, we cannot exclude any remotes from being queried a priori and
> must do *all* filtering when we already have the data.
>
> Similar thing here. If we cannot reach the remote, then we don't have a
> list of nodes for this remote. Without the list of nodes, we cannot do
> any checks with the node resource. This means if we do not filter out
> the failed remote, we would leak its existence in a view which
> potentially would not match on any of the node's resources.
>
> I hope you could follow this explanation. Do you maybe have an idea on
> how to fix this?
>
> Only thing that comes to mind is making the include/exclude
> remote:{remote} rules required, so that a view *always* needs to specify
> which remotes to (not) consider. If we make these required, we probably
> should make these a separate key in the config file again.
>
ok i understand the issue, but instead of excluding all remotes that
have errors, we could at least include those that we know should be
included by the 'remote' filter?
IMHO a remote is already a resource, so if i include some remote with
it's name (or by type, etc.), even if it does not contain any sub
resources, i should get back an (empty) remote
does that make sense?
i get we can't include/exclude based on other filters though, that
must remain hidden as you mentioned, but for explicitly included
remotes we should include them also with errors.
>>> + return None;
>>> + }
>>>
>>> - if let Some(node_status) = &node_status {
>>> - state = map_node_subscription_list_to_state(node_status);
>>> - }
>>> + if node_status.is_empty() {
>>> + return None;
>>> + }
>>>
>>> - RemoteSubscriptions {
>>> + map_node_subscription_list_to_state(node_status)
>>> + } else {
>>> + RemoteSubscriptionState::Unknown
>>> + };
>>> +
>>> + Some(RemoteSubscriptions {
>>> remote: remote_name,
>>> error,
>>> state,
>>> node_status: if verbose { node_status } else { None },
>>> - }
>>> + })
>>> };
>>>
>>> futures.push(future);
>>> }
>>>
>>> - Ok(join_all(futures).await)
>>> + let status = join_all(futures).await.into_iter().flatten().collect();
>>> +
>>> + Ok(status)
>>> }
>>>
>>> // FIXME: make timeframe and count parameters?
>
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager v2 09/12] api: subscription status: add support for view-filter parameter
2025-11-05 14:28 ` Dominik Csapak
@ 2025-11-05 14:35 ` Lukas Wagner
0 siblings, 0 replies; 32+ messages in thread
From: Lukas Wagner @ 2025-11-05 14:35 UTC (permalink / raw)
To: Dominik Csapak, Lukas Wagner,
Proxmox Datacenter Manager development discussion
On Wed Nov 5, 2025 at 3:28 PM CET, Dominik Csapak wrote:
> On 11/5/25 3:11 PM, Lukas Wagner wrote:
>> On Wed Nov 5, 2025 at 11:08 AM CET, Dominik Csapak wrote:
>>>>
>>>> - let mut state = RemoteSubscriptionState::Unknown;
>>>> + let state = if let Some(node_status) = &node_status {
>>>> + if error.is_some() && view_filter_clone.is_some() {
>>>> + // Don't leak the existence of failed remotes, since we cannot apply
>>>> + // view-filters here.
>>>
>>> why not? we can check if the remote should be included, that does not
>>> requires any more info?
>>>
>>>
>>
>> The problem is that we cannot always reliably tell if a remote is
>> included or not without checking the remote's resources. There is
>> ViewFilter::can_skip_remote - but since that one only checks
>> `include/exclude remote:...` rules, it might return false (meaning,
>> the remote must be considered) even if at the end the data from this
>> remote might be filtered out due to the other rules (and if no resources
>> remain, the remote as a whole would then be filtered out from the
>> response - similar to what I did in patch 6 to which you replied).
>>
>> Example:
>>
>> Consider only having `include tag:sometag` in the config. From this
>> alone, we cannot exclude any remotes from being queried a priori and
>> must do *all* filtering when we already have the data.
>>
>> Similar thing here. If we cannot reach the remote, then we don't have a
>> list of nodes for this remote. Without the list of nodes, we cannot do
>> any checks with the node resource. This means if we do not filter out
>> the failed remote, we would leak its existence in a view which
>> potentially would not match on any of the node's resources.
>>
>> I hope you could follow this explanation. Do you maybe have an idea on
>> how to fix this?
>>
>> Only thing that comes to mind is making the include/exclude
>> remote:{remote} rules required, so that a view *always* needs to specify
>> which remotes to (not) consider. If we make these required, we probably
>> should make these a separate key in the config file again.
>>
>
> ok i understand the issue, but instead of excluding all remotes that
> have errors, we could at least include those that we know should be
> included by the 'remote' filter?
>
> IMHO a remote is already a resource, so if i include some remote with
> it's name (or by type, etc.), even if it does not contain any sub
> resources, i should get back an (empty) remote
>
> does that make sense?
>
> i get we can't include/exclude based on other filters though, that
> must remain hidden as you mentioned, but for explicitly included
> remotes we should include them also with errors.
>
Yeah, I think this is a reasonable approach. I'll try this for v3.
Thanks for the feedback, Dominik!
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2025-11-05 14:34 UTC | newest]
Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-03 12:35 [pdm-devel] [PATCH datacenter-manager v2 00/12] backend implementation for view filters Lukas Wagner
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 01/12] pdm-api-types: views: add ViewFilterConfig type Lukas Wagner
2025-11-05 10:07 ` Dominik Csapak
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 02/12] pdm-config: views: add support for view-filters Lukas Wagner
2025-11-05 10:07 ` Dominik Csapak
2025-11-05 10:37 ` Lukas Wagner
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 03/12] acl: add '/view' and '/view/{view-id}' as allowed ACL paths Lukas Wagner
2025-11-05 10:07 ` Dominik Csapak
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 04/12] views: add implementation for view filters Lukas Wagner
2025-11-05 10:08 ` Dominik Csapak
2025-11-05 10:58 ` Lukas Wagner
2025-11-05 11:48 ` Dominik Csapak
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 05/12] views: add tests for view filter implementation Lukas Wagner
2025-11-05 10:08 ` Dominik Csapak
2025-11-05 10:58 ` Lukas Wagner
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 06/12] api: resources: list: add support for view-filter parameter Lukas Wagner
2025-11-05 10:08 ` Dominik Csapak
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 07/12] api: resources: top entities: " Lukas Wagner
2025-11-05 10:08 ` Dominik Csapak
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 08/12] api: resources: status: " Lukas Wagner
2025-11-05 10:08 ` Dominik Csapak
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 09/12] api: subscription " Lukas Wagner
2025-11-05 10:08 ` Dominik Csapak
2025-11-05 14:11 ` Lukas Wagner
2025-11-05 14:28 ` Dominik Csapak
2025-11-05 14:35 ` Lukas Wagner
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 10/12] api: remote-tasks: " Lukas Wagner
2025-11-05 10:09 ` Dominik Csapak
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 11/12] pdm-client: resource list: add " Lukas Wagner
2025-11-05 10:11 ` Dominik Csapak
2025-11-03 12:35 ` [pdm-devel] [PATCH datacenter-manager v2 12/12] pdm-client: top entities: " Lukas Wagner
2025-11-05 10:12 ` Dominik Csapak
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox