Skip to content

Commit b532978

Browse files
Return 409 Conflict to tenant user deleting port attached to FIP
When a tenant user try to delete a port that has attached a FIP by an admin user is getting a 500 ServerError. This patch improves the error to 409 Conflict doing some additionals checks on the delete_port method. New exception has been included locally, but will be removed as soon neutron-lib bumps to a newer release. Closes-Bug: 2017680 Change-Id: Iab77c64c03fd0d44ff7a3fc1c556d85a8c480bb9 (cherry picked from commit 9f6f6d5)
1 parent 64e3752 commit b532978

File tree

2 files changed

+44
-1
lines changed

2 files changed

+44
-1
lines changed

neutron/db/l3_db.py

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,13 @@
8282
'port: %(port_id)s.')
8383

8484

85+
# TODO(froyo): Move this exception to neutron-lib as soon as possible, and when
86+
# a new release is created and pointed to in the requirements remove this code.
87+
class FipAssociated(n_exc.InUse):
88+
message = _('Unable to complete the operation on port "%(port_id)s" '
89+
'because the port still has an associated floating IP.')
90+
91+
8592
@registry.has_registry_receivers
8693
class L3_NAT_dbonly_mixin(l3.RouterPluginBase,
8794
base_services.WorkerBase,
@@ -1768,12 +1775,27 @@ def disassociate_floatingips(self, context, port_id, do_notify=True):
17681775
@return: set of router-ids that require notification updates
17691776
"""
17701777
with db_api.CONTEXT_WRITER.using(context):
1778+
# NOTE(froyo): Context is elevated to confirm the presence of at
1779+
# least one FIP associated to the port_id. Additional checks
1780+
# regarding the tenant's grants will be carried out in following
1781+
# lines.
17711782
if not l3_obj.FloatingIP.objects_exist(
1772-
context, fixed_port_id=port_id):
1783+
context.elevated(), fixed_port_id=port_id):
17731784
return []
17741785

17751786
floating_ip_objs = l3_obj.FloatingIP.get_objects(
17761787
context, fixed_port_id=port_id)
1788+
1789+
# NOTE(froyo): To ensure that a FIP assigned by an admin user
1790+
# cannot be disassociated by a tenant user, we raise exception to
1791+
# generate a 409 Conflict response message that prompts the tenant
1792+
# user to contact an admin, rather than a 500 error message.
1793+
if not context.is_admin:
1794+
floating_ip_objs_admin = l3_obj.FloatingIP.get_objects(
1795+
context.elevated(), fixed_port_id=port_id)
1796+
if floating_ip_objs_admin != floating_ip_objs:
1797+
raise FipAssociated(port_id=port_id)
1798+
17771799
router_ids = {fip.router_id for fip in floating_ip_objs}
17781800
old_fips = {fip.id: self._make_floatingip_dict(fip)
17791801
for fip in floating_ip_objs}

neutron/tests/unit/db/test_l3_db.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,27 @@ def test_prevent_l3_port_existing_floating_ip(self, gp):
328328

329329
self.db.prevent_l3_port_deletion(ctx, None)
330330

331+
@mock.patch.object(l3_obj.FloatingIP, 'objects_exist')
332+
@mock.patch.object(l3_obj.FloatingIP, 'get_objects')
333+
def test_disassociate_floatingips_conflict_by_fip_attached(self,
334+
get_objects,
335+
objects_exist):
336+
context_tenant = context.Context('tenant', 'tenant', is_admin=False)
337+
objects_exist.return_value = True
338+
get_objects.side_effect = [
339+
[],
340+
[{'id': 'floating_ip1', 'port_id': 'port_id'}]]
341+
self.assertRaises(l3_db.FipAssociated,
342+
self.db.disassociate_floatingips,
343+
context_tenant,
344+
'port_id')
345+
objects_exist.assert_called_once_with(
346+
mock.ANY, fixed_port_id='port_id')
347+
expected_calls = [
348+
mock.call(context_tenant, fixed_port_id='port_id'),
349+
mock.call(mock.ANY, fixed_port_id='port_id')]
350+
get_objects.assert_has_calls(expected_calls)
351+
331352
@mock.patch.object(directory, 'get_plugin')
332353
def test_subscribe_address_scope_of_subnetpool(self, gp):
333354
l3_db.L3RpcNotifierMixin()

0 commit comments

Comments
 (0)