Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions 9.2/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
FROM centos:centos7
MAINTAINER Martin Nagy <mnagy@redhat.com>

# PostgreSQL image for OpenShift.
# Volumes:
# * /var/lib/psql/data - Database cluster for PostgreSQL
# Environment:
# * $POSTGRESQL_USER - Database user name
# * $POSTGRESQL_PASSWORD - User's password
# * $POSTGRESQL_DATABASE - Name of the database to create
# * $POSTGRESQL_ADMIN_PASSWORD (Optional) - Password for the 'postgres'
# PostgreSQL administrative account

# Image metadata
ENV POSTGRESQL_VERSION 9.2
ENV IMAGE_DESCRIPTION PostgreSQL 9.2
ENV IMAGE_TAGS postgresql,postgresql92
ENV IMAGE_EXPOSE_SERVICES 5432:postgresql

EXPOSE 5432

# This image must forever use UID 26 for postgres user so our volumes are
# safe in the future. This should *never* change, the last test is there
# to make sure of that.
RUN rpmkeys --import file:///etc/pki/rpm-gpg/RPM-GPG-KEY-CentOS-7 && \
yum -y --setopt=tsflags=nodocs install https://www.softwarecollections.org/en/scls/rhscl/postgresql92/epel-7-x86_64/download/rhscl-postgresql92-epel-7-x86_64.noarch.rpm && \
yum -y --setopt=tsflags=nodocs install postgresql92 && \
yum clean all && \
mkdir -p /var/lib/pgsql/data && chown postgres.postgres /var/lib/pgsql/data && \
test "$(id postgres)" = "uid=26(postgres) gid=26(postgres) groups=26(postgres)"

COPY run-postgresql.sh /usr/local/bin/
COPY contrib/.bashrc /var/lib/pgsql/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even more, can you add that to all our images 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@soltysh For mysql, see sclorg/mysql-container#24
For MongoDB, @jhadvig will do that. :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

many kudos @mnagy

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@soltysh thanks :) I'd also appreciate a certain four-letter acronym :P


VOLUME ["/var/lib/pgsql/data"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not have /var/lib/pgsql as VOLUME? Then you can persistently store your config files

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/var/lib/pgsql/data is a commonly used path for a database cluster, which also contains within the configuration that is necessary, so config files are persistently stored. It didn't make much sense to me to use /var/lib/pgsql. Besides, I think that while experimenting I did see some psql history files there, or something like that. Probably not relevant to us, but stil..


USER postgres

ENTRYPOINT ["run-postgresql.sh"]
CMD ["postgres"]
41 changes: 41 additions & 0 deletions 9.2/Dockerfile.rhel7
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
FROM rhel7
MAINTAINER Martin Nagy <mnagy@redhat.com>

# PostgreSQL image for OpenShift.
# Volumes:
# * /var/lib/psql/data - Database cluster for PostgreSQL
# Environment:
# * $POSTGRESQL_USER - Database user name
# * $POSTGRESQL_PASSWORD - User's password
# * $POSTGRESQL_DATABASE - Name of the database to create
# * $POSTGRESQL_ADMIN_PASSWORD (Optional) - Password for the 'postgres'
# PostgreSQL administrative account

# Image metadata
ENV POSTGRESQL_VERSION 9.2
ENV IMAGE_DESCRIPTION PostgreSQL 9.2
ENV IMAGE_TAGS postgresql,postgresql92
ENV IMAGE_EXPOSE_SERVICES 5432:postgresql

EXPOSE 5432

# This image must forever use UID 26 for postgres user so our volumes are
# safe in the future. This should *never* change, the last test is there
# to make sure of that.
RUN yum install -y yum-utils && \
yum-config-manager --enable rhel-server-rhscl-7-rpms && \
yum-config-manager --enable rhel-7-server-optional-rpms && \
yum install -y --setopt=tsflags=nodocs postgresql92 && \
yum clean all && \
mkdir -p /var/lib/pgsql/data && chown postgres.postgres /var/lib/pgsql/data && \
test "$(id postgres)" = "uid=26(postgres) gid=26(postgres) groups=26(postgres)"

COPY run-postgresql.sh /usr/local/bin/
COPY contrib/.bashrc /var/lib/pgsql/

VOLUME ["/var/lib/pgsql/data"]

USER postgres

ENTRYPOINT ["run-postgresql.sh"]
CMD ["postgres"]
2 changes: 2 additions & 0 deletions 9.2/contrib/.bashrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# This will make scl collection binaries work out of box.
source scl_source enable postgresql92
91 changes: 91 additions & 0 deletions 9.2/run-postgresql.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
#!/bin/bash

# For SCL enablement
source $HOME/.bashrc

set -eu

# Data dir
export PGDATA=/var/lib/pgsql/data

# Be paranoid and stricter than we should be.
psql_identifier_regex='^[a-zA-Z_][a-zA-Z0-9_]*$'
psql_password_regex='^[a-zA-Z0-9_~!@#$%^&*()-=<>,.?;:|]+$'

function usage() {
if [ $# == 2 ]; then
echo "error: $1"
fi
echo "You must specify following environment variables:"
echo " \$POSTGRESQL_USER (regex: '$psql_identifier_regex')"
echo " \$POSTGRESQL_PASSWORD (regex: '$psql_password_regex')"
echo " \$POSTGRESQL_DATABASE (regex: '$psql_identifier_regex')"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In MongoDB I am creating the production database if none is defined. I think some similar approach we had in v2. Wouldn't it be better to have it in all DB images (also MySQL) ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather see a failure when that name isn't specified. It's required the
same way username and password is. This is the behavior I'd vote for all
images.

On 6:27pm, Thu, Mar 19, 2015 Jakub Hadvig notifications@github.com wrote:

In 9.2/run-postgresql.sh
#1 (comment):

@@ -0,0 +1,83 @@
+#!/bin/bash -e
+
+# Be paranoid and stricter than we should be.
+psql_identifier_regex='^[a-zA-Z_][a-zA-Z0-9_]$'
+psql_password_regex='^[a-zA-Z0-9
~!@#$%^&_()-=<>,.?;:|]+$'
+
+function usage {

  • echo "You must specify following environment variables:"
  • echo " $POSTGRESQL_USER (regex: '$psql_identifier_regex')"
  • echo " $POSTGRESQL_PASSWORD (regex: '$psql_password_regex')"
  • echo " $POSTGRESQL_DATABASE (regex: '$psql_identifier_regex')"

In MongoDB I am creating the production database if none is defined. I
think some similar approach we had in v2. Wouldn't it be better to have it
in all DB images (also MySQL) ?


Reply to this email directly or view it on GitHub
https://github.com/openshift/postgresql/pull/1/files#r26777138.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we discussed somewhere for MySQL image that we don't want optional vars and that we recommend that in the image writing guide, that's why it's not optional.

Personally, I'd be in favour of that, it would make using the image outside Openshift a LOT easier.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont recall any discussion about optional vars. Also the MYSQL_ROOT_PASSWORD var is optional, is it not ?
I would go with similar behavior as the MongoDB, less typing => happy users :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm. We can talk about that on scrum I suppose?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

database name is required, always... we need to fix mongo

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jhadvig also, MYSQL_ROOT_PASSWORD is a special case since it doesn't have a default but not specifying it alters the behavior (remote login to root account is disabled).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you docker exec to running container and run psql, does it connect to postres? it should without having to specify any arguments... please test that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll get command not found error because of SCL :) But yeah, if you run it through scl, psql will work as is.

echo "Optional:"
echo " \$POSTGRESQL_ADMIN_PASSWORD (regex: '$psql_password_regex')"
exit 1
}

function check_env_vars() {
[[ "$POSTGRESQL_USER" =~ $psql_identifier_regex ]] || usage
[ ${#POSTGRESQL_USER} -le 63 ] || usage "PostgreSQL username too long (maximum 63 characters)"
[[ "$POSTGRESQL_PASSWORD" =~ $psql_password_regex ]] || usage
[[ "$POSTGRESQL_DATABASE" =~ $psql_identifier_regex ]] || usage
[ ${#POSTGRESQL_DATABASE} -le 63 ] || usage "Database name too long (maximum 63 characters)"
if [ -v POSTGRESQL_ADMIN_PASSWORD ]; then
[[ "$POSTGRESQL_ADMIN_PASSWORD" =~ $psql_password_regex ]] || usage
fi
}

# Make sure env variables don't propagate to PostgreSQL process.
function unset_env_vars() {
unset POSTGRESQL_USER
unset POSTGRESQL_PASSWORD
unset POSTGRESQL_DATABASE
unset POSTGRESQL_ADMIN_PASSWORD
}

if [ "$1" = "postgres" -a ! -f "$PGDATA/postgresql.conf" ]; then

check_env_vars

# Initialize the database cluster with utf8 support enabled by default.
# This might affect performance, see:
# http://www.postgresql.org/docs/9.2/static/locale.html
LANG=${LANG:-en_US.utf8} initdb

# PostgreSQL configuration.
cat >> "$PGDATA/postgresql.conf" <<-EOF

#
# Custom OpenShift configuration starting at this point.
#

# Listen on all interfaces.
listen_addresses = '*'
EOF

# Access control configuration.
cat >> "$PGDATA/pg_hba.conf" <<-EOF

#
# Custom OpenShift configuration starting at this point.
#

# Allow connections from all hosts.
host all all all md5
EOF

pg_ctl -w start
createuser "$POSTGRESQL_USER"
createdb --owner="$POSTGRESQL_USER" "$POSTGRESQL_DATABASE"
psql --command "ALTER USER \"${POSTGRESQL_USER}\" WITH ENCRYPTED PASSWORD '${POSTGRESQL_PASSWORD}';"

if [ -v POSTGRESQL_ADMIN_PASSWORD ]; then
psql --command "ALTER USER \"postgres\" WITH ENCRYPTED PASSWORD '${POSTGRESQL_ADMIN_PASSWORD}';"
fi

pg_ctl stop
fi

unset_env_vars
exec "$@"
169 changes: 169 additions & 0 deletions 9.2/test/run
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
#!/bin/bash
#
# Test the PostgreSQL image.
#
# IMAGE_NAME specifies the name of the candidate image used for testing.
# The image has to be available before this script is executed.
#

set -exo nounset
shopt -s nullglob

IMAGE_NAME=${IMAGE_NAME-openshift/postgresql-92-centos7-candidate}

CIDFILE_DIR=$(mktemp --suffix=postgresql_test_cidfiles -d)

function cleanup() {
for cidfile in $CIDFILE_DIR/* ; do
CONTAINER=$(cat $cidfile)

echo "Stopping and removing container $CONTAINER..."
docker stop $CONTAINER
exit_status=$(docker inspect -f '{{.State.ExitCode}}' $CONTAINER)
if [ "$exit_status" != "0" ]; then
echo "Dumping logs for $CONTAINER"
docker logs $CONTAINER
fi
docker rm $CONTAINER
rm $cidfile
echo "Done."
done
rmdir $CIDFILE_DIR
}
trap cleanup EXIT
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add SIGINT

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I take that back; it now calls cleanup two times. EXIT trap is invoked after an interrupt as well.


function get_cid() {
local id="$1" ; shift || return 1
echo $(cat "$CIDFILE_DIR/$id")
}

function get_container_ip() {
local id="$1" ; shift
docker inspect --format='{{.NetworkSettings.IPAddress}}' $(get_cid "$id")
}

function postgresql_cmd() {
docker run --rm -e PGPASSWORD="${PASS}" $IMAGE_NAME psql postgresql://$USER@$CONTAINER_IP:5432/db "$@"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This uses PASS and USER as global variables, so we're never actually testing the access of the admin user...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you open a bug report against this repo, so it doesn't get forgotten?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wrong. In fact, what happens is that assert_login_access shadows USER and PASS with local variables of the same name, and because in bash local variables are visible in the scope of a function and its children, postgresql_cmd eventually uses those local variables when called from within assert_login_access.

Needless to say, that's very very error prone and hard to reason about...

}

function test_connection() {
local name=$1 ; shift
ip=$(get_container_ip $name)
echo " Testing PostgreSQL connection to $ip..."
local max_attempts=20
local sleep_time=2
for i in $(seq $max_attempts); do
echo " Trying to connect..."
set +e
postgresql_cmd <<< "SELECT 1;"
status=$?
set -e
if [ $status -eq 0 ]; then
echo " Success!"
return 0
fi
sleep $sleep_time
done
return 1
}

function test_postgresql() {
echo " Testing PostgreSQL"
postgresql_cmd <<< "CREATE TABLE tbl (col1 VARCHAR(20), col2 VARCHAR(20));"
postgresql_cmd <<< "INSERT INTO tbl VALUES ('foo1', 'bar1');"
postgresql_cmd <<< "INSERT INTO tbl VALUES ('foo2', 'bar2');"
postgresql_cmd <<< "INSERT INTO tbl VALUES ('foo3', 'bar3');"
postgresql_cmd <<< "SELECT * FROM tbl;"
postgresql_cmd <<< "DROP TABLE tbl;"
echo " Success!"
}

function create_container() {
local name=$1 ; shift
cidfile="$CIDFILE_DIR/$name"
# create container with a cidfile in a directory for cleanup
docker run --cidfile $cidfile -d "$@" $IMAGE_NAME
echo "Created container $(cat $cidfile)"
}

function assert_login_access() {
local USER=$1 ; shift
local PASS=$1 ; shift
local success=$1 ; shift

if $success; then
postgresql_cmd <<< "SELECT 1;" &&
echo " $USER($PASS) access granted as expected"
else
postgresql_cmd <<< "SELECT 1;" ||
echo " $USER($PASS) access denied as expected"
fi
}

# Make sure the invocation of docker run fails.
function assert_container_creation_fails() {

# Time the docker run command. It should fail. If it doesn't fail,
# postgresql will keep running so we kill it with SIGKILL to make sure
# timeout returns a non-zero value.
set +e
timeout -s 9 --preserve-status 60s docker run --rm "$@" $IMAGE_NAME
ret=$?
set -e

# Timeout will exit with a high number.
if [ $ret -gt 30 ]; then
return 1
fi
}

function try_image_invalid_combinations() {
assert_container_creation_fails "$@"
assert_container_creation_fails -e POSTGRESQL_USER=user -e POSTGRESQL_PASSWORD=pass "$@"
assert_container_creation_fails -e POSTGRESQL_USER=user -e POSTGRESQL_DATABASE=db "$@"
assert_container_creation_fails -e POSTGRESQL_PASSWORD=pass -e POSTGRESQL_DATABASE=db "$@"
}

function run_container_creation_tests() {
echo " Testing image entrypoint usage"
try_image_invalid_combinations
try_image_invalid_combinations -e POSTGRESQL_ADMIN_PASSWORD=admin_pass

VERY_LONG_IDENTIFIER="very_long_identifier_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
assert_container_creation_fails -e POSTGRESQL_USER=0invalid -e POSTGRESQL_PASSWORD=pass -e POSTGRESQL_DATABASE=db -e POSTGRESQL_ADMIN_PASSWORD=admin_pass
assert_container_creation_fails -e POSTGRESQL_USER=$VERY_LONG_IDENTIFIER -e POSTGRESQL_PASSWORD=pass -e POSTGRESQL_DATABASE=db -e POSTGRESQL_ADMIN_PASSWORD=admin_pass
assert_container_creation_fails -e POSTGRESQL_USER=user -e POSTGRESQL_PASSWORD="\"" -e POSTGRESQL_DATABASE=db -e POSTGRESQL_ADMIN_PASSWORD=admin_pass
assert_container_creation_fails -e POSTGRESQL_USER=user -e POSTGRESQL_PASSWORD=pass -e POSTGRESQL_DATABASE=9invalid -e POSTGRESQL_ADMIN_PASSWORD=admin_pass
assert_container_creation_fails -e POSTGRESQL_USER=user -e POSTGRESQL_PASSWORD=pass -e POSTGRESQL_DATABASE=$VERY_LONG_IDENTIFIER -e POSTGRESQL_ADMIN_PASSWORD=admin_pass
assert_container_creation_fails -e POSTGRESQL_USER=user -e POSTGRESQL_PASSWORD=pass -e POSTGRESQL_DATABASE=db -e POSTGRESQL_ADMIN_PASSWORD="\""
echo " Success!"
}

function run_tests() {
local name=$1 ; shift
envs="-e POSTGRESQL_USER=$USER -e POSTGRESQL_PASSWORD=$PASS -e POSTGRESQL_DATABASE=db"
if [ -v ADMIN_PASS ]; then
envs="$envs -e POSTGRESQL_ADMIN_PASSWORD=$ADMIN_PASS"
fi
create_container $name $envs
CONTAINER_IP=$(get_container_ip $name)
test_connection $name
echo " Testing login accesses"
assert_login_access $USER $PASS true
assert_login_access $USER "${PASS}_foo" false
if [ -v ADMIN_PASS ]; then
assert_login_access postgres $ADMIN_PASS true
assert_login_access postgres "${ADMIN_PASS}_foo" false
else
assert_login_access postgres "foo" false
assert_login_access postgres "" false
fi
echo " Success!"
test_postgresql $name
}

# Tests.

run_container_creation_tests
USER=user PASS=pass run_tests no_admin
USER=user1 PASS=pass1 ADMIN_PASS=r00t run_tests admin
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should be also testing the admin user, not only standalone user.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add in access tests that we do in mysql.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also test failures ... no user set, etc...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean no env vars set when creating the container? Please elaborate.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

invalid values, empty values.... exercise the validate functions in image

20 changes: 20 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@

ifeq ($(TARGET),rhel7)
OS := rhel7
else
OS := centos7
endif

ifeq ($(VERSION), 9.2)
VERSION := 9.2
else
VERSION :=
endif

.PHONY: build
build:
hack/build.sh $(OS) $(VERSION)

.PHONY: test
test:
TEST_MODE=true hack/build.sh $(OS) $(VERSION)
Loading