From 3d146d0e79c5411f7b156cae2085b17ef10d671f Mon Sep 17 00:00:00 2001 From: Bernard Cafarelli Date: Thu, 19 Jan 2017 14:14:12 +0100 Subject: [PATCH] Revert "Setup firewall filters only for required ports" This reverts commit 75edc1ff28a460342a9b5e5b7d63c6f4fb59862d. Ports with port security disabled require firewall entries in neutron-openvswi-FORWARD chain to work properly. Ports created with no security groups will not get skipped with current code. With fixed security groups check, these ports' security groups can not be updated after creation. Closes-Bug: #1549443 Conflicts: neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py neutron/tests/functional/agent/l2/base.py neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py Change-Id: I95ddbe38d8ac8a927a860a98f54e41e17fb71d43 (cherry picked from commit a8b6a597b6aab7cd3b0a5d0c3baad75af395fe1d) --- .../openvswitch/agent/ovs_neutron_agent.py | 13 ++----- neutron/plugins/ml2/rpc.py | 2 -- neutron/tests/functional/agent/l2/base.py | 2 -- .../agent/test_ovs_neutron_agent.py | 35 ++++--------------- 4 files changed, 8 insertions(+), 44 deletions(-) diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py index 3248b4f8e1e..4a9e71fc2ae 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py @@ -1488,7 +1488,6 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin, def treat_devices_added_or_updated(self, devices, ovs_restarted): skipped_devices = [] need_binding_devices = [] - security_disabled_devices = [] devices_details_list = ( self.plugin_rpc.get_devices_details_list_and_failed_devices( self.context, @@ -1526,11 +1525,6 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin, ovs_restarted) if need_binding: need_binding_devices.append(details) - - port_security = details['port_security_enabled'] - has_sgs = 'security_groups' in details - if not port_security or not has_sgs: - security_disabled_devices.append(device) self._update_port_network(details['port_id'], details['network_id']) self.ext_manager.handle_port(self.context, details) @@ -1541,7 +1535,7 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin, if (port and port.ofport != -1): self.port_dead(port) return (skipped_devices, need_binding_devices, - security_disabled_devices, failed_devices) + failed_devices) def _update_port_network(self, port_id, network_id): self._clean_network_ports(port_id) @@ -1621,11 +1615,10 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin, devices_added_updated = (port_info.get('added', set()) | port_info.get('updated', set())) need_binding_devices = [] - security_disabled_ports = [] if devices_added_updated: start = time.time() (skipped_devices, need_binding_devices, - security_disabled_ports, failed_devices['added']) = ( + failed_devices['added']) = ( self.treat_devices_added_or_updated( devices_added_updated, ovs_restarted)) LOG.debug("process_network_ports - iteration:%(iter_num)d - " @@ -1646,8 +1639,6 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin, # unnecessarily, (eg: when there are no IP address changes) added_ports = port_info.get('added', set()) self._add_port_tag_info(need_binding_devices) - if security_disabled_ports: - added_ports -= set(security_disabled_ports) self.sg_agent.setup_port_filters(added_ports, port_info.get('updated', set())) failed_devices['added'] |= self._bind_devices(need_binding_devices) diff --git a/neutron/plugins/ml2/rpc.py b/neutron/plugins/ml2/rpc.py index 62f235928dc..59660690787 100644 --- a/neutron/plugins/ml2/rpc.py +++ b/neutron/plugins/ml2/rpc.py @@ -128,8 +128,6 @@ class RpcCallbacks(type_tunnel.TunnelRpcCallbackMixin): 'qos_policy_id': port.get(qos_consts.QOS_POLICY_ID), 'network_qos_policy_id': network_qos_policy_id, 'profile': port[portbindings.PROFILE]} - if 'security_groups' in port: - entry['security_groups'] = port['security_groups'] LOG.debug("Returning: %s", entry) return entry diff --git a/neutron/tests/functional/agent/l2/base.py b/neutron/tests/functional/agent/l2/base.py index 2e091046806..7a03af6391e 100644 --- a/neutron/tests/functional/agent/l2/base.py +++ b/neutron/tests/functional/agent/l2/base.py @@ -214,8 +214,6 @@ class OVSAgentTestFramework(base.BaseOVSLinuxTestCase): 'segmentation_id': network.get('segmentation_id', 1), 'fixed_ips': port['fixed_ips'], 'device_owner': 'compute', - 'port_security_enabled': True, - 'security_groups': ['default'], 'admin_state_up': True} return dev diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py index 7e6a701bfbe..783d5ce046c 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py @@ -767,7 +767,7 @@ class TestOvsNeutronAgent(object): 'get_port_tag_dict', return_value={}),\ mock.patch.object(self.agent, func_name) as func: - skip_devs, need_bound_devices, insecure_ports, _ = ( + skip_devs, need_bound_devices, _ = ( self.agent.treat_devices_added_or_updated([], False)) # The function should not raise self.assertFalse(skip_devs) @@ -841,7 +841,7 @@ class TestOvsNeutronAgent(object): skip_devs = self.agent.treat_devices_added_or_updated([], False) # The function should return False for resync and no device # processed - self.assertEqual((['the_skipped_one'], [], [], set()), skip_devs) + self.assertEqual((['the_skipped_one'], [], set()), skip_devs) self.assertFalse(treat_vif_port.called) def test_treat_devices_added_failed_devices(self): @@ -856,7 +856,7 @@ class TestOvsNeutronAgent(object): mock.patch.object(self.agent, 'treat_vif_port') as treat_vif_port: failed_devices = {'added': set(), 'removed': set()} - (_, _, _, failed_devices['added']) = ( + (_, _, failed_devices['added']) = ( self.agent.treat_devices_added_or_updated([], False)) # The function should return False for resync and no device # processed @@ -873,8 +873,7 @@ class TestOvsNeutronAgent(object): 'network_type': 'baz', 'fixed_ips': [{'subnet_id': 'my-subnet-uuid', 'ip_address': '1.1.1.1'}], - 'device_owner': DEVICE_OWNER_COMPUTE, - 'port_security_enabled': True + 'device_owner': DEVICE_OWNER_COMPUTE } with mock.patch.object(self.agent.plugin_rpc, @@ -888,7 +887,7 @@ class TestOvsNeutronAgent(object): return_value={}),\ mock.patch.object(self.agent, 'treat_vif_port') as treat_vif_port: - skip_devs, need_bound_devices, insecure_ports, _ = ( + skip_devs, need_bound_devices, _ = ( self.agent.treat_devices_added_or_updated([], False)) # The function should return False for resync self.assertFalse(skip_devs) @@ -953,7 +952,7 @@ class TestOvsNeutronAgent(object): mock.patch.object( self.agent, "treat_devices_added_or_updated", return_value=( - [], [], [], + [], [], failed_devices['added'])) as device_added_updated,\ mock.patch.object(self.agent.int_br, "get_ports_attributes", return_value=[]),\ @@ -992,28 +991,6 @@ class TestOvsNeutronAgent(object): def test_process_network_port_with_empty_port(self): self._test_process_network_ports({}) - def test_process_network_ports_with_insecure_ports(self): - port_info = {'current': set(['tap0', 'tap1']), - 'updated': set(['tap1']), - 'removed': set([]), - 'added': set(['eth1'])} - failed_dev = {'added': set(), 'removed': set()} - with mock.patch.object(self.agent.sg_agent, - "setup_port_filters") as setup_port_filters,\ - mock.patch.object( - self.agent, - "treat_devices_added_or_updated", - return_value=( - [], [], ['eth1'], - failed_dev['added'])) as device_added_updated: - self.assertEqual( - failed_dev, - self.agent.process_network_ports(port_info, False)) - device_added_updated.assert_called_once_with( - set(['eth1', 'tap1']), False) - setup_port_filters.assert_called_once_with( - set(), port_info.get('updated', set())) - def test_hybrid_plug_flag_based_on_firewall(self): cfg.CONF.set_default( 'firewall_driver',