Refactor MAC to IP association checking (#361)

CHANGELOG.md

@@ -5,7 +5,11 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
-<!-- ## Unreleased -->
+## Unreleased
+
+### Fixed
+
+- `dhcp assoc` and `host add` commands displaying unique constraint error instead of a proper error message when a MAC is already associated with an IP.
 
 ## [1.2.1](https://github.com/unioslo/mreg-cli/releases/tag/1.2.1) - 2024-12-02
 

ci/testsuite-result.json

@@ -27485,7 +27485,7 @@
     "command_issued": "host add -ip 10.0.0.0/24 -macaddress aa:bb:cc:cc:bb:aa directfail",
     "ok": [],
     "warning": [
-      "MAC address aa:bb:cc:cc:bb:aa is already associated with 10.0.0.4"
+      "MAC address aa:bb:cc:cc:bb:aa is already associated with IP address 10.0.0.184, must force."
     ],
     "error": [],
     "output": [],
@@ -27502,7 +27502,7 @@
           "results": [
             {
               "macaddress": "aa:bb:cc:cc:bb:aa",
-              "ipaddress": "10.0.0.4",
+              "ipaddress": "10.0.0.184",
               "host": 18
             }
           ]

mreg_cli/api/models.py

@@ -1991,13 +1991,56 @@ def get_by_ip(cls, ip: IP_AddressT) -> list[Self]:
         return cls.get_list_by_field("ipaddress", str(ip))
 
     @classmethod
-    def get_by_mac(cls, mac: MacAddress) -> IPAddress | None:
+    def ensure_associable(cls, mac: MacAddress, force: bool) -> None:
+        """Check if a MAC address is available to be associated with an IP address.
+
+        Raise an exception if the MAC address is already associated with an IP address,
+        and force is not set.
+
+        :param mac: The MAC address to check.
+        :param force: Force is active. If True, the check is skipped.
+        :raises EntityAlreadyExists: If the MAC address is already associated with one IP.
+        :raises MultipleEntitiesFound: If the MAC address is already associated multiple IPs.
+        """
+        if force:
+            return
+
+        ips = cls.get_ips_by_mac(mac)
+        if not ips:
+            return
+
+        if len(ips) == 1:
+            raise EntityAlreadyExists(
+                f"MAC address {mac} is already associated with IP address {ips[0].ipaddress}"
+                ", must force."
+            )
+        else:
+            ips_str = ", ".join([str(ip.ipaddress) for ip in ips])
+            raise MultipleEntitiesFound(
+                f"MAC address {mac} is already associated with multiple IP addresses: {ips_str}"
+                ", must force."
+            )
+
+    @classmethod
+    def get_by_mac(cls, mac: MacAddress) -> Self | None:
         """Get the IP address objects by MAC address.
 
         :param mac: The MAC address to search for.
         :returns: The IP address if found, None otherwise.
         """
-        return cls.get_by_field("macaddress", mac)
+        try:
+            return cls.get_by_field("macaddress", mac)
+        except MultipleEntitiesFound as e:
+            raise MultipleEntitiesFound(f"Multiple IPs found with MAC address {mac}.") from e
+
+    @classmethod
+    def get_ips_by_mac(cls, mac: MacAddress) -> list[Self]:
+        """Get a list of IP addresses by MAC address.
+
+        :param mac: The MAC address to search for.
+        :returns: A list of IP addresses.
+        """
+        return cls.get_list_by_field("macaddress", mac)
 
     @classmethod
     def endpoint(cls) -> Endpoint:
@@ -2038,7 +2081,6 @@ def associate_mac(self, mac: MacAddress, force: bool = False) -> IPAddress:
             raise EntityAlreadyExists(
                 f"IP address {self.ipaddress} already has MAC address {self.macaddress}."
             )
-
         return self.patch(fields={"macaddress": mac})
 
     def disassociate_mac(self) -> IPAddress:
@@ -2918,9 +2960,9 @@ def associate_mac_to_ip(
 
         if len(ipadresses) and not force:
             raise EntityOwnershipMismatch(
-                "mac {} already in use by: {}. Use force to add {} -> {} as well.".format(
-                    mac, ipadresses, ip, mac
-                )
+                f"mac {mac} already in use by: "
+                f"{', '.join(str(ip.ipaddress) for ip in ipadresses)}. "
+                f"Use force to add {ip} -> {mac} as well."
             )
 
         ip_found_in_host = False

mreg_cli/commands/dhcp.py

@@ -70,9 +70,7 @@ def assoc(args: argparse.Namespace) -> None:
     force: bool = args.force
 
     mac = MacAddress.parse_or_raise(mac)
-    in_use = IPAddress.get_by_mac(mac)
-    if in_use:
-        raise InputFailure(f"MAC {mac} is already in use by {in_use.ipaddress}.")
+    IPAddress.ensure_associable(mac, force=force)
 
     ipaddress = ipaddress_from_ip_arg(name)
     if not ipaddress:

mreg_cli/commands/host_submodules/core.py

@@ -89,18 +89,14 @@ def add(args: argparse.Namespace) -> None:
     :param args: argparse.Namespace (name, ip, contact, comment, force, macaddress)
 
     """
-    network_or_ip: str = args.ip
     hname = HostName.parse_or_raise(args.name)
+    network_or_ip: str = args.ip
     macaddress: str | None = args.macaddress
+    force: bool = args.force
 
     if macaddress is not None:
         macaddress = MacAddress.parse_or_raise(macaddress)
-        ip_address = IPAddress.get_by_mac(macaddress)
-
-        if ip_address:
-            raise EntityAlreadyExists(
-                f"MAC address {macaddress} is already associated with {ip_address}"
-            )
+        IPAddress.ensure_associable(macaddress, force=force)
 
     host = Host.get_by_any_means(hname)
     if host:
@@ -110,12 +106,12 @@ def add(args: argparse.Namespace) -> None:
             raise EntityAlreadyExists(f"Host {hname} already exists.")
 
     zone = ForwardZone.get_from_hostname(hname)
-    if not zone and not args.force:
+    if not zone and not force:
         raise ForceMissing(f"{hname} isn't in a zone controlled by MREG, must force")
-    if zone and zone.is_delegated() and not args.force:
+    if zone and zone.is_delegated() and not force:
         raise ForceMissing(f"{hname} is in zone delegation {zone.name}, must force")
 
-    if "*" in hname and not args.force:
+    if "*" in hname and not force:
         raise ForceMissing("Wildcards must be forced.")
 
     data: JsonMapping = {
@@ -153,7 +149,7 @@ def add(args: argparse.Namespace) -> None:
                             f"IP {ipaddr} is a broadcast address, not a host address"
                         )
             except (EntityNotFound, APINotOk) as e:
-                if not args.force:
+                if not force:
                     raise ForceMissing(f"IP {ipaddr} is not in a network, must force") from e
             data["ipaddress"] = str(network_or_ip)
 
@@ -172,7 +168,7 @@ def add(args: argparse.Namespace) -> None:
             raise EntityNotFound(f"Invalid ip or network: {network_or_ip}")
 
         if network:
-            if network.frozen and not args.force:
+            if network.frozen and not force:
                 raise ForceMissing(f"Network {network.network} is frozen, must force")
             else:
                 net_or_ip = NetworkOrIP.validate(network.network)
@@ -186,15 +182,17 @@ def add(args: argparse.Namespace) -> None:
 
     if macaddress is not None and net_or_ip is not None:
         if net_or_ip.is_ip():
-            host = host.associate_mac_to_ip(macaddress, str(network_or_ip))
+            host = host.associate_mac_to_ip(macaddress, network_or_ip, force=force)
         else:
             # We passed a network to create the host, so we need to find the IP
             # that was assigned to the host. We don't get that in the response
             # per se, so we check to see if there is only one IP in the host and
             # use that. If there are more than one, we can't know which one was
             # assigned to the host during create, so we abort.
             if len(host.ipaddresses) == 1:
-                host = host.associate_mac_to_ip(macaddress, host.ipaddresses[0].ipaddress)
+                host = host.associate_mac_to_ip(
+                    macaddress, host.ipaddresses[0].ipaddress, force=force
+                )
             else:
                 OutputManager().add_ok(
                     "Failed to associate MAC address to IP, multiple IP addresses after creation."