From 9271d5cac09b6d6887cb73b4e79aa3a822a4894b Mon Sep 17 00:00:00 2001 From: Thierry Bordaz Date: Oct 19 2017 13:30:55 +0000 Subject: Ticket 49386 - Memberof should be ignore MODRDN when the pre/post entry are identical Bug Description: If a modrdn command renames an entry in itself it is inexpensive for the core server but can be expensive for memberof. member will update all entries referring (memberof value) the target entry. The updates will delete and add back the target entry DN. Fix Description: In the early step of memberof modrdn callback, checks if pre_sdn and post_sdn are identical. If they are it skips the update https://pagure.io/389-ds-base/issue/49386 Reviewed by: Mark Reynolds Platforms tested: F23 Flag Day: no Doc impact: no foo --- diff --git a/dirsrvtests/tests/tickets/ticket49386_test.py b/dirsrvtests/tests/tickets/ticket49386_test.py new file mode 100644 index 0000000..2419092 --- /dev/null +++ b/dirsrvtests/tests/tickets/ticket49386_test.py @@ -0,0 +1,150 @@ +import logging +import pytest +import os +import ldap +import time +from lib389.utils import ds_is_older +from lib389.topologies import topology_st as topo +from lib389._constants import * +from lib389.config import Config +from lib389 import Entry + +pytestmark = pytest.mark.skipif(ds_is_older('1.3.7'), reason="Not implemented") + +USER_CN='user_' +GROUP_CN='group_' + +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__) + +def add_user(server, no, desc='dummy', sleep=True): + cn = '%s%d' % (USER_CN, no) + dn = 'cn=%s,ou=people,%s' % (cn, SUFFIX) + log.fatal('Adding user (%s): ' % dn) + server.add_s(Entry((dn, {'objectclass': ['top', 'person', 'inetuser'], + 'sn': ['_%s' % cn], + 'description': [desc]}))) + if sleep: + time.sleep(2) + +def add_group(server, nr, sleep=True): + cn = '%s%d' % (GROUP_CN, nr) + dn = 'cn=%s,ou=groups,%s' % (cn, SUFFIX) + server.add_s(Entry((dn, {'objectclass': ['top', 'groupofnames'], + 'description': 'group %d' % nr}))) + if sleep: + time.sleep(2) + +def update_member(server, member_dn, group_dn, op, sleep=True): + mod = [(op, 'member', member_dn)] + server.modify_s(group_dn, mod) + if sleep: + time.sleep(2) + +def config_memberof(server): + + server.plugins.enable(name=PLUGIN_MEMBER_OF) + MEMBEROF_PLUGIN_DN = ('cn=' + PLUGIN_MEMBER_OF + ',cn=plugins,cn=config') + server.modify_s(MEMBEROF_PLUGIN_DN, [(ldap.MOD_REPLACE, + 'memberOfAllBackends', + 'on'), + (ldap.MOD_REPLACE, 'memberOfAutoAddOC', 'nsMemberOf')]) + + +def _find_memberof(server, member_dn, group_dn, find_result=True): + ent = server.getEntry(member_dn, ldap.SCOPE_BASE, "(objectclass=*)", ['memberof']) + found = False + if ent.hasAttr('memberof'): + + for val in ent.getValues('memberof'): + server.log.info("!!!!!!! %s: memberof->%s" % (member_dn, val)) + server.log.info("!!!!!!! %s" % (val)) + server.log.info("!!!!!!! %s" % (group_dn)) + if val.lower() == group_dn.lower(): + found = True + break + + if find_result: + assert (found) + else: + assert (not found) + +def test_ticket49386(topo): + """Specify a test case purpose or name here + + :id: ceb1e2b7-42cb-49f9-8ddd-bc752aa4a589 + :setup: Fill in set up configuration here + :steps: + 1. Configure memberof + 2. Add users (user_1) + 3. Add groups (group_1) + 4. Make user_1 member of group_1 + 5. Check that user_1 has the memberof attribute to group_1 + 6. Enable plugin log to capture memberof modrdn callback notification + 7. Rename group_1 in itself + 8. Check that the operation was skipped by memberof + + :expectedresults: + 1. memberof modrdn callbackk to log notfication that the update is skipped + """ + + S1 = topo.standalone + + # Step 1 + config_memberof(S1) + S1.restart() + + # Step 2 + for i in range(10): + add_user(S1, i, desc='add on S1') + + # Step 3 + for i in range(3): + add_group(S1, i) + + # Step 4 + member_dn = 'cn=%s%d,ou=people,%s' % (USER_CN, 1, SUFFIX) + group_parent_dn = 'ou=groups,%s' % (SUFFIX) + group_rdn = 'cn=%s%d' % (GROUP_CN, 1) + group_dn = '%s,%s' % (group_rdn, group_parent_dn) + update_member(S1, member_dn, group_dn, ldap.MOD_ADD, sleep=False) + + # Step 5 + _find_memberof(S1, member_dn, group_dn, find_result=True) + + # Step 6 + S1.config.loglevel(vals=[LOG_PLUGIN, LOG_DEFAULT], service='error') + + # Step 7 + S1.rename_s(group_dn, group_rdn, newsuperior=group_parent_dn, delold=0) + + # Step 8 + time.sleep(2) # should not be useful.. + found = False + for i in S1.ds_error_log.match('.*Skip modrdn operation because src/dst identical.*'): + log.info('memberof log found: %s' % i) + found = True + assert(found) + + # If you need any test suite initialization, + # please, write additional fixture for that (including finalizer). + # Topology for suites are predefined in lib389/topologies.py. + + # If you need host, port or any other data about instance, + # Please, use the instance object attributes for that (for example, topo.ms["master1"].serverid) + + if DEBUGGING: + # Add debugging steps(if any)... + pass + + +if __name__ == '__main__': + # Run isolated + # -s for DEBUG mode + CURRENT_FILE = os.path.realpath(__file__) + pytest.main("-s %s" % CURRENT_FILE) + diff --git a/ldap/servers/plugins/memberof/memberof.c b/ldap/servers/plugins/memberof/memberof.c index 44b52ed..6245930 100644 --- a/ldap/servers/plugins/memberof/memberof.c +++ b/ldap/servers/plugins/memberof/memberof.c @@ -886,6 +886,16 @@ memberof_postop_modrdn(Slapi_PBlock *pb) pre_sdn = slapi_entry_get_sdn(pre_e); post_sdn = slapi_entry_get_sdn(post_e); } + + if (pre_sdn && post_sdn && slapi_sdn_compare(pre_sdn, post_sdn) == 0) { + /* Regarding memberof plugin, this rename is a no-op + * but it can be expensive to process it. So skip it + */ + slapi_log_err(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, + "memberof_postop_modrdn: Skip modrdn operation because src/dst identical %s\n", + slapi_sdn_get_dn(post_sdn)); + goto skip_op; + } /* copy config so it doesn't change out from under us */ memberof_rlock_config(); @@ -968,6 +978,7 @@ memberof_postop_modrdn(Slapi_PBlock *pb) memberof_free_config(&configCopy); } +skip_op: if (ret) { slapi_pblock_set(pb, SLAPI_RESULT_CODE, &ret); ret = SLAPI_PLUGIN_FAILURE;