You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by vi...@apache.org on 2026/10/19 18:16:07 UTC
(airflow) branch backport-a26f9f9-v3-4-test updated (fecc77f1445 -> 5e8d4c6de02)
This is an automated email from the ASF dual-hosted git repository.
vincbeck pushed a change to branch backport-a26f9f9-v3-4-test
in repository https://gitbox.apache.org/repos/asf/airflow.git
omit fecc77f1445 [v3-4-test] Fix missing and incorrect endpoints in the API permission reference (#74871) (cherry picked from commit a26f9f9926c29d9126fc5ca5f33b4e724c8bc509)
omit 2c6493b7636 [v3-4-test] Render XCom values on the API server without rebuilding objects (#74829) (#75088)
omit a658a2458dd [v3-4-test] Bump the github-actions-updates group with 10 updates (#74880) (#75021)
new 5e8d4c6de02 [v3-4-test] Fix missing and incorrect endpoints in the API permission reference (#74871) (cherry picked from commit a26f9f9926c29d9126fc5ca5f33b4e724c8bc509)
This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version. This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:
* -- * -- B -- O -- O -- O (fecc77f1445)
\
N -- N -- N refs/heads/backport-a26f9f9-v3-4-test (5e8d4c6de02)
You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.
Any revisions marked "omit" are not gone; other references still
refer to them. Any revisions marked "discard" are gone forever.
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.github/workflows/airflow-e2e-tests.yml | 4 +-
.github/workflows/asf-allowlist-check.yml | 2 +-
.github/workflows/basic-tests.yml | 2 +-
.github/workflows/ci-amd.yml | 12 +--
.github/workflows/ci-arm.yml | 12 +--
.github/workflows/ci-duration-monitor.yml | 2 +-
.github/workflows/ci-image-build.yml | 16 ++--
.github/workflows/ci-image-checks.yml | 10 +--
.github/workflows/ci-notification.yml | 4 +-
.github/workflows/codeql-analysis.yml | 6 +-
.github/workflows/e2e-flaky-tests-report.yml | 2 +-
.github/workflows/finalize-tests.yml | 2 +-
.github/workflows/generate-constraints.yml | 2 +-
.github/workflows/helm-tests.yml | 2 +-
.github/workflows/k8s-tests.yml | 6 +-
.github/workflows/notify-uv-lock-conflicts.yml | 2 +-
.github/workflows/openlineage-e2e-compat-tests.yml | 2 +-
.github/workflows/prod-image-build.yml | 8 +-
.github/workflows/publish-docs-to-s3.yml | 20 ++---
.github/workflows/push-image-cache.yml | 2 +-
.github/workflows/registry-backfill.yml | 2 +-
.github/workflows/registry-build.yml | 4 +-
.github/workflows/registry-tests.yml | 2 +-
.github/workflows/release-constraints.yml | 2 +-
.../workflows/release_single_dockerhub_image.yml | 4 +-
.../scheduled-verify-release-calendar.yml | 2 +-
.github/workflows/ts-sdk-release.yml | 8 +-
.github/workflows/ui-e2e-tests.yml | 6 +-
.../update-constraints-on-push-stable.yml | 2 +-
.github/workflows/update-constraints-on-push.yml | 2 +-
.../api_fastapi/core_api/routes/public/xcom.py | 23 ++++--
.../serialization/definitions/operatorlink.py | 14 +++-
.../src/airflow/serialization/stringify.py | 82 ++++---------------
.../core_api/routes/public/test_xcom.py | 65 +---------------
.../tests/unit/serialization/test_operatorlink.py | 21 -----
.../tests/unit/serialization/test_stringify.py | 91 ++--------------------
scripts/ci/prek/fab_permissions_doc.py | 13 +---
37 files changed, 130 insertions(+), 331 deletions(-)
(airflow) 01/01: [v3-4-test] Fix missing and incorrect endpoints in the API permission reference (#74871) (cherry picked from commit a26f9f9926c29d9126fc5ca5f33b4e724c8bc509)
Posted by vi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
vincbeck pushed a commit to branch backport-a26f9f9-v3-4-test
in repository https://gitbox.apache.org/repos/asf/airflow.git
commit 5e8d4c6de0236e3e30d373c3ad9bb3619ec69852
Author: Pushkal Gupta <16...@users.noreply.github.com>
AuthorDate: Mon Oct 19 19:30:59 2026 +0530
[v3-4-test] Fix missing and incorrect endpoints in the API permission reference (#74871)
(cherry picked from commit a26f9f9926c29d9126fc5ca5f33b4e724c8bc509)
Co-authored-by: Pushkal Gupta <16...@users.noreply.github.com>
---
airflow-core/docs/security/api_permissions_ref.rst | 54 +++++---
scripts/ci/prek/extract_permissions.py | 35 ++++-
scripts/tests/ci/prek/test_extract_permissions.py | 148 +++++++++++++++++++++
3 files changed, 216 insertions(+), 21 deletions(-)
diff --git a/airflow-core/docs/security/api_permissions_ref.rst b/airflow-core/docs/security/api_permissions_ref.rst
index cf5e188ff15..9d47f6bba3b 100644
--- a/airflow-core/docs/security/api_permissions_ref.rst
+++ b/airflow-core/docs/security/api_permissions_ref.rst
@@ -99,15 +99,15 @@ source code so it stays up to date as endpoints are added or changed.
- ``Asset``
- ``GET``
* - ``DELETE``
- - ``/api/v2/assets/{asset_id}/state-store/{key:path}``
+ - ``/api/v2/assets/{asset_id}/state-store/{key}``
- ``Asset``
- ``DELETE``
* - ``GET``
- - ``/api/v2/assets/{asset_id}/state-store/{key:path}``
+ - ``/api/v2/assets/{asset_id}/state-store/{key}``
- ``Asset``
- ``GET``
* - ``PUT``
- - ``/api/v2/assets/{asset_id}/state-store/{key:path}``
+ - ``/api/v2/assets/{asset_id}/state-store/{key}``
- ``Asset``
- ``PUT``
* - ``GET``
@@ -126,8 +126,28 @@ source code so it stays up to date as endpoints are added or changed.
- ``/api/v2/backfills``
- ``DAG.RUN``
- ``POST``
+ * - ``POST``
+ - ``/api/v2/backfills/dry_run``
+ - ``DAG.RUN``
+ - ``POST``
+ * - ``GET``
+ - ``/api/v2/backfills/{backfill_id}``
+ - ``DAG.RUN``
+ - ``GET``
* - ``PUT``
- - ``/api/v2/backfills``
+ - ``/api/v2/backfills/{backfill_id}/cancel``
+ - ``DAG.RUN``
+ - ``PUT``
+ * - ``GET``
+ - ``/api/v2/backfills/{backfill_id}/dag_runs``
+ - ``DAG.RUN``
+ - ``GET``
+ * - ``PUT``
+ - ``/api/v2/backfills/{backfill_id}/pause``
+ - ``DAG.RUN``
+ - ``PUT``
+ * - ``PUT``
+ - ``/api/v2/backfills/{backfill_id}/unpause``
- ``DAG.RUN``
- ``PUT``
* - ``GET``
@@ -375,19 +395,19 @@ source code so it stays up to date as endpoints are added or changed.
- ``DAG.TASK_INSTANCE``
- ``GET``
* - ``DELETE``
- - ``/api/v2/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/state-store/{key:path}``
+ - ``/api/v2/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/state-store/{key}``
- ``DAG.TASK_INSTANCE``
- ``DELETE``
* - ``GET``
- - ``/api/v2/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/state-store/{key:path}``
+ - ``/api/v2/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/state-store/{key}``
- ``DAG.TASK_INSTANCE``
- ``GET``
* - ``PATCH``
- - ``/api/v2/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/state-store/{key:path}``
+ - ``/api/v2/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/state-store/{key}``
- ``DAG.TASK_INSTANCE``
- ``PUT``
* - ``PUT``
- - ``/api/v2/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/state-store/{key:path}``
+ - ``/api/v2/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/state-store/{key}``
- ``DAG.TASK_INSTANCE``
- ``PUT``
* - ``GET``
@@ -407,15 +427,15 @@ source code so it stays up to date as endpoints are added or changed.
- ``DAG.XCOM``
- ``POST``
* - ``DELETE``
- - ``/api/v2/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/xcomEntries/{xcom_key:path}``
+ - ``/api/v2/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/xcomEntries/{xcom_key}``
- ``DAG.XCOM``
- ``DELETE``
* - ``GET``
- - ``/api/v2/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/xcomEntries/{xcom_key:path}``
+ - ``/api/v2/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/xcomEntries/{xcom_key}``
- ``DAG.XCOM``
- ``GET``
* - ``PATCH``
- - ``/api/v2/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/xcomEntries/{xcom_key:path}``
+ - ``/api/v2/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/xcomEntries/{xcom_key}``
- ``DAG.XCOM``
- ``PUT``
* - ``GET``
@@ -547,15 +567,15 @@ source code so it stays up to date as endpoints are added or changed.
- ``Pool``
- ``POST``
* - ``DELETE``
- - ``/api/v2/pools/{pool_name:path}``
+ - ``/api/v2/pools/{pool_name}``
- ``Pool``
- ``DELETE``
* - ``GET``
- - ``/api/v2/pools/{pool_name:path}``
+ - ``/api/v2/pools/{pool_name}``
- ``Pool``
- ``GET``
* - ``PATCH``
- - ``/api/v2/pools/{pool_name:path}``
+ - ``/api/v2/pools/{pool_name}``
- ``Pool``
- ``PUT``
* - ``GET``
@@ -575,15 +595,15 @@ source code so it stays up to date as endpoints are added or changed.
- ``Variable``
- ``POST``
* - ``DELETE``
- - ``/api/v2/variables/{variable_key:path}``
+ - ``/api/v2/variables/{variable_key}``
- ``Variable``
- ``DELETE``
* - ``GET``
- - ``/api/v2/variables/{variable_key:path}``
+ - ``/api/v2/variables/{variable_key}``
- ``Variable``
- ``GET``
* - ``PATCH``
- - ``/api/v2/variables/{variable_key:path}``
+ - ``/api/v2/variables/{variable_key}``
- ``Variable``
- ``PUT``
* - ``GET``
diff --git a/scripts/ci/prek/extract_permissions.py b/scripts/ci/prek/extract_permissions.py
index b3f86e55a07..f2a14ab5879 100644
--- a/scripts/ci/prek/extract_permissions.py
+++ b/scripts/ci/prek/extract_permissions.py
@@ -33,6 +33,7 @@ from __future__ import annotations
import ast
import pathlib
+import re
import sys
from dataclasses import dataclass
@@ -95,6 +96,34 @@ def _resolve_string_node(node: ast.expr, module_consts: dict[str, str]) -> str:
return f"<unresolved:{ast.unparse(node)}>"
+def strip_path_converters(path: str) -> str:
+ """
+ Drop Starlette path converters from a route path.
+
+ A route declared as ``/{pool_name:path}`` is served, and documented in the
+ OpenAPI spec, as ``/{pool_name}``. The converter only tells Starlette how to
+ match the segment, so leaving it in prints a URL that no client ever calls.
+ """
+ return re.sub(r"\{([^{}:]+):[^{}]+\}", r"{\1}", path)
+
+
+def _extract_route_path(decorator: ast.Call, module_consts: dict[str, str]) -> str:
+ """
+ Resolve the route path from a route decorator.
+
+ FastAPI accepts the path either positionally or as ``path=``, and both forms
+ are in use here. Reading only the positional form silently resolves the whole
+ file to its router prefix, which collapses every route onto one bogus row
+ instead of failing.
+ """
+ if decorator.args:
+ return _resolve_string_node(decorator.args[0], module_consts)
+ path_kwarg = next((kw for kw in decorator.keywords if kw.arg == "path"), None)
+ if path_kwarg is not None:
+ return _resolve_string_node(path_kwarg.value, module_consts)
+ raise RouteParseError(f"Route decorator has no path: {ast.unparse(decorator)}")
+
+
def _extract_module_string_constants(tree: ast.Module) -> dict[str, str]:
"""
Walk top-level assignments and collect simple string assignments.
@@ -313,9 +342,7 @@ def extract_from_file(path: pathlib.Path) -> list[PermissionEntry]:
continue
# Resolve the route path
- route_suffix = ""
- if decorator.args:
- route_suffix = _resolve_string_node(decorator.args[0], module_consts)
+ route_suffix = _extract_route_path(decorator, module_consts)
# Resolve the prefix based on the router variable used in the decorator
router_prefix = ""
@@ -323,7 +350,7 @@ def extract_from_file(path: pathlib.Path) -> list[PermissionEntry]:
router_var = decorator.func.value.id
router_prefix = routers.get(router_var, "")
- full_path = API_PREFIX + router_prefix + route_suffix
+ full_path = strip_path_converters(API_PREFIX + router_prefix + route_suffix)
# Extract tag (for grouping in the RST table)
tag = _extract_tag_from_decorator(decorator)
diff --git a/scripts/tests/ci/prek/test_extract_permissions.py b/scripts/tests/ci/prek/test_extract_permissions.py
index 269f2989877..0ffef7b1bb6 100644
--- a/scripts/tests/ci/prek/test_extract_permissions.py
+++ b/scripts/tests/ci/prek/test_extract_permissions.py
@@ -32,6 +32,7 @@ Run with (no Airflow env needed — extractor is stdlib-only):
from __future__ import annotations
import ast
+import re
import sys
import textwrap
from pathlib import Path
@@ -54,11 +55,13 @@ from extract_permissions import ( # noqa: E402
_build_resource_label,
_extract_method_arg,
_extract_module_string_constants,
+ _extract_route_path,
_extract_routers,
_resolve_string_node,
extract_all_permissions,
extract_from_file,
render_rst,
+ strip_path_converters,
)
PUBLIC_ROUTES_DIR = REPO_ROOT / "airflow-core/src/airflow/api_fastapi/core_api/routes/public"
@@ -293,6 +296,59 @@ class TestBuildResourceLabel:
assert _build_resource_label("requires_access_new_thing", None) == "requires_access_new_thing"
+# ===========================================================================
+# Unit tests: strip_path_converters
+# ===========================================================================
+
+
+class TestStripPathConverters:
+ @pytest.mark.parametrize(
+ "declared, served",
+ [
+ ("/api/v2/pools/{pool_name:path}", "/api/v2/pools/{pool_name}"),
+ ("/api/v2/variables/{variable_key:path}", "/api/v2/variables/{variable_key}"),
+ # more than one parameter, only the converted one changes
+ (
+ "/api/v2/assets/{asset_id}/state-store/{key:path}",
+ "/api/v2/assets/{asset_id}/state-store/{key}",
+ ),
+ # converters other than :path are normalised the same way
+ ("/api/v2/dags/{dag_id:str}/runs/{n:int}", "/api/v2/dags/{dag_id}/runs/{n}"),
+ # nothing to do
+ ("/api/v2/dags/{dag_id}", "/api/v2/dags/{dag_id}"),
+ ("/api/v2/backfills", "/api/v2/backfills"),
+ ],
+ )
+ def test_converter_is_dropped(self, declared, served):
+ assert strip_path_converters(declared) == served
+
+
+# ===========================================================================
+# Unit tests: _extract_route_path
+# ===========================================================================
+
+
+class TestExtractRoutePath:
+ def test_positional_path(self):
+ deco = parse_expr('router.get("/{dag_id}", dependencies=[])')
+ assert _extract_route_path(deco, {}) == "/{dag_id}"
+
+ def test_keyword_path(self):
+ """``backfills.py`` passes the path as ``path=``, which must resolve the same."""
+ deco = parse_expr('router.get(path="/{backfill_id}/pause", dependencies=[])')
+ assert _extract_route_path(deco, {}) == "/{backfill_id}/pause"
+
+ def test_keyword_path_resolves_module_constants(self):
+ deco = parse_expr('router.get(path=my_prefix + "/{id}")')
+ assert _extract_route_path(deco, {"my_prefix": "/things"}) == "/things/{id}"
+
+ def test_no_path_at_all_raises(self):
+ """Falling back to an empty suffix would collapse the route onto its router prefix."""
+ deco = parse_expr("router.get(dependencies=[])")
+ with pytest.raises(RouteParseError):
+ _extract_route_path(deco, {})
+
+
# ===========================================================================
# Unit tests: extract_from_file (synthetic route files)
# ===========================================================================
@@ -538,6 +594,57 @@ class TestExtractFromFile:
assert len(entries) == 1
assert entries[0].resource == "DAG"
+ def test_keyword_path_is_not_collapsed_onto_the_router_prefix(self, tmp_path):
+ """
+ Mirrors ``backfills.py``, which declares every route as ``path=``.
+
+ Reading only the positional argument left each of these resolving to the
+ bare router prefix, so eight endpoints became three wrong rows.
+ """
+ f = _make_route_file(
+ tmp_path,
+ """
+ from fastapi import Depends
+ backfills_router = AirflowRouter(tags=["Backfill"], prefix="/backfills")
+
+ @backfills_router.get(
+ path="/{backfill_id}",
+ dependencies=[Depends(requires_access_backfill(method="GET"))],
+ )
+ def get_backfill(backfill_id: int): ...
+
+ @backfills_router.put(
+ path="/{backfill_id}/pause",
+ dependencies=[Depends(requires_access_backfill(method="PUT"))],
+ )
+ def pause_backfill(backfill_id: int): ...
+ """,
+ )
+ entries = extract_from_file(f)
+ assert {(e.http_method, e.full_path) for e in entries} == {
+ ("GET", "/api/v2/backfills/{backfill_id}"),
+ ("PUT", "/api/v2/backfills/{backfill_id}/pause"),
+ }
+
+ def test_path_converter_is_stripped_from_the_documented_path(self, tmp_path):
+ """The served URL is ``/pools/{pool_name}``; the converter is matching syntax."""
+ f = _make_route_file(
+ tmp_path,
+ """
+ from fastapi import Depends
+ pools_router = AirflowRouter(tags=["Pool"], prefix="/pools")
+
+ @pools_router.get(
+ "/{pool_name:path}",
+ dependencies=[Depends(requires_access_pool(method="GET"))],
+ )
+ def get_pool(pool_name: str): ...
+ """,
+ )
+ entries = extract_from_file(f)
+ assert len(entries) == 1
+ assert entries[0].full_path == "/api/v2/pools/{pool_name}"
+
def test_syntax_error_raises_instead_of_dropping_entries(self, tmp_path):
f = tmp_path / "broken.py"
f.write_text("def broken(:\n")
@@ -647,6 +754,47 @@ class TestExtractAllPermissions:
"""
assert len(all_entries) >= 100, f"Expected ≥100 entries, got {len(all_entries)}"
+ def test_every_declared_route_reaches_the_table(self, all_entries):
+ """
+ Each route decorator in the source must produce its own row.
+
+ This is the invariant that the ``path=`` bug broke: the eight routes in
+ ``backfills.py`` all resolved to ``/api/v2/backfills`` and deduplicated
+ down to three, without any warning.
+ """
+ verbs = {"GET", "POST", "PATCH", "PUT", "DELETE", "HEAD"}
+ declared: dict[str, int] = {}
+ for route_file in sorted(PUBLIC_ROUTES_DIR.glob("*.py")):
+ if route_file.name == "__init__.py":
+ continue
+ count = 0
+ for node in ast.walk(ast.parse(route_file.read_text(encoding="utf-8"))):
+ if not isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)):
+ continue
+ count += sum(
+ 1
+ for deco in node.decorator_list
+ if isinstance(deco, ast.Call)
+ and isinstance(deco.func, ast.Attribute)
+ and deco.func.attr.upper() in verbs
+ )
+ declared[route_file.name] = count
+
+ extracted: dict[str, set[tuple[str, str]]] = {}
+ for entry in all_entries:
+ extracted.setdefault(entry.source_file, set()).add((entry.full_path, entry.http_method))
+
+ for name, count in declared.items():
+ assert len(extracted.get(name, set())) == count, (
+ f"{name} declares {count} routes but the table holds "
+ f"{len(extracted.get(name, set()))} distinct operations"
+ )
+
+ def test_no_documented_path_contains_a_converter(self, all_entries):
+ """``{key:path}`` is Starlette matching syntax, not part of the URL clients call."""
+ offenders = [e.full_path for e in all_entries if re.search(r"\{[^{}]*:", e.full_path)]
+ assert offenders == []
+
def test_output_is_sorted(self, all_entries):
expected = sorted(
all_entries,