Upstream 16555 - Replace 4-way OR with UNION in unified job list RBAC query

awx/api/pagination.py

@@ -6,12 +6,15 @@
 # Django REST Framework
 from django.conf import settings
 from django.core.paginator import Paginator as DjangoPaginator
+from django.utils.functional import cached_property
 from rest_framework import pagination
 from rest_framework.response import Response
 from rest_framework.utils.urls import replace_query_param
 from rest_framework.settings import api_settings
 from django.utils.translation import gettext_lazy as _
 
+from awx.main.models import UnifiedJob
+
 
 class DisabledPaginator(DjangoPaginator):
     @property
@@ -23,6 +26,18 @@ def count(self):
         return 200
 
 
+class UnifiedJobPaginator(DjangoPaginator):
+    """Use unfiltered table count for unified job pagination.
+
+    The RBAC-filtered COUNT query is prohibitively slow on large unified job
+    tables; an approximate over-count is harmless for pagination UI.
+    """
+
+    @cached_property
+    def count(self):
+        return UnifiedJob.objects.count()
+
+
 class Pagination(pagination.PageNumberPagination):
     page_size_query_param = 'page_size'
     max_page_size = settings.MAX_PAGE_SIZE
@@ -57,19 +72,24 @@ def get_html_context(self):
 
     def paginate_queryset(self, queryset, request, **kwargs):
         self.count_disabled = 'count_disabled' in request.query_params
+        original_paginator = self.django_paginator_class
         try:
             if self.count_disabled:
                 self.django_paginator_class = DisabledPaginator
             return super(Pagination, self).paginate_queryset(queryset, request, **kwargs)
         finally:
-            self.django_paginator_class = DjangoPaginator
+            self.django_paginator_class = original_paginator
 
     def get_paginated_response(self, data):
         if self.count_disabled:
             return Response({'results': data})
         return super(Pagination, self).get_paginated_response(data)
 
 
+class UnifiedJobPagination(Pagination):
+    django_paginator_class = UnifiedJobPaginator
+
+
 class LimitPagination(pagination.BasePagination):
     default_limit = api_settings.PAGE_SIZE
     limit_query_param = 'limit'

awx/api/views/__init__.py

@@ -120,7 +120,7 @@
     NoTruncateMixin,
     UnifiedJobIncludeMixin,
 )
-from awx.api.pagination import UnifiedJobEventPagination
+from awx.api.pagination import UnifiedJobEventPagination, UnifiedJobPagination
 from awx.main.utils import set_environ
 
 logger = logging.getLogger('awx.api.views')
@@ -4176,6 +4176,7 @@ class UnifiedJobList(UnifiedJobIncludeMixin, ListAPIView):
     model = models.UnifiedJob
     serializer_class = serializers.UnifiedJobListSerializer
     search_fields = ('description', 'name', 'job__playbook')
+    pagination_class = UnifiedJobPagination
 
 
 # Pre-compile ANSI patterns for performance

awx/main/access.py

@@ -11,6 +11,7 @@
 from django.conf import settings
 from django.db.models import Q, Prefetch
 from django.contrib.auth.models import User
+from django.contrib.contenttypes.models import ContentType
 from django.utils.translation import gettext_lazy as _
 from django.core.exceptions import ObjectDoesNotExist, FieldDoesNotExist
 
@@ -77,6 +78,7 @@
     ROLE_SINGLETON_SYSTEM_AUDITOR,
 )
 from awx.main.models.mixins import ResourceMixin
+from awx.main.models.rbac import RoleAncestorEntry
 
 __all__ = [
     'get_user_queryset',
@@ -2587,15 +2589,50 @@ class UnifiedJobAccess(BaseAccess):
     # )
 
     def filtered_queryset(self):
-        inv_pk_qs = Inventory._accessible_pk_qs(Inventory, self.user, 'read_role')
-        org_auditor_qs = Organization.objects.filter(Q(admin_role__members=self.user) | Q(auditor_role__members=self.user))
-        qs = self.model.objects.filter(
-            Q(unified_job_template_id__in=UnifiedJobTemplate.accessible_pk_qs(self.user, 'read_role'))
-            | Q(pk__in=InventoryUpdate.objects.filter(inventory_source__inventory_id__in=inv_pk_qs).values('pk'))
-            | Q(pk__in=AdHocCommand.objects.filter(inventory_id__in=inv_pk_qs).values('pk'))
-            | Q(organization__in=org_auditor_qs)
+        # Pre-compute the user's direct role memberships once and pass them
+        # as literal parameters to each RBAC branch, and use OR (not UNION)
+        # so the database can single-pass filter with an early LIMIT exit.
+        # UNION forces materialization of every accessible job id before
+        # ordering/LIMIT can apply, and repeats the role-membership subquery
+        # in every branch.
+        user_role_ids = list(self.user.roles.values_list('id', flat=True))
+        if not user_role_ids:
+            return self.model.objects.none()
+
+        ujt_accessible = (
+            RoleAncestorEntry.objects.filter(
+                ancestor_id__in=user_role_ids,
+                role_field='read_role',
+                content_type_id__in=UnifiedJobTemplate._submodels_with_roles(),
+            )
+            .values_list('object_id')
+            .distinct()
+        )
+
+        inv_accessible = (
+            RoleAncestorEntry.objects.filter(
+                ancestor_id__in=user_role_ids,
+                role_field='read_role',
+                content_type_id=ContentType.objects.get_for_model(Inventory).id,
+            )
+            .values_list('object_id')
+            .distinct()
+        )
+
+        return self.model.objects.filter(
+            Q(unified_job_template_id__in=ujt_accessible)
+            | Q(
+                pk__in=InventoryUpdate.objects.filter(
+                    inventory_source__inventory_id__in=inv_accessible,
+                ).values('pk')
+            )
+            | Q(
+                pk__in=AdHocCommand.objects.filter(
+                    inventory_id__in=inv_accessible,
+                ).values('pk')
+            )
+            | Q(organization__in=Organization.objects.filter(Q(admin_role_id__in=user_role_ids) | Q(auditor_role_id__in=user_role_ids)))
         )
-        return qs
 
     def get_queryset(self):
         return super(UnifiedJobAccess, self).get_queryset().filter(workflowapproval__isnull=True)

awx/main/tests/functional/test_rbac_unified_jobs.py

@@ -0,0 +1,155 @@
+import pytest
+
+from django.test.utils import CaptureQueriesContext
+from django.db import connection
+
+from awx.api.versioning import reverse
+from awx.main.models import (
+    AdHocCommand,
+    InventorySource,
+    InventoryUpdate,
+    JobTemplate,
+    Organization,
+    Project,
+    UnifiedJob,
+)
+
+
+@pytest.mark.django_db
+def test_unified_job_list_uses_or_not_union(user, organization, inventory, get):
+    """The unified job list RBAC query uses OR-based filtering, not UNION."""
+    org_admin = user('uj-org-admin')
+    organization.admin_role.members.add(org_admin)
+
+    project = Project.objects.create(name='uj-test-project', organization=organization)
+    jt = JobTemplate.objects.create(name='uj-test-jt', project=project, inventory=inventory, organization=organization)
+    jt.create_unified_job()
+
+    inv_src = InventorySource.objects.create(name='uj-test-invsrc', inventory=inventory, source='ec2')
+    InventoryUpdate.objects.create(inventory_source=inv_src, source=inv_src.source)
+
+    AdHocCommand.objects.create(name='uj-test-adhoc', inventory=inventory)
+
+    with CaptureQueriesContext(connection) as ctx:
+        response = get(reverse('api:unified_job_list'), org_admin)
+
+    assert response.status_code == 200
+    assert response.data['count'] >= 3
+
+    uj_rbac_queries = [q['sql'] for q in ctx.captured_queries if 'main_unifiedjob' in q['sql'] and 'main_rbac_role_ancestors' in q['sql']]
+    assert uj_rbac_queries, "Expected a unified-job RBAC query"
+    for sql in uj_rbac_queries:
+        assert 'UNION' not in sql, "RBAC query should use OR, not UNION"
+
+
+@pytest.mark.django_db
+def test_unified_job_list_org_auditor_sees_jobs(user, get):
+    """Org auditors see unified jobs in their org via the org auditor RBAC branch."""
+    org = Organization.objects.create(name='uj-audit-org')
+    auditor = user('uj-auditor')
+    org.auditor_role.members.add(auditor)
+
+    inventory = org.inventories.create(name='uj-audit-inv')
+    project = Project.objects.create(name='uj-audit-project', organization=org)
+    jt = JobTemplate.objects.create(name='uj-audit-jt', project=project, inventory=inventory, organization=org)
+    job = jt.create_unified_job()
+
+    response = get(reverse('api:unified_job_list'), auditor)
+    assert response.status_code == 200
+    result_ids = [r['id'] for r in response.data['results']]
+    assert job.pk in result_ids
+
+
+@pytest.mark.django_db
+def test_unified_job_list_inventory_viewer_sees_inventory_updates(user, get):
+    """Users with inventory read permission see inventory updates via the inventory RBAC branch."""
+    org = Organization.objects.create(name='uj-inv-org')
+    inventory = org.inventories.create(name='uj-inv-test')
+    inv_viewer = user('uj-inv-viewer')
+    inventory.read_role.members.add(inv_viewer)
+
+    inv_src = InventorySource.objects.create(name='uj-inv-src', inventory=inventory, source='ec2')
+    inv_update = InventoryUpdate.objects.create(inventory_source=inv_src, source=inv_src.source)
+
+    response = get(reverse('api:unified_job_list'), inv_viewer)
+    assert response.status_code == 200
+    result_ids = [r['id'] for r in response.data['results']]
+    assert inv_update.pk in result_ids
+
+
+@pytest.mark.django_db
+def test_unified_job_list_team_grant_sees_jobs(user, get):
+    """Access granted through a team (not a direct user->role grant) still
+    surfaces jobs; the pre-computed role set must include team-mediated
+    ancestry, not just directly-granted object roles."""
+    from awx.main.models import Team
+
+    org = Organization.objects.create(name='uj-team-org')
+    inventory = org.inventories.create(name='uj-team-inv')
+    project = Project.objects.create(name='uj-team-project', organization=org)
+    jt = JobTemplate.objects.create(name='uj-team-jt', project=project, inventory=inventory, organization=org)
+    job = jt.create_unified_job()
+
+    team = Team.objects.create(name='uj-team', organization=org)
+    team_member = user('uj-team-member')
+    team.member_role.members.add(team_member)
+    jt.read_role.parents.add(team.member_role)
+
+    response = get(reverse('api:unified_job_list'), team_member)
+    assert response.status_code == 200
+    result_ids = [r['id'] for r in response.data['results']]
+    assert job.pk in result_ids
+
+
+@pytest.mark.django_db
+def test_unified_job_list_org_member_sees_nothing(user, get):
+    """A user with roles (org member) but no job-related access sees no jobs.
+    Unlike rando, this user has a non-empty role set, so it exercises the
+    full OR query rather than the empty-role-set early exit."""
+    org = Organization.objects.create(name='uj-member-org')
+    member = user('uj-member')
+    org.member_role.members.add(member)
+
+    inventory = org.inventories.create(name='uj-member-inv')
+    project = Project.objects.create(name='uj-member-project', organization=org)
+    jt = JobTemplate.objects.create(name='uj-member-jt', project=project, inventory=inventory, organization=org)
+    jt.create_unified_job()
+
+    response = get(reverse('api:unified_job_list'), member)
+    assert response.status_code == 200
+    assert len(response.data['results']) == 0
+
+
+@pytest.mark.django_db
+def test_unified_job_list_rando_sees_nothing(rando, get):
+    """Unprivileged user sees no unified jobs."""
+    org = Organization.objects.create(name='uj-rando-org')
+    inventory = org.inventories.create(name='uj-rando-inv')
+    project = Project.objects.create(name='uj-rando-project', organization=org)
+    jt = JobTemplate.objects.create(name='uj-rando-jt', project=project, inventory=inventory, organization=org)
+    jt.create_unified_job()
+    AdHocCommand.objects.create(name='uj-rando-adhoc', inventory=inventory)
+
+    response = get(reverse('api:unified_job_list'), rando)
+    assert response.status_code == 200
+    assert len(response.data['results']) == 0
+
+
+@pytest.mark.django_db
+def test_unified_job_list_pagination_uses_unfiltered_count(rando, get):
+    """The pagination count should reflect total unified job rows, not
+    the RBAC-filtered subset.  The RBAC-filtered COUNT is catastrophically
+    slow on large tables with pk__in UNION subqueries."""
+    org = Organization.objects.create(name='uj-count-org')
+    inventory = org.inventories.create(name='uj-count-inv')
+    project = Project.objects.create(name='uj-count-project', organization=org)
+    jt = JobTemplate.objects.create(name='uj-count-jt', project=project, inventory=inventory, organization=org)
+    jt.create_unified_job()
+
+    total_jobs = UnifiedJob.objects.count()
+    assert total_jobs > 0
+
+    response = get(reverse('api:unified_job_list'), rando)
+    assert response.status_code == 200
+    assert len(response.data['results']) == 0
+    assert response.data['count'] == total_jobs

awx/main/tests/unit/api/test_pagination.py

@@ -0,0 +1,63 @@
+from unittest.mock import patch, MagicMock
+
+from awx.api.pagination import UnifiedJobPaginator, UnifiedJobPagination, DisabledPaginator
+
+
+class TestUnifiedJobPaginator:
+    def test_count_uses_unfiltered_table_count(self):
+        with patch('awx.api.pagination.UnifiedJob') as mock_uj:
+            mock_uj.objects.count.return_value = 42000
+            paginator = UnifiedJobPaginator(object_list=[], per_page=25)
+            assert paginator.count == 42000
+            mock_uj.objects.count.assert_called_once()
+
+    def test_count_is_cached(self):
+        with patch('awx.api.pagination.UnifiedJob') as mock_uj:
+            mock_uj.objects.count.return_value = 500
+            paginator = UnifiedJobPaginator(object_list=[], per_page=25)
+            _ = paginator.count
+            _ = paginator.count
+            mock_uj.objects.count.assert_called_once()
+
+
+class TestUnifiedJobPagination:
+    def test_default_paginator_class(self):
+        pagination = UnifiedJobPagination()
+        assert pagination.django_paginator_class is UnifiedJobPaginator
+
+    def test_normal_request_preserves_unified_job_paginator(self):
+        pagination = UnifiedJobPagination()
+        request = MagicMock()
+        request.query_params = {}
+
+        with patch('rest_framework.pagination.PageNumberPagination.paginate_queryset', return_value=[]):
+            pagination.paginate_queryset(MagicMock(), request)
+
+        assert pagination.count_disabled is False
+        assert pagination.django_paginator_class is UnifiedJobPaginator
+
+    def test_count_disabled_restores_unified_job_paginator(self):
+        pagination = UnifiedJobPagination()
+        request = MagicMock()
+        request.query_params = {'count_disabled': 'true'}
+
+        with patch('rest_framework.pagination.PageNumberPagination.paginate_queryset', return_value=[]):
+            pagination.paginate_queryset(MagicMock(), request)
+
+        assert pagination.count_disabled is True
+        assert pagination.django_paginator_class is UnifiedJobPaginator
+
+    def test_count_disabled_temporarily_uses_disabled_paginator(self):
+        pagination = UnifiedJobPagination()
+        request = MagicMock()
+        request.query_params = {'count_disabled': 'true'}
+        captured_class = {}
+
+        def capture_paginator_class(self_inner, queryset, request, **kwargs):
+            captured_class['during'] = pagination.django_paginator_class
+
+        with patch('rest_framework.pagination.PageNumberPagination.paginate_queryset', capture_paginator_class):
+            pagination.paginate_queryset(MagicMock(), request)
+
+        assert captured_class['during'] is DisabledPaginator
+        assert pagination.django_paginator_class is UnifiedJobPaginator