diff --git a/deckhand/control/buckets.py b/deckhand/control/buckets.py index c8a72af9..4f221c77 100644 --- a/deckhand/control/buckets.py +++ b/deckhand/control/buckets.py @@ -53,8 +53,7 @@ class BucketsResource(api_base.BaseResource): try: doc_validator = document_validation.DocumentValidation(documents) validations = doc_validator.validate_all() - except (deckhand_errors.InvalidDocumentFormat, - deckhand_errors.InvalidDocumentSchema) as e: + except deckhand_errors.InvalidDocumentFormat as e: LOG.exception(e.format_message()) raise falcon.HTTPBadRequest(description=e.format_message()) diff --git a/deckhand/control/revision_documents.py b/deckhand/control/revision_documents.py index c633275a..04e0794d 100644 --- a/deckhand/control/revision_documents.py +++ b/deckhand/control/revision_documents.py @@ -170,8 +170,7 @@ class RenderedDocumentsResource(api_base.BaseResource): doc_validator = document_validation.DocumentValidation(documents) try: doc_validator.validate_all() - except (errors.InvalidDocumentFormat, - errors.InvalidDocumentSchema) as e: + except errors.InvalidDocumentFormat as e: LOG.error('Failed to post-validate rendered documents.') LOG.exception(e.format_message()) raise falcon.HTTPInternalServerError( diff --git a/deckhand/engine/document_validation.py b/deckhand/engine/document_validation.py index efee4320..e365167b 100644 --- a/deckhand/engine/document_validation.py +++ b/deckhand/engine/document_validation.py @@ -12,170 +12,294 @@ # See the License for the specific language governing permissions and # limitations under the License. -import copy +import abc import re import jsonschema from oslo_log import log as logging +import six from deckhand.db.sqlalchemy import api as db_api -from deckhand.engine import document as document_wrapper from deckhand.engine.schema import base_schema from deckhand.engine.schema import v1_0 from deckhand import errors from deckhand import types +from deckhand import utils LOG = logging.getLogger(__name__) +@six.add_metaclass(abc.ABCMeta) +class BaseValidator(object): + """Abstract base validator. + + Sub-classes should override this to implement schema-specific document + validation. + """ + + _supported_versions = ('v1',) + _schema_re = re.compile(r'^[a-zA-Z]+\/[a-zA-Z]+\/v\d+(.0)?$') + + @abc.abstractmethod + def matches(self, document): + """Whether this Validator should be used to validate ``document``. + + :param dict document: Document to validate. + :returns: True if Validator applies to ``document``, else False. + """ + + @abc.abstractmethod + def validate(self, document): + """Validate whether ``document`` passes schema validation.""" + + +class GenericValidator(BaseValidator): + """Validator used for validating all documents, regardless whether concrete + or abstract, or what version its schema is. + """ + + def matches(self, document): + # Applies to all schemas, so unconditionally returns True. + return True + + def validate(self, document): + """Validate ``document``against basic schema validation. + + Sanity-checks each document for mandatory keys like "metadata" and + "schema". + + Applies even to abstract documents, as they must be consumed by + concrete documents, so basic formatting is mandatory. + + Failure to pass this check results in an error. + + :param dict document: Document to validate. + :raises RuntimeError: If the Deckhand schema itself is invalid. + :raises errors.InvalidDocumentFormat: If the document failed schema + validation. + :returns: None + + """ + try: + jsonschema.Draft4Validator.check_schema(base_schema.schema) + schema_validator = jsonschema.Draft4Validator(base_schema.schema) + error_messages = [ + e.message for e in schema_validator.iter_errors(document)] + except Exception as e: + raise RuntimeError( + 'Unknown error occurred while attempting to use Deckhand ' + 'schema. Details: %s' % six.text_type(e)) + else: + if error_messages: + LOG.error( + 'Failed sanity-check validation for document [%s] %s. ' + 'Details: %s', document.get('schema', 'N/A'), + document.get('metadata', {}).get('name'), error_messages) + raise errors.InvalidDocumentFormat(details=error_messages) + + +class SchemaValidator(BaseValidator): + """Validator for validating built-in document kinds.""" + + _schema_map = { + 'v1': { + 'deckhand/CertificateAuthorityKey': + v1_0.certificate_authority_key_schema, + 'deckhand/CertificateAuthority': v1_0.certificate_authority_schema, + 'deckhand/CertificateKey': v1_0.certificate_key_schema, + 'deckhand/Certificate': v1_0.certificate_schema, + 'deckhand/DataSchema': v1_0.data_schema_schema, + 'deckhand/LayeringPolicy': v1_0.layering_policy_schema, + 'deckhand/Passphrase': v1_0.passphrase_schema, + 'deckhand/PrivateKey': v1_0.private_key_schema, + 'deckhand/PublicKey': v1_0.public_key_schema, + 'deckhand/ValidationPolicy': v1_0.validation_policy_schema, + } + } + + # Represents a generic document schema. + _fallback_schema = v1_0.document_schema + + def _get_schemas(self, document): + """Retrieve the relevant schemas based on the document's + ``schema``. + + :param dict doc: The document used for finding the correct schema + to validate it based on its ``schema``. + :returns: A schema to be used by ``jsonschema`` for document + validation. + :rtype: dict + + """ + schema_prefix, schema_version = get_schema_parts(document) + matching_schemas = [] + relevant_schemas = self._schema_map.get(schema_version, {}) + for candidae_schema_prefix, schema in relevant_schemas.items(): + if candidae_schema_prefix == schema_prefix: + if schema not in matching_schemas: + matching_schemas.append(schema) + return matching_schemas + + def matches(self, document): + if is_abstract(document) is True: + LOG.info('Skipping schema validation for abstract document [%s]: ' + '%s.', document['schema'], document['metadata']['name']) + return False + return True + + def validate(self, document, validate_section='', + use_fallback_schema=True): + """Validate ``document`` against built-in ``schema``-specific schemas. + + Does not apply to abstract documents. + + :param dict document: Document to validate. + :param str validate_section: Document section to validate. If empty + string, validates entire ``document``. + :param bool use_fallback_schema: Whether to use the "fallback" schema + if no matching schemas are found by :method:``matches``. + + :raises RuntimeError: If the Deckhand schema itself is invalid. + :returns: Tuple of (error message, parent path for failing property) + following schema validation failure. + :rtype: Generator[Tuple[str, str]] + + """ + schemas_to_use = self._get_schemas(document) + if not schemas_to_use and use_fallback_schema: + LOG.debug('Document schema %s not recognized. Using "fallback" ' + 'schema.', document['schema']) + schemas_to_use = [SchemaValidator._fallback_schema] + + for schema_to_use in schemas_to_use: + schema = schema_to_use.schema + if validate_section: + to_validate = document.get(validate_section, None) + root_path = '.' + validate_section + '.' + else: + to_validate = document + root_path = '.' + try: + jsonschema.Draft4Validator.check_schema(schema) + schema_validator = jsonschema.Draft4Validator(schema) + errors = schema_validator.iter_errors(to_validate) + except Exception as e: + LOG.exception(six.text_type(e)) + raise RuntimeError( + 'Unknown error occurred while attempting to use schema ' + 'for validation. Details: %s.' % six.text_type(e)) + else: + for error in errors: + LOG.error( + 'Failed schema validation for document [%s] %s. ' + 'Details: %s.', document['schema'], + document['metadata']['name'], error.message) + parent_path = root_path + '.'.join( + [six.text_type(x) for x in error.path]) + yield error.message, parent_path + + +class DataSchemaValidator(SchemaValidator): + """Validator for validating ``DataSchema`` documents.""" + + def __init__(self, data_schemas): + super(DataSchemaValidator, self).__init__() + self._schema_map = self._build_schema_map(data_schemas) + + def _build_schema_map(self, data_schemas): + schema_map = {k: {} for k in self._supported_versions} + + for data_schema in data_schemas: + # Ensure that each `DataSchema` document has required properties + # before they themselves can be used to validate other documents. + if 'name' not in data_schema.get('metadata', {}): + continue + if self._schema_re.match(data_schema['metadata']['name']) is None: + continue + if 'data' not in data_schema: + continue + schema_prefix, schema_version = get_schema_parts(data_schema, + 'metadata.name') + + class Schema(object): + schema = data_schema['data'] + + schema_map[schema_version].setdefault(schema_prefix, Schema()) + + return schema_map + + def matches(self, document): + if is_abstract(document) is True: + LOG.info('Skipping schema validation for abstract document [%s]: ' + '%s.', document['schema'], document['metadata']['name']) + return False + schema_prefix, schema_version = get_schema_parts(document) + return schema_prefix in self._schema_map.get(schema_version, {}) + + def validate(self, document): + return super(DataSchemaValidator, self).validate( + document, validate_section='data', use_fallback_schema=False) + + class DocumentValidation(object): def __init__(self, documents): - """Class for document validation logic for YAML files. + """Class for document validation logic for documents. - This class is responsible for validating YAML files according to their + 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] + :type documents: List[dict] + """ + self.documents = [] + data_schemas = db_api.revision_documents_get( + schema=types.DATA_SCHEMA_SCHEMA, deleted=False) + db_data_schemas = {d['metadata']['name']: d for d in data_schemas} if not isinstance(documents, (list, tuple)): documents = [documents] - try: - for document in documents: - doc = copy.deepcopy(document) - # NOTE(fmontei): Remove extraneous top-level keys so that fully - # rendered documents pass schema validation. - for key in doc.copy(): - if key not in ('metadata', 'schema', 'data'): - doc.pop(key) - self.documents.append(document_wrapper.Document(doc)) - except Exception: - raise errors.InvalidDocumentFormat( - detail='Document could not be converted into a dictionary', - schema='Unknown') + for document in documents: + if document.get('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. + document_name = document.get('metadata', {}).get('name') + if document_name in db_data_schemas: + data_schemas.remove(db_data_schemas.pop(document_name)) + self.documents.append(document) - class SchemaType(object): - """Class for retrieving correct schema for pre-validation on YAML. + # NOTE(fmontei): The order of the validators is important. The + # ``GenericValidator`` must come first. + self._validators = [ + GenericValidator(), + SchemaValidator(), + DataSchemaValidator(data_schemas) + ] - Retrieves the schema that corresponds to "apiVersion" in the YAML - data. This schema is responsible for performing pre-validation on - YAML data. - """ - - schema_versions_info = [ - {'id': 'deckhand/CertificateAuthorityKey', - 'schema': v1_0.certificate_authority_key_schema, - 'version': '1.0'}, - {'id': 'deckhand/CertificateAuthority', - 'schema': v1_0.certificate_authority_schema, - 'version': '1.0'}, - {'id': 'deckhand/CertificateKey', - 'schema': v1_0.certificate_key_schema, - 'version': '1.0'}, - {'id': 'deckhand/Certificate', - 'schema': v1_0.certificate_schema, - 'version': '1.0'}, - {'id': 'deckhand/PrivateKey', - 'schema': v1_0.private_key_schema, - 'version': '1.0'}, - {'id': 'deckhand/PublicKey', - 'schema': v1_0.public_key_schema, - 'version': '1.0'}, - {'id': 'deckhand/DataSchema', - 'schema': v1_0.data_schema_schema, - 'version': '1.0'}, - {'id': 'deckhand/LayeringPolicy', - 'schema': v1_0.layering_policy_schema, - 'version': '1.0'}, - {'id': 'deckhand/Passphrase', - 'schema': v1_0.passphrase_schema, - 'version': '1.0'}, - {'id': 'deckhand/ValidationPolicy', - 'schema': v1_0.validation_policy_schema, - 'version': '1.0'}, - # FIXME(fmontei): Remove this once all Deckhand tests have been - # refactored to account for dynamic schema registeration via - # `DataSchema` documents. Otherwise most tests will fail. - {'id': 'metadata/Document', - 'schema': v1_0.document_schema, - 'version': '1.0'}] - - schema_re = re.compile( - '^([A-Za-z]+\/[A-Za-z]+\/v[1]{1}(\.[0]{1}){0,1})$') - - @classmethod - def _register_data_schemas(cls): - """Dynamically detect schemas for document validation that have - been registered by external services via ``DataSchema`` documents. - """ - data_schemas = db_api.document_get_all( - schema=types.DATA_SCHEMA_SCHEMA, deleted=False) - - for data_schema in data_schemas: - if cls.schema_re.match(data_schema['metadata']['name']): - schema_id = '/'.join( - data_schema['metadata']['name'].split('/')[:2]) - else: - schema_id = data_schema['metadata']['name'] - cls.schema_versions_info.append({ - 'id': schema_id, - 'schema': data_schema['data'], - 'version': '1.0', - 'registered': True, - }) - - @classmethod - def _get_schema_by_property(cls, schema_re, field): - if schema_re.match(field): - schema_id = '/'.join(field.split('/')[:2]) - else: - schema_id = field - - matching_schemas = [] - - for schema in cls.schema_versions_info: - # Can't use `startswith` below to avoid namespace false - # positives like `CertificateKey` and `Certificate`. - if schema_id == schema['id']: - if schema not in matching_schemas: - matching_schemas.append(schema) - return matching_schemas - - @classmethod - def get_schemas(cls, doc): - """Retrieve the relevant schema based on the document's ``schema``. - - :param dict doc: The document used for finding the correct schema - to validate it based on its ``schema``. - :returns: A schema to be used by ``jsonschema`` for document - validation. - :rtype: dict - """ - cls._register_data_schemas() - - # FIXME(fmontei): Remove this once all Deckhand tests have been - # refactored to account for dynamic schema registeration via - # ``DataSchema`` documents. Otherwise most tests will fail. - for doc_field in [doc['schema'], doc['metadata']['schema']]: - matching_schemas = cls._get_schema_by_property( - cls.schema_re, doc_field) - if matching_schemas: - return matching_schemas - - return [] + def _get_supported_schema_list(self): + schema_list = [] + for validator in self._validators[1:]: + for schema_version, schema_map in validator._schema_map.items(): + for schema_prefix in schema_map: + schema_list.append(schema_prefix + '/' + schema_version) + return schema_list def _format_validation_results(self, results): """Format the validation result to be compatible with database formatting. :results: The validation results generated during document validation. - :type results: list[dict] + :type results: List[dict] :returns: List of formatted validation results. - :rtype: `func`:list[dict] + :rtype: List[dict] + """ internal_validator = { 'name': 'deckhand', @@ -195,61 +319,35 @@ class DocumentValidation(object): return formatted_results def _validate_one(self, document): - raw_dict = document.to_dict() - try: - # Subject every document to basic validation to verify that each - # main section is present (schema, metadata, data). - jsonschema.validate(raw_dict, base_schema.schema) - except jsonschema.exceptions.ValidationError as e: - LOG.debug('Document failed top-level schema validation. Details: ' - '%s', e.message) - # NOTE(fmontei): Raise here because if we fail basic schema - # validation, then there is no point in continuing. - raise errors.InvalidDocumentFormat( - detail=e.message, schema=e.schema) - - schemas_to_use = self.SchemaType.get_schemas(raw_dict) - - if not schemas_to_use: - LOG.debug('Document schema %s not recognized.', - document.get_schema()) - # NOTE(fmontei): Raise here because if Deckhand cannot even - # determine which schema to use for further validation, then there - # is no point in trying to continue validation. - raise errors.InvalidDocumentSchema( - document_schema=document.get_schema(), - schema_list=[ - s['id'] for s in self.SchemaType.schema_versions_info]) - result = {'errors': []} - # Perform more detailed validation on each document depending on - # its schema. If the document is abstract, validation errors are - # ignored. - if document.is_abstract(): - LOG.info('Skipping schema validation for abstract ' - 'document: [%s] %s.', document.get_schema(), - document.get_name()) - else: + supported_schema_list = self._get_supported_schema_list() + 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': '.' + }) - for schema_to_use in schemas_to_use: - try: - if isinstance(schema_to_use['schema'], dict): - schema_validator = schema_to_use['schema'] - jsonschema.validate(raw_dict.get('data', {}), - schema_validator) - else: - schema_validator = schema_to_use['schema'].schema - jsonschema.validate(raw_dict, schema_validator) - except jsonschema.exceptions.ValidationError as e: - LOG.error( - 'Document failed schema validation for schema %s.' - 'Details: %s.', document.get_schema(), e.message) - result['errors'].append({ - 'schema': document.get_schema(), - 'name': document.get_name(), - 'message': e.message.replace('u\'', '\'') - }) + for validator in self._validators: + if validator.matches(document): + error_messages = validator.validate(document) + if error_messages: + for error_msg, error_path in error_messages: + result['errors'].append({ + 'schema': document['schema'], + 'name': document['metadata']['name'], + 'message': error_msg, + 'path': error_path + }) if result['errors']: result.setdefault('status', 'failure') @@ -259,17 +357,19 @@ class DocumentValidation(object): return result def validate_all(self): - """Pre-validate that the YAML file is correctly formatted. + """Pre-validate that all documents are correctly formatted. - All concrete documents in the revision successfully pass their JSON - schema validations. The result of the validation is stored under + All concrete documents in the revision must successfully pass their + JSON schema validations. The result of the validation is stored under the "deckhand-document-schema-validation" validation namespace for a document revision. - Validation is broken up into 2 stages: + All abstract documents must themselves be sanity-checked. + + Validation is broken up into 3 stages: 1) Validate that each document contains the basic bulding blocks - needed: ``schema`` and ``metadata`` using a "base" schema. + needed: i.e. ``schema`` and ``metadata`` using a "base" schema. Failing this validation is deemed a critical failure, resulting in an exception. @@ -287,18 +387,45 @@ class DocumentValidation(object): any other non-critical exceptions, which are returned together later. + 3) Execute ``DataSchema`` validations if applicable. + :returns: A list of validations (one for each document validated). - :rtype: `func`:list[dict] + :rtype: List[dict] :raises errors.InvalidDocumentFormat: If the document failed schema validation and the failure is deemed critical. - :raises errors.InvalidDocumentSchema: If no JSON schema for could be - found for executing document validation. + :raises RuntimeError: If a Deckhand schema itself is invalid. + """ + validation_results = [] for document in self.documents: - result = self._validate_one(document) - validation_results.append(result) + # 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) validations = self._format_validation_results(validation_results) return validations + + +def is_abstract(document): + try: + return document['metadata']['layeringDefinition']['abstract'] + except Exception: + return False + + +def get_schema_parts(document, schema_key='schema'): + schema_parts = utils.jsonpath_parse(document, schema_key).split('/') + schema_prefix = '/'.join(schema_parts[:2]) + schema_version = schema_parts[2] + if schema_version.endswith('.0'): + schema_version = schema_version[:-2] + return schema_prefix, schema_version diff --git a/deckhand/engine/schema/base_schema.py b/deckhand/engine/schema/base_schema.py index f9ed563c..97882da4 100644 --- a/deckhand/engine/schema/base_schema.py +++ b/deckhand/engine/schema/base_schema.py @@ -18,7 +18,7 @@ schema = { 'schema': { 'type': 'string', # Currently supported versions include v1/v1.0 only. - 'pattern': '^([A-Za-z]+\/[A-Za-z]+\/v[1]{1}(\.[0]{1}){0,1})$' + 'pattern': '^[A-Za-z]+\/[A-Za-z]+\/v\d+(.0)?$' }, 'metadata': { 'type': 'object', diff --git a/deckhand/engine/schema/v1_0/certificate_key_schema.py b/deckhand/engine/schema/v1_0/certificate_key_schema.py index be3b95ce..76d6c9d0 100644 --- a/deckhand/engine/schema/v1_0/certificate_key_schema.py +++ b/deckhand/engine/schema/v1_0/certificate_key_schema.py @@ -17,21 +17,22 @@ schema = { 'properties': { 'schema': { 'type': 'string', - 'pattern': '^(deckhand/CertificateKey/v[1]{1}(\.[0]{1}){0,1})$' + 'pattern': '^deckhand/CertificateKey/v\d+(.0)?$' }, 'metadata': { 'type': 'object', 'properties': { 'schema': { 'type': 'string', - 'pattern': '^(metadata/Document/v[1]{1}(\.[0]{1}){0,1})$', + 'pattern': '^metadata/Document/v\d+(.0)?$', }, 'name': {'type': 'string'}, # Not strictly needed for secrets. 'layeringDefinition': { 'type': 'object', 'properties': { - 'layer': {'type': 'string'} + 'layer': {'type': 'string'}, + 'abstract': {'type': 'boolean'} } }, 'storagePolicy': { diff --git a/deckhand/engine/schema/v1_0/certificate_schema.py b/deckhand/engine/schema/v1_0/certificate_schema.py index ef26c11f..54c87a19 100644 --- a/deckhand/engine/schema/v1_0/certificate_schema.py +++ b/deckhand/engine/schema/v1_0/certificate_schema.py @@ -17,21 +17,22 @@ schema = { 'properties': { 'schema': { 'type': 'string', - 'pattern': '^(deckhand/Certificate/v[1]{1}(\.[0]{1}){0,1})$' + 'pattern': '^deckhand/Certificate/v\d+(.0)?$' }, 'metadata': { 'type': 'object', 'properties': { 'schema': { 'type': 'string', - 'pattern': '^(metadata/Document/v[1]{1}(\.[0]{1}){0,1})$', + 'pattern': '^metadata/Document/v\d+(.0)?$', }, 'name': {'type': 'string'}, # Not strictly needed for secrets. 'layeringDefinition': { 'type': 'object', 'properties': { - 'layer': {'type': 'string'} + 'layer': {'type': 'string'}, + 'abstract': {'type': 'boolean'} } }, 'storagePolicy': { diff --git a/deckhand/engine/schema/v1_0/data_schema_schema.py b/deckhand/engine/schema/v1_0/data_schema_schema.py index d4dbaf11..0549adde 100644 --- a/deckhand/engine/schema/v1_0/data_schema_schema.py +++ b/deckhand/engine/schema/v1_0/data_schema_schema.py @@ -20,19 +20,18 @@ schema = { 'properties': { 'schema': { 'type': 'string', - 'pattern': '^(deckhand/DataSchema/v[1]{1}(\.[0]{1}){0,1})$' + 'pattern': '^deckhand/DataSchema/v\d+(.0)?$' }, 'metadata': { 'type': 'object', 'properties': { 'schema': { 'type': 'string', - 'pattern': '^(metadata/Control/v[1]{1}(\.[0]{1}){0,1})$' + 'pattern': '^metadata/Control/v\d+(.0)?$' }, 'name': { 'type': 'string', - 'pattern': ( - '^([A-Za-z]+\/[A-Za-z]+\/v[1]{1}(\.[0]{1}){0,1})$') + 'pattern': '^[A-Za-z]+\/[A-Za-z]+\/v\d+(.0)?$' }, # Labels are optional. 'labels': { @@ -43,7 +42,7 @@ schema = { 'enum': ['encrypted', 'cleartext'] } }, - 'additionalProperties': False, + 'additionalProperties': True, # Can include layeringDefinition. 'required': ['schema', 'name'] }, 'data': { @@ -65,7 +64,7 @@ schema = { .. literalinclude:: ../../deckhand/engine/schema/v1_0/data_schema_schema.py :language: python - :lines: 15-62 + :lines: 15-61 This schema is used to sanity-check all DataSchema documents that are passed to Deckhand. This schema is only enforced after validation for diff --git a/deckhand/engine/schema/v1_0/document_schema.py b/deckhand/engine/schema/v1_0/document_schema.py index 5f5a2651..bc1c9185 100644 --- a/deckhand/engine/schema/v1_0/document_schema.py +++ b/deckhand/engine/schema/v1_0/document_schema.py @@ -44,14 +44,14 @@ schema = { 'properties': { 'schema': { 'type': 'string', - 'pattern': '^([A-Za-z]+/[A-Za-z]+/v[1]{1}(\.[0]{1}){0,1})$' + 'pattern': '^[A-Za-z]+/[A-Za-z]+/v\d+(.0)?$' }, 'metadata': { 'type': 'object', 'properties': { 'schema': { 'type': 'string', - 'pattern': '^(metadata/Document/v[1]{1}(\.[0]{1}){0,1})$' + 'pattern': '^metadata/Document/v\d+(.0)?$' }, 'name': {'type': 'string'}, 'labels': {'type': 'object'}, diff --git a/deckhand/engine/schema/v1_0/layering_policy_schema.py b/deckhand/engine/schema/v1_0/layering_policy_schema.py index 1ac4e810..277f63f9 100644 --- a/deckhand/engine/schema/v1_0/layering_policy_schema.py +++ b/deckhand/engine/schema/v1_0/layering_policy_schema.py @@ -17,14 +17,14 @@ schema = { 'properties': { 'schema': { 'type': 'string', - 'pattern': '^(deckhand/LayeringPolicy/v[1]{1}(\.[0]{1}){0,1})$' + 'pattern': '^deckhand/LayeringPolicy/v\d+(.0)?$' }, 'metadata': { 'type': 'object', 'properties': { 'schema': { 'type': 'string', - 'pattern': '^(metadata/Control/v[1]{1}(\.[0]{1}){0,1})$' + 'pattern': '^metadata/Control/v\d+(.0)?$' }, 'name': {'type': 'string'}, 'storagePolicy': { diff --git a/deckhand/engine/schema/v1_0/passphrase_schema.py b/deckhand/engine/schema/v1_0/passphrase_schema.py index 98dc553b..02ddb928 100644 --- a/deckhand/engine/schema/v1_0/passphrase_schema.py +++ b/deckhand/engine/schema/v1_0/passphrase_schema.py @@ -17,21 +17,22 @@ schema = { 'properties': { 'schema': { 'type': 'string', - 'pattern': '^(deckhand/Passphrase/v[1]{1}(\.[0]{1}){0,1})$' + 'pattern': '^deckhand/Passphrase/v\d+(.0)?$' }, 'metadata': { 'type': 'object', 'properties': { 'schema': { 'type': 'string', - 'pattern': '^(metadata/Document/v[1]{1}(\.[0]{1}){0,1})$', + 'pattern': '^metadata/Document/v\d+(.0)?$', }, 'name': {'type': 'string'}, - # Not strictly needed for secrets. + # Not strictly needed. 'layeringDefinition': { 'type': 'object', 'properties': { - 'layer': {'type': 'string'} + 'layer': {'type': 'string'}, + 'abstract': {'type': 'boolean'} } }, 'storagePolicy': { diff --git a/deckhand/engine/schema/v1_0/validation_policy_schema.py b/deckhand/engine/schema/v1_0/validation_policy_schema.py index 7c2a0995..08fef701 100644 --- a/deckhand/engine/schema/v1_0/validation_policy_schema.py +++ b/deckhand/engine/schema/v1_0/validation_policy_schema.py @@ -17,14 +17,14 @@ schema = { 'properties': { 'schema': { 'type': 'string', - 'pattern': '^(deckhand/ValidationPolicy/v[1]{1}(\.[0]{1}){0,1})$' + 'pattern': '^deckhand/ValidationPolicy/v\d+(.0)?$' }, 'metadata': { 'type': 'object', 'properties': { 'schema': { 'type': 'string', - 'pattern': '^(metadata/Control/v[1]{1}(\.[0]{1}){0,1})$' + 'pattern': '^metadata/Control/v\d+(.0)?$' }, 'name': {'type': 'string'}, 'storagePolicy': { diff --git a/deckhand/errors.py b/deckhand/errors.py index 2fea8c14..aa52462c 100644 --- a/deckhand/errors.py +++ b/deckhand/errors.py @@ -171,14 +171,8 @@ class DeckhandException(Exception): class InvalidDocumentFormat(DeckhandException): - msg_fmt = ("The provided document YAML failed schema validation. Details: " - "%(detail)s. Schema: %(schema)s.") - code = 400 - - -class InvalidDocumentSchema(DeckhandException): - msg_fmt = ("The provided %(document_schema)s is invalid. Supported " - "schemas: %(schema_list)s.") + msg_fmt = ("The provided document failed schema validation. Details: " + "%(details)s") code = 400 diff --git a/deckhand/factories.py b/deckhand/factories.py index ee73f84a..8a06ef3a 100644 --- a/deckhand/factories.py +++ b/deckhand/factories.py @@ -79,7 +79,8 @@ class DataSchemaFactory(DeckhandFactory): data_schema_template['metadata']['name'] = metadata_name data_schema_template['metadata']['labels'] = metadata_labels - data_schema_template['data'] = data + if data: + data_schema_template['data'] = data return data_schema_template diff --git a/deckhand/tests/functional/gabbits/document-crud-success-single-bucket.yaml b/deckhand/tests/functional/gabbits/document-crud-success-single-bucket.yaml index ba11d139..dc8b9ee6 100644 --- a/deckhand/tests/functional/gabbits/document-crud-success-single-bucket.yaml +++ b/deckhand/tests/functional/gabbits/document-crud-success-single-bucket.yaml @@ -54,12 +54,14 @@ tests: - name: verify_initial desc: Verify initial document count and revisions GET: /api/v1.0/revisions/$RESPONSE['$.[0].status.revision']/documents + query_parameters: + sort: metadata.name status: 200 response_multidoc_jsonpaths: $.`len`: 4 $.[*].metadata.name: - - layering-policy - global-1234 + - layering-policy - region-1234 - site-1234 $.[*].status.revision: @@ -83,11 +85,16 @@ tests: desc: Verify duplicate documents were ignored GET: /api/v1.0/revisions/$HISTORY['initialize'].$RESPONSE['$.[0].status.revision']/documents status: 200 + response_multidoc_jsonpaths: + $.`len`: 4 + query_parameters: + sort: metadata.name + status: 200 response_multidoc_jsonpaths: $.`len`: 4 $.[*].metadata.name: - - layering-policy - global-1234 + - layering-policy - region-1234 - site-1234 $.[*].status.revision: @@ -110,12 +117,14 @@ tests: - name: verify_update desc: Verify updated document count and revisions GET: /api/v1.0/revisions/$RESPONSE['$.[0].status.revision']/documents + query_parameters: + sort: metadata.name status: 200 response_multidoc_jsonpaths: $.`len`: 4 $.[*].metadata.name: - - layering-policy - global-1234 + - layering-policy - region-1234 - site-1234 $.[*].status.revision: @@ -133,12 +142,14 @@ tests: - name: verify_initial_documents_preserved_after_update desc: Verify initial documents count and revisions preserved after update GET: /api/v1.0/revisions/$HISTORY['initialize'].$RESPONSE['$.[0].status.revision']/documents + query_parameters: + sort: metadata.name status: 200 response_multidoc_jsonpaths: $.`len`: 4 $.[*].metadata.name: - - layering-policy - global-1234 + - layering-policy - region-1234 - site-1234 $.[*].status.revision: @@ -162,6 +173,8 @@ tests: - name: verify_delete desc: Verify document deletion GET: /api/v1.0/revisions/$RESPONSE['$.[0].status.revision']/documents + query_parameters: + sort: metadata.name status: 200 response_multidoc_jsonpaths: $.`len`: 3 @@ -170,8 +183,8 @@ tests: - "$HISTORY['initialize'].$RESPONSE['$.[0].status.revision']" - "$HISTORY['update_single_document'].$RESPONSE['$.[0].status.revision']" $.[*].metadata.name: - - layering-policy - global-1234 + - layering-policy - site-1234 $.[*].status.bucket: - mop @@ -182,12 +195,14 @@ tests: - name: verify_initial_documents_preserved_after_delete desc: Verify initial documents count and revisions GET: /api/v1.0/revisions/$HISTORY['initialize'].$RESPONSE['$.[0].status.revision']/documents + query_parameters: + sort: metadata.name status: 200 response_multidoc_jsonpaths: $.`len`: 4 $.[*].metadata.name: - - layering-policy - global-1234 + - layering-policy - region-1234 - site-1234 $.[*].status.revision: @@ -205,12 +220,14 @@ tests: - name: verify_updated_documents_preserved_after_delete desc: Verify updated documents count and revisions preserved after delete GET: /api/v1.0/revisions/$HISTORY['update_single_document'].$RESPONSE['$.[0].status.revision']/documents + query_parameters: + sort: metadata.name status: 200 response_multidoc_jsonpaths: $.`len`: 4 $.[*].metadata.name: - - layering-policy - global-1234 + - layering-policy - region-1234 - site-1234 $.[*].status.revision: diff --git a/deckhand/tests/functional/gabbits/revision-diff-success.yaml b/deckhand/tests/functional/gabbits/revision-diff-success.yaml index 83ee37d1..37eb2244 100644 --- a/deckhand/tests/functional/gabbits/revision-diff-success.yaml +++ b/deckhand/tests/functional/gabbits/revision-diff-success.yaml @@ -196,7 +196,6 @@ tests: layer: site data: value: mistake - ... - name: delete_mistake desc: Delete documents from bucket mistake diff --git a/deckhand/tests/functional/gabbits/revision-documents-filters.yaml b/deckhand/tests/functional/gabbits/revision-documents-filters.yaml index 0ca9b2ea..c491f39c 100644 --- a/deckhand/tests/functional/gabbits/revision-documents-filters.yaml +++ b/deckhand/tests/functional/gabbits/revision-documents-filters.yaml @@ -123,12 +123,13 @@ tests: GET: /api/v1.0/revisions/$RESPONSE['$.[0].status.revision']/documents query_parameters: status.bucket: mop + sort: metadata.name status: 200 response_multidoc_jsonpaths: $.`len`: 4 $.[*].metadata.name: - - layering-policy - global-1234 + - layering-policy - region-1234 - site-1234 $.[*].status.bucket: diff --git a/deckhand/tests/functional/gabbits/revision-tag-success.yaml b/deckhand/tests/functional/gabbits/revision-tag-success.yaml index 0eae78e8..6ec283b0 100644 --- a/deckhand/tests/functional/gabbits/revision-tag-success.yaml +++ b/deckhand/tests/functional/gabbits/revision-tag-success.yaml @@ -36,7 +36,6 @@ tests: status: 204 response_headers: null - # Create a revision implicitly by creating a document. - name: initialize desc: Create initial documents PUT: /api/v1.0/buckets/mop/documents @@ -76,7 +75,7 @@ tests: GET: /api/v1.0/revisions status: 200 response_multidoc_jsonpaths: - $.`len`: 1 + $.[0].results.`len`: 1 $.[0].results[0].tags.`len`: 1 $.[0].results[0].tags: [foo] @@ -124,7 +123,7 @@ tests: GET: /api/v1.0/revisions status: 200 response_multidoc_jsonpaths: - $.`len`: 1 + $.[0].results.`len`: 1 $.[0].results[0].tags.`len`: 2 $.[0].results[0].tags: [bar, foo] @@ -162,7 +161,7 @@ tests: GET: /api/v1.0/revisions status: 200 response_multidoc_jsonpaths: - $.`len`: 1 + $.[0].results.`len`: 1 $.[0].results[0].tags.`len`: 1 $.[0].results[0].tags: [bar] @@ -185,7 +184,7 @@ tests: GET: /api/v1.0/revisions status: 200 response_multidoc_jsonpaths: - $.`len`: 1 + $.[0].results.`len`: 1 $.[0].results[0].tags: [] - name: verify_tag_delete_all diff --git a/deckhand/tests/functional/gabbits/rollback-success-single-bucket.yaml b/deckhand/tests/functional/gabbits/rollback-success-single-bucket.yaml index 25f3e2b3..b82543a4 100644 --- a/deckhand/tests/functional/gabbits/rollback-success-single-bucket.yaml +++ b/deckhand/tests/functional/gabbits/rollback-success-single-bucket.yaml @@ -52,12 +52,14 @@ tests: - name: verify_revision_1 desc: Verify initial document count and revisions GET: /api/v1.0/revisions/$HISTORY['initialize'].$RESPONSE['$.[0].status.revision']/documents + query_parameters: + sort: metadata.name status: 200 response_multidoc_jsonpaths: $.`len`: 4 $.[*].metadata.name: - - layering-policy - global-1234 + - layering-policy - region-1234 - site-1234 $.[*].status.revision: @@ -70,17 +72,23 @@ tests: - mop - mop - mop + $.[0].data.a: + x: 1 + y: 2 + $.[2].data.a.z: 3 $.[3].data.b: 4 - name: verify_revision_2 desc: Verify updated document count and revisions GET: /api/v1.0/revisions/$HISTORY['update_single_document'].$RESPONSE['$.[0].status.revision']/documents + query_parameters: + sort: metadata.name status: 200 response_multidoc_jsonpaths: $.`len`: 4 $.[*].metadata.name: - - layering-policy - global-1234 + - layering-policy - region-1234 - site-1234 $.[*].status.revision: @@ -93,17 +101,23 @@ tests: - mop - mop - mop + $.[0].data.a: + x: 1 + y: 2 + $.[2].data.a.z: 3 $.[3].data.b: 5 - name: verify_revision_3 desc: Verify document deletion GET: /api/v1.0/revisions/$HISTORY['delete_document'].$RESPONSE['$.[0].status.revision']/documents + query_parameters: + sort: metadata.name status: 200 response_multidoc_jsonpaths: $.`len`: 3 $.[*].metadata.name: - - layering-policy - global-1234 + - layering-policy - site-1234 $.[*].status.revision: - "$HISTORY['initialize'].$RESPONSE['$.[0].status.revision']" @@ -113,17 +127,22 @@ tests: - mop - mop - mop + $.[0].data.a: + x: 1 + y: 2 $.[2].data.b: 5 - name: verify_revision_4 desc: Verify rollback revision GET: /api/v1.0/revisions/$HISTORY['rollback'].$RESPONSE['$.[0].id']/documents + query_parameters: + sort: metadata.name status: 200 response_multidoc_jsonpaths: $.`len`: 4 $.[*].metadata.name: - - layering-policy - global-1234 + - layering-policy - region-1234 - site-1234 $.[*].status.revision: @@ -136,4 +155,8 @@ tests: - mop - mop - mop + $.[0].data.a: + x: 1 + y: 2 + $.[2].data.a.z: 3 $.[3].data.b: 4 diff --git a/deckhand/tests/unit/base.py b/deckhand/tests/unit/base.py index fec6514a..f8155292 100644 --- a/deckhand/tests/unit/base.py +++ b/deckhand/tests/unit/base.py @@ -88,14 +88,14 @@ class DeckhandTestCase(testtools.TestCase): self.addCleanup(p.stop) return m - def patchobject(self, target, attribute, new=mock.DEFAULT, autospec=True): + def patchobject(self, target, attribute, new=mock.DEFAULT, **kwargs): """Convenient wrapper around `mock.patch.object` Returns a started mock that will be automatically stopped after the test ran. """ - p = mock.patch.object(target, attribute, new, autospec=autospec) + p = mock.patch.object(target, attribute, new, **kwargs) m = p.start() self.addCleanup(p.stop) return m diff --git a/deckhand/tests/unit/control/test_buckets_controller.py b/deckhand/tests/unit/control/test_buckets_controller.py index 5a0935b8..377663d2 100644 --- a/deckhand/tests/unit/control/test_buckets_controller.py +++ b/deckhand/tests/unit/control/test_buckets_controller.py @@ -68,14 +68,15 @@ class TestBucketsController(test_base.BaseControllerTest): def test_put_bucket_with_secret(self): def _do_test(payload): + bucket_name = test_utils.rand_name('bucket') resp = self.app.simulate_put( - '/api/v1.0/buckets/mop/documents', + '/api/v1.0/buckets/%s/documents' % bucket_name, headers={'Content-Type': 'application/x-yaml'}, body=yaml.safe_dump_all(payload)) self.assertEqual(200, resp.status_code) created_documents = list(yaml.safe_load_all(resp.text)) - self.assertEqual(1, len(created_documents)) + self.assertEqual(len(payload), len(created_documents)) expected = sorted([(d['schema'], d['metadata']['name']) for d in payload]) actual = sorted([(d['schema'], d['metadata']['name']) @@ -108,16 +109,16 @@ class TestBucketsController(test_base.BaseControllerTest): # `metadata.storagePolicy`='encrypted'. In the case below, # a generic document is tested. documents_factory = factories.DocumentFactory(1, [1]) - document_mapping = { - "_GLOBAL_DATA_1_": {"data": {"a": {"x": 1, "y": 2}}} - } - payload = documents_factory.gen_test(document_mapping, - global_abstract=False) - payload[-1]['metadata']['storagePolicy'] = 'encrypted' + document = documents_factory.gen_test({}, global_abstract=False)[-1] + document['metadata']['storagePolicy'] = 'encrypted' + + data_schema_factory = factories.DataSchemaFactory() + data_schema = data_schema_factory.gen_test(document['schema'], {}) + with mock.patch.object(buckets.BucketsResource, 'secrets_mgr', autospec=True) as mock_secrets_mgr: - mock_secrets_mgr.create.return_value = payload[-1]['data'] - _do_test([payload[-1]]) + mock_secrets_mgr.create.return_value = document['data'] + _do_test([document, data_schema]) def test_create_delete_then_recreate_document_in_different_bucket(self): """Ordiniarly creating a document with the same metadata.name/schema @@ -164,28 +165,6 @@ class TestBucketsController(test_base.BaseControllerTest): class TestBucketsControllerNegative(test_base.BaseControllerTest): """Test suite for validating negative scenarios for bucket controller.""" - def test_put_bucket_with_invalid_document_payload(self): - rules = {'deckhand:create_cleartext_documents': '@'} - self.policy.set_rules(rules) - - no_colon_spaces = """ -name:foo -schema: - layeringDefinition: - layer:site -""" - invalid_payloads = ['garbage', no_colon_spaces] - error_re = ['.*The provided document YAML failed schema validation.*', - '.*mapping values are not allowed here.*'] - - for idx, payload in enumerate(invalid_payloads): - resp = self.app.simulate_put( - '/api/v1.0/buckets/mop/documents', - headers={'Content-Type': 'application/x-yaml'}, - body=payload) - self.assertEqual(400, resp.status_code) - self.assertRegexpMatches(resp.text, error_re[idx]) - def test_put_conflicting_layering_policy(self): rules = {'deckhand:create_cleartext_documents': '@'} self.policy.set_rules(rules) diff --git a/deckhand/tests/unit/control/test_middleware.py b/deckhand/tests/unit/control/test_middleware.py index aa345960..c2c76442 100644 --- a/deckhand/tests/unit/control/test_middleware.py +++ b/deckhand/tests/unit/control/test_middleware.py @@ -16,6 +16,7 @@ import yaml import mock +from deckhand import factories from deckhand.tests.unit.control import base as test_base @@ -24,11 +25,14 @@ class TestYAMLTranslator(test_base.BaseControllerTest): def test_request_with_correct_content_type(self): rules = {'deckhand:create_cleartext_documents': '@'} self.policy.set_rules(rules) - self._read_data('sample_document_simple') + + documents_factory = factories.DocumentFactory(1, [1]) + document = documents_factory.gen_test({})[-1] + resp = self.app.simulate_put( '/api/v1.0/buckets/b1/documents', headers={'Content-Type': 'application/x-yaml'}, - body=yaml.safe_dump(self.data), + body=yaml.safe_dump(document), ) self.assertEqual(200, resp.status_code) @@ -84,11 +88,14 @@ class TestYAMLTranslator(test_base.BaseControllerTest): def test_request_with_correct_content_type_plus_encoding(self): rules = {'deckhand:create_cleartext_documents': '@'} self.policy.set_rules(rules) - self._read_data('sample_document_simple') + + documents_factory = factories.DocumentFactory(1, [1]) + document = documents_factory.gen_test({})[-1] + resp = self.app.simulate_put( '/api/v1.0/buckets/b1/documents', headers={'Content-Type': 'application/x-yaml;encoding=utf-8'}, - body=yaml.safe_dump(self.data), + body=yaml.safe_dump(document), ) self.assertEqual(200, resp.status_code) diff --git a/deckhand/tests/unit/control/test_rendered_documents_controller.py b/deckhand/tests/unit/control/test_rendered_documents_controller.py index 17f90ce4..dd9b28c2 100644 --- a/deckhand/tests/unit/control/test_rendered_documents_controller.py +++ b/deckhand/tests/unit/control/test_rendered_documents_controller.py @@ -52,24 +52,22 @@ class TestRenderedDocumentsController(test_base.BaseControllerTest): '/api/v1.0/revisions/%s/rendered-documents' % revision_id, headers={'Content-Type': 'application/x-yaml'}) self.assertEqual(200, resp.status_code) - rendered_documents = list(yaml.safe_load_all(resp.text)) - # TODO(fmontei): Implement "negative" filter server-side. - rendered_documents = [ - d for d in rendered_documents - if not d['schema'].startswith(types.LAYERING_POLICY_SCHEMA) - ] - self.assertEqual(1, len(rendered_documents)) - is_abstract = rendered_documents[0]['metadata']['layeringDefinition'][ + self.assertEqual(2, len(rendered_documents)) + rendered_documents = list(filter( + lambda x: not x['schema'].startswith(types.LAYERING_POLICY_SCHEMA), + rendered_documents)) + + is_abstract = rendered_documents[-1]['metadata']['layeringDefinition'][ 'abstract'] self.assertFalse(is_abstract) for key, value in concrete_doc.items(): if isinstance(value, dict): self.assertDictContainsSubset(value, - rendered_documents[0][key]) + rendered_documents[-1][key]) else: - self.assertEqual(value, rendered_documents[0][key]) + self.assertEqual(value, rendered_documents[-1][key]) def test_list_rendered_documents_exclude_deleted_documents(self): """Verifies that documents from previous revisions that have been @@ -83,40 +81,40 @@ class TestRenderedDocumentsController(test_base.BaseControllerTest): 'deckhand:create_cleartext_documents': '@'} self.policy.set_rules(rules) - # Create 1st document. + # PUT a bunch of documents, include a layeringPolicy. documents_factory = factories.DocumentFactory(1, [1]) - payload = documents_factory.gen_test({}, global_abstract=False)[1:] + payload = documents_factory.gen_test({}, global_abstract=False) resp = self.app.simulate_put( '/api/v1.0/buckets/mop/documents', headers={'Content-Type': 'application/x-yaml'}, body=yaml.safe_dump_all(payload)) self.assertEqual(200, resp.status_code) - # Create 2nd document (exclude 1st document in new payload). + # PUT new document (exclude original documents from this payload). payload = documents_factory.gen_test({}, global_abstract=False) - new_name = payload[-1]['metadata']['name'] + new_name = payload[1]['metadata']['name'] resp = self.app.simulate_put( '/api/v1.0/buckets/mop/documents', headers={'Content-Type': 'application/x-yaml'}, - body=yaml.safe_dump_all(payload)) + body=yaml.safe_dump_all([payload[1]])) self.assertEqual(200, resp.status_code) revision_id = list(yaml.safe_load_all(resp.text))[0]['status'][ 'revision'] - # Verify that only the 2nd is returned for revision_id=2. + # Verify that only the document with `new_name` is returned. (The + # layeringPolicy) is omitted from the response even though it still + # exists. resp = self.app.simulate_get( '/api/v1.0/revisions/%s/rendered-documents' % revision_id, headers={'Content-Type': 'application/x-yaml'}) self.assertEqual(200, resp.status_code) - rendered_documents = list(yaml.safe_load_all(resp.text)) - # TODO(fmontei): Implement "negative" filter server-side. - rendered_documents = [ - d for d in rendered_documents - if not d['schema'].startswith(types.LAYERING_POLICY_SCHEMA) - ] - self.assertEqual(1, len(rendered_documents)) + self.assertEqual(2, len(rendered_documents)) + rendered_documents = list(filter( + lambda x: not x['schema'].startswith(types.LAYERING_POLICY_SCHEMA), + rendered_documents)) + self.assertEqual(new_name, rendered_documents[0]['metadata']['name']) self.assertEqual(2, rendered_documents[0]['status']['revision']) diff --git a/deckhand/tests/unit/control/test_validations_controller.py b/deckhand/tests/unit/control/test_validations_controller.py index 35728efc..7b50f921 100644 --- a/deckhand/tests/unit/control/test_validations_controller.py +++ b/deckhand/tests/unit/control/test_validations_controller.py @@ -59,8 +59,13 @@ class TestValidationsController(test_base.BaseControllerTest): def _create_revision(self, payload=None): if not payload: - documents_factory = factories.DocumentFactory(2, [1, 1]) + documents_factory = factories.DocumentFactory(1, [1]) payload = documents_factory.gen_test({}) + data_schema_factory = factories.DataSchemaFactory() + data_schema = data_schema_factory.gen_test( + payload[1]['schema'], data={}) + payload.append(data_schema) + resp = self.app.simulate_put( '/api/v1.0/buckets/mop/documents', headers={'Content-Type': 'application/x-yaml'}, @@ -339,7 +344,7 @@ class TestValidationsController(test_base.BaseControllerTest): 'deckhand:list_validations': '@'} self.policy.set_rules(rules) - # Register a `DataSchema` against which the test document will be + # Create a `DataSchema` against which the test document will be # validated. data_schema_factory = factories.DataSchemaFactory() metadata_name = 'example/Doc/v1' @@ -357,22 +362,6 @@ class TestValidationsController(test_base.BaseControllerTest): data_schema = data_schema_factory.gen_test( metadata_name, data=schema_to_use) - revision_id = self._create_revision(payload=[data_schema]) - - # Validate that the internal deckhand validation was created. - 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) - # Create the test document whose data section adheres to the # `DataSchema` above. doc_factory = factories.DocumentFactory(1, [1]) @@ -381,10 +370,9 @@ class TestValidationsController(test_base.BaseControllerTest): global_abstract=False)[-1] doc_to_test['schema'] = 'example/Doc/v1' - revision_id = self._create_revision( - payload=[doc_to_test]) + revision_id = self._create_revision(payload=[doc_to_test, data_schema]) - # Validate that the validation was created and passed. + # Validate that the validation was created and succeeded. resp = self.app.simulate_get( '/api/v1.0/revisions/%s/validations' % revision_id, headers={'Content-Type': 'application/x-yaml'}) @@ -398,12 +386,16 @@ class TestValidationsController(test_base.BaseControllerTest): } self.assertEqual(expected_body, body) - def test_validation_with_registered_data_schema_expect_failure(self): + def test_validation_data_schema_different_revision_expect_failure(self): + """Validates that creating a ``DataSchema`` in one revision and then + creating a document in another revision that relies on the previously + created ``DataSchema`` results in an expected failure. + """ rules = {'deckhand:create_cleartext_documents': '@', 'deckhand:list_validations': '@'} self.policy.set_rules(rules) - # Register a `DataSchema` against which the test document will be + # Create a `DataSchema` against which the test document will be # validated. data_schema_factory = factories.DataSchemaFactory() metadata_name = 'example/foo/v1' @@ -419,7 +411,6 @@ class TestValidationsController(test_base.BaseControllerTest): } data_schema = data_schema_factory.gen_test( metadata_name, data=schema_to_use) - revision_id = self._create_revision(payload=[data_schema]) # Validate that the internal deckhand validation was created. @@ -461,13 +452,15 @@ class TestValidationsController(test_base.BaseControllerTest): } self.assertEqual(expected_body, body) - def test_validation_with_registered_data_schema_expect_mixed(self): + 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. + """ rules = {'deckhand:create_cleartext_documents': '@', - 'deckhand:list_validations': '@', - 'deckhand:show_validation': '@'} + 'deckhand:list_validations': '@'} self.policy.set_rules(rules) - # Register a `DataSchema` against which the test document will be + # Create a `DataSchema` against which the test document will be # validated. data_schema_factory = factories.DataSchemaFactory() metadata_name = 'example/foo/v1' @@ -484,9 +477,18 @@ class TestValidationsController(test_base.BaseControllerTest): data_schema = data_schema_factory.gen_test( metadata_name, data=schema_to_use) - revision_id = self._create_revision(payload=[data_schema]) + # 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( + {'_GLOBAL_DATA_1_': {'data': {'a': 'fail'}}}, + global_abstract=False)[-1] + doc_to_test['schema'] = 'example/foo/v1' + doc_to_test['metadata']['name'] = 'test_doc' - # Validate that the internal deckhand validation was created. + revision_id = self._create_revision(payload=[doc_to_test, data_schema]) + + # Validate that the validation was created and reports failure. resp = self.app.simulate_get( '/api/v1.0/revisions/%s/validations' % revision_id, headers={'Content-Type': 'application/x-yaml'}) @@ -495,11 +497,110 @@ class TestValidationsController(test_base.BaseControllerTest): expected_body = { 'count': 1, 'results': [ - {'name': types.DECKHAND_SCHEMA_VALIDATION, 'status': 'success'} + {'name': types.DECKHAND_SCHEMA_VALIDATION, 'status': 'failure'} ] } self.assertEqual(expected_body, body) + def test_validation_with_registered_data_schema_expect_multi_failure(self): + rules = {'deckhand:create_cleartext_documents': '@', + 'deckhand:list_validations': '@', + 'deckhand:show_validation': '@'} + 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) + + # Failure #1. + # 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( + {'_GLOBAL_DATA_1_': {'data': {'a': 'fail'}}}, + global_abstract=False)[-1] + doc_to_test['schema'] = 'example/foo/v1' + doc_to_test['metadata']['name'] = 'test_doc' + # Failure #2. + # Remove required metadata property, causing error to be generated. + del doc_to_test['metadata']['layeringDefinition'] + + revision_id = self._create_revision(payload=[doc_to_test, data_schema]) + + # Validate that the validation was created and reports failure. + 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': 'failure'} + ] + } + self.assertEqual(expected_body, body) + + # Validate that both expected errors are present for validation. + expected_errors = [ + { + 'message': "'layeringDefinition' is a required property", + 'name': 'test_doc', + 'schema': 'example/foo/v1', + 'path': '.metadata' + }, { + 'message': "'fail' is not of type 'integer'", + 'name': 'test_doc', + 'schema': 'example/foo/v1', + 'path': '.data.a' + } + ] + resp = self.app.simulate_get( + '/api/v1.0/revisions/%s/validations/%s/entries/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) + + self.assertEqual('failure', body['status']) + self.assertEqual(expected_errors, body['errors']) + + def test_validation_with_registered_data_schema_expect_mixed(self): + rules = {'deckhand:create_cleartext_documents': '@', + 'deckhand:list_validations': '@', + 'deckhand:show_validation': '@'} + 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( @@ -511,7 +612,8 @@ class TestValidationsController(test_base.BaseControllerTest): pass_doc = copy.deepcopy(fail_doc) pass_doc['data']['a'] = 5 - revision_id = self._create_revision(payload=[fail_doc, pass_doc]) + revision_id = self._create_revision( + payload=[fail_doc, pass_doc, data_schema]) # Validate that the validation reports failure since `fail_doc` # should've failed validation. @@ -535,9 +637,10 @@ class TestValidationsController(test_base.BaseControllerTest): self.assertEqual(200, resp.status_code) body = yaml.safe_load(resp.text) expected_body = { - 'count': 2, + 'count': 3, 'results': [{'id': 0, 'status': 'failure'}, # fail_doc failed. - {'id': 1, 'status': 'success'}] # pass_doc succeeded. + {'id': 1, 'status': 'success'}, # DataSchema passed. + {'id': 2, 'status': 'success'}] # pass_doc succeeded. } self.assertEqual(expected_body, body) @@ -551,7 +654,8 @@ class TestValidationsController(test_base.BaseControllerTest): expected_errors = [{ 'schema': 'example/foo/v1', 'name': 'test_doc', - 'message': "'fail' is not of type 'integer'" + 'message': "'fail' is not of type 'integer'", + 'path': '.data.a' }] self.assertIn('errors', body) self.assertEqual(expected_errors, body['errors']) @@ -563,14 +667,18 @@ class TestValidationsController(test_base.BaseControllerTest): depends on substitution from another document. """ rules = {'deckhand:create_cleartext_documents': '@', - 'deckhand:list_validations': '@'} + 'deckhand:list_validations': '@', + 'deckhand:show_validation': '@'} self.policy.set_rules(rules) documents_factory = factories.DocumentFactory(1, [1]) - payload = documents_factory.gen_test({}, global_abstract=False)[-1] - del payload['data'] + document = documents_factory.gen_test({}, global_abstract=False)[-1] + del document['data'] - revision_id = self._create_revision(payload=[payload]) + data_schema_factory = factories.DataSchemaFactory() + data_schema = data_schema_factory.gen_test(document['schema'], {}) + + revision_id = self._create_revision(payload=[document, data_schema]) # Validate that the entry is present. resp = self.app.simulate_get( @@ -581,7 +689,92 @@ class TestValidationsController(test_base.BaseControllerTest): body = yaml.safe_load(resp.text) expected_body = { - 'count': 1, - 'results': [{'id': 0, 'status': 'failure'}] + 'count': 2, + 'results': [{'id': 0, 'status': 'failure'}, # Document. + {'id': 1, 'status': 'success'}] # DataSchema. + } + self.assertEqual(expected_body, body) + + # Validate that the created document failed validation for the expected + # reason. + resp = self.app.simulate_get( + '/api/v1.0/revisions/%s/validations/%s/entries/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': document['schema'], + 'name': document['metadata']['name'], + 'message': "'data' is a required property", + 'path': '.' + }] + self.assertIn('errors', body) + self.assertEqual(expected_errors, body['errors']) + + def test_validation_only_new_data_schema_registered(self): + """Validate whether newly created DataSchemas replace old DataSchemas + when it comes to validation. + """ + rules = {'deckhand:create_cleartext_documents': '@', + 'deckhand:list_validations': '@'} + self.policy.set_rules(rules) + + # Create 2 DataSchemas that will fail if they're used. These shouldn't + # be used for validation. + data_schema_factory = factories.DataSchemaFactory() + metadata_names = ['exampleA/Doc/v1', 'exampleB/Doc/v1'] + schemas_to_use = [{ + '$schema': 'http://json-schema.org/schema#', + 'type': 'object', + 'properties': { + 'a': { + 'type': 'integer' + } + }, + 'required': ['a'], + 'additionalProperties': False + }] * 2 + old_data_schemas = [ + data_schema_factory.gen_test( + metadata_names[i], data=schemas_to_use[i]) + for i in range(2) + ] + # Save the DataSchemas in the first revision. + revision_id = self._create_revision(payload=old_data_schemas) + + # Create 2 DataSchemas that will pass if they're used. These should + # be used for validation. + for schema_to_use in schemas_to_use: + schema_to_use['properties']['a']['type'] = 'string' + new_data_schemas = [ + data_schema_factory.gen_test( + metadata_names[i], data=schemas_to_use[i]) + for i in range(2) + ] + doc_factory = factories.DocumentFactory(1, [1]) + example1_doc = doc_factory.gen_test( + {'_GLOBAL_DATA_1_': {'data': {'a': 'whatever'}}}, + global_abstract=False)[-1] + example1_doc['schema'] = metadata_names[0] + example2_doc = copy.deepcopy(example1_doc) + example2_doc['schema'] = metadata_names[1] + # Save the documents that will be validated alongside the DataSchemas + # that will be used to validate them. + revision_id = self._create_revision( + payload=[example1_doc, example2_doc] + new_data_schemas) + + # Validate that the validation was created and succeeded: This means + # that the new DataSchemas were used, not the old ones. + 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/base.py b/deckhand/tests/unit/engine/base.py index 71e2a9fd..7a9decdd 100644 --- a/deckhand/tests/unit/engine/base.py +++ b/deckhand/tests/unit/engine/base.py @@ -28,9 +28,9 @@ class TestDocumentValidationBase(test_base.DeckhandTestCase): with open(test_yaml_path, 'r') as yaml_file: yaml_data = yaml_file.read() - self.data = yaml.safe_load(yaml_data) + return yaml.safe_load(yaml_data) - def _corrupt_data(self, key, value=None, data=None, op='delete'): + def _corrupt_data(self, data, key, value=None, op='delete'): """Corrupt test data to check that pre-validation works. Corrupt data by removing a key from the document (if ``op`` is delete) @@ -54,8 +54,6 @@ class TestDocumentValidationBase(test_base.DeckhandTestCase): :type op: string :returns: Corrupted data. """ - if data is None: - data = self.data if op not in ('delete', 'replace'): raise ValueError("The ``op`` argument must either be 'delete' or " "'replace'.") diff --git a/deckhand/tests/unit/engine/test_document_validation.py b/deckhand/tests/unit/engine/test_document_validation.py index c1387514..3cc1a6f7 100644 --- a/deckhand/tests/unit/engine/test_document_validation.py +++ b/deckhand/tests/unit/engine/test_document_validation.py @@ -17,32 +17,33 @@ import mock from deckhand.engine import document_validation from deckhand.tests.unit.engine import base as engine_test_base +from deckhand import factories +from deckhand import utils + class TestDocumentValidation(engine_test_base.TestDocumentValidationBase): def setUp(self): super(TestDocumentValidation, self).setUp() - # Mock out DB module (i.e. retrieving DataSchema docs from DB). - self.patch('deckhand.db.sqlalchemy.api.document_get_all') + self.test_document = self._read_data('sample_document') + dataschema_factory = factories.DataSchemaFactory() + self.dataschema = dataschema_factory.gen_test( + self.test_document['schema'], {}) - def test_init_document_validation(self): - self._read_data('sample_document') - doc_validation = document_validation.DocumentValidation( - self.data) - self.assertIsInstance(doc_validation, - document_validation.DocumentValidation) + # Stub out the DB call for retrieving DataSchema documents. + self.patchobject(document_validation.db_api, 'revision_documents_get', + lambda *a, **k: []) def test_data_schema_missing_optional_sections(self): - self._read_data('sample_data_schema') optional_missing_data = [ - self._corrupt_data('metadata.labels'), + self._corrupt_data(self.test_document, 'metadata.labels'), ] for missing_data in optional_missing_data: - document_validation.DocumentValidation(missing_data).validate_all() + payload = [missing_data, self.dataschema] + document_validation.DocumentValidation(payload).validate_all() def test_document_missing_optional_sections(self): - self._read_data('sample_document') properties_to_remove = ( 'metadata.layeringDefinition.actions', 'metadata.layeringDefinition.parentSelector', @@ -50,21 +51,19 @@ class TestDocumentValidation(engine_test_base.TestDocumentValidationBase): 'metadata.substitutions.2.dest.pattern') for property_to_remove in properties_to_remove: - optional_data_removed = self._corrupt_data(property_to_remove) - document_validation.DocumentValidation( - optional_data_removed).validate_all() + missing_data = self._corrupt_data(self.test_document, + property_to_remove) + payload = [missing_data, self.dataschema] + document_validation.DocumentValidation(payload).validate_all() @mock.patch.object(document_validation, 'LOG', autospec=True) def test_abstract_document_not_validated(self, mock_log): - self._read_data('sample_document') + test_document = self._read_data('sample_passphrase') # Set the document to abstract. - updated_data = self._corrupt_data( - 'metadata.layeringDefinition.abstract', True, op='replace') - # Guarantee that a validation error is thrown by removing a required - # property. - del updated_data['metadata']['layeringDefinition']['layer'] - - document_validation.DocumentValidation(updated_data).validate_all() + abstract_document = utils.jsonpath_replace( + test_document, True, 'metadata.layeringDefinition.abstract') + document_validation.DocumentValidation( + abstract_document).validate_all() self.assertTrue(mock_log.info.called) self.assertIn("Skipping schema validation for abstract document", mock_log.info.mock_calls[0][1][0]) diff --git a/deckhand/tests/unit/engine/test_document_validation_negative.py b/deckhand/tests/unit/engine/test_document_validation_negative.py index 9dcfab4d..e4ad7ac6 100644 --- a/deckhand/tests/unit/engine/test_document_validation_negative.py +++ b/deckhand/tests/unit/engine/test_document_validation_negative.py @@ -12,76 +12,93 @@ # See the License for the specific language governing permissions and # limitations under the License. +import mock + from deckhand.engine import document_validation from deckhand import errors -from deckhand.tests.unit.engine import base as engine_test_base +from deckhand import factories +from deckhand.tests.unit.engine import base as test_base from deckhand import types -class TestDocumentValidationNegative( - engine_test_base.TestDocumentValidationBase): +class TestDocumentValidationNegative(test_base.TestDocumentValidationBase): """Negative testing suite for document validation.""" # The 'data' key is mandatory but not critical if excluded. - CRITICAL_ATTRS = ( - 'schema', 'metadata', 'metadata.schema', 'metadata.name') - SCHEMA_ERR = "'%s' is a required property" + exception_map = { + 'metadata': errors.InvalidDocumentFormat, + 'metadata.schema': errors.InvalidDocumentFormat, + 'metadata.name': errors.InvalidDocumentFormat, + 'schema': errors.InvalidDocumentFormat, + } def setUp(self): super(TestDocumentValidationNegative, self).setUp() - # Mock out DB module (i.e. retrieving DataSchema docs from DB). - self.patch('deckhand.db.sqlalchemy.api.document_get_all') + # Stub out the DB call for retrieving DataSchema documents. + self.patchobject(document_validation.db_api, 'revision_documents_get', + lambda *a, **k: []) - def _test_missing_required_sections(self, properties_to_remove): + def _do_validations(self, document_validator, expected, expected_err): + validations = document_validator.validate_all() + self.assertEqual(2, len(validations)) + # The DataSchema document itself should've validated + # successfully. + self.assertEqual('success', validations[0]['status']) + self.assertEqual('failure', validations[-1]['status']) + self.assertEqual({'version': '1.0', 'name': 'deckhand'}, + validations[-1]['validator']) + self.assertEqual(types.DECKHAND_SCHEMA_VALIDATION, + validations[-1]['name']) + self.assertEqual(1, len(validations[-1]['errors'])) + self.assertEqual(expected['metadata']['name'], + validations[-1]['errors'][-1]['name']) + self.assertEqual(expected['schema'], + validations[-1]['errors'][-1]['schema']) + self.assertEqual(expected_err, + validations[-1]['errors'][-1]['message']) + + def _test_missing_required_sections(self, document, properties_to_remove): for idx, property_to_remove in enumerate(properties_to_remove): - critical = property_to_remove in self.CRITICAL_ATTRS - missing_prop = property_to_remove.split('.')[-1] - invalid_data = self._corrupt_data(property_to_remove) - expected_err = self.SCHEMA_ERR % missing_prop + invalid_data = self._corrupt_data(document, property_to_remove) - doc_validator = document_validation.DocumentValidation( - invalid_data) - if critical: - self.assertRaisesRegexp( - errors.InvalidDocumentFormat, expected_err, - doc_validator.validate_all) + exception_raised = self.exception_map.get(property_to_remove, None) + expected_err_msg = "'%s' is a required property" % missing_prop + + dataschema_factory = factories.DataSchemaFactory() + dataschema = dataschema_factory.gen_test( + invalid_data.get('schema', ''), {}) + payload = [dataschema, invalid_data] + + doc_validator = document_validation.DocumentValidation(payload) + if exception_raised: + self.assertRaises( + exception_raised, doc_validator.validate_all) else: - validations = doc_validator.validate_all() - self.assertEqual(1, len(validations)) - self.assertEqual('failure', validations[0]['status']) - self.assertEqual({'version': '1.0', 'name': 'deckhand'}, - validations[0]['validator']) - self.assertEqual(types.DECKHAND_SCHEMA_VALIDATION, - validations[0]['name']) - self.assertEqual(1, len(validations[0]['errors'])) - self.assertEqual(self.data['metadata']['name'], - validations[0]['errors'][0]['name']) - self.assertEqual(self.data['schema'], - validations[0]['errors'][0]['schema']) - self.assertEqual(expected_err, - validations[0]['errors'][0]['message']) + self._do_validations(doc_validator, invalid_data, + expected_err_msg) def test_certificate_key_missing_required_sections(self): - self._read_data('sample_certificate_key') - properties_to_remove = self.CRITICAL_ATTRS + ( + document = self._read_data('sample_certificate_key') + properties_to_remove = tuple(self.exception_map.keys()) + ( 'data', 'metadata.storagePolicy',) - self._test_missing_required_sections(properties_to_remove) + self._test_missing_required_sections(document, properties_to_remove) def test_certificate_missing_required_sections(self): - self._read_data('sample_certificate') - properties_to_remove = self.CRITICAL_ATTRS + ( + document = self._read_data('sample_certificate') + properties_to_remove = tuple(self.exception_map.keys()) + ( 'data', 'metadata.storagePolicy',) - self._test_missing_required_sections(properties_to_remove) + self._test_missing_required_sections(document, properties_to_remove) def test_data_schema_missing_required_sections(self): - self._read_data('sample_data_schema') - properties_to_remove = self.CRITICAL_ATTRS + ('data', 'data.$schema',) - self._test_missing_required_sections(properties_to_remove) + document = self._read_data('sample_data_schema') + properties_to_remove = tuple(self.exception_map.keys()) + ( + 'data', 'data.$schema',) + self._test_missing_required_sections(document, properties_to_remove) def test_document_missing_required_sections(self): - self._read_data('sample_document') - properties_to_remove = self.CRITICAL_ATTRS + ( + document = self._read_data('sample_document') + properties_to_remove = tuple(self.exception_map.keys()) + ( 'data', 'metadata.layeringDefinition', 'metadata.layeringDefinition.layer', @@ -93,45 +110,135 @@ class TestDocumentValidationNegative( 'metadata.substitutions.0.src.schema', 'metadata.substitutions.0.src.name', 'metadata.substitutions.0.src.path') - self._test_missing_required_sections(properties_to_remove) + self._test_missing_required_sections(document, properties_to_remove) + + def test_document_missing_multiple_required_sections(self): + """Validates that multiple errors are reported for a document with + multiple validation errors. + """ + document = self._read_data('sample_document') + properties_to_remove = ( + 'metadata.layeringDefinition.layer', + 'metadata.layeringDefinition.actions.0.method', + 'metadata.layeringDefinition.actions.0.path', + 'metadata.substitutions.0.dest.path', + 'metadata.substitutions.0.src.name', + 'metadata.substitutions.0.src.path', + 'metadata.substitutions.0.src.schema', + ) + for property_to_remove in properties_to_remove: + document = self._corrupt_data(document, property_to_remove) + + doc_validator = document_validation.DocumentValidation(document) + 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']) + + # Sort the errors to match the order in ``properties_to_remove``. + errors = sorted(errors[1:], 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``. + for idx, property_to_remove in enumerate(properties_to_remove): + parts = property_to_remove.split('.') + parent_path = '.' + '.'.join(parts[:-1]) + missing_property = parts[-1] + + expected_err = "'%s' is a required property" % missing_property + self.assertEqual(expected_err, errors[idx]['message']) + self.assertEqual(parent_path, errors[idx]['path']) def test_document_invalid_layering_definition_action(self): - self._read_data('sample_document') - corrupted_data = self._corrupt_data( - 'metadata.layeringDefinition.actions.0.method', 'invalid', - op='replace') + document = self._read_data('sample_document') + missing_data = self._corrupt_data( + document, 'metadata.layeringDefinition.actions.0.method', + 'invalid', op='replace') expected_err = "'invalid' is not one of ['replace', 'delete', 'merge']" - doc_validator = document_validation.DocumentValidation(corrupted_data) - validations = doc_validator.validate_all() - self.assertEqual(1, len(validations)) - self.assertEqual('failure', validations[0]['status']) - self.assertEqual({'version': '1.0', 'name': 'deckhand'}, - validations[0]['validator']) - self.assertEqual(types.DECKHAND_SCHEMA_VALIDATION, - validations[0]['name']) - self.assertEqual(1, len(validations[0]['errors'])) - self.assertEqual(self.data['metadata']['name'], - validations[0]['errors'][0]['name']) - self.assertEqual(self.data['schema'], - validations[0]['errors'][0]['schema']) - self.assertEqual(expected_err, - validations[0]['errors'][0]['message']) + # Ensure that a dataschema document exists for the random document + # schema via mocking. + dataschema_factory = factories.DataSchemaFactory() + dataschema = dataschema_factory.gen_test(document['schema'], {}) + payload = [dataschema, missing_data] + doc_validator = document_validation.DocumentValidation(payload) + self._do_validations(doc_validator, document, expected_err) def test_layering_policy_missing_required_sections(self): - self._read_data('sample_layering_policy') - properties_to_remove = self.CRITICAL_ATTRS + ( + document = self._read_data('sample_layering_policy') + properties_to_remove = tuple(self.exception_map.keys()) + ( 'data', 'data.layerOrder',) - self._test_missing_required_sections(properties_to_remove) + self._test_missing_required_sections(document, properties_to_remove) def test_passphrase_missing_required_sections(self): - self._read_data('sample_passphrase') - properties_to_remove = self.CRITICAL_ATTRS + ( + document = self._read_data('sample_passphrase') + properties_to_remove = tuple(self.exception_map.keys()) + ( 'data', 'metadata.storagePolicy',) - self._test_missing_required_sections(properties_to_remove) + self._test_missing_required_sections(document, properties_to_remove) def test_validation_policy_missing_required_sections(self): - self._read_data('sample_validation_policy') - properties_to_remove = self.CRITICAL_ATTRS + ( + document = self._read_data('sample_validation_policy') + properties_to_remove = tuple(self.exception_map.keys()) + ( 'data', 'data.validations', 'data.validations.0.name') - self._test_missing_required_sections(properties_to_remove) + self._test_missing_required_sections(document, properties_to_remove) + + @mock.patch.object(document_validation, 'LOG', autospec=True) + def test_invalid_document_schema_generates_error(self, mock_log): + document = self._read_data('sample_document') + document['schema'] = 'foo/bar/v1' + + 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.patch.object(document_validation, 'LOG', autospec=True) + def test_invalid_document_schema_version_generates_error(self, mock_log): + document = self._read_data('sample_passphrase') + document['schema'] = 'deckhand/Passphrase/v5' + + 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']) + + def test_invalid_validation_schema_raises_runtime_error(self): + document = self._read_data('sample_passphrase') + fake_schema = mock.MagicMock(schema='fake') + fake_schema_map = {'v1': {'deckhand/Passphrase': fake_schema}} + + # Validate that broken built-in base schema raises RuntimeError. + with mock.patch.object(document_validation, 'base_schema', + new_callable=mock.PropertyMock( + return_value=fake_schema)): + doc_validator = document_validation.DocumentValidation(document) + with self.assertRaisesRegexp(RuntimeError, 'Unknown error'): + doc_validator.validate_all() + + # Validate that broken built-in schema for ``SchemaValidator`` raises + # RuntimeError. + with mock.patch.object(document_validation.SchemaValidator, + '_schema_map', new_callable=mock.PropertyMock( + return_value=fake_schema_map)): + doc_validator = document_validation.DocumentValidation(document) + with self.assertRaisesRegexp(RuntimeError, 'Unknown error'): + doc_validator.validate_all() + + # Validate that broken data schema for ``DataSchemaValidator`` raises + # RuntimeError. + document = self._read_data('sample_document') + data_schema = self._read_data('sample_data_schema') + data_schema['metadata']['name'] = document['schema'] + data_schema['data'] = 'fake' + doc_validator = document_validation.DocumentValidation( + [document, data_schema]) + with self.assertRaisesRegexp(RuntimeError, 'Unknown error'): + doc_validator.validate_all() diff --git a/deckhand/tests/unit/resources/sample_passphrase.yaml b/deckhand/tests/unit/resources/sample_passphrase.yaml index 1d043773..21caeb6b 100644 --- a/deckhand/tests/unit/resources/sample_passphrase.yaml +++ b/deckhand/tests/unit/resources/sample_passphrase.yaml @@ -4,4 +4,6 @@ metadata: schema: metadata/Document/v1.0 name: application-admin-password storagePolicy: encrypted + layeringDefinition: + abstract: False data: some-password diff --git a/deckhand/utils.py b/deckhand/utils.py index b582963e..d2b75c02 100644 --- a/deckhand/utils.py +++ b/deckhand/utils.py @@ -181,7 +181,7 @@ def _add_microversion(value): """Hack for coercing all Deckhand schema fields (``schema`` and ``metadata.schema``) into ending with v1.0 rather than v1, for example. """ - microversion_re = r'^.*/.*/v[0-9]+$' + microversion_re = r'^.*/.*/v[1-9]\d*$' if re.match(value, microversion_re): return value + '.0' return value diff --git a/tools/functional-tests.sh b/tools/functional-tests.sh index 3d4533a6..3e5c8389 100755 --- a/tools/functional-tests.sh +++ b/tools/functional-tests.sh @@ -149,6 +149,12 @@ connection = $DATABASE_URL # auth_type = password EOCONF +# Only set up logging if running Deckhand via uwsgi. The container already has +# values for logging. +if [ -z "$DECKHAND_IMAGE" ]; then + sed '1 a log_config_append = '"$CONF_DIR"'/logging.conf' $CONF_DIR/deckhand.conf +fi + # Only set up logging if running Deckhand via uwsgi. The container already has # values for logging. if [ -z "$DECKHAND_IMAGE" ]; then