From 9f875767cbeb5adf43ce3cdb124e4614ea876767 Mon Sep 17 00:00:00 2001 From: Scott Hussey Date: Fri, 31 Aug 2018 10:29:43 -0500 Subject: [PATCH] [458884] Refactor validation - This addresses a bug where Promenade doesn't detect some invalid configurations during genesis script generation. - Refactor some validation checks for performance Change-Id: I8b39caaab04819a935b83eb544979eac333fe409 --- promenade/config.py | 4 +- promenade/control/validatedesign.py | 2 +- promenade/utils/validation_message.py | 2 +- promenade/validation.py | 76 ++++++++++++++++++--------- 4 files changed, 55 insertions(+), 29 deletions(-) diff --git a/promenade/config.py b/promenade/config.py index 41a3f05a..b7a156fc 100644 --- a/promenade/config.py +++ b/promenade/config.py @@ -22,8 +22,6 @@ class Configuration: leave_kubectl=False, validate=True): LOG.info("Parsing document schemas.") - schema_set = validation.load_schemas_from_docs(documents) - LOG.info("Parsed %d document schemas." % len(schema_set)) LOG.info("Building config from %d documents." % len(documents)) if substitute: LOG.info("Rendering documents via Deckhand engine.") @@ -40,7 +38,7 @@ class Configuration: LOG.info("Deckhand engine returned %d documents." % len(documents)) if validate: - validation.check_schemas(documents, schemas=schema_set) + validation.validate_all(documents) self.debug = debug self.documents = documents self.leave_kubectl = leave_kubectl diff --git a/promenade/control/validatedesign.py b/promenade/control/validatedesign.py index b367432e..57c750d5 100644 --- a/promenade/control/validatedesign.py +++ b/promenade/control/validatedesign.py @@ -32,7 +32,7 @@ class ValidateDesignResource(base.BaseResource): href = json_data.get('href', None) config = Configuration.from_design_ref( href, allow_missing_substitutions=False) - result = validation.check_design(config) + result = validation.validate_all(config) except exceptions.InvalidFormatError as e: msg = "Invalid JSON Format: %s" % str(e) result.add_error_message(msg, name=e.title) diff --git a/promenade/utils/validation_message.py b/promenade/utils/validation_message.py index faf686ac..dace1590 100644 --- a/promenade/utils/validation_message.py +++ b/promenade/utils/validation_message.py @@ -55,7 +55,7 @@ class ValidationMessage(Message): 'kind': 'ValidationMessage' } if schema and doc_name: - self.output['documents'].append(dict(schema=schema, name=doc_name)) + new_error['documents'].append(dict(schema=schema, name=doc_name)) self.details['errorCount'] += 1 self.details['messageList'].append(new_error) diff --git a/promenade/validation.py b/promenade/validation.py index 3705eeda..d78823be 100644 --- a/promenade/validation.py +++ b/promenade/validation.py @@ -20,42 +20,70 @@ import os import pkg_resources import yaml -__all__ = ['check_schema', 'check_schemas'] +__all__ = ['check_schema', 'check_schemas', 'validate_all'] LOG = logging.getLogger(__name__) -def check_design(config): - kinds = ['Docker', 'Genesis', 'HostSystem', 'Kubelet', 'KubernetesNetwork'] +def validate_all(config): + """Run all available Promenade validations on the config.""" + + exception_list = check_schemas(config.documents) + exception_list.extend(check_design(config)) + validation_msg = ValidationMessage() - for kind in kinds: - count = 0 - schema = None - name = None - for doc in config.documents: - schema = doc.get('schema', None) - if not schema: - msg = '"schema" is a required document key.' - exc = exceptions.ValidationException(msg) - validation_msg.add_error_message(str(exc), name=exc.title) - return validation_msg - name = schema.split('/')[1] - if name == kind: - count += 1 - if count != 1: - msg = ('There are {0} {1} documents. However, there should be one.' - ).format(count, kind) - exc = exceptions.ValidationException(msg) - validation_msg.add_error_message( - str(exc), name=exc.title, schema=schema, doc_name=kind) + + for ex in exception_list: + validation_msg.add_error_message(str(ex)) + return validation_msg +def check_design(config): + """Check that each document type has the correct cardinality.""" + + expected_count = { + 'Docker': 1, + 'Genesis': 1, + 'HostSystem': 1, + 'Kubelet': 1, + 'KubernetesNetwork': 1, + } + + counts = {} + exception_list = [] + + for k in expected_count.keys(): + counts[k] = 0 + + for doc in config.documents: + schema = doc.get('schema', None) + if not schema: + msg = '"schema" is a required document key.' + exception_list.append(exceptions.ValidationException(msg)) + continue + name = schema.split('/')[1] + if name in counts: + counts[name] += 1 + + for kind, cnt in counts.items(): + if cnt != 1: + msg = ('There are {0} {1} documents. However, there should be one.' + ).format(cnt, kind) + exception_list.append(exceptions.ValidationException(msg)) + return exception_list + + def check_schemas(documents, schemas=None): if not schemas: schemas = load_schemas_from_docs(documents) + exception_list = [] for document in documents: - check_schema(document, schemas=schemas) + try: + check_schema(document, schemas=schemas) + except exceptions.ValidationException as ex: + exception_list.append(ex) + return exception_list def check_schema(document, schemas=None):