From 8c841946d441d6c90ba61ee0a64698b59c711c3a Mon Sep 17 00:00:00 2001 From: "Frank A. Zdarsky" Date: Thu, 5 Jan 2017 12:55:55 +0100 Subject: Add check that ovs_extra is passed as list This patch adds a check that ensures the ovs_extra option, if present, is passed in as list and raises an InvalidConfigException if not. It addresses the issue that a user may mistakingly pass the value as string, which would cause an error later when appending the failure mode or when formatting the ovs_extra parameter. Note: Also fixes a sample file in which ovs_extra was passed as string. Change-Id: I9e8e47390b63d284de10d27b1db2c2cc54c86924 Closes-Bug: #1654196 --- etc/os-net-config/samples/ovs_patch_port.yaml | 2 +- os_net_config/objects.py | 25 ++++++++++++++++++++++-- os_net_config/tests/test_objects.py | 28 +++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 3 deletions(-) diff --git a/etc/os-net-config/samples/ovs_patch_port.yaml b/etc/os-net-config/samples/ovs_patch_port.yaml index 91858be..bae8880 100644 --- a/etc/os-net-config/samples/ovs_patch_port.yaml +++ b/etc/os-net-config/samples/ovs_patch_port.yaml @@ -10,7 +10,7 @@ network_config: # force the MAC address of the bridge to this interface primary: true mtu: 1500 - ovs_extra: "br-set-external-id br-ctlplane bridge-id br-ctlplane" + ovs_extra: ["br-set-external-id br-ctlplane bridge-id br-ctlplane"] - type: ovs_patch_port name: br_pub-patch diff --git a/os_net_config/objects.py b/os_net_config/objects.py index 741f304..e3d8978 100644 --- a/os_net_config/objects.py +++ b/os_net_config/objects.py @@ -439,7 +439,10 @@ class OvsBridge(_BaseOpts): dhclient_args, dns_servers) = _BaseOpts.base_opts_from_json( json, include_primary=False) ovs_options = json.get('ovs_options') - ovs_extra = json.get('ovs_extra') + ovs_extra = json.get('ovs_extra', []) + if not isinstance(ovs_extra, list): + msg = 'ovs_extra must be a list.' + raise InvalidConfigException(msg) fail_mode = json.get('ovs_fail_mode', DEFAULT_OVS_BRIDGE_FAIL_MODE) members = [] @@ -504,7 +507,10 @@ class OvsUserBridge(_BaseOpts): dhclient_args, dns_servers) = _BaseOpts.base_opts_from_json( json, include_primary=False) ovs_options = json.get('ovs_options') - ovs_extra = json.get('ovs_extra') + ovs_extra = json.get('ovs_extra', []) + if not isinstance(ovs_extra, list): + msg = 'ovs_extra must be a list.' + raise InvalidConfigException(msg) fail_mode = json.get('ovs_fail_mode', DEFAULT_OVS_BRIDGE_FAIL_MODE) members = [] @@ -860,6 +866,9 @@ class OvsBond(_BaseOpts): json, include_primary=False) ovs_options = json.get('ovs_options') ovs_extra = json.get('ovs_extra', []) + if ovs_extra and not isinstance(ovs_extra, list): + msg = 'ovs_extra must be a list.' + raise InvalidConfigException(msg) members = [] # members @@ -906,6 +915,9 @@ class OvsTunnel(_BaseOpts): ovs_options = json.get('ovs_options', []) ovs_options = ['options:%s' % opt for opt in ovs_options] ovs_extra = json.get('ovs_extra', []) + if not isinstance(ovs_extra, list): + msg = 'ovs_extra must be a list.' + raise InvalidConfigException(msg) opts = _BaseOpts.base_opts_from_json(json) return OvsTunnel(name, *opts, tunnel_type=tunnel_type, ovs_options=ovs_options, ovs_extra=ovs_extra) @@ -940,6 +952,9 @@ class OvsPatchPort(_BaseOpts): ovs_options = json.get('ovs_options', []) ovs_options = ['options:%s' % opt for opt in ovs_options] ovs_extra = json.get('ovs_extra', []) + if not isinstance(ovs_extra, list): + msg = 'ovs_extra must be a list.' + raise InvalidConfigException(msg) opts = _BaseOpts.base_opts_from_json(json) return OvsPatchPort(name, *opts, bridge_name=bridge_name, peer=peer, ovs_options=ovs_options, ovs_extra=ovs_extra) @@ -1023,6 +1038,9 @@ class OvsDpdkPort(_BaseOpts): ovs_options = json.get('ovs_options', []) ovs_options = ['options:%s' % opt for opt in ovs_options] ovs_extra = json.get('ovs_extra', []) + if not isinstance(ovs_extra, list): + msg = 'ovs_extra must be a list.' + raise InvalidConfigException(msg) opts = _BaseOpts.base_opts_from_json(json) return OvsDpdkPort(name, *opts, members=members, driver=driver, ovs_options=ovs_options, ovs_extra=ovs_extra) @@ -1067,6 +1085,9 @@ class OvsDpdkBond(_BaseOpts): json, include_primary=False) ovs_options = json.get('ovs_options') ovs_extra = json.get('ovs_extra', []) + if not isinstance(ovs_extra, list): + msg = 'ovs_extra must be a list.' + raise InvalidConfigException(msg) members = [] # members diff --git a/os_net_config/tests/test_objects.py b/os_net_config/tests/test_objects.py index 23a3bbe..200d7b7 100644 --- a/os_net_config/tests/test_objects.py +++ b/os_net_config/tests/test_objects.py @@ -295,6 +295,34 @@ class TestBridge(base.TestCase): self.assertTrue(interface2.ovs_port) self.assertEqual("br-foo", interface2.bridge_name) + def test_from_json_ovs_extra(self): + data = """{ +"type": "ovs_bridge", +"name": "br-foo", +"ovs_extra": ["bar"], +"ovs_fail_mode": "standalone" +} +""" + bridge = objects.object_from_json(json.loads(data)) + self.assertTrue(2 == len(bridge.ovs_extra)) + self.assertEqual("bar", bridge.ovs_extra[0]) + self.assertEqual("set bridge br-foo fail_mode=standalone", + bridge.ovs_extra[1]) + + def test_from_json_ovs_extra_invalid(self): + data = """{ +"type": "ovs_bridge", +"name": "br-foo", +"ovs_extra": "bar", +"ovs_fail_mode": "standalone" +} +""" + json_data = json.loads(data) + err = self.assertRaises(objects.InvalidConfigException, + objects.object_from_json, json_data) + expected = 'ovs_extra must be a list.' + self.assertIn(expected, six.text_type(err)) + class TestLinuxBridge(base.TestCase): -- cgit 1.2.3-korg