* [pdm-devel] [PATCH datacenter-manager v3 00/11] backend implementation for view filters
@ 2025-11-06 13:43 Lukas Wagner
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 01/11] pdm-api-types: views: add ViewConfig type Lukas Wagner
` (12 more replies)
0 siblings, 13 replies; 23+ messages in thread
From: Lukas Wagner @ 2025-11-06 13:43 UTC (permalink / raw)
To: pdm-devel
Key aspects:
- new config file at /etc/proxmox-datacenter-manager/views.cfg
- ViewConfig as a definition type, has
- {include,exclude} {remote,resource-id,resource-type,resource-pool,tag}:{value}
- View resource filter implementation & big suite of unit tests
- excludes are processed after includes
- if no include rules are defined, all resources but those which were
excluded are matched
- if no rules are defined in a filter, everything is matched
- Added the 'view' parameter to a couple of API endpoints
- /resources/list
- /resources/status
- /resources/subscription
- /resources/top-entities
- /remote-tasks/list
- /remote-tasks/statistis
- ACL checks are done on /view/{view-filter-id} for now, replace
any other permission check in the handler iff the view-filter paramter
is set
Left to do:
- CRUD for filter definition
- UI for filter rules
Changes since v2 (thx for the review @Dominik):
- Renamed:
- ViewFilter -> View
- ViewFilterConfig -> ViewConfig
- 'view-filter' parameters to 'view'
- Use ApiSectionDataEntry trait for ViewConfig
- Use SAFE_ID_FORMAT to validate filter values
- Changed path for config file from `views/filter.cfg` to just `views.cfg`
- Include failed remotes in API responses iff they have been explicitly included
via `include remote:<...>`. Previously, they had been filtered out to avoid
leaking the existence of remotes, but if they have been explicitly included,
this is fine.
- Simplify `check_rules` function by using `.any()` instead of looping manually
- Merged the rule implementation and test commits
- Added views::get_optional_view as a convenience helper. This one is similar to
views::get_view, but accepts Option<&str> and returns Option<View>.
Changes since v1 (RFC):
- Change config key structure, moving the type into the value
e.g.
include-remote foo
became
include remote:foo
- Minor fixes from the review (thanks Wolfgang & Shannon)
proxmox-datacenter-manager:
Lukas Wagner (11):
pdm-api-types: views: add ViewConfig type
pdm-config: views: add support for views
acl: add '/view' and '/view/{view-id}' as allowed ACL paths
views: add implementation for view resource filtering
api: resources: list: add support for view parameter
api: resources: top entities: add support for view parameter
api: resources: status: add support for view parameter
api: subscription status: add support for view parameter
api: remote-tasks: add support for view parameter
pdm-client: resource list: add view-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 | 202 ++++++
lib/pdm-client/src/lib.rs | 19 +-
lib/pdm-config/src/lib.rs | 2 +-
lib/pdm-config/src/views.rs | 17 +
server/src/acl.rs | 6 +
server/src/api/remote_tasks.rs | 36 +-
server/src/api/resources.rs | 166 ++++-
server/src/lib.rs | 1 +
server/src/metric_collection/top_entities.rs | 5 +
server/src/remote_tasks/mod.rs | 37 +-
server/src/resource_cache.rs | 3 +-
server/src/views/mod.rs | 205 ++++++
server/src/views/tests.rs | 619 +++++++++++++++++++
ui/src/dashboard/view.rs | 2 +-
ui/src/sdn/zone_tree.rs | 2 +-
17 files changed, 1283 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
Summary over all repositories:
17 files changed, 1283 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] 23+ messages in thread
* [pdm-devel] [PATCH datacenter-manager v3 01/11] pdm-api-types: views: add ViewConfig type
2025-11-06 13:43 [pdm-devel] [PATCH datacenter-manager v3 00/11] backend implementation for view filters Lukas Wagner
@ 2025-11-06 13:43 ` Lukas Wagner
2025-11-11 10:57 ` Michael Köppl
2025-11-11 10:58 ` Michael Köppl
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 02/11] pdm-config: views: add support for views Lukas Wagner
` (11 subsequent siblings)
12 siblings, 2 replies; 23+ messages in thread
From: Lukas Wagner @ 2025-11-06 13:43 UTC (permalink / raw)
To: pdm-devel
This type will be used to define views.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
Notes:
Changes since v2:
- use ApiSectionDataEntry, as suggested by Dominik
- Renamed ViewFilter* -> View*
- Use SAFE_ID_FORMAT to check filter values where possible
Changes since RFC:
- Change config structure, instead of 'include-{x} value' we now do
'include {x}:value'
lib/pdm-api-types/src/lib.rs | 8 ++
lib/pdm-api-types/src/views.rs | 202 +++++++++++++++++++++++++++++++++
2 files changed, 210 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 ee4dfb2b..20327aba 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_:, +!\-@=.]+$";
@@ -153,6 +155,12 @@ pub const REALM_ID_SCHEMA: Schema = StringSchema::new("Realm name.")
.max_length(32)
.schema();
+pub const VIEW_ID_SCHEMA: Schema = StringSchema::new("View name.")
+ .format(&PROXMOX_SAFE_ID_FORMAT)
+ .min_length(2)
+ .max_length(32)
+ .schema();
+
pub const VMID_SCHEMA: Schema = IntegerSchema::new("A guest ID").minimum(1).schema();
pub const SNAPSHOT_NAME_SCHEMA: Schema = StringSchema::new("The name of the snapshot")
.format(&PROXMOX_SAFE_ID_FORMAT)
diff --git a/lib/pdm-api-types/src/views.rs b/lib/pdm-api-types/src/views.rs
new file mode 100644
index 00000000..165b29d6
--- /dev/null
+++ b/lib/pdm-api-types/src/views.rs
@@ -0,0 +1,202 @@
+use std::{fmt::Display, str::FromStr, sync::OnceLock};
+
+use anyhow::bail;
+use proxmox_section_config::{typed::ApiSectionDataEntry, SectionConfig, SectionConfigPlugin};
+use serde::{Deserialize, Serialize};
+
+use proxmox_schema::{
+ api, api_types::SAFE_ID_FORMAT, ApiStringFormat, ApiType, ArraySchema, Schema, StringSchema,
+ Updater,
+};
+
+use crate::{remotes::REMOTE_ID_SCHEMA, resource::ResourceType, VIEW_ID_SCHEMA};
+
+/// Schema for filter rules.
+pub const FILTER_RULE_SCHEMA: Schema = StringSchema::new("Filter rule for resources.")
+ .format(&ApiStringFormat::VerifyFn(verify_filter_rule))
+ .type_text(
+ "resource-type:storage|qemu|lxc|sdn-zone|datastore|node>\
+ |resource-pool:<pool-name>\
+ |tag:<tag-name>\
+ |remote:<remote-name>\
+ |resource-id:<resource-id>",
+ )
+ .schema();
+
+/// Schema for list of filter rules.
+pub const FILTER_RULE_LIST_SCHEMA: Schema =
+ ArraySchema::new("List of filter rules.", &FILTER_RULE_SCHEMA).schema();
+
+#[api(
+ properties: {
+ "id": {
+ schema: VIEW_ID_SCHEMA,
+ },
+ "include": {
+ schema: FILTER_RULE_LIST_SCHEMA,
+ optional: true,
+ },
+ "exclude": {
+ schema: FILTER_RULE_LIST_SCHEMA,
+ optional: true,
+ }
+ }
+)]
+#[derive(Clone, Debug, Default, Deserialize, Serialize, Updater, PartialEq)]
+#[serde(rename_all = "kebab-case")]
+/// View definition
+pub struct ViewConfig {
+ /// View name.
+ #[updater(skip)]
+ pub id: String,
+
+ /// List of includes.
+ #[serde(default, skip_serializing_if = "Vec::is_empty")]
+ #[updater(serde(skip_serializing_if = "Option::is_none"))]
+ pub include: Vec<FilterRule>,
+
+ /// List of excludes.
+ #[serde(default, skip_serializing_if = "Vec::is_empty")]
+ #[updater(serde(skip_serializing_if = "Option::is_none"))]
+ pub exclude: Vec<FilterRule>,
+}
+
+#[derive(Clone, Debug, Deserialize, Serialize, PartialEq)]
+#[serde(rename_all = "kebab-case")]
+/// Enum for the different sections in the 'views.cfg' file.
+pub enum ViewConfigEntry {
+ /// 'view' section
+ View(ViewConfig),
+}
+
+const VIEW_SECTION_NAME: &str = "view";
+
+impl ApiSectionDataEntry for ViewConfigEntry {
+ fn section_config() -> &'static SectionConfig {
+ static CONFIG: OnceLock<SectionConfig> = OnceLock::new();
+
+ CONFIG.get_or_init(|| {
+ let mut this = SectionConfig::new(&VIEW_ID_SCHEMA);
+
+ this.register_plugin(SectionConfigPlugin::new(
+ VIEW_SECTION_NAME.into(),
+ Some("id".to_string()),
+ ViewConfig::API_SCHEMA.unwrap_object_schema(),
+ ));
+ this
+ })
+ }
+
+ fn section_type(&self) -> &'static str {
+ match self {
+ ViewConfigEntry::View(_) => VIEW_SECTION_NAME,
+ }
+ }
+}
+
+#[derive(Clone, Debug, PartialEq)]
+/// Filter rule for includes/excludes.
+pub enum FilterRule {
+ /// Match a resource type.
+ ResourceType(ResourceType),
+ /// Match a resource pools (for PVE guests).
+ ResourcePool(String),
+ /// Match a (global) resource ID, e.g. 'remote/<remote>/guest/<vmid>'.
+ ResourceId(String),
+ /// Match a tag (for PVE guests).
+ Tag(String),
+ /// Match a remote.
+ Remote(String),
+}
+
+impl FromStr for FilterRule {
+ type Err = anyhow::Error;
+
+ fn from_str(s: &str) -> Result<Self, Self::Err> {
+ Ok(match s.split_once(':') {
+ Some(("resource-type", value)) => FilterRule::ResourceType(value.parse()?),
+ Some(("resource-pool", value)) => {
+ if !SAFE_ID_FORMAT.unwrap_pattern_format().is_match(value) {
+ bail!("invalid tag value: {value}");
+ }
+ FilterRule::ResourcePool(value.to_string())
+ }
+ Some(("resource-id", value)) => {
+ // TODO: Define schema and use it to validate. Can't use SAFE_ID_FORMAT since it does
+ // not allow '/'.
+ if value.is_empty() {
+ bail!("empty resource-id rule not allowed");
+ }
+ FilterRule::ResourceId(value.to_string())
+ }
+ Some(("tag", value)) => {
+ if !SAFE_ID_FORMAT.unwrap_pattern_format().is_match(value) {
+ bail!("invalid tag value: {value}");
+ }
+ FilterRule::Tag(value.to_string())
+ }
+ Some(("remote", value)) => {
+ let _ = REMOTE_ID_SCHEMA.parse_simple_value(value)?;
+ FilterRule::Remote(value.to_string())
+ }
+ Some((ty, _)) => bail!("invalid type: {ty}"),
+ None => bail!("invalid filter rule: {s}"),
+ })
+ }
+}
+
+// used for serializing below, caution!
+impl Display for FilterRule {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ match self {
+ FilterRule::ResourceType(resource_type) => write!(f, "resource-type:{resource_type}"),
+ FilterRule::ResourcePool(pool) => write!(f, "resource-pool:{pool}"),
+ FilterRule::ResourceId(id) => write!(f, "resource-id:{id}"),
+ FilterRule::Tag(tag) => write!(f, "tag:{tag}"),
+ FilterRule::Remote(remote) => write!(f, "remote:{remote}"),
+ }
+ }
+}
+
+proxmox_serde::forward_deserialize_to_from_str!(FilterRule);
+proxmox_serde::forward_serialize_to_display!(FilterRule);
+
+fn verify_filter_rule(input: &str) -> Result<(), anyhow::Error> {
+ FilterRule::from_str(input).map(|_| ())
+}
+
+#[cfg(test)]
+mod test {
+ use anyhow::Error;
+
+ use crate::views::FilterRule;
+
+ fn parse_and_check_display(input: &str) -> Result<bool, Error> {
+ let rule: FilterRule = input.parse()?;
+
+ Ok(input == format!("{rule}"))
+ }
+
+ #[test]
+ fn test_filter_rule() {
+ assert!(parse_and_check_display("abc").is_err());
+ assert!(parse_and_check_display("abc:").is_err());
+
+ assert!(parse_and_check_display("resource-type:").is_err());
+ assert!(parse_and_check_display("resource-type:lxc").unwrap());
+ assert!(parse_and_check_display("resource-type:qemu").unwrap());
+ assert!(parse_and_check_display("resource-type:abc").is_err());
+
+ assert!(parse_and_check_display("resource-pool:").is_err());
+ assert!(parse_and_check_display("resource-pool:somepool").unwrap());
+
+ assert!(parse_and_check_display("resource-id:").is_err());
+ assert!(parse_and_check_display("resource-id:remote/someremote/guest/100").unwrap());
+
+ assert!(parse_and_check_display("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] 23+ messages in thread
* [pdm-devel] [PATCH datacenter-manager v3 02/11] pdm-config: views: add support for views
2025-11-06 13:43 [pdm-devel] [PATCH datacenter-manager v3 00/11] backend implementation for view filters Lukas Wagner
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 01/11] pdm-api-types: views: add ViewConfig type Lukas Wagner
@ 2025-11-06 13:43 ` Lukas Wagner
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 03/11] acl: add '/view' and '/view/{view-id}' as allowed ACL paths Lukas Wagner
` (10 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Lukas Wagner @ 2025-11-06 13:43 UTC (permalink / raw)
To: pdm-devel
This allows to read ViewConfig entries from a new config file at
/etc/proxmox-datacenter-manager/views.cfg
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
Notes:
Changes since v2:
- Change config path to 'views.cfg' instead of 'views/filters.cfg'
lib/pdm-config/src/lib.rs | 2 +-
lib/pdm-config/src/views.rs | 17 +++++++++++++++++
2 files changed, 18 insertions(+), 1 deletion(-)
create mode 100644 lib/pdm-config/src/views.rs
diff --git a/lib/pdm-config/src/lib.rs b/lib/pdm-config/src/lib.rs
index ac398cab..4c490541 100644
--- a/lib/pdm-config/src/lib.rs
+++ b/lib/pdm-config/src/lib.rs
@@ -1,6 +1,5 @@
use anyhow::{format_err, Error};
use nix::unistd::{Gid, Group, Uid, User};
-
pub use pdm_buildcfg::{BACKUP_GROUP_NAME, BACKUP_USER_NAME};
pub mod certificate_config;
@@ -8,6 +7,7 @@ pub mod domains;
pub mod node;
pub mod remotes;
pub mod setup;
+pub mod views;
mod config_version_cache;
pub use config_version_cache::ConfigVersionCache;
diff --git a/lib/pdm-config/src/views.rs b/lib/pdm-config/src/views.rs
new file mode 100644
index 00000000..59c02a66
--- /dev/null
+++ b/lib/pdm-config/src/views.rs
@@ -0,0 +1,17 @@
+use anyhow::Error;
+
+use proxmox_section_config::typed::{ApiSectionDataEntry, SectionConfigData};
+
+use pdm_api_types::views::ViewConfigEntry;
+
+use pdm_buildcfg::configdir;
+
+const VIEW_CFG_FILENAME: &str = configdir!("/views.cfg");
+
+/// Get the `views.cfg` config file contents.
+pub fn config() -> Result<SectionConfigData<ViewConfigEntry>, Error> {
+ let content =
+ proxmox_sys::fs::file_read_optional_string(VIEW_CFG_FILENAME)?.unwrap_or_default();
+
+ ViewConfigEntry::parse_section_config(VIEW_CFG_FILENAME, &content)
+}
--
2.47.3
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 23+ messages in thread
* [pdm-devel] [PATCH datacenter-manager v3 03/11] acl: add '/view' and '/view/{view-id}' as allowed ACL paths
2025-11-06 13:43 [pdm-devel] [PATCH datacenter-manager v3 00/11] backend implementation for view filters Lukas Wagner
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 01/11] pdm-api-types: views: add ViewConfig type Lukas Wagner
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 02/11] pdm-config: views: add support for views Lukas Wagner
@ 2025-11-06 13:43 ` Lukas Wagner
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 04/11] views: add implementation for view resource filtering Lukas Wagner
` (9 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Lukas Wagner @ 2025-11-06 13:43 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>
Reviewed-by: Dominik Csapak <d.csapak@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] 23+ messages in thread
* [pdm-devel] [PATCH datacenter-manager v3 04/11] views: add implementation for view resource filtering
2025-11-06 13:43 [pdm-devel] [PATCH datacenter-manager v3 00/11] backend implementation for view filters Lukas Wagner
` (2 preceding siblings ...)
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 03/11] acl: add '/view' and '/view/{view-id}' as allowed ACL paths Lukas Wagner
@ 2025-11-06 13:43 ` Lukas Wagner
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 05/11] api: resources: list: add support for view parameter Lukas Wagner
` (8 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Lukas Wagner @ 2025-11-06 13:43 UTC (permalink / raw)
To: pdm-devel
This commit adds the resource filter implementation for the previously
defined ViewConfig type.
There are include/exclude rules for the following properties:
- (global) resource-id
- resource pool
- resource type
- remote
- tags
The rules are interpreted as follows:
- no rules: everything matches
- only includes: included resources match
- only excluded: everything *but* the excluded resources match
- include and exclude: excludes are applied *after* includes, meaning if
one has a `include-remote foo` and `exclude-remote foo` at the same
time, the remote `foo` will never match
Some experiments were performed with a cache that works roughly as following:
- HashMap<ViewId, HashMap<ResourceId, bool>> in a mutex
- Cache invalidated if view config digest changed
- Cache invalidated if certain resource fields such as tags or resource pools change
from the last time (also with a digest-based implementation)
Experimented with the `fake-remote` feature and and 15000 guests showed
that caching was only faster than direct evaluation if the number of
rules in the ViewConfig is *huge* (e.g. >1000 `include-resource-id`
entries). But even for those, direct evaluation was always plenty fast,
with evaluation times ~20ms for *all* resources.
-> for any *realistic* view config, we should be good with direct
evaluation, as long as we don't add any filter rules which are very
expensive to evaluate.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
Notes:
Changes since v2:
- add is_remote_explicitly_included helper
- merge commit that added tests into this one
- simply `check_rules` using .any on the rules vec
- add get_optional_view helper
- rename ViewFilter* -> View*
server/src/lib.rs | 1 +
server/src/views/mod.rs | 205 +++++++++++++
server/src/views/tests.rs | 619 ++++++++++++++++++++++++++++++++++++++
3 files changed, 825 insertions(+)
create mode 100644 server/src/views/mod.rs
create mode 100644 server/src/views/tests.rs
diff --git a/server/src/lib.rs b/server/src/lib.rs
index 964807eb..0f25aa71 100644
--- a/server/src/lib.rs
+++ b/server/src/lib.rs
@@ -12,6 +12,7 @@ pub mod remote_tasks;
pub mod remote_updates;
pub mod resource_cache;
pub mod task_utils;
+pub mod views;
pub mod connection;
pub mod pbs_client;
diff --git a/server/src/views/mod.rs b/server/src/views/mod.rs
new file mode 100644
index 00000000..b5d79e0e
--- /dev/null
+++ b/server/src/views/mod.rs
@@ -0,0 +1,205 @@
+use anyhow::{format_err, Error};
+
+use pdm_api_types::{
+ resource::{Resource, ResourceType},
+ views::{FilterRule, ViewConfig, ViewConfigEntry},
+};
+
+#[cfg(test)]
+mod tests;
+
+/// Get view with a given ID.
+///
+/// Returns an error if the view configuration file could not be read, or
+/// if the view with the provided ID does not exist.
+pub fn get_view(view_id: &str) -> Result<View, Error> {
+ let config = pdm_config::views::config()?;
+
+ let entry = config
+ .get(view_id)
+ .cloned()
+ .ok_or_else(|| format_err!("unknown view: {view_id}"))?;
+
+ match entry {
+ ViewConfigEntry::View(view_config) => Ok(View::new(view_config)),
+ }
+}
+
+/// Get (optional) view with a given ID.
+///
+/// Returns an error if the view configuration file could not be read, or
+/// if the view with the provided ID does not exist.
+pub fn get_optional_view(view_id: Option<&str>) -> Result<Option<View>, Error> {
+ view_id.map(get_view).transpose()
+}
+
+/// View implementation.
+///
+/// Given a [`ViewConfig`], this struct can be used to check if a resource/remote/node
+/// matches the include/exclude rules.
+#[derive(Clone)]
+pub struct View {
+ config: ViewConfig,
+}
+
+impl View {
+ /// Create a new [`View`].
+ pub fn new(config: ViewConfig) -> Self {
+ Self { config }
+ }
+
+ /// Check if a [`Resource`] matches the filter rules.
+ pub fn resource_matches(&self, remote: &str, resource: &Resource) -> bool {
+ // NOTE: Establishing a cache here is not worth the effort at the moment, evaluation of
+ // rules is *very* fast.
+
+ let resource_data = resource.into();
+
+ self.check_if_included(remote, &resource_data)
+ && !self.check_if_excluded(remote, &resource_data)
+ }
+
+ /// Check if a remote can be safely skipped based on the filter rule definition.
+ ///
+ /// When there are `include remote:<...>` or `exclude remote:<...>` rules, we can use these to
+ /// check if a remote needs to be considered at all.
+ pub fn can_skip_remote(&self, remote: &str) -> bool {
+ let mut has_any_include_remote = false;
+ let mut matches_any_include_remote = false;
+
+ let mut any_other = false;
+
+ for include in &self.config.include {
+ if let FilterRule::Remote(r) = include {
+ has_any_include_remote = true;
+ if r == remote {
+ matches_any_include_remote = true;
+ break;
+ }
+ } else {
+ any_other = true;
+ }
+ }
+
+ let matches_any_exclude_remote = self
+ .config
+ .exclude
+ .iter()
+ .any(|rule| Self::matches_remote_rule(remote, rule));
+
+ (has_any_include_remote && !matches_any_include_remote && !any_other)
+ || matches_any_exclude_remote
+ }
+
+ /// Check if a remote is *explicitly* included (and not excluded).
+ ///
+ /// A subset of the resources of a remote might still be pulled in by other rules,
+ /// but this function check if the remote as a whole is matched.
+ pub fn is_remote_explicitly_included(&self, remote: &str) -> bool {
+ let matches_include_remote = self
+ .config
+ .include
+ .iter()
+ .any(|rule| Self::matches_remote_rule(remote, rule));
+ let matches_exclude_remote = self
+ .config
+ .exclude
+ .iter()
+ .any(|rule| Self::matches_remote_rule(remote, rule));
+
+ matches_include_remote && !matches_exclude_remote
+ }
+
+ /// Check if a node is matched by the filter rules.
+ ///
+ /// This is equivalent to checking an actual node resource.
+ pub fn is_node_included(&self, remote: &str, node: &str) -> bool {
+ let resource_data = ResourceData {
+ resource_type: ResourceType::Node,
+ tags: None,
+ resource_pool: None,
+ resource_id: &format!("remote/{remote}/node/{node}"),
+ };
+
+ self.check_if_included(remote, &resource_data)
+ && !self.check_if_excluded(remote, &resource_data)
+ }
+
+ /// Returns the name of the view.
+ pub fn name(&self) -> &str {
+ &self.config.id
+ }
+
+ fn check_if_included(&self, remote: &str, resource: &ResourceData) -> bool {
+ if self.config.include.is_empty() {
+ // If there are no include rules, any resource is included (unless excluded)
+ return true;
+ }
+
+ check_rules(&self.config.include, remote, resource)
+ }
+
+ fn check_if_excluded(&self, remote: &str, resource: &ResourceData) -> bool {
+ check_rules(&self.config.exclude, remote, resource)
+ }
+
+ fn matches_remote_rule(remote: &str, rule: &FilterRule) -> bool {
+ if let FilterRule::Remote(r) = rule {
+ r == remote
+ } else {
+ false
+ }
+ }
+}
+
+fn check_rules(rules: &[FilterRule], remote: &str, resource: &ResourceData) -> bool {
+ rules.iter().any(|rule| match rule {
+ FilterRule::ResourceType(resource_type) => resource.resource_type == *resource_type,
+ FilterRule::ResourcePool(pool) => resource.resource_pool == Some(pool),
+ FilterRule::ResourceId(resource_id) => resource.resource_id == resource_id,
+ FilterRule::Tag(tag) => {
+ if let Some(resource_tags) = resource.tags {
+ resource_tags.contains(tag)
+ } else {
+ false
+ }
+ }
+ FilterRule::Remote(included_remote) => included_remote == remote,
+ })
+}
+
+struct ResourceData<'a> {
+ resource_type: ResourceType,
+ tags: Option<&'a [String]>,
+ resource_pool: Option<&'a String>,
+ resource_id: &'a str,
+}
+
+impl<'a> From<&'a Resource> for ResourceData<'a> {
+ fn from(value: &'a Resource) -> Self {
+ match value {
+ Resource::PveQemu(resource) => ResourceData {
+ resource_type: value.resource_type(),
+ tags: Some(&resource.tags),
+ resource_pool: Some(&resource.pool),
+ resource_id: value.global_id(),
+ },
+ Resource::PveLxc(resource) => ResourceData {
+ resource_type: value.resource_type(),
+ tags: Some(&resource.tags),
+ resource_pool: Some(&resource.pool),
+ resource_id: value.global_id(),
+ },
+ Resource::PveNode(_)
+ | Resource::PveSdn(_)
+ | Resource::PbsNode(_)
+ | Resource::PbsDatastore(_)
+ | Resource::PveStorage(_) => ResourceData {
+ resource_type: value.resource_type(),
+ tags: None,
+ resource_pool: None,
+ resource_id: value.global_id(),
+ },
+ }
+ }
+}
diff --git a/server/src/views/tests.rs b/server/src/views/tests.rs
new file mode 100644
index 00000000..030b7994
--- /dev/null
+++ b/server/src/views/tests.rs
@@ -0,0 +1,619 @@
+use pdm_api_types::{
+ resource::{PveLxcResource, PveQemuResource, PveStorageResource, Resource, ResourceType},
+ views::{FilterRule, ViewConfig},
+};
+
+use super::View;
+
+fn make_storage_resource(remote: &str, node: &str, storage_name: &str) -> Resource {
+ Resource::PveStorage(PveStorageResource {
+ disk: 1000,
+ maxdisk: 2000,
+ id: format!("remote/{remote}/storage/{node}/{storage_name}"),
+ storage: storage_name.into(),
+ node: node.into(),
+ status: "available".into(),
+ })
+}
+
+fn make_qemu_resource(
+ remote: &str,
+ node: &str,
+ vmid: u32,
+ pool: Option<&str>,
+ tags: &[&str],
+) -> Resource {
+ Resource::PveQemu(PveQemuResource {
+ disk: 1000,
+ maxdisk: 2000,
+ id: format!("remote/{remote}/guest/{vmid}"),
+ node: node.into(),
+ status: "available".into(),
+ cpu: 0.0,
+ maxcpu: 0.0,
+ maxmem: 1024,
+ mem: 512,
+ name: format!("vm-{vmid}"),
+ // TODO: Check the API type - i guess it should be an option?
+ pool: pool.map_or_else(String::new, |a| a.into()),
+ tags: tags.iter().map(|tag| String::from(*tag)).collect(),
+ template: false,
+ uptime: 1337,
+ vmid,
+ })
+}
+
+fn make_lxc_resource(
+ remote: &str,
+ node: &str,
+ vmid: u32,
+ pool: Option<&str>,
+ tags: &[&str],
+) -> Resource {
+ Resource::PveLxc(PveLxcResource {
+ disk: 1000,
+ maxdisk: 2000,
+ id: format!("remote/{remote}/guest/{vmid}"),
+ node: node.into(),
+ status: "available".into(),
+ cpu: 0.0,
+ maxcpu: 0.0,
+ maxmem: 1024,
+ mem: 512,
+ name: format!("vm-{vmid}"),
+ // TODO: Check the API type - i guess it should be an option?
+ pool: pool.map_or_else(String::new, |a| a.into()),
+ tags: tags.iter().map(|tag| String::from(*tag)).collect(),
+ template: false,
+ uptime: 1337,
+ vmid,
+ })
+}
+
+fn run_test(config: ViewConfig, tests: &[((&str, &Resource), bool)]) {
+ let filter = View::new(config);
+
+ for ((remote_name, resource), expected) in tests {
+ eprintln!("remote: {remote_name}, resource: {resource:?}");
+ assert_eq!(filter.resource_matches(remote_name, resource), *expected);
+ }
+}
+
+const NODE: &str = "somenode";
+const STORAGE: &str = "somestorage";
+const REMOTE: &str = "someremote";
+
+#[test]
+fn include_remotes() {
+ let config = ViewConfig {
+ id: "only-includes".into(),
+ include: vec![
+ FilterRule::Remote("remote-a".into()),
+ FilterRule::Remote("remote-b".into()),
+ ],
+ ..Default::default()
+ };
+ run_test(
+ config.clone(),
+ &[
+ (
+ (
+ "remote-a",
+ &make_storage_resource("remote-a", NODE, STORAGE),
+ ),
+ true,
+ ),
+ (
+ (
+ "remote-b",
+ &make_storage_resource("remote-b", NODE, STORAGE),
+ ),
+ true,
+ ),
+ (
+ (
+ "remote-c",
+ &make_storage_resource("remote-c", NODE, STORAGE),
+ ),
+ false,
+ ),
+ ],
+ );
+
+ let view = View::new(config);
+
+ assert!(!view.can_skip_remote("remote-a"));
+ assert!(!view.can_skip_remote("remote-b"));
+ assert!(view.can_skip_remote("remote-c"));
+}
+
+#[test]
+fn exclude_remotes() {
+ let config = ViewConfig {
+ id: "only-excludes".into(),
+ exclude: vec![
+ FilterRule::Remote("remote-a".into()),
+ FilterRule::Remote("remote-b".into()),
+ ],
+ ..Default::default()
+ };
+
+ run_test(
+ config.clone(),
+ &[
+ (
+ (
+ "remote-a",
+ &make_storage_resource("remote-a", NODE, STORAGE),
+ ),
+ false,
+ ),
+ (
+ (
+ "remote-b",
+ &make_storage_resource("remote-b", NODE, STORAGE),
+ ),
+ false,
+ ),
+ (
+ (
+ "remote-c",
+ &make_storage_resource("remote-c", NODE, STORAGE),
+ ),
+ true,
+ ),
+ ],
+ );
+
+ let view = View::new(config);
+
+ assert!(view.can_skip_remote("remote-a"));
+ assert!(view.can_skip_remote("remote-b"));
+ assert!(!view.can_skip_remote("remote-c"));
+}
+
+#[test]
+fn include_exclude_remotes() {
+ let config = ViewConfig {
+ id: "both".into(),
+ include: vec![
+ FilterRule::Remote("remote-a".into()),
+ FilterRule::Remote("remote-b".into()),
+ ],
+ exclude: vec![
+ FilterRule::Remote("remote-b".into()),
+ FilterRule::Remote("remote-c".into()),
+ ],
+ };
+ run_test(
+ config.clone(),
+ &[
+ (
+ (
+ "remote-a",
+ &make_storage_resource("remote-a", NODE, STORAGE),
+ ),
+ true,
+ ),
+ (
+ (
+ "remote-b",
+ &make_storage_resource("remote-b", NODE, STORAGE),
+ ),
+ false,
+ ),
+ (
+ (
+ "remote-c",
+ &make_storage_resource("remote-c", NODE, STORAGE),
+ ),
+ false,
+ ),
+ ],
+ );
+
+ let view = View::new(config);
+
+ assert!(!view.can_skip_remote("remote-a"));
+ assert!(view.can_skip_remote("remote-b"));
+ assert!(view.can_skip_remote("remote-c"));
+ assert!(view.can_skip_remote("remote-d"));
+}
+
+#[test]
+fn empty_config() {
+ let config = ViewConfig {
+ id: "empty".into(),
+ ..Default::default()
+ };
+ run_test(
+ config.clone(),
+ &[
+ (
+ (
+ "remote-a",
+ &make_storage_resource("remote-a", NODE, STORAGE),
+ ),
+ true,
+ ),
+ (
+ (
+ "remote-b",
+ &make_storage_resource("remote-b", NODE, STORAGE),
+ ),
+ true,
+ ),
+ (
+ (
+ "remote-c",
+ &make_storage_resource("remote-c", NODE, STORAGE),
+ ),
+ true,
+ ),
+ (
+ (REMOTE, &make_qemu_resource(REMOTE, NODE, 100, None, &[])),
+ true,
+ ),
+ ],
+ );
+
+ let view = View::new(config);
+
+ assert!(!view.can_skip_remote("remote-a"));
+ assert!(!view.can_skip_remote("remote-b"));
+ assert!(!view.can_skip_remote("remote-c"));
+}
+
+#[test]
+fn include_type() {
+ run_test(
+ ViewConfig {
+ id: "include-resource-type".into(),
+ include: vec![
+ FilterRule::ResourceType(ResourceType::PveStorage),
+ FilterRule::ResourceType(ResourceType::PveQemu),
+ ],
+ ..Default::default()
+ },
+ &[
+ (
+ (REMOTE, &make_storage_resource(REMOTE, NODE, STORAGE)),
+ true,
+ ),
+ (
+ (REMOTE, &make_qemu_resource(REMOTE, NODE, 100, None, &[])),
+ true,
+ ),
+ (
+ (REMOTE, &make_lxc_resource(REMOTE, NODE, 101, None, &[])),
+ false,
+ ),
+ ],
+ );
+}
+
+#[test]
+fn exclude_type() {
+ run_test(
+ ViewConfig {
+ id: "exclude-resource-type".into(),
+ exclude: vec![
+ FilterRule::ResourceType(ResourceType::PveStorage),
+ FilterRule::ResourceType(ResourceType::PveQemu),
+ ],
+ ..Default::default()
+ },
+ &[
+ (
+ (REMOTE, &make_storage_resource(REMOTE, NODE, STORAGE)),
+ false,
+ ),
+ (
+ (REMOTE, &make_qemu_resource(REMOTE, NODE, 100, None, &[])),
+ false,
+ ),
+ (
+ (REMOTE, &make_lxc_resource(REMOTE, NODE, 101, None, &[])),
+ true,
+ ),
+ ],
+ );
+}
+
+#[test]
+fn include_exclude_type() {
+ run_test(
+ ViewConfig {
+ id: "exclude-resource-type".into(),
+ include: vec![FilterRule::ResourceType(ResourceType::PveQemu)],
+ exclude: vec![FilterRule::ResourceType(ResourceType::PveStorage)],
+ },
+ &[
+ (
+ (REMOTE, &make_storage_resource(REMOTE, NODE, STORAGE)),
+ false,
+ ),
+ (
+ (REMOTE, &make_qemu_resource(REMOTE, NODE, 100, None, &[])),
+ true,
+ ),
+ (
+ (REMOTE, &make_lxc_resource(REMOTE, NODE, 101, None, &[])),
+ false,
+ ),
+ ],
+ );
+}
+
+#[test]
+fn include_exclude_tags() {
+ run_test(
+ ViewConfig {
+ id: "include-tags".into(),
+ include: vec![
+ FilterRule::Tag("tag1".to_string()),
+ FilterRule::Tag("tag2".to_string()),
+ ],
+ exclude: vec![FilterRule::Tag("tag3".to_string())],
+ },
+ &[
+ (
+ (REMOTE, &make_storage_resource(REMOTE, NODE, STORAGE)),
+ // only qemu/lxc can match tags for now
+ false,
+ ),
+ (
+ (
+ REMOTE,
+ &make_qemu_resource(REMOTE, NODE, 100, None, &["tag1", "tag3"]),
+ ),
+ // because tag3 is excluded
+ false,
+ ),
+ (
+ (
+ REMOTE,
+ &make_lxc_resource(REMOTE, NODE, 101, None, &["tag1"]),
+ ),
+ // matches since it's in the includes
+ true,
+ ),
+ (
+ (
+ REMOTE,
+ &make_lxc_resource(REMOTE, NODE, 102, None, &["tag4"]),
+ ),
+ // Not in includes, can never match
+ false,
+ ),
+ ],
+ );
+}
+
+#[test]
+fn include_exclude_resource_pool() {
+ run_test(
+ ViewConfig {
+ id: "pools".into(),
+ include: vec![
+ FilterRule::ResourcePool("pool1".to_string()),
+ FilterRule::ResourcePool("pool2".to_string()),
+ ],
+ exclude: vec![FilterRule::ResourcePool("pool2".to_string())],
+ },
+ &[
+ (
+ (REMOTE, &make_storage_resource(REMOTE, NODE, STORAGE)),
+ // only qemu/lxc can match pools for now
+ false,
+ ),
+ (
+ (
+ REMOTE,
+ &make_qemu_resource(REMOTE, NODE, 100, Some("pool2"), &[]),
+ ),
+ // because pool2 is excluded (takes precedence over includes)
+ false,
+ ),
+ (
+ (
+ REMOTE,
+ &make_lxc_resource(REMOTE, NODE, 101, Some("pool1"), &[]),
+ ),
+ // matches since it's in the includes
+ true,
+ ),
+ (
+ (
+ REMOTE,
+ &make_lxc_resource(REMOTE, NODE, 102, Some("pool4"), &[]),
+ ),
+ // Not in includes, can never match
+ false,
+ ),
+ ],
+ );
+}
+
+#[test]
+fn include_exclude_resource_id() {
+ run_test(
+ ViewConfig {
+ id: "resource-id".into(),
+ include: vec![
+ FilterRule::ResourceId(format!("remote/{REMOTE}/guest/100")),
+ FilterRule::ResourceId(format!("remote/{REMOTE}/storage/{NODE}/{STORAGE}")),
+ ],
+ exclude: vec![
+ FilterRule::ResourceId(format!("remote/{REMOTE}/guest/101")),
+ FilterRule::ResourceId("remote/otherremote/guest/101".to_string()),
+ FilterRule::ResourceId(format!("remote/{REMOTE}/storage/{NODE}/otherstorage")),
+ ],
+ },
+ &[
+ (
+ (REMOTE, &make_storage_resource(REMOTE, NODE, STORAGE)),
+ true,
+ ),
+ (
+ (REMOTE, &make_qemu_resource(REMOTE, NODE, 100, None, &[])),
+ true,
+ ),
+ (
+ (REMOTE, &make_lxc_resource(REMOTE, NODE, 101, None, &[])),
+ false,
+ ),
+ (
+ (REMOTE, &make_lxc_resource(REMOTE, NODE, 102, None, &[])),
+ false,
+ ),
+ (
+ (
+ "otherremote",
+ &make_lxc_resource("otherremote", NODE, 101, None, &[]),
+ ),
+ false,
+ ),
+ (
+ (
+ "yetanoterremote",
+ &make_lxc_resource("yetanotherremote", NODE, 104, None, &[]),
+ ),
+ false,
+ ),
+ ],
+ );
+}
+
+#[test]
+fn node_included() {
+ let view = View::new(ViewConfig {
+ id: "both".into(),
+
+ include: vec![
+ FilterRule::Remote("remote-a".to_string()),
+ FilterRule::ResourceId("remote/someremote/node/test".to_string()),
+ ],
+ exclude: vec![FilterRule::Remote("remote-b".to_string())],
+ });
+
+ assert!(view.is_node_included("remote-a", "somenode"));
+ assert!(view.is_node_included("remote-a", "somenode2"));
+ assert!(!view.is_node_included("remote-b", "somenode"));
+ assert!(!view.is_node_included("remote-b", "somenode2"));
+ assert!(view.is_node_included("someremote", "test"));
+
+ assert_eq!(view.name(), "both");
+}
+
+#[test]
+fn can_skip_remote_if_excluded() {
+ let view = View::new(ViewConfig {
+ id: "abc".into(),
+ include: vec![],
+ exclude: vec![FilterRule::Remote("remote-b".to_string())],
+ });
+
+ assert!(!view.can_skip_remote("remote-a"));
+ assert!(view.can_skip_remote("remote-b"));
+}
+
+#[test]
+fn can_skip_remote_if_included() {
+ let view = View::new(ViewConfig {
+ id: "abc".into(),
+ include: vec![FilterRule::Remote("remote-b".to_string())],
+ exclude: vec![],
+ });
+
+ assert!(!view.can_skip_remote("remote-b"));
+ assert!(view.can_skip_remote("remote-a"));
+}
+
+#[test]
+fn can_skip_remote_cannot_skip_if_any_other_include() {
+ let view = View::new(ViewConfig {
+ id: "abc".into(),
+ include: vec![
+ FilterRule::Remote("remote-b".to_string()),
+ FilterRule::ResourceId("resource/remote-a/guest/100".to_string()),
+ ],
+ exclude: vec![],
+ });
+
+ assert!(!view.can_skip_remote("remote-b"));
+ assert!(!view.can_skip_remote("remote-a"));
+}
+
+#[test]
+fn can_skip_remote_explicit_remote_exclude() {
+ let view = View::new(ViewConfig {
+ id: "abc".into(),
+ include: vec![FilterRule::ResourceId(
+ "resource/remote-a/guest/100".to_string(),
+ )],
+ exclude: vec![FilterRule::Remote("remote-a".to_string())],
+ });
+
+ assert!(view.can_skip_remote("remote-a"));
+}
+
+#[test]
+fn can_skip_remote_with_empty_config() {
+ let view = View::new(ViewConfig {
+ id: "abc".into(),
+ include: vec![],
+ exclude: vec![],
+ });
+
+ assert!(!view.can_skip_remote("remote-a"));
+ assert!(!view.can_skip_remote("remote-b"));
+}
+
+#[test]
+fn can_skip_remote_with_no_remote_includes() {
+ let view = View::new(ViewConfig {
+ id: "abc".into(),
+ include: vec![FilterRule::ResourceId(
+ "resource/remote-a/guest/100".to_string(),
+ )],
+ exclude: vec![],
+ });
+
+ assert!(!view.can_skip_remote("remote-a"));
+ assert!(!view.can_skip_remote("remote-b"));
+}
+
+#[test]
+fn explicitly_included_remote() {
+ let view = View::new(ViewConfig {
+ id: "abc".into(),
+ include: vec![FilterRule::Remote("remote-b".to_string())],
+ exclude: vec![],
+ });
+
+ assert!(view.is_remote_explicitly_included("remote-b"));
+}
+
+#[test]
+fn included_and_excluded_same_remote() {
+ let view = View::new(ViewConfig {
+ id: "abc".into(),
+ include: vec![FilterRule::Remote("remote-b".to_string())],
+ exclude: vec![FilterRule::Remote("remote-b".to_string())],
+ });
+
+ assert!(!view.is_remote_explicitly_included("remote-b"));
+}
+
+#[test]
+fn not_explicitly_included_remote() {
+ let view = View::new(ViewConfig {
+ id: "abc".into(),
+ include: vec![],
+ exclude: vec![],
+ });
+
+ // Assert that is not *explicitly* included
+ assert!(!view.is_remote_explicitly_included("remote-b"));
+}
--
2.47.3
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 23+ messages in thread
* [pdm-devel] [PATCH datacenter-manager v3 05/11] api: resources: list: add support for view parameter
2025-11-06 13:43 [pdm-devel] [PATCH datacenter-manager v3 00/11] backend implementation for view filters Lukas Wagner
` (3 preceding siblings ...)
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 04/11] views: add implementation for view resource filtering Lukas Wagner
@ 2025-11-06 13:43 ` Lukas Wagner
2025-11-11 14:31 ` Michael Köppl
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 06/11] api: resources: top entities: " Lukas Wagner
` (7 subsequent siblings)
12 siblings, 1 reply; 23+ messages in thread
From: Lukas Wagner @ 2025-11-06 13:43 UTC (permalink / raw)
To: pdm-devel
A view allows one to get filtered subset of all resources, based on
filter rules defined in a config file. View integrate with the
permission system - if a user has permissions on /view/{view-id}, then
these privileges are transitively applied to all resources which are
matched by the rules. All other permission checks are replaced if
requesting data through a view.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
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 dad3e6b6..1b3bfda2 100644
--- a/server/src/api/resources.rs
+++ b/server/src/api/resources.rs
@@ -18,7 +18,7 @@ use pdm_api_types::resource::{
use pdm_api_types::subscription::{
NodeSubscriptionInfo, RemoteSubscriptionState, RemoteSubscriptions, SubscriptionLevel,
};
-use pdm_api_types::{Authid, PRIV_RESOURCE_AUDIT};
+use pdm_api_types::{Authid, PRIV_RESOURCE_AUDIT, VIEW_ID_SCHEMA};
use pdm_search::{Search, SearchTerm};
use proxmox_access_control::CachedUserInfo;
use proxmox_router::{
@@ -30,8 +30,8 @@ use proxmox_sortable_macro::sortable;
use proxmox_subscription::SubscriptionStatus;
use pve_api_types::{ClusterResource, ClusterResourceType};
-use crate::connection;
use crate::metric_collection::top_entities;
+use crate::{connection, views};
pub const ROUTER: Router = Router::new()
.get(&list_subdirs_api_method!(SUBDIRS))
@@ -221,6 +221,10 @@ impl From<RemoteWithResources> for RemoteResources {
type: ResourceType,
optional: true,
},
+ view: {
+ schema: VIEW_ID_SCHEMA,
+ optional: true,
+ },
}
},
returns: {
@@ -236,10 +240,17 @@ pub async fn get_resources(
max_age: u64,
resource_type: Option<ResourceType>,
search: Option<String>,
+ view: Option<String>,
rpcenv: &mut dyn RpcEnvironment,
) -> Result<Vec<RemoteResources>, Error> {
- let remotes_with_resources =
- get_resources_impl(max_age, search, resource_type, Some(rpcenv)).await?;
+ let remotes_with_resources = get_resources_impl(
+ max_age,
+ search,
+ resource_type,
+ view.as_deref(),
+ Some(rpcenv),
+ )
+ .await?;
let resources = remotes_with_resources.into_iter().map(Into::into).collect();
Ok(resources)
}
@@ -276,6 +287,7 @@ pub(crate) async fn get_resources_impl(
max_age: u64,
search: Option<String>,
resource_type: Option<ResourceType>,
+ view: Option<&str>,
rpcenv: Option<&mut dyn RpcEnvironment>,
) -> Result<Vec<RemoteWithResources>, Error> {
let user_info = CachedUserInfo::new()?;
@@ -285,9 +297,15 @@ pub(crate) async fn get_resources_impl(
.get_auth_id()
.ok_or_else(|| format_err!("no authid available"))?
.parse()?;
- if !user_info.any_privs_below(&auth_id, &["resource"], PRIV_RESOURCE_AUDIT)? {
+
+ // NOTE: Assumption is that the regular permission check is completely replaced by a check
+ // on the view ACL object *if* a view parameter is passed.
+ if let Some(view) = &view {
+ user_info.check_privs(&auth_id, &["view", view], PRIV_RESOURCE_AUDIT, false)?;
+ } else if !user_info.any_privs_below(&auth_id, &["resource"], PRIV_RESOURCE_AUDIT)? {
http_bail!(FORBIDDEN, "user has no access to resources");
}
+
opt_auth_id = Some(auth_id);
}
@@ -296,12 +314,22 @@ pub(crate) async fn get_resources_impl(
let filters = search.map(Search::from).unwrap_or_default();
+ let view = views::get_optional_view(view.as_deref())?;
+
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.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) = &view {
+ if view.can_skip_remote(&remote_name) {
continue;
}
}
@@ -374,6 +402,17 @@ pub(crate) async fn get_resources_impl(
}
}
+ if let Some(view) = &view {
+ remote_resources.retain_mut(|r| {
+ r.resources
+ .retain(|resource| view.resource_matches(&r.remote_name, resource));
+
+ let has_any_matched_resources = !r.resources.is_empty();
+ has_any_matched_resources
+ || (r.error.is_some() && view.is_remote_explicitly_included(&r.remote_name))
+ });
+ }
+
Ok(remote_resources)
}
@@ -405,7 +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] 23+ messages in thread
* [pdm-devel] [PATCH datacenter-manager v3 06/11] api: resources: top entities: add support for view parameter
2025-11-06 13:43 [pdm-devel] [PATCH datacenter-manager v3 00/11] backend implementation for view filters Lukas Wagner
` (4 preceding siblings ...)
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 05/11] api: resources: list: add support for view parameter Lukas Wagner
@ 2025-11-06 13:43 ` Lukas Wagner
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 07/11] api: resources: status: " Lukas Wagner
` (6 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Lukas Wagner @ 2025-11-06 13:43 UTC (permalink / raw)
To: pdm-devel
A view allows one to get filtered subset of all resources, based on
filter rules defined in a config file. Views integrate with the
permission system - if a user has permissions on /view/{view-id}, then
these privileges are transitively applied to all resources which are
matched by the rules. All other permission checks are replaced if
requesting data through a view.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
---
server/src/api/resources.rs | 39 ++++++++++++++++++--
server/src/metric_collection/top_entities.rs | 5 +++
2 files changed, 40 insertions(+), 4 deletions(-)
diff --git a/server/src/api/resources.rs b/server/src/api/resources.rs
index 1b3bfda2..68ab1372 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: {
+ schema: VIEW_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: Option<String>,
rpcenv: &mut dyn RpcEnvironment,
) -> Result<TopEntities, Error> {
let user_info = CachedUserInfo::new()?;
@@ -644,17 +649,43 @@ async fn get_top_entities(
.ok_or_else(|| format_err!("no authid available"))?
.parse()?;
- if !user_info.any_privs_below(&auth_id, &["resource"], PRIV_RESOURCE_AUDIT)? {
+ if let Some(view) = &view {
+ user_info.check_privs(&auth_id, &["view", view], PRIV_RESOURCE_AUDIT, false)?;
+ } else if !user_info.any_privs_below(&auth_id, &["resource"], PRIV_RESOURCE_AUDIT)? {
http_bail!(FORBIDDEN, "user has no access to resources");
}
+ let view = views::get_optional_view(view.as_deref())?;
+
let (remotes_config, _) = pdm_config::remotes::config()?;
+
let check_remote_privs = |remote_name: &str| {
- user_info.lookup_privs(&auth_id, &["resource", remote_name]) & PRIV_RESOURCE_AUDIT != 0
+ if let Some(view) = &view {
+ // if `include-remote` or `exclude-remote` are used we can limit the
+ // number of remotes to check.
+ !view.can_skip_remote(remote_name)
+ } else {
+ user_info.lookup_privs(&auth_id, &["resource", remote_name]) & PRIV_RESOURCE_AUDIT != 0
+ }
+ };
+
+ let is_resource_included = |remote: &str, resource: &Resource| {
+ if let Some(view) = &view {
+ view.resource_matches(remote, resource)
+ } else {
+ true
+ }
};
let timeframe = timeframe.unwrap_or(RrdTimeframe::Day);
- let res = top_entities::calculate_top(&remotes_config, timeframe, 10, check_remote_privs);
+ let res = top_entities::calculate_top(
+ &remotes_config,
+ timeframe,
+ 10,
+ check_remote_privs,
+ is_resource_included,
+ );
+
Ok(res)
}
diff --git a/server/src/metric_collection/top_entities.rs b/server/src/metric_collection/top_entities.rs
index 73a3e63f..a91d586c 100644
--- a/server/src/metric_collection/top_entities.rs
+++ b/server/src/metric_collection/top_entities.rs
@@ -37,6 +37,7 @@ pub fn calculate_top(
timeframe: proxmox_rrd_api_types::RrdTimeframe,
num: usize,
check_remote_privs: impl Fn(&str) -> bool,
+ is_resource_included: impl Fn(&str, &Resource) -> bool,
) -> TopEntities {
let mut guest_cpu = Vec::new();
let mut node_cpu = Vec::new();
@@ -51,6 +52,10 @@ pub fn calculate_top(
crate::api::resources::get_cached_resources(remote_name, i64::MAX as u64)
{
for res in data.resources {
+ if !is_resource_included(remote_name, &res) {
+ continue;
+ }
+
let id = res.id().to_string();
let name = format!("pve/{remote_name}/{id}");
match &res {
--
2.47.3
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 23+ messages in thread
* [pdm-devel] [PATCH datacenter-manager v3 07/11] api: resources: status: add support for view parameter
2025-11-06 13:43 [pdm-devel] [PATCH datacenter-manager v3 00/11] backend implementation for view filters Lukas Wagner
` (5 preceding siblings ...)
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 06/11] api: resources: top entities: " Lukas Wagner
@ 2025-11-06 13:43 ` Lukas Wagner
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 08/11] api: subscription " Lukas Wagner
` (5 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Lukas Wagner @ 2025-11-06 13:43 UTC (permalink / raw)
To: pdm-devel
A view allows one to get filtered subset of all resources, based on
filter rules defined in a config file. View integrate with the
permission system - if a user has permissions on /view/{view-id}, then
these privileges are transitively applied to all resources which are
matched by the rules. All other permission checks are replaced if
requesting data through a view.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
---
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 68ab1372..143335fe 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: {
+ schema: VIEW_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: Option<String>,
rpcenv: &mut dyn RpcEnvironment,
) -> Result<ResourcesStatus, Error> {
let remotes_with_resources =
- get_resources_impl(max_age, None, None, None, Some(rpcenv)).await?;
+ get_resources_impl(max_age, None, None, view.as_deref(), Some(rpcenv)).await?;
let mut counts = ResourcesStatus::default();
for remote_with_resources in remotes_with_resources {
if let Some(err) = remote_with_resources.error {
--
2.47.3
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 23+ messages in thread
* [pdm-devel] [PATCH datacenter-manager v3 08/11] api: subscription status: add support for view parameter
2025-11-06 13:43 [pdm-devel] [PATCH datacenter-manager v3 00/11] backend implementation for view filters Lukas Wagner
` (6 preceding siblings ...)
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 07/11] api: resources: status: " Lukas Wagner
@ 2025-11-06 13:43 ` Lukas Wagner
2025-11-11 14:46 ` Michael Köppl
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 09/11] api: remote-tasks: " Lukas Wagner
` (4 subsequent siblings)
12 siblings, 1 reply; 23+ messages in thread
From: Lukas Wagner @ 2025-11-06 13:43 UTC (permalink / raw)
To: pdm-devel
A view allows one to get filtered subset of all resources, based on
filter rules defined in a config file. Views integrate with the
permission system - if a user has permissions on /view/{view-id}, then
these privileges are transitively applied to all resources which are
matched by the rules. All other permission checks are replaced if
requesting data through a view.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
Notes:
Changes since v2:
- make sure to not filter out a remote if it has been explicitly
included
server/src/api/resources.rs | 66 ++++++++++++++++++++++++++++++-------
1 file changed, 54 insertions(+), 12 deletions(-)
diff --git a/server/src/api/resources.rs b/server/src/api/resources.rs
index 143335fe..2930da8b 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: {
+ schema: VIEW_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: Option<String>,
rpcenv: &mut dyn RpcEnvironment,
) -> Result<Vec<RemoteSubscriptions>, Error> {
let (remotes_config, _) = pdm_config::remotes::config()?;
@@ -574,9 +579,17 @@ pub async fn get_subscription_status(
let auth_id = rpcenv.get_auth_id().unwrap().parse()?;
let user_info = CachedUserInfo::new()?;
- let allow_all = user_info
- .check_privs(&auth_id, &["resource"], PRIV_RESOURCE_AUDIT, false)
- .is_ok();
+
+ let allow_all = if let Some(view) = &view {
+ user_info.check_privs(&auth_id, &["view", view], PRIV_RESOURCE_AUDIT, false)?;
+ false
+ } else {
+ user_info
+ .check_privs(&auth_id, &["resource"], PRIV_RESOURCE_AUDIT, false)
+ .is_ok()
+ };
+
+ let view = views::get_optional_view(view.as_deref())?;
let check_priv = |remote_name: &str| -> bool {
user_info
@@ -590,35 +603,64 @@ pub async fn get_subscription_status(
};
for (remote_name, remote) in remotes_config {
- if !allow_all && !check_priv(&remote_name) {
+ if let Some(view) = &view {
+ if view.can_skip_remote(&remote_name) {
+ continue;
+ }
+ } else if !allow_all && !check_priv(&remote_name) {
continue;
}
+ let view = view.clone();
+
let future = async move {
let (node_status, error) =
match get_subscription_info_for_remote(&remote, max_age).await {
- Ok(node_status) => (Some(node_status), None),
+ Ok(mut node_status) => {
+ node_status.retain(|node, _| {
+ if let Some(view) = &view {
+ view.is_node_included(&remote.id, node)
+ } else {
+ true
+ }
+ });
+ (Some(node_status), None)
+ }
Err(error) => (None, Some(error.to_string())),
};
- let mut state = RemoteSubscriptionState::Unknown;
+ let state = if let Some(node_status) = &node_status {
+ if let Some(view) = view {
+ if error.is_some() && !view.is_remote_explicitly_included(&remote.id) {
+ // Don't leak the existence of failed remotes unless they were explicitly
+ // pulled in by a `include remote:<id>` rule.
+ return None;
+ }
+ }
- 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] 23+ messages in thread
* [pdm-devel] [PATCH datacenter-manager v3 09/11] api: remote-tasks: add support for view parameter
2025-11-06 13:43 [pdm-devel] [PATCH datacenter-manager v3 00/11] backend implementation for view filters Lukas Wagner
` (7 preceding siblings ...)
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 08/11] api: subscription " Lukas Wagner
@ 2025-11-06 13:43 ` Lukas Wagner
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 10/11] pdm-client: resource list: add view-filter parameter Lukas Wagner
` (3 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Lukas Wagner @ 2025-11-06 13:43 UTC (permalink / raw)
To: pdm-devel
A view allows one to get filtered subset of all resources, based on
filter rules defined in a config file. View filters integrate with the
permission system - if a user has permissions on /view/{view-id}, then
these privileges are transitively applied to all resources which are
matched by the rules. All other permission checks are replaced if
requesting data through a view.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
---
server/src/api/remote_tasks.rs | 36 +++++++++++++++++++++++++++++----
server/src/remote_tasks/mod.rs | 37 +++++++++++++++++++++++-----------
2 files changed, 57 insertions(+), 16 deletions(-)
diff --git a/server/src/api/remote_tasks.rs b/server/src/api/remote_tasks.rs
index 7b97b9cd..02b6383b 100644
--- a/server/src/api/remote_tasks.rs
+++ b/server/src/api/remote_tasks.rs
@@ -4,9 +4,10 @@ use anyhow::Error;
use pdm_api_types::{
remotes::REMOTE_ID_SCHEMA, RemoteUpid, TaskCount, TaskFilters, TaskListItem, TaskStateType,
- TaskStatistics,
+ TaskStatistics, PRIV_RESOURCE_AUDIT, VIEW_ID_SCHEMA,
};
-use proxmox_router::{list_subdirs_api_method, Permission, Router, SubdirMap};
+use proxmox_access_control::CachedUserInfo;
+use proxmox_router::{list_subdirs_api_method, Permission, Router, RpcEnvironment, SubdirMap};
use proxmox_schema::api;
use proxmox_sortable_macro::sortable;
@@ -41,6 +42,11 @@ const SUBDIRS: SubdirMap = &sorted!([
schema: REMOTE_ID_SCHEMA,
optional: true,
},
+ view: {
+ schema: VIEW_ID_SCHEMA,
+ optional: true,
+ },
+
},
},
)]
@@ -48,8 +54,17 @@ const SUBDIRS: SubdirMap = &sorted!([
async fn list_tasks(
filters: TaskFilters,
remote: Option<String>,
+ view: Option<String>,
+ rpcenv: &mut dyn RpcEnvironment,
) -> Result<Vec<TaskListItem>, Error> {
- let tasks = remote_tasks::get_tasks(filters, remote).await?;
+ let auth_id = rpcenv.get_auth_id().unwrap().parse()?;
+ let user_info = CachedUserInfo::new()?;
+
+ if let Some(view) = &view {
+ user_info.check_privs(&auth_id, &["view", view], PRIV_RESOURCE_AUDIT, false)?;
+ }
+
+ let tasks = remote_tasks::get_tasks(filters, remote, view).await?;
Ok(tasks)
}
@@ -70,6 +85,10 @@ async fn list_tasks(
schema: REMOTE_ID_SCHEMA,
optional: true,
},
+ view: {
+ schema: VIEW_ID_SCHEMA,
+ optional: true,
+ },
},
},
)]
@@ -77,8 +96,17 @@ async fn list_tasks(
async fn task_statistics(
filters: TaskFilters,
remote: Option<String>,
+ view: Option<String>,
+ rpcenv: &mut dyn RpcEnvironment,
) -> Result<TaskStatistics, Error> {
- let tasks = remote_tasks::get_tasks(filters, remote).await?;
+ let auth_id = rpcenv.get_auth_id().unwrap().parse()?;
+ let user_info = CachedUserInfo::new()?;
+
+ if let Some(view) = &view {
+ user_info.check_privs(&auth_id, &["view", view], PRIV_RESOURCE_AUDIT, false)?;
+ }
+
+ let tasks = remote_tasks::get_tasks(filters, remote, view).await?;
let mut by_type: HashMap<String, TaskCount> = HashMap::new();
let mut by_remote: HashMap<String, TaskCount> = HashMap::new();
diff --git a/server/src/remote_tasks/mod.rs b/server/src/remote_tasks/mod.rs
index c4939742..bdcfa256 100644
--- a/server/src/remote_tasks/mod.rs
+++ b/server/src/remote_tasks/mod.rs
@@ -9,6 +9,8 @@ pub mod task_cache;
use task_cache::{GetTasks, TaskCache, TaskCacheItem};
+use crate::views;
+
/// Base directory for the remote task cache.
pub const REMOTE_TASKS_DIR: &str = concat!(pdm_buildcfg::PDM_CACHE_DIR_M!(), "/remote-tasks");
@@ -29,7 +31,10 @@ const NUMBER_OF_UNCOMPRESSED_FILES: u32 = 2;
pub async fn get_tasks(
filters: TaskFilters,
remote_filter: Option<String>,
+ view: Option<String>,
) -> Result<Vec<TaskListItem>, Error> {
+ let view = views::get_optional_view(view.as_deref())?;
+
tokio::task::spawn_blocking(move || {
let cache = get_cache()?.read()?;
@@ -52,21 +57,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) = &view {
+ if !view.is_node_included(task.upid.remote(), &pve_upid.node) {
+ return None;
+ }
+ }
+ Some(TaskListItem {
+ upid: task.upid.to_string(),
+ node: pve_upid.node,
+ pid: pve_upid.pid as i64,
+ pstart: pve_upid.pstart,
+ starttime: pve_upid.starttime,
+ worker_type: pve_upid.worker_type,
+ worker_id: None,
+ user: pve_upid.auth_id,
+ endtime: task.endtime,
+ status: task.status,
+ })
+ }
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] 23+ messages in thread
* [pdm-devel] [PATCH datacenter-manager v3 10/11] pdm-client: resource list: add view-filter parameter
2025-11-06 13:43 [pdm-devel] [PATCH datacenter-manager v3 00/11] backend implementation for view filters Lukas Wagner
` (8 preceding siblings ...)
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 09/11] api: remote-tasks: " Lukas Wagner
@ 2025-11-06 13:43 ` Lukas Wagner
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 11/11] pdm-client: top entities: " Lukas Wagner
` (2 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Lukas Wagner @ 2025-11-06 13:43 UTC (permalink / raw)
To: pdm-devel
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
---
cli/client/src/resources.rs | 2 +-
lib/pdm-client/src/lib.rs | 9 ++++++++-
ui/src/sdn/zone_tree.rs | 2 +-
3 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/cli/client/src/resources.rs b/cli/client/src/resources.rs
index dbf9f265..bc3a98ba 100644
--- a/cli/client/src/resources.rs
+++ b/cli/client/src/resources.rs
@@ -31,7 +31,7 @@ pub fn cli() -> CommandLineInterface {
)]
/// List all the remotes this instance is managing.
async fn get_resources(max_age: Option<u64>) -> Result<(), Error> {
- let mut resources = client()?.resources(max_age).await?;
+ let mut resources = client()?.resources(max_age, None).await?;
let output_format = env().format_args.output_format;
if output_format == OutputFormat::Text {
if resources.is_empty() {
diff --git a/lib/pdm-client/src/lib.rs b/lib/pdm-client/src/lib.rs
index 0cab7691..46707e31 100644
--- a/lib/pdm-client/src/lib.rs
+++ b/lib/pdm-client/src/lib.rs
@@ -863,9 +863,14 @@ impl<T: HttpApiClient> PdmClient<T> {
Ok(self.0.get(&path).await?.expect_json()?.data)
}
- pub async fn resources(&self, max_age: Option<u64>) -> Result<Vec<RemoteResources>, Error> {
+ pub async fn resources(
+ &self,
+ max_age: Option<u64>,
+ view: Option<&str>,
+ ) -> Result<Vec<RemoteResources>, Error> {
let path = ApiPathBuilder::new("/api2/extjs/resources/list")
.maybe_arg("max-age", &max_age)
+ .maybe_arg("view", &view)
.build();
Ok(self.0.get(&path).await?.expect_json()?.data)
}
@@ -874,10 +879,12 @@ impl<T: HttpApiClient> PdmClient<T> {
&self,
max_age: Option<u64>,
resource_type: ResourceType,
+ view: Option<&str>,
) -> Result<Vec<RemoteResources>, Error> {
let path = ApiPathBuilder::new("/api2/extjs/resources/list")
.maybe_arg("max-age", &max_age)
.arg("resource-type", resource_type)
+ .maybe_arg("view", &view)
.build();
Ok(self.0.get(&path).await?.expect_json()?.data)
diff --git a/ui/src/sdn/zone_tree.rs b/ui/src/sdn/zone_tree.rs
index 1f534388..7475483d 100644
--- a/ui/src/sdn/zone_tree.rs
+++ b/ui/src/sdn/zone_tree.rs
@@ -281,7 +281,7 @@ impl LoadableComponent for ZoneTreeComponent {
Box::pin(async move {
let client = pdm_client();
let remote_resources = client
- .resources_by_type(None, ResourceType::PveSdnZone)
+ .resources_by_type(None, ResourceType::PveSdnZone, None)
.await?;
link.send_message(Self::Message::LoadFinished(remote_resources));
--
2.47.3
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 23+ messages in thread
* [pdm-devel] [PATCH datacenter-manager v3 11/11] pdm-client: top entities: add view-filter parameter
2025-11-06 13:43 [pdm-devel] [PATCH datacenter-manager v3 00/11] backend implementation for view filters Lukas Wagner
` (9 preceding siblings ...)
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 10/11] pdm-client: resource list: add view-filter parameter Lukas Wagner
@ 2025-11-06 13:43 ` Lukas Wagner
2025-11-11 15:00 ` [pdm-devel] [PATCH datacenter-manager v3 00/11] backend implementation for view filters Michael Köppl
2025-11-12 10:37 ` [pdm-devel] superseded: " Lukas Wagner
12 siblings, 0 replies; 23+ messages in thread
From: Lukas Wagner @ 2025-11-06 13:43 UTC (permalink / raw)
To: pdm-devel
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
---
lib/pdm-client/src/lib.rs | 10 +++++++---
ui/src/dashboard/view.rs | 2 +-
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/lib/pdm-client/src/lib.rs b/lib/pdm-client/src/lib.rs
index 46707e31..9ea0be9b 100644
--- a/lib/pdm-client/src/lib.rs
+++ b/lib/pdm-client/src/lib.rs
@@ -956,9 +956,13 @@ impl<T: HttpApiClient> PdmClient<T> {
Ok(self.0.get(&path).await?.expect_json()?.data)
}
- pub async fn get_top_entities(&self) -> Result<TopEntities, Error> {
- let path = "/api2/extjs/resources/top-entities";
- Ok(self.0.get(path).await?.expect_json()?.data)
+ pub async fn get_top_entities(&self, view: Option<&str>) -> Result<TopEntities, Error> {
+ let builder = ApiPathBuilder::new("/api2/extjs/resources/top-entities".to_string())
+ .maybe_arg("view", &view);
+
+ let path = builder.build();
+
+ Ok(self.0.get(&path).await?.expect_json()?.data)
}
pub async fn pve_node_status(&self, remote: &str, node: &str) -> Result<NodeStatus, Error> {
diff --git a/ui/src/dashboard/view.rs b/ui/src/dashboard/view.rs
index c781d991..c973d321 100644
--- a/ui/src/dashboard/view.rs
+++ b/ui/src/dashboard/view.rs
@@ -162,7 +162,7 @@ impl ViewComp {
if top_entities {
let client: pdm_client::PdmClient<Rc<proxmox_yew_comp::HttpClientWasm>> =
pdm_client();
- let res = client.get_top_entities().await;
+ let res = client.get_top_entities(None).await;
link.send_message(Msg::LoadingResult(LoadingResult::TopEntities(res)));
}
};
--
2.47.3
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager v3 01/11] pdm-api-types: views: add ViewConfig type
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 01/11] pdm-api-types: views: add ViewConfig type Lukas Wagner
@ 2025-11-11 10:57 ` Michael Köppl
2025-11-12 10:04 ` Lukas Wagner
2025-11-11 10:58 ` Michael Köppl
1 sibling, 1 reply; 23+ messages in thread
From: Michael Köppl @ 2025-11-11 10:57 UTC (permalink / raw)
To: Proxmox Datacenter Manager development discussion; +Cc: pdm-devel
On Thu Nov 6, 2025 at 2:43 PM CET, Lukas Wagner wrote:
> +#[derive(Clone, Debug, PartialEq)]
> +/// Filter rule for includes/excludes.
> +pub enum FilterRule {
> + /// Match a resource type.
> + ResourceType(ResourceType),
> + /// Match a resource pools (for PVE guests).
> + ResourcePool(String),
> + /// Match a (global) resource ID, e.g. 'remote/<remote>/guest/<vmid>'.
> + ResourceId(String),
> + /// Match a tag (for PVE guests).
> + Tag(String),
> + /// Match a remote.
> + Remote(String),
> +}
> +
> +impl FromStr for FilterRule {
> + type Err = anyhow::Error;
> +
> + fn from_str(s: &str) -> Result<Self, Self::Err> {
> + Ok(match s.split_once(':') {
> + Some(("resource-type", value)) => FilterRule::ResourceType(value.parse()?),
> + Some(("resource-pool", value)) => {
> + if !SAFE_ID_FORMAT.unwrap_pattern_format().is_match(value) {
> + bail!("invalid tag value: {value}");
> + }
> + FilterRule::ResourcePool(value.to_string())
> + }
> + Some(("resource-id", value)) => {
> + // TODO: Define schema and use it to validate. Can't use SAFE_ID_FORMAT since it does
> + // not allow '/'.
Is there a plan for this TODO?
> + if value.is_empty() {
> + bail!("empty resource-id rule not allowed");
> + }
> + FilterRule::ResourceId(value.to_string())
> + }
> + Some(("tag", value)) => {
> + if !SAFE_ID_FORMAT.unwrap_pattern_format().is_match(value) {
> + bail!("invalid tag value: {value}");
> + }
> + FilterRule::Tag(value.to_string())
> + }
> + Some(("remote", value)) => {
> + let _ = REMOTE_ID_SCHEMA.parse_simple_value(value)?;
> + FilterRule::Remote(value.to_string())
> + }
> + Some((ty, _)) => bail!("invalid type: {ty}"),
> + None => bail!("invalid filter rule: {s}"),
> + })
> + }
> +}
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager v3 01/11] pdm-api-types: views: add ViewConfig type
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 01/11] pdm-api-types: views: add ViewConfig type Lukas Wagner
2025-11-11 10:57 ` Michael Köppl
@ 2025-11-11 10:58 ` Michael Köppl
2025-11-12 10:05 ` Lukas Wagner
1 sibling, 1 reply; 23+ messages in thread
From: Michael Köppl @ 2025-11-11 10:58 UTC (permalink / raw)
To: Proxmox Datacenter Manager development discussion; +Cc: pdm-devel
2 comments inline
On Thu Nov 6, 2025 at 2:43 PM CET, Lukas Wagner wrote:
> This type will be used to define views.
>
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
>
> Notes:
> Changes since v2:
> - use ApiSectionDataEntry, as suggested by Dominik
> - Renamed ViewFilter* -> View*
> - Use SAFE_ID_FORMAT to check filter values where possible
>
> Changes since RFC:
> - Change config structure, instead of 'include-{x} value' we now do
> 'include {x}:value'
>
> lib/pdm-api-types/src/lib.rs | 8 ++
> lib/pdm-api-types/src/views.rs | 202 +++++++++++++++++++++++++++++++++
> 2 files changed, 210 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 ee4dfb2b..20327aba 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_:, +!\-@=.]+$";
> @@ -153,6 +155,12 @@ pub const REALM_ID_SCHEMA: Schema = StringSchema::new("Realm name.")
> .max_length(32)
> .schema();
>
> +pub const VIEW_ID_SCHEMA: Schema = StringSchema::new("View name.")
> + .format(&PROXMOX_SAFE_ID_FORMAT)
> + .min_length(2)
> + .max_length(32)
> + .schema();
> +
> pub const VMID_SCHEMA: Schema = IntegerSchema::new("A guest ID").minimum(1).schema();
> pub const SNAPSHOT_NAME_SCHEMA: Schema = StringSchema::new("The name of the snapshot")
> .format(&PROXMOX_SAFE_ID_FORMAT)
> diff --git a/lib/pdm-api-types/src/views.rs b/lib/pdm-api-types/src/views.rs
> new file mode 100644
> index 00000000..165b29d6
> --- /dev/null
> +++ b/lib/pdm-api-types/src/views.rs
> @@ -0,0 +1,202 @@
> +use std::{fmt::Display, str::FromStr, sync::OnceLock};
> +
> +use anyhow::bail;
> +use proxmox_section_config::{typed::ApiSectionDataEntry, SectionConfig, SectionConfigPlugin};
> +use serde::{Deserialize, Serialize};
> +
> +use proxmox_schema::{
> + api, api_types::SAFE_ID_FORMAT, ApiStringFormat, ApiType, ArraySchema, Schema, StringSchema,
> + Updater,
> +};
> +
> +use crate::{remotes::REMOTE_ID_SCHEMA, resource::ResourceType, VIEW_ID_SCHEMA};
> +
> +/// Schema for filter rules.
> +pub const FILTER_RULE_SCHEMA: Schema = StringSchema::new("Filter rule for resources.")
> + .format(&ApiStringFormat::VerifyFn(verify_filter_rule))
> + .type_text(
> + "resource-type:storage|qemu|lxc|sdn-zone|datastore|node>\
This line seems to be missing a <
> + |resource-pool:<pool-name>\
> + |tag:<tag-name>\
> + |remote:<remote-name>\
> + |resource-id:<resource-id>",
> + )
> + .schema();
> +
> +/// Schema for list of filter rules.
> +pub const FILTER_RULE_LIST_SCHEMA: Schema =
> + ArraySchema::new("List of filter rules.", &FILTER_RULE_SCHEMA).schema();
> +
> +#[api(
> + properties: {
> + "id": {
> + schema: VIEW_ID_SCHEMA,
> + },
> + "include": {
> + schema: FILTER_RULE_LIST_SCHEMA,
> + optional: true,
> + },
> + "exclude": {
> + schema: FILTER_RULE_LIST_SCHEMA,
> + optional: true,
> + }
> + }
> +)]
> +#[derive(Clone, Debug, Default, Deserialize, Serialize, Updater, PartialEq)]
> +#[serde(rename_all = "kebab-case")]
> +/// View definition
> +pub struct ViewConfig {
> + /// View name.
> + #[updater(skip)]
> + pub id: String,
> +
> + /// List of includes.
> + #[serde(default, skip_serializing_if = "Vec::is_empty")]
> + #[updater(serde(skip_serializing_if = "Option::is_none"))]
> + pub include: Vec<FilterRule>,
> +
> + /// List of excludes.
> + #[serde(default, skip_serializing_if = "Vec::is_empty")]
> + #[updater(serde(skip_serializing_if = "Option::is_none"))]
> + pub exclude: Vec<FilterRule>,
> +}
> +
> +#[derive(Clone, Debug, Deserialize, Serialize, PartialEq)]
> +#[serde(rename_all = "kebab-case")]
> +/// Enum for the different sections in the 'views.cfg' file.
> +pub enum ViewConfigEntry {
> + /// 'view' section
> + View(ViewConfig),
> +}
> +
> +const VIEW_SECTION_NAME: &str = "view";
> +
> +impl ApiSectionDataEntry for ViewConfigEntry {
> + fn section_config() -> &'static SectionConfig {
> + static CONFIG: OnceLock<SectionConfig> = OnceLock::new();
> +
> + CONFIG.get_or_init(|| {
> + let mut this = SectionConfig::new(&VIEW_ID_SCHEMA);
> +
> + this.register_plugin(SectionConfigPlugin::new(
> + VIEW_SECTION_NAME.into(),
> + Some("id".to_string()),
> + ViewConfig::API_SCHEMA.unwrap_object_schema(),
> + ));
> + this
> + })
> + }
> +
> + fn section_type(&self) -> &'static str {
> + match self {
> + ViewConfigEntry::View(_) => VIEW_SECTION_NAME,
> + }
> + }
> +}
> +
> +#[derive(Clone, Debug, PartialEq)]
> +/// Filter rule for includes/excludes.
> +pub enum FilterRule {
> + /// Match a resource type.
> + ResourceType(ResourceType),
> + /// Match a resource pools (for PVE guests).
> + ResourcePool(String),
> + /// Match a (global) resource ID, e.g. 'remote/<remote>/guest/<vmid>'.
> + ResourceId(String),
> + /// Match a tag (for PVE guests).
> + Tag(String),
> + /// Match a remote.
> + Remote(String),
> +}
> +
> +impl FromStr for FilterRule {
> + type Err = anyhow::Error;
> +
> + fn from_str(s: &str) -> Result<Self, Self::Err> {
> + Ok(match s.split_once(':') {
> + Some(("resource-type", value)) => FilterRule::ResourceType(value.parse()?),
> + Some(("resource-pool", value)) => {
> + if !SAFE_ID_FORMAT.unwrap_pattern_format().is_match(value) {
> + bail!("invalid tag value: {value}");
This should probably have different error message such as "invalid
resource pool ID".
> + }
> + FilterRule::ResourcePool(value.to_string())
> + }
> + Some(("resource-id", value)) => {
> + // TODO: Define schema and use it to validate. Can't use SAFE_ID_FORMAT since it does
> + // not allow '/'.
> + if value.is_empty() {
> + bail!("empty resource-id rule not allowed");
> + }
> + FilterRule::ResourceId(value.to_string())
> + }
> + Some(("tag", value)) => {
> + if !SAFE_ID_FORMAT.unwrap_pattern_format().is_match(value) {
> + bail!("invalid tag value: {value}");
> + }
> + FilterRule::Tag(value.to_string())
> + }
> + Some(("remote", value)) => {
> + let _ = REMOTE_ID_SCHEMA.parse_simple_value(value)?;
> + FilterRule::Remote(value.to_string())
> + }
> + Some((ty, _)) => bail!("invalid type: {ty}"),
> + None => bail!("invalid filter rule: {s}"),
> + })
> + }
> +}
> +
> +// used for serializing below, caution!
> +impl Display for FilterRule {
> + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
> + match self {
> + FilterRule::ResourceType(resource_type) => write!(f, "resource-type:{resource_type}"),
> + FilterRule::ResourcePool(pool) => write!(f, "resource-pool:{pool}"),
> + FilterRule::ResourceId(id) => write!(f, "resource-id:{id}"),
> + FilterRule::Tag(tag) => write!(f, "tag:{tag}"),
> + FilterRule::Remote(remote) => write!(f, "remote:{remote}"),
> + }
> + }
> +}
> +
> +proxmox_serde::forward_deserialize_to_from_str!(FilterRule);
> +proxmox_serde::forward_serialize_to_display!(FilterRule);
> +
> +fn verify_filter_rule(input: &str) -> Result<(), anyhow::Error> {
> + FilterRule::from_str(input).map(|_| ())
> +}
> +
> +#[cfg(test)]
> +mod test {
> + use anyhow::Error;
> +
> + use crate::views::FilterRule;
> +
> + fn parse_and_check_display(input: &str) -> Result<bool, Error> {
> + let rule: FilterRule = input.parse()?;
> +
> + Ok(input == format!("{rule}"))
> + }
> +
> + #[test]
> + fn test_filter_rule() {
> + assert!(parse_and_check_display("abc").is_err());
> + assert!(parse_and_check_display("abc:").is_err());
> +
> + assert!(parse_and_check_display("resource-type:").is_err());
> + assert!(parse_and_check_display("resource-type:lxc").unwrap());
> + assert!(parse_and_check_display("resource-type:qemu").unwrap());
> + assert!(parse_and_check_display("resource-type:abc").is_err());
> +
> + assert!(parse_and_check_display("resource-pool:").is_err());
> + assert!(parse_and_check_display("resource-pool:somepool").unwrap());
> +
> + assert!(parse_and_check_display("resource-id:").is_err());
> + assert!(parse_and_check_display("resource-id:remote/someremote/guest/100").unwrap());
> +
> + assert!(parse_and_check_display("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] 23+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager v3 05/11] api: resources: list: add support for view parameter
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 05/11] api: resources: list: add support for view parameter Lukas Wagner
@ 2025-11-11 14:31 ` Michael Köppl
2025-11-12 10:14 ` Lukas Wagner
0 siblings, 1 reply; 23+ messages in thread
From: Michael Köppl @ 2025-11-11 14:31 UTC (permalink / raw)
To: Proxmox Datacenter Manager development discussion; +Cc: pdm-devel
2 comments inline
On Thu Nov 6, 2025 at 2:43 PM CET, Lukas Wagner wrote:
> A view allows one to get filtered subset of all resources, based on
> filter rules defined in a config file. View integrate with the
> permission system - if a user has permissions on /view/{view-id}, then
> these privileges are transitively applied to all resources which are
> matched by the rules. All other permission checks are replaced if
> requesting data through a view.
>
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
> 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 dad3e6b6..1b3bfda2 100644
> --- a/server/src/api/resources.rs
> +++ b/server/src/api/resources.rs
> @@ -18,7 +18,7 @@ use pdm_api_types::resource::{
> use pdm_api_types::subscription::{
> NodeSubscriptionInfo, RemoteSubscriptionState, RemoteSubscriptions, SubscriptionLevel,
> };
> -use pdm_api_types::{Authid, PRIV_RESOURCE_AUDIT};
> +use pdm_api_types::{Authid, PRIV_RESOURCE_AUDIT, VIEW_ID_SCHEMA};
> use pdm_search::{Search, SearchTerm};
> use proxmox_access_control::CachedUserInfo;
> use proxmox_router::{
> @@ -30,8 +30,8 @@ use proxmox_sortable_macro::sortable;
> use proxmox_subscription::SubscriptionStatus;
> use pve_api_types::{ClusterResource, ClusterResourceType};
>
> -use crate::connection;
> use crate::metric_collection::top_entities;
> +use crate::{connection, views};
>
> pub const ROUTER: Router = Router::new()
> .get(&list_subdirs_api_method!(SUBDIRS))
> @@ -221,6 +221,10 @@ impl From<RemoteWithResources> for RemoteResources {
> type: ResourceType,
> optional: true,
> },
> + view: {
> + schema: VIEW_ID_SCHEMA,
> + optional: true,
> + },
> }
> },
> returns: {
> @@ -236,10 +240,17 @@ pub async fn get_resources(
> max_age: u64,
> resource_type: Option<ResourceType>,
> search: Option<String>,
> + view: Option<String>,
> rpcenv: &mut dyn RpcEnvironment,
> ) -> Result<Vec<RemoteResources>, Error> {
> - let remotes_with_resources =
> - get_resources_impl(max_age, search, resource_type, Some(rpcenv)).await?;
> + let remotes_with_resources = get_resources_impl(
> + max_age,
> + search,
> + resource_type,
> + view.as_deref(),
> + Some(rpcenv),
> + )
> + .await?;
> let resources = remotes_with_resources.into_iter().map(Into::into).collect();
> Ok(resources)
> }
> @@ -276,6 +287,7 @@ pub(crate) async fn get_resources_impl(
> max_age: u64,
> search: Option<String>,
> resource_type: Option<ResourceType>,
> + view: Option<&str>,
> rpcenv: Option<&mut dyn RpcEnvironment>,
> ) -> Result<Vec<RemoteWithResources>, Error> {
> let user_info = CachedUserInfo::new()?;
> @@ -285,9 +297,15 @@ pub(crate) async fn get_resources_impl(
> .get_auth_id()
> .ok_or_else(|| format_err!("no authid available"))?
> .parse()?;
> - if !user_info.any_privs_below(&auth_id, &["resource"], PRIV_RESOURCE_AUDIT)? {
> +
> + // NOTE: Assumption is that the regular permission check is completely replaced by a check
> + // on the view ACL object *if* a view parameter is passed.
> + if let Some(view) = &view {
> + user_info.check_privs(&auth_id, &["view", view], PRIV_RESOURCE_AUDIT, false)?;
> + } else if !user_info.any_privs_below(&auth_id, &["resource"], PRIV_RESOURCE_AUDIT)? {
> http_bail!(FORBIDDEN, "user has no access to resources");
> }
> +
> opt_auth_id = Some(auth_id);
> }
>
> @@ -296,12 +314,22 @@ pub(crate) async fn get_resources_impl(
>
> let filters = search.map(Search::from).unwrap_or_default();
>
> + let view = views::get_optional_view(view.as_deref())?;
Is this .as_deref() needed? `view` already is a Option<&str>.
> +
> 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.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) = &view {
> + if view.can_skip_remote(&remote_name) {
> continue;
> }
> }
Wouldn't it make more sense to switch the two if-blocks above?
if let Some(view) = &view {
if view.can_skip_remote(&remote_name) {
continue;
}
}
if let Some(ref auth_id) = opt_auth_id {
if view.is_none() {
let remote_privs = user_info.lookup_privs(auth_id, &["resource", &remote_name]);
if remote_privs & PRIV_RESOURCE_AUDIT == 0 {
continue;
}
}
}
Then view filtering takes precedence over checking the permissions, same
as in get_subscription_status. Might be that I'm misinterpreting this,
though.
> @@ -374,6 +402,17 @@ pub(crate) async fn get_resources_impl(
> }
> }
>
> + if let Some(view) = &view {
> + remote_resources.retain_mut(|r| {
> + r.resources
> + .retain(|resource| view.resource_matches(&r.remote_name, resource));
> +
> + let has_any_matched_resources = !r.resources.is_empty();
> + has_any_matched_resources
> + || (r.error.is_some() && view.is_remote_explicitly_included(&r.remote_name))
> + });
> + }
> +
> Ok(remote_resources)
> }
>
> @@ -405,7 +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] 23+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager v3 08/11] api: subscription status: add support for view parameter
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 08/11] api: subscription " Lukas Wagner
@ 2025-11-11 14:46 ` Michael Köppl
2025-11-12 8:19 ` Shannon Sterz
0 siblings, 1 reply; 23+ messages in thread
From: Michael Köppl @ 2025-11-11 14:46 UTC (permalink / raw)
To: Proxmox Datacenter Manager development discussion; +Cc: pdm-devel
1 comment inline
On Thu Nov 6, 2025 at 2:43 PM CET, Lukas Wagner wrote:
> A view allows one to get filtered subset of all resources, based on
> filter rules defined in a config file. Views integrate with the
> permission system - if a user has permissions on /view/{view-id}, then
> these privileges are transitively applied to all resources which are
> matched by the rules. All other permission checks are replaced if
> requesting data through a view.
>
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
>
> Notes:
> Changes since v2:
> - make sure to not filter out a remote if it has been explicitly
> included
>
> server/src/api/resources.rs | 66 ++++++++++++++++++++++++++++++-------
> 1 file changed, 54 insertions(+), 12 deletions(-)
>
> diff --git a/server/src/api/resources.rs b/server/src/api/resources.rs
> index 143335fe..2930da8b 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: {
> + schema: VIEW_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: Option<String>,
> rpcenv: &mut dyn RpcEnvironment,
> ) -> Result<Vec<RemoteSubscriptions>, Error> {
> let (remotes_config, _) = pdm_config::remotes::config()?;
> @@ -574,9 +579,17 @@ pub async fn get_subscription_status(
>
> let auth_id = rpcenv.get_auth_id().unwrap().parse()?;
> let user_info = CachedUserInfo::new()?;
> - let allow_all = user_info
> - .check_privs(&auth_id, &["resource"], PRIV_RESOURCE_AUDIT, false)
> - .is_ok();
> +
> + let allow_all = if let Some(view) = &view {
> + user_info.check_privs(&auth_id, &["view", view], PRIV_RESOURCE_AUDIT, false)?;
> + false
> + } else {
> + user_info
> + .check_privs(&auth_id, &["resource"], PRIV_RESOURCE_AUDIT, false)
> + .is_ok()
> + };
> +
> + let view = views::get_optional_view(view.as_deref())?;
>
> let check_priv = |remote_name: &str| -> bool {
> user_info
> @@ -590,35 +603,64 @@ pub async fn get_subscription_status(
> };
>
> for (remote_name, remote) in remotes_config {
> - if !allow_all && !check_priv(&remote_name) {
> + if let Some(view) = &view {
> + if view.can_skip_remote(&remote_name) {
> + continue;
> + }
> + } else if !allow_all && !check_priv(&remote_name) {
> continue;
> }
>
> + let view = view.clone();
> +
> let future = async move {
> let (node_status, error) =
> match get_subscription_info_for_remote(&remote, max_age).await {
> - Ok(node_status) => (Some(node_status), None),
> + Ok(mut node_status) => {
> + node_status.retain(|node, _| {
> + if let Some(view) = &view {
> + view.is_node_included(&remote.id, node)
> + } else {
> + true
> + }
> + });
> + (Some(node_status), None)
> + }
> Err(error) => (None, Some(error.to_string())),
> };
>
> - let mut state = RemoteSubscriptionState::Unknown;
> + let state = if let Some(node_status) = &node_status {
> + if let Some(view) = view {
> + if error.is_some() && !view.is_remote_explicitly_included(&remote.id) {
Can this ever be be true? From the code above, you can never have the
case that node_status and error are both Some(...) at the same time, so
error.is_some() would always evaluate to false here, unless I'm missing
something.
I think you'd have to put the check outside of this block.
if let Some(view) = &view {
if error.is_some() && !view.is_remote_explicitly_included(&remote.id) {
return None;
}
}
let state = if let Some(node_status) = &node_status {
...
}
> + // Don't leak the existence of failed remotes unless they were explicitly
> + // pulled in by a `include remote:<id>` rule.
> + 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] 23+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager v3 00/11] backend implementation for view filters
2025-11-06 13:43 [pdm-devel] [PATCH datacenter-manager v3 00/11] backend implementation for view filters Lukas Wagner
` (10 preceding siblings ...)
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 11/11] pdm-client: top entities: " Lukas Wagner
@ 2025-11-11 15:00 ` Michael Köppl
2025-11-12 10:37 ` [pdm-devel] superseded: " Lukas Wagner
12 siblings, 0 replies; 23+ messages in thread
From: Michael Köppl @ 2025-11-11 15:00 UTC (permalink / raw)
To: Proxmox Datacenter Manager development discussion; +Cc: pdm-devel
Gave this a closer look. Apart from my comments on the individual
patches, the changes lgtm! Thanks for also including a rather extensive
test suite.
With my comments on the individual patches addressed, consider this:
Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>
On Thu Nov 6, 2025 at 2:43 PM CET, Lukas Wagner wrote:
> Key aspects:
> - new config file at /etc/proxmox-datacenter-manager/views.cfg
> - ViewConfig as a definition type, has
> - {include,exclude} {remote,resource-id,resource-type,resource-pool,tag}:{value}
>
> - View resource filter implementation & big suite of unit tests
> - excludes are processed after includes
> - if no include rules are defined, all resources but those which were
> excluded are matched
> - if no rules are defined in a filter, everything is matched
>
> - Added the 'view' parameter to a couple of API endpoints
> - /resources/list
> - /resources/status
> - /resources/subscription
> - /resources/top-entities
> - /remote-tasks/list
> - /remote-tasks/statistis
>
> - ACL checks are done on /view/{view-filter-id} for now, replace
> any other permission check in the handler iff the view-filter paramter
> is set
>
> Left to do:
> - CRUD for filter definition
> - UI for filter rules
>
> Changes since v2 (thx for the review @Dominik):
>
> - Renamed:
> - ViewFilter -> View
> - ViewFilterConfig -> ViewConfig
> - 'view-filter' parameters to 'view'
>
> - Use ApiSectionDataEntry trait for ViewConfig
> - Use SAFE_ID_FORMAT to validate filter values
> - Changed path for config file from `views/filter.cfg` to just `views.cfg`
>
> - Include failed remotes in API responses iff they have been explicitly included
> via `include remote:<...>`. Previously, they had been filtered out to avoid
> leaking the existence of remotes, but if they have been explicitly included,
> this is fine.
>
> - Simplify `check_rules` function by using `.any()` instead of looping manually
>
> - Merged the rule implementation and test commits
>
> - Added views::get_optional_view as a convenience helper. This one is similar to
> views::get_view, but accepts Option<&str> and returns Option<View>.
>
> Changes since v1 (RFC):
> - Change config key structure, moving the type into the value
> e.g.
> include-remote foo
> became
> include remote:foo
>
> - Minor fixes from the review (thanks Wolfgang & Shannon)
>
>
> proxmox-datacenter-manager:
>
> Lukas Wagner (11):
> pdm-api-types: views: add ViewConfig type
> pdm-config: views: add support for views
> acl: add '/view' and '/view/{view-id}' as allowed ACL paths
> views: add implementation for view resource filtering
> api: resources: list: add support for view parameter
> api: resources: top entities: add support for view parameter
> api: resources: status: add support for view parameter
> api: subscription status: add support for view parameter
> api: remote-tasks: add support for view parameter
> pdm-client: resource list: add view-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 | 202 ++++++
> lib/pdm-client/src/lib.rs | 19 +-
> lib/pdm-config/src/lib.rs | 2 +-
> lib/pdm-config/src/views.rs | 17 +
> server/src/acl.rs | 6 +
> server/src/api/remote_tasks.rs | 36 +-
> server/src/api/resources.rs | 166 ++++-
> server/src/lib.rs | 1 +
> server/src/metric_collection/top_entities.rs | 5 +
> server/src/remote_tasks/mod.rs | 37 +-
> server/src/resource_cache.rs | 3 +-
> server/src/views/mod.rs | 205 ++++++
> server/src/views/tests.rs | 619 +++++++++++++++++++
> ui/src/dashboard/view.rs | 2 +-
> ui/src/sdn/zone_tree.rs | 2 +-
> 17 files changed, 1283 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
>
>
> Summary over all repositories:
> 17 files changed, 1283 insertions(+), 49 deletions(-)
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager v3 08/11] api: subscription status: add support for view parameter
2025-11-11 14:46 ` Michael Köppl
@ 2025-11-12 8:19 ` Shannon Sterz
2025-11-12 10:26 ` Lukas Wagner
0 siblings, 1 reply; 23+ messages in thread
From: Shannon Sterz @ 2025-11-12 8:19 UTC (permalink / raw)
To: Michael Köppl; +Cc: Proxmox Datacenter Manager development discussion
On Tue Nov 11, 2025 at 3:46 PM CET, Michael Köppl wrote:
> 1 comment inline
>
> On Thu Nov 6, 2025 at 2:43 PM CET, Lukas Wagner wrote:
>> A view allows one to get filtered subset of all resources, based on
>> filter rules defined in a config file. Views integrate with the
>> permission system - if a user has permissions on /view/{view-id}, then
>> these privileges are transitively applied to all resources which are
>> matched by the rules. All other permission checks are replaced if
>> requesting data through a view.
>>
>> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
>> ---
>>
>> Notes:
>> Changes since v2:
>> - make sure to not filter out a remote if it has been explicitly
>> included
>>
>> server/src/api/resources.rs | 66 ++++++++++++++++++++++++++++++-------
>> 1 file changed, 54 insertions(+), 12 deletions(-)
>>
>> diff --git a/server/src/api/resources.rs b/server/src/api/resources.rs
>> index 143335fe..2930da8b 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: {
>> + schema: VIEW_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: Option<String>,
>> rpcenv: &mut dyn RpcEnvironment,
>> ) -> Result<Vec<RemoteSubscriptions>, Error> {
>> let (remotes_config, _) = pdm_config::remotes::config()?;
>> @@ -574,9 +579,17 @@ pub async fn get_subscription_status(
>>
>> let auth_id = rpcenv.get_auth_id().unwrap().parse()?;
>> let user_info = CachedUserInfo::new()?;
>> - let allow_all = user_info
>> - .check_privs(&auth_id, &["resource"], PRIV_RESOURCE_AUDIT, false)
>> - .is_ok();
>> +
>> + let allow_all = if let Some(view) = &view {
>> + user_info.check_privs(&auth_id, &["view", view], PRIV_RESOURCE_AUDIT, false)?;
>> + false
>> + } else {
>> + user_info
>> + .check_privs(&auth_id, &["resource"], PRIV_RESOURCE_AUDIT, false)
>> + .is_ok()
>> + };
>> +
>> + let view = views::get_optional_view(view.as_deref())?;
>>
>> let check_priv = |remote_name: &str| -> bool {
>> user_info
>> @@ -590,35 +603,64 @@ pub async fn get_subscription_status(
>> };
>>
>> for (remote_name, remote) in remotes_config {
>> - if !allow_all && !check_priv(&remote_name) {
>> + if let Some(view) = &view {
>> + if view.can_skip_remote(&remote_name) {
>> + continue;
>> + }
>> + } else if !allow_all && !check_priv(&remote_name) {
>> continue;
>> }
>>
>> + let view = view.clone();
>> +
>> let future = async move {
>> let (node_status, error) =
>> match get_subscription_info_for_remote(&remote, max_age).await {
>> - Ok(node_status) => (Some(node_status), None),
>> + Ok(mut node_status) => {
>> + node_status.retain(|node, _| {
>> + if let Some(view) = &view {
>> + view.is_node_included(&remote.id, node)
>> + } else {
>> + true
>> + }
>> + });
>> + (Some(node_status), None)
>> + }
>> Err(error) => (None, Some(error.to_string())),
>> };
>>
>> - let mut state = RemoteSubscriptionState::Unknown;
>> + let state = if let Some(node_status) = &node_status {
>> + if let Some(view) = view {
>> + if error.is_some() && !view.is_remote_explicitly_included(&remote.id) {
>
> Can this ever be be true? From the code above, you can never have the
> case that node_status and error are both Some(...) at the same time, so
> error.is_some() would always evaluate to false here, unless I'm missing
> something.
>
> I think you'd have to put the check outside of this block.
>
> if let Some(view) = &view {
> if error.is_some() && !view.is_remote_explicitly_included(&remote.id) {
> return None;
> }
> }
>
> let state = if let Some(node_status) = &node_status {
> ...
> }
>
i thikn this is right, at least i noted something similar in a previous
version of this patch series already. thanks for spotting it again!
https://lore.proxmox.com/all/DDVMQ0KGW0IP.50W15XZS8TQ@proxmox.com/
>> + // Don't leak the existence of failed remotes unless they were explicitly
>> + // pulled in by a `include remote:<id>` rule.
>> + 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
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager v3 01/11] pdm-api-types: views: add ViewConfig type
2025-11-11 10:57 ` Michael Köppl
@ 2025-11-12 10:04 ` Lukas Wagner
0 siblings, 0 replies; 23+ messages in thread
From: Lukas Wagner @ 2025-11-12 10:04 UTC (permalink / raw)
To: Proxmox Datacenter Manager development discussion, Michael Köppl
On Tue Nov 11, 2025 at 11:57 AM CET, Michael Köppl wrote:
> On Thu Nov 6, 2025 at 2:43 PM CET, Lukas Wagner wrote:
>> +#[derive(Clone, Debug, PartialEq)]
>> +/// Filter rule for includes/excludes.
>> +pub enum FilterRule {
>> + /// Match a resource type.
>> + ResourceType(ResourceType),
>> + /// Match a resource pools (for PVE guests).
>> + ResourcePool(String),
>> + /// Match a (global) resource ID, e.g. 'remote/<remote>/guest/<vmid>'.
>> + ResourceId(String),
>> + /// Match a tag (for PVE guests).
>> + Tag(String),
>> + /// Match a remote.
>> + Remote(String),
>> +}
>> +
>> +impl FromStr for FilterRule {
>> + type Err = anyhow::Error;
>> +
>> + fn from_str(s: &str) -> Result<Self, Self::Err> {
>> + Ok(match s.split_once(':') {
>> + Some(("resource-type", value)) => FilterRule::ResourceType(value.parse()?),
>> + Some(("resource-pool", value)) => {
>> + if !SAFE_ID_FORMAT.unwrap_pattern_format().is_match(value) {
>> + bail!("invalid tag value: {value}");
>> + }
>> + FilterRule::ResourcePool(value.to_string())
>> + }
>> + Some(("resource-id", value)) => {
>> + // TODO: Define schema and use it to validate. Can't use SAFE_ID_FORMAT since it does
>> + // not allow '/'.
>
> Is there a plan for this TODO?
>
Well, the plan is to define a schema and then use it for validation :D
It's not super critical to validate the format here (worst case, somebody
defines a rule which can never match a real resource), so I think this
can be done in a follow-up. In the follow-up the schema should then be
used in a few other places in the code as well.
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager v3 01/11] pdm-api-types: views: add ViewConfig type
2025-11-11 10:58 ` Michael Köppl
@ 2025-11-12 10:05 ` Lukas Wagner
0 siblings, 0 replies; 23+ messages in thread
From: Lukas Wagner @ 2025-11-12 10:05 UTC (permalink / raw)
To: Proxmox Datacenter Manager development discussion, Michael Köppl
On Tue Nov 11, 2025 at 11:58 AM CET, Michael Köppl wrote:
>> +/// Schema for filter rules.
>> +pub const FILTER_RULE_SCHEMA: Schema = StringSchema::new("Filter rule for resources.")
>> + .format(&ApiStringFormat::VerifyFn(verify_filter_rule))
>> + .type_text(
>> + "resource-type:storage|qemu|lxc|sdn-zone|datastore|node>\
>
> This line seems to be missing a <
>
Fixed, thank you!
>> + |resource-pool:<pool-name>\
>> + |tag:<tag-name>\
>> + |remote:<remote-name>\
>> + |resource-id:<resource-id>",
>> + )
>> + .schema();
>> +
>> +/// Schema for list of filter rules.
>> +pub const FILTER_RULE_LIST_SCHEMA: Schema =
>> + ArraySchema::new("List of filter rules.", &FILTER_RULE_SCHEMA).schema();
>> +
>> +#[api(
>> + properties: {
>> + "id": {
>> + schema: VIEW_ID_SCHEMA,
>> + },
>> + "include": {
>> + schema: FILTER_RULE_LIST_SCHEMA,
>> + optional: true,
>> + },
>> + "exclude": {
>> + schema: FILTER_RULE_LIST_SCHEMA,
>> + optional: true,
>> + }
>> + }
>> +)]
>> +#[derive(Clone, Debug, Default, Deserialize, Serialize, Updater, PartialEq)]
>> +#[serde(rename_all = "kebab-case")]
>> +/// View definition
>> +pub struct ViewConfig {
>> + /// View name.
>> + #[updater(skip)]
>> + pub id: String,
>> +
>> + /// List of includes.
>> + #[serde(default, skip_serializing_if = "Vec::is_empty")]
>> + #[updater(serde(skip_serializing_if = "Option::is_none"))]
>> + pub include: Vec<FilterRule>,
>> +
>> + /// List of excludes.
>> + #[serde(default, skip_serializing_if = "Vec::is_empty")]
>> + #[updater(serde(skip_serializing_if = "Option::is_none"))]
>> + pub exclude: Vec<FilterRule>,
>> +}
>> +
>> +#[derive(Clone, Debug, Deserialize, Serialize, PartialEq)]
>> +#[serde(rename_all = "kebab-case")]
>> +/// Enum for the different sections in the 'views.cfg' file.
>> +pub enum ViewConfigEntry {
>> + /// 'view' section
>> + View(ViewConfig),
>> +}
>> +
>> +const VIEW_SECTION_NAME: &str = "view";
>> +
>> +impl ApiSectionDataEntry for ViewConfigEntry {
>> + fn section_config() -> &'static SectionConfig {
>> + static CONFIG: OnceLock<SectionConfig> = OnceLock::new();
>> +
>> + CONFIG.get_or_init(|| {
>> + let mut this = SectionConfig::new(&VIEW_ID_SCHEMA);
>> +
>> + this.register_plugin(SectionConfigPlugin::new(
>> + VIEW_SECTION_NAME.into(),
>> + Some("id".to_string()),
>> + ViewConfig::API_SCHEMA.unwrap_object_schema(),
>> + ));
>> + this
>> + })
>> + }
>> +
>> + fn section_type(&self) -> &'static str {
>> + match self {
>> + ViewConfigEntry::View(_) => VIEW_SECTION_NAME,
>> + }
>> + }
>> +}
>> +
>> +#[derive(Clone, Debug, PartialEq)]
>> +/// Filter rule for includes/excludes.
>> +pub enum FilterRule {
>> + /// Match a resource type.
>> + ResourceType(ResourceType),
>> + /// Match a resource pools (for PVE guests).
>> + ResourcePool(String),
>> + /// Match a (global) resource ID, e.g. 'remote/<remote>/guest/<vmid>'.
>> + ResourceId(String),
>> + /// Match a tag (for PVE guests).
>> + Tag(String),
>> + /// Match a remote.
>> + Remote(String),
>> +}
>> +
>> +impl FromStr for FilterRule {
>> + type Err = anyhow::Error;
>> +
>> + fn from_str(s: &str) -> Result<Self, Self::Err> {
>> + Ok(match s.split_once(':') {
>> + Some(("resource-type", value)) => FilterRule::ResourceType(value.parse()?),
>> + Some(("resource-pool", value)) => {
>> + if !SAFE_ID_FORMAT.unwrap_pattern_format().is_match(value) {
>> + bail!("invalid tag value: {value}");
>
> This should probably have different error message such as "invalid
> resource pool ID".
>
Fixed as well.
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager v3 05/11] api: resources: list: add support for view parameter
2025-11-11 14:31 ` Michael Köppl
@ 2025-11-12 10:14 ` Lukas Wagner
0 siblings, 0 replies; 23+ messages in thread
From: Lukas Wagner @ 2025-11-12 10:14 UTC (permalink / raw)
To: Proxmox Datacenter Manager development discussion, Michael Köppl
On Tue Nov 11, 2025 at 3:31 PM CET, Michael Köppl wrote:
> 2 comments inline
>
> On Thu Nov 6, 2025 at 2:43 PM CET, Lukas Wagner wrote:
>> A view allows one to get filtered subset of all resources, based on
>> filter rules defined in a config file. View integrate with the
>> permission system - if a user has permissions on /view/{view-id}, then
>> these privileges are transitively applied to all resources which are
>> matched by the rules. All other permission checks are replaced if
>> requesting data through a view.
>>
>> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
>> ---
>> 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 dad3e6b6..1b3bfda2 100644
>> --- a/server/src/api/resources.rs
>> +++ b/server/src/api/resources.rs
>> @@ -18,7 +18,7 @@ use pdm_api_types::resource::{
>> use pdm_api_types::subscription::{
>> NodeSubscriptionInfo, RemoteSubscriptionState, RemoteSubscriptions, SubscriptionLevel,
>> };
>> -use pdm_api_types::{Authid, PRIV_RESOURCE_AUDIT};
>> +use pdm_api_types::{Authid, PRIV_RESOURCE_AUDIT, VIEW_ID_SCHEMA};
>> use pdm_search::{Search, SearchTerm};
>> use proxmox_access_control::CachedUserInfo;
>> use proxmox_router::{
>> @@ -30,8 +30,8 @@ use proxmox_sortable_macro::sortable;
>> use proxmox_subscription::SubscriptionStatus;
>> use pve_api_types::{ClusterResource, ClusterResourceType};
>>
>> -use crate::connection;
>> use crate::metric_collection::top_entities;
>> +use crate::{connection, views};
>>
>> pub const ROUTER: Router = Router::new()
>> .get(&list_subdirs_api_method!(SUBDIRS))
>> @@ -221,6 +221,10 @@ impl From<RemoteWithResources> for RemoteResources {
>> type: ResourceType,
>> optional: true,
>> },
>> + view: {
>> + schema: VIEW_ID_SCHEMA,
>> + optional: true,
>> + },
>> }
>> },
>> returns: {
>> @@ -236,10 +240,17 @@ pub async fn get_resources(
>> max_age: u64,
>> resource_type: Option<ResourceType>,
>> search: Option<String>,
>> + view: Option<String>,
>> rpcenv: &mut dyn RpcEnvironment,
>> ) -> Result<Vec<RemoteResources>, Error> {
>> - let remotes_with_resources =
>> - get_resources_impl(max_age, search, resource_type, Some(rpcenv)).await?;
>> + let remotes_with_resources = get_resources_impl(
>> + max_age,
>> + search,
>> + resource_type,
>> + view.as_deref(),
>> + Some(rpcenv),
>> + )
>> + .await?;
>> let resources = remotes_with_resources.into_iter().map(Into::into).collect();
>> Ok(resources)
>> }
>> @@ -276,6 +287,7 @@ pub(crate) async fn get_resources_impl(
>> max_age: u64,
>> search: Option<String>,
>> resource_type: Option<ResourceType>,
>> + view: Option<&str>,
>> rpcenv: Option<&mut dyn RpcEnvironment>,
>> ) -> Result<Vec<RemoteWithResources>, Error> {
>> let user_info = CachedUserInfo::new()?;
>> @@ -285,9 +297,15 @@ pub(crate) async fn get_resources_impl(
>> .get_auth_id()
>> .ok_or_else(|| format_err!("no authid available"))?
>> .parse()?;
>> - if !user_info.any_privs_below(&auth_id, &["resource"], PRIV_RESOURCE_AUDIT)? {
>> +
>> + // NOTE: Assumption is that the regular permission check is completely replaced by a check
>> + // on the view ACL object *if* a view parameter is passed.
>> + if let Some(view) = &view {
>> + user_info.check_privs(&auth_id, &["view", view], PRIV_RESOURCE_AUDIT, false)?;
>> + } else if !user_info.any_privs_below(&auth_id, &["resource"], PRIV_RESOURCE_AUDIT)? {
>> http_bail!(FORBIDDEN, "user has no access to resources");
>> }
>> +
>> opt_auth_id = Some(auth_id);
>> }
>>
>> @@ -296,12 +314,22 @@ pub(crate) async fn get_resources_impl(
>>
>> let filters = search.map(Search::from).unwrap_or_default();
>>
>> + let view = views::get_optional_view(view.as_deref())?;
>
> Is this .as_deref() needed? `view` already is a Option<&str>.
>
Good catch!
>> +
>> 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.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) = &view {
>> + if view.can_skip_remote(&remote_name) {
>> continue;
>> }
>> }
>
> Wouldn't it make more sense to switch the two if-blocks above?
>
> if let Some(view) = &view {
> if view.can_skip_remote(&remote_name) {
> continue;
> }
> }
>
> if let Some(ref auth_id) = opt_auth_id {
> if view.is_none() {
> let remote_privs = user_info.lookup_privs(auth_id, &["resource", &remote_name]);
> if remote_privs & PRIV_RESOURCE_AUDIT == 0 {
> continue;
> }
> }
> }
>
> Then view filtering takes precedence over checking the permissions, same
> as in get_subscription_status. Might be that I'm misinterpreting this,
> though.
>
I think the order does not really make a difference, no? Sine the
regular check is gated by the `if view.is_none()`.
Anyways, I changed it to the following, since it is a bit nicer to read:
if let Some(view) = &view {
if view.can_skip_remote(&remote_name) {
continue;
}
} else if let Some(ref auth_id) = opt_auth_id {
let remote_privs = user_info.lookup_privs(auth_id, &["resource", &remote_name]);
if remote_privs & PRIV_RESOURCE_AUDIT == 0 {
continue;
}
}
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager v3 08/11] api: subscription status: add support for view parameter
2025-11-12 8:19 ` Shannon Sterz
@ 2025-11-12 10:26 ` Lukas Wagner
0 siblings, 0 replies; 23+ messages in thread
From: Lukas Wagner @ 2025-11-12 10:26 UTC (permalink / raw)
To: Proxmox Datacenter Manager development discussion, Shannon Sterz,
Michael Köppl
On Wed Nov 12, 2025 at 9:19 AM CET, Shannon Sterz wrote:
> On Tue Nov 11, 2025 at 3:46 PM CET, Michael Köppl wrote:
>> 1 comment inline
>>
>> On Thu Nov 6, 2025 at 2:43 PM CET, Lukas Wagner wrote:
>>> A view allows one to get filtered subset of all resources, based on
>>> filter rules defined in a config file. Views integrate with the
>>> permission system - if a user has permissions on /view/{view-id}, then
>>> these privileges are transitively applied to all resources which are
>>> matched by the rules. All other permission checks are replaced if
>>> requesting data through a view.
>>>
>>> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
>>> ---
>>>
>>> Notes:
>>> Changes since v2:
>>> - make sure to not filter out a remote if it has been explicitly
>>> included
>>>
>>> server/src/api/resources.rs | 66 ++++++++++++++++++++++++++++++-------
>>> 1 file changed, 54 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/server/src/api/resources.rs b/server/src/api/resources.rs
>>> index 143335fe..2930da8b 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: {
>>> + schema: VIEW_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: Option<String>,
>>> rpcenv: &mut dyn RpcEnvironment,
>>> ) -> Result<Vec<RemoteSubscriptions>, Error> {
>>> let (remotes_config, _) = pdm_config::remotes::config()?;
>>> @@ -574,9 +579,17 @@ pub async fn get_subscription_status(
>>>
>>> let auth_id = rpcenv.get_auth_id().unwrap().parse()?;
>>> let user_info = CachedUserInfo::new()?;
>>> - let allow_all = user_info
>>> - .check_privs(&auth_id, &["resource"], PRIV_RESOURCE_AUDIT, false)
>>> - .is_ok();
>>> +
>>> + let allow_all = if let Some(view) = &view {
>>> + user_info.check_privs(&auth_id, &["view", view], PRIV_RESOURCE_AUDIT, false)?;
>>> + false
>>> + } else {
>>> + user_info
>>> + .check_privs(&auth_id, &["resource"], PRIV_RESOURCE_AUDIT, false)
>>> + .is_ok()
>>> + };
>>> +
>>> + let view = views::get_optional_view(view.as_deref())?;
>>>
>>> let check_priv = |remote_name: &str| -> bool {
>>> user_info
>>> @@ -590,35 +603,64 @@ pub async fn get_subscription_status(
>>> };
>>>
>>> for (remote_name, remote) in remotes_config {
>>> - if !allow_all && !check_priv(&remote_name) {
>>> + if let Some(view) = &view {
>>> + if view.can_skip_remote(&remote_name) {
>>> + continue;
>>> + }
>>> + } else if !allow_all && !check_priv(&remote_name) {
>>> continue;
>>> }
>>>
>>> + let view = view.clone();
>>> +
>>> let future = async move {
>>> let (node_status, error) =
>>> match get_subscription_info_for_remote(&remote, max_age).await {
>>> - Ok(node_status) => (Some(node_status), None),
>>> + Ok(mut node_status) => {
>>> + node_status.retain(|node, _| {
>>> + if let Some(view) = &view {
>>> + view.is_node_included(&remote.id, node)
>>> + } else {
>>> + true
>>> + }
>>> + });
>>> + (Some(node_status), None)
>>> + }
>>> Err(error) => (None, Some(error.to_string())),
>>> };
>>>
>>> - let mut state = RemoteSubscriptionState::Unknown;
>>> + let state = if let Some(node_status) = &node_status {
>>> + if let Some(view) = view {
>>> + if error.is_some() && !view.is_remote_explicitly_included(&remote.id) {
>>
>> Can this ever be be true? From the code above, you can never have the
>> case that node_status and error are both Some(...) at the same time, so
>> error.is_some() would always evaluate to false here, unless I'm missing
>> something.
>>
>> I think you'd have to put the check outside of this block.
>>
>> if let Some(view) = &view {
>> if error.is_some() && !view.is_remote_explicitly_included(&remote.id) {
>> return None;
>> }
>> }
>>
>> let state = if let Some(node_status) = &node_status {
>> ...
>> }
>>
>
> i thikn this is right, at least i noted something similar in a previous
> version of this patch series already. thanks for spotting it again!
>
> https://lore.proxmox.com/all/DDVMQ0KGW0IP.50W15XZS8TQ@proxmox.com/
>
Ah, my bad. I remember reading your comment about this patch, but I
think it was overshadowed by some other changes, including the
`is_remote_explicitly_included` thing.
I moved the block as suggested.
Thanks :)
>>> + // Don't leak the existence of failed remotes unless they were explicitly
>>> + // pulled in by a `include remote:<id>` rule.
>>> + return 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] 23+ messages in thread
* [pdm-devel] superseded: [PATCH datacenter-manager v3 00/11] backend implementation for view filters
2025-11-06 13:43 [pdm-devel] [PATCH datacenter-manager v3 00/11] backend implementation for view filters Lukas Wagner
` (11 preceding siblings ...)
2025-11-11 15:00 ` [pdm-devel] [PATCH datacenter-manager v3 00/11] backend implementation for view filters Michael Köppl
@ 2025-11-12 10:37 ` Lukas Wagner
12 siblings, 0 replies; 23+ messages in thread
From: Lukas Wagner @ 2025-11-12 10:37 UTC (permalink / raw)
To: Proxmox Datacenter Manager development discussion
superseded by v4:
https://lore.proxmox.com/all/20251112103510.158127-1-l.wagner@proxmox.com/T/#t
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2025-11-12 10:37 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-06 13:43 [pdm-devel] [PATCH datacenter-manager v3 00/11] backend implementation for view filters Lukas Wagner
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 01/11] pdm-api-types: views: add ViewConfig type Lukas Wagner
2025-11-11 10:57 ` Michael Köppl
2025-11-12 10:04 ` Lukas Wagner
2025-11-11 10:58 ` Michael Köppl
2025-11-12 10:05 ` Lukas Wagner
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 02/11] pdm-config: views: add support for views Lukas Wagner
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 03/11] acl: add '/view' and '/view/{view-id}' as allowed ACL paths Lukas Wagner
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 04/11] views: add implementation for view resource filtering Lukas Wagner
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 05/11] api: resources: list: add support for view parameter Lukas Wagner
2025-11-11 14:31 ` Michael Köppl
2025-11-12 10:14 ` Lukas Wagner
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 06/11] api: resources: top entities: " Lukas Wagner
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 07/11] api: resources: status: " Lukas Wagner
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 08/11] api: subscription " Lukas Wagner
2025-11-11 14:46 ` Michael Köppl
2025-11-12 8:19 ` Shannon Sterz
2025-11-12 10:26 ` Lukas Wagner
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 09/11] api: remote-tasks: " Lukas Wagner
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 10/11] pdm-client: resource list: add view-filter parameter Lukas Wagner
2025-11-06 13:43 ` [pdm-devel] [PATCH datacenter-manager v3 11/11] pdm-client: top entities: " Lukas Wagner
2025-11-11 15:00 ` [pdm-devel] [PATCH datacenter-manager v3 00/11] backend implementation for view filters Michael Köppl
2025-11-12 10:37 ` [pdm-devel] superseded: " Lukas Wagner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox