Support subdomain and paths in process labels

[dlee]

Support targeting specific subdomain and paths:

[www.myapp]
directory = /Users/bob/rails/
command = rails s -p $PORT

[www.myapp/search]
directory = /Users/bob/search/
command = searchd -p $PORT

[www.myapp/chat]
directory = /Users/bob/chat/
command = chatd -p $PORT

[myapp/api]
directory = /Users/bob/api/
command = rails s -p $PORT

The routing first resolves the subdomain (longest matching suffix) and then resolves the path (longest matching prefix). For example:

blah.myapp.dev/stuff      => 404
blah.myapp.dev/api/stuff  => [myapp/api]
www.myapp.dev/api/stuff   => [www.myapp]
www.myapp.dev/chat/stuff  => [www.myapp/chat]
www.myapp.dev/chat2/stuff => [www.myapp]

[dlee]

Fixes #121

[nisanthchunduru]

This is great. Not sure about messing with process labels. May be invoker.ini can look like

[myapp]
directory = /Users/bob/rails/
command = rails s -p $PORT
location = www.myapp

[search]
directory = /Users/bob/search/
command = searchd -p $PORT
location = www.myapp/search

[chat]
directory = /Users/bob/chat/
command = chatd -p $PORT
location = www.myapp/chat

...

[dlee]

This PR doesn't anything fundamental about process labels. We're just making the DnsCheck handle a subset of them more robustly (previously, processes were still allowed to have names with / or ., but there was no way to proxy to them).

If we don't like having / or . in the process name, should we just ban them from the start? We can do that in one of the parsing stages in https://github.com/code-mancers/invoker/blob/master/lib/invoker/parsers/config.rb.

I think location is a good idea (I had also been considering that), especially in order to support multiple locations:

[chat]
directory = /Users/bob/chat/
command = chatd -p $PORT
location = www.myapp/chat chat.myapp support.myapp/chat

[dlee]

Another part to consider is the add_http command. Do you want to support invoker add_http www.myapp/chat 8000?

If so, the easiest way would be to support having subdomains and paths in the process name.

The alternative would be to change the add_http command so that the first argument is not a process name but a location.

[dlee]

I added support for location. We can now do what you suggested:

[myapp]
directory = /Users/bob/rails/
command = rails s -p $PORT
location = www.myapp

[search]
directory = /Users/bob/search/
command = searchd -p $PORT
location = www.myapp/search

[chat]
directory = /Users/bob/chat/
command = chatd -p $PORT
location = www.myapp/chat

[gnufied]

Thank you for this, I will try to review and merge this one this week.

[dlee]

Hi, were you able to take a look at this?

[gnufied]

This is sort of unrelated to this change isn't it? Do you think this should be a good idea in general or the path matching specifically made something worse, thereby highlighting a bug we haven't seen before?

[dlee]

This is actually required since the previously, there was a one-to-one mapping between incoming domain and backend. Now, since a single incoming domain could have multiple backends depending on the path, we need to make sure that the backend is not persisted across the same request coming in from the same domain.

[gnufied]

@dlee thank you so much for writing this largish patch. It looks mostly good. I am so sorry for not able to review the patch until today. But hopefully, I am still not too late. :-)

[dlee]

No worries, thanks for taking a look!

[localredhead]

Is this functionality supported with Procfiles too? or is this an INI only feature?

[gnufied]

Currently the way this is shaping up to be, this is looking like a ini file only feature. But - there are still some problems with this work actually. One of them is, after bit of thought I am not liking duplication in process name label and explicit location field inside ini file. I think, these can be unified. Obviously using location won't work because then the app names itself won't be different and hence path must be part of process label too. But then specifying location field separately becomes pointless.

[localredhead]

#129 I think this PR provides everything I need in terms of the functionality that I need mentioned in this PR for subdomains.

@gnufied I would hesitate to break away from Procfile's, but that is just my opinion. I heavily use them ;) If only there was a way to export a procfile to ini -- is there?

I do think procfile's are more likely used by those who are switching from something like POW which makes the friction of switching almost 0.

[dlee]

@gnufied An important distinction with location and the process label is that the location may have multiple space-delimited entries, whereas it only makes sense to have a singular process label.

@localredhead I'm not sure what you mean by Procfiles being more likely to be used by pow users. As far as I can tell (I being a former pow user), pow doesn't use Procfiles?

[pboling]

He means that often people using pow also use procfiles to manage their menagerie of services (we work on the same team), and that can include other web apps performing a service role.

[pboling]

because Heroku

[gnufied]

@dlee do you think #129 provides enough features to not have this feature? I know one is redirection over subdomain while another is over request path, but I suppose they can serve similar purpose in development environment.

Obviously #129 works for Procfiles as well and if intention was to share cookies etc by having same domain, that can be achieved by using subdomains too.

[dlee]

@gnufied having different apps serve different request paths is pretty important for me, but it doesn't have to be through this PR.

It could also be achieved via another way, perhaps through a more general rewrite mechanism.

[dlee]

Hi @gnufied, it's been three weeks since our last discussion and I didn't want to let this fall through the cracks. What were you were planning on doing for request path support?

[alexpeattie]

+1 to #129 😄 👍

[lcmen]

Any chances to get it merged? It would be cool to have support for specifying location in order to use subdomains for different processes.

[iffyuva]

+1 for this feature if we wont have to fiddle with nginx locally!