fix to pooler TLS support

[2tvenom]

last PR #2216 checked and not work. added security context fsGroup for correct load tls cert from mounted volume

[FxKu]

Thanks @2tvenom . Does it work for you with this fix? Do we need the symbolic links in pgBounder Docker file entrypoint.sh as mentioned here?

[2tvenom]

i checked it with next configs

entrypoint.sh

#!/bin/sh

set -ex

if [ "$PGUSER" = "postgres" ]; then
    echo "WARNING: pgbouncer will connect with a superuser privileges!"
    echo "You need to fix this as soon as possible."
fi

if [ -z "${CONNECTION_POOLER_CLIENT_TLS_CRT}" ]; then
    openssl req -nodes -new -x509 -subj /CN=spilo.dummy.org \
        -keyout /etc/ssl/certs/pgbouncer.key \
        -out /etc/ssl/certs/pgbouncer.crt
else
    ln -s /tls/tls.crt /etc/ssl/certs/pgbouncer.crt
    ln -s /tls/tls.key /etc/ssl/certs/pgbouncer.key
    ln -s /tls/ca.crt /etc/ssl/certs/ca.crt
fi

envsubst < /etc/pgbouncer/pgbouncer.ini.tmpl > /etc/pgbouncer/pgbouncer.ini
envsubst < /etc/pgbouncer/auth_file.txt.tmpl > /etc/pgbouncer/auth_file.txt

exec /bin/pgbouncer /etc/pgbouncer/pgbouncer.ini

pgbouncer.ini.tmpl

# vim: set ft=dosini:

[databases]
* = host=$PGHOST port=$PGPORT auth_user=$PGUSER

[pgbouncer]
pool_mode = $CONNECTION_POOLER_MODE
listen_port = $CONNECTION_POOLER_PORT
listen_addr = *
auth_type = md5
auth_file = /etc/pgbouncer/auth_file.txt
admin_users = $PGUSER
stats_users_prefix = robot_
auth_query = SELECT * FROM $PGSCHEMA.user_lookup($1)
logfile = /var/log/pgbouncer/pgbouncer.log
pidfile = /var/run/pgbouncer/pgbouncer.pid

server_tls_sslmode = verify-ca
server_tls_ca_file = /etc/ssl/certs/ca.crt
server_tls_key_file = /etc/ssl/certs/pgbouncer.key
server_tls_cert_file = /etc/ssl/certs/pgbouncer.crt
server_tls_protocols = secure

client_tls_sslmode = verify-ca
client_tls_key_file = /etc/ssl/certs/pgbouncer.key
client_tls_cert_file = /etc/ssl/certs/pgbouncer.crt
client_tls_ca_file = /etc/ssl/certs/ca.crt

log_connections = 0
log_disconnections = 0

# How many server connections to allow per user/database pair.
default_pool_size = $CONNECTION_POOLER_DEFAULT_SIZE

# Add more server connections to pool if below this number. Improves behavior
# when usual load comes suddenly back after period of total inactivity.
#
# NOTE: This value is per pool, i.e. a pair of (db, user), not a global one.
# Which means on the higher level it has to be calculated from the max allowed
# database connections and number of databases and users. If not taken into
# account, then for too many users or databases PgBouncer will go crazy
# opening/evicting connections. For now disable it.
#
# min_pool_size = $CONNECTION_POOLER_MIN_SIZE

# How many additional connections to allow to a pool
reserve_pool_size = $CONNECTION_POOLER_RESERVE_SIZE

# Maximum number of client connections allowed.
max_client_conn = $CONNECTION_POOLER_MAX_CLIENT_CONN

# Do not allow more than this many connections per database (regardless of
# pool, i.e. user)
max_db_connections = $CONNECTION_POOLER_MAX_DB_CONN

# If a client has been in "idle in transaction" state longer, it will be
# disconnected. [seconds]
idle_transaction_timeout = 600

# If login failed, because of failure from connect() or authentication that
# pooler waits this much before retrying to connect. Default is 15. [seconds]
server_login_retry = 5

# To ignore extra parameter in startup packet. By default only 'database' and
# 'user' are allowed, all others raise error.  This is needed to tolerate
# overenthusiastic JDBC wanting to unconditionally set 'extra_float_digits=2'
# in startup packet.
ignore_startup_parameters = extra_float_digits,options,search_path

[FxKu]

Ok, you have one more line in the entrypoint script ln -s /tls/ca.crt /etc/ssl/certs/ca.crt which makes me realize that the operator does not support ca files for connection pooler like we do with spilo pods. I feel this code should be unified.

[2tvenom]

this endpoint show what this worked with ln -s
i can check with standard endpoint.sh

[FxKu]

What I meant is that I can change the entrypoint of our pgBouncer docker image to:

...
    ln -s ${CONNECTION_POOLER_CLIENT_TLS_CRT} /etc/ssl/certs/pgbouncer.crt
    ln -s ${CONNECTION_POOLER_CLIENT_TLS_KEY} /etc/ssl/certs/pgbouncer.key
    if [ -v "${CONNECTION_POOLER_CLIENT_CA_FILE}" ]; then
        ln -s ${CONNECTION_POOLER_CLIENT_CA_FILE} /etc/ssl/certs/ca.crt
    fi
...

But the operator does not set this CONNECTION_POOLER_CLIENT_CA_FILE environment variable, yet. Maybe you can add support for it with this PR.

[2tvenom]

@FxKu done

[FxKu]

Thanks, but the code looks quite different to what we do with spilo pods. CA should not be set when it's not defined in the manifest. For spilo pods we also use a second volume mount (tlsca). Can't remember why we did that back then.

[2tvenom]

@FxKu done

[FxKu]

I believe, we need a second volume mount when introducing CA file support. Hm ... can you remove everything about CA file support from this PR as it looks pretty half baked at the moment and not tested.

[2tvenom]

i will mount CA

[FxKu]

Why should it get the spilo FS group btw? Can it also get it's own? Would an extra config option make sense?

[2tvenom]

you mean additional fsGroup in CRD pooler block?

[FxKu]

There could be extra settings in the pooler struct, yes. But, on the other hand we're already re-using TLS settings from postgresql.spec - so I'm fine by copying SpiloFSGroup, too, as it would be convenient to users of this feature. No extra config necessary in the manifest.

Was just wondering if somebody might want this FSGroup setting to be different from spilo...

[FxKu]

👍

[idanovinda]

👍

[FxKu]

Ok @2tvenom. Thanks for your input so far. I will take it from here 😃