From 02528bc3afa4b807124029961e832d841e165cd7 Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Thu, 8 Feb 2018 22:14:11 +0000 Subject: [PATCH] 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 --- deckhand/control/buckets.py | 2 +- deckhand/control/revision_documents.py | 11 +- deckhand/engine/document_validation.py | 63 +++--- deckhand/engine/document_wrapper.py | 8 +- deckhand/engine/schema/base_schema.py | 2 +- deckhand/factories.py | 4 +- .../gabbits/schema-validation-success.yaml | 188 +++++++++++------- .../control/test_validations_controller.py | 124 ++++++++++-- .../unit/engine/test_document_validation.py | 2 +- .../test_document_validation_negative.py | 33 ++- 10 files changed, 289 insertions(+), 148 deletions(-) diff --git a/deckhand/control/buckets.py b/deckhand/control/buckets.py index a825f2f7..44d37a3d 100644 --- a/deckhand/control/buckets.py +++ b/deckhand/control/buckets.py @@ -55,7 +55,7 @@ class BucketsResource(api_base.BaseResource): schema=types.DATA_SCHEMA_SCHEMA, deleted=False) try: doc_validator = document_validation.DocumentValidation( - documents, data_schemas) + documents, data_schemas, pre_validate=True) validations = doc_validator.validate_all() except deckhand_errors.InvalidDocumentFormat as e: LOG.exception(e.format_message()) diff --git a/deckhand/control/revision_documents.py b/deckhand/control/revision_documents.py index 98a6965d..9fc8976a 100644 --- a/deckhand/control/revision_documents.py +++ b/deckhand/control/revision_documents.py @@ -174,17 +174,22 @@ class RenderedDocumentsResource(api_base.BaseResource): return db_api.document_get_all( **{'metadata.layeringDefinition.abstract': False}) - def _post_validate(self, documents): + def _post_validate(self, rendered_documents): # Perform schema validation post-rendering to ensure that rendering # and substitution didn't break anything. data_schemas = db_api.revision_documents_get( schema=types.DATA_SCHEMA_SCHEMA, deleted=False) doc_validator = document_validation.DocumentValidation( - documents, data_schemas) + rendered_documents, data_schemas) try: - doc_validator.validate_all() + validations = doc_validator.validate_all() except errors.InvalidDocumentFormat as e: LOG.error('Failed to post-validate rendered documents.') LOG.exception(e.format_message()) raise falcon.HTTPInternalServerError( 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) diff --git a/deckhand/engine/document_validation.py b/deckhand/engine/document_validation.py index 6c3ba982..d4a11707 100644 --- a/deckhand/engine/document_validation.py +++ b/deckhand/engine/document_validation.py @@ -95,7 +95,7 @@ class GenericValidator(BaseValidator): LOG.error( 'Failed sanity-check validation for document [%s] %s. ' '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) @@ -243,15 +243,13 @@ class DataSchemaValidator(SchemaValidator): 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. This class is responsible for validating documents according to their schema. - ``DataSchema`` documents must be validated first, as they are in turn - used to validate other documents. - :param documents: Documents to be validated. :type documents: List[dict] :param existing_data_schemas: ``DataSchema`` documents created in prior @@ -259,6 +257,11 @@ class DocumentValidation(object): ``documents``. Additional ``DataSchema`` documents in ``documents`` are combined with these. :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 = [] @@ -267,17 +270,25 @@ class DocumentValidation(object): for d in existing_data_schemas] _data_schema_map = {d.name: d for d in data_schemas} + raw_properties = ('data', 'metadata', 'schema') + if not isinstance(documents, list): documents = [documents] for document in documents: - if not isinstance(document, document_wrapper.DocumentDict): - document = document_wrapper.DocumentDict(document) + # For post-validation documents are retrieved from the DB so those + # 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): data_schemas.append(document) # If a newer version of the same DataSchema was passed in, # only use the new one and discard the old one. if document.name in _data_schema_map: data_schemas.remove(_data_schema_map.pop(document.name)) + self.documents.append(document) # NOTE(fmontei): The order of the validators is important. The @@ -288,9 +299,11 @@ class DocumentValidation(object): DataSchemaValidator(data_schemas) ] + self._pre_validate = pre_validate + def _get_supported_schema_list(self): 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_prefix in schema_map: schema_list.append(schema_prefix + '/' + schema_version) @@ -330,19 +343,17 @@ class DocumentValidation(object): document_schema = None if not document.get('schema') else '/'.join( _get_schema_parts(document)) if document_schema not in supported_schema_list: - error_msg = ("The provided document schema %s is invalid. " - "Supported schemas include: %s" % ( - document.get('schema', 'N/A'), - supported_schema_list)) - LOG.error(error_msg) - result['errors'].append({ - 'schema': document.get('schema', 'N/A'), - 'name': document.get('metadata', {}).get('name', 'N/A'), - 'message': error_msg, - 'path': '.' - }) + message = ("The provided document schema %s is not registered. " + "Registered schemas include: %s" % ( + document.get('schema', 'N/A'), + supported_schema_list)) + LOG.info(message) - 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): error_outputs = validator.validate(document) if error_outputs: @@ -400,16 +411,8 @@ class DocumentValidation(object): validation_results = [] for document in self.documents: - # NOTE(fmontei): Since ``DataSchema`` documents created in previous - # revisions are retrieved and combined with new ``DataSchema`` - # 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) + result = self._validate_one(document) + validation_results.append(result) validations = self._format_validation_results(validation_results) return validations diff --git a/deckhand/engine/document_wrapper.py b/deckhand/engine/document_wrapper.py index 1b33fdba..ccae7978 100644 --- a/deckhand/engine/document_wrapper.py +++ b/deckhand/engine/document_wrapper.py @@ -35,15 +35,15 @@ class DocumentDict(dict): @property def schema(self): - return self.get('schema', '') + return self.get('schema') or '' @property def metadata(self): - return self.get('metadata', {}) + return self.get('metadata') or {} @property def data(self): - return self.get('data', {}) + return self.get('data') or {} @data.setter def data(self, value): @@ -51,7 +51,7 @@ class DocumentDict(dict): @property def name(self): - return utils.jsonpath_parse(self, 'metadata.name') + return utils.jsonpath_parse(self, 'metadata.name') or '' @property def layering_definition(self): diff --git a/deckhand/engine/schema/base_schema.py b/deckhand/engine/schema/base_schema.py index 97882da4..39295006 100644 --- a/deckhand/engine/schema/base_schema.py +++ b/deckhand/engine/schema/base_schema.py @@ -29,7 +29,7 @@ schema = { 'additionalProperties': True, 'required': ['schema', 'name'] }, - 'data': {'type': ['string', 'integer', 'array', 'object']} + 'data': {'type': ['null', 'string', 'integer', 'array', 'object']} }, 'additionalProperties': False, 'required': ['schema', 'metadata'] diff --git a/deckhand/factories.py b/deckhand/factories.py index 8a06ef3a..d3eb1aba 100644 --- a/deckhand/factories.py +++ b/deckhand/factories.py @@ -22,6 +22,8 @@ from deckhand.tests import test_utils LOG = logging.getLogger(__name__) +DOCUMENT_TEST_SCHEMA = 'example/Kind/v1' + @six.add_metaclass(abc.ABCMeta) class DeckhandFactory(object): @@ -111,7 +113,7 @@ class DocumentFactory(DeckhandFactory): "name": "", "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): diff --git a/deckhand/tests/functional/gabbits/schema-validation-success.yaml b/deckhand/tests/functional/gabbits/schema-validation-success.yaml index d6e97bfc..d13d9cfd 100644 --- a/deckhand/tests/functional/gabbits/schema-validation-success.yaml +++ b/deckhand/tests/functional/gabbits/schema-validation-success.yaml @@ -3,14 +3,17 @@ # 1. Purges existing data to ensure test isolation # 2. Creates a DataSchema # 3. Checks that schema validation for the DataSchema passes -# 4. Puts a valid document -# 5. Checks that the document passes schema validation -# 6. Puts an invalid document -# 7. Checks that the document fails schema validation -# 8. Checks that the document entry details adhere to expected validation +# 4. Puts a valid document (and LayeringPolicy) +# 5. Checks that the document passes schema pre-validation +# 6. Checks that the document passes schema post-validation +# 7. Puts an invalid document +# 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 -# 9. Re-puts the same invalid document with substitutions -# 10. Verify that the substitutions were sanitized in the validation output +# 11. Re-puts the same invalid document with substitutions +# 12. Verify that the substitutions were sanitized in the validation output defaults: request_headers: @@ -56,6 +59,15 @@ tests: PUT: /api/v1.0/buckets/good/documents status: 200 data: |- + --- + schema: deckhand/LayeringPolicy/v1 + metadata: + schema: metadata/Control/v1 + name: layering-policy + data: + layerOrder: + - site + --- schema: example/Doc/v1 metadata: schema: metadata/Document/v1 @@ -67,18 +79,18 @@ tests: a: this-one-is-required b: 77 - - name: verify_document_is_valid - desc: Check schema validation of the added document + - name: verify_document_is_valid_pre_validation + 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 status: 200 response_multidoc_jsonpaths: $.`len`: 1 - $.[0].count: 1 + $.[0].count: 2 $.[0].results[0].id: 0 $.[0].results[0].status: success - - name: verify_document_validation_success_in_list_view - desc: Check document validation success shows in list view + - name: verify_document_pre_validation_success_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 status: 200 response_multidoc_jsonpaths: @@ -87,6 +99,11 @@ tests: $.[0].results[*].name: deckhand-schema-validation $.[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 desc: Add a document that does not follow the schema PUT: /api/v1.0/buckets/bad/documents @@ -103,61 +120,68 @@ tests: a: this-one-is-required-and-can-be-different b: 177 - - name: verify_document_is_not_valid - desc: Check failure of schema validation of the added document + - name: verify_invalid_document_is_valid_pre_validation + 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 status: 200 response_multidoc_jsonpaths: $.`len`: 1 $.[0].count: 1 - $.[0].results[*].status: failure + $.[0].results[*].status: success - - name: verify_document_validation_failure_in_list_view - desc: Check document validation failure shows in list view + - name: verify_document_pre_validation_failure_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 status: 200 response_multidoc_jsonpaths: $.`len`: 1 $.[0].count: 1 $.[0].results[0].name: deckhand-schema-validation - $.[0].results[0].status: failure + $.[0].results[0].status: success - - name: verify_document_validation_failure_entry_details - desc: Check document validation failure details for specific entry - GET: /api/v1.0/revisions/$HISTORY['add_invalid_document'].$RESPONSE['$.[0].status.revision']/validations/deckhand-schema-validation/entries/0 - status: 200 + - name: verify_document_is_invalid_post_validation + desc: Check that the document fails post-validation + GET: /api/v1.0/revisions/$HISTORY['add_invalid_document'].$RESPONSE['$.[0].status.revision']/rendered-documents + status: 400 response_multidoc_jsonpaths: $.`len`: 1 - $.[0].status: failure - $.[0].errors: - - name: bad - schema: example/Doc/v1 - path: .data.b - schema_path: .properties.b.maximum - error_section: - a: this-one-is-required-and-can-be-different - b: 177 - message: 177 is greater than the maximum of 100 - validation_schema: - $schema: http://json-schema.org/schema# - additionalProperties: False - properties: - a: - type: string - b: - maximum: 100 - minimum: 0 - type: integer - required: + $.[0].status: Failure + $.[0].message: + - errors: + - validation_schema: + "$schema": http://json-schema.org/schema# + properties: + a: + type: string + b: + maximum: 100 + type: integer + minimum: 0 + type: object + required: - a - 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 desc: Add a document that does not follow the schema PUT: /api/v1.0/buckets/bad/documents status: 200 data: |- + --- schema: example/Doc/v1 metadata: schema: metadata/Document/v1 @@ -166,46 +190,58 @@ tests: abstract: false layer: site 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: - schema: None - name: None + schema: deckhand/Certificate/v1 + name: test-certificate path: . dest: path: .a data: a: this-one-is-required-and-can-be-different 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 - GET: /api/v1.0/revisions/$HISTORY['add_invalid_document_with_substitutions'].$RESPONSE['$.[0].status.revision']/validations/deckhand-schema-validation/entries/0 - status: 200 + GET: /api/v1.0/revisions/$HISTORY['add_invalid_document_with_substitutions'].$RESPONSE['$.[0].status.revision']/rendered-documents + status: 400 response_multidoc_jsonpaths: $.`len`: 1 - $.[0].status: failure - $.[0].errors: - - name: bad - schema: example/Doc/v1 - path: .data.b - schema_path: .properties.b.maximum - error_section: - a: Sanitized to avoid exposing secret. - b: 177 - message: 177 is greater than the maximum of 100 - validation_schema: - $schema: http://json-schema.org/schema# - additionalProperties: False - properties: - a: - type: string - b: - maximum: 100 - minimum: 0 - type: integer - required: - - a - - b - type: object + $.[0].status: Failure + $.[0].message: + - errors: + - name: bad + schema: example/Doc/v1 + path: .data.b + schema_path: .properties.b.maximum + error_section: + a: Sanitized to avoid exposing secret. + b: 177 + message: 177 is greater than the maximum of 100 + validation_schema: + $schema: http://json-schema.org/schema# + additionalProperties: False + properties: + a: + type: string + b: + maximum: 100 + minimum: 0 + type: integer + required: + - a + - b + type: object + name: deckhand-schema-validation + validator: + name: deckhand + version: '1.0' + status: failure diff --git a/deckhand/tests/unit/control/test_validations_controller.py b/deckhand/tests/unit/control/test_validations_controller.py index bed130ca..355cdeb5 100644 --- a/deckhand/tests/unit/control/test_validations_controller.py +++ b/deckhand/tests/unit/control/test_validations_controller.py @@ -15,8 +15,11 @@ import copy import yaml +import mock 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 import factories from deckhand.tests import test_utils @@ -55,8 +58,7 @@ validator: """ -class TestValidationsController(test_base.BaseControllerTest): - """Test suite for validating positive scenarios for bucket controller.""" +class ValidationsControllerBaseTest(test_base.BaseControllerTest): def _create_revision(self, payload=None): if not payload: @@ -83,6 +85,32 @@ class TestValidationsController(test_base.BaseControllerTest): headers={'Content-Type': 'application/x-yaml'}, body=policy) 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): rules = {'deckhand:create_cleartext_documents': '@', 'deckhand:create_validation': '@'} @@ -393,7 +421,9 @@ class TestValidationsController(test_base.BaseControllerTest): created ``DataSchema`` results in an expected failure. """ rules = {'deckhand:create_cleartext_documents': '@', - 'deckhand:list_validations': '@'} + 'deckhand:list_validations': '@', + 'deckhand:list_cleartext_documents': '@', + 'deckhand:list_encrypted_documents': '@'} self.policy.set_rules(rules) # 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 # schema defined by the `DataSchema` document. 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_abstract=False)[-1] - doc_to_test['schema'] = 'example/foo/v1' - doc_to_test['metadata']['name'] = 'test_doc' + global_abstract=False) + docs_to_test[1]['schema'] = 'example/foo/v1' + 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. resp = self.app.simulate_get( @@ -453,6 +484,12 @@ class TestValidationsController(test_base.BaseControllerTest): } 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): """Validates that creating a ``DataSchema`` alongside a document 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) body = yaml.safe_load(resp.text) 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'], - 'path': '.', + 'path': '.data', 'schema': document['schema'], - 'message': "'data' is a required property", + 'message': ( + "None is not of type 'string', 'integer', 'array', 'object'"), 'validation_schema': document_schema.schema, - 'schema_path': '.required' + 'schema_path': '.properties.data.type' }] self.assertIn('errors', body) self.assertEqual(expected_errors, body['errors']) @@ -805,3 +852,56 @@ class TestValidationsController(test_base.BaseControllerTest): ] } 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) diff --git a/deckhand/tests/unit/engine/test_document_validation.py b/deckhand/tests/unit/engine/test_document_validation.py index 9df6782e..4a38d113 100644 --- a/deckhand/tests/unit/engine/test_document_validation.py +++ b/deckhand/tests/unit/engine/test_document_validation.py @@ -84,7 +84,7 @@ class TestDocumentValidation(engine_test_base.TestDocumentValidationBase): validations = document_validation.DocumentValidation( 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.', str(validations[0]['errors'][-1])) self.assertNotIn('scary-secret.', str(validations[0]['errors'][-1])) diff --git a/deckhand/tests/unit/engine/test_document_validation_negative.py b/deckhand/tests/unit/engine/test_document_validation_negative.py index 85bf59aa..d8fa2514 100644 --- a/deckhand/tests/unit/engine/test_document_validation_negative.py +++ b/deckhand/tests/unit/engine/test_document_validation_negative.py @@ -80,25 +80,24 @@ class TestDocumentValidationNegative(test_base.TestDocumentValidationBase): def test_certificate_key_missing_required_sections(self): document = self._read_data('sample_certificate_key') properties_to_remove = tuple(self.exception_map.keys()) + ( - 'data', 'metadata.storagePolicy',) + 'metadata.storagePolicy',) self._test_missing_required_sections(document, properties_to_remove) def test_certificate_missing_required_sections(self): document = self._read_data('sample_certificate') properties_to_remove = tuple(self.exception_map.keys()) + ( - 'data', 'metadata.storagePolicy',) + 'metadata.storagePolicy',) self._test_missing_required_sections(document, properties_to_remove) def test_data_schema_missing_required_sections(self): document = self._read_data('sample_data_schema') properties_to_remove = tuple(self.exception_map.keys()) + ( - 'data', 'data.$schema',) + 'data.$schema',) self._test_missing_required_sections(document, properties_to_remove) def test_document_missing_required_sections(self): document = self._read_data('sample_document') properties_to_remove = tuple(self.exception_map.keys()) + ( - 'data', 'metadata.layeringDefinition', 'metadata.layeringDefinition.layer', 'metadata.layeringDefinition.actions.0.method', @@ -132,16 +131,10 @@ class TestDocumentValidationNegative(test_base.TestDocumentValidationBase): validations = doc_validator.validate_all() errors = validations[0]['errors'] - self.assertEqual(len(properties_to_remove) + 1, 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']) + self.assertEqual(len(properties_to_remove), len(errors)) # 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 # ``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): document = self._read_data('sample_layering_policy') properties_to_remove = tuple(self.exception_map.keys()) + ( - 'data', 'data.layerOrder',) + 'data.layerOrder',) self._test_missing_required_sections(document, properties_to_remove) def test_passphrase_missing_required_sections(self): document = self._read_data('sample_passphrase') properties_to_remove = tuple(self.exception_map.keys()) + ( - 'data', 'metadata.storagePolicy',) + 'metadata.storagePolicy',) self._test_missing_required_sections(document, properties_to_remove) def test_validation_policy_missing_required_sections(self): document = self._read_data('sample_validation_policy') 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) @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.validate_all() self.assertRegex( - mock_log.error.mock_calls[0][1][0], - 'The provided document schema %s is invalid.' % document['schema']) + mock_log.info.mock_calls[0][1][0], + 'The provided document schema %s is not registered.' + % document['schema']) @mock.patch.object(document_validation, 'LOG', autospec=True) 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.validate_all() self.assertRegex( - mock_log.error.mock_calls[0][1][0], - 'The provided document schema %s is invalid.' % document['schema']) + mock_log.info.mock_calls[0][1][0], + 'The provided document schema %s is not registered.' + % document['schema']) def test_invalid_validation_schema_raises_runtime_error(self): document = self._read_data('sample_passphrase')