sts-sponsors team mailing list archive

[Alexsander de Souza <mp+444129@xxxxxxxxxxxxxxxxxx>]

Alexsander de Souza has proposed merging ~alexsander-souza/maas:lp1807725_to_3_3 into maas:3.3.

Commit message:
Fix incorrect hostname from interface name

MAAS currently creates a DNS record for each interface in a host
by simply using its interface name. Whenever an interface has the
'_' character, the code uses it anyway, which is currently breaking
bind as this character is not allowed on domain names.
This patch fixes that by verifying it and replacing the incorrect
character.

LP: #1807725

(cherry picked from commit 83b123eb37b88dd0b9974bed9ee4352474ddd0a0)


Requested reviews:
  MAAS Maintainers (maas-maintainers)
Related bugs:
  Bug #1807725 in MAAS: "Machine interfaces allow '_' character, results on a interface based domain breaking bind (as it doesn't allow it for the host part)."
  https://bugs.launchpad.net/maas/+bug/1807725

For more details, see:
https://code.launchpad.net/~alexsander-souza/maas/+git/maas/+merge/444129
-- 
Your team MAAS Maintainers is requested to review the proposed merge of ~alexsander-souza/maas:lp1807725_to_3_3 into maas:3.3.

diff --git a/src/maasserver/models/staticipaddress.py b/src/maasserver/models/staticipaddress.py
index b5794a1..81c7a82 100644
--- a/src/maasserver/models/staticipaddress.py
+++ b/src/maasserver/models/staticipaddress.py
@@ -52,7 +52,10 @@ from maasserver.models.domain import Domain
 from maasserver.models.subnet import Subnet
 from maasserver.models.timestampedmodel import TimestampedModel
 from maasserver.utils import orm
-from maasserver.utils.dns import get_ip_based_hostname
+from maasserver.utils.dns import (
+    get_iface_name_based_hostname,
+    get_ip_based_hostname,
+)
 from provisioningserver.utils.enum import map_enum_reverse
 
 StaticIPAddress = TypeVar("StaticIPAddress")
@@ -841,7 +844,11 @@ class StaticIPAddressManager(Manager):
             # node, then consider adding the IP.
             if result.assigned or not assigned_ips[result.fqdn]:
                 if result.ip not in mapping[result.fqdn].ips:
-                    entry = mapping[f"{result.iface_name}.{result.fqdn}"]
+                    fqdn = "{}.{}".format(
+                        get_iface_name_based_hostname(result.iface_name),
+                        result.fqdn,
+                    )
+                    entry = mapping[fqdn]
                     entry.node_type = result.node_type
                     entry.system_id = result.system_id
                     if result.user_id is not None:
diff --git a/src/maasserver/models/tests/test_staticipaddress.py b/src/maasserver/models/tests/test_staticipaddress.py
index ee9c08a..08cdff1 100644
--- a/src/maasserver/models/tests/test_staticipaddress.py
+++ b/src/maasserver/models/tests/test_staticipaddress.py
@@ -50,7 +50,10 @@ from maasserver.testing.testcase import (
     MAASTransactionServerTestCase,
 )
 from maasserver.utils import orm
-from maasserver.utils.dns import get_ip_based_hostname
+from maasserver.utils.dns import (
+    get_iface_name_based_hostname,
+    get_ip_based_hostname,
+)
 from maasserver.utils.orm import reload_object, transactional
 from maasserver.websockets.base import dehydrate_datetime
 
@@ -483,6 +486,47 @@ class TestStaticIPAddressManagerMapping(MAASServerTestCase):
         }
         self.assertEqual(expected, mapping)
 
+    def test_get_hostname_ip_mapping_sanitized_iface_name(self):
+        hostname = factory.make_name("hostname")
+        domainname = factory.make_name("domain")
+        factory.make_Domain(name=domainname)
+        full_hostname = f"{hostname}.{domainname}"
+        subnet = factory.make_Subnet()
+        node = factory.make_Node_with_Interface_on_Subnet(
+            extra_ifnames=["eth_1"],
+            hostname=full_hostname,
+            interface_count=2,
+            subnet=subnet,
+        )
+        boot_interface = node.get_boot_interface()
+        staticip = factory.make_StaticIPAddress(
+            alloc_type=IPADDRESS_TYPE.STICKY,
+            ip=factory.pick_ip_in_Subnet(subnet),
+            subnet=subnet,
+            interface=boot_interface,
+        )
+        iface2 = node.current_config.interface_set.exclude(
+            id=boot_interface.id
+        ).first()
+        sip2 = factory.make_StaticIPAddress(
+            alloc_type=IPADDRESS_TYPE.STICKY,
+            ip=factory.pick_ip_in_Subnet(subnet),
+            subnet=subnet,
+            interface=iface2,
+        )
+        mapping = StaticIPAddress.objects.get_hostname_ip_mapping(subnet)
+        sanitized_if2name = get_iface_name_based_hostname(iface2.name)
+        expected = {
+            full_hostname: HostnameIPMapping(
+                node.system_id, 30, {staticip.ip}, node.node_type
+            ),
+            "%s.%s"
+            % (sanitized_if2name, full_hostname): HostnameIPMapping(
+                node.system_id, 30, {sip2.ip}, node.node_type
+            ),
+        }
+        self.assertEqual(expected, mapping)
+
     def make_mapping(self, node, raw_ttl=False):
         if raw_ttl or node.address_ttl is not None:
             ttl = node.address_ttl
diff --git a/src/maasserver/utils/dns.py b/src/maasserver/utils/dns.py
index 0c8cde5..76f553a 100644
--- a/src/maasserver/utils/dns.py
+++ b/src/maasserver/utils/dns.py
@@ -91,6 +91,16 @@ def get_ip_based_hostname(ip):
     return hostname
 
 
+def get_iface_name_based_hostname(iface_name):
+    """Given the specified interface name, creates an automatically generated
+    hostname by converting the '_' characters in it to '-' characters.
+
+    :param iface_name: Input value for the interface name.
+    """
+    hostname = iface_name.replace("_", "-")
+    return hostname
+
+
 def validate_url(url, schemes=("http", "https")):
     """Validator for URLs.
 
diff --git a/src/maasserver/utils/tests/test_dns.py b/src/maasserver/utils/tests/test_dns.py
index e6be8ab..41cfe80 100644
--- a/src/maasserver/utils/tests/test_dns.py
+++ b/src/maasserver/utils/tests/test_dns.py
@@ -8,6 +8,7 @@ from django.core.validators import URLValidator
 from testtools.matchers import Equals, HasLength
 
 from maasserver.utils.dns import (
+    get_iface_name_based_hostname,
     get_ip_based_hostname,
     validate_domain_name,
     validate_hostname,
@@ -244,3 +245,11 @@ class TestIpBasedHostnameGenerator(MAASTestCase):
             get_ip_based_hostname("2001:67c:1562::15"),
             Equals("2001-67c-1562--15"),
         )
+
+
+class TestIfaceBasedHostnameGenerator(MAASTestCase):
+    def test_interface_name_changed(self):
+        self.assertEqual(get_iface_name_based_hostname("eth_0"), "eth-0")
+
+    def test_interface_name_unchanged(self):
+        self.assertEqual(get_iface_name_based_hostname("eth0"), "eth0")