From 8e3d0540997770067238d19e7dc8216c209fb4da Mon Sep 17 00:00:00 2001
From: Achilleas Koutsou <achilleas@koutsou.net>
Date: Fri, 22 Nov 2024 17:49:00 +0100
Subject: [PATCH] stages: use util.chroot in all stages that call "chroot"

Use the chroot utility module for all cases where we need to chroot
during a stage's execution.

The advantage is that all stages use the same tested code path for
setting up a chroot and all chrooted commands run in the same
environment, with the /proc, /dev, and /sys filesystems mounted.
---
 stages/org.osbuild.authconfig | 10 +++-------
 stages/org.osbuild.firewall   | 25 +++++++++++--------------
 2 files changed, 14 insertions(+), 21 deletions(-)

diff --git a/stages/org.osbuild.authconfig b/stages/org.osbuild.authconfig
index f26fae40..2facec86 100755
--- a/stages/org.osbuild.authconfig
+++ b/stages/org.osbuild.authconfig
@@ -1,18 +1,14 @@
 #!/usr/bin/python3
 import shutil
-import subprocess
 import sys
 
 import osbuild.api
+from osbuild.util.chroot import Chroot
 
 
 def main(tree):
-    cmd = [
-        "/usr/sbin/chroot", tree,
-        "/usr/sbin/authconfig", "--nostart", "--updateall"
-    ]
-
-    subprocess.run(cmd, check=True)
+    with Chroot(tree) as chroot:
+        chroot.run(["/usr/sbin/authconfig", "--nostart", "--updateall"], check=True)
 
     shutil.rmtree(f"{tree}/var/lib/authselect/backups", ignore_errors=True)
 
diff --git a/stages/org.osbuild.firewall b/stages/org.osbuild.firewall
index 0f71b832..ec1c34ff 100755
--- a/stages/org.osbuild.firewall
+++ b/stages/org.osbuild.firewall
@@ -1,8 +1,8 @@
 #!/usr/bin/python3
-import subprocess
 import sys
 
 import osbuild.api
+from osbuild.util.chroot import Chroot
 
 
 def main(tree, options):
@@ -18,14 +18,14 @@ def main(tree, options):
 
     # firewall-offline-cmd does not implement --root option so we must chroot it
     if default_zone:
-        subprocess.run(["chroot", tree, "firewall-offline-cmd", f"--set-default-zone={default_zone}"], check=True)
+        with Chroot(tree) as chroot:
+            chroot.run(["firewall-offline-cmd", f"--set-default-zone={default_zone}"], check=True)
 
     # The options below are "lokkit" compatibility options and can not be used
     # with other options.
     if ports or enabled_services or disabled_services:
-        subprocess.run(["chroot",
-                        tree,
-                        "firewall-offline-cmd"] +
+        with Chroot(tree) as chroot:
+            chroot.run(["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)),
@@ -37,24 +37,21 @@ def main(tree, options):
         zone_name = zone_item['name']
         # check that the given zone exists, if not create it
         if zone_name != "":
-            res = subprocess.run(["chroot",
-                                  tree,
-                                  "firewall-offline-cmd",
+            with Chroot(tree) as chroot:
+                res = chroot.run(["firewall-offline-cmd",
                                   f"--info-zone={zone_name}"],
                                  check=False)
             # INVALID_ZONE error code
             if res.returncode == 112:
-                res = subprocess.run(["chroot",
-                                      tree,
-                                      "firewall-offline-cmd",
+                with Chroot(tree) as chroot:
+                    res = chroot.run(["firewall-offline-cmd",
                                       f"--new-zone={zone_name}"],
                                      check=False)
             if res.returncode != 0:
                 return 1
         if zone_item.get("sources", []):
-            subprocess.run(["chroot",
-                            tree,
-                            "firewall-offline-cmd", f"--zone={zone_name}"] +
+            with Chroot(tree) as chroot:
+                chroot.run(["firewall-offline-cmd", f"--zone={zone_name}"] +
                            list(map(lambda x: f"--add-source={x}",
                                     zone_item['sources'])),
                            check=True)