From 807990a099b3deca2e191ba5d82a1e0ac77de7f9 Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Tue, 26 Jun 2018 20:47:44 -0400 Subject: [PATCH] Fix gate following strange PyYAML 4.1 behavior This patchset adds a dict() cast as a workaround the fact that PyYAML 4.1 recently changed yaml.dump to yaml.safe_dump, compelling developers to use yaml.danger_dump to achieve the previous behavior of yaml.dump [0]. However, yaml.danger_dump should not be used and this technically corrects antecedent use of yaml.dump by introducing a recursive function that ensures the dictionary prior to being dumped is compatible with yaml.safe_dump. Such a function is needed because yaml.safe_dump rejects serialization of Deckhand's DocumentDict dictionary wrapper helper -- even though it is a subclass of a dict. Thus, the recursive function simply casts each instance of DocumentDict into a dictionary. [0] https://stackoverflow.com/questions/51053903/new-pyyaml-version-breaks-on-most-custom-python-objects-representererror Change-Id: I67966b45e0865864bd5e6bb4578548769fc13eeb --- deckhand/engine/document_validation.py | 14 +++++++---- deckhand/errors.py | 34 +++++++++++++++++++++++--- 2 files changed, 40 insertions(+), 8 deletions(-) diff --git a/deckhand/engine/document_validation.py b/deckhand/engine/document_validation.py index 70e5abac..68f8a978 100644 --- a/deckhand/engine/document_validation.py +++ b/deckhand/engine/document_validation.py @@ -240,14 +240,18 @@ class DataSchemaValidator(GenericValidator): parent_path_to_error_in_document = '.'.join( path_to_error_in_document.split('.')[:-1]) or '.' try: - # NOTE(fmontei): Because validation is performed on fully rendered - # documents, it is necessary to omit the parts of the data section - # where substitution may have occurred to avoid exposing any - # secrets. While this may make debugging a few validation failures - # more difficult, it is a necessary evil. + # NOTE(felipemonteiro): Because validation is performed on fully + # rendered documents, it is necessary to omit the parts of the data + # section where substitution may have occurred to avoid exposing + # any secrets. While this may make debugging a few validation + # failures more difficult, it is a necessary evil. sanitized_document = ( SecretsSubstitution.sanitize_potential_secrets( error, document)) + # This incurs some degree of overhead as caching here won't make + # a big difference as we are not parsing commonly referenced + # JSON paths -- but this branch is only hit during error handling + # so this should be OK. parent_error_section = utils.jsonpath_parse( sanitized_document, parent_path_to_error_in_document) except Exception: diff --git a/deckhand/errors.py b/deckhand/errors.py index 5918d3b5..c5303d57 100644 --- a/deckhand/errors.py +++ b/deckhand/errors.py @@ -12,11 +12,12 @@ # See the License for the specific language governing permissions and # limitations under the License. -import yaml +import collections import falcon from oslo_log import log as logging import six +import yaml LOG = logging.getLogger(__name__) @@ -29,6 +30,34 @@ def get_version_from_request(req): return 'N/A' +def _safe_yaml_dump(error_response): + """Cast every instance of ``DocumentDict`` into a dictionary for + compatibility with ``yaml.safe_dump``. + + This should only be called for error formatting. + """ + is_dict_sublcass = ( + lambda v: type(v) is not dict and issubclass(v.__class__, dict) + ) + + def _to_dict(value, parent): + if isinstance(value, (list, tuple, set)): + for v in value: + _to_dict(v, value) + elif isinstance(value, collections.Mapping): + for v in value.values(): + _to_dict(v, value) + else: + if isinstance(parent, (list, tuple, set)): + parent[parent.index(value)] = ( + dict(value) if is_dict_sublcass(value) else value) + elif isinstance(parent, dict): + for k, v in parent.items(): + parent[k] = dict(v) if is_dict_sublcass(v) else v + _to_dict(error_response, None) + return yaml.safe_dump(error_response) + + def format_error_resp(req, resp, status_code=falcon.HTTP_500, @@ -102,8 +131,7 @@ def format_error_resp(req, 'retry': True if status_code is falcon.HTTP_500 else False } - # Don't use yaml.safe_dump to handle unicode correctly. - resp.body = yaml.dump(error_response) + resp.body = _safe_yaml_dump(error_response) resp.status = status_code