fix(svc-prx-openresty): resolve OpenResty's own address in handlers notified from forced roles

web-app-matrix (ansible flavor) and web-app-bigbluebutton set
compose_mode_force: compose in their role vars, and those vars stay in
scope when their front-proxy includes notify the OpenResty handlers. In
swarm the handlers' container_address lookup then returned the bare
compose name, the validate retried for about 35 s against a container
that does not exist and ended ok, and the reload was skipped: 3 of 52
reloads in the WordPress#0 job of fork run 41926377524 (both matrix
.onion vhosts), so changed vhosts stayed unserved.

The validate, reload and restart handler tasks now clear
compose_mode_force at task level (task vars outrank the inherited role
vars), as the engine, rdbms and objstore entry points already do, and so
do the svc-db-openldap handlers, which the new lint also covers. The
proxy_pass lookup applies a caller's override only when the term names
the scope's own application_id, because the /logout location rendered
inside matrix and BBB vhosts pointed at 127.0.0.1 in swarm.

A real ansible-playbook integration test proves both modes and fails
without the fix; the new lint keeps every handler lookup of a fixed role
clearing the override.

🎯 Fix: 90 %
💥 Break: 4 %
🌱 Efficiency: –
⚡ Speed: –
🛡️ Security: ~100 %
🏆 Score: 86
🚦 Gate: ✅ 90/5, efficiency and speed <= 100, security 100 %

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
kevinveenbirkenbachandClaude Opus 5.5 committed 2026-11-05 11:40:41 +00:00
1 parent 56bcbb6d13
commit 7e5da9422c
8 files changed
+493 -1

No files matched your search

+12 -1
View File
@@ -1,5 +1,10 @@
"""Lookup ``proxy_pass``: thin wrapper emitting the mode-aware directive via
:func:`utils.networks.proxy.resolve_upstream` + :func:`render_proxy_pass`."""
:func:`utils.networks.proxy.resolve_upstream` + :func:`render_proxy_pass`.
Trip-wire: a role's ``compose_mode_force`` stays in scope while its vhost
renders locations of other apps (the ``/logout`` location of web-svc-logout),
so the override applies only when the term names the scope's own
``application_id``."""
from __future__ import annotations
@@ -54,9 +59,15 @@ class LookupModule(LookupBase):
real_deployment_mode = str(raw_mode).strip()
mode_force = vars_.get("compose_mode_force", "")
scope_application_id = vars_.get("application_id", "")
if templar is not None:
with contextlib.suppress(Exception):
mode_force = templar.template(mode_force)
with contextlib.suppress(Exception):
scope_application_id = templar.template(scope_application_id)
scope_application_id = _as_str(scope_application_id)
if scope_application_id and scope_application_id != application_id:
mode_force = ""
deployment_mode = _as_str(mode_force) or real_deployment_mode
applications = lookup_loader.get(
+10
View File
@@ -4,6 +4,8 @@
container exec -i {{ lookup('container_address', 'svc-db-openldap', 'openldap') }}
ldapmodify -Q -Y EXTERNAL -H {{ OPENLDAP_CONFIG_URI }}
-f "{{ [OPENLDAP_LDIF_PATH_DOCKER, 'configuration/01_member_of_configuration.ldif'] | path_join }}"
vars:
compose_mode_force: ""
listen:
- "import-configuration-ldif-files"
register: r
@@ -16,6 +18,8 @@
container exec -i {{ lookup('container_address', 'svc-db-openldap', 'openldap') }}
ldapmodify -Q -Y EXTERNAL -H {{ OPENLDAP_CONFIG_URI }}
-f "{{ [OPENLDAP_LDIF_PATH_DOCKER, 'configuration/04_refint_module.ldif'] | path_join }}"
vars:
compose_mode_force: ""
listen:
- "import-configuration-ldif-files"
register: r
@@ -28,6 +32,8 @@
container exec -i {{ lookup('container_address', 'svc-db-openldap', 'openldap') }}
ldapadd -Q -Y EXTERNAL -H {{ OPENLDAP_CONFIG_URI }}
-f "{{ [OPENLDAP_LDIF_PATH_DOCKER, 'configuration/02_member_of_configuration.ldif'] | path_join }}"
vars:
compose_mode_force: ""
listen:
- "import-configuration-ldif-files"
register: r
@@ -38,6 +44,8 @@
container exec -i {{ lookup('container_address', 'svc-db-openldap', 'openldap') }}
ldapadd -Q -Y EXTERNAL -H {{ OPENLDAP_CONFIG_URI }}
-f "{{ [OPENLDAP_LDIF_PATH_DOCKER, 'configuration/03_member_of_configuration.ldif'] | path_join }}"
vars:
compose_mode_force: ""
listen:
- "import-configuration-ldif-files"
register: r
@@ -53,6 +61,8 @@
-f "{{ [OPENLDAP_LDIF_PATH_DOCKER, 'groups', (item | basename | regex_replace('\.j2$', ''))] | path_join }}"'
environment:
OPENLDAP_BIND_PW: "{{ OPENLDAP_BIND_PW }}"
vars:
compose_mode_force: ""
listen:
- "import-groups-ldif-files"
register: ldapadd_result
@@ -8,6 +8,8 @@
- name: ✅ Validate OpenResty configuration before reloading
ansible.builtin.shell: >
container exec {{ lookup('container_address', 'svc-prx-openresty', 'openresty') }} openresty -t -q
vars:
compose_mode_force: ""
register: openresty_test_reload
changed_when: false
retries: 10
@@ -22,6 +24,8 @@
- name: ♻️ Reload OpenResty
ansible.builtin.shell: >
container exec {{ lookup('container_address', 'svc-prx-openresty', 'openresty') }} openresty -s reload
vars:
compose_mode_force: ""
register: openresty_reload
changed_when: openresty_reload.rc == 0
failed_when: >
@@ -20,6 +20,8 @@
- name: ✅ Validate OpenResty configuration (Post-Flight-Check)
ansible.builtin.shell: >
container exec {{ lookup('container_address', 'svc-prx-openresty', 'openresty') }} openresty -t -q
vars:
compose_mode_force: ""
register: openresty_test_post
changed_when: false
retries: 10
@@ -58,6 +60,8 @@
- ansible.builtin.shell: >
container logs --tail=200 {{ lookup('container_address', 'svc-prx-openresty', 'openresty') }}
vars:
compose_mode_force: ""
register: openresty_logs
changed_when: false
failed_when: false
@@ -0,0 +1,5 @@
from pathlib import Path
PROJECT_ROOT: Path = Path(__file__).resolve().parents[4]
__all__ = ["PROJECT_ROOT"]
@@ -0,0 +1,255 @@
"""The OpenResty handlers must address OpenResty at the cluster's own mode.
A role that pins ``compose_mode_force`` in its vars (web-app-matrix's ansible
flavor, web-app-bigbluebutton) keeps that value in scope for every role it
includes, and for the svc-prx-openresty handler copy that
``tasks/utils/load/handlers.yml`` loads inside that chain. The
``container_address`` lookup honours the caller's override, so in a swarm
cluster the handlers asked for the compose name of a container that only exists
as a swarm task, tolerated the "no such container" and skipped the reload.
These cases run real Ansible over the real handler files and the real lookup:
the forced role's own probe reproduces the override, so the handler assertions
cannot pass vacuously.
"""
from __future__ import annotations
import os
import shutil
import stat
import subprocess
import sys
import tempfile
import unittest
from pathlib import Path
from typing import ClassVar
from utils.cache.files import read_text
from utils.cache.yaml import dump_yaml
from utils.roles.mapping import ROLE_FILE_TASKS_MAIN, ROLE_FILE_VARS_MAIN
from . import PROJECT_ROOT
LOAD_HANDLERS = str(PROJECT_ROOT / "tasks" / "utils" / "load" / "handlers.yml")
OPENRESTY_LOOKUP = "{{ lookup('container_address', 'svc-prx-openresty', 'openresty') }}"
SWARM_MARK = "swarm-task"
MODES = ("swarm", "compose")
FAKE_CONTAINER = """#!/bin/sh
printf '%s\\n' "$*" >> "$FAKE_CONTAINER_LOG"
"""
FAKE_RESOLVER = f"""#!/bin/sh
printf '{SWARM_MARK}:%s:%s' "$1" "$2"
"""
def _load_handlers_task() -> dict:
return {
"name": "Load the OpenResty handlers",
"ansible.builtin.include_tasks": LOAD_HANDLERS,
"vars": {"handler_role_name": "svc-prx-openresty"},
}
ROLES: dict[str, dict[str, object]] = {
"probe-forced": {
ROLE_FILE_VARS_MAIN: {"compose_mode_force": "compose"},
ROLE_FILE_TASKS_MAIN: [
{
"name": "Record what the forced scope resolves for OpenResty",
"ansible.builtin.copy": {
"dest": "{{ forced_scope_out }}",
"content": OPENRESTY_LOOKUP,
"mode": "0644",
},
},
{
"name": "Serve a vhost the way sys-stk-front-proxy does",
"ansible.builtin.include_role": {"name": "probe-front-proxy"},
},
],
},
"probe-front-proxy": {
ROLE_FILE_TASKS_MAIN: [
{
"name": "Bootstrap the front base",
"ansible.builtin.include_role": {"name": "probe-front-base"},
},
{
"name": "Change a vhost",
"ansible.builtin.command": "true",
"changed_when": True,
"notify": "reload-openresty",
},
{
"name": "Change the base configuration",
"ansible.builtin.command": "true",
"changed_when": True,
"notify": "restart-openresty",
"when": "notify_restart | bool",
},
{
"name": "Run the notified handlers",
"ansible.builtin.meta": "flush_handlers",
},
],
},
"probe-front-base": {ROLE_FILE_TASKS_MAIN: [_load_handlers_task()]},
}
def _executable(path: Path, body: str) -> None:
path.write_text(body)
path.chmod(path.stat().st_mode | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH)
def _play(mode: str, resolver: Path, work: Path) -> dict:
"""Build the play for one cluster mode.
Args:
mode: the cluster ``DEPLOYMENT_MODE``, ``swarm`` or ``compose``.
resolver: the fake swarm container-id resolver.
work: the directory the play's output files land in.
"""
swarm = mode == "swarm"
return {
"name": f"OpenResty handlers notified from a forced role ({mode})",
"hosts": "localhost",
"gather_facts": False,
"connection": "local",
"environment": {"FAKE_CONTAINER_LOG": str(work / f"{mode}-container.log")},
"vars": {
"ansible_python_interpreter": sys.executable,
"DEPLOYMENT_MODE": mode,
"IS_COMPOSE_MODE": not swarm,
"IS_SWARM_MODE": swarm,
"IS_STACK_HOST": False,
"MODE_DEBUG": swarm,
"STACK_HOST": "localhost",
"BIN_RESOLVE_CONTAINER_ID": str(resolver),
"notify_restart": swarm,
"forced_scope_out": str(work / f"{mode}-forced-scope.txt"),
},
"tasks": [
{
"name": "Render the applications SPOT once, as the constructor does",
"applications_carrier": {},
},
_load_handlers_task(),
{
"name": "Run the forced role the way tasks/utils/setup/role.yml does",
"ansible.builtin.include_role": {"name": "probe-forced"},
"vars": {"application_id": "web-app-forced"},
},
],
}
def _run(work: Path) -> dict[str, tuple[str, list[str]]]:
"""Run one play per cluster mode in a single ansible-playbook process.
Args:
work: an empty directory for the fixture roles, play and fakes.
Returns:
Per mode ``(forced_scope, calls)``: what the forced role itself resolved
for OpenResty, and every argv the fake ``container`` CLI received.
Raises:
RuntimeError: when ansible-playbook exits non-zero.
"""
for role, files in ROLES.items():
for relative, content in files.items():
target = work / "roles" / role / relative
target.parent.mkdir(parents=True, exist_ok=True)
dump_yaml(target, content)
bin_dir = work / "bin"
bin_dir.mkdir()
_executable(bin_dir / "container", FAKE_CONTAINER)
resolver = bin_dir / "resolve-container-id"
_executable(resolver, FAKE_RESOLVER)
playbook = work / "play.yml"
dump_yaml(playbook, [_play(mode, resolver, work) for mode in MODES])
env = dict(os.environ)
env.update(
{
"ANSIBLE_CONFIG": str(PROJECT_ROOT / "ansible.cfg"),
"ANSIBLE_LOCAL_TEMP": str(work / "ansible-local"),
"ANSIBLE_REMOTE_TEMP": str(work / "ansible-remote"),
"ANSIBLE_NOCOLOR": "1",
"PATH": f"{bin_dir}{os.pathsep}{env['PATH']}",
}
)
result = subprocess.run(
["ansible-playbook", "-i", "localhost,", str(playbook)],
cwd=str(PROJECT_ROOT),
env=env,
capture_output=True,
text=True,
check=False,
)
if result.returncode != 0:
raise RuntimeError(
f"ansible-playbook failed\nSTDOUT:\n{result.stdout}\nSTDERR:\n{result.stderr}"
)
observed = {}
for mode in MODES:
log = read_text(str(work / f"{mode}-container.log"))
observed[mode] = (
read_text(str(work / f"{mode}-forced-scope.txt")),
[line for line in log.splitlines() if line],
)
return observed
def _targets(calls: list[str], verb: str) -> list[str]:
position = -1 if verb == "logs" else 1
return [line.split()[position] for line in calls if line.startswith(f"{verb} ")]
@unittest.skipUnless(shutil.which("ansible-playbook"), "ansible-playbook is required")
class TestOpenRestyHandlersIgnoreCallerModeForce(unittest.TestCase):
observed: ClassVar[dict[str, tuple[str, list[str]]]] = {}
@classmethod
def setUpClass(cls) -> None:
"""One ansible-playbook run answers every case, so it runs once."""
with tempfile.TemporaryDirectory(prefix="openresty-mode-force-") as tmp:
cls.observed = _run(Path(tmp))
def test_the_forced_scope_really_forces_the_lookup(self) -> None:
forced_scope, _ = self.observed["swarm"]
self.assertEqual(
forced_scope,
"openresty",
"the forced role must resolve OpenResty's compose name, or the "
"handler cases below prove nothing",
)
def test_swarm_reload_validates_and_reloads_the_swarm_task(self) -> None:
_, calls = self.observed["swarm"]
execs = [line for line in calls if line.startswith("exec ")]
self.assertIn("-s reload", " ".join(execs), calls)
targets = _targets(calls, "exec")
self.assertTrue(targets, calls)
for target in targets:
self.assertTrue(target.startswith(f"{SWARM_MARK}:"), calls)
def test_swarm_restart_debug_logs_read_the_swarm_task(self) -> None:
_, calls = self.observed["swarm"]
targets = _targets(calls, "logs")
self.assertTrue(targets, calls)
for target in targets:
self.assertTrue(target.startswith(f"{SWARM_MARK}:"), calls)
def test_compose_cluster_keeps_the_compose_name(self) -> None:
_, calls = self.observed["compose"]
targets = _targets(calls, "exec")
self.assertTrue(targets, calls)
self.assertEqual(set(targets), {"openresty"}, calls)
if __name__ == "__main__":
unittest.main()
@@ -0,0 +1,167 @@
"""A handler that resolves a fixed app must not inherit a caller's mode override.
A role that pins ``compose_mode_force`` in its vars (web-app-matrix's ansible
flavor, web-app-bigbluebutton) keeps it in scope for every handler copy loaded
inside its include chain. A lookup that reads the caller's override then
resolves the named app at the caller's mode instead of the mode that app runs
at: the OpenResty reload asked a swarm cluster for a compose container name and
skipped the reload.
So a handler task whose lookup names a fixed application clears the override
in its effective vars: its own ``vars:``, an enclosing block's, or those of the
``handlers/main.yml`` entry that includes its file by a literal path. A named
app that forces its own mode is exempt, since the override is then its own.
"""
from __future__ import annotations
import re
import unittest
from pathlib import Path
from typing import TYPE_CHECKING, Any
from utils.cache.files import iter_project_files, read_text
from utils.cache.yaml import load_yaml_any
from utils.roles.mapping import ROLE_FILE_VARS_MAIN
from . import PROJECT_ROOT
if TYPE_CHECKING:
from collections.abc import Iterator
ROLES = PROJECT_ROOT / "roles"
LOOKUPS = PROJECT_ROOT / "plugins" / "lookup"
READS_CALLER_OVERRIDE = re.compile(r"""\.get\(\s*["']compose_mode_force["']""")
SHARED_CONTEXT = re.compile(r"build_context\((?![^)]*honour_mode_force=False)")
INCLUDE_KEYS = (
"include_tasks",
"import_tasks",
"ansible.builtin.include_tasks",
"ansible.builtin.import_tasks",
)
BLOCK_KEYS = ("block", "rescue", "always")
def _honouring_lookups() -> set[str]:
names = set()
for path in LOOKUPS.glob("*.py"):
text = read_text(str(path))
if READS_CALLER_OVERRIDE.search(text) or SHARED_CONTEXT.search(text):
names.add(path.stem)
return names
def _forced_roles() -> set[str]:
forced = set()
for vars_main in ROLES.glob(f"*/{ROLE_FILE_VARS_MAIN}"):
data = load_yaml_any(str(vars_main), default_if_missing={})
if isinstance(data, dict) and str(data.get("compose_mode_force") or "").strip():
forced.add(vars_main.relative_to(ROLES).parts[0])
return forced
def _vars(node: dict) -> dict:
found = node.get("vars")
return found if isinstance(found, dict) else {}
def _included_vars(handlers_dir: Path) -> dict[Path, dict]:
entries = load_yaml_any(str(handlers_dir / "main.yml"), default_if_missing=[])
found: dict[Path, dict] = {}
for entry in entries if isinstance(entries, list) else []:
if not isinstance(entry, dict):
continue
for key in INCLUDE_KEYS:
target = entry.get(key)
if isinstance(target, dict):
target = target.get("file")
if isinstance(target, str) and "{{" not in target:
found[(handlers_dir / target).resolve()] = _vars(entry)
return found
def _tasks(items: Any, inherited: dict) -> Iterator[tuple[dict, dict]]:
for item in items if isinstance(items, list) else []:
if not isinstance(item, dict):
continue
scope = {**inherited, **_vars(item)}
bodies = [item[key] for key in BLOCK_KEYS if key in item]
if not bodies:
yield item, scope
for body in bodies:
yield from _tasks(body, scope)
def _handler_files() -> list[Path]:
found = []
for raw in iter_project_files(extensions=(".yml",), exclude_tests=True):
relative = Path(raw).relative_to(PROJECT_ROOT).parts
if len(relative) > 3 and relative[0] == "roles" and relative[2] == "handlers":
found.append(Path(raw))
return sorted(found)
def _strings(node: Any) -> Iterator[str]:
if isinstance(node, str):
yield node
elif isinstance(node, dict):
for value in node.values():
yield from _strings(value)
elif isinstance(node, list):
for value in node:
yield from _strings(value)
def _scan() -> tuple[list[str], int]:
names = "|".join(sorted(re.escape(name) for name in _honouring_lookups()))
call = re.compile(
rf"""lookup\(\s*['"](?P<plugin>{names})['"]\s*,\s*['"](?P<app>[a-z0-9][a-z0-9-]*)['"]"""
)
forced = _forced_roles()
included: dict[Path, dict[Path, dict]] = {}
offenders: list[str] = []
checked = 0
for path in _handler_files():
handlers_dir = ROLES / path.relative_to(ROLES).parts[0] / "handlers"
if handlers_dir not in included:
included[handlers_dir] = _included_vars(handlers_dir)
tasks = load_yaml_any(str(path), default_if_missing=[])
inherited = included[handlers_dir].get(path.resolve(), {})
for task, scope in _tasks(tasks, inherited):
for text in _strings(task):
for match in call.finditer(text):
app = match["app"]
if app in forced or not (ROLES / app).is_dir():
continue
checked += 1
if scope.get("compose_mode_force") != "":
offenders.append(
f"{path.relative_to(PROJECT_ROOT)}: "
f"{task.get('name', '<unnamed>')}: "
f"lookup('{match['plugin']}', '{app}', ...)"
)
return offenders, checked
class TestHandlerFixedAppLookupClearsModeForce(unittest.TestCase):
def test_container_address_is_known_to_honour_the_override(self) -> None:
self.assertIn("container_address", _honouring_lookups())
def test_every_fixed_app_lookup_in_a_handler_clears_the_override(self) -> None:
offenders, checked = _scan()
self.assertGreater(
checked, 0, "no handler lookup names a fixed app, so nothing was checked"
)
self.assertEqual(
offenders,
[],
f"{len(offenders)} handler lookup(s) resolve a fixed app under the "
f'caller\'s compose_mode_force. Add `compose_mode_force: ""` to the '
f"task's vars (or to the handlers/main.yml entry that includes it):\n"
+ "\n".join(offenders),
)
if __name__ == "__main__":
unittest.main()
@@ -29,6 +29,13 @@ def _apps() -> dict[str, Any]:
},
},
},
"web-svc-logout": {
"services": {
"logout": {
"ports": {"local": {"http": 8048}, "internal": {"http": 8000}},
},
},
},
}
@@ -194,6 +201,35 @@ class TestProxyPassLookup(unittest.TestCase):
)
self.assertEqual(out[0], "proxy_pass http://127.0.0.1:8021/;")
def test_a_forced_scope_leaves_a_foreign_app_at_the_cluster_mode(self):
out = _run(
["web-svc-logout", "logout", "http"],
variables={
"DEPLOYMENT_MODE": "swarm",
"compose_mode_force": "compose",
"application_id": "web-app-baserow",
},
tail="/logout",
)
self.assertEqual(
out[0],
'set $proxy_pass_upstream "logout:8000";\n'
" proxy_pass http://$proxy_pass_upstream/logout;",
)
def test_a_forced_scope_still_forces_its_own_app(self):
out = _run(
["web-app-baserow", "application", "http"],
variables={
"DEPLOYMENT_MODE": "swarm",
"compose_mode_force": "compose",
"application_id": "web-app-baserow",
},
tail="request",
location="/",
)
self.assertEqual(out[0], "proxy_pass http://127.0.0.1:8017/;")
def test_resolution_error_becomes_ansible_error(self):
lm = LookupModule()
lm._templar = _Templar({"DEPLOYMENT_MODE": "swarm"})