Skip to content

Use the row's own model and host type in regenerate-config - #5813

Open
vzagorovskiy wants to merge 1 commit into
NginxProxyManager:developfrom
vzagorovskiy:fix/regenerate-config-wrong-host-type
Open

Use the row's own model and host type in regenerate-config#5813
vzagorovskiy wants to merge 1 commit into
NginxProxyManager:developfrom
vzagorovskiy:fix/regenerate-config-wrong-host-type

Conversation

@vzagorovskiy

Copy link
Copy Markdown
Contributor

Problem

backend/scripts/regenerate-config walks four host types, but processItems() hands the same model and the literal "proxy_host" to configure() for all of them:

await internalNginx.configure(proxyHostModel, "proxy_host", row);

host_type picks both the template and the output path: generateConfig() reads templates/${nice_host_type}.conf and writes /data/nginx/${nice_host_type}/${id}.conf. Each host type has its own id sequence, so redirection host 1, 404 host 1 and stream 1 all end up pointed at /data/nginx/proxy_host/1.conf.

For every redirection host, 404 host and stream, the script deletes the config of the proxy host that happens to share the id, renders the row through proxy_host.conf (which has no forward_scheme, forward_host or forward_port to fill in), fails nginx -t, and writes the error into the proxy_host row's meta.

The proxy host ends up offline and blamed for a config it never had. Its own meta is overwritten, and the redirection host's meta is never updated.

Reproduction

Stock 2.15.1 container with one proxy host and one redirection host, both id 1:

$ node scripts/regenerate-config -y
[Proxy Host] Regenerating config #1: proxy.example.com
[Redirection Host] Regenerating config #1: redirect.example.com
Completed

$ ls /data/nginx/proxy_host/
(empty)

sqlite> select meta from proxy_host where id = 1;
{"nginx_online":false,"nginx_err":"nginx: [emerg] invalid number of arguments in
\"set\" directive in /data/nginx/proxy_host/1.conf:14\n..."}

proxy.example.com stops being served, and the error recorded against it points at line 14 of a file that was generated from the redirection host row. Nothing is left on disk to show what happened, which is #5812.

With this change, on the same instance:

$ node scripts/regenerate-config -y
$ ls /data/nginx/proxy_host/ /data/nginx/redirection_host/
/data/nginx/proxy_host/:       1.conf
/data/nginx/redirection_host/: 1.conf

$ grep server_name /data/nginx/proxy_host/1.conf /data/nginx/redirection_host/1.conf
  server_name proxy.example.com;
  server_name redirect.example.com;

proxy_host 1       {"nginx_online":true,"nginx_err":null}
redirection_host 1 {"nginx_online":true,"nginx_err":null}

Fix

Pass the model and the host type of the rows being processed. type keeps its current job as the label used in log lines.

processItems() passed proxyHostModel and the literal "proxy_host" to
configure() for every host type. host_type selects both the template and
the output path, and each host type has its own id sequence, so
redirection hosts, 404 hosts and streams were rendered through
proxy_host.conf and written over /data/nginx/proxy_host/<id>.conf. The
proxy host sharing that id lost its config file and had the resulting
nginx error recorded in its own meta.
@nginxproxymanagerci

Copy link
Copy Markdown

Docker Image for build 1 is available on DockerHub:

nginxproxymanager/nginx-proxy-manager-dev:pr-5813

Note

Ensure you backup your NPM instance before testing this image! Especially if there are database changes.
This is a different docker image namespace than the official image.

Warning

Changes and additions to DNS Providers require verification by at least 2 members of the community!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant