diff --git a/neutron/api/v2/attributes.py b/neutron/api/v2/attributes.py index b12472070ff..6f67c258fae 100644 --- a/neutron/api/v2/attributes.py +++ b/neutron/api/v2/attributes.py @@ -437,17 +437,55 @@ def convert_value(attr_info, res_dict, exc_cls=ValueError): raise exc_cls(msg) -def populate_tenant_id(context, res_dict, attr_info, is_create): - if (('tenant_id' in res_dict and - res_dict['tenant_id'] != context.tenant_id and - not context.is_admin)): - msg = _("Specifying 'tenant_id' other than authenticated " - "tenant in request requires admin privileges") +def populate_project_info(attributes): + """ + Ensure that both project_id and tenant_id attributes are present. + + If either project_id or tenant_id is present in attributes then ensure + that both are present. + + If neither are present then attributes is not updated. + + :param attributes: a dictionary of resource/API attributes + :type attributes: dict + + :return: the updated attributes dictionary + :rtype: dict + """ + if 'tenant_id' in attributes and 'project_id' not in attributes: + # TODO(HenryG): emit a deprecation warning here + attributes['project_id'] = attributes['tenant_id'] + elif 'project_id' in attributes and 'tenant_id' not in attributes: + # Backward compatibility for code still using tenant_id + attributes['tenant_id'] = attributes['project_id'] + + if attributes.get('project_id') != attributes.get('tenant_id'): + msg = _("'project_id' and 'tenant_id' do not match") raise webob.exc.HTTPBadRequest(msg) - if is_create and 'tenant_id' not in res_dict: - if context.tenant_id: - res_dict['tenant_id'] = context.tenant_id + return attributes + + +def _validate_privileges(context, res_dict): + if ('project_id' in res_dict and + res_dict['project_id'] != context.project_id and + not context.is_admin): + msg = _("Specifying 'project_id' or 'tenant_id' other than " + "authenticated project in request requires admin privileges") + raise webob.exc.HTTPBadRequest(msg) + + +def populate_tenant_id(context, res_dict, attr_info, is_create): + populate_project_info(res_dict) + _validate_privileges(context, res_dict) + + if is_create and 'project_id' not in res_dict: + if context.project_id: + res_dict['project_id'] = context.project_id + + # For backward compatibility + res_dict['tenant_id'] = context.project_id + elif 'tenant_id' in attr_info: msg = _("Running without keystone AuthN requires " "that tenant_id is specified") @@ -455,6 +493,8 @@ def populate_tenant_id(context, res_dict, attr_info, is_create): def verify_attributes(res_dict, attr_info): + populate_project_info(attr_info) + extra_keys = set(res_dict.keys()) - set(attr_info.keys()) if extra_keys: msg = _("Unrecognized attribute(s) '%s'") % ', '.join(extra_keys) diff --git a/neutron/api/v2/base.py b/neutron/api/v2/base.py index be79f719666..5ca5ddefc71 100644 --- a/neutron/api/v2/base.py +++ b/neutron/api/v2/base.py @@ -75,6 +75,23 @@ class Controller(object): def member_actions(self): return self._member_actions + def _init_policy_attrs(self): + """Create the list of attributes required by policy. + + If the attribute map contains a tenant_id policy, then include + project_id to bring the resource into the brave new world. + + :return: sorted list of attributes required by policy + + """ + policy_attrs = {name for (name, info) in self._attr_info.items() + if info.get('required_by_policy')} + if 'tenant_id' in policy_attrs: + policy_attrs.add('project_id') + + # Could use list(), but sorted() makes testing easier. + return sorted(policy_attrs) + def __init__(self, plugin, collection, resource, attr_info, allow_bulk=False, member_actions=None, parent=None, allow_pagination=False, allow_sorting=False): @@ -90,8 +107,7 @@ class Controller(object): self._native_bulk = self._is_native_bulk_supported() self._native_pagination = self._is_native_pagination_supported() self._native_sorting = self._is_native_sorting_supported() - self._policy_attrs = [name for (name, info) in self._attr_info.items() - if info.get('required_by_policy')] + self._policy_attrs = self._init_policy_attrs() self._notifier = n_rpc.get_notifier('network') self._member_actions = member_actions self._primary_key = self._get_primary_key() @@ -146,6 +162,13 @@ class Controller(object): """ attributes_to_exclude = [] for attr_name in data.keys(): + # TODO(amotoki): At now, all attribute maps have tenant_id and + # determine excluded attributes based on tenant_id. + # We need to migrate tenant_id to project_id later + # as attr_info is referred to in various places and we need + # to check all logis carefully. + if attr_name == 'project_id': + continue attr_data = self._attr_info.get(attr_name) if attr_data and attr_data['is_visible']: if policy.check( @@ -159,6 +182,12 @@ class Controller(object): # if the code reaches this point then either the policy check # failed or the attribute was not visible in the first place attributes_to_exclude.append(attr_name) + # TODO(amotoki): As mentioned in the above TODO, + # we treat project_id and tenant_id equivalently. + # This should be migrated to project_id in Ocata. + if attr_name == 'tenant_id': + attributes_to_exclude.append('project_id') + return attributes_to_exclude def _view(self, context, data, fields_to_strip=None): diff --git a/neutron/db/common_db_mixin.py b/neutron/db/common_db_mixin.py index 397bc5ad030..bebe6d829e2 100644 --- a/neutron/db/common_db_mixin.py +++ b/neutron/db/common_db_mixin.py @@ -27,6 +27,8 @@ from sqlalchemy import or_ from sqlalchemy import sql from neutron._i18n import _LE +from neutron.api.v2 import attributes + LOG = logging.getLogger(__name__) @@ -189,9 +191,9 @@ class CommonDbMixin(object): def _fields(self, resource, fields): if fields: - return dict(((key, item) for key, item in resource.items() - if key in fields)) - return resource + resource = {key: item for key, item in resource.items() + if key in fields} + return attributes.populate_project_info(resource) def _get_by_id(self, context, model, id): query = self._model_query(context, model) @@ -294,7 +296,8 @@ class CommonDbMixin(object): limit=limit, marker_obj=marker_obj, page_reverse=page_reverse) - items = [dict_func(c, fields) for c in query] + items = [attributes.populate_project_info(dict_func(c, fields)) + for c in query] if limit and page_reverse: items.reverse() return items diff --git a/neutron/extensions/project_id.py b/neutron/extensions/project_id.py new file mode 100644 index 00000000000..678bfa94416 --- /dev/null +++ b/neutron/extensions/project_id.py @@ -0,0 +1,52 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +# implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from neutron.api import extensions + + +_ALIAS = 'project-id' + + +class Project_id(extensions.ExtensionDescriptor): + """Extension that indicates that project_id is enabled. + + This extension indicates that the Keystone V3 'project_id' field + is supported in the API. + """ + + extensions.register_custom_supported_check( + _ALIAS, lambda: True, plugin_agnostic=True + ) + + @classmethod + def get_name(cls): + return "project_id field enabled" + + @classmethod + def get_alias(cls): + return _ALIAS + + @classmethod + def get_description(cls): + return "Extension that indicates that project_id field is enabled." + + @classmethod + def get_updated(cls): + return "2016-09-09T09:09:09-09:09" + + @classmethod + def get_resources(cls): + return [] + + def get_extended_resources(self, version): + return {} diff --git a/neutron/tests/contrib/hooks/api_extensions b/neutron/tests/contrib/hooks/api_extensions index 1d5d6001046..677724003d2 100644 --- a/neutron/tests/contrib/hooks/api_extensions +++ b/neutron/tests/contrib/hooks/api_extensions @@ -23,6 +23,7 @@ NETWORK_API_EXTENSIONS=" network_availability_zone, \ pagination, \ port-security, \ + project-id, \ provider, \ qos, \ quotas, \ diff --git a/neutron/tests/tempest/api/test_extensions.py b/neutron/tests/tempest/api/test_extensions.py index 28029d8c352..b1b09e8c801 100644 --- a/neutron/tests/tempest/api/test_extensions.py +++ b/neutron/tests/tempest/api/test_extensions.py @@ -34,3 +34,7 @@ class ExtensionsTest(base.BaseNetworkTest): @test.idempotent_id('19db409e-a23f-445d-8bc8-ca3d64c84706') def test_list_extensions_pagination(self): self._test_list_extensions_includes('pagination') + + @test.idempotent_id('155b7bc2-e358-4dd8-bf3e-1774c084567f') + def test_list_extensions_project_id(self): + self._test_list_extensions_includes('project-id') diff --git a/neutron/tests/tempest/api/test_networks.py b/neutron/tests/tempest/api/test_networks.py index 2c2991a2460..a6d82623192 100644 --- a/neutron/tests/tempest/api/test_networks.py +++ b/neutron/tests/tempest/api/test_networks.py @@ -46,6 +46,10 @@ class NetworksTestJSON(base.BaseNetworkTest): fields.append('mtu') for key in fields: self.assertEqual(network[key], self.network[key]) + project_id = self.client.tenant_id + self.assertEqual(project_id, network['tenant_id']) + if test.is_extension_enabled('project-id', 'network'): + self.assertEqual(project_id, network['project_id']) @test.idempotent_id('867819bb-c4b6-45f7-acf9-90edcf70aa5e') def test_show_network_fields(self): @@ -59,6 +63,27 @@ class NetworksTestJSON(base.BaseNetworkTest): self.assertEqual(sorted(network.keys()), sorted(fields)) for field_name in fields: self.assertEqual(network[field_name], self.network[field_name]) + self.assertNotIn('tenant_id', network) + self.assertNotIn('project_id', network) + + @test.idempotent_id('26f2b7a5-2cd1-4f3a-b11f-ad259b099b11') + @test.requires_ext(extension="project-id", service="network") + def test_show_network_fields_keystone_v3(self): + + def _check_show_network_fields(fields, expect_project_id, + expect_tenant_id): + params = {} + if fields: + params['fields'] = fields + body = self.client.show_network(self.network['id'], **params) + network = body['network'] + self.assertEqual(expect_project_id, 'project_id' in network) + self.assertEqual(expect_tenant_id, 'tenant_id' in network) + + _check_show_network_fields(None, True, True) + _check_show_network_fields(['tenant_id'], False, True) + _check_show_network_fields(['project_id'], True, False) + _check_show_network_fields(['project_id', 'tenant_id'], True, True) @test.idempotent_id('c72c1c0c-2193-4aca-ccc4-b1442640bbbb') @test.requires_ext(extension="standard-attr-description", @@ -75,6 +100,28 @@ class NetworksTestJSON(base.BaseNetworkTest): body = self.client.list_networks(id=net_id)['networks'][0] self.assertEqual('d2', body['description']) + @test.idempotent_id('0cc0552f-afaf-4231-b7a7-c2a1774616da') + @test.requires_ext(extension="project-id", service="network") + def test_create_network_keystone_v3(self): + project_id = self.client.tenant_id + + name = 'created-with-project_id' + body = self.client.create_network_keystone_v3(name, project_id) + new_net = body['network'] + self.assertEqual(name, new_net['name']) + self.assertEqual(project_id, new_net['project_id']) + self.assertEqual(project_id, new_net['tenant_id']) + + body = self.client.list_networks(id=new_net['id'])['networks'][0] + self.assertEqual(name, body['name']) + + new_name = 'create-with-project_id-2' + body = self.client.update_network(new_net['id'], name=new_name) + new_net = body['network'] + self.assertEqual(new_name, new_net['name']) + self.assertEqual(project_id, new_net['project_id']) + self.assertEqual(project_id, new_net['tenant_id']) + @test.idempotent_id('6ae6d24f-9194-4869-9c85-c313cb20e080') def test_list_networks_fields(self): # Verify specific fields of the networks @@ -87,6 +134,26 @@ class NetworksTestJSON(base.BaseNetworkTest): for network in networks: self.assertEqual(sorted(network.keys()), sorted(fields)) + @test.idempotent_id('a23186b9-aa6f-4b08-b877-35ca3b9cd54c') + @test.requires_ext(extension="project-id", service="network") + def test_list_networks_fields_keystone_v3(self): + def _check_list_networks_fields(fields, expect_project_id, + expect_tenant_id): + params = {} + if fields: + params['fields'] = fields + body = self.client.list_networks(**params) + networks = body['networks'] + self.assertNotEmpty(networks, "Network list returned is empty") + for network in networks: + self.assertEqual(expect_project_id, 'project_id' in network) + self.assertEqual(expect_tenant_id, 'tenant_id' in network) + + _check_list_networks_fields(None, True, True) + _check_list_networks_fields(['tenant_id'], False, True) + _check_list_networks_fields(['project_id'], True, False) + _check_list_networks_fields(['project_id', 'tenant_id'], True, True) + class NetworksSearchCriteriaTest(base.BaseSearchCriteriaTest): diff --git a/neutron/tests/tempest/api/test_qos.py b/neutron/tests/tempest/api/test_qos.py index a2a9941301d..b9572ca4404 100644 --- a/neutron/tests/tempest/api/test_qos.py +++ b/neutron/tests/tempest/api/test_qos.py @@ -554,6 +554,7 @@ class RbacSharedQosPoliciesTest(base.BaseAdminNetworkTest): self.client2.show_qos_policy(qos_pol['id']) rbac_pol = {'target_tenant': '*', 'tenant_id': self.admin_client.tenant_id, + 'project_id': self.admin_client.tenant_id, 'object_type': 'qos_policy', 'object_id': qos_pol['id'], 'action': 'access_as_shared'} diff --git a/neutron/tests/tempest/services/network/json/network_client.py b/neutron/tests/tempest/services/network/json/network_client.py index bcb55d7374b..48280598bf3 100644 --- a/neutron/tests/tempest/services/network/json/network_client.py +++ b/neutron/tests/tempest/services/network/json/network_client.py @@ -872,6 +872,19 @@ class NetworkClientJSON(service_client.RestClient): body = jsonutils.loads(body) return service_client.ResponseBody(resp, body) + def create_network_keystone_v3(self, name, project_id): + uri = '%s/networks' % self.uri_prefix + post_data = { + 'network': { + 'name': name, + 'project_id': project_id + } + } + resp, body = self.post(uri, self.serialize(post_data)) + body = self.deserialize_single(body) + self.expected_success(201, resp.status) + return service_client.ResponseBody(resp, body) + def list_extensions(self, **filters): uri = self.get_uri("extensions") if filters: diff --git a/neutron/tests/unit/api/v2/test_attributes.py b/neutron/tests/unit/api/v2/test_attributes.py index 400d40d6157..3c959caccd2 100644 --- a/neutron/tests/unit/api/v2/test_attributes.py +++ b/neutron/tests/unit/api/v2/test_attributes.py @@ -129,7 +129,9 @@ class TestResDict(base.BaseTestCase): # req body res_dict2 = {} attributes.populate_tenant_id(ctx, res_dict2, None, True) - self.assertEqual({'tenant_id': ctx.tenant_id}, res_dict2) + self.assertEqual( + {'tenant_id': ctx.tenant_id, 'project_id': ctx.tenant_id}, + res_dict2) # if the tenant_id is mandatory for the resource and not specified # in the request nor in the context, an exception should be raised diff --git a/neutron/tests/unit/api/v2/test_base.py b/neutron/tests/unit/api/v2/test_base.py index 05c462394fc..ebf75e8c3b0 100644 --- a/neutron/tests/unit/api/v2/test_base.py +++ b/neutron/tests/unit/api/v2/test_base.py @@ -137,10 +137,18 @@ def _list_cmp(l1, l2): class APIv2TestCase(APIv2TestBase): + + @staticmethod + def _get_policy_attrs(attr_info): + policy_attrs = {name for (name, info) in attr_info.items() + if info.get('required_by_policy')} + if 'tenant_id' in policy_attrs: + policy_attrs.add('project_id') + return sorted(policy_attrs) + def _do_field_list(self, resource, base_fields): attr_info = attributes.RESOURCE_ATTRIBUTE_MAP[resource] - policy_attrs = [name for (name, info) in attr_info.items() - if info.get('required_by_policy')] + policy_attrs = self._get_policy_attrs(attr_info) for name, info in attr_info.items(): if info.get('primary_key'): policy_attrs.append(name) @@ -488,6 +496,7 @@ class APIv2TestCase(APIv2TestBase): 'id', 'subnets', 'shared', + 'project_id', 'tenant_id'])) instance.get_networks.assert_called_once_with(mock.ANY, **kwargs) @@ -771,7 +780,11 @@ class JSONV2TestCase(APIv2TestBase, testlib_api.WebTestCase): def test_create_use_defaults(self): net_id = _uuid() - initial_input = {'network': {'name': 'net1', 'tenant_id': _uuid()}} + tenant_id = _uuid() + + initial_input = {'network': {'name': 'net1', + 'tenant_id': tenant_id, + 'project_id': tenant_id}} full_input = {'network': {'admin_state_up': True, 'shared': False}} full_input['network'].update(initial_input['network']) @@ -807,7 +820,8 @@ class JSONV2TestCase(APIv2TestBase, testlib_api.WebTestCase): # tenant_id should be fetched from env initial_input = {'network': {'name': 'net1'}} full_input = {'network': {'admin_state_up': True, - 'shared': False, 'tenant_id': tenant_id}} + 'shared': False, 'tenant_id': tenant_id, + 'project_id': tenant_id}} full_input['network'].update(initial_input['network']) return_value = {'id': net_id, 'status': "ACTIVE"} @@ -918,6 +932,7 @@ class JSONV2TestCase(APIv2TestBase, testlib_api.WebTestCase): device_id = _uuid() initial_input = {'port': {'name': '', 'network_id': net_id, 'tenant_id': tenant_id, + 'project_id': tenant_id, 'device_id': device_id, 'admin_state_up': True}} full_input = {'port': {'admin_state_up': True, @@ -1218,8 +1233,10 @@ class SubresourceTest(base.BaseTestCase): def test_create_sub_resource(self): instance = self.plugin.return_value + tenant_id = _uuid() - body = {'dummy': {'foo': 'bar', 'tenant_id': _uuid()}} + body = {'dummy': {'foo': 'bar', 'tenant_id': tenant_id, + 'project_id': tenant_id}} self.api.post_json('/networks/id1/dummies', body) instance.create_network_dummy.assert_called_once_with(mock.ANY, network_id='id1', @@ -1499,7 +1516,9 @@ class ExtensionTestCase(base.BaseTestCase): def test_extended_create(self): net_id = _uuid() - initial_input = {'network': {'name': 'net1', 'tenant_id': _uuid(), + tenant_id = _uuid() + initial_input = {'network': {'name': 'net1', 'tenant_id': tenant_id, + 'project_id': tenant_id, 'v2attrs:something_else': "abc"}} data = {'network': {'admin_state_up': True, 'shared': False}} data['network'].update(initial_input['network']) diff --git a/neutron/tests/unit/db/test_db_base_plugin_v2.py b/neutron/tests/unit/db/test_db_base_plugin_v2.py index 4c916a668b6..53875a7a674 100644 --- a/neutron/tests/unit/db/test_db_base_plugin_v2.py +++ b/neutron/tests/unit/db/test_db_base_plugin_v2.py @@ -2925,14 +2925,16 @@ class TestNetworksV2(NeutronDbPluginV2TestCase): query_params=query_params) def test_list_networks_with_fields(self): - with self.network(name='net1') as net1: + with self.network(name='net1'): req = self.new_list_request('networks', params='fields=name') res = self.deserialize(self.fmt, req.get_response(self.api)) self.assertEqual(1, len(res['networks'])) - self.assertEqual(res['networks'][0]['name'], - net1['network']['name']) - self.assertIsNone(res['networks'][0].get('id')) + net = res['networks'][0] + self.assertEqual('net1', net['name']) + self.assertNotIn('id', net) + self.assertNotIn('tenant_id', net) + self.assertNotIn('project_id', net) def test_list_networks_with_parameters_invalid_values(self): with self.network(name='net1', admin_state_up=False),\ diff --git a/neutron/tests/unit/extensions/test_flavors.py b/neutron/tests/unit/extensions/test_flavors.py index 2ebe5272dc0..b036e3c62fd 100644 --- a/neutron/tests/unit/extensions/test_flavors.py +++ b/neutron/tests/unit/extensions/test_flavors.py @@ -62,6 +62,7 @@ class FlavorExtensionTestCase(extension.ExtensionTestCase): 'service_type': constants.FLAVORS, 'description': 'the best flavor', 'tenant_id': tenant_id, + 'project_id': tenant_id, 'enabled': True}} expected = copy.deepcopy(data) @@ -228,6 +229,7 @@ class FlavorExtensionTestCase(extension.ExtensionTestCase): expected = {'service_profile': {'description': 'the best sp', 'driver': '', 'tenant_id': tenant_id, + 'project_id': tenant_id, 'enabled': True, 'metainfo': '{"data": "value"}'}} @@ -374,7 +376,8 @@ class FlavorExtensionTestCase(extension.ExtensionTestCase): def test_associate_service_profile_with_flavor(self): tenant_id = uuidutils.generate_uuid() expected = {'service_profile': {'id': _uuid(), - 'tenant_id': tenant_id}} + 'tenant_id': tenant_id, + 'project_id': tenant_id}} instance = self.plugin.return_value instance.create_flavor_service_profile.return_value = ( expected['service_profile']) diff --git a/neutron/tests/unit/extensions/test_l3.py b/neutron/tests/unit/extensions/test_l3.py index 7722832073d..9b26696c6b7 100644 --- a/neutron/tests/unit/extensions/test_l3.py +++ b/neutron/tests/unit/extensions/test_l3.py @@ -100,8 +100,9 @@ class L3NatExtensionTestCase(test_extensions_base.ExtensionTestCase): def test_router_create(self): router_id = _uuid() + tenant_id = _uuid() data = {'router': {'name': 'router1', 'admin_state_up': True, - 'tenant_id': _uuid(), + 'tenant_id': tenant_id, 'project_id': tenant_id, 'external_gateway_info': None}} return_value = copy.deepcopy(data['router']) return_value.update({'status': "ACTIVE", 'id': router_id}) diff --git a/neutron/tests/unit/extensions/test_providernet.py b/neutron/tests/unit/extensions/test_providernet.py index ef425780cf1..fcd98a3056e 100644 --- a/neutron/tests/unit/extensions/test_providernet.py +++ b/neutron/tests/unit/extensions/test_providernet.py @@ -125,12 +125,14 @@ class ProvidernetExtensionTestCase(testlib_api.WebTestCase): def test_network_create_with_provider_attrs(self): ctx = context.get_admin_context() - ctx.tenant_id = 'an_admin' + tenant_id = 'an_admin' + ctx.tenant_id = tenant_id res, data = self._post_network_with_provider_attrs(ctx) instance = self.plugin.return_value exp_input = {'network': data} exp_input['network'].update({'admin_state_up': True, - 'tenant_id': 'an_admin', + 'tenant_id': tenant_id, + 'project_id': tenant_id, 'shared': False}) instance.create_network.assert_called_with(mock.ANY, network=exp_input) diff --git a/releasenotes/notes/project_id-d5ea7a42be428230.yaml b/releasenotes/notes/project_id-d5ea7a42be428230.yaml new file mode 100644 index 00000000000..dbf78a97393 --- /dev/null +++ b/releasenotes/notes/project_id-d5ea7a42be428230.yaml @@ -0,0 +1,7 @@ +--- +features: + - The Networking API now supports the 'project_id' field in requests and + responses, for compatibility with the Identity (Keystone) API V3. A new + API extension, 'project-id', has been added to allow API users to detect + if the 'project_id' field is supported. Note that the 'tenant_id' field + is still supported, and the two fields are functionally equivalent.