From 55be530e279ae39d4c66145095f3b37de1d4583f Mon Sep 17 00:00:00 2001 From: Ryan Petrello Date: Fri, 13 Jul 2018 09:44:11 -0400 Subject: [PATCH] awx metavars for job.created_by should be more robust to DoesNotExist see: https://github.com/ansible/tower/issues/2509 --- awx/main/models/unified_jobs.py | 35 ++++++++++--------- .../functional/models/test_unified_job.py | 24 +++++++++++++ awx/main/utils/common.py | 9 ++++- 3 files changed, 51 insertions(+), 17 deletions(-) diff --git a/awx/main/models/unified_jobs.py b/awx/main/models/unified_jobs.py index 1be11ebcca..864655f60a 100644 --- a/awx/main/models/unified_jobs.py +++ b/awx/main/models/unified_jobs.py @@ -36,7 +36,7 @@ from awx.main.models.mixins import ResourceMixin, TaskManagerUnifiedJobMixin from awx.main.utils import ( encrypt_dict, decrypt_field, _inventory_updates, copy_model_by_class, copy_m2m_relationships, - get_type_for_model, parse_yaml_or_json + get_type_for_model, parse_yaml_or_json, getattr_dne ) from awx.main.utils import polymorphic from awx.main.constants import ACTIVE_STATES, CAN_CANCEL @@ -1376,27 +1376,30 @@ class UnifiedJob(PolymorphicModel, PasswordFieldsModel, CommonModelNameNotUnique for name in ('awx', 'tower'): r['{}_job_id'.format(name)] = self.pk r['{}_job_launch_type'.format(name)] = self.launch_type - if self.created_by: - for name in ('awx', 'tower'): - r['{}_user_id'.format(name)] = self.created_by.pk - r['{}_user_name'.format(name)] = self.created_by.username - r['{}_user_email'.format(name)] = self.created_by.email - r['{}_user_first_name'.format(name)] = self.created_by.first_name - r['{}_user_last_name'.format(name)] = self.created_by.last_name - else: + + created_by = getattr_dne(self, 'created_by') + + if not created_by: wj = self.get_workflow_job() if wj: for name in ('awx', 'tower'): r['{}_workflow_job_id'.format(name)] = wj.pk r['{}_workflow_job_name'.format(name)] = wj.name - if wj.created_by: - for name in ('awx', 'tower'): - r['{}_user_id'.format(name)] = wj.created_by.pk - r['{}_user_name'.format(name)] = wj.created_by.username - if self.schedule: + created_by = getattr_dne(wj, 'created_by') + + schedule = getattr_dne(self, 'schedule') + if schedule: for name in ('awx', 'tower'): - r['{}_schedule_id'.format(name)] = self.schedule.pk - r['{}_schedule_name'.format(name)] = self.schedule.name + r['{}_schedule_id'.format(name)] = schedule.pk + r['{}_schedule_name'.format(name)] = schedule.name + + if created_by: + for name in ('awx', 'tower'): + r['{}_user_id'.format(name)] = created_by.pk + r['{}_user_name'.format(name)] = created_by.username + r['{}_user_email'.format(name)] = created_by.email + r['{}_user_first_name'.format(name)] = created_by.first_name + r['{}_user_last_name'.format(name)] = created_by.last_name return r def get_celery_queue_name(self): diff --git a/awx/main/tests/functional/models/test_unified_job.py b/awx/main/tests/functional/models/test_unified_job.py index 0ee56349fe..f85cc4fe5d 100644 --- a/awx/main/tests/functional/models/test_unified_job.py +++ b/awx/main/tests/functional/models/test_unified_job.py @@ -1,3 +1,4 @@ +import itertools import pytest import mock @@ -113,6 +114,27 @@ class TestMetaVars: Extension of unit tests with same class name ''' + def test_deleted_user(self, admin_user): + job = Job.objects.create( + name='job', + created_by=admin_user + ) + job.save() + + user_vars = ['_'.join(x) for x in itertools.product( + ['tower', 'awx'], + ['user_name', 'user_id', 'user_email', 'user_first_name', 'user_last_name'] + )] + + for key in user_vars: + assert key in job.awx_meta_vars() + + # deleted user is hard to simulate as this test occurs within one transaction + job = Job.objects.get(pk=job.id) + job.created_by_id = 999999999 + for key in user_vars: + assert key not in job.awx_meta_vars() + def test_workflow_job_metavars(self, admin_user): workflow_job = WorkflowJob.objects.create( name='workflow-job', @@ -124,6 +146,7 @@ class TestMetaVars: ) workflow_job.workflow_nodes.create(job=job) data = job.awx_meta_vars() + assert data['awx_user_id'] == admin_user.id assert data['awx_user_name'] == admin_user.username assert data['awx_workflow_job_id'] == workflow_job.pk @@ -141,6 +164,7 @@ class TestMetaVars: ) data = job.awx_meta_vars() assert data['awx_schedule_id'] == schedule.pk + assert 'awx_user_name' not in data @pytest.mark.django_db diff --git a/awx/main/utils/common.py b/awx/main/utils/common.py index 99141a02c2..a1373894d8 100644 --- a/awx/main/utils/common.py +++ b/awx/main/utils/common.py @@ -47,7 +47,7 @@ __all__ = ['get_object_or_400', 'get_object_or_403', 'camelcase_to_underscore', 'get_type_for_model', 'get_model_for_type', 'copy_model_by_class', 'region_sorting', 'copy_m2m_relationships', 'prefetch_page_capabilities', 'to_python_boolean', 'ignore_inventory_computed_fields', 'ignore_inventory_group_removal', - '_inventory_updates', 'get_pk_from_dict', 'getattrd', 'NoDefaultProvided', + '_inventory_updates', 'get_pk_from_dict', 'getattrd', 'getattr_dne', 'NoDefaultProvided', 'get_current_apps', 'set_current_apps', 'OutputEventFilter', 'OutputVerboseFilter', 'extract_ansible_vars', 'get_search_fields', 'get_system_task_capacity', 'get_cpu_capacity', 'get_mem_capacity', 'wrap_args_with_proot', 'build_proot_temp_dir', 'check_proot_installed', 'model_to_dict', @@ -908,6 +908,13 @@ def getattrd(obj, name, default=NoDefaultProvided): raise +def getattr_dne(obj, name, notfound=ObjectDoesNotExist): + try: + return getattr(obj, name) + except notfound: + return None + + current_apps = apps