Looks like we also need LDAPsubentry objectClass here.

If you are mentioning the objectClass here - it should be nsNestedRoleDefinition

We also need 'self._must_attributes' here which should contain nsRoleDN and cn

Typo. And variable should be lowercase. Please, run pytest-pylint here, also

Instead of UserAccount(topo.standalone, f'uid=test_user_1,ou=People,{DEFAULT_SUFFIX}') you can use existing UserAccounts instance and get users from it by selector.

The goal here is not to create a connection but to search while bound as the user

Looks like we also need LDAPsubentry objectClass here.

That will not be required , as it will come by default

If you are mentioning the objectClass here - it should be nsNestedRoleDefinition

In previous reviews @tbordaz asked me to remove all the 'ns' (except from object classes ) , for this one i will change it to NestedRole

@spichugi , all other changes are done as per your suggestion

Please, add another attribute I've asked to add. The definition won't work without it.

I think it makes sense to join the lists and run delete() operation on all of them. It will be 2 lines instead of 6.

Issue: 50313 - Making an NestedRoleDefinition type - is an unneccessary long line, I think.
And follow the basic commit message rules, please.
It should be something like - Issue: 50313 - Add a NestedRole type to lib389

And the commit message body should have some info about the test you've added too.

Typo. And variable should be lowercase. Please, run pytest-pylint here, also

Please, don't ignore my comments. :) You still have a bunch of pylint issues.
The code will be much more maintainable if we follow basic lint rules.

Just notice this, I would expect a role definition to be a subentry. This applies to managed/filtered/nested roles.
Could you confirm that the 'objectclass: ldapsubentry' was missing in original role definitions ?

I think that once the test is completed, all items added by the test should be deleted (roles (managed and nested), users and ACI).
This deletion should occur if the test succeeds or not (so may be using a python directtive like 'finally')

Just notice this, I would expect a role definition to be a subentry. This applies to managed/filtered/nested roles.
Could you confirm that the 'objectclass: ldapsubentry' was missing in original role definitions ?

objectclass , ldapsubentry will come by default . No need to put differently

I think that once the test is completed, all items added by the test should be deleted (roles (managed and nested), users and ACI).
This deletion should occur if the test succeeds or not (so may be using a python directtive like 'finally')

I have added one fixture to clean up the (roles (managed and nested), users and ACI).

@spichugi , all changes are done as per your suggestion

@tbordaz , @spichugi , checkout the out put for ldapsubentry , its coming by default without putting it separately

(Pdb) NestedRoles(topo.standalone, DEFAULT_SUFFIX).list()
[]
(Pdb) NestedRoles(topo.standalone, DEFAULT_SUFFIX).list()[0].dn
'cn=nested_role,dc=example,dc=com'
(Pdb) NestedRoles(topo.standalone, DEFAULT_SUFFIX).list()[0]._unsafe_raw_entry()
dn: cn=nested_role,dc=example,dc=com
cn: nested_role
objectClass: top
objectClass: nsRoleDefinition
objectClass: nsComplexRoleDefinition
objectClass: nsNestedRoleDefinition
objectClass: ldapSubEntry

Even though it is created automatically, we better have it as a part of the DSLdapObject and DSLdapObjects.
The explicitness will ensure that everything is in order.

Even though it is created automatically, we better have it as a part of the DSLdapObject and DSLdapObjects.
The explicitness will ensure that everything is in order. '

Done

@spichugi , just to be sure. Did ldapsubentry come from DSLdapObject or DSLdapObjects ?

@spichugi , just to be sure. Did ldapsubentry come from DSLdapObject or DSLdapObjects ?

DSLdapObject._create_objectclasses sets the objectClasses that will be created in a new entry
DSLdapObjects._objectclasses sets the objectClasses that will be used as a filter for the searches.

ManagedRole is an unused import

The commit message body should follow the same consistent imperative rule as the subject. And the class name should be consistent too. Something like this:

Add the NestedRole and the NestedRoles classes to src/lib389/lib389/idm/role.py
Add one test case that will test that the new class NestedRoles is
working fine.

@spichugi , all changes are done as per your suggestion

LGTM. Ack!

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/3373

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.