Merge "Rework DEFAULT_SERVICE_REGIONS"

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``.