Merge ~ltrager/maas:lp1911825 into maas:master

[Lee Trager]

LP: #1911825 - Don't set BMC.ip_address when power_address contains an FQDN

When a BMC is created or edited MAAS maps its IP address to a subnet which
is stored in the model. This allows MAAS to send power commands only to
rack controllers which are on the same subnet as the BMC. If no rack
controller is on the same subnet as the BMC the power command is sent to
all rack controllers. When an FQDN or hostname were given an exception was
raised as MAAS expected an IP address. The exception is now avoided so the
fallback code is used which allow for FQDN or hostnames in the
power_address.

[MAAS Lander (maas-lander)]

UNIT TESTS
-b lp1911825 lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: f50b041262946304722be1eb4342ef27826c52f4

review: Approve

[Alberto Donato (ack)]

one question inline

review: Needs Information

077855c... by Lee Trager on 2021-01-18

ack fix

[MAAS Lander (maas-lander)]

UNIT TESTS
-b lp1911825 lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 077855cdbea784ef8e831add002c0592b16119e3

review: Approve

[Alberto Donato (ack)]

+1 looks good. thanks for the fix

review: Approve

diff --git a/src/maasserver/models/bmc.py b/src/maasserver/models/bmc.py
index 5802c5c..6eefa76 100644
--- a/src/maasserver/models/bmc.py
+++ b/src/maasserver/models/bmc.py
@@ -1,4 +1,4 @@
-# Copyright 2015-2020 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2021 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """BMC objects."""
@@ -30,6 +30,7 @@ from django.db.models import (
 )
 from django.db.models.query import QuerySet
 from django.shortcuts import get_object_or_404
+from netaddr import AddrFormatError, IPAddress
 import petname
 from twisted.internet.defer import inlineCallbacks
 
@@ -327,10 +328,6 @@ class BMC(CleanSave, TimestampedModel):
         parameters has changed."""
         new_ip = BMC.extract_ip_address(self.power_type, self.power_parameters)
         current_ip = None if self.ip_address is None else self.ip_address.ip
-        # Set the ip_address field.  If we have a bracketed address, assume
-        # it's IPv6, and strip the brackets.
-        if new_ip and new_ip.startswith("[") and new_ip.endswith("]"):
-            new_ip = new_ip[1:-1]
         if new_ip != current_ip:
             if not new_ip:
                 self.ip_address = None
@@ -425,7 +422,29 @@ class BMC(CleanSave, TimestampedModel):
             return None
         match = re.match(extraction_pattern, field_value)
         if match:
-            return match.group("address")
+            ip = match.group("address")
+            # If we have a bracketed address, assume it's IPv6, and strip the
+            # brackets.
+            if ip.startswith("[") and ip.endswith("]"):
+                ip = ip[1:-1]
+            if ip == "":
+                return ip
+            # self.clean() attempts to map the return value of this method to
+            # a subnet. If the user gives an FQDN or hostname the mapping fails
+            # when Subnet.objects.get_best_subnet_for_ip() is called and an
+            # exception is raised as an IP is expected. If the BMC does not
+            # have an IP address MAAS will fall back on sending power requests
+            # to all connected rack controllers.
+            try:
+                IPAddress(ip)
+            except AddrFormatError:
+                maaslog.info(
+                    "BMC uses FQDN, power action will be sent to all "
+                    "rack controllers"
+                )
+                return None
+            else:
+                return ip
         # no match found - return None
         return None
 
diff --git a/src/maasserver/models/tests/test_bmc.py b/src/maasserver/models/tests/test_bmc.py
index adf1fe4..18597b5 100644
--- a/src/maasserver/models/tests/test_bmc.py
+++ b/src/maasserver/models/tests/test_bmc.py
@@ -1,4 +1,4 @@
-# Copyright 2016-2020 Canonical Ltd.  This software is licensed under the
+# Copyright 2016-2021 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test maasserver models."""
@@ -433,14 +433,6 @@ class TestBMC(MAASServerTestCase):
         power_parameters = {"power_address": None}
         self.assertEqual(None, BMC.extract_ip_address("hmc", power_parameters))
 
-    def test_bmc_extract_ip_address_from_url(self):
-        power_parameters = {
-            "power_address": "protocol://somehost:8080/path/to/thing#tag"
-        }
-        self.assertEqual(
-            "somehost", BMC.extract_ip_address("virsh", power_parameters)
-        )
-
     def test_bmc_extract_ip_address_from_url_blank_gives_none(self):
         self.assertEqual(None, BMC.extract_ip_address("virsh", None))
         self.assertEqual(None, BMC.extract_ip_address("virsh", {}))
@@ -459,6 +451,13 @@ class TestBMC(MAASServerTestCase):
         power_parameters = {"power_address": "http://:8080/foo/#baz"}
         self.assertEqual("", BMC.extract_ip_address("virsh", power_parameters))
 
+    def test_bmc_extract_ip_address_with_fqdn_returns_none(self):
+        self.assertIsNone(
+            BMC.extract_ip_address(
+                "webhook", {"power_address": factory.make_url()}
+            )
+        )
+
     def test_get_usable_rack_controllers_returns_empty_when_none(self):
         bmc = factory.make_BMC()
         self.assertThat(bmc.get_usable_rack_controllers(), HasLength(0))