From 3687fbb7acb766c479cb4b78d75c9d574a07b4ce Mon Sep 17 00:00:00 2001 From: David Shrewsbury Date: Mon, 31 Jan 2022 11:03:34 -0500 Subject: [PATCH] Fix bug with sandboxing with values from env/settings (#978) (#982) [backport][release_2.0] Fix bug with sandboxing with values from env/settings (#978) Backport of PR #978 This also standardizes .gitignore from a change that was not backported to this branch. (cherry picked from commit 996a00d) Reviewed-by: Alexander Sowitzki Reviewed-by: None --- .gitignore | 55 ++++++++++--------- ansible_runner/config/runner.py | 11 +++- test/conftest.py | 12 ++++ .../projects/containerized/env/envvars | 0 .../projects/containerized/env/settings | 0 .../projects/containerized/inventory/hosts | 0 .../containerized/project/test-container.yml | 0 .../fixtures/projects/debug/env/envvars | 0 .../fixtures/projects/debug/inventory/inv_1 | 0 .../fixtures/projects/debug/inventory/inv_2 | 0 .../fixtures/projects/debug/project/debug.yml | 0 .../project/roles/hello_world/tasks/main.yml | 0 .../projects/directory_isolation/env/settings | 3 + .../directory_isolation/project/main.yml | 5 ++ .../fixtures/projects/files/test_ee.py | 0 .../fixtures/projects/host_status/env/envvars | 0 .../fixtures/projects/host_status/inventory | 0 .../host_status/project/gen_host_status.yml | 0 .../fixtures/projects/job_env/env/envvars | 0 .../fixtures/projects/job_env/env/settings | 0 .../fixtures/projects/job_env/inventory/hosts | 0 .../projects/job_env/project/printenv.yml | 0 .../fixtures/projects/printenv/env/envvars | 0 .../projects/printenv/inventory/hosts | 0 .../action_plugins/look_at_environment.py | 0 .../printenv/project/get_environment.yml | 0 .../fixtures/projects/sleep/env/envvars | 0 .../fixtures/projects/sleep/inventory/hosts | 0 .../fixtures/projects/sleep/project/sleep.yml | 0 .../fixtures/projects/use_role/env/envvars | 0 .../projects/use_role/inventory/hosts | 0 .../meta/.galaxy_install_info | 0 .../benthomasson.hello_role/meta/main.yml | 0 .../benthomasson.hello_role/tasks/main.yml | 0 .../fixtures/projects/use_role/use_role.yml | 0 test/integration/conftest.py | 15 ----- test/unit/config/test_runner.py | 33 ++++++++++- 37 files changed, 90 insertions(+), 44 deletions(-) rename test/{integration => }/fixtures/projects/containerized/env/envvars (100%) rename test/{integration => }/fixtures/projects/containerized/env/settings (100%) rename test/{integration => }/fixtures/projects/containerized/inventory/hosts (100%) rename test/{integration => }/fixtures/projects/containerized/project/test-container.yml (100%) rename test/{integration => }/fixtures/projects/debug/env/envvars (100%) rename test/{integration => }/fixtures/projects/debug/inventory/inv_1 (100%) rename test/{integration => }/fixtures/projects/debug/inventory/inv_2 (100%) rename test/{integration => }/fixtures/projects/debug/project/debug.yml (100%) rename test/{integration => }/fixtures/projects/debug/project/roles/hello_world/tasks/main.yml (100%) create mode 100644 test/fixtures/projects/directory_isolation/env/settings create mode 100644 test/fixtures/projects/directory_isolation/project/main.yml rename test/{integration => }/fixtures/projects/files/test_ee.py (100%) rename test/{integration => }/fixtures/projects/host_status/env/envvars (100%) rename test/{integration => }/fixtures/projects/host_status/inventory (100%) rename test/{integration => }/fixtures/projects/host_status/project/gen_host_status.yml (100%) rename test/{integration => }/fixtures/projects/job_env/env/envvars (100%) rename test/{integration => }/fixtures/projects/job_env/env/settings (100%) rename test/{integration => }/fixtures/projects/job_env/inventory/hosts (100%) rename test/{integration => }/fixtures/projects/job_env/project/printenv.yml (100%) rename test/{integration => }/fixtures/projects/printenv/env/envvars (100%) rename test/{integration => }/fixtures/projects/printenv/inventory/hosts (100%) rename test/{integration => }/fixtures/projects/printenv/project/action_plugins/look_at_environment.py (100%) rename test/{integration => }/fixtures/projects/printenv/project/get_environment.yml (100%) rename test/{integration => }/fixtures/projects/sleep/env/envvars (100%) rename test/{integration => }/fixtures/projects/sleep/inventory/hosts (100%) rename test/{integration => }/fixtures/projects/sleep/project/sleep.yml (100%) rename test/{integration => }/fixtures/projects/use_role/env/envvars (100%) rename test/{integration => }/fixtures/projects/use_role/inventory/hosts (100%) rename test/{integration => }/fixtures/projects/use_role/roles/benthomasson.hello_role/meta/.galaxy_install_info (100%) rename test/{integration => }/fixtures/projects/use_role/roles/benthomasson.hello_role/meta/main.yml (100%) rename test/{integration => }/fixtures/projects/use_role/roles/benthomasson.hello_role/tasks/main.yml (100%) rename test/{integration => }/fixtures/projects/use_role/use_role.yml (100%) diff --git a/.gitignore b/.gitignore index 693b8a857..f342e789b 100644 --- a/.gitignore +++ b/.gitignore @@ -1,29 +1,34 @@ -# Mac OS X -*.DS_Store +# Test artifacts +*.py,cover +.pytest_cache/ +docs/_build/ +docs/build/ +pytestdebug.log +test/coverage/ + +# Byte-complied files +*.py[cod] +__pycache__/ -# Editors -*.sw[poj] -*~ -/.idea +# Distribution / packaging +*.egg +*.egg-info/ +.eggs/ +AUTHORS +Changelog +build/ +deb-build/ +dist/ +eggs/ +rpm-build/ + +# Environments +.env +.python-version +.tox/ +.venv +venv/ +# Demo files /demo/artifacts /demo/daemon.log -/docs/_build -/.tox -/dist -/build -/rpm-build -/deb-build -/*.egg-info -*.py[c,o] -.pytest_cache -pytestdebug.log -.coverage -*,cover -.venv -/venv -.env -/.eggs/ -docs/build -AUTHORS -ChangeLog diff --git a/ansible_runner/config/runner.py b/ansible_runner/config/runner.py index 27ed7ef99..6928342c9 100644 --- a/ansible_runner/config/runner.py +++ b/ansible_runner/config/runner.py @@ -23,8 +23,8 @@ import stat import tempfile import six +import distutils.dir_util -from distutils.dir_util import copy_tree from six import string_types, text_type from ansible_runner import output @@ -134,14 +134,18 @@ def prepare(self): raise ConfigurationError("Only one of playbook and module options are allowed") if not os.path.exists(self.artifact_dir): os.makedirs(self.artifact_dir, mode=0o700) + + # Since the `sandboxed` property references attributes that may come from `env/settings`, + # we must call prepare_env() before we can reference it. + self.prepare_env() + if self.sandboxed and self.directory_isolation_path is not None: self.directory_isolation_path = tempfile.mkdtemp(prefix='runner_di_', dir=self.directory_isolation_path) if os.path.exists(self.project_dir): output.debug("Copying directory tree from {} to {} for working directory isolation".format(self.project_dir, self.directory_isolation_path)) - copy_tree(self.project_dir, self.directory_isolation_path, preserve_symlinks=True) + distutils.dir_util.copy_tree(self.project_dir, self.directory_isolation_path, preserve_symlinks=True) - self.prepare_env() self.prepare_inventory() self.prepare_command() @@ -185,6 +189,7 @@ def prepare_env(self): self.process_isolation_hide_paths = self.settings.get('process_isolation_hide_paths', self.process_isolation_hide_paths) self.process_isolation_show_paths = self.settings.get('process_isolation_show_paths', self.process_isolation_show_paths) self.process_isolation_ro_paths = self.settings.get('process_isolation_ro_paths', self.process_isolation_ro_paths) + self.directory_isolation_path = self.settings.get('directory_isolation_base_path', self.directory_isolation_path) self.directory_isolation_cleanup = bool(self.settings.get('directory_isolation_cleanup', True)) self.resource_profiling = self.settings.get('resource_profiling', self.resource_profiling) diff --git a/test/conftest.py b/test/conftest.py index feea1d082..2ef211d5c 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -1,6 +1,7 @@ import shutil from distutils.version import LooseVersion +from pathlib import Path from ansible_runner import defaults @@ -64,3 +65,14 @@ def pytest_generate_tests(metafunc): ) metafunc.parametrize('runtime', args) break + + +@pytest.fixture +def project_fixtures(tmp_path): + source = Path(__file__).parent / 'fixtures' / 'projects' + dest = tmp_path / 'projects' + shutil.copytree(source, dest) + + yield dest + + shutil.rmtree(dest, ignore_errors=True) diff --git a/test/integration/fixtures/projects/containerized/env/envvars b/test/fixtures/projects/containerized/env/envvars similarity index 100% rename from test/integration/fixtures/projects/containerized/env/envvars rename to test/fixtures/projects/containerized/env/envvars diff --git a/test/integration/fixtures/projects/containerized/env/settings b/test/fixtures/projects/containerized/env/settings similarity index 100% rename from test/integration/fixtures/projects/containerized/env/settings rename to test/fixtures/projects/containerized/env/settings diff --git a/test/integration/fixtures/projects/containerized/inventory/hosts b/test/fixtures/projects/containerized/inventory/hosts similarity index 100% rename from test/integration/fixtures/projects/containerized/inventory/hosts rename to test/fixtures/projects/containerized/inventory/hosts diff --git a/test/integration/fixtures/projects/containerized/project/test-container.yml b/test/fixtures/projects/containerized/project/test-container.yml similarity index 100% rename from test/integration/fixtures/projects/containerized/project/test-container.yml rename to test/fixtures/projects/containerized/project/test-container.yml diff --git a/test/integration/fixtures/projects/debug/env/envvars b/test/fixtures/projects/debug/env/envvars similarity index 100% rename from test/integration/fixtures/projects/debug/env/envvars rename to test/fixtures/projects/debug/env/envvars diff --git a/test/integration/fixtures/projects/debug/inventory/inv_1 b/test/fixtures/projects/debug/inventory/inv_1 similarity index 100% rename from test/integration/fixtures/projects/debug/inventory/inv_1 rename to test/fixtures/projects/debug/inventory/inv_1 diff --git a/test/integration/fixtures/projects/debug/inventory/inv_2 b/test/fixtures/projects/debug/inventory/inv_2 similarity index 100% rename from test/integration/fixtures/projects/debug/inventory/inv_2 rename to test/fixtures/projects/debug/inventory/inv_2 diff --git a/test/integration/fixtures/projects/debug/project/debug.yml b/test/fixtures/projects/debug/project/debug.yml similarity index 100% rename from test/integration/fixtures/projects/debug/project/debug.yml rename to test/fixtures/projects/debug/project/debug.yml diff --git a/test/integration/fixtures/projects/debug/project/roles/hello_world/tasks/main.yml b/test/fixtures/projects/debug/project/roles/hello_world/tasks/main.yml similarity index 100% rename from test/integration/fixtures/projects/debug/project/roles/hello_world/tasks/main.yml rename to test/fixtures/projects/debug/project/roles/hello_world/tasks/main.yml diff --git a/test/fixtures/projects/directory_isolation/env/settings b/test/fixtures/projects/directory_isolation/env/settings new file mode 100644 index 000000000..d8c807218 --- /dev/null +++ b/test/fixtures/projects/directory_isolation/env/settings @@ -0,0 +1,3 @@ +process_isolation: True +process_isolation_executable: bwrap +directory_isolation_base_path: /tmp/runner diff --git a/test/fixtures/projects/directory_isolation/project/main.yml b/test/fixtures/projects/directory_isolation/project/main.yml new file mode 100644 index 000000000..8f4d6d4b5 --- /dev/null +++ b/test/fixtures/projects/directory_isolation/project/main.yml @@ -0,0 +1,5 @@ +- hosts: all + gather_facts: no + + tasks: + - debug: msg="directory_isolation test" diff --git a/test/integration/fixtures/projects/files/test_ee.py b/test/fixtures/projects/files/test_ee.py similarity index 100% rename from test/integration/fixtures/projects/files/test_ee.py rename to test/fixtures/projects/files/test_ee.py diff --git a/test/integration/fixtures/projects/host_status/env/envvars b/test/fixtures/projects/host_status/env/envvars similarity index 100% rename from test/integration/fixtures/projects/host_status/env/envvars rename to test/fixtures/projects/host_status/env/envvars diff --git a/test/integration/fixtures/projects/host_status/inventory b/test/fixtures/projects/host_status/inventory similarity index 100% rename from test/integration/fixtures/projects/host_status/inventory rename to test/fixtures/projects/host_status/inventory diff --git a/test/integration/fixtures/projects/host_status/project/gen_host_status.yml b/test/fixtures/projects/host_status/project/gen_host_status.yml similarity index 100% rename from test/integration/fixtures/projects/host_status/project/gen_host_status.yml rename to test/fixtures/projects/host_status/project/gen_host_status.yml diff --git a/test/integration/fixtures/projects/job_env/env/envvars b/test/fixtures/projects/job_env/env/envvars similarity index 100% rename from test/integration/fixtures/projects/job_env/env/envvars rename to test/fixtures/projects/job_env/env/envvars diff --git a/test/integration/fixtures/projects/job_env/env/settings b/test/fixtures/projects/job_env/env/settings similarity index 100% rename from test/integration/fixtures/projects/job_env/env/settings rename to test/fixtures/projects/job_env/env/settings diff --git a/test/integration/fixtures/projects/job_env/inventory/hosts b/test/fixtures/projects/job_env/inventory/hosts similarity index 100% rename from test/integration/fixtures/projects/job_env/inventory/hosts rename to test/fixtures/projects/job_env/inventory/hosts diff --git a/test/integration/fixtures/projects/job_env/project/printenv.yml b/test/fixtures/projects/job_env/project/printenv.yml similarity index 100% rename from test/integration/fixtures/projects/job_env/project/printenv.yml rename to test/fixtures/projects/job_env/project/printenv.yml diff --git a/test/integration/fixtures/projects/printenv/env/envvars b/test/fixtures/projects/printenv/env/envvars similarity index 100% rename from test/integration/fixtures/projects/printenv/env/envvars rename to test/fixtures/projects/printenv/env/envvars diff --git a/test/integration/fixtures/projects/printenv/inventory/hosts b/test/fixtures/projects/printenv/inventory/hosts similarity index 100% rename from test/integration/fixtures/projects/printenv/inventory/hosts rename to test/fixtures/projects/printenv/inventory/hosts diff --git a/test/integration/fixtures/projects/printenv/project/action_plugins/look_at_environment.py b/test/fixtures/projects/printenv/project/action_plugins/look_at_environment.py similarity index 100% rename from test/integration/fixtures/projects/printenv/project/action_plugins/look_at_environment.py rename to test/fixtures/projects/printenv/project/action_plugins/look_at_environment.py diff --git a/test/integration/fixtures/projects/printenv/project/get_environment.yml b/test/fixtures/projects/printenv/project/get_environment.yml similarity index 100% rename from test/integration/fixtures/projects/printenv/project/get_environment.yml rename to test/fixtures/projects/printenv/project/get_environment.yml diff --git a/test/integration/fixtures/projects/sleep/env/envvars b/test/fixtures/projects/sleep/env/envvars similarity index 100% rename from test/integration/fixtures/projects/sleep/env/envvars rename to test/fixtures/projects/sleep/env/envvars diff --git a/test/integration/fixtures/projects/sleep/inventory/hosts b/test/fixtures/projects/sleep/inventory/hosts similarity index 100% rename from test/integration/fixtures/projects/sleep/inventory/hosts rename to test/fixtures/projects/sleep/inventory/hosts diff --git a/test/integration/fixtures/projects/sleep/project/sleep.yml b/test/fixtures/projects/sleep/project/sleep.yml similarity index 100% rename from test/integration/fixtures/projects/sleep/project/sleep.yml rename to test/fixtures/projects/sleep/project/sleep.yml diff --git a/test/integration/fixtures/projects/use_role/env/envvars b/test/fixtures/projects/use_role/env/envvars similarity index 100% rename from test/integration/fixtures/projects/use_role/env/envvars rename to test/fixtures/projects/use_role/env/envvars diff --git a/test/integration/fixtures/projects/use_role/inventory/hosts b/test/fixtures/projects/use_role/inventory/hosts similarity index 100% rename from test/integration/fixtures/projects/use_role/inventory/hosts rename to test/fixtures/projects/use_role/inventory/hosts diff --git a/test/integration/fixtures/projects/use_role/roles/benthomasson.hello_role/meta/.galaxy_install_info b/test/fixtures/projects/use_role/roles/benthomasson.hello_role/meta/.galaxy_install_info similarity index 100% rename from test/integration/fixtures/projects/use_role/roles/benthomasson.hello_role/meta/.galaxy_install_info rename to test/fixtures/projects/use_role/roles/benthomasson.hello_role/meta/.galaxy_install_info diff --git a/test/integration/fixtures/projects/use_role/roles/benthomasson.hello_role/meta/main.yml b/test/fixtures/projects/use_role/roles/benthomasson.hello_role/meta/main.yml similarity index 100% rename from test/integration/fixtures/projects/use_role/roles/benthomasson.hello_role/meta/main.yml rename to test/fixtures/projects/use_role/roles/benthomasson.hello_role/meta/main.yml diff --git a/test/integration/fixtures/projects/use_role/roles/benthomasson.hello_role/tasks/main.yml b/test/fixtures/projects/use_role/roles/benthomasson.hello_role/tasks/main.yml similarity index 100% rename from test/integration/fixtures/projects/use_role/roles/benthomasson.hello_role/tasks/main.yml rename to test/fixtures/projects/use_role/roles/benthomasson.hello_role/tasks/main.yml diff --git a/test/integration/fixtures/projects/use_role/use_role.yml b/test/fixtures/projects/use_role/use_role.yml similarity index 100% rename from test/integration/fixtures/projects/use_role/use_role.yml rename to test/fixtures/projects/use_role/use_role.yml diff --git a/test/integration/conftest.py b/test/integration/conftest.py index 6a3660d5b..780542f09 100644 --- a/test/integration/conftest.py +++ b/test/integration/conftest.py @@ -1,11 +1,7 @@ import json import os -import shutil import subprocess import yaml - -from pathlib import Path - import pytest import pexpect @@ -80,14 +76,3 @@ def run(args, *a, **kw): return ret return run - - -@pytest.fixture -def project_fixtures(tmp_path): - source = Path(__file__).parent / 'fixtures' / 'projects' - dest = tmp_path / 'projects' - shutil.copytree(source, dest) - - yield dest - - shutil.rmtree(dest, ignore_errors=True) diff --git a/test/unit/config/test_runner.py b/test/unit/config/test_runner.py index 2940712fa..4bc0b5b3e 100644 --- a/test/unit/config/test_runner.py +++ b/test/unit/config/test_runner.py @@ -4,8 +4,8 @@ from io import StringIO import os import re - import six + from pexpect import TIMEOUT, EOF import pytest @@ -217,6 +217,37 @@ def test_prepare_env_directory_isolation(mocker): assert rc.cwd == '/tmp/foo' +def test_prepare_env_directory_isolation_from_settings(mocker, project_fixtures): + ''' + Test that sandboxing with directory isolation works correctly with `env/settings` values. + ''' + # Mock away the things that would actually prepare the isolation directory. + mocker.patch('os.makedirs', return_value=True) + copy_tree = mocker.patch('distutils.dir_util.copy_tree') + mkdtemp = mocker.patch('tempfile.mkdtemp') + mkdtemp.return_value = '/tmp/runner/runner_di_XYZ' + mocker.patch('ansible_runner.config.runner.RunnerConfig.build_process_isolation_temp_dir') + + # The `directory_isolation` test data sets up an `env/settings` file for us. + private_data_dir = project_fixtures / 'directory_isolation' + rc = RunnerConfig(private_data_dir=str(private_data_dir), playbook='main.yaml') + + # This is where all the magic happens + rc.prepare() + + assert rc.sandboxed + assert rc.process_isolation_executable == 'bwrap' + assert rc.project_dir == str(private_data_dir / 'project') + assert os.path.exists(rc.project_dir) + + # `directory_isolation_path` should be used to create a new temp path underneath + assert rc.directory_isolation_path == '/tmp/runner/runner_di_XYZ' + mkdtemp.assert_called_once_with(prefix='runner_di_', dir='/tmp/runner') + + # The project files should be copied to the isolation path. + copy_tree.assert_called_once_with(rc.project_dir, rc.directory_isolation_path, preserve_symlinks=True) + + def test_prepare_inventory(mocker): mocker.patch('os.makedirs', return_value=True) path_exists = mocker.patch('os.path.exists', return_value=True)