[pve-devel] [PATCH proxmox-firewall] firewall: properly cleanup tables when firewall is inactive

[pve-devel] [PATCH proxmox-firewall] firewall: properly cleanup tables when firewall is inactive

[Stefan Hanreich]

When executing multiple nft commands they are transactional, either
all get applied or none. When only the host or guest firewall is
active, only one table exists and this causes the delete commands to
fail. To fix this we need to send the delete commands separately.

It might make sense to support running multiple separate batches in
the NftClient in the future in order to avoid having to call nft
twice.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 proxmox-firewall/src/bin/proxmox-firewall.rs |  9 +++++----
 proxmox-firewall/src/firewall.rs             | 10 +++++-----
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/proxmox-firewall/src/bin/proxmox-firewall.rs b/proxmox-firewall/src/bin/proxmox-firewall.rs
index 2f4875f..4e07993 100644
--- a/proxmox-firewall/src/bin/proxmox-firewall.rs
+++ b/proxmox-firewall/src/bin/proxmox-firewall.rs
@@ -12,11 +12,12 @@ const RULE_BASE: &str = include_str!("../../resources/proxmox-firewall.nft");
 
 fn remove_firewall() -> Result<(), std::io::Error> {
     log::info!("removing existing firewall rules");
-    let commands = Firewall::remove_commands();
 
-    // can ignore other errors, since it fails when tables do not exist
-    if let Err(NftError::Io(err)) = NftClient::run_json_commands(&commands) {
-        return Err(err);
+    for command in Firewall::remove_commands() {
+        // can ignore other errors, since it fails when tables do not exist
+        if let Err(NftError::Io(err)) = NftClient::run_json_commands(&command) {
+            return Err(err);
+        }
     }
 
     Ok(())
diff --git a/proxmox-firewall/src/firewall.rs b/proxmox-firewall/src/firewall.rs
index 2195a07..b137f58 100644
--- a/proxmox-firewall/src/firewall.rs
+++ b/proxmox-firewall/src/firewall.rs
@@ -157,11 +157,11 @@ impl Firewall {
         }
     }
 
-    pub fn remove_commands() -> Commands {
-        Commands::new(vec![
-            Delete::table(Self::cluster_table()),
-            Delete::table(Self::guest_table()),
-        ])
+    pub fn remove_commands() -> Vec<Commands> {
+        vec![
+            Commands::new(vec![Delete::table(Self::cluster_table())]),
+            Commands::new(vec![Delete::table(Self::guest_table())]),
+        ]
     }
 
     fn create_management_ipset(&self, commands: &mut Commands) -> Result<(), Error> {
-- 
2.39.2


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

[pve-devel] applied: [PATCH proxmox-firewall] firewall: properly cleanup tables when firewall is inactive

[Thomas Lamprecht]

Am 23/04/2024 um 11:21 schrieb Stefan Hanreich:
> When executing multiple nft commands they are transactional, either
> all get applied or none. When only the host or guest firewall is
> active, only one table exists and this causes the delete commands to
> fail. To fix this we need to send the delete commands separately.
> 
> It might make sense to support running multiple separate batches in
> the NftClient in the future in order to avoid having to call nft
> twice.
> 
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
>  proxmox-firewall/src/bin/proxmox-firewall.rs |  9 +++++----
>  proxmox-firewall/src/firewall.rs             | 10 +++++-----
>  2 files changed, 10 insertions(+), 9 deletions(-)
> 
>

applied, thanks!


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