At -vvv Ansible prints each module argument and result, so a password in
a command line, a request body, a header or a fact lands in the deploy
log. Nothing enforced masking outside the MCP task files, and no check
looked at parents at all.
A new lint, test_credential_tasks_are_masked, finds tasks whose printed
parts (module arguments, environment, loop) reference an identifier ending
in password, secret, token, api_key or private_key, or
secrets.credentials. A task passes when it or an enclosing block sets
no_log, when every file that pulls it in does so through include_tasks
with apply.no_log or import_tasks with no_log (transitively), when the
secret only reaches a parameter the module masks itself, or when the
module masks its values. no_log on an include_tasks task itself does not
count, because it does not reach the included tasks. The rule can be
suppressed per task with `# nocheck: credential-masking`, now listed in
the suppression catalog, and the lint tests its parent and suppression
logic on synthetic cases.
It found 39 unmasked tasks in 19 roles, all real: join tokens, client
secrets, API tokens in Cloudflare headers, database passwords in
command lines, the ChatGPT bot login body and more. Each now sets
no_log from MASK_CREDENTIALS_IN_LOGS. The sys-dns-hetzner-rdns alias
hetzner_no_log, which only restated that switch, is gone.
Not covered yet: template and copy tasks whose rendered file carries a
secret print it as a diff under --diff.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
bench new-site --admin-password lives in create_site.yml, which runs only
`when: erpnext_site_probe.rc != 0`. Once the site exists nothing touches the
Administrator again, so a credential rotated between the sync and the async pass
leaves the instance authenticating against the secret the deployment has forgotten.
The playwright trace of the run says it plainly: the native login form comes back
with "Invalid credentials, try again." for Administrator.
set-admin-password is frappe's own command and idempotent, so it runs
unconditionally, placed after the conditional bootstrap and before anything that
uses the site. The password reaches the container through `-e ADMIN_PASSWORD` with
no value on the flag, the same way the neighbouring create_site.yml passes it, so it
never appears in an argument list.
ERPNEXT_BACKEND_LOCAL_CID is available here regardless of whether the site was just
created: 01_manager_ops.yml resolves it in its first task and includes this file
twenty lines later.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every script a role ships now lives under `files/<language>/`, and the unit
suite is split the same way: `tests/unit/python/` beside new sibling trees for
the other languages. `tests/lint/filesystem/test_role_files_layout.py` keeps it
that way; directories whose shape is dictated from outside carry a `.nocheck`
naming the constraint (Docker build contexts, WordPress mu-plugins, the Joomla
extension, Odoo addons, Playwright suites, the `files/test/test.sh` harness
that `test-e2e-cli` resolves generically).
This cannot be applied in halves. Moving a file and repointing what names it
are the same change: 14 references resolve only at deploy time, so the tree is
green either before or after, never between.
Those 14 were found by static resolution over every reference class, not by the
test suite -- a full `make quality-high` was green while all of them were
broken:
* 7 `src:` values built from a role variable holding a bare filename
(svc-bkp-remote-2-local, decidim, espocrm, friendica, joomla, pixelfed x2).
The variable doubles as the destination name inside the container, so the
language segment belongs at the call site, not in the variable.
* `sys-service/tasks/04_base.yml` resolved every consumer's systemd script
through `first_found(..., errors='strict')` over `files/script.{sh,py}`.
18 roles ship exactly that and have no template fallback: the play would
have died on each.
* `sys-front-inj-javascript` fell back to `<role>/files/javascript.js`; the
interface now reads `files/javascript/javascript.js`.
* `sys-front-inj-logout/templates/head_sub.j2` built the cache-busting mtime
path through `path_absolute(... 'files', ...)`.
* 4 `ansible.builtin.script:` invocations naming a bare command
(pull.py, prune.sh, set_postgres_superuser_password.sh, ganesha-serve.sh).
* `dev-python/files/shell/install.sh` pointed at a sibling role's script.
Depth-dependent paths inside the moved trees were bumped accordingly: 59
`parents[N]` computations, 12 relative climbs, and the per-role recover CLI,
whose path `cli/administration/recover/recoverers.py` still assembled from the
pre-move location.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four tasks spelled the same container command out in full:
bash -c 'cd {{ lookup('volume', 'web-app-erpnext', 'sites').target }} && /home/frappe/frappe-bench/env/bin/python -'
Three facts were repeated in it: the sites mount, the bench root, and the role
id -- hardcoded right next to a lookup whose whole purpose is to avoid that. A
rename of the mount in meta/volumes.yml would have had to land in four places
and could half-succeed in silence, because the tasks still parse, still deploy,
and cd into a directory that no longer holds the site.
vars/main.yml derives all of it. ERPNEXT_SITES_DIR reads meta/volumes.yml, where
the path is already the single truth, addressing the role through application_id
instead of a literal name. ERPNEXT_BENCH_PYTHON builds the command from it and
takes the bench root as its dirname, so the last hardcoded
/home/frappe/frappe-bench leaves the tasks entirely. compose.yml.j2 uses the same
var for gunicorn's --chdir rather than keeping a second copy of the lookup.
Rendered with Ansible's own filters and trim_blocks, the expression reproduces
the four replaced lines byte for byte. ansible-lint passes the role at profile
production (0 failures, 0 warnings, 29 files); the volume spot lints, the
volume-lookup lint and the comment lint pass.
Left alone deliberately: files/scripts/apply/*.py name the sites path as well,
but run inside the container with no Ansible to evaluate a lookup -- they would
need an environment variable, which is a separate change. The bench-relative
binaries (env/bin/gunicorn, apps/frappe/socketio.js) and the two
site_config.json paths appear once each, so extracting them would add
indirection without removing a duplicate.
Not verified: compose.yml.j2 drives the backend entrypoint, and that path only
proves itself on a deploy.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
meta/volumes.yml says where a volume lands inside its container. Wherever a
second file spelled the same path out, the two drifted independently: the mount
moves, the consumer keeps pointing at the old place, and nothing fails until the
process looks for content that is not there. e031b7f61 fixed exactly this for
web-app-moodle; this generalises it and makes the rule hold.
The lookup gains what a caller needs. `targets` maps each mounting service to
its container path, and `target` is the one path an entry mounts at -- present
only when every mount agrees. An entry that lands at different paths per service
omits `target` rather than picking one, and the caller names the service:
"{{ lookup('volume', application_id, 'prometheus_config').target }}"
"{{ lookup('volume', 'web-app-prometheus', 'host_sys').targets['node-exporter'] }}"
66 literals across 48 role files now derive. Replacement only happened where the
path stood alone -- /srv/app inside /srv/app/data is a different path -- and
every rewritten file was re-parsed, Jinja and YAML alike. Three sites the tool
refused rather than guess were done by hand: zammad's path sat in a Jinja list
literal, seaweedfs' inside a quoted string feeding a filter, and openresty's
/var/run/docker.sock is not a mount target at all but the socket on the host,
which only happens to spell the same string. That one carries the single
nocheck.
tests/lint/repository/spot/test_volumes_path_single_spot.py keeps it that way.
roles/*/files/ is out of scope by construction: it is copied verbatim into an
image and executed there, so a Dockerfile or entrypoint has no Ansible to
evaluate a lookup with.
One defect this found, and it was in the tooling rather than the roles: deciding
ambiguity AFTER filtering short paths made web-app-prometheus' host_sys look
single-valued -- it mounts /sys for cadvisor and /host/sys for node-exporter,
and /sys is below the length filter. The template would have rendered an
undefined `.target`. Both the converter and the rule now decide ambiguity over
every target and filter afterwards, and that call reads targets['node-exporter'].
Verified by resolving rather than by reading: all 70 accesses in the diff were
run against the real registry and every one yields a non-empty path equal to the
literal it replaced. make test-lint: 328 passed.
Not verified: no Ansible render, so a call site whose surrounding context cannot
build the applications registry would still fail at deploy.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Add tests/lint/repository/test_task_file_numbering.py: numbered task
files use exactly two leading digits, a numbered *_core.yml must be
00_core.yml, per directory the numbers are unique/consecutive and start
at 01 (00 with a core file), numbered includes appear in ascending
order, and shared helpers live in tasks/utils/ instead of the numbered
chain.
Apply it across every role: rename NN_core.yml to 00_core.yml with a
consecutive down-shift of the siblings, move letter-suffix children into
a subdirectory named after their parent task, relocate helpers included
from more than one place into tasks/utils/, and update every reference
(include_tasks, path_absolute, .ansible-lint-ignore, requirement docs).
Legacy narration comments in the touched files are deleted; the upstream
xwiki.properties reference block opts out via nocheck. The unused
MAILU_IP6_PUBLIC var (only ever named in now-removed comments) is dropped.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Resolve every 3+-file same-prefix group flagged by the new prefix lint
by moving the files into a subfolder named after the shared prefix and
dropping the prefix from the file names, updating all references:
- utils/env/handlers/infinito_*.py -> infinito/ (dir/, package_cache/
nested a second level); registry imports and the four handler-dir
globs switched to iter_project_files
- utils/install/lint/ansible_*.py -> ansible/ (aliased imports)
- roles/sys-svc-compose/files/swarm_*.{py,sh} -> files/swarm/
- web-app-erpnext/web-app-zammad files/scripts/apply_*.{py,rb} -> apply/
- web-app-odoo files/sql/{select,update}-* and sql/oidc/{grant,insert,
select,update}-* -> matching subfolders
- web-app-keycloak tasks/update/05_ldap*.yml -> 05_ldap/ (internal
includes made role_path-absolute)
- plugins/filter/get_*.py -> plugins/filter/get/ (filter names are the
filters() dict keys, so Jinja usage is unaffected; ansible discovers
the one-level-deep subdir)
Ansible lookup dirs, addon dirs and .github/workflows keep their flat
layout (the filename is the public id / GitHub only discovers flat).
Unit tests mirror the new source layout.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Follow-up to the swarm CID/redis fix that make quality flagged:
- Extract the site-bootstrap sequence into 02b_create_site.yml (renamed from
02b_restart_frappe.yml; now new-site + restart + re-resolve + wait) and include
it with when: rc \!= 0, so the oversized when-block disappears and the sub-file
is not parsed at all when the site already exists.
- The backend entrypoint now execs the image's real /usr/local/bin/entrypoint.sh
after the config-ready gate, so the sites/assets symlink that entrypoint creates
is preserved instead of relying on a sibling replica to recreate it.
- Drop the now-dead ERPNEXT_BACKEND_CONTAINER_ADDRESS var, shorten two over-long
OIDC restart task names, and remove stale narration comments.
Also suppress the INFINITO_MAIN_REMOTE/BRANCH env lints in the git pre-push helper:
they are maintainer tooling knobs, not deployment keys.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The manager-op tasks ran 'container exec {{ ERPNEXT_BACKEND_CONTAINER_ADDRESS }}'
where the address resolves a LOCAL container id. Under swarm the backend task can
run on another node (and 'service update --force' reschedules it), so the id came
back empty, collapsing the command and failing 'apply API-bot' with a bogus 'No
such file'. Resolve the backend's hosting node + local id via resolve_host_cid,
delegate_to that node, and re-resolve after each forced restart. The four apply_*.py
tasks can no longer use a host '< file' redirect once delegated, so feed the script
through 'stdin:' instead; the OIDC on-change restart splits into compose vs swarm.
docker stack deploy strips depends_on conditions, so queue/websocket/scheduler
booted before the one-shot configurator wrote redis_* into common_site_config.json
and wedged on frappe's 127.0.0.1 defaults. Gate each worker (and the backend
entrypoint) to block until redis_socketio appears, closing the race in swarm and
staying a no-op in compose where the config already exists.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
On swarm the bare compose container names (erpnext-backend, ...) do not exist,
so `container restart` failed with "No such container". Split the restart into a
compose path (container restart) and a swarm path (service update --force
<entity>_<service>), extracted into 02b_restart_frappe.yml to keep the bootstrap
block within the size limit.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add tests/lint/repository/test_name_emoji_and_length.py: every Ansible task
name and GitHub Actions workflow/job/step name must start with an emoji (first
char not 0-9/A-Za-z) and be <= 80 chars.
- emit emoji names at the source: cli/build/include generator now writes
"🔧 setup <app>" / "🚿 flush handlers after <app>" so generated
tasks/groups/*-roles.yml satisfy the convention too
- convert existing task/step/job/workflow names to emoji-prefixed, choosing a
task-fitting icon per action (📦 install, ✅ ensure, 🔍 probe, 🥾 bootstrap,
🚀 deploy, 🧩 service-module load, ⬇️ checkout, 📂 include-routines, ...)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
bench new-site never passed --db-host, so Frappe defaulted to 127.0.0.1 (MySQLdb OperationalError 2002). The DB host is only written to common_site_config.json by the one-off configurator, which swarm never gates (docker stack deploy ignores depends_on; a restart:none one-off shows 0/1 forever, so the convergence wait can't gate on it either). Pass host/port explicitly; the vars already resolve to mariadb:3306.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
container_address(app, key) resolves services.<key>.name; a missing key or
empty name raises at task-arg finalization and killed the compose deploys for
erpnext (no 'backend' service) and friendica (service had no name).
- web-app-friendica: add name to the friendica service
- web-app-bigbluebutton: add names for postgres/greenlight (Compose default
<project>-<service>-<index>, upstream sets no container_name)
- web-app-erpnext: short-key all 7 sub-containers (configurator/backend/
websocket/scheduler/queue-short/queue-long/frontend) with prefixed
container_names, so the swarm resolver (<stack>_<key>) and compose exec
(<container_name>) both resolve; backend addressed via container_address('backend')
- tests/lint: new guard replays container_address._resolve_bare_name over the
static services config so a missing service/name is caught before CI deploys
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds two structural ansible lints under tests/lint/ansible/structure/:
- ansible-block-oversized: forbid `- block:` constructs with more than
3 direct child tasks. Big block bodies hide intent, accumulate
unrelated guards on the wrapper, and signal that the body wants to
be its own include_tasks sub-file. Extraction keeps the parent
scannable; the sub-file MAY carry the `# include-gated:
when: …` header marker so swarm-compat lints exempt it.
- ansible-repeated-when-run: forbid 3+ consecutive sibling tasks
carrying the SAME `when:` expression. Copy-pasted guards drift,
silently disable each other over time, and read worse than a single
`- name: …` + `when: <expr>` + `block:` wrapper. Per-line opt-out
for runs where wrapping would alter semantics (cross-task register
dependencies).
Both lints use the line-based scanner pattern from existing
tests/lint/ansible/jinja/* + `is_suppressed_at` from
utils.annotations.suppress for the per-line nocheck markers.
Refactors five roles into compliance:
- svc-prx-openresty/tasks/02_assert.yml: heavy rescue body extracted
to 02a_assert_rescue_debug.yml.
- test-e2e-playwright/tasks/01_setup.yml: 3 install-needed siblings
wrapped in a single when-block.
- web-app-erpnext/tasks/02_bench_bootstrap.yml: 3 site-probe-failed
siblings wrapped likewise.
- web-app-odoo/tasks/02_install_modules.yml: 3 OIDC-enabled siblings
wrapped. tasks/03_oidc.yml: dropped redundant single-task wrapper
block (the original wrapper carried name only and would have
required extraction for size — inlining the 23 tasks at file root
keeps the file purpose-narrow and removes one layer of indirection).
- web-app-xwiki/tasks/04_extensions.yml: large block body + rescue
body both extracted to 04a_extensions_rescue.yml +
04b_extensions_execute.yml.
Mixed-scope offenders (web-app-keycloak/tasks/{02_manager_ops,05a,
assert/05_ldap,update/05_ldap_shared,02a} and
web-app-matomo/tasks/{03_manager_ops,05_bootstrap,05a}) deliberately
stay unstaged; they ship with their respective swarm-aware
restructure commits where they belong by topic, not bundled with the
structural-lint cleanup.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds a mode-aware `container_address` lookup plugin and a
`resolve-container-id` helper script that resolve swarm task IDs at
exec time (compose mode returns bare names unchanged). Migrates ~25
roles + ~28 task files from `command:` to `shell:` so the lookup's
`$(...)` subshell evaluates correctly, and extends `database` lookup
with an `address` key for cross-role DB exec.
Three new lint tests enforce the contract: every container exec
target goes through the resolver, lives behind an IS_STACK_HOST gate
(walks the include graph one hop deep), and uses `shell:` rather than
`command:` when the target is a Jinja reference. Existing nocheck
suppressions on bare-name DB exec sites (nextcloud redis, listmonk,
moodle OIDC, mailu) are replaced with the lookup-based form.
Fixes the systemd Environment= injection in sys-ctl-cln-disc-space
(it never expanded $(...)) by passing STACK / SERVICE_KEY parts and
resolving inside the script. Extracts shopware admin and mediawiki
extension install bodies into dedicated .sh files invoked via env.
Rewrites password-bearing shell tasks to either `| quote` directly or
the `-e VAR=... | quote` + `sh -c '... ${VAR} ...'` env-bridge pattern.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
