Ldap: Complete renew of the Ldap plugin

5 files changed, 267 insertions, 255 deletions

diff --git a/doc/man/bcfg2.conf.txt b/doc/man/bcfg2.conf.txt
index 2f014812e..6c801ff1e 100644
--- a/doc/man/bcfg2.conf.txt
+++ b/doc/man/bcfg2.conf.txt
@@ -267,7 +267,7 @@ revision information out of your repository for reporting purposes.
 Ldap Plugin
 +++++++++++
 
-The Ldap plugin makes it possible to fetch data from an LDAP directory,
+The Ldap plugin makes it possible to fetch data from a LDAP directory,
 process it and attach it to your metadata.
 
 Metadata Plugin
diff --git a/doc/releases/1.4.0pre2.txt b/doc/releases/1.4.0pre2.txt
index 195f81df0..1dcdf237b 100644
--- a/doc/releases/1.4.0pre2.txt
+++ b/doc/releases/1.4.0pre2.txt
@@ -36,6 +36,15 @@ backwards-incompatible user-facing changes
 
   This allows to set arbitrary options with nested settings.
 
+* The Ldap plugin changed significantly. The configuration interface was
+  simplified and new configuration options for the number of retries and the
+  delay in between were added.
+
+  You have to register your ldap queries in the global list, there is no
+  distinction between LdapQueries and LdapSubQueries anymore, the names of
+  your queries default to the class names and the Ldap plugin expires
+  the metadata caches if the config file changes.
+
 Thanks
 ------
 
diff --git a/doc/server/plugins/grouping/ldap.txt b/doc/server/plugins/grouping/ldap.txt
index 90590a272..96e224761 100644
--- a/doc/server/plugins/grouping/ldap.txt
+++ b/doc/server/plugins/grouping/ldap.txt
@@ -33,39 +33,38 @@ next section.
 Configuration
 -------------
 
-As processing LDAP search results can get pretty complex, the configuration has 
+As processing LDAP search results can get pretty complex, the configuration has
 to be written in Python.
 
 Here is a minimal example to get you started::
 
-    from Bcfg2.Server.Plugins.Ldap import LdapConnection, LdapQuery, LdapSubQuery, register_query
-    
-    conn_default = LdapConnection()
-    conn_default.binddn = "uid=example,ou=People,dc=example,dc=com"
-    conn_default.bindpw = "foobat"
-    
-    @register_query
+    from Bcfg2.Server.Plugins.Ldap import LdapConnection, LdapQuery
+
+    __queries__ = ['ExampleQuery']
+
+    conn_default = LdapConnection(
+        binddn="uid=example,ou=People,dc=example,dc=com",
+        bindpw = "foobat")
+
     class ExampleQuery(LdapQuery):
-        name = "example"
         base = "ou=People,dc=example,dc=com"
         scope = "one"
         attrs = ["cn", "uid"]
         connection = conn_default
-    
+
         def prepare_query(self, metadata):
             self.filter = "(personalServer=" + metadata.hostname + ")"
-    
+
         def process_result(self, metadata):
             if not self.result:
                 admin_uid = None
                 admin_name = "This server has no admin."
-            return { 
+            return {
                 "admin_uid" : self.result[0][1]["uid"],
                 "admin_name" : self.result[0][1]["cn"]
             }
 
-The first line provides three classes for dealing with connections and queries
-(details below) and a decorator function for registering your queries with the plugin.
+The first line provides the two required classes for dealing with connections and queries.
 
 In this example our LDAP directory has a number of user objects in it. Each of those
 may have a personal server they administer. Whenever metadata for this machine is being
@@ -73,7 +72,20 @@ generated by the Bcfg2 server, the UID and name of the admin are retrieved from
 
 In your bundles and config templates, you can access this data via the metadata object::
 
-    ${metadata.Ldap["example"]["admin_name"]}
+    ${metadata.Ldap["ExampleQuery"]["admin_name"]}
+
+Connection retry
+++++++++++++++++
+
+If the LDAP server is down during a request, the LDAP plugin tries to reconnect after a
+short delay. By default, it waits 3 seconds during the retries and tries to reconnect
+up to three times.
+
+If you wish, you could customize these values in your ``bcfg2.conf``::
+
+    [ldap]
+    retries = 3
+    retry_delay = 3.0
 
 Class reference
 ---------------
@@ -83,23 +95,23 @@ LdapConnection
 
 .. class:: LdapConnection
 
-   This class represents an LDAP connection. Every query must be associated with exactly 
+   This class represents an LDAP connection. Every query must be associated with exactly
    one connection.
-   
-.. attribute:: LdapConnection.binddn 
-   
+
+.. attribute:: LdapConnection.binddn
+
    DN used to authenticate against LDAP (required).
-   
+
 .. attribute:: LdapConnection.bindpw
-   
+
    Password for the previously mentioned **binddn** (required).
-  
+
 .. attribute:: LdapConnection.host
-   
+
    Hostname of host running the LDAP server (defaults to "localhost").
 
 .. attribute:: LdapConnection.port
-   
+
    Port where LDAP server is listening (defaults to 389).
 
 You may pass any of these attributes as keyword arguments when creating the connection object.
@@ -108,143 +120,140 @@ LdapQuery
 +++++++++
 
 .. class:: LdapQuery
-   
+
    This class defines a single query that may adapt itself depending on the current metadata.
 
 .. attribute:: LdapQuery.attrs
-   
+
    Can be used to retrieve only a certain subset of attributes. May either be a list of
    strings (attribute names) or ``None``, meaning all attributes (defaults to ``None``).
 
 .. attribute:: LdapQuery.base
-   
-   This is the search base. Only LDAP entries below this DN will be included in your 
+
+   This is the search base. Only LDAP entries below this DN will be included in your
    search results (required).
-   
+
 .. attribute:: LdapQuery.connection
-   
+
    Set this to an instance of the LdapConnection class (required).
 
 .. attribute:: LdapQuery.filter
-   
+
    LDAP search filter used to narrow down search results (defaults to ``(objectClass=*)``).
 
 .. attribute:: LdapQuery.name
-   
+
    This will be used as the dictionary key that provides access to the query results from
-   the metadata object (``metadata.Ldap["NAMEGOESHERE"]``) (required).
+   the metadata object: ``metadata.Ldap["NAMEGOESHERE"]`` (defaults to the class name).
 
 .. attribute:: LdapQuery.scope
-   
-   Set this to one of "base", "one" or "sub" to specify LDAP search depth (defaults to "sub"). 
+
+   Set this to one of "base", "one" or "sub" to specify LDAP search depth (defaults to "sub").
 
 .. method:: LdapQuery.is_applicable(self, metadata)
-   
+
    You can override this method to indicate whether this query makes sense for a given
    set of metadata (e.g. you need a query only for a certain bundle or group).
-   
+
    (defaults to returning True)
-   
-.. method:: LdapQuery.prepare_query(self, metadata)
-   
+
+.. method:: LdapQuery.prepare_query(self, metadata, \**kwargs)
+
    Override this method to alter the query prior to execution. This is useful if your filter
    depends on the current metadata, e.g.::
-   
+
        self.filter = "(cn=" + metadata.hostname + ")"
-   
+
    (defaults to doing nothing)
 
-.. method:: LdapQuery.process_result(self, metadata)
-   
+.. method:: LdapQuery.process_result(self, metadata, \**kwargs)
+
    You will probably override this method in every query to reformat the results from LDAP.
    The raw result is stored in ``self.result``, you must return the altered data. Note that LDAP
    search results are presented in this structure::
-   
+
       (
           ("DN of first entry returned",
               {
                   "firstAttribute" : 1,
                   "secondAttribute" : 2,
-              } 
+              }
           ),
           ("DN of second entry returned",
               {
                   "firstAttribute" : 1,
                   "secondAttribute" : 2,
-              } 
+              }
           ),
       )
-   
+
    Therefore, to return just the value of the firstAttribute of the second object returned,
    you'd write::
-   
+
       return self.result[1][1][0]
-   
+
    (defaults to returning ``self.result`` unaltered)
 
-LdapSubQuery
-++++++++++++
-
-.. class:: LdapSubQuery
-
-   Sometimes you need more than one query to obtain the data you need (e.g. use the first
-   query to return all websites running on metadata.hostname and another query to find all
-   customers that should have access to those sites).
-   
-   LdapSubQueries are the same as LdapQueries, except for that the methods
-   
-   * ``get_result()``
-   * ``prepare_query()``
-   * ``process_result()``
-     
-   allow any additional keyword arguments that may contain additional data as needed. Note 
-   that ``get_result()`` will call ``prepare_query()`` and ``process_result()`` for you,
-   so you shouldn't ever need to invoke these yourself, just override them.
-   
-Here is another example that uses LdapSubQuery::
-
-	class WebSitesQuery(LdapSubQuery):
-	    name = "web_sites"
+.. method:: LdapQuery.get_result(self, metadata, \**kwargs)
+
+   This executes the query. First it will call ``prepare_query() for you, then it will try
+   to execute the query with the specified connection and last it will call ``process_result()``
+   and return that return value.
+
+If you use a LdapQuery class by yourself, you could pass additional keyword arguments to
+``get_result()``. It will call ``prepare_query()`` and ``process_result()`` for you and
+also supply this additional arguments to this methods.
+
+Here is an example::
+
+	__queries__ = ['WebPackageQuery']
+
+	class WebSitesQuery(LdapQuery):
 	    filter = "(objectClass=webHostingSite)"
 	    attrs = ["dc"]
 	    connection = conn_default
-	    
+
 	    def prepare_query(self, metadata, base_dn):
 	        self.base = base_dn
-	    
-	    def process_result(self, metadata):
+
+	    def process_result(self, metadata, **kwargs):
 	        [...] # build sites dict from returned dc attributes
 	        return sites
-	
-	@register_query
+
 	class WebPackagesQuery(LdapQuery):
-	    name = "web_packages"
 	    base = "dc=example,dc=com"
 	    attrs = ["customerId"]
 	    connection = conn_default
-	    
+
 	    def prepare_query(self, metadata):
 	        self.filter = "(&(objectClass=webHostingPackage)(cn:dn:=" + metadata.hostname + "))"
-	    
+
 	    def process_result(self, metadata):
 	        customers = {}
 	        for customer in self.result:
 	            dn = customer[0]
 	            cid = customer[1]["customerId"][0]
-	            customers[cid]["sites"] = WebSitesQuery().get_result(metadata, base_dn = dn)
+	            customers[cid]["sites"] = WebSitesQuery().get_result(metadata, base_dn=dn)
 	        return customers
 
 This example assumes that we have a number of webhosting packages that contain various
-sites. We need a first query ("web_packages") to get a list of the packages our customers
-have and another query for each of those to find out what sites are contained in each 
-package. The magic happens in the second class where ``WebSitesQuery.get_result()`` is 
-called with the additional ``base_dn`` parameter that allows our LdapSubQuery to only 
+sites. We need the ``WebPackagesQuery`` to get a list of the packages our customers
+have and another query for each of those to find out what sites are contained in each
+package. The magic happens in the second class where ``WebSitesQuery.get_result()`` is
+called with the additional ``base_dn`` parameter that allows our LdapQuery to only
 search below that DN.
 
-.. warning::
-    Do NOT apply the ``register_query`` decorator to LdapSubQueries.
+You do not need to add all LdapQueries to the ``__queries__`` list. Only add those to
+that list, that should be called automatically and whose results should be added to the
+client metadata.
 
 Known Issues
 ------------
 
 * At this point there is no support for SSL/TLS.
+* This module could not know, if a value changed on the LDAP server. So it could not
+  expire the client metadata cache sanely.
+  If you are using aggressive caching mode, this plugin will expire the metadata cache
+  for a single client at the start of a client run. If you are using LDAP data from
+  another client in a template, you will probably get the cached values from the last
+  client run of that other client.
diff --git a/man/bcfg2.conf.5 b/man/bcfg2.conf.5
index ba5158a0d..43a28bad0 100644
--- a/man/bcfg2.conf.5
+++ b/man/bcfg2.conf.5
@@ -286,7 +286,7 @@ a Mercurial version control backend. Currently, it enables you to get
 revision information out of your repository for reporting purposes.
 .SS Ldap Plugin
 .sp
-The Ldap plugin makes it possible to fetch data from an LDAP directory,
+The Ldap plugin makes it possible to fetch data from a LDAP directory,
 process it and attach it to your metadata.
 .SS Metadata Plugin
 .sp
diff --git a/src/lib/Bcfg2/Server/Plugins/Ldap.py b/src/lib/Bcfg2/Server/Plugins/Ldap.py
index 553ddbc47..66f317c20 100644
--- a/src/lib/Bcfg2/Server/Plugins/Ldap.py
+++ b/src/lib/Bcfg2/Server/Plugins/Ldap.py
@@ -1,120 +1,132 @@
+""" A plugin to fetch data from a LDAP directory """
+
 import imp
-import logging
+import os
 import sys
 import time
 import traceback
+import Bcfg2.Options
 import Bcfg2.Server.Plugin
-
-logger = logging.getLogger('Bcfg2.Plugins.Ldap')
+from Bcfg2.Logger import Debuggable
+from Bcfg2.Utils import ClassName, safe_module_name
 
 try:
     import ldap
+    HAS_LDAP = True
 except ImportError:
-    logger.error("Unable to load ldap module. Is python-ldap installed?")
-    raise ImportError
-
-# time in seconds between retries after failed LDAP connection
-RETRY_DELAY = 5
-# how many times to try reaching the LDAP server if a connection is broken
-# at the very minimum, one retry is needed to handle a restarted LDAP daemon
-RETRY_COUNT = 3
-
-SCOPE_MAP = {
-    "base": ldap.SCOPE_BASE,
-    "one": ldap.SCOPE_ONELEVEL,
-    "sub": ldap.SCOPE_SUBTREE,
-}
-
-LDAP_QUERIES = []
-
-
-def register_query(query):
-    LDAP_QUERIES.append(query)
+    HAS_LDAP = False
 
 
 class ConfigFile(Bcfg2.Server.Plugin.FileBacked):
-    """
-    Config file for the Ldap plugin
-
-    The config file cannot be 'parsed' in the traditional sense as we would
-    need some serious type checking ugliness to just get the LdapQuery
-    subclasses. The alternative would be to have the user create a list with
-    a predefined name that contains all queries.
-    The approach implemented here is having the user call a registering
-    decorator that updates a global variable in this module.
-    """
-    def __init__(self, filename):
-        self.filename = filename
-        Bcfg2.Server.Plugin.FileBacked.__init__(self, self.filename)
-        self.fam.AddMonitor(self.filename, self)
+    """ Config file for the Ldap plugin  """
+
+    def __init__(self, name, core):
+        Bcfg2.Server.Plugin.FileBacked.__init__(self, name)
+        self.core = core
+        self.queries = list()
+        self.fam.AddMonitor(name, self)
 
     def Index(self):
-        """
-        Reregisters the queries in the config file
+        """ Get the queries from the config file """
+        try:
+            module = imp.load_source(safe_module_name('Ldap', self.name),
+                                     self.name)
+        except:  # pylint: disable=W0702
+            err = sys.exc_info()[1]
+            self.logger.error("Ldap: Failed to import %s: %s" %
+                              (self.name, err))
+            return
+
+        if not hasattr(module, "__queries__"):
+            self.logger.error("Ldap: %s has no __queries__ list" % self.name)
+            return
+
+        self.queries = list()
+        for query in module.__queries__:
+            try:
+                self.queries.append(getattr(module, query))
+            except AttributeError:
+                self.logger.warning(
+                    "Ldap: %s exports %s, but has no such attribute" %
+                    (self.name, query))
 
-        The config will take care of actually registering the queries,
-        so we just load it once and don't keep it.
-        """
-        global LDAP_QUERIES
-        LDAP_QUERIES = []
-        imp.load_source("ldap_cfg", self.filename)
+        if self.core.metadata_cache_mode in ['cautious', 'aggressive']:
+            self.core.metadata_cache.expire()
 
 
-class Ldap(Bcfg2.Server.Plugin.Plugin, Bcfg2.Server.Plugin.Connector):
-    """
-    The Ldap plugin allows adding data from an LDAP server to your metadata.
-    """
-    name = "Ldap"
+class Ldap(Bcfg2.Server.Plugin.Plugin,
+           Bcfg2.Server.Plugin.ClientRunHooks,
+           Bcfg2.Server.Plugin.Connector):
+    """ The Ldap plugin allows adding data from an LDAP server
+    to your metadata. """
+
     experimental = True
-    debug_flag = False
+
+    options = [
+        Bcfg2.Options.Option(
+            cf=('ldap', 'retries'), type=int, default=3,
+            help='The number of times to retry reaching the '
+                 'LDAP server if a connection is broken'),
+        Bcfg2.Options.Option(
+            cf=('ldap', 'retry_delay'), type=float, default=5.0,
+            help='The time in seconds betreen retries')]
 
     def __init__(self, core):
         Bcfg2.Server.Plugin.Plugin.__init__(self, core)
         Bcfg2.Server.Plugin.Connector.__init__(self)
-        self.config = ConfigFile(self.data + "/config.py")
 
-    def debug_log(self, message, flag=None):
-        if (flag is None) and self.debug_flag or flag:
-            self.logger.error(message)
+        if not HAS_LDAP:
+            msg = "Python ldap module is required for Ldap plugin"
+            self.logger.error(msg)
+            raise Bcfg2.Server.Plugin.PluginInitError(msg)
+
+        self.config = ConfigFile(os.path.join(self.data, 'config.py'))
 
     def get_additional_data(self, metadata):
         query = None
         try:
             data = {}
-            self.debug_log("LdapPlugin debug: found queries " +
-                           str(LDAP_QUERIES))
-            for QueryClass in LDAP_QUERIES:
-                query = QueryClass()
+            self.debug_log("Found queries %s" % self.config.queries)
+            for query_class in self.config.queries:
+                query = query_class()
                 if query.is_applicable(metadata):
-                    self.debug_log("LdapPlugin debug: processing query '" +
-                                   query.name + "'")
+                    self.debug_log("Processing query '%s'" % query.name)
                     data[query.name] = query.get_result(metadata)
                 else:
-                    self.debug_log("LdapPlugin debug: query '" + query.name +
-                                   "' not applicable to host '" +
-                                   metadata.hostname + "'")
+                    self.debug_log("query '%s' not applicable to host '%s'" %
+                                   (query.name, metadata.hostname))
             return data
-        except Exception:
+        except:  # pylint: disable=W0702
             if hasattr(query, "name"):
-                logger.error("LdapPlugin error: " +
-                             "Exception during processing of query named '" +
-                             str(query.name) +
-                             "', query results will be empty" +
-                             " and may cause bind failures")
-            for line in traceback.format_exception(sys.exc_info()[0],
-                                                   sys.exc_info()[1],
-                                                   sys.exc_info()[2]):
-                logger.error("LdapPlugin error: " +
-                             line.replace("\n", ""))
+                self.logger.error(
+                    "Exception during processing of query named '%s', query "
+                    "results will be empty and may cause bind failures" %
+                    query.name)
+            for line in traceback.format_exc().split('\n'):
+                self.logger.error(line)
             return {}
 
+    def start_client_run(self, metadata):
+        if self.core.metadata_cache_mode == 'aggressive':
+            self.logger.warning("Ldap is incompatible with aggressive "
+                                "client metadata caching, try 'cautious' "
+                                "or 'initial'")
+        self.core.metadata_cache.expire(metadata.hostname)
+
+
+class LdapConnection(Debuggable):
+    """ Connection to an LDAP server. """
+
+    __scopes__ = {
+        'base': ldap.SCOPE_BASE,
+        'one': ldap.SCOPE_ONELEVEL,
+        'sub': ldap.SCOPE_SUBTREE,
+    }
+
+    def __init__(self, host="localhost", port=389, binddn=None,
+                 bindpw=None):
+        Debuggable.__init__(self)
 
-class LdapConnection(object):
-    """
-    Connection to an LDAP server.
-    """
-    def __init__(self, host="localhost", port=389,
-                 binddn=None, bindpw=None):
         self.host = host
         self.port = port
         self.binddn = binddn
@@ -122,48 +134,62 @@ class LdapConnection(object):
         self.conn = None
 
     def __del__(self):
+        """ Disconnection if the instance is destroyed. """
+        self.disconnect()
+
+    def disconnect(self):
+        """ If a connection to an LDAP server is available, disconnect it. """
         if self.conn:
-            self.conn.unbind()
+            self.conn.unbund()
+            self.conn = None
 
-    def init_conn(self):
+    def connect(self):
+        """ Open a connection to the configured LDAP server, and do a simple
+        bind ff both binddn and bindpw are set. """
+        self.disconnect()
         self.conn = ldap.initialize(self.url)
         if self.binddn is not None and self.bindpw is not None:
             self.conn.simple_bind_s(self.binddn, self.bindpw)
 
     def run_query(self, query):
-        result = None
-        for attempt in range(RETRY_COUNT + 1):
-            if attempt >= 1:
-                logger.error("LdapPlugin error: " +
-                             "LDAP server down (retry " + str(attempt) + "/" +
-                             str(RETRY_COUNT) + ")")
+        """ Connect to the server and execute the query. If the server is
+        down, wait the configured amount and try to reconnect.
+
+        :param query: The query to execute on the LDAP server.
+        :type query: Bcfg.Server.Plugins.Ldap.LdapQuery
+        """
+        for attempt in range(Bcfg2.Options.setup.ldap_retries + 1):
             try:
                 if not self.conn:
-                    self.init_conn()
-                result = self.conn.search_s(
-                    query.base,
-                    SCOPE_MAP[query.scope],
-                    query.filter.replace("\\", "\\\\"),
-                    query.attrs,
-                )
-                break
+                    self.connect()
+
+                return self.conn.search_s(
+                    query.base, self.__scopes__[query.scope],
+                    query.filter.replace('\\', '\\\\'), query.attrs)
+
             except ldap.SERVER_DOWN:
                 self.conn = None
-                time.sleep(RETRY_DELAY)
-        return result
+                self.logger.error(
+                    "LdapConnection: Server %s down. Retry %d/%d in %.2fs." %
+                    (self.url, attempt + 1, Bcfg2.Options.setup.ldap_retries,
+                     Bcfg2.Options.setup.ldap_retry_delay))
+                time.sleep(Bcfg2.Options.setup.ldap_retry_delay)
+
+        return None
 
     @property
     def url(self):
-        return "ldap://" + self.host + ":" + str(self.port)
+        """ The URL of the LDAP server. """
+        return "ldap://%s:%d" % (self.host, self.port)
 
 
 class LdapQuery(object):
-    """
-    Query referencing an LdapConnection and providing several
-    methods for query manipulation.
-    """
+    """ Query referencing an LdapConnection and providing several
+    methods for query manipulation. """
+
+    #: Name of the Query, used to register it in additional data.
+    name = ClassName()
 
-    name = "unknown"
     base = ""
     scope = "sub"
     filter = "(objectClass=*)"
@@ -172,80 +198,48 @@ class LdapQuery(object):
     result = None
 
     def __unicode__(self):
-        return "LdapQuery:" + self.name
+        return "LdapQuery: %s" % self.name
 
-    def is_applicable(self, metadata):
-        """
-        Overrideable method to determine if the query is to be executed for
-        the given metadata object.
-        Defaults to true.
-        """
-        return True
+    def is_applicable(self, metadata):  # pylint: disable=W0613
+        """ Check is the query should be executed for a given metadata
+        object.
 
-    def prepare_query(self, metadata):
+        :param metadata: The client metadata
+        :type metadata: Bcfg2.Server.Plugins.Metadata.ClientMetadata
         """
-        Overrideable method to alter the query based on metadata.
-        Defaults to doing nothing.
-
-        In most cases, you will do something like
+        return True
 
-            self.filter = "(cn=" + metadata.hostname + ")"
+    def prepare_query(self, metadata, **kwargs):  # pylint: disable=W0613
+        """ Prepares the query based on the client metadata. You can
+        for example modify the filter based on the client hostname.
 
-        here.
+        :param metadata: The client metadata
+        :type metadata: Bcfg2.Server.Plugins.Metadata.ClientMetadata
         """
         pass
 
-    def process_result(self, metadata):
-        """
-        Overrideable method to post-process the query result.
-        Defaults to returning the unaltered result.
-        """
-        return self.result
-
-    def get_result(self, metadata):
-        """
-        Method to handle preparing, executing and processing the query.
-        """
-        if isinstance(self.connection, LdapConnection):
-            self.prepare_query(metadata)
-            self.result = self.connection.run_query(self)
-            self.result = self.process_result(metadata)
-            return self.result
-        else:
-            logger.error("LdapPlugin error: " +
-                         "No valid connection defined for query " + str(self))
-            return None
-
-
-class LdapSubQuery(LdapQuery):
-    """
-    SubQueries are meant for internal use only and are not added
-    to the metadata object. They are useful for situations where
-    you need to run more than one query to obtain some data.
-    """
-    def prepare_query(self, metadata, **kwargs):
-        """
-        Overrideable method to alter the query based on metadata.
-        Defaults to doing nothing.
-        """
-        pass
+    def process_result(self, metadata, **kwargs):  # pylint: disable=W0613
+        """ Post-process the query result.
 
-    def process_result(self, metadata, **kwargs):
-        """
-        Overrideable method to post-process the query result.
-        Defaults to returning the unaltered result.
+        :param metadata: The client metadata
+        :type metadata: Bcfg2.Server.Plugins.Metadata.ClientMetadata
         """
         return self.result
 
     def get_result(self, metadata, **kwargs):
+        """ Handle the perparation, execution and processing of the query.
+
+        :param metadata: The client metadata
+        :type metadata: Bcfg2.Server.Plugins.Metadata.ClientMetadata
+        :raises: :class:`Bcfg2.Server.Plugin.exceptions.PluginExecutionError`
         """
-        Method to handle preparing, executing and processing the query.
-        """
-        if isinstance(self.connection, LdapConnection):
+
+        if self.connection is not None:
             self.prepare_query(metadata, **kwargs)
             self.result = self.connection.run_query(self)
-            return self.process_result(metadata, **kwargs)
+            self.result = self.process_result(metadata, **kwargs)
         else:
-            logger.error("LdapPlugin error: " +
-                         "No valid connection defined for query " + str(self))
-            return None
+            raise Bcfg2.Server.Plugin.PluginExecutionError(
+                'No connection defined for %s' % self.name)
+
+        return self.result