From 497aa13562b56288a0ad56ff37ce86f08548abc7 Mon Sep 17 00:00:00 2001 From: Aaron Tan Date: Wed, 9 Aug 2017 16:30:10 -0400 Subject: [PATCH] Prevent deleting unified jobs with active status --- awx/api/views.py | 58 ++++++++----------- .../functional/api/test_unified_jobs_view.py | 52 ++++++++++++++++- awx/main/tests/functional/conftest.py | 45 +++++++++++++- 3 files changed, 120 insertions(+), 35 deletions(-) diff --git a/awx/api/views.py b/awx/api/views.py index a37827b2cb..d116862877 100644 --- a/awx/api/views.py +++ b/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): @@ -3580,7 +3581,7 @@ class WorkflowJobList(WorkflowsEnforcementMixin, ListCreateAPIView): new_in_310 = True -class WorkflowJobDetail(WorkflowsEnforcementMixin, RetrieveDestroyAPIView): +class WorkflowJobDetail(WorkflowsEnforcementMixin, UnifiedJobDeletionMixin, RetrieveDestroyAPIView): model = WorkflowJob serializer_class = WorkflowJobSerializer @@ -3738,7 +3739,7 @@ class JobList(ListCreateAPIView): return methods -class JobDetail(RetrieveUpdateDestroyAPIView): +class JobDetail(UnifiedJobDeletionMixin, RetrieveUpdateDestroyAPIView): model = Job metadata_class = JobTypeMetadata @@ -3751,15 +3752,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): @@ -4074,7 +4066,7 @@ class HostAdHocCommandsList(AdHocCommandList, SubListCreateAPIView): relationship = 'ad_hoc_commands' -class AdHocCommandDetail(RetrieveDestroyAPIView): +class AdHocCommandDetail(UnifiedJobDeletionMixin, RetrieveDestroyAPIView): model = AdHocCommand serializer_class = AdHocCommandSerializer @@ -4225,7 +4217,7 @@ class SystemJobList(ListCreateAPIView): return super(SystemJobList, self).get(request, *args, **kwargs) -class SystemJobDetail(RetrieveDestroyAPIView): +class SystemJobDetail(UnifiedJobDeletionMixin, RetrieveDestroyAPIView): model = SystemJob serializer_class = SystemJobSerializer diff --git a/awx/main/tests/functional/api/test_unified_jobs_view.py b/awx/main/tests/functional/api/test_unified_jobs_view.py index 842f195c85..e38ef51669 100644 --- a/awx/main/tests/functional/api/test_unified_jobs_view.py +++ b/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) diff --git a/awx/main/tests/functional/conftest.py b/awx/main/tests/functional/conftest.py index 67ea92ae62..456d1a409e 100644 --- a/awx/main/tests/functional/conftest.py +++ b/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)