From bba7f45972f92693ac0837f476011f0649c79b4d 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.py | 14 +- awx/main/tasks.py | 17 ++- .../tests/unit/models/test_survey_models.py | 8 +- awx/main/tests/unit/test_tasks.py | 141 +++++++++++------- 4 files changed, 118 insertions(+), 62 deletions(-) diff --git a/awx/main/models/credential.py b/awx/main/models/credential.py index f33ed07ee4..e4bbfff675 100644 --- a/awx/main/models/credential.py +++ b/awx/main/models/credential.py @@ -586,11 +586,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 52d70f51d3..1aeae770d2 100644 --- a/awx/main/tasks.py +++ b/awx/main/tasks.py @@ -622,6 +622,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, env, add_awx_lib=True): env['VIRTUAL_ENV'] = settings.ANSIBLE_VENV_PATH env['PATH'] = os.path.join(settings.ANSIBLE_VENV_PATH, "bin") + ":" + env['PATH'] @@ -1205,7 +1213,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) @@ -1460,7 +1469,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 @@ -2220,7 +2230,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 abd0b5fbba..284f69296c 100644 --- a/awx/main/tests/unit/models/test_survey_models.py +++ b/awx/main/tests/unit/models/test_survey_models.py @@ -71,7 +71,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$' @@ -80,7 +82,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 20a051ec30..f807e02165 100644 --- a/awx/main/tests/unit/test_tasks.py +++ b/awx/main/tests/unit/test_tasks.py @@ -172,6 +172,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 @@ -296,15 +305,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({ @@ -313,12 +325,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'}) @@ -385,16 +400,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): @@ -986,14 +1004,16 @@ class TestJobCredentials(TestJobExecution): inputs = {'api_token': 'ABC123'} ) self.instance.extra_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', @@ -1018,12 +1038,15 @@ class TestJobCredentials(TestJobExecution): inputs={'turbo_button': True} ) self.instance.extra_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( @@ -1049,12 +1072,15 @@ class TestJobCredentials(TestJobExecution): inputs={'turbo_button': True} ) self.instance.extra_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): """ @@ -1085,13 +1111,16 @@ class TestJobCredentials(TestJobExecution): ) credential.inputs['password'] = encrypt_field(credential, 'password') self.instance.extra_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): @@ -1217,20 +1246,22 @@ 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', + os.path.realpath(settings.PROJECTS_ROOT), + os.path.realpath(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