Conversation
0d5e0f2 to
8674135
Compare
There was a problem hiding this comment.
Pull request overview
Adds structured audit logging across the FastAPI proxy, with optional JSON output, and updates the runtime entrypoint to start via python -m powerdns_api_proxy so Uvicorn can be configured with a custom logging config.
Changes:
- Introduces
AuditMiddlewareto emit audit log events for/api/v1/*requests (including unauthorized/forbidden responses). - Adds
AuditLogger.audit()and aJSONFormatterto support structured JSON logs whenLOG_FORMAT=json. - Updates runtime/docs/tooling: Uvicorn
log_config, new__main__.pyrunner, README logging docs, Docker/Makefile tweaks, and new unit tests.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/middleware_test.py | Adds tests asserting audit events are emitted by middleware. |
| tests/unit/logging_test.py | Adds tests for audit log formatting (text + JSON). |
| powerdns_api_proxy/uvicorn_config.py | Adds dictConfig for Uvicorn logging with optional JSON formatter. |
| powerdns_api_proxy/proxy.py | Registers AuditMiddleware on the FastAPI app. |
| powerdns_api_proxy/middleware.py | Implements audit logging middleware capturing env/method/path/status/payload/query params. |
| powerdns_api_proxy/logging.py | Adds AuditLogger + JSONFormatter and toggles formatter via LOG_FORMAT. |
| powerdns_api_proxy/config.py | Minor refactor in load_config return path (no functional change). |
| powerdns_api_proxy/main.py | Runs Uvicorn programmatically with LOGGING_CONFIG. |
| README.md | Documents JSON/audit logging and adds env var examples. |
| Makefile | Switches make run to python -m powerdns_api_proxy. |
| Dockerfile | Switches CMD to python -m powerdns_api_proxy and installs jq. |
Comments suppressed due to low confidence (1)
powerdns_api_proxy/logging.py:91
- Audit events are logged at INFO (
AuditLogger.audit()usesself.info(...)), but the stderr handler is filtered byLOG_LEVEL. WithLOG_LEVEL=WARNING(as shown in the README), audit logs won’t be emitted to stderr. Consider ensuring audit logs bypass this filter (e.g., a dedicated audit handler/logger at INFO) or documenting the requirement explicitly.
default_stream_handler = logging.StreamHandler(stderr)
default_stream_handler.setLevel(LOG_LEVEL)
default_stream_handler.setFormatter(default_formatter)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
6d8e212 to
93f37d6
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 10 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try: | ||
| token = request.headers.get("X-API-Key", "") | ||
| if token: | ||
| environment = get_environment_for_token(_config, token) | ||
| environment_name = environment.name | ||
| except Exception: | ||
| pass | ||
|
|
||
| # Store request body for logging (only for write operations) | ||
| payload = None | ||
| if request.method in ["POST", "PUT", "PATCH"]: | ||
| try: | ||
| # Read raw request body bytes; FastAPI/Starlette will cache the body for downstream handlers | ||
| body_bytes = await request.body() | ||
| if body_bytes: | ||
| payload = json.loads(body_bytes) | ||
| except Exception: | ||
| pass | ||
|
|
There was a problem hiding this comment.
There are multiple broad except Exception: pass blocks around token parsing and payload JSON parsing. This can hide real bugs (e.g., config errors) and makes troubleshooting difficult. Narrow these to the expected exceptions (e.g., ValueError from get_environment_for_token, json.JSONDecodeError) and consider logging at debug level when unexpected exceptions occur.
WIP feat: audit log and structured logging
Add Audit Logging with JSON Support
Adds automatic audit logging for all API operations via middleware.
Features
LOG_FORMAT=jsonCloses #182