Fix privsep issues inside virtual env

When a virtual environment is created with the "--system-site-packages"
option and privsep is installed on the system privsep will only use the
system packages and completely ignore the ones in the virtual
environment.

This results in errors such as the ones we see:

- In the Ussuri gate: ModuleNotFoundError: No module named
  'os_brick.privileged.rootwrap'

- In the Wallaby gate: ModuleNotFoundError: No module named
  'os_brick.privileged.nvmeof'

This happens because os-brick and cinder are starting privsep using the
"privsep-helper" mechanism, and privsep was not installed in the virtual
env because it was already present system wide, so the "privsep-helper"
that is executed is the one from "/usr/local/bin/privsep-helper".

This python script "privsep-helper" ignores the virtual environment and
forces usage of the system's python, for example in a Wallaby
installation this could be "#!/usr/bin/python3.6".

Since it ignores the virtual environment it won't use its packages and
anything that's not present on system wide will not be found, and if
found it may be executing different code.

This patch fixes this issue by replacing the helper used to start
privsep with our own command.

This command is the same as the one usually installed in /usr/local/bin
but using /usr/bin/env to select the python to use.

This new script has been included as data in the cinderlib namespace
instead of making it install as a system script (like the original
privsep command) because we don't want to polute the system wide
binaries directory just for a corner case.

We also need to preserve user site-packages for the running Python when
calling root from the virtual environment, since the packages installed
on the virtual environment with "--system-site-packages" would have
taken those into consideration during the installation and not the ones
present on the root user.

To help debug issues at the gate all functional tests are now running
with debug logs.

Change-Id: I0278b42785d14f92a521e6deff872dcba6505270
Related-Bug: #1958159
Closes-Bug: #1979534

cinderlib/bin/venv-privsep-helper

@@ -0,0 +1,10 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+# Copy of oslo.privsep's privsep-helper that uses the virtual env python
+# instead of a hardcoded Python version
+import re
+import sys
+from oslo_privsep.daemon import helper_main
+if __name__ == '__main__':
+    sys.argv[0] = re.sub(r'(-script\.pyw|\.exe)?$', '', sys.argv[0])
+    sys.exit(helper_main())

cinderlib/cinderlib.py

@@ -407,6 +407,27 @@ class Backend(object):
                 # Initialize privsep's context to not use 'sudo'
                 priv_context.init(root_helper=[root_helper])
 
+        # When using privsep from the system we need to replace the
+        # privsep-helper with our own to use the virtual env libraries.
+        if venv and not priv_context.__file__.startswith(venv):
+            # Use importlib.resources to support PEP 302-based import hooks
+            # Can only use importlib.resources on 3.10 because it was added to
+            # 3.7, but files to 3.9 and namespace packages only to 3.10
+            import sys
+            if sys.version_info[:2] > (3, 10):
+                from importlib.resources import files
+            else:
+                from importlib_resources import files
+            privhelper = files('cinderlib.bin').joinpath('venv-privsep-helper')
+            cmd = f'{root_helper} {privhelper}'
+
+            # Change default of the option instead of the value of the
+            # different contexts
+            for opt in priv_context.OPTS:
+                if opt.name == 'helper_command':
+                    opt.default = cmd
+                    break
+
         # Don't use server/client mode when running as root
         client_mode = not cls.im_root
         cinder.privsep.sys_admin_pctxt.set_client_mode(client_mode)

cinderlib/tests/functional/base_tests.py

@@ -62,6 +62,7 @@ class BaseFunctTestCase(unittest.TestCase):
     CONFIG_FILE = os.environ.get('CL_FTEST_CFG', '/etc/cinder/cinder.conf')
     PRECISION = os.environ.get('CL_FTEST_PRECISION', 0)
     LOGGING_ENABLED = get_bool_env('CL_FTEST_LOGGING', False)
+    DEBUG_ENABLED = get_bool_env('CL_FTEST_DEBUG', False)
     ROOT_HELPER = os.environ.get('CL_FTEST_ROOT_HELPER', 'sudo')
     MEMORY_PERSISTENCE = get_bool_env('CL_FTEST_MEMORY_PERSISTENCE', True)
     DEFAULT_POOL = os.environ.get('CL_FTEST_POOL_NAME', None)
@@ -78,6 +79,7 @@ class BaseFunctTestCase(unittest.TestCase):
                     cls.tests_config = yaml.safe_load(f)
             cls.tests_config.setdefault('logs', cls.LOGGING_ENABLED)
             cls.tests_config.setdefault('size_precision', cls.PRECISION)
+            cls.tests_config.setdefault('debug', cls.DEBUG_ENABLED)
 
             backend = cls.tests_config['backends'][0]
             if backend['volume_driver'].endswith('.RBDDriver'):
@@ -104,6 +106,7 @@ class BaseFunctTestCase(unittest.TestCase):
         # Use memory_db persistence instead of memory to ensure migrations work
         cinderlib.setup(root_helper=cls.ROOT_HELPER,
                         disable_logs=not config['logs'],
+                        debug=config['debug'],
                         persistence_config={'storage': 'memory_db'})
 
         if cls.MEMORY_PERSISTENCE:
@@ -113,6 +116,7 @@ class BaseFunctTestCase(unittest.TestCase):
             cinderlib.Backend.global_initialization = False
             cinderlib.setup(root_helper=cls.ROOT_HELPER,
                             disable_logs=not config['logs'],
+                            debug=config['debug'],
                             persistence_config={'storage': 'memory'})
 
         # Initialize backends

cinderlib/tests/functional/ceph.yaml

@@ -1,5 +1,6 @@
 # Logs are way too verbose, so we disable them
-logs: false
+logs: true
+debug: true
 
 # We only define one backend
 backends:

cinderlib/tests/functional/lvm.yaml

@@ -3,7 +3,8 @@
 #
 
 # Logs are way too verbose, so we disable them
-logs: false
+logs: true
+debug: true
 
 # We only define one backend
 backends:

cinderlib/tests/unit/test_cinderlib.py

@@ -21,6 +21,7 @@ from unittest import mock
 from cinder import utils
 import ddt
 from oslo_config import cfg
+from oslo_privsep import priv_context
 
 import cinderlib
 from cinderlib import objects
@@ -506,6 +507,15 @@ class TestCinderlib(base.BaseTest):
         self.assertEqual(mock_conf.list_all_sections.return_value,
                          mock_conf.list_all_sections())
 
+    def _check_privsep_root_helper_opt(self, is_changed):
+        for opt in priv_context.OPTS:
+            if opt.name == 'helper_command':
+                break
+        helper_path = os.path.join(os.path.dirname(cinderlib.__file__),
+                                   'bin/venv-privsep-helper')
+        self.assertIs(is_changed,
+                      f'mysudo {helper_path}' == opt.default)
+
     @mock.patch.dict(os.environ, {}, clear=True)
     @mock.patch('os.path.exists')
     @mock.patch('configparser.ConfigParser')
@@ -527,6 +537,7 @@ class TestCinderlib(base.BaseTest):
             mock_ctxt_init.assert_not_called()
             self.assertIs(original_helper_func, utils.get_root_helper)
             self.assertIs(rootwrap_config, cfg.CONF.rootwrap_config)
+            self._check_privsep_root_helper_opt(is_changed=False)
         finally:
             cfg.CONF.rootwrap_config = original_rootwrap_config
 
@@ -582,6 +593,7 @@ class TestCinderlib(base.BaseTest):
 
             self.assertIs(original_helper_func, utils.get_root_helper)
             self.assertEqual(venv_wrap_cfg, cfg.CONF.rootwrap_config)
+            self._check_privsep_root_helper_opt(is_changed=True)
         finally:
             cfg.CONF.rootwrap_config = original_rootwrap_config
             utils.get_root_helper = original_helper_func
@@ -657,6 +669,7 @@ class TestCinderlib(base.BaseTest):
 
             self.assertIs(original_helper_func, utils.get_root_helper)
             self.assertEqual(venv_wrap_cfg, cfg.CONF.rootwrap_config)
+            self._check_privsep_root_helper_opt(is_changed=True)
         finally:
             cfg.CONF.rootwrap_config = original_rootwrap_config
             utils.get_root_helper = original_helper_func

releasenotes/notes/fix-privsep-venv-2ae8ce791136ae73.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    `Bug #1979534 <https://bugs.launchpad.net/cinderlib/+bug/1979534>`_: Fix
+    issues running privileged commands within a virtual environments that use
+    system site-packages when the privsep package is installed system-wide or
+    when privileged commands use user site-packages.

requirements.txt

@@ -4,3 +4,4 @@ cinder>=20.0.0.0,<21.0.0  # Apache-2.0
 os-brick>=5.2.0  # Apache-2.0
 
 importlib_metadata>=1.7.0;python_version<'3.8' # Apache-2.0
+importlib_resources>=3.2.1;python_version<'3.10' # Apache-2.0

roles/run-cinderlib-tests/defaults/main.yaml

@@ -1,5 +1,6 @@
 cinderlib_base_dir: "{{ devstack_base_dir | default('/opt/stack') }}/cinderlib"
 cinderlib_config_file: '/etc/cinder/cinder.conf'
 cinderlib_envlist: functional
-cinderlib_root_helper: sudo
 cinderlib_pool_name: ''
+cinderlib_logs: true
+cinderlib_debug: true

roles/run-cinderlib-tests/tasks/main.yaml

@@ -6,5 +6,8 @@
     zuul_work_dir: "{{ cinderlib_base_dir }}"
     tox_environment:
       CL_FTEST_CFG: "{{ cinderlib_config_file }}"
-      CL_FTEST_ROOT_HELPER: "{{ cinderlib_root_helper }}"
       CL_FTEST_POOL_NAME: "{{ cinderlib_pool_name }}"
+      # Workaround for https://github.com/pypa/pip/issues/6264
+      PIP_OPTIONS: "--no-use-pep517"
+      CL_FTEST_DEBUG: " {{cinderlib_logs }}"
+      CL_FTEST_LOGGING: " {{cinderlib_debug }}"

setup.cfg

@@ -29,6 +29,8 @@ classifier =
 [files]
 packages =
     cinderlib
+package_data =
+    cinderlib.bin = *
 
 [entry_points]
 cinderlib.persistence.storage =

tools/virtualenv-sudo.sh

@@ -5,4 +5,14 @@
 params=()
 for arg in "$@"; do params+=("\"$arg\""); done
 params="${params[@]}"
-sudo -E --preserve-env=PATH,VIRTUAL_ENV /bin/bash -c "$params"
+# Preserve user site-packages from the caller on the root user call in case
+# it's a python program we are calling.
+local_path=`python -c "import site; print(site.USER_SITE)"`
+if [[ -n "$local_path" ]]; then
+    if [[ -z "$PYTHONPATH" ]]; then
+        PYTHONPATH="$local_path"
+    else
+        PYTHONPATH="$PYTHONPATH:$local_path"
+    fi
+fi
+sudo -E --preserve-env=PATH,VIRTUAL_ENV,PYTHONPATH PYTHONPATH="$PYTHONPATH" /bin/bash -c "$params"

tox.ini

@@ -38,7 +38,7 @@ passenv = *_proxy *_PROXY
 
 [testenv:functional]
 usedevelop=True
-passenv = CL_FTEST_POOL_NAME
+passenv = CL_FTEST_POOL_NAME CL_FTEST_LOGGING CL_FTEST_DEBUG
 setenv = OS_TEST_PATH=./cinderlib/tests/functional
          CL_FTEST_CFG={env:CL_FTEST_CFG:{toxinidir}/cinderlib/tests/functional/lvm.yaml}
          CL_FTEST_ROOT_HELPER={env:CL_FTEST_ROOT_HELPER:{toxinidir}/tools/virtualenv-sudo.sh}
@@ -62,6 +62,8 @@ allowlist_externals =
 
 [testenv:functional-py36]
 usedevelop=True
+passenv =
+  {[testenv:functional]passenv}
 setenv =
   {[testenv:functional]setenv}
 sitepackages = True
@@ -73,6 +75,8 @@ allowlist_externals = {[testenv:functional]allowlist_externals}
 
 [testenv:functional-py39]
 usedevelop=True
+passenv =
+  {[testenv:functional]passenv}
 setenv =
   {[testenv:functional]setenv}
 sitepackages = True