Merge pull request #237 from jangsutsr/7412_update_unified_job_deletion_behavior

Prevent deleting unified jobs with active status

awx/api/views.py

@@ -127,6 +127,25 @@ class WorkflowsEnforcementMixin(object):
         return super(WorkflowsEnforcementMixin, self).check_permissions(request)
 
 
+class UnifiedJobDeletionMixin(object):
+    '''
+    Special handling when deleting a running unified job object.
+    '''
+    def destroy(self, request, *args, **kwargs):
+        obj = self.get_object()
+        if not request.user.can_access(self.model, 'delete', obj):
+            raise PermissionDenied()
+        try:
+            if obj.unified_job_node.workflow_job.status in ACTIVE_STATES:
+                raise PermissionDenied(detail=_('Cannot delete job resource when associated workflow job is running.'))
+        except self.model.unified_job_node.RelatedObjectDoesNotExist:
+            pass
+        if obj.status in ACTIVE_STATES:
+            raise PermissionDenied(detail=_("Cannot delete running job resource."))
+        obj.delete()
+        return Response(status=status.HTTP_204_NO_CONTENT)
+
+
 class ApiRootView(APIView):
 
     authentication_classes = []
@@ -1296,21 +1315,12 @@ class ProjectUpdateList(ListAPIView):
     new_in_13 = True
 
 
-class ProjectUpdateDetail(RetrieveDestroyAPIView):
+class ProjectUpdateDetail(UnifiedJobDeletionMixin, RetrieveDestroyAPIView):
 
     model = ProjectUpdate
     serializer_class = ProjectUpdateSerializer
     new_in_13 = True
 
-    def destroy(self, request, *args, **kwargs):
-        obj = self.get_object()
-        try:
-            if obj.unified_job_node.workflow_job.status in ACTIVE_STATES:
-                raise PermissionDenied(detail=_('Cannot delete job resource when associated workflow job is running.'))
-        except ProjectUpdate.unified_job_node.RelatedObjectDoesNotExist:
-            pass
-        return super(ProjectUpdateDetail, self).destroy(request, *args, **kwargs)
-
 
 class ProjectUpdateCancel(RetrieveAPIView):
 
@@ -2663,21 +2673,12 @@ class InventoryUpdateList(ListAPIView):
     serializer_class = InventoryUpdateListSerializer
 
 
-class InventoryUpdateDetail(RetrieveDestroyAPIView):
+class InventoryUpdateDetail(UnifiedJobDeletionMixin, RetrieveDestroyAPIView):
 
     model = InventoryUpdate
     serializer_class = InventoryUpdateSerializer
     new_in_14 = True
 
-    def destroy(self, request, *args, **kwargs):
-        obj = self.get_object()
-        try:
-            if obj.unified_job_node.workflow_job.status in ACTIVE_STATES:
-                raise PermissionDenied(detail=_('Cannot delete job resource when associated workflow job is running.'))
-        except InventoryUpdate.unified_job_node.RelatedObjectDoesNotExist:
-            pass
-        return super(InventoryUpdateDetail, self).destroy(request, *args, **kwargs)
-
 
 class InventoryUpdateCancel(RetrieveAPIView):
 
@@ -3581,7 +3582,7 @@ class WorkflowJobList(WorkflowsEnforcementMixin, ListCreateAPIView):
     new_in_310 = True
 
 
-class WorkflowJobDetail(WorkflowsEnforcementMixin, RetrieveDestroyAPIView):
+class WorkflowJobDetail(WorkflowsEnforcementMixin, UnifiedJobDeletionMixin, RetrieveDestroyAPIView):
 
     model = WorkflowJob
     serializer_class = WorkflowJobSerializer
@@ -3739,7 +3740,7 @@ class JobList(ListCreateAPIView):
         return methods
 
 
-class JobDetail(RetrieveUpdateDestroyAPIView):
+class JobDetail(UnifiedJobDeletionMixin, RetrieveUpdateDestroyAPIView):
 
     model = Job
     metadata_class = JobTypeMetadata
@@ -3752,15 +3753,6 @@ class JobDetail(RetrieveUpdateDestroyAPIView):
             return self.http_method_not_allowed(request, *args, **kwargs)
         return super(JobDetail, self).update(request, *args, **kwargs)
 
-    def destroy(self, request, *args, **kwargs):
-        obj = self.get_object()
-        try:
-            if obj.unified_job_node.workflow_job.status in ACTIVE_STATES:
-                raise PermissionDenied(detail=_('Cannot delete job resource when associated workflow job is running.'))
-        except Job.unified_job_node.RelatedObjectDoesNotExist:
-            pass
-        return super(JobDetail, self).destroy(request, *args, **kwargs)
-
 
 class JobExtraCredentialsList(SubListAPIView):
 
@@ -4075,7 +4067,7 @@ class HostAdHocCommandsList(AdHocCommandList, SubListCreateAPIView):
     relationship = 'ad_hoc_commands'
 
 
-class AdHocCommandDetail(RetrieveDestroyAPIView):
+class AdHocCommandDetail(UnifiedJobDeletionMixin, RetrieveDestroyAPIView):
 
     model = AdHocCommand
     serializer_class = AdHocCommandSerializer
@@ -4226,7 +4218,7 @@ class SystemJobList(ListCreateAPIView):
         return super(SystemJobList, self).get(request, *args, **kwargs)
 
 
-class SystemJobDetail(RetrieveDestroyAPIView):
+class SystemJobDetail(UnifiedJobDeletionMixin, RetrieveDestroyAPIView):
 
     model = SystemJob
     serializer_class = SystemJobSerializer

awx/main/tests/functional/api/test_unified_jobs_view.py

@@ -1,8 +1,9 @@
 import pytest
 
 from awx.api.versioning import reverse
-from awx.main.models import UnifiedJob, ProjectUpdate
+from awx.main.models import UnifiedJob, ProjectUpdate, InventoryUpdate
 from awx.main.tests.base import URI
+from awx.main.models.unified_jobs import ACTIVE_STATES
 
 
 TEST_STDOUTS = []
@@ -90,3 +91,52 @@ def test_options_fields_choices(instance, options, user):
     assert UnifiedJob.LAUNCH_TYPE_CHOICES == response.data['actions']['GET']['launch_type']['choices']
     assert 'choice' == response.data['actions']['GET']['status']['type']
     assert UnifiedJob.STATUS_CHOICES == response.data['actions']['GET']['status']['choices']
+
+
+@pytest.mark.parametrize("status", list(ACTIVE_STATES))
+@pytest.mark.django_db
+def test_delete_job_in_active_state(job_factory, delete, admin, status):
+    j = job_factory(initial_state=status)
+    url = reverse('api:job_detail', kwargs={'pk': j.pk})
+    delete(url, None, admin, expect=403)
+
+
+@pytest.mark.parametrize("status", list(ACTIVE_STATES))
+@pytest.mark.django_db
+def test_delete_project_update_in_active_state(project, delete, admin, status):
+    p = ProjectUpdate(project=project, status=status)
+    p.save()
+    url = reverse('api:project_update_detail', kwargs={'pk': p.pk})
+    delete(url, None, admin, expect=403)
+
+
+@pytest.mark.parametrize("status", list(ACTIVE_STATES))
+@pytest.mark.django_db
+def test_delete_inventory_update_in_active_state(inventory_source, delete, admin, status):
+    i = InventoryUpdate.objects.create(inventory_source=inventory_source, status=status)
+    url = reverse('api:inventory_update_detail', kwargs={'pk': i.pk})
+    delete(url, None, admin, expect=403)
+
+
+@pytest.mark.parametrize("status", list(ACTIVE_STATES))
+@pytest.mark.django_db
+def test_delete_workflow_job_in_active_state(workflow_job_factory, delete, admin, status):
+    wj = workflow_job_factory(initial_state=status)
+    url = reverse('api:workflow_job_detail', kwargs={'pk': wj.pk})
+    delete(url, None, admin, expect=403)
+
+
+@pytest.mark.parametrize("status", list(ACTIVE_STATES))
+@pytest.mark.django_db
+def test_delete_system_job_in_active_state(system_job_factory, delete, admin, status):
+    sys_j = system_job_factory(initial_state=status)
+    url = reverse('api:system_job_detail', kwargs={'pk': sys_j.pk})
+    delete(url, None, admin, expect=403)
+
+
+@pytest.mark.parametrize("status", list(ACTIVE_STATES))
+@pytest.mark.django_db
+def test_delete_ad_hoc_command_in_active_state(ad_hoc_command_factory, delete, admin, status):
+    adhoc = ad_hoc_command_factory(initial_state=status)
+    url = reverse('api:ad_hoc_command_detail', kwargs={'pk': adhoc.pk})
+    delete(url, None, admin, expect=403)

awx/main/tests/functional/conftest.py

@@ -28,7 +28,7 @@ from rest_framework.test import (
 )
 
 from awx.main.models.credential import CredentialType, Credential
-from awx.main.models.jobs import JobTemplate
+from awx.main.models.jobs import JobTemplate, SystemJobTemplate
 from awx.main.models.inventory import (
     Group,
     Inventory,
@@ -44,6 +44,8 @@ from awx.main.models.notifications import (
     NotificationTemplate,
     Notification
 )
+from awx.main.models.workflow import WorkflowJobTemplate
+from awx.main.models.ad_hoc_commands import AdHocCommand
 
 
 @pytest.fixture(autouse=True)
@@ -612,6 +614,18 @@ def fact_services_json():
     return _fact_json('services')
 
 
+@pytest.fixture
+def ad_hoc_command_factory(inventory, machine_credential, admin):
+    def factory(inventory=inventory, credential=machine_credential, initial_state='new', created_by=admin):
+        adhoc = AdHocCommand(
+            name='test-adhoc', inventory=inventory, credential=credential,
+            status=initial_state, created_by=created_by
+        )
+        adhoc.save()
+        return adhoc
+    return factory
+
+
 @pytest.fixture
 def job_template(organization):
     jt = JobTemplate(name='test-job_template')
@@ -628,6 +642,35 @@ def job_template_labels(organization, job_template):
     return job_template
 
 
+@pytest.fixture
+def workflow_job_template(organization):
+    wjt = WorkflowJobTemplate(name='test-workflow_job_template')
+    wjt.save()
+
+    return wjt
+
+
+@pytest.fixture
+def workflow_job_factory(workflow_job_template, admin):
+    def factory(workflow_job_template=workflow_job_template, initial_state='new', created_by=admin):
+        return workflow_job_template.create_unified_job(created_by=created_by, status=initial_state)
+    return factory
+
+
+@pytest.fixture
+def system_job_template():
+    sys_jt = SystemJobTemplate(name='test-system_job_template', job_type='cleanup_jobs')
+    sys_jt.save()
+    return sys_jt
+
+
+@pytest.fixture
+def system_job_factory(system_job_template, admin):
+    def factory(system_job_template=system_job_template, initial_state='new', created_by=admin):
+        return system_job_template.create_unified_job(created_by=created_by, status=initial_state)
+    return factory
+
+
 def dumps(value):
     return DjangoJSONEncoder().encode(value)