diff --git a/neutron/db/common_db_mixin.py b/neutron/db/common_db_mixin.py index 2a65420e5e4..cf319cd3797 100644 --- a/neutron/db/common_db_mixin.py +++ b/neutron/db/common_db_mixin.py @@ -129,9 +129,8 @@ class CommonDbMixin(object): query_filter = None if self.model_query_scope(context, model): if hasattr(model, 'rbac_entries'): - rbac_model, join_params = self._get_rbac_query_params( - model)[:2] - query = query.outerjoin(*join_params) + query = query.outerjoin(model.rbac_entries) + rbac_model = model.rbac_entries.property.mapper.class_ query_filter = ( (model.tenant_id == context.tenant_id) | ((rbac_model.action == 'access_as_shared') & @@ -184,27 +183,6 @@ class CommonDbMixin(object): query = self._model_query(context, model) return query.filter(model.id == id).one() - @staticmethod - def _get_rbac_query_params(model): - """Return the parameters required to query an model's RBAC entries. - - Returns a tuple of 3 containing: - 1. the relevant RBAC model for a given model - 2. the join parameters required to query the RBAC entries for the model - 3. the ID column of the passed in model that matches the object_id - in the rbac entries. - """ - try: - cls = model.rbac_entries.property.mapper.class_ - return (cls, (cls, ), model.id) - except AttributeError: - # an association proxy is being used (e.g. subnets - # depends on network's rbac entries) - rbac_model = (model.rbac_entries.target_class. - rbac_entries.property.mapper.class_) - return (rbac_model, model.rbac_entries.attr, - model.rbac_entries.remote_attr.class_.id) - def _apply_filters_to_query(self, query, model, filters, context=None): if filters: for key, value in six.iteritems(filters): @@ -222,9 +200,8 @@ class CommonDbMixin(object): elif key == 'shared' and hasattr(model, 'rbac_entries'): # translate a filter on shared into a query against the # object's rbac entries - rbac, join_params, oid_col = self._get_rbac_query_params( - model) - query = query.outerjoin(*join_params, aliased=True) + query = query.outerjoin(model.rbac_entries) + rbac = model.rbac_entries.property.mapper.class_ matches = [rbac.target_tenant == '*'] if context: matches.append(rbac.target_tenant == context.tenant_id) @@ -240,6 +217,12 @@ class CommonDbMixin(object): # because that will still give us a network shared to # our tenant (or wildcard) if it's shared to another # tenant. + # This is the column joining the table to rbac via + # the object_id. We can't just use model.id because + # subnets join on network.id so we have to inspect the + # relationship. + join_cols = model.rbac_entries.property.local_columns + oid_col = list(join_cols)[0] is_shared = ~oid_col.in_( query.session.query(rbac.object_id). filter(is_shared) diff --git a/neutron/db/models_v2.py b/neutron/db/models_v2.py index 51a483c0084..9ecba83ba9c 100644 --- a/neutron/db/models_v2.py +++ b/neutron/db/models_v2.py @@ -14,7 +14,6 @@ # under the License. import sqlalchemy as sa -from sqlalchemy.ext.associationproxy import association_proxy from sqlalchemy import orm from neutron.api.v2 import attributes as attr @@ -205,7 +204,12 @@ class Subnet(model_base.BASEV2, HasId, HasTenant): constants.DHCPV6_STATEFUL, constants.DHCPV6_STATELESS, name='ipv6_address_modes'), nullable=True) - rbac_entries = association_proxy('networks', 'rbac_entries') + # subnets don't have their own rbac_entries, they just inherit from + # the network rbac entries + rbac_entries = orm.relationship( + rbac_db_models.NetworkRBAC, lazy='joined', + foreign_keys='Subnet.network_id', + primaryjoin='Subnet.network_id==NetworkRBAC.object_id') class SubnetPoolPrefix(model_base.BASEV2): diff --git a/neutron/tests/api/admin/test_shared_network_extension.py b/neutron/tests/api/admin/test_shared_network_extension.py index 87bf8ebb91c..1979e25622a 100644 --- a/neutron/tests/api/admin/test_shared_network_extension.py +++ b/neutron/tests/api/admin/test_shared_network_extension.py @@ -347,16 +347,20 @@ class RBACSharedNetworksTest(base.BaseAdminNetworkTest): def test_filtering_works_with_rbac_records_present(self): resp = self._make_admin_net_and_subnet_shared_to_tenant_id( self.client.tenant_id) - net = resp['network'] - sub = resp['subnet'] + net = resp['network']['id'] + sub = resp['subnet']['id'] self.admin_client.create_rbac_policy( - object_type='network', object_id=net['id'], + object_type='network', object_id=net, action='access_as_shared', target_tenant='*') - for state, assertion in ((False, self.assertNotIn), - (True, self.assertIn)): - nets = [n['id'] for n in - self.admin_client.list_networks(shared=state)['networks']] - assertion(net['id'], nets) - subs = [s['id'] for s in - self.admin_client.list_subnets(shared=state)['subnets']] - assertion(sub['id'], subs) + self._assert_shared_object_id_listing_presence('subnets', False, sub) + self._assert_shared_object_id_listing_presence('subnets', True, sub) + self._assert_shared_object_id_listing_presence('networks', False, net) + self._assert_shared_object_id_listing_presence('networks', True, net) + + def _assert_shared_object_id_listing_presence(self, resource, shared, oid): + lister = getattr(self.admin_client, 'list_%s' % resource) + objects = [o['id'] for o in lister(shared=shared)[resource]] + if shared: + self.assertIn(oid, objects) + else: + self.assertNotIn(oid, objects)