#95 Extend tls certmap test
Closed: Fixed 6 years ago Opened 6 years ago by firstyear.

Extend the tls certmap test to account for more configurations.


Metadata Update from @firstyear:
- Custom field Origin adjusted to None
- Custom field Review Status adjusted to None

6 years ago

Adding 'nsMemberOf' to self._create_objectclasses in idm/user.py makes a lot of tests fail where we don't have the objectclass in 389-ds-base (as I understand, we don't have it on RHEL 7).
So, probably, we need to deal with it in lib389, because a lot of tests rely on idm/user.py module.

Beside that, I know not much about https://pagure.io/389-ds-base/issue/49218
so I'll give a chance to @mreynolds to review the patch also. I'll read about the feature anyway.

Also, with 389-ds-base-1.3.7.1-20170822git8965a8f.fc26.x86_64 while creating a UserAccounts,
there is a lot of failures like this:

    master_users = UserAccounts(topology.master, SUFFIX)
    consumer_users = UserAccounts(topology.consumer, SUFFIX)

   >      testuser = master_users.create(properties=TEST_USER_PROPERTIES)

   lib389/tests/agreement_test.py:116:
   _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
   lib389/_mapped_object.py:632: in create
       return co.create(rdn, properties, self._basedn)
   lib389/_mapped_object.py:475: in create
       self._instance.add_s(e)
   lib389/__init__.py:158: in inner
       return f(ent.dn, ent.toTupleList(), *args[2:])
   /usr/lib64/python2.7/site-packages/ldap/ldapobject.py:216: in add_s
       return self.add_ext_s(dn,modlist,None,None)
   lib389/__init__.py:160: in inner
       return f(*args, **kwargs)
   /usr/lib64/python2.7/site-packages/ldap/ldapobject.py:202: in add_ext_s
       resp_type, resp_data, resp_msgid, resp_ctrls = self.result3(msgid,all=1,timeout=self.timeout)
   lib389/__init__.py:162: in inner
       return f(*args, **kwargs)
   /usr/lib64/python2.7/site-packages/ldap/ldapobject.py:519: in result3
       resp_ctrl_classes=resp_ctrl_classes
   lib389/__init__.py:162: in inner
       return f(*args, **kwargs)
   /usr/lib64/python2.7/site-packages/ldap/ldapobject.py:526: in result4
       ldap_result = self._ldap_call(self._l.result4,msgid,all,timeout,add_ctrls,add_intermediates,add_extop)
   lib389/__init__.py:162: in inner
       return f(*args, **kwargs)
   _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

   self = <lib389.DirSrv object at 0x7fb8fa0d7ed0>
   func = <built-in method result4 of LDAP object at 0x7fb8fa127af8>, args = (20, 1, -1, 0, 0, 0)
   kwargs = {}, diagnostic_message_success = None
   e = TYPE_OR_VALUE_EXISTS({'desc': 'Type or value exists'},)

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         TYPE_OR_VALUE_EXISTS: {'desc': 'Type or value exists'}

   /usr/lib64/python2.7/site-packages/ldap/ldapobject.py:108: TYPE_OR_VALUE_EXISTS

I haven't seen that error before myself :s

With the nsMemberOf, maybe I could add a version gate to that? so that only if 1.3.7 or higher perhaps? because I have another objectClass in this patch which is being added as part of the ticket as well.

Maybe this means I need to version gate the TLS external test, because it relies on a schema feature I'm adding in https://pagure.io/389-ds-base/issue/49218

@spichugi @mreynolds Do you mind looking over this patch as well? :) Thanks!

The code looks good to me. Can we already test it or do we need to wait till #49218 will be merged?
Also, as the patch relies on a new change, should we skip the test on the older versions?

I think we can now merge this if I tweak the change in users.py to be versioned to 1.4.x not 1.3.x as there is a missing objectClass. That or I can add that seperately. That's the main blocker.

0009-Ticket-49218-Certmap-support-TLS-tests.patch

@spichugi This is the improvements to the tls system to support tls on instances, seperate CA, client certs and more.

I have kept the certmap patches seperate from this, so it should work with current master (IE no certmap patch)

Thanks!

Big and nice foundation for future full working SSL topology setup. Thanks!

A few issues that we already discussed:
1) Some debug leftovers:

174 +        print(slapd)

2) Wrong version checks for new schema items like 'nsAccount', etc.
3) You've changed 'openConnection' to 'clone' methods. We still use it somewhere
dirsrvtests/tests/suites/sasl/plain_test.py
dirsrvteststests/stress/reliabilty/reliab_conn_test.py
dirsrvteststests/stress/reliabilty/reliab_7_5_test.py
dirsrvteststests/tickets/ticket47931_test.py

And for 'nss_ssl' module cahnge:
src/lib389/lib389/tests/tls_external_test.py
dirsrvteststests/suites/sasl/plain_test.py
dirsrvteststests/tickets/ticket47838_test.py
dirsrvteststests/tickets/ticket49039_test.py
dirsrvteststests/tickets/ticket49303_test.py
dirsrvteststests/tickets/ticket48784_test.py
dirsrvteststests/tickets/ticket48798_test.py
dirsrvteststests/tickets/ticket48194_test.py

I hope, I didn't miss anything.

There is a huge refactor for the tls_external_test coming in the certmap ticket, so I'll ignore that for now.

I'll fix the others now,

0001-Ticket-49218-Certmap-support-TLS-tests.patch

Okay, this fixes some of the single master tests to work with this (ie sasl_plain). It has a small number of refactors to help with it. The biggest issue is MMR versions, I need to issue to many fixesto agreement/replica, so I'd like to do those as a follow up patch.

If we review this now, I'll keep working on it :)

It was intense, great patch!

One small thing about this. I think it'll be more readable with as topology_st.standalone.enable_tls().

239 +    [i.enable_tls() for i in topology_st]
452 +    [i.enable_tls() for i in topology_st]

Metadata Update from @spichugi:
- Custom field Review Status adjusted to ack (was: None)

6 years ago

commit d9ad6fd9b0cec2b41f0a21798958f97d37e018e2
To ssh://git@pagure.io/389-ds-base.git
20efeea..d9ad6fd master -> master

Made those two changes and re-tested (passed). Thanks for your great help reviewing and your comments. Much appreciated :)

Metadata Update from @firstyear:
- Issue close_status updated to: Fixed
- Issue status updated to: Closed (was: Open)

6 years ago

Login to comment on this ticket.

Metadata