jroachgolf84 opened a new issue, #72266: URL: https://github.com/apache/airflow/issues/72266
## Description > This is an issue that is reserved for the Airflow Summit "Contributors" Workshop. This is denoted with the label `contributors-workshop`. Out of respect for the organizers and participants of this workshop, **please do not implement a PR that addresses this issue.** > > If this issue is still open following Airflow Summit, the label will be removed and the issue can be picked up. Two modules in the `edge3` provider have no dedicated test module, and (unlike most entries on the `OVERLOOKED_TESTS` list) they are not covered indirectly. Nothing under any `providers/*/tests/` directory imports them at all: | Module | Expected test file | | --- | --- | | `providers/edge3/src/airflow/providers/edge3/worker_api/auth.py` | `providers/edge3/tests/unit/edge3/worker_api/test_auth.py` | | `providers/edge3/src/airflow/providers/edge3/worker_api/app.py` | `providers/edge3/tests/unit/edge3/worker_api/test_app.py` | Both are currently suppressed in the `OVERLOOKED_TESTS` allowlist in [`airflow-core/tests/unit/always/test_project_structure.py`](https://github.com/apache/airflow/blob/main/airflow-core/tests/unit/always/test_project_structure.py). This is a scoped subset of the meta issue #35442, limited to the `edge3` provider. ### What should these tests cover? `auth.py` is the higher value of the two because it is the token check standing in front of every `/edge_worker/v1/` endpoint, and it has no direct coverage at all today. `jwt_token_authorization` deliberately collapses a table of PyJWT failures onto a single anonymised 403 via `_forbidden_response`, which logs the real reason server side and returns only an `error_id` to the caller. That is exactly the kind of contract that should be pinned by tests: - a token whose `method` claim matches the called method passes - a token whose `method` claim is missing, or does not match the called method, raises `HTTPException` with status 403 - each handled failure produces a 403, covering `BadSignature`, `InvalidAudienceError`, `InvalidSignatureError`, `ImmatureSignatureError`, `ExpiredSignatureError`, `InvalidIssuedAtError`, and the catch all `Exception` branch - the 403 detail stays anonymised and does not leak the underlying reason, since that anonymisation is the whole point of `_forbidden_response` - `jwt_token_authorization_rest` strips the `/edge_worker/v1/` prefix and passes the remainder as the method name, and falls back to the full request path when that prefix is absent Two practical notes for whoever picks this up. `jwt_validator()` is decorated with `@cache`, so a test that changes the configured secret needs `jwt_validator.cache_clear()` first or it will silently reuse the previous validator. The exception table is a natural fit for `@pytest.mark.parametrize`, and `conf_vars` should be used for the `api_auth` `jwt_secret` and `jwt_leeway` overrides. `create_edge_worker_api_app` is mostly wiring, so keep it proportionate: - the five routers are mounted under the expected prefixes, meaning jobs, logs, worker and health under `/v1` and the UI router under `/ui` - the `.cjs` mimetype is registered, since serving that incorrectly breaks the Edge Worker UI - the `/static` and `/res` static mounts are present ## Definition of Done 1. Add the two test modules at the paths in the table above. 2. Remove the two corresponding entries from `OVERLOOKED_TESTS` in `airflow-core/tests/unit/always/test_project_structure.py` 3. Both of these tests should pass: ```bash breeze testing providers-tests providers/edge3/tests/unit/edge3/worker_api/test_auth.py breeze testing providers-tests providers/edge3/tests/unit/edge3/worker_api/test_app.py ``` --- Drafted-by: Claude Code (Opus 5); reviewed and edited by @jroachgolf84 before posting. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
