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

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

[Hannes Laimer]

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.

v2, thanks @Lukas:
 - show positions of rules in groups as `3.1`
 - highlight rules in groups and add small visual hint so it is clear
   these rules are part of the group
 - add placeholder for groups with no rules
 - split the rename into a small, separate series[1]
 - generally reworked code a little

(this does depend on [1], it uses the re-named name)

[1] https://lore.proxmox.com/pdm-devel/20251217150831.199100-1-h.laimer@proxmox.com/T/#t 

proxmox:

Hannes Laimer (2):
  pve-api-types: add security group GET endpoints
  pve-api-types: regenerate

 pve-api-types/generate.pl            |   5 +
 pve-api-types/src/generated/code.rs  |  77 ++++++-
 pve-api-types/src/generated/types.rs | 294 +++++++++++++++------------
 3 files changed, 234 insertions(+), 142 deletions(-)


proxmox-datacenter-manager:

Hannes Laimer (1):
  api: firewall: add pve firewall security group GET endpoints

 server/src/api/pve/firewall.rs | 65 ++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)


proxmox-yew-comp:

Hannes Laimer (1):
  firewall: rules: make security group entries expandable

 src/firewall/context.rs |  12 ++
 src/firewall/rules.rs   | 384 +++++++++++++++++++++++++++++++++++++---
 2 files changed, 369 insertions(+), 27 deletions(-)


Summary over all repositories:
  6 files changed, 668 insertions(+), 169 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 v2 1/2] 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 v2 2/2] 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 v2 1/1] 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 v2 1/1] 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   | 384 +++++++++++++++++++++++++++++++++++++---
 2 files changed, 369 insertions(+), 27 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..9ee7f56 100644
--- a/src/firewall/rules.rs
+++ b/src/firewall/rules.rs
@@ -1,12 +1,17 @@
+use std::collections::HashMap;
+use std::collections::HashSet;
 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,111 @@ impl FirewallRules {
     }
 }
 
+#[derive(Clone, Debug, PartialEq)]
+pub enum FirewallRuleEntry {
+    Root,
+    Rule {
+        rule: pve_api_types::FirewallRule,
+        parent_pos: Option<i64>,
+    },
+    Placeholder {
+        kind: PlaceholderKind,
+        parent_pos: i64,
+    },
+}
+
+#[derive(Copy, Clone, Debug, PartialEq, Eq)]
+pub enum PlaceholderKind {
+    Loading,
+    Empty,
+    Error,
+}
+
+impl FirewallRuleEntry {
+    fn rule(&self) -> Option<&pve_api_types::FirewallRule> {
+        match self {
+            FirewallRuleEntry::Rule { rule, .. } => Some(rule),
+            FirewallRuleEntry::Root | FirewallRuleEntry::Placeholder { .. } => None,
+        }
+    }
+
+    fn placeholder_kind(&self) -> Option<PlaceholderKind> {
+        match self {
+            FirewallRuleEntry::Placeholder { kind, .. } => Some(*kind),
+            FirewallRuleEntry::Root | FirewallRuleEntry::Rule { .. } => None,
+        }
+    }
+
+    fn is_child_row(&self) -> bool {
+        matches!(self, FirewallRuleEntry::Placeholder { .. })
+            || matches!(
+                self,
+                FirewallRuleEntry::Rule {
+                    parent_pos: Some(_),
+                    ..
+                }
+            )
+    }
+
+    fn is_group_row(&self) -> bool {
+        matches!(
+            self,
+            FirewallRuleEntry::Rule { rule, parent_pos: None } if rule.ty == "group"
+        )
+    }
+}
+
+impl ExtractPrimaryKey for FirewallRuleEntry {
+    fn extract_key(&self) -> Key {
+        match self {
+            FirewallRuleEntry::Root => Key::from("root"),
+            FirewallRuleEntry::Placeholder { kind, parent_pos } => {
+                let kind = match kind {
+                    PlaceholderKind::Loading => "loading",
+                    PlaceholderKind::Empty => "empty",
+                    PlaceholderKind::Error => "error",
+                };
+                Key::from(format!("group-{parent_pos}-__{kind}__"))
+            }
+            FirewallRuleEntry::Rule {
+                rule, parent_pos, ..
+            } => {
+                if let Some(parent_pos) = parent_pos {
+                    Key::from(format!("group-{parent_pos}-{}", rule.pos))
+                } else {
+                    Key::from(format!("top-{}", rule.pos))
+                }
+            }
+        }
+    }
+}
+
 pub enum FirewallMsg {
     DataChange,
+    ToggleGroup(String, i64),
+    GroupRulesLoaded(
+        String,
+        i64,
+        Result<Vec<pve_api_types::FirewallRule>, anyhow::Error>,
+    ),
+}
+
+#[derive(Clone, Debug)]
+enum GroupLoadState {
+    Loading,
+    Loaded(Vec<pve_api_types::FirewallRule>),
+    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>>>,
+    expanded_groups: HashSet<i64>,
+    group_state: HashMap<String, GroupLoadState>,
+    base_rules: Vec<pve_api_types::FirewallRule>,
 }
 
 fn pill(text: impl Into<AttrValue>) -> Container {
@@ -148,50 +248,201 @@ fn format_firewall_rule(rule: &pve_api_types::FirewallRule) -> Html {
         .collect::<Html>()
 }
 
+impl Default for FirewallRuleEntry {
+    fn default() -> Self {
+        Self::Root
+    }
+}
+
 impl ProxmoxFirewallRules {
-    fn update_data(&mut self) {
-        if let Some(Ok(data)) = &self.loader.read().data {
-            self.store.set_data((**data).clone());
+    fn rebuild_tree(&mut self) {
+        let mut tree = SlabTree::new();
+        let mut root = tree.set_root(FirewallRuleEntry::default());
+
+        for rule in self.base_rules.iter() {
+            let is_group = rule.ty == "group";
+            let parent_pos = rule.pos;
+            let group_name = rule.action.as_str();
+
+            let entry = FirewallRuleEntry::Rule {
+                rule: rule.clone(),
+                parent_pos: None,
+            };
+
+            let mut node = root.append(entry);
+
+            if is_group && self.expanded_groups.contains(&parent_pos) {
+                node.set_expanded(true);
+
+                match self.group_state.get(group_name) {
+                    Some(GroupLoadState::Loaded(rules)) => {
+                        if rules.is_empty() {
+                            node.append(Self::placeholder_entry(
+                                PlaceholderKind::Empty,
+                                parent_pos,
+                            ));
+                        } else {
+                            for rule in rules.iter().cloned() {
+                                node.append(FirewallRuleEntry::Rule {
+                                    rule,
+                                    parent_pos: Some(parent_pos),
+                                });
+                            }
+                        }
+                    }
+                    Some(GroupLoadState::Error) => {
+                        node.append(Self::placeholder_entry(PlaceholderKind::Error, parent_pos));
+                    }
+                    Some(GroupLoadState::Loading) | None => {
+                        node.append(Self::placeholder_entry(
+                            PlaceholderKind::Loading,
+                            parent_pos,
+                        ));
+                    }
+                }
+            }
         }
+
+        self.store.set_data(tree);
+    }
+
+    fn update_data(&mut self) {
+        let data = match &self.loader.read().data {
+            Some(Ok(data)) => (**data).clone(),
+            _ => return,
+        };
+
+        self.base_rules = data;
+
+        self.expanded_groups.clear();
+        self.group_state.clear();
+
+        self.rebuild_tree();
+    }
+
+    fn placeholder_entry(kind: PlaceholderKind, parent_pos: i64) -> FirewallRuleEntry {
+        FirewallRuleEntry::Placeholder { kind, parent_pos }
     }
 
-    fn build_columns() -> Rc<Vec<DataTableHeader<pve_api_types::FirewallRule>>> {
+    fn build_columns(
+        on_toggle: Callback<(String, i64)>,
+    ) -> Rc<Vec<DataTableHeader<FirewallRuleEntry>>> {
         Rc::new(vec![
             DataTableColumn::new("")
-                .width("30px")
-                .justify("right")
+                .width("28px")
+                .justify("start")
                 .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 record = args.record();
+                    let (is_group, group_name, group_pos) = match record {
+                        FirewallRuleEntry::Rule { rule, .. } if rule.ty == "group" => {
+                            (true, rule.action.clone(), rule.pos)
+                        }
+                        _ => (false, String::new(), 0),
+                    };
+
+                    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((group_name.clone(), group_pos));
+                        };
+
+                        html! { <i role="none" style="cursor: pointer;" class={caret} {onclick} /> }
+                    } else {
+                        html! { }
+                    };
+
+                    Row::new()
+                        .class(pwt::css::AlignItems::Baseline)
+                        .with_child(expander)
+                        .into()
+                }))
+                .into(),
+            DataTableColumn::new("")
+                .width("40px")
+                .justify("start")
+                .show_menu(false)
+                .resizable(false)
+                .render(|entry: &FirewallRuleEntry| {
+                    match entry {
+                        FirewallRuleEntry::Rule { rule, parent_pos: Some(parent_pos), .. } => {
+                            html! {
+                                <span style="font-size: 0.9em; opacity: 0.9;">
+                                    { format!("{parent_pos}.{}", rule.pos) }
+                                </span>
+                            }
+                        }
+                        FirewallRuleEntry::Rule { rule, .. } => html! { {rule.pos} },
+                        FirewallRuleEntry::Root | FirewallRuleEntry::Placeholder { .. } => "".into(),
+                    }
+                })
                 .into(),
             DataTableColumn::new(tr!("On"))
                 .width("40px")
                 .justify("center")
                 .resizable(false)
-                .render(
-                    |rule: &pve_api_types::FirewallRule| match rule.enable {
+                .render(|entry: &FirewallRuleEntry| {
+                    let Some(rule) = entry.rule() else {
+                        return "".into();
+                    };
+
+                    match rule.enable {
                         Some(1) => Fa::new("check").into(),
                         Some(0) | None => Fa::new("minus").into(),
                         _ => "-".into(),
-                    },
-                )
+                    }
+                })
                 .into(),
             DataTableColumn::new(tr!("Type"))
                 .width("80px")
-                .render(|rule: &pve_api_types::FirewallRule| html! {&rule.ty})
+                .render(|entry: &FirewallRuleEntry| match entry.rule() {
+                    Some(rule) => html! {&rule.ty},
+                    None => html! {""},
+                })
                 .into(),
             DataTableColumn::new(tr!("Action"))
                 .width("100px")
-                .render(|rule: &pve_api_types::FirewallRule| html! {&rule.action})
+                .render(|entry: &FirewallRuleEntry| match entry {
+                    FirewallRuleEntry::Rule { rule, .. } => html! {&rule.action},
+                    FirewallRuleEntry::Root | FirewallRuleEntry::Placeholder { .. } => "".into(),
+                })
                 .into(),
             DataTableColumn::new(tr!("Rule"))
                 .flex(1)
-                .render(|rule: &pve_api_types::FirewallRule| format_firewall_rule(rule))
+                .render(|entry: &FirewallRuleEntry| {
+                    if let Some(kind) = entry.placeholder_kind() {
+                        let text = match kind {
+                            PlaceholderKind::Loading => tr!("Loading…"),
+                            PlaceholderKind::Empty => tr!("No rules in this group"),
+                            PlaceholderKind::Error => {
+                                format!(
+                                    "{} ({})",
+                                    tr!("Failed to load group rules"),
+                                    tr!("collapse/expand to retry")
+                                )
+                            }
+                        };
+                        return html! { <span>{text}</span> };
+                    }
+                    match entry.rule() {
+                        Some(rule) => format_firewall_rule(rule),
+                        None => "".into(),
+                    }
+                })
                 .into(),
             DataTableColumn::new(tr!("Comment"))
                 .width("150px")
-                .render(|rule: &pve_api_types::FirewallRule| {
-                    rule.comment.as_deref().unwrap_or("-").into()
+                .render(|entry: &FirewallRuleEntry| match entry.rule() {
+                    Some(rule) => rule.comment.as_deref().unwrap_or("-").into(),
+                    None => "".into(),
                 })
                 .into(),
         ])
@@ -207,9 +458,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 +473,20 @@ 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
+            expanded_groups: HashSet::new(),
+            group_state: HashMap::new(),
+            base_rules: Vec::new(),
         };
 
+        me.columns = Self::build_columns(
+            ctx.link()
+                .callback(|(group, pos)| FirewallMsg::ToggleGroup(group, pos)),
+        );
+
         me.update_data();
         me
     }
@@ -241,12 +498,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(group_name, group_pos) => {
+                if self.expanded_groups.contains(&group_pos) {
+                    self.expanded_groups.remove(&group_pos);
+                    self.rebuild_tree();
+                    return true;
+                }
+
+                self.expanded_groups.insert(group_pos);
+
+                let should_load = match self.group_state.get(&group_name) {
+                    Some(GroupLoadState::Loaded(_)) | Some(GroupLoadState::Loading) => false,
+                    Some(GroupLoadState::Error) | None => true,
+                };
+
+                if should_load {
+                    self.group_state
+                        .insert(group_name.clone(), GroupLoadState::Loading);
+                    let url = ctx.props().context.group_rules_url(&group_name);
+                    ctx.link().send_future(async move {
+                        let result = crate::http_get(url, None).await;
+                        FirewallMsg::GroupRulesLoaded(group_name, group_pos, result)
+                    });
+                }
+
+                self.rebuild_tree();
+                true
+            }
+            FirewallMsg::GroupRulesLoaded(group_name, group_pos, result) => {
+                match result {
+                    Ok(mut rules) => {
+                        rules.sort_by_key(|r| r.pos);
+                        self.group_state
+                            .insert(group_name, GroupLoadState::Loaded(rules));
+                    }
+                    Err(err) => {
+                        log::warn!(
+                            "failed to load firewall security group rules for {group_pos}: {err:#}"
+                        );
+                        self.group_state.insert(group_name, GroupLoadState::Error);
+                    }
+                }
+                self.rebuild_tree();
+                true
+            }
         }
     }
 
@@ -261,6 +562,35 @@ impl Component for ProxmoxFirewallRules {
                 DataTable::new(self.columns.clone(), self.store.clone())
                     .show_header(true)
                     .striped(true)
+                    .row_render_callback({
+                        let store = self.store.clone();
+                        move |args: &mut pwt::widget::data_table::DataTableRowRenderArgs<
+                            FirewallRuleEntry,
+                        >| {
+                            let mut style = String::new();
+
+                            if args.record().is_child_row() {
+                                style.push_str(
+                                    "background-color: var(--pwt-color-neutral-container);",
+                                );
+                            }
+
+                            if args.record().is_group_row() {
+                                style.push_str("font-weight: 600;");
+                                if let Some(node) = store.read().lookup_node(args.key()) {
+                                    if node.expanded() {
+                                        style.push_str(
+                                            "box-shadow: inset 0 -2px 0 var(--pwt-color-border);",
+                                        );
+                                    }
+                                }
+                            }
+
+                            if !style.is_empty() {
+                                args.set_attribute("style", Some(style));
+                            }
+                        }
+                    })
                     .into()
             }
         })
-- 
2.47.3



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