Skip to content

Commit f265351

Browse files
author
Kokoro Connector Build
committed
refactor: Align Python connector fallback and global region behavior with Go
- Reject 'global' region in direct PSC DNS resolution in DnsResolver to force CNAME/TXT fallback. - Remove fallback to Private/Public IP in Connector when DNS fails and preferred IP is a hostname. - Update test_connector.py to assert no fallback from PSC to Private IP when DNS fails. - Add test_resolver.py test to verify global region skips direct resolution.
1 parent a2b59be commit f265351

4 files changed

Lines changed: 49 additions & 48 deletions

File tree

google/cloud/sql/connector/connector.py

Lines changed: 4 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818

1919
import asyncio
2020
from functools import partial
21-
import ipaddress
2221
import logging
2322
import os
2423
import socket
@@ -51,26 +50,8 @@
5150
logger = logging.getLogger(name=__name__)
5251

5352

54-
def _is_ip_address(ip: str) -> bool:
55-
try:
56-
ipaddress.ip_address(ip)
57-
return True
58-
except ValueError:
59-
return False
6053

6154

62-
def _get_fallback_ips(
63-
current_ips: list[str], ip_addresses: dict[str, list[str]]
64-
) -> list[str]:
65-
if not current_ips:
66-
return current_ips
67-
if _is_ip_address(current_ips[0]):
68-
return current_ips
69-
fallback = ip_addresses.get("PRIVATE")
70-
if not fallback:
71-
fallback = ip_addresses.get("PRIMARY")
72-
return fallback if fallback else current_ips
73-
7455
ASYNC_DRIVERS = ["asyncpg"]
7556
SERVER_PROXY_PORT = 3307
7657
_DEFAULT_SCHEME = "https://"
@@ -430,25 +411,19 @@ async def connect_async(
430411
"using it to connect"
431412
)
432413
else:
433-
fallback_ips = _get_fallback_ips(
434-
preferred_ips, conn_info.ip_addrs
435-
)
436414
logger.debug(
437415
f"['{instance_connection_string}']: Custom DNS name "
438416
f"'{conn_info.conn_name.domain_name}' resolved but returned no "
439-
f"entries, using '{fallback_ips[0]}' from instance metadata"
417+
f"entries, using '{preferred_ips}' from instance metadata"
440418
)
441-
targets.extend(fallback_ips)
419+
targets.extend(preferred_ips)
442420
except Exception as e:
443-
fallback_ips = _get_fallback_ips(
444-
preferred_ips, conn_info.ip_addrs
445-
)
446421
logger.debug(
447422
f"['{instance_connection_string}']: Custom DNS name "
448423
f"'{conn_info.conn_name.domain_name}' did not resolve to an IP "
449-
f"address: {e}, using '{fallback_ips[0]}' from instance metadata"
424+
f"address: {e}, using '{preferred_ips}' from instance metadata"
450425
)
451-
targets.extend(fallback_ips)
426+
targets.extend(preferred_ips)
452427
else:
453428
targets.extend(preferred_ips)
454429

google/cloud/sql/connector/resolver.py

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -69,19 +69,20 @@ async def resolve(self, dns: str) -> ConnectionName: # type: ignore
6969
match = PSC_DNS_PATTERN.match(dns_normalized.lower())
7070
if match:
7171
region = match.group(3)
72-
if self._client is None:
73-
raise ValueError(
74-
"SQLAdmin client is not configured in the resolver."
72+
if region != "global":
73+
if self._client is None:
74+
raise ValueError(
75+
"SQLAdmin client is not configured in the resolver."
76+
)
77+
78+
dns_name_with_dot = dns_normalized + "."
79+
resp = await self._client.resolve_connect_settings(
80+
dns_name_with_dot, region
81+
)
82+
resolved_conn_name = resp["connectionName"]
83+
return _parse_connection_name_with_domain_name(
84+
resolved_conn_name, dns
7585
)
76-
77-
dns_name_with_dot = dns_normalized + "."
78-
resp = await self._client.resolve_connect_settings(
79-
dns_name_with_dot, region
80-
)
81-
resolved_conn_name = resp["connectionName"]
82-
return _parse_connection_name_with_domain_name(
83-
resolved_conn_name, dns
84-
)
8586

8687
if not _is_valid_domain(current):
8788
raise ValueError(

tests/unit/test_connector.py

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -660,10 +660,10 @@ async def test_Connector_connect_async_custom_dns_resolver_fallback(
660660

661661

662662
@pytest.mark.asyncio
663-
async def test_Connector_connect_async_custom_dns_resolver_fallback_psc_to_private_ip(
663+
async def test_Connector_connect_async_custom_dns_resolver_no_fallback_psc_to_private_ip(
664664
fake_credentials: Credentials, fake_client: CloudSQLClient
665665
) -> None:
666-
"""Test that Connector.connect_async falls back to Private IP if CNAME/PSC DNS resolution fails."""
666+
"""Test that Connector.connect_async does not fall back to Private IP if CNAME/PSC DNS resolution fails."""
667667

668668
with patch(
669669
"google.cloud.sql.connector.resolver.DnsResolver.resolve_a_record"
@@ -688,8 +688,10 @@ async def test_Connector_connect_async_custom_dns_resolver_fallback_psc_to_priva
688688
connector._client = fake_client
689689

690690
original_ips = fake_client.instance.ip_addrs
691-
# Configure instance to be PSC enabled, but also have a PRIVATE IP fallback!
691+
original_dns_names = fake_client.instance.dns_names
692+
# Configure instance to be PSC enabled, but also have a PRIVATE IP!
692693
fake_client.instance.psc_enabled = True
694+
fake_client.instance.dns_names = ["1ad3b5d73f10.3oxon2yfo9tob.us-east1.sql.goog"]
693695
fake_client.instance.ip_addrs = {
694696
"PSC": "1ad3b5d73f10.3oxon2yfo9tob.us-east1.sql.goog",
695697
"PRIVATE": "10.0.0.1",
@@ -709,13 +711,14 @@ async def test_Connector_connect_async_custom_dns_resolver_fallback_psc_to_priva
709711
db="my-db",
710712
)
711713

712-
# Verify mock_connect fell back to PRIVATE IP "10.0.0.1"!
714+
# Verify mock_connect used PSC DNS instead of falling back to PRIVATE IP "10.0.0.1"!
713715
args, _ = mock_connect.call_args
714-
assert args[0] == "10.0.0.1"
716+
assert args[0] == "1ad3b5d73f10.3oxon2yfo9tob.us-east1.sql.goog"
715717
assert connection is True
716718
finally:
717-
# Restore original IPs
719+
# Restore original IPs and DNS names
718720
fake_client.instance.ip_addrs = original_ips
721+
fake_client.instance.dns_names = original_dns_names
719722
fake_client.instance.psc_enabled = False
720723

721724

tests/unit/test_resolver.py

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -277,4 +277,26 @@ async def test_DnsResolver_cname_loop_throws_error() -> None:
277277

278278
with pytest.raises(DnsResolutionError) as exc_info:
279279
await resolver.resolve(dns_name)
280-
assert "CNAME loop detected" in str(exc_info.value)
280+
assert "CNAME loop detected" in str(exc_info.value)
281+
282+
283+
async def test_DnsResolver_global_region_skips_direct_resolution() -> None:
284+
"""Test DnsResolver skips direct resolution if the PSC DNS name has a global region."""
285+
dns_name = "0123456789ab.fedcba9876543.global.sql-psc.goog"
286+
287+
mock_client = AsyncMock()
288+
resolver = DnsResolver()
289+
resolver.set_client(mock_client)
290+
291+
# Patch CNAME and TXT to fail, so it eventually raises DnsResolutionError
292+
with patch.object(
293+
resolver, "resolve_cname", AsyncMock(side_effect=DnsResolutionError("No CNAME"))
294+
), patch.object(
295+
resolver, "resolve_txt", AsyncMock(side_effect=DnsResolutionError("No TXT"))
296+
):
297+
298+
with pytest.raises(DnsResolutionError):
299+
await resolver.resolve(dns_name)
300+
301+
# Verify mock_client was NOT called because direct resolution was skipped
302+
mock_client.resolve_connect_settings.assert_not_called()

0 commit comments

Comments
 (0)