[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

[pdm-devel] [PATCH proxmox{, -datacenter-manager} v2 0/5] Add support for network resource type.

[Stefan Hanreich]

The patch series [1] introduced a new resource type for network entities. This
patch series implements support for handling this new resource type in the
backend, as well as the frontend. The backend can now handle both formats and
maps them to the same Resource internally.

Without this patch series applied, PDM cannot handle any Proxmox VE remotes,
that have the patch series in [1] applied.

A new match category is also introduced, in order to be able to filter for
specific network types in the UI / API.

For more information about the new resource type see the respective patch
series [1].

Changes from v1 to v2:
* kebab-cased zone-type
* dropped applied patches
* rebased on top of current master

[1] https://lore.proxmox.com/pve-devel/20251107143201.689035-1-s.hanreich@proxmox.com/T/

proxmox:

Stefan Hanreich (2):
  pve-api-types: update /cluster/resources endpoint
  pve-api-types: regenerate

 pve-api-types/pve-api.json           | 2 +-
 pve-api-types/src/generated/types.rs | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)


proxmox-datacenter-manager:

Stefan Hanreich (3):
  resources: views: support new pve network resource type
  resources: api: add network type match category
  ui: use new network resource type

 cli/client/src/resources.rs                  |   9 +-
 lib/pdm-api-types/src/resource.rs            | 115 +++++++++++-----
 lib/pdm-client/src/lib.rs                    |   2 +
 server/src/api/resources.rs                  |  98 ++++++++++---
 server/src/metric_collection/top_entities.rs |   2 +-
 server/src/views/mod.rs                      |   2 +-
 ui/src/dashboard/sdn_zone_panel.rs           |   7 +-
 ui/src/lib.rs                                |   6 +-
 ui/src/pve/remote.rs                         |   2 +-
 ui/src/pve/tree.rs                           |   2 +-
 ui/src/pve/utils.rs                          |  14 +-
 ui/src/renderer.rs                           |   7 +-
 ui/src/sdn/zone_tree.rs                      | 136 +++++++++++--------
 13 files changed, 275 insertions(+), 127 deletions(-)


Summary over all repositories:
  15 files changed, 278 insertions(+), 129 deletions(-)

-- 
Generated by git-murpp 0.8.0

_______________________________________________
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

[Stefan Hanreich]

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 pve-api-types/src/generated/types.rs | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/pve-api-types/src/generated/types.rs b/pve-api-types/src/generated/types.rs
index e19b88ae..fb16fff2 100644
--- a/pve-api-types/src/generated/types.rs
+++ b/pve-api-types/src/generated/types.rs
@@ -818,7 +818,7 @@ fn test_regex_compilation_4() {
             optional: true,
             type: Integer,
         },
-        zone_type: {
+        "zone-type": {
             optional: true,
             type: String,
         },
@@ -983,6 +983,7 @@ pub struct ClusterResource {
 
     /// The type of an SDN zone (for type 'sdn').
     #[serde(default, skip_serializing_if = "Option::is_none")]
+    #[serde(rename = "zone-type")]
     pub zone_type: Option<String>,
 }
 
-- 
2.47.3


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