From fc3c27246b56358b49709ea6ae3c59a8553e1787 Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Tue, 22 Sep 2026 13:36:30 +0200 Subject: [PATCH 1/4] fix(svc-net-tor): give every onion port probe its own circuit Tor attaches a stream to an onion to the rendezvous circuit that matches its isolation and keeps it there until SocksTimeout; it never moves the stream to a fresh circuit. The probe connected without SOCKS authentication and gave up after 60s, so a dead rendezvous circuit would absorb both passes. That is the most likely reading of run 35661520542 (web-app-mastodon#1, public Tor), where :80 timed out twice right after curl had reached the same onion port. Authenticate every connect with fresh random username/password credentials. Tor accepts any and, under its default IsolateSOCKSAuth, builds a separate rendezvous circuit per credential set, so a retry can no longer land on the circuit that failed the attempt before. Co-Authored-By: Claude Opus 5 (1M context) --- roles/svc-net-tor/files/test/onion_ports.py | 26 ++++++++-- .../svc-net-tor/test_onion_ports_probe.py | 51 +++++++++++++++++-- 2 files changed, 67 insertions(+), 10 deletions(-) diff --git a/roles/svc-net-tor/files/test/onion_ports.py b/roles/svc-net-tor/files/test/onion_ports.py index a372b65d47..adb6a81c66 100755 --- a/roles/svc-net-tor/files/test/onion_ports.py +++ b/roles/svc-net-tor/files/test/onion_ports.py @@ -40,12 +40,15 @@ Env (rendered into test.env from templates/test.env.j2): from __future__ import annotations import os +import secrets import socket import sys import time -_GREETING_NO_AUTH = b"\x05\x01\x00" -_GREETING_OK = b"\x05\x00" +_GREETING_USER_PASS = b"\x05\x01\x02" +_GREETING_OK = b"\x05\x02" +_AUTH_VERSION = 0x01 +_AUTH_OK = b"\x01\x00" _CONNECT_TO_DOMAIN = b"\x05\x01\x00\x03" _REPLY_SUCCESS = 0x00 @@ -62,8 +65,16 @@ def _required(name: str, hint: str) -> str: def connect_through_socks(proxy: str, host: str, port: int, timeout: float) -> None: - """Open a SOCKS5 CONNECT to ``host:port`` and raise when it is refused.""" + """Open a SOCKS5 CONNECT to ``host:port`` and raise when it is refused. + + Every call authenticates with fresh random credentials. Tor accepts any and, + under its default IsolateSOCKSAuth, builds a separate rendezvous circuit for + each. Tor never moves an onion stream off the rendezvous circuit it attached + to, so a retry sharing the credentials of the failed attempt would wait on + the same dead circuit again. + """ proxy_host, _, proxy_port = proxy.rpartition(":") + token = secrets.token_hex(8).encode() try: sock = socket.create_connection((proxy_host, int(proxy_port)), timeout=timeout) except OSError as error: @@ -72,9 +83,14 @@ def connect_through_socks(proxy: str, host: str, port: int, timeout: float) -> N with sock: sock.settimeout(timeout) try: - sock.sendall(_GREETING_NO_AUTH) + sock.sendall(_GREETING_USER_PASS) if sock.recv(2) != _GREETING_OK: - raise ProbeError("socks proxy refused the no-auth method") + raise ProbeError("socks proxy refused the username/password method") + sock.sendall( + bytes([_AUTH_VERSION, len(token)]) + token + bytes([len(token)]) + token + ) + if sock.recv(2) != _AUTH_OK: + raise ProbeError("socks proxy rejected the isolation credentials") target = host.encode() sock.sendall( _CONNECT_TO_DOMAIN diff --git a/tests/unit/python/roles/svc-net-tor/test_onion_ports_probe.py b/tests/unit/python/roles/svc-net-tor/test_onion_ports_probe.py index e9ab2bb600..1c825cb070 100644 --- a/tests/unit/python/roles/svc-net-tor/test_onion_ports_probe.py +++ b/tests/unit/python/roles/svc-net-tor/test_onion_ports_probe.py @@ -30,14 +30,22 @@ PROBE = _load_probe() class _FakeSocks(threading.Thread): """A one-shot SOCKS5 proxy that replies with a scripted CONNECT code.""" - def __init__(self, reply_code: int | None, greeting: bytes = b"\x05\x00") -> None: + def __init__( + self, + reply_code: int | None, + greeting: bytes = b"\x05\x02", + auth_reply: bytes = b"\x01\x00", + ) -> None: super().__init__(daemon=True) self._reply_code = reply_code self._greeting = greeting + self._auth_reply = auth_reply self._server = socket.socket() self._server.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) self._server.bind(("127.0.0.1", 0)) self._server.listen(1) + self.offered = b"" + self.auth = b"" self.request = b"" @property @@ -47,16 +55,19 @@ class _FakeSocks(threading.Thread): def run(self) -> None: conn, _ = self._server.accept() - with conn: - conn.recv(3) + with self._server, conn: + self.offered = conn.recv(3) conn.sendall(self._greeting) - if self._greeting != b"\x05\x00": + if self._greeting != b"\x05\x02": + return + self.auth = conn.recv(512) + conn.sendall(self._auth_reply) + if self._auth_reply != b"\x01\x00": return self.request = conn.recv(512) if self._reply_code is None: return conn.sendall(bytes([0x05, self._reply_code, 0x00, 0x01])) - self._server.close() class TestConnectThroughSocks(unittest.TestCase): @@ -75,6 +86,36 @@ class TestConnectThroughSocks(unittest.TestCase): "the port must travel big-endian in the last two bytes", ) + def test_every_attempt_isolates_itself_with_fresh_credentials(self) -> None: + seen: list[bytes] = [] + for _ in range(2): + proxy = _FakeSocks(reply_code=0x00) + proxy.start() + PROBE.connect_through_socks(proxy.endpoint, "example.onion", 80, 5) + proxy.join(timeout=5) + self.assertEqual( + proxy.offered, + b"\x05\x01\x02", + "only username/password may be offered, or Tor may pick no-auth " + "and put the attempt on the shared circuit", + ) + self.assertEqual( + proxy.auth[0], 0x01, f"bad RFC 1929 version: {proxy.auth!r}" + ) + seen.append(proxy.auth) + self.assertNotEqual( + seen[0], + seen[1], + "a retry that reuses the credentials reuses the dead rendezvous circuit", + ) + + def test_rejected_credentials_fail(self) -> None: + proxy = _FakeSocks(reply_code=None, auth_reply=b"\x01\x01") + proxy.start() + with self.assertRaises(PROBE.ProbeError) as raised: + PROBE.connect_through_socks(proxy.endpoint, "example.onion", 25, 5) + self.assertIn("credentials", str(raised.exception)) + def test_a_refusal_reply_fails(self) -> None: proxy = _FakeSocks(reply_code=0x05) proxy.start() From 290d9e0d976e1ba7a8b43222780e29949a70fc58 Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Tue, 22 Sep 2026 13:37:26 +0200 Subject: [PATCH 2/4] fix(svc-net-tor): hand every forwarded onion port to the port probe test-e2e-cli loads test.env with `set -a && . test.env && set +a`. The unquoted `ONION_PORTS=25 80 110 ...` is not an assignment there but the command `80` with a one-word env prefix: bash prints `command not found`, leaves ONION_PORTS unset, and onion_ports.py reports "no onion-forwarded ports declared; nothing to probe". The port check passed vacuously in every tor job with more than one forwarded port (OpenResty, Listmonk, Pixelfed in run 35661520542) and only ran where a single port was declared. Pass the value through dotenv_quote, which quotes a space-separated value in both compose and swarm and keeps the env-file lint satisfied. A unit test renders the template in both modes and sources it the way test-e2e-cli does. Co-Authored-By: Claude Opus 5 (1M context) --- roles/svc-net-tor/templates/test.env.j2 | 2 +- .../svc-net-tor/test_test_env_sourcing.py | 75 +++++++++++++++++++ 2 files changed, 76 insertions(+), 1 deletion(-) create mode 100644 tests/unit/python/roles/svc-net-tor/test_test_env_sourcing.py diff --git a/roles/svc-net-tor/templates/test.env.j2 b/roles/svc-net-tor/templates/test.env.j2 index 0b58f45bb0..6e0bec9054 100644 --- a/roles/svc-net-tor/templates/test.env.j2 +++ b/roles/svc-net-tor/templates/test.env.j2 @@ -1,6 +1,6 @@ NGINX_SERVERS_DIR={{ lookup('nginx', 'directories.configuration.servers') }} ONION_HOST={{ lookup('config', 'svc-net-tor', 'services.tor.node') }} -ONION_PORTS={{ lookup('tor_ports') | map(attribute='onion_port') | unique | sort | join(' ') }} +ONION_PORTS={{ lookup('tor_ports') | map(attribute='onion_port') | unique | sort | join(' ') | dotenv_quote }} ONION_PORT_RETRIES=2 ONION_PORT_TIMEOUT=60 RETRIES=5 diff --git a/tests/unit/python/roles/svc-net-tor/test_test_env_sourcing.py b/tests/unit/python/roles/svc-net-tor/test_test_env_sourcing.py new file mode 100644 index 0000000000..e0a3f8ed8c --- /dev/null +++ b/tests/unit/python/roles/svc-net-tor/test_test_env_sourcing.py @@ -0,0 +1,75 @@ +"""The rendered test.env survives the shell that sources it. + +test-e2e-cli loads test.env with ``set -a && . test.env && set +a``. An unquoted +space-separated value there is not an assignment but a command with a one-word +environment prefix: ``ONION_PORTS=25 80`` runs ``80``, bash reports +``command not found`` and carries on, and the probe reads an empty port list as +nothing to check. +""" + +from __future__ import annotations + +import subprocess +import unittest + +from jinja2 import Environment, StrictUndefined, select_autoescape + +from plugins.filter.dotenv import FilterModule +from utils.cache.files import read_text + +from . import PROJECT_ROOT + +TEMPLATE = PROJECT_ROOT / "roles" / "svc-net-tor" / "templates" / "test.env.j2" +PORTS = [25, 80, 587] + + +def _lookup(name: str, *_args: str) -> object: + return { + "config": "example.onion", + "nginx": "/etc/nginx/conf.d/servers", + "tor_ports": [ + {"onion_port": port, "target": f"127.0.0.1:{port}"} for port in PORTS + ], + "tor_socks": "127.0.0.1:9050", + }[name] + + +def _render(mode: str) -> str: + env = Environment( + undefined=StrictUndefined, + autoescape=select_autoescape(default_for_string=False), + ) + env.filters.update(FilterModule().filters()) + return env.from_string(read_text(str(TEMPLATE))).render( + lookup=_lookup, + DEPLOYMENT_MODE=mode, + TOR_DNSMASQ_CONF="/etc/dnsmasq.d/tor-onion.conf", + ) + + +class TestTestEnvSourcing(unittest.TestCase): + def test_every_onion_port_reaches_the_probe(self) -> None: + for mode in ("compose", "swarm"): + with self.subTest(mode=mode): + result = subprocess.run( + [ + "bash", + "-c", + ( + "set -a && . /dev/stdin && set +a && " + 'printf %s "${ONION_PORTS-unset}"' + ), + ], + input=_render(mode), + capture_output=True, + text=True, + check=False, + ) + self.assertEqual( + result.stderr, "", "sourcing test.env must not run anything" + ) + self.assertEqual(result.stdout, " ".join(str(port) for port in PORTS)) + + +if __name__ == "__main__": + unittest.main() From cc762722175324c0a5019b1c189090e773593f9c Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Tue, 22 Sep 2026 17:06:05 +0200 Subject: [PATCH 3/4] fix(test-e2e-cli): fail the CLI test on a test.env line that fails run_one.yml sourced the rendered env with `set -a && . test.env && set +a`. Nothing aborted on a failing line, and `.` returned only the status of the file's last line, so a broken assignment in the middle printed a bash error to stderr and the test ran on with the variable unset. That is how `ONION_PORTS=25 80 ...` left the svc-net-tor port probe checking nothing. Source it as a standalone command under `set -e`. bash ignores errexit inside a file sourced as part of an && list, so `set -ea && . test.env` would not help; a unit test runs the real command from run_one.yml and fails on both the old form and that one. The env-file quoting lint told authors that spaces are fine unquoted. For a bash-sourced CLI test.env they are not; its message now points to dotenv_quote instead. Co-Authored-By: Claude Opus 5 (1M context) --- roles/test-e2e-cli/tasks/run_one.yml | 2 +- ...est_env_file_template_no_literal_quotes.py | 11 ++- .../test-e2e-cli/test_run_one_env_sourcing.py | 75 +++++++++++++++++++ 3 files changed, 83 insertions(+), 5 deletions(-) create mode 100644 tests/unit/python/roles/test-e2e-cli/test_run_one_env_sourcing.py diff --git a/roles/test-e2e-cli/tasks/run_one.yml b/roles/test-e2e-cli/tasks/run_one.yml index 9e0f9dbdea..5cd9ac913d 100644 --- a/roles/test-e2e-cli/tasks/run_one.yml +++ b/roles/test-e2e-cli/tasks/run_one.yml @@ -97,7 +97,7 @@ - name: 🚀 Run CLI test script for {{ application_id }} ansible.builtin.shell: - cmd: "set -a && . {{ _cli_stage_dir }}/test.env && set +a && timeout {{ _cli_timeout }} bash {{ _cli_test_script }}" + cmd: "set -ea; . {{ _cli_stage_dir }}/test.env; set +ea; timeout {{ _cli_timeout }} bash {{ _cli_test_script }}" executable: /bin/bash vars: _cli_timeout_base: "{{ lookup('role_tests', application_id, 'cli.timeout', default='') }}" diff --git a/tests/lint/ansible/jinja/test_env_file_template_no_literal_quotes.py b/tests/lint/ansible/jinja/test_env_file_template_no_literal_quotes.py index 6c225faea7..6c002fbc3c 100644 --- a/tests/lint/ansible/jinja/test_env_file_template_no_literal_quotes.py +++ b/tests/lint/ansible/jinja/test_env_file_template_no_literal_quotes.py @@ -7,7 +7,8 @@ breaks. Allowed forms: -* ``KEY=value`` (unquoted; spaces are fine in env-file syntax) +* ``KEY=value`` (unquoted; only for values without whitespace, because a + CLI ``test.env`` is sourced by bash and ``KEY=a b`` runs ``b``) * ``KEY={{ ... | dotenv_quote }}`` (explicit escape that produces the correctly escaped value for both modes) @@ -76,9 +77,11 @@ class TestEnvFileTemplateNoLiteralQuotes(unittest.TestCase): "double quotes. compose strips them, swarm preserves them " 'literally - the container sees `KEY="value"` instead of ' "`KEY=value` and `int()` / URL parsers break.\n\n" - "Fix: drop the surrounding quotes (env-file syntax allows " - "spaces unquoted), or use `KEY={{ ... | dotenv_quote }}` " - "for values with shell metachars / dollars. Mark with " + "Fix: drop the surrounding quotes when the value never holds " + "whitespace, shell metachars or dollars; otherwise use " + "`KEY={{ ... | dotenv_quote }}`. Never drop them from a value " + "with spaces in a CLI `test.env`: test-e2e-cli sources it with " + "bash, where `KEY=a b` runs `b` instead of assigning. Mark with " "`# nocheck: env-file-literal-quoted-value` only when the " "consumer specifically needs the literal quotes (very rare).\n\n" f"Offending lines:\n{formatted}" diff --git a/tests/unit/python/roles/test-e2e-cli/test_run_one_env_sourcing.py b/tests/unit/python/roles/test-e2e-cli/test_run_one_env_sourcing.py new file mode 100644 index 0000000000..ddaad0713b --- /dev/null +++ b/tests/unit/python/roles/test-e2e-cli/test_run_one_env_sourcing.py @@ -0,0 +1,75 @@ +"""A broken line in test.env fails the CLI test instead of vanishing into stderr. + +bash ignores ``set -e`` inside a file sourced as part of an ``&&`` list, so +``set -a && . test.env && set +a && ...`` ran on past any failing line and only +the exit code of the file's last line counted. ``ONION_PORTS=25 80`` ran ``80`` +there, left the variable unset, and the svc-net-tor port probe passed on an +empty list. +""" + +from __future__ import annotations + +import subprocess +import tempfile +import unittest +from pathlib import Path + +from jinja2 import Environment, StrictUndefined, select_autoescape + +from utils.cache.yaml import load_yaml_any + +from . import PROJECT_ROOT + +RUN_ONE = PROJECT_ROOT / "roles" / "test-e2e-cli" / "tasks" / "run_one.yml" + + +def _run_command() -> str: + for task in load_yaml_any(str(RUN_ONE)): + shell = task.get("ansible.builtin.shell") + if isinstance(shell, dict) and "test.env" in str(shell.get("cmd", "")): + return shell["cmd"] + raise AssertionError(f"no shell task sources test.env in {RUN_ONE}") + + +def _run(env: str, script: str) -> subprocess.CompletedProcess[str]: + with tempfile.TemporaryDirectory() as stage: + stage_dir = Path(stage) + (stage_dir / "test.env").write_text(env, encoding="utf-8") + (stage_dir / "test.sh").write_text(script, encoding="utf-8") + cmd = ( + Environment( + undefined=StrictUndefined, + autoescape=select_autoescape(default_for_string=False), + ) + .from_string(_run_command()) + .render( + _cli_stage_dir=stage, + _cli_timeout=30, + _cli_test_script=str(stage_dir / "test.sh"), + ) + ) + return subprocess.run( + ["bash", "-c", cmd], capture_output=True, text=True, check=False + ) + + +class TestRunOneEnvSourcing(unittest.TestCase): + def test_a_broken_line_fails_before_the_script_runs(self) -> None: + result = _run("A=1\nPORTS=25 80\nZ=2\n", "echo script-ran\n") + self.assertNotEqual( + result.returncode, + 0, + "a test.env line that fails must fail the CLI test; sourcing it " + "inside an && list makes bash ignore set -e", + ) + self.assertNotIn("script-ran", result.stdout) + self.assertIn("command not found", result.stderr) + + def test_a_valid_env_reaches_the_script_and_its_exit_code_counts(self) -> None: + result = _run('A=1\nPORTS="25 80"\n', 'printf %s "$PORTS"\nexit 3\n') + self.assertEqual(result.stdout, "25 80", "test.env values must be exported") + self.assertEqual(result.returncode, 3, result.stderr) + + +if __name__ == "__main__": + unittest.main() From cf08eed1f87f74c25b87bedacef814577c87529a Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Tue, 22 Sep 2026 17:12:13 +0200 Subject: [PATCH 4/4] fix(svc-bkp-volume-2-local): owe no dump for a provider nobody consumes databases.csv rows come only from consumers, through sys-svc-rdbms. A database provider deployed without one, like the lean svc-db-postgres#1 variant (mirror, tor, backup and postgres, nothing else), therefore has no row. baudolo copies its volume live and records it undumped, and the drill failed with "postgres_data (postgres) ... no sql dump" in fork run 35731501845, on a cluster that holds only what the engine creates itself. The new database_consumerless_volumes lookup names the data volume of every deployed provider that no deployed role consumes as a shared database; a dedicated database brings its own volume and does not count. test.env hands the list to verify/backup.sh, which notes those volumes instead of failing. Every other undumped database volume still fails, above all one whose consumer forgot to seed its row, since that consumer is deployed. Only the drill's verdict changes; what gets backed up does not, and the CLI tests run on dev, act and github runtimes only. The one case it now lets pass is a dev host still carrying the database of an app its inventory no longer lists: that data sits undumped in the provider volume and reads as NOTE instead of FAIL. A CI job always starts on a fresh host. Counting the databases in the copied data directory was rejected: the bootstrap set depends on the image, and postgis/postgis adds template_postgis to it. Co-Authored-By: Claude Opus 5 (1M context) --- .../lookup/database_consumerless_volumes.py | 72 +++++++++++ .../files/test/verify/backup.sh | 21 +++- .../templates/test.env.j2 | 1 + .../test_database_consumerless_volumes.py | 70 +++++++++++ .../test_database_consumerless_volumes.py | 117 ++++++++++++++++++ .../files/test_verify_backup.py | 92 ++++++++++++++ 6 files changed, 370 insertions(+), 3 deletions(-) create mode 100644 plugins/lookup/database_consumerless_volumes.py create mode 100644 tests/integration/ansible/lookups/test_database_consumerless_volumes.py create mode 100644 tests/unit/python/plugins/lookup/test_database_consumerless_volumes.py create mode 100644 tests/unit/python/roles/svc-bkp-volume-2-local/files/test_verify_backup.py diff --git a/plugins/lookup/database_consumerless_volumes.py b/plugins/lookup/database_consumerless_volumes.py new file mode 100644 index 0000000000..c18e3a557d --- /dev/null +++ b/plugins/lookup/database_consumerless_volumes.py @@ -0,0 +1,72 @@ +"""Name the data volume of every database provider on this host nobody consumes. + +A consumer is a deployed role whose merged config enables a shared database of +the provider's engine; a role with a dedicated database brings its own volume +and is no consumer of the provider. A provider without a consumer holds only +the databases its engine creates for itself, so no databases.csv row names it +and baudolo records its volume undumped. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, Any + +from ansible.errors import AnsibleError +from ansible.plugins.loader import lookup_loader +from ansible.plugins.lookup import LookupBase + +from utils.roles.applications.services.database import ( + RDBMS_SERVICE_KEYS, + get_database_service_config, + resolve_database_service_key, +) + +if TYPE_CHECKING: + from collections.abc import Sequence + +PROVIDER_VOLUME = "data" + + +def provider_of(engine: str) -> str: + return f"svc-db-{engine}" + + +class LookupModule(LookupBase): + def run( + self, + terms: Sequence[Any] | None, + variables: dict[str, Any] | None = None, + **kwargs: Any, + ) -> list[Any]: + if terms: + raise AnsibleError("database_consumerless_volumes: expects no terms.") + + vars_ = variables or getattr(self._templar, "available_variables", {}) or {} + group_names = [str(group) for group in vars_.get("group_names") or []] + templar = getattr(self, "_templar", None) + applications = lookup_loader.get( + "applications", loader=self._loader, templar=templar + ).run([], variables=vars_)[0] + volume = lookup_loader.get("volume", loader=self._loader, templar=templar) + + providers = {provider_of(engine) for engine in RDBMS_SERVICE_KEYS} + consumed: set[str] = set() + for role_id in group_names: + if role_id in providers: + continue + try: + engine = resolve_database_service_key(applications, role_id) + except ValueError as error: + raise AnsibleError(f"database_consumerless_volumes: {error}") from error + if engine and get_database_service_config(applications, role_id).get( + "shared" + ): + consumed.add(provider_of(engine)) + + return [ + [ + volume.run([provider, PROVIDER_VOLUME], variables=vars_)[0]["name"] + for provider in sorted(providers) + if provider in group_names and provider not in consumed + ] + ] diff --git a/roles/svc-bkp-volume-2-local/files/test/verify/backup.sh b/roles/svc-bkp-volume-2-local/files/test/verify/backup.sh index bae07a2ad0..8186ff0733 100755 --- a/roles/svc-bkp-volume-2-local/files/test/verify/backup.sh +++ b/roles/svc-bkp-volume-2-local/files/test/verify/backup.sh @@ -1,7 +1,8 @@ #!/usr/bin/env bash # Verify the newest backup generation is stored, non-empty, and dumped where a # dump was owed: a volume counts as payload with a files/ tree or a non-empty -# sql dump, and a volume the run recorded as a database must carry the dump. +# sql dump, and a volume the run recorded as a database must carry the dump +# unless its provider has no consumer on this host. # The generation's own manifest is the authority; a generation written before # baudolo wrote one is judged by the engine's on-disk footprint instead. # Requires REPO_DIR/NEWEST_GENERATION/BKP_TEST_REPO_ROOT from test.sh. @@ -11,6 +12,7 @@ set -euo pipefail : "${NEWEST_GENERATION:?}" : "${BKP_TEST_REPO_ROOT:?}" : "${BKP_TEST_PYTHON:?}" +: "${BKP_TEST_CONSUMERLESS_DB_VOLUMES?}" GEN_DIR="${REPO_DIR}/${NEWEST_GENERATION}" @@ -39,6 +41,19 @@ engine_of() { return 1 } +read -r -a CONSUMERLESS <<< "${BKP_TEST_CONSUMERLESS_DB_VOLUMES}" + +owe_dump() { + local volume="$1" engine="$2" idle + for idle in "${CONSUMERLESS[@]}"; do + if [[ "${idle}" == "${volume}" ]]; then + echo "NOTE: ${volume} (${engine}) belongs to a database provider no deployed app consumes; nothing owes a dump" + return 0 + fi + done + UNDUMPED+=("${volume} (${engine})") +} + MANIFEST_LINES="" if MANIFEST_LINES="$(PYTHONPATH="${BKP_TEST_REPO_ROOT}" "${BKP_TEST_PYTHON}" -m utils.recovery.manifest "${GEN_DIR}")"; then FROM_MANIFEST=true @@ -61,14 +76,14 @@ for vol_dir in "${VOLUME_DIRS[@]}"; do if [[ "${FROM_MANIFEST}" == "false" ]] \ && engine="$(engine_of "${vol_dir}/files")" \ && [[ -z "${has_sql}" ]]; then - UNDUMPED+=("${vol_dir##*/} (${engine})") + owe_dump "${vol_dir##*/}" "${engine}" fi done if [[ "${FROM_MANIFEST}" == "true" ]] && [[ -n "${MANIFEST_LINES}" ]]; then while IFS=$'\t' read -r volume engine; do if [[ -n "${volume}" ]]; then - UNDUMPED+=("${volume} (${engine})") + owe_dump "${volume}" "${engine}" fi done <<< "${MANIFEST_LINES}" fi diff --git a/roles/svc-bkp-volume-2-local/templates/test.env.j2 b/roles/svc-bkp-volume-2-local/templates/test.env.j2 index e28d7de49c..e534ec3611 100644 --- a/roles/svc-bkp-volume-2-local/templates/test.env.j2 +++ b/roles/svc-bkp-volume-2-local/templates/test.env.j2 @@ -1,5 +1,6 @@ ASYNC_ENABLED={{ ASYNC_ENABLED | bool | lower }} BKP_TEST_BACKUPS_DIR={{ DIR_BACKUPS }} +BKP_TEST_CONSUMERLESS_DB_VOLUMES={{ lookup('database_consumerless_volumes') | join(' ') | dotenv_quote }} BKP_TEST_HEALTH_SCRIPT={{ [ DIR_SCRIPTS_SYSTEMCTL, 'sys-ctl-hlth-container', 'script.sh' ] | path_join }} BKP_TEST_HEALTH_TIMEOUT={{ lookup('timeout', 1200) | int }} BKP_TEST_IS_STACK_HOST={{ IS_STACK_HOST | bool | lower }} diff --git a/tests/integration/ansible/lookups/test_database_consumerless_volumes.py b/tests/integration/ansible/lookups/test_database_consumerless_volumes.py new file mode 100644 index 0000000000..556a41b588 --- /dev/null +++ b/tests/integration/ansible/lookups/test_database_consumerless_volumes.py @@ -0,0 +1,70 @@ +"""Integration test: database_consumerless_volumes against the real role tree. + +The unit tests hand the lookup a literal ``shared: true``. The roles declare it +as ``{{ 'svc-db-postgres' in group_names }}``, so only a render through the real +applications lookup shows whether the predicate sees the rendered boolean. +""" + +from __future__ import annotations + +import unittest + +from ansible.parsing.dataloader import DataLoader +from ansible.template import Templar + +from plugins.lookup.applications import LookupModule as ApplicationsLookup +from plugins.lookup.applications import _reset_cache_for_tests +from plugins.lookup.database_consumerless_volumes import ( + LookupModule as ConsumerlessLookup, +) + +from . import PROJECT_ROOT + +ROLES_DIR = PROJECT_ROOT / "roles" +LEAN = ["svc-bkp-volume-2-local", "svc-db-postgres", "svc-net-tor", "web-svc-mirror"] + + +def _variables(applications: dict, group_names: list[str]) -> dict: + return { + "applications": applications, + "users": {}, + "DOMAIN_PRIMARY": "infinito.test", + "SYSTEM_EMAIL_DOMAIN": "infinito.test", + "DIR_COMPOSITIONS": "/opt/compose/", + "group_names": list(group_names), + } + + +def _consumerless(group_names: list[str]) -> list[str]: + _reset_cache_for_tests() + applications = ApplicationsLookup() + applications._templar = Templar(loader=DataLoader()) + merged = applications.run( + [], variables=_variables({}, group_names), roles_dir=str(ROLES_DIR) + )[0] + lookup = ConsumerlessLookup() + lookup._loader = DataLoader() + lookup._templar = Templar(loader=lookup._loader) + return lookup.run([], variables=_variables(merged, group_names))[0] + + +class TestDatabaseConsumerlessVolumesIntegration(unittest.TestCase): + @classmethod + def tearDownClass(cls) -> None: + _reset_cache_for_tests() + + def test_the_lean_postgres_variant_leaves_its_volume_unconsumed(self) -> None: + self.assertEqual(_consumerless(LEAN), ["postgres_data"]) + + def test_a_shared_postgres_consumer_claims_the_volume(self) -> None: + self.assertEqual(_consumerless([*LEAN, "web-app-mastodon"]), []) + + def test_a_mariadb_consumer_leaves_postgres_unconsumed(self) -> None: + self.assertEqual( + _consumerless([*LEAN, "svc-db-mariadb", "web-app-matomo"]), + ["postgres_data"], + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/python/plugins/lookup/test_database_consumerless_volumes.py b/tests/unit/python/plugins/lookup/test_database_consumerless_volumes.py new file mode 100644 index 0000000000..fb58888587 --- /dev/null +++ b/tests/unit/python/plugins/lookup/test_database_consumerless_volumes.py @@ -0,0 +1,117 @@ +"""Unit tests for ``plugins/lookup/database_consumerless_volumes.py``. + +Pins the consumer predicate: a deployed role consumes a provider when it +enables a shared database of that provider's engine. A dedicated database, a +provider itself, or a consumer of the other engine never counts. +""" + +import importlib.util +import unittest +import unittest.mock as mock + +from ansible.errors import AnsibleError + +from . import PROJECT_ROOT + + +def _load_module(rel_path: str, name: str): + path = PROJECT_ROOT / rel_path + spec = importlib.util.spec_from_file_location(name, str(path)) + mod = importlib.util.module_from_spec(spec) + assert spec and spec.loader + spec.loader.exec_module(mod) + return mod + + +def _database(engine: str, *, shared: bool = True) -> dict: + return {"services": {engine: {"enabled": True, "shared": shared}}} + + +class DatabaseConsumerlessVolumesTests(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.mod = _load_module( + "plugins/lookup/database_consumerless_volumes.py", + "lookup_database_consumerless_volumes", + ) + + def setUp(self): + self._original_loader = self.mod.lookup_loader + self._applications: dict = {} + + def _get(name, **_kwargs): + if name == "applications": + return mock.MagicMock(run=lambda *_a, **_k: [self._applications]) + return mock.MagicMock(run=self._volume_run) + + loader_mock = mock.MagicMock() + loader_mock.get.side_effect = _get + self.mod.lookup_loader = loader_mock + + def tearDown(self): + self.mod.lookup_loader = self._original_loader + + @staticmethod + def _volume_run(terms, **_kwargs): + provider, semantic = terms + return [{"name": f"{provider.removeprefix('svc-db-')}_{semantic}"}] + + def _run(self, group_names, applications, terms=()): + self._applications = applications + lk = self.mod.LookupModule() + lk._loader = mock.MagicMock() + lk._templar = mock.MagicMock() + return lk.run(list(terms), variables={"group_names": list(group_names)}) + + def test_terms_are_refused(self): + with self.assertRaises(AnsibleError): + self._run([], {}, terms=("postgres",)) + + def test_a_provider_without_a_consumer_is_named(self): + result = self._run( + ["svc-db-postgres", "web-svc-mirror"], + {"svc-db-postgres": _database("postgres")}, + ) + self.assertEqual(result, [["postgres_data"]]) + + def test_a_shared_consumer_claims_its_provider(self): + result = self._run( + ["svc-db-postgres", "web-app-matomo"], + {"web-app-matomo": _database("postgres")}, + ) + self.assertEqual(result, [[]]) + + def test_a_dedicated_database_claims_no_provider(self): + result = self._run( + ["svc-db-postgres", "web-app-a"], + {"web-app-a": _database("postgres", shared=False)}, + ) + self.assertEqual(result, [["postgres_data"]]) + + def test_a_consumer_claims_only_its_own_engine(self): + result = self._run( + ["svc-db-mariadb", "svc-db-postgres", "web-app-matomo"], + {"web-app-matomo": _database("mariadb")}, + ) + self.assertEqual(result, [["postgres_data"]]) + + def test_a_provider_that_is_not_deployed_is_not_named(self): + self.assertEqual(self._run(["web-app-a"], {}), [[]]) + + def test_a_role_with_two_database_services_fails_loudly(self): + with self.assertRaises(AnsibleError): + self._run( + ["svc-db-postgres", "web-app-a"], + { + "web-app-a": { + "services": { + "mariadb": {"enabled": True, "shared": True}, + "postgres": {"enabled": True, "shared": True}, + } + } + }, + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/python/roles/svc-bkp-volume-2-local/files/test_verify_backup.py b/tests/unit/python/roles/svc-bkp-volume-2-local/files/test_verify_backup.py new file mode 100644 index 0000000000..038d2b093f --- /dev/null +++ b/tests/unit/python/roles/svc-bkp-volume-2-local/files/test_verify_backup.py @@ -0,0 +1,92 @@ +"""verify/backup.sh owes no dump for a provider nobody on the host consumes. + +A database provider deployed without a consumer holds only the databases its +engine creates for itself. No databases.csv row names it, so baudolo copies the +volume live and records it undumped, and the drill failed on data nobody wrote. +Every other undumped database volume, above all one whose consumer forgot to +seed its row, stays a failure. +""" + +from __future__ import annotations + +import json +import os +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path + +from utils.recovery.layout import FILES_DIR, MANIFEST_FILE, MANIFEST_SCHEMA, SQL_DIR + +from . import PROJECT_ROOT + +SCRIPT = ( + PROJECT_ROOT + / "roles" + / "svc-bkp-volume-2-local" + / "files" + / "test" + / "verify" + / "backup.sh" +) +GENERATION = "20260922135655" + + +def _verify( + volume: str, engine: str, consumerless: str +) -> subprocess.CompletedProcess[str]: + with tempfile.TemporaryDirectory() as repo: + generation = Path(repo) / GENERATION + files = generation / volume / FILES_DIR + files.mkdir(parents=True) + (files / "PG_VERSION").write_text("17", encoding="utf-8") + (generation / MANIFEST_FILE).write_text( + json.dumps( + { + "schema": MANIFEST_SCHEMA, + "layout": {"files_dir": FILES_DIR, "sql_dir": SQL_DIR}, + "volumes": { + volume: {"database": True, "dumped": False, "engine": engine} + }, + } + ), + encoding="utf-8", + ) + env = { + **os.environ, + "REPO_DIR": repo, + "NEWEST_GENERATION": GENERATION, + "BKP_TEST_REPO_ROOT": str(PROJECT_ROOT), + "BKP_TEST_PYTHON": sys.executable, + "BKP_TEST_CONSUMERLESS_DB_VOLUMES": consumerless, + } + return subprocess.run( + ["bash", str(SCRIPT)], capture_output=True, text=True, env=env, check=False + ) + + +class TestVerifyBackup(unittest.TestCase): + def assert_passes(self, result: subprocess.CompletedProcess[str]) -> None: + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn("nothing owes a dump", result.stdout) + + def assert_fails(self, result: subprocess.CompletedProcess[str]) -> None: + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertIn("but no sql dump", result.stdout) + + def test_an_unconsumed_provider_owes_no_dump(self) -> None: + self.assert_passes(_verify("postgres_data", "postgres", "postgres_data")) + + def test_a_consumed_provider_owes_a_dump(self) -> None: + self.assert_fails(_verify("postgres_data", "postgres", "")) + + def test_only_the_named_volume_is_released(self) -> None: + self.assert_fails(_verify("mariadb_data", "mariadb", "postgres_data")) + + def test_a_name_is_matched_whole_not_as_a_prefix(self) -> None: + self.assert_fails(_verify("postgres_data", "postgres", "postgres")) + + +if __name__ == "__main__": + unittest.main()