Fix _validate_mac_address method

The method _validate_mac_address previously used: netaddr.EUI(data)
which would convert data passed in to the correct mac_address representation.
For example if data=123 it would return: EUI('00-00-00-00-00-7B'). To fix
this issue I used netaddr.valid_mac() instead which returns a bool if a mac is
valid or not and does not raise. Note this file needs some improvements to
improve how exception handling is done that will come later.

Change-Id: I4fb868e40abbad9e743028cc768f47ca9b3e0e70
Closes-bug: 1291163

diff --git a/neutron/api/v2/attributes.py b/neutron/api/v2/attributes.py
index 1b90bb73ad71cc107b8a26aacb7ae9505a198ad4..5be3d16f9284ab3892c5577eeef735eb12bcbcc8 100644 (file)
--- a/neutron/api/v2/attributes.py
+++ b/neutron/api/v2/attributes.py
@@ -153,12 +153,19 @@ def _validate_no_whitespace(data):
 
 
 def _validate_mac_address(data, valid_values=None):
+    valid_mac = False
     try:
-        netaddr.EUI(_validate_no_whitespace(data))
+        valid_mac = netaddr.valid_mac(_validate_no_whitespace(data))
     except Exception:
-        msg = _("'%s' is not a valid MAC address") % data
-        LOG.debug(msg)
-        return msg
+        pass
+    finally:
+        # TODO(arosen): The code in this file should be refactored
+        # so it catches the correct exceptions. _validate_no_whitespace
+        # raises AttributeError if data is None.
+        if valid_mac is False:
+            msg = _("'%s' is not a valid MAC address") % data
+            LOG.debug(msg)
+            return msg
 
 
 def _validate_mac_address_or_none(data, valid_values=None):

diff --git a/neutron/tests/unit/test_attributes.py b/neutron/tests/unit/test_attributes.py
index b4a44a6eda68a5d6f05f72743bbd04ca67566448..bab45fde021f6432c8e01e7d2fdf99906fe48742 100644 (file)
--- a/neutron/tests/unit/test_attributes.py
+++ b/neutron/tests/unit/test_attributes.py
@@ -176,14 +176,19 @@ class TestAttributes(base.BaseTestCase):
 
         mac_addr = "ffa:16:3e:4f:00:00"
         msg = validator(mac_addr)
-        self.assertEqual(msg, "'%s' is not a valid MAC address" % mac_addr)
+        err_msg = "'%s' is not a valid MAC address"
+        self.assertEqual(msg, err_msg % mac_addr)
+
+        mac_addr = "123"
+        msg = validator(mac_addr)
+        self.assertEqual(msg, err_msg % mac_addr)
 
         mac_addr = None
         msg = validator(mac_addr)
         if allow_none:
             self.assertIsNone(msg)
         else:
-            self.assertEqual(msg, "'None' is not a valid MAC address")
+            self.assertEqual(msg, err_msg % mac_addr)
 
     def test_validate_mac_address(self):
         self._test_validate_mac_address(attributes._validate_mac_address)