Alberto Donato has proposed merging ~ack/maas:macaddress-field-as-text into
maas:master.
Commit message:
use text db field for MACs, normalise values in MACAddressFormField
Requested reviews:
MAAS Maintainers (maas-maintainers)
For more details, see:
https://code.launchpad.net/~ack/maas/+git/maas/+merge/438199
--
Your team MAAS Committers is subscribed to branch maas:master.
diff --git a/src/maasserver/api/nodes.py b/src/maasserver/api/nodes.py
index f6d58e6..5754853 100644
--- a/src/maasserver/api/nodes.py
+++ b/src/maasserver/api/nodes.py
@@ -41,7 +41,7 @@ from maasserver.exceptions import (
NoScriptsFound,
StaticIPAddressExhaustion,
)
-from maasserver.fields import MAC_RE
+from maasserver.fields import MAC_RE, normalise_macaddress
from maasserver.forms import BulkNodeSetZoneForm
from maasserver.forms.ephemeral import TestForm
from maasserver.models import Filesystem, Interface, Node, OwnerData
@@ -209,13 +209,17 @@ def filtered_nodes_list_from_request(request, model=None):
def is_registered(request, ignore_statuses=None):
"""Used by both `NodesHandler` and `AnonNodesHandler`."""
- mac_address = get_mandatory_param(request.GET, "mac_address")
- interfaces = Interface.objects.filter(mac_address=mac_address)
- interfaces = interfaces.exclude(node_config__node__isnull=True)
if ignore_statuses is None:
ignore_statuses = [NODE_STATUS.RETIRED]
- interfaces = interfaces.exclude(
- node_config__node__status__in=ignore_statuses
+
+ mac_address = normalise_macaddress(
+ get_mandatory_param(request.GET, "mac_address")
+ )
+
+ interfaces = (
+ Interface.objects.filter(mac_address=mac_address)
+ .exclude(node_config__node__isnull=True)
+ .exclude(node_config__node__status__in=ignore_statuses)
)
return interfaces.exists()
diff --git a/src/maasserver/api/tests/test_nodes.py b/src/maasserver/api/tests/test_nodes.py
index 91922f8..2b252b2 100644
--- a/src/maasserver/api/tests/test_nodes.py
+++ b/src/maasserver/api/tests/test_nodes.py
@@ -63,20 +63,14 @@ class TestIsRegisteredAnonAPI(APITestCase.ForAnonymousAndUserAndAdmin):
def test_is_registered_normalizes_mac_address(self):
node = self.make_node()
- # These two non-normalized MAC addresses are the same.
- non_normalized_mac_address = "AA-bb-cc-dd-ee-ff"
- non_normalized_mac_address2 = "aabbccddeeff"
factory.make_Interface(
INTERFACE_TYPE.PHYSICAL,
- mac_address=non_normalized_mac_address,
+ mac_address="aa:bb:cc:dd:ee:ff",
node=node,
)
response = self.client.get(
reverse("nodes_handler"),
- {
- "op": "is_registered",
- "mac_address": non_normalized_mac_address2,
- },
+ {"op": "is_registered", "mac_address": "aabbccddeeff"},
)
self.assertEqual(
(http.client.OK.value, "true"),
diff --git a/src/maasserver/fields.py b/src/maasserver/fields.py
index 0d014ff..be67650 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
@@ -40,28 +41,38 @@ MAC_VALIDATOR = RegexValidator(
regex=MAC_RE, message="'%(value)s' is not a valid MAC address."
)
+MAC_SPLIT_RE = re.compile(r"[-:.]")
-class MACAddressFormField(forms.CharField):
- """Form field type: MAC address."""
- def __init__(self, *args, **kwargs):
- super().__init__(*args, **kwargs)
- self.validators.append(MAC_VALIDATOR)
+def normalise_macaddress(mac: str) -> str:
+ """Return a colon-separated format for the specified MAC.
+ This supports converting from input formats matching the MAC_RE regexp.
+ """
-class MACAddressField(Field):
- """Model field type: MAC address."""
+ tokens = MAC_SPLIT_RE.split(mac.lower())
+ match len(tokens):
+ case 1: # no separator
+ tokens = re.findall("..", tokens[0])
+ case 3: # each token is two bytes
+ tokens = chain(
+ *(re.findall("..", token.zfill(4)) for token in tokens)
+ )
+ case _: # single-byte tokens
+ tokens = (token.zfill(2) for token in tokens)
+ return ":".join(tokens)
- description = "MAC address"
- default_validators = [MAC_VALIDATOR]
+class MACAddressFormField(forms.CharField):
+ """Form field type: MAC address."""
- def db_type(self, *args, **kwargs):
- return "macaddr"
+ def validate(self, value):
+ if value:
+ MAC_VALIDATOR(value)
- def get_prep_value(self, value):
- # Convert empty string to None.
- return super().get_prep_value(value) or None
+ def clean(self, value):
+ value = super().clean(value)
+ return normalise_macaddress(value)
class XMLField(Field):
diff --git a/src/maasserver/forms/tests/test_machinewithmacaddresses.py b/src/maasserver/forms/tests/test_machinewithmacaddresses.py
index 2fed36d..6af84d5 100644
--- a/src/maasserver/forms/tests/test_machinewithmacaddresses.py
+++ b/src/maasserver/forms/tests/test_machinewithmacaddresses.py
@@ -62,6 +62,22 @@ class TestMachineWithMACAddressesForm(MAASServerTestCase):
)
self.assertEqual(architecture, form.cleaned_data["architecture"])
+ def test_valid_values_normalised(self):
+ architecture = make_usable_architecture(self)
+ form = MachineWithMACAddressesForm(
+ data=self.make_params(
+ mac_addresses=["aabbccddeeff", "9a-bb-c3-33-e5-7f"],
+ architecture=architecture,
+ )
+ )
+
+ self.assertTrue(form.is_valid(), form.errors)
+ self.assertEqual(
+ ["aa:bb:cc:dd:ee:ff", "9a:bb:c3:33:e5:7f"],
+ form.cleaned_data["mac_addresses"],
+ )
+ self.assertEqual(architecture, form.cleaned_data["architecture"])
+
def test_simple_invalid(self):
# If the form only has one (invalid) MAC address field to validate,
# the error message in form.errors['mac_addresses'] is the
diff --git a/src/maasserver/migrations/fields.py b/src/maasserver/migrations/fields.py
index 2ccad90..edcee82 100644
--- a/src/maasserver/migrations/fields.py
+++ b/src/maasserver/migrations/fields.py
@@ -67,3 +67,13 @@ class JSONObjectField(Field):
if form_class is None:
form_class = forms.Field
return super().formfield(form_class=form_class, **kwargs)
+
+
+class MACAddressField(Field):
+ """Model for MAC addresses."""
+
+ def db_type(self, *args, **kwargs):
+ return "macaddr"
+
+ def get_prep_value(self, value):
+ return super().get_prep_value(value) or None
diff --git a/src/maasserver/migrations/maasserver/0001_initial.py b/src/maasserver/migrations/maasserver/0001_initial.py
index ddff643..bf1d1a9 100644
--- a/src/maasserver/migrations/maasserver/0001_initial.py
+++ b/src/maasserver/migrations/maasserver/0001_initial.py
@@ -802,7 +802,9 @@ class Migration(migrations.Migration):
),
(
"mac_address",
- maasserver.fields.MACAddressField(null=True, blank=True),
+ maasserver.migrations.fields.MACAddressField(
+ null=True, blank=True
+ ),
),
(
"ipv4_params",
@@ -1029,7 +1031,7 @@ class Migration(migrations.Migration):
"routers",
django.contrib.postgres.fields.ArrayField(
size=None,
- base_field=maasserver.fields.MACAddressField(),
+ base_field=maasserver.migrations.fields.MACAddressField(),
null=True,
blank=True,
default=list,
diff --git a/src/maasserver/migrations/maasserver/0008_use_new_arrayfield.py b/src/maasserver/migrations/maasserver/0008_use_new_arrayfield.py
index 5f4d628..1fad8dc 100644
--- a/src/maasserver/migrations/maasserver/0008_use_new_arrayfield.py
+++ b/src/maasserver/migrations/maasserver/0008_use_new_arrayfield.py
@@ -2,6 +2,7 @@ import django.contrib.postgres.fields
from django.db import migrations, models
import maasserver.fields
+import maasserver.migrations.fields
class Migration(migrations.Migration):
@@ -68,7 +69,7 @@ class Migration(migrations.Migration):
name="routers",
field=django.contrib.postgres.fields.ArrayField(
size=None,
- base_field=maasserver.fields.MACAddressField(),
+ base_field=maasserver.migrations.fields.MACAddressField(),
null=True,
blank=True,
default=list,
diff --git a/src/maasserver/migrations/maasserver/0076_interface_discovery_rescue_mode.py b/src/maasserver/migrations/maasserver/0076_interface_discovery_rescue_mode.py
index 7ac37ef..12d4131 100644
--- a/src/maasserver/migrations/maasserver/0076_interface_discovery_rescue_mode.py
+++ b/src/maasserver/migrations/maasserver/0076_interface_discovery_rescue_mode.py
@@ -81,7 +81,7 @@ class Migration(migrations.Migration):
("count", models.IntegerField(default=1)),
(
"mac_address",
- maasserver.fields.MACAddressField(
+ maasserver.migrations.fields.MACAddressField(
null=True, blank=True, editable=False
),
),
diff --git a/src/maasserver/migrations/maasserver/0083_device_discovery.py b/src/maasserver/migrations/maasserver/0083_device_discovery.py
index 39a1b04..739695d 100644
--- a/src/maasserver/migrations/maasserver/0083_device_discovery.py
+++ b/src/maasserver/migrations/maasserver/0083_device_discovery.py
@@ -2,6 +2,7 @@ import django
from django.db import migrations, models
import maasserver.fields
+import maasserver.migrations.fields
import maasserver.models.cleansave
@@ -39,7 +40,7 @@ class Migration(migrations.Migration):
),
(
"mac_address",
- maasserver.fields.MACAddressField(
+ maasserver.migrations.fields.MACAddressField(
blank=True, null=True, editable=False
),
),
diff --git a/src/maasserver/migrations/maasserver/0219_vm_nic_link.py b/src/maasserver/migrations/maasserver/0219_vm_nic_link.py
index ded377a..c5967b6 100644
--- a/src/maasserver/migrations/maasserver/0219_vm_nic_link.py
+++ b/src/maasserver/migrations/maasserver/0219_vm_nic_link.py
@@ -4,6 +4,7 @@ from django.db import migrations, models
import django.db.models.deletion
import maasserver.fields
+import maasserver.migrations.fields
import maasserver.models.cleansave
@@ -29,7 +30,9 @@ class Migration(migrations.Migration):
("updated", models.DateTimeField(editable=False)),
(
"mac_address",
- maasserver.fields.MACAddressField(blank=True, null=True),
+ maasserver.migrations.fields.MACAddressField(
+ blank=True, null=True
+ ),
),
(
"attachment_type",
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..13fe495
--- /dev/null
+++ b/src/maasserver/migrations/maasserver/0295_macaddress_text_field.py
@@ -0,0 +1,63 @@
+# Generated by Django 3.2.12 on 2023-03-01 16:07
+
+import re
+
+import django.core.validators
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+ dependencies = [
+ ("maasserver", "0294_keyring_data_binary_field"),
+ ]
+
+ operations = [
+ migrations.AlterField(
+ model_name="interface",
+ name="mac_address",
+ field=models.TextField(
+ blank=True,
+ null=True,
+ validators=[
+ django.core.validators.RegexValidator(
+ message="'%(value)s' is not a valid MAC address.",
+ regex=re.compile(
+ "^([0-9a-fA-F]{1,2}:){5}[0-9a-fA-F]{1,2}|([0-9a-fA-F]{1,2}-){5}[0-9a-fA-F]{1,2}|([0-9a-fA-F]{3,4}.){2}[0-9a-fA-F]{3,4}$"
+ ),
+ )
+ ],
+ ),
+ ),
+ migrations.AlterField(
+ model_name="neighbour",
+ name="mac_address",
+ field=models.TextField(
+ blank=True,
+ null=True,
+ validators=[
+ django.core.validators.RegexValidator(
+ message="'%(value)s' is not a valid MAC address.",
+ regex=re.compile(
+ "^([0-9a-fA-F]{1,2}:){5}[0-9a-fA-F]{1,2}|([0-9a-fA-F]{1,2}-){5}[0-9a-fA-F]{1,2}|([0-9a-fA-F]{3,4}.){2}[0-9a-fA-F]{3,4}$"
+ ),
+ )
+ ],
+ ),
+ ),
+ migrations.AlterField(
+ model_name="virtualmachineinterface",
+ name="mac_address",
+ field=models.TextField(
+ blank=True,
+ null=True,
+ validators=[
+ django.core.validators.RegexValidator(
+ message="'%(value)s' is not a valid MAC address.",
+ regex=re.compile(
+ "^([0-9a-fA-F]{1,2}:){5}[0-9a-fA-F]{1,2}|([0-9a-fA-F]{1,2}-){5}[0-9a-fA-F]{1,2}|([0-9a-fA-F]{3,4}.){2}[0-9a-fA-F]{3,4}$"
+ ),
+ )
+ ],
+ ),
+ ),
+ ]
diff --git a/src/maasserver/models/discovery.py b/src/maasserver/models/discovery.py
index 21bf8ff..60061f7 100644
--- a/src/maasserver/models/discovery.py
+++ b/src/maasserver/models/discovery.py
@@ -13,10 +13,11 @@ from django.db.models import (
GenericIPAddressField,
IntegerField,
Manager,
+ TextField,
)
from django.db.models.query import QuerySet
-from maasserver.fields import CIDRField, DomainNameField, MACAddressField
+from maasserver.fields import CIDRField, DomainNameField, MAC_VALIDATOR
from maasserver.models.cleansave import CleanSave
from maasserver.models.viewmodel import ViewModel
from maasserver.utils.orm import MAASQueriesMixin
@@ -208,8 +209,11 @@ class Discovery(CleanSave, ViewModel):
verbose_name="IP",
)
- mac_address = MACAddressField(
- unique=False, null=True, blank=True, editable=False
+ mac_address = TextField(
+ unique=False,
+ null=True,
+ blank=True,
+ validators=[MAC_VALIDATOR],
)
first_seen = DateTimeField(editable=False)
diff --git a/src/maasserver/models/interface.py b/src/maasserver/models/interface.py
index 1b89447..a861807 100644
--- a/src/maasserver/models/interface.py
+++ b/src/maasserver/models/interface.py
@@ -49,7 +49,7 @@ from maasserver.exceptions import (
StaticIPAddressOutOfRange,
StaticIPAddressUnavailable,
)
-from maasserver.fields import MAC_VALIDATOR, MACAddressField
+from maasserver.fields import MAC_VALIDATOR
from maasserver.models.cleansave import CleanSave
from maasserver.models.staticipaddress import StaticIPAddress
from maasserver.models.timestampedmodel import TimestampedModel
@@ -575,7 +575,7 @@ class Interface(CleanSave, TimestampedModel):
"StaticIPAddress", editable=True, blank=True
)
- mac_address = MACAddressField(unique=False, null=True, blank=True)
+ mac_address = TextField(null=True, blank=True, validators=[MAC_VALIDATOR])
ipv4_params = JSONField(blank=True, default=dict)
diff --git a/src/maasserver/models/neighbour.py b/src/maasserver/models/neighbour.py
index 9b88885..36b301d 100644
--- a/src/maasserver/models/neighbour.py
+++ b/src/maasserver/models/neighbour.py
@@ -10,10 +10,11 @@ from django.db.models import (
GenericIPAddressField,
IntegerField,
Manager,
+ TextField,
)
from django.db.models.query import QuerySet
-from maasserver.fields import MACAddressField
+from maasserver.fields import MAC_VALIDATOR
from maasserver.models.cleansave import CleanSave
from maasserver.models.interface import Interface
from maasserver.models.timestampedmodel import TimestampedModel
@@ -171,9 +172,7 @@ class Neighbour(CleanSave, TimestampedModel):
)
# Observed MAC address.
- mac_address = MACAddressField(
- unique=False, null=True, blank=True, editable=False
- )
+ mac_address = TextField(null=True, blank=True, validators=[MAC_VALIDATOR])
objects = NeighbourManager()
diff --git a/src/maasserver/models/virtualmachine.py b/src/maasserver/models/virtualmachine.py
index 55c7900..b43da7f 100644
--- a/src/maasserver/models/virtualmachine.py
+++ b/src/maasserver/models/virtualmachine.py
@@ -30,7 +30,7 @@ from django.db.models import (
from django.db.models.constraints import UniqueConstraint
from django.db.models.functions import Coalesce
-from maasserver.fields import MACAddressField
+from maasserver.fields import MAC_VALIDATOR
from maasserver.models.blockdevice import BlockDevice
from maasserver.models.bmc import BMC
from maasserver.models.cleansave import CleanSave
@@ -125,7 +125,7 @@ class VirtualMachineInterface(CleanSave, TimestampedModel):
on_delete=CASCADE,
related_name="+",
)
- mac_address = MACAddressField(null=True, blank=True)
+ mac_address = TextField(null=True, blank=True, validators=[MAC_VALIDATOR])
host_interface = ForeignKey(Interface, null=True, on_delete=SET_NULL)
attachment_type = CharField(
max_length=10,
diff --git a/src/maasserver/pytest_tests/test_fields.py b/src/maasserver/pytest_tests/test_fields.py
new file mode 100644
index 0000000..a8f8736
--- /dev/null
+++ b/src/maasserver/pytest_tests/test_fields.py
@@ -0,0 +1,39 @@
+from django.core.exceptions import ValidationError
+import pytest
+
+from maasserver.fields import MACAddressFormField, normalise_macaddress
+
+
+@pytest.mark.parametrize(
+ ("value", "expected"),
+ [
+ ("aabbccddeeff", "aa:bb:cc:dd:ee:ff"),
+ ("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
+
+
+class TestMACAddressFormField:
+ def test_validate_valid(self):
+ assert MACAddressFormField().validate("aa-bb-cc-dd-ee-ff") is None
+
+ def test_validate_invalid(self):
+ with pytest.raises(ValidationError):
+ MACAddressFormField().validate("invalid")
+
+ @pytest.mark.parametrize("value", ["", None])
+ def test_validate_empty(self, value):
+ assert MACAddressFormField().validate(value) is None
+
+ def test_normalise_mac_format(self):
+ assert (
+ MACAddressFormField().clean("aa-bb-cc-dd-ee-ff")
+ == "aa:bb:cc:dd:ee:ff"
+ )
diff --git a/src/maasserver/tests/test_fields.py b/src/maasserver/tests/test_fields.py
index 24014da..643366f 100644
--- a/src/maasserver/tests/test_fields.py
+++ b/src/maasserver/tests/test_fields.py
@@ -9,7 +9,6 @@ from django.db import connection, DatabaseError
from psycopg2 import OperationalError
from testtools import ExpectedException
-from maasserver.enum import INTERFACE_TYPE
from maasserver.fields import (
HostListFormField,
IPListFormField,
@@ -23,7 +22,7 @@ from maasserver.fields import (
URLOrPPAValidator,
VersionedTextFileField,
)
-from maasserver.models import Interface, Node, VersionedTextFile
+from maasserver.models import Node, VersionedTextFile
from maasserver.testing.factory import factory
from maasserver.testing.testcase import (
MAASLegacyServerTestCase,
@@ -61,15 +60,6 @@ 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 "
- )
- loaded_mac = Interface.objects.get(id=interface.id)
- self.assertEqual("aa:bb:cc:dd:ee:ff", loaded_mac.mac_address)
-
-
class TestXMLField(MAASLegacyServerTestCase):
apps = ["maasserver.tests"]
--
Mailing list: https://launchpad.net/~sts-sponsors
Post to : sts-sponsors@lists.launchpad.net
Unsubscribe : https://launchpad.net/~sts-sponsors
More help : https://help.launchpad.net/ListHelp
