diff --git a/changelogs/fragments/72607.yml b/changelogs/fragments/72607.yml new file mode 100644 index 00000000000..10b25192d4c --- /dev/null +++ b/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 diff --git a/lib/ansible/cli/arguments/option_helpers.py b/lib/ansible/cli/arguments/option_helpers.py index 0e7445423bf..d440521bac6 100644 --- a/lib/ansible/cli/arguments/option_helpers.py +++ b/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 diff --git a/lib/ansible/cli/doc.py b/lib/ansible/cli/doc.py index bd2896411f5..9959aac6d97 100644 --- a/lib/ansible/cli/doc.py +++ b/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('') diff --git a/lib/ansible/cli/galaxy.py b/lib/ansible/cli/galaxy.py index 2293149708c..3a24c0dbfa9 100644 --- a/lib/ansible/cli/galaxy.py +++ b/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.] 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 diff --git a/lib/ansible/config/manager.py b/lib/ansible/config/manager.py index 51f6b40be94..d1503218de6 100644 --- a/lib/ansible/config/manager.py +++ b/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)) diff --git a/lib/ansible/galaxy/__init__.py b/lib/ansible/galaxy/__init__.py index a73baac8881..d3b9035f9d2 100644 --- a/lib/ansible/galaxy/__init__.py +++ b/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): diff --git a/lib/ansible/galaxy/collection/__init__.py b/lib/ansible/galaxy/collection/__init__.py index ed99da34b78..b6b7c9b340d 100644 --- a/lib/ansible/galaxy/collection/__init__.py +++ b/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: diff --git a/lib/ansible/galaxy/collection/concrete_artifact_manager.py b/lib/ansible/galaxy/collection/concrete_artifact_manager.py index b2550abdc03..bdf20218b5d 100644 --- a/lib/ansible/galaxy/collection/concrete_artifact_manager.py +++ b/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( diff --git a/lib/ansible/galaxy/role.py b/lib/ansible/galaxy/role.py index 391df72545c..2c072a6fd59 100644 --- a/lib/ansible/galaxy/role.py +++ b/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: diff --git a/lib/ansible/galaxy/token.py b/lib/ansible/galaxy/token.py index 52bff211aeb..11a12ce45ad 100644 --- a/lib/ansible/galaxy/token.py +++ b/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 = {} diff --git a/lib/ansible/module_utils/common/yaml.py b/lib/ansible/module_utils/common/yaml.py new file mode 100644 index 00000000000..4d64dbb6c45 --- /dev/null +++ b/lib/ansible/module_utils/common/yaml.py @@ -0,0 +1,42 @@ +# (c) 2020 Matt Martz +# 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) diff --git a/lib/ansible/parsing/yaml/dumper.py b/lib/ansible/parsing/yaml/dumper.py index 67a2efb36dd..e0bf8c32c3f 100644 --- a/lib/ansible/parsing/yaml/dumper.py +++ b/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 - represent_binary = yaml.representer.SafeRepresenter.represent_binary + def represent_unicode(self, data): + 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 - represent_binary = yaml.representer.SafeRepresenter.represent_str + def represent_unicode(self, data): + 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, diff --git a/lib/ansible/parsing/yaml/loader.py b/lib/ansible/parsing/yaml/loader.py index b6650041c41..8a050fe6c7e 100644 --- a/lib/ansible/parsing/yaml/loader.py +++ b/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) diff --git a/lib/ansible/plugins/filter/core.py b/lib/ansible/plugins/filter/core.py index 06570c27cbb..a30b52cb18f 100644 --- a/lib/ansible/plugins/filter/core.py +++ b/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 diff --git a/lib/ansible/utils/collection_loader/_collection_meta.py b/lib/ansible/utils/collection_loader/_collection_meta.py index b306b810a2b..b14917c30e2 100644 --- a/lib/ansible/utils/collection_loader/_collection_meta.py +++ b/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): diff --git a/test/integration/targets/filter_core/tasks/main.yml b/test/integration/targets/filter_core/tasks/main.yml index 8ab9d44676b..567a27f10ed 100644 --- a/test/integration/targets/filter_core/tasks/main.yml +++ b/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: diff --git a/test/lib/ansible_test/_data/legacy_collection_loader/_collection_meta.py b/test/lib/ansible_test/_data/legacy_collection_loader/_collection_meta.py index b306b810a2b..b14917c30e2 100644 --- a/test/lib/ansible_test/_data/legacy_collection_loader/_collection_meta.py +++ b/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):