Merge "Refactor nwfilter parameters"

4 files changed, 117 insertions, 62 deletions

diff --git a/nova/tests/test_libvirt.py b/nova/tests/test_libvirt.py
index 6882bb082..981cb20bd 100644
--- a/nova/tests/test_libvirt.py
+++ b/nova/tests/test_libvirt.py
@@ -63,6 +63,7 @@ from nova.virt.libvirt import driver as libvirt_driver
 from nova.virt.libvirt import firewall
 from nova.virt.libvirt import imagebackend
 from nova.virt.libvirt import utils as libvirt_utils
+from nova.virt import netutils
 
 try:
     import libvirt
@@ -1667,10 +1668,7 @@ class LibvirtConnTestCase(test.TestCase):
         tree = etree.fromstring(xml)
         interfaces = tree.findall("./devices/interface")
         self.assertEquals(len(interfaces), 2)
-        parameters = interfaces[0].findall('./filterref/parameter')
         self.assertEquals(interfaces[0].get('type'), 'bridge')
-        self.assertEquals(parameters[0].get('name'), 'IP')
-        self.assertTrue(_ipv4_like(parameters[0].get('value'), '192.168'))
 
     def _check_xml_and_container(self, instance):
         user_context = context.RequestContext(self.user_id,
@@ -1947,12 +1945,8 @@ class LibvirtConnTestCase(test.TestCase):
                         'type'), 'pty')
                 check_list.append(check)
 
-        parameter = './devices/interface/filterref/parameter'
         common_checks = [
             (lambda t: t.find('.').tag, 'domain'),
-            (lambda t: t.find(parameter).get('name'), 'IP'),
-            (lambda t: _ipv4_like(t.find(parameter).get('value'), '192.168'),
-             True),
             (lambda t: t.find('./memory').text, '2097152')]
         if rescue:
             common_checks += [
@@ -1993,6 +1987,14 @@ class LibvirtConnTestCase(test.TestCase):
                                  '%s != %s failed common check %d' %
                                  (check(tree), expected_result, i))
 
+            filterref = './devices/interface/filterref'
+            (network, mapping) = network_info[0]
+            nic_id = mapping['mac'].replace(':', '')
+            fw = firewall.NWFilterFirewall(fake.FakeVirtAPI(), conn)
+            instance_filter_name = fw._instance_filter_name(instance_ref,
+                                                            nic_id)
+            self.assertEqual(tree.find(filterref).get('filter'),
+                             instance_filter_name)
         # This test is supposed to make sure we don't
         # override a specifically set uri
         #
@@ -3517,9 +3519,10 @@ class NWFilterFakes:
 
     def filterDefineXMLMock(self, xml):
         class FakeNWFilterInternal:
-            def __init__(self, parent, name):
+            def __init__(self, parent, name, xml):
                 self.name = name
                 self.parent = parent
+                self.xml = xml
 
             def undefine(self):
                 del self.parent.filters[self.name]
@@ -3527,7 +3530,7 @@ class NWFilterFakes:
         tree = etree.fromstring(xml)
         name = tree.get('name')
         if name not in self.filters:
-            self.filters[name] = FakeNWFilterInternal(self, name)
+            self.filters[name] = FakeNWFilterInternal(self, name, xml)
         return True
 
 
@@ -4050,6 +4053,67 @@ class NWFilterTestCase(test.TestCase):
 
         db.instance_destroy(admin_ctxt, instance_ref['uuid'])
 
+    def test_nwfilter_parameters(self):
+        admin_ctxt = context.get_admin_context()
+
+        fakefilter = NWFilterFakes()
+        self.fw._conn.nwfilterDefineXML = fakefilter.filterDefineXMLMock
+        self.fw._conn.nwfilterLookupByName = fakefilter.nwfilterLookupByName
+
+        instance_ref = self._create_instance()
+        inst_id = instance_ref['id']
+        inst_uuid = instance_ref['uuid']
+
+        self.security_group = self.setup_and_return_security_group()
+
+        db.instance_add_security_group(self.context, inst_uuid,
+                                       self.security_group['id'])
+
+        instance = db.instance_get(self.context, inst_id)
+
+        network_info = _fake_network_info(self.stubs, 1)
+        self.fw.setup_basic_filtering(instance, network_info)
+
+        (network, mapping) = network_info[0]
+        nic_id = mapping['mac'].replace(':', '')
+        instance_filter_name = self.fw._instance_filter_name(instance, nic_id)
+        f = fakefilter.nwfilterLookupByName(instance_filter_name)
+        tree = etree.fromstring(f.xml)
+
+        for fref in tree.findall('filterref'):
+            parameters = fref.findall('./parameter')
+            for parameter in parameters:
+                if parameter.get('name') == 'IP':
+                    self.assertTrue(_ipv4_like(parameter.get('value'),
+                                                             '192.168'))
+                elif parameter.get('name') == 'DHCPSERVER':
+                    dhcp_server = mapping['dhcp_server']
+                    self.assertEqual(parameter.get('value'), dhcp_server)
+                elif parameter.get('name') == 'RASERVER':
+                    ra_server = mapping.get('gateway_v6') + "/128"
+                    self.assertEqual(parameter.get('value'), ra_server)
+                elif parameter.get('name') == 'PROJNET':
+                    ipv4_cidr = network['cidr']
+                    net, mask = netutils.get_net_and_mask(ipv4_cidr)
+                    self.assertEqual(parameter.get('value'), net)
+                elif parameter.get('name') == 'PROJMASK':
+                    ipv4_cidr = network['cidr']
+                    net, mask = netutils.get_net_and_mask(ipv4_cidr)
+                    self.assertEqual(parameter.get('value'), mask)
+                elif parameter.get('name') == 'PROJNET6':
+                    ipv6_cidr = network['cidr_v6']
+                    net, prefix = netutils.get_net_and_prefixlen(ipv6_cidr)
+                    self.assertEqual(parameter.get('value'), net)
+                elif parameter.get('name') == 'PROJMASK6':
+                    ipv6_cidr = network['cidr_v6']
+                    net, prefix = netutils.get_net_and_prefixlen(ipv6_cidr)
+                    self.assertEqual(parameter.get('value'), prefix)
+                else:
+                    raise exception.InvalidParameterValue('unknown parameter '
+                                                          'in filter')
+
+        db.instance_destroy(admin_ctxt, instance_ref['uuid'])
+
 
 class LibvirtUtilsTestCase(test.TestCase):
     def test_get_iscsi_initiator(self):
diff --git a/nova/virt/libvirt/designer.py b/nova/virt/libvirt/designer.py
index 3ccf1b8c6..0625d407b 100644
--- a/nova/virt/libvirt/designer.py
+++ b/nova/virt/libvirt/designer.py
@@ -21,8 +21,6 @@ This module provides helper APIs for populating the config.py
 classes based on common operational needs / policies
 """
 
-from nova.virt import netutils
-
 
 def set_vif_guest_frontend_config(conf, mac, model, driver):
     """Populate a LibvirtConfigGuestInterface instance
@@ -102,37 +100,6 @@ def set_vif_host_backend_802qbh_config(conf, devname, profileid,
         conf.target_dev = tapname
 
 
-def set_vif_host_backend_filter_config(conf, name,
-                                       primary_addr,
-                                       dhcp_server=None,
-                                       ra_server=None,
-                                       allow_same_net=False,
-                                       ipv4_cidr=None,
-                                       ipv6_cidr=None):
-    """Populate a LibvirtConfigGuestInterface instance
-    with host backend details for traffic filtering"""
-
-    conf.filtername = name
-    conf.add_filter_param("IP", primary_addr)
-
-    if dhcp_server:
-        conf.add_filter_param("DHCPSERVER", dhcp_server)
-
-    if ra_server:
-        conf.add_filter_param("RASERVER", ra_server)
-
-    if allow_same_net:
-        if ipv4_cidr:
-            net, mask = netutils.get_net_and_mask(ipv4_cidr)
-            conf.add_filter_param("PROJNET", net)
-            conf.add_filter_param("PROJMASK", mask)
-
-        if ipv6_cidr:
-            net, prefix = netutils.get_net_and_prefixlen(ipv6_cidr)
-            conf.add_filter_param("PROJNET6", net)
-            conf.add_filter_param("PROJMASK6", prefix)
-
-
 def set_vif_bandwidth_config(conf, extra_specs):
     """Config vif inbound/outbound bandwidth limit."""
 
diff --git a/nova/virt/libvirt/firewall.py b/nova/virt/libvirt/firewall.py
index 054ec4c75..90155baf3 100644
--- a/nova/virt/libvirt/firewall.py
+++ b/nova/virt/libvirt/firewall.py
@@ -23,6 +23,7 @@ from oslo.config import cfg
 from nova.cloudpipe import pipelib
 from nova.openstack.common import log as logging
 import nova.virt.firewall as base_firewall
+from nova.virt import netutils
 
 LOG = logging.getLogger(__name__)
 CONF = cfg.CONF
@@ -123,10 +124,48 @@ class NWFilterFirewall(base_firewall.FirewallDriver):
         base_filter = self.get_base_filter_list(instance, allow_dhcp)
 
         for (network, mapping) in network_info:
-            nic_id = mapping['mac'].replace(':', '')
-            instance_filter_name = self._instance_filter_name(instance, nic_id)
-            self._define_filter(self._filter_container(instance_filter_name,
-                                                       base_filter))
+            self._define_filter(self._get_instance_filter_xml(instance,
+                                                              base_filter,
+                                                              network,
+                                                              mapping))
+
+    def _get_instance_filter_parameters(self, network, mapping):
+        parameters = []
+
+        def format_parameter(parameter, value):
+            return ("<parameter name='%s' value='%s'/>" % (parameter, value))
+
+        for address in mapping['ips']:
+            parameters.append(format_parameter('IP', address['ip']))
+        if mapping['dhcp_server']:
+            parameters.append(format_parameter('DHCPSERVER',
+                                               mapping['dhcp_server']))
+        if CONF.use_ipv6:
+            ra_server = mapping.get('gateway_v6') + "/128"
+            parameters.append(format_parameter('RASERVER', ra_server))
+        if CONF.allow_same_net_traffic:
+            ipv4_cidr = network['cidr']
+            net, mask = netutils.get_net_and_mask(ipv4_cidr)
+            parameters.append(format_parameter('PROJNET', net))
+            parameters.append(format_parameter('PROJMASK', mask))
+            if CONF.use_ipv6:
+                ipv6_cidr = network['cidr_v6']
+                net, prefix = netutils.get_net_and_prefixlen(ipv6_cidr)
+                parameters.append(format_parameter('PROJNET6', net))
+                parameters.append(format_parameter('PROJMASK6', prefix))
+        return parameters
+
+    def _get_instance_filter_xml(self, instance, filters, network, mapping):
+        nic_id = mapping['mac'].replace(':', '')
+        instance_filter_name = self._instance_filter_name(instance, nic_id)
+        parameters = self._get_instance_filter_parameters(network, mapping)
+        xml = '''<filter name='%s' chain='root'>''' % instance_filter_name
+        for f in filters:
+            xml += '''<filterref filter='%s'>''' % f
+            xml += ''.join(parameters)
+            xml += '</filterref>'
+        xml += '</filter>'
+        return xml
 
     def get_base_filter_list(self, instance, allow_dhcp):
         """
diff --git a/nova/virt/libvirt/vif.py b/nova/virt/libvirt/vif.py
index 4be8be33f..523857e42 100644
--- a/nova/virt/libvirt/vif.py
+++ b/nova/virt/libvirt/vif.py
@@ -172,22 +172,7 @@ class LibvirtGenericVIFDriver(LibvirtBaseVIFDriver):
 
         mac_id = mapping['mac'].replace(':', '')
         name = "nova-instance-" + instance['name'] + "-" + mac_id
-        primary_addr = mapping['ips'][0]['ip']
-        dhcp_server = ra_server = ipv4_cidr = ipv6_cidr = None
-
-        if mapping['dhcp_server']:
-            dhcp_server = mapping['dhcp_server']
-        if CONF.use_ipv6:
-            ra_server = mapping.get('gateway_v6') + "/128"
-        if CONF.allow_same_net_traffic:
-            ipv4_cidr = network['cidr']
-            if CONF.use_ipv6:
-                ipv6_cidr = network['cidr_v6']
-
-        if self.get_firewall_required():
-            designer.set_vif_host_backend_filter_config(
-                conf, name, primary_addr, dhcp_server,
-                ra_server, ipv4_cidr, ipv6_cidr)
+        conf.filtername = name
         designer.set_vif_bandwidth_config(conf, instance)
 
         return conf