Adding hosts bulk deletion feature (#14462)

* Adding hosts bulk deletion feature Signed-off-by: Avi Layani <[email protected]> * fix the type of the argument Signed-off-by: Avi Layani <[email protected]> * fixing activity_entry tracking Signed-off-by: Avi Layani <[email protected]> * Revert "fixing activity_entry tracking" This reverts commit c8eab52. Since the bulk_delete is not related to an inventory, only hosts which can be from different inventories. * get only needed vars to reduce memory consumption Signed-off-by: Avi Layani <[email protected]> * filtering the data to reduce memory increase the number of queries Signed-off-by: Avi Layani <[email protected]> * update the activity stream for inventories Signed-off-by: Avi Layani <[email protected]> * fix the changes dict initialiazation Signed-off-by: Avi Layani <[email protected]> --------- Signed-off-by: Avi Layani <[email protected]>
ansible · Dec 13, 2023 · df24cb6 · df24cb6
1 parent 0d825a7
commit df24cb6
Show file tree

Hide file tree

Showing 15 changed files with 498 additions and 0 deletions.
diff --git a/awx/api/serializers.py b/awx/api/serializers.py
@@ -2201,6 +2201,99 @@ def create(self, validated_data):
         return return_data
 
 
+class BulkHostDeleteSerializer(serializers.Serializer):
+    hosts = serializers.ListField(
+        allow_empty=False,
+        max_length=100000,
+        write_only=True,
+        help_text=_('List of hosts ids to be deleted, e.g. [105, 130, 131, 200]'),
+    )
+
+    class Meta:
+        model = Host
+        fields = ('hosts',)
+
+    def validate(self, attrs):
+        request = self.context.get('request', None)
+        max_hosts = settings.BULK_HOST_MAX_DELETE
+        # Validating the number of hosts to be deleted
+        if len(attrs['hosts']) > max_hosts:
+            raise serializers.ValidationError(
+                {
+                    "ERROR": 'Number of hosts exceeds system setting BULK_HOST_MAX_DELETE',
+                    "BULK_HOST_MAX_DELETE": max_hosts,
+                    "Hosts_count": len(attrs['hosts']),
+                }
+            )
+
+        # Getting list of all host objects, filtered by the list of the hosts to delete
+        attrs['host_qs'] = Host.objects.get_queryset().filter(pk__in=attrs['hosts']).only('id', 'inventory_id', 'name')
+
+        # Converting the queryset data in a dict. to reduce the number of queries when
+        # manipulating the data
+        attrs['hosts_data'] = attrs['host_qs'].values()
+
+        if len(attrs['host_qs']) == 0:
+            error_hosts = {host: "Hosts do not exist or you lack permission to delete it" for host in attrs['hosts']}
+            raise serializers.ValidationError({'hosts': error_hosts})
+
+        if len(attrs['host_qs']) < len(attrs['hosts']):
+            hosts_exists = [host['id'] for host in attrs['hosts_data']]
+            failed_hosts = list(set(attrs['hosts']).difference(hosts_exists))
+            error_hosts = {host: "Hosts do not exist or you lack permission to delete it" for host in failed_hosts}
+            raise serializers.ValidationError({'hosts': error_hosts})
+
+        # Getting all inventories that the hosts can be in
+        inv_list = list(set([host['inventory_id'] for host in attrs['hosts_data']]))
+
+        # Checking that the user have permission to all inventories
+        errors = dict()
+        for inv in Inventory.objects.get_queryset().filter(pk__in=inv_list):
+            if request and not request.user.is_superuser:
+                if request.user not in inv.admin_role:
+                    errors[inv.name] = "Lack permissions to delete hosts from this inventory."
+        if errors != {}:
+            raise PermissionDenied({"inventories": errors})
+
+        # check the inventory type only if the user have permission to it.
+        errors = dict()
+        for inv in Inventory.objects.get_queryset().filter(pk__in=inv_list):
+            if inv.kind != '':
+                errors[inv.name] = "Hosts can only be deleted from manual inventories."
+        if errors != {}:
+            raise serializers.ValidationError({"inventories": errors})
+        attrs['inventories'] = inv_list
+        return attrs
+
+    def delete(self, validated_data):
+        result = {"hosts": dict()}
+        changes = {'deleted_hosts': dict()}
+        for inventory in validated_data['inventories']:
+            changes['deleted_hosts'][inventory] = list()
+
+        for host in validated_data['hosts_data']:
+            result["hosts"][host["id"]] = f"The host {host['name']} was deleted"
+            changes['deleted_hosts'][host["inventory_id"]].append({"host_id": host["id"], "host_name": host["name"]})
+
+        try:
+            validated_data['host_qs'].delete()
+        except Exception as e:
+            raise serializers.ValidationError({"detail": _(f"cannot delete hosts, host deletion error {e}")})
+
+        request = self.context.get('request', None)
+
+        for inventory in validated_data['inventories']:
+            activity_entry = ActivityStream.objects.create(
+                operation='update',
+                object1='inventory',
+                changes=json.dumps(changes['deleted_hosts'][inventory]),
+                actor=request.user,
+            )
+            activity_entry.inventory.add(inventory)
+
+        return result
+
+
 class GroupTreeSerializer(GroupSerializer):
     children = serializers.SerializerMethodField()
 

diff --git a/awx/api/templates/api/bulk_host_delete_view.md b/awx/api/templates/api/bulk_host_delete_view.md
@@ -0,0 +1,22 @@
+# Bulk Host Delete
+
+This endpoint allows the client to delete multiple hosts from inventories.
+They may do this by providing a list of hosts ID's to be deleted.
+
+Example:
+
+    {
+        "hosts": [1, 2, 3, 4, 5]
+    }
+
+Return data:
+
+    {
+        "hosts": {
+            "1": "The host a1 was deleted",
+            "2": "The host a2 was deleted",
+            "3": "The host a3 was deleted",
+            "4": "The host a4 was deleted",
+            "5": "The host a5 was deleted",
+        }
+    }
diff --git a/awx/api/urls/urls.py b/awx/api/urls/urls.py
@@ -36,6 +36,7 @@
 from awx.api.views.bulk import (
     BulkView,
     BulkHostCreateView,
+    BulkHostDeleteView,
     BulkJobLaunchView,
 )
 
@@ -152,6 +153,7 @@
     re_path(r'^workflow_approvals/', include(workflow_approval_urls)),
     re_path(r'^bulk/$', BulkView.as_view(), name='bulk'),
     re_path(r'^bulk/host_create/$', BulkHostCreateView.as_view(), name='bulk_host_create'),
+    re_path(r'^bulk/host_delete/$', BulkHostDeleteView.as_view(), name='bulk_host_delete'),
     re_path(r'^bulk/job_launch/$', BulkJobLaunchView.as_view(), name='bulk_job_launch'),
 ]
 

diff --git a/awx/api/views/bulk.py b/awx/api/views/bulk.py
@@ -34,6 +34,7 @@ def get(self, request, format=None):
         '''List top level resources'''
         data = OrderedDict()
         data['host_create'] = reverse('api:bulk_host_create', request=request)
+        data['host_delete'] = reverse('api:bulk_host_delete', request=request)
         data['job_launch'] = reverse('api:bulk_job_launch', request=request)
         return Response(data)
 
@@ -72,3 +73,20 @@ def post(self, request):
             result = serializer.create(serializer.validated_data)
             return Response(result, status=status.HTTP_201_CREATED)
         return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST)
+
+
+class BulkHostDeleteView(GenericAPIView):
+    permission_classes = [IsAuthenticated]
+    model = Host
+    serializer_class = serializers.BulkHostDeleteSerializer
+    allowed_methods = ['GET', 'POST', 'OPTIONS']
+
+    def get(self, request):
+        return Response({"detail": "Bulk delete hosts with this endpoint"}, status=status.HTTP_200_OK)
+
+    def post(self, request):
+        serializer = serializers.BulkHostDeleteSerializer(data=request.data, context={'request': request})
+        if serializer.is_valid():
+            result = serializer.delete(serializer.validated_data)
+            return Response(result, status=status.HTTP_201_CREATED)
+        return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST)
diff --git a/awx/main/conf.py b/awx/main/conf.py
@@ -827,6 +827,16 @@
     category_slug='bulk',
 )
 
+register(
+    'BULK_HOST_MAX_DELETE',
+    field_class=fields.IntegerField,
+    default=250,
+    label=_('Max number of hosts to allow to be deleted in a single bulk action'),
+    help_text=_('Max number of hosts to allow to be deleted in a single bulk action'),
+    category=_('Bulk Actions'),
+    category_slug='bulk',
+)
+
 register(
     'UI_NEXT',
     field_class=fields.BooleanField,

diff --git a/awx/main/tests/functional/test_bulk.py b/awx/main/tests/functional/test_bulk.py
@@ -309,3 +309,139 @@ def test_bulk_job_set_all_prompt(job_template, organization, inventory, project,
     assert node[0].limit == 'kansas'
     assert node[0].skip_tags == 'foobar'
     assert node[0].job_tags == 'untagged'
+
+
+@pytest.mark.django_db
+@pytest.mark.parametrize('num_hosts, num_queries', [(1, 70), (10, 150), (25, 250)])
+def test_bulk_host_delete_num_queries(organization, inventory, post, get, user, num_hosts, num_queries, django_assert_max_num_queries):
+    '''
+    If I am a...
+      org admin
+      inventory admin at org level
+      admin of a particular inventory
+      superuser
+
+    Bulk Host delete should take under a certain number of queries
+    '''
+    users_list = setup_admin_users_list(organization, inventory, user)
+    for u in users_list:
+        hosts = [{'name': str(uuid4())} for i in range(num_hosts)]
+        with django_assert_max_num_queries(num_queries):
+            bulk_host_create_response = post(reverse('api:bulk_host_create'), {'inventory': inventory.id, 'hosts': hosts}, u, expect=201).data
+            assert len(bulk_host_create_response['hosts']) == len(hosts), f"unexpected number of hosts created for user {u}"
+            hosts_ids_created = get_inventory_hosts(get, inventory.id, u)
+            bulk_host_delete_response = post(reverse('api:bulk_host_delete'), {'hosts': hosts_ids_created}, u, expect=201).data
+            assert len(bulk_host_delete_response['hosts'].keys()) == len(hosts), f"unexpected number of hosts deleted for user {u}"
+
+
+@pytest.mark.django_db
+def test_bulk_host_delete_rbac(organization, inventory, post, get, user):
+    '''
+    If I am a...
+      org admin
+      inventory admin at org level
+      admin of a particular invenotry
+          ... I can bulk delete hosts
+
+    Everyone else cannot
+    '''
+    admin_users_list = setup_admin_users_list(organization, inventory, user)
+    users_list = setup_none_admin_uses_list(organization, inventory, user)
+
+    for indx, u in enumerate(admin_users_list):
+        bulk_host_create_response = post(
+            reverse('api:bulk_host_create'), {'inventory': inventory.id, 'hosts': [{'name': f'foobar-{indx}'}]}, u, expect=201
+        ).data
+        assert len(bulk_host_create_response['hosts']) == 1, f"unexpected number of hosts created for user {u}"
+        assert Host.objects.filter(inventory__id=inventory.id)[0].name == f'foobar-{indx}'
+        hosts_ids_created = get_inventory_hosts(get, inventory.id, u)
+        bulk_host_delete_response = post(reverse('api:bulk_host_delete'), {'hosts': hosts_ids_created}, u, expect=201).data
+        assert len(bulk_host_delete_response['hosts'].keys()) == 1, f"unexpected number of hosts deleted by user {u}"
+
+    for indx, create_u in enumerate(admin_users_list):
+        bulk_host_create_response = post(
+            reverse('api:bulk_host_create'), {'inventory': inventory.id, 'hosts': [{'name': f'foobar2-{indx}'}]}, create_u, expect=201
+        ).data
+        print(bulk_host_create_response)
+        assert bulk_host_create_response['hosts'][0]['name'] == f'foobar2-{indx}'
+        hosts_ids_created = get_inventory_hosts(get, inventory.id, create_u)
+        print(f"Try to delete {hosts_ids_created}")
+        for delete_u in users_list:
+            bulk_host_delete_response = post(reverse('api:bulk_host_delete'), {'hosts': hosts_ids_created}, delete_u, expect=403).data
+            assert "Lack permissions to delete hosts from this inventory." in bulk_host_delete_response['inventories'].values()
+
+
+@pytest.mark.django_db
+def test_bulk_host_delete_from_multiple_inv(organization, inventory, post, get, user):
+    '''
+    If I am inventory admin at org level
+
+    Bulk Host delete should be enabled only on my inventory
+    '''
+    num_hosts = 10
+    inventory.organization = organization
+
+    # Create second inventory
+    inv2 = organization.inventories.create(name="second-test-inv")
+    inv2.organization = organization
+    admin2_user = user('inventory2_admin', False)
+    inv2.admin_role.members.add(admin2_user)
+
+    admin_user = user('inventory_admin', False)
+    inventory.admin_role.members.add(admin_user)
+
+    organization.member_role.members.add(admin_user)
+    organization.member_role.members.add(admin2_user)
+
+    hosts = [{'name': str(uuid4())} for i in range(num_hosts)]
+    hosts2 = [{'name': str(uuid4())} for i in range(num_hosts)]
+
+    # create hosts in each of the inventories
+    bulk_host_create_response = post(reverse('api:bulk_host_create'), {'inventory': inventory.id, 'hosts': hosts}, admin_user, expect=201).data
+    assert len(bulk_host_create_response['hosts']) == len(hosts), f"unexpected number of hosts created for user {admin_user}"
+
+    bulk_host_create_response2 = post(reverse('api:bulk_host_create'), {'inventory': inv2.id, 'hosts': hosts2}, admin2_user, expect=201).data
+    assert len(bulk_host_create_response2['hosts']) == len(hosts), f"unexpected number of hosts created for user {admin2_user}"
+
+    # get all hosts ids - from both inventories
+    hosts_ids_created = get_inventory_hosts(get, inventory.id, admin_user)
+    hosts_ids_created += get_inventory_hosts(get, inv2.id, admin2_user)
+
+    expected_error = "Lack permissions to delete hosts from this inventory."
+    # try to delete ALL hosts with admin user of inventory 1.
+    for inv_name, invadmin in zip([inv2.name, inventory.name], [admin_user, admin2_user]):
+        bulk_host_delete_response = post(reverse('api:bulk_host_delete'), {'hosts': hosts_ids_created}, invadmin, expect=403).data
+        result_message = bulk_host_delete_response['inventories'][inv_name]
+        assert result_message == expected_error, f"deleted hosts without permission by user {invadmin}"
+
+
+def setup_admin_users_list(organization, inventory, user):
+    inventory.organization = organization
+    inventory_admin = user('inventory_admin', False)
+    org_admin = user('org_admin', False)
+    org_inv_admin = user('org_admin', False)
+    superuser = user('admin', True)
+    for u in [org_admin, org_inv_admin, inventory_admin]:
+        organization.member_role.members.add(u)
+    organization.admin_role.members.add(org_admin)
+    organization.inventory_admin_role.members.add(org_inv_admin)
+    inventory.admin_role.members.add(inventory_admin)
+    return [inventory_admin, org_inv_admin, superuser, org_admin]
+
+
+def setup_none_admin_uses_list(organization, inventory, user):
+    inventory.organization = organization
+    auditor = user('auditor', False)
+    member = user('member', False)
+    use_inv_member = user('member', False)
+    for u in [auditor, member, use_inv_member]:
+        organization.member_role.members.add(u)
+    inventory.use_role.members.add(use_inv_member)
+    organization.auditor_role.members.add(auditor)
+    return [auditor, member, use_inv_member]
+
+
+def get_inventory_hosts(get, inv_id, use_user):
+    data = get(reverse('api:inventory_hosts_list', kwargs={'pk': inv_id}), use_user, expect=200).data
+    results = [host['id'] for host in data['results']]
+    return results
diff --git a/awx/settings/defaults.py b/awx/settings/defaults.py
@@ -131,6 +131,9 @@
 # Maximum number of host that can be created in 1 bulk host create
 BULK_HOST_MAX_CREATE = 100
 
+# Maximum number of host that can be deleted in 1 bulk host delete
+BULK_HOST_MAX_DELETE = 250
+
 SITE_ID = 1
 
 # Make this unique, and don't share it with anybody.

diff --git a/awx_collection/meta/runtime.yml b/awx_collection/meta/runtime.yml
@@ -8,6 +8,7 @@ action_groups:
     - application
     - bulk_job_launch
     - bulk_host_create
+    - bulk_host_delete
     - controller_meta
     - credential_input_source
     - credential