Merge pull request #8185 from AlanCoding/sniff_files_final

Use inventory and env private_data_dir subfolders

SUMMARY
This is another part of trying to get AWX to follow ansible-runner best practices.
See docs:
https://ansible-runner.readthedocs.io/en/stable/intro.html#runner-input-directory-hierarchy
So what runner & people expect is:
.
├── env
│   ├── envvars
│   ├── extravars
│   ├── passwords
│   ├── cmdline
│   ├── settings
│   └── ssh_key
├── inventory
│   └── hosts
└── project
    ├── test.yml
    └── roles
        └── testrole
            ├── defaults
            ├── handlers
            ├── meta
            ├── README.md
            ├── tasks
            ├── tests
            └── vars

What we were producing was something more like
├── cp
├── env
│   ├── cmdline
│   ├── envvars
│   ├── extravars
│   ├── passwords
│   ├── settings
│   └── ssh_key
├── project
│   ├── ansible.cfg
│   ├── ansible_env.yml
│   ├── async_tasks.yml
<snip>
│   └── vault.yml
└── tmp3evnorsu

There was no inventory folder, and inventory was stored in tmp3evnorsu as a top-level file. Any credential files were also saved in that same structure.
With this change, it's more like:
├── cp
├── env
│   ├── cmdline
│   ├── envvars
│   ├── extravars
│   ├── passwords
│   └── settings
├── inventory
│   └── tmp94xjm0d7
└── project
    ├── ansible.cfg
    ├── ansible_env.yml
    ├── async_tasks.yml
<snip>
    └── vault.yml

and any credential files (starting with tmp) are dumped into the env folder.
ISSUE TYPE

Feature Pull Request

COMPONENT NAME

API

AWX VERSION
14.1.0

ADDITIONAL INFORMATION
The motivation for this is that ansible-runner may require specific mounts for every file of directory in the private_data_dir. As such, we really don't want randomized filenames in the top level directory. I don't know for sure this is the direction we are going to go, but this seems like a defensible change on its own, to better adhere to the practices someone familiar with ansible-runner would expect.

Reviewed-by: Alan Rominger <arominge@redhat.com>
Reviewed-by: Chris Meyers <None>
This commit is contained in:
softwarefactory-project-zuul[bot] 2021-05-19 17:24:33 +00:00 committed by GitHub
commit 68f214c2be
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 139 additions and 51 deletions

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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