Skip to content

[iris] Fix SSH OS Login to use impersonation SA from config#4399

Merged
rjpower merged 1 commit intomainfrom
fix/iris-ssh-impersonation-fallback
Apr 3, 2026
Merged

[iris] Fix SSH OS Login to use impersonation SA from config#4399
rjpower merged 1 commit intomainfrom
fix/iris-ssh-impersonation-fallback

Conversation

@rjpower
Copy link
Copy Markdown
Collaborator

@rjpower rjpower commented Apr 3, 2026

ssh_key_file() in workers.py was called without the impersonation service
account, so OS Login key registration ran as the user's personal Google
account and failed with an IAM permission error. Now ssh_key_file() falls
back to ssh_config.impersonate_service_account when no explicit SA is
passed. Also surfaces the gcloud stderr in the error message instead of
swallowing it behind capture_output.

…account

ssh_key_file() was called without the impersonation SA in workers.py,
causing OS Login registration to fail for users authenticating via a
personal Google account. Now falls back to ssh_config.impersonate_service_account
when no explicit SA is passed. Also surfaces gcloud stderr on failure
instead of swallowing it.
@rjpower rjpower added the agent-generated Created by automation/agent label Apr 3, 2026
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 3, 2026

Claude finished @rjpower's task in 59s —— View job


PR Review

  • Read changed files and gather context
  • Analyze changes for correctness
  • Post review feedback

Verdict: Looks good

This is a clean, well-scoped fix. Two changes:

  1. ssh_key_file() fallback (ssh.py:96): When no explicit impersonate_service_account is passed, it now falls back to ssh_config.impersonate_service_account. This fixes the bug where workers.py:240 and workers.py:650 call ssh_key_file(self._ssh_config) without the SA arg, causing OS Login key registration to run as the user's personal account.

  2. Better error surfacing (ssh.py:79-81): Replaces check=True + capture_output=True (which swallows stderr on failure) with manual returncode check that includes result.stderr in the error message. Good improvement — the old pattern meant gcloud errors were invisible.

The test updates are consistent: mocks now return CompletedProcess([], 0) to match the new manual returncode check, and test_ssh_key_file_os_login_uses_config_sa_as_fallback directly validates the new fallback path.

One minor observation: the handles.py call sites already pass ssh_impersonate_service_account(ssh_config) explicitly, so the fallback is a no-op there (correct behavior — explicit arg wins via or). The fix specifically addresses the workers.py callers that omit it.

@rjpower rjpower merged commit 2c75d2f into main Apr 3, 2026
33 of 35 checks passed
@rjpower rjpower deleted the fix/iris-ssh-impersonation-fallback branch April 3, 2026 20:00
Helw150 pushed a commit that referenced this pull request Apr 8, 2026
ssh_key_file() in workers.py was called without the impersonation
service
account, so OS Login key registration ran as the user's personal Google
account and failed with an IAM permission error. Now ssh_key_file()
falls
back to ssh_config.impersonate_service_account when no explicit SA is
passed. Also surfaces the gcloud stderr in the error message instead of
swallowing it behind capture_output.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent-generated Created by automation/agent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant