From e72b89cde9d80215d3fa164854bc8777b01ea20d Mon Sep 17 00:00:00 2001 From: Alex Pyrgiotis 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 +++++++++------------- tests/isolation_provider/test_container.py | 6 +- 2 files changed, 43 insertions(+), 61 deletions(-) diff --git a/dangerzone/isolation_provider/container.py b/dangerzone/isolation_provider/container.py index 0954a538e..36376def7 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}" diff --git a/tests/isolation_provider/test_container.py b/tests/isolation_provider/test_container.py index 3fb324322..a1a844d32 100644 --- a/tests/isolation_provider/test_container.py +++ b/tests/isolation_provider/test_container.py @@ -69,10 +69,11 @@ def test_install_raise_if_image_cant_be_installed( "image", "list", "--format", - "{{.ID}}", + "json", "dangerzone.rocks/dangerzone", ], occurrences=2, + stdout="{}", ) # Make podman load fail @@ -102,10 +103,11 @@ def test_install_raises_if_still_not_installed( "image", "list", "--format", - "{{.ID}}", + "json", "dangerzone.rocks/dangerzone", ], occurrences=2, + stdout="{}", ) # Patch gzip.open and podman load so that it works