Merge ~cgrabowski/maas:fix_dynamic_reverse_dns_update into maas:master

[Christian Grabowski]

lookup subnet for reverse record zone generation

[Alexsander de Souza (alexsander-souza)]

+1

review: Approve

[MAAS Lander (maas-lander)]

UNIT TESTS
-b fix_dynamic_reverse_dns_update lp:~cgrabowski/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas-tester/3212/console
COMMIT: 8d0875076551a1af4f1e1f86c2ab88022ab16854

review: Needs Fixing

[MAAS Lander (maas-lander)]

UNIT TESTS
-b fix_dynamic_reverse_dns_update lp:~cgrabowski/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas-tester/3214/console
COMMIT: 566455a3a2db4c6efc6daa906fce874433cc663b

review: Needs Fixing

[MAAS Lander (maas-lander)]

UNIT TESTS
-b fix_dynamic_reverse_dns_update lp:~cgrabowski/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 144270d806c5b144d2d6d5e3ab3a0a427031b73a

review: Approve

diff --git a/src/maasserver/dns/config.py b/src/maasserver/dns/config.py
index 91b1e72..2515a5a 100644
--- a/src/maasserver/dns/config.py
+++ b/src/maasserver/dns/config.py
@@ -7,7 +7,7 @@
 from collections import defaultdict
 
 from django.conf import settings
-from netaddr import IPAddress
+from netaddr import IPAddress, IPNetwork
 
 from maasserver.dns.zonegenerator import (
     InternalDomain,
@@ -32,6 +32,7 @@ from provisioningserver.dns.actions import (
     bind_write_zones,
 )
 from provisioningserver.dns.config import DynamicDNSUpdate
+from provisioningserver.dns.zoneconfig import DNSReverseZoneConfig
 from provisioningserver.logger import get_maas_logger
 from provisioningserver.prometheus.metrics import PROMETHEUS_METRICS
 from provisioningserver.utils.shell import ExternalProcessError
@@ -295,6 +296,16 @@ def get_internal_domain():
     )
 
 
+def get_reverse_zone_for_answer(answer):
+    subnet = Subnet.objects.get_best_subnet_for_ip(answer)
+    if not subnet:
+        return None
+
+    network = IPNetwork(subnet.cidr)
+    zone_info = DNSReverseZoneConfig.compose_zone_info(network)
+    return zone_info[0].zone_name
+
+
 def process_dns_update_notify(message):
     updates = []
     update_list = message.split(" ")
@@ -321,12 +332,17 @@ def process_dns_update_notify(message):
             ttl = int(update_list[-2]) if update_list[-2] else None
             answer = update_list[-1]
 
+    rev_zone = None
+    if rectype in ("A", "AAAA") and answer is not None:
+        rev_zone = get_reverse_zone_for_answer(answer)
+
     match op:
         case "UPDATE":
             updates.append(
                 DynamicDNSUpdate.create_from_trigger(
                     operation="DELETE",
                     zone=zone,
+                    rev_zone=rev_zone,
                     name=name,
                     rectype=rectype,
                     answer=answer,
@@ -336,6 +352,7 @@ def process_dns_update_notify(message):
                 DynamicDNSUpdate.create_from_trigger(
                     operation="INSERT",
                     zone=zone,
+                    rev_zone=rev_zone,
                     name=name,
                     rectype=rectype,
                     ttl=ttl,
@@ -347,6 +364,7 @@ def process_dns_update_notify(message):
                 DynamicDNSUpdate.create_from_trigger(
                     operation=op,
                     zone=zone,
+                    rev_zone=rev_zone,
                     name=name,
                     rectype=rectype,
                     ttl=ttl,
@@ -361,6 +379,7 @@ def process_dns_update_notify(message):
                     DynamicDNSUpdate.create_from_trigger(
                         operation="DELETE",
                         zone=zone,
+                        rev_zone=rev_zone,
                         name=name,
                         rectype=rectype,
                     )
@@ -370,6 +389,7 @@ def process_dns_update_notify(message):
                         DynamicDNSUpdate.create_from_trigger(
                             operation="DELETE",
                             zone=zone,
+                            rev_zone=rev_zone,
                             name=name,
                             rectype="AAAA",
                         )
@@ -403,6 +423,7 @@ def process_dns_update_notify(message):
                     DynamicDNSUpdate.create_from_trigger(
                         operation="INSERT",
                         zone=zone,
+                        rev_zone=get_reverse_zone_for_answer(ip.ip),
                         name=name,
                         rectype=rectype,
                         ttl=ttl,
@@ -415,6 +436,7 @@ def process_dns_update_notify(message):
                     DynamicDNSUpdate.create_from_trigger(
                         operation=op,
                         zone=zone,
+                        rev_zone=rev_zone,
                         name=name,
                         rectype=rectype,
                         answer=update_list[-1],
@@ -425,6 +447,7 @@ def process_dns_update_notify(message):
                     DynamicDNSUpdate.create_from_trigger(
                         operation=op,
                         zone=zone,
+                        rev_zone=rev_zone,
                         name=name,
                         rectype=rectype,
                     )
diff --git a/src/maasserver/dns/tests/test_config.py b/src/maasserver/dns/tests/test_config.py
index 0a96814..dedeff2 100644
--- a/src/maasserver/dns/tests/test_config.py
+++ b/src/maasserver/dns/tests/test_config.py
@@ -7,7 +7,7 @@ import time
 
 from django.conf import settings
 import dns.resolver
-from netaddr import IPAddress
+from netaddr import IPAddress, IPNetwork
 from testtools.matchers import (
     Contains,
     Equals,
@@ -26,6 +26,7 @@ from maasserver.dns.config import (
     forward_domains_to_forwarded_zones,
     get_internal_domain,
     get_resource_name_for_subnet,
+    get_reverse_zone_for_answer,
     get_trusted_acls,
     get_trusted_networks,
     get_upstream_dns,
@@ -797,6 +798,7 @@ class TestProcessDNSUpdateNotify(MAASServerTestCase):
                 DynamicDNSUpdate(
                     operation="INSERT",
                     zone=domain.name,
+                    rev_zone=get_reverse_zone_for_answer(ip),
                     name=f"{resource.name}.{domain.name}",
                     ttl=resource.address_ttl if resource.address_ttl else 60,
                     answer=ip,
@@ -853,6 +855,7 @@ class TestProcessDNSUpdateNotify(MAASServerTestCase):
                 DynamicDNSUpdate(
                     operation="DELETE",
                     zone=domain.name,
+                    rev_zone=get_reverse_zone_for_answer(ip),
                     name=f"{resource.name}.{domain.name}",
                     answer=ip,
                     rectype="A" if IPAddress(ip).version == 4 else "AAAA",
@@ -860,6 +863,7 @@ class TestProcessDNSUpdateNotify(MAASServerTestCase):
                 DynamicDNSUpdate(
                     operation="INSERT",
                     zone=domain.name,
+                    rev_zone=get_reverse_zone_for_answer(ip),
                     name=f"{resource.name}.{domain.name}",
                     ttl=resource.address_ttl if resource.address_ttl else 60,
                     answer=ip,
@@ -898,6 +902,7 @@ class TestProcessDNSUpdateNotify(MAASServerTestCase):
                 DynamicDNSUpdate(
                     operation="INSERT",
                     zone=domain.name,
+                    rev_zone=get_reverse_zone_for_answer(ip2.ip),
                     name=f"{resource.name}.{domain.name}",
                     rectype="A" if IPAddress(ip2.ip).version == 4 else "AAAA",
                     answer=ip2.ip,
@@ -932,6 +937,7 @@ class TestProcessDNSUpdateNotify(MAASServerTestCase):
                 DynamicDNSUpdate(
                     operation="INSERT",
                     zone=domain.name,
+                    rev_zone=get_reverse_zone_for_answer(ip2.ip),
                     name=f"{node.hostname}.{domain.name}",
                     rectype="A" if IPAddress(ip2.ip).version == 4 else "AAAA",
                     answer=ip2.ip,
@@ -939,3 +945,60 @@ class TestProcessDNSUpdateNotify(MAASServerTestCase):
             ],
             result,
         )
+
+    def test_insert_reverse_glue_zone(self):
+        domain = factory.make_Domain()
+        subnet = factory.make_Subnet(cidr="192.168.1.64/26")
+        ip = subnet.get_next_ip_for_allocation()[0]
+        resource = factory.make_DNSResource(domain=domain, ip=ip)
+        message = f"INSERT {domain.name} {resource.name} A {resource.address_ttl if resource.address_ttl else 60} {ip}"
+        zone = get_reverse_zone_for_answer(ip)
+        result, _ = process_dns_update_notify(message)
+        ip_split = ip.split(".")
+        self.assertCountEqual(
+            [
+                DynamicDNSUpdate(
+                    operation="INSERT",
+                    zone=zone,
+                    name=".".join([ip_split[-1], zone]),
+                    ttl=resource.address_ttl if resource.address_ttl else 60,
+                    subnet=subnet.cidr,
+                    answer=f"{resource.name}.{domain.name}",
+                    rectype="PTR",
+                )
+            ],
+            [
+                DynamicDNSUpdate.as_reverse_record_update(
+                    res, IPNetwork(subnet.cidr)
+                )
+                for res in result
+            ],
+        )
+
+
+class TestGetReverseZoneForUpdate(MAASServerTestCase):
+    def test_get_reverse_zone_for_answer_v4_24(self):
+        subnet = factory.make_Subnet(cidr="192.168.1.0/24")
+        ip = subnet.get_next_ip_for_allocation()[0]
+        rev_zone = get_reverse_zone_for_answer(ip)
+        self.assertEqual("1.168.192.in-addr.arpa", rev_zone)
+
+    def test_get_reverse_zone_for_update_v4_26(self):
+        subnet = factory.make_Subnet(cidr="192.168.1.0/26")
+        ip = subnet.get_next_ip_for_allocation()[0]
+        rev_zone = get_reverse_zone_for_answer(ip)
+        self.assertEqual("0-26.1.168.192.in-addr.arpa", rev_zone)
+
+    def test_get_reverse_zone_for_update_v6_64(self):
+        subnet = factory.make_Subnet(cidr="de:ad:be:ef:ca::/64")
+        ip = subnet.get_next_ip_for_allocation()[0]
+        rev_zone = get_reverse_zone_for_answer(ip)
+        self.assertEqual("f.e.0.0.e.b.0.0.d.a.0.0.e.d.0.0.ip6.arpa", rev_zone)
+
+    def test_get_reverse_zone_for_update_v6_65(self):
+        subnet = factory.make_Subnet(cidr="de:ad:be:ef:ca::/65")
+        ip = subnet.get_next_ip_for_allocation()[0]
+        rev_zone = get_reverse_zone_for_answer(ip)
+        self.assertEqual(
+            "0.f.e.0.0.e.b.0.0.d.a.0.0.e.d.0.0.ip6.arpa", rev_zone
+        )
diff --git a/src/maasserver/tests/test_region_controller.py b/src/maasserver/tests/test_region_controller.py
index 1a9ee32..03c198f 100644
--- a/src/maasserver/tests/test_region_controller.py
+++ b/src/maasserver/tests/test_region_controller.py
@@ -864,7 +864,8 @@ class TestRegionControllerServiceTransactional(MAASTransactionServerTestCase):
         check_serial.return_value = succeed(update_result)
 
         service._dns_update_in_progress = True
-        service.queueDynamicDNSUpdate(
+        yield deferToDatabase(
+            service.queueDynamicDNSUpdate,
             factory.make_name(),
             f"INSERT {domain.name} {record.name} A 30 10.10.10.10",
         )
@@ -913,7 +914,8 @@ class TestRegionControllerServiceTransactional(MAASTransactionServerTestCase):
         check_serial.return_value = succeed(update_result)
 
         service._dns_update_in_progress = True
-        service.queueDynamicDNSUpdate(
+        yield deferToDatabase(
+            service.queueDynamicDNSUpdate,
             factory.make_name(),
             f"INSERT {domain.name} {record.name} A 30 10.10.10.10",
         )
diff --git a/src/provisioningserver/dns/config.py b/src/provisioningserver/dns/config.py
index de7aba1..264139e 100644
--- a/src/provisioningserver/dns/config.py
+++ b/src/provisioningserver/dns/config.py
@@ -44,6 +44,7 @@ class DynamicDNSUpdate:
     name: str
     zone: str
     rectype: str
+    rev_zone: Optional[str] = None
     ttl: Optional[int] = None
     subnet: Optional[str] = None  # for reverse updates
     answer: Optional[str] = None
@@ -72,19 +73,22 @@ class DynamicDNSUpdate:
             return None
         ip = IPAddress(fwd_update.answer)
         name = ip.reverse_dns
+        zone = fwd_update.rev_zone
         if (ip.version == 4 and subnet.prefixlen > 24) or (
             ip.version == 6 and subnet.prefixlen > 124
         ):
             name_split = ip.reverse_dns.split(".")
-            name_split = (
-                name_split[:1] + [f"0-{subnet.prefixlen}"] + name_split[1:]
-            )
+            suffix = [zone]
+            if not zone:
+                suffix = [f"0-{subnet.prefixlen}"] + name_split[1:]
+
+            name_split = name_split[:1] + suffix
             name = ".".join(name_split)
 
         return cls(
             operation=fwd_update.operation,
             name=name,
-            zone=fwd_update.zone,
+            zone=zone or fwd_update.zone,
             subnet=str(subnet.cidr),
             ttl=fwd_update.ttl,
             answer=fwd_update.name,