From 67eec9ca96b2748d608f551910d2351c19664d5b Mon Sep 17 00:00:00 2001 From: blag Date: Thu, 28 Jan 2021 01:38:40 -0700 Subject: [PATCH 01/15] Bring back old sandboxing code --- st2common/st2common/util/sandboxing.py | 68 +++++++++++++++++++++++++- 1 file changed, 67 insertions(+), 1 deletion(-) diff --git a/st2common/st2common/util/sandboxing.py b/st2common/st2common/util/sandboxing.py index 6b3c4e29b9..1da56186c4 100644 --- a/st2common/st2common/util/sandboxing.py +++ b/st2common/st2common/util/sandboxing.py @@ -20,6 +20,7 @@ from __future__ import absolute_import +import fnmatch import os import sys from distutils.sysconfig import get_python_lib @@ -27,6 +28,7 @@ from oslo_config import cfg from st2common.constants.pack import SYSTEM_PACK_NAMES +from st2common.content.utils import get_pack_base_path __all__ = [ 'get_sandbox_python_binary_path', @@ -132,10 +134,74 @@ def get_sandbox_python_path_for_python_action(pack, inherit_from_parent=True, Same as get_sandbox_python_path() function, but it's intended to be used for Python runner actions. """ - return get_sandbox_python_path( + sandbox_python_path = get_sandbox_python_path( inherit_from_parent=inherit_from_parent, inherit_parent_virtualenv=inherit_parent_virtualenv) + pack_base_path = get_pack_base_path(pack_name=pack) + virtualenv_path = get_sandbox_virtualenv_path(pack=pack) + + if not virtualenv_path: + return sandbox_python_path + + pack_virtualenv_lib_path = os.path.join(virtualenv_path, 'lib') + + virtualenv_directories = os.listdir(pack_virtualenv_lib_path) + virtualenv_directories = [dir_name for dir_name in virtualenv_directories if + fnmatch.fnmatch(dir_name, 'python*')] + + # Add Python 3 lib directory (lib/python3.x) in front of the PYTHONPATH. This way we avoid + # issues with scripts trying to use packages / modules from Python 2.7 site-packages + # directory instead of the versions from Python 3 stdlib. + pack_actions_lib_paths = os.path.join(pack_base_path, 'actions/lib/') + pack_virtualenv_lib_path = os.path.join(virtualenv_path, 'lib') + python3_lib_directory = os.path.join(pack_virtualenv_lib_path, virtualenv_directories[0]) + + # Add Python 3 site-packages directory (lib/python3.x/site-packages) in front of the Python + # 2.7 system site-packages This is important because we want Python 3 compatible libraries + # to be used from the pack virtual environment and not system ones. + python3_site_packages_directory = os.path.join(pack_virtualenv_lib_path, + virtualenv_directories[0], + 'site-packages') + + # Work around to make sure we also add system lib dir to PYTHONPATH and not just virtualenv + # one (e.g. /usr/lib/python3.6) + # NOTE: We can't simply use sys.prefix dir since it will be set to /opt/stackstorm/st2 + + system_prefix_dirs = [] + # Take custom prefix into account (if specified) + if cfg.CONF.actionrunner.python3_prefix: + system_prefix_dirs.append(cfg.CONF.actionrunner.python3_prefix) + + # By default, Python libs are installed either in /usr/lib/python3.x or + # /usr/local/lib/python3.x + system_prefix_dirs.extend(['/usr/lib', '/usr/local/lib']) + + for system_prefix_dir in system_prefix_dirs: + python3_system_lib_directory = os.path.join(system_prefix_dir, + virtualenv_directories[0]) + + if os.path.exists(python3_system_lib_directory): + break + + if not python3_system_lib_directory or not os.path.exists(python3_system_lib_directory): + python3_system_lib_directory = None + + full_sandbox_python_path = [] + + # NOTE: Order here is very important for imports to function correctly + if python3_system_lib_directory: + full_sandbox_python_path.append(python3_system_lib_directory) + + full_sandbox_python_path.append(python3_lib_directory) + full_sandbox_python_path.append(python3_site_packages_directory) + full_sandbox_python_path.append(pack_actions_lib_paths) + full_sandbox_python_path.append(sandbox_python_path) + + sandbox_python_path = ':'.join(full_sandbox_python_path) + + return sandbox_python_path + def get_sandbox_virtualenv_path(pack): """ From 7fe81728f05d9d21974ea1f634118ab8594d86a3 Mon Sep 17 00:00:00 2001 From: blag Date: Mon, 8 Feb 2021 02:22:16 -0700 Subject: [PATCH 02/15] Simplify the sandboxing code a bit --- st2common/st2common/util/sandboxing.py | 60 +++++++------------------- 1 file changed, 16 insertions(+), 44 deletions(-) diff --git a/st2common/st2common/util/sandboxing.py b/st2common/st2common/util/sandboxing.py index 1da56186c4..0aef37a112 100644 --- a/st2common/st2common/util/sandboxing.py +++ b/st2common/st2common/util/sandboxing.py @@ -150,53 +150,25 @@ def get_sandbox_python_path_for_python_action(pack, inherit_from_parent=True, virtualenv_directories = [dir_name for dir_name in virtualenv_directories if fnmatch.fnmatch(dir_name, 'python*')] - # Add Python 3 lib directory (lib/python3.x) in front of the PYTHONPATH. This way we avoid - # issues with scripts trying to use packages / modules from Python 2.7 site-packages - # directory instead of the versions from Python 3 stdlib. - pack_actions_lib_paths = os.path.join(pack_base_path, 'actions/lib/') + # Add the pack's lib directory (lib/python3.x) in front of the PYTHONPATH. + pack_actions_lib_paths = os.path.join(pack_base_path, 'actions', 'lib') pack_virtualenv_lib_path = os.path.join(virtualenv_path, 'lib') - python3_lib_directory = os.path.join(pack_virtualenv_lib_path, virtualenv_directories[0]) + pack_venv_lib_directory = os.path.join(pack_virtualenv_lib_path, virtualenv_directories[0]) - # Add Python 3 site-packages directory (lib/python3.x/site-packages) in front of the Python - # 2.7 system site-packages This is important because we want Python 3 compatible libraries + # Add the pack's site-packages directory (lib/python3.x/site-packages) in front of the Python + # system site-packages This is important because we want Python 3 compatible libraries # to be used from the pack virtual environment and not system ones. - python3_site_packages_directory = os.path.join(pack_virtualenv_lib_path, - virtualenv_directories[0], - 'site-packages') - - # Work around to make sure we also add system lib dir to PYTHONPATH and not just virtualenv - # one (e.g. /usr/lib/python3.6) - # NOTE: We can't simply use sys.prefix dir since it will be set to /opt/stackstorm/st2 - - system_prefix_dirs = [] - # Take custom prefix into account (if specified) - if cfg.CONF.actionrunner.python3_prefix: - system_prefix_dirs.append(cfg.CONF.actionrunner.python3_prefix) - - # By default, Python libs are installed either in /usr/lib/python3.x or - # /usr/local/lib/python3.x - system_prefix_dirs.extend(['/usr/lib', '/usr/local/lib']) - - for system_prefix_dir in system_prefix_dirs: - python3_system_lib_directory = os.path.join(system_prefix_dir, - virtualenv_directories[0]) - - if os.path.exists(python3_system_lib_directory): - break - - if not python3_system_lib_directory or not os.path.exists(python3_system_lib_directory): - python3_system_lib_directory = None - - full_sandbox_python_path = [] - - # NOTE: Order here is very important for imports to function correctly - if python3_system_lib_directory: - full_sandbox_python_path.append(python3_system_lib_directory) - - full_sandbox_python_path.append(python3_lib_directory) - full_sandbox_python_path.append(python3_site_packages_directory) - full_sandbox_python_path.append(pack_actions_lib_paths) - full_sandbox_python_path.append(sandbox_python_path) + pack_venv_site_packages_directory = os.path.join(pack_virtualenv_lib_path, + virtualenv_directories[0], + 'site-packages') + + full_sandbox_python_path = [ + # NOTE: Order here is very important for imports to function correctly + pack_venv_lib_directory, + pack_venv_site_packages_directory, + pack_actions_lib_paths, + sandbox_python_path, + ] sandbox_python_path = ':'.join(full_sandbox_python_path) From de982938ecbbf183fe56a63fde6131c42dbd52ed Mon Sep 17 00:00:00 2001 From: blag Date: Mon, 8 Feb 2021 02:35:16 -0700 Subject: [PATCH 03/15] Bring back some of the sandboxing test code --- st2common/tests/unit/test_util_sandboxing.py | 33 +++++++++++++++++--- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/st2common/tests/unit/test_util_sandboxing.py b/st2common/tests/unit/test_util_sandboxing.py index af3c4328da..0358f597f9 100644 --- a/st2common/tests/unit/test_util_sandboxing.py +++ b/st2common/tests/unit/test_util_sandboxing.py @@ -120,7 +120,17 @@ def test_get_sandbox_python_path_for_python_action_python2_used_for_venv(self, inherit_from_parent=False, inherit_parent_virtualenv=False) - self.assertEqual(python_path, ':') + split = python_path.strip(':').split(':') + self.assertEqual(len(split), 3) + + # First entry should be lib/python3 dir from venv + self.assertTrue('virtualenvs/dummy_pack/lib/python3.6' in split[1]) + + # Second entry should be python3 site-packages dir from venv + self.assertTrue('virtualenvs/dummy_pack/lib/python3.6/site-packages' in split[2]) + + # Third entry should be actions/lib dir from pack root directory + self.assertTrue('packs/dummy_pack/actions/lib/' in split[3]) # Inherit python path from current process # Mock the current process python path @@ -130,6 +140,14 @@ def test_get_sandbox_python_path_for_python_action_python2_used_for_venv(self, inherit_parent_virtualenv=False) self.assertEqual(python_path, ':/data/test1:/data/test2') + python_path = get_sandbox_python_path_for_python_action(pack='dummy_pack', + inherit_from_parent=True, + inherit_parent_virtualenv=False) + expected = ('/tmp/virtualenvs/dummy_pack/lib/python3.6:' + '/tmp/virtualenvs/dummy_pack/lib/python3.6/site-packages:' + '/tmp/packs/dummy_pack/actions/lib/::/data/test1:/data/test2') + self.assertEqual(python_path, expected) + # Inherit from current process and from virtualenv (not running inside virtualenv) clear_virtualenv_prefix() @@ -140,7 +158,12 @@ def test_get_sandbox_python_path_for_python_action_python2_used_for_venv(self, # Inherit from current process and from virtualenv (running inside virtualenv) sys.real_prefix = '/usr' mock_get_python_lib.return_value = sys.prefix + '/virtualenvtest' - python_path = get_sandbox_python_path(inherit_from_parent=True, - inherit_parent_virtualenv=True) - self.assertEqual(python_path, ':/data/test1:/data/test2:%s/virtualenvtest' % - (sys.prefix)) + python_path = get_sandbox_python_path_for_python_action(pack='dummy_pack', + inherit_from_parent=True, + inherit_parent_virtualenv=True) + + expected = ('/tmp/virtualenvs/dummy_pack/lib/python3.6:' + '/tmp/virtualenvs/dummy_pack/lib/python3.6/site-packages:' + '/tmp/packs/dummy_pack/actions/lib/::/data/test1:/data/test2:' + '%s/virtualenvtest' % (sys.prefix)) + self.assertEqual(python_path, expected) From c8fd2768fb167d7776ad896400d65511316df2c5 Mon Sep 17 00:00:00 2001 From: blag Date: Mon, 8 Feb 2021 03:04:33 -0700 Subject: [PATCH 04/15] Refactor some of the test code --- st2common/tests/unit/test_util_sandboxing.py | 64 +++++++++++++------- 1 file changed, 41 insertions(+), 23 deletions(-) diff --git a/st2common/tests/unit/test_util_sandboxing.py b/st2common/tests/unit/test_util_sandboxing.py index 0358f597f9..71d00b0076 100644 --- a/st2common/tests/unit/test_util_sandboxing.py +++ b/st2common/tests/unit/test_util_sandboxing.py @@ -120,17 +120,17 @@ def test_get_sandbox_python_path_for_python_action_python2_used_for_venv(self, inherit_from_parent=False, inherit_parent_virtualenv=False) - split = python_path.strip(':').split(':') - self.assertEqual(len(split), 3) - - # First entry should be lib/python3 dir from venv - self.assertTrue('virtualenvs/dummy_pack/lib/python3.6' in split[1]) - - # Second entry should be python3 site-packages dir from venv - self.assertTrue('virtualenvs/dummy_pack/lib/python3.6/site-packages' in split[2]) - - # Third entry should be actions/lib dir from pack root directory - self.assertTrue('packs/dummy_pack/actions/lib/' in split[3]) + actual = python_path.strip(':').split(':') + self.assertEqual(len(actual), 3) + + self.assertEqual(actual, [ + # First entry should be lib/python3 dir from venv + 'virtualenvs/dummy_pack/lib/python3.6', + # Second entry should be python3 site-packages dir from venv + 'virtualenvs/dummy_pack/lib/python3.6/site-packages', + # Third entry should be actions/lib dir from pack root directory + 'packs/dummy_pack/actions/lib', + ]) # Inherit python path from current process # Mock the current process python path @@ -140,13 +140,21 @@ def test_get_sandbox_python_path_for_python_action_python2_used_for_venv(self, inherit_parent_virtualenv=False) self.assertEqual(python_path, ':/data/test1:/data/test2') - python_path = get_sandbox_python_path_for_python_action(pack='dummy_pack', + actual_path = get_sandbox_python_path_for_python_action(pack='dummy_pack', inherit_from_parent=True, inherit_parent_virtualenv=False) - expected = ('/tmp/virtualenvs/dummy_pack/lib/python3.6:' - '/tmp/virtualenvs/dummy_pack/lib/python3.6/site-packages:' - '/tmp/packs/dummy_pack/actions/lib/::/data/test1:/data/test2') - self.assertEqual(python_path, expected) + self.assertEqual(actual_path, [ + # First entry should be lib/python3 dir from venv + '/tmp/virtualenvs/dummy_pack/lib/python3.6', + # Second entry should be python3 site-packages dir from venv + '/tmp/virtualenvs/dummy_pack/lib/python3.6/site-packages', + # Third entry should be actions/lib dir from pack root directory + '/tmp/packs/dummy_pack/actions/lib', + # And the rest of the paths from get_sandbox_python_path + '', + '/data/test1', + '/data/test2', + ]) # Inherit from current process and from virtualenv (not running inside virtualenv) clear_virtualenv_prefix() @@ -155,15 +163,25 @@ def test_get_sandbox_python_path_for_python_action_python2_used_for_venv(self, inherit_parent_virtualenv=False) self.assertEqual(python_path, ':/data/test1:/data/test2') + actual_path = get_sandbox_python_path_for_python_action(pack='dummy_pack', + inherit_from_parent=True, + inherit_parent_virtualenv=True) + + self.assertEqual(actual_path, ':/data/test1:/data/test2') + # Inherit from current process and from virtualenv (running inside virtualenv) sys.real_prefix = '/usr' - mock_get_python_lib.return_value = sys.prefix + '/virtualenvtest' - python_path = get_sandbox_python_path_for_python_action(pack='dummy_pack', + mock_get_python_lib.return_value = f'{sys.prefix}virtualenvtest' + actual_path = get_sandbox_python_path_for_python_action(pack='dummy_pack', inherit_from_parent=True, inherit_parent_virtualenv=True) - expected = ('/tmp/virtualenvs/dummy_pack/lib/python3.6:' - '/tmp/virtualenvs/dummy_pack/lib/python3.6/site-packages:' - '/tmp/packs/dummy_pack/actions/lib/::/data/test1:/data/test2:' - '%s/virtualenvtest' % (sys.prefix)) - self.assertEqual(python_path, expected) + self.assertEqual(actual_path, [ + '/tmp/virtualenvs/dummy_pack/lib/python3.6', + '/tmp/virtualenvs/dummy_pack/lib/python3.6/site-packages', + '/tmp/packs/dummy_pack/actions/lib', + '', + '/data/test1', + '/data/test2', + f'{sys.prefix}/virtualenvtest', + ]) From 9b2caeeba78131249ccf5462c667b3a2cd3e91b9 Mon Sep 17 00:00:00 2001 From: blag Date: Tue, 9 Feb 2021 04:20:14 -0700 Subject: [PATCH 05/15] Ignore Python 2.7 now --- st2common/tests/unit/test_util_sandboxing.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/st2common/tests/unit/test_util_sandboxing.py b/st2common/tests/unit/test_util_sandboxing.py index 71d00b0076..72885ea51a 100644 --- a/st2common/tests/unit/test_util_sandboxing.py +++ b/st2common/tests/unit/test_util_sandboxing.py @@ -110,9 +110,9 @@ def test_get_sandbox_python_path(self, mock_get_python_lib): (sys.prefix)) @mock.patch('os.path.isdir', mock.Mock(return_value=True)) - @mock.patch('os.listdir', mock.Mock(return_value=['python2.7'])) + @mock.patch('os.listdir', mock.Mock(return_value=['python3.6'])) @mock.patch('st2common.util.sandboxing.get_python_lib') - def test_get_sandbox_python_path_for_python_action_python2_used_for_venv(self, + def test_get_sandbox_python_path_for_python_action_for_venv(self, mock_get_python_lib): # No inheritance From e8327f9c092642e31c31e9378111da6995fbdd50 Mon Sep 17 00:00:00 2001 From: blag Date: Tue, 9 Feb 2021 04:21:02 -0700 Subject: [PATCH 06/15] Add an assertEndsWith assertion method and use it --- st2common/tests/unit/test_util_sandboxing.py | 54 ++++++++++++-------- 1 file changed, 32 insertions(+), 22 deletions(-) diff --git a/st2common/tests/unit/test_util_sandboxing.py b/st2common/tests/unit/test_util_sandboxing.py index 72885ea51a..8c0d8df1d2 100644 --- a/st2common/tests/unit/test_util_sandboxing.py +++ b/st2common/tests/unit/test_util_sandboxing.py @@ -61,6 +61,16 @@ def tearDownClass(cls): os.environ['PYTHONPATH'] = cls.old_python_path set_virtualenv_prefix(cls.old_virtualenv_prefix) + def assertEndsWith(self, string, ending_substr, msg=None): + msg = msg or "'{string}'' does not end with '{ending_substr}'" + try: + assert string.endswith(ending_substr) is True + except AssertionError as e: + print(dir(e)) + print(e.args) + e.args = (msg.format(string=string, ending_substr=ending_substr),) + raise e + def test_get_sandbox_python_binary_path(self): # Non-system content pack, should use pack specific virtualenv binary result = get_sandbox_python_binary_path(pack='mapack') @@ -123,14 +133,12 @@ def test_get_sandbox_python_path_for_python_action_for_venv(self, actual = python_path.strip(':').split(':') self.assertEqual(len(actual), 3) - self.assertEqual(actual, [ - # First entry should be lib/python3 dir from venv - 'virtualenvs/dummy_pack/lib/python3.6', - # Second entry should be python3 site-packages dir from venv - 'virtualenvs/dummy_pack/lib/python3.6/site-packages', - # Third entry should be actions/lib dir from pack root directory - 'packs/dummy_pack/actions/lib', - ]) + # First entry should be lib/python3 dir from venv + self.assertEndsWith(actual[0], 'virtualenvs/dummy_pack/lib/python3.6') + # Second entry should be python3 site-packages dir from venv + self.assertEndsWith(actual[1], 'virtualenvs/dummy_pack/lib/python3.6/site-packages') + # Third entry should be actions/lib dir from pack root directory + self.assertEndsWith(actual[2], 'packs/dummy_pack/actions/lib') # Inherit python path from current process # Mock the current process python path @@ -140,21 +148,23 @@ def test_get_sandbox_python_path_for_python_action_for_venv(self, inherit_parent_virtualenv=False) self.assertEqual(python_path, ':/data/test1:/data/test2') - actual_path = get_sandbox_python_path_for_python_action(pack='dummy_pack', + python_path = get_sandbox_python_path_for_python_action(pack='dummy_pack', inherit_from_parent=True, inherit_parent_virtualenv=False) - self.assertEqual(actual_path, [ - # First entry should be lib/python3 dir from venv - '/tmp/virtualenvs/dummy_pack/lib/python3.6', - # Second entry should be python3 site-packages dir from venv - '/tmp/virtualenvs/dummy_pack/lib/python3.6/site-packages', - # Third entry should be actions/lib dir from pack root directory - '/tmp/packs/dummy_pack/actions/lib', - # And the rest of the paths from get_sandbox_python_path - '', - '/data/test1', - '/data/test2', - ]) + + actual = python_path.strip(':').split(':') + self.assertEqual(len(actual), 6) + + # First entry should be lib/python3 dir from venv + self.assertEndsWith(actual[0], 'virtualenvs/dummy_pack/lib/python3.6') + # Second entry should be python3 site-packages dir from venv + self.assertEndsWith(actual[1], 'virtualenvs/dummy_pack/lib/python3.6/site-packages') + # Third entry should be actions/lib dir from pack root directory + self.assertEndsWith(actual[2], 'packs/dummy_pack/actions/lib') + # And the rest of the paths from get_sandbox_python_path + self.assertEqual(actual[3], '') + self.assertEqual(actual[4], '/data/test1') + self.assertEqual(actual[5], '/data/test2') # Inherit from current process and from virtualenv (not running inside virtualenv) clear_virtualenv_prefix() @@ -167,7 +177,7 @@ def test_get_sandbox_python_path_for_python_action_for_venv(self, inherit_from_parent=True, inherit_parent_virtualenv=True) - self.assertEqual(actual_path, ':/data/test1:/data/test2') + self.assertEndsWith(actual_path, ':/data/test1:/data/test2') # Inherit from current process and from virtualenv (running inside virtualenv) sys.real_prefix = '/usr' From 4087e5388e1b0e2d68eb309b71c3ce84698f5830 Mon Sep 17 00:00:00 2001 From: blag Date: Thu, 11 Feb 2021 13:08:54 -0700 Subject: [PATCH 07/15] Silence a completely unrelated file --- st2common/st2common/models/db/execution.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/st2common/st2common/models/db/execution.py b/st2common/st2common/models/db/execution.py index 887cedea55..3e8f3c7742 100644 --- a/st2common/st2common/models/db/execution.py +++ b/st2common/st2common/models/db/execution.py @@ -97,8 +97,8 @@ class ActionExecutionDB(stormbase.StormFoundationDB): } def get_uid(self): - # TODO Construct od from non id field: - uid = [self.RESOURCE_TYPE, str(self.id)] + # TODO Construct id from non id field: + uid = [self.RESOURCE_TYPE, str(self.id)] # pylint: disable=no-member return ':'.join(uid) def mask_secrets(self, value): From 5f66c492ac3ed59dd6373779e576a876baab0d5b Mon Sep 17 00:00:00 2001 From: blag Date: Thu, 11 Feb 2021 13:13:30 -0700 Subject: [PATCH 08/15] Set maxDiff = None --- st2common/tests/unit/test_util_sandboxing.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/st2common/tests/unit/test_util_sandboxing.py b/st2common/tests/unit/test_util_sandboxing.py index 8c0d8df1d2..642959caa7 100644 --- a/st2common/tests/unit/test_util_sandboxing.py +++ b/st2common/tests/unit/test_util_sandboxing.py @@ -38,6 +38,8 @@ class SandboxingUtilsTestCase(unittest.TestCase): + maxDiff = None + def setUp(self): super(SandboxingUtilsTestCase, self).setUp() From f9c363e87a23f9cbf3e09d226ce12f0700f727a4 Mon Sep 17 00:00:00 2001 From: blag Date: Thu, 11 Feb 2021 13:14:03 -0700 Subject: [PATCH 09/15] Convert the PYTHONPATH string to a list before comparing --- st2common/tests/unit/test_util_sandboxing.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/st2common/tests/unit/test_util_sandboxing.py b/st2common/tests/unit/test_util_sandboxing.py index 642959caa7..46cc093986 100644 --- a/st2common/tests/unit/test_util_sandboxing.py +++ b/st2common/tests/unit/test_util_sandboxing.py @@ -184,10 +184,12 @@ def test_get_sandbox_python_path_for_python_action_for_venv(self, # Inherit from current process and from virtualenv (running inside virtualenv) sys.real_prefix = '/usr' mock_get_python_lib.return_value = f'{sys.prefix}virtualenvtest' - actual_path = get_sandbox_python_path_for_python_action(pack='dummy_pack', + python_path = get_sandbox_python_path_for_python_action(pack='dummy_pack', inherit_from_parent=True, inherit_parent_virtualenv=True) + actual_path = python_path.strip(':').split(':') + self.assertEqual(actual_path, [ '/tmp/virtualenvs/dummy_pack/lib/python3.6', '/tmp/virtualenvs/dummy_pack/lib/python3.6/site-packages', From 74fb938c83d02361e13f2c92a092feced0de9eda Mon Sep 17 00:00:00 2001 From: blag Date: Thu, 11 Feb 2021 16:32:25 -0700 Subject: [PATCH 10/15] Fix assertions --- st2common/tests/unit/test_util_sandboxing.py | 66 ++++++++++++-------- 1 file changed, 41 insertions(+), 25 deletions(-) diff --git a/st2common/tests/unit/test_util_sandboxing.py b/st2common/tests/unit/test_util_sandboxing.py index 46cc093986..602d7a4778 100644 --- a/st2common/tests/unit/test_util_sandboxing.py +++ b/st2common/tests/unit/test_util_sandboxing.py @@ -132,15 +132,15 @@ def test_get_sandbox_python_path_for_python_action_for_venv(self, inherit_from_parent=False, inherit_parent_virtualenv=False) - actual = python_path.strip(':').split(':') - self.assertEqual(len(actual), 3) + actual_path = python_path.strip(':').split(':') + self.assertEqual(len(actual_path), 3) # First entry should be lib/python3 dir from venv - self.assertEndsWith(actual[0], 'virtualenvs/dummy_pack/lib/python3.6') + self.assertEndsWith(actual_path[0], 'virtualenvs/dummy_pack/lib/python3.6') # Second entry should be python3 site-packages dir from venv - self.assertEndsWith(actual[1], 'virtualenvs/dummy_pack/lib/python3.6/site-packages') + self.assertEndsWith(actual_path[1], 'virtualenvs/dummy_pack/lib/python3.6/site-packages') # Third entry should be actions/lib dir from pack root directory - self.assertEndsWith(actual[2], 'packs/dummy_pack/actions/lib') + self.assertEndsWith(actual_path[2], 'packs/dummy_pack/actions/lib') # Inherit python path from current process # Mock the current process python path @@ -154,19 +154,19 @@ def test_get_sandbox_python_path_for_python_action_for_venv(self, inherit_from_parent=True, inherit_parent_virtualenv=False) - actual = python_path.strip(':').split(':') - self.assertEqual(len(actual), 6) + actual_path = python_path.strip(':').split(':') + self.assertEqual(len(actual_path), 6) # First entry should be lib/python3 dir from venv - self.assertEndsWith(actual[0], 'virtualenvs/dummy_pack/lib/python3.6') + self.assertEndsWith(actual_path[0], 'virtualenvs/dummy_pack/lib/python3.6') # Second entry should be python3 site-packages dir from venv - self.assertEndsWith(actual[1], 'virtualenvs/dummy_pack/lib/python3.6/site-packages') + self.assertEndsWith(actual_path[1], 'virtualenvs/dummy_pack/lib/python3.6/site-packages') # Third entry should be actions/lib dir from pack root directory - self.assertEndsWith(actual[2], 'packs/dummy_pack/actions/lib') + self.assertEndsWith(actual_path[2], 'packs/dummy_pack/actions/lib') # And the rest of the paths from get_sandbox_python_path - self.assertEqual(actual[3], '') - self.assertEqual(actual[4], '/data/test1') - self.assertEqual(actual[5], '/data/test2') + self.assertEqual(actual_path[3], '') + self.assertEqual(actual_path[4], '/data/test1') + self.assertEqual(actual_path[5], '/data/test2') # Inherit from current process and from virtualenv (not running inside virtualenv) clear_virtualenv_prefix() @@ -175,27 +175,43 @@ def test_get_sandbox_python_path_for_python_action_for_venv(self, inherit_parent_virtualenv=False) self.assertEqual(python_path, ':/data/test1:/data/test2') - actual_path = get_sandbox_python_path_for_python_action(pack='dummy_pack', + python_path = get_sandbox_python_path_for_python_action(pack='dummy_pack', inherit_from_parent=True, inherit_parent_virtualenv=True) - self.assertEndsWith(actual_path, ':/data/test1:/data/test2') + actual_path = python_path.strip(':').split(':') + self.assertEqual(len(actual_path), 6) + + # First entry should be lib/python3 dir from venv + self.assertEndsWith(actual_path[0], 'virtualenvs/dummy_pack/lib/python3.6') + # Second entry should be python3 site-packages dir from venv + self.assertEndsWith(actual_path[1], 'virtualenvs/dummy_pack/lib/python3.6/site-packages') + # Third entry should be actions/lib dir from pack root directory + self.assertEndsWith(actual_path[2], 'packs/dummy_pack/actions/lib') + # And the rest of the paths from get_sandbox_python_path + self.assertEqual(actual_path[3], '') + self.assertEqual(actual_path[4], '/data/test1') + self.assertEqual(actual_path[5], '/data/test2') # Inherit from current process and from virtualenv (running inside virtualenv) sys.real_prefix = '/usr' - mock_get_python_lib.return_value = f'{sys.prefix}virtualenvtest' + mock_get_python_lib.return_value = f'{sys.prefix}/virtualenvtest' python_path = get_sandbox_python_path_for_python_action(pack='dummy_pack', inherit_from_parent=True, inherit_parent_virtualenv=True) actual_path = python_path.strip(':').split(':') + self.assertEqual(len(actual_path), 7) - self.assertEqual(actual_path, [ - '/tmp/virtualenvs/dummy_pack/lib/python3.6', - '/tmp/virtualenvs/dummy_pack/lib/python3.6/site-packages', - '/tmp/packs/dummy_pack/actions/lib', - '', - '/data/test1', - '/data/test2', - f'{sys.prefix}/virtualenvtest', - ]) + # First entry should be lib/python3 dir from venv + self.assertEndsWith(actual_path[0], 'virtualenvs/dummy_pack/lib/python3.6') + # Second entry should be python3 site-packages dir from venv + self.assertEndsWith(actual_path[1], 'virtualenvs/dummy_pack/lib/python3.6/site-packages') + # Third entry should be actions/lib dir from pack root directory + self.assertEndsWith(actual_path[2], 'packs/dummy_pack/actions/lib') + # The paths from get_sandbox_python_path + self.assertEqual(actual_path[3], '') + self.assertEqual(actual_path[4], '/data/test1') + self.assertEqual(actual_path[5], '/data/test2') + # And the parent virtualenv + self.assertEqual(actual_path[6], f'{sys.prefix}/virtualenvtest') From 25bca88fb91f14ec73e6c6e2cf0ff0b6a7e3ee78 Mon Sep 17 00:00:00 2001 From: blag Date: Thu, 11 Feb 2021 18:02:52 -0700 Subject: [PATCH 11/15] Check that the pack virtualenv exists before trying to list its contents --- st2common/st2common/util/sandboxing.py | 58 +++++++++++++------------- 1 file changed, 28 insertions(+), 30 deletions(-) diff --git a/st2common/st2common/util/sandboxing.py b/st2common/st2common/util/sandboxing.py index 0aef37a112..d9f902b7aa 100644 --- a/st2common/st2common/util/sandboxing.py +++ b/st2common/st2common/util/sandboxing.py @@ -141,36 +141,34 @@ def get_sandbox_python_path_for_python_action(pack, inherit_from_parent=True, pack_base_path = get_pack_base_path(pack_name=pack) virtualenv_path = get_sandbox_virtualenv_path(pack=pack) - if not virtualenv_path: - return sandbox_python_path - - pack_virtualenv_lib_path = os.path.join(virtualenv_path, 'lib') - - virtualenv_directories = os.listdir(pack_virtualenv_lib_path) - virtualenv_directories = [dir_name for dir_name in virtualenv_directories if - fnmatch.fnmatch(dir_name, 'python*')] - - # Add the pack's lib directory (lib/python3.x) in front of the PYTHONPATH. - pack_actions_lib_paths = os.path.join(pack_base_path, 'actions', 'lib') - pack_virtualenv_lib_path = os.path.join(virtualenv_path, 'lib') - pack_venv_lib_directory = os.path.join(pack_virtualenv_lib_path, virtualenv_directories[0]) - - # Add the pack's site-packages directory (lib/python3.x/site-packages) in front of the Python - # system site-packages This is important because we want Python 3 compatible libraries - # to be used from the pack virtual environment and not system ones. - pack_venv_site_packages_directory = os.path.join(pack_virtualenv_lib_path, - virtualenv_directories[0], - 'site-packages') - - full_sandbox_python_path = [ - # NOTE: Order here is very important for imports to function correctly - pack_venv_lib_directory, - pack_venv_site_packages_directory, - pack_actions_lib_paths, - sandbox_python_path, - ] - - sandbox_python_path = ':'.join(full_sandbox_python_path) + if virtualenv_path and os.path.isdir(virtualenv_path): + pack_virtualenv_lib_path = os.path.join(virtualenv_path, 'lib') + + virtualenv_directories = os.listdir(pack_virtualenv_lib_path) + virtualenv_directories = [dir_name for dir_name in virtualenv_directories if + fnmatch.fnmatch(dir_name, 'python*')] + + # Add the pack's lib directory (lib/python3.x) in front of the PYTHONPATH. + pack_actions_lib_paths = os.path.join(pack_base_path, 'actions', 'lib') + pack_virtualenv_lib_path = os.path.join(virtualenv_path, 'lib') + pack_venv_lib_directory = os.path.join(pack_virtualenv_lib_path, virtualenv_directories[0]) + + # Add the pack's site-packages directory (lib/python3.x/site-packages) in front of the Python + # system site-packages This is important because we want Python 3 compatible libraries + # to be used from the pack virtual environment and not system ones. + pack_venv_site_packages_directory = os.path.join(pack_virtualenv_lib_path, + virtualenv_directories[0], + 'site-packages') + + full_sandbox_python_path = [ + # NOTE: Order here is very important for imports to function correctly + pack_venv_lib_directory, + pack_venv_site_packages_directory, + pack_actions_lib_paths, + sandbox_python_path, + ] + + sandbox_python_path = ':'.join(full_sandbox_python_path) return sandbox_python_path From 302d55099806a988f2edcb568d85f6209a03be26 Mon Sep 17 00:00:00 2001 From: blag Date: Thu, 11 Feb 2021 18:09:05 -0700 Subject: [PATCH 12/15] Reformat comment --- st2common/st2common/util/sandboxing.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/st2common/st2common/util/sandboxing.py b/st2common/st2common/util/sandboxing.py index d9f902b7aa..02871f7472 100644 --- a/st2common/st2common/util/sandboxing.py +++ b/st2common/st2common/util/sandboxing.py @@ -153,9 +153,10 @@ def get_sandbox_python_path_for_python_action(pack, inherit_from_parent=True, pack_virtualenv_lib_path = os.path.join(virtualenv_path, 'lib') pack_venv_lib_directory = os.path.join(pack_virtualenv_lib_path, virtualenv_directories[0]) - # Add the pack's site-packages directory (lib/python3.x/site-packages) in front of the Python - # system site-packages This is important because we want Python 3 compatible libraries - # to be used from the pack virtual environment and not system ones. + # Add the pack's site-packages directory (lib/python3.x/site-packages) + # in front of the Python system site-packages This is important because + # we want Python 3 compatible libraries to be used from the pack virtual + # environment and not system ones. pack_venv_site_packages_directory = os.path.join(pack_virtualenv_lib_path, virtualenv_directories[0], 'site-packages') From 5c3c6240e2983a5203d108a695f90d3c2b30938c Mon Sep 17 00:00:00 2001 From: blag Date: Thu, 11 Feb 2021 23:10:36 -0700 Subject: [PATCH 13/15] Split up test_get_sandbox_python_path_for_python_action function --- st2common/tests/unit/test_util_sandboxing.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/st2common/tests/unit/test_util_sandboxing.py b/st2common/tests/unit/test_util_sandboxing.py index 602d7a4778..42d4eedb4f 100644 --- a/st2common/tests/unit/test_util_sandboxing.py +++ b/st2common/tests/unit/test_util_sandboxing.py @@ -124,7 +124,7 @@ def test_get_sandbox_python_path(self, mock_get_python_lib): @mock.patch('os.path.isdir', mock.Mock(return_value=True)) @mock.patch('os.listdir', mock.Mock(return_value=['python3.6'])) @mock.patch('st2common.util.sandboxing.get_python_lib') - def test_get_sandbox_python_path_for_python_action_for_venv(self, + def test_get_sandbox_python_path_for_python_action_no_inheritance(self, mock_get_python_lib): # No inheritance @@ -142,6 +142,12 @@ def test_get_sandbox_python_path_for_python_action_for_venv(self, # Third entry should be actions/lib dir from pack root directory self.assertEndsWith(actual_path[2], 'packs/dummy_pack/actions/lib') + @mock.patch('os.path.isdir', mock.Mock(return_value=True)) + @mock.patch('os.listdir', mock.Mock(return_value=['python3.6'])) + @mock.patch('st2common.util.sandboxing.get_python_lib') + def test_get_sandbox_python_path_for_python_action_inherit_from_parent_process_only(self, + mock_get_python_lib): + # Inherit python path from current process # Mock the current process python path os.environ['PYTHONPATH'] = ':/data/test1:/data/test2' @@ -168,6 +174,16 @@ def test_get_sandbox_python_path_for_python_action_for_venv(self, self.assertEqual(actual_path[4], '/data/test1') self.assertEqual(actual_path[5], '/data/test2') + @mock.patch('os.path.isdir', mock.Mock(return_value=True)) + @mock.patch('os.listdir', mock.Mock(return_value=['python3.6'])) + @mock.patch('st2common.util.sandboxing.get_python_lib') + def test_get_sandbox_python_path_for_python_action_inherit_from_parent_process_and_venv(self, + mock_get_python_lib): + + # Inherit python path from current process + # Mock the current process python path + os.environ['PYTHONPATH'] = ':/data/test1:/data/test2' + # Inherit from current process and from virtualenv (not running inside virtualenv) clear_virtualenv_prefix() From c244946ed16f28ef077c248cc6a5942fb37c7561 Mon Sep 17 00:00:00 2001 From: blag Date: Thu, 11 Feb 2021 23:21:44 -0700 Subject: [PATCH 14/15] Use f-strings --- st2common/tests/unit/test_util_sandboxing.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/st2common/tests/unit/test_util_sandboxing.py b/st2common/tests/unit/test_util_sandboxing.py index 42d4eedb4f..e3e4917e39 100644 --- a/st2common/tests/unit/test_util_sandboxing.py +++ b/st2common/tests/unit/test_util_sandboxing.py @@ -89,7 +89,8 @@ def test_get_sandbox_path(self): virtualenv_path = '/home/venv/test' result = get_sandbox_path(virtualenv_path=virtualenv_path) - self.assertEqual(result, '/home/venv/test/bin/:/home/path1:/home/path2:/home/path3') + + self.assertEqual(result, f'{virtualenv_path}/bin/:/home/path1:/home/path2:/home/path3') @mock.patch('st2common.util.sandboxing.get_python_lib') def test_get_sandbox_python_path(self, mock_get_python_lib): @@ -115,11 +116,10 @@ def test_get_sandbox_python_path(self, mock_get_python_lib): # Inherit from current process and from virtualenv (running inside virtualenv) sys.real_prefix = '/usr' - mock_get_python_lib.return_value = sys.prefix + '/virtualenvtest' + mock_get_python_lib.return_value = f'{sys.prefix}/virtualenvtest' python_path = get_sandbox_python_path(inherit_from_parent=True, inherit_parent_virtualenv=True) - self.assertEqual(python_path, ':/data/test1:/data/test2:%s/virtualenvtest' % - (sys.prefix)) + self.assertEqual(python_path, f':/data/test1:/data/test2:{sys.prefix}/virtualenvtest') @mock.patch('os.path.isdir', mock.Mock(return_value=True)) @mock.patch('os.listdir', mock.Mock(return_value=['python3.6'])) From 7bdb1373bce2c3acecee112378a61ec04dc3d3aa Mon Sep 17 00:00:00 2001 From: blag Date: Fri, 12 Feb 2021 00:12:42 -0700 Subject: [PATCH 15/15] Properly mock os.environ --- st2common/tests/unit/test_util_sandboxing.py | 79 ++++++++++---------- 1 file changed, 41 insertions(+), 38 deletions(-) diff --git a/st2common/tests/unit/test_util_sandboxing.py b/st2common/tests/unit/test_util_sandboxing.py index e3e4917e39..5f387e0067 100644 --- a/st2common/tests/unit/test_util_sandboxing.py +++ b/st2common/tests/unit/test_util_sandboxing.py @@ -43,9 +43,7 @@ class SandboxingUtilsTestCase(unittest.TestCase): def setUp(self): super(SandboxingUtilsTestCase, self).setUp() - # Restore PATH and other variables before each test case - os.environ['PATH'] = self.old_path - os.environ['PYTHONPATH'] = self.old_python_path + # Restore the virtualenv before each test case set_virtualenv_prefix(self.old_virtualenv_prefix) @classmethod @@ -53,14 +51,10 @@ def setUpClass(cls): tests_config.parse_args() # Store original values so we can restore them in setUp - cls.old_path = os.environ.get('PATH', '') - cls.old_python_path = os.environ.get('PYTHONPATH', '') cls.old_virtualenv_prefix = get_virtualenv_prefix() @classmethod def tearDownClass(cls): - os.environ['PATH'] = cls.old_path - os.environ['PYTHONPATH'] = cls.old_python_path set_virtualenv_prefix(cls.old_virtualenv_prefix) def assertEndsWith(self, string, ending_substr, msg=None): @@ -84,11 +78,11 @@ def test_get_sandbox_python_binary_path(self): self.assertEqual(result, sys.executable) def test_get_sandbox_path(self): - # Mock the current PATH value - os.environ['PATH'] = '/home/path1:/home/path2:/home/path3:' - virtualenv_path = '/home/venv/test' - result = get_sandbox_path(virtualenv_path=virtualenv_path) + + # Mock the current PATH value + with mock.patch.dict(os.environ, {'PATH': '/home/path1:/home/path2:/home/path3:'}): + result = get_sandbox_path(virtualenv_path=virtualenv_path) self.assertEqual(result, f'{virtualenv_path}/bin/:/home/path1:/home/path2:/home/path3') @@ -101,24 +95,29 @@ def test_get_sandbox_python_path(self, mock_get_python_lib): # Inherit python path from current process # Mock the current process python path - os.environ['PYTHONPATH'] = ':/data/test1:/data/test2' + with mock.patch.dict(os.environ, {'PYTHONPATH': ':/data/test1:/data/test2'}): + python_path = get_sandbox_python_path(inherit_from_parent=True, + inherit_parent_virtualenv=False) - python_path = get_sandbox_python_path(inherit_from_parent=True, - inherit_parent_virtualenv=False) self.assertEqual(python_path, ':/data/test1:/data/test2') # Inherit from current process and from virtualenv (not running inside virtualenv) clear_virtualenv_prefix() - python_path = get_sandbox_python_path(inherit_from_parent=True, - inherit_parent_virtualenv=False) + with mock.patch.dict(os.environ, {'PYTHONPATH': ':/data/test1:/data/test2'}): + python_path = get_sandbox_python_path(inherit_from_parent=True, + inherit_parent_virtualenv=False) + self.assertEqual(python_path, ':/data/test1:/data/test2') # Inherit from current process and from virtualenv (running inside virtualenv) sys.real_prefix = '/usr' mock_get_python_lib.return_value = f'{sys.prefix}/virtualenvtest' - python_path = get_sandbox_python_path(inherit_from_parent=True, - inherit_parent_virtualenv=True) + + with mock.patch.dict(os.environ, {'PYTHONPATH': ':/data/test1:/data/test2'}): + python_path = get_sandbox_python_path(inherit_from_parent=True, + inherit_parent_virtualenv=True) + self.assertEqual(python_path, f':/data/test1:/data/test2:{sys.prefix}/virtualenvtest') @mock.patch('os.path.isdir', mock.Mock(return_value=True)) @@ -150,15 +149,15 @@ def test_get_sandbox_python_path_for_python_action_inherit_from_parent_process_o # Inherit python path from current process # Mock the current process python path - os.environ['PYTHONPATH'] = ':/data/test1:/data/test2' + with mock.patch.dict(os.environ, {'PYTHONPATH': ':/data/test1:/data/test2'}): + python_path = get_sandbox_python_path(inherit_from_parent=True, + inherit_parent_virtualenv=False) - python_path = get_sandbox_python_path(inherit_from_parent=True, - inherit_parent_virtualenv=False) - self.assertEqual(python_path, ':/data/test1:/data/test2') + self.assertEqual(python_path, ':/data/test1:/data/test2') - python_path = get_sandbox_python_path_for_python_action(pack='dummy_pack', - inherit_from_parent=True, - inherit_parent_virtualenv=False) + python_path = get_sandbox_python_path_for_python_action(pack='dummy_pack', + inherit_from_parent=True, + inherit_parent_virtualenv=False) actual_path = python_path.strip(':').split(':') self.assertEqual(len(actual_path), 6) @@ -180,20 +179,20 @@ def test_get_sandbox_python_path_for_python_action_inherit_from_parent_process_o def test_get_sandbox_python_path_for_python_action_inherit_from_parent_process_and_venv(self, mock_get_python_lib): - # Inherit python path from current process - # Mock the current process python path - os.environ['PYTHONPATH'] = ':/data/test1:/data/test2' - # Inherit from current process and from virtualenv (not running inside virtualenv) clear_virtualenv_prefix() - python_path = get_sandbox_python_path(inherit_from_parent=True, - inherit_parent_virtualenv=False) - self.assertEqual(python_path, ':/data/test1:/data/test2') + # Inherit python path from current process + # Mock the current process python path + with mock.patch.dict(os.environ, {'PYTHONPATH': ':/data/test1:/data/test2'}): + python_path = get_sandbox_python_path(inherit_from_parent=True, + inherit_parent_virtualenv=False) - python_path = get_sandbox_python_path_for_python_action(pack='dummy_pack', - inherit_from_parent=True, - inherit_parent_virtualenv=True) + self.assertEqual(python_path, ':/data/test1:/data/test2') + + python_path = get_sandbox_python_path_for_python_action(pack='dummy_pack', + inherit_from_parent=True, + inherit_parent_virtualenv=True) actual_path = python_path.strip(':').split(':') self.assertEqual(len(actual_path), 6) @@ -212,9 +211,13 @@ def test_get_sandbox_python_path_for_python_action_inherit_from_parent_process_a # Inherit from current process and from virtualenv (running inside virtualenv) sys.real_prefix = '/usr' mock_get_python_lib.return_value = f'{sys.prefix}/virtualenvtest' - python_path = get_sandbox_python_path_for_python_action(pack='dummy_pack', - inherit_from_parent=True, - inherit_parent_virtualenv=True) + + # Inherit python path from current process + # Mock the current process python path + with mock.patch.dict(os.environ, {'PYTHONPATH': ':/data/test1:/data/test2'}): + python_path = get_sandbox_python_path_for_python_action(pack='dummy_pack', + inherit_from_parent=True, + inherit_parent_virtualenv=True) actual_path = python_path.strip(':').split(':') self.assertEqual(len(actual_path), 7)