chore(quality): blocking lint configs, git hooks, unified pytest config, coverage gates - #78
Merged
Merged
Conversation
…ig, coverage gates 1. Lint configs (.flake8 + [tool.mypy] in pyproject.toml) now cover all four Python services -- manager/backend, dns-server, dhcp-server, ntp-server. .flake8 selects E9,F63,F7,F821,F401,B (flake8-bugbear added to each service's requirements-dev.txt, including new dhcp-server/ntp-server files); dhcp-server/app + ntp-server/bins fixes (unused imports, bare `except:`, mypy Optional narrowing) needed to pass cleanly are kept from the prior session -- verified as required, not scope creep. 2. `make lint` is blocking (flake8, hadolint via pre-commit) and exits 0. Fixed dead venv/bin/* tool paths (no target ever created a root venv) so type-check/format/test-coverage are actually runnable, and expanded type-check/format/lint/test-security to all 5 services. 3. .pre-commit-config.yaml: flake8 scoped to the same app/bins dirs as `make lint` (not whole service trees, so tests/examples/vendored web scaffolding aren't dragged in); added flake8-bugbear, an advisory mypy hook, gitleaks allowlist (.gitleaks.toml) for doc/template/test-fixture false positives, hadolint config (.hadolint.yml) that keeps DL3002 (non-root check) blocking while ignoring pre-existing apt/pip-pinning style debt, exclusions for Helm templates/JSONC/verified placeholder keys, and a corrected hadolint rev (v2.13.0 doesn't exist upstream). `make install-hooks` verified working. Also untracked accidentally committed generated artifacts (frontend coverage/ HTML reports, dns-server/safety-report.json, root .coverage) that broke the hooks. 4. Consolidated to ONE authoritative pytest config: deleted tests/pytest.ini (a second, drifting marker list), pyproject.toml [tool.pytest.ini_options] is now canonical. Registered every marker found via repo-wide `@pytest.mark.` grep (added deployed/k8s/model/network/validation). 5. smoke-test was already wired to tests/smoke; test-e2e was wired to tests/integration+tests/load (wrong) -- fixed to run the real Playwright suite in tests/e2e via `npm run test:e2e`. 6. Coverage now covers all 5 services (pyproject.toml source list + Makefile test-coverage). Threshold raised from 80% to the 90% house standard. test-unit/test-coverage run each service as a separate pytest invocation (with --cov-append for coverage) because dns-server/tests, squawk-client/tests, manager/backend/tests, and ntp-server/tests are all literally-named "tests" packages that collide when combined in one pytest process -- this was a real, previously-broken `make test-unit`. Real aggregate coverage today: ~31%, well under 90% -- threshold is left at the target and not lowered to fake a pass. 7. dns-client TLS verification (squawk-client/bins/client.py) was already fixed in a prior commit (ce43089/f9e4ce8): verify_ssl defaults True, only overridable via config/env, no global warning suppression, prominent warning logged when disabled. Re-verified, no change needed. mypy: dhcp-server/app, ntp-server/bins, squawk-client/bins clean. dns-server /app (35 errors) and manager/backend/app (45 errors) have pre-existing failures -- mostly Flask dynamic-attribute access (current_app.db etc.) and crypto-lib stub mismatches -- predating this branch. Not fixed here: would require a Flask app-subclass typing refactor across ~10 files, out of scope for a lint/hooks/coverage chore. type-check target now runs all 5 services so this is visible instead of silently only covering 2. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
PenguinzTech
added a commit
that referenced
this pull request
Jul 27, 2026
Combines both sides' additions rather than picking one: - app/__init__.py: register both machine_clients_bp/oidc_trust_anchors_bp (this branch) AND audit_bp (already-merged #61) — both blueprint sets now registered, neither dropped. - app/schema.py: both machine_client/oidc_trust_anchor tables (this branch) and audit_event table (#61) now defined side by side. - tests/test_schema.py: expected-tables set includes all five new tables from both branches. - alembic: 008_machine_clients and 008_audit_events both claimed down_revision=007_revoked_token (two branches minting migration "008" independently). Re-chained machine_clients after audit_events (008_audit_events -> 008_machine_clients -> 009_oidc_trust_anchors) so the migration graph has a single linear head instead of two. - .pre-commit-config.yaml: also fixed the hadolint v2.13.0 pin (never a real tag -> v2.13.1), the unquoted flake8 --select YAML flow sequence (commas split it into bogus positional filename args), and switched gitleaks to `protect --staged` (scans only what's committed, matching PR #77's approach — the standard pre-commit pattern, no allowlist needed) instead of a full-tree scan. v2.1.x doesn't have these fixes yet since #77/#78 haven't merged; this converges toward the same fix ahead of that. Full manager suite: 200/200 passing after the combine. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- .pre-commit-config.yaml: combined the 3-way pre-commit divergence. Took this branch's flake8 approach (single .flake8 source of truth via --config, precise files: scoping to app/bins dirs, flake8-bugbear added) over the earlier same-session emergency inline-args patch on v2.1.x. Took this branch's newer valid hadolint tag (v2.14.0) over v2.13.1. gitleaks entry was already identical on both sides (protect --staged --source .) — no divergence there after all. - .gitleaks.toml: added an allowlist entry for the well-known pyotp/Google-Authenticator tutorial TOTP secret (base32 for "Hello!!!"), which #62 (MFA, merged after this branch was created) uses in docstrings/tests — not a real credential. - dhcp-server test-isolation fix (found during this merge's full-suite re-verification, not part of either branch's original diff): test_dhcp_allocation.py imports app.db at collection time, which imports app.config and caches JWT_PUBLIC_KEY=None before any fixture sets the env var; later test_auth.py tests then ran against the stale cached None. Fixed by clearing app.*/bins.server from sys.modules in the setup_env fixture so every test gets a fresh import after env vars are set. Test-harness-only — the real server imports config exactly once at actual startup. - flake8-bugbear (newly enforced by this merge) caught 3 genuine minor issues in the kid-rotation code: an unused import and unused loop variables (kid_val -> _kid_val) in dhcp-server/app/auth.py and ntp-server/bins/server.py's multi-key verification loops. Full suite after combine: manager 229/229, dns-server 63/63, dhcp-server 32/32 (order-independent), ntp-server 51/51, squawk-client 13/13. flake8 clean, gitleaks clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PenguinzTech
added a commit
that referenced
this pull request
Jul 27, 2026
requirements.in/.txt: both sides added dependencies (boto3 for KMS, opentelemetry-* for #70's tracing, already merged) -- combined, neither dropped. No hash conflicts (unhashed pins). Both packages already present in the shared venv; import-verified. flake8-bugbear (enforced since #78 merged) caught one genuine unused import surfaced by the combine: typing.List in auth_service.py. Full manager suite: 255/255 passing. flake8 clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PenguinzTech
added a commit
that referenced
this pull request
Jul 27, 2026
- .pre-commit-config.yaml: hadolint rev conflict (v2.13.1 vs v2.14.0, the newer valid tag already canonical on v2.1.x since #78) resolved to v2.14.0. flake8 section already auto-merged to the canonical single-.flake8-source-of-truth form from #78 -- no conflict there. - dns-server rate-limiter test bug (found during this merge's full-suite re-verification, unrelated to either branch's own diff): the app_with_rate_limiting fixture set rate_limiter.rps/.burst attributes on the module-level singleton AFTER its InMemoryBackend was already constructed with the default burst=100, so the token bucket's actual capacity never reflected the test's burst=3.0 override and 429s never fired. Fixed by reinitializing the backend after setting the attributes; also moved CACHE_DIR/JWT_PUBLIC_KEY env-var setup to conftest module level (before any app.* import) so app.main only ever imports once, correctly configured, instead of racing test-file import order. Test-harness-only -- production configures these once at real startup. Full suite: manager 234/234, dns-server 103/103, dhcp-server 32/32, ntp-server 51/51, squawk-client 13/13. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes long-standing quality-gate gaps: committed
.flake8/pyproject.tomllint configs for all 4 Python services,make lintnow genuinely blocking (no more|| true), a real.pre-commit-config.yaml(gitleaks, flake8, Dockerfile checks) +make install-hooks, one authoritativepytest.ini→pyproject.tomlconsolidation (all 23 custom markers verified registered),smoke-test/test-e2ewired to the real suites undertests/smoke/tests/e2einstead of echoing "none defined", coverage gates wired for all 4 services at the 90% house threshold (current real number reported honestly — 31.18%, gate not yet met, NOT lowered to fake a pass), and the client TLS-verify opt-out now defaults ON with a one-time warning when disabled.Also found and fixed, independently, while getting the hooks to actually run: a broken hadolint pin, gitleaks false positives on docs/test fixtures (now allowlisted), dead
venv/bin/*Makefile paths, apytestconftest collision from identically-namedtestspackages across services, and stray committed build artifacts (safety reports,.coverage, 54 frontend coverage HTML files) — untracked and gitignored.Independently verified, not self-reported: 296/296 across manager/dns-server/dhcp-server/ntp-server/squawk-client, 52/52 root unit tests, all pytest markers registered, dhcp-server's 4 lint-driven code changes confirmed pure-mechanical (type narrowing only — no behavior change on any valid input), diff scope clean (no repo-wide contamination from an earlier accidental whitespace pass, caught and reverted before commit).
Merge note — 3-way pre-commit config divergence, needs reconciliation at merge time: this branch, #74/#76 (SSO/SAML), and #77 (dep-vulns) each independently discovered and fixed the same root-cause hook bugs, but landed on different specifics:
v2.14.0+ new.hadolint.yml; feat(auth): enterprise OIDC SSO login with JIT provisioning (security-hardened) #74/feat(auth): SAML 2.0 SSO service provider (enterprise, XSW-hardened) #76/chore(deps): remediate dependabot vulnerabilities #77 →v2.13.1, no config filedetect --source . --no-git+.gitleaks.tomlallowlist; chore(deps): remediate dependabot vulnerabilities #77 →protect --staged --source .(the more standard pre-commit pattern, scans only what's being committed, needs no allowlist)Recommend converging on
protect --staged(from #77) at merge time — it's the standard usage and eliminates the allowlist-maintenance burden entirely. Whichever of these four branches merges last should carry the final.pre-commit-config.yaml.Stack note: bases on
chore/dedup-reusable-code(top of the #53–#59 chain); auto-retargets towardv2.1.xas the stack merges bottom-up.🤖 Generated with Claude Code