Skip to content

Commit f2438a0

Browse files
authored
Fix server error from PATCH to inventory source (#16274)
Fix server error from PATCH to inventory source, co-authored with Claude opus 4.6
1 parent 707f2fa commit f2438a0

File tree

3 files changed

+70
-1
lines changed

3 files changed

+70
-1
lines changed

awx/api/fields.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ def __init__(self, **kwargs):
8989
def to_internal_value(self, pk):
9090
try:
9191
pk = int(pk)
92-
except ValueError:
92+
except (ValueError, TypeError):
9393
self.fail('invalid')
9494
try:
9595
Credential.objects.get(pk=pk)

awx/main/tests/functional/api/test_inventory.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -463,6 +463,26 @@ def test_validating_credential_type(self, organization, inventory, admin_user, p
463463
assert 'Cloud-based inventory sources (such as ec2)' in r.data['credential'][0]
464464
assert 'require credentials for the matching cloud service' in r.data['credential'][0]
465465

466+
def test_credential_dict_value_returns_400(self, inventory, admin_user, put):
467+
"""Passing a dict for the credential field should return 400, not 500.
468+
469+
Reproduces a bug where int() raises TypeError on non-scalar types
470+
(dict, list) which was uncaught, resulting in a 500 Internal Server Error.
471+
"""
472+
inv_src = InventorySource.objects.create(name='test-src', inventory=inventory, source='ec2')
473+
r = put(
474+
url=reverse('api:inventory_source_detail', kwargs={'pk': inv_src.pk}),
475+
data={
476+
'name': 'test-src',
477+
'inventory': inventory.pk,
478+
'source': 'ec2',
479+
'credential': {'username': 'admin', 'password': 'secret'},
480+
},
481+
user=admin_user,
482+
expect=400,
483+
)
484+
assert r.status_code == 400
485+
466486
def test_vault_credential_not_allowed(self, project, inventory, vault_credential, admin_user, post):
467487
"""Vault credentials cannot be associated via the deprecated field"""
468488
# TODO: when feature is added, add tests to use the related credentials
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
import pytest
2+
from collections import OrderedDict
3+
from unittest import mock
4+
5+
from rest_framework.exceptions import ValidationError
6+
7+
from awx.api.fields import DeprecatedCredentialField
8+
9+
10+
class TestDeprecatedCredentialField:
11+
"""Test that DeprecatedCredentialField handles unexpected input types gracefully."""
12+
13+
def test_dict_value_raises_validation_error(self):
14+
"""Passing a dict instead of an integer should return a 400 validation error, not a 500 TypeError."""
15+
field = DeprecatedCredentialField()
16+
with pytest.raises(ValidationError):
17+
field.to_internal_value({"username": "admin", "password": "secret"})
18+
19+
def test_ordered_dict_value_raises_validation_error(self):
20+
"""Passing an OrderedDict should return a 400 validation error, not a 500 TypeError."""
21+
field = DeprecatedCredentialField()
22+
with pytest.raises(ValidationError):
23+
field.to_internal_value(OrderedDict([("username", "admin")]))
24+
25+
def test_list_value_raises_validation_error(self):
26+
"""Passing a list should return a 400 validation error, not a 500 TypeError."""
27+
field = DeprecatedCredentialField()
28+
with pytest.raises(ValidationError):
29+
field.to_internal_value([1, 2, 3])
30+
31+
def test_string_value_raises_validation_error(self):
32+
"""Passing a non-numeric string should return a 400 validation error."""
33+
field = DeprecatedCredentialField()
34+
with pytest.raises(ValidationError):
35+
field.to_internal_value("not_a_number")
36+
37+
@mock.patch('awx.api.fields.Credential.objects')
38+
def test_valid_integer_value_works(self, mock_cred_objects):
39+
"""Passing a valid integer PK should work when the credential exists."""
40+
mock_cred_objects.get.return_value = mock.MagicMock()
41+
field = DeprecatedCredentialField()
42+
assert field.to_internal_value(42) == 42
43+
44+
@mock.patch('awx.api.fields.Credential.objects')
45+
def test_valid_string_integer_value_works(self, mock_cred_objects):
46+
"""Passing a numeric string PK should work when the credential exists."""
47+
mock_cred_objects.get.return_value = mock.MagicMock()
48+
field = DeprecatedCredentialField()
49+
assert field.to_internal_value("42") == 42

0 commit comments

Comments
 (0)