diff --git a/src/containerapp/HISTORY.rst b/src/containerapp/HISTORY.rst index 9616545fbd6..c392f71167e 100644 --- a/src/containerapp/HISTORY.rst +++ b/src/containerapp/HISTORY.rst @@ -4,6 +4,7 @@ Release History =============== upcoming ++++++ +* Resolve Docker with Azure CLI's safe executable lookup before invoking it from local build and patch flows 1.3.0b5 ++++++ diff --git a/src/containerapp/azext_containerapp/_up_utils.py b/src/containerapp/azext_containerapp/_up_utils.py index 4ff360213dc..eb39deb2839 100644 --- a/src/containerapp/azext_containerapp/_up_utils.py +++ b/src/containerapp/azext_containerapp/_up_utils.py @@ -21,6 +21,7 @@ CLIError, CLIInternalError, ) from azure.cli.core.commands.client_factory import get_subscription_id +from azure.cli.core.util import get_safe_executable_path from azure.cli.command_modules.appservice._create_util import ( check_resource_group_exists, ) @@ -534,7 +535,11 @@ def _docker_push_to_container_registry(self, image_name, forced_acr_login=False) from azure.cli.command_modules.acr.custom import acr_login from azure.cli.core.profiles import ResourceType - command = ['docker', 'push', image_name] + docker_exec_path = get_safe_executable_path("docker") + if not docker_exec_path: + raise CLIError("Docker executable could not be found.") + + command = [docker_exec_path, 'push', image_name] logger.debug(f"Calling '{' '.join(command)}'") logger.warning(f"Built image {image_name} locally using buildpacks, attempting to push to registry...") try: @@ -547,14 +552,26 @@ def _docker_push_to_container_registry(self, image_name, forced_acr_login=False) logger.warning(f"The current user isn't authenticated to the {self.acr.name} ACR instance. Triggering an ACR login and retrying to push the image...") # Logic to login to ACR task_command_kwargs = {"resource_type": ResourceType.MGMT_CONTAINERREGISTRY, 'operation_group': 'webhooks'} + missing = object() old_command_kwargs = {} for key in task_command_kwargs: # pylint: disable=consider-using-dict-items - old_command_kwargs[key] = self.cmd.command_kwargs.get(key) + old_command_kwargs[key] = self.cmd.command_kwargs.get(key, missing) self.cmd.command_kwargs[key] = task_command_kwargs[key] - if self.acr and self.acr.name is not None: - acr_login(self.cmd, self.acr.name) - for k, v in old_command_kwargs.items(): - self.cmd.command_kwargs[k] = v + old_docker_command = os.environ.get("DOCKER_COMMAND") + try: + os.environ["DOCKER_COMMAND"] = docker_exec_path + if self.acr and self.acr.name is not None: + acr_login(self.cmd, self.acr.name) + finally: + if old_docker_command is None: + os.environ.pop("DOCKER_COMMAND", None) + else: + os.environ["DOCKER_COMMAND"] = old_docker_command + for k, v in old_command_kwargs.items(): + if v is missing: + self.cmd.command_kwargs.pop(k, None) + else: + self.cmd.command_kwargs[k] = v self._docker_push_to_container_registry(image_name, True) else: diff --git a/src/containerapp/azext_containerapp/_utils.py b/src/containerapp/azext_containerapp/_utils.py index b7015e9d4ac..1f66cbbd88b 100644 --- a/src/containerapp/azext_containerapp/_utils.py +++ b/src/containerapp/azext_containerapp/_utils.py @@ -13,7 +13,6 @@ import hashlib import re import requests -import shutil import packaging.version as SemVer import random @@ -28,6 +27,7 @@ from azure.cli.core._profile import Profile from azure.cli.core.azclierror import (ValidationError, ResourceNotFoundError, CLIError, InvalidArgumentValueError) from azure.cli.core.commands.client_factory import get_mgmt_service_client, get_subscription_id +from azure.cli.core.util import get_safe_executable_path from azure.cli.command_modules.containerapp._utils import is_registry_msi_system from azure.mgmt.core.tools import parse_resource_id, is_valid_resource_id from ._utils_validation import validate_image_name # noqa: F401 @@ -264,13 +264,17 @@ def list_environment_locations(cmd, resource_type=MANAGED_ENVIRONMENT_RESOURCE_T def is_docker_running(): + docker_exec_path = get_safe_executable_path("docker") + if not docker_exec_path: + return False + try: # Run a simple 'docker stats --no-stream' command to check if the Docker daemon is running - command = ["docker", "stats", "--no-stream"] + command = [docker_exec_path, "stats", "--no-stream"] with subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE) as process: _, _ = process.communicate() return process.returncode == 0 - except Exception: + except (OSError, subprocess.SubprocessError): return False diff --git a/src/containerapp/azext_containerapp/custom.py b/src/containerapp/azext_containerapp/custom.py index 183b9cb51e4..e0689d2c758 100644 --- a/src/containerapp/azext_containerapp/custom.py +++ b/src/containerapp/azext_containerapp/custom.py @@ -25,6 +25,7 @@ ResourceNotFoundError, ArgumentUsageError, MutuallyExclusiveArgumentError) +from azure.cli.core.util import get_safe_executable_path from azure.cli.core.commands.client_factory import get_subscription_id from azure.cli.command_modules.containerapp.custom import set_secrets, open_containerapp_in_browser from azure.cli.command_modules.containerapp.containerapp_job_decorator import ContainerAppJobDecorator @@ -1898,12 +1899,15 @@ def patch_cli_call(cmd, resource_group, container_app_name, container_name, targ logger.warning("Applying patch for container app: " + container_app_name + " container: " + container_name) validate_image_name(target_image_name) validate_image_name(new_run_image) + docker_exec_path = get_safe_executable_path("docker") + if not docker_exec_path: + raise CLIError("Docker executable could not be found.") subprocess.run([pack_exec_path, "rebase", "-q", target_image_name, "--run-image", new_run_image, "--force"], check=True) new_target_image_name = target_image_name.split(":")[0] + ":" + new_run_image.split(":")[1] validate_image_name(new_target_image_name) - subprocess.run(["docker", "tag", target_image_name, new_target_image_name], check=True) + subprocess.run([docker_exec_path, "tag", target_image_name, new_target_image_name], check=True) logger.debug(f"Publishing {new_target_image_name} to registry...") - subprocess.run(["docker", "push", "-q", new_target_image_name], check=True) + subprocess.run([docker_exec_path, "push", "-q", new_target_image_name], check=True) logger.warning("Patch applied and published successfully.\nNew image: " + new_target_image_name) except Exception: logger.error("Error: Failed to apply patch and publish. Check if registry is logged in and has write access.") diff --git a/src/containerapp/azext_containerapp/tests/latest/test_containerapp_patch_unit.py b/src/containerapp/azext_containerapp/tests/latest/test_containerapp_patch_unit.py index 691d375dfd2..7f9f6fa81ca 100644 --- a/src/containerapp/azext_containerapp/tests/latest/test_containerapp_patch_unit.py +++ b/src/containerapp/azext_containerapp/tests/latest/test_containerapp_patch_unit.py @@ -8,7 +8,7 @@ import unittest from unittest import mock -from azure.cli.core.azclierror import ValidationError +from azure.cli.core.azclierror import CLIError, ValidationError from azext_containerapp._utils_validation import validate_image_name @@ -82,7 +82,9 @@ def test_rejects_newline(self): "subprocess": __import__("subprocess"), "json": __import__("json"), "logger": mock.MagicMock(), + "CLIError": CLIError, "validate_image_name": validate_image_name, + "get_safe_executable_path": lambda _: "/usr/local/bin/docker", "update_containerapp": mock.MagicMock(return_value={}), } @@ -207,16 +209,36 @@ def test_calls_subprocess_run_with_list_args(self): tag_call = mock_subprocess.run.call_args_list[1] self.assertEqual( tag_call[0][0], - ["docker", "tag", "myregistry.azurecr.io/myapp:old-tag", + ["/usr/local/bin/docker", "tag", "myregistry.azurecr.io/myapp:old-tag", "myregistry.azurecr.io/myapp:7.1.0-debian-bullseye"] ) push_call = mock_subprocess.run.call_args_list[2] self.assertEqual( push_call[0][0], - ["docker", "push", "-q", "myregistry.azurecr.io/myapp:7.1.0-debian-bullseye"] + ["/usr/local/bin/docker", "push", "-q", "myregistry.azurecr.io/myapp:7.1.0-debian-bullseye"] ) + def test_does_not_run_when_docker_is_not_found(self): + mock_subprocess = mock.MagicMock() + + with mock.patch.dict( + _FUNC_GLOBALS, + {"subprocess": mock_subprocess, "get_safe_executable_path": lambda _: None} + ): + fn = _extract_function("patch_cli_call") + with self.assertRaises(CLIError): + fn( + cmd=mock.MagicMock(), + resource_group="myrg", + container_app_name="myapp", + container_name="mycontainer", + target_image_name="myregistry.azurecr.io/myapp:old-tag", + new_run_image="mcr.microsoft.com/oryx/dotnetcore:7.1.0-debian-bullseye", + pack_exec_path="/usr/local/bin/pack", + ) + mock_subprocess.run.assert_not_called() + def test_rejects_malicious_target_image_name(self): mock_subprocess = mock.MagicMock() diff --git a/src/containerapp/azext_containerapp/tests/latest/test_utils.py b/src/containerapp/azext_containerapp/tests/latest/test_utils.py index 3c0bc35d192..ea4d65dc1d8 100644 --- a/src/containerapp/azext_containerapp/tests/latest/test_utils.py +++ b/src/containerapp/azext_containerapp/tests/latest/test_utils.py @@ -3,9 +3,173 @@ # Licensed under the MIT License. See License.txt in the project root for license information. # -------------------------------------------------------------------------------------------- +import os import unittest -from azure.cli.core.azclierror import ValidationError -from azext_containerapp._utils import validate_environment_mode_and_workload_profiles_compatible +from unittest import mock + +from azure.cli.core.azclierror import CLIError, ValidationError +from azext_containerapp._utils import ( + is_docker_running, + validate_environment_mode_and_workload_profiles_compatible, +) + + +def _extract_up_utils_method(class_name, method_name): + import ast + import textwrap + + src_path = os.path.normpath(os.path.join(os.path.dirname(__file__), "..", "..", "_up_utils.py")) + with open(src_path, "r", encoding="utf-8") as file: + source = file.read() + tree = ast.parse(source, filename=src_path) + + for node in tree.body: + if isinstance(node, ast.ClassDef) and node.name == class_name: + for child in node.body: + if isinstance(child, ast.FunctionDef) and child.name == method_name: + namespace = { + "CLIError": CLIError, + "get_safe_executable_path": mock.MagicMock(), + "logger": mock.MagicMock(), + "os": os, + "subprocess": __import__("subprocess"), + } + method_source = ast.get_source_segment(source, child) + exec(compile(textwrap.dedent(method_source), src_path, "exec"), namespace) # pylint: disable=exec-used + return namespace[method_name] + + raise ValueError(f"Method {class_name}.{method_name} not found in {src_path}") + + +class TestDockerExecutablePath(unittest.TestCase): + def test_is_docker_running_uses_resolved_path(self): + docker_path = os.path.join(os.path.abspath(os.sep), "trusted", "docker.exe") + process = mock.MagicMock() + process.returncode = 0 + process.communicate.return_value = (b"", b"") + process.__enter__.return_value = process + + with mock.patch("azext_containerapp._utils.get_safe_executable_path", return_value=docker_path) as resolve, \ + mock.patch("azext_containerapp._utils.subprocess.Popen", return_value=process) as popen: + self.assertTrue(is_docker_running()) + + resolve.assert_called_once_with("docker") + popen.assert_called_once_with( + [docker_path, "stats", "--no-stream"], + stdout=mock.ANY, + stderr=mock.ANY, + ) + + def test_is_docker_running_returns_false_when_docker_is_not_found(self): + with mock.patch("azext_containerapp._utils.get_safe_executable_path", return_value=None) as resolve, \ + mock.patch("azext_containerapp._utils.subprocess.Popen") as popen: + self.assertFalse(is_docker_running()) + + resolve.assert_called_once_with("docker") + popen.assert_not_called() + + +class TestDockerPushPath(unittest.TestCase): + @staticmethod + def _create_app(method): + app_type = type("TestContainerApp", (), {"_docker_push_to_container_registry": method}) + app = app_type() + app.acr = mock.MagicMock() + app.acr.name = "testregistry" + app.cmd = mock.MagicMock() + app.cmd.command_kwargs = { + "resource_type": "old-resource-type", + "operation_group": "old-operation-group", + } + return app + + @staticmethod + def _create_process(returncode, stderr=b""): + process = mock.MagicMock() + process.returncode = returncode + process.communicate.return_value = (b"", stderr) + process.__enter__.return_value = process + return process + + def test_push_uses_resolved_docker_path(self): + method = _extract_up_utils_method("ContainerApp", "_docker_push_to_container_registry") + app = self._create_app(method) + docker_path = os.path.join(os.path.abspath(os.sep), "trusted", "docker.exe") + mock_subprocess = mock.MagicMock() + mock_subprocess.PIPE = __import__("subprocess").PIPE + mock_subprocess.Popen.return_value = self._create_process(0) + + with mock.patch.dict( + method.__globals__, + {"get_safe_executable_path": lambda _: docker_path, "subprocess": mock_subprocess} + ): + app._docker_push_to_container_registry("registry/image:tag") + + mock_subprocess.Popen.assert_called_once_with( + [docker_path, "push", "registry/image:tag"], + stdout=mock_subprocess.PIPE, + stderr=mock_subprocess.PIPE, + ) + + def test_acr_login_uses_resolved_path_and_restores_environment(self): + method = _extract_up_utils_method("ContainerApp", "_docker_push_to_container_registry") + app = self._create_app(method) + docker_path = os.path.join(os.path.abspath(os.sep), "trusted", "docker.exe") + mock_subprocess = mock.MagicMock() + mock_subprocess.PIPE = __import__("subprocess").PIPE + mock_subprocess.Popen.side_effect = [ + self._create_process(1, b"unauthorized"), + self._create_process(0), + ] + + def assert_safe_docker_command(*_): + self.assertEqual(os.environ["DOCKER_COMMAND"], docker_path) + + original_command_kwargs = dict(app.cmd.command_kwargs) + with mock.patch.dict( + method.__globals__, + {"get_safe_executable_path": lambda _: docker_path, "subprocess": mock_subprocess} + ), mock.patch( + "azure.cli.command_modules.acr.custom.acr_login", + side_effect=assert_safe_docker_command + ) as acr_login, mock.patch.dict( + os.environ, + {"DOCKER_COMMAND": "original-docker-command"} + ): + app._docker_push_to_container_registry("testregistry.azurecr.io/image:tag") + self.assertEqual(os.environ["DOCKER_COMMAND"], "original-docker-command") + + acr_login.assert_called_once_with(app.cmd, "testregistry") + self.assertEqual(app.cmd.command_kwargs, original_command_kwargs) + self.assertEqual(mock_subprocess.Popen.call_count, 2) + for call in mock_subprocess.Popen.call_args_list: + self.assertEqual(call.args[0][0], docker_path) + + def test_acr_login_restores_absent_command_kwargs(self): + method = _extract_up_utils_method("ContainerApp", "_docker_push_to_container_registry") + app = self._create_app(method) + app.cmd.command_kwargs = {} + docker_path = os.path.join(os.path.abspath(os.sep), "trusted", "docker.exe") + mock_subprocess = mock.MagicMock() + mock_subprocess.PIPE = __import__("subprocess").PIPE + mock_subprocess.Popen.return_value = self._create_process(1, b"unauthorized") + + with mock.patch.dict( + method.__globals__, + {"get_safe_executable_path": lambda _: docker_path, "subprocess": mock_subprocess} + ), mock.patch( + "azure.cli.command_modules.acr.custom.acr_login", + side_effect=CLIError("login failed") + ), mock.patch.dict( + os.environ, + {}, + clear=True + ): + with self.assertRaises(CLIError): + app._docker_push_to_container_registry("testregistry.azurecr.io/image:tag") + + self.assertEqual(app.cmd.command_kwargs, {}) + self.assertNotIn("DOCKER_COMMAND", os.environ) class TestResolveEnvironmentModeAndWorkloadProfiles(unittest.TestCase):