Merge lp:~rvb/maas/maas-forwarders-1.8 into lp:maas/1.8

[Raphaël Badin]

Backport 4062: upstream_dns: accept a list of forwarders (and not just *one* forwarder).

[Gavin Panella (allenap)]

I know this is a back-port even though I've reviewed it like a new change. There's nothing in here that I think should prevent landing at all, but I have a number of comments anyway.

review: Approve

[Mike Pontillo (mpontillo)]

A couple quick comments.

[Raphaël Badin (rvb)]

Thanks for the review. I'll forward port the fixes made here to trunk.

[MAAS Lander (maas-lander)]

There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.

=== modified file 'src/maasserver/dns/tests/test_config.py'
--- src/maasserver/dns/tests/test_config.py	2015-05-16 06:21:20 +0000
+++ src/maasserver/dns/tests/test_config.py	2015-07-06 08:31:26 +0000
@@ -496,15 +496,16 @@
     def test_dns_update_all_zones_now_passes_upstream_dns_parameter(self):
         self.patch(settings, 'DNS_CONNECT', True)
         self.create_managed_nodegroup()
-        random_ip = factory.make_ipv4_address()
-        Config.objects.set_config("upstream_dns", random_ip)
+        ips = [factory.make_ipv4_address() for _ in range(3)]
+        input_ips = " ".join(ips)
+        Config.objects.set_config("upstream_dns", input_ips)
         bind_write_options = self.patch_autospec(
             dns_config_module, "bind_write_options")
         dns_update_all_zones_now()
         self.assertThat(
             bind_write_options,
             MockCalledOnceWith(
-                dnssec_validation='auto', upstream_dns=[random_ip]))
+                dnssec_validation='auto', upstream_dns=ips))
 
     def test_dns_update_all_zones_now_writes_trusted_networks_parameter(self):
         self.patch(settings, 'DNS_CONNECT', True)
@@ -667,12 +668,13 @@
         self.assertEqual([], get_upstream_dns())
 
     def test__returns_list_of_one_address_if_set(self):
-        address = factory.make_ipv4_address()
+        address = factory.make_ip_address()
         Config.objects.set_config("upstream_dns", address)
         self.assertEqual([address], get_upstream_dns())
 
-    def test__returns_list_of_many_address_if_set(self):
-        addresses = [factory.make_ipv4_address(), factory.make_ipv4_address()]
+    def test__returns_list_if_space_separated_ips(self):
+        addresses = [
+            factory.make_ip_address() for _ in range(3)]
         Config.objects.set_config("upstream_dns", " ".join(addresses))
         self.assertEqual(addresses, get_upstream_dns())
 

=== modified file 'src/maasserver/fields.py'
--- src/maasserver/fields.py	2015-05-13 11:33:20 +0000
+++ src/maasserver/fields.py	2015-07-06 08:31:26 +0000
@@ -1,7 +1,7 @@
 # Copyright 2012-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-"""Custom model fields."""
+"""Custom model and form fields."""
 
 from __future__ import (
     absolute_import,
@@ -14,6 +14,7 @@
 __metaclass__ = type
 __all__ = [
     "EditableBinaryField",
+    "IPListFormField",
     "MAASIPAddressField",
     "MAC",
     "MACAddressField",
@@ -532,3 +533,26 @@
         raise AssertionError(
             "Invalid LargeObjectField value (expected integer): '%s'"
             % repr(value))
+
+
+class IPListFormField(CharField):
+    """Accepts a space/comma separated list of IP addresses.
+
+    This field normalizes the list to a space-separated list.
+    """
+    separators = re.compile('[,\s]+')
+
+    def clean(self, value):
+        if value is None:
+            return None
+        else:
+            ips = re.split(self.separators, value)
+            ips = [ip.strip() for ip in ips if ip != '']
+            for ip in ips:
+                try:
+                    GenericIPAddressField().clean(ip, model_instance=None)
+                except ValidationError:
+                    raise ValidationError(
+                        "Invalid IP address: %s; provide a list of "
+                        "space-separated IP addresses" % ip)
+            return ' '.join(ips)

=== modified file 'src/maasserver/forms_settings.py'
--- src/maasserver/forms_settings.py	2015-06-04 14:31:33 +0000
+++ src/maasserver/forms_settings.py	2015-07-06 08:31:26 +0000
@@ -26,6 +26,7 @@
 from django import forms
 from django.core.exceptions import ValidationError
 from maasserver.bootresources import IMPORT_RESOURCES_SERVICE_PERIOD
+from maasserver.fields import IPListFormField
 from maasserver.models.config import (
     Config,
     DEFAULT_OS,
@@ -180,11 +181,11 @@
     },
     'upstream_dns': {
         'default': None,
-        'form': forms.GenericIPAddressField,
+        'form': IPListFormField,
         'form_kwargs': {
             'label': (
                 "Upstream DNS used to resolve domains not managed by this "
-                "MAAS"),
+                "MAAS (space-separated IP addresses)"),
             'required': False,
             'help_text': (
                 "Only used when MAAS is running its own DNS server. This "

=== modified file 'src/maasserver/tests/test_fields.py'
--- src/maasserver/tests/test_fields.py	2015-05-07 18:14:38 +0000
+++ src/maasserver/tests/test_fields.py	2015-07-06 08:31:26 +0000
@@ -16,6 +16,7 @@
 
 import json
 from random import randint
+import re
 
 from django.core import serializers
 from django.core.exceptions import ValidationError
@@ -27,6 +28,7 @@
 from maasserver.enum import NODEGROUPINTERFACE_MANAGEMENT
 from maasserver.fields import (
     EditableBinaryField,
+    IPListFormField,
     LargeObjectField,
     LargeObjectFile,
     MAC,
@@ -518,3 +520,38 @@
         field = LargeObjectField()
         self.assertRaises(
             AssertionError, field.to_python, factory.make_string())
+
+
+class IPListFormFieldTest(MAASServerTestCase):
+
+    def test_accepts_none(self):
+        self.assertIsNone(IPListFormField().clean(None))
+
+    def test_accepts_single_ip(self):
+        ip = factory.make_ip_address()
+        self.assertEquals(ip, IPListFormField().clean(ip))
+
+    def test_accepts_space_separated_ips(self):
+        ips = [factory.make_ip_address() for _ in range(5)]
+        input = ' '.join(ips)
+        self.assertEquals(input, IPListFormField().clean(input))
+
+    def test_accepts_comma_separated_ips(self):
+        ips = [factory.make_ip_address() for _ in range(5)]
+        input = ','.join(ips)
+        self.assertEquals(' '.join(ips), IPListFormField().clean(input))
+
+    def test_rejects_invalid_input(self):
+        invalid = factory.make_name('invalid')
+        input = ' '.join([factory.make_ip_address(), invalid])
+        error = self.assertRaises(
+            ValidationError, IPListFormField().clean, input)
+        self.assertIn("Invalid IP address: %s" % invalid, error.message)
+
+    def test_separators_dont_conflict_with_ipv4_address(self):
+        self.assertIsNone(re.search(
+            IPListFormField.separators, factory.make_ipv4_address()))
+
+    def test_separators_dont_conflict_with_ipv6_address(self):
+        self.assertIsNone(re.search(
+            IPListFormField.separators, factory.make_ipv6_address()))

=== modified file 'src/maasserver/tests/test_forms_settings.py'
--- src/maasserver/tests/test_forms_settings.py	2015-05-07 18:14:38 +0000
+++ src/maasserver/tests/test_forms_settings.py	2015-07-06 08:31:26 +0000
@@ -14,6 +14,7 @@
 __metaclass__ = type
 __all__ = []
 
+
 from django import forms
 from maasserver.forms_settings import (
     CONFIG_ITEMS,
@@ -70,3 +71,10 @@
             compose_invalid_choice_text(
                 'commissioning_distro_series', field.choices),
             field.error_messages['invalid_choice'])
+
+    def test_upstream_dns_accepts_ip_list(self):
+        field = get_config_field('upstream_dns')
+        ips1 = [factory.make_ip_address() for _ in range(3)]
+        ips2 = [factory.make_ip_address() for _ in range(3)]
+        input = ' '.join(ips1) + ' ' + ','.join(ips2)
+        self.assertEqual(' '.join(ips1 + ips2), field.clean(input))