slap: improve invalid instance parameters reporting

 - warn with all json validation errors, not just the first one
 - make validation error message easier to understand by not including
   so many useless details
 - do not let jsonschema fetch schema references by providing our own
   fetch implementation

slapos/slap/slap.py

@@ -43,13 +43,12 @@ from functools import wraps
 import warnings
 
 import json
-import jsonschema
 import six
 
 from .exception import ResourceNotReady, ServerError, NotFoundError, \
           ConnectionError
 from .hateoas import SlapHateoasNavigator, ConnectionHelper
-from slapos.util import (SoftwareReleaseSchema,
+from slapos.util import (SoftwareReleaseSchema, SoftwareReleaseSchemaValidationError,
                          bytes2str, calculate_dict_hash, dict2xml, dumps, loads,
                          unicode2str, xml2dict)
 
@@ -105,9 +104,9 @@ class SlapRequester(SlapDocument):
         software_release,
         software_type,
       ).validateInstanceParameterDict(parameter_dict)
-    except jsonschema.ValidationError as e:
+    except SoftwareReleaseSchemaValidationError as e:
       warnings.warn(
-        "Request parameters do not validate against schema definition:\n{e}".format(e=e),
+        "Request parameters do not validate against schema definition:\n{e}".format(e=e.format_error(indent=2)),
         UserWarning,
       )
     except Exception as e:

slapos/tests/test_slap.py

@@ -903,6 +903,7 @@ class TestComputerPartition(SlapMixin):
                         "$ref": "./schemas-definitions.json#/foo"
                     }
                 },
+                "additionalProperties": False,
                 "type": "object"
             })
       if url.path.endswith('/schemas-definitions.json'):
@@ -930,18 +931,37 @@ class TestComputerPartition(SlapMixin):
       if PY3:
         warn.assert_called_with(
           "Request parameters do not validate against schema definition:\n"
-          "'bar' was expected\n\n"
-          "Failed validating 'const' in schema['properties']['foo']:\n"
-          "    {'const': 'bar', 'type': 'string'}\n\n"
-          "On instance['foo']:\n    'baz'", UserWarning
+          "  $.foo: 'bar' was expected",
+          UserWarning
         )
       else: # BBB
         warn.assert_called_with(
           "Request parameters do not validate against schema definition:\n"
-          "u'bar' was expected\n\n"
-          "Failed validating u'const' in schema[u'properties'][u'foo']:\n"
-          "    {u'const': u'bar', u'type': u'string'}\n\n"
-          "On instance[u'foo']:\n    'baz'", UserWarning
+          "  $.foo: u'bar' was expected",
+          UserWarning
+        )
+
+    with httmock.HTTMock(handler):
+      with mock.patch.object(warnings, 'warn') as warn:
+        cp = slapos.slap.ComputerPartition('computer_id', 'partition_id')
+        cp._connection_helper = mock.Mock()
+        cp._connection_helper.POST.side_effect = slapos.slap.ResourceNotReady
+        cp.request(
+            'https://example.com/software.cfg', 'default', 'reference',
+            partition_parameter_kw={'fooo': 'xxx'})
+      if PY3:
+        warn.assert_called_with(
+          "Request parameters do not validate against schema definition:\n"
+          "  $: 'foo' is a required property\n"
+          "  $: Additional properties are not allowed ('fooo' was unexpected)",
+          UserWarning
+        )
+      else: # BBB
+        warn.assert_called_with(
+          "Request parameters do not validate against schema definition:\n"
+          "  $: u'foo' is a required property\n"
+          "  $: Additional properties are not allowed ('fooo' was unexpected)",
+          UserWarning
         )
 
   def test_request_validate_request_parameter_broken_software_release_schema(self):

slapos/tests/test_util.py

@@ -36,12 +36,15 @@ import warnings
 from pwd import getpwnam
 
 from six.moves import SimpleHTTPServer
-import jsonschema
 
 import slapos.util
 from slapos.testing.utils import ManagedHTTPServer
-from slapos.util import (SoftwareReleaseSchema, SoftwareReleaseSerialisation,
-                         string_to_boolean, unicode2str)
+from slapos.util import (
+    SoftwareReleaseSchema,
+    SoftwareReleaseSerialisation,
+    SoftwareReleaseSchemaValidationError,
+    string_to_boolean,
+)
 
 
 class TestUtil(unittest.TestCase):
@@ -300,12 +303,12 @@ class SoftwareReleaseSchemaTestMixin(object):
       # already serialized values are also tolerated
       schema.validateInstanceParameterDict({'_': json.dumps(instance_ok)})
 
-    with self.assertRaises(jsonschema.ValidationError):
+    with self.assertRaises(SoftwareReleaseSchemaValidationError):
       schema.validateInstanceParameterDict({"wrong": True})
     instance_ok['key'] = False # wrong type
-    with self.assertRaises(jsonschema.ValidationError):
+    with self.assertRaises(SoftwareReleaseSchemaValidationError):
       schema.validateInstanceParameterDict(instance_ok)
-    with self.assertRaises(jsonschema.ValidationError):
+    with self.assertRaises(SoftwareReleaseSchemaValidationError):
       schema.validateInstanceParameterDict({'_': json.dumps(instance_ok)})
 
   def test_instance_request_parameter_validate_alternate_software_type(self):
@@ -318,12 +321,12 @@ class SoftwareReleaseSchemaTestMixin(object):
       # already serialized values are also tolerated
       schema.validateInstanceParameterDict({'_': json.dumps(instance_ok)})
 
-    with self.assertRaises(jsonschema.ValidationError):
+    with self.assertRaises(SoftwareReleaseSchemaValidationError):
       schema.validateInstanceParameterDict({"wrong": True})
     instance_ok['type'] = 'wrong'
-    with self.assertRaises(jsonschema.ValidationError):
+    with self.assertRaises(SoftwareReleaseSchemaValidationError):
       schema.validateInstanceParameterDict(instance_ok)
-    with self.assertRaises(jsonschema.ValidationError):
+    with self.assertRaises(SoftwareReleaseSchemaValidationError):
       schema.validateInstanceParameterDict({'_': json.dumps(instance_ok)})
 
   def test_instance_request_parameter_schema_alternate_software_type(self):

slapos/util.py

@@ -51,7 +51,7 @@ from six.moves.urllib_parse import urljoin
 from xml_marshaller.xml_marshaller import Marshaller, Unmarshaller
 
 try:
-  from typing import Dict, Optional, IO
+  from typing import Dict, Iterator, List, Optional
 except ImportError:
   pass
 
@@ -429,11 +429,59 @@ def _readAsJson(url, set_schema_id=False):
                   % (url, type(e).__name__, e))
 
 
+class _RefResolver(jsonschema.validators.RefResolver):
+  def resolve_remote(self, uri):
+    result = _readAsJson(uri)
+    if self.cache_remote:
+        self.store[uri] = result
+    return result
+
+
 class SoftwareReleaseSerialisation(str, enum.Enum):
   Xml = 'xml'
   JsonInXml = 'json-in-xml'
 
 
+class SoftwareReleaseSchemaValidationError(ValueError):
+  """Error raised when a request is made with invalid parameters.
+
+  This collects all the json schema validation errors.
+  """
+  def __init__(self, validation_errors):
+    # type: (List[jsonschema.ValidationError]) -> None
+    self.validation_errors = validation_errors
+    super(SoftwareReleaseSchemaValidationError, self).__init__()
+
+  def format_error(self, indent=0):
+    def _iter_validation_error(err):
+      # type: (jsonschema.ValidationError) -> Iterator[jsonschema.ValidationError]
+      if err.context:
+        for e in err.context:
+          yield e
+          # BBB PY3
+          # yield from _iter_validation_error(e)
+          for sube in _iter_validation_error(e):
+            yield sube
+
+    msg_list = []
+    for e in self.validation_errors:
+      if six.PY2:  # BBB
+        def json_path(e):
+          path = "$"
+          for elem in e.absolute_path:
+              if isinstance(elem, int):
+                  path += "[" + str(elem) + "]"
+              else:
+                  path += "." + elem
+          return path
+        msg_list.append("{e_json_path}: {e.message}".format(e=e, e_json_path=json_path(e)))
+      else:
+        msg_list.append("{e.json_path}: {e.message}".format(e=e))
+
+    indent_str = "\n" + (" " * indent)
+    return (" " * indent) + indent_str.join(msg_list)
+
+
 class SoftwareReleaseSchema(object):
 
   def __init__(self, software_url, software_type):
@@ -537,7 +585,8 @@ class SoftwareReleaseSchema(object):
     # type: (Dict) -> None
     """Validate instance parameters against the software schema.
 
-    Raise jsonschema.ValidationError if parameters does not validate.
+    Raise SoftwareReleaseSchemaValidationError if parameters do not
+    validate.
     """
     schema = self.getInstanceRequestParameterSchema()
     if schema:
@@ -547,7 +596,14 @@ class SoftwareReleaseSchema(object):
         except KeyError:
           pass
       parameter_dict.pop('$schema', None)
-      jsonschema.validate(instance=parameter_dict, schema=schema)
+
+      validator = jsonschema.validators.validator_for(schema)(
+        schema,
+        resolver=_RefResolver.from_schema(schema),
+      )
+      errors = list(validator.iter_errors(parameter_dict))
+      if errors:
+        raise SoftwareReleaseSchemaValidationError(errors)
 
 
 # BBB on python3 we can use pprint.pformat