From 1874e8bb4c52125ee6f1d062735cc8dd1fc95c95 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Tue, 7 May 2019 21:22:15 -0400 Subject: [PATCH 1/2] Reduce passing around of passwords dictionary --- awx/main/tasks.py | 38 ++++++++----------------------- awx/main/tests/unit/test_tasks.py | 22 +++++++++--------- 2 files changed, 20 insertions(+), 40 deletions(-) diff --git a/awx/main/tasks.py b/awx/main/tasks.py index e241fe2efc..8abf8680fb 100644 --- a/awx/main/tasks.py +++ b/awx/main/tasks.py @@ -840,7 +840,7 @@ class BaseTask(object): '': '', } - def build_extra_vars_file(self, instance, private_data_dir, passwords): + def build_extra_vars_file(self, instance, private_data_dir): ''' Build ansible yaml file filled with extra vars to be passed via -e@file.yml ''' @@ -1155,7 +1155,7 @@ class BaseTask(object): # May have to serialize the value private_data_files = self.build_private_data_files(self.instance, private_data_dir) passwords = self.build_passwords(self.instance, kwargs) - self.build_extra_vars_file(self.instance, private_data_dir, passwords) + self.build_extra_vars_file(self.instance, private_data_dir) args = self.build_args(self.instance, private_data_dir, passwords) cwd = self.build_cwd(self.instance, private_data_dir) process_isolation_params = self.build_params_process_isolation(self.instance, @@ -1507,7 +1507,7 @@ class RunJob(BaseTask): def build_playbook_path_relative_to_cwd(self, job, private_data_dir): return os.path.join(job.playbook) - def build_extra_vars_file(self, job, private_data_dir, passwords): + def build_extra_vars_file(self, job, private_data_dir): # Define special extra_vars for AWX, combine with job.extra_vars. extra_vars = job.awx_meta_vars() @@ -1679,12 +1679,14 @@ class RunProjectUpdate(BaseTask): env['ANSIBLE_CALLBACK_PLUGINS'] = self.get_path_to('..', 'plugins', 'callback') return env - def _build_scm_url_extra_vars(self, project_update, scm_username='', scm_password=''): + def _build_scm_url_extra_vars(self, project_update): ''' Helper method to build SCM url and extra vars with parameters needed for authentication. ''' extra_vars = {} + scm_username = project_update.credential.get_input('username', default='') + scm_password = project_update.credential.get_input('password', default='') scm_type = project_update.scm_type scm_url = update_scm_url(scm_type, project_update.scm_url, check_special_cases=False) @@ -1730,11 +1732,9 @@ class RunProjectUpdate(BaseTask): args.append('-v') return args - def build_extra_vars_file(self, project_update, private_data_dir, passwords): + def build_extra_vars_file(self, project_update, private_data_dir): extra_vars = {} - scm_url, extra_vars_new = self._build_scm_url_extra_vars(project_update, - passwords.get('scm_username', ''), - passwords.get('scm_password', '')) + scm_url, extra_vars_new = self._build_scm_url_extra_vars(project_update) extra_vars.update(extra_vars_new) if project_update.project.scm_revision and project_update.job_type == 'run': @@ -1939,26 +1939,6 @@ class RunInventoryUpdate(BaseTask): injector = InventorySource.injectors[inventory_update.source](self.get_ansible_version(inventory_update)) return injector.build_private_data(inventory_update, private_data_dir) - def build_passwords(self, inventory_update, runtime_passwords): - """Build a dictionary of authentication/credential information for - an inventory source. - - This dictionary is used by `build_env`, below. - """ - # Run the superclass implementation. - passwords = super(RunInventoryUpdate, self).build_passwords(inventory_update, runtime_passwords) - - # Take key fields from the credential in use and add them to the - # passwords dictionary. - credential = inventory_update.get_cloud_credential() - if credential: - for subkey in ('username', 'host', 'project', 'client', 'tenant', 'subscription'): - passwords['source_%s' % subkey] = credential.get_input(subkey, default='') - for passkey in ('password', 'ssh_key_data', 'security_token', 'secret'): - k = 'source_%s' % passkey - passwords[k] = credential.get_input(passkey, default='') - return passwords - def build_env(self, inventory_update, private_data_dir, isolated, private_data_files=None): """Build environment dictionary for inventory import. @@ -2302,7 +2282,7 @@ class RunAdHocCommand(BaseTask): return args - def build_extra_vars_file(self, ad_hoc_command, private_data_dir, passwords={}): + def build_extra_vars_file(self, ad_hoc_command, private_data_dir): extra_vars = ad_hoc_command.awx_meta_vars() if ad_hoc_command.extra_vars_dict: diff --git a/awx/main/tests/unit/test_tasks.py b/awx/main/tests/unit/test_tasks.py index 491e85d130..cae88a6622 100644 --- a/awx/main/tests/unit/test_tasks.py +++ b/awx/main/tests/unit/test_tasks.py @@ -258,7 +258,7 @@ class TestExtraVarSanitation(TestJobExecution): job.created_by = User(pk=123, username='angry-spud') task = tasks.RunJob() - task.build_extra_vars_file(job, private_data_dir, {}) + task.build_extra_vars_file(job, private_data_dir) fd = open(os.path.join(private_data_dir, 'env', 'extravars')) extra_vars = yaml.load(fd, Loader=SafeLoader) @@ -282,7 +282,7 @@ class TestExtraVarSanitation(TestJobExecution): job.extra_vars = json.dumps({'msg': self.UNSAFE}) task = tasks.RunJob() - task.build_extra_vars_file(job, private_data_dir, {}) + task.build_extra_vars_file(job, private_data_dir) fd = open(os.path.join(private_data_dir, 'env', 'extravars')) extra_vars = yaml.load(fd, Loader=SafeLoader) @@ -293,7 +293,7 @@ class TestExtraVarSanitation(TestJobExecution): job.extra_vars = json.dumps({'msg': {'a': [self.UNSAFE]}}) task = tasks.RunJob() - task.build_extra_vars_file(job, private_data_dir, {}) + task.build_extra_vars_file(job, private_data_dir) fd = open(os.path.join(private_data_dir, 'env', 'extravars')) extra_vars = yaml.load(fd, Loader=SafeLoader) @@ -304,7 +304,7 @@ class TestExtraVarSanitation(TestJobExecution): job.job_template.extra_vars = job.extra_vars = json.dumps({'msg': self.UNSAFE}) task = tasks.RunJob() - task.build_extra_vars_file(job, private_data_dir, {}) + task.build_extra_vars_file(job, private_data_dir) fd = open(os.path.join(private_data_dir, 'env', 'extravars')) extra_vars = yaml.load(fd, Loader=SafeLoader) @@ -316,7 +316,7 @@ class TestExtraVarSanitation(TestJobExecution): job.job_template.extra_vars = job.extra_vars task = tasks.RunJob() - task.build_extra_vars_file(job, private_data_dir, {}) + task.build_extra_vars_file(job, private_data_dir) fd = open(os.path.join(private_data_dir, 'env', 'extravars')) extra_vars = yaml.load(fd, Loader=SafeLoader) @@ -333,7 +333,7 @@ class TestExtraVarSanitation(TestJobExecution): }) task = tasks.RunJob() - task.build_extra_vars_file(job, private_data_dir, {}) + task.build_extra_vars_file(job, private_data_dir) fd = open(os.path.join(private_data_dir, 'env', 'extravars')) extra_vars = yaml.load(fd, Loader=SafeLoader) @@ -348,7 +348,7 @@ class TestExtraVarSanitation(TestJobExecution): job.extra_vars = json.dumps({'msg': self.UNSAFE}) task = tasks.RunJob() - task.build_extra_vars_file(job, private_data_dir, {}) + task.build_extra_vars_file(job, private_data_dir) fd = open(os.path.join(private_data_dir, 'env', 'extravars')) extra_vars = yaml.load(fd, Loader=SafeLoader) @@ -468,7 +468,7 @@ class TestGenericRun(): task = tasks.RunJob() task._write_extra_vars_file = mock.Mock() - task.build_extra_vars_file(job, None, dict()) + task.build_extra_vars_file(job, None) call_args, _ = task._write_extra_vars_file.call_args_list[0] @@ -489,7 +489,7 @@ class TestGenericRun(): task = tasks.RunJob() task._write_extra_vars_file = mock.Mock() - task.build_extra_vars_file(job, None, dict()) + task.build_extra_vars_file(job, None) call_args, _ = task._write_extra_vars_file.call_args_list[0] @@ -577,7 +577,7 @@ class TestAdhocRun(TestJobExecution): task = tasks.RunAdHocCommand() task._write_extra_vars_file = mock.Mock() - task.build_extra_vars_file(adhoc_job, None, dict()) + task.build_extra_vars_file(adhoc_job, None) call_args, _ = task._write_extra_vars_file.call_args_list[0] @@ -1685,7 +1685,7 @@ class TestProjectUpdateCredentials(TestJobExecution): assert settings.PROJECTS_ROOT in process_isolation['process_isolation_show_paths'] task._write_extra_vars_file = mock.Mock() - task.build_extra_vars_file(project_update, private_data_dir, {}) + task.build_extra_vars_file(project_update, private_data_dir) call_args, _ = task._write_extra_vars_file.call_args_list[0] _, extra_vars = call_args From 7e6a73f892a35f0b8617862586a1353d1034f343 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Wed, 8 May 2019 21:18:16 -0400 Subject: [PATCH 2/2] fix bug with null credential --- awx/main/tasks.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/awx/main/tasks.py b/awx/main/tasks.py index 8abf8680fb..d0caa8d527 100644 --- a/awx/main/tasks.py +++ b/awx/main/tasks.py @@ -1079,7 +1079,7 @@ class BaseTask(object): if status_data['status'] == 'starting': job_env = dict(runner_config.env) ''' - Take the safe environment variables and overwrite + Take the safe environment variables and overwrite ''' for k, v in self.safe_env.items(): if k in job_env: @@ -1685,8 +1685,12 @@ class RunProjectUpdate(BaseTask): for authentication. ''' extra_vars = {} - scm_username = project_update.credential.get_input('username', default='') - scm_password = project_update.credential.get_input('password', default='') + if project_update.credential: + scm_username = project_update.credential.get_input('username', default='') + scm_password = project_update.credential.get_input('password', default='') + else: + scm_username = '' + scm_password = '' scm_type = project_update.scm_type scm_url = update_scm_url(scm_type, project_update.scm_url, check_special_cases=False)