From d56f12d4676c1cc26d0da80e053da07e0253dbe8 Mon Sep 17 00:00:00 2001 From: Salvatore Orlando Date: Fri, 14 Mar 2014 17:43:42 -0700 Subject: [PATCH] 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) --- .../plugins/vmware/api_client/exception.py | 17 +++++- neutron/plugins/vmware/common/exceptions.py | 7 +++ neutron/plugins/vmware/nsxlib/l2gateway.py | 22 ++++--- neutron/plugins/vmware/plugins/base.py | 61 ++++++++++--------- .../unit/vmware/extensions/test_networkgw.py | 33 +++++++++- 5 files changed, 99 insertions(+), 41 deletions(-) diff --git a/neutron/plugins/vmware/api_client/exception.py b/neutron/plugins/vmware/api_client/exception.py index 46b870423..b3facfcaa 100644 --- 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 cb2cf8660..8b7bfa5bc 100644 --- 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 9d34a988a..11c32f376 100644 --- 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 457fa8bc0..9e2c1e996 100644 --- 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 6c81f9c33..765f075ab 100644 --- 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): -- 2.45.2