Skip to content

Conversation

arrestle
Copy link
Contributor

@arrestle arrestle commented Sep 8, 2025

AAP-52518: Fix Role User Assignment Cleanup on Inventory Deletion

SUMMARY

Fixed role assignment orphaning issue where RoleUserAssignment records were not properly cleaned up when Controller inventories were deleted, causing 404 errors in the UI when users attempted to access role assignments for deleted inventories.

Root Cause: RBAC post-delete signals (rbac_post_delete_remove_object_roles) were not reliably cleaning up ObjectRole records during inventory deletion, particularly in async Celery task contexts. Since RoleUserAssignment records cascade delete via foreign key to ObjectRole, when ObjectRole cleanup failed, the assignments remained orphaned.

Solution: Implemented defense-in-depth explicit role cleanup in both deletion paths:

  • Async path: Enhanced delete_inventory() Celery task in system.py
  • Sync path: Enhanced Inventory.delete() model method in inventory.py

This ensures role assignment cleanup works regardless of deletion context, signal reliability, or process isolation issues.

Related AAP-52518

ISSUE TYPE
  • Bug, Docs Fix or other nominal change
COMPONENT NAME
  • API
ADDITIONAL INFORMATION

Files Modified:

  1. awx/awx/main/models/inventory.py - Added explicit ObjectRole cleanup in delete() method
  2. awx/awx/main/tasks/system.py - Added defense-in-depth cleanup in delete_inventory() task
  3. awx/awx/main/tests/functional/dab_rbac/test_inventory_role_cleanup.py - Comprehensive test suite

Reproduction Steps (Before Fix):

  1. Create organization and user
  2. Create Controller inventory
  3. Assign user as "Inventory Admin"
  4. Delete inventory
  5. Check role_user_assignments API - orphaned records remain
  6. UI: Access Management > Users > [User] > Roles > [Assignment] → 404 Not Found

Expected Behavior (After Fix):

  • Role assignments automatically deleted with inventory
  • Clean role_user_assignments API response
  • No 404 errors in UI when browsing user roles
  • No orphaned database records

Test Coverage:

# Run specific tests for this fix
make test TEST_DIRS=awx/main/tests/functional/dab_rbac/test_inventory_role_cleanup.py

# Tests cover:
- Synchronous deletion cleanup
- Asynchronous deletion cleanup (Celery task)  
- Multiple role assignments per inventory
- Multiple users with roles on same inventory
- Other inventories unaffected by deletion
- Exact AAP-52518 scenario reproduction

Technical Implementation:

  • inventory.py: Explicit ObjectRole.objects.filter(content_type=ct, object_id=self.pk).delete() before super().delete()
  • system.py: Explicit cleanup before calling inventory.delete() in Celery task
  • Cascade deletion: RoleUserAssignment records automatically deleted via FK constraint
  • Error handling: Cleanup failures logged but don't prevent inventory deletion
  • Idempotent: Safe to run multiple times, no side effects

Verification:

# Before fix: Orphaned assignments remain
RoleUserAssignment.objects.filter(object_id=str(deleted_inventory_id)).count()  # > 0

# After fix: Clean state  
RoleUserAssignment.objects.filter(object_id=str(deleted_inventory_id)).count()  # = 0

Copy link

sonarqubecloud bot commented Sep 8, 2025

Copy link

codecov bot commented Sep 8, 2025

⚠️ Parser warning

The parser emitted a warning. Please review your JUnit XML file:
Warning while parsing testcase attributes: Limit of string is 1000 chars, for name, we got 1931 at 20989:26 in /home/runner/work/awx/awx/reports/junit.xml

For more help, visit our troubleshooting guide.


except Exception as e:
# Log the error but don't prevent inventory deletion
logger.warning(f"Failed to clean up role assignments for inventory " f"{self.pk}: {e}")
Copy link
Member

Choose a reason for hiding this comment

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

I would consider this to be redundant with the existing signals.

But for the background task, that makes sense to me.

@ansible ansible deleted a comment from cursor bot Sep 12, 2025
@AlanCoding
Copy link
Member

Based on updated information I don't expect this is needed anymore.

@AlanCoding AlanCoding closed this Sep 16, 2025
@arrestle arrestle deleted the aap-52513-delete-role-with-inventory branch September 22, 2025 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants