mirror of
https://github.com/ansible/awx.git
synced 2026-06-23 07:37:50 -02:30
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 <noreply@anthropic.com>
This commit is contained in:
@@ -4,7 +4,7 @@
|
|||||||
import dateutil
|
import dateutil
|
||||||
import logging
|
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.models.functions import Coalesce
|
||||||
from django.db import transaction
|
from django.db import transaction
|
||||||
from django.shortcuts import get_object_or_404
|
from django.shortcuts import get_object_or_404
|
||||||
@@ -16,7 +16,8 @@ from rest_framework.response import Response
|
|||||||
from rest_framework import status
|
from rest_framework import status
|
||||||
|
|
||||||
from awx.main.constants import ACTIVE_STATES
|
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.utils import get_object_or_400
|
||||||
from awx.main.models.ha import Instance, InstanceGroup, schedule_policy_task
|
from awx.main.models.ha import Instance, InstanceGroup, schedule_policy_task
|
||||||
from awx.main.models.organization import Team
|
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')
|
db_results['projects'] = project_qs.values('organization').annotate(Count('organization')).order_by('organization')
|
||||||
|
|
||||||
# Other members and admins of organization are always viewable
|
member_rd = RoleDefinition.objects.get(name='Organization Member')
|
||||||
#
|
admin_rd = RoleDefinition.objects.get(name='Organization Admin')
|
||||||
# Use independent subqueries instead of double-JOIN Count to avoid
|
|
||||||
# cartesian product.
|
def assignment_count(rd):
|
||||||
role_members_through = Role.members.through
|
return Coalesce(
|
||||||
member_count = Subquery(
|
Subquery(
|
||||||
role_members_through.objects.filter(role_id=OuterRef('member_role_id'))
|
RoleUserAssignment.objects.filter(
|
||||||
.values('role_id')
|
object_id=OuterRef('pk'),
|
||||||
.annotate(cnt=Count('user_id', distinct=True))
|
role_definition=rd,
|
||||||
.values('cnt'),
|
)
|
||||||
output_field=IntegerField(),
|
.values('role_definition')
|
||||||
)
|
.annotate(c=Count('pk'))
|
||||||
admin_count = Subquery(
|
.values('c')
|
||||||
role_members_through.objects.filter(role_id=OuterRef('admin_role_id'))
|
),
|
||||||
.values('role_id')
|
0,
|
||||||
.annotate(cnt=Count('user_id', distinct=True))
|
)
|
||||||
.values('cnt'),
|
|
||||||
output_field=IntegerField(),
|
|
||||||
)
|
|
||||||
db_results['users'] = org_qs.annotate(
|
db_results['users'] = org_qs.annotate(
|
||||||
users=Coalesce(member_count, 0),
|
users=assignment_count(member_rd),
|
||||||
admins=Coalesce(admin_count, 0),
|
admins=assignment_count(admin_rd),
|
||||||
).values('id', 'users', 'admins')
|
).values('id', 'users', 'admins')
|
||||||
|
|
||||||
count_context = {}
|
count_context = {}
|
||||||
|
|||||||
@@ -5,12 +5,13 @@
|
|||||||
import logging
|
import logging
|
||||||
|
|
||||||
# Django
|
# 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.db.models.functions import Coalesce
|
||||||
from django.contrib.contenttypes.models import ContentType
|
from django.contrib.contenttypes.models import ContentType
|
||||||
from django.utils.translation import gettext_lazy as _
|
from django.utils.translation import gettext_lazy as _
|
||||||
|
|
||||||
# AWX
|
# AWX
|
||||||
|
from ansible_base.rbac.models import RoleDefinition, RoleUserAssignment
|
||||||
from awx.main.models import (
|
from awx.main.models import (
|
||||||
ActivityStream,
|
ActivityStream,
|
||||||
Inventory,
|
Inventory,
|
||||||
@@ -78,28 +79,28 @@ class OrganizationDetail(RelatedJobsPreventDeleteMixin, RetrieveUpdateDestroyAPI
|
|||||||
|
|
||||||
org_counts = {}
|
org_counts = {}
|
||||||
access_kwargs = {'accessor': self.request.user, 'role_field': 'read_role'}
|
access_kwargs = {'accessor': self.request.user, 'role_field': 'read_role'}
|
||||||
# Use independent subqueries instead of double-JOIN Count to avoid
|
member_rd = RoleDefinition.objects.get(name='Organization Member')
|
||||||
# cartesian product.
|
admin_rd = RoleDefinition.objects.get(name='Organization Admin')
|
||||||
role_members_through = Role.members.through
|
|
||||||
member_count = Subquery(
|
def assignment_count(rd):
|
||||||
role_members_through.objects.filter(role_id=OuterRef('member_role_id'))
|
return Coalesce(
|
||||||
.values('role_id')
|
Subquery(
|
||||||
.annotate(cnt=Count('user_id', distinct=True))
|
RoleUserAssignment.objects.filter(
|
||||||
.values('cnt'),
|
object_id=OuterRef('pk'),
|
||||||
output_field=IntegerField(),
|
role_definition=rd,
|
||||||
)
|
)
|
||||||
admin_count = Subquery(
|
.values('role_definition')
|
||||||
role_members_through.objects.filter(role_id=OuterRef('admin_role_id'))
|
.annotate(c=Count('pk'))
|
||||||
.values('role_id')
|
.values('c')
|
||||||
.annotate(cnt=Count('user_id', distinct=True))
|
),
|
||||||
.values('cnt'),
|
0,
|
||||||
output_field=IntegerField(),
|
)
|
||||||
)
|
|
||||||
direct_counts = (
|
direct_counts = (
|
||||||
Organization.objects.filter(id=org_id)
|
Organization.objects.filter(id=org_id)
|
||||||
.annotate(
|
.annotate(
|
||||||
users=Coalesce(member_count, 0),
|
users=assignment_count(member_rd),
|
||||||
admins=Coalesce(admin_count, 0),
|
admins=assignment_count(admin_rd),
|
||||||
)
|
)
|
||||||
.values('users', 'admins')
|
.values('users', 'admins')
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -1,20 +1,23 @@
|
|||||||
import pytest
|
import pytest
|
||||||
|
|
||||||
|
from ansible_base.rbac.models import RoleDefinition
|
||||||
from awx.api.versioning import reverse
|
from awx.api.versioning import reverse
|
||||||
|
|
||||||
from awx.main.models import Project, Host
|
from awx.main.models import Project, Host
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture
|
@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):
|
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
|
# Associate one resource of every type with the organization
|
||||||
for i in range(users):
|
for i in range(users):
|
||||||
member_user = user('org-member %s' % i)
|
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):
|
for i in range(admins):
|
||||||
admin_user = user('org-admin %s' % i)
|
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):
|
for i in range(teams):
|
||||||
organization.teams.create(name='org-team %s' % i)
|
organization.teams.create(name='org-team %s' % i)
|
||||||
for i in range(inventories):
|
for i in range(inventories):
|
||||||
|
|||||||
@@ -56,7 +56,7 @@ def test_organization_list_access_tests(options, head, get, admin, alice):
|
|||||||
|
|
||||||
|
|
||||||
@pytest.mark.django_db
|
@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)
|
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=admin, expect=200)
|
||||||
get(reverse('api:organization_detail', kwargs={'pk': organization.id}), user=alice, 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
|
@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)
|
res = get(reverse('api:organization_list'), user=admin)
|
||||||
for field in ['id', 'url', 'name', 'description', 'created']:
|
for field in ['id', 'url', 'name', 'description', 'created']:
|
||||||
assert field in res.data['results'][0]
|
assert field in res.data['results'][0]
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.django_db
|
@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.
|
# check that the order of the organization list retains integrity.
|
||||||
orgs = organizations(4)
|
orgs = organizations(4)
|
||||||
org_ids = {}
|
org_ids = {}
|
||||||
@@ -83,7 +83,7 @@ def test_organization_list_order_integrity(organizations, get, admin):
|
|||||||
|
|
||||||
|
|
||||||
@pytest.mark.django_db
|
@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)
|
orgs = organizations(2)
|
||||||
|
|
||||||
res = get(reverse('api:organization_list'), user=admin)
|
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
|
@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'}
|
new_org = {'name': 'new org', 'description': 'my description'}
|
||||||
res = post(reverse('api:organization_list'), new_org, user=admin, expect=201)
|
res = post(reverse('api:organization_list'), new_org, user=admin, expect=201)
|
||||||
assert res.data['name'] == new_org['name']
|
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
|
@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)
|
organization.admin_role.members.add(alice)
|
||||||
data = get(reverse('api:organization_detail', kwargs={'pk': organization.id}), user=alice, expect=200).data
|
data = get(reverse('api:organization_detail', kwargs={'pk': organization.id}), user=alice, expect=200).data
|
||||||
data['description'] = 'hi'
|
data['description'] = 'hi'
|
||||||
@@ -209,7 +209,7 @@ def test_update_organization(get, put, organization, alice, bob):
|
|||||||
|
|
||||||
|
|
||||||
@pytest.mark.django_db
|
@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
|
# Admin users can get and update max_hosts
|
||||||
data = get(reverse('api:organization_detail', kwargs={'pk': organization.id}), user=admin, expect=200).data
|
data = get(reverse('api:organization_detail', kwargs={'pk': organization.id}), user=admin, expect=200).data
|
||||||
assert organization.max_hosts == 0
|
assert organization.max_hosts == 0
|
||||||
|
|||||||
Reference in New Issue
Block a user