]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
Make properties direct attribute of KeyProperties
authorMatthijs Mekking <matthijs@isc.org>
Thu, 2 Oct 2025 08:22:54 +0000 (10:22 +0200)
committerMatthijs Mekking <matthijs@isc.org>
Wed, 8 Oct 2025 07:56:58 +0000 (09:56 +0200)
There is no real reason to keep those in a dictionary.

bin/tests/system/isctest/kasp.py
bin/tests/system/kasp/tests_kasp.py
bin/tests/system/migrate2kasp/tests_migrate2kasp.py
bin/tests/system/rollover-multisigner/tests_rollover_multisigner.py

index 16f968bc10fd3a4cfc75fbf9b80247b01eaa9d67..0fac77ff45340fc41f72bcbba156160d7497395f 100644 (file)
@@ -171,15 +171,30 @@ class KeyProperties:
     def __init__(
         self,
         name: str,
-        properties: dict,
         metadata: dict,
         timing: Dict[str, KeyTimingMetadata],
+        private: bool = True,
+        legacy: bool = False,
+        role: str = "csk",
+        ttl: int = 3600,
+        flags: int = 257,
+        keytag_min: int = 0,
+        keytag_max: int = 65535,
+        offset: Union[timedelta, int] = 0,
     ):
         self.name = name
         self.key = None
-        self.properties = properties
         self.metadata = metadata
         self.timing = timing
+        # Properties
+        self.private = private
+        self.legacy = legacy
+        self.role = role
+        self.ttl = ttl
+        self.flags = flags
+        self.keytag_min = keytag_min
+        self.keytag_max = keytag_max
+        self.offset = offset
 
     def __repr__(self):
         return self.name
@@ -189,14 +204,6 @@ class KeyProperties:
 
     @staticmethod
     def default(with_state=True) -> "KeyProperties":
-        properties = {
-            "private": True,
-            "legacy": False,
-            "role": "csk",
-            "role_full": "key-signing",
-            "dnskey_ttl": 3600,
-            "flags": 257,
-        }
         metadata = {
             "Algorithm": isctest.vars.algorithms.ECDSAP256SHA256.number,
             "Length": 256,
@@ -206,9 +213,7 @@ class KeyProperties:
         }
         timing: Dict[str, KeyTimingMetadata] = {}
 
-        result = KeyProperties(
-            name="DEFAULT", properties=properties, metadata=metadata, timing=timing
-        )
+        result = KeyProperties(name="DEFAULT", metadata=metadata, timing=timing)
         result.name = "DEFAULT"
         result.key = None
         if with_state:
@@ -220,6 +225,11 @@ class KeyProperties:
 
         return result
 
+    def role_full(self) -> str:
+        if self.flags == 256:
+            return "zone-signing"
+        return "key-signing"
+
     def Ipub(self, config):
         ipub = timedelta(0)
 
@@ -257,11 +267,11 @@ class KeyProperties:
         if self.key is None:
             raise ValueError("KeyProperties must be attached to a Key")
 
-        if self.properties["legacy"]:
+        if self.legacy:
             return
 
         if offset is None:
-            offset = self.properties["offset"]
+            offset = self.offset
 
         self.timing["Generated"] = self.key.get_timing("Created")
         self.timing["Published"] = self.key.get_timing("Created")
@@ -555,14 +565,14 @@ class Key:
         # Noop. If file is missing then the get_metadata calls will fail.
 
         # Check the public key file.
-        role = properties.properties["role_full"]
+        role = properties.role_full()
         comment = f"This is a {role} key, keyid {self.tag}, for {zone}."
         if not isctest.util.file_contents_contain(self.keyfile, comment):
             isctest.log.debug(f"{self.name} COMMENT MISMATCH: expected '{comment}'")
             return False
 
-        ttl = properties.properties["dnskey_ttl"]
-        flags = properties.properties["flags"]
+        ttl = properties.ttl
+        flags = properties.flags
         alg = properties.metadata["Algorithm"]
         dnskey = f"{zone}. {ttl} IN DNSKEY {flags} 3 {alg}"
         if not isctest.util.file_contents_contain(self.keyfile, dnskey):
@@ -570,7 +580,7 @@ class Key:
             return False
 
         # Now check the private key file.
-        if properties.properties["private"]:
+        if properties.private:
             # Retrieve creation date.
             created = self.get_metadata("Generated")
 
@@ -594,7 +604,7 @@ class Key:
                 return False
 
         # Now check the key state file.
-        if properties.properties["legacy"]:
+        if properties.legacy:
             return True
 
         comment = f"This is the state of key {self.tag}, for {zone}."
@@ -619,12 +629,10 @@ class Key:
                 return False
 
         # Check tag range.
-        if "keytag-min" in properties.properties:
-            if self.tag < properties.properties["keytag-min"]:
-                return False
-        if "keytag-max" in properties.properties:
-            if self.tag > properties.properties["keytag-max"]:
-                return False
+        if self.tag < properties.keytag_min:
+            return False
+        if self.tag > properties.keytag_max:
+            return False
 
         # A match is found.
         return True
@@ -692,7 +700,7 @@ def check_keys(zone, keys, expected):
                 if expected[i].key is None:
                     found = key.match_properties(zone, expected[i])
                     if found:
-                        key.external = expected[i].properties["legacy"]
+                        key.external = expected[i].legacy
                         expected[i].key = key
                 i += 1
             if not found:
@@ -714,7 +722,7 @@ def check_keytimes(keys, expected):
 
     for key in keys:
         for expect in expected:
-            if expect.properties["legacy"]:
+            if expect.legacy:
                 continue
 
             if not key is expect.key:
@@ -739,9 +747,9 @@ def check_keytimes(keys, expected):
                 synonyms["Delete"] = expect.timing["Removed"]
 
             assert key.match_timingmetadata(synonyms, file=key.keyfile, comment=True)
-            if expect.properties["private"]:
+            if expect.private:
                 assert key.match_timingmetadata(synonyms, file=key.privatefile)
-            if not expect.properties["legacy"]:
+            if not expect.legacy:
                 assert key.match_timingmetadata(expect.timing)
 
                 state_changes = [
@@ -762,7 +770,7 @@ def check_keyrelationships(keys, expected):
     """
     for key in keys:
         for expect in expected:
-            if expect.properties["legacy"]:
+            if expect.legacy:
                 continue
 
             if not key is expect.key:
@@ -1518,59 +1526,75 @@ def policy_to_properties(ttl, keys: List[str]) -> List[KeyProperties]:
     for key in keys:
         count += 1
         line = key.split()
-        keyprop = KeyProperties(f"KEY{count}", {}, {}, {})
-        keyprop.properties["private"] = True
-        keyprop.properties["legacy"] = False
-        keyprop.properties["offset"] = timedelta(0)
-        keyprop.properties["role"] = line[0]
-        if line[0] == "zsk":
-            keyprop.properties["role_full"] = "zone-signing"
-            keyprop.properties["flags"] = 256
-            keyprop.metadata["ZSK"] = "yes"
-            keyprop.metadata["KSK"] = "no"
+
+        # defaults
+        metadata: Dict[str, Union[str, int]] = {}
+        timing: Dict[str, KeyTimingMetadata] = {}
+        private = True
+        legacy = False
+        keytag_min = 0
+        keytag_max = 65535
+        offset = timedelta(0)
+
+        role = line[0]
+        if role == "zsk":
+            flags = 256
+            metadata["ZSK"] = "yes"
+            metadata["KSK"] = "no"
         else:
-            keyprop.properties["role_full"] = "key-signing"
-            keyprop.properties["flags"] = 257
-            keyprop.metadata["ZSK"] = "yes" if line[0] == "csk" else "no"
-            keyprop.metadata["KSK"] = "yes"
-
-        keyprop.properties["dnskey_ttl"] = ttl
-        keyprop.metadata["Algorithm"] = line[2]
-        keyprop.metadata["Length"] = line[3]
+            flags = 257
+            metadata["ZSK"] = "yes" if role == "csk" else "no"
+            metadata["KSK"] = "yes"
+
+        metadata["Algorithm"] = line[2]
+        metadata["Length"] = line[3]
         if line[1] == "unlimited":
-            keyprop.metadata["Lifetime"] = 0
+            metadata["Lifetime"] = 0
         elif line[1] != "-":
-            keyprop.metadata["Lifetime"] = int(line[1])
+            metadata["Lifetime"] = int(line[1])
 
         for i in range(4, len(line)):
             if line[i].startswith("goal:"):
                 keyval = line[i].split(":")
-                keyprop.metadata["GoalState"] = keyval[1]
+                metadata["GoalState"] = keyval[1]
             elif line[i].startswith("dnskey:"):
                 keyval = line[i].split(":")
-                keyprop.metadata["DNSKEYState"] = keyval[1]
+                metadata["DNSKEYState"] = keyval[1]
             elif line[i].startswith("krrsig:"):
                 keyval = line[i].split(":")
-                keyprop.metadata["KRRSIGState"] = keyval[1]
+                metadata["KRRSIGState"] = keyval[1]
             elif line[i].startswith("zrrsig:"):
                 keyval = line[i].split(":")
-                keyprop.metadata["ZRRSIGState"] = keyval[1]
+                metadata["ZRRSIGState"] = keyval[1]
             elif line[i].startswith("ds:"):
                 keyval = line[i].split(":")
-                keyprop.metadata["DSState"] = keyval[1]
+                metadata["DSState"] = keyval[1]
             elif line[i].startswith("offset:"):
                 keyval = line[i].split(":")
-                keyprop.properties["offset"] = timedelta(seconds=int(keyval[1]))
+                offset = timedelta(seconds=int(keyval[1]))
             elif line[i].startswith("tag-range:"):
                 keyval = line[i].split(":")
                 tagrange = keyval[1].split("-")
-                keyprop.properties["keytag-min"] = int(tagrange[0])
-                keyprop.properties["keytag-max"] = int(tagrange[1])
+                keytag_min = int(tagrange[0])
+                keytag_max = int(tagrange[1])
             elif line[i] == "missing":
-                keyprop.properties["private"] = False
+                private = False
             else:
                 assert False, f"undefined optional data {line[i]}"
 
+        keyprop = KeyProperties(
+            name=f"KEY{count}",
+            metadata=metadata,
+            timing=timing,
+            private=private,
+            legacy=legacy,
+            role=role,
+            ttl=ttl,
+            flags=flags,
+            keytag_min=keytag_min,
+            keytag_max=keytag_max,
+            offset=offset,
+        )
         proplist.append(keyprop)
 
     return proplist
index 4844e13bf086ac04d869b74021a7d9050886234e..c494b7a13cb99661e798f1dbbf6bddd372e1a28b 100644 (file)
@@ -912,7 +912,7 @@ def test_kasp_default(ns3):
         ns3.rndc(f"loadkeys {zone}", log=False)
         watcher.wait_for_line(f"zone {zone}/IN (signed): {expectmsg}")
     # Nothing has changed.
-    expected[0].properties["private"] = False
+    expected[0].private = False  # noqa
     isctest.kasp.check_dnssec_verify(ns3, zone)
     isctest.kasp.check_keys(zone, keys, expected)
     isctest.kasp.check_keytimes(keys, expected)
index 92642c1f29833647d0b8c848cdc5efa28828f888..426ef6d32a8aec2482a0124cb1eb21dd9b3b72d3 100644 (file)
@@ -333,8 +333,7 @@ def test_migrate2kasp_case(servers, params):
 
     # Special case: CSK without SEP bit set.
     if zone == "csk-nosep.kasp":
-        expected[0].properties["role_full"] = "zone-signing"
-        expected[0].properties["flags"] = 256
+        expected[0].flags = 256
 
     # Key files.
     keys = isctest.kasp.keydir_to_keylist(zone, params["config"]["key-directory"])
index 2d49ef7cf4742f205c5a3cc00a3dfa52d5e55c13..6b5a624403131bc724bf9e1722fce0dada7997fa 100644 (file)
@@ -74,8 +74,8 @@ def test_rollover_multisigner(ns3, alg, size):
 
     newprops = [f"zsk unlimited {alg} {size} tag-range:0-32767"]
     expected2 = isctest.kasp.policy_to_properties(ttl, newprops)
-    expected2[0].properties["private"] = False
-    expected2[0].properties["legacy"] = True
+    expected2[0].private = False  # noqa
+    expected2[0].legacy = True  # noqa
     expected = expected + expected2
 
     ownkeys = isctest.kasp.keydir_to_keylist(zone, ns3.identifier)
@@ -98,8 +98,8 @@ def test_rollover_multisigner(ns3, alg, size):
     newkeys = isctest.kasp.keystr_to_keylist(out)
     newprops = [f"zsk unlimited {alg} {size} tag-range:0-32767"]
     expected2 = isctest.kasp.policy_to_properties(ttl, newprops)
-    expected2[0].properties["private"] = False
-    expected2[0].properties["legacy"] = True
+    expected2[0].private = False  # noqa
+    expected2[0].legacy = True  # noqa
     expected = expected + expected2
 
     dnskey = newkeys[0].dnskey().split()