Merge "Support JSON data for port binding profile"

2 files changed, 128 insertions, 6 deletions

diff --git a/openstackclient/network/v2/port.py b/openstackclient/network/v2/port.py
index 57461f89..5d1431b5 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):