airship
/
deckhand
Code Issues Proposed changes

Prevent same DataSchema from being used more than once for validation. 

This PS prevents the same DataSchema from being used more than once
for validation. Otherwise the list of errors generated will be
duplicated.

Change-Id: I4eb1d33cdbe084ddea522b2c9ea91d507de4ca23
This commit is contained in:
Felipe Monteiro 2017-10-23 23:26:52 +01:00 committed by Scott Hussey
parent 1ef4a78f02
commit 52a9632e40
5 changed files with 82 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
deckhand/control/validations.py
View File

@ -80,7 +80,7 @@ class ValidationsResource(api_base.BaseResource):
try:
entry = db_api.validation_get_entry(
revision_id, validation_name, entry_id)
except errors.RevisionNotFound as e:
except (errors.RevisionNotFound, errors.ValidationNotFound) as e:
raise falcon.HTTPNotFound(description=e.format_message())
resp_body = self.view_builder.show_entry(entry)

7
deckhand/db/sqlalchemy/api.py
View File

@ -1100,4 +1100,9 @@ def validation_get_entry(revision_id, val_name, entry_id, session=None):
session = session or get_session()
entries = validation_get_all_entries(
revision_id, val_name, session=session)
return entries[entry_id]
try:
return entries[entry_id]
except IndexError:
raise errors.ValidationNotFound(
revision_id=revision_id, validation_name=val_name,
entry_id=entry_id)

6
deckhand/engine/document_validation.py
View File

@ -113,7 +113,8 @@ class DocumentValidation(object):
# Can't use `startswith` below to avoid namespace false
# positives like `CertificateKey` and `Certificate`.
if schema_id == schema['id']:
matching_schemas.append(schema)
if schema not in matching_schemas:
matching_schemas.append(schema)
return matching_schemas
@classmethod
@ -201,6 +202,7 @@ class DocumentValidation(object):
LOG.info('Skipping schema validation for abstract '
'document: %s.', raw_dict)
else:
for schema_to_use in schemas_to_use:
try:
if isinstance(schema_to_use['schema'], dict):
@ -217,7 +219,7 @@ class DocumentValidation(object):
result['errors'].append({
'schema': document.get_schema(),
'name': document.get_name(),
'message': e.message.replace('\\', '')
'message': e.message.replace('u\'', '\'')
})
if result['errors']:

19
deckhand/errors.py
View File

@ -196,12 +196,6 @@ class SingletonDocumentConflict(DeckhandException):
code = 409
class LayeringPolicyNotFound(DeckhandException):
msg_fmt = ("LayeringPolicy with schema %(schema)s not found in the "
"system.")
code = 400
class LayeringPolicyMalformed(DeckhandException):
msg_fmt = ("LayeringPolicy with schema %(schema)s is improperly formatted:"
" %(document)s.")
@ -239,6 +233,12 @@ class DocumentNotFound(DeckhandException):
code = 404
class LayeringPolicyNotFound(DeckhandException):
msg_fmt = ("LayeringPolicy with schema %(schema)s not found in the "
"system.")
code = 404
class RevisionNotFound(DeckhandException):
msg_fmt = "The requested revision %(revision)s was not found."
code = 404
@ -250,6 +250,13 @@ class RevisionTagNotFound(DeckhandException):
code = 404
class ValidationNotFound(DeckhandException):
msg_fmt = ("The requested validation entry %(entry_id)s was not found "
"for validation name %(validation_name)s and revision ID "
"%(revision_id)s.")
code = 404
class RevisionTagBadFormat(DeckhandException):
msg_fmt = ("The requested tag data %(data)s must either be null or "
"dictionary.")

65
deckhand/tests/unit/control/test_validations_controller.py
View File

@ -251,10 +251,10 @@ class TestValidationsController(test_base.BaseControllerTest):
revision_id = self._create_revision()
validation_name = test_utils.rand_name('validation')
resp = self._create_validation(revision_id, validation_name,
VALIDATION_RESULT)
resp = resp = self._create_validation(revision_id, validation_name,
VALIDATION_RESULT_ALT)
self._create_validation(revision_id, validation_name,
VALIDATION_RESULT)
self._create_validation(revision_id, validation_name,
VALIDATION_RESULT_ALT)
resp = self.app.simulate_get(
'/api/v1.0/revisions/%s/validations/%s' % (revision_id,
@ -279,8 +279,8 @@ class TestValidationsController(test_base.BaseControllerTest):
revision_id = self._create_revision()
validation_name = test_utils.rand_name('validation')
resp = resp = self._create_validation(revision_id, validation_name,
VALIDATION_RESULT)
resp = self._create_validation(revision_id, validation_name,
VALIDATION_RESULT)
resp = self.app.simulate_get(
'/api/v1.0/revisions/%s/validations/%s/0' % (revision_id,
@ -312,6 +312,28 @@ class TestValidationsController(test_base.BaseControllerTest):
}
self.assertEqual(expected_body, body)
def test_show_nonexistent_validation_entry_returns_404(self):
rules = {'deckhand:create_cleartext_documents': '@',
'deckhand:create_validation': '@',
'deckhand:show_validation': '@'}
self.policy.set_rules(rules)
revision_id = self._create_revision()
validation_name = test_utils.rand_name('validation')
resp = self._create_validation(revision_id, validation_name,
VALIDATION_RESULT)
self.assertEqual(201, resp.status_code)
expected_error = ('The requested validation entry 5 was not found for '
'validation name %s and revision ID %d.' % (
validation_name, revision_id))
resp = self.app.simulate_get(
'/api/v1.0/revisions/%s/validations/%s/5' % (revision_id,
validation_name),
headers={'Content-Type': 'application/x-yaml'})
self.assertEqual(404, resp.status_code)
self.assertEqual(expected_error, yaml.safe_load(resp.text)['message'])
def test_validation_with_registered_data_schema(self):
rules = {'deckhand:create_cleartext_documents': '@',
'deckhand:list_validations': '@'}
@ -441,7 +463,8 @@ class TestValidationsController(test_base.BaseControllerTest):
def test_validation_with_registered_data_schema_expect_mixed(self):
rules = {'deckhand:create_cleartext_documents': '@',
'deckhand:list_validations': '@'}
'deckhand:list_validations': '@',
'deckhand:show_validation': '@'}
self.policy.set_rules(rules)
# Register a `DataSchema` against which the test document will be
@ -505,6 +528,34 @@ class TestValidationsController(test_base.BaseControllerTest):
}
self.assertEqual(expected_body, body)
resp = self.app.simulate_get(
'/api/v1.0/revisions/%s/validations/%s' % (
revision_id, types.DECKHAND_SCHEMA_VALIDATION),
headers={'Content-Type': 'application/x-yaml'})
self.assertEqual(200, resp.status_code)
body = yaml.safe_load(resp.text)
expected_body = {
'count': 2,
'results': [{'id': 0, 'status': 'failure'}, # fail_doc failed.
{'id': 1, 'status': 'success'}] # pass_doc succeeded.
}
self.assertEqual(expected_body, body)
# Validate that fail_doc validation failed for the expected reason.
resp = self.app.simulate_get(
'/api/v1.0/revisions/%s/validations/%s/0' % (
revision_id, types.DECKHAND_SCHEMA_VALIDATION),
headers={'Content-Type': 'application/x-yaml'})
self.assertEqual(200, resp.status_code)
body = yaml.safe_load(resp.text)
expected_errors = [{
'schema': 'example/foo/v1',
'name': 'test_doc',
'message': "'fail' is not of type 'integer'"
}]
self.assertIn('errors', body)
self.assertEqual(expected_errors, body['errors'])
def test_document_without_data_section_saves_but_fails_validation(self):
"""Validate that a document without the data section is saved to the
database, but fails validation. This is a valid use case because a