Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v2: Merge upstream changes, doc updates #3

Closed
wants to merge 24 commits into from
Closed

v2: Merge upstream changes, doc updates #3

wants to merge 24 commits into from

Conversation

nyurik
Copy link
Owner

@nyurik nyurik commented May 24, 2024

Also removed the --create rights parameter, more test targets

Deletes locale test because its incompatible with PG16

nyurik and others added 23 commits December 28, 2022 21:07
The `::set-output` command has been deprecated and is scheduled for
removal on Jun 1, 2023 [1]. The recommended way to set output parameters
today is by writing them to `$GITHUB_OUTPUT` file [2].

[1] https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/
[2] https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#setting-an-output-parameter

Resolves: #6
Use `$GITHUB_OUTPUT` instead of `::set-output`
The 'setup-postgres' action used to create a superuser that hasn't been
exposed to users via 'connection-uri' output. The superuser has been
named after GitHub Action's system user (i.e. 'runner'), had no password
and could have been used via PostgreSQL client applications [1] or when
using manually constructed connection URI with no user set.

The user set via action's input parameters used to be unprivileged with
escalated permissions to create databases on-demand. I don't remember
why I made things this way, maybe I got confused somewhere along the
way, but I don't think having both private superuser and public
unprivileged user is a good idea. It's quite common in tests to
dynamically create databases and/or users for applications under test,
thus superuser permissions are required.

This patch removes a private superuser named after the GitHub Action's
system user (i.e. 'runner') in favor of granting superuser permissions
to a user set via action's input parameters. Those who explicitly relied
on 'runner' user might got affected as the user WON'T exist anymore.

[1] https://www.postgresql.org/docs/15/reference-client.html
BREAKING CHANGE: create superuser
It turns out that PostgreSQL comes with weird default that allows
passwordless authentication for localhost connections [1]. This
essentially means that 'password' input parameter for this action was
ignored.

The 'setup-postgres' action's primary use case is to be used on CI where
most of the time authentication is desired in order to verify that
passwords are passed correctly from applications under test.

This patch enforces password authentication even for localhost
connections, making sure that passwords are verified and not ignored.
This will break everyone who previously passed wrong password or didn't
pass it at all.

[1] https://www.postgresql.org/docs/15/auth-trust.html

Fixes: #5
BREAKING CHANGE: enforce password authentication
The 'setup-postgres' action used to set libpq environment variables [1]
with connection parameters so the PostgreSQL client applications [2],
such as 'psql' or 'createuser', won't require any configuration before
using.

Unfortunately these libpq environment variables are also used by all
libpq clients including database drivers such as 'psycopg'. While this
may sound good, it may as well lead to undesired behaviour and unobvious
issues when connection parameters are automatically pulled from
environment but most not.

Nevertheless, the need to easy usage of the client applications [2] is
indisputable because providing a bunch of connection parameters all the
time is tedious. Therefore this patch pushes requires connection
parameters to the connection service file [3], so the client
applications can pull the data on-demand. E.g:

    $ psql service=superuser -c "SELECT 1;"
    $ PGSERVICE=superuser createuser myuser

[1] https://www.postgresql.org/docs/15/libpq-envars.html
[2] https://www.postgresql.org/docs/15/reference-client.html
[3] https://www.postgresql.org/docs/15/libpq-pgservice.html
BREAKING CHANGE: don't set connection env vars
The checkout v2, the version currently in use on CI, depends on node12
which is going to be removed soon. Let's use the latest version in order
to get rid of deprecation warnings in logs and remain compatible with
GitHun runners.
The macOS 13 runner image isn't shipped with pre-installed PostgreSQL
server. Even though it has Beta status, it'd be nice to support this
runner image too.

Reported-by: @baconcheese113
Fixes: #16
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
Unset PG* env vars except PGSERVICEFILE
Due to some dark magic used to prepare the macOS 13 runner, the brew
install command fails to link unrelated kegs, such as `[email protected]`.
Fortunately, since `[email protected]` is a dependency of a dependency, and
that dependency is already installed, we can workaround the issue by
forcing Homebrew to do not try upgrading dependencies unless absolutely
neccessary.

Fixes: #27
The macOS 14 runner image isn't shipped with pre-installed PostgreSQL
server. Even though it has Beta status, it'd be nice to support this
runner image too.

Co-authored-by: hfhbd <[email protected]>
Turns out that Windows Server 2019 does not support locale names
specified in the format defined by the POSIX standard. This patch
converts the POSIX format to the one supported by Windows 2019.

In addition to that, this patch adds all supported action runners to
the continue integration, so we can make sure that this action works
properly on every supported platform.

Reported-by: Irena Rindos
Fixes: #25
@nyurik nyurik force-pushed the upgrade branch 2 times, most recently from 0a5d757 to ccc81ae Compare May 24, 2024 04:11
delete locale test - incompatible with PG16
@nyurik nyurik closed this May 24, 2024
@nyurik nyurik deleted the upgrade branch May 24, 2024 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants