openstack
/
neutron-fwaas
Code Issues Proposed changes

Fix bug when updating policy in firewall group 

When updating only the policy in firewall group, the 'del-port-ids'
and 'add-port-ids' return empty list, which causes the fwg status
to be inactive and iptables in the router namespace are not changed.
This patch fixes the above problem.

Change-Id: I1a4bc0a8258fbbc340825cccb6d287c94304d3c5
Closes-Bug: #1836015
This commit is contained in:
zhanghao2 2019-07-23 06:30:24 -04:00
parent 243a7ef575
commit 3817119959
2 changed files with 100 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
neutron_fwaas/services/firewall/service_drivers/agents/l3reference/firewall_l3_agent_v2.py
View File

@ -142,8 +142,8 @@ class FWaaSL3AgentExtension(l3_extension.L3AgentExtension):
def _get_firewall_group_ports(self, context, firewall_group,
to_delete=False, require_new_plugin=False):
"""Returns in-namespace ports, either from firewall group dict if newer
version of plugin or from project routers otherwise.
"""Returns in-namespace ports, either from firewall group dict if ports
update or from project routers otherwise if only policies update.
NOTE: Vernacular move from "tenant" to "project" doesn't yet appear
as a key in router or firewall group objects.
@ -154,7 +154,7 @@ class FWaaSL3AgentExtension(l3_extension.L3AgentExtension):
fwg_port_ids = firewall_group['del-port-ids']
else:
fwg_port_ids = firewall_group['add-port-ids']
elif not require_new_plugin:
if not require_new_plugin and not fwg_port_ids:
routers = self.agent_api.get_routers_in_project(
firewall_group['tenant_id'])
for router in routers:

104
neutron_fwaas/tests/unit/services/firewall/service_drivers/agents/l3reference/test_firewall_l3_agent_v2.py
View File

@ -250,11 +250,16 @@ class TestFWaaSL3AgentExtension(base.BaseTestCase):
'ports': [],
'add-port-ids': [],
'del-port-ids': [],
'router_ids': []}
'last-port': True}
self.api.plugin_rpc = mock.Mock()
with mock.patch.object(self.api.fwaas_driver, 'update_firewall_group'
) as mock_driver_update_firewall_group, \
with mock.patch.object(self.api, '_get_firewall_group_ports'
) as mock_get_firewall_group_ports, \
mock.patch.object(self.api, '_get_in_ns_ports'
) as mock_get_in_ns_ports, \
mock.patch.object(self.api.fwaas_driver,
'update_firewall_group'
) as mock_driver_update_firewall_group, \
mock.patch.object(self.api.fwplugin_rpc,
'set_firewall_group_status'
) as mock_set_firewall_group_status:
@ -263,6 +268,15 @@ class TestFWaaSL3AgentExtension(base.BaseTestCase):
self.api.update_firewall_group(self.context, firewall_group,
host='host')
calls = [
mock.call._get_firewall_group_ports(
self.context, firewall_group, require_new_plugin=True,
to_delete=True),
mock.call._get_firewall_group_ports(
self.context, firewall_group)
]
mock_get_firewall_group_ports.assert_has_calls(calls)
mock_get_in_ns_ports.assert_called
mock_set_firewall_group_status.assert_called_once_with(
self.context, firewall_group['id'], 'INACTIVE')
@ -275,8 +289,32 @@ class TestFWaaSL3AgentExtension(base.BaseTestCase):
'del-port-ids': [],
'last-port': False
}
self.api.update_firewall_group(self.context, firewall_group,
host='host')
with mock.patch.object(self.api, '_get_firewall_group_ports'
) as mock_get_firewall_group_ports, \
mock.patch.object(self.api, '_get_in_ns_ports'
) as mock_get_in_ns_ports, \
mock.patch.object(self.api.fwaas_driver,
'update_firewall_group'
) as mock_driver_update_firewall_group, \
mock.patch.object(self.api.fwplugin_rpc,
'set_firewall_group_status'
) as mock_set_firewall_group_status:
mock_driver_update_firewall_group.return_value = True
self.api.update_firewall_group(self.context, firewall_group,
host='host')
calls = [
mock.call._get_firewall_group_ports(
self.context, firewall_group, require_new_plugin=True,
to_delete=True),
mock.call._get_firewall_group_ports(
self.context, firewall_group)
]
mock_get_firewall_group_ports.assert_has_calls(calls)
mock_get_in_ns_ports.assert_called
mock_set_firewall_group_status.assert_called_once_with(
self.context, firewall_group['id'], 'ACTIVE')
def test_update_firewall_group_with_only_ports_removed(self):
firewall_group = {'id': 0, 'project_id': 1,
@ -287,15 +325,67 @@ class TestFWaaSL3AgentExtension(base.BaseTestCase):
'last-port': False
}
self.api.plugin_rpc = mock.Mock()
with mock.patch.object(self.api.fwaas_driver, 'update_firewall_group'
) as mock_driver_update_firewall_group, \
with mock.patch.object(self.api, '_get_firewall_group_ports'
) as mock_get_firewall_group_ports, \
mock.patch.object(self.api, '_get_in_ns_ports'
) as mock_get_in_ns_ports, \
mock.patch.object(self.api.fwaas_driver,
'update_firewall_group'
) as mock_driver_update_firewall_group, \
mock.patch.object(self.api.fwplugin_rpc,
'set_firewall_group_status'
) as mock_set_firewall_group_status:
mock_driver_update_firewall_group.return_value = True
self.api.update_firewall_group(self.context, firewall_group,
host='host')
calls = [
mock.call._get_firewall_group_ports(
self.context, firewall_group, require_new_plugin=True,
to_delete=True),
mock.call._get_firewall_group_ports(
self.context, firewall_group)
]
mock_get_firewall_group_ports.assert_has_calls(calls)
mock_get_in_ns_ports.assert_called
mock_set_firewall_group_status.assert_called_once_with(
self.context, firewall_group['id'], 'ACTIVE')
def test_update_firewall_group_with_only_policies_added_or_deleted(self):
firewall_group = {'id': 0, 'project_id': 1,
'tenant_id': 1,
'admin_state_up': True,
'ports': [1, 2],
'add-port-ids': [],
'del-port-ids': [],
'last-port': False
}
self.api.plugin_rpc = mock.Mock()
with mock.patch.object(self.api, '_get_firewall_group_ports'
) as mock_get_firewall_group_ports, \
mock.patch.object(self.api, '_get_in_ns_ports'
) as mock_get_in_ns_ports, \
mock.patch.object(self.api.fwaas_driver,
'update_firewall_group'
) as mock_driver_update_firewall_group, \
mock.patch.object(self.api.fwplugin_rpc,
'set_firewall_group_status'
) as mock_set_firewall_group_status:
mock_driver_update_firewall_group.return_value = True
self.api.update_firewall_group(self.context, firewall_group,
host='host')
calls = [
mock.call._get_firewall_group_ports(
self.context, firewall_group, require_new_plugin=True,
to_delete=True),
mock.call._get_firewall_group_ports(
self.context, firewall_group)
]
mock_get_firewall_group_ports.assert_has_calls(calls)
mock_get_in_ns_ports.assert_called
mock_set_firewall_group_status.assert_called_once_with(
self.context, firewall_group['id'], 'ACTIVE')