From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 8F78D1FF185 for ; Mon, 17 Nov 2025 15:12:05 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 6D9E21AC08; Mon, 17 Nov 2025 15:12:08 +0100 (CET) From: Lukas Wagner To: pdm-devel@lists.proxmox.com Date: Mon, 17 Nov 2025 15:11:21 +0100 Message-ID: <20251117141122.328559-3-l.wagner@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20251117141122.328559-1-l.wagner@proxmox.com> References: <20251117141122.328559-1-l.wagner@proxmox.com> MIME-Version: 1.0 X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1763388665557 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.030 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pdm-devel] [PATCH datacenter-manager 2/3] views: add 'include-all' param; change semantics when there are no includes X-BeenThere: pdm-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Datacenter Manager development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Datacenter Manager development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pdm-devel-bounces@lists.proxmox.com Sender: "pdm-devel" 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 Signed-off-by: Lukas Wagner --- 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, + /// 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 _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel