Merge "Fixing TLS configuration issues"

neutron_lbaas/common/cert_manager/barbican_cert_manager.py

@@ -14,6 +14,7 @@
 
 from barbicanclient import client as barbican_client
 from neutron.i18n import _LI, _LW, _LE
+from neutron.plugins.common import constants
 from oslo_config import cfg
 from oslo_log import log as logging
 from oslo_utils import excutils
@@ -169,13 +170,13 @@ class CertManager(cert_manager.CertManager):
 
     @staticmethod
     def get_cert(cert_ref, service_name='lbaas',
-                 resource_ref=None,
+                 lb_id=None,
                  check_only=False, **kwargs):
         """Retrieves the specified cert and registers as a consumer.
 
         :param cert_ref: the UUID of the cert to retrieve
         :param service_name: Friendly name for the consuming service
-        :param resource_ref: Full HATEOAS reference to the consuming resource
+        :param lb_id: Loadbalancer id for building resource consumer URL
         :param check_only: Read Certificate data without registering
 
         :return: octavia.certificates.common.Cert representation of the
@@ -196,7 +197,7 @@ class CertManager(cert_manager.CertManager):
                 cert_container = connection.containers.register_consumer(
                     container_ref=cert_ref,
                     name=service_name,
-                    url=resource_ref
+                    url=CertManager._get_service_url(lb_id)
                 )
             return Cert(cert_container)
         except Exception:
@@ -204,12 +205,12 @@ class CertManager(cert_manager.CertManager):
                 LOG.exception(_LE("Error getting {0}").format(cert_ref))
 
     @staticmethod
-    def delete_cert(cert_ref, resource_ref, service_name='lbaas', **kwargs):
+    def delete_cert(cert_ref, lb_id, service_name='lbaas', **kwargs):
         """Deregister as a consumer for the specified cert.
 
         :param cert_ref: the UUID of the cert to retrieve
         :param service_name: Friendly name for the consuming service
-        :param resource_ref: Full HATEOAS reference to the consuming resource
+        :param lb_id: Loadbalancer id for building resource consumer URL
 
         :raises Exception: if deregistration fails
         """
@@ -222,7 +223,7 @@ class CertManager(cert_manager.CertManager):
             connection.containers.remove_consumer(
                 container_ref=cert_ref,
                 name=service_name,
-                url=resource_ref
+                url=CertManager._get_service_url(lb_id)
             )
         except Exception:
             with excutils.save_and_reraise_exception():
@@ -256,3 +257,12 @@ class CertManager(cert_manager.CertManager):
                 LOG.exception(_LE(
                     "Error recursively deleting certificate container {0}"
                 ).format(cert_ref))
+
+    @staticmethod
+    def _get_service_url(lb_id):
+        # Format: <servicename>://<region>/<resource>/<object_id>
+        return "{0}://{1}/{2}/{3}".format(
+            cfg.CONF.service_auth.service_name,
+            cfg.CONF.service_auth.region,
+            constants.LOADBALANCER,
+            lb_id)

neutron_lbaas/services/loadbalancer/plugin.py

@@ -566,14 +566,17 @@ class LoadBalancerPluginv2(loadbalancerv2.LoadBalancerPluginBaseV2):
     def _validate_tls(self, listener, curr_listener=None):
         def validate_tls_container(container_ref):
             cert_container = None
+            lb_id = None
+
             if curr_listener:
-                service_url = self._get_service_url(curr_listener)
+                lb_id = curr_listener['loadbalancer_id']
             else:
-                service_url = self._get_service_url(listener)
+                lb_id = listener.get('loadbalancer_id')
+
             try:
                 cert_container = CERT_MANAGER_PLUGIN.CertManager.get_cert(
                     container_ref,
-                    resource_ref=service_url)
+                    lb_id=lb_id)
             except Exception as e:
                 if hasattr(e, 'status_code') and e.status_code == 404:
                     raise loadbalancerv2.TLSContainerNotFound(
@@ -593,7 +596,7 @@ class LoadBalancerPluginv2(loadbalancerv2.LoadBalancerPluginBaseV2):
                     intermediates=cert_container.get_intermediates())
             except Exception as e:
                 CERT_MANAGER_PLUGIN.CertManager.delete_cert(
-                    container_ref, self._get_service_url(listener))
+                    container_ref, lb_id)
                 raise loadbalancerv2.TLSContainerInvalid(
                     container_id=container_ref, reason=str(e))
 
@@ -629,14 +632,6 @@ class LoadBalancerPluginv2(loadbalancerv2.LoadBalancerPluginBaseV2):
 
         return len(to_validate) > 0
 
-    def _get_service_url(self, listener):
-        # Format: <servicename>://<region>/<resource>/<object_id>
-        return "{0}://{1}/{2}/{3}".format(
-            cfg.CONF.service_auth.service_name,
-            cfg.CONF.service_auth.region,
-            constants.LOADBALANCER,
-            listener['loadbalancer_id'])
-
     def create_listener(self, context, listener):
         listener = listener.get('listener')
         lb_id = listener.get('loadbalancer_id')

neutron_lbaas/tests/unit/common/cert_manager/test_barbican.py

@@ -18,6 +18,7 @@ import mock
 import neutron_lbaas.common.cert_manager.barbican_cert_manager as bbq_common
 from neutron_lbaas.common import keystone
 import neutron_lbaas.tests.base as base
+from oslo_config import cfg
 
 
 class TestBarbicanAuth(base.BaseTestCase):
@@ -51,6 +52,18 @@ class TestBarbicanAuth(base.BaseTestCase):
         bc2 = bbq_common.BarbicanKeystoneAuth.get_barbican_client()
         self.assertIs(bc1, bc2)
 
+    def test_get_service_url(self):
+        # Format: <servicename>://<region>/<resource>/<object_id>
+        cfg.CONF.set_override('service_name',
+                              'lbaas',
+                              'service_auth')
+        cfg.CONF.set_override('region',
+                              'RegionOne',
+                              'service_auth')
+        self.assertEqual(
+            'lbaas://RegionOne/LOADBALANCER/LB-ID',
+            bbq_common.CertManager._get_service_url('LB-ID'))
+
 
 class TestBarbicanCert(base.BaseTestCase):
 

neutron_lbaas/tests/unit/db/loadbalancer/test_db_loadbalancerv2.py

@@ -925,21 +925,6 @@ class LbaasListenerTests(ListenerTestBase):
                               context.get_admin_context(),
                               {'listener': listener_data})
 
-    def test_get_service_url(self):
-        # Format: <servicename>://<region>/<resource>/<object_id>
-        cfg.CONF.set_override('service_name',
-                              'lbaas',
-                              'service_auth')
-        cfg.CONF.set_override('region',
-                              'RegionOne',
-                              'service_auth')
-        listner = {
-            'loadbalancer_id': self.lb_id
-        }
-        self.assertEqual(
-            'lbaas://RegionOne/LOADBALANCER/{0}'.format(self.lb_id),
-            self.plugin._get_service_url(listner))
-
     def test_create_listener_with_tls_invalid_container(self, **extras):
         default_tls_container_ref = uuidutils.generate_uuid()
         cfg.CONF.set_override('service_name',
@@ -977,7 +962,7 @@ class LbaasListenerTests(ListenerTestBase):
                               {'listener': listener_data})
             rm_consumer_mock.assert_called_once_with(
                 listener_data['default_tls_container_ref'],
-                'lbaas://RegionOne/LOADBALANCER/{0}'.format(self.lb_id))
+                self.lb_id)
 
     def test_create_listener_with_tls(self, **extras):
         default_tls_container_ref = uuidutils.generate_uuid()