shahar1 commented on PR #61654:
URL: https://github.com/apache/airflow/pull/61654#issuecomment-3949910264
> Thanks @shahar1! Yeah sure,
>
> ### Scenario: ADC has no default project, but explicit project_id is
provided
>
> This is the core regression from #61217.
>
> **1. CloudSecretManagerBackend (secrets backend)**
>
> <details open>
> <summary>Code</summary>
>
> ~~~python
> from unittest import mock
>
> with mock.patch("google.auth.default", return_value=(mock.MagicMock(),
None)):
> from airflow.providers.google.cloud.secrets.secret_manager import
CloudSecretManagerBackend
>
> # BEFORE: raises AirflowException from _get_credentials_using_adc()
> # before __init__ can apply project_id - broken
> # AFTER: initializes successfully, explicit project_id is honored
> backend = CloudSecretManagerBackend(project_id="my-project")
> print(backend.project_id) # "my-project"
> ~~~
>
> </details>
>
> **2. BigQueryHook (operator-level usage)**
>
> Hooks and operators that call `get_credentials_and_project_id()` and then
override `project_id` from the connection or operator parameter follow the same
pattern:
>
> <details open>
> <summary>Code</summary>
>
> ~~~python
> from unittest import mock
> from airflow.providers.google.cloud.utils.credentials_provider import
get_credentials_and_project_id
>
> with mock.patch("google.auth.default", return_value=(mock.MagicMock(),
None)):
> creds, project_id = get_credentials_and_project_id()
> # BEFORE: AirflowException raised here, never reaches the operator
> # AFTER: returns (credentials, "") - empty string, not an exception
>
> effective_project_id = "my-explicit-project" or project_id
> print(effective_project_id) # "my-explicit-project"
> ~~~
>
> </details>
>
> **3. GCSHook / DataprocHook / any hook using BaseGoogleHook**
>
> All Google hooks ultimately go through `get_credentials_and_project_id()`.
The behavior is the same:
> - If the hook/operator/connection provides an explicit `project_id`, it
overrides the empty string and works correctly
> - If no explicit `project_id` is provided and ADC has none, the empty
string propagates to the Google API, which returns an API-level error (e.g.,
"Invalid project") - same end result as before, just a different error source
>
> ### Scenario: ADC has no default project and NO explicit project_id
>
> <details open>
> <summary>Code</summary>
>
> ~~~python
> from unittest import mock
>
> with mock.patch("google.auth.default", return_value=(mock.MagicMock(),
None)):
> from airflow.providers.google.cloud.secrets.secret_manager import
CloudSecretManagerBackend
>
> backend = CloudSecretManagerBackend()
> # ValueError: Project ID could not be determined. Please provide
> # 'project_id' in backend configuration or ensure your credentials
> # include a default project.
> ~~~
>
> </details>
>
> The user still gets a clear, actionable error.
>
> ### Scenario: ADC has a default project (normal/happy path)
>
> <details open>
> <summary>Code</summary>
>
> ~~~python
> from unittest import mock
>
> with mock.patch("google.auth.default", return_value=(mock.MagicMock(),
"adc-project")):
> from airflow.providers.google.cloud.secrets.secret_manager import
CloudSecretManagerBackend
>
> backend = CloudSecretManagerBackend()
> print(backend.project_id) # "adc-project" - unchanged behavior
> ~~~
>
> </details>
>
> Tested locally by running pytest on the two affected test files:
> - `pytest
providers/google/tests/unit/google/cloud/secrets/test_secret_manager.py` - 30
tests passed
> - `pytest
providers/google/tests/unit/google/cloud/utils/test_credentials_provider.py` -
43 tests passed
>
>
Sorry for the delay in response, currently on low availability - it will
take a while until I'll be able to fully review it again properly.
What I meant in last comment is to create actual Dags working against a real
GCP instance that will demonstrate the regressions and the fix, including
screenshots.
Given the blast radius of auth-related changes, I’d like to see real Dag
runs against GCP demonstrating both regression and fix. This is a sensitive
area, so I want to be extra careful here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
