#51006 Issue 51005 - AttributeUniqueness plugin's DN parameter should not have a default value
Closed by spichugi. Opened by vashirov.
vashirov/389-ds-base uniqattr  into  master

Download 51006.patch

Bug Description:
Currently we have an optional dn parameter with a default value:

def init(self, instance, dn="cn=plugins,cn=config"):

This breaks some tests that assume the default base DN for this plugin.
But it can have multiple instances, so we should always specify the DN.
Making this parameter mandatory will ensure this.

Fix Description:
Remove the default DN value and update tests.

Fixes: https://pagure.io/389-ds-base/issue/51005

Reviewed by: ???

This change is going to break the CLI and UI.

I don't understand the problem here. In your CI test when it creates the entry this should work without setting the basedn. It uses the "cn" attribute/property to generate the RDN of the new entry:

attruniq = AttributeUniquenessPlugin(topology_st.standalone)
attruniq.create(properties={'cn': 'attruniq'})

This should create an entry: cn=attruniq,cn=plugins,cn=config

The CLI uses this code:

PLUGIN_DN = "cn=plugins,cn=config"
...
def attruniq_add(inst, basedn, log, args):
    log = log.getChild('attruniq_add')
    props = {'cn': args.NAME}
    ...
    generic_object_add(AttributeUniquenessPlugin, inst, log, args, arg_to_attr, basedn=PLUGIN_DN, props=props)

I confirmed this works as expected:

# dsconf -v localhost plugin attr-uniq add attruniq --attr-name cn --subtree ou=people,dc=example,dc=com
...
DEBUG: Using first property cn: attruniq as rdn
DEBUG: Validated dn cn=attruniq,cn=plugins,cn=config
DEBUG: Creating cn=attruniq,cn=plugins,cn=config
...

Actually this won't break the CLI or UI, but it should still work as I suggested

The problem is if I don't specify the dn, it fails on latest master:

        attruniq = AttributeUniquenessPlugin(topology_st.standalone)
>       attruniq.enable()
dirsrvtests/tests/suites/betxns/betxn_test.py:101:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
src/lib389/lib389/plugins.py:58: in enable
    self.set('nsslapd-pluginEnabled', 'on')
src/lib389/lib389/_mapped_object.py:448: in set
    escapehatch='i am sure')
src/lib389/lib389/__init__.py:180: in inner
    return f(*args, **kwargs)
/usr/lib64/python3.6/site-packages/ldap/ldapobject.py:602: in modify_ext_s
    resp_type, resp_data, resp_msgid, resp_ctrls = self.result3(msgid,all=1,timeout=self.timeout)
src/lib389/lib389/__init__.py:180: in inner
    return f(*args, **kwargs)
/usr/lib64/python3.6/site-packages/ldap/ldapobject.py:749: in result3
    resp_ctrl_classes=resp_ctrl_classes
src/lib389/lib389/__init__.py:180: in inner
    return f(*args, **kwargs)
/usr/lib64/python3.6/site-packages/ldap/ldapobject.py:756: in result4
    ldap_result = self._ldap_call(self._l.result4,msgid,all,timeout,add_ctrls,add_intermediates,add_extop)
src/lib389/lib389/__init__.py:180: in inner
    return f(*args, **kwargs)
/usr/lib64/python3.6/site-packages/ldap/ldapobject.py:329: in _ldap_call
    reraise(exc_type, exc_value, exc_traceback)
/usr/lib64/python3.6/site-packages/ldap/compat.py:44: in reraise
    raise exc_value
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
self = <lib389.DirSrv object at 0x7fe597ef3b38>, func = <built-in method result4 of LDAP object at 0x7fe597ebb440>, args = (3, 1, -1, 0, 0, 0), kwargs = {}
diagnostic_message_success = None, exc_type = None, exc_value = None, exc_traceback = None
    def _ldap_call(self,func,*args,**kwargs):
      """
        Wrapper method mainly for serializing calls into OpenLDAP libs
        and trace logs
        """
      self._ldap_object_lock.acquire()
      if __debug__:
        if self._trace_level>=1:
          self._trace_file.write('*** %s %s - %s\n%s\n' % (
            repr(self),
            self._uri,
            '.'.join((self.__class__.__name__,func.__name__)),
            pprint.pformat((args,kwargs))
          ))
          if self._trace_level>=9:
            traceback.print_stack(limit=self._trace_stack_limit,file=self._trace_file)
      diagnostic_message_success = None
      try:
        try:
>         result = func(*args,**kwargs)
E         ldap.OBJECT_CLASS_VIOLATION: {'desc': 'Object class violation', 'info': 'attribute "nsslapd-pluginEnabled" not allowed\n'}
/usr/lib64/python3.6/site-packages/ldap/ldapobject.py:313: OBJECT_CLASS_VIOLATION
>

Previously it was working, because the default instance of the plugin was used (cn=attribute uniqueness,cn=plugins,cn=config). So I fixed the tests to always specify the dn.

But I also changed the function signature so that the failure would be more obvious:

>       attruniq = AttributeUniquenessPlugin(topology_st.standalone)
E       TypeError: __init__() missing 1 required positional argument: 'dn'
dirsrvtests/tests/suites/betxns/betxn_test.py:100: TypeError

This still doesn't add up. I don't see the plugin being created in your stack trace, just a modify operation. Are you looking for an existing plugin? I would think you should be doing this like:

    plugins = AttributeUniquenessPlugins(inst)
    for plugin in plugins.list():
         if plugin.rdn == "attribute uniqueness": 
            plugin.enable()

This still doesn't add up. I don't see the plugin being created in your stack trace, just a modify operation. Are you looking for an existing plugin? I would think you should be doing this like:
plugins = AttributeUniquenessPlugins(inst)
for plugin in plugins.list():
if plugin.rdn == "attribute uniqueness":
plugin.enable()

Okay, I see what you are trying to enforce. I got mixed up between AttributeUniquenessPlugins & AttributeUniquenessPlugin.

The current patch does look good, ack!

Thanks, Mark!
I'd like to add some clarification. I mentioned some issues with the Plugin class some time ago in #50516.
We have some plugins already preconfigured in template-dse.ldif. They have a default configuration entry. So from that point of view, we have 4 types of plugins:
1. Plugins that can have only one instance
So we can just enable them and start working with it:

plugin.enable()
  1. Plugins that can have only one instance but don't have an entry in cn=config.
    So we first have to create a plugin entry to start working with it:
plugin.create(dn='foo') or  plugin.ensure_state(dn='foo')
plugin.enable()
  1. Plugins that can have multiple instances with the default one preconfigured in the template.
    First instance can be enabled right away, but the next ones should be created first:
plugin1.enable()
plugin2.create(dn='foo')
plugin2.enable()
  1. Plugins that can have multiple instances but don't have an entry in cn=config
    Each instance should be created first:
plugin1.create(dn='bar')
plugin1.enable()
plugin2.create(dn='foo')
plugin2.enable()

Just looking at the plugin in lib389 it's hard to tell what approach I should use.
So with this change option [3] should go away for AttributeUniqueness plugin and only [4] should be used with it.

rebased onto 11bf641283a9283c45fba97831e5dc590a53a411

rebased onto 426e4e087e630c87fa87e850c78c0f4bb832e18d

Pull-Request has been merged by vashirov

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

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

Metadata