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
Added VIRTUAL_HOST_ALIAS support #1369
base: main
Are you sure you want to change the base?
Added VIRTUAL_HOST_ALIAS support #1369
Conversation
Do you guys gave any plans to merge this? I would like to use it, but without having to build my own image. |
@radoslav-stefanov you can use dannycarrera/nginx-proxy until this gets merged |
Very interested in seeing this merged as well. |
I think this PR should be updated according to #1338. |
nginx.tmpl
Outdated
{{ end }} | ||
access_log /var/log/nginx/access.log vhost; | ||
return 301 https://{{ $first_host }}$request_uri; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the http and the https server blocks can be merged into a single one.
Besides that it would be good to support the new variables fo http and https port. (rebase to current master to get these)
The new block could look something like that:
server {
 server_name {{ $host_alias }};
{{ if eq $https_method "redirect" }}
listen {{ $external_http_port }} {{ $default_server }};
{{ end }}
 listen {{ $external_https_port }} ssl http2 {{ $default_server }};
 {{ if $enable_ipv6 }}
{{ if eq $https_method "redirect" }}
listen [::]:{{ $external_http_port }} {{ $default_server }};
{{ end }}
 listen [::]:{{ $external_https_port }} ssl http2 {{ $default_server }};
 {{ end }}
 access_log /var/log/nginx/access.log vhost;
# ... (rest of the ssl block)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review @aiomaster . I've merged the 2 blocks together.
Do you also suggest replacing all other instances where http and https hardcoded ports with the new variables?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes the ports 80 and 443 should be replaced with the corresponding variables everywhere. My snippet was just an example and not a complete full review. Thanks for updating this.
Hi, nice feature. Does it also work when I have multiple entries in VIRTUAL_HOST like
This is how I use it usually because my DNS for the domains point to the same IP and I combine more names in on VIRTUAL_HOST. Cheers Jens |
@jheinitz yes, it works with multiple entries separated by a comma. |
Perfect. Any estimate of merging this? |
@dannycarrera please see aiomaster's review of your PR 😇 |
e3d9830
to
b4b5d63
Compare
Hi @dannycarrera thank you for working on this. I am encountering a problem when using the new template file. I am using a 3 container setup (nginx, docker-gen, and JrCs LE container) , and am using the .tmpl file from: My objective is to have the non-www URL redirect to the www URL, with the minimum of redirects, and have that happen at the first entry point (the nginx-proxy). I have added the following lines to my app docker-compose: When I bring up my app docker-compose, I can see the following errors: /etc/nginx/certs/www.URL.com /app During handling of the above exception, another exception occurred: Traceback (most recent call last): During handling of the above exception, another exception occurred: Traceback (most recent call last): Unhandled error has happened, traceback is above Debugging tips: -v improves output verbosity. Help is available under --help. |
I hope that adding a few more details can be useful, and diagnose whether the issue is with my config, or a bug in the proposed solution. I wanted to note that prior to trying the new template file, I was not having problems with the services running on my server, getting certificates. However, none of the services had more than 1 URL. If I examine my default.conf file that is created, and look at one of the previous services the blocks are as follows:
The blocks that are created using the new tmpl file look quite different (Is that because it encountered errors when the service came up?):
|
@MarkErik unfortunately I'm not an expert with nginx. The only suggestions I have are:
|
Hi @dannycarrera thank you for your response. My goal is to have a solution to redirecting URLs with the minimum of redirects, and this seemed like the right approach to handle it at the proxy level. However, in the meantime I have adopted a workaround solution using a second nginx container at the app service level that simply redirects incoming requests to the desired URL. Regarding your suggestion 1, is that the configuration you tested with? (The two containers for the proxy being a plain Nginx container and the LetsE-Nginx-Proxy-companion) |
@MarkErik You will need to tweak the template as in #750 to detect the proxy container. The 3 containers setup gives you more flexibility, but also requires more effort. I am using a 3 containers setup with a customized version of an updated template file available at https://github.com/sgabe/nginx-proxy/blob/master/nginx.tmpl which also includes this modification and might help you. |
@sgabe thank you for responding and pointing me at the other .tmpl file. I looked it over and I'm worried that since I am not too familiar with the structure of the file, and that as it has some additions, and is also a few commits behind the master, that I would not be able and comfortable to put it into my setup. I'll stick with an additional 'redirect' container for now, and keep an eye on the idea of a new .tmpl file. |
…st_alias Feature/virtual host alias
@dannycarrera Thank you for this PR. IMO this is an essential feature if one wants to run this in production. IIRC:
I'd like to put some minor changes up for discussion: I tested this like above and it seemed ok to me. @dannycarrera If you like the changes, feel free to please integrate them in your PR, which will hopefully going to be merged sometime. |
@shiz0 I've been using: https://github.com/cusspvz/redirect.docker to do the redirect (I include it as a service in my project's docker-compose file) and it has been sufficient as a solution. If you are curious here are the Nginx portions of my Ghost blog's docker-compose (the outside network is the one that is shared between the proxy and the containers that need to communicate with it):
|
@dannycarrera Since this reply has taken me almost two months, I can say I know too well. :-p |
+1 for merging this feature |
Hello All, Maybe somebody can help me? I also would like to do an redirection but anyway, doesn't work.
But when I'm trying with
With this last config, all my site lose certificate and are not available. Or worst, for About My DNS, all are an Maybe someone has an idea what I'm doing wrong? Thanks for your help. |
I found my issue there: |
You should try the version from @shiz0: https://github.com/shiz0/nginx-proxy/blob/virtual_host_alias/nginx.tmpl I don't have those issues with it. |
@buchdag - This has been open for a while. I believe there is still interest in this feature, but it's held up by lack of tests (based on the pr-needs-tests label). If testing were written, could this get merged? |
@cynicated yep, useful features like this one will be merged if they have proper tests and documentation (provided they don't introduce backward incompatible behaviour of course). However the |
+1. Domain Aliases should be a core feature. |
for what it's worth, i'm also +1 ing this pr. domain aliases out-of-the box (without maintainng a priv.fork) would be very useful |
Is there a dockerhub repo with an image of this MR I can pull from? I pulled this MR myself and ran docker build but the image it produced didn't run on my droplet. Also +1 this PR. I've tried like, 5 different ways to redirect www traffic to non-www for my domain and none of them have worked so far so hoping this can do it! |
@SKeeneCode hub.docker.com/r/dannycarrera/nginx-proxy
|
Hello @dannycarrera is it nowadays safe to use the dannycarrera/nginx-proxy to get the VIRTUAL_HOST_ALIAS feature, given that the fork is 295 commits behind nginx-proxy:main? UPDATE: it seems not. We are using |
@paloha You could try my version, (https://github.com/shiz0/nginx-proxy) |
@shiz0 & @dannycarrera thanks a lot for your replies. Before you replied, I already solved it by using the |
@buchdag any chance to merge this? It's almost 4 years |
Unfortunately, this no longer works with the latest version of nginx-proxy and a few nginx warnings are issued. In my opinion, using another container for redirecting is not a good solution. I and certainly many others would be very happy if this feature were to become a reality soon. Unfortunately I can't help much, so many thanks in advance! |
Oof.. yeah I did not update my fork for a while and it's like 300 commits behind with lot of conflicts. |
# Conflicts: # README.md # nginx.tmpl
This feature adds support for VIRTUAL_HOST_ALIAS and is in response to issues #180, #958 and #1204.
You can add aliases that will redirect (301) to the first entry in
VIRTUAL_HOST
by adding theVIRTUAL_HOST_ALIAS
env var:This will setup the following redirects:
http://www.example.com
→http://example.com
http://old.example.com
→http://example.com
If you are using letsencrypt-nginx-proxy-companion for SSL support, then you would run:
This will setup the following redirects:
http://example.com
→https://example.com
http://www.example.com
→https://example.com
http://old.example.com
→https://example.com
https://www.example.com
→https://example.com
https://old.example.com
→https://example.com