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

@@ -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):

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
 
 

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)

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()

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')