Skip to content

Apply impersonation_chain to deferred BigQuery existence checks - #71648

Merged
potiuk merged 2 commits into
apache:mainfrom
SEPURI-SAI-KRISHNA:fix-bigquery-sensor-impersonation
Oct 5, 2026
Merged

potiuk merged 2 commits into
apache:mainfrom
SEPURI-SAI-KRISHNA:fix-bigquery-sensor-impersonation

Conversation

@SEPURI-SAI-KRISHNA

Copy link
Copy Markdown
Contributor

BigQueryTableExistenceSensor and BigQueryTablePartitionExistenceSensor hand their
impersonation chain to the trigger inside hook_params:

trigger=BigQueryTableExistenceTrigger(
    ...,
    hook_params={"impersonation_chain": self.impersonation_chain},
)

but the trigger authenticates from a different attribute entirely:

def _get_async_hook(self) -> BigQueryTableAsyncHook:
    return BigQueryTableAsyncHook(
        gcp_conn_id=self.gcp_conn_id, impersonation_chain=self.impersonation_chain
    )

Nothing ever sets self.impersonation_chain from those call sites, so it stays None and
the deferred existence check runs as the connection's service account. hook_params is
stored and serialized but never read anywhere in the module — it carries the value and
then drops it.

poke() builds its hook correctly, so the sensor honours impersonation when
deferrable=False and ignores it when deferrable=True. That divergence is the awkward
part: turning deferrable off appears to "fix" the permissions error, which points
investigation away from the real cause.

The trigger already grew a proper impersonation_chain parameter in #36341; the sensors
were simply never moved onto it. This change passes it explicitly at both defer sites.

hook_params is left untouched here. It is a required argument on the trigger's public
__init__, and now that nothing reads it, removing or deprecating it is a separate
decision — happy to follow up if maintainers would like it gone.

For reviewers: GoogleBaseHook.__init__ falls back to the connection's
impersonation_chain extra when the argument is absent, so deployments that configure
impersonation on the connection were never affected. This only bites when impersonation is
set on the sensor, which is the documented parameter.

No newsfragment: this is a provider change, and provider changelogs are regenerated from
git log by the release manager.


Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Opus 5)

Generated-by: Claude Code (Opus 5) following the guidelines

@boring-cyborg boring-cyborg Bot added area:providers provider:google Google (including GCP) related issues labels Aug 15, 2026
Comment thread providers/google/tests/unit/google/cloud/sensors/test_bigquery.py Outdated
@SEPURI-SAI-KRISHNA

Copy link
Copy Markdown
Contributor Author

Thanks @SameerMesiah97, moved.

The standalone parametrized test is gone. Each sensor now has its own test_deferred_trigger_receives_impersonation_chain sitting next to that class's existing deferral test — in TestBigqueryTableExistenceSensor and in TestBigqueryTablePartitionExistenceSensor.

I dropped the isinstance(exc.value.trigger, ...) assertion during the move: the sibling test_execute_deferred / test_execute_with_deferrable_mode in each class already covers the trigger type, so the new tests assert only the argument that was being lost.

Verified locally: 25 passed with the change, and exactly the 2 new tests fail with it reverted (23 pre-existing tests unaffected either way).


Drafted-by: Claude Code (Opus 5); reviewed by @SEPURI-SAI-KRISHNA before posting

@SEPURI-SAI-KRISHNA

Copy link
Copy Markdown
Contributor Author

Gentle ping, the review feedback here was addressed on 18 August and CI has been green since. Anything else you'd like changed before this can go in?

The sensors handed the impersonation chain to the trigger inside hook_params,
which nothing reads, while the trigger authenticated from an attribute those
call sites never set. A deferred existence check therefore ran as the
connection's service account, and only in deferrable mode, so disabling
deferral appeared to fix the resulting permission error.
A reviewer asked for the coverage to live with each sensor's own tests rather than in a standalone parametrized test, so the file keeps one place to look per sensor.
@potiuk
potiuk force-pushed the fix-bigquery-sensor-impersonation branch from 19fe69f to e800da9 Compare October 5, 2026 14:25

@potiuk potiuk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving. Deferred table and partition existence checks now run with the configured impersonation_chain instead of the triggerer's default credentials; the triggers already supported it but never received it. Thanks!


Drafted-by: Claude Code (Opus 5.5); reviewed by @potiuk before posting

@potiuk
potiuk merged commit 00bc444 into apache:main Oct 5, 2026
87 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providers provider:google Google (including GCP) related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants