Skip to content

Commit d15bac5

Browse files
committed
[iris] Drop implicit service_account fallback from ssh_impersonate_service_account
Remove the service_account parameter from ssh_impersonate_service_account() so it only returns a value when impersonate_service_account is explicitly set in SshConfig. In metadata mode the controller's ambient VM identity is used for gcloud SSH — no impersonation needed. OS Login setups that need impersonation must set ssh.impersonate_service_account explicitly.
1 parent 5b51055 commit d15bac5

5 files changed

Lines changed: 13 additions & 26 deletions

File tree

lib/iris/src/iris/cluster/providers/gcp/controller.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,6 @@ def tunnel(
117117
project=self.worker_provider.project_id,
118118
label_prefix=self.worker_provider.label_prefix,
119119
ssh_config=self.worker_provider.ssh_config,
120-
service_account=self.controller_service_account,
121120
local_port=local_port,
122121
)
123122

@@ -228,7 +227,6 @@ def _gcp_tunnel(
228227
project: str,
229228
label_prefix: str,
230229
ssh_config: config_pb2.SshConfig | None,
231-
service_account: str | None = None,
232230
local_port: int | None = None,
233231
timeout: float = 60.0,
234232
) -> Iterator[str]:
@@ -238,7 +236,7 @@ def _gcp_tunnel(
238236
that may be listening on the same port on a different address family (IPv6).
239237
Picks a free port automatically if none is specified.
240238
"""
241-
effective_service_account = ssh_impersonate_service_account(ssh_config, service_account)
239+
effective_service_account = ssh_impersonate_service_account(ssh_config)
242240
key_file = ssh_key_file(ssh_config, effective_service_account)
243241
_check_gcloud_ssh_key(key_file)
244242

lib/iris/src/iris/cluster/providers/gcp/handles.py

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -65,15 +65,12 @@
6565

6666
def _os_login_user(
6767
ssh_config: config_pb2.SshConfig | None,
68-
service_account: str | None = None,
6968
) -> str:
7069
if ssh_config and ssh_config.os_login_user:
7170
return ssh_config.os_login_user
7271
if ssh_config and ssh_config.user and ssh_config.user != "root":
7372
return ssh_config.user
74-
return resolve_current_os_login_user(
75-
impersonate_service_account=ssh_impersonate_service_account(ssh_config, service_account)
76-
)
73+
return resolve_current_os_login_user(impersonate_service_account=ssh_impersonate_service_account(ssh_config))
7774

7875

7976
def _vm_slice_metadata_user(ssh_config: config_pb2.SshConfig | None) -> str:
@@ -319,10 +316,10 @@ def _describe_cloud(self) -> SliceStatus:
319316
direct_host = external_ip or internal_ip
320317
remote_exec = DirectSshRemoteExec(
321318
host=direct_host,
322-
user=_os_login_user(self._ssh_config, self._service_account),
319+
user=_os_login_user(self._ssh_config),
323320
key_file=ssh_key_file(
324321
self._ssh_config,
325-
ssh_impersonate_service_account(self._ssh_config, self._service_account),
322+
ssh_impersonate_service_account(self._ssh_config),
326323
),
327324
connect_timeout=(
328325
duration_from_proto(self._ssh_config.connect_timeout)
@@ -339,12 +336,9 @@ def _describe_cloud(self) -> SliceStatus:
339336
ssh_user=_vm_slice_metadata_user(self._ssh_config),
340337
ssh_key_file=ssh_key_file(
341338
self._ssh_config,
342-
ssh_impersonate_service_account(self._ssh_config, self._service_account),
343-
),
344-
impersonate_service_account=ssh_impersonate_service_account(
345-
self._ssh_config,
346-
self._service_account,
339+
ssh_impersonate_service_account(self._ssh_config),
347340
),
341+
impersonate_service_account=ssh_impersonate_service_account(self._ssh_config),
348342
_address=internal_ip,
349343
)
350344
workers.append(
@@ -449,12 +443,9 @@ def _describe_cloud(self) -> SliceStatus:
449443
ssh_user=None if uses_os_login(self._ssh_config) else _vm_slice_metadata_user(self._ssh_config),
450444
ssh_key_file=ssh_key_file(
451445
self._ssh_config,
452-
ssh_impersonate_service_account(self._ssh_config, self._service_account),
453-
),
454-
impersonate_service_account=ssh_impersonate_service_account(
455-
self._ssh_config,
456-
self._service_account,
446+
ssh_impersonate_service_account(self._ssh_config),
457447
),
448+
impersonate_service_account=ssh_impersonate_service_account(self._ssh_config),
458449
)
459450
worker = GcpStandaloneWorkerHandle(
460451
_vm_id=f"{self._slice_id}-worker-0",

lib/iris/src/iris/cluster/providers/gcp/ssh.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -98,10 +98,8 @@ def ssh_key_file(
9898

9999
def ssh_impersonate_service_account(
100100
ssh_config: config_pb2.SshConfig | None,
101-
service_account: str | None = None,
102101
) -> str | None:
102+
"""Return the explicit impersonation SA from ssh_config, or None."""
103103
if ssh_config and ssh_config.impersonate_service_account:
104104
return ssh_config.impersonate_service_account
105-
if service_account:
106-
return service_account
107105
return None

lib/iris/src/iris/cluster/providers/gcp/workers.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ def create_vm(self, config: config_pb2.VmConfig) -> GcpStandaloneWorkerHandle:
236236
zone=zone,
237237
vm_name=config.name,
238238
ssh_key_file=ssh_key_file(self._ssh_config),
239-
impersonate_service_account=ssh_impersonate_service_account(self._ssh_config, request.service_account),
239+
impersonate_service_account=ssh_impersonate_service_account(self._ssh_config),
240240
)
241241

242242
return GcpStandaloneWorkerHandle(
@@ -536,7 +536,7 @@ def list_vms(
536536
zone=vm.zone,
537537
vm_name=vm.name,
538538
ssh_key_file=ssh_key_file(self._ssh_config),
539-
impersonate_service_account=ssh_impersonate_service_account(self._ssh_config, vm.service_account),
539+
impersonate_service_account=ssh_impersonate_service_account(self._ssh_config),
540540
)
541541
handles.append(
542542
GcpStandaloneWorkerHandle(

lib/iris/tests/cluster/providers/gcp/test_platform.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -475,7 +475,7 @@ def test_gcp_vm_slice_os_login_uses_service_account_impersonation():
475475

476476
handle = platform.create_slice(cfg)
477477
status = handle.describe()
478-
assert status.workers[0]._remote_exec.impersonate_service_account == cfg.gcp.service_account
478+
assert status.workers[0]._remote_exec.impersonate_service_account is None
479479

480480

481481
def test_gcp_vm_slice_os_login_prefers_explicit_ssh_impersonation_account():
@@ -857,7 +857,7 @@ def test_gcp_tpu_slice_os_login_resolves_user_from_service_account():
857857
handle = platform.create_slice(cfg)
858858
status = handle.describe()
859859

860-
resolve_user.assert_called_with(impersonate_service_account=cfg.gcp.service_account)
860+
resolve_user.assert_called_with(impersonate_service_account=None)
861861
assert status.workers[0]._remote_exec.user == "svc-user"
862862

863863

0 commit comments

Comments
 (0)