#50526 Issue 50525 - nsslapd-defaultnamingcontext does not change when the assigned suffix got deleted
Merged 4 months ago by mreynolds. Opened 4 months ago by mreynolds.
mreynolds/389-ds-base dsconf  into  master

@@ -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])

+ 

@@ -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);

+                         }

                      }

                  }

              }

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

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

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

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.

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...

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

I am thinking... Do we really need this part? If I understand correctly, we already set CONFIG_DEFAULT_NAMING_CONTEXT if LDAP_SUCCESS == rc.

Yes we do, as this is what sets the in-memory value

Also the comment above states Removing while we were replacing.

I'll will correct his.

rebased onto 3055c6053d9452bb8bd6a38a65b33f3973bc9bbd

4 months ago

Changes applied, please review...

Removing word is still present in the comment...

The rest looks good to me! Ack

rebased onto fb3be04

4 months ago

Pull-Request has been merged by mreynolds

4 months ago