Infer version from old versioned service type aliases

The last piece of service type alias support is to handle volumev2,
volumev3, workflowv2, workflowv3 and friends.

Although it's an annoying scenario, luckily legacy code that uses them
has a clear meaning. volumev2, version='3' is just legit not a thing.

Needed-By: https://review.openstack.org/564494
Change-Id: Iec09bcb16d8e9b09e09bf12d03c2a55e679ad70c

keystoneauth1/discover.py

@@ -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
 
 

keystoneauth1/exceptions/discovery.py

@@ -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'

keystoneauth1/identity/base.py

@@ -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,

keystoneauth1/tests/unit/identity/test_identity_common.py

@@ -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,

keystoneauth1/tests/unit/test_discovery.py

@@ -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):
+        def normalize_raises(ver, min, max, in_type):
-            self.assertRaises(ValueError,
+            self.assertRaises(
-                              discover._normalize_version_args, ver, min, max)
+                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', 'v2', None, None)
-        normalize_raises('v1', None, 'v2')
+        normalize_raises('v1', None, 'v2', None)
-        normalize_raises(None, 'latest', 'v1')
+        normalize_raises(None, 'latest', 'v1', None)
-        normalize_raises(None, 'v1.2', 'v1.1')
+        normalize_raises(None, 'v1.2', 'v1.1', None)
-        normalize_raises(None, 'v1.2', 1)
+        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):