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>
Eliminates the last 102 violations of test_project_root_import by
making PROJECT_ROOT in each package's __init__.py the single source
and inlining every wrapper helper.
Migrations
==========
- ~70 files: ``Path(__file__).resolve().parents[N]`` → ``from <pkg>
import PROJECT_ROOT`` (single-expression shape, scripted).
- ~12 files: two-statement helpers (``T = Path(__file__).resolve();
R = T.parents[N]``) and multi-line ``os.path.abspath(os.path.join(...
os.pardir...))`` chains rewritten by a follow-up scripted pass.
- ~20 files migrated by hand for shapes the scripts couldn't safely
rewrite (helper functions like ``_repo_root_from_test_file``,
embedded ``os.pardir`` chains, etc.).
Helpers removed (inlined to PROJECT_ROOT)
=========================================
- All ``def repo_root_from_here() / _project_root_from_here()`` and
same-shape variants in cli/ (``cli/deploy/development/{down,common,
build,deploy}.py``, ``cli/meta/categories/invokable/command.py``,
``cli/meta/applications/resolution/{combined/repo_paths,
dependencies/__main__,run_after/__main__}.py``).
- All zero-arg ``def <name>() -> Path: return PROJECT_ROOT`` helpers
(utils/invokable.py, utils/roles/resource_validation.py, cli/meta/
runtime/__init__.py, cli/meta/callorder/__main__.py, plus a dozen
test files).
- 4 ``PROJECT_ROOT = PROJECT_ROOT`` self-assignments removed from
test files where the class attribute became dead after migration.
Test-mock updates
=================
``patch.object(repo_paths, 'repo_root_from_here', return_value=ROOT)``
→ ``patch.object(repo_paths, 'PROJECT_ROOT', ROOT)`` (27 patches across
8 unit-test files); same shape for ``patch('utils.invokable._repo_root',
return_value=…)``.
Suppressions documented
=======================
5 files marked ``# nocheck: project-root-import`` for the catalog's
documented exemptions:
- 2 lints walking from a discovered glob match (<role>/templates/...)
up to its role dir, NOT the repo root.
- ``cli/validate/inventory/__main__.py`` invoked as standalone
subprocess (relative imports unavailable).
- ``plugins/filter/value_generator.py`` loaded by Ansible outside the
project package (relative imports broken).
- ``roles/test-e2e-playwright/filter_plugins/discover_playwright_roles.py``.
Bugs surfaced and fixed in the same commit
==========================================
The migration uncovered two latent role-content violations that were
silently masked by buggy ``_repo_root()`` helpers in two integration
tests (the helpers walked upward looking for marker dirs and returned
``tests/integration/`` instead of the repo root, so the tests scanned
nothing under ``roles/`` for years):
- roles/web-app-minio: extracted 6-task OIDC block into 02_oidc.yml +
``include_tasks`` (the documented "Good" pattern from
test_block_size; parsing cost is paid only when OIDC is enabled).
- roles/web-app-bluesky/tasks/01_core.yml: collapsed 3 sequential
``when: BLUESKY_WEB_ENABLED | bool`` tasks into one ``block:`` with
the guard at block level (4 → 2 occurrences, under threshold).
Two semantic-bug fixes carried alongside
========================================
- cli/core/app.py: ``cli_dir`` was previously ``parents[1]`` (= cli/),
not the repo root; rewritten to ``PROJECT_ROOT / "cli"`` to match
the variable's intent.
- cli/make/__main__.py: pre-existing bug — ``repo_root`` was actually
the ``cli/`` directory (so ``cli/Makefile`` was the search target,
which never existed). Now correctly uses PROJECT_ROOT.
- scripts/lint/sort_claude_settings.sh: invokes utils/claude/
settings_sort.py as a module (``python -m``) so its
``from . import PROJECT_ROOT`` resolves.
Verification
============
- ``ruff check .`` → clean
- ``tests.lint.repository.test_project_root_import`` → OK (102 → 0)
- 1621 unit tests → all pass
- Integration suite: 44 failures (baseline, pre-existing, none from
this commit), 1 skipped
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The literal `# noqa` substring inside the descriptive comment, even
wrapped in backticks, was caught by ruff's noqa parser. Rephrased to
"via `nocheck`-keyword suppression markers" so ruff no longer matches.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reason: ruff parses every `# noqa: <code>` marker as a flake8 directive
and warns on every non-flake8 rule key. Every catalog rule in
`docs/contributing/actions/testing/suppression.md` is project-specific
(kebab-case), so `# noqa:` markers fired the warning on every parse.
`# nocheck:` is the project synonym ruff ignores.
Scope:
- 41 `roles/*/meta/services.yml` + 1 task + 1 .cfg.j2 template: pure
noqa→nocheck swap for catalog rules. Mixed flake8 markers
(`# noqa: E402, X`) preserved verbatim.
- 5 production .py + 2 production scripts: same swap.
- All catalog-rule lints' docstrings updated; redundant "or # nocheck:"
parentheticals collapsed to a single nocheck form.
- `utils/annotations/suppress.py`: docstring rewritten to spell out
the new convention. Parser still accepts both keywords as synonyms.
- `docs/contributing/actions/testing/suppression.md`: convention
rewritten to mandate `nocheck:`; missing `domain-spot` and
`literal-protocol-lookup` rows added; "enforced by" pointer added.
New guard:
- `tests/lint/repository/test_noqa_only_ruff_codes.py`: meta-lint
failing any `# noqa: <code>` whose codes don't match `[A-Z]+\d+`
(real ruff/flake8 shape). Allowlists the impl, its unit test, the
catalog page, the direct-yaml lint, and itself.
- `tests/lint/repository/test_no_direct_yaml_calls.py`: hardened to
reject the `noqa:` form via inlined regex (no longer routed through
`utils.annotations.suppress`).
Tangential cleanups carried in this same commit:
- `utils/cache/yaml.py`, `utils/handler/yaml.py`: 6 `direct-yaml`
markers swapped.
- `utils/cache/base.py`: `from . import PROJECT_ROOT, ROLES_DIR` moved
up to the import block (E402).
- `cli/create/role/__init__.py`: reordered so PROJECT_ROOT is defined
before `__main__` is imported (also fixed a latent circular import).
- PROJECT_ROOT migration tail: 2 new `__init__.py` modules + 10 test
files swapped from `def repo_root()` to `from . import PROJECT_ROOT`.
- ~20 ruff auto-fix removals of unused `from pathlib import Path`.
Verification:
- `ruff check .` clean.
- 23 suppression-related tests pass.
- The new lint detects all three regression shapes (project rule alone,
mixed with real ruff code, lowercase-only).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drop the second name for the same constant. Every consumer now uses
`PROJECT_ROOT` exclusively, matching the canonical
`utils.cache.__init__.PROJECT_ROOT` source.
The rename is a word-boundary substitution applied across every
Python file in the repo, validated by `ast.parse` after each edit.
Three files additionally drop a redundant
`PROJECT_ROOT = PROJECT_ROOT` class attribute (it pointed the class-
level name at the module-level name) and inline the
`self.PROJECT_ROOT` reads to plain `PROJECT_ROOT`.
The pass also closes a latent wave-1 inconsistency: a handful of
files received the `from . import PROJECT_ROOT` shim earlier but
their consumer expressions still spelled `REPO_ROOT / "scripts" / …`,
so the shim was unused. The rename ties shim and consumer together.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The `parents[N]` / `os.pardir` / `".."` / `def repo_root` regexes
need to fire on real Python code, never on documentation that
mentions the patterns by name. The `_FORBIDDEN_PATTERNS` definition
inside this file, the module docstring of `utils/cache/__init__.py`,
and several catalog notes elsewhere all spell out the patterns
verbatim and otherwise self-flag.
Add `_strip_strings_and_comments(text)` that walks the source once
and replaces every `"""…"""` docstring, `"…"` / `'…'` string
literal, and `#` comment with whitespace that preserves line
numbers and column offsets. The scanner now matches against the
stripped variant while the suppression check (`is_suppressed_at`)
still consults the raw lines, so a `# nocheck` marker keeps working
exactly as before.
Effect: 64 false positives go away; the remaining 114 violations
across 108 files are all real code that needs migration to the
package-init `PROJECT_ROOT` rule.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Migrate the simple `PROJECT_ROOT = Path(__file__).resolve().parents[N]`
shape across the repo to the new "import from `__init__.py`" rule
introduced by the `project-root-import` lint:
- `utils/cache/__init__.py` is the canonical source. `PROJECT_ROOT`
and `ROLES_DIR` move out of `utils/cache/base.py`; `base.py`
re-exports them for callers that still import from there.
Internal cache modules (`files.py`, `gitignore.py`) switch to
`from . import PROJECT_ROOT`.
- 24 packages receive a `PROJECT_ROOT` declaration in their own
`__init__.py`, each with the parents[N] depth that resolves to
the repo root.
- ~36 consumer files swap their local
`PROJECT_ROOT = Path(__file__).resolve().parents[N]` for
`from . import PROJECT_ROOT`. `ROLES_DIR` and similar derived
paths stay local (`PROJECT_ROOT / "roles"`).
Status: this commit closes the simple-shape violations. The
multi-line `os.path.abspath(os.path.join(here, os.pardir, …))`
shape, `def repo_root()` while-loops, and `roles/<role>/filter_plugins`
files without a package container remain. The lint stays scharf and
will track each subsequent wave.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pin `PROJECT_ROOT` to a single canonical source so a future move of
any consumer file does not silently break a hard-coded
`Path(__file__).parents[N]` index.
The lint walks every `*.py` file in the repo and flags four shapes
that compute the repo root locally:
- a top-level `parents[N]` index expression on `__file__`,
- any `os.pardir` reference,
- any `".."` path segment in a path-construction context,
- a `def repo_root()` / `def project_root()` function that searches
upward for `pyproject.toml`.
An `__init__.py` MAY define
`PROJECT_ROOT = Path(__file__).resolve().parents[N]`. The lint
cross-checks that the resolved path actually carries
`pyproject.toml` so a wrong index fails loudly.
The companion suppression rule `project-root-import` covers two
unavoidable cases: a `__main__.py` `sys.path` bootstrap shim that
runs before any package import resolves, and a standalone script
under `roles/<role>/files/` that has no package container to import
from. The catalog entry and example block are added to
`docs/contributing/actions/testing/suppression.md`. The same pass
removes em dashes from earlier catalog entries and pulls every
mid-sentence line break onto a single line, in line with the
`documentation.md` rules updated in the prior commit.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
A new rule under "Semantics and Writing" requires every sentence in a
Markdown file to live on a single logical line. Reflowable Markdown
viewers otherwise introduce stray line breaks at the source-line
boundary, and search / grep results that show only one matched line
lose half the sentence's context. A new line in the source MUST
correspond to a new sentence, a new paragraph, or a structural
element (heading, list item, table row, fenced block).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two intertwined changes that share the same touched files:
1. Move every meta-related integration test under
`tests/integration/roles/meta/` into one of three sibling
sub-clusters and shorten the redundant filename prefixes:
- `services/` — `test_services_dynamic_flags.py` →
`test_dynamic_flags.py`, `test_services_no_self_reference.py` →
`test_no_self_reference.py`. The `run_after` cluster moves
under it (it is a `meta/services.yml` concern, not a
stand-alone dependency one): `test_services_explicit.py` keeps
its name, the rest drop the redundant prefix.
- `variants/` — `test_variants_coverage.py` → `test_coverage.py`,
plus the two new lints `test_auth_coverage.py` (LDAP / OIDC /
OAuth2 matrix completeness) and `test_services_match.py`
(every variant override key MUST exist in `services.yml`).
- `dependencies/` — relocates the legacy
`tests/integration/roles/dependencies/` cluster so all
`meta/`-shaped tests live under one parent.
Path-index constants (`Path(__file__).resolve().parents[N]`,
`os.pardir` chains) updated for the deeper nesting.
2. Route every yaml read in the migrated tests through
`utils.cache.yaml`. The single-doc helper `load_yaml_any` covers
most of them; the new `load_yaml_all` mirrors `load_yaml_any` for
the multi-doc case (`yaml.safe_load_all`) and is keyed off the
same `(path, mtime, size)` signature, with parallel `_drop_path`
/ `_reset` housekeeping. `tests/lint/ansible/variables/
test_vars_usage_in_yaml.py` is the first user.
READMEs cleaned up to match the new directory structure (path
crosslinks, level-up counts) and four new READMEs added for the
new sub-clusters under the `docs/` README guidelines (RFC 2119,
emojis after headlines, no em dashes, no SPOT self-labels).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous two-variant layout failed `test_variants_coverage`: with
`ldap.enabled: true` pinned in BOTH variants there was no false-pin
for the dynamic `services.ldap.enabled` flag.
Restructure into three variants so the matrix exercises every branch
the role actually supports AND every dynamic flag gets the
true/false coverage the lint requires:
- variant 0 — all-on: every dynamic flag pinned `true` /
`shared: true`. Exercises the in-`group_names` branch end-to-end.
- variant 1 — LDAP-only (req 018): `oidc.enabled: false` with
`ldap.enabled: true`, `ldap.shared: true`. The LDAP backend stays
available via `svc-db-openldap`, so the role can hit its LDAP-bind
login path in isolation from OIDC. The corresponding spec
scenario is still missing — tracked in
docs/requirements/018-playwright-ldap-coverage.md.
- variant 2 — all-off: every dynamic flag pinned `false`. Exercises
the out-of-`group_names` branch and gives every dynamic flag its
required false-pin.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Resolve every remaining mismatch flagged by the new
test_role_and_group_vars_used lint:
- group_vars/all/02_tls.yml — mark TLS_SELFSIGNED_SCOPE as
`nocheck: unused-var`; the self-signed cert generator consumes it
outside the YAML/Jinja scope.
- group_vars/all/08_networks.yml — mark PORT_BANDS as
`nocheck: unused-var`; the port-allocation plugin reads it via
Python, not YAML.
- roles/sys-stk-front-proxy/vars/main.yml — delete the role-local
one-liner whose only var (`front_proxy_domain_conf_dst`) was never
consumed.
- roles/test-e2e-playwright/defaults/main.yml — drop
TEST_E2E_PLAYWRIGHT_IMAGE_DISTRO; the image-derivation comment
that referenced it is gone too.
- roles/web-app-opentalk/templates/playwright.env.j2 — switch
OPENTALK_OIDC_ENABLED to render the inventory-resolved var
directly instead of duplicating the lookup.
- roles/web-app-discourse/tasks/04_network.yml — trailing-newline
fix (incidental autoformat).
After this commit the lint passes cleanly across the role tree and
the two `group_vars` markers stand as the single canonical place a
contributor finds when researching "why is X exempt".
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wire the existing `noqa` / `nocheck` grammar into the role/group var
usage lint via `utils.annotations.suppress.is_suppressed_at`. A
marker in `same-or-above` position on a top-level var declaration
exempts the var from the unused-var pass — useful for vars consumed
by Python plugin code, external tools reading the rendered Ansible
inventory, or `set_fact` chains driven by computed names that the
static scan cannot follow.
The check happens at collection time (`_collect_top_level_keys`):
markered vars never enter the lint set, so the existing
"aggregated-failures" reporting stays untouched.
Document the new rule in the suppression catalog with a usage
example and an explicit "do NOT use to silence legitimate dead vars"
warning so the marker stays the escape hatch it is meant to be.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Move the existing `test_vars_usage_in_yaml.py` (task-level `vars:`
block scanner) and the new `test_role_and_group_vars_used.py`
(role/group var top-level scanner) into a dedicated
`tests/lint/ansible/variables/` subpackage. The two checks address
orthogonal scopes — task-local `vars:` blocks vs. role-local
`vars/main.yml` / `defaults/main.yml` and `group_vars/**/*.yml` — so
co-locating them documents the variable-lifecycle coverage in one
spot and leaves room for follow-up scanners (host_vars, set_fact,
register, …) without cluttering the parent directory.
The new lint:
- Extracts top-level mapping keys from every
`roles/<role>/{vars,defaults}/main.yml` and
`group_vars/**/*.{yml,yaml}` via the cached `load_yaml_any`,
filtered to identifiers (`[A-Za-z_][A-Za-z0-9_]*`).
- Single project-tree walk over `.yml` / `.yaml` / `.j2` builds two
identifier sets: tokens inside Jinja `{{ … }}` / `{% … %}` blocks,
and identifiers in Ansible expression-bearing keys (`when:`,
`loop:`, `with_*:`). Tokens immediately followed by `(` are
excluded so function/macro calls do not count as variable uses.
- Routes every read through the project caches: `iter_project_files`
(`lru_cache(maxsize=1)`), `read_text` (`lru_cache(maxsize=8192)`),
and `load_yaml_any` (signature-keyed cache). No `yaml.safe_load*`
bypass.
- Whitelists `ansible_python_interpreter` and `ansible_shell_executable`
(Ansible connection-behavior keys consumed by the runner itself).
- Aggregates every undecidable variable into one
`<file>:<line>: '<name>' is never referenced` line and fails once
with the full list, so the reviewer sees the whole drift surface
in a single run.
Surfaces seven existing dead declarations on first run (across
`sys-stk-front-proxy`, `test-e2e-playwright`, `web-app-discourse`,
`web-app-opentalk`, `group_vars/all/`); pruning them is left for
follow-up commits so this one stays purely the lint introduction.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`plugins/lookup/config.py` flips to `strict=False` when a 4th
positional `terms` element is present, so a missing path silently
falls through to the caller-supplied default. Such calls cannot fail
at runtime no matter what the static scan says about the path, so
treating them as lint targets is a false positive — the path is
intentionally optional.
Add a `_has_default_arg(text, scan_from)` helper that does a
character-level look-ahead from the end of the path literal to the
next significant character inside the same `lookup(...)` call. A `,`
indicates a 4th positional argument exists; a `)` (the call's closing
paren) indicates none does. The check is value-type agnostic — it
never inspects the default itself, so any literal (`False`, `''`,
`[]`, `{1: 2}`), variable reference, or sub-expression behind the
comma works.
`_emit_literal_match` invokes the helper after the comment- and
suppression-filters; default-bearing calls are dropped from the match
stream and therefore from all four downstream classifiers
(literal / variable / wildcard / role-local) in lockstep.
`_emit_concat_match` needs no change: `expr_to_wildcard_path` already
returns `None` whenever an unquoted comma appears in the expression
body, which is exactly the shape a concat-pattern call with a default
takes.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drop env declarations that were rendered but never consumed by the
matching playwright.spec.js, surfaced by
tests/lint/ansible/roles/test_playwright_env_keys_used.py:
- web-app-friendica: OIDC_ISSUER_URL
- web-app-mailu: MAILU_DOMAIN
- web-app-opentalk: OPENTALK_OPENCLOUD_URL
- web-svc-xmpp: OIDC_ISSUER_URL, ADMIN_USERNAME, ADMIN_PASSWORD
The keys will land again only when an actual scenario consumes them.
For the deeper coverage gap that some of these env files imply (xmpp
in particular has no auth scenarios despite declaring admin
creds), see requirements 017 (biber RBAC coverage) and 018 (LDAP
authentication coverage).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Migrate `web-app-opencloud/meta/services.yml` to the dynamic-flag form
for `ldap.enabled` ("{{ 'svc-db-openldap' in group_names }}") so the
flag follows the same convention as the other shared-service entries
and the matrix-deploy variants stay the single point of truth for
which branch the spec exercises. Variants 0 and 1 pin
`ldap.enabled: true` explicitly so the LDAP path stays on for both
of them; that is what the matrix is meant to cover.
Drop two stale env keys from `templates/playwright.env.j2`:
`OPENCLOUD_LDAP_ENABLED` and `OPENCLOUD_OPENTALK_URL` were declared
but never read by the spec — caught by the
`test_playwright_env_keys_used.py` lint. They will land again only
when an actual scenario consumes them.
Add requirement 018 (Playwright LDAP authentication coverage):
the policy is that every role with `ldap.enabled` and a Playwright-
drivable login surface MUST ship a separate LDAP scenario, gated via
`skipUnlessServiceEnabled('ldap')`. Today most specs only gate on
`oidc`, so a LDAP-only deploy (variant 1 here) skips every
authenticated test instead of validating LDAP. The requirement lists
the in-scope roles and the verification criteria.
Link the OpenCloud-specific follow-up from the role TODO so the gap
is visible from the role artefact tree, not only from the requirement
index.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The lookup-config-key lint surfaced that web-app-joomla declares
BIBER_USERNAME / BIBER_PASSWORD / CANONICAL_DOMAIN in its
playwright.env.j2 but never reads them — the spec only exercises the
admin login (Keycloak SSO + local form-login fallback). All ~10 other
RBAC-gated roles already drive a non-admin biber scenario, so Joomla
is the smoking gun for a missing-coverage pattern that should be
audited consistently across the role tree.
- Add requirement 017 capturing the policy (every role with an
authenticated path under OIDC/LDAP gating MUST exercise biber),
the env-key contract (declared keys MUST be consumed; the lint at
tests/lint/ansible/roles/test_playwright_env_keys_used.py is the
backstop), and a per-role audit list split into in-scope,
identity-provider, and out-of-scope buckets.
- Document the gap in roles/web-app-joomla/README.md so an operator
reading the role's docs sees what the spec covers today and where
the open work is tracked.
- Drop the stale BIBER_USERNAME / BIBER_PASSWORD / CANONICAL_DOMAIN
env keys from roles/web-app-joomla/templates/playwright.env.j2.
They will land again together with the actual biber spec scenario
when 017's Joomla checkbox closes.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
New per-role lint that enforces two consistency rules between
`roles/<role>/templates/playwright.env.j2` and
`roles/<role>/files/playwright.spec.js`:
- A role that ships an env file MUST also ship a sibling spec file —
otherwise the rendered variables go nowhere.
- Every `KEY=` declared in the env file MUST be consumed via
`process.env.KEY` (or the bracket form) in some `.js` under the
role's `files/` tree or under
`roles/test-e2e-playwright/files/` (where shared helpers like
`service-gating.js` live).
Both failure modes are aggregated into a single `self.fail` listing
all violations, so reviewers get the full drift picture in one shot.
Whitelist for keys that are legitimately not matched by a literal
`process.env.<KEY>` regex:
- `*_SERVICE_ENABLED` — `service-gating.js` reads them dynamically
via `Object.keys(process.env)` plus a suffix strip.
- `NODE_TLS_REJECT_UNAUTHORIZED` — read by the Node runtime itself,
not by user code.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Five web-app roles read their own `credentials.database_password` via
`lookup('config', application_id, …)` from `vars/main.yml`. The
credential is wired in by the inventory's apply_schema step rather
than declared in each role's `meta/schema.yml`, so the static
lookup-config check cannot see it. Add the `lookup-config-path`
marker to each call so the role-local pass stays clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The role-local pass used to fire for every variable app argument,
which surfaced false positives for calls that explicitly target a
different role at runtime (`_BBB_COTURN_ROLE`,
`oauth2_proxy_application_id`, `NEXTCLOUD_HPB_TURN_STANDALONE_ROLE`).
Restrict the classifier to `app_arg == "application_id"` so only the
"this role reads its own config" shape is enforced; cross-role
lookups land in test_variable_paths' lax check, where they belong.
Document the new scoping in the `lookup-config-path` catalog entry so
contributors know cross-role calls need no marker for the role-local
pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Make the lookup-config test package SRP-compliant. The scanner used to
own all four classifications (literal / variable / wildcard /
role-local), so changing one rule risked leaking into another.
- `_scan.py` now exposes only the qualifying match stream
(`iter_matches() -> tuple[LookupMatch, ...]`) and the repo-derived
`ScanContext`. It knows the regex and the comment / suppression
filter; it does not know which test consumes which subset.
- Each `test_*.py` carries a private `_build_<name>_paths(matches, …)`
builder that filters and groups the stream into its own collection.
A classification rule change now stays inside one file.
- Performance unchanged: `iter_matches` keeps the
`lru_cache(maxsize=1)` so all four classes share a single project
walk; the per-test builders are O(N) in-memory iterations over the
cached tuple.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Wire the existing `noqa` / `nocheck` grammar into the lookup-config
scanner via `utils.annotations.suppress.is_suppressed_at`. A
marker in `same-or-above` position skips the call from every
downstream check (literal / variable / wildcard / role-local) so
the rule can never drift between the four test classes.
- Document `lookup-config-path` in the suppression catalog and add
examples for both the `# nocheck:` (YAML / shell) and `{# nocheck: #}`
(Jinja) comment forms.
- Apply the marker to the three legitimate self-referential calls in
`roles/web-app-oauth2-proxy/templates/oauth2-proxy-keycloak.cfg.j2`,
where `oauth2_proxy_application_id` resolves at runtime to the
gated app — not to `web-app-oauth2-proxy` itself, which is what
the static role-context inference would otherwise assume.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Add `test_role_local_literal_paths` to enforce that
`lookup('config', application_id, '<path>')` calls inside
`roles/<role>/...` resolve against `application_defaults[<role>]`
instead of just "any role somewhere", which `test_variable_paths`
was the only check for.
- Split the monolithic `test_config_paths.py` into one TestCase per
concern under `tests/integration/lookups/config/`:
literal / variable / wildcard / role-local. Shared scan and
validation helpers live in `_scan.py` / `_validate.py`.
- Cache the project-wide scan via `functools.lru_cache(maxsize=1)`
on `_scan.get_scan` so all four classes share a single pass; route
every YAML read through `utils.cache.yaml.load_yaml_any` and
replace the per-app O(n) `plugins.filter.get_role.get_role` calls
with one `_build_role_for_app_map` walk (was O(n²) over
`vars/main.yml`).
- Pre-filter the file walk by extension (`.yml`/`.yaml`/`.j2`) and a
`"lookup" in text` substring check before the regex passes.
- Switch all four checks to a collected-failures pattern: each test
reports a single aggregated failure listing every mismatch instead
of emitting one subTest failure per occurrence.
- Allow `_*.py` filenames under `tests/` in the file-naming linter so
the new shared helpers (`_scan.py`, `_validate.py`) pass discovery
without being mistaken for test cases.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The pre-005 flat-layout purge has run on every deployed instance;
new deploys never had pre-005 entries to begin with. The task is now
permanently no-op and only adds startup latency + a transient task
in the play recap.
Removed:
- tasks/01_core.yml include block (+ header comment)
- tasks/06_migrate_005.yml (purge + Keycloak logout-all)
- files/migrate_005_purge_stale.sh
- docs/administration/configuration/rbac.md migration section
(described the now-removed migration)
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
New repository lint catches `Path.read_text(...)` and `Path.read_bytes(...)`
calls inside `tests/lint/` test code, which bypass the LRU-cached
`utils.cache.files.read_text` and re-read the same project files for
every test that scans them. Routing through the cache shares one decoded
read per file across the whole pytest session.
Per-line opt-out: `# noqa: cache-read` for genuinely out-of-cache reads
(tempdirs, synthetic fixtures, files outside the project tree).
Documented in docs/contributing/actions/testing/suppression.md alongside
the existing `project-walk` marker.
All 16 pre-existing violations in 9 lint tests retrofitted onto the
cached helper — no opt-out markers used; every site was a legitimate
project-tree read. UnicodeDecodeError added to existing OSError except
clauses where applicable (the cache helper is UTF-8 strict, replacing
the lenient `errors="replace"` of the raw call).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
New repository lint catches multi-line `php -r '...'` and `php <<EOF`
bodies inside *.sh / *.bash files under roles/, scripts/, .github/.
The escape-layering of bash + (template/YAML) + PHP makes such blocks
brittle and quote-injection-prone. The fix is to drop the PHP body
into a sibling files/<name>.php and pipe it via stdin (preferred from
Ansible: `shell:` with `stdin: "{{ lookup('file', '<name>.php') }}"`).
Three pre-existing violations transformed by replacing the .sh wrapper
with one or more .php sidecars and rewriting the caller task:
- web-app-espocrm: set_siteurl.sh -> set_siteurl.php
Single shell task pipes the .php via stdin; ESPOCRM_URL forwarded
through `compose exec -e` so the URL never lands in the PHP literal.
- web-app-snipe-it: ldap.sh (wrapper + 2 PHP blocks) -> 3 ansible
tasks + wait_settings.php + apply_ldap_config.php
The bash retry-loop becomes Ansible retries/delay/until; LDAP env
vars are forwarded via `compose exec -e` (not via task-level
environment: on a wrapper script). APP_KEY is preserved for
Laravel Crypt::encrypt.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Inline-script-size lint regex previously matched only bare keys
(shell:/command:/script:/raw:/run:). Extended to also catch FQCN
forms (ansible.builtin.*, ansible.legacy.*, ansible.windows.win_shell)
which are interchangeable in Ansible and must be linted identically.
Four pre-existing violations exposed and fixed by extracting the
shell bodies to files/ and calling them via ansible.builtin.script:
- sys-cleanup/main.yml (21 logical lines)
-> files/cleanup_caches.sh
- svc-db-openldap/06_migrate_005.yml (30 logical lines)
-> files/migrate_005_purge_stale.sh (4 args, no_log preserved)
- dev-nix/02_non_arch_installer.yml (24 logical lines)
-> files/aggressive_prune_nix.sh (1 arg)
- web-app-espocrm/01_patch_config.yml (13 logical lines)
-> files/set_siteurl.sh (URL via env, fixes quote injection
risk in original Jinja-into-PHP-single-quoted body)
Removed cargo-culted args.executable: /bin/bash from the four
script: tasks; the script: module ignores it (shebang controls
interpreter).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The schema-privileges block in svc-db-postgres/tasks/02_init.yml mixed
GRANT/ALTER DEFAULT PRIVILEGES SQL with Jinja interpolation in a 4-line
inline `query: |` scalar. Move the statements into
`roles/svc-db-postgres/templates/sql/grant_schema_privileges.sql.j2` and
load via `lookup('template', ...)` so the SQL stays reviewable as plain
SQL (Jinja stays only for identifier interpolation, since Postgres
prepared-statement bindings cannot substitute identifiers).
Also adds tests/lint/ansible/test_no_inline_multiline_sql.py that fails
when more than 3 SQL-leader lines (INSERT/UPDATE/SELECT/GRANT/ALTER/…)
appear in a single inline YAML scalar under roles/, tasks/, or
playbooks/. The error message points at the extraction-via-lookup
pattern so future blocks land in files/<name>.sql or
templates/sql/<name>.sql.j2 by default.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Two issues bundled because the changes overlap on the same role:
1. SQL extraction
The admin-user bootstrap and the OIDC-provider UPSERT lived inside
bash heredocs in `files/*.sh`, which Ansible invoked via the `script:`
module. The colon-prefix in `:'var'` only routes through psql's lexer
when SQL comes from stdin or `-f`, so passing it through `-c "..."` —
the original form before this branch — failed with `syntax error at
or near ":"`. Extract the SQL into `files/create_admin_user.sql` and
`files/configure_oidc_provider.sql`, embed via `lookup('file', ...)`
into an `ansible.builtin.shell` task, and feed psql via `stdin:`.
Drops the bash wrappers entirely.
2. EMAIL_* env always rendered
Fider's env loader `panic`s at startup with `the environment variable
"EMAIL_SMTP_HOST" is missing` (and friends) regardless of whether the
relay is reachable. Drop the `services.email.enabled` gate around the
SMTP block so the container starts on email-disabled stacks. The
`lookup('email', ...)` helper returns sane infinito.nexus defaults
when the service flag is off.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The admin SSO test was timing out at `getByRole('banner', { name: /top bar/i })`
because the assertion raced the multi-hop OIDC callback chain
(auth → /oidc-callback → /web-oidc-callback → SPA), so the Files-view
banner had not yet mounted by the 60s timeout. biber's variant happened
to win the race. Add a `page.waitForURL(baseUrlPattern)` after the
password Enter so the assertions only run once the SPA is back on its
own origin, and bump the banner timeout from 60s → 90s for headroom.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Plugin marketplace upstream lags Nextcloud version: deploy fails with
`App "BPM Files" cannot be installed because it is not compatible with
this version of the server.` Comment out the entry rather than flipping
`enabled: false` so re-enabling later is a single-line restore once the
plugin ships a release matching the pinned Nextcloud major.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Variant 1 inherited the empty default credential, so the matrix-deploy
round 2 couldn't authenticate as the administrator. Carry the same
``administrator_password`` literal as variant 0 so both rounds share
the same credentials and the Playwright admin scenarios stay
deterministic.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
InventoryManager auto-generated the cookie_secret on a strict
``oauth2.get('enabled') is True`` check, which silently stopped
firing after the dynamic-flag migration replaced the literal True
with ``"{{ '<role>' in group_names }}"`` Jinja strings. Result:
every oauth2 / oidc consumer (web-app-prometheus, akaunting,
funkwhale, etc.) deployed without a cookie secret, and the
oauth2-proxy template hard-failed with
``Key 'oauth2_proxy_cookie_secret' not found``.
Replace the strict identity check with the same ``is_explicit_truth``
helper already used by ``resolve_service_dependency_roles_from_config``
and ``ServicesResolver``. Two unit tests cover the new behaviour:
the dynamic-Jinja form seeds the secret, and an explicit
``enabled: false`` skips it.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The lookup expected ports.local.database, but svc-db-mariadb /
svc-db-postgres declare per-engine port-keys (mariadb:3306,
postgres:5432) to match the per-engine PORT_BANDS introduced in
1e94b14dd. Result: the resolved database_port came back empty, and
sys-svc-rdbms's seed-database-to-backup assertion blocked every
deploy that wired up a shared database.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The cache compose profile (registry-cache + package-cache) writes to
/var/cache/infinito/core/cache/ — `make cache-clean` and the
container runtime need write access there. Without this entry the
sandbox blocked the agent from cleaning the host-side caches when a
build container's apt layer got stale.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two new integration lints under tests/integration/roles/meta/:
* test_variants_coverage.py — every services.yml flag declared via
the dynamic ``"{{ '<role>' in group_names }}"`` form MUST be
exercised by meta/variants.yml: at least one variant entry pins
it to literal ``true`` AND at least one pins it to ``false``.
Pairs may share variant entries — the test only checks the union
of all variants covers both polarities for every dynamic flag.
* test_services_no_self_reference.py — a role's own primary entity
(matching ``get_entity_name(role)``) MUST NOT use the dynamic form
for ``enabled`` / ``shared``. Both MUST be literal ``true``: the
role's own services.yml is loaded only when the role itself is in
group_names, so a ``'<self>' in group_names`` Jinja is tautological.
The dynamic-flag test now skips the role's own entity to avoid a
contradiction with this new guard.
Self-reference fixes (Jinja → literal ``true: true``) on 19 roles:
svc-ai-ollama, svc-db-{mariadb, openldap, postgres}, web-app-{
dashboard, discourse, keycloak, mailu, matomo, prometheus},
web-svc-{asset, cdn, collabora, coturn, file, libretranslate, logout,
onlyoffice, simpleicons}.
Variants coverage rollout (62 new + 17 merged meta/variants.yml):
* Roles without a pre-existing variants.yml get a new file with two
entries — one pinning every dynamic flag true, one pinning every
dynamic flag false. No ``- {}`` baseline (matrix runs only the
two coverage variants).
* Roles with a pre-existing variants.yml (akaunting/baserow/bluesky/
bookwyrm/flowise/fusiondirectory/jenkins/jira/joomla/keycloak/lam/
minio/phpmyadmin/postmarks/wordpress/web-svc-xmpp) keep their
matrix variants (OIDC vs LDAP, Single-Site vs Multisite, dev creds
+ TOTP-disabled realm, autologin, etc.); the all-true coverage is
deep-merged into variant 0 and the all-false coverage into
variant 1, so each role still ships exactly two matrix rounds.
* svc-ai-ollama keeps its single CI image-shrink variant; after the
self-reference fix it has no remaining dynamic flags so no extra
coverage entry is required.
* svc-db-{mariadb, openldap, postgres} drop variants.yml entirely
for the same reason.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
