Port ldap-sshkey2file.py to Python 3

It now also passes pytype and (mostly) flake8 checks.

2 files changed, 91 insertions, 96 deletions

diff --git a/modules/openssh/manifests/ssh_keys_from_ldap.pp b/modules/openssh/manifests/ssh_keys_from_ldap.pp
index 5528c700..9ea6c139 100644
--- a/modules/openssh/manifests/ssh_keys_from_ldap.pp
+++ b/modules/openssh/manifests/ssh_keys_from_ldap.pp
@@ -1,24 +1,13 @@
 class openssh::ssh_keys_from_ldap inherits server {
-if versioncmp($::lsbdistrelease, '7') < 0 {
-    package { 'python-ldap': }
-} else {
-    package { 'python2-ldap': }
-}
+    package { 'python3-ldap': }
 
     $ldap_pwfile = '/etc/ldap.secret'
     $nslcd_conf_file = '/etc/nslcd.conf'
     $ldap_servers = get_ldap_servers()
-if versioncmp($::lsbdistrelease, '7') < 0 {
     mga_common::local_script { 'ldap-sshkey2file.py':
         content => template('openssh/ldap-sshkey2file.py'),
-        require => Package['python-ldap']
+        require => Package['python3-ldap']
     }
-} else {
-    mga_common::local_script { 'ldap-sshkey2file.py':
-        content => template('openssh/ldap-sshkey2file.py'),
-        require => Package['python2-ldap']
-    }
-}
 
     cron { 'sshkey2file':
         command     => '/bin/bash -c "/usr/local/bin/ldap-sshkey2file.py && ( [[ -f /usr/bin/mgagit && -d /var/lib/git/.gitolite ]] && /bin/su -c \'/usr/bin/mgagit glrun\' - git ) ||:"',
diff --git a/modules/openssh/templates/ldap-sshkey2file.py b/modules/openssh/templates/ldap-sshkey2file.py
index 0737d822..7a0a7de5 100755
--- a/modules/openssh/templates/ldap-sshkey2file.py
+++ b/modules/openssh/templates/ldap-sshkey2file.py
@@ -1,53 +1,57 @@
-#!/usr/bin/python2
+#!/usr/bin/python3
 
-import sys
+import argparse
 import os
 import random
 import shutil
+import sys
 import tempfile
-import argparse
 import textwrap
+from typing import Iterable
 
 try:
     import ldap
-except ImportError, e:
-    print "Please install python-ldap before running this program"
+except ImportError:
+    print("Please install python-ldap before running this program")
     sys.exit(1)
 
-basedn="<%= dc_suffix %>"
-peopledn="ou=people,%s" % basedn
+basedn = "<%= dc_suffix %>"
+peopledn = f"ou=people,{basedn}"
+'''
 <%-
   ldap_servers.map! { |l| "'ldaps://#{l}'" }
 -%>
-uris=[<%= ldap_servers.join(", ") %>]
+'''
+uris = ['''<%= ldap_servers.join(", ") %>''']
 random.shuffle(uris)
 uri = " ".join(uris)
-timeout=5
-binddn="cn=<%= fqdn %>,ou=Hosts,%s" % basedn
-ldap_secret_file="<%= ldap_pwfile %>"
-nslcd_conf_file="<%= nslcd_conf_file %>"
+timeout = 5
+binddn = f"cn=<%= fqdn %>,ou=Hosts,{basedn}"
+ldap_secret_file = "<%= ldap_pwfile %>"
+nslcd_conf_file = "<%= nslcd_conf_file %>"
 # filter out disabled accounts also
 # too bad uidNumber doesn't support >= filters
-filter="(&(objectClass=inetOrgPerson)(objectClass=ldapPublicKey)(objectClass=posixAccount)(sshPublicKey=*))"
-keypathprefix='/home'
+objfilter = "(&(objectClass=inetOrgPerson)(objectClass=ldapPublicKey)(objectClass=posixAccount)(sshPublicKey=*))"
+keypathprefix = "/home"
 
 parser = argparse.ArgumentParser(
     formatter_class=argparse.RawDescriptionHelpFormatter,
-    description=textwrap.dedent('''\
-        Will fetch all enabled user accounts under %s
+    description=textwrap.dedent(f'''\
+        Will fetch all enabled user accounts under {peopledn}
         with ssh keys in them and write each one to
-        %s/<login>/.ssh/authorized_keys
+        {keypathprefix}/<login>/.ssh/authorized_keys
 
         It will return failure when no keys are updated and success
         when one or more keys have changed.
 
         This script is intended to be run from cron as root;
-        ''' % (peopledn, keypathprefix)))
+        '''))
 parser.add_argument('-n', '--dry-run', action='store_true')
 parser.add_argument('-v', '--verbose', action='store_true')
 args = parser.parse_args()
 
-def get_bindpw():
+
+def get_bindpw() -> str:
     try:
         return get_nslcd_bindpw(nslcd_conf_file)
     except:
@@ -58,91 +62,90 @@ def get_bindpw():
     except:
         pass
 
-    print "Error while reading password file, aborting"
+    print("Error while reading password file, aborting")
     sys.exit(1)
 
-def get_nslcd_bindpw(pwfile):
+
+def get_nslcd_bindpw(pwfile: str) -> str:
     try:
-        f = open(pwfile, 'r')
-    except IOError, e:
-        print "Error while reading nslcd file " + pwfile
-        print e
+        with open(pwfile, 'r') as f:
+            pwfield = "bindpw"
+            for line in f:
+                ls = line.strip().split()
+                if len(ls) == 2 and ls[0] == pwfield:
+                    return ls[1]
+    except IOError as e:
+        print("Error while reading nslcd file " + pwfile)
+        print(e)
         raise
 
-    pwfield = "bindpw"
-    for line in f:
-        ls = line.strip().split()
-        if len(ls) == 2 and ls[0] == pwfield:
-            f.close()
-            return ls[1]
-    f.close()
-
-    print "No " + pwfield + " field found in nslcd file " + pwfile
+    print("No " + pwfield + " field found in nslcd file " + pwfile)
     raise Exception()
 
-def get_ldap_secret(pwfile):
+
+def get_ldap_secret(pwfile: str) -> str:
     try:
-        f = open(pwfile, 'r')
-    except IOError, e:
-        print "Error while reading password file " + pwfile
-        print e
+        with open(pwfile, 'r') as f:
+            pw = f.readline().strip()
+    except IOError as e:
+        print("Error while reading password file " + pwfile)
+        print(e)
         raise
-    pw = f.readline().strip()
-    f.close()
     return pw
 
-def write_keys(keys, user, uid, gid):
-    keyfile = "%s/%s/.ssh/authorized_keys" % (keypathprefix,user)
 
-    fromldap = ''
+def write_keys(keys: Iterable[bytes], user: bytes, uid: int, gid: int) -> bool:
+    userdir = f"{keypathprefix}/{user.decode('utf-8')}"
+    keyfile = f"{userdir}/.ssh/authorized_keys"
+
+    fromldap = ""
     for key in keys:
-        fromldap += key.strip() + "\n"
+        fromldap += key.decode("utf-8").strip() + "\n"
 
-    fromfile = ''
+    fromfile = ""
     try:
-        f = open(keyfile, 'r')
-        fromfile = f.read()
-        f.close()
-    except:
+        with open(keyfile, 'r') as f:
+            fromfile = f.read()
+    except FileNotFoundError:
         pass
 
     if fromldap == fromfile:
         return False
 
     if args.dry_run:
-        print "Would write %s" % keyfile
+        print(f"Would write {keyfile}")
         return True
 
     if args.verbose:
-        print "Writing %s" % keyfile
+        print(f"Writing {keyfile}")
 
-    if not os.path.isdir("%s/%s" % (keypathprefix,user)):
-       shutil.copytree('/etc/skel', "%s/%s" % (keypathprefix,user))
-       os.chown("%s/%s" % (keypathprefix,user), uid, gid)
-       for root, dirs, files in os.walk("%s/%s" % (keypathprefix,user)):
-           for d in dirs:
-               os.chown(os.path.join(root, d), uid, gid)
-           for f in files:
-               os.chown(os.path.join(root, f), uid, gid)
+    if not os.path.isdir(userdir):
+        shutil.copytree('/etc/skel', userdir)
+        os.chown(userdir, uid, gid)
+        for root, dirs, files in os.walk(userdir):
+            for d in dirs:
+                os.chown(os.path.join(root, d), uid, gid)
+            for f in files:
+                os.chown(os.path.join(root, f), uid, gid)
 
     try:
-        os.makedirs("%s/%s/.ssh" % (keypathprefix,user), 0700)
-    except:
+        os.makedirs(f"{userdir}/.ssh", 0o700)
+    except FileExistsError:
         pass
-    os.chmod("%s/%s/.ssh" % (keypathprefix,user), 0700)
-    os.chown("%s/%s/.ssh" % (keypathprefix,user), uid, gid)
-
-    (fd, tmpname) = tempfile.mkstemp('', 'ldap-sshkey2file-')
-    os.write(fd, fromldap);
-    os.close(fd)
-    os.chmod(tmpname, 0600)
-    os.chown(tmpname, uid, gid)
-    shutil.move(tmpname, keyfile)
-    # Hmm, apparently shutil.move does not preserve user/group so lets reapply
-    # them. I still like doing it before as this should be more "automic"
+    os.chmod(f"{userdir}/.ssh", 0o700)
+    os.chown(f"{userdir}/.ssh", uid, gid)
+
+    with tempfile.NamedTemporaryFile(
+            prefix='ldap-sshkey2file-', mode='w', delete=False) as tmpfile:
+        tmpfile.write(fromldap)
+    os.chmod(tmpfile.name, 0o600)
+    os.chown(tmpfile.name, uid, gid)
+    shutil.move(tmpfile.name, keyfile)
+    # Hmm, apparently shutil.move does not preserve user/group so let's reapply
+    # them. I still like doing it before as this should be more "atomic"
     # if it actually worked, so it's "good practice", even if shutil.move sucks
     os.chown(keyfile, uid, gid)
-    os.chmod(keyfile, 0600)
+    os.chmod(keyfile, 0o600)
     return True
 
 
@@ -155,35 +158,38 @@ try:
     if uri.startswith("ldap:/"):
         ld.start_tls_s()
     ld.bind_s(binddn, bindpw)
-    res = ld.search_s(peopledn, ldap.SCOPE_ONELEVEL, filter, ['uid','sshPublicKey','uidNumber','gidNumber'])
+    res = ld.search_s(peopledn, ldap.SCOPE_ONELEVEL, objfilter,
+                      ['uid', 'sshPublicKey', 'uidNumber', 'gidNumber'])
     try:
-        os.makedirs(keypathprefix, 0701)
-    except:
+        os.makedirs(keypathprefix, 0o701)
+    except FileExistsError:
         pass
 
     if args.verbose:
-        print "Found users: " + ", ".join(sorted(map(lambda x: x[1]['uid'][0], res)))
+        print("Found users:",
+              ", ".join(sorted([x[1]['uid'][0].decode('utf-8') for x in res])))
 
     for result in res:
         dn, entry = result
         # skip possible system users
         if 'uidNumber' not in entry or int(entry['uidNumber'][0]) < 500:
             continue
-        if write_keys(entry['sshPublicKey'], entry['uid'][0], int(entry['uidNumber'][0]), int(entry['gidNumber'][0])):
+        if write_keys(entry['sshPublicKey'], entry['uid'][0],
+                      int(entry['uidNumber'][0]), int(entry['gidNumber'][0])):
             changed = True
 
     ld.unbind_s()
-except Exception, e:
-    print "Error"
+except Exception:
+    print("Error")
     raise
 
 if changed:
     if args.verbose:
-        print "SSH keys changed"
+        print("SSH keys changed")
     sys.exit(0)
 
 if args.verbose:
-    print "No changes in SSH keys"
+    print("No changes in SSH keys")
 sys.exit(1)