Use a MultiMACField in the Register Nodes form

Make it possible to specify multiple MAC addresses and use
a textarea field for entering them.

Change-Id: I1a2241d591f4174e08e7c7cc560f4e43facc6457

tuskar_ui/forms.py

@@ -11,11 +11,16 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import re
+
 from django import forms
 from django.utils.translation import ugettext_lazy as _
 import netaddr
 
 
+SEPARATOR_RE = re.compile('[\s,;|]+', re.UNICODE)
+
+
 def fieldset(self, *args, **kwargs):
     """A helper function for grouping fields based on their names."""
 
@@ -26,11 +31,28 @@ def fieldset(self, *args, **kwargs):
             yield forms.forms.BoundField(self, self.fields[name], name)
 
 
+class MACDialect(netaddr.mac_eui48):
+    """For validating MAC addresses. Same validation as Nova uses."""
+    word_fmt = '%.02x'
+    word_sep = ':'
+
+
+def normalize_MAC(value):
+    try:
+        return str(netaddr.EUI(
+            value.strip(), version=48, dialect=MACDialect)).upper()
+    except (netaddr.AddrFormatError, TypeError):
+        raise ValueError('Invalid MAC address')
+
+
 class NumberInput(forms.widgets.TextInput):
+    """A form input for numbers."""
     input_type = 'number'
 
 
 class NumberPickerInput(NumberInput):
+    """A form input that is rendered as a big number picker."""
+
     def __init__(self, attrs=None):
         default_attrs = {'hr-number-picker': '', 'ng-cloak': '', }
         if attrs:
@@ -39,20 +61,43 @@ class NumberPickerInput(NumberInput):
 
 
 class MACField(forms.fields.Field):
+    """A form field for entering a single MAC address."""
+
     def clean(self, value):
-        class mac_dialect(netaddr.mac_eui48):
+        value = super(MACField, self).clean(value)
-            """Same validation as Nova uses."""
-            word_fmt = '%.02x'
-            word_sep = ':'
         try:
-            return str(netaddr.EUI(
+            return normalize_MAC(value)
-                value.strip(), version=48, dialect=mac_dialect)).upper()
+        except ValueError:
-        except (netaddr.AddrFormatError, TypeError):
             raise forms.ValidationError(_(u'Enter a valid MAC address.'))
 
 
-class NetworkField(forms.fields.Field):
+class MultiMACField(forms.fields.Field):
+    """A form field for entering multiple MAC addresses.
+
+       The individual MAC addresses can be separated by any whitespace,
+       commas, semicolons or pipe characters.
+
+       Gives a string of normalized MAC addresses separated by spaces.
+    """
+
     def clean(self, value):
+        value = super(MultiMACField, self).clean(value)
+        try:
+            macs = []
+            for mac in SEPARATOR_RE.split(value):
+                if mac:
+                    macs.append(normalize_MAC(mac))
+            return ' '.join(macs)
+        except ValueError:
+            raise forms.ValidationError(
+                _(u'%r is not a valid MAC address.') % mac)
+
+
+class NetworkField(forms.fields.Field):
+    """A form field for entering a network specification with a mask."""
+
+    def clean(self, value):
+        value = super(NetworkField, self).clean(value)
         try:
             return str(netaddr.IPNetwork(value, version=4))
         except netaddr.AddrFormatError:

tuskar_ui/infrastructure/nodes/forms.py

@@ -42,10 +42,11 @@ class NodeForm(django.forms.Form):
         widget=django.forms.PasswordInput(
             render_value=False, attrs={'class': 'input input-medium'}),
     )
-    mac_address = tuskar_ui.forms.MACField(
+    mac_addresses = tuskar_ui.forms.MultiMACField(
-        label=_("NIC MAC Address"),
+        label=_("NIC MAC Addresses"),
-        widget=django.forms.TextInput(attrs={
+        widget=django.forms.Textarea(attrs={
-            'class': 'input input-medium'
+            'class': 'input input-medium',
+            'rows': '2',
         }),
     )
     cpus = django.forms.IntegerField(
@@ -94,7 +95,7 @@ class BaseNodeFormset(django.forms.formsets.BaseFormSet):
                     form.cleaned_data.get('cpus'),
                     form.cleaned_data.get('memory'),
                     form.cleaned_data.get('local_disk'),
-                    form.cleaned_data['mac_address'],
+                    form.cleaned_data['mac_addresses'].split(),
                     form.cleaned_data.get('ipmi_username'),
                     form.cleaned_data.get('ipmi_password'),
                 )

tuskar_ui/infrastructure/nodes/templates/nodes/_nodes_formset_form.html

@@ -13,7 +13,7 @@
     </div>
     <div class="row-fluid">
       <h5>Networking</h5>
-      {% include 'infrastructure/nodes/_nodes_formset_field.html' with field=form.mac_address required=True %}
+      {% include 'infrastructure/nodes/_nodes_formset_field.html' with field=form.mac_addresses required=True %}
     </div>
     <div class="row-fluid">
       <div class="span4">

tuskar_ui/infrastructure/nodes/tests.py

@@ -167,13 +167,13 @@ class NodesTests(test.BaseAdminViewTests, helpers.APITestCase):
             'register_nodes-0-ipmi_address': '127.0.0.1',
             'register_nodes-0-ipmi_username': 'username',
             'register_nodes-0-ipmi_password': 'password',
-            'register_nodes-0-mac_address': 'de:ad:be:ef:ca:fe',
+            'register_nodes-0-mac_addresses': 'de:ad:be:ef:ca:fe',
             'register_nodes-0-cpus': '1',
             'register_nodes-0-memory': '2',
             'register_nodes-0-local_disk': '3',
 
             'register_nodes-1-ipmi_address': '127.0.0.2',
-            'register_nodes-1-mac_address': 'de:ad:be:ef:ca:ff',
+            'register_nodes-1-mac_addresses': 'de:ad:be:ef:ca:ff',
             'register_nodes-1-cpus': '4',
             'register_nodes-1-memory': '5',
             'register_nodes-1-local_disk': '6',
@@ -186,9 +186,9 @@ class NodesTests(test.BaseAdminViewTests, helpers.APITestCase):
             request = Node.create.call_args_list[0][0][0]  # This is a hack.
             self.assertListEqual(Node.create.call_args_list, [
                 call(request, u'127.0.0.1', 1, 2, 3,
-                     'DE:AD:BE:EF:CA:FE', u'username', u'password'),
+                     ['DE:AD:BE:EF:CA:FE'], u'username', u'password'),
                 call(request, u'127.0.0.2', 4, 5, 6,
-                     'DE:AD:BE:EF:CA:FF', None, None),
+                     ['DE:AD:BE:EF:CA:FF'], None, None),
             ])
         self.assertRedirectsNoFollow(res, INDEX_URL)
 
@@ -201,13 +201,13 @@ class NodesTests(test.BaseAdminViewTests, helpers.APITestCase):
             'register_nodes-0-ipmi_address': '127.0.0.1',
             'register_nodes-0-ipmi_username': 'username',
             'register_nodes-0-ipmi_password': 'password',
-            'register_nodes-0-mac_address': 'de:ad:be:ef:ca:fe',
+            'register_nodes-0-mac_addresses': 'de:ad:be:ef:ca:fe',
             'register_nodes-0-cpus': '1',
             'register_nodes-0-memory': '2',
             'register_nodes-0-local_disk': '3',
 
             'register_nodes-1-ipmi_address': '127.0.0.2',
-            'register_nodes-1-mac_address': 'de:ad:be:ef:ca:ff',
+            'register_nodes-1-mac_addresses': 'de:ad:be:ef:ca:ff',
             'register_nodes-1-cpus': '4',
             'register_nodes-1-memory': '5',
             'register_nodes-1-local_disk': '6',
@@ -220,9 +220,9 @@ class NodesTests(test.BaseAdminViewTests, helpers.APITestCase):
             request = Node.create.call_args_list[0][0][0]  # This is a hack.
             self.assertListEqual(Node.create.call_args_list, [
                 call(request, u'127.0.0.1', 1, 2, 3,
-                     'DE:AD:BE:EF:CA:FE', u'username', u'password'),
+                     ['DE:AD:BE:EF:CA:FE'], u'username', u'password'),
                 call(request, u'127.0.0.2', 4, 5, 6,
-                     'DE:AD:BE:EF:CA:FF', None, None),
+                     ['DE:AD:BE:EF:CA:FF'], None, None),
             ])
         self.assertTemplateUsed(
             res, 'infrastructure/nodes/register.html')

tuskar_ui/test/test_forms.py

@@ -0,0 +1,59 @@
+# -*- coding: utf8 -*-
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+
+from django.utils.translation import ugettext_lazy as _
+
+from tuskar_ui import forms
+from tuskar_ui.test import helpers as test
+
+
+class MultiMACFieldTests(test.TestCase):
+    def test_empty(self):
+        field = forms.MultiMACField(required=False)
+        cleaned = field.clean("")
+        self.assertEqual(cleaned, "")
+
+    def test_required(self):
+        field = forms.MultiMACField(required=True)
+        with self.assertRaises(forms.forms.ValidationError) as raised:
+            field.clean("")
+        self.assertEqual(unicode(raised.exception.messages[0]),
+                         unicode(_('This field is required.')))
+
+    def test_malformed(self):
+        field = forms.MultiMACField(required=True)
+        with self.assertRaises(forms.forms.ValidationError) as raised:
+            field.clean("de.ad:be.ef:ca.fe")
+        self.assertEqual(
+            unicode(raised.exception.messages[0]),
+            unicode(_(u"'de.ad:be.ef:ca.fe' is not a valid MAC address.")),
+        )
+
+    def test_single(self):
+        field = forms.MultiMACField(required=False)
+        cleaned = field.clean("de:AD:be:ef:Ca:FE")
+        self.assertEqual(cleaned, "DE:AD:BE:EF:CA:FE")
+
+    def test_multiple(self):
+        field = forms.MultiMACField(required=False)
+        cleaned = field.clean(
+            "de:AD:be:ef:Ca:FC, de:AD:be:ef:Ca:FD de:AD:be:ef:Ca:FE\n"
+            "de:AD:be:ef:Ca:FF",
+        )
+        self.assertEqual(
+            cleaned,
+            "DE:AD:BE:EF:CA:FC DE:AD:BE:EF:CA:FD DE:AD:BE:EF:CA:FE "
+            "DE:AD:BE:EF:CA:FF",
+        )