From 1e907602e37fb55bbe5a20164db6d074f87369af Mon Sep 17 00:00:00 2001 From: Eric Fried Date: Thu, 26 Sep 2019 16:52:12 -0500 Subject: [PATCH] Allow versioned discovery unauthenticated Make routes to the versioned discovery documents (/v2, /v2.1) go through paste pipelines that don't require authentication, while leaving their sub-URLs (/v2.1/servers etc) requiring authentication. To make this work, our URLMap matcher gets support for a very rudimentary wildcard syntax, whereby api-paste.ini can differentiate between {/v2.1, /v2.1/} and /v2.1/$anything_else. The former points to the unauthenticated discovery app pipeline; the latter points to the existing "real API" pipeline. Similar for legacy v2. This entails a slight behavior change: requests to /v2 and /v2.1 used to 302 redirect to /v2/ and /v2.1/, respectively. Now they just work. Change-Id: Id47515017982850b167d5c637d93b96ae00ba793 Closes-Bug: #1845530 Closes-Bug: #1728732 --- etc/nova/api-paste.ini | 15 ++++++-- nova/api/openstack/compute/versions.py | 15 ++++++++ nova/api/openstack/urlmap.py | 19 ++++++++++ .../api_sample_tests/test_versions.py | 7 +--- .../test_legacy_v2_compatible_wrapper.py | 4 +-- .../unit/api/openstack/compute/test_urlmap.py | 22 ++++++++++++ .../api/openstack/compute/test_versions.py | 18 +--------- nova/tests/unit/api/openstack/fakes.py | 35 ++++++++++++------- ...ed-version-discovery-cc38986617dc1c02.yaml | 22 ++++++++++++ 9 files changed, 116 insertions(+), 41 deletions(-) create mode 100644 releasenotes/notes/unauthed-version-discovery-cc38986617dc1c02.yaml diff --git a/etc/nova/api-paste.ini b/etc/nova/api-paste.ini index 2aaa7bf808c0..7e20eaa7e20d 100644 --- a/etc/nova/api-paste.ini +++ b/etc/nova/api-paste.ini @@ -18,13 +18,15 @@ paste.app_factory = nova.api.metadata.handler:MetadataRequestHandler.factory [composite:osapi_compute] use = call:nova.api.openstack.urlmap:urlmap_factory /: oscomputeversions +/v2: oscomputeversion_legacy_v2 +/v2.1: oscomputeversion_v2 # v21 is an exactly feature match for v2, except it has more stringent # input validation on the wsgi surface (prevents fuzzing early on the # API). It also provides new features via API microversions which are # opt into for clients. Unaware clients will receive the same frozen # v2 API feature set, but with some relaxed validation -/v2: openstack_compute_api_v21_legacy_v2_compatible -/v2.1: openstack_compute_api_v21 +/v2/+: openstack_compute_api_v21_legacy_v2_compatible +/v2.1/+: openstack_compute_api_v21 [composite:openstack_compute_api_v21] use = call:nova.api.auth:pipeline_factory_v21 @@ -72,9 +74,18 @@ paste.app_factory = nova.api.openstack.compute:APIRouterV21.factory [pipeline:oscomputeversions] pipeline = cors faultwrap request_log http_proxy_to_wsgi oscomputeversionapp +[pipeline:oscomputeversion_v2] +pipeline = cors compute_req_id faultwrap request_log http_proxy_to_wsgi oscomputeversionapp_v2 + +[pipeline:oscomputeversion_legacy_v2] +pipeline = cors compute_req_id faultwrap request_log http_proxy_to_wsgi legacy_v2_compatible oscomputeversionapp_v2 + [app:oscomputeversionapp] paste.app_factory = nova.api.openstack.compute.versions:Versions.factory +[app:oscomputeversionapp_v2] +paste.app_factory = nova.api.openstack.compute.versions:VersionsV2.factory + ########## # Shared # ########## diff --git a/nova/api/openstack/compute/versions.py b/nova/api/openstack/compute/versions.py index 480b2af1f0b9..be45eb01017f 100644 --- a/nova/api/openstack/compute/versions.py +++ b/nova/api/openstack/compute/versions.py @@ -100,3 +100,18 @@ class Versions(wsgi.Resource): args['action'] = 'multi' return args + + +class VersionsV2(wsgi.Resource): + + def __init__(self): + super(VersionsV2, self).__init__(None) + + def index(self, req, body=None): + builder = views_versions.get_view_builder(req) + ver = 'v2.0' if req.is_legacy_v2() else 'v2.1' + return builder.build_version(VERSIONS[ver]) + + def get_action_args(self, request_environment): + """Parse dictionary created by routes library.""" + return {'action': 'index'} diff --git a/nova/api/openstack/urlmap.py b/nova/api/openstack/urlmap.py index 4d353aabc5cd..851ae8a3a6a6 100644 --- a/nova/api/openstack/urlmap.py +++ b/nova/api/openstack/urlmap.py @@ -168,6 +168,25 @@ class URLMap(paste.urlmap.URLMap): for (domain, app_url), app in self.applications: if domain and domain != host and domain != host + ':' + port: continue + # Rudimentary "wildcard" support: + # By declaring a urlmap path ending in '/+', you're saying the + # incoming path must start with everything up to and including the + # '/' *and* have something after that as well. For example, path + # /foo/bar/+ will match /foo/bar/baz, but not /foo/bar/ or /foo/bar + # NOTE(efried): This assumes we'll never need a path URI component + # that legitimately starts with '+'. (We could use a + # more obscure character/sequence here in that case.) + if app_url.endswith('/+'): + # Must be requesting at least the path element (including /) + if not path_info.startswith(app_url[:-1]): + continue + # ...but also must be requesting something after that / + if len(path_info) < len(app_url): + continue + # Trim the /+ off the app_url to make it look "normal" for e.g. + # proper splitting of SCRIPT_NAME and PATH_INFO. + return app, app_url[:-2] + # Normal (non-wildcarded) prefix match if (path_info == app_url or path_info.startswith(app_url + '/')): return app, app_url diff --git a/nova/tests/functional/api_sample_tests/test_versions.py b/nova/tests/functional/api_sample_tests/test_versions.py index 373b87c002ea..e356188e0828 100644 --- a/nova/tests/functional/api_sample_tests/test_versions.py +++ b/nova/tests/functional/api_sample_tests/test_versions.py @@ -67,9 +67,4 @@ class VersionsSampleJsonTest(api_sample_base.ApiSampleTestBaseV21): @ddt.unpack def test_versions_get_versioned(self, url, tplname, subs): response = self._get(url) - # TODO(efried): This is bug 1845530 whereby we try to authenticate at - # the versioned discovery endpoint. - self.assertEqual(401, response.status_code) - # TODO(efried): Uncomment when bug 1845530 is resolved - # self._verify_response(tplname, subs, response, 200, - # update_links=False) + self._verify_response(tplname, subs, response, 200, update_links=False) diff --git a/nova/tests/functional/test_legacy_v2_compatible_wrapper.py b/nova/tests/functional/test_legacy_v2_compatible_wrapper.py index 91dbe61125fe..19efaa3a212b 100644 --- a/nova/tests/functional/test_legacy_v2_compatible_wrapper.py +++ b/nova/tests/functional/test_legacy_v2_compatible_wrapper.py @@ -14,7 +14,6 @@ # under the License. from nova.api import openstack -from nova.api.openstack import compute from nova.api.openstack import wsgi from nova.tests.functional.api import client from nova.tests.functional import test_servers @@ -25,8 +24,7 @@ class LegacyV2CompatibleTestBase(test_servers.ServersTestBase): def setUp(self): super(LegacyV2CompatibleTestBase, self).setUp() - self._check_api_endpoint('/v2', [compute.APIRouterV21, - openstack.LegacyV2CompatibleWrapper]) + self._check_api_endpoint('/v2', [openstack.LegacyV2CompatibleWrapper]) def test_request_with_microversion_headers(self): self.api.microversion = '2.100' diff --git a/nova/tests/unit/api/openstack/compute/test_urlmap.py b/nova/tests/unit/api/openstack/compute/test_urlmap.py index d52bf87ede6e..44e2d1907940 100644 --- a/nova/tests/unit/api/openstack/compute/test_urlmap.py +++ b/nova/tests/unit/api/openstack/compute/test_urlmap.py @@ -114,3 +114,25 @@ class UrlmapTest(test.NoDBTestCase): self.assertEqual("application/json", res.content_type) body = jsonutils.loads(res.body) self.assertEqual('v2.1', body['version']['id']) + + def test_script_name_path_info(self): + """Ensure URLMap preserves SCRIPT_NAME and PATH_INFO correctly.""" + data = ( + ('', '', ''), + ('/', '', '/'), + ('/v2', '/v2', ''), + ('/v2/', '/v2', '/'), + ('/v2.1', '/v2.1', ''), + ('/v2.1/', '/v2.1', '/'), + ('/v2/foo', '/v2', '/foo'), + ('/v2.1/foo', '/v2.1', '/foo'), + ('/bar/baz', '', '/bar/baz') + ) + app = fakes.wsgi_app_v21() + for url, exp_script_name, exp_path_info in data: + req = fakes.HTTPRequest.blank(url) + req.get_response(app) + # The app uses /v2 as the base URL :( + exp_script_name = '/v2' + exp_script_name + self.assertEqual(exp_script_name, req.environ['SCRIPT_NAME']) + self.assertEqual(exp_path_info, req.environ['PATH_INFO']) diff --git a/nova/tests/unit/api/openstack/compute/test_versions.py b/nova/tests/unit/api/openstack/compute/test_versions.py index 4f6a7d8aff10..7e921526d447 100644 --- a/nova/tests/unit/api/openstack/compute/test_versions.py +++ b/nova/tests/unit/api/openstack/compute/test_versions.py @@ -141,14 +141,6 @@ class VersionsTestV21WithV2CompatibleWrapper(test.NoDBTestCase): ] self.assertEqual(expected, versions) - def test_get_version_list_302(self): - req = fakes.HTTPRequest.blank('/v2') - req.accept = "application/json" - res = req.get_response(self.wsgi_app) - self.assertEqual(302, res.status_int) - redirect_req = fakes.HTTPRequest.blank('/v2/') - self.assertEqual(redirect_req.url, res.location) - def _test_get_version_2_detail(self, url, accept=None): if accept is None: accept = "application/json" @@ -252,7 +244,7 @@ class VersionsTestV21WithV2CompatibleWrapper(test.NoDBTestCase): """Make sure multi choice responses do not have content-type application/atom+xml (should use default of json) """ - req = fakes.HTTPRequest.blank('/servers') + req = fakes.HTTPRequest.blank('/servers', base_url='') req.accept = "application/atom+xml" res = req.get_response(self.wsgi_app) self.assertEqual(300, res.status_int) @@ -448,14 +440,6 @@ class VersionsTestV21(test.NoDBTestCase): def wsgi_app(self): return fakes.wsgi_app_v21() - def test_get_version_list_302(self): - req = fakes.HTTPRequest.blank('/v2.1') - req.accept = "application/json" - res = req.get_response(self.wsgi_app) - self.assertEqual(302, res.status_int) - redirect_req = fakes.HTTPRequest.blank('/v2.1/') - self.assertEqual(redirect_req.url, res.location) - def test_get_version_21_detail(self): req = fakes.HTTPRequest.blank('/v2.1/', base_url='') req.accept = "application/json" diff --git a/nova/tests/unit/api/openstack/fakes.py b/nova/tests/unit/api/openstack/fakes.py index 6c8e2a17f412..5d798cfbe003 100644 --- a/nova/tests/unit/api/openstack/fakes.py +++ b/nova/tests/unit/api/openstack/fakes.py @@ -62,23 +62,32 @@ def fake_wsgi(self, req): def wsgi_app_v21(fake_auth_context=None, v2_compatible=False, custom_routes=None): + # NOTE(efried): Keep this (roughly) in sync with api-paste.ini + + def wrap(app, use_context=False): + if v2_compatible: + app = openstack_api.LegacyV2CompatibleWrapper(app) + + if use_context: + if fake_auth_context is not None: + ctxt = fake_auth_context + else: + ctxt = context.RequestContext( + 'fake', FAKE_PROJECT_ID, auth_token=True) + app = api_auth.InjectContext(ctxt, app) + + app = openstack_api.FaultWrapper(app) + + return app inner_app_v21 = compute.APIRouterV21(custom_routes=custom_routes) - if v2_compatible: - inner_app_v21 = openstack_api.LegacyV2CompatibleWrapper(inner_app_v21) - - if fake_auth_context is not None: - ctxt = fake_auth_context - else: - ctxt = context.RequestContext( - 'fake', FAKE_PROJECT_ID, auth_token=True) - api_v21 = openstack_api.FaultWrapper( - api_auth.InjectContext(ctxt, inner_app_v21)) mapper = urlmap.URLMap() - mapper['/v2'] = api_v21 - mapper['/v2.1'] = api_v21 - mapper['/'] = openstack_api.FaultWrapper(versions.Versions()) + mapper['/'] = wrap(versions.Versions()) + mapper['/v2'] = wrap(versions.VersionsV2()) + mapper['/v2.1'] = wrap(versions.VersionsV2()) + mapper['/v2/+'] = wrap(inner_app_v21, use_context=True) + mapper['/v2.1/+'] = wrap(inner_app_v21, use_context=True) return mapper diff --git a/releasenotes/notes/unauthed-version-discovery-cc38986617dc1c02.yaml b/releasenotes/notes/unauthed-version-discovery-cc38986617dc1c02.yaml new file mode 100644 index 000000000000..37cf254870a5 --- /dev/null +++ b/releasenotes/notes/unauthed-version-discovery-cc38986617dc1c02.yaml @@ -0,0 +1,22 @@ +--- +upgrade: + - | + New paste pipelines and middleware have been created to allow API version + discovery to be performed without authentication or redirects. Because this + involves an ``api-paste.ini`` change, you will need to manually update your + ``api-paste.ini`` with the one from the release to get this functionality. +fixes: + - | + When using the ``api-paste.ini`` from the release, version discovery + requests without a trailing slash will no longer receive a 302 redirect to + the corresponding URL with a trailing slash (e.g. a request for ``/v2.1`` + will no longer redirect to ``/v2.1/``). Instead, such requests will respond + with the version discovery document regardless of the presence of the + trailing slash. See + `bug 1728732 `_ for details. + - | + When using the ``api-paste.ini`` from the release, requests to the + versioned discovery endpoints (``/v2.1`` and ``/v2``) no longer require + authentication. When using the compute API through certain clients, such as + openstacksdk, this eliminates an unnecessary additional query. See + `bug 1845530 `_ for details.