Fix Google IdP credentials failing when project id cannot be resolved - #72799
Conversation
Signed-off-by: PoAn Yang <payang@apache.org>
e9d5c87 to
c85a29c
Compare
potiuk
left a comment
There was a problem hiding this comment.
Verified small fix: the IdP branch now returns an empty project id (like the ADC branch after #61654) so GoogleBaseHook can apply the connection's project extra, with a regression test that fails on main.
I checked the claims against the code: GoogleBaseHook.get_credentials_and_project_id overrides project_id with the project extra only after get_credentials_and_project_id(...) returns, so the old in-provider raise really did pre-empt an explicitly configured project; the IdP branch's only consumer is GoogleBaseHook, and fallback_to_default_project_id still raises (naming the right fields) when nothing provides a project. The known_airflow_exceptions.txt count (16 → 15) matches the removed raise, and no existing test was changed.
Smaller observations
test_get_credentials_using_idp_no_project_id:mock_credentials = mock.MagicMock()is unspec'd;mock.create_autospec(Credentials, instance=True)would be slightly stricter. The neighbouring ADC test uses the same pattern, so this is just a nit.- Optional follow-up, not for this PR: the plain
credential_config_filebranch can also hand backNonefor the project id despite itsstrannotation; normalising it to""like the ADC and IdP branches would make the three paths consistent.
This review was drafted by an AI-assisted tool and
confirmed by an Apache Airflow maintainer. The maintainer
approving this PR has read the findings and signed off. If
something feels off, please reply on the PR and a maintainer
will follow up.More on how Apache Airflow handles maintainer review:
contributing-docs/05_pull_requests.rst.
Why
google.auth.default()started returningstr | None. It did so by raisingAirflowExceptionwhenever the resolved project id isNone, in three branches ofcredentials_provider.py. That raise fires inside the provider, before the caller can apply a project id it was given explicitly.CloudSecretManagerBackendfailed even when configured with an explicitproject_id. Fix CloudSecretManagerBackend regression with explicit project_id #61654 fixed that branch by returning an empty string instead of raising, and moved the check to the consumer. The backend now raisesValueErroronly when neither its explicitproject_idnor ADC provides one.GoogleBaseHookcan apply the connection'sprojectextra, so every Google hook on an IdP connection fails once google-auth cannot resolve a project id. The message also asks forkey_secret_project_id, which only the Secret Manager key path reads.How
GoogleBaseHook, which already overrides the value with the connection'sprojectextra and, throughfallback_to_default_project_id, raises a message naming the fields that work when nothing provides a project id.Verification
uv run --frozen --project providers/google pytest providers/google/tests/unit/google/cloud/utils/test_credentials_provider.pyuv run --frozen airflow connections add gcp_idp --conn-type google_cloud_platform --conn-extra '{"credential_config_file": {"type": "external_account", "audience": "//iam.googleapis.com/locations/global/workforcePools/airflow-pool/providers/okta", "subject_token_type": "urn:ietf:params:oauth:token-type:id_token", "token_url": "https://sts.googleapis.com/v1/token"}, "idp_issuer_url": "https://login.example.com/oauth2/token", "client_id": "airflow-prod", "client_secret": "not-used-offline", "project": "my-explicit-project"}'uv run --frozen airflow dags test gcp_idp_checkOn main branch, this fails with
airflow.sdk.exceptions.AirflowException: Project ID could not be determined from default credentials. Please providekey_secret_project_idparameter..On this PR, this runs successfully.
Was generative AI tooling used to co-author this PR?
{pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.