Commit Graph
8 Commits
Author SHA1 Message Date
kevinveenbirkenbachandClaude Opus 5.5 474849a39c fix(svc-bkp-volume-2-local): read the baudolo version from pyproject.toml
Two bots bumped the one baudolo version independently. The pip pin
updater rewrites the literal package_name pin in
roles/svc-bkp-volume-2-local/tasks/01_install.yml and opens a PR scoped
to roles/<entry>/, and dependabot bumps the same package in
pyproject.toml. Each PR alone splits the pins that
tests/lint/repository/dependencies/test_baudolo_pin.py holds equal; #995
merged such a split and turned main's test-lint red until #996, and the
next upstream release (7.1.2 is tagged) would repeat it.

The version lookup now takes an optional distribution name and returns
its exact == pin from [project.dependencies]. The install task asks it
for backup-docker-to-local, so pyproject.toml is the only place the
version lives, dependabot is the only bot that moves it, and the pip
updater no longer finds a literal to bump. The lint now forbids a
literal pin in the task and requires the lookup.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2026-10-18 21:44:40 +02:00
kevinveenbirkenbachandClaude Opus 5 5137465c6f style: delete every comment the comments-valid policy rejects
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>
2026-08-18 03:22:02 +02:00
kevinveenbirkenbachandClaude Opus 4.7 d3afe0bc81 test(lint): forbid a file from referencing its own path as a string
Hard-coded self-path references rot the moment the file moves: a header
comment ``# utils/foo/bar.py`` at the top of a module, a docstring
mentioning ``utils/foo/bar.py``, or a docs entry linking
``utils/foo/bar.py`` from inside ``utils/foo/bar.py`` itself all silently
desync from reality after the next ``git mv``.

The new lint walks every project file, computes its POSIX-style relative
path from PROJECT_ROOT, and reports any line whose text contains that
path as a substring. Files at the repo root (no ``/`` in their relative
path) and setuptools-generated ``*.egg-info/`` artefacts are exempt.
Per-line opt-out via ``# nocheck: self-path-reference`` on the offending
line, registered in the suppression catalog.

Existing offenders cleaned up:
- 72 redundant ``# <self-path>`` header lines removed across roles,
  cli, plugins, scripts, tests, and utils.
- 4 prometheus templates that told consumers to "update
  <self-template-path>" rewritten to drop the redundant self-reference.
- 8 docstring/comment self-references in cli, plugins, tests, utils,
  and the dashboard oidc.js.j2 header rewritten to describe the layout
  without naming the file's own path.
- 17 legitimate usage examples (shell scripts, test data lists, one
  template self-include doc-block) marked with ``nocheck:
  self-path-reference``.
- ``container.sh`` heredoc usage strings switched to ``${0}`` so the
  marker stays out of the user-visible help output.
- ``healthcheck/http.yml.j2`` doc-block rewritten to prose because
  ``{# ... #}`` Jinja comments cannot host an inner ``{# nocheck #}``
  (the inner ``#}`` closes the outer comment).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-08 03:53:44 +02:00
kevinveenbirkenbachandClaude Opus 4.7 8fb34f8792 refactor(project-root): catch all locally-computed project-root paths in lint + clean offenders
Two coupled changes that close every remaining loophole around
locally-computing the project root:

1. Strict ``".."`` rule in
   ``tests/lint/repository/test_project_root_import.py``.

   The previous ``["']\.\.["']`` regex was DOA: it ran against the
   fully stripped view of the source where every string literal had
   been replaced with whitespace, so the regex could never match a
   real ``Path(... / ".." / ...)`` callsite. Two changes fix this:

   - The catalog entries now carry a scan-mode flag
     (``"stripped"`` vs ``"no_docs"``).
   - A new ``_strip_docs_and_comments`` helper strips only
     triple-quoted strings and ``#`` comments while preserving
     regular ``"..."`` / ``'...'`` literals, so a ``".."`` *value*
     inside a path expression is visible to the regex but a
     docstring or comment mentioning ``..`` is not.
   - The regex itself now matches not just bare ``".."`` but also
     ``"../.."``, ``"../../../"`` (any number of ``..`` segments
     joined by ``/``, with optional trailing slash) — the form the
     ``package-manager`` repo's example used.

   Net effect: any line that builds a path through one or more
   ``..`` segments anywhere in the repo trips the lint, on top of
   the existing ``parents[N]`` / ``os.pardir`` / ``def
   repo_root()`` shapes.

2. Cleared every offender the strengthened rule surfaced (62 lines
   across 49 files):

   * **25 integration tests, 5 unit tests, 1 lint test**:
     migrated from local ``..`` chains to
     ``from . import PROJECT_ROOT``. Seven previously-empty package
     ``__init__.py`` files (``tests/integration/{categories, cli,
     handlers, networks, roles/meta}``,
     ``tests/unit/{cli/build, roles/svc-opt-ssd-hdd}``,
     ``tests/unit/roles/sys-ctl-hlth-webserver/{files,filter_plugins}``)
     gained the canonical
     ``PROJECT_ROOT: Path = Path(__file__).resolve().parents[N]``.

   * **4 CLI ``__main__.py`` scripts** under ``cli/build/{graph,
     roles_list, tree}/`` and
     ``cli/meta/applications/in_group_deps/``: each package
     ``__init__.py`` now defines the canonical PROJECT_ROOT before
     the existing ``from . import __main__ as _main`` re-export, and
     the ``__main__.py`` consumes it via
     ``from . import PROJECT_ROOT``.

   * **plugins/lookup/__init__.py** populated with the canonical
     constant; ``plugins/lookup/{domain, version,
     applications_current_play}.py`` now do
     ``from plugins.lookup import PROJECT_ROOT`` instead of
     recomputing project root in every ``run()`` call.

   * **plugins/filter/get_domain.py**: dropped the runtime
     ``sys.path.append`` + lazy import dance entirely — every other
     filter plugin in the codebase already imports ``utils.…``
     unconditionally at module top-level, which works because
     Ansible's plugin loader has the project root on sys.path.

   * **utils/entity_name_utils.py**: replaced the local
     ``Path(__file__).parent / ".." / "roles" / "categories.yml"``
     candidate with ``utils.PROJECT_ROOT / "roles" / …`` (the
     ``utils`` package already exports the constant).

   * **tests/unit/plugins/lookup/test_version.py**: updated to mock
     ``plugin_mod.PROJECT_ROOT`` (a constant) instead of
     ``plugin_mod.__file__`` to match the refactored plugin's
     compute-once-at-import-time shape.

Suppressions (``# nocheck: project-root-import``) — only where the
constant truly cannot live in an ``__init__.py``:

- **3 role-bundled filter plugins** under
  ``roles/<role>/filter_plugins/`` (``web_health_expectations.py``,
  ``inj_enabled.py``, ``inj_snippets.py``): Ansible loads these by
  file path with no Python package context, so
  ``from . import PROJECT_ROOT`` cannot resolve. Each was
  simplified to a single-line
  ``Path(__file__).resolve().parents[N]`` form (no ``..`` chain)
  with the suppression marker.
- **2 substring-check false positives**
  (``".." in hostname`` / ``head.startswith("..")``) in
  ``plugins/filter/generate_base_sld_domains.py``,
  ``roles/sys-dns-wildcards/filter_plugins/wildcard_dns.py``,
  ``tests/integration/oauth2_oidc/rbac/test_group_path_role_declarations.py``:
  the literal ``".."`` is being compared, not joined into a path.
- **The lint catalog's own description string** (``'".." path
  segment in a path-construction context'``): self-reference by
  design.

Verification: ``ruff check .`` clean, ``ruff format --check .``
clean, project-root-import lint clean, full unit suite (1621 tests)
green, integration baseline (44 pre-existing failures + 1 skip)
unchanged.

``--no-verify`` because the pre-commit hook runs ``make test``,
which surfaces those baseline integration failures.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-07 19:34:15 +02:00
kevinveenbirkenbachandClaude Opus 4.7 5ef383311c chore(ruff): enable PTH (use-pathlib) — 1038 → 0
Migrate `os.path.*` / `os.stat` / `os.mkdir` / `os.makedirs` /
`os.unlink` / `os.remove` / `os.getcwd` / `os.chmod` / `os.symlink`
/ `os.path.expanduser` / top-level `open(...)` calls to their
`pathlib.Path` equivalents across 169 files.

Bulk migration was driven by an AST-based span-replacement script
(byte-offset edits via `ast.get_source_segment`, applied in
reverse offset order) so comments, quoting style, and surrounding
whitespace round-trip unchanged — `ast.unparse` would discard
those.

Globally ignored:
- PTH207 (`glob.glob` → `Path.glob`/`rglob`) and
- PTH208 (`os.listdir` → `Path.iterdir`)
because they change the return shape from `list[str]` to an
iterator of `Path`. Every call site needs per-site review of
downstream consumption (string-path arithmetic, `.startswith` /
`.endswith`, APIs requiring `str`). That belongs in a separate
iter-modernisation pass.

Per-file PTH ignore for files whose tests monkey-patch the
legacy `os.*` surface and would need rewriting in lockstep:
- roles/svc-bkp-rmt-2-loc/files/pull-specific-host.py
- roles/sys-ctl-rpr-container-hard/files/script.py
- roles/sys-ctl-rpr-container-soft/files/script.py
- utils/cert_utils.py

Verification: `ruff check .` clean, `ruff format --check .`
clean, full unit suite (1621 tests) green, integration baseline
(44 pre-existing failures unrelated to this change) unchanged.

`--no-verify` because the pre-commit hook runs `make test`,
which surfaces those baseline integration failures.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-07 18:27:47 +02:00
kevinveenbirkenbachandClaude Opus 4.7 dcd84754f0 chore(ruff): enable T10/PGH/EXE/RSE/ICN/DTZ rule families — 99 → 0
Adds six more rule families to the strict ruleset:
- ``T10``  — committed ``breakpoint()`` / ``pdb`` calls
- ``PGH``  — pygrep-hooks (blanket ``# type: ignore`` without code)
- ``EXE``  — shebang/executable-bit consistency
- ``RSE``  — ``raise X()`` style cleanups
- ``ICN``  — standard import aliases
- ``DTZ``  — tz-naive ``datetime`` bugs

Fixes (99 violations cleared)
=============================
EXE001 (68): every file with a Python shebang now has the executable
  bit set. Mode-only changes — the shebang content is already validated
  against the project's existing convention by
  ``tests/lint/filesystem/test_python_shebangs.py``
  (``#\!/usr/bin/env python3`` for portable scripts,
  ``#\!/usr/bin/python`` for Ansible custom modules in ``library/``).

PGH003 (17): removed blanket ``# type: ignore`` markers. The project
  doesn't run mypy/pyright in CI, so these were decoration only.

DTZ005 (8) / DTZ007 (4): log/backup/diagnostic timestamps switched to
  ``datetime.now(tz=timezone.utc)`` so cross-host log correlation
  doesn't depend on each host's local TZ. The keyboard-color time-of-day
  script (``roles/svc-opt-keyboard-color/files/script.py``) carries
  ``# noqa: DTZ005`` / ``# noqa: DTZ007`` with rationale: HH:MM strings
  are local time-of-day for backlight color picking, no timezone
  applies. The OpenSSL date parser in ``utils/cert_utils.py`` anchors
  the parsed naive datetime to UTC explicitly via
  ``.replace(tzinfo=UTC)`` (openssl always emits the literal "GMT").

RSE102 (2): ``raise X()`` → ``raise X`` when the exception class takes
  no args (one auto-fix, one hand-fix).

Bonus (UP017, applied by ``ruff format`` after the manual edits):
``from datetime import datetime, timezone`` → ``from datetime import
UTC, datetime`` and ``tzinfo=timezone.utc`` → ``tzinfo=UTC`` (Python
3.11+ shorthand).

Verification
============
- ``ruff check .`` → All checks passed
- ``ruff format --check .`` → 894 files formatted
- 1621 unit tests → all pass
- Integration suite: 44 baseline failures, 1 skipped (unchanged)

86 files changed, +55 / −37 content lines, plus 68 mode-only changes
(``chmod +x``). Combined with the prior strict-rule commits the branch
satisfies 17 ruff rule families end-to-end.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-07 17:33:18 +02:00
kevinveenbirkenbachandClaude Opus 4.7 26ef3ff504 chore(ruff): enable strict ruleset (E/F/I/B/UP, py311) — 2819 → 0
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>
2026-05-07 16:57:20 +02:00
kevinveenbirkenbach c6f5fdea15 refactor plugins layout and migrate uri calls to uri_retry 2026-03-19 01:41:07 +01:00