api: Reject duplicate port IDs in server create

Specifying a duplicate port ID is currently "allowed" but results in an
integrity error when nova attempts to create a duplicate
'VirtualInterface' entry. Start rejecting these requests by checking for
duplicate IDs and rejecting offending requests. This is arguably an API
change because there isn't a HTTP 5xx error (server create is an async
operation), however, users shouldn't have to opt in to non-broken
behavior and the underlying instance was never actually created
previously, meaning automation that relied on this "feature" was always
going to fail in a later step. We're also silently failing to do what
the user asked (per flow chart at [1]).

[1] https://docs.openstack.org/nova/latest/contributor/microversions.html#when-do-i-need-a-new-microversion

Change-Id: Ie90fb83662dd06e7188f042fc6340596f93c5ef9
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
Closes-Bug: #1821088
(cherry picked from commit 9fe4654273)

nova/api/openstack/compute/servers.py

@@ -409,6 +409,7 @@ class ServersController(wsgi.Controller):
 
         networks = []
         network_uuids = []
+        port_uuids = []
         for network in requested_networks:
             request = objects.NetworkRequest()
             try:
@@ -417,18 +418,31 @@ class ServersController(wsgi.Controller):
                 # it will use one of the available IP address from the network
                 request.address = network.get('fixed_ip', None)
                 request.port_id = network.get('port', None)
-
                 request.tag = network.get('tag', None)
 
                 if request.port_id:
-                    request.network_id = None
-                    if request.address is not None:
-                        msg = _("Specified Fixed IP '%(addr)s' cannot be used "
-                                "with port '%(port)s': the two cannot be "
-                                "specified together.") % {
-                                    "addr": request.address,
-                                    "port": request.port_id}
+                    if request.port_id in port_uuids:
+                        msg = _(
+                            "Port ID '%(port)s' was specified twice: you "
+                            "cannot attach a port multiple times."
+                        ) % {
+                            "port": request.port_id,
+                        }
                         raise exc.HTTPBadRequest(explanation=msg)
+
+                    if request.address is not None:
+                        msg = _(
+                            "Specified Fixed IP '%(addr)s' cannot be used "
+                            "with port '%(port)s': the two cannot be "
+                            "specified together."
+                        ) % {
+                            "addr": request.address,
+                            "port": request.port_id,
+                        }
+                        raise exc.HTTPBadRequest(explanation=msg)
+
+                    request.network_id = None
+                    port_uuids.append(request.port_id)
                 else:
                     request.network_id = network['uuid']
                     self._validate_network_id(

nova/tests/unit/api/openstack/compute/test_servers.py

@@ -389,17 +389,36 @@ class ServersControllerTest(_ServersControllerTest):
                           (network, None, None, None, None, None)],
                           res.as_tuples())
 
-    def test_requested_networks_enabled_conflict_on_fixed_ip(self):
-        network = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa'
+    def test_requested_networks_duplicate_ports(self):
+        """The same port can't be specified twice."""
         port = 'eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee'
-        addr = '10.0.0.1'
-        requested_networks = [{'uuid': network,
-                               'fixed_ip': addr,
-                               'port': port}]
-        self.assertRaises(
+        requested_networks = [{'port': port}, {'port': port}]
+        exc = self.assertRaises(
             webob.exc.HTTPBadRequest,
             self.controller._get_requested_networks,
-            requested_networks)
+            requested_networks,
+        )
+        self.assertIn(
+            "Port ID 'eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee' was specified "
+            "twice",
+            str(exc),
+        )
+
+    def test_requested_networks_conflict_on_fixed_ip(self):
+        """A fixed IP can't be specified at the same as a port ID."""
+        port = 'eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee'
+        addr = '10.0.0.1'
+        requested_networks = [{'fixed_ip': addr, 'port': port}]
+        exc = self.assertRaises(
+            webob.exc.HTTPBadRequest,
+            self.controller._get_requested_networks,
+            requested_networks,
+        )
+        self.assertIn(
+            "Specified Fixed IP '10.0.0.1' cannot be used with port "
+            "'eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee'",
+            str(exc),
+        )
 
     def test_requested_networks_api_enabled_with_v2_subclass(self):
         network = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa'

releasenotes/notes/bug-1821088-reject-duplicate-port-ids-a38739d67d5d7c5d.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    The ``POST /servers`` (create server) API will now reject attempts to
+    create a server with the same port specified multiple times. This was
+    previously accepted by the API but the instance would fail to spawn and
+    would instead transition to the error state.