Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

is nsscache python3 compatible? #89

Closed
sandrotosi opened this issue Oct 2, 2019 · 25 comments
Closed

is nsscache python3 compatible? #89

sandrotosi opened this issue Oct 2, 2019 · 25 comments

Comments

@sandrotosi
Copy link

travis config points to some python3 images, but if you run tests there, they fail as the code is clearly not ready yet (print statements all over, and other python2-isms).

is there any effort to port it to python3 anytime soon?

In Debian we're trying to phase out python2 during this development cycle.

thanks!
@jaqx0r
Copy link
Contributor

jaqx0r commented Oct 2, 2019 via email

@jaqx0r jaqx0r mentioned this issue Oct 27, 2019
Closed
jaqx0r added a commit that referenced this issue Oct 27, 2019
@jaqx0r
Update debian packages to the python3 versions.
823945d 
Issue #89
jaqx0r added a commit that referenced this issue Oct 27, 2019
@jaqx0r
More hacks to try to get pycurl to install.
320987e 
Issue #89
jaqx0r added a commit that referenced this issue Oct 27, 2019
@jaqx0r
Remove version restriction on ldap3.
ccf221b 
Issue #89
@3c2b2ff5
Copy link
Contributor

I modifed the the print functions, exceptions, imports, assets in test files and modified the nss.pyand nss_tets.py and the maps.py to pass Python 3 test.
Now I am struggling with string/byte handling for ldapsource.

jaqx0r added a commit that referenced this issue Oct 28, 2019
@jaqx0r
Let's try to get any old build succeeding.
c29ffec 
Issue #89
@jaqx0r
Copy link
Contributor

jaqx0r commented Oct 28, 2019 via email

@3c2b2ff5
Copy link
Contributor

3c2b2ff5 commented Oct 28, 2019 •
edited
Loading

I thought about it a while a go. Well that means a lot of work. I found python3-ldap, isn't it the python3 version of python-ldap?

@3c2b2ff5
Copy link
Contributor

when I run my pull request with Python 3 I get the following error, this has to do with python-ldap library I guess and how it returns the data, so either go through the code and make it Python 3 compatible, which can be very time consuming and frustrating or import to python3-ldap which is Python3 only as I understand or migrate to ldap3, which claims to do both.

root@test: # python3 /usr/sbin/nsscache update -f
Traceback (most recent call last):
  File "/usr/sbin/nsscache", line 34, in <module>
    return_value = nsscache_app.Run(sys.argv[1:], os.environ)
  File "/usr/lib/python3/dist-packages/nss_cache/app.py", line 240, in Run
    retval = command_callable().Run(conf=conf, args=args)
  File "/usr/lib/python3/dist-packages/nss_cache/command.py", line 230, in Run
    force_lock=options.force_lock)
  File "/usr/lib/python3/dist-packages/nss_cache/command.py", line 303, in UpdateMaps
    force_write=force_write)
  File "/usr/lib/python3/dist-packages/nss_cache/update/updater.py", line 275, in UpdateFromSource
    force_write, location=None)
  File "/usr/lib/python3/dist-packages/nss_cache/update/map_updater.py", line 75, in UpdateCacheFromSource
    location=location)
  File "/usr/lib/python3/dist-packages/nss_cache/sources/source.py", line 63, in GetMap
    return self.GetPasswdMap(since)
  File "/usr/lib/python3/dist-packages/nss_cache/sources/ldapsource.py", line 420, in GetPasswdMap
    since=since)
  File "/usr/lib/python3/dist-packages/nss_cache/sources/ldapsource.py", line 664, in GetUpdates
    obj_ts = self.FromLdapToTimestamp(obj['whenChanged'][0])
  File "/usr/lib/python3/dist-packages/nss_cache/sources/ldapsource.py", line 572, in FromLdapToTimestamp
    t = time.strptime(ldap_ts_string, '%Y%m%d%H%M%S.0Z')
  File "/usr/lib/python3.7/_strptime.py", line 571, in _strptime_time
    tt = _strptime(data_string, format)[0]
  File "/usr/lib/python3.7/_strptime.py", line 326, in _strptime
    raise TypeError(msg.format(index, type(arg)))
TypeError: strptime() argument 0 must be str, not <class 'bytes'>

@jaqx0r
Copy link
Contributor

jaqx0r commented Oct 29, 2019 via email

@3c2b2ff5
Copy link
Contributor

I would also suggest to go with python3 only, since Python2 EOL is in few months, so it doesn't make any sense to go through he code or use cross-version framework like six.

@3c2b2ff5
Copy link
Contributor

Python3 represent the object data as bytes. An object looks like this:

{'displayName': [b'Test User'], 'objectSid': [b'\x01\x05\x00\x00\x00\x00\x00\x05\x15\x00\x00\x00\xa0e\xcf~xK\x9b_\xe7|\x87p\t\x1c\x01\x00'], 'sAMAccountName': [b'user'], 'pwdLastSet': [b'132144982087435710'], 'whenChanged': [b'20191020171614.0Z']}

nsscachedoes string as well bytes operations with the object data, for instance writing to cache needs a byte-like object, however string methods need a string object.

In Python 2 this wasn't problem, since bytes are treated as strings an vise versa. Now we need to specify it all over the code, so if we keep the the byte objects, we have to decode them to string where ever needed in the code, or the other way around, which I think is the better way, especially for writing the cache, a passwd entry must be a string, other wise will end up with an empty cache.

@3c2b2ff5
Copy link
Contributor

I made few changes to merge to Python 3 only, caching and verifying works, at least for AD. Still have 34 errors when I do the runtests. But I am on it.

@jaqx0r
Copy link
Contributor

jaqx0r commented Oct 31, 2019 via email

jaqx0r added a commit that referenced this issue Oct 31, 2019
@jaqx0r
Merge branch '2to3'
58cc284 
Issue #89
@3c2b2ff5
Copy link
Contributor

So you want to git rid of Python 2 completely and not supporting both. Its good. I am curious about the errors you will fix.

@3c2b2ff5
Copy link
Contributor

If you want me to work on anything, just let me know.

@3c2b2ff5
Copy link
Contributor

3c2b2ff5 commented Oct 31, 2019 •
edited
Loading

I could fix some nssdb errors and failures in python3_second, further more I changed some imports to import only the library needed, I removed duplicates in requirements.txt.
I noticed you have changed the logging.warning back to logging.warn, which is now deprecated

>>> import logging
>>> logging.warn is logging.warning
True

Now there are FAILED (failures=2, errors=3, skipped=7) plus the annoying unclosed files warning.

I hope I can get through the rest, unless you already have a fix.

jaqx0r added a commit that referenced this issue Oct 31, 2019
@jaqx0r
Fix some byte conversions.
7b44b8f 
Issue #89
@jaqx0r
Copy link
Contributor

jaqx0r commented Oct 31, 2019 via email

@3c2b2ff5
Copy link
Contributor

3c2b2ff5 commented Oct 31, 2019 •
edited
Loading

I fixed one failure in conulsource.py it seems that if the default password is empty the returned default value * will be encoded:

    map_entry.passwd = entry.get('passwd', '*')
    if isinstance(map_entry.passwd, bytes):
      map_entry.passwd = map_entry.passwd.decode('ascii')

But I didn't find the fix for the groups yet

@3c2b2ff5
Copy link
Contributor

I just pushed my last changes to python3_second

@3c2b2ff5
Copy link
Contributor

btw if you changed the lambda expression this way:

if any(c in name for c in map(lambda x: x, [' ', ':', '\n'])):

you can pass strings in all the tests except the load bdb cache file. It seems btopen expect a byte-like object. In my changes I modified the lambda expression and removed the byte like from consulsource_test.py.

I just pushed a cosmetic change in ldapsource_test.py

@3c2b2ff5
Copy link
Contributor

3c2b2ff5 commented Oct 31, 2019 •
edited
Loading

shall I make a pull request with my resent changes and you modify it accordingly to your fixes for the errors?

@jaqx0r
Copy link
Contributor

jaqx0r commented Nov 1, 2019 via email

@3c2b2ff5
Copy link
Contributor

3c2b2ff5 commented Nov 1, 2019

I realized that self.assertEqual(self.good_entry, group_map.PopItem()) in consulsource_test.py breaks the tests for empty members, it doesn't get further to self.assertEqual(self.good_entry, entry) in empty groups, if it is commented out the test succeeds.
The open still need to be closed, I closed the others but I've been struggling with these two:

  func(self, *args, **kwargs)
Object allocated at (most recent call last):
  File "/usr/lib/python3.7/os.py", lineno 1026
    return io.open(fd, *args, **kwargs)
..../opt/nsscache/nss_cache/caches/caches.py:101: ResourceWarning: unclosed file <_io.BufferedRandom name=3>
  self.temp_cache_file = os.fdopen(fd, 'w+b')
Object allocated at (most recent call last):
  File "/usr/lib/python3.7/os.py", lineno 1026
    return io.open(fd, *args, **kwargs)
/usr/lib/python3/dist-packages/mox3/mox.py:2139: ResourceWarning: unclosed file <_io.BufferedRandom name=4>
  func(self, *args, **kwargs)
Object allocated at (most recent call last):
  File "/usr/lib/python3.7/os.py", lineno 1026
    return io.open(fd, *args, **kwargs)
...............................................................................................s....ss....s.s....ss.........................../usr/lib/python3/dist-packages/mox3/mox.py:2139: ResourceWarning: unclosed file <_io.TextIOWrapper name='/tmp/tmpht28x68x/pidfile' mode='a+' encoding='UTF-8'>
  func(self, *args, **kwargs)
Object allocated at (most recent call last):
  File "/opt/nsscache/nss_cache/lock.py", lineno 108
    self._file = open(filename, 'a+')
.../usr/lib/python3/dist-packages/mox3/mox.py:2139: ResourceWarning: unclosed file <_io.TextIOWrapper name='/tmp/tmpw8266zwv/pidfile' mode='a+' encoding='UTF-8'>
  func(self, *args, **kwargs)
Object allocated at (most recent call last):
  File "/opt/nsscache/nss_cache/lock.py", lineno 108
    self._file = open(filename, 'a+')

@jaqx0r
Copy link
Contributor

jaqx0r commented Nov 1, 2019

As of HEAD now ./runtests.py passes on all tests in python3.

There's more work to do but I think we can close this bug and tag a release.

Thanks @3c2b2ff5 for your help!

@3c2b2ff5
Copy link
Contributor

3c2b2ff5 commented Nov 1, 2019

Grandiose! Please let me know if how you want to proceed and what is the next step to take. I'll be happy to help.

@jaqx0r
Copy link
Contributor

jaqx0r commented Nov 1, 2019 via email

@3c2b2ff5
Copy link
Contributor

3c2b2ff5 commented Nov 1, 2019

Normally yes. I'll do next week excessive tests on ldapsource, with AD and openldap. Unfortunately I cannot test the other modules, we have multiple consul cluster running in the company but as long I don't understand the code, any approach in this direction will fail.

@jaqx0r
Copy link
Contributor

jaqx0r commented Nov 1, 2019 via email

@jaqx0r jaqx0r closed this as completed Nov 1, 2019
@alexeiser alexeiser mentioned this issue Nov 3, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@sandrotosi @jaqx0r @3c2b2ff5