From aa5bd9f5bf5c6871795a3b7dacfdfd3a7774f43d Mon Sep 17 00:00:00 2001 From: Bill Nottingham Date: Fri, 2 Feb 2018 23:30:51 -0500 Subject: [PATCH] Pass extra vars via file rather than via commandline, including custom creds. The extra vars file created lives in the playbook private runtime directory, and will be reaped along with the rest of the directory. Adjust assorted unit tests as necessary. --- awx/main/models/credential/__init__.py | 14 +- awx/main/tasks.py | 17 ++- .../tests/unit/models/test_survey_models.py | 8 +- awx/main/tests/unit/test_tasks.py | 137 +++++++++++------- awx/main/utils/common.py | 2 + 5 files changed, 116 insertions(+), 62 deletions(-) diff --git a/awx/main/models/credential/__init__.py b/awx/main/models/credential/__init__.py index 86c3930299..e51758d2b3 100644 --- a/awx/main/models/credential/__init__.py +++ b/awx/main/models/credential/__init__.py @@ -654,11 +654,21 @@ class CredentialType(CommonModelNameNotUnique): extra_vars[var_name] = Template(tmpl).render(**namespace) safe_extra_vars[var_name] = Template(tmpl).render(**safe_namespace) + def build_extra_vars_file(vars, private_dir): + handle, path = tempfile.mkstemp(dir = private_dir) + f = os.fdopen(handle, 'w') + f.write(json.dumps(vars)) + f.close() + os.chmod(path, stat.S_IRUSR) + return path + if extra_vars: - args.extend(['-e', json.dumps(extra_vars)]) + path = build_extra_vars_file(extra_vars, private_data_dir) + args.extend(['-e', '@%s' % path]) if safe_extra_vars: - safe_args.extend(['-e', json.dumps(safe_extra_vars)]) + path = build_extra_vars_file(safe_extra_vars, private_data_dir) + safe_args.extend(['-e', '@%s' % path]) @CredentialType.default diff --git a/awx/main/tasks.py b/awx/main/tasks.py index 10fde0d82b..30c804db7e 100644 --- a/awx/main/tasks.py +++ b/awx/main/tasks.py @@ -727,6 +727,14 @@ class BaseTask(LogErrorsTask): '': '', } + def build_extra_vars_file(self, vars, **kwargs): + handle, path = tempfile.mkstemp(dir=kwargs.get('private_data_dir', None)) + f = os.fdopen(handle, 'w') + f.write(json.dumps(vars)) + f.close() + os.chmod(path, stat.S_IRUSR) + return path + def add_ansible_venv(self, venv_path, env, add_awx_lib=True): env['VIRTUAL_ENV'] = venv_path env['PATH'] = os.path.join(venv_path, "bin") + ":" + env['PATH'] @@ -1236,7 +1244,8 @@ class RunJob(BaseTask): extra_vars.update(json.loads(job.display_extra_vars())) else: extra_vars.update(json.loads(job.decrypted_extra_vars())) - args.extend(['-e', json.dumps(extra_vars)]) + extra_vars_path = self.build_extra_vars_file(vars=extra_vars, **kwargs) + args.extend(['-e', '@%s' % (extra_vars_path)]) # Add path to playbook (relative to project.local_path). args.append(job.playbook) @@ -1466,7 +1475,8 @@ class RunProjectUpdate(BaseTask): 'scm_revision_output': self.revision_path, 'scm_revision': project_update.project.scm_revision, }) - args.extend(['-e', json.dumps(extra_vars)]) + extra_vars_path = self.build_extra_vars_file(vars=extra_vars, **kwargs) + args.extend(['-e', '@%s' % (extra_vars_path)]) args.append('project_update.yml') return args @@ -2183,7 +2193,8 @@ class RunAdHocCommand(BaseTask): "{} are prohibited from use in ad hoc commands." ).format(", ".join(removed_vars))) extra_vars.update(ad_hoc_command.extra_vars_dict) - args.extend(['-e', json.dumps(extra_vars)]) + extra_vars_path = self.build_extra_vars_file(vars=extra_vars, **kwargs) + args.extend(['-e', '@%s' % (extra_vars_path)]) args.extend(['-m', ad_hoc_command.module_name]) args.extend(['-a', ad_hoc_command.module_args]) diff --git a/awx/main/tests/unit/models/test_survey_models.py b/awx/main/tests/unit/models/test_survey_models.py index e6388a890c..8a93902f9e 100644 --- a/awx/main/tests/unit/models/test_survey_models.py +++ b/awx/main/tests/unit/models/test_survey_models.py @@ -120,7 +120,9 @@ def test_job_safe_args_redacted_passwords(job): run_job = RunJob() safe_args = run_job.build_safe_args(job, **kwargs) ev_index = safe_args.index('-e') + 1 - extra_vars = json.loads(safe_args[ev_index]) + extra_var_file = open(safe_args[ev_index][1:], 'r') + extra_vars = json.load(extra_var_file) + extra_var_file.close() assert extra_vars['secret_key'] == '$encrypted$' @@ -129,7 +131,9 @@ def test_job_args_unredacted_passwords(job, tmpdir_factory): run_job = RunJob() args = run_job.build_args(job, **kwargs) ev_index = args.index('-e') + 1 - extra_vars = json.loads(args[ev_index]) + extra_var_file = open(args[ev_index][1:], 'r') + extra_vars = json.load(extra_var_file) + extra_var_file.close() assert extra_vars['secret_key'] == 'my_password' diff --git a/awx/main/tests/unit/test_tasks.py b/awx/main/tests/unit/test_tasks.py index d83dea71dd..34542eb9e1 100644 --- a/awx/main/tests/unit/test_tasks.py +++ b/awx/main/tests/unit/test_tasks.py @@ -174,6 +174,15 @@ def pytest_generate_tests(metafunc): ) +def parse_extra_vars(args): + extra_vars = {} + for chunk in args: + if chunk.startswith('@/tmp/'): + with open(chunk.strip('@'), 'r') as f: + extra_vars.update(json.load(f)) + return extra_vars + + class TestJobExecution: """ For job runs, test that `ansible-playbook` is invoked with the proper @@ -318,15 +327,18 @@ class TestGenericRun(TestJobExecution): def test_created_by_extra_vars(self): self.instance.created_by = User(pk=123, username='angry-spud') - self.task.run(self.pk) - assert self.run_pexpect.call_count == 1 - call_args, _ = self.run_pexpect.call_args_list[0] - args, cwd, env, stdout = call_args - assert '"tower_user_id": 123,' in ' '.join(args) - assert '"tower_user_name": "angry-spud"' in ' '.join(args) - assert '"awx_user_id": 123,' in ' '.join(args) - assert '"awx_user_name": "angry-spud"' in ' '.join(args) + def run_pexpect_side_effect(*args, **kwargs): + args, cwd, env, stdout = args + extra_vars = parse_extra_vars(args) + assert extra_vars['tower_user_id'] == 123 + assert extra_vars['tower_user_name'] == "angry-spud" + assert extra_vars['awx_user_id'] == 123 + assert extra_vars['awx_user_name'] == "angry-spud" + return ['successful', 0] + + self.run_pexpect.side_effect = run_pexpect_side_effect + self.task.run(self.pk) def test_survey_extra_vars(self): self.instance.extra_vars = json.dumps({ @@ -335,12 +347,15 @@ class TestGenericRun(TestJobExecution): self.instance.survey_passwords = { 'super_secret': '$encrypted$' } - self.task.run(self.pk) - assert self.run_pexpect.call_count == 1 - call_args, _ = self.run_pexpect.call_args_list[0] - args, cwd, env, stdout = call_args - assert '"super_secret": "CLASSIFIED"' in ' '.join(args) + def run_pexpect_side_effect(*args, **kwargs): + args, cwd, env, stdout = args + extra_vars = parse_extra_vars(args) + assert extra_vars['super_secret'] == "CLASSIFIED" + return ['successful', 0] + + self.run_pexpect.side_effect = run_pexpect_side_effect + self.task.run(self.pk) def test_awx_task_env(self): patch = mock.patch('awx.main.tasks.settings.AWX_TASK_ENV', {'FOO': 'BAR'}) @@ -394,16 +409,19 @@ class TestAdhocRun(TestJobExecution): def test_created_by_extra_vars(self): self.instance.created_by = User(pk=123, username='angry-spud') - self.task.run(self.pk) - assert self.run_pexpect.call_count == 1 - call_args, _ = self.run_pexpect.call_args_list[0] - args, cwd, env, stdout = call_args - assert '"tower_user_id": 123,' in ' '.join(args) - assert '"tower_user_name": "angry-spud"' in ' '.join(args) - assert '"awx_user_id": 123,' in ' '.join(args) - assert '"awx_user_name": "angry-spud"' in ' '.join(args) - assert '"awx_foo": "awx-bar' in ' '.join(args) + def run_pexpect_side_effect(*args, **kwargs): + args, cwd, env, stdout = args + extra_vars = parse_extra_vars(args) + assert extra_vars['tower_user_id'] == 123 + assert extra_vars['tower_user_name'] == "angry-spud" + assert extra_vars['awx_user_id'] == 123 + assert extra_vars['awx_user_name'] == "angry-spud" + assert extra_vars['awx_foo'] == "awx-bar" + return ['successful', 0] + + self.run_pexpect.side_effect = run_pexpect_side_effect + self.task.run(self.pk) class TestIsolatedExecution(TestJobExecution): @@ -1082,14 +1100,16 @@ class TestJobCredentials(TestJobExecution): inputs = {'api_token': 'ABC123'} ) self.instance.credentials.add(credential) + + def run_pexpect_side_effect(*args, **kwargs): + args, cwd, env, stdout = args + extra_vars = parse_extra_vars(args) + assert extra_vars["api_token"] == "ABC123" + return ['successful', 0] + + self.run_pexpect.side_effect = run_pexpect_side_effect self.task.run(self.pk) - assert self.run_pexpect.call_count == 1 - call_args, _ = self.run_pexpect.call_args_list[0] - args, cwd, env, stdout = call_args - - assert '-e {"api_token": "ABC123"}' in ' '.join(args) - def test_custom_environment_injectors_with_boolean_extra_vars(self): some_cloud = CredentialType( kind='cloud', @@ -1114,12 +1134,15 @@ class TestJobCredentials(TestJobExecution): inputs={'turbo_button': True} ) self.instance.credentials.add(credential) - self.task.run(self.pk) - assert self.run_pexpect.call_count == 1 - call_args, _ = self.run_pexpect.call_args_list[0] - args, cwd, env, stdout = call_args - assert '-e {"turbo_button": "True"}' in ' '.join(args) + def run_pexpect_side_effect(*args, **kwargs): + args, cwd, env, stdout = args + extra_vars = parse_extra_vars(args) + assert extra_vars["turbo_button"] == "True" + return ['successful', 0] + + self.run_pexpect.side_effect = run_pexpect_side_effect + self.task.run(self.pk) def test_custom_environment_injectors_with_complicated_boolean_template(self): some_cloud = CredentialType( @@ -1145,12 +1168,15 @@ class TestJobCredentials(TestJobExecution): inputs={'turbo_button': True} ) self.instance.credentials.add(credential) - self.task.run(self.pk) - assert self.run_pexpect.call_count == 1 - call_args, _ = self.run_pexpect.call_args_list[0] - args, cwd, env, stdout = call_args - assert '-e {"turbo_button": "FAST!"}' in ' '.join(args) + def run_pexpect_side_effect(*args, **kwargs): + args, cwd, env, stdout = args + extra_vars = parse_extra_vars(args) + assert extra_vars["turbo_button"] == "FAST!" + return ['successful', 0] + + self.run_pexpect.side_effect = run_pexpect_side_effect + self.task.run(self.pk) def test_custom_environment_injectors_with_secret_extra_vars(self): """ @@ -1181,13 +1207,16 @@ class TestJobCredentials(TestJobExecution): ) credential.inputs['password'] = encrypt_field(credential, 'password') self.instance.credentials.add(credential) + + def run_pexpect_side_effect(*args, **kwargs): + args, cwd, env, stdout = args + extra_vars = parse_extra_vars(args) + assert extra_vars["password"] == "SUPER-SECRET-123" + return ['successful', 0] + + self.run_pexpect.side_effect = run_pexpect_side_effect self.task.run(self.pk) - assert self.run_pexpect.call_count == 1 - call_args, _ = self.run_pexpect.call_args_list[0] - args, cwd, env, stdout = call_args - - assert '-e {"password": "SUPER-SECRET-123"}' in ' '.join(args) assert 'SUPER-SECRET-123' not in json.dumps(self.task.update_model.call_args_list) def test_custom_environment_injectors_with_file(self): @@ -1358,20 +1387,18 @@ class TestProjectUpdateCredentials(TestJobExecution): pk=1, credential_type=ssh, ) + + def run_pexpect_side_effect(*args, **kwargs): + args, cwd, env, stdout = args + extra_vars = parse_extra_vars(args) + assert ' '.join(args).startswith('bwrap') + assert ' '.join(['--bind', settings.PROJECTS_ROOT, settings.PROJECTS_ROOT]) in ' '.join(args) + assert extra_vars["scm_revision_output"].startswith(settings.PROJECTS_ROOT) + return ['successful', 0] + + self.run_pexpect.side_effect = run_pexpect_side_effect self.task.run(self.pk) - assert self.run_pexpect.call_count == 1 - call_args, call_kwargs = self.run_pexpect.call_args_list[0] - args, cwd, env, stdout = call_args - - assert ' '.join(args).startswith('bwrap') - ' '.join([ - '--bind', - settings.PROJECTS_ROOT, - settings.PROJECTS_ROOT, - ]) in ' '.join(args) - assert '"scm_revision_output": "/projects/tmp' in ' '.join(args) - def test_username_and_password_auth(self, scm_type): ssh = CredentialType.defaults['ssh']() self.instance.scm_type = scm_type diff --git a/awx/main/utils/common.py b/awx/main/utils/common.py index c32821169a..ce6f66ede1 100644 --- a/awx/main/utils/common.py +++ b/awx/main/utils/common.py @@ -803,6 +803,8 @@ def wrap_args_with_proot(args, cwd, **kwargs): if not os.path.exists(path): continue path = os.path.realpath(path) + if os.path.isdir(path): + path = os.path.join(path, '') # add a trailing slash new_args.extend(['--bind', '%s' % (path,), '%s' % (path,)]) if kwargs.get('isolated'): if 'ansible-playbook' in args: