NSX plugin: return 400 for invalid gw certificate

Gateway certificates are validated by the NSX backend.
The code currently treats a failure in certification
validation as a backend failure and therefore returns
a 500 status code.

This patch changes this behaviour by returning a 400
status code and an appropriate error description.
To this aim a handler for 400 errors has been added to
the NSX API client.

Closes-Bug: #1293508

Change-Id: I196f14337e47cd40710a6d8a30bbe1cac5ffe05b
(cherry picked from commit 13c9f3b813f5bb368e311ba0d428fa759d68289a)

diff --git a/neutron/plugins/vmware/api_client/exception.py b/neutron/plugins/vmware/api_client/exception.py
index 46b8704234d22b8fa8876ab25cef9f70cafe3584..b3facfcaa5ddb49d1a62a411adb72d0985a0f27c 100644 (file)
--- a/neutron/plugins/vmware/api_client/exception.py
+++ b/neutron/plugins/vmware/api_client/exception.py
@@ -68,6 +68,21 @@ class RequestTimeout(NsxApiException):
     message = _("The request has timed out.")
 
 
+class BadRequest(NsxApiException):
+    message = _("The server is unable to fulfill the request due "
+                "to a bad syntax")
+
+
+class InvalidSecurityCertificate(BadRequest):
+    message = _("The backend received an invalid security certificate.")
+
+
+def fourZeroZero(response=None):
+    if response and "Invalid SecurityCertificate" in response.body:
+        raise InvalidSecurityCertificate()
+    raise BadRequest()
+
+
 def fourZeroFour(response=None):
     raise ResourceNotFound()
 
@@ -92,6 +107,7 @@ def zero(self, response=None):
 
 
 ERROR_MAPPINGS = {
+    400: fourZeroZero,
     404: fourZeroFour,
     405: zero,
     409: fourZeroNine,
@@ -99,7 +115,6 @@ ERROR_MAPPINGS = {
     403: fourZeroThree,
     301: zero,
     307: zero,
-    400: zero,
     500: zero,
     501: zero,
     503: zero

diff --git a/neutron/plugins/vmware/common/exceptions.py b/neutron/plugins/vmware/common/exceptions.py
index cb2cf866090e5ba50a622ddf431272026cdc0fe2..8b7bfa5bc1d6707c27b8c4d6925bc72e9cbe6b06 100644 (file)
--- a/neutron/plugins/vmware/common/exceptions.py
+++ b/neutron/plugins/vmware/common/exceptions.py
@@ -65,6 +65,13 @@ class L2GatewayAlreadyInUse(n_exc.Conflict):
     message = _("Gateway Service %(gateway)s is already in use")
 
 
+class InvalidSecurityCertificate(NsxPluginException):
+    message = _("An invalid security certificate was specified for the "
+                "gateway device. Certificates must be enclosed between "
+                "'-----BEGIN CERTIFICATE-----' and "
+                "'-----END CERTIFICATE-----'")
+
+
 class ServiceOverQuota(n_exc.Conflict):
     message = _("Quota exceeded for Vcns resource: %(overs)s: %(err_msg)s")
 

diff --git a/neutron/plugins/vmware/nsxlib/l2gateway.py b/neutron/plugins/vmware/nsxlib/l2gateway.py
index 9d34a988a7645a4e6723795378db1f08d6129fc0..11c32f3766c84df4e4b034cdb6ced97cc461af3b 100644 (file)
--- a/neutron/plugins/vmware/nsxlib/l2gateway.py
+++ b/neutron/plugins/vmware/nsxlib/l2gateway.py
@@ -17,6 +17,8 @@
 import json
 
 from neutron.openstack.common import log
+from neutron.plugins.vmware.api_client import exception as api_exc
+from neutron.plugins.vmware.common import exceptions as nsx_exc
 from neutron.plugins.vmware.common import utils
 from neutron.plugins.vmware.nsxlib import _build_uri_path
 from neutron.plugins.vmware.nsxlib import do_request
@@ -145,9 +147,12 @@ def create_gateway_device(cluster, tenant_id, display_name, neutron_id,
     body = _build_gateway_device_body(tenant_id, display_name, neutron_id,
                                       connector_type, connector_ip,
                                       client_certificate, tz_uuid)
-    return do_request(
-        HTTP_POST, _build_uri_path(TRANSPORTNODE_RESOURCE),
-        json.dumps(body), cluster=cluster)
+    try:
+        return do_request(
+            HTTP_POST, _build_uri_path(TRANSPORTNODE_RESOURCE),
+            json.dumps(body), cluster=cluster)
+    except api_exc.InvalidSecurityCertificate:
+        raise nsx_exc.InvalidSecurityCertificate()
 
 
 def update_gateway_device(cluster, gateway_id, tenant_id,
@@ -157,10 +162,13 @@ def update_gateway_device(cluster, gateway_id, tenant_id,
     body = _build_gateway_device_body(tenant_id, display_name, neutron_id,
                                       connector_type, connector_ip,
                                       client_certificate, tz_uuid)
-    return do_request(
-        HTTP_PUT,
-        _build_uri_path(TRANSPORTNODE_RESOURCE, resource_id=gateway_id),
-        json.dumps(body), cluster=cluster)
+    try:
+        return do_request(
+            HTTP_PUT,
+            _build_uri_path(TRANSPORTNODE_RESOURCE, resource_id=gateway_id),
+            json.dumps(body), cluster=cluster)
+    except api_exc.InvalidSecurityCertificate:
+        raise nsx_exc.InvalidSecurityCertificate()
 
 
 def delete_gateway_device(cluster, device_uuid):

diff --git a/neutron/plugins/vmware/plugins/base.py b/neutron/plugins/vmware/plugins/base.py
index 457fa8bc040bd6a3bdfd84f2817c004be188cd61..9e2c1e996e8e47a83f2ca0d1053c8ff22f9ca8d7 100644 (file)
--- a/neutron/plugins/vmware/plugins/base.py
+++ b/neutron/plugins/vmware/plugins/base.py
@@ -738,7 +738,9 @@ class NsxPluginV2(addr_pair_db.AllowedAddressPairsMixin,
                                nsx_exc.NoMorePortsException:
                                webob.exc.HTTPBadRequest,
                                nsx_exc.MaintenanceInProgress:
-                               webob.exc.HTTPServiceUnavailable})
+                               webob.exc.HTTPServiceUnavailable,
+                               nsx_exc.InvalidSecurityCertificate:
+                               webob.exc.HTTPBadRequest})
 
     def _validate_provider_create(self, context, network):
         if not attr.is_attr_set(network.get(mpnet.SEGMENTS)):
@@ -2098,6 +2100,26 @@ class NsxPluginV2(addr_pair_db.AllowedAddressPairsMixin,
     def _get_nsx_device_id(self, context, device_id):
         return self._get_gateway_device(context, device_id)['nsx_id']
 
+    def _rollback_gw_device(self, context, device_id,
+                            gw_data=None, new_status=None,
+                            is_create=False, log_level=logging.ERROR):
+        LOG.log(log_level,
+                _("Rolling back database changes for gateway device %s "
+                  "because of an error in the NSX backend"), device_id)
+        with context.session.begin(subtransactions=True):
+            query = self._model_query(
+                context, networkgw_db.NetworkGatewayDevice).filter(
+                    networkgw_db.NetworkGatewayDevice.id == device_id)
+            if is_create:
+                query.delete(synchronize_session=False)
+            else:
+                super(NsxPluginV2, self).update_gateway_device(
+                    context, device_id,
+                    {networkgw.DEVICE_RESOURCE_NAME: gw_data})
+                if new_status:
+                    query.update({'status': new_status},
+                                 synchronize_session=False)
+
     # TODO(salv-orlando): Handlers for Gateway device operations should be
     # moved into the appropriate nsx_handlers package once the code for the
     # blueprint nsx-async-backend-communication merges
@@ -2134,16 +2156,9 @@ class NsxPluginV2(addr_pair_db.AllowedAddressPairsMixin,
                        'nsx_id': nsx_res['uuid'],
                        'status': device_status})
             return device_status
-        except api_exc.NsxApiException:
-            # Remove gateway device from neutron database
+        except (nsx_exc.InvalidSecurityCertificate, api_exc.NsxApiException):
             with excutils.save_and_reraise_exception():
-                LOG.exception(_("Unable to create gateway device: %s on NSX "
-                                "backend."), neutron_id)
-                with context.session.begin(subtransactions=True):
-                    query = self._model_query(
-                        context, networkgw_db.NetworkGatewayDevice).filter(
-                            networkgw_db.NetworkGatewayDevice.id == neutron_id)
-                    query.delete(synchronize_session=False)
+                self._rollback_gw_device(context, neutron_id, is_create=True)
 
     def update_gateway_device_handler(self, context, gateway_device,
                                       old_gateway_device_data,
@@ -2165,7 +2180,6 @@ class NsxPluginV2(addr_pair_db.AllowedAddressPairsMixin,
             # Fetch status (it needs another NSX API call)
             device_status = nsx_utils.get_nsx_device_status(self.cluster,
                                                             nsx_id)
-
             # update status
             with context.session.begin(subtransactions=True):
                 query = self._model_query(
@@ -2180,31 +2194,18 @@ class NsxPluginV2(addr_pair_db.AllowedAddressPairsMixin,
                        'nsx_id': nsx_id,
                        'status': device_status})
             return device_status
-        except api_exc.NsxApiException:
-            # Rollback gateway device on neutron database
-            # As the NSX failure could be transient, we don't put the
-            # gateway device in error status here.
+        except (nsx_exc.InvalidSecurityCertificate, api_exc.NsxApiException):
             with excutils.save_and_reraise_exception():
-                LOG.exception(_("Unable to update gateway device: %s on NSX "
-                                "backend."), neutron_id)
-                super(NsxPluginV2, self).update_gateway_device(
-                    context, neutron_id, old_gateway_device_data)
+                self._rollback_gw_device(context, neutron_id,
+                                         gw_data=old_gateway_device_data)
         except n_exc.NotFound:
             # The gateway device was probably deleted in the backend.
             # The DB change should be rolled back and the status must
             # be put in error
             with excutils.save_and_reraise_exception():
-                LOG.exception(_("Unable to update gateway device: %s on NSX "
-                                "backend, as the gateway was not found on "
-                                "the NSX backend."), neutron_id)
-            with context.session.begin(subtransactions=True):
-                super(NsxPluginV2, self).update_gateway_device(
-                    context, neutron_id, old_gateway_device_data)
-                query = self._model_query(
-                    context, networkgw_db.NetworkGatewayDevice).filter(
-                        networkgw_db.NetworkGatewayDevice.id == neutron_id)
-                query.update({'status': networkgw_db.ERROR},
-                             synchronize_session=False)
+                self._rollback_gw_device(context, neutron_id,
+                                         gw_data=old_gateway_device_data,
+                                         new_status=networkgw_db.ERROR)
 
     def get_gateway_device(self, context, device_id, fields=None):
         # Get device from database

diff --git a/neutron/tests/unit/vmware/extensions/test_networkgw.py b/neutron/tests/unit/vmware/extensions/test_networkgw.py
index 6c81f9c33c41bbe3192bdd2ab8fd79dd79c7839f..765f075ab70f87e79a8cea33f29a532dc604f4a8 100644 (file)
--- a/neutron/tests/unit/vmware/extensions/test_networkgw.py
+++ b/neutron/tests/unit/vmware/extensions/test_networkgw.py
@@ -28,6 +28,7 @@ from neutron.db import api as db_api
 from neutron.db import db_base_plugin_v2
 from neutron import manager
 from neutron.plugins.vmware.api_client import exception as api_exc
+from neutron.plugins.vmware.common import exceptions as nsx_exc
 from neutron.plugins.vmware.dbexts import networkgw_db
 from neutron.plugins.vmware.extensions import networkgw
 from neutron.plugins.vmware import nsxlib
@@ -861,9 +862,9 @@ class TestNetworkGateway(NsxPluginV2TestCase,
             l2gwlib, 'delete_gateway_device')
         get_gw_dev_status_patcher = mock.patch.object(
             l2gwlib, 'get_gateway_device_status')
-        mock_create_gw_dev = create_gw_dev_patcher.start()
-        mock_create_gw_dev.return_value = {'uuid': 'callejon'}
-        update_gw_dev_patcher.start()
+        self.mock_create_gw_dev = create_gw_dev_patcher.start()
+        self.mock_create_gw_dev.return_value = {'uuid': 'callejon'}
+        self.mock_update_gw_dev = update_gw_dev_patcher.start()
         delete_gw_dev_patcher.start()
         self.mock_get_gw_dev_status = get_gw_dev_status_patcher.start()
 
@@ -969,6 +970,18 @@ class TestNetworkGateway(NsxPluginV2TestCase,
         super(TestNetworkGateway, self).test_create_gateway_device(
             expected_status=networkgw_db.STATUS_DOWN)
 
+    def test_create_gateway_device_invalid_cert_returns_400(self):
+        self.mock_create_gw_dev.side_effect = (
+            nsx_exc.InvalidSecurityCertificate)
+        res = self._create_gateway_device(
+            'json',
+            _uuid(),
+            connector_type='stt',
+            connector_ip='1.1.1.1',
+            client_certificate='invalid_certificate',
+            name='whatever')
+        self.assertEqual(res.status_int, 400)
+
     def test_get_gateway_device(self):
         self.mock_get_gw_dev_status.return_value = True
         super(TestNetworkGateway, self).test_get_gateway_device(
@@ -989,6 +1002,20 @@ class TestNetworkGateway(NsxPluginV2TestCase,
         super(TestNetworkGateway, self).test_update_gateway_device(
             expected_status=networkgw_db.STATUS_DOWN)
 
+    def test_update_gateway_device_invalid_cert_returns_400(self):
+        with self._gateway_device(
+            name='whaterver',
+            connector_type='stt',
+            connector_ip='1.1.1.1',
+            client_certificate='iminvalidbutiitdoesnotmatter') as dev:
+            self.mock_update_gw_dev.side_effect = (
+                nsx_exc.InvalidSecurityCertificate)
+            res = self._update_gateway_device(
+                'json',
+                dev[self.dev_resource]['id'],
+                client_certificate='invalid_certificate')
+            self.assertEqual(res.status_int, 400)
+
 
 class TestNetworkGatewayPlugin(db_base_plugin_v2.NeutronDbPluginV2,
                                networkgw_db.NetworkGatewayMixin):