diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 3db78eb82c..d9c15eb458 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -1524,7 +1524,8 @@ class InventorySourceSerializer(UnifiedJobTemplateSerializer, InventorySourceOpt class Meta: model = InventorySource - fields = ('*', 'name', 'inventory', 'update_on_launch', 'update_cache_timeout', 'scm_project') + \ + fields = ('*', 'name', 'inventory', 'update_on_launch', 'update_cache_timeout', + 'scm_project', 'update_on_project_update') + \ ('last_update_failed', 'last_updated', 'group') # Backwards compatibility. def get_related(self, obj): @@ -1596,13 +1597,21 @@ class InventorySourceSerializer(UnifiedJobTemplateSerializer, InventorySourceOpt return ret def validate(self, attrs): - # source_path = attrs.get('source_path', self.instance and self.instance.source_path) + def get_field_from_model_or_attrs(fd): + return attrs.get(fd, self.instance and getattr(self.instance, fd) or None) + update_on_launch = attrs.get('update_on_launch', self.instance and self.instance.update_on_launch) - scm_project = attrs.get('scm_project', self.instance and self.instance.scm_project) - if attrs.get('source_path', None) and not scm_project: + update_on_project_update = get_field_from_model_or_attrs('update_on_project_update') + source = get_field_from_model_or_attrs('source') + + if attrs.get('source_path', None) and source!='scm': raise serializers.ValidationError({"detail": _("Cannot set source_path if not SCM type.")}) - elif update_on_launch and scm_project: + elif update_on_launch and source=='scm' and update_on_project_update: raise serializers.ValidationError({"detail": _("Cannot update SCM-based inventory source on launch.")}) + elif not self.instance and attrs.get('inventory', None) and InventorySource.objects.filter( + inventory=attrs.get('inventory', None), update_on_project_update=True, source='scm').exists(): + raise serializers.ValidationError({"detail": _("Inventory controlled by project-following SCM.")}) + return super(InventorySourceSerializer, self).validate(attrs) diff --git a/awx/api/views.py b/awx/api/views.py index 48f2e43632..40ddefc472 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -38,7 +38,7 @@ from django.utils.translation import ugettext_lazy as _ # Django REST Framework from rest_framework.exceptions import PermissionDenied, ParseError from rest_framework.parsers import FormParser -from rest_framework.permissions import AllowAny, IsAuthenticated +from rest_framework.permissions import AllowAny, IsAuthenticated, SAFE_METHODS from rest_framework.response import Response from rest_framework.settings import api_settings from rest_framework.views import exception_handler @@ -1681,7 +1681,38 @@ class InventoryList(ListCreateAPIView): return qs -class InventoryDetail(RetrieveUpdateDestroyAPIView): +class ControlledByScmMixin(object): + ''' + Special method to lock-down items managed by SCM inventory source via + project update, which are not protected by the task manager. + ''' + + def _raise_if_unallowed(self, obj): + if (self.request.method not in SAFE_METHODS and obj and + obj.inventory_sources.filter( + update_on_project_update=True, source='scm').exists()): + # Allow inventory changes unrelated to variables + if self.model == Inventory and ( + not self.request or not self.request.data or + parse_yaml_or_json(self.request.data.get('variables', '')) == parse_yaml_or_json(obj.variables)): + return + raise PermissionDenied(detail=_( + 'This object is managed by updates to the project ' + 'of its inventory source. Remove the inventory source ' + 'in order to make this edit.')) + + def get_object(self): + obj = super(ControlledByScmMixin, self).get_object() + self._raise_if_unallowed(obj) + return obj + + def get_parent_object(self): + obj = super(ControlledByScmMixin, self).get_parent_object() + self._raise_if_unallowed(obj) + return obj + + +class InventoryDetail(ControlledByScmMixin, RetrieveUpdateDestroyAPIView): model = Inventory serializer_class = InventoryDetailSerializer @@ -1788,7 +1819,7 @@ class HostList(ListCreateAPIView): return Response(dict(error=_(unicode(e))), status=status.HTTP_400_BAD_REQUEST) -class HostDetail(RetrieveUpdateDestroyAPIView): +class HostDetail(ControlledByScmMixin, RetrieveUpdateDestroyAPIView): always_allow_superuser = False model = Host @@ -1812,7 +1843,7 @@ class InventoryHostsList(SubListCreateAttachDetachAPIView): parent_key = 'inventory' -class HostGroupsList(SubListCreateAttachDetachAPIView): +class HostGroupsList(ControlledByScmMixin, SubListCreateAttachDetachAPIView): ''' the list of groups a host is directly a member of ''' model = Group @@ -1956,7 +1987,7 @@ class EnforceParentRelationshipMixin(object): return super(EnforceParentRelationshipMixin, self).create(request, *args, **kwargs) -class GroupChildrenList(EnforceParentRelationshipMixin, SubListCreateAttachDetachAPIView): +class GroupChildrenList(ControlledByScmMixin, EnforceParentRelationshipMixin, SubListCreateAttachDetachAPIView): model = Group serializer_class = GroupSerializer @@ -1993,7 +2024,7 @@ class GroupPotentialChildrenList(SubListAPIView): return qs.exclude(pk__in=except_pks) -class GroupHostsList(SubListCreateAttachDetachAPIView): +class GroupHostsList(ControlledByScmMixin, SubListCreateAttachDetachAPIView): ''' the list of hosts directly below a group ''' model = Host @@ -2059,7 +2090,7 @@ class GroupActivityStreamList(ActivityStreamEnforcementMixin, SubListAPIView): return qs.filter(Q(group=parent) | Q(host__in=parent.hosts.all())) -class GroupDetail(RetrieveUpdateDestroyAPIView): +class GroupDetail(ControlledByScmMixin, RetrieveUpdateDestroyAPIView): model = Group serializer_class = GroupSerializer @@ -2361,19 +2392,23 @@ class InventorySourceUpdateView(RetrieveAPIView): is_job_start = True new_in_14 = True + def _build_update_response(self, update, request): + if not update: + return Response({}, status=status.HTTP_400_BAD_REQUEST) + else: + headers = {'Location': update.get_absolute_url(request=request)} + return Response(dict(inventory_update=update.id), + status=status.HTTP_202_ACCEPTED, headers=headers) + def post(self, request, *args, **kwargs): obj = self.get_object() - if obj.source == 'scm': - raise PermissionDenied(detail=_( - 'Update the project `{}` in order to update this inventory source.'.format( - obj.scm_project.name))) if obj.can_update: - inventory_update = obj.update() - if not inventory_update: - return Response({}, status=status.HTTP_400_BAD_REQUEST) - else: - headers = {'Location': inventory_update.get_absolute_url(request=request)} - return Response(dict(inventory_update=inventory_update.id), status=status.HTTP_202_ACCEPTED, headers=headers) + if obj.source == 'scm' and obj.update_on_project_update: + if not self.request.user or self.request.user.can_access(self.model, 'update', obj): + raise PermissionDenied(detail=_( + 'You do not have permission to update project `{}`.'.format(obj.scm_project.name))) + return self._build_update_response(obj.scm_project.update(), request) + return self._build_update_response(obj.update(), request) else: return self.http_method_not_allowed(request, *args, **kwargs) diff --git a/awx/main/migrations/0038_v320_release.py b/awx/main/migrations/0038_v320_release.py index 9cf9afe692..5949be3653 100644 --- a/awx/main/migrations/0038_v320_release.py +++ b/awx/main/migrations/0038_v320_release.py @@ -119,6 +119,11 @@ class Migration(migrations.Migration): name='launch_type', field=models.CharField(default=b'manual', max_length=20, editable=False, choices=[(b'manual', 'Manual'), (b'relaunch', 'Relaunch'), (b'callback', 'Callback'), (b'scheduled', 'Scheduled'), (b'dependency', 'Dependency'), (b'workflow', 'Workflow'), (b'sync', 'Sync'), (b'scm', 'SCM Update')]), ), + migrations.AddField( + model_name='inventorysource', + name='update_on_project_update', + field=models.BooleanField(default=False), + ), # Named URL migrations.AlterField( diff --git a/awx/main/models/inventory.py b/awx/main/models/inventory.py index 6389a946a1..d6ed975545 100644 --- a/awx/main/models/inventory.py +++ b/awx/main/models/inventory.py @@ -1137,6 +1137,9 @@ class InventorySource(UnifiedJobTemplate, InventorySourceOptions): default='', editable=False, ) + update_on_project_update = models.BooleanField( + default=False, + ) update_on_launch = models.BooleanField( default=False, ) @@ -1159,7 +1162,6 @@ class InventorySource(UnifiedJobTemplate, InventorySourceOptions): # if it hasn't been specified, then we're just doing a normal save. update_fields = kwargs.get('update_fields', []) is_new_instance = not bool(self.pk) - is_scm_type = self.scm_project_id is not None and self.source == 'scm' # Set name automatically. Include PK (or placeholder) to make sure the names are always unique. replace_text = '__replace_%s__' % now() @@ -1176,7 +1178,7 @@ class InventorySource(UnifiedJobTemplate, InventorySourceOptions): if 'name' not in update_fields: update_fields.append('name') # Reset revision if SCM source has changed parameters - if is_scm_type and not is_new_instance: + if self.source=='scm' and not is_new_instance: before_is = self.__class__.objects.get(pk=self.pk) if before_is.source_path != self.source_path or before_is.scm_project_id != self.scm_project_id: # Reset the scm_revision if file changed to force update @@ -1191,9 +1193,9 @@ class InventorySource(UnifiedJobTemplate, InventorySourceOptions): if replace_text in self.name: self.name = self.name.replace(replace_text, str(self.pk)) super(InventorySource, self).save(update_fields=['name']) - if is_scm_type and is_new_instance: + if self.source=='scm' and is_new_instance and self.update_on_project_update: # Schedule a new Project update if one is not already queued - if not self.scm_project.project_updates.filter( + if self.scm_project and not self.scm_project.project_updates.filter( status__in=['new', 'pending', 'waiting']).exists(): self.scm_project.update() if not getattr(_inventory_updates, 'is_updating', False): @@ -1218,6 +1220,8 @@ class InventorySource(UnifiedJobTemplate, InventorySourceOptions): def _can_update(self): if self.source == 'custom': return bool(self.source_script) + elif self.source == 'scm': + return bool(self.scm_project) else: return bool(self.source in CLOUD_INVENTORY_SOURCES) @@ -1351,16 +1355,18 @@ class InventoryUpdate(UnifiedJob, InventorySourceOptions, JobNotificationMixin): return 50 # InventoryUpdate credential required - # Custom InventoryUpdate credential not required + # Custom and SCM InventoryUpdate credential not required @property def can_start(self): if not super(InventoryUpdate, self).can_start: return False - if (self.source not in ('custom', 'ec2') and + if (self.source not in ('custom', 'ec2', 'scm') and not (self.credential)): return False - elif self.source in ('file', 'scm'): + elif self.source == 'scm' and not self.inventory_source.scm_project: + return False + elif self.source == 'file': return False return True diff --git a/awx/main/tasks.py b/awx/main/tasks.py index fd8bfd078d..893220b3ea 100644 --- a/awx/main/tasks.py +++ b/awx/main/tasks.py @@ -1350,8 +1350,12 @@ class RunProjectUpdate(BaseTask): return OutputEventFilter(stdout_handle, raw_callback=raw_callback) def _update_dependent_inventories(self, project_update, dependent_inventory_sources): + project_request_id = '' if self.request.id is None else self.request.id + scm_revision = project_update.project.scm_revision for inv_src in dependent_inventory_sources: - if inv_src.scm_last_revision == project_update.project.scm_revision: + if not inv_src.update_on_project_update: + continue + if inv_src.scm_last_revision == scm_revision: logger.debug('Skipping SCM inventory update for `{}` because ' 'project has not changed.'.format(inv_src.name)) continue @@ -1362,7 +1366,6 @@ class RunProjectUpdate(BaseTask): logger.info('Skipping SCM inventory update for `{}` because ' 'another update is already active.'.format(inv.name)) continue - project_request_id = '' if self.request.id is None else self.request.id local_inv_update = inv_src.create_inventory_update( scm_project_update_id=project_update.id, launch_type='scm', @@ -1375,12 +1378,11 @@ class RunProjectUpdate(BaseTask): # Runs in the same Celery task as project update task_instance.request.id = project_request_id task_instance.run(local_inv_update.id) - inv_src.scm_last_revision = project_update.project.scm_revision - inv_src.save(update_fields=['scm_last_revision']) except Exception as e: # A failed file update does not block other actions logger.error('Encountered error updating project dependent inventory: {}'.format(e)) - continue + inv_src.scm_last_revision = scm_revision + inv_src.save(update_fields=['scm_last_revision']) def release_lock(self, instance): try: @@ -1797,9 +1799,39 @@ class RunInventoryUpdate(BaseTask): def get_idle_timeout(self): return getattr(settings, 'INVENTORY_UPDATE_IDLE_TIMEOUT', None) - def pre_run_hook(self, instance, **kwargs): + def pre_run_hook(self, inventory_update, **kwargs): self.custom_dir_path = [] + scm_project = None + if inventory_update.inventory_source: + scm_project = inventory_update.inventory_source.scm_project + if (inventory_update.source=='scm' and scm_project and + not inventory_update.inventory_source.update_on_project_update): + request_id = '' if self.request.id is None else self.request.id + local_project_sync = scm_project.create_project_update( + launch_type="sync", + _eager_params=dict( + job_type='run', + status='running', + celery_task_id=request_id)) + # associate the inventory update before calling run() so that a + # cancel() call on the inventory update can cancel the project update + local_project_sync.scm_inventory_updates.add(inventory_update) + + project_update_task = local_project_sync._get_task_class() + try: + task_instance = project_update_task() + task_instance.request.id = request_id + task_instance.run(local_project_sync.id) + inventory_update.inventory_source.scm_last_revision = local_project_sync.project.scm_revision + inventory_update.inventory_source.save(update_fields=['scm_last_revision']) + except Exception: + inventory_update = self.update_model( + inventory_update.pk, status='failed', + job_explanation=('Previous Task Failed: {"job_type": "%s", "job_name": "%s", "job_id": "%s"}' % + ('project_update', local_project_sync.name, local_project_sync.id))) + raise + def post_run_hook(self, instance, status, **kwargs): print("In post run hook") if self.custom_dir_path: diff --git a/awx/main/tests/functional/api/test_inventory.py b/awx/main/tests/functional/api/test_inventory.py index 27b929f372..03d5dc78b4 100644 --- a/awx/main/tests/functional/api/test_inventory.py +++ b/awx/main/tests/functional/api/test_inventory.py @@ -197,3 +197,62 @@ def test_inventory_source_update(post, inventory_source, alice, role_field, expe if role_field: getattr(inventory_source.inventory, role_field).members.add(alice) post(reverse('api:inventory_source_update_view', kwargs={'pk': inventory_source.id}), {}, alice, expect=expected_status_code) + + +@pytest.fixture +def scm_inventory(inventory, project): + inventory.inventory_sources.create( + name='foobar', update_on_project_update=True, source='scm', + scm_project=project) + return inventory + + +@pytest.mark.django_db +class TestControlledBySCM: + ''' + Check that various actions are correctly blocked if object is controlled + by an SCM follow-project inventory source + ''' + def test_safe_method_works(self, get, options, scm_inventory, admin_user): + get(scm_inventory.get_absolute_url(), admin_user, expect=200) + options(scm_inventory.get_absolute_url(), admin_user, expect=200) + + def test_vars_edit_prohibited(self, patch, scm_inventory, admin_user): + patch(scm_inventory.get_absolute_url(), {'variables': 'hello: world'}, + admin_user, expect=403) + + def test_name_edit_allowed(self, patch, scm_inventory, admin_user): + patch(scm_inventory.get_absolute_url(), {'variables': '---', 'name': 'newname'}, + admin_user, expect=200) + + def test_host_associations_prohibited(self, post, scm_inventory, admin_user): + inv_src = scm_inventory.inventory_sources.first() + h = inv_src.hosts.create(name='barfoo', inventory=scm_inventory) + g = inv_src.groups.create(name='fooland', inventory=scm_inventory) + post(reverse('api:host_groups_list', kwargs={'pk': h.id}), {'id': g.id}, + admin_user, expect=403) + post(reverse('api:group_hosts_list', kwargs={'pk': g.id}), {'id': h.id}, + admin_user, expect=403) + + def test_group_group_associations_prohibited(self, post, scm_inventory, admin_user): + inv_src = scm_inventory.inventory_sources.first() + g1 = inv_src.groups.create(name='barland', inventory=scm_inventory) + g2 = inv_src.groups.create(name='fooland', inventory=scm_inventory) + post(reverse('api:group_children_list', kwargs={'pk': g1.id}), {'id': g2.id}, + admin_user, expect=403) + + def test_host_group_delete_prohibited(self, delete, scm_inventory, admin_user): + inv_src = scm_inventory.inventory_sources.first() + h = inv_src.hosts.create(name='barfoo', inventory=scm_inventory) + g = inv_src.groups.create(name='fooland', inventory=scm_inventory) + delete(h.get_absolute_url(), admin_user, expect=403) + delete(g.get_absolute_url(), admin_user, expect=403) + + def test_remove_scm_inv_src(self, delete, scm_inventory, admin_user): + inv_src = scm_inventory.inventory_sources.first() + delete(inv_src.get_absolute_url(), admin_user, expect=204) + assert scm_inventory.inventory_sources.count() == 0 + + def test_adding_inv_src_prohibited(self, post, scm_inventory, admin_user): + post(reverse('api:inventory_inventory_sources_list', kwargs={'pk': scm_inventory.id}), + {'name': 'new inv src'}, admin_user, expect=400) diff --git a/awx/main/tests/functional/conftest.py b/awx/main/tests/functional/conftest.py index 3e7248f4eb..f4605e0398 100644 --- a/awx/main/tests/functional/conftest.py +++ b/awx/main/tests/functional/conftest.py @@ -240,7 +240,9 @@ def scm_inventory_source(inventory, project): return InventorySource.objects.create( name="test-scm-inv", scm_project=project, + source='scm', source_path='inventory_file', + update_on_project_update=True, inventory=inventory) diff --git a/awx/main/tests/functional/models/test_inventory.py b/awx/main/tests/functional/models/test_inventory.py index 04aa7c556c..861ff7c607 100644 --- a/awx/main/tests/functional/models/test_inventory.py +++ b/awx/main/tests/functional/models/test_inventory.py @@ -17,6 +17,7 @@ class TestSCMUpdateFeatures: scm_project=project, source_path='inventory_file', inventory=inventory, + update_on_project_update=True, source='scm') with mock.patch.object(inv_src.scm_project, 'update') as mck_update: inv_src.save()