From: Salvatore Orlando <salv.orlando@gmail.com>
Date: Thu, 10 Jul 2014 21:55:04 +0000 (-0700)
Subject: NSX: fix validation logic on network gateway connect
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=ff1ce62a0b9f504fd1dd46fb569ab85f5b56e4ff;p=openstack-build%2Fneutron-build.git

NSX: fix validation logic on network gateway connect

This patch adds validation for the segmentation ID when the
network type for the gateway connection is vlan.
This will avoid requests with invalid vlan IDs are sent to
the backend resulting in 500 error responses being
returned to API users.

To this aim this patch slightly alters the current validation
logic due to the fact that some checks are unnecessary since
the same routine sets default values which avoid the
conditions being checked.

Change-Id: If0e71f6fdf27a49f0eda727e21405cffbc260a7a
Closes-Bug: #1340431
---

diff --git a/neutron/plugins/vmware/dbexts/networkgw_db.py b/neutron/plugins/vmware/dbexts/networkgw_db.py
index fb5eb6268..40113d129 100644
--- a/neutron/plugins/vmware/dbexts/networkgw_db.py
+++ b/neutron/plugins/vmware/dbexts/networkgw_db.py
@@ -19,6 +19,7 @@ from sqlalchemy.orm import exc as sa_orm_exc
 
 from neutron.api.v2 import attributes
 from neutron.common import exceptions
+from neutron.common import utils
 from neutron.db import model_base
 from neutron.db import models_v2
 from neutron.openstack.common import log as logging
@@ -199,14 +200,16 @@ class NetworkGatewayMixin(networkgw.NetworkGatewayPluginBase):
                                connection_attrs))
         seg_type = network_mapping_info.get(SEGMENTATION_TYPE)
         seg_id = network_mapping_info.get(SEGMENTATION_ID)
-        if not seg_type and seg_id:
-            msg = _("In order to specify a segmentation id the "
-                    "segmentation type must be specified as well")
-            raise exceptions.InvalidInput(error_message=msg)
-        elif seg_type and seg_type.lower() == 'flat' and seg_id:
+        # The NSX plugin accepts 0 as a valid vlan tag
+        seg_id_valid = seg_id == 0 or utils.is_valid_vlan_tag(seg_id)
+        if seg_type.lower() == 'flat' and seg_id:
             msg = _("Cannot specify a segmentation id when "
                     "the segmentation type is flat")
             raise exceptions.InvalidInput(error_message=msg)
+        elif (seg_type.lower() == 'vlan' and not seg_id_valid):
+            msg = _("Invalid segmentation id (%d) for "
+                    "vlan segmentation type") % seg_id
+            raise exceptions.InvalidInput(error_message=msg)
         return network_id
 
     def _retrieve_gateway_connections(self, context, gateway_id,
diff --git a/neutron/tests/unit/vmware/extensions/test_networkgw.py b/neutron/tests/unit/vmware/extensions/test_networkgw.py
index ac4caaee7..dd15dccb3 100644
--- a/neutron/tests/unit/vmware/extensions/test_networkgw.py
+++ b/neutron/tests/unit/vmware/extensions/test_networkgw.py
@@ -652,9 +652,12 @@ class NetworkGatewayDbTestCase(test_db_plugin.NeutronDbPluginV2TestCase):
     def test_connect_and_disconnect_network_no_seg_type(self):
         self._test_connect_and_disconnect_network(None)
 
-    def test_connect_and_disconnect_network_with_segmentation_id(self):
+    def test_connect_and_disconnect_network_vlan_with_segmentation_id(self):
         self._test_connect_and_disconnect_network('vlan', 999)
 
+    def test_connect_and_disconnect_network_vlan_without_segmentation_id(self):
+        self._test_connect_and_disconnect_network('vlan')
+
     def test_connect_network_multiple_times(self):
         with self._network_gateway() as gw:
             with self.network() as net_1:
@@ -715,6 +718,22 @@ class NetworkGatewayDbTestCase(test_db_plugin.NeutronDbPluginV2TestCase):
                                      net_1['network']['id'],
                                      'vlan', 555)
 
+    def test_connect_network_vlan_invalid_seg_id_returns_400(self):
+        with self._network_gateway() as gw:
+            with self.network() as net:
+                # above upper bound
+                self._gateway_action('connect',
+                                     gw[self.gw_resource]['id'],
+                                     net['network']['id'],
+                                     'vlan', 4095,
+                                     expected_status=exc.HTTPBadRequest.code)
+                # below lower bound (0 is valid for NSX plugin)
+                self._gateway_action('connect',
+                                     gw[self.gw_resource]['id'],
+                                     net['network']['id'],
+                                     'vlan', -1,
+                                     expected_status=exc.HTTPBadRequest.code)
+
     def test_connect_invalid_network_returns_400(self):
         with self._network_gateway() as gw:
                 self._gateway_action('connect',