[pdm-devel] [PATCH datacenter-manager 2/3] views: add 'include-all' param; change semantics when there are no includes
Lukas Wagner
l.wagner at proxmox.com
Mon Nov 17 15:11:21 CET 2025
Previously, having no 'include' rules would mean that *all* resources
are included. Due view permissions being transitively applied to all
included resources, this also has some security consequences.
For instance, if there is only a single include rule which is then
removed by accident, suddenly any AuthID having privileges on the view
ACL object would then be granted these privileges on *all* resources.
To somewhat mitigate this, the semantics are slightly changed. If there
are no include rules, then no resources will be contained in the view.
A new 'include-all' parameter is added, which aims to emphasize the
intent of *really* including everything.
Suggested-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
Signed-off-by: Lukas Wagner <l.wagner at proxmox.com>
---
lib/pdm-api-types/src/views.rs | 4 +++
server/src/views/mod.rs | 60 +++++++++++++++++++---------------
server/src/views/tests.rs | 35 ++++++++++++++++++--
3 files changed, 69 insertions(+), 30 deletions(-)
diff --git a/lib/pdm-api-types/src/views.rs b/lib/pdm-api-types/src/views.rs
index 950a3a04..82ab8781 100644
--- a/lib/pdm-api-types/src/views.rs
+++ b/lib/pdm-api-types/src/views.rs
@@ -58,6 +58,9 @@ pub struct ViewConfig {
#[updater(skip)]
pub id: String,
+ /// Include all resources by default.
+ pub include_all: Option<bool>,
+
/// List of includes.
#[serde(default, skip_serializing_if = "Vec::is_empty")]
#[updater(serde(skip_serializing_if = "Option::is_none"))]
@@ -261,6 +264,7 @@ mod test {
fn config_smoke_test() {
let config = "
view: some-view
+ include-all true
include exact:remote=someremote
include remote=someremote
include resource-type=qemu
diff --git a/server/src/views/mod.rs b/server/src/views/mod.rs
index 8b4c70a4..3669ace4 100644
--- a/server/src/views/mod.rs
+++ b/server/src/views/mod.rs
@@ -64,31 +64,34 @@ impl View {
/// 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.matches(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
+ if matches_any_exclude_remote {
+ return true;
+ }
+
+ if self.config.include_all.unwrap_or_default() {
+ return false;
+ }
+
+ for include in &self.config.include {
+ if let FilterRule::Remote(r) = include {
+ if r.matches(remote) {
+ return false;
+ }
+ } else {
+ // If there is any other type of rule, we cannot safely infer whether we can skip
+ // the remote (e.g. for 'tag' matches, we have to check *all* remotes for resources
+ // with a given tag)
+ return false;
+ }
+ }
+
+ true
}
/// Check if a remote is *explicitly* included (and not excluded).
@@ -96,18 +99,22 @@ impl View {
/// 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 included = if self.config.include_all.unwrap_or_default() {
+ true
+ } else {
+ 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
+ included && !matches_exclude_remote
}
/// Check if a node is matched by the filter rules.
@@ -131,8 +138,7 @@ impl View {
}
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)
+ if self.config.include_all.unwrap_or_default() {
return true;
}
diff --git a/server/src/views/tests.rs b/server/src/views/tests.rs
index 14d94ac9..0d83ae70 100644
--- a/server/src/views/tests.rs
+++ b/server/src/views/tests.rs
@@ -135,6 +135,7 @@ fn exclude_remotes() {
FilterRule::Remote(StringMatcher::Exact("remote-a".into())),
FilterRule::Remote(StringMatcher::Exact("remote-b".into())),
],
+ include_all: Some(true),
..Default::default()
};
@@ -184,6 +185,8 @@ fn include_exclude_remotes() {
FilterRule::Remote(StringMatcher::Exact("remote-b".into())),
FilterRule::Remote(StringMatcher::Exact("remote-c".into())),
],
+
+ ..Default::default()
};
run_test(
config.clone(),
@@ -224,6 +227,7 @@ fn include_exclude_remotes() {
fn empty_config() {
let config = ViewConfig {
id: "empty".into(),
+ include_all: Some(true),
..Default::default()
};
run_test(
@@ -301,6 +305,7 @@ fn exclude_type() {
FilterRule::ResourceType(EnumMatcher(ResourceType::PveStorage)),
FilterRule::ResourceType(EnumMatcher(ResourceType::PveQemu)),
],
+ include_all: Some(true),
..Default::default()
},
&[
@@ -329,6 +334,7 @@ fn include_exclude_type() {
exclude: vec![FilterRule::ResourceType(EnumMatcher(
ResourceType::PveStorage,
))],
+ ..Default::default()
},
&[
(
@@ -357,6 +363,7 @@ fn include_exclude_tags() {
FilterRule::Tag(StringMatcher::Exact("tag2".to_string())),
],
exclude: vec![FilterRule::Tag(StringMatcher::Exact("tag3".to_string()))],
+ ..Default::default()
},
&[
(
@@ -404,6 +411,7 @@ fn include_exclude_resource_pool() {
exclude: vec![FilterRule::ResourcePool(StringMatcher::Exact(
"pool2".to_string(),
))],
+ ..Default::default()
},
&[
(
@@ -459,6 +467,7 @@ fn include_exclude_resource_id() {
"remote/{REMOTE}/storage/{NODE}/otherstorage"
))),
],
+ ..Default::default()
},
&[
(
@@ -509,6 +518,7 @@ fn node_included() {
exclude: vec![FilterRule::Remote(StringMatcher::Exact(
"remote-b".to_string(),
))],
+ ..Default::default()
});
assert!(view.is_node_included("remote-a", "somenode"));
@@ -528,6 +538,7 @@ fn can_skip_remote_if_excluded() {
exclude: vec![FilterRule::Remote(StringMatcher::Exact(
"remote-b".to_string(),
))],
+ include_all: Some(true),
});
assert!(!view.can_skip_remote("remote-a"));
@@ -542,6 +553,7 @@ fn can_skip_remote_if_included() {
"remote-b".to_string(),
))],
exclude: vec![],
+ ..Default::default()
});
assert!(!view.can_skip_remote("remote-b"));
@@ -559,6 +571,7 @@ fn can_skip_remote_cannot_skip_if_any_other_include() {
)),
],
exclude: vec![],
+ ..Default::default()
});
assert!(!view.can_skip_remote("remote-b"));
@@ -575,6 +588,7 @@ fn can_skip_remote_explicit_remote_exclude() {
exclude: vec![FilterRule::Remote(StringMatcher::Exact(
"remote-a".to_string(),
))],
+ ..Default::default()
});
assert!(view.can_skip_remote("remote-a"));
@@ -584,8 +598,19 @@ fn can_skip_remote_explicit_remote_exclude() {
fn can_skip_remote_with_empty_config() {
let view = View::new(ViewConfig {
id: "abc".into(),
- include: vec![],
- exclude: vec![],
+ ..Default::default()
+ });
+
+ assert!(view.can_skip_remote("remote-a"));
+ assert!(view.can_skip_remote("remote-b"));
+}
+
+#[test]
+fn can_skip_remote_cannot_skip_if_all_included() {
+ let view = View::new(ViewConfig {
+ id: "abc".into(),
+ include_all: Some(true),
+ ..Default::default()
});
assert!(!view.can_skip_remote("remote-a"));
@@ -600,6 +625,7 @@ fn can_skip_remote_with_no_remote_includes() {
"resource/remote-a/guest/100".to_string(),
))],
exclude: vec![],
+ ..Default::default()
});
assert!(!view.can_skip_remote("remote-a"));
@@ -614,6 +640,7 @@ fn explicitly_included_remote() {
"remote-b".to_string(),
))],
exclude: vec![],
+ ..Default::default()
});
assert!(view.is_remote_explicitly_included("remote-b"));
@@ -629,6 +656,7 @@ fn included_and_excluded_same_remote() {
exclude: vec![FilterRule::Remote(StringMatcher::Exact(
"remote-b".to_string(),
))],
+ ..Default::default()
});
assert!(!view.is_remote_explicitly_included("remote-b"));
@@ -640,8 +668,9 @@ fn not_explicitly_included_remote() {
id: "abc".into(),
include: vec![],
exclude: vec![],
+ include_all: Some(true),
});
// Assert that is not *explicitly* included
- assert!(!view.is_remote_explicitly_included("remote-b"));
+ assert!(view.is_remote_explicitly_included("remote-b"));
}
--
2.47.3
More information about the pdm-devel
mailing list