From 9054da7d25ee9fffae4e69d36dbfb14d451b6ce5 Mon Sep 17 00:00:00 2001
From: Tomas Hozza <thozza@redhat.com>
Date: Thu, 3 Mar 2022 21:24:12 +0100
Subject: [PATCH] stages/firewall: fix fail when setting only the default zone

When the firewall stage is provided with stage options, which set only
the default firewall zone, the `firewall-offline-cmd` command is
executed unconditionally without any parameters. This is because in this
case `ports`, `enabled_services` and `disabled_services` are all an
empty lists. This results in a failure with the following error message:

`Opening of '/etc/sysconfig/system-config-firewall' failed, exiting.`

Make sure that the second invocation of `firewall-offline-cmd` happens
conditionally, only when at least one of the `ports`, `enabled_services`
or `disabled_services` is a non-empty list.

Adjust the stage test to cover this scenario.

Signed-off-by: Tomas Hozza <thozza@redhat.com>
---
 stages/org.osbuild.firewall          | 15 ++++++++-------
 test/data/stages/firewall/b.json     |  9 +++++++--
 test/data/stages/firewall/b.mpp.json |  9 +++++++--
 3 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/stages/org.osbuild.firewall b/stages/org.osbuild.firewall
index f2f53056..544373ce 100755
--- a/stages/org.osbuild.firewall
+++ b/stages/org.osbuild.firewall
@@ -84,13 +84,14 @@ def main(tree, options):
 
     # The options below are "lokkit" compatibility options and can not be used
     # with other options.
-    subprocess.run(["chroot",
-                    tree,
-                    "firewall-offline-cmd"] +
-                   list(map(lambda x: f"--port={x}", ports)) +
-                   list(map(lambda x: f"--service={x}", enabled_services)) +
-                   list(map(lambda x: f"--remove-service={x}", disabled_services)),
-                   check=True)
+    if ports or enabled_services or disabled_services:
+        subprocess.run(["chroot",
+                        tree,
+                        "firewall-offline-cmd"] +
+                      list(map(lambda x: f"--port={x}", ports)) +
+                      list(map(lambda x: f"--service={x}", enabled_services)) +
+                      list(map(lambda x: f"--remove-service={x}", disabled_services)),
+                      check=True)
 
     return 0
 
diff --git a/test/data/stages/firewall/b.json b/test/data/stages/firewall/b.json
index a95161ab..61f381c6 100644
--- a/test/data/stages/firewall/b.json
+++ b/test/data/stages/firewall/b.json
@@ -475,6 +475,12 @@
           ]
         }
       },
+      {
+        "name": "org.osbuild.firewall",
+        "options": {
+          "default_zone": "trusted"
+        }
+      },
       {
         "name": "org.osbuild.firewall",
         "options": {
@@ -488,8 +494,7 @@
           ],
           "disabled_services": [
             "telnet"
-          ],
-          "default_zone": "trusted"
+          ]
         }
       }
     ]
diff --git a/test/data/stages/firewall/b.mpp.json b/test/data/stages/firewall/b.mpp.json
index 5d2b83fb..9a75c2a4 100644
--- a/test/data/stages/firewall/b.mpp.json
+++ b/test/data/stages/firewall/b.mpp.json
@@ -29,6 +29,12 @@
           }
         }
       },
+      {
+        "name": "org.osbuild.firewall",
+        "options": {
+          "default_zone": "trusted"
+        }
+      },
       {
         "name": "org.osbuild.firewall",
         "options": {
@@ -42,8 +48,7 @@
           ],
           "disabled_services": [
             "telnet"
-          ],
-          "default_zone": "trusted"
+          ]
         }
       }
     ]