From 37febca4ee6dd02f12df6188e502569feb447959 Mon Sep 17 00:00:00 2001 From: Dirk Julich Date: Thu, 18 Jun 2026 14:26:03 +0200 Subject: [PATCH] Rewrite org count subqueries to use DAB RBAC models Replace old RBAC Role.members.through subqueries with RoleUserAssignment-based correlated subqueries, querying managed RoleDefinitions ('Organization Member' / 'Organization Admin') directly. This aligns with the DAB RBAC migration direction and eliminates dependency on the deprecated ImplicitRoleField M2M tables for these counts. Update test fixtures to use RoleDefinition.give_permission() and add setup_managed_roles where needed. Co-Authored-By: Claude Opus 4.6 --- awx/api/views/mixin.py | 45 +++++++++---------- awx/api/views/organization.py | 41 ++++++++--------- .../api/test_organization_counts.py | 9 ++-- .../functional/api/test_organizations.py | 14 +++--- 4 files changed, 56 insertions(+), 53 deletions(-) diff --git a/awx/api/views/mixin.py b/awx/api/views/mixin.py index dc72a994b7..686d4d0ace 100644 --- a/awx/api/views/mixin.py +++ b/awx/api/views/mixin.py @@ -4,7 +4,7 @@ import dateutil import logging -from django.db.models import Count, IntegerField, OuterRef, Subquery +from django.db.models import Count, OuterRef, Subquery from django.db.models.functions import Coalesce from django.db import transaction from django.shortcuts import get_object_or_404 @@ -16,7 +16,8 @@ from rest_framework.response import Response from rest_framework import status from awx.main.constants import ACTIVE_STATES -from awx.main.models import Organization, Role +from ansible_base.rbac.models import RoleDefinition, RoleUserAssignment +from awx.main.models import Organization from awx.main.utils import get_object_or_400 from awx.main.models.ha import Instance, InstanceGroup, schedule_policy_task from awx.main.models.organization import Team @@ -178,28 +179,26 @@ class OrganizationCountsMixin(object): db_results['projects'] = project_qs.values('organization').annotate(Count('organization')).order_by('organization') - # Other members and admins of organization are always viewable - # - # Use independent subqueries instead of double-JOIN Count to avoid - # cartesian product. - role_members_through = Role.members.through - member_count = Subquery( - role_members_through.objects.filter(role_id=OuterRef('member_role_id')) - .values('role_id') - .annotate(cnt=Count('user_id', distinct=True)) - .values('cnt'), - output_field=IntegerField(), - ) - admin_count = Subquery( - role_members_through.objects.filter(role_id=OuterRef('admin_role_id')) - .values('role_id') - .annotate(cnt=Count('user_id', distinct=True)) - .values('cnt'), - output_field=IntegerField(), - ) + member_rd = RoleDefinition.objects.get(name='Organization Member') + admin_rd = RoleDefinition.objects.get(name='Organization Admin') + + def assignment_count(rd): + return Coalesce( + Subquery( + RoleUserAssignment.objects.filter( + object_id=OuterRef('pk'), + role_definition=rd, + ) + .values('role_definition') + .annotate(c=Count('pk')) + .values('c') + ), + 0, + ) + db_results['users'] = org_qs.annotate( - users=Coalesce(member_count, 0), - admins=Coalesce(admin_count, 0), + users=assignment_count(member_rd), + admins=assignment_count(admin_rd), ).values('id', 'users', 'admins') count_context = {} diff --git a/awx/api/views/organization.py b/awx/api/views/organization.py index 787624549b..952ad8b286 100644 --- a/awx/api/views/organization.py +++ b/awx/api/views/organization.py @@ -5,12 +5,13 @@ import logging # Django -from django.db.models import Count, IntegerField, OuterRef, Subquery +from django.db.models import Count, OuterRef, Subquery from django.db.models.functions import Coalesce from django.contrib.contenttypes.models import ContentType from django.utils.translation import gettext_lazy as _ # AWX +from ansible_base.rbac.models import RoleDefinition, RoleUserAssignment from awx.main.models import ( ActivityStream, Inventory, @@ -78,28 +79,28 @@ class OrganizationDetail(RelatedJobsPreventDeleteMixin, RetrieveUpdateDestroyAPI org_counts = {} access_kwargs = {'accessor': self.request.user, 'role_field': 'read_role'} - # Use independent subqueries instead of double-JOIN Count to avoid - # cartesian product. - role_members_through = Role.members.through - member_count = Subquery( - role_members_through.objects.filter(role_id=OuterRef('member_role_id')) - .values('role_id') - .annotate(cnt=Count('user_id', distinct=True)) - .values('cnt'), - output_field=IntegerField(), - ) - admin_count = Subquery( - role_members_through.objects.filter(role_id=OuterRef('admin_role_id')) - .values('role_id') - .annotate(cnt=Count('user_id', distinct=True)) - .values('cnt'), - output_field=IntegerField(), - ) + member_rd = RoleDefinition.objects.get(name='Organization Member') + admin_rd = RoleDefinition.objects.get(name='Organization Admin') + + def assignment_count(rd): + return Coalesce( + Subquery( + RoleUserAssignment.objects.filter( + object_id=OuterRef('pk'), + role_definition=rd, + ) + .values('role_definition') + .annotate(c=Count('pk')) + .values('c') + ), + 0, + ) + direct_counts = ( Organization.objects.filter(id=org_id) .annotate( - users=Coalesce(member_count, 0), - admins=Coalesce(admin_count, 0), + users=assignment_count(member_rd), + admins=assignment_count(admin_rd), ) .values('users', 'admins') ) diff --git a/awx/main/tests/functional/api/test_organization_counts.py b/awx/main/tests/functional/api/test_organization_counts.py index d1790f413c..f647cf7d53 100644 --- a/awx/main/tests/functional/api/test_organization_counts.py +++ b/awx/main/tests/functional/api/test_organization_counts.py @@ -1,20 +1,23 @@ import pytest +from ansible_base.rbac.models import RoleDefinition from awx.api.versioning import reverse from awx.main.models import Project, Host @pytest.fixture -def organization_resource_creator(organization, user): +def organization_resource_creator(organization, user, setup_managed_roles): def rf(users, admins, job_templates, projects, inventories, teams): + member_rd = RoleDefinition.objects.get(name='Organization Member') + admin_rd = RoleDefinition.objects.get(name='Organization Admin') # Associate one resource of every type with the organization for i in range(users): member_user = user('org-member %s' % i) - organization.member_role.members.add(member_user) + member_rd.give_permission(member_user, organization) for i in range(admins): admin_user = user('org-admin %s' % i) - organization.admin_role.members.add(admin_user) + admin_rd.give_permission(admin_user, organization) for i in range(teams): organization.teams.create(name='org-team %s' % i) for i in range(inventories): diff --git a/awx/main/tests/functional/api/test_organizations.py b/awx/main/tests/functional/api/test_organizations.py index af2f918ba0..6a234c09f5 100644 --- a/awx/main/tests/functional/api/test_organizations.py +++ b/awx/main/tests/functional/api/test_organizations.py @@ -56,7 +56,7 @@ def test_organization_list_access_tests(options, head, get, admin, alice): @pytest.mark.django_db -def test_organization_access_tests(organization, get, admin, alice, bob): +def test_organization_access_tests(organization, get, admin, alice, bob, setup_managed_roles): organization.member_role.members.add(alice) get(reverse('api:organization_detail', kwargs={'pk': organization.id}), user=admin, expect=200) get(reverse('api:organization_detail', kwargs={'pk': organization.id}), user=alice, expect=200) @@ -65,14 +65,14 @@ def test_organization_access_tests(organization, get, admin, alice, bob): @pytest.mark.django_db -def test_organization_list_integrity(organization, get, admin, alice): +def test_organization_list_integrity(organization, get, admin, alice, setup_managed_roles): res = get(reverse('api:organization_list'), user=admin) for field in ['id', 'url', 'name', 'description', 'created']: assert field in res.data['results'][0] @pytest.mark.django_db -def test_organization_list_order_integrity(organizations, get, admin): +def test_organization_list_order_integrity(organizations, get, admin, setup_managed_roles): # check that the order of the organization list retains integrity. orgs = organizations(4) org_ids = {} @@ -83,7 +83,7 @@ def test_organization_list_order_integrity(organizations, get, admin): @pytest.mark.django_db -def test_organization_list_visibility(organizations, get, admin, alice): +def test_organization_list_visibility(organizations, get, admin, alice, setup_managed_roles): orgs = organizations(2) res = get(reverse('api:organization_list'), user=admin) @@ -151,7 +151,7 @@ def test_organization_inventory_list_order_integrity(organization, admin, invent @pytest.mark.django_db -def test_create_organization(post, admin, alice): +def test_create_organization(post, admin, alice, setup_managed_roles): new_org = {'name': 'new org', 'description': 'my description'} res = post(reverse('api:organization_list'), new_org, user=admin, expect=201) assert res.data['name'] == new_org['name'] @@ -197,7 +197,7 @@ def test_add_admin_to_organization_xfail(post, organization, alice, bob): @pytest.mark.django_db -def test_update_organization(get, put, organization, alice, bob): +def test_update_organization(get, put, organization, alice, bob, setup_managed_roles): organization.admin_role.members.add(alice) data = get(reverse('api:organization_detail', kwargs={'pk': organization.id}), user=alice, expect=200).data data['description'] = 'hi' @@ -209,7 +209,7 @@ def test_update_organization(get, put, organization, alice, bob): @pytest.mark.django_db -def test_update_organization_max_hosts(get, put, organization, admin, alice, bob): +def test_update_organization_max_hosts(get, put, organization, admin, alice, bob, setup_managed_roles): # Admin users can get and update max_hosts data = get(reverse('api:organization_detail', kwargs={'pk': organization.id}), user=admin, expect=200).data assert organization.max_hosts == 0