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

@@ -3,7 +3,6 @@ nxtify
 sdig
 *_test
 gsstest
-conf.sh
 dlopen
 keycreate
 keydelete

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

bin/tests/system/conf.sh

@@ -20,9 +20,6 @@ testsock6() {
 	fi
 }
 
-export LANG=C
-export SYSTESTDIR="$(basename $PWD)"
-
 echofail () {
     echo "$*"
 }

bin/tests/system/conf.sh.in

@@ -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

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)

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

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

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

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}

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@",
+}

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",
+}

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