From ac2be27f8f894aad570cc7b1b0e5aa1786f8e6ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicki=20K=C5=99=C3=AD=C5=BEek?= Date: Wed, 1 Oct 2025 16:35:45 +0200 Subject: [PATCH 01/10] Utilize nsX.rndc() helper Remove the duplicated code and replace it with nsX.rndc() call. --- bin/tests/system/dnstap/tests_dnstap.py | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/bin/tests/system/dnstap/tests_dnstap.py b/bin/tests/system/dnstap/tests_dnstap.py index ba3c39c709..a8680fed1d 100644 --- a/bin/tests/system/dnstap/tests_dnstap.py +++ b/bin/tests/system/dnstap/tests_dnstap.py @@ -33,20 +33,7 @@ pytestmark = [ ] -def run_rndc(server, rndc_command): - """ - Send the specified 'rndc_command' to 'server' with a timeout of 10 seconds - """ - rndc = isctest.vars.ALL["RNDC"] - port = isctest.vars.ALL["CONTROLPORT"] - - cmdline = [rndc, "-c", "../_common/rndc.conf", "-p", port, "-s", server] - cmdline.extend(rndc_command) - - isctest.run.cmd(cmdline) - - -def test_dnstap_dispatch_socket_addresses(): +def test_dnstap_dispatch_socket_addresses(ns3): # Send some query to ns3 so that it records something in its dnstap file. msg = isctest.query.create("mail.example.", "A") res = isctest.query.tcp(msg, "10.53.0.2", expected_rcode=dns.rcode.NOERROR) @@ -55,7 +42,7 @@ def test_dnstap_dispatch_socket_addresses(): ] # Before continuing, roll dnstap file to ensure it is flushed to disk. - run_rndc("10.53.0.3", ["dnstap", "-roll", "1"]) + ns3.rndc("dnstap -roll 1") # Move the dnstap file aside so that it is retained for troubleshooting. os.rename(os.path.join("ns3", "dnstap.out.0"), "dnstap.out.resolver_addresses") From ac998da3f6c45073ea188763e7b9c84fa34e45ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicki=20K=C5=99=C3=AD=C5=BEek?= Date: Wed, 1 Oct 2025 17:53:18 +0200 Subject: [PATCH 02/10] Use CmdResult to decode stdout/stderr from isctest.run.cmd() Avoid repeating the .decode("utf-8") snippet when processing command output and provide a helper instead, which leads to more concise code. --- .../checkconf-keys/tests_checkconf_keys.py | 58 +++++++-------- bin/tests/system/checkconf/tests_checkconf.py | 24 +++---- bin/tests/system/checkds/tests_checkds.py | 8 +-- bin/tests/system/dnssec/tests_delv.py | 70 +++++++++---------- bin/tests/system/dnssec/tests_signing.py | 4 +- bin/tests/system/dnstap/tests_dnstap.py | 4 +- bin/tests/system/doth/conftest.py | 6 +- bin/tests/system/isctest/kasp.py | 19 ++--- bin/tests/system/isctest/run.py | 24 +++++-- bin/tests/system/kasp/tests_kasp.py | 10 +-- bin/tests/system/keepalive/tests_keepalive.py | 21 +++--- .../system/keyfromlabel/tests_keyfromlabel.py | 26 +++---- bin/tests/system/ksr/tests_ksr.py | 4 +- .../system/masterfile/tests_masterfile.py | 6 +- .../system/multisigner/tests_multisigner.py | 9 ++- bin/tests/system/nzd2nzf/tests_nzd2nzf.py | 8 +-- .../tests_rollover_multisigner.py | 2 +- bin/tests/system/rrchecker/tests_rrchecker.py | 18 ++--- .../system/tools/tests_tools_nsec3hash.py | 32 ++++----- bin/tests/system/verify/tests_verify.py | 29 ++++---- 20 files changed, 180 insertions(+), 202 deletions(-) diff --git a/bin/tests/system/checkconf-keys/tests_checkconf_keys.py b/bin/tests/system/checkconf-keys/tests_checkconf_keys.py index c89f156f82..1e512cf246 100644 --- a/bin/tests/system/checkconf-keys/tests_checkconf_keys.py +++ b/bin/tests/system/checkconf-keys/tests_checkconf_keys.py @@ -38,125 +38,117 @@ def test_dnssecpolicy_keystore(): # Superfluous key file. zone = "superfluous-keyfile.kz.example" - out = isctest.run.cmd( + cmd = isctest.run.cmd( [CHECKCONF, "-k", "bad-superfluous-keyfile.conf"], raise_on_exception=False ) - err = out.stdout.decode("utf-8") - assert f"zone '{zone}': wrong number of key files (3, expected 2)" in err + assert f"zone '{zone}': wrong number of key files (3, expected 2)" in cmd.out # Missing key file. zone = "missing-keyfile.kz.example" - out = isctest.run.cmd( + cmd = isctest.run.cmd( [CHECKCONF, "-k", "bad-missing-keyfile.conf"], raise_on_exception=False ) - err = out.stdout.decode("utf-8") - assert f"zone '{zone}': wrong number of key files (1, expected 2)" in err + assert f"zone '{zone}': wrong number of key files (1, expected 2)" in cmd.out # Mismatch algorithm. zone = "bad-algorithm.kz.example" - out = isctest.run.cmd( + cmd = isctest.run.cmd( [CHECKCONF, "-k", "bad-algorithm.conf"], raise_on_exception=False ) - err = out.stdout.decode("utf-8") keys = isctest.kasp.keydir_to_keylist(zone) assert len(keys) == 2 assert ( f"zone '{zone}': key file '{zone}/ECDSAP256SHA256/{keys[0].tag}' does not match dnssec-policy alternative-kz" - in err + in cmd.out ) assert ( f"zone '{zone}': key file '{zone}/ECDSAP256SHA256/{keys[1].tag}' does not match dnssec-policy alternative-kz" - in err + in cmd.out ) assert ( f"zone '{zone}': no key file found matching dnssec-policy alternative-kz key:'ksk algorithm:RSASHA256 length:2048 tag-range:0-65535'" - in err + in cmd.out ) assert ( f"zone '{zone}': no key file found matching dnssec-policy alternative-kz key:'zsk algorithm:RSASHA256 length:2048 tag-range:0-65535'" - in err + in cmd.out ) # Mismatch length zone = "bad-length.csk.example" - out = isctest.run.cmd( + cmd = isctest.run.cmd( [CHECKCONF, "-k", "bad-length.conf"], raise_on_exception=False ) - err = out.stdout.decode("utf-8") keys = isctest.kasp.keydir_to_keylist(zone) assert len(keys) == 1 assert ( f"zone '{zone}': key file '{zone}/RSASHA256/{keys[0].tag}' does not match dnssec-policy alternative-csk" - in err + in cmd.out ) assert ( f"zone '{zone}': no key file found matching dnssec-policy alternative-csk key:'csk algorithm:RSASHA256 length:2048 tag-range:0-65535'" - in err + in cmd.out ) # Mismatch tag range zone = "bad-tagrange.csk.example" - out = isctest.run.cmd( + cmd = isctest.run.cmd( [CHECKCONF, "-k", "bad-tagrange.conf"], raise_on_exception=False ) - err = out.stdout.decode("utf-8") keys = isctest.kasp.keydir_to_keylist(zone) assert len(keys) == 1 assert ( f"zone '{zone}': key file '{zone}/ECDSAP256SHA256/{keys[0].tag}' does not match dnssec-policy tagrange-csk" - in err + in cmd.out ) assert ( f"zone '{zone}': no key file found matching dnssec-policy tagrange-csk key:'csk algorithm:ECDSAP256SHA256 length:256 tag-range:0-32767'" - in err + in cmd.out ) # Mismatch role zone = "bad-role.kz.example" - out = isctest.run.cmd([CHECKCONF, "-k", "bad-role.conf"], raise_on_exception=False) - err = out.stdout.decode("utf-8") + cmd = isctest.run.cmd([CHECKCONF, "-k", "bad-role.conf"], raise_on_exception=False) keys = isctest.kasp.keydir_to_keylist(zone) assert len(keys) == 2 assert ( f"zone '{zone}': no key file found matching dnssec-policy default-kz key:'zsk algorithm:ECDSAP256SHA256 length:256 tag-range:0-65535'" - in err + in cmd.out ) # Mismatch algorithm (default policy) zone = "bad-default-algorithm.example" - out = isctest.run.cmd( + cmd = isctest.run.cmd( [CHECKCONF, "-k", "bad-default-algorithm.conf"], raise_on_exception=False ) - err = out.stdout.decode("utf-8") keys = isctest.kasp.keydir_to_keylist(zone) assert len(keys) == 1 assert ( f"zone '{zone}': key file '{zone}/RSASHA256/{keys[0].tag}' does not match dnssec-policy default" - in err + in cmd.out ) assert ( f"zone '{zone}': no key file found matching dnssec-policy default key:'csk algorithm:ECDSAP256SHA256 length:256 tag-range:0-65535'" - in err + in cmd.out ) # Mismatch role (default policy) zone = "bad-default-kz.example" - out = isctest.run.cmd( + cmd = isctest.run.cmd( [CHECKCONF, "-k", "bad-default-kz.conf"], raise_on_exception=False ) - err = out.stdout.decode("utf-8") keys = isctest.kasp.keydir_to_keylist(zone) assert len(keys) == 2 assert ( f"zone '{zone}': key file '{zone}/ECDSAP256SHA256/{keys[0].tag}' does not match dnssec-policy default" - in err + in cmd.out ) assert ( f"zone '{zone}': key file '{zone}/ECDSAP256SHA256/{keys[1].tag}' does not match dnssec-policy default" - in err + in cmd.out ) assert ( f"zone '{zone}': no key file found matching dnssec-policy default key:'csk algorithm:ECDSAP256SHA256 length:256 tag-range:0-65535'" - in err + in cmd.out ) - assert f"zone '{zone}': wrong number of key files (2, expected 1)" in err + assert f"zone '{zone}': wrong number of key files (2, expected 1)" in cmd.out diff --git a/bin/tests/system/checkconf/tests_checkconf.py b/bin/tests/system/checkconf/tests_checkconf.py index 83abdb1e2f..3b7ef06780 100644 --- a/bin/tests/system/checkconf/tests_checkconf.py +++ b/bin/tests/system/checkconf/tests_checkconf.py @@ -15,25 +15,23 @@ import isctest def test_checkconf_effective(): - proc = isctest.run.cmd([os.environ["CHECKCONF"], "-e", "effective.conf"]) - checkconf_output = proc.stdout.decode() - assert "listen-on port 5353 {\n\t\t127.1.2.3/32;\n\t};" in checkconf_output - assert 'view "_bind" chaos {' in checkconf_output - assert 'remote-servers "_default_iana_root_zone_primaries" {' in checkconf_output - assert 'view "foo" {\n}' in checkconf_output + cmd = isctest.run.cmd([os.environ["CHECKCONF"], "-e", "effective.conf"]) + assert b"listen-on port 5353 {\n\t\t127.1.2.3/32;\n\t};" in cmd.proc.stdout + assert 'view "_bind" chaos {' in cmd.out + assert 'remote-servers "_default_iana_root_zone_primaries" {' in cmd.out + assert b'view "foo" {\n}' in cmd.proc.stdout # builtin-trust-anchors is non documented and internal clause only, it must # not be visible. - assert "builtin-trust-anchors" not in checkconf_output + assert "builtin-trust-anchors" not in cmd.out def test_checkconf_builtin(): - proc = isctest.run.cmd([os.environ["CHECKCONF"], "-b"]) - checkconf_output = proc.stdout.decode() - assert 'listen-on {\n\t\t"any";\n\t};' in checkconf_output - assert 'view "_bind" chaos {' in checkconf_output - assert 'remote-servers "_default_iana_root_zone_primaries" {' in checkconf_output + cmd = isctest.run.cmd([os.environ["CHECKCONF"], "-b"]) + assert b'listen-on {\n\t\t"any";\n\t};' in cmd.proc.stdout + assert 'view "_bind" chaos {' in cmd.out + assert 'remote-servers "_default_iana_root_zone_primaries" {' in cmd.out # builtin-trust-anchors is non documented and internal clause only, it must # not be visible. - assert "builtin-trust-anchors" not in checkconf_output + assert "builtin-trust-anchors" not in cmd.out diff --git a/bin/tests/system/checkds/tests_checkds.py b/bin/tests/system/checkds/tests_checkds.py index 5e859a44c2..b6c36e3908 100755 --- a/bin/tests/system/checkds/tests_checkds.py +++ b/bin/tests/system/checkds/tests_checkds.py @@ -102,10 +102,10 @@ def verify_zone(zone, transfer): verifier = isctest.run.cmd(verify_cmd) - if verifier.returncode != 0: + if verifier.rc != 0: isctest.log.error(f"dnssec-verify {zone} failed") - return verifier.returncode == 0 + return verifier.rc == 0 def read_statefile(server, zone): @@ -210,10 +210,10 @@ def rekey(zone): ] controller = isctest.run.cmd(rndc_cmd) - if controller.returncode != 0: + if controller.rc != 0: isctest.log.error(f"rndc loadkeys {zone} failed") - assert controller.returncode == 0 + assert controller.rc == 0 class CheckDSTest(NamedTuple): diff --git a/bin/tests/system/dnssec/tests_delv.py b/bin/tests/system/dnssec/tests_delv.py index aa374584ba..9727db3a01 100644 --- a/bin/tests/system/dnssec/tests_delv.py +++ b/bin/tests/system/dnssec/tests_delv.py @@ -64,125 +64,121 @@ def delv(*args, tkeys=False): delv_cmd.extend(["@10.53.0.4", "-a", tfile, "-p", os.environ["PORT"]]) delv_cmd.extend(args) - return ( - isctest.run.cmd(delv_cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT) - .stdout.decode("utf-8") - .strip() - ) + return isctest.run.cmd(delv_cmd, stderr=subprocess.STDOUT) def test_positive_validation_delv(): # check positive validation NSEC response = delv("a", "a.example") - assert grep_c("a.example..*10.0.0.1", response) - assert grep_c("a.example..*.RRSIG.A [0-9][0-9]* 2 300 .*", response) + assert grep_c("a.example..*10.0.0.1", response.out) + assert grep_c("a.example..*.RRSIG.A [0-9][0-9]* 2 300 .*", response.out) # check positive validation NSEC (trsuted-keys) response = delv("a", "a.example", tkeys=True) - assert grep_c("a.example..*10.0.0.1", response) - assert grep_c("a.example..*.RRSIG.A [0-9][0-9]* 2 300 .*", response) + assert grep_c("a.example..*10.0.0.1", response.out) + assert grep_c("a.example..*.RRSIG.A [0-9][0-9]* 2 300 .*", response.out) # check positive validation NSEC3 response = delv("a", "a.nsec3.example") - assert grep_c("a.nsec3.example..*10.0.0.1", response) - assert grep_c("a.nsec3.example..*.RRSIG.A [0-9][0-9]* 3 300 .*", response) + assert grep_c("a.nsec3.example..*10.0.0.1", response.out) + assert grep_c("a.nsec3.example..*.RRSIG.A [0-9][0-9]* 3 300 .*", response.out) # check positive validation OPTOUT response = delv("a", "a.optout.example") - assert grep_c("a.optout.example..*10.0.0.1", response) - assert grep_c("a.optout.example..*.RRSIG.A [0-9][0-9]* 3 300 .*", response) + assert grep_c("a.optout.example..*10.0.0.1", response.out) + assert grep_c("a.optout.example..*.RRSIG.A [0-9][0-9]* 3 300 .*", response.out) # check positive wildcard validation NSEC response = delv("a", "a.wild.example") - assert grep_c("a.wild.example..*10.0.0.27", response) - assert grep_c("a.wild.example..*.RRSIG.A [0-9][0-9]* 2 300 .*", response) + assert grep_c("a.wild.example..*10.0.0.27", response.out) + assert grep_c("a.wild.example..*.RRSIG.A [0-9][0-9]* 2 300 .*", response.out) # check positive wildcard validation NSEC3 response = delv("a", "a.wild.nsec3.example") - assert grep_c("a.wild.nsec3.example..*10.0.0.6", response) - assert grep_c("a.wild.nsec3.example..*.RRSIG.A [0-9][0-9]* 3 300 .*", response) + assert grep_c("a.wild.nsec3.example..*10.0.0.6", response.out) + assert grep_c("a.wild.nsec3.example..*.RRSIG.A [0-9][0-9]* 3 300 .*", response.out) # check positive wildcard validation OPTOUT response = delv("a", "a.wild.optout.example") - assert grep_c("a.wild.optout.example..*10.0.0.6", response) - assert grep_c("a.wild.optout.example..*.RRSIG.A [0-9][0-9]* 3 300 .*", response) + assert grep_c("a.wild.optout.example..*10.0.0.6", response.out) + assert grep_c("a.wild.optout.example..*.RRSIG.A [0-9][0-9]* 3 300 .*", response.out) def test_negative_validation_delv(): # checking negative validation NXDOMAIN NSEC response = delv("a", "q.example") - assert grep_c("resolution failed: ncache nxdomain", response) + assert grep_c("resolution failed: ncache nxdomain", response.out) # checking negative validation NODATA NSEC response = delv("txt", "a.example") - assert grep_c("resolution failed: ncache nxrrset", response) + assert grep_c("resolution failed: ncache nxrrset", response.out) # checking negative validation NXDOMAIN NSEC3 response = delv("a", "q.nsec3.example") - assert grep_c("resolution failed: ncache nxdomain", response) + assert grep_c("resolution failed: ncache nxdomain", response.out) # checking negative validation NODATA NSEC3 response = delv("txt", "a.nsec3.example") - assert grep_c("resolution failed: ncache nxrrset", response) + assert grep_c("resolution failed: ncache nxrrset", response.out) # checking negative validation NXDOMAIN OPTOUT response = delv("a", "q.optout.example") - assert grep_c("resolution failed: ncache nxdomain", response) + assert grep_c("resolution failed: ncache nxdomain", response.out) # checking negative validation NODATA OPTOUT response = delv("txt", "a.optout.example") - assert grep_c("resolution failed: ncache nxrrset", response) + assert grep_c("resolution failed: ncache nxrrset", response.out) # checking negative wildcard validation NSEC response = delv("txt", "b.wild.example") - assert grep_c("resolution failed: ncache nxrrset", response) + assert grep_c("resolution failed: ncache nxrrset", response.out) # checking negative wildcard validation NSEC3 response = delv("txt", "b.wild.nsec3.example") - assert grep_c("resolution failed: ncache nxrrset", response) + assert grep_c("resolution failed: ncache nxrrset", response.out) # checking negative wildcard validation OPTOUT response = delv("txt", "b.wild.optout.example") - assert grep_c("resolution failed: ncache nxrrset", response) + assert grep_c("resolution failed: ncache nxrrset", response.out) def test_insecure_validation_delv(): # check 1-server insecurity proof NSEC response = delv("a", "a.insecure.example") - assert grep_c("a.insecure.example..*10.0.0.1", response) + assert grep_c("a.insecure.example..*10.0.0.1", response.out) # check 1-server insecurity proof NSEC3 response = delv("a", "a.insecure.nsec3.example") - assert grep_c("a.insecure.nsec3.example..*10.0.0.1", response) + assert grep_c("a.insecure.nsec3.example..*10.0.0.1", response.out) # check 1-server insecurity proof NSEC3 response = delv("a", "a.insecure.optout.example") - assert grep_c("a.insecure.optout.example..*10.0.0.1", response) + assert grep_c("a.insecure.optout.example..*10.0.0.1", response.out) # check 1-server negative insecurity proof NSEC response = delv("a", "q.insecure.example") - assert grep_c("resolution failed: ncache nxdomain", response) + assert grep_c("resolution failed: ncache nxdomain", response.out) # check 1-server negative insecurity proof NSEC3 response = delv("a", "q.insecure.nsec3.example") - assert grep_c("resolution failed: ncache nxdomain", response) + assert grep_c("resolution failed: ncache nxdomain", response.out) # check 1-server negative insecurity proof OPTOUT response = delv("a", "q.insecure.optout.example") - assert grep_c("resolution failed: ncache nxdomain", response) + assert grep_c("resolution failed: ncache nxdomain", response.out) def test_validation_failure_delv(): # check failed validation due to bogus data response = delv("+cd", "a", "a.bogus.example") - assert grep_c("resolution failed: RRSIG failed to verify", response) + assert grep_c("resolution failed: RRSIG failed to verify", response.out) # check failed validation due to missing key record response = delv("+cd", "a", "a.b.keyless.example") - assert grep_c("resolution failed: insecurity proof failed", response) + assert grep_c("resolution failed: insecurity proof failed", response.out) def test_revoked_key_delv(): # check failed validation succeeds when a revoked key is encountered response = delv("+cd", "soa", "revkey.example") - assert grep_c("fully validated", response) + assert grep_c("fully validated", response.out) diff --git a/bin/tests/system/dnssec/tests_signing.py b/bin/tests/system/dnssec/tests_signing.py index 2ff3e0e063..b0295e5840 100644 --- a/bin/tests/system/dnssec/tests_signing.py +++ b/bin/tests/system/dnssec/tests_signing.py @@ -63,14 +63,14 @@ def grep_c(regex, filename): def keygen(*args): keygen_cmd = [os.environ.get("KEYGEN")] keygen_cmd.extend(args) - return isctest.run.cmd(keygen_cmd, log_stdout=True).stdout.decode("utf-8").strip() + return isctest.run.cmd(keygen_cmd).out.strip() # run dnssec-settime def settime(*args): settime_cmd = [os.environ.get("SETTIME")] settime_cmd.extend(args) - return isctest.run.cmd(settime_cmd, log_stdout=True).stdout.decode("utf-8").strip() + return isctest.run.cmd(settime_cmd).out.strip() @pytest.mark.parametrize( diff --git a/bin/tests/system/dnstap/tests_dnstap.py b/bin/tests/system/dnstap/tests_dnstap.py index a8680fed1d..aec43785ea 100644 --- a/bin/tests/system/dnstap/tests_dnstap.py +++ b/bin/tests/system/dnstap/tests_dnstap.py @@ -48,7 +48,7 @@ def test_dnstap_dispatch_socket_addresses(ns3): os.rename(os.path.join("ns3", "dnstap.out.0"), "dnstap.out.resolver_addresses") # Read the contents of the dnstap file using dnstap-read. - run = isctest.run.cmd( + dnstapread = isctest.run.cmd( [isctest.vars.ALL["DNSTAPREAD"], "dnstap.out.resolver_addresses"], ) @@ -64,7 +64,7 @@ def test_dnstap_dispatch_socket_addresses(ns3): bad_frames = [] inspected_frames = 0 addr_regex = r"^10\.53\.0\.[0-9]+:[0-9]{1,5}$" - for line in run.stdout.decode("utf-8").splitlines(): + for line in dnstapread.out.splitlines(): _, _, frame_type, addr1, _, addr2, _ = line.split(" ", 6) # Only inspect RESOLVER_QUERY and RESOLVER_RESPONSE frames. if frame_type not in ("RQ", "RR"): diff --git a/bin/tests/system/doth/conftest.py b/bin/tests/system/doth/conftest.py index 27dca7ee8a..b95baeaab5 100644 --- a/bin/tests/system/doth/conftest.py +++ b/bin/tests/system/doth/conftest.py @@ -25,10 +25,10 @@ def gnutls_cli_executable(): pytest.skip("gnutls-cli not found in PATH") # Ensure gnutls-cli supports the --logfile command-line option. - output = isctest.run.cmd( + cmd = isctest.run.cmd( [executable, "--logfile=/dev/null"], log_stderr=False, raise_on_exception=False - ).stdout - if b"illegal option" in output: + ) + if "illegal option" in cmd.out: pytest.skip("gnutls-cli does not support the --logfile option") return executable diff --git a/bin/tests/system/isctest/kasp.py b/bin/tests/system/isctest/kasp.py index 1a657a26db..2e1b2c36fb 100644 --- a/bin/tests/system/isctest/kasp.py +++ b/bin/tests/system/isctest/kasp.py @@ -15,7 +15,6 @@ import glob import os from pathlib import Path import re -import subprocess import time from typing import Dict, List, Optional, Tuple, Union @@ -533,8 +532,8 @@ class Key: str(self.keyfile), ] - out = isctest.run.cmd(dsfromkey_command) - dsfromkey = out.stdout.decode("utf-8").split() + cmd = isctest.run.cmd(dsfromkey_command) + dsfromkey = cmd.out.split() rdata_fromfile = " ".join(dsfromkey[:7]) rdata_fromwire = " ".join(cds[:7]) @@ -837,18 +836,14 @@ def check_dnssec_verify(server, zone, tsig=None): file.write(rr.to_text()) file.write("\n") - try: - verify_command = [os.environ.get("VERIFY"), "-z", "-o", zone, zonefile] - verified = isctest.run.cmd(verify_command) - except subprocess.CalledProcessError: - pass - - if verified: - break + verify_command = [os.environ.get("VERIFY"), "-z", "-o", zone, zonefile] + verified = isctest.run.cmd(verify_command, raise_on_exception=False) + if verified.rc == 0: + return time.sleep(1) - assert verified + assert False, "zone not verified" def check_dnssecstatus(server, zone, keys, policy=None, view=None, verbose=False): diff --git a/bin/tests/system/isctest/run.py b/bin/tests/system/isctest/run.py index e4fbd459dd..a052f65d3d 100644 --- a/bin/tests/system/isctest/run.py +++ b/bin/tests/system/isctest/run.py @@ -18,6 +18,18 @@ from typing import List, Optional import isctest.log +class CmdResult: + def __init__(self, proc=None): + self.proc = proc + self.rc = self.proc.returncode + self.out = "" + self.err = "" + if self.proc.stdout: + self.out = self.proc.stdout.decode("utf-8") + if self.proc.stderr: + self.err = self.proc.stderr.decode("utf-8") + + def cmd( args, cwd=None, @@ -29,7 +41,7 @@ def cmd( input_text: Optional[bytes] = None, raise_on_exception=True, env: Optional[dict] = None, -): +) -> CmdResult: """Execute a command with given args as subprocess.""" isctest.log.debug(f"isctest.run.cmd(): {' '.join(args)}") @@ -59,13 +71,13 @@ def cmd( env=env, ) print_debug_logs(proc) - return proc + return CmdResult(proc) except subprocess.CalledProcessError as exc: print_debug_logs(exc) isctest.log.debug(f"isctest.run.cmd(): (return code) {exc.returncode}") if raise_on_exception: raise exc - return exc + return CmdResult(exc) def _run_script( @@ -107,11 +119,11 @@ class Dig: def __init__(self, base_params: str = ""): self.base_params = base_params - def __call__(self, params: str) -> str: - """Run the dig command with the given parameters and return the decoded output.""" + def __call__(self, params: str) -> CmdResult: + """Run the dig command with the given parameters.""" return cmd( [os.environ.get("DIG")] + f"{self.base_params} {params}".split(), - ).stdout.decode("utf-8") + ) def shell(script: str, args: Optional[List[str]] = None) -> None: diff --git a/bin/tests/system/kasp/tests_kasp.py b/bin/tests/system/kasp/tests_kasp.py index 2d6aa4b46e..d5acb31c15 100644 --- a/bin/tests/system/kasp/tests_kasp.py +++ b/bin/tests/system/kasp/tests_kasp.py @@ -1282,7 +1282,7 @@ def test_kasp_dnssec_keygen(): zone, ] - return isctest.run.cmd(keygen_command).stdout.decode("utf-8") + return isctest.run.cmd(keygen_command).out isctest.log.info( "check that 'dnssec-keygen -k' (configured policy) created valid files" @@ -1327,7 +1327,7 @@ def test_kasp_dnssec_keygen(): str(publish), key.path, ] - out = isctest.run.cmd(settime).stdout.decode("utf-8") + isctest.run.cmd(settime) isctest.check.file_contents_equal(f"{key.statefile}", f"{key.statefile}.backup") assert key.get_metadata("Publish", file=key.privatefile) == str(publish) @@ -1378,7 +1378,7 @@ def test_kasp_dnssec_keygen(): str(now), key.path, ] - out = isctest.run.cmd(settime).stdout.decode("utf-8") + isctest.run.cmd(settime) isctest.kasp.check_keys("kasp", keys, expected) isctest.kasp.check_keytimes(keys, expected) @@ -1415,7 +1415,7 @@ def test_kasp_dnssec_keygen(): str(now), key.path, ] - out = isctest.run.cmd(settime).stdout.decode("utf-8") + isctest.run.cmd(settime) isctest.kasp.check_keys("kasp", keys, expected) isctest.kasp.check_keytimes(keys, expected) @@ -1463,7 +1463,7 @@ def test_kasp_dnssec_keygen(): str(soon), key.path, ] - out = isctest.run.cmd(settime).stdout.decode("utf-8") + isctest.run.cmd(settime) isctest.kasp.check_keys("kasp", keys, expected) isctest.kasp.check_keytimes(keys, expected) diff --git a/bin/tests/system/keepalive/tests_keepalive.py b/bin/tests/system/keepalive/tests_keepalive.py index 9190ddd8b8..5314d7ee57 100644 --- a/bin/tests/system/keepalive/tests_keepalive.py +++ b/bin/tests/system/keepalive/tests_keepalive.py @@ -31,25 +31,27 @@ def test_dig_tcp_keepalive_handling(named_port, ns2): dig = isctest.run.Dig(f"-p {str(named_port)}") isctest.log.info("check that dig handles TCP keepalive in query") - assert "; TCP-KEEPALIVE" in dig("+qr +keepalive foo.example. @10.53.0.2") + assert "; TCP-KEEPALIVE" in dig("+qr +keepalive foo.example. @10.53.0.2").out isctest.log.info("check that dig added TCP keepalive was received") assert get_keepalive_options_received() == 1 isctest.log.info("check that TCP keepalive is added for TCP responses") - assert "; TCP-KEEPALIVE" in dig("+tcp +keepalive foo.example. @10.53.0.2") + assert "; TCP-KEEPALIVE" in dig("+tcp +keepalive foo.example. @10.53.0.2").out isctest.log.info("check that TCP keepalive requires TCP") - assert "; TCP-KEEPALIVE" not in dig("+keepalive foo.example. @10.53.0.2") + assert "; TCP-KEEPALIVE" not in dig("+keepalive foo.example. @10.53.0.2").out isctest.log.info("check the default keepalive value") - assert "; TCP-KEEPALIVE: 30.0 secs" in dig( - "+tcp +keepalive foo.example. @10.53.0.3" + assert ( + "; TCP-KEEPALIVE: 30.0 secs" + in dig("+tcp +keepalive foo.example. @10.53.0.3").out ) isctest.log.info("check a keepalive configured value") - assert "; TCP-KEEPALIVE: 15.0 secs" in dig( - "+tcp +keepalive foo.example. @10.53.0.2" + assert ( + "; TCP-KEEPALIVE: 15.0 secs" + in dig("+tcp +keepalive foo.example. @10.53.0.2").out ) isctest.log.info("check a re-configured keepalive value") @@ -59,8 +61,9 @@ def test_dig_tcp_keepalive_handling(named_port, ns2): assert "tcp-keepalive-timeout=300" in response assert "tcp-advertised-timeout=200" in response assert "tcp-primaries-timeout=100" in response - assert "; TCP-KEEPALIVE: 20.0 secs" in dig( - "+tcp +keepalive foo.example. @10.53.0.2" + assert ( + "; TCP-KEEPALIVE: 20.0 secs" + in dig("+tcp +keepalive foo.example. @10.53.0.2").out ) isctest.log.info("check server config entry") diff --git a/bin/tests/system/keyfromlabel/tests_keyfromlabel.py b/bin/tests/system/keyfromlabel/tests_keyfromlabel.py index 8fddbd4dd5..da9b391313 100644 --- a/bin/tests/system/keyfromlabel/tests_keyfromlabel.py +++ b/bin/tests/system/keyfromlabel/tests_keyfromlabel.py @@ -74,18 +74,16 @@ def token_init_and_cleanup(): ) try: - output = isctest.run.cmd( - token_init_command, env=EMPTY_OPENSSL_CONF_ENV - ).stdout.decode("utf-8") - assert "The token has been initialized and is reassigned to slot" in output + cmd = isctest.run.cmd(token_init_command, env=EMPTY_OPENSSL_CONF_ENV) + assert "The token has been initialized and is reassigned to slot" in cmd.out yield finally: - output = isctest.run.cmd( + cmd = isctest.run.cmd( token_cleanup_command, env=EMPTY_OPENSSL_CONF_ENV, raise_on_exception=False, - ).stdout.decode("utf-8") - assert re.search("Found token (.*) with matching token label", output) + ) + assert re.search("Found token (.*) with matching token label", cmd.out) # pylint: disable-msg=too-many-locals @@ -125,11 +123,9 @@ def test_keyfromlabel(alg_name, alg_type, alg_bits): HSMPIN, ] - output = isctest.run.cmd( - pkcs11_command, env=EMPTY_OPENSSL_CONF_ENV - ).stdout.decode("utf-8") + cmd = isctest.run.cmd(pkcs11_command, env=EMPTY_OPENSSL_CONF_ENV) - assert "Key pair generated" in output + assert "Key pair generated" in cmd.out def keyfromlabel(alg_name, zone, key_id, key_flag): key_flag = key_flag.split() if key_flag else [] @@ -145,12 +141,12 @@ def test_keyfromlabel(alg_name, alg_type, alg_bits): zone, ] - output = isctest.run.cmd(keyfrlab_command) - output_decoded = output.stdout.decode("utf-8").rstrip() + ".key" + cmd = isctest.run.cmd(keyfrlab_command) + keyfile = cmd.out.rstrip() + ".key" - assert os.path.exists(output_decoded) + assert os.path.exists(keyfile) - return output_decoded + return keyfile if f"{alg_name.upper()}_SUPPORTED" not in os.environ: pytest.skip(f"{alg_name} is not supported") diff --git a/bin/tests/system/ksr/tests_ksr.py b/bin/tests/system/ksr/tests_ksr.py index 374865b042..fc869bb527 100644 --- a/bin/tests/system/ksr/tests_ksr.py +++ b/bin/tests/system/ksr/tests_ksr.py @@ -97,8 +97,8 @@ def ksr(zone, policy, action, options="", raise_on_exception=True): zone, ] - out = isctest.run.cmd(ksr_command, raise_on_exception=raise_on_exception) - return out.stdout.decode("utf-8"), out.stderr.decode("utf-8") + cmd = isctest.run.cmd(ksr_command, raise_on_exception=raise_on_exception) + return cmd.out, cmd.err # TODO return cmd instead def check_keys( diff --git a/bin/tests/system/masterfile/tests_masterfile.py b/bin/tests/system/masterfile/tests_masterfile.py index 9b3d0b114f..ae85d7bde4 100644 --- a/bin/tests/system/masterfile/tests_masterfile.py +++ b/bin/tests/system/masterfile/tests_masterfile.py @@ -149,7 +149,7 @@ def test_masterfile_missing_master_file_servfail(): def test_masterfile_owner_inheritance(): """Test owner inheritance after $INCLUDE""" - checker_output = isctest.run.cmd( + cmd = isctest.run.cmd( [ os.environ["CHECKZONE"], "-D", @@ -157,12 +157,12 @@ def test_masterfile_owner_inheritance(): "example", "zone/inheritownerafterinclude.db", ] - ).stdout.decode("utf-8") + ) owner_inheritance_zone = """ example. 0 IN SOA . . 0 0 0 0 0 example. 0 IN TXT "this should be at the zone apex" example. 0 IN NS . """ - checker_zone = dns.zone.from_text(checker_output, origin="example.") + checker_zone = dns.zone.from_text(cmd.out, origin="example.") expected = dns.zone.from_text(owner_inheritance_zone, origin="example.") isctest.check.zones_equal(checker_zone, expected, compare_ttl=True) diff --git a/bin/tests/system/multisigner/tests_multisigner.py b/bin/tests/system/multisigner/tests_multisigner.py index 695f2420b9..c356595afe 100644 --- a/bin/tests/system/multisigner/tests_multisigner.py +++ b/bin/tests/system/multisigner/tests_multisigner.py @@ -73,8 +73,8 @@ def dsfromkey(key): "-w", str(key.keyfile), ] - out = isctest.run.cmd(dsfromkey_command) - return out.stdout.decode("utf-8").split() + cmd = isctest.run.cmd(dsfromkey_command) + return cmd.out.split() def check_dnssec(server, zone, keys, expected): @@ -102,12 +102,11 @@ def check_no_dnssec_in_journal(server, zone): f"{server.identifier}/{zone}.db.jnl", ] - out = isctest.run.cmd(journalprint) - contents = out.stdout.decode("utf-8") + cmd = isctest.run.cmd(journalprint) pattern = re.compile( r"^\s*(?:\S+\s+){4}(NSEC|NSEC3|NSEC3PARAM|RRSIG)", flags=re.MULTILINE ) - match = pattern.search(contents) + match = pattern.search(cmd.out) assert not match, f"{match.group(1)} record found in journal" diff --git a/bin/tests/system/nzd2nzf/tests_nzd2nzf.py b/bin/tests/system/nzd2nzf/tests_nzd2nzf.py index e25f2dfc9a..5ad766c4cf 100644 --- a/bin/tests/system/nzd2nzf/tests_nzd2nzf.py +++ b/bin/tests/system/nzd2nzf/tests_nzd2nzf.py @@ -44,13 +44,11 @@ def test_nzd2nzf(ns1): # dump "_default.nzd" to "_default.nzf" and check that it contains the expected content cfg_dir = "ns1" - stdout = isctest.run.cmd( - [os.environ["NZD2NZF"], "_default.nzd"], cwd=cfg_dir - ).stdout.decode("utf-8") - assert f"zone {zone_data}" in stdout + cmd = isctest.run.cmd([os.environ["NZD2NZF"], "_default.nzd"], cwd=cfg_dir) + assert f"zone {zone_data}" in cmd.out nzf_filename = os.path.join(cfg_dir, "_default.nzf") with open(nzf_filename, "w", encoding="utf-8") as nzf_file: - nzf_file.write(stdout) + nzf_file.write(cmd.out) # delete "_default.nzd" database nzd_filename = os.path.join(cfg_dir, "_default.nzd") diff --git a/bin/tests/system/rollover-multisigner/tests_rollover_multisigner.py b/bin/tests/system/rollover-multisigner/tests_rollover_multisigner.py index 7aad2d98cd..9c4cc47b8b 100644 --- a/bin/tests/system/rollover-multisigner/tests_rollover_multisigner.py +++ b/bin/tests/system/rollover-multisigner/tests_rollover_multisigner.py @@ -58,7 +58,7 @@ def test_rollover_multisigner(ns3, alg, size): zone, ] - return isctest.run.cmd(keygen_command).stdout.decode("utf-8") + return isctest.run.cmd(keygen_command).out zone = "multisigner-model2.kasp" diff --git a/bin/tests/system/rrchecker/tests_rrchecker.py b/bin/tests/system/rrchecker/tests_rrchecker.py index d1cc1b7192..715e891af8 100644 --- a/bin/tests/system/rrchecker/tests_rrchecker.py +++ b/bin/tests/system/rrchecker/tests_rrchecker.py @@ -121,22 +121,18 @@ pytestmark = pytest.mark.extra_artifacts( ], ) def test_rrchecker_list_standard_names(option, expected_result): - stdout = isctest.run.cmd([os.environ["RRCHECKER"], option]).stdout.decode("utf-8") - values = [line for line in stdout.split("\n") if line.strip()] + cmd = isctest.run.cmd([os.environ["RRCHECKER"], option]) + values = [line for line in cmd.out.split("\n") if line.strip()] assert sorted(values) == sorted(expected_result) def run_rrchecker(option, rr_class, rr_type, rr_rest): - rrchecker_output = ( - isctest.run.cmd( - [os.environ["RRCHECKER"], option], - input_text=f"{rr_class} {rr_type} {rr_rest}".encode("utf-8"), - ) - .stdout.decode("utf-8") - .strip() + cmd = isctest.run.cmd( + [os.environ["RRCHECKER"], option], + input_text=f"{rr_class} {rr_type} {rr_rest}".encode("utf-8"), ) - return rrchecker_output.split() + return cmd.out.strip().split() @pytest.mark.parametrize( @@ -162,7 +158,7 @@ def test_rrchecker_conversions(option): ".", tempzone_file, ], - ).stdout.decode("utf-8") + ).out checkzone_output = [ line for line in checkzone_output.splitlines() if not line.startswith(";") ] diff --git a/bin/tests/system/tools/tests_tools_nsec3hash.py b/bin/tests/system/tools/tests_tools_nsec3hash.py index 605e5e55af..19571882c1 100644 --- a/bin/tests/system/tools/tests_tools_nsec3hash.py +++ b/bin/tests/system/tools/tests_tools_nsec3hash.py @@ -52,16 +52,12 @@ def test_nsec3_hashes(domain, nsec3hash): algorithm = "1" iterations = "12" - output = isctest.run.cmd( - [NSEC3HASH, salt, algorithm, iterations, domain] - ).stdout.decode("utf-8") - assert nsec3hash in output + cmd = isctest.run.cmd([NSEC3HASH, salt, algorithm, iterations, domain]) + assert nsec3hash in cmd.out flags = "0" - output = isctest.run.cmd( - [NSEC3HASH, "-r", algorithm, flags, iterations, salt, domain] - ).stdout.decode("utf-8") - assert nsec3hash in output + cmd = isctest.run.cmd([NSEC3HASH, "-r", algorithm, flags, iterations, salt, domain]) + assert nsec3hash in cmd.out @pytest.mark.parametrize( @@ -78,11 +74,11 @@ def test_nsec3_empty_salt(salt_emptiness_args): iterations = "0" domain = "com" - output = isctest.run.cmd( + cmd = isctest.run.cmd( [NSEC3HASH] + salt_emptiness_args + [algorithm, iterations, domain] - ).stdout.decode("utf-8") - assert "CK0POJMG874LJREF7EFN8430QVIT8BSM" in output - assert "salt=-" in output + ) + assert "CK0POJMG874LJREF7EFN8430QVIT8BSM" in cmd.out + assert "salt=-" in cmd.out @pytest.mark.parametrize( @@ -98,7 +94,7 @@ def test_nsec3_empty_salt_r(salt_emptiness_arg): iterations = "0" domain = "com" - output = isctest.run.cmd( + cmd = isctest.run.cmd( [ NSEC3HASH, "-r", @@ -108,8 +104,8 @@ def test_nsec3_empty_salt_r(salt_emptiness_arg): salt_emptiness_arg, domain, ] - ).stdout.decode("utf-8") - assert " - CK0POJMG874LJREF7EFN8430QVIT8BSM" in output + ) + assert " - CK0POJMG874LJREF7EFN8430QVIT8BSM" in cmd.out @pytest.mark.parametrize( @@ -145,10 +141,8 @@ def test_nsec3hash_acceptable_values(domain, it, salt_bytes) -> None: ) # calculate the hash using nsec3hash: - output = isctest.run.cmd( - [NSEC3HASH, salt_text, "1", str(it), str(domain)] - ).stdout.decode("ascii") - hash2 = output.partition(" ")[0] + cmd = isctest.run.cmd([NSEC3HASH, salt_text, "1", str(it), str(domain)]) + hash2 = cmd.out.partition(" ")[0] assert hash1 == hash2 diff --git a/bin/tests/system/verify/tests_verify.py b/bin/tests/system/verify/tests_verify.py index 203c2181be..9ec0933176 100644 --- a/bin/tests/system/verify/tests_verify.py +++ b/bin/tests/system/verify/tests_verify.py @@ -11,6 +11,7 @@ import os import re +import subprocess import pytest @@ -63,12 +64,12 @@ def test_verify_good_zone_nsec_next_name_case_mismatch(): def get_bad_zone_output(zone): only_opt = ["-z"] if re.match(r"[zk]sk-only", zone) else [] - output = isctest.run.cmd( + cmd = isctest.run.cmd( [VERIFY, *only_opt, "-o", zone, f"zones/{zone}.bad"], + stderr=subprocess.STDOUT, raise_on_exception=False, ) - stream = (output.stdout + output.stderr).decode("utf-8").replace("\n", "") - return stream + return cmd.out @pytest.mark.parametrize( @@ -153,27 +154,25 @@ def test_verify_bad_zone_files_unequal_nsec3_chains(): def test_verify_soa_not_at_top_error(): # when -o is not used, origin is set to zone file name, # which should cause an error in this case - output = isctest.run.cmd( - [VERIFY, "zones/ksk+zsk.nsec.good"], raise_on_exception=False - ).stderr.decode("utf-8") - assert "not at top of zone" in output - assert "use -o to specify a different zone origin" in output + cmd = isctest.run.cmd([VERIFY, "zones/ksk+zsk.nsec.good"], raise_on_exception=False) + assert "not at top of zone" in cmd.err + assert "use -o to specify a different zone origin" in cmd.err # checking error message when an invalid -o is specified # and a SOA record not at top of zone is found def test_verify_invalid_o_option_soa_not_at_top_error(): - output = isctest.run.cmd( + cmd = isctest.run.cmd( [VERIFY, "-o", "invalid.origin", "zones/ksk+zsk.nsec.good"], raise_on_exception=False, - ).stderr.decode("utf-8") - assert "not at top of zone" in output - assert "use -o to specify a different zone origin" not in output + ) + assert "not at top of zone" in cmd.err + assert "use -o to specify a different zone origin" not in cmd.err # checking dnssec-verify -J reads journal file def test_verify_j_reads_journal_file(): - output = isctest.run.cmd( + cmd = isctest.run.cmd( [ VERIFY, "-o", @@ -182,5 +181,5 @@ def test_verify_j_reads_journal_file(): "zones/updated.other.jnl", "zones/updated.other", ] - ).stdout.decode("utf-8") - assert "Loading zone 'updated' from file 'zones/updated.other'" in output + ) + assert "Loading zone 'updated' from file 'zones/updated.other'" in cmd.out From ac7127d620614ccf54237e42c299fc238da759d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicki=20K=C5=99=C3=AD=C5=BEek?= Date: Thu, 2 Oct 2025 11:47:56 +0200 Subject: [PATCH 03/10] Use Re() for creating regular expressions It's a fairly common pattern to use regular expression in our tests. Instead of using the fairly verbose re.compile(), import that function as Re() instead to allow for more brevity in the test syntax. --- .../system/cipher-suites/tests_cipher_suites.py | 4 ++-- .../system/configloading/tests_configloading.py | 5 ++--- bin/tests/system/conftest.py | 10 +++++----- bin/tests/system/dnssec/tests_signing.py | 3 ++- bin/tests/system/dnssec/tests_tat.py | 6 +++--- .../system/dnssec/tests_validation_multiview.py | 4 ++-- bin/tests/system/isctest/kasp.py | 3 ++- bin/tests/system/isctest/log/watchlog.py | 12 ++++++++---- bin/tests/system/isctest/vars/openssl.py | 4 ++-- bin/tests/system/multisigner/tests_multisigner.py | 5 ++--- .../xfer-servers-list/tests_xfer_servers_list.py | 4 ++-- bin/tests/system/xferquota/tests_xferquota.py | 3 ++- 12 files changed, 34 insertions(+), 29 deletions(-) diff --git a/bin/tests/system/cipher-suites/tests_cipher_suites.py b/bin/tests/system/cipher-suites/tests_cipher_suites.py index edc3c09598..86adc29da1 100644 --- a/bin/tests/system/cipher-suites/tests_cipher_suites.py +++ b/bin/tests/system/cipher-suites/tests_cipher_suites.py @@ -9,7 +9,7 @@ # See the COPYRIGHT file distributed with this work for additional # information regarding copyright ownership. -import re +from re import compile as Re import pytest @@ -31,7 +31,7 @@ pytestmark = pytest.mark.extra_artifacts( @pytest.fixture(scope="module") def transfers_complete(servers): for zone in ["example", "example-aes-128", "example-aes-256", "example-chacha-20"]: - pattern = re.compile( + pattern = Re( f"transfer of '{zone}/IN' from 10.53.0.1#[0-9]+: Transfer completed" ) for ns in ["ns2", "ns3", "ns4", "ns5"]: diff --git a/bin/tests/system/configloading/tests_configloading.py b/bin/tests/system/configloading/tests_configloading.py index de3d25aa71..ec017a8bfe 100644 --- a/bin/tests/system/configloading/tests_configloading.py +++ b/bin/tests/system/configloading/tests_configloading.py @@ -9,8 +9,7 @@ # See the COPYRIGHT file distributed with this work for additional # information regarding copyright ownership. -import re - +from re import compile as Re import isctest @@ -59,7 +58,7 @@ def test_reload_fails_log(ns1, templates): "apply_configuration", "loop exclusive mode: starting", "apply_configuration: configure_views", - re.compile(r".*port '9999999' out of range"), + Re(r".*port '9999999' out of range"), "apply_configuration: detaching views", "loop exclusive mode: ending", "reloading configuration failed", diff --git a/bin/tests/system/conftest.py b/bin/tests/system/conftest.py index bbb2c28385..9eff2ee8b2 100644 --- a/bin/tests/system/conftest.py +++ b/bin/tests/system/conftest.py @@ -12,7 +12,7 @@ import filecmp import os from pathlib import Path -import re +from re import compile as Re import shutil import subprocess import tempfile @@ -53,7 +53,7 @@ else: XDIST_WORKER = os.environ.get("PYTEST_XDIST_WORKER", "") FILE_DIR = os.path.abspath(Path(__file__).parent) -ENV_RE = re.compile(b"([^=]+)=(.*)") +ENV_RE = Re(b"([^=]+)=(.*)") PRIORITY_TESTS = [ # Tests that are scheduled first. Speeds up parallel execution. "rpz/", @@ -62,9 +62,9 @@ PRIORITY_TESTS = [ "timeouts/", "upforwd/", ] -PRIORITY_TESTS_RE = re.compile("|".join(PRIORITY_TESTS)) -SYSTEM_TEST_NAME_RE = re.compile(f"{SYSTEM_TEST_DIR_GIT_PATH}" + r"/([^/]+)") -SYMLINK_REPLACEMENT_RE = re.compile(r"/tests(_.*)\.py") +PRIORITY_TESTS_RE = Re("|".join(PRIORITY_TESTS)) +SYSTEM_TEST_NAME_RE = Re(f"{SYSTEM_TEST_DIR_GIT_PATH}" + r"/([^/]+)") +SYMLINK_REPLACEMENT_RE = Re(r"/tests(_.*)\.py") # ----------------------- Global requirements ---------------------------- diff --git a/bin/tests/system/dnssec/tests_signing.py b/bin/tests/system/dnssec/tests_signing.py index b0295e5840..2a1abd531d 100644 --- a/bin/tests/system/dnssec/tests_signing.py +++ b/bin/tests/system/dnssec/tests_signing.py @@ -12,6 +12,7 @@ from collections import namedtuple import os import re +from re import compile as Re import struct import time @@ -475,7 +476,7 @@ def test_offline_ksk_signing(ns2): os.rename(f"ns2/{KSK}.private.bak", f"ns2/{KSK}.private") def loadkeys(): - pattern = re.compile(f"{zone}/IN.*next key event") + pattern = Re(f"{zone}/IN.*next key event") with ns2.watch_log_from_here() as watcher: ns2.rndc(f"loadkeys {zone}", log=False) watcher.wait_for_line(pattern) diff --git a/bin/tests/system/dnssec/tests_tat.py b/bin/tests/system/dnssec/tests_tat.py index 609f110c59..e37644f3a9 100644 --- a/bin/tests/system/dnssec/tests_tat.py +++ b/bin/tests/system/dnssec/tests_tat.py @@ -10,7 +10,7 @@ # information regarding copyright ownership. import os -import re +from re import compile as Re from dns import edns @@ -67,7 +67,7 @@ def test_tat_queries(ns1, ns6): msg = isctest.query.create(".", "DNSKEY") opt = edns.GenericOption(14, b"\xff\xff") msg.use_edns(edns=True, options=[opt]) - pattern = re.compile("trust-anchor-telemetry './IN' from .* 65535") + pattern = Re("trust-anchor-telemetry './IN' from .* 65535") with ns1.watch_log_from_here() as watcher: res = isctest.query.tcp(msg, "10.53.0.1") watcher.wait_for_line(pattern) @@ -79,7 +79,7 @@ def test_tat_queries(ns1, ns6): opt1 = edns.GenericOption(14, b"\xff\xff") opt2 = edns.GenericOption(14, b"\xff\xfe") msg.use_edns(edns=True, options=[opt2, opt1]) - pattern = re.compile("trust-anchor-telemetry './IN' from .* 65534") + pattern = Re("trust-anchor-telemetry './IN' from .* 65534") with ns1.watch_log_from_here() as watcher: res = isctest.query.tcp(msg, "10.53.0.1") isctest.check.noerror(res) diff --git a/bin/tests/system/dnssec/tests_validation_multiview.py b/bin/tests/system/dnssec/tests_validation_multiview.py index 085f388b2c..015e458349 100644 --- a/bin/tests/system/dnssec/tests_validation_multiview.py +++ b/bin/tests/system/dnssec/tests_validation_multiview.py @@ -10,7 +10,7 @@ # information regarding copyright ownership. import os -import re +from re import compile as Re import isctest @@ -47,7 +47,7 @@ def test_staticstub_delegations(): def test_validator_logging(ns4): # check that validator logging includes the view name with multiple views - pattern = re.compile("view rec: *validat") + pattern = Re("view rec: *validat") with ns4.watch_log_from_start() as watcher: msg = isctest.query.create("secure.example", "NS") isctest.query.tcp(msg, "10.53.0.4") diff --git a/bin/tests/system/isctest/kasp.py b/bin/tests/system/isctest/kasp.py index 2e1b2c36fb..f46ceabe73 100644 --- a/bin/tests/system/isctest/kasp.py +++ b/bin/tests/system/isctest/kasp.py @@ -15,6 +15,7 @@ import glob import os from pathlib import Path import re +from re import compile as Re import time from typing import Dict, List, Optional, Tuple, Union @@ -1475,7 +1476,7 @@ def next_key_event_equals(server, zone, next_event): waitfor = rf".*zone {zone}.*: next key event in (?!3600$)(.*) seconds" with server.watch_log_from_start() as watcher: - watcher.wait_for_line(re.compile(waitfor)) + watcher.wait_for_line(Re(waitfor)) # WMM: The with code below is extracting the line the watcher was # waiting for. If WatchLog.wait_for_line()` returned the matched string, diff --git a/bin/tests/system/isctest/log/watchlog.py b/bin/tests/system/isctest/log/watchlog.py index 6d85e9817d..d2d8521ebd 100644 --- a/bin/tests/system/isctest/log/watchlog.py +++ b/bin/tests/system/isctest/log/watchlog.py @@ -14,6 +14,7 @@ from typing import Any, Iterator, List, Match, Optional, Pattern, TextIO, TypeVa import abc import os import re +from re import compile as Re import time @@ -213,7 +214,7 @@ class WatchLog(abc.ABC): if isinstance(string, Pattern): patterns.append(string) elif isinstance(string, str): - pattern = re.compile(re.escape(string)) + pattern = Re(re.escape(string)) patterns.append(pattern) else: raise WatchLogException( @@ -256,13 +257,14 @@ class WatchLog(abc.ABC): Recommended use: ```python + from re import compile as Re import isctest def test_foo(servers): with servers["ns1"].watch_log_from_start() as watcher: watcher.wait_for_line("all zones loaded") - pattern = re.compile(r"next key event in ([0-9]+) seconds") + pattern = Re(r"next key event in ([0-9]+) seconds") with servers["ns1"].watch_log_from_here() as watcher: # ... do stuff here ... match = watcher.wait_for_line(pattern) @@ -321,7 +323,8 @@ class WatchLog(abc.ABC): >>> # Different values must be returned depending on which line is >>> # found in the log file. >>> import tempfile - >>> patterns = [re.compile(r"bar ([0-9])"), "qux"] + >>> from re import compile as Re + >>> patterns = [Re(r"bar ([0-9])"), "qux"] >>> with tempfile.NamedTemporaryFile("w") as file: ... print("foo bar 3", file=file, flush=True) ... with WatchLogFromStart(file.name) as watcher: @@ -443,7 +446,8 @@ class WatchLog(abc.ABC): >>> assert ret[1].group(0) == "foo" >>> import tempfile - >>> bar_pattern = re.compile('bar') + >>> from re import compile as Re + >>> bar_pattern = Re('bar') >>> patterns = ['foo', bar_pattern] >>> with tempfile.NamedTemporaryFile("w") as file: ... print("bar", file=file, flush=True) diff --git a/bin/tests/system/isctest/vars/openssl.py b/bin/tests/system/isctest/vars/openssl.py index 19d62d1811..3d1829e723 100644 --- a/bin/tests/system/isctest/vars/openssl.py +++ b/bin/tests/system/isctest/vars/openssl.py @@ -10,7 +10,7 @@ # information regarding copyright ownership. import os -import re +from re import compile as Re from typing import Optional from .. import log @@ -30,7 +30,7 @@ def parse_openssl_config(path: Optional[str]): return assert os.path.isfile(path), f"{path} exists, but it's not a file" - regex = re.compile(r"([^=]+)=(.*)") + regex = Re(r"([^=]+)=(.*)") log.debug(f"parsing openssl config: {path}") with open(path, "r", encoding="utf-8") as conf: for line in conf: diff --git a/bin/tests/system/multisigner/tests_multisigner.py b/bin/tests/system/multisigner/tests_multisigner.py index c356595afe..900cc7e7b4 100644 --- a/bin/tests/system/multisigner/tests_multisigner.py +++ b/bin/tests/system/multisigner/tests_multisigner.py @@ -12,6 +12,7 @@ from datetime import timedelta import os import re +from re import compile as Re import pytest @@ -103,9 +104,7 @@ def check_no_dnssec_in_journal(server, zone): ] cmd = isctest.run.cmd(journalprint) - pattern = re.compile( - r"^\s*(?:\S+\s+){4}(NSEC|NSEC3|NSEC3PARAM|RRSIG)", flags=re.MULTILINE - ) + pattern = Re(r"^\s*(?:\S+\s+){4}(NSEC|NSEC3|NSEC3PARAM|RRSIG)", flags=re.MULTILINE) match = pattern.search(cmd.out) assert not match, f"{match.group(1)} record found in journal" diff --git a/bin/tests/system/xfer-servers-list/tests_xfer_servers_list.py b/bin/tests/system/xfer-servers-list/tests_xfer_servers_list.py index 612be36b1d..54cf2fcaf2 100644 --- a/bin/tests/system/xfer-servers-list/tests_xfer_servers_list.py +++ b/bin/tests/system/xfer-servers-list/tests_xfer_servers_list.py @@ -9,7 +9,7 @@ # See the COPYRIGHT file distributed with this work for additional # information regarding copyright ownership. -import re +from re import compile as Re import isctest @@ -32,7 +32,7 @@ def wait_for_initial_xfrin(ns): def wait_for_sending_notify(ns1, ns, key_name): - pattern = re.compile( + pattern = Re( f"zone test/IN: sending notify to {ns.ip}#[0-9]+ : TSIG \\({key_name}\\)" ) with ns1.watch_log_from_start() as watcher: diff --git a/bin/tests/system/xferquota/tests_xferquota.py b/bin/tests/system/xferquota/tests_xferquota.py index a4edc5998f..a0d042d135 100644 --- a/bin/tests/system/xferquota/tests_xferquota.py +++ b/bin/tests/system/xferquota/tests_xferquota.py @@ -12,6 +12,7 @@ import glob import os import re +from re import compile as Re import shutil import signal import time @@ -71,7 +72,7 @@ def test_xferquota(named_port, ns1, ns2): isctest.check.rrsets_equal(ns1response.answer, ns2response.answer) query_and_compare(axfr_msg) - pattern = re.compile( + pattern = Re( f"transfer of 'changing/IN' from 10.53.0.1#{named_port}: " f"Transfer completed: .*\\(serial 2\\)" ) From be6bae2a75f17676ca90cc9dc34ac0a6a822933c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicki=20K=C5=99=C3=AD=C5=BEek?= Date: Thu, 2 Oct 2025 12:05:27 +0200 Subject: [PATCH 04/10] Move text-related operations into isctest.text module Add a new module for working with text and keep the isctest.log.watchlog module focused on its purpose. Move LogFile and LineReader into the new module. Add compile_pattern() helper which will be useful in subsequent commits. --- bin/tests/system/isctest/log/watchlog.py | 139 +-------------------- bin/tests/system/isctest/text.py | 150 +++++++++++++++++++++++ 2 files changed, 154 insertions(+), 135 deletions(-) create mode 100644 bin/tests/system/isctest/text.py diff --git a/bin/tests/system/isctest/log/watchlog.py b/bin/tests/system/isctest/log/watchlog.py index d2d8521ebd..3504452964 100644 --- a/bin/tests/system/isctest/log/watchlog.py +++ b/bin/tests/system/isctest/log/watchlog.py @@ -9,16 +9,15 @@ # See the COPYRIGHT file distributed with this work for additional # information regarding copyright ownership. -from typing import Any, Iterator, List, Match, Optional, Pattern, TextIO, TypeVar, Union +from typing import Any, List, Match, Optional, Pattern, TextIO, TypeVar, Union import abc import os -import re -from re import compile as Re import time +from isctest.text import compile_pattern, FlexPattern, LineReader, LogFile + -FlexPattern = Union[str, Pattern] T = TypeVar("T") OneOrMore = Union[T, List[T]] @@ -31,128 +30,6 @@ class WatchLogTimeout(WatchLogException): pass -class LogFile: - """ - Log file wrapper with a path and means to find a string in its contents. - """ - - def __init__(self, path: str): - self.path = path - - @property - def _lines(self) -> Iterator[str]: - with open(self.path, encoding="utf-8") as f: - yield from f - - def __contains__(self, substring: str) -> bool: - """ - Return whether any of the lines in the log contains a given string. - """ - for line in self._lines: - if substring in line: - return True - return False - - def expect(self, msg: str): - """Check the string is present anywhere in the log file.""" - if msg in self: - return - assert False, f"log message not found in log {self.path}: {msg}" - - def prohibit(self, msg: str): - """Check the string is not present in the entire log file.""" - if msg in self: - assert False, f"forbidden message appeared in log {self.path}: {msg}" - - -class LineReader: - """ - >>> import io - - >>> file = io.StringIO("complete line\\n") - >>> line_reader = LineReader(file) - >>> for line in line_reader.readlines(): - ... print(line.strip()) - complete line - - >>> file = io.StringIO("complete line\\nand then incomplete line") - >>> line_reader = LineReader(file) - >>> for line in line_reader.readlines(): - ... print(line.strip()) - complete line - - >>> file = io.StringIO("complete line\\nand then another complete line\\n") - >>> line_reader = LineReader(file) - >>> for line in line_reader.readlines(): - ... print(line.strip()) - complete line - and then another complete line - - >>> file = io.StringIO() - >>> line_reader = LineReader(file) - >>> for chunk in ( - ... "first line\\nsecond line\\nthi", - ... "rd ", - ... "line\\nfour", - ... "th line\\n\\nfifth line\\n" - ... ): - ... print("=== OUTER ITERATION ===") - ... pos = file.tell() - ... print(chunk, end="", file=file) - ... _ = file.seek(pos) - ... for line in line_reader.readlines(): - ... print("--- inner iteration ---") - ... print(line.strip() or "") - === OUTER ITERATION === - --- inner iteration --- - first line - --- inner iteration --- - second line - === OUTER ITERATION === - === OUTER ITERATION === - --- inner iteration --- - third line - === OUTER ITERATION === - --- inner iteration --- - fourth line - --- inner iteration --- - - --- inner iteration --- - fifth line - """ - - def __init__(self, stream: TextIO): - self._stream = stream - self._linebuf = "" - - def readline(self) -> Optional[str]: - """ - Wrapper around io.readline() function to handle unfinished lines. - - If a line ends with newline character, it's returned immediately. - If a line doesn't end with a newline character, the read contents are - buffered until the next call of this function and None is returned - instead. - """ - read = self._stream.readline() - if not read.endswith("\n"): - self._linebuf += read - return None - read = self._linebuf + read - self._linebuf = "" - return read - - def readlines(self) -> Iterator[str]: - """ - Wrapper around io.readline() which only returns finished lines. - """ - while True: - line = self.readline() - if line is None: - return - yield line - - class WatchLog(abc.ABC): """ Wait for a log message to appear in a text file. @@ -211,15 +88,7 @@ class WatchLog(abc.ABC): if not isinstance(strings, list): strings = [strings] for string in strings: - if isinstance(string, Pattern): - patterns.append(string) - elif isinstance(string, str): - pattern = Re(re.escape(string)) - patterns.append(pattern) - else: - raise WatchLogException( - "only string and re.Pattern allowed for matching" - ) + patterns.append(compile_pattern(string)) return patterns def _wait_for_match(self, regexes: List[Pattern]) -> Match: diff --git a/bin/tests/system/isctest/text.py b/bin/tests/system/isctest/text.py new file mode 100644 index 0000000000..902917ce19 --- /dev/null +++ b/bin/tests/system/isctest/text.py @@ -0,0 +1,150 @@ +#!/usr/bin/python3 + +# Copyright (C) Internet Systems Consortium, Inc. ("ISC") +# +# SPDX-License-Identifier: MPL-2.0 +# +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, you can obtain one at https://mozilla.org/MPL/2.0/. +# +# See the COPYRIGHT file distributed with this work for additional +# information regarding copyright ownership. + +import abc +import re +from re import compile as Re +from typing import Iterator, Match, Optional, Pattern, TextIO, Union + + +FlexPattern = Union[str, Pattern] + + +def compile_pattern(string: FlexPattern) -> Pattern: + if isinstance(string, Pattern): + return string + if isinstance(string, str): + return Re(re.escape(string)) + raise TypeError("only string and re.Pattern allowed") + + +class LogFile: + """ + Log file wrapper with a path and means to find a string in its contents. + """ + + def __init__(self, path: str): + self.path = path + + @property + def _lines(self) -> Iterator[str]: + with open(self.path, encoding="utf-8") as f: + yield from f + + def __contains__(self, substring: str) -> bool: + """ + Return whether any of the lines in the log contains a given string. + """ + for line in self._lines: + if substring in line: + return True + return False + + def expect(self, msg: str): + """Check the string is present anywhere in the log file.""" + if msg in self: + return + assert False, f"log message not found in log {self.path}: {msg}" + + def prohibit(self, msg: str): + """Check the string is not present in the entire log file.""" + if msg in self: + assert False, f"forbidden message appeared in log {self.path}: {msg}" + + +class LineReader: + """ + >>> import io + + >>> file = io.StringIO("complete line\\n") + >>> line_reader = LineReader(file) + >>> for line in line_reader.readlines(): + ... print(line.strip()) + complete line + + >>> file = io.StringIO("complete line\\nand then incomplete line") + >>> line_reader = LineReader(file) + >>> for line in line_reader.readlines(): + ... print(line.strip()) + complete line + + >>> file = io.StringIO("complete line\\nand then another complete line\\n") + >>> line_reader = LineReader(file) + >>> for line in line_reader.readlines(): + ... print(line.strip()) + complete line + and then another complete line + + >>> file = io.StringIO() + >>> line_reader = LineReader(file) + >>> for chunk in ( + ... "first line\\nsecond line\\nthi", + ... "rd ", + ... "line\\nfour", + ... "th line\\n\\nfifth line\\n" + ... ): + ... print("=== OUTER ITERATION ===") + ... pos = file.tell() + ... print(chunk, end="", file=file) + ... _ = file.seek(pos) + ... for line in line_reader.readlines(): + ... print("--- inner iteration ---") + ... print(line.strip() or "") + === OUTER ITERATION === + --- inner iteration --- + first line + --- inner iteration --- + second line + === OUTER ITERATION === + === OUTER ITERATION === + --- inner iteration --- + third line + === OUTER ITERATION === + --- inner iteration --- + fourth line + --- inner iteration --- + + --- inner iteration --- + fifth line + """ + + def __init__(self, stream: TextIO): + self._stream = stream + self._linebuf = "" + + def readline(self) -> Optional[str]: + """ + Wrapper around io.readline() function to handle unfinished lines. + + If a line ends with newline character, it's returned immediately. + If a line doesn't end with a newline character, the read contents are + buffered until the next call of this function and None is returned + instead. + """ + read = self._stream.readline() + if not read.endswith("\n"): + self._linebuf += read + return None + read = self._linebuf + read + self._linebuf = "" + return read + + def readlines(self) -> Iterator[str]: + """ + Wrapper around io.readline() which only returns finished lines. + """ + while True: + line = self.readline() + if line is None: + return + yield line From 7743bab5fc08cfafdf106f4127266c0e1c5cd1b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicki=20K=C5=99=C3=AD=C5=BEek?= Date: Thu, 2 Oct 2025 17:06:25 +0200 Subject: [PATCH 05/10] Refactor LogFile into TextFile with Grep support Add a new Grep-like interface which can be used for searching for regular expressions in files. Replace the prior LogFile used for named logs with the new TextFile interface. --- bin/tests/system/dnssec/tests_tat.py | 2 +- bin/tests/system/dnssec/tests_validation.py | 35 ++++------ bin/tests/system/isctest/instance.py | 8 ++- bin/tests/system/isctest/kasp.py | 5 +- bin/tests/system/isctest/log/__init__.py | 2 +- bin/tests/system/isctest/log/watchlog.py | 2 +- bin/tests/system/isctest/text.py | 69 ++++++++++++------- bin/tests/system/kasp/tests_kasp.py | 8 +-- .../system/multisigner/tests_multisigner.py | 4 +- .../tests_rollover_algo_csk_reconfig.py | 8 +-- .../tests_rollover_algo_ksk_zsk_reconfig.py | 12 ++-- .../tests_rollover_csk_roll1.py | 7 +- .../tests_rollover_csk_roll2.py | 8 +-- .../tests_rollover_enable_dnssec.py | 4 +- .../tests_rollover_ksk_doubleksk.py | 8 +-- .../tests_rollover_zsk_prepublication.py | 6 +- bin/tests/system/spf/tests_spf_zones.py | 4 +- 17 files changed, 101 insertions(+), 91 deletions(-) diff --git a/bin/tests/system/dnssec/tests_tat.py b/bin/tests/system/dnssec/tests_tat.py index e37644f3a9..543207a117 100644 --- a/bin/tests/system/dnssec/tests_tat.py +++ b/bin/tests/system/dnssec/tests_tat.py @@ -60,7 +60,7 @@ def test_tat_queries(ns1, ns6): watcher.wait_for_line("trust-anchor-telemetry '_ta-") # check that _ta-AAAA trust-anchor-telemetry are not sent when disabled - ns1.log.prohibit("sending trust-anchor-telemetry query '_ta") + assert "sending trust-anchor-telemetry query '_ta" not in ns1.log # check that KEY-TAG (ednsopt 14) trust-anchor-telemetry queries are # logged. this matches "dig . dnskey +ednsopt=KEY-TAG:ffff": diff --git a/bin/tests/system/dnssec/tests_validation.py b/bin/tests/system/dnssec/tests_validation.py index b8f6baa4f8..7c3cc24d38 100644 --- a/bin/tests/system/dnssec/tests_validation.py +++ b/bin/tests/system/dnssec/tests_validation.py @@ -9,8 +9,8 @@ # See the COPYRIGHT file distributed with this work for additional # information regarding copyright ownership. +from re import compile as Re import os -import re import shutil import time @@ -54,14 +54,6 @@ pytestmark = pytest.mark.extra_artifacts( ) -# helper functions -def grep_q(regex, filename): - with open(filename, "r", encoding="utf-8") as f: - blob = f.read().splitlines() - results = [x for x in blob if re.search(regex, x)] - return len(results) != 0 - - def getfrom(file): with open(file, encoding="utf-8") as f: return f.read().strip() @@ -501,13 +493,10 @@ def test_negative_validation_nsec3(): isctest.check.servfail(res2) -def test_excessive_nsec3_iterations(): - assert grep_q( - "zone too-many-iterations/IN: excessive NSEC3PARAM iterations", "ns2/named.run" - ) - assert grep_q( - "zone too-many-iterations/IN: excessive NSEC3PARAM iterations", "ns3/named.run" - ) +def test_excessive_nsec3_iterations(ns2, ns3): + msg = "zone too-many-iterations/IN: excessive NSEC3PARAM iterations" + assert msg in ns2.log + assert msg in ns3.log # check fallback to insecure with NSEC3 iterations is too high msg = isctest.query.create("does-not-exist.too-many-iterations", "A") @@ -721,10 +710,11 @@ def test_negative_validation_optout(): def test_cache(ns4): # check that key id's are logged when dumping the cache ns4.rndc("dumpdb -cache", log=False) - assert grep_q("; key id = ", "ns4/named_dump.db") + dumpdb = isctest.text.TextFile("ns4/named_dump.db") + assert "; key id = " in dumpdb # check for RRSIG covered type in negative cache - assert grep_q("; example. RRSIG NSEC ", "ns4/named_dump.db") + assert "; example. RRSIG NSEC " in dumpdb # check validated data are not cached longer than originalttl msg = isctest.query.create("a.ttlpatch.example", "A") @@ -952,7 +942,8 @@ def test_validation_recovery(ns2, ns4): res = isctest.query.tcp(msg, "10.53.0.4") isctest.check.servfail(res) ns4.rndc("dumpdb", log=False) - grep_q("10.53.0.100", "ns4/named_dump.db") + dumpdb = isctest.text.TextFile("ns4/named_dump.db") + assert "10.53.0.100" in dumpdb # then reload server with properly signed zone shutil.copyfile( @@ -1136,7 +1127,7 @@ def test_expired_signatures(ns4): isctest.check.servfail(res) isctest.check.noadflag(res) isctest.check.ede(res, EDECode.SIGNATURE_EXPIRED) - assert grep_q("expired.example/.*: RRSIG has expired", "ns4/named.run") + assert Re("expired.example/.*: RRSIG has expired") in ns4.log # check future signatures do not validate msg = isctest.query.create("future.example", "SOA") @@ -1144,9 +1135,7 @@ def test_expired_signatures(ns4): isctest.check.servfail(res) isctest.check.noadflag(res) isctest.check.ede(res, EDECode.SIGNATURE_NOT_YET_VALID) - assert grep_q( - "future.example/.*: RRSIG validity period has not begun", "ns4/named.run" - ) + assert Re("future.example/.*: RRSIG validity period has not begun") in ns4.log # check that a dynamic zone with future signatures is re-signed on load msg = isctest.query.create("managed-future.example", "A") diff --git a/bin/tests/system/isctest/instance.py b/bin/tests/system/isctest/instance.py index d6400c5e39..9e8ac6b5dc 100644 --- a/bin/tests/system/isctest/instance.py +++ b/bin/tests/system/isctest/instance.py @@ -21,10 +21,11 @@ import re import dns.message import dns.rcode -from .log import debug, info, LogFile, WatchLogFromStart, WatchLogFromHere +from .log import debug, info, WatchLogFromStart, WatchLogFromHere from .rndc import RNDCBinaryExecutor, RNDCException, RNDCExecutor from .run import perl from .query import udp +from .text import TextFile class NamedPorts(NamedTuple): @@ -87,7 +88,7 @@ class NamedInstance: if ports is None: ports = NamedPorts.from_env() self.ports = ports - self.log = LogFile(os.path.join(identifier, "named.run")) + self.log = TextFile(os.path.join(identifier, "named.run")) self._rndc_executor = rndc_executor or RNDCBinaryExecutor() self._rndc_logger = rndc_logger @@ -239,3 +240,6 @@ class NamedInstance: f"{os.environ['srcdir']}/start.pl", [self.system_test_name, self.identifier] + args, ) + + def __repr__(self): + return self.identifier diff --git a/bin/tests/system/isctest/kasp.py b/bin/tests/system/isctest/kasp.py index f46ceabe73..de3edc8e55 100644 --- a/bin/tests/system/isctest/kasp.py +++ b/bin/tests/system/isctest/kasp.py @@ -1099,9 +1099,8 @@ def check_cdslog(server, zone, key, substr): def check_cdslog_prohibit(server, zone, key, substr): - server.log.prohibit( - f"{substr} for key {zone}/{key.algorithm.name}/{key.tag} is now published" - ) + msg = f"{substr} for key {zone}/{key.algorithm.name}/{key.tag} is now published" + assert msg not in server.log def check_cdsdelete(rrset, expected): diff --git a/bin/tests/system/isctest/log/__init__.py b/bin/tests/system/isctest/log/__init__.py index 5975ae7892..228f69e5bf 100644 --- a/bin/tests/system/isctest/log/__init__.py +++ b/bin/tests/system/isctest/log/__init__.py @@ -23,4 +23,4 @@ from .basic import ( critical, ) -from .watchlog import LogFile, WatchLogFromStart, WatchLogFromHere +from .watchlog import WatchLogFromStart, WatchLogFromHere diff --git a/bin/tests/system/isctest/log/watchlog.py b/bin/tests/system/isctest/log/watchlog.py index 3504452964..447879662a 100644 --- a/bin/tests/system/isctest/log/watchlog.py +++ b/bin/tests/system/isctest/log/watchlog.py @@ -15,7 +15,7 @@ import abc import os import time -from isctest.text import compile_pattern, FlexPattern, LineReader, LogFile +from isctest.text import compile_pattern, FlexPattern, LineReader T = TypeVar("T") diff --git a/bin/tests/system/isctest/text.py b/bin/tests/system/isctest/text.py index 902917ce19..46b710e454 100644 --- a/bin/tests/system/isctest/text.py +++ b/bin/tests/system/isctest/text.py @@ -14,7 +14,7 @@ import abc import re from re import compile as Re -from typing import Iterator, Match, Optional, Pattern, TextIO, Union +from typing import Iterator, List, Match, Optional, Pattern, TextIO, Union FlexPattern = Union[str, Pattern] @@ -28,41 +28,60 @@ def compile_pattern(string: FlexPattern) -> Pattern: raise TypeError("only string and re.Pattern allowed") -class LogFile: +class Grep(abc.ABC): """ - Log file wrapper with a path and means to find a string in its contents. + Implement a grep-like interface for pattern matching in texts and files. + """ + + @abc.abstractmethod + def readlines(self) -> Iterator[str]: + raise NotImplementedError + + def igrep(self, pattern: FlexPattern) -> Iterator[Match]: + """ + Iterate over the lines matching the pattern. + """ + regex = compile_pattern(pattern) + + for line in self.readlines(): + match = regex.search(line) + if match: + yield match + + def grep(self, pattern: FlexPattern) -> List[Match]: + """ + Get list of lines matching the pattern. + """ + return list(self.igrep(pattern)) + + def __contains__(self, pattern: FlexPattern) -> bool: + """ + Return whether any of the lines in the log contains matches the pattern. + """ + try: + next(self.igrep(pattern)) + except StopIteration: + return False + return True + + +class TextFile(Grep): + """ + Text file wrapper with grep support. """ def __init__(self, path: str): self.path = path - @property - def _lines(self) -> Iterator[str]: + def readlines(self) -> Iterator[str]: with open(self.path, encoding="utf-8") as f: yield from f - def __contains__(self, substring: str) -> bool: - """ - Return whether any of the lines in the log contains a given string. - """ - for line in self._lines: - if substring in line: - return True - return False - - def expect(self, msg: str): - """Check the string is present anywhere in the log file.""" - if msg in self: - return - assert False, f"log message not found in log {self.path}: {msg}" - - def prohibit(self, msg: str): - """Check the string is not present in the entire log file.""" - if msg in self: - assert False, f"forbidden message appeared in log {self.path}: {msg}" + def __repr__(self): + return self.path -class LineReader: +class LineReader(Grep): """ >>> import io diff --git a/bin/tests/system/kasp/tests_kasp.py b/bin/tests/system/kasp/tests_kasp.py index d5acb31c15..2e8d038e29 100644 --- a/bin/tests/system/kasp/tests_kasp.py +++ b/bin/tests/system/kasp/tests_kasp.py @@ -1601,7 +1601,7 @@ def test_kasp_zsk_retired(ns3): watcher.wait_for_line(f"keymgr: {zone} done") msg = f"zone {zone}/IN (signed): zone_rekey:zone_verifykeys failed: some key files are missing" - ns3.log.prohibit(msg) + assert msg not in ns3.log def test_kasp_purge_keys(ns4): @@ -1625,10 +1625,10 @@ def test_kasp_purge_keys(ns4): watcher.wait_for_line(f"keymgr: {zone} done") msg = f"zone {zone}/IN/example1 (signed): zone_rekey:zone_verifykeys failed: some key files are missing" - ns4.log.prohibit(msg) + assert msg not in ns4.log msg = f"zone {zone}/IN/example2 (signed): zone_rekey:zone_verifykeys failed: some key files are missing" - ns4.log.prohibit(msg) + assert msg not in ns4.log def test_kasp_reload_restart(ns6): @@ -1730,7 +1730,7 @@ def test_kasp_manual_mode(ns3): # Key rollover should have been be blocked. tag = expected[1].key.tag blockmsg = f"keymgr-manual-mode: block ZSK rollover for key {zone}/ECDSAP256SHA256/{tag} (policy {policy})" - ns3.log.expect(blockmsg) + assert blockmsg in ns3.log # Remove files. for key in ksks + zsks: diff --git a/bin/tests/system/multisigner/tests_multisigner.py b/bin/tests/system/multisigner/tests_multisigner.py index 900cc7e7b4..40f6d9e04c 100644 --- a/bin/tests/system/multisigner/tests_multisigner.py +++ b/bin/tests/system/multisigner/tests_multisigner.py @@ -179,7 +179,7 @@ def check_add_zsk(server, zone, keys, expected, extra_keys, extra, primary=None) isctest.log.info( f"- zone {zone} {server.identifier}: make sure we did not try to sign with the keys added with nsupdate" ) - server.log.prohibit(f"dns_zone_findkeys: error reading ./K{zone}") + assert f"dns_zone_findkeys: error reading ./K{zone}" not in server.log # Trigger keymgr. with server.watch_log_from_here() as watcher: @@ -189,7 +189,7 @@ def check_add_zsk(server, zone, keys, expected, extra_keys, extra, primary=None) # Check again. isctest.log.info(f"- zone {zone} {server.identifier}: check again after keymgr run") check_dnssec(server, zone, keys + extra_keys, expected + extra) - server.log.prohibit(f"dns_zone_findkeys: error reading ./K{zone}") + assert f"dns_zone_findkeys: error reading ./K{zone}" not in server.log def _check_remove_zsk_fail( diff --git a/bin/tests/system/rollover-algo-csk/tests_rollover_algo_csk_reconfig.py b/bin/tests/system/rollover-algo-csk/tests_rollover_algo_csk_reconfig.py index 2fcbd6290a..f9d5edb455 100644 --- a/bin/tests/system/rollover-algo-csk/tests_rollover_algo_csk_reconfig.py +++ b/bin/tests/system/rollover-algo-csk/tests_rollover_algo_csk_reconfig.py @@ -82,8 +82,8 @@ def test_algoroll_csk_reconfig_step1(tld, ns6, alg, size): tag = keys[0].key.tag msg1 = f"keymgr-manual-mode: block retire DNSKEY {zone}/RSASHA256/{tag} (CSK)" msg2 = f"keymgr-manual-mode: block new key generation for zone {zone} (policy {policy})" - ns6.log.expect(msg1) - ns6.log.expect(msg2) + assert msg1 in ns6.log + assert msg2 in ns6.log # Force step. with ns6.watch_log_from_here() as watcher: @@ -175,7 +175,7 @@ def test_algoroll_csk_reconfig_step3(tld, ns6, alg, size): # Check logs. tag = keys[1].key.tag msg = f"keymgr-manual-mode: block transition CSK {zone}/ECDSAP256SHA256/{tag} type DS state HIDDEN to state RUMOURED" - ns6.log.expect(msg) + assert msg in ns6.log # Force step. with ns6.watch_log_from_here() as watcher: @@ -244,7 +244,7 @@ def test_algoroll_csk_reconfig_step4(tld, ns6, alg, size): # Check logs. tag = keys[0].key.tag msg = f"keymgr-manual-mode: block transition CSK {zone}/RSASHA256/{tag} type DNSKEY state OMNIPRESENT to state UNRETENTIVE" - ns6.log.expect(msg) + assert msg in ns6.log # Force step. tag = keys[1].key.tag diff --git a/bin/tests/system/rollover-algo-ksk-zsk/tests_rollover_algo_ksk_zsk_reconfig.py b/bin/tests/system/rollover-algo-ksk-zsk/tests_rollover_algo_ksk_zsk_reconfig.py index 590dc4ec3c..228e65045e 100644 --- a/bin/tests/system/rollover-algo-ksk-zsk/tests_rollover_algo_ksk_zsk_reconfig.py +++ b/bin/tests/system/rollover-algo-ksk-zsk/tests_rollover_algo_ksk_zsk_reconfig.py @@ -85,9 +85,9 @@ def test_algoroll_ksk_zsk_reconfig_step1(tld, ns6, alg, size): msg1 = f"keymgr-manual-mode: block retire DNSKEY {zone}/RSASHA256/{ktag} (KSK)" msg2 = f"keymgr-manual-mode: block retire DNSKEY {zone}/RSASHA256/{ztag} (ZSK)" msg3 = f"keymgr-manual-mode: block new key generation for zone {zone} (policy {policy})" # twice - ns6.log.expect(msg1) - ns6.log.expect(msg2) - ns6.log.expect(msg3) + assert msg1 in ns6.log + assert msg2 in ns6.log + assert len(ns6.log.grep(msg3)) >= 2 # Force step. with ns6.watch_log_from_here() as watcher: @@ -184,7 +184,7 @@ def test_algoroll_ksk_zsk_reconfig_step3(tld, ns6, alg, size): # Check logs. tag = keys[2].key.tag msg = f"keymgr-manual-mode: block transition KSK {zone}/ECDSAP256SHA256/{tag} type DS state HIDDEN to state RUMOURED" - ns6.log.expect(msg) + assert msg in ns6.log # Force step. with ns6.watch_log_from_here() as watcher: @@ -259,8 +259,8 @@ def test_algoroll_ksk_zsk_reconfig_step4(tld, ns6, alg, size): ztag = keys[1].key.tag msg1 = f"keymgr-manual-mode: block transition KSK {zone}/RSASHA256/{ktag} type DNSKEY state OMNIPRESENT to state UNRETENTIVE" msg2 = f"keymgr-manual-mode: block transition ZSK {zone}/RSASHA256/{ztag} type DNSKEY state OMNIPRESENT to state UNRETENTIVE" - ns6.log.expect(msg1) - ns6.log.expect(msg2) + assert msg1 in ns6.log + assert msg2 in ns6.log # Force step. ktag = keys[3].key.tag diff --git a/bin/tests/system/rollover-csk-roll1/tests_rollover_csk_roll1.py b/bin/tests/system/rollover-csk-roll1/tests_rollover_csk_roll1.py index b4b31df6fd..fcf4876b90 100644 --- a/bin/tests/system/rollover-csk-roll1/tests_rollover_csk_roll1.py +++ b/bin/tests/system/rollover-csk-roll1/tests_rollover_csk_roll1.py @@ -125,7 +125,7 @@ def test_csk_roll1_step2(tld, alg, size, ns3): # Check logs. tag = keys[0].key.tag msg = f"keymgr-manual-mode: block CSK rollover for key {zone}/ECDSAP256SHA256/{tag} (policy {policy})" - ns3.log.expect(msg) + assert msg in ns3.log # Force step. with ns3.watch_log_from_here() as watcher: @@ -185,7 +185,7 @@ def test_csk_roll1_step3(tld, alg, size, ns3): # Check logs. tag = keys[1].key.tag msg = f"keymgr-manual-mode: block transition CSK {zone}/ECDSAP256SHA256/{tag} type ZRRSIG state HIDDEN to state RUMOURED" - ns3.log.expect(msg) + assert msg in ns3.log # Force step. with ns3.watch_log_from_here() as watcher: @@ -274,8 +274,7 @@ def test_csk_roll1_step4(tld, alg, size, ns3): # Check logs. tag = keys[0].key.tag msg = f"keymgr-manual-mode: block transition CSK {zone}/ECDSAP256SHA256/{tag} type KRRSIG state OMNIPRESENT to state UNRETENTIVE" - - ns3.log.expect(msg) + assert msg in ns3.log # Force step. tag = keys[1].key.tag diff --git a/bin/tests/system/rollover-csk-roll2/tests_rollover_csk_roll2.py b/bin/tests/system/rollover-csk-roll2/tests_rollover_csk_roll2.py index d18b13fe5d..5a3ae2205f 100644 --- a/bin/tests/system/rollover-csk-roll2/tests_rollover_csk_roll2.py +++ b/bin/tests/system/rollover-csk-roll2/tests_rollover_csk_roll2.py @@ -128,7 +128,7 @@ def test_csk_roll2_step2(tld, alg, size, ns3): # Check logs. tag = keys[0].key.tag msg = f"keymgr-manual-mode: block CSK rollover for key {zone}/ECDSAP256SHA256/{tag} (policy {policy})" - ns3.log.expect(msg) + assert msg in ns3.log # Force step. with ns3.watch_log_from_here() as watcher: @@ -188,7 +188,7 @@ def test_csk_roll2_step3(tld, alg, size, ns3): # Check logs. tag = keys[1].key.tag msg = f"keymgr-manual-mode: block transition CSK {zone}/ECDSAP256SHA256/{tag} type ZRRSIG state HIDDEN to state RUMOURED" - ns3.log.expect(msg) + assert msg in ns3.log # Force step. with ns3.watch_log_from_here() as watcher: @@ -315,8 +315,8 @@ def test_csk_roll2_step5(tld, alg, size, ns3): tag = keys[0].key.tag msg1 = f"keymgr-manual-mode: block transition CSK {zone}/ECDSAP256SHA256/{tag} type DNSKEY state OMNIPRESENT to state UNRETENTIVE" msg2 = f"keymgr-manual-mode: block transition CSK {zone}/ECDSAP256SHA256/{tag} type KRRSIG state OMNIPRESENT to state UNRETENTIVE" - ns3.log.expect(msg1) - ns3.log.expect(msg2) + assert msg1 in ns3.log + assert msg2 in ns3.log # Force step. tag = keys[1].key.tag diff --git a/bin/tests/system/rollover-enable-dnssec/tests_rollover_enable_dnssec.py b/bin/tests/system/rollover-enable-dnssec/tests_rollover_enable_dnssec.py index e4d83be42f..ddcb6a5c79 100644 --- a/bin/tests/system/rollover-enable-dnssec/tests_rollover_enable_dnssec.py +++ b/bin/tests/system/rollover-enable-dnssec/tests_rollover_enable_dnssec.py @@ -74,7 +74,7 @@ def test_rollover_enable_dnssec_step1(tld, alg, size, ns3): # Check logs. msg = f"keymgr-manual-mode: block new key generation for zone {zone} (policy {policy})" - ns3.log.expect(msg) + assert msg in ns3.log # Force step. with ns3.watch_log_from_here() as watcher: @@ -155,7 +155,7 @@ def test_rollover_enable_dnssec_step3(tld, alg, size, ns3): # Check logs. tag = keys[0].key.tag msg = f"keymgr-manual-mode: block transition CSK {zone}/ECDSAP256SHA256/{tag} type DS state HIDDEN to state RUMOURED" - ns3.log.expect(msg) + assert msg in ns3.log # Force step. with ns3.watch_log_from_here() as watcher: diff --git a/bin/tests/system/rollover-ksk-doubleksk/tests_rollover_ksk_doubleksk.py b/bin/tests/system/rollover-ksk-doubleksk/tests_rollover_ksk_doubleksk.py index 68dc712b0b..26653a6b53 100644 --- a/bin/tests/system/rollover-ksk-doubleksk/tests_rollover_ksk_doubleksk.py +++ b/bin/tests/system/rollover-ksk-doubleksk/tests_rollover_ksk_doubleksk.py @@ -110,7 +110,7 @@ def test_ksk_doubleksk_step2(tld, alg, size, ns3): # Check logs. tag = keys[1].key.tag msg = f"keymgr-manual-mode: block KSK rollover for key {zone}/ECDSAP256SHA256/{tag} (policy {policy})" - ns3.log.expect(msg) + assert msg in ns3.log # Force step. with ns3.watch_log_from_here() as watcher: @@ -171,7 +171,7 @@ def test_ksk_doubleksk_step3(tld, alg, size, ns3): # Check logs. tag = keys[2].key.tag msg = f"keymgr-manual-mode: block transition KSK {zone}/ECDSAP256SHA256/{tag} type DS state HIDDEN to state RUMOURED" - ns3.log.expect(msg) + assert msg in ns3.log # Force step. with ns3.watch_log_from_here() as watcher: @@ -252,8 +252,8 @@ def test_ksk_doubleksk_step4(tld, alg, size, ns3): tag = keys[1].key.tag msg1 = f"keymgr-manual-mode: block transition KSK {zone}/ECDSAP256SHA256/{tag} type DNSKEY state OMNIPRESENT to state UNRETENTIVE" msg2 = f"keymgr-manual-mode: block transition KSK {zone}/ECDSAP256SHA256/{tag} type KRRSIG state OMNIPRESENT to state UNRETENTIVE" - ns3.log.expect(msg1) - ns3.log.expect(msg2) + assert msg1 in ns3.log + assert msg2 in ns3.log # Force step. tag = keys[2].key.tag diff --git a/bin/tests/system/rollover-zsk-prepub/tests_rollover_zsk_prepublication.py b/bin/tests/system/rollover-zsk-prepub/tests_rollover_zsk_prepublication.py index 73f249b258..17b7d38a57 100644 --- a/bin/tests/system/rollover-zsk-prepub/tests_rollover_zsk_prepublication.py +++ b/bin/tests/system/rollover-zsk-prepub/tests_rollover_zsk_prepublication.py @@ -117,7 +117,7 @@ def test_zsk_prepub_step2(tld, alg, size, ns3): # Check logs. tag = keys[1].key.tag msg = f"keymgr-manual-mode: block ZSK rollover for key {zone}/ECDSAP256SHA256/{tag} (policy {policy})" - ns3.log.expect(msg) + assert msg in ns3.log # Force step. with ns3.watch_log_from_here() as watcher: @@ -176,7 +176,7 @@ def test_zsk_prepub_step3(tld, alg, size, ns3): # Check logs. tag = keys[2].key.tag msg = f"keymgr-manual-mode: block transition ZSK {zone}/ECDSAP256SHA256/{tag} type ZRRSIG state HIDDEN to state RUMOURED" - ns3.log.expect(msg) + assert msg in ns3.log # Force step. with ns3.watch_log_from_here() as watcher: @@ -263,7 +263,7 @@ def test_zsk_prepub_step4(tld, alg, size, ns3): # Check logs. tag = keys[1].key.tag msg = f"keymgr-manual-mode: block transition ZSK {zone}/ECDSAP256SHA256/{tag} type DNSKEY state OMNIPRESENT to state UNRETENTIVE" - ns3.log.expect(msg) + assert msg in ns3.log # Force step. tag = keys[2].key.tag diff --git a/bin/tests/system/spf/tests_spf_zones.py b/bin/tests/system/spf/tests_spf_zones.py index bcb08e9e5b..5cb7348c3c 100644 --- a/bin/tests/system/spf/tests_spf_zones.py +++ b/bin/tests/system/spf/tests_spf_zones.py @@ -21,7 +21,7 @@ def test_spf_log(ns1): "zone warn/IN: loaded serial 0", "zone nowarn/IN: loaded serial 0", ): - ns1.log.expect(msg) + assert msg in ns1.log for msg in ( "zone nowarn/IN: 'y.nowarn' found type SPF record but no SPF TXT record found", @@ -29,4 +29,4 @@ def test_spf_log(ns1): "zone warn/IN: 'warn' found type SPF record but no SPF TXT record found", "zone nowarn/IN: 'nowarn' found type SPF record but no SPF TXT record found", ): - ns1.log.prohibit(msg) + assert msg not in ns1.log From 4b6a86b029a34c939b2e265ccab43f4f1b9a5808 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicki=20K=C5=99=C3=AD=C5=BEek?= Date: Thu, 2 Oct 2025 18:14:13 +0200 Subject: [PATCH 06/10] Use Text with Grep support in isctest.run.cmd() When commands are executed using the isctest.run.cmd() command, allow the output to be Grep-able like logs and text files. --- bin/tests/system/dnssec/tests_delv.py | 73 +++++++++---------- bin/tests/system/dnssec/tests_signing.py | 13 +--- bin/tests/system/isctest/run.py | 9 ++- bin/tests/system/isctest/text.py | 9 +++ .../system/keyfromlabel/tests_keyfromlabel.py | 4 +- .../system/multisigner/tests_multisigner.py | 7 +- bin/tests/system/verify/tests_verify.py | 50 ++++++------- 7 files changed, 75 insertions(+), 90 deletions(-) diff --git a/bin/tests/system/dnssec/tests_delv.py b/bin/tests/system/dnssec/tests_delv.py index 9727db3a01..5c24abb2cf 100644 --- a/bin/tests/system/dnssec/tests_delv.py +++ b/bin/tests/system/dnssec/tests_delv.py @@ -10,7 +10,7 @@ # information regarding copyright ownership. import os -import re +from re import compile as Re import subprocess import pytest @@ -49,13 +49,6 @@ pytestmark = pytest.mark.extra_artifacts( ) -# helper functions -def grep_c(regex, data): - blob = data.splitlines() - results = [x for x in blob if re.search(regex, x)] - return len(results) - - # run delv def delv(*args, tkeys=False): delv_cmd = [os.environ.get("DELV")] @@ -70,115 +63,115 @@ def delv(*args, tkeys=False): def test_positive_validation_delv(): # check positive validation NSEC response = delv("a", "a.example") - assert grep_c("a.example..*10.0.0.1", response.out) - assert grep_c("a.example..*.RRSIG.A [0-9][0-9]* 2 300 .*", response.out) + assert Re("a.example..*10.0.0.1") in response.out + assert Re("a.example..*.RRSIG.A [0-9][0-9]* 2 300 .*") in response.out # check positive validation NSEC (trsuted-keys) response = delv("a", "a.example", tkeys=True) - assert grep_c("a.example..*10.0.0.1", response.out) - assert grep_c("a.example..*.RRSIG.A [0-9][0-9]* 2 300 .*", response.out) + assert Re("a.example..*10.0.0.1") in response.out + assert Re("a.example..*.RRSIG.A [0-9][0-9]* 2 300 .*") in response.out # check positive validation NSEC3 response = delv("a", "a.nsec3.example") - assert grep_c("a.nsec3.example..*10.0.0.1", response.out) - assert grep_c("a.nsec3.example..*.RRSIG.A [0-9][0-9]* 3 300 .*", response.out) + assert Re("a.nsec3.example..*10.0.0.1") in response.out + assert Re("a.nsec3.example..*.RRSIG.A [0-9][0-9]* 3 300 .*") in response.out # check positive validation OPTOUT response = delv("a", "a.optout.example") - assert grep_c("a.optout.example..*10.0.0.1", response.out) - assert grep_c("a.optout.example..*.RRSIG.A [0-9][0-9]* 3 300 .*", response.out) + assert Re("a.optout.example..*10.0.0.1") in response.out + assert Re("a.optout.example..*.RRSIG.A [0-9][0-9]* 3 300 .*") in response.out # check positive wildcard validation NSEC response = delv("a", "a.wild.example") - assert grep_c("a.wild.example..*10.0.0.27", response.out) - assert grep_c("a.wild.example..*.RRSIG.A [0-9][0-9]* 2 300 .*", response.out) + assert Re("a.wild.example..*10.0.0.27") in response.out + assert Re("a.wild.example..*.RRSIG.A [0-9][0-9]* 2 300 .*") in response.out # check positive wildcard validation NSEC3 response = delv("a", "a.wild.nsec3.example") - assert grep_c("a.wild.nsec3.example..*10.0.0.6", response.out) - assert grep_c("a.wild.nsec3.example..*.RRSIG.A [0-9][0-9]* 3 300 .*", response.out) + assert Re("a.wild.nsec3.example..*10.0.0.6") in response.out + assert Re("a.wild.nsec3.example..*.RRSIG.A [0-9][0-9]* 3 300 .*") in response.out # check positive wildcard validation OPTOUT response = delv("a", "a.wild.optout.example") - assert grep_c("a.wild.optout.example..*10.0.0.6", response.out) - assert grep_c("a.wild.optout.example..*.RRSIG.A [0-9][0-9]* 3 300 .*", response.out) + assert Re("a.wild.optout.example..*10.0.0.6") in response.out + assert Re("a.wild.optout.example..*.RRSIG.A [0-9][0-9]* 3 300 .*") in response.out def test_negative_validation_delv(): # checking negative validation NXDOMAIN NSEC response = delv("a", "q.example") - assert grep_c("resolution failed: ncache nxdomain", response.out) + assert "resolution failed: ncache nxdomain" in response.out # checking negative validation NODATA NSEC response = delv("txt", "a.example") - assert grep_c("resolution failed: ncache nxrrset", response.out) + assert "resolution failed: ncache nxrrset" in response.out # checking negative validation NXDOMAIN NSEC3 response = delv("a", "q.nsec3.example") - assert grep_c("resolution failed: ncache nxdomain", response.out) + assert "resolution failed: ncache nxdomain" in response.out # checking negative validation NODATA NSEC3 response = delv("txt", "a.nsec3.example") - assert grep_c("resolution failed: ncache nxrrset", response.out) + assert "resolution failed: ncache nxrrset" in response.out # checking negative validation NXDOMAIN OPTOUT response = delv("a", "q.optout.example") - assert grep_c("resolution failed: ncache nxdomain", response.out) + assert "resolution failed: ncache nxdomain" in response.out # checking negative validation NODATA OPTOUT response = delv("txt", "a.optout.example") - assert grep_c("resolution failed: ncache nxrrset", response.out) + assert "resolution failed: ncache nxrrset" in response.out # checking negative wildcard validation NSEC response = delv("txt", "b.wild.example") - assert grep_c("resolution failed: ncache nxrrset", response.out) + assert "resolution failed: ncache nxrrset" in response.out # checking negative wildcard validation NSEC3 response = delv("txt", "b.wild.nsec3.example") - assert grep_c("resolution failed: ncache nxrrset", response.out) + assert "resolution failed: ncache nxrrset" in response.out # checking negative wildcard validation OPTOUT response = delv("txt", "b.wild.optout.example") - assert grep_c("resolution failed: ncache nxrrset", response.out) + assert "resolution failed: ncache nxrrset" in response.out def test_insecure_validation_delv(): # check 1-server insecurity proof NSEC response = delv("a", "a.insecure.example") - assert grep_c("a.insecure.example..*10.0.0.1", response.out) + assert Re("a.insecure.example..*10.0.0.1") in response.out # check 1-server insecurity proof NSEC3 response = delv("a", "a.insecure.nsec3.example") - assert grep_c("a.insecure.nsec3.example..*10.0.0.1", response.out) + assert Re("a.insecure.nsec3.example..*10.0.0.1") in response.out # check 1-server insecurity proof NSEC3 response = delv("a", "a.insecure.optout.example") - assert grep_c("a.insecure.optout.example..*10.0.0.1", response.out) + assert Re("a.insecure.optout.example..*10.0.0.1") in response.out # check 1-server negative insecurity proof NSEC response = delv("a", "q.insecure.example") - assert grep_c("resolution failed: ncache nxdomain", response.out) + assert "resolution failed: ncache nxdomain" in response.out # check 1-server negative insecurity proof NSEC3 response = delv("a", "q.insecure.nsec3.example") - assert grep_c("resolution failed: ncache nxdomain", response.out) + assert "resolution failed: ncache nxdomain" in response.out # check 1-server negative insecurity proof OPTOUT response = delv("a", "q.insecure.optout.example") - assert grep_c("resolution failed: ncache nxdomain", response.out) + assert "resolution failed: ncache nxdomain" in response.out def test_validation_failure_delv(): # check failed validation due to bogus data response = delv("+cd", "a", "a.bogus.example") - assert grep_c("resolution failed: RRSIG failed to verify", response.out) + assert "resolution failed: RRSIG failed to verify" in response.out # check failed validation due to missing key record response = delv("+cd", "a", "a.b.keyless.example") - assert grep_c("resolution failed: insecurity proof failed", response.out) + assert "resolution failed: insecurity proof failed" in response.out def test_revoked_key_delv(): # check failed validation succeeds when a revoked key is encountered response = delv("+cd", "soa", "revkey.example") - assert grep_c("fully validated", response.out) + assert "fully validated" in response.out diff --git a/bin/tests/system/dnssec/tests_signing.py b/bin/tests/system/dnssec/tests_signing.py index 2a1abd531d..682c3d7cfb 100644 --- a/bin/tests/system/dnssec/tests_signing.py +++ b/bin/tests/system/dnssec/tests_signing.py @@ -52,14 +52,6 @@ pytestmark = pytest.mark.extra_artifacts( ) -# helper functions -def grep_c(regex, filename): - with open(filename, "r", encoding="utf-8") as f: - blob = f.read().splitlines() - results = [x for x in blob if re.search(regex, x)] - return len(results) - - # run dnssec-keygen def keygen(*args): keygen_cmd = [os.environ.get("KEYGEN")] @@ -126,7 +118,7 @@ def test_split_dnssec(): isctest.check.adflag(res2) -def test_expiring_rrsig(): +def test_expiring_rrsig(ns3): # check soon-to-expire RRSIGs without a replacement private # key aren't deleted. this response has to have an RRSIG: msg = isctest.query.create("expiring.example.", "NS") @@ -135,8 +127,7 @@ def test_expiring_rrsig(): assert sigs # check that named doesn't loop when private keys are not available - n = grep_c("reading private key file expiring.example", "ns3/named.run") - assert n < 15 + assert len(ns3.log.grep("reading private key file expiring.example")) < 15 # check expired signatures stay place when updates are disabled msg = isctest.query.create("expired.example", "SOA") diff --git a/bin/tests/system/isctest/run.py b/bin/tests/system/isctest/run.py index a052f65d3d..530d413751 100644 --- a/bin/tests/system/isctest/run.py +++ b/bin/tests/system/isctest/run.py @@ -16,18 +16,19 @@ import time from typing import List, Optional import isctest.log +import isctest.text class CmdResult: def __init__(self, proc=None): self.proc = proc self.rc = self.proc.returncode - self.out = "" - self.err = "" + self.out = isctest.text.Text("") + self.err = isctest.text.Text("") if self.proc.stdout: - self.out = self.proc.stdout.decode("utf-8") + self.out = isctest.text.Text(self.proc.stdout.decode("utf-8")) if self.proc.stderr: - self.err = self.proc.stderr.decode("utf-8") + self.err = isctest.text.Text(self.proc.stderr.decode("utf-8")) def cmd( diff --git a/bin/tests/system/isctest/text.py b/bin/tests/system/isctest/text.py index 46b710e454..ca3cc835e7 100644 --- a/bin/tests/system/isctest/text.py +++ b/bin/tests/system/isctest/text.py @@ -65,6 +65,15 @@ class Grep(abc.ABC): return True +class Text(Grep, str): # type: ignore + """ + Wrapper around classic string with grep support. + """ + + def readlines(self): + yield from self.splitlines(keepends=True) + + class TextFile(Grep): """ Text file wrapper with grep support. diff --git a/bin/tests/system/keyfromlabel/tests_keyfromlabel.py b/bin/tests/system/keyfromlabel/tests_keyfromlabel.py index da9b391313..0eb9370060 100644 --- a/bin/tests/system/keyfromlabel/tests_keyfromlabel.py +++ b/bin/tests/system/keyfromlabel/tests_keyfromlabel.py @@ -11,7 +11,7 @@ import hashlib import os -import re +from re import compile as Re import shutil import pytest @@ -83,7 +83,7 @@ def token_init_and_cleanup(): env=EMPTY_OPENSSL_CONF_ENV, raise_on_exception=False, ) - assert re.search("Found token (.*) with matching token label", cmd.out) + assert Re("Found token (.*) with matching token label") in cmd.out # pylint: disable-msg=too-many-locals diff --git a/bin/tests/system/multisigner/tests_multisigner.py b/bin/tests/system/multisigner/tests_multisigner.py index 40f6d9e04c..020d39791d 100644 --- a/bin/tests/system/multisigner/tests_multisigner.py +++ b/bin/tests/system/multisigner/tests_multisigner.py @@ -11,7 +11,6 @@ from datetime import timedelta import os -import re from re import compile as Re import pytest @@ -104,9 +103,9 @@ def check_no_dnssec_in_journal(server, zone): ] cmd = isctest.run.cmd(journalprint) - pattern = Re(r"^\s*(?:\S+\s+){4}(NSEC|NSEC3|NSEC3PARAM|RRSIG)", flags=re.MULTILINE) - match = pattern.search(cmd.out) - assert not match, f"{match.group(1)} record found in journal" + assert ( + Re(r"^\s*(?:\S+\s+){4}(NSEC|NSEC3|NSEC3PARAM|RRSIG)") not in cmd.out + ), "dnssec record found in journal" def wait_for_serial(primary, server, zone): diff --git a/bin/tests/system/verify/tests_verify.py b/bin/tests/system/verify/tests_verify.py index 9ec0933176..e5e780a532 100644 --- a/bin/tests/system/verify/tests_verify.py +++ b/bin/tests/system/verify/tests_verify.py @@ -11,7 +11,7 @@ import os import re -import subprocess +from re import compile as Re import pytest @@ -62,14 +62,14 @@ def test_verify_good_zone_nsec_next_name_case_mismatch(): ) -def get_bad_zone_output(zone): - only_opt = ["-z"] if re.match(r"[zk]sk-only", zone) else [] +def verify_bad_zone(zone): + only_opt = ["-z"] if re.search(r"^[zk]sk-only", zone) else [] cmd = isctest.run.cmd( [VERIFY, *only_opt, "-o", zone, f"zones/{zone}.bad"], - stderr=subprocess.STDOUT, raise_on_exception=False, ) - return cmd.out + assert cmd.rc != 0 + return cmd @pytest.mark.parametrize( @@ -81,7 +81,8 @@ def get_bad_zone_output(zone): ], ) def test_verify_bad_zone_files_dnskeyonly(zone): - assert re.match(r".*DNSKEY is not signed.*", get_bad_zone_output(zone)) + cmd = verify_bad_zone(zone) + assert "DNSKEY is not signed" in cmd.err @pytest.mark.parametrize( @@ -98,10 +99,8 @@ def test_verify_bad_zone_files_dnskeyonly(zone): ], ) def test_verify_bad_zone_files_expired(zone): - assert re.match( - r".*signature has expired.*|.*No self-signed .*DNSKEY found.*", - get_bad_zone_output(zone), - ) + cmd = verify_bad_zone(zone) + assert Re("signature has expired|No self-signed DNSKEY found") in cmd.err @pytest.mark.parametrize( @@ -113,40 +112,33 @@ def test_verify_bad_zone_files_expired(zone): ], ) def test_verify_bad_zone_files_unexpected_nsec_rrset(zone): - assert re.match(r".*unexpected NSEC RRset at.*", get_bad_zone_output(zone)) + cmd = verify_bad_zone(zone) + assert "unexpected NSEC RRset at" in cmd.err def test_verify_bad_zone_files_bad_nsec_record(): - assert re.match( - r".*Bad NSEC record for.*, next name mismatch.*", - get_bad_zone_output("ksk+zsk.nsec.broken-chain"), - ) + cmd = verify_bad_zone("ksk+zsk.nsec.broken-chain") + assert Re("Bad NSEC record for.*, next name mismatch") in cmd.err def test_verify_bad_zone_files_bad_bitmap(): - assert re.match( - r".*bit map mismatch.*", get_bad_zone_output("ksk+zsk.nsec.bad-bitmap") - ) + cmd = verify_bad_zone("ksk+zsk.nsec.bad-bitmap") + assert "bit map mismatch" in cmd.err def test_verify_bad_zone_files_missing_nsec3_record(): - assert re.match( - r".*Missing NSEC3 record for.*", - get_bad_zone_output("ksk+zsk.nsec3.missing-empty"), - ) + cmd = verify_bad_zone("ksk+zsk.nsec3.missing-empty") + assert "Missing NSEC3 record for" in cmd.err def test_verify_bad_zone_files_no_dnssec_keys(): - assert re.match( - r".*Zone contains no DNSSEC keys.*", get_bad_zone_output("unsigned") - ) + cmd = verify_bad_zone("unsigned") + assert "Zone contains no DNSSEC keys" in cmd.err def test_verify_bad_zone_files_unequal_nsec3_chains(): - assert re.match( - r".*Expected and found NSEC3 chains not equal.*", - get_bad_zone_output("ksk+zsk.nsec3.extra-nsec3"), - ) + cmd = verify_bad_zone("ksk+zsk.nsec3.extra-nsec3") + assert "Expected and found NSEC3 chains not equal" in cmd.err # checking error message when -o is not used From 9bad9491a14710208d0acab68631106665231f3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicki=20K=C5=99=C3=AD=C5=BEek?= Date: Thu, 2 Oct 2025 19:12:50 +0200 Subject: [PATCH 07/10] Improve file handling in ksr test Refactor the file handling to write to a file directly when calling isctest.run.cmd(). Refactor the existing code to use CmdResult rather than out and err separately. --- bin/tests/system/ksr/tests_ksr.py | 466 ++++++++++++++++-------------- 1 file changed, 242 insertions(+), 224 deletions(-) diff --git a/bin/tests/system/ksr/tests_ksr.py b/bin/tests/system/ksr/tests_ksr.py index fc869bb527..553eda1dff 100644 --- a/bin/tests/system/ksr/tests_ksr.py +++ b/bin/tests/system/ksr/tests_ksr.py @@ -85,7 +85,7 @@ def between(value, start, end): return start < value < end -def ksr(zone, policy, action, options="", raise_on_exception=True): +def ksr(zone, policy, action, options="", raise_on_exception=True, to_file=""): ksr_command = [ os.environ.get("KSR"), "-l", @@ -97,8 +97,14 @@ def ksr(zone, policy, action, options="", raise_on_exception=True): zone, ] - cmd = isctest.run.cmd(ksr_command, raise_on_exception=raise_on_exception) - return cmd.out, cmd.err # TODO return cmd instead + if to_file: + with open(to_file, "wb") as f: + cmd = isctest.run.cmd( + ksr_command, raise_on_exception=raise_on_exception, stdout=f + ) + else: + cmd = isctest.run.cmd(ksr_command, raise_on_exception=raise_on_exception) + return cmd def check_keys( @@ -259,8 +265,9 @@ def check_rrsig_bundle(bundle_keys, bundle_lines, zone, rrtype, sigend, sigstart assert count == len(bundle_lines) -def check_keysigningrequest(out, zsks, start, end): - lines = out.split("\n") +def check_keysigningrequest(path, zsks, start, end): + with open(path, "r", encoding="utf-8") as f: + lines = f.readlines() line_no = 0 inception = start @@ -302,14 +309,14 @@ def check_keysigningrequest(out, zsks, start, end): # trailing empty lines while line_no < len(lines): - assert lines[line_no] == "" + assert lines[line_no].rstrip() == "" line_no += 1 assert line_no == len(lines) def check_signedkeyresponse( - out, + path, zone, ksks, zsks, @@ -319,7 +326,8 @@ def check_signedkeyresponse( cdnskey=True, cds="SHA-256", ): - lines = out.split("\n") + with open(path, "r", encoding="utf-8") as f: + lines = f.readlines() line_no = 0 next_bundle = end + 1 @@ -341,7 +349,7 @@ def check_signedkeyresponse( # ignore empty lines while line_no < len(lines): - if lines[line_no] == "": + if lines[line_no].rstrip() == "": line_no += 1 else: break @@ -537,32 +545,32 @@ def check_signedkeyresponse( def test_ksr_errors(): # check that 'dnssec-ksr' errors on unknown action - _, err = ksr("common.test", "common", "foobar", raise_on_exception=False) - assert "dnssec-ksr: fatal: unknown command 'foobar'" in err + cmd = ksr("common.test", "common", "foobar", raise_on_exception=False) + assert "dnssec-ksr: fatal: unknown command 'foobar'" in cmd.err # check that 'dnssec-ksr keygen' errors on missing end date - _, err = ksr("common.test", "common", "keygen", raise_on_exception=False) - assert "dnssec-ksr: fatal: keygen requires an end date" in err + cmd = ksr("common.test", "common", "keygen", raise_on_exception=False) + assert "dnssec-ksr: fatal: keygen requires an end date" in cmd.err # check that 'dnssec-ksr keygen' errors on zone with csk - _, err = ksr( + cmd = ksr( "csk.test", "csk", "keygen", options="-K ns1 -e +2y", raise_on_exception=False ) - assert "dnssec-ksr: fatal: no keys created for policy 'csk'" in err + assert "dnssec-ksr: fatal: no keys created for policy 'csk'" in cmd.err # check that 'dnssec-ksr request' errors on missing end date - _, err = ksr("common.test", "common", "request", raise_on_exception=False) - assert "dnssec-ksr: fatal: request requires an end date" in err + cmd = ksr("common.test", "common", "request", raise_on_exception=False) + assert "dnssec-ksr: fatal: request requires an end date" in cmd.err # check that 'dnssec-ksr sign' errors on missing ksr file - _, err = ksr( + cmd = ksr( "common.test", "common", "sign", options="-K ns1/offline -i now -e +1y", raise_on_exception=False, ) - assert "dnssec-ksr: fatal: 'sign' requires a KSR file" in err + assert "dnssec-ksr: fatal: 'sign' requires a KSR file" in cmd.err def test_ksr_common(ns1): @@ -573,15 +581,15 @@ def test_ksr_common(ns1): # create ksk kskdir = "ns1/offline" - out, _ = ksr(zone, policy, "keygen", options=f"-K {kskdir} -i now -e +1y -o") - ksks = isctest.kasp.keystr_to_keylist(out, kskdir) + cmd = ksr(zone, policy, "keygen", options=f"-K {kskdir} -i now -e +1y -o") + ksks = isctest.kasp.keystr_to_keylist(cmd.out, kskdir) assert len(ksks) == 1 check_keys(ksks, None) # check that 'dnssec-ksr keygen' pregenerates right amount of keys - out, _ = ksr(zone, policy, "keygen", options="-i now -e +1y") - zsks = isctest.kasp.keystr_to_keylist(out) + cmd = ksr(zone, policy, "keygen", options="-i now -e +1y") + zsks = isctest.kasp.keystr_to_keylist(cmd.out) assert len(zsks) == 2 lifetime = timedelta(days=31 * 6) @@ -590,8 +598,8 @@ def test_ksr_common(ns1): # check that 'dnssec-ksr keygen' pregenerates right amount of keys # in the given key directory zskdir = "ns1" - out, _ = ksr(zone, policy, "keygen", options=f"-K {zskdir} -i now -e +1y") - zsks = isctest.kasp.keystr_to_keylist(out, zskdir) + cmd = ksr(zone, policy, "keygen", options=f"-K {zskdir} -i now -e +1y") + zsks = isctest.kasp.keystr_to_keylist(cmd.out, zskdir) assert len(zsks) == 2 lifetime = timedelta(days=31 * 6) @@ -608,33 +616,35 @@ def test_ksr_common(ns1): # check that 'dnssec-ksr request' creates correct ksr now = zsks[0].get_timing("Created") until = now + timedelta(days=365) - out, _ = ksr(zone, policy, "request", options=f"-K {zskdir} -i {now} -e +1y") - - fname = f"{zone}.ksr.{n}" - with open(fname, "w", encoding="utf-8") as file: - file.write(out) - - check_keysigningrequest(out, zsks, now, until) + ksr_fname = f"{zone}.ksr.{n}" + ksr( + zone, + policy, + "request", + options=f"-K {zskdir} -i {now} -e +1y", + to_file=ksr_fname, + ) + check_keysigningrequest(ksr_fname, zsks, now, until) # check that 'dnssec-ksr sign' creates correct skr - out, _ = ksr( - zone, policy, "sign", options=f"-K {kskdir} -f {fname} -i {now} -e +1y" - ) - - fname = f"{zone}.skr.{n}" - with open(fname, "w", encoding="utf-8") as file: - file.write(out) - refresh = -432000 # 5 days - check_signedkeyresponse(out, zone, ksks, zsks, now, until, refresh) + skr_fname = f"{zone}.skr.{n}" + ksr( + zone, + policy, + "sign", + options=f"-K {kskdir} -f {ksr_fname} -i {now} -e +1y", + to_file=skr_fname, + ) + check_signedkeyresponse(skr_fname, zone, ksks, zsks, now, until, refresh) # common test cases (2) n = 2 # check that 'dnssec-ksr keygen' selects pregenerated keys for # the same time bundle - out, _ = ksr(zone, policy, "keygen", options=f"-K {zskdir} -i {now} -e +1y") - selected_zsks = isctest.kasp.keystr_to_keylist(out, zskdir) + cmd = ksr(zone, policy, "keygen", options=f"-K {zskdir} -i {now} -e +1y") + selected_zsks = isctest.kasp.keystr_to_keylist(cmd.out, zskdir) assert len(selected_zsks) == 2 for index, key in enumerate(selected_zsks): assert zsks[index] == key @@ -648,13 +658,13 @@ def test_ksr_common(ns1): # check that 'dnssec-ksr keygen' generates only necessary keys for # overlapping time bundle - out, err = ksr(zone, policy, "keygen", options=f"-K {zskdir} -i {now} -e +2y -v 1") - overlapping_zsks = isctest.kasp.keystr_to_keylist(out, zskdir) + cmd = ksr(zone, policy, "keygen", options=f"-K {zskdir} -i {now} -e +2y -v 1") + overlapping_zsks = isctest.kasp.keystr_to_keylist(cmd.out, zskdir) assert len(overlapping_zsks) == 4 - selected = len(re.findall("Selecting key pair", err)) - generated = len(re.findall("Generating key pair", err)) - len( - re.findall("collide", err) + selected = len(re.findall("Selecting key pair", cmd.err)) + generated = len(re.findall("Generating key pair", cmd.err)) - len( + re.findall("collide", cmd.err) ) assert selected == 2 @@ -673,8 +683,8 @@ def test_ksr_common(ns1): ) # run 'dnssec-ksr keygen' again with verbosity 0 - out, _ = ksr(zone, policy, "keygen", options=f"-K {zskdir} -i {now} -e +2y") - overlapping_zsks2 = isctest.kasp.keystr_to_keylist(out, zskdir) + cmd = ksr(zone, policy, "keygen", options=f"-K {zskdir} -i {now} -e +2y") + overlapping_zsks2 = isctest.kasp.keystr_to_keylist(cmd.out, zskdir) assert len(overlapping_zsks2) == 4 check_keys(overlapping_zsks2, lifetime) for index, key in enumerate(overlapping_zsks2): @@ -682,47 +692,41 @@ def test_ksr_common(ns1): # check that 'dnssec-ksr request' creates correct ksr if the # interval is shorter - out, _ = ksr(zone, policy, "request", options=f"-K ns1 -i {now} -e +1y") - - fname = f"{zone}.ksr.{n}.shorter" - with open(fname, "w", encoding="utf-8") as file: - file.write(out) - - check_keysigningrequest(out, zsks, now, until) + ksr_fname = f"{zone}.ksr.{n}.shorter" + ksr(zone, policy, "request", options=f"-K ns1 -i {now} -e +1y", to_file=ksr_fname) + check_keysigningrequest(ksr_fname, zsks, now, until) # check that 'dnssec-ksr request' creates correct ksr with new interval - out, _ = ksr(zone, policy, "request", options=f"-K ns1 -i {now} -e +2y") - - fname = f"{zone}.ksr.{n}" - with open(fname, "w", encoding="utf-8") as file: - file.write(out) - until = now + timedelta(days=365 * 2) - check_keysigningrequest(out, overlapping_zsks, now, until) + ksr_fname = f"{zone}.ksr.{n}" + ksr(zone, policy, "request", options=f"-K ns1 -i {now} -e +2y", to_file=ksr_fname) + check_keysigningrequest(ksr_fname, overlapping_zsks, now, until) # check that 'dnssec-ksr request' errors if there are not enough keys - _, err = ksr( + cmd = ksr( zone, policy, "request", options=f"-K ns1 -i {now} -e +3y", raise_on_exception=False, ) - error = f"no {zone}/ECDSAP256SHA256 zsk key pair found for bundle" - assert f"dnssec-ksr: fatal: {error}" in err + errmsg = ( + f"dnssec-ksr: fatal: no {zone}/ECDSAP256SHA256 zsk key pair found for bundle" + ) + assert errmsg in cmd.err # check that 'dnssec-ksr sign' creates correct skr - out, _ = ksr( - zone, policy, "sign", options=f"-K ns1/offline -f {fname} -i {now} -e +2y" - ) - - fname = f"{zone}.skr.{n}" - with open(fname, "w", encoding="utf-8") as file: - file.write(out) - refresh = -432000 # 5 days + skr_fname = f"{zone}.skr.{n}" + ksr( + zone, + policy, + "sign", + options=f"-K ns1/offline -f {ksr_fname} -i {now} -e +2y", + to_file=skr_fname, + ) check_signedkeyresponse( - out, + skr_fname, zone, ksks, overlapping_zsks, @@ -741,8 +745,8 @@ def test_ksr_common(ns1): ) # import skr - shutil.copyfile(fname, f"ns1/{fname}") - ns1.rndc(f"skr -import {fname} {zone}", log=False) + shutil.copyfile(skr_fname, f"ns1/{skr_fname}") + ns1.rndc(f"skr -import {skr_fname} {zone}", log=False) # test zone is correctly signed # - check rndc dnssec -status output @@ -767,16 +771,16 @@ def test_ksr_lastbundle(ns1): # create ksk kskdir = "ns1/offline" offset = -timedelta(days=365) - out, _ = ksr(zone, policy, "keygen", options=f"-K {kskdir} -i -1y -e +1d -o") - ksks = isctest.kasp.keystr_to_keylist(out, kskdir) + cmd = ksr(zone, policy, "keygen", options=f"-K {kskdir} -i -1y -e +1d -o") + ksks = isctest.kasp.keystr_to_keylist(cmd.out, kskdir) assert len(ksks) == 1 check_keys(ksks, None, offset=offset) # check that 'dnssec-ksr keygen' pregenerates right amount of keys zskdir = "ns1" - out, _ = ksr(zone, policy, "keygen", options=f"-K {zskdir} -i -1y -e +1d") - zsks = isctest.kasp.keystr_to_keylist(out, zskdir) + cmd = ksr(zone, policy, "keygen", options=f"-K {zskdir} -i -1y -e +1d") + zsks = isctest.kasp.keystr_to_keylist(cmd.out, zskdir) assert len(zsks) == 2 lifetime = timedelta(days=31 * 6) @@ -785,25 +789,27 @@ def test_ksr_lastbundle(ns1): # check that 'dnssec-ksr request' creates correct ksr then = zsks[0].get_timing("Created") + offset until = then + timedelta(days=366) - out, _ = ksr(zone, policy, "request", options=f"-K {zskdir} -i {then} -e +1d") - - fname = f"{zone}.ksr.{n}" - with open(fname, "w", encoding="utf-8") as file: - file.write(out) - - check_keysigningrequest(out, zsks, then, until) + ksr_fname = f"{zone}.ksr.{n}" + ksr( + zone, + policy, + "request", + options=f"-K {zskdir} -i {then} -e +1d", + to_file=ksr_fname, + ) + check_keysigningrequest(ksr_fname, zsks, then, until) # check that 'dnssec-ksr sign' creates correct skr - out, _ = ksr( - zone, policy, "sign", options=f"-K {kskdir} -f {fname} -i {then} -e +1d" - ) - - fname = f"{zone}.skr.{n}" - with open(fname, "w", encoding="utf-8") as file: - file.write(out) - refresh = -432000 # 5 days - check_signedkeyresponse(out, zone, ksks, zsks, then, until, refresh) + skr_fname = f"{zone}.skr.{n}" + ksr( + zone, + policy, + "sign", + options=f"-K {kskdir} -f {ksr_fname} -i {then} -e +1d", + to_file=skr_fname, + ) + check_signedkeyresponse(skr_fname, zone, ksks, zsks, then, until, refresh) # add zone ns1.rndc( @@ -815,8 +821,8 @@ def test_ksr_lastbundle(ns1): ) # import skr - shutil.copyfile(fname, f"ns1/{fname}") - ns1.rndc(f"skr -import {fname} {zone}", log=False) + shutil.copyfile(skr_fname, f"ns1/{skr_fname}") + ns1.rndc(f"skr -import {skr_fname} {zone}", log=False) # test zone is correctly signed # - check rndc dnssec -status output @@ -843,16 +849,16 @@ def test_ksr_inthemiddle(ns1): # create ksk kskdir = "ns1/offline" offset = -timedelta(days=365) - out, _ = ksr(zone, policy, "keygen", options=f"-K {kskdir} -i -1y -e +1y -o") - ksks = isctest.kasp.keystr_to_keylist(out, kskdir) + cmd = ksr(zone, policy, "keygen", options=f"-K {kskdir} -i -1y -e +1y -o") + ksks = isctest.kasp.keystr_to_keylist(cmd.out, kskdir) assert len(ksks) == 1 check_keys(ksks, None, offset=offset) # check that 'dnssec-ksr keygen' pregenerates right amount of keys zskdir = "ns1" - out, _ = ksr(zone, policy, "keygen", options=f"-K {zskdir} -i -1y -e +1y") - zsks = isctest.kasp.keystr_to_keylist(out, zskdir) + cmd = ksr(zone, policy, "keygen", options=f"-K {zskdir} -i -1y -e +1y") + zsks = isctest.kasp.keystr_to_keylist(cmd.out, zskdir) assert len(zsks) == 4 lifetime = timedelta(days=31 * 6) @@ -862,25 +868,27 @@ def test_ksr_inthemiddle(ns1): then = zsks[0].get_timing("Created") then = then + offset until = then + timedelta(days=365 * 2) - out, _ = ksr(zone, policy, "request", options=f"-K {zskdir} -i {then} -e +1y") - - fname = f"{zone}.ksr.{n}" - with open(fname, "w", encoding="utf-8") as file: - file.write(out) - - check_keysigningrequest(out, zsks, then, until) + ksr_fname = f"{zone}.ksr.{n}" + ksr( + zone, + policy, + "request", + options=f"-K {zskdir} -i {then} -e +1y", + to_file=ksr_fname, + ) + check_keysigningrequest(ksr_fname, zsks, then, until) # check that 'dnssec-ksr sign' creates correct skr - out, _ = ksr( - zone, policy, "sign", options=f"-K {kskdir} -f {fname} -i {then} -e +1y" - ) - - fname = f"{zone}.skr.{n}" - with open(fname, "w", encoding="utf-8") as file: - file.write(out) - refresh = -432000 # 5 days - check_signedkeyresponse(out, zone, ksks, zsks, then, until, refresh) + skr_fname = f"{zone}.skr.{n}" + ksr( + zone, + policy, + "sign", + options=f"-K {kskdir} -f {ksr_fname} -i {then} -e +1y", + to_file=skr_fname, + ) + check_signedkeyresponse(skr_fname, zone, ksks, zsks, then, until, refresh) # add zone ns1.rndc( @@ -892,8 +900,8 @@ def test_ksr_inthemiddle(ns1): ) # import skr - shutil.copyfile(fname, f"ns1/{fname}") - ns1.rndc(f"skr -import {fname} {zone}", log=False) + shutil.copyfile(skr_fname, f"ns1/{skr_fname}") + ns1.rndc(f"skr -import {skr_fname} {zone}", log=False) # test zone is correctly signed # - check rndc dnssec -status output @@ -920,34 +928,38 @@ def check_ksr_rekey_logs_error(server, zone, policy, offset, end): now = KeyTimingMetadata.now() then = now + offset until = now + end - out, _ = ksr(zone, policy, "keygen", options=f"-K {kskdir} -i {then} -e {until} -o") - ksks = isctest.kasp.keystr_to_keylist(out, kskdir) + cmd = ksr(zone, policy, "keygen", options=f"-K {kskdir} -i {then} -e {until} -o") + ksks = isctest.kasp.keystr_to_keylist(cmd.out, kskdir) assert len(ksks) == 1 # key generation zskdir = "ns1" - out, _ = ksr(zone, policy, "keygen", options=f"-K {zskdir} -i {then} -e {until}") - zsks = isctest.kasp.keystr_to_keylist(out, zskdir) + cmd = ksr(zone, policy, "keygen", options=f"-K {zskdir} -i {then} -e {until}") + zsks = isctest.kasp.keystr_to_keylist(cmd.out, zskdir) assert len(zsks) == 2 # create request now = zsks[0].get_timing("Created") then = now + offset until = now + end - out, _ = ksr(zone, policy, "request", options=f"-K {zskdir} -i {then} -e {until}") - - fname = f"{zone}.ksr.{n}" - with open(fname, "w", encoding="utf-8") as file: - file.write(out) - - # sign request - out, _ = ksr( - zone, policy, "sign", options=f"-K {kskdir} -f {fname} -i {then} -e {until}" + ksr_fname = f"{zone}.ksr.{n}" + ksr( + zone, + policy, + "request", + options=f"-K {zskdir} -i {then} -e {until}", + to_file=ksr_fname, ) - fname = f"{zone}.skr.{n}" - with open(fname, "w", encoding="utf-8") as file: - file.write(out) + # sign request + skr_fname = f"{zone}.skr.{n}" + ksr( + zone, + policy, + "sign", + options=f"-K {kskdir} -f {ksr_fname} -i {then} -e {until}", + to_file=skr_fname, + ) # add zone server.rndc( @@ -959,8 +971,8 @@ def check_ksr_rekey_logs_error(server, zone, policy, offset, end): ) # import skr - shutil.copyfile(fname, f"ns1/{fname}") - server.rndc(f"skr -import {fname} {zone}", log=False) + shutil.copyfile(skr_fname, f"ns1/{skr_fname}") + server.rndc(f"skr -import {skr_fname} {zone}", log=False) # test that rekey logs error time_remaining = 10 @@ -989,16 +1001,16 @@ def test_ksr_unlimited(ns1): # create ksk kskdir = "ns1/offline" - out, _ = ksr(zone, policy, "keygen", options=f"-K {kskdir} -i now -e +2y -o") - ksks = isctest.kasp.keystr_to_keylist(out, kskdir) + cmd = ksr(zone, policy, "keygen", options=f"-K {kskdir} -i now -e +2y -o") + ksks = isctest.kasp.keystr_to_keylist(cmd.out, kskdir) assert len(ksks) == 1 check_keys(ksks, None) # check that 'dnssec-ksr keygen' pregenerates right amount of keys zskdir = "ns1" - out, _ = ksr(zone, policy, "keygen", options=f"-K {zskdir} -i now -e +2y") - zsks = isctest.kasp.keystr_to_keylist(out, zskdir) + cmd = ksr(zone, policy, "keygen", options=f"-K {zskdir} -i now -e +2y") + zsks = isctest.kasp.keystr_to_keylist(cmd.out, zskdir) assert len(zsks) == 1 lifetime = None @@ -1007,26 +1019,28 @@ def test_ksr_unlimited(ns1): # check that 'dnssec-ksr request' creates correct ksr now = zsks[0].get_timing("Created") until = now + timedelta(days=365 * 4) - out, _ = ksr(zone, policy, "request", options=f"-K {zskdir} -i {now} -e +4y") - - fname = f"{zone}.ksr.{n}" - with open(fname, "w", encoding="utf-8") as file: - file.write(out) - - check_keysigningrequest(out, zsks, now, until) + ksr_fname = f"{zone}.ksr.{n}" + ksr( + zone, + policy, + "request", + options=f"-K {zskdir} -i {now} -e +4y", + to_file=ksr_fname, + ) + check_keysigningrequest(ksr_fname, zsks, now, until) # check that 'dnssec-ksr sign' creates correct skr without cdnskey - out, _ = ksr( - zone, "no-cdnskey", "sign", options=f"-K {kskdir} -f {fname} -i {now} -e +4y" - ) - - skrfile = f"{zone}.no-cdnskey.skr.{n}" - with open(skrfile, "w", encoding="utf-8") as file: - file.write(out) - refresh = -432000 # 5 days + skr_fname = f"{zone}.no-cdnskey.skr.{n}" + ksr( + zone, + "no-cdnskey", + "sign", + options=f"-K {kskdir} -f {ksr_fname} -i {now} -e +4y", + to_file=skr_fname, + ) check_signedkeyresponse( - out, + skr_fname, zone, ksks, zsks, @@ -1038,17 +1052,17 @@ def test_ksr_unlimited(ns1): ) # check that 'dnssec-ksr sign' creates correct skr without cds - out, _ = ksr( - zone, "no-cds", "sign", options=f"-K {kskdir} -f {fname} -i {now} -e +4y" - ) - - skrfile = f"{zone}.no-cds.skr.{n}" - with open(skrfile, "w", encoding="utf-8") as file: - file.write(out) - refresh = -432000 # 5 days + skr_fname = f"{zone}.no-cds.skr.{n}" + ksr( + zone, + "no-cds", + "sign", + options=f"-K {kskdir} -f {ksr_fname} -i {now} -e +4y", + to_file=skr_fname, + ) check_signedkeyresponse( - out, + skr_fname, zone, ksks, zsks, @@ -1059,16 +1073,16 @@ def test_ksr_unlimited(ns1): ) # check that 'dnssec-ksr sign' creates correct skr - out, _ = ksr( - zone, policy, "sign", options=f"-K {kskdir} -f {fname} -i {now} -e +4y" - ) - - skrfile = f"{zone}.{policy}.skr.{n}" - with open(skrfile, "w", encoding="utf-8") as file: - file.write(out) - refresh = -432000 # 5 days - check_signedkeyresponse(out, zone, ksks, zsks, now, until, refresh) + skr_fname = f"{zone}.{policy}.skr.{n}" + ksr( + zone, + policy, + "sign", + options=f"-K {kskdir} -f {ksr_fname} -i {now} -e +4y", + to_file=skr_fname, + ) + check_signedkeyresponse(skr_fname, zone, ksks, zsks, now, until, refresh) # add zone ns1.rndc( @@ -1080,8 +1094,8 @@ def test_ksr_unlimited(ns1): ) # import skr - shutil.copyfile(skrfile, f"ns1/{skrfile}") - ns1.rndc(f"skr -import {skrfile} {zone}", log=False) + shutil.copyfile(skr_fname, f"ns1/{skr_fname}") + ns1.rndc(f"skr -import {skr_fname} {zone}", log=False) # test zone is correctly signed # - check rndc dnssec -status output @@ -1103,8 +1117,8 @@ def test_ksr_twotone(ns1): # create ksk kskdir = "ns1/offline" - out, _ = ksr(zone, policy, "keygen", options=f"-K {kskdir} -i now -e +1y -o") - ksks = isctest.kasp.keystr_to_keylist(out, kskdir) + cmd = ksr(zone, policy, "keygen", options=f"-K {kskdir} -i now -e +1y -o") + ksks = isctest.kasp.keystr_to_keylist(cmd.out, kskdir) assert len(ksks) == 2 ksks_defalg = [] @@ -1127,8 +1141,8 @@ def test_ksr_twotone(ns1): # check that 'dnssec-ksr keygen' pregenerates right amount of keys zskdir = "ns1" - out, _ = ksr(zone, policy, "keygen", options=f"-K {zskdir} -i now -e +1y") - zsks = isctest.kasp.keystr_to_keylist(out, zskdir) + cmd = ksr(zone, policy, "keygen", options=f"-K {zskdir} -i now -e +1y") + zsks = isctest.kasp.keystr_to_keylist(cmd.out, zskdir) # First algorithm keys have a lifetime of 3 months, so there should # be 4 created keys. Second algorithm keys have a lifetime of 5 # months, so there should be 3 created keys. While only two time @@ -1159,25 +1173,27 @@ def test_ksr_twotone(ns1): # check that 'dnssec-ksr request' creates correct ksr now = zsks[0].get_timing("Created") until = now + timedelta(days=365) - out, _ = ksr(zone, policy, "request", options=f"-K {zskdir} -i {now} -e +1y") - - fname = f"{zone}.ksr.{n}" - with open(fname, "w", encoding="utf-8") as file: - file.write(out) - - check_keysigningrequest(out, zsks, now, until) + ksr_fname = f"{zone}.ksr.{n}" + ksr( + zone, + policy, + "request", + options=f"-K {zskdir} -i {now} -e +1y", + to_file=ksr_fname, + ) + check_keysigningrequest(ksr_fname, zsks, now, until) # check that 'dnssec-ksr sign' creates correct skr - out, _ = ksr( - zone, policy, "sign", options=f"-K {kskdir} -f {fname} -i {now} -e +1y" - ) - - skrfile = f"{zone}.skr.{n}" - with open(skrfile, "w", encoding="utf-8") as file: - file.write(out) - refresh = -timedelta(days=5) - check_signedkeyresponse(out, zone, ksks, zsks, now, until, refresh) + skr_fname = f"{zone}.skr.{n}" + ksr( + zone, + policy, + "sign", + options=f"-K {kskdir} -f {ksr_fname} -i {now} -e +1y", + to_file=skr_fname, + ) + check_signedkeyresponse(skr_fname, zone, ksks, zsks, now, until, refresh) # add zone ns1.rndc( @@ -1189,8 +1205,8 @@ def test_ksr_twotone(ns1): ) # import skr - shutil.copyfile(skrfile, f"ns1/{skrfile}") - ns1.rndc(f"skr -import {skrfile} {zone}", log=False) + shutil.copyfile(skr_fname, f"ns1/{skr_fname}") + ns1.rndc(f"skr -import {skr_fname} {zone}", log=False) # test zone is correctly signed # - check rndc dnssec -status output @@ -1218,8 +1234,8 @@ def test_ksr_kskroll(ns1): # create ksk kskdir = "ns1/offline" - out, _ = ksr(zone, policy, "keygen", options=f"-K {kskdir} -i now -e +1y -o") - ksks = isctest.kasp.keystr_to_keylist(out, kskdir) + cmd = ksr(zone, policy, "keygen", options=f"-K {kskdir} -i now -e +1y -o") + ksks = isctest.kasp.keystr_to_keylist(cmd.out, kskdir) assert len(ksks) == 2 lifetime = timedelta(days=31 * 6) @@ -1227,8 +1243,8 @@ def test_ksr_kskroll(ns1): # check that 'dnssec-ksr keygen' pregenerates right amount of keys zskdir = "ns1" - out, _ = ksr(zone, policy, "keygen", options=f"-K {zskdir} -i now -e +1y") - zsks = isctest.kasp.keystr_to_keylist(out, zskdir) + cmd = ksr(zone, policy, "keygen", options=f"-K {zskdir} -i now -e +1y") + zsks = isctest.kasp.keystr_to_keylist(cmd.out, zskdir) assert len(zsks) == 1 check_keys(zsks, None) @@ -1236,25 +1252,27 @@ def test_ksr_kskroll(ns1): # check that 'dnssec-ksr request' creates correct ksr now = zsks[0].get_timing("Created") until = now + timedelta(days=365) - out, _ = ksr(zone, policy, "request", options=f"-K {zskdir} -i {now} -e +1y") - - fname = f"{zone}.ksr.{n}" - with open(fname, "w", encoding="utf-8") as file: - file.write(out) - - check_keysigningrequest(out, zsks, now, until) + ksr_fname = f"{zone}.ksr.{n}" + ksr( + zone, + policy, + "request", + options=f"-K {zskdir} -i {now} -e +1y", + to_file=ksr_fname, + ) + check_keysigningrequest(ksr_fname, zsks, now, until) # check that 'dnssec-ksr sign' creates correct skr - out, _ = ksr( - zone, policy, "sign", options=f"-K {kskdir} -f {fname} -i {now} -e +1y" - ) - - skrfile = f"{zone}.skr.{n}" - with open(skrfile, "w", encoding="utf-8") as file: - file.write(out) - refresh = -432000 # 5 days - check_signedkeyresponse(out, zone, ksks, zsks, now, until, refresh) + skr_fname = f"{zone}.skr.{n}" + ksr( + zone, + policy, + "sign", + options=f"-K {kskdir} -f {ksr_fname} -i {now} -e +1y", + to_file=skr_fname, + ) + check_signedkeyresponse(skr_fname, zone, ksks, zsks, now, until, refresh) # add zone ns1.rndc( @@ -1266,8 +1284,8 @@ def test_ksr_kskroll(ns1): ) # import skr - shutil.copyfile(skrfile, f"ns1/{skrfile}") - ns1.rndc(f"skr -import {skrfile} {zone}", log=False) + shutil.copyfile(skr_fname, f"ns1/{skr_fname}") + ns1.rndc(f"skr -import {skr_fname} {zone}", log=False) # test zone is correctly signed # - check rndc dnssec -status output From a8bf53411d14b8b009cb3a6b3c5bc12a5fd7a2ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicki=20K=C5=99=C3=AD=C5=BEek?= Date: Tue, 21 Oct 2025 18:58:26 +0200 Subject: [PATCH 08/10] Add pylint check for re.compile() alias Ensure that Re() is used consistently across our code base. --- .gitlab-ci.yml | 4 +-- bin/tests/system/re_compile_checker.py | 46 ++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) create mode 100644 bin/tests/system/re_compile_checker.py diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 9a0c47b7b1..89463e5f6f 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -671,7 +671,7 @@ vulture: <<: *python_triggering_rules needs: [] script: - - vulture --exclude "*ans.py,conftest.py,isctest" --ignore-names "after_servers_start,bootstrap,pytestmark" bin/tests/system/ + - vulture --exclude "*ans.py,conftest.py,re_compile_checker.py,isctest" --ignore-names "after_servers_start,bootstrap,pytestmark" bin/tests/system/ ci-variables: <<: *precheck_job @@ -780,7 +780,7 @@ pylint: script: - pylint --rcfile $CI_PROJECT_DIR/.pylintrc $(git ls-files '*.py' | grep -vE '(ans\.py|dangerfile\.py|^bin/tests/system/|^contrib/)') # Ignore Pylint wrong-import-position error in system test to enable use of pytest.importorskip - - pylint --rcfile $CI_PROJECT_DIR/.pylintrc --disable=wrong-import-position $(git ls-files 'bin/tests/system/*.py' | grep -vE '(ans\.py|vulture_ignore_list\.py)') + - pylint --rcfile $CI_PROJECT_DIR/.pylintrc --load-plugins re_compile_checker --disable=wrong-import-position $(git ls-files 'bin/tests/system/*.py' | grep -vE '(ans\.py|vulture_ignore_list\.py)') reuse: <<: *precheck_job diff --git a/bin/tests/system/re_compile_checker.py b/bin/tests/system/re_compile_checker.py new file mode 100644 index 0000000000..efa0e9a038 --- /dev/null +++ b/bin/tests/system/re_compile_checker.py @@ -0,0 +1,46 @@ +# Copyright (C) Internet Systems Consortium, Inc. ("ISC") +# +# SPDX-License-Identifier: MPL-2.0 +# +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, you can obtain one at https://mozilla.org/MPL/2.0/. +# +# See the COPYRIGHT file distributed with this work for additional +# information regarding copyright ownership. + +# pylint: disable=unknown-option-value,re-compile-alias + +import re + +from astroid import nodes + +from pylint.checkers import BaseRawFileChecker +from pylint.lint import PyLinter + + +class ReCompileChecker(BaseRawFileChecker): + + name = "custom_raw" + msgs = { + "R9901": ( + "Replace re.compile() with Re() using `from re import compile as Re`", + "re-compile-alias", + ( + "Use a Re() alias instead of re.compile() by importing the " + "re.compile() function as Re()" + ), + ), + } + options = () + + def process_module(self, node: nodes.Module) -> None: + pattern = re.compile(r"re\.compile\(") + with node.stream() as stream: + for lineno, line in enumerate(stream): + if pattern.search(line.decode("utf-8")): + self.add_message("re-compile-alias", line=lineno) + + +def register(linter: PyLinter) -> None: + linter.register_checker(ReCompileChecker(linter)) From ff613a72d73ba6b778d24c4e0bd3190d6c8930c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicki=20K=C5=99=C3=AD=C5=BEek?= Date: Thu, 23 Oct 2025 12:36:56 +0200 Subject: [PATCH 09/10] Add generic isctest.run.EnvCmd helper to pytest A generic helper that calls the environment-specified binaries in a developer-friendly manner, i.e. passing arguments as strings rather than having to split them first. The isctest.run.cmd() remains as the basis which provides a clean and robust interface, while the isctest.run.EnvCmd() can be used as a convenient wrapper for tests, or when there are some shared default parameters. The isctest.run.Dig() is superseded with the isctest.run.EnvCmd(). In the future, we might revisit adding Dig() or command-specific helpers again, but it probably only makes sense if they offer command-aware attributes / methods, rather than just being shortcuts to isctest.run.EnvCmd(). --- bin/tests/system/isctest/run.py | 24 ++++++++++--------- bin/tests/system/keepalive/tests_keepalive.py | 2 +- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/bin/tests/system/isctest/run.py b/bin/tests/system/isctest/run.py index 530d413751..9b6ef31236 100644 --- a/bin/tests/system/isctest/run.py +++ b/bin/tests/system/isctest/run.py @@ -81,6 +81,19 @@ def cmd( return CmdResult(exc) +class EnvCmd: + """Helper for executing binaries from env with optional base parameters.""" + + def __init__(self, name: str, base_params: str = ""): + self.bin_path = os.environ[name] + self.base_params = base_params.split() + + def __call__(self, params: str, **kwargs) -> CmdResult: + """Call the command. Keyword arguments from isctest.run.cmd() are supported.""" + args = self.base_params + params.split() + return cmd([self.bin_path] + args, **kwargs) + + def _run_script( interpreter: str, script: str, @@ -116,17 +129,6 @@ def _run_script( isctest.log.debug(" exited with %d", returncode) -class Dig: - def __init__(self, base_params: str = ""): - self.base_params = base_params - - def __call__(self, params: str) -> CmdResult: - """Run the dig command with the given parameters.""" - return cmd( - [os.environ.get("DIG")] + f"{self.base_params} {params}".split(), - ) - - def shell(script: str, args: Optional[List[str]] = None) -> None: """Run a given script with system's shell interpreter.""" _run_script(os.environ["SHELL"], script, args) diff --git a/bin/tests/system/keepalive/tests_keepalive.py b/bin/tests/system/keepalive/tests_keepalive.py index 5314d7ee57..1025ac5384 100644 --- a/bin/tests/system/keepalive/tests_keepalive.py +++ b/bin/tests/system/keepalive/tests_keepalive.py @@ -28,7 +28,7 @@ def test_dig_tcp_keepalive_handling(named_port, ns2): options_received = line.split()[0] return int(options_received) - dig = isctest.run.Dig(f"-p {str(named_port)}") + dig = isctest.run.EnvCmd("DIG", f"-p {str(named_port)}") isctest.log.info("check that dig handles TCP keepalive in query") assert "; TCP-KEEPALIVE" in dig("+qr +keepalive foo.example. @10.53.0.2").out From f33e2b6d877146576d9384f21dbe1ff075df6205 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicki=20K=C5=99=C3=AD=C5=BEek?= Date: Thu, 23 Oct 2025 15:08:35 +0200 Subject: [PATCH 10/10] Refactor NamedInstance.rndc() to use EnvCmd() interface To unify the command handling, utilize EnvCmd() to handle rndc commands: 1. Remove isctest.rndc abstractions. They were intended for an upcoming python-only implementation. A couple of years later, it doesn't seem to be coming any time soon, so let's stick with the interface that makes sense today, i.e. use the same command handling interface everywhere. 2. Remove the specialized rndc.log in favor of the generic logging already implemented by isctest.run.cmd(). I believe the cause of the many rndc(log=False) invocations was that nobody wanted this extra file. Yet, logging everything by default makes sense for debugging, unless there's a good reason not to. In almost all cases, logging was switched to the default (enabled). 3. With the NamedInstance.rndc() call now returning CmdResult rather than combined stdout+stderr string, adjust all the invocations to use `.out` or `.err` as necessary. 4. Replace some manual rndc invocation and its base argument construction with the standardized nsX.rndc() call. 5. In cases where rndc is expected to fail, utilize raise_on_exception=False and check the `.rc` from the result, rather than handling an exception. 6. In addzone/tests_rndc_deadlock.py, refactor the test slightly to avoid using EnvCmd() entirely to avoid spamming the logs. This test calls rndc in a loop from multiple threads and such test case is an exception which doesn't warrant changing the `isctest.run.cmd()` implementation. --- .../system/addzone/tests_rndc_deadlock.py | 15 +- .../system/addzone/tests_showzone_static.py | 8 +- bin/tests/system/checkds/tests_checkds.py | 29 +--- .../configloading/tests_configloading.py | 14 +- bin/tests/system/dnssec/tests_policy.py | 4 +- bin/tests/system/dnssec/tests_signing.py | 21 ++- bin/tests/system/dnssec/tests_validation.py | 33 ++-- .../dnssec/tests_validation_accept_expired.py | 6 +- .../dnssec/tests_validation_managed_keys.py | 9 +- .../dnssec/tests_validation_multiview.py | 7 +- bin/tests/system/isctest/__init__.py | 1 - bin/tests/system/isctest/instance.py | 94 +++--------- bin/tests/system/isctest/kasp.py | 10 +- bin/tests/system/isctest/rndc.py | 69 --------- bin/tests/system/kasp/tests_kasp.py | 55 +++---- bin/tests/system/keepalive/tests_keepalive.py | 14 +- bin/tests/system/ksr/tests_ksr.py | 23 +-- .../system/multisigner/tests_multisigner.py | 18 +-- bin/tests/system/nsec3/tests_nsec3_change.py | 2 +- bin/tests/system/nta/tests_nta.py | 143 +++++++++--------- bin/tests/system/nzd2nzf/tests_nzd2nzf.py | 2 +- bin/tests/system/resolver/tests_resolver.py | 32 ++-- .../tests_rollover_zsk_prepublication.py | 2 +- .../system/rollover/tests_rollover_manual.py | 2 +- bin/tests/system/showconf/tests_showconf.py | 44 +++--- bin/tests/system/shutdown/tests_shutdown.py | 7 +- .../system/stress/tests_stress_update.py | 17 +-- .../system/synthrecord/tests_synthrecord.py | 7 +- .../system/views/tests_views_addzones.py | 2 +- 29 files changed, 246 insertions(+), 444 deletions(-) delete mode 100644 bin/tests/system/isctest/rndc.py diff --git a/bin/tests/system/addzone/tests_rndc_deadlock.py b/bin/tests/system/addzone/tests_rndc_deadlock.py index fd2c9d5897..f805897f29 100755 --- a/bin/tests/system/addzone/tests_rndc_deadlock.py +++ b/bin/tests/system/addzone/tests_rndc_deadlock.py @@ -10,12 +10,12 @@ # information regarding copyright ownership. import concurrent.futures +import os +import subprocess import time import pytest -import isctest - pytestmark = pytest.mark.extra_artifacts( [ "ns*/*.nzf*", @@ -43,20 +43,19 @@ def rndc_loop(test_state, domain, ns3): ["delzone", domain], ] + args = [os.environ["RNDC"]] + ns3.rndc_args.split() while not test_state["finished"]: for command in rndc_commands: - ns3.rndc(" ".join(command), ignore_errors=True, log=False) + # avoid using ns3.rndc() directly to avoid log spam + subprocess.run(args + " ".join(command), timeout=10, check=False) def check_if_server_is_responsive(ns3): """ Check if server status can be successfully retrieved using "rndc status" """ - try: - ns3.rndc("status", log=False) - return True - except isctest.rndc.RNDCException: - return False + cmd = ns3.rndc("status", raise_on_exception=False) + return cmd.rc == 0 def test_rndc_deadlock(ns3): diff --git a/bin/tests/system/addzone/tests_showzone_static.py b/bin/tests/system/addzone/tests_showzone_static.py index 6edaa497f6..fcf278346b 100644 --- a/bin/tests/system/addzone/tests_showzone_static.py +++ b/bin/tests/system/addzone/tests_showzone_static.py @@ -23,9 +23,9 @@ import pytest ) def test_showzone_static(ns1, templates, allow): templates.render("ns1/named.conf", {"allownewzones": allow}) - ns1.rndc("reload", log=False) - zoneconfig = ns1.rndc("showzone inlinesec.example", log=False) + ns1.rndc("reload") + response = ns1.rndc("showzone inlinesec.example") assert ( - zoneconfig - == 'zone "inlinesec.example" { type primary; file "inlinesec.db"; };\n' + 'zone "inlinesec.example" { type primary; file "inlinesec.db"; };' + in response.out ) diff --git a/bin/tests/system/checkds/tests_checkds.py b/bin/tests/system/checkds/tests_checkds.py index b6c36e3908..8be24a830b 100755 --- a/bin/tests/system/checkds/tests_checkds.py +++ b/bin/tests/system/checkds/tests_checkds.py @@ -189,33 +189,6 @@ def keystate_check(server, zone, key): assert val != 0 -def rekey(zone): - rndc = os.getenv("RNDC") - assert rndc is not None - - port = os.getenv("CONTROLPORT") - assert port is not None - - # rndc loadkeys. - rndc_cmd = [ - rndc, - "-c", - "../_common/rndc.conf", - "-p", - port, - "-s", - "10.53.0.9", - "loadkeys", - zone, - ] - controller = isctest.run.cmd(rndc_cmd) - - if controller.rc != 0: - isctest.log.error(f"rndc loadkeys {zone} failed") - - assert controller.rc == 0 - - class CheckDSTest(NamedTuple): zone: str logs_to_wait_for: Tuple[str] @@ -472,7 +445,7 @@ def test_checkds(ns2, ns9, params): for log_string in params.logs_to_wait_for: line = f"zone {params.zone}/IN (signed): checkds: {log_string}" while line not in ns9.log: - rekey(params.zone) + ns9.rndc(f"loadkeys {params.zone}") time_remaining -= 1 assert time_remaining, f'Timed out waiting for "{log_string}" to be logged' time.sleep(1) diff --git a/bin/tests/system/configloading/tests_configloading.py b/bin/tests/system/configloading/tests_configloading.py index ec017a8bfe..f5b512861b 100644 --- a/bin/tests/system/configloading/tests_configloading.py +++ b/bin/tests/system/configloading/tests_configloading.py @@ -11,8 +11,6 @@ from re import compile as Re -import isctest - def test_configloading_log(ns1): """ @@ -38,11 +36,11 @@ def test_configloading_log(ns1): watcher.wait_for_sequence(log_sequence) with ns1.watch_log_from_here() as watcher: - ns1.rndc("reconfig", log=False) + ns1.rndc("reconfig") watcher.wait_for_sequence(log_sequence) with ns1.watch_log_from_here() as watcher: - ns1.rndc("reload", log=False) + ns1.rndc("reload") watcher.wait_for_sequence(log_sequence) @@ -66,8 +64,6 @@ def test_reload_fails_log(ns1, templates): with ns1.watch_log_from_here() as watcher: templates.render("ns1/named.conf", {"wrongoption": True}) - try: - ns1.rndc("reload", log=False) - assert False - except isctest.rndc.RNDCException: - watcher.wait_for_sequence(log_sequence) + cmd = ns1.rndc("reload", raise_on_exception=False) + assert cmd.rc != 0 + watcher.wait_for_sequence(log_sequence) diff --git a/bin/tests/system/dnssec/tests_policy.py b/bin/tests/system/dnssec/tests_policy.py index e813bea448..51801d5d86 100644 --- a/bin/tests/system/dnssec/tests_policy.py +++ b/bin/tests/system/dnssec/tests_policy.py @@ -62,7 +62,7 @@ def test_signatures_validity(ns3, templates): templates.render("ns3/named.conf", {"long_sigs": True}) with ns3.watch_log_from_here() as watcher: - ns3.reconfigure(log=False) + ns3.reconfigure() watcher.wait_for_line( "zone_needdump: zone siginterval.example/IN (signed): enter" ) @@ -72,7 +72,7 @@ def test_signatures_validity(ns3, templates): assert after != before - ns3.rndc("sign siginterval.example", log=False) + ns3.rndc("sign siginterval.example") msg = isctest.query.create("siginterval.example.", "SOA") res = isctest.query.tcp(msg, "10.53.0.3") diff --git a/bin/tests/system/dnssec/tests_signing.py b/bin/tests/system/dnssec/tests_signing.py index 682c3d7cfb..657e50b76b 100644 --- a/bin/tests/system/dnssec/tests_signing.py +++ b/bin/tests/system/dnssec/tests_signing.py @@ -381,14 +381,13 @@ def test_cdnskey_signing(): ) def test_rndc_signing_except(cmd, ns3): # check that 'rndc signing' errors are handled - with pytest.raises(isctest.rndc.RNDCException): - ns3.rndc(cmd, log=False) - ns3.rndc("status", log=False) + ret = ns3.rndc(cmd, raise_on_exception=False) + assert ret.rc != 0 def test_rndc_signing_output(ns3): - response = ns3.rndc("signing -list dynamic.example", log=False) - assert "No signing records found" in response + response = ns3.rndc("signing -list dynamic.example") + assert "No signing records found" in response.out def test_zonestatus_signing(ns3): @@ -398,14 +397,14 @@ def test_zonestatus_signing(ns3): # for the name and type, and check that the resigning time is # after the inception and before the expiration. - response = ns3.rndc("zonestatus secure.example", log=False) + response = ns3.rndc("zonestatus secure.example") # next resign node: secure.example/DNSKEY - nrn = [r for r in response.splitlines() if "next resign node" in r][0] + nrn = [r for r in response.out.splitlines() if "next resign node" in r][0] rdname, rdtype = nrn.split()[3].split("/") # next resign time: Thu, 24 Apr 2014 10:38:16 GMT - nrt = [r for r in response.splitlines() if "next resign time" in r][0] + nrt = [r for r in response.out.splitlines() if "next resign time" in r][0] rtime = " ".join(nrt.split()[3:]) rt = time.strptime(rtime, "%a, %d %b %Y %H:%M:%S %Z") when = int(time.strftime("%s", rt)) @@ -469,7 +468,7 @@ def test_offline_ksk_signing(ns2): def loadkeys(): pattern = Re(f"{zone}/IN.*next key event") with ns2.watch_log_from_here() as watcher: - ns2.rndc(f"loadkeys {zone}", log=False) + ns2.rndc(f"loadkeys {zone}") watcher.wait_for_line(pattern) ksk_only_types = ["DNSKEY", "CDNSKEY", "CDS"] @@ -506,7 +505,7 @@ def test_offline_ksk_signing(ns2): ZSKID2 = getkeyid(ZSK2) isctest.log.info("prepublish new ZSK") - ns2.rndc(f"dnssec -rollover -key {ZSKID} {zone}", log=False) + ns2.rndc(f"dnssec -rollover -key {ZSKID} {zone}") isctest.run.retry_with_timeout(check_zskcount, 5) isctest.log.info("make the new ZSK active") @@ -561,7 +560,7 @@ def test_offline_ksk_signing(ns2): settime("-sKns2", "-k", "HIDDEN", "now", "-z", "HIDDEN", "now", "-Dnow", ZSK) settime("-sKns2", "-k", "OMNIPRESENT", "now", "-z", "OMNIPRESENT", "now", ZSK2) loadkeys() - ns2.rndc(f"dnssec -rollover -key {ZSKID2} {zone}", log=False) + ns2.rndc(f"dnssec -rollover -key {ZSKID2} {zone}") with ns2.watch_log_from_start() as watcher: watcher.wait_for_line(f"{ZSKID3} (ZSK) is now published") diff --git a/bin/tests/system/dnssec/tests_validation.py b/bin/tests/system/dnssec/tests_validation.py index 7c3cc24d38..9446f73da1 100644 --- a/bin/tests/system/dnssec/tests_validation.py +++ b/bin/tests/system/dnssec/tests_validation.py @@ -134,10 +134,9 @@ def test_secure_root(ns4): # check that "rndc secroots" dumps the trusted keys key = int(getfrom("ns1/managed.key.id")) alg = os.environ["DEFAULT_ALGORITHM"] - expected = f"./{alg}/{key} ; static" - response = ns4.rndc("secroots -", log=False).splitlines() - assert expected in response - assert len(response) == 10 + response = ns4.rndc("secroots -") + assert f"./{alg}/{key} ; static" in response.out + assert len(response.out.splitlines()) == 10 def test_positive_validation_nsec(): @@ -709,7 +708,7 @@ def test_negative_validation_optout(): def test_cache(ns4): # check that key id's are logged when dumping the cache - ns4.rndc("dumpdb -cache", log=False) + ns4.rndc("dumpdb -cache") dumpdb = isctest.text.TextFile("ns4/named_dump.db") assert "; key id = " in dumpdb @@ -811,7 +810,7 @@ def test_insecure_proof_nsec(ns4): isctest.check.noadflag(res2) # insecurity proof using negative cache - ns4.rndc("flush", log=False) + ns4.rndc("flush") msg = isctest.query.create("insecure.example", "DS", cd=True) isctest.query.tcp(msg, "10.53.0.4") @@ -941,7 +940,7 @@ def test_validation_recovery(ns2, ns4): msg = isctest.query.create("target.peer-ns-spoof", "A", cd=True) res = isctest.query.tcp(msg, "10.53.0.4") isctest.check.servfail(res) - ns4.rndc("dumpdb", log=False) + ns4.rndc("dumpdb") dumpdb = isctest.text.TextFile("ns4/named_dump.db") assert "10.53.0.100" in dumpdb @@ -950,7 +949,7 @@ def test_validation_recovery(ns2, ns4): "ns2/peer.peer-ns-spoof.db.next", "ns2/peer.peer-ns-spoof.db.signed" ) with ns2.watch_log_from_here() as watcher: - ns2.rndc("reload peer.peer-ns-spoof", log=False) + ns2.rndc("reload peer.peer-ns-spoof") watcher.wait_for_line("zone peer.peer-ns-spoof/IN: loaded serial 2000042408") # and check we can resolve with the correct server address @@ -971,7 +970,7 @@ def test_validation_recovery(ns2, ns4): "ns2/dnskey-rrsigs-stripped.db.next", "ns2/dnskey-rrsigs-stripped.db.signed" ) with ns2.watch_log_from_here() as watcher: - ns2.rndc("reload dnskey-rrsigs-stripped", log=False) + ns2.rndc("reload dnskey-rrsigs-stripped") watcher.wait_for_line( "zone dnskey-rrsigs-stripped/IN: loaded serial 2000042408" ) @@ -995,7 +994,7 @@ def test_validation_recovery(ns2, ns4): "ns2/ds-rrsigs-stripped.db.next", "ns2/ds-rrsigs-stripped.db.signed" ) with ns2.watch_log_from_here() as watcher: - ns2.rndc("reload ds-rrsigs-stripped", log=False) + ns2.rndc("reload ds-rrsigs-stripped") watcher.wait_for_line("zone ds-rrsigs-stripped/IN: loaded serial 2000042408") # and check we can now resolve with the correct server address @@ -1006,7 +1005,7 @@ def test_validation_recovery(ns2, ns4): isctest.check.adflag(res2) # check recovery with mismatching NS - ns4.rndc("flush", log=False) + ns4.rndc("flush") msg = isctest.query.create("inconsistent", "NS", dnssec=False, cd=True) res = isctest.query.tcp(msg, "10.53.0.4") isctest.check.noadflag(res) @@ -1074,7 +1073,7 @@ def test_transitions(): def test_validating_forwarder(ns4, ns9): # check validating forwarder behavior with mismatching NS - ns4.rndc("flush", log=False) + ns4.rndc("flush") msg = isctest.query.create("inconsistent", "NS", dnssec=False, cd=True) res = isctest.query.tcp(msg, "10.53.0.9") isctest.check.noerror(res) @@ -1098,7 +1097,7 @@ def test_validating_forwarder(ns4, ns9): isctest.check.adflag(res) # check validating forwarder sends CD to validate with a local trust anchor - ns4.rndc("flush", log=False) + ns4.rndc("flush") msg = isctest.query.create("localkey.example", "SOA") res = isctest.query.tcp(msg, "10.53.0.4") isctest.check.servfail(res) @@ -1144,7 +1143,7 @@ def test_expired_signatures(ns4): isctest.check.noerror(res) # test TTL is capped at RRSIG expiry time - ns4.rndc("flush", log=False) + ns4.rndc("flush") msg = isctest.query.create("expiring.example", "SOA", cd=True) res1 = isctest.query.tcp(msg, "10.53.0.4") msg = isctest.query.create("expiring.example", "SOA") @@ -1155,7 +1154,7 @@ def test_expired_signatures(ns4): assert rrset.ttl <= 60 # test TTL is capped at RRSIG expiry time in the additional section (NS) - ns4.rndc("flush", log=False) + ns4.rndc("flush") msg = isctest.query.create("expiring.example", "NS", cd=True) res1 = isctest.query.tcp(msg, "10.53.0.4") msg = isctest.query.create("expiring.example", "NS") @@ -1166,7 +1165,7 @@ def test_expired_signatures(ns4): assert rrset.ttl <= 60 # test TTL is capped at RRSIG expiry time in the additional section (MX) - ns4.rndc("flush", log=False) + ns4.rndc("flush") msg = isctest.query.create("expiring.example", "MX", cd=True) res1 = isctest.query.tcp(msg, "10.53.0.4") msg = isctest.query.create("expiring.example", "MX") @@ -1254,7 +1253,7 @@ def test_pending_ds(ns4): # a negative cache entry with trust level "pending" for the DS. prime # with a +cd DS query to produce the negative cache entry, then send a # query that uses that entry as part of the validation process. - ns4.rndc("flush", log=False) + ns4.rndc("flush") msg = isctest.query.create("insecure.example", "DS", cd=True) res = isctest.query.tcp(msg, "10.53.0.4") isctest.check.noerror(res) diff --git a/bin/tests/system/dnssec/tests_validation_accept_expired.py b/bin/tests/system/dnssec/tests_validation_accept_expired.py index 51283aedf9..d8f97f227b 100644 --- a/bin/tests/system/dnssec/tests_validation_accept_expired.py +++ b/bin/tests/system/dnssec/tests_validation_accept_expired.py @@ -27,7 +27,7 @@ def bootstrap(): def test_accept_expired(ns4): # test TTL of about-to-expire rrsets with accept-expired - ns4.rndc("flush", log=False) + ns4.rndc("flush") msg = isctest.query.create("expiring.example", "SOA") msg.flags |= flags.CD res1 = isctest.query.tcp(msg, "10.53.0.4") @@ -40,7 +40,7 @@ def test_accept_expired(ns4): # test TTL is capped at RRSIG expiry time in the additional section # with accept-expired - ns4.rndc("flush", log=False) + ns4.rndc("flush") msg = isctest.query.create("expiring.example", "MX") msg.flags |= flags.CD res1 = isctest.query.tcp(msg, "10.53.0.4") @@ -52,7 +52,7 @@ def test_accept_expired(ns4): assert rrset.ttl <= 120 # test TTL of expired rrsets with accept-expired - ns4.rndc("flush", log=False) + ns4.rndc("flush") msg = isctest.query.create("expired.example", "SOA") msg.flags |= flags.CD res1 = isctest.query.tcp(msg, "10.53.0.4") diff --git a/bin/tests/system/dnssec/tests_validation_managed_keys.py b/bin/tests/system/dnssec/tests_validation_managed_keys.py index e6079e4b5a..3cef4d67c9 100644 --- a/bin/tests/system/dnssec/tests_validation_managed_keys.py +++ b/bin/tests/system/dnssec/tests_validation_managed_keys.py @@ -44,10 +44,9 @@ def test_secure_root_managed(ns4): # check that "rndc secroots" dumps the trusted keys key = int(getfrom("ns1/managed.key.id")) alg = os.environ["DEFAULT_ALGORITHM"] - expected = f"./{alg}/{key} ; managed" - response = ns4.rndc("secroots -", log=False).splitlines() - assert expected in response - assert len(response) == 10 + response = ns4.rndc("secroots -") + assert f"./{alg}/{key} ; managed" in response.out + assert len(response.out.splitlines()) == 10 def test_positive_validation_nsec_managed(): @@ -103,6 +102,6 @@ def test_ds_managed(): def test_keydata_storage(ns4): - ns4.rndc("managed-keys sync", log=False) + ns4.rndc("managed-keys sync") with isctest.log.WatchLogFromStart("ns4/managed-keys.bind") as watcher: watcher.wait_for_line(["KEYDATA", "next refresh:"]) diff --git a/bin/tests/system/dnssec/tests_validation_multiview.py b/bin/tests/system/dnssec/tests_validation_multiview.py index 015e458349..e22f1d420b 100644 --- a/bin/tests/system/dnssec/tests_validation_multiview.py +++ b/bin/tests/system/dnssec/tests_validation_multiview.py @@ -58,7 +58,6 @@ def test_secure_roots(ns4): # check that "rndc secroots" dumps the trusted keys with multiple views key = int(getfrom("ns1/managed.key.id")) alg = os.environ["DEFAULT_ALGORITHM"] - expected = f"./{alg}/{key} ; static" - response = ns4.rndc("secroots -", log=False).splitlines() - assert expected in response, response - assert len(response) == 17 + response = ns4.rndc("secroots -") + assert f"./{alg}/{key} ; static" in response.out + assert len(response.out.splitlines()) == 17 diff --git a/bin/tests/system/isctest/__init__.py b/bin/tests/system/isctest/__init__.py index fb04a2a1e8..ce0dd75508 100644 --- a/bin/tests/system/isctest/__init__.py +++ b/bin/tests/system/isctest/__init__.py @@ -13,7 +13,6 @@ from . import check from . import instance from . import query from . import kasp -from . import rndc from . import run from . import template from . import log diff --git a/bin/tests/system/isctest/instance.py b/bin/tests/system/isctest/instance.py index 9e8ac6b5dc..83bd51ffa5 100644 --- a/bin/tests/system/isctest/instance.py +++ b/bin/tests/system/isctest/instance.py @@ -13,7 +13,6 @@ from typing import List, NamedTuple, Optional -import logging import os from pathlib import Path import re @@ -21,9 +20,8 @@ import re import dns.message import dns.rcode -from .log import debug, info, WatchLogFromStart, WatchLogFromHere -from .rndc import RNDCBinaryExecutor, RNDCException, RNDCExecutor -from .run import perl +from .log import debug, WatchLogFromStart, WatchLogFromHere +from .run import CmdResult, EnvCmd, perl from .query import udp from .text import TextFile @@ -57,8 +55,6 @@ class NamedInstance: identifier: str, num: Optional[int] = None, ports: Optional[NamedPorts] = None, - rndc_logger: Optional[logging.Logger] = None, - rndc_executor: Optional[RNDCExecutor] = None, ) -> None: """ `identifier` is the name of the instance's directory @@ -71,12 +67,6 @@ class NamedInstance: this `named` instance is listening for various types of traffic (both DNS traffic and RNDC commands). Defaults to ports set by the test framework. - - `rndc_logger` is the `logging.Logger` to use for logging RNDC - commands sent to this `named` instance. - - `rndc_executor` is an object implementing the `RNDCExecutor` interface - that is used for executing RNDC commands on this `named` instance. """ self.directory = Path(identifier).absolute() if not self.directory.is_dir(): @@ -89,8 +79,14 @@ class NamedInstance: ports = NamedPorts.from_env() self.ports = ports self.log = TextFile(os.path.join(identifier, "named.run")) - self._rndc_executor = rndc_executor or RNDCBinaryExecutor() - self._rndc_logger = rndc_logger + + self._rndc_conf = Path("../_common/rndc.conf").absolute() + self._rndc = EnvCmd("RNDC", self.rndc_args) + + @property + def rndc_args(self) -> str: + """Base arguments for calling RNDC to control the instance.""" + return f"-c {self._rndc_conf} -s {self.ip} -p {self.ports.rndc}" @property def ip(self) -> str: @@ -108,52 +104,16 @@ class NamedInstance: assert num is None or num == parsed_num, "mismatched num and identifier" return parsed_num - def rndc(self, command: str, ignore_errors: bool = False, log: bool = True) -> str: + def rndc(self, command: str, timeout=10, **kwargs) -> CmdResult: """ Send `command` to this named instance using RNDC. Return the server's response. - If the RNDC command fails, an `RNDCException` is raised unless - `ignore_errors` is set to `True`. - - The RNDC command will be logged to `rndc.log` (along with the server's - response) unless `log` is set to `False`. - - ```python - def test_foo(servers): - # Send the "status" command to ns1. An `RNDCException` will be - # raised if the RNDC command fails. This command will be logged. - response = servers["ns1"].rndc("status") - - # Send the "thaw foo" command to ns2. No exception will be raised - # in case the RNDC command fails. This command will be logged - # (even if it fails). - response = servers["ns2"].rndc("thaw foo", ignore_errors=True) - - # Send the "stop" command to ns3. An `RNDCException` will be - # raised if the RNDC command fails, but this command will not be - # logged (the server's response will still be returned to the - # caller, though). - response = servers["ns3"].rndc("stop", log=False) - - # Send the "halt" command to ns4 in "fire & forget mode": no - # exceptions will be raised and no logging will take place (the - # server's response will still be returned to the caller, though). - response = servers["ns4"].rndc("stop", ignore_errors=True, log=False) - ``` + To suppress exceptions, redirect outputs, control logging change + timeout etc. use keyword arguments which are passed to + isctest.cmd.run(). """ - try: - response = self._rndc_executor.call(self.ip, self.ports.rndc, command) - if log: - self._rndc_log(command, response) - except RNDCException as exc: - response = str(exc) - if log: - self._rndc_log(command, response) - if not ignore_errors: - raise - - return response + return self._rndc(command, timeout=timeout, **kwargs) def nsupdate( self, update_msg: dns.message.Message, expected_rcode=dns.rcode.NOERROR @@ -199,31 +159,15 @@ class NamedInstance: """ return WatchLogFromHere(self.log.path, timeout) - def reconfigure(self, **kwargs) -> None: + def reconfigure(self, **kwargs) -> CmdResult: """ Reconfigure this named `instance` and wait until reconfiguration is - finished. Raise an `RNDCException` if reconfiguration fails. + finished. """ with self.watch_log_from_here() as watcher: - self.rndc("reconfig", **kwargs) + cmd = self.rndc("reconfig", **kwargs) watcher.wait_for_line("any newly configured zones are now loaded") - - def _rndc_log(self, command: str, response: str) -> None: - """ - Log an `rndc` invocation (and its output) to the `rndc.log` file in the - current working directory. - """ - fmt = '%(ip)s: "%(command)s"\n%(separator)s\n%(response)s%(separator)s' - args = { - "ip": self.ip, - "command": command, - "separator": "-" * 80, - "response": response, - } - if self._rndc_logger is None: - info(fmt, args) - else: - self._rndc_logger.info(fmt, args) + return cmd def stop(self, args: Optional[List[str]] = None) -> None: """Stop the instance.""" diff --git a/bin/tests/system/isctest/kasp.py b/bin/tests/system/isctest/kasp.py index de3edc8e55..a3b71577d9 100644 --- a/bin/tests/system/isctest/kasp.py +++ b/bin/tests/system/isctest/kasp.py @@ -859,19 +859,19 @@ def check_dnssecstatus(server, zone, keys, policy=None, view=None, verbose=False v = "-v " if view is None: - response = server.rndc(f"dnssec -status {v}{zone}", log=False) + response = server.rndc(f"dnssec -status {v}{zone}") else: - response = server.rndc(f"dnssec -status {v}{zone} in {view}", log=False) + response = server.rndc(f"dnssec -status {v}{zone} in {view}") if policy is None: - assert "Zone does not have dnssec-policy" in response + assert "Zone does not have dnssec-policy" in response.out return - assert f"DNSSEC status for zone '{zone}' using policy '{policy}'" in response + assert f"DNSSEC status for zone '{zone}' using policy '{policy}'" in response.out for key in keys: if not key.external: - assert f"{key.role()} {key.tag}" in response + assert f"{key.role()} {key.tag}" in response.out def _check_signatures( diff --git a/bin/tests/system/isctest/rndc.py b/bin/tests/system/isctest/rndc.py deleted file mode 100644 index d4a0a1bd77..0000000000 --- a/bin/tests/system/isctest/rndc.py +++ /dev/null @@ -1,69 +0,0 @@ -# Copyright (C) Internet Systems Consortium, Inc. ("ISC") -# -# SPDX-License-Identifier: MPL-2.0 -# -# This Source Code Form is subject to the terms of the Mozilla Public -# License, v. 2.0. If a copy of the MPL was not distributed with this -# file, you can obtain one at https://mozilla.org/MPL/2.0/. -# -# See the COPYRIGHT file distributed with this work for additional -# information regarding copyright ownership. - -import abc -import os -import subprocess - - -class RNDCExecutor(abc.ABC): - """ - An interface which RNDC executors have to implement in order for the - `NamedInstance` class to be able to use them. - """ - - @abc.abstractmethod - def call(self, ip: str, port: int, command: str) -> str: - """ - Send RNDC `command` to the `named` instance at `ip:port` and return the - server's response. - """ - - -class RNDCException(Exception): - """ - Raised by classes implementing the `RNDCExecutor` interface when sending an - RNDC command fails for any reason. - """ - - -class RNDCBinaryExecutor(RNDCExecutor): - """ - An `RNDCExecutor` which sends RNDC commands to servers using the `rndc` - binary. - """ - - def __init__(self) -> None: - """ - This class needs the `RNDC` environment variable to be set to the path - to the `rndc` binary to use. - """ - rndc_path = os.environ.get("RNDC", "/bin/false") - rndc_conf = os.path.join("..", "_common", "rndc.conf") - self._base_cmdline = [rndc_path, "-c", rndc_conf] - - def call(self, ip: str, port: int, command: str) -> str: - """ - Send RNDC `command` to the `named` instance at `ip:port` and return the - server's response. - """ - cmdline = self._base_cmdline[:] - cmdline.extend(["-s", ip]) - cmdline.extend(["-p", str(port)]) - cmdline.extend(command.split()) - - try: - return subprocess.check_output( - cmdline, stderr=subprocess.STDOUT, timeout=10, encoding="utf-8" - ) - except subprocess.SubprocessError as exc: - msg = getattr(exc, "output", "RNDC exception occurred") - raise RNDCException(msg) from exc diff --git a/bin/tests/system/kasp/tests_kasp.py b/bin/tests/system/kasp/tests_kasp.py index 2e8d038e29..6b9fd9d228 100644 --- a/bin/tests/system/kasp/tests_kasp.py +++ b/bin/tests/system/kasp/tests_kasp.py @@ -11,6 +11,7 @@ import os import shutil +import subprocess import time from datetime import timedelta @@ -200,7 +201,7 @@ def cb_ixfr_is_signed(expected_updates, params, ksks=None, zsks=None): f"expected updates {expected_updates} policy {policy} ksks {ksks} zsks {zsks}" ) shutil.copyfile(f"ns2/{zone}.db.in2", f"ns2/{zone}.db") - servers["ns2"].rndc(f"reload {zone}", log=False) + servers["ns2"].rndc(f"reload {zone}") def update_is_signed(): parts = update.split() @@ -314,7 +315,7 @@ def cb_remove_keyfiles(params, ksks=None, zsks=None): os.remove(k.statefile) with servers["ns3"].watch_log_from_here() as watcher: - servers["ns3"].rndc(f"loadkeys {zone}", log=False) + servers["ns3"].rndc(f"loadkeys {zone}") watcher.wait_for_line( f"zone {zone}/IN (signed): zone_rekey:zone_verifykeys failed: some key files are missing" ) @@ -806,9 +807,9 @@ def test_kasp_inherit_view(number, dynamic, inline_signing, txt_rdata, ns4): isctest.kasp.check_dnssecstatus(ns4, zone, keys, policy=policy, view=view) isctest.kasp.check_apex(ns4, zone, keys, [], tsig=tsig) # check zonestatus - response = ns4.rndc(f"zonestatus {zone} in {view}", log=False) - assert f"dynamic: {dynamic}" in response - assert f"inline signing: {inline_signing}" in response + response = ns4.rndc(f"zonestatus {zone} in {view}") + assert f"dynamic: {dynamic}" in response.out + assert f"inline signing: {inline_signing}" in response.out # check subdomain fqdn = f"{zone}." qname = f"view.{zone}." @@ -869,7 +870,7 @@ def test_kasp_default(ns3): state_stat = os.stat(key.statefile) with ns3.watch_log_from_here() as watcher: - ns3.rndc(f"loadkeys {zone}", log=False) + ns3.rndc(f"loadkeys {zone}") watcher.wait_for_line(f"keymgr: {zone} done") assert privkey_stat.st_mtime == os.stat(key.privatefile).st_mtime @@ -878,7 +879,7 @@ def test_kasp_default(ns3): # again with ns3.watch_log_from_here() as watcher: - ns3.rndc(f"loadkeys {zone}", log=False) + ns3.rndc(f"loadkeys {zone}") watcher.wait_for_line(f"keymgr: {zone} done") assert privkey_stat.st_mtime == os.stat(key.privatefile).st_mtime @@ -888,7 +889,7 @@ def test_kasp_default(ns3): # modify unsigned zone file and check that new record is signed. isctest.log.info("check that an updated zone signs the new record") shutil.copyfile("ns3/template2.db.in", f"ns3/{zone}.db") - ns3.rndc(f"reload {zone}", log=False) + ns3.rndc(f"reload {zone}") def update_is_signed(): parts = update.split() @@ -909,7 +910,7 @@ def test_kasp_default(ns3): shutil.move(f"{key.privatefile}", f"{key.path}.offline") expectmsg = "zone_rekey:zone_verifykeys failed: some key files are missing" with ns3.watch_log_from_here() as watcher: - ns3.rndc(f"loadkeys {zone}", log=False) + ns3.rndc(f"loadkeys {zone}") watcher.wait_for_line(f"zone {zone}/IN (signed): {expectmsg}") # Nothing has changed. expected[0].private = False # noqa @@ -986,7 +987,7 @@ def test_kasp_dynamic(ns3): # Update zone with freeze/thaw. isctest.log.info("check dynamic zone is updated and signed after freeze and thaw") with ns3.watch_log_from_here() as watcher: - ns3.rndc(f"freeze {zone}", log=False) + ns3.rndc(f"freeze {zone}") watcher.wait_for_line(f"freezing zone '{zone}/IN': success") time.sleep(1) @@ -995,7 +996,7 @@ def test_kasp_dynamic(ns3): time.sleep(1) with ns3.watch_log_from_here() as watcher: - ns3.rndc(f"thaw {zone}", log=False) + ns3.rndc(f"thaw {zone}") watcher.wait_for_line(f"thawing zone '{zone}/IN': success") expected_updates = [f"a.{zone}. A 10.0.0.1", f"d.{zone}. A 10.0.0.44"] @@ -1025,7 +1026,7 @@ def test_kasp_dynamic(ns3): "check dynamic inline-signed zone is updated and signed after freeze and thaw" ) with ns3.watch_log_from_here() as watcher: - ns3.rndc(f"freeze {zone}", log=False) + ns3.rndc(f"freeze {zone}") watcher.wait_for_line(f"freezing zone '{zone}/IN': success") time.sleep(1) @@ -1033,7 +1034,7 @@ def test_kasp_dynamic(ns3): time.sleep(1) with ns3.watch_log_from_here() as watcher: - ns3.rndc(f"thaw {zone}", log=False) + ns3.rndc(f"thaw {zone}") watcher.wait_for_line(f"thawing zone '{zone}/IN': success") expected_updates = [f"a.{zone}. A 10.0.0.11", f"d.{zone}. A 10.0.0.44"] @@ -1090,7 +1091,7 @@ def test_kasp_checkds(ns3): ksk = ksks[0] isctest.log.info("check if checkds -publish correctly sets DSPublish") - ns3.rndc(f"dnssec -checkds -when {now} published {zone}", log=False) + ns3.rndc(f"dnssec -checkds -when {now} published {zone}") metadata = f"DSPublish: {now}" isctest.run.retry_with_timeout(wait_for_metadata, timeout=3) expected[0].metadata["DSState"] = "rumoured" @@ -1098,7 +1099,7 @@ def test_kasp_checkds(ns3): isctest.kasp.check_keys(zone, keys, expected) isctest.log.info("check if checkds -withdrawn correctly sets DSRemoved") - ns3.rndc(f"dnssec -checkds -when {now} withdrawn {zone}", log=False) + ns3.rndc(f"dnssec -checkds -when {now} withdrawn {zone}") metadata = f"DSRemoved: {now}" isctest.run.retry_with_timeout(wait_for_metadata, timeout=3) expected[0].metadata["DSState"] = "unretentive" @@ -1138,8 +1139,8 @@ def test_kasp_checkds_doubleksk(ns3): isctest.log.info("check invalid checkds commands") def check_error(): - response = ns3.rndc(test["command"], log=False) - assert test["error"] in response + response = ns3.rndc(test["command"], stderr=subprocess.STDOUT) + assert test["error"] in response.out test_cases = [ { @@ -1163,7 +1164,7 @@ def test_kasp_checkds_doubleksk(ns3): check_error() isctest.log.info("check if checkds -publish -key correctly sets DSPublish") - ns3.rndc(f"dnssec -checkds -when {now} -key {ksk.tag} published {zone}", log=False) + ns3.rndc(f"dnssec -checkds -when {now} -key {ksk.tag} published {zone}") metadata = f"DSPublish: {now}" isctest.run.retry_with_timeout(wait_for_metadata, timeout=3) expected[0].metadata["DSState"] = "rumoured" @@ -1172,7 +1173,7 @@ def test_kasp_checkds_doubleksk(ns3): isctest.log.info("check if checkds -withdrawn -key correctly sets DSRemoved") ksk = ksks[1] - ns3.rndc(f"dnssec -checkds -when {now} -key {ksk.tag} withdrawn {zone}", log=False) + ns3.rndc(f"dnssec -checkds -when {now} -key {ksk.tag} withdrawn {zone}") metadata = f"DSRemoved: {now}" isctest.run.retry_with_timeout(wait_for_metadata, timeout=3) expected[1].metadata["DSState"] = "unretentive" @@ -1205,7 +1206,7 @@ def test_kasp_checkds_csk(ns3): ksk = keys[0] isctest.log.info("check if checkds -publish csk correctly sets DSPublish") - ns3.rndc(f"dnssec -checkds -when {now} published {zone}", log=False) + ns3.rndc(f"dnssec -checkds -when {now} published {zone}") metadata = f"DSPublish: {now}" isctest.run.retry_with_timeout(wait_for_metadata, timeout=3) expected[0].metadata["DSState"] = "rumoured" @@ -1213,7 +1214,7 @@ def test_kasp_checkds_csk(ns3): isctest.kasp.check_keys(zone, keys, expected) isctest.log.info("check if checkds -withdrawn csk correctly sets DSRemoved") - ns3.rndc(f"dnssec -checkds -when {now} withdrawn {zone}", log=False) + ns3.rndc(f"dnssec -checkds -when {now} withdrawn {zone}") metadata = f"DSRemoved: {now}" isctest.run.retry_with_timeout(wait_for_metadata, timeout=3) expected[0].metadata["DSState"] = "unretentive" @@ -1597,7 +1598,7 @@ def test_kasp_zsk_retired(ns3): # Load again, make sure the purged key is not an issue when verifying keys. with ns3.watch_log_from_here() as watcher: - ns3.rndc(f"loadkeys {zone}", log=False) + ns3.rndc(f"loadkeys {zone}") watcher.wait_for_line(f"keymgr: {zone} done") msg = f"zone {zone}/IN (signed): zone_rekey:zone_verifykeys failed: some key files are missing" @@ -1621,7 +1622,7 @@ def test_kasp_purge_keys(ns4): # Reconfig, make sure the purged key is not an issue when verifying keys. shutil.copyfile("ns4/purgekeys2.conf", "ns4/purgekeys.conf") with ns4.watch_log_from_here() as watcher: - ns4.rndc("reconfig", log=False) + ns4.rndc("reconfig") watcher.wait_for_line(f"keymgr: {zone} done") msg = f"zone {zone}/IN/example1 (signed): zone_rekey:zone_verifykeys failed: some key files are missing" @@ -1667,7 +1668,7 @@ def test_kasp_reload_restart(ns6): shutil.copyfile(f"ns6/{zone}2.db.in", f"ns6/{zone}.db") with ns6.watch_log_from_here() as watcher: - ns6.rndc("reload", log=False) + ns6.rndc("reload") watcher.wait_for_line("all zones loaded") newttl = 300 @@ -1744,7 +1745,7 @@ def test_kasp_manual_mode(ns3): # Force step. with ns3.watch_log_from_here() as watcher: - ns3.rndc(f"dnssec -step {zone}", log=False) + ns3.rndc(f"dnssec -step {zone}") watcher.wait_for_line( f"zone {zone}/IN (signed): zone_rekey:zone_verifykeys failed: some key files are missing" ) @@ -1757,7 +1758,7 @@ def test_kasp_manual_mode(ns3): # Load keys. with ns3.watch_log_from_here() as watcher: - ns3.rndc(f"loadkeys {zone}", log=False) + ns3.rndc(f"loadkeys {zone}") watcher.wait_for_line(blockmsg) # Check keys again, make sure no new keys are created. @@ -1768,7 +1769,7 @@ def test_kasp_manual_mode(ns3): # Force step. with ns3.watch_log_from_here() as watcher: - ns3.rndc(f"dnssec -step {zone}", log=False) + ns3.rndc(f"dnssec -step {zone}") watcher.wait_for_line( f"zone {zone}/IN (signed): zone_rekey done: key {tag}/ECDSAP256SHA256" ) diff --git a/bin/tests/system/keepalive/tests_keepalive.py b/bin/tests/system/keepalive/tests_keepalive.py index 1025ac5384..d0efc6af54 100644 --- a/bin/tests/system/keepalive/tests_keepalive.py +++ b/bin/tests/system/keepalive/tests_keepalive.py @@ -20,7 +20,7 @@ pytestmark = pytest.mark.extra_artifacts( def test_dig_tcp_keepalive_handling(named_port, ns2): def get_keepalive_options_received(): - ns2.rndc("stats", log=False) + ns2.rndc("stats") options_received = 0 with open("ns2/named.stats", "r", encoding="utf-8") as ns2_stats_file: for line in ns2_stats_file: @@ -55,12 +55,12 @@ def test_dig_tcp_keepalive_handling(named_port, ns2): ) isctest.log.info("check a re-configured keepalive value") - response = ns2.rndc("tcp-timeouts 300 300 300 200 100", log=False) - assert "tcp-initial-timeout=300" in response - assert "tcp-idle-timeout=300" in response - assert "tcp-keepalive-timeout=300" in response - assert "tcp-advertised-timeout=200" in response - assert "tcp-primaries-timeout=100" in response + response = ns2.rndc("tcp-timeouts 300 300 300 200 100") + assert "tcp-initial-timeout=300" in response.out + assert "tcp-idle-timeout=300" in response.out + assert "tcp-keepalive-timeout=300" in response.out + assert "tcp-advertised-timeout=200" in response.out + assert "tcp-primaries-timeout=100" in response.out assert ( "; TCP-KEEPALIVE: 20.0 secs" in dig("+tcp +keepalive foo.example. @10.53.0.2").out diff --git a/bin/tests/system/ksr/tests_ksr.py b/bin/tests/system/ksr/tests_ksr.py index 553eda1dff..78c8f841b9 100644 --- a/bin/tests/system/ksr/tests_ksr.py +++ b/bin/tests/system/ksr/tests_ksr.py @@ -740,13 +740,12 @@ def test_ksr_common(ns1): f"addzone {zone} " + "{ type primary; file " + f'"{zone}.db"; dnssec-policy {policy}; ' - + "};", - log=False, + + "};" ) # import skr shutil.copyfile(skr_fname, f"ns1/{skr_fname}") - ns1.rndc(f"skr -import {skr_fname} {zone}", log=False) + ns1.rndc(f"skr -import {skr_fname} {zone}") # test zone is correctly signed # - check rndc dnssec -status output @@ -817,12 +816,11 @@ def test_ksr_lastbundle(ns1): + "{ type primary; file " + f'"{zone}.db"; dnssec-policy {policy}; ' + "};", - log=False, ) # import skr shutil.copyfile(skr_fname, f"ns1/{skr_fname}") - ns1.rndc(f"skr -import {skr_fname} {zone}", log=False) + ns1.rndc(f"skr -import {skr_fname} {zone}") # test zone is correctly signed # - check rndc dnssec -status output @@ -896,12 +894,11 @@ def test_ksr_inthemiddle(ns1): + "{ type primary; file " + f'"{zone}.db"; dnssec-policy {policy}; ' + "};", - log=False, ) # import skr shutil.copyfile(skr_fname, f"ns1/{skr_fname}") - ns1.rndc(f"skr -import {skr_fname} {zone}", log=False) + ns1.rndc(f"skr -import {skr_fname} {zone}") # test zone is correctly signed # - check rndc dnssec -status output @@ -967,12 +964,11 @@ def check_ksr_rekey_logs_error(server, zone, policy, offset, end): + "{ type primary; file " + f'"{zone}.db"; dnssec-policy {policy}; ' + "};", - log=False, ) # import skr shutil.copyfile(skr_fname, f"ns1/{skr_fname}") - server.rndc(f"skr -import {skr_fname} {zone}", log=False) + server.rndc(f"skr -import {skr_fname} {zone}") # test that rekey logs error time_remaining = 10 @@ -1090,12 +1086,11 @@ def test_ksr_unlimited(ns1): + "{ type primary; file " + f'"{zone}.db"; dnssec-policy {policy}; ' + "};", - log=False, ) # import skr shutil.copyfile(skr_fname, f"ns1/{skr_fname}") - ns1.rndc(f"skr -import {skr_fname} {zone}", log=False) + ns1.rndc(f"skr -import {skr_fname} {zone}") # test zone is correctly signed # - check rndc dnssec -status output @@ -1201,12 +1196,11 @@ def test_ksr_twotone(ns1): + "{ type primary; file " + f'"{zone}.db"; dnssec-policy {policy}; ' + "};", - log=False, ) # import skr shutil.copyfile(skr_fname, f"ns1/{skr_fname}") - ns1.rndc(f"skr -import {skr_fname} {zone}", log=False) + ns1.rndc(f"skr -import {skr_fname} {zone}") # test zone is correctly signed # - check rndc dnssec -status output @@ -1280,12 +1274,11 @@ def test_ksr_kskroll(ns1): + "{ type primary; file " + f'"{zone}.db"; dnssec-policy {policy}; ' + "};", - log=False, ) # import skr shutil.copyfile(skr_fname, f"ns1/{skr_fname}") - ns1.rndc(f"skr -import {skr_fname} {zone}", log=False) + ns1.rndc(f"skr -import {skr_fname} {zone}") # test zone is correctly signed # - check rndc dnssec -status output diff --git a/bin/tests/system/multisigner/tests_multisigner.py b/bin/tests/system/multisigner/tests_multisigner.py index 020d39791d..6fdb177bd9 100644 --- a/bin/tests/system/multisigner/tests_multisigner.py +++ b/bin/tests/system/multisigner/tests_multisigner.py @@ -182,7 +182,7 @@ def check_add_zsk(server, zone, keys, expected, extra_keys, extra, primary=None) # Trigger keymgr. with server.watch_log_from_here() as watcher: - server.rndc(f"loadkeys {zone}", log=False) + server.rndc(f"loadkeys {zone}") watcher.wait_for_line(f"keymgr: {zone} done") # Check again. @@ -220,7 +220,7 @@ def _check_remove_zsk_fail( # Trigger keymgr. with server.watch_log_from_here() as watcher: - server.rndc(f"loadkeys {zone}", log=False) + server.rndc(f"loadkeys {zone}") watcher.wait_for_line(f"keymgr: {zone} done") # Check again. @@ -263,7 +263,7 @@ def check_remove_zsk( # Trigger keymgr. with server.watch_log_from_here() as watcher: - server.rndc(f"loadkeys {zone}", log=False) + server.rndc(f"loadkeys {zone}") watcher.wait_for_line(f"keymgr: {zone} done") # Check again. @@ -299,7 +299,7 @@ def check_add_cdnskey(server, zone, keys, expected, extra_keys, extra, primary=N # Trigger keymgr. with server.watch_log_from_here() as watcher: - server.rndc(f"loadkeys {zone}", log=False) + server.rndc(f"loadkeys {zone}") watcher.wait_for_line(f"keymgr: {zone} done") # Check again. @@ -336,7 +336,7 @@ def _check_remove_cdnskey_fail( # Trigger keymgr. with server.watch_log_from_here() as watcher: - server.rndc(f"loadkeys {zone}", log=False) + server.rndc(f"loadkeys {zone}") watcher.wait_for_line(f"keymgr: {zone} done") # Check again. @@ -379,7 +379,7 @@ def check_remove_cdnskey( # Trigger keymgr. with server.watch_log_from_here() as watcher: - server.rndc(f"loadkeys {zone}", log=False) + server.rndc(f"loadkeys {zone}") watcher.wait_for_line(f"keymgr: {zone} done") # Check again. @@ -415,7 +415,7 @@ def check_add_cds(server, zone, keys, expected, extra_keys, extra, primary=None) # Trigger keymgr. with server.watch_log_from_here() as watcher: - server.rndc(f"loadkeys {zone}", log=False) + server.rndc(f"loadkeys {zone}") watcher.wait_for_line(f"keymgr: {zone} done") # Check again. @@ -452,7 +452,7 @@ def _check_remove_cds_fail( # Trigger keymgr. with server.watch_log_from_here() as watcher: - server.rndc(f"loadkeys {zone}", log=False) + server.rndc(f"loadkeys {zone}") watcher.wait_for_line(f"keymgr: {zone} done") # Check again. @@ -495,7 +495,7 @@ def check_remove_cds( # Trigger keymgr. with server.watch_log_from_here() as watcher: - server.rndc(f"loadkeys {zone}", log=False) + server.rndc(f"loadkeys {zone}") watcher.wait_for_line(f"keymgr: {zone} done") # Check again. diff --git a/bin/tests/system/nsec3/tests_nsec3_change.py b/bin/tests/system/nsec3/tests_nsec3_change.py index a65a1789be..4ebb2be2bc 100644 --- a/bin/tests/system/nsec3/tests_nsec3_change.py +++ b/bin/tests/system/nsec3/tests_nsec3_change.py @@ -114,4 +114,4 @@ def test_nsec3_case(ns3): # Using rndc signing -nsec3param (should fail) isctest.log.info(f"use rndc signing -nsec3param {zone} to change NSEC3 settings") response = ns3.rndc(f"signing -nsec3param 1 1 12 ffff {zone}") - assert "zone uses dnssec-policy, use rndc dnssec command instead" in response + assert "zone uses dnssec-policy, use rndc dnssec command instead" in response.out diff --git a/bin/tests/system/nta/tests_nta.py b/bin/tests/system/nta/tests_nta.py index f4ca8d3e7f..70e8f08891 100644 --- a/bin/tests/system/nta/tests_nta.py +++ b/bin/tests/system/nta/tests_nta.py @@ -10,17 +10,12 @@ # information regarding copyright ownership. import os -import re +from re import compile as Re import time import isctest -# helper functions -def hasmatch(regex, blob): - return re.search(regex, blob, flags=re.MULTILINE) - - def active(blob): return len([x for x in blob.splitlines() if " expiry" in x]) @@ -48,8 +43,8 @@ def test_initial(): def test_nta_validate_except(servers): ns4 = servers["ns4"] - response = ns4.rndc("secroots -", log=False) - assert hasmatch("^corp: permanent", response) + response = ns4.rndc("secroots -") + assert Re("^corp: permanent") in response.out # check insecure local domain works with validate-except m = isctest.query.create("www.corp", "NS") @@ -62,41 +57,39 @@ def test_nta_bogus_lifetimes(servers): ns4 = servers["ns4"] # no nta lifetime specified: - response = ns4.rndc("nta -l '' foo", ignore_errors=True, log=False) - assert "'nta' failed: bad ttl" in response + response = ns4.rndc("nta -l '' foo", raise_on_exception=False) + assert "'nta' failed: bad ttl" in response.err # bad nta lifetime: - response = ns4.rndc("nta -l garbage foo", ignore_errors=True, log=False) - assert "'nta' failed: bad ttl" in response + response = ns4.rndc("nta -l garbage foo", raise_on_exception=False) + assert "'nta' failed: bad ttl" in response.err # excessive nta lifetime: - response = ns4.rndc("nta -l 7d1h foo", ignore_errors=True, log=False) - assert "'nta' failed: out of range" in response + response = ns4.rndc("nta -l 7d1h foo", raise_on_exception=False) + assert "'nta' failed: out of range" in response.err def test_nta_install(servers): global start ns4 = servers["ns4"] - ns4.rndc("nta -f -l 20s bogus.example", log=False) - ns4.rndc("nta badds.example", log=False) + ns4.rndc("nta -f -l 20s bogus.example") + ns4.rndc("nta badds.example") # NTAs should persist after reconfig - with ns4.watch_log_from_here() as watcher: - ns4.reconfigure(log=False) - watcher.wait_for_line("any newly configured zones are now loaded") + ns4.reconfigure() - response = ns4.rndc("nta -d", log=False) - assert len(response.splitlines()) == 3 + response = ns4.rndc("nta -d") + assert len(response.out.splitlines()) == 3 - ns4.rndc("nta secure.example", log=False) - ns4.rndc("nta fakenode.secure.example", log=False) + ns4.rndc("nta secure.example") + ns4.rndc("nta fakenode.secure.example") with ns4.watch_log_from_here() as watcher: - ns4.rndc("reload", log=False) + ns4.rndc("reload") watcher.wait_for_line("all zones loaded") - response = ns4.rndc("nta -d", log=False) - assert len(response.splitlines()) == 5 + response = ns4.rndc("nta -d") + assert len(response.out.splitlines()) == 5 start = time.time() @@ -125,11 +118,11 @@ def test_nta_behavior(servers): isctest.check.noadflag(res) ns4 = servers["ns4"] - response = ns4.rndc("secroots -", log=False) - assert hasmatch("^bogus.example: expiry", response) - assert hasmatch("^badds.example: expiry", response) - assert hasmatch("^secure.example: expiry", response) - assert hasmatch("^fakenode.secure.example: expiry", response) + response = ns4.rndc("secroots -") + assert Re("^bogus.example: expiry") in response.out + assert Re("^badds.example: expiry") in response.out + assert Re("^secure.example: expiry") in response.out + assert Re("^fakenode.secure.example: expiry") in response.out # secure.example and badds.example used the default nta-duration # (configured as 12s in ns4/named1.conf), but the nta recheck interval @@ -162,12 +155,12 @@ def test_nta_behavior(servers): if delay > 0: time.sleep(delay) - response = ns4.rndc("nta -d", log=False) - assert active(response) <= 2 + response = ns4.rndc("nta -d") + assert active(response.out) <= 2 - response = ns4.rndc("secroots -", log=False) - assert hasmatch("bogus.example: expiry", response) - assert not hasmatch("badds.example: expiry", response) + response = ns4.rndc("secroots -") + assert Re("bogus.example: expiry") in response.out + assert Re("badds.example: expiry") not in response.out m = isctest.query.create("b.bogus.example", "A") res = isctest.query.tcp(m, "10.53.0.4") @@ -188,8 +181,8 @@ def test_nta_behavior(servers): if delay > 0: time.sleep(delay) - response = ns4.rndc("nta -d", log=False) - assert active(response) == 0 + response = ns4.rndc("nta -d") + assert active(response.out) == 0 m = isctest.query.create("d.secure.example", "A") res = isctest.query.tcp(m, "10.53.0.4") @@ -204,31 +197,31 @@ def test_nta_behavior(servers): def test_nta_removals(servers): ns4 = servers["ns4"] - ns4.rndc("nta badds.example", log=False) + ns4.rndc("nta badds.example") - response = ns4.rndc("nta -d", log=False) - assert hasmatch("^badds.example/_default: expiry", response) + response = ns4.rndc("nta -d") + assert Re("^badds.example/_default: expiry") in response.out m = isctest.query.create("a.badds.example", "A") res = isctest.query.tcp(m, "10.53.0.4") isctest.check.noerror(res) isctest.check.noadflag(res) - response = ns4.rndc("nta -remove badds.example", log=False) - assert "Negative trust anchor removed: badds.example" in response + response = ns4.rndc("nta -remove badds.example") + assert "Negative trust anchor removed: badds.example" in response.out - response = ns4.rndc("nta -d", log=False) - assert not hasmatch("^badds.example/_default: expiry", response) + response = ns4.rndc("nta -d") + assert Re("^badds.example/_default: expiry") not in response.out res = isctest.query.tcp(m, "10.53.0.4") isctest.check.servfail(res) isctest.check.noadflag(res) # remove non-existent NTA three times - ns4.rndc("nta -r foo", log=False) - ns4.rndc("nta -remove foo", log=False) - response = ns4.rndc("nta -r foo", log=False) - assert "not found" in response + ns4.rndc("nta -r foo") + ns4.rndc("nta -remove foo") + response = ns4.rndc("nta -r foo") + assert "not found" in response.out def test_nta_restarts(servers): @@ -237,14 +230,14 @@ def test_nta_restarts(servers): # test NTA persistence across restarts ns4 = servers["ns4"] - response = ns4.rndc("nta -d", log=False) - assert active(response) == 0 + response = ns4.rndc("nta -d") + assert active(response.out) == 0 start = time.time() - ns4.rndc("nta -f -l 30s bogus.example", log=False) - ns4.rndc("nta -f -l 10s badds.example", log=False) - response = ns4.rndc("nta -d", log=False) - assert active(response) == 2 + ns4.rndc("nta -f -l 30s bogus.example") + ns4.rndc("nta -f -l 10s badds.example") + response = ns4.rndc("nta -d") + assert active(response.out) == 2 # stop the server ns4.stop() @@ -256,9 +249,9 @@ def test_nta_restarts(servers): time.sleep(delay) ns4.start(["--noclean", "--restart", "--port", os.environ["PORT"]]) - response = ns4.rndc("nta -d", log=False) - assert active(response) == 1 - assert hasmatch("^bogus.example/_default: expiry", response) + response = ns4.rndc("nta -d") + assert active(response.out) == 1 + assert Re("^bogus.example/_default: expiry") in response.out m = isctest.query.create("a.badds.example", "A") res = isctest.query.tcp(m, "10.53.0.4") @@ -269,7 +262,7 @@ def test_nta_restarts(servers): isctest.check.noerror(res) isctest.check.noadflag(res) - ns4.rndc("nta -r bogus.example", log=False) + ns4.rndc("nta -r bogus.example") def test_nta_regular(servers): @@ -279,8 +272,8 @@ def test_nta_regular(servers): # check "regular" attribute in NTA file ns4 = servers["ns4"] - response = ns4.rndc("nta -d", log=False) - assert active(response) == 0 + response = ns4.rndc("nta -d") + assert active(response.out) == 0 # secure.example validates with AD=1 m = isctest.query.create("a.secure.example", "A") @@ -309,12 +302,12 @@ def test_nta_regular(servers): if delay > 0: time.sleep(delay) - response = ns4.rndc("nta -d", log=False) - assert active(response) == 0 + response = ns4.rndc("nta -d") + assert active(response.out) == 0 # NTA lifted; secure.example. flush the cache to trigger a new query, # and it should now return an AD=1 answer. - ns4.rndc("flushtree secure.example", log=False) + ns4.rndc("flushtree secure.example") res = isctest.query.tcp(m, "10.53.0.4") isctest.check.noerror(res) isctest.check.adflag(res) @@ -328,10 +321,10 @@ def test_nta_forced(servers): ns4 = servers["ns4"] # just to be certain, clean up any existing NTA first - ns4.rndc("nta -r secure.example", log=False) + ns4.rndc("nta -r secure.example") - response = ns4.rndc("nta -d", log=False) - assert active(response) == 0 + response = ns4.rndc("nta -d") + assert active(response.out) == 0 # secure.example validates with AD=1 m = isctest.query.create("a.secure.example", "A") @@ -361,7 +354,7 @@ def test_nta_forced(servers): time.sleep(delay) # NTA lifted; secure.example. should still return an AD=0 answer - ns4.rndc("flushtree secure.example", log=False) + ns4.rndc("flushtree secure.example") res = isctest.query.tcp(m, "10.53.0.4") isctest.check.noerror(res) isctest.check.noadflag(res) @@ -371,7 +364,7 @@ def test_nta_clamping(servers): ns4 = servers["ns4"] # clean up any existing NTA - ns4.rndc("nta -r secure.example", log=False) + ns4.rndc("nta -r secure.example") # stop the server, update _default.nta, restart ns4.stop() @@ -383,10 +376,10 @@ def test_nta_clamping(servers): ns4.start(["--noclean", "--restart", "--port", os.environ["PORT"]]) # check that NTA lifetime read from file is clamped to 1 week. - response = ns4.rndc("nta -d", log=False) - assert active(response) == 1 + response = ns4.rndc("nta -d") + assert active(response.out) == 1 - nta = next((s for s in response.splitlines() if " expiry" in s), None) + nta = next((s for s in response.out.splitlines() if " expiry" in s), None) assert nta is not None nta = nta.split(" ") @@ -401,7 +394,7 @@ def test_nta_clamping(servers): assert abs(nextweek - then < 3610) # remove the NTA - ns4.rndc("nta -r secure.example", log=False) + ns4.rndc("nta -r secure.example") def test_nta_forward(servers): @@ -414,14 +407,14 @@ def test_nta_forward(servers): isctest.check.noadflag(res) # add NTA and expect resolution to succeed - ns9.rndc("nta badds.example", log=False) + ns9.rndc("nta badds.example") res = isctest.query.tcp(m, "10.53.0.9") isctest.check.noerror(res) isctest.check.rr_count_eq(res.answer, 2) isctest.check.noadflag(res) # remove NTA and expect resolution to fail again - ns9.rndc("nta -remove badds.example", log=False) + ns9.rndc("nta -remove badds.example") res = isctest.query.tcp(m, "10.53.0.9") isctest.check.servfail(res) isctest.check.empty_answer(res) diff --git a/bin/tests/system/nzd2nzf/tests_nzd2nzf.py b/bin/tests/system/nzd2nzf/tests_nzd2nzf.py index 5ad766c4cf..790e937556 100644 --- a/bin/tests/system/nzd2nzf/tests_nzd2nzf.py +++ b/bin/tests/system/nzd2nzf/tests_nzd2nzf.py @@ -34,7 +34,7 @@ def test_nzd2nzf(ns1): isctest.check.refused(res) # add new zone into the default NZD using "rndc addzone" - ns1.rndc(f"addzone {zone_data}", log=False) + ns1.rndc(f"addzone {zone_data}") # query for existing zone data res = isctest.query.tcp(msg, ns1.ip) diff --git a/bin/tests/system/resolver/tests_resolver.py b/bin/tests/system/resolver/tests_resolver.py index 286a33e4e0..4e99315bcb 100644 --- a/bin/tests/system/resolver/tests_resolver.py +++ b/bin/tests/system/resolver/tests_resolver.py @@ -22,18 +22,20 @@ def test_resolver_cache_reloadfails(ns1, templates): isctest.check.noerror(res) assert res.answer[0].ttl == 300 templates.render("ns1/named.conf", {"wrongoption": True}) - try: - # The first reload fails, and the old cache list will be preserved - ns1.rndc("reload") - except isctest.rndc.RNDCException: - templates.render("ns1/named.conf", {"wrongoption": False}) - # The second reload succeed, and the cache is still there, as preserved - # from the old cache list - ns1.rndc("reload") - time.sleep(3) - msg = isctest.query.create("www.example.org.", "A") - res = isctest.query.udp(msg, "10.53.0.1") - isctest.check.noerror(res) - # The ttl being lower than 300 (provided by fake authoritative) proves - # the cache is still in use - assert res.answer[0].ttl < 300 + + # The first reload fails, and the old cache list will be preserved + cmd = ns1.rndc("reload", raise_on_exception=False) + assert cmd.rc != 0 + + templates.render("ns1/named.conf", {"wrongoption": False}) + # The second reload succeed, and the cache is still there, as preserved + # from the old cache list + ns1.rndc("reload") + time.sleep(3) + msg = isctest.query.create("www.example.org.", "A") + res = isctest.query.udp(msg, "10.53.0.1") + isctest.check.noerror(res) + + # The ttl being lower than 300 (provided by fake authoritative) proves + # the cache is still in use + assert res.answer[0].ttl < 300 diff --git a/bin/tests/system/rollover-zsk-prepub/tests_rollover_zsk_prepublication.py b/bin/tests/system/rollover-zsk-prepub/tests_rollover_zsk_prepublication.py index 17b7d38a57..f0201296fc 100644 --- a/bin/tests/system/rollover-zsk-prepub/tests_rollover_zsk_prepublication.py +++ b/bin/tests/system/rollover-zsk-prepub/tests_rollover_zsk_prepublication.py @@ -224,7 +224,7 @@ def test_zsk_prepub_step3(tld, alg, size, ns3): # Force full resign and check all signatures have been replaced. with ns3.watch_log_from_here() as watcher: - ns3.rndc(f"sign {zone}", log=False) + ns3.rndc(f"sign {zone}") watcher.wait_for_line(f"zone_needdump: zone {zone}/IN (signed): enter") step["smooth"] = False diff --git a/bin/tests/system/rollover/tests_rollover_manual.py b/bin/tests/system/rollover/tests_rollover_manual.py index 0fa5edc58f..75ae7d86c7 100644 --- a/bin/tests/system/rollover/tests_rollover_manual.py +++ b/bin/tests/system/rollover/tests_rollover_manual.py @@ -154,7 +154,7 @@ def test_rollover_manual(ns3): # Try to schedule a ZSK rollover for an inactive key (should fail). zsk = expected[3].key response = ns3.rndc(f"dnssec -rollover -key {zsk.tag} {zone}") - assert "key is not actively signing" in response + assert "key is not actively signing" in response.out def test_rollover_manual_zrrsig_rumoured(ns3): diff --git a/bin/tests/system/showconf/tests_showconf.py b/bin/tests/system/showconf/tests_showconf.py index ae7e3833cc..e7dff5c748 100644 --- a/bin/tests/system/showconf/tests_showconf.py +++ b/bin/tests/system/showconf/tests_showconf.py @@ -19,44 +19,38 @@ def test_showconf(ns1): res = isctest.query.udp(msg, "10.53.0.1") isctest.check.rcode(res, dns.rcode.NOERROR) - effectiveconfig = ns1.rndc("showconf -effective", log=False) - assert 'zone "example.com"' in effectiveconfig - assert 'view "_bind" chaos {' in effectiveconfig + effectiveconfig = ns1.rndc("showconf -effective") + assert 'zone "example.com"' in effectiveconfig.out + assert 'view "_bind" chaos {' in effectiveconfig.out # builtin-trust-anchors is non documented and internal clause only, it must # not be visible. - assert "builtin-trust-anchors" not in effectiveconfig + assert "builtin-trust-anchors" not in effectiveconfig.out # Dynamically added zones are not visible from the effectiveconfig zonedata = '"added.example" { type primary; file "example.db"; };' - ns1.rndc(f"addzone {zonedata}", log=False) + ns1.rndc(f"addzone {zonedata}") msg = isctest.query.create("a.added.example", "A") res = isctest.query.udp(msg, "10.53.0.1") isctest.check.rcode(res, dns.rcode.NOERROR) - effectiveconfig = ns1.rndc("showconf -effective", log=False) - assert 'zone "added.example"' not in effectiveconfig + effectiveconfig = ns1.rndc("showconf -effective") + assert 'zone "added.example"' not in effectiveconfig.out - userconfig = ns1.rndc("showconf -user", log=False) - assert 'zone "example.com"' in userconfig - assert 'view "_bind" chaos {' not in userconfig + userconfig = ns1.rndc("showconf -user") + assert 'zone "example.com"' in userconfig.out + assert 'view "_bind" chaos {' not in userconfig.out - builtinconfig = ns1.rndc("showconf -builtin", log=False) - assert len(userconfig.split()) < len(builtinconfig.split()) - assert len(builtinconfig.split()) < len(effectiveconfig.split()) + builtinconfig = ns1.rndc("showconf -builtin") + assert len(userconfig.out.split()) < len(builtinconfig.out.split()) + assert len(builtinconfig.out.split()) < len(effectiveconfig.out.split()) # Errors handling - error_msg = "" + response = ns1.rndc("showconf -idontexist", raise_on_exception=False) + assert response.rc != 0 + assert "rndc: 'showconf' failed: syntax error" in response.err - try: - ns1.rndc("showconf -idontexist", log=False) - except isctest.rndc.RNDCException as e: - error_msg = str(e) - assert error_msg == "rndc: 'showconf' failed: syntax error\n" - - try: - ns1.rndc("showconf", log=False) - except isctest.rndc.RNDCException as e: - error_msg = str(e) - assert error_msg == "rndc: 'showconf' failed: unexpected end of input\n" + response = ns1.rndc("showconf", raise_on_exception=False) + assert response.rc != 0 + assert "rndc: 'showconf' failed: unexpected end of input" in response.err diff --git a/bin/tests/system/shutdown/tests_shutdown.py b/bin/tests/system/shutdown/tests_shutdown.py index d746b3821d..6f5ecd538e 100755 --- a/bin/tests/system/shutdown/tests_shutdown.py +++ b/bin/tests/system/shutdown/tests_shutdown.py @@ -71,11 +71,8 @@ def do_work(named_proc, resolver_ip, instance, kill_method, n_workers, n_queries # helper function, 'command' is the rndc command to run def launch_rndc(command): - try: - instance.rndc(command, log=False) - return 0 - except isctest.rndc.RNDCException: - return -1 + ret = instance.rndc(command, raise_on_exception=False) + return 0 if ret.rc == 0 else -1 # We're going to execute queries in parallel by means of a thread pool. # dnspython functions block, so we need to circumvent that. diff --git a/bin/tests/system/stress/tests_stress_update.py b/bin/tests/system/stress/tests_stress_update.py index cbc678f6ba..ef98b41137 100644 --- a/bin/tests/system/stress/tests_stress_update.py +++ b/bin/tests/system/stress/tests_stress_update.py @@ -10,7 +10,6 @@ # information regarding copyright ownership. import concurrent.futures -import os import time import dns.update @@ -27,22 +26,8 @@ pytestmark = pytest.mark.extra_artifacts( def rndc_loop(test_state, server): - rndc = os.getenv("RNDC") - port = os.getenv("CONTROLPORT") - - cmdline = [ - rndc, - "-c", - "../_common/rndc.conf", - "-p", - port, - "-s", - server, - "reload", - ] - while not test_state["finished"]: - isctest.run.cmd(cmdline, raise_on_exception=False) + server.rndc("reload", raise_on_exception=False) time.sleep(1) diff --git a/bin/tests/system/synthrecord/tests_synthrecord.py b/bin/tests/system/synthrecord/tests_synthrecord.py index 3af7b3d57a..9e2243d700 100644 --- a/bin/tests/system/synthrecord/tests_synthrecord.py +++ b/bin/tests/system/synthrecord/tests_synthrecord.py @@ -499,7 +499,6 @@ def test_sythreverse_arpa_v6_nxdomain_toomanylabels(domain): def test_synthrecord_inview(ns1, templates): templates.render("ns1/named.conf", {"inview": True}) with ns1.watch_log_from_here() as watcher: - try: - ns1.rndc("reconfig") - except isctest.rndc.RNDCException: - watcher.wait_for_line("'synthrecord' must be configured as a zone plugin") + cmd = ns1.rndc("reconfig", raise_on_exception=False) + assert cmd.rc != 0 + watcher.wait_for_line("'synthrecord' must be configured as a zone plugin") diff --git a/bin/tests/system/views/tests_views_addzones.py b/bin/tests/system/views/tests_views_addzones.py index c362b47721..2baf745fdf 100644 --- a/bin/tests/system/views/tests_views_addzones.py +++ b/bin/tests/system/views/tests_views_addzones.py @@ -20,6 +20,6 @@ def test_views_add_zones(ns2, templates): templates.render("ns2/named.conf", {"zone_names": zone_names}) shutil.copyfile("ns2/zone.db.in", f"ns2/{name}.db") with ns2.watch_log_from_here() as watcher: - ns2.rndc("reconfig", log=False) + ns2.rndc("reconfig") log_seq = ["any newly configured zones are now loaded", "running"] watcher.wait_for_sequence(log_seq)