diff --git a/changelog/62791.fixed b/changelog/62791.fixed new file mode 100644 index 000000000000..4b7bada96fd6 --- /dev/null +++ b/changelog/62791.fixed @@ -0,0 +1,12 @@ +Improvements to `salt.state.ldap` and `salt.modules.ldap3`: + * The order of an attribute's values is now preserved when reading from and + writing to the LDAP server. + * The order of an attribute's values is now ignored when checking value + equality, as required by RFC 4511. + * Fixed attribute value encoding/decoding corner cases. + * Worked around a flaw in the way `python-ldap` adds new attribute values (the + flaw only affects entries where attribute value changes can trigger server + behavior changes, such as some of OpenLDAP's `cn=config` entries). + * Fixed support for `bytes` attribute values that are not in a list. + * Documentation improvements. + * Miscellaneous code cleanups. diff --git a/changelog/62932.fixed b/changelog/62932.fixed new file mode 100644 index 000000000000..7740357c4c61 --- /dev/null +++ b/changelog/62932.fixed @@ -0,0 +1,18 @@ +Improvements to YAML processing (`salt.utils.yaml`): + * Loading an `!!omap` node now always returns a `collections.OrderedMap` + object. Before it would sometimes return a list of (key, value) tuples + and sometimes raise an exception. + * Dumping a `collections.OrderedMap` now consistently produces an `!!omap` + node (a sequence of single-entry mappings). + * Loading a sequence node tagged with `!!python/tuple` is now supported, and + produces a Python `tuple` object. + * Dumping a `tuple` object now consistently produces a sequence node + explicitly tagged with `!!python/tuple`. + * Loading an explicitly tagged `!!timestamp` node now produces a + `datetime.datetime` object instead of a string. + * Dumping a `datetime.datetime` object now explicitly tags the node with + `!!timestamp`. + * `salt.utils.yaml.dump()` now defaults to `salt.utils.yaml.OrderedDumper` + instead of `yaml.Dumper`. + * Fixed indentation in `salt.utils.yaml.IndentedSafeOrderedDumper` output. + * Code health clean-ups. diff --git a/doc/topics/troubleshooting/yaml_idiosyncrasies.rst b/doc/topics/troubleshooting/yaml_idiosyncrasies.rst index 1ee1f5326f21..b51bf6c0eda0 100644 --- a/doc/topics/troubleshooting/yaml_idiosyncrasies.rst +++ b/doc/topics/troubleshooting/yaml_idiosyncrasies.rst @@ -382,50 +382,126 @@ 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:: 3006.0 + + Loading a YAML ``!!timestamp`` node now produces a ``datetime.datetime`` + object. Previously, nodes tagged with ``!!timestamp`` produced strings. + +.. versionchanged:: 3006.0 + + Dumping a ``datetime.datetime`` object to YAML now explicitly tags the node + with ``!!timestamp``. Previously the ``!!timestamp`` tag was omitted. + +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") + '2014-01-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:: 3006.0 + + Loading a YAML ``!!omap`` node now reliably produces a + ``collections.OrderedDict`` object. Previously, an ``!!omap`` node would + sometimes produce a ``list`` of (key, value) ``tuple`` objects and sometimes + raise an exception. + +.. versionchanged:: 3006.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 produced a plain + mapping node. + +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:: 3006.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. + +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/modules/ldap3.py b/salt/modules/ldap3.py index a065b306b6d4..368fefcbe8ae 100644 --- a/salt/modules/ldap3.py +++ b/salt/modules/ldap3.py @@ -13,7 +13,7 @@ import logging -import salt.utils.data +from salt.utils.ldap import AttributeValueSet, LDAPError available_backends = set() try: @@ -35,21 +35,6 @@ def __virtual__(): return bool(len(available_backends)) -class LDAPError(Exception): - """Base class of all LDAP exceptions raised by backends. - - This is only used for errors encountered while interacting with - the LDAP server; usage errors (e.g., invalid backend name) will - have a different type. - - :ivar cause: backend exception object, if applicable - """ - - def __init__(self, message, cause=None): - super().__init__(message) - self.cause = cause - - def _convert_exception(e): """Convert an ldap backend exception to an LDAPError and raise it.""" raise LDAPError("exception in ldap backend: {!r}".format(e), e) from e @@ -79,20 +64,6 @@ def _bind(l, bind=None): ) -def _format_unicode_password(pwd): - """Formats a string per Microsoft AD password specifications. - The string must be enclosed in double quotes and UTF-16 encoded. - See: https://msdn.microsoft.com/en-us/library/cc223248.aspx - - :param pwd: - The desired password as a string - - :returns: - A unicode string - """ - return '"{}"'.format(pwd).encode("utf-16-le") - - class _connect_ctx: def __init__(self, c): self.c = c @@ -250,13 +221,15 @@ def connect(connect_spec=None): .. code-block:: bash - salt '*' ldap3.connect "{ - 'url': 'ldaps://ldap.example.com/', - 'bind': { - 'method': 'simple', - 'dn': 'cn=admin,dc=example,dc=com', - 'password': 'secret'} - }" + salt minion.example.com ldap3.connect \ + "{ + 'url': 'ldaps://ldap.example.com/', + 'bind': { + 'method': 'simple', + 'dn': 'cn=admin,dc=example,dc=com', + 'password': 'secret', + }, + }" """ if isinstance(connect_spec, _connect_ctx): return connect_spec @@ -318,6 +291,9 @@ def search( ): """Search an LDAP database. + .. versionchanged:: 3006.0 + The returned attribute values are now decoded to strings when possible. + :param connect_spec: See the documentation for the ``connect_spec`` parameter for :py:func:`connect`. @@ -357,14 +333,16 @@ def search( .. code-block:: bash - salt '*' ldap3.search "{ - 'url': 'ldaps://ldap.example.com/', - 'bind': { - 'method': 'simple', - 'dn': 'cn=admin,dc=example,dc=com', - 'password': 'secret', - }, - }" "base='dc=example,dc=com'" + salt minion.example.com ldap3.search \ + "{ + 'url': 'ldaps://ldap.example.com/', + 'bind': { + 'method': 'simple', + 'dn': 'cn=admin,dc=example,dc=com', + 'password': 'secret', + }, + }" \ + "base='dc=example,dc=com'" """ l = connect(connect_spec) scope = getattr(ldap, "SCOPE_" + scope.upper()) @@ -374,7 +352,13 @@ def search( results = [] except ldap.LDAPError as e: _convert_exception(e) - return dict(results) + return { + dn: { + attr: list(AttributeValueSet(attr, encvals)) + for attr, encvals in encattrs.items() + } + for dn, encattrs in results + } def add(connect_spec, dn, attributes): @@ -398,29 +382,24 @@ def add(connect_spec, dn, attributes): .. code-block:: bash - salt '*' ldap3.add "{ - 'url': 'ldaps://ldap.example.com/', - 'bind': { - 'method': 'simple', - 'password': 'secret', - }, - }" "dn='dc=example,dc=com'" "attributes={'example': 'values'}" + salt minion.example.com ldap3.add \ + "{ + 'url': 'ldaps://ldap.example.com/', + 'bind': { + 'method': 'simple', + 'password': 'secret', + }, + }" \ + "dn='dc=example,dc=com'" \ + "attributes={'example': ['values']}" """ l = connect(connect_spec) - # convert the "iterable of values" to lists in case that's what - # addModlist() expects (also to ensure that the caller's objects - # are not modified) - attributes = { - attr: salt.utils.data.encode(list(vals)) for attr, vals in attributes.items() - } log.info("adding entry: dn: %s attributes: %s", repr(dn), repr(attributes)) - - if "unicodePwd" in attributes: - attributes["unicodePwd"] = [ - _format_unicode_password(x) for x in attributes["unicodePwd"] - ] - - modlist = ldap.modlist.addModlist(attributes) + encattrs = { + attr: AttributeValueSet(attr, vals).encode() + for attr, vals in attributes.items() + } + modlist = ldap.modlist.addModlist(encattrs) try: l.c.add_s(dn, modlist) except ldap.LDAPError as e: @@ -445,12 +424,15 @@ def delete(connect_spec, dn): .. code-block:: bash - salt '*' ldap3.delete "{ - 'url': 'ldaps://ldap.example.com/', - 'bind': { - 'method': 'simple', - 'password': 'secret'} - }" dn='cn=admin,dc=example,dc=com' + salt minion.example.com ldap3.delete \ + "{ + 'url': 'ldaps://ldap.example.com/', + 'bind': { + 'method': 'simple', + 'password': 'secret', + }, + }" \ + "dn='cn=admin,dc=example,dc=com'" """ l = connect(connect_spec) log.info("deleting entry: dn: %s", repr(dn)) @@ -500,32 +482,26 @@ def modify(connect_spec, dn, directives): .. code-block:: bash - salt '*' ldap3.modify "{ - 'url': 'ldaps://ldap.example.com/', - 'bind': { - 'method': 'simple', - 'password': 'secret'} - }" dn='cn=admin,dc=example,dc=com' - directives="('add', 'example', ['example_val'])" + salt minion.example.com ldap3.modify \ + "{ + 'url': 'ldaps://ldap.example.com/', + 'bind': { + 'method': 'simple', + 'password': 'secret', + }, + }" \ + "dn='cn=admin,dc=example,dc=com'" \ + "directives=[('add', 'example', ['example_val'])]" """ l = connect(connect_spec) - # convert the "iterable of values" to lists in case that's what - # modify_s() expects (also to ensure that the caller's objects are - # not modified) modlist = [ - (getattr(ldap, "MOD_" + op.upper()), attr, list(vals)) + ( + getattr(ldap, "MOD_" + op.upper()), + attr, + AttributeValueSet(attr, vals).encode(), + ) for op, attr, vals in directives ] - - for idx, mod in enumerate(modlist): - if mod[1] == "unicodePwd": - modlist[idx] = ( - mod[0], - mod[1], - [_format_unicode_password(x) for x in mod[2]], - ) - - modlist = salt.utils.data.decode(modlist, to_str=True, preserve_tuples=True) try: l.c.modify_s(dn, modlist) except ldap.LDAPError as e: @@ -536,18 +512,29 @@ def modify(connect_spec, dn, directives): def change(connect_spec, dn, before, after): """Modify an entry in an LDAP database. - This does the same thing as :py:func:`modify`, but with a simpler - interface. Instead of taking a list of directives, it takes a - before and after view of an entry, determines the differences - between the two, computes the directives, and executes them. + This does the same thing as :py:func:`modify`, but with a simpler interface. + Instead of taking a list of directives, it takes a before and after view of + an entry, determines the differences between the two, computes directives + based on the differences, and executes the directives. + + The directives are computed as follows: + + * If an attribute name is present in ``before`` but missing or mapped to a + zero-length iterable of values in ``after``, the attribute is deleted + (regardless of whether the values in the database match the values in + ``before``). - Any attribute value present in ``before`` but missing in ``after`` - is deleted. Any attribute value present in ``after`` but missing - in ``before`` is added. Any attribute value in the database that - is not mentioned in either ``before`` or ``after`` is not altered. - Any attribute value that is present in both ``before`` and - ``after`` is ignored, regardless of whether that attribute value - exists in the database. + * Otherwise, if some values are present in ``before`` but missing from + ``after`` and some values are present in ``after`` but missing from + ``before``, all of the attribute's values are replaced with the values in + ``after`` (regardless of whether the values in the database match the + values in ``before``). + + * Otherwise, if some values are present in ``before`` but missing from + ``after``, those specific values are deleted. + + * Otherwise, if some values are present in ``after`` but missing from + ``before``, those specific values are added. :param connect_spec: See the documentation for the ``connect_spec`` parameter for @@ -557,12 +544,12 @@ def change(connect_spec, dn, before, after): Distinguished name of the entry. :param before: - The expected state of the entry before modification. This is - a dict mapping each attribute name to an iterable of values. + The expected state of the entry before modification. This is a mapping + that maps each attribute name to an iterable of values. :param after: - The desired state of the entry after modification. This is a - dict mapping each attribute name to an iterable of values. + The desired state of the entry after modification. This is a mapping + that maps each attribute name to an iterable of values. :returns: ``True`` if successful, raises an exception otherwise. @@ -571,29 +558,51 @@ def change(connect_spec, dn, before, after): .. code-block:: bash - salt '*' ldap3.change "{ - 'url': 'ldaps://ldap.example.com/', - 'bind': { - 'method': 'simple', - 'password': 'secret'} - }" dn='cn=admin,dc=example,dc=com' - before="{'example_value': 'before_val'}" - after="{'example_value': 'after_val'}" + salt minion.example.com ldap3.change \ + "{ + 'url': 'ldaps://ldap.example.com/', + 'bind': { + 'method': 'simple', + 'password': 'secret', + }, + }" \ + "dn='cn=admin,dc=example,dc=com'" \ + "before={'example_value': ['before_val']}" \ + "after={'example_value': ['after_val']}" """ - l = connect(connect_spec) - # convert the "iterable of values" to lists in case that's what - # modifyModlist() expects (also to ensure that the caller's dicts - # are not modified) - before = {attr: salt.utils.data.encode(list(vals)) for attr, vals in before.items()} - after = {attr: salt.utils.data.encode(list(vals)) for attr, vals in after.items()} - - if "unicodePwd" in after: - after["unicodePwd"] = [_format_unicode_password(x) for x in after["unicodePwd"]] - - modlist = ldap.modlist.modifyModlist(before, after) - - try: - l.c.modify_s(dn, modlist) - except ldap.LDAPError as e: - _convert_exception(e) - return True + # This function could instead use `ldap.modlist.modifyModlist()` to build a + # modlist from `before` and `after`, but the behavior of that function is + # unfortunate: When adding attribute values, the modlist that function + # returns first deletes the attribute then adds it back with the + # original+new values. This is problematic for certain OpenLDAP `cn=config` + # entries where adding and removing values triggers behavioral changes + # (e.g., `olcModuleLoad` in `cn=module{0},cn=config`). + + # Don't encode the values here -- modify() will encode them for us. + before = {attr: AttributeValueSet(attr, vals) for attr, vals in before.items()} + after = {attr: AttributeValueSet(attr, vals) for attr, vals in after.items()} + directives = [] + for attr, before_vals in before.items(): + after_vals = after.get(attr, AttributeValueSet(attr)) + if not after_vals: + directives.append(("delete", attr, ())) + continue + only_in_before = before_vals - after_vals + only_in_after = after_vals - before_vals + if only_in_before: + if only_in_after: + directives.append(("replace", attr, after_vals)) + else: + directives.append(("delete", attr, only_in_before)) + else: + if only_in_after: + directives.append(("add", attr, only_in_after)) + else: + # Nothing to do for this attribute because they already match. + assert before_vals == after_vals + for attr, after_vals in after.items(): + if attr in before or not after_vals: + # Either already handled above or nothing to add. + continue + directives.append(("add", attr, after_vals)) + return modify(connect_spec, dn, directives) diff --git a/salt/states/ldap.py b/salt/states/ldap.py index 227182b828b1..4a577d8c3597 100644 --- a/salt/states/ldap.py +++ b/salt/states/ldap.py @@ -8,19 +8,17 @@ their attributes. """ -import copy -import inspect import logging +from collections import OrderedDict -from salt.utils.odict import OrderedDict +from salt.utils.ldap import AttributeValueSet, LDAPError from salt.utils.oset import OrderedSet -from salt.utils.stringutils import to_bytes log = logging.getLogger(__name__) def managed(name, entries, connect_spec=None): - """Ensure the existence (or not) of LDAP entries and their attributes + """Ensure the existence (or not) of LDAP entries and their attributes. Example: @@ -162,10 +160,9 @@ def managed(name, entries, connect_spec=None): of the attribute's values are deleted. * ``'replace'`` - Attributes to replace. This is a dict mapping an - attribute name to an iterable of values. Any existing - values for the attribute are deleted, then the given - values are added. The iterable may be empty. + Attribute values that will replace any existing values. This is a + dict (which may be empty) mapping an attribute name to an iterable + (which may be empty) of values. In the above directives, the iterables of attribute values may instead be ``None``, in which case an empty list is used, or a @@ -175,6 +172,13 @@ def managed(name, entries, connect_spec=None): Note that if all attribute values are removed from an entry, the entire entry is deleted. + `RFC 4511 section 4.1.7 + `_ says, + "The set of attribute values is unordered." Despite this, if an + attribute has more than one new value, the new values are sent to the + server in the given order in case order is meaningful to the server (as + an extension to the standard). + :param connect_spec: See the description of the ``connect_spec`` parameter of the :py:func:`ldap3.connect ` function @@ -246,20 +250,10 @@ def managed(name, entries, connect_spec=None): # already a connection object pass - connect = __salt__["ldap3.connect"] - - # hack to get at the ldap3 module to access the ldap3.LDAPError - # exception class. https://github.com/saltstack/salt/issues/27578 - ldap3 = inspect.getmodule(connect) - - with connect(connect_spec) as l: - + with __salt__["ldap3.connect"](connect_spec) as l: old, new = _process_entries(l, entries) - # collect all of the affected entries (only the key is - # important in this dict; would have used an OrderedSet if - # there was one) - dn_set = OrderedDict() + dn_set = OrderedSet() dn_set.update(old) dn_set.update(new) @@ -282,7 +276,7 @@ def managed(name, entries, connect_spec=None): for dn in dn_to_delete: for x in old, new: x.pop(dn, None) - del dn_set[dn] + dn_set.remove(dn) ret = { "name": name, @@ -309,7 +303,7 @@ def managed(name, entries, connect_spec=None): ret["result"] = True ret["comment"] = "Successfully updated LDAP entries" errs = [] - success_dn_set = OrderedDict() + success_dn_set = OrderedSet() for dn in dn_set: o = old.get(dn, {}) n = new.get(dn, {}) @@ -333,8 +327,8 @@ def managed(name, entries, connect_spec=None): # is raised changed_old[dn] = o changed_new[dn] = n - success_dn_set[dn] = True - except ldap3.LDAPError as err: + success_dn_set.add(dn) + except LDAPError as err: log.exception("failed to %s entry %s (%s)", op, dn, err) errs.append((op, dn, err)) continue @@ -347,7 +341,7 @@ def managed(name, entries, connect_spec=None): # set ret['changes']. filter out any unchanged attributes, and # convert the value sets to lists before returning them to the - # user (sorted for easier comparisons) + # user. for dn in success_dn_set: o = changed_old.get(dn, {}) n = changed_new.get(dn, {}) @@ -358,55 +352,57 @@ def managed(name, entries, connect_spec=None): changes[xn] = None continue changes[xn] = { - attr: sorted(vals) + attr: list(vals) for attr, vals in x.items() - if o.get(attr, ()) != n.get(attr, ()) + if ( + o.get(attr, AttributeValueSet(attr)) + != n.get(attr, AttributeValueSet(attr)) + ) } return ret def _process_entries(l, entries): - """Helper for managed() to process entries and return before/after views + """Helper for managed() to process entries and obtain before/after views. - Collect the current database state and update it according to the - data in :py:func:`managed`'s ``entries`` parameter. Return the - current database state and what it will look like after - modification. + Collects the current database state and updates it according to the data in + :py:func:`managed`'s ``entries`` parameter. Returns the current database + state and what it will look like after modification. :param l: - the LDAP connection object + The LDAP connection object. :param entries: - the same object passed to the ``entries`` parameter of - :py:func:`manage` - - :return: - an ``(old, new)`` tuple that describes the current state of - the entries and what they will look like after modification. - Each item in the tuple is an OrderedDict that maps an entry DN - to another dict that maps an attribute name to a set of its - values (it's a set because according to the LDAP spec, - attribute value ordering is unspecified and there can't be - duplicates). The structure looks like this: - - {dn1: {attr1: set([val1])}, - dn2: {attr1: set([val2]), attr2: set([val3, val4])}} - - All of an entry's attributes and values will be included, even - if they will not be modified. If an entry mentioned in the - entries variable doesn't yet exist in the database, the DN in - ``old`` will be mapped to an empty dict. If an entry in the - database will be deleted, the DN in ``new`` will be mapped to - an empty dict. All value sets are non-empty: An attribute - that will be added to an entry is not included in ``old``, and - an attribute that will be deleted frm an entry is not included - in ``new``. - - These are OrderedDicts to ensure that the user-supplied - entries are processed in the user-specified order (in case - there are dependencies, such as ACL rules specified in an - early entry that make it possible to modify a later entry). + The same object passed to the ``entries`` parameter of + :py:func:`manage`. + + :returns: + An ``(old, new)`` tuple that describes the current state of the entries + and what they will look like after modification. Each item in the tuple + is an ``OrderedDict`` that maps an entry DN to a ``dict`` that maps an + attribute name to an :py:class:`~salt.utils.ldap.AttributeValueSet` of + its values. The structure looks like this:: + + OrderedDict([ + (dn1, {attr1: AttributeValueSet(attr1, [val1])}), + (dn2, {attr1: AttributeValueSet(attr1, [val2]), + attr2: AttributeValueSet(attr2, [val3, val4])}), + ]) + + All of an entry's attributes and values will be included, even if they + will not be modified. If an entry mentioned in ``entries`` does not yet + exist in the database, the DN in ``old`` will be mapped to an empty + ``dict``. If an entry in the database will be deleted, the DN in ``new`` + will be mapped to an empty ``dict``. All value sets are non-empty: An + attribute that will be added to an entry is not included in ``old``, and + an attribute that will be deleted from an entry is not included in + ``new``. + + These are ``OrderedDicts`` to ensure that the user-supplied entries are + processed in the user-specified order (in case there are dependencies, + such as ACL rules specified in an early entry that make it possible to + modify a later entry). """ old = OrderedDict() @@ -423,19 +419,18 @@ def _process_entries(l, entries): if len(results) == 1: attrs = results[dn] olde = { - attr: OrderedSet(attrs[attr]) - for attr in attrs - if len(attrs[attr]) + attr: AttributeValueSet(attr, vals) + for attr, vals in attrs.items() + if len(vals) } else: # nothing, so it must be a brand new entry assert len(results) == 0 olde = {} old[dn] = olde - # copy the old entry to create the new (don't do a simple - # assignment or else modifications to newe will affect - # olde) - newe = copy.deepcopy(olde) + # Deep copy the old entry to create the new (don't do a simple + # assignment or else modifications to `newe` will affect `olde`). + newe = {attr: AttributeValueSet(attr, vals) for attr, vals in olde.items()} new[dn] = newe # process the directives @@ -472,64 +467,77 @@ def _update_entry(entry, status, directives): continue for attr, vals in state.items(): status["mentioned_attributes"].add(attr) - vals = _toset(vals) + vals = _toset(attr, vals) if directive == "default": if vals and (attr not in entry or not entry[attr]): entry[attr] = vals elif directive == "add": - vals.update(entry.get(attr, OrderedSet())) - if vals: - entry[attr] = vals + existing_vals = entry.get(attr, AttributeValueSet(attr)) + # Preserve the order of pre-existing values by updating + # existing_vals with vals rather than the other way around. + existing_vals |= vals + if existing_vals: + entry[attr] = existing_vals elif directive == "delete": - existing_vals = entry.pop(attr, OrderedSet()) + existing_vals = entry.pop(attr, AttributeValueSet(attr)) if vals: existing_vals -= vals if existing_vals: entry[attr] = existing_vals elif directive == "replace": - entry.pop(attr, None) - if vals: - entry[attr] = vals + existing_vals = entry.pop(attr, AttributeValueSet(attr)) + # Preserve the order of pre-existing values by first keeping + # the common values then inserting the new values. + existing_vals &= vals + existing_vals |= vals + if existing_vals: + entry[attr] = existing_vals else: raise ValueError("unknown directive: " + directive) -def _toset(thing): - """helper to convert various things to a set +def _normalize(thing): + """Convert thing to a str or bytes. - This enables flexibility in what users provide as the list of LDAP - entry attribute values. Note that the LDAP spec prohibits - duplicate values in an attribute. + Details: + * bytes-like values are copied and returned. + * str values are returned as-is. + * int values (including Booleans) are converted to str. + * Everything else raises an exception. + """ + try: + # If thing is already a bytes-like object, return a copy of the bytes. + # The intermediate call to memoryview prevents this function from + # interpreting an iterable of 8-bit integer values as a string of bytes. + thing = bytes(memoryview(thing)) + except: # pylint: disable=bare-except + pass + if isinstance(thing, int): + thing = str(thing) + if not isinstance(thing, (str, bytes)): + raise TypeError(f"expected an int, str, or bytes-like value, got {type(thing)}") + return thing - RFC 2251 states that: - "The order of attribute values within the vals set is undefined and - implementation-dependent, and MUST NOT be relied upon." - However, OpenLDAP have an X-ORDERED that is used in the config schema. - Using sets would mean we can't pass ordered values and therefore can't - manage parts of the OpenLDAP configuration, hence the use of OrderedSet. - Sets are also good for automatically removing duplicates. +def _toset(attr, thing): + """Helper to convert various things to an ``AttributeValueSet``. - None becomes an empty set. Iterables except for strings have - their elements added to a new set. Non-None scalars (strings, - numbers, non-iterable objects, etc.) are added as the only member - of a new set. + This enables flexibility in what users provide as the list of LDAP entry + attribute values. + Details: + * ``None`` becomes a new empty set. + * ``bytes``-like, ``str``, and ``int`` values are normalized with + ``_normalize()`` and returned as the only member of a new set. + * Other values are assumed to be iterables of values to normalize and + return in the new set. (If it is not an iterable, or if an entry can't + be normalized, an exception is raised). """ if thing is None: - return OrderedSet() - if isinstance(thing, str): - return OrderedSet((to_bytes(thing),)) - if isinstance(thing, int): - return OrderedSet((to_bytes(str(thing)),)) - # convert numbers to strings and then bytes - # so that equality checks work - # (LDAP stores numbers as strings) - try: - return OrderedSet( - to_bytes(str(x)) if isinstance(x, int) else to_bytes(x) for x in thing - ) - except TypeError: - return OrderedSet( - str(thing), - ) + coll = () + else: + try: + coll = (_normalize(thing),) + except TypeError: + coll = (_normalize(x) for x in thing) + return AttributeValueSet(attr, coll) diff --git a/salt/utils/ldap.py b/salt/utils/ldap.py new file mode 100644 index 000000000000..e1a990011624 --- /dev/null +++ b/salt/utils/ldap.py @@ -0,0 +1,118 @@ +"""Common classes shared between LDAP execution and state modules.""" + + +from salt.utils.oset import OrderedSet + + +class AttributeValueSet(OrderedSet): + """Holds an attribute's values as an ordered set. + + `RFC 4511 section 4.1.7 + `_ says, "The + set of attribute values is unordered." Despite this, this set is ordered so + that it can support the `X-ORDERED + `_ + extension. (OpenLDAP has some X-ORDERED attributes in its ``cn=config`` + DIT.) + + RFC 4511 goes on to say, "Implementations MUST NOT rely upon the ordering + being repeatable." To conform to this, the + :py:meth:`~AttributeValueSet.__eq__` method ignores order. Salt will report + no differences and take no action when a desired set of values already + matches what is in LDAP, even if the reported order differs from the desired + order. + + ``str`` values are stored as-is. Other types are first converted to + ``bytes``, then decoded before being stored. If decoding fails, the + ``bytes`` object is stored instead. (This makes it possible for users to + manually pre-encode a value in case this class's encoding behavior is not + suitable for the attribute.) + + ``bytes`` objects are decoded from UTF-8, with one exception: If the + attribute name is ``'unicodePwd'``, the values are decoded according to `the + Microsoft AD password specification + `_. + """ + + def __init__(self, attr, vals=None): + self.attr = attr + super().__init__(vals) + + # Used by collections.abc.MutableSet to construct a new AttributeValueSet + # object for operations such as set difference. + def _from_iterable(self, it): + return type(self)(self.attr, it) + + def _decode_val(self, v): + if isinstance(v, str): + return v + v = bytes(v) + try: + if self.attr == "unicodePwd": + tmp = v.decode("utf-16-le") + if len(tmp) < 2 or tmp[0] != '"' or tmp[-1] != '"': + raise ValueError("not enclosed in double quotes") + return tmp[1:-1] + else: + return v.decode() + except: # pylint: disable=bare-except + pass + return v + + def _encode_val(self, v): + if isinstance(v, bytes): + return v + assert isinstance(v, str) + if self.attr == "unicodePwd": + return f'"{v}"'.encode("utf-16-le") + return v.encode() + + def encode(self): + """Encodes the values for writing to LDAP. + + When writing to LDAP, the Python ``ldap`` module `expects attribute + values to be ``bytes`` objects + `_. + + :returns: + A list of ``bytes`` objects containing the encoded values. See the + class description for details about how the values are encoded. + """ + return [self._encode_val(v) for v in self] + + def copy(self): + return self.__class__(self.attr, self) + + def __contains__(self, key): + return super().__contains__(self._decode_val(key)) + + def add(self, key): + return super().add(self._decode_val(key)) + + def index(self, key): + return super().index(self._decode_val(key)) + + def __eq__(self, other): + if other is None: + return False + if other is self: + return True + return set(self) == {self._decode_val(v) for v in other} + + def __repr__(self): + return f"{self.__class__.__name__}({self.attr!r}, {list(self)!r})" + + +class LDAPError(Exception): + """Base class of all LDAP exceptions raised by backends. + + This is only used for errors encountered while interacting with + the LDAP server; usage errors (e.g., invalid backend name) will + have a different type. + + :ivar cause: backend exception object, if applicable + """ + + def __init__(self, message, cause=None): + super().__init__(message) + self.cause = cause diff --git a/salt/utils/yamldumper.py b/salt/utils/yamldumper.py index e5e937cac7d6..9f3b18805acb 100644 --- a/salt/utils/yamldumper.py +++ b/salt/utils/yamldumper.py @@ -12,7 +12,6 @@ import yaml # pylint: disable=blacklisted-import import salt.utils.context -from salt.utils.odict import OrderedDict try: from yaml import CDumper as Dumper @@ -31,69 +30,88 @@ ] -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. - """ +class _RemoveImplicitResolverMixin: + @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 increase_indent(self, flow=False, indentless=False): - return super().increase_indent(flow, False) +class SafeOrderedDumper(SafeDumper, _RemoveImplicitResolverMixin): + """A safe YAML dumper that uses the YAML ``!!omap`` type for ``OrderedDict`` -class OrderedDumper(Dumper): - """ - A YAML dumper that represents python OrderedDict as simple YAML map. - """ + ``OrderedDict``s are represented as a a sequence of single-entry mappings + and tagged with ``!!omap``: + .. code-block:: yaml -class SafeOrderedDumper(SafeDumper): - """ - A YAML safe dumper that represents python OrderedDict as simple YAML map. + !!omap + - first key: first value + - second key: second value + + See https://yaml.org/type/omap.html for details. """ -class IndentedSafeOrderedDumper(IndentMixin, SafeOrderedDumper): - """ - A YAML safe dumper that represents python OrderedDict as simple YAML map, - and also indents lists by two spaces. +class OrderedDumper(Dumper, _RemoveImplicitResolverMixin): + """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(yaml.SafeDumper, _RemoveImplicitResolverMixin): + """Like ``SafeOrderedDumper``, except it indents lists for readability.""" + + def increase_indent(self, flow=False, indentless=False): + return super().increase_indent(flow, False) + + def represent_ordereddict(dumper, data): - return dumper.represent_dict(list(data.items())) + seq = [{k: v} for k, v in data.items()] + return dumper.represent_sequence("tag:yaml.org,2002:omap", seq) def represent_undefined(dumper, data): return dumper.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 -) -SafeOrderedDumper.add_representer( - collections.defaultdict, yaml.representer.SafeRepresenter.represent_dict -) -OrderedDumper.add_representer( - salt.utils.context.NamespacedDictWrapper, - yaml.representer.SafeRepresenter.represent_dict, -) -SafeOrderedDumper.add_representer( - salt.utils.context.NamespacedDictWrapper, - yaml.representer.SafeRepresenter.represent_dict, -) - -OrderedDumper.add_representer( - "tag:yaml.org,2002:timestamp", OrderedDumper.represent_scalar -) -SafeOrderedDumper.add_representer( - "tag:yaml.org,2002:timestamp", SafeOrderedDumper.represent_scalar -) +# The above Dumper classes do not inherit from each other, so any applicable +# representers must be added to each. +for D in (SafeOrderedDumper, IndentedSafeOrderedDumper): + # TODO: Why does this representer exist? It doesn't seem to do anything + # different compared to PyYAML's yaml.SafeDumper. + # TODO: Why isn't this representer also registered with OrderedDumper? + D.add_representer(None, represent_undefined) +for D in (SafeOrderedDumper, IndentedSafeOrderedDumper, OrderedDumper): + # This multi representer covers collections.OrderedDict and all of its + # subclasses, including salt.utils.odict.OrderedDict. + D.add_multi_representer(collections.OrderedDict, represent_ordereddict) + # 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.) + D.add_representer(collections.OrderedDict, represent_ordereddict) + D.add_representer( + collections.defaultdict, yaml.representer.SafeRepresenter.represent_dict + ) + D.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. + D.add_multi_representer(tuple, Dumper.yaml_representers[tuple]) + D.add_representer(tuple, Dumper.yaml_representers[tuple]) + # Explicitly include the `!!timestamp` tag when dumping datetime objects. + D.remove_implicit_resolver("tag:yaml.org,2002:timestamp") +del D def get_dumper(dumper_name): @@ -108,12 +126,20 @@ def dump(data, stream=None, **kwargs): """ .. versionadded:: 2018.3.0 + .. versionchanged:: 3006.0 + + The default ``Dumper`` class is now ``OrderedDumper`` instead of + ``yaml.Dumper``. + 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("default_flow_style", None) + kwargs = { + "allow_unicode": True, + "default_flow_style": None, + "Dumper": OrderedDumper, + **kwargs, + } return yaml.dump(data, stream, **kwargs) @@ -123,7 +149,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..c76551656b7a 100644 --- a/salt/utils/yamlloader.py +++ b/salt/utils/yamlloader.py @@ -3,6 +3,8 @@ """ +import collections + import yaml # pylint: disable=blacklisted-import from yaml.constructor import ConstructorError from yaml.nodes import MappingNode, SequenceNode @@ -24,27 +26,46 @@ class SaltYamlSafeLoader(BaseLoader): 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(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) + 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 +107,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 +179,23 @@ def flatten_mapping(self, node): node.value = mergeable_items + node.value +# BaseLoader.add_constructor() is a class method, not an instance method, so +# custom constructors should be registered at class creation time, not instance +# creation time. +for tag, constructor in [ + ("tag:yaml.org,2002:map", SaltYamlSafeLoader.construct_yaml_map), + ("tag:yaml.org,2002:omap", SaltYamlSafeLoader.construct_yaml_omap), + ("tag:yaml.org,2002:str", SaltYamlSafeLoader.construct_yaml_str), + ("tag:yaml.org,2002:python/tuple", SaltYamlSafeLoader.construct_python_tuple), + ("tag:yaml.org,2002:python/unicode", SaltYamlSafeLoader.construct_unicode), +]: + SaltYamlSafeLoader.add_constructor(tag, constructor) + +# Require users to explicitly provide the `!!timestamp` tag if a datetime object +# is desired. +SaltYamlSafeLoader.remove_implicit_resolver("tag:yaml.org,2002:timestamp") + + def load(stream, Loader=SaltYamlSafeLoader): return yaml.load(stream, Loader=Loader) diff --git a/tests/pytests/integration/modules/test_ldap3.py b/tests/pytests/integration/modules/test_ldap3.py new file mode 100644 index 000000000000..e45d6617be9a --- /dev/null +++ b/tests/pytests/integration/modules/test_ldap3.py @@ -0,0 +1,39 @@ +import pytest + +pytest_plugins = [ + "tests.support.pytest.ldap", +] +pytestmark = [ + pytest.mark.destructive_test, + pytest.mark.skip_if_binaries_missing("docker"), + pytest.mark.slow_test, +] + + +def test_search(openldap_minion_run, subtree, u0dn): + assert openldap_minion_run("ldap3.search", base=subtree) == { + subtree: { + "objectClass": ["dcObject", "organization"], + "dc": ["test_search"], + "o": ["test_search"], + }, + u0dn: { + "objectClass": ["person"], + "cn": ["u0"], + "sn": ["Lastname"], + "description": ["desc", "another desc"], + }, + } + + +def test_search_filter(openldap_minion_run, subtree, u0dn): + assert openldap_minion_run( + "ldap3.search", base=subtree, filterstr="(sn=Lastname)" + ) == { + u0dn: { + "objectClass": ["person"], + "cn": ["u0"], + "sn": ["Lastname"], + "description": ["desc", "another desc"], + }, + } 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..fcccdc4b2694 --- /dev/null +++ b/tests/pytests/integration/pillar/test_pillar_map_order.py @@ -0,0 +1,112 @@ +import random +import textwrap + +import pytest + +pytestmark = [ + pytest.mark.slow_test, +] + + +@pytest.fixture(scope="module") +def minion_run(salt_minion, salt_cli): + """Convenience fixture that runs the ``salt`` CLI targeting the minion.""" + + def _run(*args, **kwargs): + ret = salt_cli.run(*args, **{"minion_tgt": salt_minion.id, **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): + """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/integration/states/test_ldap.py b/tests/pytests/integration/states/test_ldap.py new file mode 100644 index 000000000000..cea9575c591f --- /dev/null +++ b/tests/pytests/integration/states/test_ldap.py @@ -0,0 +1,750 @@ +import pytest + +pytest_plugins = [ + "tests.support.pytest.ldap", +] +pytestmark = [ + pytest.mark.destructive_test, + pytest.mark.skip_if_binaries_missing("docker"), + pytest.mark.slow_test, +] + + +# These tests assume OpenLDAP returns attribute values in insertion order, which +# is not guaranteed but is the current behavior. We could define a custom +# X-ORDERED attribute to enforce this, but that adds a lot of complexity and +# there are several awkward corner cases that make it impractical. + + +def test_managed_add_new_entry(openldap_minion_run, openldap_minion_apply, subtree): + u1dn = f"cn=u1,{subtree}" + entries = [ + { + u1dn: [ + { + "add": { + "objectClass": ["person"], + # Note that this is not a list. It should behave as if + # we passed the list ["u1"]. + "cn": "u1", + # Note that this is not a list, nor is it a string. It + # should behave as if we passed the list ["1234"] + # (numbers are stringified and then put in a list). + "sn": 1234, + # List of values of various types. + "description": [ + "Non-ASCII characters should be supported: 🙂", + 4567, + # This will become a str when returned because it is + # valid UTF-8. + b"abcd", + ], + # Non-list iterable to exercise support for arbitrary + # iterables of values, not just lists. Note that tuples + # are currently (as of 2022-10-23) serialized as lists, + # but this value is not serialized before it is + # processed by the minion. (The state SLS file + # generated from this object by the + # openldap_minion_apply fixture is loaded on the minion + # and processed directly, not loaded on the master and + # sent to the minion in serialized form). + "userPassword": ( + "password", + # Intentionally invalid UTF-8. The syntax for + # userPassword is Octet String, not Directory String + # (like description), so this is acceptable. + b"\x00\x01\x02\x03\x80", + ), + # Empty list should be a no-op. + "telephoneNumber": [], + # None should be equivalent to an empty list. + "seeAlso": None, + }, + }, + ], + }, + ] + want = { + "objectClass": ["person"], + "cn": ["u1"], + "sn": ["1234"], + "description": [ + "Non-ASCII characters should be supported: 🙂", + "4567", + "abcd", + ], + "userPassword": [ + "password", + b"\x00\x01\x02\x03\x80", + ], + } + assert openldap_minion_apply("ldap.managed", entries=entries) == { + "changes": {u1dn: {"old": None, "new": want}}, + "comment": "Successfully updated LDAP entries", + "result": True, + } + assert openldap_minion_run("ldap3.search", base=u1dn) == {u1dn: want} + + +def test_managed_add_new_attribute(openldap_minion_run, openldap_minion_apply, u0dn): + entries = [{u0dn: [{"add": {"userPassword": ["p"]}}]}] + assert openldap_minion_apply("ldap.managed", entries=entries) == { + "changes": {u0dn: {"old": {}, "new": {"userPassword": ["p"]}}}, + "comment": "Successfully updated LDAP entries", + "result": True, + } + assert openldap_minion_run("ldap3.search", base=u0dn) == { + u0dn: { + "objectClass": ["person"], + "cn": ["u0"], + "sn": ["Lastname"], + "description": ["desc", "another desc"], + "userPassword": ["p"], + }, + } + + +def test_managed_add_no_values_to_existing_attribute( + openldap_minion_run, openldap_minion_apply, u0dn +): + entries = [{u0dn: [{"add": {"description": []}}]}] + assert openldap_minion_apply("ldap.managed", entries=entries) == { + "changes": {}, + "comment": "LDAP entries already set", + "result": True, + } + assert openldap_minion_run("ldap3.search", base=u0dn) == { + u0dn: { + "objectClass": ["person"], + "cn": ["u0"], + "sn": ["Lastname"], + "description": ["desc", "another desc"], + }, + } + + +def test_managed_add_no_values_to_new_attribute( + openldap_minion_run, openldap_minion_apply, u0dn +): + entries = [{u0dn: [{"add": {"telephoneNumber": []}}]}] + assert openldap_minion_apply("ldap.managed", entries=entries) == { + "changes": {}, + "comment": "LDAP entries already set", + "result": True, + } + assert openldap_minion_run("ldap3.search", base=u0dn) == { + u0dn: { + "objectClass": ["person"], + "cn": ["u0"], + "sn": ["Lastname"], + "description": ["desc", "another desc"], + }, + } + + +def test_managed_add_new_value_to_existing_attribute( + openldap_minion_run, openldap_minion_apply, u0dn +): + entries = [{u0dn: [{"add": {"description": ["and another"]}}]}] + assert openldap_minion_apply("ldap.managed", entries=entries) == { + "changes": { + u0dn: { + "old": {"description": ["desc", "another desc"]}, + "new": {"description": ["desc", "another desc", "and another"]}, + }, + }, + "comment": "Successfully updated LDAP entries", + "result": True, + } + assert openldap_minion_run("ldap3.search", base=u0dn) == { + u0dn: { + "objectClass": ["person"], + "cn": ["u0"], + "sn": ["Lastname"], + "description": ["desc", "another desc", "and another"], + }, + } + + +def test_managed_add_same_values_to_existing_attribute( + openldap_minion_run, openldap_minion_apply, u0dn +): + entries = [{u0dn: [{"add": {"description": ["desc", "another desc"]}}]}] + assert openldap_minion_apply("ldap.managed", entries=entries) == { + "changes": {}, + "comment": "LDAP entries already set", + "result": True, + } + assert openldap_minion_run("ldap3.search", base=u0dn) == { + u0dn: { + "objectClass": ["person"], + "cn": ["u0"], + "sn": ["Lastname"], + "description": ["desc", "another desc"], + }, + } + + +def test_managed_add_same_values_different_order( + openldap_minion_run, openldap_minion_apply, u0dn +): + entries = [{u0dn: [{"add": {"description": ["another desc", "desc"]}}]}] + assert openldap_minion_apply("ldap.managed", entries=entries) == { + "changes": {}, + "comment": "LDAP entries already set", + "result": True, + } + assert openldap_minion_run("ldap3.search", base=u0dn) == { + u0dn: { + "objectClass": ["person"], + "cn": ["u0"], + "sn": ["Lastname"], + "description": ["desc", "another desc"], + }, + } + + +def test_managed_add_overlapping_values( + openldap_minion_run, openldap_minion_apply, u0dn +): + entries = [{u0dn: [{"add": {"description": ["desc", "and another"]}}]}] + assert openldap_minion_apply("ldap.managed", entries=entries) == { + "changes": { + u0dn: { + "old": {"description": ["desc", "another desc"]}, + "new": {"description": ["desc", "another desc", "and another"]}, + }, + }, + "comment": "Successfully updated LDAP entries", + "result": True, + } + assert openldap_minion_run("ldap3.search", base=u0dn) == { + u0dn: { + "objectClass": ["person"], + "cn": ["u0"], + "sn": ["Lastname"], + "description": ["desc", "another desc", "and another"], + }, + } + + +def test_managed_add_overlapping_values_different_order( + openldap_minion_run, openldap_minion_apply, u0dn +): + entries = [{u0dn: [{"add": {"description": ["and another", "desc"]}}]}] + assert openldap_minion_apply("ldap.managed", entries=entries) == { + "changes": { + u0dn: { + "old": {"description": ["desc", "another desc"]}, + "new": {"description": ["desc", "another desc", "and another"]}, + }, + }, + "comment": "Successfully updated LDAP entries", + "result": True, + } + assert openldap_minion_run("ldap3.search", base=u0dn) == { + u0dn: { + "objectClass": ["person"], + "cn": ["u0"], + "sn": ["Lastname"], + "description": ["desc", "another desc", "and another"], + }, + } + + +def test_managed_add_repeated_values(openldap_minion_run, openldap_minion_apply, u0dn): + entries = [{u0dn: [{"add": {"description": ["val", "val"]}}]}] + assert openldap_minion_apply("ldap.managed", entries=entries) == { + "changes": { + u0dn: { + "old": {"description": ["desc", "another desc"]}, + "new": {"description": ["desc", "another desc", "val"]}, + }, + }, + "comment": "Successfully updated LDAP entries", + "result": True, + } + assert openldap_minion_run("ldap3.search", base=u0dn) == { + u0dn: { + "objectClass": ["person"], + "cn": ["u0"], + "sn": ["Lastname"], + "description": ["desc", "another desc", "val"], + }, + } + + +def test_managed_replace_new_entry(openldap_minion_run, openldap_minion_apply, subtree): + u1dn = f"cn=u1,{subtree}" + entries = [ + { + u1dn: [ + { + "replace": { + "objectClass": ["person"], + "cn": "u1", + "sn": "surname", + }, + }, + ], + }, + ] + want = { + "objectClass": ["person"], + "cn": ["u1"], + "sn": ["surname"], + } + assert openldap_minion_apply("ldap.managed", entries=entries) == { + "changes": {u1dn: {"old": None, "new": want}}, + "comment": "Successfully updated LDAP entries", + "result": True, + } + assert openldap_minion_run("ldap3.search", base=u1dn) == {u1dn: want} + + +def test_managed_replace_new_attribute( + openldap_minion_run, openldap_minion_apply, u0dn +): + entries = [{u0dn: [{"replace": {"userPassword": ["p"]}}]}] + assert openldap_minion_apply("ldap.managed", entries=entries) == { + "changes": { + u0dn: { + "old": {}, + "new": {"userPassword": ["p"]}, + }, + }, + "comment": "Successfully updated LDAP entries", + "result": True, + } + assert openldap_minion_run("ldap3.search", base=u0dn) == { + u0dn: { + "objectClass": ["person"], + "cn": ["u0"], + "sn": ["Lastname"], + "description": ["desc", "another desc"], + "userPassword": ["p"], + }, + } + + +def test_managed_replace_no_value_for_one_attribute( + openldap_minion_run, openldap_minion_apply, u0dn +): + entries = [{u0dn: [{"replace": {"description": []}}]}] + assert openldap_minion_apply("ldap.managed", entries=entries) == { + "changes": { + u0dn: { + "old": {"description": ["desc", "another desc"]}, + "new": {}, + }, + }, + "comment": "Successfully updated LDAP entries", + "result": True, + } + assert openldap_minion_run("ldap3.search", base=u0dn) == { + u0dn: { + "objectClass": ["person"], + "cn": ["u0"], + "sn": ["Lastname"], + }, + } + + +def test_managed_replace_no_values_for_all_attributes( + openldap_minion_run, openldap_minion_apply, u0dn +): + entries = [ + { + u0dn: [ + { + "replace": { + "objectClass": [], + "cn": [], + "sn": [], + "description": [], + }, + }, + ], + }, + ] + assert openldap_minion_apply("ldap.managed", entries=entries) == { + "changes": { + u0dn: { + "old": { + "objectClass": ["person"], + "cn": ["u0"], + "sn": ["Lastname"], + "description": ["desc", "another desc"], + }, + "new": None, + }, + }, + "comment": "Successfully updated LDAP entries", + "result": True, + } + assert openldap_minion_run("ldap3.search", base=u0dn) == {} + + +def test_managed_replace_no_values_for_new_attribute( + openldap_minion_run, openldap_minion_apply, u0dn +): + entries = [{u0dn: [{"replace": {"userPassword": []}}]}] + assert openldap_minion_apply("ldap.managed", entries=entries) == { + "changes": {}, + "comment": "LDAP entries already set", + "result": True, + } + assert openldap_minion_run("ldap3.search", base=u0dn) == { + u0dn: { + "objectClass": ["person"], + "cn": ["u0"], + "sn": ["Lastname"], + "description": ["desc", "another desc"], + }, + } + + +def test_managed_replace_new_values(openldap_minion_run, openldap_minion_apply, u0dn): + entries = [{u0dn: [{"replace": {"description": ["new desc"]}}]}] + assert openldap_minion_apply("ldap.managed", entries=entries) == { + "changes": { + u0dn: { + "old": {"description": ["desc", "another desc"]}, + "new": {"description": ["new desc"]}, + }, + }, + "comment": "Successfully updated LDAP entries", + "result": True, + } + assert openldap_minion_run("ldap3.search", base=u0dn) == { + u0dn: { + "objectClass": ["person"], + "cn": ["u0"], + "sn": ["Lastname"], + "description": ["new desc"], + }, + } + + +def test_managed_replace_same_values(openldap_minion_run, openldap_minion_apply, u0dn): + entries = [{u0dn: [{"replace": {"description": ["desc", "another desc"]}}]}] + assert openldap_minion_apply("ldap.managed", entries=entries) == { + "changes": {}, + "comment": "LDAP entries already set", + "result": True, + } + assert openldap_minion_run("ldap3.search", base=u0dn) == { + u0dn: { + "objectClass": ["person"], + "cn": ["u0"], + "sn": ["Lastname"], + "description": ["desc", "another desc"], + }, + } + + +def test_managed_replace_same_values_different_order( + openldap_minion_run, openldap_minion_apply, u0dn +): + entries = [{u0dn: [{"replace": {"description": ["another desc", "desc"]}}]}] + assert openldap_minion_apply("ldap.managed", entries=entries) == { + "changes": {}, + "comment": "LDAP entries already set", + "result": True, + } + assert openldap_minion_run("ldap3.search", base=u0dn) == { + u0dn: { + "objectClass": ["person"], + "cn": ["u0"], + "sn": ["Lastname"], + "description": ["desc", "another desc"], + }, + } + + +def test_managed_replace_overlapping_values( + openldap_minion_run, openldap_minion_apply, u0dn +): + entries = [{u0dn: [{"replace": {"description": ["desc", "new desc"]}}]}] + assert openldap_minion_apply("ldap.managed", entries=entries) == { + "changes": { + u0dn: { + "old": {"description": ["desc", "another desc"]}, + "new": {"description": ["desc", "new desc"]}, + }, + }, + "comment": "Successfully updated LDAP entries", + "result": True, + } + assert openldap_minion_run("ldap3.search", base=u0dn) == { + u0dn: { + "objectClass": ["person"], + "cn": ["u0"], + "sn": ["Lastname"], + "description": ["desc", "new desc"], + }, + } + + +def test_managed_replace_overlapping_values_different_order( + openldap_minion_run, openldap_minion_apply, u0dn +): + entries = [{u0dn: [{"replace": {"description": ["new desc", "desc"]}}]}] + assert openldap_minion_apply("ldap.managed", entries=entries) == { + "changes": { + u0dn: { + "old": {"description": ["desc", "another desc"]}, + "new": {"description": ["desc", "new desc"]}, + }, + }, + "comment": "Successfully updated LDAP entries", + "result": True, + } + assert openldap_minion_run("ldap3.search", base=u0dn) == { + u0dn: { + "objectClass": ["person"], + "cn": ["u0"], + "sn": ["Lastname"], + "description": ["desc", "new desc"], + }, + } + + +def test_managed_delete_value_from_nonexistent_entry( + openldap_minion_run, openldap_minion_apply, subtree +): + u1dn = f"cn=u1,{subtree}" + entries = [{u1dn: [{"delete": {"description": ["foo"]}}]}] + assert openldap_minion_apply("ldap.managed", entries=entries) == { + "changes": {}, + "comment": "LDAP entries already set", + "result": True, + } + assert openldap_minion_run("ldap3.search", base=u1dn) == {} + + +def test_managed_delete_value_from_nonexistent_attribute( + openldap_minion_run, openldap_minion_apply, u0dn +): + entries = [{u0dn: [{"delete": {"userPassword": ["foo"]}}]}] + assert openldap_minion_apply("ldap.managed", entries=entries) == { + "changes": {}, + "comment": "LDAP entries already set", + "result": True, + } + assert openldap_minion_run("ldap3.search", base=u0dn) == { + u0dn: { + "objectClass": ["person"], + "cn": ["u0"], + "sn": ["Lastname"], + "description": ["desc", "another desc"], + }, + } + + +def test_managed_delete_nonexistent_value( + openldap_minion_run, openldap_minion_apply, u0dn +): + entries = [{u0dn: [{"delete": {"description": ["foo"]}}]}] + assert openldap_minion_apply("ldap.managed", entries=entries) == { + "changes": {}, + "comment": "LDAP entries already set", + "result": True, + } + assert openldap_minion_run("ldap3.search", base=u0dn) == { + u0dn: { + "objectClass": ["person"], + "cn": ["u0"], + "sn": ["Lastname"], + "description": ["desc", "another desc"], + }, + } + + +def test_managed_delete_all_values_from_nonexistent_entry( + openldap_minion_run, openldap_minion_apply, subtree +): + u1dn = f"cn=u1,{subtree}" + entries = [{u1dn: [{"delete": {"description": []}}]}] + assert openldap_minion_apply("ldap.managed", entries=entries) == { + "changes": {}, + "comment": "LDAP entries already set", + "result": True, + } + assert openldap_minion_run("ldap3.search", base=u1dn) == {} + + +def test_managed_delete_all_values_from_nonexistent_attribute( + openldap_minion_run, openldap_minion_apply, u0dn +): + entries = [{u0dn: [{"delete": {"userPassword": []}}]}] + assert openldap_minion_apply("ldap.managed", entries=entries) == { + "changes": {}, + "comment": "LDAP entries already set", + "result": True, + } + assert openldap_minion_run("ldap3.search", base=u0dn) == { + u0dn: { + "objectClass": ["person"], + "cn": ["u0"], + "sn": ["Lastname"], + "description": ["desc", "another desc"], + }, + } + + +def test_managed_delete_remaining_attribute_values( + openldap_minion_run, openldap_minion_apply, u0dn +): + entries = [{u0dn: [{"delete": {"description": ["desc", "another desc"]}}]}] + assert openldap_minion_apply("ldap.managed", entries=entries) == { + "changes": { + u0dn: { + "old": {"description": ["desc", "another desc"]}, + "new": {}, + }, + }, + "comment": "Successfully updated LDAP entries", + "result": True, + } + assert openldap_minion_run("ldap3.search", base=u0dn) == { + u0dn: { + "objectClass": ["person"], + "cn": ["u0"], + "sn": ["Lastname"], + }, + } + + +def test_managed_delete_all_attribute_values( + openldap_minion_run, openldap_minion_apply, u0dn +): + entries = [{u0dn: [{"delete": {"description": []}}]}] + assert openldap_minion_apply("ldap.managed", entries=entries) == { + "changes": { + u0dn: { + "old": {"description": ["desc", "another desc"]}, + "new": {}, + }, + }, + "comment": "Successfully updated LDAP entries", + "result": True, + } + assert openldap_minion_run("ldap3.search", base=u0dn) == { + u0dn: { + "objectClass": ["person"], + "cn": ["u0"], + "sn": ["Lastname"], + }, + } + + +def test_managed_delete_all_values_all_attributes( + openldap_minion_run, openldap_minion_apply, u0dn +): + entries = [ + { + u0dn: [ + { + "delete": { + "objectClass": [], + "cn": [], + "sn": [], + "description": [], + }, + }, + ], + }, + ] + assert openldap_minion_apply("ldap.managed", entries=entries) == { + "changes": { + u0dn: { + "old": { + "objectClass": ["person"], + "cn": ["u0"], + "sn": ["Lastname"], + "description": ["desc", "another desc"], + }, + "new": None, + }, + }, + "comment": "Successfully updated LDAP entries", + "result": True, + } + assert openldap_minion_run("ldap3.search", base=u0dn) == {} + + +def test_managed_delete_remaining_values_all_attributes( + openldap_minion_run, openldap_minion_apply, u0dn +): + entries = [ + { + u0dn: [ + { + "delete": { + "objectClass": ["person"], + "cn": ["u0"], + "sn": ["Lastname"], + "description": ["desc", "another desc"], + }, + }, + ], + }, + ] + assert openldap_minion_apply("ldap.managed", entries=entries) == { + "changes": { + u0dn: { + "old": { + "objectClass": ["person"], + "cn": ["u0"], + "sn": ["Lastname"], + "description": ["desc", "another desc"], + }, + "new": None, + }, + }, + "comment": "Successfully updated LDAP entries", + "result": True, + } + assert openldap_minion_run("ldap3.search", base=u0dn) == {} + + +def test_managed_delete_not_all_values( + openldap_minion_run, openldap_minion_apply, u0dn +): + entries = [{u0dn: [{"delete": {"description": ["another desc"]}}]}] + assert openldap_minion_apply("ldap.managed", entries=entries) == { + "changes": { + u0dn: { + "old": {"description": ["desc", "another desc"]}, + "new": {"description": ["desc"]}, + }, + }, + "comment": "Successfully updated LDAP entries", + "result": True, + } + assert openldap_minion_run("ldap3.search", base=u0dn) == { + u0dn: { + "objectClass": ["person"], + "cn": ["u0"], + "sn": ["Lastname"], + "description": ["desc"], + }, + } + + +def test_managed_delete_empty_dict(openldap_minion_run, openldap_minion_apply, u0dn): + entries = [{u0dn: [{"delete": {}}]}] + assert openldap_minion_apply("ldap.managed", entries=entries) == { + "changes": {}, + "comment": "LDAP entries already set", + "result": True, + } + assert openldap_minion_run("ldap3.search", base=u0dn) == { + u0dn: { + "objectClass": ["person"], + "cn": ["u0"], + "sn": ["Lastname"], + "description": ["desc", "another desc"], + }, + } diff --git a/tests/pytests/unit/modules/test_ldap3.py b/tests/pytests/unit/modules/test_ldap3.py new file mode 100644 index 000000000000..6f94b5323efb --- /dev/null +++ b/tests/pytests/unit/modules/test_ldap3.py @@ -0,0 +1,17 @@ +import salt.modules.ldap3 as ldap3 +from tests.support.mock import patch + + +def test_change_add_value(): + with patch.object(ldap3, "modify", autospec=True): + ldap3.change( + "connect_spec", + "dn", + {"attr": ["val before"]}, + {"attr": ["val before", "val after"]}, + ) + ldap3.modify.assert_called_once_with( + "connect_spec", + "dn", + [("add", "attr", ["val after"])], + ) diff --git a/tests/pytests/unit/states/test_ldap.py b/tests/pytests/unit/states/test_ldap.py index bf57549fd9c0..ccd990a21ff3 100644 --- a/tests/pytests/unit/states/test_ldap.py +++ b/tests/pytests/unit/states/test_ldap.py @@ -1,418 +1,67 @@ -"""Test cases for the ``ldap`` state module - -This code is gross. I started out trying to remove some of the -duplicate code in the test cases, and before I knew it the test code -was an ugly second implementation. - -I'm leaving it for now, but this should really be gutted and replaced -with something sensible. -""" -import copy -import logging - -import attr import pytest -import salt.states.ldap -from salt.utils.oset import OrderedSet -from salt.utils.stringutils import to_bytes - -log = logging.getLogger(__name__) - - -# emulates the LDAP database. each key is the DN of an entry and it -# maps to a dict which maps attribute names to sets of values. -@attr.s -class LdapDB: - db = attr.ib(init=False, default=attr.Factory(dict)) - - def dummy_connect(self, connect_spec): - return _dummy_ctx() - - def dummy_search(self, connect_spec, base, scope): - if base not in self.db: - return {} - return { - base: { - attr: list(self.db[base][attr]) - for attr in self.db[base] - if len(self.db[base][attr]) - } - } - - def dummy_add(self, connect_spec, dn, attributes): - assert dn not in self.db - assert attributes - self.db[dn] = {} - for attr, vals in attributes.items(): - assert vals - self.db[dn][attr] = OrderedSet(vals) - return True - - def dummy_delete(self, connect_spec, dn): - assert dn in self.db - del self.db[dn] - return True - - def dummy_change(self, connect_spec, dn, before, after): - assert before != after - assert before - assert after - assert dn in self.db - e = self.db[dn] - assert e == before - all_attrs = OrderedSet() - all_attrs.update(before) - all_attrs.update(after) - directives = [] - for attr in all_attrs: - if attr not in before: - assert attr in after - assert after[attr] - directives.append(("add", attr, after[attr])) - elif attr not in after: - assert attr in before - assert before[attr] - directives.append(("delete", attr, ())) - else: - assert before[attr] - assert after[attr] - to_del = before[attr] - after[attr] - if to_del: - directives.append(("delete", attr, to_del)) - to_add = after[attr] - before[attr] - if to_add: - directives.append(("add", attr, to_add)) - return self.dummy_modify(connect_spec, dn, directives) - - def dummy_modify(self, connect_spec, dn, directives): - assert dn in self.db - e = self.db[dn] - for op, attr, vals in directives: - if op == "add": - assert vals - existing_vals = e.setdefault(attr, OrderedSet()) - for val in vals: - assert val not in existing_vals - existing_vals.add(val) - elif op == "delete": - assert attr in e - existing_vals = e[attr] - assert existing_vals - if not vals: - del e[attr] - continue - for val in vals: - assert val in existing_vals - existing_vals.remove(val) - if not existing_vals: - del e[attr] - elif op == "replace": - e.pop(attr, None) - e[attr] = OrderedSet(vals) - else: - raise ValueError() - return True - - def dump_db(self, d=None): - if d is None: - d = self.db - return {dn: {attr: list(d[dn][attr]) for attr in d[dn]} for dn in d} - - -@pytest.fixture -def db(): - return LdapDB() - - -@pytest.fixture -def complex_db(db): - db.db = { - "dnfoo": { - "attrfoo1": OrderedSet( - ( - b"valfoo1.1", - b"valfoo1.2", - ) - ), - "attrfoo2": OrderedSet((b"valfoo2.1",)), - }, - "dnbar": { - "attrbar1": OrderedSet( - ( - b"valbar1.1", - b"valbar1.2", - ) - ), - "attrbar2": OrderedSet((b"valbar2.1",)), - }, - } - return db - - -@pytest.fixture -def no_change_complex_db(db): - db.db = { - "dnfoo": { - "attrfoo1": OrderedSet( - ( - b"valfoo1.1", - b"valfoo1.2", - ) - ), - "attrfoo2": OrderedSet((b"valfoo2.1",)), - }, - "dnbar": { - "attrbar1": OrderedSet( - ( - b"valbar1.1", - b"valbar1.2", - ) - ), - "attrbar2": OrderedSet((b"valbar2.1",)), - }, - } - return db - - -class _dummy_ctx: - def __init__(self): - pass - - def __enter__(self): - return self - - def __exit__(self, *exc): - pass - - -@pytest.fixture -def configure_loader_modules(db): - salt_dunder = { - "ldap3.connect": db.dummy_connect, - "ldap3.search": db.dummy_search, - "ldap3.add": db.dummy_add, - "ldap3.delete": db.dummy_delete, - "ldap3.change": db.dummy_change, - "ldap3.modify": db.dummy_modify, - } - return {salt.states.ldap: {"__opts__": {"test": False}, "__salt__": salt_dunder}} - - -def _test_helper(init_db, expected_ret, replace, delete_others=False): - old = init_db.dump_db() - new = init_db.dump_db() - expected_db = copy.deepcopy(init_db.db) - for dn, attrs in replace.items(): - for attr, vals in attrs.items(): - vals = [to_bytes(val) for val in vals] - if vals: - new.setdefault(dn, {})[attr] = list(OrderedSet(vals)) - expected_db.setdefault(dn, {})[attr] = OrderedSet(vals) - elif dn in expected_db: - new[dn].pop(attr, None) - expected_db[dn].pop(attr, None) - if not expected_db.get(dn, {}): - new.pop(dn, None) - expected_db.pop(dn, None) - if delete_others: - dn_to_delete = OrderedSet() - for dn, attrs in expected_db.items(): - if dn in replace: - to_delete = OrderedSet() - for attr, vals in attrs.items(): - if attr not in replace[dn]: - to_delete.add(attr) - for attr in to_delete: - del attrs[attr] - del new[dn][attr] - if not attrs: - dn_to_delete.add(dn) - for dn in dn_to_delete: - del new[dn] - del expected_db[dn] - name = "ldapi:///" - expected_ret["name"] = name - expected_ret.setdefault("result", True) - expected_ret.setdefault("comment", "Successfully updated LDAP entries") - expected_ret.setdefault( - "changes", - { - dn: { - "old": { - attr: vals - for attr, vals in old[dn].items() - if vals != new.get(dn, {}).get(attr, ()) - } - if dn in old - else None, - "new": { - attr: vals - for attr, vals in new[dn].items() - if vals != old.get(dn, {}).get(attr, ()) - } - if dn in new - else None, - } - for dn in replace - if old.get(dn, {}) != new.get(dn, {}) - }, - ) - entries = [ - {dn: [{"replace": attrs}, {"delete_others": delete_others}]} - for dn, attrs in replace.items() - ] - actual = salt.states.ldap.managed(name, entries) - assert expected_ret == actual - assert expected_db == init_db.db - - -def _test_helper_success(db, replace, delete_others=False): - _test_helper(db, {}, replace, delete_others) - - -def _test_helper_nochange(db, replace, delete_others=False): - expected = { - "changes": {}, - "comment": "LDAP entries already set", - } - _test_helper(db, expected, replace, delete_others) - - -def _test_helper_add(db, expected_ret, add_items, delete_others=False): - old = db.dump_db() - new = db.dump_db() - expected_db = copy.deepcopy(db.db) - for dn, attrs in add_items.items(): - for attr, vals in attrs.items(): - vals = [to_bytes(val) for val in vals] - - vals.extend(old.get(dn, {}).get(attr, OrderedSet())) - vals.sort() - - if vals: - new.setdefault(dn, {})[attr] = list(OrderedSet(vals)) - expected_db.setdefault(dn, {})[attr] = OrderedSet(vals) - elif dn in expected_db: - new[dn].pop(attr, None) - expected_db[dn].pop(attr, None) - if not expected_db.get(dn, {}): - new.pop(dn, None) - expected_db.pop(dn, None) - if delete_others: - dn_to_delete = OrderedSet() - for dn, attrs in expected_db.items(): - if dn in add_items: - to_delete = OrderedSet() - for attr, vals in attrs.items(): - if attr not in add_items[dn]: - to_delete.add(attr) - for attr in to_delete: - del attrs[attr] - del new[dn][attr] - if not attrs: - dn_to_delete.add(dn) - for dn in dn_to_delete: - del new[dn] - del expected_db[dn] - name = "ldapi:///" - expected_ret["name"] = name - expected_ret.setdefault("result", True) - expected_ret.setdefault("comment", "Successfully updated LDAP entries") - expected_ret.setdefault( - "changes", - { - dn: { - "old": { - attr: vals - for attr, vals in old[dn].items() - if vals != new.get(dn, {}).get(attr, ()) - } - if dn in old - else None, - "new": { - attr: vals - for attr, vals in new[dn].items() - if vals != old.get(dn, {}).get(attr, ()) - } - if dn in new - else None, - } - for dn in add_items - if old.get(dn, {}) != new.get(dn, {}) - }, - ) - entries = [ - {dn: [{"add": attrs}, {"delete_others": delete_others}]} - for dn, attrs in add_items.items() +import salt.states.ldap as ldap +from salt.utils.ldap import AttributeValueSet + + +def _toset_testcases(): + def _gen(x): + yield x + + # Single values: + for input, want_list in [ + ("", [""]), + (b"", [""]), + (False, ["False"]), + (True, ["True"]), + (0, ["0"]), + (-1, ["-1"]), + (0xF, ["15"]), + ("🚀", ["🚀"]), + ("🚀".encode(), ["🚀"]), + (bytearray("🚀".encode()), ["🚀"]), + (memoryview("🚀".encode()), ["🚀"]), + (b"\x80", [b"\x80"]), # Intentionally invalid UTF-8. + ]: + # Single values can be provided directly or in an iterable. + for xform in [ + lambda x: x, + lambda x: [x], + lambda x: (x,), + _gen, + ]: + yield (xform(input), want_list) + + yield from [ + # Sequences: + (None, []), + ([], []), + ((), []), + (set(), []), + (["a", "b"], ["a", "b"]), + ([b"a", b"b"], ["a", "b"]), + (["a", b"b", 0], ["a", "b", "0"]), # Mix of types. + # A sequence of integers in [0, 256) can be converted to a bytes object, + # but they shouldn't be -- they should be treated as a sequence of + # integers. (Otherwise, it would be impossible to store integers unless + # one of the values was outside [0, 256).) + ([128], ["128"]), + ((128,), ["128"]), + (list("🚀".encode()), ["240", "159", "154", "128"]), + (tuple("🚀".encode()), ["240", "159", "154", "128"]), + # Invalid values: + (1.1, TypeError), + ([[]], TypeError), ] - actual = salt.states.ldap.managed(name, entries) - assert expected_ret == actual - assert expected_db == db.db - - -def _test_helper_success_add(db, add_items, delete_others=False): - _test_helper_add(db, {}, add_items, delete_others) - - -def test_managed_empty(db): - name = "ldapi:///" - expected = { - "name": name, - "changes": {}, - "result": True, - "comment": "LDAP entries already set", - } - actual = salt.states.ldap.managed(name, {}) - assert expected == actual - - -def test_managed_add_entry(db): - _test_helper_success_add(db, {"dummydn": {"foo": ["bar", "baz"]}}) - - -def test_managed_add_attr(complex_db): - _test_helper_success_add(complex_db, {"dnfoo": {"attrfoo1": ["valfoo1.3"]}}) - _test_helper_success_add(complex_db, {"dnfoo": {"attrfoo4": ["valfoo4.1"]}}) - - -def test_managed_replace_attr(complex_db): - _test_helper_success(complex_db, {"dnfoo": {"attrfoo3": ["valfoo3.1"]}}) - - -def test_managed_simplereplace(complex_db): - _test_helper_success(complex_db, {"dnfoo": {"attrfoo1": ["valfoo1.3"]}}) - - -def test_managed_deleteattr(complex_db): - _test_helper_success(complex_db, {"dnfoo": {"attrfoo1": []}}) - - -def test_managed_deletenonexistattr(no_change_complex_db): - _test_helper_nochange(no_change_complex_db, {"dnfoo": {"dummyattr": []}}) - - -def test_managed_deleteentry(complex_db): - _test_helper_success(complex_db, {"dnfoo": {}}, True) - - -def test_managed_deletenonexistentry(no_change_complex_db): - _test_helper_nochange(no_change_complex_db, {"dummydn": {}}, True) - - -def test_managed_deletenonexistattrinnonexistentry(no_change_complex_db): - _test_helper_nochange(no_change_complex_db, {"dummydn": {"dummyattr": []}}) - - -def test_managed_add_attr_delete_others(complex_db): - _test_helper_success(complex_db, {"dnfoo": {"dummyattr": ["dummyval"]}}, True) - - -def test_managed_no_net_change(no_change_complex_db): - _test_helper_nochange( - no_change_complex_db, {"dnfoo": {"attrfoo1": ["valfoo1.1", "valfoo1.2"]}} - ) -def test_managed_repeated_values(db): - _test_helper_success(db, {"dummydn": {"dummyattr": ["dummyval", "dummyval"]}}) +@pytest.mark.parametrize("input,want_list", _toset_testcases()) +def test__toset(input, want_list): + if isinstance(want_list, type): + with pytest.raises(want_list): + got = ldap._toset("attr", input) + else: + want = AttributeValueSet("attr", want_list) + got = ldap._toset("attr", input) + assert got == want + assert list(got) == want_list diff --git a/tests/pytests/unit/utils/jinja/test_custom_extensions.py b/tests/pytests/unit/utils/jinja/test_custom_extensions.py index 1fa3c9c678a3..0d6c84f6744f 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 @@ -131,10 +132,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_ldap.py b/tests/pytests/unit/utils/test_ldap.py new file mode 100644 index 000000000000..849cc3554f35 --- /dev/null +++ b/tests/pytests/unit/utils/test_ldap.py @@ -0,0 +1,86 @@ +import random + +import pytest + +from salt.utils.ldap import AttributeValueSet, LDAPError + + +def test_attribute_value_set_empty(): + assert len(AttributeValueSet("attr")) == 0 + + +def test_attribute_value_set_no_duplicates(): + assert list(AttributeValueSet("attr", ["a", "a"])) == ["a"] + + +def test_attribute_value_set_ordered(): + # Filter the values through a set to avoid duplicates. + v = list({str(random.getrandbits(32)) for x in range(100)}) + assert len(v) > 90 + # Avoid unintended correlation with set()'s iteration order. + random.shuffle(v) + assert list(AttributeValueSet("attr", v)) == v + + +def test_attribute_value_set_eq(): + s = AttributeValueSet("attr", ["a", "b"]) + assert s is not None + assert s != [] + assert s != AttributeValueSet("attr") + assert s != AttributeValueSet("attr", ["x", "y"]) + assert s == s + assert s == ["a", "b"] + assert s == {"a", "b"} + assert s == AttributeValueSet("attr", ["a", "b"]) + + +def test_attribute_value_set_eq_unordered(): + s = AttributeValueSet("attr", ["a", "b"]) + assert s == ["b", "a"] + assert s == {"b", "a"} + assert s == AttributeValueSet("attr", ["b", "a"]) + + +# attr: Attribute Name. +# input: Input value. +# v: Wanted stored value. +# vx: Wanted encoded value. +@pytest.mark.parametrize( + "attr,input,v,vx", + [ + # str inputs are stored as-is. + ("attr", "🚀", "🚀", b"\xf0\x9f\x9a\x80"), + ("unicodePwd", "🚀", "🚀", b'"\x00=\xd8\x80\xde"\x00'), + # bytes inputs that can be decoded are decoded. + ("attr", b"\xf0\x9f\x9a\x80", "🚀", b"\xf0\x9f\x9a\x80"), + ("unicodePwd", b'"\x00=\xd8\x80\xde"\x00', "🚀", b'"\x00=\xd8\x80\xde"\x00'), + # bytes inputs that can't be decoded are stored as-is. + ("attr", b"\x80", b"\x80", b"\x80"), + ("unicodePwd", b"x", b"x", b"x"), + ("unicodePwd", b'"x"', b'"x"', b'"x"'), # Not utf-16-le encoded. + ("unicodePwd", b"x\x00", b"x\x00", b"x\x00"), # Missing double quotes. + # Non-bytes, non-str inputs. + ("attr", [], "", b""), + ("attr", (), "", b""), + ("attr", [112], "p", b"p"), + ("attr", [128], b"\x80", b"\x80"), + ("attr", bytearray(b"p"), "p", b"p"), + ("attr", memoryview(b"p"), "p", b"p"), + ], +) +def test_attribute_value_set_encode_decode(attr, input, v, vx): + s = AttributeValueSet(attr, [input]) + assert v in s + assert vx in s + assert s == [v] + assert s == [vx] + assert list(s) == [v] + assert s.encode() == [vx] + + +def test_ldap_error(): + cause = RuntimeError("cause") + err = LDAPError("foo", cause) + assert isinstance(err, Exception) + assert err.cause is cause + assert "foo" in str(err) diff --git a/tests/pytests/unit/utils/test_yaml.py b/tests/pytests/unit/utils/test_yaml.py new file mode 100644 index 000000000000..8558c93e1240 --- /dev/null +++ b/tests/pytests/unit/utils/test_yaml.py @@ -0,0 +1,333 @@ +import collections +import datetime +import random +import re +import textwrap + +import pytest +import yaml +from yaml.constructor import ConstructorError + +import salt.utils.files +import salt.utils.yaml as salt_yaml +from salt.utils.odict import OrderedDict +from tests.support.mock import mock_open, patch + + +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" + + +def test_dump_indented(): + data = {"foo": ["bar"]} + got = salt_yaml.dump( + data, + Dumper=salt_yaml.IndentedSafeOrderedDumper, + default_flow_style=False, + ) + want = "foo:\n - bar\n" + assert got == want + + +@pytest.mark.parametrize("dictcls", [OrderedDict, collections.OrderedDict]) +@pytest.mark.parametrize( + "dumpercls", + [ + salt_yaml.OrderedDumper, + salt_yaml.SafeOrderedDumper, + salt_yaml.IndentedSafeOrderedDumper, + ], +) +def test_dump_omap(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) + d = dictcls((k, i) for i, k in enumerate(keys)) + # 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}: {i}\n" for i, k in enumerate(keys)) + got = salt_yaml.dump(d, Dumper=dumpercls, default_flow_style=False) + assert got == want + + +@pytest.mark.parametrize( + "dumpercls", + [ + salt_yaml.OrderedDumper, + salt_yaml.SafeOrderedDumper, + salt_yaml.IndentedSafeOrderedDumper, + ], +) +def test_dump_timestamp(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"""!!timestamp (['"]?)2022-10-21[T ]18:16:03.10*-04:00\1\n""" + 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 + + +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")]) + + +def test_load_omap(): + """Test OrderedDict values via the YAML ``!!omap`` tag. + + This test uses random keys to ensure that iteration order does not + coincidentally match. The generated YAML looks like this: + + .. code-block:: yaml + + !!omap + - 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) + want_items = [(k, i) for i, k in enumerate(keys)] + yaml = "!!omap\n" + "".join(f"- {k}: {v}\n" for k, v in want_items) + got = salt_yaml.load(yaml) + assert isinstance(got, collections.OrderedDict) + assert got == collections.OrderedDict(want_items) + assert list(got.items()) == want_items + + +def test_load_omap_empty(): + got = salt_yaml.load("!!omap []\n") + assert isinstance(got, collections.OrderedDict) + assert got == collections.OrderedDict() + assert list(got.items()) == [] + + +@pytest.mark.parametrize( + "input", + [ + "!!omap {}\n", # Not a sequence node. + "!!omap\n- this is not a mapping node\n", + "!!omap\n- k1: 0\n k2: multiple entries in mapping node\n", + "!!omap [{}]\n", # Mapping node has no entries. + "!!omap\n- duplicate key: 0\n- duplicate key: 1\n", + ], +) +def test_load_omap_invalid(input): + with pytest.raises(ConstructorError): + salt_yaml.load(input) + + +def test_load_untagged_omaplike_is_seq(): + # 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. + got = salt_yaml.load("- a: 0\n- b: 1\n") + assert not isinstance(got, collections.OrderedDict) + assert got == [{"a": 0}, {"b": 1}] + + +@pytest.mark.parametrize( + "input_yaml,want", + [ + ( + "!!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)), + ), + ), + ("2022-10-21T18:16:03.1-04:00", "2022-10-21T18:16:03.1-04:00"), + ], +) +def test_load_timestamp(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/ldap.py b/tests/support/pytest/ldap.py new file mode 100644 index 000000000000..eff5f236d523 --- /dev/null +++ b/tests/support/pytest/ldap.py @@ -0,0 +1,392 @@ +import logging +import os +import re +import tempfile +import textwrap +import time +from typing import Optional + +import attr +import importlib_metadata +import pytest +import saltfactories.cli.salt +from pytestshellutils.utils import ports +from saltfactories.daemons.container import SaltMinion +from saltfactories.utils import random_string + +import salt.utils.yaml +from tests.support.runtests import RUNTIME_VARS + +_outarg_regex = re.compile(r"^--out(?:put)?(?:=(.*))?$") +log = logging.getLogger(__name__) + + +@attr.s(kw_only=True) +class Salt(saltfactories.cli.salt.Salt): + """``salt`` command-line factory that defaults to YAML instead of JSON. + + Unlike JSON, YAML supports bytes objects via the ``!!binary`` tag. + + TODO: Modify saltfactories.bases.SaltCli to support --out=yaml and delete + this class. + """ + + out: str = "yaml" + minion_tgt: Optional[str] = None + + def cmdline(self, *args, minion_tgt=None, **kwargs): + self.minion_tgt = minion_tgt + has_out = False + for i, arg in enumerate(args): + m = _outarg_regex.fullmatch(arg) + if not m: + continue + has_out = True + fmt = m.group(1) + arg_next = args[i + 1] if len(args) > i else None + self.out = fmt if fmt is not None else arg_next + if not self.out: + raise ValueError("missing outputter name") + break + if not has_out: + args = ["--out", self.out] + list(args) + return super().cmdline(*args, minion_tgt=minion_tgt, **kwargs) + + def process_output(self, stdout, stderr, cmdline=None): + try: + obj = self._decode(stdout) + except NotImplementedError: + return super().process_output(stdout, stderr, cmdline) + t = self.minion_tgt + if t is not None and t != "*" and not isinstance(obj, str) and t in obj: + obj = obj[t] + return (stdout, stderr, obj) + + def _decode(self, out): + if self.out != "yaml": + raise NotImplementedError(f"unsupported outputter: {self.out!r}") + # yaml.load() (when using CLoader at least) does not accept subclasses + # of str or bytes, but out is probably a + # pytestshellutils.utils.processes.MatchString which is a subclass of + # str. Convert subclasses of str/bytes to actual str/bytes objects. + if isinstance(out, str) and type(out) != str: + out = str(out) + elif isinstance(out, bytes) and type(out) != bytes: + out = bytes(out) + return salt.utils.yaml.load(out) + + +@pytest.fixture(scope="module") +def salt_yaml_cli(salt_master): + assert salt_master.is_running() + return salt_master.salt_cli(factory_class=Salt) + + +@attr.s(kw_only=True, slots=True) +class SlapdMinion(SaltMinion): + """Minion in a Docker container with OpenLDAP installed and running.""" + + base: str = "dc=example,dc=org" + password: str = "adminpassword" + port: int = attr.ib(default=attr.Factory(ports.get_unused_localhost_port)) + user: str = "admin" + + @property + def userdn(self) -> str: + return f"cn={self.user},{self.base}" + + @property + def uri(self) -> str: + """LDAP URI for use inside and outside the container. + + This URI works both inside and outside because the network mode is set + to "host". + """ + return f"ldap://localhost:{self.port}" + + @property + def connect_spec(self): + return { + "url": self.uri, + "bind": { + "method": "simple", + "dn": self.userdn, + "password": self.password, + }, + } + + @classmethod + def default_config(cls, *args, **kwargs): + cfg = super().default_config(*args, **kwargs) + + # Requirements on the minion daemon user: + # + # * The `RUNTIME_VARS.CODE_DIR` directory must be readable by the + # minion daemon user, otherwise Python will fail to import required + # modules during minion start-up. + # + # * As of 2022-10-05, the minion code requires the minion daemon user + # to have an entry in the password database (`/etc/passwd`). + # + # The current user (`os.getuid()`) can access `RUNTIME_VARS.CODE_DIR`, + # but is unlikely to have an entry in the Docker container's password + # database. We could pick a user that is known to exist in the image, + # but that user might not have access to `RUNTIME_VARS.CODE_DIR`. The + # easiest way to ensure that both requirements are satisfied is to run + # the minion as root. If the password database entry requirement is + # removed, the minion can be run as the current user by replacing + # `"root"` here with `os.getuid()` and changing `cmdline()` to pass + # `["--user", f"{os.getuid()}:{os.getgid()}"]`. + cfg.setdefault("user", "root") + + return cfg + + def __attrs_post_init__(self): + super().__attrs_post_init__() + self.container_run_kwargs.setdefault("environment", {}).update( + { + "LDAP_ADMIN_PASSWORD": self.password, + "LDAP_ADMIN_USERNAME": self.user, + "LDAP_PORT_NUMBER": self.port, + "LDAP_ROOT": self.base, + }, + ) + # Note that the target directories have the same names as the source + # directories. This avoids the need to translate path names. + self.container_run_kwargs.setdefault("volumes", {}).update( + { + # Bind mount the checked-out source code into the Docker + # container so that a minion daemon started inside the container + # will run the code to be tested. + RUNTIME_VARS.CODE_DIR: { + "bind": RUNTIME_VARS.CODE_DIR, + "mode": "z", + }, + RUNTIME_VARS.TMP: {"bind": RUNTIME_VARS.TMP, "mode": "z"}, + }, + ) + # Use host network mode so that we don't have to define a network and + # configure the minion with the master's IP address (we can simply use + # localhost). Note that port mapping (the "ports" option) is not + # applicable in host mode. + self.container_run_kwargs["network_mode"] = "host" + self.check_ports[self.port] = self.port + self.container_start_check(self.__start_check) + + def __run_unpriv(self, args): + ret = self.run(*args, user=f"{os.getuid()}:{os.getgid()}") + cmd_str = " ".join(args) + msg = f"command '{cmd_str}' returned {ret.returncode}" + if ret.stdout: + msg += f"\n >>>>> STDOUT >>>>>\n{ret.stdout}" + if not msg.endswith("\n"): + msg += "\n" + msg += " <<<<< STDOUT <<<<<" + if ret.stderr: + msg += f"\n >>>>> STDERR >>>>>\n{ret.stderr}" + if not msg.endswith("\n"): + msg += "\n" + msg += " <<<<< STDERR <<<<<" + if ret.returncode != 0: + raise Exception(msg) + log.debug(msg) + return ret + + def __slapd_running(self) -> bool: + try: + self.__run_unpriv( + [ + "ldapsearch", + "-H", + self.uri, + "-x", + "-D", + self.userdn, + "-w", + self.password, + "-b", + self.base, + ], + ) + log.debug("slapd is running") + return True + except Exception as ex: # pylint: disable=broad-except + log.debug(ex) + log.debug("slapd is not running yet") + return False + + def __start_check(self, timeout_at) -> bool: + while time.time() <= timeout_at: + if self.__slapd_running(): + return True + time.sleep(1) + return False + + def cmdline(self, *args): + cmd = list(super().cmdline(*args)) + # See the comment in `default_config()` for why the minion runs as root. + cmd[2:2] = ["--user", "0:0"] + return cmd + + def ldapadd(self, ldif): + if isinstance(ldif, str): + ldif = ldif.encode() + tmpf = tempfile.NamedTemporaryFile(dir=RUNTIME_VARS.TMP, suffix=".ldif") + with tmpf as f: + f.write(ldif) + f.flush() + self.__run_unpriv(["cat", f.name]) + self.__run_unpriv( + [ + "ldapadd", + "-H", + self.uri, + "-x", + "-D", + self.userdn, + "-w", + self.password, + "-f", + # RUNTIME_VARS.TMP is bind-mounted to RUNTIME_VARS.TMP so + # the path name is the same inside and outside the + # container. + f.name, + ], + ) + + def ldapdelete(self, *dns, recursive=False): + cmd = [ + "ldapdelete", + "-H", + self.uri, + "-x", + "-D", + self.userdn, + "-w", + self.password, + ] + if recursive: + cmd.append("-r") + cmd.extend(dns) + self.__run_unpriv(cmd) + + +@pytest.fixture(scope="module") +def openldap_minion(salt_master, salt_yaml_cli, salt_key_cli): + name = random_string("openldap-minion-") + c = salt_master.salt_minion_daemon( + name, + image="ghcr.io/saltstack/salt-ci-containers/openldap-minion:latest", + factory_class=SlapdMinion, + pull_before_start=True, + skip_on_pull_failure=True, + skip_if_docker_client_not_connectable=True, + ) + # For some reason passing this as an argument to the salt_minion_daemon() + # call above fails with "TypeError: type object got multiple values for + # keyword argument 'python_executable'". + c.python_executable = "python3" + + # Workaround for: + # https://github.com/saltstack/pytest-salt-factories/issues/139 + def _after_start(): + v = importlib_metadata.version("pytest-salt-factories") + for cmd in [ + ("install_packages", "python3-pip"), + ("pip", "install", f"pytest-salt-factories=={v}"), + ]: + ret = c.run(*cmd, user="0:0") + assert ret.returncode == 0, ret + c.after_start(_after_start) + + log.debug(f"starting OpenLDAP minion container {name}...") + with c.started(): + assert c.is_running() + ret = salt_yaml_cli.run("test.ping", minion_tgt=c.id) + assert ret.returncode == 0, ret + assert ret.data is True + yield c + log.debug(f"stopping OpenLDAP minion container {name}...") + assert not c.is_running() + salt_key_cli.run("-y", "-d", c.id) + log.debug(f"OpenLDAP minion container {name} stopped") + + +@pytest.fixture(scope="module") +def openldap_minion_run(openldap_minion, salt_yaml_cli): + def _run(fn, *args, **kwargs): + if fn.startswith("ldap3."): + kwargs.setdefault("connect_spec", openldap_minion.connect_spec) + ret = salt_yaml_cli.run( + fn, + *args, + minion_tgt=openldap_minion.id, + **kwargs, + ) + assert ret.returncode == 0, ret + return ret.data + + yield _run + + +@pytest.fixture(scope="module") +def openldap_minion_apply(salt_master, openldap_minion, openldap_minion_run): + def _apply(fn, **kwargs): + has_name = "name" in kwargs + name = kwargs.pop("name", "x") + if fn.startswith("ldap."): + kwargs.setdefault("connect_spec", openldap_minion.connect_spec) + sls_data = { + name: { + fn: [{k: v} for k, v in kwargs.items()], + }, + } + sls_yaml = salt.utils.yaml.dump(sls_data) + with salt_master.state_tree.base.temp_file("test_state.sls", sls_yaml): + ret = openldap_minion_run("state.apply", "test_state") + # Normalize the return value for easier checking. + assert len(ret) == 1 + (ret,) = ret.values() + assert ret["name"] == name + entries = ["changes", "comment", "result"] + if has_name: + entries.append("name") + ret = {k: ret[k] for k in entries if k in ret} + return ret + + yield _apply + + +@pytest.fixture +def subtree(openldap_minion, request): + dc = request.function.__name__ + dn = f"dc={dc},{openldap_minion.base}" + log.debug(f"Creating and populating temporary subtree {dn}...") + openldap_minion.ldapadd( + textwrap.dedent( + f"""\ + dn: {dn} + objectClass: dcObject + objectClass: organization + dc: {dc} + o: {dc} + + dn: cn=u0,{dn} + objectClass: person + cn: u0 + sn: Lastname + description: desc + description: another desc + """, + ), + ) + log.debug("Created temporary subtree") + yield dn + log.debug("Cleaning up temporary subtree...") + openldap_minion.ldapdelete(dn, recursive=True) + log.debug("Temporary subtree cleaned up") + + +@pytest.fixture +def u0dn(subtree): + yield f"cn=u0,{subtree}" 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