Skip to content
Open
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,18 @@ jobs:
python-version: "3.11"
sonar: false
junit-xml-upload: false
- env: py310-django4
python-version: "3.10"
sonar: false
junit-xml-upload: false
- env: py311-django4
python-version: "3.11"
sonar: false
junit-xml-upload: false
- env: py312-django4
python-version: "3.12"
sonar: false
junit-xml-upload: false
steps:
- uses: actions/checkout@v4
with:
Expand Down
42 changes: 42 additions & 0 deletions ansible_base/rbac/migrations/0009_objectrole_help_text_change.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# Generated by Django 5.2.7 on 2025-10-06 13:50
#
# This migration updates the help_text for the `users` and `teams` ManyToManyField
# on the ObjectRole model to add trailing periods for consistency.
#
# WHY DJANGO 5 CREATES THIS BUT DJANGO 4 DOES NOT:
#
# Django 4's migration detector has a bug/limitation where it does not properly detect
# help_text changes on ManyToManyField instances that use custom `through` tables.
# The help_text was updated in the model code (ansible_base/rbac/models/role.py:518, 525)
# to add trailing periods, but Django 4's makemigrations did not detect this change.
#
# Django 5 improved its field state serialization and comparison logic for ManyToManyFields,
# particularly for fields with through tables, and now properly detects these help_text changes.
#
# This is a harmless migration that only updates field metadata (help_text) and does not
# modify the database schema or affect data.

from django.conf import settings
from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('dab_rbac', '0008_remote_permissions_cleanup'),
migrations.swappable_dependency(settings.AUTH_USER_MODEL),
migrations.swappable_dependency(settings.ANSIBLE_BASE_TEAM_MODEL),
]

operations = [
migrations.AlterField(
model_name='objectrole',
name='teams',
field=models.ManyToManyField(help_text='Teams or groups who have access to the permissions defined by this object role.', related_name='has_roles', through='dab_rbac.RoleTeamAssignment', through_fields=('object_role', 'team'), to=settings.ANSIBLE_BASE_TEAM_MODEL),
),
migrations.AlterField(
model_name='objectrole',
name='users',
field=models.ManyToManyField(help_text='Users who have access to the permissions defined by this object role.', related_name='has_roles', through='dab_rbac.RoleUserAssignment', through_fields=('object_role', 'user'), to=settings.AUTH_USER_MODEL),
),
]
13 changes: 10 additions & 3 deletions ansible_base/resource_registry/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@ class CustomForwardOneToOneDescriptor(ForwardOneToOneDescriptor):
def get_queryset(self, **hints):
return self.field.remote_field.model._base_manager.db_manager(hints=hints).filter(content_type=ContentType.objects.get_for_model(self.field.model))

def get_prefetch_queryset(self, instances, queryset=None):
if not queryset:
queryset = self.get_queryset()
def get_prefetch_querysets(self, instances, querysets=None):
if querysets and len(querysets) != 1:
raise ValueError("querysets argument of get_prefetch_querysets() should have a length " "of 1.")
queryset = querysets[0] if querysets else self.get_queryset()
queryset._add_hints(instance=instances[0])

query = models.Q.create(
Expand Down Expand Up @@ -47,6 +48,12 @@ def get_prefetch_queryset(self, instances, queryset=None):
False,
)

def get_prefetch_queryset(self, instances, queryset=None):
# Django 4 compatibility: renamed to get_prefetch_querysets in Django 5
if queryset is None:
return self.get_prefetch_querysets(instances)
return self.get_prefetch_querysets(instances, [queryset])
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Prefetch Queryset Compatibility Issue

The get_prefetch_queryset compatibility method changed its check from if not queryset to if queryset is None. This alters behavior when a falsy but non-None value, like an empty list, is passed as queryset. Instead of generating a default queryset as before, the method now passes the falsy value directly, which can lead to an empty queryset being used where a default was expected, potentially breaking prefetch functionality.

Fix in Cursor Fix in Web



class AnsibleResourceField(models.ForeignObject):
"""
Expand Down
29 changes: 28 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,11 @@ legacy_tox_ini = """
py310
py311
py311sqlite
py310-django4
py311-django4
py312-django4
labels =
test = py312, py310, py311, py311sqlite, check
test = py312, py310, py311, py311sqlite, py310-django4, py311-django4, py312-django4, check
lint = flake8, black, isort
[testenv]
Expand All @@ -128,6 +131,30 @@ legacy_tox_ini = """
setenv =
DJANGO_SETTINGS_MODULE = test_app.sqlite3settings
[testenv:py310-django4]
deps =
-r{toxinidir}/requirements/requirements_all.txt
-r{toxinidir}/requirements/requirements_dev.txt
commands_pre =
python -m pip install --upgrade "Django>=4.2.21,<4.3.0"
docker = db
[testenv:py311-django4]
deps =
-r{toxinidir}/requirements/requirements_all.txt
-r{toxinidir}/requirements/requirements_dev.txt
commands_pre =
python -m pip install --upgrade "Django>=4.2.21,<4.3.0"
docker = db
[testenv:py312-django4]
deps =
-r{toxinidir}/requirements/requirements_all.txt
-r{toxinidir}/requirements/requirements_dev.txt
commands_pre =
python -m pip install --upgrade "Django>=4.2.21,<4.3.0"
docker = db
[docker:db]
dockerfile = {toxinidir}/tools/dev_postgres/Dockerfile
expose =
Expand Down
2 changes: 1 addition & 1 deletion requirements/requirements.in
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
# if you are add a new feature which requires dependencies they should be in a separate requirements_<feature>.in file
#
cryptography
Django>=4.2.21,<4.3.0 # CVE-2024-45230, CVE-2024-56374
Django>=4.2.21,<6.0
djangorestframework
django-crum
inflection
Expand Down
2 changes: 1 addition & 1 deletion requirements/requirements_all.txt
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ defusedxml==0.8.0rc2
# via
# python3-openid
# social-auth-core
django==4.2.21
django==5.2.7
# via
# -r requirements/requirements.in
# channels
Expand Down
2 changes: 1 addition & 1 deletion requirements/requirements_dev.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
ansible # Used in build process to generate some configs
black==25.1.0 # Linting tool, if changed update pyproject.toml as well
build
django==4.2.21
django==5.2.7
django-debug-toolbar
django-extensions
djangorestframework
Expand Down
8 changes: 8 additions & 0 deletions requirements/updater.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,14 @@ set -ue

PYTHON=python3.11

# Ensure script is run from the requirements/ directory
if [[ "$(basename $(pwd))" != "requirements" ]]; then
echo "ERROR: This script must be run from the requirements/ directory"
echo "Current directory: $(pwd)"
echo "Please run: cd requirements && ./updater.sh [run|upgrade]"
exit 1
fi

for FILE in requirements.in requirements_all.txt ; do
if [ ! -f ${FILE} ] ; then
touch ${FILE}
Expand Down
47 changes: 46 additions & 1 deletion test_app/tests/resource_registry/models/test_resource_field.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from django.contrib.contenttypes.models import ContentType

from ansible_base.resource_registry.models import Resource
from test_app.models import Organization
from test_app.models import Inventory, Organization


@pytest.mark.django_db
Expand Down Expand Up @@ -100,3 +100,48 @@ def test_resource_field_filtering(organization):

org = Organization.objects.get(resource__name=organization.name)
assert org.resource.pk == resource.pk


@pytest.mark.django_db
def test_resource_field_prefetch_related_across_foreign_key(organization, organization_1, organization_2, django_assert_num_queries):
"""
Generated by Claude Code (claude-sonnet-4-5@20250929)
Test that prefetch_related works across a ForeignKey to a model with a resource field.
This tests prefetching organization__resource on Inventory objects.
"""
# Create inventory objects linked to organizations
Inventory.objects.create(name="Inventory 1", organization=organization)
Inventory.objects.create(name="Inventory 2", organization=organization_1)
Inventory.objects.create(name="Inventory 3", organization=organization_2)

org_ctype = ContentType.objects.get_for_model(Organization)

# Prefetch organization__resource should result in 3 queries:
# 1. Fetch all Inventory objects
# 2. Fetch all related Organization objects
# 3. Fetch all related Resource objects
with django_assert_num_queries(3) as captured:
inventory_qs = list(Inventory.objects.prefetch_related("organization__resource").all())

# Verify the queries were as expected
assert "test_app_inventory" in captured[0]["sql"]
assert "test_app_organization" in captured[1]["sql"]
assert "dab_resource_registry_resource" in captured[2]["sql"]

assert len(inventory_qs) == 3

# Collect resource pks for later verification
resource_pks = {}
with django_assert_num_queries(0):
for inv in inventory_qs:
assert inv.organization is not None
assert inv.organization.resource is not None
# Verify the resource data is correct
assert inv.organization.name == inv.organization.resource.name
assert str(inv.organization.pk) == inv.organization.resource.object_id
resource_pks[inv.organization.pk] = inv.organization.resource.pk

# Verify the resources match what's in the database
for org_pk, resource_pk in resource_pks.items():
expected_resource = Resource.objects.get(object_id=org_pk, content_type=org_ctype)
assert resource_pk == expected_resource.pk