Fix version selector when for proxy-style URLs

When manila API is served behind a proxy, the
"script_name" in the request can have the proxy
component in it. So, this patch fixes the version
selection logic by looking for the version in the
script name string instead of equivalence.

In addition, this patch adds some missing unit
tests and fixes tests that invoke a mocked
wsgi app for testing request context.

Change-Id: I0363d7174f3d7ddefa8ced59b182faed665e9c36
Partial-Bug: #1815038
Closes-Bug: #1818081

manila/api/openstack/wsgi.py

@@ -219,10 +219,9 @@ class Request(webob.Request):
         Microversions starts with /v2, so if a client sends a /v1 URL, then
         ignore the headers and request 1.0 APIs.
         """
-
         if not self.script_name:
             self.api_version_request = api_version.APIVersionRequest()
-        elif self.script_name == V1_SCRIPT_NAME:
+        elif V1_SCRIPT_NAME in self.script_name:
             self.api_version_request = api_version.APIVersionRequest('1.0')
         else:
             if API_VERSION_REQUEST_HEADER in self.headers:

manila/tests/api/openstack/test_wsgi.py

@@ -17,6 +17,7 @@ import webob
 
 import inspect
 
+from manila.api.openstack import api_version_request as api_version
 from manila.api.openstack import wsgi
 from manila import context
 from manila import exception
@@ -142,6 +143,91 @@ class RequestTest(test.TestCase):
             {'id%s' % i: resources[i] for i in res_range},
             getattr(r, get_db_all_func)())
 
+    def test_set_api_version_request_exception(self):
+        min_version = api_version.APIVersionRequest('2.0')
+        max_version = api_version.APIVersionRequest('2.45')
+        self.mock_object(api_version, 'max_api_version',
+                         mock.Mock(return_value=max_version))
+        self.mock_object(api_version, 'min_api_version',
+                         mock.Mock(return_value=min_version))
+        headers = {'X-OpenStack-Manila-API-Version': '2.50'}
+        request = wsgi.Request.blank(
+            'https://openstack.acme.com/v2/shares', method='GET',
+            headers=headers, script_name='/v2/shares')
+
+        self.assertRaises(exception.InvalidGlobalAPIVersion,
+                          request.set_api_version_request)
+        self.assertEqual(api_version.APIVersionRequest('2.50'),
+                         request.api_version_request)
+
+    @ddt.data('', '/share', '/v1', '/v2/shares', '/v1.1/', '/share/v1',
+              '/shared-file-sytems/v2', '/share/v3.5/share-replicas',
+              '/shared-file-sytems/v2/shares/xyzzy/action')
+    def test_set_api_version_request(self, resource):
+        min_version = api_version.APIVersionRequest('2.0')
+        max_version = api_version.APIVersionRequest('3.0')
+        self.mock_object(api_version, 'max_api_version',
+                         mock.Mock(return_value=max_version))
+        self.mock_object(api_version, 'min_api_version',
+                         mock.Mock(return_value=min_version))
+        request = wsgi.Request.blank(
+            'https://openstack.acme.com%s' % resource, method='GET',
+            headers={'X-OpenStack-Manila-API-Version': '2.117'},
+            script_name=resource)
+
+        self.assertIsNone(request.set_api_version_request())
+
+        if not resource:
+            self.assertEqual(api_version.APIVersionRequest(),
+                             request.api_version_request)
+        elif 'v1' in resource:
+            self.assertEqual(api_version.APIVersionRequest('1.0'),
+                             request.api_version_request)
+        else:
+            self.assertEqual(api_version.APIVersionRequest('2.117'),
+                             request.api_version_request)
+
+    def test_set_api_version_request_no_version_header(self):
+        min_version = api_version.APIVersionRequest('2.0')
+        max_version = api_version.APIVersionRequest('2.45')
+        self.mock_object(api_version, 'max_api_version',
+                         mock.Mock(return_value=max_version))
+        self.mock_object(api_version, 'min_api_version',
+                         mock.Mock(return_value=min_version))
+        headers = {}
+        request = wsgi.Request.blank(
+            'https://openstack.acme.com/v2/shares', method='GET',
+            headers=headers, script_name='/v2/shares')
+
+        self.assertIsNone(request.set_api_version_request())
+
+        self.assertEqual(api_version.APIVersionRequest('2.0'),
+                         request.api_version_request)
+
+    @ddt.data(None, 'true', 'false')
+    def test_set_api_version_request_experimental_header(self, experimental):
+        min_version = api_version.APIVersionRequest('2.0')
+        max_version = api_version.APIVersionRequest('2.45')
+        self.mock_object(api_version, 'max_api_version',
+                         mock.Mock(return_value=max_version))
+        self.mock_object(api_version, 'min_api_version',
+                         mock.Mock(return_value=min_version))
+        headers = {'X-OpenStack-Manila-API-Version': '2.38'}
+        if experimental:
+            headers['X-OpenStack-Manila-API-Experimental'] = experimental
+        request = wsgi.Request.blank(
+            'https://openstack.acme.com/v2/shares', method='GET',
+            headers=headers, script_name='/v2/shares')
+
+        self.assertIsNone(request.set_api_version_request())
+
+        self.assertEqual(request.api_version_request,
+                         api_version.APIVersionRequest('2.38'))
+
+        expected_experimental = experimental == 'true' or False
+        self.assertEqual(expected_experimental,
+                         request.api_version_request.experimental)
+
 
 class ActionDispatcherTest(test.TestCase):
     def test_dispatch(self):

manila/tests/api/v2/test_share_group_snapshots.py

@@ -548,9 +548,10 @@ class ShareGroupSnapshotAPITest(test.TestCase):
         if share_group_snapshot is None:
             share_group_snapshot = db_utils.create_share_group_snapshot(
                 'fake_id', status=constants.STATUS_AVAILABLE)
-        req = fakes.HTTPRequest.blank(
+
-            '/v2/fake/share-group-snapshots/%s/action' %
+        path = ('/v2/fake/share-group-snapshots/%s/action' %
-            share_group_snapshot['id'], version=version)
+                share_group_snapshot['id'])
+        req = fakes.HTTPRequest.blank(path, script_name=path, version=version)
         req.headers[wsgi.API_VERSION_REQUEST_HEADER] = version
         req.headers[wsgi.EXPERIMENTAL_API_REQUEST_HEADER] = 'True'
         return share_group_snapshot, req

manila/tests/api/v2/test_share_groups.py

@@ -74,9 +74,8 @@ class ShareGroupAPITest(test.TestCase):
         if share_group is None:
             share_group = db_utils.create_share_group(
                 status=constants.STATUS_AVAILABLE)
-        req = fakes.HTTPRequest.blank(
+        path = '/v2/fake/share-groups/%s/action' % share_group['id']
-            '/v2/fake/share-groups/%s/action' % share_group['id'],
+        req = fakes.HTTPRequest.blank(path, script_name=path, version=version)
-            version=version)
         req.headers[wsgi.API_VERSION_REQUEST_HEADER] = version
         req.headers[wsgi.EXPERIMENTAL_API_REQUEST_HEADER] = 'True'
 

manila/tests/api/v2/test_share_instances.py

@@ -51,9 +51,8 @@ class ShareInstancesAPITest(test.TestCase):
         if instance is None:
             instance = db_utils.create_share(status=constants.STATUS_AVAILABLE,
                                              size='1').instance
-        req = fakes.HTTPRequest.blank(
+        path = '/v2/fake/share_instances/%s/action' % instance['id']
-            '/v2/fake/share_instances/%s/action' % instance['id'],
+        req = fakes.HTTPRequest.blank(path, script_name=path, version=version)
-            version=version)
         return instance, req
 
     def _get_request(self, uri, context=None, version="2.3"):

manila/tests/api/v2/test_share_replicas.py

@@ -62,9 +62,9 @@ class ShareReplicasApiTest(test.TestCase):
         if 'replica_state' not in kwargs:
             kwargs['replica_state'] = constants.REPLICA_STATE_IN_SYNC
         replica = db_utils.create_share_replica(**kwargs)
-        req = fakes.HTTPRequest.blank(
+        path = '/v2/fake/share-replicas/%s/action' % replica['id']
-            '/v2/fake/share-replicas/%s/action' % replica['id'],
+        req = fakes.HTTPRequest.blank(path, script_name=path,
-            version=self.api_version)
+                                      version=self.api_version)
         req.method = 'POST'
         req.headers['content-type'] = 'application/json'
         req.headers['X-Openstack-Manila-Api-Version'] = self.api_version

manila/tests/api/v2/test_share_snapshot_instances.py

@@ -89,9 +89,9 @@ class ShareSnapshotInstancesApiTest(test.TestCase):
                 status=constants.STATUS_AVAILABLE,
                 share_instance_id=share_instance['id'])
 
-        req = fakes.HTTPRequest.blank(
+        path = '/v2/fake/snapshot-instances/%s/action' % instance['id']
-            '/v2/fake/snapshot-instances/%s/action' % instance['id'],
+        req = fakes.HTTPRequest.blank(path, version=self.api_version,
-            version=self.api_version)
+                                      script_name=path)
         req.method = 'POST'
         req.headers['content-type'] = 'application/json'
         req.headers['X-Openstack-Manila-Api-Version'] = self.api_version

manila/tests/api/v2/test_share_snapshots.py

@@ -603,8 +603,8 @@ class ShareSnapshotAdminActionsAPITest(test.TestCase):
             share = db_utils.create_share()
             snapshot = db_utils.create_snapshot(
                 status=constants.STATUS_AVAILABLE, share_id=share['id'])
-        req = fakes.HTTPRequest.blank('/v2/fake/snapshots/%s/action' %
+        path = '/v2/fake/snapshots/%s/action' % snapshot['id']
-                                      snapshot['id'], version=version)
+        req = fakes.HTTPRequest.blank(path, script_name=path, version=version)
         return snapshot, req
 
     def _reset_status(self, ctxt, model, req, db_access_method,

manila/tests/api/v2/test_shares.py

@@ -2390,8 +2390,8 @@ class ShareAdminActionsAPITest(test.TestCase):
             share = db_utils.create_share(status=constants.STATUS_AVAILABLE,
                                           size='1',
                                           override_defaults=True)
-        req = fakes.HTTPRequest.blank(
+        path = '/v2/fake/shares/%s/action' % share['id']
-            '/v2/fake/shares/%s/action' % share['id'], version=version)
+        req = fakes.HTTPRequest.blank(path, script_name=path, version=version)
         return share, req
 
     def _reset_status(self, ctxt, model, req, db_access_method,

releasenotes/notes/bug-1818081-fix-inferred-script-name-in-case-of-proxy-urls-e33466af856708b4.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    When manila API is run behind a proxy webserver, the API service was
+    parsing the major API version requested incorrectly, leading to incorrect
+    responses. This behavior has now been fixed. See `launchpad bug 1815038
+    <https://bugs.launchpad.net/manila/+bug/1818081>`_ for more details.