Fix microversion header parsing for service type prefix

The OpenStack API-WG microversion specification defines the standard
header format as:

  OpenStack-API-Version: <service-type> <version>

For the accelerator service this means clients should send:

  OpenStack-API-Version: accelerator 2.1

Commit 279a260b removed the microversion_parse library call that handled
this format and replaced it with a bare headers.get() lookup. That does
not strip the service type prefix, so clients following the standard
format, including keystoneauth and openstacksdk, could not request newer
Cyborg API microversions correctly.

The response header was also changed to omit the service type, returning
bare "2.1" instead of the standard "accelerator 2.1".

Restore service-type-aware parsing for the standard prefixed format.
Keep bare version strings accepted for compatibility, and return the
service type prefix in OpenStack-API-Version response headers.

Related-Bug: #2049489
Change-Id: Ibf4c2a0e1d3b5a7f9c8e6d4b2a0f8e6d4c2b0a8e
Signed-off-by: Sean Mooney <work@seanmooney.info>
Assisted-By: pi opus 4.6

cyborg/api/controllers/base.py

@@ -17,16 +17,21 @@ import datetime
 import functools
 import inspect
 
+import microversion_parse
 import pecan
 import wsme
 
+from oslo_log import log as logging
 from pecan import rest
 from webob import exc
 from wsme import types as wtypes
 
 
+LOG = logging.getLogger(__name__)
+
+
 API_V2 = 'v2'
-# name of attribute to keep version method information
+SERVICE_TYPE = 'accelerator'
 
 
 class APIBase(wtypes.Base):
@@ -104,6 +109,18 @@ class Version:
     def parse_headers(headers, default_version, latest_version):
         """Determine the API version requested based on the headers supplied.
 
+        Per the `OpenStack API-WG guideline`_ the ``OpenStack-API-Version``
+        header carries the service type followed by the requested version,
+        for example ``OpenStack-API-Version: accelerator 2.1``.
+        ``microversion_parse`` handles this standard format.  A bare
+        version string without the service type prefix is also accepted
+        for backward compatibility with older clients but triggers a
+        deprecation warning.
+
+        .. _OpenStack API-WG guideline:
+           https://specs.openstack.org/openstack/api-wg/guidelines/
+           microversion_specification.html
+
         :param headers: webob headers
         :param default_version: version to use if not specified in headers
         :param latest_version: version to use if latest is requested
@@ -111,12 +128,32 @@ class Version:
         :raises: webob.HTTPNotAcceptable
 
         """
-        version_str = headers.get(Version.current_api_version, default_version)
         minimal_version = (2, 0)
+        service_type = SERVICE_TYPE
+
+        # Try the standard format first:
+        #   OpenStack-API-Version: accelerator <version>
+        version_str = microversion_parse.get_version(
+            headers, service_type=service_type
+        )
+
+        if version_str is None:
+            # Fall back to a bare version string without the service
+            # type prefix for backward compatibility.
+            raw = headers.get(Version.current_api_version)
+            if raw is not None:
+                LOG.warning(
+                    'Received OpenStack-API-Version header without '
+                    'the service type prefix.  Clients should send '
+                    "'OpenStack-API-Version: %s <version>' "
+                    'per the API-WG guideline.',
+                    service_type,
+                )
+                version_str = raw
+            else:
+                version_str = default_version
 
         if version_str is None:
-            # If requested header is wrong, Cyborg answers with the minimal
-            # supported version.
             return minimal_version
 
         if version_str.lower() == 'latest':
@@ -125,15 +162,12 @@ class Version:
             parse_str = version_str
 
         try:
-            version = tuple(int(i) for i in parse_str.split('.'))
-        except ValueError:
-            version = minimal_version
-
-        if len(version) != 2:
+            version = microversion_parse.parse_version_string(parse_str)
+        except TypeError:
             raise exc.HTTPNotAcceptable(
                 "Invalid value for %s header" % Version.current_api_version
             )
-        return version
+        return (version.major, version.minor)
 
     def __gt__(self, other):
         return (self.major, self.minor) > (other.major, other.minor)

cyborg/api/controllers/v2/__init__.py

@@ -34,7 +34,10 @@ from cyborg.api.controllers.v2 import versions
 
 def min_version():
     return base.Version(
-        {base.Version.current_api_version: versions.min_version_string()},
+        {
+            base.Version.current_api_version: '%s %s'
+            % (versions.service_type_string(), versions.min_version_string())
+        },
         versions.min_version_string(),
         versions.max_version_string(),
     )
@@ -42,7 +45,10 @@ def min_version():
 
 def max_version():
     return base.Version(
-        {base.Version.current_api_version: versions.max_version_string()},
+        {
+            base.Version.current_api_version: '%s %s'
+            % (versions.service_type_string(), versions.max_version_string())
+        },
         versions.min_version_string(),
         versions.max_version_string(),
     )
@@ -145,7 +151,10 @@ class Controller(rest.RestController):
 
         # assert that requested version is supported
         self._check_version(v, pecan.response.headers)
-        pecan.response.headers[base.Version.current_api_version] = str(v)
+        pecan.response.headers[base.Version.current_api_version] = "%s %s" % (
+            versions.service_type_string(),
+            v,
+        )
         pecan.request.version = v
 
         return super()._route(args, request)

cyborg/api/controllers/v2/versions.py

@@ -14,6 +14,9 @@
 #    under the License.
 
 
+from cyborg.api.controllers import base
+
+
 # This is the version 2 API
 BASE_VERSION = 2
 
@@ -45,7 +48,7 @@ _MAX_VERSION_STRING = '{}.{}'.format(BASE_VERSION, MINOR_MAX_VERSION)
 
 
 def service_type_string():
-    return 'accelerator'
+    return base.SERVICE_TYPE
 
 
 def min_version_string():

cyborg/tests/unit/api/controllers/v2/test_microversion.py

@@ -20,6 +20,9 @@ H_MAX_VER = 'openstack-api-maximum-version'
 H_RESP_VER = 'openstack-api-version'
 MIN_VER = versions.min_version_string()
 MAX_VER = versions.max_version_string()
+# Response header value includes the service type prefix per API-WG spec.
+RESP_MIN = '%s %s' % (SERVICE_TYPE, MIN_VER)
+RESP_MAX = '%s %s' % (SERVICE_TYPE, MAX_VER)
 
 
 class TestMicroversions(api_base.BaseApiTest):
@@ -54,7 +57,7 @@ class TestMicroversions(api_base.BaseApiTest):
         response = self.get_json('/v2', return_json=False)
         self.assertEqual(response.headers[H_MIN_VER], MIN_VER)
         self.assertEqual(response.headers[H_MAX_VER], MAX_VER)
-        self.assertEqual(response.headers[H_RESP_VER], MIN_VER)
+        self.assertEqual(response.headers[H_RESP_VER], RESP_MIN)
         self.assertTrue(
             all(
                 x in response.json.keys()
@@ -68,7 +71,7 @@ class TestMicroversions(api_base.BaseApiTest):
         )
         self.assertEqual(response.headers[H_MIN_VER], MIN_VER)
         self.assertEqual(response.headers[H_MAX_VER], MAX_VER)
-        self.assertEqual(response.headers[H_RESP_VER], '2.0')
+        self.assertEqual(response.headers[H_RESP_VER], RESP_MIN)
         self.assertTrue(
             all(
                 x in response.json.keys()
@@ -84,7 +87,7 @@ class TestMicroversions(api_base.BaseApiTest):
         )
         self.assertEqual(response.headers[H_MIN_VER], MIN_VER)
         self.assertEqual(response.headers[H_MAX_VER], MAX_VER)
-        self.assertEqual(response.headers[H_RESP_VER], MAX_VER)
+        self.assertEqual(response.headers[H_RESP_VER], RESP_MAX)
         self.assertTrue(
             all(
                 x in response.json.keys()
@@ -109,3 +112,28 @@ class TestMicroversions(api_base.BaseApiTest):
         )
         self.assertTrue(response.json['error_message'])
         self.assertIn(expected_error_msg, response.json['error_message'])
+
+    def test_standard_header_with_service_type(self):
+        """The API-WG standard format includes the service type.
+
+        OpenStack-API-Version: accelerator 2.0
+        """
+        response = self.get_json(
+            '/v2',
+            headers={'OpenStack-API-Version': 'accelerator 2.0'},
+            return_json=False,
+        )
+        self.assertEqual(response.headers[H_MIN_VER], MIN_VER)
+        self.assertEqual(response.headers[H_MAX_VER], MAX_VER)
+        self.assertEqual(response.headers[H_RESP_VER], RESP_MIN)
+
+    def test_standard_header_with_service_type_latest(self):
+        """The API-WG standard format with 'latest'."""
+        response = self.get_json(
+            '/v2',
+            headers={'OpenStack-API-Version': 'accelerator latest'},
+            return_json=False,
+        )
+        self.assertEqual(response.headers[H_MIN_VER], MIN_VER)
+        self.assertEqual(response.headers[H_MAX_VER], MAX_VER)
+        self.assertEqual(response.headers[H_RESP_VER], RESP_MAX)

releasenotes/notes/fix-microversion-header-service-type-parsing-a1b2c3d4e5f6g7h8.yaml

@@ -0,0 +1,14 @@
+---
+fixes:
+  - |
+    Restored support for the standard ``OpenStack-API-Version`` header
+    format that includes the service type prefix
+    (``OpenStack-API-Version: accelerator 2.1``), as specified by the
+    `API-WG microversion guideline
+    <https://specs.openstack.org/openstack/api-wg/guidelines/microversion_specification.html>`_.
+    A previous change inadvertently removed parsing of the service type
+    prefix, causing clients that follow the standard (including
+    keystoneauth and openstacksdk) to always fall back to the minimum
+    API version.  Both the prefixed and bare version formats are now
+    accepted; a deprecation warning is logged when the service type
+    prefix is omitted.

requirements.txt

@@ -24,6 +24,7 @@ SQLAlchemy>=1.4.0 # MIT
 alembic>=0.8.10 # MIT
 stevedore>=1.5.0 # Apache-2.0
 keystonemiddleware>=4.17.0 # Apache-2.0
+microversion-parse>=1.0.0 # Apache-2.0
 jsonpatch>=1.16,!=1.20 # BSD
 psutil>=3.2.2 # BSD
 python-glanceclient>=2.3.0 # Apache-2.0