Implemented more elegant way for entire plugin module to be conditionally skipped; updated cert.py and ra.py modules to use this

7 files changed, 140 insertions, 82 deletions

diff --git a/ipalib/__init__.py b/ipalib/__init__.py
index 870c9c18..76310cad 100644
--- a/ipalib/__init__.py
+++ b/ipalib/__init__.py
@@ -878,6 +878,7 @@ from frontend import Object, Method, Property
 from crud import Create, Retrieve, Update, Delete, Search
 from parameters import DefaultFrom, Bool, Flag, Int, Float, Bytes, Str, Password
 from parameters import BytesEnum, StrEnum
+from errors2 import SkipPluginModule
 
 try:
     import uuid
diff --git a/ipalib/constants.py b/ipalib/constants.py
index ab35eb4b..2bf3adae 100644
--- a/ipalib/constants.py
+++ b/ipalib/constants.py
@@ -64,7 +64,7 @@ FORMAT_FILE = '\t'.join([
     '%(process)d',
     '%(threadName)s',
     '%(levelname)s',
-    '%(message)r', # Using %r for repr() so message is a single line
+    '%(message)s',
 ])
 
 
diff --git a/ipalib/errors2.py b/ipalib/errors2.py
index ebb1fdaf..6d26ee57 100644
--- a/ipalib/errors2.py
+++ b/ipalib/errors2.py
@@ -208,6 +208,21 @@ class PluginMissingOverrideError(PrivateError):
     format = '%(base)s.%(name)s not registered, cannot override with %(plugin)r'
 
 
+class SkipPluginModule(PrivateError):
+    """
+    Raised to abort the loading of a plugin module.
+    """
+
+    format = '%(reason)s'
+
+
+class PluginsPackageError(PrivateError):
+    """
+    Raised when ``package.plugins`` is a module instead of a sub-package.
+    """
+
+    format = '%(name)s must be sub-package, not module: %(file)r'
+
 
 ##############################################################################
 # Public errors:
diff --git a/ipalib/plugable.py b/ipalib/plugable.py
index 2c862a95..213b5978 100644
--- a/ipalib/plugable.py
+++ b/ipalib/plugable.py
@@ -646,9 +646,43 @@ class API(DictProxy):
         self.__do_if_not_done('bootstrap')
         if self.env.mode in ('dummy', 'unit_test'):
             return
-        util.import_plugins_subpackage('ipalib')
+        self.import_plugins('ipalib')
         if self.env.in_server:
-            util.import_plugins_subpackage('ipaserver')
+            self.import_plugins('ipaserver')
+
+    # FIXME: This method has no unit test
+    def import_plugins(self, package):
+        """
+        Import modules in ``plugins`` sub-package of ``package``.
+        """
+        subpackage = '%s.plugins' % package
+        try:
+            parent = __import__(package)
+            plugins = __import__(subpackage).plugins
+        except ImportError, e:
+            self.log.error(
+                'cannot import plugins sub-package %s: %s', subpackage, e
+            )
+            raise e
+        parent_dir = path.dirname(path.abspath(parent.__file__))
+        plugins_dir = path.dirname(path.abspath(plugins.__file__))
+        if parent_dir == plugins_dir:
+            raise errors2.PluginsPackageError(
+                name=subpackage, file=plugins.__file__
+            )
+        self.log.debug('importing all plugin modules in %r...', plugins_dir)
+        for (name, pyfile) in util.find_modules_in_dir(plugins_dir):
+            fullname = '%s.%s' % (subpackage, name)
+            self.log.debug('importing plugin module %r', pyfile)
+            try:
+                __import__(fullname)
+            except errors2.SkipPluginModule, e:
+                self.log.info(
+                    'skipping plugin module %s: %s', fullname, e.reason
+                )
+            except StandardError, e:
+                self.log.error('could not load plugin module %r', pyfile)
+                raise e
 
     def finalize(self):
         """
diff --git a/ipalib/plugins/cert.py b/ipalib/plugins/cert.py
index 22ec4c6a..96e2667b 100644
--- a/ipalib/plugins/cert.py
+++ b/ipalib/plugins/cert.py
@@ -22,110 +22,114 @@
 Command plugins for IPA-RA certificate operations.
 """
 
-from ipalib import api
+from ipalib import api, SkipPluginModule
 
-if api.env.enable_ra:
-    from ipalib import Command, Str, Int
+if api.env.enable_ra is not True:
+    raise SkipPluginModule(reason='env.enable_ra=%r' % (api.env.enable_ra,))
 
-    class cert_request(Command):
-        """
-        Submit a certificate singing request.
-        """
+from ipalib import Command, Str, Int
 
-        takes_args = ('csr',)
+assert False
 
-        takes_options = (
-            Str('request_type', default=u'pkcs10', autofill=True),
-        )
+class cert_request(Command):
+    """
+    Submit a certificate singing request.
+    """
 
-        def execute(self, csr, **options):
-            return self.Backend.ra.request_certificate(csr, **options)
+    takes_args = ('csr',)
 
-        def output_for_cli(self, textui, result, *args, **options):
-            if isinstance(result, dict) and len(result) > 0:
-                textui.print_entry(result, 0)
-            else:
-                textui.print_plain('Failed to submit a certificate request.')
+    takes_options = (
+        Str('request_type', default=u'pkcs10', autofill=True),
+    )
 
-    api.register(cert_request)
+    def execute(self, csr, **options):
+        return self.Backend.ra.request_certificate(csr, **options)
 
+    def output_for_cli(self, textui, result, *args, **options):
+        if isinstance(result, dict) and len(result) > 0:
+            textui.print_entry(result, 0)
+        else:
+            textui.print_plain('Failed to submit a certificate request.')
 
-    class cert_status(Command):
-        """
-        Check status of a certificate signing request.
-        """
+api.register(cert_request)
 
-        takes_args = ['request_id']
 
+class cert_status(Command):
+    """
+    Check status of a certificate signing request.
+    """
 
-        def execute(self, request_id, **options):
-            return self.Backend.ra.check_request_status(request_id)
+    takes_args = ['request_id']
 
-        def output_for_cli(self, textui, result, *args, **options):
-            if isinstance(result, dict) and len(result) > 0:
-                textui.print_entry(result, 0)
-            else:
-                textui.print_plain('Failed to retrieve a request status.')
 
-    api.register(cert_status)
+    def execute(self, request_id, **options):
+        return self.Backend.ra.check_request_status(request_id)
 
+    def output_for_cli(self, textui, result, *args, **options):
+        if isinstance(result, dict) and len(result) > 0:
+            textui.print_entry(result, 0)
+        else:
+            textui.print_plain('Failed to retrieve a request status.')
 
-    class cert_get(Command):
-        """
-        Retrieve an existing certificate.
-        """
+api.register(cert_status)
 
-        takes_args = ['serial_number']
 
-        def execute(self, serial_number):
-            return self.Backend.ra.get_certificate(serial_number)
+class cert_get(Command):
+    """
+    Retrieve an existing certificate.
+    """
 
-        def output_for_cli(self, textui, result, *args, **options):
-            if isinstance(result, dict) and len(result) > 0:
-                textui.print_entry(result, 0)
-            else:
-                textui.print_plain('Failed to obtain a certificate.')
+    takes_args = ['serial_number']
 
-    api.register(cert_get)
+    def execute(self, serial_number):
+        return self.Backend.ra.get_certificate(serial_number)
 
+    def output_for_cli(self, textui, result, *args, **options):
+        if isinstance(result, dict) and len(result) > 0:
+            textui.print_entry(result, 0)
+        else:
+            textui.print_plain('Failed to obtain a certificate.')
 
-    class cert_revoke(Command):
-        """
-        Revoke a certificate.
-        """
+api.register(cert_get)
 
-        takes_args = ['serial_number']
 
-        # FIXME: The default is 0.  Is this really an Int param?
-        takes_options = [Int('revocation_reason?', default=0)]
+class cert_revoke(Command):
+    """
+    Revoke a certificate.
+    """
 
+    takes_args = ['serial_number']
 
-        def execute(self, serial_number, **options):
-            return self.Backend.ra.revoke_certificate(serial_number, **options)
+    # FIXME: The default is 0.  Is this really an Int param?
+    takes_options = [Int('revocation_reason?', default=0)]
 
-        def output_for_cli(self, textui, result, *args, **options):
-            if isinstance(result, dict) and len(result) > 0:
-                textui.print_entry(result, 0)
-            else:
-                textui.print_plain('Failed to revoke a certificate.')
 
-    api.register(cert_revoke)
+    def execute(self, serial_number, **options):
+        return self.Backend.ra.revoke_certificate(serial_number, **options)
 
+    def output_for_cli(self, textui, result, *args, **options):
+        if isinstance(result, dict) and len(result) > 0:
+            textui.print_entry(result, 0)
+        else:
+            textui.print_plain('Failed to revoke a certificate.')
 
-    class cert_remove_hold(Command):
-        """
-        Take a revoked certificate off hold.
-        """
+api.register(cert_revoke)
 
-        takes_args = ['serial_number']
 
-        def execute(self, serial_number, **options):
-            return self.Backend.ra.take_certificate_off_hold(serial_number)
+class cert_remove_hold(Command):
+    """
+    Take a revoked certificate off hold.
+    """
 
-        def output_for_cli(self, textui, result, *args, **options):
-            if isinstance(result, dict) and len(result) > 0:
-                textui.print_entry(result, 0)
-            else:
-                textui.print_plain('Failed to take a revoked certificate off hold.')
+    takes_args = ['serial_number']
 
-    api.register(cert_remove_hold)
+    def execute(self, serial_number, **options):
+        return self.Backend.ra.take_certificate_off_hold(serial_number)
+
+    def output_for_cli(self, textui, result, *args, **options):
+        if isinstance(result, dict) and len(result) > 0:
+            textui.print_entry(result, 0)
+        else:
+            textui.print_plain('Failed to take a revoked certificate off hold.')
+
+api.register(cert_remove_hold)
diff --git a/ipalib/util.py b/ipalib/util.py
index 13f75082..f1a7928a 100644
--- a/ipalib/util.py
+++ b/ipalib/util.py
@@ -54,13 +54,13 @@ def find_modules_in_dir(src_dir):
     for name in sorted(os.listdir(src_dir)):
         if not name.endswith(suffix):
             continue
-        py_file = path.join(src_dir, name)
-        if path.islink(py_file) or not path.isfile(py_file):
+        pyfile = path.join(src_dir, name)
+        if path.islink(pyfile) or not path.isfile(pyfile):
             continue
         module = name[:-len(suffix)]
         if module == '__init__':
             continue
-        yield module
+        yield (module, pyfile)
 
 
 # FIXME: This function has no unit test
diff --git a/ipaserver/plugins/ra.py b/ipaserver/plugins/ra.py
index 622e4d9f..471276c3 100644
--- a/ipaserver/plugins/ra.py
+++ b/ipaserver/plugins/ra.py
@@ -31,6 +31,11 @@ certificates via the following methods:
     * `ra.take_certificate_off_hold()` - take a certificate off hold.
 """
 
+from ipalib import api, SkipPluginModule
+
+if api.env.enable_ra is not True:
+    raise SkipPluginModule(reason='env.enable_ra=%r' % (api.env.enable_ra,))
+
 import os, stat, subprocess
 import array
 import errno
@@ -40,7 +45,7 @@ from urllib import urlencode, quote
 from socket import gethostname
 import socket
 
-from ipalib import api, Backend
+from ipalib import Backend
 from ipalib.errors2 import NetworkError
 from ipaserver import servercore
 from ipaserver import ipaldap
@@ -418,5 +423,4 @@ class ra(Backend):
         # self.debug("IPA-RA: stderr: '%s'" % stderr)
         return (p.returncode, stdout, stderr)
 
-if api.env.enable_ra:
-    api.register(ra)
+api.register(ra)