diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 632212e..5a83700 100644 --- a/CHANGELOG.rst +++ b/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/ diff --git a/COT/data_validation.py b/COT/data_validation.py index a746e4c..610553a 100644 --- a/COT/data_validation.py +++ b/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.""" diff --git a/COT/edit_properties.py b/COT/edit_properties.py index e21816a..cddac5f 100644 --- a/COT/edit_properties.py +++ b/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='+', diff --git a/COT/ovf/name_helper.py b/COT/ovf/name_helper.py index c843704..29e1edc 100644 --- a/COT/ovf/name_helper.py +++ b/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'), ) diff --git a/COT/ovf/ovf.py b/COT/ovf/ovf.py index 7bf2910..e24d53e 100644 --- a/COT/ovf/ovf.py +++ b/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. diff --git a/COT/tests/test_cli.py b/COT/tests/test_cli.py index 9b25f29..ea8e47a 100644 --- a/COT/tests/test_cli.py +++ b/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.""" diff --git a/COT/tests/test_data_validation.py b/COT/tests/test_data_validation.py index b8ec259..1eca7ec 100644 --- a/COT/tests/test_data_validation.py +++ b/COT/tests/test_data_validation.py @@ -23,13 +23,14 @@ except ImportError: import unittest -from COT.data_validation import canonicalize_helper, match_or_die -from COT.data_validation import canonicalize_nic_subtype, NIC_TYPES -from COT.data_validation import mac_address, device_address, no_whitespace -from COT.data_validation import validate_int, non_negative_int, positive_int -from COT.data_validation import InvalidInputError -from COT.data_validation import ValueMismatchError, ValueUnsupportedError -from COT.data_validation import ValueTooLowError, ValueTooHighError +from COT.data_validation import ( + match_or_die, + canonicalize_helper, canonicalize_nic_subtype, NIC_TYPES, + mac_address, device_address, no_whitespace, truth_value, + validate_int, non_negative_int, positive_int, + InvalidInputError, ValueMismatchError, ValueUnsupportedError, + ValueTooLowError, ValueTooHighError, +) class TestValidationFunctions(unittest.TestCase): @@ -139,6 +140,12 @@ def test_positive_int(self): self.assertEqual(positive_int("10"), 10) self.assertRaises(ValueTooLowError, positive_int, "0") + def test_truth_value(self): + """Test the truth_value() validator.""" + self.assertTrue(truth_value('y')) + self.assertFalse(truth_value('n')) + self.assertRaises(ValueUnsupportedError, truth_value, "foo") + def test_custom_error_attributes(self): """Test the properties of ValueUnsupportedError and its children.""" with self.assertRaises(ValueUnsupportedError) as cm: diff --git a/COT/tests/test_edit_properties.py b/COT/tests/test_edit_properties.py index e715f22..eacb6a8 100644 --- a/COT/tests/test_edit_properties.py +++ b/COT/tests/test_edit_properties.py @@ -86,14 +86,18 @@ def test_set_multiple_property_values(self): """) def test_create_property(self): - """Create a new property but do not set its value yet.""" + """Create new properties but do not set their values yet.""" self.instance.package = self.input_ovf - self.instance.properties = ["new-property-2="] + self.instance.properties = [ + "new-property-2=", # default value is empty string + "new-property-3", # no default value + ] self.instance.run() self.instance.finished() self.check_diff(""" + ++ """) @@ -110,6 +114,63 @@ def test_create_and_set_property(self): """) + def test_create_property_variants(self): + """Variant options for creating new properties.""" + self.instance.package = self.input_ovf + self.instance.properties = [ + "empty-property", + "property-with-value=value", + "prop-with-type+string", + "prop-with-value-and-type=yes+boolean", + ] + self.instance.run() + self.instance.finished() + self.check_diff(""" + ++ ++ ++ ++ + +""") + + def test_create_edit_and_user_configurable(self): + """Create new props, edit existing, and set user-configable flag.""" + self.instance.package = self.input_ovf + self.instance.properties = [ + 'new-property=false+boolean', + 'domain-name=example.com', + 'another-new=yep!', + 'enable-https-server+string', + ] + self.instance.user_configurable = False + self.instance.run() + self.instance.finished() + self.check_diff(""" + +- ++ + Enable HTTPS Server +... + +- ++ + Domain Name +... + ++ ++ + +""") + def test_load_config_file(self): """Inject a sequence of properties from a config file.""" self.instance.package = self.input_ovf @@ -136,6 +197,7 @@ def test_combined(self): "sample_cfg.txt") self.instance.properties = ["login-password=cisco123", "enable-ssh-server=1"] + self.instance.user_configurable = True self.instance.run() self.instance.finished() self.check_diff(""" @@ -157,12 +219,13 @@ def test_combined(self): ... + +ovf:userConfigurable="true" ovf:value="interface GigabitEthernet0/0/0/0" /> + +ovf:userConfigurable="true" ovf:value="no shutdown" /> + -+ +ovf:userConfigurable="true" ovf:value="interface Loopback0" /> ++ """) diff --git a/COT/vm_description.py b/COT/vm_description.py index b8f78fb..2d4b796 100644 --- a/COT/vm_description.py +++ b/COT/vm_description.py @@ -647,19 +647,25 @@ def get_property_value(self, key): """ raise NotImplementedError("get_property_value not implemented") - 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 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. """ raise NotImplementedError("set_property_value not implemented") - 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. :param str file_path: File name to import. + :param bool user_configurable: Should the properties be configurable at + deployment time by the user? """ raise NotImplementedError("config_file_to_properties not implemented")