Skip to content

add custom options to sshdriver #1073

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ New Features in 23.0
display device, ``fb-headless`` will create a headless framebuffer device
for software rendering, and ``egl-headless`` will create a headless GPU
device for accelerated rendering (but requires host support)
- Possible to define extra option for ssh connection

Bug fixes in 23.0
~~~~~~~~~~~~~~~~~
Expand Down
6 changes: 6 additions & 0 deletions doc/configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1483,6 +1483,10 @@ Implements:

SSHDriver:
keyfile: example.key
extra_options:
- "-o option1"
- "-o options2"
- "-4"
Comment on lines +1486 to +1489
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems inconsistent. Why use an explicit list if not to exactly device the argv elements?


Arguments:
- keyfile (str): optional, filename of private key to login into the remote system
Expand All @@ -1491,6 +1495,8 @@ Arguments:
stdout, and an empty list as second element.
- connection_timeout (float, default=30.0): timeout when trying to establish connection to
target.
- extra_options (list or str): optional, a list of extra options required for
the ssh connection, e.g. defining host key algorithms.

UBootDriver
~~~~~~~~~~~
Expand Down
7 changes: 7 additions & 0 deletions labgrid/driver/sshdriver.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class SSHDriver(CommandMixin, Driver, CommandProtocol, FileTransferProtocol):
bindings = {"networkservice": "NetworkService", }
priorities = {CommandProtocol: 10, FileTransferProtocol: 10}
keyfile = attr.ib(default="", validator=attr.validators.instance_of(str))
extra_options = attr.ib(default="", validator=attr.validators.instance_of((str, list)))
stderr_merge = attr.ib(default=False, validator=attr.validators.instance_of(bool))
connection_timeout = attr.ib(default=float(get_ssh_connect_timeout()), validator=attr.validators.instance_of(float))

Expand All @@ -48,6 +49,12 @@ def on_activate(self):
if not self.networkservice.password:
self.ssh_prefix += ["-o", "PasswordAuthentication=no"]

if isinstance(self.extra_options, str) and len(self.extra_options) > 0:
self.ssh_prefix += shlex.split(self.extra_options)
elif isinstance(self.extra_options, list) and len(self.extra_options) > 0:
for eo in self.extra_options:
self.ssh_prefix += shlex.split(eo)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would we split each option in the list? This makes it impossible to pass options with spaces.


self.control = self._start_own_master()
self.ssh_prefix += ["-F", "none"]
if self.control:
Expand Down
40 changes: 40 additions & 0 deletions tests/test_sshdriver.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,46 @@ def test_create(target, mocker):
s = SSHDriver(target, "ssh")
assert isinstance(s, SSHDriver)

def test_can_define_ssh_options(target, mocker):
NetworkService(target, "service", "1.2.3.4", "root")
call = mocker.patch('subprocess.call')
call.return_value = 0
popen = mocker.patch('subprocess.Popen', autospec=True)
path = mocker.patch('os.path.exists')
path.return_value = True
instance_mock = mocker.MagicMock()
popen.return_value = instance_mock
instance_mock.wait = mocker.MagicMock(return_value=0)

tp = SSHDriver(target, "ssh")
tp.extra_options = "-o HostKeyAlgorithms=+ssh-rsa"
s = target.get_driver("SSHDriver")
assert "HostKeyAlgorithms=+ssh-rsa" in s.ssh_prefix

def test_extra_options_can_be_a_list(target, mocker):
NetworkService(target, "service", "1.2.3.4", "root")
call = mocker.patch('subprocess.call')
call.return_value = 0
popen = mocker.patch('subprocess.Popen', autospec=True)
path = mocker.patch('os.path.exists')
path.return_value = True
instance_mock = mocker.MagicMock()
popen.return_value = instance_mock
instance_mock.wait = mocker.MagicMock(return_value=0)

tp = SSHDriver(target, "ssh")
tp.extra_options = [
"-o HostKeyAlgorithms=+ssh-rsa",
"-o HostKeyAlgorithms=+ssh-dsa",
]
s = target.get_driver("SSHDriver")
assert "HostKeyAlgorithms=+ssh-rsa" in s.ssh_prefix
assert "HostKeyAlgorithms=+ssh-dsa" in s.ssh_prefix

def test_no_options_will_not_result_in_none_prefix(ssh_driver_mocked_and_activated, mocker):
s = ssh_driver_mocked_and_activated
assert not '' in s.ssh_prefix

def test_run_check(ssh_driver_mocked_and_activated, mocker):
s = ssh_driver_mocked_and_activated
s._run = mocker.MagicMock(return_value=(['success'], [], 0))
Expand Down