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

1 files changed, 53 insertions, 9 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: