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

Allow VIRTUAL_PATH to run multiple upstreams and target them appropriately #1257

Closed
wants to merge 4 commits into from
Closed

Conversation

christhomas
Copy link

This modification allows you to target a VIRTUAL_HOST multiple times in various containers, with different VIRTUAL_PATH environment variables. Letting you specify multiple containers to run under the same "domain name" with different subpaths. We use this at our company and seems to be working correctly.

@christhomas
Copy link
Author

Any interest in merging these changes? I've got a prebuilt image on https://hub.docker.com/r/christhomas/nginx-proxy that demonstrates the solution. I hope that it works for all cases. But it works for ours and doesn't have any problems so far

@m4ci3k2
Copy link

m4ci3k2 commented Apr 22, 2019

@christhomas I've tested it and works very well for me, but I also needed a small subfeature to strip the VIRTUAL_PATH from upstream urls. I've just made a pull request on your fork for that (but if there is a better workflow for such suggestions in github please do tell me).

@jeffshpe
Copy link

Our organization does not allow subdomains. We can assign multiple hostnames to the same IP, which gets around the lack of subdomains, but it requires a ticket to be created for each hostname. Having this feature where you can use part of the URL to redirect would be great for enabling quick testing for me, I'd love to see something like this merged in to the nginx-proxy image. It would be good to have the option of stripping the additional path out, such as what m4ci3k2 has added.

@christhomas
Copy link
Author

try out my image jeff and see if that works in the way that you want, or that we need to change something, I'm happily accepting modifications if they are good

@jeffshpe
Copy link

It looks like this might need some enhancement to handle HTTPS_METHOD and redirection. Maybe it's asking too much, but I was trying to get a site which only uses HTTPS to co-exist with a site which uses HTTP with this, which didn't quite work. I also found that some of the containers I was attempting to host serve static files without relative paths, which leads to 404s when trying to retrieve the resources, so it might be more effort than it's worth.

@jplock
Copy link

jplock commented Nov 1, 2019

@christhomas is it possible to rebase this and have @jwilder review? this would be a great additional for a use-case I just ran into today.

@christhomas
Copy link
Author

You can try out my docker image which is already pre-built with this functionality and then at least you can try it today

@christhomas
Copy link
Author

@buchdag I have synchronised my fork with your changes and re-applied the ability to specify the VIRTUAL_PATH on containers, which allows for a single VIRTUAL_HOST to support multiple containers mounted at various paths instead of only allowing one container per VIRTUAL_HOST.

It's a solution that I've tried to get into the repository 2 years ago and I use everyday for my company and in my personal projects. But the original maintainer was not very responsive.

Are you interested in trying this out and letting me know how this works for you and perhaps merging it? Then I don't have to support my own custom version for this feature.

We use this feature extensively at our company because we have a single virtual host api.company.com and we host multiple microservices based off of paths on that virtual host. You can imagine REST endpoints inside that api, all being separate containers.

It's a very useful modification. What are your thoughts?

@buchdag
Copy link
Member

buchdag commented Jun 15, 2021

@christhomas your approach is interesting, unfortunately it seems to be breaking basic multiple virtual host functionality.

Failing tests are test_unknown_virtual_host_is_503, test_webA_is_forwarded and test_webB_is_forwarded from test_multiple-hosts.py.

@buchdag
Copy link
Member

buchdag commented Jun 15, 2021

Also, we're working toward a similar feature in #1607

@christhomas
Copy link
Author

christhomas commented Jun 16, 2021 via email

@buchdag buchdag added the scope/path-based-routing Issue or PR related to path based routing label Jul 16, 2021
@buchdag
Copy link
Member

buchdag commented Jul 19, 2021

This features has been added to the dev branch with #1607

It's available now on the dev images nginxproxy/nginx-proxy:dev and nginxproxy/nginx-proxy:dev-alpine. Testing and feedback are welcome, the docs for the feature is here and there is another PR opened for additional improvements and comments.

@buchdag buchdag closed this Jul 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope/path-based-routing Issue or PR related to path based routing status/needs-more-info type/feat PR for a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants