improve error formatting for jsonschema failures on Credential.inputs

this provides error messages keyed by input fields, so that instead of
e.g.,

{
    'inputs': ['Invalid certificate or key: u'XYZ']
}

...you get:

{
    'inputs': {
        'ssh_key_data': ['Invalid certificate or key: u'XYZ']
    }
}

Includes /api/v1/ compatability for error message format.  Requests to
/api/v1/ will get:

{'ssh_key_data': ['Invalid certificate or key: u'XYZ']}
This commit is contained in:
Ryan Petrello
2017-06-01 09:14:12 -04:00
parent 98fa654be2
commit e0a629db58
4 changed files with 68 additions and 35 deletions

View File

@@ -2087,7 +2087,19 @@ class CredentialSerializerCreate(CredentialSerializer):
attrs.pop(field) attrs.pop(field)
if not owner_fields: if not owner_fields:
raise serializers.ValidationError({"detail": _("Missing 'user', 'team', or 'organization'.")}) raise serializers.ValidationError({"detail": _("Missing 'user', 'team', or 'organization'.")})
return super(CredentialSerializerCreate, self).validate(attrs) try:
return super(CredentialSerializerCreate, self).validate(attrs)
except ValidationError as e:
# TODO: remove when API v1 is removed
# If we have an `inputs` error on `/api/v1/`:
# {'inputs': {'username': [...]}}
# ...instead, send back:
# {'username': [...]}
if self.version == 1 and isinstance(e.detail.get('inputs'), dict):
e.detail = e.detail['inputs']
raise e
else:
raise
def create(self, validated_data): def create(self, validated_data):
user = validated_data.pop('user', None) user = validated_data.pop('user', None)

View File

@@ -35,6 +35,9 @@ import jsonschema.exceptions
from jsonfield import JSONField as upstream_JSONField from jsonfield import JSONField as upstream_JSONField
from jsonbfield.fields import JSONField as upstream_JSONBField from jsonbfield.fields import JSONField as upstream_JSONBField
# DRF
from rest_framework import serializers
# AWX # AWX
from awx.main.utils.filters import SmartFilter from awx.main.utils.filters import SmartFilter
from awx.main.validators import validate_ssh_private_key from awx.main.validators import validate_ssh_private_key
@@ -410,7 +413,7 @@ def format_ssh_private_key(value):
value = value.replace(r'\u003d', '=') value = value.replace(r'\u003d', '=')
try: try:
validate_ssh_private_key(value) validate_ssh_private_key(value)
except jsonschema.exceptions.ValidationError as e: except django_exceptions.ValidationError as e:
raise jsonschema.exceptions.FormatError(e.message) raise jsonschema.exceptions.FormatError(e.message)
return value return value
@@ -445,7 +448,7 @@ class CredentialInputField(JSONSchemaField):
properties = {} properties = {}
for field in model_instance.credential_type.inputs.get('fields', []): for field in model_instance.credential_type.inputs.get('fields', []):
field = field.copy() field = field.copy()
properties[field.pop('id')] = field properties[field['id']] = field
return { return {
'type': 'object', 'type': 'object',
'properties': properties, 'properties': properties,
@@ -471,37 +474,43 @@ class CredentialInputField(JSONSchemaField):
else: else:
decrypted_values[k] = v decrypted_values[k] = v
super(CredentialInputField, self).validate(decrypted_values, super(JSONSchemaField, self).validate(decrypted_values, model_instance)
model_instance) errors = {}
for error in Draft4Validator(
self.schema(model_instance),
format_checker=self.format_checker
).iter_errors(decrypted_values):
if error.validator == 'pattern' and 'error' in error.schema:
error.message = error.schema['error'] % error.instance
if 'id' not in error.schema:
# If the error is not for a specific field, it's specific to
# `inputs` in general
raise django_exceptions.ValidationError(
error.message,
code='invalid',
params={'value': value},
)
errors[error.schema['id']] = [error.message]
errors = []
inputs = model_instance.credential_type.inputs inputs = model_instance.credential_type.inputs
for field in inputs.get('required', []): for field in inputs.get('required', []):
if not value.get(field, None): if not value.get(field, None):
errors.append( errors[field] = [_('required for %s') % (
_('%s required for %s credential.') % ( model_instance.credential_type.name
field, model_instance.credential_type.name )]
)
)
# `ssh_key_unlock` requirements are very specific and can't be # `ssh_key_unlock` requirements are very specific and can't be
# represented without complicated JSON schema # represented without complicated JSON schema
if model_instance.credential_type.kind == 'ssh': if model_instance.credential_type.kind == 'ssh':
if model_instance.has_encrypted_ssh_key_data and not value.get('ssh_key_unlock'): if model_instance.has_encrypted_ssh_key_data and not value.get('ssh_key_unlock'):
errors.append( errors['ssh_key_unlock'] = [_('must be set when SSH key is encrypted.')]
_('SSH key unlock must be set when SSH key is encrypted.')
)
if not model_instance.has_encrypted_ssh_key_data and value.get('ssh_key_unlock'): if not model_instance.has_encrypted_ssh_key_data and value.get('ssh_key_unlock'):
errors.append( errors['ssh_key_unlock'] = [_('should not be set when SSH key is not encrypted.')]
_('SSH key unlock should not be set when SSH key is not encrypted.')
)
if errors: if errors:
raise django_exceptions.ValidationError( raise serializers.ValidationError({
errors, 'inputs': errors
code='invalid', })
params={'value': value},
)
class CredentialTypeInputField(JSONSchemaField): class CredentialTypeInputField(JSONSchemaField):

View File

@@ -1,4 +1,3 @@
import json
import mock # noqa import mock # noqa
import pytest import pytest
@@ -636,7 +635,7 @@ def test_inputs_cannot_contain_extra_fields(get, post, organization, admin, cred
admin admin
) )
assert response.status_code == 400 assert response.status_code == 400
assert "'invalid_field' was unexpected" in json.dumps(response.data) assert "'invalid_field' was unexpected" in response.data['inputs'][0]
# #
@@ -1032,8 +1031,11 @@ def test_aws_create_fail_required_fields(post, organization, admin, version, par
assert response.status_code == 400 assert response.status_code == 400
assert Credential.objects.count() == 0 assert Credential.objects.count() == 0
assert 'username' in json.dumps(response.data) errors = response.data
assert 'password' in json.dumps(response.data) if version == 'v2':
errors = response.data['inputs']
assert errors['username'] == ['required for %s' % aws.name]
assert errors['password'] == ['required for %s' % aws.name]
# #
@@ -1100,9 +1102,12 @@ def test_vmware_create_fail_required_fields(post, organization, admin, version,
assert response.status_code == 400 assert response.status_code == 400
assert Credential.objects.count() == 0 assert Credential.objects.count() == 0
assert 'username' in json.dumps(response.data) errors = response.data
assert 'password' in json.dumps(response.data) if version == 'v2':
assert 'host' in json.dumps(response.data) errors = response.data['inputs']
assert errors['username'] == ['required for %s' % vmware.name]
assert errors['password'] == ['required for %s' % vmware.name]
assert errors['host'] == ['required for %s' % vmware.name]
# #
@@ -1160,10 +1165,13 @@ def test_openstack_create_fail_required_fields(post, organization, admin, versio
admin admin
) )
assert response.status_code == 400 assert response.status_code == 400
assert 'username' in json.dumps(response.data) errors = response.data
assert 'password' in json.dumps(response.data) if version == 'v2':
assert 'host' in json.dumps(response.data) errors = response.data['inputs']
assert 'project' in json.dumps(response.data) assert errors['username'] == ['required for %s' % openstack.name]
assert errors['password'] == ['required for %s' % openstack.name]
assert errors['host'] == ['required for %s' % openstack.name]
assert errors['project'] == ['required for %s' % openstack.name]
@pytest.mark.django_db @pytest.mark.django_db

View File

@@ -7,6 +7,8 @@ from django.core.exceptions import ValidationError
from awx.main.utils.common import decrypt_field from awx.main.utils.common import decrypt_field
from awx.main.models import Credential, CredentialType from awx.main.models import Credential, CredentialType
from rest_framework import serializers
EXAMPLE_PRIVATE_KEY = '-----BEGIN PRIVATE KEY-----\nxyz==\n-----END PRIVATE KEY-----' EXAMPLE_PRIVATE_KEY = '-----BEGIN PRIVATE KEY-----\nxyz==\n-----END PRIVATE KEY-----'
EXAMPLE_ENCRYPTED_PRIVATE_KEY = '-----BEGIN PRIVATE KEY-----\nProc-Type: 4,ENCRYPTED\nxyz==\n-----END PRIVATE KEY-----' EXAMPLE_ENCRYPTED_PRIVATE_KEY = '-----BEGIN PRIVATE KEY-----\nProc-Type: 4,ENCRYPTED\nxyz==\n-----END PRIVATE KEY-----'
@@ -86,8 +88,9 @@ def test_cred_type_input_schema_validity(input_, valid):
inputs=input_ inputs=input_
) )
if valid is False: if valid is False:
with pytest.raises(ValidationError): with pytest.raises(Exception) as e:
type_.full_clean() type_.full_clean()
assert e.type in (ValidationError, serializers.ValidationError)
else: else:
type_.full_clean() type_.full_clean()
@@ -216,8 +219,9 @@ def test_ssh_key_data_validation(credentialtype_ssh, organization, ssh_key_data,
if valid: if valid:
cred.full_clean() cred.full_clean()
else: else:
with pytest.raises(ValidationError): with pytest.raises(Exception) as e:
cred.full_clean() cred.full_clean()
assert e.type in (ValidationError, serializers.ValidationError)
@pytest.mark.django_db @pytest.mark.django_db