From 6d49834976bc08d2b8f7939bc2a217f5cc81db47 Mon Sep 17 00:00:00 2001 From: Claudiu Belu Date: Thu, 27 Mar 2014 18:36:44 +0200 Subject: [PATCH] Fixes Hyper-V agent security groups disabling Adds a check if the security groups have been disabled and removes the switch port's ACLs accordingly. Change-Id: I20d716a3f182b8ea62da6b436b150aa9dafdb1c5 Closes-Bug: #1299156 --- .../hyperv/agent/hyperv_neutron_agent.py | 15 ++++++++--- neutron/plugins/hyperv/agent/utilsv2.py | 27 ++++++++++++++++--- .../tests/unit/hyperv/test_hyperv_utilsv2.py | 23 ++++++++++++++++ 3 files changed, 58 insertions(+), 7 deletions(-) diff --git a/neutron/plugins/hyperv/agent/hyperv_neutron_agent.py b/neutron/plugins/hyperv/agent/hyperv_neutron_agent.py index c86626508..3c4ff1b2d 100644 --- a/neutron/plugins/hyperv/agent/hyperv_neutron_agent.py +++ b/neutron/plugins/hyperv/agent/hyperv_neutron_agent.py @@ -85,9 +85,9 @@ class HyperVSecurityAgent(sg_rpc.SecurityGroupAgentRpcMixin): def __init__(self, context, plugin_rpc): self.context = context self.plugin_rpc = plugin_rpc - self.init_firewall() if sg_rpc.is_firewall_enabled(): + self.init_firewall() self._setup_rpc() def _setup_rpc(self): @@ -216,8 +216,9 @@ class HyperVNeutronAgent(object): def port_update(self, context, port=None, network_type=None, segmentation_id=None, physical_network=None): LOG.debug(_("port_update received")) - if 'security_groups' in port: - self.sec_groups_agent.refresh_firewall() + if CONF.SECURITYGROUP.enable_security_group: + if 'security_groups' in port: + self.sec_groups_agent.refresh_firewall() self._treat_vif_port( port['id'], port['network_id'], @@ -383,7 +384,13 @@ class HyperVNeutronAgent(object): device_details['segmentation_id'], device_details['admin_state_up']) - self.sec_groups_agent.prepare_devices_filter(devices) + # check if security groups is enabled. + # if not, teardown the security group rules + if CONF.SECURITYGROUP.enable_security_group: + self.sec_groups_agent.prepare_devices_filter([device]) + else: + self._utils.remove_all_security_rules( + device_details['port_id']) self.plugin_rpc.update_device_up(self.context, device, self.agent_id, diff --git a/neutron/plugins/hyperv/agent/utilsv2.py b/neutron/plugins/hyperv/agent/utilsv2.py index 9b1b467b6..a55839487 100644 --- a/neutron/plugins/hyperv/agent/utilsv2.py +++ b/neutron/plugins/hyperv/agent/utilsv2.py @@ -62,6 +62,9 @@ class HyperVUtilsV2(utils.HyperVUtils): _ICMP_PROTOCOL = '1' _MAX_WEIGHT = 65500 + # 2 directions x 2 address types = 4 ACLs + _REJECT_ACLS_COUNT = 4 + _wmi_namespace = '//./root/virtualization/v2' def __init__(self): @@ -105,9 +108,12 @@ class HyperVUtilsV2(utils.HyperVUtils): self._check_job_status(ret_val, job_path) def _remove_virt_feature(self, feature_resource): + self._remove_multiple_virt_features([feature_resource]) + + def _remove_multiple_virt_features(self, feature_resources): vs_man_svc = self._conn.Msvm_VirtualSystemManagementService()[0] (job_path, ret_val) = vs_man_svc.RemoveFeatureSettings( - FeatureSettings=[feature_resource.path_()]) + FeatureSettings=[f.path_() for f in feature_resources]) self._check_job_status(ret_val, job_path) def disconnect_switch_port( @@ -294,6 +300,19 @@ class HyperVUtilsV2(utils.HyperVUtils): for acl in filtered_acls: self._remove_virt_feature(acl) + def remove_all_security_rules(self, switch_port_name): + port, found = self._get_switch_port_allocation(switch_port_name, False) + if not found: + # Port not found. It happens when the VM was already deleted. + return + + acls = port.associators(wmi_result_class=self._PORT_EXT_ACL_SET_DATA) + filtered_acls = [a for a in acls if + a.Action is not self._ACL_ACTION_METER] + + if filtered_acls: + self._remove_multiple_virt_features(filtered_acls) + def create_default_reject_all_rules(self, switch_port_name): port, found = self._get_switch_port_allocation(switch_port_name, False) if not found: @@ -303,8 +322,7 @@ class HyperVUtilsV2(utils.HyperVUtils): acls = port.associators(wmi_result_class=self._PORT_EXT_ACL_SET_DATA) filtered_acls = [v for v in acls if v.Action == self._ACL_ACTION_DENY] - # 2 directions x 2 address types x 2 protocols = 8 ACLs - if len(filtered_acls) >= 8: + if len(filtered_acls) >= self._REJECT_ACLS_COUNT: return for acl in filtered_acls: @@ -383,6 +401,9 @@ class HyperVUtilsV2R2(HyperVUtilsV2): _PORT_EXT_ACL_SET_DATA = 'Msvm_EthernetSwitchPortExtendedAclSettingData' _MAX_WEIGHT = 65500 + # 2 directions x 2 address types x 3 protocols = 12 ACLs + _REJECT_ACLS_COUNT = 12 + def _create_security_acl(self, direction, acl_type, action, local_port, protocol, remote_addr, weight): acl = self._get_default_setting_data(self._PORT_EXT_ACL_SET_DATA) diff --git a/neutron/tests/unit/hyperv/test_hyperv_utilsv2.py b/neutron/tests/unit/hyperv/test_hyperv_utilsv2.py index af70f33b9..c020f16e0 100644 --- a/neutron/tests/unit/hyperv/test_hyperv_utilsv2.py +++ b/neutron/tests/unit/hyperv/test_hyperv_utilsv2.py @@ -371,6 +371,21 @@ class TestHyperVUtilsV2(base.BaseTestCase): self._utils._remove_virt_feature.assert_called_once_with(m_acl) self._utils._bind_security_rule.assert_has_calls(calls) + @mock.patch('neutron.plugins.hyperv.agent.utilsv2.HyperVUtilsV2' + '._remove_virt_feature') + @mock.patch('neutron.plugins.hyperv.agent.utilsv2.HyperVUtilsV2' + '._bind_security_rule') + def test_create_default_reject_all_rules_already_added(self, mock_bind, + mock_remove): + (m_port, m_acl) = self._setup_security_rule_test() + m_acl.Action = self._utils._ACL_ACTION_DENY + m_port.associators.return_value = [ + m_acl] * self._utils._REJECT_ACLS_COUNT + self._utils.create_default_reject_all_rules(self._FAKE_PORT_NAME) + + self.assertFalse(self._utils._remove_virt_feature.called) + self.assertFalse(self._utils._bind_security_rule.called) + @mock.patch('neutron.plugins.hyperv.agent.utilsv2.HyperVUtilsV2' '._remove_virt_feature') @mock.patch('neutron.plugins.hyperv.agent.utilsv2.HyperVUtilsV2' @@ -397,6 +412,14 @@ class TestHyperVUtilsV2(base.BaseTestCase): self._FAKE_LOCAL_PORT, self._FAKE_PROTOCOL, self._FAKE_REMOTE_ADDR) self._utils._remove_virt_feature.assert_called_once_with(mock_acl) + @mock.patch('neutron.plugins.hyperv.agent.utilsv2.HyperVUtilsV2' + '._remove_multiple_virt_features') + def test_remove_all_security_rules(self, mock_remove_feature): + mock_acl = self._setup_security_rule_test()[1] + self._utils.remove_all_security_rules(self._FAKE_PORT_NAME) + self._utils._remove_multiple_virt_features.assert_called_once_with( + [mock_acl]) + def _setup_security_rule_test(self): mock_port = mock.MagicMock() mock_acl = mock.MagicMock() -- 2.45.2