Skip to content

Commit 11ff483

Browse files
committed
Unset PG* env vars except PGSERVICEFILE
Unfortunately, the Windows runner has some PostgreSQL environment variables set, which are neither set for Linux or macOS runners. This is may be especially confusing since variables such as PGUSER or PGPASSWORD may point to a user that doesn't exist. Since one of the design decisions made previously is to restrain this action from pointing to any user by default, we better unset these environment variables to avoid confusion. In addition to that change, let's also mention in the README file that it's up to users to set connection parameters whatever way they prefer. Reported-by: bkoelman Fixes: #17
1 parent 7685acb commit 11ff483

File tree

3 files changed

+59
-2
lines changed

3 files changed

+59
-2
lines changed

README.md

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,19 @@ key features:
1414

1515
## Usage
1616

17+
> [!IMPORTANT]
18+
>
19+
> In order to connect to a PostgreSQL server, use either connection parameters
20+
> from the table below ([link](#connection-parameters)), or retrieve a
21+
> connection URI from the `connection-uri` output ([link](#advanced)).
22+
23+
> [!TIP]
24+
>
25+
> `libpq`-using applications may choose to set the `PGSERVICE=postgres`
26+
> environment variable instead ([link](#create-a-new-user-w-database-via-cli)),
27+
> where `postgres` is the service name extracted from the `service-name`
28+
> output.
29+
1730
#### Connection parameters
1831

1932
| Key | Value |
@@ -73,11 +86,10 @@ steps:
7386
createuser myuser
7487
createdb --owner myuser mydatabase
7588
psql -c "ALTER USER myuser WITH PASSWORD 'mypassword'"
76-
7789
env:
7890
# This activates connection parameters for the superuser created by
7991
# the action in the step above. It's mandatory to set this before using
80-
# createuser/psql/etc tools.
92+
# createuser/psql and other libpq-using applications.
8193
#
8294
# The service name is the same as the username (i.e. 'postgres') but
8395
# it's recommended to use action's output to get the name in order to

action.yml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,13 @@ runs:
3838
elif [ "$RUNNER_OS" == "Windows" ]; then
3939
echo "$PGBIN" >> $GITHUB_PATH
4040
echo "PQ_LIB_DIR=$PGROOT\lib" >> $GITHUB_ENV
41+
42+
# The Windows runner has some PostgreSQL environment variables set
43+
# that may confuse users since they may be irrelevant to the
44+
# PostgreSQL server we're using.
45+
for name in "PGROOT" "PGDATA" "PGBIN" "PGUSER" "PGPASSWORD"; do
46+
echo "$name=" >> $GITHUB_ENV
47+
done
4148
elif [ "$RUNNER_OS" == "macOS" ]; then
4249
case "$(sw_vers -productVersion)" in
4350
13.*)

test_action.py

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import locale
22
import os
3+
import pathlib
34
import subprocess
45
import typing as t
56

@@ -84,6 +85,30 @@ def test_locale(connection: psycopg.Connection):
8485
assert locale.normalize(lc_ctype) == "en_US.UTF-8"
8586

8687

88+
def test_environment_variables():
89+
"""Test that only expected 'PG*' variables are set."""
90+
91+
pg_environ = {k: v for k, v in os.environ.items() if k.startswith("PG")}
92+
93+
# In case of Windows, there might be a mix of forward and backward slashes
94+
# as separators. So let's compare paths semantically instead.
95+
pg_servicefile = pathlib.Path(pg_environ.pop("PGSERVICEFILE", ""))
96+
pg_servicefile_exp = pathlib.Path(os.environ["RUNNER_TEMP"], "pgdata", "pg_service.conf")
97+
assert pg_servicefile.resolve() == pg_servicefile_exp.resolve()
98+
99+
if os.name == "nt":
100+
pg_environ_exp = {
101+
"PGBIN": "",
102+
"PGDATA": "",
103+
"PGPASSWORD": "",
104+
"PGROOT": "",
105+
"PGUSER": "",
106+
}
107+
else:
108+
pg_environ_exp = {}
109+
assert pg_environ == pg_environ_exp
110+
111+
87112
def test_user_permissions(connection: psycopg.Connection):
88113
"""Test that a user has super/createdb permissions."""
89114

@@ -201,6 +226,19 @@ def test_client_applications(
201226
subprocess.check_call(["dropuser", username])
202227

203228

229+
def test_libpq_applications(service_name: str, monkeypatch: pytest.MonkeyPatch):
230+
"""Test that libpq-using applications can be used."""
231+
232+
# Request connection parameters from the connection service file prepared
233+
# by our action.
234+
monkeypatch.setenv("PGSERVICE", service_name)
235+
236+
with psycopg.connect() as connection:
237+
assert connection \
238+
.execute("SELECT usename FROM pg_user WHERE usename = CURRENT_USER") \
239+
.fetchone()
240+
241+
204242
def test_auth_wrong_username(connection_factory: ConnectionFactory, connection_uri: str):
205243
"""Test that wrong username is rejected!"""
206244

0 commit comments

Comments
 (0)