Repository navigation
Let asset watchers write asset state over the Execution API - #76376
Conversation
The attempt-liveness check rejects any mutation whose token subject is not a live task attempt or callback. Asset watchers have no task instance: the asset state store routes identify them by the null UUID and record their writes as WATCHER. In-process callers skip the check, so the built-in triggerer was unaffected, but a triggerer that reaches the Execution API over HTTP (as TriggererJobRunner.make_client allows) got a 404 on every asset state write. Accept the watcher identity on the asset state store mutation routes only; it is still rejected everywhere else. The watcher tests now go through the real require_auth instead of replacing it, which is what hid this.
amoghrajesh
left a comment
amoghrajesh
left a comment
There was a problem hiding this comment.
LGTM with one nit / race condition on merge
pierrejeambrun
left a comment
pierrejeambrun
left a comment
There was a problem hiding this comment.
LGTM once Amogh comments are addressed.
amoghrajesh
left a comment
amoghrajesh
left a comment
There was a problem hiding this comment.
@amoghrajesh handled his own comments :)
Nice I was about to do the same since Jed will wake up in a bit and I believe 3.4.0rc3 is waiting for this. |
allow_non_ti_writer and allow_asset_watcher now record which tokens may skip require_auth's liveness check, instead of each adding its own flag and condition. Routes are unchanged.
Backport failed to create: v3-4-test. View the failure log Run detailsNote: As of Merging PRs targeted for Airflow 3.X In matter of doubt please ask in #release-management Slack channel.
You can attempt to backport this manually by running: cherry_picker 4ef770b v3-4-testThis should apply the commit to the v3-4-test branch and leave the commit in conflict state marking After you have resolved the conflicts, you can continue the backport process by running: cherry_picker --continueIf you don't have cherry-picker installed, see the installation guide. |
Co-authored-by: Amogh Desai <amoghrajesh1999@gmail.com> Co-authored-by: Tzu-ping Chung <tp@astronomer.io> (cherry picked from commit 4ef770b)
Co-authored-by: Amogh Desai <amoghrajesh1999@gmail.com> Co-authored-by: Tzu-ping Chung <tp@astronomer.io> (cherry picked from commit 4ef770b)
The attempt-liveness check rejects any Execution API mutation whose token subject isn't a live task attempt or callback. Asset watchers have no task instance: the asset state store routes identify them by the null UUID (
ASSET_WATCHER_ID) and record their writes asWATCHER. In-process callers skip the check, so the built-in triggerer is unaffected, but a triggerer that reaches the Execution API over HTTP (asTriggererJobRunner.make_clientallows) gets a 404 on every asset state write.This lets the watcher identity skip the check on the asset state store mutation routes. Other tokens without a live attempt are still rejected there, and the watcher identity is still rejected on routes that track attempts, such as XCom. Variable writes accept any caller since #76375.
allow_asset_watcherand #76375'sallow_non_ti_writernow share one mechanism: each route records which tokens may skip the check.The existing watcher tests replaced
require_authentirely, which hid this. They now go through the realrequire_auth.