From 62caa85423f9ab2d4e7d01e9e57226359bc7b7f8 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Wed, 1 Apr 2020 12:21:52 -0400 Subject: [PATCH 1/5] feat: add ability to change or disable the access/error log path Closes #3911 --- docs/configuration/nginx.md | 23 +++++++ plugins/nginx-vhosts/command-functions | 2 + plugins/nginx-vhosts/functions | 41 ++++++++++-- plugins/nginx-vhosts/subcommands/set | 4 +- .../nginx-vhosts/templates/nginx.conf.sigil | 16 ++--- tests/unit/40_nginx-vhosts_2.bats | 66 ++++++++++++++++++- tests/unit/test_helper.bash | 12 ---- 7 files changed, 135 insertions(+), 29 deletions(-) diff --git a/docs/configuration/nginx.md b/docs/configuration/nginx.md index a7a744806..961c653a5 100644 --- a/docs/configuration/nginx.md +++ b/docs/configuration/nginx.md @@ -82,6 +82,29 @@ You may also follow the logs by specifying the `-t` flag. dokku nginx:error-logs node-js-app -t ``` +## Changing log path + +The path to where log files are stored can be changed by calling the `nginx:set` command with the following options: + +- `access-log-path` (type: string, default: `${NGINX_LOG_ROOT}/${APP}-error.log`): Log path for nginx access logs +- `error-log-path` (type: string, default: `${NGINX_LOG_ROOT}/${APP}-error.log`): Log path for nginx error logs + +The defaults should not be changed without verifying that the paths will be writeable by nginx. However, this setting is useful for enabling or disabling logging by setting the values to `off` + +```shell +dokku nginx:set node-js-app access-log-path off +dokku nginx:set node-js-app error-log-path off +``` + +The default value may be set by passing an empty value for the option: + +```shell +dokku nginx:set node-js-app access-log-path +dokku nginx:set node-js-app error-log-path +``` + +In all cases, the nginx config must be regenerated after setting the above values. + ## Regenerating nginx config In certain cases, your app nginx configs may drift from the correct config for your app. You may regenerate the config at any point via the `nginx:build-config` command. This may fail if there are no current web listeners for your app. diff --git a/plugins/nginx-vhosts/command-functions b/plugins/nginx-vhosts/command-functions index 0cf9790ec..2d65389f8 100755 --- a/plugins/nginx-vhosts/command-functions +++ b/plugins/nginx-vhosts/command-functions @@ -37,8 +37,10 @@ cmd-nginx-report-single() { fi verify_app_name "$APP" local flag_map=( + "--nginx-access-log-path: $(fn-nginx-access-log-path "$APP")" "--nginx-bind-address-ipv4: $(fn-plugin-property-get-default "nginx" "$APP" "bind-address-ipv4" "")" "--nginx-bind-address-ipv6: $(fn-plugin-property-get-default "nginx" "$APP" "bind-address-ipv6" "::")" + "--nginx-error-log-path: $(fn-nginx-error-log-path "$APP")" "--nginx-hsts: $(fn-plugin-property-get-default "nginx" "$APP" "hsts" "true")" "--nginx-hsts-include-subdomains: $(fn-plugin-property-get-default "nginx" "$APP" "hsts-include-subdomains" "true")" "--nginx-hsts-max-age: $(fn-plugin-property-get-default "nginx" "$APP" "hsts-max-age" "15724800")" diff --git a/plugins/nginx-vhosts/functions b/plugins/nginx-vhosts/functions index 330a3cae9..702830405 100755 --- a/plugins/nginx-vhosts/functions +++ b/plugins/nginx-vhosts/functions @@ -8,6 +8,29 @@ source "$PLUGIN_AVAILABLE_PATH/domains/functions" source "$PLUGIN_AVAILABLE_PATH/ps/functions" source "$PLUGIN_AVAILABLE_PATH/nginx-vhosts/internal-functions" +fn-nginx-log-root() { + declare desc="get the nginx log root" + local NGINX_LOG_ROOT="/var/log/nginx" + fn-nginx-vhosts-uses-openresty && NGINX_LOG_ROOT="/var/log/openresty" + echo "$NGINX_LOG_ROOT" +} + +fn-nginx-access-log-path() { + declare desc="get the configured access log path" + declare APP="$1" + local NGINX_LOG_ROOT="$(fn-nginx-log-root)" + + fn-plugin-property-get-default "nginx" "$APP" "access-log-path" "${NGINX_LOG_ROOT}/${APP}-access.log" +} + +fn-nginx-error-log-path() { + declare desc="get the configured access log path" + declare APP="$1" + local NGINX_LOG_ROOT="$(fn-nginx-log-root)" + + fn-plugin-property-get-default "nginx" "$APP" "error-log-path" "${NGINX_LOG_ROOT}/${APP}-error.log" +} + get_nginx_location() { declare desc="check that nginx is at the expected location and return it" fn-nginx-vhosts-nginx-location @@ -63,12 +86,14 @@ restart_nginx() { nginx_logs() { declare desc="display app nginx logs" declare NGINX_LOGS_TYPE="${1#nginx:}" APP="$2" - local NGINX_LOG_ROOT="/var/log/nginx" verify_app_name "$APP" - fn-nginx-vhosts-uses-openresty && NGINX_LOG_ROOT="/var/log/openresty" local NGINX_LOGS_TYPE=${NGINX_LOGS_TYPE%-logs} - local NGINX_LOGS_PATH="$NGINX_LOG_ROOT/$APP-$NGINX_LOGS_TYPE.log" + local NGINX_LOGS_PATH="$("fn-nginx-${NGINX_LOGS_TYPE}-log-path" "$APP")" + + if [[ "$NGINX_LOGS_PATH" == "off" ]] || [[ "$NGINX_LOGS_PATH" == "/dev/null" ]]; then + dokku_log_fail "$NGINX_LOGS_TYPE logs are disabled for this app" + fi if [[ $3 == "-t" ]]; then local NGINX_LOGS_ARGS="-F" @@ -355,8 +380,9 @@ nginx_build_config() { PROXY_PORT_MAP=$(echo "$PROXY_PORT_MAP" | xargs) # trailing spaces mess up default template - local NGINX_LOG_ROOT="/var/log/nginx" - fn-nginx-vhosts-uses-openresty && NGINX_LOG_ROOT="/var/log/openresty" + local NGINX_LOG_ROOT="$(fn-nginx-log-root)" + local NGINX_ACCESS_LOG_PATH="$(fn-nginx-access-log-path "$APP")" + local NGINX_ERROR_LOG_PATH="$(fn-nginx-error-log-path "$APP")" if [[ -z "$DOKKU_APP_LISTENERS" ]]; then dokku_log_warn_quiet "No web listeners specified for $APP" @@ -384,6 +410,9 @@ nginx_build_config() { local NGINX_BIND_ADDRESS_IP6="$(fn-plugin-property-get-default "nginx" "$APP" "bind-address-ipv6" "::")" [[ -z "$NGINX_BIND_ADDRESS_IP6" ]] && NGINX_BIND_ADDRESS_IP6="::" + local NGINX_ACCESS_LOGS_ENABLED="$(fn-plugin-property-get-default "nginx" "$APP" "bind-address-ipv4" "")" + local NGINX_ERROR_LOGS_ENABLED="$(fn-plugin-property-get-default "nginx" "$APP" "bind-address-ipv6" "::")" + eval "$(config_export app "$APP")" local SIGIL_PARAMS=(-f "$NGINX_TEMPLATE" APP="$APP" DOKKU_ROOT="$DOKKU_ROOT" NOSSL_SERVER_NAME="$NOSSL_SERVER_NAME" @@ -395,6 +424,8 @@ nginx_build_config() { TLS13_SUPPORTED="$TLS13_SUPPORTED" HTTP2_SUPPORTED="$HTTP2_SUPPORTED" NGINX_LOG_ROOT="$NGINX_LOG_ROOT" + NGINX_ACCESS_LOG_PATH="$NGINX_ACCESS_LOG_PATH" + NGINX_ERROR_LOG_PATH="$NGINX_ERROR_LOG_PATH" NGINX_BIND_ADDRESS_IP4="$NGINX_BIND_ADDRESS_IP4" NGINX_BIND_ADDRESS_IP6="$NGINX_BIND_ADDRESS_IP6" HTTP2_PUSH_SUPPORTED="$HTTP2_PUSH_SUPPORTED" diff --git a/plugins/nginx-vhosts/subcommands/set b/plugins/nginx-vhosts/subcommands/set index 475d0188a..cec931169 100755 --- a/plugins/nginx-vhosts/subcommands/set +++ b/plugins/nginx-vhosts/subcommands/set @@ -9,13 +9,13 @@ cmd-nginx-set() { declare cmd="nginx:set" [[ "$1" == "$cmd" ]] && shift 1 declare APP="$1" KEY="$2" VALUE="$3" - local VALID_KEYS=("bind-address-ipv4" "bind-address-ipv6" "hsts" "hsts-include-subdomains" "hsts-preload" "hsts-max-age") + local VALID_KEYS=("access-log-path" "bind-address-ipv4" "bind-address-ipv6" "error-log-path" "hsts" "hsts-include-subdomains" "hsts-preload" "hsts-max-age") [[ -z "$APP" ]] && dokku_log_fail "Please specify an app to run the command on" [[ -z "$KEY" ]] && dokku_log_fail "No key specified" if ! fn-in-array "$KEY" "${VALID_KEYS[@]}"; then - dokku_log_fail "Invalid key specified, valid keys include: bind-address-ipv4, bind-address-ipv6, hsts, hsts-include-subdomains, hsts-preload, hsts-max-age" + dokku_log_fail "Invalid key specified, valid keys include: access-log-path, bind-address-ipv4, bind-address-ipv6, error-log-path, hsts, hsts-include-subdomains, hsts-preload, hsts-max-age" fi if [[ -n "$VALUE" ]]; then diff --git a/plugins/nginx-vhosts/templates/nginx.conf.sigil b/plugins/nginx-vhosts/templates/nginx.conf.sigil index dc642e5a6..b1d6d1a24 100644 --- a/plugins/nginx-vhosts/templates/nginx.conf.sigil +++ b/plugins/nginx-vhosts/templates/nginx.conf.sigil @@ -9,8 +9,8 @@ server { listen [{{ $.NGINX_BIND_ADDRESS_IP6 }}]:{{ $listen_port }}; listen {{ if $.NGINX_BIND_ADDRESS_IP4 }}{{ $.NGINX_BIND_ADDRESS_IP4 }}:{{end}}{{ $listen_port }}; {{ if $.NOSSL_SERVER_NAME }}server_name {{ $.NOSSL_SERVER_NAME }}; {{ end }} - access_log {{ $.NGINX_LOG_ROOT }}/{{ $.APP }}-access.log; - error_log {{ $.NGINX_LOG_ROOT }}/{{ $.APP }}-error.log; + access_log {{ $.NGINX_ACCESS_LOG_PATH }}; + error_log {{ $.NGINX_ERROR_LOG_PATH }}; {{ if (and (eq $listen_port "80") ($.SSL_INUSE)) }} return 301 https://$host:{{ $.PROXY_SSL_PORT }}$request_uri; {{ else }} @@ -60,8 +60,8 @@ server { listen {{ if $.NGINX_BIND_ADDRESS_IP4 }}{{ $.NGINX_BIND_ADDRESS_IP4 }}:{{end}}{{ $listen_port }} ssl {{ if eq $.HTTP2_SUPPORTED "true" }}http2{{ else if eq $.SPDY_SUPPORTED "true" }}spdy{{ end }}; {{ if $.SSL_SERVER_NAME }}server_name {{ $.SSL_SERVER_NAME }}; {{ end }} {{ if $.NOSSL_SERVER_NAME }}server_name {{ $.NOSSL_SERVER_NAME }}; {{ end }} - access_log {{ $.NGINX_LOG_ROOT }}/{{ $.APP }}-access.log; - error_log {{ $.NGINX_LOG_ROOT }}/{{ $.APP }}-error.log; + access_log {{ $.NGINX_ACCESS_LOG_PATH }}; + error_log {{ $.NGINX_ERROR_LOG_PATH }}; ssl_certificate {{ $.APP_SSL_PATH }}/server.crt; ssl_certificate_key {{ $.APP_SSL_PATH }}/server.key; @@ -123,8 +123,8 @@ server { listen [{{ $.NGINX_BIND_ADDRESS_IP6 }}]:{{ $listen_port }} http2; listen {{ if $.NGINX_BIND_ADDRESS_IP4 }}{{ $.NGINX_BIND_ADDRESS_IP4 }}:{{end}}{{ $listen_port }} http2; {{ if $.NOSSL_SERVER_NAME }}server_name {{ $.NOSSL_SERVER_NAME }}; {{ end }} - access_log {{ $.NGINX_LOG_ROOT }}/{{ $.APP }}-access.log; - error_log {{ $.NGINX_LOG_ROOT }}/{{ $.APP }}-error.log; + access_log {{ $.NGINX_ACCESS_LOG_PATH }}; + error_log {{ $.NGINX_ERROR_LOG_PATH }}; location / { grpc_pass grpc://{{ $.APP }}-{{ $upstream_port }}; } @@ -137,8 +137,8 @@ server { listen [{{ $.NGINX_BIND_ADDRESS_IP6 }}]:{{ $listen_port }} ssl http2; listen {{ if $.NGINX_BIND_ADDRESS_IP4 }}{{ $.NGINX_BIND_ADDRESS_IP4 }}:{{end}}{{ $listen_port }} ssl http2; {{ if $.NOSSL_SERVER_NAME }}server_name {{ $.NOSSL_SERVER_NAME }}; {{ end }} - access_log {{ $.NGINX_LOG_ROOT }}/{{ $.APP }}-access.log; - error_log {{ $.NGINX_LOG_ROOT }}/{{ $.APP }}-error.log; + access_log {{ $.NGINX_ACCESS_LOG_PATH }}; + error_log {{ $.NGINX_ERROR_LOG_PATH }}; ssl_certificate {{ $.APP_SSL_PATH }}/server.crt; ssl_certificate_key {{ $.APP_SSL_PATH }}/server.key; diff --git a/tests/unit/40_nginx-vhosts_2.bats b/tests/unit/40_nginx-vhosts_2.bats index 6fbc2dd64..de1a72dbf 100644 --- a/tests/unit/40_nginx-vhosts_2.bats +++ b/tests/unit/40_nginx-vhosts_2.bats @@ -26,8 +26,70 @@ teardown() { @test "(nginx-vhosts) logging" { deploy_app - assert_access_log ${TEST_APP} - assert_error_log ${TEST_APP} + + run [ -a "/var/log/nginx/$TEST_APP-access.log" ] + echo "output: $output" + echo "status: $status" + assert_success + + run [ -a "/var/log/nginx/$TEST_APP-error.log" ] + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku nginx:access-logs $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku nginx:error-logs $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success +} + +@test "(nginx-vhosts) log-path" { + deploy_app + + run /bin/bash -c "dokku nginx:set $TEST_APP access-log-path off" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku nginx:show-config $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_output_contains "off;" + + run /bin/bash -c "dokku nginx:set $TEST_APP access-log-path" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku nginx:show-config $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_output_contains "off;" 0 + + run /bin/bash -c "dokku nginx:set $TEST_APP error-log-path off" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku nginx:show-config $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_output_contains "off;" + + run /bin/bash -c "dokku nginx:set $TEST_APP error-log-path" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku nginx:show-config $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_output_contains "off;" 0 } @test "(nginx-vhosts) nginx:build-config (with SSL and unrelated domain)" { diff --git a/tests/unit/test_helper.bash b/tests/unit/test_helper.bash index b02ba0389..32a612619 100644 --- a/tests/unit/test_helper.bash +++ b/tests/unit/test_helper.bash @@ -256,18 +256,6 @@ assert_urls() { assert_output < <(tr ' ' '\n' <<< "${urls}" | sort) } -assert_access_log() { - local prefix=$1 - run [ -a "/var/log/nginx/$prefix-access.log" ] - assert_success -} - -assert_error_log() { - local prefix=$1 - run [ -a "/var/log/nginx/$prefix-error.log" ] - assert_success -} - deploy_app() { declare APP_TYPE="$1" GIT_REMOTE="$2" CUSTOM_TEMPLATE="$3" CUSTOM_PATH="$4" local APP_TYPE=${APP_TYPE:="nodejs-express"} From cef73e78723c3cdca04f1061dcfc3f3bdf169e12 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Wed, 1 Apr 2020 15:27:17 -0400 Subject: [PATCH 2/5] fix: build config after nginx:set --- tests/unit/40_nginx-vhosts_2.bats | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/unit/40_nginx-vhosts_2.bats b/tests/unit/40_nginx-vhosts_2.bats index de1a72dbf..e5d944879 100644 --- a/tests/unit/40_nginx-vhosts_2.bats +++ b/tests/unit/40_nginx-vhosts_2.bats @@ -56,6 +56,11 @@ teardown() { echo "status: $status" assert_success + run /bin/bash -c "dokku nginx:build-config $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + run /bin/bash -c "dokku nginx:show-config $TEST_APP" echo "output: $output" echo "status: $status" @@ -66,6 +71,11 @@ teardown() { echo "status: $status" assert_success + run /bin/bash -c "dokku nginx:build-config $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + run /bin/bash -c "dokku nginx:show-config $TEST_APP" echo "output: $output" echo "status: $status" @@ -76,6 +86,11 @@ teardown() { echo "status: $status" assert_success + run /bin/bash -c "dokku nginx:build-config $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + run /bin/bash -c "dokku nginx:show-config $TEST_APP" echo "output: $output" echo "status: $status" @@ -86,6 +101,11 @@ teardown() { echo "status: $status" assert_success + run /bin/bash -c "dokku nginx:build-config $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + run /bin/bash -c "dokku nginx:show-config $TEST_APP" echo "output: $output" echo "status: $status" From 88631483b4cf03e70bf7553364e4621302900483 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Wed, 1 Apr 2020 15:57:54 -0400 Subject: [PATCH 3/5] Update docs/configuration/nginx.md Co-Authored-By: decentral1se --- docs/configuration/nginx.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/configuration/nginx.md b/docs/configuration/nginx.md index 961c653a5..51a04dc06 100644 --- a/docs/configuration/nginx.md +++ b/docs/configuration/nginx.md @@ -86,7 +86,7 @@ dokku nginx:error-logs node-js-app -t The path to where log files are stored can be changed by calling the `nginx:set` command with the following options: -- `access-log-path` (type: string, default: `${NGINX_LOG_ROOT}/${APP}-error.log`): Log path for nginx access logs +- `access-log-path` (type: string, default: `${NGINX_LOG_ROOT}/${APP}-access.log`): Log path for nginx access logs - `error-log-path` (type: string, default: `${NGINX_LOG_ROOT}/${APP}-error.log`): Log path for nginx error logs The defaults should not be changed without verifying that the paths will be writeable by nginx. However, this setting is useful for enabling or disabling logging by setting the values to `off` From 908d203a64eb383f7571de3c6805c590b1da9fcf Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Wed, 1 Apr 2020 15:58:01 -0400 Subject: [PATCH 4/5] Update docs/configuration/nginx.md Co-Authored-By: decentral1se --- docs/configuration/nginx.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/configuration/nginx.md b/docs/configuration/nginx.md index 51a04dc06..2cf4cf9b8 100644 --- a/docs/configuration/nginx.md +++ b/docs/configuration/nginx.md @@ -89,7 +89,7 @@ The path to where log files are stored can be changed by calling the `nginx:set` - `access-log-path` (type: string, default: `${NGINX_LOG_ROOT}/${APP}-access.log`): Log path for nginx access logs - `error-log-path` (type: string, default: `${NGINX_LOG_ROOT}/${APP}-error.log`): Log path for nginx error logs -The defaults should not be changed without verifying that the paths will be writeable by nginx. However, this setting is useful for enabling or disabling logging by setting the values to `off` +The defaults should not be changed without verifying that the paths will be writeable by nginx. However, this setting is useful for enabling or disabling logging by setting the values to `off`. ```shell dokku nginx:set node-js-app access-log-path off From 57da0e1ff06e38d87c1bc5644e44dec2a3e0d811 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Wed, 1 Apr 2020 20:06:37 -0400 Subject: [PATCH 5/5] docs: add "new as of" blockquote [ci skip] --- docs/configuration/nginx.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/configuration/nginx.md b/docs/configuration/nginx.md index 2cf4cf9b8..941ba31f3 100644 --- a/docs/configuration/nginx.md +++ b/docs/configuration/nginx.md @@ -84,6 +84,8 @@ dokku nginx:error-logs node-js-app -t ## Changing log path +> New as of 0.20.1 + The path to where log files are stored can be changed by calling the `nginx:set` command with the following options: - `access-log-path` (type: string, default: `${NGINX_LOG_ROOT}/${APP}-access.log`): Log path for nginx access logs