potiuk commented on code in PR #73646:
URL: https://github.com/apache/airflow/pull/73646#discussion_r4186262113
##########
providers/google/src/airflow/providers/google/cloud/hooks/cloud_sql.py:
##########
@@ -588,14 +603,42 @@ def __init__(
self.cloud_sql_proxy_socket_directory = self.path_prefix
self.sql_proxy_path = sql_proxy_binary_path or
f"{self.path_prefix}_cloud_sql_proxy"
self.credentials_path = self.path_prefix + "_credentials.json"
+ self._validate_sql_proxy_configuration()
self._build_command_line_parameters()
+ def _validate_sql_proxy_configuration(self) -> None:
+ # Validated here rather than in start_proxy(): callers stop the proxy
on failure, and
+ # stop_proxy() on a proxy that never started raises and hides the
original error.
+ if self.sql_proxy_major_version == 1:
+ if self.sql_proxy_version and
self.sql_proxy_version.lstrip("v").startswith("2."):
+ raise ValueError(
+ f"The sql_proxy_version {self.sql_proxy_version!r} is a
Cloud SQL Auth Proxy v2 "
+ "release. Set sql_proxy_major_version to 2 to use it!"
+ )
+ return
+ if not self.instance_specification:
+ raise ValueError(
+ "Cloud SQL Auth Proxy v2 does not support forwarding all
instances of a project. "
+ "The instance_specification must be provided!"
+ )
+ if not os.path.isfile(self.sql_proxy_path):
+ self._get_sql_proxy_download_url()
+
def _build_command_line_parameters(self) -> None:
+ if self.sql_proxy_major_version == 2:
+ self._build_v2_command_line_parameters()
+ return
self.command_line_parameters.extend(["-dir",
self.cloud_sql_proxy_socket_directory])
self.command_line_parameters.extend(["-instances",
self.instance_specification])
if self.sql_proxy_enable_iam_login:
self.command_line_parameters.append("-enable_iam_login")
+ def _build_v2_command_line_parameters(self) -> None:
+ self.command_line_parameters.extend(["--unix-socket",
self.cloud_sql_proxy_socket_directory])
+ if self.sql_proxy_enable_iam_login:
+ self.command_line_parameters.append("--auto-iam-authn")
+ self.command_line_parameters.append(self.instance_specification)
Review Comment:
v1 and v2 pick the instance's IP differently, and this command line doesn't
account for it. v1 defaults to `-ip_address_types=PUBLIC,PRIVATE` (public
first, falling back to private), while v2 uses only the public IP unless
`--private-ip` or `--auto-ip` is passed. So a connection to a Cloud SQL
instance with only a private IP works on v1 but fails after switching to
`"sql_proxy_major_version": 2`, which the docs now recommend for MySQL IAM
login. The v2 proxy still logs that it's ready, so the task only fails later,
at query time, with a confusing connection error. There's also no extra or
parameter that would let users fix it. Could you either default v2 to
`--auto-ip` (closest to v1's behaviour), or expose an IP-type option mapped to
`--private-ip` / `--psc`, and document it?
--
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]
