Merge pull request #52427 from garethgreenaway/52350_readd_and_gate_u…

…nicode_string_literal_support

[2019.2] Support for old yaml render

doc/topics/releases/2019.2.1.rst

@@ -4,3 +4,31 @@ In Progress: Salt 2019.2.1 Release Notes
 
 Version 2019.2.1 is an **unreleased** bugfix release for :ref:`2019.2.0 <release-2019-2-0>`.
 This release is still in progress and has not been released yet.
+
+Change to YAML Renderer
+=======================
+
+.. code-block:: jinja
+
+    /etc/foo.conf:
+      file.managed:
+        - source: salt://foo.conf.jinja
+        - template: jinja
+        - context:
+            data: {{ data }}
+
+In 2019.2.0, the above SLS will result in an error message following changes to
+the YAML renderer that now require the new Jinja filter `tojson`.
+
+.. code-block:: jinja
+
+    /etc/foo.conf:
+      file.managed:
+        - source: salt://foo.conf.jinja
+        - template: jinja
+        - context:
+            data: {{ data|tojson }}
+
+In 2019.2.1, we introduce a new configuration option for both the Salt master and Salt minion
+configurations to be able to support the older YAML renderer.  Using the option
+`use_yamlloader_old` will allow the YAML renderer to function as before.

salt/renderers/yaml.py

@@ -16,7 +16,8 @@
 
 # Import salt libs
 import salt.utils.url
-from salt.utils.yamlloader import SaltYamlSafeLoader, load
+import salt.utils.yamlloader as yamlloader_new
+import salt.utils.yamlloader_old as yamlloader_old
 from salt.utils.odict import OrderedDict
 from salt.exceptions import SaltRenderError
 from salt.ext import six
@@ -35,7 +36,11 @@ def get_yaml_loader(argline):
     Return the ordered dict yaml loader
     '''
     def yaml_loader(*args):
-        return SaltYamlSafeLoader(*args, dictclass=OrderedDict)
+        if __opts__.get('use_yamlloader_old'):
+            yamlloader = yamlloader_old
+        else:
+            yamlloader = yamlloader_new
+        return yamlloader.SaltYamlSafeLoader(*args, dictclass=OrderedDict)
     return yaml_loader
 
 
@@ -46,11 +51,18 @@ def render(yaml_data, saltenv='base', sls='', argline='', **kws):
 
     :rtype: A Python data structure
     '''
+    if __opts__.get('use_yamlloader_old'):
+        log.warning('Using the old YAML Loader for rendering, '
+                    'consider disabling this and using the tojson'
+                    ' filter.')
+        yamlloader = yamlloader_old
+    else:
+        yamlloader = yamlloader_new
     if not isinstance(yaml_data, string_types):
         yaml_data = yaml_data.read()
     with warnings.catch_warnings(record=True) as warn_list:
         try:
-            data = load(yaml_data, Loader=get_yaml_loader(argline))
+            data = yamlloader.load(yaml_data, Loader=get_yaml_loader(argline))
         except ScannerError as exc:
             err_type = _ERROR_MAP.get(exc.problem, exc.problem)
             line_num = exc.problem_mark.line + 1

salt/utils/yamlloader_old.py

@@ -0,0 +1,224 @@
+# -*- coding: utf-8 -*-
+'''
+Custom YAML loading in Salt
+'''
+
+# Import python libs
+from __future__ import absolute_import, print_function, unicode_literals
+import re
+import warnings
+
+import yaml  # pylint: disable=blacklisted-import
+from yaml.nodes import MappingNode, SequenceNode
+from yaml.constructor import ConstructorError
+try:
+    yaml.Loader = yaml.CLoader
+    yaml.Dumper = yaml.CDumper
+except Exception:
+    pass
+
+import salt.utils.stringutils
+
+__all__ = ['SaltYamlSafeLoader', 'load', 'safe_load']
+
+
+class DuplicateKeyWarning(RuntimeWarning):
+    '''
+    Warned when duplicate keys exist
+    '''
+
+
+warnings.simplefilter('always', category=DuplicateKeyWarning)
+
+
+# with code integrated from https://gist.github.com/844388
+class SaltYamlSafeLoader(yaml.SafeLoader):
+    '''
+    Create a custom YAML loader that uses the custom constructor. This allows
+    for the YAML loading defaults to be manipulated based on needs within salt
+    to make things like sls file more intuitive.
+    '''
+    def __init__(self, stream, dictclass=dict):
+        super(SaltYamlSafeLoader, self).__init__(stream)
+        if dictclass is not dict:
+            # then assume ordered dict and use it for both !map and !omap
+            self.add_constructor(
+                'tag:yaml.org,2002:map',
+                type(self).construct_yaml_map)
+            self.add_constructor(
+                'tag:yaml.org,2002:omap',
+                type(self).construct_yaml_map)
+        self.add_constructor(
+            'tag:yaml.org,2002:str',
+            type(self).construct_yaml_str)
+        self.add_constructor(
+            'tag:yaml.org,2002:python/unicode',
+            type(self).construct_unicode)
+        self.add_constructor(
+            'tag:yaml.org,2002:timestamp',
+            type(self).construct_scalar)
+        self.dictclass = dictclass
+
+    def construct_yaml_map(self, node):
+        data = self.dictclass()
+        yield data
+        value = self.construct_mapping(node)
+        data.update(value)
+
+    def construct_unicode(self, node):
+        return node.value
+
+    def construct_mapping(self, node, deep=False):
+        '''
+        Build the mapping for YAML
+        '''
+        if not isinstance(node, MappingNode):
+            raise ConstructorError(
+                None,
+                None,
+                'expected a mapping node, but found {0}'.format(node.id),
+                node.start_mark)
+
+        self.flatten_mapping(node)
+
+        context = 'while constructing a mapping'
+        mapping = self.dictclass()
+        for key_node, value_node in node.value:
+            key = self.construct_object(key_node, deep=deep)
+            try:
+                hash(key)
+            except TypeError:
+                raise ConstructorError(
+                    context,
+                    node.start_mark,
+                    "found unacceptable key {0}".format(key_node.value),
+                    key_node.start_mark)
+            value = self.construct_object(value_node, deep=deep)
+            if key in mapping:
+                raise ConstructorError(
+                    context,
+                    node.start_mark,
+                    "found conflicting ID '{0}'".format(key),
+                    key_node.start_mark)
+            mapping[key] = value
+        return mapping
+
+    def construct_scalar(self, node):
+        '''
+        Verify integers and pass them in correctly is they are declared
+        as octal
+        '''
+        if node.tag == 'tag:yaml.org,2002:int':
+            if node.value == '0':
+                pass
+            elif node.value.startswith('0') and not node.value.startswith(('0b', '0x')):
+                node.value = node.value.lstrip('0')
+                # If value was all zeros, node.value would have been reduced to
+                # an empty string. Change it to '0'.
+                if node.value == '':
+                    node.value = '0'
+        elif node.tag == 'tag:yaml.org,2002:str':
+            # If any string comes in as a quoted unicode literal, eval it into
+            # the proper unicode string type.
+            if re.match(r'^u([\'"]).+\1$', node.value, flags=re.IGNORECASE):
+                node.value = eval(node.value, {}, {})  # pylint: disable=W0123
+        return super(SaltYamlSafeLoader, self).construct_scalar(node)
+
+    def construct_yaml_str(self, node):
+        value = self.construct_scalar(node)
+        return salt.utils.stringutils.to_unicode(value)
+
+    def fetch_plain(self):
+        '''
+        Handle unicode literal strings which appear inline in the YAML
+        '''
+        orig_line = self.line
+        orig_column = self.column
+        orig_pointer = self.pointer
+        try:
+            return super(SaltYamlSafeLoader, self).fetch_plain()
+        except yaml.scanner.ScannerError as exc:
+            problem_line = self.line
+            problem_column = self.column
+            problem_pointer = self.pointer
+            if exc.problem == "found unexpected ':'":
+                # Reset to prior position
+                self.line = orig_line
+                self.column = orig_column
+                self.pointer = orig_pointer
+                if self.peek(0) == 'u':
+                    # Might be a unicode literal string, check for 2nd char and
+                    # call the appropriate fetch func if it's a quote
+                    quote_char = self.peek(1)
+                    if quote_char in ("'", '"'):
+                        # Skip the "u" prefix by advancing the column and
+                        # pointer by 1
+                        self.column += 1
+                        self.pointer += 1
+                        if quote_char == '\'':
+                            return self.fetch_single()
+                        else:
+                            return self.fetch_double()
+                    else:
+                        # This wasn't a unicode literal string, so the caught
+                        # exception was correct. Restore the old position and
+                        # then raise the caught exception.
+                        self.line = problem_line
+                        self.column = problem_column
+                        self.pointer = problem_pointer
+            # Raise the caught exception
+            raise exc
+
+    def flatten_mapping(self, node):
+        merge = []
+        index = 0
+        while index < len(node.value):
+            key_node, value_node = node.value[index]
+
+            if key_node.tag == 'tag:yaml.org,2002:merge':
+                del node.value[index]
+                if isinstance(value_node, MappingNode):
+                    self.flatten_mapping(value_node)
+                    merge.extend(value_node.value)
+                elif isinstance(value_node, SequenceNode):
+                    submerge = []
+                    for subnode in value_node.value:
+                        if not isinstance(subnode, MappingNode):
+                            raise ConstructorError("while constructing a mapping",
+                                                   node.start_mark,
+                                                   "expected a mapping for merging, but found {0}".format(subnode.id),
+                                                   subnode.start_mark)
+                        self.flatten_mapping(subnode)
+                        submerge.append(subnode.value)
+                    submerge.reverse()
+                    for value in submerge:
+                        merge.extend(value)
+                else:
+                    raise ConstructorError("while constructing a mapping",
+                                           node.start_mark,
+                                           "expected a mapping or list of mappings for merging, but found {0}".format(value_node.id),
+                                           value_node.start_mark)
+            elif key_node.tag == 'tag:yaml.org,2002:value':
+                key_node.tag = 'tag:yaml.org,2002:str'
+                index += 1
+            else:
+                index += 1
+        if merge:
+            # Here we need to discard any duplicate entries based on key_node
+            existing_nodes = [name_node.value for name_node, value_node in node.value]
+            mergeable_items = [x for x in merge if x[0].value not in existing_nodes]
+
+            node.value = mergeable_items + node.value
+
+
+def load(stream, Loader=SaltYamlSafeLoader):
+    return yaml.load(stream, Loader=Loader)
+
+
+def safe_load(stream, Loader=SaltYamlSafeLoader):
+    '''
+    .. versionadded:: 2018.3.0
+
+    Helper function which automagically uses our custom loader.
+    '''
+    return yaml.load(stream, Loader=Loader)

tests/unit/renderers/test_yaml.py

@@ -3,19 +3,47 @@
 # Import Python Libs
 from __future__ import absolute_import, print_function, unicode_literals
 
+import collections
+import textwrap
+
 # Import Salt Testing libs
 from tests.support.mixins import LoaderModuleMockMixin
 from tests.support.unit import TestCase
+from tests.support.mock import (
+    patch
+)
 
 # Import Salt libs
 import salt.renderers.yaml as yaml
+from salt.ext import six
 
 
 class YAMLRendererTestCase(TestCase, LoaderModuleMockMixin):
 
     def setup_loader_modules(self):
         return {yaml: {}}
 
+    def assert_unicode(self, value):
+        '''
+        Make sure the entire data structure is unicode
+        '''
+        if six.PY3:
+            return
+        if isinstance(value, six.string_types):
+            if not isinstance(value, six.text_type):
+                self.raise_error(value)
+        elif isinstance(value, collections.Mapping):
+            for k, v in six.iteritems(value):
+                self.assert_unicode(k)
+                self.assert_unicode(v)
+        elif isinstance(value, collections.Iterable):
+            for item in value:
+                self.assert_unicode(item)
+
+    def assert_matches(self, ret, expected):
+        self.assertEqual(ret, expected)
+        self.assert_unicode(ret)
+
     def test_yaml_render_string(self):
         data = 'string'
         result = yaml.render(data)
@@ -27,3 +55,14 @@ def test_yaml_render_unicode(self):
         result = yaml.render(data)
 
         self.assertEqual(result, u'python unicode string')
+
+    def test_yaml_render_old_unicode(self):
+        config = {'use_yamlloader_old': True}
+        with patch.dict(yaml.__opts__, config):  # pylint: disable=no-member
+            self.assert_matches(
+                yaml.render(textwrap.dedent('''\
+                    foo:
+                      a: Д
+                      b: {'a': u'\\u0414'}''')),
+                {'foo': {'a': u'\u0414', 'b': {'a': u'\u0414'}}}
+            )