Let an auth manager filter dags in SQL instead of materializing every id - #71341
Open
1fanwang wants to merge 1 commit into
Open
Let an auth manager filter dags in SQL instead of materializing every id#713411fanwang wants to merge 1 commit into
1fanwang wants to merge 1 commit into
Conversation
1fanwang
requested review from
bugraoz93,
choo121600,
ephraimbuddy,
henry3260,
jason810496,
pierrejeambrun,
rawwar,
shubhamraj-git and
vincbeck
as code owners
August 9, 2026 06:19
1fanwang
force-pushed
the
stewang/authorized-dag-ids-select
branch
3 times, most recently
from
August 9, 2026 08:37
cb4476c to
6c06640
Compare
get_authorized_dag_ids returns a set, so every authorized dag id is loaded into memory before pagination is applied. FabAuthManager keeps its grants in the metadata database and still does this: a user authorized on all dags gets select(DagModel.dag_id) materialized on every list request. get_authorized_dag_ids_select lets a manager return a select instead, which the permitted-dag filters apply as a subquery. Returning None, the default, keeps the existing behaviour, and every permitted-* filter inherits it because they all build the clause with in_(). Signed-off-by: 1fanwang <1fannnw@gmail.com>
1fanwang
force-pushed
the
stewang/authorized-dag-ids-select
branch
from
August 9, 2026 09:26
6c06640 to
fe5c60f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes: #71309
Why
get_authorized_dag_idsis where every list endpoint starts, and its default implementation readsthe whole dag table before any manager is consulted:
It then groups by team and calls
filter_authorized_dag_idsonce per team. So a request for 50dags loads every dag id in the deployment, and the cost scales with the deployment rather than the
page. That is core's own path, not one provider's:
FabAuthManageroverrides it and still readsevery row, and Keycloak inherits it and then fires a call per dag, which is what
#69041 and
#61686 report as ten and twenty-five second
pages.
The docs point managers at Dag tags and bundles for attribute-based access control, and
get_db_managerlets a manager add tables of its own. Tags, bundles and teams are all rows inthis database. The
set[str]return type is what stops any of them being answered as a query.What changed
BaseAuthManagergainsget_authorized_dag_ids_select, returning a select of dag ids orNone.Noneis the default and keeps today's behaviour, so managers backed by an external policy service(Keycloak, Amazon Verified Permissions) are unaffected.
A returned select is applied as
dag_id IN (subquery). Endpoints that need the ids in memory, thedependency graph services and the run-state counts endpoint, still get them:
PermittedDagFiltermaterializes on first read, so nothing is loaded unless something asks.
A select replaces
get_authorized_dag_idsoutright, including its per-team grouping, so amulti-team manager scopes the select itself or returns
Noneand keeps the fan-out.FabAuthManagerimplements it with the grant query it already knows how to write.Testing Done
Measured, no FAB. A tag-based manager, which is what the docs recommend. MySQL 8 with 41,606
dags, each carrying an environment tag and a team tag. Page of 50, median of 15 rounds, client
inside the cluster.
Measured, FAB. Same database at 41,606 dags, 1,610 roles, 66,529 per-dag edit grants.
The expensive row in both is the ordinary one: a tag most dags carry, or a role with a global
can_read, which is Viewer and up.Red. A select has no truth value, so a filter falling back with
orraises. With thatfallback restored:
Green.
The eight cover the
Nonedefault, a select reaching the SQL as a subquery, an empty selecthonoured as "nothing is permitted" rather than collapsing to no filter, a subclass filter
inheriting it, a tag-based select needing no FAB, a team-scoped select staying one statement, and the set
materializing once and only when read.
Ruff and mypy are clean over the six modules the change touches.