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>

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()

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()

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:

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()

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
 

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
 

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

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"

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):

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))