From 45a3503700d718a75aa2124c6dff8f23adb4ea50 Mon Sep 17 00:00:00 2001
From: Alex Pyrgiotis <alex.p@freedom.press>
Date: Mon, 2 Dec 2024 17:05:38 +0200
Subject: [PATCH] container: Revamp container image installation

Revamp the container image installation process in a way that does not
involve using image IDs. We don't want to rely on image IDs anymore,
since they are brittle (see
https://github.com/freedomofpress/dangerzone/issues/933). Instead, we
use image tags, as provided in the `image-id.txt` file.  This allows us
to check fast if an image is up to date, and we no longer need to
maintain multiple image IDs from various container runtimes.

Refs #933
Refs #988
Fixes #1020
---
 dangerzone/isolation_provider/container.py | 98 +++++++++-------------
 1 file changed, 39 insertions(+), 59 deletions(-)

diff --git a/dangerzone/isolation_provider/container.py b/dangerzone/isolation_provider/container.py
index 068b5d813..86fd1a659 100644
--- a/dangerzone/isolation_provider/container.py
+++ b/dangerzone/isolation_provider/container.py
@@ -226,7 +226,7 @@ def add_image_tag(cur_tag: str, new_tag: str) -> None:
     def get_expected_tag() -> str:
         """Get the tag of the Dangerzone image tarball from the image-id.txt file."""
         with open(get_resource_path("image-id.txt")) as f:
-            return f.read.strip()
+            return f.read().strip()
 
     @staticmethod
     def load_image_tarball() -> None:
@@ -261,18 +261,50 @@ def load_image_tarball() -> None:
 
     @staticmethod
     def install() -> bool:
+        """Install the container image tarball, or verify that it's already installed.
+
+        Perform the following actions:
+        1. Get the tags of any locally available images that match Dangerzone's image
+           name.
+        2. Get the expected image tag from the image-id.txt file.
+           - If this tag is present in the local images, and that image is also tagged
+             as "latest", then we can return.
+           - Else, prune the older container images and continue.
+        3. Load the image tarball and make sure it matches the expected tag.
+        4. Tag that image as "latest", and mark the installation as finished.
         """
-        Make sure the podman container is installed. Linux only.
-        """
-        if Container.is_container_installed():
+        old_tags = Container.list_image_tags()
+        expected_tag = Container.get_expected_tag()
+
+        if expected_tag not in old_tags:
+            # Prune older container images.
+            log.info(
+                f"Could not find a Dangerzone container image with tag '{expected_tag}'"
+            )
+            for tag in old_tags.keys():
+                Container.delete_image_tag(tag)
+        elif old_tags[expected_tag] != old_tags.get("latest"):
+            log.info(f"The expected tag '{expected_tag}' is not the latest one")
+            Container.add_image_tag(expected_tag, "latest")
+            return True
+        else:
             return True
 
+        # Load the image tarball into the container runtime.
         Container.load_image_tarball()
 
-        if not Container.is_container_installed(raise_on_error=True):
-            return False
+        # Check that the container image has the expected image tag.
+        # See https://github.com/freedomofpress/dangerzone/issues/988 for an example
+        # where this was not the case.
+        new_tags = Container.list_image_tags()
+        if expected_tag not in new_tags:
+            raise ImageNotPresentException(
+                f"Could not find expected tag '{expected_tag}' after loading the"
+                " container image tarball"
+            )
 
-        log.info("Container image installed")
+        # Mark the expected tag as "latest".
+        Container.add_image_tag(expected_tag, "latest")
         return True
 
     @staticmethod
@@ -291,58 +323,6 @@ def is_runtime_available() -> bool:
                 raise NotAvailableContainerTechException(runtime_name, stderr.decode())
             return True
 
-    @staticmethod
-    def is_container_installed(raise_on_error: bool = False) -> bool:
-        """
-        See if the container is installed.
-        """
-        # Get the image id
-        with open(get_resource_path("image-id.txt")) as f:
-            expected_image_ids = f.read().strip().split()
-
-        # See if this image is already installed
-        installed = False
-        found_image_id = subprocess.check_output(
-            [
-                Container.get_runtime(),
-                "image",
-                "list",
-                "--format",
-                "{{.ID}}",
-                Container.CONTAINER_NAME,
-            ],
-            text=True,
-            startupinfo=get_subprocess_startupinfo(),
-        )
-        found_image_id = found_image_id.strip()
-
-        if found_image_id in expected_image_ids:
-            installed = True
-        elif found_image_id == "":
-            if raise_on_error:
-                raise ImageNotPresentException(
-                    "Image is not listed after installation. Bailing out."
-                )
-        else:
-            msg = (
-                f"{Container.CONTAINER_NAME} images found, but IDs do not match."
-                f" Found: {found_image_id}, Expected: {','.join(expected_image_ids)}"
-            )
-            if raise_on_error:
-                raise ImageNotPresentException(msg)
-            log.info(msg)
-            log.info("Deleting old dangerzone container image")
-
-            try:
-                subprocess.check_output(
-                    [Container.get_runtime(), "rmi", "--force", found_image_id],
-                    startupinfo=get_subprocess_startupinfo(),
-                )
-            except Exception:
-                log.warning("Couldn't delete old container image, so leaving it there")
-
-        return installed
-
     def doc_to_pixels_container_name(self, document: Document) -> str:
         """Unique container name for the doc-to-pixels phase."""
         return f"dangerzone-doc-to-pixels-{document.id}"