Add IMPORT_ERRORS_ALL permission for import errors of files with no registered Dag#69790
Open
potiuk wants to merge 12 commits into
Open
Add IMPORT_ERRORS_ALL permission for import errors of files with no registered Dag#69790potiuk wants to merge 12 commits into
potiuk wants to merge 12 commits into
Conversation
potiuk
requested review from
bugraoz93,
choo121600,
ephraimbuddy,
henry3260,
jason810496,
pierrejeambrun,
rawwar,
shubhamraj-git and
vincbeck
as code owners
July 12, 2026 23:56
boring-cyborg
Bot
added
area:API
Member
Author
|
@vincbeck — you'd offered to pick up #67461, so heads-up that I went ahead and implemented it here. Would you mind reviewing when you get a chance? Particularly keen on your read of the |
henry3260
reviewed
Jul 13, 2026
henry3260
left a comment
henry3260
left a comment
Contributor
There was a problem hiding this comment.
I think we should check bundle_name as well here — otherwise a same-named file in another bundle that does have Dags will match, and this unregistered file skips the IMPORT_ERRORS_ALL check.
Contributor
Maybe we can add a regression test here :) |
vincbeck
reviewed
Jul 13, 2026
…egistered Dag The Import Errors API authorizes each error per Dag defined in its file. Files with no registered Dag have no per-Dag key to authorize on. Gate the raw stack trace for those files on a dedicated, admin-by-default IMPORT_ERRORS_ALL view, scoped per team via the file's bundle where the auth manager supports multi-team isolation, and redact it for callers without the permission on both the list and single endpoints. Closes: apache#67461 Generated-by: Claude Opus 4.8 (1M context) following the guidelines at https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions
… view The provider is released independently of core and may run against an older Airflow that predates the AccessView.IMPORT_ERRORS_ALL member (added in 3.4.0). Referencing it directly in the resource map raised AttributeError at import time under the provider compatibility matrix. Resolve the member through a common.compat shim and map it only when the running core defines it.
…lease The IMPORT_ERRORS_ALL access-view shim is added to the common.compat provider in this same PR, so fab and keycloak require the next common.compat release. The selective-check common-compat guard enforces the '# use next version' marker on those dependents; add it so the build-info check passes and the compatibility tests run.
The same relative_fileloc can exist in more than one bundle, so matching only on the filename treated an import error whose file has no Dag in its own bundle as registered when another bundle happened to define a Dag for the same path. Join the has-any-Dag subquery on both filename and bundle_name so unregistered-file import errors are detected per bundle.
The shim module was added without a matching test, which the provider project-structure check flags. Cover both branches: it resolves to the enum member on a core that defines it, and to None on an older core that does not.
The list endpoint joins the has-any-Dag subquery twice; only the first was scoped to the bundle. Since the subquery now yields one row per (file, bundle), the remaining filename-only join could duplicate an error across bundles and mis-detect registration. Join on bundle_name there too, add a regression test for a file registered only in another bundle, and update the existing bundle -join test, whose old expectation encoded the pre-fix cross-bundle leak.
The provider compatibility matrix runs this test against older cores that predate AccessView.IMPORT_ERRORS_ALL (or lack api_fastapi entirely), where referencing the member directly raised AttributeError. Compare the shim against whatever the running core exposes (the member, or None) so it holds everywhere.
…point The has-any-Dag lookup in get_import_error is scoped per bundle. Without that scoping a same-named file registered in a different bundle resolved as a Dag match, so the endpoint returned the raw stacktrace on the strength of an unrelated bundle's read permission instead of gating on IMPORT_ERRORS_ALL. The list endpoint already had this coverage.
potiuk
force-pushed
the
import-errors-all-permission-unregistered-files
branch
from
July 20, 2026 13:47
56d620e to
7f0cf5d
Compare
Per review, add the optional team_name to is_authorized_view directly rather than introducing a parallel is_authorized_view_for_team. Duplicating every authorization method for its team-aware variant would bloat the auth manager interface; the other is_authorized_* methods already carry team scoping through their details object, and this keeps is_authorized_view consistent with them. This is a breaking change for out-of-tree auth managers: an override that keeps the old (access_view, user) signature imports cleanly but raises TypeError at runtime the first time core authorizes a team-scoped view. Documented in a significant newsfragment with the one-line fix.
…_view Folding team_name into is_authorized_view made it a hard breaking change: an out-of-tree auth manager, or a provider released before the argument existed, keeps the old (access_view, user) signature and would raise TypeError the first time core authorized a team-scoped view -- a runtime 500 with no import- or startup-time signal. Route team-scoped authorization through a new BaseAuthManager.authorize_view, which detects (once, cached) whether the manager's override accepts team_name. A legacy manager is called without it and keeps working; team scoping is ignored so the view is authorized globally -- exactly what a manager with no multi-team support does anyway -- and a RemovedInAirflow4Warning tells the operator that some views (e.g. import errors for files with no registered Dag) are visible across teams until the auth manager is upgraded. The public interface stays a single is_authorized_view; the fallback is removed in Airflow 4.
This PR changes the common.compat provider alongside the amazon, fab and keycloak providers. When common.compat changes with other providers in the same PR, each provider depending on it must carry a '# use next version' comment so the release manager bumps the constraint to the not-yet-released common.compat at release time. fab and keycloak already had it; amazon was pulled into the changed set by the auth manager update and was missing it, which the selective-checks Build info job flagged.
The team_name argument no longer forces a hard break — auth managers that predate it keep working with a deprecation warning — so it is a feature rather than a significant/breaking change. Replace the significant newsfragment with a single feature note that states when team_name became available (Airflow 3.4.0) and which bundled auth managers support it, and move the implementation detail (the authorize_view fallback, the deprecation, how to make a manager team-aware) into the auth manager documentation where implementers will look for it.
vincbeck
reviewed
Jul 21, 2026
vincbeck
approved these changes
Jul 21, 2026
bugraoz93
approved these changes
Jul 21, 2026
pierrejeambrun
approved these changes
Jul 24, 2026
pierrejeambrun
left a comment
pierrejeambrun
left a comment
Member
There was a problem hiding this comment.
LGTM. I would trim down some of the very verbose comments. But non blocking.
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
5 participants
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.
Import errors are authorized per Dag defined in the file. Files with no registered Dag (parse failed before any Dag was defined, or all Dags removed) have no per-Dag key to authorize on. This adds a dedicated
IMPORT_ERRORS_ALLview -- admin-granted by default, scoped per team via the file's bundle where the auth manager supports multi-team isolation -- and gates such files on it: callers without the permission are denied those import errors (403on the single endpoint; excluded from the list endpoint) instead of receiving the raw stack trace.Implements #67461.
AccessView.IMPORT_ERRORS_ALL.is_authorized_view_for_teammethod that defaults tois_authorized_view, so auth managers that do not implement it (including out-of-tree ones) keep working unchanged. The simple and Keycloak managers override it to enforce team scoping; the FAB manager grants the new permission toAdminby default.total_entriesso their existence does not surface.Tests
test_import_error.py-- with the permission the raw stack trace is returned; without it the single endpoint returns403and the list endpoint omits the rows; team scoping is honored; no query-count regression.test_base_auth_manager.py--is_authorized_view_for_teamfalls back tois_authorized_viewby default.Was generative AI tooling used to co-author this PR?
Generated-by: Claude Opus 4.8 (1M context) following the guidelines at
https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions