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
(cherry picked from commit 0d8310ec7a)
This commit is contained in:
Goutham Pacha Ravi 2019-02-27 11:58:58 -08:00 committed by Tom Barron
parent c9f581caeb
commit 6b39562c0e
10 changed files with 112 additions and 21 deletions

View File

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

View File

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

View File

@ -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' %
share_group_snapshot['id'], version=version)
path = ('/v2/fake/share-group-snapshots/%s/action' %
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

View File

@ -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(
'/v2/fake/share-groups/%s/action' % share_group['id'],
version=version)
path = '/v2/fake/share-groups/%s/action' % share_group['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'

View File

@ -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(
'/v2/fake/share_instances/%s/action' % instance['id'],
version=version)
path = '/v2/fake/share_instances/%s/action' % instance['id']
req = fakes.HTTPRequest.blank(path, script_name=path, version=version)
return instance, req
def _get_request(self, uri, context=None, version="2.3"):

View File

@ -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(
'/v2/fake/share-replicas/%s/action' % replica['id'],
version=self.api_version)
path = '/v2/fake/share-replicas/%s/action' % replica['id']
req = fakes.HTTPRequest.blank(path, script_name=path,
version=self.api_version)
req.method = 'POST'
req.headers['content-type'] = 'application/json'
req.headers['X-Openstack-Manila-Api-Version'] = self.api_version

View File

@ -89,9 +89,9 @@ class ShareSnapshotInstancesApiTest(test.TestCase):
status=constants.STATUS_AVAILABLE,
share_instance_id=share_instance['id'])
req = fakes.HTTPRequest.blank(
'/v2/fake/snapshot-instances/%s/action' % instance['id'],
version=self.api_version)
path = '/v2/fake/snapshot-instances/%s/action' % instance['id']
req = fakes.HTTPRequest.blank(path, 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

View File

@ -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' %
snapshot['id'], version=version)
path = '/v2/fake/snapshots/%s/action' % snapshot['id']
req = fakes.HTTPRequest.blank(path, script_name=path, version=version)
return snapshot, req
def _reset_status(self, ctxt, model, req, db_access_method,

View File

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

View File

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