Abstract away libyaml details into yaml utils (#72607)

* Add yaml utils file and use it

* Linting fix and missing import

* Abstract a few more details

* Parser imports

* Don't use CSafeDumper in AnsibleDumper

* Move and use convert_yaml_objects_to_native when libyaml is present

* yaml_load and yaml_dump, instead of safe_*

* re-use HAS_LIBYAML from utils.yaml

* add changelog fragment

* Address recent changes

* Use representer instead of recursive type converter

* Restore needed import

* move yaml utils to module_utils

* Properly guard imports

* Update from_yaml(_all)? to handle text wrappers with CSafeLoader

* Use yaml utils for legacy_collection_loader

* Add HAS_YAML, and ignore pylint issue

* oops

* GPL->BSD

changelogs/fragments/72607.yml

@@ -0,0 +1,2 @@
+minor_changes:
+- YAML parsing - Create common utils for loading and dumping YAML that prefer the C extensions if available

lib/ansible/cli/arguments/option_helpers.py

@@ -11,19 +11,13 @@ import os
 import os.path
 import sys
 import time
-import yaml
-
-try:
-    import _yaml
-    HAS_LIBYAML = True
-except ImportError:
-    HAS_LIBYAML = False
 
 from jinja2 import __version__ as j2_version
 
 import ansible
 from ansible import constants as C
 from ansible.module_utils._text import to_native
+from ansible.module_utils.common.yaml import HAS_LIBYAML, yaml_load
 from ansible.release import __version__
 from ansible.utils.path import unfrackpath
 
@@ -113,7 +107,8 @@ def _git_repo_info(repo_path):
         # Check if the .git is a file. If it is a file, it means that we are in a submodule structure.
         if os.path.isfile(repo_path):
             try:
-                gitdir = yaml.safe_load(open(repo_path)).get('gitdir')
+                with open(repo_path) as f:
+                    gitdir = yaml_load(f).get('gitdir')
                 # There is a possibility the .git file to have an absolute path.
                 if os.path.isabs(gitdir):
                     repo_path = gitdir

lib/ansible/cli/doc.py

@@ -26,6 +26,7 @@ from ansible.errors import AnsibleError, AnsibleOptionsError, AnsibleParserError
 from ansible.module_utils._text import to_native, to_text
 from ansible.module_utils.common._collections_compat import Container, Sequence
 from ansible.module_utils.common.json import AnsibleJSONEncoder
+from ansible.module_utils.common.yaml import yaml_dump
 from ansible.module_utils.compat import importlib
 from ansible.module_utils.six import iteritems, string_types
 from ansible.parsing.plugin_docs import read_docstub
@@ -1188,7 +1189,7 @@ class DocCLI(CLI, RoleMixin):
             if isinstance(doc['plainexamples'], string_types):
                 text.append(doc.pop('plainexamples').strip())
             else:
-                text.append(yaml.dump(doc.pop('plainexamples'), indent=2, default_flow_style=False))
+                text.append(yaml_dump(doc.pop('plainexamples'), indent=2, default_flow_style=False))
             text.append('')
             text.append('')
 

lib/ansible/cli/galaxy.py

@@ -12,7 +12,6 @@ import shutil
 import sys
 import textwrap
 import time
-import yaml
 
 from yaml.error import YAMLError
 
@@ -42,6 +41,7 @@ from ansible.galaxy.role import GalaxyRole
 from ansible.galaxy.token import BasicAuthToken, GalaxyToken, KeycloakToken, NoTokenSentinel
 from ansible.module_utils.ansible_release import __version__ as ansible_version
 from ansible.module_utils.common.collections import is_iterable
+from ansible.module_utils.common.yaml import yaml_dump, yaml_load
 from ansible.module_utils._text import to_bytes, to_native, to_text
 from ansible.module_utils import six
 from ansible.parsing.dataloader import DataLoader
@@ -483,7 +483,7 @@ class GalaxyCLI(CLI):
             # Config definitions are looked up dynamically based on the C.GALAXY_SERVER_LIST entry. We look up the
             # section [galaxy_server.<server>] for the values url, username, password, and token.
             config_dict = dict((k, server_config_def(server_key, k, req)) for k, req in server_def)
-            defs = AnsibleLoader(yaml.safe_dump(config_dict)).get_single_data()
+            defs = AnsibleLoader(yaml_dump(config_dict)).get_single_data()
             C.config.initialize_plugin_configuration_definitions('galaxy_server', server_key, defs)
 
             server_options = C.config.get_plugin_options('galaxy_server', server_key)
@@ -613,7 +613,7 @@ class GalaxyCLI(CLI):
         display.vvv("Reading requirement file at '%s'" % requirements_file)
         with open(b_requirements_file, 'rb') as req_obj:
             try:
-                file_requirements = yaml.safe_load(req_obj)
+                file_requirements = yaml_load(req_obj)
             except YAMLError as err:
                 raise AnsibleError(
                     "Failed to parse the requirements yml at '%s' with the following error:\n%s"
@@ -638,7 +638,7 @@ class GalaxyCLI(CLI):
                 with open(b_include_path, 'rb') as f_include:
                     try:
                         return [GalaxyRole(self.galaxy, self.api, **r) for r in
-                                (RoleRequirement.role_yaml_parse(i) for i in yaml.safe_load(f_include))]
+                                (RoleRequirement.role_yaml_parse(i) for i in yaml_load(f_include))]
                     except Exception as e:
                         raise AnsibleError("Unable to load data from include requirements file: %s %s"
                                            % (to_native(requirements_file), to_native(e)))
@@ -1503,7 +1503,7 @@ class GalaxyCLI(CLI):
         if output_format == 'json':
             display.display(json.dumps(collections_in_paths))
         elif output_format == 'yaml':
-            display.display(yaml.safe_dump(collections_in_paths))
+            display.display(yaml_dump(collections_in_paths))
 
         return 0
 

lib/ansible/config/manager.py

@@ -14,17 +14,11 @@ import tempfile
 import traceback
 from collections import namedtuple
 
-from yaml import load as yaml_load
-try:
-    # use C version if possible for speedup
-    from yaml import CSafeLoader as SafeLoader
-except ImportError:
-    from yaml import SafeLoader
-
 from ansible.config.data import ConfigData
 from ansible.errors import AnsibleOptionsError, AnsibleError
 from ansible.module_utils._text import to_text, to_bytes, to_native
 from ansible.module_utils.common._collections_compat import Mapping, Sequence
+from ansible.module_utils.common.yaml import yaml_load
 from ansible.module_utils.six import PY3, string_types
 from ansible.module_utils.six.moves import configparser
 from ansible.module_utils.parsing.convert_bool import boolean
@@ -315,7 +309,7 @@ class ConfigManager(object):
         yml_file = to_bytes(yml_file)
         if os.path.exists(yml_file):
             with open(yml_file, 'rb') as config_def:
-                return yaml_load(config_def, Loader=SafeLoader) or {}
+                return yaml_load(config_def) or {}
         raise AnsibleError(
             "Missing base YAML definition file (bad install?): %s" % to_native(yml_file))
 
@@ -349,7 +343,7 @@ class ConfigManager(object):
             # FIXME: this should eventually handle yaml config files
             # elif ftype == 'yaml':
             #     with open(cfile, 'rb') as config_stream:
-            #         self._parsers[cfile] = yaml.safe_load(config_stream)
+            #         self._parsers[cfile] = yaml_load(config_stream)
             else:
                 raise AnsibleOptionsError("Unsupported configuration file type: %s" % to_native(ftype))
 

lib/ansible/galaxy/__init__.py

@@ -24,11 +24,11 @@ from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
 import os
-import yaml
 
 import ansible.constants as C
 from ansible import context
 from ansible.module_utils._text import to_bytes
+from ansible.module_utils.common.yaml import yaml_load
 
 #      default_readme_template
 #      default_meta_template
@@ -37,7 +37,7 @@ from ansible.module_utils._text import to_bytes
 def get_collections_galaxy_meta_info():
     meta_path = os.path.join(os.path.dirname(__file__), 'data', 'collections_galaxy_meta.yml')
     with open(to_bytes(meta_path, errors='surrogate_or_strict'), 'rb') as galaxy_obj:
-        return yaml.safe_load(galaxy_obj)
+        return yaml_load(galaxy_obj)
 
 
 class Galaxy(object):

lib/ansible/galaxy/collection/__init__.py

@@ -117,6 +117,7 @@ from ansible.galaxy.dependency_resolution.errors import (
 from ansible.galaxy.dependency_resolution.versioning import meets_requirements
 from ansible.module_utils.six import raise_from
 from ansible.module_utils._text import to_bytes, to_native, to_text
+from ansible.module_utils.common.yaml import yaml_dump
 from ansible.utils.collection_loader import AnsibleCollectionRef
 from ansible.utils.display import Display
 from ansible.utils.hashing import secure_hash, secure_hash_s
@@ -386,7 +387,7 @@ def download_collections(
             "to '{path!s}'".format(path=to_text(requirements_path)),
         )
         yaml_bytes = to_bytes(
-            yaml.safe_dump({'collections': requirements}),
+            yaml_dump({'collections': requirements}),
             errors='surrogate_or_strict',
         )
         with open(b_requirements_path, mode='wb') as req_fd:

lib/ansible/galaxy/collection/concrete_artifact_manager.py

@@ -38,6 +38,7 @@ from ansible.galaxy import get_collections_galaxy_meta_info
 from ansible.galaxy.dependency_resolution.dataclasses import _GALAXY_YAML
 from ansible.galaxy.user_agent import user_agent
 from ansible.module_utils._text import to_bytes, to_native, to_text
+from ansible.module_utils.common.yaml import yaml_load
 from ansible.module_utils.six.moves.urllib.error import URLError
 from ansible.module_utils.six.moves.urllib.parse import urldefrag
 from ansible.module_utils.six import raise_from
@@ -524,7 +525,7 @@ def _get_meta_from_src_dir(
 
     with open(galaxy_yml, 'rb') as manifest_file_obj:
         try:
-            manifest = yaml.safe_load(manifest_file_obj)
+            manifest = yaml_load(manifest_file_obj)
         except yaml.error.YAMLError as yaml_err:
             raise_from(
                 AnsibleError(

lib/ansible/galaxy/role.py

@@ -27,7 +27,6 @@ import datetime
 import os
 import tarfile
 import tempfile
-import yaml
 from distutils.version import LooseVersion
 from shutil import rmtree
 
@@ -35,6 +34,7 @@ from ansible import context
 from ansible.errors import AnsibleError
 from ansible.galaxy.user_agent import user_agent
 from ansible.module_utils._text import to_native, to_text
+from ansible.module_utils.common.yaml import yaml_dump, yaml_load
 from ansible.module_utils.urls import open_url
 from ansible.playbook.role.requirement import RoleRequirement
 from ansible.utils.display import Display
@@ -111,7 +111,7 @@ class GalaxyRole(object):
                     if os.path.isfile(meta_path):
                         try:
                             with open(meta_path, 'r') as f:
-                                self._metadata = yaml.safe_load(f)
+                                self._metadata = yaml_load(f)
                         except Exception:
                             display.vvvvv("Unable to load metadata for %s" % self.name)
                             return False
@@ -130,7 +130,7 @@ class GalaxyRole(object):
             if os.path.isfile(info_path):
                 try:
                     f = open(info_path, 'r')
-                    self._install_info = yaml.safe_load(f)
+                    self._install_info = yaml_load(f)
                 except Exception:
                     display.vvvvv("Unable to load Galaxy install info for %s" % self.name)
                     return False
@@ -162,7 +162,7 @@ class GalaxyRole(object):
         info_path = os.path.join(self.path, self.META_INSTALL)
         with open(info_path, 'w+') as f:
             try:
-                self._install_info = yaml.safe_dump(info, f)
+                self._install_info = yaml_dump(info, f)
             except Exception:
                 return False
 
@@ -299,7 +299,7 @@ class GalaxyRole(object):
                     raise AnsibleError("this role does not appear to have a meta/main.yml file.")
                 else:
                     try:
-                        self._metadata = yaml.safe_load(role_tar_file.extractfile(meta_file))
+                        self._metadata = yaml_load(role_tar_file.extractfile(meta_file))
                     except Exception:
                         raise AnsibleError("this role does not appear to have a valid meta/main.yml file.")
 
@@ -392,7 +392,7 @@ class GalaxyRole(object):
                 if os.path.isfile(meta_path):
                     try:
                         f = open(meta_path, 'r')
-                        self._requirements = yaml.safe_load(f)
+                        self._requirements = yaml_load(f)
                     except Exception:
                         display.vvvvv("Unable to load requirements for %s" % self.name)
                     finally:

lib/ansible/galaxy/token.py

@@ -26,11 +26,10 @@ import os
 import json
 from stat import S_IRUSR, S_IWUSR
 
-import yaml
-
 from ansible import constants as C
 from ansible.galaxy.user_agent import user_agent
 from ansible.module_utils._text import to_bytes, to_native, to_text
+from ansible.module_utils.common.yaml import yaml_dump, yaml_load
 from ansible.module_utils.urls import open_url
 from ansible.utils.display import Display
 
@@ -126,7 +125,7 @@ class GalaxyToken(object):
             action = 'Created'
 
         with open(self.b_file, 'r') as f:
-            config = yaml.safe_load(f)
+            config = yaml_load(f)
 
         display.vvv('%s %s' % (action, to_text(self.b_file)))
 
@@ -145,7 +144,7 @@ class GalaxyToken(object):
 
     def save(self):
         with open(self.b_file, 'w') as f:
-            yaml.safe_dump(self.config, f, default_flow_style=False)
+            yaml_dump(self.config, f, default_flow_style=False)
 
     def headers(self):
         headers = {}

lib/ansible/module_utils/common/yaml.py

@@ -0,0 +1,42 @@
+# (c) 2020 Matt Martz <matt@sivel.net>
+# Simplified BSD License (see licenses/simplified_bsd.txt or https://opensource.org/licenses/BSD-2-Clause)
+
+"""
+This file provides ease of use shortcuts for loading and dumping YAML,
+preferring the YAML compiled C extensions to reduce duplicated code.
+"""
+
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+from functools import partial as _partial
+
+HAS_LIBYAML = False
+try:
+    import yaml as _yaml
+except ImportError:
+    HAS_YAML = False
+    SafeLoader = None
+    SafeDumper = None
+    Parser = None
+    yaml_load = None
+    yaml_load_all = None
+    yaml_dump = None
+    yaml_dump_all = None
+else:
+    HAS_YAML = True
+    try:
+        SafeLoader = _yaml.CSafeLoader
+        SafeDumper = _yaml.CSafeDumper
+        Parser = _yaml.cyaml.CParser
+        HAS_LIBYAML = True
+    except AttributeError:
+        SafeLoader = _yaml.SafeLoader
+        SafeDumper = _yaml.SafeDumper
+        Parser = _yaml.parser.Parser
+
+    yaml_load = _partial(_yaml.load, Loader=SafeLoader)
+    yaml_load_all = _partial(_yaml.load_all, Loader=SafeLoader)
+
+    yaml_dump = _partial(_yaml.dump, Dumper=SafeDumper)
+    yaml_dump_all = _partial(_yaml.dump_all, Dumper=SafeDumper)

lib/ansible/parsing/yaml/dumper.py

@@ -21,18 +21,18 @@ __metaclass__ = type
 
 import yaml
 
-from ansible.module_utils.six import PY3
+from ansible.module_utils.six import PY3, text_type, binary_type
+from ansible.module_utils.common.yaml import SafeDumper
 from ansible.parsing.yaml.objects import AnsibleUnicode, AnsibleSequence, AnsibleMapping, AnsibleVaultEncryptedUnicode
 from ansible.utils.unsafe_proxy import AnsibleUnsafeText, AnsibleUnsafeBytes
 from ansible.vars.hostvars import HostVars, HostVarsVars
 
 
-class AnsibleDumper(yaml.SafeDumper):
+class AnsibleDumper(SafeDumper):
     '''
     A simple stub class that allows us to add representers
     for our overridden object types.
     '''
-    pass
 
 
 def represent_hostvars(self, data):
@@ -45,11 +45,18 @@ def represent_vault_encrypted_unicode(self, data):
 
 
 if PY3:
-    represent_unicode = yaml.representer.SafeRepresenter.represent_str
+    def represent_unicode(self, data):
-    represent_binary = yaml.representer.SafeRepresenter.represent_binary
+        return yaml.representer.SafeRepresenter.represent_str(self, text_type(data))
+
+    def represent_binary(self, data):
+        return yaml.representer.SafeRepresenter.represent_binary(self, binary_type(data))
 else:
-    represent_unicode = yaml.representer.SafeRepresenter.represent_unicode
+    def represent_unicode(self, data):
-    represent_binary = yaml.representer.SafeRepresenter.represent_str
+        return yaml.representer.SafeRepresenter.represent_unicode(self, text_type(data))
+
+    def represent_binary(self, data):
+        return yaml.representer.SafeRepresenter.represent_str(self, binary_type(data))
+
 
 AnsibleDumper.add_representer(
     AnsibleUnicode,

lib/ansible/parsing/yaml/loader.py

@@ -19,21 +19,16 @@
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
-try:
-    from _yaml import CParser, CEmitter
-    HAVE_PYYAML_C = True
-except ImportError:
-    HAVE_PYYAML_C = False
-
 from yaml.resolver import Resolver
 
 from ansible.parsing.yaml.constructor import AnsibleConstructor
+from ansible.module_utils.common.yaml import HAS_LIBYAML, Parser
 
-if HAVE_PYYAML_C:
+if HAS_LIBYAML:
 
-    class AnsibleLoader(CParser, AnsibleConstructor, Resolver):
+    class AnsibleLoader(Parser, AnsibleConstructor, Resolver):
         def __init__(self, stream, file_name=None, vault_secrets=None):
-            CParser.__init__(self, stream)
+            Parser.__init__(self, stream)  # pylint: disable=non-parent-init-called
             AnsibleConstructor.__init__(self, file_name=file_name, vault_secrets=vault_secrets)
             Resolver.__init__(self)
 else:
@@ -46,7 +41,7 @@ else:
         def __init__(self, stream, file_name=None, vault_secrets=None):
             Reader.__init__(self, stream)
             Scanner.__init__(self)
-            Parser.__init__(self)
+            Parser.__init__(self)  # pylint: disable=non-parent-init-called
             Composer.__init__(self)
             AnsibleConstructor.__init__(self, file_name=file_name, vault_secrets=vault_secrets)
             Resolver.__init__(self)

lib/ansible/plugins/filter/core.py

@@ -43,6 +43,7 @@ from ansible.module_utils.six.moves import shlex_quote
 from ansible.module_utils._text import to_bytes, to_native, to_text
 from ansible.module_utils.common.collections import is_sequence
 from ansible.module_utils.common._collections_compat import Mapping
+from ansible.module_utils.common.yaml import yaml_load, yaml_load_all
 from ansible.parsing.ajson import AnsibleJSONEncoder
 from ansible.parsing.yaml.dumper import AnsibleDumper
 from ansible.template import recursive_check_defined
@@ -208,13 +209,19 @@ def regex_escape(string, re_type='python'):
 
 def from_yaml(data):
     if isinstance(data, string_types):
-        return yaml.safe_load(data)
+        # The ``text_type`` call here strips any custom
+        # string wrapper class, so that CSafeLoader can
+        # read the data
+        return yaml_load(text_type(to_text(data, errors='surrogate_or_strict')))
     return data
 
 
 def from_yaml_all(data):
     if isinstance(data, string_types):
-        return yaml.safe_load_all(data)
+        # The ``text_type`` call here strips any custom
+        # string wrapper class, so that CSafeLoader can
+        # read the data
+        return yaml_load_all(text_type(to_text(data, errors='surrogate_or_strict')))
     return data
 
 

lib/ansible/utils/collection_loader/_collection_meta.py

@@ -4,17 +4,13 @@
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
-from yaml import load
-try:
-    from yaml import CSafeLoader as SafeLoader
-except ImportError:
-    from yaml import SafeLoader
-
 try:
     from collections.abc import Mapping   # pylint: disable=ansible-bad-import-from
 except ImportError:
     from collections import Mapping  # pylint: disable=ansible-bad-import-from
 
+from ansible.module_utils.common.yaml import yaml_load
+
 
 def _meta_yml_to_dict(yaml_string_data, content_id):
     """
@@ -25,7 +21,7 @@ def _meta_yml_to_dict(yaml_string_data, content_id):
     :return: a Python dictionary representing the YAML dictionary content
     """
     # NB: content_id is passed in, but not used by this implementation
-    routing_dict = load(yaml_string_data, Loader=SafeLoader)
+    routing_dict = yaml_load(yaml_string_data)
     if not routing_dict:
         routing_dict = {}
     if not isinstance(routing_dict, Mapping):

test/integration/targets/filter_core/tasks/main.yml

@@ -378,6 +378,18 @@
       - "2|from_yaml == 2"
       - "'---\nbananas: yellow\n---\napples: red'|from_yaml_all|list == [{'bananas': 'yellow'}, {'apples': 'red'}]"
       - "2|from_yaml_all == 2"
+      - "unsafe_fruit|from_yaml == {'bananas': 'yellow', 'apples': 'red'}"
+      - "unsafe_fruit_all|from_yaml_all|list == [{'bananas': 'yellow'}, {'apples': 'red'}]"
+  vars:
+    unsafe_fruit: !unsafe |
+      ---
+      bananas: yellow
+      apples: red
+    unsafe_fruit_all: !unsafe |
+      ---
+      bananas: yellow
+      ---
+      apples: red
 
 - name: Verify random raises on non-iterable input (failure expected)
   set_fact:

test/lib/ansible_test/_data/legacy_collection_loader/_collection_meta.py

@@ -4,17 +4,13 @@
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
-from yaml import load
-try:
-    from yaml import CSafeLoader as SafeLoader
-except ImportError:
-    from yaml import SafeLoader
-
 try:
     from collections.abc import Mapping   # pylint: disable=ansible-bad-import-from
 except ImportError:
     from collections import Mapping  # pylint: disable=ansible-bad-import-from
 
+from ansible.module_utils.common.yaml import yaml_load
+
 
 def _meta_yml_to_dict(yaml_string_data, content_id):
     """
@@ -25,7 +21,7 @@ def _meta_yml_to_dict(yaml_string_data, content_id):
     :return: a Python dictionary representing the YAML dictionary content
     """
     # NB: content_id is passed in, but not used by this implementation
-    routing_dict = load(yaml_string_data, Loader=SafeLoader)
+    routing_dict = yaml_load(yaml_string_data)
     if not routing_dict:
         routing_dict = {}
     if not isinstance(routing_dict, Mapping):