diff --git a/changelog/62932.fixed b/changelog/62932.fixed new file mode 100644 index 000000000000..c4a737a7a0d2 --- /dev/null +++ b/changelog/62932.fixed @@ -0,0 +1,37 @@ +Improvements to YAML processing (`salt.utils.yaml`) that take effect +immediately: + * Passing a non-`dict` class to the `salt.utils.yaml.SaltYamlSafeLoader` + constructor no longer causes all future `!!omap` nodes to raise an exception + when loaded. + * Loading a sequence node tagged with `!!python/tuple` is now supported, and + produces a Python `tuple` object. + +Improvements to YAML processing that are expected to take effect in Salt 3007 +but can be previewed now by setting the option `yaml_compatibility` to `3007`: + * Loading a well-formed `!!omap` node (a sequence of single-entry mapping + nodes) will always produce a `collections.OrderedDict` object. (Currently + it sometimes produces a `list` of `tuple` (key, value) pairs and sometimes + raises an exception, depending on the value of + `salt.utils.yaml.SaltYamlSafeLoader`'s `dictclass` constructor parameter.) + * Loading a mapping node tagged with `!!omap` will always raise an exception + due to invalid syntax. (Currently it sometimes raises an exception and + sometimes produces a `collections.OrderedDict` object, depending on the + value of `salt.utils.yaml.SaltYamlSafeLoader`'s `dictclass` constructor + parameter.) + * Dumping a `collections.OrderedMap` will consistently produce an `!!omap` + node (a sequence of single-entry mapping nodes). + * Loading an explicitly tagged `!!timestamp` node will produce a + `datetime.datetime` object instead of a string. + * Dumping a `datetime.datetime` object will explicitly tag the node with + `!!timestamp`. (Currently the nodes are untagged.) + * Dumping a `tuple` object will consistently produce a sequence node + explicitly tagged with `!!python/tuple`. (Currently `safe_dump()` omits the + tag and `dump()` includes it.) + * `salt.utils.yaml.dump()` will default to `salt.utils.yaml.OrderedDumper` + instead of `yaml.Dumper`. + * Dumping a YAML sequence with `salt.utils.yaml.IndentedSafeOrderedDumper` + will be properly indented. + * Dumping an object with an unsupported type via `salt.utils.yaml.safe_dump()` + (or via `dump()` with the `SafeOrderedDumper` or `IndentedSafeOrderedDumper` + classes) will raise an exception. (Currently such objects produce a null + node.) diff --git a/changelog/63158.fixed b/changelog/63158.fixed new file mode 100644 index 000000000000..76f019daf155 --- /dev/null +++ b/changelog/63158.fixed @@ -0,0 +1,2 @@ +Updated YAML idiosyncrasies documentation, improved YAML tests, and improved +readability of YAML code diff --git a/doc/ref/configuration/master.rst b/doc/ref/configuration/master.rst index 2e2086e0e789..22f7965925e4 100644 --- a/doc/ref/configuration/master.rst +++ b/doc/ref/configuration/master.rst @@ -1177,6 +1177,62 @@ See the :ref:`2019.2.1 release notes ` for more details. use_yamlloader_old: False +.. conf_master:: yaml_compatibility + +``yaml_compatibility`` +---------------------- + +.. versionadded:: 3006.0 + +Default: ``None`` + +Force the behavior of the YAML loader (parser) and dumper (generator) to match +the default behavior from a specific version of Salt. Omitting this or setting +it to ``None`` (in YAML: ``null``, ``~``, or an empty value) is equivalent to +setting it to the current version of Salt. + +Changes that are expected in a future version of Salt can be previewed by +setting this to that future version number. + +This setting does not take effect until after the config file is parsed (so +setting this value does not change how the config file itself is parsed). + +Note that pillar `.sls` files are parsed on the master so setting this on the +master affects pillar YAML files. State `.sls` files are parsed by the minions, +so setting this on the master does not affect how state YAML files are parsed. + +.. code-block:: yaml + + # Use the default behavior. + yaml_compatibility: null + + # Any of the following will force the YAML loader/dumper to behave like + # it did in Salt v3006.0: + yaml_compatibility: 3006 + yaml_compatibility: 3006.0 + yaml_compatibility: v3006 + yaml_compatibility: v3006.0 + yaml_compatibility: sulfur + yaml_compatibility: Sulfur + yaml_compatibility: SULFUR + +.. conf_master:: yaml_compatibility_warnings + +``yaml_compatibility_warnings`` +------------------------------- + +.. versionadded:: 3006.0 + +Default: ``True`` + +Boolean indicating whether to print warnings about upcoming changes to the YAML +loader/dumper that could affect compatibility. Set to `False` to silence the +warnings. + +.. code-block:: yaml + + yaml_compatibility_warnings: True + .. conf_master:: req_server_niceness ``req_server_niceness`` diff --git a/doc/ref/configuration/minion.rst b/doc/ref/configuration/minion.rst index bf2fe0e69586..593a013e875c 100644 --- a/doc/ref/configuration/minion.rst +++ b/doc/ref/configuration/minion.rst @@ -1687,6 +1687,63 @@ See the :ref:`2019.2.1 release notes ` for more details. use_yamlloader_old: False +.. conf_minion:: yaml_compatibility + +``yaml_compatibility`` +---------------------- + +.. versionadded:: 3006.0 + +Default: ``None`` + +Force the behavior of the YAML loader (parser) and dumper (generator) to match +the default behavior from a specific version of Salt. Omitting this or setting +it to ``None`` (in YAML: ``null``, ``~``, or an empty value) is equivalent to +setting it to the current version of Salt. + +Changes that are expected in a future version of Salt can be previewed by +setting this to that future version number. + +This setting does not take effect until after the config file is parsed (so +setting this value does not change how the config file itself is parsed). + +Note that pillar `.sls` files are parsed on the master so setting this on the +minion does not affect pillar YAML files. State `.sls` files are parsed by the +minions, so setting this on the minion does affect how state YAML files are +parsed. + +.. code-block:: yaml + + # Use the default behavior. + yaml_compatibility: null + + # Any of the following will force the YAML loader/dumper to behave like + # it did in Salt v3006.0: + yaml_compatibility: 3006 + yaml_compatibility: 3006.0 + yaml_compatibility: v3006 + yaml_compatibility: v3006.0 + yaml_compatibility: sulfur + yaml_compatibility: Sulfur + yaml_compatibility: SULFUR + +.. conf_minion:: yaml_compatibility_warnings + +``yaml_compatibility_warnings`` +------------------------------- + +.. versionadded:: 3006.0 + +Default: ``True`` + +Boolean indicating whether to print warnings about upcoming changes to the YAML +loader/dumper that could affect compatibility. Set to `False` to silence the +warnings. + +.. code-block:: yaml + + yaml_compatibility_warnings: True + Docker Configuration ==================== diff --git a/doc/topics/troubleshooting/yaml_idiosyncrasies.rst b/doc/topics/troubleshooting/yaml_idiosyncrasies.rst index 1ee1f5326f21..a49d682a2f65 100644 --- a/doc/topics/troubleshooting/yaml_idiosyncrasies.rst +++ b/doc/topics/troubleshooting/yaml_idiosyncrasies.rst @@ -13,6 +13,25 @@ unexpected times. .. _`YAML`: https://yaml.org/spec/1.1/ +.. versionchanged:: 3006.0 + + All of the behavior changes slated for Salt v3007.0 can be previewed by + setting the ``yaml_compatibility`` option to 3007. + +.. versionchanged:: 3006.0 + + YAML compatibility warnings can be disabled by setting the + ``yaml_compatibility_warnings`` option to False. + +.. versionchanged:: 3007.0 + + Dumping an object of unsupported type to YAML with a one of the safe dumpers + (:py:func:`~salt.utils.yaml.safe_dump`, or :py:func:`~salt.utils.yaml.dump` + with the :py:class:`~salt.utils.yaml.SafeOrderedDumper` or + :py:class:`~salt.utils.yaml.IndentedSafeOrderedDumper` classes) now raises + an exception. Previously it produced a YAML ``NULL`` node. Set the + ``yaml_compatibility`` option to 3006 to revert to the previous behavior. + Spaces vs Tabs ============== @@ -382,50 +401,134 @@ Here's an example: Automatic ``datetime`` conversion ================================= -If there is a value in a YAML file formatted ``2014-01-20 14:23:23`` or -similar, YAML will automatically convert this to a Python ``datetime`` object. -These objects are not msgpack serializable, and so may break core salt -functionality. If values such as these are needed in a salt YAML file -(specifically a configuration file), they should be formatted with surrounding -strings to force YAML to serialize them as strings: +.. versionchanged:: 2018.3.0 + + A YAML scalar node containing a timestamp now always produces a string. + Previously, Salt would attempt to create a Python ``datetime.datetime`` + object, even if the node contained an invalid date (for example, + ``4017-16-20``). + +.. versionchanged:: 3007.0 + + Loading a YAML ``!!timestamp`` node now produces a ``datetime.datetime`` + object. Previously, nodes tagged with ``!!timestamp`` produced strings. + Set the ``yaml_compatibility`` option to 3006 to revert to the previous + behavior. + +.. versionchanged:: 3007.0 + + Dumping a ``datetime.datetime`` object to YAML now explicitly tags the node + with ``!!timestamp``. Previously, the ``!!timestamp`` tag was omitted. + Set the ``yaml_compatibility`` option to 3006 to revert to the previous + behavior. + +Salt overrides PyYAML's default behavior and loads YAML nodes that look like +timestamps as strings: .. code-block:: pycon - >>> import yaml - >>> yaml.safe_load("2014-01-20 14:23:23") - datetime.datetime(2014, 1, 20, 14, 23, 23) - >>> yaml.safe_load('"2014-01-20 14:23:23"') + >>> import salt.utils.yaml + >>> salt.utils.yaml.safe_load("2014-01-20 14:23:23") '2014-01-20 14:23:23' -Additionally, numbers formatted like ``XXXX-XX-XX`` will also be converted (or -YAML will attempt to convert them, and error out if it doesn't think the date -is a real one). Thus, for example, if a minion were to have an ID of -``4017-16-20`` the minion would not start because YAML would complain that the -date was out of range. The workaround is the same, surround the offending -string with quotes: +To force Salt to produce a ``datetime.datetime`` object instead of a string, +explicitly tag the node with ``!!timestamp``: .. code-block:: pycon - >>> import yaml - >>> yaml.safe_load("4017-16-20") - Traceback (most recent call last): - File "", line 1, in - File "/usr/local/lib/python2.7/site-packages/yaml/__init__.py", line 93, in safe_load - return load(stream, SafeLoader) - File "/usr/local/lib/python2.7/site-packages/yaml/__init__.py", line 71, in load - return loader.get_single_data() - File "/usr/local/lib/python2.7/site-packages/yaml/constructor.py", line 39, in get_single_data - return self.construct_document(node) - File "/usr/local/lib/python2.7/site-packages/yaml/constructor.py", line 43, in construct_document - data = self.construct_object(node) - File "/usr/local/lib/python2.7/site-packages/yaml/constructor.py", line 88, in construct_object - data = constructor(self, node) - File "/usr/local/lib/python2.7/site-packages/yaml/constructor.py", line 312, in construct_yaml_timestamp - return datetime.date(year, month, day) - ValueError: month must be in 1..12 - >>> yaml.safe_load('"4017-16-20"') - '4017-16-20' + >>> import salt.utils.yaml + >>> salt.utils.yaml.safe_load("!!timestamp 2014-01-20 14:23:23") + datetime.datetime(2014, 1, 20, 14, 23, 23) + +When dumping a ``datetime.datetime`` object to YAML, Salt tags the node with +``!!timestamp`` so that it will be loaded back as a ``datetime.datetime`` +object. + +Beware that Salt is currently unable to serialize ``datetime.datetime`` objects, +so ``!!timestamp`` nodes cannot be used in pillar SLS files. + +Ordered Dictionaries +==================== + +.. versionchanged:: 3007.0 + + Loading a YAML ``!!omap`` node now reliably produces a + ``collections.OrderedDict`` object. Previously, an ``!!omap`` node would + sometimes produce a ``list`` of ``tuple`` (key, value) pairs and sometimes + raise an exception. Set the ``yaml_compatibility`` option to 3006 to revert + to the previous behavior. + +.. versionchanged:: 3007.0 + + Dumping any ``collections.OrderedDict`` object to YAML now reliably produces + an ``!!omap`` node. Previously, only the subtype + ``salt.utils.odict.OrderedDict`` was supported, and it always produced a + plain mapping node. Other types produced various constructs that could not + be loaded back as any kind of ``dict``. Set the ``yaml_compatibility`` + option to 3006 to revert to the previous behavior. + +The YAML specification defines an `ordered mapping type +`_ which is equivalent to a plain mapping except +iteration order is preserved. (YAML makes no guarantees about iteration order +for entries loaded from a plain mapping.) + +Ordered mappings are represented as an ``!!omap`` tagged sequence of +single-entry mappings: + +.. code-block:: yaml + + !!omap + - key1: value1 + - key2: value2 + +Starting with Python 3.6, plain ``dict`` objects iterate in insertion order so +there is no longer a strong need for the ``!!omap`` type. However, some users +may prefer the ``!!omap`` type over the plain ``!!map`` type because (1) it +makes it obvious that the order of entries is significant, and (2) it provides a +stronger guarantee of iteration order (plain mapping iteration order can be +thought of as a Salt implementation detail that may change in the future). + +Salt produces a ``collections.OrderedDict`` object when it loads an ``!!omap`` +node. (Salt's behavior differs from PyYAML's default behavior, which is to +produce a ``list`` of (key, value) ``tuple`` objects.) These objects are a +subtype of ``dict``, so ``!!omap`` is a drop-in replacement for a plain mapping. + +When dumping a ``collections.OrderedDict`` object to YAML, Salt produces an +``!!omap`` node. + +Beware that Salt currently serializes ``collections.OrderedDict`` objects the +same way it serializes plain ``dict`` objects, so they become plain ``dict`` +objects when deserialized by the recipient. + +Tuples +====== + +.. versionchanged:: 3006.0 + + Loading a YAML ``!!python/tuple`` node is now supported. + +.. versionchanged:: 3007.0 + + Dumping a ``tuple`` object to YAML now always produces a sequence node + tagged with ``!!python/tuple``. Previously, ``salt.utils.yaml.safe_dump()`` + did not tag the node. Set the ``yaml_compatibility`` option to 3006 to + revert to the previous behavior. + +The YAML ``!!python/tuple`` type can be used to produce a Python ``tuple`` +object when loaded: + +.. code-block:: yaml + + !!python/tuple + - first item + - second item + +When dumped to YAML, a ``tuple`` object produces a sequence node tagged with +``!!python/tuple``. +Beware that Salt currently serializes ``tuple`` objects the same way it +serializes ``list`` objects, so they become ``list`` objects when deserialized +by the recipient. Keys Limited to 1024 Characters =============================== diff --git a/salt/config/__init__.py b/salt/config/__init__.py index 372555969d2a..9214f885377a 100644 --- a/salt/config/__init__.py +++ b/salt/config/__init__.py @@ -27,7 +27,6 @@ import salt.utils.validate.path import salt.utils.versions import salt.utils.xdg -import salt.utils.yaml from salt._logging import ( DFLT_LOG_DATEFMT, DFLT_LOG_DATEFMT_LOGFILE, @@ -979,6 +978,8 @@ def _gather_buffer_space(): "pass_gnupghome": str, # pass renderer: Set PASSWORD_STORE_DIR env for Pass "pass_dir": str, + "yaml_compatibility": (type(None), str, int), + "yaml_compatibility_warnings": (type(None), bool), } ) @@ -1283,6 +1284,8 @@ def _gather_buffer_space(): "global_state_conditions": None, "reactor_niceness": None, "fips_mode": False, + "yaml_compatibility": None, + "yaml_compatibility_warnings": True, } ) @@ -1626,6 +1629,8 @@ def _gather_buffer_space(): "pass_gnupghome": "", "pass_dir": "", "netapi_enable_clients": [], + "yaml_compatibility": None, + "yaml_compatibility_warnings": True, } ) @@ -1995,6 +2000,7 @@ def _read_conf_file(path): """ Read in a config file from a given path and process it into a dictionary """ + import salt.utils.yaml log.debug("Reading configuration from %s", path) append_file_suffix_YAMLError = False with salt.utils.files.fopen(path, "r") as conf_file: @@ -2310,6 +2316,7 @@ def mminion_config(path, overrides, ignore_config_errors=True): apply_sdb(opts) _validate_opts(opts) + import salt.loader opts["grains"] = salt.loader.grains(opts) opts["pillar"] = {} return opts diff --git a/salt/loader/__init__.py b/salt/loader/__init__.py index 72a5e5440128..956ec701a062 100644 --- a/salt/loader/__init__.py +++ b/salt/loader/__init__.py @@ -12,7 +12,6 @@ import time import types -import salt.config import salt.defaults.events import salt.defaults.exitcodes import salt.loader.context @@ -1060,8 +1059,9 @@ def grains(opts, force_refresh=False, proxy=None, context=None, loaded_base_name __grains__ = salt.loader.grains(__opts__) print __grains__['id'] """ - # Need to re-import salt.config, somehow it got lost when a minion is starting - import salt.config + # Without this global declaration, the late `import` statement below causes + # `salt` to become a function-local variable. + global salt # if we have no grains, lets try loading from disk (TODO: move to decorator?) cfn = os.path.join(opts["cachedir"], "grains.cache.p") @@ -1076,6 +1076,7 @@ def grains(opts, force_refresh=False, proxy=None, context=None, loaded_base_name return {} grains_deep_merge = opts.get("grains_deep_merge", False) is True if "conf_file" in opts: + import salt.config pre_opts = {} pre_opts.update( salt.config.load_config( diff --git a/salt/minion.py b/salt/minion.py index 7cbaa35f3e24..ac7fa40e3a5c 100644 --- a/salt/minion.py +++ b/salt/minion.py @@ -123,7 +123,6 @@ def resolve_dns(opts, fallback=True): "use_master_when_local", False ): check_dns = False - import salt.utils.network if check_dns is True: try: @@ -914,9 +913,11 @@ class SMinion(MinionBase): """ def __init__(self, opts, context=None): - # Late setup of the opts grains, so we can log from the grains module - import salt.loader + # Without this global declaration, the late `import` statement below + # causes `salt` to become a function-local variable. + global salt + # Late setup of the opts grains, so we can log from the grains module opts["grains"] = salt.loader.grains(opts) super().__init__(opts) diff --git a/salt/utils/_yaml_common.py b/salt/utils/_yaml_common.py new file mode 100644 index 000000000000..c8edb2427e9f --- /dev/null +++ b/salt/utils/_yaml_common.py @@ -0,0 +1,323 @@ +import collections +import contextvars +import logging +import warnings + +import salt.utils.versions +import salt.version +from salt.utils.decorators import classproperty +from salt.version import SaltStackVersion + +__opts__ = { + "yaml_compatibility": None, + # Suppress the warnings until the config file has been loaded. + "yaml_compatibility_warnings": False, +} +__salt_loader__ = None + +log = logging.getLogger(__name__) + +_compat_opt = contextvars.ContextVar(f"{__name__}._compat_opt") +_compat_ver = contextvars.ContextVar(f"{__name__}._compat_ver") + +# Current Salt release. Changing this value potentially changes the default +# behavior of the YAML loader/dumper logic (the behavior when the +# `yaml_compatibility` option is left unset). This should be kept in sync with +# `SaltStackVersion.current_release()` (modulo +# https://github.com/saltstack/salt/issues/62972). +# +# This variable could be initialized to `SaltStackVersion.current_release()` to +# avoid the need to manually keep it in sync, but updating it separately has a +# few advantages: +# * It is easier to create a new release because doing so can't trigger YAML +# behavior changes that cause test failures that are difficult to root +# cause. +# * Updating this in a separate commit makes it possible for `git bisect` to +# reveal that a mysterious new bug is caused by a YAML behavior change. +# * Updating this in a separate PR provides a good opportunity to mention in +# the changelog that previously announced opt-in YAML behavior changes are +# now opt-out. +_current_ver = SaltStackVersion(3006) + + +class OverrideNotice(UserWarning): + pass + + +class UnsupportedValueWarning(UserWarning): + pass + + +# Delayed import of salt.loader.context to avoid a circular import error. +def _init(): + global __salt_loader__, __opts__ + if __salt_loader__ is None: + import salt.loader.context + __salt_loader__ = salt.loader.context.LoaderContext() + __opts__ = __salt_loader__.named_context("__opts__", __opts__) + + +def compat_ver(): + """Gets the Salt version the YAML loader/dumper should try to match + + Occasionally, the behavior of :py:mod:`~salt.utils.yaml` is changed in ways + that can subtly break backwards compatibility. This function returns a + :py:class:`salt.version.SaltStackVersion` object that represents the parsed + and validated value of the ``yaml_compatibility`` configuration option. + That option provides a way for users to force the behavior of the YAML + loader/dumper to match that of a specific version of Salt until they work + out any compatibility issues. + """ + _init() + v = _compat_ver.get(None) + opt = __opts__.get("yaml_compatibility") + # This function is called very early during start-up -- before the config is + # loaded -- so __opts__["yaml_compatibility"] might not be populated yet. + if v is None or opt != _compat_opt.get(None): + _compat_opt.set(opt) + show_warnings = __opts__.get("yaml_compatibility_warnings") + if show_warnings is None: + show_warnings = True + + def _warn(msg, category, *, force_show=False): + log.warning(msg) + if show_warnings or force_show: + warnings.warn(msg, category) + + # Warn devs to update _current_ver after each release. + actual_current_ver = SaltStackVersion.current_release() + # TODO: This loop can be removed after fixing + # https://github.com/saltstack/salt/issues/62972 + for v in salt.version.SaltVersionsInfo.versions(): + if not v.released: + actual_current_ver = SaltStackVersion(*v.info) + break + if _current_ver < actual_current_ver: + _warn( + f"{__name__}._current_ver is behind ({_current_ver}); please " + f"update it to the current release ({actual_current_ver})", + RuntimeWarning, + force_show=True, + ) + + v = _current_ver + if opt: + if isinstance(opt, SaltStackVersion): + v = opt + elif isinstance(opt, int): + v = SaltStackVersion(opt) + else: + v = SaltStackVersion.parse(opt) + opt_supported = True # Whether the value in opt is supported. + for changev, dropv in [ + # List of version pairs. The first version in the pair indicates + # when a change in the default behavior took (or will take) effect. + # The second version in the pair indicates when support for + # yaml_compatibility values less than the first version will be + # dropped. + (3006, 3006), # Versions < 3006 have never been supported. + (3007, 3011), # Support for < 3007 will be dropped in 3011. + ]: + changev = SaltStackVersion(changev) + dropv = SaltStackVersion(dropv) + if v < changev: + if _current_ver >= dropv: + opt_supported = False + v = changev + elif opt: + _warn( + "support for yaml_compatibility option values less " + f"than {changev} will be removed in Salt {dropv}", + FutureWarning, + ) + if not opt and changev > _current_ver and v < changev: + _warn( + "Salt's default YAML processing behavior will change in " + f"version {changev}; to preview the changes set the " + f"yaml_compatibility option to {changev} or higher", + FutureWarning, + ) + if not opt_supported: + _warn( + f"the value of the yaml_compatibility option is less than the " + f"minimum supported value {v}: {opt!r}", + UnsupportedValueWarning, + # This is a bug in the user's config (or in Salt tests), so + # always show the warning. + force_show=True, + ) + if opt: + _warn( + f"forcing YAML processing behavior to match Salt version {v} " + "due to yaml_compatibility option", + OverrideNotice, + ) + log.debug(f"YAML compatibility version: {v}") + _compat_ver.set(v) + return v + + +class VersionedSubclassesMixin: + """Automatically adds version-specific subclasses as class attributes. + + For each main type that inherits this mixin, an additional subclass per + supported ``yaml_compatibility`` version range is automatically defined and + saved as an attribute of the main type. Each of these version-specific + subtypes has multiple base classes: the associated main type, and the + version-specific subtypes associated with the main type's base types. For + example, with types Foo◀─Bar◀─Baz and versions 3006 and 3007, the + inheritance DAG will look like this:: + + VersionedSubclassesMixin + ▲ + │ + Foo◀─────╮──────────╮ + ▲ │ │ + │ Foo.V3006 Foo.V3007 + │ ▲ ▲ + Bar◀───╮─│────────╮ │ + ▲ ╰─┤ ╰─┤ + │ Bar.V3006 Bar.V3007 + │ ▲ ▲ + Baz◀───╮─│────────╮ │ + ╰─┤ ╰─┤ + Baz.V3006 Baz.V3007 + + """ + + @classmethod + def __init_subclass__( + cls, *, versioned_properties=(), _versioned_subclass=False, **kwargs + ): + """Automatically adds version-specific subclasses as class attributes. + + :param versioned_properties: + Optional iterable of names that refer to getters decorated by + :py:func:`~salt.utils.decorators.classproperty`. For each name, a + new ``classproperty``-decorated getter is defined that uses the + return value of :py:func:`compat_ver` to automatically select the + getter from the appropriate versioned subclass. + + """ + super().__init_subclass__(**kwargs) + if _versioned_subclass: + return + + for name in versioned_properties: + + def make_getter(name): # Capture the loop value in the closure. + def getter(cls): + # This getter is inherited by the versioned subclasses. + if getattr(cls, "_versioned_subclass_for", None): + return getattr(super(), name) + elif compat_ver() < SaltStackVersion(3007): + return getattr(cls.V3006, name) + else: + return getattr(cls.V3007, name) + + getter.__name__ = name + getter.__qualname__ = f"{cls.__qualname__}.{name}" + return getter + + setattr(cls, name, classproperty(make_getter(name))) + + def add_versioned_subclass(attr): + bases = tuple( + [cls] + + [ + getattr(b, attr) + for b in cls.__bases__ + if ( + issubclass(b, VersionedSubclassesMixin) + and not issubclass(VersionedSubclassesMixin, b) + ) + ] + ) + subcls = type( + f"{cls.__name__}.{attr}", + bases, + {"_versioned_subclass_for": cls}, + # This new type is derived from VersionedSubclassesMixin, so we + # need to prevent infinite recursion (versioned subclasses + # should not have their own nested versioned subclasses). + _versioned_subclass=True, + ) + setattr(cls, attr, subcls) + + add_versioned_subclass("V3006") + add_versioned_subclass("V3007") + + +class InheritMapMixin: + """Adds :py:class:`collections.ChainMap` class attributes based on MRO. + + The added class attributes provide each subclass with its own mapping that + works just like method resolution: each ancestor type in ``__mro__`` is + visited until an entry is found in the ancestor-owned map. + + For example, for an inheritance DAG of ``InheritMapMixin`` <- ``Foo`` <- + ``Bar`` <- ``Baz`` and a desired attribute name of ``"_m"``: + + 1. ``Foo._m`` is set to ``ChainMap({})``. + 2. ``Bar._m`` is set to ``ChainMap({}, Foo._m.maps[0])``. + 3. ``Baz._m`` is set to ``ChainMap({}, Bar._m.maps[0], Foo._m.maps[0])``. + + This mixin makes it possible to improve PyYAML's representer and constructor + registration APIs. Currently, the PyYAML + :py:class:`yaml.representer.BaseRepresenter` and + :py:class:`yaml.constructor.BaseConstructor` classes do not use base class + inheritance of registered representers/constructors. Instead, the base + class's registrations are copied to the subclass's registration dict the + first time a registration is added to the subclass. (Before the first + registration is added to the subclass, MRO lookup finds the base class's + registration dict.) This means that any registrations added to a base class + after a registration is added to a subclass do not automatically appear in + the subclass. This results in the need to either add all registrations + before any subclass registrations are added, or manually add registrations + to each subclass in addition to the base class. + + If this mixin is used to provide ``yaml_representers`` and + ``yaml_multi_representers`` class attributes for PyYAML dumper classes, + then registered representers are truly inherited, not copied, from their + base classes. The same goes for constructors registered in the + ``yaml_constructors`` and ``yaml_multi_constructors`` class attributes for + PyYAML loader classes. + + """ + + @classmethod + def __init_subclass__(cls, *, inherit_map_attrs=None, **kwargs): + """Adds :py:class:`collections.ChainMap` class attributes based on MRO. + + :param inherit_map_attrs: + Optional mapping: + + * Each key in the mapping is the name of a class attribute that will + be set to a :py:class:`~collections.ChainMap` containing the + MRO-based list of ancestor maps. + * Each value is an optional (may be ``None``) fallback class + attribute name to try if an ancestor class in the MRO does not + have a chain map provided by ``InheritMapMixin``. For such + ancestor classes, if it has a non-``None`` attribute with that + name, the value is added to the chain if it is not already + present. + + """ + super().__init_subclass__(**kwargs) + attrs = getattr(cls, "_inherit_map_attrs", {}) + if inherit_map_attrs: + attrs = {**attrs, **inherit_map_attrs} + cls._inherit_map_attrs = attrs + for name, fallback_name in attrs.items(): + maps = [{}] + for c in cls.__mro__[1:]: # cls.__mro__[0] is cls itself. + if issubclass(InheritMapMixin, c): + continue # Ignore InheritMapMixin itself and object. + c_attrs = getattr(c, "_inherit_map_attrs", {}) + if issubclass(c, InheritMapMixin) and name in c_attrs: + maps.append(getattr(c, name).maps[0]) + elif fallback_name: + m = getattr(c, fallback_name, None) + if m is not None and m not in maps: + maps.append(m) + setattr(cls, name, collections.ChainMap(*maps)) diff --git a/salt/utils/args.py b/salt/utils/args.py index 536aea38166c..7aaf7c8b3400 100644 --- a/salt/utils/args.py +++ b/salt/utils/args.py @@ -13,7 +13,6 @@ import salt.utils.data import salt.utils.jid import salt.utils.versions -import salt.utils.yaml from salt.exceptions import SaltInvocationError log = logging.getLogger(__name__) @@ -134,6 +133,7 @@ def yamlify_arg(arg): """ yaml.safe_load the arg """ + import salt.utils.yaml if not isinstance(arg, str): return arg @@ -170,9 +170,6 @@ def yamlify_arg(arg): return arg try: - # Explicit late import to avoid circular import. DO NOT MOVE THIS. - import salt.utils.yaml - original_arg = arg if "#" in arg: # Only yamlify if it parses into a non-string type, to prevent @@ -360,9 +357,6 @@ def test_mode(**kwargs): "Test" in any variation on capitalization (i.e. "TEST", "Test", "TeSt", etc) contains a True value (as determined by salt.utils.data.is_true). """ - # Once is_true is moved, remove this import and fix the ref below - import salt.utils - for arg, value in kwargs.items(): try: if arg.lower() == "test" and salt.utils.data.is_true(value): diff --git a/salt/utils/data.py b/salt/utils/data.py index f50ebbab789f..947e5100f226 100644 --- a/salt/utils/data.py +++ b/salt/utils/data.py @@ -16,7 +16,6 @@ import salt.utils.dictupdate import salt.utils.stringutils -import salt.utils.yaml from salt.defaults import DEFAULT_TARGET_DELIM from salt.exceptions import SaltException from salt.utils.decorators.jinja import jinja_filter @@ -1042,6 +1041,7 @@ def repack_dictlist(data, strict=False, recurse=False, key_cb=None, val_cb=None) repacks into a single dictionary. """ if isinstance(data, str): + import salt.utils.yaml try: data = salt.utils.yaml.safe_load(data) except salt.utils.yaml.parser.ParserError as err: diff --git a/salt/utils/decorators/__init__.py b/salt/utils/decorators/__init__.py index 1f62d5f3d654..f8c97413d300 100644 --- a/salt/utils/decorators/__init__.py +++ b/salt/utils/decorators/__init__.py @@ -12,8 +12,6 @@ from collections import defaultdict from functools import wraps -import salt.utils.args -import salt.utils.data import salt.utils.versions from salt.exceptions import ( CommandExecutionError, @@ -117,6 +115,7 @@ class wide dependency_dict @staticmethod def run_command(dependency, mod_name, func_name): + import salt.utils.args full_name = "{}.{}".format(mod_name, func_name) log.trace("Running '%s' for '%s'", dependency, full_name) if IS_WINDOWS: @@ -256,6 +255,7 @@ def timing(function): @wraps(function) def wrapped(*args, **kwargs): + import salt.utils.args start_time = time.time() ret = function(*args, **salt.utils.args.clean_kwargs(**kwargs)) end_time = time.time() @@ -866,3 +866,42 @@ def wrapped(*args, **kwargs): return function(*args, **kwargs) return wrapped + + +class classproperty(property): + """Decorator like :py:func:`property`, but for class properties. + + Usage example: + + .. code-block:: python + + class Foo: + @classproperty + def custom_prop(cls): + return "hello world" + + assert Foo.custom_prop == "hello world" + + """ + + def __init__(self, fget=None, fset=None, fdel=None, doc=None): + if fset is not None: + raise NotImplementedError( + "classproperty setters are not supported due to a limitation " + "in Python: " + ) + if fdel is not None: + raise NotImplementedError( + "classproperty deleters are not supported due to a limitation " + "in Python: " + ) + super().__init__(fget, doc=doc) + + def __get__(self, obj, objtype=None): + if objtype is None: + objtype = type(obj) + # The second argument probably doesn't matter, but @classmethod does + # `__get__(objtype, objtype)`, not `__get__(objtype, type(objtype))`, so + # the same thing is done here for consistency. + # https://github.com/python/cpython/blob/v3.11.0/Objects/funcobject.c#L900-L901 + return super().__get__(objtype, objtype) diff --git a/salt/utils/yamldumper.py b/salt/utils/yamldumper.py index e5e937cac7d6..121ac038fa63 100644 --- a/salt/utils/yamldumper.py +++ b/salt/utils/yamldumper.py @@ -11,8 +11,11 @@ import yaml # pylint: disable=blacklisted-import +import salt.utils._yaml_common as _yaml_common import salt.utils.context +from salt.utils.decorators import classproperty from salt.utils.odict import OrderedDict +from salt.version import SaltStackVersion try: from yaml import CDumper as Dumper @@ -31,69 +34,201 @@ ] -class IndentMixin(Dumper): - """ - Mixin that improves YAML dumped list readability - by indenting them by two spaces, - instead of being flush with the key they are under. - """ - - def increase_indent(self, flow=False, indentless=False): - return super().increase_indent(flow, False) - - -class OrderedDumper(Dumper): - """ - A YAML dumper that represents python OrderedDict as simple YAML map. +class _InheritedRepresentersMixin( + _yaml_common.InheritMapMixin, + inherit_map_attrs={ + # The ChainMap used for yaml_representers is not saved directly to + # cls.yaml_representers because _yaml_common.VersionedSubclassesMixin is + # used to automatically switch between versioned variants of the + # ChainMap, and we still need access to the unversioned ChainMap (for + # add_representer(), and for subclasses to chain off of). + "_reps": "yaml_representers", + # Same goes for _mreps. + "_mreps": "yaml_multi_representers", + }, +): + @classproperty + def yaml_representers(cls): # pylint: disable=no-self-argument + return cls._reps + + @classproperty + def yaml_multi_representers(cls): # pylint: disable=no-self-argument + return cls._mreps + + @classmethod + def add_representer(cls, data_type, rep): + cls._reps[data_type] = rep + + @classmethod + def add_multi_representer(cls, data_type, rep): + cls._mreps[data_type] = rep + + +class _VersionedRepresentersMixin( + _yaml_common.VersionedSubclassesMixin, + versioned_properties=( + "yaml_implicit_resolvers", + "yaml_multi_representers", + "yaml_representers", + ), +): + pass + + +class _ExplicitTimestampMixin: + """Disables the implicit timestamp resolver for Salt 3007 and later. + + Once support for ``yaml_compatibility`` less than 3007 is dropped, we can + instead remove the implicit resolver entirely: + + .. code-block:: python + + class _RemoveImplicitResolverMixin: + @classmethod + def remove_implicit_resolver(cls, tag): + cls.yaml_implicit_resolvers = { + first_char: [r for r in resolver_list if r[0] != tag] + for first_char, resolver_list in cls.yaml_implicit_resolvers.items() + } + + _CommonMixin.remove_implicit_resolver("tag:yaml.org,2002:timestamp") """ + def resolve(self, kind, value, implicit): + tag = super().resolve(kind, value, implicit) + if ( + kind is yaml.ScalarNode + and implicit[0] + and tag == "tag:yaml.org,2002:timestamp" + and _yaml_common.compat_ver() >= SaltStackVersion(3007) + ): + return self.DEFAULT_SCALAR_TAG + return tag -class SafeOrderedDumper(SafeDumper): - """ - A YAML safe dumper that represents python OrderedDict as simple YAML map. - """ +class _CommonMixin( + _ExplicitTimestampMixin, + _VersionedRepresentersMixin, + _InheritedRepresentersMixin, +): + def _rep_ordereddict_as_plain_map(self, data): + return self.represent_dict(list(data.items())) -class IndentedSafeOrderedDumper(IndentMixin, SafeOrderedDumper): - """ - A YAML safe dumper that represents python OrderedDict as simple YAML map, - and also indents lists by two spaces. - """ + def _rep_ordereddict(self, data): + seq = [{k: v} for k, v in data.items()] + return self.represent_sequence("tag:yaml.org,2002:omap", seq) + def _rep_default(self, data): + """Represent types that don't match any other registered representers. -def represent_ordereddict(dumper, data): - return dumper.represent_dict(list(data.items())) + PyYAML's default behavior for unsupported types is to raise an + exception. This representer instead produces null nodes. + Note: This representer does not affect ``Dumper``-derived classes + because ``Dumper`` has a multi representer registered for ``object`` + that will match every object before PyYAML falls back to this + representer. -def represent_undefined(dumper, data): - return dumper.represent_scalar("tag:yaml.org,2002:null", "NULL") + """ + return self.represent_scalar("tag:yaml.org,2002:null", "NULL") -OrderedDumper.add_representer(OrderedDict, represent_ordereddict) -SafeOrderedDumper.add_representer(OrderedDict, represent_ordereddict) -SafeOrderedDumper.add_representer(None, represent_undefined) - -OrderedDumper.add_representer( - collections.defaultdict, yaml.representer.SafeRepresenter.represent_dict +_CommonMixin.V3006.add_representer(None, _CommonMixin._rep_default) +_CommonMixin.V3006.add_representer( + OrderedDict, _CommonMixin._rep_ordereddict_as_plain_map ) -SafeOrderedDumper.add_representer( - collections.defaultdict, yaml.representer.SafeRepresenter.represent_dict +# This multi representer covers collections.OrderedDict and all of its +# subclasses, including salt.utils.odict.OrderedDict. +_CommonMixin.V3007.add_multi_representer( + collections.OrderedDict, _CommonMixin._rep_ordereddict ) -OrderedDumper.add_representer( - salt.utils.context.NamespacedDictWrapper, - yaml.representer.SafeRepresenter.represent_dict, +# This non-multi representer may seem redundant given the multi representer +# registered above, but it is needed to override the non-multi representer +# that exists in the ancestor Representer class. (Non-multi representers +# take priority over multi representers.) +_CommonMixin.V3007.add_representer( + collections.OrderedDict, _CommonMixin._rep_ordereddict ) -SafeOrderedDumper.add_representer( +_CommonMixin.add_representer( + collections.defaultdict, yaml.representer.SafeRepresenter.represent_dict +) +_CommonMixin.add_representer( salt.utils.context.NamespacedDictWrapper, yaml.representer.SafeRepresenter.represent_dict, ) +# SafeDumper represents tuples as lists, but Dumper's behavior (sequence +# tagged with `!!python/tuple`) is safe, so use it for all dumpers. +_CommonMixin.add_multi_representer(tuple, Dumper.yaml_representers[tuple]) +_CommonMixin.add_representer(tuple, Dumper.yaml_representers[tuple]) -OrderedDumper.add_representer( - "tag:yaml.org,2002:timestamp", OrderedDumper.represent_scalar -) -SafeOrderedDumper.add_representer( - "tag:yaml.org,2002:timestamp", SafeOrderedDumper.represent_scalar -) + +class SafeOrderedDumper(_CommonMixin, SafeDumper): + """A safe YAML dumper that uses the YAML ``!!omap`` type for ``OrderedDict`` + + ``OrderedDict``s are represented as a a sequence of single-entry mappings + and tagged with ``!!omap``: + + .. code-block:: yaml + + !!omap + - first key: first value + - second key: second value + + See https://yaml.org/type/omap.html for details. + """ + + +class OrderedDumper(_CommonMixin, Dumper): + """A YAML dumper that uses the YAML ``!!omap`` type for ``OrderedDict`` + + See ``SafeOrderedDumper`` for details. + """ + + +# This must inherit from yaml.SafeDumper, not yaml.CSafeDumper, because the +# increase_indent hack doesn't work with yaml.CSafeDumper. +# https://github.com/yaml/pyyaml/issues/234#issuecomment-786026671 +class IndentedSafeOrderedDumper(_CommonMixin, yaml.SafeDumper): + """Like ``SafeOrderedDumper``, except it indents lists for readability.""" + + def increase_indent(self, flow=False, indentless=False): + return super().increase_indent(flow, False) + + # TODO: Everything below this point is to provide backwards compatibility. + # It can be removed once support for yaml_compatibility=3006 is dropped. + + class _Legacy(SafeOrderedDumper): + def increase_indent(self, flow=False, indentless=False): + return super().increase_indent(flow, False) + + def __init__(self, *args, **kwargs): + super().__setattr__("_in_init", True) + try: + super().__init__(*args, **kwargs) + Legacy = super().__getattribute__("_Legacy") + super().__setattr__("_legacy", Legacy(*args, **kwargs)) + finally: + super().__setattr__("_in_init", False) + + def _use_legacy(self): + if super().__getattribute__("_in_init"): + return False + return _yaml_common.compat_ver() < SaltStackVersion(3007) + + def __getattribute__(self, name): + if super().__getattribute__("_use_legacy")(): + return getattr(super().__getattribute__("_legacy"), name) + return super().__getattribute__(name) + + def __setattr__(self, name, value): + if super().__getattribute__("_use_legacy")(): + return setattr(super().__getattribute__("_legacy"), name, value) + return super().__setattr__(name, value) + + def __delattr__(self, name): + if super().__getattribute__("_use_legacy")(): + return delattr(super().__getattribute__("_legacy"), name) + return super().__delattr__(name) def get_dumper(dumper_name): @@ -108,12 +243,19 @@ def dump(data, stream=None, **kwargs): """ .. versionadded:: 2018.3.0 + .. versionchanged:: 3007.0 + + The default ``Dumper`` class is now ``OrderedDumper`` instead of + ``yaml.Dumper``. Set the ``yaml_compatibility`` option to "3006" to + revert to the previous behavior. + Helper that wraps yaml.dump and ensures that we encode unicode strings unless explicitly told not to. """ - if "allow_unicode" not in kwargs: - kwargs["allow_unicode"] = True + kwargs.setdefault("allow_unicode", True) kwargs.setdefault("default_flow_style", None) + if "Dumper" not in kwargs and _yaml_common.compat_ver() >= SaltStackVersion(3007): + kwargs["Dumper"] = OrderedDumper return yaml.dump(data, stream, **kwargs) @@ -123,7 +265,4 @@ def safe_dump(data, stream=None, **kwargs): represented properly. Ensure that unicode strings are encoded unless explicitly told not to. """ - if "allow_unicode" not in kwargs: - kwargs["allow_unicode"] = True - kwargs.setdefault("default_flow_style", None) - return yaml.dump(data, stream, Dumper=SafeOrderedDumper, **kwargs) + return dump(data, stream, Dumper=SafeOrderedDumper, **kwargs) diff --git a/salt/utils/yamlloader.py b/salt/utils/yamlloader.py index 25b4b3bb9360..eb37c40599b8 100644 --- a/salt/utils/yamlloader.py +++ b/salt/utils/yamlloader.py @@ -3,11 +3,15 @@ """ +import collections + import yaml # pylint: disable=blacklisted-import from yaml.constructor import ConstructorError from yaml.nodes import MappingNode, SequenceNode +import salt.utils._yaml_common as _yaml_common import salt.utils.stringutils +from salt.utils.decorators import classproperty # prefer C bindings over python when available BaseLoader = getattr(yaml, "CSafeLoader", yaml.SafeLoader) @@ -16,35 +20,106 @@ __all__ = ["SaltYamlSafeLoader", "load", "safe_load"] +class _InheritedConstructorsMixin( + _yaml_common.InheritMapMixin, + inherit_map_attrs={ + # The ChainMap used for yaml_constructors is not saved directly to + # cls.yaml_constructors because _yaml_common.VersionedSubclassesMixin is + # used to automatically switch between versioned variants of the + # ChainMap, and we still need access to the unversioned ChainMap (for + # add_constructor(), and for subclasses to chain off of). + "_ctors": "yaml_constructors", + # Same goes for _mctors. + "_mctors": "yaml_multi_constructors", + }, +): + @classproperty + def yaml_constructors(cls): # pylint: disable=no-self-argument + return cls._ctors + + @classproperty + def yaml_multi_constructors(cls): # pylint: disable=no-self-argument + return cls._mctors + + @classmethod + def add_constructor(cls, tag, constructor): + cls._ctors[tag] = constructor + + @classmethod + def add_multi_constructor(cls, tag_prefix, multi_constructor): + cls._mctors[tag_prefix] = multi_constructor + + +class _VersionedConstructorsMixin( + _yaml_common.VersionedSubclassesMixin, + versioned_properties=( + "yaml_constructors", + "yaml_implicit_resolvers", + "yaml_multi_constructors", + ), +): + pass + + # with code integrated from https://gist.github.com/844388 -class SaltYamlSafeLoader(BaseLoader): +class SaltYamlSafeLoader( + _VersionedConstructorsMixin, + _InheritedConstructorsMixin, + BaseLoader, +): """ 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. """ + @classmethod + def remove_implicit_resolver(cls, tag): + """Remove a previously registered implicit resolver for a tag.""" + cls.yaml_implicit_resolvers = { + first_char: [r for r in resolver_list if r[0] != tag] + for first_char, resolver_list in cls.yaml_implicit_resolvers.items() + } + def __init__(self, stream, dictclass=dict): super().__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): + if self.dictclass is dict: + return (yield from super().construct_yaml_map(node)) data = self.dictclass() yield data value = self.construct_mapping(node) data.update(value) + def construct_yaml_omap_3006(self, node): + if self.dictclass is dict: + return (yield from super().construct_yaml_omap(node)) + return (yield from self.construct_yaml_map(node)) + + def construct_yaml_omap(self, node): + # BaseLoader.construct_yaml_omap() returns a list of (key, value) + # tuples, which doesn't match the semantics of the `!!omap` YAML type. + # Convert the list of tuples to an OrderedDict. + d = collections.OrderedDict() + yield d + (entries,) = super().construct_yaml_omap(node) + # All of the following lines could be replaced with `d.update(entries)`, + # but we want to detect and reject any duplicate keys in `entries`. + if hasattr(entries, "keys"): + entries = ((k, entries[k]) for k in entries.keys()) + for k, v in entries: + if k in d: + raise ConstructorError( + f"while constructing an ordered map", + node.start_mark, + f"duplicate key encountered: {k!r}", + # TODO: Can we get the location of the duplicate key? + node.start_mark, + ) + d[k] = v + def construct_unicode(self, node): return node.value @@ -86,6 +161,9 @@ def construct_mapping(self, node, deep=False): mapping[key] = value return mapping + def construct_python_tuple(self, node): + return tuple(self.construct_sequence(node)) + def construct_scalar(self, node): """ Verify integers and pass them in correctly is they are declared @@ -155,6 +233,36 @@ def flatten_mapping(self, node): node.value = mergeable_items + node.value +# The add_constructor() method is a class method, not an instance method, so +# custom constructors should be registered at class creation time, not instance +# creation time. +SaltYamlSafeLoader.add_constructor( + "tag:yaml.org,2002:map", SaltYamlSafeLoader.construct_yaml_map +) +SaltYamlSafeLoader.add_constructor( + "tag:yaml.org,2002:omap", SaltYamlSafeLoader.construct_yaml_omap +) +SaltYamlSafeLoader.V3006.add_constructor( + "tag:yaml.org,2002:omap", SaltYamlSafeLoader.construct_yaml_omap_3006 +) +SaltYamlSafeLoader.add_constructor( + "tag:yaml.org,2002:python/tuple", SaltYamlSafeLoader.construct_python_tuple +) +SaltYamlSafeLoader.add_constructor( + "tag:yaml.org,2002:python/unicode", SaltYamlSafeLoader.construct_unicode +) +SaltYamlSafeLoader.add_constructor( + "tag:yaml.org,2002:str", SaltYamlSafeLoader.construct_yaml_str +) +SaltYamlSafeLoader.V3006.add_constructor( + "tag:yaml.org,2002:timestamp", SaltYamlSafeLoader.construct_scalar +) + +# Require users to explicitly provide the `!!timestamp` tag if a datetime object +# is desired. +SaltYamlSafeLoader.V3007.remove_implicit_resolver("tag:yaml.org,2002:timestamp") + + def load(stream, Loader=SaltYamlSafeLoader): return yaml.load(stream, Loader=Loader) diff --git a/tests/conftest.py b/tests/conftest.py index d0672f95d779..5299a786c0ee 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -73,7 +73,12 @@ os.environ["COVERAGE_PROCESS_START"] = str(COVERAGERC_FILE) # Define the pytest plugins we rely on -pytest_plugins = ["tempdir", "helpers_namespace"] +pytest_plugins = [ + "tempdir", + "helpers_namespace", + # Suppress YAML compatibility warnings by default. + "tests.support.pytest.yaml", +] # Define where not to collect tests from collect_ignore = ["setup.py"] diff --git a/tests/pytests/integration/pillar/test_pillar_map_order.py b/tests/pytests/integration/pillar/test_pillar_map_order.py new file mode 100644 index 000000000000..b3ef96163387 --- /dev/null +++ b/tests/pytests/integration/pillar/test_pillar_map_order.py @@ -0,0 +1,142 @@ +import random +import textwrap + +import pytest + +pytestmark = [ + pytest.mark.slow_test, +] + +# TODO: Remove the salt_master, salt_cli, and salt_minion fixtures below after +# 3006 is released to revert to the same-named fixtures provided in conftest.py. +# (Leaving these won't hurt, but it will no longer be necessary). + + +@pytest.fixture(scope="module") +def salt_master(salt_factories): + m = salt_factories.salt_master_daemon( + "master-test_pillar_map_order", + overrides={"yaml_compatibility": 3007}, + ) + with m.started(): + yield m + + +@pytest.fixture(scope="module") +def salt_cli(salt_master): + assert salt_master.is_running() + return salt_master.salt_cli() + + +@pytest.fixture(scope="module") +def salt_minion(salt_master): + m = salt_master.salt_minion_daemon( + "minion-test_pillar_map_order", + overrides={"yaml_compatibility": 3007}, + ) + with m.started(): + yield m + + +@pytest.fixture(scope="module") +def minion_run(salt_minion, salt_cli): + """Convenience fixture that runs the ``salt`` CLI targeting the minion.""" + + def _run(*args, minion_tgt=salt_minion.id, **kwargs): + ret = salt_cli.run(*args, minion_tgt=minion_tgt, **kwargs) + assert ret.returncode == 0 + return ret.data + + yield _run + + +@pytest.mark.parametrize("omap", [False, True]) +def test_pillar_map_order(salt_master, minion_run, omap, yaml_compatibility): + """Test iteration order of YAML map entries in a Pillar ``.sls`` file. + + This test generates a Pillar ``.sls`` file containing either an ordinary + YAML map or a YAML `!!omap` and tests whether the resulting Python object + preserves iteration order. Random keys are used to ensure that iteration + order does not coincidentally match. Depending on the `omap` parameter, the + generated Pillar YAML file looks like this: + + .. code-block:: yaml + + data: + k3334244338: 0 + k3444116829: 1 + k2072366017: 2 + # ... omitted for brevity ... + k1638299831: 19 + + or like this: + + .. code-block:: yaml + + data: !!omap + - k3334244338: 0 + - k3444116829: 1 + - k2072366017: 2 + # ... omitted for brevity ... + - k1638299831: 19 + + A jinja template iterates over the entries in the resulting object to ensure + that iteration order is preserved. The expected output looks like: + + .. code-block:: text + + k3334244338 0 + k3444116829 1 + k2072366017 2 + ... omitted for brevity ... + k1638299831 19 + + Note: Python 3.6 switched to a new ``dict`` implementation that iterates in + insertion order. This behavior was made an official part of the ``dict`` + API in Python 3.7: + + * https://docs.python.org/3.6/whatsnew/3.6.html#new-dict-implementation + * https://mail.python.org/pipermail/python-dev/2017-December/151283.html + * https://docs.python.org/3.7/whatsnew/3.7.html + + Thus, this test may fail on Python 3.5 and older. However, Salt currently + requires a newer version of Python, so this should not be a problem. + + The non-``!!omap`` case is a regression test for: + https://github.com/saltstack/salt/issues/12161 + """ + # Filter the random keys through a set to avoid duplicates. + keys = list({f"k{random.getrandbits(32)}" for _ in range(20)}) + # Avoid unintended correlation with set()'s iteration order. + random.shuffle(keys) + items = [(k, i) for i, k in enumerate(keys)] + top_yaml = "base: {'*': [data]}\n" + top_sls = salt_master.pillar_tree.base.temp_file("top.sls", top_yaml) + data_yaml = "data:" + if omap: + data_yaml += " !!omap\n" + "".join(f" - {k}: {v}\n" for k, v in items) + else: + data_yaml += "\n" + "".join(f" {k}: {v}\n" for k, v in items) + data_sls = salt_master.pillar_tree.base.temp_file("data.sls", data_yaml) + tmpl_jinja = textwrap.dedent( + """\ + {%- for k, v in pillar['data'].items() %} + {{ k }} {{ v }} + {%- endfor %} + """ + ) + want = "\n" + "".join(f"{k} {v}\n" for k, v in items) + try: + with top_sls, data_sls: + assert minion_run("saltutil.refresh_pillar", wait=True) is True + got = minion_run( + "file.apply_template_on_contents", + tmpl_jinja, + template="jinja", + context={}, + defaults={}, + saltenv="base", + ) + assert got == want + finally: + assert minion_run("saltutil.refresh_pillar", wait=True) is True diff --git a/tests/pytests/unit/modules/test_schedule.py b/tests/pytests/unit/modules/test_schedule.py index d3488559143f..ac0fb1a1388d 100644 --- a/tests/pytests/unit/modules/test_schedule.py +++ b/tests/pytests/unit/modules/test_schedule.py @@ -9,6 +9,7 @@ import salt.modules.schedule as schedule import salt.utils.odict +import salt.utils.yaml from salt.utils.event import SaltEvent from salt.utils.odict import OrderedDict from tests.support.mock import MagicMock, call, mock_open, patch @@ -278,7 +279,22 @@ def test_add(): ) == {"comment": comm1, "changes": changes1, "result": True} _call = call( - b"schedule:\n job3: {function: test.ping, seconds: 3600, maxrunning: 1, name: job3, enabled: true,\n jid_include: true}\n" + salt.utils.yaml.safe_dump( + { + "schedule": { + "job3": OrderedDict( + [ + ("function", "test.ping"), + ("seconds", 3600), + ("maxrunning", 1), + ("name", "job3"), + ("enabled", True), + ("jid_include", True), + ], + ), + }, + } + ).encode() ) write_calls = fopen_mock.filehandles[schedule_config_file][ 1 diff --git a/tests/pytests/unit/modules/test_seed.py b/tests/pytests/unit/modules/test_seed.py index ee51cdc3a5cd..360e10dff3b2 100644 --- a/tests/pytests/unit/modules/test_seed.py +++ b/tests/pytests/unit/modules/test_seed.py @@ -11,6 +11,7 @@ import salt.modules.seed as seed import salt.utils.files import salt.utils.odict +import salt.utils.yaml from tests.support.mock import MagicMock, patch @@ -28,7 +29,7 @@ def test_mkconfig_odict(): data = seed.mkconfig(ddd, approve_key=False) with salt.utils.files.fopen(data["config"]) as fic: fdata = fic.read() - assert fdata == "b: b\na: b\nmaster: foo\n" + assert fdata == salt.utils.yaml.safe_dump(ddd, default_flow_style=False) def test_prep_bootstrap(): diff --git a/tests/pytests/unit/utils/jinja/test_custom_extensions.py b/tests/pytests/unit/utils/jinja/test_custom_extensions.py index 4d004230fcbb..fbb26c6238c4 100644 --- a/tests/pytests/unit/utils/jinja/test_custom_extensions.py +++ b/tests/pytests/unit/utils/jinja/test_custom_extensions.py @@ -3,6 +3,7 @@ """ import ast +import collections import itertools import os import pprint @@ -129,10 +130,13 @@ def test_serialize_yaml(): "baz": [1, 2, 3], "qux": 2.0, "spam": OrderedDict([("foo", OrderedDict([("bar", "baz"), ("qux", 42)]))]), + "tuple": ("foo", "bar"), } env = Environment(extensions=[SerializerExtension]) rendered = env.from_string("{{ dataset|yaml }}").render(dataset=dataset) assert dataset == salt.utils.yaml.safe_load(rendered) + assert isinstance(dataset["spam"], collections.OrderedDict) + assert isinstance(dataset["spam"]["foo"], collections.OrderedDict) def test_serialize_yaml_str(): diff --git a/tests/pytests/unit/utils/test_decorators.py b/tests/pytests/unit/utils/test_decorators.py new file mode 100644 index 000000000000..f45229810137 --- /dev/null +++ b/tests/pytests/unit/utils/test_decorators.py @@ -0,0 +1,53 @@ +import pytest + +import salt.utils.decorators + + +def test_classproperty(): + class Foo: + @salt.utils.decorators.classproperty + def prop(cls): # pylint: disable=no-self-argument + return cls + + assert Foo.prop is Foo + assert Foo().prop is Foo + + +def test_classproperty_getter(): + class Foo: + @salt.utils.decorators.classproperty + def prop(cls): # pylint: disable=no-self-argument + return "old" + + @prop.getter + def prop(cls): # pylint: disable=no-self-argument + return "new" + + assert Foo.prop == "new" + assert Foo().prop == "new" + + +def test_classproperty_setter(): + with pytest.raises(NotImplementedError): + + class Foo: + @salt.utils.decorators.classproperty + def prop(cls): # pylint: disable=no-self-argument + return "getter" + + @prop.setter + def prop(cls, val): # pylint: disable=no-self-argument + return "setter" + + +def test_classproperty_deleter(): + with pytest.raises(NotImplementedError): + + class Foo: + @salt.utils.decorators.classproperty + def prop(cls): # pylint: disable=no-self-argument + return "getter" + + @prop.deleter + def prop(cls): # pylint: disable=no-self-argument + return "deleter" diff --git a/tests/pytests/unit/utils/test_yaml.py b/tests/pytests/unit/utils/test_yaml.py new file mode 100644 index 000000000000..59824b831d7a --- /dev/null +++ b/tests/pytests/unit/utils/test_yaml.py @@ -0,0 +1,597 @@ +import collections +import datetime +import random +import re +import textwrap +import warnings + +import pytest +import yaml +from yaml.constructor import ConstructorError + +import salt.utils._yaml_common as _yc +import salt.utils.files +import salt.utils.yaml as salt_yaml +from salt.utils.odict import OrderedDict +from salt.version import SaltStackVersion +from tests.support.mock import mock_open, patch + + +class _OrderedDictLoader(salt_yaml.SaltYamlSafeLoader): + def __init__(self, stream): + super().__init__(stream, dictclass=collections.OrderedDict) + + +@pytest.mark.parametrize( + "yaml_compatibility,want", + [ + (None, SaltStackVersion(3006)), + (3005, SaltStackVersion(3006)), + (3006, SaltStackVersion(3006)), + ("3006", SaltStackVersion(3006)), + ("3006.0", SaltStackVersion(3006)), + ("v3006.0", SaltStackVersion(3006)), + ("sulfur", SaltStackVersion(3006)), + ("Sulfur", SaltStackVersion(3006)), + ("SULFUR", SaltStackVersion(3006)), + (3007, SaltStackVersion(3007)), + ], + indirect=["yaml_compatibility"], +) +def test_compat_ver(yaml_compatibility, want): + with warnings.catch_warnings(): + warnings.filterwarnings( + "ignore", category=_yc.UnsupportedValueWarning, module=_yc.__name__ + ) + got = _yc.compat_ver() + assert got == want + + +@pytest.mark.show_yaml_compatibility_warnings +@pytest.mark.parametrize( + "yaml_compatibility,want", + [ + (None, [(FutureWarning, r"behavior will change in version 3007(?:\.0)?")]), + ( + 3005, + [ + (FutureWarning, r"less than 3007(?:\.0)? will be removed in Salt 3011(?:\.0)?"), + (_yc.UnsupportedValueWarning, r"minimum supported value 3006(?:\.0)?"), + (_yc.OverrideNotice, r"3006(?:\.0)?"), + ], + ), + ( + 3006, + [ + (FutureWarning, r"less than 3007(?:\.0)? will be removed in Salt 3011(?:\.0)?"), + (_yc.OverrideNotice, r"3006(?:\.0)?"), + ], + ), + (3007, [(_yc.OverrideNotice, r"3007(?:\.0)?")]), + ], + indirect=["yaml_compatibility"], +) +def test_compat_ver_warnings(yaml_compatibility, want): + with warnings.catch_warnings(record=True) as got: + _yc.compat_ver() + for category, regexp in want: + found = False + for warn in got: + if warn.category is not category: + continue + if not re.search(regexp, str(warn.message)): + continue + found = True + break + assert found, f"no {category.__name__} warning with message matching {regexp!r}; all warnings: {[warn.message for warn in got]!r}" + assert len(got) == len(want) + + +def test_dump(): + data = {"foo": "bar"} + assert salt_yaml.dump(data) == "{foo: bar}\n" + assert salt_yaml.dump(data, default_flow_style=False) == "foo: bar\n" + + +def test_safe_dump(): + data = {"foo": "bar"} + assert salt_yaml.safe_dump(data) == "{foo: bar}\n" + assert salt_yaml.safe_dump(data, default_flow_style=False) == "foo: bar\n" + + +@pytest.mark.parametrize( + "yaml_compatibility,input_dumper,want_dumper", + [ + (3006, None, None), + (3006, yaml.Dumper, yaml.Dumper), + (3007, None, salt_yaml.OrderedDumper), + (3007, yaml.Dumper, yaml.Dumper), + ], + indirect=["yaml_compatibility"], +) +def test_dump_default_dumper(yaml_compatibility, input_dumper, want_dumper): + with patch.object(yaml, "dump") as mock: + kwargs = {} + if input_dumper is not None: + kwargs["Dumper"] = input_dumper + salt_yaml.dump([], **kwargs) + mock.assert_called_once() + got_kwargs = mock.mock_calls[0].kwargs + if want_dumper is None: + assert "Dumper" not in got_kwargs + else: + assert got_kwargs["Dumper"] is want_dumper + + +@pytest.mark.parametrize( + "yaml_compatibility,want", + [ + # With v3006, sometimes it indents and sometimes it doesn't depending on + # whether yaml.CSafeDumper exists on the system. + (3006, re.compile(r"foo:\n(?: )?- bar\n")), + (3007, "foo:\n - bar\n"), + ], + indirect=["yaml_compatibility"], +) +def test_dump_indented(yaml_compatibility, want): + data = {"foo": ["bar"]} + got = salt_yaml.dump( + data, + Dumper=salt_yaml.IndentedSafeOrderedDumper, + default_flow_style=False, + ) + try: + assert want.fullmatch(got) + except AttributeError: + assert got == want + + +@pytest.mark.parametrize("yaml_compatibility", [3006, 3007], indirect=True) +@pytest.mark.parametrize("dictcls", [OrderedDict, collections.OrderedDict]) +@pytest.mark.parametrize( + "dumpercls", + [ + salt_yaml.OrderedDumper, + salt_yaml.SafeOrderedDumper, + salt_yaml.IndentedSafeOrderedDumper, + ], +) +def test_dump_omap(yaml_compatibility, dictcls, dumpercls): + # The random keys are filtered through a set to avoid duplicates. + keys = list({f"random key {random.getrandbits(32)}" for _ in range(20)}) + # Avoid unintended correlation with set()'s iteration order. + random.shuffle(keys) + items = [(k, i) for i, k in enumerate(keys)] + d = dictcls(items) + if yaml_compatibility == 3006 and dictcls is collections.OrderedDict: + # Buggy behavior preserved for backwards compatibility. + if dumpercls is salt_yaml.OrderedDumper: + want = ( + "!!python/object/apply:collections.OrderedDict\n-" + + "".join(f" - - {k}\n - {v}\n" for k, v in items)[1:] + ) + else: + # With v3006, sometimes it prints "..." on a new line as an end of + # document indicator depending on whether yaml.CSafeDumper is + # available on the system or not (yaml.CSafeDumper and + # yaml.SafeDumper do not always behave the same). + want = re.compile(r"NULL\n(?:\.\.\.\n)?") + elif yaml_compatibility == 3006: + want = "".join(f"{k}: {v}\n" for k, v in items) + else: + # Note that there is no extra indentation added for the + # IndentedSafeOrderedDumper case because the !!omap node is the + # top-level node so there is no indentation for the sequence elements. + want = "!!omap\n" + "".join(f"- {k}: {v}\n" for k, v in items) + got = salt_yaml.dump(d, Dumper=dumpercls, default_flow_style=False) + try: + assert want.fullmatch(got) + except AttributeError: + assert got == want + + +@pytest.mark.parametrize( + "dumpercls", + [ + salt_yaml.OrderedDumper, + salt_yaml.SafeOrderedDumper, + salt_yaml.IndentedSafeOrderedDumper, + ], +) +@pytest.mark.parametrize( + "yaml_compatibility,want_tag", + [(3006, False), (3007, True)], + indirect=["yaml_compatibility"], +) +def test_dump_timestamp(yaml_compatibility, want_tag, dumpercls): + dt = datetime.datetime( + *(2022, 10, 21, 18, 16, 3, 100000), + tzinfo=datetime.timezone(datetime.timedelta(hours=-4)), + ) + got = salt_yaml.dump(dt, Dumper=dumpercls) + want_re = r"""(['"]?)2022-10-21[T ]18:16:03.10*-04:00\1\n(?:\.\.\.\n)?""" + if want_tag: + want_re = f"!!timestamp {want_re}" + assert re.fullmatch(want_re, got) + + +@pytest.mark.parametrize( + "mktuple", + [ + lambda *args: tuple(args), + collections.namedtuple("TestTuple", "a b"), + ], +) +@pytest.mark.parametrize( + "dumpercls", + [ + salt_yaml.OrderedDumper, + salt_yaml.SafeOrderedDumper, + salt_yaml.IndentedSafeOrderedDumper, + ], +) +def test_dump_tuple(mktuple, dumpercls): + data = mktuple("foo", "bar") + want = "!!python/tuple [foo, bar]\n" + got = salt_yaml.dump(data, Dumper=dumpercls) + assert got == want + + +@pytest.mark.parametrize("obj", [type("Generated", (), {})()]) +@pytest.mark.parametrize( + "yaml_compatibility,dumpercls,want", + [ + ( + 3006, + salt_yaml.OrderedDumper, + "!!python/object:tests.pytests.unit.utils.test_yaml.Generated {}\n", + ), + # With v3006, sometimes it prints "..." on a new line as an end of + # document indicator depending on whether yaml.CSafeDumper is available + # on the system or not (yaml.CSafeDumper and yaml.SafeDumper do not + # always behave the same). + (3006, salt_yaml.SafeOrderedDumper, re.compile(r"NULL\n(?:\.\.\.\n)?")), + (3006, salt_yaml.IndentedSafeOrderedDumper, re.compile(r"NULL\n(?:\.\.\.\n)?")), + ( + 3007, + salt_yaml.OrderedDumper, + "!!python/object:tests.pytests.unit.utils.test_yaml.Generated {}\n", + ), + (3007, salt_yaml.SafeOrderedDumper, yaml.representer.RepresenterError), + (3007, salt_yaml.IndentedSafeOrderedDumper, yaml.representer.RepresenterError), + ], + indirect=["yaml_compatibility"], +) +def test_dump_unsupported(yaml_compatibility, dumpercls, obj, want): + if isinstance(want, type) and issubclass(want, Exception): + with pytest.raises(want): + salt_yaml.dump(obj, Dumper=dumpercls) + else: + got = salt_yaml.dump(obj, Dumper=dumpercls) + try: + assert want.fullmatch(got) + except AttributeError: + assert got == want + + +def render_yaml(data): + """ + Takes a YAML string, puts it into a mock file, passes that to the YAML + SaltYamlSafeLoader and then returns the rendered/parsed YAML data + """ + with patch("salt.utils.files.fopen", mock_open(read_data=data)) as mocked_file: + with salt.utils.files.fopen(mocked_file) as mocked_stream: + return salt_yaml.SaltYamlSafeLoader(mocked_stream).get_data() + + +def test_load_basics(): + """ + Test parsing an ordinary path + """ + assert ( + render_yaml( + textwrap.dedent( + """\ + p1: + - alpha + - beta + """ + ) + ) + == {"p1": ["alpha", "beta"]} + ) + + +def test_load_merge(): + """ + Test YAML anchors + """ + # Simple merge test + assert ( + render_yaml( + textwrap.dedent( + """\ + p1: &p1 + v1: alpha + p2: + <<: *p1 + v2: beta + """ + ) + ) + == {"p1": {"v1": "alpha"}, "p2": {"v1": "alpha", "v2": "beta"}} + ) + + # Test that keys/nodes are overwritten + assert ( + render_yaml( + textwrap.dedent( + """\ + p1: &p1 + v1: alpha + p2: + <<: *p1 + v1: new_alpha + """ + ) + ) + == {"p1": {"v1": "alpha"}, "p2": {"v1": "new_alpha"}} + ) + + # Test merging of lists + assert ( + render_yaml( + textwrap.dedent( + """\ + p1: &p1 + v1: &v1 + - t1 + - t2 + p2: + v2: *v1 + """ + ) + ) + == {"p2": {"v2": ["t1", "t2"]}, "p1": {"v1": ["t1", "t2"]}} + ) + + +def test_load_duplicates(): + """ + Test that duplicates still throw an error + """ + with pytest.raises(ConstructorError): + render_yaml( + textwrap.dedent( + """\ + p1: alpha + p1: beta + """ + ) + ) + + with pytest.raises(ConstructorError): + render_yaml( + textwrap.dedent( + """\ + p1: &p1 + v1: alpha + p2: + <<: *p1 + v2: beta + v2: betabeta + """ + ) + ) + + +def test_load_with_plain_scalars(): + """ + Test that plain (i.e. unqoted) string and non-string scalars are + properly handled + """ + assert ( + render_yaml( + textwrap.dedent( + """\ + foo: + b: {foo: bar, one: 1, list: [1, two, 3]} + """ + ) + ) + == {"foo": {"b": {"foo": "bar", "one": 1, "list": [1, "two", 3]}}} + ) + + +@pytest.mark.parametrize("dictclass", [dict, collections.OrderedDict]) +def test_load_dictclass(dictclass): + l = salt_yaml.SaltYamlSafeLoader("k1: v1\nk2: v2\n", dictclass=dictclass) + try: + d = l.get_single_data() + finally: + l.dispose() + assert isinstance(d, dictclass) + assert d == dictclass([("k1", "v1"), ("k2", "v2")]) + + +@pytest.mark.parametrize( + # Parameters: + # - yaml_compatibility: Force the YAML loader to be compatible with this + # version of Salt. + # - seq_input: Boolean. True if the input YAML node should be a sequence + # of single-entry mappings, False if it should be a mapping. + # - Loader: YAML Loader class. + # - wantclass: Expected return type. + "yaml_compatibility,seq_input,Loader,wantclass", + [ + # Salt v3006 and earlier required !!omap nodes to be mapping nodes if + # the SaltYamlSafeLoader dictclass argument is not dict. To preserve + # compatibility, that erroneous behavior is preserved if + # yaml_compatibility is set to 3006. + (3006, False, _OrderedDictLoader, collections.OrderedDict), + # However, with dictclass=dict (the default), an !!omap node was + # correctly required to be a sequence of mapping nodes. Unfortunately, + # the return value was not a Mapping type -- it was a list of (key, + # value) tuples (PyYAML's default behavior for !!omap nodes). + (3006, True, None, list), + # Starting with Salt v3007, an !!omap node is always required to be a + # sequence of mapping nodes, and always returns an OrderedDict. + (3007, True, _OrderedDictLoader, collections.OrderedDict), + (3007, True, None, collections.OrderedDict), + ], + indirect=["yaml_compatibility"], +) +def test_load_omap(yaml_compatibility, seq_input, Loader, wantclass): + """Test loading of `!!omap` YAML nodes. + + This test uses random keys to ensure that iteration order does not + coincidentally match. The generated items look like this: + + .. code-block:: python + + [ + ("k3334244338", 0), + ("k3444116829", 1), + ("k2072366017", 2), + # ... omitted for brevity ... + ("k1638299831", 19), + ] + """ + # Filter the random keys through a set to avoid duplicates. + keys = list({f"k{random.getrandbits(32)}" for _ in range(20)}) + # Avoid unintended correlation with set()'s iteration order. + random.shuffle(keys) + items = [(k, i) for i, k in enumerate(keys)] + input_yaml = "!!omap\n" + if seq_input: + input_yaml += "".join(f"- {k}: {v}\n" for k, v in items) + else: + input_yaml += "".join(f"{k}: {v}\n" for k, v in items) + kwargs = {} + if Loader is not None: + kwargs["Loader"] = Loader + got = salt_yaml.load(input_yaml, **kwargs) + assert isinstance(got, wantclass) + if isinstance(got, list): + assert got == items + else: + assert got == collections.OrderedDict(items) + assert list(got.items()) == items + + +@pytest.mark.parametrize( + "yaml_compatibility,input_yaml,Loader,want", + [ + # See comments in test_load_omap() above for the differences in !!omap + # loading behavior between Salt v3006 and v3007. + (3006, "!!omap {}\n", _OrderedDictLoader, collections.OrderedDict()), + (3006, "!!omap []\n", None, []), + (3007, "!!omap []\n", _OrderedDictLoader, collections.OrderedDict()), + (3007, "!!omap []\n", None, collections.OrderedDict()), + ], + indirect=["yaml_compatibility"], +) +def test_load_omap_empty(yaml_compatibility, input_yaml, Loader, want): + kwargs = {} + if Loader is not None: + kwargs["Loader"] = Loader + got = salt_yaml.load(input_yaml, **kwargs) + assert isinstance(got, type(want)) + assert got == want + + +@pytest.mark.parametrize( + "yaml_compatibility,input_yaml,Loader", + [ + # Buggy v3006 behavior kept for compatibility. See comments in + # test_load_omap() above for details. + (3006, "!!omap []\n", _OrderedDictLoader), # Not a mapping node. + (3006, "!!omap\ndup key: 0\ndup key: 1\n", _OrderedDictLoader), + # Invald because the !!omap node is not a sequence node. + (3006, "!!omap {}\n", None), + (3007, "!!omap {}\n", _OrderedDictLoader), + (3007, "!!omap {}\n", None), + # Invalid because a sequence entry is not a mapping node. + (3006, "!!omap\n- this is a str not a map\n", None), + (3007, "!!omap\n- this is a str not a map\n", _OrderedDictLoader), + (3007, "!!omap\n- this is a str not a map\n", None), + # Invalid because a sequence entry's mapping has multiple entries. + (3006, "!!omap\n- k1: v\n k2: v\n", None), + (3007, "!!omap\n- k1: v\n k2: v\n", _OrderedDictLoader), + (3007, "!!omap\n- k1: v\n k2: v\n", None), + # Invalid because a sequence entry's mapping has no entries. + (3006, "!!omap [{}]\n", None), + (3007, "!!omap [{}]\n", _OrderedDictLoader), + (3007, "!!omap [{}]\n", None), + # Invalid because there are duplicate keys. Note that the Loader=None + # case for v3006 is missing here; this is because the default v3006 + # Loader matches PyYAML's behavior, and PyYAML permits duplicate keys in + # !!omap nodes. + (3007, "!!omap\n- dup key: 0\n- dup key: 1\n", _OrderedDictLoader), + (3007, "!!omap\n- dup key: 0\n- dup key: 1\n", None), + ], + indirect=["yaml_compatibility"], +) +def test_load_omap_invalid(yaml_compatibility, input_yaml, Loader): + kwargs = {} + if Loader is not None: + kwargs["Loader"] = Loader + with pytest.raises(ConstructorError): + salt_yaml.load(input_yaml, **kwargs) + + +@pytest.mark.parametrize("yaml_compatibility", [3006, 3007], indirect=True) +@pytest.mark.parametrize("Loader", [_OrderedDictLoader, None]) +def test_load_untagged_omaplike_is_seq(yaml_compatibility, Loader): + # The YAML spec allows the loader to interpret something that looks like an + # !!omap but doesn't actually have an !!omap tag as an !!omap. (If the user + # intends to express a sequence of single-entry maps and not an ordered map, + # the user must explicitly tag the sequence node with !seq.) Out of concern + # for backwards compatibility, and to avoid ambiguity with an empty + # sequence, implicit !!omap behavior is currently not supported. That may + # change in the future, but for now make sure that sequences are not + # interpreted as ordered maps. + kwargs = {} + if Loader is not None: + kwargs["Loader"] = Loader + got = salt_yaml.load("- a: 0\n- b: 1\n", **kwargs) + assert not isinstance(got, collections.OrderedDict) + assert got == [{"a": 0}, {"b": 1}] + + +@pytest.mark.parametrize( + "yaml_compatibility,input_yaml,want", + [ + ( + 3006, + "!!timestamp 2022-10-21T18:16:03.1-04:00", + "2022-10-21T18:16:03.1-04:00", + ), + ( + 3007, + "!!timestamp 2022-10-21T18:16:03.1-04:00", + datetime.datetime( + *(2022, 10, 21, 18, 16, 3, 100000), + tzinfo=datetime.timezone(datetime.timedelta(hours=-4)), + ), + ), + (3006, "2022-10-21T18:16:03.1-04:00", "2022-10-21T18:16:03.1-04:00"), + (3007, "2022-10-21T18:16:03.1-04:00", "2022-10-21T18:16:03.1-04:00"), + ], + indirect=["yaml_compatibility"], +) +def test_load_timestamp(yaml_compatibility, input_yaml, want): + got = salt_yaml.load(input_yaml) + assert got == want + + +def test_load_tuple(): + input = "!!python/tuple\n- foo\n- bar\n" + got = salt_yaml.load(input) + want = ("foo", "bar") + assert got == want + + +def test_not_yaml_monkey_patching(): + if hasattr(yaml, "CSafeLoader"): + assert yaml.SafeLoader != yaml.CSafeLoader diff --git a/tests/support/pytest/yaml.py b/tests/support/pytest/yaml.py new file mode 100644 index 000000000000..a979e47fe7db --- /dev/null +++ b/tests/support/pytest/yaml.py @@ -0,0 +1,99 @@ +import logging +import warnings + +import pytest + +import salt.utils._yaml_common as _yc +from tests.support.mock import patch + +log = logging.getLogger(__name__) + + +def pytest_configure(config): + config.addinivalue_line("markers", "show_yaml_compatibility_warnings") + + +@pytest.fixture(autouse=True) +def suppress_yaml_compatibility_warnings(request): + """Silence the warnings produced by certain ``yaml_compatibility`` values. + + This is an autouse fixture, so warnings are always suppressed by default + assuming this module is loaded. To un-suppress the warnings, you can either + redefine this fixture to be a no-op: + + .. code-block:: python + + @pytest.fixture + def suppress_yaml_compatibility_warnings(): + pass + + or you can mark your tests with ``show_yaml_compatibility_warnings``: + + .. code-block:: python + + @pytest.mark.show_yaml_compatibility_warnings + @pytest.mark.parametrize("yaml_compatibility", [3006], indirect=True) + def test_yaml_compatibility_warning_3006(yaml_compatibility): + with pytest.warns(FutureWarning): + salt.utils.yaml.load("{}") + + """ + if "show_yaml_compatibility_warnings" in request.keywords: + yield + return + log.debug("suppressing YAML compatibility warnings") + with warnings.catch_warnings(): + # Don't suppress UnsupportedValueWarning in _yc.compat_ver() -- that + # warning means that a test needs to be updated. + for category in [FutureWarning, _yc.OverrideNotice]: + warnings.filterwarnings("ignore", category=category, module=_yc.__name__) + yield + + +@pytest.fixture +def clear_internal_compat_ver_state(): + """Reprocess ``yaml_compatibility`` next time ``compat_ver`` is called. + + This temporarily clears the internal state containing the processed + ``yaml_compatibility`` version, forcing the next ``compat_ver`` call to + reprocess the ``yaml_compatibility`` option. + + """ + # It would be nice if contextvars.copy_context() could be used as a context + # manager, but alas we have to individually back up the context variables + # and restore them later. https://github.com/python/cpython/issues/99633 + opt_tok = _yc._compat_opt.set(None) + ver_tok = _yc._compat_ver.set(None) + yield + _yc._compat_ver.reset(ver_tok) + _yc._compat_opt.reset(opt_tok) + + +@pytest.fixture +def yaml_compatibility( + request, suppress_yaml_compatibility_warnings, clear_internal_compat_ver_state +): + """Temporarily override the ``yaml_compatibility`` option. + + To specify the desired ``yaml_compatibility`` value, use indirect + parametrization: + + .. code-block:: python + + @pytest.mark.parametrize("yaml_compatibility", [3006], indirect=True) + def test_foo(yaml_compatibility): + assert yaml_compatibility == 3006 + want = salt.version.SaltStackVersion(3006) + got = salt.utils._yaml_common.compat_ver() + assert got == want + + If a parameter is not given, ``yaml_compatibility`` behaves as if the + parameter was ``None``. + + """ + v = getattr(request, "param", None) + log.debug(f"setting yaml_compatibility to {v!r}") + _yc._init() # Ensure _yc.__opts__ is initialized before patching it. + with patch.dict(_yc.__opts__, {"yaml_compatibility": v}): + yield v + log.debug(f"restoring yaml_compatibility") diff --git a/tests/unit/utils/test_yamldumper.py b/tests/unit/utils/test_yamldumper.py deleted file mode 100644 index 9a1a6ab103ba..000000000000 --- a/tests/unit/utils/test_yamldumper.py +++ /dev/null @@ -1,37 +0,0 @@ -""" - Unit tests for salt.utils.yamldumper -""" - -import salt.utils.yamldumper -from tests.support.unit import TestCase - - -class YamlDumperTestCase(TestCase): - """ - TestCase for salt.utils.yamldumper module - """ - - def test_yaml_dump(self): - """ - Test yaml.dump a dict - """ - data = {"foo": "bar"} - exp_yaml = "{foo: bar}\n" - - assert salt.utils.yamldumper.dump(data) == exp_yaml - - assert salt.utils.yamldumper.dump( - data, default_flow_style=False - ) == exp_yaml.replace("{", "").replace("}", "") - - def test_yaml_safe_dump(self): - """ - Test yaml.safe_dump a dict - """ - data = {"foo": "bar"} - assert salt.utils.yamldumper.safe_dump(data) == "{foo: bar}\n" - - assert ( - salt.utils.yamldumper.safe_dump(data, default_flow_style=False) - == "foo: bar\n" - ) diff --git a/tests/unit/utils/test_yamlloader.py b/tests/unit/utils/test_yamlloader.py deleted file mode 100644 index 04370a97083c..000000000000 --- a/tests/unit/utils/test_yamlloader.py +++ /dev/null @@ -1,140 +0,0 @@ -""" - Unit tests for salt.utils.yamlloader.SaltYamlSafeLoader -""" - -import textwrap - -from yaml.constructor import ConstructorError - -import salt.utils.files -from salt.utils.yamlloader import SaltYamlSafeLoader, yaml -from tests.support.mock import mock_open, patch -from tests.support.unit import TestCase - - -class YamlLoaderTestCase(TestCase): - """ - TestCase for salt.utils.yamlloader module - """ - - @staticmethod - def render_yaml(data): - """ - Takes a YAML string, puts it into a mock file, passes that to the YAML - SaltYamlSafeLoader and then returns the rendered/parsed YAML data - """ - with patch("salt.utils.files.fopen", mock_open(read_data=data)) as mocked_file: - with salt.utils.files.fopen(mocked_file) as mocked_stream: - return SaltYamlSafeLoader(mocked_stream).get_data() - - def test_yaml_basics(self): - """ - Test parsing an ordinary path - """ - self.assertEqual( - self.render_yaml( - textwrap.dedent( - """\ - p1: - - alpha - - beta""" - ) - ), - {"p1": ["alpha", "beta"]}, - ) - - def test_yaml_merge(self): - """ - Test YAML anchors - """ - # Simple merge test - self.assertEqual( - self.render_yaml( - textwrap.dedent( - """\ - p1: &p1 - v1: alpha - p2: - <<: *p1 - v2: beta""" - ) - ), - {"p1": {"v1": "alpha"}, "p2": {"v1": "alpha", "v2": "beta"}}, - ) - - # Test that keys/nodes are overwritten - self.assertEqual( - self.render_yaml( - textwrap.dedent( - """\ - p1: &p1 - v1: alpha - p2: - <<: *p1 - v1: new_alpha""" - ) - ), - {"p1": {"v1": "alpha"}, "p2": {"v1": "new_alpha"}}, - ) - - # Test merging of lists - self.assertEqual( - self.render_yaml( - textwrap.dedent( - """\ - p1: &p1 - v1: &v1 - - t1 - - t2 - p2: - v2: *v1""" - ) - ), - {"p2": {"v2": ["t1", "t2"]}, "p1": {"v1": ["t1", "t2"]}}, - ) - - def test_yaml_duplicates(self): - """ - Test that duplicates still throw an error - """ - with self.assertRaises(ConstructorError): - self.render_yaml( - textwrap.dedent( - """\ - p1: alpha - p1: beta""" - ) - ) - - with self.assertRaises(ConstructorError): - self.render_yaml( - textwrap.dedent( - """\ - p1: &p1 - v1: alpha - p2: - <<: *p1 - v2: beta - v2: betabeta""" - ) - ) - - def test_yaml_with_plain_scalars(self): - """ - Test that plain (i.e. unqoted) string and non-string scalars are - properly handled - """ - self.assertEqual( - self.render_yaml( - textwrap.dedent( - """\ - foo: - b: {foo: bar, one: 1, list: [1, two, 3]}""" - ) - ), - {"foo": {"b": {"foo": "bar", "one": 1, "list": [1, "two", 3]}}}, - ) - - def test_not_yaml_monkey_patching(self): - if hasattr(yaml, "CSafeLoader"): - assert yaml.SafeLoader != yaml.CSafeLoader