From 468d532c3ce75292cebee44872e0e1d8e327f4d0 Mon Sep 17 00:00:00 2001 From: xinyu pang Date: Tue, 13 Oct 2026 18:01:00 +0800 Subject: [PATCH 1/2] Fix containerapp Docker executable resolution Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/containerapp/HISTORY.rst | 1 + .../azext_containerapp/_up_utils.py | 24 ++- src/containerapp/azext_containerapp/_utils.py | 38 +++- src/containerapp/azext_containerapp/custom.py | 9 +- .../latest/test_containerapp_patch_unit.py | 28 ++- .../tests/latest/test_utils.py | 175 +++++++++++++++++- 6 files changed, 259 insertions(+), 16 deletions(-) diff --git a/src/containerapp/HISTORY.rst b/src/containerapp/HISTORY.rst index 9616545fbd6..def0a5814ec 100644 --- a/src/containerapp/HISTORY.rst +++ b/src/containerapp/HISTORY.rst @@ -4,6 +4,7 @@ Release History =============== upcoming ++++++ +* Resolve Docker to a trusted absolute path 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..9122626bc2b 100644 --- a/src/containerapp/azext_containerapp/_up_utils.py +++ b/src/containerapp/azext_containerapp/_up_utils.py @@ -56,7 +56,7 @@ from ._utils import ( is_docker_running, - get_pack_exec_path, _validate_custom_loc_and_location, _validate_connected_k8s_exists, get_custom_location, + get_docker_exec_path, get_pack_exec_path, _validate_custom_loc_and_location, _validate_connected_k8s_exists, get_custom_location, create_extension, create_custom_location, get_cluster_extension, validate_environment_location, list_environment_locations, get_randomized_name_with_dash, get_randomized_name, get_connected_k8s, list_cluster_extensions, list_custom_location, is_cloud_supported_by_connected_env @@ -534,7 +534,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_docker_exec_path() + 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: @@ -551,10 +555,18 @@ def _docker_push_to_container_registry(self, image_name, forced_acr_login=False) for key in task_command_kwargs: # pylint: disable=consider-using-dict-items old_command_kwargs[key] = self.cmd.command_kwargs.get(key) 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(): + 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..83420adc052 100644 --- a/src/containerapp/azext_containerapp/_utils.py +++ b/src/containerapp/azext_containerapp/_utils.py @@ -263,14 +263,48 @@ def list_environment_locations(cmd, resource_type=MANAGED_ENVIRONMENT_RESOURCE_T return res_locations +def get_docker_exec_path(): + docker_exec_path = shutil.which("docker") + if not docker_exec_path: + return None + + try: + cwd = os.path.abspath(os.getcwd()) + docker_exec_path = os.path.abspath(docker_exec_path) + resolved_paths = (docker_exec_path, os.path.realpath(docker_exec_path)) + + for resolved_path in resolved_paths: + resolved_dir = os.path.dirname(resolved_path) + try: + is_cwd = os.path.samefile(resolved_dir, cwd) + except (OSError, ValueError): + is_cwd = os.path.normcase(os.path.normpath(resolved_dir)) == os.path.normcase(os.path.normpath(cwd)) + + if is_cwd: + raise CLIError( + "Refusing to use Docker executable '{}' because it is located in the current working directory." + .format(resolved_path) + ) + except CLIError: + raise + except (OSError, ValueError) as ex: + raise CLIError("Unable to verify Docker executable path safety.") from ex + + return docker_exec_path + + def is_docker_running(): + docker_exec_path = get_docker_exec_path() + 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..cf194f427c8 100644 --- a/src/containerapp/azext_containerapp/custom.py +++ b/src/containerapp/azext_containerapp/custom.py @@ -143,7 +143,7 @@ from ._ssh_utils import (SSH_DEFAULT_ENCODING, DebugWebSocketConnection, read_debug_ssh) from ._utils import (connected_env_check_cert_name_availability, get_oryx_run_image_tags, patchable_check, - get_pack_exec_path, is_docker_running, parse_build_env_vars, env_has_managed_identity) + get_docker_exec_path, get_pack_exec_path, is_docker_running, parse_build_env_vars, env_has_managed_identity) from ._utils_validation import validate_image_name from ._arc_utils import (extract_domain_from_configmap, get_core_dns_deployment, get_core_dns_configmap, backup_custom_core_dns_configmap, @@ -1898,12 +1898,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_docker_exec_path() + 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..a839b2a109a 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_docker_exec_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_docker_exec_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..63d42842982 100644 --- a/src/containerapp/azext_containerapp/tests/latest/test_utils.py +++ b/src/containerapp/azext_containerapp/tests/latest/test_utils.py @@ -3,9 +3,180 @@ # 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 ( + get_docker_exec_path, + 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_docker_exec_path": get_docker_exec_path, + "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_returns_absolute_path_outside_current_directory(self): + cwd = os.path.join(os.path.abspath(os.sep), "repo") + docker_path = os.path.join(os.path.abspath(os.sep), "trusted", "docker.exe") + + with mock.patch("azext_containerapp._utils.shutil.which", return_value=docker_path), \ + mock.patch("azext_containerapp._utils.os.getcwd", return_value=cwd): + self.assertEqual(get_docker_exec_path(), os.path.abspath(docker_path)) + + def test_rejects_executable_in_current_directory(self): + cwd = os.path.join(os.path.abspath(os.sep), "repo") + docker_path = os.path.join(cwd, "docker.exe") + + with mock.patch("azext_containerapp._utils.shutil.which", return_value=docker_path), \ + mock.patch("azext_containerapp._utils.os.getcwd", return_value=cwd): + with self.assertRaises(CLIError): + get_docker_exec_path() + + def test_rejects_executable_resolving_to_current_directory(self): + cwd = os.path.join(os.path.abspath(os.sep), "repo") + docker_path = os.path.join(os.path.abspath(os.sep), "trusted", "docker.exe") + resolved_path = os.path.join(cwd, "docker.exe") + + with mock.patch("azext_containerapp._utils.shutil.which", return_value=docker_path), \ + mock.patch("azext_containerapp._utils.os.getcwd", return_value=cwd), \ + mock.patch("azext_containerapp._utils.os.path.realpath", return_value=resolved_path): + with self.assertRaises(CLIError): + get_docker_exec_path() + + @mock.patch("azext_containerapp._utils.shutil.which", return_value=None) + def test_returns_none_when_docker_is_not_found(self, _): + self.assertIsNone(get_docker_exec_path()) + + 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_docker_exec_path", return_value=docker_path), \ + mock.patch("azext_containerapp._utils.subprocess.Popen", return_value=process) as popen: + self.assertTrue(is_docker_running()) + + popen.assert_called_once_with( + [docker_path, "stats", "--no-stream"], + stdout=mock.ANY, + stderr=mock.ANY, + ) + + def test_is_docker_running_does_not_hide_unsafe_path(self): + with mock.patch( + "azext_containerapp._utils.get_docker_exec_path", + side_effect=CLIError("unsafe Docker path") + ), mock.patch("azext_containerapp._utils.subprocess.Popen") as popen: + with self.assertRaises(CLIError): + is_docker_running() + 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_docker_exec_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_docker_exec_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) class TestResolveEnvironmentModeAndWorkloadProfiles(unittest.TestCase): From a102f9ad1a220a2d8219cc4b9124dff4e573a18b Mon Sep 17 00:00:00 2001 From: xinyu pang Date: Wed, 14 Oct 2026 15:40:23 +0800 Subject: [PATCH 2/2] Use Core safe executable path helper Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/containerapp/HISTORY.rst | 2 +- .../azext_containerapp/_up_utils.py | 13 ++- src/containerapp/azext_containerapp/_utils.py | 34 +------- src/containerapp/azext_containerapp/custom.py | 5 +- .../latest/test_containerapp_patch_unit.py | 4 +- .../tests/latest/test_utils.py | 83 +++++++++---------- 6 files changed, 55 insertions(+), 86 deletions(-) diff --git a/src/containerapp/HISTORY.rst b/src/containerapp/HISTORY.rst index def0a5814ec..c392f71167e 100644 --- a/src/containerapp/HISTORY.rst +++ b/src/containerapp/HISTORY.rst @@ -4,7 +4,7 @@ Release History =============== upcoming ++++++ -* Resolve Docker to a trusted absolute path before invoking it from local build and patch flows +* 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 9122626bc2b..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, ) @@ -56,7 +57,7 @@ from ._utils import ( is_docker_running, - get_docker_exec_path, get_pack_exec_path, _validate_custom_loc_and_location, _validate_connected_k8s_exists, get_custom_location, + get_pack_exec_path, _validate_custom_loc_and_location, _validate_connected_k8s_exists, get_custom_location, create_extension, create_custom_location, get_cluster_extension, validate_environment_location, list_environment_locations, get_randomized_name_with_dash, get_randomized_name, get_connected_k8s, list_cluster_extensions, list_custom_location, is_cloud_supported_by_connected_env @@ -534,7 +535,7 @@ 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 - docker_exec_path = get_docker_exec_path() + docker_exec_path = get_safe_executable_path("docker") if not docker_exec_path: raise CLIError("Docker executable could not be found.") @@ -551,9 +552,10 @@ 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] old_docker_command = os.environ.get("DOCKER_COMMAND") try: @@ -566,7 +568,10 @@ def _docker_push_to_container_registry(self, image_name, forced_acr_login=False) else: os.environ["DOCKER_COMMAND"] = old_docker_command for k, v in old_command_kwargs.items(): - self.cmd.command_kwargs[k] = v + 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 83420adc052..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 @@ -263,38 +263,8 @@ def list_environment_locations(cmd, resource_type=MANAGED_ENVIRONMENT_RESOURCE_T return res_locations -def get_docker_exec_path(): - docker_exec_path = shutil.which("docker") - if not docker_exec_path: - return None - - try: - cwd = os.path.abspath(os.getcwd()) - docker_exec_path = os.path.abspath(docker_exec_path) - resolved_paths = (docker_exec_path, os.path.realpath(docker_exec_path)) - - for resolved_path in resolved_paths: - resolved_dir = os.path.dirname(resolved_path) - try: - is_cwd = os.path.samefile(resolved_dir, cwd) - except (OSError, ValueError): - is_cwd = os.path.normcase(os.path.normpath(resolved_dir)) == os.path.normcase(os.path.normpath(cwd)) - - if is_cwd: - raise CLIError( - "Refusing to use Docker executable '{}' because it is located in the current working directory." - .format(resolved_path) - ) - except CLIError: - raise - except (OSError, ValueError) as ex: - raise CLIError("Unable to verify Docker executable path safety.") from ex - - return docker_exec_path - - def is_docker_running(): - docker_exec_path = get_docker_exec_path() + docker_exec_path = get_safe_executable_path("docker") if not docker_exec_path: return False diff --git a/src/containerapp/azext_containerapp/custom.py b/src/containerapp/azext_containerapp/custom.py index cf194f427c8..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 @@ -143,7 +144,7 @@ from ._ssh_utils import (SSH_DEFAULT_ENCODING, DebugWebSocketConnection, read_debug_ssh) from ._utils import (connected_env_check_cert_name_availability, get_oryx_run_image_tags, patchable_check, - get_docker_exec_path, get_pack_exec_path, is_docker_running, parse_build_env_vars, env_has_managed_identity) + get_pack_exec_path, is_docker_running, parse_build_env_vars, env_has_managed_identity) from ._utils_validation import validate_image_name from ._arc_utils import (extract_domain_from_configmap, get_core_dns_deployment, get_core_dns_configmap, backup_custom_core_dns_configmap, @@ -1898,7 +1899,7 @@ 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_docker_exec_path() + 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) 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 a839b2a109a..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 @@ -84,7 +84,7 @@ def test_rejects_newline(self): "logger": mock.MagicMock(), "CLIError": CLIError, "validate_image_name": validate_image_name, - "get_docker_exec_path": lambda: "/usr/local/bin/docker", + "get_safe_executable_path": lambda _: "/usr/local/bin/docker", "update_containerapp": mock.MagicMock(return_value={}), } @@ -224,7 +224,7 @@ def test_does_not_run_when_docker_is_not_found(self): with mock.patch.dict( _FUNC_GLOBALS, - {"subprocess": mock_subprocess, "get_docker_exec_path": lambda: None} + {"subprocess": mock_subprocess, "get_safe_executable_path": lambda _: None} ): fn = _extract_function("patch_cli_call") with self.assertRaises(CLIError): diff --git a/src/containerapp/azext_containerapp/tests/latest/test_utils.py b/src/containerapp/azext_containerapp/tests/latest/test_utils.py index 63d42842982..ea4d65dc1d8 100644 --- a/src/containerapp/azext_containerapp/tests/latest/test_utils.py +++ b/src/containerapp/azext_containerapp/tests/latest/test_utils.py @@ -9,7 +9,6 @@ from azure.cli.core.azclierror import CLIError, ValidationError from azext_containerapp._utils import ( - get_docker_exec_path, is_docker_running, validate_environment_mode_and_workload_profiles_compatible, ) @@ -30,7 +29,7 @@ def _extract_up_utils_method(class_name, method_name): if isinstance(child, ast.FunctionDef) and child.name == method_name: namespace = { "CLIError": CLIError, - "get_docker_exec_path": get_docker_exec_path, + "get_safe_executable_path": mock.MagicMock(), "logger": mock.MagicMock(), "os": os, "subprocess": __import__("subprocess"), @@ -43,38 +42,6 @@ def _extract_up_utils_method(class_name, method_name): class TestDockerExecutablePath(unittest.TestCase): - def test_returns_absolute_path_outside_current_directory(self): - cwd = os.path.join(os.path.abspath(os.sep), "repo") - docker_path = os.path.join(os.path.abspath(os.sep), "trusted", "docker.exe") - - with mock.patch("azext_containerapp._utils.shutil.which", return_value=docker_path), \ - mock.patch("azext_containerapp._utils.os.getcwd", return_value=cwd): - self.assertEqual(get_docker_exec_path(), os.path.abspath(docker_path)) - - def test_rejects_executable_in_current_directory(self): - cwd = os.path.join(os.path.abspath(os.sep), "repo") - docker_path = os.path.join(cwd, "docker.exe") - - with mock.patch("azext_containerapp._utils.shutil.which", return_value=docker_path), \ - mock.patch("azext_containerapp._utils.os.getcwd", return_value=cwd): - with self.assertRaises(CLIError): - get_docker_exec_path() - - def test_rejects_executable_resolving_to_current_directory(self): - cwd = os.path.join(os.path.abspath(os.sep), "repo") - docker_path = os.path.join(os.path.abspath(os.sep), "trusted", "docker.exe") - resolved_path = os.path.join(cwd, "docker.exe") - - with mock.patch("azext_containerapp._utils.shutil.which", return_value=docker_path), \ - mock.patch("azext_containerapp._utils.os.getcwd", return_value=cwd), \ - mock.patch("azext_containerapp._utils.os.path.realpath", return_value=resolved_path): - with self.assertRaises(CLIError): - get_docker_exec_path() - - @mock.patch("azext_containerapp._utils.shutil.which", return_value=None) - def test_returns_none_when_docker_is_not_found(self, _): - self.assertIsNone(get_docker_exec_path()) - 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() @@ -82,24 +49,24 @@ def test_is_docker_running_uses_resolved_path(self): process.communicate.return_value = (b"", b"") process.__enter__.return_value = process - with mock.patch("azext_containerapp._utils.get_docker_exec_path", return_value=docker_path), \ + 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_does_not_hide_unsafe_path(self): - with mock.patch( - "azext_containerapp._utils.get_docker_exec_path", - side_effect=CLIError("unsafe Docker path") - ), mock.patch("azext_containerapp._utils.subprocess.Popen") as popen: - with self.assertRaises(CLIError): - is_docker_running() - popen.assert_not_called() + 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): @@ -134,7 +101,7 @@ def test_push_uses_resolved_docker_path(self): with mock.patch.dict( method.__globals__, - {"get_docker_exec_path": lambda: docker_path, "subprocess": mock_subprocess} + {"get_safe_executable_path": lambda _: docker_path, "subprocess": mock_subprocess} ): app._docker_push_to_container_registry("registry/image:tag") @@ -161,7 +128,7 @@ def assert_safe_docker_command(*_): original_command_kwargs = dict(app.cmd.command_kwargs) with mock.patch.dict( method.__globals__, - {"get_docker_exec_path": lambda: docker_path, "subprocess": mock_subprocess} + {"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 @@ -178,6 +145,32 @@ def assert_safe_docker_command(*_): 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): """Tests for the new resolve_environment_mode_and_workload_profiles function."""