Merge ~cgrabowski/maas:revert_lp2056050_3.3 into maas:3.3

[Christian Grabowski]

Revert "fix: allow non-default ports for forward dns servers"
This reverts commit e33b5bedca4eb202df47ad34b804144766da72e9.

[Björn Tillenius (bjornt)]

+1

review: Approve

[MAAS Lander (maas-lander)]

UNIT TESTS
-b revert_lp2056050_3.3 lp:~cgrabowski/maas/+git/maas into -b 3.3 lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 32ecd82f99fd0258f7a55f24dba6533000b1409d

review: Approve

diff --git a/src/maasserver/dns/config.py b/src/maasserver/dns/config.py
index bbaea1a..0d55e7f 100644
--- a/src/maasserver/dns/config.py
+++ b/src/maasserver/dns/config.py
@@ -59,7 +59,7 @@ def forward_domains_to_forwarded_zones(forward_domains):
         (
             domain.name,
             [
-                (fwd_dns_srvr.ip_address, fwd_dns_srvr.port)
+                fwd_dns_srvr.ip_address
                 for fwd_dns_srvr in domain.forward_dns_servers
             ],
         )
diff --git a/src/maasserver/dns/tests/test_config.py b/src/maasserver/dns/tests/test_config.py
index 620cf44..44507a0 100644
--- a/src/maasserver/dns/tests/test_config.py
+++ b/src/maasserver/dns/tests/test_config.py
@@ -102,10 +102,7 @@ class TestDNSUtilities(MAASServerTestCase):
         )
         self.assertCountEqual(
             fwd_zones,
-            [
-                (name1, [(fwd_srvr1.ip_address, fwd_srvr1.port)]),
-                (name2, [(fwd_srvr2.ip_address, fwd_srvr2.port)]),
-            ],
+            [(name1, [fwd_srvr1.ip_address]), (name2, [fwd_srvr2.ip_address])],
         )
 
 
diff --git a/src/maasserver/fields.py b/src/maasserver/fields.py
index 941992b..7eeaad1 100644
--- a/src/maasserver/fields.py
+++ b/src/maasserver/fields.py
@@ -24,7 +24,6 @@ __all__ = [
 from copy import deepcopy
 from json import dumps, loads
 import re
-import urllib
 
 from django import forms
 from django.core.exceptions import ObjectDoesNotExist, ValidationError
@@ -604,44 +603,6 @@ class IPListFormField(forms.CharField):
             return " ".join(ips)
 
 
-class IPPortListFormField(IPListFormField):
-    def __init__(self, default_port=None, *args, **kwargs):
-        super().__init__(*args, **kwargs)
-        self._default_port = default_port
-
-    def clean(self, value):
-        if value is None:
-            return None
-        else:
-            ip_ports = re.split(self.separators, value)
-            ip_ports = [
-                ip_port.strip() for ip_port in ip_ports if ip_ports != ""
-            ]
-            result = []
-            for ip_port in ip_ports:
-                if "." in ip_port or ("[" == ip_port[0] and "]:" in ip_port):
-                    sock_addr = urllib.parse.urlsplit("//" + ip_port)
-                    ip = sock_addr.hostname
-                    port = (
-                        int(sock_addr.port)
-                        if sock_addr.port
-                        else self._default_port
-                    )
-                else:
-                    ip = ip_port
-                    port = self._default_port
-                try:
-                    GenericIPAddressField().clean(ip, model_instance=None)
-                    IntegerField().clean(port, model_instance=None)
-                except ValidationError:
-                    raise ValidationError(
-                        f"Invalid IP and port combination: {ip_port};"
-                        f"please provide a list of space-separated IP addresses {'and port' if not self._default_port else ''}"
-                    )
-                result.append((ip, port))
-        return result
-
-
 class HostListFormField(forms.CharField):
     """Accepts a space/comma separated list of hostnames or IP addresses.
 
diff --git a/src/maasserver/forms/domain.py b/src/maasserver/forms/domain.py
index d5739a4..b530f3f 100644
--- a/src/maasserver/forms/domain.py
+++ b/src/maasserver/forms/domain.py
@@ -7,7 +7,7 @@
 from django import forms
 from django.core.exceptions import ValidationError
 
-from maasserver.fields import IPPortListFormField
+from maasserver.fields import IPListFormField
 from maasserver.forms import APIEditMixin, MAASModelForm
 from maasserver.models.domain import Domain
 from maasserver.models.forwarddnsserver import ForwardDNSServer
@@ -24,16 +24,16 @@ class DomainForm(MAASModelForm):
 
     authoritative = forms.NullBooleanField(required=False)
 
-    forward_dns_servers = IPPortListFormField(default_port=53, required=False)
+    forward_dns_servers = IPListFormField(required=False)
 
     def save(self):
         super(MAASModelForm, self).save()
         fwd_srvrs = self.cleaned_data.get("forward_dns_servers")
         if fwd_srvrs is not None:
-            for fwd_srvr_ip, fwd_srvr_port in fwd_srvrs:
+            fwd_srvrs_list = fwd_srvrs.split(" ")
+            for fwd_srvr_ip in fwd_srvrs_list:
                 fwd_srvr = ForwardDNSServer.objects.get_or_create(
-                    ip_address=fwd_srvr_ip,
-                    port=fwd_srvr_port,
+                    ip_address=fwd_srvr_ip
                 )[0]
                 fwd_srvr.domains.add(self.instance)
                 fwd_srvr.save()
diff --git a/src/maasserver/forms/tests/test_domain.py b/src/maasserver/forms/tests/test_domain.py
index e9a3a3e..043dc47 100644
--- a/src/maasserver/forms/tests/test_domain.py
+++ b/src/maasserver/forms/tests/test_domain.py
@@ -104,30 +104,6 @@ class TestDomainForm(MAASServerTestCase):
                 for fwd_dns_srvr in domain.forward_dns_servers
             ],
         )
-        for fwd_dns_srvr in domain.forward_dns_servers:
-            self.assertEqual(fwd_dns_srvr.port, 53)
-
-    def test_can_create_forward_dns_server_with_port(self):
-        name = factory.make_name("domain")
-        forward_dns_servers = [
-            f"{factory.make_ip_address(ipv6=False)}:5353" for _ in range(0, 2)
-        ]
-        form = DomainForm(
-            {
-                "name": name,
-                "authoritative": False,
-                "forward_dns_servers": "  ".join(forward_dns_servers),
-            }
-        )
-        self.assertTrue(form.is_valid(), form.errors)
-        domain = form.save()
-        self.assertEqual(
-            forward_dns_servers,
-            [
-                fwd_dns_srvr.ip_and_port
-                for fwd_dns_srvr in domain.forward_dns_servers
-            ],
-        )
 
     def test_validate_authority(self):
         name = factory.make_name("domain")
diff --git a/src/maasserver/migrations/maasserver/0291_add_port_to_forward_dns_servers.py b/src/maasserver/migrations/maasserver/0291_add_port_to_forward_dns_servers.py
deleted file mode 100644
index ec8244e..0000000
--- a/src/maasserver/migrations/maasserver/0291_add_port_to_forward_dns_servers.py
+++ /dev/null
@@ -1,17 +0,0 @@
-# Generated by Django 3.2.12 on 2024-03-07 17:16
-
-from django.db import migrations, models
-
-
-class Migration(migrations.Migration):
-    dependencies = [
-        ("maasserver", "0290_migrate_node_power_parameters"),
-    ]
-
-    operations = [
-        migrations.AddField(
-            model_name="forwarddnsserver",
-            name="port",
-            field=models.IntegerField(default=53),
-        ),
-    ]
diff --git a/src/maasserver/models/forwarddnsserver.py b/src/maasserver/models/forwarddnsserver.py
index fc27fd0..9fe07be 100644
--- a/src/maasserver/models/forwarddnsserver.py
+++ b/src/maasserver/models/forwarddnsserver.py
@@ -6,12 +6,7 @@ __all__ = [
 ]
 
 
-from django.db.models import (
-    GenericIPAddressField,
-    IntegerField,
-    Manager,
-    ManyToManyField,
-)
+from django.db.models import GenericIPAddressField, Manager, ManyToManyField
 
 from maasserver.models.cleansave import CleanSave
 from maasserver.models.domain import Domain
@@ -35,10 +30,4 @@ class ForwardDNSServer(CleanSave, TimestampedModel):
         null=False, default=None, editable=False, unique=True
     )
 
-    port = IntegerField(null=False, default=53)
-
     domains = ManyToManyField(Domain)
-
-    @property
-    def ip_and_port(self):
-        return f"{self.ip_address}:{self.port}"
diff --git a/src/maasserver/tests/test_fields.py b/src/maasserver/tests/test_fields.py
index 166b5b1..8a4d610 100644
--- a/src/maasserver/tests/test_fields.py
+++ b/src/maasserver/tests/test_fields.py
@@ -20,7 +20,6 @@ from maasserver.fields import (
     EditableBinaryField,
     HostListFormField,
     IPListFormField,
-    IPPortListFormField,
     JSONObjectField,
     LargeObjectField,
     LargeObjectFile,
@@ -582,54 +581,6 @@ class TestIPListFormField(MAASTestCase):
         )
 
 
-class TestIPPortListFormField(MAASTestCase):
-    def test_accepts_ipv4_with_port(self):
-        ips = [factory.make_ip_address(ipv6=False) for _ in range(5)]
-        ip_ports = [f"{ip}:80" for ip in ips]
-        input = ",".join(ip_ports)
-        self.assertEqual(
-            [(ip, 80) for ip in ips], IPPortListFormField().clean(input)
-        )
-
-    def test_accepts_ipv4_without_port(self):
-        ips = [factory.make_ip_address(ipv6=False) for _ in range(5)]
-        input = ",".join(ips)
-        self.assertEqual(
-            [(ip, 80) for ip in ips],
-            IPPortListFormField(default_port=80).clean(input),
-        )
-
-    def test_accepts_ipv6_with_port(self):
-        ips = [factory.make_ip_address(ipv6=True) for _ in range(5)]
-        ip_ports = [f"[{ip}]:80" for ip in ips]
-        input = ",".join(ip_ports)
-        self.assertEqual(
-            [(ip, 80) for ip in ips], IPPortListFormField().clean(input)
-        )
-
-    def test_accepts_ipv6_without_port(self):
-        ips = [factory.make_ip_address(ipv6=True) for _ in range(5)]
-        input = ",".join(ips)
-        self.assertEqual(
-            [(ip, 80) for ip in ips],
-            IPPortListFormField(default_port=80).clean(input),
-        )
-
-    def test_rejects_invalid_ip(self):
-        ip_ports = [
-            f"{factory.make_ip_address(ipv6=False)}:80" for _ in range(4)
-        ]
-        invalid = f"{factory.make_name()}:80"
-        ip_ports.append(invalid)
-        input = ",".join(ip_ports)
-        error = self.assertRaises(
-            ValidationError, IPPortListFormField().clean, input
-        )
-        self.assertIn(
-            f"Invalid IP and port combination: {invalid}", error.message
-        )
-
-
 class TestHostListFormField(MAASTestCase):
     def test_accepts_none(self):
         self.assertIsNone(HostListFormField().clean(None))
diff --git a/src/provisioningserver/dns/tests/test_config.py b/src/provisioningserver/dns/tests/test_config.py
index 9c7ad2f..8c57ce4 100644
--- a/src/provisioningserver/dns/tests/test_config.py
+++ b/src/provisioningserver/dns/tests/test_config.py
@@ -601,7 +601,7 @@ class TestDNSConfig(MAASTestCase):
     def test_write_config_with_forwarded_zones(self):
         name = factory.make_name("domain")
         ip = factory.make_ip_address()
-        forwarded_zones = [(name, [(ip, None)])]
+        forwarded_zones = [(name, [ip])]
         target_dir = patch_dns_config_path(self)
         DNSConfig(forwarded_zones=forwarded_zones).write_config()
         config_path = os.path.join(target_dir, MAAS_NAMED_CONF_NAME)
@@ -611,7 +611,7 @@ class TestDNSConfig(MAASTestCase):
             type forward;
             forward only;
             forwarders {{
-                {ip} port 53;
+                {ip};
             }};
         }};
         """
@@ -620,30 +620,6 @@ class TestDNSConfig(MAASTestCase):
         expected = parse_isc_string(expected_content)
         self.assertEqual(expected[f'zone "{name}"'], config[f'zone "{name}"'])
 
-    def test_write_config_with_forwarded_zones_with_custom_port(self):
-        name = factory.make_name("domain")
-        ip = factory.make_ip_address()
-        port = 5353
-        forwarded_zones = [(name, [(ip, port)])]
-        target_dir = patch_dns_config_path(self)
-        DNSConfig(forwarded_zones=forwarded_zones).write_config()
-        config_path = os.path.join(target_dir, MAAS_NAMED_CONF_NAME)
-        expected_content = dedent(
-            f"""
-        zone "{name}" {{
-            type forward;
-            forward only;
-            forwarders {{
-                {ip} port {port};
-            }};
-        }};
-        """
-        )
-        config = read_isc_file(config_path)
-        print(config)
-        expected = parse_isc_string(expected_content)
-        self.assertEqual(expected[f'zone "{name}"'], config[f'zone "{name}"'])
-
     def test_write_config_makes_config_world_readable(self):
         target_dir = patch_dns_config_path(self)
         DNSConfig().write_config()
diff --git a/src/provisioningserver/templates/dns/named.conf.template b/src/provisioningserver/templates/dns/named.conf.template
index a909560..7217ca8 100644
--- a/src/provisioningserver/templates/dns/named.conf.template
+++ b/src/provisioningserver/templates/dns/named.conf.template
@@ -21,7 +21,7 @@ zone "{{forwarded_zone[0]}}" {
     forward only;
     forwarders {
     {{for forward_server in forwarded_zone[1]}}
-        {{forward_server[0]}} port {{ forward_server[1] if forward_server[1] else 53 }};
+        {{forward_server}};
     {{endfor}}
     };
 };