Fix handling of attributes. Fix tests

src/netns/interface.py

@@ -53,6 +53,9 @@ class _NSInterface(_Interface):
 
     def __setattr__(self, name, value):
         if (name not in interface.changeable_attributes):
+            if name[0] != '_': # forbid anything that doesn't start with a _
+                raise AttributeError("'%s' object has no attribute '%s'" %
+                        (self.__class__.__name__, name))
             super(_Interface, self).__setattr__(name, value)
             return
         iface = interface(index = self._ns_if)
@@ -164,10 +167,36 @@ def _any_to_bool(any):
         return any != ""
     return bool(any)
 
+def _positive(val):
+    v = int(val)
+    if v <= 0:
+        raise ValueError("Invalid value: %d" % v)
+    return v
+
+def _fix_lladdr(addr):
+    foo = addr.lower()
+    if ':' in addr:
+        # Verify sanity and split
+        m = re.search('^' + ':'.join(['([0-9a-f]{1,2})'] * 6) + '$', foo)
+        if m is None:
+            raise ValueError("Invalid address: `%s'." % addr)
+        # Fill missing zeros and glue again
+        return ':'.join(('0' * (2 - len(x)) + x for x in m.groups()))
+
+    # Fill missing zeros
+    foo = '0' * (12 - len(foo)) + foo
+    # Verify sanity and split
+    m = re.search('^' + '([0-9a-f]{2})' * 6 + '$', foo)
+    if m is None:
+        raise ValueError("Invalid address: `%s'." % addr)
+    # Glue
+    return ":".join(m.groups())
+
 def _make_getter(attr, conv = lambda x: x):
     def getter(self):
         return conv(getattr(self, attr))
     return getter
+
 def _make_setter(attr, conv = lambda x: x):
     def setter(self, value):
         if value == None:
@@ -201,15 +230,18 @@ class interface(object):
     changeable_attributes = ["name", "mtu", "lladdr", "broadcast", "up",
             "multicast", "arp"]
 
-    index = property(_make_getter("_index"), _make_setter("_index", int))
+    # Index should be read-only
+    index = property(_make_getter("_index"))
     up = property(_make_getter("_up"), _make_setter("_up", _any_to_bool))
-    mtu = property(_make_getter("_mtu"), _make_setter("_mtu", int))
+    mtu = property(_make_getter("_mtu"), _make_setter("_mtu", _positive))
+    lladdr = property(_make_getter("_lladdr"),
+            _make_setter("_lladdr", _fix_lladdr))
     arp = property(_make_getter("_arp"), _make_setter("_arp", _any_to_bool))
     multicast = property(_make_getter("_mc"), _make_setter("_mc", _any_to_bool))
 
     def __init__(self, index = None, name = None, up = None, mtu = None,
             lladdr = None, broadcast = None, multicast = None, arp = None):
-        self.index = index
+        self._index = _positive(index) if index is not None else None
         self.name = name
         self.up = up
         self.mtu = mtu

t/test_interfaces.py

@@ -37,20 +37,33 @@ class TestInterfaces(unittest.TestCase):
             peer_name = netns.iproute.get_if(ifaces[i].control_index).name
             self.assertTrue(peer_name in devs)
 
-        self.assertEquals(set(ifaces), node0.get_interfaces())
+        self.assertEquals(set(ifaces), set(node0.get_interfaces()))
 
     @test_util.skipUnless(os.getuid() == 0, "Test requires root privileges")
     def test_interface_settings(self):
         node0 = netns.Node()
         if0 = node0.add_if(lladdr = '42:71:e0:90:ca:42', mtu = 1492)
-        self.assertEquals(if0.lladdr, '42:71:e0:90:ca:42')
+        self.assertEquals(if0.lladdr, '42:71:e0:90:ca:42',
+                "Constructor parameters")
+        self.assertEquals(if0.mtu, 1492, "Constructor parameters")
         if0.lladdr = '4271E090CA42'
-        self.assertEquals(if0.lladdr, '42:71:e0:90:ca:42')
-        self.assertRaises(BaseException, setattr, if0, 'lladdr', 'foo')
-        self.assertRaises(BaseException, setattr, if0, 'lladdr', '12345678901')
+        self.assertEquals(if0.lladdr, '42:71:e0:90:ca:42', """Normalization of
+                link-level address: missing colons and upper caps""")
+        if0.lladdr = '2:71:E0:90:CA:42'
+        self.assertEquals(if0.lladdr, '02:71:e0:90:ca:42',
+                """Normalization of link-level address: missing zeroes""")
+        if0.lladdr = '271E090CA42'
+        self.assertEquals(if0.lladdr, '02:71:e0:90:ca:42',
+                """Automatic normalization of link-level address: missing
+                colons and zeroes""")
+        self.assertRaises(ValueError, setattr, if0, 'lladdr', 'foo')
+        self.assertRaises(ValueError, setattr, if0, 'lladdr', '1234567890123')
         self.assertEquals(if0.mtu, 1492)
-        self.assertRaises(BaseException, setattr, if0, 'mtu', 0)
-        self.assertRaises(BaseException, setattr, if0, 'mtu', 65537)
+        # detected by setter
+        self.assertRaises(ValueError, setattr, if0, 'mtu', 0)
+        # error from ip
+        self.assertRaises(RuntimeError, setattr, if0, 'mtu', 1)
+        self.assertRaises(RuntimeError, setattr, if0, 'mtu', 65537)
 
         devs = get_devs_netns(node0)
         self.assertTrue(if0.name in devs)
@@ -58,12 +71,13 @@ class TestInterfaces(unittest.TestCase):
         self.assertEquals(devs[if0.name]['lladdr'], if0.lladdr)
         self.assertEquals(devs[if0.name]['mtu'], if0.mtu)
 
-        if0.enable = True
+        if0.up = True
         devs = get_devs_netns(node0)
         self.assertTrue(devs[if0.name]['up'])
 
         # Verify that data is actually read from the kernel
-        node0.run_process(["ip", "link", "set", if0.name, "mtu", "1500"])
+        r = node0.system(["ip", "link", "set", if0.name, "mtu", "1500"])
+        self.assertEquals(r, 0)
         devs = get_devs_netns(node0)
         self.assertEquals(devs[if0.name]['mtu'], 1500)
         self.assertEquals(devs[if0.name]['mtu'], if0.mtu)
@@ -100,7 +114,6 @@ class TestInterfaces(unittest.TestCase):
                 prefix_len = 64)
 
         devs = get_devs_netns(node0)
-        print devs
         self.assertTrue( {
             'addr': '10.0.0.1', 'plen': 24,
             'bcast': '10.0.0.255', 'family': 'inet'

t/test_util.py

@@ -18,7 +18,7 @@ def process_ipcmd(str):
             out[cur] = {
                     'idx':      match.group(1),
                     'flags':    match.group(3).split(","),
-                    'mtu':      match.group(4),
+                    'mtu':      int(match.group(4)),
                     'qdisc':    match.group(5),
                     'addr':     []
                     }