From 1d9abd1b39e2db7322986418436bf6849267edad Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Mon, 29 Apr 2013 12:37:16 +1000 Subject: [PATCH 1/6] ENH: allow recursive tag substitution in action files. --- server/action.py | 35 ++++++++++++++++++++++++++++++++++- testcases/actiontestcase.py | 21 +++++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/server/action.py b/server/action.py index ce9651e8..3221f150 100644 --- a/server/action.py +++ b/server/action.py @@ -28,7 +28,7 @@ __copyright__ = "Copyright (c) 2004 Cyril Jaquier" __license__ = "GPL" import logging, os -import threading +import threading, re #from subprocess import call # Gets the instance of the logger. @@ -143,6 +143,9 @@ class Action: # @return True if the command succeeded def execActionStart(self): + if self.__cInfo: + if not Action.substituteRecursiveTags(self.__cInfo): + return False startCmd = Action.replaceTag(self.__actionStart, self.__cInfo) return Action.executeCmd(startCmd) @@ -242,6 +245,36 @@ class Action: stopCmd = Action.replaceTag(self.__actionStop, self.__cInfo) return Action.executeCmd(stopCmd) + ## + # Sort out tag definations within other tags + # + # so: becomes: + # a = 3 a = 3 + # b = _3 b = 3_3 + # @param tags, a dictionary + # @returns tags altered or False if there is a recursive defination + #@staticmethod + def substituteRecursiveTags(tags): + t = re.compile(r'<([^ >]+)>') + for tag, value in tags.iteritems(): + value = str(value) + m = t.search(value) + while m: + if m.group(1) == tag: + # recursive definations are bad + return False + else: + if tags.has_key(m.group(1)): + value = value[0:m.start()] + tags[m.group(1)] + value[m.end():] + m = t.search(value, m.start()) + else: + # TODO missing tag? to abort or not? there is the case maybe + m = t.search(value, m.start() + 1) + tags[tag] = value + return tags + substituteRecursiveTags = staticmethod(substituteRecursiveTags) + + #@staticmethod def escapeTag(tag): for c in '\\#&;`|*?~<>^()[]{}$\n\'"': if c in tag: diff --git a/testcases/actiontestcase.py b/testcases/actiontestcase.py index 50ae2816..281cb0b6 100644 --- a/testcases/actiontestcase.py +++ b/testcases/actiontestcase.py @@ -61,6 +61,27 @@ class ExecuteAction(unittest.TestCase): def _is_logged(self, s): return s in self._log.getvalue() + def testSubstituteRecursiveTags(self): + aInfo = { + 'HOST': "192.0.2.0", + 'ABC': "123 ", + 'xyz': "890 ", + } + # Recursion is bad + self.assertFalse(Action.substituteRecursiveTags({'A': ''})) + self.assertFalse(Action.substituteRecursiveTags({'A': '', 'B': ''})) + self.assertFalse(Action.substituteRecursiveTags({'A': '', 'B': '', 'C': ''})) + # missing tags are ok + self.assertEquals(Action.substituteRecursiveTags({'A': ''}), {'A': ''}) + self.assertEquals(Action.substituteRecursiveTags({'A': ' ','X':'fun'}), {'A': ' fun', 'X':'fun'}) + self.assertEquals(Action.substituteRecursiveTags({'A': ' ', 'B': 'cool'}), {'A': ' cool', 'B': 'cool'}) + # rest is just cool + self.assertEquals(Action.substituteRecursiveTags(aInfo), + { 'HOST': "192.0.2.0", + 'ABC': '123 192.0.2.0', + 'xyz': '890 123 192.0.2.0', + }) + def testReplaceTag(self): aInfo = { 'HOST': "192.0.2.0", From e5474e57aa9cdef95e9b2425de8c8eaf867c4a78 Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Mon, 29 Apr 2013 12:38:42 +1000 Subject: [PATCH 2/6] DOC: ChangeLog for recursive tag substition --- ChangeLog | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ChangeLog b/ChangeLog index fde414f4..3240e8f3 100644 --- a/ChangeLog +++ b/ChangeLog @@ -90,6 +90,8 @@ Borreli, blotus: * [c8c7b0b,23bbc60] Better logging of log file read errors. * [3665e6d] Added code coverage to development process. * [41b9f7b,32d10e9] More complete ssh filter rules to match openssh source. + * [1d9abd1] Action files can have tags in defination that refer to other + tags. Pascal Borreli * [a2b29b4] Fixed lots of typos in config files and documentation. hamilton5 From 2403f395e9ed62dcac004fe7915c84c4fc660608 Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Mon, 29 Apr 2013 15:33:45 +1000 Subject: [PATCH 3/6] ENH: remove stats of config files and use results of SafeConfigParserWithIncludes.read to facilitate meaningful error messages --- client/configreader.py | 36 ++++++++++++++---------------------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/client/configreader.py b/client/configreader.py index 243c843c..44ebdd3d 100644 --- a/client/configreader.py +++ b/client/configreader.py @@ -52,9 +52,9 @@ class ConfigReader(SafeConfigParserWithIncludes): return self._basedir def read(self, filename): - if not (os.path.exists(self._basedir) and os.access(self._basedir, os.R_OK | os.X_OK)): - raise ValueError("Base configuration directory %s either does not exist " - "or is not accessible" % self._basedir) + if not os.path.exists(self._basedir): + raise ValueError("Base configuration directory %s does not exist " + % self._basedir) basename = os.path.join(self._basedir, filename) logSys.debug("Reading configs for %s under %s " % (basename, self._basedir)) config_files = [ basename + ".conf", @@ -65,27 +65,19 @@ class ConfigReader(SafeConfigParserWithIncludes): # possible further customizations under a .conf.d directory config_dir = basename + '.d' - if os.path.exists(config_dir): - if os.path.isdir(config_dir) and os.access(config_dir, os.X_OK | os.R_OK): - # files must carry .conf suffix as well - config_files += sorted(glob.glob('%s/*.conf' % config_dir)) - else: - logSys.warn("%s exists but not a directory or not accessible" - % config_dir) + config_files += sorted(glob.glob('%s/*.conf' % config_dir)) - # check if files are accessible, warn if any is not accessible - # and remove it from the list - config_files_accessible = [] - for f in config_files: - if os.access(f, os.R_OK): - config_files_accessible.append(f) - else: - logSys.warn("%s exists but not accessible - skipping" % f) - - if len(config_files_accessible): + if len(config_files): # at least one config exists and accessible - SafeConfigParserWithIncludes.read(self, config_files_accessible) - return True + logSys.debug("Reading config files: " + ', '.join(config_files)) + config_files_read = SafeConfigParserWithIncludes.read(self, config_files) + missed = [ cf for cf in config_files if cf not in config_files_read ] + logSys.error("Error reading files: " + ', '.join(missed)) + if config_files_read: + return True + logSys.error("Found no accessible config files for %r under %s" % + ( filename, self.getBaseDir() )) + return False else: logSys.error("Found no accessible config files for %r " % filename + (["under %s" % self.getBaseDir(), From d28f3fa285c0a8edd134052057a1c526bfaf3985 Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Tue, 30 Apr 2013 08:07:21 +1000 Subject: [PATCH 4/6] DOC: s/defination/definition/g learn to spell --- ChangeLog | 2 +- server/action.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ChangeLog b/ChangeLog index 3240e8f3..6e00364b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -90,7 +90,7 @@ Borreli, blotus: * [c8c7b0b,23bbc60] Better logging of log file read errors. * [3665e6d] Added code coverage to development process. * [41b9f7b,32d10e9] More complete ssh filter rules to match openssh source. - * [1d9abd1] Action files can have tags in defination that refer to other + * [1d9abd1] Action files can have tags in definition that refer to other tags. Pascal Borreli * [a2b29b4] Fixed lots of typos in config files and documentation. diff --git a/server/action.py b/server/action.py index 3221f150..22a96fa2 100644 --- a/server/action.py +++ b/server/action.py @@ -246,13 +246,13 @@ class Action: return Action.executeCmd(stopCmd) ## - # Sort out tag definations within other tags + # Sort out tag definitions within other tags # # so: becomes: # a = 3 a = 3 # b = _3 b = 3_3 # @param tags, a dictionary - # @returns tags altered or False if there is a recursive defination + # @returns tags altered or False if there is a recursive definition #@staticmethod def substituteRecursiveTags(tags): t = re.compile(r'<([^ >]+)>') @@ -261,7 +261,7 @@ class Action: m = t.search(value) while m: if m.group(1) == tag: - # recursive definations are bad + # recursive definitions are bad return False else: if tags.has_key(m.group(1)): From d7862266d6bbab8dac3ca6d9941bc118d42325cd Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Tue, 30 Apr 2013 08:14:50 +1000 Subject: [PATCH 5/6] DOC: missing cinfo tags are ok. Log error for self referencing definitions --- server/action.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/server/action.py b/server/action.py index 22a96fa2..c2f1455b 100644 --- a/server/action.py +++ b/server/action.py @@ -145,6 +145,7 @@ class Action: def execActionStart(self): if self.__cInfo: if not Action.substituteRecursiveTags(self.__cInfo): + logSys.error("Cinfo/definitions contain self referencing definitions and cannot be resolved") return False startCmd = Action.replaceTag(self.__actionStart, self.__cInfo) return Action.executeCmd(startCmd) @@ -268,7 +269,9 @@ class Action: value = value[0:m.start()] + tags[m.group(1)] + value[m.end():] m = t.search(value, m.start()) else: - # TODO missing tag? to abort or not? there is the case maybe + # Missing tags are ok so we just continue on searching. + # cInfo can contain aInfo elements like and valid shell + # constructs like . m = t.search(value, m.start() + 1) tags[tag] = value return tags From 98aa0e23eb6c20ecac7e8e7533b08333fceda879 Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Tue, 30 Apr 2013 08:19:11 +1000 Subject: [PATCH 6/6] BF: log error only if there were missed config files that couldn't be read --- client/configreader.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/client/configreader.py b/client/configreader.py index 44ebdd3d..5d52d6ad 100644 --- a/client/configreader.py +++ b/client/configreader.py @@ -72,7 +72,8 @@ class ConfigReader(SafeConfigParserWithIncludes): logSys.debug("Reading config files: " + ', '.join(config_files)) config_files_read = SafeConfigParserWithIncludes.read(self, config_files) missed = [ cf for cf in config_files if cf not in config_files_read ] - logSys.error("Error reading files: " + ', '.join(missed)) + if missed: + logSys.error("Could not read config files: " + ', '.join(missed)) if config_files_read: return True logSys.error("Found no accessible config files for %r under %s" %