From d48fcf7303a40d91da754b09aa05957c164ef20e Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Sat, 24 Oct 2026 15:35:09 +0200 Subject: [PATCH 01/17] perf(ansible): keep SSH masters for two minutes and drop the group name warning ControlPersist=20s closed the SSH master to a remote host whenever the controller worked longer than 20 s on its own, as it does while rendering the applications SPOT (up to 21 s), reaping async jobs or running localhost tasks, so the next task paid a fresh handshake. 120 s bridges those gaps; ServerAliveInterval still drops a dead connection within 45 s. force_valid_group_names = ignore keeps the role-named groups exactly as never did and only stops the 'Invalid characters were found in group names' warning every play printed. Co-Authored-By: Claude Opus 5.5 (1M context) --- ansible.cfg | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ansible.cfg b/ansible.cfg index 3d7e98e8e4..e11d4fa469 100644 --- a/ansible.cfg +++ b/ansible.cfg @@ -10,6 +10,7 @@ host_key_checking = True deprecation_warnings = True interpreter_python = auto_silent no_target_syslog = True +force_valid_group_names = ignore collections_paths = ./collections:~/.ansible/collections:/usr/share/ansible/collections roles_path = ./roles @@ -30,7 +31,7 @@ module_utils = ./utils task_output_limit = all [ssh_connection] -ssh_args = -o ControlMaster=auto -o ControlPersist=20s -o ControlPath=~/.ssh/ansible-%h-%p-%r -o ServerAliveInterval=15 -o ServerAliveCountMax=3 -o StrictHostKeyChecking=accept-new -o PreferredAuthentications=publickey,password,keyboard-interactive +ssh_args = -o ControlMaster=auto -o ControlPersist=120s -o ControlPath=~/.ssh/ansible-%h-%p-%r -o ServerAliveInterval=15 -o ServerAliveCountMax=3 -o StrictHostKeyChecking=accept-new -o PreferredAuthentications=publickey,password,keyboard-interactive pipelining = True usetty = False transfer_method = smart From 372142848bde133a23f123bc72174b9e9525eab5 Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Sat, 24 Oct 2026 18:01:19 +0200 Subject: [PATCH 02/17] test(web-app-stalwart): decide the IMAP parallelism instead of racing it The parallel-sessions test asserted that more than one of four sessions appended something, which a loaded host where one worker drains the shared iterator fails with nothing broken. The sessions now meet at a barrier before their first append, so a serialised pool still fails the assertion while a concurrent one always passes. (cherry picked from commit 657015852a) Co-Authored-By: Claude Opus 5.5 (1M context) --- .../files/test_migrate_from_mailu.py | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/tests/unit/python/roles/web-app-stalwart/files/test_migrate_from_mailu.py b/tests/unit/python/roles/web-app-stalwart/files/test_migrate_from_mailu.py index 2e7888e70e..ba062ea1c0 100644 --- a/tests/unit/python/roles/web-app-stalwart/files/test_migrate_from_mailu.py +++ b/tests/unit/python/roles/web-app-stalwart/files/test_migrate_from_mailu.py @@ -1,7 +1,9 @@ +import contextlib import importlib.util import os import sys import tempfile +import threading import time import unittest from pathlib import Path @@ -74,7 +76,23 @@ class _FakeImap: class _SlowImap(_FakeImap): + """A session that waits for its siblings before it appends anything. + + `ThreadPoolExecutor` promises a thread count, never that every worker + receives an item from a shared iterator, so counting the sessions that + happened to append is a race rather than a check. + """ + + def __init__(self, barrier=None, **kwargs): + super().__init__(**kwargs) + self._barrier = barrier + self._waited = False + def append(self, mailbox, flags, date_time, message): + if self._barrier is not None and not self._waited: + self._waited = True + with contextlib.suppress(threading.BrokenBarrierError): + self._barrier.wait(timeout=10) time.sleep(0.002) return super().append(mailbox, flags, date_time, message) @@ -141,7 +159,8 @@ class TestMigrateAccount(unittest.TestCase): (self.account / "cur" / "99.host:2,").write_bytes( b"Message-ID: <5@x>\r\n\r\nduplicate" ) - imaps = [_SlowImap() for _ in range(4)] + barrier = threading.Barrier(4) + imaps = [_SlowImap(barrier=barrier) for _ in range(4)] stats = self.m.AccountStats() self.m.migrate_account(imaps, self.account, stats) appended = [entry for imap in imaps for entry in imap.appended] From 6c31c3fd79e9a5eccf521ac4d980794734052dc5 Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Sat, 24 Oct 2026 18:07:19 +0200 Subject: [PATCH 03/17] fix(inventory): declare the mapache password the matrix aborts on The deploy validates every inventory key against the names core reads, and PLAYWRIGHT_MAPACHE_PASSWORD had lost its declaration when it left secrets.credentials, so every local compose deploy aborted with "'PLAYWRIGHT_MAPACHE_PASSWORD' is not read by the deploy". The persona role now declares it; the inventory keeps owning the value. (cherry picked from commit 323f3acfd8, playwright part only: web-svc-design does not exist on this branch) Co-Authored-By: Claude Opus 5.5 (1M context) --- roles/test-e2e-playwright/defaults/main.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/roles/test-e2e-playwright/defaults/main.yml b/roles/test-e2e-playwright/defaults/main.yml index eabbcd85f9..3eabaa62d7 100644 --- a/roles/test-e2e-playwright/defaults/main.yml +++ b/roles/test-e2e-playwright/defaults/main.yml @@ -21,6 +21,11 @@ # mask the result of the deploy that actually ran. # Param test_e2e_playwright_skip_roles: roles excluded from the run. # Param test_e2e_playwright_keep: keep the sidecar and cluster after the run. +# Param PLAYWRIGHT_MAPACHE_PASSWORD: password the provisioning spec sets on the +# mapache persona and every consumer spec logs in with. The inventory owns it +# rather than `secrets.credentials`, because a generated credential rotates +# between the matrix's two passes and pass 2 would then face the password +# pass 1 stored. # Param VARIANT_INDEX: 0-based matrix-deploy round id, pushed as an extra var by # `cli/administration/deploy/development/deploy.py` when the matrix # orchestrator runs. Defaults to 0 for dev / non-matrix invocations so @@ -40,4 +45,6 @@ test_e2e_playwright_only_roles: "{{ lookup('deployment').running }}" test_e2e_playwright_skip_roles: [] test_e2e_playwright_keep: false +PLAYWRIGHT_MAPACHE_PASSWORD: "" + VARIANT_INDEX: 0 From 84159801f29108149ac96e670eabe95b44ea4c3a Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Sat, 24 Oct 2026 18:58:12 +0200 Subject: [PATCH 04/17] fix(tasks): name looped includes without their loop variable Ansible renders a looped task's name before the loop variable exists, so the GitLab and Mastodon wait includes and the Keycloak realm RBAC group loop printed "<< error 1 - 'wait_container' is undefined >>" (or 'item') in their task header and raised "Encountered 1 template error" on every deploy. The names are static now; the per-item output still shows the container or group path through the included tasks and the loop label. Co-Authored-By: Claude Opus 5.5 (1M context) --- roles/web-app-gitlab/tasks/02_wait.yml | 2 +- roles/web-app-keycloak/tasks/update/08_rbac_groups_realm.yml | 2 +- roles/web-app-mastodon/tasks/03_wait.yml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/roles/web-app-gitlab/tasks/02_wait.yml b/roles/web-app-gitlab/tasks/02_wait.yml index 9e10da2d24..81e191f827 100644 --- a/roles/web-app-gitlab/tasks/02_wait.yml +++ b/roles/web-app-gitlab/tasks/02_wait.yml @@ -1,5 +1,5 @@ --- -- name: "๐Ÿ”Ž Wait until '{{ wait_container }}' container is healthy" +- name: "๐Ÿ”Ž Wait until every GitLab service container is healthy" ansible.builtin.include_tasks: 02_wait/wait_one.yml loop_control: loop_var: wait_container diff --git a/roles/web-app-keycloak/tasks/update/08_rbac_groups_realm.yml b/roles/web-app-keycloak/tasks/update/08_rbac_groups_realm.yml index 9db287036c..823098a45b 100644 --- a/roles/web-app-keycloak/tasks/update/08_rbac_groups_realm.yml +++ b/roles/web-app-keycloak/tasks/update/08_rbac_groups_realm.yml @@ -5,7 +5,7 @@ # task oauth2-proxy's allowed_groups check has no groups to match and every # group-gated app returns 403 for otherwise-authenticated users. -- name: "๐Ÿ” RBAC realm groups: provision groups + memberships for '{{ item.path }}'" +- name: "๐Ÿ” RBAC realm groups: provision groups + memberships per rbac app" ansible.builtin.script: cmd: "files/shell/rbac_realm/ensure_group_path_members.sh" executable: /bin/bash diff --git a/roles/web-app-mastodon/tasks/03_wait.yml b/roles/web-app-mastodon/tasks/03_wait.yml index 45f5936cef..3c39951647 100644 --- a/roles/web-app-mastodon/tasks/03_wait.yml +++ b/roles/web-app-mastodon/tasks/03_wait.yml @@ -1,5 +1,5 @@ --- -- name: "๐Ÿ”Ž Wait until '{{ wait_container }}' container is healthy" +- name: "๐Ÿ”Ž Wait until every Mastodon service container is healthy" ansible.builtin.include_tasks: 03_wait/wait_one.yml loop_control: loop_var: wait_container From 2515565e31bda97ff3b3f41bd1b49cc96c66e42d Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Sat, 24 Oct 2026 19:04:20 +0200 Subject: [PATCH 05/17] fix(sys-stk-front-proxy): probe the vhost itself, not where it redirects The reachability probe and the post-reload verify followed redirects. A vhost whose root redirects to a host the stack does not serve yet, such as icons.infinito.nexus answering 302 to docs.infinito.nexus before web-app-sphinx is deployed, made the probe fail the TLS handshake on the default server (ssl_reject_handshake), report -1, and -1 is a stale-config signature, so OpenResty was force-recreated on every deploy (echoserver full-6 and full-7). Both probes now stop at the vhost's own answer; 301 and 302 are in DEFAULT_OK, and roles with 303/307/308 declare them. A vhost OpenResty has not loaded still answers -1 or 404 without any redirect, so stale-config detection is unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) --- roles/sys-stk-front-proxy/tasks/main.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/roles/sys-stk-front-proxy/tasks/main.yml b/roles/sys-stk-front-proxy/tasks/main.yml index 581710e5da..646bf994dc 100644 --- a/roles/sys-stk-front-proxy/tasks/main.yml +++ b/roles/sys-stk-front-proxy/tasks/main.yml @@ -113,6 +113,7 @@ uri_probe: url: "{{ front_proxy_probe_url }}" status_code: "{{ front_proxy_probe_status_codes }}" + follow_redirects: none register: site_check failed_when: false changed_when: false @@ -133,6 +134,7 @@ uri_probe: url: "{{ front_proxy_probe_url }}" status_code: "{{ front_proxy_probe_status_codes }}" + follow_redirects: none register: front_proxy_reload_check failed_when: false changed_when: false From 352dceb0edf870f1320f7581ff0929eb5c375d70 Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Sat, 24 Oct 2026 19:06:35 +0200 Subject: [PATCH 06/17] perf(web-app-bigbluebutton): render the compose override once sys-svc-compose already renders the role's compose.override.yml.j2 for every app (04_files.yml, mode 0640, notify compose-up). manager_ops.yml rendered the same template a second time without a mode, which stack_host_template defaults to 0644, so every deploy flipped the mode, both writers reported changed and BigBlueButton's whole stack was force-recreated (58 s up plus the Greenlight wait in echoserver full-7). The template renders identically in both places: domain is a role var and service_name is set inside the template. The role's own writer is gone; the first deploy after this settles the mode to 0640 once. Co-Authored-By: Claude Opus 5.5 (1M context) --- roles/web-app-bigbluebutton/tasks/00_core/manager_ops.yml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/roles/web-app-bigbluebutton/tasks/00_core/manager_ops.yml b/roles/web-app-bigbluebutton/tasks/00_core/manager_ops.yml index afb9259865..8895906c56 100644 --- a/roles/web-app-bigbluebutton/tasks/00_core/manager_ops.yml +++ b/roles/web-app-bigbluebutton/tasks/00_core/manager_ops.yml @@ -25,12 +25,6 @@ BBB_COMPOSE_FILE_ORIGINE: "{{ [compose_repository_path, 'docker-compose.yml'] | path_join }}" BBB_COMPOSE_FILE_FINAL: "{{ [lookup('container', application_id, 'directories.instance'), 'compose.yml'] | path_join }}" -- name: โœ๏ธ Write compose.override.yml for BigBlueButton - stack_host_template: - src: compose.override.yml.j2 - dest: "{{ [lookup('container', application_id, 'directories.instance'), 'compose.override.yml'] | path_join }}" - notify: compose-up - - name: ๐Ÿ“ Create symbolic link from .env file to target location file: src: "{{ BBB_ENV_FILE_ORIGINE }}" From 6aac7efb5306d057829e39a93d4a9e78e11b0e17 Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Sat, 24 Oct 2026 19:09:33 +0200 Subject: [PATCH 07/17] perf(svc-bkp-volume-2-local): install baudolo once per host Every database seed included 01_install.yml unguarded, so the pinned backup-docker-to-local package was installed again for each consumer (15x per echoserver deploy) and the seed re-created DIR_SECRETS with mode 0755 each time (14x). The 0755 also fought the 0700 that sys-token-store and this role's core set, so the secrets directory was world-listable for stretches of every run and both sides reported changed. 01_install.yml now ensures DIR_SECRETS as root:root 0700 and sets run_once_svc_bkp_volume_2_local_install; the core and the seed include it only while that fact is undefined, following the run_once_ convention of sys-svc-certs. The old guard on backup_docker_to_local_folder was dead (defined nowhere). The reset only removes databases.csv, so ensuring the directory before it is safe. A new ansible-playbook process installs once again. Co-Authored-By: Claude Opus 5.5 (1M context) --- roles/svc-bkp-volume-2-local/tasks/00_core.yml | 10 +--------- roles/svc-bkp-volume-2-local/tasks/01_install.yml | 12 ++++++++++++ .../tasks/03_seed-database-to-backup.yml | 9 +-------- 3 files changed, 14 insertions(+), 17 deletions(-) diff --git a/roles/svc-bkp-volume-2-local/tasks/00_core.yml b/roles/svc-bkp-volume-2-local/tasks/00_core.yml index f722ad8c1f..2230c11636 100644 --- a/roles/svc-bkp-volume-2-local/tasks/00_core.yml +++ b/roles/svc-bkp-volume-2-local/tasks/00_core.yml @@ -14,20 +14,12 @@ loop_var: svc_bkp_volume_2_local_dependency - include_tasks: 01_install.yml - when: backup_docker_to_local_folder is not defined + when: run_once_svc_bkp_volume_2_local_install is not defined - name: "โ™ป๏ธ reset (if enabled) for {{ role_name}}" include_tasks: 02_reset.yml when: MODE_RESET | bool -- name: โœ… Ensure secrets directory exists (root only) - ansible.builtin.file: - path: "{{ DIR_SECRETS }}" - state: directory - owner: root - group: root - mode: "0700" - - name: โœ… Ensure databases.csv exists (root only) ansible.builtin.file: path: "{{ FILE_DATABASE_SECRETS }}" diff --git a/roles/svc-bkp-volume-2-local/tasks/01_install.yml b/roles/svc-bkp-volume-2-local/tasks/01_install.yml index 78939d60cc..1be7cbf0b4 100644 --- a/roles/svc-bkp-volume-2-local/tasks/01_install.yml +++ b/roles/svc-bkp-volume-2-local/tasks/01_install.yml @@ -16,3 +16,15 @@ loop_var: cmd_name changed_when: false when: MODE_ASSERT | bool + +- name: โœ… Ensure secrets directory exists (root only) + ansible.builtin.file: + path: "{{ DIR_SECRETS }}" + state: directory + owner: root + group: root + mode: "0700" + +- name: ๐Ÿ”ง Mark the baudolo install as done on this host + ansible.builtin.set_fact: + run_once_svc_bkp_volume_2_local_install: true diff --git a/roles/svc-bkp-volume-2-local/tasks/03_seed-database-to-backup.yml b/roles/svc-bkp-volume-2-local/tasks/03_seed-database-to-backup.yml index f5f1154808..f7da52467e 100644 --- a/roles/svc-bkp-volume-2-local/tasks/03_seed-database-to-backup.yml +++ b/roles/svc-bkp-volume-2-local/tasks/03_seed-database-to-backup.yml @@ -1,13 +1,6 @@ --- - include_tasks: 01_install.yml - -- name: "โœ… Ensure /etc/baudolo exists" - ansible.builtin.file: - path: "{{ DIR_SECRETS }}" - state: directory - owner: root - group: root - mode: "0755" + when: run_once_svc_bkp_volume_2_local_install is not defined - name: "๐Ÿ› BKP | Debug seed context (sanitized)" ansible.builtin.debug: From 9301220d05a30f3190a53795fc50f6cb9845cff3 Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Sat, 24 Oct 2026 19:28:31 +0200 Subject: [PATCH 08/17] perf(web-app-keycloak): reconcile per-app RBAC over the admin REST API The per-app RBAC groups and LDAP group mappers were reconciled by four kcadm shell scripts that started a JVM per call and re-synced every app on every deploy: 582 s for the mappers plus 324 s for the orphan purge on echoserver (39 apps). rbac_reconcile.py now talks to the admin REST API from inside the Keycloak network namespace (nsenter --net) with the admin credentials read from /proc//environ, so nothing secret reaches argv or output. It drops the shared ldap-roles mapper, purges root groups of undeployed apps, ensures each app group and mapper, and syncs a mapper only when the group is new, the mapper config drifted, or the fingerprint stored on the group (mapper payload, app rbac and domains, group-logic digest) differs. The per-app payload no longer carries groups.path.group: Keycloak 26.8.1 does not store that key, so a payload with it never matched the stored mapper and every deploy re-synced every app. The admin client lives in keycloak_admin.py; tasks/utils/rest_script.yml stages it with the script in a 0700 temp dir on KEYCLOAK_HOST_NODE, runs it and removes the dir in an always block. Evidence (local compose, slot 104): matrix iteration green in both variants and passes; update runs show synced=1 in 1.1 s, then synced=0 unchanged=1 in 0.81 s through the final delivery path, no staging dir left behind. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../files/python/keycloak_admin.py | 131 ++++++++ .../files/python/rbac_reconcile.py | 310 ++++++++++++++++++ .../delete_default_ldap_roles_mapper.sh | 30 -- .../per_app_mapper/purge/root_orphans.sh | 44 --- .../shell/per_app_mapper/reconcile_apps.sh | 147 --------- .../per_app_mapper/resolve_root_group_id.sh | 22 -- .../filter_plugins/rbac_per_app.py | 2 +- .../tasks/update/05_per_app_mappers.yml | 96 ++---- .../tasks/utils/rest_script.yml | 72 ++++ roles/web-app-keycloak/vars/main.yml | 48 ++- .../roles/web-app-keycloak/files/__init__.py | 5 + .../files/test_keycloak_admin.py | 79 +++++ .../files/test_rbac_reconcile.py | 226 +++++++++++++ 13 files changed, 880 insertions(+), 332 deletions(-) create mode 100644 roles/web-app-keycloak/files/python/keycloak_admin.py create mode 100755 roles/web-app-keycloak/files/python/rbac_reconcile.py delete mode 100755 roles/web-app-keycloak/files/shell/per_app_mapper/delete_default_ldap_roles_mapper.sh delete mode 100755 roles/web-app-keycloak/files/shell/per_app_mapper/purge/root_orphans.sh delete mode 100755 roles/web-app-keycloak/files/shell/per_app_mapper/reconcile_apps.sh delete mode 100755 roles/web-app-keycloak/files/shell/per_app_mapper/resolve_root_group_id.sh create mode 100644 roles/web-app-keycloak/tasks/utils/rest_script.yml create mode 100644 tests/unit/python/roles/web-app-keycloak/files/__init__.py create mode 100644 tests/unit/python/roles/web-app-keycloak/files/test_keycloak_admin.py create mode 100644 tests/unit/python/roles/web-app-keycloak/files/test_rbac_reconcile.py diff --git a/roles/web-app-keycloak/files/python/keycloak_admin.py b/roles/web-app-keycloak/files/python/keycloak_admin.py new file mode 100644 index 0000000000..5966d048f1 --- /dev/null +++ b/roles/web-app-keycloak/files/python/keycloak_admin.py @@ -0,0 +1,131 @@ +"""Keycloak admin REST client shared by the scripts beside this module. + +Exports: + KeycloakError: an HTTP error status, naming method, path and status but never the body. + container_credentials: the permanent admin read from ``/proc//environ`` of the Keycloak container. + Keycloak: client whose ``call`` paths are relative to ``/admin/realms//``. + ADMIN_ENV_KEYS: container environment keys of the permanent admin user and password. + PAGE: page size of every paged listing. + TOKEN_MARGIN: seconds before expiry at which the master realm token is renewed. +""" + +from __future__ import annotations + +import json +import threading +import time +import urllib.error +import urllib.parse +import urllib.request +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from pathlib import Path + +ADMIN_ENV_KEYS = ( + "KEYCLOAK_PERMANENT_ADMIN_USERNAME", + "KEYCLOAK_PERMANENT_ADMIN_PASSWORD", +) +PAGE = 500 +TOKEN_MARGIN = 30 + + +class KeycloakError(Exception): + pass + + +def container_credentials(environ: Path) -> tuple[str, str]: + """Return the admin user and password from the Keycloak container's environment. + + Args: + environ: ``/proc//environ`` of the Keycloak container. + """ + with environ.open("rb") as handle: + raw = handle.read() + env = dict(item.split("=", 1) for item in raw.decode().split("\0") if "=" in item) + user_key, password_key = ADMIN_ENV_KEYS + return env[user_key], env[password_key] + + +class Keycloak: + """Admin REST client that renews its master realm token before it expires. + + Args: + url: Keycloak base URL. + realm: realm every ``call`` path is relative to. + user: master realm administrator. + password: that administrator's password. + """ + + def __init__(self, url: str, realm: str, user: str, password: str) -> None: + self.url = url.rstrip("/") + self.realm = realm + self._form = urllib.parse.urlencode( + { + "client_id": "admin-cli", + "grant_type": "password", + "username": user, + "password": password, + } + ).encode() + self._lock = threading.Lock() + self._token = "" + self._expires = 0.0 + + def _bearer(self) -> str: + with self._lock: + if time.monotonic() > self._expires - TOKEN_MARGIN: + request = urllib.request.Request( # noqa: S310 fixed local Keycloak origin + f"{self.url}/realms/master/protocol/openid-connect/token", + data=self._form, + ) + with urllib.request.urlopen(request, timeout=30) as response: # noqa: S310 fixed local Keycloak origin + body = json.load(response) + self._token = body["access_token"] + self._expires = time.monotonic() + int(body["expires_in"]) + return f"Bearer {self._token}" + + def call( + self, method: str, path: str, payload: object = None, timeout: int = 60 + ) -> tuple[object, str]: + """Return the decoded body and the id the ``Location`` header names. + + Args: + method: HTTP method. + path: path below ``/admin/realms//``. + payload: JSON body, or None for none. + timeout: seconds the request may take. + """ + data = b"" if payload is None else json.dumps(payload).encode() + request = urllib.request.Request( # noqa: S310 fixed local Keycloak origin + f"{self.url}/admin/realms/{self.realm}/{path}", + data=data if method in ("POST", "PUT") else None, + headers={ + "Authorization": self._bearer(), + "Content-Type": "application/json", + }, + method=method, + ) + try: + with urllib.request.urlopen(request, timeout=timeout) as response: # noqa: S310 fixed local Keycloak origin + raw = response.read() + location = response.headers.get("Location") or "" + except urllib.error.HTTPError as error: + raise KeycloakError(f"{method} {path}: HTTP {error.code}") from None + return (json.loads(raw) if raw else None), location.rsplit("/", 1)[-1] + + def children(self, group_id: str) -> list[dict]: + """Return every direct subgroup of a group, page by page. + + Args: + group_id: the parent group. + """ + found: list[dict] = [] + while True: + query = urllib.parse.urlencode( + {"first": len(found), "max": PAGE, "briefRepresentation": "true"} + ) + page, _ = self.call("GET", f"groups/{group_id}/children?{query}") + found.extend(page) + if len(page) < PAGE: + return found diff --git a/roles/web-app-keycloak/files/python/rbac_reconcile.py b/roles/web-app-keycloak/files/python/rbac_reconcile.py new file mode 100755 index 0000000000..7fa2525fea --- /dev/null +++ b/roles/web-app-keycloak/files/python/rbac_reconcile.py @@ -0,0 +1,310 @@ +#!/usr/bin/env python3 +"""Reconcile Keycloak's per-application RBAC groups and LDAP group mappers over the admin REST API. + +Env: + KC_URL: Keycloak base URL reachable from the network namespace the script runs in. + KC_PID: host PID of the Keycloak container, whose environment holds the admin credentials. + KC_REALM: realm holding the RBAC groups. + LDAP_COMPONENT_ID: LDAP user storage component id, parent of the group mappers. + RBAC_ROOT_PATH: path of the RBAC root group, e.g. ``/roles``. + DEPLOYED_APP_IDS: space-separated application ids whose groups stay under the root. + PAYLOADS_FILE: one `` `` line per app. + WORKERS: apps reconciled in parallel. + SYNC_ATTEMPTS: mapper sync attempts per app. + SYNC_DELAY: seconds between two sync attempts. + SYNC_TIMEOUT: seconds one sync attempt may take. + +Output: one ``-failed `` line per failed app, then one ``summary`` line. +Exit: 0 every app reconciled, 2 only mapper syncs failed, 1 otherwise. +""" + +from __future__ import annotations + +import base64 +import json +import os +import sys +import time +import urllib.parse +from concurrent.futures import ThreadPoolExecutor +from dataclasses import dataclass +from pathlib import Path + +from keycloak_admin import Keycloak, KeycloakError, container_credentials + +SHARED_MAPPER = "ldap-roles" +MAPPER_TYPE = "org.keycloak.storage.ldap.mappers.LDAPStorageMapper" +FINGERPRINT = "infinito.rbac.fingerprint" + + +class StepError(Exception): + def __init__(self, step: str, cause: Exception) -> None: + super().__init__(f"{step}: {cause}") + self.step = step + + +@dataclass +class App: + app_id: str + mapper_name: str + payload: str + fingerprint: str + + +def read_payloads(path: str) -> list[App]: + """Return the apps of a payloads file. + + Args: + path: file with one `` `` line per app. + """ + with Path(path).open(encoding="utf-8") as handle: + lines = handle.read().splitlines() + apps = [] + for line in lines: + if line.strip(): + app_id, mapper_name, encoded, fingerprint = line.split() + apps.append( + App( + app_id, mapper_name, base64.b64decode(encoded).decode(), fingerprint + ) + ) + return apps + + +def ldap_mappers(kc: Keycloak, ldap_id: str) -> dict[str, dict]: + """Return the LDAP group mappers by name. + + Args: + kc: client. + ldap_id: LDAP user storage component id. + """ + query = urllib.parse.urlencode({"parent": ldap_id, "type": MAPPER_TYPE}) + found, _ = kc.call("GET", f"components?{query}") + return {mapper["name"]: mapper for mapper in found} + + +def drop_shared_mapper(kc: Keycloak, mappers: dict[str, dict]) -> list[str]: + """Delete the shared mapper whose flat groups would shadow the per-app ones. + + Args: + kc: client. + mappers: LDAP group mappers by name. + + Returns: + One ``drop-failed`` line per mapper that could not be deleted. + """ + shared = mappers.pop(SHARED_MAPPER, None) + if shared is None: + return [] + try: + kc.call("DELETE", f"components/{shared['id']}") + except (KeycloakError, OSError) as error: + return [f"drop-failed {SHARED_MAPPER}: {error}"] + return [] + + +def purge_orphans( + kc: Keycloak, root_id: str, keep: set[str], workers: int +) -> tuple[dict[str, str], list[str]]: + """Delete root subgroups that belong to no deployed app. + + Args: + kc: client. + root_id: RBAC root group id. + keep: application ids whose groups stay. + workers: parallel deletes. + + Returns: + The kept subgroup ids by name, and one ``purge-failed`` line per failed delete. + """ + kept, orphans = {}, [] + for group in kc.children(root_id): + if group["name"] in keep: + kept[group["name"]] = group["id"] + else: + orphans.append(group) + + def delete(group: dict) -> str: + try: + kc.call("DELETE", f"groups/{group['id']}") + except (KeycloakError, OSError) as error: + return f"purge-failed {group['name']}: {error}" + return "" + + with ThreadPoolExecutor(max_workers=workers) as pool: + failures = [line for line in pool.map(delete, orphans) if line] + return kept, failures + + +def config_matches(current: dict, desired: dict) -> bool: + """True when every config key the payload sets already has its value. + + Args: + current: mapper as Keycloak returns it. + desired: mapper payload. + """ + have = current.get("config") or {} + return all(have.get(key) == value for key, value in desired["config"].items()) + + +class Reconciler: + """Bring one app's group, mapper and LDAP group import up to date. + + Args: + kc: client. + ldap_id: LDAP user storage component id. + root_id: RBAC root group id. + groups: existing app group ids by name. + mappers: existing LDAP group mappers by name. + sync_attempts: mapper sync attempts. + sync_delay: seconds between two attempts. + sync_timeout: seconds one attempt may take. + """ + + def __init__( + self, + kc: Keycloak, + ldap_id: str, + root_id: str, + groups: dict[str, str], + mappers: dict[str, dict], + sync_attempts: int, + sync_delay: float, + sync_timeout: int, + ) -> None: + self.kc = kc + self.ldap_id = ldap_id + self.root_id = root_id + self.groups = groups + self.mappers = mappers + self.sync_attempts = sync_attempts + self.sync_delay = sync_delay + self.sync_timeout = sync_timeout + + def _step(self, step: str, action, *args): + try: + return action(*args) + except (KeycloakError, OSError, ValueError, KeyError) as error: + raise StepError(step, error) from None + + def _ensure_group(self, app: App) -> tuple[str, bool]: + group_id = self.groups.get(app.app_id) + if group_id: + return group_id, False + _, group_id = self.kc.call( + "POST", f"groups/{self.root_id}/children", {"name": app.app_id} + ) + if not group_id: + raise KeycloakError("created group carries no id") + return group_id, True + + def _upsert_mapper(self, app: App) -> tuple[str, bool]: + desired = json.loads(app.payload) + current = self.mappers.get(app.mapper_name) + if current is None: + _, mapper_id = self.kc.call("POST", "components", desired) + if not mapper_id: + raise KeycloakError("created mapper carries no id") + return mapper_id, True + if config_matches(current, desired): + return current["id"], False + self.kc.call( + "PUT", f"components/{current['id']}", {**desired, "id": current["id"]} + ) + return current["id"], True + + def _sync(self, mapper_id: str) -> None: + path = f"user-storage/{self.ldap_id}/mappers/{mapper_id}/sync?direction=fedToKeycloak" + for attempt in range(1, self.sync_attempts + 1): + try: + self.kc.call("POST", path, timeout=self.sync_timeout) + except (KeycloakError, OSError): + if attempt == self.sync_attempts: + raise + time.sleep(self.sync_delay) + else: + return + + def _record(self, group_id: str, group: dict, fingerprint: str) -> None: + attributes = {**(group.get("attributes") or {}), FINGERPRINT: [fingerprint]} + self.kc.call("PUT", f"groups/{group_id}", {**group, "attributes": attributes}) + + def run(self, app: App) -> str: + """Return ``synced`` or ``unchanged`` for an app. + + Args: + app: the app to reconcile. + """ + group_id, created = self._step("ensure", self._ensure_group, app) + mapper_id, changed = self._step("upsert", self._upsert_mapper, app) + group, _ = self._step("read", self.kc.call, "GET", f"groups/{group_id}") + stored = ((group.get("attributes") or {}).get(FINGERPRINT) or [""])[0] + if not (created or changed or stored != app.fingerprint): + return "unchanged" + self._step("sync", self._sync, mapper_id) + self._step("record", self._record, group_id, group, app.fingerprint) + return "synced" + + +def main() -> int: + env = os.environ + workers = int(env["WORKERS"]) + try: + kc = Keycloak( + env["KC_URL"], + env["KC_REALM"], + *container_credentials(Path("/proc") / env["KC_PID"] / "environ"), + ) + mappers = ldap_mappers(kc, env["LDAP_COMPONENT_ID"]) + warnings = drop_shared_mapper(kc, mappers) + root, _ = kc.call( + "GET", + f"group-by-path/{urllib.parse.quote(env['RBAC_ROOT_PATH'].strip('/'))}", + ) + groups, purge_failures = purge_orphans( + kc, root["id"], set(env["DEPLOYED_APP_IDS"].split()), workers + ) + except (KeycloakError, OSError, KeyError, ValueError) as error: + print(f"listing-failed {error}") + return 1 + for line in warnings + purge_failures: + print(line, file=sys.stderr) + + reconciler = Reconciler( + kc, + env["LDAP_COMPONENT_ID"], + root["id"], + groups, + mappers, + int(env["SYNC_ATTEMPTS"]), + float(env["SYNC_DELAY"]), + int(env["SYNC_TIMEOUT"]), + ) + + def run(app: App) -> tuple[App, str]: + try: + return app, reconciler.run(app) + except StepError as error: + print(f"{app.app_id}: {error}", file=sys.stderr) + return app, f"{error.step}-failed" + + with ThreadPoolExecutor(max_workers=workers) as pool: + results = list(pool.map(run, read_payloads(env["PAYLOADS_FILE"]))) + + failed = [(app, outcome) for app, outcome in results if outcome.endswith("-failed")] + for app, outcome in failed: + print(f"{outcome} {app.app_id}") + counts = { + key: sum(1 for _, o in results if o == key) for key in ("synced", "unchanged") + } + print( + f"summary synced={counts['synced']} unchanged={counts['unchanged']} " + f"failed={len(failed)} purged-failures={len(purge_failures)}" + ) + if any(outcome != "sync-failed" for _, outcome in failed): + return 1 + return 2 if failed else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/roles/web-app-keycloak/files/shell/per_app_mapper/delete_default_ldap_roles_mapper.sh b/roles/web-app-keycloak/files/shell/per_app_mapper/delete_default_ldap_roles_mapper.sh deleted file mode 100755 index e5c39c52fb..0000000000 --- a/roles/web-app-keycloak/files/shell/per_app_mapper/delete_default_ldap_roles_mapper.sh +++ /dev/null @@ -1,30 +0,0 @@ -#!/bin/bash -# Delete the shared default 'ldap-roles' group-ldap-mapper that -# update/05_ldap/main.yml maintains for the assert/01_ldap.yml contract. -# The per-application mappers (ldap-roles-) replace it; leaving -# the default mapper in place would re-import every LDAP groupOfNames -# flat under /roles on every sync and shadow the per-app structure. -# -# Required env: -# KC_CONTAINER keycloak container name -# KC_REALM realm name -# LDAP_COMPONENT_ID Keycloak LDAP component id -set -o pipefail -: "${KC_CONTAINER:?KC_CONTAINER is required}" -: "${KC_REALM:?KC_REALM is required}" -: "${LDAP_COMPONENT_ID:?LDAP_COMPONENT_ID is required}" - -container exec -i "$KC_CONTAINER" /opt/keycloak/bin/kcadm.sh get components \ - -r "$KC_REALM" \ - -q "parentId=$LDAP_COMPONENT_ID" \ - -q providerId=group-ldap-mapper \ - --fields id,name --format csv --noquotes \ - | tr -d '\r' \ - | awk -F',' '$2=="ldap-roles"{print $1}' \ - > /tmp/kc_ldap_roles_ids.txt - -while IFS= read -r m_id; do - [ -z "$m_id" ] && continue - container exec -i "$KC_CONTAINER" /opt/keycloak/bin/kcadm.sh delete \ - "components/$m_id" -r "$KC_REALM" - or /roles/ collisions -# from earlier mapper configurations). Children whose name is one of -# the deployed application IDs are kept; everything else is deleted -# so the next per-app mapper sync rebuilds a clean tree. -# -# Required env: -# KC_CONTAINER keycloak container name -# KC_REALM realm name -# KC_RBAC_ROOT_PATH e.g. /roles -# DEPLOYED_APP_IDS space-separated list of application IDs -# KC_PURGE_WORKERS parallel kcadm deletes -set -o pipefail -: "${KC_CONTAINER:?KC_CONTAINER is required}" -: "${KC_REALM:?KC_REALM is required}" -: "${KC_RBAC_ROOT_PATH:?KC_RBAC_ROOT_PATH is required}" -: "${DEPLOYED_APP_IDS:?DEPLOYED_APP_IDS is required}" -: "${KC_PURGE_WORKERS:?KC_PURGE_WORKERS is required}" - -[ -z "$DEPLOYED_APP_IDS" ] && exit 0 - -container exec -i "$KC_CONTAINER" /opt/keycloak/bin/kcadm.sh get groups \ - -r "$KC_REALM" \ - -q max=500 --fields id,path --format csv --noquotes \ - > /tmp/kc_top_groups.txt - -ROOT_ID=$(awk -F',' -v p="$KC_RBAC_ROOT_PATH" \ - '!/^\[/ && !/Cgroup/ && $2==p {print $1; exit}' \ - /tmp/kc_top_groups.txt | tr -d '\r') -[ -z "$ROOT_ID" ] && exit 0 - -container exec -i "$KC_CONTAINER" /opt/keycloak/bin/kcadm.sh get \ - "groups/$ROOT_ID/children" -r "$KC_REALM" \ - -q max=500 --fields id,name --format csv --noquotes \ - > /tmp/kc_root_children.txt - -awk -F',' -v keep="$DEPLOYED_APP_IDS" ' - BEGIN { n=split(keep,a," "); for(i=1;i<=n;i++) ok[a[i]]=1 } - !/^\[/ && !/Cgroup/ && $2 != "" && !ok[$2] { print $1 } -' /tmp/kc_root_children.txt | tr -d '\r' > /tmp/kc_root_to_delete.txt - -xargs -r -P "$KC_PURGE_WORKERS" -I '{}' container exec -i "$KC_CONTAINER" /opt/keycloak/bin/kcadm.sh delete \ - "groups/{}" -r "$KC_REALM" < /tmp/kc_root_to_delete.txt || true # nocheck: shell-or-true -- grandfathered: worked in practice; TODO: sharpen to catch only the exact tolerated error diff --git a/roles/web-app-keycloak/files/shell/per_app_mapper/reconcile_apps.sh b/roles/web-app-keycloak/files/shell/per_app_mapper/reconcile_apps.sh deleted file mode 100755 index 08bdefa132..0000000000 --- a/roles/web-app-keycloak/files/shell/per_app_mapper/reconcile_apps.sh +++ /dev/null @@ -1,147 +0,0 @@ -#!/bin/bash -# Required env: -# KC_CONTAINER keycloak container name -# KC_REALM realm name -# LDAP_COMPONENT_ID Keycloak LDAP component id (mapper parentId) -# ROOT_GROUP_ID Keycloak group id of the RBAC root container -# PAYLOADS_FILE host-side file, one ` ` line per app -# GROUP_ID_PLACEHOLDER token in the mapper JSON that stands for the app group id -# KC_WORKERS apps reconciled in parallel -# KC_SYNC_ATTEMPTS mapper sync attempts per app -# KC_SYNC_DELAY seconds between two sync attempts -# KC_SYNC_TIMEOUT seconds one sync attempt may take -# -# Output: one `-failed ` line per app that failed. -# Exit: 0 every app reconciled, 2 only mapper syncs failed, 1 otherwise. -set -o pipefail -: "${KC_CONTAINER:?KC_CONTAINER is required}" -: "${KC_REALM:?KC_REALM is required}" -: "${LDAP_COMPONENT_ID:?LDAP_COMPONENT_ID is required}" -: "${ROOT_GROUP_ID:?ROOT_GROUP_ID is required}" -: "${PAYLOADS_FILE:?PAYLOADS_FILE is required}" -: "${GROUP_ID_PLACEHOLDER:?GROUP_ID_PLACEHOLDER is required}" -: "${KC_WORKERS:?KC_WORKERS is required}" -: "${KC_SYNC_ATTEMPTS:?KC_SYNC_ATTEMPTS is required}" -: "${KC_SYNC_DELAY:?KC_SYNC_DELAY is required}" -: "${KC_SYNC_TIMEOUT:?KC_SYNC_TIMEOUT is required}" -[ -r "$PAYLOADS_FILE" ] || { echo "PAYLOADS_FILE not readable: $PAYLOADS_FILE" >&2; exit 1; } -awk 'NF != 3 { bad = 1 } END { exit bad }' "$PAYLOADS_FILE" \ - || { echo "PAYLOADS_FILE has a line without exactly 3 fields: $PAYLOADS_FILE" >&2; exit 1; } - -KCADM=/opt/keycloak/bin/kcadm.sh -work="$(mktemp -d)" -trap 'rm -rf "$work"' EXIT -export KCADM work - -if ! container exec -i "$KC_CONTAINER" "$KCADM" get "groups/$ROOT_GROUP_ID/children" \ - -r "$KC_REALM" -q max=500 --fields id,name --format csv --noquotes \ - "$work/groups.csv"; then - echo "listing-failed groups/$ROOT_GROUP_ID/children" - exit 1 -fi - -if ! container exec -i "$KC_CONTAINER" "$KCADM" get components -r "$KC_REALM" \ - -q "parentId=$LDAP_COMPONENT_ID" -q providerId=group-ldap-mapper \ - -q max=500 --fields id,name --format csv --noquotes \ - "$work/mappers.csv"; then - echo "listing-failed components" - exit 1 -fi - -# shellcheck disable=SC2329,SC2317 # invoked indirectly through reconcile_steps -kcw() { - container exec -i "$KC_CONTAINER" "$KCADM" "$@" --config "$cfg" -} - -# Args: app_id, mapper_name, base64 mapper JSON. Sets failed_step on failure. -# shellcheck disable=SC2329,SC2317 # invoked indirectly through reconcile_app -reconcile_steps() { - local app="$1" mapper_name="$2" payload_b64="$3" - local payload="$work/$1.json" group_id mapper_id stale_id attempt=1 - - group_id="$(awk -F',' -v n="$app" '!/^\[/ && !/Cgroup/ && $2==n {print $1; exit}' \ - "$work/groups.csv" | tr -d '\r')" - if [ -z "$group_id" ]; then - group_id="$(kcw create "groups/$ROOT_GROUP_ID/children" -r "$KC_REALM" -s "name=$app" -i \ - "$work/$app.children" - while IFS= read -r stale_id; do - [ -z "$stale_id" ] && continue - kcw delete "groups/$stale_id" -r "$KC_REALM" "$payload" \ - || { failed_step=render; return 1; } - - mapper_id="$(awk -F',' -v n="$mapper_name" '!/^\[/ && !/Cgroup/ && $2==n {print $1; exit}' \ - "$work/mappers.csv" | tr -d '\r')" - if [ -n "$mapper_id" ]; then - sed "s|\"name\":|\"id\": \"$mapper_id\",\n \"name\":|" "$payload" \ - | kcw update "components/$mapper_id" -r "$KC_REALM" -f - \ - >/dev/null || true # nocheck: shell-or-true -- grandfathered: worked in practice; TODO: sharpen to catch only the exact tolerated error - else - mapper_id="$(kcw create components -r "$KC_REALM" -f - -i <"$payload" \ - | grep -vE '(^\[|Cgroup)' | grep -E '^[0-9a-fA-F-]{8,}$' | head -1 | tr -d '\r')" - fi - [ -n "$mapper_id" ] || { failed_step=upsert; return 1; } - - until timeout "$KC_SYNC_TIMEOUT" container exec -i "$KC_CONTAINER" "$KCADM" create \ - "user-storage/$LDAP_COMPONENT_ID/mappers/$mapper_id/sync?direction=fedToKeycloak" \ - -r "$KC_REALM" --config "$cfg" "$work/$app.sync" 2>&1; do - if [ "$attempt" -ge "$KC_SYNC_ATTEMPTS" ]; then - sed "s|^|$app sync: |" "$work/$app.sync" >&2 - failed_step=sync - return 1 - fi - attempt=$((attempt + 1)) - sleep "$KC_SYNC_DELAY" - done -} - -# Args: app_id, mapper_name, base64 mapper JSON. Leaves `.done` or `.failed` in $work. -# shellcheck disable=SC2329,SC2317 # invoked indirectly through xargs + bash -c -reconcile_app() { - local app="$1" cfg="/tmp/kcadm-$1-$$.config" failed_step="" - # shellcheck disable=SC2016 # $HOME and $1 expand in the container shell - if container exec -i "$KC_CONTAINER" sh -c 'cp "$HOME/.keycloak/kcadm.config" "$1"' sh "$cfg" "$work/$app.failed" - else - : >"$work/$app.done" - fi -} -export -f kcw reconcile_steps reconcile_app - -# shellcheck disable=SC2016 # the single-quoted body expands inside the child bash -xargs -r -n 3 -P "$KC_WORKERS" bash -c 'reconcile_app "$@"' _ <"$PAYLOADS_FILE" -xargs_rc=$? - -hard=0 -sync=0 -while read -r app _ || [ -n "$app" ]; do - [ -e "$work/$app.done" ] && continue - step=incomplete - [ -s "$work/$app.failed" ] && step="$(<"$work/$app.failed")" - printf '%s-failed %s\n' "$step" "$app" - if [ "$step" = sync ]; then sync=1; else hard=1; fi -done <"$PAYLOADS_FILE" - -if [ "$xargs_rc" -ne 0 ]; then - echo "xargs exited $xargs_rc" >&2 - hard=1 -fi -[ "$hard" -eq 1 ] && exit 1 -[ "$sync" -eq 1 ] && exit 2 -exit 0 diff --git a/roles/web-app-keycloak/files/shell/per_app_mapper/resolve_root_group_id.sh b/roles/web-app-keycloak/files/shell/per_app_mapper/resolve_root_group_id.sh deleted file mode 100755 index 6ab598e467..0000000000 --- a/roles/web-app-keycloak/files/shell/per_app_mapper/resolve_root_group_id.sh +++ /dev/null @@ -1,22 +0,0 @@ -#!/bin/bash -# Resolve the Keycloak group id for the RBAC root path (e.g. /roles). -# Echoes the id on stdout; empty when the path has no group; exits -# non-zero when the groups listing itself cannot be fetched. -# -# Required env: -# KC_CONTAINER keycloak container name -# KC_REALM realm name -# KC_RBAC_ROOT_PATH e.g. /roles -set -eo pipefail -: "${KC_CONTAINER:?KC_CONTAINER is required}" -: "${KC_REALM:?KC_REALM is required}" -: "${KC_RBAC_ROOT_PATH:?KC_RBAC_ROOT_PATH is required}" - -container exec -i "$KC_CONTAINER" /opt/keycloak/bin/kcadm.sh get groups \ - -r "$KC_REALM" \ - -q max=500 --fields id,path --format csv --noquotes \ - > /tmp/kc_groups_top.txt - -awk -F',' -v p="$KC_RBAC_ROOT_PATH" \ - '!/^\[/ && !/Cgroup/ && $2==p {print $1; exit}' \ - /tmp/kc_groups_top.txt | tr -d '\r' diff --git a/roles/web-app-keycloak/filter_plugins/rbac_per_app.py b/roles/web-app-keycloak/filter_plugins/rbac_per_app.py index 8f070fad02..b7524e2c29 100644 --- a/roles/web-app-keycloak/filter_plugins/rbac_per_app.py +++ b/roles/web-app-keycloak/filter_plugins/rbac_per_app.py @@ -8,7 +8,7 @@ per-app `group-ldap-mapper` setup: These names are referenced from `vars/main.yml` (`KEYCLOAK_PER_APP_LDAP_ROLES_MAPPER_PAYLOADS`), which hands them to the -host-side `reconcile_apps.sh` script. Centralising them here means a future +`files/python/rbac_reconcile.py` REST client. Centralising them here means a future rename of the mapper convention is a one-line change in this plugin and stays consistent everywhere. """ diff --git a/roles/web-app-keycloak/tasks/update/05_per_app_mappers.yml b/roles/web-app-keycloak/tasks/update/05_per_app_mappers.yml index 24cbc7fa35..4d2c6dd7d4 100644 --- a/roles/web-app-keycloak/tasks/update/05_per_app_mappers.yml +++ b/roles/web-app-keycloak/tasks/update/05_per_app_mappers.yml @@ -23,10 +23,7 @@ # # The shared default `ldap-roles` mapper (created by update/05_ldap/main.yml) # is removed here so it does not shadow the per-app mappers, and any -# Keycloak groups it had imported flat under /roles are pruned. Each -# step delegates to a dedicated `.sh` under -# `files/shell/per_app_mapper/` so the shell logic lives outside the -# YAML structure and stays easy to lint and reuse. +# Keycloak groups it had imported flat under /roles are pruned. - name: "๐Ÿšฟ per-app mappers: flush pending handlers before mutating the shared mapper" ansible.builtin.meta: flush_handlers @@ -37,81 +34,38 @@ all_deployed: "{{ KEYCLOAK_RBAC_ALL_APP_IDS }}" current_run: "{{ KEYCLOAK_RBAC_RUN_APP_IDS }}" -- name: "๐Ÿ—‘๏ธ per-app mappers: delete shared 'ldap-roles' mapper if present" - ansible.builtin.script: - cmd: "files/shell/per_app_mapper/delete_default_ldap_roles_mapper.sh" - executable: /bin/bash - environment: - KC_CONTAINER: "{{ KEYCLOAK_TARGET_CONTAINER }}" - KC_REALM: "{{ KEYCLOAK_REALM }}" - LDAP_COMPONENT_ID: "{{ ldap_cmp_id | trim }}" - changed_when: true - failed_when: false - no_log: "{{ MASK_CREDENTIALS_IN_LOGS | bool }}" - -- name: "๐Ÿงน Purge stale flat / orphan groups under /" - ansible.builtin.script: - cmd: "files/shell/per_app_mapper/purge/root_orphans.sh" - executable: /bin/bash - environment: - KC_CONTAINER: "{{ KEYCLOAK_TARGET_CONTAINER }}" - KC_REALM: "{{ KEYCLOAK_REALM }}" - KC_RBAC_ROOT_PATH: "/{{ KEYCLOAK_RBAC_GROUP_NAME | trim('/') }}" - DEPLOYED_APP_IDS: "{{ KEYCLOAK_RBAC_ALL_APP_IDS | join(' ') }}" - KC_PURGE_WORKERS: "{{ KEYCLOAK_KCADM_WORKERS }}" - changed_when: true - failed_when: false - no_log: "{{ MASK_CREDENTIALS_IN_LOGS | bool }}" - -- name: "๐Ÿ” per-app mappers: resolve the root group id once, it is loop-invariant" - ansible.builtin.script: - cmd: "files/shell/per_app_mapper/resolve_root_group_id.sh" - executable: /bin/bash - environment: - KC_CONTAINER: "{{ KEYCLOAK_TARGET_CONTAINER }}" - KC_REALM: "{{ KEYCLOAK_REALM }}" - KC_RBAC_ROOT_PATH: "/{{ KEYCLOAK_RBAC_GROUP_NAME | trim('/') }}" - register: per_app_root_id - changed_when: false - no_log: "{{ MASK_CREDENTIALS_IN_LOGS | bool }}" - -- name: "โœ… per-app mappers: assert root group was resolved" - ansible.builtin.assert: - that: (per_app_root_id.stdout | default('') | trim) | length > 0 - fail_msg: "could not resolve /{{ KEYCLOAK_RBAC_GROUP_NAME | trim('/') }} group id" - -- name: "๐Ÿ”ง per-app mappers: set root group id fact" - ansible.builtin.set_fact: - kc_root_group_id: "{{ (per_app_root_id.stdout | default('')) | trim }}" - changed_when: false - - name: "๐Ÿ“„ per-app mappers: render the mapper payloads of every rbac app in current run" ansible.builtin.copy: dest: "{{ KEYCLOAK_PER_APP_LDAP_ROLES_MAPPER_PAYLOADS_FILE }}" mode: "0600" content: "{{ KEYCLOAK_PER_APP_LDAP_ROLES_MAPPER_PAYLOADS }}" changed_when: false + delegate_to: "{{ KEYCLOAK_HOST_NODE }}" no_log: "{{ MASK_CREDENTIALS_IN_LOGS | bool }}" -- name: "โœ… per-app mappers: ensure Keycloak group and mapper per rbac app in current run" - ansible.builtin.script: - cmd: "files/shell/per_app_mapper/reconcile_apps.sh" - executable: /bin/bash - environment: - KC_CONTAINER: "{{ KEYCLOAK_TARGET_CONTAINER }}" - KC_REALM: "{{ KEYCLOAK_REALM }}" - LDAP_COMPONENT_ID: "{{ ldap_cmp_id | trim }}" - ROOT_GROUP_ID: "{{ kc_root_group_id }}" - PAYLOADS_FILE: "{{ KEYCLOAK_PER_APP_LDAP_ROLES_MAPPER_PAYLOADS_FILE }}" - GROUP_ID_PLACEHOLDER: "{{ KEYCLOAK_PER_APP_GROUP_ID_PLACEHOLDER }}" - KC_WORKERS: "{{ KEYCLOAK_KCADM_WORKERS }}" - KC_SYNC_ATTEMPTS: "3" - KC_SYNC_DELAY: "20" - KC_SYNC_TIMEOUT: "{{ lookup('timeout', 60) | int }}" - register: per_app_reconcile - changed_when: true - failed_when: per_app_reconcile.rc not in [0, 1, 2] - no_log: "{{ MASK_CREDENTIALS_IN_LOGS | bool }}" +- name: "โœ… per-app mappers: reconcile groups and mappers of every rbac app over REST" + ansible.builtin.include_tasks: + file: "{{ role_path }}/tasks/utils/rest_script.yml" + apply: + delegate_to: "{{ KEYCLOAK_HOST_NODE }}" + vars: + rest_script: rbac_reconcile.py + rest_script_env: + LDAP_COMPONENT_ID: "{{ ldap_cmp_id | trim }}" + RBAC_ROOT_PATH: "/{{ KEYCLOAK_RBAC_GROUP_NAME | trim('/') }}" + DEPLOYED_APP_IDS: "{{ KEYCLOAK_RBAC_ALL_APP_IDS | join(' ') }}" + PAYLOADS_FILE: "{{ KEYCLOAK_PER_APP_LDAP_ROLES_MAPPER_PAYLOADS_FILE }}" + WORKERS: "{{ KEYCLOAK_RBAC_WORKERS }}" + SYNC_ATTEMPTS: "3" + SYNC_DELAY: "20" + SYNC_TIMEOUT: "{{ lookup('timeout', 60) | int }}" + rest_script_ok_rcs: [0, 1, 2] + rest_script_changed: "synced=[1-9]" + rest_script_register: per_app_reconcile + +- name: "๐Ÿ“Š per-app mappers: report how many rbac apps were synced" + ansible.builtin.debug: + msg: "{{ per_app_reconcile.stdout_lines | select('match', '^summary ') | list }}" - name: "โš ๏ธ per-app mappers: report the rbac apps that failed" ansible.builtin.fail: diff --git a/roles/web-app-keycloak/tasks/utils/rest_script.yml b/roles/web-app-keycloak/tasks/utils/rest_script.yml new file mode 100644 index 0000000000..60c6e99885 --- /dev/null +++ b/roles/web-app-keycloak/tasks/utils/rest_script.yml @@ -0,0 +1,72 @@ +--- +# Param: rest_script - script under files/python/ that imports keycloak_admin.py +# Param: rest_script_env - script environment besides KC_URL, KC_PID, KC_REALM and PYTHONPATH +# Param: rest_script_ok_rcs - exit codes that do not fail the run +# Param: rest_script_changed - regex on stdout that marks the run changed +# Param: rest_script_register - fact the command result is stored under +- name: "๐Ÿ” Keycloak REST | Resolve the host PID of the Keycloak container" + ansible.builtin.command: + argv: + - container + - inspect + - --type + - container + - --format + - "{{ '{{' }}.State.Pid{{ '}}' }}" + - "{{ KEYCLOAK_LOCAL_CID }}" # nocheck: container-exec-resolver KEYCLOAK_LOCAL_CID resolved at delegate_to runtime; container_address would return a manager-only subshell + register: kc_rest_script_pid + changed_when: false + +- name: "๐Ÿ“ Keycloak REST | Create the staging directory of {{ rest_script }}" + ansible.builtin.tempfile: + state: directory + suffix: -keycloak-rest + register: kc_rest_script_dir + changed_when: false + +- name: "๐Ÿ Keycloak REST | Stage, run and remove {{ rest_script }}" + block: + - name: "๐Ÿ“„ Keycloak REST | Stage {{ rest_script }} with its client module" + ansible.builtin.copy: + src: "python/{{ kc_rest_script_file }}" + dest: "{{ kc_rest_script_dir.path }}/{{ kc_rest_script_file }}" + mode: "0600" + loop: + - keycloak_admin.py + - "{{ rest_script }}" + loop_control: + loop_var: kc_rest_script_file + changed_when: false + + - name: "โ–ถ๏ธ Keycloak REST | Run {{ rest_script }} in the Keycloak network namespace" + ansible.builtin.command: + argv: + - nsenter + - --target + - "{{ kc_rest_script_pid.stdout | trim }}" + - --net + - -- + - python3 + - "{{ kc_rest_script_dir.path }}/{{ rest_script }}" + environment: "{{ rest_script_env | combine(kc_rest_script_base_env) }}" + vars: + kc_rest_script_base_env: + KC_URL: "{{ KEYCLOAK_SERVER_INTERNAL_URL }}" + KC_PID: "{{ kc_rest_script_pid.stdout | trim }}" + KC_REALM: "{{ KEYCLOAK_REALM }}" + PYTHONPATH: "{{ kc_rest_script_dir.path }}" + register: kc_rest_script_result + changed_when: kc_rest_script_result.stdout is search(rest_script_changed) + failed_when: kc_rest_script_result.rc not in rest_script_ok_rcs + no_log: "{{ MASK_CREDENTIALS_IN_LOGS | bool }}" + + - name: "๐Ÿ“Œ Keycloak REST | Store the result of {{ rest_script }}" + ansible.builtin.set_fact: + "{{ rest_script_register }}": "{{ kc_rest_script_result }}" + no_log: "{{ MASK_CREDENTIALS_IN_LOGS | bool }}" + always: + - name: "๐Ÿ—‘๏ธ Keycloak REST | Remove the staging directory of {{ rest_script }}" + ansible.builtin.file: + path: "{{ kc_rest_script_dir.path }}" + state: absent + changed_when: false diff --git a/roles/web-app-keycloak/vars/main.yml b/roles/web-app-keycloak/vars/main.yml index ae0d3cebfa..16ea4bdc45 100644 --- a/roles/web-app-keycloak/vars/main.yml +++ b/roles/web-app-keycloak/vars/main.yml @@ -17,33 +17,40 @@ KEYCLOAK_RBAC_GROUP_NAME: "{{ RBAC.GROUP.NAME }}" KEYCLOAK_RBAC_ALL_APP_IDS: "{{ lookup('deployment').groups | rbac_app_ids }}" KEYCLOAK_RBAC_RUN_APP_IDS: "{{ lookup('deployment').running | rbac_app_ids }}" -KEYCLOAK_KCADM_WORKERS: "{{ [RESOURCE_HOST_CPUS | int // 2, 1] | max }}" -KEYCLOAK_PER_APP_GROUP_ID_PLACEHOLDER: "__KC_APP_GROUP_ID__" +KEYCLOAK_RBAC_WORKERS: "{{ [RESOURCE_HOST_CPUS | int // 2, 1] | max }}" +KEYCLOAK_RBAC_GROUP_LOGIC_DIGEST: "{{ lookup('file', lookup('path_absolute', 'roles/svc-db-openldap/filter_plugins/build_ldap_role_entries.py')) | hash('sha256') }}" KEYCLOAK_PER_APP_LDAP_ROLES_MAPPER_PAYLOADS_FILE: "/tmp/kc_per_app_mapper_payloads.txt" -KEYCLOAK_PER_APP_LDAP_ROLES_MAPPER_BASE: >- +KEYCLOAK_PER_APP_LDAP_ROLES_MAPPER_TEMPLATE: >- {{ - ((KEYCLOAK_DICTIONARY_REALM.components['org.keycloak.storage.UserStorageProvider'] + (KEYCLOAK_DICTIONARY_REALM.components['org.keycloak.storage.UserStorageProvider'] | selectattr('name','equalto', KEYCLOAK_LDAP_CMP_NAME) | list | first - ).subComponents['org.keycloak.storage.ldap.mappers.LDAPStorageMapper'] - | selectattr('name','equalto','ldap-roles') - | list | first + ).subComponents['org.keycloak.storage.ldap.mappers.LDAPStorageMapper'] + | selectattr('name','equalto','ldap-roles') + | list | first + }} +KEYCLOAK_PER_APP_LDAP_ROLES_MAPPER_BASE: >- + {{ + (KEYCLOAK_PER_APP_LDAP_ROLES_MAPPER_TEMPLATE | dict2items - | rejectattr('key','in',['subComponents','id']) + | rejectattr('key','in',['subComponents','id','config']) | list | items2dict) | combine({ 'parentId': ldap_cmp_id | trim, 'providerId': 'group-ldap-mapper', 'providerType': 'org.keycloak.storage.ldap.mappers.LDAPStorageMapper', - 'config': { - 'preserve.group.inheritance': ['false'], - 'group.name.ldap.attribute': ['description'], - 'groups.path.group': [KEYCLOAK_PER_APP_GROUP_ID_PLACEHOLDER], - 'drop.non.existing.groups.during.sync': ['true'], - } - }, recursive=True) + 'config': (KEYCLOAK_PER_APP_LDAP_ROLES_MAPPER_TEMPLATE.config + | dict2items + | rejectattr('key','equalto','groups.path.group') + | list | items2dict) + | combine({ + 'preserve.group.inheritance': ['false'], + 'group.name.ldap.attribute': ['description'], + 'drop.non.existing.groups.during.sync': ['true'], + }), + }) }} -# One ` ` line per rbac app of the current run. +# One ` ` line per rbac app of the current run. KEYCLOAK_PER_APP_LDAP_ROLES_MAPPER_PAYLOADS: |- {% for app_id in KEYCLOAK_RBAC_RUN_APP_IDS -%} {%- set mapper = KEYCLOAK_PER_APP_LDAP_ROLES_MAPPER_BASE | combine({ @@ -53,7 +60,14 @@ KEYCLOAK_PER_APP_LDAP_ROLES_MAPPER_PAYLOADS: |- 'groups.path': ['/' ~ (KEYCLOAK_RBAC_GROUP_NAME | trim('/')) ~ '/' ~ app_id], } }, recursive=True) -%} - {{ app_id }} {{ mapper['name'] }} {{ mapper | to_nice_json | b64encode }} + {%- set app = lookup('applications')[app_id] -%} + {%- set fingerprint = { + 'mapper': mapper, + 'rbac': app.get('rbac'), + 'domains': app.get('domains'), + 'logic': KEYCLOAK_RBAC_GROUP_LOGIC_DIGEST, + } | to_json(sort_keys=True) | hash('sha256') -%} + {{ app_id }} {{ mapper['name'] }} {{ mapper | to_nice_json | b64encode }} {{ fingerprint }} {% endfor %} KEYCLOAK_HEALTH_ENABLED: true diff --git a/tests/unit/python/roles/web-app-keycloak/files/__init__.py b/tests/unit/python/roles/web-app-keycloak/files/__init__.py new file mode 100644 index 0000000000..087c384b57 --- /dev/null +++ b/tests/unit/python/roles/web-app-keycloak/files/__init__.py @@ -0,0 +1,5 @@ +from pathlib import Path + +PROJECT_ROOT: Path = Path(__file__).resolve().parents[6] + +__all__ = ["PROJECT_ROOT"] diff --git a/tests/unit/python/roles/web-app-keycloak/files/test_keycloak_admin.py b/tests/unit/python/roles/web-app-keycloak/files/test_keycloak_admin.py new file mode 100644 index 0000000000..bfe6aeed0e --- /dev/null +++ b/tests/unit/python/roles/web-app-keycloak/files/test_keycloak_admin.py @@ -0,0 +1,79 @@ +import io +import json +import sys +import tempfile +import unittest +import urllib.error +from pathlib import Path +from unittest import mock + +from . import PROJECT_ROOT + +sys.path.insert(0, str(PROJECT_ROOT / "roles/web-app-keycloak/files/python")) + +import keycloak_admin as m # noqa: E402 + + +class TestContainerCredentials(unittest.TestCase): + def test_credentials_come_from_the_container_environment(self): + with tempfile.TemporaryDirectory() as tmp: + environ = Path(tmp) / "environ" + environ.write_bytes( + b"PATH=/bin\0KEYCLOAK_PERMANENT_ADMIN_USERNAME=admin\0" + b"KEYCLOAK_PERMANENT_ADMIN_PASSWORD=a=b\0" + ) + self.assertEqual(m.container_credentials(environ), ("admin", "a=b")) + + +class _Response(io.BytesIO): + def __init__(self, body=b"", location=""): + super().__init__(body) + self.headers = {"Location": location} if location else {} + + def __enter__(self): + return self + + def __exit__(self, *exc): + return False + + +class TestKeycloakClient(unittest.TestCase): + def test_one_token_serves_calls_until_it_nears_expiry(self): + token = json.dumps({"access_token": "t", "expires_in": 300}).encode() + calls = [] + + def urlopen(request, timeout): + calls.append(request.full_url) + if request.full_url.endswith("/token"): + return _Response(token) + return _Response(b"", "http://kc/admin/realms/r/groups/new-id") + + kc = m.Keycloak("http://kc", "r", "admin", "pw") + with mock.patch.object(m.urllib.request, "urlopen", urlopen): + _, created = kc.call("POST", "groups/root/children", {"name": "x"}) + kc.call("GET", "groups/new-id") + self.assertEqual(created, "new-id") + self.assertEqual(sum(url.endswith("/token") for url in calls), 1) + + def test_an_http_error_names_the_status_but_not_the_body(self): + token = json.dumps({"access_token": "t", "expires_in": 300}).encode() + + def urlopen(request, timeout): + if request.full_url.endswith("/token"): + return _Response(token) + raise urllib.error.HTTPError( + request.full_url, 409, "Conflict", {}, io.BytesIO(b"secret detail") + ) + + kc = m.Keycloak("http://kc", "r", "admin", "pw") + with ( + mock.patch.object(m.urllib.request, "urlopen", urlopen), + self.assertRaises(m.KeycloakError) as raised, + ): + kc.call("POST", "groups/root/children", {"name": "x"}) + self.assertIn("HTTP 409", str(raised.exception)) + self.assertNotIn("secret", str(raised.exception)) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/python/roles/web-app-keycloak/files/test_rbac_reconcile.py b/tests/unit/python/roles/web-app-keycloak/files/test_rbac_reconcile.py new file mode 100644 index 0000000000..90bd28bba5 --- /dev/null +++ b/tests/unit/python/roles/web-app-keycloak/files/test_rbac_reconcile.py @@ -0,0 +1,226 @@ +import base64 +import json +import sys +import tempfile +import unittest +from pathlib import Path + +from . import PROJECT_ROOT + +sys.path.insert(0, str(PROJECT_ROOT / "roles/web-app-keycloak/files/python")) + +import rbac_reconcile as m # noqa: E402 + +LDAP = "ldap-1" + + +class FakeKeycloak: + def __init__(self): + self.groups = {"root": {"id": "root", "name": "roles", "attributes": {}}} + self.parents = {} + self.components = {} + self.syncs = [] + self.failing_syncs = set() + self._next = 0 + + def _id(self, prefix): + self._next += 1 + return f"{prefix}-{self._next}" + + def add_group(self, name, parent="root", attributes=None): + group_id = self._id("g") + self.groups[group_id] = { + "id": group_id, + "name": name, + "attributes": attributes or {}, + } + self.parents[group_id] = parent + return group_id + + def add_mapper(self, name, config): + mapper_id = self._id("c") + self.components[mapper_id] = { + "id": mapper_id, + "name": name, + "parentId": LDAP, + "config": config, + } + return mapper_id + + def children(self, group_id): + return [ + dict(g) + for gid, g in self.groups.items() + if self.parents.get(gid) == group_id + ] + + def call(self, method, path, payload=None, timeout=60): + route = path.split("?", 1)[0] + if method == "GET" and route == "components": + return [dict(c) for c in self.components.values()], "" + if method == "GET" and route == "group-by-path/roles": + return dict(self.groups["root"]), "" + if method == "POST" and route == "components": + mapper_id = self._id("c") + self.components[mapper_id] = {**payload, "id": mapper_id} + return None, mapper_id + if method == "PUT" and route.startswith("components/"): + self.components[route.split("/")[1]] = dict(payload) + return None, "" + if method == "DELETE" and route.startswith("components/"): + del self.components[route.split("/")[1]] + return None, "" + if ( + method == "POST" + and route.startswith("groups/") + and route.endswith("/children") + ): + return None, self.add_group(payload["name"], route.split("/")[1]) + if method == "GET" and route.startswith("groups/"): + return json.loads(json.dumps(self.groups[route.split("/")[1]])), "" + if method == "PUT" and route.startswith("groups/"): + self.groups[route.split("/")[1]] = dict(payload) + return None, "" + if method == "DELETE" and route.startswith("groups/"): + del self.groups[route.split("/")[1]] + return None, "" + if method == "POST" and route.startswith(f"user-storage/{LDAP}/mappers/"): + mapper_id = route.split("/")[3] + if mapper_id in self.failing_syncs: + raise m.KeycloakError("POST sync: HTTP 500") + self.syncs.append(mapper_id) + return None, "" + raise AssertionError(f"unexpected call {method} {path}") + + +def _payload(app_id, filter_suffix=""): + return json.dumps( + { + "name": f"ldap-roles-{app_id}", + "parentId": LDAP, + "providerId": "group-ldap-mapper", + "config": { + "groups.path": [f"/roles/{app_id}"], + "groups.ldap.filter": [f"(cn={app_id}-*){filter_suffix}"], + }, + } + ) + + +def _app(app_id, fingerprint="fp-1", filter_suffix=""): + return m.App( + app_id, f"ldap-roles-{app_id}", _payload(app_id, filter_suffix), fingerprint + ) + + +def _reconciler(kc, attempts=2): + groups = {g["name"]: g["id"] for g in kc.children("root")} + mappers = {c["name"]: c for c in kc.components.values()} + return m.Reconciler(kc, LDAP, "root", groups, mappers, attempts, 0, 5) + + +class TestReconciler(unittest.TestCase): + def test_a_new_app_gets_its_group_mapper_sync_and_fingerprint(self): + kc = FakeKeycloak() + self.assertEqual(_reconciler(kc).run(_app("web-app-a")), "synced") + group = next(g for g in kc.groups.values() if g["name"] == "web-app-a") + mapper = next( + c for c in kc.components.values() if c["name"] == "ldap-roles-web-app-a" + ) + self.assertEqual(mapper["config"]["groups.path"], ["/roles/web-app-a"]) + self.assertEqual(kc.syncs, [mapper["id"]]) + self.assertEqual(group["attributes"][m.FINGERPRINT], ["fp-1"]) + + def test_an_unchanged_app_is_not_synced_again(self): + kc = FakeKeycloak() + _reconciler(kc).run(_app("web-app-a")) + self.assertEqual(_reconciler(kc).run(_app("web-app-a")), "unchanged") + self.assertEqual(len(kc.syncs), 1) + + def test_a_new_fingerprint_syncs_again(self): + kc = FakeKeycloak() + _reconciler(kc).run(_app("web-app-a")) + self.assertEqual(_reconciler(kc).run(_app("web-app-a", "fp-2")), "synced") + self.assertEqual(len(kc.syncs), 2) + + def test_a_drifted_mapper_is_updated_and_synced(self): + kc = FakeKeycloak() + _reconciler(kc).run(_app("web-app-a")) + self.assertEqual( + _reconciler(kc).run(_app("web-app-a", filter_suffix="(drift)")), "synced" + ) + mapper = next( + c for c in kc.components.values() if c["name"] == "ldap-roles-web-app-a" + ) + self.assertEqual( + mapper["config"]["groups.ldap.filter"], ["(cn=web-app-a-*)(drift)"] + ) + + def test_extra_config_keycloak_adds_does_not_count_as_drift(self): + kc = FakeKeycloak() + _reconciler(kc).run(_app("web-app-a")) + mapper = next( + c for c in kc.components.values() if c["name"] == "ldap-roles-web-app-a" + ) + mapper["config"]["mapped.group.attributes"] = [""] + self.assertEqual(_reconciler(kc).run(_app("web-app-a")), "unchanged") + + def test_a_failed_sync_records_no_fingerprint(self): + kc = FakeKeycloak() + group_id = kc.add_group("web-app-a") + mapper_id = kc.add_mapper( + "ldap-roles-web-app-a", + { + "groups.path": ["/roles/web-app-a"], + "groups.ldap.filter": ["(cn=web-app-a-*)"], + }, + ) + kc.failing_syncs.add(mapper_id) + with self.assertRaises(m.StepError) as raised: + _reconciler(kc).run(_app("web-app-a")) + self.assertEqual(raised.exception.step, "sync") + self.assertNotIn(m.FINGERPRINT, kc.groups[group_id]["attributes"]) + + def test_existing_group_attributes_survive_the_fingerprint(self): + kc = FakeKeycloak() + kc.add_group("web-app-a", attributes={"LDAP_ID": ["x"]}) + _reconciler(kc).run(_app("web-app-a")) + group = next(g for g in kc.groups.values() if g["name"] == "web-app-a") + self.assertEqual(group["attributes"]["LDAP_ID"], ["x"]) + + +class TestHousekeeping(unittest.TestCase): + def test_orphans_go_and_app_groups_stay(self): + kc = FakeKeycloak() + keep = kc.add_group("web-app-a") + kc.add_group("web-app-a-administrator") + kc.add_group("administrator") + kept, failures = m.purge_orphans(kc, "root", {"web-app-a"}, 2) + self.assertEqual(kept, {"web-app-a": keep}) + self.assertEqual(failures, []) + self.assertEqual(sorted(g["name"] for g in kc.children("root")), ["web-app-a"]) + + def test_the_shared_mapper_is_dropped(self): + kc = FakeKeycloak() + kc.add_mapper(m.SHARED_MAPPER, {}) + per_app = kc.add_mapper("ldap-roles-web-app-a", {}) + mappers = m.ldap_mappers(kc, LDAP) + self.assertEqual(m.drop_shared_mapper(kc, mappers), []) + self.assertEqual(list(kc.components), [per_app]) + self.assertNotIn(m.SHARED_MAPPER, mappers) + + def test_payload_lines_are_parsed(self): + encoded = base64.b64encode(_payload("web-app-a").encode()).decode() + with tempfile.NamedTemporaryFile("w", suffix=".txt", delete=False) as handle: + handle.write(f"web-app-a ldap-roles-web-app-a {encoded} fp-9\n\n") + apps = m.read_payloads(handle.name) + Path(handle.name).unlink() + self.assertEqual( + [(a.app_id, a.mapper_name, a.fingerprint) for a in apps], + [("web-app-a", "ldap-roles-web-app-a", "fp-9")], + ) + self.assertEqual(json.loads(apps[0].payload)["name"], "ldap-roles-web-app-a") + + +if __name__ == "__main__": + unittest.main() From 49aa9200341ba7775b5374bf8ba62620e08a0ce6 Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Sat, 24 Oct 2026 19:39:53 +0200 Subject: [PATCH 09/17] perf(web-app-keycloak): mark user emails verified in one REST call The emailVerified update looped over every non-reserved user, uploaded a shell script per user and ran kcadm with a JVM login each time, and it always reported changed: 7 users took 23.8 s on echoserver. email_verified.py uses the shared keycloak_admin client through tasks/utils/rest_script.yml: it looks each user up by exact username, skips users already verified, and writes emailVerified with a GET plus a full-representation PUT, as kcadm update did. The task reports changed only when a user was updated, and a failed lookup now fails the task instead of being treated as a missing user. The old lookup piped kcadm -q username=X into head -n1, which could have hit a longer username sharing the prefix. Evidence (local compose, slot 104): summary updated=0 unchanged=8 missing=0 failed=0 in 0.60 s, task ok, staging dir removed. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../files/python/email_verified.py | 84 ++++++++++++ .../files/shell/set_email_verified.sh | 64 --------- .../tasks/update/08_users/email_verified.yml | 34 ++--- .../files/test_email_verified.py | 129 ++++++++++++++++++ 4 files changed, 228 insertions(+), 83 deletions(-) create mode 100755 roles/web-app-keycloak/files/python/email_verified.py delete mode 100644 roles/web-app-keycloak/files/shell/set_email_verified.sh create mode 100644 tests/unit/python/roles/web-app-keycloak/files/test_email_verified.py diff --git a/roles/web-app-keycloak/files/python/email_verified.py b/roles/web-app-keycloak/files/python/email_verified.py new file mode 100755 index 0000000000..1f3ea428d1 --- /dev/null +++ b/roles/web-app-keycloak/files/python/email_verified.py @@ -0,0 +1,84 @@ +#!/usr/bin/env python3 +"""Mark the email address of every listed Keycloak user verified over the admin REST API. + +Env: + KC_URL: Keycloak base URL reachable from the network namespace the script runs in. + KC_PID: host PID of the Keycloak container, whose environment holds the admin credentials. + KC_REALM: realm holding the users. + USERNAMES: space-separated usernames to mark verified. + +Output: one `` `` line per user, then one ``summary`` line. +Exit: 0 every user is verified or absent from the realm, 1 otherwise. +""" + +from __future__ import annotations + +import os +import sys +import urllib.parse +from pathlib import Path + +from keycloak_admin import Keycloak, KeycloakError, container_credentials + +OUTCOMES = ("updated", "unchanged", "missing", "failed") + + +def find_user(kc: Keycloak, username: str) -> dict | None: + """Return the user whose username equals ``username``, or None. + + Args: + kc: client. + username: the username, compared case-insensitively as Keycloak stores it lowercased. + """ + query = urllib.parse.urlencode({"username": username, "exact": "true"}) + found, _ = kc.call("GET", f"users?{query}") + wanted = username.lower() + return next((user for user in found if user["username"].lower() == wanted), None) + + +def verify(kc: Keycloak, username: str) -> str: + """Return ``updated``, ``unchanged`` or ``missing`` for a user. + + Args: + kc: client. + username: the user whose email address gets marked verified. + """ + user = find_user(kc, username) + if user is None: + return "missing" + if user.get("emailVerified") is True: + return "unchanged" + current, _ = kc.call("GET", f"users/{user['id']}") + kc.call("PUT", f"users/{user['id']}", {**current, "emailVerified": True}) + return "updated" + + +def main() -> int: + env = os.environ + try: + kc = Keycloak( + env["KC_URL"], + env["KC_REALM"], + *container_credentials(Path("/proc") / env["KC_PID"] / "environ"), + ) + usernames = env["USERNAMES"].split() + except (OSError, KeyError, ValueError) as error: + print(f"setup-failed {error}") + return 1 + + outcomes = [] + for username in usernames: + try: + outcome = verify(kc, username) + except (KeycloakError, OSError, KeyError, ValueError) as error: + print(f"{username}: {error}", file=sys.stderr) + outcome = "failed" + print(f"{outcome} {username}") + outcomes.append(outcome) + + print("summary " + " ".join(f"{key}={outcomes.count(key)}" for key in OUTCOMES)) + return 1 if "failed" in outcomes else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/roles/web-app-keycloak/files/shell/set_email_verified.sh b/roles/web-app-keycloak/files/shell/set_email_verified.sh deleted file mode 100644 index d5bdf95026..0000000000 --- a/roles/web-app-keycloak/files/shell/set_email_verified.sh +++ /dev/null @@ -1,64 +0,0 @@ -#!/usr/bin/env bash -set -euo pipefail - -############################################################################### -# Set emailVerified for a Keycloak user (robust against noisy kcadm output) -# -# Runs on the host and executes kcadm inside the Keycloak container. -# -# REQUIRED environment variables: -# KEYCLOAK_EXEC_CONTAINER e.g. "container exec -i keycloak" -# KEYCLOAK_KCADM e.g. "/opt/keycloak/bin/kcadm.sh" -# KEYCLOAK_REALM Realm name -# KEYCLOAK_USERNAME Username to update -# -# OPTIONAL: -# KEYCLOAK_EMAIL_VERIFIED true|false (default: true) -############################################################################### - -: "${KEYCLOAK_EXEC_CONTAINER:?missing KEYCLOAK_EXEC_CONTAINER}" -: "${KEYCLOAK_KCADM:?missing KEYCLOAK_KCADM}" -: "${KEYCLOAK_REALM:?missing KEYCLOAK_REALM}" -: "${KEYCLOAK_USERNAME:?missing KEYCLOAK_USERNAME}" - -KEYCLOAK_EMAIL_VERIFIED="${KEYCLOAK_EMAIL_VERIFIED:-true}" - -${KEYCLOAK_EXEC_CONTAINER} sh -lc " - set -euo pipefail - - USERNAME=\"${KEYCLOAK_USERNAME}\" - REALM=\"${KEYCLOAK_REALM}\" - VERIFIED=\"${KEYCLOAK_EMAIL_VERIFIED}\" - - RAW=\"\$(${KEYCLOAK_KCADM} get users -r \"\$REALM\" -q username=\"\$USERNAME\" --fields id --format csv --noquotes 2>&1 || true)\" - - USER_ID=\"\$(printf '%s\n' \"\$RAW\" \ - | grep -Eio '[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}' \ - | head -n1 || true)\" - - if [ -z \"\${USER_ID:-}\" ]; then - echo \"[keycloak][emailVerified] user not found: \$USERNAME\" >&2 - exit 0 - fi - - CURRENT_RAW=\"\$(${KEYCLOAK_KCADM} get users/\$USER_ID -r \"\$REALM\" 2>&1 || true)\" - - CURRENT_LINE=\"\$(printf '%s\n' \"\$CURRENT_RAW\" \ - | tr -d '\r' \ - | grep -Eo '\"emailVerified\"[[:space:]]*:[[:space:]]*(true|false)' \ - | head -n1 || true)\" - - CURRENT=\"\$(printf '%s' \"\$CURRENT_LINE\" \ - | sed -E 's/.*:[[:space:]]*(true|false).*/\\1/' \ - | tr -d ' ' \ - || true)\" - - if [ \"\${CURRENT:-}\" = \"\$VERIFIED\" ]; then - echo \"[keycloak][emailVerified] unchanged: \$USERNAME (\$USER_ID) = \$CURRENT\" - exit 0 - fi - - ${KEYCLOAK_KCADM} update users/\$USER_ID -r \"\$REALM\" -s emailVerified=\"\$VERIFIED\" - - echo \"[keycloak][emailVerified] updated: \$USERNAME (\$USER_ID) -> \$VERIFIED\" -" diff --git a/roles/web-app-keycloak/tasks/update/08_users/email_verified.yml b/roles/web-app-keycloak/tasks/update/08_users/email_verified.yml index 6c8318fcdf..f092932e5c 100644 --- a/roles/web-app-keycloak/tasks/update/08_users/email_verified.yml +++ b/roles/web-app-keycloak/tasks/update/08_users/email_verified.yml @@ -1,21 +1,17 @@ --- -# Set emailVerified=true for all configured users from vars. -# Contract: -# - users is ALWAYS a dict -# - username is ALWAYS stored in users..username -# - dict key is ignored completely - - name: "๐Ÿ”ง Set emailVerified=true for users in realm '{{ KEYCLOAK_REALM }}'" - ansible.builtin.script: shell/set_email_verified.sh - environment: - KEYCLOAK_EXEC_CONTAINER: "{{ KEYCLOAK_EXEC_CONTAINER }}" - KEYCLOAK_KCADM: "{{ KEYCLOAK_KCADM }}" - KEYCLOAK_REALM: "{{ KEYCLOAK_REALM }}" - KEYCLOAK_USERNAME: "{{ item.value.username }}" - KEYCLOAK_EMAIL_VERIFIED: "true" - loop: "{{ lookup('users_non_reserved') | dict2items }}" - loop_control: - label: "{{ item.value.username }}" - register: kc_email_verified_updates - changed_when: true - no_log: "{{ MASK_CREDENTIALS_IN_LOGS | bool }}" + ansible.builtin.include_tasks: + file: "{{ role_path }}/tasks/utils/rest_script.yml" + apply: + delegate_to: "{{ KEYCLOAK_HOST_NODE }}" + vars: + rest_script: email_verified.py + rest_script_env: + USERNAMES: "{{ lookup('users_non_reserved') | dict2items | map(attribute='value.username') | join(' ') }}" + rest_script_ok_rcs: [0] + rest_script_changed: "updated=[1-9]" + rest_script_register: kc_email_verified + +- name: "๐Ÿ“Š Report how many users got their email marked verified" + ansible.builtin.debug: + msg: "{{ kc_email_verified.stdout_lines | select('match', '^summary ') | list }}" diff --git a/tests/unit/python/roles/web-app-keycloak/files/test_email_verified.py b/tests/unit/python/roles/web-app-keycloak/files/test_email_verified.py new file mode 100644 index 0000000000..0cd86d74dc --- /dev/null +++ b/tests/unit/python/roles/web-app-keycloak/files/test_email_verified.py @@ -0,0 +1,129 @@ +import contextlib +import io +import os +import sys +import unittest +import urllib.parse +from unittest import mock + +from . import PROJECT_ROOT + +sys.path.insert(0, str(PROJECT_ROOT / "roles/web-app-keycloak/files/python")) + +import email_verified as m # noqa: E402 + + +class FakeKeycloak: + def __init__(self): + self.users = {} + self.puts = [] + self.failing = set() + + def add_user(self, username, verified, **extra): + user_id = f"u-{len(self.users) + 1}" + self.users[user_id] = { + "id": user_id, + "username": username, + "emailVerified": verified, + **extra, + } + return user_id + + def call(self, method, path, payload=None, timeout=60): + route, _, query = path.partition("?") + if method == "GET" and route == "users": + wanted = urllib.parse.parse_qs(query)["username"][0] + if wanted in self.failing: + raise m.KeycloakError(f"GET {path}: HTTP 500") + return [ + dict(u) for u in self.users.values() if wanted.lower() in u["username"] + ], "" + if method == "GET" and route.startswith("users/"): + return dict(self.users[route.split("/")[1]]), "" + if method == "PUT" and route.startswith("users/"): + user_id = route.split("/")[1] + self.puts.append(user_id) + self.users[user_id] = dict(payload) + return None, "" + raise AssertionError(f"unexpected call {method} {path}") + + +class TestVerify(unittest.TestCase): + def test_an_unverified_user_is_updated_and_keeps_its_other_fields(self): + kc = FakeKeycloak() + user_id = kc.add_user("biber", False, email="biber@example.org") + self.assertEqual(m.verify(kc, "biber"), "updated") + self.assertIs(kc.users[user_id]["emailVerified"], True) + self.assertEqual(kc.users[user_id]["email"], "biber@example.org") + + def test_a_verified_user_is_left_alone(self): + kc = FakeKeycloak() + kc.add_user("biber", True) + self.assertEqual(m.verify(kc, "biber"), "unchanged") + self.assertEqual(kc.puts, []) + + def test_an_absent_user_is_reported_missing(self): + kc = FakeKeycloak() + self.assertEqual(m.verify(kc, "biber"), "missing") + self.assertEqual(kc.puts, []) + + def test_only_the_exact_username_is_touched(self): + kc = FakeKeycloak() + other = kc.add_user("contact2", False) + target = kc.add_user("contact", False) + self.assertEqual(m.verify(kc, "Contact"), "updated") + self.assertEqual(kc.puts, [target]) + self.assertIs(kc.users[other]["emailVerified"], False) + + +class TestMain(unittest.TestCase): + def _run(self, kc, usernames): + out = io.StringIO() + env = { + "KC_URL": "http://kc", + "KC_PID": "1", + "KC_REALM": "r", + "USERNAMES": usernames, + } + with ( + mock.patch.dict(os.environ, env), + mock.patch.object(m, "Keycloak", lambda *args: kc), + mock.patch.object(m, "container_credentials", lambda path: ("a", "p")), + contextlib.redirect_stdout(out), + contextlib.redirect_stderr(io.StringIO()), + ): + code = m.main() + return code, out.getvalue().splitlines() + + def test_the_summary_counts_every_outcome(self): + kc = FakeKeycloak() + kc.add_user("administrator", True) + kc.add_user("biber", False) + code, lines = self._run(kc, "administrator biber ghost") + self.assertEqual(code, 0) + self.assertEqual(lines[-1], "summary updated=1 unchanged=1 missing=1 failed=0") + + def test_one_failed_user_fails_the_run_but_not_the_others(self): + kc = FakeKeycloak() + kc.add_user("administrator", False) + kc.add_user("biber", False) + kc.failing.add("administrator") + code, lines = self._run(kc, "administrator biber") + self.assertEqual(code, 1) + self.assertIn("failed administrator", lines) + self.assertIn("updated biber", lines) + + def test_a_missing_environment_key_fails_before_any_call(self): + kc = FakeKeycloak() + with ( + mock.patch.dict(os.environ, {}, clear=True), + mock.patch.object(m, "Keycloak", lambda *args: kc), + contextlib.redirect_stdout(io.StringIO()) as out, + ): + code = m.main() + self.assertEqual(code, 1) + self.assertTrue(out.getvalue().startswith("setup-failed")) + + +if __name__ == "__main__": + unittest.main() From dba6aa7407aa6354ca173c60957b4d27a432501f Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Sat, 24 Oct 2026 19:48:43 +0200 Subject: [PATCH 10/17] docs(requirements): add 045 stack service Records the operator-confirmed design for replacing the sys-stk-full, sys-stk-backend and sys-stk-front-proxy includes (115 call sites in 94 roles) with one shared `stack` service that sys-service-loader runs once before the application roles and that processes every application of the play in one batch: per-role literal `stack` and `openresty` keys, front proxy and database derived from enabled services, per-domain vhost data in meta without exceptions, all roles in one change. The document carries the verified codebase facts with file:line on origin/main 6997dd3dbb, the ordering and binding constraints, the cost measured on the echoserver full-6 and full-7 deploys, a target design, grouped acceptance criteria, validation commands and an implementation order, so another agent can implement it without this conversation. Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/requirements/045-stack-service.md | 182 +++++++++++++++++++++++++ 1 file changed, 182 insertions(+) create mode 100644 docs/requirements/045-stack-service.md diff --git a/docs/requirements/045-stack-service.md b/docs/requirements/045-stack-service.md new file mode 100644 index 0000000000..60e5ddf598 --- /dev/null +++ b/docs/requirements/045-stack-service.md @@ -0,0 +1,182 @@ +# 045 - Stack Service: One Shared Service Replaces the Stack Role Includes + +## User Story + +As an operator who deploys many applications per host, I want the application stack (front proxy, backend, database) to be one shared `stack` service that the service loader runs once, before the application roles, and that processes every application of the play in that one invocation, so that a deploy stops repeating host-wide stack work per application and the stack has a single point of truth instead of 115 include sites. + +## Decisions (operator-confirmed 2026-10-24; do not re-litigate) + +1. **One service key, `stack`.** There is no separate key for front proxy, backend or full. What the stack does for an application is derived automatically from the files the role ships and the services the role enables. +2. **Declared per application, literally.** Every consumer role declares `stack: {enabled: true, shared: true}` in its own `meta/services.yml`. The values are fixed literals, not `'' in group_names` expressions. A central per-category declaration and the `add_host` injection used for OpenResty were rejected. +3. **Front proxy is triggered by OpenResty.** An application gets a vhost exactly when its own `meta/services.yml` enables and shares `openresty`. Today no application declares that key (only `svc-prx-openresty` does), so every role that needs a vhost gets the new key. +4. **Database is triggered by the database service.** An application gets its database exactly when it enables and shares `postgres` or `mariadb` (55 roles already declare one of them). Object storage and engines (`sys-svc-rdbms`, objstore, engine includes of `sys-stk-backend`) follow their own existing service keys the same way. +5. **Completely without includes.** No role includes, imports or calls `sys-stk-full`, `sys-stk-backend` or `sys-stk-front-proxy` anymore. The three roles are removed. The stack drives the per-application work itself. +6. **Runs before the application role, in the dependency/service phase, once for all applications.** The `stack` service is loaded by `sys-service-loader` like the other shared services and processes every stack consumer of the play in one batch invocation. The application roles run afterwards and contain only application-specific configuration. +7. **Per-vhost parameters move into meta, per domain, without exceptions.** Every value an include site passes today moves into the role's meta, keyed by domain. This includes the multi-vhost roles (Matrix, Stalwart, Bluesky and the others listed below). +8. **All roles in one change.** All ~94 consumer roles are migrated in this requirement. There is no pilot subset. +9. **Definition of done:** code complete, `make test` green, and a local compose measurement against `origin/main` that shows the saving. + +## Codebase facts (verified on `origin/main` 6997dd3dbb, 2026-10-24; build on these) + +### How the stack is entered today + +- **Mechanism:** dynamic `include_role` from the application's own tasks only. No `meta/main.yml` dependency, no `import_role`, no central loader. +- **Call sites:** `sys-stk-full` 78 (in 78 roles), `sys-stk-front-proxy` 24 external plus 1 inside `sys-stk-full` (18 roles), `sys-stk-backend` 13 external plus 1 inside `sys-stk-full` (12 roles). 94 distinct consumer roles in total, all `web-app-*` or `web-svc-*`. +- **`compose_handlers_flush`:** of the counted full/backend call sites, 29 pass `true` and 42 pass `false`. `false` defers the compose flush into the application role. +- **`sys-stk-full/tasks/main.yml`:** `:3-16` freezes the application context; `:18-28` includes front-proxy with `apply: delegate_to: groups['svc-prx-openresty'][0], run_once: true`, only when that group is non-empty; `:30-34` then includes backend. +- **`sys-stk-front-proxy/tasks/main.yml`** (one vhost per call): `:4-52` freezes domain, service key, ports, conf path (`lookup('nginx','files.domain')`), probe URL and accepted status codes; `:54-57` `sys-svc-nss`; `:59-61` `sys-stk-front-base`; `:63-69` `sys-front-tls` (letsencrypt or selfsigned, then `sys-svc-certs`); `:71-76` `sys-front-inj-all` (webserver core, cdn, inj-logout/dashboard/css/matomo/javascript); `:78-93` template choice (the role's own `templates/proxy.conf.j2`, else the basic vhost); `:95-109` vhost render with `notify: reload-openresty`; `:111-154` probe, restart decision and two `meta: flush_handlers`. +- **`sys-stk-backend/tasks/main.yml`:** `:2` comment `# run_once_sys_stk_backend: disabled`; `:4-10` database consumer resolution; `:21-27` `sys-svc-rdbms`; `:29-38` objstore; `:40-45` engine; `:47-52` `sys-svc-compose` with `compose_handlers_flush: false`; `:54-60` Keycloak SSO proxy render; `:62-66` re-owns the compose handler topics; `:68-70` flush; `:72-77` dedicated-DB realign on `STACK_HOST` with `run_once`. +- **Per-vhost parameters passed at call sites:** `domain`, `http_port`, `proxy_internal_port`, `webserver_proxy_service_key`, `webserver_vhost_flavour`, `webserver_websocket_location`, `webserver_client_max_body_size`, `webserver_extra_configuration`. +- **Multi-vhost call patterns:** loops with `loop_var: domain` in mastodon `00_core.yml:6`, peertube `:6`, wordpress `:10`, joomla `:4`; list loops in minio `:21`, seaweedfs `:40`; several explicit calls in bluesky `:58/67/88`, matrix compose `04_webserver.yml:8,17` and ansible flavor `05_proxy_wiring.yml:132,140,161`; `web-svc-html :8` and `web-svc-file :8` call it inside a `name: "{{ item }}"` loop together with dev-git; stalwart `:90` and `web-svc-cdn :19` call front-proxy in addition to full; confluence, jira and xwiki wrap full in a block guarded by `run_once_`. + +### How a shared service is loaded today (example `svc-db-postgres`) + +1. `playbook.yml:19-20` includes `tasks/stages/01_constructor.yml`. +2. `01_constructor.yml:45-48` sets `CURRENT_PLAY_APPLICATIONS`; `:159-176` adds the host to the `svc-prx-openresty` group when any `web-*` group is non-empty; `:187-191` opens the preload context; `:211-213` runs `sys-service-loader`; `:215-218` closes it. +3. `roles/sys-service-loader/tasks/main.yml`: `:18-27` ordered list (topological `run_after` sort, bucket order `universal < workstation < server < web-svc < web-app`, see `utils/roles/applications/services/registry.py:373-456`); `:29-41` required services via `query('service', โ€ฆ) | selectattr('required')`; `:43-45` `_preload_closure = application_closure(whitelist)`; `:47-56` loops `load_service.yml`; `:58-63` runs `service_loader_post_run`. +4. `load_service.yml:2-7` loads a service when `run_once_` is undefined AND (its key is required OR the role is in `group_names`) AND (no whitelist OR the role is in the closure). +5. `preload_service.yml:2-21` flushes handlers, includes `tasks/utils/load/app.yml` (`include_role` with `public: false`, `application_id=`, `domain=lookup('domain', )`), flushes again. +6. `svc-db-postgres/tasks/main.yml:2-3` runs `00_core` once (`utils/once/flag.yml` sets `run_once_`); the per-consumer part re-enters later: application โ†’ `sys-stk-full` โ†’ `sys-stk-backend:21-27` โ†’ `sys-svc-rdbms:37-49` asserts the preload, `:51-59` re-includes `svc-db-postgres` with `database_init: true`, which runs `01_init.yml` outside the run-once guard. +7. App stage: `tasks/stages/02_server.yml:25-30` โ†’ `tasks/utils/setup/group.yml` โ†’ `tasks/utils/setup/role.yml:2-5` includes each role and flushes handlers after each application (`:7-8`). + +### Service resolution facts + +- A role is discovered as a provider only if its `vars/main.yml` declares `application_id` (`registry.py:102-122`) and its primary entity has a truthy `shared`, a `provides` or alias entries (`:142-148`). +- Static resolution (`is_explicit_truth`, `registry.py:41-65`) accepts literal `True` or strings containing `in group_names`; runtime resolution (`plugins/lookup/service.py:20-38`) takes `bool` of the rendered value; `required` means some deployed application has the key enabled and shared (`:77-116`). +- No `sys-*` role is a provider today; the `sys` and `stk` categories are `invokable: false` (`meta/categories.yml:34-39,99-104`), so a `sys-*` role is never in `group_names`. +- The three stack roles already have a `meta/services.yml` (`backend`, `full`, `front-proxy`), but none is a provider. `required_by` only feeds the coverage lint and the post-deploy verifier (`cli/meta/roles/services/called/__init__.py`), it loads nothing. +- Entity-name collisions to avoid: `backend` exists in `web-app-kix:90`, `web-app-erpnext:89`, `web-app-penpot:88`; `proxy` in `kix:84`, `nextcloud:118`, `seaweedfs:87`, `openproject:81`. +- Literal true/true consumer keys need `# nocheck: dynamic-flag` and a `bond` (`tests/integration/roles/meta/services/test_dynamic_flags.py`, `tests/lint/ansible/roles/test_service_bond.py`). +- `roles/sys-service-loader/TODO.md:3-4` already lists "Implement loading of backend roles" and "Transform to general loader role for full stack". + +### Ordering and binding constraints + +- **C1 Stage timing:** the loader runs in the constructor stage, before any application stage. The batch must therefore not depend on work an application role performs in its own tasks before today's stack include. +- **C2 Handler binding:** `sys-svc-compose/handlers/compose.yml` evaluates `lookup('container', application_id, โ€ฆ)` at flush time (`:12-14,30,43-50,66,113`) and `guard_instance_dir` fails on drift (`utils/guard_instance_dir.yml:9-18`). Every compose flush must happen while `application_id` names the application whose files changed. +- **C3 Context switching:** handler loading sets `application_id` to the handler role (`tasks/utils/load/handlers.yml:8`) and rdbms switches it to the provider (`sys-svc-rdbms:51-59`); the stack freezes its context first (`front-proxy:4-46`, `full:3-16`). +- **C4 Injection before render:** the `sys-front-inj-*` roles accumulate `inj_enabled` and `webserver_csp_hashes_extra_by_app` that the vhost render consumes (`inj-all:13-51`, `front-proxy:95`); Matomo injection needs Matomo deployed and its token present (`sys-front-inj-matomo/tasks/main.yml:10-27`). +- **C5 Probe after reload:** front-proxy probes each vhost right after the reload (`:111-154`); application tasks that run after the stack need their vhost live (BigBlueButton Greenlight wait, Matomo API calls). +- **C6 Front proxy flush:** front-proxy's `meta: flush_handlers` (`:129-130,152-154`) flushes every pending handler of the play; this is why `svc-ai-mcp-adapter/tasks/instance.yml:10-16` notifies nothing during staging. +- **C7 Placement:** front-proxy runs on the OpenResty host via delegate plus `run_once` (`full:21-23`); compose prep and up only where `IS_STACK_HOST` (`sys-svc-compose/tasks/main.yml:25-34`, `group_vars/all/18_swarm.yml:11-19`); NFS subdirs on every swarm node (`main.yml:29-30`); swarm deploys through a notify (`utils/up.yml:44-49`); dedicated-DB realign on `STACK_HOST` with `run_once`; the Postgres container is re-resolved per node (`svc-db-postgres/tasks/01_init.yml:16`). +- **C8 Preload assertion:** `sys-svc-rdbms:37-49` asserts the provider was preloaded; `list_or_shoot.yml:23-35` behaves differently inside and outside the preload context. +- **C9 Serial seeding:** `baudolo-seed` runs with `throttle: 1` (`svc-bkp-volume-2-local/tasks/03_seed-database-to-backup.yml:57-68`). +- **Known bug to fix on the way:** `sys-svc-compose-ca/tasks/main.yml:2-9` prints "mark done" but never sets its run-once flag, so it runs 21-25 times per deploy. + +### Measured cost (echoserver full-6 complete and full-7 partial, `feature/updater` code, 2026-10-24) + +- Time inside stack windows: 2454 s of 5917 s parsed (40 %) in full-6, 1969 s of 6320 s (31 %) in full-7. ROLES RECAP of stack-family roles: 3066 s of 6154 s (49.8 %, includes non-stack users of `sys-svc-compose`). +- Per application, inside stack windows, excluding compose handlers: 71.7 s (full-6), 73.4 s (full-7). Steady-state means: full 68 to 69 s, front-proxy 28 to 32 s, backend 27 to 51 s. +- Split of stack window time without handlers: per-application work 61.5 to 69.4 %; host-wide work repeated per application 13.0 to 14.5 % (about 9.3 to 10.7 s per application); guard-skip overhead 3.4 to 4.1 %; one-time initialisation billed to the first application 5.4 to 9.4 %; include and dispatch overhead 8.8 to 10.5 %. +- Top repeated host-wide tasks (full-7, count x average): Reload OpenResty 24 x 0.86 s; force-recreate OpenResty 2 x 10.15 s; "Shoot post-load roles" (always skipped) 81 x 0.24 s; Validate OpenResty config 12 x 1.57 s; `sys-svc-cdn` handler overwrite 28 x 0.54 s; Wait for Postgres TCP auth 12 x 1.16 s; `resolve_host_cid` 12 x 1.07 s; "List post-load roles" 81 x 0.14 s; front-base re-import of OpenResty handlers 28 x 0.33 s; Cloudflare zone id checks 28 x 0.32 s plus cache dict 28 x 0.19 s; webserver health routing 31 x 0.23 s; re-own compose handler topics 24 x 0.29 s. (pip install and `/etc/baudolo` per seed are already fixed on the updater line.) +- Expected saving if repeated host-wide work runs once per host: about 3 to 4.5 minutes per full echoserver deploy; ceiling about 6.5 to 9 minutes if guard checks and include dispatch also disappear. Batching vhost rendering and OpenResty reloads (decision 6) is what moves beyond the lower figure. + +## Target Design (binding where it restates a decision; the rest is the starting proposal) + +### Provider role + +- One provider role owns the `stack` service. It declares `application_id` in `vars/main.yml` so the registry discovers it, and its `meta/services.yml` primary entity is `stack` with `enabled: true` and `shared: true`. Choose a role name and category that `sys-service-loader` loads in the dependency phase and that does not collide with the entity names listed above. +- Host-wide work runs once per host inside the provider (the run-once parts of `sys-svc-compose`, `sys-stk-front-base`, `sys-svc-nss`, `sys-svc-webserver-*`, `sys-svc-cdn`, handler loading, Cloudflare zone cache, service-loader routing). + +### Consumer declaration + +```yaml +stack: + enabled: true # nocheck: dynamic-flag + shared: true + bond: 1 +openresty: + enabled: true # nocheck: dynamic-flag + shared: true + bond: 1 +``` + +The exact nocheck placement and `bond` value follow `test_dynamic_flags.py` and `test_service_bond.py`. `openresty` appears only in roles that serve a vhost; `postgres`/`mariadb` keys keep their current form. + +### Per-domain vhost meta + +- Every value listed under "Per-vhost parameters passed at call sites" moves into the role's meta keyed by domain (proposal: `meta/server.yml`, one entry per domain key the role's `lookup('domains', role)` returns). Each entry carries the internal port, the proxy service key, the vhost flavour, the websocket location, the client body size and a reference to the role's extra-configuration template, so the provider can render every vhost without call-site variables. +- A role's own `templates/proxy.conf.j2` stays the template override, resolved by the provider as front-proxy does today (`:78-93`). + +### Batch algorithm (one invocation, all applications) + +1. Collect the stack consumers of the play: applications in `lookup('deployment').running` (respecting the whitelist closure) whose `stack` key is enabled and shared. +2. Per consumer and with its `application_id` frozen: stage the compose project (directories, `.env`, override, compose files, Dockerfiles, build contexts) from the role's templates and files; provision its database through the existing rdbms per-consumer path; provision objstore and engines by their keys. +3. Per consumer domain: TLS, injection accumulation, vhost render. Then one OpenResty config validation and one reload for the whole batch, followed by the per-vhost probes and the existing stale-config restart decision. +4. Per consumer, again with its `application_id` frozen: compose pull, build and up with the handler semantics of today (C2), on the stack host only (C7). +5. Only then do the application roles run. + +### Work that preceded the include + +Everything an application role does today before its stack include must be available to the provider through templates or meta, never through role tasks that run later. Known cases: `svc-ai-mcp-adapter/tasks/instance.yml` (MCP adapter build context, 16 providers), roles that render their own Dockerfiles or build contexts before calling the stack, and roles that set facts the stack reads. The implementation must list every such case from the 115 call sites and move it. + +## Acceptance Criteria + +### Provider and loading + +- [ ] One role provides the `stack` service: its `vars/main.yml` declares `application_id` and its `meta/services.yml` declares the primary entity `stack` with `shared: true` and `enabled: true`. +- [ ] `sys-service-loader` runs the `stack` service in the dependency phase, and the deploy log shows the provider's tasks ahead of every `web-app-*` and `web-svc-*` role task. +- [ ] The `stack` provider is invoked once per play and host and processes every stack consumer of the play in that invocation; its log shows one task header per stage, not one stack block per application. +- [ ] Host-wide work listed under "Measured cost" runs at most once per host per play. +- [ ] `sys-svc-compose-ca` sets its run-once flag and runs once per host. + +### Consumers + +- [ ] Every role that included `sys-stk-full`, `sys-stk-backend` or `sys-stk-front-proxy` on `origin/main` declares `stack: {enabled: true, shared: true}` in its own `meta/services.yml`. +- [ ] Every role that serves a vhost declares `openresty: {enabled: true, shared: true}`; a role without that key gets no vhost. +- [ ] A role gets its database exactly when it enables and shares `postgres` or `mariadb`; objstore and engines follow their own keys. +- [ ] No file under `roles/`, `tasks/`, `playbooks/` or `plugins/` includes, imports or names `sys-stk-full`, `sys-stk-backend` or `sys-stk-front-proxy`, and the three role directories are removed. + +### Per-domain data + +- [ ] Every per-vhost parameter from the call-site list is declared per domain in the role's meta, and the provider reads it only from there. +- [ ] `web-app-matrix` (both flavours), `web-app-stalwart`, `web-app-bluesky`, `web-app-mastodon`, `web-app-peertube`, `web-app-wordpress`, `web-app-joomla`, `web-app-minio`, `web-svc-seaweedfs`, `web-svc-cdn`, `web-svc-html` and `web-svc-file` get every vhost from their meta, with no loop or front proxy call in the role. +- [ ] Every pre-include step found in the 115 call sites (MCP adapter build contexts, role Dockerfiles, facts the stack reads) is provided through templates or meta and is in place before the provider needs it. + +### Behaviour + +- [ ] Within one invocation OpenResty validates and reloads once per batch, and a full deploy shows fewer `Reload OpenResty` handler runs than `origin/main` for the same application set. +- [ ] Compose handlers run with the `application_id` of the application whose files changed, and `guard_instance_dir` never fails on drift. +- [ ] Every vhost answers its accepted status codes before the first application role that depends on it runs. +- [ ] Swarm deploys keep today's placement: the front proxy on the OpenResty host, compose on the stack host, NFS preparation on every node, dedicated-DB realign on `STACK_HOST`. + +### Tests and lints + +- [ ] A lint fails when any file names a removed stack role, when a role with a compose project lacks the `stack` key, or when a role with a vhost lacks the `openresty` key. +- [ ] A lint fails when a role's meta lacks the per-domain vhost data for a domain the role serves. +- [ ] Every test that pinned the include shape is updated to the new contract: `tests/unit/python/roles/sys-stk-backend/test_handler_registration_order.py`, `tests/lint/ansible/roles/test_docker_calls_follow_the_stack.py`, `tests/lint/ansible/roles/test_stack_vars_resolve_in_their_own_role.py`, `test_redundant_compose_handlers_flush.py`, `test_no_leaky_proxy_role_vars.py`, `test_vhost_cross_cutting_includes.py`, `meta/test_sso_contract.py`, `modules/test_compose_repository_url_ref.py`, `tests/integration/infrastructure/services/test_frontend_spot.py`, `tests/integration/roles/run_once/test_shared_primary_entity_run_once.py`, `tests/lint/ansible/roles/test_service_core_first_task_run_once.py`, `tests/unit/python/roles/web-app-openbao/test_credential_rotation.py`. +- [ ] The post-deploy `required_by` verifier (`cli/meta/roles/services/called/__init__.py`) checks the new provider's task headers instead of the removed roles. +- [ ] `make test` passes. + +### Measurement + +- [ ] A local compose deploy of the same application set with `full_cycle=true` on this branch and on `origin/main` shows a lower total wall clock on this branch; both durations, the application set and the per-target table are recorded in the pull request. +- [ ] Every role that is green end to end in compose on `origin/main` stays green end to end in compose on this branch. + +## Out of scope + +- Changing what an application role configures after its stack has run. +- New service keys besides `stack` and the per-application `openresty` key. +- Swarm performance measurement (swarm must keep working; only compose is measured). + +## Validation + +```bash +make test +INFINITO_PLAYWRIGHT_KEEP=true make compose-deploy mode=reinstall apps="web-app-gitea web-app-nextcloud web-app-mastodon web-app-matrix web-svc-cdn web-app-bigbluebutton" full_cycle=true variant=0 2>&1 | tee /tmp/compose-deploy-stack-service.log +git grep -nE "sys-stk-(full|backend|front-proxy)" -- roles tasks playbooks plugins +grep -c "RUNNING HANDLER \[.*Reload OpenResty" /tmp/compose-deploy-stack-service.log +``` + +Run the same deploy command on `origin/main` into a second log and compare the PLAYBOOK RECAP wall clock and the reload count. + +## Implementation order + +1. Provider role, loader integration and the batch skeleton, with the host-wide work moved out of the three stack roles. +2. Per-domain meta schema plus its lint; generate the meta entries for all 94 roles from today's call-site variables. +3. Consumer migration in groups (single-vhost web-svc, single-vhost web-app, multi-vhost roles, backend-only roles), removing every include in the same change as its meta. +4. Remove the three stack roles; update the pinned tests and the `required_by` verifier. +5. `make test`, then the compose measurement against `origin/main`. + +## Commit Policy + +One concern per commit: provider and loader, meta schema and lint, each migration group, removal and test updates, measurement evidence in the pull request. No commit leaves `make test` red for the files it touches. From 269a15863fcf479660964ea08c2ca6d94ff77da2 Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Sat, 24 Oct 2026 19:58:29 +0200 Subject: [PATCH 11/17] perf(web-app-friendica,web-app-matomo): report config patches changed only on change Friendica's local.config.php patch (two sed -ri tasks) and Matomo's config.ini.php database patch (four sed -i tasks) carried no changed_when and notified compose-up, so every deploy reported them changed and force-recreated the stack: a second compose flush for Friendica (about 19 s plus a restart) and pull, up and a health wait for Matomo (about 15 s plus a restart). Matomo's trusted-hosts script was already idempotent but reported changed every run as well. Each patch is now one script under files/shell/ that dry-runs its sed expressions, compares cksum with the file, rewrites it only on a difference and prints PATCHED; the tasks set changed_when on that marker. The trusted-hosts script compares cksum before and after. Values reach the scripts through the environment and are written literally, so a password containing $ or a backtick is no longer mangled by host-shell double quotes. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../files/shell/patch_local_config.sh | 25 +++++++++++++ .../tasks/utils/patch/config.yml | 34 +++++++++--------- .../files/shell/patch_database.sh | 23 ++++++++++++ .../files/shell/patch_trusted_hosts.sh | 5 +++ .../tasks/05_patch_database.yml | 36 +++++-------------- .../tasks/06_patch_trusted_hosts.yml | 2 ++ 6 files changed, 80 insertions(+), 45 deletions(-) create mode 100644 roles/web-app-friendica/files/shell/patch_local_config.sh create mode 100644 roles/web-app-matomo/files/shell/patch_database.sh diff --git a/roles/web-app-friendica/files/shell/patch_local_config.sh b/roles/web-app-friendica/files/shell/patch_local_config.sh new file mode 100644 index 0000000000..97503095d2 --- /dev/null +++ b/roles/web-app-friendica/files/shell/patch_local_config.sh @@ -0,0 +1,25 @@ +#!/bin/sh +# Param: $1 local.config.php to rewrite in place. +# Param: PATCH_HOSTNAME database host:port, escaped for the / delimiter. +# Param: PATCH_DATABASE database name, escaped for the / delimiter. +# Param: PATCH_USERNAME database user, escaped for the / delimiter. +# Param: PATCH_PASSWORD database password, escaped for the / delimiter. +# Param: PATCH_URL system.url, escaped for the # delimiter. +# Output: "PATCHED" when the content of $1 changed, nothing when it already matched. +set -eu + +config="$1" + +rewrite() { + sed -E "$@" \ + -e "s/('hostname'\s*=>\s*')[^']*(',)/\1${PATCH_HOSTNAME}\2/" \ + -e "s/('database'\s*=>\s*')[^']*(',)/\1${PATCH_DATABASE}\2/" \ + -e "s/('username'\s*=>\s*')[^']*(',)/\1${PATCH_USERNAME}\2/" \ + -e "s/('password'\s*=>\s*')[^']*(',)/\1${PATCH_PASSWORD}\2/" \ + -e "s#('url'\s*=>\s*')[^']*(',)#\1${PATCH_URL}\2#" \ + "${config}" +} + +[ "$(rewrite | cksum)" = "$(cksum <"${config}")" ] && exit 0 +rewrite -i +echo PATCHED diff --git a/roles/web-app-friendica/tasks/utils/patch/config.yml b/roles/web-app-friendica/tasks/utils/patch/config.yml index 5f0bc0e29d..83bd41d96c 100644 --- a/roles/web-app-friendica/tasks/utils/patch/config.yml +++ b/roles/web-app-friendica/tasks/utils/patch/config.yml @@ -1,21 +1,19 @@ --- -- name: ๐Ÿฉน Patch Friendica local.config.php (DB host, name, user, system.url) - shell: > # nocheck: container-exec-resolver FRIENDICA_LOCAL_CID resolved at delegate_to runtime - container exec --user {{ FRIENDICA_USER }} {{ FRIENDICA_LOCAL_CID | quote }} - sed -ri - -e "s/('hostname'\s*=>\s*')[^']*(',)/\1{{ lookup('database', application_id, 'host') | sed_escape('/') }}:{{ lookup('database', application_id, 'port') | sed_escape('/') - }}\2/" - -e "s/('database'\s*=>\s*')[^']*(',)/\1{{ lookup('database', application_id, 'name') | sed_escape('/') }}\2/" - -e "s/('username'\s*=>\s*')[^']*(',)/\1{{ lookup('database', application_id, 'username') | sed_escape('/') }}\2/" - -e "s#('url'\s*=>\s*')[^']*(',)#\1{{ FRIENDICA_BASE_URL | sed_escape('#') }}\2#" - {{ FRIENDICA_CONFIG_FILE }} - delegate_to: "{{ FRIENDICA_HOST_NODE }}" # nocheck: include-uniform-apply-keyword - -- name: ๐Ÿฉน Patch Friendica local.config.php (DB password) - shell: > - container exec --user {{ FRIENDICA_USER }} - -e PATCH_PASSWORD={{ lookup('database', application_id, 'password') | sed_escape('/') | quote }} - {{ FRIENDICA_LOCAL_CID | quote }} - sh -c "sed -ri -e \"s/('password'\\s*=>\\s*')[^']*(',)/\\1\${PATCH_PASSWORD}\\2/\" {{ FRIENDICA_CONFIG_FILE }}" +- name: ๐Ÿฉน Patch Friendica local.config.php (DB credentials, system.url) + ansible.builtin.shell: + cmd: >- # nocheck: container-exec-resolver FRIENDICA_LOCAL_CID resolved at delegate_to runtime + container exec -i --user {{ FRIENDICA_USER }} + -e PATCH_HOSTNAME -e PATCH_DATABASE -e PATCH_USERNAME -e PATCH_PASSWORD -e PATCH_URL + {{ FRIENDICA_LOCAL_CID | quote }} + sh -s -- {{ FRIENDICA_CONFIG_FILE | quote }} + stdin: "{{ lookup('file', 'shell/patch_local_config.sh') }}" + environment: + PATCH_HOSTNAME: "{{ lookup('database', application_id, 'host') | sed_escape('/') }}:{{ lookup('database', application_id, 'port') | sed_escape('/') }}" + PATCH_DATABASE: "{{ lookup('database', application_id, 'name') | sed_escape('/') }}" + PATCH_USERNAME: "{{ lookup('database', application_id, 'username') | sed_escape('/') }}" + PATCH_PASSWORD: "{{ lookup('database', application_id, 'password') | sed_escape('/') }}" + PATCH_URL: "{{ FRIENDICA_BASE_URL | sed_escape('#') }}" delegate_to: "{{ FRIENDICA_HOST_NODE }}" + register: friendica_config_patch + changed_when: "'PATCHED' in friendica_config_patch.stdout" no_log: "{{ MASK_CREDENTIALS_IN_LOGS | bool }}" diff --git a/roles/web-app-matomo/files/shell/patch_database.sh b/roles/web-app-matomo/files/shell/patch_database.sh new file mode 100644 index 0000000000..a455d44c6b --- /dev/null +++ b/roles/web-app-matomo/files/shell/patch_database.sh @@ -0,0 +1,23 @@ +#!/bin/sh +# Param: $1 config.ini.php to rewrite in place. +# Param: PATCH_HOST database host, escaped for the / delimiter. +# Param: PATCH_NAME database name, escaped for the / delimiter. +# Param: PATCH_USERNAME database user, escaped for the / delimiter. +# Param: PATCH_PASSWORD database password, escaped for the / delimiter. +# Output: "PATCHED" when the content of $1 changed, nothing when it already matched. +set -eu + +config="$1" + +rewrite() { + sed "$@" \ + -e "s/^host *=.*/host = ${PATCH_HOST}/" \ + -e "s/^dbname *=.*/dbname = ${PATCH_NAME}/" \ + -e "s/^username *=.*/username = ${PATCH_USERNAME}/" \ + -e "s/^password *=.*/password = ${PATCH_PASSWORD}/" \ + "${config}" +} + +[ "$(rewrite | cksum)" = "$(cksum <"${config}")" ] && exit 0 +rewrite -i +echo PATCHED diff --git a/roles/web-app-matomo/files/shell/patch_trusted_hosts.sh b/roles/web-app-matomo/files/shell/patch_trusted_hosts.sh index be4cf1d636..8014a8db73 100755 --- a/roles/web-app-matomo/files/shell/patch_trusted_hosts.sh +++ b/roles/web-app-matomo/files/shell/patch_trusted_hosts.sh @@ -3,8 +3,11 @@ # Env: DOMAIN Public canonical domain to land in the first trusted_hosts slot. # Env: SERVICE Stack-internal service name (added as a trusted host). # Env: LOCAL host:port the role uses for in-host probes (added as a trusted host). +# Output: "PATCHED" when the content of CFG changed, nothing when it already matched. set -eu +before="$(cksum <"$CFG")" + ensure_host() { host="$1" if ! grep -qE "^trusted_hosts\[\][[:space:]]*= \"${host}\"$" "$CFG"; then @@ -39,3 +42,5 @@ if ! grep -qE "^enable_framed_pages[[:space:]]*=" "$CFG"; then { print } ' "$CFG" > "$CFG.tmp" && mv "$CFG.tmp" "$CFG" fi + +[ "$(cksum <"$CFG")" = "$before" ] || echo PATCHED diff --git a/roles/web-app-matomo/tasks/05_patch_database.yml b/roles/web-app-matomo/tasks/05_patch_database.yml index 1099f50aef..41e4568752 100644 --- a/roles/web-app-matomo/tasks/05_patch_database.yml +++ b/roles/web-app-matomo/tasks/05_patch_database.yml @@ -2,33 +2,15 @@ - name: "๐Ÿงญ Resolve host path to config.ini.php" ansible.builtin.include_tasks: utils/resolve/config_host.yml -- name: "๐Ÿ”„ Update DB host" - ansible.builtin.command: - argv: - - sed - - -i - - "s/^host *=.*/host = {{ lookup('database', application_id, 'host') | sed_escape('/') }}/" - - "{{ MATOMO_CONFIG_HOST }}" - -- name: "๐Ÿ”„ Update DB name" - ansible.builtin.command: - argv: - - sed - - -i - - "s/^dbname *=.*/dbname = {{ lookup('database', application_id, 'name') | sed_escape('/') }}/" - - "{{ MATOMO_CONFIG_HOST }}" - -- name: "๐Ÿ”„ Update DB user" - ansible.builtin.command: - argv: - - sed - - -i - - "s/^username *=.*/username = {{ lookup('database', application_id, 'username') | sed_escape('/') }}/" - - "{{ MATOMO_CONFIG_HOST }}" - -- name: "๐Ÿ”„ Update DB password" - ansible.builtin.shell: - cmd: sed -i "s/^password *=.*/password = ${PATCH_PASSWORD}/" {{ MATOMO_CONFIG_HOST | quote }} +- name: "๐Ÿ”„ Update DB host, name, user and password" + ansible.builtin.shell: sh -s -- {{ MATOMO_CONFIG_HOST | quote }} + args: + stdin: "{{ lookup('file', 'shell/patch_database.sh') }}" environment: + PATCH_HOST: "{{ lookup('database', application_id, 'host') | sed_escape('/') }}" + PATCH_NAME: "{{ lookup('database', application_id, 'name') | sed_escape('/') }}" + PATCH_USERNAME: "{{ lookup('database', application_id, 'username') | sed_escape('/') }}" PATCH_PASSWORD: "{{ lookup('database', application_id, 'password') | sed_escape('/') }}" + register: matomo_database_patch + changed_when: "'PATCHED' in matomo_database_patch.stdout" no_log: "{{ MASK_CREDENTIALS_IN_LOGS | bool }}" diff --git a/roles/web-app-matomo/tasks/06_patch_trusted_hosts.yml b/roles/web-app-matomo/tasks/06_patch_trusted_hosts.yml index ec5c8eb1b5..75578e23e6 100644 --- a/roles/web-app-matomo/tasks/06_patch_trusted_hosts.yml +++ b/roles/web-app-matomo/tasks/06_patch_trusted_hosts.yml @@ -18,4 +18,6 @@ SERVICE: "{{ MATOMO_SERVICE }}" LOCAL: "{{ DOCKER_REACH_HOST }}:{{ http_port }}" delegate_to: "{{ MATOMO_HOST_NODE }}" + register: matomo_trusted_hosts_patch + changed_when: "'PATCHED' in matomo_trusted_hosts_patch.stdout" notify: compose-up From 2949d46bff9a1772372c562e8b85d1c6a22ebe99 Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Sat, 24 Oct 2026 20:07:16 +0200 Subject: [PATCH 12/17] perf(web-app-discourse): apply settings and the admin in one rails runner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Applying the site settings and ensuring the administrator were two container exec calls, each booting Rails (10.0 s and 8.8 s on echoserver). They now run as one `rails r` with apply_settings.rb and ensure_admin_user.rb concatenated in that order; the read-back stays a separate call because it proves persistence. The administrator password, the OIDC client secret and the LDAP site settings (which carry ldap_bind_password) no longer travel as `container exec -e NAME=value` argv entries: they are stdin lines that `sh -c 'IFS= read -r โ€ฆ; export โ€ฆ; exec "$@"'` exports in the container before exec'ing rails, so no secret appears in any process argv. stdin is a `|-` block because `>- {{ a ~ '\n' ~ b }}` sends a literal backslash-n in ansible-core 2.20. The read-back gets its two secrets the same way. feature/store-i18n carries 463053e061 (`-e NAME` plus `environment:`) for the same file; on merge, keep this version. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../tasks/stack_host/02_credentials.yml | 42 ++++++++----------- 1 file changed, 17 insertions(+), 25 deletions(-) diff --git a/roles/web-app-discourse/tasks/stack_host/02_credentials.yml b/roles/web-app-discourse/tasks/stack_host/02_credentials.yml index 5e6efbb6c2..9c4e6701bc 100644 --- a/roles/web-app-discourse/tasks/stack_host/02_credentials.yml +++ b/roles/web-app-discourse/tasks/stack_host/02_credentials.yml @@ -1,5 +1,5 @@ --- -- name: "โš™๏ธ Apply the site settings to the running instance" +- name: "โš™๏ธ Apply the site settings and the current administrator password" ansible.builtin.command: argv: - container @@ -10,8 +10,6 @@ - -e - "DISCOURSE_LOCAL_LOGINS={{ DISCOURSE_LOCAL_LOGIN_ENABLED | bool | string | lower }}" - -e - - "DISCOURSE_LDAP_SITE_SETTINGS={{ (DISCOURSE_PLUGINS['discourse-ldap-auth'].config.site_settings if DISCOURSE_LDAP_ENABLED | bool else {}) | to_json }}" - - -e - "DISCOURSE_OIDC_ENABLED={{ DISCOURSE_OIDC_ENABLED | bool | string | lower }}" - -e - "DISCOURSE_OIDC_VERBOSE={{ MODE_DEBUG | bool | string | lower }}" @@ -20,32 +18,23 @@ - -e - "DISCOURSE_OIDC_CLIENT_ID={{ OIDC.CLIENT.ID if DISCOURSE_OIDC_ENABLED | bool else '' }}" - -e - - "DISCOURSE_OIDC_CLIENT_SECRET={{ OIDC.CLIENT.SECRET if DISCOURSE_OIDC_ENABLED | bool else '' }}" - - -e - "DISCOURSE_OIDC_LOGOUT_REDIRECT={{ DISCOURSE_OIDC_LOGOUT_REDIRECT_URL if DISCOURSE_OIDC_ENABLED | bool else '' }}" - - "{{ DISCOURSE_CONTAINER }}" # nocheck: container-exec-resolver the launcher names the container itself, so the bare name resolves in both modes - - rails - - r - - "{{ lookup('file', 'ruby/apply_settings.rb') }}" - changed_when: false - no_log: "{{ MASK_CREDENTIALS_IN_LOGS | bool }}" - -- name: "๐Ÿ”‘ Ensure the administrator account carries the current password" - ansible.builtin.command: - argv: - - container - - exec - - -i - -e - "DISCOURSE_ADMIN_EMAIL={{ lookup('users', 'administrator').email }}" - -e - "DISCOURSE_ADMIN_USERNAME={{ lookup('users', 'administrator').username }}" - - -e - - "DISCOURSE_ADMIN_PASSWORD={{ lookup('users', 'administrator').password }}" - "{{ DISCOURSE_CONTAINER }}" # nocheck: container-exec-resolver the launcher names the container itself, so the bare name resolves in both modes + - sh + - -c + - 'IFS= read -r DISCOURSE_LDAP_SITE_SETTINGS; IFS= read -r DISCOURSE_OIDC_CLIENT_SECRET; IFS= read -r DISCOURSE_ADMIN_PASSWORD; export DISCOURSE_LDAP_SITE_SETTINGS DISCOURSE_OIDC_CLIENT_SECRET DISCOURSE_ADMIN_PASSWORD; exec "$@"' + - sh - rails - r - - "{{ lookup('file', 'ruby/ensure_admin_user.rb') }}" + - "{{ lookup('file', 'ruby/apply_settings.rb') }}\n{{ lookup('file', 'ruby/ensure_admin_user.rb') }}" + stdin: |- + {{ (DISCOURSE_PLUGINS['discourse-ldap-auth'].config.site_settings if DISCOURSE_LDAP_ENABLED | bool else {}) | to_json }} + {{ OIDC.CLIENT.SECRET if DISCOURSE_OIDC_ENABLED | bool else '' }} + {{ lookup('users', 'administrator').password }} changed_when: false no_log: "{{ MASK_CREDENTIALS_IN_LOGS | bool }}" @@ -58,15 +47,18 @@ - -e - "DISCOURSE_ADMIN_EMAIL={{ lookup('users', 'administrator').email }}" - -e - - "DISCOURSE_ADMIN_PASSWORD={{ lookup('users', 'administrator').password }}" - - -e - "DISCOURSE_OIDC_ENABLED={{ DISCOURSE_OIDC_ENABLED | bool | string | lower }}" - - -e - - "DISCOURSE_OIDC_CLIENT_SECRET={{ OIDC.CLIENT.SECRET if DISCOURSE_OIDC_ENABLED | bool else '' }}" - "{{ DISCOURSE_CONTAINER }}" # nocheck: container-exec-resolver the launcher names the container itself, so the bare name resolves in both modes + - sh + - -c + - 'IFS= read -r DISCOURSE_ADMIN_PASSWORD; IFS= read -r DISCOURSE_OIDC_CLIENT_SECRET; export DISCOURSE_ADMIN_PASSWORD DISCOURSE_OIDC_CLIENT_SECRET; exec "$@"' + - sh - rails - r - "{{ lookup('file', 'ruby/verify_settings.rb') }}" + stdin: |- + {{ lookup('users', 'administrator').password }} + {{ OIDC.CLIENT.SECRET if DISCOURSE_OIDC_ENABLED | bool else '' }} register: discourse_settings_readback changed_when: false no_log: "{{ MASK_CREDENTIALS_IN_LOGS | bool }}" From c78d5f906c9a3a9e4c3f185ebdda4e4e05ca1797 Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Sat, 24 Oct 2026 20:17:56 +0200 Subject: [PATCH 13/17] perf(web-app-openproject): apply settings and the administrator in one rails runner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three `bundle exec rails runner -` calls each booted OpenProject's Rails app (35.2 s, 33.3 s and 33.1 s on echoserver): the settings in 02_settings.yml, the administrator and the LDAP link in 04_admin.yml. They now run as one runner in 04_admin.yml with apply_settings.rb, ensure_admin_user.rb and, under header SSO, link_admin_to_ldap.rb in that order. It runs after 03_ldap.yml, which creates the ldap_auth_sources row the link needs; nothing between 02 and 04 reads the settings. 02_settings.yml keeps only the migration and becomes 02_migrate.yml. The administrator password and OPENPROJECT_RAILS_SETTINGS (which carries smtp_password) leave argv: they are the first two stdin lines, exported by `sh -c 'IFS= read -r โ€ฆ; export โ€ฆ; exec "$@"'` before rails reads the script from the rest of stdin. stdin is a `|-` block, one expression per line. On merge: feature/store-i18n 463053e061 moved these secrets to `environment:`; keep this version. feature/iam-profile-rbac 89e291e738 adds a fourth runner (reconcile_admins.rb) to 04_admin.yml; fold it into this single runner. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../tasks/01_manager_ops.yml | 6 +- .../tasks/{02_settings.yml => 02_migrate.yml} | 15 ----- roles/web-app-openproject/tasks/04_admin.yml | 56 +++++++------------ .../templates/playwright.env.j2 | 4 +- 4 files changed, 24 insertions(+), 57 deletions(-) rename roles/web-app-openproject/tasks/{02_settings.yml => 02_migrate.yml} (75%) diff --git a/roles/web-app-openproject/tasks/01_manager_ops.yml b/roles/web-app-openproject/tasks/01_manager_ops.yml index fd34a3a50f..d2c0628857 100644 --- a/roles/web-app-openproject/tasks/01_manager_ops.yml +++ b/roles/web-app-openproject/tasks/01_manager_ops.yml @@ -33,12 +33,12 @@ - name: ๐Ÿšฟ flush docker service meta: flush_handlers -- name: ๐Ÿš€ Deploy Settings - include_tasks: 02_settings.yml +- name: โ–ถ๏ธ Migrate the OpenProject database + include_tasks: 02_migrate.yml - name: ๐Ÿ”ง Setup LDAP include_tasks: 03_ldap.yml when: OPENPROJECT_LDAP_ENABLED | bool -- name: ๐Ÿ“ Create OpenProject Administrator +- name: ๐Ÿ”ง Apply settings and the OpenProject administrator include_tasks: 04_admin.yml diff --git a/roles/web-app-openproject/tasks/02_settings.yml b/roles/web-app-openproject/tasks/02_migrate.yml similarity index 75% rename from roles/web-app-openproject/tasks/02_settings.yml rename to roles/web-app-openproject/tasks/02_migrate.yml index 50a30dfd6e..0a6bd547bc 100644 --- a/roles/web-app-openproject/tasks/02_settings.yml +++ b/roles/web-app-openproject/tasks/02_migrate.yml @@ -50,18 +50,3 @@ ansible.builtin.include_tasks: "{{ lookup('path_absolute', 'roles/sys-svc-compose/tasks/utils/swarm/wait_converged.yml') }}" vars: wait_service_key: "{{ OPENPROJECT_WEB_SERVICE }}" - -- name: ๐Ÿ”ง Set settings in OpenProject (rails runner via STDIN from an external script) - ansible.builtin.shell: - cmd: >- # nocheck: container-exec-resolver OPENPROJECT_WEB_LOCAL_CID resolved at delegate_to runtime; container_address would return a manager-only subshell - container exec -i - -e OPENPROJECT_RAILS_SETTINGS={{ OPENPROJECT_RAILS_SETTINGS | to_json | quote }} - {{ OPENPROJECT_WEB_LOCAL_CID | quote }} - bash -lc "cd /app && bundle exec rails runner -" - stdin: "{{ lookup('file', 'ruby/apply_settings.rb') }}" - delegate_to: "{{ OPENPROJECT_WEB_HOST_NODE }}" - register: openproject_apply_settings - retries: 30 - delay: 5 - until: openproject_apply_settings.rc == 0 - no_log: "{{ MASK_CREDENTIALS_IN_LOGS | bool }}" diff --git a/roles/web-app-openproject/tasks/04_admin.yml b/roles/web-app-openproject/tasks/04_admin.yml index f964176b91..798c22e1a9 100644 --- a/roles/web-app-openproject/tasks/04_admin.yml +++ b/roles/web-app-openproject/tasks/04_admin.yml @@ -6,42 +6,24 @@ resolve_output_prefix: OPENPROJECT_WEB resolve_compose_cid: "{{ OPENPROJECT_WEB_CONTAINER_ADDRESS }}" -- name: โœ… Ensure administrator user exists and is admin - ansible.builtin.command: - argv: - - container - - exec - - -i - - -e - - "OPENPROJECT_ADMIN_PASSWORD={{ OPENPROJECT_ADMINISTRATOR_PASSWORD }}" - - -e - - "OPENPROJECT_ADMIN_LOGIN={{ OPENPROJECT_ADMINISTRATOR_USERNAME }}" - - -e - - "OPENPROJECT_ADMIN_EMAIL={{ OPENPROJECT_ADMINISTRATOR_EMAIL }}" - - "{{ OPENPROJECT_WEB_LOCAL_CID }}" # nocheck: container-exec-resolver CID resolved at delegate_to runtime; container_address targets a manager-only subshell - - bash - - -lc - - "cd /app && bundle exec rails runner -" - stdin: "{{ lookup('file', 'ruby/ensure_admin_user.rb') }}" +- name: ๐Ÿ”ง Apply settings, ensure the administrator and link it to LDAP + ansible.builtin.shell: + cmd: >- # nocheck: container-exec-resolver CID resolved at delegate_to runtime; container_address targets a manager-only subshell + container exec -i + -e {{ ('OPENPROJECT_ADMIN_LOGIN=' ~ OPENPROJECT_ADMINISTRATOR_USERNAME) | quote }} + -e {{ ('OPENPROJECT_ADMIN_EMAIL=' ~ OPENPROJECT_ADMINISTRATOR_EMAIL) | quote }} + {% if OPENPROJECT_HEADER_SSO_ENABLED | bool %}-e {{ ('OPENPROJECT_LDAP_NAME=' ~ OPENPROJECT_LDAP.name) | quote }}{% endif %} + {{ OPENPROJECT_WEB_LOCAL_CID | quote }} + sh -c 'IFS= read -r OPENPROJECT_ADMIN_PASSWORD; IFS= read -r OPENPROJECT_RAILS_SETTINGS; export OPENPROJECT_ADMIN_PASSWORD OPENPROJECT_RAILS_SETTINGS; exec "$@"' sh bash -lc 'cd /app && bundle exec rails runner -' + stdin: |- + {{ OPENPROJECT_ADMINISTRATOR_PASSWORD }} + {{ OPENPROJECT_RAILS_SETTINGS | to_json }} + {{ lookup('file', 'ruby/apply_settings.rb') }} + {{ lookup('file', 'ruby/ensure_admin_user.rb') }} + {% if OPENPROJECT_HEADER_SSO_ENABLED | bool %}{{ lookup('file', 'ruby/link_admin_to_ldap.rb') }}{% endif %} delegate_to: "{{ OPENPROJECT_WEB_HOST_NODE }}" + register: openproject_rails_runner + retries: 30 + delay: 5 + until: openproject_rails_runner.rc == 0 no_log: "{{ MASK_CREDENTIALS_IN_LOGS | bool }}" - -- name: ๐Ÿ”ง Link admin to LDAP auth source so trusted-header bridge can resolve it - ansible.builtin.command: - argv: - - container - - exec - - -i - - -e - - "OPENPROJECT_LDAP_NAME={{ OPENPROJECT_LDAP.name }}" - - -e - - "OPENPROJECT_ADMIN_LOGIN={{ OPENPROJECT_ADMINISTRATOR_USERNAME }}" - - "{{ OPENPROJECT_WEB_LOCAL_CID }}" # nocheck: container-exec-resolver CID resolved at delegate_to runtime; container_address targets a manager-only subshell - - bash - - -lc - - "cd /app && bundle exec rails runner -" - stdin: "{{ lookup('file', 'ruby/link_admin_to_ldap.rb') }}" - delegate_to: "{{ OPENPROJECT_WEB_HOST_NODE }}" - when: OPENPROJECT_HEADER_SSO_ENABLED | bool - register: openproject_ldap_link - changed_when: "'Linked' in openproject_ldap_link.stdout" diff --git a/roles/web-app-openproject/templates/playwright.env.j2 b/roles/web-app-openproject/templates/playwright.env.j2 index f93ab1cbd2..2abad26e23 100644 --- a/roles/web-app-openproject/templates/playwright.env.j2 +++ b/roles/web-app-openproject/templates/playwright.env.j2 @@ -25,10 +25,10 @@ OIDC_ISSUER_URL={{ OIDC.CLIENT.ISSUER_URL | dotenv_quote }} OPENPROJECT_BASE_URL={{ lookup('tls', application_id, 'url.base') | dotenv_quote }} -# nocheck: persona-block-rationale โ€” administrator is deliberately NOT blocked: tasks/04_admin.yml:9-27 seeds a native OpenProject account whose password is the Keycloak administrator secret (files/ruby/ensure_admin_user.rb:5-18), so the journey runs in every variant; the flag stays rendered because the contract wants the opt-out state declared, not implied. +# nocheck: persona-block-rationale โ€” administrator is deliberately NOT blocked: tasks/04_admin.yml:9-29 seeds a native OpenProject account whose password is the Keycloak administrator secret (files/ruby/ensure_admin_user.rb:5-18), so the journey runs in every variant; the flag stays rendered because the contract wants the opt-out state declared, not implied. PERSONA_ADMINISTRATOR_BLOCKED=false -# nocheck: persona-block-rationale โ€” OpenProject is seeded with the administrator only (files/ruby/ensure_admin_user.rb:5-18); biber owns no OpenProject account and is auto-provisioned solely through the oauth2-proxy trusted-header bridge onto the LDAP auth source (meta/services.yml:96-109, vars/main.yml:45, tasks/04_admin.yml:29-45), which the sso: false variants remove (see README "Persona contract opt-outs"). +# nocheck: persona-block-rationale โ€” OpenProject is seeded with the administrator only (files/ruby/ensure_admin_user.rb:5-18); biber owns no OpenProject account and is auto-provisioned solely through the oauth2-proxy trusted-header bridge onto the LDAP auth source (meta/services.yml:96-109, vars/main.yml:69, tasks/04_admin.yml:9-29), which the sso: false variants remove (see README "Persona contract opt-outs"). PERSONA_BIBER_BLOCKED={{ 'false' if (lookup('config', application_id, 'services.sso.enabled') | bool) else 'true' }} SSO_SERVICE_ENABLED={{ lookup('config', application_id, 'services.sso.enabled') | string | lower }} From 8e35d053a24a6ccd72af947ea14d1fd80918fc17 Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Sat, 24 Oct 2026 20:56:00 +0200 Subject: [PATCH 14/17] fix(security): stop seeding demo data on every deploy, gate it on MODE_DEMO Snipe-IT ran `php artisan db:seed --force` on every deploy. Its upstream DatabaseSeeder (v8.8.3) truncates users, assets, licenses, orders and settings, deletes uploads and creates about 2100 users, among them the superusers `admin` and `snipe`, all with the factory password `password`, plus a settings row against the public demo LDAP ldap.forumsys.com. Every redeploy wiped real data and planted known superuser logins. Decidim ran `rails db:seed` on every deploy with failed_when: false. The generated seeds call Decidim.seed!, whose participatory-space seeds run in production too: from the second deploy on they add demo processes and, once a non-admin user exists, confirmed users with admin, moderator and valuator roles and the password `decidim123456789`. Snipe-IT now only creates its settings row when none exists, through files/php/ensure_settings.php and SNIPE_IT_INITIAL_SETTINGS, and records what already-seeded installs keep in TODO.md (taken verbatim from d12b57be90 on feature/store-i18n). Decidim's seed task is gone; the role creates its organization, pages and admin itself. MODE_DEMO (group_vars/all/01_modes.yml, default false) brings both demo seeds back on purpose; Decidim's then runs after the organization exists and fails loudly. tests/lint/ansible/tasks/test_no_default_database_seeder.py rejects Laravel's default seeder and Rails db:seed/seed:replant/setup/ reset/prepare unless the task is gated on MODE_DEMO; a Rails task whose seeds are production-safe carries `# nocheck: default-seeder `, which Mastodon does (find_or_create seeds, development-only admin seed). Co-Authored-By: Claude Opus 5.5 (1M context) --- .../actions/testing/suppression.md | 1 + group_vars/all/01_modes.yml | 1 + roles/web-app-decidim/tasks/01_setup.yml | 22 +- roles/web-app-mastodon/tasks/02_setup.yml | 1 + roles/web-app-snipe-it/TODO.md | 4 + .../files/php/ensure_settings.php | 19 + roles/web-app-snipe-it/tasks/01_install.yml | 18 +- roles/web-app-snipe-it/vars/main.yml | 13 + .../tasks/test_no_default_database_seeder.py | 432 ++++++++++++++++++ 9 files changed, 496 insertions(+), 15 deletions(-) create mode 100644 roles/web-app-snipe-it/TODO.md create mode 100644 roles/web-app-snipe-it/files/php/ensure_settings.php create mode 100644 tests/lint/ansible/tasks/test_no_default_database_seeder.py diff --git a/docs/contributing/actions/testing/suppression.md b/docs/contributing/actions/testing/suppression.md index 56eff73931..5006f5c8a2 100644 --- a/docs/contributing/actions/testing/suppression.md +++ b/docs/contributing/actions/testing/suppression.md @@ -114,6 +114,7 @@ The placement rule is per check. The catalog "Position" column uses these labels | `galaxy-git-parity` | same or above | [test_galaxy_entries_have_git_source.py](../../../../tests/lint/repository/dependencies/test_galaxy_entries_have_git_source.py) | Exempts a `requirements/requirements.galaxy.yml` collection from the requirement to also carry a `type: git` entry in `requirements/requirements.git.yml`. Reserved for collections that publish no usable git remote; without the git entry a `galaxy.ansible.com` outage fails `make install`, because the fallback in `scripts/install/ansible.sh` cannot resolve the collection either. | | `inventory-variable` | same or above | [variables.py](../../../../cli/administration/inventory/validate/variables.py) | Allows a top-level key in an inventory's `host_vars/**` or `group_vars/**` that no `group_vars`, role `defaults` or role `vars` declares and that the validator does not list as read without a declaration. Default path: move the value to the variable the deploy reads, or delete it. Reserved for a helper the inventory reads itself, such as a domain its own templates interpolate. Place it on or directly above the key's line. | | `localhost-become` | same or above | [test_localhost_delegation_drops_become.py](../../../../tests/lint/ansible/tasks/test_localhost_delegation_drops_become.py) | Allows a task delegated to `localhost` to keep the play's `become`. Default path: set `become: false` on the task or its enclosing block. Reserved for a step that needs root on the controller, such as a mount or a write below `/var/lib/infinito`; the marker MUST name that step. Place it on or directly above the `delegate_to:` line. | +| `default-seeder` | same or above | [test_no_default_database_seeder.py](../../../../tests/lint/ansible/tasks/test_no_default_database_seeder.py) | Allows a task to load a Rails application's `db/seeds.rb` through `db:seed`, `db:seed:replant`, `db:setup`, `db:reset` or `db:prepare`. Default path: drop the seed or gate it on `when: MODE_DEMO \| bool`, because upstream seeds fill a development database and may plant users with a known password. Reserved for seeds verified safe to repeat in production; the marker MUST name why. Place it on or directly above the line where the command value starts (a block scalar's `key: >` line, an inline string, or the first `argv:` item). It never exempts Laravel's default `DatabaseSeeder`, which takes `--class=` instead. | | `loop-var-shadow` | same or above | [test_loop_var_shadowing.py](../../../../tests/lint/ansible/tasks/test_loop_var_shadowing.py) | Allows a `loop_var` to take the name of a variable declared in `group_vars/**` or in a role's `vars`/`defaults`. Inside the loop that name carries the item, so every lookup, template and nested include reading it sees the item instead, and ansible only warns. Reserved for a loop whose body reads the name as the item on purpose, typically because the item IS that value: state which meaning the body relies on. | | `firewall-fragment-accept` | same or above | [test_firewall_fragment_verdicts.py](../../../../tests/lint/ansible/jinja/test_firewall_fragment_verdicts.py) | Allows a rule in a `roles/*/templates/nftables.conf.j2` fragment to end in an `accept` verdict. Default path: express the intent as a `drop` of what must not pass, or put the rule in the chain carrying the policy it has to overrule, because an `accept` terminates only its own base chain and every other chain at that hook still runs, docker's `drop` policy on forwarded packets included. Reserved for an accept whose only job is to skip the fragment's own later rules. | | `shell-template` | head (first 30 lines) | [test_no_shell_templates.py](../../../../tests/lint/filesystem/shell/test_no_shell_templates.py) | Allows a `*.sh.j2` to exist. Default path: ship the script as a plain `.sh` under `files/shell/` and pass its values as `ExecStart` arguments or `Environment=` entries, because shellcheck, the repository's `.sh` guards and every test skip a Jinja template, so nothing reads what actually runs on the host. Reserved for a script whose shape is generated, not just its values. Place the marker BELOW the shebang, never above it, and prefer the Jinja comment form so it does not reach the rendered script. | diff --git a/group_vars/all/01_modes.yml b/group_vars/all/01_modes.yml index 42271490c1..7b49b95c60 100644 --- a/group_vars/all/01_modes.yml +++ b/group_vars/all/01_modes.yml @@ -9,3 +9,4 @@ MODE_CLEANUP: true # Cleanup unused files and configurations MODE_ASSERT: "{{ MODE_DEBUG | bool }}" # Executes validation tasks during the run. MODE_ASSERT_LOCAL: "{{ (MODE_ASSERT | bool) and (IS_STACK_HOST | bool) }}" # MODE_ASSERT, but only on the host that owns the asserted state (preload flags, openresty config, ...). MODE_BACKUP: true # Executes the Backup before the deployment +MODE_DEMO: false # Seeds the upstream demo data of roles that ship it on every deploy, replacing their data and adding demo accounts with known passwords; never enable on production servers diff --git a/roles/web-app-decidim/tasks/01_setup.yml b/roles/web-app-decidim/tasks/01_setup.yml index 72c8b660b8..bdee01699d 100644 --- a/roles/web-app-decidim/tasks/01_setup.yml +++ b/roles/web-app-decidim/tasks/01_setup.yml @@ -26,17 +26,7 @@ changed_when: decidim_migrate_result.stdout_lines | default([]) | length > 0 delegate_to: "{{ DECIDIM_HOST_NODE }}" -- name: "๐ŸŒฑ Decidim | Seed database on first deploy" - ansible.builtin.shell: > # nocheck: container-exec-resolver DECIDIM_LOCAL_CID resolved at delegate_to runtime - container exec -e RAILS_ENV={{ ENVIRONMENT | quote }} - {{ DECIDIM_LOCAL_CID | quote }} - bundle exec rails db:seed - register: decidim_seed_result - changed_when: decidim_seed_result.stdout_lines | default([]) | length > 0 - failed_when: false - delegate_to: "{{ DECIDIM_HOST_NODE }}" - -- name: "๐Ÿ›๏ธ Decidim | Create the organization that db:seed skips in production" +- name: "๐Ÿ›๏ธ Decidim | Create the organization" ansible.builtin.shell: > # nocheck: container-exec-resolver DECIDIM_LOCAL_CID resolved at delegate_to runtime container exec -i -e RAILS_ENV={{ ENVIRONMENT | quote }} -e DECIDIM_ORG_HOST={{ domain | quote }} @@ -50,6 +40,16 @@ when: ENVIRONMENT == 'production' delegate_to: "{{ DECIDIM_HOST_NODE }}" +- name: "๐ŸŒฑ Decidim | Seed the demo data" + ansible.builtin.shell: > # nocheck: container-exec-resolver DECIDIM_LOCAL_CID resolved at delegate_to runtime + container exec -e RAILS_ENV={{ ENVIRONMENT | quote }} + {{ DECIDIM_LOCAL_CID | quote }} + bundle exec rails db:seed + register: decidim_demo_seed + changed_when: true + when: MODE_DEMO | bool + delegate_to: "{{ DECIDIM_HOST_NODE }}" + - name: "๐Ÿ“„ Decidim | Ensure default static pages exist (terms-of-service)" ansible.builtin.shell: > # nocheck: container-exec-resolver DECIDIM_LOCAL_CID resolved at delegate_to runtime container exec -i -e RAILS_ENV={{ ENVIRONMENT | quote }} diff --git a/roles/web-app-mastodon/tasks/02_setup.yml b/roles/web-app-mastodon/tasks/02_setup.yml index 4de7887b2c..3d0858ffa8 100644 --- a/roles/web-app-mastodon/tasks/02_setup.yml +++ b/roles/web-app-mastodon/tasks/02_setup.yml @@ -7,6 +7,7 @@ resolve_compose_cid: "{{ MASTODON_CONTAINER_ADDRESS }}" - name: "๐ŸŒฑ MASTODON | Seed database (idempotent)" + # nocheck: default-seeder Mastodon seeds are find_or_create and its admin seed is development-only ansible.builtin.shell: > # nocheck: container-exec-resolver runtime-resolved CID, delegated to the hosting node container exec {{ MASTODON_LOCAL_CID | quote }} bash -lc 'bin/rails db:seed' diff --git a/roles/web-app-snipe-it/TODO.md b/roles/web-app-snipe-it/TODO.md new file mode 100644 index 0000000000..2bbcf1d2e8 --- /dev/null +++ b/roles/web-app-snipe-it/TODO.md @@ -0,0 +1,4 @@ +# TODO + +- Existing installs that were seeded before the demo seeder was removed still hold the demo users, among them superusers with the upstream factory password; their removal is an open operator decision. +- Installs seeded before the demo seeder was removed keep `ldap_enabled = 1` against `ldap://ldap.forumsys.com` while the `ldap` service is off, because `tasks/02_ldap.yml` only runs with `ldap` on; the upstream login controller then attempts an LDAP login with the submitted user name and password before local authentication (one form sign-in took 1.9 s on the local stack). Fresh installs do not get these settings. Options: write `ldap_enabled = 0` through the Setting model when the service is off. diff --git a/roles/web-app-snipe-it/files/php/ensure_settings.php b/roles/web-app-snipe-it/files/php/ensure_settings.php new file mode 100644 index 0000000000..f94f853526 --- /dev/null +++ b/roles/web-app-snipe-it/files/php/ensure_settings.php @@ -0,0 +1,19 @@ +make(Illuminate\Contracts\Console\Kernel::class)->bootstrap(); + +if (\App\Models\Setting::query()->exists()) { + echo "unchanged\n"; + exit(0); +} + +$settings = new \App\Models\Setting; +$settings->forceFill(json_decode(getenv('SNIPE_IT_INITIAL_SETTINGS'), true, 512, JSON_THROW_ON_ERROR)); +if (! $settings->save()) { + fwrite(STDERR, json_encode($settings->getErrors()) . "\n"); + exit(1); +} +echo "changed\n"; diff --git a/roles/web-app-snipe-it/tasks/01_install.yml b/roles/web-app-snipe-it/tasks/01_install.yml index ffe2af0b82..f9275dc2ed 100644 --- a/roles/web-app-snipe-it/tasks/01_install.yml +++ b/roles/web-app-snipe-it/tasks/01_install.yml @@ -35,13 +35,23 @@ or ('Migrated:' in snipeit_migrate.stdout) failed_when: snipeit_migrate.rc != 0 -- name: ๐ŸŒฑ Seed Snipe-IT database (optional, safe to rerun if seeder is idempotent) +- name: ๐ŸŒฑ Seed the Snipe-IT demo data ansible.builtin.shell: >- container exec -i -e APP_KEY="{{ SNIPE_IT_APP_KEY }}" -e XDG_CONFIG_HOME=/tmp {{ SNIPE_IT_CONTAINER_ADDRESS }} php -d memory_limit={{ lookup('service_memory_mb', application_id, SNIPE_IT_SERVICE, 0.25) }}M artisan db:seed --force - register: snipeit_seed - changed_when: "'Seeding:' in snipeit_seed.stdout" - failed_when: snipeit_seed.rc != 0 + register: snipeit_demo_seed + changed_when: true + when: MODE_DEMO | bool + +- name: "โš™๏ธ Create the Snipe-IT settings row when none exists" + ansible.builtin.shell: + cmd: >- + container exec -i + -e SNIPE_IT_INITIAL_SETTINGS={{ SNIPE_IT_INITIAL_SETTINGS | to_json | quote }} + {{ SNIPE_IT_CONTAINER_ADDRESS }} php + stdin: "{{ lookup('file', 'php/ensure_settings.php') }}" + register: snipeit_settings + changed_when: snipeit_settings.stdout_lines | last == 'changed' diff --git a/roles/web-app-snipe-it/vars/main.yml b/roles/web-app-snipe-it/vars/main.yml index 689acd417d..672cbf6998 100644 --- a/roles/web-app-snipe-it/vars/main.yml +++ b/roles/web-app-snipe-it/vars/main.yml @@ -14,6 +14,19 @@ SNIPE_IT_SERVICE: "{{ entity_name }}" SNIPE_IT_CONTAINER_ADDRESS: "{{ lookup('container_address', application_id, SNIPE_IT_SERVICE) }}" SNIPE_IT_APP_KEY: "{{ lookup('config', application_id, 'secrets.credentials.app_key') }}" +SNIPE_IT_INITIAL_SETTINGS: + alert_email: "{{ lookup('users', 'administrator').email }}" + alerts_enabled: 1 + auto_increment_assets: 0 + brand: 1 + default_currency: "USD" + full_multiple_companies_support: 0 + locale: "{{ HOST_LL_CC }}" + next_auto_tag_base: 1 + pwd_secure_min: 10 + site_name: "Snipe-IT" + zerofill_count: 0 + SNIPE_IT_LDAP_ENABLED: "{{ lookup('config', application_id, 'services.ldap.enabled') }}" SNIPE_IT_TLS_ENABLED: "{{ lookup('tls', application_id, 'enabled') | bool }}" diff --git a/tests/lint/ansible/tasks/test_no_default_database_seeder.py b/tests/lint/ansible/tasks/test_no_default_database_seeder.py new file mode 100644 index 0000000000..3ac3f4ece5 --- /dev/null +++ b/tests/lint/ansible/tasks/test_no_default_database_seeder.py @@ -0,0 +1,432 @@ +"""Lint: no Ansible task runs an application's default database seeds. + +Laravel: ``php artisan db:seed`` without ``--class`` and ``php artisan migrate +--seed`` without ``--seeder`` run ``Database\\Seeders\\DatabaseSeeder``; naming +``DatabaseSeeder`` explicitly counts too. Rails: ``rails``/``rake`` +``db:seed``, ``db:seed:replant``, ``db:setup``, ``db:reset`` and ``db:prepare`` +load ``db/seeds.rb``, ``db:prepare`` whenever it sets up an empty database. + +A Laravel task MAY seed a named class that is safe to repeat:: + + php artisan db:seed --class=Database\\Seeders\\RolesSeeder --force + +A task whose own or enclosing ``when:`` carries ``MODE_DEMO | bool`` or bare +``MODE_DEMO`` as an ``and`` term, without ``or``, is allowed. A Rails task whose +seeds are safe in production carries ``# nocheck: default-seeder `` on, +or directly above, the line where its command value starts: a block scalar's +``key: >`` line, an inline string, or the first ``argv:`` item. The marker +never exempts Laravel's default seeder. + +Scans every YAML file under ``roles/*/tasks/``, ``roles/*/handlers/`` and the +top-level ``tasks/``. Module arguments are read as composed YAML, so an +``argv:`` list, a folded or literal shell block and a ``set_fact`` that +assembles an argv are all covered; the task ``name`` is not. +""" + +from __future__ import annotations + +import re +import unittest +from pathlib import Path +from typing import TYPE_CHECKING, Any + +import yaml + +from utils.annotations.suppress import is_suppressed_at +from utils.cache.files import iter_project_files_with_content +from utils.cache.yaml import dump_yaml_str + +from . import PROJECT_ROOT + +if TYPE_CHECKING: + from collections.abc import Iterator, Sequence + +_RULE = "default-seeder" +_ARTISAN = re.compile(r"\bartisan\b") +_DB_SEED = re.compile(r"(?[\w\\]+)") +_SEEDER = re.compile(r"--seeder(?:=|\s+)[\"']?(?P[\w\\]+)") +_RAILS = re.compile(r"\b(?:rails|rake)\b") +_RAILS_SEED = re.compile( + r"(? bool: + """Report whether a ``--class`` / ``--seeder`` match selects the default. + + Args: + match: the option match within one command, or ``None`` when absent. + + Returns: + ``True`` when the option is missing or names ``DatabaseSeeder``. + """ + return match is None or match.group("cls").split("\\")[-1] == _DEFAULT_SEEDER + + +def _runs_laravel_default_seeder(command: str) -> bool: + """Report whether one shell command runs the default Laravel seeder. + + Args: + command: a single command, already split from its neighbours. + + Returns: + ``True`` for ``artisan db:seed`` or ``artisan migrate --seed`` that + does not name a seeder class other than ``DatabaseSeeder``. + """ + if not _ARTISAN.search(command): + return False + if _DB_SEED.search(command): + return _names_default_seeder(_CLASS.search(command)) + if _MIGRATE.search(command) and _SEED_FLAG.search(command): + return _names_default_seeder(_SEEDER.search(command)) + return False + + +def _runs_rails_seeds(command: str) -> bool: + """Report whether one shell command loads a Rails application's seeds. + + Args: + command: a single command, already split from its neighbours. + + Returns: + ``True`` for a ``rails``/``rake`` call of a task that loads ``db/seeds.rb``. + """ + return bool(_RAILS.search(command) and _RAILS_SEED.search(command)) + + +def _runs_default_seeds(text: str, line: int, lines: Sequence[str]) -> bool: + """Report whether one module argument runs default seeds no marker exempts. + + Args: + text: the argument, possibly holding several shell commands. + line: the 1-based line where the argument's value starts. + lines: the lines of the task file. + + Returns: + ``True`` for a Laravel default seeder, or for Rails seeds without a + ``default-seeder`` marker on or directly above ``line``. + """ + commands = _COMMAND_SEPARATOR.split(text.replace("\\\n", " ")) + if any(_runs_laravel_default_seeder(command) for command in commands): + return True + return any(_runs_rails_seeds(command) for command in commands) and not ( + is_suppressed_at(lines, line, _RULE) + ) + + +def _texts(node: yaml.Node) -> Iterator[tuple[str, int]]: + """Yield the command-like strings inside a task's module arguments. + + Args: + node: any composed YAML node of a task. + + Returns: + An iterator over ``(text, line)`` pairs, ``line`` being the 1-based + line where the value starts; a list of scalars is joined with spaces + so an ``argv:`` list reads as one command. + """ + if isinstance(node, yaml.ScalarNode): + yield node.value, node.start_mark.line + 1 + elif isinstance(node, yaml.SequenceNode): + if all(isinstance(item, yaml.ScalarNode) for item in node.value): + joined = " ".join(item.value for item in node.value) + yield joined, node.start_mark.line + 1 + else: + for item in node.value: + yield from _texts(item) + elif isinstance(node, yaml.MappingNode): + for _key, value in node.value: + yield from _texts(value) + + +def _requires_demo(when: yaml.Node | None) -> bool: + """Report whether a ``when:`` value requires ``MODE_DEMO``. + + Args: + when: the composed ``when:`` value of a task or block, or ``None``. + + Returns: + ``True`` when one condition without ``or`` carries ``MODE_DEMO | bool`` + or bare ``MODE_DEMO`` as one of its ``and`` terms. + """ + if isinstance(when, yaml.ScalarNode): + conditions = [when.value] + elif isinstance(when, yaml.SequenceNode): + conditions = [ + item.value for item in when.value if isinstance(item, yaml.ScalarNode) + ] + else: + return False + return any( + not _OR.search(condition) + and any(_DEMO_TERM.fullmatch(term.strip()) for term in _AND.split(condition)) + for condition in conditions + ) + + +def _offending_tasks( + tasks: yaml.Node | None, lines: Sequence[str], gated: bool +) -> Iterator[str]: + """Yield the name of every task that runs default seeds. + + Args: + tasks: a composed file root or ``block``/``rescue``/``always`` value. + lines: the lines of the task file. + gated: whether an enclosing block's ``when:`` requires ``MODE_DEMO``. + + Returns: + An iterator over offending task names, nested tasks included. + """ + if not isinstance(tasks, yaml.SequenceNode): + return + for task in tasks.value: + if not isinstance(task, yaml.MappingNode): + continue + items = { + key.value: value + for key, value in task.value + if isinstance(key, yaml.ScalarNode) + } + task_gated = gated or _requires_demo(items.get("when")) + for key in _NESTED_TASK_KEYS: + yield from _offending_tasks(items.get(key), lines, task_gated) + if task_gated: + continue + texts = ( + pair + for key, value in items.items() + if key != "name" and key not in _NESTED_TASK_KEYS + for pair in _texts(value) + ) + if any(_runs_default_seeds(text, line, lines) for text, line in texts): + name = items.get("name") + yield name.value if isinstance(name, yaml.ScalarNode) else "" + + +def _offending_task_names(content: str) -> list[str]: + """Return the name of every task in a task file that runs default seeds. + + Args: + content: the text of a YAML task file. + + Returns: + The offending task names in file order. + """ + lines = content.splitlines() + return [ + name + for document in yaml.compose_all(content, Loader=yaml.SafeLoader) + for name in _offending_tasks(document, lines, gated=False) + ] + + +def _is_scan_target(rel: str) -> bool: + """Report whether a repository-relative YAML path holds Ansible tasks. + + Args: + rel: POSIX path relative to the project root. + + Returns: + ``True`` for the top-level ``tasks/`` tree and a role's ``tasks/`` or + ``handlers/`` tree. + """ + parts = rel.split("/") + if parts[0] == "tasks": + return True + return parts[0] == "roles" and len(parts) > 3 and parts[2] in _TASK_DIRS + + +class TestNoDefaultDatabaseSeeder(unittest.TestCase): + def _offenders(self, tasks: list[dict[str, Any]]) -> list[str]: + return _offending_task_names(dump_yaml_str(tasks)) + + def test_no_task_runs_default_seeds(self) -> None: + offenders: list[str] = [] + for path, content in iter_project_files_with_content( + extensions=(".yml", ".yaml"), exclude_tests=True + ): + rel = Path(path).relative_to(PROJECT_ROOT).as_posix() + if not _is_scan_target(rel): + continue + if "artisan" not in content and not _RAILS_SEED.search(content): + continue + offenders.extend( + f"{rel}: task '{name}'" for name in _offending_task_names(content) + ) + + self.assertFalse( + offenders, + "these tasks run an application's default database seeds, which " + "upstream ships for development data and which may truncate " + "production tables or create known logins:\n " + + "\n ".join(offenders) + + "\n\nFix: drop the seed, or gate it on `when: MODE_DEMO | bool`. " + "Laravel: name a seeder class that is safe to repeat with " + "`artisan db:seed --class=` / " + "`artisan migrate --seed --seeder=`. Rails seeds verified " + "safe in production: add `# nocheck: default-seeder ` on or " + "directly above the line where the command value starts.", + ) + + def test_detector_separates_the_laravel_default_from_named_seeders( + self, + ) -> None: + flagged = ( + "php artisan db:seed --force", + "php -d memory_limit=512M artisan db:seed --force", + "php artisan db:seed --class=DatabaseSeeder --force", + "php artisan db:seed --class='Database\\Seeders\\DatabaseSeeder'", + "php artisan migrate --force --seed", + "php artisan migrate:fresh --seed --seeder=DatabaseSeeder", + "sh -c 'php artisan migrate --force && php artisan db:seed --force'", + "container exec -i app \\\n php artisan db:seed \\\n --force", + ) + passed = ( + "php artisan migrate --force", + "php artisan migrate:status", + "php artisan db:seed --class=RolesSeeder --force", + "php artisan db:seed --class Database\\Seeders\\RolesSeeder", + "php artisan migrate --seed --seeder=RolesSeeder", + "php artisan migrate --force && php artisan db:seed --class=RolesSeeder", + ) + for command in flagged: + with self.subTest(flagged=command): + tasks = [{"name": "t", "ansible.builtin.shell": command}] + self.assertEqual(self._offenders(tasks), ["t"]) + for command in passed: + with self.subTest(passed=command): + tasks = [{"name": "t", "ansible.builtin.shell": command}] + self.assertEqual(self._offenders(tasks), []) + + def test_detector_flags_every_rails_task_that_loads_the_seeds(self) -> None: + flagged = ( + "bin/rails db:seed", + "bundle exec rails db:seed", + "RAILS_ENV=production bundle exec rake db:setup", + "rails db:reset", + "bin/rails db:prepare", + "bin/rails db:seed:replant", + "container exec cid bash -lc 'bin/rails db:migrate && bin/rails db:seed'", + ) + passed = ( + "bundle exec rails db:migrate", + "bin/rails db:schema:load", + "bin/rails db:test:prepare", + "bundle exec rails runner -", + "npm run db:seed", + ) + for command in flagged: + with self.subTest(flagged=command): + tasks = [{"name": "t", "ansible.builtin.shell": command}] + self.assertEqual(self._offenders(tasks), ["t"]) + for command in passed: + with self.subTest(passed=command): + tasks = [{"name": "t", "ansible.builtin.shell": command}] + self.assertEqual(self._offenders(tasks), []) + + def test_detector_reads_argv_lists_and_nested_blocks(self) -> None: + laravel = ["container", "exec", "app", "php", "artisan", "db:seed", "--force"] + rails = ["container", "exec", "cid", "bin/rails", "db:seed"] + tasks = [ + {"name": "only the title says artisan db:seed", "debug": {"msg": "x"}}, + {"block": [{"name": "argv", "ansible.builtin.command": {"argv": laravel}}]}, + {"name": "rails", "ansible.builtin.command": {"argv": rails}}, + ] + self.assertEqual(self._offenders(tasks), ["argv", "rails"]) + + def test_a_mode_demo_gate_allows_the_default_seeds(self) -> None: + gates = ( + "MODE_DEMO | bool", + "MODE_DEMO|bool", + "MODE_DEMO", + "ENVIRONMENT == 'production' and MODE_DEMO | bool", + ["ENVIRONMENT == 'production'", "MODE_DEMO | bool"], + ) + for when in gates: + with self.subTest(gated=when): + tasks = [ + {"name": "l", "shell": "php artisan db:seed --force", "when": when}, + {"name": "r", "shell": "bin/rails db:seed", "when": when}, + ] + self.assertEqual(self._offenders(tasks), []) + block = [ + { + "block": [{"name": "r", "shell": "bin/rails db:seed"}], + "rescue": [{"name": "l", "shell": "php artisan db:seed --force"}], + "when": "MODE_DEMO | bool", + } + ] + self.assertEqual(self._offenders(block), []) + + def test_a_missing_or_negated_mode_demo_gate_is_flagged(self) -> None: + gates = ( + None, + "ENVIRONMENT == 'production'", + "not MODE_DEMO | bool", + "not MODE_DEMO", + "MODE_DEMO | bool or ENVIRONMENT == 'development'", + ["ENVIRONMENT == 'production'", "not MODE_DEMO | bool"], + ) + for when in gates: + with self.subTest(ungated=when): + task: dict[str, Any] = {"name": "t", "shell": "bin/rails db:seed"} + if when is not None: + task["when"] = when + self.assertEqual(self._offenders([task]), ["t"]) + + def test_a_marker_exempts_rails_seeds_only(self) -> None: + exempt = ( + ( + "- name: t\n" + " # nocheck: default-seeder seeds are find_or_create\n" + " ansible.builtin.shell: > # nocheck: container-exec-resolver cid\n" + " container exec cid\n" + " bash -lc 'bin/rails db:seed'\n" + ), + "- name: t\n shell: bin/rails db:seed # nocheck: default-seeder why\n", + ( + "- name: t\n" + " command:\n" + " argv: # nocheck: default-seeder why\n" + " - bin/rails\n" + " - db:seed\n" + ), + ) + flagged = ( + ( + "- name: t\n" + " # nocheck: default-seeder why\n" + " shell: php artisan db:seed --force\n" + ), + ( + "- name: t # nocheck: default-seeder why\n" + " become: true\n" + " shell: bin/rails db:seed\n" + ), + ( + "- name: t\n" + " ansible.builtin.shell: > # nocheck: container-exec-resolver cid\n" + " container exec cid\n" + " bash -lc 'bin/rails db:seed'\n" + ), + ) + for text in exempt: + with self.subTest(exempt=text): + self.assertEqual(_offending_task_names(text), []) + for text in flagged: + with self.subTest(flagged=text): + self.assertEqual(_offending_task_names(text), ["t"]) + + +if __name__ == "__main__": + unittest.main() From e4090ffce99915ad667c6200cf26b32d9e2be055 Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Sat, 24 Oct 2026 20:58:26 +0200 Subject: [PATCH 15/17] refactor(web-app-friendica,web-app-matomo): group the patch scripts in a patch folder 269a15863f added a second `patch_*.sh` to the shell files of Friendica and Matomo, so test_prefix_grouped_files reported two prefix groups. The scripts now live in files/shell/patch/ without the prefix (ldapauth_autocreate.sh, local_config.sh, database.sh, trusted_hosts.sh) and every task that ships them points at the new path. Content is unchanged apart from the usage line of ldapauth_autocreate.sh. On merge: feature/store-i18n changes the old patch_trusted_hosts.sh (SIBLINGS); apply that change to patch/trusted_hosts.sh. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../ldapauth_autocreate.sh} | 2 +- .../shell/{patch_local_config.sh => patch/local_config.sh} | 0 roles/web-app-friendica/tasks/03_addons.yml | 2 +- roles/web-app-friendica/tasks/utils/patch/config.yml | 2 +- .../files/shell/{patch_database.sh => patch/database.sh} | 0 .../shell/{patch_trusted_hosts.sh => patch/trusted_hosts.sh} | 0 roles/web-app-matomo/tasks/05_patch_database.yml | 2 +- roles/web-app-matomo/tasks/06_patch_trusted_hosts.yml | 2 +- 8 files changed, 5 insertions(+), 5 deletions(-) rename roles/web-app-friendica/files/shell/{patch_ldapauth_autocreate.sh => patch/ldapauth_autocreate.sh} (96%) rename roles/web-app-friendica/files/shell/{patch_local_config.sh => patch/local_config.sh} (100%) rename roles/web-app-matomo/files/shell/{patch_database.sh => patch/database.sh} (100%) rename roles/web-app-matomo/files/shell/{patch_trusted_hosts.sh => patch/trusted_hosts.sh} (100%) diff --git a/roles/web-app-friendica/files/shell/patch_ldapauth_autocreate.sh b/roles/web-app-friendica/files/shell/patch/ldapauth_autocreate.sh similarity index 96% rename from roles/web-app-friendica/files/shell/patch_ldapauth_autocreate.sh rename to roles/web-app-friendica/files/shell/patch/ldapauth_autocreate.sh index 9fa9f95ba1..f2c1c18f4a 100644 --- a/roles/web-app-friendica/files/shell/patch_ldapauth_autocreate.sh +++ b/roles/web-app-friendica/files/shell/patch/ldapauth_autocreate.sh @@ -18,7 +18,7 @@ # returns whenever the friendica image is re-pulled. # # Usage: -# patch_ldapauth_autocreate.sh EXEC_PREFIX LDAPAUTH_PATH +# ldapauth_autocreate.sh EXEC_PREFIX LDAPAUTH_PATH # EXEC_PREFIX full container-exec command prefix as a single arg # (typical: 'docker compose exec -T --user root # friendica') diff --git a/roles/web-app-friendica/files/shell/patch_local_config.sh b/roles/web-app-friendica/files/shell/patch/local_config.sh similarity index 100% rename from roles/web-app-friendica/files/shell/patch_local_config.sh rename to roles/web-app-friendica/files/shell/patch/local_config.sh diff --git a/roles/web-app-friendica/tasks/03_addons.yml b/roles/web-app-friendica/tasks/03_addons.yml index 31f4a32523..8b75a657a5 100644 --- a/roles/web-app-friendica/tasks/03_addons.yml +++ b/roles/web-app-friendica/tasks/03_addons.yml @@ -59,7 +59,7 @@ - name: "๐Ÿฉน Patch upstream friendica-addons/ldapauth autocreate return shape" script: >- # nocheck: container-exec-resolver FRIENDICA_LOCAL_CID resolved at delegate_to runtime - shell/patch_ldapauth_autocreate.sh "container exec -i --user root {{ FRIENDICA_LOCAL_CID | quote }}" + shell/patch/ldapauth_autocreate.sh "container exec -i --user root {{ FRIENDICA_LOCAL_CID | quote }}" "{{ FRIENDICA_APPLICATION_DIR }}/addon/ldapauth/ldapauth.php" args: executable: /bin/bash diff --git a/roles/web-app-friendica/tasks/utils/patch/config.yml b/roles/web-app-friendica/tasks/utils/patch/config.yml index 83bd41d96c..556528c37f 100644 --- a/roles/web-app-friendica/tasks/utils/patch/config.yml +++ b/roles/web-app-friendica/tasks/utils/patch/config.yml @@ -6,7 +6,7 @@ -e PATCH_HOSTNAME -e PATCH_DATABASE -e PATCH_USERNAME -e PATCH_PASSWORD -e PATCH_URL {{ FRIENDICA_LOCAL_CID | quote }} sh -s -- {{ FRIENDICA_CONFIG_FILE | quote }} - stdin: "{{ lookup('file', 'shell/patch_local_config.sh') }}" + stdin: "{{ lookup('file', 'shell/patch/local_config.sh') }}" environment: PATCH_HOSTNAME: "{{ lookup('database', application_id, 'host') | sed_escape('/') }}:{{ lookup('database', application_id, 'port') | sed_escape('/') }}" PATCH_DATABASE: "{{ lookup('database', application_id, 'name') | sed_escape('/') }}" diff --git a/roles/web-app-matomo/files/shell/patch_database.sh b/roles/web-app-matomo/files/shell/patch/database.sh similarity index 100% rename from roles/web-app-matomo/files/shell/patch_database.sh rename to roles/web-app-matomo/files/shell/patch/database.sh diff --git a/roles/web-app-matomo/files/shell/patch_trusted_hosts.sh b/roles/web-app-matomo/files/shell/patch/trusted_hosts.sh similarity index 100% rename from roles/web-app-matomo/files/shell/patch_trusted_hosts.sh rename to roles/web-app-matomo/files/shell/patch/trusted_hosts.sh diff --git a/roles/web-app-matomo/tasks/05_patch_database.yml b/roles/web-app-matomo/tasks/05_patch_database.yml index 41e4568752..9d1db31813 100644 --- a/roles/web-app-matomo/tasks/05_patch_database.yml +++ b/roles/web-app-matomo/tasks/05_patch_database.yml @@ -5,7 +5,7 @@ - name: "๐Ÿ”„ Update DB host, name, user and password" ansible.builtin.shell: sh -s -- {{ MATOMO_CONFIG_HOST | quote }} args: - stdin: "{{ lookup('file', 'shell/patch_database.sh') }}" + stdin: "{{ lookup('file', 'shell/patch/database.sh') }}" environment: PATCH_HOST: "{{ lookup('database', application_id, 'host') | sed_escape('/') }}" PATCH_NAME: "{{ lookup('database', application_id, 'name') | sed_escape('/') }}" diff --git a/roles/web-app-matomo/tasks/06_patch_trusted_hosts.yml b/roles/web-app-matomo/tasks/06_patch_trusted_hosts.yml index 75578e23e6..86e5108416 100644 --- a/roles/web-app-matomo/tasks/06_patch_trusted_hosts.yml +++ b/roles/web-app-matomo/tasks/06_patch_trusted_hosts.yml @@ -11,7 +11,7 @@ application_id: "web-app-matomo" ansible.builtin.shell: sh -s args: - stdin: "{{ lookup('file', role_path ~ '/files/shell/patch_trusted_hosts.sh') }}" + stdin: "{{ lookup('file', role_path ~ '/files/shell/patch/trusted_hosts.sh') }}" environment: CFG: "{{ MATOMO_CONFIG_HOST }}" DOMAIN: "{{ lookup('domain', 'web-app-matomo') }}" From 3f331cd8b81a2edcaf5ab260f2e459f6a48ec35c Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Sat, 24 Oct 2026 21:00:44 +0200 Subject: [PATCH 16/17] perf(web-app-moodle): converge plugin settings in one CLI run per plugin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 04_oidc.yml and 05_ldap_sync.yml looped over 69 auth_oidc and 73 auth_ldap settings and ran one `container exec โ€ฆ php cfg.php --set` per setting; every exec booted Moodle once, 62 s plus 73 s per deploy on echoserver. tasks/utils/plugin_config.yml stages files/php/provision/plugin_config.php in the container and runs it once per plugin with `{component, settings}` as JSON on stdin. The script validates the whole payload before writing, refuses `core` and settings forced in config.php, and for each setting compares get_config with the wanted value and calls set_config plus add_to_config_log only on a difference, as cfg.php does; set_config invalidates the plugin's config cache itself (Moodle v5.3.0 moodlelib.php). It prints `CHANGED `, which drives changed_when. bind_pw and clientsecret no longer sit on the argv of every exec as `--set="โ€ฆ"`; they only travel in the stdin payload. Values that the old host-shell double quotes altered are now written exactly, so the first run may report them changed. Evidence: PHPUnit in the test container 42/42 including the 9 new PluginConfigTest cases; Templar render of both mappings identical to the old loop items (69/69, 73/73). On merge: feature/iam-profile-rbac a46d93ff96 replaces several field_map values with OIDC.ATTRIBUTES.*; carry them into the new mappings. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../files/php/provision/plugin_config.php | 57 +++++ roles/web-app-moodle/tasks/04_oidc.yml | 151 ++++++------ roles/web-app-moodle/tasks/05_ldap_sync.yml | 159 ++++++------ .../tasks/utils/plugin_config.yml | 22 ++ roles/web-app-moodle/vars/main.yml | 1 + .../files/provision/PluginConfigTest.php | 232 ++++++++++++++++++ 6 files changed, 462 insertions(+), 160 deletions(-) create mode 100644 roles/web-app-moodle/files/php/provision/plugin_config.php create mode 100644 roles/web-app-moodle/tasks/utils/plugin_config.yml create mode 100644 tests/unit/php/roles/web-app-moodle/files/provision/PluginConfigTest.php diff --git a/roles/web-app-moodle/files/php/provision/plugin_config.php b/roles/web-app-moodle/files/php/provision/plugin_config.php new file mode 100644 index 0000000000..522e6390fb --- /dev/null +++ b/roles/web-app-moodle/files/php/provision/plugin_config.php @@ -0,0 +1,57 @@ +", "settings": {"": , ...}} + * Stdout: + * last line "CHANGED ", n = settings whose stored value differed and was written + * Exit: + * 2 malformed payload, 4 a setting is forced in config.php + */ +define('CLI_SCRIPT', true); + +$payload = json_decode((string) stream_get_contents(STDIN), true, 512, JSON_THROW_ON_ERROR); +if ( + !is_array($payload) + || !is_string($payload['component'] ?? null) + || in_array($payload['component'], ['', 'core', 'moodle'], true) + || !is_array($payload['settings'] ?? null) + || array_is_list($payload['settings']) +) { + fwrite(STDERR, 'stdin must be {"component": "", "settings": {"": , ...}}' . PHP_EOL); + exit(2); +} + +$component = $payload['component']; +$settings = []; +foreach ($payload['settings'] as $name => $value) { + if (!is_string($value) && !is_int($value)) { + fwrite(STDERR, "{$component}/{$name} must be a string or an integer" . PHP_EOL); + exit(2); + } + $settings[$name] = (string) $value; +} + +require(getenv('MOODLE_CONFIG')); + +$forced = array_intersect_key($settings, $CFG->forced_plugin_settings[$component] ?? []); +if ($forced) { + fwrite(STDERR, "set in config.php for {$component}, unable to change: " . implode(', ', array_keys($forced)) . PHP_EOL); + exit(4); +} + +$changed = 0; +foreach ($settings as $name => $value) { + $old = get_config($component, $name); + if ($value !== $old) { + set_config($name, $value, $component); + add_to_config_log($name, $old, $value, $component); + $changed++; + } +} + +echo 'CHANGED ', $changed, PHP_EOL; diff --git a/roles/web-app-moodle/tasks/04_oidc.yml b/roles/web-app-moodle/tasks/04_oidc.yml index 2eaf016e11..4349df8908 100644 --- a/roles/web-app-moodle/tasks/04_oidc.yml +++ b/roles/web-app-moodle/tasks/04_oidc.yml @@ -22,84 +22,79 @@ failed_when: false - name: "๐Ÿ”ง Set Moodle OIDC configuration via CLI" - no_log: "{{ MASK_CREDENTIALS_IN_LOGS | bool }}" - loop: - - { name: "idptype", value: 3 } - - { name: "clientauthmethod", value: 1 } - - { name: "clientid", value: "{{ OIDC.CLIENT.ID }}" } - - { name: "clientsecret", value: "{{ OIDC.CLIENT.SECRET }}" } - - { name: "opname", value: "{{ OIDC.BUTTON_TEXT }}" } - - { name: "oidcscope", value: "{{ MOODLE_OIDC_SCOPES }}" } - - { name: "authendpoint", value: "{{ OIDC.CLIENT.AUTHORIZE_URL }}" } - - { name: "tokenendpoint", value: "{{ OIDC.CLIENT.TOKEN_URL }}" } - - { name: "bindingusernameclaim", value: "{{ OIDC.ATTRIBUTES.USERNAME }}" } - - { name: "single_sign_off", value: 1 } - - { name: "logouturi", value: "{{ OIDC.CLIENT.LOGOUT_URL }}" } - - { name: "icon", value: "moodle:t/lock" } - - { name: "field_map_firstname", value: "given_name" } - - { name: "field_lock_firstname", value: "locked" } - - { name: "field_updatelocal_firstname", value: "oneveryloginupdate" } - - { name: "field_map_lastname", value: "family_name" } - - { name: "field_lock_lastname", value: "locked" } - - { name: "field_updatelocal_lastname", value: "oneveryloginupdate" } - - { name: "field_map_middlename", value: "middle_name" } - - { name: "field_lock_middlename", value: "locked" } - - { name: "field_updatelocal_middlename", value: "oneveryloginupdate" } - - { name: "field_map_alternatename", value: "nickname" } - - { name: "field_lock_alternatename", value: "locked" } - - { name: "field_updatelocal_alternatename", value: "oneveryloginupdate" } - - { name: "field_map_email", value: "email" } - - { name: "field_lock_email", value: "locked" } - - { name: "field_updatelocal_email", value: "oneveryloginupdate" } - - { name: "field_map_phone1", value: "phone_number" } - - { name: "field_lock_phone1", value: "locked" } - - { name: "field_updatelocal_phone1", value: "oneveryloginupdate" } - - { name: "field_map_phone2", value: "phone_number_alt" } - - { name: "field_lock_phone2", value: "locked" } - - { name: "field_updatelocal_phone2", value: "oneveryloginupdate" } - - { name: "field_map_address", value: "address" } - - { name: "field_lock_address", value: "locked" } - - { name: "field_updatelocal_address", value: "oneveryloginupdate" } - - { name: "field_map_city", value: "locality" } - - { name: "field_lock_city", value: "locked" } - - { name: "field_updatelocal_city", value: "oneveryloginupdate" } - - { name: "field_map_country", value: "country" } - - { name: "field_lock_country", value: "locked" } - - { name: "field_updatelocal_country", value: "oneveryloginupdate" } - - { name: "field_map_institution", value: "institution" } - - { name: "field_lock_institution", value: "locked" } - - { name: "field_updatelocal_institution", value: "oneveryloginupdate" } - - { name: "field_map_department", value: "department" } - - { name: "field_lock_department", value: "locked" } - - { name: "field_updatelocal_department", value: "oneveryloginupdate" } - - { name: "field_map_description", value: "description" } - - { name: "field_lock_description", value: "locked" } - - { name: "field_updatelocal_description", value: "oneveryloginupdate" } - - { name: "field_map_idnumber", value: "id_number" } - - { name: "field_lock_idnumber", value: "locked" } - - { name: "field_updatelocal_idnumber", value: "oneveryloginupdate" } - - { name: "field_map_url", value: "website" } - - { name: "field_lock_url", value: "locked" } - - { name: "field_updatelocal_url", value: "oneveryloginupdate" } - - { name: "field_map_lang", value: "locale" } - - { name: "field_lock_lang", value: "locked" } - - { name: "field_updatelocal_lang", value: "oneveryloginupdate" } - - { name: "field_map_timezone", value: "zoneinfo" } - - { name: "field_lock_timezone", value: "locked" } - - { name: "field_updatelocal_timezone", value: "oneveryloginupdate" } - - { name: "field_map_firstnamephonetic", value: "name_phonetic_given" } - - { name: "field_lock_firstnamephonetic", value: "locked" } - - { name: "field_updatelocal_firstnamephonetic", value: "oneveryloginupdate" } - - { name: "field_map_lastnamephonetic", value: "name_phonetic_family" } - - { name: "field_lock_lastnamephonetic", value: "locked" } - - { name: "field_updatelocal_lastnamephonetic", value: "oneveryloginupdate" } - loop_control: - label: "{{ item.name }}" - shell: > - container exec --user {{ MOODLE_RUNTIME_USER }} {{ MOODLE_CONTAINER_ADDRESS }} - php {{ MOODLE_CLI_DIR }}/cfg.php --component=auth_oidc - --name={{ item.name }} --set="{{ item.value }}" - changed_when: false + include_tasks: utils/plugin_config.yml + vars: + moodle_plugin_config_component: auth_oidc + moodle_plugin_config_settings: + idptype: 3 + clientauthmethod: 1 + clientid: "{{ OIDC.CLIENT.ID }}" + clientsecret: "{{ OIDC.CLIENT.SECRET }}" + opname: "{{ OIDC.BUTTON_TEXT }}" + oidcscope: "{{ MOODLE_OIDC_SCOPES }}" + authendpoint: "{{ OIDC.CLIENT.AUTHORIZE_URL }}" + tokenendpoint: "{{ OIDC.CLIENT.TOKEN_URL }}" + bindingusernameclaim: "{{ OIDC.ATTRIBUTES.USERNAME }}" + single_sign_off: 1 + logouturi: "{{ OIDC.CLIENT.LOGOUT_URL }}" + icon: "moodle:t/lock" + field_map_firstname: "given_name" + field_lock_firstname: "locked" + field_updatelocal_firstname: "oneveryloginupdate" + field_map_lastname: "family_name" + field_lock_lastname: "locked" + field_updatelocal_lastname: "oneveryloginupdate" + field_map_middlename: "middle_name" + field_lock_middlename: "locked" + field_updatelocal_middlename: "oneveryloginupdate" + field_map_alternatename: "nickname" + field_lock_alternatename: "locked" + field_updatelocal_alternatename: "oneveryloginupdate" + field_map_email: "email" + field_lock_email: "locked" + field_updatelocal_email: "oneveryloginupdate" + field_map_phone1: "phone_number" + field_lock_phone1: "locked" + field_updatelocal_phone1: "oneveryloginupdate" + field_map_phone2: "phone_number_alt" + field_lock_phone2: "locked" + field_updatelocal_phone2: "oneveryloginupdate" + field_map_address: "address" + field_lock_address: "locked" + field_updatelocal_address: "oneveryloginupdate" + field_map_city: "locality" + field_lock_city: "locked" + field_updatelocal_city: "oneveryloginupdate" + field_map_country: "country" + field_lock_country: "locked" + field_updatelocal_country: "oneveryloginupdate" + field_map_institution: "institution" + field_lock_institution: "locked" + field_updatelocal_institution: "oneveryloginupdate" + field_map_department: "department" + field_lock_department: "locked" + field_updatelocal_department: "oneveryloginupdate" + field_map_description: "description" + field_lock_description: "locked" + field_updatelocal_description: "oneveryloginupdate" + field_map_idnumber: "id_number" + field_lock_idnumber: "locked" + field_updatelocal_idnumber: "oneveryloginupdate" + field_map_url: "website" + field_lock_url: "locked" + field_updatelocal_url: "oneveryloginupdate" + field_map_lang: "locale" + field_lock_lang: "locked" + field_updatelocal_lang: "oneveryloginupdate" + field_map_timezone: "zoneinfo" + field_lock_timezone: "locked" + field_updatelocal_timezone: "oneveryloginupdate" + field_map_firstnamephonetic: "name_phonetic_given" + field_lock_firstnamephonetic: "locked" + field_updatelocal_firstnamephonetic: "oneveryloginupdate" + field_map_lastnamephonetic: "name_phonetic_family" + field_lock_lastnamephonetic: "locked" + field_updatelocal_lastnamephonetic: "oneveryloginupdate" - name: "๐ŸŸข Enable the auth_oidc plugin in Moodle's core auth list" shell: > diff --git a/roles/web-app-moodle/tasks/05_ldap_sync.yml b/roles/web-app-moodle/tasks/05_ldap_sync.yml index aeaa866c19..74887b8819 100644 --- a/roles/web-app-moodle/tasks/05_ldap_sync.yml +++ b/roles/web-app-moodle/tasks/05_ldap_sync.yml @@ -12,88 +12,83 @@ changed_when: false - name: "๐Ÿ”ง Configure auth_ldap connection + field mapping" - loop: - - { name: "host_url", value: "{{ LDAP.SERVER.URI }}" } - - { name: "ldap_version", value: 3 } - - { name: "start_tls", value: 0 } - - { name: "user_type", value: "rfc2307" } - - { name: "contexts", value: "{{ LDAP.DN.OU.USERS }}" } - - { name: "search_sub", value: 1 } - - { name: "user_attribute", value: "uid" } - - { name: "objectclass", value: "(objectClass=inetOrgPerson)" } - - { name: "memberattribute", value: "memberOf" } - - { name: "memberattribute_isdn", value: 1 } - - { name: "bind_dn", value: "{{ LDAP.DN.ADMINISTRATOR.DATA }}" } - - { name: "bind_pw", value: "{{ LDAP.BIND_CREDENTIAL }}" } - - { name: "creators", value: "*" } - - { name: "auth_user_create", value: "{{ MOODLE_OIDC_ENABLED | bool | ternary(0, 1) }}" } - - { name: "removeuser", value: 1 } # 1 = suspend (preserve history); 2 = delete - - { name: "sync_suspended", value: 1 } - - { name: "field_map_firstname", value: "givenName" } - - { name: "field_lock_firstname", value: "locked" } - - { name: "field_updatelocal_firstname", value: "oncreate" } - - { name: "field_map_lastname", value: "sn" } - - { name: "field_lock_lastname", value: "locked" } - - { name: "field_updatelocal_lastname", value: "oncreate" } - - { name: "field_map_middlename", value: "initials" } - - { name: "field_lock_middlename", value: "locked" } - - { name: "field_updatelocal_middlename", value: "oncreate" } - - { name: "field_map_alternatename", value: "displayName" } - - { name: "field_lock_alternatename", value: "locked" } - - { name: "field_updatelocal_alternatename", value: "oncreate" } - - { name: "field_map_email", value: "mail" } - - { name: "field_lock_email", value: "locked" } - - { name: "field_updatelocal_email", value: "oncreate" } - - { name: "field_map_phone1", value: "telephoneNumber" } - - { name: "field_lock_phone1", value: "locked" } - - { name: "field_updatelocal_phone1", value: "oncreate" } - - { name: "field_map_phone2", value: "mobile" } - - { name: "field_lock_phone2", value: "locked" } - - { name: "field_updatelocal_phone2", value: "oncreate" } - - { name: "field_map_address", value: "street" } - - { name: "field_lock_address", value: "locked" } - - { name: "field_updatelocal_address", value: "oncreate" } - - { name: "field_map_city", value: "l" } - - { name: "field_lock_city", value: "locked" } - - { name: "field_updatelocal_city", value: "oncreate" } - - { name: "field_map_country", value: "c" } - - { name: "field_lock_country", value: "locked" } - - { name: "field_updatelocal_country", value: "oncreate" } - - { name: "field_map_institution", value: "o" } - - { name: "field_lock_institution", value: "locked" } - - { name: "field_updatelocal_institution", value: "oncreate" } - - { name: "field_map_department", value: "ou" } - - { name: "field_lock_department", value: "locked" } - - { name: "field_updatelocal_department", value: "oncreate" } - - { name: "field_map_description", value: "description" } - - { name: "field_lock_description", value: "locked" } - - { name: "field_updatelocal_description", value: "oncreate" } - - { name: "field_map_idnumber", value: "employeeNumber" } - - { name: "field_lock_idnumber", value: "locked" } - - { name: "field_updatelocal_idnumber", value: "oncreate" } - - { name: "field_map_url", value: "labeledURI" } - - { name: "field_lock_url", value: "locked" } - - { name: "field_updatelocal_url", value: "oncreate" } - - { name: "field_map_lang", value: "preferredLanguage" } - - { name: "field_lock_lang", value: "locked" } - - { name: "field_updatelocal_lang", value: "oncreate" } - - { name: "field_map_firstnamephonetic", value: "infinitoNamePhoneticGiven" } - - { name: "field_lock_firstnamephonetic", value: "locked" } - - { name: "field_updatelocal_firstnamephonetic", value: "oncreate" } - - { name: "field_map_lastnamephonetic", value: "infinitoNamePhoneticFamily" } - - { name: "field_lock_lastnamephonetic", value: "locked" } - - { name: "field_updatelocal_lastnamephonetic", value: "oncreate" } - - { name: "field_map_timezone", value: "infinitoTimezone" } - - { name: "field_lock_timezone", value: "locked" } - - { name: "field_updatelocal_timezone", value: "oncreate" } - loop_control: - label: "{{ item.name }}" - shell: > - container exec --user {{ MOODLE_RUNTIME_USER }} {{ MOODLE_CONTAINER_ADDRESS }} - php {{ MOODLE_CLI_DIR }}/cfg.php --component=auth_ldap - --name={{ item.name }} --set="{{ item.value }}" - no_log: "{{ MASK_CREDENTIALS_IN_LOGS | bool }}" - changed_when: false + include_tasks: utils/plugin_config.yml + vars: + moodle_plugin_config_component: auth_ldap + moodle_plugin_config_settings: + host_url: "{{ LDAP.SERVER.URI }}" + ldap_version: 3 + start_tls: 0 + user_type: "rfc2307" + contexts: "{{ LDAP.DN.OU.USERS }}" + search_sub: 1 + user_attribute: "uid" + objectclass: "(objectClass=inetOrgPerson)" + memberattribute: "memberOf" + memberattribute_isdn: 1 + bind_dn: "{{ LDAP.DN.ADMINISTRATOR.DATA }}" + bind_pw: "{{ LDAP.BIND_CREDENTIAL }}" + creators: "*" + auth_user_create: "{{ MOODLE_OIDC_ENABLED | bool | ternary(0, 1) }}" + removeuser: 1 # 1 = suspend (preserve history); 2 = delete + sync_suspended: 1 + field_map_firstname: "givenName" + field_lock_firstname: "locked" + field_updatelocal_firstname: "oncreate" + field_map_lastname: "sn" + field_lock_lastname: "locked" + field_updatelocal_lastname: "oncreate" + field_map_middlename: "initials" + field_lock_middlename: "locked" + field_updatelocal_middlename: "oncreate" + field_map_alternatename: "displayName" + field_lock_alternatename: "locked" + field_updatelocal_alternatename: "oncreate" + field_map_email: "mail" + field_lock_email: "locked" + field_updatelocal_email: "oncreate" + field_map_phone1: "telephoneNumber" + field_lock_phone1: "locked" + field_updatelocal_phone1: "oncreate" + field_map_phone2: "mobile" + field_lock_phone2: "locked" + field_updatelocal_phone2: "oncreate" + field_map_address: "street" + field_lock_address: "locked" + field_updatelocal_address: "oncreate" + field_map_city: "l" + field_lock_city: "locked" + field_updatelocal_city: "oncreate" + field_map_country: "c" + field_lock_country: "locked" + field_updatelocal_country: "oncreate" + field_map_institution: "o" + field_lock_institution: "locked" + field_updatelocal_institution: "oncreate" + field_map_department: "ou" + field_lock_department: "locked" + field_updatelocal_department: "oncreate" + field_map_description: "description" + field_lock_description: "locked" + field_updatelocal_description: "oncreate" + field_map_idnumber: "employeeNumber" + field_lock_idnumber: "locked" + field_updatelocal_idnumber: "oncreate" + field_map_url: "labeledURI" + field_lock_url: "locked" + field_updatelocal_url: "oncreate" + field_map_lang: "preferredLanguage" + field_lock_lang: "locked" + field_updatelocal_lang: "oncreate" + field_map_firstnamephonetic: "infinitoNamePhoneticGiven" + field_lock_firstnamephonetic: "locked" + field_updatelocal_firstnamephonetic: "oncreate" + field_map_lastnamephonetic: "infinitoNamePhoneticFamily" + field_lock_lastnamephonetic: "locked" + field_updatelocal_lastnamephonetic: "oncreate" + field_map_timezone: "infinitoTimezone" + field_lock_timezone: "locked" + field_updatelocal_timezone: "oncreate" - name: "๐ŸŸข Enable auth_ldap plugin" shell: > diff --git a/roles/web-app-moodle/tasks/utils/plugin_config.yml b/roles/web-app-moodle/tasks/utils/plugin_config.yml new file mode 100644 index 0000000000..6efff74262 --- /dev/null +++ b/roles/web-app-moodle/tasks/utils/plugin_config.yml @@ -0,0 +1,22 @@ +--- +# Param: moodle_plugin_config_component - frankenstyle plugin the settings belong to, e.g. auth_ldap +# Param: moodle_plugin_config_settings - mapping of setting name to its string or integer value +- name: "๐Ÿ“„ Stage the plugin settings applier in the Moodle container" + ansible.builtin.shell: > + container exec -i --user {{ MOODLE_RUNTIME_USER }} {{ MOODLE_CONTAINER_ADDRESS }} + tee {{ MOODLE_PLUGIN_CONFIG_SCRIPT | quote }} > /dev/null + args: + stdin: "{{ lookup('file', 'php/provision/plugin_config.php') }}" + changed_when: false + +- name: "๐Ÿ”ง Apply the {{ moodle_plugin_config_component }} settings in one Moodle CLI run" + ansible.builtin.shell: > + container exec -i --user {{ MOODLE_RUNTIME_USER }} + -e MOODLE_CONFIG={{ MOODLE_CONFIG | quote }} + {{ MOODLE_CONTAINER_ADDRESS }} + php {{ MOODLE_PLUGIN_CONFIG_SCRIPT | quote }} + args: + stdin: "{{ {'component': moodle_plugin_config_component, 'settings': moodle_plugin_config_settings} | to_json }}" + register: moodle_plugin_config + changed_when: moodle_plugin_config.stdout is search('^CHANGED [1-9]', multiline=True) + no_log: "{{ MASK_CREDENTIALS_IN_LOGS | bool }}" diff --git a/roles/web-app-moodle/vars/main.yml b/roles/web-app-moodle/vars/main.yml index 1bd301b8ca..8336709be2 100644 --- a/roles/web-app-moodle/vars/main.yml +++ b/roles/web-app-moodle/vars/main.yml @@ -39,6 +39,7 @@ MOODLE_OIDC_PLUGIN_RELPATH: "{{ [MOODLE_PUBLIC_SUBDIR, MOODLE_OIDC_PLUGIN_URL_PA MOODLE_OIDC_PLUGIN_DIR: "{{ [MOODLE_CODE_DIR, MOODLE_OIDC_PLUGIN_RELPATH] | path_join }}" MOODLE_CLI_DIR: "{{ [MOODLE_CODE_DIR, MOODLE_CLI_RELPATH] | path_join }}" MOODLE_CONFIG: "{{ [MOODLE_CODE_DIR, 'config.php'] | path_join }}" +MOODLE_PLUGIN_CONFIG_SCRIPT: "/tmp/infinito-plugin-config.php" MOODLE_RUNTIME_USER: "www-data" MOODLE_RUNTIME_USER_GROUP: "{{ MOODLE_RUNTIME_USER }}:{{ MOODLE_RUNTIME_USER }}" diff --git a/tests/unit/php/roles/web-app-moodle/files/provision/PluginConfigTest.php b/tests/unit/php/roles/web-app-moodle/files/provision/PluginConfigTest.php new file mode 100644 index 0000000000..b6909528d3 --- /dev/null +++ b/tests/unit/php/roles/web-app-moodle/files/provision/PluginConfigTest.php @@ -0,0 +1,232 @@ +forced_plugin_settings = json_decode((string) getenv('STUB_FORCED'), true); + + function stub_state(): array { + $file = getenv('STUB_STATE'); + return is_file($file) + ? json_decode(file_get_contents($file), true) + : ['config' => [], 'log' => []]; + } + + function stub_store(array $state): void { + file_put_contents(getenv('STUB_STATE'), json_encode($state)); + } + + function get_config($plugin, $name = null) { + return stub_state()['config'][$plugin][$name] ?? false; + } + + function set_config($name, $value, $plugin = null) { + $state = stub_state(); + $state['config'][$plugin][$name] = $value; + stub_store($state); + return true; + } + + function add_to_config_log($name, $oldvalue, $value, $plugin) { + $state = stub_state(); + $state['log'][] = [$plugin, $name, $oldvalue, $value]; + stub_store($state); + } + PHP; + + private string $config; + + private string $state; + + protected function setUp(): void + { + $this->config = (string) tempnam(sys_get_temp_dir(), 'mdlcfg'); + $this->state = (string) tempnam(sys_get_temp_dir(), 'mdlplg'); + file_put_contents($this->config, self::MOODLE); + unlink($this->state); + } + + protected function tearDown(): void + { + unlink($this->config); + if (is_file($this->state)) { + unlink($this->state); + } + } + + /** + * Store plugin settings the way an earlier run or an administrator left them. + * + * @param array> $config stored values by plugin and name + */ + private function seed(array $config): void + { + file_put_contents($this->state, json_encode(['config' => $config, 'log' => []])); + } + + /** + * Encode the stdin payload the Ansible task sends. + * + * @param array $settings setting values by name + * @param string $component plugin the settings belong to + */ + private static function payload(array $settings, string $component = 'auth_ldap'): string + { + return json_encode(['component' => $component, 'settings' => $settings]) . "\n"; + } + + /** + * Run the script against the stubbed Moodle. + * + * @param string $stdin payload piped to the script + * @param array> $forced $CFG->forced_plugin_settings + * + * @return array{0: string, 1: string, 2: int, 3: array|null} + * stdout, stderr, exit code and the stub state, null when nothing was written + */ + private function runScript(string $stdin, array $forced = []): array + { + $script = dirname(__DIR__, 7) . '/roles/web-app-moodle/files/php/provision/plugin_config.php'; + $process = proc_open( + [PHP_BINARY, $script], + [0 => ['pipe', 'r'], 1 => ['pipe', 'w'], 2 => ['pipe', 'w']], + $pipes, + null, + [ + 'MOODLE_CONFIG' => $this->config, + 'STUB_STATE' => $this->state, + 'STUB_FORCED' => json_encode((object) $forced), + 'PATH' => (string) getenv('PATH'), + ] + ); + fwrite($pipes[0], $stdin); + fclose($pipes[0]); + $stdout = (string) stream_get_contents($pipes[1]); + fclose($pipes[1]); + $stderr = (string) stream_get_contents($pipes[2]); + fclose($pipes[2]); + $code = proc_close($process); + + $state = is_file($this->state) + ? json_decode((string) file_get_contents($this->state), true) + : null; + + return [$stdout, $stderr, $code, $state]; + } + + public function testAFreshPluginGetsEverySettingStoredVerbatimAsAString(): void + { + [$stdout, , $code, $state] = $this->runScript(self::payload([ + 'host_url' => 'ldap://ldap:389', + 'ldap_version' => 3, + 'bind_pw' => self::SECRET, + ])); + + $this->assertSame(0, $code); + $this->assertSame("CHANGED 3\n", $stdout); + $this->assertSame( + ['auth_ldap' => ['host_url' => 'ldap://ldap:389', 'ldap_version' => '3', 'bind_pw' => self::SECRET]], + $state['config'] + ); + } + + public function testEveryWriteIsLoggedWithTheValueItReplaced(): void + { + $this->seed(['auth_ldap' => ['ldap_version' => '2']]); + + [, , , $state] = $this->runScript(self::payload(['ldap_version' => 3, 'start_tls' => 0])); + + $this->assertSame( + [['auth_ldap', 'ldap_version', '2', '3'], ['auth_ldap', 'start_tls', false, '0']], + $state['log'] + ); + } + + public function testOnlyTheSettingsThatDifferAreWritten(): void + { + $this->seed([ + 'auth_ldap' => ['host_url' => 'ldap://ldap:389', 'ldap_version' => '2', 'removecontexts' => '1'], + 'auth_oidc' => ['clientid' => 'moodle'], + ]); + + [$stdout, , $code, $state] = $this->runScript(self::payload([ + 'host_url' => 'ldap://ldap:389', + 'ldap_version' => 3, + ])); + + $this->assertSame(0, $code); + $this->assertSame("CHANGED 1\n", $stdout); + $this->assertSame( + [ + 'auth_ldap' => ['host_url' => 'ldap://ldap:389', 'ldap_version' => '3', 'removecontexts' => '1'], + 'auth_oidc' => ['clientid' => 'moodle'], + ], + $state['config'] + ); + } + + public function testASecondRunChangesNothing(): void + { + $payload = self::payload(['host_url' => 'ldap://ldap:389', 'search_sub' => 1, 'bind_pw' => self::SECRET]); + [, , , $first] = $this->runScript($payload); + [$stdout, , $code, $second] = $this->runScript($payload); + + $this->assertSame(0, $code); + $this->assertSame("CHANGED 0\n", $stdout); + $this->assertSame($first, $second); + } + + public function testNoValueReachesTheOutput(): void + { + [$stdout, $stderr] = $this->runScript(self::payload(['bind_pw' => self::SECRET])); + + $this->assertStringNotContainsString(self::SECRET, $stdout . $stderr); + } + + public function testASettingForcedInConfigPhpAbortsBeforeAnyWrite(): void + { + [, $stderr, $code, $state] = $this->runScript( + self::payload(['host_url' => 'ldap://ldap:389', 'bind_pw' => self::SECRET]), + ['auth_ldap' => ['bind_pw' => 'from-config-php']] + ); + + $this->assertSame(4, $code); + $this->assertStringContainsString('bind_pw', $stderr); + $this->assertNull($state); + } + + public function testAValueThatIsNeitherStringNorIntegerAbortsBeforeAnyWrite(): void + { + [, $stderr, $code, $state] = $this->runScript( + self::payload(['host_url' => 'ldap://ldap:389', 'search_sub' => true]) + ); + + $this->assertSame(2, $code); + $this->assertStringContainsString('auth_ldap/search_sub', $stderr); + $this->assertNull($state); + } + + public function testCoreIsRefusedBecauseSetConfigWouldFileItAsAPlugin(): void + { + [, , $code, $state] = $this->runScript(self::payload(['debug' => '0'], 'core')); + + $this->assertSame(2, $code); + $this->assertNull($state); + } + + public function testASettingsListIsRefused(): void + { + [, , $code, $state] = $this->runScript(self::payload(['ldap://ldap:389', 3])); + + $this->assertSame(2, $code); + $this->assertNull($state); + } +} From e80eec2acb6f46914a353d1427eb0e888f017723 Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Sat, 24 Oct 2026 21:04:10 +0200 Subject: [PATCH 17/17] fix(tls-selfsigned): cap the leaf certificate CN at 64 characters X.509 limits commonName to 64 characters, and openssl refuses a longer one while generating the CSR: ASN1_mbstring_ncopy:string too long:../crypto/asn1/a_mbstr.c:106:maxsize=64 The global leaf builds its CN as ` Global TLS`, so any primary domain longer than 53 characters fails the deploy in sys-front-tls-selfsigned. Worktree stacks hit it first: their domain is derived from the branch name (`fix-security-exec-argv-secrets-store-1153 .infinito.test` gives a 66 character CN). The CN is now truncated to 64 characters in all three branches (global, explicit subject CN, per-app primary domain). Clients verify the SAN list, not the CN, so a truncated CN changes nothing for them, and an existing certificate is only regenerated on SAN drift, as before. sys-ca-selfsigned already caps the root CA's CN. Co-Authored-By: Claude Opus 5.5 (1M context) --- roles/sys-front-tls-selfsigned/tasks/00_core.yml | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/roles/sys-front-tls-selfsigned/tasks/00_core.yml b/roles/sys-front-tls-selfsigned/tasks/00_core.yml index 94f6ecfdab..021c23d3db 100644 --- a/roles/sys-front-tls-selfsigned/tasks/00_core.yml +++ b/roles/sys-front-tls-selfsigned/tasks/00_core.yml @@ -47,12 +47,14 @@ _ss_csr: "{{ (_cert.files.cert | dirname) ~ '/req.csr' }}" _ss_cn: >- {{ - (_cert.scope == 'global') - | ternary( - (DOMAIN_PRIMARY | upper) ~ ' Global TLS', - ((tls_selfsigned_subject.CN | default('') | length > 0) - | ternary(tls_selfsigned_subject.CN, _tls.domains.primary)) - ) + ( + (_cert.scope == 'global') + | ternary( + (DOMAIN_PRIMARY | upper) ~ ' Global TLS', + ((tls_selfsigned_subject.CN | default('') | length > 0) + | ternary(tls_selfsigned_subject.CN, _tls.domains.primary)) + ) + )[:64] }} changed_when: false