diff --git a/neutron/db/l3_db.py b/neutron/db/l3_db.py index 9e1468d1926..026aa440acf 100644 --- a/neutron/db/l3_db.py +++ b/neutron/db/l3_db.py @@ -54,6 +54,7 @@ from neutron.extensions import l3 from neutron.extensions import segment as segment_ext from neutron.objects import base as base_obj +from neutron.objects import network as network_obj from neutron.objects import port_forwarding from neutron.objects import ports as port_obj from neutron.objects import router as l3_obj @@ -856,9 +857,24 @@ def _add_interface_by_subnet(self, context, router, subnet_id, owner): msg = _('Subnet for router interface must have a gateway IP') raise n_exc.BadRequest(resource='router', msg=msg) if subnet['project_id'] != context.project_id and not context.is_admin: - msg = (_('Cannot add interface to router because subnet %s is not ' - 'owned by project making the request') % subnet_id) - raise n_exc.BadRequest(resource='router', msg=msg) + # NOTE(amorin): check if network is RBAC or globaly shared + # globaly shared --> disallow adding interface (see LP-1757482) + # RBAC shared --> allow adding interface (see LP-1975603) + elevated = context.elevated() + + with db_api.CONTEXT_READER.using(elevated): + rbac_allowed_projects = network_obj.NetworkRBAC.get_projects( + elevated, object_id=subnet['network_id'], + action='access_as_shared', + target_project=context.project_id) + + # Fail if the current project_id is NOT in the allowed + # projects + if context.project_id not in rbac_allowed_projects: + msg = (_('Cannot add interface to router because subnet ' + '%s is not owned by project making the request') + % subnet_id) + raise n_exc.BadRequest(resource='router', msg=msg) self._validate_subnet_address_mode(subnet) self._check_for_dup_router_subnets(context, router, subnet['network_id'], [subnet]) diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py index 9626a58ecb3..04e14a1bc87 100644 --- a/neutron/plugins/ml2/plugin.py +++ b/neutron/plugins/ml2/plugin.py @@ -355,7 +355,9 @@ def _port_provisioned(self, rtype, event, trigger, payload=None): # a host ID and the dhcp agent notifies that its wiring is done LOG.debug('Port %s cannot update to ACTIVE because it ' 'is not bound.', port_id) - if count == MAX_PROVISIONING_TRIES: + owner = port.device_owner + if (count == MAX_PROVISIONING_TRIES or not + owner.startswith(const.DEVICE_OWNER_COMPUTE_PREFIX)): return # Wait 0.5 seconds before checking again if the port is bound. diff --git a/neutron/tests/unit/extensions/test_l3.py b/neutron/tests/unit/extensions/test_l3.py index 5791dcd2002..0621c40a53f 100644 --- a/neutron/tests/unit/extensions/test_l3.py +++ b/neutron/tests/unit/extensions/test_l3.py @@ -57,6 +57,7 @@ from neutron.db.models import l3 as l3_models from neutron.db import models_v2 from neutron.extensions import l3 +from neutron.objects import network as network_obj from neutron.services.revisions import revision_plugin from neutron.tests import base from neutron.tests.unit.api import test_extensions @@ -1320,6 +1321,26 @@ def test_router_add_interface_by_subnet_other_tenant_subnet_returns_400( expected_code=err_code, tenant_id=router_tenant_id) + def test_router_add_interface_by_subnet_other_tenant_subnet_rbac_shared( + self, + ): + router_tenant_id = _uuid() + with mock.patch.object(network_obj.NetworkRBAC, "get_projects") as g: + with self.router( + tenant_id=router_tenant_id, set_context=True + ) as r: + with self.network(shared=True) as n: + with self.subnet(network=n) as s: + g.return_value = [router_tenant_id] + self._router_interface_action( + "add", + r["router"]["id"], + s["subnet"]["id"], + None, + expected_code=exc.HTTPOk.code, + tenant_id=router_tenant_id, + ) + def _test_router_add_interface_by_port_allocation_pool( self, out_of_pool=False, router_action_as_admin=False, expected_code=exc.HTTPOk.code): diff --git a/neutron/tests/unit/plugins/ml2/test_plugin.py b/neutron/tests/unit/plugins/ml2/test_plugin.py index 28fed2046bb..b211e31022a 100644 --- a/neutron/tests/unit/plugins/ml2/test_plugin.py +++ b/neutron/tests/unit/plugins/ml2/test_plugin.py @@ -1126,7 +1126,9 @@ def test__port_provisioned_port_retry_port_binding_unbound( ml2_plugin.MAX_PROVISIONING_TRIES = 2 plugin = directory.get_plugin() port_id = 'fake_port_id' - port = mock.Mock(id=port_id, admin_state_up=True) + device_owner = constants.DEVICE_OWNER_COMPUTE_PREFIX + 'nova' + port = mock.Mock(id=port_id, admin_state_up=True, + device_owner=device_owner) mock_get_port.return_value = port with mock.patch.object(plugin, 'update_port_status') as mock_pstatus: pb1 = mock.MagicMock(vif_type=portbindings.VIF_TYPE_UNBOUND) @@ -1139,6 +1141,23 @@ def test__port_provisioned_port_retry_port_binding_unbound( mock_pstatus.assert_called_once_with(self.context, port_id, constants.PORT_STATUS_ACTIVE) + @mock.patch('neutron.plugins.ml2.plugin.db.get_port') + @mock.patch.object(p_utils, 'get_port_binding_by_status_and_host') + def test__port_provisioned_port_retry_port_binding_unbound_no_vm_port( + self, mock_get_pb, mock_get_port): + plugin = directory.get_plugin() + port_id = 'fake_port_id' + port = mock.Mock(id=port_id, admin_state_up=True, + device_owner='other_value') + mock_get_port.return_value = port + with mock.patch.object(plugin, 'update_port_status') as mock_pstatus: + pb1 = mock.MagicMock(vif_type=portbindings.VIF_TYPE_UNBOUND) + mock_get_pb.return_value = pb1 + plugin._port_provisioned('port', 'evt', 'trigger', + payload=events.DBEventPayload( + self.context, resource_id=port_id)) + mock_pstatus.assert_not_called() + def test_port_after_create_outside_transaction(self): self.tx_open = True