Refactor host remove override check, add CNAME+MX confirmation

Author: pederhan

ci/testsuite-result.json

@@ -26410,7 +26410,7 @@
     "command_issued": "host remove bar # should fail, because it has a cname record, must force and override with 'cname'",
     "ok": [],
     "warning": [
-      "bar.example.org requires force and override for deletion:\n  1 cnames\n    - fubar.example.org\n  multiple ipaddresses on the same VLAN\nUse `-force -override cname` to override."
+      "bar.example.org requires force and override for deletion:\n  1 CNAME records\n    - fubar.example.org\n  multiple ipaddresses on the same VLAN\nUse `-force -override cname` to override."
     ],
     "error": [],
     "output": [],
@@ -26424,8 +26424,8 @@
           "excluded_ranges": [],
           "policy": null,
           "communities": [],
-          "created_at": "2025-12-03T13:44:43.360753+01:00",
-          "updated_at": "2025-12-03T13:44:43.360763+01:00",
+          "created_at": "2026-03-27T11:43:07.055968+01:00",
+          "updated_at": "2026-03-27T11:43:07.055977+01:00",
           "network": "10.0.0.0/24",
           "description": "lorem ipsum",
           "vlan": null,
@@ -26446,8 +26446,8 @@
           "excluded_ranges": [],
           "policy": null,
           "communities": [],
-          "created_at": "2025-12-03T13:44:43.360753+01:00",
-          "updated_at": "2025-12-03T13:44:43.360763+01:00",
+          "created_at": "2026-03-27T11:43:07.055968+01:00",
+          "updated_at": "2026-03-27T11:43:07.055977+01:00",
           "network": "10.0.0.0/24",
           "description": "lorem ipsum",
           "vlan": null,
@@ -27189,7 +27189,7 @@
     "command_issued": "host remove baz # Should fail, because it has an MX record, must force and override with 'mx'",
     "ok": [],
     "warning": [
-      "baz.example.org requires force and override for deletion:\n  multiple ipaddresses on the same VLAN\n  1 MX records\n    - mail.example.org (priority: 10)\nUse `-force -override mx` to override."
+      "baz.example.org requires force and override for deletion:\n  1 MX records\n    - mail.example.org (priority: 10)\n  multiple ipaddresses on the same VLAN\nUse `-force -override mx` to override."
     ],
     "error": [],
     "output": [],
@@ -27203,8 +27203,8 @@
           "excluded_ranges": [],
           "policy": null,
           "communities": [],
-          "created_at": "2025-12-03T13:44:43.360753+01:00",
-          "updated_at": "2025-12-03T13:44:43.360763+01:00",
+          "created_at": "2026-03-27T11:43:07.055968+01:00",
+          "updated_at": "2026-03-27T11:43:07.055977+01:00",
           "network": "10.0.0.0/24",
           "description": "lorem ipsum",
           "vlan": null,
@@ -27225,8 +27225,8 @@
           "excluded_ranges": [],
           "policy": null,
           "communities": [],
-          "created_at": "2025-12-03T13:44:43.439716+01:00",
-          "updated_at": "2025-12-03T13:44:43.439728+01:00",
+          "created_at": "2026-03-27T11:43:07.091223+01:00",
+          "updated_at": "2026-03-27T11:43:07.091237+01:00",
           "network": "2001:db8::/64",
           "description": "dolor sit amet",
           "vlan": null,
@@ -30576,6 +30576,7 @@
     "command_filter_negate": false,
     "command_issued": "host remove foo -force -override mx",
     "ok": [
+      "deleted MX record mail.example.org (priority: 10) when removing foo.example.org",
       "removed foo.example.org"
     ],
     "warning": [],
@@ -32287,7 +32288,7 @@
     "command_issued": "host remove foo # Should fail, because it has a CNAME record, must force and override with 'cname'",
     "ok": [],
     "warning": [
-      "foo.example.org requires force and override for deletion:\n  1 cnames\n    - fubar.example.org\nUse `-force -override cname` to override."
+      "foo.example.org requires force and override for deletion:\n  1 CNAME records\n    - fubar.example.org\nUse `-force -override cname` to override."
     ],
     "error": [],
     "output": [],
@@ -32300,6 +32301,7 @@
     "command_filter_negate": false,
     "command_issued": "host remove foo -force -override cname",
     "ok": [
+      "deleted CNAME record fubar.example.org when removing foo.example.org",
       "removed foo.example.org"
     ],
     "warning": [],
@@ -33345,7 +33347,7 @@
     "command_issued": "host remove foo # Should fail, because it has multiple records, must force and override with everything.",
     "ok": [],
     "warning": [
-      "foo.example.org requires force and override for deletion:\n  1 cnames\n    - fubar.example.org\n  1 MX records\n    - mail.example.org (priority: 10)\n  1 NAPTR records\n    - wonk\n  1 SRV records\n    - _sip._tcp.example.org\n  1 PTR records\n    - 10.0.0.11\nUse `-force -override cname,mx,naptr,ptr,srv` to override."
+      "foo.example.org requires force and override for deletion:\n  1 CNAME records\n    - fubar.example.org\n  1 MX records\n    - mail.example.org (priority: 10)\n  1 SRV records\n    - _sip._tcp.example.org\n  1 PTR records\n    - 10.0.0.11\n  1 NAPTR records\n    - wonk\nUse `-force -override cname,mx,naptr,ptr,srv` to override."
     ],
     "error": [],
     "output": [],
@@ -33358,9 +33360,11 @@
     "command_filter_negate": false,
     "command_issued": "host remove foo -force -override mx,ptr,naptr,srv,cname",
     "ok": [
-      "deleted NAPTR record wonk when removing foo.example.org",
+      "deleted CNAME record fubar.example.org when removing foo.example.org",
+      "deleted MX record mail.example.org (priority: 10) when removing foo.example.org",
       "deleted SRV record _sip._tcp.example.org when removing foo.example.org",
       "deleted PTR record 10.0.0.11 when removing foo.example.org",
+      "deleted NAPTR record wonk when removing foo.example.org",
       "removed foo.example.org"
     ],
     "warning": [],

mreg_cli/commands/host_submodules/core.py

@@ -16,9 +16,11 @@
 import argparse
 import re
 from enum import Enum
-from typing import Self, assert_never
+from typing import Any, Generic, NamedTuple, Self, TypeVar
 
 from mreg_api.models import (
+    CNAME,
+    MX,
     NAPTR,
     ForwardZone,
     Host,
@@ -29,6 +31,7 @@
     PTR_override,
     Srv,
 )
+from mreg_api.models.abstracts import FrozenModel
 from mreg_api.models.fields import HostName, MacAddress
 from typing_extensions import override
 
@@ -274,21 +277,34 @@ def parse_overrides(cls, overrides: str) -> list[Override]:
         ]
 
 
-def get_record_identifier(record: NAPTR | PTR_override | Srv) -> str:
+def get_record_identifier(record: CNAME | MX | NAPTR | PTR_override | Srv) -> str:
     """Get a human readable identifier for a record.
 
     :param record: The record to get the identifier for.
     :returns: A human readable identifier for the record.
     """
     match record:
-        case NAPTR():
-            return record.replacement
+        case CNAME() | Srv():
+            return record.name
         case PTR_override():
             return str(record.ipaddress)
-        case Srv():
-            return record.name
+        case NAPTR():
+            return record.replacement
+        case MX():
+            return f"{record.mx} (priority: {record.priority})"
         case _:
-            assert_never(record)
+            return repr(record)  # pyright: ignore[reportUnreachable] # for safety
+
+
+T = TypeVar("T")
+
+
+class OverrideRequired(NamedTuple, Generic[T]):
+    """Check for overrides required for a specific host record type."""
+
+    override: Override
+    name: str
+    items: list[T]
 
 
 @command_registry.register_command(
@@ -340,15 +356,25 @@ def forced(override_required: Override | None = None) -> bool:
         # And the fallback is "no".
         return False
 
+    override_checks: list[OverrideRequired[Any]] = [
+        OverrideRequired(Override.CNAME, "CNAME", host.cnames),
+        OverrideRequired(Override.MX, "MX", host.mxs),
+        OverrideRequired(Override.SRV, "SRV", host.srvs),
+        OverrideRequired(Override.PTR, "PTR", host.ptr_overrides),
+        OverrideRequired(Override.NAPTR, "NAPTR", host.naptrs),
+    ]
+
+    # Determine required overrides and build warning message
+    # if force and override requirements are not met
     warnings: list[str] = []
     overrides_required: set[Override] = set()
-
-    # Require force if host has any cnames.
-    if host.cnames and not forced(Override.CNAME):
-        overrides_required.add(Override.CNAME)
-        warnings.append(f"  {len(host.cnames)} cnames")
-        for cname in host.cnames:
-            warnings.append(f"    - {cname.name}")
+    for check in override_checks:
+        if check.items and not forced(check.override):
+            overrides_required.add(check.override)
+            warnings.append(f"  {len(check.items)} {check.name} records")
+            for item in check.items:
+                value = get_record_identifier(item)
+                warnings.append(f"    - {value}")
 
     # Require force if host has multiple A/AAAA records and they are not in the same VLAN.
     if len(host.ipaddresses) > 1:
@@ -365,39 +391,6 @@ def forced(override_required: Override | None = None) -> bool:
                 ip_strings.sort()
                 warnings.append(f"    - {', '.join(ip_strings)} (vlan: {vlan_id})")
 
-    if host.mxs and not forced(Override.MX):
-        overrides_required.add(Override.MX)
-        warnings.append(f"  {len(host.mxs)} MX records")
-        for mx in host.mxs:
-            warnings.append(f"    - {mx.mx} (priority: {mx.priority})")
-
-    # Require force if host has any NAPTR records. Delete the NAPTR records if
-    # force
-    naptrs = host.naptrs
-    if len(naptrs) > 0:
-        if not forced(Override.NAPTR):
-            overrides_required.add(Override.NAPTR)
-            warnings.append(f"  {len(naptrs)} NAPTR records")
-            for naptr in naptrs:
-                warnings.append(f"    - {naptr.replacement}")
-
-    # Require force if host has any SRV records. Delete the SRV records if force
-    srvs = host.srvs
-    if len(srvs) > 0:
-        if not forced(Override.SRV):
-            overrides_required.add(Override.SRV)
-            warnings.append(f"  {len(srvs)} SRV records")
-            for srv in srvs:
-                warnings.append(f"    - {srv.name}")
-
-    # Require force if host has any PTR records. Delete the PTR records if force
-    if len(host.ptr_overrides) > 0:
-        if not forced(Override.PTR):
-            overrides_required.add(Override.PTR)
-            warnings.append(f"  {len(host.ptr_overrides)} PTR records")
-            for ptr in host.ptr_overrides:
-                warnings.append(f"    - {ptr.ipaddress}")
-
     # Warn user and raise exception if any force requirements was found
     if warnings:
         # Build the override command suggestion
@@ -428,16 +421,13 @@ def forced(override_required: Override | None = None) -> bool:
     if not host.delete():
         raise DeleteError(f"failed to remove {host.name}")
 
-    for record_name, record in [
-        ("NAPTR", host.naptrs),
-        ("SRV", host.srvs),
-        ("PTR", host.ptr_overrides),
-    ]:
-        for rec in record:
+    # Print messages for associated records that were deleted
+    for check in override_checks:
+        for item in check.items:
             OutputManager().add_ok(
                 (
-                    f"deleted {record_name} record "
-                    f"{get_record_identifier(rec)} when removing {host.name}"
+                    f"deleted {check.name} record "
+                    f"{get_record_identifier(item)} when removing {host.name}"
                 )
             )
     OutputManager().add_ok(f"removed {host.name}")

tests/commands/test_host.py

@@ -5,7 +5,7 @@
 
 import pytest
 from inline_snapshot import snapshot
-from mreg_api.models import NAPTR, PTR_override, Srv
+from mreg_api.models import CNAME, MX, NAPTR, SSHFP, PTR_override, Srv
 
 from mreg_cli.commands.host_submodules.core import Override, get_record_identifier
 from mreg_cli.exceptions import InputFailure
@@ -117,8 +117,48 @@ def test_override_from_string() -> None:
             ),
             "srv.example.com",
         ),
+        pytest.param(
+            CNAME(
+                host=123,
+                created_at=datetime.datetime(2026, 1, 1),
+                updated_at=datetime.datetime(2026, 1, 1),
+                id=1,
+                name="alias.example.com",  # pyright: ignore[reportArgumentType]
+            ),
+            "alias.example.com",
+        ),
+        pytest.param(
+            MX(
+                host=123,
+                created_at=datetime.datetime(2026, 1, 1),
+                updated_at=datetime.datetime(2026, 1, 1),
+                id=1,
+                mx="mail.example.com",
+                priority=10,
+            ),
+            "mail.example.com (priority: 10)",
+        ),
     ],
 )
-def test_get_record_identifier(record: NAPTR | PTR_override | Srv, expected: str) -> None:
+def test_get_record_identifier(
+    record: NAPTR | PTR_override | Srv | CNAME | MX, expected: str
+) -> None:
     """Test get_record_identifier with different record types."""
     assert get_record_identifier(record) == expected
+
+
+def test_get_record_identifier_unknown() -> None:
+    """Test get_record_identifier repr() fallback with an unhandled record type."""
+    record = SSHFP(
+        host=123,
+        created_at=datetime.datetime(2026, 1, 1),
+        updated_at=datetime.datetime(2026, 1, 1),
+        id=1,
+        algorithm=1,
+        hash_type=2,
+        fingerprint="abc123",
+        ttl=3600,
+    )
+    assert get_record_identifier(record) == snapshot(  # pyright: ignore[reportArgumentType]
+        "SSHFP(host=123, created_at=datetime.datetime(2026, 1, 1, 0, 0), updated_at=datetime.datetime(2026, 1, 1, 0, 0), id=1, algorithm=1, hash_type=2, fingerprint='abc123', ttl=3600)"
+    )