From b3caa9226db6d4cfaf613e41f5dfc0b98afe5d3e Mon Sep 17 00:00:00 2001
From: Tom Krizek <tkrizek@isc.org>
Date: Tue, 2 Jan 2024 16:17:43 +0100
Subject: [PATCH 01/15] Remove terminal color support from conf.sh.common

Pytest processes all the output, so the terminal colors are no longer
interpreted. Remove the obsolete code.
---
 bin/tests/system/conf.sh.common | 129 ++++++++------------------------
 1 file changed, 33 insertions(+), 96 deletions(-)

diff --git a/bin/tests/system/conf.sh.common b/bin/tests/system/conf.sh.common
index 7a4d12e256..8b5d6ec636 100644
--- a/bin/tests/system/conf.sh.common
+++ b/bin/tests/system/conf.sh.common
@@ -21,107 +21,44 @@ testsock6() {
 }
 
 export LANG=C
-
-
-#
-# Set up color-coded test output
-#
-if [ ${SYSTEMTEST_FORCE_COLOR:-0} -eq 1 ] || test -t 1 && type tput > /dev/null 2>&1 && tput setaf 7 > /dev/null 2>&1 ; then
-    export COLOR_END=$(tput setaf 4)    # blue
-    export COLOR_FAIL=$(tput setaf 1)   # red
-    export COLOR_INFO=$(tput bold)      # bold
-    export COLOR_NONE=$(tput sgr0)
-    export COLOR_PASS=$(tput setaf 2)   # green
-    export COLOR_START=$(tput setaf 4)  # blue
-    export COLOR_WARN=$(tput setaf 3)   # yellow
-else
-    # set to empty strings so printf succeeds
-    export COLOR_END=''
-    export COLOR_FAIL=''
-    export COLOR_INFO=''
-    export COLOR_NONE=''
-    export COLOR_PASS=''
-    export COLOR_START=''
-    export COLOR_WARN=''
-fi
-
 export SYSTESTDIR="$(basename $PWD)"
 
-if type printf > /dev/null 2>&1
-then
-	echofail () {
-		printf "${COLOR_FAIL}%s${COLOR_NONE}\n" "$*"
-	}
-	echowarn () {
-		printf "${COLOR_WARN}%s${COLOR_NONE}\n" "$*"
-	}
-	echopass () {
-		printf "${COLOR_PASS}%s${COLOR_NONE}\n" "$*"
-	}
-	echoinfo () {
-		printf "${COLOR_INFO}%s${COLOR_NONE}\n" "$*"
-	}
-	echostart () {
-		printf "${COLOR_START}%s${COLOR_NONE}\n" "$*"
-	}
-	echoend () {
-		printf "${COLOR_END}%s${COLOR_NONE}\n" "$*"
-	}
-	echo_i() {
-	    printf '%s\n' "$*" | while IFS= read -r __LINE ; do
-	       echoinfo "I:$__LINE"
-	    done
-	}
+echofail () {
+    echo "$*"
+}
+echowarn () {
+    echo "$*"
+}
+echopass () {
+    echo "$*"
+}
+echoinfo () {
+    echo "$*"
+}
+echostart () {
+    echo "$*"
+}
+echoend () {
+    echo "$*"
+}
 
-	echo_ic() {
-	    printf '%s\n' "$*" | while IFS= read -r __LINE ; do
-	       echoinfo "I:  $__LINE"
-	    done
-	}
+echo_i() {
+    echo "$@" | while IFS= read -r __LINE ; do
+       echoinfo "I:$__LINE"
+    done
+}
 
-	echo_d() {
-	    printf '%s\n' "$*" | while IFS= read -r __LINE ; do
-	       echoinfo "D:$__LINE"
-	    done
-	}
-else
-	echofail () {
-		echo "$*"
-	}
-	echowarn () {
-		echo "$*"
-	}
-	echopass () {
-		echo "$*"
-	}
-	echoinfo () {
-		echo "$*"
-	}
-	echostart () {
-		echo "$*"
-	}
-	echoend () {
-		echo "$*"
-	}
+echo_ic() {
+    echo "$@" | while IFS= read -r __LINE ; do
+       echoinfo "I:  $__LINE"
+    done
+}
 
-	echo_i() {
-	    echo "$@" | while IFS= read -r __LINE ; do
-	       echoinfo "I:$__LINE"
-	    done
-	}
-
-	echo_ic() {
-	    echo "$@" | while IFS= read -r __LINE ; do
-	       echoinfo "I:  $__LINE"
-	    done
-	}
-
-	echo_d() {
-	    echo "$@" | while IFS= read -r __LINE ; do
-	       echoinfo "D:$__LINE"
-	    done
-	}
-fi
+echo_d() {
+    echo "$@" | while IFS= read -r __LINE ; do
+       echoinfo "D:$__LINE"
+    done
+}
 
 cat_i() {
     while IFS= read -r __LINE ; do

From ab27f504ca52b04ceca06bd1493690cc32ff73eb Mon Sep 17 00:00:00 2001
From: Tom Krizek <tkrizek@isc.org>
Date: Thu, 29 Feb 2024 14:51:54 +0100
Subject: [PATCH 02/15] Move environment variables from conf.sh to pytest

Remove conf.sh.in and move the environment variables into isctest/vars
python package. This enabled the removal of an ugly pytest hack which
loaded and parsed these variables from the environment.
---
 bin/tests/.gitignore                         |   1 -
 bin/tests/system/.gitignore                  |   1 +
 bin/tests/system/{conf.sh.common => conf.sh} |   3 -
 bin/tests/system/conf.sh.in                  | 100 -------------------
 bin/tests/system/conftest.py                 |  49 +++------
 bin/tests/system/isctest/__init__.py         |   1 +
 bin/tests/system/isctest/check.py            |   6 ++
 bin/tests/system/isctest/vars/__init__.py    |  12 +++
 bin/tests/system/isctest/vars/all.py         |  18 ++++
 bin/tests/system/isctest/vars/autoconf.py.in |  23 +++++
 bin/tests/system/isctest/vars/basic.py       |  64 ++++++++++++
 configure.ac                                 |   2 +-
 12 files changed, 138 insertions(+), 142 deletions(-)
 rename bin/tests/system/{conf.sh.common => conf.sh} (99%)
 delete mode 100644 bin/tests/system/conf.sh.in
 create mode 100644 bin/tests/system/isctest/vars/__init__.py
 create mode 100644 bin/tests/system/isctest/vars/all.py
 create mode 100644 bin/tests/system/isctest/vars/autoconf.py.in
 create mode 100644 bin/tests/system/isctest/vars/basic.py

diff --git a/bin/tests/.gitignore b/bin/tests/.gitignore
index 26185a7889..9a60c2ca80 100644
--- a/bin/tests/.gitignore
+++ b/bin/tests/.gitignore
@@ -3,7 +3,6 @@ nxtify
 sdig
 *_test
 gsstest
-conf.sh
 dlopen
 keycreate
 keydelete
diff --git a/bin/tests/system/.gitignore b/bin/tests/system/.gitignore
index 731e60692c..05ae7f8b4e 100644
--- a/bin/tests/system/.gitignore
+++ b/bin/tests/system/.gitignore
@@ -19,6 +19,7 @@ named.run
 /start.sh
 /stop.sh
 /ifconfig.sh
+/isctest/vars/autoconf.py
 
 # Ignore file names with underscore in their name except python or shell files.
 # This is done to ignore the temporary directories and symlinks created by the
diff --git a/bin/tests/system/conf.sh.common b/bin/tests/system/conf.sh
similarity index 99%
rename from bin/tests/system/conf.sh.common
rename to bin/tests/system/conf.sh
index 8b5d6ec636..131c72dbbb 100644
--- a/bin/tests/system/conf.sh.common
+++ b/bin/tests/system/conf.sh
@@ -20,9 +20,6 @@ testsock6() {
 	fi
 }
 
-export LANG=C
-export SYSTESTDIR="$(basename $PWD)"
-
 echofail () {
     echo "$*"
 }
diff --git a/bin/tests/system/conf.sh.in b/bin/tests/system/conf.sh.in
deleted file mode 100644
index 7b5db05baf..0000000000
--- a/bin/tests/system/conf.sh.in
+++ /dev/null
@@ -1,100 +0,0 @@
-#!/bin/sh
-#
-# 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.
-
-#
-# Common configuration data for system tests, to be sourced into
-# other shell scripts.
-#
-
-# Find the top of the BIND9 tree.
-export TOP_BUILDDIR=@abs_top_builddir@
-export TOP_SRCDIR=@abs_top_srcdir@
-
-# Provide TMPDIR variable for tests that need it.
-export TMPDIR=${TMPDIR:-/tmp}
-
-export ARPANAME=$TOP_BUILDDIR/bin/tools/arpaname
-export CDS=$TOP_BUILDDIR/bin/dnssec/dnssec-cds
-export CHECKCONF=$TOP_BUILDDIR/bin/check/named-checkconf
-export CHECKZONE=$TOP_BUILDDIR/bin/check/named-checkzone
-if [ -z "$TSAN_OPTIONS" ]; then # workaround for GL#4119
-  export DELV=$TOP_BUILDDIR/bin/delv/delv
-else
-  export DELV=:
-fi
-export DIG=$TOP_BUILDDIR/bin/dig/dig
-export DNSTAPREAD=$TOP_BUILDDIR/bin/tools/dnstap-read
-export DSFROMKEY=$TOP_BUILDDIR/bin/dnssec/dnssec-dsfromkey
-export FEATURETEST=$TOP_BUILDDIR/bin/tests/system/feature-test
-export FSTRM_CAPTURE=@FSTRM_CAPTURE@
-export HOST=$TOP_BUILDDIR/bin/dig/host
-export IMPORTKEY=$TOP_BUILDDIR/bin/dnssec/dnssec-importkey
-export JOURNALPRINT=$TOP_BUILDDIR/bin/tools/named-journalprint
-export KEYFRLAB=$TOP_BUILDDIR/bin/dnssec/dnssec-keyfromlabel
-export KEYGEN=$TOP_BUILDDIR/bin/dnssec/dnssec-keygen
-export KSR=$TOP_BUILDDIR/bin/dnssec/dnssec-ksr
-export MDIG=$TOP_BUILDDIR/bin/tools/mdig
-export NAMED=$TOP_BUILDDIR/bin/named/named
-export NSEC3HASH=$TOP_BUILDDIR/bin/tools/nsec3hash
-export NSLOOKUP=$TOP_BUILDDIR/bin/dig/nslookup
-export NSUPDATE=$TOP_BUILDDIR/bin/nsupdate/nsupdate
-export NZD2NZF=$TOP_BUILDDIR/bin/tools/named-nzd2nzf
-export REVOKE=$TOP_BUILDDIR/bin/dnssec/dnssec-revoke
-export RNDC=$TOP_BUILDDIR/bin/rndc/rndc
-export RNDCCONFGEN=$TOP_BUILDDIR/bin/confgen/rndc-confgen
-export RRCHECKER=$TOP_BUILDDIR/bin/tools/named-rrchecker
-export SETTIME=$TOP_BUILDDIR/bin/dnssec/dnssec-settime
-export SIGNER=$TOP_BUILDDIR/bin/dnssec/dnssec-signzone
-export TSIGKEYGEN=$TOP_BUILDDIR/bin/confgen/tsig-keygen
-export VERIFY=$TOP_BUILDDIR/bin/dnssec/dnssec-verify
-export WIRETEST=$TOP_BUILDDIR/bin/tests/wire_test
-
-export BIGKEY=$TOP_BUILDDIR/bin/tests/system/rsabigexponent/bigkey
-export GENCHECK=$TOP_BUILDDIR/bin/tests/system/rndc/gencheck
-export MAKEJOURNAL=$TOP_BUILDDIR/bin/tests/system/makejournal
-export PIPEQUERIES=$TOP_BUILDDIR/bin/tests/system/pipelined/pipequeries
-
-# we don't want a KRB5_CONFIG setting breaking the tests
-export KRB5_CONFIG=/dev/null
-# use local keytab instead of default /etc/krb5.keytab
-export KRB5_KTNAME=dns.keytab
-
-export ANS_LOG_LEVEL=debug
-
-#
-# Programs detected by configure
-# Variables will be empty if no program was found by configure
-#
-export SHELL=@SHELL@
-export CURL=@CURL@
-export NC=@NC@
-export XMLLINT=@XMLLINT@
-export XSLTPROC=@XSLTPROC@
-export PYTEST=@PYTEST@
-
-#
-# Interpreters for system tests detected by configure
-#
-export PERL=$(command -v "@PERL@" || true)
-if ! test -x "$PERL"; then
-  echo "Perl interpreter is required for system tests."
-  exit 77
-fi
-export PYTHON=$(command -v "@PYTHON@" || true)
-if ! test -x "$PYTHON"; then
-  echo "Python interpreter is required for system tests."
-  exit 77
-fi
-
-# Load common values
-. $TOP_SRCDIR/bin/tests/system/conf.sh.common
diff --git a/bin/tests/system/conftest.py b/bin/tests/system/conftest.py
index 0558bfd68a..109618e1aa 100644
--- a/bin/tests/system/conftest.py
+++ b/bin/tests/system/conftest.py
@@ -70,42 +70,16 @@ SYMLINK_REPLACEMENT_RE = re.compile(r"/tests(_.*)\.py")
 
 # ---------------------- Module initialization ---------------------------
 
+# Set environment variables for tests.
+os.environ.update(isctest.vars.ALL)
+isctest.log.debug(
+    "variables in env: %s", ", ".join([str(key) for key in isctest.vars.ALL])
+)
 
-def parse_env(env_bytes):
-    """Parse the POSIX env format into Python dictionary."""
-    out = {}
-    for line in env_bytes.splitlines():
-        match = ENV_RE.match(line)
-        if match:
-            # EL8+ workaround for https://access.redhat.com/solutions/6994985
-            # FUTURE: can be removed when we no longer need to parse env vars
-            if match.groups()[0] in [b"which_declare", b"BASH_FUNC_which%%"]:
-                continue
-            out[match.groups()[0]] = match.groups()[1]
-    return out
+# ----------------------- Global requirements ----------------------------
 
-
-def get_env_bytes(cmd):
-    try:
-        proc = subprocess.run(
-            [cmd],
-            shell=True,
-            check=True,
-            cwd=FILE_DIR,
-            stdout=subprocess.PIPE,
-        )
-    except subprocess.CalledProcessError as exc:
-        isctest.log.error("failed to get shell env: %s", exc)
-        raise exc
-    env_bytes = proc.stdout
-    return parse_env(env_bytes)
-
-
-# Read common environment variables for running tests from conf.sh.
-# FUTURE: Remove conf.sh entirely and define all variables in pytest only.
-CONF_ENV = get_env_bytes(". ./conf.sh && env")
-os.environb.update(CONF_ENV)
-isctest.log.debug("variables in env: %s", ", ".join([str(key) for key in CONF_ENV]))
+isctest.check.is_executable(isctest.vars.ALL["PYTHON"], "Python interpreter required")
+isctest.check.is_executable(isctest.vars.ALL["PERL"], "Perl interpreter required")
 
 # --------------------------- pytest hooks -------------------------------
 
@@ -348,7 +322,7 @@ def logger(request, system_test_name):
 
 @pytest.fixture(scope="module")
 def system_test_dir(
-    request, env, system_test_name
+    request, system_test_name
 ):  # pylint: disable=too-many-statements,too-many-locals
     """
     Temporary directory for executing the test.
@@ -398,7 +372,9 @@ def system_test_dir(
             pass
 
     # Create a temporary directory with a copy of the original system test dir contents
-    system_test_root = Path(f"{env['TOP_BUILDDIR']}/{SYSTEM_TEST_DIR_GIT_PATH}")
+    system_test_root = Path(
+        f"{isctest.vars.ALL['TOP_BUILDDIR']}/{SYSTEM_TEST_DIR_GIT_PATH}"
+    )
     testdir = Path(
         tempfile.mkdtemp(prefix=f"{system_test_name}_tmp_", dir=system_test_root)
     )
@@ -597,7 +573,6 @@ def system_test(  # pylint: disable=too-many-arguments,too-many-statements
             isctest.log.error("Found core dumps or sanitizer reports")
             pytest.fail(f"get_core_dumps.sh exited with {exc.returncode}")
 
-    os.environ.update(env)  # Ensure pytests have the same env vars as shell tests.
     isctest.log.info(f"test started: {request.node.name}")
     port = int(env["PORT"])
     isctest.log.info("using port range: <%d, %d>", port, port + PORTS_PER_TEST - 1)
diff --git a/bin/tests/system/isctest/__init__.py b/bin/tests/system/isctest/__init__.py
index e0014adbba..73ac413013 100644
--- a/bin/tests/system/isctest/__init__.py
+++ b/bin/tests/system/isctest/__init__.py
@@ -15,6 +15,7 @@ from . import query
 from . import rndc
 from . import run
 from . import log
+from . import vars  # pylint: disable=redefined-builtin
 
 # isctest.mark module is intentionally NOT imported, because it relies on
 # environment variables which might not be set at the time of import of the
diff --git a/bin/tests/system/isctest/check.py b/bin/tests/system/isctest/check.py
index e6fe020df3..2ef37ed05e 100644
--- a/bin/tests/system/isctest/check.py
+++ b/bin/tests/system/isctest/check.py
@@ -9,6 +9,7 @@
 # See the COPYRIGHT file distributed with this work for additional
 # information regarding copyright ownership.
 
+import shutil
 from typing import Any, Optional
 
 import dns.rcode
@@ -95,3 +96,8 @@ def zones_equal(
                 )
                 assert found_rdataset
                 assert found_rdataset.ttl == rdataset.ttl
+
+
+def is_executable(cmd: str, errmsg: str) -> None:
+    executable = shutil.which(cmd)
+    assert executable is not None, errmsg
diff --git a/bin/tests/system/isctest/vars/__init__.py b/bin/tests/system/isctest/vars/__init__.py
new file mode 100644
index 0000000000..b12df88f70
--- /dev/null
+++ b/bin/tests/system/isctest/vars/__init__.py
@@ -0,0 +1,12 @@
+# 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.
+
+from .all import ALL
diff --git a/bin/tests/system/isctest/vars/all.py b/bin/tests/system/isctest/vars/all.py
new file mode 100644
index 0000000000..7eaa48f3d4
--- /dev/null
+++ b/bin/tests/system/isctest/vars/all.py
@@ -0,0 +1,18 @@
+# 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=import-error
+from .autoconf import AC_VARS  # type: ignore
+
+# pylint: enable=import-error
+from .basic import BASIC_VARS
+
+ALL = {**AC_VARS, **BASIC_VARS}
diff --git a/bin/tests/system/isctest/vars/autoconf.py.in b/bin/tests/system/isctest/vars/autoconf.py.in
new file mode 100644
index 0000000000..5f1741bd73
--- /dev/null
+++ b/bin/tests/system/isctest/vars/autoconf.py.in
@@ -0,0 +1,23 @@
+# 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.
+
+AC_VARS = {
+    "TOP_BUILDDIR": "@abs_top_builddir@",
+    "TOP_SRCDIR": "@abs_top_srcdir@",
+    "FSTRM_CAPTURE": "@FSTRM_CAPTURE@",
+    "SHELL": "@SHELL@",
+    "PYTHON": "@PYTHON@",
+    "PERL": "@PERL@",
+    "CURL": "@CURL@",
+    "NC": "@NC@",
+    "XSLTPROC": "@XSLTPROC@",
+    "PYTEST": "@PYTEST@",
+}
diff --git a/bin/tests/system/isctest/vars/basic.py b/bin/tests/system/isctest/vars/basic.py
new file mode 100644
index 0000000000..0d4a9cfa51
--- /dev/null
+++ b/bin/tests/system/isctest/vars/basic.py
@@ -0,0 +1,64 @@
+# 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 os
+
+# pylint: disable=import-error
+from .autoconf import AC_VARS  # type: ignore
+
+# pylint: enable=import-error
+
+
+BASIC_VARS = {
+    "ARPANAME": f"{AC_VARS['TOP_BUILDDIR']}/bin/tools/arpaname",
+    "CDS": f"{AC_VARS['TOP_BUILDDIR']}/bin/dnssec/dnssec-cds",
+    "CHECKCONF": f"{AC_VARS['TOP_BUILDDIR']}/bin/check/named-checkconf",
+    "CHECKZONE": f"{AC_VARS['TOP_BUILDDIR']}/bin/check/named-checkzone",
+    "DIG": f"{AC_VARS['TOP_BUILDDIR']}/bin/dig/dig",
+    "DNSTAPREAD": f"{AC_VARS['TOP_BUILDDIR']}/bin/tools/dnstap-read",
+    "DSFROMKEY": f"{AC_VARS['TOP_BUILDDIR']}/bin/dnssec/dnssec-dsfromkey",
+    "FEATURETEST": f"{AC_VARS['TOP_BUILDDIR']}/bin/tests/system/feature-test",
+    "HOST": f"{AC_VARS['TOP_BUILDDIR']}/bin/dig/host",
+    "IMPORTKEY": f"{AC_VARS['TOP_BUILDDIR']}/bin/dnssec/dnssec-importkey",
+    "JOURNALPRINT": f"{AC_VARS['TOP_BUILDDIR']}/bin/tools/named-journalprint",
+    "KEYFRLAB": f"{AC_VARS['TOP_BUILDDIR']}/bin/dnssec/dnssec-keyfromlabel",
+    "KEYGEN": f"{AC_VARS['TOP_BUILDDIR']}/bin/dnssec/dnssec-keygen",
+    "KSR": f"{AC_VARS['TOP_BUILDDIR']}/bin/dnssec/dnssec-ksr",
+    "MDIG": f"{AC_VARS['TOP_BUILDDIR']}/bin/tools/mdig",
+    "NAMED": f"{AC_VARS['TOP_BUILDDIR']}/bin/named/named",
+    "NSEC3HASH": f"{AC_VARS['TOP_BUILDDIR']}/bin/tools/nsec3hash",
+    "NSLOOKUP": f"{AC_VARS['TOP_BUILDDIR']}/bin/dig/nslookup",
+    "NSUPDATE": f"{AC_VARS['TOP_BUILDDIR']}/bin/nsupdate/nsupdate",
+    "NZD2NZF": f"{AC_VARS['TOP_BUILDDIR']}/bin/tools/named-nzd2nzf",
+    "REVOKE": f"{AC_VARS['TOP_BUILDDIR']}/bin/dnssec/dnssec-revoke",
+    "RNDC": f"{AC_VARS['TOP_BUILDDIR']}/bin/rndc/rndc",
+    "RNDCCONFGEN": f"{AC_VARS['TOP_BUILDDIR']}/bin/confgen/rndc-confgen",
+    "RRCHECKER": f"{AC_VARS['TOP_BUILDDIR']}/bin/tools/named-rrchecker",
+    "SETTIME": f"{AC_VARS['TOP_BUILDDIR']}/bin/dnssec/dnssec-settime",
+    "SIGNER": f"{AC_VARS['TOP_BUILDDIR']}/bin/dnssec/dnssec-signzone",
+    "TSIGKEYGEN": f"{AC_VARS['TOP_BUILDDIR']}/bin/confgen/tsig-keygen",
+    "VERIFY": f"{AC_VARS['TOP_BUILDDIR']}/bin/dnssec/dnssec-verify",
+    "WIRETEST": f"{AC_VARS['TOP_BUILDDIR']}/bin/tests/wire_test",
+    "BIGKEY": f"{AC_VARS['TOP_BUILDDIR']}/bin/tests/system/rsabigexponent/bigkey",
+    "GENCHECK": f"{AC_VARS['TOP_BUILDDIR']}/bin/tests/system/rndc/gencheck",
+    "MAKEJOURNAL": f"{AC_VARS['TOP_BUILDDIR']}/bin/tests/system/makejournal",
+    "PIPEQUERIES": f"{AC_VARS['TOP_BUILDDIR']}/bin/tests/system/pipelined/pipequeries",
+    "TMPDIR": os.getenv("TMPDIR", "/tmp"),
+    "KRB5_CONFIG": "/dev/null",  # we don't want a KRB5_CONFIG setting breaking the tests
+    "KRB5_KTNAME": "dns.keytab",  # use local keytab instead of default /etc/krb5.keytab
+    "DELV": (
+        f"{AC_VARS['TOP_BUILDDIR']}/bin/delv/delv"
+        if not os.getenv("TSAN_OPTIONS", "")
+        else ":"  # workaround for GL#4119
+    ),
+    "LANG": "C",
+    "ANS_LOG_LEVEL": "debug",
+}
diff --git a/configure.ac b/configure.ac
index e586c818a9..31c965697c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1656,7 +1656,7 @@ AC_CONFIG_FILES([tests/unit-test-driver.sh],
 
 AC_CONFIG_FILES([bin/tests/Makefile
 		 bin/tests/system/Makefile
-		 bin/tests/system/conf.sh
+		 bin/tests/system/isctest/vars/autoconf.py
 		 bin/tests/system/dyndb/driver/Makefile
 		 bin/tests/system/dlzexternal/driver/Makefile
 		 bin/tests/system/hooks/driver/Makefile

From 8c6e6758b51394f49b1fda9e19601da8c0865ce3 Mon Sep 17 00:00:00 2001
From: Tom Krizek <tkrizek@isc.org>
Date: Thu, 29 Feb 2024 14:49:38 +0100
Subject: [PATCH 03/15] Load env vars in shell-only processing of conf.sh

While this isn't required for pytest operation and execution of the
system test suite, it can be handy to allow test script development and
debugging. Especially setup scripts often source conf.sh and expect
environment variables to be loaded. If these scripts are executed
stand-alone, the environment variables need to be loaded from the python
package.
---
 bin/tests/system/conf.sh             |  7 +++++++
 bin/tests/system/isctest/__main__.py | 17 +++++++++++++++++
 2 files changed, 24 insertions(+)
 create mode 100644 bin/tests/system/isctest/__main__.py

diff --git a/bin/tests/system/conf.sh b/bin/tests/system/conf.sh
index 131c72dbbb..ec65c6f412 100644
--- a/bin/tests/system/conf.sh
+++ b/bin/tests/system/conf.sh
@@ -11,6 +11,13 @@
 # See the COPYRIGHT file distributed with this work for additional
 # information regarding copyright ownership.
 
+# When sourcing the script outside the pytest environment (e.g. during helper
+# script development), the env variables have to be loaded.
+if [ -z "$TOP_SRCDIR" ]; then
+    SCRIPT_DIR=$(cd "$(dirname "$0")" && pwd | sed -E 's|(.*bin/tests/system).*|\1|')
+    eval "$(PYTHONPATH="$SCRIPT_DIR:$PYTHONPATH" /usr/bin/env python3 -m isctest)"
+fi
+
 testsock6() {
 	if test -n "$PERL" && $PERL -e "use IO::Socket::IP;" 2> /dev/null
 	then
diff --git a/bin/tests/system/isctest/__main__.py b/bin/tests/system/isctest/__main__.py
new file mode 100644
index 0000000000..2b82a81d46
--- /dev/null
+++ b/bin/tests/system/isctest/__main__.py
@@ -0,0 +1,17 @@
+# 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.
+
+from .vars import ALL
+
+
+if __name__ == "__main__":
+    for name, value in ALL.items():
+        print(f"export {name}={value}")

From 308a8727e5b11e0f7ffe772dab50cfb79f604bd5 Mon Sep 17 00:00:00 2001
From: Tom Krizek <tkrizek@isc.org>
Date: Wed, 10 Jan 2024 16:06:54 +0100
Subject: [PATCH 04/15] Format conf.sh with shfmt

No manual changes in this commit - simply running shfmt for proper shell
code formatting.
---
 bin/tests/system/conf.sh | 482 ++++++++++++++++++++-------------------
 1 file changed, 243 insertions(+), 239 deletions(-)

diff --git a/bin/tests/system/conf.sh b/bin/tests/system/conf.sh
index ec65c6f412..545919a5d9 100644
--- a/bin/tests/system/conf.sh
+++ b/bin/tests/system/conf.sh
@@ -14,84 +14,89 @@
 # When sourcing the script outside the pytest environment (e.g. during helper
 # script development), the env variables have to be loaded.
 if [ -z "$TOP_SRCDIR" ]; then
-    SCRIPT_DIR=$(cd "$(dirname "$0")" && pwd | sed -E 's|(.*bin/tests/system).*|\1|')
-    eval "$(PYTHONPATH="$SCRIPT_DIR:$PYTHONPATH" /usr/bin/env python3 -m isctest)"
+  SCRIPT_DIR=$(cd "$(dirname "$0")" && pwd | sed -E 's|(.*bin/tests/system).*|\1|')
+  eval "$(PYTHONPATH="$SCRIPT_DIR:$PYTHONPATH" /usr/bin/env python3 -m isctest)"
 fi
 
 testsock6() {
-	if test -n "$PERL" && $PERL -e "use IO::Socket::IP;" 2> /dev/null
-	then
-		$PERL "$TOP_SRCDIR/bin/tests/system/testsock6.pl" "$@"
-	else
-		false
-	fi
+  if test -n "$PERL" && $PERL -e "use IO::Socket::IP;" 2>/dev/null; then
+    $PERL "$TOP_SRCDIR/bin/tests/system/testsock6.pl" "$@"
+  else
+    false
+  fi
 }
 
-echofail () {
-    echo "$*"
+echofail() {
+  echo "$*"
 }
-echowarn () {
-    echo "$*"
+echowarn() {
+  echo "$*"
 }
-echopass () {
-    echo "$*"
+echopass() {
+  echo "$*"
 }
-echoinfo () {
-    echo "$*"
+echoinfo() {
+  echo "$*"
 }
-echostart () {
-    echo "$*"
+echostart() {
+  echo "$*"
 }
-echoend () {
-    echo "$*"
+echoend() {
+  echo "$*"
 }
 
 echo_i() {
-    echo "$@" | while IFS= read -r __LINE ; do
-       echoinfo "I:$__LINE"
-    done
+  echo "$@" | while IFS= read -r __LINE; do
+    echoinfo "I:$__LINE"
+  done
 }
 
 echo_ic() {
-    echo "$@" | while IFS= read -r __LINE ; do
-       echoinfo "I:  $__LINE"
-    done
+  echo "$@" | while IFS= read -r __LINE; do
+    echoinfo "I:  $__LINE"
+  done
 }
 
 echo_d() {
-    echo "$@" | while IFS= read -r __LINE ; do
-       echoinfo "D:$__LINE"
-    done
+  echo "$@" | while IFS= read -r __LINE; do
+    echoinfo "D:$__LINE"
+  done
 }
 
 cat_i() {
-    while IFS= read -r __LINE ; do
-       echoinfo "I:$__LINE"
-    done
+  while IFS= read -r __LINE; do
+    echoinfo "I:$__LINE"
+  done
 }
 
 cat_d() {
-    while IFS= read -r __LINE ; do
-       echoinfo "D:$__LINE"
-    done
+  while IFS= read -r __LINE; do
+    echoinfo "D:$__LINE"
+  done
 }
 
 digcomp() {
-    { output=$($PERL $TOP_SRCDIR/bin/tests/system/digcomp.pl "$@"); result=$?; } || true
-    [ -n "$output" ] &&  { echo "digcomp failed:"; echo "$output"; } | cat_i
-    return $result
+  {
+    output=$($PERL $TOP_SRCDIR/bin/tests/system/digcomp.pl "$@")
+    result=$?
+  } || true
+  [ -n "$output" ] && {
+    echo "digcomp failed:"
+    echo "$output"
+  } | cat_i
+  return $result
 }
 
 start_server() {
-    $PERL "$TOP_SRCDIR/bin/tests/system/start.pl" "$SYSTESTDIR" "$@"
+  $PERL "$TOP_SRCDIR/bin/tests/system/start.pl" "$SYSTESTDIR" "$@"
 }
 
 stop_server() {
-    $PERL "$TOP_SRCDIR/bin/tests/system/stop.pl" "$SYSTESTDIR" "$@"
+  $PERL "$TOP_SRCDIR/bin/tests/system/stop.pl" "$SYSTESTDIR" "$@"
 }
 
 send() {
-    $PERL "$TOP_SRCDIR/bin/tests/system/send.pl" "$@"
+  $PERL "$TOP_SRCDIR/bin/tests/system/send.pl" "$@"
 }
 
 #
@@ -140,94 +145,94 @@ export DEFAULT_HMAC=hmac-sha256
 # the error using the description of the tested variable provided in $3
 # and return 1.
 assert_int_equal() {
-	found="$1"
-	expected="$2"
-	description="$3"
+  found="$1"
+  expected="$2"
+  description="$3"
 
-	if [ "${expected}" -ne "${found}" ]; then
-		echo_i "incorrect ${description}: got ${found}, expected ${expected}"
-		return 1
-	fi
+  if [ "${expected}" -ne "${found}" ]; then
+    echo_i "incorrect ${description}: got ${found}, expected ${expected}"
+    return 1
+  fi
 
-	return 0
+  return 0
 }
 
 # keyfile_to_keys_section: helper function for keyfile_to_*_keys() which
 # converts keyfile data into a key-style trust anchor configuration
 # section using the supplied parameters
 keyfile_to_keys() {
-    section_name=$1
-    key_prefix=$2
-    shift
-    shift
-    echo "$section_name {"
-    for keyname in $*; do
-	awk '!/^; /{
+  section_name=$1
+  key_prefix=$2
+  shift
+  shift
+  echo "$section_name {"
+  for keyname in $*; do
+    awk '!/^; /{
 	    printf "\t\""$1"\" "
 	    printf "'"$key_prefix "'"
 	    printf $4 " " $5 " " $6 " \""
 	    for (i=7; i<=NF; i++) printf $i
 	    printf "\";\n"
 	}' $keyname.key
-    done
-    echo "};"
+  done
+  echo "};"
 }
 
 # keyfile_to_dskeys_section: helper function for keyfile_to_*_dskeys()
 # converts keyfile data into a DS-style trust anchor configuration
 # section using the supplied parameters
 keyfile_to_dskeys() {
-    section_name=$1
-    key_prefix=$2
-    shift
-    shift
-    echo "$section_name {"
-    for keyname in $*; do
-        $DSFROMKEY $keyname.key | \
-	awk '!/^; /{
+  section_name=$1
+  key_prefix=$2
+  shift
+  shift
+  echo "$section_name {"
+  for keyname in $*; do
+    $DSFROMKEY $keyname.key \
+      | awk '!/^; /{
 	    printf "\t\""$1"\" "
 	    printf "'"$key_prefix "'"
 	    printf $4 " " $5 " " $6 " \""
 	    for (i=7; i<=NF; i++) printf $i
 	    printf "\";\n"
 	}'
-    done
-    echo "};"
+  done
+  echo "};"
 }
 
 # keyfile_to_trusted_keys: convert key data contained in the keyfile(s)
 # provided to a "trust-keys" section suitable for including in a
 # resolver's configuration file
 keyfile_to_trusted_keys() {
-    keyfile_to_keys "trusted-keys" "" $*
+  keyfile_to_keys "trusted-keys" "" $*
 }
 
 # keyfile_to_static_keys: convert key data contained in the keyfile(s)
 # provided to a *static-key* "trust-anchors" section suitable for including in
 # a resolver's configuration file
 keyfile_to_static_keys() {
-    keyfile_to_keys "trust-anchors" "static-key" $*
+  keyfile_to_keys "trust-anchors" "static-key" $*
 }
 
 # keyfile_to_initial_keys: convert key data contained in the keyfile(s)
 # provided to an *initial-key* "trust-anchors" section suitable for including
 # in a resolver's configuration file
 keyfile_to_initial_keys() {
-    keyfile_to_keys "trust-anchors" "initial-key" $*
+  keyfile_to_keys "trust-anchors" "initial-key" $*
 }
 
 # keyfile_to_static_ds_keys: convert key data contained in the keyfile(s)
 # provided to a *static-ds* "trust-anchors" section suitable for including in a
 # resolver's configuration file
 keyfile_to_static_ds() {
-    keyfile_to_dskeys "trust-anchors" "static-ds" $*
+  keyfile_to_dskeys "trust-anchors" "static-ds" $*
 }
 
 # keyfile_to_initial_ds_keys: convert key data contained in the keyfile(s)
 # provided to an *initial-ds* "trust-anchors" section suitable for including
 # in a resolver's configuration file
 keyfile_to_initial_ds() {
-    keyfile_to_dskeys "trust-anchors" "initial-ds" $*
+  keyfile_to_dskeys "trust-anchors" "initial-ds" $*
 }
 
 # keyfile_to_key_id: convert a key file name to a key ID
@@ -236,7 +241,7 @@ keyfile_to_initial_ds() {
 # print the key ID with leading zeros stripped ("6160" for the
 # aforementioned example).
 keyfile_to_key_id() {
-	echo "$1" | sed "s/.*+0\{0,4\}//"
+  echo "$1" | sed "s/.*+0\{0,4\}//"
 }
 
 # private_type_record: write a private type record recording the state of the
@@ -246,13 +251,13 @@ keyfile_to_key_id() {
 # private type record with default type value of 65534, indicating that the
 # signing process for this key is completed.
 private_type_record() {
-        _zone=$1
-        _algorithm=$2
-        _keyfile=$3
+  _zone=$1
+  _algorithm=$2
+  _keyfile=$3
 
-        _id=$(keyfile_to_key_id "$_keyfile")
+  _id=$(keyfile_to_key_id "$_keyfile")
 
-        printf "%s. 0 IN TYPE65534 %s 5 %02x%04x0000\n" "$_zone" "\\#" "$_algorithm" "$_id"
+  printf "%s. 0 IN TYPE65534 %s 5 %02x%04x0000\n" "$_zone" "\\#" "$_algorithm" "$_id"
 }
 
 # nextpart*() - functions for reading files incrementally
@@ -303,51 +308,51 @@ private_type_record() {
 # nextpartreset: reset the marker used by nextpart() and nextpartpeek()
 # so that it points to the start of the given file
 nextpartreset() {
-    echo "0" > $1.prev
+  echo "0" >$1.prev
 }
 
 # nextpartread: read everything that's been appended to a file since the
 # last time nextpart() was called and print it to stdout, print the
 # total number of lines read from that file so far to file descriptor 3
 nextpartread() {
-    [ -f $1.prev ] || nextpartreset $1
-    prev=$(cat $1.prev)
-    awk "NR > $prev "'{ print }
+  [ -f $1.prev ] || nextpartreset $1
+  prev=$(cat $1.prev)
+  awk "NR > $prev "'{ print }
 	 END          { print NR > "/dev/stderr" }' $1 2>&3
 }
 
 # nextpart: read everything that's been appended to a file since the
 # last time nextpart() was called
 nextpart() {
-	nextpartread $1 3> $1.prev.tmp
-	mv $1.prev.tmp $1.prev
+  nextpartread $1 3>$1.prev.tmp
+  mv $1.prev.tmp $1.prev
 }
 
 # nextpartpeek: read everything that's been appended to a file since the
 # last time nextpart() was called
 nextpartpeek() {
-	nextpartread $1 3> /dev/null
+  nextpartread $1 3>/dev/null
 }
 
 # _search_log: look for message $1 in file $2 with nextpart().
 _search_log() (
-	msg="$1"
-	file="$2"
-	nextpart "$file" | grep -F -e "$msg" > /dev/null
+  msg="$1"
+  file="$2"
+  nextpart "$file" | grep -F -e "$msg" >/dev/null
 )
 
 # _search_log_re: same as _search_log but the message is an grep -E regex
 _search_log_re() (
-	msg="$1"
-	file="$2"
-	nextpart "$file" | grep -E -e "$msg" > /dev/null
+  msg="$1"
+  file="$2"
+  nextpart "$file" | grep -E -e "$msg" >/dev/null
 )
 
 # _search_log_peek: look for message $1 in file $2 with nextpartpeek().
 _search_log_peek() (
-	msg="$1"
-	file="$2"
-	nextpartpeek "$file" | grep -F -e "$msg" > /dev/null
+  msg="$1"
+  file="$2"
+  nextpartpeek "$file" | grep -F -e "$msg" >/dev/null
 )
 
 # wait_for_log: wait until message $2 in file $3 appears.  Bail out after
@@ -356,108 +361,108 @@ _search_log_peek() (
 # set correctly.  Tests using wait_for_log() are responsible for cleaning up
 # the created <file>.prev files.
 wait_for_log() (
-	timeout="$1"
-	msg="$2"
-	file="$3"
-	retry_quiet "$timeout" _search_log "$msg" "$file" && return 0
-	echo_i "exceeded time limit waiting for literal '$msg' in $file"
-        return 1
+  timeout="$1"
+  msg="$2"
+  file="$3"
+  retry_quiet "$timeout" _search_log "$msg" "$file" && return 0
+  echo_i "exceeded time limit waiting for literal '$msg' in $file"
+  return 1
 )
 
 # wait_for_log_re: same as wait_for_log, but the message is an grep -E regex
 wait_for_log_re() (
-	timeout="$1"
-	msg="$2"
-	file="$3"
-	retry_quiet "$timeout" _search_log_re "$msg" "$file" && return 0
-	echo_i "exceeded time limit waiting for regex '$msg' in $file"
-        return 1
+  timeout="$1"
+  msg="$2"
+  file="$3"
+  retry_quiet "$timeout" _search_log_re "$msg" "$file" && return 0
+  echo_i "exceeded time limit waiting for regex '$msg' in $file"
+  return 1
 )
 
 # wait_for_log_peek: similar to wait_for_log() but peeking, so the file offset
 # does not change.
 wait_for_log_peek() (
-	timeout="$1"
-	msg="$2"
-	file="$3"
-	retry_quiet "$timeout" _search_log_peek "$msg" "$file" && return 0
-	echo_i "exceeded time limit waiting for literal '$msg' in $file"
-        return 1
+  timeout="$1"
+  msg="$2"
+  file="$3"
+  retry_quiet "$timeout" _search_log_peek "$msg" "$file" && return 0
+  echo_i "exceeded time limit waiting for literal '$msg' in $file"
+  return 1
 )
 
 # _retry: keep running a command until it succeeds, up to $1 times, with
 # one-second intervals, optionally printing a message upon every attempt
 _retry() {
-	__retries="${1}"
-	shift
+  __retries="${1}"
+  shift
 
-	while :; do
-		if "$@"; then
-			return 0
-		fi
-		__retries=$((__retries-1))
-		if [ "${__retries}" -gt 0 ]; then
-			if [ "${__retry_quiet}" -ne 1 ]; then
-				echo_i "retrying"
-			fi
-			sleep 1
-		else
-			return 1
-		fi
-	done
+  while :; do
+    if "$@"; then
+      return 0
+    fi
+    __retries=$((__retries - 1))
+    if [ "${__retries}" -gt 0 ]; then
+      if [ "${__retry_quiet}" -ne 1 ]; then
+        echo_i "retrying"
+      fi
+      sleep 1
+    else
+      return 1
+    fi
+  done
 }
 
 # retry: call _retry() in verbose mode
 retry() {
-	__retry_quiet=0
-	_retry "$@"
+  __retry_quiet=0
+  _retry "$@"
 }
 
 # retry_quiet: call _retry() in silent mode
 retry_quiet() {
-	__retry_quiet=1
-	_retry "$@"
+  __retry_quiet=1
+  _retry "$@"
 }
 
 # _repeat: keep running command up to $1 times, unless it fails
 _repeat() (
-    __retries="${1}"
-    shift
-    while :; do
-	if ! "$@"; then
-	    return 1
-	fi
-	__retries=$((__retries-1))
-	if [ "${__retries}" -le 0 ]; then
-	    break
-	fi
-    done
-    return 0
+  __retries="${1}"
+  shift
+  while :; do
+    if ! "$@"; then
+      return 1
+    fi
+    __retries=$((__retries - 1))
+    if [ "${__retries}" -le 0 ]; then
+      break
+    fi
+  done
+  return 0
 )
 
 _times() {
-	awk "BEGIN{ for(i = 1; i <= $1; i++) print i}";
+  awk "BEGIN{ for(i = 1; i <= $1; i++) print i}"
 }
 
 rndc_reload() {
-    $RNDC -c ../_common/rndc.conf -s $2 -p ${CONTROLPORT} reload $3 2>&1 | sed 's/^/'"I:$1"' /'
-    # reloading single zone is synchronous, if we're reloading whole server
-    # we need to wait for reload to finish
-    if [ -z "$3" ]; then
-        for _ in $(_times 10); do
-            $RNDC -c ../_common/rndc.conf -s $2 -p ${CONTROLPORT} status | grep "reload/reconfig in progress" > /dev/null || break
-            sleep 1
-        done
-    fi
+  $RNDC -c ../_common/rndc.conf -s $2 -p ${CONTROLPORT} reload $3 2>&1 | sed 's/^/'"I:$1"' /'
+  # reloading single zone is synchronous, if we're reloading whole server
+  # we need to wait for reload to finish
+  if [ -z "$3" ]; then
+    for _ in $(_times 10); do
+      $RNDC -c ../_common/rndc.conf -s $2 -p ${CONTROLPORT} status | grep "reload/reconfig in progress" >/dev/null || break
+      sleep 1
+    done
+  fi
 }
 
 rndc_reconfig() {
-    seconds=${3:-10}
-    $RNDC -c ../_common/rndc.conf -s "$2" -p "${CONTROLPORT}" reconfig 2>&1 | sed 's/^/'"I:$1"' /'
-    for _ in $(_times "$seconds"); do
-        "$RNDC" -c ../_common/rndc.conf -s "$2" -p "${CONTROLPORT}" status | grep "reload/reconfig in progress" > /dev/null || break
-        sleep 1
-    done
+  seconds=${3:-10}
+  $RNDC -c ../_common/rndc.conf -s "$2" -p "${CONTROLPORT}" reconfig 2>&1 | sed 's/^/'"I:$1"' /'
+  for _ in $(_times "$seconds"); do
+    "$RNDC" -c ../_common/rndc.conf -s "$2" -p "${CONTROLPORT}" status | grep "reload/reconfig in progress" >/dev/null || break
+    sleep 1
+  done
 }
 
 # rndc_dumpdb: call "rndc dumpdb [...]" and wait until it completes
@@ -476,39 +481,38 @@ rndc_reconfig() {
 # code other than 0 or if the "; Dump complete" string does not appear in the
 # dump within 10 seconds.
 rndc_dumpdb() {
-	__ret=0
-	__dump_complete=0
-	__server="${1}"
-	__ip="10.53.0.$(echo "${__server}" | tr -c -d "0-9")"
+  __ret=0
+  __dump_complete=0
+  __server="${1}"
+  __ip="10.53.0.$(echo "${__server}" | tr -c -d "0-9")"
 
-	shift
-	${RNDC} -c ../_common/rndc.conf -p "${CONTROLPORT}" -s "${__ip}" dumpdb "$@" > "rndc.out.test${n}" 2>&1 || __ret=1
+  shift
+  ${RNDC} -c ../_common/rndc.conf -p "${CONTROLPORT}" -s "${__ip}" dumpdb "$@" >"rndc.out.test${n}" 2>&1 || __ret=1
 
-	for _ in 0 1 2 3 4 5 6 7 8 9
-	do
-		if grep '^; Dump complete$' "${__server}/named_dump.db" > /dev/null; then
-			mv "${__server}/named_dump.db" "${__server}/named_dump.db.test${n}"
-			__dump_complete=1
-			break
-		fi
-		sleep 1
-	done
+  for _ in 0 1 2 3 4 5 6 7 8 9; do
+    if grep '^; Dump complete$' "${__server}/named_dump.db" >/dev/null; then
+      mv "${__server}/named_dump.db" "${__server}/named_dump.db.test${n}"
+      __dump_complete=1
+      break
+    fi
+    sleep 1
+  done
 
-	if [ ${__dump_complete} -eq 0 ]; then
-		echo_i "timed out waiting for 'rndc dumpdb' to finish"
-		__ret=1
-	fi
+  if [ ${__dump_complete} -eq 0 ]; then
+    echo_i "timed out waiting for 'rndc dumpdb' to finish"
+    __ret=1
+  fi
 
-	return ${__ret}
+  return ${__ret}
 }
 
 # get_dig_xfer_stats: extract transfer statistics from dig output stored
 # in $1, converting them to a format used by some system tests.
 get_dig_xfer_stats() {
-	LOGFILE="$1"
-	sed -n "s/^;; XFR size: .*messages \([0-9][0-9]*\).*/messages=\1/p" "${LOGFILE}"
-	sed -n "s/^;; XFR size: \([0-9][0-9]*\) records.*/records=\1/p" "${LOGFILE}"
-	sed -n "s/^;; XFR size: .*bytes \([0-9][0-9]*\).*/bytes=\1/p" "${LOGFILE}"
+  LOGFILE="$1"
+  sed -n "s/^;; XFR size: .*messages \([0-9][0-9]*\).*/messages=\1/p" "${LOGFILE}"
+  sed -n "s/^;; XFR size: \([0-9][0-9]*\) records.*/records=\1/p" "${LOGFILE}"
+  sed -n "s/^;; XFR size: .*bytes \([0-9][0-9]*\).*/bytes=\1/p" "${LOGFILE}"
 }
 
 # get_named_xfer_stats: from named log file $1, extract transfer
@@ -516,16 +520,16 @@ get_dig_xfer_stats() {
 # message which has to contain the string provided in $4), converting
 # them to a format used by some system tests.
 get_named_xfer_stats() {
-	LOGFILE="$1"
-	PEER="$(echo $2 | sed 's/\./\\./g')"
-	ZONE="$(echo $3 | sed 's/\./\\./g')"
-	MESSAGE="$4"
-	grep " ${PEER}#.*${MESSAGE}:" "${LOGFILE}" | \
-		sed -n "s/.* '${ZONE}\/.* \([0-9][0-9]*\) messages.*/messages=\1/p" | tail -1
-	grep " ${PEER}#.*${MESSAGE}:" "${LOGFILE}" | \
-		sed -n "s/.* '${ZONE}\/.* \([0-9][0-9]*\) records.*/records=\1/p" | tail -1
-	grep " ${PEER}#.*${MESSAGE}:" "${LOGFILE}" | \
-		sed -n "s/.* '${ZONE}\/.* \([0-9][0-9]*\) bytes.*/bytes=\1/p" | tail -1
+  LOGFILE="$1"
+  PEER="$(echo $2 | sed 's/\./\\./g')"
+  ZONE="$(echo $3 | sed 's/\./\\./g')"
+  MESSAGE="$4"
+  grep " ${PEER}#.*${MESSAGE}:" "${LOGFILE}" \
+    | sed -n "s/.* '${ZONE}\/.* \([0-9][0-9]*\) messages.*/messages=\1/p" | tail -1
+  grep " ${PEER}#.*${MESSAGE}:" "${LOGFILE}" \
+    | sed -n "s/.* '${ZONE}\/.* \([0-9][0-9]*\) records.*/records=\1/p" | tail -1
+  grep " ${PEER}#.*${MESSAGE}:" "${LOGFILE}" \
+    | sed -n "s/.* '${ZONE}\/.* \([0-9][0-9]*\) bytes.*/bytes=\1/p" | tail -1
 }
 
 # copy_setports - Copy Configuration File and Replace Ports
@@ -539,57 +543,57 @@ get_named_xfer_stats() {
 #   copy_setports infile outfile
 #
 copy_setports() {
-    dir=$(echo "$TMPDIR" | sed 's/\//\\\//g')
+  dir=$(echo "$TMPDIR" | sed 's/\//\\\//g')
 
-    sed -e "s/@TMPDIR@/${dir}/g" \
-        -e "s/@PORT@/${PORT}/g" \
-        -e "s/@TLSPORT@/${TLSPORT}/g" \
-        -e "s/@HTTPPORT@/${HTTPPORT}/g" \
-        -e "s/@HTTPSPORT@/${HTTPSPORT}/g" \
-        -e "s/@EXTRAPORT1@/${EXTRAPORT1}/g" \
-        -e "s/@EXTRAPORT2@/${EXTRAPORT2}/g" \
-        -e "s/@EXTRAPORT3@/${EXTRAPORT3}/g" \
-        -e "s/@EXTRAPORT4@/${EXTRAPORT4}/g" \
-        -e "s/@EXTRAPORT5@/${EXTRAPORT5}/g" \
-        -e "s/@EXTRAPORT6@/${EXTRAPORT6}/g" \
-        -e "s/@EXTRAPORT7@/${EXTRAPORT7}/g" \
-        -e "s/@EXTRAPORT8@/${EXTRAPORT8}/g" \
-        -e "s/@CONTROLPORT@/${CONTROLPORT}/g" \
-        -e "s/@DEFAULT_ALGORITHM@/${DEFAULT_ALGORITHM}/g" \
-        -e "s/@DEFAULT_ALGORITHM_NUMBER@/${DEFAULT_ALGORITHM_NUMBER}/g" \
-        -e "s/@DEFAULT_BITS@/${DEFAULT_BITS}/g" \
-        -e "s/@ALTERNATIVE_ALGORITHM@/${ALTERNATIVE_ALGORITHM}/g" \
-        -e "s/@ALTERNATIVE_ALGORITHM_NUMBER@/${ALTERNATIVE_ALGORITHM_NUMBER}/g" \
-        -e "s/@ALTERNATIVE_BITS@/${ALTERNATIVE_BITS}/g" \
-        -e "s/@DEFAULT_HMAC@/${DEFAULT_HMAC}/g" \
-        -e "s/@DISABLED_ALGORITHM@/${DISABLED_ALGORITHM}/g" \
-        -e "s/@DISABLED_ALGORITHM_NUMBER@/${DISABLED_ALGORITHM_NUMBER}/g" \
-        -e "s/@DISABLED_BITS@/${DISABLED_BITS}/g" \
-        $1 > $2
+  sed -e "s/@TMPDIR@/${dir}/g" \
+    -e "s/@PORT@/${PORT}/g" \
+    -e "s/@TLSPORT@/${TLSPORT}/g" \
+    -e "s/@HTTPPORT@/${HTTPPORT}/g" \
+    -e "s/@HTTPSPORT@/${HTTPSPORT}/g" \
+    -e "s/@EXTRAPORT1@/${EXTRAPORT1}/g" \
+    -e "s/@EXTRAPORT2@/${EXTRAPORT2}/g" \
+    -e "s/@EXTRAPORT3@/${EXTRAPORT3}/g" \
+    -e "s/@EXTRAPORT4@/${EXTRAPORT4}/g" \
+    -e "s/@EXTRAPORT5@/${EXTRAPORT5}/g" \
+    -e "s/@EXTRAPORT6@/${EXTRAPORT6}/g" \
+    -e "s/@EXTRAPORT7@/${EXTRAPORT7}/g" \
+    -e "s/@EXTRAPORT8@/${EXTRAPORT8}/g" \
+    -e "s/@CONTROLPORT@/${CONTROLPORT}/g" \
+    -e "s/@DEFAULT_ALGORITHM@/${DEFAULT_ALGORITHM}/g" \
+    -e "s/@DEFAULT_ALGORITHM_NUMBER@/${DEFAULT_ALGORITHM_NUMBER}/g" \
+    -e "s/@DEFAULT_BITS@/${DEFAULT_BITS}/g" \
+    -e "s/@ALTERNATIVE_ALGORITHM@/${ALTERNATIVE_ALGORITHM}/g" \
+    -e "s/@ALTERNATIVE_ALGORITHM_NUMBER@/${ALTERNATIVE_ALGORITHM_NUMBER}/g" \
+    -e "s/@ALTERNATIVE_BITS@/${ALTERNATIVE_BITS}/g" \
+    -e "s/@DEFAULT_HMAC@/${DEFAULT_HMAC}/g" \
+    -e "s/@DISABLED_ALGORITHM@/${DISABLED_ALGORITHM}/g" \
+    -e "s/@DISABLED_ALGORITHM_NUMBER@/${DISABLED_ALGORITHM_NUMBER}/g" \
+    -e "s/@DISABLED_BITS@/${DISABLED_BITS}/g" \
+    $1 >$2
 }
 
 # parse_openssl_config - Parse OpenSSL configuration for HSM settings
 #
 # Will set SOFTHSM2_MODULE, OPENSSL_ENGINE and ENGINE_ARG based on openssl configuration.
 parse_openssl_config() {
-    ENGINE_ARG=""
-    [ -f "$OPENSSL_CONF" ] || return 0
-    while IFS="=" read key val; do
-        # trim variables
-        key="${key## }"
-        key="${key%% }"
-        val="${val## }"
-        val="${val%% }"
-        case "$key" in
-        "engine_id")
-            OPENSSL_ENGINE="$val"
-            ENGINE_ARG="-E $OPENSSL_ENGINE"
-            ;;
-        "MODULE_PATH"|"pkcs11-module-path")
-            SOFTHSM2_MODULE="$val"
-            ;;
-        esac
-    done < "$OPENSSL_CONF"
+  ENGINE_ARG=""
+  [ -f "$OPENSSL_CONF" ] || return 0
+  while IFS="=" read key val; do
+    # trim variables
+    key="${key## }"
+    key="${key%% }"
+    val="${val## }"
+    val="${val%% }"
+    case "$key" in
+      "engine_id")
+        OPENSSL_ENGINE="$val"
+        ENGINE_ARG="-E $OPENSSL_ENGINE"
+        ;;
+      "MODULE_PATH" | "pkcs11-module-path")
+        SOFTHSM2_MODULE="$val"
+        ;;
+    esac
+  done <"$OPENSSL_CONF"
 }
 
 grep_v() { grep -v "$@" || test $? = 1; }

From 46433ae17bc8384ad23b2ea926a5680efe6da88a Mon Sep 17 00:00:00 2001
From: Tom Krizek <tkrizek@isc.org>
Date: Thu, 4 Jan 2024 14:48:04 +0100
Subject: [PATCH 05/15] Coalesce system test variables

Provide a single point of access to all the variables used by tests. Use
a custom dict-like structure to access the underlying data without
making a copy. This allows the individual modules to update the contents
at runtime, which is used for some variables.
---
 bin/tests/system/conftest.py         |  3 ++-
 bin/tests/system/isctest/vars/all.py | 35 +++++++++++++++++++++++++++-
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/bin/tests/system/conftest.py b/bin/tests/system/conftest.py
index 109618e1aa..75d665d9fb 100644
--- a/bin/tests/system/conftest.py
+++ b/bin/tests/system/conftest.py
@@ -276,11 +276,12 @@ def control_port(ports):
 @pytest.fixture(scope="module")
 def env(ports):
     """Dictionary containing environment variables for the test."""
-    env = os.environ.copy()
+    env = dict(isctest.vars.ALL)
     for portname, portnum in ports.items():
         env[portname] = str(portnum)
     env["builddir"] = f"{env['TOP_BUILDDIR']}/{SYSTEM_TEST_DIR_GIT_PATH}"
     env["srcdir"] = f"{env['TOP_SRCDIR']}/{SYSTEM_TEST_DIR_GIT_PATH}"
+    os.environ.update(env)
     return env
 
 
diff --git a/bin/tests/system/isctest/vars/all.py b/bin/tests/system/isctest/vars/all.py
index 7eaa48f3d4..58e1689af2 100644
--- a/bin/tests/system/isctest/vars/all.py
+++ b/bin/tests/system/isctest/vars/all.py
@@ -9,10 +9,43 @@
 # See the COPYRIGHT file distributed with this work for additional
 # information regarding copyright ownership.
 
+from collections import ChainMap
+
 # pylint: disable=import-error
 from .autoconf import AC_VARS  # type: ignore
 
 # pylint: enable=import-error
 from .basic import BASIC_VARS
 
-ALL = {**AC_VARS, **BASIC_VARS}
+
+class VarLookup(ChainMap):
+    """A dictionary-like structure to coalesce the variables from different
+    modules without making a copy (which would prevent updating these values
+    from inside the modules)."""
+
+    def __init__(self, *maps):
+        keys = set()
+        for m in maps:
+            overlap = keys.intersection(m.keys())
+            if overlap:
+                raise RuntimeError(f"key(s) are defined multiple times: {overlap}")
+            keys = keys.union(m.keys())
+        super().__init__(*maps)
+
+    def __setitem__(self, *args, **kwargs):
+        raise RuntimeError("read-only structure")
+
+    def keys(self):
+        result = set()
+        for m in self.maps:
+            for key, val in m.items():
+                if val is None:  # treat None as unset
+                    continue
+                result.add(key)
+        return list(result)
+
+    def __iter__(self):
+        return iter(self.keys())
+
+
+ALL = VarLookup(AC_VARS, BASIC_VARS)

From e531bfc3b388512ffb1b1831511be3360adf378b Mon Sep 17 00:00:00 2001
From: Tom Krizek <tkrizek@isc.org>
Date: Wed, 3 Jan 2024 14:08:58 +0100
Subject: [PATCH 06/15] Adjust .gitlab-ci env var detection with sed

The environment variables set by autoconf were moved to autoconf.py.
---
 .gitlab-ci.yml | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 058b94f1b1..3cb73e0d2e 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -271,11 +271,11 @@ stages:
 
 # change directory to the workspace before including this
 .find_python: &find_python
-  - PYTHON="$(source bin/tests/system/conf.sh; echo $PYTHON)"
+  - PYTHON="$(sed -n -E 's|^[[:space:]]*"PYTHON":[[:space:]]*"([^"]*)",[[:space:]]*$|\1|p' bin/tests/system/isctest/vars/autoconf.py)"
   - test -x "$PYTHON"
 
 .find_pytest: &find_pytest
-  - PYTEST="$(source bin/tests/system/conf.sh; echo $PYTEST)"
+  - PYTEST="$(sed -n -E 's|^[[:space:]]*"PYTEST":[[:space:]]*"([^"]*)",[[:space:]]*$|\1|p' bin/tests/system/isctest/vars/autoconf.py)"
   - test -x "$PYTEST"
 
 .parse_tsan: &parse_tsan
@@ -702,7 +702,8 @@ cross-version-config-tests:
     # Run the setup phase of all system tests in the most recently tagged BIND 9
     # release using the binaries built for the current BIND 9 version.  This
     # intends to detect obvious backward compatibility issues with the latter.
-    - sed -i -E "s|(export TOP_BUILDDIR)=.*|\1=${CI_PROJECT_DIR}|" conf.sh
+    - >
+      sed -i -E "s|(\s* \"TOP_BUILDDIR\"):.*|\1: \"${CI_PROJECT_DIR}\",|" isctest/vars/autoconf.py
     - >
       "$PYTEST" --setup-only --junit-xml="$CI_PROJECT_DIR"/junit.xml -n "${TEST_PARALLEL_JOBS:-1}"
   needs:

From b100ce4c88ea7f149e93d65bbe4b5e99e1c877b5 Mon Sep 17 00:00:00 2001
From: Tom Krizek <tkrizek@isc.org>
Date: Mon, 26 Feb 2024 13:52:55 +0100
Subject: [PATCH 07/15] Parse openssl-related vars in pytest

The openssl config needs to be parsed for some tests that use SoftHSM2.
Rewrite the parsing to python and ensure the required variables are
properly set test-wide.
---
 bin/tests/system/conf.sh                 | 24 ------------
 bin/tests/system/enginepkcs11/prereq.sh  |  1 -
 bin/tests/system/enginepkcs11/setup.sh   |  1 -
 bin/tests/system/enginepkcs11/tests.sh   |  1 -
 bin/tests/system/isctest/vars/all.py     |  3 +-
 bin/tests/system/isctest/vars/openssl.py | 49 ++++++++++++++++++++++++
 bin/tests/system/keyfromlabel/prereq.sh  |  1 -
 bin/tests/system/keyfromlabel/tests.sh   |  1 -
 8 files changed, 51 insertions(+), 30 deletions(-)
 create mode 100644 bin/tests/system/isctest/vars/openssl.py

diff --git a/bin/tests/system/conf.sh b/bin/tests/system/conf.sh
index 545919a5d9..e3e9a08fb9 100644
--- a/bin/tests/system/conf.sh
+++ b/bin/tests/system/conf.sh
@@ -572,28 +572,4 @@ copy_setports() {
     $1 >$2
 }
 
-# parse_openssl_config - Parse OpenSSL configuration for HSM settings
-#
-# Will set SOFTHSM2_MODULE, OPENSSL_ENGINE and ENGINE_ARG based on openssl configuration.
-parse_openssl_config() {
-  ENGINE_ARG=""
-  [ -f "$OPENSSL_CONF" ] || return 0
-  while IFS="=" read key val; do
-    # trim variables
-    key="${key## }"
-    key="${key%% }"
-    val="${val## }"
-    val="${val%% }"
-    case "$key" in
-      "engine_id")
-        OPENSSL_ENGINE="$val"
-        ENGINE_ARG="-E $OPENSSL_ENGINE"
-        ;;
-      "MODULE_PATH" | "pkcs11-module-path")
-        SOFTHSM2_MODULE="$val"
-        ;;
-    esac
-  done <"$OPENSSL_CONF"
-}
-
 grep_v() { grep -v "$@" || test $? = 1; }
diff --git a/bin/tests/system/enginepkcs11/prereq.sh b/bin/tests/system/enginepkcs11/prereq.sh
index 4eb2788a62..335b348a63 100644
--- a/bin/tests/system/enginepkcs11/prereq.sh
+++ b/bin/tests/system/enginepkcs11/prereq.sh
@@ -23,7 +23,6 @@
   exit 255
 }
 
-parse_openssl_config
 [ -f "$SOFTHSM2_MODULE" ] || {
   echo_i "skip: softhsm2 module not available"
   exit 1
diff --git a/bin/tests/system/enginepkcs11/setup.sh b/bin/tests/system/enginepkcs11/setup.sh
index bf140f1895..51d59dd854 100644
--- a/bin/tests/system/enginepkcs11/setup.sh
+++ b/bin/tests/system/enginepkcs11/setup.sh
@@ -20,7 +20,6 @@ $SHELL clean.sh
 
 OPENSSL_CONF= softhsm2-util --init-token --free --pin 1234 --so-pin 1234 --label "softhsm2-enginepkcs11" | awk '/^The token has been initialized and is reassigned to slot/ { print $NF }'
 
-parse_openssl_config
 printf '%s' "${HSMPIN:-1234}" >ns1/pin
 PWD=$(pwd)
 
diff --git a/bin/tests/system/enginepkcs11/tests.sh b/bin/tests/system/enginepkcs11/tests.sh
index 9db388f22b..7b0c1072bf 100644
--- a/bin/tests/system/enginepkcs11/tests.sh
+++ b/bin/tests/system/enginepkcs11/tests.sh
@@ -16,7 +16,6 @@ set -e
 # shellcheck source=conf.sh
 . ../conf.sh
 
-parse_openssl_config
 PWD=$(pwd)
 
 status=0
diff --git a/bin/tests/system/isctest/vars/all.py b/bin/tests/system/isctest/vars/all.py
index 58e1689af2..2126c1c220 100644
--- a/bin/tests/system/isctest/vars/all.py
+++ b/bin/tests/system/isctest/vars/all.py
@@ -16,6 +16,7 @@ from .autoconf import AC_VARS  # type: ignore
 
 # pylint: enable=import-error
 from .basic import BASIC_VARS
+from .openssl import OPENSSL_VARS
 
 
 class VarLookup(ChainMap):
@@ -48,4 +49,4 @@ class VarLookup(ChainMap):
         return iter(self.keys())
 
 
-ALL = VarLookup(AC_VARS, BASIC_VARS)
+ALL = VarLookup(AC_VARS, BASIC_VARS, OPENSSL_VARS)
diff --git a/bin/tests/system/isctest/vars/openssl.py b/bin/tests/system/isctest/vars/openssl.py
new file mode 100644
index 0000000000..1dcef67faf
--- /dev/null
+++ b/bin/tests/system/isctest/vars/openssl.py
@@ -0,0 +1,49 @@
+# 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 os
+import re
+
+from .. import log
+
+
+OPENSSL_VARS = {
+    "OPENSSL_CONF": os.getenv("OPENSSL_CONF", ""),
+    "SOFTHSM2_CONF": os.getenv("SOFTHSM2_CONF", ""),
+    "SOFTHSM2_MODULE": "",
+    "ENGINE_ARG": "",
+}
+
+
+def parse_openssl_config(path: str):
+    if not os.path.isfile(path):
+        return
+    regex = re.compile(r"([^=]+)=(.*)")
+    log.debug(f"parsing openssl config: {path}")
+    with open(path, "r", encoding="utf-8") as conf:
+        for line in conf:
+            res = regex.match(line)
+            if res:
+                key = res.group(1).strip()
+                val = res.group(2).strip()
+                if key == "engine_id":
+                    OPENSSL_VARS["ENGINE_ARG"] = f"-E {val}"
+                    os.environ["ENGINE_ARG"] = f"-E {val}"
+                    log.debug("ENGINE_ARG set to {OPENSSL_VARS['ENGINE_ARG']}")
+                elif key in ["MODULE_PATH", "pkcs11-module-path"]:
+                    OPENSSL_VARS["SOFTHSM2_MODULE"] = val
+                    os.environ["SOFTHSM2_MODULE"] = val
+                    log.debug(
+                        "SOFTHSM2_MODULE set to {OPENSSL_VARS['SOFTHSM2_MODULE']}"
+                    )
+
+
+parse_openssl_config(OPENSSL_VARS["OPENSSL_CONF"])
diff --git a/bin/tests/system/keyfromlabel/prereq.sh b/bin/tests/system/keyfromlabel/prereq.sh
index c6caa0dc88..be1850a1fa 100644
--- a/bin/tests/system/keyfromlabel/prereq.sh
+++ b/bin/tests/system/keyfromlabel/prereq.sh
@@ -18,7 +18,6 @@
   exit 255
 }
 
-parse_openssl_config
 [ -f "$SOFTHSM2_MODULE" ] || {
   echo_i "skip: softhsm2 module not available"
   exit 1
diff --git a/bin/tests/system/keyfromlabel/tests.sh b/bin/tests/system/keyfromlabel/tests.sh
index 2f818c5d77..f29f327098 100644
--- a/bin/tests/system/keyfromlabel/tests.sh
+++ b/bin/tests/system/keyfromlabel/tests.sh
@@ -16,7 +16,6 @@ set -e
 # shellcheck source=conf.sh
 . ../conf.sh
 
-parse_openssl_config
 PWD=$(pwd)
 
 keygen() {

From cca26efe52a02134c0f7b7f06ef770055efe2979 Mon Sep 17 00:00:00 2001
From: Tom Krizek <tkrizek@isc.org>
Date: Wed, 28 Feb 2024 10:44:58 +0100
Subject: [PATCH 08/15] Don't export openssl-related env vars unless set

If OPENSSL_CONF is exported as an empty string, it will cause issues on
rhel9fips. Allow the environment variables to be set and exported, but
make sure to only export them if they have been set by the user.
---
 bin/tests/system/isctest/vars/all.py     |  3 ++-
 bin/tests/system/isctest/vars/openssl.py | 18 ++++++++++++------
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/bin/tests/system/isctest/vars/all.py b/bin/tests/system/isctest/vars/all.py
index 2126c1c220..3c2bc9251b 100644
--- a/bin/tests/system/isctest/vars/all.py
+++ b/bin/tests/system/isctest/vars/all.py
@@ -22,7 +22,8 @@ from .openssl import OPENSSL_VARS
 class VarLookup(ChainMap):
     """A dictionary-like structure to coalesce the variables from different
     modules without making a copy (which would prevent updating these values
-    from inside the modules)."""
+    from inside the modules). Values which are None are treated as unset when
+    iterating."""
 
     def __init__(self, *maps):
         keys = set()
diff --git a/bin/tests/system/isctest/vars/openssl.py b/bin/tests/system/isctest/vars/openssl.py
index 1dcef67faf..5659222c09 100644
--- a/bin/tests/system/isctest/vars/openssl.py
+++ b/bin/tests/system/isctest/vars/openssl.py
@@ -11,21 +11,27 @@
 
 import os
 import re
+from typing import Optional
 
 from .. import log
 
 
 OPENSSL_VARS = {
-    "OPENSSL_CONF": os.getenv("OPENSSL_CONF", ""),
-    "SOFTHSM2_CONF": os.getenv("SOFTHSM2_CONF", ""),
-    "SOFTHSM2_MODULE": "",
-    "ENGINE_ARG": "",
+    "OPENSSL_CONF": os.getenv("OPENSSL_CONF", None),
+    "SOFTHSM2_CONF": os.getenv("SOFTHSM2_CONF", None),
+    "SOFTHSM2_MODULE": None,
+    "ENGINE_ARG": None,
 }
 
 
-def parse_openssl_config(path: str):
-    if not os.path.isfile(path):
+def parse_openssl_config(path: Optional[str]):
+    if path is None or not os.path.isfile(path):
+        OPENSSL_VARS["ENGINE_ARG"] = None
+        OPENSSL_VARS["SOFTHSM2_MODULE"] = None
+        os.environ.pop("ENGINE_ARG", None)
+        os.environ.pop("SOFTHSM2_MODULE", None)
         return
+
     regex = re.compile(r"([^=]+)=(.*)")
     log.debug(f"parsing openssl config: {path}")
     with open(path, "r", encoding="utf-8") as conf:

From 41cb553bdd0c2cf0330efaef30264a9b06ee0cfe Mon Sep 17 00:00:00 2001
From: Tom Krizek <tkrizek@isc.org>
Date: Wed, 28 Feb 2024 16:29:13 +0100
Subject: [PATCH 09/15] Move port env vars into isctest.vars.ports module

The fixture for port assignment isn't needed, replace it with the common
way of handling environment variables.
---
 bin/tests/system/conftest.py                | 69 +++++++--------------
 bin/tests/system/isctest/instance.py        | 14 ++++-
 bin/tests/system/isctest/vars/all.py        |  3 +-
 bin/tests/system/isctest/vars/ports.py      | 54 ++++++++++++++++
 bin/tests/system/shutdown/tests_shutdown.py |  6 +-
 bin/tests/system/statschannel/conftest.py   |  6 +-
 6 files changed, 98 insertions(+), 54 deletions(-)
 create mode 100644 bin/tests/system/isctest/vars/ports.py

diff --git a/bin/tests/system/conftest.py b/bin/tests/system/conftest.py
index 75d665d9fb..af178aec89 100644
--- a/bin/tests/system/conftest.py
+++ b/bin/tests/system/conftest.py
@@ -52,9 +52,6 @@ else:
 XDIST_WORKER = os.environ.get("PYTEST_XDIST_WORKER", "")
 FILE_DIR = os.path.abspath(Path(__file__).parent)
 ENV_RE = re.compile(b"([^=]+)=(.*)")
-PORT_MIN = 5001
-PORT_MAX = 32767
-PORTS_PER_TEST = 20
 PRIORITY_TESTS = [
     # Tests that are scheduled first. Speeds up parallel execution.
     "rpz/",
@@ -210,8 +207,10 @@ def module_base_ports(modules):
     exactly what happens - every worker thread will call this fixture to
     determine test ports.
     """
-    port_min = PORT_MIN
-    port_max = PORT_MAX - len(modules) * PORTS_PER_TEST
+    port_min = isctest.vars.ports.PORT_MIN
+    port_max = (
+        isctest.vars.ports.PORT_MAX - len(modules) * isctest.vars.ports.PORTS_PER_TEST
+    )
     if port_max < port_min:
         raise RuntimeError("not enough ports to assign unique port set to each module")
 
@@ -223,62 +222,44 @@ def module_base_ports(modules):
     # be misleading.
     base_port = int(time.time() // 3600) % (port_max - port_min) + port_min
 
-    return {mod: base_port + i * PORTS_PER_TEST for i, mod in enumerate(modules)}
+    return {
+        mod: base_port + i * isctest.vars.ports.PORTS_PER_TEST
+        for i, mod in enumerate(modules)
+    }
 
 
-@pytest.fixture(scope="module")
+@pytest.fixture(autouse=True, scope="module")
 def base_port(request, module_base_ports):
     """Start of the port range assigned to a particular test module."""
     port = module_base_ports[request.fspath]
+    isctest.vars.ports.set_base_port(port)
     return port
 
 
 @pytest.fixture(scope="module")
-def ports(base_port):
-    """Dictionary containing port names and their assigned values."""
-    return {
-        "PORT": base_port,
-        "TLSPORT": base_port + 1,
-        "HTTPPORT": base_port + 2,
-        "HTTPSPORT": base_port + 3,
-        "EXTRAPORT1": base_port + 4,
-        "EXTRAPORT2": base_port + 5,
-        "EXTRAPORT3": base_port + 6,
-        "EXTRAPORT4": base_port + 7,
-        "EXTRAPORT5": base_port + 8,
-        "EXTRAPORT6": base_port + 9,
-        "EXTRAPORT7": base_port + 10,
-        "EXTRAPORT8": base_port + 11,
-        "CONTROLPORT": base_port + 12,
-    }
+def named_port():
+    return int(os.environ["PORT"])
 
 
 @pytest.fixture(scope="module")
-def named_port(ports):
-    return ports["PORT"]
+def named_tlsport():
+    return int(os.environ["TLSPORT"])
 
 
 @pytest.fixture(scope="module")
-def named_tlsport(ports):
-    return ports["TLSPORT"]
+def named_httpsport():
+    return int(os.environ["HTTPSPORT"])
 
 
 @pytest.fixture(scope="module")
-def named_httpsport(ports):
-    return ports["HTTPSPORT"]
+def control_port():
+    return int(os.environ["CONTROLPORT"])
 
 
 @pytest.fixture(scope="module")
-def control_port(ports):
-    return ports["CONTROLPORT"]
-
-
-@pytest.fixture(scope="module")
-def env(ports):
+def env():
     """Dictionary containing environment variables for the test."""
     env = dict(isctest.vars.ALL)
-    for portname, portnum in ports.items():
-        env[portname] = str(portnum)
     env["builddir"] = f"{env['TOP_BUILDDIR']}/{SYSTEM_TEST_DIR_GIT_PATH}"
     env["srcdir"] = f"{env['TOP_SRCDIR']}/{SYSTEM_TEST_DIR_GIT_PATH}"
     os.environ.update(env)
@@ -576,7 +557,9 @@ def system_test(  # pylint: disable=too-many-arguments,too-many-statements
 
     isctest.log.info(f"test started: {request.node.name}")
     port = int(env["PORT"])
-    isctest.log.info("using port range: <%d, %d>", port, port + PORTS_PER_TEST - 1)
+    isctest.log.info(
+        "using port range: <%d, %d>", port, port + isctest.vars.ports.PORTS_PER_TEST - 1
+    )
 
     if not hasattr(request.node, "stash"):  # compatibility with pytest<7.0.0
         request.node.stash = {}  # use regular dict instead of pytest.Stash
@@ -604,17 +587,13 @@ def system_test(  # pylint: disable=too-many-arguments,too-many-statements
 
 
 @pytest.fixture
-def servers(ports, system_test_dir):
+def servers(system_test_dir):
     instances = {}
     for entry in system_test_dir.rglob("*"):
         if entry.is_dir():
             try:
                 dir_name = entry.name
-                # LATER: Make ports fixture return NamedPorts directly
-                named_ports = isctest.instance.NamedPorts(
-                    dns=int(ports["PORT"]), rndc=int(ports["CONTROLPORT"])
-                )
-                instance = isctest.instance.NamedInstance(dir_name, named_ports)
+                instance = isctest.instance.NamedInstance(dir_name)
                 instances[dir_name] = instance
             except ValueError:
                 continue
diff --git a/bin/tests/system/isctest/instance.py b/bin/tests/system/isctest/instance.py
index 9db27621d6..e0e612fda0 100644
--- a/bin/tests/system/isctest/instance.py
+++ b/bin/tests/system/isctest/instance.py
@@ -25,6 +25,13 @@ class NamedPorts(NamedTuple):
     dns: int = 53
     rndc: int = 953
 
+    @staticmethod
+    def from_env():
+        return NamedPorts(
+            dns=int(os.environ["PORT"]),
+            rndc=int(os.environ["CONTROLPORT"]),
+        )
+
 
 class NamedInstance:
     """
@@ -42,7 +49,7 @@ class NamedInstance:
     def __init__(
         self,
         identifier: str,
-        ports: NamedPorts = NamedPorts(),
+        ports: Optional[NamedPorts] = None,
         rndc_logger: Optional[logging.Logger] = None,
         rndc_executor: Optional[RNDCExecutor] = None,
     ) -> None:
@@ -52,7 +59,8 @@ class NamedInstance:
 
         `ports` is the `NamedPorts` instance listing the UDP/TCP ports on which
         this `named` instance is listening for various types of traffic (both
-        DNS traffic and RNDC commands).
+        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.
@@ -61,6 +69,8 @@ class NamedInstance:
         that is used for executing RNDC commands on this `named` instance.
         """
         self.ip = self._identifier_to_ip(identifier)
+        if ports is None:
+            ports = NamedPorts.from_env()
         self.ports = ports
         self.log = LogFile(os.path.join(identifier, "named.run"))
         self._rndc_executor = rndc_executor or RNDCBinaryExecutor()
diff --git a/bin/tests/system/isctest/vars/all.py b/bin/tests/system/isctest/vars/all.py
index 3c2bc9251b..cc2a7f8836 100644
--- a/bin/tests/system/isctest/vars/all.py
+++ b/bin/tests/system/isctest/vars/all.py
@@ -17,6 +17,7 @@ from .autoconf import AC_VARS  # type: ignore
 # pylint: enable=import-error
 from .basic import BASIC_VARS
 from .openssl import OPENSSL_VARS
+from .ports import PORT_VARS
 
 
 class VarLookup(ChainMap):
@@ -50,4 +51,4 @@ class VarLookup(ChainMap):
         return iter(self.keys())
 
 
-ALL = VarLookup(AC_VARS, BASIC_VARS, OPENSSL_VARS)
+ALL = VarLookup(AC_VARS, BASIC_VARS, OPENSSL_VARS, PORT_VARS)
diff --git a/bin/tests/system/isctest/vars/ports.py b/bin/tests/system/isctest/vars/ports.py
new file mode 100644
index 0000000000..c4ff5331f5
--- /dev/null
+++ b/bin/tests/system/isctest/vars/ports.py
@@ -0,0 +1,54 @@
+# 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 os
+
+from .. import log
+
+PORT_MIN = 5001
+PORT_MAX = 32767
+PORTS_PER_TEST = 20
+
+PORT_VARS = {
+    "PORT": "5300",
+    "TLSPORT": "5301",
+    "HTTPPORT": "5302",
+    "HTTPSPORT": "5303",
+    "EXTRAPORT1": "5304",
+    "EXTRAPORT2": "5305",
+    "EXTRAPORT3": "5306",
+    "EXTRAPORT4": "5307",
+    "EXTRAPORT5": "5308",
+    "EXTRAPORT6": "5309",
+    "EXTRAPORT7": "5310",
+    "EXTRAPORT8": "5311",
+    "CONTROLPORT": "5312",
+}
+
+
+def set_base_port(base_port: int):
+    log.debug(f"setting base port {base_port}")
+    assert base_port >= PORT_MIN
+    assert base_port <= PORT_MAX
+    PORT_VARS["PORT"] = str(base_port)
+    PORT_VARS["TLSPORT"] = str(base_port + 1)
+    PORT_VARS["HTTPPORT"] = str(base_port + 2)
+    PORT_VARS["HTTPSPORT"] = str(base_port + 3)
+    PORT_VARS["EXTRAPORT1"] = str(base_port + 4)
+    PORT_VARS["EXTRAPORT2"] = str(base_port + 5)
+    PORT_VARS["EXTRAPORT3"] = str(base_port + 6)
+    PORT_VARS["EXTRAPORT4"] = str(base_port + 7)
+    PORT_VARS["EXTRAPORT5"] = str(base_port + 8)
+    PORT_VARS["EXTRAPORT6"] = str(base_port + 9)
+    PORT_VARS["EXTRAPORT7"] = str(base_port + 10)
+    PORT_VARS["EXTRAPORT8"] = str(base_port + 11)
+    PORT_VARS["CONTROLPORT"] = str(base_port + 12)
+    os.environ.update(PORT_VARS)
diff --git a/bin/tests/system/shutdown/tests_shutdown.py b/bin/tests/system/shutdown/tests_shutdown.py
index 1796114655..a993cc9ffa 100755
--- a/bin/tests/system/shutdown/tests_shutdown.py
+++ b/bin/tests/system/shutdown/tests_shutdown.py
@@ -170,7 +170,7 @@ def wait_for_proc_termination(proc, max_timeout=10):
     "kill_method",
     ["rndc", "sigterm"],
 )
-def test_named_shutdown(ports, kill_method):
+def test_named_shutdown(kill_method):
     # pylint: disable-msg=too-many-locals
     cfg_dir = os.path.join(os.getcwd(), "resolver")
     assert os.path.isdir(cfg_dir)
@@ -186,9 +186,7 @@ def test_named_shutdown(ports, kill_method):
     # necessary for sending RNDC commands to that instance.  This "custom"
     # instance listens on 10.53.0.3, so use "ns3" as the identifier passed to
     # the NamedInstance constructor.
-    named_ports = isctest.instance.NamedPorts(
-        dns=ports["PORT"], rndc=ports["CONTROLPORT"]
-    )
+    named_ports = isctest.instance.NamedPorts.from_env()
     instance = isctest.instance.NamedInstance("ns3", named_ports)
 
     # We create a resolver instance that will be used to send queries.
diff --git a/bin/tests/system/statschannel/conftest.py b/bin/tests/system/statschannel/conftest.py
index c26935b0ab..4c8eb6818d 100644
--- a/bin/tests/system/statschannel/conftest.py
+++ b/bin/tests/system/statschannel/conftest.py
@@ -9,9 +9,11 @@
 # See the COPYRIGHT file distributed with this work for additional
 # information regarding copyright ownership.
 
+import os
+
 import pytest
 
 
 @pytest.fixture(scope="module")
-def statsport(ports):
-    return ports["EXTRAPORT1"]
+def statsport():
+    return int(os.environ["EXTRAPORT1"])

From 1f6f2234d8a20a71019aae92d69f4393f2b74f70 Mon Sep 17 00:00:00 2001
From: Tom Krizek <tkrizek@isc.org>
Date: Wed, 28 Feb 2024 16:49:22 +0100
Subject: [PATCH 10/15] Move dir env var handling to isctest.vars.dirs

---
 bin/tests/system/conftest.py              |  9 +++----
 bin/tests/system/isctest/vars/all.py      |  3 ++-
 bin/tests/system/isctest/vars/dirs.py     | 31 +++++++++++++++++++++++
 bin/tests/system/legacy/ns6/sign.sh       |  2 --
 bin/tests/system/legacy/ns7/sign.sh       |  2 --
 bin/tests/system/verify/zones/genzones.sh |  2 --
 bin/tests/system/wildcard/ns1/sign.sh     |  2 --
 7 files changed, 36 insertions(+), 15 deletions(-)
 create mode 100644 bin/tests/system/isctest/vars/dirs.py

diff --git a/bin/tests/system/conftest.py b/bin/tests/system/conftest.py
index af178aec89..8813dadd08 100644
--- a/bin/tests/system/conftest.py
+++ b/bin/tests/system/conftest.py
@@ -24,6 +24,7 @@ import pytest
 pytest.register_assert_rewrite("isctest")
 
 import isctest
+from isctest.vars.dirs import SYSTEM_TEST_DIR_GIT_PATH
 
 
 # Silence warnings caused by passing a pytest fixture to another fixture.
@@ -61,7 +62,6 @@ PRIORITY_TESTS = [
     "upforwd/",
 ]
 PRIORITY_TESTS_RE = re.compile("|".join(PRIORITY_TESTS))
-SYSTEM_TEST_DIR_GIT_PATH = "bin/tests/system"
 SYSTEM_TEST_NAME_RE = re.compile(f"{SYSTEM_TEST_DIR_GIT_PATH}" + r"/([^/]+)")
 SYMLINK_REPLACEMENT_RE = re.compile(r"/tests(_.*)\.py")
 
@@ -260,8 +260,6 @@ def control_port():
 def env():
     """Dictionary containing environment variables for the test."""
     env = dict(isctest.vars.ALL)
-    env["builddir"] = f"{env['TOP_BUILDDIR']}/{SYSTEM_TEST_DIR_GIT_PATH}"
-    env["srcdir"] = f"{env['TOP_SRCDIR']}/{SYSTEM_TEST_DIR_GIT_PATH}"
     os.environ.update(env)
     return env
 
@@ -354,14 +352,13 @@ def system_test_dir(
             pass
 
     # Create a temporary directory with a copy of the original system test dir contents
-    system_test_root = Path(
-        f"{isctest.vars.ALL['TOP_BUILDDIR']}/{SYSTEM_TEST_DIR_GIT_PATH}"
-    )
+    system_test_root = Path(os.environ["builddir"])
     testdir = Path(
         tempfile.mkdtemp(prefix=f"{system_test_name}_tmp_", dir=system_test_root)
     )
     shutil.rmtree(testdir)
     shutil.copytree(system_test_root / system_test_name, testdir)
+    isctest.vars.dirs.set_system_test_name(testdir.name)
 
     # Create a convenience symlink with a stable and predictable name
     module_name = SYMLINK_REPLACEMENT_RE.sub(r"\1", request.node.name)
diff --git a/bin/tests/system/isctest/vars/all.py b/bin/tests/system/isctest/vars/all.py
index cc2a7f8836..3e6d49dc74 100644
--- a/bin/tests/system/isctest/vars/all.py
+++ b/bin/tests/system/isctest/vars/all.py
@@ -16,6 +16,7 @@ from .autoconf import AC_VARS  # type: ignore
 
 # pylint: enable=import-error
 from .basic import BASIC_VARS
+from .dirs import DIR_VARS
 from .openssl import OPENSSL_VARS
 from .ports import PORT_VARS
 
@@ -51,4 +52,4 @@ class VarLookup(ChainMap):
         return iter(self.keys())
 
 
-ALL = VarLookup(AC_VARS, BASIC_VARS, OPENSSL_VARS, PORT_VARS)
+ALL = VarLookup(AC_VARS, BASIC_VARS, OPENSSL_VARS, PORT_VARS, DIR_VARS)
diff --git a/bin/tests/system/isctest/vars/dirs.py b/bin/tests/system/isctest/vars/dirs.py
new file mode 100644
index 0000000000..56f4c55375
--- /dev/null
+++ b/bin/tests/system/isctest/vars/dirs.py
@@ -0,0 +1,31 @@
+# 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 os
+
+# pylint: disable=import-error
+from .autoconf import AC_VARS  # type: ignore
+
+# pylint: enable=import-error
+
+
+SYSTEM_TEST_DIR_GIT_PATH = "bin/tests/system"
+
+DIR_VARS = {
+    "builddir": f"{AC_VARS['TOP_BUILDDIR']}/{SYSTEM_TEST_DIR_GIT_PATH}",
+    "srcdir": f"{AC_VARS['TOP_SRCDIR']}/{SYSTEM_TEST_DIR_GIT_PATH}",
+    "SYSTESTDIR": None,
+}
+
+
+def set_system_test_name(name: str):
+    DIR_VARS["SYSTESTDIR"] = name
+    os.environ["SYSTESTDIR"] = name
diff --git a/bin/tests/system/legacy/ns6/sign.sh b/bin/tests/system/legacy/ns6/sign.sh
index 96ce2855fe..49a44bb91a 100755
--- a/bin/tests/system/legacy/ns6/sign.sh
+++ b/bin/tests/system/legacy/ns6/sign.sh
@@ -13,8 +13,6 @@
 
 . ../../conf.sh
 
-SYSTESTDIR=legacy
-
 echo_i "sign edns512"
 
 zone=edns512
diff --git a/bin/tests/system/legacy/ns7/sign.sh b/bin/tests/system/legacy/ns7/sign.sh
index 82c6b7e2f5..9dfa9f6732 100755
--- a/bin/tests/system/legacy/ns7/sign.sh
+++ b/bin/tests/system/legacy/ns7/sign.sh
@@ -13,8 +13,6 @@
 
 . ../../conf.sh
 
-SYSTESTDIR=legacy
-
 echo_i "sign edns512-notcp"
 
 zone=edns512-notcp
diff --git a/bin/tests/system/verify/zones/genzones.sh b/bin/tests/system/verify/zones/genzones.sh
index 56866f9c79..4c999db98d 100644
--- a/bin/tests/system/verify/zones/genzones.sh
+++ b/bin/tests/system/verify/zones/genzones.sh
@@ -13,8 +13,6 @@
 
 . ../../conf.sh
 
-SYSTESTDIR=verify
-
 dumpit() {
   echo_d "${debug}: dumping ${1}"
   cat "${1}" | cat_d
diff --git a/bin/tests/system/wildcard/ns1/sign.sh b/bin/tests/system/wildcard/ns1/sign.sh
index d1d86260f7..7af0b83f72 100755
--- a/bin/tests/system/wildcard/ns1/sign.sh
+++ b/bin/tests/system/wildcard/ns1/sign.sh
@@ -13,8 +13,6 @@
 
 . ../../conf.sh
 
-SYSTESTDIR=wildcard
-
 dssets=
 
 # RFC 4592 example zone.

From 2d1f3484839c30c342c464770d953f56334fe042 Mon Sep 17 00:00:00 2001
From: Tom Krizek <tkrizek@isc.org>
Date: Wed, 28 Feb 2024 17:04:40 +0100
Subject: [PATCH 11/15] Move env var initialization to isctest.vars

Make sure all initialization takes place in isctest.vars.__init__ and
export the initial env vars there. Remove the no longer needed env
fixture and use os.environ instead.
---
 bin/tests/system/conftest.py              | 35 ++++++-----------------
 bin/tests/system/isctest/vars/__init__.py | 11 +++++++
 bin/tests/system/isctest/vars/openssl.py  |  3 --
 3 files changed, 19 insertions(+), 30 deletions(-)

diff --git a/bin/tests/system/conftest.py b/bin/tests/system/conftest.py
index 8813dadd08..77b1948eaf 100644
--- a/bin/tests/system/conftest.py
+++ b/bin/tests/system/conftest.py
@@ -17,7 +17,7 @@ import shutil
 import subprocess
 import tempfile
 import time
-from typing import Any, Dict, List, Optional
+from typing import Any, List, Optional
 
 import pytest
 
@@ -65,14 +65,6 @@ 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")
 
-# ---------------------- Module initialization ---------------------------
-
-# Set environment variables for tests.
-os.environ.update(isctest.vars.ALL)
-isctest.log.debug(
-    "variables in env: %s", ", ".join([str(key) for key in isctest.vars.ALL])
-)
-
 # ----------------------- Global requirements ----------------------------
 
 isctest.check.is_executable(isctest.vars.ALL["PYTHON"], "Python interpreter required")
@@ -256,14 +248,6 @@ def control_port():
     return int(os.environ["CONTROLPORT"])
 
 
-@pytest.fixture(scope="module")
-def env():
-    """Dictionary containing environment variables for the test."""
-    env = dict(isctest.vars.ALL)
-    os.environ.update(env)
-    return env
-
-
 @pytest.fixture(scope="module")
 def system_test_name(request):
     """Name of the system test directory."""
@@ -413,7 +397,6 @@ def system_test_dir(
 
 
 def _run_script(  # pylint: disable=too-many-arguments
-    env,
     system_test_dir: Path,
     interpreter: str,
     script: str,
@@ -437,7 +420,6 @@ def _run_script(  # pylint: disable=too-many-arguments
     cmd = [interpreter, script] + args
     with subprocess.Popen(
         cmd,
-        env=env,
         stdout=subprocess.PIPE,
         stderr=subprocess.STDOUT,
         bufsize=1,
@@ -455,15 +437,15 @@ def _run_script(  # pylint: disable=too-many-arguments
 
 
 @pytest.fixture(scope="module")
-def shell(env, system_test_dir):
+def shell(system_test_dir):
     """Function to call a shell script with arguments."""
-    return partial(_run_script, env, system_test_dir, env["SHELL"])
+    return partial(_run_script, system_test_dir, os.environ["SHELL"])
 
 
 @pytest.fixture(scope="module")
-def perl(env, system_test_dir):
+def perl(system_test_dir):
     """Function to call a perl script with arguments."""
-    return partial(_run_script, env, system_test_dir, env["PERL"])
+    return partial(_run_script, system_test_dir, os.environ["PERL"])
 
 
 @pytest.fixture(scope="module")
@@ -479,7 +461,6 @@ def run_tests_sh(system_test_dir, shell):
 @pytest.fixture(scope="module", autouse=True)
 def system_test(  # pylint: disable=too-many-arguments,too-many-statements
     request,
-    env: Dict[str, str],
     system_test_dir,
     shell,
     perl,
@@ -508,7 +489,7 @@ def system_test(  # pylint: disable=too-many-arguments,too-many-statements
 
     def check_net_interfaces():
         try:
-            perl("testsock.pl", ["-p", env["PORT"]])
+            perl("testsock.pl", ["-p", os.environ["PORT"]])
         except subprocess.CalledProcessError as exc:
             isctest.log.error("testsock.pl: exited with code %d", exc.returncode)
             pytest.skip("Network interface aliases not set up.")
@@ -532,7 +513,7 @@ def system_test(  # pylint: disable=too-many-arguments,too-many-statements
 
     def start_servers():
         try:
-            perl("start.pl", ["--port", env["PORT"], system_test_dir.name])
+            perl("start.pl", ["--port", os.environ["PORT"], system_test_dir.name])
         except subprocess.CalledProcessError as exc:
             isctest.log.error("Failed to start servers")
             pytest.fail(f"start.pl exited with {exc.returncode}")
@@ -553,7 +534,7 @@ def system_test(  # pylint: disable=too-many-arguments,too-many-statements
             pytest.fail(f"get_core_dumps.sh exited with {exc.returncode}")
 
     isctest.log.info(f"test started: {request.node.name}")
-    port = int(env["PORT"])
+    port = int(os.environ["PORT"])
     isctest.log.info(
         "using port range: <%d, %d>", port, port + isctest.vars.ports.PORTS_PER_TEST - 1
     )
diff --git a/bin/tests/system/isctest/vars/__init__.py b/bin/tests/system/isctest/vars/__init__.py
index b12df88f70..d4e26a892f 100644
--- a/bin/tests/system/isctest/vars/__init__.py
+++ b/bin/tests/system/isctest/vars/__init__.py
@@ -9,4 +9,15 @@
 # See the COPYRIGHT file distributed with this work for additional
 # information regarding copyright ownership.
 
+import os
+
 from .all import ALL
+from .openssl import parse_openssl_config
+from .. import log
+
+
+# env variable initialization
+parse_openssl_config(ALL["OPENSSL_CONF"])
+
+os.environ.update(ALL)
+log.debug("setting following env vars: %s", ", ".join([str(key) for key in ALL]))
diff --git a/bin/tests/system/isctest/vars/openssl.py b/bin/tests/system/isctest/vars/openssl.py
index 5659222c09..9ad2020938 100644
--- a/bin/tests/system/isctest/vars/openssl.py
+++ b/bin/tests/system/isctest/vars/openssl.py
@@ -50,6 +50,3 @@ def parse_openssl_config(path: Optional[str]):
                     log.debug(
                         "SOFTHSM2_MODULE set to {OPENSSL_VARS['SOFTHSM2_MODULE']}"
                     )
-
-
-parse_openssl_config(OPENSSL_VARS["OPENSSL_CONF"])

From b99cdfab9d012bce63dff8463eae87772034138a Mon Sep 17 00:00:00 2001
From: Tom Krizek <tkrizek@isc.org>
Date: Thu, 29 Feb 2024 10:53:41 +0100
Subject: [PATCH 12/15] Allow failure of cross-version-config test

The test is bound to fail until it is executed against a new release
which will include the pytest env var refactoring.
---
 .gitlab-ci.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 3cb73e0d2e..27dcb698c7 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -720,6 +720,7 @@ cross-version-config-tests:
     untracked: true
     expire_in: "1 day"
     when: always
+  allow_failure: true  # pytest env variable refactoring
 
 # Jobs for regular GCC builds on Alpine Linux 3.19 (amd64)
 

From 2a09f632abce1602f01742bfc7e816efa740ba43 Mon Sep 17 00:00:00 2001
From: Tom Krizek <tkrizek@isc.org>
Date: Mon, 25 Mar 2024 14:56:08 +0100
Subject: [PATCH 13/15] Use a dedicated file for each autoconf variable

To avoid any escaping issues or messing with a language-specific format
when the variable has to be parsed, create a dedicated file for each
variable that is obtained from autoconf.
---
 .gitlab-ci.yml                                |  4 +--
 .reuse/dep5                                   |  1 +
 bin/tests/system/.gitignore                   |  3 +-
 .../system/isctest/vars/.ac_vars/CURL.in      |  1 +
 .../isctest/vars/.ac_vars/FSTRM_CAPTURE.in    |  1 +
 bin/tests/system/isctest/vars/.ac_vars/NC.in  |  1 +
 .../system/isctest/vars/.ac_vars/PERL.in      |  1 +
 .../system/isctest/vars/.ac_vars/PYTEST.in    |  1 +
 .../system/isctest/vars/.ac_vars/PYTHON.in    |  1 +
 .../system/isctest/vars/.ac_vars/SHELL.in     |  1 +
 .../isctest/vars/.ac_vars/TOP_BUILDDIR.in     |  1 +
 .../isctest/vars/.ac_vars/TOP_SRCDIR.in       |  1 +
 .../system/isctest/vars/.ac_vars/XSLTPROC.in  |  1 +
 bin/tests/system/isctest/vars/autoconf.py     | 29 +++++++++++++++++++
 bin/tests/system/isctest/vars/autoconf.py.in  | 23 ---------------
 configure.ac                                  | 11 ++++++-
 16 files changed, 54 insertions(+), 27 deletions(-)
 create mode 100644 bin/tests/system/isctest/vars/.ac_vars/CURL.in
 create mode 100644 bin/tests/system/isctest/vars/.ac_vars/FSTRM_CAPTURE.in
 create mode 100644 bin/tests/system/isctest/vars/.ac_vars/NC.in
 create mode 100644 bin/tests/system/isctest/vars/.ac_vars/PERL.in
 create mode 100644 bin/tests/system/isctest/vars/.ac_vars/PYTEST.in
 create mode 100644 bin/tests/system/isctest/vars/.ac_vars/PYTHON.in
 create mode 100644 bin/tests/system/isctest/vars/.ac_vars/SHELL.in
 create mode 100644 bin/tests/system/isctest/vars/.ac_vars/TOP_BUILDDIR.in
 create mode 100644 bin/tests/system/isctest/vars/.ac_vars/TOP_SRCDIR.in
 create mode 100644 bin/tests/system/isctest/vars/.ac_vars/XSLTPROC.in
 create mode 100644 bin/tests/system/isctest/vars/autoconf.py
 delete mode 100644 bin/tests/system/isctest/vars/autoconf.py.in

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 27dcb698c7..8ae413727f 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -271,11 +271,11 @@ stages:
 
 # change directory to the workspace before including this
 .find_python: &find_python
-  - PYTHON="$(sed -n -E 's|^[[:space:]]*"PYTHON":[[:space:]]*"([^"]*)",[[:space:]]*$|\1|p' bin/tests/system/isctest/vars/autoconf.py)"
+  - PYTHON="$(cat bin/tests/system/isctest/vars/.ac_vars/PYTHON)"
   - test -x "$PYTHON"
 
 .find_pytest: &find_pytest
-  - PYTEST="$(sed -n -E 's|^[[:space:]]*"PYTEST":[[:space:]]*"([^"]*)",[[:space:]]*$|\1|p' bin/tests/system/isctest/vars/autoconf.py)"
+  - PYTEST="$(cat bin/tests/system/isctest/vars/.ac_vars/PYTEST)"
   - test -x "$PYTEST"
 
 .parse_tsan: &parse_tsan
diff --git a/.reuse/dep5 b/.reuse/dep5
index 0de1a85da0..1becbd1686 100644
--- a/.reuse/dep5
+++ b/.reuse/dep5
@@ -47,6 +47,7 @@ Files: **/*.after*
        bin/tests/system/forward/CA/index.txt
        bin/tests/system/forward/CA/index.txt.attr
        bin/tests/system/forward/CA/serial
+       bin/tests/system/isctest/vars/.ac_vars/*
        bin/tests/system/journal/ns1/managed-keys.bind.in
        bin/tests/system/journal/ns1/managed-keys.bind.jnl.in
        bin/tests/system/journal/ns2/managed-keys.bind.in
diff --git a/bin/tests/system/.gitignore b/bin/tests/system/.gitignore
index 05ae7f8b4e..3c2db219b0 100644
--- a/bin/tests/system/.gitignore
+++ b/bin/tests/system/.gitignore
@@ -19,7 +19,8 @@ named.run
 /start.sh
 /stop.sh
 /ifconfig.sh
-/isctest/vars/autoconf.py
+/isctest/vars/.ac_vars/*
+!/isctest/vars/.ac_vars/*.in
 
 # Ignore file names with underscore in their name except python or shell files.
 # This is done to ignore the temporary directories and symlinks created by the
diff --git a/bin/tests/system/isctest/vars/.ac_vars/CURL.in b/bin/tests/system/isctest/vars/.ac_vars/CURL.in
new file mode 100644
index 0000000000..0772e63bb0
--- /dev/null
+++ b/bin/tests/system/isctest/vars/.ac_vars/CURL.in
@@ -0,0 +1 @@
+@CURL@
diff --git a/bin/tests/system/isctest/vars/.ac_vars/FSTRM_CAPTURE.in b/bin/tests/system/isctest/vars/.ac_vars/FSTRM_CAPTURE.in
new file mode 100644
index 0000000000..d6b9931d01
--- /dev/null
+++ b/bin/tests/system/isctest/vars/.ac_vars/FSTRM_CAPTURE.in
@@ -0,0 +1 @@
+@FSTRM_CAPTURE@
diff --git a/bin/tests/system/isctest/vars/.ac_vars/NC.in b/bin/tests/system/isctest/vars/.ac_vars/NC.in
new file mode 100644
index 0000000000..b1181ca7dc
--- /dev/null
+++ b/bin/tests/system/isctest/vars/.ac_vars/NC.in
@@ -0,0 +1 @@
+@NC@
diff --git a/bin/tests/system/isctest/vars/.ac_vars/PERL.in b/bin/tests/system/isctest/vars/.ac_vars/PERL.in
new file mode 100644
index 0000000000..f6644c1d08
--- /dev/null
+++ b/bin/tests/system/isctest/vars/.ac_vars/PERL.in
@@ -0,0 +1 @@
+@PERL@
diff --git a/bin/tests/system/isctest/vars/.ac_vars/PYTEST.in b/bin/tests/system/isctest/vars/.ac_vars/PYTEST.in
new file mode 100644
index 0000000000..f6700d5f65
--- /dev/null
+++ b/bin/tests/system/isctest/vars/.ac_vars/PYTEST.in
@@ -0,0 +1 @@
+@PYTEST@
diff --git a/bin/tests/system/isctest/vars/.ac_vars/PYTHON.in b/bin/tests/system/isctest/vars/.ac_vars/PYTHON.in
new file mode 100644
index 0000000000..5d697c390e
--- /dev/null
+++ b/bin/tests/system/isctest/vars/.ac_vars/PYTHON.in
@@ -0,0 +1 @@
+@PYTHON@
diff --git a/bin/tests/system/isctest/vars/.ac_vars/SHELL.in b/bin/tests/system/isctest/vars/.ac_vars/SHELL.in
new file mode 100644
index 0000000000..e65178bcca
--- /dev/null
+++ b/bin/tests/system/isctest/vars/.ac_vars/SHELL.in
@@ -0,0 +1 @@
+@SHELL@
diff --git a/bin/tests/system/isctest/vars/.ac_vars/TOP_BUILDDIR.in b/bin/tests/system/isctest/vars/.ac_vars/TOP_BUILDDIR.in
new file mode 100644
index 0000000000..ad1e3ece17
--- /dev/null
+++ b/bin/tests/system/isctest/vars/.ac_vars/TOP_BUILDDIR.in
@@ -0,0 +1 @@
+@abs_top_builddir@
diff --git a/bin/tests/system/isctest/vars/.ac_vars/TOP_SRCDIR.in b/bin/tests/system/isctest/vars/.ac_vars/TOP_SRCDIR.in
new file mode 100644
index 0000000000..ab6784b0e5
--- /dev/null
+++ b/bin/tests/system/isctest/vars/.ac_vars/TOP_SRCDIR.in
@@ -0,0 +1 @@
+@abs_top_srcdir@
diff --git a/bin/tests/system/isctest/vars/.ac_vars/XSLTPROC.in b/bin/tests/system/isctest/vars/.ac_vars/XSLTPROC.in
new file mode 100644
index 0000000000..86d98e0a10
--- /dev/null
+++ b/bin/tests/system/isctest/vars/.ac_vars/XSLTPROC.in
@@ -0,0 +1 @@
+@XSLTPROC@
diff --git a/bin/tests/system/isctest/vars/autoconf.py b/bin/tests/system/isctest/vars/autoconf.py
new file mode 100644
index 0000000000..7000ef2d72
--- /dev/null
+++ b/bin/tests/system/isctest/vars/autoconf.py
@@ -0,0 +1,29 @@
+# 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.
+
+from pathlib import Path
+from typing import Dict
+
+
+def load_ac_vars_from_files() -> Dict[str, str]:
+    ac_vars = {}
+    ac_vars_dir = Path(__file__).resolve().parent / ".ac_vars"
+    var_paths = [
+        path
+        for path in ac_vars_dir.iterdir()
+        if path.is_file() and not path.name.endswith(".in")
+    ]
+    for var_path in var_paths:
+        ac_vars[var_path.name] = var_path.read_text(encoding="utf-8").strip()
+    return ac_vars
+
+
+AC_VARS = load_ac_vars_from_files()
diff --git a/bin/tests/system/isctest/vars/autoconf.py.in b/bin/tests/system/isctest/vars/autoconf.py.in
deleted file mode 100644
index 5f1741bd73..0000000000
--- a/bin/tests/system/isctest/vars/autoconf.py.in
+++ /dev/null
@@ -1,23 +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.
-
-AC_VARS = {
-    "TOP_BUILDDIR": "@abs_top_builddir@",
-    "TOP_SRCDIR": "@abs_top_srcdir@",
-    "FSTRM_CAPTURE": "@FSTRM_CAPTURE@",
-    "SHELL": "@SHELL@",
-    "PYTHON": "@PYTHON@",
-    "PERL": "@PERL@",
-    "CURL": "@CURL@",
-    "NC": "@NC@",
-    "XSLTPROC": "@XSLTPROC@",
-    "PYTEST": "@PYTEST@",
-}
diff --git a/configure.ac b/configure.ac
index 31c965697c..bf7405cc1b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1656,7 +1656,16 @@ AC_CONFIG_FILES([tests/unit-test-driver.sh],
 
 AC_CONFIG_FILES([bin/tests/Makefile
 		 bin/tests/system/Makefile
-		 bin/tests/system/isctest/vars/autoconf.py
+		 bin/tests/system/isctest/vars/.ac_vars/TOP_BUILDDIR
+		 bin/tests/system/isctest/vars/.ac_vars/TOP_SRCDIR
+		 bin/tests/system/isctest/vars/.ac_vars/FSTRM_CAPTURE
+		 bin/tests/system/isctest/vars/.ac_vars/SHELL
+		 bin/tests/system/isctest/vars/.ac_vars/PYTHON
+		 bin/tests/system/isctest/vars/.ac_vars/PERL
+		 bin/tests/system/isctest/vars/.ac_vars/CURL
+		 bin/tests/system/isctest/vars/.ac_vars/NC
+		 bin/tests/system/isctest/vars/.ac_vars/XSLTPROC
+		 bin/tests/system/isctest/vars/.ac_vars/PYTEST
 		 bin/tests/system/dyndb/driver/Makefile
 		 bin/tests/system/dlzexternal/driver/Makefile
 		 bin/tests/system/hooks/driver/Makefile

From ac7c657d191edbf7061b2620a4a511f51661213b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nicki=20K=C5=99=C3=AD=C5=BEek?= <nicki@isc.org>
Date: Mon, 6 May 2024 16:55:42 +0200
Subject: [PATCH 14/15] Move isctest.var initialization to conftest.py

The environment variable initialization requires logging to be set up
first. Ensure the initialization is delayed until loggers have been set
up.
---
 bin/tests/system/conftest.py              | 1 +
 bin/tests/system/isctest/vars/__init__.py | 9 +++++----
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/bin/tests/system/conftest.py b/bin/tests/system/conftest.py
index 77b1948eaf..5585d1e402 100644
--- a/bin/tests/system/conftest.py
+++ b/bin/tests/system/conftest.py
@@ -33,6 +33,7 @@ from isctest.vars.dirs import SYSTEM_TEST_DIR_GIT_PATH
 
 isctest.log.init_conftest_logger()
 isctest.log.avoid_duplicated_logs()
+isctest.vars.init_vars()
 
 # ----------------- Older pytest / xdist compatibility -------------------
 # As of 2023-01-11, the minimal supported pytest / xdist versions are
diff --git a/bin/tests/system/isctest/vars/__init__.py b/bin/tests/system/isctest/vars/__init__.py
index d4e26a892f..7c06a247a2 100644
--- a/bin/tests/system/isctest/vars/__init__.py
+++ b/bin/tests/system/isctest/vars/__init__.py
@@ -16,8 +16,9 @@ from .openssl import parse_openssl_config
 from .. import log
 
 
-# env variable initialization
-parse_openssl_config(ALL["OPENSSL_CONF"])
+def init_vars():
+    """Initializes the environment variables."""
+    parse_openssl_config(ALL["OPENSSL_CONF"])
 
-os.environ.update(ALL)
-log.debug("setting following env vars: %s", ", ".join([str(key) for key in ALL]))
+    os.environ.update(ALL)
+    log.debug("setting following env vars: %s", ", ".join([str(key) for key in ALL]))

From faeec83b6473989e16f5e062237ff9331a8d135b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nicki=20K=C5=99=C3=AD=C5=BEek?= <nicki@isc.org>
Date: Mon, 6 May 2024 18:04:46 +0200
Subject: [PATCH 15/15] Ensure OPENSSL_CONF is a file if it exists

Prevent unexpected behavior in cases where the OPENSSL_CONF path would
exist, but it wouldn't point to a file.
---
 bin/tests/system/isctest/vars/openssl.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/bin/tests/system/isctest/vars/openssl.py b/bin/tests/system/isctest/vars/openssl.py
index 9ad2020938..5df12b7247 100644
--- a/bin/tests/system/isctest/vars/openssl.py
+++ b/bin/tests/system/isctest/vars/openssl.py
@@ -25,12 +25,13 @@ OPENSSL_VARS = {
 
 
 def parse_openssl_config(path: Optional[str]):
-    if path is None or not os.path.isfile(path):
+    if path is None or not os.path.exists(path):
         OPENSSL_VARS["ENGINE_ARG"] = None
         OPENSSL_VARS["SOFTHSM2_MODULE"] = None
         os.environ.pop("ENGINE_ARG", None)
         os.environ.pop("SOFTHSM2_MODULE", None)
         return
+    assert os.path.isfile(path), f"{path} exists, but it's not a file"
 
     regex = re.compile(r"([^=]+)=(.*)")
     log.debug(f"parsing openssl config: {path}")