Skip to content

Commit a98fb62

Browse files
authored
Revert "fix: send PostgreSQL v3 StartupMessage in proactive IAM refresh probe for MCP" (GoogleCloudPlatform#1483)
1 parent 50f4e19 commit a98fb62

7 files changed

Lines changed: 0 additions & 272 deletions

File tree

‎google/cloud/sql/connector/connection_info.py‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,6 @@ async def connect_info(self) -> ConnectionInfo:
4545
async def force_refresh(self) -> None:
4646
pass
4747

48-
@abc.abstractmethod
49-
def record_principal(self, user: str, database: str) -> None:
50-
pass
51-
5248
@abc.abstractmethod
5349
async def close(self) -> None:
5450
pass

‎google/cloud/sql/connector/connector.py‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -339,8 +339,6 @@ async def connect_async(
339339
self._client,
340340
self._keys,
341341
enable_iam_auth,
342-
ip_type=self._ip_type,
343-
timeout=self._timeout,
344342
)
345343
else:
346344
logger.debug(
@@ -351,8 +349,6 @@ async def connect_async(
351349
self._client,
352350
self._keys,
353351
enable_iam_auth,
354-
ip_type=self._ip_type,
355-
timeout=self._timeout,
356352
)
357353
# wrap cache as a MonitoredCache
358354
monitored_cache = MonitoredCache(
@@ -442,9 +438,6 @@ async def connect_async(
442438
f"['{instance_connection_string}']: Truncated IAM database username from {kwargs['user']} to {formatted_user}"
443439
)
444440
kwargs["user"] = formatted_user
445-
if conn_info.database_version.startswith("POSTGRES"):
446-
db_name = kwargs.get("db") or kwargs.get("database") or kwargs["user"]
447-
monitored_cache.record_principal(kwargs["user"], str(db_name))
448441

449442
try:
450443
last_ex = None

‎google/cloud/sql/connector/instance.py‎

Lines changed: 0 additions & 150 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
from google.cloud.sql.connector.connection_info import ConnectionInfo
2727
from google.cloud.sql.connector.connection_info import ConnectionInfoCache
2828
from google.cloud.sql.connector.connection_name import ConnectionName
29-
from google.cloud.sql.connector.enums import IPTypes
3029
from google.cloud.sql.connector.exceptions import RefreshNotValidError
3130
from google.cloud.sql.connector.rate_limiter import AsyncRateLimiter
3231
from google.cloud.sql.connector.refresh_utils import _is_valid
@@ -35,8 +34,6 @@
3534
logger = logging.getLogger(name=__name__)
3635

3736
APPLICATION_NAME = "cloud-sql-python-connector"
38-
SERVER_PROXY_PORT = 3307
39-
DEFAULT_CONNECT_TIMEOUT = 30
4037

4138

4239
class RefreshAheadCache(ConnectionInfoCache):
@@ -53,8 +50,6 @@ def __init__(
5350
client: CloudSQLClient,
5451
keys: asyncio.Future,
5552
enable_iam_auth: bool = False,
56-
ip_type: IPTypes | str = IPTypes.PUBLIC,
57-
timeout: int = DEFAULT_CONNECT_TIMEOUT,
5853
) -> None:
5954
"""Initializes a RefreshAheadCache instance.
6055
@@ -67,32 +62,21 @@ def __init__(
6762
enable_iam_auth (bool): Enables automatic IAM database authentication
6863
(Postgres and MySQL) as the default authentication method for all
6964
connections.
70-
ip_type (IPTypes | str): Preferred IP type used to connect to the instance.
71-
timeout (int): Connect timeout in seconds.
7265
"""
7366
self._conn_name = conn_name
7467

7568
self._enable_iam_auth = enable_iam_auth
76-
if isinstance(ip_type, str):
77-
ip_type = IPTypes._from_str(ip_type)
78-
self._ip_type = ip_type
79-
self._timeout = timeout
8069
self._keys = keys
8170
self._client = client
8271
self._refresh_rate_limiter = AsyncRateLimiter(
8372
max_capacity=2,
8473
rate=1 / 30,
8574
)
8675
self._refresh_in_progress = asyncio.locks.Event()
87-
self._iam_principals: list[tuple[str, str]] = []
8876
self._current: asyncio.Task = self._schedule_refresh(0)
8977
self._next: asyncio.Task = self._current
9078
self._closed = False
9179

92-
def record_principal(self, user: str, database: str) -> None:
93-
"""Records a PostgreSQL (user, database) pair observed on an IAM connection."""
94-
_append_iam_principal(self._iam_principals, user, database)
95-
9680
@property
9781
def conn_name(self) -> ConnectionName:
9882
return self._conn_name
@@ -135,8 +119,6 @@ async def _perform_refresh(self) -> ConnectionInfo:
135119
self._keys,
136120
self._enable_iam_auth,
137121
)
138-
if self._enable_iam_auth:
139-
await self._probe_connection(connection_info)
140122
logger.debug(
141123
f"['{self._conn_name}']: Connection info refresh operation complete"
142124
)
@@ -156,17 +138,6 @@ async def _perform_refresh(self) -> ConnectionInfo:
156138
self._refresh_in_progress.clear()
157139
return connection_info
158140

159-
async def _probe_connection(self, conn_info: ConnectionInfo) -> None:
160-
"""Proactively probes the database to refresh IAM tokens on server-side MCP."""
161-
await _probe_instance_connection(
162-
self._conn_name,
163-
conn_info,
164-
self._enable_iam_auth,
165-
self._ip_type,
166-
self._timeout,
167-
self._iam_principals,
168-
)
169-
170141
def _schedule_refresh(self, delay: int) -> asyncio.Task:
171142
"""
172143
Schedule task to sleep and then perform refresh to get ConnectionInfo.
@@ -252,124 +223,3 @@ async def close(self) -> None:
252223
tasks = asyncio.gather(self._current, self._next, return_exceptions=True)
253224
await asyncio.wait_for(tasks, timeout=2.0)
254225
self._closed = True
255-
256-
257-
def _append_iam_principal(
258-
principals: list[tuple[str, str]], user: str, database: str
259-
) -> None:
260-
"""Appends (user, database) to principals if not already present."""
261-
if not user:
262-
return
263-
if not database:
264-
database = user
265-
pair = (user, database)
266-
if pair not in principals:
267-
principals.append(pair)
268-
269-
270-
async def _probe_instance_connection(
271-
conn_name: ConnectionName,
272-
conn_info: ConnectionInfo,
273-
enable_iam_auth: bool,
274-
ip_type: IPTypes,
275-
timeout: int,
276-
iam_principals: list[tuple[str, str]],
277-
) -> None:
278-
"""Proactively probes the database to refresh IAM tokens on server-side MCP."""
279-
targets: list[str] = []
280-
if conn_name.domain_name:
281-
targets.append(conn_name.domain_name)
282-
else:
283-
if ip_type.value in conn_info.ip_addrs:
284-
targets.extend(conn_info.ip_addrs[ip_type.value])
285-
286-
if not targets:
287-
logger.debug(
288-
f"['{conn_name}']: Proactive IAM token refresh probe skipped: no target IP addresses"
289-
)
290-
return
291-
292-
port = SERVER_PROXY_PORT
293-
try:
294-
ssl_context = await conn_info.create_ssl_context(enable_iam_auth)
295-
except Exception as e: # noqa: BLE001
296-
logger.debug(
297-
f"['{conn_name}']: Failed to create SSL context for probe: {e!s}"
298-
)
299-
return
300-
301-
principals: list[tuple[str, str] | None] = []
302-
if conn_info.database_version.startswith("POSTGRES") and iam_principals:
303-
principals.extend(iam_principals)
304-
else:
305-
principals.append(None)
306-
307-
all_succeeded = True
308-
for principal in principals:
309-
probed_principal = False
310-
for target in targets:
311-
try:
312-
logger.debug(
313-
f"['{conn_name}']: Probing IAM token refresh on {target}:{port}"
314-
)
315-
reader, writer = await asyncio.wait_for(
316-
asyncio.open_connection(
317-
host=target,
318-
port=port,
319-
ssl=ssl_context,
320-
server_hostname=(
321-
conn_name.domain_name
322-
if conn_name.domain_name
323-
else None
324-
),
325-
),
326-
timeout=float(timeout),
327-
)
328-
try:
329-
if principal is not None:
330-
user, database = principal
331-
writer.write(
332-
_build_postgres_startup_packet(user, database)
333-
)
334-
await writer.drain()
335-
await asyncio.wait_for(
336-
reader.read(1024),
337-
timeout=float(timeout),
338-
)
339-
writer.write(b"X\x00\x00\x00\x04")
340-
await writer.drain()
341-
finally:
342-
writer.close()
343-
await writer.wait_closed()
344-
probed_principal = True
345-
break
346-
except Exception as e: # noqa: BLE001
347-
logger.debug(
348-
f"['{conn_name}']: Probing IAM token refresh on {target}:{port} failed: {e!s}"
349-
)
350-
if not probed_principal:
351-
all_succeeded = False
352-
353-
if all_succeeded:
354-
logger.debug(
355-
f"['{conn_name}']: Proactive IAM token refresh probe successful"
356-
)
357-
else:
358-
logger.debug(
359-
f"['{conn_name}']: Proactive IAM token refresh probe encountered error across all targets"
360-
)
361-
362-
363-
def _build_postgres_startup_packet(user: str, database: str) -> bytes:
364-
"""Builds a PostgreSQL v3.0 StartupMessage for (user, database)."""
365-
if not database:
366-
database = user
367-
body = (
368-
b"user\x00"
369-
+ user.encode("utf-8")
370-
+ b"\x00database\x00"
371-
+ database.encode("utf-8")
372-
+ b"\x00\x00"
373-
)
374-
total_len = 8 + len(body)
375-
return total_len.to_bytes(4, byteorder="big") + b"\x00\x03\x00\x00" + body

‎google/cloud/sql/connector/lazy.py‎

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,6 @@
2424
from google.cloud.sql.connector.connection_info import ConnectionInfo
2525
from google.cloud.sql.connector.connection_info import ConnectionInfoCache
2626
from google.cloud.sql.connector.connection_name import ConnectionName
27-
from google.cloud.sql.connector.enums import IPTypes
28-
from google.cloud.sql.connector.instance import _append_iam_principal
29-
from google.cloud.sql.connector.instance import _probe_instance_connection
30-
from google.cloud.sql.connector.instance import DEFAULT_CONNECT_TIMEOUT
3127
from google.cloud.sql.connector.refresh_utils import _refresh_buffer
3228

3329
logger = logging.getLogger(name=__name__)
@@ -48,8 +44,6 @@ def __init__(
4844
client: CloudSQLClient,
4945
keys: asyncio.Future,
5046
enable_iam_auth: bool = False,
51-
ip_type: IPTypes | str = IPTypes.PUBLIC,
52-
timeout: int = DEFAULT_CONNECT_TIMEOUT,
5347
) -> None:
5448
"""Initializes a LazyRefreshCache instance.
5549
@@ -62,27 +56,16 @@ def __init__(
6256
enable_iam_auth (bool): Enables automatic IAM database authentication
6357
(Postgres and MySQL) as the default authentication method for all
6458
connections.
65-
ip_type (IPTypes | str): Preferred IP type used to connect to the instance.
66-
timeout (int): Connect timeout in seconds.
6759
"""
6860
self._conn_name = conn_name
6961
self._enable_iam_auth = enable_iam_auth
70-
if isinstance(ip_type, str):
71-
ip_type = IPTypes._from_str(ip_type)
72-
self._ip_type = ip_type
73-
self._timeout = timeout
7462
self._keys = keys
7563
self._client = client
7664
self._lock = asyncio.Lock()
77-
self._iam_principals: list[tuple[str, str]] = []
7865
self._cached: ConnectionInfo | None = None
7966
self._needs_refresh = False
8067
self._closed = False
8168

82-
def record_principal(self, user: str, database: str) -> None:
83-
"""Records a PostgreSQL (user, database) pair observed on an IAM connection."""
84-
_append_iam_principal(self._iam_principals, user, database)
85-
8669
@property
8770
def conn_name(self) -> ConnectionName:
8871
return self._conn_name
@@ -99,17 +82,6 @@ async def force_refresh(self) -> None:
9982
async with self._lock:
10083
self._needs_refresh = True
10184

102-
async def _probe_connection(self, conn_info: ConnectionInfo) -> None:
103-
"""Proactively probes the database to refresh IAM tokens on server-side MCP."""
104-
await _probe_instance_connection(
105-
self._conn_name,
106-
conn_info,
107-
self._enable_iam_auth,
108-
self._ip_type,
109-
self._timeout,
110-
self._iam_principals,
111-
)
112-
11385
async def connect_info(self) -> ConnectionInfo:
11486
"""Retrieves ConnectionInfo instance for establishing a secure
11587
connection to the Cloud SQL instance.
@@ -138,8 +110,6 @@ async def connect_info(self) -> ConnectionInfo:
138110
self._keys,
139111
self._enable_iam_auth,
140112
)
141-
if self._enable_iam_auth:
142-
await self._probe_connection(conn_info)
143113
except Exception as e:
144114
logger.debug(
145115
f"['{self._conn_name}']: Connection info "

‎google/cloud/sql/connector/monitored_cache.py‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -140,9 +140,6 @@ async def force_refresh(self) -> None:
140140
return
141141
return await self.cache.force_refresh()
142142

143-
def record_principal(self, user: str, database: str) -> None:
144-
self.cache.record_principal(user, database)
145-
146143
async def close(self) -> None:
147144
# Cancel domain name ticker task.
148145
if self.domain_name_ticker:

‎tests/unit/test_instance.py‎

Lines changed: 0 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -391,37 +391,3 @@ async def test_ConnectionInfo_create_ssl_context_no_tls1_3_warning() -> None:
391391
in mock_logger.warning.call_args[0][0]
392392
)
393393

394-
395-
@pytest.mark.asyncio
396-
async def test_probe_connection_postgres_startup_packet(
397-
cache: RefreshAheadCache,
398-
) -> None:
399-
"""Test that _probe_connection sends PostgreSQL v3 StartupMessage and Terminate when IAM principal is recorded."""
400-
from unittest.mock import MagicMock
401-
402-
from google.cloud.sql.connector.instance import _build_postgres_startup_packet
403-
404-
cache._enable_iam_auth = True
405-
cache.record_principal("iam-user@example.com", "mydb")
406-
conn_info = await cache.connect_info()
407-
408-
mock_reader = AsyncMock()
409-
mock_reader.read = AsyncMock(return_value=b"R\x00\x00\x00\x08\x00\x00\x00\x00")
410-
mock_writer = MagicMock()
411-
mock_writer.drain = AsyncMock()
412-
mock_writer.wait_closed = AsyncMock()
413-
414-
with patch(
415-
"google.cloud.sql.connector.instance.asyncio.open_connection",
416-
AsyncMock(return_value=(mock_reader, mock_writer)),
417-
) as mock_open_conn:
418-
await cache._probe_connection(conn_info)
419-
420-
mock_open_conn.assert_awaited_once()
421-
written_packets = [call.args[0] for call in mock_writer.write.call_args_list]
422-
expected_startup = _build_postgres_startup_packet("iam-user@example.com", "mydb")
423-
assert written_packets == [expected_startup, b"X\x00\x00\x00\x04"]
424-
mock_reader.read.assert_awaited_once()
425-
mock_writer.close.assert_called_once()
426-
mock_writer.wait_closed.assert_awaited_once()
427-

0 commit comments

Comments
 (0)