Merge pull request #2522 from ryanpetrello/fix-2509

awx metavars for job.created_by should be more robust to DoesNotExist

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

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

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