Merge branch 'feature/ldap-enhancements' of https://github.com/AlexanderS/bcfg2

6 files changed, 235 insertions, 100 deletions

diff --git a/doc/development/caching.txt b/doc/development/caching.txt
index 83ec0290f..c8b7aba14 100644
--- a/doc/development/caching.txt
+++ b/doc/development/caching.txt
@@ -67,6 +67,9 @@ Currently known caches are:
 | pkg_sets    | <Collection.cachekey>`,               |                                                 | for clients                                          |
 |             | hash of the initial package selection |                                                 |                                                      |
 +-------------+---------------------------------------+-------------------------------------------------+------------------------------------------------------+
+| Ldap,       | Hostname, ``<query name>``            | :func:`processed result of the query            | Cached results from the Ldap queries                 |
+| results,    |                                       | <Bcfg2.Server.Plugins.LdapQuery.process_result>`|                                                      |
++-------------+---------------------------------------+-------------------------------------------------+------------------------------------------------------+
 
 These are enumerated so that they can be expired as needed by other
 plugins or other code points.
diff --git a/doc/server/plugins/grouping/ldap.txt b/doc/server/plugins/grouping/ldap.txt
index af18680d2..abbd5e005 100644
--- a/doc/server/plugins/grouping/ldap.txt
+++ b/doc/server/plugins/grouping/ldap.txt
@@ -7,7 +7,7 @@ Ldap
 ====
 
 .. warning::
-    This plugin is considered experimental and has known issues (see below).
+    This plugin is considered experimental.
 
 Purpose
 -------
@@ -87,6 +87,26 @@ If you wish, you could customize these values in your ``bcfg2.conf``::
     retries = 3
     retry_delay = 3.0
 
+Caching
++++++++
+
+This module could not know, if a value changed on the LDAP server. So it does not cache
+the results of the LDAP queries by default.
+
+You could enable the cache of the results in your ``bcfg2.conf``:
+
+    [ldap]
+    cache = on
+
+If you enable the caching, you have to expire it manually. This module provides a XML-RPC
+method for this purpose: :func:`Ldap.expire_cache
+<Bcfg2.Server.Plugins.Ldap.expire_cache>`.
+
+Even without enabling caching, the results of the LDAP queries are cached, but are
+discarded before each client run. If you access the Ldap results of different client, you
+may get cached results of the last run of this client. If you do not want this behaviour,
+you can disable the caching completely by setting it to ``off``.
+
 Class reference
 ---------------
 
@@ -95,8 +115,8 @@ LdapConnection
 
 .. class:: LdapConnection
 
-   This class represents an LDAP connection. Every query must be associated with exactly
-   one connection.
+   This class represents an LDAP connection. Every query must be associated
+   with exactly one connection.
 
 .. attribute:: LdapConnection.binddn
 
@@ -112,7 +132,24 @@ LdapConnection
 
 .. attribute:: LdapConnection.port
 
-   Port where LDAP server is listening (defaults to 389).
+   Port where LDAP server is listening (defaults to 389). If you use
+   port 636 this module will use ldaps to connect to the server.
+
+.. attribute:: LdapConnection.uri
+
+   LDAP URI of the LDAP server to connect to. This is prefered over
+   :attr:`LdapConnection.host` and :attr:`LdapConnection.port`.
+
+   .. note::
+
+      If you are using ldaps you may have to specify additional options
+      for enabling the certificate validation or setting the path for
+      the trusted certificates with :attr:`LdapConnection.options`.
+
+.. attribute:: LdapConnection.options
+
+   Arbitrary options for the LDAP connection. You should specify it
+   as a dict and use the ``OPT_*`` constants from ``python-ldap``.
 
 You may pass any of these attributes as keyword arguments when creating the connection object.
 
@@ -246,14 +283,3 @@ search below that DN.
 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/src/lib/Bcfg2/Server/Cache.py b/src/lib/Bcfg2/Server/Cache.py
index d05eb0bf6..b3b906b2c 100644
--- a/src/lib/Bcfg2/Server/Cache.py
+++ b/src/lib/Bcfg2/Server/Cache.py
@@ -96,15 +96,19 @@ class _Cache(MutableMapping):
         return len(list(iter(self)))
 
     def expire(self, key=None):
-        """ expire all items, or a specific item, from the cache """
+        """ expire all items, or a specific item, from the cache
+
+        :returns: number of expired entries
+        """
+
         if key is None:
-            expire(*self._tags)
+            return expire(*self._tags)
         else:
             tags = self._tags | set([key])
             # py 2.5 doesn't support mixing *args and explicit keyword
             # args
             kwargs = dict(exact=True)
-            expire(*tags, **kwargs)
+            return expire(*tags, **kwargs)
 
     def __repr__(self):
         return repr(dict(self))
@@ -152,7 +156,10 @@ def expire(*tags, **kwargs):
     """ Expire all items, a set of items, or one specific item from
     the cache.  If ``exact`` is set to True, then if the given tag set
     doesn't match exactly one item in the cache, nothing will be
-    expired. """
+    expired.
+
+    :returns: number of expired entries
+    """
     exact = kwargs.pop("exact", False)
     count = 0
     if not tags:
@@ -170,6 +177,8 @@ def expire(*tags, **kwargs):
     for hook in _hooks:
         hook(tags, exact, count)
 
+    return count
+
 
 def add_expire_hook(func):
     """ Add a hook that will be called when an item is expired from
diff --git a/src/lib/Bcfg2/Server/Plugin/helpers.py b/src/lib/Bcfg2/Server/Plugin/helpers.py
index 6b521dfd6..762d018eb 100644
--- a/src/lib/Bcfg2/Server/Plugin/helpers.py
+++ b/src/lib/Bcfg2/Server/Plugin/helpers.py
@@ -13,7 +13,7 @@ import Bcfg2.Server
 import Bcfg2.Options
 import Bcfg2.Server.FileMonitor
 from Bcfg2.Logger import Debuggable
-from Bcfg2.Compat import CmpMixin, wraps
+from Bcfg2.Compat import CmpMixin, MutableMapping, wraps
 from Bcfg2.Server.Plugin.base import Plugin
 from Bcfg2.Server.Plugin.interfaces import Generator, TemplateDataProvider
 from Bcfg2.Server.Plugin.exceptions import SpecificityError, \
@@ -1698,3 +1698,84 @@ class GroupSpool(Plugin, Generator):
                 return
             reqid = self.fam.AddMonitor(name, self)
             self.handles[reqid] = relative
+
+
+class CallableDict(MutableMapping):
+    """ This maps a set of keys to a set of value-getting functions;
+    the values are populated on-the-fly by the functions as the values
+    are needed (and not before).  This is for example used by
+    :func:`Bcfg2.Server.Plugins.Packages.Packages.get_additional_data`;
+    see the docstring for that function for details on why.
+
+    Unlike a dict, you can specify values or functions for the
+    righthand side of this mapping. If you specify a function, it will
+    be evaluated everytime you access the value. E.g.:
+
+    .. code-block:: python
+
+        d = CallableDict(foo=load_foo,
+                         bar="bar")
+    """
+
+    def __init__(self, **getters):
+        self._getters = dict(**getters)
+
+    def __getitem__(self, key):
+        if callable(self._getters[key]):
+            return self._getters[key]()
+        else:
+            return self._getters[key]
+
+    def __setitem__(self, key, getter):
+        self._getters[key] = getter
+
+    def __delitem__(self, key):
+        del self._getters[key]
+
+    def __len__(self):
+        return len(self._getters)
+
+    def __iter__(self):
+        return iter(self._getters.keys())
+
+    def _current_data(self):
+        """ Return a dict with the current available static data
+        and ``unknown`` for all callable values.
+        """
+        rv = dict()
+        for key in self._getters.keys():
+            if callable(self._getters[key]):
+                rv[key] = 'unknown'
+            else:
+                rv[key] = self._getters[key]
+        return rv
+
+    def __repr__(self):
+        return str(self._current_data())
+
+
+class OnDemandDict(CallableDict):
+    """ This is like a :class:`CallableDict` but it will cache
+    the results of the callable getters, so that it is only evaluated
+    once when you first access it.
+    """
+
+    def __init__(self, **getters):
+        CallableDict.__init__(self, **getters)
+        self._values = dict()
+
+    def __getitem__(self, key):
+        if key not in self._values:
+            self._values[key] = super(OnDemandDict, self).__getitem__(key)
+        return self._values[key]
+
+    def __delitem__(self, key):
+        super(OnDemandDict, self).__delitem__(key)
+        del self._values[key]
+
+    def _current_data(self):
+        rv = super(OnDemandDict, self)._current_data()
+        for (key, value) in rv.items():
+            if key in self._values:
+                rv[key] = value
+        return rv
diff --git a/src/lib/Bcfg2/Server/Plugins/Ldap.py b/src/lib/Bcfg2/Server/Plugins/Ldap.py
index 757150300..770419ba5 100644
--- a/src/lib/Bcfg2/Server/Plugins/Ldap.py
+++ b/src/lib/Bcfg2/Server/Plugins/Ldap.py
@@ -5,7 +5,10 @@ import os
 import sys
 import time
 import traceback
+from functools import partial
+
 import Bcfg2.Options
+import Bcfg2.Server.Cache
 import Bcfg2.Server.Plugin
 from Bcfg2.Logger import Debuggable
 from Bcfg2.Utils import ClassName, safe_module_name
@@ -20,16 +23,18 @@ except ImportError:
 class ConfigFile(Bcfg2.Server.Plugin.FileBacked):
     """ Config file for the Ldap plugin  """
 
-    def __init__(self, name, core):
+    def __init__(self, name, core, plugin):
         Bcfg2.Server.Plugin.FileBacked.__init__(self, name)
         self.core = core
+        self.plugin = plugin
         self.queries = list()
         self.fam.AddMonitor(name, self)
 
     def Index(self):
         """ Get the queries from the config file """
         try:
-            module = imp.load_source(safe_module_name('Ldap', self.name),
+            module_name = os.path.splitext(os.path.basename(self.name))[0]
+            module = imp.load_source(safe_module_name('Ldap', module_name),
                                      self.name)
         except:  # pylint: disable=W0702
             err = sys.exc_info()[1]
@@ -53,12 +58,15 @@ class ConfigFile(Bcfg2.Server.Plugin.FileBacked):
         if self.core.metadata_cache_mode in ['cautious', 'aggressive']:
             self.core.metadata_cache.expire()
 
+        self.plugin.expire_cache()
+
 
 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. """
+    __rmi__ = Bcfg2.Server.Plugin.Plugin.__rmi__ + ['expire_cache']
 
     experimental = True
 
@@ -71,7 +79,11 @@ class Ldap(Bcfg2.Server.Plugin.Plugin,
         Bcfg2.Options.Option(
             cf=('ldap', 'retry_delay'), type=float, default=5.0,
             dest='ldap_retry_delay',
-            help='The time in seconds betreen retries')]
+            help='The time in seconds betreen retries'),
+        Bcfg2.Options.BooleanOption(
+            cf=('ldap', 'cache'), default=None, dest='ldap_cache',
+            help='Cache the results of the LDAP Queries until they '
+                 'are expired using the XML-RPC RMI')]
 
     def __init__(self, core):
         Bcfg2.Server.Plugin.Plugin.__init__(self, core)
@@ -82,45 +94,84 @@ class Ldap(Bcfg2.Server.Plugin.Plugin,
             self.logger.error(msg)
             raise Bcfg2.Server.Plugin.PluginInitError(msg)
 
-        self.config = ConfigFile(os.path.join(self.data, 'config.py'))
+        self.config = ConfigFile(os.path.join(self.data, 'config.py'),
+                                 core, self)
+        self._hosts = dict()
+
+    def _cache(self, query_name):
+        """ Return the :class:`Cache <Bcfg2.Server.Cache>` for the
+        given query name. """
+        return Bcfg2.Server.Cache.Cache('Ldap', 'results', query_name)
+
+    def _execute_query(self, query, metadata):
+        """ Return the cached result of the given query for this host or
+        execute the given query and cache the result. """
+        result = None
+
+        if Bcfg2.Options.setup.ldap_cache is not False:
+            cache = self._cache(query.name)
+            result = cache.get(metadata.hostname, None)
+
+        if result is None:
+            try:
+                self.debug_log("Processing query '%s'" % query.name)
+                result = query.get_result(metadata)
+                if Bcfg2.Options.setup.ldap_cache is not False:
+                    cache[metadata.hostname] = result
+            except:  # pylint: disable=W0702
+                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 result
 
     def get_additional_data(self, metadata):
-        query = None
-        try:
-            data = {}
-            self.debug_log("Found queries %s" % self.config.queries)
-            for query_class in self.config.queries:
+        data = {}
+        self.debug_log("Found queries %s" % self.config.queries)
+        for query_class in self.config.queries:
+            try:
                 query = query_class()
                 if query.is_applicable(metadata):
                     self.debug_log("Processing query '%s'" % query.name)
-                    data[query.name] = query.get_result(metadata)
+                    data[query.name] = partial(
+                        self._execute_query, query, metadata)
                 else:
                     self.debug_log("query '%s' not applicable to host '%s'" %
                                    (query.name, metadata.hostname))
-            return data
-        except:  # pylint: disable=W0702
-            if hasattr(query, "name"):
+            except:  # pylint: disable=W0702
                 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 {}
+                    "Exception during preparation of query named '%s'. "
+                    "Query will be ignored." % query_class.__name__)
+                for line in traceback.format_exc().split('\n'):
+                    self.logger.error(line)
+
+        return Bcfg2.Server.Plugin.CallableDict(**data)
 
     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)
+        if Bcfg2.Options.setup.ldap_cache is None:
+            self.expire_cache(hostname=metadata.hostname)
+
+    def expire_cache(self, query=None, hostname=None):
+        """ Expire the cache. You can select the items to purge
+        per query and/or per host, or you can purge all cached
+        data. This is exposed as an XML-RPC RMI. """
+
+        tags = ['Ldap', 'results']
+        if query:
+            tags.append(query)
+        if hostname:
+            tags.append(hostname)
+
+        return Bcfg2.Server.Cache.expire(*tags)
 
 
 class LdapConnection(Debuggable):
     """ Connection to an LDAP server. """
 
-    def __init__(self, host="localhost", port=389, binddn=None,
-                 bindpw=None):
+    def __init__(self, host="localhost", port=389, uri=None, options=None,
+                 binddn=None, bindpw=None):
         Debuggable.__init__(self)
 
         if HAS_LDAP:
@@ -130,6 +181,8 @@ class LdapConnection(Debuggable):
 
         self.host = host
         self.port = port
+        self.uri = uri
+        self.options = options
         self.binddn = binddn
         self.bindpw = bindpw
         self.conn = None
@@ -154,7 +207,12 @@ class LdapConnection(Debuggable):
         """ 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)
+        self.conn = ldap.initialize(self.get_uri())
+
+        if self.options is not None:
+            for (option, value) in self.options.items():
+                self.conn.set_option(option, value)
+
         if self.binddn is not None and self.bindpw is not None:
             self.conn.simple_bind_s(self.binddn, self.bindpw)
 
@@ -178,16 +236,20 @@ class LdapConnection(Debuggable):
                 self.conn = None
                 self.logger.error(
                     "LdapConnection: Server %s down. Retry %d/%d in %.2fs." %
-                    (self.url, attempt + 1, Bcfg2.Options.setup.ldap_retries,
+                    (self.get_uri(), 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):
+    def get_uri(self):
         """ The URL of the LDAP server. """
-        return "ldap://%s:%d" % (self.host, self.port)
+        if self.uri is None:
+            if self.port == 636:
+                return "ldaps://%s" % self.host
+            return "ldap://%s:%d" % (self.host, self.port)
+        return self.uri
 
 
 class LdapQuery(object):
diff --git a/src/lib/Bcfg2/Server/Plugins/Packages/__init__.py b/src/lib/Bcfg2/Server/Plugins/Packages/__init__.py
index 3aa5c415f..23ccd7b8e 100644
--- a/src/lib/Bcfg2/Server/Plugins/Packages/__init__.py
+++ b/src/lib/Bcfg2/Server/Plugins/Packages/__init__.py
@@ -10,7 +10,7 @@ import lxml.etree
 import Bcfg2.Options
 import Bcfg2.Server.Cache
 import Bcfg2.Server.Plugin
-from Bcfg2.Compat import urlopen, HTTPError, URLError, MutableMapping
+from Bcfg2.Compat import urlopen, HTTPError, URLError
 from Bcfg2.Server.Plugins.Packages.Collection import Collection, \
     get_collection_class
 from Bcfg2.Server.Plugins.Packages.PackagesSources import PackagesSources
@@ -36,52 +36,6 @@ class PackagesBackendAction(Bcfg2.Options.ComponentAction):
     fail_silently = True
 
 
-class OnDemandDict(MutableMapping):
-    """ This maps a set of keys to a set of value-getting functions;
-    the values are populated on-the-fly by the functions as the values
-    are needed (and not before).  This is used by
-    :func:`Bcfg2.Server.Plugins.Packages.Packages.get_additional_data`;
-    see the docstring for that function for details on why.
-
-    Unlike a dict, you should not specify values for for the righthand
-    side of this mapping, but functions that get values.  E.g.:
-
-    .. code-block:: python
-
-        d = OnDemandDict(foo=load_foo,
-                         bar=lambda: "bar");
-    """
-
-    def __init__(self, **getters):
-        self._values = dict()
-        self._getters = dict(**getters)
-
-    def __getitem__(self, key):
-        if key not in self._values:
-            self._values[key] = self._getters[key]()
-        return self._values[key]
-
-    def __setitem__(self, key, getter):
-        self._getters[key] = getter
-
-    def __delitem__(self, key):
-        del self._values[key]
-        del self._getters[key]
-
-    def __len__(self):
-        return len(self._getters)
-
-    def __iter__(self):
-        return iter(self._getters.keys())
-
-    def __repr__(self):
-        rv = dict(self._values)
-        for key in self._getters.keys():
-            if key not in rv:
-                rv[key] = 'unknown'
-        return str(rv)
-
-
 class Packages(Bcfg2.Server.Plugin.Plugin,
                Bcfg2.Server.Plugin.StructureValidator,
                Bcfg2.Server.Plugin.Generator,
@@ -578,7 +532,7 @@ class Packages(Bcfg2.Server.Plugin.Plugin,
 
     def get_additional_data(self, metadata):
         """ Return additional data for the given client.  This will be
-        an :class:`Bcfg2.Server.Plugins.Packages.OnDemandDict`
+        an :class:`Bcfg2.Server.Plugin.OnDemandDict`
         containing two keys:
 
         * ``sources``, whose value is a list of data returned from
@@ -610,7 +564,7 @@ class Packages(Bcfg2.Server.Plugin.Plugin,
             get_collection() until it's absolutely necessary. """
             return self.get_collection(metadata).get_additional_data()
 
-        return OnDemandDict(
+        return Bcfg2.Server.Plugin.OnDemandDict(
             sources=get_sources,
             get_config=lambda: self.get_config)