diff --git a/docs/appendices/file-formats/nginx-conf-sigil.md b/docs/appendices/file-formats/nginx-conf-sigil.md index 3de3fdbf6..8c1acf152 100644 --- a/docs/appendices/file-formats/nginx-conf-sigil.md +++ b/docs/appendices/file-formats/nginx-conf-sigil.md @@ -8,6 +8,8 @@ A custom `nginx.conf.sigil` is pre-validated at the start of every deploy, immed Pre-validation is skipped when the proxy type is not `nginx` or when `disable-custom-config` is set to `true` for the app. +The wrapper declares the same `user` directive as the host's main nginx config, exposed to the wrapper template as `$.NGINX_USER`. This matters because `nginx -t` runs as `root` and chowns every directory the config registers - in particular any `proxy_cache_path` - to whichever user the config resolves to, whether or not that directory already exists. Without a matching `user`, validating a template that declares a `proxy_cache_path` would hand the running server's live cache directory to nginx's compile-time default user for the length of the build, and every request served from or stored in that cache would fail with a permission error until the deploy's own nginx reload restored the ownership. + ### Custom nginx modules Pre-validation runs `nginx -t` against a minimal wrapper config that does _not_ include the top-level `load_module` directives from the global `/etc/nginx/nginx.conf`. A `nginx.conf.sigil` that uses a directive provided by a dynamically loaded module - such as `image_filter`, provided by the [ngx_http_image_filter_module](https://nginx.org/en/docs/http/ngx_http_image_filter_module.html) - therefore fails pre-validation with an `unknown directive` error, even though `nginx -t` succeeds against the real server config where the module is loaded. @@ -35,6 +37,7 @@ The custom `validate.conf.sigil`, which is the [default wrapper](https://github. ``` load_module modules/ngx_http_image_filter_module.so; +{{ if $.NGINX_USER }}user {{ $.NGINX_USER }};{{ end }} events { worker_connections 768; } http { access_log off; @@ -43,6 +46,8 @@ http { } ``` +A custom wrapper that omits the `user` line still has one injected before `nginx -t` runs, so wrappers written against older Dokku releases keep working, but declaring it explicitly documents the behavior and keeps the template self-contained. + The same override also governs the standalone `dokku nginx:validate-config` command, which renders the `validate-config` template as well. ## HTTP/2 diff --git a/docs/development/plugin-triggers.md b/docs/development/plugin-triggers.md index 365461f31..5be733618 100644 --- a/docs/development/plugin-triggers.md +++ b/docs/development/plugin-triggers.md @@ -1346,6 +1346,7 @@ set -eo pipefail; [[ $DOKKU_TRACE ]] && set -x - Invoked by: `nginx-vhosts#build-config` - Arguments: `$APP $TEMPLATE_TYPE` - The `TEMPLATE_TYPE` argument can be one of: `[app-config, hsts-config, validate-config]` + - The `validate-config` template can make use of the following variables: `$.NGINX_CONF $.NGINX_USER` - Example: ```shell diff --git a/docs/networking/proxies/nginx.md b/docs/networking/proxies/nginx.md index 70843947f..633a76030 100644 --- a/docs/networking/proxies/nginx.md +++ b/docs/networking/proxies/nginx.md @@ -123,6 +123,8 @@ App-supplied `nginx.conf.sigil` files are pre-validated automatically at the sta The wrapper config used for validation does not include the top-level `load_module` directives from the global nginx config, so a `nginx.conf.sigil` that relies on a directive from a dynamically loaded module will fail this validation even when `nginx -t` passes against the real server config. See [Custom nginx modules](/docs/appendices/file-formats/nginx-conf-sigil.md#custom-nginx-modules) for how to supply a custom validation wrapper via the `nginx-app-template-source` trigger's `validate-config` template type. +The wrapper does declare the same `user` directive as the host's main nginx config, so that validating a template which declares a `proxy_cache_path` does not take the running server's cache directory away from the user its workers run as. + It may also be desired to validate an nginx config outside of the deployment process. To do so, run the `nginx:validate-config` command. With no arguments, this will validate all app nginx configs, one at a time. A minimal wrapper nginx config is generated for each app's nginx config, upon which `nginx -t` will be run. ```shell diff --git a/plugins/nginx-vhosts/functions b/plugins/nginx-vhosts/functions index 730edba72..a4e670fc4 100755 --- a/plugins/nginx-vhosts/functions +++ b/plugins/nginx-vhosts/functions @@ -409,12 +409,6 @@ fn-nginx-vhosts-pre-validate-custom-template() { local TMP_RENDERED="$TMP_WORK_DIR/nginx.conf" local TMP_WRAPPER="$TMP_WORK_DIR/wrapper.conf" - local VALIDATE_TEMPLATE="$PLUGIN_AVAILABLE_PATH/nginx-vhosts/templates/validate.conf.sigil" - local CUSTOM_VALIDATE_TEMPLATE - CUSTOM_VALIDATE_TEMPLATE="$(plugn trigger nginx-app-template-source "$APP" "validate-config")" - if [[ -n "$CUSTOM_VALIDATE_TEMPLATE" ]]; then - VALIDATE_TEMPLATE="$CUSTOM_VALIDATE_TEMPLATE" - fi dokku_log_info1 "Pre-validating custom nginx.conf.sigil" @@ -424,7 +418,7 @@ fn-nginx-vhosts-pre-validate-custom-template() { dokku_log_fail "Custom nginx.conf.sigil failed to render via sigil" fi - sigil -f "$VALIDATE_TEMPLATE" NGINX_CONF="$TMP_RENDERED" | cat -s >"$TMP_WRAPPER" + fn-nginx-vhosts-render-validate-wrapper "$APP" "$TMP_RENDERED" "$TMP_WRAPPER" if ! sudo "$NGINX_LOCATION" -t -c "$TMP_WRAPPER" 2>/dev/null; then dokku_log_warn "Failed to validate custom nginx.conf.sigil. Rendered output below..." diff --git a/plugins/nginx-vhosts/install b/plugins/nginx-vhosts/install index 94bb56442..b18a3e128 100755 --- a/plugins/nginx-vhosts/install +++ b/plugins/nginx-vhosts/install @@ -37,12 +37,11 @@ trigger-nginx-vhosts-install() { fn-plugin-property-setup "nginx" NGINX_BIN="$(fn-nginx-vhosts-nginx-location)" - NGINX_ROOT="/etc/nginx" + NGINX_ROOT="$(fn-nginx-vhosts-nginx-conf-root)" NGINX_LOG_ROOT="/var/log/nginx" NGINX_INIT_NAME="nginx" NGINX_SUDOERS_FILE="/etc/sudoers.d/dokku-nginx" if fn-nginx-vhosts-uses-openresty; then - NGINX_ROOT="/usr/local/openresty/nginx/conf" NGINX_LOG_ROOT="/var/log/openresty" NGINX_INIT_NAME="openresty" NGINX_SUDOERS_FILE="/etc/sudoers.d/dokku-openresty" diff --git a/plugins/nginx-vhosts/internal-functions b/plugins/nginx-vhosts/internal-functions index 19186c4dd..8016d4747 100755 --- a/plugins/nginx-vhosts/internal-functions +++ b/plugins/nginx-vhosts/internal-functions @@ -686,6 +686,58 @@ fn-nginx-vhosts-uses-openresty() { return 1 } +fn-nginx-vhosts-nginx-conf-root() { + declare desc="returns the directory holding the main nginx config" + local NGINX_CONF_ROOT="/etc/nginx" + + fn-nginx-vhosts-uses-openresty && NGINX_CONF_ROOT="/usr/local/openresty/nginx/conf" + echo "$NGINX_CONF_ROOT" +} + +fn-nginx-vhosts-nginx-user() { + declare desc="returns the user declared by an nginx config, if any" + declare NGINX_CONF_PATH="$1" + + if [[ ! -r "$NGINX_CONF_PATH" ]]; then + return 0 + fi + + awk ' + { sub(/#.*/, "") } + $1 == "user" && NF >= 2 { + value = $2 + sub(/;.*$/, "", value) + gsub(/"/, "", value) + if (value != "") { + print value + exit + } + } + ' "$NGINX_CONF_PATH" +} + +fn-nginx-vhosts-validation-user() { + declare desc="returns the user to declare in the wrapper config used for nginx -t validation" + local NGINX_CONF_PATH NGINX_USER + + NGINX_CONF_PATH="$(fn-nginx-vhosts-nginx-conf-root)/nginx.conf" + if [[ -r "$NGINX_CONF_PATH" ]]; then + NGINX_USER="$(fn-nginx-vhosts-nginx-user "$NGINX_CONF_PATH")" + else + NGINX_USER="www-data" + fi + + if [[ ! "$NGINX_USER" =~ ^[a-zA-Z_][a-zA-Z0-9_.-]*$ ]]; then + return 0 + fi + + if ! id -u "$NGINX_USER" &>/dev/null; then + return 0 + fi + + echo "$NGINX_USER" +} + fn-nginx-vhosts-nginx-location() { declare desc="check that nginx is at the expected location and return it" local NGINX_LOCATION @@ -839,19 +891,48 @@ nginx_vhosts_validate_single_func() { fi } -nginx_vhosts_is_valid_nginx_config_func() { - declare desc="checks if an app has a valid nginx config" - declare APP="$1" +fn-nginx-vhosts-backfill-validate-user() { + declare desc="adds a user directive to a rendered wrapper config that declares none" + declare OUTPUT_PATH="$1" NGINX_USER="$2" + local WRAPPER_CONTENTS + + if [[ -z "$NGINX_USER" ]]; then + return 0 + fi + + if [[ -n "$(fn-nginx-vhosts-nginx-user "$OUTPUT_PATH")" ]]; then + return 0 + fi + + WRAPPER_CONTENTS="$(cat "$OUTPUT_PATH")" + printf 'user %s;\n%s\n' "$NGINX_USER" "$WRAPPER_CONTENTS" >"$OUTPUT_PATH" +} + +fn-nginx-vhosts-render-validate-wrapper() { + declare desc="renders the wrapper config used to run nginx -t against an app nginx config" + declare APP="$1" NGINX_CONF="$2" OUTPUT_PATH="$3" local VALIDATE_TEMPLATE="$PLUGIN_AVAILABLE_PATH/nginx-vhosts/templates/validate.conf.sigil" - local TMP_OUTPUT=$(mktemp "/tmp/dokku-${DOKKU_PID}-${FUNCNAME[0]}.XXXXXX") - trap "rm -rf '$TMP_OUTPUT' >/dev/null" RETURN INT TERM EXIT + local CUSTOM_VALIDATE_TEMPLATE NGINX_USER CUSTOM_VALIDATE_TEMPLATE="$(plugn trigger nginx-app-template-source "$APP" "validate-config")" if [[ -n "$CUSTOM_VALIDATE_TEMPLATE" ]]; then VALIDATE_TEMPLATE="$CUSTOM_VALIDATE_TEMPLATE" fi - sigil -f "$VALIDATE_TEMPLATE" NGINX_CONF="$DOKKU_ROOT/$APP/nginx.conf" | cat -s >"$TMP_OUTPUT" + NGINX_USER="$(fn-nginx-vhosts-validation-user)" + sigil -f "$VALIDATE_TEMPLATE" NGINX_CONF="$NGINX_CONF" NGINX_USER="$NGINX_USER" | cat -s >"$OUTPUT_PATH" + fn-nginx-vhosts-backfill-validate-user "$OUTPUT_PATH" "$NGINX_USER" +} + +nginx_vhosts_is_valid_nginx_config_func() { + declare desc="checks if an app has a valid nginx config" + declare APP="$1" + local NGINX_LOCATION + local TMP_OUTPUT=$(mktemp "/tmp/dokku-${DOKKU_PID}-${FUNCNAME[0]}.XXXXXX") + trap "rm -rf '$TMP_OUTPUT' >/dev/null" RETURN INT TERM EXIT + + NGINX_LOCATION="$(fn-nginx-vhosts-nginx-location)" + fn-nginx-vhosts-render-validate-wrapper "$APP" "$DOKKU_ROOT/$APP/nginx.conf" "$TMP_OUTPUT" sudo "$NGINX_LOCATION" -t -c "$TMP_OUTPUT" 2>/dev/null } diff --git a/plugins/nginx-vhosts/templates/validate.conf.sigil b/plugins/nginx-vhosts/templates/validate.conf.sigil index cb6a556c5..719b1a3da 100644 --- a/plugins/nginx-vhosts/templates/validate.conf.sigil +++ b/plugins/nginx-vhosts/templates/validate.conf.sigil @@ -1,3 +1,4 @@ +{{ if $.NGINX_USER }}user {{ $.NGINX_USER }};{{ end }} events { worker_connections 768; } http { access_log off; diff --git a/plugins/nginx-vhosts/validate_template_test.go b/plugins/nginx-vhosts/validate_template_test.go new file mode 100644 index 000000000..438064fbd --- /dev/null +++ b/plugins/nginx-vhosts/validate_template_test.go @@ -0,0 +1,57 @@ +package nginxvhosts + +import ( + "os" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/gliderlabs/sigil" + _ "github.com/gliderlabs/sigil/builtin" +) + +func validateTemplatePath(t *testing.T) string { + t.Helper() + _, file, _, ok := runtime.Caller(0) + if !ok { + t.Fatal("runtime.Caller failed") + } + return filepath.Join(filepath.Dir(file), "templates", "validate.conf.sigil") +} + +func renderValidateTemplate(t *testing.T, nginxUser string) string { + t.Helper() + data, err := os.ReadFile(validateTemplatePath(t)) + if err != nil { + t.Fatalf("read template: %v", err) + } + + vars := map[string]interface{}{ + "NGINX_CONF": "/home/dokku/app/nginx.conf", + "NGINX_USER": nginxUser, + } + buf, err := sigil.Execute(data, vars, "validate.conf.sigil") + if err != nil { + t.Fatalf("sigil.Execute: %v", err) + } + return buf.String() +} + +func TestValidateTemplate_DeclaresNginxUser(t *testing.T) { + out := renderValidateTemplate(t, "www-data") + mustContain(t, out, "user www-data;") + mustContain(t, out, "include /home/dokku/app/nginx.conf;") + + userIndex := strings.Index(out, "user www-data;") + eventsIndex := strings.Index(out, "events {") + if userIndex > eventsIndex { + t.Errorf("expected the user directive before the events block\n--- output ---\n%s", out) + } +} + +func TestValidateTemplate_OmitsEmptyNginxUser(t *testing.T) { + out := renderValidateTemplate(t, "") + mustNotContain(t, out, "user ") + mustContain(t, out, "include /home/dokku/app/nginx.conf;") +} diff --git a/tests.mk b/tests.mk index 1548c8f52..0f66ae16a 100644 --- a/tests.mk +++ b/tests.mk @@ -185,6 +185,7 @@ go-tests: @$(MAKE) go-test-plugin PLUGIN_NAME=docker-options @$(MAKE) go-test-plugin PLUGIN_NAME=logs @$(MAKE) go-test-plugin PLUGIN_NAME=network + @$(MAKE) go-test-plugin PLUGIN_NAME=nginx-vhosts @$(MAKE) go-test-plugin PLUGIN_NAME=ps @$(MAKE) go-test-plugin PLUGIN_NAME=buildpacks @$(MAKE) go-test-plugin PLUGIN_NAME=scheduler-k3s diff --git a/tests/unit/nginx-vhosts_15.bats b/tests/unit/nginx-vhosts_15.bats index 3de423ca1..1d1e3fb58 100644 --- a/tests/unit/nginx-vhosts_15.bats +++ b/tests/unit/nginx-vhosts_15.bats @@ -3,7 +3,10 @@ load test_helper NGINX_VALIDATE_PLUGIN_NAME="test-nginx-validate" +NGINX_VALIDATE_LEGACY_PLUGIN_NAME="test-nginx-validate-legacy" NGINX_VALIDATE_ZONE_CONF="/etc/nginx/conf.d/00-dokku-test-limit-req.conf" +NGINX_VALIDATE_BROKEN_CONF="/etc/nginx/conf.d/00-dokku-test-broken.conf" +NGINX_VALIDATE_CACHE_DIR="/var/cache/nginx/dokku-test-cache" setup() { global_setup @@ -12,9 +15,12 @@ setup() { } teardown() { + rm -f "$NGINX_VALIDATE_ZONE_CONF" "$NGINX_VALIDATE_BROKEN_CONF" + rm -rf "$NGINX_VALIDATE_CACHE_DIR" rm -rf "${PLUGIN_ENABLED_PATH:?}/$NGINX_VALIDATE_PLUGIN_NAME" "${PLUGIN_AVAILABLE_PATH:?}/$NGINX_VALIDATE_PLUGIN_NAME" + rm -rf "${PLUGIN_ENABLED_PATH:?}/$NGINX_VALIDATE_LEGACY_PLUGIN_NAME" "${PLUGIN_AVAILABLE_PATH:?}/$NGINX_VALIDATE_LEGACY_PLUGIN_NAME" + rm -rf "$DOKKU_LIB_ROOT/data/nginx-vhosts/app-$TEST_APP" || true destroy_app - rm -f "$NGINX_VALIDATE_ZONE_CONF" [[ -f "$DOKKU_ROOT/VHOST.bak" ]] && mv "$DOKKU_ROOT/VHOST.bak" "$DOKKU_ROOT/VHOST" && chown dokku:dokku "$DOKKU_ROOT/VHOST" global_teardown } @@ -84,6 +90,102 @@ teardown() { assert_output_contains "Pre-validating custom nginx.conf.sigil" } +@test "(nginx-vhosts) [pre-validate] fn-nginx-vhosts-nginx-user parses the main nginx config" { + source "$PLUGIN_AVAILABLE_PATH/nginx-vhosts/internal-functions" + + cat <<'EOF' >"$BATS_TEST_TMPDIR/nginx.conf" +#user nobody; + user www-data www-data; # workers run as www-data +userid on; +EOF + + run fn-nginx-vhosts-nginx-user "$BATS_TEST_TMPDIR/nginx.conf" + echo "output: $output" + echo "status: $status" + assert_output "www-data" +} + +@test "(nginx-vhosts) [pre-validate] fn-nginx-vhosts-nginx-user ignores configs declaring no user" { + source "$PLUGIN_AVAILABLE_PATH/nginx-vhosts/internal-functions" + + cat <<'EOF' >"$BATS_TEST_TMPDIR/nginx.conf" +#user nobody; +userid on; +auth_basic_user_file conf/htpasswd; +EOF + + run fn-nginx-vhosts-nginx-user "$BATS_TEST_TMPDIR/nginx.conf" + echo "output: $output" + echo "status: $status" + assert_output "" + + run fn-nginx-vhosts-nginx-user "$BATS_TEST_TMPDIR/missing.conf" + echo "output: $output" + echo "status: $status" + assert_output "" +} + +@test "(nginx-vhosts) [pre-validate] does not change proxy_cache_path directory ownership" { + local NGINX_USER + NGINX_USER="$(nginx_host_user)" + [[ -n "$NGINX_USER" ]] || skip "host nginx config declares no user directive" + + setup_nginx_cache_dir "$NGINX_USER" + nginx_cache_config "$BATS_TEST_TMPDIR/nginx.conf.sigil" + + run_plugin_script nginx-vhosts core-post-extract "$TEST_APP" "$BATS_TEST_TMPDIR" + echo "output: $output" + echo "status: $status" + assert_success + assert_output_contains "Pre-validating custom nginx.conf.sigil" + + run /bin/bash -c "stat -c %U $NGINX_VALIDATE_CACHE_DIR" + echo "output: $output" + echo "status: $status" + assert_output "$NGINX_USER" +} + +@test "(nginx-vhosts) [pre-validate] preserves ownership with a validate-config template predating NGINX_USER" { + local NGINX_USER + NGINX_USER="$(nginx_host_user)" + [[ -n "$NGINX_USER" ]] || skip "host nginx config declares no user directive" + + setup_nginx_validate_legacy_plugin + setup_nginx_cache_dir "$NGINX_USER" + nginx_cache_config "$BATS_TEST_TMPDIR/nginx.conf.sigil" + + run_plugin_script nginx-vhosts core-post-extract "$TEST_APP" "$BATS_TEST_TMPDIR" + echo "output: $output" + echo "status: $status" + assert_success + assert_output_contains "Pre-validating custom nginx.conf.sigil" + + run /bin/bash -c "stat -c %U $NGINX_VALIDATE_CACHE_DIR" + echo "output: $output" + echo "status: $status" + assert_output "$NGINX_USER" +} + +@test "(nginx-vhosts:validate-config) does not change proxy_cache_path directory ownership" { + local NGINX_USER + NGINX_USER="$(nginx_host_user)" + [[ -n "$NGINX_USER" ]] || skip "host nginx config declares no user directive" + + setup_nginx_cache_dir "$NGINX_USER" + nginx_cache_config "$DOKKU_ROOT/$TEST_APP/nginx.conf" + echo 'not_a_directive;' >"$NGINX_VALIDATE_BROKEN_CONF" + + run /bin/bash -c "dokku nginx:validate-config $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_failure + + run /bin/bash -c "stat -c %U $NGINX_VALIDATE_CACHE_DIR" + echo "output: $output" + echo "status: $status" + assert_output "$NGINX_USER" +} + setup_nginx_validate_plugin() { declare desc="installs a plugin that overrides the validate-config nginx template" local PLUGIN_DIR="$PLUGIN_AVAILABLE_PATH/$NGINX_VALIDATE_PLUGIN_NAME" @@ -110,6 +212,7 @@ EOF chmod +x "$PLUGIN_DIR/nginx-app-template-source" cat <<'EOF' >"$PLUGIN_DIR/validate.conf.sigil" +{{ if $.NGINX_USER }}user {{ $.NGINX_USER }};{{ end }} events { worker_connections 768; } http { access_log off; @@ -170,3 +273,74 @@ upstream {{ $.APP }}-{{ \$upstream_port }} { EOF cat "$APP_REPO_DIR/nginx.conf.sigil" } + +nginx_host_user() { + declare desc="returns the user the host nginx runs its workers as" + + awk '{ sub(/#.*/, "") } $1 == "user" && NF >= 2 { value = $2; sub(/;.*$/, "", value); print value; exit }' /etc/nginx/nginx.conf +} + +setup_nginx_cache_dir() { + declare desc="creates a proxy_cache_path directory owned by the host nginx user" + local NGINX_USER="$1" + + mkdir -p "$NGINX_VALIDATE_CACHE_DIR" + chown "$NGINX_USER" "$NGINX_VALIDATE_CACHE_DIR" +} + +nginx_cache_config() { + declare desc="writes an nginx config declaring a proxy_cache_path at the test cache directory" + local OUTPUT_PATH="$1" + + echo "injecting nginx_cache_config -> $OUTPUT_PATH" + cat <"$OUTPUT_PATH" +proxy_cache_path $NGINX_VALIDATE_CACHE_DIR levels=1:2 keys_zone=dokkuvalidatecache:1m max_size=10m inactive=1m use_temp_path=off; + +server { + listen 127.0.0.1:18999; + server_name dokku-validate-cache.test; + + location / { + proxy_cache dokkuvalidatecache; + proxy_pass http://127.0.0.1:18998; + } +} +EOF +} + +setup_nginx_validate_legacy_plugin() { + declare desc="installs a plugin whose validate-config template predates the NGINX_USER variable" + local PLUGIN_DIR="$PLUGIN_AVAILABLE_PATH/$NGINX_VALIDATE_LEGACY_PLUGIN_NAME" + mkdir -p "$PLUGIN_DIR" + + cat <"$PLUGIN_DIR/plugin.toml" +[plugin] +description = "test plugin overriding the validate-config nginx template without a user directive" +version = "0.1.0" +[plugin.config] +EOF + + cat <<'EOF' >"$PLUGIN_DIR/nginx-app-template-source" +#!/usr/bin/env bash + +set -eo pipefail +[[ $DOKKU_TRACE ]] && set -x + +TEMPLATE_TYPE="$2" +if [[ "$TEMPLATE_TYPE" == "validate-config" ]]; then + echo "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/validate.conf.sigil" +fi +EOF + chmod +x "$PLUGIN_DIR/nginx-app-template-source" + + cat <<'EOF' >"$PLUGIN_DIR/validate.conf.sigil" +events { worker_connections 768; } +http { + access_log off; + error_log /dev/null; + include {{ $.NGINX_CONF }}; +} +EOF + + dokku plugin:enable "$NGINX_VALIDATE_LEGACY_PLUGIN_NAME" +}