Changed filenames to include first domain or inbound port

[joewesch]

Closes: #746

This PR is an upgraded replacement for #2606.

This PR changes all config filenames to be id.first_domain for proxy_host, redirection_host or dead_host and id.incoming_port for stream. Each type will fall back to just id if there are no domains or if the type is undefined. All log files will also be renamed accordingly automatically.

# Before
[root@docker-a8439ada89f2:/app]# ls /data/nginx/proxy_host/
1.conf
[root@docker-a8439ada89f2:/app]# ls /data/logs/
fallback_error.log
fallback_http_access.log
fallback_http_error.log
fallback_stream_access.log
proxy-host-1_access.log
proxy-host-1_error.log

# After
[root@docker-a8439ada89f2:/app]# ls /data/nginx/proxy_host/
1.test.example.com.conf
[root@docker-a8439ada89f2:/app]# ls /data/logs/
fallback_error.log
fallback_http_access.log
fallback_http_error.log
fallback_stream_access.log
proxy-host-1.test.example.com_access.log
proxy-host-1.test.example.com_error.log

This will also happen if the user either adds a domain to the host that sorts alphabetically first or if they remove the first domain. These changes will not take effect immediately, which seems to be in-line with the rest of the upgrades in this project. They will instead update the next time each host is saved.

[nginxproxymanagerci]

Docker Image for build 1 is available on DockerHub:

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

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!

[jc21]

Thanks for the contribution. I've asked Claude for a review and here's the parts I agree with:

---

Issues

1. Bug: letsencryptTempConfigFilenamesToDelete only deletes letsencrypt_{id}.{slug}.conf, not letsencrypt_{id}.conf

// backend/lib/utils.js
const prefix = `letsencrypt_${certificateId}.`;
return dirEntries.filter((name) => name.startsWith(prefix) && name.endsWith(".conf"));

"letsencrypt_3.conf".startsWith("letsencrypt_3.") is actually true (the .conf follows the .), so this does work. However, the test for this function doesn't exercise the bare letsencrypt_{id}.conf form with deleteErrFile=false/true, so the coverage is incomplete. Worth adding.

2. Mutation side-effect on host object

// backend/internal/nginx.js:122
host.nginx_file_stem = utils.getNginxFileStem(nice_host_type, host);

generateConfig mutates the caller's object to inject nginx_file_stem for template rendering. This is a hidden side-effect — callers don't expect their host object to gain a new property. The stem could instead be passed as a separate template variable rather than attaching it to host.

[joewesch]

generateConfig mutates the caller's object to inject nginx_file_stem for template rendering.

This is actually incorrect. The host constant was already created as a clone (deep copy) of the host_row from the caller just a few lines above to prevent this exact situation (hence the comment):

nginx-proxy-manager/backend/internal/nginx.js

Lines 188 to 190 in 9875a76

	generateConfig: (host_type, host_row) => {
	// Prevent modifying the original object:
	const host = JSON.parse(JSON.stringify(host_row));

[joewesch]

Bug: letsencryptTempConfigFilenamesToDelete only deletes letsencrypt_{id}.{slug}.conf, not letsencrypt_{id}.conf

However, the test for this function doesn't exercise the bare letsencrypt_{id}.conf form with deleteErrFile=false/true, so the coverage is incomplete. Worth adding.

Claude seems to be hallucinating here, I think? The deleteErrFile param is part of diskConfigFilenamesToDelete, not letsencryptTempConfigFilenamesToDelete. Regardless, I explicitly test the bare ID in both cases. I have 1.conf on line 94 and letsencrypt_3.conf on line 106.