Merge ~ack/maas:1871584-subnet-no-dns-fix-2.7 into maas:2.7

[Alberto Donato]

LP: #1871584 - include MAAS IP in DNS zone even if subnet has allow_dns false

[Alberto Donato (ack)]

review: Approve

diff --git a/src/maasserver/dns/tests/test_zonegenerator.py b/src/maasserver/dns/tests/test_zonegenerator.py
index 7499924..5655873 100644
--- a/src/maasserver/dns/tests/test_zonegenerator.py
+++ b/src/maasserver/dns/tests/test_zonegenerator.py
@@ -27,6 +27,7 @@ from maasserver.dns import zonegenerator
 from maasserver.dns.zonegenerator import (
     get_dns_search_paths,
     get_dns_server_address,
+    get_dns_server_addresses,
     get_hostname_dnsdata_mapping,
     get_hostname_ip_mapping,
     InternalDomain,
@@ -82,7 +83,7 @@ class TestGetDNSServerAddress(MAASServerTestCase):
         self.assertThat(
             patch,
             MockCalledOnceWith(
-                ANY,
+                rack_controller=None,
                 include_alternates=False,
                 ipv4=ipv4,
                 ipv6=ipv6,
@@ -135,6 +136,37 @@ class TestGetDNSServerAddress(MAASServerTestCase):
         self.assertIsNone(get_dns_server_address(rack_controller))
 
 
+class TestGetDNSServerAddresses(MAASServerTestCase):
+    def test_no_rack_all_subnets(self):
+        subnet1 = factory.make_Subnet(cidr="10.10.0.0/24", allow_dns=False)
+        subnet2 = factory.make_Subnet(cidr="10.20.0.0/24", allow_dns=True)
+        ip1 = factory.make_StaticIPAddress(subnet=subnet1)
+        ip2 = factory.make_StaticIPAddress(subnet=subnet2)
+        ips = {IPAddress(ip1.ip), IPAddress(ip2.ip)}
+        resolver = self.patch(server_address, "resolve_hostname")
+        resolver.return_value = ips
+        rack_controller = factory.make_RackController()
+        self.assertCountEqual(
+            get_dns_server_addresses(
+                rack_controller=rack_controller, filter_allowed_dns=False
+            ),
+            ips,
+        )
+
+    def test_with_rack_only_allow_dns(self):
+        subnet1 = factory.make_Subnet(cidr="10.10.0.0/24", allow_dns=False)
+        subnet2 = factory.make_Subnet(cidr="10.20.0.0/24", allow_dns=True)
+        ip1 = factory.make_StaticIPAddress(subnet=subnet1)
+        ip2 = factory.make_StaticIPAddress(subnet=subnet2)
+        resolver = self.patch(server_address, "resolve_hostname")
+        resolver.return_value = {IPAddress(ip1.ip), IPAddress(ip2.ip)}
+        rack_controller = factory.make_RackController()
+        self.assertCountEqual(
+            get_dns_server_addresses(rack_controller=rack_controller),
+            [IPAddress(ip2.ip)],
+        )
+
+
 class TestGetDNSSearchPaths(MAASServerTestCase):
     def test__returns_all_authoritative_domains(self):
         domain_names = get_dns_search_paths()
@@ -541,6 +573,24 @@ class TestZoneGenerator(MAASServerTestCase):
         )
         self.assertEqual({}, zones[2]._mapping)
 
+    def test_forward_zone_includes_subnets_with_allow_dns_false(self):
+        default_ttl = random.randint(10, 300)
+        Config.objects.set_config("default_dns_ttl", default_ttl)
+        default_domain = Domain.objects.get_default_domain()
+        subnet = factory.make_Subnet(cidr="10.10.0.0/24", allow_dns=False)
+        ip = factory.make_StaticIPAddress(subnet=subnet)
+        resolver = self.patch(server_address, "resolve_hostname")
+        resolver.return_value = {IPAddress(ip.ip)}
+        zones = ZoneGenerator(
+            [default_domain], subnet, serial=random.randint(0, 65535)
+        )
+        [forward_zone] = [
+            zone for zone in zones if isinstance(zone, DNSForwardZoneConfig)
+        ]
+        self.assertEqual(
+            forward_zone._other_mapping["@"].rrset, {(default_ttl, "A", ip.ip)}
+        )
+
     def rfc2317_network(self, network):
         """Returns the network that rfc2317 glue goes in, if any."""
         net = network
diff --git a/src/maasserver/dns/zonegenerator.py b/src/maasserver/dns/zonegenerator.py
index 140871d..cda8222 100644
--- a/src/maasserver/dns/zonegenerator.py
+++ b/src/maasserver/dns/zonegenerator.py
@@ -120,6 +120,7 @@ def get_dns_server_addresses(
     ipv6=True,
     include_alternates=False,
     default_region_ip=None,
+    filter_allowed_dns=True,
 ):
     """Return the DNS server's IP addresses.
 
@@ -136,13 +137,15 @@ def get_dns_server_addresses(
     :param include_alternates: Include IP addresses from other regions?
     :param default_region_ip: The default source IP address to be used, if a
         specific URL is not defined.
+    :param filter_allowed_dns: If true, only include addresses for subnets
+        with allow_dns=True.
     :return: List of IPAddress to use.  Loopback addresses are removed from the
         list, unless there are no non-loopback addresses.
 
     """
     try:
-        iplist = get_maas_facing_server_addresses(
-            rack_controller,
+        ips = get_maas_facing_server_addresses(
+            rack_controller=rack_controller,
             ipv4=ipv4,
             ipv6=ipv6,
             include_alternates=include_alternates,
@@ -157,22 +160,21 @@ def get_dns_server_addresses(
             "local_config_set --maas-url' command." % e.strerror
         )
 
-    # LP:1847537 - Filter out MAAS DNS servers running on subnets which do not
-    # allow DNS to be provided from MAAS.
-    filtered_list = [
-        ip
-        for ip in iplist
-        if getattr(
-            Subnet.objects.get_best_subnet_for_ip(ip), "allow_dns", True
-        )
-    ]
-    non_loop = [ip for ip in filtered_list if not ip.is_loopback()]
-    if len(non_loop) > 0:
+    if filter_allowed_dns:
+        ips = [
+            ip
+            for ip in ips
+            if getattr(
+                Subnet.objects.get_best_subnet_for_ip(ip), "allow_dns", True
+            )
+        ]
+    non_loop = [ip for ip in ips if not ip.is_loopback()]
+    if non_loop:
         return non_loop
     else:
-        for ip in filtered_list:
+        for ip in ips:
             warn_loopback(ip)
-        return filtered_list
+        return ips
 
 
 def get_dns_search_paths():
@@ -237,7 +239,7 @@ class ZoneGenerator:
         internal_domains,
     ):
         """Generator of forward zones, collated by domain name."""
-        dns_ip_list = get_dns_server_addresses()
+        dns_ip_list = get_dns_server_addresses(filter_allowed_dns=False)
         domains = set(domains)
 
         # For each of the domains that we are generating, create the zone from: