From a6a4001f5739e124a0ea9abb0f064a89e4ba5aae Mon Sep 17 00:00:00 2001 From: Carlos Goncalves Date: Wed, 16 Sep 2020 12:21:18 +0000 Subject: [PATCH] Fix AttributeError on TLS-enabled pool provisioning This patch fixes an hidden AttributeError exception in the HAProxy driver when adding TLS-enabled pools to listeners. Task: 40895 Story: 2008150 Change-Id: If165e995a8b61d8a8ed6bc0c4dd036af0c55c6e0 --- octavia/amphorae/drivers/haproxy/rest_api_driver.py | 11 ++++++----- .../drivers/haproxy/test_rest_api_driver_0_5.py | 12 +++++++----- .../drivers/haproxy/test_rest_api_driver_1_0.py | 12 +++++++----- ...s-enabled-pool-provisioning-e3adb987244a025a.yaml | 4 ++++ 4 files changed, 24 insertions(+), 15 deletions(-) create mode 100644 releasenotes/notes/fix-tls-enabled-pool-provisioning-e3adb987244a025a.yaml diff --git a/octavia/amphorae/drivers/haproxy/rest_api_driver.py b/octavia/amphorae/drivers/haproxy/rest_api_driver.py index 1a05896532..562cc13e88 100644 --- a/octavia/amphorae/drivers/haproxy/rest_api_driver.py +++ b/octavia/amphorae/drivers/haproxy/rest_api_driver.py @@ -203,9 +203,9 @@ class HaproxyAmphoraLoadBalancerDriver( else: listeners_to_update.append(listener) except Exception as e: - LOG.error('Unable to update listener {0} due to "{1}". ' - 'Skipping this listener.'.format( - listener.id, str(e))) + LOG.exception('Unable to update listener {0} due to ' + '"{1}". Skipping this listener.'.format( + listener.id, e)) listener_repo = repo.ListenerRepository() listener_repo.update(db_apis.get_session(), listener.id, provisioning_status=consts.ERROR, @@ -531,13 +531,14 @@ class HaproxyAmphoraLoadBalancerDriver( # Handle the client cert(s) and key if pool.tls_certificate_id: data = cert_parser.load_certificates_data(self.cert_manager, pool) - pem = cert_parser.build_pem(data) + tls_cert = data['tls_cert'] + pem = cert_parser.build_pem(tls_cert) try: pem = pem.encode('utf-8') except AttributeError: pass md5 = hashlib.md5(pem).hexdigest() # nosec - name = '{id}.pem'.format(id=data.id) + name = '{id}.pem'.format(id=tls_cert.id) if amphora and obj_id: self._upload_cert(amphora, obj_id, pem=pem, md5=md5, name=name) pool_cert_dict['client_cert'] = os.path.join( diff --git a/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver_0_5.py b/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver_0_5.py index 9a0b4fca4f..003bdc59c2 100644 --- a/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver_0_5.py +++ b/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver_0_5.py @@ -25,6 +25,7 @@ from octavia.amphorae.driver_exceptions import exceptions as driver_except from octavia.amphorae.drivers.haproxy import exceptions as exc from octavia.amphorae.drivers.haproxy import rest_api_driver as driver from octavia.common import constants +from octavia.common import data_models from octavia.common import utils as octavia_utils from octavia.db import models from octavia.network import data_models as network_models @@ -399,13 +400,14 @@ class TestHaproxyAmphoraLoadBalancerDriverTest(base.TestCase): conf.config(group="haproxy_amphora", base_cert_dir=fake_cert_dir) sample_listener = sample_configs_split.sample_listener_tuple( pool_cert=True, pool_ca_cert=True, pool_crl=True) - cert_data_mock = mock.MagicMock() - cert_data_mock.id = uuidutils.generate_uuid() - mock_load_certs.return_value = cert_data_mock + pool_cert = data_models.TLSContainer( + id=uuidutils.generate_uuid(), certificate='pool cert') + pool_data = {'tls_cert': pool_cert, 'sni_certs': []} + mock_load_certs.return_value = pool_data fake_pem = b'fake pem' mock_build_pem.return_value = fake_pem ref_md5 = hashlib.md5(fake_pem).hexdigest() # nosec - ref_name = '{id}.pem'.format(id=cert_data_mock.id) + ref_name = '{id}.pem'.format(id=pool_cert.id) ref_path = '{cert_dir}/{list_id}/{name}'.format( cert_dir=fake_cert_dir, list_id=sample_listener.id, name=ref_name) ref_ca_name = 'fake_ca.pem' @@ -432,7 +434,7 @@ class TestHaproxyAmphoraLoadBalancerDriverTest(base.TestCase): sample_listener.default_pool.crl_container_id, self.amp, sample_listener.load_balancer.id)] - mock_build_pem.assert_called_once_with(cert_data_mock) + mock_build_pem.assert_called_once_with(pool_cert) mock_upload_cert.assert_called_once_with( self.amp, sample_listener.load_balancer.id, pem=fake_pem, md5=ref_md5, name=ref_name) diff --git a/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver_1_0.py b/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver_1_0.py index 13bd6011da..37024591b3 100644 --- a/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver_1_0.py +++ b/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver_1_0.py @@ -25,6 +25,7 @@ from octavia.amphorae.driver_exceptions import exceptions as driver_except from octavia.amphorae.drivers.haproxy import exceptions as exc from octavia.amphorae.drivers.haproxy import rest_api_driver as driver from octavia.common import constants +from octavia.common import data_models from octavia.common import utils as octavia_utils from octavia.db import models from octavia.network import data_models as network_models @@ -400,13 +401,14 @@ class TestHaproxyAmphoraLoadBalancerDriverTest(base.TestCase): conf.config(group="haproxy_amphora", base_cert_dir=fake_cert_dir) sample_listener = sample_configs_combined.sample_listener_tuple( pool_cert=True, pool_ca_cert=True, pool_crl=True) - cert_data_mock = mock.MagicMock() - cert_data_mock.id = uuidutils.generate_uuid() - mock_load_certs.return_value = cert_data_mock + pool_cert = data_models.TLSContainer( + id=uuidutils.generate_uuid(), certificate='pool cert') + pool_data = {'tls_cert': pool_cert, 'sni_certs': []} + mock_load_certs.return_value = pool_data fake_pem = b'fake pem' mock_build_pem.return_value = fake_pem ref_md5 = hashlib.md5(fake_pem).hexdigest() # nosec - ref_name = '{id}.pem'.format(id=cert_data_mock.id) + ref_name = '{id}.pem'.format(id=pool_cert.id) ref_path = '{cert_dir}/{list_id}/{name}'.format( cert_dir=fake_cert_dir, list_id=sample_listener.id, name=ref_name) ref_ca_name = 'fake_ca.pem' @@ -433,7 +435,7 @@ class TestHaproxyAmphoraLoadBalancerDriverTest(base.TestCase): sample_listener.default_pool.crl_container_id, self.amp, sample_listener.load_balancer.id)] - mock_build_pem.assert_called_once_with(cert_data_mock) + mock_build_pem.assert_called_once_with(pool_cert) mock_upload_cert.assert_called_once_with( self.amp, sample_listener.load_balancer.id, pem=fake_pem, md5=ref_md5, name=ref_name) diff --git a/releasenotes/notes/fix-tls-enabled-pool-provisioning-e3adb987244a025a.yaml b/releasenotes/notes/fix-tls-enabled-pool-provisioning-e3adb987244a025a.yaml new file mode 100644 index 0000000000..8d233cf2f0 --- /dev/null +++ b/releasenotes/notes/fix-tls-enabled-pool-provisioning-e3adb987244a025a.yaml @@ -0,0 +1,4 @@ +--- +fixes: + - | + Fixed an issue where TLS-enabled pools would fail to provision.