Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for custom server directive using default_server broken by 9b4bb07 #2212

Closed
Enchiridion opened this issue Apr 4, 2023 · 5 comments · Fixed by #2214
Closed

Support for custom server directive using default_server broken by 9b4bb07 #2212

Enchiridion opened this issue Apr 4, 2023 · 5 comments · Fixed by #2214
Labels
kind/bug Issue reporting a bug

Comments

@Enchiridion
Copy link

Enchiridion commented Apr 4, 2023

I run several proxied sites simultaneously, to make it easier to see which sites are active I'm using docker-gen with a custom .tmpl file to auto-generate an html file, with links and tools for each site, whenever sites are started/stopped. I'm displaying the page at http://localhost/, to do so my custom nginx config takes over being the default server. I don't override /etc/nginx/conf.d/default.conf, I named mine so it loads first. My custom config is really small:

/etc/nginx/conf.d/01-custom.conf

server {
  server_name __; # This is just an invalid value which will never trigger on a real hostname.
  listen 80 default_server;
  access_log /var/log/nginx/access.log;
  root /app/web/public;
  index index.htm;
}

An update in commit 9b4bb07 broke it because it added default_server on lines 406, 408, 412, and 414. Nginx throws an error if there are multiple default servers for the same port.

From what I've read in the nginx docs, adding default_server is unnecessary in default.conf since the first server block becomes the default and default_server is only needed if you want to override that, which is what I need to do. I don't know enough about the codebase of nginx-proxy to know if removing those now would be ok or not, else I'd make a PR to remove them.

In the meantime, I've made a copy of nginx.tmpl, removed default_server, and replaced it in container using my Dockerfile, but it's not a long-term solution.

@buchdag
Copy link
Member

buchdag commented Apr 5, 2023

@rhansen could you take a look at this ?

@buchdag
Copy link
Member

buchdag commented Apr 5, 2023

@Enchiridion I suspect you're using a method to get a default virtual host that previously only worked "by accident".

The correct, tested method is using DEFAULT_HOST

@rhansen rhansen changed the title Commit 9b4bb07 broke my nginx config Support for custom server directive using default_server broken by 9b4bb07 Apr 7, 2023
@rhansen
Copy link
Collaborator

rhansen commented Apr 7, 2023

While users could just run a new container and point DEFAULT_HOST at it, it seems reasonable to me to support users that want a custom server directive, not a container, as the fallback handler. I'll open a PR with a fix and test coverage.

@Enchiridion
Copy link
Author

Thanks for looking into and fixing it @buchdag and @rhansen !

@buchdag
Copy link
Member

buchdag commented Apr 11, 2023

@Enchiridion could you try the fix proposed by @rhansen in #2214 and tell us if it works for you ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Issue reporting a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants