MaksYermak commented on code in PR #59535:
URL: https://github.com/apache/airflow/pull/59535#discussion_r2754131131
##########
providers/google/src/airflow/providers/google/cloud/hooks/cloud_sql.py:
##########
@@ -597,10 +597,14 @@ def _get_sql_proxy_download_url(self):
return download_url
def _get_credential_parameters(self) -> list[str]:
- extras =
GoogleBaseHook.get_connection(conn_id=self.gcp_conn_id).extra_dejson
+ connection = GoogleBaseHook.get_connection(conn_id=self.gcp_conn_id)
+ extras = connection.extra_dejson
key_path = get_field(extras, "key_path")
keyfile_dict = get_field(extras, "keyfile_dict")
- if key_path:
+ if connection.login:
+ # if the connection already has an associated user account, then
we can use auto auth
+ credential_params = ["--auto-iam-authn", self.gcp_conn_id]
Review Comment:
Hello @shahar1, thank you for your suggestion for these changes it makes
sense and I agree with you. But lets talk about these changes in general
because IAM authentication in Cloud SQL is quite a complicated topic. And as I
see this PR may break the CloudSQL operator because Cloud SQL Auth Proxy has
two versions(v1 and v2) and as I know v1 doesn't have a `--auto-iam-authn` flag.
**About IAM authentication**
We have two options for using it with Cloud SQL Auth Proxy and without.
For enabling IAM without Proxy we already have this functionality in
CloudSQL operator here the [PR](https://github.com/apache/airflow/pull/43631).
But from a user perspective they need to do a lot of manually work in Google
Cloud to configure it and it can be inconvenient for them.
The second option is to enable IAM with Cloud SQL Proxy. It is a great idea
because in this case it may exclude a lot of manual work for users. But in this
case the code needs to check what version of Proxy the user uses and depends on
it choosing how to configure IAM. Because it is a big difference in how to
configure IAM for `v1` and `v2` you can read by this
[link](https://docs.cloud.google.com/sql/docs/postgres/iam-logins#cloud-sql-auth-proxy)
in `Important` bubble how different is it.
In my opinion this code needs improvements considering the version of Proxy
and, also, including a new system test using as example our current [System
Tests](https://github.com/apache/airflow/blob/main/providers/google/tests/system/google/cloud/cloud_sql/example_cloud_sql_query_iam.py)
for manually IAM configure. WDYT?
--
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]
