PR#50526: Issue 50525 - nsslapd-defaultnamingcontext does not change when the assigned suffix got deleted - 389-ds-base

389-ds-base

#50526 Issue 50525 - nsslapd-defaultnamingcontext does not change when the assigned suffix got deleted

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

mreynolds/389-ds-base dsconf into master

Issue 50525 - nsslapd-defaultnamingcontext does not change when the assigned suffix gets deleted

Mark Reynolds • 4 years ago

fb3be04

dirsrvtests/tests/suites/mapping_tree/be_del_and_default_naming_attr_test.py

file added

+90

		`@@ -0,0 +1,90 @@`
		`+ import logging`
		`+ import pytest`
		`+ import os`
		`+ from lib389._constants import DEFAULT_SUFFIX`
		`+ from lib389.topologies import topology_m1 as topo`
		`+ from lib389.backend import Backends`
		`+ from lib389.encrypted_attributes import EncryptedAttrs`
		`+`
		`+ DEBUGGING = os.getenv("DEBUGGING", default=False)`
		`+ if DEBUGGING:`
		`+ logging.getLogger(__name__).setLevel(logging.DEBUG)`
		`+ else:`
		`+ logging.getLogger(__name__).setLevel(logging.INFO)`
		`+ log = logging.getLogger(__name__)`
		`+`
		`+ SECOND_SUFFIX = 'o=namingcontext'`
		`+ THIRD_SUFFIX = 'o=namingcontext2'`
		`+`
		`+ def test_be_delete(topo):`
		`+ """Test that we can delete a backend that contains replication`
		`+ configuration and encrypted attributes. The default naming`
		`+ context should also be updated to reflect the next available suffix`
		`+`
		`+ :id: 5208f897-7c95-4925-bad0-9ceb95fee678`
		`+ :setup: Master Instance`
		`+ :steps:`
		`+ 1. Create second backend/suffix`
		`+ 2. Add an encrypted attribute to the default suffix`
		`+ 2. Delete default suffix`
		`+ 3. Check the nsslapd-defaultnamingcontext is updated`
		`+ 4. Delete the last backend`
		`+ 5. Check the namingcontext has not changed`
		`+ 6. Add new backend`
		`+ 7. Set default naming context`
		`+ 8. Verify the naming context is correct`
		`+ :expectedresults:`
		`+ 1. Success`
		`+ 2. Success`
		`+ 3. Success`
		`+ 4. Success`
		`+ 5. Success`
		`+ 6. Success`
		`+ 7. Success`
		`+ 8. Success`
		`+ """`
		`+`
		`+ inst = topo.ms["master1"]`
		`+`
		`+ # Create second suffix`
		`+ backends = Backends(inst)`
		`+ default_backend = backends.get(DEFAULT_SUFFIX)`
		`+ new_backend = backends.create(properties={'nsslapd-suffix': SECOND_SUFFIX,`
		`+ 'name': 'namingRoot'})`
		`+`
		`+ # Add encrypted attribute entry under default suffix`
		`+ encrypt_attrs = EncryptedAttrs(inst, basedn='cn=encrypted attributes,{}'.format(default_backend.dn))`
		`+ encrypt_attrs.create(properties={'cn': 'employeeNumber', 'nsEncryptionAlgorithm': 'AES'})`
		`+`
		`+ # Delete default suffix`
		`+ default_backend.delete()`
		`+`
		`+ # Check that the default naming context is set to the new/second suffix`
		`+ default_naming_ctx = inst.config.get_attr_val_utf8('nsslapd-defaultnamingcontext')`
		`+ assert default_naming_ctx == SECOND_SUFFIX`
		`+`
		`+ # delete new backend, but the naming context should not change`
		`+ new_backend.delete()`
		`+`
		`+ # Check that the default naming context is still set to the new/second suffix`
		`+ default_naming_ctx = inst.config.get_attr_val_utf8('nsslapd-defaultnamingcontext')`
		`+ assert default_naming_ctx == SECOND_SUFFIX`
		`+`
		`+ # Add new backend`
		`+ new_backend = backends.create(properties={'nsslapd-suffix': THIRD_SUFFIX,`
		`+ 'name': 'namingRoot2'})`
		`+`
		`+ # manaully set naming context`
		`+ inst.config.set('nsslapd-defaultnamingcontext', THIRD_SUFFIX)`
		`+`
		`+ # Verify naming context is correct`
		`+ default_naming_ctx = inst.config.get_attr_val_utf8('nsslapd-defaultnamingcontext')`
		`+ assert default_naming_ctx == THIRD_SUFFIX`
		`+`
		`+`
		`+ if __name__ == '__main__':`
		`+ # Run isolated`
		`+ # -s for DEBUG mode`
		`+ CURRENT_FILE = os.path.realpath(__file__)`
		`+ pytest.main(["-s", CURRENT_FILE])`
		`+`

ldap/servers/slapd/mapping_tree.c

file modified

+30 -20

		`@@ -1519,26 +1519,36 @@`
		`strcpy_unescape_value(escaped, suffix);`
		`}`
		`if (escaped && (0 == strcasecmp(escaped, default_naming_context))) {`
		`- int rc = _mtn_update_config_param(LDAP_MOD_DELETE,`
		`- CONFIG_DEFAULT_NAMING_CONTEXT,`
		`- NULL);`
		`- if (rc) {`
		`- slapi_log_err(SLAPI_LOG_ERR,`
		`- "mapping_tree_entry_delete_callback",`
		`- "deleting config param %s failed: RC=%d\n",`
		`- CONFIG_DEFAULT_NAMING_CONTEXT, rc);`
		`- }`
		`- if (LDAP_SUCCESS == rc) {`
		`- char errorbuf[SLAPI_DSE_RETURNTEXT_SIZE] = {0};`
		`- /* Removing defaultNamingContext from cn=config entry`
		`- * was successful. The remove does not reset the`
		`- * global parameter. We need to reset it separately. */`
		`- if (config_set_default_naming_context(`
		`- CONFIG_DEFAULT_NAMING_CONTEXT,`
		`- NULL, errorbuf, CONFIG_APPLY)) {`
		`- slapi_log_err(SLAPI_LOG_ERR, "mapping_tree_entry_delete_callback",`
		`- "Setting NULL to %s failed. %s\n",`
		`- CONFIG_DEFAULT_NAMING_CONTEXT, errorbuf);`
		`+ /*`
		`+ * We can not delete the default naming attribute, so instead`
		`+ * replace it only if there is another suffix available`
		`+ */`
		`+ void *node = NULL;`
		`+ Slapi_DN *sdn;`
		`+ sdn = slapi_get_first_suffix(&node, 0);`
		`+ if (sdn) {`
		`+ char replacement_suffix = (char )slapi_sdn_get_dn(sdn);`
		`+ int rc = _mtn_update_config_param(LDAP_MOD_REPLACE,`
		`+ CONFIG_DEFAULT_NAMING_CONTEXT,`
		`+ replacement_suffix);`
		`+ if (rc) {`
		`+ slapi_log_err(SLAPI_LOG_ERR,`
		`+ "mapping_tree_entry_delete_callback",`
		`+ "replacing config param %s failed: RC=%d\n",`
		`+ CONFIG_DEFAULT_NAMING_CONTEXT, rc);`
		`+ }`
		`+ if (LDAP_SUCCESS == rc) {`
		`+ char errorbuf[SLAPI_DSE_RETURNTEXT_SIZE] = {0};`
		`+ /* Replacing defaultNamingContext from cn=config entry`
		`+ * was successful. The replace does not reset the`
		`+ * global parameter. We need to reset it separately. */`
		`+ if (config_set_default_naming_context(`
		`+ CONFIG_DEFAULT_NAMING_CONTEXT,`
		`+ replacement_suffix, errorbuf, CONFIG_APPLY)) {`
		`+ slapi_log_err(SLAPI_LOG_ERR, "mapping_tree_entry_delete_callback",`
		`+ "Setting %s tp %s failed. %s\n",`
		`+ CONFIG_DEFAULT_NAMING_CONTEXT, replacement_suffix, errorbuf);`
		`+ }`
		`}`
		`}`
		`}`

src/lib389/lib389/backend.py

file modified

+11 -9

		`@@ -18,6 +18,7 @@`
		`from lib389._mapped_object import DSLdapObjects, DSLdapObject`
		`from lib389.mappingTree import MappingTrees`
		`from lib389.exceptions import NoSuchEntryError, InvalidArgumentError`
		`+ from lib389.replica import Replicas`

		`# We need to be a factor to the backend monitor`
		`from lib389.monitor import MonitorBackend`
		`@@ -530,23 +531,24 @@`
		`mt = self._mts.get(selector=bename)`
		`# Assert the type is "backend"`
		`# Are these the right types....?`
		`- if mt.get_attr_val('nsslapd-state').lower() != ensure_bytes('backend'):`
		`+ if mt.get_attr_val_utf8('nsslapd-state').lower() != 'backend':`
		`raise ldap.UNWILLING_TO_PERFORM('Can not delete the mapping tree, not for a backend! You may need to delete this backend via cn=config .... ;_; ')`
		`+`
		`+ # Delete replicas first`
		`+ try:`
		`+ Replicas(self._instance).get(mt.get_attr_val_utf8('cn')).delete()`
		`+ except ldap.NO_SUCH_OBJECT:`
		`+ # No replica, no problem`
		`+ pass`
		`+`
		`# Delete our mapping tree if it exists.`
		`mt.delete()`
		`except ldap.NO_SUCH_OBJECT:`
		`# Righto, it's already gone! Do nothing ...`
		`pass`
		`- # Delete all our related indices`
		`- self._instance.index.delete_all(bename)`

		`# Now remove our children, this is all ldbm config`
		`-`
		`- configs = self._instance.search_s(self._dn, ldap.SCOPE_ONELEVEL)`
		`- for c in configs:`
		`- self._instance.delete_branch_s(c.dn, ldap.SCOPE_SUBTREE)`
		`- # The super will actually delete ourselves.`
		`- super(Backend, self).delete()`
		`+ self._instance.delete_branch_s(self._dn, ldap.SCOPE_SUBTREE)`

		`def _lint_mappingtree(self):`
		`"""Backend lint`

src/lib389/lib389/replica.py

file modified

+1 -1

		`@@ -472,7 +472,7 @@`
		`try:`
		`self.deleteAgreements(nsuffix)`
		`except ldap.LDAPError as e:`
		`- self.log.fatal('Failed to delete replica agreements!')`
		`+ self.log.fatal('Failed to delete replica agreements! ' + str(e))`
		`raise`

		`# Delete the replica`

mreynolds commented 4 years ago

Bug Description:

If you delete the suffix that is set as the default naming context, the attribute
is not reset.

Also using dsconf to delete a nackend/suffix fails if there are vlv indexes, encrypted
attributes, or replication is configured.

Fix Description:

As for thedefault anming contetx, if there is a second suffix configured, it will be
automatically set as the new default naming context, otherwisethe attribute is not
modified.

For dsconf backend delete issue, it now checks and removes replication configuration
and agreements, and removes all the child entries under the backend entry.

relates: https://pagure.io/389-ds-base/issue/50525

spichugi commented on line 48 of ldap/servers/slapd/mapping_tree.c 4 years ago

I am thinking... Do we really need this part? If I understand correctly, we already set CONFIG_DEFAULT_NAMING_CONTEXT if LDAP_SUCCESS == rc.
Also the comment above states Removing while we were replacing.

spichugi commented on line 56 of dirsrvtests/tests/suites/mapping_tree/be_del_and_default_naming_attr_test.py 4 years ago

Can we also add a case when we remove all of the backends? Just to check that nothing unexpected happens with nsslapd-defaultnamingcontext.
Here or to the basic testdirsrvtests/tests/suites/config/config_test.py::test_defaultnamingcontext...

Edited 4 years ago by spichugi

spichugi commented 4 years ago

The commit message has a couple of typos...
nackend/suffix
thedefault anming contetx
otherwisethe