PR#50333: Ticket 50327 - Add replication conflict entry support to lib389/CLI - 389-ds-base

389-ds-base

#50333 Ticket 50327 - Add replication conflict entry support to lib389/CLI

Closed 3 years ago by spichugi. Opened 5 years ago by mreynolds.

mreynolds/389-ds-base ticket50327 into master

Ticket 50327 - Add replication conflict entry support to lib389/CLI

Mark Reynolds • 5 years ago

4f7c05e

src/lib389/cli/dsconf

file modified

		`@@ -31,6 +31,7 @@`
		`from lib389.cli_conf import backup as cli_backup`
		`from lib389.cli_conf import replication as cli_replication`
		`from lib389.cli_conf import chaining as cli_chaining`
		`+ from lib389.cli_conf import conflicts as cli_repl_conflicts`
		`from lib389.cli_base import disconnect_instance, connect_instance`
		`from lib389.cli_base.dsrc import dsrc_to_ldap, dsrc_arg_concat`
		`from lib389.cli_base import setup_script_logger`
		`@@ -87,6 +88,7 @@`
		`cli_replication.create_parser(subparsers)`
		`cli_sasl.create_parser(subparsers)`
		`cli_schema.create_parser(subparsers)`
		`+ cli_repl_conflicts.create_parser(subparsers)`

		`argcomplete.autocomplete(parser)`

src/lib389/lib389/_mapped_object.py

file modified

+52 -35

		`@@ -1,5 +1,5 @@`
		`# --- BEGIN COPYRIGHT BLOCK ---`
		`- # Copyright (C) 2016 Red Hat, Inc.`
		`+ # Copyright (C) 2019 Red Hat, Inc.`
		`# Copyright (C) 2019 William Brown <william@blackhats.net.au>`
		`# All rights reserved.`
		`#`
		`@@ -129,7 +129,9 @@`
		`:returns: Entry object`
		`"""`

		`- return self._instance.search_ext_s(self._dn, ldap.SCOPE_BASE, self._object_filter, attrlist=["*"], serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure')[0]`
		`+ return self._instance.search_ext_s(self._dn, ldap.SCOPE_BASE, self._object_filter, attrlist=["*"],`
		`+ serverctrls=self._server_controls, clientctrls=self._client_controls,`
		`+ escapehatch='i am sure')[0]`

		`def exists(self):`
		`"""Check if the entry exists`
		`@@ -138,19 +140,22 @@`
		`"""`

		`try:`
		`- self._instance.search_ext_s(self._dn, ldap.SCOPE_BASE, self._object_filter, attrsonly=1, serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure')`
		`+ self._instance.search_ext_s(self._dn, ldap.SCOPE_BASE, self._object_filter, attrsonly=1,`
		`+ serverctrls=self._server_controls, clientctrls=self._client_controls,`
		`+ escapehatch='i am sure')`
		`except ldap.NO_SUCH_OBJECT:`
		`return False`

		`return True`

		`- def display(self):`
		`+ def display(self, attrlist=['*']):`
		`"""Get an entry but represent it as a string LDIF`

		`:returns: LDIF formatted string`
		`"""`
		`-`
		`- e = self._instance.search_ext_s(self._dn, ldap.SCOPE_BASE, self._object_filter, attrlist=["*"], serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure')[0]`
		`+ e = self._instance.search_ext_s(self._dn, ldap.SCOPE_BASE, self._object_filter, attrlist=attrlist,`
		`+ serverctrls=self._server_controls, clientctrls=self._client_controls,`
		`+ escapehatch='i am sure')[0]`
		`return e.__repr__()`

		`def display_attr(self, attr):`
		`@@ -227,7 +232,9 @@`
		`raise ValueError("Invalid state. Cannot get presence on instance that is not ONLINE")`
		`self._log.debug("%s present(%r) %s" % (self._dn, attr, value))`

		`- e = self._instance.search_ext_s(self._dn, ldap.SCOPE_BASE, self._object_filter, attrlist=[attr, ], serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure')[0]`
		`+ e = self._instance.search_ext_s(self._dn, ldap.SCOPE_BASE, self._object_filter, attrlist=[attr, ],`
		`+ serverctrls=self._server_controls, clientctrls=self._client_controls,`
		`+ escapehatch='i am sure')[0]`
		`values = self.get_attr_vals_bytes(attr)`
		`self._log.debug("%s contains %s" % (self._dn, values))`

		`@@ -280,7 +287,8 @@`
		`else:`
		`value = [ensure_bytes(arg[1])]`
		`mods.append((ldap.MOD_REPLACE, ensure_str(arg[0]), value))`
		`- return self._instance.modify_ext_s(self._dn, mods, serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure')`
		`+ return self._instance.modify_ext_s(self._dn, mods, serverctrls=self._server_controls,`
		`+ clientctrls=self._client_controls, escapehatch='i am sure')`

		`# This needs to work on key + val, and key`
		`def remove(self, key, value):`
		`@@ -375,7 +383,8 @@`
		`value = [ensure_bytes(value)]`

		`return self._instance.modify_ext_s(self._dn, [(action, key, value)],`
		`- serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure')`
		`+ serverctrls=self._server_controls, clientctrls=self._client_controls,`
		`+ escapehatch='i am sure')`

		`def apply_mods(self, mods):`
		`"""Perform modification operation using several mods at once`
		`@@ -415,7 +424,7 @@`
		`raise ValueError('Too many arguments in the mod op')`
		`return self._instance.modify_ext_s(self._dn, mod_list, serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure')`

		`- def _unsafe_compare_attribute(self, attr, values):`
		`+ def _unsafe_compare_attribute(self, other):`
		`"""Compare two attributes from two objects. This is currently marked unsafe as it's`
		`not complete yet.`

		`@@ -430,12 +439,7 @@`

		`To allow schema aware checking, we need to call ldap compare extop here.`
		`"""`
		`- return all([`
		`- self._instance.compare_ext_s(self._dn, attr, value, serverctrls=self._server_controls,`
		`- clientctrls=self._client_controls, escapehatch='i am sure')`
		`- for value in values`
		`- ])`
		`-`
		`+ pass`

		`@classmethod`
		`def compare(cls, obj1, obj2):`
		`@@ -474,14 +478,10 @@`
		`if set(obj1_attrs.keys()) != set(obj2_attrs.keys()):`
		`obj1._log.debug("%s != %s" % (obj1_attrs.keys(), obj2_attrs.keys()))`
		`return False`
		`- obj1._log.debug(sorted(obj1_attrs.keys()))`
		`# Check the values of each key`
		`# using obj1_attrs.keys() because obj1_attrs.iterkleys() is not supported in python3`
		`for key in obj1_attrs.keys():`
		`- # Check if they are offline/online?`
		`- # if set(obj1_attrs[key]) != set(obj2_attrs[key]):`
		`- obj1._log.debug("checking %s: %s ..." % (key, obj2_attrs[key]))`
		`- if not obj1._unsafe_compare_attribute(key, obj2_attrs[key]):`
		`+ if set(obj1_attrs[key]) != set(obj2_attrs[key]):`
		`obj1._log.debug(" v-- %s != %s" % (key, key))`
		`obj1._log.debug("%s != %s" % (obj1_attrs[key], obj2_attrs[key]))`
		`return False`
		`@@ -503,7 +503,7 @@`
		`cx = [x.lower() for x in self._compare_exclude]`
		`compare_attrs = set(all_attrs_lower.keys()) - set(cx)`

		`- compare_attrs_dict = {attr.lower():all_attrs_lower[attr] for attr in compare_attrs}`
		`+ compare_attrs_dict = {attr.lower(): all_attrs_lower[attr] for attr in compare_attrs}`

		`return compare_attrs_dict`

		`@@ -518,7 +518,9 @@`
		`raise ValueError("Invalid state. Cannot get properties on instance that is not ONLINE")`
		`else:`
		`# retrieving real(*) and operational attributes(+)`
		`- attrs_entry = self._instance.search_ext_s(self._dn, ldap.SCOPE_BASE, self._object_filter, attrlist=["*", "+"], serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure')[0]`
		`+ attrs_entry = self._instance.search_ext_s(self._dn, ldap.SCOPE_BASE, self._object_filter,`
		`+ attrlist=["*", "+"], serverctrls=self._server_controls,`
		`+ clientctrls=self._client_controls, escapehatch='i am sure')[0]`
		`# getting dict from 'entry' object`
		`attrs_dict = attrs_entry.data`
		`# Should we normalise the attr names here to lower()?`
		`@@ -530,14 +532,18 @@`
		`if self._instance.state != DIRSRV_STATE_ONLINE:`
		`raise ValueError("Invalid state. Cannot get properties on instance that is not ONLINE")`
		`else:`
		`- entry = self._instance.search_ext_s(self._dn, ldap.SCOPE_BASE, self._object_filter, attrlist=keys, serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure')[0]`
		`+ entry = self._instance.search_ext_s(self._dn, ldap.SCOPE_BASE, self._object_filter,`
		`+ attrlist=keys, serverctrls=self._server_controls,`
		`+ clientctrls=self._client_controls, escapehatch='i am sure')[0]`
		`return entry.getValuesSet(keys)`

		`def get_attrs_vals_utf8(self, keys, use_json=False):`
		`self._log.debug("%s get_attrs_vals_utf8(%r)" % (self._dn, keys))`
		`if self._instance.state != DIRSRV_STATE_ONLINE:`
		`raise ValueError("Invalid state. Cannot get properties on instance that is not ONLINE")`
		`- entry = self._instance.search_ext_s(self._dn, ldap.SCOPE_BASE, self._object_filter, attrlist=keys, serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure')[0]`
		`+ entry = self._instance.search_ext_s(self._dn, ldap.SCOPE_BASE, self._object_filter, attrlist=keys,`
		`+ serverctrls=self._server_controls, clientctrls=self._client_controls,`
		`+ escapehatch='i am sure')[0]`
		`vset = entry.getValuesSet(keys)`
		`r = {}`
		`for (k, vo) in vset.items():`
		`@@ -554,7 +560,9 @@`
		`else:`
		`# It would be good to prevent the entry code intercepting this ....`
		`# We have to do this in this method, because else we ignore the scope base.`
		`- entry = self._instance.search_ext_s(self._dn, ldap.SCOPE_BASE, self._object_filter, attrlist=[key], serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure')[0]`
		`+ entry = self._instance.search_ext_s(self._dn, ldap.SCOPE_BASE, self._object_filter,`
		`+ attrlist=[key], serverctrls=self._server_controls,`
		`+ clientctrls=self._client_controls, escapehatch='i am sure')[0]`
		`vals = entry.getValues(key)`
		`if use_json:`
		`result = {key: []}`
		`@@ -572,7 +580,9 @@`
		`# In the future, I plan to add a mode where if local == true, we`
		`# can use get on dse.ldif to get values offline.`
		`else:`
		`- entry = self._instance.search_ext_s(self._dn, ldap.SCOPE_BASE, self._object_filter, attrlist=[key], serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure')[0]`
		`+ entry = self._instance.search_ext_s(self._dn, ldap.SCOPE_BASE, self._object_filter,`
		`+ attrlist=[key], serverctrls=self._server_controls,`
		`+ clientctrls=self._client_controls, escapehatch='i am sure')[0]`
		`return entry.getValue(key)`

		`def get_attr_val_bytes(self, key, use_json=False):`
		`@@ -679,7 +689,7 @@`
		`pass`

		`# Modifies the DN of an entry to the new fqdn provided`
		`- def rename(self, new_rdn, newsuperior=None):`
		`+ def rename(self, new_rdn, newsuperior=None, deloldrdn=True):`
		`"""Renames the object within the tree.`

		`If you provide a newsuperior, this will move the object in the tree.`
		`@@ -700,9 +710,12 @@`
		`# and the superior as the base (if it changed)`
		`if self._protected:`
		`return`
		`- self._instance.rename_s(self._dn, new_rdn, newsuperior, serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure')`
		`+`
		`+ self._instance.rename_s(self._dn, new_rdn, newsuperior,`
		`+ serverctrls=self._server_controls, clientctrls=self._client_controls,`
		`+ delold=deloldrdn, escapehatch='i am sure')`
		`search_base = self._basedn`
		`- if newsuperior != None:`
		`+ if newsuperior is not None:`
		`# Well, the new DN should be rdn + newsuperior.`
		`self._dn = '%s,%s' % (new_rdn, newsuperior)`
		`else:`
		`@@ -714,7 +727,7 @@`

		`# assert we actually got the change right ....`

		`- def delete(self):`
		`+ def delete(self, recursive=False):`
		`"""Deletes the object defined by self._dn.`
		`This can be changed with the self._protected flag!`
		`"""`
		`@@ -722,7 +735,13 @@`
		`self._log.debug("%s delete" % (self._dn))`
		`if not self._protected:`
		`# Is there a way to mark this as offline and kill it`
		`- self._instance.delete_ext_s(self._dn, serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure')`
		`+ if recursive:`
		`+ filterstr = "(\|(objectclass=*)(objectclass=ldapsubentry))"`
		`+ ents = self._instance.search_s(self._dn, ldap.SCOPE_SUBTREE, filterstr, escapehatch='i am sure')`
		`+ for ent in sorted(ents, key=lambda e: len(e.dn), reverse=True):`
		`+ self._instance.delete_ext_s(ent.dn, serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure')`
		`+ else:`
		`+ self._instance.delete_ext_s(self._dn, serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure')`

		`def _validate(self, rdn, properties, basedn):`
		`"""Used to validate a create request.`
		`@@ -1121,7 +1140,7 @@`

		`def filter(self, search):`
		`# This will yield and & filter for objectClass with as many terms as needed.`
		`- search_filter = _gen_and([self._get_objectclass_filter(),search])`
		`+ search_filter = _gen_and([self._get_objectclass_filter(), search])`
		`self._log.debug('list filter = %s' % search_filter)`
		`try:`
		`results = self._instance.search_ext_s(`
		`@@ -1137,5 +1156,3 @@`
		`# There are no objects to select from, se we return an empty array`
		`insts = []`
		`return insts`
		`-`
		`-`

src/lib389/lib389/cli_conf/conflicts.py

file added

+127

		`@@ -0,0 +1,127 @@`
		`+ # --- BEGIN COPYRIGHT BLOCK ---`
		`+ # Copyright (C) 2019 Red Hat, Inc.`
		`+ # All rights reserved.`
		`+ #`
		`+ # License: GPL (version 3 or any later version).`
		`+ # See LICENSE for details.`
		`+ # --- END COPYRIGHT BLOCK ---`
		`+`
		`+ import json`
		`+ from lib389.conflicts import (ConflictEntries, ConflictEntry, GlueEntries, GlueEntry)`
		`+`
		`+ conflict_attrs = ['nsds5replconflict', '*']`
		`+`
		`+`
		`+ def list_conflicts(inst, basedn, log, args):`
		`+ conflicts = ConflictEntries(inst, args.suffix).list()`
		`+ if args.json:`
		`+ results = []`
		`+ for conflict in conflicts:`
		`+ results.append(json.loads(conflict.get_all_attrs_json()))`
		`+ log.info(json.dumps({'type': 'list', 'items': results}))`
		`+ else:`
		`+ if len(conflicts) > 0:`
		`+ for conflict in conflicts:`
		`+ log.info(conflict.display(conflict_attrs))`
		`+ else:`
		`+ log.info("There were no conflict entries found under the suffix")`
		`+`
		`+`
		`+ def cmp_conflict(inst, basedn, log, args):`
		`+ conflict = ConflictEntry(inst, args.DN)`
		`+ valid_entry = conflict.get_valid_entry()`
		`+`
		`+ if args.json:`
		`+ results = []`
		`+ results.append(json.loads(conflict.get_all_attrs_json()))`
		`+ results.append(json.loads(valid_entry.get_all_attrs_json()))`
		`+ log.info(json.dumps({'type': 'list', 'items': results}))`
		`+ else:`
		`+ log.info("Conflict Entry:\n")`
		`+ log.info(conflict.display(conflict_attrs))`
		`+ log.info("Valid Entry:\n")`
		`+ log.info(valid_entry.display(conflict_attrs))`
		`+`
		`+`
		`+ def del_conflict(inst, basedn, log, args):`
		`+ conflict = ConflictEntry(inst, args.DN)`
		`+ conflict.delete()`
		`+`
		`+`
		`+ def swap_conflict(inst, basedn, log, args):`
		`+ conflict = ConflictEntry(inst, args.DN)`
		`+ conflict.swap()`
		`+`
		`+`
		`+ def convert_conflict(inst, basedn, log, args):`
		`+ conflict = ConflictEntry(inst, args.DN)`
		`+ conflict.convert(args.new_rdn)`
		`+`
		`+`
		`+ def list_glue(inst, basedn, log, args):`
		`+ glues = GlueEntries(inst, args.suffix).list()`
		`+ if args.json:`
		`+ results = []`
		`+ for glue in glues:`
		`+ results.append(json.loads(glue.get_all_attrs_json()))`
		`+ log.info(json.dumps({'type': 'list', 'items': results}))`
		`+ else:`
		`+ if len(glues) > 0:`
		`+ for glue in glues:`
		`+ log.info(glue.display(conflict_attrs))`
		`+ else:`
		`+ log.info("There were no glue entries found under the suffix")`
		`+`
		`+`
		`+ def del_glue(inst, basedn, log, args):`
		`+ glue = GlueEntry(inst, args.DN)`
		`+ glue.delete_all()`
		`+`
		`+`
		`+ def convert_glue(inst, basedn, log, args):`
		`+ glue = GlueEntry(inst, args.DN)`
		`+ glue.convert()`
		`+`
		`+`
		`+ def create_parser(subparsers):`
		`+ conflict_parser = subparsers.add_parser('repl-conflict', help="Manage replication conflicts")`
		`+ subcommands = conflict_parser.add_subparsers(help='action')`
		`+`
		`+ # coinflict entry arguments`
		`+ list_parser = subcommands.add_parser('list', help="List conflict entries")`
		`+ list_parser.add_argument('suffix', help='The backend name, or suffix, to look for conflict entries')`
		`+ list_parser.set_defaults(func=list_conflicts)`
		`+`
		`+ cmp_parser = subcommands.add_parser('compare', help="Compare the conflict entry with its valid counterpart")`
		`+ cmp_parser.add_argument('DN', help='The DN of the conflict entry')`
		`+ cmp_parser.set_defaults(func=cmp_conflict)`
		`+`
		`+ del_parser = subcommands.add_parser('delete', help="Delete a conflict entry")`
		`+ del_parser.add_argument('DN', help='The DN of the conflict entry')`
		`+ del_parser.set_defaults(func=del_conflict)`
		`+`
		`+ replace_parser = subcommands.add_parser('swap', help="Replace the valid entry with the conflict entry")`
		`+ replace_parser.add_argument('DN', help='The DN of the conflict entry')`
		`+ replace_parser.set_defaults(func=swap_conflict)`
		`+`
		`+ replace_parser = subcommands.add_parser('convert', help="Convert the conflict entry to a valid entry, "`
		`+ "while keeping the original valid entry counterpart. "`
		`+ "This requires that the converted conflict entry have "`
		`+ "a new RDN value. For example: \"cn=my_new_rdn_value\".")`
		`+ replace_parser.add_argument('DN', help='The DN of the conflict entry')`
		`+ replace_parser.add_argument('--new-rdn', required=True, help="The new RDN for the converted conflict entry. "`
		`+ "For example: \"cn=my_new_rdn_value\"")`
		`+ replace_parser.set_defaults(func=convert_conflict)`
		`+`
		`+ # Glue entry arguments`
		`+ list_glue_parser = subcommands.add_parser('list-glue', help="List replication glue entries")`
		`+ list_glue_parser.add_argument('suffix', help='The backend name, or suffix, to look for glue entries')`
		`+ list_glue_parser.set_defaults(func=list_glue)`
		`+`
		`+ del_glue_parser = subcommands.add_parser('delete-glue', help="Delete the glue entry and its child entries")`
		`+ del_glue_parser.add_argument('DN', help='The DN of the glue entry')`
		`+ del_glue_parser.set_defaults(func=del_glue)`
		`+`
		`+ convert_glue_parser = subcommands.add_parser('convert-glue', help="Convert the glue entry into a regular entry")`
		`+ convert_glue_parser.add_argument('DN', help='The DN of the glue entry')`
		`+ convert_glue_parser.set_defaults(func=convert_glue)`

src/lib389/lib389/cli_conf/monitor.py

file modified

		`@@ -1,5 +1,6 @@`
		`# --- BEGIN COPYRIGHT BLOCK ---`
		`# Copyright (C) 2019 William Brown <william@blackhats.net.au>`
		`+ # Copyright (C) 2019 Red Hat, Inc.`
		`# All rights reserved.`
		`#`
		`# License: GPL (version 3 or any later version).`

src/lib389/lib389/conflicts.py

file added

+175

		`@@ -0,0 +1,175 @@`
		`+ # --- BEGIN COPYRIGHT BLOCK ---`
		`+ # Copyright (C) 2019 Red Hat, Inc.`
		`+ # All rights reserved.`
		`+ #`
		`+ # License: GPL (version 3 or any later version).`
		`+ # See LICENSE for details.`
		`+ # --- END COPYRIGHT BLOCK ---`
		`+ #`
		`+`
		`+ import ldap`
		`+ from lib389._mapped_object import DSLdapObject, DSLdapObjects, _gen_filter`
		`+`
		`+`
		`+ class ConflictEntry(DSLdapObject):`
		`+ """A replication conflict entry`
		`+`
		`+ :param instance: An instance`
		`+ :type instance: lib389.DirSrv`
		`+ :param dn: The DN of the conflict entry`
		`+ :type dn: str`
		`+ """`
		`+ def __init__(self, instance, dn=None):`
		`+ super(ConflictEntry, self).__init__(instance, dn)`
		`+ self._rdn_attribute = 'cn'`
		`+ self._create_objectclasses = ['ldapsubentry']`
		`+ self._protected = False`
		`+ self._object_filter = '(objectclass=ldapsubentry)'`
		`+`
		`+ def convert(self, new_rdn):`
		`+ """Convert conflict entry to a vlid entry, but we need to`
		`+ give the conflict entry a new rdn since we are not replacing`
		`+ the existing valid counterpart entry.`
		`+ """`
		`+`
		`+ # Get the conflict entry info`
		`+ conflict_value = self.get_attr_val_utf8('nsds5ReplConflict')`
		`+ entry_dn = conflict_value.split(' ', 3)[2]`
		`+ entry_rdn = ldap.explode_dn(entry_dn, 1)[0]`
		`+ rdn_attr = entry_dn.split('=', 1)[0]`
		`+`
		`+ # Rename conflict entry`
		`+ self.rename(new_rdn, deloldrdn=False)`
		`+`
		`+ # Cleanup entry`
		`+ self.remove(rdn_attr, entry_rdn)`
		`+ if self.present('objectclass', 'ldapsubentry'):`
		`+ self.remove('objectclass', 'ldapsubentry')`
		`+ self.remove_all('nsds5ReplConflict')`
		`+`
		`+ def swap(self):`
		`+ """Make the conflict entry the real valid entry. Delete old valid entry,`
		`+ and rename the conflict`
		`+ """`
		`+`
		`+ # Get the conflict entry info`
		`+ conflict_value = self.get_attr_val_utf8('nsds5ReplConflict')`
		`+ entry_dn = conflict_value.split(' ', 3)[2]`
		`+ entry_rdn = ldap.explode_dn(entry_dn, 1)[0]`
		`+`
		`+ # Gather the RDN details`
		`+ rdn_attr = entry_dn.split('=', 1)[0]`
		`+ new_rdn = "{}={}".format(rdn_attr, entry_rdn)`
		`+ tmp_rdn = new_rdn + 'tmp'`
		`+`
		`+ # Delete valid entry (to be replaced by conflict entry)`
		`+ original_entry = DSLdapObject(self._instance, dn=entry_dn)`
		`+ original_entry._protected = False`
		`+ original_entry.delete()`
		`+`
		`+ # Rename conflict entry to tmp rdn so we can clean up the rdn attr`
		`+ self.rename(tmp_rdn, deloldrdn=False)`
		`+`
		`+ # Cleanup entry`
		`+ self.remove(rdn_attr, entry_rdn)`
		`+ if self.present('objectclass', 'ldapsubentry'):`
		`+ self.remove('objectclass', 'ldapsubentry')`
		`+ self.remove_all('nsds5ReplConflict')`
		`+`
		`+ # Rename to the final/correct rdn`
		`+ self.rename(new_rdn, deloldrdn=True)`
		`+`
		`+ def get_valid_entry(self):`
		`+ """Get the conflict entry's valid counterpart entry`
		`+ """`
		`+ # Get the conflict entry info`
		`+ conflict_value = self.get_attr_val_utf8('nsds5ReplConflict')`
		`+ entry_dn = conflict_value.split(' ', 3)[2]`
		`+`
		`+ # Get the valid entry`
		`+ return DSLdapObject(self._instance, dn=entry_dn)`
		`+`
		`+`
		`+ class ConflictEntries(DSLdapObjects):`
		`+ """Represents the set of tombstone objects that may exist on`
		`+ this replica. Tombstones are locally generated, so they are`
		`+ unique to individual masters, and may or may not correlate`
		`+ to tombstones on other masters.`
		`+`
		`+ :param instance: An instance`
		`+ :type instance: lib389.DirSrv`
		`+ :param basedn: Tree to search for tombstones in`
		`+ :type basedn: str`
		`+ """`
		`+ def __init__(self, instance, basedn):`
		`+ super(ConflictEntries, self).__init__(instance)`
		`+ self._objectclasses = ['ldapsubentry']`
		`+ # Try some common ones ....`
		`+ self._filterattrs = ['nsds5replconflict', 'objectclass']`
		`+ self._childobject = ConflictEntry`
		`+ self._basedn = basedn`
		`+`
		`+ def _get_objectclass_filter(self):`
		`+ return "(&(objectclass=ldapsubentry)(nsds5replconflict=*))"`
		`+`
		`+`
		`+ class GlueEntry(DSLdapObject):`
		`+ """A replication glue entry`
		`+`
		`+ :param instance: An instance`
		`+ :type instance: lib389.DirSrv`
		`+ :param dn: The DN of the conflict entry`
		`+ :type dn: str`
		`+ """`
		`+ def __init__(self, instance, dn=None):`
		`+ super(GlueEntry, self).__init__(instance, dn)`
		`+ self._rdn_attribute = ''`
		`+ self._create_objectclasses = ['glue']`
		`+ self._protected = False`
		`+ self._object_filter = '(objectclass=glue)'`
		`+`
		`+ def convert(self):`
		`+ """Convert entry into real entry`
		`+ """`
		`+ self.remove_all('nsds5replconflict')`
		`+ self.remove('objectclass', 'glue')`
		`+`
		`+ def delete_all(self):`
		`+ """Remove glue entry and its children. Depending on the situation the URP`
		`+ mechanism can turn the parent glue entry into a tombstone before we get`
		`+ a chance to delete it. This results in a NO_SUCH_OBJECT exception`
		`+ """`
		`+ delete_count = 0`
		`+ filterstr = "(\|(objectclass=*)(objectclass=ldapsubentry))"`
		`+ ents = self._instance.search_s(self._dn, ldap.SCOPE_SUBTREE, filterstr, escapehatch='i am sure')`
		`+ for ent in sorted(ents, key=lambda e: len(e.dn), reverse=True):`
		`+ try:`
		`+ self._instance.delete_ext_s(ent.dn, serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure')`
		`+ delete_count += 1`
		`+ except ldap.NO_SUCH_OBJECT as e:`
		`+ if len(ents) > 0 and delete_count == (len(ents) - 1):`
		`+ # This is the parent glue entry - it was removed by URP`
		`+ pass`
		`+ else:`
		`+ raise e`
		`+`
		`+`
		`+ class GlueEntries(DSLdapObjects):`
		`+ """Represents the set of glue entries that may exist on`
		`+ this replica.`
		`+`
		`+ :param instance: An instance`
		`+ :type instance: lib389.DirSrv`
		`+ :param basedn: Tree to search for tombstones in`
		`+ :type basedn: str`
		`+ """`
		`+ def __init__(self, instance, basedn):`
		`+ super(GlueEntries, self).__init__(instance)`
		`+ self._objectclasses = ['glue']`
		`+ # Try some common ones ....`
		`+ self._filterattrs = ['nsds5replconflict', 'objectclass']`
		`+ self._childobject = GlueEntry`
		`+ self._basedn = basedn`
		`+`
		`+ def _get_objectclass_filter(self):`
		`+ return _gen_filter(['objectclass'], ['glue'])`

src/lib389/lib389/tests/cli/conf_conflicts_test.py

file added

+161

		`@@ -0,0 +1,161 @@`
		`+ # --- BEGIN COPYRIGHT BLOCK ---`
		`+ # Copyright (C) 2018 Red Hat, Inc.`
		`+ # All rights reserved.`
		`+ #`
		`+ # License: GPL (version 3 or any later version).`
		`+ # See LICENSE for details.`
		`+ # --- END COPYRIGHT BLOCK ---`
		`+`
		`+`
		`+ import io`
		`+ import sys`
		`+ import pytest`
		`+ import time`
		`+ import json`
		`+ from lib389.cli_base import LogCapture, FakeArgs`
		`+ from lib389.utils import *`
		`+ from lib389._constants import *`
		`+ from lib389.idm.nscontainer import nsContainers`
		`+ from lib389.topologies import topology_m2 as topo`
		`+ from lib389.cli_conf.conflicts import (list_conflicts, cmp_conflict, del_conflict, swap_conflict,`
		`+ convert_conflict, list_glue, del_glue, convert_glue)`
		`+ from lib389.utils import ds_is_older`
		`+ pytestmark = pytest.mark.skipif(ds_is_older('1.4.0'), reason="Not implemented")`
		`+`
		`+`
		`+ def _create_container(inst, dn, name):`
		`+ """Creates container entry"""`
		`+ containers = nsContainers(inst, dn)`
		`+ container = containers.create(properties={'cn': name})`
		`+ time.sleep(1)`
		`+ return container`
		`+`
		`+`
		`+ def _delete_container(container):`
		`+ """Deletes container entry"""`
		`+ container.delete()`
		`+ time.sleep(1)`
		`+`
		`+`
		`+ def test_conflict_cli(topo):`
		`+ """Test manageing replication conflict entries`
		`+ :id: 800f432a-52ab-4661-ac66-a2bdd9b984d8`
		`+ :setup: two masters`
		`+ :steps:`
		`+ 1. Create replication conflict entries`
		`+ 2. List conflicts`
		`+ 3. Compare conflict entry`
		`+ 4. Delete conflict`
		`+ 5. Resurrect conflict`
		`+ 6. Swap conflict`
		`+ 7. List glue entry`
		`+ 8. Delete glue entry`
		`+ 9. Convert glue entry`
		`+`
		`+ :expectedresults:`
		`+ 1. Success`
		`+ 2. Success`
		`+ 3. Success`
		`+ 4. Success`
		`+ 5. Success`
		`+ 6. Success`
		`+ 7. Success`
		`+ 8. Success`
		`+ 9. Success`
		`+ 10. Success`
		`+ """`
		`+`
		`+ # Setup our default parameters for CLI functions`
		`+ topo.logcap = LogCapture()`
		`+ sys.stdout = io.StringIO()`
		`+ args = FakeArgs()`
		`+ args.DN = ""`
		`+ args.suffix = DEFAULT_SUFFIX`
		`+ args.json = True`
		`+`
		`+ m1 = topo.ms["master1"]`
		`+ m2 = topo.ms["master2"]`
		`+`
		`+ topo.pause_all_replicas()`
		`+`
		`+ # Create entries`
		`+ _create_container(m1, DEFAULT_SUFFIX, 'conflict_parent1')`
		`+ _create_container(m2, DEFAULT_SUFFIX, 'conflict_parent1')`
		`+ _create_container(m1, DEFAULT_SUFFIX, 'conflict_parent2')`
		`+ _create_container(m2, DEFAULT_SUFFIX, 'conflict_parent2')`
		`+ cont_parent_m1 = _create_container(m1, DEFAULT_SUFFIX, 'conflict_parent3')`
		`+ cont_parent_m2 = _create_container(m2, DEFAULT_SUFFIX, 'conflict_parent3')`
		`+ cont_glue_m1 = _create_container(m1, DEFAULT_SUFFIX, 'conflict_parent4')`
		`+ cont_glue_m2 = _create_container(m2, DEFAULT_SUFFIX, 'conflict_parent4')`
		`+`
		`+ # Create the conflicts`
		`+ _delete_container(cont_parent_m1)`
		`+ _create_container(m2, cont_parent_m2.dn, 'conflict_child1')`
		`+ _delete_container(cont_glue_m1)`
		`+ _create_container(m2, cont_glue_m2.dn, 'conflict_child2')`
		`+`
		`+ # Resume replication`
		`+ topo.resume_all_replicas()`
		`+ time.sleep(5)`
		`+`
		`+ # Test "list"`
		`+ list_conflicts(m2, None, topo.logcap.log, args)`
		`+ conflicts = json.loads(topo.logcap.outputs[0].getMessage())`
		`+ assert len(conflicts['items']) == 4`
		`+ conflict_1_DN = conflicts['items'][0]['dn']`
		`+ conflict_2_DN = conflicts['items'][1]['dn']`
		`+ conflict_3_DN = conflicts['items'][2]['dn']`
		`+ topo.logcap.flush()`
		`+`
		`+ # Test compare`
		`+ args.DN = conflict_1_DN`
		`+ cmp_conflict(m2, None, topo.logcap.log, args)`
		`+ conflicts = json.loads(topo.logcap.outputs[0].getMessage())`
		`+ assert len(conflicts['items']) == 2`
		`+ topo.logcap.flush()`
		`+`
		`+ # Test delete`
		`+ del_conflict(m2, None, topo.logcap.log, args)`
		`+ list_conflicts(m2, None, topo.logcap.log, args)`
		`+ conflicts = json.loads(topo.logcap.outputs[0].getMessage())`
		`+ assert len(conflicts['items']) == 3`
		`+ topo.logcap.flush()`
		`+`
		`+ # Test swap`
		`+ args.DN = conflict_2_DN`
		`+ swap_conflict(m2, None, topo.logcap.log, args)`
		`+ list_conflicts(m2, None, topo.logcap.log, args)`
		`+ conflicts = json.loads(topo.logcap.outputs[0].getMessage())`
		`+ assert len(conflicts['items']) == 2`
		`+ topo.logcap.flush()`
		`+`
		`+ # Test conflict convert`
		`+ args.DN = conflict_3_DN`
		`+ args.new_rdn = "cn=testing convert"`
		`+ convert_conflict(m2, None, topo.logcap.log, args)`
		`+ list_conflicts(m2, None, topo.logcap.log, args)`
		`+ conflicts = json.loads(topo.logcap.outputs[0].getMessage())`
		`+ assert len(conflicts['items']) == 1`
		`+ topo.logcap.flush()`
		`+`
		`+ # Test list glue entries`
		`+ list_glue(m2, None, topo.logcap.log, args)`
		`+ glues = json.loads(topo.logcap.outputs[0].getMessage())`
		`+ assert len(glues['items']) == 2`
		`+ topo.logcap.flush()`
		`+`
		`+ # Test delete glue entries`
		`+ args.DN = "cn=conflict_parent3,dc=example,dc=com"`
		`+ del_glue(m2, None, topo.logcap.log, args)`
		`+ list_glue(m2, None, topo.logcap.log, args)`
		`+ glues = json.loads(topo.logcap.outputs[0].getMessage())`
		`+ assert len(glues['items']) == 1`
		`+ topo.logcap.flush()`
		`+`
		`+ # Test convert glue entries`
		`+ args.DN = "cn=conflict_parent4,dc=example,dc=com"`
		`+ convert_glue(m2, None, topo.logcap.log, args)`
		`+ list_glue(m2, None, topo.logcap.log, args)`
		`+ glues = json.loads(topo.logcap.outputs[0].getMessage())`
		`+ assert len(glues['items']) == 0`
		`+ topo.logcap.flush()`

mreynolds commented 5 years ago

Description:

Added Conflict Entry and Glue entry classes to lib389, and updated dsconf to allow for conflict entry management.

Made some other minor changes to mapped objects:

            -  Added an attribute list option to display()
            -  Added a recursive delete option to delete()
            -  _gen_filter - had to prevent escape_filter_chars()
               from escaping "*" as it would convert it to /02A
               and the server would not process pres filters
               correctly.

https://pagure.io/389-ds-base/issue/50327

firstyear commented on line 13 of src/lib389/lib389/_mapped_object.py 5 years ago

This is actually the whole point of this here is to prevent terms like * and () because there are security risks with allowing them to be input by users. If you need a * search, then there are probably other ways to construct it?

firstyear commented on line 28 of src/lib389/lib389/conflicts.py 5 years ago

isn't it objectClass=ldapsubentry and nsds5replconflict? Atm this would pick up old COS defs that had ldapsubentry on them.

firstyear commented on line 42 of src/lib389/lib389/conflicts.py 5 years ago

IIRC there is already a ressurct method, and already a conflict type?

firstyear commented on line 30 of src/lib389/lib389/conflicts.py 5 years ago

Have you see lib389/tombstone.py? Maybe this is similar and could be helpful?

firstyear commented on line 139 of src/lib389/lib389/conflicts.py 5 years ago

It may not be a good idea to remove these, given they are part of the replication machinery, it could be better to leave them alone?

firstyear commented on line 136 of src/lib389/lib389/conflicts.py 5 years ago

It may be safer to duplicate the entry content back to a new entry rather than trying to revive this as it's part of the replication machinery.

firstyear commented 5 years ago

@lkrispen What do you think here? there are some functions to revive/convert conflict entries and glue entries. I think if we want to revive these it would be safer to make new entries and copy the attributes to the new entry rather than deleting/modifying the existing conflict/glue. What do you think about this?

mreynolds commented 5 years ago

This is actually the whole point of this here is to prevent terms like * and () because there are security risks with allowing them to be input by users. If you need a * search, then there are probably other ways to construct it?

Well I need to construct a filter like "(&(objectclass=ldapsubentry)(nsds5replconflict=*))"

mreynolds commented 5 years ago

isn't it objectClass=ldapsubentry and nsds5replconflict? Atm this would pick up old COS defs that had ldapsubentry on them.

Hmm this does looks wrong, I'll look into it tomorrow

mreynolds commented 5 years ago

It may not be a good idea to remove these, given they are part of the replication machinery, it could be better to leave them alone?

It may be safer to duplicate the entry content back to a new entry rather than trying to revive this as it's part of the replication machinery.

This is how the admin guide describes how to handle these conflict & glue entries. Just following the documented design...

lkrispen commented 5 years ago

This is actually the whole point of this here is to prevent terms like * and () because there are security risks with allowing them to be input by users. If you need a * search, then there are probably other ways to construct it?

Well I need to construct a filter like "(&(objectclass=ldapsubentry)(nsds5replconflict=*))"

I agree, to find conflict entries we need this filter, the nsds5replconflict attribute is a string encoding where the conflict was coming from ADD/MODRDN and what teh dn of the conflicting entry was ..... So the presence search is the only really prdictable ay to address these entries.

What William said was to prevent filters like these entered by users, but I think this is not the case, it is an internally generated filter.

@mreynolds It would be nice to see the intended usage by the CLI to see what the user can ar has to provide as input

lkrispen commented 5 years ago

IIRC there is already a ressurct method, and already a conflict type?

for this and the following comments: please do not confuse conflicts with tombstones. Conflicts are "real" entries, now hidden (and more consistently created), but all the managent operations like cleanup as described in the admin guide do still apply.

There is no need to copy/delete these entries, they can be handled like any other entry with modrdn and modify operations

lkrispen commented 5 years ago

It may not be a good idea to remove these, given they are part of the replication machinery, it could be better to leave them alone?
It may be safer to duplicate the entry content back to a new entry rather than trying to revive this as it's part of the replication machinery.

This is how the admin guide describes how to handle these conflict & glue entries. Just following the documented design...

correct, I just don't see the usage in the cli, and for glue entries, if you want to delete them
- you have to delete all their children as well, but
- you could do this with a normal recursive delete, don't know if we need a special function

lkrispen commented 5 years ago

@lkrispen What do you think here? there are some functions to revive/convert conflict entries and glue entries. I think if we want to revive these it would be safer to make new entries and copy the attributes to the new entry rather than deleting/modifying the existing conflict/glue. What do you think about this?

no, no need to create new entries first and it would probably also have unexpected side effects. if you create and copy you get a new entry with a different nsuniqueid, but you want to keep the entry, just rename it, or delete some attribute.

Conflicts are not really "part of the replication machinery", they come int existence by replication, buit once created they are "normal", valid entries

spichugi commented on line 27 of src/lib389/lib389/_mapped_object.py 5 years ago

If you decided to format it properly, could you please check the pep8 and use it then? :)
https://legacy.python.org/dev/peps/pep-0008/#indentation

lkrispen commented 5 years ago

isn't it objectClass=ldapsubentry and nsds5replconflict? Atm this would pick up old COS defs that had ldapsubentry on them.

Hmm this does looks wrong, I'll look into it tomorrow

as far as I see you are using it in BASE searche, so this would only require the objectclass filter jut to be able to see the entry if it is an ldapsubentry

spichugi commented on line 9 of src/lib389/lib389/tests/cli/conf_conflicts_test.py 5 years ago

As far as I recall, we try to get rid of lib389 tests and move them to dirsrvtests/tests/suites/lib389/

mreynolds commented 5 years ago

@lkrispen here is an example of using the CLI (as requested).

List conflict entries:

dsconf slapd-localhost repl-conflict list

Compare conflict entry with its "valid" entry counterpart

dsconf slapd-localhost repl-conflict compare <DN of conflict entry>

--> List lists two entries, the conflict and the valid entry so you can see what is different so you can decided if you want to swap them, or "resurrect", for lack of a better term, the conflict entry.

To swap the conflict entry with the "valid" entry do this

dsconf slapd-localhost repl-conflict swap <DN of conflict entry>

To resurrect conflict (meaning we keep the valid entry as well). We use this command with a new rdn value

dsconf slapd-localhost repl-conflict  resurrect <DN of conflict>  --new-rdn "cn=new rdn"

To delete conflict entry run:

dsconf slapd-localhost repl-conflict delete <DN of conflict entry>

Now for Glue entries...

List Glue entries

dsconf slapd-localhost repl-conflict list-glue

Delete Glue entry and its children

dsconf slapd-localhost repl-conflict delete-glue <DN of glue entry>

Convert Glue entry to "normal" entry

dsconf slapd-localhost repl-conflict convert-glue <DN of Glue entry>

Edited 5 years ago by mreynolds

mreynolds commented 5 years ago

As far as I recall, we try to get rid of lib389 tests and move them to dirsrvtests/tests/suites/lib389/

That was the plan, but it was never started and we were still adding CLI tests to the old location. So until it was ported to /dirsrvtests I was updating everything in one location.

mreynolds commented 5 years ago

If you decided to format it properly, could you please check the pep8 and use it then? :)
https://legacy.python.org/dev/peps/pep-0008/#indentation

Sure I'll check this out!

rebased onto 01f907b1783ffd73f4dfc5905ba26c0293b71b89

5 years ago

rebased onto cbec098cfaadee1c329214084eb9c853282113f8

5 years ago

rebased onto baff11cafda98107e76e503350e629d191d21f42

5 years ago

mreynolds commented 5 years ago

This is actually the whole point of this here is to prevent terms like * and () because there are security risks with allowing them to be input by users. If you need a * search, then there are probably other ways to construct it?
Well I need to construct a filter like "(&(objectclass=ldapsubentry)(nsds5replconflict=*))"

I agree, to find conflict entries we need this filter, the nsds5replconflict attribute is a string encoding where the conflict was coming from ADD/MODRDN and what teh dn of the conflicting entry was ..... So the presence search is the only really prdictable ay to address these entries.
What William said was to prevent filters like these entered by users, but I think this is not the case, it is an internally generated filter.

Then why are we using it for internal classes @firstyear?

Well I'll just hardcoded the filter, but I really don't like that we are using these filter generation functions that will cause substring and presence filters to silently fail. They don't belong in our internal API IMO. Anyway it's no biggie, I worked around it by hardcoding the filter.

@spichugi - fixed pep8 errors in mapped_object.py

@lkrispen - I provided the CLI examples in the previous comment.

Please review...

lkrispen commented 5 years ago

@lkrispen - I provided the CLI examples in the previous comment.
Please review...

thanks for the examples, they look good and useful, with one exception. I would not use "resurrect", it is used for tombstones and will be confusing. What you want to do is to make the conflict entry a valid entry with a newrdn, so I would use something like "rename", "make -valid", "keep-as"

in general I think this is very helpful to deal with conflicts, the procedures in the admin guide are good, but complicated.

I let you settle the lib389/python arguments with the pythonians :-)

mreynolds commented 5 years ago

@lkrispen - I provided the CLI examples in the previous comment.
Please review...

thanks for the examples, they look good and useful, with one exception. I would not use "resurrect", it is used for tombstones and will be confusing. What you want to do is to make the conflict entry a valid entry with a newrdn, so I would use something like "rename", "make -valid", "keep-as"

Agreed, the more I look at this the more I hate "resurrect", originally it was "convert", but I changed it. I think "rename" is the more accurate argument name so I will change it, but I'm open to other name suggestions :-)

lkrispen commented 5 years ago

In fact I do like "convert" , it indicates that it is a bit more than rename, eg under the hood there is the removal of th ldapsubentry and nsds5replconflict. So I think your initial choice was the best

rebased onto 9f55a3c0cd2a94bcfbef9def8ca1fa0d822a3a7c

5 years ago

mreynolds commented 5 years ago

In fact I do like "convert" , it indicates that it is a bit more than rename, eg under the hood there is the removal of th ldapsubentry and nsds5replconflict. So I think your initial choice was the best

Hahaha, I thought same thing after the fact, and the rebase uses "convert". :-D It is also more consistent with the glue function argument names

mreynolds commented 5 years ago

As far as I recall, we try to get rid of lib389 tests and move them to dirsrvtests/tests/suites/lib389/

That was the plan, but it was never started and we were still adding CLI tests to the old location. So until it was ported to /dirsrvtests I was updating everything in one location.

@spichugi - FYI - looks like this is still being debated as well:

https://pagure.io/389-ds-base/issue/49911

Either way we should move the tests via a different ticket, not this one...

spichugi commented 5 years ago

@spichugi - FYI - looks like this is still being debated as well:
https://pagure.io/389-ds-base/issue/49911
Either way we should move the tests via a different ticket, not this one...

Agree.
I think we should decide it sooner though because CLI is growing and more test will appear.
But I think the decision does belong more to @vashirov . So I'd wait for his return.
For now, I think it is okay to have it like this. :)

firstyear commented 5 years ago

This is actually the whole point of this here is to prevent terms like * and () because there are security risks with allowing them to be input by users. If you need a * search, then there are probably other ways to construct it?
Well I need to construct a filter like "(&(objectclass=ldapsubentry)(nsds5replconflict=*))"
I agree, to find conflict entries we need this filter, the nsds5replconflict attribute is a string encoding where the conflict was coming from ADD/MODRDN and what teh dn of the conflicting entry was ..... So the presence search is the only really prdictable ay to address these entries.
What William said was to prevent filters like these entered by users, but I think this is not the case, it is an internally generated filter.

Then why are we using it for internal classes @firstyear?

Because we don't know where input could come from because python so we can't exactly do tracking of input data, so we assume everything is untrusted.

The way you get around this, is something like:

    def _get_objectclass_filter(self):
        return '(objectclass=ldapsubentry)(nsds5conflict=*)'

Which then will work in this case. All the gen instructions do is yield strings, so there are cases where you can just return them, and they should work just fine.

Well I'll just hardcoded the filter, but I really don't like that we are using these filter generation functions that will cause substring and presence filters to silently fail. They don't belong in our internal API IMO. Anyway it's no biggie, I worked around it by hardcoding the filter.

Which is exactly the point :) if we hardcode it internally, it's internal, we can do this safely. But we have to always un-trust all other input. So in fact, hardcoding the filter is correct here, but so is leaving the safety-escaping alone :)

@spichugi - fixed pep8 errors in mapped_object.py
@lkrispen - I provided the CLI examples in the previous comment.
Please review...

mreynolds commented 5 years ago

Which is exactly the point :) if we hardcode it internally, it's internal, we can do this safely. But we have to always un-trust all other input. So in fact, hardcoding the filter is correct here, but so is leaving the safety-escaping alone :)

It's all good, I agree!

So, Is that an ack then? :-)

firstyear commented 5 years ago

So I think all of @lkrispen's comments are addressed (it sounded like my misunderstanding anyway, but always safer to ask than assume), and I think @spichugi's comments are addressed, so ack here.

rebased onto 4f7c05e

5 years ago

Pull-Request has been merged by mreynolds

5 years ago

spichugi commented 3 years ago

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/3392

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.