Skip to content

Commit

Permalink
Unset PG* env vars except PGSERVICEFILE
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ikalnytskyi committed Jan 9, 2024
1 parent 7685acb commit 11ff483
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 2 deletions.
16 changes: 14 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,19 @@ key features:

## Usage

> [!IMPORTANT]
>
> In order to connect to a PostgreSQL server, use either connection parameters
> from the table below ([link](#connection-parameters)), or retrieve a
> connection URI from the `connection-uri` output ([link](#advanced)).
> [!TIP]
>
> `libpq`-using applications may choose to set the `PGSERVICE=postgres`
> environment variable instead ([link](#create-a-new-user-w-database-via-cli)),
> where `postgres` is the service name extracted from the `service-name`
> output.
#### Connection parameters

| Key | Value |
Expand Down Expand Up @@ -73,11 +86,10 @@ steps:
createuser myuser
createdb --owner myuser mydatabase
psql -c "ALTER USER myuser WITH PASSWORD 'mypassword'"
env:
# This activates connection parameters for the superuser created by
# the action in the step above. It's mandatory to set this before using
# createuser/psql/etc tools.
# createuser/psql and other libpq-using applications.
#
# The service name is the same as the username (i.e. 'postgres') but
# it's recommended to use action's output to get the name in order to
Expand Down
7 changes: 7 additions & 0 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,13 @@ runs:
elif [ "$RUNNER_OS" == "Windows" ]; then
echo "$PGBIN" >> $GITHUB_PATH
echo "PQ_LIB_DIR=$PGROOT\lib" >> $GITHUB_ENV
# The Windows runner has some PostgreSQL environment variables set
# that may confuse users since they may be irrelevant to the
# PostgreSQL server we're using.
for name in "PGROOT" "PGDATA" "PGBIN" "PGUSER" "PGPASSWORD"; do
echo "$name=" >> $GITHUB_ENV
done
elif [ "$RUNNER_OS" == "macOS" ]; then
case "$(sw_vers -productVersion)" in
13.*)
Expand Down
38 changes: 38 additions & 0 deletions test_action.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import locale
import os
import pathlib
import subprocess
import typing as t

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


def test_environment_variables():
"""Test that only expected 'PG*' variables are set."""

pg_environ = {k: v for k, v in os.environ.items() if k.startswith("PG")}

# In case of Windows, there might be a mix of forward and backward slashes
# as separators. So let's compare paths semantically instead.
pg_servicefile = pathlib.Path(pg_environ.pop("PGSERVICEFILE", ""))
pg_servicefile_exp = pathlib.Path(os.environ["RUNNER_TEMP"], "pgdata", "pg_service.conf")
assert pg_servicefile.resolve() == pg_servicefile_exp.resolve()

if os.name == "nt":
pg_environ_exp = {
"PGBIN": "",
"PGDATA": "",
"PGPASSWORD": "",
"PGROOT": "",
"PGUSER": "",
}
else:
pg_environ_exp = {}
assert pg_environ == pg_environ_exp


def test_user_permissions(connection: psycopg.Connection):
"""Test that a user has super/createdb permissions."""

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


def test_libpq_applications(service_name: str, monkeypatch: pytest.MonkeyPatch):
"""Test that libpq-using applications can be used."""

# Request connection parameters from the connection service file prepared
# by our action.
monkeypatch.setenv("PGSERVICE", service_name)

with psycopg.connect() as connection:
assert connection \
.execute("SELECT usename FROM pg_user WHERE usename = CURRENT_USER") \
.fetchone()


def test_auth_wrong_username(connection_factory: ConnectionFactory, connection_uri: str):
"""Test that wrong username is rejected!"""

Expand Down

0 comments on commit 11ff483

Please sign in to comment.