Add properties to SSHWrapperClient

[aesteve-rh]

Add command, identity, and username as class properties for SSHWrapperClient.

Summary by CodeRabbit

• New Features

  • Introduced accessible properties on the SSH client to expose SSH configuration details for easier access

• Refactor

  • Streamlined internal SSH command construction logic and optimized parameter handling throughout the connection flow

• Tests

  • Added test coverage for newly exposed client properties and their configurations

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[netlify]

✅ Deploy Preview for jumpstarter-docs ready!

Name	Link
🔨 Latest commit	d946560
🔍 Latest deploy log	https://app.netlify.com/projects/jumpstarter-docs/deploys/691f2fbcc5a8a900078bfd27
😎 Deploy Preview	https://deploy-preview-750--jumpstarter-docs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[coderabbitai]

Walkthrough

The SSHWrapperClient is refactored to expose command, identity, and username as public properties. Internal methods _run_ssh_local and _build_ssh_command_args are refactored to derive these values from properties rather than accepting them as parameters, simplifying their signatures.

Changes

Cohort / File(s)	Summary
Public property exposure packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/client.py	Added three public properties: command (returns SSH command), identity (returns SSH identity content, may raise ConfigurationError), and username (returns default SSH username).
Internal method refactoring packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/client.py	Updated _run_ssh_local signature from (host, port, ssh_command, options, default_username, ssh_identity, args) to (host, port, options, args), now deriving identity from self.identity and handling temporary identity files internally. Updated _build_ssh_command_args signature from (ssh_command, port, default_username, identity_file, args) to (port, identity_file, args), now using self.command and self.username internally.
Test coverage packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/driver_test.py	Added test_ssh_client_properties test function that verifies the username, identity, and command properties return expected values based on SSHWrapper configuration.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

• Method signature interdependencies: Verify all callers of _run_ssh_local and _build_ssh_command_args are updated consistently with the new signatures and that parameter derivation from properties is correct.
• Temporary identity file handling: Ensure cleanup and error reporting logic in the refactored _run_ssh_local is robust, particularly around temporary file creation and deletion.
• Property implementations: Confirm that command, identity, and username properties correctly delegate to the underlying get-methods with appropriate error handling (particularly the documented ConfigurationError in identity).
• Control flow preservation: Verify that the refactored internal flow logic and conditional checks (e.g., username defaulting when no -l flag is present) remain functionally equivalent.

Possibly related PRs

• driver-ssh: SSHWrapperClient.run return stdout and stderr #748: Directly modifies the same _run_ssh_local and SSH command-building flow internals in the SSHWrapperClient.
• Add SSHWrapper driver #647: Originally introduced the SSHWrapperClient and its SSH command construction flow that this PR now refactors with new property exposure and method signature updates.
• add ssh identity configuration #687: Prior changes to SSHWrapperClient involving ssh_identity handling, temporary identity file management, and _run_ssh_local/_build_ssh_command_args signature updates.

Suggested reviewers

• kirkbrauer
• NickCao
• mangelajo

Poem

🐰 Hop, hop, hooray! The properties now gleam so bright,
Command, identity, username—exposed to light!
No more parameters passed through the SSH night,
Refactored methods dance with signatures tight!
A cleaner way to SSH, our rabbit's delight! 🎉

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: adding three new properties (command, identity, username) to SSHWrapperClient, which is the primary focus of the changeset.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/client.py (1)

197-223: Minor edge cases and small duplication in SSH arg building

The overall behavior of _build_ssh_command_args—deriving from self.command, adding -i and -p, and injecting -l default_username only when no -l is present in SSH options—looks correct and satisfies the existing tests, including the “command -l vs SSH -l” scenarios.

Two low‑priority observations:

• args are parsed twice via _separate_ssh_options_and_command_args (here and again in _run_ssh_local), which also doubles the debug logging; you could optionally parse once in _run_ssh_local and pass ssh_options/command_args into this helper.
• If the user passes a bare -l with no following username, has_user_flag stays false, so the final command will contain both the raw -l from ssh_options and the injected -l default_username, which might be surprising; you may want to treat a lone -l as “user already managing login” or raise early.

Both are minor and can be deferred if current behavior is acceptable.

packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/driver_test.py (1)

697-709: Property test covers new client surface well

test_ssh_client_properties is a good, focused check that the client’s username, identity, and command properties reflect the driver configuration over the real serve(...) pathway, matching the refactor intent. If you ever extend identity sourcing further (e.g., more complex file‑based flows), you could add a similar test for the ssh_identity_file case, but this is already solid as‑is.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3d65e4b and d946560.

📒 Files selected for processing (2)

• packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/client.py (5 hunks)
• packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/driver_test.py (1 hunks)

🧰 Additional context used

🧠 Learnings (2)

📓 Common learnings

Learnt from: mangelajo
Repo: jumpstarter-dev/jumpstarter PR: 647
File: packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/client.py:0-0
Timestamp: 2025-09-25T05:52:27.157Z
Learning: In the SSHWrapper driver's client.py, hostname replacement should only occur for "userhostname" format arguments that get preprocessed into "-l user hostname". Regular hostname arguments (like "device" in SSH commands) should be passed through unchanged to allow the wrapper to handle connections internally.

📚 Learning: 2025-09-25T05:52:27.157Z

Learnt from: mangelajo
Repo: jumpstarter-dev/jumpstarter PR: 647
File: packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/client.py:0-0
Timestamp: 2025-09-25T05:52:27.157Z
Learning: In the SSHWrapper driver's client.py, hostname replacement should only occur for "userhostname" format arguments that get preprocessed into "-l user hostname". Regular hostname arguments (like "device" in SSH commands) should be passed through unchanged to allow the wrapper to handle connections internally.

Applied to files:

• packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/driver_test.py
• packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/client.py

🧬 Code graph analysis (2)

packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/driver_test.py (3)

packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/driver.py (2)

• SSHWrapper (9-50)
• client (28-29)

packages/jumpstarter-driver-network/jumpstarter_driver_network/driver.py (1)

• TcpNetwork (88-121)

packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/client.py (3)

• username (117-119)
• identity (103-114)
• command (98-100)

packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/client.py (1)

packages/jumpstarter/jumpstarter/client/base.py (1)

• call (42-52)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)

• GitHub Check: build
• GitHub Check: pytest-matrix (macos-15, 3.11)
• GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
• GitHub Check: pytest-matrix (macos-15, 3.13)
• GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
• GitHub Check: pytest-matrix (macos-15, 3.12)
• GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
• GitHub Check: Redirect rules - jumpstarter-docs
• GitHub Check: e2e
• GitHub Check: Header rules - jumpstarter-docs
• GitHub Check: Pages changed - jumpstarter-docs

🔇 Additional comments (3)

packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/client.py (3)

97-120: Public SSH client properties wired correctly

command, identity, and username are clean property wrappers over the corresponding driver exports and match the new test expectations. This also makes the client API easier to use without changing behavior.

---

121-151: Refactored run() path keeps hostname vs command separation intact

Delegating both direct and port‑forward flows to _run_ssh_local(host, port, options, args) keeps all SSH construction in one place and still appends the TCP host separately from args, so command arguments like "hostname" remain untouched and are not misinterpreted as SSH hostnames. This aligns with the previous requirement that only preprocessed -l user hostname triplets affect the SSH host, while regular command hostnames are preserved.

Based on learnings

---

152-196: Identity temp‑file handling is robust

Using self.identity to derive key material, writing it to a NamedTemporaryFile with 0600 permissions, and always cleaning up in the finally block (including best‑effort cleanup on creation failure) gives correct and predictable lifecycle behavior for the transient identity file. This matches and is well‑covered by the existing temp‑file creation/cleanup tests.

[jumpstarter-backport-bot]

Backport failed for release-0.7, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin release-0.7
git worktree add -d .worktree/backport-750-to-release-0.7 origin/release-0.7
cd .worktree/backport-750-to-release-0.7
git switch --create backport-750-to-release-0.7
git cherry-pick -x 7427850d033913410d4b125bc6b97527f13243d9 d946560313a4cc720dd79fd0c47ae3acf800b3dc

[jumpstarter-backport-bot]

Successfully created backport PR for release-0.7:

• [Backport release-0.7] Add properties to SSHWrapperClient #785