Skip to content

Commit 3058952

Browse files
john-westcott-ivClaude
andauthored
[AAP-51620] Enhance UUID-based test to handle edge case where UUID ends with PK digits (#805)
- Enhanced test_user_access_list_related_links_use_ansible_id with more robust URL validation - Enhanced test_team_access_list_related_links_use_ansible_id with same improvements - Added test_user_access_list_uuid_ending_with_pk_digits to specifically test edge case - Fixed issue where test would fail if UUID happened to end with same digits as user's primary key - Tests now extract and compare full URL segments instead of using endswith() checks - Added detailed error messages for better debugging ## Description <!-- Mandatory: Provide a clear, concise description of the changes and their purpose --> - What is being changed? The test has a random chance to fail if the UUID of the user ends with the user ID. - Why is this change needed? Because the test intermittently fails depending on ID and UUID generation. - How does this change address the issue? Does a more strict compare. ## Type of Change <!-- Mandatory: Check one or more boxes that apply --> - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] Documentation update - [X] Test update - [ ] Refactoring (no functional changes) - [ ] Development environment change - [ ] Configuration change ## Self-Review Checklist <!-- These items help ensure quality - they complement our automated CI checks --> - [x] I have performed a self-review of my code - [x] I have added relevant comments to complex code sections - [X] I have updated documentation where needed - [x] I have considered the security impact of these changes - [x] I have considered performance implications - [x] I have thought about error handling and edge cases - [X] I have tested the changes in my local environment ## Testing Instructions <!-- Optional for test-only changes. Mandatory for all other changes --> <!-- Must be detailed enough for reviewers to reproduce --> ### Prerequisites <!-- List any specific setup required --> ### Steps to Test 1. 2. 3. ### Expected Results <!-- Describe what should happen after following the steps --> ## Additional Context <!-- Optional but helpful information --> ### Required Actions <!-- Check if changes require work in other areas --> <!-- Remove section if no external actions needed --> - [ ] Requires documentation updates <!-- API docs, feature docs, deployment guides --> - [ ] Requires downstream repository changes <!-- Specify repos: django-ansible-base, eda-server, etc. --> - [ ] Requires infrastructure/deployment changes <!-- CI/CD, installer updates, new services --> - [ ] Requires coordination with other teams <!-- UI team, platform services, infrastructure --> - [ ] Blocked by PR/MR: #XXX <!-- Reference blocking PRs/MRs with brief context --> ### Screenshots/Logs <!-- Add if relevant to demonstrate the changes --> Co-authored-by: Claude <[email protected]>
1 parent dd97ad4 commit 3058952

File tree

1 file changed

+82
-2
lines changed

1 file changed

+82
-2
lines changed

test_app/tests/rbac/api/test_access_assignment_uuid_lookup.py

Lines changed: 82 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,9 +123,23 @@ def test_user_access_list_related_links_use_ansible_id(self, admin_api_client, i
123123
# The details URL should use ansible_id instead of primary key
124124
expected_ansible_id = str(user.resource.ansible_id)
125125
assert expected_ansible_id in user_data['related']['details']
126+
126127
# Check that the URL ends with the ansible_id, not the primary key
127128
assert user_data['related']['details'].endswith(f'{expected_ansible_id}/')
128-
assert not user_data['related']['details'].endswith(f'{user.pk}/')
129+
130+
# More robust check: ensure the URL contains the full UUID, not just ends with PK digits
131+
# This handles the edge case where UUID might end with the same digits as the PK
132+
details_url = user_data['related']['details']
133+
134+
# Extract the last path segment (should be the ansible_id)
135+
url_parts = details_url.rstrip('/').split('/')
136+
last_segment = url_parts[-1]
137+
138+
# The last segment should be the full UUID, not just the PK
139+
assert last_segment == expected_ansible_id, f"Expected UUID {expected_ansible_id}, but URL ends with {last_segment}"
140+
141+
# Additional check: ensure it's not just the primary key
142+
assert last_segment != str(user.pk), f"URL incorrectly uses primary key {user.pk} instead of ansible_id"
129143

130144
def test_team_access_list_related_links_use_ansible_id(self, admin_api_client, inventory, inv_rd, organization):
131145
"""Test that team access list provides related links with ansible_id"""
@@ -150,9 +164,75 @@ def test_team_access_list_related_links_use_ansible_id(self, admin_api_client, i
150164
# The details URL should use ansible_id instead of primary key
151165
expected_ansible_id = str(team.resource.ansible_id)
152166
assert expected_ansible_id in team_data['related']['details']
167+
153168
# Check that the URL ends with the ansible_id, not the primary key
154169
assert team_data['related']['details'].endswith(f'{expected_ansible_id}/')
155-
assert not team_data['related']['details'].endswith(f'/{team.pk}/')
170+
171+
# More robust check: ensure the URL contains the full UUID, not just ends with PK digits
172+
# This handles the edge case where UUID might end with the same digits as the PK
173+
details_url = team_data['related']['details']
174+
175+
# Extract the last path segment (should be the ansible_id)
176+
url_parts = details_url.rstrip('/').split('/')
177+
last_segment = url_parts[-1]
178+
179+
# The last segment should be the full UUID, not just the PK
180+
assert last_segment == expected_ansible_id, f"Expected UUID {expected_ansible_id}, but URL ends with {last_segment}"
181+
182+
# Additional check: ensure it's not just the primary key
183+
assert last_segment != str(team.pk), f"URL incorrectly uses primary key {team.pk} instead of ansible_id"
184+
185+
def test_user_access_list_uuid_ending_with_pk_digits(self, admin_api_client, inventory, inv_rd):
186+
"""Test edge case where UUID happens to end with the same digits as the user's PK"""
187+
# Create multiple users to increase the chance of getting a higher PK
188+
for i in range(10):
189+
User.objects.create(username=f'dummy-user-{i}')
190+
191+
user = User.objects.create(username='test-user-edge-case')
192+
inv_rd.give_permission(user, inventory)
193+
194+
# Get the user's UUID and PK
195+
expected_ansible_id = str(user.resource.ansible_id)
196+
user_pk = str(user.pk)
197+
198+
# This test specifically validates the scenario mentioned in the bug report
199+
# Even if UUID ends with PK digits, the test should correctly identify the full UUID
200+
201+
url = get_relative_url('role-user-access', kwargs={'pk': inventory.pk, 'model_name': 'aap.inventory'})
202+
response = admin_api_client.get(url)
203+
assert response.status_code == 200
204+
205+
# Find our test user in the results
206+
user_data = None
207+
for user_detail in response.data['results']:
208+
if user_detail['username'] == 'test-user-edge-case':
209+
user_data = user_detail
210+
break
211+
212+
assert user_data is not None
213+
assert 'related' in user_data
214+
assert 'details' in user_data['related']
215+
216+
details_url = user_data['related']['details']
217+
218+
# Extract the last path segment (should be the ansible_id)
219+
url_parts = details_url.rstrip('/').split('/')
220+
last_segment = url_parts[-1]
221+
222+
# The critical test: last segment should be the full UUID, not just the PK
223+
# This should pass even if the UUID ends with the same digits as the PK
224+
assert last_segment == expected_ansible_id, (
225+
f"URL should end with full UUID {expected_ansible_id}, not PK {user_pk}. " f"Actual last segment: {last_segment}. Full URL: {details_url}"
226+
)
227+
228+
# Ensure the full UUID is present in the URL
229+
assert expected_ansible_id in details_url
230+
231+
# Additional validation: if UUID ends with PK digits, ensure we're still using the full UUID
232+
if expected_ansible_id.endswith(user_pk):
233+
# This is the problematic case that was failing
234+
# The URL should still contain the full UUID, not just end with PK
235+
assert len(last_segment) > len(user_pk), f"UUID {expected_ansible_id} ends with PK {user_pk}, but URL should use full UUID, not just PK"
156236

157237
def test_user_access_list_fallback_to_pk_when_no_resource(self, admin_api_client, inventory, inv_rd):
158238
"""Test fallback to primary key when user has no resource (edge case)"""

0 commit comments

Comments
 (0)