Merge "Infer version from old versioned service type aliases"

This commit is contained in:
Zuul 2018-05-08 15:40:15 +00:00 committed by Gerrit Code Review
commit 3308cd9944
5 changed files with 141 additions and 32 deletions

View File

@ -220,7 +220,28 @@ def normalize_version_number(version):
raise TypeError('Invalid version specified: %s' % version)
def _normalize_version_args(version, min_version, max_version):
def _normalize_version_args(
version, min_version, max_version, service_type=None):
# The sins of our fathers become the blood on our hands.
# If a user requests an old-style service type such as volumev2, then they
# are inherently requesting the major API version 2. It's not a good
# interface, but it's the one that was imposed on the world years ago
# because the client libraries all hid the version discovery document.
# In order to be able to ensure that a user who requests volumev2 does not
# get a block-storage endpoint that only provides v3 of the block-storage
# service, we need to pull the version out of the service_type. The
# service-types-authority will prevent the growth of new monstrosities such
# as this, but in order to move forward without breaking people, we have
# to just cry in the corner while striking ourselves with thorned branches.
# That said, for sure only do this hack for officially known service_types.
if (service_type and
_SERVICE_TYPES.is_known(service_type) and
service_type[-1].isdigit() and
service_type[-2] == 'v'):
implied_version = normalize_version_number(service_type[-1])
else:
implied_version = None
if version and (min_version or max_version):
raise ValueError(
"version is mutually exclusive with min_version and max_version")
@ -229,6 +250,12 @@ def _normalize_version_args(version, min_version, max_version):
# Explode this into min_version and max_version
min_version = normalize_version_number(version)
max_version = (min_version[0], LATEST)
if implied_version:
if min_version[0] != implied_version[0]:
raise exceptions.ImpliedVersionMismatch(
service_type=service_type,
implied=implied_version,
given=version_to_string(version))
return min_version, max_version
if min_version == 'latest':
@ -257,6 +284,27 @@ def _normalize_version_args(version, min_version, max_version):
if None not in (min_version, max_version) and max_version < min_version:
raise ValueError("min_version cannot be greater than max_version")
if implied_version:
if min_version:
if min_version[0] != implied_version[0]:
raise exceptions.ImpliedMinVersionMismatch(
service_type=service_type,
implied=implied_version,
given=version_to_string(min_version))
else:
min_version = implied_version
# If 'latest' is provided with a versioned service-type like
# volumev2 - the user wants the latest of volumev2, not the latest
# of block-storage.
if max_version and max_version[0] != LATEST:
if max_version[0] != implied_version[0]:
raise exceptions.ImpliedMaxVersionMismatch(
service_type=service_type,
implied=implied_version,
given=version_to_string(max_version))
else:
max_version = (implied_version[0], LATEST)
return min_version, max_version

View File

@ -10,10 +10,17 @@
# License for the specific language governing permissions and limitations
# under the License.
import os_service_types
from keystoneauth1.exceptions import base
_SERVICE_TYPES = os_service_types.ServiceTypes()
__all__ = ('DiscoveryFailure',
'ImpliedVersionMismatch',
'ImpliedMinVersionMismatch',
'ImpliedMaxVersionMismatch',
'VersionNotAvailable')
@ -23,3 +30,28 @@ class DiscoveryFailure(base.ClientException):
class VersionNotAvailable(DiscoveryFailure):
message = "Discovery failed. Requested version is not available."
class ImpliedVersionMismatch(ValueError):
label = 'version'
def __init__(self, service_type, implied, given):
super(ImpliedVersionMismatch, self).__init__(
"service_type {service_type} was given which implies"
" major API version {implied} but {label} of"
" {given} was also given. Please update your code"
" to use the official service_type {official_type}.".format(
service_type=service_type,
implied=str(implied[0]),
given=given,
label=self.label,
official_type=_SERVICE_TYPES.get_service_type(service_type),
))
class ImpliedMinVersionMismatch(ImpliedVersionMismatch):
label = 'min_version'
class ImpliedMaxVersionMismatch(ImpliedVersionMismatch):
label = 'max_version'

View File

@ -226,7 +226,7 @@ class BaseIdentityPlugin(plugin.BaseAuthPlugin):
allow = allow or {}
min_version, max_version = discover._normalize_version_args(
None, min_version, max_version)
None, min_version, max_version, service_type=service_type)
# NOTE(jamielennox): if you specifically ask for requests to be sent to
# the auth url then we can ignore many of the checks. Typically if you
@ -365,7 +365,7 @@ class BaseIdentityPlugin(plugin.BaseAuthPlugin):
# Explode `version` into min_version and max_version - everything below
# here uses the latter rather than the former.
min_version, max_version = discover._normalize_version_args(
version, min_version, max_version)
version, min_version, max_version, service_type=service_type)
# Set discover_versions to False since we're only going to return
# a URL. Fetching the microversion data would be needlessly
# expensive in the common case. However, discover_versions=False
@ -487,7 +487,7 @@ class BaseIdentityPlugin(plugin.BaseAuthPlugin):
# Explode `version` into min_version and max_version - everything below
# here uses the latter rather than the former.
min_version, max_version = discover._normalize_version_args(
version, min_version, max_version)
version, min_version, max_version, service_type=service_type)
# Using functools.partial here just to reduce copy-pasta of params
get_endpoint_data = functools.partial(
self.get_endpoint_data,

View File

@ -1053,7 +1053,7 @@ class CommonIdentityTests(object):
# We should only try to fetch the versioned discovery url once
self.requests_mock.get(
self.TEST_VOLUME.versions['v3'].discovery.public, resps)
self.TEST_VOLUME.versions['v3'].discovery.public + '/', resps)
data = a.get_endpoint_data(session=s,
service_type='volumev3',
@ -1128,7 +1128,7 @@ class CommonIdentityTests(object):
# Fetch v2.0 first - since that doesn't match endpoint optimization,
# it should fetch the unversioned endpoint
v2_data = s.get_endpoint_data(service_type='volumev3',
v2_data = s.get_endpoint_data(service_type='block-storage',
interface='public',
min_version='2.0',
max_version='2.latest',
@ -1179,10 +1179,10 @@ class CommonIdentityTests(object):
self.requests_mock.get(
self.TEST_VOLUME.unversioned.public + '/', json=disc)
# We're requesting version 2 of volumev3 to make sure we
# We're requesting version 2 of block-storage to make sure we
# trigger the logic constructing the unversioned endpoint from the
# versioned endpoint in the catalog
s.get_endpoint_data(service_type='volumev3',
s.get_endpoint_data(service_type='block-storage',
interface='public',
min_version='2.0',
max_version='2.latest',
@ -1413,6 +1413,15 @@ class V3(CommonIdentityTests, utils.TestCase):
internal=self.TEST_VOLUME.versions['v3'].service.internal,
region=region)
# Add block-storage as a versioned endpoint so that we can test
# versioned to unversioned inference.
svc = token.add_service('block-storage')
svc.add_standard_endpoints(
admin=self.TEST_VOLUME.versions['v3'].service.admin,
public=self.TEST_VOLUME.versions['v3'].service.public,
internal=self.TEST_VOLUME.versions['v3'].service.internal,
region=region)
svc = token.add_service('baremetal')
svc.add_standard_endpoints(
internal=self.TEST_BAREMETAL_INTERNAL,
@ -1490,6 +1499,15 @@ class V2(CommonIdentityTests, utils.TestCase):
internal=self.TEST_VOLUME.versions['v3'].service.internal,
region=region)
# Add block-storage as a versioned endpoint so that we can test
# versioned to unversioned inferance.
svc = token.add_service('block-storage')
svc.add_endpoint(
admin=self.TEST_VOLUME.versions['v3'].service.admin,
public=self.TEST_VOLUME.versions['v3'].service.public,
internal=self.TEST_VOLUME.versions['v3'].service.internal,
region=region)
svc = token.add_service('baremetal')
svc.add_endpoint(
public=None, admin=None,

View File

@ -323,52 +323,63 @@ class DiscoverUtils(utils.TestCase):
def test_version_args(self):
"""Validate _normalize_version_args."""
def assert_min_max(in_ver, in_min, in_max, out_min, out_max):
def assert_min_max(in_ver, in_min, in_max, in_type, out_min, out_max):
self.assertEqual(
(out_min, out_max),
discover._normalize_version_args(in_ver, in_min, in_max))
discover._normalize_version_args(
in_ver, in_min, in_max, service_type=in_type))
def normalize_raises(ver, min, max):
self.assertRaises(ValueError,
discover._normalize_version_args, ver, min, max)
def normalize_raises(ver, min, max, in_type):
self.assertRaises(
ValueError,
discover._normalize_version_args,
ver, min, max, service_type=in_type)
assert_min_max(None, None, None,
assert_min_max(None, None, None, None,
None, None)
assert_min_max(None, None, 'v1.2',
assert_min_max(None, None, 'v1.2', None,
None, (1, 2))
assert_min_max(None, 'v1.2', 'latest',
assert_min_max(None, 'v1.2', 'latest', None,
(1, 2), (discover.LATEST, discover.LATEST))
assert_min_max(None, 'v1.2', '1.6',
assert_min_max(None, 'v1.2', '1.6', None,
(1, 2), (1, 6))
assert_min_max(None, 'v1.2', '1.latest',
assert_min_max(None, 'v1.2', '1.latest', None,
(1, 2), (1, discover.LATEST))
assert_min_max(None, 'latest', 'latest',
assert_min_max(None, 'latest', 'latest', None,
(discover.LATEST, discover.LATEST),
(discover.LATEST, discover.LATEST))
assert_min_max(None, 'latest', None,
assert_min_max(None, 'latest', None, None,
(discover.LATEST, discover.LATEST),
(discover.LATEST, discover.LATEST))
assert_min_max(None, (1, 2), None,
assert_min_max(None, (1, 2), None, None,
(1, 2), (discover.LATEST, discover.LATEST))
assert_min_max('', ('1', '2'), (1, 6),
assert_min_max('', ('1', '2'), (1, 6), None,
(1, 2), (1, 6))
assert_min_max(None, ('1', '2'), (1, discover.LATEST),
assert_min_max(None, ('1', '2'), (1, discover.LATEST), None,
(1, 2), (1, discover.LATEST))
assert_min_max('v1.2', '', None,
assert_min_max('v1.2', '', None, None,
(1, 2), (1, discover.LATEST))
assert_min_max('1.latest', None, '',
assert_min_max('1.latest', None, '', None,
(1, discover.LATEST), (1, discover.LATEST))
assert_min_max('v1', None, None,
assert_min_max('v1', None, None, None,
(1, 0), (1, discover.LATEST))
assert_min_max('latest', None, None,
assert_min_max('latest', None, None, None,
(discover.LATEST, discover.LATEST),
(discover.LATEST, discover.LATEST))
assert_min_max(None, None, 'latest', 'volumev2',
(2, 0), (2, discover.LATEST))
assert_min_max(None, None, None, 'volumev2',
(2, 0), (2, discover.LATEST))
normalize_raises('v1', 'v2', None)
normalize_raises('v1', None, 'v2')
normalize_raises(None, 'latest', 'v1')
normalize_raises(None, 'v1.2', 'v1.1')
normalize_raises(None, 'v1.2', 1)
normalize_raises('v1', 'v2', None, None)
normalize_raises('v1', None, 'v2', None)
normalize_raises(None, 'latest', 'v1', None)
normalize_raises(None, 'v1.2', 'v1.1', None)
normalize_raises(None, 'v1.2', 1, None)
normalize_raises('v2', None, None, 'volumev3')
normalize_raises('v3', None, None, 'volumev2')
normalize_raises(None, 'v2', None, 'volumev3')
normalize_raises(None, None, 'v3', 'volumev2')
def test_version_to_string(self):
def assert_string(out, inp):