From 2425cddaed1514237a46c1ec05cf0aad05d75de7 Mon Sep 17 00:00:00 2001 From: Remo Rechkemmer <59358383+monacoremo@users.noreply.github.com> Date: Thu, 24 Dec 2020 16:09:25 +0100 Subject: [PATCH] improve io-tests Tests are now run through unix socket by default allowing parallel execution. Test setup configuration is done via environment variables instead of config files. --- nix/README.md | 16 +- nix/tests.nix | 5 +- test/io-tests/configs/app-settings.config | 2 - .../configs/base64-secret-from-file.config | 2 - test/io-tests/configs/dburi-from-file.config | 2 - test/io-tests/configs/role-claim-key.config | 2 - test/io-tests/configs/secret-from-file.config | 2 - test/io-tests/configs/sigusr2-settings.config | 2 - test/io-tests/configs/simple.config | 2 - test/io-tests/test_io.py | 164 ++++++++++-------- test/memory-tests.sh | 6 +- test/with_tmp_db | 2 +- 12 files changed, 117 insertions(+), 90 deletions(-) diff --git a/nix/README.md b/nix/README.md index e014a6926..5da08b9d7 100644 --- a/nix/README.md +++ b/nix/README.md @@ -160,7 +160,21 @@ $ nix-shell --run postgrest-test-spec-all # Run the tests against a specific version of PostgreSQL (use tab-completion in # nix-shell to see all available versions): -$ nix-shell --run postgrest-test-spec-postgresql-9.5 +$ nix-shell --run postgrest-test-spec-postgresql-13 + +``` + +The io-test that test PostgREST as a black box with inputs and outputs can be +run with `postgrest-test-io`. The test runner under the hood is +[pytest](https://docs.pytest.org/) and you can pass it the usual options: + +```bash +# Filter the tests to run by name, including all that contain 'config': +postgrest-test-io -k config + +# Run tests in parallel using xdist, specifying the number of processes: +postgrest-test-io -n auto +postgrest-test-io -n 8 ``` diff --git a/nix/tests.nix b/nix/tests.nix index e69392557..087d9d744 100644 --- a/nix/tests.nix +++ b/nix/tests.nix @@ -90,11 +90,12 @@ let ioTestPython = python3.withPackages (ps: [ + ps.pyjwt ps.pytest + ps.pytest_xdist + ps.pyyaml ps.requests ps.requests-unixsocket - ps.pyjwt - ps.pyyaml ]); testIO = diff --git a/test/io-tests/configs/app-settings.config b/test/io-tests/configs/app-settings.config index bc216f9de..20a811abd 100644 --- a/test/io-tests/configs/app-settings.config +++ b/test/io-tests/configs/app-settings.config @@ -1,6 +1,4 @@ db-pool = 1 db-pool-timeout = 1 -server-host = "127.0.0.1" -server-port = 49421 app.settings.external_api_secret = "0123456789abcdef" diff --git a/test/io-tests/configs/base64-secret-from-file.config b/test/io-tests/configs/base64-secret-from-file.config index 03cd05272..5040da58d 100644 --- a/test/io-tests/configs/base64-secret-from-file.config +++ b/test/io-tests/configs/base64-secret-from-file.config @@ -1,6 +1,4 @@ db-pool = 1 -server-host = "127.0.0.1" -server-port = 49421 # Read secret from a file: /dev/stdin (alias for standard input) jwt-secret = "@/dev/stdin" diff --git a/test/io-tests/configs/dburi-from-file.config b/test/io-tests/configs/dburi-from-file.config index cac279f06..2e52488ca 100644 --- a/test/io-tests/configs/dburi-from-file.config +++ b/test/io-tests/configs/dburi-from-file.config @@ -1,5 +1,3 @@ db-uri = "@/dev/stdin" db-pool = 1 -server-host = "127.0.0.1" -server-port = 49421 jwt-secret = "reallyreallyreallyreallyverysafe" diff --git a/test/io-tests/configs/role-claim-key.config b/test/io-tests/configs/role-claim-key.config index 63284abb9..a0bd6605a 100644 --- a/test/io-tests/configs/role-claim-key.config +++ b/test/io-tests/configs/role-claim-key.config @@ -1,5 +1,3 @@ db-pool = 1 -server-host = "127.0.0.1" -server-port = 49421 jwt-role-claim-key = "$(ROLE_CLAIM_KEY)" jwt-secret = "reallyreallyreallyreallyverysafe" diff --git a/test/io-tests/configs/secret-from-file.config b/test/io-tests/configs/secret-from-file.config index 813a25a8a..a80404bda 100644 --- a/test/io-tests/configs/secret-from-file.config +++ b/test/io-tests/configs/secret-from-file.config @@ -1,6 +1,4 @@ db-pool = 1 -server-host = "127.0.0.1" -server-port = 49421 # Read secret from a file: /dev/stdin (alias for standard input) jwt-secret = "@/dev/stdin" diff --git a/test/io-tests/configs/sigusr2-settings.config b/test/io-tests/configs/sigusr2-settings.config index 9d1a914fe..1ebfdc148 100644 --- a/test/io-tests/configs/sigusr2-settings.config +++ b/test/io-tests/configs/sigusr2-settings.config @@ -1,7 +1,5 @@ db-schemas = "test" db-pool = 1 -server-host = "127.0.0.1" -server-port = 49421 app.settings.name_var = "John" jwt-secret = "invalidinvalidinvalidinvalidinvalid" diff --git a/test/io-tests/configs/simple.config b/test/io-tests/configs/simple.config index 3554a0f24..5073cce00 100644 --- a/test/io-tests/configs/simple.config +++ b/test/io-tests/configs/simple.config @@ -1,4 +1,2 @@ db-pool = 1 -server-host = "127.0.0.1" -server-port = 49421 jwt-secret = "reallyreallyreallyreallyverysafe" diff --git a/test/io-tests/test_io.py b/test/io-tests/test_io.py index 96a2f1183..5bb543354 100644 --- a/test/io-tests/test_io.py +++ b/test/io-tests/test_io.py @@ -3,11 +3,14 @@ import contextlib import dataclasses from datetime import datetime -import pathlib -import subprocess from operator import attrgetter import os +import pathlib +import shutil import signal +import socket +import subprocess +import tempfile import time import urllib.parse @@ -21,7 +24,7 @@ import yaml BASEDIR = pathlib.Path(os.path.realpath(__file__)).parent CONFIGSDIR = BASEDIR / "configs" FIXTURES = yaml.load((BASEDIR / "fixtures.yaml").read_text(), Loader=yaml.Loader) -BASEURL = "http://127.0.0.1:49421" +POSTGREST_BIN = shutil.which("postgrest") SECRET = "reallyreallyreallyreallyverysafe" @@ -41,7 +44,9 @@ class PostgrestSession(requests_unixsocket.Session): self.baseurl = baseurl def request(self, method, url, *args, **kwargs): - fullurl = urllib.parse.urljoin(self.baseurl, url) + # Not using urllib.parse.urljoin to compose the url, as it doesn't play + # well with our 'http+unix://' unix domain socket urls. + fullurl = self.baseurl + url return super(PostgrestSession, self).request(method, fullurl, *args, **kwargs) @@ -58,25 +63,27 @@ def dburi(): return os.getenv("PGRST_DB_URI").encode("utf-8") -def mkenv(moreenv): - """ - Create env from os.environ and moreenv, while - filtering None values to allow overriding "unset". - """ - env = {**os.environ, **(moreenv or {})} - return {k: v for k, v in env.items() if v is not None} +@pytest.fixture +def defaultenv(): + "Default environment for PostgREST." + return { + "PGRST_DB_URI": os.environ["PGRST_DB_URI"], + "PGRST_DB_SCHEMAS": os.environ["PGRST_DB_SCHEMAS"], + "PGRST_DB_ANON_ROLE": os.environ["PGRST_DB_ANON_ROLE"], + } -def dumpconfig(configpath=None, moreenv=None, stdin=None): +def dumpconfig(configpath=None, env=None, stdin=None): "Dump the config as parsed by PostgREST." + command = [POSTGREST_BIN, "--dump-config"] - command = ["postgrest", "--dump-config"] if configpath: - command += [configpath] + command.append(configpath) process = subprocess.Popen( - command, env=mkenv(moreenv), stdin=subprocess.PIPE, stdout=subprocess.PIPE + command, env=env or {}, stdin=subprocess.PIPE, stdout=subprocess.PIPE ) + process.stdin.write(stdin or b"") result = process.communicate()[0] process.kill() @@ -87,27 +94,44 @@ def dumpconfig(configpath=None, moreenv=None, stdin=None): @contextlib.contextmanager -def run(configpath, stdin=None, moreenv=None, socket=None): +def run(configpath=None, stdin=None, env=None, port=None): "Run PostgREST and yield an endpoint that is ready for connections." - if socket: - baseurl = "http+unix://" + urllib.parse.quote_plus(str(socket)) - else: - baseurl = BASEURL + with tempfile.TemporaryDirectory() as tmpdir: + if port: + env["PGRST_SERVER_PORT"] = str(port) + env["PGRST_SERVER_HOST"] = "localhost" + baseurl = f"http://localhost:{port}" + else: + socketfile = pathlib.Path(tmpdir) / "postgrest.sock" + env["PGRST_SERVER_UNIX_SOCKET"] = str(socketfile) + baseurl = "http+unix://" + urllib.parse.quote_plus(str(socketfile)) - command = ["postgrest", configpath] - process = subprocess.Popen(command, stdin=subprocess.PIPE, env=mkenv(moreenv)) + command = [POSTGREST_BIN] - try: - process.stdin.write(stdin or b"") - process.stdin.close() + if configpath: + command.append(configpath) - wait_until_ready(baseurl) + process = subprocess.Popen(command, stdin=subprocess.PIPE, env=env or {}) - yield PostgrestProcess(process=process, session=PostgrestSession(baseurl)) - finally: - process.kill() - process.wait() + try: + process.stdin.write(stdin or b"") + process.stdin.close() + + wait_until_ready(baseurl) + + yield PostgrestProcess(process=process, session=PostgrestSession(baseurl)) + finally: + process.kill() + process.wait() + + +def freeport(): + "Find a free port on localhost." + with contextlib.closing(socket.socket(socket.AF_INET, socket.SOCK_STREAM)) as s: + s.bind(("", 0)) + s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + return s.getsockname()[1] def wait_until_ready(url): @@ -153,8 +177,7 @@ def test_expected_config(expectedconfig): expected = expectedconfig.read_text() config = CONFIGSDIR / expectedconfig.name - unset = {"PGRST_DB_URI": None, "PGRST_DB_ANON_ROLE": None, "PGRST_DB_SCHEMAS": None} - assert dumpconfig(config, moreenv=unset) == expected + assert dumpconfig(config) == expected def test_expected_config_from_environment(): @@ -164,7 +187,7 @@ def test_expected_config_from_environment(): env = {k: str(v) for k, v in yaml.load(envfile, Loader=yaml.Loader).items()} expected = (CONFIGSDIR / "expected" / "no-defaults.config").read_text() - assert dumpconfig(moreenv=env) == expected + assert dumpconfig(env=env) == expected @pytest.mark.parametrize( @@ -172,7 +195,7 @@ def test_expected_config_from_environment(): [conf for conf in CONFIGSDIR.iterdir() if conf.suffix == ".config"], ids=attrgetter("name"), ) -def test_stable_config(tmp_path, config): +def test_stable_config(tmp_path, config, defaultenv): """ A dumped, re-read and re-dumped config should match the dumped config. @@ -184,30 +207,27 @@ def test_stable_config(tmp_path, config): # Set environment variables that some of the configs expect. Using a # complex ROLE_CLAIM_KEY to make sure quoting works. env = { + **defaultenv, "ROLE_CLAIM_KEY": '."https://www.example.com/roles"[0].value', "POSTGREST_TEST_SOCKET": "/tmp/postgrest.sock", + "POSTGREST_TEST_PORT": "80", } # Some configs expect input from stdin, at least on base64. stdin = b"Y29ubmVjdGlvbl9zdHJpbmc=" - dumped = dumpconfig(config, moreenv=env, stdin=stdin) + dumped = dumpconfig(config, env=env, stdin=stdin) tmpconfigpath = tmp_path / "config" tmpconfigpath.write_text(dumped) - redumped = dumpconfig(tmpconfigpath, moreenv=env) + redumped = dumpconfig(tmpconfigpath, env=env) assert dumped == redumped -def test_socket_connection(tmp_path): - "Connections via unix domain sockets should work." - socket = tmp_path / "postgrest.sock" - env = { - "POSTGREST_TEST_SOCKET": str(socket), - } - - with run(CONFIGSDIR / "unix-socket.config", socket=socket, moreenv=env): +def test_port_connection(defaultenv): + "Connections via a port on localhost should work." + with run(env=defaultenv, port=freeport()): pass @@ -216,7 +236,7 @@ def test_socket_connection(tmp_path): [path for path in (BASEDIR / "secrets").iterdir() if path.suffix != ".jwt"], ids=attrgetter("name"), ) -def test_read_secret_from_file(secretpath): +def test_read_secret_from_file(secretpath, defaultenv): "Authorization should succeed when the secret is read from a file." if secretpath.suffix == ".b64": configfile = CONFIGSDIR / "base64-secret-from-file.config" @@ -226,53 +246,59 @@ def test_read_secret_from_file(secretpath): secret = secretpath.read_bytes() headers = authheader(secretpath.with_suffix(".jwt").read_text()) - with run(configfile, stdin=secret) as postgrest: + with run(configfile, stdin=secret, env=defaultenv) as postgrest: response = postgrest.session.get("/authors_only", headers=headers) assert response.status_code == 200 -def test_read_dburi_from_file_without_eol(dburi): +def test_read_dburi_from_file_without_eol(dburi, defaultenv): "Reading the dburi from a file with a single line should work." config = CONFIGSDIR / "dburi-from-file.config" - unset = {"PGRST_DB_URI": None} - with run(config, moreenv=unset, stdin=dburi): + env = {key: value for key, value in defaultenv.items() if key != "PGRST_DB_URI"} + with run(config, env=env, stdin=dburi): pass -def test_read_dburi_from_file_with_eol(dburi): +def test_read_dburi_from_file_with_eol(dburi, defaultenv): "Reading the dburi from a file containing a newline should work." config = CONFIGSDIR / "dburi-from-file.config" - unset = {"PGRST_DB_URI": None} - with run(config, moreenv=unset, stdin=dburi + b"\n"): + env = {key: value for key, value in defaultenv.items() if key != "PGRST_DB_URI"} + with run(config, env=env, stdin=dburi + b"\n"): pass @pytest.mark.parametrize( "roleclaim", FIXTURES["roleclaims"], ids=lambda claim: claim["key"] ) -def test_role_claim_key(roleclaim): +def test_role_claim_key(roleclaim, defaultenv): "Authorization should depend on a correct role-claim-key and JWT claim." - env = {"ROLE_CLAIM_KEY": roleclaim["key"]} + env = { + **defaultenv, + "ROLE_CLAIM_KEY": roleclaim["key"], + } headers = jwtauthheader(roleclaim["data"], SECRET) - with run(CONFIGSDIR / "role-claim-key.config", moreenv=env) as postgrest: + with run(CONFIGSDIR / "role-claim-key.config", env=env) as postgrest: response = postgrest.session.get("/authors_only", headers=headers) assert response.status_code == roleclaim["expected_status"] @pytest.mark.parametrize("invalidroleclaimkey", FIXTURES["invalidroleclaimkeys"]) -def test_invalid_role_claim_key(invalidroleclaimkey): +def test_invalid_role_claim_key(invalidroleclaimkey, defaultenv): "Given an invalid role-claim-key, Postgrest should exit with a non-zero exit code." - env = {"ROLE_CLAIM_KEY": invalidroleclaimkey} + env = { + **defaultenv, + "ROLE_CLAIM_KEY": invalidroleclaimkey, + } with pytest.raises(PostgrestError): - dump = dumpconfig(CONFIGSDIR / "role-claim-key.config", moreenv=env) + dump = dumpconfig(CONFIGSDIR / "role-claim-key.config", env=env) for line in dump.split("\n"): if line.startswith("jwt-role-claim-key"): print(line) -def test_iat_claim(): +def test_iat_claim(defaultenv): """ A claim with an 'iat' (issued at) attribute should be successful. @@ -283,7 +309,7 @@ def test_iat_claim(): claim = {"role": "postgrest_test_author", "iat": datetime.utcnow()} headers = jwtauthheader(claim, SECRET) - with run(CONFIGSDIR / "simple.config") as postgrest: + with run(CONFIGSDIR / "simple.config", env=defaultenv) as postgrest: for _ in range(10): response = postgrest.session.get("/authors_only", headers=headers) assert response.status_code == 200 @@ -291,14 +317,14 @@ def test_iat_claim(): time.sleep(0.5) -def test_app_settings(): +def test_app_settings(defaultenv): """ App settings should not reset when the db pool times out. See: https://github.com/PostgREST/postgrest/issues/1141 """ - with run(CONFIGSDIR / "app-settings.config") as postgrest: + with run(CONFIGSDIR / "app-settings.config", env=defaultenv) as postgrest: # Wait for the db pool to time out, set to 1s in config time.sleep(2) @@ -309,14 +335,14 @@ def test_app_settings(): assert response.text == '"0123456789abcdef"' -def test_app_settings_reload(tmp_path): +def test_app_settings_reload(tmp_path, defaultenv): "App settings should be reloaded when PostgREST is sent SIGUSR2." config = (CONFIGSDIR / "sigusr2-settings.config").read_text() configfile = tmp_path / "test.config" configfile.write_text(config) uri = "/rpc/get_guc_value?name=app.settings.name_var" - with run(configfile) as postgrest: + with run(configfile, env=defaultenv) as postgrest: response = postgrest.session.get(uri) assert response.status_code == 200 assert response.text == '"John"' @@ -333,7 +359,7 @@ def test_app_settings_reload(tmp_path): assert response.text == '"Jane"' -def test_jwt_secret_reload(tmp_path): +def test_jwt_secret_reload(tmp_path, defaultenv): "JWT secret should be reloaded when PostgREST is sent SIGUSR2." config = (CONFIGSDIR / "sigusr2-settings.config").read_text() configfile = tmp_path / "test.config" @@ -341,7 +367,7 @@ def test_jwt_secret_reload(tmp_path): headers = jwtauthheader({"role": "postgrest_test_author"}, SECRET) - with run(configfile) as postgrest: + with run(configfile, env=defaultenv) as postgrest: response = postgrest.session.get("/authors_only", headers=headers) assert response.status_code == 401 @@ -357,16 +383,16 @@ def test_jwt_secret_reload(tmp_path): assert response.status_code == 200 -def test_db_schema_reload(tmp_path): +def test_db_schema_reload(tmp_path, defaultenv): "DB schema should be reloaded when PostgREST is sent SIGUSR2." config = (CONFIGSDIR / "sigusr2-settings.config").read_text() configfile = tmp_path / "test.config" configfile.write_text(config) headers = {"Accept-Profile": "v1"} - unset = {"PGRST_DB_SCHEMAS": None} + env = {key: value for key, value in defaultenv.items() if key != "PGRST_DB_SCHEMAS"} - with run(configfile, moreenv=unset) as postgrest: + with run(configfile, env=env) as postgrest: response = postgrest.session.get("/parents", headers=headers) assert response.status_code == 404 diff --git a/test/memory-tests.sh b/test/memory-tests.sh index 1dfba36ff..790f7c5c7 100755 --- a/test/memory-tests.sh +++ b/test/memory-tests.sh @@ -5,10 +5,12 @@ set -eu +pgrPort=49421 + # PGRST_DB_URI, PGRST_DB_ANON_ROLE and PGRST_DB_SCHEMAS are expected to be set by with_tmp_db export PGRST_DB_POOL="1" export PGRST_SERVER_HOST="127.0.0.1" -export PGRST_SERVER_PORT="49421" +export PGRST_SERVER_PORT="$pgrPort" export PGRST_JWT_SECRET="reallyreallyreallyreallyverysafe" trap "kill 0" int term exit @@ -19,8 +21,6 @@ result(){ echo "$1 $currentTest $2"; currentTest=$(( $currentTest + 1 )); } ok(){ result 'ok' "- $1"; } ko(){ result 'not ok' "- $1"; failedTests=$(( $failedTests + 1 )); } -pgrPort=49421 - pgrStart(){ postgrest +RTS -p -h > /dev/null & pgrPID="$!"; } pgrStop(){ kill "$pgrPID" 2>/dev/null; } diff --git a/test/with_tmp_db b/test/with_tmp_db index e7147db56..9e7d54901 100755 --- a/test/with_tmp_db +++ b/test/with_tmp_db @@ -56,7 +56,7 @@ export PGDATA="$tmpdir/db" export PGHOST="$tmpdir/socket" export PGUSER=postgrest_test_authenticator export PGDATABASE=postgres -export DB_URI="postgresql://$PGDATABASE?host=$PGHOST&user=$PGUSER" +export DB_URI="postgresql:///$PGDATABASE?host=$PGHOST&user=$PGUSER" export PGRST_DB_URI="$DB_URI" export PGRST_DB_SCHEMAS="test" export PGRST_DB_ANON_ROLE="postgrest_test_anonymous"