Preserve already configured options in openldap conf

We should respect already configured options present in
/etc/openldap/ldap.conf when generating our own configuration.

With this patch, we only rewrite URI, BASE and TLS_CACERT options
only if they are not configured. In the case they are, our suggested
configuration is inserted as a comment.

Also adds tab as a delimeter character in /etc/openldap/ldap.conf

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

2 files changed, 65 insertions, 11 deletions

diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index a60124bb7..8cd253eed 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -817,19 +817,61 @@ def configure_nslcd_conf(fstore, cli_basedn, cli_realm, cli_domain, cli_server,
 
 def configure_openldap_conf(fstore, cli_basedn, cli_server):
     ldapconf = ipaclient.ipachangeconf.IPAChangeConf("IPA Installer")
-    ldapconf.setOptionAssignment(" ")
+    ldapconf.setOptionAssignment((" ", "\t"))
 
-    opts = [{'name':'comment', 'type':'comment', 'value':'File modified by ipa-client-install'},
+    opts = [{'name':'comment', 'type':'comment',
+                'value':' File modified by ipa-client-install'},
             {'name':'empty', 'type':'empty'},
-            {'name':'URI', 'type':'option', 'value':'ldaps://'+  cli_server[0]},
-            {'name':'BASE', 'type':'option', 'value':cli_basedn},
-            {'name':'TLS_CACERT', 'type':'option', 'value':CACERT},
-            {'name':'empty', 'type':'empty'}]
+            {'name':'comment', 'type':'comment',
+                'value':' We do not want to break your existing configuration, '
+                        'hence:'},
+            # this needs to be kept updated if we change more options
+            {'name':'comment', 'type':'comment',
+                'value':'   URI, BASE and TLS_CACERT have been added if they '
+                        'were not set.'},
+            {'name':'comment', 'type':'comment',
+                'value':'   In case any of them were set, a comment with '
+                         'trailing note'},
+            {'name':'comment', 'type':'comment',
+                'value':'   "# modified by IPA" note has been inserted.'},
+            {'name':'comment', 'type':'comment',
+                'value':' To use IPA server with openLDAP tools, please comment '
+                        'out your'},
+            {'name':'comment', 'type':'comment',
+                'value':' existing configuration for these options and '
+                        'uncomment the'},
+            {'name':'comment', 'type':'comment',
+                'value':' corresponding lines generated by IPA.'},
+            {'name':'empty', 'type':'empty'},
+            {'name':'empty', 'type':'empty'},
+            {'action':'addifnotset', 'name':'URI', 'type':'option',
+                'value':'ldaps://'+  cli_server[0]},
+            {'action':'addifnotset', 'name':'BASE', 'type':'option',
+                'value':str(cli_basedn)},
+            {'action':'addifnotset', 'name':'TLS_CACERT', 'type':'option',
+                'value':CACERT},]
 
     target_fname = '/etc/openldap/ldap.conf'
     fstore.backup_file(target_fname)
-    ldapconf.newConf(target_fname, opts)
+
+    error_msg = "Configuring {path} failed with: {err}"
+
+    try:
+        ldapconf.changeConf(target_fname, opts)
+    except SyntaxError, e:
+        root_logger.info("Could not parse {path}".format(path=target_fname))
+        root_logger.debug(error_msg.format(path=target_fname, err=str(e)))
+        return False
+    except IOError,e :
+        root_logger.info("{path} does not exist.".format(path=target_fname))
+        root_logger.debug(error_msg.format(path=target_fname, err=str(e)))
+        return False
+    except Exception, e: #  we do not want to fail in an optional step
+        root_logger.debug(error_msg.format(path=target_fname, err=str(e)))
+        return False
+
     os.chmod(target_fname, 0644)
+    return True
 
 def hardcode_ldap_server(cli_server):
     """
@@ -2379,8 +2421,10 @@ def install(options, env, fstore, statestore):
                         "%s configured using configuration file(s) %s",
                         conf, filenames)
 
-        configure_openldap_conf(fstore, cli_basedn, cli_server)
-        root_logger.info("Configured /etc/openldap/ldap.conf")
+        if configure_openldap_conf(fstore, cli_basedn, cli_server):
+            root_logger.info("Configured /etc/openldap/ldap.conf")
+        else:
+            root_logger.info("Failed to configure /etc/openldap/ldap.conf")
 
         #Check that nss is working properly
         if not options.on_master:
diff --git a/ipa-client/ipaclient/ipachangeconf.py b/ipa-client/ipaclient/ipachangeconf.py
index bdc5579fc..e802e177e 100644
--- a/ipa-client/ipaclient/ipachangeconf.py
+++ b/ipa-client/ipaclient/ipachangeconf.py
@@ -338,7 +338,16 @@ class IPAChangeConf:
                 if no['action'] == 'set':
                     opts.append(no)
                     continue
-                raise SyntaxError('Unknown action: [%s]' % o['action'])
+                if no['action'] == 'addifnotset':
+                    opts.append({'name': 'comment', 'type': 'comment',
+                                'value': self._dump_line(no['name'],
+                                                         self.dassign,
+                                                         no['value'],
+                                                         u' # modified by IPA'
+                                                         )})
+                    opts.append(o)
+                    continue
+                raise SyntaxError('Unknown action: [%s]' % no['action'])
 
             raise SyntaxError('Unknown type: [%s]' % o['type'])
 
@@ -365,7 +374,7 @@ class IPAChangeConf:
             if no['type'] == "option":
                 (num, o) = self.findOpts(opts, no['type'], no['name'], True)
                 if not o:
-                    if no['action'] == 'set':
+                    if no['action'] == 'set' or no['action'] == 'addifnotset':
                         opts.append(no)
                     continue
                 cline = num + 1
@@ -385,6 +394,7 @@ class IPAChangeConf:
         #  the options as indicated by the contents of newopts
         #Second we fill in the new opts tree with options as indicated
         #  in the newopts tree (this is becaus eentire (sub)sections may
+        #  in the newopts tree (this is becaus entire (sub)sections may
         #  exist in the newopts that do not exist in oldopts)
 
         opts = self.mergeOld(oldopts, newopts)