Rework DEFAULT_SERVICE_REGIONS

DEFAULT_SERVICE_REGIONS is cumbersome when you have
more than one keystone endpoint and all you want to do
is set a global default service region.

This adds '*' as an optional fallback key to mean global default.
If an endpoint matches it will take precedence over the '*' value.

This also fixes the precedence order for DEFAULT_SERVICE_REGIONS
so that a user controlled cookie is used instead when that cookie
is valid for the given catalog. This changes the way the setting
works, but retains the intended result the setting was originally
intended for.

Change-Id: Ieefbd642d853fcfcf22a17d9edcc7daae72790a4
blueprint: global-default-service-region
Closes-Bug: #1772345
Related-Bugs: #1359774 #1703390

doc/source/configuration/settings.rst

@@ -1145,6 +1145,18 @@ Example:
         OPENSTACK_KEYSTONE_URL: 'RegionOne'
     }
 
+As of Rocky you can optionally you can set ``'*'`` as the key, and if no
+matching endpoint is found this will be treated as a global default.
+
+Example:
+
+.. code-block:: python
+
+    DEFAULT_SERVICE_REGIONS = {
+        '*': 'RegionOne',
+        OPENSTACK_KEYSTONE_URL: 'RegionTwo'
+    }
+
 ENABLE_CLIENT_TOKEN
 ~~~~~~~~~~~~~~~~~~~
 

openstack_auth/tests/unit/test_utils.py

@@ -14,11 +14,26 @@
 from django.conf import settings
 from django import http
 from django import test
+from django.test import client
 from django.test.utils import override_settings
 
 from openstack_auth import utils
 
 
+FAKE_CATALOG = [
+    {
+        'name': 'fake_service',
+        'type': "not-identity",
+        'endpoints': [
+            {'region': 'RegionOne'},
+            {'region': 'RegionTwo'},
+            {'region': 'RegionThree'},
+            {'region': 'RegionFour'},
+        ]
+    },
+]
+
+
 class RoleTestCaseAdmin(test.TestCase):
 
     def test_get_admin_roles_with_default_value(self):
@@ -75,6 +90,33 @@ class UtilsTestCase(test.TestCase):
         for src, expected in test_urls:
             self.assertEqual(expected, utils.fix_auth_url_version_prefix(src))
 
+    @override_settings(DEFAULT_SERVICE_REGIONS={
+        'http://example.com': 'RegionThree', '*': 'RegionFour'})
+    def test_default_services_region_precedence(self):
+        request = client.RequestFactory().get('fake')
+
+        # Cookie is valid, so should be region source
+        request.COOKIES['services_region'] = "RegionTwo"
+        default_region = utils.default_services_region(
+            FAKE_CATALOG, request=request, ks_endpoint='http://example.com')
+        self.assertEqual("RegionTwo", default_region)
+
+        # Cookie is invalid, so ks_endpoint is source
+        request.COOKIES['services_region'] = "Not_valid_region"
+        default_region = utils.default_services_region(
+            FAKE_CATALOG, request=request, ks_endpoint='http://example.com')
+        self.assertEqual("RegionThree", default_region)
+
+        # endpoint and cookie are invalid, so source is "*" key
+        default_region = utils.default_services_region(
+            FAKE_CATALOG, request=request, ks_endpoint='not_a_match')
+        self.assertEqual("RegionFour", default_region)
+
+    def test_default_services_region_fallback(self):
+        # Test that first region found in catalog is returned
+        default_region = utils.default_services_region(FAKE_CATALOG)
+        self.assertEqual("RegionOne", default_region)
+
 
 class BehindProxyTestCase(test.TestCase):
 

openstack_auth/user.py

@@ -40,11 +40,8 @@ def set_session_from_user(request, user):
 
 def create_user_from_token(request, token, endpoint, services_region=None):
     # if the region is provided, use that, otherwise use the preferred region
-    default_service_regions = getattr(settings, 'DEFAULT_SERVICE_REGIONS', {})
-    default_service_region = default_service_regions.get(endpoint)
     svc_region = services_region or \
-        utils.default_services_region(token.serviceCatalog, request,
-                                      selected_region=default_service_region)
+        utils.default_services_region(token.serviceCatalog, request, endpoint)
     return User(id=token.user['id'],
                 token=token,
                 user=token.user['name'],

openstack_auth/utils.py

@@ -343,10 +343,17 @@ def get_project_list(*args, **kwargs):
 
 
 def default_services_region(service_catalog, request=None,
-                            selected_region=None):
-    """Returns the first endpoint region for first non-identity service.
+                            ks_endpoint=None):
+    """Return the default service region.
 
-    Extracted from the service catalog.
+    Order of precedence:
+    1. 'services_region' cookie value
+    2. Matching endpoint in DEFAULT_SERVICE_REGIONS
+    3. '*' key in DEFAULT_SERVICE_REGIONS
+    4. First valid region from catalog
+
+    In each case the value must also be present in available_regions or
+    we move to the next level of precedence.
     """
     if service_catalog:
         available_regions = [get_endpoint_region(endpoint) for service
@@ -367,12 +374,20 @@ def default_services_region(service_catalog, request=None,
                 LOG.error('No regions can be found in the service catalog.')
                 return None
 
-        if request and selected_region is None:
-            selected_region = request.COOKIES.get('services_region',
-                                                  available_regions[0])
-        if selected_region not in available_regions:
-            selected_region = available_regions[0]
-        return selected_region
+        region_options = []
+        if request:
+            region_options.append(request.COOKIES.get('services_region'))
+        if ks_endpoint:
+            default_service_regions = getattr(
+                settings, 'DEFAULT_SERVICE_REGIONS', {})
+            region_options.append(default_service_regions.get(ks_endpoint))
+        region_options.append(
+            getattr(settings, 'DEFAULT_SERVICE_REGIONS', {}).get('*'))
+
+        for region in region_options:
+            if region in available_regions:
+                return region
+        return available_regions[0]
     return None
 
 

openstack_dashboard/local/local_settings.py.example

@@ -190,8 +190,10 @@ OPENSTACK_KEYSTONE_DEFAULT_ROLE = "_member_"
 # For setting the default service region on a per-endpoint basis. Note that the
 # default value for this setting is {}, and below is just an example of how it
 # should be specified.
+# A key of '*' is an optional global default if no other key matches.
 #DEFAULT_SERVICE_REGIONS = {
-#    OPENSTACK_KEYSTONE_URL: 'RegionOne'
+#    '*': 'RegionOne'
+#    OPENSTACK_KEYSTONE_URL: 'RegionTwo'
 #}
 
 # Enables keystone web single-sign-on if set to True.

releasenotes/notes/bp-global-default-service-region-0cafecaafa1af5be.yaml

@@ -0,0 +1,16 @@
+---
+features:
+  - |
+    DEFAULT_SERVICE_REGIONS can now take '*' as a key which serves either as a
+    fallback service region, or the default region if no other keys are set.
+upgrade:
+  - |
+    [:bug:`1772345`]
+    ``DEFAULT_SERVICE_REGIONS`` no longer overrides the cookie value from
+    ``services_region``. This fixes the UX where a user controlled value
+    keeps being overridden by a setting and changes ``DEFAULT_SERVICE_REGIONS``
+    to act as a default (as the name implies) per endpoint if the cookie is not
+    set rather than an override. The cookie will still be overridden when
+    it is for a region not present in the user's current catalog, so this will
+    still handle the original multi-keystone case that requried the
+    introduction of ``DEFAULT_SERVICE_REGIONS``.