sts-sponsors team mailing list archive

[Alberto Donato <mp+438041@xxxxxxxxxxxxxxxxxx>]

Alberto Donato has proposed merging ~ack/maas:macaddress-field-rework into maas:master.

Commit message:
rework MACAddresField to use text as storage type


Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~ack/maas/+git/maas/+merge/438041
-- 
Your team MAAS Committers is subscribed to branch maas:master.

diff --git a/src/maasserver/fields.py b/src/maasserver/fields.py
index 276d4d4..4270f3f 100644
--- a/src/maasserver/fields.py
+++ b/src/maasserver/fields.py
@@ -3,6 +3,7 @@
 
 """Custom model and form fields."""
 
+from itertools import chain
 import re
 
 from django import forms
@@ -15,6 +16,7 @@ from django.db.models import (
     GenericIPAddressField,
     IntegerField,
     Q,
+    TextField,
     URLField,
 )
 from django.utils.deconstruct import deconstructible
@@ -39,6 +41,21 @@ MAC_RE = re.compile(
 MAC_ERROR_MSG = "'%(value)s' is not a valid MAC address."
 MAC_VALIDATOR = RegexValidator(regex=MAC_RE, message=MAC_ERROR_MSG)
 
+MAC_SPLIT_RE = re.compile(r"[-:.]")
+
+
+def normalise_macaddress(mac: str) -> str:
+    """Return a colon-separated format for the specified MAC.
+
+    This supportes converting from input formats matching the MAC_RE regexp.
+    """
+    tokens = MAC_SPLIT_RE.split(mac.lower())
+    if len(tokens) == 3:  # each token is two bytes
+        tokens = chain(*(re.findall("..", token.zfill(4)) for token in tokens))
+    else:  # single-byte tokens
+        tokens = (token.zfill(2) for token in tokens)
+    return ":".join(tokens)
+
 
 class MACAddressFormField(forms.CharField):
     """Form field type: MAC address."""
@@ -50,19 +67,17 @@ class MACAddressFormField(forms.CharField):
         )
 
 
-class MACAddressField(Field):
+class MACAddressField(TextField):
     """Model field type: MAC address."""
 
     description = "MAC address"
 
     default_validators = [MAC_VALIDATOR]
 
-    def db_type(self, *args, **kwargs):
-        return "macaddr"
-
-    def get_prep_value(self, value):
-        # Convert empty string to None.
-        return super().get_prep_value(value) or None
+    def to_python(self, value):
+        if not value:
+            return value
+        return normalise_macaddress(value)
 
 
 class XMLField(Field):
diff --git a/src/maasserver/migrations/maasserver/0295_macaddress_text_field.py b/src/maasserver/migrations/maasserver/0295_macaddress_text_field.py
new file mode 100644
index 0000000..071b63a
--- /dev/null
+++ b/src/maasserver/migrations/maasserver/0295_macaddress_text_field.py
@@ -0,0 +1,16 @@
+# Generated by Django 3.2.12 on 2023-02-28 16:03
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("maasserver", "0294_keyring_data_binary_field"),
+    ]
+
+    operations = [
+        migrations.RunSQL(
+            f"ALTER TABLE maasserver_{table} ALTER COLUMN mac_address TYPE TEXT"
+        )
+        for table in ("interface", "neighbour", "virtualmachineinterface")
+    ]
diff --git a/src/maasserver/pytest_tests/test_fields.py b/src/maasserver/pytest_tests/test_fields.py
new file mode 100644
index 0000000..d818248
--- /dev/null
+++ b/src/maasserver/pytest_tests/test_fields.py
@@ -0,0 +1,18 @@
+import pytest
+
+from maasserver.fields import normalise_macaddress
+
+
+@pytest.mark.parametrize(
+    ("value", "expected"),
+    [
+        ("aa:bb:cc:dd:ee:ff", "aa:bb:cc:dd:ee:ff"),
+        ("aa:b:cc:d:ee:f", "aa:0b:cc:0d:ee:0f"),
+        ("aa-bb-cc-dd-ee-ff", "aa:bb:cc:dd:ee:ff"),
+        ("aa-b-cc-d-ee-f", "aa:0b:cc:0d:ee:0f"),
+        ("aabb.ccdd.eeff", "aa:bb:cc:dd:ee:ff"),
+        ("abb.cdd.eeff", "0a:bb:0c:dd:ee:ff"),
+    ],
+)
+def test_normalise_mac_address(value, expected):
+    assert normalise_macaddress(value) == expected
diff --git a/src/maasserver/rpc/tests/test_rackcontrollers.py b/src/maasserver/rpc/tests/test_rackcontrollers.py
index 88b0591..05de7b4 100644
--- a/src/maasserver/rpc/tests/test_rackcontrollers.py
+++ b/src/maasserver/rpc/tests/test_rackcontrollers.py
@@ -212,7 +212,7 @@ class TestRegisterRackController(MAASServerTestCase):
 
     def test_creates_new_rackcontroller(self):
         existing_machine = factory.make_Machine()
-        rack_mac = (factory.make_mac_address(),)
+        rack_mac = factory.make_mac_address()
         interfaces = {
             factory.make_name("eth0"): {
                 "type": "physical",
diff --git a/src/maasserver/tests/test_fields.py b/src/maasserver/tests/test_fields.py
index 24014da..99cc72d 100644
--- a/src/maasserver/tests/test_fields.py
+++ b/src/maasserver/tests/test_fields.py
@@ -64,7 +64,7 @@ class TestModelNameValidator(MAASServerTestCase):
 class TestMACAddressField(MAASServerTestCase):
     def test_mac_address_is_stored_normalized_and_loaded(self):
         interface = factory.make_Interface(
-            INTERFACE_TYPE.PHYSICAL, mac_address=" AA-bb-CC-dd-EE-Ff "
+            INTERFACE_TYPE.PHYSICAL, mac_address="AA-bb-CC-dd-EE-Ff"
         )
         loaded_mac = Interface.objects.get(id=interface.id)
         self.assertEqual("aa:bb:cc:dd:ee:ff", loaded_mac.mac_address)