From 4244c87802f0cc88072a7bac3574d1fb1377eb3d Mon Sep 17 00:00:00 2001 From: sebres Date: Thu, 9 Oct 2014 14:51:08 +0200 Subject: [PATCH] ConfigWrapper class introduced: sharing of the same ConfigReader object between JailsReader and JailReader (don't read jail config each jail); sharing of the same DefinitionInitConfigReader (ActionReader, FilterReader) between all jails using that; cache of read a config files was optimized; test case extended for all types of config readers; --- fail2ban/client/actionreader.py | 12 +++-- fail2ban/client/configparserinc.py | 13 ++--- fail2ban/client/configreader.py | 75 ++++++++++++++++++++++---- fail2ban/client/configurator.py | 4 +- fail2ban/client/fail2banreader.py | 13 ++--- fail2ban/client/filterreader.py | 10 ++-- fail2ban/client/jailreader.py | 20 +++---- fail2ban/client/jailsreader.py | 19 ++++--- fail2ban/tests/clientreadertestcase.py | 49 ++++++++++++----- 9 files changed, 153 insertions(+), 62 deletions(-) diff --git a/fail2ban/client/actionreader.py b/fail2ban/client/actionreader.py index 91f3322b..8022ecc1 100644 --- a/fail2ban/client/actionreader.py +++ b/fail2ban/client/actionreader.py @@ -26,7 +26,7 @@ __license__ = "GPL" import os -from .configreader import ConfigReader, DefinitionInitConfigReader +from .configreader import DefinitionInitConfigReader from ..helpers import getLogger # Gets the instance of the logger. @@ -47,15 +47,19 @@ class ActionReader(DefinitionInitConfigReader): DefinitionInitConfigReader.__init__( self, file_, jailName, initOpts, **kwargs) + def setFile(self, fileName): + self.__file = fileName + DefinitionInitConfigReader.setFile(self, os.path.join("action.d", fileName)) + + def getFile(self): + return self.__file + def setName(self, name): self._name = name def getName(self): return self._name - def read(self): - return ConfigReader.read(self, os.path.join("action.d", self._file)) - def convert(self): head = ["set", self._jailName] stream = list() diff --git a/fail2ban/client/configparserinc.py b/fail2ban/client/configparserinc.py index 7aac111b..31945849 100644 --- a/fail2ban/client/configparserinc.py +++ b/fail2ban/client/configparserinc.py @@ -253,17 +253,14 @@ after = 1.conf if i: ret += i # merge defaults and all sections to self: - for (n, v) in cfg.get_defaults().items(): - alld[n] = v - for (n, s) in cfg.get_sections().items(): + alld.update(cfg.get_defaults()) + for n, s in cfg.get_sections().iteritems(): if isinstance(s, dict): s2 = alls.get(n) - if s2 is not None: - for (n, v) in s.items(): - s2[n] = v + if isinstance(s2, dict): + s2.update(s) else: - s2 = s.copy() - alls[n] = s2 + alls[n] = s.copy() else: alls[n] = s diff --git a/fail2ban/client/configreader.py b/fail2ban/client/configreader.py index ada48803..d1cc7924 100644 --- a/fail2ban/client/configreader.py +++ b/fail2ban/client/configreader.py @@ -32,6 +32,65 @@ from ..helpers import getLogger # Gets the instance of the logger. logSys = getLogger(__name__) +logLevel = 6 + +class ConfigWrapper(): + + def __init__(self, use_config=None, share_config=None, **kwargs): + # use given shared config if possible (see read): + self._cfg_share = None + self._cfg = None + if use_config is not None: + self._cfg = use_config + else: + # share config if possible: + if share_config is not None: + self._cfg_share = share_config + self._cfg_share_kwargs = kwargs + else: + self._cfg = ConfigReader(**kwargs) + + def setBaseDir(self, basedir): + self._cfg.setBaseDir(basedir) + + def getBaseDir(self): + return self._cfg.getBaseDir() + + def read(self, name, once=True): + # shared ? + if not self._cfg and self._cfg_share is not None: + self._cfg = self._cfg_share.get(name) + if not self._cfg: + self._cfg = ConfigReader(**self._cfg_share_kwargs) + self._cfg_share[name] = self._cfg + # performance feature - read once if using shared config reader: + rc = self._cfg.read_cfg_files + if once and rc.get(name) is not None: + return rc.get(name) + + # read: + ret = self._cfg.read(name) + + # save already read: + if once: + rc[name] = ret + return ret + + def sections(self): + return self._cfg.sections() + + def has_section(self, sec): + return self._cfg.has_section(sec) + + def options(self, *args): + return self._cfg.options(*args) + + def get(self, sec, opt): + return self._cfg.get(sec, opt) + + def getOptions(self, *args, **kwargs): + return self._cfg.getOptions(*args, **kwargs) + class ConfigReader(SafeConfigParserWithIncludes): @@ -39,8 +98,8 @@ class ConfigReader(SafeConfigParserWithIncludes): def __init__(self, basedir=None): SafeConfigParserWithIncludes.__init__(self) + self.read_cfg_files = dict() self.setBaseDir(basedir) - self.__opts = None def setBaseDir(self, basedir): if basedir is None: @@ -122,17 +181,15 @@ class ConfigReader(SafeConfigParserWithIncludes): logSys.warning("'%s' not defined in '%s'. Using default one: %r" % (option[1], sec, option[2])) values[option[1]] = option[2] - else: - logSys.debug( - "Non essential option '%s' not defined in '%s'.", - option[1], sec) + elif logSys.getEffectiveLevel() <= logLevel: + logSys.log(logLevel, "Non essential option '%s' not defined in '%s'.", option[1], sec) except ValueError: logSys.warning("Wrong value for '" + option[1] + "' in '" + sec + "'. Using default one: '" + `option[2]` + "'") values[option[1]] = option[2] return values -class DefinitionInitConfigReader(ConfigReader): +class DefinitionInitConfigReader(ConfigWrapper): """Config reader for files with options grouped in [Definition] and [Init] sections. @@ -144,7 +201,7 @@ class DefinitionInitConfigReader(ConfigReader): _configOpts = [] def __init__(self, file_, jailName, initOpts, **kwargs): - ConfigReader.__init__(self, **kwargs) + ConfigWrapper.__init__(self, **kwargs) self.setFile(file_) self.setJailName(jailName) self._initOpts = initOpts @@ -163,14 +220,14 @@ class DefinitionInitConfigReader(ConfigReader): return self._jailName def read(self): - return ConfigReader.read(self, self._file) + return ConfigWrapper.read(self, self._file) # needed for fail2ban-regex that doesn't need fancy directories def readexplicit(self): return SafeConfigParserWithIncludes.read(self, self._file) def getOptions(self, pOpts): - self._opts = ConfigReader.getOptions( + self._opts = ConfigWrapper.getOptions( self, "Definition", self._configOpts, pOpts) if self.has_section("Init"): diff --git a/fail2ban/client/configurator.py b/fail2ban/client/configurator.py index 0dd9f955..a29fe94d 100644 --- a/fail2ban/client/configurator.py +++ b/fail2ban/client/configurator.py @@ -33,11 +33,11 @@ logSys = getLogger(__name__) class Configurator: - def __init__(self): + def __init__(self, force_enable=False): self.__settings = dict() self.__streams = dict() self.__fail2ban = Fail2banReader() - self.__jails = JailsReader() + self.__jails = JailsReader(force_enable=force_enable) def setBaseDir(self, folderName): self.__fail2ban.setBaseDir(folderName) diff --git a/fail2ban/client/fail2banreader.py b/fail2ban/client/fail2banreader.py index 361a5e54..a151ebf0 100644 --- a/fail2ban/client/fail2banreader.py +++ b/fail2ban/client/fail2banreader.py @@ -24,31 +24,32 @@ __author__ = "Cyril Jaquier" __copyright__ = "Copyright (c) 2004 Cyril Jaquier" __license__ = "GPL" -from .configreader import ConfigReader +from .configreader import ConfigWrapper from ..helpers import getLogger # Gets the instance of the logger. logSys = getLogger(__name__) -class Fail2banReader(ConfigReader): +class Fail2banReader(ConfigWrapper): def __init__(self, **kwargs): - ConfigReader.__init__(self, **kwargs) + self.__opts = None + ConfigWrapper.__init__(self, **kwargs) def read(self): - ConfigReader.read(self, "fail2ban") + ConfigWrapper.read(self, "fail2ban") def getEarlyOptions(self): opts = [["string", "socket", "/var/run/fail2ban/fail2ban.sock"], ["string", "pidfile", "/var/run/fail2ban/fail2ban.pid"]] - return ConfigReader.getOptions(self, "Definition", opts) + return ConfigWrapper.getOptions(self, "Definition", opts) def getOptions(self): opts = [["string", "loglevel", "INFO" ], ["string", "logtarget", "STDERR"], ["string", "dbfile", "/var/lib/fail2ban/fail2ban.sqlite3"], ["int", "dbpurgeage", 86400]] - self.__opts = ConfigReader.getOptions(self, "Definition", opts) + self.__opts = ConfigWrapper.getOptions(self, "Definition", opts) def convert(self): stream = list() diff --git a/fail2ban/client/filterreader.py b/fail2ban/client/filterreader.py index 40d74c45..fe657025 100644 --- a/fail2ban/client/filterreader.py +++ b/fail2ban/client/filterreader.py @@ -26,7 +26,7 @@ __license__ = "GPL" import os, shlex -from .configreader import ConfigReader, DefinitionInitConfigReader +from .configreader import DefinitionInitConfigReader from ..server.action import CommandAction from ..helpers import getLogger @@ -40,8 +40,12 @@ class FilterReader(DefinitionInitConfigReader): ["string", "failregex", ""], ] - def read(self): - return ConfigReader.read(self, os.path.join("filter.d", self._file)) + def setFile(self, fileName): + self.__file = fileName + DefinitionInitConfigReader.setFile(self, os.path.join("filter.d", fileName)) + + def getFile(self): + return self.__file def convert(self): stream = list() diff --git a/fail2ban/client/jailreader.py b/fail2ban/client/jailreader.py index a4bf5174..a3cc939c 100644 --- a/fail2ban/client/jailreader.py +++ b/fail2ban/client/jailreader.py @@ -27,7 +27,7 @@ __license__ = "GPL" import re, glob, os.path import json -from .configreader import ConfigReader +from .configreader import ConfigReader, ConfigWrapper from .filterreader import FilterReader from .actionreader import ActionReader from ..helpers import getLogger @@ -35,17 +35,19 @@ from ..helpers import getLogger # Gets the instance of the logger. logSys = getLogger(__name__) -class JailReader(ConfigReader): +class JailReader(ConfigWrapper): optionCRE = re.compile("^((?:\w|-|_|\.)+)(?:\[(.*)\])?$") optionExtractRE = re.compile( r'([\w\-_\.]+)=(?:"([^"]*)"|\'([^\']*)\'|([^,]*))(?:,|$)') - def __init__(self, name, force_enable=False, **kwargs): - ConfigReader.__init__(self, **kwargs) + def __init__(self, name, force_enable=False, cfg_share=None, **kwargs): + # use shared config if possible: + ConfigWrapper.__init__(self, **kwargs) self.__name = name self.__filter = None self.__force_enable = force_enable + self.__cfg_share = cfg_share self.__actions = list() self.__opts = None @@ -60,7 +62,7 @@ class JailReader(ConfigReader): return self.__name def read(self): - out = ConfigReader.read(self, "jail") + out = ConfigWrapper.read(self, "jail") # Before returning -- verify that requested section # exists at all if not (self.__name in self.sections()): @@ -101,7 +103,7 @@ class JailReader(ConfigReader): ["string", "ignoreip", None], ["string", "filter", ""], ["string", "action", ""]] - self.__opts = ConfigReader.getOptions(self, self.__name, opts) + self.__opts = ConfigWrapper.getOptions(self, self.__name, opts) if not self.__opts: return False @@ -111,7 +113,7 @@ class JailReader(ConfigReader): filterName, filterOpt = JailReader.extractOptions( self.__opts["filter"]) self.__filter = FilterReader( - filterName, self.__name, filterOpt, basedir=self.getBaseDir()) + filterName, self.__name, filterOpt, share_config=self.__cfg_share, basedir=self.getBaseDir()) ret = self.__filter.read() if ret: self.__filter.getOptions(self.__opts) @@ -141,7 +143,7 @@ class JailReader(ConfigReader): else: action = ActionReader( actName, self.__name, actOpt, - basedir=self.getBaseDir()) + share_config=self.__cfg_share, basedir=self.getBaseDir()) ret = action.read() if ret: action.getOptions(self.__opts) @@ -213,7 +215,7 @@ class JailReader(ConfigReader): if self.__filter: stream.extend(self.__filter.convert()) for action in self.__actions: - if isinstance(action, ConfigReader): + if isinstance(action, (ConfigReader, ConfigWrapper)): stream.extend(action.convert()) else: stream.append(action) diff --git a/fail2ban/client/jailsreader.py b/fail2ban/client/jailsreader.py index 84c614b9..37d37e01 100644 --- a/fail2ban/client/jailsreader.py +++ b/fail2ban/client/jailsreader.py @@ -24,14 +24,14 @@ __author__ = "Cyril Jaquier" __copyright__ = "Copyright (c) 2004 Cyril Jaquier" __license__ = "GPL" -from .configreader import ConfigReader +from .configreader import ConfigReader, ConfigWrapper from .jailreader import JailReader from ..helpers import getLogger # Gets the instance of the logger. logSys = getLogger(__name__) -class JailsReader(ConfigReader): +class JailsReader(ConfigWrapper): def __init__(self, force_enable=False, **kwargs): """ @@ -41,7 +41,9 @@ class JailsReader(ConfigReader): Passed to JailReader to force enable the jails. It is for internal use """ - ConfigReader.__init__(self, **kwargs) + # use shared config if possible: + ConfigWrapper.__init__(self, **kwargs) + self.__cfg_share = dict() self.__jails = list() self.__force_enable = force_enable @@ -50,13 +52,13 @@ class JailsReader(ConfigReader): return self.__jails def read(self): - return ConfigReader.read(self, "jail") + return ConfigWrapper.read(self, "jail") def getOptions(self, section=None): """Reads configuration for jail(s) and adds enabled jails to __jails """ opts = [] - self.__opts = ConfigReader.getOptions(self, "Definition", opts) + self.__opts = ConfigWrapper.getOptions(self, "Definition", opts) if section is None: sections = self.sections() @@ -68,9 +70,10 @@ class JailsReader(ConfigReader): for sec in sections: if sec == 'INCLUDES': continue - jail = JailReader(sec, basedir=self.getBaseDir(), - force_enable=self.__force_enable) - jail.read() + # use the cfg_share for filter/action caching and the same config for all + # jails (use_config=...), therefore don't read it here: + jail = JailReader(sec, force_enable=self.__force_enable, + cfg_share=self.__cfg_share, use_config=self._cfg) ret = jail.getOptions() if ret: if jail.isEnabled(): diff --git a/fail2ban/tests/clientreadertestcase.py b/fail2ban/tests/clientreadertestcase.py index 5c5c75bc..a5fe7b6e 100644 --- a/fail2ban/tests/clientreadertestcase.py +++ b/fail2ban/tests/clientreadertestcase.py @@ -347,6 +347,23 @@ class FilterReaderTest(unittest.TestCase): class JailsReaderTestCache(LogCaptureTestCase): + def _readWholeConf(self, basedir, force_enable=False): + # read whole configuration like a file2ban-client ... + configurator = Configurator(force_enable=force_enable) + configurator.setBaseDir(basedir) + configurator.readEarly() + configurator.getEarlyOptions() + configurator.readAll() + # from here we test a cache with all includes / before / after : + self.assertTrue(configurator.getOptions(None)) + + def _getLoggedReadCount(self, filematch): + cnt = 0 + for s in self.getLog().rsplit('\n'): + if re.match(r"^Reading files?: .*/"+filematch, s): + cnt += 1 + return cnt + def testTestJailConfCache(self): basedir = tempfile.mkdtemp("fail2ban_conf") try: @@ -356,21 +373,27 @@ class JailsReaderTestCache(LogCaptureTestCase): shutil.copy(CONFIG_DIR + '/fail2ban.conf', basedir + '/fail2ban.local') # read whole configuration like a file2ban-client ... - configurator = Configurator() - configurator.setBaseDir(basedir) - configurator.readEarly() - configurator.getEarlyOptions() - configurator.readAll() - # from here we test a cache : - self.assertTrue(configurator.getOptions(None)) - cnt = 0 - for s in self.getLog().rsplit('\n'): - if re.match(r"^Reading files?: .*jail.local", s): - cnt += 1 + self._readWholeConf(basedir) + # how many times jail.local was read: + cnt = self._getLoggedReadCount('jail.local') # if cnt > 1: # self.printLog() - self.assertFalse(cnt > 1, "Too many times reading of config files, cnt = %s" % cnt) - self.assertFalse(cnt <= 0) + self.assertFalse(cnt > 1, "Too many times reading of jail files, cnt = %s" % cnt) + self.assertNotEqual(cnt, 0) + + # read whole configuration like a file2ban-client, again ... + # but this time force enable all jails, to check filter and action cached also: + self._readWholeConf(basedir, force_enable=True) + cnt = self._getLoggedReadCount(r'jail\.local') + # still one (no more reads): + self.assertFalse(cnt > 1, "Too many times second reading of jail files, cnt = %s" % cnt) + + # same with filter: + cnt = self._getLoggedReadCount(r'filter\.d/common\.conf') + self.assertFalse(cnt > 1, "Too many times reading of filter files, cnt = %s" % cnt) + # same with action: + cnt = self._getLoggedReadCount(r'action\.d/iptables-common\.conf') + self.assertFalse(cnt > 1, "Too many times reading of action files, cnt = %s" % cnt) finally: shutil.rmtree(basedir)