Improve environment variable handling in OSMount tools

The OSMount setup commands relied on `sudo -E` to forward the proxy
configuration to the package managers, which does not work on sudo
implementations that do not support the flag.

Signed-off-by: Mihaela Balutoiu <mbalutoiu@cloudbasesolutions.com>

coriolis/osmorphing/osmount/base.py

@@ -131,6 +131,31 @@ class BaseSSHOSMountTools(BaseOSMountTools):
             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=True,
+                timeout=timeout,
+            )
+        except exception.MinionMachineCommandTimeout as ex:
+            raise exception.OSMorphingSSHOperationTimeout(
+                cmd=cmd,
+                timeout=timeout,
+            ) from ex
+
     def get_connection(self):
         return self._ssh
 

coriolis/osmorphing/osmount/redhat.py

@@ -20,7 +20,7 @@ class RedHatOSMountTools(base.BaseLinuxOSMountTools):
 
     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")

coriolis/osmorphing/osmount/suse.py

@@ -39,9 +39,9 @@ class SUSEOSMountTools(base.BaseLinuxOSMountTools):
     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")

coriolis/osmorphing/osmount/ubuntu.py

@@ -25,8 +25,8 @@ class UbuntuOSMountTools(base.BaseLinuxOSMountTools):
         # 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 @@ class UbuntuOSMountTools(base.BaseLinuxOSMountTools):
         # 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")

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
@@ -146,6 +147,96 @@ class BaseSSHOSMountToolsTestCase(test_base.CoriolisBaseTestCase):
             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 <cmd>'."""
+        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=True, 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=True,
+            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):
@@ -1146,3 +1237,15 @@ class BaseLinuxOSMountToolsTestCase(test_base.CoriolisBaseTestCase):
         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)

coriolis/tests/osmorphing/osmount/test_redhat.py

@@ -31,15 +31,17 @@ class BaseRedHatOSMountToolsTestCase(test_base.CoriolisBaseTestCase):
         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")
         ])

coriolis/tests/osmorphing/osmount/test_suse.py

@@ -53,9 +53,11 @@ class BaseSUSEOSMountToolsTestCase(test_base.CoriolisBaseTestCase):
         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 @@ class BaseSUSEOSMountToolsTestCase(test_base.CoriolisBaseTestCase):
         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")

coriolis/tests/osmorphing/osmount/test_ubuntu.py

@@ -30,22 +30,52 @@ class UbuntuOSMountToolsTestCase(test_base.CoriolisBaseTestCase):
         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):