From 0d8310ec7a169f6de113080c0cb335247805ee29 Mon Sep 17 00:00:00 2001 From: Goutham Pacha Ravi Date: Wed, 27 Feb 2019 11:58:58 -0800 Subject: [PATCH] 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 | 3 +- manila/tests/api/openstack/test_wsgi.py | 86 +++++++++++++++++++ .../api/v2/test_share_group_snapshots.py | 7 +- manila/tests/api/v2/test_share_groups.py | 5 +- manila/tests/api/v2/test_share_instances.py | 5 +- manila/tests/api/v2/test_share_replicas.py | 6 +- .../api/v2/test_share_snapshot_instances.py | 6 +- manila/tests/api/v2/test_share_snapshots.py | 4 +- manila/tests/api/v2/test_shares.py | 4 +- ...of-proxy-urls-e33466af856708b4.yaml\t\t\t" | 7 ++ 10 files changed, 112 insertions(+), 21 deletions(-) create mode 100644 "releasenotes/notes/bug-1818081-fix-inferred-script-name-in-case-of-proxy-urls-e33466af856708b4.yaml\t\t\t" diff --git a/manila/api/openstack/wsgi.py b/manila/api/openstack/wsgi.py index cd7642dce2..27f9d6b6e7 100644 --- a/manila/api/openstack/wsgi.py +++ b/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: diff --git a/manila/tests/api/openstack/test_wsgi.py b/manila/tests/api/openstack/test_wsgi.py index 7a8c6529dd..c6b8f89550 100644 --- a/manila/tests/api/openstack/test_wsgi.py +++ b/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): diff --git a/manila/tests/api/v2/test_share_group_snapshots.py b/manila/tests/api/v2/test_share_group_snapshots.py index f9cb654f05..59e24a776b 100644 --- a/manila/tests/api/v2/test_share_group_snapshots.py +++ b/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' % - 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 diff --git a/manila/tests/api/v2/test_share_groups.py b/manila/tests/api/v2/test_share_groups.py index 1776b91095..a15ba8406b 100644 --- a/manila/tests/api/v2/test_share_groups.py +++ b/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( - '/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' diff --git a/manila/tests/api/v2/test_share_instances.py b/manila/tests/api/v2/test_share_instances.py index 2d33fda4e3..4f21a6cf8e 100644 --- a/manila/tests/api/v2/test_share_instances.py +++ b/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( - '/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"): diff --git a/manila/tests/api/v2/test_share_replicas.py b/manila/tests/api/v2/test_share_replicas.py index a88fa3b015..a5c69b4f2d 100644 --- a/manila/tests/api/v2/test_share_replicas.py +++ b/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( - '/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 diff --git a/manila/tests/api/v2/test_share_snapshot_instances.py b/manila/tests/api/v2/test_share_snapshot_instances.py index de30e7078a..89f92e1166 100644 --- a/manila/tests/api/v2/test_share_snapshot_instances.py +++ b/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( - '/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 diff --git a/manila/tests/api/v2/test_share_snapshots.py b/manila/tests/api/v2/test_share_snapshots.py index d5abddfa85..c73a3a9235 100644 --- a/manila/tests/api/v2/test_share_snapshots.py +++ b/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' % - 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, diff --git a/manila/tests/api/v2/test_shares.py b/manila/tests/api/v2/test_shares.py index 66a3e1069f..4d98b3971a 100644 --- a/manila/tests/api/v2/test_shares.py +++ b/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( - '/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, diff --git "a/releasenotes/notes/bug-1818081-fix-inferred-script-name-in-case-of-proxy-urls-e33466af856708b4.yaml\t\t\t" "b/releasenotes/notes/bug-1818081-fix-inferred-script-name-in-case-of-proxy-urls-e33466af856708b4.yaml\t\t\t" new file mode 100644 index 0000000000..57bcb3dd3e --- /dev/null +++ "b/releasenotes/notes/bug-1818081-fix-inferred-script-name-in-case-of-proxy-urls-e33466af856708b4.yaml\t\t\t" @@ -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 + `_ for more details. \ No newline at end of file