First draft for global syntax validation - report all validation errors at once

Signed-off-by: David Martin <david@chaoiq.io>

Author: David Martin

chaoslib/activity.py

@@ -36,67 +36,73 @@ def ensure_activity_is_valid(activity: Activity):
 
     In all failing cases, raises :exc:`InvalidActivity`.
     """
+    errors = []
     if not activity:
-        raise InvalidActivity("empty activity is no activity")
+        errors.append(InvalidActivity("empty activity is no activity"))
+        return errors
 
     # when the activity is just a ref, there is little to validate
     ref = activity.get("ref")
     if ref is not None:
         if not isinstance(ref, str) or ref == '':
-            raise InvalidActivity(
-                "reference to activity must be non-empty strings")
-        return
+            errors.append(InvalidActivity(
+                "reference to activity must be non-empty strings"))
+        return errors
 
     activity_type = activity.get("type")
     if not activity_type:
-        raise InvalidActivity("an activity must have a type")
+        errors.append(InvalidActivity("an activity must have a type"))
 
     if activity_type not in ("probe", "action"):
-        raise InvalidActivity(
-            "'{t}' is not a supported activity type".format(t=activity_type))
+        errors.append(InvalidActivity(
+            "'{t}' is not a supported activity type".format(t=activity_type)))
 
     if not activity.get("name"):
-        raise InvalidActivity("an activity must have a name")
+        errors.append(InvalidActivity("an activity must have a name"))
 
     provider = activity.get("provider")
     if not provider:
-        raise InvalidActivity("an activity requires a provider")
+        errors.append(InvalidActivity("an activity requires a provider"))
+        provider_type = None
+    else:
+        provider_type = provider.get("type")
+        if not provider_type:
+            errors.append(InvalidActivity("a provider must have a type"))
 
-    provider_type = provider.get("type")
-    if not provider_type:
-        raise InvalidActivity("a provider must have a type")
-
-    if provider_type not in ("python", "process", "http"):
-        raise InvalidActivity(
-            "unknown provider type '{type}'".format(type=provider_type))
-
-    if not activity.get("name"):
-        raise InvalidActivity("activity must have a name (cannot be empty)")
+        if provider_type not in ("python", "process", "http"):
+            errors.append(InvalidActivity(
+                "unknown provider type '{type}'".format(type=provider_type)))
 
     timeout = activity.get("timeout")
     if timeout is not None:
         if not isinstance(timeout, numbers.Number):
-            raise InvalidActivity("activity timeout must be a number")
+            errors.append(
+                InvalidActivity("activity timeout must be a number"))
 
     pauses = activity.get("pauses")
     if pauses is not None:
         before = pauses.get("before")
         if before is not None and not isinstance(before, numbers.Number):
-            raise InvalidActivity("activity before pause must be a number")
+            errors.append(
+                InvalidActivity("activity before pause must be a number"))
         after = pauses.get("after")
         if after is not None and not isinstance(after, numbers.Number):
-            raise InvalidActivity("activity after pause must be a number")
+            errors.append(
+                InvalidActivity("activity after pause must be a number"))
 
     if "background" in activity:
         if not isinstance(activity["background"], bool):
-            raise InvalidActivity("activity background must be a boolean")
+            errors.append(
+                InvalidActivity("activity background must be a boolean"))
 
     if provider_type == "python":
-        validate_python_activity(activity)
+        errors.extend(validate_python_activity(activity))
     elif provider_type == "process":
-        validate_process_activity(activity)
+        errors.extend(validate_process_activity(activity))
     elif provider_type == "http":
-        validate_http_activity(activity)
+        errors.extend(validate_http_activity(activity))
+
+    return errors
 
 
 def run_activities(experiment: Experiment, configuration: Configuration,

chaoslib/control/__init__.py

@@ -7,7 +7,8 @@
 
 from chaoslib.control.python import apply_python_control, cleanup_control, \
     initialize_control, validate_python_control, import_control
-from chaoslib.exceptions import InterruptExecution, InvalidControl
+from chaoslib.exceptions import InterruptExecution, InvalidControl, \
+    ChaosException
 from chaoslib.settings import get_loaded_settings
 from chaoslib.types import Settings
 from chaoslib.types import Activity, Configuration, Control as ControlType, \
@@ -85,12 +86,11 @@ def cleanup_controls(experiment: Experiment):
             cleanup_control(control)
 
 
-def validate_controls(experiment: Experiment):
+def validate_controls(experiment: Experiment) -> List[ChaosException]:
     """
     Validate that all declared controls respect the specification.
-
-    Raises :exc:`chaoslib.exceptions.InvalidControl` when they are not valid.
     """
+    errors = []
     controls = get_controls(experiment)
     references = [
         c["name"] for c in get_controls(experiment)
@@ -99,26 +99,29 @@ def validate_controls(experiment: Experiment):
     for c in controls:
         if "ref" in c:
             if c["ref"] not in references:
-                raise InvalidControl(
-                    "Control reference '{}' declaration cannot be found")
+                errors.append(InvalidControl(
+                    "Control reference '{}' declaration cannot be found"))
 
         if "name" not in c:
-            raise InvalidControl("A control must have a `name` property")
+            errors.append(
+                InvalidControl("A control must have a `name` property"))
 
-        name = c["name"]
+        name = c.get("name", '')
         if "provider" not in c:
-            raise InvalidControl(
-                "Control '{}' must have a `provider` property".format(name))
+            errors.append(InvalidControl(
+                "Control '{}' must have a `provider` property".format(name)))
 
         scope = c.get("scope")
         if scope and scope not in ("before", "after"):
-            raise InvalidControl(
+            errors.append(InvalidControl(
                 "Control '{}' scope property must be 'before' or "
-                "'after' only".format(name))
+                "'after' only".format(name)))
 
         provider_type = c.get("provider", {}).get("type")
         if provider_type == "python":
-            validate_python_control(c)
+            errors.extend(validate_python_control(c))
+
+    return errors
 
 
 def initialize_global_controls(experiment: Experiment,

chaoslib/control/python.py

@@ -7,7 +7,7 @@
 from logzero import logger
 
 from chaoslib import substitute
-from chaoslib.exceptions import InvalidActivity
+from chaoslib.exceptions import InvalidActivity, ChaosException
 from chaoslib.types import Activity, Configuration, Control, Experiment, \
     Journal, Run, Secrets, Settings
 
@@ -83,16 +83,19 @@ def cleanup_control(control: Control):
     func()
 
 
-def validate_python_control(control: Control):
+def validate_python_control(control: Control) -> List[ChaosException]:
     """
     Verify that a control block matches the specification
     """
+    errors = []
     name = control["name"]
     provider = control["provider"]
     mod_name = provider.get("module")
     if not mod_name:
-        raise InvalidActivity(
-            "Control '{}' must have a module path".format(name))
+        errors.append(InvalidActivity(
+            "Control '{}' must have a module path".format(name)))
+        # can not continue any longer - must exit this function
+        return errors
 
     try:
         importlib.import_module(mod_name)

chaoslib/experiment.py

@@ -48,54 +48,66 @@ def ensure_experiment_is_valid(experiment: Experiment):
     another set of of  ̀close` probes to sense the state of the system
     post-action.
 
-    This function raises :exc:`InvalidExperiment`, :exc:`InvalidProbe` or
-    :exc:`InvalidAction` depending on where it fails.
+    This function raises an :exc:`InvalidExperiment` error
+    if the experiment is not valid.
+    If multiple validation errors are found, the errors are listed
+    as part of the exception message
     """
     logger.info("Validating the experiment's syntax")
 
+    errors = []
+
     if not experiment:
         raise InvalidExperiment("an empty experiment is not an experiment")
 
     if not experiment.get("title"):
-        raise InvalidExperiment("experiment requires a title")
+        errors.append(InvalidExperiment("experiment requires a title"))
 
     if not experiment.get("description"):
-        raise InvalidExperiment("experiment requires a description")
+        errors.append(InvalidExperiment("experiment requires a description"))
 
     tags = experiment.get("tags")
     if tags:
         if list(filter(lambda t: t == '' or not isinstance(t, str), tags)):
-            raise InvalidExperiment(
-                "experiment tags must be a non-empty string")
+            errors.append(InvalidExperiment(
+                "experiment tags must be a non-empty string"))
 
-    validate_extensions(experiment)
+    errors.extend(validate_extensions(experiment))
 
     config = load_configuration(experiment.get("configuration", {}))
     load_secrets(experiment.get("secrets", {}), config)
 
-    ensure_hypothesis_is_valid(experiment)
+    errors.extend(ensure_hypothesis_is_valid(experiment))
 
     method = experiment.get("method")
     if not method:
-        raise InvalidExperiment("an experiment requires a method with "
-                                "at least one activity")
-
-    for activity in method:
-        ensure_activity_is_valid(activity)
-
-        # let's see if a ref is indeed found in the experiment
-        ref = activity.get("ref")
-        if ref and not lookup_activity(ref):
-            raise InvalidActivity("referenced activity '{r}' could not be "
-                                  "found in the experiment".format(r=ref))
+        errors.append(InvalidExperiment("an experiment requires a method with "
+                                        "at least one activity"))
+    else:
+        for activity in method:
+            errors.extend(ensure_activity_is_valid(activity))
+
+            # let's see if a ref is indeed found in the experiment
+            ref = activity.get("ref")
+            if ref and not lookup_activity(ref):
+                errors.append(
+                    InvalidActivity("referenced activity '{r}' could not be "
+                                    "found in the experiment".format(r=ref)))
 
     rollbacks = experiment.get("rollbacks", [])
     for activity in rollbacks:
-        ensure_activity_is_valid(activity)
+        errors.extend(ensure_activity_is_valid(activity))
 
     warn_about_deprecated_features(experiment)
 
-    validate_controls(experiment)
+    errors.extend(validate_controls(experiment))
+
+    if errors:
+        full_validation_msg = 'Experiment is not valid, ' \
+                              'please fix the following errors:'
+        for error in errors:
+            full_validation_msg += '\n- {}'.format(error)
+        raise InvalidExperiment(full_validation_msg)
 
     logger.info("Experiment looks valid")
 

chaoslib/extension.py

@@ -1,25 +1,29 @@
 # -*- coding: utf-8 -*-
-from typing import Optional
+from typing import Optional, List
 
-from chaoslib.exceptions import InvalidExperiment
+from chaoslib.exceptions import InvalidExperiment, ChaosException
 from chaoslib.types import Experiment, Extension
 
 __all__ = ["get_extension", "has_extension", "set_extension",
            "merge_extension", "remove_extension", "validate_extensions"]
 
 
-def validate_extensions(experiment: Experiment):
+def validate_extensions(experiment: Experiment) -> List[ChaosException]:
     """
     Validate that extensions respect the specification.
     """
     extensions = experiment.get("extensions")
     if not extensions:
-        return
+        return []
 
+    errors = []
     for ext in extensions:
         ext_name = ext.get('name')
         if not ext_name or not ext_name.strip():
-            raise InvalidExperiment("All extensions require a non-empty name")
+            errors.append(
+                InvalidExperiment("All extensions require a non-empty name"))
+
+    return errors
 
 
 def get_extension(experiment: Experiment, name: str) -> Optional[Extension]:

chaoslib/hypothesis.py

@@ -34,21 +34,25 @@ def ensure_hypothesis_is_valid(experiment: Experiment):
     """
     hypo = experiment.get("steady-state-hypothesis")
     if hypo is None:
-        return
+        return []
 
+    errors = []
     if not hypo.get("title"):
-        raise InvalidExperiment("hypothesis requires a title")
+        errors.append(InvalidExperiment("hypothesis requires a title"))
 
     probes = hypo.get("probes")
     if probes:
         for probe in probes:
-            ensure_activity_is_valid(probe)
+            errors.extend(ensure_activity_is_valid(probe))
 
             if "tolerance" not in probe:
-                raise InvalidActivity(
-                    "hypothesis probe must have a tolerance entry")
+                errors.append(InvalidActivity(
+                    "hypothesis probe must have a tolerance entry"))
+            else:
+                errors.extend(
+                    ensure_hypothesis_tolerance_is_valid(probe["tolerance"]))
 
-            ensure_hypothesis_tolerance_is_valid(probe["tolerance"])
+    return errors
 
 
 def ensure_hypothesis_tolerance_is_valid(tolerance: Tolerance):
@@ -81,6 +85,9 @@ def ensure_hypothesis_tolerance_is_valid(tolerance: Tolerance):
                 "hypothesis probe tolerance type '{}' is unsupported".format(
                     tolerance_type))
 
+    # TODO
+    return []
+
 
 def check_regex_pattern(tolerance: Tolerance):
     """