Fix _get_id_for

Raise BadRequest on the absense of the attribute.
Also, ensure that the value is actually a UUID-like.

Backport note: this is a backport from neutron-dynamic-routing
repository, commit e2581d50e76ac05488a284b4a1efb069248ba9f1 .

Closes-Bug: #1595078
Change-Id: I6c236d5e4acdd6f13eca2877163facc2860a9445

neutron/db/bgp_db.py

@@ -23,6 +23,7 @@
 from sqlalchemy.orm import aliased
 from sqlalchemy.orm import exc as sa_exc
 
+from neutron_lib.api import validators
 from neutron_lib import constants as lib_consts
 
 from neutron.api.v2 import attributes as attr
@@ -308,11 +309,14 @@ def get_advertised_routes(self, context, bgp_speaker_id):
 
     def _get_id_for(self, resource, id_name):
         try:
-            return resource.get(id_name)
-        except AttributeError:
+            uuid = resource[id_name]
+            msg = validators.validate_uuid(uuid)
+        except KeyError:
             msg = _("%s must be specified") % id_name
+        if msg:
             raise n_exc.BadRequest(resource=bgp_ext.BGP_SPEAKER_RESOURCE_NAME,
                                    msg=msg)
+        return uuid
 
     def _get_bgp_peers_by_bgp_speaker_binding(self, context, bgp_speaker_id):
         with context.session.begin(subtransactions=True):

neutron/tests/unit/db/test_bgp_db.py

@@ -29,6 +29,7 @@
 _uuid = uuidutils.generate_uuid
 
 ADVERTISE_FIPS_KEY = 'advertise_floating_ip_host_routes'
+IMAGINARY = '2b2334c8-adfe-42d9-82c6-ad866c7fc5d8'  # non existent resource id
 
 
 class BgpEntityCreationMixin(object):
@@ -304,7 +305,7 @@ def test_remove_unassociated_bgp_peer(self):
                                   {'bgp_peer_id': peer['id']})
 
     def test_remove_non_existent_bgp_peer(self):
-        bgp_peer_id = "imaginary"
+        bgp_peer_id = IMAGINARY
         with self.subnetpool_with_address_scope(4,
                                                 prefixes=['8.0.0.0/8']) as sp:
             with self.bgp_speaker(sp['ip_version'], 1234) as speaker:
@@ -315,7 +316,7 @@ def test_remove_non_existent_bgp_peer(self):
                                   {'bgp_peer_id': bgp_peer_id})
 
     def test_add_non_existent_bgp_peer(self):
-        bgp_peer_id = "imaginary"
+        bgp_peer_id = IMAGINARY
         with self.subnetpool_with_address_scope(4,
                                                 prefixes=['8.0.0.0/8']) as sp:
             with self.bgp_speaker(sp['ip_version'], 1234) as speaker:
@@ -325,6 +326,36 @@ def test_add_non_existent_bgp_peer(self):
                                   speaker['id'],
                                   {'bgp_peer_id': bgp_peer_id})
 
+    def test_add_bgp_peer_without_id(self):
+        with self.subnetpool_with_address_scope(4,
+                                                prefixes=['8.0.0.0/8']) as sp:
+            with self.bgp_speaker(sp['ip_version'], 1234) as speaker:
+                self.assertRaises(n_exc.BadRequest,
+                                  self.bgp_plugin.add_bgp_peer,
+                                  self.context,
+                                  speaker['id'],
+                                  {})
+
+    def test_add_bgp_peer_with_bad_id(self):
+        with self.subnetpool_with_address_scope(4,
+                                                prefixes=['8.0.0.0/8']) as sp:
+            with self.bgp_speaker(sp['ip_version'], 1234) as speaker:
+                self.assertRaises(n_exc.BadRequest,
+                                  self.bgp_plugin.add_bgp_peer,
+                                  self.context,
+                                  speaker['id'],
+                                  {'bgp_peer_id': 'aaa'})
+
+    def test_add_bgp_peer_with_none_id(self):
+        with self.subnetpool_with_address_scope(4,
+                                                prefixes=['8.0.0.0/8']) as sp:
+            with self.bgp_speaker(sp['ip_version'], 1234) as speaker:
+                self.assertRaises(n_exc.BadRequest,
+                                  self.bgp_plugin.add_bgp_peer,
+                                  self.context,
+                                  speaker['id'],
+                                  {'bgp_peer_id': None})
+
     def test_add_gateway_network(self):
         with self.subnetpool_with_address_scope(4,
                                                 prefixes=['8.0.0.0/8']) as sp:
@@ -371,7 +402,7 @@ def test_remove_gateway_network(self):
                 self.assertEqual(1, len(new_speaker['networks']))
 
     def test_add_non_existent_gateway_network(self):
-        network_id = "imaginary"
+        network_id = IMAGINARY
         with self.subnetpool_with_address_scope(4,
                                                 prefixes=['8.0.0.0/8']) as sp:
             with self.bgp_speaker(sp['ip_version'], 1234) as speaker:
@@ -381,7 +412,7 @@ def test_add_non_existent_gateway_network(self):
                                   {'network_id': network_id})
 
     def test_remove_non_existent_gateway_network(self):
-        network_id = "imaginary"
+        network_id = IMAGINARY
         with self.subnetpool_with_address_scope(4,
                                                 prefixes=['8.0.0.0/8']) as sp:
             with self.bgp_speaker(sp['ip_version'], 1234) as speaker: