diff --git a/doc/configuration.rst b/doc/configuration.rst index cb7e4c520..59d197e62 100644 --- a/doc/configuration.rst +++ b/doc/configuration.rst @@ -694,6 +694,8 @@ A :any:`NetworkService` describes a remote SSH connection. The example describes a remote SSH connection to the computer ``example.computer`` with the username ``root``. +If ``username`` is omitted, labgrid will let SSH resolve the username through +the local SSH configuration or the default SSH user selection. Set the optional password password property to make SSH login with a password instead of the key file. @@ -706,7 +708,7 @@ These and the sudo configuration needs to be prepared by the administrator. Arguments: - address (str): hostname of the remote system - - username (str): username used by SSH + - username (str, default=""): optional, username used by SSH - password (str, default=None): optional, password used by SSH - port (int, default=22): port used by SSH @@ -2086,7 +2088,9 @@ Arguments: will explicitly use the SFTP protocol for file transfers instead of scp's default protocol - explicit_scp_mode (bool, default=False): if set to True, ``put()``, ``get()``, and ``scp()`` will explicitly use the SCP protocol for file transfers instead of scp's default protocol - - username (str, default=username from `NetworkService`_): username used by SSH + - username (str, default=username from `NetworkService`_): optional, username used by SSH + If neither `SSHDriver`_ nor `NetworkService`_ specifies a username, SSH's + own username resolution is used. - password (str, default=password from `NetworkService`_): password used by SSH UBootDriver diff --git a/labgrid/driver/sshdriver.py b/labgrid/driver/sshdriver.py index 110a4f707..42232373b 100644 --- a/labgrid/driver/sshdriver.py +++ b/labgrid/driver/sshdriver.py @@ -118,8 +118,10 @@ def _start_own_master_once(self, timeout): "-o", "ControlPersist=300", "-o", "UserKnownHostsFile=/dev/null", "-o", "StrictHostKeyChecking=no", "-o", "ServerAliveInterval=15", "-MN", "-S", control.replace('%', '%%'), "-p", - str(self.networkservice.port), "-l", self._get_username(), - self.networkservice.address] + str(self.networkservice.port)] + if self._get_username(): + args += ["-l", self._get_username()] + args += [self.networkservice.address] # proxy via the exporter if we have an ifname suffix address = self.networkservice.address @@ -214,9 +216,10 @@ def _run(self, cmd, codec="utf-8", decodeerrors="strict", timeout=None): raise ExecutionError("Keepalive no longer running") complete_cmd = [self._ssh, "-x", *self.ssh_prefix, - "-p", str(self.networkservice.port), "-l", self._get_username(), - self.networkservice.address - ] + cmd.split(" ") + "-p", str(self.networkservice.port)] + if self._get_username(): + complete_cmd += ["-l", self._get_username()] + complete_cmd += [self.networkservice.address] + cmd.split(" ") self.logger.debug("Sending command: %s", complete_cmd) if self.stderr_merge: stderr_pipe = subprocess.STDOUT @@ -482,6 +485,10 @@ def _scp_supports_explicit_scp_mode(self): @Driver.check_active @step(args=['filename', 'remotepath']) def put(self, filename, remotepath=''): + destination = f"{self.networkservice.address}:{remotepath}" + if self._get_username(): + destination = f"{self._get_username()}@{destination}" + transfer_cmd = [ self._scp, "-S", self._ssh, @@ -489,7 +496,7 @@ def put(self, filename, remotepath=''): "-P", str(self.networkservice.port), "-r", filename, - f"{self._get_username()}@{self.networkservice.address}:{remotepath}" + destination ] if self.explicit_sftp_mode and self._scp_supports_explicit_sftp_mode(): @@ -513,13 +520,17 @@ def put(self, filename, remotepath=''): @Driver.check_active @step(args=['filename', 'destination']) def get(self, filename, destination="."): + source = f"{self.networkservice.address}:{filename}" + if self._get_username(): + source = f"{self._get_username()}@{source}" + transfer_cmd = [ self._scp, "-S", self._ssh, *self.ssh_prefix, "-P", str(self.networkservice.port), "-r", - f"{self._get_username()}@{self.networkservice.address}:{filename}", + source, destination ] @@ -543,7 +554,10 @@ def get(self, filename, destination="."): def _cleanup_own_master(self): """Exit the controlmaster and delete the tmpdir""" - complete_cmd = f"{self._ssh} -x -o ControlPath={self.control.replace('%', '%%')} -O exit -p {self.networkservice.port} -l {self._get_username()} {self.networkservice.address}".split(' ') # pylint: disable=line-too-long + complete_cmd = [self._ssh, "-x", "-o", f"ControlPath={self.control.replace('%', '%%')}", "-O", "exit", "-p", str(self.networkservice.port)] # pylint: disable=line-too-long + if self._get_username(): + complete_cmd += ["-l", self._get_username()] + complete_cmd += [self.networkservice.address] res = subprocess.call( complete_cmd, stdin=subprocess.DEVNULL, @@ -560,7 +574,10 @@ def _cleanup_own_master(self): def _start_keepalive(self): """Starts a keepalive connection via the own or external master.""" - args = [self._ssh, *self.ssh_prefix, self.networkservice.address, "cat"] + args = [self._ssh, *self.ssh_prefix] + if self._get_username(): + args += ["-l", self._get_username()] + args += [self.networkservice.address, "cat"] assert self._keepalive is None self._keepalive = subprocess.Popen( diff --git a/labgrid/remote/client.py b/labgrid/remote/client.py index 4d2eb0bfa..fe239f441 100755 --- a/labgrid/remote/client.py +++ b/labgrid/remote/client.py @@ -1302,7 +1302,7 @@ def _get_ssh(self): ip = self._get_ip(place) if not ip: return - resource = NetworkService(target, address=str(ip), username="root") + resource = NetworkService(target, name=None, address=str(ip)) drv = self._get_driver_or_new(target, "SSHDriver", name=resource.name) return drv diff --git a/labgrid/resource/networkservice.py b/labgrid/resource/networkservice.py index 832cfd707..fd564f856 100644 --- a/labgrid/resource/networkservice.py +++ b/labgrid/resource/networkservice.py @@ -8,6 +8,6 @@ @attr.s(eq=False) class NetworkService(Resource): address = attr.ib(validator=attr.validators.instance_of(str)) - username = attr.ib(validator=attr.validators.instance_of(str)) + username = attr.ib(default="", validator=attr.validators.instance_of(str)) password = attr.ib(default=None, validator=attr.validators.optional(attr.validators.instance_of(str))) port = attr.ib(default=22, validator=attr.validators.instance_of(int)) diff --git a/tests/test_client.py b/tests/test_client.py index 8d2cc1c9f..6ff7ead40 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -76,6 +76,27 @@ def test_connect_timeout(coordinator): coordinator.resume_tree() pass +def test_get_ssh_no_username(target, mocker): + from labgrid.remote.client import ClientSession + from labgrid.resource import NetworkService + + driver = object() + + session = object.__new__(ClientSession) + session.args = type("Args", (), {"name": None})() + session.get_acquired_place = lambda: "test-place" + session._get_target = lambda place: target + session._get_ip = lambda place: "192.0.2.10" + session._get_driver_or_new = mocker.MagicMock(return_value=driver) + + result = session._get_ssh() + resource = target.get_resource(NetworkService) + + assert resource.address == "192.0.2.10" + assert resource.username == "" + session._get_driver_or_new.assert_called_once_with(target, "SSHDriver", name=None) + assert result is driver + def test_place_show(place): with pexpect.spawn('python -m labgrid.remote.client -p test show') as spawn: spawn.expect("Place 'test':") diff --git a/tests/test_sshdriver.py b/tests/test_sshdriver.py index 4c233a834..06f0f2b51 100644 --- a/tests/test_sshdriver.py +++ b/tests/test_sshdriver.py @@ -80,6 +80,62 @@ def test_custom_tools(target, tmpdir): s = SSHDriver(target, "ssh") assert [s._ssh, s._scp, s._sshfs, s._rsync] == [f"/path/to/{t}" for t in ("ssh", "scp", "sshfs", "rsync")] +def test_run_no_username(target, mocker): + NetworkService(target, "service", "1.2.3.4") + mocker.patch('os.path.exists', return_value=True) + mocker.patch('subprocess.call', return_value=0) + popen = mocker.patch('subprocess.Popen', autospec=True) + master = mocker.MagicMock() + master.wait = mocker.MagicMock(return_value=0) + master.communicate = mocker.MagicMock(return_value=(b"", b"")) + keepalive = mocker.MagicMock() + keepalive.poll = mocker.MagicMock(return_value=None) + keepalive.communicate = mocker.MagicMock(return_value=("", "")) + run = mocker.MagicMock() + run.communicate = mocker.MagicMock(return_value=(b"Hello\n", b"")) + run.returncode = 0 + popen.side_effect = [ + master, + keepalive, + run, + ] + + SSHDriver(target, "ssh") + s = target.get_driver("SSHDriver") + + assert s.networkservice.username == "" + assert s.run("echo Hello") == (["Hello"], [], 0) + for call in popen.call_args_list: + assert "-l" not in call.args[0] + + target.deactivate(s) + +def test_put_get_no_username(target, mocker): + NetworkService(target, "service", "1.2.3.4") + popen = mocker.patch('subprocess.Popen', autospec=True) + mocker.patch('os.path.exists', return_value=True) + call = mocker.patch('subprocess.call', return_value=0) + master = mocker.MagicMock() + master.wait = mocker.MagicMock(return_value=0) + master.communicate = mocker.MagicMock(return_value=(b"", b"")) + keepalive = mocker.MagicMock() + keepalive.poll = mocker.MagicMock(return_value=None) + keepalive.communicate = mocker.MagicMock(return_value=("", "")) + popen.side_effect = [master, keepalive] + + SSHDriver(target, "ssh") + s = target.get_driver("SSHDriver") + + s.put("/tmp/local-file", "/tmp/remote-file") + s.get("/tmp/remote-file", "/tmp/local-file") + + assert call.call_args_list[0].args[0][-1] == "1.2.3.4:/tmp/remote-file" + assert "@" not in call.call_args_list[0].args[0][-1] + assert call.call_args_list[1].args[0][-2] == "1.2.3.4:/tmp/remote-file" + assert "@" not in call.call_args_list[1].args[0][-2] + + target.deactivate(s) + @pytest.fixture(scope='function') def ssh_localhost(target, pytestconfig): name = pytestconfig.getoption("--ssh-username")