On Wed, 2011-06-22 at 18:03 -0400, Rob Crittenden wrote: > Martin Kosek wrote: > > Install tools may fail with unexpected error when IPA server is not > > installed on a system. Improve user experience by implementing > > a check to affected tools. > > > > https://fedorahosted.org/freeipa/ticket/1327 > > https://fedorahosted.org/freeipa/ticket/1347 > > Can you add a docstring to the check_server_configuration() function? > > Looking in each utility it isn't necessarily obvious what this does but > my meager attempts at renaming it all failed. I considered > is_server_installed() but that implies it would return True/False. Then > I considered require_server_configured() but that didn't seem to fit > either. We have lots of other check_* so I guess it is fine, but some > docs on where/why it is used would be nice. > > rob
I see you undertake the same function naming dilemma as I do. I improved documentation for the function, it should help. Martin
>From c5ede07c01d57c3cb33f80cc0b2057d2298b0e66 Mon Sep 17 00:00:00 2001 From: Martin Kosek <mko...@redhat.com> Date: Wed, 22 Jun 2011 13:10:15 +0200 Subject: [PATCH] Check IPA configuration in install tools Install tools may fail with unexpected error when IPA server is not installed on a system. Improve user experience by implementing a check to affected tools. https://fedorahosted.org/freeipa/ticket/1327 https://fedorahosted.org/freeipa/ticket/1347 --- install/tools/ipa-compliance | 14 +++++++++++--- install/tools/ipa-dns-install | 3 +++ install/tools/ipa-ldap-updater | 7 ++----- install/tools/ipa-nis-manage | 2 ++ install/tools/ipa-replica-manage | 7 +++++++ install/tools/ipa-replica-prepare | 4 ++++ install/tools/ipa-server-certinstall | 13 ++++++++++++- ipaserver/install/installutils.py | 20 ++++++++++++++++++-- 8 files changed, 59 insertions(+), 11 deletions(-) diff --git a/install/tools/ipa-compliance b/install/tools/ipa-compliance index e1de25283ba00761f3fbe363f2323fa2caa2076a..47a515bfc7b0244e3b1c2b50b5b0bb1ccc6013d0 100644 --- a/install/tools/ipa-compliance +++ b/install/tools/ipa-compliance @@ -35,6 +35,7 @@ try: from ipaserver.plugins.ldap2 import ldap2 from ipalib import api, errors, backend + from ipaserver.install import installutils except ImportError, e: # If python-rhsm isn't installed exit gracefully and quietly. if e.args[0] == 'No module named rhsm.certificate': @@ -165,8 +166,7 @@ def check_compliance(tmpdir, debug=False): print 'IPA is in compliance: %d of %d entitlements used.' % (hostcount, available) def main(): - if os.getegid() != 0: - sys.exit("Must be root to check compliance") + installutils.check_server_configuration() if not os.path.exists('/etc/ipa/default.conf'): return 0 @@ -189,4 +189,12 @@ def main(): return 0 -sys.exit(main()) +try: + if not os.geteuid()==0: + sys.exit("\nMust be root to check compliance\n") + + main() +except SystemExit, e: + sys.exit(e) +except RuntimeError, e: + sys.exit(e) diff --git a/install/tools/ipa-dns-install b/install/tools/ipa-dns-install index b5295b5c7567c3d559ad808d1752d79c1d915573..16cc33b38fbe842fdf2b5a82ca42601d6a9ab0ac 100755 --- a/install/tools/ipa-dns-install +++ b/install/tools/ipa-dns-install @@ -24,6 +24,7 @@ import traceback from ipaserver.plugins.ldap2 import ldap2 from ipaserver.install import bindinstance, ntpinstance from ipaserver.install.installutils import * +from ipaserver.install import installutils from ipapython import version from ipapython import ipautil, sysrestore from ipalib import api, errors, util @@ -69,6 +70,8 @@ def main(): if os.getegid() != 0: sys.exit("Must be root to setup server") + installutils.check_server_configuration() + standard_logging_setup("/var/log/ipaserver-install.log", options.debug, filemode='a') print "\nThe log file for this installation can be found in /var/log/ipaserver-install.log" diff --git a/install/tools/ipa-ldap-updater b/install/tools/ipa-ldap-updater index ec57109d3c03fae11c8028edd8d20a88e40cc02a..5b63c120ec83a03fa0cc7ba9aab1cb60bda23e31 100755 --- a/install/tools/ipa-ldap-updater +++ b/install/tools/ipa-ldap-updater @@ -85,9 +85,7 @@ def main(): loglevel = logging.DEBUG if os.getegid() == 0: - fstore = sysrestore.FileStore('/var/lib/ipa/sysrestore') - if not fstore.has_files(): - sys.exit("IPA is not configured on this system.") + installutils.check_server_configuration() elif not os.path.exists('/etc/ipa/default.conf'): sys.exit("IPA is not configured on this system.") @@ -149,8 +147,7 @@ except BadSyntax, e: print " %s" % e sys.exit(1) except RuntimeError, e: - print "%s" % e - sys.exit(1) + sys.exit(e) except SystemExit, e: sys.exit(e) except KeyboardInterrupt, e: diff --git a/install/tools/ipa-nis-manage b/install/tools/ipa-nis-manage index 2c0936b49d630f18cbabc85e65e9ef3760172db7..52edb1442b0dc8236f12fc768b4890b52c12a220 100755 --- a/install/tools/ipa-nis-manage +++ b/install/tools/ipa-nis-manage @@ -87,6 +87,8 @@ def main(): if os.getegid() != 0: sys.exit('Must be root to use this tool.') + installutils.check_server_configuration() + options, args = parse_options() if options.debug: loglevel = logging.DEBUG diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage index 1adf0cebbcf0e59a199822eddffcc8201262bd12..c3dd0b3fe9e2efcc32e20286ef42d798bf567e08 100755 --- a/install/tools/ipa-replica-manage +++ b/install/tools/ipa-replica-manage @@ -412,6 +412,11 @@ def force_sync(realm, thishost, fromhost, dirman_passwd): repl.force_sync(repl.conn, thishost) def main(): + if os.getegid() == 0: + installutils.check_server_configuration() + elif not os.path.exists('/etc/ipa/default.conf'): + sys.exit("IPA is not configured on this system.") + options, args = parse_options() # Just initialize the environment. This is so the installer can have @@ -480,6 +485,8 @@ except KeyboardInterrupt: sys.exit(1) except SystemExit, e: sys.exit(e) +except RuntimeError, e: + sys.exit(e) except ldap.INVALID_CREDENTIALS: print "Invalid password" sys.exit(1) diff --git a/install/tools/ipa-replica-prepare b/install/tools/ipa-replica-prepare index 97dd96a19befe3740bfde5fa8e1ab062c875c583..9518825192b0593c9fd08d90a179d537ca97c403 100755 --- a/install/tools/ipa-replica-prepare +++ b/install/tools/ipa-replica-prepare @@ -62,6 +62,7 @@ def parse_options(): default=True, help="disables pkinit setup steps") options, args = parser.parse_args() + config.init_config() # If any of the PKCS#12 options are selected, all are required. Create a # list of the options and count it to enforce that all are required without @@ -219,6 +220,7 @@ def get_dirman_password(): return installutils.read_password("Directory Manager (existing master)", confirm=False, validate=False) def main(): + installutils.check_server_configuration() if not check_replication_plugin(): sys.exit(1) options, args = parse_options() @@ -457,6 +459,8 @@ try: main() except SystemExit, e: sys.exit(e) +except RuntimeError, e: + sys.exit(e) except Exception, e: print "preparation of replica failed: %s" % str(e) message = str(e) diff --git a/install/tools/ipa-server-certinstall b/install/tools/ipa-server-certinstall index 74ded157cde95902332cd28cbf2e8ed610aab82c..312d412024fb23a91eacd25f75a2e89bc25f5b1c 100755 --- a/install/tools/ipa-server-certinstall +++ b/install/tools/ipa-server-certinstall @@ -32,6 +32,7 @@ from ipapython.ipautil import user_input from ipaserver.install import certs, dsinstance, httpinstance, installutils from ipalib import api from ipaserver.plugins.ldap2 import ldap2 +from ipaserver.install import installutils def get_realm_name(): c = krbV.default_context() @@ -120,6 +121,8 @@ def import_cert(dirname, pkcs12_fname, pkcs12_passwd, db_password): return server_cert def main(): + installutils.check_server_configuration() + options, pkcs12_fname = parse_options() cfg = dict(in_server=True,) @@ -160,4 +163,12 @@ def main(): return 0 -sys.exit(main()) +try: + if not os.geteuid()==0: + sys.exit("\nYou must be root to run this script.\n") + + main() +except SystemExit, e: + sys.exit(e) +except RuntimeError, e: + sys.exit(e) diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py index f5a862599dd6b41e28ef8b4f04c2ea09d0102ec4..d6638b2a34856cf0a42220b3100793265fd91c46 100644 --- a/ipaserver/install/installutils.py +++ b/ipaserver/install/installutils.py @@ -30,8 +30,7 @@ import fcntl import netaddr import time -from ipapython import ipautil -from ipapython import dnsclient +from ipapython import ipautil, dnsclient, sysrestore class HostnameLocalhost(Exception): pass @@ -442,3 +441,20 @@ def resolve_host(host_name): return addrinfos[0][4][0] except: return None + +def check_server_configuration(): + """ + Check if IPA server is configured on the system. + + This is done by checking if there are system restore (uninstall) files + present on the system. Note that this check can only be run with root + privileges. + + When IPA is not configured, this function raises a RuntimeError exception. + Most convenient use case for the function is in install tools that require + configured IPA for its function. + """ + server_fstore = sysrestore.FileStore('/var/lib/ipa/sysrestore') + if not server_fstore.has_files(): + raise RuntimeError("IPA is not configured on this system.") + -- 1.7.5.4
_______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel