Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support specifying mac address without quotation mark #364

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion module_utils/network_lsr/argument_validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
from ansible.module_utils.network_lsr.utils import Util # noqa:E501

UINT32_MAX = 0xFFFFFFFF
# a MAC address for type ethernet requires 6 octets
# a MAC address for type infiniband requires 20 octets
MAC_ADDR_OCTETS = [6, 20]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wrong.

The kernel has no limitation minimum MAC address length.
And the maximum is not 20, but in /usr/include/linux/netdevice.h:

#define MAX_ADDR_LEN 32 /* Largest hardware address length */

That is 32 octets and 64 hex string characters.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason that I define MAC_ADDR_OCTETS = [6, 20] is because I saw the validation check here,
https://github.com/linux-system-roles/network/blob/main/module_utils/network_lsr/argument_validator.py#L1483

And in NM,it was also clear indicated,

     * ---ifcfg-rh---
     * property: mac-address
     * variable: HWADDR
     * description: IBoIP 20-byte hardware address of the device (in traditional
     *    hex-digits-and-colons notation).
     *    Note that for initscripts this is the current MAC address of the device as found
     *    during ifup. For NetworkManager this is the permanent MAC address. Or in case no
     *    permanent MAC address exists, the MAC address initially configured on the device.
     * example: HWADDR=01:02:03:04:05:06:07:08:09:0A:01:02:03:04:05:06:07:08:09:11
     * ---end---
     
     /* ---keyfile---
     * property: mac-address
     * format: usual hex-digits-and-colons notation
     * description: MAC address in traditional hex-digits-and-colons notation
     *   (e.g. 00:22:68:12:79:A2), or semicolon separated list of 6 bytes (obsolete)
     *   (e.g. 0;34;104;18;121;162)
     * ---end---



class ArgUtil:
Expand Down Expand Up @@ -416,11 +419,25 @@ def _validate_impl(self, value, name):


class ArgValidatorMac(ArgValidatorStr):
def __init__(self, name, force_len=None, required=False, default_value=None):
def __init__(
self, name, force_len=MAC_ADDR_OCTETS, required=False, default_value=None
):
ArgValidatorStr.__init__(self, name, required, default_value, None)
self.force_len = force_len

def _validate_impl(self, value, name):
if type(value) == int:
try:
value = Util.num_to_mac(value, self.force_len)
except MyError:
raise ValidationError(
name, "value '%s' is not a valid MAC address" % (value)
)
if not isinstance(value, Util.STRING_TYPE):
raise ValidationError(
name,
"must be a string and value should be quoted, but is '%s', " % (value),
)
v = ArgValidatorStr._validate_impl(self, value, name)
try:
addr = Util.mac_aton(v, self.force_len)
Expand Down
20 changes: 19 additions & 1 deletion module_utils/network_lsr/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ def mac_aton(mac_str, force_len=None):
if i == 1:
raise MyError("not a valid MAC address: '%s'" % (mac_str))
if force_len is not None:
if force_len != len(b):
if not any(a for a in force_len if a == len(b)):
raise MyError(
"not a valid MAC address of length %s: '%s'" % (force_len, mac_str)
)
Expand All @@ -272,6 +272,24 @@ def mac_ntoa(mac):
def mac_norm(mac_str, force_len=None):
return Util.mac_ntoa(Util.mac_aton(mac_str, force_len))

@staticmethod
def num_to_mac(num, force_len=None):
mac = []
s_num = num
while num:
num, mod = divmod(num, 60)
if mod < 10:
mac.insert(0, "0" + str(mod))
else:
mac.insert(0, str(mod))
mac_addr = ":".join(mac)
if force_len is not None:
if not any(a for a in force_len if a == len(mac)):
raise MyError(
"not a valid number '%d' for MAC address: '%s'" % (s_num, mac_addr)
)
return mac_addr

@staticmethod
def boolean(arg):
if arg is None or isinstance(arg, bool):
Expand Down
35 changes: 35 additions & 0 deletions tests/unit/test_network_connections.py
Original file line number Diff line number Diff line change
Expand Up @@ -3361,6 +3361,41 @@ def test_set_deprecated_slave_type(self):
self.assertTrue("port_type" in connection)
self.assertTrue("slave_type" not in connection)

def test_mac_address_argvalidator(self):
liangwen12year marked this conversation as resolved.
Show resolved Hide resolved
"""
Test that argvalidator for validating mac address is correctly defined.
"""
validator = network_lsr.argument_validator.ArgValidatorMac("mac")
self.assertEqual(validator.validate(41135085296), "52:54:00:12:34:56")
self.assertEqual(validator.validate("00:00:5e:00:53:5d"), "00:00:5e:00:53:5d")
self.assertEqual(validator.validate("00:00:00:00:00:00"), "00:00:00:00:00:00")
self.assertEqual(validator.validate("ff:ff:ff:ff:ff:ff"), "ff:ff:ff:ff:ff:ff")
self.assertEqual(
validator.validate(
"80:00:00:6d:fe:80:00:00:00:00:00:00:00:02:55:00:70:33:cf:01"
),
"80:00:00:6d:fe:80:00:00:00:00:00:00:00:02:55:00:70:33:cf:01",
)
self.assertEqual(
validator.validate(
"01:02:03:04:05:06:07:08:09:0A:01:02:03:04:05:06:07:08:09:11"
),
"01:02:03:04:05:06:07:08:09:0a:01:02:03:04:05:06:07:08:09:11",
)
self.assertValidationError(validator, 1234)
self.assertValidationError(validator, "aa:bb")
self.assertValidationError(validator, sys.maxsize)
self.assertValidationError(validator, 0)
self.assertValidationError(
validator, "80:00:00:6d:fe:80:00:00:00:00:00:00:00:02:55:00:70:33:cf:"
)
self.assertValidationError(
validator, "80:00:00:6d:fe:80:00:00:00:00:00:00:00:02:55:00:70:33:cf:xx"
)
self.assertValidationError(
validator, "80:00:00:6d:fe:80:00:00:00:00:00:00:00:02:55:000:70:33:cf:01"
)


@my_test_skipIf(nmutil is None, "no support for NM (libnm via pygobject)")
class TestNM(unittest.TestCase):
Expand Down