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

Listen only on the unix socket during init #440

Merged
merged 1 commit into from
May 4, 2018
Merged

Listen only on the unix socket during init #440

merged 1 commit into from
May 4, 2018

Conversation

sdwolfz
Copy link
Contributor

@sdwolfz sdwolfz commented May 3, 2018

Healthchecks that used pg_isready -p 5432 were incorrectly flagging
the container as being healthy during initialization phase since
healthchecks are being run inside the container itself, so
listen_addresses='localhost' was not enough. The port 65432 was
chosen at random.

@yosifkit
Copy link
Member

yosifkit commented May 3, 2018

If we do change this, I'd rather just switch it to -c listen_addresses='' so that it only listens on the socket and will be less likely to break users. Then users can implement their healthchecks to connect via ip or 127.0.0.1 as we have in the example postgres healthcheck.

If the list is empty, the server does not listen on any IP interface at all, in which case only Unix-domain sockets can be used to connect to it.

https://www.postgresql.org/docs/current/static/runtime-config-connection.html

@yosifkit yosifkit requested a review from tianon May 3, 2018 17:37
@tianon
Copy link
Member

tianon commented May 3, 2018 via email

@sdwolfz sdwolfz changed the title Use nonstandard port for the initialization phase Listen only on the unix socket during init May 3, 2018
@sdwolfz
Copy link
Contributor Author

sdwolfz commented May 3, 2018

I've removed localhost from the listen_addresses as you suggested. I'm happy with this approach since it seems to have the same behaviour.

Healthchecks that used `pg_isready -p 5432` were incorrectly flagging
the container as being healthy during initialization phase since
healthchecks are being run inside the container itself, so
`listen_addresses='localhost'` was not enough. Setting
`listen_addresses=''` forces the server to only listen on the unix
socket so no ports are open that might incorrectly interfeer with the
healthchecks.
@sdwolfz
Copy link
Contributor Author

sdwolfz commented May 4, 2018

I see the CI fails randomly due to keyservers so I'll push a commit that implements the approach detailed here.

@yosifkit
Copy link
Member

yosifkit commented May 4, 2018

Let's not complicate this PR with unrelated gpg changes please. docker-library/official-images#4252 (comment)

@sdwolfz
Copy link
Contributor Author

sdwolfz commented May 4, 2018

OK, I removed the keyserver commit. You probably need to rerun the failing jobs manually.

@tianon
Copy link
Member

tianon commented May 4, 2018

(Builds in progress are duplicates -- the functionality has been tested sufficiently by other builds. 👍)

@tianon tianon merged commit 01cf838 into docker-library:master May 4, 2018
@sdwolfz sdwolfz deleted the fix/postgres-healthcheck branch May 4, 2018 19:56
tianon added a commit to infosiftr/stackbrew that referenced this pull request May 8, 2018
- `docker`: docker-library/docker#110 (updated `dind` wrapper)
- `ghost`: 1.22.6
- `mariadb`: MariaDB/mariadb-docker#161 (remove unnecessary `FLUSH PRIVILEGES`)
- `openjdk`: `debian 11~12-1`, `debian 10.0.1+10-4`
- `postgres`: docker-library/postgres#440 (listen only on the unix socket during initdb)
- `tomcat`: 8.5.31, 9.0.8
@red-defender
Copy link

Pretty good, you've broken our initialization scripts used TCP connection. And we were forced to find the reason why our builds become failed in registry for about two hours...

Yes, we have taken unix socket as a workaround, BUT, there's no "local trust" entries in default pg_hba file, and we use other roles besides postgres, so the only fix is to add missing lines manually:

{
    echo
    echo 'local all all trust'
} >> "$PGDATA/pg_hba.conf"
pg_ctl -D "$PGDATA" reload

@yosifkit
Copy link
Member

@red-defender, there is local trust by default. Here is the contents of the default pg_hba.conf:

# PostgreSQL Client Authentication Configuration File
# ===================================================
#
# Refer to the "Client Authentication" section in the PostgreSQL
# documentation for a complete description of this file.  A short
# synopsis follows.
#
# This file controls: which hosts are allowed to connect, how clients
# are authenticated, which PostgreSQL user names they can use, which
# databases they can access.  Records take one of these forms:
#
# local      DATABASE  USER  METHOD  [OPTIONS]
# host       DATABASE  USER  ADDRESS  METHOD  [OPTIONS]
# hostssl    DATABASE  USER  ADDRESS  METHOD  [OPTIONS]
# hostnossl  DATABASE  USER  ADDRESS  METHOD  [OPTIONS]
#
# (The uppercase items must be replaced by actual values.)
#
# The first field is the connection type: "local" is a Unix-domain
# socket, "host" is either a plain or SSL-encrypted TCP/IP socket,
# "hostssl" is an SSL-encrypted TCP/IP socket, and "hostnossl" is a
# plain TCP/IP socket.
#
# DATABASE can be "all", "sameuser", "samerole", "replication", a
# database name, or a comma-separated list thereof. The "all"
# keyword does not match "replication". Access to replication
# must be enabled in a separate record (see example below).
#
# USER can be "all", a user name, a group name prefixed with "+", or a
# comma-separated list thereof.  In both the DATABASE and USER fields
# you can also write a file name prefixed with "@" to include names
# from a separate file.
#
# ADDRESS specifies the set of hosts the record matches.  It can be a
# host name, or it is made up of an IP address and a CIDR mask that is
# an integer (between 0 and 32 (IPv4) or 128 (IPv6) inclusive) that
# specifies the number of significant bits in the mask.  A host name
# that starts with a dot (.) matches a suffix of the actual host name.
# Alternatively, you can write an IP address and netmask in separate
# columns to specify the set of hosts.  Instead of a CIDR-address, you
# can write "samehost" to match any of the server's own IP addresses,
# or "samenet" to match any address in any subnet that the server is
# directly connected to.
#
# METHOD can be "trust", "reject", "md5", "password", "scram-sha-256",
# "gss", "sspi", "ident", "peer", "pam", "ldap", "radius" or "cert".
# Note that "password" sends passwords in clear text; "md5" or
# "scram-sha-256" are preferred since they send encrypted passwords.
#
# OPTIONS are a set of options for the authentication in the format
# NAME=VALUE.  The available options depend on the different
# authentication methods -- refer to the "Client Authentication"
# section in the documentation for a list of which options are
# available for which authentication methods.
#
# Database and user names containing spaces, commas, quotes and other
# special characters must be quoted.  Quoting one of the keywords
# "all", "sameuser", "samerole" or "replication" makes the name lose
# its special character, and just match a database or username with
# that name.
#
# This file is read on server startup and when the server receives a
# SIGHUP signal.  If you edit the file on a running system, you have to
# SIGHUP the server for the changes to take effect, run "pg_ctl reload",
# or execute "SELECT pg_reload_conf()".
#
# Put your actual configuration here
# ----------------------------------
#
# If you want to allow non-local connections, you need to add more
# "host" records.  In that case you will also need to make PostgreSQL
# listen on a non-local interface via the listen_addresses
# configuration parameter, or via the -i or -h command line switches.

# CAUTION: Configuring the system for local "trust" authentication
# allows any local user to connect as any PostgreSQL user, including
# the database superuser.  If you do not trust all your local users,
# use another authentication method.


# TYPE  DATABASE        USER            ADDRESS                 METHOD

# "local" is for Unix domain socket connections only
local   all             all                                     trust
# IPv4 local connections:
host    all             all             127.0.0.1/32            trust
# IPv6 local connections:
host    all             all             ::1/128                 trust
# Allow replication connections from localhost, by a user with the
# replication privilege.
local   replication     all                                     trust
host    replication     all             127.0.0.1/32            trust
host    replication     all             ::1/128                 trust

host all all all md5

(The last two lines being added by the entrypoint when a password is supplied via POSTGRES_PASSWORD.)

@red-defender
Copy link

@yosifkit Really, you are right, I'm used to production installations. 😇

@sdwolfz
Copy link
Contributor Author

sdwolfz commented May 13, 2018

@nmpacheco I just tried this locally with the latest postgres:

docker run --rm -it -e POSTGRES_USER=newuser POSTGRES_DB=newdb postgres

Then inside the container I executed:

psql -U newuser -d newdb

And inside psql I listed the databases:

newdb=# \l
                                 List of databases
   Name    |  Owner   | Encoding |  Collate   |   Ctype    |   Access privileges
-----------+----------+----------+------------+------------+-----------------------
 newdb     | postgres | UTF8     | en_US.utf8 | en_US.utf8 |
 postgres  | postgres | UTF8     | en_US.utf8 | en_US.utf8 |
 template0 | postgres | UTF8     | en_US.utf8 | en_US.utf8 | =c/postgres          +
           |          |          |            |            | postgres=CTc/postgres
 template1 | postgres | UTF8     | en_US.utf8 | en_US.utf8 | =c/postgres          +
           |          |          |            |            | postgres=CTc/postgres
(4 rows)

Are you doing some other initlaizations in your image? Maybe this helps you:
#441 (comment)

@nmpacheco
Copy link

@sdwolfz you're right! I was using a swarm stack compose file that also sets PGHOST to 127.0.0.1 as part of the environment for the postgres service. I'll have to check why I need that.
Thanks!

@zimmicz
Copy link

zimmicz commented Dec 17, 2019

How are you supposed to connect to postgres_fdw as a non-superuser during init phase when postgres only listens on socket?

abretaud added a commit to abretaud/chado-schema-builder that referenced this pull request Apr 4, 2022
abretaud added a commit to galaxy-genome-annotation/chado-schema-builder that referenced this pull request Apr 4, 2022
@fixermark
Copy link

Note: I believe that unfortunately, this change significantly complicates using the Tiger loader in PostGIS (https://postgis.net/docs/Loader_Generate_Script.html), which emits a shell script that only knows how to connect to the server via TCP/IP.

Probably too many layers of indirection removed from this project to care, but noting it on this merge so that other people who encounter the issue may be aware.

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.

None yet