sts-sponsors team mailing list archive

[Christian Grabowski <mp+434659@xxxxxxxxxxxxxxxxxx>]

Christian Grabowski has proposed merging ~cgrabowski/maas:ensure_all_reverse_dns_records_are_accounted_for into maas:master.

Commit message:
fix adding dynamic updates for reverse glue zones



Requested reviews:
  MAAS Maintainers (maas-maintainers)
Related bugs:
  Bug #1999668 in MAAS: "[3.3.0~rc1-13133-g.67fd5b9af] reverse DNS not working for some interfaces"
  https://bugs.launchpad.net/maas/+bug/1999668

For more details, see:
https://code.launchpad.net/~cgrabowski/maas/+git/maas/+merge/434659
-- 
Your team MAAS Committers is subscribed to branch maas:master.

diff --git a/src/maasserver/dns/tests/test_zonegenerator.py b/src/maasserver/dns/tests/test_zonegenerator.py
index 5a3aee1..f89e00a 100644
--- a/src/maasserver/dns/tests/test_zonegenerator.py
+++ b/src/maasserver/dns/tests/test_zonegenerator.py
@@ -48,6 +48,7 @@ from maasserver.utils.orm import transactional
 from maastesting.factory import factory as maastesting_factory
 from maastesting.fakemethod import FakeMethod
 from maastesting.matchers import MockAnyCall, MockCalledOnceWith, MockNotCalled
+from provisioningserver.dns.config import DynamicDNSUpdate
 from provisioningserver.dns.zoneconfig import (
     DNSForwardZoneConfig,
     DNSReverseZoneConfig,
@@ -510,6 +511,37 @@ class TestZoneGenerator(MAASServerTestCase):
         }
         self.assertEqual(expected_map, zones[0]._other_mapping)
 
+    def test_glue_receives_correct_dynamic_updates(self):
+        domain = factory.make_Domain()
+        subnet = factory.make_Subnet(cidr=str(IPNetwork("10/29").cidr))
+        sip = factory.make_StaticIPAddress(subnet=subnet)
+        factory.make_Node_with_Interface_on_Subnet(
+            subnet=subnet, vlan=subnet.vlan, fabric=subnet.vlan.fabric
+        )
+        update_rec = factory.make_DNSResource(name=factory.make_name(), domain=domain, ip_addresses=[sip])
+        updates = [
+            DynamicDNSUpdate(
+                operation="INSERT",
+                name=update_rec.name,
+                zone=domain.name,
+                rectype="A",
+                answer=sip.ip,
+            )
+        ]
+        zones = ZoneGenerator(
+            domain, subnet, serial=random.randint(0, 65535), dynamic_updates=updates
+        ).as_list()
+        print(zones[0])
+        self.assertCountEqual(
+            zones[0]._dynamic_updates, updates
+        )
+        self.assertCountEqual(
+            zones[1]._dynamic_updates, [ DynamicDNSUpdate.as_reverse_record_update(updates[0], str(IPNetwork("10/29"))) ]
+        )
+        self.assertCountEqual(
+            zones[2]._dynamic_updates, [ DynamicDNSUpdate.as_reverse_record_update(updates[0], str(IPNetwork("10/24"))) ]
+        )
+
     def test_parent_of_default_domain_gets_glue(self):
         default_domain = Domain.objects.get_default_domain()
         default_domain.name = "maas.example.com"
diff --git a/src/maasserver/dns/zonegenerator.py b/src/maasserver/dns/zonegenerator.py
index ef31cfa..b726097 100644
--- a/src/maasserver/dns/zonegenerator.py
+++ b/src/maasserver/dns/zonegenerator.py
@@ -452,7 +452,7 @@ class ZoneGenerator:
                 glue = set()
 
             domain_updates = [
-                DynamicDNSUpdate.as_reverse_record_update(update, subnet)
+                DynamicDNSUpdate.as_reverse_record_update(update, str(subnet.cidr))
                 for update in dynamic_updates
                 if update.answer
                 and update.answer_is_ip
@@ -476,6 +476,20 @@ class ZoneGenerator:
             )
         # Now provide any remaining rfc2317 glue networks.
         for network, ranges in rfc2317_glue.items():
+            exclude_set = {
+                IPNetwork(s.cidr)
+                for s in subnets
+                if network in IPNetwork(s.cidr)
+            }
+            domain_updates = []
+            for update in dynamic_updates:
+                glue_update = True
+                for exclude_net in exclude_set:
+                    if update.answer and update.answer_is_ip and IPAddress(update.answer) in exclude_net:
+                        glue_update = False
+                        break
+                if glue_update:
+                    domain_updates.append(DynamicDNSUpdate.as_reverse_record_update(update, str(network)))
             yield DNSReverseZoneConfig(
                 ns_host_name,
                 serial=serial,
@@ -483,11 +497,7 @@ class ZoneGenerator:
                 network=network,
                 ns_host_name=ns_host_name,
                 rfc2317_ranges=ranges,
-                exclude={
-                    IPNetwork(s.cidr)
-                    for s in subnets
-                    if network in IPNetwork(s.cidr)
-                },
+                exclude=exclude_set,
                 dynamic_updates=domain_updates,
                 force_config_write=force_config_write,
             )
diff --git a/src/provisioningserver/dns/tests/test_zoneconfig.py b/src/provisioningserver/dns/tests/test_zoneconfig.py
index 948d219..8d523e0 100644
--- a/src/provisioningserver/dns/tests/test_zoneconfig.py
+++ b/src/provisioningserver/dns/tests/test_zoneconfig.py
@@ -967,6 +967,91 @@ class TestDNSReverseZoneConfig(MAASTestCase):
             stdin=expected_stdin.encode("ascii"),
         )
 
+    def test_glue_network_zone_contains_appropriate_dynamic_updates(self):
+        patch_zone_file_config_path(self)
+        domain = factory.make_string()
+        network = IPNetwork("10.0.0.0/26")
+        glue_network = IPNetwork("10.0.0.0/24")
+        ip1 = factory.pick_ip_in_network(network)
+        ip2 = factory.pick_ip_in_network(network)
+        hostname1 = f"{factory.make_string()}.{domain}"
+        hostname2 = f"{factory.make_string()}.{domain}"
+        fwd_updates = [
+            DynamicDNSUpdate(
+                operation="INSERT",
+                zone=domain,
+                name=hostname1,
+                rectype="A",
+                answer=ip1,
+            ),
+            DynamicDNSUpdate(
+                operation="INSERT",
+                zone=domain,
+                name=hostname2,
+                rectype="A",
+                answer=ip2,
+            ),
+        ]
+        rev_updates = [
+            DynamicDNSUpdate.as_reverse_record_update(update, str(network))
+            for update in fwd_updates
+        ]
+        zone = DNSReverseZoneConfig(
+            domain,
+            serial=random.randint(1, 100),
+            network=network,
+            dynamic_updates=rev_updates,
+        )
+        glue_rev_updates = [
+            DynamicDNSUpdate.as_reverse_record_update(update, str(glue_network))
+            for update in fwd_updates
+        ]
+        glue_zone = DNSReverseZoneConfig(
+            domain,
+            serial=random.randint(1, 100),
+            network=glue_network,
+            dynamic_updates=glue_rev_updates,
+        )
+        expected_stdin = "\n".join(
+            [
+                "server localhost",
+                "zone 0-26.0.0.10.in-addr.arpa",
+                f"update add {IPAddress(ip1).reverse_dns} {zone.default_ttl} PTR {hostname1}",
+                f"update add {IPAddress(ip2).reverse_dns} {zone.default_ttl} PTR {hostname2}",
+                f"update add 0-26.0.0.10.in-addr.arpa {zone.default_ttl} SOA 0-26.0.0.10.in-addr.arpa. nobody.example.com. {zone.serial} 600 1800 604800 {zone.default_ttl}",
+                "send\n",
+            ]
+        )
+        glue_expected_stdin = "\n".join(
+            [
+                "server localhost",
+                "zone 0.0.10.in-addr.arpa",
+                f"update add {IPAddress(ip1).reverse_dns} {zone.default_ttl} PTR {hostname1}",
+                f"update add {IPAddress(ip2).reverse_dns} {zone.default_ttl} PTR {hostname2}",
+                f"update add 0.0.10.in-addr.arpa {glue_zone.default_ttl} SOA 0.0.10.in-addr.arpa. nobody.example.com. {glue_zone.serial} 600 1800 604800 {glue_zone.default_ttl}",
+                "send\n",
+            ]
+        )
+        run_command = self.patch(actions, "run_command")
+        glue_zone.write_config()
+        glue_zone.write_config()
+        run_command.assert_called_with(
+            "nsupdate",
+            "-k",
+            get_nsupdate_key_path(),
+            "-v",
+            stdin=glue_expected_stdin.encode("ascii"),
+        )
+        zone.write_config()
+        zone.write_config()
+        run_command.assert_called_with(
+            "nsupdate",
+            "-k",
+            get_nsupdate_key_path(),
+            "-v",
+            stdin=expected_stdin.encode("ascii"),
+        )
+
 
 class TestDNSReverseZoneConfig_GetGenerateDirectives(MAASTestCase):
     """Tests for `DNSReverseZoneConfig.get_GENERATE_directives()`."""
diff --git a/src/provisioningserver/dns/zoneconfig.py b/src/provisioningserver/dns/zoneconfig.py
index eed539a..9989743 100644
--- a/src/provisioningserver/dns/zoneconfig.py
+++ b/src/provisioningserver/dns/zoneconfig.py
@@ -177,7 +177,7 @@ class DomainConfigBase:
             [
                 update
                 for update in self._dynamic_updates
-                if update.zone == zone_info.zone_name or update.subnet
+                if update.zone == zone_info.zone_name or IPNetwork(update.subnet) == zone_info.subnetwork
             ],
             serial=self.serial,
             ttl=self.default_ttl,