[vsock] Add ssh coordinates#5070
Draft
tobe2098 wants to merge 7 commits into
Draft
Conversation
Stack garbage would populate the variable and cause a SIGSEGV when freed, if the libssh function failed to parse the key.
tobe2098
force-pushed
the
add-ssh-coordinates
branch
from
331cbd5 to
499d507
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a new SSHCoordinates interface (backed by the RPC proto) to centralize SSH connection parameters for creating SSHSessions, as groundwork for future VSOCK-based sessions. It refactors VM, daemon, CLI/GUI clients, and tests to use the new coordinates object rather than separate host/port/username/key fields.
Changes:
- Adds
SSHCoordinatesto the RPC API and updates daemon/client code paths to usessh_coordinatesmaps inSSHInfoReply. - Refactors
PlainSSHSession,SSHClient,SFTPClient, andSFTPUtilsto acceptSSHCoordinates. - Updates VM backends/tests to provide
ssh_coordinates()and switches SSH port types touint32_t.
Reviewed changes
Copilot reviewed 44 out of 44 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_sshfsmount.cpp | Builds PlainSSHSession using SSHCoordinates in mount tests |
| tests/unit/test_sshfs_mount_handler.cpp | Updates expectations to use ssh_coordinates() |
| tests/unit/test_ssh_process.cpp | Updates session creation to use SSHCoordinates helper |
| tests/unit/test_ssh_client.cpp | Refactors tests to construct SSHClient via SSHCoordinates |
| tests/unit/test_sftpserver.cpp | Updates SFTP server tests to use SSHCoordinates |
| tests/unit/test_sftp_client.cpp | Updates SFTP client tests to use SSHCoordinates |
| tests/unit/test_plain_ssh_session.cpp | Updates plain session tests to pass SSHCoordinates |
| tests/unit/test_cli_client.cpp | Updates CLI client tests to use ssh_coordinates reply map |
| tests/unit/test_base_virtual_machine.cpp | Adjusts stub ssh_port() return type to uint32_t |
| tests/unit/stub_virtual_machine.h | Adds stub ssh_coordinates() implementation |
| tests/unit/qemu/test_qemu_backend.cpp | Updates backend tests to read hostname via ssh_coordinates().tcp_host() |
| tests/unit/mock_virtual_machine.h | Adds mock ssh_coordinates() and updates ssh port mock type |
| tests/unit/mock_sftp_utils.h | Updates mock make_SFTPClient signature to take SSHCoordinates |
| src/sshfs_mount/sshfs_server.cpp | Constructs SSHCoordinates and passes to PlainSSHSession |
| src/sshfs_mount/sshfs_mount_handler.cpp | Uses ssh_coordinates() for username/host/port population |
| src/ssh/ssh_client.cpp | Replaces ctor taking host/port/user/key with SSHCoordinates |
| src/ssh/ssh_client_key_provider.cpp | Fixes ssh_key initialization before import |
| src/ssh/sftp_utils.cpp | Updates factory to create SFTPClient from SSHCoordinates |
| src/ssh/sftp_client.cpp | Refactors ctor to accept SSHCoordinates |
| src/ssh/plain_ssh_session.cpp | Refactors session setup/auth to use SSHCoordinates fields |
| src/rpc/multipass.proto | Replaces SSHInfo with SSHCoordinates and updates reply map field |
| src/platform/backends/virtualbox/virtualbox_virtual_machine.h | Moves ssh accessors to new signatures/types |
| src/platform/backends/virtualbox/virtualbox_virtual_machine.cpp | Updates ssh_port() return type |
| src/platform/backends/shared/base_virtual_machine.h | Adds ssh_coordinates() override |
| src/platform/backends/shared/base_virtual_machine.cpp | Implements ssh_coordinates() and updates session creation sites |
| src/platform/backends/qemu/qemu_virtual_machine.h | Updates ssh accessors signatures/types |
| src/platform/backends/qemu/qemu_virtual_machine.cpp | Updates ssh_port() return type |
| src/platform/backends/hyperv/hyperv_virtual_machine.h | Updates ssh accessors signatures/types |
| src/platform/backends/hyperv/hyperv_virtual_machine.cpp | Refactors remote IP lookup to accept SSHCoordinates |
| src/platform/backends/hyperv_api/hcs_virtual_machine.h | Updates ssh accessors signatures/types |
| src/platform/backends/hyperv_api/hcs_virtual_machine.cpp | Updates ssh_port() return type |
| src/platform/backends/applevz/applevz_virtual_machine.h | Updates ssh accessors signatures/types |
| src/platform/backends/applevz/applevz_virtual_machine.cpp | Updates ssh_port() return type |
| src/daemon/daemon.cpp | Populates SSHInfoReply via ssh_coordinates map |
| src/client/gui/lib/vm_details/terminal.dart | Switches GUI terminal isolate wiring to SSHCoordinates |
| src/client/gui/lib/grpc_client.dart | Updates GUI RPC logging and sshInfo() accessor to use sshCoordinates |
| src/client/cli/cmd/transfer.cpp | Uses SSHCoordinates to construct SFTP client via utils |
| src/client/cli/cmd/shell.cpp | Uses SSHCoordinates to construct SSHClient |
| src/client/cli/cmd/exec.cpp | Uses SSHCoordinates to construct SSHClient and for sudo username |
| include/multipass/virtual_machine.h | Adds ssh_coordinates() to VM interface and updates access levels/types |
| include/multipass/ssh/ssh_client.h | Updates SSHClient public ctor to accept SSHCoordinates |
| include/multipass/ssh/sftp_utils.h | Updates SFTP utils API to accept SSHCoordinates |
| include/multipass/ssh/sftp_client.h | Updates SFTPClient ctor to accept SSHCoordinates |
| include/multipass/ssh/plain_ssh_session.h | Updates PlainSSHSession ctor to accept SSHCoordinates |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| mp::SSHCoordinates coord; | ||
| coord.set_username("ubuntu"); | ||
| coord.set_priv_key_base64(key_provider.private_key_as_base64()); | ||
| coord.set_port(42); |
Comment on lines
126
to
130
| config{"", | ||
| 0, | ||
| vm->ssh_username(), | ||
| vm->ssh_coordinates().username(), | ||
| vm->get_name(), | ||
| ssh_key_provider->private_key_as_base64(), |
Comment on lines
+161
to
+163
| auto coordinates = vm->ssh_coordinates(); | ||
| config.host = coordinates.tcp_host(); | ||
| config.port = coordinates.port(); |
Comment on lines
+24
to
+29
| #include <multipass/rpc/multipass.pb.h> | ||
|
|
||
| #include <fmt/format.h> | ||
|
|
||
| #include <QDir> | ||
|
|
Comment on lines
+227
to
232
| Future<SSHCoordinates?> sshInfo(String name) { | ||
| return doRpc( | ||
| _client.ssh_info, | ||
| SSHInfoRequest(instanceName: [name]), | ||
| ).then((r) => r!.sshInfo[name]); | ||
| ).then((r) => r!.sshCoordinates[name]); | ||
| } |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #5070 +/- ##
==========================================
+ Coverage 73.12% 73.17% +0.05%
==========================================
Files 331 331
Lines 17712 17700 -12
==========================================
Hits 12950 12950
+ Misses 4762 4750 -12 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The method does the same thing in all backends
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Testing
Checklist
MULTI-2742