From 8726573940c435ec75ca72a9ec20d9744561c07c Mon Sep 17 00:00:00 2001 From: John Dennis Date: Tue, 29 Nov 2016 11:36:32 -0500 Subject: [PATCH] Fix keystone-manage mapping_engine tester There were several problems with keystone-manage mapping_engine * It aborts with a backtrace because of wrong number of arguments passed to the RuleProcessor, it was missing the mapping_id parameter. * Error messages related to input data were cryptic and inprecise. * The --engine-debug option did not work. A fake mapping_id is now generated and passed to the RuleProcessor. If there was invalid data passed it was nearly impossible to determine what was causing the error, the command takes 2 input files, but which file contained the error? At what line? Why? For example I was consistently getting this error: Error while parsing line: '{': need more than 1 value to unpack and had no idea of what was wrong, the JSON looked valid to me. Turns out the assertion file is not formatted as JSON (yes this is documented in the help message but given the rules are JSON formatted and the RuleProcessor expects a dict for the assertion_data it's reasonsable to assume the data in the assertion file is formatted as a JSON object). The documentation in mapping_combinations.rst added a note in the section suggesting the use of the keystone-manage mapping_engine tester alerting the reader to the expected file formats. The MappingEngineTester class was refactored slighly to allow each method to know what file it was operating on and emit error messages that identify the file. The error message in addition to the pathname now includes the offending line number as well. As a bonus it doesn't fail if there is a blank line. The error message now looks like this: assertion file input.txt at line 4 expected 'key: value' but found 'foo' see help for file format The mapping_engine.LOG.logger level is now explictily set to DEBUG when --engine-debug is passed instead of (mistakenly assuming it defaulted to DEBUG) otherwise it's set to WARN. Closes-Bug: 1655182 Signed-off-by: John Dennis Change-Id: I2dea0f38b127ec185b79bfe06dd6a212da75cbca (cherry picked from commit f2d0f8c9ab38172a6e37b02339eac59da911435c) --- .../federation/mapping_combinations.rst | 6 ++ keystone/cmd/cli.py | 84 ++++++++++++------- 2 files changed, 60 insertions(+), 30 deletions(-) diff --git a/doc/source/federation/mapping_combinations.rst b/doc/source/federation/mapping_combinations.rst index ddfdfaf242..45118add86 100644 --- a/doc/source/federation/mapping_combinations.rst +++ b/doc/source/federation/mapping_combinations.rst @@ -69,6 +69,12 @@ tested with the ``keystone-manage mapping_engine`` command: $ keystone-manage mapping_engine --rules --input +.. NOTE:: + Although the rules file is formated as json the input file of assertion + data is formatted as individual lines of key: value pairs, + see `keystone-manage mapping_engine --help` for details. + + Mapping Conditions ------------------ diff --git a/keystone/cmd/cli.py b/keystone/cmd/cli.py index 3335ee839f..54050215cc 100644 --- a/keystone/cmd/cli.py +++ b/keystone/cmd/cli.py @@ -1083,65 +1083,89 @@ class MappingEngineTester(BaseApp): name = 'mapping_engine' - @staticmethod - def read_rules(path): + def __init__(self): + super(MappingEngineTester, self).__init__() + self.mapping_id = uuid.uuid4().hex + self.rules_pathname = None + self.rules = None + self.assertion_pathname = None + self.assertion = None + + def read_rules(self, path): + self.rules_pathname = path try: with open(path) as file: - return jsonutils.load(file) + self.rules = jsonutils.load(file) except ValueError as e: raise SystemExit(_('Error while parsing rules ' '%(path)s: %(err)s') % {'path': path, 'err': e}) - @staticmethod - def read_file(path): + def read_assertion(self, path): + self.assertion_pathname = path try: with open(path) as file: - return file.read().strip() + self.assertion = file.read().strip() except IOError as e: raise SystemExit(_("Error while opening file " "%(path)s: %(err)s") % {'path': path, 'err': e}) - @staticmethod - def normalize_assertion(assertion): - def split(line): + def normalize_assertion(self): + def split(line, line_num): try: k, v = line.split(':', 1) return k.strip(), v.strip() - except ValueError as e: - msg = _("Error while parsing line: '%(line)s': %(err)s") - raise SystemExit(msg % {'line': line, 'err': e}) - assertion = assertion.split('\n') + except ValueError: + msg = _("assertion file %(pathname)s at line %(line_num)d " + "expected 'key: value' but found '%(line)s' " + "see help for file format") + raise SystemExit(msg % {'pathname': self.assertion_pathname, + 'line_num': line_num, + 'line': line}) + assertion = self.assertion.split('\n') assertion_dict = {} prefix = CONF.command.prefix - for line in assertion: - k, v = split(line) + for line_num, line in enumerate(assertion, 1): + line = line.strip() + if line == '': + continue + k, v = split(line, line_num) if prefix: if k.startswith(prefix): assertion_dict[k] = v else: assertion_dict[k] = v - return assertion_dict + self.assertion = assertion_dict - @staticmethod - def normalize_rules(rules): - if isinstance(rules, list): - return {'rules': rules} - else: - return rules + def normalize_rules(self): + if isinstance(self.rules, list): + self.rules = {'rules': self.rules} @classmethod def main(cls): - if not CONF.command.engine_debug: + if CONF.command.engine_debug: + mapping_engine.LOG.logger.setLevel('DEBUG') + else: mapping_engine.LOG.logger.setLevel('WARN') - rules = MappingEngineTester.read_rules(CONF.command.rules) - rules = MappingEngineTester.normalize_rules(rules) - mapping_engine.validate_mapping_structure(rules) + tester = cls() - assertion = MappingEngineTester.read_file(CONF.command.input) - assertion = MappingEngineTester.normalize_assertion(assertion) - rp = mapping_engine.RuleProcessor(rules['rules']) - print(jsonutils.dumps(rp.process(assertion), indent=2)) + tester.read_rules(CONF.command.rules) + tester.normalize_rules() + mapping_engine.validate_mapping_structure(tester.rules) + + tester.read_assertion(CONF.command.input) + tester.normalize_assertion() + + if CONF.command.engine_debug: + print("Using Rules:\n%s" % ( + jsonutils.dumps(tester.rules, indent=2))) + print("Using Assertion:\n%s" % ( + jsonutils.dumps(tester.assertion, indent=2))) + + rp = mapping_engine.RuleProcessor(tester.mapping_id, + tester.rules['rules']) + mapped = rp.process(tester.assertion) + print(jsonutils.dumps(mapped, indent=2)) @classmethod def add_argument_parser(cls, subparsers):