You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "Pushkal-Gupta (via GitHub)" <gi...@apache.org> on 2026/10/16 16:45:10 UTC
[PR] Fix missing and incorrect endpoints in the API permission reference" [airflow]
Pushkal-Gupta opened a new pull request, #74871:
URL: https://github.com/apache/airflow/pull/74871
Depends on #74654. That one is approved and green and needs nothing more from me, it just needs a committer. This branch is stacked on it and touches the same two scripts, so until it lands the diff here carries its commits too. I'll rebase once it merges.
The generated API permission reference lists `PUT /api/v2/backfills`, which is not a real endpoint, and leaves out six endpoints that are. It also prints five paths with a `:path` suffix that no client ever calls.
### What's wrong
`airflow-core/docs/security/api_permissions_ref.rst` is generated by `scripts/ci/prek/extract_permissions.py`, which statically reads the route decorators under `api_fastapi/core_api/routes/public/`. I compared what it produces against FastAPI's own `v2-rest-api-generated.yaml`, which covers the same routers, so the two should agree exactly. The spec has 134 operations. 22 of them are missing from the doc, and 17 rows in the doc have no matching operation.
Six of the missing ones are backfill endpoints:
```
POST /api/v2/backfills/dry_run
GET /api/v2/backfills/{backfill_id}
GET /api/v2/backfills/{backfill_id}/dag_runs
PUT /api/v2/backfills/{backfill_id}/cancel
PUT /api/v2/backfills/{backfill_id}/pause
PUT /api/v2/backfills/{backfill_id}/unpause
```
`backfills.py` is the one route file that passes the path as a keyword:
```python
@backfills_router.put(
path="/{backfill_id}/pause",
dependencies=[Depends(action_logging()), Depends(requires_access_backfill(method="PUT"))],
)
```
The extractor only read `decorator.args[0]`, so for all eight routes in that file the path came out empty and `full_path` ended up as just the router prefix, `/api/v2/backfills`. Dedup then folded eight operations into three rows, and one of those three is `PUT /api/v2/backfills`, which doesn't exist, since PUT only appears on the sub-paths. 126 routes in the repo pass the path positionally and 8 pass it as `path=`, which is probably why nobody hit this.
The other 16 rows are a smaller thing. Routes declared as `/{pool_name:path}` get printed with the converter still attached. That's Starlette's matching syntax rather than part of the URL, and the OpenAPI spec and the real request path are both `/{pool_name}`. It affects pools, variables, xcoms and the two state-store paths.
The FAB page comes out of the same extractor, so it has the same holes. There it also means an operator looking up any of the six backfill endpoints finds no row telling them which FAB permissions and minimum role it needs.
### The fix
Read the path whichever way the decorator passes it, and drop the converter so the documented path is the one clients call. If a decorator has no path at all, raise instead of quietly falling back to the prefix, since that fallback is what turned a parsing gap into wrong documentation.
After the change the doc covers all 134 operations with nothing left over: both the 22 missing and the 17 phantom rows go to 0, and the entry count goes from 137 to 142.
I made the same two changes in `fab_permissions_doc.py`, and also let it walk `async def` handlers the way `extract_permissions.py` already does. Nothing there is broken today, since every FAB route is a sync `def` with a positional path, but it would drop rows silently rather than fail if that changed. I left its tolerant `continue` for a non-constant path alone rather than making that raise too, since that part isn't what this PR is about.
### Tests
Unit tests for both helpers, including the `path=` form taken from `backfills.py` and a decorator with no path at all, plus two end-to-end cases on synthetic route files for the keyword path and the converter.
The guard I most wanted is an invariant rather than a snapshot: every route decorator in the source has to produce its own row. Against the current code it fails with `backfills.py declares 8 routes but the table holds 3 distinct operations`, so it would have caught this when `backfills.py` was written. There's a second check that no documented path contains a converter.
All four of those fail without the change. `scripts/tests/` goes from 1684 to 1698 passed, 1 skipped. ruff, mypy and the 43 prek checks that apply to these five files are clean, and both hooks pass leaving the docs byte-identical on a second run.
No newsfragment, since this is a generated doc and the tooling behind it, with no user-facing code change.
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes (please specify the tool below)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix missing and incorrect endpoints in the API permission reference [airflow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #74871:
URL: https://github.com/apache/airflow/pull/74871#issuecomment-6269091725
### Backport successfully created: v3-4-test
Note: As of [Merging PRs targeted for Airflow 3.X](https://github.com/apache/airflow/blob/main/dev/README_AIRFLOW3_DEV.md#merging-prs-targeted-for-airflow-3x)
the committer who merges the PR is responsible for backporting the PRs that are bug fixes (generally speaking) to the maintenance branches.
In matter of doubt please ask in [#release-management](https://apache-airflow.slack.com/archives/C03G9H97MM2) Slack channel.
<table>
<tr>
<th>Status</th>
<th>Branch</th>
<th>Result</th>
</tr>
<tr>
<td>✅</td>
<td>v3-4-test</td>
<td><a href="https://github.com/apache/airflow/pull/75099"><img src="https://img.shields.io/badge/PR-75099-blue" alt="PR Link"></a></td>
</tr>
</table>
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix missing and incorrect endpoints in the API permission reference [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck merged PR #74871:
URL: https://github.com/apache/airflow/pull/74871
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org