From 1f16a763e793098b26f37110db35828024e44025 Mon Sep 17 00:00:00 2001 From: Sean Dague Date: Fri, 15 Jan 2016 07:46:41 -0500 Subject: [PATCH] Make project_id optional in v2.1 urls This introduces microversion 2.18 which signals that the {project_id} is no longer required in URLs. It tests this with an additional scenario in api_samples which makes all the requests without the project_id in the url (using a different noauth middleware to accomplish this). Update the link fixer in the ApiSamples matching code to also update for optional project_id. This is the least worse approach here, because if we set request_api_version, then we have to duplicate the entire template tree as well, which we definitely don't want to do, as it now correctly handles either url form. This updates the auth tests to bifurcate with testscenarios instead of the subclass model, which makes for more consistent tests. In order to support adding routes without project_id we have to be able to restrict project_id something that doesn't match any of our top level routes. The default for this is [0-9a-f\-]+ which will match all of the following: - keystone default generated project_ids [0-9a-f]{32} - integer project_ids (\d+) - known in use by RAX - uuids with dashes (no known users, but suspect there might be) This can be overrided with the new (but already deprecated) ``project_id_regex`` config option. NOTE: we used this feature to expand the regex to match 'fake' and 'openstack' as valid project ids in tests. Those concepts are deeply embedded in our tests, and need to be unwound independently. APIImpact Implements bp:service-catalog-tng Co-Authored-By: Augustina Ragwitz Change-Id: Id92251243d9e92f30e466419110fce5781304823 --- .../versions/v21-version-get-resp.json | 2 +- .../versions/versions-get-resp.json | 2 +- nova/api/openstack/__init__.py | 40 +++++++++++++++++-- nova/api/openstack/api_version_request.py | 3 +- nova/api/openstack/auth.py | 12 ++++-- nova/api/openstack/compute/image_metadata.py | 5 +++ nova/api/openstack/compute/server_metadata.py | 5 +++ .../openstack/rest_api_version_history.rst | 4 ++ nova/tests/functional/api_paste_fixture.py | 11 +++++ .../api_sample_tests/api_sample_base.py | 9 ++++- .../versions/v21-version-get-resp.json.tpl | 2 +- .../versions/versions-get-resp.json.tpl | 2 +- .../tests/functional/api_samples_test_base.py | 24 +++++++++-- .../unit/api/openstack/compute/test_auth.py | 39 +++++++++--------- .../api/openstack/compute/test_versions.py | 8 ++-- nova/tests/unit/conf_fixture.py | 5 +++ .../optional_project_id-6aebf1cb394d498f.yaml | 16 ++++++++ 17 files changed, 149 insertions(+), 40 deletions(-) create mode 100644 releasenotes/notes/optional_project_id-6aebf1cb394d498f.yaml diff --git a/doc/api_samples/versions/v21-version-get-resp.json b/doc/api_samples/versions/v21-version-get-resp.json index d5c0c6658713..6688800f12ed 100644 --- a/doc/api_samples/versions/v21-version-get-resp.json +++ b/doc/api_samples/versions/v21-version-get-resp.json @@ -19,7 +19,7 @@ } ], "status": "CURRENT", - "version": "2.17", + "version": "2.18", "min_version": "2.1", "updated": "2013-07-23T11:33:21Z" } diff --git a/doc/api_samples/versions/versions-get-resp.json b/doc/api_samples/versions/versions-get-resp.json index 068d70fb2ebf..655cb9666961 100644 --- a/doc/api_samples/versions/versions-get-resp.json +++ b/doc/api_samples/versions/versions-get-resp.json @@ -22,7 +22,7 @@ } ], "status": "CURRENT", - "version": "2.17", + "version": "2.18", "min_version": "2.1", "updated": "2013-07-23T11:33:21Z" } diff --git a/nova/api/openstack/__init__.py b/nova/api/openstack/__init__.py index ab1f864ae687..690396e427bc 100644 --- a/nova/api/openstack/__init__.py +++ b/nova/api/openstack/__init__.py @@ -60,7 +60,13 @@ api_opts = [ 'list. Specify the extension aliases here. ' 'This option will be removed in the near future. ' 'After that point you have to run all of the API.', - deprecated_for_removal=True, deprecated_group='osapi_v21') + deprecated_for_removal=True, deprecated_group='osapi_v21'), + cfg.StrOpt('project_id_regex', + default=None, + help='DEPRECATED: The validation regex for project_ids ' + 'used in urls. This defaults to [0-9a-f\-]+ if not set, ' + 'which matches normal uuids created by keystone.', + deprecated_for_removal=True, deprecated_group='osapi_v21') ] api_opts_group = cfg.OptGroup(name='osapi_v21', title='API v2.1 Options') @@ -196,14 +202,40 @@ class APIMapper(routes.Mapper): class ProjectMapper(APIMapper): def resource(self, member_name, collection_name, **kwargs): + # NOTE(sdague): project_id parameter is only valid if its hex + # or hex + dashes (note, integers are a subset of this). This + # is required to hand our overlaping routes issues. + project_id_regex = '[0-9a-f\-]+' + if CONF.osapi_v21.project_id_regex: + project_id_regex = CONF.osapi_v21.project_id_regex + + project_id_token = '{project_id:%s}' % project_id_regex if 'parent_resource' not in kwargs: - kwargs['path_prefix'] = '{project_id}/' + kwargs['path_prefix'] = '%s/' % project_id_token else: parent_resource = kwargs['parent_resource'] p_collection = parent_resource['collection_name'] p_member = parent_resource['member_name'] - kwargs['path_prefix'] = '{project_id}/%s/:%s_id' % (p_collection, - p_member) + kwargs['path_prefix'] = '%s/%s/:%s_id' % ( + project_id_token, + p_collection, + p_member) + routes.Mapper.resource( + self, + member_name, + collection_name, + **kwargs) + + # while we are in transition mode, create additional routes + # for the resource that do not include project_id. + if 'parent_resource' not in kwargs: + del kwargs['path_prefix'] + else: + parent_resource = kwargs['parent_resource'] + p_collection = parent_resource['collection_name'] + p_member = parent_resource['member_name'] + kwargs['path_prefix'] = '%s/:%s_id' % (p_collection, + p_member) routes.Mapper.resource(self, member_name, collection_name, **kwargs) diff --git a/nova/api/openstack/api_version_request.py b/nova/api/openstack/api_version_request.py index da9af09ba510..97188c12edee 100644 --- a/nova/api/openstack/api_version_request.py +++ b/nova/api/openstack/api_version_request.py @@ -59,6 +59,7 @@ REST_API_VERSION_HISTORY = """REST API Version History: * 2.15 - Add soft-affinity and soft-anti-affinity policies * 2.16 - Exposes host_status for servers/detail and servers/{server_id} * 2.17 - Add trigger_crash_dump to server actions + * 2.18 - Makes project_id optional in v2.1 """ # The minimum and maximum versions of the API supported @@ -67,7 +68,7 @@ REST_API_VERSION_HISTORY = """REST API Version History: # Note(cyeoh): This only applies for the v2.1 API once microversions # support is fully merged. It does not affect the V2 API. _MIN_API_VERSION = "2.1" -_MAX_API_VERSION = "2.17" +_MAX_API_VERSION = "2.18" DEFAULT_API_VERSION = _MIN_API_VERSION diff --git a/nova/api/openstack/auth.py b/nova/api/openstack/auth.py index 61b9773d1b46..acd0338cd7d3 100644 --- a/nova/api/openstack/auth.py +++ b/nova/api/openstack/auth.py @@ -74,10 +74,14 @@ class NoAuthMiddleware(NoAuthMiddlewareBase): return self.base_call(req, True, always_admin=False) -# TODO(johnthetubaguy) this should be removed in the M release -class NoAuthMiddlewareV3(NoAuthMiddlewareBase): - """Return a fake token if one isn't specified.""" +class NoAuthMiddlewareV2_17(NoAuthMiddlewareBase): + """Return a fake token if one isn't specified. + + This provides a version of the middleware which does not add + project_id into server management urls. + + """ @webob.dec.wsgify(RequestClass=wsgi.Request) def __call__(self, req): - return self.base_call(req, False) + return self.base_call(req, False, always_admin=False) diff --git a/nova/api/openstack/compute/image_metadata.py b/nova/api/openstack/compute/image_metadata.py index 0d2f450fc4ae..f9b0f43bb020 100644 --- a/nova/api/openstack/compute/image_metadata.py +++ b/nova/api/openstack/compute/image_metadata.py @@ -154,3 +154,8 @@ class ImageMetadata(extensions.V21APIExtensionBase): "/{project_id}/images/{image_id}/metadata", controller=wsgi_resource, action='update_all', conditions={"method": ['PUT']}) + # Also connect the non project_id route + mapper.connect("metadata", + "/images/{image_id}/metadata", + controller=wsgi_resource, + action='update_all', conditions={"method": ['PUT']}) diff --git a/nova/api/openstack/compute/server_metadata.py b/nova/api/openstack/compute/server_metadata.py index cbec6d5dd3c9..f7be44f97c06 100644 --- a/nova/api/openstack/compute/server_metadata.py +++ b/nova/api/openstack/compute/server_metadata.py @@ -191,3 +191,8 @@ class ServerMetadata(extensions.V21APIExtensionBase): "/{project_id}/servers/{server_id}/metadata", controller=wsgi_resource, action='update_all', conditions={"method": ['PUT']}) + # Also connect the non project_id routes + mapper.connect("metadata", + "/servers/{server_id}/metadata", + controller=wsgi_resource, + action='update_all', conditions={"method": ['PUT']}) diff --git a/nova/api/openstack/rest_api_version_history.rst b/nova/api/openstack/rest_api_version_history.rst index 92732e197649..c66e90b3f439 100644 --- a/nova/api/openstack/rest_api_version_history.rst +++ b/nova/api/openstack/rest_api_version_history.rst @@ -163,3 +163,7 @@ user documentation. Add a new API for triggering crash dump in an instance. Different operation systems in instance may need different configurations to trigger crash dump. + +2.18 +---- + Establishes a set of routes that makes project_id an optional construct in v2.1. diff --git a/nova/tests/functional/api_paste_fixture.py b/nova/tests/functional/api_paste_fixture.py index 709bbc067f3d..5dfa4848a2e7 100644 --- a/nova/tests/functional/api_paste_fixture.py +++ b/nova/tests/functional/api_paste_fixture.py @@ -54,3 +54,14 @@ class ApiPasteLegacyV2Fixture(ApiPasteV21Fixture): "/v2: openstack_compute_api_v21_legacy_v2_compatible", "/v2: openstack_compute_api_legacy_v2") target_file.write(line) + + +class ApiPasteNoProjectId(ApiPasteV21Fixture): + + def _replace_line(self, target_file, line): + line = line.replace( + "paste.filter_factory = nova.api.openstack.auth:" + "NoAuthMiddleware.factory", + "paste.filter_factory = nova.api.openstack.auth:" + "NoAuthMiddlewareV2_17.factory") + target_file.write(line) diff --git a/nova/tests/functional/api_sample_tests/api_sample_base.py b/nova/tests/functional/api_sample_tests/api_sample_base.py index 6c97e19a9306..92c6e2c597d6 100644 --- a/nova/tests/functional/api_sample_tests/api_sample_base.py +++ b/nova/tests/functional/api_sample_tests/api_sample_base.py @@ -69,6 +69,7 @@ class ApiSampleTestBaseV21(testscenarios.WithScenarios, sample_dir = None extra_extensions_to_load = None _legacy_v2_code = False + _project_id = True scenarios = [ # test v2 with the v2.1 compatibility stack @@ -82,7 +83,13 @@ class ApiSampleTestBaseV21(testscenarios.WithScenarios, 'api_major_version': 'v2', '_legacy_v2_code': True, '_additional_fixtures': [ - api_paste_fixture.ApiPasteLegacyV2Fixture]}) + api_paste_fixture.ApiPasteLegacyV2Fixture]}), + # test v2.16 code without project id + ('v2_1_noproject_id', { + 'api_major_version': 'v2.1', + '_project_id': False, + '_additional_fixtures': [ + api_paste_fixture.ApiPasteNoProjectId]}) ] def setUp(self): diff --git a/nova/tests/functional/api_sample_tests/api_samples/versions/v21-version-get-resp.json.tpl b/nova/tests/functional/api_sample_tests/api_samples/versions/v21-version-get-resp.json.tpl index 25978ae2fc72..fcd4fa32fbaa 100644 --- a/nova/tests/functional/api_sample_tests/api_samples/versions/v21-version-get-resp.json.tpl +++ b/nova/tests/functional/api_sample_tests/api_samples/versions/v21-version-get-resp.json.tpl @@ -19,7 +19,7 @@ } ], "status": "CURRENT", - "version": "2.17", + "version": "2.18", "min_version": "2.1", "updated": "2013-07-23T11:33:21Z" } diff --git a/nova/tests/functional/api_sample_tests/api_samples/versions/versions-get-resp.json.tpl b/nova/tests/functional/api_sample_tests/api_samples/versions/versions-get-resp.json.tpl index d472ed1389bd..b1667a76fb80 100644 --- a/nova/tests/functional/api_sample_tests/api_samples/versions/versions-get-resp.json.tpl +++ b/nova/tests/functional/api_sample_tests/api_samples/versions/versions-get-resp.json.tpl @@ -22,7 +22,7 @@ } ], "status": "CURRENT", - "version": "2.17", + "version": "2.18", "min_version": "2.1", "updated": "2013-07-23T11:33:21Z" } diff --git a/nova/tests/functional/api_samples_test_base.py b/nova/tests/functional/api_samples_test_base.py index 68eb36551eed..b7ef4d90c741 100644 --- a/nova/tests/functional/api_samples_test_base.py +++ b/nova/tests/functional/api_samples_test_base.py @@ -250,9 +250,19 @@ class ApiSampleTestBase(integrated_helpers._IntegratedTestBase): def _update_links(self, sample_data): """Process sample data and update version specific links.""" - url_re = self._get_host() + "/v(2\.1|2)" + # replace version urls + url_re = self._get_host() + "/v(2|2\.1)/openstack" new_url = self._get_host() + "/" + self.api_major_version + if self._project_id: + new_url += "/openstack" updated_data = re.sub(url_re, new_url, sample_data) + + # replace unversioned urls + url_re = self._get_host() + "/openstack" + new_url = self._get_host() + if self._project_id: + new_url += "/openstack" + updated_data = re.sub(url_re, new_url, updated_data) return updated_data def _verify_response(self, name, subs, response, exp_code, @@ -360,13 +370,19 @@ class ApiSampleTestBase(integrated_helpers._IntegratedTestBase): def _get_compute_endpoint(self): # NOTE(sdague): "openstack" is stand in for project_id, it # should be more generic in future. - return '%s/%s' % (self._get_host(), 'openstack') + if self._project_id: + return '%s/%s' % (self._get_host(), 'openstack') + else: + return self._get_host() def _get_vers_compute_endpoint(self): # NOTE(sdague): "openstack" is stand in for project_id, it # should be more generic in future. - return '%s/%s/%s' % (self._get_host(), self.api_major_version, - 'openstack') + if self._project_id: + return '%s/%s/%s' % (self._get_host(), self.api_major_version, + 'openstack') + else: + return '%s/%s' % (self._get_host(), self.api_major_version) def _get_response(self, url, method, body=None, strip_version=False, api_version=None, headers=None): diff --git a/nova/tests/unit/api/openstack/compute/test_auth.py b/nova/tests/unit/api/openstack/compute/test_auth.py index 83e495918c20..928af29450e7 100644 --- a/nova/tests/unit/api/openstack/compute/test_auth.py +++ b/nova/tests/unit/api/openstack/compute/test_auth.py @@ -17,6 +17,8 @@ import webob import webob.dec +import testscenarios + from nova.api import openstack as openstack_api from nova.api.openstack import auth from nova.api.openstack import compute @@ -25,15 +27,29 @@ from nova import test from nova.tests.unit.api.openstack import fakes -class TestNoAuthMiddlewareV21(test.NoDBTestCase): +class TestNoAuthMiddleware(testscenarios.WithScenarios, test.NoDBTestCase): + + scenarios = [ + ('project_id', { + 'expected_url': 'http://localhost/v2.1/user1_project', + 'auth_middleware': auth.NoAuthMiddleware}), + ('no_project_id', { + 'expected_url': 'http://localhost/v2.1', + 'auth_middleware': auth.NoAuthMiddlewareV2_17}), + ] def setUp(self): - super(TestNoAuthMiddlewareV21, self).setUp() + super(TestNoAuthMiddleware, self).setUp() fakes.stub_out_rate_limiting(self.stubs) fakes.stub_out_networking(self) - self.wsgi_app = fakes.wsgi_app_v21(use_no_auth=True) - self.req_url = '/v2' - self.expected_url = "http://localhost/v2/user1_project" + api_v21 = openstack_api.FaultWrapper( + self.auth_middleware( + compute.APIRouterV21() + ) + ) + self.wsgi_app = urlmap.URLMap() + self.wsgi_app['/v2.1'] = api_v21 + self.req_url = '/v2.1' def test_authorize_user(self): req = webob.Request.blank(self.req_url) @@ -66,16 +82,3 @@ class TestNoAuthMiddlewareV21(test.NoDBTestCase): self.assertEqual(result.status, '204 No Content') self.assertNotIn('X-CDN-Management-Url', result.headers) self.assertNotIn('X-Storage-Url', result.headers) - - -class TestNoAuthMiddlewareV3(TestNoAuthMiddlewareV21): - - def setUp(self): - super(TestNoAuthMiddlewareV3, self).setUp() - api_router = compute.APIRouterV3() - api_v3 = openstack_api.FaultWrapper(auth.NoAuthMiddlewareV3( - api_router)) - self.wsgi_app = urlmap.URLMap() - self.wsgi_app['/v3'] = api_v3 - self.req_url = '/v3' - self.expected_url = "http://localhost/v3" diff --git a/nova/tests/unit/api/openstack/compute/test_versions.py b/nova/tests/unit/api/openstack/compute/test_versions.py index 653c3d625bc0..92a4c638d9c8 100644 --- a/nova/tests/unit/api/openstack/compute/test_versions.py +++ b/nova/tests/unit/api/openstack/compute/test_versions.py @@ -66,7 +66,7 @@ EXP_VERSIONS = { "v2.1": { "id": "v2.1", "status": "CURRENT", - "version": "2.17", + "version": "2.18", "min_version": "2.1", "updated": "2013-07-23T11:33:21Z", "links": [ @@ -128,7 +128,7 @@ class VersionsTestV20(test.NoDBTestCase): { "id": "v2.1", "status": "CURRENT", - "version": "2.17", + "version": "2.18", "min_version": "2.1", "updated": "2013-07-23T11:33:21Z", "links": [ @@ -194,7 +194,7 @@ class VersionsTestV20(test.NoDBTestCase): self._test_get_version_2_detail('/', accept=accept) def test_get_version_2_versions_invalid(self): - req = webob.Request.blank('/v2/versions/1234') + req = webob.Request.blank('/v2/versions/1234/foo') req.accept = "application/json" res = req.get_response(self.wsgi_app) self.assertEqual(404, res.status_int) @@ -483,7 +483,7 @@ class VersionsTestV21(test.NoDBTestCase): self.assertEqual(expected, version) def test_get_version_21_versions_invalid(self): - req = webob.Request.blank('/v2.1/versions/1234') + req = webob.Request.blank('/v2.1/versions/1234/foo') req.accept = "application/json" res = req.get_response(fakes.wsgi_app_v21()) self.assertEqual(404, res.status_int) diff --git a/nova/tests/unit/conf_fixture.py b/nova/tests/unit/conf_fixture.py index d893bfee2a42..2f585572bdab 100644 --- a/nova/tests/unit/conf_fixture.py +++ b/nova/tests/unit/conf_fixture.py @@ -63,6 +63,11 @@ class ConfFixture(config_fixture.Config): group='api_database') self.conf.set_default('fatal_exception_format_errors', True) self.conf.set_default('enabled', True, 'osapi_v21') + # TODO(sdague): this makes our project_id match 'fake' and + # 'openstack' as well. We should fix the tests to use real + # UUIDs then drop this work around. + self.conf.set_default('project_id_regex', + '[0-9a-fopnstk\-]+', 'osapi_v21') self.conf.set_default('force_dhcp_release', False) self.conf.set_default('periodic_enable', False) policy_opts.set_defaults(self.conf) diff --git a/releasenotes/notes/optional_project_id-6aebf1cb394d498f.yaml b/releasenotes/notes/optional_project_id-6aebf1cb394d498f.yaml new file mode 100644 index 000000000000..6bce851bb28e --- /dev/null +++ b/releasenotes/notes/optional_project_id-6aebf1cb394d498f.yaml @@ -0,0 +1,16 @@ +--- +features: + + - Provides API 2.17, which makes the use of project_ids in API urls + optional. + +upgrade: + + - In order to make project_id optional in urls, we must constrain + the set of allowed values for project_id in our urls. This + defaults to a regex of ``[0-9a-f\-]+``, which will match hex uuids + (with / without dashes), and integers. This covers all known + project_id formats in the wild. + + If your site uses other values for project_id, you can set a site + specific validation with ``project_id_regex`` config variable.