launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/maas/big-networks into lp:maas.

Commit message:
Reject networks >= /8 networks.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~rvb/maas/big-networks/+merge/128269

Reject networks >= /8 networks.  This ends up pre-populating a huge zone file that neither celeryd nor bind handle properly.
-- 
https://code.launchpad.net/~rvb/maas/big-networks/+merge/128269
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/big-networks into lp:maas.

=== modified file 'src/maasserver/models/nodegroupinterface.py'
--- src/maasserver/models/nodegroupinterface.py	2012-10-04 11:43:37 +0000
+++ src/maasserver/models/nodegroupinterface.py	2012-10-05 15:04:23 +0000
@@ -38,6 +38,13 @@
     )
 from netaddr.core import AddrFormatError
 
+# The minimum number of leading bits of the routing prefix for a
+# network.  A smaller number will be rejected as it creates a huge
+# address space that is currently not well supported by the DNS
+# machinery.
+# For instance, if MINIMUM_PREFIX_LEN is 9, a /8 will be rejected.
+MINIMUM_PREFIX_LEN = 9
+
 
 class NodeGroupInterface(CleanSave, TimestampedModel):
 
@@ -93,8 +100,8 @@
         return "<NodeGroupInterface %s,%s>" % (
             self.nodegroup.uuid, self.interface)
 
-    def clean_network(self):
-        """Validate that the network is valid.
+    def clean_network_valid(self):
+        """Validate the network.
 
         This validates that the network defined by broadcast_ip and
         subnet_mask is valid.
@@ -111,7 +118,19 @@
                     'subnet_mask': [e.message],
                 })
 
-            raise ValidationError(e.message)
+    def clean_network_not_too_big(self):
+        network = self.network
+        if network is not None:
+            if network.prefixlen < MINIMUM_PREFIX_LEN:
+                message = (
+                    "Cannot create an address space bigger than "
+                    "a /%d network.  This network is a /%d network." %
+                        (MINIMUM_PREFIX_LEN, network.prefixlen))
+                raise ValidationError(
+                {
+                    'broadcast_ip': [message],
+                    'subnet_mask': [message],
+                })
 
     def clean_management(self):
         # XXX: rvb 2012-09-18 bug=1052339: Only one "managed" interface
@@ -181,7 +200,8 @@
 
     def clean_fields(self, *args, **kwargs):
         super(NodeGroupInterface, self).clean_fields(*args, **kwargs)
-        self.clean_network()
+        self.clean_network_valid()
+        self.clean_network_not_too_big()
         self.clean_ips_in_network()
         self.clean_management()
         self.clean_network_config_if_managed()

=== modified file 'src/maasserver/tests/test_nodegroupinterface.py'
--- src/maasserver/tests/test_nodegroupinterface.py	2012-10-04 11:43:37 +0000
+++ src/maasserver/tests/test_nodegroupinterface.py	2012-10-05 15:04:23 +0000
@@ -18,6 +18,8 @@
     NODEGROUPINTERFACE_MANAGEMENT,
     NODEGROUPINTERFACE_MANAGEMENT_CHOICES_DICT,
     )
+from maasserver.models import NodeGroup
+from maasserver.models.nodegroupinterface import MINIMUM_PREFIX_LEN
 from maasserver.testing.factory import factory
 from maasserver.testing.testcase import TestCase
 from netaddr import IPNetwork
@@ -97,6 +99,26 @@
             },
             exception.message_dict)
 
+    def test_clean_network_rejects_huge_network(self):
+        big_network = IPNetwork('1.2.3.4/%d' % (MINIMUM_PREFIX_LEN - 1))
+        exception = self.assertRaises(
+            ValidationError, factory.make_node_group, network=big_network)
+        message = (
+            "Cannot create an address space bigger than a /%d network.  "
+            "This network is a /%d network." %
+                (MINIMUM_PREFIX_LEN, MINIMUM_PREFIX_LEN - 1))
+        self.assertEqual(
+            {
+                'subnet_mask': [message],
+                'broadcast_ip': [message],
+            },
+            exception.message_dict)
+
+    def test_clean_network_accepts_network_if_not_too_big(self):
+        network = IPNetwork('1.2.3.4/%d' % MINIMUM_PREFIX_LEN)
+        self.assertIsInstance(
+            factory.make_node_group(network=network), NodeGroup)
+
     def test_clean_network_config_if_managed(self):
         network = IPNetwork('192.168.0.3/24')
         checked_fields = [