3529 invalid comments in 528 files, grandfathered in while the linter
only saw the working-tree diff. Removal ran through the linter's own
collectors, so detection and deletion cannot drift: a comment whose
opener line is blank before it and whose closer line is blank after it
loses all its lines, anything else loses only the comment region, and a
line left holding a bare delimiter goes with it.
Five files keep their comments behind a nocheck marker instead. Four are
upstream configs whose inline docs are upstream's own (mautrix, systemd
journald, nextcloud-oidc-login, MediaWiki). In littlejs the
commented-out entries ARE the deactivation mechanism for an example
tile, so deleting them would delete configuration.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Consumer lookup plugins now reach the merged config through the Ansible
lookup loader instead of importing get_merged_applications /
get_merged_domains from utils.cache. The merge stays one cached
implementation surfaced via the applications/config/domains/users lookups;
consumers call lookup_loader.get(...).run() (the compose_networks idiom).
- get_merged_applications walks the whole meta/ tree recursively
(meta/**/*.yml to applications.<app>.<path>, addons and any depth) and
folds the raw meta/volumes.yml subtree onto applications.<app>.volumes
after the templar render so the Jinja source: strings stay unrendered.
- 31 consumer lookups rerouted; providers (applications/config/
applications_current_play/users/domains) and the raw-volume accessors
(volume/nginx) keep the cache import behind # nocheck: lookup-cache-import.
- New lint test_no_cache_imports_in_lookups enforces the contract.
- nfs_flavor is now the flavor SPOT (svc-storage-nfs-server + nfs_client_src
resolve via lookup('nfs_flavor')); removed the unused STORAGE_NFS_FLAVOR
group_var. Extracted extract_default_credentials to
utils/cache/schema_credentials.py to keep applications.py under the cap.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Strip every `req <NNN>`, `req-<NNN>`, `Requirement <NNN>`,
`requirement-<NNN>`, and `docs/requirements/<NNN>-...md` mention from
non-Markdown source files. Comments anchored to requirement numbers
rot when requirements are renumbered or rewritten and conflate
present-state behaviour (which the comment should describe) with the
audit trail of how the code got there (which belongs in commit
messages, PR descriptions, and the requirement file itself).
* Document the rule in `docs/contributing/documentation.md` under
"Comments". Code MUST describe the *why* in present-state language;
contract enforcement lives in tests, not folklore.
* Clean 254 source files across `roles/`, `tests/`, `plugins/`,
`cli/`, `scripts/`, `utils/`, `group_vars/`, and the repo-root
`eslint.config.js`. Pure comment / docstring / log-message edits;
no behavioural change.
* Add `tests/lint/repository/test_no_req_references_in_code.py` to
enforce the rule going forward across `.js`, `.py`, `.yml`,
`.yaml`, `.j2`, and `.sh` files. The legacy
`test_no_stale_requirement_refs.py` is exempt as the only file
whose purpose is to detect these very tokens.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds ``[tool.ruff.lint]`` with ``select = ["E", "F", "I", "B", "UP"]``
and ``target-version = "py311"``. ``E501`` is ignored because
``ruff format`` already reflows what it can; the remaining long lines
are unsplittable string literals / URLs.
Auto-fixed (2753 issues, behavior-preserving):
- UP006/UP035: ``List[X]`` → ``list[X]``, ``Dict[X]`` → ``dict[X]``,
``from typing import …`` redirected where superseded by builtins.
- UP045: ``Optional[X]`` → ``X | None``.
- UP010/UP009/UP015: dropped Python-2-era pragmas (``from __future__
import absolute_import, division, print_function``,
``# -*- coding: utf-8 -*-``), redundant ``open(..., "r")`` mode.
- UP004: dropped implicit ``class X(object):``.
- UP022/UP008/UP032: stdlib idiom modernization (``capture_output``,
super-call simplification, etc.).
- B905: ``zip(...)`` → ``zip(..., strict=False)`` (preserves prior
behavior; switching to ``strict=True`` is a separate semantic
choice not made here).
- I001: import-block ordering.
Hand-fixed (66 issues):
- B904 × 54: appended ``from <bound>`` (when the enclosing ``except``
binds a name) or ``from None`` (when it doesn't) so chained
exceptions stay distinguishable from errors-during-exception-
handling. Applied via a small AST-aware script that walked back to
the enclosing ``except`` clause and matched the binding.
- B017 × 4: narrowed broad ``assertRaises(Exception)`` to the actual
``AnsibleError`` / ``AnsibleFilterError`` raised by the code under
test.
- B023 × 2: bound captured loop variables via default args
(``def _record(name, *, _path_real=path_real, ...)``) so each
closure gets its own snapshot.
- B007 × 2: unused loop control variables renamed to ``_`` /
rewritten to ``.values()``.
- UP007 × 2: ``Union[X, Y]`` → ``X | Y`` (plus dropping the now-unused
``from typing import Union``).
- B018 × 1: Odoo addon manifests are bare dict expressions at module
level by design — suppressed with an inline ``# noqa: B018`` and
rationale comment.
- UP031 × 1: printf-style format → f-string.
The 65 ``# noqa: …`` markers introduced by the auto-fixer all carry
real ruff codes (``B018``, ``E402``, ``F401``, etc.), so
``test_noqa_only_ruff_codes`` stays green.
Verification
============
- ``ruff check .`` → All checks passed
- ``ruff format --check .`` → 894 files already formatted
- ``tests.lint.repository.{test_project_root_import,
test_no_direct_yaml_calls, test_noqa_only_ruff_codes}`` and
``tests.unit.utils.annotations.test_suppress`` → 20 tests OK
- 1621 unit tests → all pass
- Integration suite: 44 failures (pre-existing baseline, none from
this change), 1 skipped.
Going forward, the new rules block regressions on E (style), F
(pyflakes), I (import order), B (likely-bug patterns), and UP
(modernization to py311) for any new code.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Single atomic landing for three back-to-back role-meta refactor requirements
(see docs/requirements/008-role-meta-layout.md, 009-per-role-networks-and-ports.md,
010-role-meta-runafter-lifecycle-migration.md).
req-008 — role meta layout
* Move every role's metadata under meta/<topic>.yml with file-root
convention (no wrapping key matching the basename):
- schema/main.yml + config/main.yml.credentials → meta/schema.yml
- users/main.yml → meta/users.yml
- config/main.yml.server → meta/server.yml
- config/main.yml.rbac → meta/rbac.yml
- config/main.yml.compose.services + every other top-level key
of config/main.yml inlined under <entity>.<key>
→ meta/services.yml
- config/main.yml.compose.volumes → meta/volumes.yml
* Drop the legacy schema/, users/, config/ directories from every role.
* meta/schema.yml: support nested credential keys, optional `default:`
field (Jinja literal, not rendered/validated/regenerated), implicit
`algorithm: plain`.
* Materialised paths drop the `compose.` prefix:
compose.services.<e>.<…> → services.<e>.<…>
compose.volumes.<k> → volumes.<k>
<former-top-level>.<…> → services.<entity_name>.<former-top-level>.<…>
* Update every Python consumer (utils/cache, utils/manager, utils/applications,
plugins/lookup, plugins/filter, cli/*) plus every Jinja template, vars/*.yml
and task file to the new paths.
* No legacy fallback. Repository-wide grep for `compose.services.` /
`compose.volumes.` / `schema/main.yml` / `users/main.yml` / `config/main.yml`
is empty outside docs/requirements/ and tasks/utils/migrate_meta_layout.py.
* Add tests/lint/repository/test_role_meta_layout.py guarding against the
old paths returning, and an explicit lint guard that meta/variants.yml
files do not carry a `compose:` wrapper.
req-009 — per-role networks/ports
* Move group_vars/all/08_networks.yml.defaults_networks.local.<role>.{subnet,
dns_resolver} into roles/<role>/meta/server.yml.networks.local.{subnet,
dns_resolver}.
* Delete group_vars/all/09_ports.yml entirely; ports now live on the
service entity that exposes them at
roles/<role>/meta/services.yml.<entity>.ports.{inter,local,public}.
* Rename + int-cast every former top-level `services.<entity>.port`
field to `services.<entity>.ports.inter` (single int).
* Always-category-keyed maps for ports.local / ports.public; relay
spans use {start, end} integers.
* Add PORT_BANDS map to group_vars/all/08_networks.yml — single source
of truth for the per-<scope>.<category> band ranges, consumed by both
the new CLI helpers and the lint test.
* New CLI helpers (cli meta ports suggest, cli meta networks suggest)
walk the live role tree gap-first then increment-fallback, with
deterministic output and non-zero exit on capacity overflow.
* cli create role consumes both helpers when scaffolding a new role.
* Lint extensions: bare-int port shape rejection, host-bound port
collision check (single ints + relay span integers, inter excluded),
band-membership check with documented allow-list for the legacy
web-app-bigbluebutton.bigbluebutton.local.http: 48087 entry.
req-010 — run_after / lifecycle migration
* Move galaxy_info.run_after and galaxy_info.lifecycle out of
meta/main.yml into meta/services.yml.<primary_entity>.{run_after,
lifecycle}; <primary_entity> is the value returned by
get_entity_name(role_name).
* For multi-entity roles whose primary entity has no compose service
(web-app-bluesky → bluesky, web-app-matrix → matrix, desk/dev/drv/
gen roles), create a dedicated metadata-only top-level entry under
<primary_entity>.
* Introduce utils/roles/meta_lookup.py with get_role_run_after /
get_role_lifecycle that resolve <primary_entity> once and read from
meta/services.yml; degrade gracefully when meta/services.yml is
absent OR the field is absent; raise MetaServicesShapeError when the
file exists but is malformed.
* Update every consumer (utils/service_registry, utils/invokable,
cli/meta/applications/resolution/run_after/__main__, cli/meta/roles/
lifecycle_filter/__main__, cli/meta/applications/resolution/combined,
cli/build/{tree,graph,role_include}, cli/deploy/development/{common,
init,deps,deploy}, plugins/filter/canonical_domains_map, etc.) to
use the helper.
* Lint guards: no run_after/lifecycle in meta/main.yml; lifecycle
values restricted to {planned,pre-alpha,alpha,beta,stable,deprecated};
run_after/lifecycle never on a non-primary entity; empty run_after
lists rejected.
Also folded into this change set
* services_disabler now writes `services.<svc>.{enabled,shared}` into
host_vars instead of the old `compose.services.<svc>.{...}` path; the
discover/find pair walks meta/services.yml and only treats entries
carrying enabled/shared/provides as provider declarations (so
metadata-only entries from req-010 do not create spurious provider
rows).
* service_registry skips `run_after` deps whose target was filtered
out via SERVICES_DISABLED instead of failing the whole load.
* cli/mirror/resolver and cli/create/inventory/mirror_overrides emit/
consume the new applications.<role>.services.<svc>.image shape
(mirror.md updated to match).
* utils/domains/list.build_applications_from_roles now reads
meta/server.yml directly (it had been silently returning {} since
config/main.yml stopped existing).
* utils/roles/resource_validation.filter_roles_by_min_storage reads
meta/services.yml.<entity>.min_storage instead of the legacy
compose.services path.
* web-app-bluesky pre-clones the social-app build context BEFORE
sys-svc-compose so the build context is on disk by the time
sys-stk-front-proxy/tasks/main.yml flushes the docker build handler.
* 4 meta/variants.yml files (svc-ai-ollama, web-app-phpmyadmin,
web-app-wordpress, web-app-keycloak) lose their `compose: services:`
wrapper so deep-merge over meta/services.yml lands on the new
file-root shape.
Validation
* Unit tests: 1571 passed.
* Lint tests: 50 passed.
* Integration tests: 161 passed (1 skipped).
* Full-cycle fresh-purged-apps deploy succeeded for the 8-app validation
set: web-app-{nextcloud,wordpress,yourls,bigbluebutton,bluesky,gitea,
discourse,mattermost}.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The 934-line `utils/cache/data.py` monolith mixed cross-cutting helpers
(constants, yaml-loaders, deep-merge, signatures, render machinery)
with three independent domain caches (applications, users, domains).
Per the KISS/SPOT/DRY guidelines, split it along domain boundaries so
each file has a single, well-scoped responsibility:
utils/cache/__init__.py — package; owns _reset_cache_for_tests()
orchestrator
utils/cache/base.py — cross-cutting helpers: constants,
yaml-loaders, _deep_merge, cache-key +
content-fingerprint signatures, the
cross-domain re-entry guard, and the
templar-render machinery (the only
ansible-coupled symbol, lazy-imports
its dependency)
utils/cache/applications.py — variants + defaults +
get_merged_applications. Strictly
ansible-free at import time so the
GitHub Actions runner-host CLI path
(cli.deploy.development.init →
plan_dev_inventory_matrix →
get_variants) keeps working without
ansible installed
utils/cache/users.py — user defs, token store hydration,
alias materialization,
get_user_defaults, get_merged_users
utils/cache/domains.py — get_merged_domains, derived from the
merged applications view
utils/cache/data.py — DELETED. Façade is gone; the package
exports nothing the domain modules
don't already
Backward-compat: the facade was deleted (per operator request) and all
36+ consumers — 18 plugins, 2 CLI entry points, 9 test files, 4 doc
files, plus a role-internal lookup_plugin — were rewired to import
directly from the owning domain module. `_reset_cache_for_tests` is
now exposed at `from utils.cache import _reset_cache_for_tests`.
Tests added (per-module, focused):
tests/unit/utils/cache/test_base.py (31 cases)
tests/unit/utils/cache/test_applications.py ( 9 cases)
tests/unit/utils/cache/test_users_module.py (21 cases; suffix
disambiguates from
the existing
tests/unit/plugins/
lookup/test_users.py)
tests/unit/utils/cache/test_domains.py ( 6 cases)
The ansible-block invariants from CI runs 24934007615 and 24935979190
are now pinned via subprocess isolation: each ansible-less import test
spawns a fresh `python3 -c '…'` with a meta_path filter denying every
`ansible*` import and asserts the import + invocation succeed in that
clean process. The earlier in-process sys.modules-manipulation shims
fragmented module identity and broke under Python 3.13 inside the
infinito container; subprocess isolation has neither problem.
Doc-style audit (per docs/contributing/documentation.md):
- Removed em-dash from utils/cache/README.md (split into two
sentences)
- Updated stale `utils.runtime_data` reference in
docs/contributing/design/matrix-deploy.md
- Removed two SPOT self-declarations
(docs/agents/files/group_vars/applications.md and
docs/contributing/artefact/files/plugins/lookup/users.md) per
the documentation rule that SPOT status is conferred by
cross-links, not by the page asserting it about itself
Empirical proof:
- All four domain modules import without ansible (subprocess
isolation tests)
- get_variants(roles_dir=ROLES_DIR) returns 99 apps in the
ansible-blocked subprocess
- 142 cache tests + 1085 wider-suite tests pass; the only
remaining failures (test_host_vars free-function fixture bug,
test_runtime_performance timing flake) are pre-existing and
unrelated to this refactor (verified via `git stash`).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Both in-process cache layers — the cached YAML loader and the cached
runtime-data getters — now live next to each other under
`utils/cache/`. The package gains a README that documents the
"orthogonal layers, no third cache" rule so future contributors hit
the right primitive on the first look.
Moves:
- `utils/yaml_cache.py` -> `utils/cache/yaml.py`
- `utils/runtime_data.py` -> `utils/cache/data.py`
- `tests/unit/utils/test_yaml_cache.py` -> `tests/unit/utils/cache/test_yaml.py`
- `tests/unit/utils/test_runtime_data.py` -> `tests/unit/utils/cache/test_data.py`
- `tests/unit/utils/test_variants.py` -> `tests/unit/utils/cache/test_variants.py`
New:
- `utils/cache/__init__.py` (empty per project convention)
- `utils/cache/README.md` describing the two layers, when to reach for
which, lifetime, test-only helpers, and design guidelines (RFC 2119
keywords, emoji-after-headline, no self-SPOT declaration, no
slash-in-link-text).
- `tests/unit/utils/cache/__init__.py`
Mechanical updates:
- All imports across cli/, plugins/lookup/, tests/, and roles/ rewritten
from `utils.yaml_cache` -> `utils.cache.yaml` and
`utils.runtime_data` -> `utils.cache.data` (40+ files).
- `REPO_ROOT` parents-index in the moved tests bumped from `parents[3]`
to `parents[4]` to reflect the deeper nesting.
- Six broken markdown links in docs/ updated to the new module paths
(test_markdown_links lint test stays green).
- Two test files that used the `from utils import runtime_data` form
now use `from utils.cache import data as runtime_data` so the local
alias remains.
Tests: 848 passed across unit + lint suites (1 pre-existing collection
error in test_host_vars.py unrelated to this change).
kevinveenbirkenbachandClaude Opus 5
Alejandro Roman