Hi,

the attached patches provide an attempt to fix <https://fedorahosted.org/freeipa/ticket/3090>.

Patch 401 serves as an example and modifies ipa-advise to use its own API instance for Advice plugins.

Honza

--
Jan Cholasta
>From 3715c9b4ca43eab6c5ad01b34cd1b14838241bde Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Mon, 16 Feb 2015 13:11:38 +0000
Subject: [PATCH 1/3] ipalib: Allow multiple API instances

Merged the Registrar class into the Registry class. Plugins are now
registered globally instead of in ipalib.api and are instantiated per-API
instance. Different set of plugin base classes can be used in each API
instance.

https://fedorahosted.org/freeipa/ticket/3090
---
 ipalib/backend.py                     |   3 +
 ipalib/frontend.py                    |  10 +-
 ipalib/plugable.py                    | 204 ++++++++++++++++------------------
 ipatests/test_ipalib/test_plugable.py | 119 ++++++++++++--------
 4 files changed, 185 insertions(+), 151 deletions(-)

diff --git a/ipalib/backend.py b/ipalib/backend.py
index 2100589..4c1001d 100644
--- a/ipalib/backend.py
+++ b/ipalib/backend.py
@@ -27,7 +27,10 @@ import os
 from errors import PublicError, InternalError, CommandError
 from request import context, Connection, destroy_context
 
+register = plugable.Registry()
 
+
+@register.base()
 class Backend(plugable.Plugin):
     """
     Base class for all backend plugins.
diff --git a/ipalib/frontend.py b/ipalib/frontend.py
index 98070b8..e82a03a 100644
--- a/ipalib/frontend.py
+++ b/ipalib/frontend.py
@@ -27,7 +27,7 @@ from distutils import version
 from ipapython.version import API_VERSION
 from ipapython.ipa_log_manager import root_logger
 from base import NameSpace
-from plugable import Plugin, is_production_mode
+from plugable import Plugin, Registry, is_production_mode
 from parameters import create_param, Param, Str, Flag, Password
 from output import Output, Entry, ListOfEntries
 from text import _
@@ -40,6 +40,9 @@ from textwrap import wrap
 
 RULE_FLAG = 'validation_rule'
 
+register = Registry()
+
+
 def rule(obj):
     assert not hasattr(obj, RULE_FLAG)
     setattr(obj, RULE_FLAG, True)
@@ -366,6 +369,7 @@ class HasParam(Plugin):
         setattr(self, name, namespace)
 
 
+@register.base()
 class Command(HasParam):
     """
     A public IPA atomic operation.
@@ -1120,6 +1124,7 @@ class Local(Command):
         return self.forward(*args, **options)
 
 
+@register.base()
 class Object(HasParam):
     finalize_early = False
 
@@ -1278,6 +1283,7 @@ class Attribute(Plugin):
         super(Attribute, self)._on_finalize()
 
 
+@register.base()
 class Method(Attribute, Command):
     """
     A command with an associated object.
@@ -1364,6 +1370,7 @@ class Method(Attribute, Command):
             yield param
 
 
+@register.base()
 class Updater(Method):
     """
     An LDAP update with an associated object (always update).
@@ -1423,6 +1430,7 @@ class _AdviceOutput(object):
         self.content.append(line)
 
 
+@register.base()
 class Advice(Plugin):
     """
     Base class for advices, plugins for ipa-advise.
diff --git a/ipalib/plugable.py b/ipalib/plugable.py
index a6504d1..aae7626 100644
--- a/ipalib/plugable.py
+++ b/ipalib/plugable.py
@@ -74,18 +74,94 @@ class Registry(object):
     For forward compatibility, make sure that the module-level instance of
     this object is named "register".
     """
-    # TODO: Instead of auto-loading when plugin modules are imported,
-    # plugins should be stored in this object.
-    # The API should examine it and load plugins explicitly.
-    def __call__(self):
-        from ipalib import api
 
-        def decorator(cls):
-            api.register(cls)
-            return cls
+    __allowed = {}
+    __registered = set()
+
+    def base(self):
+        def decorator(base):
+            if not inspect.isclass(base):
+                raise TypeError('plugin base must be a class; got %r' % base)
+
+            if base in self.__allowed:
+                raise errors.PluginDuplicateError(plugin=base)
+
+            self.__allowed[base] = {}
+
+            return base
+
+        return decorator
+
+    def __findbases(self, klass):
+        """
+        Iterates through allowed bases that ``klass`` is a subclass of.
+
+        Raises `errors.PluginSubclassError` if ``klass`` is not a subclass of
+        any allowed base.
+
+        :param klass: The plugin class to find bases for.
+        """
+        found = False
+        for (base, sub_d) in self.__allowed.iteritems():
+            if issubclass(klass, base):
+                found = True
+                yield (base, sub_d)
+        if not found:
+            raise errors.PluginSubclassError(
+                plugin=klass, bases=self.__allowed.keys()
+            )
+
+    def __call__(self, override=False):
+        def decorator(klass):
+            if not inspect.isclass(klass):
+                raise TypeError('plugin must be a class; got %r' % klass)
+
+            # Raise DuplicateError if this exact class was already registered:
+            if klass in self.__registered:
+                raise errors.PluginDuplicateError(plugin=klass)
+
+            # Find the base class or raise SubclassError:
+            for (base, sub_d) in self.__findbases(klass):
+                # Check override:
+                if klass.__name__ in sub_d:
+                    if not override:
+                        # Must use override=True to override:
+                        raise errors.PluginOverrideError(
+                            base=base.__name__,
+                            name=klass.__name__,
+                            plugin=klass,
+                        )
+                else:
+                    if override:
+                        # There was nothing already registered to override:
+                        raise errors.PluginMissingOverrideError(
+                            base=base.__name__,
+                            name=klass.__name__,
+                            plugin=klass,
+                        )
+
+                # The plugin is okay, add to sub_d:
+                sub_d[klass.__name__] = klass
+
+            # The plugin is okay, add to __registered:
+            self.__registered.add(klass)
+
+            return klass
 
         return decorator
 
+    def __base_iter(self, *allowed):
+        for base in allowed:
+            sub_d = self.__allowed[base]
+            subclasses = set(sub_d.itervalues())
+            yield (base, subclasses)
+
+    def iter(self, *allowed):
+        for base in allowed:
+            if base not in self.__allowed:
+                raise TypeError("unknown plugin base %r" % base)
+        return self.__base_iter(*allowed)
+
 
 class SetProxy(ReadOnly):
     """
@@ -365,111 +441,28 @@ class Plugin(ReadOnly):
         )
 
 
-class Registrar(DictProxy):
-    """
-    Collects plugin classes as they are registered.
-
-    The Registrar does not instantiate plugins... it only implements the
-    override logic and stores the plugins in a namespace per allowed base
-    class.
-
-    The plugins are instantiated when `API.finalize()` is called.
-    """
-    def __init__(self, *allowed):
-        """
-        :param allowed: Base classes from which plugins accepted by this
-            Registrar must subclass.
-        """
-        self.__allowed = dict((base, {}) for base in allowed)
-        self.__registered = set()
-        super(Registrar, self).__init__(
-            dict(self.__base_iter())
-        )
-
-    def __base_iter(self):
-        for (base, sub_d) in self.__allowed.iteritems():
-            if not is_production_mode(self):
-                assert inspect.isclass(base)
-            name = base.__name__
-            if not is_production_mode(self):
-                assert not hasattr(self, name)
-            setattr(self, name, MagicDict(sub_d))
-            yield (name, base)
-
-    def __findbases(self, klass):
-        """
-        Iterates through allowed bases that ``klass`` is a subclass of.
-
-        Raises `errors.PluginSubclassError` if ``klass`` is not a subclass of
-        any allowed base.
-
-        :param klass: The plugin class to find bases for.
-        """
-        if not is_production_mode(self):
-            assert inspect.isclass(klass)
-        found = False
-        for (base, sub_d) in self.__allowed.iteritems():
-            if issubclass(klass, base):
-                found = True
-                yield (base, sub_d)
-        if not found:
-            raise errors.PluginSubclassError(
-                plugin=klass, bases=self.__allowed.keys()
-            )
-
-    def __call__(self, klass, override=False):
-        """
-        Register the plugin ``klass``.
-
-        :param klass: A subclass of `Plugin` to attempt to register.
-        :param override: If true, override an already registered plugin.
-        """
-        if not inspect.isclass(klass):
-            raise TypeError('plugin must be a class; got %r' % klass)
-
-        # Raise DuplicateError if this exact class was already registered:
-        if klass in self.__registered:
-            raise errors.PluginDuplicateError(plugin=klass)
-
-        # Find the base class or raise SubclassError:
-        for (base, sub_d) in self.__findbases(klass):
-            # Check override:
-            if klass.__name__ in sub_d:
-                if not override:
-                    # Must use override=True to override:
-                    raise errors.PluginOverrideError(
-                        base=base.__name__,
-                        name=klass.__name__,
-                        plugin=klass,
-                    )
-            else:
-                if override:
-                    # There was nothing already registered to override:
-                    raise errors.PluginMissingOverrideError(
-                        base=base.__name__,
-                        name=klass.__name__,
-                        plugin=klass,
-                    )
-
-            # The plugin is okay, add to sub_d:
-            sub_d[klass.__name__] = klass
-
-        # The plugin is okay, add to __registered:
-        self.__registered.add(klass)
-
-
 class API(DictProxy):
     """
     Dynamic API object through which `Plugin` instances are accessed.
     """
 
     def __init__(self, *allowed):
+        self.__allowed = allowed
         self.__d = dict()
         self.__done = set()
-        self.register = Registrar(*allowed)
+        self.__registry = Registry()
         self.env = Env()
         super(API, self).__init__(self.__d)
 
+    def register(self, klass, override=False):
+        """
+        Register the plugin ``klass``.
+
+        :param klass: A subclass of `Plugin` to attempt to register.
+        :param override: If true, override an already registered plugin.
+        """
+        self.__registry(override)(klass)
+
     def __doing(self, name):
         if name in self.__done:
             raise StandardError(
@@ -752,11 +745,10 @@ class API(DictProxy):
                 yield p.instance
 
         production_mode = is_production_mode(self)
-        for name in self.register:
-            base = self.register[name]
-            magic = getattr(self.register, name)
+        for base, subclasses in self.__registry.iter(*self.__allowed):
+            name = base.__name__
             namespace = NameSpace(
-                plugin_iter(base, (magic[k] for k in magic))
+                plugin_iter(base, subclasses)
             )
             if not production_mode:
                 assert not (
diff --git a/ipatests/test_ipalib/test_plugable.py b/ipatests/test_ipalib/test_plugable.py
index 6762e70..ad1f79f 100644
--- a/ipatests/test_ipalib/test_plugable.py
+++ b/ipatests/test_ipalib/test_plugable.py
@@ -287,9 +287,9 @@ class test_Plugin(ClassChecker):
         assert e.argv == (paths.BIN_FALSE,)
 
 
-def test_Registrar():
+def test_Registry():
     """
-    Test the `ipalib.plugable.Registrar` class
+    Test the `ipalib.plugable.Registry` class
     """
     class Base1(object):
         pass
@@ -304,20 +304,47 @@ def test_Registrar():
     class plugin3(Base3):
         pass
 
-    # Test creation of Registrar:
-    r = plugable.Registrar(Base1, Base2)
+    # Test creation of Registry:
+    register = plugable.Registry()
+    def b(klass):
+        register.base()(klass)
+    def r(klass, override=False):
+        register(override=override)(klass)
 
-    # Test __iter__:
-    assert list(r) == ['Base1', 'Base2']
+    # Check that TypeError is raised trying to register base that isn't
+    # a class:
+    p = Base1()
+    e = raises(TypeError, b, p)
+    assert str(e) == 'plugin base must be a class; got %r' % p
 
-    # Test __hasitem__, __getitem__:
-    for base in [Base1, Base2]:
-        name = base.__name__
-        assert name in r
-        assert r[name] is base
-        magic = getattr(r, name)
-        assert type(magic) is plugable.MagicDict
-        assert len(magic) == 0
+    # Check that base registration works
+    b(Base1)
+    i = tuple(register.iter(Base1))
+    assert len(i) == 1
+    assert i[0][0] is Base1
+    assert not i[0][1]
+
+    # Check that DuplicateError is raised trying to register exact class
+    # again:
+    e = raises(errors.PluginDuplicateError, b, Base1)
+    assert e.plugin is Base1
+
+    # Test that another base can be registered:
+    b(Base2)
+    i = tuple(register.iter(Base2))
+    assert len(i) == 1
+    assert i[0][0] is Base2
+    assert not i[0][1]
+
+    # Test iter:
+    i = tuple(register.iter(Base1, Base2))
+    assert len(i) == 2
+    assert i[0][0] is Base1
+    assert not i[0][1]
+    assert i[1][0] is Base2
+    assert not i[1][1]
+    e = raises(TypeError, register.iter, Base1, Base2, Base3)
+    assert str(e) == 'unknown plugin base %r' % Base3
 
     # Check that TypeError is raised trying to register something that isn't
     # a class:
@@ -332,9 +359,10 @@ def test_Registrar():
 
     # Check that registration works
     r(plugin1)
-    assert len(r.Base1) == 1
-    assert r.Base1['plugin1'] is plugin1
-    assert r.Base1.plugin1 is plugin1
+    i = tuple(register.iter(Base1))
+    assert len(i) == 1
+    assert i[0][0] is Base1
+    assert i[0][1] == {plugin1}
 
     # Check that DuplicateError is raised trying to register exact class
     # again:
@@ -355,9 +383,10 @@ def test_Registrar():
 
     # Check that overriding works
     r(plugin1, override=True)
-    assert len(r.Base1) == 1
-    assert r.Base1.plugin1 is plugin1
-    assert r.Base1.plugin1 is not orig1
+    i = tuple(register.iter(Base1))
+    assert len(i) == 1
+    assert i[0][0] is Base1
+    assert i[0][1] == {plugin1}
 
     # Check that MissingOverrideError is raised trying to override a name
     # not yet registerd:
@@ -367,10 +396,15 @@ def test_Registrar():
     assert e.plugin is plugin2
 
     # Test that another plugin can be registered:
-    assert len(r.Base2) == 0
+    i = tuple(register.iter(Base2))
+    assert len(i) == 1
+    assert i[0][0] is Base2
+    assert not i[0][1]
     r(plugin2)
-    assert len(r.Base2) == 1
-    assert r.Base2.plugin2 is plugin2
+    i = tuple(register.iter(Base2))
+    assert len(i) == 1
+    assert i[0][0] is Base2
+    assert i[0][1] == {plugin2}
 
     # Setup to test more registration:
     class plugin1a(Base1):
@@ -389,17 +423,13 @@ def test_Registrar():
         pass
     r(plugin2b)
 
-    # Again test __hasitem__, __getitem__:
-    for base in [Base1, Base2]:
-        name = base.__name__
-        assert name in r
-        assert r[name] is base
-        magic = getattr(r, name)
-        assert len(magic) == 3
-        for key in magic:
-            klass = magic[key]
-            assert getattr(magic, key) is klass
-            assert issubclass(klass, base)
+    # Again test iter:
+    i = tuple(register.iter(Base1, Base2))
+    assert len(i) == 2
+    assert i[0][0] is Base1
+    assert i[0][1] == {plugin1, plugin1a, plugin1b}
+    assert i[1][0] is Base2
+    assert i[1][1] == {plugin2, plugin2a, plugin2b}
 
 
 class test_API(ClassChecker):
@@ -415,45 +445,46 @@ class test_API(ClassChecker):
         """
         assert issubclass(plugable.API, plugable.ReadOnly)
 
+        register = plugable.Registry()
+
         # Setup the test bases, create the API:
+        @register.base()
         class base0(plugable.Plugin):
             def method(self, n):
                 return n
 
+        @register.base()
         class base1(plugable.Plugin):
             def method(self, n):
                 return n + 1
 
-        api = plugable.API(base0, base1)
+        api = plugable.API([base0, base1], [])
         api.env.mode = 'unit_test'
         api.env.in_tree = True
-        r = api.register
-        assert isinstance(r, plugable.Registrar)
-        assert read_only(api, 'register') is r
 
+        @register()
         class base0_plugin0(base0):
             pass
-        r(base0_plugin0)
 
+        @register()
         class base0_plugin1(base0):
             pass
-        r(base0_plugin1)
 
+        @register()
         class base0_plugin2(base0):
             pass
-        r(base0_plugin2)
 
+        @register()
         class base1_plugin0(base1):
             pass
-        r(base1_plugin0)
 
+        @register()
         class base1_plugin1(base1):
             pass
-        r(base1_plugin1)
 
+        @register()
         class base1_plugin2(base1):
             pass
-        r(base1_plugin2)
 
         # Test API instance:
         assert api.isdone('bootstrap') is False
-- 
2.1.0

>From 39b9a76716de3178ec001a28230e924b26d707e0 Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Tue, 3 Mar 2015 14:33:48 +0000
Subject: [PATCH 2/3] ipalib: Move plugin package setup to ipalib-specific API
 subclass

https://fedorahosted.org/freeipa/ticket/3090
---
 ipalib/__init__.py | 17 ++++++++++++++++-
 ipalib/plugable.py | 12 ++++--------
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/ipalib/__init__.py b/ipalib/__init__.py
index 6b1ec57..381f381 100644
--- a/ipalib/__init__.py
+++ b/ipalib/__init__.py
@@ -898,6 +898,21 @@ else:
     __version__ = '%d.%d.%d.%s.%d' % version_info
 
 
+class API(plugable.API):
+    def __init__(self, allowed):
+        super(API, self).__init__(allowed, ['ipalib'])
+
+    def bootstrap(self, parser=None, **overrides):
+        super(API, self).bootstrap(parser, **overrides)
+
+        if self.env.context in ('server', 'lite'):
+            self.packages.append('ipaserver')
+        if self.env.context in ('installer', 'updates'):
+            self.packages.append('ipaserver/install/plugins')
+        if self.env.context in ('advise',):
+            self.packages.append('ipaserver/advise/plugins')
+
+
 def create_api(mode='dummy'):
     """
     Return standard `plugable.API` instance.
@@ -915,7 +930,7 @@ def create_api(mode='dummy'):
 
         - `backend.Backend`
     """
-    api = plugable.API(Command, Object, Method, Backend, Updater, Advice)
+    api = API((Command, Object, Method, Backend, Updater, Advice))
     if mode is not None:
         api.env.mode = mode
     assert mode != 'production'
diff --git a/ipalib/plugable.py b/ipalib/plugable.py
index aae7626..4c42e1e 100644
--- a/ipalib/plugable.py
+++ b/ipalib/plugable.py
@@ -446,8 +446,9 @@ class API(DictProxy):
     Dynamic API object through which `Plugin` instances are accessed.
     """
 
-    def __init__(self, *allowed):
+    def __init__(self, allowed, packages):
         self.__allowed = allowed
+        self.packages = packages
         self.__d = dict()
         self.__done = set()
         self.__registry = Registry()
@@ -635,13 +636,8 @@ class API(DictProxy):
         self.__do_if_not_done('bootstrap')
         if self.env.mode in ('dummy', 'unit_test'):
             return
-        self.import_plugins('ipalib')
-        if self.env.context in ('server', 'lite'):
-            self.import_plugins('ipaserver')
-        if self.env.context in ('installer', 'updates'):
-            self.import_plugins('ipaserver/install/plugins')
-        if self.env.context in ('advise'):
-            self.import_plugins('ipaserver/advise/plugins')
+        for package in self.packages:
+            self.import_plugins(package)
 
     # FIXME: This method has no unit test
     def import_plugins(self, package):
-- 
2.1.0

>From 8af1bc7935da191e5cbe5fa7e105d435c3485d8b Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Mon, 16 Feb 2015 16:34:01 +0000
Subject: [PATCH 3/3] advise: Add separate API object for ipa-advise

---
 ipalib/__init__.py                            |  8 +---
 ipalib/frontend.py                            | 50 --------------------
 ipaserver/advise/base.py                      | 67 +++++++++++++++++++++++++--
 ipaserver/advise/plugins/fedora_authconfig.py |  9 ++--
 ipaserver/advise/plugins/legacy_clients.py    | 27 ++++-------
 5 files changed, 79 insertions(+), 82 deletions(-)

diff --git a/ipalib/__init__.py b/ipalib/__init__.py
index 381f381..0b58790 100644
--- a/ipalib/__init__.py
+++ b/ipalib/__init__.py
@@ -882,7 +882,7 @@ freeIPA.org:
 import os
 import plugable
 from backend import Backend
-from frontend import Command, LocalOrRemote, Updater, Advice
+from frontend import Command, LocalOrRemote, Updater
 from frontend import Object, Method
 from crud import Create, Retrieve, Update, Delete, Search
 from parameters import DefaultFrom, Bool, Flag, Int, Decimal, Bytes, Str, IA5Str, Password, DNParam, DeprecatedParam
@@ -909,8 +909,6 @@ class API(plugable.API):
             self.packages.append('ipaserver')
         if self.env.context in ('installer', 'updates'):
             self.packages.append('ipaserver/install/plugins')
-        if self.env.context in ('advise',):
-            self.packages.append('ipaserver/advise/plugins')
 
 
 def create_api(mode='dummy'):
@@ -926,11 +924,9 @@ def create_api(mode='dummy'):
 
         - `frontend.Method`
 
-        - `frontend.Advice`
-
         - `backend.Backend`
     """
-    api = API((Command, Object, Method, Backend, Updater, Advice))
+    api = API((Command, Object, Method, Backend, Updater))
     if mode is not None:
         api.env.mode = mode
     assert mode != 'production'
diff --git a/ipalib/frontend.py b/ipalib/frontend.py
index e82a03a..36d9ab2 100644
--- a/ipalib/frontend.py
+++ b/ipalib/frontend.py
@@ -1406,53 +1406,3 @@ class Updater(Method):
         )
 
         return self.execute(**options)
-
-
-class _AdviceOutput(object):
-
-    def __init__(self):
-        self.content = []
-        self.prefix = '# '
-        self.options = None
-
-    def comment(self, line, wrapped=True):
-        if wrapped:
-            for wrapped_line in wrap(line, 70):
-                self.content.append(self.prefix + wrapped_line)
-        else:
-            self.content.append(self.prefix + line)
-
-    def debug(self, line):
-        if self.options.verbose:
-            self.comment('DEBUG: ' + line)
-
-    def command(self, line):
-        self.content.append(line)
-
-
-@register.base()
-class Advice(Plugin):
-    """
-    Base class for advices, plugins for ipa-advise.
-    """
-
-    options = None
-    require_root = False
-    description = ''
-
-    def __init__(self):
-        super(Advice, self).__init__()
-        self.log = _AdviceOutput()
-
-    def set_options(self, options):
-        self.options = options
-        self.log.options = options
-
-    def get_info(self):
-        """
-        This method should be overriden by child Advices.
-
-        Returns a string with instructions.
-        """
-
-        raise NotImplementedError
diff --git a/ipaserver/advise/base.py b/ipaserver/advise/base.py
index 3f57f35..0c68358 100644
--- a/ipaserver/advise/base.py
+++ b/ipaserver/advise/base.py
@@ -19,11 +19,14 @@
 
 import os
 from ipalib import api
+from ipalib.plugable import Plugin, Registry, API
 from ipalib.errors import ValidationError
 from ipapython import admintool
 from textwrap import wrap
 from ipapython.ipa_log_manager import log_mgr
 
+register = Registry()
+
 
 """
 To add configuration instructions for a new use case, define a new class that
@@ -72,6 +75,58 @@ Important! Do not forget to register the class to the API.
 """
 
 
+class _AdviceOutput(object):
+
+    def __init__(self):
+        self.content = []
+        self.prefix = '# '
+        self.options = None
+
+    def comment(self, line, wrapped=True):
+        if wrapped:
+            for wrapped_line in wrap(line, 70):
+                self.content.append(self.prefix + wrapped_line)
+        else:
+            self.content.append(self.prefix + line)
+
+    def debug(self, line):
+        if self.options.verbose:
+            self.comment('DEBUG: ' + line)
+
+    def command(self, line):
+        self.content.append(line)
+
+
+@register.base()
+class Advice(Plugin):
+    """
+    Base class for advices, plugins for ipa-advise.
+    """
+
+    options = None
+    require_root = False
+    description = ''
+
+    def __init__(self):
+        super(Advice, self).__init__()
+        self.log = _AdviceOutput()
+
+    def set_options(self, options):
+        self.options = options
+        self.log.options = options
+
+    def get_info(self):
+        """
+        This method should be overriden by child Advices.
+
+        Returns a string with instructions.
+        """
+
+        raise NotImplementedError
+
+advise_api = API((Advice,), ('ipaserver/advise/plugins',))
+
+
 class IpaAdvise(admintool.AdminTool):
     """
     Admin tool that given systems's configuration provides instructions how to
@@ -104,10 +159,10 @@ class IpaAdvise(admintool.AdminTool):
     def print_config_list(self):
         self.print_header('List of available advices')
 
-        max_keyword_len = max((len(keyword) for keyword in api.Advice))
+        max_keyword_len = max((len(keyword) for keyword in advise_api.Advice))
 
-        for keyword in api.Advice:
-            advice = getattr(api.Advice, keyword, '')
+        for keyword in advise_api.Advice:
+            advice = getattr(advise_api.Advice, keyword, '')
             description = getattr(advice, 'description', '')
             keyword = keyword.replace('_', '-')
 
@@ -139,7 +194,7 @@ class IpaAdvise(admintool.AdminTool):
             print(prefix + '-' * 70)
 
     def print_advice(self, keyword):
-        advice = getattr(api.Advice, keyword, None)
+        advice = getattr(advise_api.Advice, keyword, None)
 
         # Ensure that Configuration class for given --setup option value exists
         if advice is None:
@@ -172,8 +227,10 @@ class IpaAdvise(admintool.AdminTool):
     def run(self):
         super(IpaAdvise, self).run()
 
-        api.bootstrap(in_server=False, context='advise')
+        api.bootstrap(in_server=False, context='cli')
         api.finalize()
+        advise_api.bootstrap(in_server=False, context='cli')
+        advise_api.finalize()
         if not self.options.verbose:
             # Do not print connection information by default
             logger_name = r'ipa\.ipalib\.plugins\.rpcclient'
diff --git a/ipaserver/advise/plugins/fedora_authconfig.py b/ipaserver/advise/plugins/fedora_authconfig.py
index 915877d..3a46d7f 100644
--- a/ipaserver/advise/plugins/fedora_authconfig.py
+++ b/ipaserver/advise/plugins/fedora_authconfig.py
@@ -18,9 +18,13 @@
 #
 
 from ipalib import api
-from ipalib.frontend import Advice
+from ipalib.plugable import Registry
+from ipaserver.advise.base import Advice
 
+register = Registry()
 
+
+@register()
 class config_fedora_authconfig(Advice):
     """
     Provides client configuration instructions using authconfig.
@@ -36,6 +40,3 @@ class config_fedora_authconfig(Advice):
                    "--enablerfc2307bis --enablekrb5"
         advice = template.format(server=api.env.host)
         self.log.command(advice)
-
-
-api.register(config_fedora_authconfig)
diff --git a/ipaserver/advise/plugins/legacy_clients.py b/ipaserver/advise/plugins/legacy_clients.py
index 6d17f7e..e673cb4 100644
--- a/ipaserver/advise/plugins/legacy_clients.py
+++ b/ipaserver/advise/plugins/legacy_clients.py
@@ -19,9 +19,12 @@
 import os
 
 from ipalib import api
-from ipalib.frontend import Advice
+from ipalib.plugable import Registry
+from ipaserver.advise.base import Advice
 from ipapython.ipautil import template_file, SHARE_DIR
 
+register = Registry()
+
 
 class config_base_legacy_client(Advice):
     def get_uri_and_base(self):
@@ -80,6 +83,7 @@ class config_base_legacy_client(Advice):
         self.log.command('service sssd start')
 
 
+@register()
 class config_redhat_sssd_before_1_9(config_base_legacy_client):
     """
     Legacy client configuration for Red Hat based systems, using SSSD.
@@ -113,9 +117,7 @@ class config_redhat_sssd_before_1_9(config_base_legacy_client):
         super(config_redhat_sssd_before_1_9, self).configure_ca_cert()
 
 
-api.register(config_redhat_sssd_before_1_9)
-
-
+@register()
 class config_generic_linux_sssd_before_1_9(config_base_legacy_client):
     """
     Legacy client configuration for non Red Hat based linux systems,
@@ -170,9 +172,7 @@ class config_generic_linux_sssd_before_1_9(config_base_legacy_client):
                          '/etc/ldap/ldap.conf\n')
 
 
-api.register(config_generic_linux_sssd_before_1_9)
-
-
+@register()
 class config_redhat_nss_pam_ldapd(config_base_legacy_client):
     """
     Legacy client configuration for Red Hat based systems,
@@ -207,9 +207,7 @@ class config_redhat_nss_pam_ldapd(config_base_legacy_client):
         super(config_redhat_nss_pam_ldapd, self).configure_ca_cert()
 
 
-api.register(config_redhat_nss_pam_ldapd)
-
-
+@register()
 class config_generic_linux_nss_pam_ldapd(config_base_legacy_client):
     """
     Legacy client configuration for non Red Hat based linux systems,
@@ -276,9 +274,7 @@ class config_generic_linux_nss_pam_ldapd(config_base_legacy_client):
                          '/etc/ldap/ldap.conf\n')
 
 
-api.register(config_generic_linux_nss_pam_ldapd)
-
-
+@register()
 class config_freebsd_nss_pam_ldapd(config_base_legacy_client):
     """
     Legacy client configuration for FreeBSD, using nss-pam-ldapd.
@@ -343,9 +339,8 @@ class config_freebsd_nss_pam_ldapd(config_base_legacy_client):
         self.log.command('curl -k https://%s/ipa/config/ca.crt > '
                          '%s' % (api.env.host, cacrt))
 
-api.register(config_freebsd_nss_pam_ldapd)
-
 
+@register()
 class config_redhat_nss_ldap(config_base_legacy_client):
     """
     Legacy client configuration for Red Hat based systems,
@@ -378,5 +373,3 @@ class config_redhat_nss_ldap(config_base_legacy_client):
                          'Therefore, clients older than RHEL5.2 will not be '
                          'able to interoperate with IPA server 3.x.')
         super(config_redhat_nss_ldap, self).configure_ca_cert()
-
-api.register(config_redhat_nss_ldap)
-- 
2.1.0

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to