From f59da78328affe3304abf9016bde6ac080216887 Mon Sep 17 00:00:00 2001 From: Alan Rominger Date: Wed, 16 Sep 2020 20:35:27 -0400 Subject: [PATCH 01/14] Use inventory and env private_data_dir subfolders This avoids writing files to the top level of the ansible-runner private_data_dir Inventory is moved to be in the standard "inventory" folder Credential related files are moved inside of the "env" folder Also pre-create these folders when preparing for a job run With this, args is the only top-level file still remaining --- awx/main/models/credential/__init__.py | 4 ++-- awx/main/models/credential/injectors.py | 6 +++--- awx/main/tasks.py | 15 ++++++++------- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/awx/main/models/credential/__init__.py b/awx/main/models/credential/__init__.py index 3d9ef00a09..9e5e06472b 100644 --- a/awx/main/models/credential/__init__.py +++ b/awx/main/models/credential/__init__.py @@ -493,7 +493,7 @@ 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) @@ -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() diff --git a/awx/main/models/credential/injectors.py b/awx/main/models/credential/injectors.py index 246ab0d4e4..925df9daa4 100644 --- a/awx/main/models/credential/injectors.py +++ b/awx/main/models/credential/injectors.py @@ -25,7 +25,7 @@ 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() @@ -96,7 +96,7 @@ 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) @@ -111,7 +111,7 @@ 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')) diff --git a/awx/main/tasks.py b/awx/main/tasks.py index 994209aaf2..122cf88ec8 100644 --- a/awx/main/tasks.py +++ b/awx/main/tasks.py @@ -873,11 +873,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): @@ -921,7 +922,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() @@ -2460,7 +2461,7 @@ 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) From 0061c57577d64a047e60a3ed41e5abe8656d460d Mon Sep 17 00:00:00 2001 From: Alan Rominger Date: Fri, 18 Sep 2020 16:35:37 -0400 Subject: [PATCH 02/14] update inventory injector tests --- .../tests/functional/test_inventory_source_injectors.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/awx/main/tests/functional/test_inventory_source_injectors.py b/awx/main/tests/functional/test_inventory_source_injectors.py index 5fd12a332d..6e75bf2078 100644 --- a/awx/main/tests/functional/test_inventory_source_injectors.py +++ b/awx/main/tests/functional/test_inventory_source_injectors.py @@ -99,7 +99,13 @@ 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 From 3c785fbff36ce9f6598c3a1517bdba58913f1741 Mon Sep 17 00:00:00 2001 From: Alan Rominger Date: Fri, 18 Sep 2020 22:37:39 -0400 Subject: [PATCH 03/14] update unit tests to new behavior --- awx/main/tests/unit/test_tasks.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/awx/main/tests/unit/test_tasks.py b/awx/main/tests/unit/test_tasks.py index 82a8e1809f..bdfeda6c24 100644 --- a/awx/main/tests/unit/test_tasks.py +++ b/awx/main/tests/unit/test_tasks.py @@ -48,6 +48,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) From 0e17023ba36a70a6f35ca19ce012f7976739d990 Mon Sep 17 00:00:00 2001 From: Alan Rominger Date: Tue, 20 Apr 2021 10:18:23 -0400 Subject: [PATCH 04/14] Inventory directory already pre-created --- awx/main/tasks.py | 1 - 1 file changed, 1 deletion(-) diff --git a/awx/main/tasks.py b/awx/main/tasks.py index 122cf88ec8..fb4f7ef7e5 100644 --- a/awx/main/tasks.py +++ b/awx/main/tasks.py @@ -1035,7 +1035,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) From 11c5d577d68b6ccdaf21c1524fffac056c815b52 Mon Sep 17 00:00:00 2001 From: Alan Rominger Date: Wed, 21 Apr 2021 08:31:59 -0400 Subject: [PATCH 05/14] Fix rel path for other inventories --- awx/main/tasks.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/awx/main/tasks.py b/awx/main/tasks.py index fb4f7ef7e5..c27167e0dd 100644 --- a/awx/main/tasks.py +++ b/awx/main/tasks.py @@ -2465,7 +2465,8 @@ class RunInventoryUpdate(BaseTask): 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) + # rel_path = injector.filename elif src == 'scm': rel_path = os.path.join('project', inventory_update.source_path) From 8f9373085a7b07bbe8d9a1e1f9dcdaddb3491676 Mon Sep 17 00:00:00 2001 From: Alan Rominger Date: Wed, 21 Apr 2021 09:46:56 -0400 Subject: [PATCH 06/14] Fix credential env folder, test_tasks.py --- awx/main/models/credential/__init__.py | 4 ++-- awx/main/models/credential/injectors.py | 9 +++++---- awx/main/models/inventory.py | 2 +- awx/main/tasks.py | 2 +- awx/main/tests/unit/test_tasks.py | 26 ++++++++++++++----------- 5 files changed, 24 insertions(+), 19 deletions(-) diff --git a/awx/main/models/credential/__init__.py b/awx/main/models/credential/__init__.py index 9e5e06472b..2515f2f49b 100644 --- a/awx/main/models/credential/__init__.py +++ b/awx/main/models/credential/__init__.py @@ -498,7 +498,7 @@ class CredentialType(CommonModelNameNotUnique): 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 = os.path.join('/runner', 'env', os.path.basename(path)) # determine if filename indicates single file or many if file_label.find('.') == -1: @@ -536,7 +536,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 = os.path.join('/runner', 'env', os.path.basename(path)) args.extend(['-e', '@%s' % container_path]) diff --git a/awx/main/models/credential/injectors.py b/awx/main/models/credential/injectors.py index 925df9daa4..259cfc724e 100644 --- a/awx/main/models/credential/injectors.py +++ b/awx/main/models/credential/injectors.py @@ -30,8 +30,9 @@ def gce(cred, env, private_data_dir): 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)) + cred_path = os.path.join('/runner', 'env', os.path.basename(path)) + env['GCE_CREDENTIALS_FILE_PATH'] = cred_path + env['GCP_SERVICE_ACCOUNT_FILE'] = cred_path # Handle env variables for new module types. # This includes gcp_compute inventory plugin and @@ -103,7 +104,7 @@ def openstack(cred, env, private_data_dir): 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'] = os.path.join('/runner', 'env', os.path.basename(path)) def kubernetes_bearer_token(cred, env, private_data_dir): @@ -115,6 +116,6 @@ def kubernetes_bearer_token(cred, env, private_data_dir): 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'] = os.path.join('/runner', 'env', os.path.basename(path)) else: env['K8S_AUTH_VERIFY_SSL'] = 'False' diff --git a/awx/main/models/inventory.py b/awx/main/models/inventory.py index 99dc7b837b..b59b640b51 100644 --- a/awx/main/models/inventory.py +++ b/awx/main/models/inventory.py @@ -1505,7 +1505,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'] = os.path.join('/runner', 'env', os.path.basename(cred_data[credential])) return env diff --git a/awx/main/tasks.py b/awx/main/tasks.py index c27167e0dd..5c2a396c58 100644 --- a/awx/main/tasks.py +++ b/awx/main/tasks.py @@ -1532,7 +1532,7 @@ class RunJob(BaseTask): 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, ''))) + env['OS_CLIENT_CONFIG_FILE'] = os.path.join('/runner', 'env', os.path.basename(cred_files.get(cloud_cred, ''))) for network_cred in job.network_credentials: env['ANSIBLE_NET_USERNAME'] = network_cred.get_input('username', default='') diff --git a/awx/main/tests/unit/test_tasks.py b/awx/main/tests/unit/test_tasks.py index bdfeda6c24..893a3deff6 100644 --- a/awx/main/tests/unit/test_tasks.py +++ b/awx/main/tests/unit/test_tasks.py @@ -342,7 +342,7 @@ 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('@'))) + local_path = chunk[len('@') :].replace('/runner', 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 @@ -892,7 +892,10 @@ 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 = os.path.join(private_data_dir, os.path.basename(env['K8S_AUTH_SSL_CA_CERT'])) + local_path = env['K8S_AUTH_SSL_CA_CERT'].replace('/runner', private_data_dir) # container path to host path + print('env') + print(env['K8S_AUTH_SSL_CA_CERT']) cert = open(local_path, 'r').read() assert cert == 'CERTDATA' else: @@ -942,7 +945,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 = runner_path.replace('/runner', private_data_dir) # container path to host path json_data = json.load(open(local_path, 'rb')) assert json_data['type'] == 'service_account' assert json_data['private_key'] == self.EXAMPLE_PRIVATE_KEY @@ -1015,7 +1018,7 @@ class TestJobCredentials(TestJobExecution): 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 = env['OS_CLIENT_CONFIG_FILE'].replace('/runner', private_data_dir) # container path to host path shade_config = open(config_loc, 'r').read() assert shade_config == '\n'.join( [ @@ -1050,7 +1053,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 = env['OVIRT_INI_PATH'].replace('/runner', private_data_dir) # container path to host path + 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' @@ -1263,7 +1267,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 = env['MY_CLOUD_INI_FILE'].replace('/runner', private_data_dir) # container path to host path assert open(path, 'r').read() == '[mycloud]\nABC123' def test_custom_environment_injectors_with_unicode_content(self, private_data_dir): @@ -1283,7 +1287,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 = env['MY_CLOUD_INI_FILE'].replace('/runner', private_data_dir) # container path to host path assert open(path, 'r').read() == value def test_custom_environment_injectors_with_files(self, private_data_dir): @@ -1302,8 +1306,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 = env['MY_CERT_INI_FILE'].replace('/runner', private_data_dir) # container path to host path + key_path = env['MY_KEY_INI_FILE'].replace('/runner', private_data_dir) # container path to host path assert open(cert_path, 'r').read() == '[mycert]\nCERT123' assert open(key_path, 'r').read() == '[mykey]\nKEY123' @@ -1326,7 +1330,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 = env['GCE_CREDENTIALS_FILE_PATH'].replace('/runner', private_data_dir) # container path to host path json_data = json.load(open(path, 'rb')) assert json_data['type'] == 'service_account' assert json_data['private_key'] == self.EXAMPLE_PRIVATE_KEY @@ -1707,7 +1711,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 = env['OS_CLIENT_CONFIG_FILE'].replace('/runner', private_data_dir) # container path to host path shade_config = open(path, 'r').read() assert ( '\n'.join( From 334be9eb25eac7e25748c5583633b016a67ee2b2 Mon Sep 17 00:00:00 2001 From: Alan Rominger Date: Wed, 21 Apr 2021 10:14:40 -0400 Subject: [PATCH 07/14] Use durable switch from container to host path --- awx/main/tests/functional/test_inventory_source_injectors.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/awx/main/tests/functional/test_inventory_source_injectors.py b/awx/main/tests/functional/test_inventory_source_injectors.py index 6e75bf2078..5ab319aa75 100644 --- a/awx/main/tests/functional/test_inventory_source_injectors.py +++ b/awx/main/tests/functional/test_inventory_source_injectors.py @@ -111,7 +111,7 @@ def read_content(private_data_dir, raw_env, inventory_update): 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 = abs_file_path.replace(private_data_dir, '/runner') # host path to container path if runner_path in inverse_env: referenced_paths.add(abs_file_path) alias = 'file_reference' From 1f1cdf885938c1f26504ec7a7afea11f0e4eb1a9 Mon Sep 17 00:00:00 2001 From: Alan Rominger Date: Wed, 21 Apr 2021 17:19:37 -0400 Subject: [PATCH 08/14] start on path helper methods --- awx/main/utils/execution_environments.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/awx/main/utils/execution_environments.py b/awx/main/utils/execution_environments.py index ed6bb87f34..8e5ef7918a 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,16 @@ 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): + if not os.path.isabs(private_data_dir): + raise Exception + if Path(private_data_dir) not in Path(path).parents: + raise Exception + return path.replace(private_data_dir, CONTAINER_ROOT) From d33a748eea64ae86af497384025111bb0bac21de Mon Sep 17 00:00:00 2001 From: Alan Rominger Date: Thu, 22 Apr 2021 09:37:42 -0400 Subject: [PATCH 09/14] Clean up and test patch changing methods --- .../unit/utils/test_execution_environments.py | 45 +++++++++++++++++++ awx/main/utils/execution_environments.py | 16 +++++-- 2 files changed, 57 insertions(+), 4 deletions(-) create mode 100644 awx/main/tests/unit/utils/test_execution_environments.py 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 8e5ef7918a..c9aca5a380 100644 --- a/awx/main/utils/execution_environments.py +++ b/awx/main/utils/execution_environments.py @@ -37,7 +37,15 @@ CONTAINER_ROOT = '/runner' def to_container_path(path, private_data_dir): if not os.path.isabs(private_data_dir): - raise Exception - if Path(private_data_dir) not in Path(path).parents: - raise Exception - return path.replace(private_data_dir, CONTAINER_ROOT) + 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): + 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) From 623cf0b4cdbfe51e5ea27d23d43253ee0aec243b Mon Sep 17 00:00:00 2001 From: Alan Rominger Date: Thu, 22 Apr 2021 10:05:25 -0400 Subject: [PATCH 10/14] Start migrating the /runner path references --- awx/main/models/credential/__init__.py | 7 +++---- awx/main/models/credential/injectors.py | 13 +++++++------ awx/main/models/inventory.py | 3 ++- awx/main/tasks.py | 2 +- awx/main/utils/execution_environments.py | 8 ++++++++ 5 files changed, 21 insertions(+), 12 deletions(-) diff --git a/awx/main/models/credential/__init__.py b/awx/main/models/credential/__init__.py index 2515f2f49b..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 @@ -497,8 +498,7 @@ class CredentialType(CommonModelNameNotUnique): 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', 'env', 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: @@ -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', 'env', 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 259cfc724e..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='') @@ -30,9 +32,9 @@ def gce(cred, env, private_data_dir): json.dump(json_cred, f, indent=2) f.close() os.chmod(path, stat.S_IRUSR | stat.S_IWUSR) - cred_path = os.path.join('/runner', 'env', os.path.basename(path)) - env['GCE_CREDENTIALS_FILE_PATH'] = cred_path - env['GCP_SERVICE_ACCOUNT_FILE'] = cred_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 @@ -103,8 +105,7 @@ def openstack(cred, env, private_data_dir): 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', 'env', 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): @@ -116,6 +117,6 @@ def kubernetes_bearer_token(cred, env, private_data_dir): 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', 'env', 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 b59b640b51..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', 'env', 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 5c2a396c58..c9a901f26c 100644 --- a/awx/main/tasks.py +++ b/awx/main/tasks.py @@ -96,7 +96,7 @@ from awx.main.utils import ( deepmerge, parse_yaml_or_json, ) -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 diff --git a/awx/main/utils/execution_environments.py b/awx/main/utils/execution_environments.py index c9aca5a380..67fee9566d 100644 --- a/awx/main/utils/execution_environments.py +++ b/awx/main/utils/execution_environments.py @@ -36,6 +36,11 @@ 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: @@ -44,6 +49,9 @@ def to_container_path(path, private_data_dir): 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: From fd466c5cff585d1e118448bcec91294163309f3a Mon Sep 17 00:00:00 2001 From: Alan Rominger Date: Thu, 22 Apr 2021 10:56:48 -0400 Subject: [PATCH 11/14] Finish converting the runner strings --- awx/main/tasks.py | 17 +++++------ .../test_inventory_source_injectors.py | 3 +- awx/main/tests/unit/test_tasks.py | 29 +++++++++---------- 3 files changed, 22 insertions(+), 27 deletions(-) diff --git a/awx/main/tasks.py b/awx/main/tasks.py index c9a901f26c..f4364d5490 100644 --- a/awx/main/tasks.py +++ b/awx/main/tasks.py @@ -1532,7 +1532,7 @@ class RunJob(BaseTask): 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', 'env', os.path.basename(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='') @@ -1564,8 +1564,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 @@ -2391,8 +2390,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 @@ -2421,14 +2419,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 @@ -2479,10 +2477,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 5ab319aa75..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 @@ -111,7 +112,7 @@ def read_content(private_data_dir, raw_env, inventory_update): continue # Ansible runner abs_file_path = os.path.join(private_data_dir, filename) file_aliases[abs_file_path] = filename - runner_path = abs_file_path.replace(private_data_dir, '/runner') # host path to container 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 893a3deff6..77a13b24fe 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_container_path, to_host_path from awx.main.utils.licensing import Licenser @@ -341,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 = chunk[len('@') :].replace('/runner', private_data_dir) # container path to host path + 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 @@ -892,10 +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 = env['K8S_AUTH_SSL_CA_CERT'].replace('/runner', private_data_dir) # container path to host path - print('env') - print(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: @@ -945,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 = runner_path.replace('/runner', private_data_dir) # container path to host 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 @@ -1017,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 = env['OS_CLIENT_CONFIG_FILE'].replace('/runner', private_data_dir) # container path to host path + 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( [ @@ -1053,7 +1050,7 @@ class TestJobCredentials(TestJobExecution): credential.credential_type.inject_credential(credential, env, safe_env, [], private_data_dir) config = configparser.ConfigParser() - host_path = env['OVIRT_INI_PATH'].replace('/runner', private_data_dir) # container path to host 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' @@ -1267,7 +1264,7 @@ class TestJobCredentials(TestJobExecution): env = {} credential.credential_type.inject_credential(credential, env, {}, [], private_data_dir) - path = env['MY_CLOUD_INI_FILE'].replace('/runner', private_data_dir) # container path to host path + 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): @@ -1287,7 +1284,7 @@ class TestJobCredentials(TestJobExecution): env = {} credential.credential_type.inject_credential(credential, env, {}, [], private_data_dir) - path = env['MY_CLOUD_INI_FILE'].replace('/runner', private_data_dir) # container path to host path + 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): @@ -1306,8 +1303,8 @@ class TestJobCredentials(TestJobExecution): env = {} credential.credential_type.inject_credential(credential, env, {}, [], private_data_dir) - cert_path = env['MY_CERT_INI_FILE'].replace('/runner', private_data_dir) # container path to host path - key_path = env['MY_KEY_INI_FILE'].replace('/runner', private_data_dir) # container path to host path + 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' @@ -1330,7 +1327,7 @@ class TestJobCredentials(TestJobExecution): assert env['AZURE_AD_USER'] == 'bob' assert env['AZURE_PASSWORD'] == 'secret' - path = env['GCE_CREDENTIALS_FILE_PATH'].replace('/runner', private_data_dir) # container path to host 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 @@ -1711,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 = env['OS_CLIENT_CONFIG_FILE'].replace('/runner', private_data_dir) # container path to host path + path = to_host_path(env['OS_CLIENT_CONFIG_FILE'], private_data_dir) shade_config = open(path, 'r').read() assert ( '\n'.join( From 29c961e52a573c117daca31507b9bc86ad5e3ad8 Mon Sep 17 00:00:00 2001 From: Alan Rominger Date: Thu, 22 Apr 2021 11:00:01 -0400 Subject: [PATCH 12/14] Remove comment --- awx/main/tasks.py | 1 - 1 file changed, 1 deletion(-) diff --git a/awx/main/tasks.py b/awx/main/tasks.py index f4364d5490..887daac1c4 100644 --- a/awx/main/tasks.py +++ b/awx/main/tasks.py @@ -2464,7 +2464,6 @@ class RunInventoryUpdate(BaseTask): os.chmod(inventory_path, stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR) rel_path = os.path.join('inventory', injector.filename) - # rel_path = injector.filename elif src == 'scm': rel_path = os.path.join('project', inventory_update.source_path) From ae320ab2287290a51ed9a089cc386dac140dadb0 Mon Sep 17 00:00:00 2001 From: Alan Rominger Date: Thu, 22 Apr 2021 11:29:26 -0400 Subject: [PATCH 13/14] Do not set openstack env var to blank string --- awx/main/tasks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/awx/main/tasks.py b/awx/main/tasks.py index 887daac1c4..d7677c2890 100644 --- a/awx/main/tasks.py +++ b/awx/main/tasks.py @@ -1531,7 +1531,7 @@ 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': + 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: From 6649b435ce3d84dea83c98ef1cf67a4e2861978f Mon Sep 17 00:00:00 2001 From: Alan Rominger Date: Fri, 23 Apr 2021 16:06:13 -0400 Subject: [PATCH 14/14] Fix flake8 error --- awx/main/tests/unit/test_tasks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/awx/main/tests/unit/test_tasks.py b/awx/main/tests/unit/test_tasks.py index 77a13b24fe..2ec44b3c1e 100644 --- a/awx/main/tests/unit/test_tasks.py +++ b/awx/main/tests/unit/test_tasks.py @@ -37,7 +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_container_path, to_host_path +from awx.main.utils.execution_environments import CONTAINER_ROOT, to_host_path from awx.main.utils.licensing import Licenser