From 7940270b9fbebfa09b25c18198933b6a6b82b1d3 Mon Sep 17 00:00:00 2001 From: Rob Crittenden 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 e7bbc8dbe..ecce11511 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 ddf222e08..ec57109d3 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 d203f4f93..8d93a60b3 100644 --- a/ipaserver/install/installutils.py +++ b/ipaserver/install/installutils.py @@ -330,6 +330,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 @@ -339,18 +341,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)) @@ -401,7 +405,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 ad977b745..2f42358b7 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: