Fix versions for new microversion

Fix versions endpoint to match what is expected in the microversion spec.
https://specs.openstack.org/openstack/api-wg/guidelines/microversion_specification.html

We will return what we previously returned for < 1.1

Change-Id: I7db1fa9f1fccb3638911f0c90ad0718e1c53334d

barbican/api/controllers/versions.py

@@ -88,28 +88,36 @@ class BaseVersionController(object):
     """Base class for the version-specific controllers"""
 
     @classmethod
-    def get_version_info(cls, request):
-        return {
+    def get_version_info(cls, microversion_spec=True):
+        version = {
             'id': cls.version_id,
-            'status': 'stable',
-            'updated': cls.last_updated,
+            'status': 'CURRENT',
+            'min_version': cls.min_version,
+            'max_version': cls.version,
             'links': [
                 {
                     'rel': 'self',
                     'href': _get_versioned_url(cls.version_string),
-                }, {
+                },
+                {
                     'rel': 'describedby',
                     'type': 'text/html',
                     'href': 'https://docs.openstack.org/'
                 }
             ],
-            'media-types': [
+        }
+        if not microversion_spec:
+            version.pop('min_version')
+            version.pop('max_version')
+            version['status'] = 'stable'
+            version['updated']: cls.last_updated
+            version['media-types'] = [
                 {
                     'base': MIME_TYPE_JSON,
                     'type': MEDIA_TYPE_JSON % cls.version_string
                 }
             ]
-        }
+        return version
 
 
 class V1Controller(BaseVersionController):
@@ -144,7 +152,10 @@ class V1Controller(BaseVersionController):
     @controllers.handle_exceptions(u._('Version retrieval'))
     def on_get(self):
         pecan.core.override_template('json')
-        return {'version': self.get_version_info(pecan.request)}
+        if is_supported(pecan.request, max_version='1.0'):
+            return {'version': self.get_version_info(microversion_spec=False)}
+        else:
+            return {'version': self.get_version_info()}
 
 
 AVAILABLE_VERSIONS = {
@@ -172,19 +183,25 @@ class VersionsController(object):
         if 'build' in kwargs:
             return {'build': version.__version__}
 
-        versions_info = [version_class.get_version_info(pecan.request) for
-                         version_class in AVAILABLE_VERSIONS.values()]
-
-        version_output = {
-            'versions': {
-                'values': versions_info
+        if is_supported(pecan.request, max_version='1.0'):
+            resp = {
+                'versions': {
+                    'values': [
+                        V1Controller.get_version_info(microversion_spec=False)
+                    ]
+                }
+            }
+        else:
+            resp = {
+                'versions': [
+                    version_cls.get_version_info() for version_cls in
+                    AVAILABLE_VERSIONS.values()]
             }
-        }
 
         # Since we are returning all the versions available, the proper status
         # code is Multiple Choices (300)
         pecan.response.status = 300
-        return version_output
+        return resp
 
     def _redirect_to_default_json_home_if_needed(self, request):
         if self._mime_best_match(request.accept) == MIME_TYPE_JSON_HOME:

barbican/tests/api/controllers/test_versions.py

@@ -38,20 +38,33 @@ class WhenTestingVersionsResource(utils.BarbicanAPIBaseTestCase):
         resp = self.app.get('/', headers=headers)
         self.assertEqual(302, resp.status_int)
 
-    def test_should_return_version_json(self):
+    def test_should_return_version_json_v1_0(self):
+        # NOTE: this is for clients that do not have the microversion set
+        # which should default to the old-pre-microversion behavior
         resp = self.app.get('/')
 
         versions_response = resp.json['versions']['values']
         v1_info = versions_response[0]
 
-        # NOTE(jaosorior): I used assertIn instead of assertEqual because we
-        # might start using decimal numbers in the future. So when that happens
-        # this test will still be valid.
         self.assertIn('v1', v1_info['id'])
+        self.assertNotIn('min_version', v1_info)
+        self.assertNotIn('max_version', v1_info)
         self.assertEqual(1, len(v1_info['media-types']))
         self.assertEqual('application/json', v1_info['media-types'][0]['base'])
 
-    def test_when_host_href_is_not_set_in_conf(self):
+    def test_should_return_version_json_v1_1(self):
+        utils.set_version(self.app, '1.1')
+        resp = self.app.get('/')
+
+        versions_response = resp.json['versions']
+        v1_info = versions_response[0]
+
+        self.assertIn('v1', v1_info['id'])
+        self.assertEqual('1.0', v1_info['min_version'])
+        self.assertIn('max_version', v1_info)
+        self.assertNotIn('media-types', v1_info)
+
+    def test_when_host_href_is_not_set_in_conf_v0(self):
         cmn_utils.CONF.set_override('host_href', '')
         host_hdr = 'http://myproxy.server.com:9311'
         utils.mock_pecan_request(self, host=host_hdr)
@@ -61,10 +74,27 @@ class WhenTestingVersionsResource(utils.BarbicanAPIBaseTestCase):
         versions_response = resp.json['versions']['values']
 
         for v_info in versions_response:
+            self.assertNotIn('min_version', v_info)
+            self.assertNotIn('max_version', v_info)
             self.assertIn(host_hdr, v_info['links'][0]['href'])
             self.assertNotIn(dummy_root, v_info['links'][0]['href'])
 
-    def test_when_host_href_is_set_in_conf(self):
+    def test_when_host_href_is_not_set_in_conf_v1(self):
+        cmn_utils.CONF.set_override('host_href', '')
+        host_hdr = 'http://myproxy.server.com:9311'
+        utils.mock_pecan_request(self, host=host_hdr, version='1.1')
+        dummy_root = 'http://mylocalhost:9999'
+        resp = self.app.get(dummy_root)
+
+        versions_response = resp.json['versions']
+
+        for v_info in versions_response:
+            self.assertEqual('1.0', v_info['min_version'])
+            self.assertIn('max_version', v_info)
+            self.assertIn(host_hdr, v_info['links'][0]['href'])
+            self.assertNotIn(dummy_root, v_info['links'][0]['href'])
+
+    def test_when_host_href_is_set_in_conf_v0(self):
         host_href = 'http://myapp.server.com:9311/'
         cmn_utils.CONF.set_override('host_href', host_href)
         host_hdr = 'http://myproxy.server.com:9311'
@@ -75,11 +105,30 @@ class WhenTestingVersionsResource(utils.BarbicanAPIBaseTestCase):
         versions_response = resp.json['versions']['values']
 
         for v_info in versions_response:
+            self.assertNotIn('min_version', v_info)
+            self.assertNotIn('max_version', v_info)
             self.assertIn(host_href, v_info['links'][0]['href'])
             self.assertNotIn(dummy_root, v_info['links'][0]['href'])
             self.assertNotIn(host_hdr, v_info['links'][0]['href'])
 
-    def test_when_host_href_is_general(self):
+    def test_when_host_href_is_set_in_conf_v1(self):
+        host_href = 'http://myapp.server.com:9311/'
+        cmn_utils.CONF.set_override('host_href', host_href)
+        host_hdr = 'http://myproxy.server.com:9311'
+        utils.mock_pecan_request(self, host=host_hdr, version='1.1')
+        dummy_root = 'http://mylocalhost:9999'
+        resp = self.app.get(dummy_root)
+
+        versions_response = resp.json['versions']
+
+        for v_info in versions_response:
+            self.assertEqual('1.0', v_info['min_version'])
+            self.assertIn('max_version', v_info)
+            self.assertIn(host_href, v_info['links'][0]['href'])
+            self.assertNotIn(dummy_root, v_info['links'][0]['href'])
+            self.assertNotIn(host_hdr, v_info['links'][0]['href'])
+
+    def test_when_host_href_is_general_v0(self):
         host_href = 'http://myapp.server.com/key-manager'
         cmn_utils.CONF.set_override('host_href', host_href)
         host_hdr = 'http://myproxy.server.com:9311'
@@ -90,11 +139,30 @@ class WhenTestingVersionsResource(utils.BarbicanAPIBaseTestCase):
         versions_response = resp.json['versions']['values']
 
         for v_info in versions_response:
+            self.assertNotIn('min_version', v_info)
+            self.assertNotIn('max_version', v_info)
             self.assertIn(host_href, v_info['links'][0]['href'])
             self.assertNotIn(dummy_root, v_info['links'][0]['href'])
             self.assertNotIn(host_hdr, v_info['links'][0]['href'])
 
-    def test_when_host_href_is_not_set_with_general_request_url(self):
+    def test_when_host_href_is_general_v1(self):
+        host_href = 'http://myapp.server.com/key-manager'
+        cmn_utils.CONF.set_override('host_href', host_href)
+        host_hdr = 'http://myproxy.server.com:9311'
+        utils.mock_pecan_request(self, host=host_hdr, version='1.1')
+        dummy_root = 'http://mylocalhost:9999'
+        resp = self.app.get(dummy_root)
+
+        versions_response = resp.json['versions']
+
+        for v_info in versions_response:
+            self.assertEqual('1.0', v_info['min_version'])
+            self.assertIn('max_version', v_info)
+            self.assertIn(host_href, v_info['links'][0]['href'])
+            self.assertNotIn(dummy_root, v_info['links'][0]['href'])
+            self.assertNotIn(host_hdr, v_info['links'][0]['href'])
+
+    def test_when_host_href_is_not_set_with_general_request_url_v0(self):
         cmn_utils.CONF.set_override('host_href', '')
         host_hdr = 'http://myproxy.server.com/key-manager'
         utils.mock_pecan_request(self, host=host_hdr)
@@ -104,6 +172,23 @@ class WhenTestingVersionsResource(utils.BarbicanAPIBaseTestCase):
         versions_response = resp.json['versions']['values']
 
         for v_info in versions_response:
+            self.assertNotIn('min_version', v_info)
+            self.assertNotIn('max_version', v_info)
+            self.assertIn(host_hdr, v_info['links'][0]['href'])
+            self.assertNotIn(dummy_root, v_info['links'][0]['href'])
+
+    def test_when_host_href_is_not_set_with_general_request_url_v1(self):
+        cmn_utils.CONF.set_override('host_href', '')
+        host_hdr = 'http://myproxy.server.com/key-manager'
+        utils.mock_pecan_request(self, host=host_hdr, version='1.1')
+        dummy_root = 'http://mylocalhost:9999'
+        resp = self.app.get(dummy_root)
+
+        versions_response = resp.json['versions']
+
+        for v_info in versions_response:
+            self.assertEqual('1.0', v_info['min_version'])
+            self.assertIn('max_version', v_info)
             self.assertIn(host_hdr, v_info['links'][0]['href'])
             self.assertNotIn(dummy_root, v_info['links'][0]['href'])
 
@@ -125,7 +210,7 @@ class WhenTestingV1Resource(utils.BarbicanAPIBaseTestCase):
         resp = self.app.get('/', headers=headers)  # / refers to /v1 path
         self.assertEqual(200, resp.status_int)
 
-    def test_get_response_should_return_version_json(self):
+    def test_get_response_should_return_version_json_v1_0(self):
         resp = self.app.get('/')  # / refers to /v1 path
         self.assertEqual(200, resp.status_int)
 
@@ -135,5 +220,22 @@ class WhenTestingV1Resource(utils.BarbicanAPIBaseTestCase):
         # might start using decimal numbers in the future. So when that happens
         # this test will still be valid.
         self.assertIn('v1', v1_info['id'])
+        self.assertNotIn('max_version', v1_info)
+        self.assertNotIn('min_version', v1_info)
         self.assertEqual(1, len(v1_info['media-types']))
         self.assertEqual('application/json', v1_info['media-types'][0]['base'])
+
+    def test_get_response_should_return_version_json_v1_1(self):
+        utils.set_version(self.app, '1.1')
+        resp = self.app.get('/')  # / refers to /v1 path
+        self.assertEqual(200, resp.status_int)
+
+        v1_info = resp.json['version']
+
+        # NOTE(jaosorior): I used assertIn instead of assertEqual because we
+        # might start using decimal numbers in the future. So when that happens
+        # this test will still be valid.
+        self.assertIn('v1', v1_info['id'])
+        self.assertIn('max_version', v1_info)
+        self.assertIn('min_version', v1_info)
+        self.assertNotIn('media-types', v1_info)

barbican/tests/utils.py

@@ -41,12 +41,13 @@ from barbican.plugin import kmip_secret_store as kss
 from barbican.tests import database_utils
 
 
-def mock_pecan_request(test_instance, host=None):
+def mock_pecan_request(test_instance, host=None, version='1.0'):
     patcher_obj = mock.patch('pecan.request')
     mock_req = patcher_obj.start()
     test_instance.addCleanup(patcher_obj.stop)
     mock_req.url = host
     mock_req.environ = os.environ.copy()
+    mock_req.environ['key-manager.microversion'] = version
     mock_req.application_url = host
 
 

etc/barbican/barbican-api-paste.ini

@@ -6,7 +6,7 @@ use = egg:Paste#urlmap
 
 # Use this pipeline for Barbican API - versions no authentication
 [pipeline:barbican_version]
-pipeline = cors http_proxy_to_wsgi versionapp
+pipeline = cors http_proxy_to_wsgi microversion versionapp
 
 # Use this pipeline for Barbican API - DEFAULT no authentication
 [pipeline:barbican_api]