Accept and return project_id for API calls

Update the API to accept project_id in requests and return
project_id in responses.

For now, the API treats tenant_id and project_id equivalently.
It accepts either or both in requests.
It returns both in responses, depending on filters.

We include an extension to indicate that support for project_id
is enabled in the API.

Completes: blueprint keystone-v3

APIImpact: Describe how the Networking API supports Keystone V3.

Co-Authored-By: Henry Gessau <HenryG@gessau.net>
Co-Authored-By: Akihiro Motoki <amotoki@gmail.com>

Change-Id: I8775aa8a477191ef21e7c3c6da31d098befefc3c

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)

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

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

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 {}

neutron/tests/contrib/hooks/api_extensions

@@ -23,6 +23,7 @@ NETWORK_API_EXTENSIONS="
     network_availability_zone, \
     pagination, \
     port-security, \
+    project-id, \
     provider, \
     qos, \
     quotas, \

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')

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

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'}

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:

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

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'])

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),\

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'])

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})

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)

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.