#50244 Ticket 50243 - refint modrdn stress test
Closed 3 years ago by spichugi. Opened 5 years ago by firstyear.
firstyear/389-ds-base xxxxx-possible-rename-issue  into  master

@@ -0,0 +1,177 @@ 

+ # --- BEGIN COPYRIGHT BLOCK ---

+ # Copyright (C) 2019 William Brown <william@blackhats.net.au>

+ # All rights reserved.

+ #

+ # License: GPL (version 3 or any later version).

+ # See LICENSE for details.

+ # --- END COPYRIGHT BLOCK ---

+ #

+ 

+ 

+ from lib389._constants import DEFAULT_SUFFIX

+ from lib389.topologies import topology_m2

+ 

+ from lib389.replica import ReplicationManager

+ from lib389.idm.group import Groups

+ from lib389.idm.user import nsUserAccounts

+ from lib389.idm.organizationalunit import OrganizationalUnit as OrganisationalUnit

+ 

+ from lib389.plugins import AutoMembershipPlugin, ReferentialIntegrityPlugin, AutoMembershipDefinitions, MemberOfPlugin

+ 

+ UCOUNT = 400

+ 

+ def _enable_plugins(inst, group_dn):

+     # Enable automember

+     amp = AutoMembershipPlugin(inst)

+     amp.enable()

+ 

+     # Create the automember definition

+     automembers = AutoMembershipDefinitions(inst)

+ 

+     automember = automembers.create(properties={

+         'cn': 'testgroup_definition',

+         'autoMemberScope': DEFAULT_SUFFIX,

+         'autoMemberFilter': 'objectclass=nsAccount',

+         'autoMemberDefaultGroup': group_dn,

+         'autoMemberGroupingAttr': 'member:dn',

+     })

+ 

+     # Enable MemberOf

+     mop = MemberOfPlugin(inst)

+     mop.enable()

+ 

+     # Enable referint

+     rip = ReferentialIntegrityPlugin(inst)

+     # We only need to enable the plugin, the default configuration is sane and

+     # correctly coveres member as an enforced attribute.

+     rip.enable()

+ 

+     # Restart to make sure it's enabled and good to go.

+     inst.restart()

+ 

+ def test_rename_large_subtree(topology_m2):

+     """

+     A report stated that the following configuration would lead

+     to an operation failure:

+ 

+     ou=int,ou=account,dc=...

+     ou=s1,ou=int,ou=account,dc=...

+     ou=s2,ou=int,ou=account,dc=...

+ 

+     rename ou=s1 to re-parent to ou=account, leaving:

+ 

+     ou=int,ou=account,dc=...

+     ou=s1,ou=account,dc=...

+     ou=s2,ou=account,dc=...

+ 

+     The ou=s1 if it has < 100 entries below, is able to be reparented.

+ 

+     If ou=s1 has > 400 entries, it fails.

+ 

+     Other conditions was the presence of referential integrity - so one would

+     assume that all users under s1 are a member of some group external to this.

+ 

+     :id: 5915c38d-b3c2-4b7c-af76-8a1e002e27f7

+ 

+     :setup: standalone instance

+ 

+     :steps: 1. Enable automember plugin

+             2. Add UCOUNT users, and ensure they are members of a group.

+             3. Enable refer-int plugin

+             4. Move ou=s1 to a new parent

+ 

+     :expectedresults:

+         1. The plugin is enabled

+         2. The users are members of the group

+         3. The plugin is enabled

+         4. The rename operation of ou=s1 succeeds

+     """

+ 

+     st = topology_m2.ms["master1"]

+     m2 = topology_m2.ms["master2"]

+ 

+     # Create a default group

+     gps = Groups(st, DEFAULT_SUFFIX)

+     # Keep the group so we can get it's DN out.

+     group = gps.create(properties={

+         'cn': 'default_group'

+     })

+ 

+     _enable_plugins(st, group.dn)

+     _enable_plugins(m2, group.dn)

+ 

+     # Now unlike normal, we bypass the plural-create method, because we need control

+     # over the exact DN of the OU to create.

+     # Create the ou=account

+ 

+     # We don't need to set a DN here because ...

+     ou_account = OrganisationalUnit(st)

+ 

+     # It's set in the .create step.

+     ou_account.create(

+         basedn = DEFAULT_SUFFIX,

+         properties={

+             'ou': 'account'

+         })

+     # create the ou=int,ou=account

+     ou_int = OrganisationalUnit(st)

+     ou_int.create(

+         basedn = ou_account.dn,

+         properties={

+             'ou': 'int'

+         })

+     # Create the ou=s1,ou=int,ou=account

+     ou_s1 = OrganisationalUnit(st)

+     ou_s1.create(

+         basedn = ou_int.dn,

+         properties={

+             'ou': 's1'

+         })

+ 

+     # Pause replication

+     repl = ReplicationManager(DEFAULT_SUFFIX)

+     repl.disable_to_master(m2, [st, ])

+ 

+     # Create the users 1 -> UCOUNT in ou=s1

+     nsu = nsUserAccounts(st, basedn=ou_s1.dn, rdn=None)

+     for i in range(1000, 1000 + UCOUNT):

+         nsu.create_test_user(uid=i)

+ 

+     # Enable replication

+ 

+     repl.enable_to_master(m2, [st, ])

+ 

+     # Assert they are in the group as we expect

+     members = group.get_attr_vals_utf8('member')

+     assert len(members) == UCOUNT

+ 

+     # Wait for replication

+     repl.wait_for_replication(st, m2)

+ 

+     for i in range(0, 5):

+         # Move ou=s1 to ou=account as parent. We have to provide the rdn,

+         # even though it's not changing.

+         ou_s1.rename('ou=s1', newsuperior=ou_account.dn)

+ 

+         members = group.get_attr_vals_utf8('member')

+         assert len(members) == UCOUNT

+         # Check that we really did refer-int properly, and ou=int is not in the members.

+         for member in members:

+             assert 'ou=int' not in member

+ 

+         # Now move it back

+         ou_s1.rename('ou=s1', newsuperior=ou_int.dn)

+         members = group.get_attr_vals_utf8('member')

+         assert len(members) == UCOUNT

+         for member in members:

+             assert 'ou=int' in member

+ 

+     # Check everythig on the other side is good.

+     repl.wait_for_replication(st, m2)

+ 

+     group2 = Groups(m2, DEFAULT_SUFFIX).get('default_group')

+ 

+     members = group2.get_attr_vals_utf8('member')

+     assert len(members) == UCOUNT

+     for member in members:

+         assert 'ou=int' in member

file modified
+1 -1
@@ -911,7 +911,7 @@ 

      :type dn: str

      """

  

-     def __init__(self, instance, dn=None):

+     def __init__(self, instance, dn):

          super(AutoMembershipDefinition, self).__init__(instance, dn)

          self._rdn_attribute = 'cn'

          self._must_attributes = ['cn', 'autoMemberScope', 'autoMemberFilter', 'autoMemberGroupingAttr']

file modified
+4 -2
@@ -1214,7 +1214,8 @@ 

              scope=ldap.SCOPE_SUBTREE,

              filterstr='(&(nsuniqueid=ffffffff-ffffffff-ffffffff-ffffffff)(objectclass=nstombstone))',

              attrlist=['nsds50ruv'],

-             serverctrls=self._server_controls, clientctrls=self._client_controls)[0]

+             serverctrls=self._server_controls, clientctrls=self._client_controls,

+             escapehatch='i am sure')[0]

  

          data = ensure_list_str(ent.getValues('nsds50ruv'))

  
@@ -1233,7 +1234,8 @@ 

              scope=ldap.SCOPE_SUBTREE,

              filterstr='(&(nsuniqueid=ffffffff-ffffffff-ffffffff-ffffffff)(objectclass=nstombstone))',

              attrlist=['nsds5agmtmaxcsn'],

-             serverctrls=self._server_controls, clientctrls=self._client_controls)[0]

+             serverctrls=self._server_controls, clientctrls=self._client_controls,

+             escapehatch='i am sure')[0]

  

          return ensure_list_str(ent.getValues('nsds5agmtmaxcsn'))

  

Bug Description: It was reported that modrdn of an ou which
contained many items could break refint in some cases.

Fix Description: Add a stress test to try to reproduce the issue

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

Author: William Brown william@blackhats.net.au

Review by: ???

Shouldn't it be in DSLdapObjects basedn? (And you are adding it to DSLdapObject - the child)

The rest looks good to me

@spichugi Singulars allow direct initialisation in some cases for DSLdapObject, so in this case where there is only one plugin definition, we pre-set the basedn, allowing you to do:

plugin = SingularPlugin(inst)

@spichugi Singulars allow direct initialisation in some cases for DSLdapObject, so in this case where there is only one plugin definition, we pre-set the basedn, allowing you to do:
plugin = SingularPlugin(inst)

It seems odd to me...

  • First, you've set the plugin DN but in case you want to have some default definition - the definition DN should be set ("cn=Example Automember Definition,cn=Auto Membership Plugin,cn=plugins,cn=config").
  • Second, the definition should be created anyway (it is not added any place during the instance installation). So the lib389 API user will have their own definitions with the names that are used for their topology. And, I think, it is more explicit do not assume these names for him.
  • Third, you've added the feature but you don't use it in your code... Just a small nitpick :)

Ahhhhh I was misreading the patch. I was a bit unwell this morning. So I think your right, the dn= doesn't belong.

What feature did I add? I do use automembership in the code ...

rebased onto 643d8d4d48806c4fb460a6eef5ebed32babdd748

5 years ago

What feature did I add? I do use automembership in the code ...

I was talking about SingularDefinition() object. You've changed the 'dn' part but then you used the MulitpleDefinition() object and SingularDefinition() was not used in your code at all.

But now it doesn't matter. :)

Thank you! PR looks good to me.

I think that was the diff viewer messing with me maybe :) Anyway, fixed now.

rebased onto 45e8474

5 years ago

Pull-Request has been merged by firstyear

5 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/3303

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.

Pull-Request has been closed by spichugi

3 years ago