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.

Sign up for GitHub

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

envsubst: remove explicit lists of vars #818

Closed
wants to merge 1 commit into from
Closed

envsubst: remove explicit lists of vars #818

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion files/enketo/start-enketo.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ BASE_URL=$( [ "${HTTPS_PORT}" = 443 ] && echo https://"${DOMAIN}" || echo https:
SECRET=$(cat /etc/secrets/enketo-secret) \
LESS_SECRET=$(cat /etc/secrets/enketo-less-secret) \
API_KEY=$(cat /etc/secrets/enketo-api-key) \
envsubst '$DOMAIN $BASE_URL $SECRET $LESS_SECRET $API_KEY $SUPPORT_EMAIL' \
envsubst \
< "$CONFIG_PATH.template" \
> "$CONFIG_PATH"

Expand Down
9 changes: 8 additions & 1 deletion files/nginx/setup-odk.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
#!/bin/bash

nginx_envsubst() {
# Re-implementation of envsubst which is safe to call on nginx config files.
# This fn only substitutes variables in the form ${CAPS_AND_UNDERSCORES},
# allowing nginx variables like $host, $request_uri etc. through unmodified.
perl -pe 's/\$\{([A-Z_]*)\}/$ENV{$1}/g'
}
Comment on lines +3 to +8
Copy link
Contributor

Choose a reason for hiding this comment

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

So, this silently replaces with a 0-length string any variable in the template which does not exist in the environment. Envsubst does that too, so that's the original sin: it makes for hard(er) to debug problems. Depending on the config language and construct it might well make for a syntactically valid config file with an undesirable effect at runtime, not even at initalization time, of the program using that configfile.

See

$ echo -e 'hello\n${I_WILL_BE_SILENTLY_DISCARDED}\n${SHELL}' | perl -pe 's/\$\{([A-Z_]*)\}/$ENV{$1}/g'
hello

/bin/zsh

I think it would be better if it would simply bomb out if the key is not in the env (in Python os.environ['blabla'] would raise a KeyError). At least then it'd be very obvious that something is missing, and what that thing is, right at the moment when it is generated, rather than that something mysteriously goes wrong at runtime.

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 handling missing vars would be a much better aim than trying to remove the explicit list of vars 👍



echo "writing client config..."
if [[ $OIDC_ENABLED != 'true' ]] && [[ $OIDC_ENABLED != 'false' ]]; then
Expand Down Expand Up @@ -31,7 +38,7 @@ echo "writing fresh nginx templates..."
cp /usr/share/odk/nginx/redirector.conf /etc/nginx/conf.d/redirector.conf

CNAME=$( [ "$SSL_TYPE" = "customssl" ] && echo "local" || echo "$DOMAIN") \
envsubst '$SSL_TYPE $CNAME $SENTRY_ORG_SUBDOMAIN $SENTRY_KEY $SENTRY_PROJECT' \
nginx_envsubst \
< /usr/share/odk/nginx/odk.conf.template \
> /etc/nginx/conf.d/odk.conf

Expand Down
2 changes: 1 addition & 1 deletion files/service/scripts/start-odk.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ echo "generating local service configuration.."

ENKETO_API_KEY=$(cat /etc/secrets/enketo-api-key) \
BASE_URL=$( [ "${HTTPS_PORT}" = 443 ] && echo https://"${DOMAIN}" || echo https://"${DOMAIN}":"${HTTPS_PORT}" ) \
envsubst '$DOMAIN $BASE_URL $SYSADMIN_EMAIL $ENKETO_API_KEY $DB_HOST $DB_USER $DB_PASSWORD $DB_NAME $DB_SSL $EMAIL_FROM $EMAIL_HOST $EMAIL_PORT $EMAIL_SECURE $EMAIL_IGNORE_TLS $EMAIL_USER $EMAIL_PASSWORD $OIDC_ENABLED $OIDC_ISSUER_URL $OIDC_CLIENT_ID $OIDC_CLIENT_SECRET $SENTRY_ORG_SUBDOMAIN $SENTRY_KEY $SENTRY_PROJECT $S3_SERVER $S3_ACCESS_KEY $S3_SECRET_KEY $S3_BUCKET_NAME' \
envsubst \
< /usr/share/odk/config.json.template \
> /usr/odk/config/local.json

Expand Down
Loading