Add support for type and userConfigurable specification in edit-prope…

…rties (#50)

CHANGELOG.rst

@@ -9,6 +9,16 @@ This project adheres to `Semantic Versioning`_.
 **Added**
 
 - Support for Python 3.5
+- Enhancements to ``cot edit-properties`` (`#50`_):
+
+  - Added ``--user-configurable`` option to set whether created/updated
+    properties are marked as user-configurable in the OVF.
+  - It's now valid to set no default value for a property by
+    omitting the ``=value``, as in ``-p property-with-no-value``, as well as
+    the existing ``-p property-with-empty-value=`` syntax to set
+    an empty string as the value.
+  - Users can now optionally specify the property type to enforce for each
+    property by using the delimiter ``+type``, as in ``-p key=1+boolean``.
 
 **Changed**
 
@@ -468,6 +478,7 @@ Initial public release.
 .. _#47: https://github.com/glennmatthews/cot/issues/47
 .. _#48: https://github.com/glennmatthews/cot/issues/48
 .. _#49: https://github.com/glennmatthews/cot/issues/49
+.. _#50: https://github.com/glennmatthews/cot/issues/50
 
 .. _Semantic Versioning: http://semver.org/
 .. _`PEP 8`: https://www.python.org/dev/peps/pep-0008/

COT/data_validation.py

@@ -46,6 +46,7 @@
   positive_int
   to_string
   validate_int
+  truth_value
 
 **Constants**
 
@@ -55,6 +56,7 @@
 
 import xml.etree.ElementTree as ET
 import re
+from distutils.util import strtobool
 
 
 def to_string(obj):
@@ -311,6 +313,21 @@ def positive_int(string):
     return validate_int(string, minimum=1)
 
 
+def truth_value(value):
+    """Parser helper function for truth values like '0', 'y', or 'false'."""
+    if isinstance(value, bool):
+        return value
+    try:
+        return strtobool(value)
+    except ValueError:
+        raise ValueUnsupportedError(
+            "truth value",
+            value,
+            ['y', 'yes', 't', 'true', 'on', 1,
+             'n', 'no', 'f', 'false', 'off', 0]
+        )
+
+
 # Some handy exception and error types we can throw
 class ValueMismatchError(ValueError):
     """Values which were expected to be equal turned out to be not equal."""

COT/edit_properties.py

@@ -24,12 +24,16 @@
   COTEditProperties
 """
 
+import argparse
 import logging
 import os.path
+import re
 import textwrap
 
 from .submodule import COTSubmodule
-from .data_validation import ValueUnsupportedError, InvalidInputError
+from .data_validation import (
+    truth_value, ValueUnsupportedError, InvalidInputError
+)
 
 logger = logging.getLogger(__name__)
 
@@ -45,7 +49,8 @@ class COTEditProperties(COTSubmodule):
     Attributes:
     :attr:`config_file`,
     :attr:`properties`,
-    :attr:`transports`
+    :attr:`transports`,
+    :attr:`user_configurable`
     """
 
     def __init__(self, ui):
@@ -54,6 +59,8 @@ def __init__(self, ui):
         self._config_file = None
         self._properties = {}
         self._transports = None
+        self.user_configurable = None
+        """Value to set the user_configurable flag on properties we edit."""
 
     @property
     def config_file(self):
@@ -72,23 +79,27 @@ def config_file(self, value):
 
     @property
     def properties(self):
-        """List of property (key, value) tuples to update."""
+        """List of property (key, value, type) tuples to update."""
         return self._properties
 
     @properties.setter
     def properties(self, value):
         new_value = []
-        for key_value_pair in value:
-            try:
-                (k, v) = key_value_pair.split('=', 1)
-                logger.debug("key: %s value: %s", k, v)
-                if k == '':
-                    raise ValueError()
-                new_value.append((k, v))
-            except ValueError:
+        for prop in value:
+            match = re.match(r"^([^=+]+?)(=[^=+]*?)?(\+[^=+]*?)?$", prop)
+            if not match:
                 raise InvalidInputError("Invalid property '{0}' - properties "
-                                        "must be in 'key=value' form"
-                                        .format(key_value_pair))
+                                        "must be in 'key[=value][+type]' form"
+                                        .format(prop))
+            key = match.group(1)
+            # Strip the leading '=' or '+' from these matches
+            value = match.group(2)[1:] if match.group(2) else None
+            prop_type = match.group(3)[1:] if match.group(3) else None
+
+            logger.verbose("Property: key '{0}', value '{1}', type '{2}'"
+                           .format(key, value, prop_type))
+
+            new_value.append((key, value, prop_type))
         self._properties = new_value
 
     @property
@@ -123,10 +134,11 @@ def run(self):
         super(COTEditProperties, self).run()
 
         if self.config_file is not None:
-            self.vm.config_file_to_properties(self.config_file)
+            self.vm.config_file_to_properties(self.config_file,
+                                              self.user_configurable)
 
         if self.properties:
-            for key, value in self.properties:
+            for key, value, prop_type in self.properties:
                 if value == '':
                     value = self.UI.get_input(
                         "Enter value for property '{0}'",
@@ -136,8 +148,10 @@ def run(self):
                     self.UI.confirm_or_die(
                         "Property '{0}' does not yet exist.\n"
                         "Create it?".format(key))
-                    # TODO - for new property, prompt for label/descr/type?
-                self.vm.set_property_value(key, value)
+                self.vm.set_property_value(
+                    key, value,
+                    user_configurable=self.user_configurable,
+                    property_type=prop_type)
 
         if self.transports:
             self.vm.environment_transports = self.transports
@@ -195,7 +209,9 @@ def edit_properties_interactive(self):
                     break
                 else:
                     try:
-                        new_value = self.vm.set_property_value(key, new_value)
+                        new_value = self.vm.set_property_value(
+                            key, new_value,
+                            user_configurable=self.user_configurable)
                         logger.info("Successfully updated property '%s' "
                                     "value to '%s'", key, new_value)
                         # Refresh!
@@ -212,17 +228,30 @@ def create_subparser(self):
             'edit-properties',
             aliases=['set-properties', 'edit-environment', 'set-environment'],
             add_help=False,
-            help="""Edit environment properties of an OVF""",
+            help="""Edit or create environment properties of an OVF""",
             usage=self.UI.fill_usage("edit-properties", [
-                "PACKAGE [-p KEY1=VALUE1 [KEY2=VALUE2 ...]] [-c CONFIG_FILE] "
+                "PACKAGE [-p KEY1=VALUE1 ] [-p KEY2=VALUE2 ...] "
+                "[-c CONFIG_FILE] [-u [USER_CONFIGURABLE]] "
                 "[-t TRANSPORT [TRANSPORT2 ...]] [-o OUTPUT]",
                 "PACKAGE [-o OUTPUT]",
             ]),
+            formatter_class=argparse.RawDescriptionHelpFormatter,
             description="""
 Configure environment properties of the given OVF or OVA. The user may specify
-key-value pairs as command-line arguments or may provide a config-file to
-read from. If no arguments (other than optionally --output) are specified,
-the program will run interactively.""")
+keys and values as command-line arguments or may provide a config-file to
+read from. If neither --config-file, --properties, nor --transport are given,
+the program will run interactively.""",
+            epilog=self.UI.fill_examples([
+                ("Add configuration from a text file and mark the resulting"
+                 " properties as non-user-configurable.",
+                 'cot edit-properties input.ovf -c config.txt -u=0'),
+                ("Add/update two properties, one a string with no default"
+                 " value and the other a boolean defaulting to true, and"
+                 " mark both properties as user-configurable.",
+                 'cot edit-properties input.ovf -p string-property+string'
+                 ' -p bool-property=true+boolean --user-configurable'),
+            ]),
+        )
 
         p.add_argument('PACKAGE',
                        help="""OVF descriptor or OVA file to edit""")
@@ -237,12 +266,20 @@ def create_subparser(self):
 
         g = p.add_argument_group("property setting options")
 
+        g.add_argument('-u', '--user-configurable',
+                       nargs='?', const="true", type=truth_value,
+                       help="Update the 'userConfigurable' flag on all "
+                       "edited properties to True or the given value")
+
         g.add_argument('-c', '--config-file',
                        help="Read configuration CLI from this text file and "
                        "generate generic properties for each line of CLI")
         g.add_argument('-p', '--properties', action='append', nargs='+',
-                       metavar=('KEY1=VALUE1', 'KEY2=VALUE2'),
-                       help="Set the given property key-value pair(s). "
+                       metavar=('KEY1[=VALUE1][+TYPE1]', 'K2[=V2][+T2]'),
+                       help="Update or create the given property keys. "
+                       "A '=' delimits the optional value to set this key to. "
+                       "A '+' delimits the optional type to enforce for this "
+                       "key. "
                        "This argument may be repeated as needed to specify "
                        "multiple properties to edit.")
         g.add_argument('-t', '--transports', action='append', nargs='+',

COT/ovf/name_helper.py

@@ -190,6 +190,7 @@ class OVFNameHelper1(object):
         PROP_VALUE=_Tag('ovf', 'value'),
         PROP_QUAL=_Tag('ovf', 'qualifiers'),
         PROP_TYPE=_Tag('ovf', 'type'),
+        PROP_USER_CONFIGABLE=_Tag('ovf', 'userConfigurable'),
 
         # Property sub-elements
         PROPERTY_LABEL=_Tag('ovf', 'Label'),
@@ -358,6 +359,7 @@ class OVFNameHelper0(OVFNameHelper1):
         VIRTUAL_SYSTEM=_Tag('ovf', 'Content'),
         PRODUCT_SECTION=_Tag('ovf', 'Section'),
         PROP_VALUE=_Tag('ovf', 'defaultValue'),
+        PROP_USER_CONFIGABLE=_Tag('ovf', 'configurableByUser'),
         EULA_SECTION=_Tag('ovf', 'Section'),
         VIRTUAL_HW_SECTION=_Tag('ovf', 'Section'),
     )

COT/ovf/ovf.py

@@ -548,7 +548,7 @@ def environment_properties(self):
 
         :return: Array of dicts (one per property) with the keys
           ``"key"``, ``"value"``, ``"qualifiers"``, ``"type"``,
-          ``"label"``, and ``"description"``.
+          ``"user_configurable"``, ``"label"``, and ``"description"``.
         """
         result = []
         if self.ovf_version < 1.0 or self.product_section is None:
@@ -562,6 +562,7 @@ def environment_properties(self):
                 'value': elem.get(self.PROP_VALUE),
                 'qualifiers': elem.get(self.PROP_QUAL, ""),
                 'type': elem.get(self.PROP_TYPE, ""),
+                'user_configurable': elem.get(self.PROP_USER_CONFIGABLE, ""),
                 'label': label,
                 'description': descr,
             })
@@ -1659,11 +1660,15 @@ def _validate_value_for_property(self, prop, value):
 
         return value
 
-    def set_property_value(self, key, value):
+    def set_property_value(self, key, value,
+                           user_configurable=None, property_type=None):
         """Set the value of the given property (converting value if needed).
 
         :param str key: Property identifier
-        :param str value: Value to set for this property
+        :param value: Value to set for this property
+        :param bool user_configurable: Should this property be configurable at
+            deployment time by the user?
+        :param str property_type: Value type - 'string' or 'boolean'
         :return: the (converted) value that was set.
         """
         if self.ovf_version < 1.0:
@@ -1679,25 +1684,33 @@ def set_property_value(self, key, value):
         prop = self.find_child(self.product_section, self.PROPERTY,
                                attrib={self.PROP_KEY: key})
         if prop is None:
-            self.set_or_make_child(self.product_section, self.PROPERTY,
-                                   attrib={self.PROP_KEY: key,
-                                           self.PROP_VALUE: value,
-                                           self.PROP_TYPE: 'string'})
-            return value
+            prop = self.set_or_make_child(self.product_section, self.PROPERTY,
+                                          attrib={self.PROP_KEY: key})
+            # Properties *must* have a type to be valid
+            if property_type is None:
+                property_type = 'string'
+
+        if user_configurable is not None:
+            prop.set(self.PROP_USER_CONFIGABLE, str(user_configurable).lower())
+        if property_type is not None:
+            prop.set(self.PROP_TYPE, property_type)
+
+        if value is not None:
+            # Make sure the requested value is valid
+            value = self._validate_value_for_property(prop, value)
+            prop.set(self.PROP_VALUE, value)
 
-        # Else, make sure the requested value is valid
-        value = self._validate_value_for_property(prop, value)
-
-        prop.set(self.PROP_VALUE, value)
         return value
 
-    def config_file_to_properties(self, file_path):
+    def config_file_to_properties(self, file_path, user_configurable=None):
         """Import each line of a text file into a configuration property.
 
         :raise NotImplementedError: if the :attr:`platform` for this OVF
           does not define
           :const:`~COT.platforms.GenericPlatform.LITERAL_CLI_STRING`
         :param str file_path: File name to import.
+        :param bool user_configurable: Should the properties be configurable at
+            deployment time by the user?
         """
         i = 0
         if not self.platform.LITERAL_CLI_STRING:
@@ -1712,7 +1725,8 @@ def config_file_to_properties(self, file_path):
                 i += 1
                 self.set_property_value(
                     "{0}-{1:04d}".format(self.platform.LITERAL_CLI_STRING, i),
-                    line)
+                    line,
+                    user_configurable)
 
     def convert_disk_if_needed(self, file_path, kind):
         """Convert the disk to a more appropriate format if needed.

COT/tests/test_cli.py

@@ -326,7 +326,7 @@ def test_help(self):
     edit-hardware   Edit virtual machine hardware properties of an OVF
     edit-product    Edit product info in an OVF
     edit-properties
-                    Edit environment properties of an OVF
+                    Edit or create environment properties of an OVF
     help            Print help for a command
     info            Generate a description of an OVF package
     inject-config   Inject a configuration file into an OVF package
@@ -358,7 +358,7 @@ def test_help(self):
     edit-product (set-product, set-version)
                         Edit product info in an OVF
     edit-properties (set-properties, edit-environment, set-environment)
-                        Edit environment properties of an OVF
+                        Edit or create environment properties of an OVF
     help                Print help for a command
     info (describe)     Generate a description of an OVF package
     inject-config (add-bootstrap)
@@ -658,12 +658,18 @@ def test_invalid_args(self):
         self.call_cot(['edit-properties', self.input_ovf, '--config-file',
                        '/foo'], result=2)
         # Bad input format
-        self.call_cot(['edit-properties', self.input_ovf, '--properties', 'x'],
-                      result=2)
         self.call_cot(['edit-properties', self.input_ovf, '--properties', '='],
                       result=2)
         self.call_cot(['edit-properties', self.input_ovf, '--properties',
                        '=foo'], result=2)
+        self.call_cot(['edit-properties', self.input_ovf, '--properties', '+'],
+                      result=2)
+        self.call_cot(['edit-properties', self.input_ovf, '-p', '+string'],
+                      result=2)
+        self.call_cot(['edit-properties', self.input_ovf, '-p', '=foo+string'],
+                      result=2)
+        self.call_cot(['edit-properties', self.input_ovf,
+                       '--user-configurable', 'foobar'], result=2)
 
     def test_set_property_valid(self):
         """Variant property setting syntax, exercising CLI nargs/append."""