kevinveenbirkenbach/computer-playbook
1
0 Fork 0
mirror of https://github.com/kevinveenbirkenbach/computer-playbook.git synced 2026-11-06 10:59:59 +00:00
Code Issues Packages Projects Releases Wiki Activity
14,682 Commits 9 Branches 78 Tags
fix/dev-python/python-upper-bound
Commit Graph
9 Commits
Author SHA1 Message Date
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 f6c8c85b1b test(lint): role-file-spot guard + 139-file mapping migration 
Add tests/lint/repository/test_role_file_spot.py: AST-walks every .py
file under the production trees and tests/, flags any reference to a
role file (standalone literal, f-string segment, pathlib `/` chain,
`+` concat chain, os.path.join / Path.joinpath / posixpath.join /
ntpath.join call) whose value or joined trailing operands match a
ROLE_FILE_* constant in utils.roles.mapping. The flag set is
auto-discovered from the module so adding a new constant extends
coverage without further edits. ROLE_FILE_README is exempt as too
generic. Per-line `# nocheck: role-file-spot` (same-or-above) is the
documented opt-out, catalog row added in
docs/contributing/actions/testing/suppression.md.

Migrate 139 .py files (377 occurrences) to use the constants:

- 135 files via direct constant import (`from utils.roles.mapping
  import ROLE_FILE_*`), with the symbol used in pathlib chains, in
  os.path.join / Path.joinpath calls, in f-string interpolation, and
  in glob patterns.
- 4 files via `# nocheck: role-file-spot` where the literal lives in
  argparse help text or in an allowlist tuple meant to declare the
  literal path.

Adjust utils/roles/dependency_resolver.py to use lazy-formatting
positional args in the logger.exception call (G003-clean) instead of
string concatenation against the constant.

Drop a redundant `if cond: return True; return False` guard in the
new test for `return cond` (SIM103-clean).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
 2026-05-08 13:21:22 +02:00
kevinveenbirkenbach 76b139c894 fix(lint): migrate raw IO + project walks to utils.cache + variant coverage 
Cleans up the lint debt the main->feature/web-app-kix merge surfaced.
Each change addresses a real failure surfaced by `make test`; no
``# nocheck`` blanket suppressions added.

Migrations to the shared cache helpers:
- ~25 test files swap raw ``yaml.safe_load`` / ``yaml.safe_dump`` /
  ``yaml.dump`` for ``utils.cache.yaml.{load_yaml_any,load_yaml_all,
  load_yaml_str,dump_yaml,load_yaml_all_str}``.
- ~30 test files swap raw ``glob.glob`` / ``os.walk`` / ``Path.rglob``
  for ``utils.cache.files.iter_project_files`` (or
  ``iter_non_ignored_files`` for the dependabot fallback walker, or
  per-role ``Path.iterdir`` / ``Path.glob`` where the scope is a
  single subtree).
- 3 ``Path.read_text`` lint helpers now route through
  ``utils.cache.files.read_text``.

Variant + service-flag fixes:
- web-app-kix: shift its subnet from 192.168.105.80/28 to
  192.168.105.128/28 to clear the collision with web-app-hugo and
  svc-prx-openresty.
- web-app-fediwall: add the missing ``mastodon`` / ``friendica``
  service flags referenced by ``meta/variants.yml``; pin the css /
  logout / dashboard / matomo / prometheus literals with
  ``# nocheck: dynamic-flag`` and add a third "everything off"
  variant so the variant-coverage matrix sees both true and false
  pinning for every dynamic flag.
- web-app-magento, web-app-shopware, web-svc-xmpp: switch the
  ``run_after: web-app-keycloak`` matcher from a literal-false
  oauth2/oidc opt-out to the ``"{{ 'web-app-keycloak' in group_names }}"``
  dynamic form, and extend each role's ``meta/variants.yml`` with the
  matching true/false (and shopware's LDAP-only) coverage.
- web-app-fusiondirectory, web-app-matrix: declare the
  ``matomo`` flag so their ``run_after: web-app-matomo`` resolves
  through an explicit services entry; add matomo true/false to both
  variants files.
- web-app-fediwall, web-app-mini-qr: add the
  ``# nocheck: logout`` opt-out directly above ``logout:`` so the
  logout-dashboard guard accepts the legitimately-disabled state.
- ~30 ``services.yml`` files: insert
  ``# nocheck: dynamic-flag`` directly above the literal
  ``css`` / ``redis`` / ``view`` / ``worker`` / ``email`` / ``html``
  / ``mariadb`` / ``opensearch`` / ``talk`` / ``logout`` blocks that
  the dynamic-flag guard surfaced.

Code-quality follow-ups uncovered by ruff:
- utils/cache/yaml.py: collapse ``load_yaml_all_str`` to a one-line
  return + on-call ``# nocheck: direct-yaml`` so the lint accepts
  the cache module's own ``yaml.safe_load_all`` invocation.
- utils/templating.py: replace the ``Optional[Any]`` import that
  slipped in from main with the modern ``Any | None``; rewrite the
  cleanup ``try/except/pass`` blocks as
  ``contextlib.suppress(Exception)`` so the imported
  ``contextlib`` module is consumed consistently.
- 7 new-from-main lint test files (test_inline_script_block_size,
  test_no_inline_multiline_php_in_sh, test_no_lookup_config_jinja_default,
  test_no_inline_multiline_sql, test_web_role_no_web_dependency,
  test_shared_primary_entity_run_once, test_fediwall_active): drop
  their local ``Path(__file__).parents[N]`` PROJECT_ROOT
  computations and import the package-level constant instead.
- roles/web-app-moodle/templates/playwright.env.j2: drop the
  unreferenced ``KEYCLOAK_REALM_NAME`` env key that the
  playwright-env-keys-used guard flagged.
 2026-05-08 00:03:14 +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 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
kevinveenbirkenbachandClaude Opus 4.7 4d0ebc2b91 refactor(suppress): migrate # noqa: <rule> to # nocheck: <rule> repo-wide 
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>
 2026-05-07 14:07:55 +02:00
kevinveenbirkenbach 837d0c5335 test(suppression): unify # noqa / # nocheck marker grammar 
Introduce a single grammar for per-line, per-block, and per-file
suppression markers across the test suite, replacing seven ad-hoc
syntaxes that had drifted apart.

Helper
  utils/annotations/suppress.py exposes is_suppressed_at,
  is_suppressed_in_head, is_suppressed_anywhere, line_has_rule, and
  suppressed_line_numbers. Markers may use either `noqa` or `nocheck`
  (case-insensitive synonyms), live inside `#`, `//`, `{# ... #}`, or
  `<\!-- ... -->` comments, and combine multiple kebab-case rules
  comma-separated. 17 unit tests cover the grammar in isolation.

Refactored consumers
  - tests/external/repository/test_urls_reachable.py (rule: url)
  - tests/external/docker/test_image_versions.py via
    utils/docker/version_updater.py (rule: docker-version)
  - tests/lint/repository/test_no_direct_yaml_calls.py (rule:
    direct-yaml)
  - tests/lint/ansible/test_service_shared_consistency.py (rule:
    shared)
  - tests/lint/ansible/roles/test_web_app_email_integration.py (rule:
    email)
  - tests/lint/repository/test_python_file_size.py (rule: file-size)
  - tests/lint/ansible/test_run_once_tags.py +
    tests/integration/roles/run_once/test_schema.py (rules: run-once,
    run-once-suffix)

Marker migration
  12 role tasks/main.yml files: `# run_once_<key>: deactivated`
  -> `# nocheck: run-once`. roles/web-app-suitecrm/meta/services.yml
  drops a malformed inline `noqa: intentional legacy pin` from a TODO
  comment so the new parser does not pick it up as a stray rule.

Caching wins
  Test files that previously loaded YAML / read files via raw
  yaml.safe_load + Path.read_text now route through utils.cache.yaml
  (load_yaml_any / load_yaml_str) and utils.cache.files.read_text:
    - test_service_shared_consistency.py
    - test_web_app_email_integration.py
    - tests/integration/roles/run_once/test_schema.py
    - utils/docker/version_updater.py
  This both speeds up repeated reads and stays consistent with the
  direct-yaml lint policy.

Documentation
  New SPOT page at docs/contributing/actions/testing/suppression.md
  with grammar, position semantics, the catalog of rules, examples,
  and an "adding a new rule" checklist. lint.md and external.md drop
  their per-rule tables and link to the new page. All touched docs
  conform to docs/contributing/documentation.md (no em-dash thought
  breaks, no SPOT self-labels, RFC-2119 keywords used consistently).

Settings sort tooling
  scripts/lint/sort_claude_settings.sh wraps utils/claude/settings_sort.py
  and is wired into scripts/lint/autoformat.sh. The tooling fixes the
  ASCII-ascending requirement enforced by
  tests/lint/repository/test_claude_settings_sorted.py; the test's
  failure message now points operators at the script. .claude/settings.json
  is committed in sorted form.
 2026-04-30 11:54:46 +02:00
kevinveenbirkenbachandClaude Opus 4.7 02da33b0a5 refactor(tests): reorganize into topical sub-clusters and consolidate linting 
Under tests/integration/roles/ introduce four sub-clusters (meta/,
dependencies/, applications/, run_once/) plus nested dependencies/run_after/
and applications/id/. Fold tests/integration/application_id/ into
roles/applications/id/ and tests/integration/run_once/ into roles/run_once/.
Pull four application-related tests out of tests/integration/ into
roles/applications/, and test_run_after_references.py into the run_after/
sub-cluster.

Consolidate tests/integration/linting/ into tests/lint/{ansible,filesystem,
repository}/, so repository-wide lint checks now run under make test-lint
rather than make test-integration.

Each new cluster ships a README.md and __init__.py; cross-referencing
READMEs (categories, ports, services, roles, applications) are updated to
point at the new paths. Path depths inside moved tests and one hardcoded
ignore path in test_domain.py are adjusted accordingly. No behavior change;
all 32 lint tests and every moved cluster pass locally.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
 2026-04-21 12:22:33 +02:00