From 7881c921acb07090f3da8ea5bbda4f4aaefccba2 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Thu, 15 Mar 2018 15:56:52 -0400 Subject: [PATCH 1/3] block deletion of resources w unprocessed events --- awx/api/views.py | 7 +++ awx/main/models/ha.py | 10 ++-- awx/main/models/inventory.py | 22 ++++----- awx/main/models/jobs.py | 5 +- awx/main/models/mixins.py | 5 +- awx/main/models/organization.py | 16 ++----- awx/main/models/projects.py | 10 ++-- awx/main/models/workflow.py | 5 +- awx/main/tests/functional/api/test_job.py | 56 ++++++++++++++++++++++- 9 files changed, 89 insertions(+), 47 deletions(-) diff --git a/awx/api/views.py b/awx/api/views.py index a1cad8a469..2318285796 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -206,6 +206,13 @@ class RelatedJobsPreventDeleteMixin(object): active_jobs = obj.get_active_jobs() if len(active_jobs) > 0: raise ActiveJobConflict(active_jobs) + time_cutoff = now() - dateutil.relativedelta.relativedelta(minutes=1) + recent_jobs = obj._get_related_jobs().filter(finished__gte = time_cutoff) + for unified_job in recent_jobs.get_real_instances(): + if not unified_job.event_processing_finished: + raise PermissionDenied(_( + 'Related job {} is still processing events.' + ).format(unified_job.log_format)) return super(RelatedJobsPreventDeleteMixin, self).perform_destroy(obj) diff --git a/awx/main/models/ha.py b/awx/main/models/ha.py index 9c7cdc5e2d..3356dc3a5d 100644 --- a/awx/main/models/ha.py +++ b/awx/main/models/ha.py @@ -19,10 +19,7 @@ from awx.main.fields import JSONField from awx.main.models.inventory import InventoryUpdate from awx.main.models.jobs import Job from awx.main.models.projects import ProjectUpdate -from awx.main.models.unified_jobs import ( - UnifiedJob, - ACTIVE_STATES -) +from awx.main.models.unified_jobs import UnifiedJob from awx.main.utils import get_cpu_capacity, get_mem_capacity, get_system_task_capacity from awx.main.models.mixins import RelatedJobsMixin @@ -159,9 +156,8 @@ class InstanceGroup(models.Model, RelatedJobsMixin): ''' RelatedJobsMixin ''' - def _get_active_jobs(self): - return UnifiedJob.objects.filter(instance_group=self, - status__in=ACTIVE_STATES) + def _get_related_jobs(self): + return UnifiedJob.objects.filter(instance_group=self) class Meta: diff --git a/awx/main/models/inventory.py b/awx/main/models/inventory.py index 0a2c254571..28337e719e 100644 --- a/awx/main/models/inventory.py +++ b/awx/main/models/inventory.py @@ -33,7 +33,6 @@ from awx.main.managers import HostManager from awx.main.models.base import * # noqa from awx.main.models.events import InventoryUpdateEvent from awx.main.models.unified_jobs import * # noqa -from awx.main.models.unified_jobs import ACTIVE_STATES from awx.main.models.mixins import ( ResourceMixin, TaskManagerInventoryUpdateMixin, @@ -497,14 +496,11 @@ class Inventory(CommonModelNameNotUnique, ResourceMixin, RelatedJobsMixin): ''' RelatedJobsMixin ''' - def _get_active_jobs(self): + def _get_related_jobs(self): return UnifiedJob.objects.non_polymorphic().filter( - Q(status__in=ACTIVE_STATES) & - ( - Q(Job___inventory=self) | - Q(InventoryUpdate___inventory_source__inventory=self) | - Q(AdHocCommand___inventory=self) - ) + Q(Job___inventory=self) | + Q(InventoryUpdate___inventory_source__inventory=self) | + Q(AdHocCommand___inventory=self) ) @@ -963,9 +959,8 @@ class Group(CommonModelNameNotUnique, RelatedJobsMixin): ''' RelatedJobsMixin ''' - def _get_active_jobs(self): - return InventoryUpdate.objects.filter(status__in=ACTIVE_STATES, - inventory_source__in=self.inventory_sources.all()) + def _get_related_jobs(self): + return InventoryUpdate.objects.filter(inventory_source__in=self.inventory_sources.all()) class InventorySourceOptions(BaseModel): @@ -1583,9 +1578,8 @@ class InventorySource(UnifiedJobTemplate, InventorySourceOptions, RelatedJobsMix ''' RelatedJobsMixin ''' - def _get_active_jobs(self): - return InventoryUpdate.objects.filter(status__in=ACTIVE_STATES, - inventory_source=self) + def _get_related_jobs(self): + return InventoryUpdate.objects.filter(inventory_source=self) class InventoryUpdate(UnifiedJob, InventorySourceOptions, JobNotificationMixin, TaskManagerInventoryUpdateMixin): diff --git a/awx/main/models/jobs.py b/awx/main/models/jobs.py index f1f7f9e28d..f0583d10eb 100644 --- a/awx/main/models/jobs.py +++ b/awx/main/models/jobs.py @@ -29,7 +29,6 @@ from awx.api.versioning import reverse from awx.main.models.base import * # noqa from awx.main.models.events import JobEvent, SystemJobEvent from awx.main.models.unified_jobs import * # noqa -from awx.main.models.unified_jobs import ACTIVE_STATES from awx.main.models.notifications import ( NotificationTemplate, JobNotificationMixin, @@ -455,8 +454,8 @@ class JobTemplate(UnifiedJobTemplate, JobOptions, SurveyJobTemplateMixin, Resour ''' RelatedJobsMixin ''' - def _get_active_jobs(self): - return Job.objects.filter(status__in=ACTIVE_STATES, job_template=self) + def _get_related_jobs(self): + return Job.objects.filter(job_template=self) class Job(UnifiedJob, JobOptions, SurveyJobMixin, JobNotificationMixin, TaskManagerJobMixin): diff --git a/awx/main/models/mixins.py b/awx/main/models/mixins.py index 338877874c..acd35ba017 100644 --- a/awx/main/models/mixins.py +++ b/awx/main/models/mixins.py @@ -457,8 +457,11 @@ class RelatedJobsMixin(object): Returns a list of active jobs (i.e. running) associated with the calling resource (self). Expected to return a QuerySet ''' + def _get_related_jobs(self): + return self.objects.none() + def _get_active_jobs(self): - return [] + return self._get_related_jobs().filter(status__in=('new', 'pending', 'waiting', 'running')) ''' Returns [{'id': '1', 'type': 'job'}, {'id': 2, 'type': 'project_update'}, ...] diff --git a/awx/main/models/organization.py b/awx/main/models/organization.py index 14c50e2366..db406fd2ed 100644 --- a/awx/main/models/organization.py +++ b/awx/main/models/organization.py @@ -20,10 +20,7 @@ from awx.main.models.rbac import ( ROLE_SINGLETON_SYSTEM_ADMINISTRATOR, ROLE_SINGLETON_SYSTEM_AUDITOR, ) -from awx.main.models.unified_jobs import ( - UnifiedJob, - ACTIVE_STATES, -) +from awx.main.models.unified_jobs import UnifiedJob from awx.main.models.mixins import ResourceMixin, CustomVirtualEnvMixin, RelatedJobsMixin __all__ = ['Organization', 'Team', 'Profile', 'UserSessionMembership'] @@ -82,15 +79,12 @@ class Organization(CommonModel, NotificationFieldsModel, ResourceMixin, CustomVi ''' RelatedJobsMixin ''' - def _get_active_jobs(self): + def _get_related_jobs(self): project_ids = self.projects.all().values_list('id') return UnifiedJob.objects.non_polymorphic().filter( - Q(status__in=ACTIVE_STATES) & - ( - Q(Job___project__in=project_ids) | - Q(ProjectUpdate___project__in=project_ids) | - Q(InventoryUpdate___inventory_source__inventory__organization=self) - ) + Q(Job___project__in=project_ids) | + Q(ProjectUpdate___project__in=project_ids) | + Q(InventoryUpdate___inventory_source__inventory__organization=self) ) diff --git a/awx/main/models/projects.py b/awx/main/models/projects.py index 667efedfba..981c3432b4 100644 --- a/awx/main/models/projects.py +++ b/awx/main/models/projects.py @@ -454,17 +454,13 @@ class Project(UnifiedJobTemplate, ProjectOptions, ResourceMixin, CustomVirtualEn ''' RelatedJobsMixin ''' - def _get_active_jobs(self): + def _get_related_jobs(self): return UnifiedJob.objects.non_polymorphic().filter( - models.Q(status__in=ACTIVE_STATES) & - ( - models.Q(Job___project=self) | - models.Q(ProjectUpdate___project=self) - ) + models.Q(Job___project=self) | + models.Q(ProjectUpdate___project=self) ) - class ProjectUpdate(UnifiedJob, ProjectOptions, JobNotificationMixin, TaskManagerProjectUpdateMixin): ''' Internal job for tracking project updates from SCM. diff --git a/awx/main/models/workflow.py b/awx/main/models/workflow.py index 59f3bccb74..009efd0aab 100644 --- a/awx/main/models/workflow.py +++ b/awx/main/models/workflow.py @@ -30,7 +30,6 @@ from awx.main.models.mixins import ( SurveyJobMixin, RelatedJobsMixin, ) -from awx.main.models.unified_jobs import ACTIVE_STATES from awx.main.models.jobs import LaunchTimeConfig from awx.main.models.credential import Credential from awx.main.redact import REPLACE_STR @@ -414,8 +413,8 @@ class WorkflowJobTemplate(UnifiedJobTemplate, WorkflowJobOptions, SurveyJobTempl ''' RelatedJobsMixin ''' - def _get_active_jobs(self): - return WorkflowJob.objects.filter(status__in=ACTIVE_STATES, workflow_job_template=self) + def _get_related_jobs(self): + return WorkflowJob.objects.filter(workflow_job_template=self) class WorkflowJob(UnifiedJob, WorkflowJobOptions, SurveyJobMixin, JobNotificationMixin): diff --git a/awx/main/tests/functional/api/test_job.py b/awx/main/tests/functional/api/test_job.py index be87de1870..831c2edc38 100644 --- a/awx/main/tests/functional/api/test_job.py +++ b/awx/main/tests/functional/api/test_job.py @@ -1,8 +1,15 @@ import pytest +import mock + +from dateutil.parser import parse +from dateutil.relativedelta import relativedelta + +from rest_framework.exceptions import PermissionDenied from awx.api.versioning import reverse +from awx.api.views import RelatedJobsPreventDeleteMixin, UnifiedJobDeletionMixin -from awx.main.models import JobTemplate, User +from awx.main.models import JobTemplate, User, Job @pytest.mark.django_db @@ -81,3 +88,50 @@ def test_job_relaunch_on_failed_hosts(post, inventory, project, machine_credenti expect=201 ) assert r.data.get('limit') == hosts + + +@pytest.mark.django_db +def test_block_unprocessed_events(delete, admin_user, mocker): + time_of_finish = parse("Thu Feb 28 09:10:20 2013 -0500") + job = Job.objects.create( + emitted_events=1, + status='finished', + finished=time_of_finish + ) + request = mock.MagicMock() + + class MockView(UnifiedJobDeletionMixin): + model = Job + + def get_object(): + return job + + view = MockView() + + time_of_request = time_of_finish + relativedelta(seconds=2) + with mock.patch('awx.api.views.now', lambda: time_of_request): + with mock.patch.object(view, 'get_object') as get_obj: + get_obj.return_value = job + r = view.destroy(request) + assert r.status_code == 400 + + +@pytest.mark.django_db +def test_block_related_unprocessed_events(mocker, organization, project, delete, admin_user): + job_template = JobTemplate.objects.create( + project=project, + playbook='helloworld.yml' + ) + time_of_finish = parse("Thu Feb 23 14:17:24 2012 -0500") + Job.objects.create( + emitted_events=1, + status='finished', + finished=time_of_finish, + job_template=job_template, + project=project + ) + view = RelatedJobsPreventDeleteMixin() + time_of_request = time_of_finish + relativedelta(seconds=2) + with mock.patch('awx.api.views.now', lambda: time_of_request): + with pytest.raises(PermissionDenied): + view.perform_destroy(organization) From 69eccd31307a4ed2e938666453e7652800b3c5c6 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Fri, 16 Mar 2018 10:31:41 -0400 Subject: [PATCH 2/3] move ACTIVE_STATES to constants --- awx/api/serializers.py | 2 +- awx/api/views.py | 2 +- awx/main/constants.py | 8 ++++++++ awx/main/models/mixins.py | 3 ++- awx/main/models/projects.py | 1 - awx/main/models/unified_jobs.py | 4 ++-- awx/main/tasks.py | 2 +- awx/main/tests/functional/api/test_unified_jobs_view.py | 2 +- 8 files changed, 16 insertions(+), 8 deletions(-) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index fb4431adff..11635dc6d6 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -40,7 +40,7 @@ from polymorphic.models import PolymorphicModel # AWX from awx.main.constants import SCHEDULEABLE_PROVIDERS, ANSI_SGR_PATTERN from awx.main.models import * # noqa -from awx.main.models.unified_jobs import ACTIVE_STATES +from awx.main.constants import ACTIVE_STATES from awx.main.models.base import NEW_JOB_TYPE_CHOICES from awx.main.access import get_user_capabilities from awx.main.fields import ImplicitRoleField diff --git a/awx/api/views.py b/awx/api/views.py index 2318285796..029b7b480f 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -88,7 +88,7 @@ from awx.api.permissions import ( from awx.api.renderers import * # noqa from awx.api.serializers import * # noqa from awx.api.metadata import RoleMetadata, JobTypeMetadata -from awx.main.models.unified_jobs import ACTIVE_STATES +from awx.main.constants import ACTIVE_STATES from awx.main.scheduler.tasks import run_job_complete from awx.api.exceptions import ActiveJobConflict diff --git a/awx/main/constants.py b/awx/main/constants.py index c452e19640..447fed5ae6 100644 --- a/awx/main/constants.py +++ b/awx/main/constants.py @@ -5,9 +5,17 @@ import re from django.utils.translation import ugettext_lazy as _ +__all__ = [ + 'CLOUD_PROVIDERS', 'SCHEDULEABLE_PROVIDERS', 'PRIVILEGE_ESCALATION_METHODS', + 'ANSI_SGR_PATTERN', 'CAN_CANCEL', 'ACTIVE_STATES' +] + + CLOUD_PROVIDERS = ('azure_rm', 'ec2', 'gce', 'vmware', 'openstack', 'rhv', 'satellite6', 'cloudforms', 'tower') SCHEDULEABLE_PROVIDERS = CLOUD_PROVIDERS + ('custom', 'scm',) PRIVILEGE_ESCALATION_METHODS = [ ('sudo', _('Sudo')), ('su', _('Su')), ('pbrun', _('Pbrun')), ('pfexec', _('Pfexec')), ('dzdo', _('DZDO')), ('pmrun', _('Pmrun')), ('runas', _('Runas'))] ANSI_SGR_PATTERN = re.compile(r'\x1b\[[0-9;]*m') +CAN_CANCEL = ('new', 'pending', 'waiting', 'running') +ACTIVE_STATES = CAN_CANCEL diff --git a/awx/main/models/mixins.py b/awx/main/models/mixins.py index acd35ba017..7d563f1e36 100644 --- a/awx/main/models/mixins.py +++ b/awx/main/models/mixins.py @@ -24,6 +24,7 @@ from awx.main.utils import parse_yaml_or_json, get_custom_venv_choices from awx.main.utils.encryption import decrypt_value, get_encryption_key, is_encrypted from awx.main.utils.polymorphic import build_polymorphic_ctypes_map from awx.main.fields import JSONField, AskForField +from awx.main.constants import ACTIVE_STATES __all__ = ['ResourceMixin', 'SurveyJobTemplateMixin', 'SurveyJobMixin', @@ -461,7 +462,7 @@ class RelatedJobsMixin(object): return self.objects.none() def _get_active_jobs(self): - return self._get_related_jobs().filter(status__in=('new', 'pending', 'waiting', 'running')) + return self._get_related_jobs().filter(status__in=ACTIVE_STATES) ''' Returns [{'id': '1', 'type': 'job'}, {'id': 2, 'type': 'project_update'}, ...] diff --git a/awx/main/models/projects.py b/awx/main/models/projects.py index 981c3432b4..d302aa0973 100644 --- a/awx/main/models/projects.py +++ b/awx/main/models/projects.py @@ -28,7 +28,6 @@ from awx.main.models.notifications import ( from awx.main.models.unified_jobs import ( UnifiedJob, UnifiedJobTemplate, - ACTIVE_STATES, ) from awx.main.models.mixins import ( ResourceMixin, diff --git a/awx/main/models/unified_jobs.py b/awx/main/models/unified_jobs.py index fca04dc8b0..e50a686374 100644 --- a/awx/main/models/unified_jobs.py +++ b/awx/main/models/unified_jobs.py @@ -38,6 +38,7 @@ from awx.main.utils import ( copy_model_by_class, copy_m2m_relationships, get_type_for_model, parse_yaml_or_json ) +from awx.main.constants import ACTIVE_STATES, CAN_CANCEL from awx.main.redact import UriCleaner, REPLACE_STR from awx.main.consumers import emit_channel_notification from awx.main.fields import JSONField, AskForField @@ -46,8 +47,7 @@ __all__ = ['UnifiedJobTemplate', 'UnifiedJob', 'StdoutMaxBytesExceeded'] logger = logging.getLogger('awx.main.models.unified_jobs') -CAN_CANCEL = ('new', 'pending', 'waiting', 'running') -ACTIVE_STATES = CAN_CANCEL +# NOTE: ACTIVE_STATES moved to constants because it is used by parent modules class UnifiedJobTemplate(PolymorphicModel, CommonModelNameNotUnique, NotificationFieldsModel): diff --git a/awx/main/tasks.py b/awx/main/tasks.py index fdf316b17b..fc73e4eb41 100644 --- a/awx/main/tasks.py +++ b/awx/main/tasks.py @@ -49,7 +49,7 @@ from crum import impersonate from awx import __version__ as awx_application_version from awx.main.constants import CLOUD_PROVIDERS, PRIVILEGE_ESCALATION_METHODS from awx.main.models import * # noqa -from awx.main.models.unified_jobs import ACTIVE_STATES +from awx.main.constants import ACTIVE_STATES from awx.main.exceptions import AwxTaskError from awx.main.queue import CallbackQueueDispatcher from awx.main.expect import run, isolated_manager 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 c537dd941f..d4dd4bd53a 100644 --- a/awx/main/tests/functional/api/test_unified_jobs_view.py +++ b/awx/main/tests/functional/api/test_unified_jobs_view.py @@ -3,7 +3,7 @@ import pytest from awx.api.versioning import reverse from awx.main.models import UnifiedJob, ProjectUpdate, InventoryUpdate from awx.main.tests.base import URI -from awx.main.models.unified_jobs import ACTIVE_STATES +from awx.main.constants import ACTIVE_STATES TEST_STATES = list(ACTIVE_STATES) From 66108164b968478e102620b856e66643d02904b3 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Fri, 16 Mar 2018 10:55:48 -0400 Subject: [PATCH 3/3] remove unnecessary mock --- awx/main/tests/functional/api/test_job.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/awx/main/tests/functional/api/test_job.py b/awx/main/tests/functional/api/test_job.py index 831c2edc38..db5da93c2f 100644 --- a/awx/main/tests/functional/api/test_job.py +++ b/awx/main/tests/functional/api/test_job.py @@ -103,17 +103,15 @@ def test_block_unprocessed_events(delete, admin_user, mocker): class MockView(UnifiedJobDeletionMixin): model = Job - def get_object(): + def get_object(self): return job view = MockView() time_of_request = time_of_finish + relativedelta(seconds=2) with mock.patch('awx.api.views.now', lambda: time_of_request): - with mock.patch.object(view, 'get_object') as get_obj: - get_obj.return_value = job - r = view.destroy(request) - assert r.status_code == 400 + r = view.destroy(request) + assert r.status_code == 400 @pytest.mark.django_db