[pdm-devel] [RFC proxmox{, -datacenter-manager, -yew-comp} 0/8] make security groups expandable in firewall rules list

[pdm-devel] [RFC proxmox{, -datacenter-manager, -yew-comp} 0/8] make security groups expandable in firewall rules list

[Hannes Laimer]

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

[pdm-devel] [PATCH proxmox 1/4] pve-api-types: rename ListFirewallRules to FirewallRule

[Hannes Laimer]

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

[pdm-devel] [PATCH proxmox 2/4] pve-api-types: update pve-api.json

[Hannes Laimer]

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

[pdm-devel] [PATCH proxmox 3/4] pve-api-types: add security group GET endpoints

[Hannes Laimer]

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

[pdm-devel] [PATCH proxmox 4/4] pve-api-types: regenerate

[Hannes Laimer]

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

[pdm-devel] [PATCH proxmox-datacenter-manager 1/2] pdm: rename ListFirewallRules to FirewallRule

[Hannes Laimer]

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

[pdm-devel] [PATCH proxmox-datacenter-manager 2/2] api: firewall: add pve firewall security group GET endpoints

[Hannes Laimer]

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

[pdm-devel] [PATCH proxmox-yew-comp 1/2] firewall: rules: rename ListFirewallRules to FirewallRule

[Hannes Laimer]

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

[pdm-devel] [PATCH proxmox-yew-comp 2/2] firewall: rules: make security group entries expandable

[Hannes Laimer]

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

Re: [pdm-devel] [RFC proxmox{, -datacenter-manager, -yew-comp} 0/8] make security groups expandable in firewall rules list

[Lukas Wagner]

On Fri Dec 5, 2025 at 4:25 PM CET, Hannes Laimer wrote:
> 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.

Yeah, I think approach could work. But I'd also be interested what other
think.

If we ever change the view so that rules are editable, I guess we could
keep it as is, but should probably keep the rules from the group
read-only in their expanded form. To edit the actual rules of the group,
some other form of UI would be good, maybe like you suggested, in a
separate tab on the 'Datacenter'/'Remote' level.
If we go this route, the 'expanded' rules could maybe use some form of
visual distinction from the rest, to avoid confusion about their
read-only status.

Regarding the UI, I'd maybe put the caret into a separate column; the
discontinued numbering seems a bit odd to me. Alternatively, the caret
could stay in the same column, but the group keeps its numbering, e.g.
like this

  1     .....
  2     .....
> 3     .....
  4     .....

expanded it could maybe look like

 
  1     .....
  2     .....
v 3     .....
   3.1  .....
   3.2  .....
  4     .....

But that's just some idea; maybe somebody has some other input here.

>
> 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
>


Tested-by: Lukas Wagner <l.wagner@proxmox.com>

UI code was only skimmed, but I couldn't really find anything to
complain about:

Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>


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

Re: [pdm-devel] [RFC proxmox{, -datacenter-manager, -yew-comp} 0/8] make security groups expandable in firewall rules list

[Hannes Laimer]

Thanks for the feedback! some comments inline

On 12/9/25 16:23, Lukas Wagner wrote:
> On Fri Dec 5, 2025 at 4:25 PM CET, Hannes Laimer wrote:
>> 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.
> 
> Yeah, I think approach could work. But I'd also be interested what other
> think.
> 
> If we ever change the view so that rules are editable, I guess we could
> keep it as is, but should probably keep the rules from the group
> read-only in their expanded form. To edit the actual rules of the group,
> some other form of UI would be good, maybe like you suggested, in a
> separate tab on the 'Datacenter'/'Remote' level.

Since rules in groups are just rules so technically editing isn't a
problem, but yes, might be a little confusing cause it would edit the
rule for all instances where the group is used...

maybe some "reduced editing" could also make sense, maybe just order and
on/off, but that's a not relevant for this

> If we go this route, the 'expanded' rules could maybe use some form of
> visual distinction from the rest, to avoid confusion about their
> read-only status.
> 

yes, will add that for v2.



> Regarding the UI, I'd maybe put the caret into a separate column; the
> discontinued numbering seems a bit odd to me. Alternatively, the caret
> could stay in the same column, but the group keeps its numbering, e.g.
> like this
> 
>    1     .....
>    2     .....
>> 3     .....
>    4     .....
> 
> expanded it could maybe look like
> 
>   
>    1     .....
>    2     .....
> v 3     .....
>     3.1  .....
>     3.2  .....
>    4     .....
> 

I like this!

> But that's just some idea; maybe somebody has some other input here.
> 
>>
>> 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
>>
> 
> 
> Tested-by: Lukas Wagner <l.wagner@proxmox.com>
> 
> UI code was only skimmed, but I couldn't really find anything to
> complain about:
> 
> Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>



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

Re: [pdm-devel] [RFC proxmox{, -datacenter-manager, -yew-comp} 0/8] make security groups expandable in firewall rules list

[Lukas Wagner]

On Tue Dec 9, 2025 at 5:45 PM CET, Hannes Laimer wrote:
> Thanks for the feedback! some comments inline
>
> On 12/9/25 16:23, Lukas Wagner wrote:
>> On Fri Dec 5, 2025 at 4:25 PM CET, Hannes Laimer wrote:
>>> 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.
>> 
>> Yeah, I think approach could work. But I'd also be interested what other
>> think.
>> 
>> If we ever change the view so that rules are editable, I guess we could
>> keep it as is, but should probably keep the rules from the group
>> read-only in their expanded form. To edit the actual rules of the group,
>> some other form of UI would be good, maybe like you suggested, in a
>> separate tab on the 'Datacenter'/'Remote' level.
>
> Since rules in groups are just rules so technically editing isn't a
> problem, but yes, might be a little confusing cause it would edit the
> rule for all instances where the group is used...
>
> maybe some "reduced editing" could also make sense, maybe just order and
> on/off, but that's a not relevant for this
>

Yeah, I think the main issue with allowing edits would be indeed the
confusion that this would affect other guests/hosts as well, that could
be quite a surprise.


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