Merge pull request #8809 from dokku/8784-new-nginx-pre-validation-breaks-configs-that-need-plugins

Document nginx validate-config load_module override

docs/appendices/file-formats/nginx-conf-sigil.md

@@ -8,6 +8,43 @@ 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.
 
+### 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.
+
+The `load_module` directive cannot be added to the app's `nginx.conf.sigil` to work around this, as that file is included inside the `http { }` block while `load_module` is only valid in nginx's top-level main context.
+
+To make pre-validation aware of a module, override the wrapper template used for validation. The [`nginx-app-template-source`](/docs/development/plugin-triggers.md#nginx-app-template-source) trigger returns the path to the `sigil` template used to generate a given nginx configuration file, and its `validate-config` template type controls the pre-validation wrapper. Create a [custom plugin](/docs/development/plugin-creation.md) that implements the trigger and returns a `validate.conf.sigil` that adds the required `load_module` line at the top of the wrapper.
+
+The trigger file (named `nginx-app-template-source` and marked executable):
+
+```shell
+#!/usr/bin/env bash
+
+set -eo pipefail
+[[ $DOKKU_TRACE ]] && set -x
+
+APP="$1"
+TEMPLATE_TYPE="$2"
+if [[ "$TEMPLATE_TYPE" == "validate-config" ]]; then
+  echo "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/validate.conf.sigil"
+fi
+```
+
+The custom `validate.conf.sigil`, which is the [default wrapper](https://github.com/dokku/dokku/blob/master/plugins/nginx-vhosts/templates/validate.conf.sigil) with the required `load_module` line added at the top. Use the same `load_module` line that the host's global `/etc/nginx/nginx.conf` uses:
+
+```
+load_module modules/ngx_http_image_filter_module.so;
+events { worker_connections 768; }
+http {
+  access_log off;
+  error_log /dev/null;
+  include {{ $.NGINX_CONF }};
+}
+```
+
+The same override also governs the standalone `dokku nginx:validate-config` command, which renders the `validate-config` template as well.
+
 ## HTTP/2
 
 nginx 1.25.1 deprecated the `http2` parameter on the `listen` directive in favor of a standalone `http2 on;` directive. Custom `nginx.conf.sigil` templates that hardcode `listen ... ssl http2;` will produce `nginx: [warn] the "listen ... http2" directive is deprecated` warnings when run against nginx 1.25.1 or newer.

docs/development/plugin-triggers.md

@@ -1340,6 +1340,8 @@ esac
 
 The default templates are viewable here: [plugins/nginx-vhosts/templates/](https://github.com/dokku/dokku/tree/master/plugins/nginx-vhosts/templates)
 
+Overriding the `validate-config` template is the supported way to make deploy-time pre-validation aware of directives from nginx modules that the host loads via `load_module` in its global config. As implementing this trigger requires a [custom plugin](/docs/development/plugin-creation.md), see [Custom nginx modules](/docs/appendices/file-formats/nginx-conf-sigil.md#custom-nginx-modules) for a complete example.
+
 ### `nginx-dokku-template-source`
 
 - Description: Return the path to a `sigil` template that should be used to generate the `dokku.conf` nginx configuration file.

docs/networking/proxies/nginx.md

@@ -121,6 +121,8 @@ dokku nginx:show-config node-js-app
 
 App-supplied `nginx.conf.sigil` files are pre-validated automatically at the start of every deploy, immediately after the template is extracted from the source tree and before the build phase runs. The template is rendered with sigil and run through `nginx -t` against a minimal wrapper config; if validation fails, the deploy is aborted before any build work begins. To bypass this behavior, set `disable-custom-config` to `true` (see "Disabling custom nginx config" below).
 
+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.
+
 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

tests/unit/nginx-vhosts_15.bats

@@ -2,6 +2,9 @@
 
 load test_helper
 
+NGINX_VALIDATE_PLUGIN_NAME="test-nginx-validate"
+NGINX_VALIDATE_ZONE_CONF="/etc/nginx/conf.d/00-dokku-test-limit-req.conf"
+
 setup() {
   global_setup
   [[ -f "$DOKKU_ROOT/VHOST" ]] && cp -fp "$DOKKU_ROOT/VHOST" "$DOKKU_ROOT/VHOST.bak"
@@ -9,7 +12,9 @@ setup() {
 }
 
 teardown() {
+  rm -rf "${PLUGIN_ENABLED_PATH:?}/$NGINX_VALIDATE_PLUGIN_NAME" "${PLUGIN_AVAILABLE_PATH:?}/$NGINX_VALIDATE_PLUGIN_NAME"
   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
 }
@@ -58,3 +63,110 @@ teardown() {
   assert_success
   assert_output_not_contains "Pre-validating custom nginx.conf.sigil"
 }
+
+@test "(nginx-vhosts) pre-validate fails when nginx.conf.sigil needs a directive absent from the wrapper" {
+  run deploy_app python dokku@$DOKKU_DOMAIN:$TEST_APP custom_nginx_template_with_limit_req
+  echo "output: $output"
+  echo "status: $status"
+  assert_failure
+  assert_output_contains "Pre-validating custom nginx.conf.sigil"
+  assert_output_contains "Custom nginx.conf.sigil failed nginx -t validation"
+}
+
+@test "(nginx-vhosts) pre-validate passes when nginx-app-template-source overrides validate-config" {
+  echo 'limit_req_zone $binary_remote_addr zone=dokkuprevalidate:10m rate=100r/s;' >"$NGINX_VALIDATE_ZONE_CONF"
+  setup_nginx_validate_plugin
+
+  run deploy_app python dokku@$DOKKU_DOMAIN:$TEST_APP custom_nginx_template_with_limit_req
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+  assert_output_contains "Pre-validating custom nginx.conf.sigil"
+}
+
+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"
+  mkdir -p "$PLUGIN_DIR"
+
+  cat <<EOF >"$PLUGIN_DIR/plugin.toml"
+[plugin]
+description = "test plugin overriding the validate-config nginx template"
+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;
+  limit_req_zone $binary_remote_addr zone=dokkuprevalidate:10m rate=100r/s;
+  include {{ $.NGINX_CONF }};
+}
+EOF
+
+  dokku plugin:enable "$NGINX_VALIDATE_PLUGIN_NAME"
+}
+
+custom_nginx_template_with_limit_req() {
+  local APP="$1"
+  local APP_REPO_DIR="$2"
+  [[ -z "$APP" ]] && local APP="$TEST_APP"
+  mkdir -p "$APP_REPO_DIR"
+
+  echo "injecting custom_nginx_template_with_limit_req -> $APP_REPO_DIR/nginx.conf.sigil"
+  cat <<EOF >"$APP_REPO_DIR/nginx.conf.sigil"
+{{ range \$port_map := .PROXY_PORT_MAP | split " " }}
+{{ \$port_map_list := \$port_map | split ":" }}
+{{ \$scheme := index \$port_map_list 0 }}
+{{ \$listen_port := index \$port_map_list 1 }}
+{{ \$upstream_port := index \$port_map_list 2 }}
+
+server {
+  listen      [::]:{{ \$listen_port }};
+  listen      {{ \$listen_port }};
+  server_name {{ $.NOSSL_SERVER_NAME }} customtemplate.${DOKKU_DOMAIN};
+
+  location    / {
+    limit_req zone=dokkuprevalidate burst=100 nodelay;
+    proxy_pass  http://{{ $.APP }}-{{ \$upstream_port }};
+    proxy_http_version 1.1;
+    proxy_set_header Upgrade \$http_upgrade;
+    proxy_set_header Connection "upgrade";
+    proxy_set_header Host \$http_host;
+    proxy_set_header X-Forwarded-Proto \$scheme;
+    proxy_set_header X-Forwarded-For \$remote_addr;
+    proxy_set_header X-Forwarded-Port \$server_port;
+    proxy_set_header X-Request-Start \$msec;
+  }
+  include {{ $.DOKKU_ROOT }}/{{ $.APP }}/nginx.conf.d/*.conf;
+}
+{{ end }}
+
+{{ if $.DOKKU_APP_WEB_LISTENERS }}
+{{ range \$upstream_port := $.PROXY_UPSTREAM_PORTS | split " " }}
+upstream {{ $.APP }}-{{ \$upstream_port }} {
+{{ range \$listeners := $.DOKKU_APP_WEB_LISTENERS | split " " }}
+{{ \$listener_list := \$listeners | split ":" }}
+{{ \$listener_ip := index \$listener_list 0 }}
+  server {{ \$listener_ip }}:{{ \$upstream_port }};{{ end }}
+}
+{{ end }}{{ end }}
+
+EOF
+  cat "$APP_REPO_DIR/nginx.conf.sigil"
+}