From 1305e13d76192a7a1078d3248bf2609352ff2cd1 Mon Sep 17 00:00:00 2001 From: Kobi Samoray Date: Fri, 1 May 2020 21:46:00 +0300 Subject: [PATCH] Handle Octavia certificates properly Octavia certificate were mishandled as LBaaSv2 certificate objects Change-Id: Ib8ce4f735ca6fc74f6c11d91eae508fd86397dbf --- vmware_nsx/services/lbaas/lb_common.py | 5 +++ .../nsx_p/implementation/listener_mgr.py | 10 +++--- .../nsx_v/implementation/listener_mgr.py | 23 ++++++------- .../nsx_v3/implementation/listener_mgr.py | 8 ++--- .../services/lbaas/octavia/octavia_driver.py | 33 +++++++------------ .../services/lbaas/test_octavia_driver.py | 23 ++++++++----- 6 files changed, 53 insertions(+), 49 deletions(-) diff --git a/vmware_nsx/services/lbaas/lb_common.py b/vmware_nsx/services/lbaas/lb_common.py index f2af99dfe5..de20cc6981 100644 --- a/vmware_nsx/services/lbaas/lb_common.py +++ b/vmware_nsx/services/lbaas/lb_common.py @@ -58,3 +58,8 @@ def session_persistence_type_changed(pool, old_pool): oldsp['type'] == lb_const.LB_SESSION_PERSISTENCE_SOURCE_IP)): return True return False + + +def get_listener_cert_ref(listener): + return listener.get('default_tls_container_id', + listener.get('default_tls_container_ref')) diff --git a/vmware_nsx/services/lbaas/nsx_p/implementation/listener_mgr.py b/vmware_nsx/services/lbaas/nsx_p/implementation/listener_mgr.py index c680ae00ed..ec12f45317 100644 --- a/vmware_nsx/services/lbaas/nsx_p/implementation/listener_mgr.py +++ b/vmware_nsx/services/lbaas/nsx_p/implementation/listener_mgr.py @@ -52,13 +52,13 @@ class EdgeListenerManagerFromDict(base_mgr.NsxpLoadbalancerBaseManager): nsxpolicy = self.core_plugin.nsxpolicy cert_client = nsxpolicy.certificate ssl_client = nsxpolicy.load_balancer.client_ssl_profile - passphrase = certificate.get_private_key_passphrase() + passphrase = certificate.get('passphrase') if not passphrase: passphrase = core_resources.IGNORE cert_client.create_or_overwrite( cert_href, certificate_id=listener_id, - pem_encoded=certificate.get_certificate(), - private_key=certificate.get_private_key(), + pem_encoded=certificate.get('certificate'), + private_key=certificate.get('private_key'), passphrase=passphrase, tags=tags) @@ -103,7 +103,7 @@ class EdgeListenerManagerFromDict(base_mgr.NsxpLoadbalancerBaseManager): kwargs['lb_persistence_profile_id'] = '' if certificate: ssl_profile_binding = self._upload_certificate( - listener['id'], listener['default_tls_container_id'], tags, + listener['id'], certificate['ref'], tags, certificate=certificate) if (listener['protocol'] == lb_const.LB_PROTOCOL_TERMINATED_HTTPS and @@ -304,7 +304,7 @@ class EdgeListenerManagerFromDict(base_mgr.NsxpLoadbalancerBaseManager): "NSX: %s", app_profile_id, e) # Delete imported NSX cert if there is any - if listener.get('default_tls_container_id'): + if lb_common.get_listener_cert_ref(listener): cert_client = self.core_plugin.nsxpolicy.certificate try: cert_client.delete(listener['id']) diff --git a/vmware_nsx/services/lbaas/nsx_v/implementation/listener_mgr.py b/vmware_nsx/services/lbaas/nsx_v/implementation/listener_mgr.py index d47d0c863a..63db43ec08 100644 --- a/vmware_nsx/services/lbaas/nsx_v/implementation/listener_mgr.py +++ b/vmware_nsx/services/lbaas/nsx_v/implementation/listener_mgr.py @@ -25,8 +25,9 @@ from vmware_nsx.common import locking from vmware_nsx.db import nsxv_db from vmware_nsx.plugins.nsx_v.vshield.common import exceptions as vcns_exc from vmware_nsx.services.lbaas import base_mgr +from vmware_nsx.services.lbaas import lb_common from vmware_nsx.services.lbaas import lb_const -from vmware_nsx.services.lbaas.nsx_v import lbaas_common as lb_common +from vmware_nsx.services.lbaas.nsx_v import lbaas_common as lb_v_common LOG = logging.getLogger(__name__) @@ -132,9 +133,9 @@ class EdgeListenerManagerFromDict(base_mgr.EdgeLoadbalancerBaseManager): return cert_binding['edge_cert_id'] request = { - 'pemEncoding': certificate.get_certificate(), - 'privateKey': certificate.get_private_key()} - passphrase = certificate.get_private_key_passphrase() + 'pemEncoding': certificate.get('certificate'), + 'privateKey': certificate.get('private_key')} + passphrase = certificate.get('passphrase') if passphrase: request['passphrase'] = passphrase cert_obj = self.vcns.upload_edge_certificate(edge_id, request)[1] @@ -166,7 +167,7 @@ class EdgeListenerManagerFromDict(base_mgr.EdgeLoadbalancerBaseManager): if certificate: try: edge_cert_id = self._upload_certificate( - context, edge_id, listener['default_tls_container_id'], + context, edge_id, certificate['ref'], certificate) except Exception: with excutils.save_and_reraise_exception(): @@ -178,7 +179,7 @@ class EdgeListenerManagerFromDict(base_mgr.EdgeLoadbalancerBaseManager): try: with locking.LockManager.get_lock(edge_id): h = (self.vcns.create_app_profile(edge_id, app_profile))[0] - app_profile_id = lb_common.extract_resource_id(h['location']) + app_profile_id = lb_v_common.extract_resource_id(h['location']) except vcns_exc.VcnsApiException: with excutils.save_and_reraise_exception(): completor(success=False) @@ -193,7 +194,7 @@ class EdgeListenerManagerFromDict(base_mgr.EdgeLoadbalancerBaseManager): try: with locking.LockManager.get_lock(edge_id): h = self.vcns.create_vip(edge_id, vse)[0] - edge_vse_id = lb_common.extract_resource_id(h['location']) + edge_vse_id = lb_v_common.extract_resource_id(h['location']) nsxv_db.add_nsxv_lbaas_listener_binding(context.session, lb_id, @@ -231,19 +232,19 @@ class EdgeListenerManagerFromDict(base_mgr.EdgeLoadbalancerBaseManager): edge_cert_id = None if certificate: - if (old_listener['default_tls_container_id'] != - new_listener['default_tls_container_id']): + if (lb_common.get_listener_cert_ref(old_listener) != + lb_common.get_listener_cert_ref(new_listener)): try: edge_cert_id = self._upload_certificate( context, edge_id, - new_listener['default_tls_container_id'], + certificate['ref'], certificate) except Exception: with excutils.save_and_reraise_exception(): completor(success=False) else: cert_binding = nsxv_db.get_nsxv_lbaas_certificate_binding( - context.session, new_listener['default_tls_container_id'], + context.session, certificate['ref'], edge_id) edge_cert_id = cert_binding['edge_cert_id'] diff --git a/vmware_nsx/services/lbaas/nsx_v3/implementation/listener_mgr.py b/vmware_nsx/services/lbaas/nsx_v3/implementation/listener_mgr.py index 479f169890..081cc8ea9c 100644 --- a/vmware_nsx/services/lbaas/nsx_v3/implementation/listener_mgr.py +++ b/vmware_nsx/services/lbaas/nsx_v3/implementation/listener_mgr.py @@ -83,14 +83,14 @@ class EdgeListenerManagerFromDict(base_mgr.Nsxv3LoadbalancerBaseManager): # If so, use that certificate for ssl binding. Otherwise, # create a new certificate on NSX. cert_ids = tm_client.find_cert_with_pem( - certificate.get_certificate()) + certificate.get('certificate')) if cert_ids: nsx_cert_id = cert_ids[0] else: nsx_cert_id = tm_client.create_cert( - certificate.get_certificate(), - private_key=certificate.get_private_key(), - passphrase=certificate.get_private_key_passphrase(), + certificate.get('certificate'), + private_key=certificate.get('private_key'), + passphrase=certificate.get('passphrase'), tags=tags) return { 'client_ssl_profile_binding': { diff --git a/vmware_nsx/services/lbaas/octavia/octavia_driver.py b/vmware_nsx/services/lbaas/octavia/octavia_driver.py index 9a776b2b63..e1a86bdf64 100644 --- a/vmware_nsx/services/lbaas/octavia/octavia_driver.py +++ b/vmware_nsx/services/lbaas/octavia/octavia_driver.py @@ -22,8 +22,6 @@ from oslo_log import helpers as log_helpers from oslo_log import log as logging import oslo_messaging as messaging from oslo_messaging.rpc import dispatcher -import pecan -from stevedore import driver as stevedore_driver from octavia.api.drivers import utils as oct_utils from octavia.db import api as db_apis @@ -76,7 +74,6 @@ class NSXOctaviaDriver(driver_base.ProviderDriver): def __init__(self): super(NSXOctaviaDriver, self).__init__() self._init_rpc_messaging() - self._init_cert_manager() self.repositories = repositories.Repositories() @log_helpers.log_method_call @@ -88,13 +85,6 @@ class NSXOctaviaDriver(driver_base.ProviderDriver): version='1.0') self.client = messaging.RPCClient(transport, target) - @log_helpers.log_method_call - def _init_cert_manager(self): - self.cert_manager = stevedore_driver.DriverManager( - namespace='octavia.cert_manager', - name=cfg.CONF.certificates.cert_manager, - invoke_on_load=True).driver - def get_obj_project_id(self, obj_type, obj_dict): if obj_dict.get('project_id'): return obj_dict['project_id'] @@ -309,7 +299,6 @@ class NSXOctaviaDriver(driver_base.ProviderDriver): # Generate the default pool object obj_dict['default_pool'] = self._get_pool_dict( obj_dict['default_pool_id'], is_update, project_id) - # TODO(asarfaty): add default_tls_container_id elif obj_type == 'Pool': if 'listener' not in obj_dict: @@ -395,15 +384,21 @@ class NSXOctaviaDriver(driver_base.ProviderDriver): 'new_loadbalancer': new_dict} self.client.cast({}, 'loadbalancer_update', **kw) + def _create_lb_certificate(self, listener_dict): + # Extract Octavia certificate data into a dict which is readable by + # the listener_mgr + if listener_dict.get('default_tls_container_ref'): + cert_data = listener_dict.get('default_tls_container_data', {}) + return {'ref': listener_dict.get('default_tls_container_ref'), + 'certificate': cert_data.get('certificate'), + 'private_key': cert_data.get('private_key'), + 'passphrase': cert_data.get('passphrase')} + # Listener @log_helpers.log_method_call def listener_create(self, listener): - cert = None dict_list = self.obj_to_dict(listener) - if dict_list.get('tls_certificate_id'): - context = pecan.request.context.get('octavia_context') - cert = self.cert_manager.get_cert(context, - dict_list['tls_certificate_id']) + cert = self._create_lb_certificate(dict_list) kw = {'listener': dict_list, 'cert': cert} self.client.cast({}, 'listener_create', **kw) @@ -419,11 +414,7 @@ class NSXOctaviaDriver(driver_base.ProviderDriver): new_dict.update(self.obj_to_dict( new_listener, is_update=True, project_id=old_dict.get('project_id'))) - cert = None - if new_dict.get('tls_certificate_id'): - context = pecan.request.context.get('octavia_context') - cert = self.cert_manager.get_cert(context, - new_dict['tls_certificate_id']) + cert = self._create_lb_certificate(new_dict) kw = {'old_listener': old_dict, 'new_listener': new_dict, 'cert': cert} diff --git a/vmware_nsx/tests/unit/services/lbaas/test_octavia_driver.py b/vmware_nsx/tests/unit/services/lbaas/test_octavia_driver.py index 7a20858ad8..d0f5fe2a84 100644 --- a/vmware_nsx/tests/unit/services/lbaas/test_octavia_driver.py +++ b/vmware_nsx/tests/unit/services/lbaas/test_octavia_driver.py @@ -47,8 +47,7 @@ class TestNsxProviderDriver(testtools.TestCase): if not code_ok: return # init the NSX driver without the RPC & certificate - with mock.patch(DRIVER + '._init_rpc_messaging'), \ - mock.patch(DRIVER + '._init_cert_manager'): + with mock.patch(DRIVER + '._init_rpc_messaging'): self.driver = driver.NSXOctaviaDriver() self.driver.client = mock.Mock() @@ -66,6 +65,11 @@ class TestNsxProviderDriver(testtools.TestCase): self.l7rule_id = uuidutils.generate_uuid() self.project_id = uuidutils.generate_uuid() self.default_tls_container_ref = uuidutils.generate_uuid() + self.default_tls_container_data = { + 'ref': self.default_tls_container_ref, + 'certificate': 'cert_text', + 'private_key': 'pk_text', + 'passphrase': 'pp_text'} self.sni_container_ref_1 = uuidutils.generate_uuid() self.sni_container_ref_2 = uuidutils.generate_uuid() @@ -134,7 +138,7 @@ class TestNsxProviderDriver(testtools.TestCase): connection_limit=5, default_pool=self.ref_pool, default_pool_id=self.pool_id, - default_tls_container_data='default_cert_data', + default_tls_container_data=self.default_tls_container_data, default_tls_container_ref=self.default_tls_container_ref, description='The listener', insert_headers={'X-Forwarded-For': 'true'}, @@ -222,8 +226,9 @@ class TestNsxProviderDriver(testtools.TestCase): def test_listener_create(self): with mock.patch.object(self.driver.client, 'cast') as cast_method: self.driver.listener_create(self.ref_listener) - cast_method.assert_called_with({}, 'listener_create', cert=None, - listener=mock.ANY) + cast_method.assert_called_with( + {}, 'listener_create', cert=self.default_tls_container_data, + listener=mock.ANY) driver_obj = cast_method.call_args[1]['listener'] self.assertIn('id', driver_obj) self.assertIn('project_id', driver_obj) @@ -255,9 +260,11 @@ class TestNsxProviderDriver(testtools.TestCase): def test_listener_update(self): with mock.patch.object(self.driver.client, 'cast') as cast_method: self.driver.listener_update(self.ref_listener, self.ref_listener) - cast_method.assert_called_with({}, 'listener_update', cert=None, - old_listener=mock.ANY, - new_listener=mock.ANY) + cast_method.assert_called_with( + {}, 'listener_update', + cert=self.default_tls_container_data, + old_listener=mock.ANY, + new_listener=mock.ANY) driver_obj = cast_method.call_args[1]['new_listener'] self.assertIn('id', driver_obj) self.assertIn('project_id', driver_obj)