docker-zulip
docker-zulip copied to clipboard
Passphrase configuration is limited
The things which currently go wrong
- [ ] Docker Compose tries to dereference any passphrase containing
$foo
and requires$$foo
- [x] The default passphrases are wrapped in double quotes, which require escapes in YAML
- [ ] After switching to single quotes, then single quotes within the passphrases need to be doubled
''
- [ ] Single quotes, even when doubled, then fail to pass through to Postgres when it creates the initial user
- [x]
~
isn't permitted, due to a regex assumption withinentrypoint.sh
- [ ] Setting new passphrases requires changing a git-tracked file
Things which can be fixed
- [x] Passphrases should be escaped when they're sent to Postgres and other services
- [x] The regex assumption should be replaced with something which doesn't care about the value
- [ ] The passphrases should be read from env vars and handled entirely by the user; no more changes to git-tracked files should be needed (this encourages people to commit the passphrases to git)
- [ ] We should invite and encourage people, in our docs, to use the
.env
file which docker-compose reads; docs here - [ ] The variables used for passphrases should fail if not supplied; i.e.
SECRETS_secret_key: "${SECRETS_secret_key:?err}"
This fixes the regex issue 78480d4de548c6fccfc51d5434d2ad143db5f736 (and also makes the code a lot cleaner).
Single quotes are obviously the better choice than double-quotes in YAML; just merged b2bb979b4ef632b2369045c4358b03af5c107671 to migrate the project.
For
 The variables used for passphrases should fail if not supplied; i.e.
SECRETS_secret_key: "${SECRETS_secret_key:?err}"
It seems to me that we should be making a set variable that's empty fatal here:
for SECRET_KEY in "${SECRETS[@]}"; do
local key="SECRETS_$SECRET_KEY"
local SECRET_VAR="${!key}"
if [ -z "$SECRET_VAR" ]; then
echo "Empty secret for key \"$SECRET_KEY\"."
fi
crudini --set "$DATA_DIR/zulip-secrets.conf" "secrets" "${SECRET_KEY}" "${SECRET_VAR}"
done
@galexrt what's the background here?
"${SECRETS_secret_key:?err}"
is fatal (as per the ?err
), as described here.
@jeaye for this one:
 Passphrases should be escaped when they're sent to Postgres and other services
is the block that needs escaping this, or something else?
export PGPASSWORD="$SECRETS_postgres_password"
local TIMEOUT=60
echo "Waiting for database server to allow connections ..."
while ! /usr/bin/pg_isready -h "$DB_HOST" -p "$DB_HOST_PORT" -U "$DB_USER" -t 1 >/dev/null 2>&1
Not sure anymore, but that looks likely. Adding a '
to the middle of your passphrase should raise the issue.
"${SECRETS_secret_key:?err}" is fatal (as per the ?err), as described here.
Understood, I'm just noticing we seem to have code checking for blank secrets and just printing a line of output (that surely gets lost in the noise), and I'm curious why.
On the database front, it looks like it fails here:
database_1 | 2018-08-23 20:20:50.925 UTC [66] ERROR: unrecognized role option "fun" at character 52
database_1 | 2018-08-23 20:20:50.925 UTC [66] STATEMENT: CREATE USER "zulip" WITH SUPERUSER PASSWORD 'zulip'fun' ;
database_1 | ERROR: unrecognized role option "fun"
database_1 | LINE 1: CREATE USER "zulip" WITH SUPERUSER PASSWORD 'zulip'fun' ;
database_1 | ^
That's going to be an upstream issue in https://github.com/docker-library/postgres/. We can report an issue upstream, but I think probably our solution should just be to document that certain characters are invalid in a comment in docker-compose.yml
; what do you think @jeaye?
Glad you could repro. Seems like reporting it upstream and making it clear within docker-zulip is the best we can do for now, unless it's a quick fix. If we provided a tool for generating passphrases into .env
, we could take care of this for the user.
Agreed re: reporting upstream; can you take reporting this one? And yeah, if we're providing a tool, we can just have the tool not use $
and '
.
Done.
https://github.com/docker-library/postgres/issues/488 has been resolved.
Awesome! I guess we need to wait for them to do a release before we can take advantage of this?
Yeah, I'm not sure how that works. This was the first time I'd ever used Docker.
https://hub.docker.com/_/postgres/ appears to have been last pushed to 7 hours ago, so maybe that means we can grab the latest image and we'll be OK? Worth testing...