potiuk opened a new issue, #69065:
URL: https://github.com/apache/airflow/issues/69065

   ## Summary
   
   FastAPI 
[0.137.0](https://fastapi.tiangolo.com/release-notes/#01370-2026-06-14) 
reworked the router
   layer into **lazy router inclusion**: `include_router()` no longer leaves a 
flat list of materialized
   `APIRoute`s in `router.routes` — included routers can appear as 
`_IncludedRouter` wrappers, and a
   router's dependencies are now **frozen into each route at include time**.
   
   We currently cap `fastapi<0.137.0` (#68562). #68826 lifts the cap (cadwyn 
7.1.0 now requires
   `fastapi>=0.137.1`) and fixes the execution-API part — but the bump is 
really a **broader migration
   that touches security-relevant route introspection**, so we want the FastAPI 
/ token-scope / security
   owners to confirm the direction before we commit.
   
   Related: #68826 (WIP), #68562 (the cap), #68578 (added the cap).
   
   ## Why this makes our code better — 0.137 *enables* the cleaner pattern
   
   Today the execution API injects the OTel trace-context dependency by 
mutating `route.dependencies` on a
   shared, module-global router *after* assembly (`_inject_trace_context_dep`) 
— with an idempotent
   **strip-and-re-add**, because that global router is processed more than once 
per process
   (`cached_app` + `InProcessExecutionAPI`) and we have to de-dupe our own 
prior injection.
   
   0.137's lazy inclusion **breaks that today** — but that's the point: the 
strip-and-re-add only ever
   existed to work around the old *mutable-shared-router* model. 0.137 freezing 
a router's dependencies
   into each route **at include time** is exactly the contract we want: it lets 
us declare the
   trace-context dependency **once, at build time**, in a fresh router per app, 
and it stays put. No
   shared mutable dependencies after routes are built, no post-hoc patching, 
**no strip-and-re-add**, and
   the test fixture that snapshot/restored the mutated global disappears. The 
migration *removes* a
   workaround rather than adding one.
   
   ## The hard part — token scope & security boundaries
   
   Several **security guards introspect `router.routes` expecting flat 
`APIRoute`s** and break under lazy
   inclusion:
   
   - `execution_api/test_token_scope_boundaries.py` — asserts each route's 
allowed token types
     (`{execution}` / `{workload}`); iterates `execution_api_router.routes`.
   - `core_api/.../test_routes.py::test_no_auth_routes` — asserts which routes 
do/don't require 401/403;
     fails with `AttributeError: '_IncludedRouter' object has no attribute 
'path'`.
   
   These are the checks that guarantee a route doesn't **silently lose auth or 
change token scope**. They
   must be migrated to traverse the new lazy structure and assert the *same* 
invariants — **not silenced
   to go green**. This needs the token/security owners' eyes, and likely a 
rethink of how the boundaries
   are introspected and tested.
   
   ## Proposed approach
   
   **One migration PR** (re-scope #68826 → "Migrate to FastAPI 0.137"):
   
   1. Bump `cadwyn>=7.1.0` + `fastapi>=0.137.1`, drop the cap.
   2. A single shared `iter_materialized_routes()` helper, routed through 
*every* introspection site
      (both security guards + execution token-scope) — one place to get the 
lazy→materialized traversal
      right, reviewed once.
   3. The build-time router factory for the trace-context dependency (already 
in #68826).
   4. Confirm api-server / dag-processor **startup** on 0.137 end-to-end (not 
just unit tests).
   5. A negative test proving the auth guards still bite (a route stripped of 
auth must fail
      `no_auth_routes`).
   
   Splitting this doesn't work cleanly — the introspection fixes can't be 
tested without 0.137 active, and
   the cap can't lift without the fixes — so it's one coherent migration.
   
   ## Timing
   
   **Not a 3.3.0 change** — too security-sensitive and too broad to rush into 
the imminent release.
   Targeting **3.4.0**; please **do not merge #68826 until this is agreed and 
properly scoped**.
   
   ## Who should weigh in
   
   cc @vincbeck (auth manager / security), @ashb · @kaxil · @amoghrajesh 
(`execution_api/` CODEOWNERS —
   token scope + the trace-context dep), @pierrejeambrun · @bugraoz93 (core-API 
FastAPI / `no_auth_routes`
   guard), @ferruzzi (token-scope-boundaries test) — does the direction 
(build-time factory + a shared
   route-materialization helper for the security introspection) look right, and 
should we proceed with the
   single-PR migration for 3.4.0? The call is yours.
   
   ---
   Drafted-by: Claude Code (Opus 4.8); reviewed by @potiuk 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]

Reply via email to