feat(driver-ssh): add --user selection to j ssh#769
feat(driver-ssh): add --user selection to j ssh#769aesteve-rh wants to merge 1 commit intojumpstarter-dev:mainfrom
Conversation
WalkthroughA new optional CLI flag for SSH user (-u/--user) is added and threaded from the CLI handler through SSHWrapperClient.run into _run_ssh_local and _build_ssh_command_args. The provided user, when present, is preferred over the driver's default username during SSH command construction and preserved across fallback paths. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (3)packages/jumpstarter-driver-*/jumpstarter_driver_*/client.py📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Files:
packages/jumpstarter-driver-**/jumpstarter_driver_**/*.py📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Files:
**/*.py📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Files:
🧠 Learnings (5)📓 Common learnings📚 Learning: 2025-09-25T05:52:27.157ZApplied to files:
📚 Learning: 2025-11-27T09:58:41.875ZApplied to files:
📚 Learning: 2025-11-27T09:58:55.346ZApplied to files:
📚 Learning: 2025-11-27T09:58:41.875ZApplied to files:
🧬 Code graph analysis (2)packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/client.py (1)
packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/driver_test.py (3)
⏰ 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). (9)
🔇 Additional comments (7)
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. Comment |
✅ Deploy Preview for jumpstarter-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/client.py (1)
136-142: Critical: Missing user parameter in fallback path.When the direct connection fails and falls back to SSH port forwarding, the
userparameter is not passed to the recursiverun()call. This causes the explicit user override to be lost during fallback.Apply this diff to fix the issue:
return self.run(SSHCommandRunOptions( direct=False, capture_output=options.capture_output, capture_as_text=options.capture_as_text, - ), args) + ), args, user=user)
🧹 Nitpick comments (1)
packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/driver_test.py (1)
108-138: LGTM: Test comprehensively validates the user parameter feature.The test correctly verifies that the explicit user parameter overrides the default username and is properly passed through to the SSH command.
Consider adding a test case for the fallback scenario where
direct=Truebut the direct connection fails and falls back to port forwarding. This would ensure theuserparameter is preserved during the fallback (which currently has a bug in the implementation).Example test structure:
def test_ssh_command_with_explicit_user_parameter_fallback(): """Test that user parameter is preserved during direct-to-portforward fallback.""" instance = SSHWrapper( children={"tcp": TcpNetwork(host="127.0.0.1", port=22)}, default_username="testuser", ) with serve(instance) as client: with patch("subprocess.run") as mock_run: mock_run.return_value = MagicMock(returncode=0, stdout="some stdout", stderr="") # Mock tcp.address() to fail, triggering fallback with patch.object(client.tcp, 'address', side_effect=ValueError("Connection failed")): result = client.run(SSHCommandRunOptions(direct=True), ["hostname"], user="overrideuser") # Verify that overrideuser is still used after fallback call_args = mock_run.call_args[0][0] assert "-l" in call_args assert "overrideuser" in call_args assert "testuser" not in call_args
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/client.py(6 hunks)packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/driver_test.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
packages/jumpstarter-driver-**/jumpstarter_driver_**/*.py
📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Driver implementations should follow existing code style validated with
make lint(fix withmake lint-fix), perform static type checking withmake ty-pkg-${package_name}, add comprehensive tests, and verify all tests pass withmake test-pkg-${package_name}ormake test
Files:
packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/driver_test.pypackages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/client.py
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Ruff should be used for code formatting and linting, excluding
jumpstarter-protocolpackage
Files:
packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/driver_test.pypackages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/client.py
packages/jumpstarter-driver-*/jumpstarter_driver_*/client.py
📄 CodeRabbit inference engine (.cursor/rules/creating-new-drivers.mdc)
Driver client CLIs should either implement known classes that provide a CLI interface, or implement their own CLI interface. Use
CompositeClientfromjumpstarter_driver_composite.clientfor composite drivers with child drivers.
Files:
packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/client.py
🧠 Learnings (2)
📚 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/client.py
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/client.py : Driver client CLIs should either implement known classes that provide a CLI interface, or implement their own CLI interface. Use `CompositeClient` from `jumpstarter_driver_composite.client` for composite drivers with child drivers.
Applied to files:
packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/client.py
🧬 Code graph analysis (1)
packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/driver_test.py (2)
packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/driver.py (2)
SSHWrapper(9-50)client(28-29)packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/client.py (3)
run(122-151)SSHCommandRunOptions(33-47)SSHCommandRunResult(17-29)
⏰ 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). (9)
- GitHub Check: Redirect rules - jumpstarter-docs
- GitHub Check: Header rules - jumpstarter-docs
- GitHub Check: Pages changed - jumpstarter-docs
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: e2e
🔇 Additional comments (3)
packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/client.py (3)
65-75: LGTM: CLI option properly integrated.The
--useroption is correctly added to the CLI command and properly threaded through to therunmethod.
153-179: LGTM: User parameter correctly threaded through.The
_run_ssh_localmethod signature is properly updated and theuserparameter is correctly passed to_build_ssh_command_args.
198-224: LGTM: Username selection logic is correct.The
_build_ssh_command_argsmethod correctly prioritizes the explicituserparameter over the driver's default username, while still respecting any-lflags provided in the arguments.
aesteve-rh
force-pushed
the
user-arg-ssh
branch
from
7069788 to
96f220b
Compare
Allow `j ssh -u someuser` to be able to select the user with which to connect to the board. Signed-off-by: Albert Esteve <aesteve@redhat.com>
aesteve-rh
force-pushed
the
user-arg-ssh
branch
from
96f220b to
a5fa019
Compare
1 participant
Allow
j ssh -u someuserto be able to select the user with which to connect to the board.Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.