baha-bouali opened a new pull request, #72230:
URL: https://github.com/apache/airflow/pull/72230
## What
`extract_permissions.py` (added in #67606) already generates the
auth-manager-agnostic
permission reference at `airflow-core/docs/security/api_permissions_ref.rst`.
The FAB provider documents the same endpoints in its own vocabulary —
concrete permission
names such as `Pools.can_read`, plus the minimum built-in role that grants
them — and that
table in `providers/fab/docs/auth-manager/access-control.rst` is still
maintained by hand.
It has drifted, which is what #43430 describes.
This PR generates that table from the same source, and wires it into a prek
hook so it
cannot drift again.
## The drift, measured
The hand-written table has **57 rows**. The API actually exposes **143**.
Comparing them:
- **3 rows state the wrong requirement:**
- `/pools` GET is documented as `Op`, but `VIEWER_PERMISSIONS` contains
`(ACTION_CAN_READ, RESOURCE_POOL)`, so `Viewer` is sufficient.
- `/eventLogs` and `/eventLogs/{event_log_id}` GET omit the base
`DAGs.can_read`
requirement. `requires_access_event_log` delegates to
`requires_access_dag` with
`DagAccessEntity.AUDIT_LOG`, so the base Dag check does run.
- **Stale paths and verbs:** rows still use parameter names and methods that
have since
changed -- `/pools/{pool_name}`, `/roles/{role_name}` (now
`/fab/v1/roles/{name}`), and
`/dags/{dag_id}/clearTaskInstances` documented as `PUT` when the route is
`POST`.
- **Removed endpoints** are still listed, including the old batch endpoints
`/dags/~/dagRuns/list` and `/dags/~/dagRuns/~/taskInstances/list`.
- **Around 90 endpoints are undocumented entirely.**
## How
`scripts/ci/prek/fab_permissions_doc.py` reuses `extract_permissions.py`'s
parser and
renders the FAB view from the same `PermissionEntry` list.
It also parses FAB's own router (`/fab/v1/users`, `/fab/v1/roles`,
`/fab/v1/permissions`).
Those endpoints declare permissions with `requires_fab_custom_view` rather
than the core
`requires_access_*` helpers, so they need separate handling -- but they are
real endpoints
and belong in this table. The router prefix is read from `FAB_AUTH_PREFIX`,
not hardcoded.
Like `extract_permissions.py`, it runs entirely statically — the route
files, FAB's
resource maps and FAB's role definitions are all read with `ast`, so no
Airflow or FAB
import is required and no runtime environment is needed.
It reads FAB's own definitions rather than restating them:
- `_MAP_DAG_ACCESS_ENTITY_TO_FAB_RESOURCE_TYPE` and
`_MAP_ACCESS_VIEW_TO_FAB_RESOURCE_TYPE`
from `fab_auth_manager.py`, for resource expansion.
- `VIEWER_PERMISSIONS` / `USER_PERMISSIONS` / `OP_PERMISSIONS` /
`ADMIN_PERMISSIONS` from
`security_manager/override.py`, for the minimum role. `ROLE_CONFIGS`
stacks these
cumulatively, so the minimum role is the first one whose permissions cover
the
requirement.
- The `RESOURCE_*` display names from `www/security/permissions.py`.
Restating any of those here would create exactly the second source of truth
this is meant
to remove.
## Two details worth flagging for review
- **The action comes from the authorization method, not the HTTP verb.**
They differ:
`/clearTaskInstances` is a `POST` route that authorizes with
`requires_access_dag(method="PUT", ...)`. Using the verb would produce
`can_create`
where the correct answer is `can_edit`.
- **The base Dag check applies to GET as well.** `is_authorized_dag` uses
`"GET" if method == "GET" else "PUT"`, so a sub-entity endpoint requires
`DAGs.can_read` on GET and `DAGs.can_edit` otherwise — it is not skipped
for reads.
## On placement
The generator is placed alongside `extract_permissions.py` in
`scripts/ci/prek/` for
consistency with the other prek scripts, and it reads the FAB definitions
statically rather
than importing them. If you would rather it lived under `providers/fab/` so
ownership sits
with the provider, I am happy to move it — the logic is unchanged either way.
## Changes
- `scripts/ci/prek/fab_permissions_doc.py` — new generator.
- `.pre-commit-config.yaml` — new `generate-fab-permissions-doc` hook,
watching the public
route files, `security.py`, the FAB permission/role definitions, and the
generated table.
- `providers/fab/docs/auth-manager/_api_permissions_table.rst` — generated
output.
- `providers/fab/docs/auth-manager/access-control.rst` — the hand-written
table is replaced
with an `.. include::` of the generated one. The surrounding prose is
unchanged.
## Verification
The check catches a permission change that has not been reflected in the
docs:
```console
$ python scripts/ci/prek/fab_permissions_doc.py --check
[OK] .../providers/fab/docs/auth-manager/_api_permissions_table.rst is up to
date.
$ sed -i '82s/method="GET"/method="POST"/' \
airflow-core/src/airflow/api_fastapi/core_api/routes/public/pools.py
$ git diff --stat
airflow-core/src/airflow/api_fastapi/core_api/routes/public/pools.py
airflow-core/src/airflow/api_fastapi/core_api/routes/public/pools.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
$ python scripts/ci/prek/fab_permissions_doc.py --check
[FAIL] .../providers/fab/docs/auth-manager/_api_permissions_table.rst is
stale. Run: python fab_permissions_doc.py
$ git checkout
airflow-core/src/airflow/api_fastapi/core_api/routes/public/pools.py
$ python scripts/ci/prek/fab_permissions_doc.py --check
[OK] .../providers/fab/docs/auth-manager/_api_permissions_table.rst is up to
date.
```
Before this PR the same change would have gone undocumented, with nothing to
catch it.
## Regenerating
```
prek run generate-fab-permissions-doc --all-files
```
`--check` mode exits 1 when the file on disk is stale, mirroring
`extract_permissions.py`.
Closes #43430
--
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]
