Improve DN usage in ipa-client-install

A hotfix pushed in a scope of ticket 3088 forced conversion of DN
object (baseDN) in IPA client discovery so that ipa-client-install
does not crash when creating an IPA default.conf. Since this is not
a preferred way to handle DN objects, improve its usage:

- make sure, that baseDN retrieved by client discovery is always
  a DN object
- update ipachangeconf.py code to handle strings better and instead
  of concatenating objects, make sure they are converted to string
  first

As a side-effect of ipachangeconf changes, default.conf config file
generated by ipa-client-install has no longer empty new line at the
end of a file.

Whole ipachangeconf.py has been modified to be compliant with PEP8.

https://fedorahosted.org/freeipa/ticket/3088

2 files changed, 118 insertions, 63 deletions

diff --git a/ipa-client/ipaclient/ipachangeconf.py b/ipa-client/ipaclient/ipachangeconf.py
index f6288062b..6cf47b807 100644
--- a/ipa-client/ipaclient/ipachangeconf.py
+++ b/ipa-client/ipaclient/ipachangeconf.py
@@ -24,11 +24,12 @@ import string
 import time
 import shutil
 
+
 def openLocked(filename, perms):
     fd = -1
     try:
         fd = os.open(filename, os.O_RDWR | os.O_CREAT, perms)
-        
+
         fcntl.lockf(fd, fcntl.LOCK_EX)
     except OSError, (errno, strerr):
         if fd != -1:
@@ -49,15 +50,15 @@ class IPAChangeConf:
 
     def __init__(self, name):
         self.progname = name
-        self.indent = ("","","")
-        self.assign = (" = ","=")
+        self.indent = ("", "", "")
+        self.assign = (" = ", "=")
         self.dassign = self.assign[0]
         self.comment = ("#",)
         self.dcomment = self.comment[0]
         self.eol = ("\n",)
         self.deol = self.eol[0]
-        self.sectnamdel = ("[","]")
-        self.subsectdel = ("{","}")
+        self.sectnamdel = ("[", "]")
+        self.subsectdel = ("{", "}")
 
     def setProgName(self, name):
         self.progname = name
@@ -68,7 +69,7 @@ class IPAChangeConf:
         elif type(indent) is str:
             self.indent = (indent, )
         else:
-           raise ValueError, 'Indent must be a list of strings'
+            raise ValueError('Indent must be a list of strings')
 
     def setOptionAssignment(self, assign):
         if type(assign) is tuple:
@@ -116,7 +117,7 @@ class IPAChangeConf:
             return False
         if not cl.endswith(self.sectnamdel[1]):
             return False
-        return cl[len(self.sectnamdel[0]):-len(self.sectnamdel[1])]            
+        return cl[len(self.sectnamdel[0]):-len(self.sectnamdel[1])]
 
     def matchSubSection(self, line):
         if self.matchComment(line):
@@ -143,50 +144,69 @@ class IPAChangeConf:
     def getSectionLine(self, section):
         if len(self.sectnamdel) != 2:
             return section
-        return self.sectnamdel[0]+section+self.sectnamdel[1]+self.deol
+        return self._dump_line(self.sectnamdel[0],
+                               section,
+                               self.sectnamdel[1],
+                               self.deol)
+
+    def _dump_line(self, *args):
+        return u"".join(unicode(x) for x in args)
 
     def dump(self, options, level=0):
-        output = ""
+        output = []
         if level >= len(self.indent):
-            level = len(self.indent)-1
+            level = len(self.indent) - 1
 
         for o in options:
             if o['type'] == "section":
-                output += self.sectnamdel[0]+o['name']+self.sectnamdel[1]+self.deol
-                output += self.dump(o['value'], level+1)
+                output.append(self._dump_line(self.sectnamdel[0],
+                                              o['name'],
+                                              self.sectnamdel[1]))
+                output.append(self.dump(o['value'], (level + 1)))
                 continue
             if o['type'] == "subsection":
-                output += self.indent[level]+o['name']+self.dassign+self.subsectdel[0]+self.deol
-                output += self.dump(o['value'], level+1)
-                output += self.indent[level]+self.subsectdel[1]+self.deol
+                output.append(self._dump_line(self.indent[level],
+                                              o['name'],
+                                              self.dassign,
+                                              self.subsectdel[0]))
+                output.append(self.dump(o['value'], (level + 1)))
+                output.append(self._dump_line(self.indent[level],
+                                              self.subsectdel[1]))
                 continue
             if o['type'] == "option":
-                output += self.indent[level]+o['name']+self.dassign+o['value']+self.deol
+                output.append(self._dump_line(self.indent[level],
+                                              o['name'],
+                                              self.dassign,
+                                              o['value']))
                 continue
             if o['type'] == "comment":
-                output += self.dcomment+o['value']+self.deol
+                output.append(self._dump_line(self.dcomment, o['value']))
                 continue
             if o['type'] == "empty":
-                output += self.deol
+                output.append('')
                 continue
-            raise SyntaxError, 'Unknown type: ['+o['type']+']'
+            raise SyntaxError('Unknown type: [%s]' % o['type'])
 
-        return output
+        return self.deol.join(output)
 
     def parseLine(self, line):
 
         if self.matchEmpty(line):
-            return {'name':'empty', 'type':'empty'}
+            return {'name': 'empty', 'type': 'empty'}
 
         value = self.matchComment(line)
         if value:
-            return {'name':'comment', 'type':'comment', 'value':value.rstrip()} #pylint: disable=E1103
+            return {'name': 'comment',
+                    'type': 'comment',
+                    'value': value.rstrip()}  # pylint: disable=E1103
 
         parts = line.split(self.dassign, 1)
         if len(parts) < 2:
-            raise SyntaxError, 'Syntax Error: Unknown line format'
+            raise SyntaxError('Syntax Error: Unknown line format')
 
-        return {'name':parts[0].strip(), 'type':'option', 'value':parts[1].rstrip()}
+        return {'name': parts[0].strip(),
+                'type': 'option',
+                'value': parts[1].rstrip()}
 
     def findOpts(self, opts, type, name, exclude_sections=False):
 
@@ -194,46 +214,65 @@ class IPAChangeConf:
         for o in opts:
             if o['type'] == type and o['name'] == name:
                 return (num, o)
-            if exclude_sections and (o['type'] == "section" or o['type'] == "subsection"):
+            if exclude_sections and (o['type'] == "section" or
+                                     o['type'] == "subsection"):
                 return (num, None)
             num += 1
         return (num, None)
 
-    def commentOpts(self, inopts, level = 0):
+    def commentOpts(self, inopts, level=0):
 
         opts = []
 
         if level >= len(self.indent):
-            level = len(self.indent)-1
+            level = len(self.indent) - 1
 
         for o in inopts:
             if o['type'] == 'section':
-                no = self.commentOpts(o['value'], level+1)
-                val = self.dcomment+self.sectnamdel[0]+o['name']+self.sectnamdel[1]
-                opts.append({'name':'comment', 'type':'comment', 'value':val})
+                no = self.commentOpts(o['value'], (level + 1))
+                val = self._dump_line(self.dcomment,
+                                      self.sectnamdel[0],
+                                      o['name'],
+                                      self.sectnamdel[1])
+                opts.append({'name': 'comment',
+                             'type': 'comment',
+                             'value': val})
                 for n in no:
                     opts.append(n)
                 continue
             if o['type'] == 'subsection':
-                no = self.commentOpts(o['value'], level+1)
-                val = self.indent[level]+o['name']+self.dassign+self.subsectdel[0]
-                opts.append({'name':'comment', 'type':'comment', 'value':val})
-                for n in no:
-                    opts.append(n)
-                val = self.indent[level]+self.subsectdel[1]
-                opts.append({'name':'comment', 'type':'comment', 'value':val})
+                no = self.commentOpts(o['value'], (level + 1))
+                val = self._dump_line(self.indent[level],
+                                      o['name'],
+                                      self.dassign,
+                                      self.subsectdel[0])
+                opts.append({'name': 'comment',
+                             'type': 'comment',
+                             'value': val})
+                opts.extend(no)
+                val = self._dump_line(self.indent[level], self.subsectdel[1])
+                opts.append({'name': 'comment',
+                             'type': 'comment',
+                             'value': val})
                 continue
             if o['type'] == 'option':
-                val = self.indent[level]+o['name']+self.dassign+o['value']
-                opts.append({'name':'comment', 'type':'comment', 'value':val})
+                val = self._dump_line(self.indent[level],
+                                      o['name'],
+                                      self.dassign,
+                                      o['value'])
+                opts.append({'name': 'comment',
+                             'type': 'comment',
+                             'value': val})
                 continue
             if o['type'] == 'comment':
                 opts.append(o)
                 continue
             if o['type'] == 'empty':
-                opts.append({'name':'comment', 'type':'comment', 'value':''})
+                opts.append({'name': 'comment',
+                             'type': 'comment',
+                             'value': ''})
                 continue
-            raise SyntaxError, 'Unknown type: ['+o['type']+']'
+            raise SyntaxError('Unknown type: [%s]' % o['type'])
 
         return opts
 
@@ -249,7 +288,9 @@ class IPAChangeConf:
                     continue
                 if no['action'] == "set":
                     mo = self.mergeOld(o['value'], no['value'])
-                    opts.append({'name':o['name'], 'type':o['type'], 'value':mo})
+                    opts.append({'name': o['name'],
+                                 'type': o['type'],
+                                 'value': mo})
                     continue
                 if no['action'] == "comment":
                     co = self.commentOpts(o['value'])
@@ -258,11 +299,11 @@ class IPAChangeConf:
                     continue
                 if no['action'] == "remove":
                     continue
-                raise SyntaxError, 'Unknown action: ['+no['action']+']'
+                raise SyntaxError('Unknown action: [%s]' % no['action'])
 
             if o['type'] == "comment" or o['type'] == "empty":
-                 opts.append(o)
-                 continue
+                opts.append(o)
+                continue
 
             if o['type'] == "option":
                 (num, no) = self.findOpts(newopts, 'option', o['name'], True)
@@ -270,19 +311,25 @@ class IPAChangeConf:
                     opts.append(o)
                     continue
                 if no['action'] == 'comment' or no['action'] == 'remove':
-                    if no['value'] != None and o['value'] != no['value']:
+                    if (no['value'] is not None and
+                            o['value'] is not no['value']):
                         opts.append(o)
                         continue
                     if no['action'] == 'comment':
-                       opts.append({'name':'comment', 'type':'comment',
-                                    'value':self.dcomment+o['name']+self.dassign+o['value']})
+                        value = self._dump_line(self.dcomment,
+                                                o['name'],
+                                                self.dassign,
+                                                o['value'])
+                        opts.append({'name': 'comment',
+                                     'type': 'comment',
+                                     'value': value})
                     continue
                 if no['action'] == 'set':
                     opts.append(no)
                     continue
-                raise SyntaxError, 'Unknown action: ['+o['action']+']'
+                raise SyntaxError('Unknown action: [%s]' % o['action'])
 
-            raise SyntaxError, 'Unknown type: ['+o['type']+']'
+            raise SyntaxError('Unknown type: [%s]' % o['type'])
 
         return opts
 
@@ -301,7 +348,7 @@ class IPAChangeConf:
                 if no['action'] == "set":
                     self.mergeNew(o['value'], no['value'])
                     continue
-                cline = num+1
+                cline = num + 1
                 continue
 
             if no['type'] == "option":
@@ -310,7 +357,7 @@ class IPAChangeConf:
                     if no['action'] == 'set':
                         opts.append(no)
                     continue
-                cline = num+1
+                cline = num + 1
                 continue
 
             if no['type'] == "comment" or no['type'] == "empty":
@@ -318,8 +365,7 @@ class IPAChangeConf:
                 cline += 1
                 continue
 
-            raise SyntaxError, 'Unknown type: ['+no['type']+']'
-
+            raise SyntaxError('Unknown type: [%s]' % no['type'])
 
     def merge(self, oldopts, newopts):
 
@@ -352,7 +398,9 @@ class IPAChangeConf:
             value = self.matchSection(line)
             if value:
                 if section is not None:
-                    opts.append({'name':section, 'type':'section', 'value':sectopts})
+                    opts.append({'name': section,
+                                 'type': 'section',
+                                 'value': sectopts})
                 sectopts = []
                 curopts = sectopts
                 fatheropts = sectopts
@@ -363,7 +411,8 @@ class IPAChangeConf:
             value = self.matchSubSection(line)
             if value:
                 if subsection is not None:
-                    raise SyntaxError, 'nested subsections are not supported yet'
+                    raise SyntaxError('nested subsections are not '
+                                      'supported yet')
                 subsectopts = []
                 curopts = subsectopts
                 subsection = value
@@ -372,8 +421,11 @@ class IPAChangeConf:
             value = self.matchSubSectionEnd(line)
             if value:
                 if subsection is None:
-                    raise SyntaxError, 'Unmatched end subsection terminator found'
-                fatheropts.append({'name':subsection, 'type':'subsection', 'value':subsectopts})
+                    raise SyntaxError('Unmatched end subsection terminator '
+                                      'found')
+                fatheropts.append({'name': subsection,
+                                   'type': 'subsection',
+                                   'value': subsectopts})
                 subsection = None
                 curopts = fatheropts
                 continue
@@ -383,7 +435,9 @@ class IPAChangeConf:
 
         #Add last section if any
         if len(sectopts) is not 0:
-            opts.append({'name':section, 'type':'section', 'value':sectopts})
+            opts.append({'name': section,
+                         'type': 'section',
+                         'value': sectopts})
 
         return opts
 
@@ -399,8 +453,9 @@ class IPAChangeConf:
         output = ""
         f = None
         try:
-            #Do not catch an unexisting file error, we want to fail in that case
-            shutil.copy2(file, file+".ipabkp")
+            # Do not catch an unexisting file error
+            # we want to fail in that case
+            shutil.copy2(file, (file + ".ipabkp"))
 
             f = openLocked(file, 0644)
 
@@ -435,7 +490,7 @@ class IPAChangeConf:
         f = None
         try:
             try:
-                shutil.copy2(file, file+".ipabkp")
+                shutil.copy2(file, (file + ".ipabkp"))
             except IOError, err:
                 if err.errno == 2:
                     # The orign file did not exist
diff --git a/ipa-client/ipaclient/ipadiscovery.py b/ipa-client/ipaclient/ipadiscovery.py
index 234e67e9b..f91d4075a 100644
--- a/ipa-client/ipaclient/ipadiscovery.py
+++ b/ipa-client/ipaclient/ipadiscovery.py
@@ -249,7 +249,7 @@ class IPADiscovery(object):
 
         if not ldapaccess and self.basedn is None:
             # Generate suffix from realm
-            self.basedn = str(realm_to_suffix(self.realm))
+            self.basedn = realm_to_suffix(self.realm)
             self.basedn_source = 'Generated from Kerberos realm'
             root_logger.debug("Generated basedn from realm: %s" % self.basedn)