diff --git a/coriolis/osmorphing/base.py b/coriolis/osmorphing/base.py index 912a23dd8..efc318764 100644 --- a/coriolis/osmorphing/base.py +++ b/coriolis/osmorphing/base.py @@ -5,6 +5,7 @@ import itertools import os import re +import shlex import uuid from oslo_log import log as logging @@ -250,7 +251,18 @@ def post_packages_uninstall(self, package_names): pass def set_environment(self, environment): - self._environment = environment + """Merges the given environment into the tools' own environment. + + The variables declared by the tools' constructor (such as + DEBIAN_FRONTEND for the Debian-based tools) are preserved, unless + the given environment explicitly overrides them. + + The variables are copied over instead of the mapping being stored + as-is, as the environment provided by the OSMount tools is shared + between the OSDetect and the OSMorphing tools of both the export + and the import providers. + """ + self._environment.update(environment or {}) class BaseLinuxOSMorphingTools(BaseOSMorphingTools): @@ -383,12 +395,12 @@ def run_user_script(self, user_script): utils.exec_ssh_cmd( self._conn, "sudo chmod +x %s" % script_path, - get_pty=True) + get_pty=False) utils.exec_ssh_cmd( self._conn, 'sudo "%s" "%s"' % (script_path, self._os_root_dir), - get_pty=True) + get_pty=False) except Exception as err: raise exception.CoriolisException( "Failed to run user script.") from err @@ -434,7 +446,7 @@ def _exec_cmd(self, cmd, timeout=None): timeout = self._osmorphing_operation_timeout try: return utils.exec_ssh_cmd( - self._ssh, cmd, environment=self._environment, get_pty=True, + self._ssh, cmd, environment=self._environment, get_pty=False, timeout=timeout) except exception.MinionMachineCommandTimeout as ex: raise exception.OSMorphingSSHOperationTimeout( @@ -446,7 +458,7 @@ def _exec_cmd_chroot(self, cmd, timeout=None): try: return utils.exec_ssh_cmd_chroot( self._ssh, self._os_root_dir, cmd, - environment=self._environment, get_pty=True, timeout=timeout) + environment=self._environment, get_pty=False, timeout=timeout) except exception.MinionMachineCommandTimeout as ex: raise exception.OSMorphingSSHOperationTimeout( cmd=cmd, timeout=timeout) from ex @@ -465,7 +477,7 @@ def _write_file_sudo(self, chroot_path, content): self._exec_cmd_chroot("cp /%s /%s" % (tmp_file, chroot_path)) self._exec_cmd_chroot("rm /%s" % tmp_file) utils.exec_ssh_cmd( - self._ssh, "sudo sync", self._environment, get_pty=True) + self._ssh, "sudo sync", self._environment, get_pty=False) def _enable_systemd_service(self, service_name): self._exec_cmd_chroot("systemctl enable %s.service" % service_name) @@ -827,20 +839,19 @@ def _validate_grub_config_obj(self, config_obj): def set_grub_value(self, option, value, config_obj, replace=True): self._validate_grub_config_obj(config_obj) + # the sed script and the config path are shell-quoted so that + # values holding spaces, quotes or leading dashes reach sed + # as a single argument instead of being split into separate options. def append_to_cfg(opt, val): - cmd = "sed -ie '$a%(o)s=\"%(v)s\"' %(cfg)s" % { - "o": opt, - "v": val, - "cfg": config_obj["location"] - } + cmd = "sed -ie %s %s" % ( + shlex.quote('$a%s="%s"' % (opt, val)), + shlex.quote(config_obj["location"])) self._exec_cmd_chroot(cmd) def replace_in_cfg(opt, val): - cmd = "sed -i 's|^%(o)s=.*|%(o)s=\"%(v)s\"|g' %(cfg)s" % { - "o": opt, - "v": val, - "cfg": config_obj["location"] - } + cmd = "sed -i %s %s" % ( + shlex.quote('s|^%s=.*|%s="%s"|g' % (opt, opt, val)), + shlex.quote(config_obj["location"])) self._exec_cmd_chroot(cmd) if config_obj["contents"].get(option, False): diff --git a/coriolis/osmorphing/debian.py b/coriolis/osmorphing/debian.py index 440cf934a..6f17bb002 100644 --- a/coriolis/osmorphing/debian.py +++ b/coriolis/osmorphing/debian.py @@ -33,6 +33,21 @@ class BaseDebianMorphingTools(base.BaseLinuxOSMorphingTools): netplan_base = "etc/netplan" + def __init__(self, conn, os_root_dir, os_root_dev, hypervisor, + event_manager, detected_os_info, osmorphing_parameters, + operation_timeout=None): + super(BaseDebianMorphingTools, self).__init__( + conn, os_root_dir, os_root_dev, hypervisor, event_manager, + detected_os_info, osmorphing_parameters, operation_timeout) + + # NOTE: every dpkg invocation may run maintainer scripts which prompt + # through debconf (e.g. asking for a keyboard layout, or acknowledging + # a pending kernel upgrade), which would hang the OSMorphing operation + # indefinitely. Both the install and the uninstall paths are affected, + # so the frontend is declared non-interactive for all the commands run + # by these tools instead of being set on individual operations. + self.set_environment({'DEBIAN_FRONTEND': 'noninteractive'}) + @classmethod def check_os_supported(cls, detected_os_info): if detected_os_info['distribution_name'] != ( @@ -242,9 +257,8 @@ def install_packages(self, package_names): self._exec_cmd_chroot(deb_reconfigure_cmd) apt_get_cmd = ( - '/bin/bash -c "DEBIAN_FRONTEND=noninteractive ' 'apt-get install %s -y ' - '-o Dpkg::Options::=\'--force-confdef\'"' % ( + '-o Dpkg::Options::=--force-confdef' % ( " ".join(package_names))) self._exec_cmd_chroot(apt_get_cmd) except Exception as err: diff --git a/coriolis/osmorphing/osdetect/base.py b/coriolis/osmorphing/osdetect/base.py index 80cb0c7a0..132ae5bfe 100644 --- a/coriolis/osmorphing/osdetect/base.py +++ b/coriolis/osmorphing/osdetect/base.py @@ -86,7 +86,7 @@ def _exec_cmd(self, cmd, timeout=None): timeout = self._osdetect_operation_timeout try: return utils.exec_ssh_cmd( - self._conn, cmd, environment=self._environment, get_pty=True, + self._conn, cmd, environment=self._environment, get_pty=False, timeout=timeout) except exception.MinionMachineCommandTimeout as ex: raise exception.OSMorphingSSHOperationTimeout( @@ -98,7 +98,7 @@ def _exec_cmd_chroot(self, cmd, timeout=None): try: return utils.exec_ssh_cmd_chroot( self._conn, self._os_root_dir, cmd, - environment=self._environment, get_pty=True, timeout=timeout) + environment=self._environment, get_pty=False, timeout=timeout) except exception.MinionMachineCommandTimeout as ex: raise exception.OSMorphingSSHOperationTimeout( cmd=cmd, timeout=timeout) from ex diff --git a/coriolis/osmorphing/osmount/base.py b/coriolis/osmorphing/osmount/base.py index 1d6f824cc..d3c4957a6 100644 --- a/coriolis/osmorphing/osmount/base.py +++ b/coriolis/osmorphing/osmount/base.py @@ -102,6 +102,7 @@ def _connect(self): self._ssh = ssh def setup(self): + utils.check_env_command(self._ssh) if self._allow_ssh_env_vars(): self._ssh.close() self._connect() @@ -123,11 +124,36 @@ def _exec_cmd(self, cmd, timeout=None): timeout = self._osmount_operation_timeout try: return utils.exec_ssh_cmd(self._ssh, cmd, self._environment, - get_pty=True, timeout=timeout) + get_pty=False, timeout=timeout) except exception.MinionMachineCommandTimeout as ex: raise exception.OSMorphingSSHOperationTimeout( cmd=cmd, timeout=timeout) from ex + def _exec_sudo_env_cmd(self, cmd, timeout=None): + """ + Runs a sudo command that also passes all the environment variables to + the underlying command. Replaces sudo's -E flag, which is not currently + supported in all shipped sudo variants (like sudo-rs). + """ + + if not timeout: + timeout = self._osmount_operation_timeout + env_cmd = "sudo %s%s" % ( + utils.get_env_command_prefix(self._environment), cmd) + try: + return utils.exec_ssh_cmd( + self._ssh, + env_cmd, + environment=self._environment, + get_pty=False, + timeout=timeout, + ) + except exception.MinionMachineCommandTimeout as ex: + raise exception.OSMorphingSSHOperationTimeout( + cmd=cmd, + timeout=timeout, + ) from ex + def get_connection(self): return self._ssh @@ -589,6 +615,11 @@ def _find_and_mount_root(self, devices): self._exec_cmd( 'sudo mount -o bind /%(dir)s/ %(mount_dir)s' % {'dir': directory, 'mount_dir': mount_dir}) + # NOTE: the bind above joins the minion's peer group, so a unit + # restarting on the minion during OSMorphing has its per-unit + # '/run/credentials/' mount show up under the bind as well, + # which would later fail the 'umount -R'. + self._exec_cmd('sudo mount --make-private %s' % mount_dir) self._mask_minion_efi_firmware(os_root_dir) @@ -690,6 +721,15 @@ def mount_os(self): def dismount_os(self, root_dir): self._exec_cmd('sudo fuser --kill --mount %s || true' % root_dir) + # NOTE: the binds are made private as they are made, but a mount can + # still propagate in before that happens, and chroots mounted by an + # older worker were never severed at all. Nothing may be left shared: + # unmounting below must not propagate back out to the minion's own + # mounts and fail with `target is busy` on one which is still in + # use there. + self._exec_cmd( + 'mountpoint -q %s && sudo mount --make-rprivate %s || true' % ( + root_dir, root_dir)) self._exec_cmd( 'mountpoint -q %s && sudo umount -R %s' % (root_dir, root_dir)) @@ -734,12 +774,12 @@ def run_user_script(self, user_script): utils.exec_ssh_cmd( self._ssh, "sudo chmod +x %s" % script_path, - get_pty=True) + get_pty=False) utils.exec_ssh_cmd( self._ssh, f'sudo "{script_path}"', - get_pty=True) + get_pty=False) except Exception as err: raise exception.CoriolisException( "Failed to run user script.") from err diff --git a/coriolis/osmorphing/osmount/redhat.py b/coriolis/osmorphing/osmount/redhat.py index 87fe16792..98c68d13e 100644 --- a/coriolis/osmorphing/osmount/redhat.py +++ b/coriolis/osmorphing/osmount/redhat.py @@ -20,7 +20,7 @@ def check_os(self): def setup(self): super(RedHatOSMountTools, self).setup() - self._exec_cmd("sudo -E yum install -y lvm2 psmisc cryptsetup") + self._exec_sudo_env_cmd("yum install -y lvm2 psmisc cryptsetup") self._exec_cmd("sudo modprobe dm-mod") self._exec_cmd("sudo modprobe dm-crypt") self._exec_cmd("sudo rm -f /etc/lvm/devices/system.devices") diff --git a/coriolis/osmorphing/osmount/suse.py b/coriolis/osmorphing/osmount/suse.py index 15f53ada0..273b74955 100644 --- a/coriolis/osmorphing/osmount/suse.py +++ b/coriolis/osmorphing/osmount/suse.py @@ -39,9 +39,9 @@ def _allow_ssh_env_vars(self): def setup(self): super(SUSEOSMountTools, self).setup() retry_ssh_cmd = utils.retry_on_error( - max_attempts=10, sleep_seconds=30)(self._exec_cmd) + max_attempts=10, sleep_seconds=30)(self._exec_sudo_env_cmd) retry_ssh_cmd( - "sudo -E zypper --non-interactive install lvm2 psmisc cryptsetup") + "zypper --non-interactive install lvm2 psmisc cryptsetup") self._exec_cmd("sudo modprobe dm-mod") self._exec_cmd("sudo modprobe dm-crypt") self._exec_cmd("sudo rm -f /etc/lvm/devices/system.devices") diff --git a/coriolis/osmorphing/osmount/ubuntu.py b/coriolis/osmorphing/osmount/ubuntu.py index 47f8f11fe..4a498a460 100644 --- a/coriolis/osmorphing/osmount/ubuntu.py +++ b/coriolis/osmorphing/osmount/ubuntu.py @@ -25,8 +25,8 @@ def setup(self): # Apart from relying on possibly not-yet-installed tools like `fuser`, # or checking every /proc/*/fd ourselves, we simply retry it: retry_ssh_cmd = utils.retry_on_error( - max_attempts=10, sleep_seconds=30)(self._exec_cmd) - retry_ssh_cmd("sudo -E apt-get update -y") + max_attempts=10, sleep_seconds=30)(self._exec_sudo_env_cmd) + retry_ssh_cmd("apt-get update -y") # NOTE(aznashwan): in case an unattended upgrade is already happening # and is at the package installation stage (in which case the @@ -35,9 +35,10 @@ def setup(self): # prompts interactively for a keyboard layout unless # DEBIAN_FRONTEND=noninteractive is set, which would otherwise hang # the install indefinitely. - self._exec_cmd( - "sudo -E DEBIAN_FRONTEND=noninteractive apt-get " - "-o DPkg::Lock::Timeout=600 install lvm2 psmisc cryptsetup -y") + self._environment['DEBIAN_FRONTEND'] = 'noninteractive' + self._exec_sudo_env_cmd( + "apt-get -o DPkg::Lock::Timeout=600 " + "install lvm2 psmisc cryptsetup -y") self._exec_cmd("sudo modprobe dm-mod") self._exec_cmd("sudo modprobe dm-crypt") diff --git a/coriolis/providers/backup_writers.py b/coriolis/providers/backup_writers.py index 4cc9a41ad..2e8c237d2 100644 --- a/coriolis/providers/backup_writers.py +++ b/coriolis/providers/backup_writers.py @@ -100,7 +100,7 @@ def _disable_lvm2_lvmetad(ssh): utils.exec_ssh_cmd( ssh, 'sudo sed -i "s/use_lvmetad.*=.*1/use_lvmetad = 0/g" ' - '%s' % cfg, get_pty=True) + '%s' % cfg, get_pty=False) # NOTE: lvm2-lvmetad is the name of the lvmetad service # on both debian and RHEL based systems. It needs to be stopped # before we begin disk replication. We disable it in the config @@ -108,12 +108,12 @@ def _disable_lvm2_lvmetad(ssh): # a dependency. As the service may not actually exist, even though # the config is present, we ignore errors when stopping it. utils.ignore_exceptions(utils.exec_ssh_cmd)( - ssh, "sudo service lvm2-lvmetad stop", get_pty=True) + ssh, "sudo service lvm2-lvmetad stop", get_pty=False) # disable volume groups. Any volume groups that have volumes in use # will remain online. However, volume groups belonging to disks # that have been synced at least once, will be deactivated. utils.ignore_exceptions(utils.exec_ssh_cmd)( - ssh, "sudo vgchange -an", get_pty=True) + ssh, "sudo vgchange -an", get_pty=False) def _disable_lvm_metad_udev_rule(ssh): @@ -131,7 +131,7 @@ def _disable_lvm_metad_udev_rule(ssh): ] for path in rule_paths: if utils.test_ssh_path(ssh, path): - utils.exec_ssh_cmd(ssh, "sudo rm %s" % path, get_pty=True) + utils.exec_ssh_cmd(ssh, "sudo rm %s" % path, get_pty=False) def _check_deserialize_key(key): @@ -1021,7 +1021,7 @@ def _inject_dport_allow_rule(self, ssh): "sudo iptables -I INPUT -p tcp --dport %(port)s -j ACCEPT" % { "port": self._writer_port}) try: - utils.exec_ssh_cmd(ssh, cmd, get_pty=True) + utils.exec_ssh_cmd(ssh, cmd, get_pty=False) except exception.CoriolisException: LOG.warn( "Could not inject TCP FW rule. Error was: %s", @@ -1030,7 +1030,7 @@ def _inject_dport_allow_rule(self, ssh): def _add_firewalld_port(self, ssh): cmd = "sudo firewall-cmd --add-port=%s/tcp" % self._writer_port try: - utils.exec_ssh_cmd(ssh, cmd, get_pty=True) + utils.exec_ssh_cmd(ssh, cmd, get_pty=False) except exception.CoriolisException: LOG.warn("Could not add TCP port to firewalld. Error was: %s", utils.get_exception_details()) @@ -1038,7 +1038,7 @@ def _add_firewalld_port(self, ssh): def _change_binary_se_context(self, ssh): cmd = "sudo chcon -t bin_t %s" % self._writer_cmd try: - utils.exec_ssh_cmd(ssh, cmd, get_pty=True) + utils.exec_ssh_cmd(ssh, cmd, get_pty=False) except exception.CoriolisException: LOG.warn("Could not change SELinux context of writer binary. " "Error was:%s", utils.get_exception_details()) @@ -1061,12 +1061,12 @@ def _copy_writer(self, ssh): ssh, "sudo mv %s %s" % ( remote_tmp_path, self._writer_cmd), - get_pty=True + get_pty=False ) utils.exec_ssh_cmd( ssh, "sudo chmod +x %s" % self._writer_cmd, - get_pty=True + get_pty=False ) finally: sftp.close() @@ -1075,7 +1075,7 @@ def _fetch_remote_file(self, ssh, remote_file, local_file): with open(local_file, 'wb') as fd: utils.exec_ssh_cmd( ssh, - "sudo chmod +r %s" % remote_file, get_pty=True) + "sudo chmod +r %s" % remote_file, get_pty=False) data = utils.retry_on_error()( utils.read_ssh_file)(ssh, remote_file) fd.write(data) @@ -1103,7 +1103,7 @@ def _setup_certificates(self, ssh): if not all(exist): utils.exec_ssh_cmd( - ssh, "sudo mkdir -p %s" % remote_base_dir, get_pty=True) + ssh, "sudo mkdir -p %s" % remote_base_dir, get_pty=False) utils.exec_ssh_cmd( ssh, "sudo %(writer_cmd)s generate-certificates -output-dir " @@ -1112,7 +1112,7 @@ def _setup_certificates(self, ssh): "cert_dir": remote_base_dir, "extra_hosts": self._ip, }, - get_pty=True) + get_pty=False) return { "srv_crt": remote_srv_crt, @@ -1124,7 +1124,7 @@ def _setup_certificates(self, ssh): def _read_remote_file_sudo(self, remote_path): contents = utils.exec_ssh_cmd( - self._ssh, 'sudo cat "%s"' % remote_path, get_pty=True) + self._ssh, 'sudo cat "%s"' % remote_path, get_pty=False) return contents def _init_writer(self, ssh, cert_paths): diff --git a/coriolis/providers/replicator.py b/coriolis/providers/replicator.py index 06815eb3a..763f6ac2f 100644 --- a/coriolis/providers/replicator.py +++ b/coriolis/providers/replicator.py @@ -567,7 +567,7 @@ def _copy_file(self, ssh, localPath, remotePath): sftp = paramiko.SFTPClient.from_transport(ssh.get_transport()) sftp.put(localPath, tmp) utils.exec_ssh_cmd( - ssh, "sudo mv %s %s" % (tmp, remotePath), get_pty=True) + ssh, "sudo mv %s %s" % (tmp, remotePath), get_pty=False) sftp.close() def _copy_replicator_cmd(self, ssh): @@ -575,7 +575,7 @@ def _copy_replicator_cmd(self, ssh): utils.get_resources_bin_dir(), 'replicator') self._copy_file(ssh, local_path, REPLICATOR_PATH) utils.exec_ssh_cmd( - ssh, "sudo chmod +x %s" % REPLICATOR_PATH, get_pty=True) + ssh, "sudo chmod +x %s" % REPLICATOR_PATH, get_pty=False) def _setup_replicator_group(self, ssh, group_name=REPLICATOR_GROUP_NAME): """ Sets up a group with the given name and adds the @@ -589,14 +589,14 @@ def _setup_replicator_group(self, ssh, group_name=REPLICATOR_GROUP_NAME): "group": REPLICATOR_GROUP_NAME}) if int(group_exists) == 0: utils.exec_ssh_cmd( - ssh, "sudo groupadd %s" % group_name, get_pty=True) + ssh, "sudo groupadd %s" % group_name, get_pty=False) # NOTE: this is required in order for the user we connected # as to be able to read the certs: # NOTE2: the group change will only take effect after we reconnect: utils.exec_ssh_cmd( ssh, "sudo usermod -aG %s %s" % ( REPLICATOR_GROUP_NAME, self._conn_info['username']), - get_pty=True) + get_pty=False) return int(group_exists) == 1 @@ -610,10 +610,10 @@ def _setup_replicator_user(self, ssh): utils.exec_ssh_cmd( ssh, "sudo useradd -m -s /bin/bash -g %s %s" % ( REPLICATOR_GROUP_NAME, REPLICATOR_USERNAME), - get_pty=True) + get_pty=False) utils.exec_ssh_cmd( ssh, "sudo usermod -aG disk %s" % REPLICATOR_USERNAME, - get_pty=True) + get_pty=False) def _exec_replicator(self, ssh, port, certs, state_file): cmdline = ("%(replicator_path)s run -hash-method=%(hash_method)s " @@ -682,7 +682,7 @@ def _setup_certificates(self, ssh, args): force_fetch = False if not all(exist): utils.exec_ssh_cmd( - ssh, "sudo mkdir -p %s" % remote_base_dir, get_pty=True) + ssh, "sudo mkdir -p %s" % remote_base_dir, get_pty=False) utils.exec_ssh_cmd( ssh, "sudo %(replicator_cmd)s gen-certs -output-dir " @@ -691,17 +691,17 @@ def _setup_certificates(self, ssh, args): "cert_dir": remote_base_dir, "extra_hosts": ip, }, - get_pty=True) + get_pty=False) utils.exec_ssh_cmd( ssh, "sudo chown -R %(user)s:%(group)s %(cert_dir)s" % { "cert_dir": remote_base_dir, "user": REPLICATOR_USERNAME, "group": REPLICATOR_GROUP_NAME - }, get_pty=True) + }, get_pty=False) utils.exec_ssh_cmd( ssh, "sudo chmod -R g+r %(cert_dir)s" % { "cert_dir": remote_base_dir, - }, get_pty=True) + }, get_pty=False) force_fetch = True exists = [] @@ -731,7 +731,7 @@ def _setup_certificates(self, ssh, args): def _change_binary_se_context(self, ssh): cmd = "sudo chcon -t bin_t %s" % REPLICATOR_PATH try: - utils.exec_ssh_cmd(ssh, cmd, get_pty=True) + utils.exec_ssh_cmd(ssh, cmd, get_pty=False) except exception.CoriolisException: LOG.warn("Could not change SELinux context of replicator binary. " "Error was:%s", utils.get_exception_details()) @@ -742,7 +742,7 @@ def _setup_replicator(self, ssh): state_file = self._get_replicator_state_file() self._copy_file(ssh, state_file, REPLICATOR_STATE) utils.exec_ssh_cmd( - ssh, "sudo chmod 755 %s" % REPLICATOR_STATE, get_pty=True) + ssh, "sudo chmod 755 %s" % REPLICATOR_STATE, get_pty=False) os.remove(state_file) args = self._parse_replicator_conn_info(self._conn_info) diff --git a/coriolis/tests/osmorphing/osdetect/test_base.py b/coriolis/tests/osmorphing/osdetect/test_base.py index d82db36ef..963dd7871 100644 --- a/coriolis/tests/osmorphing/osdetect/test_base.py +++ b/coriolis/tests/osmorphing/osdetect/test_base.py @@ -118,7 +118,7 @@ def test__exec_cmd(self, mock_exec_ssh_cmd): mock_exec_ssh_cmd.assert_called_once_with( self.base_os_detect._conn, mock.sentinel.cmd, - environment=self.base_os_detect._environment, get_pty=True, + environment=self.base_os_detect._environment, get_pty=False, timeout=120) self.assertEqual(result, mock_exec_ssh_cmd.return_value) @@ -129,7 +129,7 @@ def test__exec_cmd_without_timeout(self, mock_exec_ssh_cmd): mock_exec_ssh_cmd.assert_called_once_with( self.base_os_detect._conn, mock.sentinel.cmd, - environment=self.base_os_detect._environment, get_pty=True, + environment=self.base_os_detect._environment, get_pty=False, timeout=self.base_os_detect._osdetect_operation_timeout) self.assertEqual(result, mock_exec_ssh_cmd.return_value) @@ -152,7 +152,7 @@ def test__exec_cmd_chroot(self, mock_exec_ssh_cmd_chroot): mock_exec_ssh_cmd_chroot.assert_called_once_with( self.base_os_detect._conn, self.base_os_detect._os_root_dir, mock.sentinel.cmd, environment=self.base_os_detect._environment, - get_pty=True, timeout=120) + get_pty=False, timeout=120) self.assertEqual(result, mock_exec_ssh_cmd_chroot.return_value) @@ -163,7 +163,7 @@ def test__exec_cmd_chroot_without_timeout(self, mock_exec_ssh_cmd_chroot): mock_exec_ssh_cmd_chroot.assert_called_once_with( self.base_os_detect._conn, self.base_os_detect._os_root_dir, mock.sentinel.cmd, environment=self.base_os_detect._environment, - get_pty=True, + get_pty=False, timeout=self.base_os_detect._osdetect_operation_timeout) self.assertEqual(result, mock_exec_ssh_cmd_chroot.return_value) diff --git a/coriolis/tests/osmorphing/osmount/test_base.py b/coriolis/tests/osmorphing/osmount/test_base.py index c4c769115..267d17ac6 100644 --- a/coriolis/tests/osmorphing/osmount/test_base.py +++ b/coriolis/tests/osmorphing/osmount/test_base.py @@ -2,6 +2,7 @@ # All Rights Reserved. import logging +import shlex from unittest import mock from coriolis import constants @@ -109,20 +110,39 @@ def test__connect( self.conn_info['ip'], 22) ) + @mock.patch.object(base.utils, 'check_env_command') @mock.patch.object(base.BaseSSHOSMountTools, '_allow_ssh_env_vars') @mock.patch.object(base.BaseSSHOSMountTools, '_connect') - def test_setup(self, mock_connect, mock_allow_ssh_vars): + def test_setup(self, mock_connect, mock_allow_ssh_vars, + mock_check_env_command): self.base_os_mount_tools.setup() mock_allow_ssh_vars.return_value = True + mock_check_env_command.assert_called_once_with(self.ssh) self.ssh.close.assert_called_once_with() mock_connect.assert_called_once_with() + @mock.patch.object(base.utils, 'check_env_command') + @mock.patch.object(base.BaseSSHOSMountTools, '_allow_ssh_env_vars') + @mock.patch.object(base.BaseSSHOSMountTools, '_connect') + def test_setup_without_env_command( + self, mock_connect, mock_allow_ssh_vars, mock_check_env_command): + # NOTE: env(1) carries the environment variables over to the + # privileged and chrooted commands, so its absence must abort the + # setup instead of silently dropping the proxy settings later on. + mock_check_env_command.side_effect = exception.CoriolisException( + "env is unavailable") + + self.assertRaises( + exception.CoriolisException, self.base_os_mount_tools.setup) + + mock_allow_ssh_vars.assert_not_called() + @mock.patch.object(base.utils, 'exec_ssh_cmd') def test__exec_cmd(self, mock_exec_ssh_cmd): result = self.base_os_mount_tools._exec_cmd(self.cmd, timeout=120) mock_exec_ssh_cmd.assert_called_once_with( - self.base_os_mount_tools._ssh, self.cmd, {}, get_pty=True, + self.base_os_mount_tools._ssh, self.cmd, {}, get_pty=False, timeout=120) self.assertEqual(result, mock_exec_ssh_cmd.return_value) @@ -132,7 +152,7 @@ def test__exec_cmd_without_timeout(self, mock_exec_ssh_cmd): result = self.base_os_mount_tools._exec_cmd(self.cmd) mock_exec_ssh_cmd.assert_called_once_with( - self.base_os_mount_tools._ssh, self.cmd, {}, get_pty=True, + self.base_os_mount_tools._ssh, self.cmd, {}, get_pty=False, timeout=self.base_os_mount_tools._osmount_operation_timeout) self.assertEqual(result, mock_exec_ssh_cmd.return_value) @@ -146,6 +166,96 @@ def test__exec_cmd_with_exception(self, mock_exec_ssh_cmd): self.base_os_mount_tools._exec_cmd, self.cmd, timeout=self.base_os_mount_tools._osmount_operation_timeout) + @mock.patch.object(base.utils, 'exec_ssh_cmd') + def test__exec_sudo_env_cmd_no_environment(self, mock_exec_ssh_cmd): + """With no proxy set, the command stays a plain 'sudo '.""" + result = self.base_os_mount_tools._exec_sudo_env_cmd( + "apt-get update -y", timeout=120) + + mock_exec_ssh_cmd.assert_called_once_with( + self.base_os_mount_tools._ssh, "sudo apt-get update -y", + environment={}, get_pty=False, timeout=120) + self.assertEqual(result, mock_exec_ssh_cmd.return_value) + + @mock.patch.object(base.utils, 'exec_ssh_cmd') + def test__exec_sudo_env_cmd_with_proxy(self, mock_exec_ssh_cmd): + """Proxy vars go through env(1), never through 'sudo -E'. + + sudo-rs, the default on Ubuntu 26.04, does not implement '-E'. + """ + environment = { + "http_proxy": "http://10.0.0.1:3128", + "HTTPS_PROXY": "http://10.0.0.1:3128", + } + self.base_os_mount_tools._environment = environment + + self.base_os_mount_tools._exec_sudo_env_cmd("apt-get update -y") + + cmd = mock_exec_ssh_cmd.call_args[0][1] + self.assertEqual( + "sudo env http_proxy=http://10.0.0.1:3128 " + "HTTPS_PROXY=http://10.0.0.1:3128 apt-get update -y", cmd) + self.assertNotIn("sudo -E", cmd) + mock_exec_ssh_cmd.assert_called_once_with( + self.base_os_mount_tools._ssh, cmd, environment=environment, + get_pty=False, + timeout=self.base_os_mount_tools._osmount_operation_timeout) + + @mock.patch.object(base.utils, 'exec_ssh_cmd') + def test__exec_sudo_env_cmd_non_proxy_environment(self, mock_exec_ssh_cmd): + """Any variable on the environment goes through env(1).""" + self.base_os_mount_tools._environment = { + "http_proxy": "http://10.0.0.1:3128", + "DEBIAN_FRONTEND": "noninteractive"} + + self.base_os_mount_tools._exec_sudo_env_cmd( + "apt-get install cryptsetup -y") + + self.assertEqual( + "sudo env http_proxy=http://10.0.0.1:3128 " + "DEBIAN_FRONTEND=noninteractive apt-get install cryptsetup -y", + mock_exec_ssh_cmd.call_args[0][1]) + + @mock.patch.object(base.utils, 'exec_ssh_cmd') + def test__exec_sudo_env_cmd_without_proxy(self, mock_exec_ssh_cmd): + """A lone variable must still go through env(1), not bare sudo.""" + self.base_os_mount_tools._environment = { + "DEBIAN_FRONTEND": "noninteractive"} + + self.base_os_mount_tools._exec_sudo_env_cmd( + "apt-get install cryptsetup -y") + + self.assertEqual( + "sudo env DEBIAN_FRONTEND=noninteractive " + "apt-get install cryptsetup -y", + mock_exec_ssh_cmd.call_args[0][1]) + + @mock.patch.object(base.utils, 'exec_ssh_cmd') + def test__exec_sudo_env_cmd_quotes_sensitive_proxy_values( + self, mock_exec_ssh_cmd): + proxy = "http://user:p@ss w0rd@10.0.0.1:3128?a=1&b=2" + self.base_os_mount_tools._environment = {"http_proxy": proxy} + + self.base_os_mount_tools._exec_sudo_env_cmd("apt-get update -y") + + self.assertEqual( + ["sudo", "env", "http_proxy=%s" % proxy, "apt-get", "update", + "-y"], + shlex.split(mock_exec_ssh_cmd.call_args[0][1])) + + @mock.patch.object(base.utils, 'exec_ssh_cmd') + def test__exec_sudo_env_cmd_timeout_does_not_leak_credentials( + self, mock_exec_ssh_cmd): + mock_exec_ssh_cmd.side_effect = exception.MinionMachineCommandTimeout() + self.base_os_mount_tools._environment = { + "http_proxy": "http://user:secret@10.0.0.1:3128"} + + exc = self.assertRaises( + exception.OSMorphingSSHOperationTimeout, + self.base_os_mount_tools._exec_sudo_env_cmd, "apt-get update -y") + + self.assertNotIn("secret", str(exc)) + class TestBaseLinuxOSMountTools(base.BaseLinuxOSMountTools): def check_os(self): @@ -824,8 +934,11 @@ def test__find_and_mount_root(self, mock_find_dev_with_contents, mock.call("mktemp -d"), mock.call("sudo mount /dev/sda /tmp/tmp_dir"), mock.call("sudo mount -o bind /proc/ /tmp/tmp_dir/proc"), + mock.call("sudo mount --make-private /tmp/tmp_dir/proc"), mock.call("sudo mount -o bind /dev/ /tmp/tmp_dir/dev"), + mock.call("sudo mount --make-private /tmp/tmp_dir/dev"), mock.call("sudo mount -o bind /run/ /tmp/tmp_dir/run"), + mock.call("sudo mount --make-private /tmp/tmp_dir/run"), ]) mock_find_dev_with_contents.assert_called_once_with( ["/dev/sdb"], all_files=self.all_files) @@ -1110,10 +1223,52 @@ def test_dismount_os(self, mock_exec_cmd): mock_exec_cmd.assert_has_calls([ mock.call("sudo fuser --kill --mount /mnt/root_dir || true"), + mock.call( + "mountpoint -q /mnt/root_dir && sudo mount --make-rprivate " + "/mnt/root_dir || true"), mock.call( "mountpoint -q /mnt/root_dir && sudo umount -R /mnt/root_dir"), ]) + @mock.patch.object(base.utils, 'test_ssh_path') + @mock.patch.object(base.BaseSSHOSMountTools, '_exec_cmd') + @mock.patch.object(base.BaseLinuxOSMountTools, '_find_dev_with_contents') + def test__find_and_mount_root_severs_mount_propagation( + self, mock_find_dev_with_contents, mock_exec_cmd, + mock_test_ssh_path): + mock_exec_cmd.return_value = "/tmp/tmp_dir" + mock_find_dev_with_contents.return_value = "/dev/sda" + mock_test_ssh_path.return_value = True + + self.base_os_mount_tools._find_and_mount_root(["/dev/sda"]) + + issued = [call.args[0] for call in mock_exec_cmd.call_args_list] + for directory in ['proc', 'sys', 'dev', 'run']: + mount_dir = "/tmp/tmp_dir/%s" % directory + bind_cmd = "sudo mount -o bind /%s/ %s" % (directory, mount_dir) + private_cmd = "sudo mount --make-private %s" % mount_dir + + self.assertIn(bind_cmd, issued) + self.assertIn(private_cmd, issued) + self.assertEqual( + issued.index(private_cmd), issued.index(bind_cmd) + 1) + + @mock.patch.object(base.BaseSSHOSMountTools, '_exec_cmd') + def test_dismount_os_severs_mount_propagation_before_unmounting( + self, mock_exec_cmd): + self.base_os_mount_tools.dismount_os("/mnt/root_dir") + + issued = [call.args[0] for call in mock_exec_cmd.call_args_list] + rprivate_cmd = ( + "mountpoint -q /mnt/root_dir && sudo mount --make-rprivate " + "/mnt/root_dir || true") + umount_cmd = ( + "mountpoint -q /mnt/root_dir && sudo umount -R /mnt/root_dir") + + self.assertIn(rprivate_cmd, issued) + self.assertIn(umount_cmd, issued) + self.assertLess(issued.index(rprivate_cmd), issued.index(umount_cmd)) + @mock.patch.object(base.utils, 'get_url_with_credentials') def test_set_proxy(self, mock_get_url_with_credentials): proxy_settings = { @@ -1144,3 +1299,15 @@ def test_set_proxy_no_url(self, mock_get_url_with_credentials): self.assertIsNone(result) mock_get_url_with_credentials.assert_not_called() + self.assertEqual({}, self.base_os_mount_tools._environment) + + def test_set_proxy_sets_lower_and_uppercase_variables(self): + url = "http://10.0.0.1:3128" + + self.base_os_mount_tools.set_proxy({'url': url}) + + self.assertEqual( + {'http_proxy': url, 'HTTP_PROXY': url, + 'https_proxy': url, 'HTTPS_PROXY': url, + 'ftp_proxy': url, 'FTP_PROXY': url}, + self.base_os_mount_tools._environment) diff --git a/coriolis/tests/osmorphing/osmount/test_redhat.py b/coriolis/tests/osmorphing/osmount/test_redhat.py index a8fbe3850..3efbbc5c9 100644 --- a/coriolis/tests/osmorphing/osmount/test_redhat.py +++ b/coriolis/tests/osmorphing/osmount/test_redhat.py @@ -31,15 +31,17 @@ def test_check_os(self, mock_get_linux_os_info): result = self.tools.check_os() self.assertTrue(result) + @mock.patch.object(redhat.base.BaseSSHOSMountTools, '_exec_sudo_env_cmd') @mock.patch.object(redhat.base.BaseSSHOSMountTools, '_exec_cmd') @mock.patch.object(redhat.base.BaseSSHOSMountTools, 'setup') - def test_setup(self, mock_setup, mock_exec_cmd): + def test_setup(self, mock_setup, mock_exec_cmd, mock_exec_sudo_env_cmd): result = self.tools.setup() self.assertIsNone(result) mock_setup.assert_called_once_with() + mock_exec_sudo_env_cmd.assert_called_once_with( + "yum install -y lvm2 psmisc cryptsetup") mock_exec_cmd.assert_has_calls([ - mock.call("sudo -E yum install -y lvm2 psmisc cryptsetup"), mock.call("sudo modprobe dm-mod"), mock.call("sudo modprobe dm-crypt") ]) diff --git a/coriolis/tests/osmorphing/osmount/test_suse.py b/coriolis/tests/osmorphing/osmount/test_suse.py index 2a2dc21df..9c97f7181 100644 --- a/coriolis/tests/osmorphing/osmount/test_suse.py +++ b/coriolis/tests/osmorphing/osmount/test_suse.py @@ -53,9 +53,11 @@ def test_check_os_not_suse(self, mock_get_linux_os_info): self.assertIsNone(result) @mock.patch.object(suse.utils, 'retry_on_error') + @mock.patch.object(suse.base.BaseSSHOSMountTools, '_exec_sudo_env_cmd') @mock.patch.object(suse.base.BaseSSHOSMountTools, '_exec_cmd') @mock.patch.object(suse.base.BaseSSHOSMountTools, 'setup') - def test_setup(self, mock_setup, mock_exec_cmd, mock_retry_on_error): + def test_setup(self, mock_setup, mock_exec_cmd, mock_exec_sudo_env_cmd, + mock_retry_on_error): mock_retry_on_error.return_value = lambda f: f result = self.tools.setup() self.assertIsNone(result) @@ -63,10 +65,9 @@ def test_setup(self, mock_setup, mock_exec_cmd, mock_retry_on_error): mock_setup.assert_called_once_with() mock_retry_on_error.assert_called_once_with( max_attempts=10, sleep_seconds=30) + mock_exec_sudo_env_cmd.assert_called_once_with( + "zypper --non-interactive install lvm2 psmisc cryptsetup") mock_exec_cmd.assert_has_calls([ - mock.call( - "sudo -E zypper --non-interactive install " - "lvm2 psmisc cryptsetup"), mock.call("sudo modprobe dm-mod"), mock.call("sudo modprobe dm-crypt"), mock.call("sudo rm -f /etc/lvm/devices/system.devices") diff --git a/coriolis/tests/osmorphing/osmount/test_ubuntu.py b/coriolis/tests/osmorphing/osmount/test_ubuntu.py index 8875aacb6..d37b6c1fb 100644 --- a/coriolis/tests/osmorphing/osmount/test_ubuntu.py +++ b/coriolis/tests/osmorphing/osmount/test_ubuntu.py @@ -30,22 +30,52 @@ def test_check_os(self, mock_get_linux_os_info): result = self.tools.check_os() self.assertTrue(result) + @mock.patch.object(ubuntu.base.BaseSSHOSMountTools, '_exec_sudo_env_cmd') @mock.patch.object(ubuntu.base.BaseSSHOSMountTools, '_exec_cmd') @mock.patch.object(ubuntu.base.BaseSSHOSMountTools, 'setup') - def test_setup(self, mock_setup, mock_exec_cmd): + def test_setup(self, mock_setup, mock_exec_cmd, mock_exec_sudo_env_cmd): result = self.tools.setup() self.assertIsNone(result) mock_setup.assert_called_once_with() + # NOTE: the apt-get calls must go through '_exec_sudo_env_cmd' so + # that any configured proxy reaches apt without relying on 'sudo -E', + # which sudo-rs (the default on Ubuntu 26.04) does not support. + mock_exec_sudo_env_cmd.assert_has_calls([ + mock.call("apt-get update -y"), + mock.call("apt-get -o DPkg::Lock::Timeout=600 " + "install lvm2 psmisc cryptsetup -y"), + ]) + # NOTE: cryptsetup pulls in keyboard-configuration, whose postinst + # would otherwise prompt for a keyboard layout and hang the install. + self.assertEqual( + 'noninteractive', self.tools._environment['DEBIAN_FRONTEND']) mock_exec_cmd.assert_has_calls([ - mock.call("sudo -E apt-get update -y"), - mock.call("sudo -E DEBIAN_FRONTEND=noninteractive apt-get " - "-o DPkg::Lock::Timeout=600 install lvm2 psmisc " - "cryptsetup -y"), mock.call("sudo modprobe dm-mod"), mock.call("sudo modprobe dm-crypt") ]) + @mock.patch.object(ubuntu.utils, 'exec_ssh_cmd') + @mock.patch.object(ubuntu.base.BaseSSHOSMountTools, 'setup') + def test_setup_propagates_proxy_to_apt(self, mock_setup, mock_exec_ssh): + """End-to-end check of proxy propagation on an Ubuntu worker.""" + proxy = "http://10.0.0.1:3128" + self.tools.set_proxy({'url': proxy}) + + self.tools.setup() + + apt_cmds = [ + call[0][1] for call in mock_exec_ssh.call_args_list + if "apt-get" in call[0][1]] + self.assertEqual(2, len(apt_cmds)) + for cmd in apt_cmds: + self.assertTrue( + cmd.startswith("sudo env "), + "apt-get command is missing its env prefix: %s" % cmd) + for var in ('http_proxy', 'HTTP_PROXY', 'https_proxy', + 'HTTPS_PROXY', 'ftp_proxy', 'FTP_PROXY'): + self.assertIn("%s=%s" % (var, proxy), cmd) + @mock.patch.object(ubuntu.base.BaseSSHOSMountTools, '_exec_cmd') @mock.patch.object(ubuntu.utils, 'restart_service') def test__allow_ssh_env_vars(self, mock_restart_service, mock_exec_cmd): diff --git a/coriolis/tests/osmorphing/test_base.py b/coriolis/tests/osmorphing/test_base.py index 8a66747e3..5edff3d60 100644 --- a/coriolis/tests/osmorphing/test_base.py +++ b/coriolis/tests/osmorphing/test_base.py @@ -2,6 +2,7 @@ # All Rights Reserved. import logging +import shlex from unittest import mock import ddt @@ -99,9 +100,26 @@ def test_check_os_supported_not_implemented(self): ) def test_set_environment(self): - self.os_morphing_tools.set_environment(mock.sentinel.environment) + environment = {'http_proxy': 'http://10.0.0.1:3128'} + + self.os_morphing_tools.set_environment(environment) + self.assertEqual( - self.os_morphing_tools._environment, mock.sentinel.environment) + {'http_proxy': 'http://10.0.0.1:3128'}, + self.os_morphing_tools._environment) + + def test_set_environment_preserves_constructor_variables(self): + """Variables declared by the constructor survive set_environment.""" + self.os_morphing_tools._environment['DEBIAN_FRONTEND'] = ( + 'noninteractive') + + self.os_morphing_tools.set_environment( + {'http_proxy': 'http://10.0.0.1:3128'}) + + self.assertEqual( + {'DEBIAN_FRONTEND': 'noninteractive', + 'http_proxy': 'http://10.0.0.1:3128'}, + self.os_morphing_tools._environment) # This class is used to test the BaseLinuxOSMorphingTools class since it is @@ -252,10 +270,10 @@ def test_run_user_script(self, mock_exec_ssh_cmd, mock_write_ssh_file): self.conn, script_path, user_script) mock_exec_ssh_cmd.assert_has_calls([ mock.call(self.conn, "sudo chmod +x %s" % script_path, - get_pty=True), + get_pty=False), mock.call(self.conn, 'sudo "%s" "%s"' % ( script_path, self.os_morphing_tools._os_root_dir), - get_pty=True)]) + get_pty=False)]) @mock.patch.object(base.utils, 'write_ssh_file') @mock.patch.object(base.utils, 'exec_ssh_cmd') @@ -356,7 +374,7 @@ def test__exec_cmd(self, mock_exec_ssh_cmd): mock_exec_ssh_cmd.assert_called_once_with( self.os_morphing_tools._ssh, mock.sentinel.cmd, - environment=self.os_morphing_tools._environment, get_pty=True, + environment=self.os_morphing_tools._environment, get_pty=False, timeout=120) self.assertEqual(result, mock_exec_ssh_cmd.return_value) @@ -367,7 +385,7 @@ def test__exec_cmd_without_timeout(self, mock_exec_ssh_cmd): mock_exec_ssh_cmd.assert_called_once_with( self.os_morphing_tools._ssh, mock.sentinel.cmd, - environment=self.os_morphing_tools._environment, get_pty=True, + environment=self.os_morphing_tools._environment, get_pty=False, timeout=self.os_morphing_tools._osmorphing_operation_timeout) self.assertEqual(result, mock_exec_ssh_cmd.return_value) @@ -387,7 +405,7 @@ def test__exec_cmd_chroot(self, mock_exec_ssh_cmd_chroot): mock_exec_ssh_cmd_chroot.assert_called_once_with( self.os_morphing_tools._ssh, self.os_morphing_tools._os_root_dir, mock.sentinel.cmd, environment=self.os_morphing_tools._environment, - get_pty=True, timeout=120) + get_pty=False, timeout=120) self.assertEqual(result, mock_exec_ssh_cmd_chroot.return_value) @mock.patch.object(base.utils, 'exec_ssh_cmd_chroot') @@ -397,7 +415,7 @@ def test__exec_cmd_chroot_without_timeout(self, mock_exec_ssh_cmd_chroot): mock_exec_ssh_cmd_chroot.assert_called_once_with( self.os_morphing_tools._ssh, self.os_morphing_tools._os_root_dir, mock.sentinel.cmd, environment=self.os_morphing_tools._environment, - get_pty=True, + get_pty=False, timeout=self.os_morphing_tools._osmorphing_operation_timeout) self.assertEqual(result, mock_exec_ssh_cmd_chroot.return_value) @@ -410,6 +428,21 @@ def test__exec_cmd_chroot_with_exception(self, mock_exec_ssh_cmd_chroot): exception.OSMorphingSSHOperationTimeout, self.os_morphing_tools._exec_cmd_chroot, mock.sentinel.cmd) + @mock.patch.object(base.utils, 'exec_ssh_cmd_chroot') + def test__exec_cmd_chroot_environment(self, mock_exec_ssh_cmd_chroot): + """The tools' environment is handed over to the chrooted command.""" + environment = { + 'http_proxy': 'http://10.0.0.1:3128', + 'DEBIAN_FRONTEND': 'noninteractive'} + self.os_morphing_tools._environment = environment + + self.os_morphing_tools._exec_cmd_chroot(mock.sentinel.cmd) + + mock_exec_ssh_cmd_chroot.assert_called_once_with( + self.os_morphing_tools._ssh, self.os_morphing_tools._os_root_dir, + mock.sentinel.cmd, environment=environment, get_pty=False, + timeout=self.os_morphing_tools._osmorphing_operation_timeout) + @mock.patch.object(base.BaseLinuxOSMorphingTools, '_exec_cmd_chroot') def test__check_user_exists(self, mock_exec_cmd_chroot): result = self.os_morphing_tools._check_user_exists( @@ -444,7 +477,7 @@ def test__write_file_sudo(self, mock_exec_ssh_cmd, mock_uuid, mock.call('rm /tmp/%s' % mock_uuid.return_value)]) mock_exec_ssh_cmd.assert_called_once_with( self.os_morphing_tools._ssh, 'sudo sync', - self.os_morphing_tools._environment, get_pty=True) + self.os_morphing_tools._environment, get_pty=False) @mock.patch.object(base.BaseLinuxOSMorphingTools, '_exec_cmd_chroot') def test__enable_systemd_service(self, mock_exec_cmd_chroot): @@ -1316,6 +1349,60 @@ def test_set_grub_value_replace(self, mock_exec_cmd_chroot, ) mock_read_file_sudo.assert_called_once_with(config_obj['location']) + @mock.patch.object(base.BaseLinuxOSMorphingTools, '_read_file_sudo') + @mock.patch.object(base.utils, 'exec_ssh_cmd') + def test_set_grub_value_serial_command_is_a_single_sed_argument( + self, mock_exec_ssh_cmd, mock_read_file_sudo): + """Regression test for the GRUB_SERIAL_COMMAND quoting bug. + + The appended value holds spaces, double quotes and '--word=8'-style + text. It must reach sed as one single argument; previously it was + word-split into separate sed options and failed with + "sed: unrecognized option '--word=8'". + """ + self.os_morphing_tools._environment = { + 'http_proxy': 'http://10.0.0.1:3128'} + self.os_morphing_tools._os_root_dir = '/tmp/tmp.q15QdW45qE' + serial_cmd = base.GRUB2_SERIAL % (115200, "no") + config_obj = { + 'location': '/tmp/tmp.OIK95wgYUb', + 'source': '/etc/default/grub', + 'contents': {'GRUB_DEFAULT': '0'}, + } + + self.os_morphing_tools.set_grub_value( + 'GRUB_SERIAL_COMMAND', serial_cmd, config_obj) + + argv = shlex.split(mock_exec_ssh_cmd.call_args[0][1]) + self.assertEqual( + ['sudo', 'env', 'http_proxy=http://10.0.0.1:3128', 'chroot', + '/tmp/tmp.q15QdW45qE', 'sed', '-ie', + '$aGRUB_SERIAL_COMMAND="%s"' % serial_cmd, + '/tmp/tmp.OIK95wgYUb'], + argv) + # None of the GRUB value may end up as a standalone sed option. + self.assertNotIn('--word=8', argv) + self.assertEqual([], [a for a in argv if a.startswith('--')]) + + @mock.patch.object(base.BaseLinuxOSMorphingTools, '_read_file_sudo') + @mock.patch.object(base.BaseLinuxOSMorphingTools, '_exec_cmd_chroot') + def test_set_grub_value_with_embedded_quotes(self, mock_exec_cmd_chroot, + mock_read_file_sudo): + """Values holding quotes must not break out of the sed script.""" + config_obj = { + 'location': '/tmp/tmp_file', + 'source': '/etc/default/grub', + 'contents': {'GRUB_DEFAULT': '0'}, + } + + self.os_morphing_tools.set_grub_value( + 'GRUB_CMDLINE_LINUX', "quiet 'splash'", config_obj) + + self.assertEqual( + ['sed', '-ie', '$aGRUB_CMDLINE_LINUX="quiet \'splash\'"', + '/tmp/tmp_file'], + shlex.split(mock_exec_cmd_chroot.call_args[0][0])) + @mock.patch.object(base.BaseLinuxOSMorphingTools, 'set_grub_value') def test__set_grub2_cmdline_clobber(self, mock_set_grub_value): config_obj = { diff --git a/coriolis/tests/osmorphing/test_debian.py b/coriolis/tests/osmorphing/test_debian.py index 722778e91..1642642dc 100644 --- a/coriolis/tests/osmorphing/test_debian.py +++ b/coriolis/tests/osmorphing/test_debian.py @@ -48,6 +48,24 @@ def test_check_os_not_supported(self): self.assertFalse(result) + def test_init_declares_noninteractive_frontend(self): + """dpkg maintainer scripts must never prompt through debconf.""" + self.assertEqual( + 'noninteractive', self.morpher._environment['DEBIAN_FRONTEND']) + + def test_noninteractive_frontend_survives_set_environment(self): + """The OSMount environment must not drop the constructor's variables. + + 'set_environment' is called by the OSMorphing manager right after the + tools are instantiated, with the environment of the OSMount tools. + """ + self.morpher.set_environment({'http_proxy': 'http://10.0.0.1:3128'}) + + self.assertEqual( + {'DEBIAN_FRONTEND': 'noninteractive', + 'http_proxy': 'http://10.0.0.1:3128'}, + self.morpher._environment) + @mock.patch.object( debian.BaseDebianMorphingTools, '_schedule_grub2_update') @mock.patch('coriolis.utils.Grub2ConfigEditor') @@ -346,9 +364,8 @@ def test_install_packages(self, mock_exec_cmd_chroot): self.morpher.install_packages(self.package_names) apt_get_cmd = ( - '/bin/bash -c "DEBIAN_FRONTEND=noninteractive ' 'apt-get install %s -y ' - '-o Dpkg::Options::=\'--force-confdef\'"' % ( + '-o Dpkg::Options::=--force-confdef' % ( " ".join(self.package_names))) deb_reconfigure_cmd = "dpkg --configure --force-confold -a" @@ -373,6 +390,28 @@ def test_uninstall_packages(self, mock_exec_cmd_chroot): mock.call('apt-get remove %s -y || true' % self.package_names[1]) ]) + @ddt.data('install_packages', 'uninstall_packages') + @mock.patch.object(base.utils, 'exec_ssh_cmd_chroot') + def test_packages_operations_environment( + self, operation, mock_exec_ssh_cmd_chroot): + """Both package operations run with the environment of the tools. + + Removing a package runs maintainer scripts just like installing one + does, so both must be shielded from debconf prompts, and both must + reach the package manager with the proxy configuration. + """ + self.morpher.set_environment({'http_proxy': 'http://10.0.0.1:3128'}) + + getattr(self.morpher, operation)(self.package_names) + + expected_environment = { + 'DEBIAN_FRONTEND': 'noninteractive', + 'http_proxy': 'http://10.0.0.1:3128'} + mock_exec_ssh_cmd_chroot.assert_called() + for call in mock_exec_ssh_cmd_chroot.call_args_list: + self.assertEqual( + expected_environment, call.kwargs['environment']) + @mock.patch.object(debian.BaseDebianMorphingTools, '_exec_cmd_chroot') def test_uninstall_packages_with_exception(self, mock_exec_cmd_chroot): mock_exec_cmd_chroot.side_effect = exception.CoriolisException() diff --git a/coriolis/tests/providers/test_backup_writers.py b/coriolis/tests/providers/test_backup_writers.py index 62ae16291..1add3744d 100644 --- a/coriolis/tests/providers/test_backup_writers.py +++ b/coriolis/tests/providers/test_backup_writers.py @@ -39,10 +39,10 @@ def test__disable_lvm2_lvmetad(self, mock_exec_ssh_cmd, mock.call( self.mock_ssh, 'sudo sed -i "s/use_lvmetad.*=.*1/use_lvmetad = 0/g" %s' % - cfg, get_pty=True), + cfg, get_pty=False), mock.call(self.mock_ssh, - 'sudo service lvm2-lvmetad stop', get_pty=True), - mock.call(self.mock_ssh, 'sudo vgchange -an', get_pty=True)] + 'sudo service lvm2-lvmetad stop', get_pty=False), + mock.call(self.mock_ssh, 'sudo vgchange -an', get_pty=False)] mock_exec_ssh_cmd.assert_has_calls(expected_calls) @mock.patch('coriolis.utils.test_ssh_path') @@ -65,7 +65,7 @@ def test__disable_lvm_metad_udev_rule(self, mock_exec_ssh_cmd, expected_calls = [ mock.call(self.mock_ssh, 'sudo rm %s' % rule_path, - get_pty=True) + get_pty=False) for rule_path in rule_paths] mock_exec_ssh_cmd.assert_has_calls(expected_calls) @@ -1357,7 +1357,7 @@ def test__inject_dport_allow_rule(self, mock_exec_ssh_cmd): "accept || " "sudo iptables -I INPUT -p tcp --dport %(port)s -j ACCEPT" % { "port": self.writer_port}, - get_pty=True) + get_pty=False) @mock.patch('coriolis.utils.exec_ssh_cmd') def test__inject_dport_allow_rule_with_exception(self, mock_exec_ssh_cmd): @@ -1374,7 +1374,7 @@ def test__add_firewalld_port(self, mock_exec_ssh_cmd): mock_exec_ssh_cmd.assert_called_once_with( self._ssh, "sudo firewall-cmd --add-port=%s/tcp" % - self.writer_port, get_pty=True) + self.writer_port, get_pty=False) @mock.patch('coriolis.utils.exec_ssh_cmd') def test__add_firewalld_port_with_exception(self, mock_exec_ssh_cmd): @@ -1390,7 +1390,7 @@ def test__change_binary_se_context(self, mock_exec_ssh_cmd): mock_exec_ssh_cmd.assert_called_once_with( self._ssh, - 'sudo chcon -t bin_t /usr/bin/coriolis-writer', get_pty=True) + 'sudo chcon -t bin_t /usr/bin/coriolis-writer', get_pty=False) @mock.patch('coriolis.utils.exec_ssh_cmd') def test__change_binary_se_context_with_exception(self, mock_exec_ssh_cmd): @@ -1440,11 +1440,11 @@ def test__copy_writer_file_does_not_exist( mock.call( self._ssh, "sudo mv %s %s" % ( remote_tmp_path, self.bootstrapper._writer_cmd), - get_pty=True + get_pty=False ), mock.call( self._ssh, "sudo chmod +x %s" % self.bootstrapper._writer_cmd, - get_pty=True)]) + get_pty=False)]) mock_sftp.close.assert_called_once() @mock.patch('coriolis.utils.exec_ssh_cmd') @@ -1472,7 +1472,7 @@ def test__fetch_remote_file(self, mock_exec_ssh_cmd, mock_read_ssh_file): data.assert_called_once_with(mock.sentinel.local_file, 'wb') mock_exec_ssh_cmd.assert_called_once_with( self._ssh, "sudo chmod +r %s" % mock.sentinel.remote_file, - get_pty=True) + get_pty=False) data.return_value.write.assert_called_once_with( mock_read_ssh_file.return_value) @@ -1504,7 +1504,7 @@ def test__setup_certificates_no_files_exist( self.bootstrapper._setup_certificates(self._ssh) mock_exec_ssh_cmd.assert_any_call( - self._ssh, "sudo mkdir -p /etc/coriolis-writer", get_pty=True) + self._ssh, "sudo mkdir -p /etc/coriolis-writer", get_pty=False) mock_exec_ssh_cmd.assert_any_call( self._ssh, "sudo %(writer_cmd)s generate-certificates -output-dir " @@ -1513,7 +1513,7 @@ def test__setup_certificates_no_files_exist( "cert_dir": "/etc/coriolis-writer", "extra_hosts": self.bootstrapper._ip, }, - get_pty=True) + get_pty=False) @mock.patch('coriolis.utils.exec_ssh_cmd') def test__read_remote_file_sudo(self, mock_exec_ssh_cmd): @@ -1522,7 +1522,7 @@ def test__read_remote_file_sudo(self, mock_exec_ssh_cmd): mock_exec_ssh_cmd.assert_called_once_with( self._ssh, 'sudo cat "%s"' % mock.sentinel.remote_path, - get_pty=True) + get_pty=False) self.assertEqual( result, mock_exec_ssh_cmd.return_value) diff --git a/coriolis/tests/providers/test_replicator.py b/coriolis/tests/providers/test_replicator.py index 39b149040..488c3aa66 100644 --- a/coriolis/tests/providers/test_replicator.py +++ b/coriolis/tests/providers/test_replicator.py @@ -798,7 +798,7 @@ def test__copy_file(self, mock_exec_ssh_cmd, mock_from_transport, mock_exec_ssh_cmd.assert_called_once_with( self._ssh, "sudo mv %s %s" % ( mock_mktemp.return_value, mock.sentinel.remotePath), - get_pty=True) + get_pty=False) mock_sftp.close.assert_called_once() @mock.patch.object(os.path, 'join') @@ -819,7 +819,7 @@ def test__copy_replicator_cmd( mock_exec_ssh_cmd.assert_called_once_with( self._ssh, "sudo chmod +x %s" % replicator_module.REPLICATOR_PATH, - get_pty=True) + get_pty=False) @mock.patch.object(replicator_module.utils, 'exec_ssh_cmd') def test_setup_replicator_group(self, mock_exec_ssh_cmd): @@ -848,10 +848,10 @@ def test__setup_replicator_no_group(self, mock_exec_ssh_cmd): "echo 1 || echo 0" % replicator_module.REPLICATOR_GROUP_NAME), mock.call(self._ssh, - "sudo groupadd %s" % group_name, get_pty=True), + "sudo groupadd %s" % group_name, get_pty=False), mock.call(self._ssh, "sudo usermod -aG %s %s" % ( replicator_module.REPLICATOR_GROUP_NAME, - self.conn_info["username"]), get_pty=True)]) + self.conn_info["username"]), get_pty=False)]) self.assertFalse(result) @@ -881,10 +881,10 @@ def test__setup_replicator_user_no_user(self, mock_exec_ssh_cmd): "sudo useradd -m -s /bin/bash -g %s %s" % (replicator_module.REPLICATOR_USERNAME, replicator_module.REPLICATOR_GROUP_NAME), - get_pty=True), + get_pty=False), mock.call(self._ssh, "sudo usermod -aG disk %s" % - replicator_module.REPLICATOR_USERNAME, get_pty=True)]) + replicator_module.REPLICATOR_USERNAME, get_pty=False)]) @mock.patch.object(replicator_module.utils, 'create_service') def test__exec_replicator_cmd(self, mock_create_service): @@ -951,18 +951,18 @@ def test_setup_certificates_no_files_exist( expected_calls = [ mock.call(self._ssh, "sudo mkdir -p %s" % - replicator_module.REPLICATOR_DIR, get_pty=True), + replicator_module.REPLICATOR_DIR, get_pty=False), mock.call(self._ssh, "sudo %s gen-certs -output-dir" % replicator_module.REPLICATOR_PATH + " %s -certificate-hosts 127.0.0.1,%s" % (replicator_module.REPLICATOR_DIR, self.conn_info['ip']), - get_pty=True), + get_pty=False), mock.call(self._ssh, "sudo chown -R %s:%s %s" % (replicator_module.REPLICATOR_USERNAME, replicator_module.REPLICATOR_GROUP_NAME, - replicator_module.REPLICATOR_DIR), get_pty=True), + replicator_module.REPLICATOR_DIR), get_pty=False), mock.call(self._ssh, "sudo chmod -R g+r %s" % - replicator_module.REPLICATOR_DIR, get_pty=True)] + replicator_module.REPLICATOR_DIR, get_pty=False)] mock_exec_ssh_cmd.assert_has_calls(expected_calls) self.assertEqual(mock_fetch_remote_file.call_count, 3) @@ -1003,12 +1003,12 @@ def test__setup_replicator( mock.call( self._ssh, "sudo chmod 755 %s" % replicator_module.REPLICATOR_STATE, - get_pty=True + get_pty=False ), mock.call( self._ssh, "sudo chcon -t bin_t /usr/bin/replicator", - get_pty=True + get_pty=False ), ]) mock_os_remove.assert_called_once_with( @@ -1040,7 +1040,7 @@ def test__change_binary_se_context(self, mock_exec_ssh_cmd): mock_exec_ssh_cmd.assert_called_once_with( self._ssh, "sudo chcon -t bin_t %s" % replicator_module.REPLICATOR_PATH, - get_pty=True) + get_pty=False) @mock.patch.object(replicator_module.utils, 'exec_ssh_cmd') def test__change_binary_se_context_with_exception(self, mock_exec_ssh_cmd): diff --git a/coriolis/tests/test_utils.py b/coriolis/tests/test_utils.py index cf68e3242..f5ce6951d 100644 --- a/coriolis/tests/test_utils.py +++ b/coriolis/tests/test_utils.py @@ -6,6 +6,7 @@ import json import logging import os +import shlex import socket from unittest import mock import uuid @@ -428,23 +429,185 @@ def test_exec_ssh_cmd_exit_code_127(self): self.assertRaises(exception.SSHCommandNotFoundException, utils.exec_ssh_cmd, self.mock_ssh, "command") - def test_exec_ssh_cmd_chroot(self): + def _setup_successful_ssh_cmd(self): self.mock_stdout.read.return_value = b'output\n' self.mock_stdout.channel.recv_exit_status.return_value = 0 self.mock_ssh.exec_command.return_value = (None, self.mock_stdout, self.mock_stdout) + def _get_executed_ssh_cmd(self): + return self.mock_ssh.exec_command.call_args[0][0] + + def test_check_env_command(self): + self._setup_successful_ssh_cmd() + + utils.check_env_command(self.mock_ssh) + + self.assertEqual("command -v env", self._get_executed_ssh_cmd()) + + def test_check_env_command_missing(self): + self.mock_stdout.read.return_value = b'' + self.mock_stdout.channel.recv_exit_status.return_value = 1 + self.mock_ssh.exec_command.return_value = (None, self.mock_stdout, + self.mock_stdout) + + self.assertRaises( + exception.CoriolisException, utils.check_env_command, + self.mock_ssh) + + def test_check_env_command_not_found(self): + # 'command -v' itself being unavailable must be reported the same + # way as a missing env(1). + self.mock_stdout.read.return_value = b'' + self.mock_stdout.channel.recv_exit_status.return_value = 127 + self.mock_ssh.exec_command.return_value = (None, self.mock_stdout, + self.mock_stdout) + + self.assertRaises( + exception.CoriolisException, utils.check_env_command, + self.mock_ssh) + + def test_get_env_command_prefix(self): + self.assertEqual("", utils.get_env_command_prefix(None)) + self.assertEqual("", utils.get_env_command_prefix({})) + self.assertEqual( + "env DEBIAN_FRONTEND=noninteractive ", + utils.get_env_command_prefix( + {"DEBIAN_FRONTEND": "noninteractive"})) + # Values holding shell-sensitive characters must be quoted whole. + self.assertEqual( + "env 'http_proxy=http://a b' ", + utils.get_env_command_prefix({"http_proxy": "http://a b"})) + self.assertEqual( + "env 'http_proxy=http://u:p'\"'\"'w@h:3128' ", + utils.get_env_command_prefix( + {"http_proxy": "http://u:p'w@h:3128"})) + + def test_exec_ssh_cmd_chroot(self): + self._setup_successful_ssh_cmd() + result = utils.exec_ssh_cmd_chroot( - self.mock_ssh, "/chroot /bin/bash -c", "command") + self.mock_ssh, "/chroot", "command") self.mock_ssh.exec_command.assert_called_once_with( - "sudo -E chroot /chroot /bin/bash -c command", + "sudo chroot /chroot command", environment=None, get_pty=False, timeout=None) expected = self.mock_stdout.read.return_value.decode( 'utf-8', errors='replace') self.assertEqual(result, expected) + def test_exec_ssh_cmd_chroot_no_environment(self): + """With no env vars set, no 'env' prefix may be added at all.""" + self._setup_successful_ssh_cmd() + + for environment in (None, {}): + self.mock_ssh.exec_command.reset_mock() + utils.exec_ssh_cmd_chroot( + self.mock_ssh, "/chroot", "apt-get update -y", + environment=environment) + + self.mock_ssh.exec_command.assert_called_once_with( + "sudo chroot /chroot apt-get update -y", + environment=environment, get_pty=False, timeout=None) + + def test_exec_ssh_cmd_chroot_with_proxy_environment(self): + """Proxy vars are passed through env(1), placed before the chroot.""" + self._setup_successful_ssh_cmd() + environment = { + "http_proxy": "http://10.0.0.1:3128", + "HTTP_PROXY": "http://10.0.0.1:3128", + "https_proxy": "http://10.0.0.1:3128", + "HTTPS_PROXY": "http://10.0.0.1:3128", + "ftp_proxy": "http://10.0.0.1:3128", + "FTP_PROXY": "http://10.0.0.1:3128", + "no_proxy": "localhost.127.0.0.1", + } + + utils.exec_ssh_cmd_chroot( + self.mock_ssh, "/chroot", "apt-get update -y", + environment=environment) + + cmd = self._get_executed_ssh_cmd() + self.assertEqual( + "sudo env " + "http_proxy=http://10.0.0.1:3128 " + "HTTP_PROXY=http://10.0.0.1:3128 " + "https_proxy=http://10.0.0.1:3128 " + "HTTPS_PROXY=http://10.0.0.1:3128 " + "ftp_proxy=http://10.0.0.1:3128 " + "FTP_PROXY=http://10.0.0.1:3128 " + "no_proxy=localhost.127.0.0.1 " + "chroot /chroot apt-get update -y", cmd) + # env(1) must run before chroot(1) so that the variables are + # inherited by the command running *inside* the chroot. + self.assertLess(cmd.index("env "), cmd.index("chroot")) + self.assertNotIn("sudo -E", cmd) + + def test_exec_ssh_cmd_chroot_proxy_with_credentials_is_quoted(self): + """Shell-sensitive proxy values survive as a single argument.""" + self._setup_successful_ssh_cmd() + proxy = "http://user:p@ss w0rd&$(reboot)@10.0.0.1:3128?a=1" + + utils.exec_ssh_cmd_chroot( + self.mock_ssh, "/chroot", "apt-get update -y", + environment={"https_proxy": proxy}) + + self.assertEqual( + ["sudo", "env", "https_proxy=%s" % proxy, "chroot", "/chroot", + "apt-get", "update", "-y"], + shlex.split(self._get_executed_ssh_cmd())) + + def test_exec_ssh_cmd_chroot_does_not_re_quote_the_command(self): + """Commands holding quotes must not get an extra quoting layer. + + Regression test for the GRUB serial command: wrapping 'cmd' in + "/bin/bash -c '...'" terminated the outer quoting early, which turned + the appended GRUB value into separate 'sed' options and failed with + "sed: unrecognized option '--word=8'". + """ + self._setup_successful_ssh_cmd() + grub_value = ( + 'serial --word=8 --stop=1 --speed=115200 --parity=no --unit=0') + sed_script = '$aGRUB_SERIAL_COMMAND="%s"' % grub_value + cmd = "sed -ie %s /tmp/tmp.OIK95wgYUb" % shlex.quote(sed_script) + + utils.exec_ssh_cmd_chroot( + self.mock_ssh, "/tmp/tmp.q15QdW45qE", cmd, + environment={"http_proxy": "http://10.0.0.1:3128"}) + + argv = shlex.split(self._get_executed_ssh_cmd()) + self.assertEqual( + ["sudo", "env", "http_proxy=http://10.0.0.1:3128", "chroot", + "/tmp/tmp.q15QdW45qE", "sed", "-ie", sed_script, + "/tmp/tmp.OIK95wgYUb"], + argv) + # No part of the GRUB value may become a standalone sed option. + self.assertNotIn("--word=8", argv) + self.assertEqual([], [a for a in argv if a.startswith("--")]) + + def test_exec_ssh_cmd_chroot_preserves_shell_operators(self): + """Host-side shell operators used by callers stay intact.""" + self._setup_successful_ssh_cmd() + + utils.exec_ssh_cmd_chroot( + self.mock_ssh, "/chroot", + '[ -f "/etc/default/grub" ] && echo 1 || echo 0') + + self.assertEqual( + 'sudo chroot /chroot [ -f "/etc/default/grub" ] ' + '&& echo 1 || echo 0', + self._get_executed_ssh_cmd()) + + def test_exec_ssh_cmd_chroot_quotes_the_chroot_dir(self): + self._setup_successful_ssh_cmd() + + utils.exec_ssh_cmd_chroot(self.mock_ssh, "/tmp/os root dir", "true") + + self.assertEqual( + "sudo chroot '/tmp/os root dir' true", + self._get_executed_ssh_cmd()) + def test_check_fs(self): self.mock_stdout.read.return_value.replace.return_value = \ self.mock_stdout.read.return_value @@ -455,7 +618,7 @@ def test_check_fs(self): utils.check_fs(self.mock_ssh, "ext4", "/dev/sda1") self.mock_ssh.exec_command.assert_called_once_with( - "sudo fsck -p -t ext4 /dev/sda1", environment=None, get_pty=True, + "sudo fsck -p -t ext4 /dev/sda1", environment=None, get_pty=False, timeout=None) @mock.patch.object(utils, 'exec_ssh_cmd') @@ -466,7 +629,7 @@ def test_check_fs_exception(self, mock_exec_ssh_cmd): self.mock_ssh, "ext4", "/dev/sda1") mock_exec_ssh_cmd.assert_called_once_with( - self.mock_ssh, "sudo fsck -p -t ext4 /dev/sda1", get_pty=True) + self.mock_ssh, "sudo fsck -p -t ext4 /dev/sda1", get_pty=False) @mock.patch.object(utils, 'exec_ssh_cmd') def test_run_xfs_repair(self, mock_exec_ssh_cmd): @@ -477,11 +640,11 @@ def test_run_xfs_repair(self, mock_exec_ssh_cmd): expected_calls = [ mock.call(self.mock_ssh, "mktemp -d"), mock.call(self.mock_ssh, "sudo mount /dev/sda1 /tmp/tmp_dir", - get_pty=True), + get_pty=False), mock.call(self.mock_ssh, "sudo umount /tmp/tmp_dir", - get_pty=True), + get_pty=False), mock.call(self.mock_ssh, "sudo xfs_repair /dev/sda1", - get_pty=True), + get_pty=False), ] mock_exec_ssh_cmd.assert_has_calls(expected_calls) @@ -968,13 +1131,13 @@ def test_write_systemd(self, mock_uuid, mock_test_ssh, mock.ANY) mock_exec_ssh_cmd.assert_has_calls([ mock.call(self.mock_ssh, 'sudo mv /tmp/uuid.service ' - '/lib/systemd/system/svc_name.service', get_pty=True), + '/lib/systemd/system/svc_name.service', get_pty=False), mock.call(self.mock_ssh, 'sudo restorecon -v ' - '/lib/systemd/system/svc_name.service', get_pty=True), + '/lib/systemd/system/svc_name.service', get_pty=False), mock.call(self.mock_ssh, 'sudo systemctl daemon-reload', - get_pty=True), + get_pty=False), mock.call(self.mock_ssh, 'sudo systemctl start svc_name', - get_pty=True)]) + get_pty=False)]) @mock.patch('coriolis.utils.exec_ssh_cmd') @mock.patch('coriolis.utils.write_ssh_file') @@ -995,7 +1158,7 @@ def test_write_systemd_usr_lib(self, mock_uuid, mock_test_ssh, mock_exec_ssh_cmd.assert_has_calls([ mock.call(self.mock_ssh, 'sudo mv /tmp/uuid.service ' '/usr/lib/systemd/system/svc_name.service', - get_pty=True)]) + get_pty=False)]) @mock.patch('coriolis.utils.exec_ssh_cmd') @mock.patch('coriolis.utils.test_ssh_path') @@ -1010,7 +1173,7 @@ def test_write_systemd_service_exists(self, mock_test_ssh, mock.call(self.mock_ssh, '/lib/systemd/system/svc_name.service')]) mock_exec_ssh_cmd.assert_called_once_with( - self.mock_ssh, 'sudo systemctl start svc_name', get_pty=True) + self.mock_ssh, 'sudo systemctl start svc_name', get_pty=False) @mock.patch('coriolis.utils.exec_ssh_cmd') @mock.patch('coriolis.utils.write_ssh_file') @@ -1070,13 +1233,13 @@ def test_test_write_systemd_with_run_as(self, mock_uuid, mock_test_ssh, mock_exec_ssh_cmd.assert_has_calls([ mock.call(self.mock_ssh, 'sudo mv /tmp/uuid.service ' - '/lib/systemd/system/svc_name.service', get_pty=True), + '/lib/systemd/system/svc_name.service', get_pty=False), mock.call(self.mock_ssh, 'sudo restorecon -v ' - '/lib/systemd/system/svc_name.service', get_pty=True), + '/lib/systemd/system/svc_name.service', get_pty=False), mock.call(self.mock_ssh, 'sudo systemctl daemon-reload', - get_pty=True), + get_pty=False), mock.call(self.mock_ssh, 'sudo systemctl start svc_name', - get_pty=True)]) + get_pty=False)]) @mock.patch('coriolis.utils.exec_ssh_cmd') @mock.patch('coriolis.utils.write_ssh_file') @@ -1098,7 +1261,7 @@ def test_write_upstart(self, mock_uuid, mock_test_ssh, mock.ANY) mock_exec_ssh_cmd.assert_has_calls([ mock.call(self.mock_ssh, 'sudo mv /tmp/uuid.conf ' - '/etc/init/svc_name.conf', get_pty=True), + '/etc/init/svc_name.conf', get_pty=False), mock.call(self.mock_ssh, 'start svc_name')]) @mock.patch('coriolis.utils.test_ssh_path') @@ -1134,7 +1297,7 @@ def test_write_upstart_with_run_as(self, mock_uuid, mock_test_ssh, mock_exec_ssh_cmd.assert_has_calls([ mock.call(self.mock_ssh, 'sudo mv /tmp/uuid.conf ' - '/etc/init/svc_name.conf', get_pty=True), + '/etc/init/svc_name.conf', get_pty=False), mock.call(self.mock_ssh, 'start svc_name')]) @mock.patch('coriolis.utils._write_systemd') @@ -1184,7 +1347,7 @@ def test_restart_service_with_systemd(self, mock_test_ssh, mock_test_ssh.assert_called_once_with(self.mock_ssh, '/lib/systemd/system') mock_exec_ssh_cmd.assert_called_once_with( - self.mock_ssh, 'sudo systemctl restart svc_name', get_pty=True) + self.mock_ssh, 'sudo systemctl restart svc_name', get_pty=False) @mock.patch('coriolis.utils.exec_ssh_cmd') @mock.patch('coriolis.utils.test_ssh_path') @@ -1218,7 +1381,7 @@ def test_start_service_with_systemd(self, mock_test_ssh, mock_test_ssh.assert_called_once_with(self.mock_ssh, '/lib/systemd/system') mock_exec_ssh_cmd.assert_called_once_with( - self.mock_ssh, 'sudo systemctl start svc_name', get_pty=True) + self.mock_ssh, 'sudo systemctl start svc_name', get_pty=False) @mock.patch('coriolis.utils.exec_ssh_cmd') @mock.patch('coriolis.utils.test_ssh_path') @@ -1252,7 +1415,7 @@ def test_stop_service_with_systemd(self, mock_test_ssh, mock_test_ssh.assert_called_once_with(self.mock_ssh, '/lib/systemd/system') mock_exec_ssh_cmd.assert_called_once_with( - self.mock_ssh, 'sudo systemctl stop svc_name', get_pty=True) + self.mock_ssh, 'sudo systemctl stop svc_name', get_pty=False) @mock.patch('coriolis.utils.exec_ssh_cmd') @mock.patch('coriolis.utils.test_ssh_path') diff --git a/coriolis/utils.py b/coriolis/utils.py index 42b660871..1541b53b0 100644 --- a/coriolis/utils.py +++ b/coriolis/utils.py @@ -13,6 +13,7 @@ import os import pickle import re +import shlex import socket import string import subprocess @@ -312,10 +313,45 @@ def list_ssh_dir(ssh, remote_path): LOG.warning( "SFTP listdir failed, falling back to shell command. " "Error: %s", get_exception_details()) - output = exec_ssh_cmd(ssh, "sudo ls -1 %s" % remote_path, get_pty=True) + output = exec_ssh_cmd( + ssh, "sudo ls -1 %s" % remote_path, get_pty=False) return [f for f in output.splitlines() if f.strip()] +def get_env_command_prefix(environment): + """ + Each variable is shell-quoted individually, so values containing + spaces or other shell-sensitive characters are passed through + verbatim. + """ + + if not environment: + return "" + return "env %s " % " ".join( + shlex.quote("%s=%s" % (key, value)) + for key, value in environment.items()) + + +def check_env_command(ssh): + """ + Checks that env(1) exists on the given machine, as it is what carries + the environment variables over to privileged and chrooted commands. + Raises if it is missing, so the cause is reported upfront instead of + surfacing as an unrelated failure much later on. + """ + + try: + exec_ssh_cmd(ssh, "command -v env", get_pty=False) + except (exception.SSHCommandFailed, + exception.SSHCommandNotFoundException) as ex: + raise exception.CoriolisException( + "The 'env' command is unavailable on the OSMorphing minion " + "machine. It is required in order to forward environment " + "variables to commands run on the migrated machine (such as " + "proxy settings). Please switch the minion machine " + "image/template to one that has this command in place.") from ex + + def _exec_ssh_cmd(ssh, cmd, environment=None, get_pty=False, timeout=None): sanitized_cmd = strutils.mask_password(cmd) remote_str = "" @@ -390,16 +426,23 @@ def wrapper(): def exec_ssh_cmd_chroot(ssh, chroot_dir, cmd, environment=None, get_pty=False, timeout=None): - return exec_ssh_cmd(ssh, "sudo -E chroot %s %s" % (chroot_dir, cmd), - environment=environment, get_pty=get_pty, - timeout=timeout) + return exec_ssh_cmd( + ssh, + "sudo %schroot %s %s" % ( + get_env_command_prefix(environment), + shlex.quote(chroot_dir), + cmd), + environment=environment, + get_pty=get_pty, + timeout=timeout, + ) def check_fs(ssh, fs_type, dev_path): try: out = exec_ssh_cmd( ssh, "sudo fsck -p -t %s %s" % (fs_type, dev_path), - get_pty=True) + get_pty=False) LOG.debug("File system checked:\n%s", out) except Exception: LOG.warn("Checking file system returned an error:\n%s" % ( @@ -414,14 +457,14 @@ def run_xfs_repair(ssh, dev_path): LOG.debug("mounting %s on %s" % (dev_path, tmp_dir)) mount_out = exec_ssh_cmd( ssh, "sudo mount %s %s" % (dev_path, tmp_dir), - get_pty=True) + get_pty=False) LOG.debug("mount returned: %s" % mount_out) LOG.debug("Umounting %s" % tmp_dir) umount_out = exec_ssh_cmd( - ssh, "sudo umount %s" % tmp_dir, get_pty=True) + ssh, "sudo umount %s" % tmp_dir, get_pty=False) LOG.debug("umounting returned: %s" % umount_out) out = exec_ssh_cmd( - ssh, "sudo xfs_repair %s" % dev_path, get_pty=True) + ssh, "sudo xfs_repair %s" % dev_path, get_pty=False) LOG.debug("File system repaired:\n%s", out) except Exception as ex: LOG.warn("xfs_repair returned an error:\n%s", str(ex)) @@ -806,22 +849,22 @@ def _write_systemd(ssh, cmdline, svcname, run_as=None, start=True): if test_ssh_path(ssh, serviceFilePath): if start: exec_ssh_cmd( - ssh, "sudo systemctl start %s" % svcname, get_pty=True) + ssh, "sudo systemctl start %s" % svcname, get_pty=False) return def _reload_and_start(start=True): exec_ssh_cmd( ssh, "sudo systemctl daemon-reload", - get_pty=True) + get_pty=False) if start: exec_ssh_cmd( ssh, "sudo systemctl start %s" % svcname, - get_pty=True) + get_pty=False) def _correct_selinux_label(): cmd = "sudo restorecon -v %s" % serviceFilePath try: - exec_ssh_cmd(ssh, cmd, get_pty=True) + exec_ssh_cmd(ssh, cmd, get_pty=False) except exception.CoriolisException: LOG.warn( "Could not relabel service '%s'. SELinux might not be " @@ -843,7 +886,7 @@ def _correct_selinux_label(): exec_ssh_cmd( ssh, "sudo mv /tmp/%s.service %s" % (name, serviceFilePath), - get_pty=True) + get_pty=False) _correct_selinux_label() _reload_and_start(start=start) @@ -867,7 +910,7 @@ def _write_upstart(ssh, cmdline, svcname, run_as=None, start=True): exec_ssh_cmd( ssh, "sudo mv /tmp/%s.conf %s" % (name, serviceFilePath), - get_pty=True) + get_pty=False) if start: exec_ssh_cmd(ssh, "start %s" % svcname) @@ -895,7 +938,7 @@ def create_service(ssh, cmdline, svcname, run_as=None, start=True): def restart_service(ssh, svcname): if _has_systemd(ssh): - exec_ssh_cmd(ssh, "sudo systemctl restart %s" % svcname, get_pty=True) + exec_ssh_cmd(ssh, "sudo systemctl restart %s" % svcname, get_pty=False) elif test_ssh_path(ssh, "/etc/init"): exec_ssh_cmd(ssh, "restart %s" % svcname) else: @@ -904,7 +947,7 @@ def restart_service(ssh, svcname): def start_service(ssh, svcname): if _has_systemd(ssh): - exec_ssh_cmd(ssh, "sudo systemctl start %s" % svcname, get_pty=True) + exec_ssh_cmd(ssh, "sudo systemctl start %s" % svcname, get_pty=False) elif test_ssh_path(ssh, "/etc/init"): exec_ssh_cmd(ssh, "start %s" % svcname) else: @@ -913,7 +956,7 @@ def start_service(ssh, svcname): def stop_service(ssh, svcname): if _has_systemd(ssh): - exec_ssh_cmd(ssh, "sudo systemctl stop %s" % svcname, get_pty=True) + exec_ssh_cmd(ssh, "sudo systemctl stop %s" % svcname, get_pty=False) elif test_ssh_path(ssh, "/etc/init"): exec_ssh_cmd(ssh, "stop %s" % svcname) else: