Allow for toggling of LDAP attribute ordering

salt/states/ldap.py

@@ -137,6 +137,13 @@ def managed(name, entries, connect_spec=None):
             directive or the ``'add'`` directive with an empty value
             list.
 
+        * ``'strict_order'``
+            Boolean indicating whether the ordering of attribute values is important.
+            With some LDAP implementations and attributes this directive should be set
+            to ``False`` when using the ``replace``` directive to ensure idempotency if
+            attributes get re-arranged by the LDAP server beyond administrative control.
+            Defaults to ``True``.
+
         * ``'default'``
             A dict mapping an attribute name to an iterable of default
             values for that attribute.  If the attribute already
@@ -347,8 +354,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)
+    # convert the value sets to lists before returning them to the user
     for dn in success_dn_set:
         o = changed_old.get(dn, {})
         n = changed_new.get(dn, {})
@@ -442,6 +448,8 @@ def _process_entries(l, entries):
             # process the directives
             entry_status = {
                 "delete_others": False,
+                # False could be a better default for "strict_order", but unsure if it would break existing implementations
+                "strict_order": True,
                 "mentioned_attributes": set(),
             }
             for directives in directives_seq:
@@ -453,6 +461,9 @@ def _process_entries(l, entries):
                         to_delete.add(attr)
                 for attr in to_delete:
                     del newe[attr]
+            if not entry_status["strict_order"]:
+                for attr in olde:
+                    olde[attr] = OrderedSet(sorted(olde[attr]))
     return old, new
 
 
@@ -468,8 +479,8 @@ def _update_entry(entry, status, directives):
         A dict mapping directive types to directive-specific state
     """
     for directive, state in directives.items():
-        if directive == "delete_others":
-            status["delete_others"] = state
+        if directive in ["delete_others", "strict_order"]:
+            status[directive] = state
             continue
         for attr, vals in state.items():
             status["mentioned_attributes"].add(attr)
@@ -490,7 +501,10 @@ def _update_entry(entry, status, directives):
             elif directive == "replace":
                 entry.pop(attr, None)
                 if vals:
-                    entry[attr] = vals
+                    if status["strict_order"] is True:
+                        entry[attr] = vals
+                    else:
+                        entry[attr] = OrderedSet(sorted(vals))
             else:
                 raise ValueError("unknown directive: " + directive)
 

tests/pytests/unit/states/test_ldap.py

@@ -200,16 +200,24 @@ def configure_loader_modules(db):
     return {salt.states.ldap: {"__opts__": {"test": False}, "__salt__": salt_dunder}}
 
 
-def _test_helper(init_db, expected_ret, replace, delete_others=False):
+def _test_helper(
+    init_db,
+    expected_ret,
+    replace,
+    delete_others=False,
+    strict_order=True,
+):
     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]
+            vals = sorted([to_bytes(val) for val in vals])
             if vals:
                 new.setdefault(dn, {})[attr] = list(OrderedSet(vals))
-                expected_db.setdefault(dn, {})[attr] = OrderedSet(vals)
+                expected_db.setdefault(dn, {})[attr] = sorted(
+                    list(OrderedSet(vals)),
+                )
             elif dn in expected_db:
                 new[dn].pop(attr, None)
                 expected_db[dn].pop(attr, None)
@@ -251,7 +259,7 @@ def _test_helper(init_db, expected_ret, replace, delete_others=False):
                 ),
                 "new": (
                     {
-                        attr: vals
+                        attr: sorted(vals)
                         for attr, vals in new[dn].items()
                         if vals != old.get(dn, {}).get(attr, ())
                     }
@@ -264,15 +272,21 @@ def _test_helper(init_db, expected_ret, replace, delete_others=False):
         },
     )
     entries = [
-        {dn: [{"replace": attrs}, {"delete_others": delete_others}]}
+        {
+            dn: [
+                {"replace": attrs},
+                {"delete_others": delete_others},
+                {"strict_order": strict_order},
+            ]
+        }
         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):
+def _test_helper_success(db, replace, delete_others=False, strict_order=True):
     _test_helper(db, {}, replace, delete_others)
 
 
@@ -385,8 +399,20 @@ def test_managed_add_attr(complex_db):
     _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_replace_attr_not_strict(complex_db):
+    _test_helper_success(
+        complex_db,
+        {"dnfoo": {"attrfoo3": ["valfoo3.2", "valfoo3.1", "valfoo3.0"]}},
+        strict_order=False,
+    )
+
+
+def test_managed_replace_attr_strict(complex_db):
+    _test_helper_success(
+        complex_db,
+        {"dnfoo": {"attrfoo3": ["valfoo3.2", "valfoo3.1", "valfoo.3.0"]}},
+        strict_order=True,
+    )
 
 
 def test_managed_simplereplace(complex_db):