From ae0b77a55ae0a96ab1944c1a23df1bc31a4a021c Mon Sep 17 00:00:00 2001 From: PanFengyun Date: Tue, 5 Jul 2016 14:35:55 +0800 Subject: [PATCH] Improve validation for the external network parameter 1. Check the network name to see if there is duplicate name. If so, advise user to use network ID. 2. External network cannot be private network, so filter the query for the external network with this attribute to validate. Change-Id: Ic02add0338cef344d02ef68c8372488fbe0b8fcd Closes-Bug: #1591018 --- magnum/api/attr_validator.py | 17 ++++++++++++++--- magnum/common/exception.py | 5 +++-- .../tests/unit/api/controllers/v1/test_bay.py | 2 +- .../unit/api/controllers/v1/test_baymodel.py | 6 ++++-- magnum/tests/unit/api/test_attr_validator.py | 14 +++++++++++++- 5 files changed, 35 insertions(+), 9 deletions(-) diff --git a/magnum/api/attr_validator.py b/magnum/api/attr_validator.py index 60f06f19fb..7a57b8e7ef 100644 --- a/magnum/api/attr_validator.py +++ b/magnum/api/attr_validator.py @@ -69,12 +69,23 @@ def validate_keypair(cli, keypair): def validate_external_network(cli, external_network): """Validate external network""" - networks = cli.neutron().list_networks() + count = 0 + ext_filter = {'router:external': True} + networks = cli.neutron().list_networks(**ext_filter) for net in networks.get('networks'): if (net.get('name') == external_network or net.get('id') == external_network): - return - raise exception.NetworkNotFound(network=external_network) + count = count + 1 + + if count == 0: + # Unable to find the external network. + # Or the network is private. + raise exception.ExternalNetworkNotFound(network=external_network) + + if count > 1: + msg = _("Multiple external networks exist with same name '%s'. " + "Please use the external network ID instead.") + raise exception.Conflict(msg % external_network) def validate_fixed_network(cli, fixed_network): diff --git a/magnum/common/exception.py b/magnum/common/exception.py index e110827c56..b52fb7175e 100644 --- a/magnum/common/exception.py +++ b/magnum/common/exception.py @@ -391,9 +391,10 @@ class FlavorNotFound(ResourceNotFound): code = 400 -class NetworkNotFound(ResourceNotFound): +class ExternalNetworkNotFound(ResourceNotFound): """The code here changed to 400 according to the latest document.""" - message = _("Unable to find network %(network)s.") + """"Ensure the network is not private.""" + message = _("Unable to find external network %(network)s.") code = 400 diff --git a/magnum/tests/unit/api/controllers/v1/test_bay.py b/magnum/tests/unit/api/controllers/v1/test_bay.py index 34382ccb64..0f2f86d3f0 100644 --- a/magnum/tests/unit/api/controllers/v1/test_bay.py +++ b/magnum/tests/unit/api/controllers/v1/test_bay.py @@ -597,7 +597,7 @@ class TestPost(api_base.FunctionalTest): def test_create_bay_with_invalid_ext_network(self): bdict = apiutils.bay_post_data() - self.mock_valid_os_res.side_effect = exception.NetworkNotFound( + self.mock_valid_os_res.side_effect = exception.ExternalNetworkNotFound( 'test-net') response = self.post_json('/bays', bdict, expect_errors=True) self.assertEqual('application/json', response.content_type) diff --git a/magnum/tests/unit/api/controllers/v1/test_baymodel.py b/magnum/tests/unit/api/controllers/v1/test_baymodel.py index 0b4972d763..36be261537 100644 --- a/magnum/tests/unit/api/controllers/v1/test_baymodel.py +++ b/magnum/tests/unit/api/controllers/v1/test_baymodel.py @@ -335,7 +335,8 @@ class TestPatch(api_base.FunctionalTest): self.assertTrue(response.json['errors']) def test_replace_baymodel_with_no_exist_external_network_id(self): - self.mock_valid_os_res.side_effect = exception.NetworkNotFound("aaa") + self.mock_valid_os_res.side_effect = exception.ExternalNetworkNotFound( + "aaa") response = self.patch_json('/baymodels/%s' % self.baymodel.uuid, [{'path': '/external_network_id', 'value': 'aaa', @@ -846,7 +847,8 @@ class TestPost(api_base.FunctionalTest): @mock.patch('magnum.api.attr_validator.validate_image') def test_create_baymodel_with_no_exist_external_network(self, mock_image_data): - self.mock_valid_os_res.side_effect = exception.NetworkNotFound("test") + self.mock_valid_os_res.side_effect = exception.ExternalNetworkNotFound( + "test") mock_image_data.return_value = {'name': 'mock_name', 'os_distro': 'fedora-atomic'} bdict = apiutils.baymodel_post_data() diff --git a/magnum/tests/unit/api/test_attr_validator.py b/magnum/tests/unit/api/test_attr_validator.py index 489ba9ce15..0b286aa755 100644 --- a/magnum/tests/unit/api/test_attr_validator.py +++ b/magnum/tests/unit/api/test_attr_validator.py @@ -59,6 +59,18 @@ class TestAttrValidator(base.BaseTestCase): attr_validator.validate_external_network(mock_os_cli, 'test_ext_net') self.assertTrue(mock_neutron.list_networks.called) + def test_validate_external_network_with_multiple_valid_network(self): + mock_networks = {'networks': + [{'name': 'test_ext_net', 'id': 'test_ext_net_id1'}, + {'name': 'test_ext_net', 'id': 'test_ext_net_id2'}]} + mock_neutron = mock.MagicMock() + mock_neutron.list_networks.return_value = mock_networks + mock_os_cli = mock.MagicMock() + mock_os_cli.neutron.return_value = mock_neutron + self.assertRaises(exception.Conflict, + attr_validator.validate_external_network, + mock_os_cli, 'test_ext_net') + def test_validate_external_network_with_invalid_network(self): mock_networks = {'networks': [{'name': 'test_ext_net_not_equal', 'id': 'test_ext_net_id_not_equal'}]} @@ -66,7 +78,7 @@ class TestAttrValidator(base.BaseTestCase): mock_neutron.list_networks.return_value = mock_networks mock_os_cli = mock.MagicMock() mock_os_cli.neutron.return_value = mock_neutron - self.assertRaises(exception.NetworkNotFound, + self.assertRaises(exception.ExternalNetworkNotFound, attr_validator.validate_external_network, mock_os_cli, 'test_ext_net')