From a4675f6f503b153d5adfcfae8c5c1a7a67bdb692 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Fri, 26 Mar 2021 12:39:20 -0400 Subject: [PATCH] Implement simple LDAP cache layer Insert a class before LDAPClient to cache the return value of get_entry() and certain exceptions (NotFound and EmptyResult). The cache uses an OrderedDict for the cases where a large cache might result an LRU model can be used. The cache be enabled (default) or disabled using ldap_cache=True/False. This cache is per-request so is not expected to grow particularly large except in the case of a large batch command. The key to the cache entry is the dn of the object being requested. Any write to or referencing a cached dn is evicted from the cache. The set of attributes is somewhat taken into consideration. "*" does not always match everything being asked for by a plugin so unless the requested set of attributes is a direct subset of what is cached it will be re-fetched. Err on the side of safety. Despite this rather conserative approach to caching 29% of queries are saved with ipatests/xmlrpc_tests/* https://pagure.io/freeipa/issue/8798 Signed-off-by: Rob Crittenden Reviewed-By: Rafael Guterres Jeffman --- ipalib/constants.py | 3 + ipapython/ipaldap.py | 239 ++++++++++++++++++++++++++++++++++++- ipaserver/plugins/ldap2.py | 31 +++-- 3 files changed, 260 insertions(+), 13 deletions(-) diff --git a/ipalib/constants.py b/ipalib/constants.py index a622f640b..79ea36f08 100644 --- a/ipalib/constants.py +++ b/ipalib/constants.py @@ -160,6 +160,9 @@ DEFAULT_CONFIG = ( ('rpc_protocol', 'jsonrpc'), + ('ldap_cache', True), + ('ldap_cache_size', 100), + # Define an inclusive range of SSL/TLS version support ('tls_version_min', TLS_VERSION_DEFAULT_MIN), ('tls_version_max', TLS_VERSION_DEFAULT_MAX), diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py index 394cd92e4..c9b54ff79 100644 --- a/ipapython/ipaldap.py +++ b/ipapython/ipaldap.py @@ -31,6 +31,8 @@ import os import pwd import warnings +from collections import OrderedDict + from cryptography import x509 as crypto_x509 from cryptography.hazmat.primitives import serialization @@ -47,7 +49,7 @@ from ipalib.constants import LDAP_GENERALIZED_TIME_FORMAT # pylint: enable=ipa-forbidden-import from ipaplatform.paths import paths from ipapython.ipautil import format_netloc, CIDict -from ipapython.dn import DN +from ipapython.dn import DN, RDN from ipapython.dnsutil import DNSName from ipapython.kerberos import Principal @@ -1698,6 +1700,7 @@ class LDAPClient: modlist = entry.generate_modlist() if not modlist: raise errors.EmptyModlist() + logger.debug("update_entry modlist %s", modlist) # pass arguments to python-ldap with self.error_handler(): @@ -1749,3 +1752,237 @@ def get_ldap_uri(host='', port=389, cacert=None, ldapi=False, realm=None, return 'ldap://%s' % format_netloc(host, port) else: raise ValueError('Protocol %r not supported' % protocol) + + +class CacheEntry: + def __init__(self, entry=None, attrs_list=None, exception=None, + get_effective_rights=False, all=False): + self.entry = entry + self.attrs_list = attrs_list + self.exception = exception + self.all = all + + +class LDAPCache(LDAPClient): + """A very basic LRU Cache using an OrderedDict""" + + def __init__(self, ldap_uri, start_tls=False, force_schema_updates=False, + no_schema=False, decode_attrs=True, cacert=None, + sasl_nocanon=True, enable_cache=True, cache_size=100): + + self.cache = OrderedDict() + self._enable_cache = True # initialize to zero to satisfy pylint + + object.__setattr__(self, '_cache_misses', 0) + object.__setattr__(self, '_cache_hits', 0) + object.__setattr__(self, '_enable_cache', + enable_cache and cache_size > 0) + object.__setattr__(self, '_cache_size', cache_size) + + super(LDAPCache, self).__init__( + ldap_uri, start_tls, force_schema_updates, no_schema, + decode_attrs, cacert, sasl_nocanon + ) + + @property + def hit(self): + return self._cache_hits # pylint: disable=no-member + + @property + def miss(self): + return self._cache_misses # pylint: disable=no-member + + @property + def max_entries(self): + return self._cache_size # pylint: disable=no-member + + def emit(self, msg, *args, **kwargs): + if self._enable_cache: + logger.debug(msg, *args, **kwargs) + + def add_cache_entry(self, dn, attrs_list=None, get_all=False, + entry=None, exception=None): + # idnsname - caching prevents delete when mod value to None + # cospriority - in a Class of Service object, uncacheable + # TODO - usercertificate was banned at one point and I don't remember + # why... + BANNED_ATTRS = {'idnsname', 'cospriority'} + if not self._enable_cache: + return + + self.remove_cache_entry(dn) + + if ( + DN('cn=config') in dn + or DN('cn=kerberos') in dn + or DN('o=ipaca') in dn + ): + return + + if exception: + self.emit("EXC: Caching exception %s", exception) + self.cache[dn] = CacheEntry(exception=exception) + else: + if not BANNED_ATTRS.intersection(attrs_list): + self.cache[dn] = CacheEntry( + entry=entry.copy(), + attrs_list=deepcopy(attrs_list), + all=get_all, + ) + else: + return + + self.cache.move_to_end(dn) + + if len(self.cache) > self.max_entries: + (dn, entry) = self.cache.popitem(last=False) + self.emit("LRU: removed %s", dn) + + def clear_cache(self): + self.cache_status('FINAL') + object.__setattr__(self, 'cache', OrderedDict()) + object.__setattr__(self, '_cache_hits', 0) + object.__setattr__(self, '_cache_misses', 0) + + def cache_status(self, type): + self.emit("%s: Hits %d Misses %d Size %d", + type, self.hit, self.miss, len(self.cache)) + + def remove_cache_entry(self, dn): + assert isinstance(dn, DN) + self.emit('DROP: %s', dn) + if dn in self.cache: + del self.cache[dn] + else: + self.emit('DROP: not in cache %s', dn) + + # Begin LDAPClient methods + + def add_entry(self, entry): + self.emit('add_entry') + self.remove_cache_entry(entry.dn) + super(LDAPCache, self).add_entry(entry) + + def update_entry(self, entry): + self.emit('update_entry') + self.remove_cache_entry(entry.dn) + super(LDAPCache, self).update_entry(entry) + + def delete_entry(self, entry_or_dn): + self.emit('delete_entry') + if isinstance(entry_or_dn, DN): + dn = entry_or_dn + else: + dn = entry_or_dn.dn + + self.remove_cache_entry(dn) + + super(LDAPCache, self).delete_entry(dn) + + def move_entry(self, dn, new_dn, del_old=True): + self.emit('move_entry') + self.remove_cache_entry(dn) + self.remove_cache_entry(new_dn) + + super(LDAPCache, self).move_entry(dn, new_dn, del_old) + + def modify_s(self, dn, modlist): + self.emit('modify_s') + if not isinstance(dn, DN): + dn = DN(dn) + + self.emit('modlist %s', modlist) + for (_op, attr, mod_dn) in modlist: + if attr.lower() in ('member', + 'ipaallowedtoperform_write_keys', + 'managedby_host'): + for d in mod_dn: + if not isinstance(d, (DN, RDN)): + d = DN(d.decode('utf-8')) + self.emit('modify_s %s', d) + self.remove_cache_entry(d) + + self.emit('modify_s %s', dn) + self.remove_cache_entry(dn) + + return super(LDAPCache, self).modify_s(dn, modlist) + + def get_entry(self, dn, attrs_list=None, time_limit=None, + size_limit=None, get_effective_rights=False): + # pylint: disable=no-member + if not self._enable_cache: + return super(LDAPCache, self).get_entry( + dn, attrs_list, time_limit, size_limit, get_effective_rights + ) + self.emit("Cache lookup: %s", dn) + + entry = self.cache.get(dn) + if get_effective_rights and entry: + # We don't cache this so do the query but don't drop the + # entry. + entry = None + + if entry and entry.exception: + hits = self._cache_hits + 1 # pylint: disable=no-member + object.__setattr__(self, '_cache_hits', hits) + self.emit("HIT: Re-raising %s", entry.exception) + self.cache_status('HIT') + raise entry.exception + + self.emit("Requested attrs_list %s", attrs_list) + if entry: + self.emit("Cached attrs_list %s", entry.attrs_list) + + if not attrs_list: + attrs_list = ['*'] + elif attrs_list == ['']: + attrs_list = ['dn'] + + get_all = False + if ( + entry + and (attrs_list in (['*'], [''])) + and (entry.attrs_list in (['*'], [''])) + ): + get_all = True + + if entry and entry.all and get_all: + # self.hit # pylint: disable=pointless-statement + hits = self._cache_hits + 1 # pylint: disable=no-member + object.__setattr__(self, '_cache_hits', hits) + self.cache_status('HIT') + return entry.entry + + # Be sure we have all the requested attributes before returning + # a cached entry. + if entry and attrs_list: + req_attrs = set(attr.lower() for attr in set(attrs_list)) + cache_attrs = set(attr.lower() for attr in entry.attrs_list) + if (req_attrs.issubset(cache_attrs)): + hits = self._cache_hits + 1 # pylint: disable=no-member + object.__setattr__(self, '_cache_hits', hits) + self.cache_status('HIT') + return entry.entry + + try: + entry = super(LDAPCache, self).get_entry( + dn, attrs_list, time_limit, size_limit, get_effective_rights + ) + except (errors.NotFound, errors.EmptyResult) as e: + # only cache these exceptions + self.add_cache_entry(dn, exception=e) + misses = self._cache_misses + 1 # pylint: disable=no-member + object.__setattr__(self, '_cache_misses', misses) + self.cache_status('MISS: %s' % e) + raise + # pylint: disable=try-except-raise + except Exception: + # re-raise anything we aren't caching + raise + else: + self.add_cache_entry(dn, attrs_list=attrs_list, get_all=get_all, + entry=entry) + misses = self._cache_misses + 1 # pylint: disable=no-member + object.__setattr__(self, '_cache_misses', misses) + self.cache_status('MISS') + return entry diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py index 8fb8ab496..095e74b2a 100644 --- a/ipaserver/plugins/ldap2.py +++ b/ipaserver/plugins/ldap2.py @@ -37,7 +37,7 @@ import ldap as _ldap from ipalib import krb_utils from ipaplatform.paths import paths from ipapython.dn import DN -from ipapython.ipaldap import (LDAPClient, AUTOBIND_AUTO, AUTOBIND_ENABLED, +from ipapython.ipaldap import (LDAPCache, AUTOBIND_AUTO, AUTOBIND_ENABLED, AUTOBIND_DISABLED) from ipalib import Registry, errors, _ @@ -52,7 +52,7 @@ _missing = object() @register() -class ldap2(CrudBackend, LDAPClient): +class ldap2(CrudBackend, LDAPCache): """ LDAP Backend Take 2. """ @@ -61,11 +61,15 @@ class ldap2(CrudBackend, LDAPClient): force_schema_updates = api.env.context in ('installer', 'updates') CrudBackend.__init__(self, api) - LDAPClient.__init__(self, None, - force_schema_updates=force_schema_updates) + LDAPCache.__init__( + self, None, + force_schema_updates=force_schema_updates, + enable_cache=api.env.ldap_cache and not force_schema_updates, + cache_size=api.env.ldap_cache_size, + ) - self._time_limit = float(LDAPClient.time_limit) - self._size_limit = int(LDAPClient.size_limit) + self._time_limit = float(LDAPCache.time_limit) + self._size_limit = int(LDAPCache.size_limit) @property def ldap_uri(self): @@ -86,7 +90,7 @@ class ldap2(CrudBackend, LDAPClient): @time_limit.deleter def time_limit(self): - object.__setattr__(self, '_time_limit', int(LDAPClient.size_limit)) + object.__setattr__(self, '_time_limit', int(LDAPCache.size_limit)) @property def size_limit(self): @@ -103,7 +107,7 @@ class ldap2(CrudBackend, LDAPClient): @size_limit.deleter def size_limit(self): - object.__setattr__(self, '_size_limit', float(LDAPClient.time_limit)) + object.__setattr__(self, '_size_limit', float(LDAPCache.time_limit)) def _connect(self): # Connectible.conn is a proxy to thread-local storage; @@ -153,9 +157,11 @@ class ldap2(CrudBackend, LDAPClient): if size_limit is not _missing: object.__setattr__(self, 'size_limit', size_limit) - client = LDAPClient(self.ldap_uri, - force_schema_updates=self._force_schema_updates, - cacert=cacert) + client = LDAPCache( + self.ldap_uri, + force_schema_updates=self._force_schema_updates, + enable_cache=self._enable_cache, + cacert=cacert) conn = client._conn with client.error_handler(): @@ -212,6 +218,7 @@ class ldap2(CrudBackend, LDAPClient): object.__delattr__(self, 'time_limit') object.__delattr__(self, 'size_limit') + self.clear_cache() def get_ipa_config(self, attrs_list=None): """Returns the IPA configuration entry (dn, entry_attrs).""" @@ -384,7 +391,7 @@ class ldap2(CrudBackend, LDAPClient): if (otp): pw = old_pass+otp - with LDAPClient(self.ldap_uri, force_schema_updates=False) as conn: + with LDAPCache(self.ldap_uri, force_schema_updates=False) as conn: conn.simple_bind(dn, pw) conn.unbind()