From 99172f9c2db51369ec85145285a3fbc3b1b257a7 Mon Sep 17 00:00:00 2001
From: jneeee <jneeee@qq.com>
Date: Tue, 29 Nov 2022 14:34:56 +0800
Subject: [PATCH] Get endpoint id from keystone by service name

Get endpoint id from keystone by service name and service type.

The doc says that either a combination of service name, service type and region name,
or simply endpoint_id is ok. but actually the combination dosn't work.

This patch fix this.

Closes-bug: #1931875
Change-Id: Ifbca3568aa46869d6e35377eab5a664d8fa49846
---
 oslo_limit/limit.py                           | 40 ++++++++++++++++++-
 oslo_limit/tests/test_limit.py                | 36 +++++++++++++++++
 .../notes/bug-1931875-b5018c49a6993688.yaml   |  6 +++
 3 files changed, 80 insertions(+), 2 deletions(-)
 create mode 100644 releasenotes/notes/bug-1931875-b5018c49a6993688.yaml

diff --git a/oslo_limit/limit.py b/oslo_limit/limit.py
index 0c6635f..a0abf69 100644
--- a/oslo_limit/limit.py
+++ b/oslo_limit/limit.py
@@ -255,7 +255,6 @@ class _EnforcerUtils(object):
     """Logic common used by multiple enforcers"""
 
     def __init__(self, cache=True):
-        self.connection = _get_keystone_connection()
         self.should_cache = cache
         # {project_id: {resource_name: project_limit}}
         self.plimit_cache = defaultdict(dict)
@@ -263,7 +262,11 @@ class _EnforcerUtils(object):
         self.rlimit_cache = {}
 
         # get and cache endpoint info
-        endpoint_id = CONF.oslo_limit.endpoint_id
+        endpoint_id = _get_endpoint_id_from_keystone()
+        # Case we set attrs to global _SDK_CONNECTION, need move new
+        # assignment below
+        self.connection = _get_keystone_connection()
+
         self._endpoint = self.connection.get_endpoint(endpoint_id)
         if not self._endpoint:
             raise ValueError("can't find endpoint for %s" % endpoint_id)
@@ -405,3 +408,36 @@ class _EnforcerUtils(object):
             self.rlimit_cache[resource_name] = reg_limit
 
         return reg_limit
+
+
+def _get_endpoint_id_from_keystone():
+    '''Return endpoint_id from CONF or from keystone.
+
+    If from keystone, write into CONF(memery, not file). So
+    that we only need connect to keystone at most once.
+
+    :return: endpoint_id, can be None just as the legacy act.
+    '''
+
+    if CONF.oslo_limit.endpoint_id:
+        return CONF.oslo_limit.endpoint_id
+
+    epid = None
+    try:
+        if CONF.oslo_limit.service_name and \
+                CONF.oslo_limit.service_type:
+            conn = _get_keystone_connection()
+            conn.service_name = CONF.oslo_limit.service_name
+            conn.service_type = CONF.oslo_limit.service_type
+            # Actually region_name is optional.
+            if CONF.oslo_limit.region_name:
+                conn.region_name = CONF.oslo_limit.region_name
+            epid = conn.get_endpoint_data().endpoint_id
+            # override CONF in memery so that only run once in begining.
+            CONF.set_override('endpoint_id', epid, group='oslo_limit')
+        else:
+            raise ValueError("service name and type aren't given")
+    except Exception as e:
+        msg = 'Get endpoint id from keystone failed: %s' % e
+        LOG.error(msg)
+    return epid
diff --git a/oslo_limit/tests/test_limit.py b/oslo_limit/tests/test_limit.py
index 840ab38..df78e2a 100644
--- a/oslo_limit/tests/test_limit.py
+++ b/oslo_limit/tests/test_limit.py
@@ -21,6 +21,7 @@ Tests for `limit` module.
 from unittest import mock
 import uuid
 
+from keystoneauth1.discover import EndpointData
 from openstack.identity.v3 import endpoint
 from openstack.identity.v3 import limit as klimit
 from openstack.identity.v3 import registered_limit
@@ -312,6 +313,7 @@ class TestEnforcerUtils(base.BaseTestCase):
         super(TestEnforcerUtils, self).setUp()
         self.mock_conn = mock.MagicMock()
         limit._SDK_CONNECTION = self.mock_conn
+        self.config_fixture = self.useFixture(config_fixture.Config(CONF))
 
     def test_get_endpoint(self):
         fake_endpoint = endpoint.Endpoint()
@@ -510,3 +512,37 @@ class TestEnforcerUtils(base.BaseTestCase):
             utils._get_limit(None, 'foo')
             mgrl.assert_called_once_with('foo')
             mgpl.assert_not_called()
+
+    def test_get_endpoint_id_with_service_name_and_type(self):
+        '''
+        Ensure that we can get endpoint_id when service_name and
+        service_type was set.
+        '''
+        fake_epdata = EndpointData(
+            api_version=(2, 9),
+            catalog_url='https://[2001::1]:9292',
+            endpoint_id='3be88764e02c4392b51asdf210c7be09',
+            interface='public',
+            raw_endpoint={
+                'id': '3be88764e02c4392b51asdf210c7be09',
+                'interface': 'public',
+                'region_id': 'RegionOne',
+                'url': 'https://[2001::1]:9292',
+                'region': 'RegionOne',
+            },
+            region_name='RegionOne',
+            service_id='424368f0173e441asdf9f9145048f8e',
+            service_name='glance', service_type='image',
+            service_url='https://[2001::1]:9292/v2/',
+        )
+        self.mock_conn.get_endpoint_data.return_value = fake_epdata
+
+        self.config_fixture.config(
+            group='oslo_limit',
+            service_name='glance',
+            service_type='image',
+        )
+        limit._EnforcerUtils()
+
+        self.mock_conn.get_endpoint.assert_called_once_with(
+            '3be88764e02c4392b51asdf210c7be09')
diff --git a/releasenotes/notes/bug-1931875-b5018c49a6993688.yaml b/releasenotes/notes/bug-1931875-b5018c49a6993688.yaml
new file mode 100644
index 0000000..763ca0c
--- /dev/null
+++ b/releasenotes/notes/bug-1931875-b5018c49a6993688.yaml
@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    [`bug 1931875 <https://bugs.launchpad.net/keystone/+bug/1931875>`_]
+    Fix the authentication error about the combination of service_name
+    and service_type in configuration.