From c5e2d4b6b4dfa64f898249d6ed871cf6d41c58a2 Mon Sep 17 00:00:00 2001 From: Hans Lindgren Date: Thu, 23 Apr 2015 22:43:24 +0200 Subject: [PATCH] Remove unused provider firewall rules functionality in nova Provider firewall rules functionality is not in use and hasn't been for a very long time. The api for this was removed in [1] and db api methods for adding/removing rows in the associated db table have not been used since. Stop refreshing those rules as it is essentially a no-op and indeed a costly one that includes a rpc round trip to the conductor to get back an always empty db result. This should have a positive impact on instance boot performance since the conductor call happens to live inside an externally syncronized block of code. Removes related compute rpcapi/manager code that were missed in a recent cleanup[2]. Since this functionality hasn't been in use since Havana timeframe(!), it should be fairly safe to remove without first deprecating it. Also removes the now unused virtapi method provider_fw_rule_get_all() and the virtapi itself from virt firewall driver initialization. [1] Commit: 62d5fae8d11b6403f9a63a709270ffafebb7ef09 [2] Commit: e6f7d8041783a0b3c740559d97b8d40b3568f214 Change-Id: Ifbb2514b9bc1445eaa07dcfe172c7405fd1a58f7 Partial-Bug: #1016633 --- nova/compute/manager.py | 8 -- nova/compute/rpcapi.py | 6 +- nova/tests/unit/compute/test_rpcapi.py | 4 - nova/tests/unit/compute/test_virtapi.py | 3 - nova/tests/unit/virt/ironic/test_driver.py | 7 -- nova/tests/unit/virt/libvirt/test_driver.py | 2 +- nova/tests/unit/virt/libvirt/test_firewall.py | 82 +------------- nova/tests/unit/virt/test_virt_drivers.py | 5 - nova/tests/unit/virt/xenapi/test_xenapi.py | 57 ---------- nova/virt/driver.py | 17 --- nova/virt/fake.py | 7 -- nova/virt/firewall.py | 106 +----------------- nova/virt/ironic/driver.py | 4 - nova/virt/libvirt/driver.py | 4 - nova/virt/libvirt/firewall.py | 18 +-- nova/virt/virtapi.py | 6 - nova/virt/xenapi/driver.py | 3 - nova/virt/xenapi/firewall.py | 62 +--------- nova/virt/xenapi/vmops.py | 4 - 19 files changed, 13 insertions(+), 392 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 9d13d030c80c..94dedc4260cb 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -602,9 +602,6 @@ class ComputeVirtAPI(virtapi.VirtAPI): super(ComputeVirtAPI, self).__init__() self._compute = compute - def provider_fw_rule_get_all(self, context): - return self._compute.conductor_api.provider_fw_rule_get_all(context) - def _default_error_callback(self, event_name, instance): raise exception.NovaException(_('Instance event failed')) @@ -1392,11 +1389,6 @@ class ComputeManager(manager.Manager): return _sync_refresh() - @wrap_exception() - def refresh_provider_fw_rules(self, context): - """This call passes straight through to the virtualization driver.""" - return self.driver.refresh_provider_fw_rules() - def _await_block_device_map_created(self, context, vol_id): # TODO(yamahata): creating volume simultaneously # reduces creation time? diff --git a/nova/compute/rpcapi.py b/nova/compute/rpcapi.py index 8f2606a64666..67ddf03da4c1 100644 --- a/nova/compute/rpcapi.py +++ b/nova/compute/rpcapi.py @@ -324,6 +324,7 @@ class ComputeAPI(object): * 4.8 - Send migrate_data in object format for live_migration, rollback_live_migration_at_destination, and pre_live_migration. + * ... - Remove refresh_provider_fw_rules() ''' VERSION_ALIASES = { @@ -727,11 +728,6 @@ class ComputeAPI(object): recreate=recreate, on_shared_storage=on_shared_storage, **extra) - def refresh_provider_fw_rules(self, ctxt, host): - version = '4.0' - cctxt = self.client.prepare(server=host, version=version) - cctxt.cast(ctxt, 'refresh_provider_fw_rules') - def remove_aggregate_host(self, ctxt, aggregate, host_param, host, slave_info=None): '''Remove aggregate host. diff --git a/nova/tests/unit/compute/test_rpcapi.py b/nova/tests/unit/compute/test_rpcapi.py index a643ce1aac96..476b317c5f29 100644 --- a/nova/tests/unit/compute/test_rpcapi.py +++ b/nova/tests/unit/compute/test_rpcapi.py @@ -383,10 +383,6 @@ class ComputeRpcAPITestCase(test.NoDBTestCase): version='4.0', _return_value=objects_block_dev.BlockDeviceMapping()) - def refresh_provider_fw_rules(self): - self._test_compute_api('refresh_provider_fw_rules', 'cast', - host='host') - def test_refresh_instance_security_rules(self): expected_args = {'instance': self.fake_instance_obj} self._test_compute_api('refresh_instance_security_rules', 'cast', diff --git a/nova/tests/unit/compute/test_virtapi.py b/nova/tests/unit/compute/test_virtapi.py index 5a236760d4c0..28a4f4cdd68a 100644 --- a/nova/tests/unit/compute/test_virtapi.py +++ b/nova/tests/unit/compute/test_virtapi.py @@ -42,9 +42,6 @@ class VirtAPIBaseTest(test.NoDBTestCase, test.APICoverage): getattr(self.virtapi, method), self.context, *args, **kwargs) - def test_provider_fw_rule_get_all(self): - self.assertExpected('provider_fw_rule_get_all') - def test_wait_for_instance_event(self): self.assertExpected('wait_for_instance_event', 'instance', ['event']) diff --git a/nova/tests/unit/virt/ironic/test_driver.py b/nova/tests/unit/virt/ironic/test_driver.py index 3c86333c5af2..55880476d8a1 100644 --- a/nova/tests/unit/virt/ironic/test_driver.py +++ b/nova/tests/unit/virt/ironic/test_driver.py @@ -1504,13 +1504,6 @@ class IronicDriverTestCase(test.NoDBTestCase): self.driver.refresh_instance_security_rules(instance) mock_risr.assert_called_once_with(instance) - @mock.patch.object(firewall.NoopFirewallDriver, - 'refresh_provider_fw_rules', create=True) - def test_refresh_provider_fw_rules(self, mock_rpfr): - fake_instance.fake_instance_obj(self.ctx) - self.driver.refresh_provider_fw_rules() - mock_rpfr.assert_called_once_with() - @mock.patch.object(firewall.NoopFirewallDriver, 'refresh_instance_security_rules', create=True) def test_refresh_security_group_rules(self, mock_risr): diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index f81dee5dd6bc..ad0873f499bd 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -6094,7 +6094,7 @@ class LibvirtConnTestCase(test.NoDBTestCase): filterref = './devices/interface/filterref' vif = network_info[0] nic_id = vif['address'].replace(':', '') - fw = firewall.NWFilterFirewall(fake.FakeVirtAPI(), drvr) + fw = firewall.NWFilterFirewall(drvr) instance_filter_name = fw._instance_filter_name(instance_ref, nic_id) self.assertEqual(tree.find(filterref).get('filter'), diff --git a/nova/tests/unit/virt/libvirt/test_firewall.py b/nova/tests/unit/virt/libvirt/test_firewall.py index 58fd290d8213..6a203a19147c 100644 --- a/nova/tests/unit/virt/libvirt/test_firewall.py +++ b/nova/tests/unit/virt/libvirt/test_firewall.py @@ -33,7 +33,6 @@ from nova.tests.unit.virt.libvirt import fakelibvirt from nova.virt.libvirt import firewall from nova.virt.libvirt import host from nova.virt import netutils -from nova.virt import virtapi _fake_network_info = fake_network.fake_get_instance_nw_info _fake_stub_out_get_nw_info = fake_network.stub_out_nw_api_get_instance_nw_info @@ -81,11 +80,6 @@ class NWFilterFakes(object): return True -class FakeVirtAPI(virtapi.VirtAPI): - def provider_fw_rule_get_all(self, context): - return [] - - class IptablesFirewallTestCase(test.NoDBTestCase): def setUp(self): super(IptablesFirewallTestCase, self).setUp() @@ -93,7 +87,6 @@ class IptablesFirewallTestCase(test.NoDBTestCase): self.useFixture(fakelibvirt.FakeLibvirtFixture()) self.fw = firewall.IptablesFirewallDriver( - FakeVirtAPI(), host=host.Host("qemu:///system")) in_rules = [ @@ -431,77 +424,6 @@ class IptablesFirewallTestCase(test.NoDBTestCase): # should undefine just the instance filter self.assertEqual(original_filter_count - len(fakefilter.filters), 1) - @mock.patch.object(FakeVirtAPI, "provider_fw_rule_get_all") - @mock.patch.object(objects.SecurityGroupRuleList, "get_by_instance") - def test_provider_firewall_rules(self, mock_secrule, mock_fwrules): - mock_secrule.return_value = objects.SecurityGroupRuleList() - - # setup basic instance data - instance_ref = self._create_instance_ref() - instance_ref.security_groups = objects.SecurityGroupList() - - # FRAGILE: peeks at how the firewall names chains - chain_name = 'inst-%s' % instance_ref['id'] - - # create a firewall via setup_basic_filtering like libvirt_conn.spawn - # should have a chain with 0 rules - network_info = _fake_network_info(self, 1) - self.fw.setup_basic_filtering(instance_ref, network_info) - self.assertIn('provider', self.fw.iptables.ipv4['filter'].chains) - rules = [rule for rule in self.fw.iptables.ipv4['filter'].rules - if rule.chain == 'provider'] - self.assertEqual(0, len(rules)) - - # add a rule angd send the update message, check for 1 rule - sec_grp_rule_obj = objects.SecurityGroupRule(protocol='tcp', - cidr='10.99.99.99/32', - from_port=1, - to_port=65535) - mock_fwrules.return_value = [sec_grp_rule_obj] - self.fw.refresh_provider_fw_rules() - rules = [rule for rule in self.fw.iptables.ipv4['filter'].rules - if rule.chain == 'provider'] - self.assertEqual(1, len(rules)) - - # Add another, refresh, and make sure number of rules goes to two - sec_grp_rule_obj1 = objects.SecurityGroupRule(protocol='tcp', - cidr='10.99.99.99/32', - from_port=1, - to_port=65535) - sec_grp_rule_obj2 = objects.SecurityGroupRule(protocol='udp', - cidr='10.99.99.99/32', - from_port=1, - to_port=65535) - mock_fwrules.return_value = [sec_grp_rule_obj1, sec_grp_rule_obj2] - self.fw.refresh_provider_fw_rules() - rules = [rule for rule in self.fw.iptables.ipv4['filter'].rules - if rule.chain == 'provider'] - self.assertEqual(2, len(rules)) - - # create the instance filter and make sure it has a jump rule - self.fw.prepare_instance_filter(instance_ref, network_info) - self.fw.apply_instance_filter(instance_ref, network_info) - inst_rules = [rule for rule in self.fw.iptables.ipv4['filter'].rules - if rule.chain == chain_name] - jump_rules = [rule for rule in inst_rules if '-j' in rule.rule] - provjump_rules = [] - # IptablesTable doesn't make rules unique internally - for rule in jump_rules: - if 'provider' in rule.rule and rule not in provjump_rules: - provjump_rules.append(rule) - self.assertEqual(1, len(provjump_rules)) - - # remove a rule from the db, cast to compute to refresh rule - sec_grp_rule_obj = objects.SecurityGroupRule(protocol='udp', - cidr='10.99.99.99/32', - from_port=1, - to_port=65535) - mock_fwrules.return_value = [sec_grp_rule_obj] - self.fw.refresh_provider_fw_rules() - rules = [rule for rule in self.fw.iptables.ipv4['filter'].rules - if rule.chain == 'provider'] - self.assertEqual(1, len(rules)) - @mock.patch.object(firewall, 'libvirt', fakelibvirt) class NWFilterTestCase(test.NoDBTestCase): @@ -510,9 +432,7 @@ class NWFilterTestCase(test.NoDBTestCase): self.useFixture(fakelibvirt.FakeLibvirtFixture()) - self.fw = firewall.NWFilterFirewall( - FakeVirtAPI(), - host=host.Host("qemu:///system")) + self.fw = firewall.NWFilterFirewall(host=host.Host("qemu:///system")) def _create_security_group(self, instance_ref): secgroup = objects.SecurityGroup(id=1, diff --git a/nova/tests/unit/virt/test_virt_drivers.py b/nova/tests/unit/virt/test_virt_drivers.py index d3b4832a2f83..73bbd6bd0205 100644 --- a/nova/tests/unit/virt/test_virt_drivers.py +++ b/nova/tests/unit/virt/test_virt_drivers.py @@ -642,11 +642,6 @@ class _VirtDriverTestCase(_FakeDriverBackendTestCase): instance_ref, network_info = self._get_running_instance() self.connection.refresh_instance_security_rules(instance_ref) - @catch_notimplementederror - def test_refresh_provider_fw_rules(self): - instance_ref, network_info = self._get_running_instance() - self.connection.refresh_provider_fw_rules() - @catch_notimplementederror def test_ensure_filtering_for_instance(self): instance = test_utils.get_test_instance(obj=True) diff --git a/nova/tests/unit/virt/xenapi/test_xenapi.py b/nova/tests/unit/virt/xenapi/test_xenapi.py index ca40549a6adb..a5eef5af32f6 100644 --- a/nova/tests/unit/virt/xenapi/test_xenapi.py +++ b/nova/tests/unit/virt/xenapi/test_xenapi.py @@ -2862,63 +2862,6 @@ class XenAPIDom0IptablesFirewallTestCase(stubs.XenAPITestBase): "Rules were not updated properly. " "The rule for UDP acceptance is missing") - def test_provider_firewall_rules(self): - # setup basic instance data - instance_ref = self._create_instance_ref() - # FRAGILE: as in libvirt tests - # peeks at how the firewall names chains - chain_name = 'inst-%s' % instance_ref['id'] - - network_info = fake_network.fake_get_instance_nw_info(self, 1, 1) - self.fw.prepare_instance_filter(instance_ref, network_info) - self.assertIn('provider', self.fw.iptables.ipv4['filter'].chains) - rules = [rule for rule in self.fw.iptables.ipv4['filter'].rules - if rule.chain == 'provider'] - self.assertEqual(0, len(rules)) - - admin_ctxt = context.get_admin_context() - # add a rule and send the update message, check for 1 rule - db.provider_fw_rule_create(admin_ctxt, - {'protocol': 'tcp', - 'cidr': '10.99.99.99/32', - 'from_port': 1, - 'to_port': 65535}) - self.fw.refresh_provider_fw_rules() - rules = [rule for rule in self.fw.iptables.ipv4['filter'].rules - if rule.chain == 'provider'] - self.assertEqual(1, len(rules)) - - # Add another, refresh, and make sure number of rules goes to two - provider_fw1 = db.provider_fw_rule_create(admin_ctxt, - {'protocol': 'udp', - 'cidr': '10.99.99.99/32', - 'from_port': 1, - 'to_port': 65535}) - self.fw.refresh_provider_fw_rules() - rules = [rule for rule in self.fw.iptables.ipv4['filter'].rules - if rule.chain == 'provider'] - self.assertEqual(2, len(rules)) - - # create the instance filter and make sure it has a jump rule - self.fw.prepare_instance_filter(instance_ref, network_info) - self.fw.apply_instance_filter(instance_ref, network_info) - inst_rules = [rule for rule in self.fw.iptables.ipv4['filter'].rules - if rule.chain == chain_name] - jump_rules = [rule for rule in inst_rules if '-j' in rule.rule] - provjump_rules = [] - # IptablesTable doesn't make rules unique internally - for rule in jump_rules: - if 'provider' in rule.rule and rule not in provjump_rules: - provjump_rules.append(rule) - self.assertEqual(1, len(provjump_rules)) - - # remove a rule from the db, cast to compute to refresh rule - db.provider_fw_rule_destroy(admin_ctxt, provider_fw1['id']) - self.fw.refresh_provider_fw_rules() - rules = [rule for rule in self.fw.iptables.ipv4['filter'].rules - if rule.chain == 'provider'] - self.assertEqual(1, len(rules)) - class XenAPISRSelectionTestCase(stubs.XenAPITestBaseNoDB): """Unit tests for testing we find the right SR.""" diff --git a/nova/virt/driver.py b/nova/virt/driver.py index 5367e8631bb5..378d1f8285ca 100644 --- a/nova/virt/driver.py +++ b/nova/virt/driver.py @@ -1016,23 +1016,6 @@ class ComputeDriver(object): # TODO(Vek): Need to pass context in for access to auth_token raise NotImplementedError() - def refresh_provider_fw_rules(self): - """This triggers a firewall update based on database changes. - - When this is called, rules have either been added or removed from the - datastore. You can retrieve rules with - :py:meth:`nova.db.provider_fw_rule_get_all`. - - Provider rules take precedence over security group rules. If an IP - would be allowed by a security group ingress rule, but blocked by - a provider rule, then packets from the IP are dropped. This includes - intra-project traffic in the case of the allow_project_net_traffic - flag for the libvirt-derived classes. - - """ - # TODO(Vek): Need to pass context in for access to auth_token - raise NotImplementedError() - def refresh_instance_security_rules(self, instance): """Refresh security group rules diff --git a/nova/virt/fake.py b/nova/virt/fake.py index d5456d6f3e53..328ef850213b 100644 --- a/nova/virt/fake.py +++ b/nova/virt/fake.py @@ -37,7 +37,6 @@ from nova.compute import power_state from nova.compute import task_states from nova.compute import vm_mode from nova.console import type as ctype -from nova import db from nova import exception from nova.i18n import _LW from nova.virt import diagnostics @@ -428,9 +427,6 @@ class FakeDriver(driver.ComputeDriver): def refresh_instance_security_rules(self, instance): return True - def refresh_provider_fw_rules(self): - pass - def get_available_resource(self, nodename): """Updates compute manager resource info on ComputeNode table. @@ -542,9 +538,6 @@ class FakeDriver(driver.ComputeDriver): class FakeVirtAPI(virtapi.VirtAPI): - def provider_fw_rule_get_all(self, context): - return db.provider_fw_rule_get_all(context) - @contextlib.contextmanager def wait_for_instance_event(self, instance, event_names, deadline=300, error_callback=None): diff --git a/nova/virt/firewall.py b/nova/virt/firewall.py index 8fc2110a6092..0b3120a5e055 100644 --- a/nova/virt/firewall.py +++ b/nova/virt/firewall.py @@ -51,12 +51,9 @@ def load_driver(default, *args, **kwargs): class FirewallDriver(object): """Firewall Driver base class. - Defines methods that any driver providing security groups - and provider firewall functionality should implement. - """ - def __init__(self, virtapi): - self._virtapi = virtapi + Defines methods that any driver providing security groups should implement. + """ def prepare_instance_filter(self, instance, network_info): """Prepare filters for the instance. @@ -103,15 +100,6 @@ class FirewallDriver(object): """ raise NotImplementedError() - def refresh_provider_fw_rules(self): - """Refresh common rules for all hosts/instances from data store. - - Gets called when a rule has been added to or removed from - the list of rules (via admin api). - - """ - raise NotImplementedError() - def setup_basic_filtering(self, instance, network_info): """Create rules to block spoofing and allow dhcp. @@ -129,11 +117,9 @@ class FirewallDriver(object): class IptablesFirewallDriver(FirewallDriver): """Driver which enforces security groups through iptables rules.""" - def __init__(self, virtapi, **kwargs): - super(IptablesFirewallDriver, self).__init__(virtapi) + def __init__(self, **kwargs): self.iptables = linux_net.iptables_manager self.instance_info = {} - self.basically_filtered = False # Flags for DHCP request rule self.dhcp_create = False @@ -172,9 +158,6 @@ class IptablesFirewallDriver(FirewallDriver): ipv6_rules) LOG.debug('Filters added to instance: %s', instance.id, instance=instance) - self.refresh_provider_fw_rules() - LOG.debug('Provider Firewall Rules refreshed: %s', instance.id, - instance=instance) # Ensure that DHCP request rule is updated if necessary if (self.dhcp_create and not self.dhcp_created): self.iptables.ipv4['filter'].add_rule( @@ -258,10 +241,6 @@ class IptablesFirewallDriver(FirewallDriver): ipv4_rules += ['-m state --state ESTABLISHED,RELATED -j ACCEPT'] ipv6_rules += ['-m state --state ESTABLISHED,RELATED -j ACCEPT'] - # Pass through provider-wide drops - ipv4_rules += ['-j $provider'] - ipv6_rules += ['-j $provider'] - def _do_dhcp_rules(self, ipv4_rules, network_info): v4_subnets = self._get_subnets(network_info, 4) dhcp_servers = [subnet.get_meta('dhcp_server') @@ -450,85 +429,6 @@ class IptablesFirewallDriver(FirewallDriver): self._inner_do_refresh_rules(instance, network_info, ipv4_rules, ipv6_rules) - def refresh_provider_fw_rules(self): - """See :class:`FirewallDriver` docs.""" - self._do_refresh_provider_fw_rules() - self.iptables.apply() - - @utils.synchronized('iptables', external=True) - def _do_refresh_provider_fw_rules(self): - """Internal, synchronized version of refresh_provider_fw_rules.""" - self._purge_provider_fw_rules() - self._build_provider_fw_rules() - - def _purge_provider_fw_rules(self): - """Remove all rules from the provider chains.""" - self.iptables.ipv4['filter'].empty_chain('provider') - if CONF.use_ipv6: - self.iptables.ipv6['filter'].empty_chain('provider') - - def _build_provider_fw_rules(self): - """Create all rules for the provider IP DROPs.""" - self.iptables.ipv4['filter'].add_chain('provider') - if CONF.use_ipv6: - self.iptables.ipv6['filter'].add_chain('provider') - ipv4_rules, ipv6_rules = self._provider_rules() - for rule in ipv4_rules: - self.iptables.ipv4['filter'].add_rule('provider', rule) - - if CONF.use_ipv6: - for rule in ipv6_rules: - self.iptables.ipv6['filter'].add_rule('provider', rule) - - def _provider_rules(self): - """Generate a list of rules from provider for IP4 & IP6.""" - ctxt = context.get_admin_context() - ipv4_rules = [] - ipv6_rules = [] - rules = self._virtapi.provider_fw_rule_get_all(ctxt) - for rule in rules: - LOG.debug('Adding provider rule: %s', rule.cidr) - version = netutils.get_ip_version(rule.cidr) - if version == 4: - fw_rules = ipv4_rules - else: - fw_rules = ipv6_rules - - protocol = rule.protocol - if version == 6 and protocol == 'icmp': - protocol = 'icmpv6' - - args = ['-p', protocol, '-s', str(rule.cidr)] - - if protocol in ['udp', 'tcp']: - if rule.from_port == rule.to_port: - args += ['--dport', '%s' % (rule.from_port,)] - else: - args += ['-m', 'multiport', - '--dports', '%s:%s' % (rule.from_port, - rule.to_port)] - elif protocol == 'icmp': - icmp_type = rule.from_port - icmp_code = rule.to_port - - if icmp_type == -1: - icmp_type_arg = None - else: - icmp_type_arg = '%s' % icmp_type - if not icmp_code == -1: - icmp_type_arg += '/%s' % icmp_code - - if icmp_type_arg: - if version == 4: - args += ['-m', 'icmp', '--icmp-type', - icmp_type_arg] - elif version == 6: - args += ['-m', 'icmp6', '--icmpv6-type', - icmp_type_arg] - args += ['-j DROP'] - fw_rules += [' '.join(args)] - return ipv4_rules, ipv6_rules - class NoopFirewallDriver(object): """Firewall driver which just provides No-op methods.""" diff --git a/nova/virt/ironic/driver.py b/nova/virt/ironic/driver.py index f2ed93ba38a8..704fa73c7900 100644 --- a/nova/virt/ironic/driver.py +++ b/nova/virt/ironic/driver.py @@ -985,10 +985,6 @@ class IronicDriver(virt_driver.ComputeDriver): """ self.firewall_driver.refresh_security_group_rules(security_group_id) - def refresh_provider_fw_rules(self): - """Triggers a firewall update based on database changes.""" - self.firewall_driver.refresh_provider_fw_rules() - def refresh_instance_security_rules(self, instance): """Refresh security group rules from data store. diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index d83f7b7342bc..695f9fea7034 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -502,7 +502,6 @@ class LibvirtDriver(driver.ComputeDriver): self._caps = None self.firewall_driver = firewall.load_driver( DEFAULT_FIREWALL_DRIVER, - self.virtapi, host=self._host) self.vif_driver = libvirt_vif.LibvirtGenericVIFDriver() @@ -5251,9 +5250,6 @@ class LibvirtDriver(driver.ComputeDriver): def refresh_instance_security_rules(self, instance): self.firewall_driver.refresh_instance_security_rules(instance) - def refresh_provider_fw_rules(self): - self.firewall_driver.refresh_provider_fw_rules() - def get_available_resource(self, nodename): """Retrieve resource information. diff --git a/nova/virt/libvirt/firewall.py b/nova/virt/libvirt/firewall.py index cbfaca461ea8..65a79bb8c320 100644 --- a/nova/virt/libvirt/firewall.py +++ b/nova/virt/libvirt/firewall.py @@ -45,15 +45,13 @@ class NWFilterFirewall(base_firewall.FirewallDriver): spoofing, IP spoofing, and ARP spoofing. """ - def __init__(self, virtapi, host, **kwargs): + def __init__(self, host, **kwargs): """Create an NWFilter firewall driver - :param virtapi: nova.virt.virtapi.VirtAPI instance :param host: nova.virt.libvirt.host.Host instance :param kwargs: currently unused """ - super(NWFilterFirewall, self).__init__(virtapi) global libvirt if libvirt is None: try: @@ -324,10 +322,9 @@ class NWFilterFirewall(base_firewall.FirewallDriver): class IptablesFirewallDriver(base_firewall.IptablesFirewallDriver): - def __init__(self, virtapi, execute=None, **kwargs): + def __init__(self, execute=None, **kwargs): """Create an IP tables firewall driver instance - :param virtapi: nova.virt.virtapi.VirtAPI instance :param execute: unused, pass None :param kwargs: extra arguments @@ -336,17 +333,12 @@ class IptablesFirewallDriver(base_firewall.IptablesFirewallDriver): class. """ - super(IptablesFirewallDriver, self).__init__(virtapi, **kwargs) - self.nwfilter = NWFilterFirewall(virtapi, kwargs['host']) + super(IptablesFirewallDriver, self).__init__(**kwargs) + self.nwfilter = NWFilterFirewall(kwargs['host']) def setup_basic_filtering(self, instance, network_info): - """Set up provider rules and basic NWFilter.""" + """Set up basic NWFilter.""" self.nwfilter.setup_basic_filtering(instance, network_info) - if not self.basically_filtered: - LOG.debug('iptables firewall: Setup Basic Filtering', - instance=instance) - self.refresh_provider_fw_rules() - self.basically_filtered = True def apply_instance_filter(self, instance, network_info): """No-op. Everything is done in prepare_instance_filter.""" diff --git a/nova/virt/virtapi.py b/nova/virt/virtapi.py index 424699d4c93c..9b4eddbab04e 100644 --- a/nova/virt/virtapi.py +++ b/nova/virt/virtapi.py @@ -16,12 +16,6 @@ import contextlib class VirtAPI(object): - def provider_fw_rule_get_all(self, context): - """Get the provider firewall rules - :param context: security context - """ - raise NotImplementedError() - @contextlib.contextmanager def wait_for_instance_event(self, instance, event_names, deadline=300, error_callback=None): diff --git a/nova/virt/xenapi/driver.py b/nova/virt/xenapi/driver.py index 18295ca512e4..dc2d17665c95 100644 --- a/nova/virt/xenapi/driver.py +++ b/nova/virt/xenapi/driver.py @@ -628,9 +628,6 @@ class XenAPIDriver(driver.ComputeDriver): """ return self._vmops.refresh_instance_security_rules(instance) - def refresh_provider_fw_rules(self): - return self._vmops.refresh_provider_fw_rules() - def get_available_nodes(self, refresh=False): stats = self.host_state.get_host_stats(refresh=refresh) return [stats["hypervisor_hostname"]] diff --git a/nova/virt/xenapi/firewall.py b/nova/virt/xenapi/firewall.py index 27fb168a7f5a..143d02932718 100644 --- a/nova/virt/xenapi/firewall.py +++ b/nova/virt/xenapi/firewall.py @@ -15,14 +15,9 @@ # License for the specific language governing permissions and limitations # under the License. -from oslo_log import log as logging from oslo_serialization import jsonutils -from nova import context from nova.virt import firewall -from nova.virt import netutils - -LOG = logging.getLogger(__name__) class Dom0IptablesFirewallDriver(firewall.IptablesFirewallDriver): @@ -41,9 +36,9 @@ class Dom0IptablesFirewallDriver(firewall.IptablesFirewallDriver): json_ret = jsonutils.loads(ret) return (json_ret['out'], json_ret['err']) - def __init__(self, virtapi, xenapi_session=None, **kwargs): + def __init__(self, xenapi_session=None, **kwargs): from nova.network import linux_net - super(Dom0IptablesFirewallDriver, self).__init__(virtapi, **kwargs) + super(Dom0IptablesFirewallDriver, self).__init__(**kwargs) self._session = xenapi_session # Create IpTablesManager with executor through plugin self.iptables = linux_net.IptablesManager(self._plugin_execute) @@ -59,56 +54,3 @@ class Dom0IptablesFirewallDriver(firewall.IptablesFirewallDriver): # No multiport needed for XS! return ['--dport', '%s:%s' % (rule.from_port, rule.to_port)] - - def _provider_rules(self): - """Generate a list of rules from provider for IP4 & IP6. - - Note: We could not use the common code from virt.firewall because - XS doesn't accept the '-m multiport' option. - """ - - ctxt = context.get_admin_context() - ipv4_rules = [] - ipv6_rules = [] - rules = self._virtapi.provider_fw_rule_get_all(ctxt) - for rule in rules: - LOG.debug('Adding provider rule: %s', rule.cidr) - version = netutils.get_ip_version(rule.cidr) - if version == 4: - fw_rules = ipv4_rules - else: - fw_rules = ipv6_rules - - protocol = rule.protocol - if version == 6 and protocol == 'icmp': - protocol = 'icmpv6' - - args = ['-p', protocol, '-s', rule.cidr] - - if protocol in ['udp', 'tcp']: - if rule.from_port == rule.to_port: - args += ['--dport', '%s' % (rule.from_port,)] - else: - args += ['--dport', '%s:%s' % (rule.from_port, - rule.to_port)] - elif protocol == 'icmp': - icmp_type = rule.from_port - icmp_code = rule.to_port - - if icmp_type == -1: - icmp_type_arg = None - else: - icmp_type_arg = '%s' % icmp_type - if not icmp_code == -1: - icmp_type_arg += '/%s' % icmp_code - - if icmp_type_arg: - if version == 4: - args += ['-m', 'icmp', '--icmp-type', - icmp_type_arg] - elif version == 6: - args += ['-m', 'icmp6', '--icmpv6-type', - icmp_type_arg] - args += ['-j DROP'] - fw_rules += [' '.join(args)] - return ipv4_rules, ipv6_rules diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py index 5cd6944b4d31..6fa7c5f1ba0b 100644 --- a/nova/virt/xenapi/vmops.py +++ b/nova/virt/xenapi/vmops.py @@ -157,7 +157,6 @@ class VMOps(object): self._volumeops = volumeops.VolumeOps(self._session) self.firewall_driver = firewall.load_driver( DEFAULT_FIREWALL_DRIVER, - self._virtapi, xenapi_session=self._session) vif_impl = importutils.import_class(CONF.xenserver.vif_driver) self.vif_driver = vif_impl(xenapi_session=self._session) @@ -2058,9 +2057,6 @@ class VMOps(object): """recreates security group rules for specified instance.""" self.firewall_driver.refresh_instance_security_rules(instance) - def refresh_provider_fw_rules(self): - self.firewall_driver.refresh_provider_fw_rules() - def unfilter_instance(self, instance_ref, network_info): """Removes filters for each VIF of the specified instance.""" self.firewall_driver.unfilter_instance(instance_ref,