diff --git a/deckhand/control/buckets.py b/deckhand/control/buckets.py index a2c33b05..910686da 100644 --- a/deckhand/control/buckets.py +++ b/deckhand/control/buckets.py @@ -51,12 +51,12 @@ class BucketsResource(api_base.BaseResource): # because we expect certain formatting of the documents while doing # policy enforcement. If any documents fail basic schema validaiton # raise an exception immediately. - doc_validator = document_validation.DocumentValidation(documents) try: + doc_validator = document_validation.DocumentValidation(documents) validations = doc_validator.validate_all() except (deckhand_errors.InvalidDocumentFormat, deckhand_errors.InvalidDocumentSchema) as e: - LOG.error(e.format_message()) + LOG.exception(e.format_message()) raise falcon.HTTPBadRequest(description=e.format_message()) for document in documents: diff --git a/deckhand/control/revision_documents.py b/deckhand/control/revision_documents.py index 6793bbb8..d9996d03 100644 --- a/deckhand/control/revision_documents.py +++ b/deckhand/control/revision_documents.py @@ -20,6 +20,7 @@ from deckhand.control import base as api_base from deckhand.control import common from deckhand.control.views import document as document_view from deckhand.db.sqlalchemy import api as db_api +from deckhand.engine import document_validation from deckhand.engine import secrets_manager from deckhand import errors from deckhand import policy @@ -116,5 +117,16 @@ class RenderedDocumentsResource(api_base.BaseResource): LOG.exception(six.text_type(e)) raise falcon.HTTPNotFound(description=e.format_message()) + # Perform schema validation post-rendering to ensure that rendering + # and substitution didn't break anything. + doc_validator = document_validation.DocumentValidation(documents) + try: + doc_validator.validate_all() + except (errors.InvalidDocumentFormat, + errors.InvalidDocumentSchema) as e: + LOG.exception(e.format_message()) + raise falcon.HTTPInternalServerError( + description=e.format_message()) + resp.status = falcon.HTTP_200 resp.body = self.view_builder.list(rendered_documents) diff --git a/deckhand/engine/document_validation.py b/deckhand/engine/document_validation.py index 5194ec2f..654d0e24 100644 --- a/deckhand/engine/document_validation.py +++ b/deckhand/engine/document_validation.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import copy import re import jsonschema @@ -38,9 +39,24 @@ class DocumentValidation(object): :param documents: Documents to be validated. :type documents: list[dict] """ + self.documents = [] + if not isinstance(documents, (list, tuple)): documents = [documents] - self.documents = [document_wrapper.Document(d) for d in 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') class SchemaType(object): """Class for retrieving correct schema for pre-validation on YAML. diff --git a/deckhand/tests/unit/control/test_rendered_documents_controller.py b/deckhand/tests/unit/control/test_rendered_documents_controller.py index 16dd4ae4..f358614a 100644 --- a/deckhand/tests/unit/control/test_rendered_documents_controller.py +++ b/deckhand/tests/unit/control/test_rendered_documents_controller.py @@ -17,6 +17,8 @@ import yaml import mock from deckhand.control import buckets +from deckhand.control import revision_documents +from deckhand import errors from deckhand import factories from deckhand.tests.unit.control import base as test_base @@ -63,6 +65,43 @@ class TestRenderedDocumentsController(test_base.BaseControllerTest): self.assertEqual(value, rendered_documents[0][key]) +class TestRenderedDocumentsControllerNegative( + test_base.BaseControllerTest): + + def test_rendered_documents_fail_schema_validation(self): + """Validates that when fully rendered documents fail schema validation, + the controller raises a 500 Internal Server Error. + """ + rules = {'deckhand:list_cleartext_documents': '@', + 'deckhand:list_encrypted_documents': '@', + 'deckhand:create_cleartext_documents': '@'} + self.policy.set_rules(rules) + + # Create a document for a bucket. + secrets_factory = factories.DocumentSecretFactory() + payload = [secrets_factory.gen_test('Certificate', 'cleartext')] + 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) + revision_id = list(yaml.safe_load_all(resp.text))[0]['status'][ + 'revision'] + + with mock.patch.object( + revision_documents, 'document_validation', + autospec=True) as m_doc_validation: + (m_doc_validation.DocumentValidation.return_value + .validate_all.side_effect) = errors.InvalidDocumentFormat + resp = self.app.simulate_get( + '/api/v1.0/revisions/%s/rendered-documents' % revision_id, + headers={'Content-Type': 'application/x-yaml'}) + + # Verify that a 500 Internal Server Error is thrown following failed + # schema validation. + self.assertEqual(500, resp.status_code) + + class TestRenderedDocumentsControllerNegativeRBAC( test_base.BaseControllerTest): """Test suite for validating negative RBAC scenarios for rendered documents diff --git a/doc/source/api_ref.rst b/doc/source/api_ref.rst index 0659a73e..966e75b7 100644 --- a/doc/source/api_ref.rst +++ b/doc/source/api_ref.rst @@ -94,6 +94,9 @@ Valid query parameters are the same as for ``/revisions/{revision_id}/documents``, minus the paremters in ``metadata.layeringDetinition``, which are not supported. +Raises a 500 Internal Server Error if rendered documents fail schema +validation. + GET ``/revisions`` ^^^^^^^^^^^^^^^^^^