diff --git a/CHANGES.md b/CHANGES.md index 790937d..d85fcba 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -6,6 +6,20 @@ the one you find pypi, but its the one in development. dev ===== +* Fix `pystemd.run(wait=True)` / `wait_for_activation=True` hanging forever on hosts where no other client is subscribed to systemd signals +* Fix `pystemd.run` missing unit state changes when several D-Bus signals arrive at once +* Fix `pystemd.futures` on Python 3.14+, where the default multiprocessing start method is no longer fork +* Fix `pystemd.futures` ignoring `User=` on systemd >= 255 +* Fix `pystemd.futures.TransientUnitPoolExecutor` leaking its unit when the parent process is killed +* Add `PrivatePIDs`, `ProtectHostnameEx` and `SystemCallErrorNumber` to systemd1 KNOWN_UNIT_SIGNATURES +* Fix `pystemd.run(wait_for_activation=True)` hanging forever when the unit fails to start +* Add `FileDescriptorStoreMax` and `FileDescriptorStorePreserve` to systemd1 KNOWN_UNIT_SIGNATURES +* Add `fds=` to `pystemd.daemon.notify` (sd_pid_notify_with_fds) and add `pystemd.daemon.listen_fds_with_names` (file descriptor store / named socket activation) +* Add `pystemd.daemon.store_fd` and `pystemd.daemon.store_fds` helpers for the file descriptor store +* Split `pystemd.utils` into a package (`pystemd.utils.{x2,unit,typing}`); existing imports keep working +* Add `pystemd.utils.ns.join_namespaces` (atomic pidfd `setns`, with a per-namespace fallback); `pystemd.futures` uses it, which also fixes leaking one fd per namespace in every worker +* Add `delegate=` to `pystemd.run`, a shortcut for `extra={"Delegate": ...}` +* Add `OOMPolicy`, `OOMScoreAdjust` and the `ManagedOOM*` properties to systemd1 KNOWN_UNIT_SIGNATURES 0.15.3 ===== diff --git a/pystemd/daemon.pyi b/pystemd/daemon.pyi index a288b01..650ac68 100644 --- a/pystemd/daemon.pyi +++ b/pystemd/daemon.pyi @@ -5,7 +5,12 @@ # the root directory of this source tree. # -from typing import Union +from typing import Iterable, Mapping, Optional, Protocol, Union + +class _HasFileno(Protocol): + def fileno(self) -> int: ... + +_Fds = Union[int, _HasFileno, Iterable[Union[int, _HasFileno]]] LISTEN_FDS_START: int @@ -14,7 +19,19 @@ class PystemdDaemonError(Exception): def listen_fds(unset_environment: bool) -> int: ... def notify( - unset_environment: bool, *args: Union[str, bytes], **kwargs: Union[int, str] -) -> None: ... + unset_environment: bool, + *args: Union[str, bytes], + fds: Optional[_Fds] = None, + **kwargs: Union[int, str], +) -> int: ... +def listen_fds_with_names(unset_environment: bool = ...) -> dict[str, list[int]]: ... +def store_fd( + name: str, fds: _Fds, unset_environment: bool = ..., **kwargs: Union[int, str] +) -> int: ... +def store_fds( + fds_by_name: Mapping[str, _Fds], + unset_environment: bool = ..., + **kwargs: Union[int, str], +) -> int: ... def booted() -> bool: ... def watchdog_enabled(unset_environment: bool) -> int: ... diff --git a/pystemd/daemon.pyx b/pystemd/daemon.pyx index e7671f3..77f029b 100644 --- a/pystemd/daemon.pyx +++ b/pystemd/daemon.pyx @@ -9,6 +9,7 @@ from libc.stdint cimport uint64_t +from libc.stdlib cimport free, malloc cimport pystemd.dbusc as dbusc @@ -24,7 +25,39 @@ def listen_fds(int unset_environment=0): return dbusc.sd_listen_fds(unset_environment) -def notify(int unset_environment, *states, **kwstates): +def listen_fds_with_names(int unset_environment=0): + """ + interface to systemd sd_listen_fds_with_names: the file descriptors passed + to us (socket activation, or the file descriptor store handing back what we + stored with FDSTORE=1), as a dict of {name: [fd, ...]}. fds start at + LISTEN_FDS_START; names come from FileDescriptorName= / FDNAME=. + + Names are not unique: a socket unit with several ListenStream= passes all + its fds under the same name, and FDNAME= can be reused; hence the lists. + + usage: + * `fd = pystemd.daemon.listen_fds_with_names()["db"][0]` + * `for fd in pystemd.daemon.listen_fds_with_names().get("web.socket", []): ...` + """ + cdef char **names = NULL + cdef int n = dbusc.sd_listen_fds_with_names(unset_environment, &names) + if n < 0: + raise PystemdDaemonError(f"sd_listen_fds_with_names failed: {n}") + + fds = {} + try: + for i in range(n): + name = names[i].decode() if names != NULL else "unknown" + fds.setdefault(name, []).append(LISTEN_FDS_START + i) + return fds + finally: + if names != NULL: + for i in range(n): + free(names[i]) + free(names) + + +def notify(int unset_environment, *states, fds=None, **kwstates): """ interface to systemd sd_notify, @@ -34,6 +67,11 @@ def notify(int unset_environment, *states, **kwstates): states/kwstates = array with states statements that are 'IDENTIFIER=VALUE'. e.g: 'READY=1' or keywords states like `ready=1` (This will be clear in usage). + fds = optional file descriptor, or list of them (ints, or objects with a + fileno() method), to send along with the message (sd_pid_notify_with_fds). Mostly + for the file descriptor store: with `fdstore=1` systemd keeps them while + the service restarts and hands them back on the next start (see + listen_fds_with_names); the unit needs FileDescriptorStoreMax= > 0. usage: * `pystemd.daemon.notify(1, ready=1)`: this will signal `READY=1` to systemd @@ -44,18 +82,114 @@ def notify(int unset_environment, *states, **kwstates): * `pystemd.daemon.notify(False, ready=1, status='gime gime gime')`: will signal systemd that the app is ready and also set the status to 'gime gime gime'. This command does not clean the notify environment. + * `pystemd.daemon.notify(False, fds=[memfd], fdstore=1, fdname="db")`: + ask systemd to keep `memfd` for us, under the name "db". For info on sd_notify, check https://github.com/systemd/systemd/blob/\ master/src/systemd/sd-daemon.h#L173-L232 """ + state = _join_states(states, kwstates) + if fds is None: + return dbusc.sd_notify(unset_environment, state) + + fd_list = _fd_list(fds) + if not fd_list: + return dbusc.sd_notify(unset_environment, state) + + cdef unsigned n_fds = len(fd_list) + cdef int *c_fds = malloc(n_fds * sizeof(int)) + if c_fds == NULL: + raise MemoryError() + + try: + for i, fd in enumerate(fd_list): + c_fds[i] = fd + return dbusc.sd_pid_notify_with_fds(0, unset_environment, state, c_fds, n_fds) + finally: + free(c_fds) + + +def store_fd(name, fds, unset_environment=False, **kwstates): + """ + hand `fds` (a single fd or a list; ints or objects with fileno()) to + systemd's file descriptor store under `name`: systemd keeps them while the + service restarts, and listen_fds_with_names()[name] gives them back on the + next start. The unit needs FileDescriptorStoreMax= > 0. + + Same as notify(unset_environment, fds=fds, fdstore=1, fdname=name, **kwstates). + + usage: + * `pystemd.daemon.store_fd("db", memfd)` + * `pystemd.daemon.store_fd("listener", sock, ready=1)` + """ + _check_fdname(name) + return notify(unset_environment, fds=fds, fdstore=1, fdname=name, **kwstates) + + +def store_fds(fds_by_name, unset_environment=False, **kwstates): + """ + store_fd() for several names at once: `fds_by_name` is {name: fds}, where + fds is a single fd or a list. One FDNAME= covers a whole notify message, + so this sends one message per name (not atomically). `unset_environment` + and the extra states (e.g. ready=1) only go with the last message. + Returns the lowest notify() result (negative errno if any send failed). + + usage: + * `pystemd.daemon.store_fds({"listener": sock, "log": memfd}, ready=1)` + * `pystemd.daemon.store_fds({...}, True)` or `store_fds({...}, unset_environment=True)` + """ + items = list(fds_by_name.items()) + for name, _ in items: + _check_fdname(name) + if not items: + return notify(unset_environment, **kwstates) if kwstates else 0 + + results = [store_fd(name, fds) for name, fds in items[:-1]] + name, fds = items[-1] + results.append(store_fd(name, fds, unset_environment=unset_environment, **kwstates)) + return min(results) + + +def _check_fdname(name): + # systemd ignores invalid names *silently* (the fds are just not stored), + # so fail here instead: LISTEN_FDNAMES is ':'-separated, printable ASCII. + if ( + not isinstance(name, str) or not 0 < len(name) <= 255 + or ":" in name or not all(32 <= ord(c) < 127 for c in name) + ): + raise ValueError(f"invalid fd name {name!r}: 1-255 printable ASCII characters, no ':'") + + +def _fd_list(fds): + """ + normalize notify()'s `fds`: a single fd (int, or object with fileno()) or an + iterable of them, to a list of ints. + """ + if isinstance(fds, (str, bytes, bytearray)): + # bytes would iterate as ints, silently sending the wrong fds + raise TypeError(f"fds must be a file descriptor or a list of them, not {type(fds).__name__}") + if isinstance(fds, int) or hasattr(fds, "fileno"): + fds = [fds] + + fd_list = [] + for fd in fds: + if isinstance(fd, int): + fd_list.append(fd) + elif hasattr(fd, "fileno"): + fd_list.append(fd.fileno()) + else: + raise TypeError(f"not a file descriptor: {fd!r} (expected an int or an object with fileno())") + return fd_list + + +def _join_states(states, kwstates): pystates = [x2char_star(s) for s in states] pystates.extend( b'='.join([x2char_star(k.upper()), x2char_star(v, convert_all=True)]) for k, v in kwstates.items() ) - state = b'\n'.join(pystates) - return dbusc.sd_notify(unset_environment, state) + return b'\n'.join(pystates) def booted(): diff --git a/pystemd/dbusc.pxd b/pystemd/dbusc.pxd index a1134fa..333fa80 100644 --- a/pystemd/dbusc.pxd +++ b/pystemd/dbusc.pxd @@ -7,6 +7,7 @@ # the root directory of this source tree. # +from posix.types cimport pid_t from libc.stdint cimport ( int16_t, int32_t, @@ -37,6 +38,9 @@ cdef extern from "systemd/sd-daemon.h": int SD_LISTEN_FDS_START int sd_listen_fds(int unset_environment) int sd_notify(int unset_environment, const char *state) + int sd_pid_notify_with_fds(pid_t pid, int unset_environment, const char *state, + const int *fds, unsigned n_fds) + int sd_listen_fds_with_names(int unset_environment, char ***names) int sd_booted() int sd_watchdog_enabled(int unset_environment, uint64_t *usec) diff --git a/pystemd/futures.py b/pystemd/futures.py index db6b80e..d0c45e3 100644 --- a/pystemd/futures.py +++ b/pystemd/futures.py @@ -3,17 +3,15 @@ import os from concurrent.futures import ProcessPoolExecutor from functools import cached_property -from multiprocessing import Process -from multiprocessing.context import BaseContext -from pathlib import Path +from multiprocessing.context import BaseContext, ForkProcess from typing import Any, Sequence, cast import psutil -import pystemd.cutils import pystemd.run import pystemd.utils from pystemd.dbuslib import DBus +from pystemd.utils.ns import join_namespaces class TransientUnitContext(BaseContext): @@ -34,10 +32,12 @@ def __init__( self.properties = properties self.user_mode = user_mode self.unit_name = unit_name + # The unit's main process lives exactly as long as we do, so the unit + # is not leaked if we die without reaching stop_unit() (e.g. SIGTERM). self.main_process_cmd = main_process or [ "/bin/bash", "-c", - "exec sleep infinity", + f"exec tail --pid={os.getpid()} -f /dev/null", ] super().__init__() @@ -49,6 +49,11 @@ def start_unit(self) -> pystemd.systemd1.Unit: name=self.unit_name, user_mode=self.user_mode, wait_for_activation=True, + # "exec": only report the unit running once the main process has + # exec'd, i.e. once its user and namespaces are final. With + # "simple", enter_unit could read the uid of the not-yet-dropped + # executor (systemd >= 255) and keep running as root. + service_type="exec", extra={ **self.properties, "Delegate": True, @@ -72,7 +77,7 @@ def enter_unit(unit): after fork, but before code to run. It basically do: 1) Attach itself to the main transient unit (making it part of the cgroup) - 2) Change its namespace too all the namespaces of the main transient unit, this is best effort + 2) Join all the namespaces of the main transient unit (see pystemd.utils.ns.join_namespaces) 3) Change group and user to the group and user of the main transient unit """ # get the unit information @@ -84,18 +89,20 @@ def enter_unit(unit): unit.Service.AttachProcesses("/", [os.getpid()]) - # attach ourselves to the namespace - for ns in Path(f"/proc/{main_pid}/ns").iterdir(): - pystemd.cutils.setns(os.open(ns, os.O_RDONLY), 0) + # enter the unit's namespaces (PrivateNetwork=, PrivateTmp=, ...) + join_namespaces(main_pid) # change the user os.setgid(gid) os.setuid(uid) -class _ProcessWithPreRun(Process): +class _ProcessWithPreRun(ForkProcess): """ - Parent class that its just a subprocess, but it execute a pre_run hook + Parent class that its just a subprocess, but it execute a pre_run hook. + + Always forks (enter_unit relies on it), even on Python 3.14+ where the + default start method on Linux is no longer fork. """ def __init__(self, *args, **kwargs) -> None: diff --git a/pystemd/run.py b/pystemd/run.py index edd9116..8730deb 100644 --- a/pystemd/run.py +++ b/pystemd/run.py @@ -83,6 +83,7 @@ def run( stop_post_cmd: Sequence[str | bytes] | str | bytes | None = None, start_pre_cmd: Sequence[str | bytes] | str | bytes | None = None, start_post_cmd: Sequence[str | bytes] | str | bytes | None = None, + delegate: bool | None = None, ) -> Unit: """ pystemd.run imitates systemd-run, but with a pythonic feel to it. @@ -146,6 +147,9 @@ def run( stderr: Specify a file descriptor for stderr. By default this is `None` and your unit will not have a stderr. slice_: the slice under you want to run the unit. + delegate: Set `Delegate=` on the unit. True hands the unit's cgroup + subtree to the service (needed for `AttachProcesses`), False turns + it off. None, the default, leaves systemd's default. More info and examples in: https://github.com/systemd/pystemd/blob/master/_docs/pystemd.run.md @@ -263,6 +267,7 @@ def bus_factory(): b"CollectMode": b"inactive-or-failed" if collect else None, b"WorkingDirectory": cwd, b"User": user, + b"Delegate": delegate, b"Nice": nice, b"RuntimeMaxUSec": runtime_max_usec, b"Environment": [ @@ -301,6 +306,12 @@ def bus_factory(): monitor_fd = monbus.get_fd() sel.register(monitor_fd, EVENT_READ) + # systemd only emits unit PropertiesChanged signals while at least + # one client is subscribed. On a desktop someone usually is; on a + # minimal host nobody is, and we would wait forever. + # pyrefly: ignore [missing-argument] + manager.Manager.Subscribe() + # start the process # pyrefly: ignore [missing-argument] unit_start_job = manager.Manager.StartTransientUnit( @@ -330,29 +341,41 @@ def bus_factory(): # pyrefly: ignore [unbound-name] if monitor_fd in _in: - # pyrefly: ignore [unbound-name] - m = monbus.process() - if m.is_empty(): - continue - - m.process_reply(False) - if ( - # pyrefly: ignore [missing-attribute] - m.get_path() == unit.path - and m.body[0] == b"org.freedesktop.systemd1.Unit" - ): - _, message_job_path = m.body[1].get(b"Job", (0, b"/")) - if wait: - if ( - message_job_path != unit_start_job - and m.body[1].get(b"SubState") in EXIT_SUBSTATES - ): - break - elif wait_for_activation: - if message_job_path == unit_start_job and m.body[1].get( - b"SubState" - ) in (b"running",): - break + # Drain every message sd-bus has buffered: one read from the + # socket can bring several signals, and select() will not + # wake up again for the ones already read. + done = False + while not done: + # pyrefly: ignore [unbound-name] + m = monbus.process() + if m.is_empty(): + break + + m.process_reply(False) + if ( + # pyrefly: ignore [missing-attribute] + m.get_path() == unit.path + and m.body[0] == b"org.freedesktop.systemd1.Unit" + ): + _, message_job_path = m.body[1].get(b"Job", (0, b"/")) + sub_state = m.body[1].get(b"SubState") + if wait: + done = ( + message_job_path != unit_start_job + and sub_state in EXIT_SUBSTATES + ) + elif wait_for_activation: + # active, or the start job is over and the unit + # already exited (e.g. it failed to start) + done = ( + message_job_path == unit_start_job + and sub_state == b"running" + ) or ( + message_job_path != unit_start_job + and sub_state in EXIT_SUBSTATES + ) + if done: + break if _wait_polling and not _in and unit.Service.MainPID == 0: # on usermode the subscribe to events does not work that well diff --git a/pystemd/systemd1/unit_signatures.py b/pystemd/systemd1/unit_signatures.py index 76b5416..5362d0e 100644 --- a/pystemd/systemd1/unit_signatures.py +++ b/pystemd/systemd1/unit_signatures.py @@ -65,6 +65,8 @@ b"DynamicUser": b"b", b"Personality": b"s", b"Description": b"s", + b"FileDescriptorStoreMax": b"u", + b"FileDescriptorStorePreserve": b"s", b"NotifyAccess": b"s", b"BusName": b"s", b"RemainAfterExit": b"b", @@ -94,6 +96,7 @@ b"LockPersonality": b"b", b"SupplementaryGroups": b"as", b"SystemCallArchitectures": b"as", + b"SystemCallErrorNumber": b"i", b"SystemCallFilter": b"(bas)", # timeouts b"RuntimeMaxUSec": b"t", @@ -168,8 +171,10 @@ b"PrivateIPC": b"b", b"PrivateMounts": b"b", b"PrivateNetwork": b"b", + b"PrivatePIDs": b"s", b"PrivateTmp": b"b", b"PrivateUsers": b"b", + b"ProtectHostnameEx": b"(ss)", b"ProtectKernelTunables": b"b", b"ProtectKernelModules": b"b", b"ProtectControlGroups": b"b", @@ -233,6 +238,13 @@ b"MemoryMaxScale": b"u", b"MemorySwapMax": b"t", b"MemorySwapMaxScale": b"u", + b"OOMPolicy": b"s", + b"OOMScoreAdjust": b"i", + b"ManagedOOMSwap": b"s", + b"ManagedOOMMemoryPressure": b"s", + b"ManagedOOMMemoryPressureLimit": b"u", + b"ManagedOOMMemoryPressureDurationUSec": b"t", + b"ManagedOOMPreference": b"s", b"MemoryLimit": b"t", b"MemoryLimitScale": b"u", b"IOAccounting": b"b", diff --git a/pystemd/utils/__init__.py b/pystemd/utils/__init__.py new file mode 100644 index 0000000..eb2cd2a --- /dev/null +++ b/pystemd/utils/__init__.py @@ -0,0 +1,29 @@ +# +# Copyright (c) 2017-present, Facebook, Inc. +# All rights reserved. +# +# This source code is licensed under the license found in the LICENSE file in +# the root directory of this source tree. +# + +""" +Helpers used across pystemd. Everything that used to live in the +pystemd/utils.py module is re-exported here, so `from pystemd.utils import +x2char_star` (and friends) keeps working. +""" + +from pystemd.utils.ns import join_namespaces +from pystemd.utils.typing import T, unwrap +from pystemd.utils.unit import random_unit_name +from pystemd.utils.x2 import str2cmd, strlist2cmd, x2char_star, x2cmdlist + +__all__ = [ + "T", + "join_namespaces", + "random_unit_name", + "str2cmd", + "strlist2cmd", + "unwrap", + "x2char_star", + "x2cmdlist", +] diff --git a/pystemd/utils/ns.py b/pystemd/utils/ns.py new file mode 100644 index 0000000..4e11987 --- /dev/null +++ b/pystemd/utils/ns.py @@ -0,0 +1,106 @@ +# +# Copyright (c) 2017-present, Facebook, Inc. +# All rights reserved. +# +# This source code is licensed under the license found in the LICENSE file in +# the root directory of this source tree. +# + +""" +Linux namespaces helpers. +""" + +from __future__ import annotations + +import os +from typing import Iterable + +# CLONE_NEW* flags, defined here because os only has them from Python 3.12. +# The order is the one nsenter(1) joins them in: user first, mount late. +NAMESPACES = { + "user": 0x10000000, # CLONE_NEWUSER + "cgroup": 0x02000000, # CLONE_NEWCGROUP + "ipc": 0x08000000, # CLONE_NEWIPC + "uts": 0x04000000, # CLONE_NEWUTS + "net": 0x40000000, # CLONE_NEWNET + "pid": 0x20000000, # CLONE_NEWPID + "mnt": 0x00020000, # CLONE_NEWNS + "time": 0x00000080, # CLONE_NEWTIME +} + + +def join_namespaces(pid: int, namespaces: Iterable[str] | None = None) -> list[str]: + """ + Move the calling process into the namespaces of process `pid`, like + `nsenter --target pid --all`. Returns the names of the namespaces it + actually joined. + + `namespaces` is an iterable of names (keys of NAMESPACES, e.g. + ["net", "mnt"]); by default all of them. Namespaces we already share with + `pid` are skipped (re-joining your own user namespace is an error). + + Joining the pid namespace only affects children created afterwards, the + calling process keeps its pid. Needs the usual privileges (CAP_SYS_ADMIN, + or being in a parent user namespace of the target). + + Uses one atomic setns() on a pidfd (Python >= 3.12, Linux >= 5.8) and + falls back to joining one namespace at a time through /proc//ns/*. + Raises OSError if a namespace can't be joined. + """ + wanted = list(NAMESPACES) if namespaces is None else list(namespaces) + if unknown := set(wanted) - set(NAMESPACES): + raise ValueError( + f"unknown namespaces {sorted(unknown)}, expected some of {list(NAMESPACES)}" + ) + + to_join = [ + name + for name in NAMESPACES # keep nsenter's order + if name in wanted and _ns_id(pid, name) not in (None, _ns_id("self", name)) + ] + if not to_join: + return [] + + flags = 0 + for name in to_join: + flags |= NAMESPACES[name] + + try: + pidfd = os.pidfd_open(pid) + except (AttributeError, OSError): # Linux < 5.3 + pidfd = None + if pidfd is not None: + try: + _setns(pidfd, flags) # all at once, atomically + return to_join + except OSError: # Linux < 5.8 doesn't take pidfds in setns() + pass + finally: + os.close(pidfd) + + for name in to_join: + fd = os.open(f"/proc/{pid}/ns/{name}", os.O_RDONLY | os.O_CLOEXEC) + try: + _setns(fd, NAMESPACES[name]) + finally: + os.close(fd) + return to_join + + +def _ns_id(pid: int | str, name: str) -> int | None: + """The inode identifying a namespace, None if the kernel doesn't have it.""" + try: + return os.stat(f"/proc/{pid}/ns/{name}").st_ino + except FileNotFoundError: + return None + + +def _setns(fd: int, nstype: int) -> None: + if hasattr(os, "setns"): # Python >= 3.12 + os.setns(fd, nstype) + return + + import pystemd.cutils + + if pystemd.cutils.setns(fd, nstype) != 0: + raise OSError(f"setns(fd={fd}, nstype={nstype:#x}) failed") diff --git a/pystemd/utils/typing.py b/pystemd/utils/typing.py new file mode 100644 index 0000000..eceaa25 --- /dev/null +++ b/pystemd/utils/typing.py @@ -0,0 +1,19 @@ +# +# Copyright (c) 2017-present, Facebook, Inc. +# All rights reserved. +# +# This source code is licensed under the license found in the LICENSE file in +# the root directory of this source tree. +# + +from __future__ import annotations + +from typing import TypeVar + +T = TypeVar("T") + + +def unwrap(obj: T | None, msg="object was None") -> T: + if obj is None: + raise ValueError(msg) + return obj diff --git a/pystemd/utils/unit.py b/pystemd/utils/unit.py new file mode 100644 index 0000000..b42eff8 --- /dev/null +++ b/pystemd/utils/unit.py @@ -0,0 +1,15 @@ +# +# Copyright (c) 2017-present, Facebook, Inc. +# All rights reserved. +# +# This source code is licensed under the license found in the LICENSE file in +# the root directory of this source tree. +# + +from __future__ import annotations + +import uuid + + +def random_unit_name(*, unit_type="service", prefix="pystemd"): + return f"{prefix}{uuid.uuid4().hex}.{unit_type}" diff --git a/pystemd/utils.py b/pystemd/utils/x2.py similarity index 92% rename from pystemd/utils.py rename to pystemd/utils/x2.py index 3fb5ff3..b1b6c11 100644 --- a/pystemd/utils.py +++ b/pystemd/utils/x2.py @@ -9,11 +9,8 @@ from __future__ import annotations import shlex -import uuid from pathlib import Path -from typing import Any, Iterable, TypeVar - -T = TypeVar("T") +from typing import Any, Iterable def x2char_star(what_to_convert: Any, convert_all: bool = False) -> bytes: @@ -115,13 +112,3 @@ def x2cmdlist( return [strlist2cmd(what_to_convert, cont)] return [strlist2cmd(_, cont) for _ in what_to_convert] - - -def unwrap(obj: T | None, msg="object was None") -> T: - if obj is None: - raise ValueError(msg) - return obj - - -def random_unit_name(*, unit_type="service", prefix="pystemd"): - return f"{prefix}{uuid.uuid4().hex}.{unit_type}" diff --git a/setup.py b/setup.py index eed81a3..d919582 100644 --- a/setup.py +++ b/setup.py @@ -66,7 +66,13 @@ version="0.15.3", author="Alvaro Leiva Geisse", author_email="aleivag@gmail.com", - packages=["pystemd", "pystemd.systemd1", "pystemd.machine1", "pystemd.DBus"], + packages=[ + "pystemd", + "pystemd.systemd1", + "pystemd.machine1", + "pystemd.DBus", + "pystemd.utils", + ], ext_modules=external_modules, package_data={ "pystemd": [ diff --git a/tests/test_daemon.py b/tests/test_daemon.py index c0f42bf..25267ea 100644 --- a/tests/test_daemon.py +++ b/tests/test_daemon.py @@ -7,6 +7,7 @@ # the root directory of this source tree. # +import sys from unittest import TestCase from pystemd import daemon @@ -23,10 +24,63 @@ def test_notify(self): ) self.assertIn(notify_status, {0, 1}) + def test_notify_with_fds(self): + # without $NOTIFY_SOCKET this is a no-op that returns 0 + with open("/dev/null") as devnull: + notify_status = daemon.notify( + # pyrefly: ignore [bad-argument-type] + False, + fds=[devnull.fileno(), devnull], + fdstore=1, + fdname="test", + ) + self.assertIn(notify_status, {0, 1}) + + def test_notify_with_single_fd(self): + # a single fd (even 0) is a list of one, not "no fds" + for fd in (0, 1, sys.stdout): + # pyrefly: ignore [bad-argument-type] + self.assertIn(daemon.notify(False, fds=fd, ready=1), {0, 1}) + + def test_notify_fds_rejects_non_fds(self): + for bad in (b"\x03", "abc", [object()], [1.5]): + with self.assertRaises(TypeError): + # pyrefly: ignore [bad-argument-type] + daemon.notify(False, fds=bad, ready=1) + + +class TestStoreFds(TestCase): + # without $NOTIFY_SOCKET these are no-ops that return 0 + def test_store_fd(self): + self.assertIn(daemon.store_fd("one", 1), {0, 1}) + self.assertIn(daemon.store_fd("many", [1, sys.stderr], status="x"), {0, 1}) + + def test_store_fds(self): + self.assertIn(daemon.store_fds({"a": 1, "b": [1, 2]}, ready=1), {0, 1}) + self.assertEqual(daemon.store_fds({}), 0) + + def test_unset_environment_positional_or_keyword(self): + self.assertIn(daemon.store_fd("x", 1, False), {0, 1}) + self.assertIn(daemon.store_fd("x", 1, unset_environment=False), {0, 1}) + self.assertIn(daemon.store_fds({"x": 1}, False), {0, 1}) + self.assertIn(daemon.store_fds({"x": 1}, unset_environment=False), {0, 1}) + + def test_invalid_names(self): + for bad in ("", "a:b", "x" * 256, "tab\there", "ñ", None): + with self.assertRaises(ValueError): + # pyrefly: ignore [bad-argument-type] + daemon.store_fd(bad, 1) + with self.assertRaises(ValueError): + # pyrefly: ignore [bad-argument-type] + daemon.store_fds({"ok": 1, bad: 1}) + class TestListen(TestCase): def test_notify(self): self.assertTrue(daemon.listen_fds(False) >= 0) + def test_listen_fds_with_names(self): + self.assertIsInstance(daemon.listen_fds_with_names(False), dict) + def test_listen_fds_start(self): self.assertEqual(daemon.LISTEN_FDS_START, 3) diff --git a/tests/test_futures.py b/tests/test_futures.py index 15b208e..7a0918c 100644 --- a/tests/test_futures.py +++ b/tests/test_futures.py @@ -38,17 +38,13 @@ def test_run(self, TransientUnitContext: MagicMock) -> None: @patch("os.setuid", autospec=True) @patch("os.setgid", autospec=True) -@patch("os.open", autospec=True) -@patch("pystemd.cutils.setns", autospec=True) +@patch.object(pystemd.futures, "join_namespaces", autospec=True) @patch.object(pystemd.futures.psutil, "Process", autospec=True) -@patch.object(pystemd.futures, "Path", autospec=True) class TestEnterUnit(unittest.TestCase): def test_enter( self, - Path: MagicMock, Process: MagicMock, - setns: MagicMock, - os_open: MagicMock, + join_namespaces: MagicMock, os_setgid: MagicMock, os_setuid: MagicMock, ) -> None: @@ -62,7 +58,7 @@ def test_enter( Process.assert_called_once_with(main_pid) unit.Service.AttachProcesses.assert_called_once_with("/", [os.getpid()]) - Path.assert_called_once_with(f"/proc/{main_pid}/ns") + join_namespaces.assert_called_once_with(main_pid) os_setgid.assert_called_once_with(gid) os_setuid.assert_called_once_with(uid) diff --git a/tests/test_ns.py b/tests/test_ns.py new file mode 100644 index 0000000..f06c5d2 --- /dev/null +++ b/tests/test_ns.py @@ -0,0 +1,34 @@ +#!/usr/bin/env python3 +# +# Copyright (c) 2017-present, Facebook, Inc. +# All rights reserved. +# +# This source code is licensed under the license found in the LICENSE file in +# the root directory of this source tree. +# + +import os +from unittest import TestCase + +from pystemd.utils import join_namespaces as reexported +from pystemd.utils.ns import NAMESPACES, join_namespaces + + +class TestJoinNamespaces(TestCase): + def test_own_namespaces_is_a_noop(self): + # we share every namespace with ourselves: nothing to join, no privileges needed + self.assertEqual(join_namespaces(os.getpid()), []) + self.assertEqual(join_namespaces(os.getpid(), ["net", "mnt"]), []) + + def test_unknown_namespace(self): + with self.assertRaises(ValueError): + join_namespaces(os.getpid(), ["net", "nope"]) + + def test_names(self): + self.assertEqual( + set(NAMESPACES), + {"user", "cgroup", "ipc", "uts", "net", "pid", "mnt", "time"}, + ) + + def test_reexported(self): + self.assertIs(reexported, join_namespaces) diff --git a/tests/test_run.py b/tests/test_run.py new file mode 100644 index 0000000..ff4762a --- /dev/null +++ b/tests/test_run.py @@ -0,0 +1,40 @@ +#!/usr/bin/env fbpython +# (c) Meta Platforms, Inc. and affiliates. Confidential and proprietary. + +from __future__ import annotations + +import unittest +from unittest.mock import MagicMock, patch + +import pystemd.run + +# pystemd.run is the function; the module it was defined in is kept here +run_module = pystemd.run.__module__ + + +@patch.object(run_module, "Unit", autospec=True) +@patch.object(run_module, "SDManager") +@patch.object(run_module, "DBus") +class TestRunDelegate(unittest.TestCase): + def unit_properties(self, SDManager: MagicMock, **kwargs: object) -> dict: + run_module.run(["/bin/true"], user_mode=False, **kwargs) + manager = SDManager.return_value.__enter__.return_value + (_name, _mode, properties), _ = manager.Manager.StartTransientUnit.call_args + return properties + + def test_delegate_default_is_not_sent(self, DBus, SDManager, Unit) -> None: + self.assertNotIn(b"Delegate", self.unit_properties(SDManager)) + + def test_delegate_true(self, DBus, SDManager, Unit) -> None: + self.assertIs(self.unit_properties(SDManager, delegate=True)[b"Delegate"], True) + + def test_delegate_false(self, DBus, SDManager, Unit) -> None: + self.assertIs( + self.unit_properties(SDManager, delegate=False)[b"Delegate"], False + ) + + def test_extra_still_wins(self, DBus, SDManager, Unit) -> None: + properties = self.unit_properties( + SDManager, delegate=True, extra={b"Delegate": False} + ) + self.assertIs(properties[b"Delegate"], False)