cli/request: support passing instance parameters from a file

Support a syntax like:

     slapos request --node=computer_guid=local --parameters-file=~/request.json ERP5 https://lab.nexedi.com/nexedi/slapos/raw/1.0.145/software/erp5/software.cfg

to request an instance with parameters in ~/request.json file.

This automatically understands the serialisation, when software uses json-in-xml
serialisation, no need to use `_` argument with the encoded json.

This also adds missing tests for slapos request command

setup.py

@@ -74,6 +74,8 @@ setup(name=name,
           'six',
           'cachecontrol',
           'lockfile',
+          'jsonschema',
+          'pyaml',
           'uritemplate', # used by hateoas navigator
           'subprocess32; python_version<"3"',
           'ipaddress; python_version<"3"',  # used by whitelistfirewall

slapos/cli/request.py

@@ -27,15 +27,48 @@
 #
 ##############################################################################
 
+import argparse
+import json
+import os.path
 import pprint
 
+import lxml.etree
+import yaml
+
 from slapos.cli.config import ClientConfigCommand
-from slapos.client import init, ClientConfig, _getSoftwareReleaseFromSoftwareString
+from slapos.client import (ClientConfig, _getSoftwareReleaseFromSoftwareString,
+                           init)
 from slapos.slap import ResourceNotReady
-
+from slapos.util import SoftwareReleaseSchema, SoftwareReleaseSerialisation
+
+try:
+    from typing import IO, Dict
+except ImportError:
+    pass
+
+
+def getParametersFromFile(file, serialisation):
+    # type: (IO[str], SoftwareReleaseSerialisation) -> Dict
+    extension = os.path.splitext(file.name)[1]
+    if extension in ('.yaml', '.yml'):
+        params = yaml.safe_load(file)
+    elif extension == '.xml':
+        tree = lxml.etree.parse(file)
+        params = {e.attrib['id']: e.text for e in tree.findall('/parameter')}
+        # because the use case of xml files is to copy paste existing XML parameters
+        # as found on slapos web interface, we don't be clever regarding the
+        # serialisation and assume they are already correct.
+        serialisation = None
+    else:
+        params = json.load(file)
+
+    if serialisation == SoftwareReleaseSerialisation.JsonInXml:
+        params = {'_': json.dumps(params)}
+    return params
 
 
 def parse_option_dict(options):
+    # type: (str) -> Dict
     """
     Parse a list of option strings like foo=bar baz=qux and return a dictionary.
     Will raise if keys are repeated.
@@ -80,8 +113,12 @@ class RequestCommand(ClientConfigCommand):
 
         ap.add_argument('--parameters',
                         nargs='+',
-                        help="Give your configuration 'option1=value1 option2=value2'")
+                        help="Instance parameters, in the form 'option1=value1 option2=value2'.")
 
+        ap.add_argument('--parameters-file',
+                        type=argparse.FileType('r'),
+                        help="Instance parameters, in a file.\n"
+                        "The file will be interpreted as json, yaml or xml depending on the file extension.")
         return ap
 
     def take_action(self, args):
@@ -104,11 +141,17 @@ def do_request(logger, conf, local):
 
     if conf.software_url in local:
         conf.software_url = local[conf.software_url]
+
+    software_schema = SoftwareReleaseSchema(conf.software_url, conf.type)
+    software_schema_serialisation = software_schema.getSerialisation()
+    parameters = conf.parameters
+    if conf.parameters_file:
+        parameters = getParametersFromFile(conf.parameters_file, software_schema_serialisation)
     try:
         partition = local['slap'].registerOpenOrder().request(
             software_release=conf.software_url,
             partition_reference=conf.reference,
-            partition_parameter_kw=conf.parameters,
+            partition_parameter_kw=parameters,
             software_type=conf.type,
             filter_kw=conf.node,
             state=conf.state,
@@ -116,7 +159,11 @@ def do_request(logger, conf, local):
         )
         logger.info('Instance requested.\nState is : %s.', partition.getState())
         logger.info('Connection parameters of instance are:')
-        logger.info(pprint.pformat(partition.getConnectionParameterDict()))
+        connection_parameter_dict = partition.getConnectionParameterDict()
+        if software_schema_serialisation == SoftwareReleaseSerialisation.JsonInXml:
+            if '_' in connection_parameter_dict:
+                connection_parameter_dict = json.loads(connection_parameter_dict['_'])
+        logger.info(pprint.pformat(connection_parameter_dict))
         logger.info('You can rerun the command to get up-to-date information.')
     except ResourceNotReady:
         logger.warning('Instance requested. Master is provisioning it. Please rerun in a '

slapos/slap/slap.py

@@ -266,6 +266,20 @@ class OpenOrder(SlapRequester):
         'state': dumps(state),
         'shared_xml': dumps(shared),
     }
+
+    import warnings
+    import jsonschema
+    from ..util import SoftwareReleaseSchema
+    try:
+      SoftwareReleaseSchema(
+          request_dict['software_release'],
+          request_dict['software_type']
+      ).validateInstanceParameterDict(partition_parameter_kw)
+    except jsonschema.ValidationError as e:
+      warnings.warn(
+        "Request parameters do not validate against schema definition:\n{e}".format(e=e),
+        UserWarning,
+      )
     return self._requestComputerPartition(request_dict)
 
   def getInformation(self, partition_reference):

slapos/tests/test_cli.py

@@ -49,10 +49,12 @@ import slapos.cli.computer_info
 import slapos.cli.computer_list
 import slapos.cli.computer_token
 import slapos.cli.supervisorctl
+import slapos.cli.request
 from slapos.cli.proxy_show import do_show, StringIO
 from slapos.cli.cache import do_lookup as cache_do_lookup
 from slapos.cli.cache_source import do_lookup as cache_source_do_lookup
 from slapos.client import ClientConfig
+from slapos.slap import SoftwareProductCollection
 import slapos.grid.svcbackend
 import slapos.proxy
 import slapos.slap
@@ -65,8 +67,8 @@ def raiseNotFoundError(*args, **kwargs):
 class CliMixin(unittest.TestCase):
   def setUp(self):
     slap = slapos.slap.slap()
-    self.local = {'slap': slap}
     self.logger = create_autospec(logging.Logger)
+    self.local = {'slap': slap, 'product': SoftwareProductCollection(self.logger, slap)}
     self.conf = create_autospec(ClientConfig)
 
 class TestCliCache(CliMixin):
@@ -641,3 +643,125 @@ class TestCliComplete(CliMixin):
     with patch.object(sys, 'stdout', StringIO()) as app_stdout:
       self.assertEqual(slapos.cli.entry.SlapOSApp().run(['complete', '--shell=fish']), 0)
     self.assertIn('__fish_seen_subcommand_from', app_stdout.getvalue())
+
+
+class TestCliRequest(CliMixin):
+  def test_parse_option_dict(self):
+    parse_option_dict = slapos.cli.request.parse_option_dict
+
+    self.assertEqual(parse_option_dict(['foo=bar', 'a=b']), {'foo': 'bar', 'a': 'b'})
+    # malformed option = assignment
+    self.assertRaises(ValueError, parse_option_dict, ['a'])
+    # duplicated key
+    self.assertRaises(ValueError, parse_option_dict, ['a=b', 'a=c'])
+    # corner cases
+    self.assertEqual(parse_option_dict(['a=a=b']), {'a': 'a=b'})
+    self.assertEqual(parse_option_dict(['a=a\nb']), {'a': 'a\nb'})
+    self.assertEqual(parse_option_dict([]), {})
+
+  def test_request(self, ):
+    self.conf.reference = 'instance reference'
+    self.conf.software_url = 'software URL'
+    self.conf.parameters = {'key': 'value'}
+    self.conf.parameters_file = None
+    self.conf.node = {'computer_guid': 'COMP-1234'}
+    self.conf.type = None
+    self.conf.state = None
+    self.conf.slave = False
+
+    with patch.object(
+        slapos.slap.slap,
+        'registerOpenOrder',
+        return_value=mock.create_autospec(slapos.slap.OpenOrder)) as registerOpenOrder:
+      slapos.cli.request.do_request(self.logger, self.conf, self.local)
+
+    registerOpenOrder().request.assert_called_once_with(
+        software_release='software URL',
+        partition_reference='instance reference',
+        partition_parameter_kw={'key': 'value'},
+        software_type=None,
+        filter_kw={'computer_guid': 'COMP-1234'},
+        state=None,
+        shared=False,
+    )
+
+    self.logger.info.assert_any_call(
+        'Requesting %s as instance of %s...',
+        'instance reference',
+        'software URL',
+    )
+
+
+class TestCliRequestParametersFileJson(CliMixin):
+  expected_partition_parameter_kw = {'foo': ['bar']}
+
+  def _makeParameterFile(self):
+    f = tempfile.NamedTemporaryFile(suffix='.yaml', mode='w', delete=False)
+    f.write(textwrap.dedent('''\
+      foo:
+      - bar
+    '''))
+    f.flush()
+    self.addCleanup(os.unlink, f.name)
+    return f.name
+
+  def test_request_parameters_file(self):
+    self.conf.reference = 'instance reference'
+    self.conf.software_url = 'software URL'
+    self.conf.parameters =  None
+    f = open(self._makeParameterFile())
+    self.addCleanup(f.close)
+    self.conf.parameters_file = f
+    self.conf.node = {'computer_guid': 'COMP-1234'}
+    self.conf.type = None
+    self.conf.state = None
+    self.conf.slave = False
+
+    with patch.object(
+        slapos.slap.slap,
+        'registerOpenOrder',
+        return_value=mock.create_autospec(slapos.slap.OpenOrder)) as registerOpenOrder:
+      slapos.cli.request.do_request(self.logger, self.conf, self.local)
+
+    registerOpenOrder().request.assert_called_once_with(
+        software_release='software URL',
+        partition_reference='instance reference',
+        partition_parameter_kw=self.expected_partition_parameter_kw,
+        software_type=None,
+        filter_kw={'computer_guid': 'COMP-1234'},
+        state=None,
+        shared=False,
+    )
+    self.logger.info.assert_any_call(
+          'Requesting %s as instance of %s...',
+          'instance reference',
+          'software URL',
+    )
+
+
+class TestCliRequestParametersFileYaml(TestCliRequestParametersFileJson):
+  def _makeParameterFile(self):
+    f = tempfile.NamedTemporaryFile(suffix='.yaml', mode='w', delete=False)
+    f.write(textwrap.dedent('''\
+    {
+      "foo": ["bar"]
+    }
+    '''))
+    f.flush()
+    self.addCleanup(os.unlink, f.name)
+    return f.name
+
+
+class TestCliRequestParametersFileXml(TestCliRequestParametersFileJson):
+  expected_partition_parameter_kw = {'foo': 'bar'}
+  def _makeParameterFile(self):
+    f = tempfile.NamedTemporaryFile(suffix='.xml', mode='w', delete=False)
+    f.write(textwrap.dedent('''\
+      <?xml version="1.0" encoding="utf-8"?>
+      <instance>
+          <parameter id="foo">bar</parameter>
+      </instance>
+    '''))
+    f.flush()
+    self.addCleanup(os.unlink, f.name)
+    return f.name

slapos/util.py

@@ -27,19 +27,32 @@
 #
 ##############################################################################
 
+import enum
 import errno
+import hashlib
+import json
 import os
+import shutil
 import socket
+import sqlite3
 import struct
 import subprocess
-import sqlite3
-from xml_marshaller.xml_marshaller import Marshaller, Unmarshaller
-from lxml import etree
+import warnings
+
+import jsonschema
+import netaddr
+import requests
 import six
+from lxml import etree
 from six.moves.urllib import parse
-import hashlib
-import netaddr
-import shutil
+from six.moves.urllib_parse import urljoin
+from xml_marshaller.xml_marshaller import Marshaller, Unmarshaller
+
+try:
+  from typing import Dict, Optional, IO
+except ImportError:
+  pass
+
 
 
 try:
@@ -295,3 +308,143 @@ def rmtree(path):
     raise e  # XXX make pylint happy
 
   shutil.rmtree(path, onerror=chmod_retry)
+
+
+
+def _readAsJson(url):
+  # type: (str) -> Optional[Dict]
+  """Reads and parse the json file located at `url`.
+
+  `url` can also be the path of a local file.
+  """
+  if url.startswith('file://'):
+    url = url[len('file://'):]
+  path = url if os.path.exists(url) else None
+
+  if path:
+    with open(path) as f:
+      try:
+        return json.load(f)
+      except ValueError:
+        return None
+  if url.startswith('http://') or url.startswith('https://'):
+    try:
+      r = requests.get(url)
+      r.raise_for_status()
+      return r.json()
+    except (requests.exceptions.HTTPError, ValueError):
+      return None
+  return None
+
+
+class SoftwareReleaseSerialisation(str, enum.Enum):
+  Xml = 'xml'
+  JsonInXml = 'json-in-xml'
+
+
+class SoftwareReleaseSchema(object):
+  def __init__(self, software_url, software_type):
+    # type: (str, Optional[str]) ->  None
+    self.software_url = software_url
+    self.software_type = software_type
+
+  def getSoftwareSchema(self):
+    # type: () -> Optional[Dict]
+    """Returns the schema for this software.
+    """
+    return _readAsJson(self.software_url + '.json')
+
+  def getSoftwareTypeSchema(self):
+    # type: () -> Optional[Dict]
+    """Returns schema for this software type.
+    """
+    software_schema = self.getSoftwareSchema()
+    if software_schema is None:
+      return None
+
+    software_type = self.software_type
+    from slapos.slap.slap import DEFAULT_SOFTWARE_TYPE # TODO
+    if software_type is None:
+      software_type = DEFAULT_SOFTWARE_TYPE
+    # XXX some software are using "default" for default software type
+    if software_type == DEFAULT_SOFTWARE_TYPE \
+        and software_type not in software_schema['software-type'] \
+        and 'default' in software_schema['software-type']:
+      warnings.warn(
+          "Software release {} does not have schema for DEFAULT_SOFTWARE_TYPE but has one for 'default'."
+          " Using 'default' instead.".format(self.software_url),
+          UserWarning,
+      )
+      software_type = 'default'
+    return software_schema['software-type'].get(software_type) 
+
+  def getSerialisation(self):
+    # type: () -> Optional[SoftwareReleaseSerialisation]
+    """Returns the serialisation method used for parameters.
+    """
+    software_schema = self.getSoftwareSchema()
+    if software_schema is None:
+      return None
+    return SoftwareReleaseSerialisation(software_schema['serialisation'])
+
+  def getInstanceRequestParameterSchemaURL(self):
+    # type: () -> Optional[str]
+    """Returns the URL of the schema defining instance parameters.
+    """
+    software_type_schema = self.getSoftwareTypeSchema()
+    if software_type_schema is None:
+      return None
+    software_url = self.software_url
+    if os.path.exists(software_url):
+      software_url = 'file://' + software_url
+    return urljoin(software_url, software_type_schema['request'])
+ 
+  def getInstanceRequestParameterSchema(self):
+    # type: () -> Optional[Dict]
+    """Returns the schema defining instance parameters.
+    """
+    instance_parameter_schema_url = self.getInstanceRequestParameterSchemaURL()
+    if instance_parameter_schema_url is None:
+      return None
+    schema = _readAsJson(instance_parameter_schema_url)
+    if schema:
+      # so that jsonschema knows how to resolve references
+      schema.setdefault('$id', instance_parameter_schema_url)
+    return schema
+
+  def getInstanceConnectionParameterSchemaURL(self):
+    # type: () -> Optional[str]
+    """Returns the URL of the schema defining connection parameters published by the instance.
+    """
+    software_type_schema = self.getSoftwareTypeSchema()
+    if software_type_schema is None:
+      return None
+    return urljoin(self.software_url, software_type_schema['response'])
+
+  def getInstanceConnectionParameterSchema(self):
+    # type: () -> Optional[Dict]
+    """Returns the schema defining connection parameters published by the instance.
+    """
+    instance_parameter_schema_url = self.getInstanceConnectionParameterSchemaURL()
+    if instance_parameter_schema_url is None:
+      return None
+    schema = _readAsJson(instance_parameter_schema_url)
+    if schema:
+      # so that jsonschema knows how to resolve references
+      schema.setdefault('$id', instance_parameter_schema_url)
+    return schema
+
+  def validateInstanceParameterDict(self, parameter_dict):
+    # type: (Dict) -> None
+    """Validate instance parameters against the software schema.
+
+    Raise jsonschema.ValidationError if parameters does not validate.
+    """
+    schema_url = self.getInstanceRequestParameterSchemaURL()
+    if schema_url:
+      instance = parameter_dict if self.getSerialisation() == SoftwareReleaseSerialisation.Xml else json.loads(parameter_dict['_'])
+      instance.pop('$schema', None)
+      jsonschema.validate(
+          instance=instance,
+          schema=self.getInstanceRequestParameterSchema(),
+      )