Re: [PR] Adding flag for Cloudsql auto IAM [airflow]

["MaksYermak (via GitHub)" <gi...@apache.org>]

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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org