all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pdm-devel] [RFC proxmox{, -datacenter-manager, -yew-comp} 0/8] make security groups expandable in firewall rules list
@ 2025-12-05 15:25 Hannes Laimer
  2025-12-05 15:25 ` [pdm-devel] [PATCH proxmox 1/4] pve-api-types: rename ListFirewallRules to FirewallRule Hannes Laimer
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: Hannes Laimer @ 2025-12-05 15:25 UTC (permalink / raw)
  To: pdm-devel

This contains some rough edges, mostly UI wise, but I'd like to get some
feedback on if we like this approach. Currently we don't really
know what a security group actually contains, in the list currently it's
a bit of a black box what a group actually does. Finding out what rules it
contains is a little cumbersome. This should make that easier. It seemed
like a good place too, I considerd an extra tab maybe. But especially
for read-only I think this is better.

This also contains a renaming, mostly cause I had it in the same repo
already. If wanted, I can split that and send it separately. The
pve-api.json patch contains changes from [1].

[1] https://lore.proxmox.com/pve-devel/20251128145846.328173-1-h.laimer@proxmox.com/T/#u

proxmox:

Hannes Laimer (4):
  pve-api-types: rename ListFirewallRules to FirewallRule
  pve-api-types: update pve-api.json
  pve-api-types: add security group GET endpoints
  pve-api-types: regenerate

 pve-api-types/generate.pl            |  15 +-
 pve-api-types/pve-api.json           |   1 +
 pve-api-types/src/generated/code.rs  |  77 ++++++-
 pve-api-types/src/generated/types.rs | 294 +++++++++++++++------------
 4 files changed, 240 insertions(+), 147 deletions(-)


proxmox-datacenter-manager:

Hannes Laimer (2):
  pdm: rename ListFirewallRules to FirewallRule
  api: firewall: add pve firewall security group GET endpoints

 lib/pdm-client/src/lib.rs      |  8 ++--
 server/src/api/pve/firewall.rs | 81 ++++++++++++++++++++++++++++++----
 2 files changed, 77 insertions(+), 12 deletions(-)


proxmox-yew-comp:

Hannes Laimer (2):
  firewall: rules: rename ListFirewallRules to FirewallRule
  firewall: rules: make security group entries expandable

 src/firewall/context.rs |  12 +++
 src/firewall/rules.rs   | 208 +++++++++++++++++++++++++++++++++++-----
 2 files changed, 197 insertions(+), 23 deletions(-)


Summary over all repositories:
  8 files changed, 514 insertions(+), 182 deletions(-)

-- 
Generated by git-murpp 0.8.1


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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [pdm-devel] [PATCH proxmox 1/4] pve-api-types: rename ListFirewallRules to FirewallRule
  2025-12-05 15:25 [pdm-devel] [RFC proxmox{, -datacenter-manager, -yew-comp} 0/8] make security groups expandable in firewall rules list Hannes Laimer
@ 2025-12-05 15:25 ` Hannes Laimer
  2025-12-05 15:25 ` [pdm-devel] [PATCH proxmox 2/4] pve-api-types: update pve-api.json Hannes Laimer
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Hannes Laimer @ 2025-12-05 15:25 UTC (permalink / raw)
  To: pdm-devel

Since this is the type for just an item, not the whole list,
having it named `ListFirewallRules` didn't make much sense.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 pve-api-types/generate.pl | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/pve-api-types/generate.pl b/pve-api-types/generate.pl
index 3cebe321..17f7e1e5 100755
--- a/pve-api-types/generate.pl
+++ b/pve-api-types/generate.pl
@@ -414,13 +414,13 @@ api(GET => '/nodes/{node}/qemu/{vmid}/firewall/options', 'qemu_firewall_options'
 api(PUT => '/nodes/{node}/qemu/{vmid}/firewall/options', 'set_qemu_firewall_options', 'param-name' => 'UpdateGuestFirewallOptions');
 
 # rules
-api(GET => '/cluster/firewall/rules', 'list_cluster_firewall_rules', 'return-name' => 'ListFirewallRules');
+api(GET => '/cluster/firewall/rules', 'list_cluster_firewall_rules', 'return-name' => 'FirewallRule');
 
-api(GET => '/nodes/{node}/firewall/rules', 'list_node_firewall_rules', 'return-name' => 'ListFirewallRules');
+api(GET => '/nodes/{node}/firewall/rules', 'list_node_firewall_rules', 'return-name' => 'FirewallRule');
 
-api(GET => '/nodes/{node}/lxc/{vmid}/firewall/rules', 'list_lxc_firewall_rules', 'return-name' => 'ListFirewallRules');
-api(GET => '/nodes/{node}/qemu/{vmid}/firewall/rules', 'list_qemu_firewall_rules', 'return-name' => 'ListFirewallRules');
-Schema2Rust::derive('ListFirewallRules' => 'Clone', 'PartialEq');
+api(GET => '/nodes/{node}/lxc/{vmid}/firewall/rules', 'list_lxc_firewall_rules', 'return-name' => 'FirewallRule');
+api(GET => '/nodes/{node}/qemu/{vmid}/firewall/rules', 'list_qemu_firewall_rules', 'return-name' => 'FirewallRule');
+Schema2Rust::derive('FirewallRule' => 'Clone', 'PartialEq');
 
 Schema2Rust::generate_enum('SdnObjectState', {
     type => 'string',
-- 
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] 9+ messages in thread

* [pdm-devel] [PATCH proxmox 2/4] pve-api-types: update pve-api.json
  2025-12-05 15:25 [pdm-devel] [RFC proxmox{, -datacenter-manager, -yew-comp} 0/8] make security groups expandable in firewall rules list Hannes Laimer
  2025-12-05 15:25 ` [pdm-devel] [PATCH proxmox 1/4] pve-api-types: rename ListFirewallRules to FirewallRule Hannes Laimer
@ 2025-12-05 15:25 ` Hannes Laimer
  2025-12-05 15:25 ` [pdm-devel] [PATCH proxmox 3/4] pve-api-types: add security group GET endpoints Hannes Laimer
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Hannes Laimer @ 2025-12-05 15:25 UTC (permalink / raw)
  To: pdm-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 pve-api-types/pve-api.json | 1 +
 1 file changed, 1 insertion(+)

diff --git a/pve-api-types/pve-api.json b/pve-api-types/pve-api.json
index 7674a7cb..4b73dbe5 100644
--- a/pve-api-types/pve-api.json
+++ b/pve-api-types/pve-api.json
@@ -5200,6 +5200,7 @@
                               "items": {
                                  "properties": {
                                     "comment": {
+                                       "description": "Optional comment or description.",
                                        "optional": 1,
                                        "type": "string"
                                     },
-- 
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] 9+ messages in thread

* [pdm-devel] [PATCH proxmox 3/4] pve-api-types: add security group GET endpoints
  2025-12-05 15:25 [pdm-devel] [RFC proxmox{, -datacenter-manager, -yew-comp} 0/8] make security groups expandable in firewall rules list Hannes Laimer
  2025-12-05 15:25 ` [pdm-devel] [PATCH proxmox 1/4] pve-api-types: rename ListFirewallRules to FirewallRule Hannes Laimer
  2025-12-05 15:25 ` [pdm-devel] [PATCH proxmox 2/4] pve-api-types: update pve-api.json Hannes Laimer
@ 2025-12-05 15:25 ` Hannes Laimer
  2025-12-05 15:25 ` [pdm-devel] [PATCH proxmox 4/4] pve-api-types: regenerate Hannes Laimer
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Hannes Laimer @ 2025-12-05 15:25 UTC (permalink / raw)
  To: pdm-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 pve-api-types/generate.pl | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/pve-api-types/generate.pl b/pve-api-types/generate.pl
index 17f7e1e5..599802c8 100755
--- a/pve-api-types/generate.pl
+++ b/pve-api-types/generate.pl
@@ -398,6 +398,11 @@ Schema2Rust::register_format('pve-fw-conntrack-helper' => {
     kind => 'array',
 });
 
+# security groups
+api(GET => '/cluster/firewall/groups', 'list_firewall_security_groups', 'return-name' => 'FirewallSecurityGroup');
+api(GET => '/cluster/firewall/groups/{group}', 'list_firewall_security_group_rules', 'return-name' => 'FirewallRule');
+api(GET => '/cluster/firewall/groups/{group}/{pos}', 'firewall_security_group_rule', 'return-name' => 'FirewallRule');
+
 # options
 # FIXME: to use a better return value than `Value`, we first must fix the return schema there
 api(GET => '/cluster/options', 'cluster_options', 'output-type' => 'serde_json::Value');
-- 
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] 9+ messages in thread

* [pdm-devel] [PATCH proxmox 4/4] pve-api-types: regenerate
  2025-12-05 15:25 [pdm-devel] [RFC proxmox{, -datacenter-manager, -yew-comp} 0/8] make security groups expandable in firewall rules list Hannes Laimer
                   ` (2 preceding siblings ...)
  2025-12-05 15:25 ` [pdm-devel] [PATCH proxmox 3/4] pve-api-types: add security group GET endpoints Hannes Laimer
@ 2025-12-05 15:25 ` Hannes Laimer
  2025-12-05 15:25 ` [pdm-devel] [PATCH proxmox-datacenter-manager 1/2] pdm: rename ListFirewallRules to FirewallRule Hannes Laimer
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Hannes Laimer @ 2025-12-05 15:25 UTC (permalink / raw)
  To: pdm-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 pve-api-types/src/generated/code.rs  |  77 ++++++-
 pve-api-types/src/generated/types.rs | 294 +++++++++++++++------------
 2 files changed, 229 insertions(+), 142 deletions(-)

diff --git a/pve-api-types/src/generated/code.rs b/pve-api-types/src/generated/code.rs
index f364f9cd..b583c2e0 100644
--- a/pve-api-types/src/generated/code.rs
+++ b/pve-api-types/src/generated/code.rs
@@ -60,9 +60,6 @@
 /// - /cluster/firewall
 /// - /cluster/firewall/aliases
 /// - /cluster/firewall/aliases/{name}
-/// - /cluster/firewall/groups
-/// - /cluster/firewall/groups/{group}
-/// - /cluster/firewall/groups/{group}/{pos}
 /// - /cluster/firewall/ipset
 /// - /cluster/firewall/ipset/{name}
 /// - /cluster/firewall/ipset/{name}/{cidr}
@@ -445,6 +442,15 @@ pub trait PveClient {
         Err(Error::Other("create_zone not implemented"))
     }
 
+    /// Get single rule data.
+    async fn firewall_security_group_rule(
+        &self,
+        group: &str,
+        pos: u64,
+    ) -> Result<FirewallRule, Error> {
+        Err(Error::Other("firewall_security_group_rule not implemented"))
+    }
+
     /// Get APT repository information.
     async fn get_apt_repositories(&self, node: &str) -> Result<APTRepositoriesResult, Error> {
         Err(Error::Other("get_apt_repositories not implemented"))
@@ -512,7 +518,7 @@ pub trait PveClient {
     }
 
     /// List rules.
-    async fn list_cluster_firewall_rules(&self) -> Result<Vec<ListFirewallRules>, Error> {
+    async fn list_cluster_firewall_rules(&self) -> Result<Vec<FirewallRule>, Error> {
         Err(Error::Other("list_cluster_firewall_rules not implemented"))
     }
 
@@ -531,6 +537,23 @@ pub trait PveClient {
         Err(Error::Other("list_domains not implemented"))
     }
 
+    /// List rules.
+    async fn list_firewall_security_group_rules(
+        &self,
+        group: &str,
+    ) -> Result<Vec<FirewallRule>, Error> {
+        Err(Error::Other(
+            "list_firewall_security_group_rules not implemented",
+        ))
+    }
+
+    /// List security groups.
+    async fn list_firewall_security_groups(&self) -> Result<Vec<FirewallSecurityGroup>, Error> {
+        Err(Error::Other(
+            "list_firewall_security_groups not implemented",
+        ))
+    }
+
     /// LXC container index (per node).
     async fn list_lxc(&self, node: &str) -> Result<Vec<LxcEntry>, Error> {
         Err(Error::Other("list_lxc not implemented"))
@@ -541,7 +564,7 @@ pub trait PveClient {
         &self,
         node: &str,
         vmid: u32,
-    ) -> Result<Vec<ListFirewallRules>, Error> {
+    ) -> Result<Vec<FirewallRule>, Error> {
         Err(Error::Other("list_lxc_firewall_rules not implemented"))
     }
 
@@ -555,7 +578,7 @@ pub trait PveClient {
     }
 
     /// List rules.
-    async fn list_node_firewall_rules(&self, node: &str) -> Result<Vec<ListFirewallRules>, Error> {
+    async fn list_node_firewall_rules(&self, node: &str) -> Result<Vec<FirewallRule>, Error> {
         Err(Error::Other("list_node_firewall_rules not implemented"))
     }
 
@@ -574,7 +597,7 @@ pub trait PveClient {
         &self,
         node: &str,
         vmid: u32,
-    ) -> Result<Vec<ListFirewallRules>, Error> {
+    ) -> Result<Vec<FirewallRule>, Error> {
         Err(Error::Other("list_qemu_firewall_rules not implemented"))
     }
 
@@ -1080,6 +1103,20 @@ where
         self.0.post(url, &params).await?.nodata()
     }
 
+    /// Get single rule data.
+    async fn firewall_security_group_rule(
+        &self,
+        group: &str,
+        pos: u64,
+    ) -> Result<FirewallRule, Error> {
+        let url = &format!(
+            "/api2/extjs/cluster/firewall/groups/{}/{}",
+            percent_encode(group.as_bytes(), percent_encoding::NON_ALPHANUMERIC),
+            pos
+        );
+        Ok(self.0.get(url).await?.expect_json()?.data)
+    }
+
     /// Get APT repository information.
     async fn get_apt_repositories(&self, node: &str) -> Result<APTRepositoriesResult, Error> {
         let url = &format!(
@@ -1222,7 +1259,7 @@ where
     }
 
     /// List rules.
-    async fn list_cluster_firewall_rules(&self) -> Result<Vec<ListFirewallRules>, Error> {
+    async fn list_cluster_firewall_rules(&self) -> Result<Vec<FirewallRule>, Error> {
         let url = "/api2/extjs/cluster/firewall/rules";
         Ok(self.0.get(url).await?.expect_json()?.data)
     }
@@ -1248,6 +1285,24 @@ where
         Ok(self.0.get(url).await?.expect_json()?.data)
     }
 
+    /// List rules.
+    async fn list_firewall_security_group_rules(
+        &self,
+        group: &str,
+    ) -> Result<Vec<FirewallRule>, Error> {
+        let url = &format!(
+            "/api2/extjs/cluster/firewall/groups/{}",
+            percent_encode(group.as_bytes(), percent_encoding::NON_ALPHANUMERIC)
+        );
+        Ok(self.0.get(url).await?.expect_json()?.data)
+    }
+
+    /// List security groups.
+    async fn list_firewall_security_groups(&self) -> Result<Vec<FirewallSecurityGroup>, Error> {
+        let url = "/api2/extjs/cluster/firewall/groups";
+        Ok(self.0.get(url).await?.expect_json()?.data)
+    }
+
     /// LXC container index (per node).
     async fn list_lxc(&self, node: &str) -> Result<Vec<LxcEntry>, Error> {
         let url = &format!(
@@ -1262,7 +1317,7 @@ where
         &self,
         node: &str,
         vmid: u32,
-    ) -> Result<Vec<ListFirewallRules>, Error> {
+    ) -> Result<Vec<FirewallRule>, Error> {
         let url = &format!(
             "/api2/extjs/nodes/{}/lxc/{}/firewall/rules",
             percent_encode(node.as_bytes(), percent_encoding::NON_ALPHANUMERIC),
@@ -1287,7 +1342,7 @@ where
     }
 
     /// List rules.
-    async fn list_node_firewall_rules(&self, node: &str) -> Result<Vec<ListFirewallRules>, Error> {
+    async fn list_node_firewall_rules(&self, node: &str) -> Result<Vec<FirewallRule>, Error> {
         let url = &format!(
             "/api2/extjs/nodes/{}/firewall/rules",
             percent_encode(node.as_bytes(), percent_encoding::NON_ALPHANUMERIC)
@@ -1317,7 +1372,7 @@ where
         &self,
         node: &str,
         vmid: u32,
-    ) -> Result<Vec<ListFirewallRules>, Error> {
+    ) -> Result<Vec<FirewallRule>, Error> {
         let url = &format!(
             "/api2/extjs/nodes/{}/qemu/{}/firewall/rules",
             percent_encode(node.as_bytes(), percent_encoding::NON_ALPHANUMERIC),
diff --git a/pve-api-types/src/generated/types.rs b/pve-api-types/src/generated/types.rs
index 26f07a5a..b7dc983f 100644
--- a/pve-api-types/src/generated/types.rs
+++ b/pve-api-types/src/generated/types.rs
@@ -1891,6 +1891,169 @@ pub enum FirewallLogLevel {
 serde_plain::derive_display_from_serialize!(FirewallLogLevel);
 serde_plain::derive_fromstr_from_deserialize!(FirewallLogLevel);
 
+#[api(
+    properties: {
+        action: {
+            type: String,
+        },
+        comment: {
+            optional: true,
+            type: String,
+        },
+        dest: {
+            optional: true,
+            type: String,
+        },
+        dport: {
+            optional: true,
+            type: String,
+        },
+        enable: {
+            optional: true,
+            type: Integer,
+        },
+        "icmp-type": {
+            optional: true,
+            type: String,
+        },
+        iface: {
+            optional: true,
+            type: String,
+        },
+        ipversion: {
+            optional: true,
+            type: Integer,
+        },
+        log: {
+            optional: true,
+            type: FirewallLogLevel,
+        },
+        "macro": {
+            optional: true,
+            type: String,
+        },
+        pos: {
+            type: Integer,
+        },
+        proto: {
+            optional: true,
+            type: String,
+        },
+        source: {
+            optional: true,
+            type: String,
+        },
+        sport: {
+            optional: true,
+            type: String,
+        },
+        type: {
+            type: String,
+        },
+    },
+)]
+/// Object.
+#[derive(Clone, Debug, PartialEq, serde::Deserialize, serde::Serialize)]
+pub struct FirewallRule {
+    /// Rule action ('ACCEPT', 'DROP', 'REJECT') or security group name
+    pub action: String,
+
+    /// Descriptive comment
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub comment: Option<String>,
+
+    /// Restrict packet destination address
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub dest: Option<String>,
+
+    /// Restrict TCP/UDP destination port
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub dport: Option<String>,
+
+    /// Flag to enable/disable a rule
+    #[serde(deserialize_with = "proxmox_serde::perl::deserialize_i64")]
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub enable: Option<i64>,
+
+    /// Specify icmp-type. Only valid if proto equals 'icmp' or
+    /// 'icmpv6'/'ipv6-icmp'
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    #[serde(rename = "icmp-type")]
+    pub icmp_type: Option<String>,
+
+    /// Network interface name. You have to use network configuration key names
+    /// for VMs and containers
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub iface: Option<String>,
+
+    /// IP version (4 or 6) - automatically determined from source/dest
+    /// addresses
+    #[serde(deserialize_with = "proxmox_serde::perl::deserialize_i64")]
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub ipversion: Option<i64>,
+
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub log: Option<FirewallLogLevel>,
+
+    /// Use predefined standard macro
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    #[serde(rename = "macro")]
+    pub r#macro: Option<String>,
+
+    /// Rule position in the ruleset
+    #[serde(deserialize_with = "proxmox_serde::perl::deserialize_i64")]
+    pub pos: i64,
+
+    /// IP protocol. You can use protocol names ('tcp'/'udp') or simple numbers,
+    /// as defined in '/etc/protocols'
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub proto: Option<String>,
+
+    /// Restrict packet source address
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub source: Option<String>,
+
+    /// Restrict TCP/UDP source port
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub sport: Option<String>,
+
+    /// Rule type
+    #[serde(rename = "type")]
+    pub ty: String,
+}
+
+#[api(
+    properties: {
+        comment: {
+            optional: true,
+            type: String,
+        },
+        digest: {
+            max_length: 64,
+            type: String,
+        },
+        group: {
+            max_length: 18,
+            min_length: 2,
+            type: String,
+        },
+    },
+)]
+/// Object.
+#[derive(Debug, serde::Deserialize, serde::Serialize)]
+pub struct FirewallSecurityGroup {
+    /// Optional comment or description.
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub comment: Option<String>,
+
+    /// Prevent changes if current configuration file has a different digest.
+    /// This can be used to prevent concurrent modifications.
+    pub digest: String,
+
+    /// Security Group name.
+    pub group: String,
+}
+
 #[api]
 /// Firewall conntrack helper.
 #[derive(Clone, Copy, Debug, Eq, PartialEq, serde::Deserialize, serde::Serialize)]
@@ -2191,137 +2354,6 @@ pub enum ListControllersType {
 serde_plain::derive_display_from_serialize!(ListControllersType);
 serde_plain::derive_fromstr_from_deserialize!(ListControllersType);
 
-#[api(
-    properties: {
-        action: {
-            type: String,
-        },
-        comment: {
-            optional: true,
-            type: String,
-        },
-        dest: {
-            optional: true,
-            type: String,
-        },
-        dport: {
-            optional: true,
-            type: String,
-        },
-        enable: {
-            optional: true,
-            type: Integer,
-        },
-        "icmp-type": {
-            optional: true,
-            type: String,
-        },
-        iface: {
-            optional: true,
-            type: String,
-        },
-        ipversion: {
-            optional: true,
-            type: Integer,
-        },
-        log: {
-            optional: true,
-            type: FirewallLogLevel,
-        },
-        "macro": {
-            optional: true,
-            type: String,
-        },
-        pos: {
-            type: Integer,
-        },
-        proto: {
-            optional: true,
-            type: String,
-        },
-        source: {
-            optional: true,
-            type: String,
-        },
-        sport: {
-            optional: true,
-            type: String,
-        },
-        type: {
-            type: String,
-        },
-    },
-)]
-/// Object.
-#[derive(Clone, Debug, PartialEq, serde::Deserialize, serde::Serialize)]
-pub struct ListFirewallRules {
-    /// Rule action ('ACCEPT', 'DROP', 'REJECT') or security group name
-    pub action: String,
-
-    /// Descriptive comment
-    #[serde(default, skip_serializing_if = "Option::is_none")]
-    pub comment: Option<String>,
-
-    /// Restrict packet destination address
-    #[serde(default, skip_serializing_if = "Option::is_none")]
-    pub dest: Option<String>,
-
-    /// Restrict TCP/UDP destination port
-    #[serde(default, skip_serializing_if = "Option::is_none")]
-    pub dport: Option<String>,
-
-    /// Flag to enable/disable a rule
-    #[serde(deserialize_with = "proxmox_serde::perl::deserialize_i64")]
-    #[serde(default, skip_serializing_if = "Option::is_none")]
-    pub enable: Option<i64>,
-
-    /// Specify icmp-type. Only valid if proto equals 'icmp' or
-    /// 'icmpv6'/'ipv6-icmp'
-    #[serde(default, skip_serializing_if = "Option::is_none")]
-    #[serde(rename = "icmp-type")]
-    pub icmp_type: Option<String>,
-
-    /// Network interface name. You have to use network configuration key names
-    /// for VMs and containers
-    #[serde(default, skip_serializing_if = "Option::is_none")]
-    pub iface: Option<String>,
-
-    /// IP version (4 or 6) - automatically determined from source/dest
-    /// addresses
-    #[serde(deserialize_with = "proxmox_serde::perl::deserialize_i64")]
-    #[serde(default, skip_serializing_if = "Option::is_none")]
-    pub ipversion: Option<i64>,
-
-    #[serde(default, skip_serializing_if = "Option::is_none")]
-    pub log: Option<FirewallLogLevel>,
-
-    /// Use predefined standard macro
-    #[serde(default, skip_serializing_if = "Option::is_none")]
-    #[serde(rename = "macro")]
-    pub r#macro: Option<String>,
-
-    /// Rule position in the ruleset
-    #[serde(deserialize_with = "proxmox_serde::perl::deserialize_i64")]
-    pub pos: i64,
-
-    /// IP protocol. You can use protocol names ('tcp'/'udp') or simple numbers,
-    /// as defined in '/etc/protocols'
-    #[serde(default, skip_serializing_if = "Option::is_none")]
-    pub proto: Option<String>,
-
-    /// Restrict packet source address
-    #[serde(default, skip_serializing_if = "Option::is_none")]
-    pub source: Option<String>,
-
-    /// Restrict TCP/UDP source port
-    #[serde(default, skip_serializing_if = "Option::is_none")]
-    pub sport: Option<String>,
-
-    /// Rule type
-    #[serde(rename = "type")]
-    pub ty: String,
-}
-
 #[api]
 /// Only list specific interface types.
 #[derive(Clone, Copy, Debug, Eq, PartialEq, serde::Deserialize, serde::Serialize)]
-- 
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] 9+ messages in thread

* [pdm-devel] [PATCH proxmox-datacenter-manager 1/2] pdm: rename ListFirewallRules to FirewallRule
  2025-12-05 15:25 [pdm-devel] [RFC proxmox{, -datacenter-manager, -yew-comp} 0/8] make security groups expandable in firewall rules list Hannes Laimer
                   ` (3 preceding siblings ...)
  2025-12-05 15:25 ` [pdm-devel] [PATCH proxmox 4/4] pve-api-types: regenerate Hannes Laimer
@ 2025-12-05 15:25 ` Hannes Laimer
  2025-12-05 15:25 ` [pdm-devel] [PATCH proxmox-datacenter-manager 2/2] api: firewall: add pve firewall security group GET endpoints Hannes Laimer
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Hannes Laimer @ 2025-12-05 15:25 UTC (permalink / raw)
  To: pdm-devel

Adjust to the rename in pve-api-types.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 lib/pdm-client/src/lib.rs      |  8 ++++----
 server/src/api/pve/firewall.rs | 16 ++++++++--------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/lib/pdm-client/src/lib.rs b/lib/pdm-client/src/lib.rs
index 01ee6f7..9467142 100644
--- a/lib/pdm-client/src/lib.rs
+++ b/lib/pdm-client/src/lib.rs
@@ -475,7 +475,7 @@ impl<T: HttpApiClient> PdmClient<T> {
     pub async fn pve_get_cluster_firewall_rules(
         &self,
         remote: &str,
-    ) -> Result<Vec<pve_api_types::ListFirewallRules>, Error> {
+    ) -> Result<Vec<pve_api_types::FirewallRule>, Error> {
         let path = format!("/api2/extjs/pve/remotes/{remote}/firewall/rules");
         Ok(self.0.get(&path).await?.expect_json()?.data)
     }
@@ -484,7 +484,7 @@ impl<T: HttpApiClient> PdmClient<T> {
         &self,
         remote: &str,
         node: &str,
-    ) -> Result<Vec<pve_api_types::ListFirewallRules>, Error> {
+    ) -> Result<Vec<pve_api_types::FirewallRule>, Error> {
         let path = format!("/api2/extjs/pve/remotes/{remote}/nodes/{node}/firewall/rules");
         Ok(self.0.get(&path).await?.expect_json()?.data)
     }
@@ -494,7 +494,7 @@ impl<T: HttpApiClient> PdmClient<T> {
         remote: &str,
         node: Option<&str>,
         vmid: u32,
-    ) -> Result<Vec<pve_api_types::ListFirewallRules>, Error> {
+    ) -> Result<Vec<pve_api_types::FirewallRule>, Error> {
         let path = ApiPathBuilder::new(format!(
             "/api2/extjs/pve/remotes/{remote}/lxc/{vmid}/firewall/rules"
         ))
@@ -508,7 +508,7 @@ impl<T: HttpApiClient> PdmClient<T> {
         remote: &str,
         node: Option<&str>,
         vmid: u32,
-    ) -> Result<Vec<pve_api_types::ListFirewallRules>, Error> {
+    ) -> Result<Vec<pve_api_types::FirewallRule>, Error> {
         let path = ApiPathBuilder::new(format!(
             "/api2/extjs/pve/remotes/{remote}/qemu/{vmid}/firewall/rules"
         ))
diff --git a/server/src/api/pve/firewall.rs b/server/src/api/pve/firewall.rs
index af11d58..e60961c 100644
--- a/server/src/api/pve/firewall.rs
+++ b/server/src/api/pve/firewall.rs
@@ -529,7 +529,7 @@ pub async fn node_firewall_status(
     returns: {
         type: Array,
         description: "List cluster firewall rules.",
-        items: { type: pve_api_types::ListFirewallRules },
+        items: { type: pve_api_types::FirewallRule },
     },
     access: {
         permission: &Permission::Privilege(&["resource", "{remote}"], PRIV_RESOURCE_AUDIT, false),
@@ -539,7 +539,7 @@ pub async fn node_firewall_status(
 pub async fn cluster_firewall_rules(
     remote: String,
     _rpcenv: &mut dyn RpcEnvironment,
-) -> Result<Vec<pve_api_types::ListFirewallRules>, Error> {
+) -> Result<Vec<pve_api_types::FirewallRule>, Error> {
     let (remotes, _) = pdm_config::remotes::config()?;
     let pve = connect_to_remote(&remotes, &remote)?;
 
@@ -646,7 +646,7 @@ pub async fn update_node_firewall_options(
     returns: {
         type: Array,
         description: "List node firewall rules.",
-        items: { type: pve_api_types::ListFirewallRules },
+        items: { type: pve_api_types::FirewallRule },
     },
     access: {
         permission: &Permission::Privilege(&["resource", "{remote}"], PRIV_RESOURCE_AUDIT, false),
@@ -657,7 +657,7 @@ pub async fn node_firewall_rules(
     remote: String,
     node: String,
     _rpcenv: &mut dyn RpcEnvironment,
-) -> Result<Vec<pve_api_types::ListFirewallRules>, Error> {
+) -> Result<Vec<pve_api_types::FirewallRule>, Error> {
     let (remotes, _) = pdm_config::remotes::config()?;
     let pve = connect_to_remote(&remotes, &remote)?;
 
@@ -782,7 +782,7 @@ pub async fn update_qemu_firewall_options(
     returns: {
         type: Array,
         description: "List LXC firewall rules.",
-        items: { type: pve_api_types::ListFirewallRules },
+        items: { type: pve_api_types::FirewallRule },
     },
     access: {
         permission: &Permission::Privilege(&["resource", "{remote}", "guest", "{vmid}"], PRIV_RESOURCE_AUDIT, false),
@@ -794,7 +794,7 @@ pub async fn lxc_firewall_rules(
     node: Option<String>,
     vmid: u32,
     _rpcenv: &mut dyn RpcEnvironment,
-) -> Result<Vec<pve_api_types::ListFirewallRules>, Error> {
+) -> Result<Vec<pve_api_types::FirewallRule>, Error> {
     let (remotes, _) = pdm_config::remotes::config()?;
 
     let pve = connect_to_remote(&remotes, &remote)?;
@@ -818,7 +818,7 @@ pub async fn lxc_firewall_rules(
     returns: {
         type: Array,
         description: "List QEMU firewall rules.",
-        items: { type: pve_api_types::ListFirewallRules },
+        items: { type: pve_api_types::FirewallRule },
     },
     access: {
         permission: &Permission::Privilege(&["resource", "{remote}", "guest", "{vmid}"], PRIV_RESOURCE_AUDIT, false),
@@ -830,7 +830,7 @@ pub async fn qemu_firewall_rules(
     node: Option<String>,
     vmid: u32,
     _rpcenv: &mut dyn RpcEnvironment,
-) -> Result<Vec<pve_api_types::ListFirewallRules>, Error> {
+) -> Result<Vec<pve_api_types::FirewallRule>, Error> {
     let (remotes, _) = pdm_config::remotes::config()?;
 
     let pve = connect_to_remote(&remotes, &remote)?;
-- 
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] 9+ messages in thread

* [pdm-devel] [PATCH proxmox-datacenter-manager 2/2] api: firewall: add pve firewall security group GET endpoints
  2025-12-05 15:25 [pdm-devel] [RFC proxmox{, -datacenter-manager, -yew-comp} 0/8] make security groups expandable in firewall rules list Hannes Laimer
                   ` (4 preceding siblings ...)
  2025-12-05 15:25 ` [pdm-devel] [PATCH proxmox-datacenter-manager 1/2] pdm: rename ListFirewallRules to FirewallRule Hannes Laimer
@ 2025-12-05 15:25 ` Hannes Laimer
  2025-12-05 15:25 ` [pdm-devel] [PATCH proxmox-yew-comp 1/2] firewall: rules: rename ListFirewallRules to FirewallRule Hannes Laimer
  2025-12-05 15:25 ` [pdm-devel] [PATCH proxmox-yew-comp 2/2] firewall: rules: make security group entries expandable Hannes Laimer
  7 siblings, 0 replies; 9+ messages in thread
From: Hannes Laimer @ 2025-12-05 15:25 UTC (permalink / raw)
  To: pdm-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 server/src/api/pve/firewall.rs | 65 ++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/server/src/api/pve/firewall.rs b/server/src/api/pve/firewall.rs
index e60961c..7957264 100644
--- a/server/src/api/pve/firewall.rs
+++ b/server/src/api/pve/firewall.rs
@@ -47,6 +47,7 @@ const PVE_FW_SUBDIRS: SubdirMap = &sorted!([("status", &PVE_STATUS_ROUTER),]);
 // cluster
 #[sortable]
 const CLUSTER_FW_SUBDIRS: SubdirMap = &sorted!([
+    ("groups", &FIREWALL_SECURITY_GROUPS_ROUTER),
     ("options", &CLUSTER_OPTIONS_ROUTER),
     ("rules", &CLUSTER_RULES_ROUTER),
     ("status", &CLUSTER_STATUS_ROUTER),
@@ -72,6 +73,13 @@ const QEMU_FW_SUBDIRS: SubdirMap = &sorted!([
     ("rules", &QEMU_RULES_ROUTER),
 ]);
 
+// /groups
+const FIREWALL_SECURITY_GROUPS_ROUTER: Router = Router::new()
+    .get(&API_METHOD_FIREWALL_SECURITY_GROUPS)
+    .match_all("group", &FIREWALL_SECURITY_GROUP_ROUTER);
+const FIREWALL_SECURITY_GROUP_ROUTER: Router =
+    Router::new().get(&API_METHOD_FIREWALL_SECURITY_GROUP);
+
 // /options
 const CLUSTER_OPTIONS_ROUTER: Router = Router::new()
     .get(&API_METHOD_CLUSTER_FIREWALL_OPTIONS)
@@ -331,6 +339,63 @@ pub async fn pve_firewall_status(
     Ok(result)
 }
 
+#[api(
+    input: {
+        properties: {
+            remote: { schema: REMOTE_ID_SCHEMA },
+        },
+    },
+    returns: {
+        type: Array,
+        description: "List of firewall security groups.",
+        items: { type: pve_api_types::FirewallSecurityGroup },
+    },
+    access: {
+        permission: &Permission::Privilege(&["resource", "{remote}"], PRIV_RESOURCE_AUDIT, false),
+    },
+)]
+/// Get firewall security groups.
+pub async fn firewall_security_groups(
+    remote: String,
+    _rpcenv: &mut dyn RpcEnvironment,
+) -> Result<Vec<pve_api_types::FirewallSecurityGroup>, Error> {
+    let (remotes, _) = pdm_config::remotes::config()?;
+    let pve = connect_to_remote(&remotes, &remote)?;
+
+    Ok(pve.list_firewall_security_groups().await?)
+}
+
+#[api(
+    input: {
+        properties: {
+            remote: { schema: REMOTE_ID_SCHEMA },
+            group: {
+                type: String,
+                description: "The security groups name",
+            },
+        },
+    },
+    returns: {
+        type: Array,
+        description: "List firewall security group rules.",
+        items: { type: pve_api_types::FirewallRule },
+    },
+    access: {
+        permission: &Permission::Privilege(&["resource", "{remote}"], PRIV_RESOURCE_AUDIT, false),
+    },
+)]
+/// Get firewall security group rules.
+pub async fn firewall_security_group(
+    remote: String,
+    group: String,
+    _rpcenv: &mut dyn RpcEnvironment,
+) -> Result<Vec<pve_api_types::FirewallRule>, Error> {
+    let (remotes, _) = pdm_config::remotes::config()?;
+    let pve = connect_to_remote(&remotes, &remote)?;
+
+    Ok(pve.list_firewall_security_group_rules(&group).await?)
+}
+
 #[api(
     input: {
         properties: {
-- 
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] 9+ messages in thread

* [pdm-devel] [PATCH proxmox-yew-comp 1/2] firewall: rules: rename ListFirewallRules to FirewallRule
  2025-12-05 15:25 [pdm-devel] [RFC proxmox{, -datacenter-manager, -yew-comp} 0/8] make security groups expandable in firewall rules list Hannes Laimer
                   ` (5 preceding siblings ...)
  2025-12-05 15:25 ` [pdm-devel] [PATCH proxmox-datacenter-manager 2/2] api: firewall: add pve firewall security group GET endpoints Hannes Laimer
@ 2025-12-05 15:25 ` Hannes Laimer
  2025-12-05 15:25 ` [pdm-devel] [PATCH proxmox-yew-comp 2/2] firewall: rules: make security group entries expandable Hannes Laimer
  7 siblings, 0 replies; 9+ messages in thread
From: Hannes Laimer @ 2025-12-05 15:25 UTC (permalink / raw)
  To: pdm-devel

Adjust to rename in pve-api-types.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/firewall/rules.rs | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/src/firewall/rules.rs b/src/firewall/rules.rs
index 0958fc0..c40fab7 100644
--- a/src/firewall/rules.rs
+++ b/src/firewall/rules.rs
@@ -82,10 +82,10 @@ pub enum FirewallMsg {
 
 #[doc(hidden)]
 pub struct ProxmoxFirewallRules {
-    store: Store<pve_api_types::ListFirewallRules>,
-    loader: Loader<Vec<pve_api_types::ListFirewallRules>>,
-    _listener: SharedStateObserver<LoaderState<Vec<pve_api_types::ListFirewallRules>>>,
-    columns: Rc<Vec<DataTableHeader<pve_api_types::ListFirewallRules>>>,
+    store: Store<pve_api_types::FirewallRule>,
+    loader: Loader<Vec<pve_api_types::FirewallRule>>,
+    _listener: SharedStateObserver<LoaderState<Vec<pve_api_types::FirewallRule>>>,
+    columns: Rc<Vec<DataTableHeader<pve_api_types::FirewallRule>>>,
 }
 
 fn pill(text: impl Into<AttrValue>) -> Container {
@@ -99,7 +99,7 @@ fn pill(text: impl Into<AttrValue>) -> Container {
         .with_child(text.into())
 }
 
-fn format_firewall_rule(rule: &pve_api_types::ListFirewallRules) -> Html {
+fn format_firewall_rule(rule: &pve_api_types::FirewallRule) -> Html {
     let mut parts: Vec<VNode> = Vec::new();
 
     if let Some(iface) = &rule.iface {
@@ -155,21 +155,21 @@ impl ProxmoxFirewallRules {
         }
     }
 
-    fn build_columns() -> Rc<Vec<DataTableHeader<pve_api_types::ListFirewallRules>>> {
+    fn build_columns() -> Rc<Vec<DataTableHeader<pve_api_types::FirewallRule>>> {
         Rc::new(vec![
             DataTableColumn::new("")
                 .width("30px")
                 .justify("right")
                 .show_menu(false)
                 .resizable(false)
-                .render(|rule: &pve_api_types::ListFirewallRules| html! {&rule.pos})
+                .render(|rule: &pve_api_types::FirewallRule| html! {&rule.pos})
                 .into(),
             DataTableColumn::new(tr!("On"))
                 .width("40px")
                 .justify("center")
                 .resizable(false)
                 .render(
-                    |rule: &pve_api_types::ListFirewallRules| match rule.enable {
+                    |rule: &pve_api_types::FirewallRule| match rule.enable {
                         Some(1) => Fa::new("check").into(),
                         Some(0) | None => Fa::new("minus").into(),
                         _ => "-".into(),
@@ -178,19 +178,19 @@ impl ProxmoxFirewallRules {
                 .into(),
             DataTableColumn::new(tr!("Type"))
                 .width("80px")
-                .render(|rule: &pve_api_types::ListFirewallRules| html! {&rule.ty})
+                .render(|rule: &pve_api_types::FirewallRule| html! {&rule.ty})
                 .into(),
             DataTableColumn::new(tr!("Action"))
                 .width("100px")
-                .render(|rule: &pve_api_types::ListFirewallRules| html! {&rule.action})
+                .render(|rule: &pve_api_types::FirewallRule| html! {&rule.action})
                 .into(),
             DataTableColumn::new(tr!("Rule"))
                 .flex(1)
-                .render(|rule: &pve_api_types::ListFirewallRules| format_firewall_rule(rule))
+                .render(|rule: &pve_api_types::FirewallRule| format_firewall_rule(rule))
                 .into(),
             DataTableColumn::new(tr!("Comment"))
                 .width("150px")
-                .render(|rule: &pve_api_types::ListFirewallRules| {
+                .render(|rule: &pve_api_types::FirewallRule| {
                     rule.comment.as_deref().unwrap_or("-").into()
                 })
                 .into(),
@@ -207,7 +207,7 @@ impl Component for ProxmoxFirewallRules {
 
         let url: AttrValue = props.context.rules_url().into();
 
-        let store = Store::with_extract_key(|item: &pve_api_types::ListFirewallRules| {
+        let store = Store::with_extract_key(|item: &pve_api_types::FirewallRule| {
             Key::from(item.pos.to_string())
         });
 
-- 
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] 9+ messages in thread

* [pdm-devel] [PATCH proxmox-yew-comp 2/2] firewall: rules: make security group entries expandable
  2025-12-05 15:25 [pdm-devel] [RFC proxmox{, -datacenter-manager, -yew-comp} 0/8] make security groups expandable in firewall rules list Hannes Laimer
                   ` (6 preceding siblings ...)
  2025-12-05 15:25 ` [pdm-devel] [PATCH proxmox-yew-comp 1/2] firewall: rules: rename ListFirewallRules to FirewallRule Hannes Laimer
@ 2025-12-05 15:25 ` Hannes Laimer
  7 siblings, 0 replies; 9+ messages in thread
From: Hannes Laimer @ 2025-12-05 15:25 UTC (permalink / raw)
  To: pdm-devel

With this is is possible to see what rules a security group contains
within the list of normal firewall rules.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/firewall/context.rs |  12 +++
 src/firewall/rules.rs   | 202 ++++++++++++++++++++++++++++++++++++----
 2 files changed, 194 insertions(+), 20 deletions(-)

diff --git a/src/firewall/context.rs b/src/firewall/context.rs
index 6495fa3..79aa69b 100644
--- a/src/firewall/context.rs
+++ b/src/firewall/context.rs
@@ -82,6 +82,18 @@ impl FirewallContext {
         }
     }
 
+    pub fn group_rules_url(&self, group: &str) -> String {
+        match self {
+            Self::Cluster { remote } | Self::Node { remote, .. } | Self::Guest { remote, .. } => {
+                format!(
+                    "/pve/remotes/{}/firewall/groups/{}",
+                    percent_encode_component(remote),
+                    percent_encode_component(group)
+                )
+            }
+        }
+    }
+
     pub fn options_url(&self) -> String {
         match self {
             Self::Cluster { remote } => {
diff --git a/src/firewall/rules.rs b/src/firewall/rules.rs
index c40fab7..234fd3c 100644
--- a/src/firewall/rules.rs
+++ b/src/firewall/rules.rs
@@ -1,12 +1,17 @@
+use std::collections::HashSet;
+use std::ops::Deref;
 use std::rc::Rc;
 
 use yew::html::{IntoEventCallback, IntoPropValue};
 use yew::virtual_dom::{Key, VComp, VNode};
 
 use pwt::prelude::*;
-use pwt::state::{Loader, LoaderState, SharedStateObserver, Store};
-use pwt::widget::data_table::{DataTable, DataTableColumn, DataTableHeader};
-use pwt::widget::{Container, Fa};
+use pwt::props::ExtractPrimaryKey;
+use pwt::state::{Loader, LoaderState, SharedStateObserver, SlabTree, TreeStore};
+use pwt::widget::data_table::{
+    DataTable, DataTableCellRenderArgs, DataTableCellRenderer, DataTableColumn, DataTableHeader,
+};
+use pwt::widget::{Container, Fa, Row};
 use pwt_macros::builder;
 
 use super::context::FirewallContext;
@@ -76,16 +81,45 @@ impl FirewallRules {
     }
 }
 
+#[derive(Clone, Debug, PartialEq)]
+pub struct FirewallRuleEntry {
+    pub rule: pve_api_types::FirewallRule,
+    pub parent_group: Option<String>,
+}
+
+impl Deref for FirewallRuleEntry {
+    type Target = pve_api_types::FirewallRule;
+
+    fn deref(&self) -> &Self::Target {
+        &self.rule
+    }
+}
+
+impl ExtractPrimaryKey for FirewallRuleEntry {
+    fn extract_key(&self) -> Key {
+        match &self.parent_group {
+            Some(group) => Key::from(format!("group-{group}-{}", self.pos)),
+            None => Key::from(format!("top-{}", self.pos)),
+        }
+    }
+}
+
 pub enum FirewallMsg {
     DataChange,
+    ToggleGroup(Key, String),
+    GroupRulesLoaded(
+        String,
+        Result<Vec<pve_api_types::FirewallRule>, anyhow::Error>,
+    ),
 }
 
 #[doc(hidden)]
 pub struct ProxmoxFirewallRules {
-    store: Store<pve_api_types::FirewallRule>,
+    store: TreeStore<FirewallRuleEntry>,
     loader: Loader<Vec<pve_api_types::FirewallRule>>,
     _listener: SharedStateObserver<LoaderState<Vec<pve_api_types::FirewallRule>>>,
-    columns: Rc<Vec<DataTableHeader<pve_api_types::FirewallRule>>>,
+    columns: Rc<Vec<DataTableHeader<FirewallRuleEntry>>>,
+    loaded_groups: HashSet<String>,
 }
 
 fn pill(text: impl Into<AttrValue>) -> Container {
@@ -148,28 +182,108 @@ fn format_firewall_rule(rule: &pve_api_types::FirewallRule) -> Html {
         .collect::<Html>()
 }
 
+// Helper to create a dummy root entry since TreeStore needs a root
+impl Default for FirewallRuleEntry {
+    fn default() -> Self {
+        Self {
+            rule: pve_api_types::FirewallRule {
+                action: "".to_string(),
+                comment: None,
+                dest: None,
+                dport: None,
+                enable: None,
+                icmp_type: None,
+                iface: None,
+                ipversion: None,
+                log: None,
+                r#macro: None,
+                pos: 0,
+                proto: None,
+                source: None,
+                sport: None,
+                ty: "".to_string(),
+            },
+            parent_group: None,
+        }
+    }
+}
+
 impl ProxmoxFirewallRules {
     fn update_data(&mut self) {
         if let Some(Ok(data)) = &self.loader.read().data {
-            self.store.set_data((**data).clone());
+            let mut tree = SlabTree::new();
+            let mut root = tree.set_root(FirewallRuleEntry::default());
+
+            for rule in data.iter() {
+                let entry = FirewallRuleEntry {
+                    rule: rule.clone(),
+                    parent_group: None,
+                };
+                root.append(entry);
+            }
+
+            self.store.set_data(tree);
+            self.loaded_groups.clear();
         }
     }
 
-    fn build_columns() -> Rc<Vec<DataTableHeader<pve_api_types::FirewallRule>>> {
+    fn build_columns(
+        on_toggle: Callback<(Key, String)>,
+    ) -> Rc<Vec<DataTableHeader<FirewallRuleEntry>>> {
         Rc::new(vec![
             DataTableColumn::new("")
-                .width("30px")
+                .width("40px")
                 .justify("right")
                 .show_menu(false)
                 .resizable(false)
-                .render(|rule: &pve_api_types::FirewallRule| html! {&rule.pos})
+                .render_cell(DataTableCellRenderer::new(move |args: &mut DataTableCellRenderArgs<FirewallRuleEntry>| {
+                    let on_toggle = on_toggle.clone();
+                    let rule = &args.record().rule;
+                    let is_group = rule.ty == "group";
+                    let group_name = rule.action.clone();
+                    let key = args.key().clone();
+
+                    let expander = if is_group {
+                        let caret = if args.is_expanded() {
+                            "pwt-tree-expander fa fa-fw fa-caret-down"
+                        } else {
+                            "pwt-tree-expander fa fa-fw fa-caret-right"
+                        };
+
+                        let onclick = move |event: MouseEvent| {
+                            event.stop_propagation();
+                            on_toggle.emit((key.clone(), group_name.clone()));
+                        };
+
+                        html! { <i role="none" style="cursor: pointer;" class={caret} {onclick} /> }
+                    } else {
+                        html! {}
+                    };
+
+                    let content = if !is_group {
+                        html! { &rule.pos }
+                    } else {
+                        html! {}
+                    };
+
+                    let indent = Container::from_tag("span")
+                        .style("flex", "0 0 auto")
+                        .padding_start(4 * args.level());
+
+                    Row::new()
+                        .class(pwt::css::AlignItems::Baseline)
+                        .with_child(indent)
+                        .with_child(expander)
+                        .with_child(content)
+                        .into()
+                }))
                 .into(),
             DataTableColumn::new(tr!("On"))
                 .width("40px")
                 .justify("center")
                 .resizable(false)
                 .render(
-                    |rule: &pve_api_types::FirewallRule| match rule.enable {
+                    |rule: &FirewallRuleEntry| match rule.enable {
                         Some(1) => Fa::new("check").into(),
                         Some(0) | None => Fa::new("minus").into(),
                         _ => "-".into(),
@@ -178,19 +292,19 @@ impl ProxmoxFirewallRules {
                 .into(),
             DataTableColumn::new(tr!("Type"))
                 .width("80px")
-                .render(|rule: &pve_api_types::FirewallRule| html! {&rule.ty})
+                .render(|rule: &FirewallRuleEntry| html! {&rule.ty})
                 .into(),
             DataTableColumn::new(tr!("Action"))
                 .width("100px")
-                .render(|rule: &pve_api_types::FirewallRule| html! {&rule.action})
+                .render(|rule: &FirewallRuleEntry| html! {&rule.action})
                 .into(),
             DataTableColumn::new(tr!("Rule"))
                 .flex(1)
-                .render(|rule: &pve_api_types::FirewallRule| format_firewall_rule(rule))
+                .render(|rule: &FirewallRuleEntry| format_firewall_rule(&rule.rule))
                 .into(),
             DataTableColumn::new(tr!("Comment"))
                 .width("150px")
-                .render(|rule: &pve_api_types::FirewallRule| {
+                .render(|rule: &FirewallRuleEntry| {
                     rule.comment.as_deref().unwrap_or("-").into()
                 })
                 .into(),
@@ -207,9 +321,7 @@ impl Component for ProxmoxFirewallRules {
 
         let url: AttrValue = props.context.rules_url().into();
 
-        let store = Store::with_extract_key(|item: &pve_api_types::FirewallRule| {
-            Key::from(item.pos.to_string())
-        });
+        let store = TreeStore::new().view_root(false);
 
         let loader = Loader::new().loader({
             let url = url.clone();
@@ -224,12 +336,18 @@ impl Component for ProxmoxFirewallRules {
         loader.load();
 
         let mut me = Self {
-            store,
+            store: store.clone(),
             loader,
             _listener,
-            columns: Self::build_columns(),
+            columns: Rc::new(Vec::new()), // Initial empty columns, will be set below
+            loaded_groups: HashSet::new(),
         };
 
+        me.columns = Self::build_columns(
+            ctx.link()
+                .callback(|(key, group)| FirewallMsg::ToggleGroup(key, group)),
+        );
+
         me.update_data();
         me
     }
@@ -241,12 +359,56 @@ impl Component for ProxmoxFirewallRules {
         true
     }
 
-    fn update(&mut self, _ctx: &Context<Self>, msg: Self::Message) -> bool {
+    fn update(&mut self, ctx: &Context<Self>, msg: Self::Message) -> bool {
         match msg {
             FirewallMsg::DataChange => {
                 self.update_data();
                 true
             }
+            FirewallMsg::ToggleGroup(key, group_name) => {
+                if let Some(mut node) = self.store.write().lookup_node_mut(&key) {
+                    let expanded = node.expanded();
+                    node.set_expanded(!expanded);
+
+                    if !expanded && !self.loaded_groups.contains(&group_name) {
+                        let url = ctx.props().context.group_rules_url(&group_name);
+                        let group_name_clone = group_name.clone();
+                        ctx.link().send_future(async move {
+                            let result = crate::http_get(url, None).await;
+                            FirewallMsg::GroupRulesLoaded(group_name_clone, result)
+                        });
+                    }
+                }
+                false
+            }
+            FirewallMsg::GroupRulesLoaded(group_name, result) => {
+                if let Ok(rules) = result {
+                    self.loaded_groups.insert(group_name.clone());
+
+                    let mut parent_key = None;
+                    {
+                        for (_, item) in self.store.filtered_data() {
+                            if item.record().ty == "group" && item.record().action == group_name {
+                                parent_key = Some(self.store.extract_key(&item.record()));
+                                break;
+                            }
+                        }
+                    }
+
+                    if let Some(key) = parent_key {
+                        if let Some(mut parent_node) = self.store.write().lookup_node_mut(&key) {
+                            for rule in rules {
+                                let entry = FirewallRuleEntry {
+                                    rule,
+                                    parent_group: Some(group_name.clone()),
+                                };
+                                parent_node.append(entry);
+                            }
+                        }
+                    }
+                }
+                true
+            }
         }
     }
 
-- 
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] 9+ messages in thread

end of thread, other threads:[~2025-12-05 15:26 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-12-05 15:25 [pdm-devel] [RFC proxmox{, -datacenter-manager, -yew-comp} 0/8] make security groups expandable in firewall rules list Hannes Laimer
2025-12-05 15:25 ` [pdm-devel] [PATCH proxmox 1/4] pve-api-types: rename ListFirewallRules to FirewallRule Hannes Laimer
2025-12-05 15:25 ` [pdm-devel] [PATCH proxmox 2/4] pve-api-types: update pve-api.json Hannes Laimer
2025-12-05 15:25 ` [pdm-devel] [PATCH proxmox 3/4] pve-api-types: add security group GET endpoints Hannes Laimer
2025-12-05 15:25 ` [pdm-devel] [PATCH proxmox 4/4] pve-api-types: regenerate Hannes Laimer
2025-12-05 15:25 ` [pdm-devel] [PATCH proxmox-datacenter-manager 1/2] pdm: rename ListFirewallRules to FirewallRule Hannes Laimer
2025-12-05 15:25 ` [pdm-devel] [PATCH proxmox-datacenter-manager 2/2] api: firewall: add pve firewall security group GET endpoints Hannes Laimer
2025-12-05 15:25 ` [pdm-devel] [PATCH proxmox-yew-comp 1/2] firewall: rules: rename ListFirewallRules to FirewallRule Hannes Laimer
2025-12-05 15:25 ` [pdm-devel] [PATCH proxmox-yew-comp 2/2] firewall: rules: make security group entries expandable Hannes Laimer

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal