Add in additional checks for ip validity

src/registrar/models/domain.py

@@ -1,4 +1,5 @@
 import logging
+import ipaddress
 
 from datetime import date
 from string import digits
@@ -298,8 +299,10 @@ class Domain(TimeStampedModel, DomainHelper):
 
     def checkHostIPCombo(self, nameserver: str, ip: list):
         if self.isSubdomain(nameserver) and (ip is None or ip == []):
-            raise ValueError("Nameserver %s needs to have an "
+            raise ValueError(
-                "ip address because it is a subdomain" % nameserver)
+                "Nameserver %s needs to have an "
+                "ip address because it is a subdomain" % nameserver
+            )
         elif not self.isSubdomain(nameserver) and (ip is not None and ip != []):
             raise ValueError(
                 "Nameserver %s cannot be linked "
@@ -307,7 +310,22 @@ class Domain(TimeStampedModel, DomainHelper):
             )
         return None
 
-    def getNameserverChanges(self, hosts: list[tuple[str]]) -> tuple[list, list, dict, list]:
+    # TODO-848: We are checking for valid ip address format
+    # Need to use before checkHostIPCombo, or discuss where best fit
+    # And confirm if AddressValueError is best choice of error to raise
+    # def _valid_ip_addr(self):
+    #     if ipaddress.IPv6Address(ip) or ipaddress.IPv4Address(ip):
+    #         return True
+    #     else:
+    #         # We will need to import this error
+    #         raise AddressValueError(
+    #             "IP Address is in an invalid format."
+    #         )
+    #         return None
+
+    def getNameserverChanges(
+        self, hosts: list[tuple[str]]
+    ) -> tuple[list, list, dict, list]:
         """
         calls self.nameserver, it should pull from cache but may result
         in an epp call
@@ -1078,9 +1096,9 @@ class Domain(TimeStampedModel, DomainHelper):
 
                 raise e
 
-    # TODO: Need to implement this
     def is_ipv6(self, ip: str):
-        return True
+        ip_addr = ipaddress.ip_address(ip)
+        return ip_addr.version == 6
 
     def _convert_ips(self, ip_list: list[str]):
         edited_ip_list = []

src/registrar/tests/common.py

@@ -616,7 +616,7 @@ class MockEppLib(TestCase):
     mockDataInfoHosts = fakedEppObject(
         "lastPw",
         cr_date=datetime.datetime(2023, 8, 25, 19, 45, 35),
-        addrs=["1.2.3", "2.3.4"],
+        addrs=["1.2.3.4", "2.3.4.5"],
     )
 
     mockDataHostChange = fakedEppObject(
@@ -631,7 +631,7 @@ class MockEppLib(TestCase):
             "ns2.nameserverwithip.gov",
             "ns3.nameserverwithip.gov",
         ],
-        addrs=["1.2.3", "2.3.4"],
+        addrs=["1.2.3.4", "2.3.4.5"],
     )
 
     # TODO-848: Fix naming

src/registrar/tests/test_models_domain.py

@@ -565,9 +565,7 @@ class TestRegistrantNameservers(MockEppLib):
         self.assertEqual(new_values, {})
         self.assertEqual(
             oldNameservers,
-            {
+            {"ns1.example.com": None, "ns2.example.com": ["1.2.3"]},
-            'ns1.example.com': None, 'ns2.example.com': ['1.2.3']
-            },
         )
 
     def test_get_nameserver_changes_success_updated_vals(self):
@@ -590,9 +588,7 @@ class TestRegistrantNameservers(MockEppLib):
         self.assertEqual(new_values, {})
         self.assertEqual(
             oldNameservers,
-            {
+            {"ns3.my-nameserver.gov": ["1.2.3"]},
-            "ns3.my-nameserver.gov": ["1.2.3"]
-            },
         )
 
     def test_get_nameserver_changes_success_new_vals(self):
@@ -635,7 +631,7 @@ class TestRegistrantNameservers(MockEppLib):
         nameserver = "ns1.my-nameserver.com"
         self.domain.nameservers = [(nameserver,)]
 
-        # when you create a host, you also have to update at same time
+        # when we create a host, we should've updated at the same time
         created_host = commands.CreateHost(nameserver)
         update_domain_with_created = commands.UpdateDomain(
             name=self.domain.name, add=[common.HostObjSet([created_host.name])]
@@ -934,9 +930,12 @@ class TestRegistrantNameservers(MockEppLib):
             name="nameserverwithip.gov", state=Domain.State.READY
         )
         domain.nameservers = [
-            ("ns1.nameserverwithip.gov", ["2.3.4", "1.2.3"]),
+            ("ns1.nameserverwithip.gov", ["2.3.4.5", "1.2.3.4"]),
-            ("ns2.nameserverwithip.gov", ["1.2.3", "2.3.4", "3.4.5"]),
+            (
-            ("ns3.nameserverwithip.gov", ["2.3.4"]),
+                "ns2.nameserverwithip.gov",
+                ["1.2.3.4", "2.3.4.5", "2001:0db8:85a3:0000:0000:8a2e:0370:7334"],
+            ),
+            ("ns3.nameserverwithip.gov", ["2.3.4.5"]),
         ]
 
         expectedCalls = [
@@ -950,7 +949,11 @@ class TestRegistrantNameservers(MockEppLib):
             call(
                 commands.UpdateHost(
                     name="ns2.nameserverwithip.gov",
-                    add=[common.Ip(addr="3.4.5", ip="v6")],
+                    add=[
+                        common.Ip(
+                            addr="2001:0db8:85a3:0000:0000:8a2e:0370:7334", ip="v6"
+                        )
+                    ],
                     rem=[],
                     chg=None,
                 ),
@@ -960,7 +963,7 @@ class TestRegistrantNameservers(MockEppLib):
                 commands.UpdateHost(
                     name="ns3.nameserverwithip.gov",
                     add=[],
-                    rem=[common.Ip(addr="1.2.3", ip="v6")],
+                    rem=[common.Ip(addr="1.2.3.4", ip=None)],
                     chg=None,
                 ),
                 cleaned=True,