From 88fb01884010758d302f651b2525d96132326db1 Mon Sep 17 00:00:00 2001 From: Adrian Turjak Date: Wed, 30 May 2018 15:49:04 +1200 Subject: [PATCH] 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 | 12 ++++++ openstack_auth/tests/unit/test_utils.py | 42 +++++++++++++++++++ openstack_auth/user.py | 5 +-- openstack_auth/utils.py | 33 +++++++++++---- .../local/local_settings.py.example | 4 +- ...fault-service-region-0cafecaafa1af5be.yaml | 16 +++++++ 6 files changed, 98 insertions(+), 14 deletions(-) create mode 100644 releasenotes/notes/bp-global-default-service-region-0cafecaafa1af5be.yaml diff --git a/doc/source/configuration/settings.rst b/doc/source/configuration/settings.rst index 8bab749aac..1c3fcfc761 100644 --- a/doc/source/configuration/settings.rst +++ b/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 ~~~~~~~~~~~~~~~~~~~ diff --git a/openstack_auth/tests/unit/test_utils.py b/openstack_auth/tests/unit/test_utils.py index ada5902bdc..012476344b 100644 --- a/openstack_auth/tests/unit/test_utils.py +++ b/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): diff --git a/openstack_auth/user.py b/openstack_auth/user.py index 3532ec0d5e..dd0f605a21 100644 --- a/openstack_auth/user.py +++ b/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'], diff --git a/openstack_auth/utils.py b/openstack_auth/utils.py index d84489da42..f17013bd9a 100644 --- a/openstack_auth/utils.py +++ b/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 diff --git a/openstack_dashboard/local/local_settings.py.example b/openstack_dashboard/local/local_settings.py.example index d21242f61f..cefe303f91 100644 --- a/openstack_dashboard/local/local_settings.py.example +++ b/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. diff --git a/releasenotes/notes/bp-global-default-service-region-0cafecaafa1af5be.yaml b/releasenotes/notes/bp-global-default-service-region-0cafecaafa1af5be.yaml new file mode 100644 index 0000000000..6bdf095d7b --- /dev/null +++ b/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``.