plugable: Load plugins only from modules imported by API

Previously all plugin modules imported from anywhere were added to the API.

https://fedorahosted.org/freeipa/ticket/3090

Reviewed-By: Martin Babinsky <mbabinsk@redhat.com>

10 files changed, 45 insertions, 30 deletions

diff --git a/ipalib/__init__.py b/ipalib/__init__.py
index 74a812033..a6fad545c 100644
--- a/ipalib/__init__.py
+++ b/ipalib/__init__.py
@@ -936,7 +936,7 @@ api = create_api(mode=None)
 
 if os.environ.get('IPA_UNIT_TEST_MODE', None) == 'cli_test':
     from cli import cli_plugins
-    for klass in cli_plugins:
-        api.register(klass)
     api.bootstrap(context='cli', in_server=False, in_tree=True)
+    for klass in cli_plugins:
+        api.add_plugin(klass)
     api.finalize()
diff --git a/ipalib/cli.py b/ipalib/cli.py
index 52529ea02..c0e8c9c93 100644
--- a/ipalib/cli.py
+++ b/ipalib/cli.py
@@ -1333,8 +1333,7 @@ def run(api):
     try:
         (options, argv) = api.bootstrap_with_global_options(context='cli')
         for klass in cli_plugins:
-            api.register(klass)
-        api.load_plugins()
+            api.add_plugin(klass)
         api.finalize()
         if not 'config_loaded' in api.env and not 'help' in argv:
             raise NotConfiguredError()
diff --git a/ipalib/plugable.py b/ipalib/plugable.py
index d98ab731e..d13e5ed88 100644
--- a/ipalib/plugable.py
+++ b/ipalib/plugable.py
@@ -612,8 +612,6 @@ class API(DictProxy):
             return
         for module in self.modules:
             self.import_plugins(module)
-        for klass, kwargs in self.register.iteritems():
-            self.add_plugin(klass, **kwargs)
 
     # FIXME: This method has no unit test
     def import_plugins(self, module):
@@ -651,7 +649,7 @@ class API(DictProxy):
         for name in modules:
             self.log.debug("importing plugin module %s", name)
             try:
-                importlib.import_module(name)
+                module = importlib.import_module(name)
             except errors.SkipPluginModule, e:
                 self.log.debug("skipping plugin module %s: %s", name, e.reason)
             except StandardError, e:
@@ -660,6 +658,23 @@ class API(DictProxy):
                     self.log.error("could not load plugin module %s\n%s", name,
                                    traceback.format_exc())
                 raise
+            else:
+                self.add_module(module)
+
+    def add_module(self, module):
+        """
+        Add plugins from the ``module``.
+
+        :param module: A module from which to add plugins.
+        """
+        for name in dir(module):
+            klass = getattr(module, name)
+            if not inspect.isclass(klass):
+                continue
+            if klass not in self.register:
+                continue
+            kwargs = self.register[klass]
+            self.add_plugin(klass, **kwargs)
 
     def add_plugin(self, klass, override=False):
         """
@@ -679,6 +694,9 @@ class API(DictProxy):
 
             found = True
 
+            if sub_d.get(klass.__name__) is klass:
+                continue
+
             # Check override:
             if klass.__name__ in sub_d:
                 if not override:
diff --git a/ipatests/test_ipalib/test_backend.py b/ipatests/test_ipalib/test_backend.py
index 121c4745b..91eb92057 100644
--- a/ipatests/test_ipalib/test_backend.py
+++ b/ipatests/test_ipalib/test_backend.py
@@ -186,7 +186,7 @@ class test_Executioner(ClassChecker):
             def execute(self, *args, **options):
                 assert type(args[1]) is tuple
                 return dict(result=args + (options,))
-        api.register(echo)
+        api.add_plugin(echo)
 
         class good(Command):
             def execute(self, **options):
@@ -194,12 +194,12 @@ class test_Executioner(ClassChecker):
                     name='nurse',
                     error=u'Not naughty!',
                 )
-        api.register(good)
+        api.add_plugin(good)
 
         class bad(Command):
             def execute(self, **options):
                 raise ValueError('This is private.')
-        api.register(bad)
+        api.add_plugin(bad)
 
         class with_name(Command):
             """
@@ -208,7 +208,7 @@ class test_Executioner(ClassChecker):
             takes_options = 'name'
             def execute(self, **options):
                 return dict(result=options['name'].upper())
-        api.register(with_name)
+        api.add_plugin(with_name)
 
         api.finalize()
         o = self.cls()
diff --git a/ipatests/test_ipalib/test_crud.py b/ipatests/test_ipalib/test_crud.py
index 6d29ab97a..f17371659 100644
--- a/ipatests/test_ipalib/test_crud.py
+++ b/ipatests/test_ipalib/test_crud.py
@@ -47,8 +47,8 @@ class CrudChecker(ClassChecker):
         class user_verb(self.cls):
             takes_args = args
             takes_options = options
-        api.register(user)
-        api.register(user_verb)
+        api.add_plugin(user)
+        api.add_plugin(user_verb)
         api.finalize()
         return api
 
diff --git a/ipatests/test_ipalib/test_frontend.py b/ipatests/test_ipalib/test_frontend.py
index b4fbf3ea3..c47113f6e 100644
--- a/ipatests/test_ipalib/test_frontend.py
+++ b/ipatests/test_ipalib/test_frontend.py
@@ -817,7 +817,7 @@ class test_LocalOrRemote(ClassChecker):
 
         # Test when in_server=False:
         (api, home) = create_test_api(in_server=False)
-        api.register(example)
+        api.add_plugin(example)
         api.finalize()
         cmd = api.Command.example
         assert cmd(version=u'2.47') == dict(
@@ -835,7 +835,7 @@ class test_LocalOrRemote(ClassChecker):
 
         # Test when in_server=True (should always call execute):
         (api, home) = create_test_api(in_server=True)
-        api.register(example)
+        api.add_plugin(example)
         api.finalize()
         cmd = api.Command.example
         assert cmd(version=u'2.47') == dict(
@@ -1012,10 +1012,10 @@ class test_Object(ClassChecker):
         (api, home) = create_test_api()
         class ldap(backend.Backend):
             whatever = 'It worked!'
-        api.register(ldap)
+        api.add_plugin(ldap)
         class user(frontend.Object):
             backend_name = 'ldap'
-        api.register(user)
+        api.add_plugin(user)
         api.finalize()
         b = api.Object.user.backend
         assert isinstance(b, ldap)
@@ -1118,8 +1118,8 @@ class test_Method(ClassChecker):
         class user_verb(self.cls):
             takes_args = args
             takes_options = options
-        api.register(user)
-        api.register(user_verb)
+        api.add_plugin(user)
+        api.add_plugin(user_verb)
         api.finalize()
         return api
 
diff --git a/ipatests/test_ipalib/test_plugable.py b/ipatests/test_ipalib/test_plugable.py
index 2a6f8aa41..b0f607024 100644
--- a/ipatests/test_ipalib/test_plugable.py
+++ b/ipatests/test_ipalib/test_plugable.py
@@ -384,7 +384,7 @@ class test_API(ClassChecker):
         api = plugable.API([base0, base1], [])
         api.env.mode = 'unit_test'
         api.env.in_tree = True
-        r = api.register
+        r = api.add_plugin
 
         class base0_plugin0(base0):
             pass
diff --git a/ipatests/test_ipaserver/test_ldap.py b/ipatests/test_ipaserver/test_ldap.py
index 8b4e50058..2c187b0c7 100644
--- a/ipatests/test_ipaserver/test_ldap.py
+++ b/ipatests/test_ipaserver/test_ldap.py
@@ -110,10 +110,10 @@ class test_ldap(object):
         # we need for the test.
         myapi = create_api(mode=None)
         myapi.bootstrap(context='cli', in_server=True, in_tree=True)
-        myapi.register(ldap2)
-        myapi.register(host)
-        myapi.register(service)
-        myapi.register(service_show)
+        myapi.add_plugin(ldap2)
+        myapi.add_plugin(host)
+        myapi.add_plugin(service)
+        myapi.add_plugin(service_show)
         myapi.finalize()
 
         pwfile = api.env.dot_ipa + os.sep + ".dmpw"
diff --git a/ipatests/util.py b/ipatests/util.py
index 247714a3b..a8899cc6f 100644
--- a/ipatests/util.py
+++ b/ipatests/util.py
@@ -505,9 +505,9 @@ class PluginTester(object):
         :param kw: Additional \**kw args to pass to `create_test_api`.
         """
         (api, home) = create_test_api(**kw)
-        api.register(self.plugin)
+        api.add_plugin(self.plugin)
         for p in plugins:
-            api.register(p)
+            api.add_plugin(p)
         return (api, home)
 
     def finalize(self, *plugins, **kw):
diff --git a/makeaci b/makeaci
index 5c441c0d3..e85060957 100755
--- a/makeaci
+++ b/makeaci
@@ -95,10 +95,8 @@ def main(options):
         basedn=DN('dc=ipa,dc=example'),
         realm='IPA.EXAMPLE',
     )
-    from ipaserver.plugins import ldap2
-    from ipaserver.install.plugins.update_managed_permissions import (
-        update_managed_permissions)
-    from ipalib.plugins.permission import permission
+    api.import_plugins('ipaserver.plugins.ldap2')
+    api.import_plugins('ipaserver.install.plugins.update_managed_permissions')
     api.finalize()
 
     output_lines = list(generate_aci_lines(api))