From 215068b41d34628d6081069b6d9b8116a0d83957 Mon Sep 17 00:00:00 2001 From: Russell Martin Date: Fri, 23 Jun 2023 16:32:12 -0400 Subject: [PATCH 1/8] Add support for Docker Desktop and rootless Docker --- changes/1083.feature.rst | 1 + src/briefcase/integrations/docker.py | 160 ++++++++++++++++++---- src/briefcase/platforms/linux/appimage.py | 8 ++ src/briefcase/platforms/linux/system.py | 25 ++-- 4 files changed, 156 insertions(+), 38 deletions(-) create mode 100644 changes/1083.feature.rst diff --git a/changes/1083.feature.rst b/changes/1083.feature.rst new file mode 100644 index 000000000..c4a902781 --- /dev/null +++ b/changes/1083.feature.rst @@ -0,0 +1 @@ +On Linux, Docker Desktop and rootless Docker are now supported. diff --git a/src/briefcase/integrations/docker.py b/src/briefcase/integrations/docker.py index cf2410088..d2c4dbd52 100644 --- a/src/briefcase/integrations/docker.py +++ b/src/briefcase/integrations/docker.py @@ -112,8 +112,17 @@ class Docker(Tool): } @classmethod - def verify_install(cls, tools: ToolCache, **kwargs) -> Docker: - """Verify Docker is installed and operational.""" + def verify_install( + cls, + tools: ToolCache, + image_tag: str | None = None, + **kwargs, + ) -> Docker: + """Verify Docker is installed and operational. + + :param tools: ToolCache of available tools + :param image_tag: Specific image to use to assess Docker's operating mode + """ # short circuit since already verified and available if hasattr(tools, "docker"): return tools.docker @@ -123,6 +132,8 @@ def verify_install(cls, tools: ToolCache, **kwargs) -> Docker: cls._buildx_installed(tools=tools) tools.docker = Docker(tools=tools) + tools.docker._determine_docker_mode(image_tag=image_tag) + return tools.docker @classmethod @@ -189,16 +200,131 @@ def _buildx_installed(cls, tools: ToolCache): except subprocess.CalledProcessError: raise BriefcaseCommandError(cls.BUILDX_PLUGIN_MISSING) + def _determine_docker_mode(self, image_tag: str | None = None): + """Determine Docker's operating mode from how it interacts with bind mounts. + + Docker can be installed in different ways on Linux that significantly impact how + containers interact with the host system. Of particular note is ownership of + files and directories in bind mounts (i.e. mounts using --volume). + + Traditionally, Docker would pass through the UID/GID of the user used inside a + container as the owner of files created within the bind mount. And since the + default user inside containers is root, the files would be owned by root on the + host file system; this prevents later interaction with those files by the host. + To work around this, the Dockerfile can use a step-down user with a UID and GID + that matches the host user running Docker. + + Other installation methods of Docker, though, are not compatible with using such + a step-down user. This includes Docker Desktop and rootless Docker. In these + modes, Docker maps the host user to the root user inside the container; this + mapping is transparent and would require changes to the host environment to + disable, if it can be disabled at all. This allows files created in bind mounts + inside the container to be owned on the host file system by the user running + Docker. Additionally, though, because the host user is mapped to root inside the + container, any files that were created by the host user in the bind mount + outside the container are owned by root inside the container; therefore, a step- + down user could not interact with such bind mount files inside the container. + + To accommodate these different modes, this checks which user owns a file that is + created inside a bind mount in the container. If the owning user of that file on + the host file system is root, then a step-down user is necessary inside + containers. If the owning user is the host user, root should be used. + """ + write_test_filename = "container_write_test" + host_write_test_dir_path = Path.cwd() + host_write_test_file_path = Path(host_write_test_dir_path, write_test_filename) + container_mount_host_dir = "/host_write_test" + container_write_test_file_path = PurePosixPath( + container_mount_host_dir, write_test_filename + ) + + docker_run_cmd = [ + "docker", + "run", + "--rm", + "--volume", + f"{host_write_test_dir_path}:{container_mount_host_dir}:z", + "alpine" if image_tag is None else image_tag, + ] + + try: + host_write_test_file_path.unlink(missing_ok=True) + except OSError as e: + raise BriefcaseCommandError( + f""" +The file path used to determine Docker's operating mode already exists and +cannot be automatically deleted. + + {host_write_test_file_path} + +Delete this file and run Briefcase again. +""" + ) from e + + try: + self.tools.subprocess.run( + docker_run_cmd + ["touch", container_write_test_file_path], + check=True, + stream_output=False, + ) + except subprocess.CalledProcessError as e: + raise BriefcaseCommandError( + "Unable to determine Docker's operating mode" + ) from e + + # if the file is not owned by `root`, then Docker is mapping usernames + self.is_userns_remap = 0 != self.tools.os.stat(host_write_test_file_path).st_uid + + try: + self.tools.subprocess.run( + docker_run_cmd + ["rm", "-f", container_write_test_file_path], + check=True, + stream_output=False, + ) + except subprocess.CalledProcessError as e: + raise BriefcaseCommandError( + "Unable to clean up from determining Docker's operating mode" + ) from e + + def cache_image(self, image_tag: str): + """Ensures an image is available and cached locally. + + While many Docker commands for an image will pull that image in-line with the + command if it isn't already cached, this pollutes the console output with + details about pulling the image. This can be particularly troublesome when the + output from a command run inside a container using the image is desired. + + Note: This will not update an already cached image if a newer version is + available in the registry. + + :param image_tag: Image name/tag to pull if not locally cached + """ + image_id = self.tools.subprocess.check_output( + ["docker", "images", "-q", image_tag] + ).strip() + + if not image_id: + try: + self.tools.subprocess.run(["docker", "pull", image_tag]) + except subprocess.CalledProcessError as e: + raise BriefcaseCommandError( + f"Unable to obtain the Docker image for {image_tag}. " + "Is the image name correct?" + ) from e + def check_output(self, args: list[SubprocessArgT], image_tag: str) -> str: """Run a process inside a Docker container, capturing output. - This is a bare Docker invocation; it's really only useful for running simple - commands on an image, ensuring that the container is destroyed afterward. In - most cases, you'll want to use an app context, rather than this. + This ensures the image is locally cached and then runs a bare Docker invocation; + it's really only useful for running simple commands on an image, ensuring that + the container is destroyed afterward. In most cases, you'll want to use an app + context, rather than this. :param args: The list of arguments to pass to the Docker instance. :param image_tag: The Docker image to run """ + self.cache_image(image_tag) + # Any exceptions from running the process are *not* caught. # This ensures that "docker.check_output()" behaves as closely to # "subprocess.check_output()" as possible. @@ -212,28 +338,6 @@ def check_output(self, args: list[SubprocessArgT], image_tag: str) -> str: + args, ) - def prepare(self, image_tag: str): - """Ensure that the given image exists, and is cached locally. - - This is achieved by trying to run a no-op command (echo) on the image; if it - succeeds, the image exists locally. - - A pull is forced, so you can be certain that the image is up-to-date. - - :param image_tag: The Docker image to prepare - """ - try: - self.tools.subprocess.run( - ["docker", "run", "--rm", image_tag, "printf", ""], - check=True, - stream_output=False, - ) - except subprocess.CalledProcessError as e: - raise BriefcaseCommandError( - f"Unable to obtain the Docker base image {image_tag}. " - "Is the image name correct?" - ) from e - class DockerAppContext(Tool): name = "docker_app_context" @@ -354,7 +458,7 @@ def _dockerize_path(self, arg: str) -> str: # pragma: no-cover-if-is-windows filesystem. Converts: - * any reference to sys.executable into the python executable in the docker container + * any reference to `sys.executable` into the python executable in the docker container * any path in into the equivalent stemming from /app * any path in into the equivalent in ~/.cache/briefcase diff --git a/src/briefcase/platforms/linux/appimage.py b/src/briefcase/platforms/linux/appimage.py index 21e27d89c..8fc392f29 100644 --- a/src/briefcase/platforms/linux/appimage.py +++ b/src/briefcase/platforms/linux/appimage.py @@ -150,6 +150,7 @@ class LinuxAppImageCreateCommand( def output_format_template_context(self, app: AppConfig): context = super().output_format_template_context(app) + # Add the manylinux tag to the template context. try: tag = getattr(app, "manylinux_image_tag", "latest") @@ -167,6 +168,13 @@ def output_format_template_context(self, app: AppConfig): except AttributeError: pass + # Use the non-root brutus user if Docker is mapping usernames + # (only relevant if Docker is being used for the target platform) + try: + context["use_non_root_user"] = not self.tools.docker.is_userns_remap + except AttributeError: + pass + return context def _cleanup_app_support_package(self, support_path): diff --git a/src/briefcase/platforms/linux/system.py b/src/briefcase/platforms/linux/system.py index e9a5fdada..3593225cf 100644 --- a/src/briefcase/platforms/linux/system.py +++ b/src/briefcase/platforms/linux/system.py @@ -3,6 +3,7 @@ import re import subprocess import sys +from contextlib import suppress from pathlib import Path from typing import List @@ -271,16 +272,15 @@ def platform_freedesktop_info(self, app): # Preserve the target image on the command line as the app's target app.target_image = self.target_image - # Ensure that the Docker base image is available. - self.logger.info(f"Checking Docker target image {app.target_image}...") - self.tools.docker.prepare(app.target_image) - # Extract release information from the image. - output = self.tools.docker.check_output( - ["cat", "/etc/os-release"], - image_tag=app.target_image, - ) - freedesktop_info = parse_freedesktop_os_release(output) + with self.input.wait_bar( + f"Checking Docker target image {app.target_image}..." + ): + output = self.tools.docker.check_output( + ["cat", "/etc/os-release"], + image_tag=app.target_image, + ) + freedesktop_info = parse_freedesktop_os_release(output) else: freedesktop_info = super().platform_freedesktop_info(app) @@ -294,7 +294,7 @@ def verify_tools(self): """If we're using Docker, verify that it is available.""" super().verify_tools() if self.use_docker: - Docker.verify(tools=self.tools) + Docker.verify(tools=self.tools, image_tag=self.target_image) def add_options(self, parser): super().add_options(parser) @@ -559,6 +559,11 @@ def output_format_template_context(self, app: AppConfig): # Add the vendor base context["vendor_base"] = app.target_vendor_base + # Use the non-root brutus user if Docker is mapping usernames + # (only relevant if Docker is being used for the target platform) + with suppress(AttributeError): + context["use_non_root_user"] = not self.tools.docker.is_userns_remap + return context From d21903586606e112f20b4b3c9441bc6c55038ec6 Mon Sep 17 00:00:00 2001 From: Russell Martin Date: Wed, 5 Jul 2023 13:23:47 -0400 Subject: [PATCH 2/8] Revise user mapping inspection implementation - Perform check in constructor so the setting is always available - Perform write test in `build` directory to help prevent project pollution --- src/briefcase/integrations/docker.py | 70 +++++++++++++++++------ src/briefcase/platforms/linux/appimage.py | 8 +-- src/briefcase/platforms/linux/system.py | 7 +-- 3 files changed, 57 insertions(+), 28 deletions(-) diff --git a/src/briefcase/integrations/docker.py b/src/briefcase/integrations/docker.py index d2c4dbd52..c28cd36b8 100644 --- a/src/briefcase/integrations/docker.py +++ b/src/briefcase/integrations/docker.py @@ -111,6 +111,18 @@ class Docker(Tool): "Linux": "https://docs.docker.com/engine/install/#server", } + def __init__(self, tools: ToolCache, image_tag: str | None = None): + """A wrapper for the user-installed Docker. + + :param tools: ToolCache of available tools + :param image_tag: An optional image used to access attributes of the Docker + environment, such as how user permissions are managed in bind mounts. A + lightweight image will be used if one is not specified but this image is not + at all bound to the instance. + """ + super().__init__(tools=tools) + self.is_users_mapped = self._is_user_mapping_enabled(image_tag) + @classmethod def verify_install( cls, @@ -121,7 +133,9 @@ def verify_install( """Verify Docker is installed and operational. :param tools: ToolCache of available tools - :param image_tag: Specific image to use to assess Docker's operating mode + :param image_tag: An optional image used during verification to access + attributes of the local Docker environment. This image is not bound to the + instance and only used during instantiation. """ # short circuit since already verified and available if hasattr(tools, "docker"): @@ -131,9 +145,7 @@ def verify_install( cls._user_access(tools=tools) cls._buildx_installed(tools=tools) - tools.docker = Docker(tools=tools) - tools.docker._determine_docker_mode(image_tag=image_tag) - + tools.docker = Docker(tools=tools, image_tag=image_tag) return tools.docker @classmethod @@ -200,8 +212,8 @@ def _buildx_installed(cls, tools: ToolCache): except subprocess.CalledProcessError: raise BriefcaseCommandError(cls.BUILDX_PLUGIN_MISSING) - def _determine_docker_mode(self, image_tag: str | None = None): - """Determine Docker's operating mode from how it interacts with bind mounts. + def _is_user_mapping_enabled(self, image_tag: str | None = None) -> bool: + """Determine whether Docker is mapping users between the container and the host. Docker can be installed in different ways on Linux that significantly impact how containers interact with the host system. Of particular note is ownership of @@ -215,9 +227,10 @@ def _determine_docker_mode(self, image_tag: str | None = None): that matches the host user running Docker. Other installation methods of Docker, though, are not compatible with using such - a step-down user. This includes Docker Desktop and rootless Docker. In these - modes, Docker maps the host user to the root user inside the container; this - mapping is transparent and would require changes to the host environment to + a step-down user. This includes Docker Desktop and rootless Docker (although, + even a traditional installation of Docker Engine can be configured similarly). + In these modes, Docker maps the host user to the root user inside the container; + this mapping is transparent and would require changes to the host environment to disable, if it can be disabled at all. This allows files created in bind mounts inside the container to be owned on the host file system by the user running Docker. Additionally, though, because the host user is mapped to root inside the @@ -229,9 +242,28 @@ def _determine_docker_mode(self, image_tag: str | None = None): created inside a bind mount in the container. If the owning user of that file on the host file system is root, then a step-down user is necessary inside containers. If the owning user is the host user, root should be used. + + On macOS, Docker Desktop is the only option to use Docker and user mapping + happens differently such that any user in the container is mapped to the host + user. Instead of leveraging user namespaces as on Linux, this user mapping + manifests as a consequence of bind mounts being implemented as NFS shares + between macOS and the Linux VM that Docker Desktop runs containers in. So, + using a step-down user on macOS is effectively inconsequential. + + On Windows WSL 2, Docker Desktop operates similarly to how it does on Linux. + However, user namespace mapping is not possible because the Docker Desktop VM + and the WSL distro are already running in different user namespaces...and + therefore, Docker cannot even see the users in the distro to map them in to the + container. So, a step-down user is always used. + + ref: https://docs.docker.com/engine/security/userns-remap/ + + :param image_tag: The image:tag to use to create the container for the test; if + one is not specified, then `alpine:latest` will be used. + :returns: True if users are being mapped; False otherwise """ write_test_filename = "container_write_test" - host_write_test_dir_path = Path.cwd() + host_write_test_dir_path = Path.cwd() / "build" host_write_test_file_path = Path(host_write_test_dir_path, write_test_filename) container_mount_host_dir = "/host_write_test" container_write_test_file_path = PurePosixPath( @@ -247,13 +279,15 @@ def _determine_docker_mode(self, image_tag: str | None = None): "alpine" if image_tag is None else image_tag, ] + host_write_test_dir_path.mkdir(exist_ok=True) + try: host_write_test_file_path.unlink(missing_ok=True) except OSError as e: raise BriefcaseCommandError( - f""" -The file path used to determine Docker's operating mode already exists and -cannot be automatically deleted. + f"""\ +The file path used to determine how Docker is mapping users between the host +and Docker containers already exists and cannot be automatically deleted. {host_write_test_file_path} @@ -269,11 +303,11 @@ def _determine_docker_mode(self, image_tag: str | None = None): ) except subprocess.CalledProcessError as e: raise BriefcaseCommandError( - "Unable to determine Docker's operating mode" + "Unable to determine if Docker is mapping users" ) from e # if the file is not owned by `root`, then Docker is mapping usernames - self.is_userns_remap = 0 != self.tools.os.stat(host_write_test_file_path).st_uid + is_users_mapped = 0 != self.tools.os.stat(host_write_test_file_path).st_uid try: self.tools.subprocess.run( @@ -283,9 +317,11 @@ def _determine_docker_mode(self, image_tag: str | None = None): ) except subprocess.CalledProcessError as e: raise BriefcaseCommandError( - "Unable to clean up from determining Docker's operating mode" + "Unable to clean up from determining if Docker is mapping users" ) from e + return is_users_mapped + def cache_image(self, image_tag: str): """Ensures an image is available and cached locally. @@ -305,7 +341,7 @@ def cache_image(self, image_tag: str): if not image_id: try: - self.tools.subprocess.run(["docker", "pull", image_tag]) + self.tools.subprocess.run(["docker", "pull", image_tag], check=True) except subprocess.CalledProcessError as e: raise BriefcaseCommandError( f"Unable to obtain the Docker image for {image_tag}. " diff --git a/src/briefcase/platforms/linux/appimage.py b/src/briefcase/platforms/linux/appimage.py index 8fc392f29..8843aca37 100644 --- a/src/briefcase/platforms/linux/appimage.py +++ b/src/briefcase/platforms/linux/appimage.py @@ -168,12 +168,8 @@ def output_format_template_context(self, app: AppConfig): except AttributeError: pass - # Use the non-root brutus user if Docker is mapping usernames - # (only relevant if Docker is being used for the target platform) - try: - context["use_non_root_user"] = not self.tools.docker.is_userns_remap - except AttributeError: - pass + # Use the non-root brutus user if Docker is not mapping usernames + context["use_non_root_user"] = not self.tools.docker.is_users_mapped return context diff --git a/src/briefcase/platforms/linux/system.py b/src/briefcase/platforms/linux/system.py index 3593225cf..2086f794a 100644 --- a/src/briefcase/platforms/linux/system.py +++ b/src/briefcase/platforms/linux/system.py @@ -3,7 +3,6 @@ import re import subprocess import sys -from contextlib import suppress from pathlib import Path from typing import List @@ -559,10 +558,8 @@ def output_format_template_context(self, app: AppConfig): # Add the vendor base context["vendor_base"] = app.target_vendor_base - # Use the non-root brutus user if Docker is mapping usernames - # (only relevant if Docker is being used for the target platform) - with suppress(AttributeError): - context["use_non_root_user"] = not self.tools.docker.is_userns_remap + # Use the non-root brutus user if Docker is not mapping usernames + context["use_non_root_user"] = not self.tools.docker.is_users_mapped return context From 24cdf4c889657e581a866ddedaff6b3dddf277c3 Mon Sep 17 00:00:00 2001 From: Russell Martin Date: Wed, 5 Jul 2023 13:27:12 -0400 Subject: [PATCH 3/8] Add tests for Docker user mapping inspection --- src/briefcase/integrations/docker.py | 27 +- src/briefcase/platforms/linux/appimage.py | 7 +- src/briefcase/platforms/linux/system.py | 7 +- tests/integrations/docker/conftest.py | 47 ++- .../docker/test_DockerAppContext__verify.py | 12 +- .../docker/test_Docker__cache_image.py | 71 +++++ .../docker/test_Docker__check_output.py | 38 ++- .../docker/test_Docker__prepare.py | 52 ---- .../docker/test_Docker__verify.py | 290 +++++++++++++----- tests/platforms/linux/appimage/test_create.py | 52 +++- tests/platforms/linux/appimage/test_mixin.py | 14 + tests/platforms/linux/system/test_create.py | 37 ++- .../system/test_mixin__finalize_app_config.py | 3 - .../linux/system/test_mixin__verify.py | 14 + 14 files changed, 508 insertions(+), 163 deletions(-) create mode 100644 tests/integrations/docker/test_Docker__cache_image.py delete mode 100644 tests/integrations/docker/test_Docker__prepare.py diff --git a/src/briefcase/integrations/docker.py b/src/briefcase/integrations/docker.py index c28cd36b8..9ba865ddf 100644 --- a/src/briefcase/integrations/docker.py +++ b/src/briefcase/integrations/docker.py @@ -212,6 +212,10 @@ def _buildx_installed(cls, tools: ToolCache): except subprocess.CalledProcessError: raise BriefcaseCommandError(cls.BUILDX_PLUGIN_MISSING) + def _write_test_path(self) -> Path: + """Host system filepath to perform write test from a container.""" + return Path.cwd() / "build" / "container_write_test" + def _is_user_mapping_enabled(self, image_tag: str | None = None) -> bool: """Determine whether Docker is mapping users between the container and the host. @@ -262,12 +266,9 @@ def _is_user_mapping_enabled(self, image_tag: str | None = None) -> bool: one is not specified, then `alpine:latest` will be used. :returns: True if users are being mapped; False otherwise """ - write_test_filename = "container_write_test" - host_write_test_dir_path = Path.cwd() / "build" - host_write_test_file_path = Path(host_write_test_dir_path, write_test_filename) - container_mount_host_dir = "/host_write_test" - container_write_test_file_path = PurePosixPath( - container_mount_host_dir, write_test_filename + host_write_test_path = self._write_test_path() + container_write_test_path = PurePosixPath( + "/host_write_test", host_write_test_path.name ) docker_run_cmd = [ @@ -275,21 +276,21 @@ def _is_user_mapping_enabled(self, image_tag: str | None = None) -> bool: "run", "--rm", "--volume", - f"{host_write_test_dir_path}:{container_mount_host_dir}:z", + f"{host_write_test_path.parent}:{container_write_test_path.parent}:z", "alpine" if image_tag is None else image_tag, ] - host_write_test_dir_path.mkdir(exist_ok=True) + host_write_test_path.parent.mkdir(exist_ok=True) try: - host_write_test_file_path.unlink(missing_ok=True) + host_write_test_path.unlink(missing_ok=True) except OSError as e: raise BriefcaseCommandError( f"""\ The file path used to determine how Docker is mapping users between the host and Docker containers already exists and cannot be automatically deleted. - {host_write_test_file_path} + {host_write_test_path} Delete this file and run Briefcase again. """ @@ -297,7 +298,7 @@ def _is_user_mapping_enabled(self, image_tag: str | None = None) -> bool: try: self.tools.subprocess.run( - docker_run_cmd + ["touch", container_write_test_file_path], + docker_run_cmd + ["touch", container_write_test_path], check=True, stream_output=False, ) @@ -307,11 +308,11 @@ def _is_user_mapping_enabled(self, image_tag: str | None = None) -> bool: ) from e # if the file is not owned by `root`, then Docker is mapping usernames - is_users_mapped = 0 != self.tools.os.stat(host_write_test_file_path).st_uid + is_users_mapped = 0 != self.tools.os.stat(host_write_test_path).st_uid try: self.tools.subprocess.run( - docker_run_cmd + ["rm", "-f", container_write_test_file_path], + docker_run_cmd + ["rm", "-f", container_write_test_path], check=True, stream_output=False, ) diff --git a/src/briefcase/platforms/linux/appimage.py b/src/briefcase/platforms/linux/appimage.py index 8843aca37..555f297ce 100644 --- a/src/briefcase/platforms/linux/appimage.py +++ b/src/briefcase/platforms/linux/appimage.py @@ -168,8 +168,11 @@ def output_format_template_context(self, app: AppConfig): except AttributeError: pass - # Use the non-root brutus user if Docker is not mapping usernames - context["use_non_root_user"] = not self.tools.docker.is_users_mapped + # Use the non-root user if Docker is not mapping usernames + try: + context["use_non_root_user"] = not self.tools.docker.is_users_mapped + except AttributeError: + pass # ignore if not using Docker return context diff --git a/src/briefcase/platforms/linux/system.py b/src/briefcase/platforms/linux/system.py index 2086f794a..116e4a208 100644 --- a/src/briefcase/platforms/linux/system.py +++ b/src/briefcase/platforms/linux/system.py @@ -558,8 +558,11 @@ def output_format_template_context(self, app: AppConfig): # Add the vendor base context["vendor_base"] = app.target_vendor_base - # Use the non-root brutus user if Docker is not mapping usernames - context["use_non_root_user"] = not self.tools.docker.is_users_mapped + # Use the non-root user if Docker is not mapping usernames + try: + context["use_non_root_user"] = not self.tools.docker.is_users_mapped + except AttributeError: + pass # ignore if not using Docker return context diff --git a/tests/integrations/docker/conftest.py b/tests/integrations/docker/conftest.py index f2eecf872..5d2e8ce1e 100644 --- a/tests/integrations/docker/conftest.py +++ b/tests/integrations/docker/conftest.py @@ -1,15 +1,17 @@ import subprocess -from unittest.mock import MagicMock +from pathlib import PurePosixPath +from unittest.mock import MagicMock, call import pytest +import briefcase from briefcase.config import AppConfig from briefcase.integrations.base import ToolCache from briefcase.integrations.docker import DockerAppContext @pytest.fixture -def mock_tools(mock_tools) -> ToolCache: +def mock_tools(mock_tools, tmp_path) -> ToolCache: # Mock stdlib subprocess module mock_tools.subprocess._subprocess = MagicMock(spec_set=subprocess) @@ -66,3 +68,44 @@ def mock_docker_app_context(tmp_path, my_app, mock_tools) -> DockerAppContext: mock_docker_app_context.tools.subprocess._subprocess.Popen.reset_mock() return mock_docker_app_context + + +@pytest.fixture +def user_mapping_run_calls(tmp_path, monkeypatch) -> list: + # Mock the container write test path in to pytest's tmp directory + monkeypatch.setattr( + briefcase.integrations.docker.Docker, + "_write_test_path", + MagicMock(return_value=tmp_path / "build" / "mock_write_test"), + ) + return [ + call( + [ + "docker", + "run", + "--rm", + "--volume", + f"{tmp_path / 'build'}:/host_write_test:z", + "alpine", + "touch", + PurePosixPath("/host_write_test/mock_write_test"), + ], + check=True, + stream_output=False, + ), + call( + [ + "docker", + "run", + "--rm", + "--volume", + f"{tmp_path / 'build'}:/host_write_test:z", + "alpine", + "rm", + "-f", + PurePosixPath("/host_write_test/mock_write_test"), + ], + check=True, + stream_output=False, + ), + ] diff --git a/tests/integrations/docker/test_DockerAppContext__verify.py b/tests/integrations/docker/test_DockerAppContext__verify.py index fc754b9b4..fc8f10875 100644 --- a/tests/integrations/docker/test_DockerAppContext__verify.py +++ b/tests/integrations/docker/test_DockerAppContext__verify.py @@ -82,7 +82,7 @@ def test_success(mock_tools, first_app_config, verify_kwargs): def test_docker_verify_fail(mock_tools, first_app_config, verify_kwargs): """Failure if Docker cannot be verified.""" mock_tools.subprocess = MagicMock(spec_set=Subprocess) - # Mock the existence of Docker. + # Mock the absence of Docker mock_tools.subprocess.check_output.side_effect = FileNotFoundError with pytest.raises(BriefcaseCommandError, match="Briefcase requires Docker"): @@ -99,9 +99,13 @@ def test_docker_image_build_fail(mock_tools, first_app_config, verify_kwargs): "github.com/docker/buildx v0.10.2 00ed17d\n", ] - mock_tools.subprocess.run.side_effect = subprocess.CalledProcessError( - returncode=80, cmd=["docker" "build"] - ) + mock_tools.subprocess.run.side_effect = [ + # Mock the user mapping inspection calls + "", + "", + # Mock the image build failing + subprocess.CalledProcessError(returncode=80, cmd=["docker" "build"]), + ] with pytest.raises( BriefcaseCommandError, diff --git a/tests/integrations/docker/test_Docker__cache_image.py b/tests/integrations/docker/test_Docker__cache_image.py new file mode 100644 index 000000000..44f62f4c6 --- /dev/null +++ b/tests/integrations/docker/test_Docker__cache_image.py @@ -0,0 +1,71 @@ +import subprocess +from unittest.mock import MagicMock, call + +import pytest + +from briefcase.exceptions import BriefcaseCommandError +from briefcase.integrations.base import ToolCache +from briefcase.integrations.docker import Docker +from briefcase.integrations.subprocess import Subprocess + + +@pytest.fixture +def mock_tools(mock_tools, user_mapping_run_calls) -> ToolCache: + mock_tools.subprocess = MagicMock(spec_set=Subprocess) + mock_tools.docker = Docker(mock_tools) + return mock_tools + + +def test_cache_image(mock_tools, user_mapping_run_calls): + """A Docker image can be cached.""" + # mock image not being cached in Docker + mock_tools.subprocess.check_output.return_value = "" + + # Cache an image + mock_tools.docker.cache_image("ubuntu:jammy") + + # Confirms that image is not available + mock_tools.subprocess.check_output.assert_called_once_with( + ["docker", "images", "-q", "ubuntu:jammy"] + ) + + # Image is pulled and cached + mock_tools.subprocess.run.assert_has_calls( + user_mapping_run_calls + [call(["docker", "pull", "ubuntu:jammy"], check=True)] + ) + + +def test_cache_image_already_cached(mock_tools, user_mapping_run_calls): + """A Docker image is not pulled if it is already cached.""" + # mock image already cached in Docker + mock_tools.subprocess.check_output.return_value = "99284ca6cea0" + + # Cache an image + mock_tools.docker.cache_image("ubuntu:jammy") + + # Confirms that image is not available + mock_tools.subprocess.check_output.assert_called_once_with( + ["docker", "images", "-q", "ubuntu:jammy"] + ) + + # Image is not pulled and cached + mock_tools.subprocess.run.assert_has_calls(user_mapping_run_calls) + + +def test_cache_bad_image(mock_tools): + """If an image is invalid, an exception raised.""" + # mock image not being cached in Docker + mock_tools.subprocess.check_output.return_value = "" + + # Mock a Docker failure due to a bad image + mock_tools.subprocess.run.side_effect = subprocess.CalledProcessError( + returncode=125, + cmd="docker...", + ) + + # Try to cache an image that doesn't exist: + with pytest.raises( + BriefcaseCommandError, + match=r"Unable to obtain the Docker image for ubuntu:does-not-exist.", + ): + mock_tools.docker.cache_image("ubuntu:does-not-exist") diff --git a/tests/integrations/docker/test_Docker__check_output.py b/tests/integrations/docker/test_Docker__check_output.py index 31887ce95..8cce147e2 100644 --- a/tests/integrations/docker/test_Docker__check_output.py +++ b/tests/integrations/docker/test_Docker__check_output.py @@ -1,4 +1,5 @@ -from unittest.mock import MagicMock +import subprocess +from unittest.mock import MagicMock, call import pytest @@ -16,10 +17,41 @@ def mock_tools(mock_tools) -> ToolCache: def test_check_output(mock_tools): """A command can be invoked on a bare Docker image.""" + # mock image already being cached in Docker + mock_tools.subprocess.check_output.side_effect = ["1ed313b0551f", "output"] # Run the command in a container mock_tools.docker.check_output(["cmd", "arg1", "arg2"], image_tag="ubuntu:jammy") - mock_tools.subprocess.check_output.assert_called_once_with( - ["docker", "run", "--rm", "ubuntu:jammy", "cmd", "arg1", "arg2"] + mock_tools.subprocess.check_output.assert_has_calls( + [ + # Verify image is cached in Docker + call(["docker", "images", "-q", "ubuntu:jammy"]), + # Run command in Docker using image + call(["docker", "run", "--rm", "ubuntu:jammy", "cmd", "arg1", "arg2"]), + ] + ) + + +def test_check_output_fail(mock_tools): + """Any subprocess errors are passed back through directly.""" + # mock image already being cached in Docker and check_output() call fails + mock_tools.subprocess.check_output.side_effect = [ + "1ed313b0551f", + subprocess.CalledProcessError(returncode=1, cmd=["cmd", "arg1", "arg2"]), + ] + + # The CalledProcessError surfaces from Docker().check_output() + with pytest.raises(subprocess.CalledProcessError): + mock_tools.docker.check_output( + ["cmd", "arg1", "arg2"], image_tag="ubuntu:jammy" + ) + + mock_tools.subprocess.check_output.assert_has_calls( + [ + # Verify image is cached in Docker + call(["docker", "images", "-q", "ubuntu:jammy"]), + # Command errors in Docker using image + call(["docker", "run", "--rm", "ubuntu:jammy", "cmd", "arg1", "arg2"]), + ] ) diff --git a/tests/integrations/docker/test_Docker__prepare.py b/tests/integrations/docker/test_Docker__prepare.py deleted file mode 100644 index be3abc556..000000000 --- a/tests/integrations/docker/test_Docker__prepare.py +++ /dev/null @@ -1,52 +0,0 @@ -import subprocess -from unittest.mock import MagicMock - -import pytest - -from briefcase.exceptions import BriefcaseCommandError -from briefcase.integrations.base import ToolCache -from briefcase.integrations.docker import Docker -from briefcase.integrations.subprocess import Subprocess - - -@pytest.fixture -def mock_tools(mock_tools) -> ToolCache: - mock_tools.subprocess = MagicMock(spec_set=Subprocess) - mock_tools.docker = Docker(mock_tools) - return mock_tools - - -def test_prepare(mock_tools): - """A docker image can be prepared.""" - - # Prepare an image - mock_tools.docker.prepare("ubuntu:jammy") - - mock_tools.subprocess.run.assert_called_once_with( - ["docker", "run", "--rm", "ubuntu:jammy", "printf", ""], - check=True, - stream_output=False, - ) - - -def test_prepare_bad_image(mock_tools): - """If an image is invalid, an exception raised,""" - # Mock a Docker failure due to a bad image - mock_tools.subprocess.run.side_effect = subprocess.CalledProcessError( - returncode=125, - cmd="docker...", - ) - - # Try to prepare an image that doesn't exist: - with pytest.raises( - BriefcaseCommandError, - match=r"Unable to obtain the Docker base image ubuntu:does-not-exist.", - ): - mock_tools.docker.prepare("ubuntu:does-not-exist") - - # The subprocess call was made. - mock_tools.subprocess.run.assert_called_once_with( - ["docker", "run", "--rm", "ubuntu:does-not-exist", "printf", ""], - check=True, - stream_output=False, - ) diff --git a/tests/integrations/docker/test_Docker__verify.py b/tests/integrations/docker/test_Docker__verify.py index 712869a85..036a4bedf 100644 --- a/tests/integrations/docker/test_Docker__verify.py +++ b/tests/integrations/docker/test_Docker__verify.py @@ -1,4 +1,6 @@ import subprocess +from collections import namedtuple +from pathlib import Path, PurePosixPath from unittest.mock import MagicMock, call import pytest @@ -8,10 +10,14 @@ from briefcase.integrations.docker import Docker from briefcase.integrations.subprocess import Subprocess - -@pytest.fixture -def valid_docker_version(): - return "Docker version 19.03.8, build afacb8b\n" +VALID_DOCKER_VERSION = "Docker version 19.03.8, build afacb8b\n" +VALID_DOCKER_INFO = "docker info printout" +VALID_BUILDX_VERSION = "github.com/docker/buildx v0.10.2 00ed17d\n" +DOCKER_VERIFICATION_CALLS = [ + call(["docker", "--version"]), + call(["docker", "info"]), + call(["docker", "buildx", "version"]), +] @pytest.fixture @@ -20,6 +26,16 @@ def mock_tools(mock_tools) -> ToolCache: return mock_tools +@pytest.fixture +def mock_write_test_path(tmp_path, monkeypatch): + """Mock the container write test path in to pytest's tmp directory.""" + write_test_path = tmp_path / "mock_write_test" + # Wrap the path so read-only methods can be replaced + write_test_path = MagicMock(wraps=write_test_path) + monkeypatch.setattr(Docker, "_write_test_path", lambda self: write_test_path) + return write_test_path + + def test_short_circuit(mock_tools): """Tool is not created if already cached.""" mock_tools.docker = "tool" @@ -47,13 +63,13 @@ def test_docker_install_url(host_os): assert host_os in Docker.DOCKER_INSTALL_URL -def test_docker_exists(mock_tools, valid_docker_version, capsys): +def test_docker_exists(mock_tools, user_mapping_run_calls, capsys, tmp_path): """If docker exists, the Docker wrapper is returned.""" # Mock the return value of Docker Version mock_tools.subprocess.check_output.side_effect = [ - valid_docker_version, - "docker info return value", - "github.com/docker/buildx v0.10.2 00ed17d\n", + VALID_DOCKER_VERSION, + VALID_DOCKER_INFO, + VALID_BUILDX_VERSION, ] # Invoke docker verify @@ -62,13 +78,11 @@ def test_docker_exists(mock_tools, valid_docker_version, capsys): # The verify call should return the Docker wrapper assert isinstance(result, Docker) - mock_tools.subprocess.check_output.assert_has_calls( - [ - call(["docker", "--version"]), - call(["docker", "info"]), - call(["docker", "buildx", "version"]), - ] - ) + # Docker version and plugins were verified + mock_tools.subprocess.check_output.assert_has_calls(DOCKER_VERIFICATION_CALLS) + + # Docker user mapping inspection occurred + mock_tools.subprocess.run.assert_has_calls(user_mapping_run_calls) # No console output output = capsys.readouterr() @@ -78,7 +92,7 @@ def test_docker_exists(mock_tools, valid_docker_version, capsys): def test_docker_doesnt_exist(mock_tools): """If docker doesn't exist, an error is raised.""" - # Mock the return value of Docker Version + # Mock Docker not installed on system mock_tools.subprocess.check_output.side_effect = FileNotFoundError # Invoke Docker verify @@ -89,17 +103,17 @@ def test_docker_doesnt_exist(mock_tools): mock_tools.subprocess.check_output.assert_called_with(["docker", "--version"]) -def test_docker_failure(mock_tools, capsys): +def test_docker_failure(mock_tools, user_mapping_run_calls, capsys): """If docker failed during execution, the Docker wrapper is returned with a warning.""" - # Mock the return value of Docker Version + # Mock Docker cannot be found mock_tools.subprocess.check_output.side_effect = [ subprocess.CalledProcessError( returncode=1, cmd="docker --version", ), "Success!", - "github.com/docker/buildx v0.10.2 00ed17d\n", + VALID_BUILDX_VERSION, ] # Invoke Docker verify @@ -108,13 +122,11 @@ def test_docker_failure(mock_tools, capsys): # The verify call should return the Docker wrapper assert isinstance(result, Docker) - mock_tools.subprocess.check_output.assert_has_calls( - [ - call(["docker", "--version"]), - call(["docker", "info"]), - call(["docker", "buildx", "version"]), - ] - ) + # Docker version and plugins were verified + mock_tools.subprocess.check_output.assert_has_calls(DOCKER_VERIFICATION_CALLS) + + # Docker user mapping inspection occurred + mock_tools.subprocess.run.assert_has_calls(user_mapping_run_calls) # console output output = capsys.readouterr() @@ -136,7 +148,7 @@ def test_docker_bad_version(mock_tools, capsys): Docker.verify(mock_tools) -def test_docker_unknown_version(mock_tools, capsys): +def test_docker_unknown_version(mock_tools, user_mapping_run_calls, capsys): """If docker exists but the version string doesn't make sense, the Docker wrapper is returned with a warning.""" # Mock a bad return value of `docker --version` @@ -148,13 +160,11 @@ def test_docker_unknown_version(mock_tools, capsys): # The verify call should return the Docker wrapper assert isinstance(result, Docker) - mock_tools.subprocess.check_output.assert_has_calls( - [ - call(["docker", "--version"]), - call(["docker", "info"]), - call(["docker", "buildx", "version"]), - ] - ) + # Docker version and plugins were verified + mock_tools.subprocess.check_output.assert_has_calls(DOCKER_VERIFICATION_CALLS) + + # Docker user mapping inspection occurred + mock_tools.subprocess.run.assert_has_calls(user_mapping_run_calls) # console output output = capsys.readouterr() @@ -162,10 +172,7 @@ def test_docker_unknown_version(mock_tools, capsys): assert output.err == "" -def test_docker_exists_but_process_lacks_permission_to_use_it( - mock_tools, - valid_docker_version, -): +def test_docker_exists_but_process_lacks_permission_to_use_it(mock_tools): """If the docker daemon isn't running, the check fails.""" error_message = """ Client: @@ -178,7 +185,7 @@ def test_docker_exists_but_process_lacks_permission_to_use_it( errors pretty printing info""" mock_tools.subprocess.check_output.side_effect = [ - valid_docker_version, + VALID_DOCKER_VERSION, subprocess.CalledProcessError( returncode=1, cmd="docker info", @@ -192,34 +199,30 @@ def test_docker_exists_but_process_lacks_permission_to_use_it( Docker.verify(mock_tools) -docker_not_running_error_messages = [ - """ - Client: - Debug Mode: false - - Server: - ERROR: Error response from daemon: dial unix docker.raw.sock: connect: connection refused - errors pretty printing info - """, # this is the error shown on mac - """ - Client: - Debug Mode: false - - Server: - ERROR: Cannot connect to the Docker daemon at unix:///var/run/docker.sock. Is the docker daemon running? - errors pretty printing info""", # this is the error show on linux -] +@pytest.mark.parametrize( + "error_message", + [ + """ +Client: + Debug Mode: false +Server: +ERROR: Error response from daemon: dial unix docker.raw.sock: connect: connection refused +errors pretty printing info +""", # this is the error shown on mac + """ +Client: + Debug Mode: false -@pytest.mark.parametrize("error_message", docker_not_running_error_messages) -def test_docker_exists_but_is_not_running( - error_message, - mock_tools, - valid_docker_version, -): +Server: +ERROR: Cannot connect to the Docker daemon at unix:///var/run/docker.sock. Is the docker daemon running? +errors pretty printing info""", # this is the error show on linux + ], +) +def test_docker_exists_but_is_not_running(error_message, mock_tools): """If the docker daemon isn't running, the check fails.""" mock_tools.subprocess.check_output.side_effect = [ - valid_docker_version, + VALID_DOCKER_VERSION, subprocess.CalledProcessError( returncode=1, cmd="docker info", @@ -233,13 +236,10 @@ def test_docker_exists_but_is_not_running( Docker.verify(mock_tools) -def test_docker_exists_but_unknown_error_when_running_command( - mock_tools, - valid_docker_version, -): +def test_docker_exists_but_unknown_error_when_running_command(mock_tools): """If docker info fails in unknown ways, the check fails.""" mock_tools.subprocess.check_output.side_effect = [ - valid_docker_version, + VALID_DOCKER_VERSION, subprocess.CalledProcessError( returncode=1, cmd="docker info", @@ -254,11 +254,11 @@ def test_docker_exists_but_unknown_error_when_running_command( Docker.verify(mock_tools) -def test_buildx_plugin_not_installed(mock_tools, valid_docker_version): +def test_buildx_plugin_not_installed(mock_tools): """If the buildx plugin is not installed, verification fails.""" mock_tools.subprocess.check_output.side_effect = [ - valid_docker_version, - "Success!", + VALID_DOCKER_VERSION, + VALID_DOCKER_INFO, subprocess.CalledProcessError( returncode=1, cmd="docker buildx version", @@ -270,3 +270,151 @@ def test_buildx_plugin_not_installed(mock_tools, valid_docker_version): match="Docker is installed and available for use but the buildx plugin\nis not installed", ): Docker.verify(mock_tools) + + +def test_docker_image_hint(mock_tools): + """If an image_tag is passed to verification, it is used for the user mapping + check.""" + # Mock the return values for Docker verification + mock_tools.subprocess.check_output.side_effect = [ + VALID_DOCKER_VERSION, + VALID_DOCKER_INFO, + VALID_BUILDX_VERSION, + ] + + Docker.verify(mock_tools, image_tag="myimage:tagtorulethemall") + + mock_tools.subprocess.run.assert_has_calls( + [ + call( + [ + "docker", + "run", + "--rm", + "--volume", + f"{Path.cwd() / 'build'}:/host_write_test:z", + "myimage:tagtorulethemall", + "touch", + PurePosixPath("/host_write_test/container_write_test"), + ], + check=True, + stream_output=False, + ), + call( + [ + "docker", + "run", + "--rm", + "--volume", + f"{Path.cwd() / 'build'}:/host_write_test:z", + "myimage:tagtorulethemall", + "rm", + "-f", + PurePosixPath("/host_write_test/container_write_test"), + ], + check=True, + stream_output=False, + ), + ] + ) + + +def test_user_mapping_write_file_path(mock_tools): + """The write test file path is as expected.""" + expected_path = Path.cwd() / "build" / "container_write_test" + assert Docker(mock_tools)._write_test_path() == expected_path + + +def test_user_mapping_write_file_exists(mock_tools, mock_write_test_path): + """Docker verification fails when the container write test file exists and cannot be + deleted.""" + # Mock the return values for Docker verification + mock_tools.subprocess.check_output.side_effect = [ + VALID_DOCKER_VERSION, + VALID_DOCKER_INFO, + VALID_BUILDX_VERSION, + ] + + # Mock failure for deleting an existing write test file + mock_write_test_path.unlink = MagicMock(side_effect=OSError("delete failed")) + + # Fails when file cannot be deleted + with pytest.raises( + BriefcaseCommandError, + match="file path used to determine how Docker is mapping users", + ): + Docker.verify(mock_tools) + + +def test_user_mapping_write_test_file_creation_fails(mock_tools, mock_write_test_path): + """Docker verification fails if the write test file cannot be written.""" + # Mock the return values for Docker verification + mock_tools.subprocess.check_output.side_effect = [ + VALID_DOCKER_VERSION, + VALID_DOCKER_INFO, + VALID_BUILDX_VERSION, + ] + + # Mock failure for deleting an existing write test file + mock_tools.subprocess.run.side_effect = subprocess.CalledProcessError( + returncode=1, cmd=["docker", "run", "..."] + ) + + # Fails when file cannot be deleted + with pytest.raises( + BriefcaseCommandError, + match="Unable to determine if Docker is mapping users", + ): + Docker.verify(mock_tools) + + +def test_user_mapping_write_test_file_cleanup_fails(mock_tools, mock_write_test_path): + """Docker verification fails if the write test file cannot be removed after the + test.""" + # Mock the return values for Docker verification + mock_tools.subprocess.check_output.side_effect = [ + VALID_DOCKER_VERSION, + VALID_DOCKER_INFO, + VALID_BUILDX_VERSION, + ] + + # Mock failure for deleting an existing write test file + mock_tools.subprocess.run.side_effect = [ + "container write test file creation succeeded", + subprocess.CalledProcessError(returncode=1, cmd=["docker", "run", "..."]), + ] + + # Fails when file cannot be deleted + with pytest.raises( + BriefcaseCommandError, + match="Unable to clean up from determining if Docker is mapping users", + ): + Docker.verify(mock_tools) + + +@pytest.mark.parametrize("file_owner_id, expected", [(1000, True), (0, False)]) +def test_user_mapping_setting( + mock_tools, + user_mapping_run_calls, + file_owner_id, + expected, +): + """If the write test file is not owned by root, user mapping is enabled, else + disabled.""" + # Mock the return values for Docker verification + mock_tools.subprocess.check_output.side_effect = [ + VALID_DOCKER_VERSION, + VALID_DOCKER_INFO, + VALID_BUILDX_VERSION, + ] + + stat_result = namedtuple("stat_result", "st_uid") + # Mock the os.stat call returning a file owned by file_owner_id + mock_tools.os.stat = MagicMock(return_value=stat_result(st_uid=file_owner_id)) + + docker = Docker.verify(mock_tools) + + # Docker user mapping inspection occurred + mock_tools.subprocess.run.assert_has_calls(user_mapping_run_calls) + + assert docker.is_users_mapped is expected diff --git a/tests/platforms/linux/appimage/test_create.py b/tests/platforms/linux/appimage/test_create.py index 90670d323..5430c7504 100644 --- a/tests/platforms/linux/appimage/test_create.py +++ b/tests/platforms/linux/appimage/test_create.py @@ -1,3 +1,5 @@ +from unittest.mock import MagicMock + import pytest from briefcase.console import Console, Log @@ -47,10 +49,7 @@ def test_unsupported_host_os_with_docker(create_command, host_os): @pytest.mark.parametrize("host_os", ["Darwin", "Windows", "WeirdOS"]) -def test_unsupported_host_os_without_docker( - create_command, - host_os, -): +def test_unsupported_host_os_without_docker(create_command, host_os): """Error raised for an unsupported OS when not using Docker.""" create_command.use_docker = False create_command.tools.host_os = host_os @@ -83,19 +82,21 @@ def test_finalize_nodocker(create_command, first_app_config, capsys): @pytest.mark.parametrize( - "manylinux, tag, host_arch, context", + "manylinux, tag, host_arch, is_users_mapped, context", [ # Fallback. - (None, None, "x86_64", {}), + (None, None, "x86_64", False, {"use_non_root_user": True}), # x86_64 architecture, all versions # Explicit tag ( "manylinux1", "2023-03-05-271004f", "x86_64", + True, { "manylinux_image": "manylinux1_x86_64:2023-03-05-271004f", "vendor_base": "centos", + "use_non_root_user": False, }, ), # Explicit latest @@ -103,31 +104,45 @@ def test_finalize_nodocker(create_command, first_app_config, capsys): "manylinux2010", "latest", "x86_64", - {"manylinux_image": "manylinux2010_x86_64:latest", "vendor_base": "centos"}, + False, + { + "manylinux_image": "manylinux2010_x86_64:latest", + "vendor_base": "centos", + "use_non_root_user": True, + }, ), # Implicit latest ( "manylinux2014", None, "x86_64", - {"manylinux_image": "manylinux2014_x86_64:latest", "vendor_base": "centos"}, + True, + { + "manylinux_image": "manylinux2014_x86_64:latest", + "vendor_base": "centos", + "use_non_root_user": False, + }, ), ( "manylinux_2_24", None, "x86_64", + True, { "manylinux_image": "manylinux_2_24_x86_64:latest", "vendor_base": "debian", + "use_non_root_user": False, }, ), ( "manylinux_2_28", None, "x86_64", + False, { "manylinux_image": "manylinux_2_28_x86_64:latest", "vendor_base": "almalinux", + "use_non_root_user": True, }, ), # non x86 architecture @@ -135,17 +150,29 @@ def test_finalize_nodocker(create_command, first_app_config, capsys): "manylinux2014", None, "aarch64", + True, { "manylinux_image": "manylinux2014_aarch64:latest", "vendor_base": "centos", + "use_non_root_user": False, }, ), ], ) def test_output_format_template_context( - create_command, first_app_config, manylinux, tag, host_arch, context + create_command, + first_app_config, + manylinux, + tag, + host_arch, + is_users_mapped, + context, ): """The template context reflects the manylinux name, tag and architecture.""" + # Mock Docker user mapping setting for `use_non_root_user` + create_command.tools.docker = MagicMock() + create_command.tools.docker.is_users_mapped = is_users_mapped + if manylinux: first_app_config.manylinux = manylinux if tag: @@ -161,3 +188,10 @@ def test_output_format_template_context_bad_tag(create_command, first_app_config first_app_config.manylinux = "unknown" with pytest.raises(BriefcaseConfigError, match=r"Unknown manylinux tag 'unknown'"): assert create_command.output_format_template_context(first_app_config) + + +def test_output_format_no_docker(create_command, first_app_config): + """If not using Docker, `use_non_root_user` default in template is used.""" + context = create_command.output_format_template_context(first_app_config) + + assert "use_non_root_user" not in context diff --git a/tests/platforms/linux/appimage/test_mixin.py b/tests/platforms/linux/appimage/test_mixin.py index ac4bd513e..ec491486f 100644 --- a/tests/platforms/linux/appimage/test_mixin.py +++ b/tests/platforms/linux/appimage/test_mixin.py @@ -113,6 +113,7 @@ def test_verify_linux_docker(create_command, tmp_path, first_app_config, monkeyp mock__version_compat = MagicMock(spec=Docker._version_compat) mock__user_access = MagicMock(spec=Docker._user_access) mock__buildx_installed = MagicMock(spec=Docker._buildx_installed) + mock__is_user_mapping_enabled = MagicMock(spec=Docker._is_user_mapping_enabled) monkeypatch.setattr( briefcase.platforms.linux.appimage.Docker, "_version_compat", @@ -128,6 +129,11 @@ def test_verify_linux_docker(create_command, tmp_path, first_app_config, monkeyp "_buildx_installed", mock__buildx_installed, ) + monkeypatch.setattr( + briefcase.platforms.linux.appimage.Docker, + "_is_user_mapping_enabled", + mock__is_user_mapping_enabled, + ) mock_docker_app_context_verify = MagicMock(spec=DockerAppContext.verify) monkeypatch.setattr( briefcase.platforms.linux.appimage.DockerAppContext, @@ -143,6 +149,7 @@ def test_verify_linux_docker(create_command, tmp_path, first_app_config, monkeyp mock__version_compat.assert_called_with(tools=create_command.tools) mock__user_access.assert_called_with(tools=create_command.tools) mock__buildx_installed.assert_called_with(tools=create_command.tools) + mock__is_user_mapping_enabled.assert_called_with(None) assert isinstance(create_command.tools.docker, Docker) mock_docker_app_context_verify.assert_called_with( tools=create_command.tools, @@ -181,6 +188,7 @@ def test_verify_non_linux_docker( mock__version_compat = MagicMock(spec=Docker._version_compat) mock__user_access = MagicMock(spec=Docker._user_access) mock__buildx_installed = MagicMock(spec=Docker._buildx_installed) + mock__is_user_mapping_enabled = MagicMock(spec=Docker._is_user_mapping_enabled) monkeypatch.setattr( briefcase.platforms.linux.appimage.Docker, "_version_compat", @@ -196,6 +204,11 @@ def test_verify_non_linux_docker( "_buildx_installed", mock__buildx_installed, ) + monkeypatch.setattr( + briefcase.platforms.linux.appimage.Docker, + "_is_user_mapping_enabled", + mock__is_user_mapping_enabled, + ) mock_docker_app_context_verify = MagicMock(spec=DockerAppContext.verify) monkeypatch.setattr( briefcase.platforms.linux.appimage.DockerAppContext, @@ -211,6 +224,7 @@ def test_verify_non_linux_docker( mock__version_compat.assert_called_with(tools=create_command.tools) mock__user_access.assert_called_with(tools=create_command.tools) mock__buildx_installed.assert_called_with(tools=create_command.tools) + mock__is_user_mapping_enabled.assert_called_with(None) assert isinstance(create_command.tools.docker, Docker) mock_docker_app_context_verify.assert_called_with( tools=create_command.tools, diff --git a/tests/platforms/linux/system/test_create.py b/tests/platforms/linux/system/test_create.py index 5475f3ecb..70930ba65 100644 --- a/tests/platforms/linux/system/test_create.py +++ b/tests/platforms/linux/system/test_create.py @@ -1,3 +1,5 @@ +from unittest.mock import MagicMock + import pytest from briefcase.exceptions import UnsupportedHostError @@ -65,8 +67,13 @@ def test_supported_host_os_without_docker(create_command): create_command.verify_host() -def test_output_format_template_context(create_command, first_app_config): - "The template context contains additional deb-specific properties" +@pytest.mark.parametrize("is_users_mapped", [True, False]) +def test_output_format_template_context( + create_command, + first_app_config, + is_users_mapped, +): + """The template context contains additional deb-specific properties.""" # Add some properties defined in config finalization first_app_config.python_version_tag = "3.X" first_app_config.target_image = "somevendor:surprising" @@ -75,6 +82,10 @@ def test_output_format_template_context(create_command, first_app_config): first_app_config.target_vendor_base = "basevendor" first_app_config.glibc_version = "2.42" + # Mock Docker user mapping setting for `use_non_root_user` + create_command.tools.docker = MagicMock() + create_command.tools.docker.is_users_mapped = is_users_mapped + # Add a long description first_app_config.long_description = "This is a long\ndescription." @@ -82,6 +93,28 @@ def test_output_format_template_context(create_command, first_app_config): context = create_command.output_format_template_context(first_app_config) # Context extras are what we expect + assert context == { + "format": "surprising", + "python_version": "3.X", + "docker_base_image": "somevendor:surprising", + "vendor_base": "basevendor", + "use_non_root_user": not is_users_mapped, + } + + +def test_output_format_template_context_no_docker(create_command, first_app_config): + """If not using Docker, `use_non_root_user` default in template is used.""" + # Add some properties defined in config finalization + first_app_config.python_version_tag = "3.X" + first_app_config.target_image = "somevendor:surprising" + first_app_config.target_vendor = "somevendor" + first_app_config.target_codename = "surprising" + first_app_config.target_vendor_base = "basevendor" + first_app_config.glibc_version = "2.42" + + context = create_command.output_format_template_context(first_app_config) + + # Context extras does not contain `use_non_root_user` assert context == { "format": "surprising", "python_version": "3.X", diff --git a/tests/platforms/linux/system/test_mixin__finalize_app_config.py b/tests/platforms/linux/system/test_mixin__finalize_app_config.py index ecb5f8b4d..8c84afe44 100644 --- a/tests/platforms/linux/system/test_mixin__finalize_app_config.py +++ b/tests/platforms/linux/system/test_mixin__finalize_app_config.py @@ -30,9 +30,6 @@ def test_docker(create_command, first_app_config): # Finalize the app config create_command.finalize_app_config(first_app_config) - # The base image has been prepared - create_command.tools.docker.prepare.assert_called_once_with("somevendor:surprising") - # The app's image, vendor and codename have been constructed from the target image assert first_app_config.target_image == "somevendor:surprising" assert first_app_config.target_vendor == "somevendor" diff --git a/tests/platforms/linux/system/test_mixin__verify.py b/tests/platforms/linux/system/test_mixin__verify.py index 88fb0ab3d..3ad4dfb16 100644 --- a/tests/platforms/linux/system/test_mixin__verify.py +++ b/tests/platforms/linux/system/test_mixin__verify.py @@ -52,6 +52,7 @@ def test_linux_docker(create_command, tmp_path, first_app_config, monkeypatch): mock__version_compat = MagicMock(spec=Docker._version_compat) mock__user_access = MagicMock(spec=Docker._user_access) mock__buildx_installed = MagicMock(spec=Docker._buildx_installed) + mock__is_user_mapping_enabled = MagicMock(spec=Docker._is_user_mapping_enabled) monkeypatch.setattr( briefcase.platforms.linux.system.Docker, "_version_compat", @@ -67,6 +68,11 @@ def test_linux_docker(create_command, tmp_path, first_app_config, monkeypatch): "_buildx_installed", mock__buildx_installed, ) + monkeypatch.setattr( + briefcase.platforms.linux.system.Docker, + "_is_user_mapping_enabled", + mock__is_user_mapping_enabled, + ) mock_docker_app_context_verify = MagicMock(spec=DockerAppContext.verify) monkeypatch.setattr( briefcase.platforms.linux.system.DockerAppContext, @@ -83,6 +89,7 @@ def test_linux_docker(create_command, tmp_path, first_app_config, monkeypatch): mock__version_compat.assert_called_with(tools=create_command.tools) mock__user_access.assert_called_with(tools=create_command.tools) mock__buildx_installed.assert_called_with(tools=create_command.tools) + mock__is_user_mapping_enabled.assert_called_with("somevendor:surprising") assert isinstance(create_command.tools.docker, Docker) mock_docker_app_context_verify.assert_called_with( tools=create_command.tools, @@ -132,6 +139,7 @@ def test_non_linux_docker(create_command, first_app_config, monkeypatch, tmp_pat mock__version_compat = MagicMock(spec=Docker._version_compat) mock__user_access = MagicMock(spec=Docker._user_access) mock__buildx_installed = MagicMock(spec=Docker._buildx_installed) + mock__is_user_mapping_enabled = MagicMock(spec=Docker._is_user_mapping_enabled) monkeypatch.setattr( briefcase.platforms.linux.system.Docker, "_version_compat", @@ -147,6 +155,11 @@ def test_non_linux_docker(create_command, first_app_config, monkeypatch, tmp_pat "_buildx_installed", mock__buildx_installed, ) + monkeypatch.setattr( + briefcase.platforms.linux.system.Docker, + "_is_user_mapping_enabled", + mock__is_user_mapping_enabled, + ) mock_docker_app_context_verify = MagicMock(spec=DockerAppContext.verify) monkeypatch.setattr( briefcase.platforms.linux.system.DockerAppContext, @@ -163,6 +176,7 @@ def test_non_linux_docker(create_command, first_app_config, monkeypatch, tmp_pat mock__version_compat.assert_called_with(tools=create_command.tools) mock__user_access.assert_called_with(tools=create_command.tools) mock__buildx_installed.assert_called_with(tools=create_command.tools) + mock__is_user_mapping_enabled.assert_called_with("somevendor:surprising") assert isinstance(create_command.tools.docker, Docker) mock_docker_app_context_verify.assert_called_with( tools=create_command.tools, From b31e51fae0f23d4fb311319336dd7ff3a8a5c128 Mon Sep 17 00:00:00 2001 From: Russell Martin Date: Sun, 9 Jul 2023 19:52:25 -0400 Subject: [PATCH 4/8] Disable building Arch packages when Docker is mapping users - Arch's `makepkg` cannot be run as root --- src/briefcase/platforms/linux/system.py | 15 +++++ .../system/test_mixin__finalize_app_config.py | 59 +++++++++++++++++-- 2 files changed, 70 insertions(+), 4 deletions(-) diff --git a/src/briefcase/platforms/linux/system.py b/src/briefcase/platforms/linux/system.py index 116e4a208..dd3842c7b 100644 --- a/src/briefcase/platforms/linux/system.py +++ b/src/briefcase/platforms/linux/system.py @@ -163,6 +163,21 @@ def finalize_app_config(self, app: AppConfig): if not self.use_docker: app.target_image = f"{app.target_vendor}:{app.target_codename}" + else: + if app.target_vendor_base == ARCH and self.tools.docker.is_users_mapped: + raise BriefcaseCommandError( + """\ +Briefcase cannot use this Docker installation to target Arch Linux since the +tools to build packages for Arch cannot be run as root. + +The Docker available to Briefcase requires the use of the root user in +containers to maintain accurate file permissions of the build artefacts. + +This most likely means you're using Docker Desktop or rootless Docker. + +Install Docker Engine and try again or run Briefcase on an Arch host system. +""" + ) # Merge target-specific configuration items into the app config This # means: diff --git a/tests/platforms/linux/system/test_mixin__finalize_app_config.py b/tests/platforms/linux/system/test_mixin__finalize_app_config.py index 8c84afe44..6e7958c9e 100644 --- a/tests/platforms/linux/system/test_mixin__finalize_app_config.py +++ b/tests/platforms/linux/system/test_mixin__finalize_app_config.py @@ -12,7 +12,7 @@ def test_docker(create_command, first_app_config): - "An app can be finalized inside docker" + """An app can be finalized inside docker.""" # Build the app on a specific target create_command.target_image = "somevendor:surprising" create_command.tools.docker = MagicMock() @@ -41,7 +41,7 @@ def test_docker(create_command, first_app_config): def test_nodocker(create_command, first_app_config, tmp_path): - "An app can be finalized without docker" + """An app can be finalized without docker.""" # Build the app without docker create_command.target_image = None create_command.target_glibc_version = MagicMock(return_value="2.42") @@ -77,7 +77,7 @@ def test_nodocker(create_command, first_app_config, tmp_path): def test_nodocker_non_freedesktop(create_command, first_app_config, tmp_path): - "If the system isn't FreeDesktop compliant raise an error" + """If the system isn't FreeDesktop compliant raise an error.""" # Build the app without docker create_command.target_image = None create_command.target_glibc_version = MagicMock(return_value="2.42") @@ -100,6 +100,57 @@ def test_nodocker_non_freedesktop(create_command, first_app_config, tmp_path): create_command.finalize_app_config(first_app_config) +def test_docker_arch_with_user_mapping(create_command, first_app_config, tmp_path): + """If Docker is mapping users and the host system is Arch, an error is raised.""" + # Build the app on a specific target + create_command.target_image = "somearch:surprising" + create_command.tools.docker = MagicMock() + create_command.tools.docker.is_users_mapped = True + create_command.target_glibc_version = MagicMock(return_value="2.42") + + # Mock a minimal response for an Arch /etc/os-release + create_command.tools.docker.check_output.return_value = "\n".join( + [ + "ID=arch", + "VERSION_ID=20230625.0.160368", + ] + ) + + # Finalize the app config + with pytest.raises( + BriefcaseCommandError, + match="Briefcase cannot use this Docker installation", + ): + create_command.finalize_app_config(first_app_config) + + +def test_docker_arch_without_user_mapping(create_command, first_app_config, tmp_path): + """If Docker is *not* mapping users and the host system is Arch, an error is not + raised.""" + # Build the app on a specific target + create_command.target_image = "somearch:surprising" + create_command.tools.docker = MagicMock() + create_command.tools.docker.is_users_mapped = False + create_command.target_glibc_version = MagicMock(return_value="2.42") + + # Mock a minimal response for an Arch /etc/os-release + create_command.tools.docker.check_output.return_value = "\n".join( + [ + "ID=arch", + "VERSION_ID=20230625.0.160368", + ] + ) + + # Finalize the app config + create_command.finalize_app_config(first_app_config) + + # The app's image, vendor and codename have been constructed from the target image + assert first_app_config.target_image == "somearch:surprising" + assert first_app_config.target_vendor == "arch" + assert first_app_config.target_codename == "20230625" + assert first_app_config.target_vendor_base == "arch" + + def test_properties(create_command, first_app_config): """The final app config is the result of merging target properties, plus other derived properties.""" @@ -414,7 +465,7 @@ def test_properties_no_version(create_command, first_app_config): def test_passive_mixin(first_app_config, tmp_path): - "An app using the PassiveMixin can be finalized" + """An app using the PassiveMixin can be finalized.""" run_command = LinuxSystemRunCommand( logger=Log(), console=Console(), From 6ab39d1f600e0ed9548bc059ff26c6c19a6863bd Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Mon, 10 Jul 2023 11:25:05 +0800 Subject: [PATCH 5/8] Tweak the pluralization of a variable. --- src/briefcase/integrations/docker.py | 6 +++--- src/briefcase/platforms/linux/appimage.py | 2 +- src/briefcase/platforms/linux/system.py | 4 ++-- tests/integrations/docker/test_Docker__verify.py | 2 +- tests/platforms/linux/appimage/test_create.py | 6 +++--- tests/platforms/linux/system/test_create.py | 8 ++++---- .../linux/system/test_mixin__finalize_app_config.py | 4 ++-- 7 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/briefcase/integrations/docker.py b/src/briefcase/integrations/docker.py index 9ba865ddf..d1b695387 100644 --- a/src/briefcase/integrations/docker.py +++ b/src/briefcase/integrations/docker.py @@ -121,7 +121,7 @@ def __init__(self, tools: ToolCache, image_tag: str | None = None): at all bound to the instance. """ super().__init__(tools=tools) - self.is_users_mapped = self._is_user_mapping_enabled(image_tag) + self.is_user_mapped = self._is_user_mapping_enabled(image_tag) @classmethod def verify_install( @@ -308,7 +308,7 @@ def _is_user_mapping_enabled(self, image_tag: str | None = None) -> bool: ) from e # if the file is not owned by `root`, then Docker is mapping usernames - is_users_mapped = 0 != self.tools.os.stat(host_write_test_path).st_uid + is_user_mapped = 0 != self.tools.os.stat(host_write_test_path).st_uid try: self.tools.subprocess.run( @@ -321,7 +321,7 @@ def _is_user_mapping_enabled(self, image_tag: str | None = None) -> bool: "Unable to clean up from determining if Docker is mapping users" ) from e - return is_users_mapped + return is_user_mapped def cache_image(self, image_tag: str): """Ensures an image is available and cached locally. diff --git a/src/briefcase/platforms/linux/appimage.py b/src/briefcase/platforms/linux/appimage.py index 555f297ce..f9916a5f4 100644 --- a/src/briefcase/platforms/linux/appimage.py +++ b/src/briefcase/platforms/linux/appimage.py @@ -170,7 +170,7 @@ def output_format_template_context(self, app: AppConfig): # Use the non-root user if Docker is not mapping usernames try: - context["use_non_root_user"] = not self.tools.docker.is_users_mapped + context["use_non_root_user"] = not self.tools.docker.is_user_mapped except AttributeError: pass # ignore if not using Docker diff --git a/src/briefcase/platforms/linux/system.py b/src/briefcase/platforms/linux/system.py index dd3842c7b..7d9a003ab 100644 --- a/src/briefcase/platforms/linux/system.py +++ b/src/briefcase/platforms/linux/system.py @@ -164,7 +164,7 @@ def finalize_app_config(self, app: AppConfig): if not self.use_docker: app.target_image = f"{app.target_vendor}:{app.target_codename}" else: - if app.target_vendor_base == ARCH and self.tools.docker.is_users_mapped: + if app.target_vendor_base == ARCH and self.tools.docker.is_user_mapped: raise BriefcaseCommandError( """\ Briefcase cannot use this Docker installation to target Arch Linux since the @@ -575,7 +575,7 @@ def output_format_template_context(self, app: AppConfig): # Use the non-root user if Docker is not mapping usernames try: - context["use_non_root_user"] = not self.tools.docker.is_users_mapped + context["use_non_root_user"] = not self.tools.docker.is_user_mapped except AttributeError: pass # ignore if not using Docker diff --git a/tests/integrations/docker/test_Docker__verify.py b/tests/integrations/docker/test_Docker__verify.py index 036a4bedf..1f1082fff 100644 --- a/tests/integrations/docker/test_Docker__verify.py +++ b/tests/integrations/docker/test_Docker__verify.py @@ -417,4 +417,4 @@ def test_user_mapping_setting( # Docker user mapping inspection occurred mock_tools.subprocess.run.assert_has_calls(user_mapping_run_calls) - assert docker.is_users_mapped is expected + assert docker.is_user_mapped is expected diff --git a/tests/platforms/linux/appimage/test_create.py b/tests/platforms/linux/appimage/test_create.py index 5430c7504..94d40b93a 100644 --- a/tests/platforms/linux/appimage/test_create.py +++ b/tests/platforms/linux/appimage/test_create.py @@ -82,7 +82,7 @@ def test_finalize_nodocker(create_command, first_app_config, capsys): @pytest.mark.parametrize( - "manylinux, tag, host_arch, is_users_mapped, context", + "manylinux, tag, host_arch, is_user_mapped, context", [ # Fallback. (None, None, "x86_64", False, {"use_non_root_user": True}), @@ -165,13 +165,13 @@ def test_output_format_template_context( manylinux, tag, host_arch, - is_users_mapped, + is_user_mapped, context, ): """The template context reflects the manylinux name, tag and architecture.""" # Mock Docker user mapping setting for `use_non_root_user` create_command.tools.docker = MagicMock() - create_command.tools.docker.is_users_mapped = is_users_mapped + create_command.tools.docker.is_user_mapped = is_user_mapped if manylinux: first_app_config.manylinux = manylinux diff --git a/tests/platforms/linux/system/test_create.py b/tests/platforms/linux/system/test_create.py index 70930ba65..db0eb50c5 100644 --- a/tests/platforms/linux/system/test_create.py +++ b/tests/platforms/linux/system/test_create.py @@ -67,11 +67,11 @@ def test_supported_host_os_without_docker(create_command): create_command.verify_host() -@pytest.mark.parametrize("is_users_mapped", [True, False]) +@pytest.mark.parametrize("is_user_mapped", [True, False]) def test_output_format_template_context( create_command, first_app_config, - is_users_mapped, + is_user_mapped, ): """The template context contains additional deb-specific properties.""" # Add some properties defined in config finalization @@ -84,7 +84,7 @@ def test_output_format_template_context( # Mock Docker user mapping setting for `use_non_root_user` create_command.tools.docker = MagicMock() - create_command.tools.docker.is_users_mapped = is_users_mapped + create_command.tools.docker.is_user_mapped = is_user_mapped # Add a long description first_app_config.long_description = "This is a long\ndescription." @@ -98,7 +98,7 @@ def test_output_format_template_context( "python_version": "3.X", "docker_base_image": "somevendor:surprising", "vendor_base": "basevendor", - "use_non_root_user": not is_users_mapped, + "use_non_root_user": not is_user_mapped, } diff --git a/tests/platforms/linux/system/test_mixin__finalize_app_config.py b/tests/platforms/linux/system/test_mixin__finalize_app_config.py index 6e7958c9e..7127776f8 100644 --- a/tests/platforms/linux/system/test_mixin__finalize_app_config.py +++ b/tests/platforms/linux/system/test_mixin__finalize_app_config.py @@ -105,7 +105,7 @@ def test_docker_arch_with_user_mapping(create_command, first_app_config, tmp_pat # Build the app on a specific target create_command.target_image = "somearch:surprising" create_command.tools.docker = MagicMock() - create_command.tools.docker.is_users_mapped = True + create_command.tools.docker.is_user_mapped = True create_command.target_glibc_version = MagicMock(return_value="2.42") # Mock a minimal response for an Arch /etc/os-release @@ -130,7 +130,7 @@ def test_docker_arch_without_user_mapping(create_command, first_app_config, tmp_ # Build the app on a specific target create_command.target_image = "somearch:surprising" create_command.tools.docker = MagicMock() - create_command.tools.docker.is_users_mapped = False + create_command.tools.docker.is_user_mapped = False create_command.target_glibc_version = MagicMock(return_value="2.42") # Mock a minimal response for an Arch /etc/os-release From 76ceb1b03cfbad8dd648e9c0eb0c1451c8a3fd7b Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Mon, 10 Jul 2023 11:26:21 +0800 Subject: [PATCH 6/8] Bump the required cookiecutter version to v2.2; Refs #1347. --- setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index f294764aa..42fae19a5 100644 --- a/setup.cfg +++ b/setup.cfg @@ -74,7 +74,7 @@ install_requires = # cause API breakages). If the package uses calver, we don't pin the upper # version, as the upper version provides no basis for determining API # stability. - cookiecutter >= 2.1, < 3.0 + cookiecutter >= 2.2, < 3.0 dmgbuild >= 1.6, < 2.0; sys_platform == "darwin" GitPython >= 3.1, < 4.0 platformdirs >= 2.6, < 4.0 From 36772300c50f2729575d31f96a454f5da17a3132 Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Mon, 10 Jul 2023 12:36:49 +0800 Subject: [PATCH 7/8] Allow the step-down user on macOS. --- src/briefcase/platforms/linux/system.py | 18 +++++++++-- tests/platforms/linux/system/test_create.py | 20 ++++++++++-- .../system/test_mixin__finalize_app_config.py | 32 +++++++++++++++++++ 3 files changed, 65 insertions(+), 5 deletions(-) diff --git a/src/briefcase/platforms/linux/system.py b/src/briefcase/platforms/linux/system.py index 7d9a003ab..e113cd270 100644 --- a/src/briefcase/platforms/linux/system.py +++ b/src/briefcase/platforms/linux/system.py @@ -164,7 +164,15 @@ def finalize_app_config(self, app: AppConfig): if not self.use_docker: app.target_image = f"{app.target_vendor}:{app.target_codename}" else: - if app.target_vendor_base == ARCH and self.tools.docker.is_user_mapped: + # If we're building for Arch, and Docker does user mapping, we can't build, + # because Arch won't let makepkg run as root. Docker on macOS *does* map the + # user, but introducing a step-down user doesn't alter behavior, so we can + # allow it. + if ( + app.target_vendor_base == ARCH + and self.tools.docker.is_user_mapped + and self.tools.host_os != "Darwin" + ): raise BriefcaseCommandError( """\ Briefcase cannot use this Docker installation to target Arch Linux since the @@ -573,9 +581,13 @@ def output_format_template_context(self, app: AppConfig): # Add the vendor base context["vendor_base"] = app.target_vendor_base - # Use the non-root user if Docker is not mapping usernames + # Use the non-root user if Docker is not mapping usernames. Also use a non-root + # user if we're on macOS; user mapping doesn't alter Docker operation, but some + # packaging tools (e.g., Arch's makepkg) don't like running as root. try: - context["use_non_root_user"] = not self.tools.docker.is_user_mapped + context["use_non_root_user"] = self.use_docker and ( + self.tools.host_os == "Darwin" or not self.tools.docker.is_user_mapped + ) except AttributeError: pass # ignore if not using Docker diff --git a/tests/platforms/linux/system/test_create.py b/tests/platforms/linux/system/test_create.py index db0eb50c5..e5e5cebbc 100644 --- a/tests/platforms/linux/system/test_create.py +++ b/tests/platforms/linux/system/test_create.py @@ -67,11 +67,21 @@ def test_supported_host_os_without_docker(create_command): create_command.verify_host() -@pytest.mark.parametrize("is_user_mapped", [True, False]) +@pytest.mark.parametrize( + "is_user_mapped, host_os, use_non_root", + [ + (False, "Darwin", True), + (True, "Darwin", True), + (False, "Linux", True), + (True, "Linux", False), + ], +) def test_output_format_template_context( create_command, first_app_config, is_user_mapped, + host_os, + use_non_root, ): """The template context contains additional deb-specific properties.""" # Add some properties defined in config finalization @@ -82,6 +92,12 @@ def test_output_format_template_context( first_app_config.target_vendor_base = "basevendor" first_app_config.glibc_version = "2.42" + # Mock the host OS + create_command.tools.host_os = host_os + + # Mock a target Docker image + create_command.target_image = "somearch:surprising" + # Mock Docker user mapping setting for `use_non_root_user` create_command.tools.docker = MagicMock() create_command.tools.docker.is_user_mapped = is_user_mapped @@ -98,7 +114,7 @@ def test_output_format_template_context( "python_version": "3.X", "docker_base_image": "somevendor:surprising", "vendor_base": "basevendor", - "use_non_root_user": not is_user_mapped, + "use_non_root_user": use_non_root, } diff --git a/tests/platforms/linux/system/test_mixin__finalize_app_config.py b/tests/platforms/linux/system/test_mixin__finalize_app_config.py index 7127776f8..abca5bc38 100644 --- a/tests/platforms/linux/system/test_mixin__finalize_app_config.py +++ b/tests/platforms/linux/system/test_mixin__finalize_app_config.py @@ -104,6 +104,7 @@ def test_docker_arch_with_user_mapping(create_command, first_app_config, tmp_pat """If Docker is mapping users and the host system is Arch, an error is raised.""" # Build the app on a specific target create_command.target_image = "somearch:surprising" + create_command.tools.host_os = "Linux" create_command.tools.docker = MagicMock() create_command.tools.docker.is_user_mapped = True create_command.target_glibc_version = MagicMock(return_value="2.42") @@ -124,11 +125,42 @@ def test_docker_arch_with_user_mapping(create_command, first_app_config, tmp_pat create_command.finalize_app_config(first_app_config) +def test_docker_arch_with_user_mapping_macOS( + create_command, first_app_config, tmp_path +): + """If we're on macOS, and the host system is Arch, we can finalize even though macOS + does user mapping.""" + # Build the app on a specific target + create_command.target_image = "somearch:surprising" + create_command.tools.host_os = "Darwin" + create_command.tools.docker = MagicMock() + create_command.tools.docker.is_user_mapped = True + create_command.target_glibc_version = MagicMock(return_value="2.42") + + # Mock a minimal response for an Arch /etc/os-release + create_command.tools.docker.check_output.return_value = "\n".join( + [ + "ID=arch", + "VERSION_ID=20230625.0.160368", + ] + ) + + # Finalize the app config + create_command.finalize_app_config(first_app_config) + + # The app's image, vendor and codename have been constructed from the target image + assert first_app_config.target_image == "somearch:surprising" + assert first_app_config.target_vendor == "arch" + assert first_app_config.target_codename == "20230625" + assert first_app_config.target_vendor_base == "arch" + + def test_docker_arch_without_user_mapping(create_command, first_app_config, tmp_path): """If Docker is *not* mapping users and the host system is Arch, an error is not raised.""" # Build the app on a specific target create_command.target_image = "somearch:surprising" + create_command.tools.host_os = "Linux" create_command.tools.docker = MagicMock() create_command.tools.docker.is_user_mapped = False create_command.target_glibc_version = MagicMock(return_value="2.42") From 38ab7999005061cd92f13101898015e9687955a5 Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Mon, 10 Jul 2023 12:54:40 +0800 Subject: [PATCH 8/8] Fall back to template default if we don't know anything about Docker --- src/briefcase/platforms/linux/system.py | 8 ++++++-- tests/platforms/linux/system/test_create.py | 3 +++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/briefcase/platforms/linux/system.py b/src/briefcase/platforms/linux/system.py index e113cd270..829f4dabd 100644 --- a/src/briefcase/platforms/linux/system.py +++ b/src/briefcase/platforms/linux/system.py @@ -583,9 +583,13 @@ def output_format_template_context(self, app: AppConfig): # Use the non-root user if Docker is not mapping usernames. Also use a non-root # user if we're on macOS; user mapping doesn't alter Docker operation, but some - # packaging tools (e.g., Arch's makepkg) don't like running as root. + # packaging tools (e.g., Arch's makepkg) don't like running as root. If we're + # not using Docker, this will fall back to the template default, which should be + # enabling the root user. This might cause problems later, but it's part of a + # much bigger "does the project need to be updated in light of configuration + # changes" problem. try: - context["use_non_root_user"] = self.use_docker and ( + context["use_non_root_user"] = ( self.tools.host_os == "Darwin" or not self.tools.docker.is_user_mapped ) except AttributeError: diff --git a/tests/platforms/linux/system/test_create.py b/tests/platforms/linux/system/test_create.py index e5e5cebbc..2039dcaed 100644 --- a/tests/platforms/linux/system/test_create.py +++ b/tests/platforms/linux/system/test_create.py @@ -120,6 +120,9 @@ def test_output_format_template_context( def test_output_format_template_context_no_docker(create_command, first_app_config): """If not using Docker, `use_non_root_user` default in template is used.""" + # Mock the host to Linux to avoid flagging any "always use non-root user on macOS" logic. + create_command.tools.host_os = "Linux" + # Add some properties defined in config finalization first_app_config.python_version_tag = "3.X" first_app_config.target_image = "somevendor:surprising"