Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions python/ray/experimental/sandbox/_internal/image_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -273,11 +273,12 @@ def extract_tar_layer(
pass
elif member.isdir():
os.makedirs(target_path, exist_ok=True)
# Applied after the loop: extracting children would bump it.
# Skip preserved symlinks (UsrMerge /bin -> usr/bin): utime
# would follow them and stamp the target with the wrong time.
# Deferred to the post-loop pass: tar lists a directory
# before its contents, so a restrictive archived mode (0500)
# applied here would break extracting the children. Preserved
# symlinks (UsrMerge) are skipped: chmod/utime follow them.
if not os.path.islink(target_path):
dir_mtimes.append((target_path, member.mtime))
dir_mtimes.append((target_path, member.mode, member.mtime))
elif member.issym():
os.makedirs(parent_dir, exist_ok=True)
try:
Expand All @@ -295,8 +296,11 @@ def extract_tar_layer(
except OSError:
pass

for dir_path, mtime in dir_mtimes:
# Children first, so a parent's restrictive mode cannot block them.
for dir_path, mode, mtime in reversed(dir_mtimes):
try:
if mode:
os.chmod(dir_path, mode)
Comment thread
cursor[bot] marked this conversation as resolved.
os.utime(dir_path, (mtime, mtime))
except OSError:
pass
Expand Down
70 changes: 70 additions & 0 deletions python/ray/experimental/sandbox/image_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
_OCI_CAPABILITY_SETS = ("bounding", "effective", "permitted")

_RESOLV_CONF = "/etc/resolv.conf"
_ETC_HOSTS = "/etc/hosts"


def get_default_oci_spec() -> Dict[str, Any]:
Expand Down Expand Up @@ -138,6 +139,7 @@ def create_oci_spec(
capabilities: Optional[List[str]] = None,
network: str = "none",
resolv_conf_source: Optional[str] = None,
hosts_source: Optional[str] = None,
base_spec: Optional[Dict[str, Any]] = None,
_oci_spec_transform_fn: Optional[Callable[[Dict], Optional[Dict]]] = None,
) -> Dict[str, Any]:
Expand All @@ -159,6 +161,9 @@ def create_oci_spec(
spec's empty network namespace.
resolv_conf_source: Optional file to bind-mount read-only at
/etc/resolv.conf.
hosts_source: Optional file to bind-mount read-write at
/etc/hosts (a per-sandbox copy, like the one container
engines inject).
base_spec: Optional base OCI spec dict to modify instead of generating a default.
_oci_spec_transform_fn: Optional callback to transform the final spec.

Expand Down Expand Up @@ -329,6 +334,7 @@ def create_oci_spec(
capabilities: Optional[List[str]] = None,
network: str = "none",
resolv_conf_source: Optional[str] = None,
hosts_source: Optional[str] = None,
base_spec: Optional[Dict[str, Any]] = None,
_oci_spec_transform_fn: Optional[Callable[[Dict], Optional[Dict]]] = None,
) -> Dict[str, Any]:
Expand All @@ -350,6 +356,9 @@ def create_oci_spec(
spec's empty network namespace.
resolv_conf_source: Optional file to bind-mount read-only at
/etc/resolv.conf.
hosts_source: Optional file to bind-mount read-write at
/etc/hosts (a per-sandbox copy, like the one container
engines inject).
base_spec: Optional base OCI spec dict to modify instead of generating a default.
_oci_spec_transform_fn: Optional callback to transform the final spec.

Expand All @@ -369,6 +378,25 @@ def create_oci_spec(
spec["root"]["path"] = rootfs
spec["root"]["readonly"] = readonly

# Docker parity: runsc mounts a private tmpfs over an *empty*
# /tmp, breaking rename(2) from /tmp with EXDEV. The placeholder
# keeps /tmp on the rootfs and stays visible in the sandbox (one
# empty dotfile in scratch space); readonly sandboxes get a tmpfs
# below, which hides it.
tmp_dir = os.path.join(rootfs, "tmp")
try:
os.makedirs(tmp_dir, exist_ok=True)
# Unconditional: /tmp must be world-writable with the sticky bit
# whether it came from the image or was just created.
os.chmod(tmp_dir, 0o1777)
Comment thread
xyuzh marked this conversation as resolved.
with open(
os.path.join(tmp_dir, ".ray-sandbox-keep"), "a", encoding="utf-8"
):
pass
except OSError:
# Best effort: runsc then falls back to its private tmpfs.
pass
Comment thread
xyuzh marked this conversation as resolved.
Comment thread
cursor[bot] marked this conversation as resolved.

spec.setdefault("process", {})
spec["process"]["args"] = ["sleep", "infinity"]
spec["process"]["cwd"] = container_cwd
Expand Down Expand Up @@ -454,6 +482,31 @@ def create_oci_spec(
)
existing_dests.add(_RESOLV_CONF)

# Read-write like the file container engines inject; the source
# is always a per-sandbox copy, never the node's own file.
if hosts_source and _ETC_HOSTS not in existing_dests:
mounts.append(
{
"destination": _ETC_HOSTS,
"type": "bind",
"source": hosts_source,
"options": ["rbind", "rw"],
}
)
existing_dests.add(_ETC_HOSTS)

# Keep /tmp writable on a readonly rootfs, as it is under Docker.
if readonly and "/tmp" not in existing_dests:
mounts.append(
{
"destination": "/tmp",
"type": "tmpfs",
"source": "tmpfs",
"options": ["nosuid", "nodev", "mode=1777"],
}
)
existing_dests.add("/tmp")

spec["mounts"] = mounts

# Configure OCI cgroup resource limits for CPU and memory
Expand Down Expand Up @@ -533,6 +586,22 @@ def prepare_oci_bundle(
elif os.path.exists(_RESOLV_CONF):
resolv_conf_source = _RESOLV_CONF

# Engines inject /etc/hosts at run time; without it `localhost`
# does not resolve. Host networking also inherits the node's entries.
hosts_source = os.path.join(root_dir, "hosts")
host_entries = ""
if network == "host" and os.path.exists(_ETC_HOSTS):
try:
with open(_ETC_HOSTS, "r", encoding="utf-8", errors="replace") as f:
host_entries = f.read()
except OSError:
host_entries = ""
Comment thread
xyuzh marked this conversation as resolved.
with open(hosts_source, "w", encoding="utf-8") as f:
f.write("127.0.0.1\tlocalhost\n")
f.write("::1\tlocalhost ip6-localhost ip6-loopback\n")
if host_entries:
f.write(host_entries)

spec = self.create_oci_spec(
image=image,
container_cwd=container_cwd,
Expand All @@ -544,6 +613,7 @@ def prepare_oci_bundle(
capabilities=capabilities,
network=network,
resolv_conf_source=resolv_conf_source,
hosts_source=hosts_source,
_oci_spec_transform_fn=_oci_spec_transform_fn,
)

Expand Down
118 changes: 118 additions & 0 deletions python/ray/experimental/sandbox/tests/test_image_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,7 @@ def test_extract_tar_layer_preserves_mtimes(tmp_path):
with tarfile.open(fileobj=buf, mode="w") as tar:
dir_info = tarfile.TarInfo("etc")
dir_info.type = tarfile.DIRTYPE
dir_info.mode = 0o755 # TarInfo defaults to 0644; dir modes are honored now
dir_info.mtime = archived_mtime
tar.addfile(dir_info)
file_info = tarfile.TarInfo("etc/os-release")
Expand All @@ -542,5 +543,122 @@ def test_extract_tar_layer_preserves_mtimes(tmp_path):
assert int(os.path.getmtime(dest / "etc")) == archived_mtime


def test_oci_spec_docker_parity_hosts_and_tmp(tmp_path):
"""/etc/hosts is a per-sandbox read-write bind (localhost must resolve),
and /tmp stays on the rootfs (readonly sandboxes get an explicit tmpfs
so it remains writable)."""
mgr = _StubImageManager(tmp_path)
hosts = tmp_path / "hosts"
hosts.write_text("127.0.0.1\tlocalhost\n")

spec = mgr.create_oci_spec(
image="fake:latest",
base_spec=_sample_base_spec(),
hosts_source=str(hosts),
readonly=True,
)
mounts = {m["destination"]: m for m in spec["mounts"]}
assert mounts["/etc/hosts"]["source"] == str(hosts)
assert "rw" in mounts["/etc/hosts"]["options"]
assert mounts["/tmp"]["type"] == "tmpfs"
# The rootfs /tmp was seeded so runsc keeps it on the rootfs device —
# and is world-writable + sticky regardless of how it was extracted.
assert (tmp_path / "rootfs" / "tmp" / ".ray-sandbox-keep").exists()
import stat

mode = stat.S_IMODE((tmp_path / "rootfs" / "tmp").stat().st_mode)
assert mode == 0o1777

spec = mgr.create_oci_spec(
image="fake:latest",
base_spec=_sample_base_spec(),
readonly=False,
)
dests = {m["destination"] for m in spec["mounts"]}
# Writable rootfs: /tmp is plain rootfs, same device — no tmpfs mount.
assert "/tmp" not in dests


def test_prepare_oci_bundle_writes_hosts_file(tmp_path, monkeypatch):
import ray.experimental.sandbox.image_manager as image_manager_mod

# The default spec shells out to runsc; substitute a static one so this
# runs on hosts without gVisor.
monkeypatch.setattr(image_manager_mod, "get_default_oci_spec", _sample_base_spec)
mgr = _StubImageManager(tmp_path)
bundle = tmp_path / "bundle"
bundle.mkdir()
mgr.prepare_oci_bundle(
root_dir=str(bundle),
workdir_path=None,
container_cwd="/",
image="fake:latest",
)
content = (bundle / "hosts").read_text()
assert "127.0.0.1\tlocalhost" in content
import json

spec = json.loads((bundle / "config.json").read_text())
dests = {m["destination"] for m in spec["mounts"]}
assert "/etc/hosts" in dests


def test_extract_tar_layer_applies_directory_modes(tmp_path):
"""Archived directory modes survive extraction: a 0755 root /tmp breaks
every non-root writer (apt-key first among them)."""
import io
import os
import stat
import tarfile

from ray.experimental.sandbox._internal.image_utils import extract_tar_layer

buf = io.BytesIO()
with tarfile.open(fileobj=buf, mode="w") as tar:
info = tarfile.TarInfo("tmp")
info.type = tarfile.DIRTYPE
info.mode = 0o1777
tar.addfile(info)

dest = tmp_path / "rootfs"
dest.mkdir()
extract_tar_layer(buf.getvalue(), str(dest))

assert stat.S_IMODE(os.stat(dest / "tmp").st_mode) == 0o1777


def test_extract_tar_layer_defers_restrictive_directory_modes(tmp_path):
"""Directory modes are applied after extraction, children first: tar
lists a directory before its contents, so applying a read-only archived
mode inline would break extracting the children."""
import io
import os
import stat
import tarfile

from ray.experimental.sandbox._internal.image_utils import extract_tar_layer

buf = io.BytesIO()
with tarfile.open(fileobj=buf, mode="w") as tar:
locked = tarfile.TarInfo("locked")
locked.type = tarfile.DIRTYPE
locked.mode = 0o500 # no write bit: inline chmod would break children
tar.addfile(locked)
inner = tarfile.TarInfo("locked/secret.txt")
data = b"contents"
inner.size = len(data)
inner.mode = 0o400
tar.addfile(inner, io.BytesIO(data))

dest = tmp_path / "rootfs"
dest.mkdir()
extract_tar_layer(buf.getvalue(), str(dest))

assert (dest / "locked" / "secret.txt").read_bytes() == b"contents"
assert stat.S_IMODE(os.stat(dest / "locked").st_mode) == 0o500
# Restore writability so pytest can clean the tmp dir up.
os.chmod(dest / "locked", 0o700)


if __name__ == "__main__":
sys.exit(pytest.main(["-v", __file__]))
Loading