Skip to content

Commit

Permalink
envsubst: remove explicit subst of exported vars
Browse files Browse the repository at this point in the history
For nginx config, a new approach is implemented with mawk.

This is similar to envusbst, but more ergonomic:

* no need to explicitly list all variables
* throw error on missing variables
* do not replace nginx vars like $host, $request_uri with empty strings (in contrast to envsubst when executed without an explicit variable list)

Risks:

There are a couple of changes which may break existing deployments:

* changing client-config.json.template
* requiring all substituted variable to be defined

Closes getodk#473
  • Loading branch information
alxndrsn committed Dec 9, 2024
1 parent 515bfb9 commit af73093
Show file tree
Hide file tree
Showing 30 changed files with 173 additions and 15 deletions.
18 changes: 14 additions & 4 deletions .github/workflows/test-nginx.yml → .github/workflows/test.yml
Original file line number Diff line number Diff line change
@@ -1,11 +1,21 @@
name: Test nginx config
name: Tests

on:
push:
pull_request:

jobs:
build:
test-envsubst:
timeout-minutes: 2
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
fetch-depth: 0
fetch-tags: true
submodules: recursive
- run: cd test/envsub && ./run-tests.sh
test-nginx:
timeout-minutes: 10
runs-on: ubuntu-latest
steps:
Expand All @@ -17,8 +27,8 @@ jobs:
- uses: actions/setup-node@v4
with:
node-version: 20.17.0
- run: cd test && npm i
- run: cd test && ./run-tests.sh
- run: cd test/nginx && npm i
- run: cd test/nginx && ./run-tests.sh

- if: always()
run: docker logs test-nginx-1 || true
Expand Down
1 change: 1 addition & 0 deletions enketo.dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ WORKDIR ${ENKETO_SRC_DIR}
# care about anything the server needs. because the client config is baked at
# build time, we therefore hand it the untemplated config.

COPY files/shared/envsub.awk /scripts/
COPY files/enketo/config.json.template ${ENKETO_SRC_DIR}/config/config.json.template
COPY files/enketo/config.json.template ${ENKETO_SRC_DIR}/config/config.json
COPY files/enketo/start-enketo.sh ${ENKETO_SRC_DIR}/start-enketo.sh
Expand Down
2 changes: 1 addition & 1 deletion files/enketo/start-enketo.sh
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' \
/scripts/envsub.awk \
< "$CONFIG_PATH.template" \
> "$CONFIG_PATH"

Expand Down
2 changes: 1 addition & 1 deletion files/nginx/client-config.json.template
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
{
"oidcEnabled": $OIDC_ENABLED
"oidcEnabled": ${OIDC_ENABLED}
}
7 changes: 4 additions & 3 deletions files/nginx/setup-odk.sh
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
#!/bin/bash


echo "writing client config..."
if [[ $OIDC_ENABLED != 'true' ]] && [[ $OIDC_ENABLED != 'false' ]]; then
echo 'OIDC_ENABLED must be either true or false'
exit 1
fi

envsubst < /usr/share/odk/nginx/client-config.json.template > /usr/share/nginx/html/client-config.json
/scripts/envsub.awk \
< /usr/share/odk/nginx/client-config.json.template \
> /usr/share/nginx/html/client-config.json


DH_PATH=/etc/dh/nginx.pem
Expand All @@ -31,7 +32,7 @@ echo "writing fresh nginx templates..."
cp /usr/share/odk/nginx/redirector.conf /etc/nginx/conf.d/redirector.conf

CERT_DOMAIN=$( [ "$SSL_TYPE" = "customssl" ] && echo "local" || echo "$DOMAIN") \
envsubst '$SSL_TYPE $CERT_DOMAIN $DOMAIN $SENTRY_ORG_SUBDOMAIN $SENTRY_KEY $SENTRY_PROJECT' \
/scripts/envsub.awk \
< /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
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' \
/scripts/envsub.awk \
< /usr/share/odk/config.json.template \
> /usr/odk/config/local.json

Expand Down
39 changes: 39 additions & 0 deletions files/shared/envsub.awk
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
#!/usr/bin/mawk -f

# Safer implemention of envsubst.
#
# Significant differences:
#
# * require curly brackets around values
# * require uppercase
# * throw error if value is not defined, instead of just using empty string
#
# mawk is the awk implementation common to the containers we need to run this
# script in, so we use it here explicitly.

BEGIN {
errorCount = 0;
}

{
while(match($0, /\$\{[A-Z_][A-Z_0-9]*\}/) > 0) {
k = substr($0, RSTART+2, RLENGTH-3);
if(k in ENVIRON) {
v = ENVIRON[k];
} else {
print "ERR: var not defined on line " NR ": ${" k "}" > "/dev/fd/2";
++errorCount;
v = "!!!VALUE-MISSING: " k "!!!"
}
gsub("\$\{" k "\}", v);
}
print $0;
}

END {
if(errorCount > 0) {
print "" > "/dev/fd/2";
print errorCount " error(s) found." > "/dev/fd/2";
exit 1;
}
}
4 changes: 3 additions & 1 deletion nginx.dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ RUN apt-get update && apt-get install -y netcat-openbsd

RUN mkdir -p /usr/share/odk/nginx/

COPY files/nginx/setup-odk.sh /scripts/
COPY files/nginx/setup-odk.sh \
files/shared/envsub.awk \
/scripts/
RUN chmod +x /scripts/setup-odk.sh

COPY files/nginx/redirector.conf /usr/share/odk/nginx/
Expand Down
1 change: 1 addition & 0 deletions service.dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ RUN apt-get update \
--fund=false --update-notifier=false

COPY server/ ./
COPY files/shared/envsub.awk /scripts/
COPY files/service/scripts/ ./

COPY files/service/config.json.template /usr/share/odk/
Expand Down
3 changes: 3 additions & 0 deletions test/envsub/bad-example.in
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
${NOT_DEFINED_1} ${NOT_DEFINED_2}

${NOT_DEFINED_3}
5 changes: 5 additions & 0 deletions test/envsub/bad-example.stderr.expected
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
ERR: var not defined on line 1: ${NOT_DEFINED_1}
ERR: var not defined on line 1: ${NOT_DEFINED_2}
ERR: var not defined on line 3: ${NOT_DEFINED_3}

3 error(s) found.
3 changes: 3 additions & 0 deletions test/envsub/bad-example.stdout.expected
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
!!!VALUE-MISSING: NOT_DEFINED_1!!! !!!VALUE-MISSING: NOT_DEFINED_2!!!

!!!VALUE-MISSING: NOT_DEFINED_3!!!
20 changes: 20 additions & 0 deletions test/envsub/good-example.expected
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Alone:
sv_simple

# end of line
Simple value: sv_simple

# Missing curlies:
Should not be substituted: $SIMPLE

# Start of line:
sv_simple and then more content.

# This is not a sub variable:
$simple

# Two values on the same line:
first: sub_val_one, second: sub_val_two!

# Two values on the same line, both repeated:
first: sub_val_one, sub_val_one, sub_val_one, second: sub_val_two sub_val_two!
20 changes: 20 additions & 0 deletions test/envsub/good-example.in
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Alone:
${SIMPLE}

# end of line
Simple value: ${SIMPLE}

# Missing curlies:
Should not be substituted: $SIMPLE

# Start of line:
${SIMPLE} and then more content.

# This is not a sub variable:
$simple

# Two values on the same line:
first: ${SUBVAL_1}, second: ${SUBVAL_2}!

# Two values on the same line, both repeated:
first: ${SUBVAL_1}, ${SUBVAL_1}, ${SUBVAL_1}, second: ${SUBVAL_2} ${SUBVAL_2}!
50 changes: 50 additions & 0 deletions test/envsub/run-tests.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
#!/bin/bash -eu
log() { echo >&2 "[test/envsub] $*"; }

failCount=0

log "should correctly substitute provided values"
if diff <( \
SIMPLE=sv_simple \
SUBVAL_1=sub_val_one \
SUBVAL_2=sub_val_two \
../../files/shared/envsub.awk \
< good-example.in
) good-example.expected; then
log " OK"
else
((++failCount))
log " FAILED"
fi

log "should fail when asked to substitute undefined value"
if ! ../../files/shared/envsub.awk <<<"\${NOT_DEFINED}" >/dev/null 2>/dev/null; then
log " OK"
else
((++failCount))
log " FAILED"
fi

log "should log all issues when asked to substitute multiple undefined values"
out="$(mktemp)"
err="$(mktemp)"
if ../../files/shared/envsub.awk < bad-example.in >"$out" 2>"$err"; then
((++failCount))
log " FAILED: expected non-zero status code"
elif ! diff "$out" bad-example.stdout.expected; then
((++failCount))
log " FAILED: generated stdout did not equal expected output"
elif ! diff "$err" bad-example.stderr.expected; then
echo "err: $err"
((++failCount))
log " FAILED: generated stderr did not equal expected output"
else
log " OK"
fi

if [[ "$failCount" = 0 ]]; then
log "All tests passed OK."
else
log "$failCount TEST(S) FAILED"
exit 1
fi
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ services:
- PORT=8383
nginx:
build:
context: ..
context: ../..
dockerfile: nginx.dockerfile
args:
SKIP_FRONTEND_BUILD: true
Expand All @@ -24,12 +24,15 @@ services:
- enketo
environment:
- DOMAIN=odk-nginx.example.test
- SENTRY_ORG_SUBDOMAIN=example
- OIDC_ENABLED=
- SENTRY_KEY=example-sentry-key
- SENTRY_ORG_SUBDOMAIN=example-sentry-org-subdomain
- SENTRY_PROJECT=example-sentry-project
- SSL_TYPE=selfsign
- OIDC_ENABLED=false
volumes:
- ../files/nginx/odk.conf.template:/usr/share/odk/nginx/odk.conf.template:ro
- ../files/nginx/client-config.json.template:/usr/share/odk/nginx/client-config.json.template:ro
- ../../files/nginx/odk.conf.template:/usr/share/odk/nginx/odk.conf.template:ro
- ../../files/nginx/client-config.json.template:/usr/share/odk/nginx/client-config.json.template:ro
ports:
- "9000:80"
- "9001:443"
Expand Down
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.

0 comments on commit af73093

Please sign in to comment.