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
(cherry picked from commit 4d784d23a9)
This commit is contained in:
Gorka Eguileor 2022-06-22 14:58:55 +02:00 committed by Luigi Toscano
parent 9ae3a1c771
commit 4fc56c815b
13 changed files with 84 additions and 6 deletions

View File

@ -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())

View File

@ -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)

View File

@ -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

View File

@ -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:

View File

@ -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:

View File

@ -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

View File

@ -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.

View File

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

View File

@ -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

View File

@ -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 }}"

View File

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

View File

@ -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"

View File

@ -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-py38]
usedevelop=True
passenv =
{[testenv:functional]passenv}
setenv =
{[testenv:functional]setenv}
sitepackages = True