Support JSON data for port binding profile

Update the "--binding-profile" option on the "port create" and
"port set" commands to support both <key>=<value> and JSON input
for the port custom binding profile data. The JSON input is
sometimes needed to maintain the value type (e.g. integer) for
more advanced data.

The port custom binding profile data is unique across neutron
so a custom argparse.Action class was created instead of
writting a generic class in osc-lib.

Change-Id: I82ac6d4f95afdc866f5282fc00d390f850f54d21
Implements: blueprint neutron-client

2 files changed, 128 insertions, 6 deletions

diff --git a/openstackclient/network/v2/port.py b/openstackclient/network/v2/port.py
index 1c5db706..a25bdf8d 100644
--- a/openstackclient/network/v2/port.py
+++ b/openstackclient/network/v2/port.py
@@ -14,6 +14,7 @@
 """Port action implementations"""
 
 import argparse
+import json
 import logging
 
 from osc_lib.cli import parseractions
@@ -63,6 +64,32 @@ def _get_columns(item):
     return tuple(sorted(columns))
 
 
+class JSONKeyValueAction(argparse.Action):
+    """A custom action to parse arguments as JSON or key=value pairs
+
+    Ensures that ``dest`` is a dict
+    """
+
+    def __call__(self, parser, namespace, values, option_string=None):
+
+        # Make sure we have an empty dict rather than None
+        if getattr(namespace, self.dest, None) is None:
+            setattr(namespace, self.dest, {})
+
+        # Try to load JSON first before falling back to <key>=<value>.
+        current_dest = getattr(namespace, self.dest)
+        try:
+            current_dest.update(json.loads(values))
+        except ValueError as e:
+            if '=' in values:
+                current_dest.update([values.split('=', 1)])
+            else:
+                msg = _("Expected '<key>=<value>' or JSON data for option "
+                        "%(option)s, but encountered JSON parsing error: "
+                        "%(error)s") % {"option": option_string, "error": e}
+                raise argparse.ArgumentTypeError(msg)
+
+
 def _get_attrs(client_manager, parsed_args):
     attrs = {}
 
@@ -219,9 +246,9 @@ class CreatePort(command.ShowOne):
         parser.add_argument(
             '--binding-profile',
             metavar='<binding-profile>',
-            action=parseractions.KeyValueAction,
-            help=_("Custom data to be passed as binding:profile: "
-                   "<key>=<value> "
+            action=JSONKeyValueAction,
+            help=_("Custom data to be passed as binding:profile. Data may "
+                   "be passed as <key>=<value> or JSON. "
                    "(repeat option to set multiple binding:profile data)")
         )
         admin_group = parser.add_mutually_exclusive_group()
@@ -390,9 +417,9 @@ class SetPort(command.Command):
         binding_profile.add_argument(
             '--binding-profile',
             metavar='<binding-profile>',
-            action=parseractions.KeyValueAction,
-            help=_("Custom data to be passed as binding:profile: "
-                   "<key>=<value> "
+            action=JSONKeyValueAction,
+            help=_("Custom data to be passed as binding:profile. Data may "
+                   "be passed as <key>=<value> or JSON. "
                    "(repeat option to set multiple binding:profile data)")
         )
         binding_profile.add_argument(
diff --git a/openstackclient/tests/network/v2/test_port.py b/openstackclient/tests/network/v2/test_port.py
index 779dca05..a998585e 100644
--- a/openstackclient/tests/network/v2/test_port.py
+++ b/openstackclient/tests/network/v2/test_port.py
@@ -11,6 +11,7 @@
 #   under the License.
 #
 
+import argparse
 import mock
 
 from mock import call
@@ -174,6 +175,58 @@ class TestCreatePort(TestPort):
         self.assertEqual(ref_columns, columns)
         self.assertEqual(ref_data, data)
 
+    def test_create_invalid_json_binding_profile(self):
+        arglist = [
+            '--network', self._port.network_id,
+            '--binding-profile', '{"parent_name":"fake_parent"',
+            'test-port',
+        ]
+        self.assertRaises(argparse.ArgumentTypeError,
+                          self.check_parser,
+                          self.cmd,
+                          arglist,
+                          None)
+
+    def test_create_invalid_key_value_binding_profile(self):
+        arglist = [
+            '--network', self._port.network_id,
+            '--binding-profile', 'key',
+            'test-port',
+        ]
+        self.assertRaises(argparse.ArgumentTypeError,
+                          self.check_parser,
+                          self.cmd,
+                          arglist,
+                          None)
+
+    def test_create_json_binding_profile(self):
+        arglist = [
+            '--network', self._port.network_id,
+            '--binding-profile', '{"parent_name":"fake_parent"}',
+            '--binding-profile', '{"tag":42}',
+            'test-port',
+        ]
+        verifylist = [
+            ('network', self._port.network_id,),
+            ('enable', True),
+            ('binding_profile', {'parent_name': 'fake_parent', 'tag': 42}),
+            ('name', 'test-port'),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        columns, data = (self.cmd.take_action(parsed_args))
+
+        self.network.create_port.assert_called_once_with(**{
+            'admin_state_up': True,
+            'network_id': self._port.network_id,
+            'binding:profile': {'parent_name': 'fake_parent', 'tag': 42},
+            'name': 'test-port',
+        })
+
+        ref_columns, ref_data = self._get_common_cols_data(self._port)
+        self.assertEqual(ref_columns, columns)
+        self.assertEqual(ref_data, data)
+
 
 class TestDeletePort(TestPort):
 
@@ -442,6 +495,48 @@ class TestSetPort(TestPort):
         self.network.update_port.assert_called_once_with(self._port, **attrs)
         self.assertIsNone(result)
 
+    def test_set_invalid_json_binding_profile(self):
+        arglist = [
+            '--binding-profile', '{"parent_name"}',
+            'test-port',
+        ]
+        self.assertRaises(argparse.ArgumentTypeError,
+                          self.check_parser,
+                          self.cmd,
+                          arglist,
+                          None)
+
+    def test_set_invalid_key_value_binding_profile(self):
+        arglist = [
+            '--binding-profile', 'key',
+            'test-port',
+        ]
+        self.assertRaises(argparse.ArgumentTypeError,
+                          self.check_parser,
+                          self.cmd,
+                          arglist,
+                          None)
+
+    def test_set_mixed_binding_profile(self):
+        arglist = [
+            '--binding-profile', 'foo=bar',
+            '--binding-profile', '{"foo2": "bar2"}',
+            self._port.name,
+        ]
+        verifylist = [
+            ('binding_profile', {'foo': 'bar', 'foo2': 'bar2'}),
+            ('port', self._port.name),
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        result = self.cmd.take_action(parsed_args)
+
+        attrs = {
+            'binding:profile': {'foo': 'bar', 'foo2': 'bar2'},
+        }
+        self.network.update_port.assert_called_once_with(self._port, **attrs)
+        self.assertIsNone(result)
+
 
 class TestShowPort(TestPort):