Martin Kosek wrote:
On Thu, 2011-06-09 at 16:32 -0400, Rob Crittenden wrote:
Rob Crittenden wrote:
There was no point in limiting autobind root to just search cn=config
since it could always just modify its way out of the box, so remove the
restriction.

The upgrade log wasn't being created. Clearing all other loggers before
we calling logging.basicConfig() fixes this.

Add a global exception when performing updates so we can gracefully
catch and log problems without leaving the server in a bad state.

https://fedorahosted.org/freeipa/ticket/1243
https://fedorahosted.org/freeipa/ticket/1254

rob

This was leaving a bogus entry in systrestore.index and an empty value
in dse.ldif. I updated the patch.

rob

Autobind portion works fine. However, upgrade failure processing can be
improved:

1) When Exception is catched in IPAUpgrade, it is neither logged nor
printed out. This can make it difficult to debug.

Yup, logging it now.


2) User running `ipa-ldap-updater --upgrade` cannot tell if the upgrade
was wrong. Success status code is returned by the program and no info
that something has failed is given.

Gah, I had a return 1 there at some point...Added back.

rob
>From 6a90910b3cdc90feffe841fbbd7082aa985f6fe3 Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcrit...@redhat.com>
Date: Thu, 9 Jun 2011 13:16:07 -0400
Subject: [PATCH] Remove root autobind search restriction, fix upgrade logging & error handling.

There was no point in limiting autobind root to just search cn=config since
it could always just modify its way out of the box, so remove the
restriction.

The upgrade log wasn't being created. Clearing all other loggers before
we calling logging.basicConfig() fixes this.

Add a global exception when performing updates so we can gracefully catch
and log problems without leaving the server in a bad state.

https://fedorahosted.org/freeipa/ticket/1243
https://fedorahosted.org/freeipa/ticket/1254
---
 install/share/root-autobind.ldif     |    5 -----
 install/tools/ipa-ldap-updater       |   21 ++++++++++++++-------
 ipaserver/install/installutils.py    |   22 +++++++++++++---------
 ipaserver/install/upgradeinstance.py |   24 ++++++++----------------
 4 files changed, 35 insertions(+), 37 deletions(-)

diff --git a/install/share/root-autobind.ldif b/install/share/root-autobind.ldif
index e7bbc8d..ecce115 100644
--- a/install/share/root-autobind.ldif
+++ b/install/share/root-autobind.ldif
@@ -17,8 +17,3 @@ changetype: modify
 replace: nsslapd-ldapimaptoentries
 nsslapd-ldapimaptoentries: on
 
-dn: cn=config
-changetype: modify
-replace: nsslapd-ldapientrysearchbase
-nsslapd-ldapientrysearchbase: cn=config
-
diff --git a/install/tools/ipa-ldap-updater b/install/tools/ipa-ldap-updater
index ddf222e..ec57109 100755
--- a/install/tools/ipa-ldap-updater
+++ b/install/tools/ipa-ldap-updater
@@ -78,6 +78,7 @@ def get_dirman_password():
 def main():
     loglevel = logging.INFO
     badsyntax = False
+    upgradefailed = False
 
     safe_options, options, args = parse_options()
     if options.debug:
@@ -102,24 +103,26 @@ def main():
     if len(args) > 0:
         files = args
 
+    # Clear all existing log handler
+    loggers = logging.getLogger()
+    if loggers.handlers:
+        for handler in loggers.handlers:
+            loggers.removeHandler(handler)
     if options.upgrade:
         if os.getegid() != 0:
             sys.exit('Upgrade can only be done as root')
         logging.basicConfig(level=loglevel,
-                            format='%(levelname)s %(message)s',
-                            filename='/var/log/ipaupgrade.log')
+                            format='%(asctime)s %(levelname)s %(message)s',
+                            filename='/var/log/ipaupgrade.log',
+                            filemode='a')
         logging.debug('%s was invoked with arguments %s and options: %s' % (sys.argv[0], args, safe_options))
         realm = krbV.default_context().default_realm
         upgrade = IPAUpgrade(realm, files, live_run=not options.test)
         upgrade.create_instance()
         modified = upgrade.modified
         badsyntax = upgrade.badsyntax
+        upgradefailed = upgrade.upgradefailed
     else:
-        # Clear all existing log handlers, this is need to log as root
-        loggers = logging.getLogger()
-        if loggers.handlers:
-            for handler in loggers.handlers:
-                loggers.removeHandler(handler)
         logging.basicConfig(level=loglevel,
                             format='%(levelname)s %(message)s')
         ld = LDAPUpdate(dm_password=dirman_password, sub_dict={}, live_run=not options.test, ldapi=options.ldapi)
@@ -128,6 +131,10 @@ def main():
         modified = ld.update(files)
 
     if badsyntax:
+        print 'Bad syntax detected in upgrade file(s).'
+        return 1
+    elif upgradefailed:
+        print 'IPA upgrade failed.'
         return 1
     elif modified and options.test:
         return 2
diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py
index d99af37..cb8c513 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -329,6 +329,8 @@ def update_file(filename, orig, subst):
 def set_directive(filename, directive, value, quotes=True, separator=' '):
     """Set a name/value pair directive in a configuration file.
 
+       A value of None means to drop the directive.
+
        This has only been tested with nss.conf
     """
     valueset = False
@@ -338,18 +340,20 @@ def set_directive(filename, directive, value, quotes=True, separator=' '):
     for line in fd:
         if directive in line:
             valueset = True
-            if quotes:
-                newfile.append('%s%s"%s"\n' % (directive, separator, value))
-            else:
-                newfile.append('%s%s%s\n' % (directive, separator, value))
+            if value is not None:
+                if quotes:
+                    newfile.append('%s%s"%s"\n' % (directive, separator, value))
+                else:
+                    newfile.append('%s%s%s\n' % (directive, separator, value))
         else:
             newfile.append(line)
     fd.close()
     if not valueset:
-        if quotes:
-            newfile.append('%s%s"%s"\n' % (directive, separator, value))
-        else:
-            newfile.append('%s%s%s\n' % (directive, separator, value))
+        if value is not None:
+            if quotes:
+                newfile.append('%s%s"%s"\n' % (directive, separator, value))
+            else:
+                newfile.append('%s%s%s\n' % (directive, separator, value))
 
     fd = open(filename, "w")
     fd.write("".join(newfile))
@@ -400,7 +404,7 @@ def wait_for_open_ports(host, ports, timeout=0):
 
     op_timeout = time.time() + timeout
     ipv6_failover = False
-    
+
     for port in ports:
         while True:
             try:
diff --git a/ipaserver/install/upgradeinstance.py b/ipaserver/install/upgradeinstance.py
index ad977b7..2f42358 100644
--- a/ipaserver/install/upgradeinstance.py
+++ b/ipaserver/install/upgradeinstance.py
@@ -21,6 +21,7 @@ import os
 import sys
 import shutil
 import random
+import logging
 
 from ipaserver.install import installutils
 from ipaserver.install import dsinstance
@@ -56,6 +57,7 @@ class IPAUpgrade(service.Service):
         self.files = files
         self.modified = False
         self.badsyntax = False
+        self.upgradefailed = False
 
     def create_instance(self):
         self.step("stopping directory server", self.stop)
@@ -75,41 +77,26 @@ class IPAUpgrade(service.Service):
                separator=':')
         security = installutils.get_directive(self.filename, 'nsslapd-security',
                    separator=':')
-        autobind = installutils.get_directive(self.filename,
-                   'nsslapd-ldapiautobind', separator=':')
-        searchbase = installutils.get_directive(self.filename,
-                   'nsslapd-ldapientrysearchbase', separator=':')
 
         self.backup_state('nsslapd-port', port)
         self.backup_state('nsslapd-security', security)
-        self.backup_state('nsslapd-ldapiautobind', autobind)
-        self.backup_state('nsslapd-ldapientrysearchbase', searchbase)
 
     def __restore_config(self):
         port = self.restore_state('nsslapd-port')
         security = self.restore_state('nsslapd-security')
-        autobind = self.restore_state('nsslapd-ldapiautobind')
-        searchbase = self.restore_state('nsslapd-ldapientrysearchbase')
 
         installutils.set_directive(self.filename, 'nsslapd-port',
             port, quotes=False, separator=':')
         installutils.set_directive(self.filename, 'nsslapd-security',
             security, quotes=False, separator=':')
-        installutils.set_directive(self.filename, 'nsslapd-ldapiautobind',
-            autobind, quotes=False, separator=':')
-        installutils.set_directive(self.filename,
-            'nsslapd-ldapientrysearchbase',
-            searchbase, quotes=False, separator=':')
 
     def __disable_listeners(self):
         installutils.set_directive(self.filename, 'nsslapd-port',
             0, quotes=False, separator=':')
         installutils.set_directive(self.filename, 'nsslapd-security',
             'off', quotes=False, separator=':')
-        installutils.set_directive(self.filename, 'nsslapd-ldapiautobind',
-            'on', quotes=False, separator=':')
         installutils.set_directive(self.filename, 'nsslapd-ldapientrysearchbase',
-            '', quotes=False, separator=':')
+            None, quotes=False, separator=':')
 
     def __upgrade(self):
         try:
@@ -120,6 +107,11 @@ class IPAUpgrade(service.Service):
         except ldapupdate.BadSyntax:
             self.modified = False
             self.badsyntax = True
+        except Exception, e:
+            # Bad things happened, return gracefully
+            self.modified = False
+            self.upgradefailed = True
+            logging.error('Upgrade failed with %s' % str(e))
 
 def main():
     if os.getegid() != 0:
-- 
1.7.4

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to