airship
/
deckhand
Code Issues Proposed changes

Reduce number of pre-validation false positives 

Currently Pegleg uses a lot of raw documents that are missing
properties at first because those properties are only included
in the documents only after they undergo substitution (are rendered).
This means that when these raw documents are PRE-validated against
registered DataSchemas a lot of noise is created.

However, after the documents are rendered (undergo substitution)
then they should be POST-validated against the registered DataSchemas.

This PS makes the changes necessary to make pre-validation ignore
validation against registered DataSchemas but makes post-validation
raise all validation errors while validating against all built-in
and registered schemas.

Necessary changes were made to tests to make them pass with the
new changes. A follow up will be needed to do better testing
for pre-validation vs. post-validation but the functional test
scenario in schema-validation-success.yaml should test both
scenarios.

Change-Id: I5c139fa528639d43fc45eda067a9ea807fe26c61
This commit is contained in:
Felipe Monteiro 2018-02-08 22:14:11 +00:00
parent 9f7ecc0582
commit 02528bc3af
10 changed files with 289 additions and 148 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
deckhand/control/buckets.py
View File

@ -55,7 +55,7 @@ class BucketsResource(api_base.BaseResource):
schema=types.DATA_SCHEMA_SCHEMA, deleted=False) schema=types.DATA_SCHEMA_SCHEMA, deleted=False)
try: try:
doc_validator = document_validation.DocumentValidation( doc_validator = document_validation.DocumentValidation(
documents, data_schemas) documents, data_schemas, pre_validate=True)
validations = doc_validator.validate_all() validations = doc_validator.validate_all()
except deckhand_errors.InvalidDocumentFormat as e: except deckhand_errors.InvalidDocumentFormat as e:
LOG.exception(e.format_message()) LOG.exception(e.format_message())

11
deckhand/control/revision_documents.py
View File

@ -174,17 +174,22 @@ class RenderedDocumentsResource(api_base.BaseResource):
return db_api.document_get_all( return db_api.document_get_all(
**{'metadata.layeringDefinition.abstract': False}) **{'metadata.layeringDefinition.abstract': False})
def _post_validate(self, documents): def _post_validate(self, rendered_documents):
# Perform schema validation post-rendering to ensure that rendering # Perform schema validation post-rendering to ensure that rendering
# and substitution didn't break anything. # and substitution didn't break anything.
data_schemas = db_api.revision_documents_get( data_schemas = db_api.revision_documents_get(
schema=types.DATA_SCHEMA_SCHEMA, deleted=False) schema=types.DATA_SCHEMA_SCHEMA, deleted=False)
doc_validator = document_validation.DocumentValidation( doc_validator = document_validation.DocumentValidation(
documents, data_schemas) rendered_documents, data_schemas)
try: try:
doc_validator.validate_all() validations = doc_validator.validate_all()
except errors.InvalidDocumentFormat as e: except errors.InvalidDocumentFormat as e:
LOG.error('Failed to post-validate rendered documents.') LOG.error('Failed to post-validate rendered documents.')
LOG.exception(e.format_message()) LOG.exception(e.format_message())
raise falcon.HTTPInternalServerError( raise falcon.HTTPInternalServerError(
description=e.format_message()) description=e.format_message())
else:
failed_validations = [
v for v in validations if v['status'] == 'failure']
if failed_validations:
raise falcon.HTTPBadRequest(description=failed_validations)

63
deckhand/engine/document_validation.py
View File

@ -95,7 +95,7 @@ class GenericValidator(BaseValidator):
LOG.error( LOG.error(
'Failed sanity-check validation for document [%s] %s. ' 'Failed sanity-check validation for document [%s] %s. '
'Details: %s', document.get('schema', 'N/A'), 'Details: %s', document.get('schema', 'N/A'),
document.get('metadata', {}).get('name'), error_messages) document.metadata.get('name'), error_messages)
raise errors.InvalidDocumentFormat(details=error_messages) raise errors.InvalidDocumentFormat(details=error_messages)
@ -243,15 +243,13 @@ class DataSchemaValidator(SchemaValidator):
class DocumentValidation(object): class DocumentValidation(object):
def __init__(self, documents, existing_data_schemas=None): def __init__(self, documents, existing_data_schemas=None,
pre_validate=False):
"""Class for document validation logic for documents. """Class for document validation logic for documents.
This class is responsible for validating documents according to their This class is responsible for validating documents according to their
schema. schema.
``DataSchema`` documents must be validated first, as they are in turn
used to validate other documents.
:param documents: Documents to be validated. :param documents: Documents to be validated.
:type documents: List[dict] :type documents: List[dict]
:param existing_data_schemas: ``DataSchema`` documents created in prior :param existing_data_schemas: ``DataSchema`` documents created in prior
@ -259,6 +257,11 @@ class DocumentValidation(object):
``documents``. Additional ``DataSchema`` documents in ``documents`` ``documents``. Additional ``DataSchema`` documents in ``documents``
are combined with these. are combined with these.
:type existing_data_schemas: dict or List[dict] :type existing_data_schemas: dict or List[dict]
:param pre_validate: Only runs validations from ``GenericValidator``
and ``SchemaValidator`` against the documents if True. Otherwise
runs them all. This is useful to avoid spurious errors arising
from missing properties that may only exist post-substitution.
Default is False.
""" """
self.documents = [] self.documents = []
@ -267,17 +270,25 @@ class DocumentValidation(object):
for d in existing_data_schemas] for d in existing_data_schemas]
_data_schema_map = {d.name: d for d in data_schemas} _data_schema_map = {d.name: d for d in data_schemas}
raw_properties = ('data', 'metadata', 'schema')
if not isinstance(documents, list): if not isinstance(documents, list):
documents = [documents] documents = [documents]
for document in documents: for document in documents:
if not isinstance(document, document_wrapper.DocumentDict): # For post-validation documents are retrieved from the DB so those
document = document_wrapper.DocumentDict(document) # DB properties need to be stripped to avoid validation errors.
raw_document = {}
for prop in raw_properties:
raw_document[prop] = document.get(prop)
document = document_wrapper.DocumentDict(raw_document)
if document.schema.startswith(types.DATA_SCHEMA_SCHEMA): if document.schema.startswith(types.DATA_SCHEMA_SCHEMA):
data_schemas.append(document) data_schemas.append(document)
# If a newer version of the same DataSchema was passed in, # If a newer version of the same DataSchema was passed in,
# only use the new one and discard the old one. # only use the new one and discard the old one.
if document.name in _data_schema_map: if document.name in _data_schema_map:
data_schemas.remove(_data_schema_map.pop(document.name)) data_schemas.remove(_data_schema_map.pop(document.name))
self.documents.append(document) self.documents.append(document)
# NOTE(fmontei): The order of the validators is important. The # NOTE(fmontei): The order of the validators is important. The
@ -288,9 +299,11 @@ class DocumentValidation(object):
DataSchemaValidator(data_schemas) DataSchemaValidator(data_schemas)
] ]
self._pre_validate = pre_validate
def _get_supported_schema_list(self): def _get_supported_schema_list(self):
schema_list = [] schema_list = []
for validator in self._validators[1:]: for validator in self._validators[1:]: # Skip over `GenericValidator`.
for schema_version, schema_map in validator._schema_map.items(): for schema_version, schema_map in validator._schema_map.items():
for schema_prefix in schema_map: for schema_prefix in schema_map:
schema_list.append(schema_prefix + '/' + schema_version) schema_list.append(schema_prefix + '/' + schema_version)
@ -330,19 +343,17 @@ class DocumentValidation(object):
document_schema = None if not document.get('schema') else '/'.join( document_schema = None if not document.get('schema') else '/'.join(
_get_schema_parts(document)) _get_schema_parts(document))
if document_schema not in supported_schema_list: if document_schema not in supported_schema_list:
error_msg = ("The provided document schema %s is invalid. " message = ("The provided document schema %s is not registered. "
"Supported schemas include: %s" % ( "Registered schemas include: %s" % (
document.get('schema', 'N/A'), document.get('schema', 'N/A'),
supported_schema_list)) supported_schema_list))
LOG.error(error_msg) LOG.info(message)
result['errors'].append({
'schema': document.get('schema', 'N/A'),
'name': document.get('metadata', {}).get('name', 'N/A'),
'message': error_msg,
'path': '.'
})
for validator in self._validators: validators = self._validators
if self._pre_validate is True:
validators = self._validators[:-1]
for validator in validators:
if validator.matches(document): if validator.matches(document):
error_outputs = validator.validate(document) error_outputs = validator.validate(document)
if error_outputs: if error_outputs:
@ -400,16 +411,8 @@ class DocumentValidation(object):
validation_results = [] validation_results = []
for document in self.documents: for document in self.documents:
# NOTE(fmontei): Since ``DataSchema`` documents created in previous result = self._validate_one(document)
# revisions are retrieved and combined with new ``DataSchema`` validation_results.append(result)
# documents, we only want to create a validation result in the DB
# for the new documents. One way to do this is to check whether the
# document contains the 'id' key which is only assigned by the DB.
requires_validation = 'id' not in document
if requires_validation:
result = self._validate_one(document)
validation_results.append(result)
validations = self._format_validation_results(validation_results) validations = self._format_validation_results(validation_results)
return validations return validations

8
deckhand/engine/document_wrapper.py
View File

@ -35,15 +35,15 @@ class DocumentDict(dict):
@property @property
def schema(self): def schema(self):
return self.get('schema', '') return self.get('schema') or ''
@property @property
def metadata(self): def metadata(self):
return self.get('metadata', {}) return self.get('metadata') or {}
@property @property
def data(self): def data(self):
return self.get('data', {}) return self.get('data') or {}
@data.setter @data.setter
def data(self, value): def data(self, value):
@ -51,7 +51,7 @@ class DocumentDict(dict):
@property @property
def name(self): def name(self):
return utils.jsonpath_parse(self, 'metadata.name') return utils.jsonpath_parse(self, 'metadata.name') or ''
@property @property
def layering_definition(self): def layering_definition(self):

2
deckhand/engine/schema/base_schema.py
View File

@ -29,7 +29,7 @@ schema = {
'additionalProperties': True, 'additionalProperties': True,
'required': ['schema', 'name'] 'required': ['schema', 'name']
}, },
'data': {'type': ['string', 'integer', 'array', 'object']} 'data': {'type': ['null', 'string', 'integer', 'array', 'object']}
}, },
'additionalProperties': False, 'additionalProperties': False,
'required': ['schema', 'metadata'] 'required': ['schema', 'metadata']

4
deckhand/factories.py
View File

@ -22,6 +22,8 @@ from deckhand.tests import test_utils
LOG = logging.getLogger(__name__) LOG = logging.getLogger(__name__)
DOCUMENT_TEST_SCHEMA = 'example/Kind/v1'
@six.add_metaclass(abc.ABCMeta) @six.add_metaclass(abc.ABCMeta)
class DeckhandFactory(object): class DeckhandFactory(object):
@ -111,7 +113,7 @@ class DocumentFactory(DeckhandFactory):
"name": "", "name": "",
"schema": "metadata/Document/v%s" % DeckhandFactory.API_VERSION "schema": "metadata/Document/v%s" % DeckhandFactory.API_VERSION
}, },
"schema": "example/Kind/v1.0" "schema": DOCUMENT_TEST_SCHEMA
} }
def __init__(self, num_layers, docs_per_layer): def __init__(self, num_layers, docs_per_layer):

188
deckhand/tests/functional/gabbits/schema-validation-success.yaml
View File

@ -3,14 +3,17 @@
# 1. Purges existing data to ensure test isolation # 1. Purges existing data to ensure test isolation
# 2. Creates a DataSchema # 2. Creates a DataSchema
# 3. Checks that schema validation for the DataSchema passes # 3. Checks that schema validation for the DataSchema passes
# 4. Puts a valid document # 4. Puts a valid document (and LayeringPolicy)
# 5. Checks that the document passes schema validation # 5. Checks that the document passes schema pre-validation
# 6. Puts an invalid document # 6. Checks that the document passes schema post-validation
# 7. Checks that the document fails schema validation # 7. Puts an invalid document
# 8. Checks that the document entry details adhere to expected validation # 8. Checks that the document fails schema pre-validation
# 9. Checks that the document fails schema post-validation by raising expected
# exception
# 10. Checks that the document entry details adhere to expected validation
# format # format
# 9. Re-puts the same invalid document with substitutions # 11. Re-puts the same invalid document with substitutions
# 10. Verify that the substitutions were sanitized in the validation output # 12. Verify that the substitutions were sanitized in the validation output
defaults: defaults:
request_headers: request_headers:
@ -56,6 +59,15 @@ tests:
PUT: /api/v1.0/buckets/good/documents PUT: /api/v1.0/buckets/good/documents
status: 200 status: 200
data: |- data: |-
---
schema: deckhand/LayeringPolicy/v1
metadata:
schema: metadata/Control/v1
name: layering-policy
data:
layerOrder:
- site
---
schema: example/Doc/v1 schema: example/Doc/v1
metadata: metadata:
schema: metadata/Document/v1 schema: metadata/Document/v1
@ -67,18 +79,18 @@ tests:
a: this-one-is-required a: this-one-is-required
b: 77 b: 77
- name: verify_document_is_valid - name: verify_document_is_valid_pre_validation
desc: Check schema validation of the added document desc: Check schema pre-validation of the added document
GET: /api/v1.0/revisions/$HISTORY['add_valid_document'].$RESPONSE['$.[0].status.revision']/validations/deckhand-schema-validation GET: /api/v1.0/revisions/$HISTORY['add_valid_document'].$RESPONSE['$.[0].status.revision']/validations/deckhand-schema-validation
status: 200 status: 200
response_multidoc_jsonpaths: response_multidoc_jsonpaths:
$.`len`: 1 $.`len`: 1
$.[0].count: 1 $.[0].count: 2
$.[0].results[0].id: 0 $.[0].results[0].id: 0
$.[0].results[0].status: success $.[0].results[0].status: success
- name: verify_document_validation_success_in_list_view - name: verify_document_pre_validation_success_in_list_view
desc: Check document validation success shows in list view desc: Check document pre-validation success shows in list view
GET: /api/v1.0/revisions/$HISTORY['add_valid_document'].$RESPONSE['$.[0].status.revision']/validations GET: /api/v1.0/revisions/$HISTORY['add_valid_document'].$RESPONSE['$.[0].status.revision']/validations
status: 200 status: 200
response_multidoc_jsonpaths: response_multidoc_jsonpaths:
@ -87,6 +99,11 @@ tests:
$.[0].results[*].name: deckhand-schema-validation $.[0].results[*].name: deckhand-schema-validation
$.[0].results[*].status: success $.[0].results[*].status: success
- name: verify_document_is_valid_post_validation
desc: Check that the document passes post-validation
GET: /api/v1.0/revisions/$HISTORY['add_valid_document'].$RESPONSE['$.[0].status.revision']/rendered-documents
status: 200
- name: add_invalid_document - name: add_invalid_document
desc: Add a document that does not follow the schema desc: Add a document that does not follow the schema
PUT: /api/v1.0/buckets/bad/documents PUT: /api/v1.0/buckets/bad/documents
@ -103,61 +120,68 @@ tests:
a: this-one-is-required-and-can-be-different a: this-one-is-required-and-can-be-different
b: 177 b: 177
- name: verify_document_is_not_valid - name: verify_invalid_document_is_valid_pre_validation
desc: Check failure of schema validation of the added document desc: Check success of schema pre-validation of the added document
GET: /api/v1.0/revisions/$HISTORY['add_invalid_document'].$RESPONSE['$.[0].status.revision']/validations/deckhand-schema-validation GET: /api/v1.0/revisions/$HISTORY['add_invalid_document'].$RESPONSE['$.[0].status.revision']/validations/deckhand-schema-validation
status: 200 status: 200
response_multidoc_jsonpaths: response_multidoc_jsonpaths:
$.`len`: 1 $.`len`: 1
$.[0].count: 1 $.[0].count: 1
$.[0].results[*].status: failure $.[0].results[*].status: success
- name: verify_document_validation_failure_in_list_view - name: verify_document_pre_validation_failure_in_list_view
desc: Check document validation failure shows in list view desc: Check document pre-validation success shows in list view
GET: /api/v1.0/revisions/$HISTORY['add_invalid_document'].$RESPONSE['$.[0].status.revision']/validations GET: /api/v1.0/revisions/$HISTORY['add_invalid_document'].$RESPONSE['$.[0].status.revision']/validations
status: 200 status: 200
response_multidoc_jsonpaths: response_multidoc_jsonpaths:
$.`len`: 1 $.`len`: 1
$.[0].count: 1 $.[0].count: 1
$.[0].results[0].name: deckhand-schema-validation $.[0].results[0].name: deckhand-schema-validation
$.[0].results[0].status: failure $.[0].results[0].status: success
- name: verify_document_validation_failure_entry_details - name: verify_document_is_invalid_post_validation
desc: Check document validation failure details for specific entry desc: Check that the document fails post-validation
GET: /api/v1.0/revisions/$HISTORY['add_invalid_document'].$RESPONSE['$.[0].status.revision']/validations/deckhand-schema-validation/entries/0 GET: /api/v1.0/revisions/$HISTORY['add_invalid_document'].$RESPONSE['$.[0].status.revision']/rendered-documents
status: 200 status: 400
response_multidoc_jsonpaths: response_multidoc_jsonpaths:
$.`len`: 1 $.`len`: 1
$.[0].status: failure $.[0].status: Failure
$.[0].errors: $.[0].message:
- name: bad - errors:
schema: example/Doc/v1 - validation_schema:
path: .data.b "$schema": http://json-schema.org/schema#
schema_path: .properties.b.maximum properties:
error_section: a:
a: this-one-is-required-and-can-be-different type: string
b: 177 b:
message: 177 is greater than the maximum of 100 maximum: 100
validation_schema: type: integer
$schema: http://json-schema.org/schema# minimum: 0
additionalProperties: False type: object
properties: required:
a:
type: string
b:
maximum: 100
minimum: 0
type: integer
required:
- a - a
- b - b
type: object additionalProperties: false
error_section:
a: this-one-is-required-and-can-be-different
b: 177
schema_path: ".properties.b.maximum"
name: bad
schema: example/Doc/v1
path: ".data.b"
message: 177 is greater than the maximum of 100
name: deckhand-schema-validation
validator:
name: deckhand
version: '1.0'
status: failure
- name: add_invalid_document_with_substitutions - name: add_invalid_document_with_substitutions
desc: Add a document that does not follow the schema desc: Add a document that does not follow the schema
PUT: /api/v1.0/buckets/bad/documents PUT: /api/v1.0/buckets/bad/documents
status: 200 status: 200
data: |- data: |-
---
schema: example/Doc/v1 schema: example/Doc/v1
metadata: metadata:
schema: metadata/Document/v1 schema: metadata/Document/v1
@ -166,46 +190,58 @@ tests:
abstract: false abstract: false
layer: site layer: site
substitutions: substitutions:
# It doesn't matter if the values below are junk, only that the
# destination path of .a is sanitized in the `error_section` of the
# validation result.
- src: - src:
schema: None schema: deckhand/Certificate/v1
name: None name: test-certificate
path: . path: .
dest: dest:
path: .a path: .a
data: data:
a: this-one-is-required-and-can-be-different a: this-one-is-required-and-can-be-different
b: 177 b: 177
---
schema: deckhand/Certificate/v1
metadata:
name: test-certificate
schema: metadata/Document/v1
layeringDefinition:
layer: site
storagePolicy: cleartext
data: this-should-definitely-be-sanitized
- name: verify_document_validation_failure_entry_details_hides_secrets - name: verify_document_post_validation_failure_entry_details_hides_secrets
desc: Check document validation failure hides secrets desc: Check document validation failure hides secrets
GET: /api/v1.0/revisions/$HISTORY['add_invalid_document_with_substitutions'].$RESPONSE['$.[0].status.revision']/validations/deckhand-schema-validation/entries/0 GET: /api/v1.0/revisions/$HISTORY['add_invalid_document_with_substitutions'].$RESPONSE['$.[0].status.revision']/rendered-documents
status: 200 status: 400
response_multidoc_jsonpaths: response_multidoc_jsonpaths:
$.`len`: 1 $.`len`: 1
$.[0].status: failure $.[0].status: Failure
$.[0].errors: $.[0].message:
- name: bad - errors:
schema: example/Doc/v1 - name: bad
path: .data.b schema: example/Doc/v1
schema_path: .properties.b.maximum path: .data.b
error_section: schema_path: .properties.b.maximum
a: Sanitized to avoid exposing secret. error_section:
b: 177 a: Sanitized to avoid exposing secret.
message: 177 is greater than the maximum of 100 b: 177
validation_schema: message: 177 is greater than the maximum of 100
$schema: http://json-schema.org/schema# validation_schema:
additionalProperties: False $schema: http://json-schema.org/schema#
properties: additionalProperties: False
a: properties:
type: string a:
b: type: string
maximum: 100 b:
minimum: 0 maximum: 100
type: integer minimum: 0
required: type: integer
- a required:
- b - a
type: object - b
type: object
name: deckhand-schema-validation
validator:
name: deckhand
version: '1.0'
status: failure

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

@ -15,8 +15,11 @@
import copy import copy
import yaml import yaml
import mock
from oslo_config import cfg from oslo_config import cfg
from deckhand.control import buckets
from deckhand.engine import document_validation
from deckhand.engine.schema.v1_0 import document_schema from deckhand.engine.schema.v1_0 import document_schema
from deckhand import factories from deckhand import factories
from deckhand.tests import test_utils from deckhand.tests import test_utils
@ -55,8 +58,7 @@ validator:
""" """
class TestValidationsController(test_base.BaseControllerTest): class ValidationsControllerBaseTest(test_base.BaseControllerTest):
"""Test suite for validating positive scenarios for bucket controller."""
def _create_revision(self, payload=None): def _create_revision(self, payload=None):
if not payload: if not payload:
@ -83,6 +85,32 @@ class TestValidationsController(test_base.BaseControllerTest):
headers={'Content-Type': 'application/x-yaml'}, body=policy) headers={'Content-Type': 'application/x-yaml'}, body=policy)
return resp return resp
class TestValidationsControllerPostValidate(ValidationsControllerBaseTest):
"""Test suite for validating positive scenarios for post-validations with
Validations controller.
"""
def setUp(self):
super(TestValidationsControllerPostValidate, self).setUp()
self._monkey_patch_document_validation()
def _monkey_patch_document_validation(self):
"""Workaround for testing complex validation scenarios by forcibly
passing in `pre_validate=False`.
"""
# TODO(fmontei): Remove this workaround by testing these more complex
# scenarios against the rendered-documents endpoint instead (which
# performs post-validation).
original_document_validation = document_validation.DocumentValidation
def monkey_patch(*args, **kwargs):
return original_document_validation(*args, pre_validate=False)
mock.patch.object(buckets.document_validation, 'DocumentValidation',
side_effect=monkey_patch, autospec=True).start()
self.addCleanup(mock.patch.stopall)
def test_create_validation(self): def test_create_validation(self):
rules = {'deckhand:create_cleartext_documents': '@', rules = {'deckhand:create_cleartext_documents': '@',
'deckhand:create_validation': '@'} 'deckhand:create_validation': '@'}
@ -393,7 +421,9 @@ class TestValidationsController(test_base.BaseControllerTest):
created ``DataSchema`` results in an expected failure. created ``DataSchema`` results in an expected failure.
""" """
rules = {'deckhand:create_cleartext_documents': '@', rules = {'deckhand:create_cleartext_documents': '@',
'deckhand:list_validations': '@'} 'deckhand:list_validations': '@',
'deckhand:list_cleartext_documents': '@',
'deckhand:list_encrypted_documents': '@'}
self.policy.set_rules(rules) self.policy.set_rules(rules)
# Create a `DataSchema` against which the test document will be # Create a `DataSchema` against which the test document will be
@ -431,13 +461,14 @@ class TestValidationsController(test_base.BaseControllerTest):
# Create the test document that fails the validation due to the # Create the test document that fails the validation due to the
# schema defined by the `DataSchema` document. # schema defined by the `DataSchema` document.
doc_factory = factories.DocumentFactory(1, [1]) doc_factory = factories.DocumentFactory(1, [1])
doc_to_test = doc_factory.gen_test( docs_to_test = doc_factory.gen_test(
{'_GLOBAL_DATA_1_': {'data': {'a': 'fail'}}}, {'_GLOBAL_DATA_1_': {'data': {'a': 'fail'}}},
global_abstract=False)[-1] global_abstract=False)
doc_to_test['schema'] = 'example/foo/v1' docs_to_test[1]['schema'] = 'example/foo/v1'
doc_to_test['metadata']['name'] = 'test_doc' docs_to_test[1]['metadata']['name'] = 'test_doc'
revision_id = self._create_revision(payload=[doc_to_test]) revision_id = self._create_revision(
payload=docs_to_test + [data_schema])
# Validate that the validation was created and reports failure. # Validate that the validation was created and reports failure.
resp = self.app.simulate_get( resp = self.app.simulate_get(
@ -453,6 +484,12 @@ class TestValidationsController(test_base.BaseControllerTest):
} }
self.assertEqual(expected_body, body) self.assertEqual(expected_body, body)
# Validate that the validation was created and reports failure.
resp = self.app.simulate_get(
'/api/v1.0/revisions/%s/rendered-documents' % revision_id,
headers={'Content-Type': 'application/x-yaml'})
self.assertEqual(400, resp.status_code)
def test_validation_data_schema_same_revision_expect_failure(self): def test_validation_data_schema_same_revision_expect_failure(self):
"""Validates that creating a ``DataSchema`` alongside a document """Validates that creating a ``DataSchema`` alongside a document
that relies on it in the same revision results in an expected failure. that relies on it in the same revision results in an expected failure.
@ -728,13 +765,23 @@ class TestValidationsController(test_base.BaseControllerTest):
self.assertEqual(200, resp.status_code) self.assertEqual(200, resp.status_code)
body = yaml.safe_load(resp.text) body = yaml.safe_load(resp.text)
expected_errors = [{ expected_errors = [{
'error_section': document, 'error_section': {
'data': None,
'metadata': {'labels': {'global': 'global1'},
'layeringDefinition': {'abstract': False,
'actions': [],
'layer': 'global'},
'name': document['metadata']['name'],
'schema': 'metadata/Document/v1.0'},
'schema': document['schema']
},
'name': document['metadata']['name'], 'name': document['metadata']['name'],
'path': '.', 'path': '.data',
'schema': document['schema'], 'schema': document['schema'],
'message': "'data' is a required property", 'message': (
"None is not of type 'string', 'integer', 'array', 'object'"),
'validation_schema': document_schema.schema, 'validation_schema': document_schema.schema,
'schema_path': '.required' 'schema_path': '.properties.data.type'
}] }]
self.assertIn('errors', body) self.assertIn('errors', body)
self.assertEqual(expected_errors, body['errors']) self.assertEqual(expected_errors, body['errors'])
@ -805,3 +852,56 @@ class TestValidationsController(test_base.BaseControllerTest):
] ]
} }
self.assertEqual(expected_body, body) self.assertEqual(expected_body, body)
class TestValidationsControllerPreValidate(ValidationsControllerBaseTest):
"""Test suite for validating positive scenarios for pre-validations with
Validations controller.
"""
def test_pre_validate_flag_skips_over_dataschema_validations(self):
rules = {'deckhand:create_cleartext_documents': '@',
'deckhand:list_validations': '@'}
self.policy.set_rules(rules)
# Create a `DataSchema` against which the test document will be
# validated.
data_schema_factory = factories.DataSchemaFactory()
metadata_name = 'example/foo/v1'
schema_to_use = {
'$schema': 'http://json-schema.org/schema#',
'type': 'object',
'properties': {
'a': {
'type': 'integer' # Test doc will fail b/c of wrong type.
}
},
'required': ['a']
}
data_schema = data_schema_factory.gen_test(
metadata_name, data=schema_to_use)
# Create a document that passes validation and another that fails it.
doc_factory = factories.DocumentFactory(1, [1])
fail_doc = doc_factory.gen_test(
{'_GLOBAL_DATA_1_': {'data': {'a': 'fail'}}},
global_abstract=False)[-1]
fail_doc['schema'] = 'example/foo/v1'
fail_doc['metadata']['name'] = 'test_doc'
revision_id = self._create_revision(payload=[data_schema, fail_doc])
# Validate that the validation reports success because `fail_doc`
# isn't validated by the `DataSchema`.
resp = self.app.simulate_get(
'/api/v1.0/revisions/%s/validations' % revision_id,
headers={'Content-Type': 'application/x-yaml'})
self.assertEqual(200, resp.status_code)
body = yaml.safe_load(resp.text)
expected_body = {
'count': 1,
'results': [
{'name': types.DECKHAND_SCHEMA_VALIDATION, 'status': 'success'}
]
}
self.assertEqual(expected_body, body)

2
deckhand/tests/unit/engine/test_document_validation.py
View File

@ -84,7 +84,7 @@ class TestDocumentValidation(engine_test_base.TestDocumentValidationBase):
validations = document_validation.DocumentValidation( validations = document_validation.DocumentValidation(
test_document).validate_all() test_document).validate_all()
self.assertEqual(2, len(validations[0]['errors'])) self.assertEqual(1, len(validations[0]['errors']))
self.assertIn('Sanitized to avoid exposing secret.', self.assertIn('Sanitized to avoid exposing secret.',
str(validations[0]['errors'][-1])) str(validations[0]['errors'][-1]))
self.assertNotIn('scary-secret.', str(validations[0]['errors'][-1])) self.assertNotIn('scary-secret.', str(validations[0]['errors'][-1]))

33
deckhand/tests/unit/engine/test_document_validation_negative.py
View File

@ -80,25 +80,24 @@ class TestDocumentValidationNegative(test_base.TestDocumentValidationBase):
def test_certificate_key_missing_required_sections(self): def test_certificate_key_missing_required_sections(self):
document = self._read_data('sample_certificate_key') document = self._read_data('sample_certificate_key')
properties_to_remove = tuple(self.exception_map.keys()) + ( properties_to_remove = tuple(self.exception_map.keys()) + (
'data', 'metadata.storagePolicy',) 'metadata.storagePolicy',)
self._test_missing_required_sections(document, properties_to_remove) self._test_missing_required_sections(document, properties_to_remove)
def test_certificate_missing_required_sections(self): def test_certificate_missing_required_sections(self):
document = self._read_data('sample_certificate') document = self._read_data('sample_certificate')
properties_to_remove = tuple(self.exception_map.keys()) + ( properties_to_remove = tuple(self.exception_map.keys()) + (
'data', 'metadata.storagePolicy',) 'metadata.storagePolicy',)
self._test_missing_required_sections(document, properties_to_remove) self._test_missing_required_sections(document, properties_to_remove)
def test_data_schema_missing_required_sections(self): def test_data_schema_missing_required_sections(self):
document = self._read_data('sample_data_schema') document = self._read_data('sample_data_schema')
properties_to_remove = tuple(self.exception_map.keys()) + ( properties_to_remove = tuple(self.exception_map.keys()) + (
'data', 'data.$schema',) 'data.$schema',)
self._test_missing_required_sections(document, properties_to_remove) self._test_missing_required_sections(document, properties_to_remove)
def test_document_missing_required_sections(self): def test_document_missing_required_sections(self):
document = self._read_data('sample_document') document = self._read_data('sample_document')
properties_to_remove = tuple(self.exception_map.keys()) + ( properties_to_remove = tuple(self.exception_map.keys()) + (
'data',
'metadata.layeringDefinition', 'metadata.layeringDefinition',
'metadata.layeringDefinition.layer', 'metadata.layeringDefinition.layer',
'metadata.layeringDefinition.actions.0.method', 'metadata.layeringDefinition.actions.0.method',
@ -132,16 +131,10 @@ class TestDocumentValidationNegative(test_base.TestDocumentValidationBase):
validations = doc_validator.validate_all() validations = doc_validator.validate_all()
errors = validations[0]['errors'] errors = validations[0]['errors']
self.assertEqual(len(properties_to_remove) + 1, len(errors)) self.assertEqual(len(properties_to_remove), len(errors))
# Validate the first error relates to the fact that the document's
# schema is unrecognized (promenade/ResourceType/v1.0) as it wasn't
# registered with a ``DataSchema``.
self.assertIn('%s is invalid' % document['schema'],
errors[0]['message'])
# Sort the errors to match the order in ``properties_to_remove``. # Sort the errors to match the order in ``properties_to_remove``.
errors = sorted(errors[1:], key=lambda x: (x['path'], x['message'])) errors = sorted(errors, key=lambda x: (x['path'], x['message']))
# Validate that an error was generated for each missing property in # Validate that an error was generated for each missing property in
# ``properties_to_remove`` that was removed from ``document``. # ``properties_to_remove`` that was removed from ``document``.
@ -172,19 +165,19 @@ class TestDocumentValidationNegative(test_base.TestDocumentValidationBase):
def test_layering_policy_missing_required_sections(self): def test_layering_policy_missing_required_sections(self):
document = self._read_data('sample_layering_policy') document = self._read_data('sample_layering_policy')
properties_to_remove = tuple(self.exception_map.keys()) + ( properties_to_remove = tuple(self.exception_map.keys()) + (
'data', 'data.layerOrder',) 'data.layerOrder',)
self._test_missing_required_sections(document, properties_to_remove) self._test_missing_required_sections(document, properties_to_remove)
def test_passphrase_missing_required_sections(self): def test_passphrase_missing_required_sections(self):
document = self._read_data('sample_passphrase') document = self._read_data('sample_passphrase')
properties_to_remove = tuple(self.exception_map.keys()) + ( properties_to_remove = tuple(self.exception_map.keys()) + (
'data', 'metadata.storagePolicy',) 'metadata.storagePolicy',)
self._test_missing_required_sections(document, properties_to_remove) self._test_missing_required_sections(document, properties_to_remove)
def test_validation_policy_missing_required_sections(self): def test_validation_policy_missing_required_sections(self):
document = self._read_data('sample_validation_policy') document = self._read_data('sample_validation_policy')
properties_to_remove = tuple(self.exception_map.keys()) + ( properties_to_remove = tuple(self.exception_map.keys()) + (
'data', 'data.validations', 'data.validations.0.name') 'data.validations', 'data.validations.0.name')
self._test_missing_required_sections(document, properties_to_remove) self._test_missing_required_sections(document, properties_to_remove)
@mock.patch.object(document_validation, 'LOG', autospec=True) @mock.patch.object(document_validation, 'LOG', autospec=True)
@ -195,8 +188,9 @@ class TestDocumentValidationNegative(test_base.TestDocumentValidationBase):
doc_validator = document_validation.DocumentValidation(document) doc_validator = document_validation.DocumentValidation(document)
doc_validator.validate_all() doc_validator.validate_all()
self.assertRegex( self.assertRegex(
mock_log.error.mock_calls[0][1][0], mock_log.info.mock_calls[0][1][0],
'The provided document schema %s is invalid.' % document['schema']) 'The provided document schema %s is not registered.'
% document['schema'])
@mock.patch.object(document_validation, 'LOG', autospec=True) @mock.patch.object(document_validation, 'LOG', autospec=True)
def test_invalid_document_schema_version_generates_error(self, mock_log): def test_invalid_document_schema_version_generates_error(self, mock_log):
@ -206,8 +200,9 @@ class TestDocumentValidationNegative(test_base.TestDocumentValidationBase):
doc_validator = document_validation.DocumentValidation(document) doc_validator = document_validation.DocumentValidation(document)
doc_validator.validate_all() doc_validator.validate_all()
self.assertRegex( self.assertRegex(
mock_log.error.mock_calls[0][1][0], mock_log.info.mock_calls[0][1][0],
'The provided document schema %s is invalid.' % document['schema']) 'The provided document schema %s is not registered.'
% document['schema'])
def test_invalid_validation_schema_raises_runtime_error(self): def test_invalid_validation_schema_raises_runtime_error(self):
document = self._read_data('sample_passphrase') document = self._read_data('sample_passphrase')