diff --git a/awx/main/models/credential/__init__.py b/awx/main/models/credential/__init__.py index 3d9ef00a09..f33a0d4671 100644 --- a/awx/main/models/credential/__init__.py +++ b/awx/main/models/credential/__init__.py @@ -31,6 +31,7 @@ from awx.main.fields import ( ) from awx.main.utils import decrypt_field, classproperty from awx.main.utils.safe_yaml import safe_dump +from awx.main.utils.execution_environments import to_container_path from awx.main.validators import validate_ssh_private_key from awx.main.models.base import CommonModelNameNotUnique, PasswordFieldsModel, PrimordialModel from awx.main.models.mixins import ResourceMixin @@ -493,12 +494,11 @@ class CredentialType(CommonModelNameNotUnique): for file_label, file_tmpl in file_tmpls.items(): data = sandbox_env.from_string(file_tmpl).render(**namespace) - _, path = tempfile.mkstemp(dir=private_data_dir) + _, path = tempfile.mkstemp(dir=os.path.join(private_data_dir, 'env')) with open(path, 'w') as f: f.write(data) os.chmod(path, stat.S_IRUSR | stat.S_IWUSR) - # FIXME: develop some better means of referencing paths inside containers - container_path = os.path.join('/runner', os.path.basename(path)) + container_path = to_container_path(path, private_data_dir) # determine if filename indicates single file or many if file_label.find('.') == -1: @@ -526,7 +526,7 @@ class CredentialType(CommonModelNameNotUnique): extra_vars[var_name] = sandbox_env.from_string(tmpl).render(**namespace) def build_extra_vars_file(vars, private_dir): - handle, path = tempfile.mkstemp(dir=private_dir) + handle, path = tempfile.mkstemp(dir=os.path.join(private_dir, 'env')) f = os.fdopen(handle, 'w') f.write(safe_dump(vars)) f.close() @@ -535,8 +535,7 @@ class CredentialType(CommonModelNameNotUnique): if extra_vars: path = build_extra_vars_file(extra_vars, private_data_dir) - # FIXME: develop some better means of referencing paths inside containers - container_path = os.path.join('/runner', os.path.basename(path)) + container_path = to_container_path(path, private_data_dir) args.extend(['-e', '@%s' % container_path]) diff --git a/awx/main/models/credential/injectors.py b/awx/main/models/credential/injectors.py index 246ab0d4e4..faafaad59b 100644 --- a/awx/main/models/credential/injectors.py +++ b/awx/main/models/credential/injectors.py @@ -6,6 +6,8 @@ import tempfile from django.conf import settings +from awx.main.utils.execution_environments import to_container_path + def aws(cred, env, private_data_dir): env['AWS_ACCESS_KEY_ID'] = cred.get_input('username', default='') @@ -25,13 +27,14 @@ def gce(cred, env, private_data_dir): env['GCE_PROJECT'] = project json_cred['token_uri'] = 'https://oauth2.googleapis.com/token' - handle, path = tempfile.mkstemp(dir=private_data_dir) + handle, path = tempfile.mkstemp(dir=os.path.join(private_data_dir, 'env')) f = os.fdopen(handle, 'w') json.dump(json_cred, f, indent=2) f.close() os.chmod(path, stat.S_IRUSR | stat.S_IWUSR) - env['GCE_CREDENTIALS_FILE_PATH'] = os.path.join('/runner', os.path.basename(path)) - env['GCP_SERVICE_ACCOUNT_FILE'] = os.path.join('/runner', os.path.basename(path)) + container_path = to_container_path(path, private_data_dir) + env['GCE_CREDENTIALS_FILE_PATH'] = container_path + env['GCP_SERVICE_ACCOUNT_FILE'] = container_path # Handle env variables for new module types. # This includes gcp_compute inventory plugin and @@ -96,14 +99,13 @@ def _openstack_data(cred): def openstack(cred, env, private_data_dir): - handle, path = tempfile.mkstemp(dir=private_data_dir) + handle, path = tempfile.mkstemp(dir=os.path.join(private_data_dir, 'env')) f = os.fdopen(handle, 'w') openstack_data = _openstack_data(cred) yaml.safe_dump(openstack_data, f, default_flow_style=False, allow_unicode=True) f.close() os.chmod(path, stat.S_IRUSR | stat.S_IWUSR) - # TODO: constant for container base path - env['OS_CLIENT_CONFIG_FILE'] = os.path.join('/runner', os.path.basename(path)) + env['OS_CLIENT_CONFIG_FILE'] = to_container_path(path, private_data_dir) def kubernetes_bearer_token(cred, env, private_data_dir): @@ -111,10 +113,10 @@ def kubernetes_bearer_token(cred, env, private_data_dir): env['K8S_AUTH_API_KEY'] = cred.get_input('bearer_token', default='') if cred.get_input('verify_ssl') and 'ssl_ca_cert' in cred.inputs: env['K8S_AUTH_VERIFY_SSL'] = 'True' - handle, path = tempfile.mkstemp(dir=private_data_dir) + handle, path = tempfile.mkstemp(dir=os.path.join(private_data_dir, 'env')) with os.fdopen(handle, 'w') as f: os.chmod(path, stat.S_IRUSR | stat.S_IWUSR) f.write(cred.get_input('ssl_ca_cert')) - env['K8S_AUTH_SSL_CA_CERT'] = os.path.join('/runner', os.path.basename(path)) + env['K8S_AUTH_SSL_CA_CERT'] = to_container_path(path, private_data_dir) else: env['K8S_AUTH_VERIFY_SSL'] = 'False' diff --git a/awx/main/models/inventory.py b/awx/main/models/inventory.py index 99dc7b837b..91719a4bad 100644 --- a/awx/main/models/inventory.py +++ b/awx/main/models/inventory.py @@ -50,6 +50,7 @@ from awx.main.models.notifications import ( from awx.main.models.credential.injectors import _openstack_data from awx.main.utils import _inventory_updates from awx.main.utils.safe_yaml import sanitize_jinja +from awx.main.utils.execution_environments import to_container_path __all__ = ['Inventory', 'Host', 'Group', 'InventorySource', 'InventoryUpdate', 'SmartInventoryMembership'] @@ -1505,7 +1506,7 @@ class openstack(PluginFileInjector): env = super(openstack, self).get_plugin_env(inventory_update, private_data_dir, private_data_files) credential = inventory_update.get_cloud_credential() cred_data = private_data_files['credentials'] - env['OS_CLIENT_CONFIG_FILE'] = os.path.join('/runner', os.path.basename(cred_data[credential])) + env['OS_CLIENT_CONFIG_FILE'] = to_container_path(cred_data[credential], private_data_dir) return env diff --git a/awx/main/tasks.py b/awx/main/tasks.py index 35f4e8df38..7642dd6bb2 100644 --- a/awx/main/tasks.py +++ b/awx/main/tasks.py @@ -98,7 +98,7 @@ from awx.main.utils import ( parse_yaml_or_json, cleanup_new_process, ) -from awx.main.utils.execution_environments import get_default_execution_environment, get_default_pod_spec +from awx.main.utils.execution_environments import get_default_execution_environment, get_default_pod_spec, CONTAINER_ROOT, to_container_path from awx.main.utils.ansible import read_ansible_config from awx.main.utils.external_logging import reconfigure_rsyslog from awx.main.utils.safe_yaml import safe_dump, sanitize_jinja @@ -875,11 +875,12 @@ class BaseTask(object): path = tempfile.mkdtemp(prefix='awx_%s_' % instance.pk, dir=pdd_wrapper_path) os.chmod(path, stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR) - runner_project_folder = os.path.join(path, 'project') - if not os.path.exists(runner_project_folder): - # Ansible Runner requires that this directory exists. - # Specifically, when using process isolation - os.mkdir(runner_project_folder) + # Ansible runner requires that project exists, + # and we will write files in the other folders without pre-creating the folder + for subfolder in ('project', 'inventory', 'env'): + runner_subfolder = os.path.join(path, subfolder) + if not os.path.exists(runner_subfolder): + os.mkdir(runner_subfolder) return path def build_private_data_files(self, instance, private_data_dir): @@ -923,7 +924,7 @@ class BaseTask(object): # Instead, ssh private key file is explicitly passed via an # env variable. else: - handle, path = tempfile.mkstemp(dir=private_data_dir) + handle, path = tempfile.mkstemp(dir=os.path.join(private_data_dir, 'env')) f = os.fdopen(handle, 'w') f.write(data) f.close() @@ -1036,7 +1037,6 @@ class BaseTask(object): self.host_map = {hostname: hv.pop('remote_tower_id', '') for hostname, hv in script_data.get('_meta', {}).get('hostvars', {}).items()} json_data = json.dumps(script_data) path = os.path.join(private_data_dir, 'inventory') - os.makedirs(path, mode=0o700) fn = os.path.join(path, 'hosts') with open(fn, 'w') as f: os.chmod(fn, stat.S_IRUSR | stat.S_IXUSR | stat.S_IWUSR) @@ -1533,8 +1533,8 @@ class RunJob(BaseTask): # Set environment variables for cloud credentials. cred_files = private_data_files.get('credentials', {}) for cloud_cred in job.cloud_credentials: - if cloud_cred and cloud_cred.credential_type.namespace == 'openstack': - env['OS_CLIENT_CONFIG_FILE'] = os.path.join('/runner', os.path.basename(cred_files.get(cloud_cred, ''))) + if cloud_cred and cloud_cred.credential_type.namespace == 'openstack' and cred_files.get(cloud_cred, ''): + env['OS_CLIENT_CONFIG_FILE'] = to_container_path(cred_files.get(cloud_cred, ''), private_data_dir) for network_cred in job.network_credentials: env['ANSIBLE_NET_USERNAME'] = network_cred.get_input('username', default='') @@ -1566,8 +1566,7 @@ class RunJob(BaseTask): for path in config_values[config_setting].split(':'): if path not in paths: paths = [config_values[config_setting]] + paths - # FIXME: again, figure out more elegant way for inside container - paths = [os.path.join('/runner', folder)] + paths + paths = [os.path.join(CONTAINER_ROOT, folder)] + paths env[env_key] = os.pathsep.join(paths) return env @@ -2394,8 +2393,7 @@ class RunInventoryUpdate(BaseTask): for path in config_values[config_setting].split(':'): if path not in paths: paths = [config_values[config_setting]] + paths - # FIXME: containers - paths = [os.path.join('/runner', folder)] + paths + paths = [os.path.join(CONTAINER_ROOT, folder)] + paths env[env_key] = os.pathsep.join(paths) return env @@ -2424,14 +2422,14 @@ class RunInventoryUpdate(BaseTask): # Add arguments for the source inventory file/script/thing rel_path = self.pseudo_build_inventory(inventory_update, private_data_dir) - container_location = os.path.join('/runner', rel_path) # TODO: make container paths elegant + container_location = os.path.join(CONTAINER_ROOT, rel_path) source_location = os.path.join(private_data_dir, rel_path) args.append('-i') args.append(container_location) args.append('--output') - args.append(os.path.join('/runner', 'artifacts', str(inventory_update.id), 'output.json')) + args.append(os.path.join(CONTAINER_ROOT, 'artifacts', str(inventory_update.id), 'output.json')) if os.path.isdir(source_location): playbook_dir = container_location @@ -2463,12 +2461,12 @@ class RunInventoryUpdate(BaseTask): if injector is not None: content = injector.inventory_contents(inventory_update, private_data_dir) # must be a statically named file - inventory_path = os.path.join(private_data_dir, injector.filename) + inventory_path = os.path.join(private_data_dir, 'inventory', injector.filename) with open(inventory_path, 'w') as f: f.write(content) os.chmod(inventory_path, stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR) - rel_path = injector.filename + rel_path = os.path.join('inventory', injector.filename) elif src == 'scm': rel_path = os.path.join('project', inventory_update.source_path) @@ -2481,10 +2479,9 @@ class RunInventoryUpdate(BaseTask): - SCM, where source needs to live in the project folder """ src = inventory_update.source - container_dir = '/runner' # TODO: make container paths elegant if src == 'scm' and inventory_update.source_project_update: - return os.path.join(container_dir, 'project') - return container_dir + return os.path.join(CONTAINER_ROOT, 'project') + return CONTAINER_ROOT def build_playbook_path_relative_to_cwd(self, inventory_update, private_data_dir): return None diff --git a/awx/main/tests/functional/test_inventory_source_injectors.py b/awx/main/tests/functional/test_inventory_source_injectors.py index 5fd12a332d..aff0356b59 100644 --- a/awx/main/tests/functional/test_inventory_source_injectors.py +++ b/awx/main/tests/functional/test_inventory_source_injectors.py @@ -9,6 +9,7 @@ from awx.main.tasks import RunInventoryUpdate from awx.main.models import InventorySource, Credential, CredentialType, UnifiedJob, ExecutionEnvironment from awx.main.constants import CLOUD_PROVIDERS, STANDARD_INVENTORY_UPDATE_ENV from awx.main.tests import data +from awx.main.utils.execution_environments import to_container_path from django.conf import settings @@ -99,13 +100,19 @@ def read_content(private_data_dir, raw_env, inventory_update): dir_contents = {} referenced_paths = set() file_aliases = {} - filename_list = sorted(os.listdir(private_data_dir), key=lambda fn: inverse_env.get(os.path.join(private_data_dir, fn), [fn])[0]) + filename_list = os.listdir(private_data_dir) + for subdir in ('env', 'inventory'): + if subdir in filename_list: + filename_list.remove(subdir) + for filename in os.listdir(os.path.join(private_data_dir, subdir)): + filename_list.append(os.path.join(subdir, filename)) + filename_list = sorted(filename_list, key=lambda fn: inverse_env.get(os.path.join(private_data_dir, fn), [fn])[0]) for filename in filename_list: if filename in ('args', 'project'): continue # Ansible runner abs_file_path = os.path.join(private_data_dir, filename) file_aliases[abs_file_path] = filename - runner_path = os.path.join('/runner', os.path.basename(abs_file_path)) + runner_path = to_container_path(abs_file_path, private_data_dir) if runner_path in inverse_env: referenced_paths.add(abs_file_path) alias = 'file_reference' diff --git a/awx/main/tests/unit/test_tasks.py b/awx/main/tests/unit/test_tasks.py index 82a8e1809f..2ec44b3c1e 100644 --- a/awx/main/tests/unit/test_tasks.py +++ b/awx/main/tests/unit/test_tasks.py @@ -37,6 +37,7 @@ from awx.main.models.credential import ManagedCredentialType from awx.main import tasks from awx.main.utils import encrypt_field, encrypt_value from awx.main.utils.safe_yaml import SafeLoader +from awx.main.utils.execution_environments import CONTAINER_ROOT, to_host_path from awx.main.utils.licensing import Licenser @@ -48,6 +49,10 @@ class TestJobExecution(object): @pytest.fixture def private_data_dir(): private_data = tempfile.mkdtemp(prefix='awx_') + for subfolder in ('inventory', 'env'): + runner_subfolder = os.path.join(private_data, subfolder) + if not os.path.exists(runner_subfolder): + os.mkdir(runner_subfolder) yield private_data shutil.rmtree(private_data, True) @@ -337,8 +342,8 @@ def pytest_generate_tests(metafunc): def parse_extra_vars(args, private_data_dir): extra_vars = {} for chunk in args: - if chunk.startswith('@/runner/'): - local_path = os.path.join(private_data_dir, os.path.basename(chunk.strip('@'))) + if chunk.startswith(f'@{CONTAINER_ROOT}'): + local_path = chunk[len('@') :].replace(CONTAINER_ROOT, private_data_dir) # container path to host path with open(local_path, 'r') as f: extra_vars.update(yaml.load(f, Loader=SafeLoader)) return extra_vars @@ -888,7 +893,7 @@ class TestJobCredentials(TestJobExecution): if verify: assert env['K8S_AUTH_VERIFY_SSL'] == 'True' - local_path = os.path.join(private_data_dir, os.path.basename(env['K8S_AUTH_SSL_CA_CERT'])) + local_path = to_host_path(env['K8S_AUTH_SSL_CA_CERT'], private_data_dir) cert = open(local_path, 'r').read() assert cert == 'CERTDATA' else: @@ -938,7 +943,7 @@ class TestJobCredentials(TestJobExecution): safe_env = {} credential.credential_type.inject_credential(credential, env, safe_env, [], private_data_dir) runner_path = env['GCE_CREDENTIALS_FILE_PATH'] - local_path = os.path.join(private_data_dir, os.path.basename(runner_path)) + local_path = to_host_path(runner_path, private_data_dir) json_data = json.load(open(local_path, 'rb')) assert json_data['type'] == 'service_account' assert json_data['private_key'] == self.EXAMPLE_PRIVATE_KEY @@ -1010,8 +1015,7 @@ class TestJobCredentials(TestJobExecution): env = task.build_env(job, private_data_dir, private_data_files=private_data_files) credential.credential_type.inject_credential(credential, env, {}, [], private_data_dir) - # convert container path to host machine path - config_loc = os.path.join(private_data_dir, os.path.basename(env['OS_CLIENT_CONFIG_FILE'])) + config_loc = to_host_path(env['OS_CLIENT_CONFIG_FILE'], private_data_dir) shade_config = open(config_loc, 'r').read() assert shade_config == '\n'.join( [ @@ -1046,7 +1050,8 @@ class TestJobCredentials(TestJobExecution): credential.credential_type.inject_credential(credential, env, safe_env, [], private_data_dir) config = configparser.ConfigParser() - config.read(os.path.join(private_data_dir, os.path.basename(env['OVIRT_INI_PATH']))) + host_path = to_host_path(env['OVIRT_INI_PATH'], private_data_dir) + config.read(host_path) assert config.get('ovirt', 'ovirt_url') == 'some-ovirt-host.example.org' assert config.get('ovirt', 'ovirt_username') == 'bob' assert config.get('ovirt', 'ovirt_password') == 'some-pass' @@ -1259,7 +1264,7 @@ class TestJobCredentials(TestJobExecution): env = {} credential.credential_type.inject_credential(credential, env, {}, [], private_data_dir) - path = os.path.join(private_data_dir, os.path.basename(env['MY_CLOUD_INI_FILE'])) + path = to_host_path(env['MY_CLOUD_INI_FILE'], private_data_dir) assert open(path, 'r').read() == '[mycloud]\nABC123' def test_custom_environment_injectors_with_unicode_content(self, private_data_dir): @@ -1279,7 +1284,7 @@ class TestJobCredentials(TestJobExecution): env = {} credential.credential_type.inject_credential(credential, env, {}, [], private_data_dir) - path = os.path.join(private_data_dir, os.path.basename(env['MY_CLOUD_INI_FILE'])) + path = to_host_path(env['MY_CLOUD_INI_FILE'], private_data_dir) assert open(path, 'r').read() == value def test_custom_environment_injectors_with_files(self, private_data_dir): @@ -1298,8 +1303,8 @@ class TestJobCredentials(TestJobExecution): env = {} credential.credential_type.inject_credential(credential, env, {}, [], private_data_dir) - cert_path = os.path.join(private_data_dir, os.path.basename(env['MY_CERT_INI_FILE'])) - key_path = os.path.join(private_data_dir, os.path.basename(env['MY_KEY_INI_FILE'])) + cert_path = to_host_path(env['MY_CERT_INI_FILE'], private_data_dir) + key_path = to_host_path(env['MY_KEY_INI_FILE'], private_data_dir) assert open(cert_path, 'r').read() == '[mycert]\nCERT123' assert open(key_path, 'r').read() == '[mykey]\nKEY123' @@ -1322,7 +1327,7 @@ class TestJobCredentials(TestJobExecution): assert env['AZURE_AD_USER'] == 'bob' assert env['AZURE_PASSWORD'] == 'secret' - path = os.path.join(private_data_dir, os.path.basename(env['GCE_CREDENTIALS_FILE_PATH'])) + path = to_host_path(env['GCE_CREDENTIALS_FILE_PATH'], private_data_dir) json_data = json.load(open(path, 'rb')) assert json_data['type'] == 'service_account' assert json_data['private_key'] == self.EXAMPLE_PRIVATE_KEY @@ -1703,7 +1708,7 @@ class TestInventoryUpdateCredentials(TestJobExecution): private_data_files = task.build_private_data_files(inventory_update, private_data_dir) env = task.build_env(inventory_update, private_data_dir, private_data_files) - path = os.path.join(private_data_dir, os.path.basename(env['OS_CLIENT_CONFIG_FILE'])) + path = to_host_path(env['OS_CLIENT_CONFIG_FILE'], private_data_dir) shade_config = open(path, 'r').read() assert ( '\n'.join( diff --git a/awx/main/tests/unit/utils/test_execution_environments.py b/awx/main/tests/unit/utils/test_execution_environments.py new file mode 100644 index 0000000000..4f9e70a1a1 --- /dev/null +++ b/awx/main/tests/unit/utils/test_execution_environments.py @@ -0,0 +1,45 @@ +import pytest + +from awx.main.utils.execution_environments import to_container_path, to_host_path + + +private_data_dir = '/tmp/pdd_iso/awx_xxx' + + +@pytest.mark.parametrize( + 'container_path,host_path', + [ + ('/runner', private_data_dir), + ('/runner/foo', '{0}/foo'.format(private_data_dir)), + ('/runner/foo/bar', '{0}/foo/bar'.format(private_data_dir)), + ('/runner{0}'.format(private_data_dir), '{0}{0}'.format(private_data_dir)), + ], +) +def test_switch_paths(container_path, host_path): + assert to_container_path(host_path, private_data_dir) == container_path + assert to_host_path(container_path, private_data_dir) == host_path + + +@pytest.mark.parametrize( + 'container_path', + [ + ('/foobar'), + ('/runner/..'), + ], +) +def test_invalid_container_path(container_path): + with pytest.raises(RuntimeError): + to_host_path(container_path, private_data_dir) + + +@pytest.mark.parametrize( + 'host_path', + [ + ('/foobar'), + ('/tmp/pdd_iso'), + ('/tmp/pdd_iso/awx_xxx/..'), + ], +) +def test_invalid_host_path(host_path): + with pytest.raises(RuntimeError): + to_container_path(host_path, private_data_dir) diff --git a/awx/main/utils/execution_environments.py b/awx/main/utils/execution_environments.py index ed6bb87f34..67fee9566d 100644 --- a/awx/main/utils/execution_environments.py +++ b/awx/main/utils/execution_environments.py @@ -1,3 +1,6 @@ +import os +from pathlib import Path + from django.conf import settings from awx.main.models.execution_environments import ExecutionEnvironment @@ -25,3 +28,32 @@ def get_default_pod_spec(): ], }, } + + +# this is the root of the private data dir as seen from inside +# of the container running a job +CONTAINER_ROOT = '/runner' + + +def to_container_path(path, private_data_dir): + """Given a path inside of the host machine filesystem, + this returns the expected path which would be observed by the job running + inside of the EE container. + This only handles the volume mount from private_data_dir to /runner + """ + if not os.path.isabs(private_data_dir): + raise RuntimeError('The private_data_dir path must be absolute') + if private_data_dir != path and Path(private_data_dir) not in Path(path).resolve().parents: + raise RuntimeError(f'Cannot convert path {path} unless it is a subdir of {private_data_dir}') + return path.replace(private_data_dir, CONTAINER_ROOT, 1) + + +def to_host_path(path, private_data_dir): + """Given a path inside of the EE container, this gives the absolute path + on the host machine within the private_data_dir + """ + if not os.path.isabs(private_data_dir): + raise RuntimeError('The private_data_dir path must be absolute') + if CONTAINER_ROOT != path and Path(CONTAINER_ROOT) not in Path(path).resolve().parents: + raise RuntimeError(f'Cannot convert path {path} unless it is a subdir of {CONTAINER_ROOT}') + return path.replace(CONTAINER_ROOT, private_data_dir, 1)