Merge pull request #11 from ikalnytskyi/superuser

ikalnytskyi · web-flow · commit ae2fb38952a7 · 2023-01-03T16:42:23.000+02:00
BREAKING CHANGE: create superuser
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -26,7 +26,7 @@ jobs:
 
       - name: Run tests
         run: |
-          python3 -m pip install --upgrade pip pytest psycopg
+          python3 -m pip install --upgrade pip pytest psycopg furl
           python3 -m pytest -vv test_action.py
         env:
           CONNECTION_URI: ${{ steps.postgres.outputs.connection-uri }}
@@ -55,7 +55,7 @@ jobs:
 
       - name: Run tests
         run: |
-          python3 -m pip install --upgrade pip pytest psycopg
+          python3 -m pip install --upgrade pip pytest psycopg furl
           python3 -m pytest -vv test_action.py
         env:
           CONNECTION_URI: ${{ steps.postgres.outputs.connection-uri }}
diff --git a/README.md b/README.md
@@ -3,17 +3,19 @@
 This action sets up a PostgreSQL server for the rest of the job. Here are some
 key features:
 
-* Runs on Linux, macOS and Windows runners.
-* Adds PostgreSQL [binaries][1] (e.g. `psql`) to `PATH`.
-* Uses PostgreSQL installed in [GitHub Actions Virtual Environments][2].
-* [Easy to check][3] that IT DOES NOT contain malicious code.
+* Runs on Linux, macOS and Windows action runners.
+* Adds PostgreSQL [client applications][1] to `PATH`.
+* Uses PostgreSQL binaries baked into [GitHub Actions Runner Images][2].
+* Easy [to prove][3] that it DOES NOT contain malicious code.
 
 [1]: https://www.postgresql.org/docs/current/reference-client.html
-[2]: https://github.com/actions/virtual-environments
+[2]: https://github.com/actions/runner-images
 [3]: action.yml
 
 ## Usage
 
+#### Connection parameters
+
 | Key      | Value                                               |
 |----------|-----------------------------------------------------|
 | URI      | `postgresql://postgres:postgres@localhost/postgres` |
@@ -23,6 +25,13 @@ key features:
 | Password | `postgres`                                          |
 | Database | `postgres`                                          |
 
+#### User permissions
+
+| Key         | Value |
+|-------------|-------|
+| usesuper    | true  |
+| usecreatedb | true  |
+
 #### Basic
 
 ```yaml
@@ -47,6 +56,13 @@ steps:
       DATABASE_URI: ${{ steps.postgres.outputs.connection-uri }}
 ```
 
+## Rationale
+
+At the time of developing there were no GitHub Actions on the marketplace to
+setup a PostgreSQL server on Linux, Windows and macOS action runners. Most
+solutions suggest using Docker which is not available on macOS and Windows
+runners.
+
 ## License
 
 The scripts and documentation in this project are released under the
diff --git a/action.yml b/action.yml
@@ -38,42 +38,56 @@ runs:
         fi
       shell: bash
 
-
     - name: Setup and start PostgreSQL
       run: |
         export PGDATA="$RUNNER_TEMP/pgdata"
-        pg_ctl init --options="--encoding=UTF-8 --locale=en_US.UTF-8"
 
-        # Forbid creating unix sockets since they are created by default in the
-        # directory we don't have permissions to.
+        # There are couple of reasons why we need to create a new PostgreSQL
+        # database cluster. First and foremost, we have to create a superuser
+        # with provided credentials. Second, we want the PostgreSQL client
+        # applications [1] to be available for execution without
+        # run-from-another-user dances. Third, we want to make sure that
+        # settings are the same between operating systems and aren't changed by
+        # package vendors.
+        #
+        # [1] https://www.postgresql.org/docs/15/reference-client.html
+        initdb \
+          --username="${{ inputs.username }}" \
+          --encoding="UTF-8" \
+          --locale="en_US.UTF-8" \
+          --no-instructions
+
+        # Do not create unix sockets since they are created by default in the
+        # directory we have no permissions to (owned by system postgres user).
         echo "unix_socket_directories = ''" >> "$PGDATA/postgresql.conf"
         echo "port = ${{ inputs.port }}" >> "$PGDATA/postgresql.conf"
         pg_ctl start
 
-        # Both PGHOST and PGUSER are used by PostgreSQL tooling such as 'psql'
-        # or 'createuser'. Since PostgreSQL data has been resetup, we cannot
-        # rely on defaults anymore.
+        # Set environment variables for PostgreSQL client applications [1] such
+        # as 'psql' or 'createuser'.
         #
-        # PGHOST is required for Linux and macOS since they default to unix
-        # sockets, and we have turned them off.
+        # PGHOST is required for Linux/macOS because we turned off unix sockets
+        # and they use them by default.
         #
-        # PGUSER is required for Windows since default the tooling's default
-        # user is 'postgres', while 'pg_ctl init' creates one with the name of
-        # the current user.
+        # PGPORT, PGUSER and PGDATABASE are required because they could be
+        # parametrized via action input parameters.
+        #
+        # [1] https://www.postgresql.org/docs/15/reference-client.html
         echo "PGHOST=localhost" >> $GITHUB_ENV
-        echo "PGUSER=${USER:-$USERNAME}" >> $GITHUB_ENV
         echo "PGPORT=${{ inputs.port }}" >> $GITHUB_ENV
+        echo "PGUSER=${{ inputs.username }}" >> $GITHUB_ENV
+        echo "PGDATABASE=${{ inputs.database }}" >> $GITHUB_ENV
       shell: bash
 
-    - name: Setup PostgreSQL user and database
+    - name: Setup PostgreSQL database
       run: |
-        createuser --createdb ${{ inputs.username }}
-
-        if [ "${{ inputs.database}}" != "postgres" ]; then
-          createdb -O ${{ inputs.username }} ${{ inputs.database }}
+        # The 'postgres' database is a pre-created database meant for use by
+        # users, utilities and third party applications. There's no way to
+        # parametrize the name, so all we can do is to avoid creating a
+        # database if provided name is 'postgres'.
+        if [ "${{ inputs.database }}" != "postgres" ]; then
+          createdb -O "${{ inputs.username }}" "${{ inputs.database }}"
         fi
-
-        psql -c "ALTER USER ${{ inputs.username }} PASSWORD '${{ inputs.password }}';" ${{ inputs.database }}
       shell: bash
 
     - name: Expose connection URI
diff --git a/test_action.py b/test_action.py
@@ -1,20 +1,40 @@
-import typing as t
+import locale
 import os
+import subprocess
+import typing as t
 
 import psycopg
+import furl
 import pytest
 
 
+ConnectionFactory = t.Callable[[str], psycopg.Connection]
+
+
+@pytest.fixture(scope="function")
+def connection_uri() -> str:
+    """Read and return connection URI from environment."""
+
+    connection_uri = os.getenv("CONNECTION_URI")
+    if connection_uri is None:
+        pytest.fail("CONNECTION_URI: environment variable is not set")
+    return connection_uri
+
+
 @pytest.fixture(scope="function")
-def connection_factory() -> t.Callable[[str], psycopg.Connection]:
+def connection_factory() -> ConnectionFactory:
+    """Return 'psycopg.Connection' factory."""
+
     def factory(connection_uri: str) -> psycopg.Connection:
         return psycopg.connect(connection_uri)
     return factory
 
 
 @pytest.fixture(scope="function")
-def connection(connection_factory) -> psycopg.Connection:
-    return connection_factory(os.getenv("CONNECTION_URI"))
+def connection(connection_uri: str, connection_factory: ConnectionFactory) -> psycopg.Connection:
+    """Return 'psycopg.Connection' for connection URI set in environment."""
+
+    return connection_factory(connection_uri)
 
 
 def test_connection_uri():
@@ -34,12 +54,15 @@ def test_server_encoding(connection: psycopg.Connection):
 def test_locale(connection: psycopg.Connection):
     """Test that PostgreSQL's locale is 'en_US.UTF-8'."""
 
-    assert connection.execute("SHOW LC_COLLATE").fetchone()[0] == "en_US.UTF-8"
-    assert connection.execute("SHOW LC_CTYPE").fetchone()[0] == "en_US.UTF-8"
+    lc_collate = connection.execute("SHOW LC_COLLATE").fetchone()[0]
+    lc_ctype = connection.execute("SHOW LC_CTYPE").fetchone()[0]
+
+    assert locale.normalize(lc_collate) == "en_US.UTF-8"
+    assert locale.normalize(lc_ctype) == "en_US.UTF-8"
 
 
 def test_user_permissions(connection: psycopg.Connection):
-    """Test that a user can create databases but is not a superuser."""
+    """Test that a user has super/createdb permissions."""
 
     with connection:
         record = connection \
@@ -49,7 +72,7 @@ def test_user_permissions(connection: psycopg.Connection):
 
         usecreatedb, usesuper = record
         assert usecreatedb
-        assert not usesuper
+        assert usesuper
 
 
 def test_user_create_insert_select(connection: psycopg.Connection):
@@ -82,12 +105,65 @@ def test_user_create_insert_non_ascii(connection: psycopg.Connection):
 
 
 def test_user_create_drop_database(connection: psycopg.Connection):
-    """Test that a user has no permissions to create databases."""
+    """Test that a user has permissions to create databases."""
 
     # CREATE/DROP DATABASE statements don't work within transactions, and with
     # autocommit disabled transactions are created by psycopg automatically.
     connection.autocommit = True
 
-    database_name = "foobar42"
-    connection.execute(f"CREATE DATABASE {database_name}")
-    connection.execute(f"DROP DATABASE {database_name}")
+    database = "databas3"
+    connection.execute(f"CREATE DATABASE {database}")
+    connection.execute(f"DROP DATABASE {database}")
+
+
+def test_user_create_drop_user(
+    connection: psycopg.Connection,
+    connection_factory: ConnectionFactory,
+    connection_uri: str
+):
+    """Test that a user has permissions to create users."""
+
+    # CREATE/DROP USER statements don't work within transactions, and with
+    # autocommit disabled transactions are created by psycopg automatically.
+    connection.autocommit = True
+
+    username = "us3rname"
+    password = "passw0rd"
+    database = "databas3"
+
+    connection.execute(f"CREATE USER {username} WITH PASSWORD '{password}'")
+    connection.execute(f"CREATE DATABASE {database} WITH OWNER '{username}'")
+
+    try:
+        # Smoke test that created user can successfully log-in and execute
+        # queries for its own database.
+        connection_uri = furl.furl(
+            connection_uri, username=username, password=password, path=database).url
+        test_user_create_insert_select(connection_factory(connection_uri))
+
+    finally:
+        connection.execute(f"DROP DATABASE {database}")
+        connection.execute(f"DROP USER {username}")
+
+
+def test_client_applications(connection_uri, connection_factory):
+    """Test that PostgreSQL client applications can be used."""
+
+    username = "us3rname"
+    password = "passw0rd"
+    database = "databas3"
+
+    subprocess.check_call(["createuser", username])
+    subprocess.check_call(["createdb", "--owner", username, database])
+    subprocess.check_call(["psql", "-c", f"ALTER USER {username} WITH PASSWORD '{password}'"])
+
+    try:
+        # Smoke test that created user can successfully log-in and execute
+        # queries for its own database.
+        connection_uri = furl.furl(
+            connection_uri, username=username, password=password, path=database).url
+        test_user_create_insert_select(connection_factory(connection_uri))
+
+    finally:
+        subprocess.check_call(["dropdb", database])
+        subprocess.check_call(["dropuser", username])
