mirror of
https://github.com/GNS3/gns3-server.git
synced 2026-09-16 06:50:41 +03:00
Merge pull request #2874 from yueguobin/fix/docker-root-owned-file-reclaim
fix: reclaim root-owned container files so docker nodes and projects can be deleted
This commit is contained in:
commit
2a3626824f
@ -35,11 +35,13 @@ from gns3server.utils.asyncio.telnet_server import AsyncioTelnetServer
|
||||
from gns3server.utils.asyncio.raw_command_server import AsyncioRawCommandServer
|
||||
from gns3server.utils.asyncio import wait_for_file_creation
|
||||
from gns3server.utils.asyncio import monitor_process
|
||||
from gns3server.utils.asyncio import wait_run_in_executor
|
||||
from gns3server.utils.get_resource import get_resource
|
||||
from gns3server.utils.hostname import is_rfc1123_hostname_valid
|
||||
from gns3server.utils import macaddress_to_int, int_to_macaddress
|
||||
|
||||
from gns3server.compute.ubridge.ubridge_error import UbridgeError, UbridgeNamespaceError
|
||||
from gns3server.compute.compute_error import ComputeError
|
||||
from ..base_node import BaseNode
|
||||
|
||||
from ..adapters.ethernet_adapter import EthernetAdapter
|
||||
@ -148,6 +150,9 @@ class DockerVM(BaseNode):
|
||||
self._console_websocket = None
|
||||
self.extra_hosts = extra_hosts
|
||||
self._extra_volumes = extra_volumes or []
|
||||
# Image ID captured at create time (see _mount_binds): unlike the
|
||||
# image tag it keeps resolving even if the tag is removed later.
|
||||
self._image_id = None
|
||||
self._extra_configs = extra_configs or []
|
||||
self._memory = memory
|
||||
self._cpus = cpus
|
||||
@ -448,6 +453,7 @@ class DockerVM(BaseNode):
|
||||
"Target": "/gns3",
|
||||
"ReadOnly": True
|
||||
}]
|
||||
self._image_id = image_info.get("Id") or self._image_id
|
||||
|
||||
# We mount our own etc/network
|
||||
try:
|
||||
@ -1024,6 +1030,105 @@ class DockerVM(BaseNode):
|
||||
else:
|
||||
self._permissions_fixed = True
|
||||
|
||||
def _directory_has_foreign_files(self, directory):
|
||||
"""
|
||||
Synchronous walk (run in an executor): return True as soon as an
|
||||
entry not owned by the server user is found under ``directory``.
|
||||
What we are looking for are files the container wrote as root —
|
||||
the unprivileged server can neither chown nor delete them.
|
||||
"""
|
||||
|
||||
uid = os.getuid()
|
||||
for root, dirs, files in os.walk(directory):
|
||||
for entry in dirs + files:
|
||||
try:
|
||||
if os.stat(os.path.join(root, entry)).st_uid != uid:
|
||||
return True
|
||||
except OSError:
|
||||
# An entry we cannot stat is not ours to delete either;
|
||||
# treat it as foreign and let the reclaim sort it out.
|
||||
return True
|
||||
return False
|
||||
|
||||
async def _reclaim_directory_ownership(self, directory):
|
||||
"""
|
||||
Best-effort reclaim of files the container left owned by root.
|
||||
|
||||
The stop-time permission pass necessarily runs before the
|
||||
container's processes exit, so anything they write during the
|
||||
shutdown window (and everything after a SIGKILL path) lands as
|
||||
root-owned on the host. An unprivileged server cannot chown or
|
||||
delete those files, but it can ask Docker to run a throwaway
|
||||
container of the node's own image — entrypoint overridden to the
|
||||
GNS3 busybox, so nothing of the guest boots — and chown the tree
|
||||
back from the container's root side. Docker is the only privilege
|
||||
door a non-root server has.
|
||||
|
||||
:returns: True when ``directory`` is (now) owned by the server
|
||||
user, False when the reclaim could not run or failed.
|
||||
"""
|
||||
|
||||
if not os.path.exists(directory):
|
||||
return True
|
||||
try:
|
||||
if not await wait_run_in_executor(self._directory_has_foreign_files, directory):
|
||||
log.debug(f"Docker container '{self._name}': no foreign-owned files under '{directory}'")
|
||||
return True
|
||||
except OSError as e:
|
||||
log.warning(f"Docker container '{self._name}': could not inspect '{directory}' for root-owned files: {e}")
|
||||
return False
|
||||
|
||||
uid, gid = os.getuid(), os.getgid()
|
||||
try:
|
||||
resources_path = self.manager.resources_path()
|
||||
except OSError as e:
|
||||
log.warning(f"Docker container '{self._name}': cannot access resources to reclaim '{directory}': {e}")
|
||||
return False
|
||||
|
||||
log.info(f"Docker container '{self._name}': reclaiming root-owned files under '{directory}' via a one-shot container")
|
||||
# Prefer the image's own chown over the static busybox one: busybox's
|
||||
# chown dlopens NSS modules from the image, which mismatch the static
|
||||
# glibc and abort on NOS images whose glibc differs (same reasoning
|
||||
# as the container-side pass in _fix_permissions). --pull=never keeps
|
||||
# a stale tag reference from turning into a registry pull attempt,
|
||||
# and the create-time image ID (when known) is immune to retagging.
|
||||
# --user 0:0 overrides a USER baked into the image (e.g.
|
||||
# ghcr.io/nokia/srlinux runs as "user"): without it the "privileged"
|
||||
# helper is exactly as unprivileged as the server itself and
|
||||
# chmod/chown fail with EPERM on files written by other uids.
|
||||
image_ref = self._image_id or self._image
|
||||
try:
|
||||
process = await asyncio.subprocess.create_subprocess_exec(
|
||||
"docker", "run", "--rm", "--network", "none", "--pull", "never", "--user", "0:0",
|
||||
"--entrypoint", "/gns3/bin/busybox",
|
||||
"-v", f"{resources_path}:/gns3:ro",
|
||||
"-v", f"{directory}:/target",
|
||||
image_ref,
|
||||
"sh", "-c",
|
||||
"/gns3/bin/busybox chmod -R u+rwX /target"
|
||||
f" && ( command -v chown >/dev/null 2>&1 && chown {uid}:{gid} -R /target"
|
||||
f" || /gns3/bin/busybox chown {uid}:{gid} -R /target )",
|
||||
stdout=asyncio.subprocess.PIPE,
|
||||
stderr=asyncio.subprocess.PIPE,
|
||||
)
|
||||
except OSError as e:
|
||||
log.warning(f"Docker container '{self._name}': could not reclaim ownership of '{directory}': {e}")
|
||||
return False
|
||||
try:
|
||||
_, stderr = await asyncio.wait_for(process.communicate(), timeout=60)
|
||||
except asyncio.TimeoutError:
|
||||
process.kill()
|
||||
await process.communicate()
|
||||
log.warning(f"Docker container '{self._name}': reclaiming '{directory}' timed out")
|
||||
return False
|
||||
if process.returncode != 0:
|
||||
log.warning(
|
||||
f"Docker container '{self._name}': reclaiming '{directory}' failed: "
|
||||
f"{stderr.decode(errors='replace').strip() or f'exit code {process.returncode}'}"
|
||||
)
|
||||
return False
|
||||
return True
|
||||
|
||||
async def _start_vnc_process(self, restart=False):
|
||||
"""
|
||||
Starts the VNC process.
|
||||
@ -1350,6 +1455,13 @@ class DockerVM(BaseNode):
|
||||
if not (await super().close()):
|
||||
return False
|
||||
await self.reset()
|
||||
# Whatever the container's processes wrote after the stop-time
|
||||
# permission pass (the shutdown window, or any SIGKILL path) is
|
||||
# still owned by root on the host and would make the node
|
||||
# directory undeletable for the unprivileged server. Reclaim it
|
||||
# before close() returns: project deletion rmtrees the directory
|
||||
# right after the nodes close.
|
||||
await self._reclaim_directory_ownership(self.working_dir)
|
||||
|
||||
async def reset(self, release_nio_udp_ports=True):
|
||||
|
||||
@ -1928,4 +2040,20 @@ class DockerVM(BaseNode):
|
||||
"""
|
||||
|
||||
await self.close()
|
||||
await super().delete()
|
||||
try:
|
||||
await super().delete()
|
||||
except ComputeError as e:
|
||||
# close() already reclaimed once; the retry covers states where
|
||||
# it could not run (e.g. Docker was down) and Docker is back now.
|
||||
if await self._reclaim_directory_ownership(self.working_dir):
|
||||
try:
|
||||
await super().delete()
|
||||
return
|
||||
except ComputeError:
|
||||
pass
|
||||
raise ComputeError(
|
||||
f"Could not delete the node directory '{self.working_dir}': files left owned by "
|
||||
f"another user could not be reclaimed ({e}). Reclaim them manually with: "
|
||||
f"docker run --rm --user 0:0 -v \"{self.working_dir}\":/target --entrypoint /bin/sh "
|
||||
f"{self._image} -c 'chown -R {os.getuid()}:{os.getgid()} /target'"
|
||||
)
|
||||
|
||||
@ -22,10 +22,12 @@ import pytest_asyncio
|
||||
import uuid
|
||||
import os
|
||||
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import patch
|
||||
from tests.utils import asyncio_patch, AsyncioMagicMock
|
||||
|
||||
from gns3server.compute.ubridge.ubridge_error import UbridgeNamespaceError
|
||||
from gns3server.compute.compute_error import ComputeError
|
||||
from gns3server.compute.docker.docker_vm import DockerVM
|
||||
from gns3server.compute.docker.docker_error import DockerError, DockerHttp404Error
|
||||
from gns3server.compute.docker import Docker
|
||||
@ -2037,6 +2039,120 @@ async def test_fix_permission_not_running(vm):
|
||||
assert process.wait.called
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Ownership reclaim for files the container left owned by root
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def _reclaim_proc(returncode=0, stderr=b""):
|
||||
|
||||
process = MagicMock()
|
||||
process.communicate = AsyncioMagicMock(return_value=(b"", stderr))
|
||||
process.returncode = returncode
|
||||
process.kill = MagicMock()
|
||||
return process
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reclaim_skips_clean_directory(vm, tmp_path):
|
||||
|
||||
(tmp_path / "file").write_text("owned by the server user")
|
||||
with patch("asyncio.subprocess.create_subprocess_exec") as mock_exec:
|
||||
assert await vm._reclaim_directory_ownership(str(tmp_path)) is True
|
||||
mock_exec.assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reclaim_skips_missing_directory(vm):
|
||||
|
||||
with patch("asyncio.subprocess.create_subprocess_exec") as mock_exec:
|
||||
assert await vm._reclaim_directory_ownership("/does/not/exist") is True
|
||||
mock_exec.assert_not_called()
|
||||
|
||||
|
||||
def test_directory_has_foreign_files_detects_foreign_owner(vm, tmp_path):
|
||||
|
||||
(tmp_path / "root-owned").write_text("written by the container as root")
|
||||
assert vm._directory_has_foreign_files(str(tmp_path)) is False
|
||||
|
||||
# A directory the unprivileged server user cannot own: pretend every
|
||||
# stat reports another uid (the walk only uses scandir for recursion).
|
||||
with patch("os.stat", return_value=SimpleNamespace(st_uid=os.getuid() + 1)):
|
||||
assert vm._directory_has_foreign_files(str(tmp_path)) is True
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reclaim_runs_helper_container(vm, tmp_path):
|
||||
|
||||
directory = tmp_path / "node"
|
||||
directory.mkdir()
|
||||
vm._image_id = "sha256:8731fa5e0f0f"
|
||||
|
||||
with patch.object(vm, "_directory_has_foreign_files", return_value=True):
|
||||
with patch.object(vm.manager, "resources_path", return_value="/gns3-share"):
|
||||
with patch("asyncio.subprocess.create_subprocess_exec", return_value=_reclaim_proc()) as mock_exec:
|
||||
assert await vm._reclaim_directory_ownership(str(directory)) is True
|
||||
|
||||
args, _ = mock_exec.call_args
|
||||
assert args[:9] == ("docker", "run", "--rm", "--network", "none", "--pull", "never", "--user", "0:0")
|
||||
assert args[args.index("--entrypoint") + 1] == "/gns3/bin/busybox"
|
||||
assert "/gns3-share:/gns3:ro" in args
|
||||
assert f"{directory}:/target" in args
|
||||
# The create-time image ID is used as the helper image (immune to retagging)
|
||||
# and sits right before the CMD ("sh -c …").
|
||||
assert args[args.index("sh") - 1] == "sha256:8731fa5e0f0f"
|
||||
script = args[args.index("-c") + 1]
|
||||
assert "chown" in script and f"{os.getuid()}:{os.getgid()}" in script
|
||||
assert "chmod -R u+rwX" in script
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reclaim_fails_when_helper_errors(vm, tmp_path):
|
||||
|
||||
directory = tmp_path / "node"
|
||||
directory.mkdir()
|
||||
|
||||
with patch.object(vm, "_directory_has_foreign_files", return_value=True):
|
||||
with patch.object(vm.manager, "resources_path", return_value="/gns3-share"):
|
||||
with patch("asyncio.subprocess.create_subprocess_exec",
|
||||
return_value=_reclaim_proc(returncode=1, stderr=b"chown: failed")):
|
||||
assert await vm._reclaim_directory_ownership(str(directory)) is False
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_close_reclaims_node_directory(vm, port_manager):
|
||||
|
||||
with asyncio_patch("gns3server.compute.docker.DockerVM._get_container_state", return_value="stopped"):
|
||||
with asyncio_patch("gns3server.compute.docker.Docker.query"):
|
||||
with patch.object(vm, "_reclaim_directory_ownership", new_callable=AsyncioMagicMock) as mock_reclaim:
|
||||
await vm.close()
|
||||
mock_reclaim.assert_called_once_with(vm.working_dir)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_delete_retries_after_reclaim(vm):
|
||||
|
||||
with patch.object(vm, "close", new_callable=AsyncioMagicMock):
|
||||
with patch.object(vm, "_reclaim_directory_ownership", new_callable=AsyncioMagicMock, return_value=True) as mock_reclaim:
|
||||
# First rmtree hits the root-owned leftovers, the retry (after
|
||||
# the reclaim) succeeds.
|
||||
with patch("gns3server.compute.base_node.shutil.rmtree",
|
||||
side_effect=[OSError("permission denied"), None]) as mock_rmtree:
|
||||
await vm.delete()
|
||||
assert mock_rmtree.call_count == 2
|
||||
mock_reclaim.assert_called_once_with(vm.working_dir)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_delete_reports_root_files_when_reclaim_fails(vm):
|
||||
|
||||
with patch.object(vm, "close", new_callable=AsyncioMagicMock):
|
||||
with patch.object(vm, "_reclaim_directory_ownership", new_callable=AsyncioMagicMock, return_value=False):
|
||||
with patch("gns3server.compute.base_node.shutil.rmtree",
|
||||
side_effect=OSError("permission denied")):
|
||||
with pytest.raises(ComputeError, match="owned by another user"):
|
||||
await vm.delete()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_read_console_output_with_binary_mode(vm):
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user